| Message ID | 20230524091722.522118-9-jiawenwu@trustnetic.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | TXGBE PHYLINK support | expand |
On Wed, May 24, 2023 at 05:17:21PM +0800, Jiawen Wu wrote: > Register MDIO bus for PCS layer to use Synopsys designware XPCS, support > 10GBASE-R interface to the controller. > > Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com> Reviewed-by: Andrew Lunn <andrew@lunn.ch> Andrew
On Wed, 24 May 2023 17:17:21 +0800 Jiawen Wu wrote: > + ret = devm_mdiobus_register(&pdev->dev, mii_bus); > + if (ret) > + return ret; > + > + mdiodev = mdio_device_create(mii_bus, 0); > + if (IS_ERR(mdiodev)) > + return PTR_ERR(mdiodev); > + > + xpcs = xpcs_create(mdiodev, PHY_INTERFACE_MODE_10GBASER); > + if (IS_ERR(xpcs)) { > + mdio_device_free(mdiodev); > + return PTR_ERR(xpcs); > + } How does the mdiodev get destroyed in case of success? Seems like either freeing it in case of xpcs error is unnecessary or it needs to also be freed when xpcs is destroyed?
On Friday, May 26, 2023 12:14 PM, Jakub Kicinski wrote: > On Wed, 24 May 2023 17:17:21 +0800 Jiawen Wu wrote: > > + ret = devm_mdiobus_register(&pdev->dev, mii_bus); > > + if (ret) > > + return ret; > > + > > + mdiodev = mdio_device_create(mii_bus, 0); > > + if (IS_ERR(mdiodev)) > > + return PTR_ERR(mdiodev); > > + > > + xpcs = xpcs_create(mdiodev, PHY_INTERFACE_MODE_10GBASER); > > + if (IS_ERR(xpcs)) { > > + mdio_device_free(mdiodev); > > + return PTR_ERR(xpcs); > > + } > > How does the mdiodev get destroyed in case of success? > Seems like either freeing it in case of xpcs error is unnecessary > or it needs to also be freed when xpcs is destroyed? When xpcs is destroyed, that means mdiodev is no longer needed. I think there is no need to free mdiodev in case of xpcs error, since devm_* function leads to free it.
On Fri, May 26, 2023 at 02:21:23PM +0800, Jiawen Wu wrote: > On Friday, May 26, 2023 12:14 PM, Jakub Kicinski wrote: > > On Wed, 24 May 2023 17:17:21 +0800 Jiawen Wu wrote: > > > + ret = devm_mdiobus_register(&pdev->dev, mii_bus); > > > + if (ret) > > > + return ret; > > > + > > > + mdiodev = mdio_device_create(mii_bus, 0); > > > + if (IS_ERR(mdiodev)) > > > + return PTR_ERR(mdiodev); > > > + > > > + xpcs = xpcs_create(mdiodev, PHY_INTERFACE_MODE_10GBASER); > > > + if (IS_ERR(xpcs)) { > > > + mdio_device_free(mdiodev); > > > + return PTR_ERR(xpcs); > > > + } > > > > How does the mdiodev get destroyed in case of success? > > Seems like either freeing it in case of xpcs error is unnecessary > > or it needs to also be freed when xpcs is destroyed? > > When xpcs is destroyed, that means mdiodev is no longer needed. > I think there is no need to free mdiodev in case of xpcs error, > since devm_* function leads to free it. If you are relying on the devm-ness of devm_mdiobus_register() then it won't. Although mdiobus_unregister() walks bus->mdio_map[], I think you are assuming that the mdio device you've created in mdio_device_create() will be in that array. MDIO devices only get added to that array when mdiobus_register_device() has been called, which must only be called from mdio_device_register(). Please arrange to call mdio_device_free() prior to destroying the XPCS in every case.
On Friday, May 26, 2023 4:43 PM, Russell King (Oracle) wrote: > On Fri, May 26, 2023 at 02:21:23PM +0800, Jiawen Wu wrote: > > On Friday, May 26, 2023 12:14 PM, Jakub Kicinski wrote: > > > On Wed, 24 May 2023 17:17:21 +0800 Jiawen Wu wrote: > > > > + ret = devm_mdiobus_register(&pdev->dev, mii_bus); > > > > + if (ret) > > > > + return ret; > > > > + > > > > + mdiodev = mdio_device_create(mii_bus, 0); > > > > + if (IS_ERR(mdiodev)) > > > > + return PTR_ERR(mdiodev); > > > > + > > > > + xpcs = xpcs_create(mdiodev, PHY_INTERFACE_MODE_10GBASER); > > > > + if (IS_ERR(xpcs)) { > > > > + mdio_device_free(mdiodev); > > > > + return PTR_ERR(xpcs); > > > > + } > > > > > > How does the mdiodev get destroyed in case of success? > > > Seems like either freeing it in case of xpcs error is unnecessary > > > or it needs to also be freed when xpcs is destroyed? > > > > When xpcs is destroyed, that means mdiodev is no longer needed. > > I think there is no need to free mdiodev in case of xpcs error, > > since devm_* function leads to free it. > > If you are relying on the devm-ness of devm_mdiobus_register() then > it won't. Although mdiobus_unregister() walks bus->mdio_map[], I > think you are assuming that the mdio device you've created in > mdio_device_create() will be in that array. MDIO devices only get > added to that array when mdiobus_register_device() has been called, > which must only be called from mdio_device_register(). > > Please arrange to call mdio_device_free() prior to destroying the > XPCS in every case. Get it.
On Fri, May 26, 2023 at 05:01:49PM +0800, Jiawen Wu wrote: > On Friday, May 26, 2023 4:43 PM, Russell King (Oracle) wrote: > > On Fri, May 26, 2023 at 02:21:23PM +0800, Jiawen Wu wrote: > > > On Friday, May 26, 2023 12:14 PM, Jakub Kicinski wrote: > > > > On Wed, 24 May 2023 17:17:21 +0800 Jiawen Wu wrote: > > > > > + ret = devm_mdiobus_register(&pdev->dev, mii_bus); > > > > > + if (ret) > > > > > + return ret; > > > > > + > > > > > + mdiodev = mdio_device_create(mii_bus, 0); > > > > > + if (IS_ERR(mdiodev)) > > > > > + return PTR_ERR(mdiodev); > > > > > + > > > > > + xpcs = xpcs_create(mdiodev, PHY_INTERFACE_MODE_10GBASER); > > > > > + if (IS_ERR(xpcs)) { > > > > > + mdio_device_free(mdiodev); > > > > > + return PTR_ERR(xpcs); > > > > > + } > > > > > > > > How does the mdiodev get destroyed in case of success? > > > > Seems like either freeing it in case of xpcs error is unnecessary > > > > or it needs to also be freed when xpcs is destroyed? > > > > > > When xpcs is destroyed, that means mdiodev is no longer needed. > > > I think there is no need to free mdiodev in case of xpcs error, > > > since devm_* function leads to free it. > > > > If you are relying on the devm-ness of devm_mdiobus_register() then > > it won't. Although mdiobus_unregister() walks bus->mdio_map[], I > > think you are assuming that the mdio device you've created in > > mdio_device_create() will be in that array. MDIO devices only get > > added to that array when mdiobus_register_device() has been called, > > which must only be called from mdio_device_register(). > > > > Please arrange to call mdio_device_free() prior to destroying the > > XPCS in every case. > > Get it. It seems this is becoming a pattern, so I think we need to solve it differently. How about something like this, which means you only have to care about calling xpcs_create_mdiodev() and xpcs_destroy() ? diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c index b87c69c4cdd7..802222581feb 100644 --- a/drivers/net/pcs/pcs-xpcs.c +++ b/drivers/net/pcs/pcs-xpcs.c @@ -1240,6 +1240,7 @@ struct dw_xpcs *xpcs_create(struct mdio_device *mdiodev, if (!xpcs) return ERR_PTR(-ENOMEM); + mdio_device_get(mdiodev); xpcs->mdiodev = mdiodev; xpcs_id = xpcs_get_id(xpcs); @@ -1272,6 +1273,7 @@ struct dw_xpcs *xpcs_create(struct mdio_device *mdiodev, ret = -ENODEV; out: + mdio_device_put(mdiodev); kfree(xpcs); return ERR_PTR(ret); @@ -1280,8 +1282,33 @@ EXPORT_SYMBOL_GPL(xpcs_create); void xpcs_destroy(struct dw_xpcs *xpcs) { + mdio_device_put(mdiodev); kfree(xpcs); } EXPORT_SYMBOL_GPL(xpcs_destroy); +struct dw_xpcs *xpcs_create_mdiodev(struct mii_bus *bus, int addr, + phy_interface_t interface) +{ + struct mdio_device *mdiodev; + struct dw_xpcs *xpcs; + + mdiodev = mdio_device_create(bus, addr); + if (IS_ERR(mdiodev)) + return ERR_CAST(mdiodev); + + xpcs = xpcs_create(mdiodev, interface); + + /* xpcs_create() has taken a refcount on the mdiodev if it was + * successful. If xpcs_create() fails, this will free the mdio + * device here. In any case, we don't need to hold our reference + * anymore, and putting it here will allow mdio_device_put() in + * xpcs_destroy() to automatically free the mdio device. + */ + mdio_device_put(mdiodev); + + return xpcs; +} +EXPORT_SYMBOL_GPL(xpcs_create_mdiodev); + MODULE_LICENSE("GPL v2"); diff --git a/include/linux/mdio.h b/include/linux/mdio.h index 1d7d550bbf1a..537b62330c90 100644 --- a/include/linux/mdio.h +++ b/include/linux/mdio.h @@ -108,6 +108,16 @@ int mdio_driver_register(struct mdio_driver *drv); void mdio_driver_unregister(struct mdio_driver *drv); int mdio_device_bus_match(struct device *dev, struct device_driver *drv); +static inline void mdio_device_get(struct mdio_device *mdiodev) +{ + get_device(&mdiodev->dev); +} + +static inline void mdio_device_put(struct mdio_device *mdiodev) +{ + mdio_device_free(mdiodev); +} + static inline bool mdio_phy_id_is_c45(int phy_id) { return (phy_id & MDIO_PHY_ID_C45) && !(phy_id & ~MDIO_PHY_ID_C45_MASK);
On Friday, May 26, 2023 5:07 PM, Russell King (Oracle) wrote: > On Fri, May 26, 2023 at 05:01:49PM +0800, Jiawen Wu wrote: > > On Friday, May 26, 2023 4:43 PM, Russell King (Oracle) wrote: > > > On Fri, May 26, 2023 at 02:21:23PM +0800, Jiawen Wu wrote: > > > > On Friday, May 26, 2023 12:14 PM, Jakub Kicinski wrote: > > > > > On Wed, 24 May 2023 17:17:21 +0800 Jiawen Wu wrote: > > > > > > + ret = devm_mdiobus_register(&pdev->dev, mii_bus); > > > > > > + if (ret) > > > > > > + return ret; > > > > > > + > > > > > > + mdiodev = mdio_device_create(mii_bus, 0); > > > > > > + if (IS_ERR(mdiodev)) > > > > > > + return PTR_ERR(mdiodev); > > > > > > + > > > > > > + xpcs = xpcs_create(mdiodev, PHY_INTERFACE_MODE_10GBASER); > > > > > > + if (IS_ERR(xpcs)) { > > > > > > + mdio_device_free(mdiodev); > > > > > > + return PTR_ERR(xpcs); > > > > > > + } > > > > > > > > > > How does the mdiodev get destroyed in case of success? > > > > > Seems like either freeing it in case of xpcs error is unnecessary > > > > > or it needs to also be freed when xpcs is destroyed? > > > > > > > > When xpcs is destroyed, that means mdiodev is no longer needed. > > > > I think there is no need to free mdiodev in case of xpcs error, > > > > since devm_* function leads to free it. > > > > > > If you are relying on the devm-ness of devm_mdiobus_register() then > > > it won't. Although mdiobus_unregister() walks bus->mdio_map[], I > > > think you are assuming that the mdio device you've created in > > > mdio_device_create() will be in that array. MDIO devices only get > > > added to that array when mdiobus_register_device() has been called, > > > which must only be called from mdio_device_register(). > > > > > > Please arrange to call mdio_device_free() prior to destroying the > > > XPCS in every case. > > > > Get it. > > It seems this is becoming a pattern, so I think we need to solve it > differently. How about something like this, which means you only have > to care about calling xpcs_create_mdiodev() and xpcs_destroy() ? > > diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c > index b87c69c4cdd7..802222581feb 100644 > --- a/drivers/net/pcs/pcs-xpcs.c > +++ b/drivers/net/pcs/pcs-xpcs.c > @@ -1240,6 +1240,7 @@ struct dw_xpcs *xpcs_create(struct mdio_device *mdiodev, > if (!xpcs) > return ERR_PTR(-ENOMEM); > > + mdio_device_get(mdiodev); > xpcs->mdiodev = mdiodev; > > xpcs_id = xpcs_get_id(xpcs); > @@ -1272,6 +1273,7 @@ struct dw_xpcs *xpcs_create(struct mdio_device *mdiodev, > ret = -ENODEV; > > out: > + mdio_device_put(mdiodev); > kfree(xpcs); > > return ERR_PTR(ret); > @@ -1280,8 +1282,33 @@ EXPORT_SYMBOL_GPL(xpcs_create); > > void xpcs_destroy(struct dw_xpcs *xpcs) > { > + mdio_device_put(mdiodev); > kfree(xpcs); > } > EXPORT_SYMBOL_GPL(xpcs_destroy); > > +struct dw_xpcs *xpcs_create_mdiodev(struct mii_bus *bus, int addr, > + phy_interface_t interface) > +{ > + struct mdio_device *mdiodev; > + struct dw_xpcs *xpcs; > + > + mdiodev = mdio_device_create(bus, addr); > + if (IS_ERR(mdiodev)) > + return ERR_CAST(mdiodev); > + > + xpcs = xpcs_create(mdiodev, interface); > + > + /* xpcs_create() has taken a refcount on the mdiodev if it was > + * successful. If xpcs_create() fails, this will free the mdio > + * device here. In any case, we don't need to hold our reference > + * anymore, and putting it here will allow mdio_device_put() in > + * xpcs_destroy() to automatically free the mdio device. > + */ > + mdio_device_put(mdiodev); > + > + return xpcs; > +} > +EXPORT_SYMBOL_GPL(xpcs_create_mdiodev); > + > MODULE_LICENSE("GPL v2"); > diff --git a/include/linux/mdio.h b/include/linux/mdio.h > index 1d7d550bbf1a..537b62330c90 100644 > --- a/include/linux/mdio.h > +++ b/include/linux/mdio.h > @@ -108,6 +108,16 @@ int mdio_driver_register(struct mdio_driver *drv); > void mdio_driver_unregister(struct mdio_driver *drv); > int mdio_device_bus_match(struct device *dev, struct device_driver *drv); > > +static inline void mdio_device_get(struct mdio_device *mdiodev) > +{ > + get_device(&mdiodev->dev); > +} > + > +static inline void mdio_device_put(struct mdio_device *mdiodev) > +{ > + mdio_device_free(mdiodev); > +} > + > static inline bool mdio_phy_id_is_c45(int phy_id) > { > return (phy_id & MDIO_PHY_ID_C45) && !(phy_id & ~MDIO_PHY_ID_C45_MASK); Looks great, it can eliminate to create mdiodev in the ethernet driver, this device only be used in xpcs.
On Fri, May 26, 2023 at 05:22:29PM +0800, Jiawen Wu wrote: > On Friday, May 26, 2023 5:07 PM, Russell King (Oracle) wrote: > > On Fri, May 26, 2023 at 05:01:49PM +0800, Jiawen Wu wrote: > > > On Friday, May 26, 2023 4:43 PM, Russell King (Oracle) wrote: > > > > On Fri, May 26, 2023 at 02:21:23PM +0800, Jiawen Wu wrote: > > > > > On Friday, May 26, 2023 12:14 PM, Jakub Kicinski wrote: > > > > > > On Wed, 24 May 2023 17:17:21 +0800 Jiawen Wu wrote: > > > > > > > + ret = devm_mdiobus_register(&pdev->dev, mii_bus); > > > > > > > + if (ret) > > > > > > > + return ret; > > > > > > > + > > > > > > > + mdiodev = mdio_device_create(mii_bus, 0); > > > > > > > + if (IS_ERR(mdiodev)) > > > > > > > + return PTR_ERR(mdiodev); > > > > > > > + > > > > > > > + xpcs = xpcs_create(mdiodev, PHY_INTERFACE_MODE_10GBASER); > > > > > > > + if (IS_ERR(xpcs)) { > > > > > > > + mdio_device_free(mdiodev); > > > > > > > + return PTR_ERR(xpcs); > > > > > > > + } > > > > > > > > > > > > How does the mdiodev get destroyed in case of success? > > > > > > Seems like either freeing it in case of xpcs error is unnecessary > > > > > > or it needs to also be freed when xpcs is destroyed? > > > > > > > > > > When xpcs is destroyed, that means mdiodev is no longer needed. > > > > > I think there is no need to free mdiodev in case of xpcs error, > > > > > since devm_* function leads to free it. > > > > > > > > If you are relying on the devm-ness of devm_mdiobus_register() then > > > > it won't. Although mdiobus_unregister() walks bus->mdio_map[], I > > > > think you are assuming that the mdio device you've created in > > > > mdio_device_create() will be in that array. MDIO devices only get > > > > added to that array when mdiobus_register_device() has been called, > > > > which must only be called from mdio_device_register(). > > > > > > > > Please arrange to call mdio_device_free() prior to destroying the > > > > XPCS in every case. > > > > > > Get it. > > > > It seems this is becoming a pattern, so I think we need to solve it > > differently. How about something like this, which means you only have > > to care about calling xpcs_create_mdiodev() and xpcs_destroy() ? > > > > diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c > > index b87c69c4cdd7..802222581feb 100644 > > --- a/drivers/net/pcs/pcs-xpcs.c > > +++ b/drivers/net/pcs/pcs-xpcs.c > > @@ -1240,6 +1240,7 @@ struct dw_xpcs *xpcs_create(struct mdio_device *mdiodev, > > if (!xpcs) > > return ERR_PTR(-ENOMEM); > > > > + mdio_device_get(mdiodev); > > xpcs->mdiodev = mdiodev; > > > > xpcs_id = xpcs_get_id(xpcs); > > @@ -1272,6 +1273,7 @@ struct dw_xpcs *xpcs_create(struct mdio_device *mdiodev, > > ret = -ENODEV; > > > > out: > > + mdio_device_put(mdiodev); > > kfree(xpcs); > > > > return ERR_PTR(ret); > > @@ -1280,8 +1282,33 @@ EXPORT_SYMBOL_GPL(xpcs_create); > > > > void xpcs_destroy(struct dw_xpcs *xpcs) > > { > > + mdio_device_put(mdiodev); > > kfree(xpcs); > > } > > EXPORT_SYMBOL_GPL(xpcs_destroy); > > > > +struct dw_xpcs *xpcs_create_mdiodev(struct mii_bus *bus, int addr, > > + phy_interface_t interface) > > +{ > > + struct mdio_device *mdiodev; > > + struct dw_xpcs *xpcs; > > + > > + mdiodev = mdio_device_create(bus, addr); > > + if (IS_ERR(mdiodev)) > > + return ERR_CAST(mdiodev); > > + > > + xpcs = xpcs_create(mdiodev, interface); > > + > > + /* xpcs_create() has taken a refcount on the mdiodev if it was > > + * successful. If xpcs_create() fails, this will free the mdio > > + * device here. In any case, we don't need to hold our reference > > + * anymore, and putting it here will allow mdio_device_put() in > > + * xpcs_destroy() to automatically free the mdio device. > > + */ > > + mdio_device_put(mdiodev); > > + > > + return xpcs; > > +} > > +EXPORT_SYMBOL_GPL(xpcs_create_mdiodev); > > + > > MODULE_LICENSE("GPL v2"); > > diff --git a/include/linux/mdio.h b/include/linux/mdio.h > > index 1d7d550bbf1a..537b62330c90 100644 > > --- a/include/linux/mdio.h > > +++ b/include/linux/mdio.h > > @@ -108,6 +108,16 @@ int mdio_driver_register(struct mdio_driver *drv); > > void mdio_driver_unregister(struct mdio_driver *drv); > > int mdio_device_bus_match(struct device *dev, struct device_driver *drv); > > > > +static inline void mdio_device_get(struct mdio_device *mdiodev) > > +{ > > + get_device(&mdiodev->dev); > > +} > > + > > +static inline void mdio_device_put(struct mdio_device *mdiodev) > > +{ > > + mdio_device_free(mdiodev); > > +} > > + > > static inline bool mdio_phy_id_is_c45(int phy_id) > > { > > return (phy_id & MDIO_PHY_ID_C45) && !(phy_id & ~MDIO_PHY_ID_C45_MASK); > > Looks great, it can eliminate to create mdiodev in the ethernet driver, this device > only be used in xpcs. I'm just creating a patch series for both xpcs and lynx, which this morning have had patches identifying similar problems with creation and destruction.
On Fri, May 26, 2023 at 10:37:04AM +0100, Russell King (Oracle) wrote: > I'm just creating a patch series for both xpcs and lynx, which this > morning have had patches identifying similar problems with creation > and destruction. https://lore.kernel.org/all/ZHCGZ8IgAAwr8bla@shell.armlinux.org.uk/
On Friday, May 26, 2023 6:42 PM, Russell King (Oracle) wrote: > On Fri, May 26, 2023 at 10:37:04AM +0100, Russell King (Oracle) wrote: > > I'm just creating a patch series for both xpcs and lynx, which this > > morning have had patches identifying similar problems with creation > > and destruction. > > https://lore.kernel.org/all/ZHCGZ8IgAAwr8bla@shell.armlinux.org.uk/ OK, I will send the updated patches after this patch series merged.
diff --git a/drivers/net/ethernet/wangxun/Kconfig b/drivers/net/ethernet/wangxun/Kconfig index 73f4492928c0..f3fb273e6fd0 100644 --- a/drivers/net/ethernet/wangxun/Kconfig +++ b/drivers/net/ethernet/wangxun/Kconfig @@ -45,6 +45,7 @@ config TXGBE select GPIOLIB select REGMAP select COMMON_CLK + select PCS_XPCS select LIBWX select SFP help diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c index 6aba22a4fc5e..d2a6f8ca78e7 100644 --- a/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c +++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c @@ -8,6 +8,8 @@ #include <linux/clk-provider.h> #include <linux/clkdev.h> #include <linux/regmap.h> +#include <linux/pcs/pcs-xpcs.h> +#include <linux/mdio.h> #include <linux/i2c.h> #include <linux/pci.h> @@ -77,6 +79,88 @@ static int txgbe_swnodes_register(struct txgbe *txgbe) return software_node_register_node_group(nodes->group); } +static int txgbe_pcs_read(struct mii_bus *bus, int addr, int devnum, int regnum) +{ + struct wx *wx = bus->priv; + u32 offset, val; + + if (addr) + return -EOPNOTSUPP; + + offset = devnum << 16 | regnum; + + /* Set the LAN port indicator to IDA_ADDR */ + wr32(wx, TXGBE_XPCS_IDA_ADDR, offset); + + /* Read the data from IDA_DATA register */ + val = rd32(wx, TXGBE_XPCS_IDA_DATA); + + return (u16)val; +} + +static int txgbe_pcs_write(struct mii_bus *bus, int addr, int devnum, int regnum, u16 val) +{ + struct wx *wx = bus->priv; + u32 offset; + + if (addr) + return -EOPNOTSUPP; + + offset = devnum << 16 | regnum; + + /* Set the LAN port indicator to IDA_ADDR */ + wr32(wx, TXGBE_XPCS_IDA_ADDR, offset); + + /* Write the data to IDA_DATA register */ + wr32(wx, TXGBE_XPCS_IDA_DATA, val); + + return 0; +} + +static int txgbe_mdio_pcs_init(struct txgbe *txgbe) +{ + struct mdio_device *mdiodev; + struct mii_bus *mii_bus; + struct dw_xpcs *xpcs; + struct pci_dev *pdev; + struct wx *wx; + int ret = 0; + + wx = txgbe->wx; + pdev = wx->pdev; + + mii_bus = devm_mdiobus_alloc(&pdev->dev); + if (!mii_bus) + return -ENOMEM; + + mii_bus->name = "txgbe_pcs_mdio_bus"; + mii_bus->read_c45 = &txgbe_pcs_read; + mii_bus->write_c45 = &txgbe_pcs_write; + mii_bus->parent = &pdev->dev; + mii_bus->phy_mask = ~0; + mii_bus->priv = wx; + snprintf(mii_bus->id, MII_BUS_ID_SIZE, "txgbe_pcs-%x", + (pdev->bus->number << 8) | pdev->devfn); + + ret = devm_mdiobus_register(&pdev->dev, mii_bus); + if (ret) + return ret; + + mdiodev = mdio_device_create(mii_bus, 0); + if (IS_ERR(mdiodev)) + return PTR_ERR(mdiodev); + + xpcs = xpcs_create(mdiodev, PHY_INTERFACE_MODE_10GBASER); + if (IS_ERR(xpcs)) { + mdio_device_free(mdiodev); + return PTR_ERR(xpcs); + } + + txgbe->xpcs = xpcs; + + return 0; +} + static int txgbe_gpio_get(struct gpio_chip *chip, unsigned int offset) { struct wx *wx = gpiochip_get_data(chip); @@ -429,16 +513,22 @@ int txgbe_init_phy(struct txgbe *txgbe) return ret; } + ret = txgbe_mdio_pcs_init(txgbe); + if (ret) { + wx_err(txgbe->wx, "failed to init mdio pcs: %d\n", ret); + goto err_unregister_swnode; + } + ret = txgbe_gpio_init(txgbe); if (ret) { wx_err(txgbe->wx, "failed to init gpio\n"); - goto err_unregister_swnode; + goto err_destroy_xpcs; } ret = txgbe_clock_register(txgbe); if (ret) { wx_err(txgbe->wx, "failed to register clock: %d\n", ret); - goto err_unregister_swnode; + goto err_destroy_xpcs; } ret = txgbe_i2c_register(txgbe); @@ -460,6 +550,8 @@ int txgbe_init_phy(struct txgbe *txgbe) err_unregister_clk: clkdev_drop(txgbe->clock); clk_unregister(txgbe->clk); +err_destroy_xpcs: + xpcs_destroy(txgbe->xpcs); err_unregister_swnode: software_node_unregister_node_group(txgbe->nodes.group); @@ -472,5 +564,6 @@ void txgbe_remove_phy(struct txgbe *txgbe) platform_device_unregister(txgbe->i2c_dev); clkdev_drop(txgbe->clock); clk_unregister(txgbe->clk); + xpcs_destroy(txgbe->xpcs); software_node_unregister_node_group(txgbe->nodes.group); } diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h b/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h index 6c903e4517c7..d1f62f62c28c 100644 --- a/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h +++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h @@ -80,6 +80,10 @@ /* I2C registers */ #define TXGBE_I2C_BASE 0x14900 +/************************************** ETH PHY ******************************/ +#define TXGBE_XPCS_IDA_ADDR 0x13000 +#define TXGBE_XPCS_IDA_DATA 0x13004 + /* Part Number String Length */ #define TXGBE_PBANUM_LENGTH 32 @@ -171,6 +175,7 @@ struct txgbe_nodes { struct txgbe { struct wx *wx; struct txgbe_nodes nodes; + struct dw_xpcs *xpcs; struct platform_device *sfp_dev; struct platform_device *i2c_dev; struct clk_lookup *clock;
Register MDIO bus for PCS layer to use Synopsys designware XPCS, support 10GBASE-R interface to the controller. Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com> --- drivers/net/ethernet/wangxun/Kconfig | 1 + .../net/ethernet/wangxun/txgbe/txgbe_phy.c | 97 ++++++++++++++++++- .../net/ethernet/wangxun/txgbe/txgbe_type.h | 5 + 3 files changed, 101 insertions(+), 2 deletions(-)