| Message ID | 20200508154343.6074-2-m.grzeschik@pengutronix.de |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | microchip: add support for ksz88x3 driver family | expand |
On 5/8/2020 8:43 AM, Michael Grzeschik wrote: > From: Andrew Lunn <andrew@lunn.ch> > > SMI0 is a mangled version of MDIO. The main low level difference is > the MDIO C22 OP code is always 0, not 0x2 or 0x1 for Read/Write. The > read/write information is instead encoded in the PHY address. > > Extend the bit-bang code to allow the op code to be overridden, but > default to normal C22 values. Add an extra compatible to the mdio-gpio > driver, and when this compatible is present, set the op codes to 0. > > A higher level driver, sitting on top of the basic MDIO bus driver can > then implement the rest of the microchip SMI0 odderties. > > Signed-off-by: Andrew Lunn <andrew@lunn.ch> > Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de> > --- > drivers/net/phy/mdio-bitbang.c | 7 ++----- > drivers/net/phy/mdio-gpio.c | 13 +++++++++++++ > include/linux/mdio-bitbang.h | 2 ++ > 3 files changed, 17 insertions(+), 5 deletions(-) > > diff --git a/drivers/net/phy/mdio-bitbang.c b/drivers/net/phy/mdio-bitbang.c > index 5136275c8e7399..11255460ecb933 100644 > --- a/drivers/net/phy/mdio-bitbang.c > +++ b/drivers/net/phy/mdio-bitbang.c > @@ -19,9 +19,6 @@ > #include <linux/types.h> > #include <linux/delay.h> > > -#define MDIO_READ 2 > -#define MDIO_WRITE 1 > - > #define MDIO_C45 (1<<15) > #define MDIO_C45_ADDR (MDIO_C45 | 0) > #define MDIO_C45_READ (MDIO_C45 | 3) > @@ -158,7 +155,7 @@ static int mdiobb_read(struct mii_bus *bus, int phy, int reg) > reg = mdiobb_cmd_addr(ctrl, phy, reg); > mdiobb_cmd(ctrl, MDIO_C45_READ, phy, reg); > } else > - mdiobb_cmd(ctrl, MDIO_READ, phy, reg); > + mdiobb_cmd(ctrl, ctrl->op_c22_read, phy, reg); > > ctrl->ops->set_mdio_dir(ctrl, 0); > > @@ -189,7 +186,7 @@ static int mdiobb_write(struct mii_bus *bus, int phy, int reg, u16 val) > reg = mdiobb_cmd_addr(ctrl, phy, reg); > mdiobb_cmd(ctrl, MDIO_C45_WRITE, phy, reg); > } else > - mdiobb_cmd(ctrl, MDIO_WRITE, phy, reg); > + mdiobb_cmd(ctrl, ctrl->op_c22_write, phy, reg); There are other users of the mdio-bitbang.c file which I believe you are going to break here because they will not initialize op_c22_write or op_c22_read, and thus they will be using 0, instead of MDIO_READ and MDIO_WRITE. I believe you need something like the patch attached.
> > - mdiobb_cmd(ctrl, MDIO_WRITE, phy, reg); > > + mdiobb_cmd(ctrl, ctrl->op_c22_write, phy, reg); > > There are other users of the mdio-bitbang.c file which I believe you are > going to break here because they will not initialize op_c22_write or > op_c22_read, and thus they will be using 0, instead of MDIO_READ and > MDIO_WRITE. I believe you need something like the patch attached. Ah, totally missed that: https://elixir.bootlin.com/linux/latest/source/arch/powerpc/platforms/82xx/ep8248e.c#L98 https://elixir.bootlin.com/linux/latest/source/drivers/net/ethernet/8390/ax88796.c#L444 https://elixir.bootlin.com/linux/latest/source/drivers/net/ethernet/freescale/fs_enet/mii-bitbang.c#L103 https://elixir.bootlin.com/linux/latest/source/drivers/net/ethernet/renesas/ravb_main.c#L165 https://elixir.bootlin.com/linux/latest/source/drivers/net/ethernet/renesas/sh_eth.c#L1257 Andrew
On Sat, May 09, 2020 at 10:28:05AM -0700, Florian Fainelli wrote: > > >On 5/8/2020 8:43 AM, Michael Grzeschik wrote: >>From: Andrew Lunn <andrew@lunn.ch> >> >>SMI0 is a mangled version of MDIO. The main low level difference is >>the MDIO C22 OP code is always 0, not 0x2 or 0x1 for Read/Write. The >>read/write information is instead encoded in the PHY address. >> >>Extend the bit-bang code to allow the op code to be overridden, but >>default to normal C22 values. Add an extra compatible to the mdio-gpio >>driver, and when this compatible is present, set the op codes to 0. >> >>A higher level driver, sitting on top of the basic MDIO bus driver can >>then implement the rest of the microchip SMI0 odderties. >> >>Signed-off-by: Andrew Lunn <andrew@lunn.ch> >>Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de> >>--- >> drivers/net/phy/mdio-bitbang.c | 7 ++----- >> drivers/net/phy/mdio-gpio.c | 13 +++++++++++++ >> include/linux/mdio-bitbang.h | 2 ++ >> 3 files changed, 17 insertions(+), 5 deletions(-) >> >>diff --git a/drivers/net/phy/mdio-bitbang.c b/drivers/net/phy/mdio-bitbang.c >>index 5136275c8e7399..11255460ecb933 100644 >>--- a/drivers/net/phy/mdio-bitbang.c >>+++ b/drivers/net/phy/mdio-bitbang.c >>@@ -19,9 +19,6 @@ >> #include <linux/types.h> >> #include <linux/delay.h> >>-#define MDIO_READ 2 >>-#define MDIO_WRITE 1 >>- >> #define MDIO_C45 (1<<15) >> #define MDIO_C45_ADDR (MDIO_C45 | 0) >> #define MDIO_C45_READ (MDIO_C45 | 3) >>@@ -158,7 +155,7 @@ static int mdiobb_read(struct mii_bus *bus, int phy, int reg) >> reg = mdiobb_cmd_addr(ctrl, phy, reg); >> mdiobb_cmd(ctrl, MDIO_C45_READ, phy, reg); >> } else >>- mdiobb_cmd(ctrl, MDIO_READ, phy, reg); >>+ mdiobb_cmd(ctrl, ctrl->op_c22_read, phy, reg); >> ctrl->ops->set_mdio_dir(ctrl, 0); >>@@ -189,7 +186,7 @@ static int mdiobb_write(struct mii_bus *bus, int phy, int reg, u16 val) >> reg = mdiobb_cmd_addr(ctrl, phy, reg); >> mdiobb_cmd(ctrl, MDIO_C45_WRITE, phy, reg); >> } else >>- mdiobb_cmd(ctrl, MDIO_WRITE, phy, reg); >>+ mdiobb_cmd(ctrl, ctrl->op_c22_write, phy, reg); > >There are other users of the mdio-bitbang.c file which I believe you >are going to break here because they will not initialize op_c22_write >or op_c22_read, and thus they will be using 0, instead of MDIO_READ >and MDIO_WRITE. I believe you need something like the patch attached. >-- >Florian I will add that change to v4. Michael >diff --git a/drivers/net/phy/mdio-bitbang.c b/drivers/net/phy/mdio-bitbang.c >index 11255460ecb9..528e255d1ffe 100644 >--- a/drivers/net/phy/mdio-bitbang.c >+++ b/drivers/net/phy/mdio-bitbang.c >@@ -19,6 +19,9 @@ > #include <linux/types.h> > #include <linux/delay.h> > >+#define MDIO_READ 2 >+#define MDIO_WRITE 1 >+ > #define MDIO_C45 (1<<15) > #define MDIO_C45_ADDR (MDIO_C45 | 0) > #define MDIO_C45_READ (MDIO_C45 | 3) >@@ -212,6 +215,10 @@ struct mii_bus *alloc_mdio_bitbang(struct mdiobb_ctrl *ctrl) > bus->read = mdiobb_read; > bus->write = mdiobb_write; > bus->priv = ctrl; >+ if (!ctrl->override_op_c22) { >+ ctrl->op_c22_read = MDIO_READ; >+ ctrl->op_c22_write = MDIO_WRITE; >+ } > > return bus; > } >diff --git a/drivers/net/phy/mdio-gpio.c b/drivers/net/phy/mdio-gpio.c >index d85bc1a98647..13ec31e89e94 100644 >--- a/drivers/net/phy/mdio-gpio.c >+++ b/drivers/net/phy/mdio-gpio.c >@@ -27,9 +27,6 @@ > #include <linux/gpio/consumer.h> > #include <linux/of_mdio.h> > >-#define MDIO_READ 2 >-#define MDIO_WRITE 1 >- > struct mdio_gpio_info { > struct mdiobb_ctrl ctrl; > struct gpio_desc *mdc, *mdio, *mdo; >@@ -139,9 +136,7 @@ static struct mii_bus *mdio_gpio_bus_init(struct device *dev, > of_device_is_compatible(dev->of_node, "microchip,mdio-smi0")) { > bitbang->ctrl.op_c22_read = 0; > bitbang->ctrl.op_c22_write = 0; >- } else { >- bitbang->ctrl.op_c22_read = MDIO_READ; >- bitbang->ctrl.op_c22_write = MDIO_WRITE; >+ bitbang->ctrl.override_op_c22 = 1; > } > > dev_set_drvdata(dev, new_bus); >diff --git a/include/linux/mdio-bitbang.h b/include/linux/mdio-bitbang.h >index 8ae0b3835233..5016e6f60de3 100644 >--- a/include/linux/mdio-bitbang.h >+++ b/include/linux/mdio-bitbang.h >@@ -33,6 +33,7 @@ struct mdiobb_ops { > > struct mdiobb_ctrl { > const struct mdiobb_ops *ops; >+ unsigned int override_op_c22; > u8 op_c22_read; > u8 op_c22_write; > };
diff --git a/drivers/net/phy/mdio-bitbang.c b/drivers/net/phy/mdio-bitbang.c index 5136275c8e7399..11255460ecb933 100644 --- a/drivers/net/phy/mdio-bitbang.c +++ b/drivers/net/phy/mdio-bitbang.c @@ -19,9 +19,6 @@ #include <linux/types.h> #include <linux/delay.h> -#define MDIO_READ 2 -#define MDIO_WRITE 1 - #define MDIO_C45 (1<<15) #define MDIO_C45_ADDR (MDIO_C45 | 0) #define MDIO_C45_READ (MDIO_C45 | 3) @@ -158,7 +155,7 @@ static int mdiobb_read(struct mii_bus *bus, int phy, int reg) reg = mdiobb_cmd_addr(ctrl, phy, reg); mdiobb_cmd(ctrl, MDIO_C45_READ, phy, reg); } else - mdiobb_cmd(ctrl, MDIO_READ, phy, reg); + mdiobb_cmd(ctrl, ctrl->op_c22_read, phy, reg); ctrl->ops->set_mdio_dir(ctrl, 0); @@ -189,7 +186,7 @@ static int mdiobb_write(struct mii_bus *bus, int phy, int reg, u16 val) reg = mdiobb_cmd_addr(ctrl, phy, reg); mdiobb_cmd(ctrl, MDIO_C45_WRITE, phy, reg); } else - mdiobb_cmd(ctrl, MDIO_WRITE, phy, reg); + mdiobb_cmd(ctrl, ctrl->op_c22_write, phy, reg); /* send the turnaround (10) */ mdiobb_send_bit(ctrl, 1); diff --git a/drivers/net/phy/mdio-gpio.c b/drivers/net/phy/mdio-gpio.c index 1b00235d7dc5b5..d85bc1a98647e2 100644 --- a/drivers/net/phy/mdio-gpio.c +++ b/drivers/net/phy/mdio-gpio.c @@ -27,6 +27,9 @@ #include <linux/gpio/consumer.h> #include <linux/of_mdio.h> +#define MDIO_READ 2 +#define MDIO_WRITE 1 + struct mdio_gpio_info { struct mdiobb_ctrl ctrl; struct gpio_desc *mdc, *mdio, *mdo; @@ -132,6 +135,15 @@ static struct mii_bus *mdio_gpio_bus_init(struct device *dev, new_bus->phy_ignore_ta_mask = pdata->phy_ignore_ta_mask; } + if (dev->of_node && + of_device_is_compatible(dev->of_node, "microchip,mdio-smi0")) { + bitbang->ctrl.op_c22_read = 0; + bitbang->ctrl.op_c22_write = 0; + } else { + bitbang->ctrl.op_c22_read = MDIO_READ; + bitbang->ctrl.op_c22_write = MDIO_WRITE; + } + dev_set_drvdata(dev, new_bus); return new_bus; @@ -196,6 +208,7 @@ static int mdio_gpio_remove(struct platform_device *pdev) static const struct of_device_id mdio_gpio_of_match[] = { { .compatible = "virtual,mdio-gpio", }, + { .compatible = "microchip,mdio-smi0" }, { /* sentinel */ } }; MODULE_DEVICE_TABLE(of, mdio_gpio_of_match); diff --git a/include/linux/mdio-bitbang.h b/include/linux/mdio-bitbang.h index 5d71e8a8500f5e..8ae0b383523371 100644 --- a/include/linux/mdio-bitbang.h +++ b/include/linux/mdio-bitbang.h @@ -33,6 +33,8 @@ struct mdiobb_ops { struct mdiobb_ctrl { const struct mdiobb_ops *ops; + u8 op_c22_read; + u8 op_c22_write; }; /* The returned bus is not yet registered with the phy layer. */