From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757827AbdEKQaA (ORCPT ); Thu, 11 May 2017 12:30:00 -0400 Received: from mail-qt0-f195.google.com ([209.85.216.195]:35205 "EHLO mail-qt0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933105AbdEKQ34 (ORCPT ); Thu, 11 May 2017 12:29:56 -0400 Subject: Re: [PATCH 5/5] phy: bcm-ns-usb3: add MDIO driver using proper bus layer To: =?UTF-8?B?UmFmYcWCIE1pxYJlY2tp?= , Kishon Vijay Abraham I References: <20170511132925.14564-1-zajec5@gmail.com> <20170511132925.14564-6-zajec5@gmail.com> Cc: Yendapally Reddy Dhananjaya Reddy , Jon Mason , linux-arm-kernel@lists.infradead.org, bcm-kernel-feedback-list@broadcom.com, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, =?UTF-8?B?UmFmYcWCIE1pxYJlY2tp?= From: Florian Fainelli Message-ID: Date: Thu, 11 May 2017 09:29:52 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <20170511132925.14564-6-zajec5@gmail.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 05/11/2017 06:29 AM, Rafał Miłecki wrote: > From: Rafał Miłecki > > As USB 3.0 PHY is attached to the MDIO bus this module should provide a > MDIO driver and use a proper bus layer. This is a proper (cleaner) > solution which doesn't require code to know this specific MDIO bus > details. It also allows reusing the driver with other MDIO buses. > > Signed-off-by: Rafał Miłecki > --- > drivers/phy/Kconfig | 1 + > drivers/phy/phy-bcm-ns-usb3.c | 98 ++++++++++++++++++++++++++++++++++++++++++- > 2 files changed, 98 insertions(+), 1 deletion(-) > > diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig > index afaf7b643eeb..2a9186b98ae0 100644 > --- a/drivers/phy/Kconfig > +++ b/drivers/phy/Kconfig > @@ -29,6 +29,7 @@ config PHY_BCM_NS_USB3 > depends on ARCH_BCM_IPROC || COMPILE_TEST > depends on HAS_IOMEM && OF > select GENERIC_PHY > + select PHYLIB Should not this be select MDIO_DEVICE instead? 4.11 introduced the possibility to build support for MDIO bus/devices without requiring PHYLIB. > help > Enable this to support Broadcom USB 3.0 PHY connected to the USB > controller on Northstar family. > diff --git a/drivers/phy/phy-bcm-ns-usb3.c b/drivers/phy/phy-bcm-ns-usb3.c > index 2c9a0d5f43d8..389f5e5a6238 100644 > --- a/drivers/phy/phy-bcm-ns-usb3.c > +++ b/drivers/phy/phy-bcm-ns-usb3.c > @@ -16,7 +16,9 @@ > #include > #include > #include > +#include > #include > +#include > #include > #include > #include > @@ -52,6 +54,7 @@ struct bcm_ns_usb3 { > enum bcm_ns_family family; > void __iomem *dmp; > void __iomem *ccb_mii; > + struct mdio_device *mdiodev; > struct phy *phy; > > int (*phy_write)(struct bcm_ns_usb3 *usb3, u16 reg, u16 value); > @@ -183,6 +186,77 @@ static const struct phy_ops ops = { > }; > > /************************************************** > + * MDIO driver code > + **************************************************/ > + > +static int bcm_ns_usb3_mdiodev_phy_write(struct bcm_ns_usb3 *usb3, u16 reg, > + u16 value) > +{ > + struct mdio_device *mdiodev = usb3->mdiodev; > + > + return mdiobus_write(mdiodev->bus, mdiodev->addr, reg, value); > +} > + > +static int bcm_ns_usb3_mdio_probe(struct mdio_device *mdiodev) > +{ > + struct device *dev = &mdiodev->dev; > + const struct of_device_id *of_id; > + struct phy_provider *phy_provider; > + struct device_node *syscon_np; > + struct bcm_ns_usb3 *usb3; > + struct resource res; > + int err; > + > + usb3 = devm_kzalloc(dev, sizeof(*usb3), GFP_KERNEL); > + if (!usb3) > + return -ENOMEM; > + > + usb3->dev = dev; > + usb3->mdiodev = mdiodev; > + > + of_id = of_match_device(bcm_ns_usb3_id_table, dev); > + if (!of_id) > + return -EINVAL; > + usb3->family = (enum bcm_ns_family)of_id->data; > + > + syscon_np = of_parse_phandle(dev->of_node, "usb3-dmp-syscon", 0); > + err = of_address_to_resource(syscon_np, 0, &res); > + of_node_put(syscon_np); > + if (err) > + return err; > + > + usb3->dmp = devm_ioremap_resource(dev, &res); > + if (IS_ERR(usb3->dmp)) { > + dev_err(dev, "Failed to map DMP regs\n"); > + return PTR_ERR(usb3->dmp); > + } > + > + usb3->phy_write = bcm_ns_usb3_mdiodev_phy_write; > + > + usb3->phy = devm_phy_create(dev, NULL, &ops); > + if (IS_ERR(usb3->phy)) { > + dev_err(dev, "Failed to create PHY\n"); > + return PTR_ERR(usb3->phy); > + } > + > + phy_set_drvdata(usb3->phy, usb3); > + > + phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate); > + > + return PTR_ERR_OR_ZERO(phy_provider); > +} > + > +static struct mdio_driver bcm_ns_usb3_mdio_driver = { > + .mdiodrv = { > + .driver = { > + .name = "bcm_ns_mdio_usb3", > + .of_match_table = bcm_ns_usb3_id_table, > + }, > + }, > + .probe = bcm_ns_usb3_mdio_probe, > +}; > + > +/************************************************** > * Platform driver code > **************************************************/ > > @@ -297,6 +371,28 @@ static struct platform_driver bcm_ns_usb3_driver = { > .of_match_table = bcm_ns_usb3_id_table, > }, > }; > -module_platform_driver(bcm_ns_usb3_driver); > + > +static int __init bcm_ns_usb3_module_init(void) > +{ > + int err; > + > + err = mdio_driver_register(&bcm_ns_usb3_mdio_driver); > + if (err) > + return err; > + > + err = platform_driver_register(&bcm_ns_usb3_driver); > + if (err) > + mdio_driver_unregister(&bcm_ns_usb3_mdio_driver); > + > + return err; > +} > +module_init(bcm_ns_usb3_module_init); > + > +static void __exit bcm_ns_usb3_module_exit(void) > +{ > + platform_driver_unregister(&bcm_ns_usb3_driver); > + mdio_driver_unregister(&bcm_ns_usb3_mdio_driver); > +} > +module_exit(bcm_ns_usb3_module_exit) Do we need to keep both platform device and mdio device registration here? Do you have a depreciation time line for when we can entirely switch to mdio device (assuming this is for backwards compatibility)? -- Florian