Re: [PATCH v4 7/7] phy: Add Northstar2 PCI Phy support

2016-06-30 Thread Kishon Vijay Abraham I
Hi,

On Monday 06 June 2016 06:11 PM, Pramod Kumar wrote:
> Add PCI Phy support for Broadcom Northstar2 SoCs.  This driver uses the
> interface from the iproc mdio mux driver to enable the devices
> respective phys.
> 
> Reviewed-by: Andrew Lunn 
> Signed-off-by: Jon Mason 
> Signed-off-by: Pramod Kumar 
> ---
>  drivers/phy/Kconfig|   8 +++
>  drivers/phy/Makefile   |   2 +-
>  drivers/phy/phy-bcm-ns2-pcie.c | 115 
> +
>  3 files changed, 124 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/phy/phy-bcm-ns2-pcie.c
> 
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index b869b98..01fb93b 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -434,4 +434,12 @@ config PHY_CYGNUS_PCIE
>  
>  source "drivers/phy/tegra/Kconfig"
>  
> +config PHY_NS2_PCIE
> + tristate "Broadcom Northstar2 PCIe PHY driver"
> + depends on OF && MDIO_BUS_MUX_BCM_IPROC
> + select GENERIC_PHY
> + default ARCH_BCM_IPROC
> + help
> +   Enable this to support the Broadcom Northstar2 PCIe PHY.
> +   If unsure, say N.
>  endmenu
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index 9c3e73c..7aea094 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -53,5 +53,5 @@ obj-$(CONFIG_PHY_TUSB1210)  += phy-tusb1210.o
>  obj-$(CONFIG_PHY_BRCM_SATA)  += phy-brcm-sata.o
>  obj-$(CONFIG_PHY_PISTACHIO_USB)  += phy-pistachio-usb.o
>  obj-$(CONFIG_PHY_CYGNUS_PCIE)+= phy-bcm-cygnus-pcie.o
> -
>  obj-$(CONFIG_ARCH_TEGRA) += tegra/
> +obj-$(CONFIG_PHY_NS2_PCIE)   += phy-bcm-ns2-pcie.o
> diff --git a/drivers/phy/phy-bcm-ns2-pcie.c b/drivers/phy/phy-bcm-ns2-pcie.c
> new file mode 100644
> index 000..9513f7a
> --- /dev/null
> +++ b/drivers/phy/phy-bcm-ns2-pcie.c
> @@ -0,0 +1,115 @@
> +/*
> + * Copyright (C) 2016 Broadcom
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation version 2.
> + *
> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
> + * kind, whether express or implied; without even the implied warranty
> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +struct ns2_pci_phy {
> + struct mdio_device *mdiodev;
> + struct phy *phy;
> +};
> +
> +#define BLK_ADDR_REG_OFFSET  0x1f
> +#define PLL_AFE1_100MHZ_BLK  0x2100
> +#define PLL_CLK_AMP_OFFSET   0x03
> +#define PLL_CLK_AMP_2P05V0x2b18
> +
> +static int ns2_pci_phy_init(struct phy *p)
> +{
> + struct ns2_pci_phy *phy = phy_get_drvdata(p);
> + int rc;
> +
> + /* select the AFE 100MHz block page */
> + rc = mdiobus_write(phy->mdiodev->bus, phy->mdiodev->addr,
> +BLK_ADDR_REG_OFFSET, PLL_AFE1_100MHZ_BLK);
> + if (rc)
> + goto err;
> +
> + /* set the 100 MHz reference clock amplitude to 2.05 v */
> + rc = mdiobus_write(phy->mdiodev->bus, phy->mdiodev->addr,
> +PLL_CLK_AMP_OFFSET, PLL_CLK_AMP_2P05V);
> + if (rc)
> + goto err;
> +
> + return 0;
> +
> +err:
> + dev_err(>mdiodev->dev, "Error %d writing to phy\n", rc);
> + return rc;
> +}
> +
> +static struct phy_ops ns2_pci_phy_ops = {
> + .init = ns2_pci_phy_init,

.owner is required for phy_ops.
> +};
> +
> +static int ns2_pci_phy_probe(struct mdio_device *mdiodev)
> +{
> + struct device *dev = >dev;
> + struct phy_provider *provider;
> + struct ns2_pci_phy *p;
> + struct phy *phy;
> +
> + phy = devm_phy_create(dev, dev->of_node, _pci_phy_ops);
> + if (IS_ERR(phy)) {
> + dev_err(dev, "failed to create Phy\n");
> + return PTR_ERR(phy);
> + }
> +
> + p = devm_kmalloc(dev, sizeof(struct ns2_pci_phy),
> +  GFP_KERNEL);
> + if (!p)
> + return -ENOMEM;
> +
> + p->mdiodev = mdiodev;
> + dev_set_drvdata(dev, p);
> +
> + p->phy = phy;
> + phy_set_drvdata(phy, p);
> +
> + provider = devm_of_phy_provider_register(>dev,
> +  of_phy_simple_xlate);

just use return PTR_ERR_OR_ZERO(provider);

All the error messages are un-necessary.

Thanks
Kishon


RE: [PATCH v4 7/7] phy: Add Northstar2 PCI Phy support

2016-06-07 Thread Pramod Kumar
Hi Florian,

> -Original Message-
> From: Florian Fainelli [mailto:f.faine...@gmail.com]
> Sent: 07 June 2016 00:12
> To: Pramod Kumar; Rob Herring; Pawel Moll; Mark Rutland; Ian Campbell;
> Kumar Gala; Catalin Marinas; Will Deacon; Kishon Vijay Abraham I; David S.
> Miller
> Cc: devicet...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; bcm-
> kernel-feedback-l...@broadcom.com; linux-ker...@vger.kernel.org;
> netdev@vger.kernel.org; Jon Mason
> Subject: Re: [PATCH v4 7/7] phy: Add Northstar2 PCI Phy support
>
> On 06/06/2016 05:41 AM, Pramod Kumar wrote:
> > Add PCI Phy support for Broadcom Northstar2 SoCs.  This driver uses
> > the interface from the iproc mdio mux driver to enable the devices
> > respective phys.
> >
> > Reviewed-by: Andrew Lunn <and...@lunn.ch>
> > Signed-off-by: Jon Mason <jonma...@broadcom.com>
> > Signed-off-by: Pramod Kumar <pramod.ku...@broadcom.com>
> > ---
> >  drivers/phy/Kconfig|   8 +++
> >  drivers/phy/Makefile   |   2 +-
> >  drivers/phy/phy-bcm-ns2-pcie.c | 115
> > +
> >  3 files changed, 124 insertions(+), 1 deletion(-)  create mode 100644
> > drivers/phy/phy-bcm-ns2-pcie.c
> >
> > diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig index
> > b869b98..01fb93b 100644
> > --- a/drivers/phy/Kconfig
> > +++ b/drivers/phy/Kconfig
> > @@ -434,4 +434,12 @@ config PHY_CYGNUS_PCIE
> >
> >  source "drivers/phy/tegra/Kconfig"
> >
> > +config PHY_NS2_PCIE
> > +   tristate "Broadcom Northstar2 PCIe PHY driver"
> > +   depends on OF && MDIO_BUS_MUX_BCM_IPROC
> > +   select GENERIC_PHY
> > +   default ARCH_BCM_IPROC
>
> Are not you missing a dependency on PHYLIB too to provide
> mdio_module_register() etc. (at least make it build)?
>

This depends on MDIO_BUS_MUX_BCM_IPROC which itself depends on PHYLIB. Hence
I don't think We need to take care of this here.

> --
> Florian

Regards,
Pramod


Re: [PATCH v4 7/7] phy: Add Northstar2 PCI Phy support

2016-06-06 Thread Florian Fainelli
On 06/06/2016 05:41 AM, Pramod Kumar wrote:
> Add PCI Phy support for Broadcom Northstar2 SoCs.  This driver uses the
> interface from the iproc mdio mux driver to enable the devices
> respective phys.
> 
> Reviewed-by: Andrew Lunn 
> Signed-off-by: Jon Mason 
> Signed-off-by: Pramod Kumar 
> ---
>  drivers/phy/Kconfig|   8 +++
>  drivers/phy/Makefile   |   2 +-
>  drivers/phy/phy-bcm-ns2-pcie.c | 115 
> +
>  3 files changed, 124 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/phy/phy-bcm-ns2-pcie.c
> 
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index b869b98..01fb93b 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -434,4 +434,12 @@ config PHY_CYGNUS_PCIE
>  
>  source "drivers/phy/tegra/Kconfig"
>  
> +config PHY_NS2_PCIE
> + tristate "Broadcom Northstar2 PCIe PHY driver"
> + depends on OF && MDIO_BUS_MUX_BCM_IPROC
> + select GENERIC_PHY
> + default ARCH_BCM_IPROC

Are not you missing a dependency on PHYLIB too to provide
mdio_module_register() etc. (at least make it build)?

-- 
Florian


[PATCH v4 7/7] phy: Add Northstar2 PCI Phy support

2016-06-06 Thread Pramod Kumar
Add PCI Phy support for Broadcom Northstar2 SoCs.  This driver uses the
interface from the iproc mdio mux driver to enable the devices
respective phys.

Reviewed-by: Andrew Lunn 
Signed-off-by: Jon Mason 
Signed-off-by: Pramod Kumar 
---
 drivers/phy/Kconfig|   8 +++
 drivers/phy/Makefile   |   2 +-
 drivers/phy/phy-bcm-ns2-pcie.c | 115 +
 3 files changed, 124 insertions(+), 1 deletion(-)
 create mode 100644 drivers/phy/phy-bcm-ns2-pcie.c

diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index b869b98..01fb93b 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -434,4 +434,12 @@ config PHY_CYGNUS_PCIE
 
 source "drivers/phy/tegra/Kconfig"
 
+config PHY_NS2_PCIE
+   tristate "Broadcom Northstar2 PCIe PHY driver"
+   depends on OF && MDIO_BUS_MUX_BCM_IPROC
+   select GENERIC_PHY
+   default ARCH_BCM_IPROC
+   help
+ Enable this to support the Broadcom Northstar2 PCIe PHY.
+ If unsure, say N.
 endmenu
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index 9c3e73c..7aea094 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -53,5 +53,5 @@ obj-$(CONFIG_PHY_TUSB1210)+= phy-tusb1210.o
 obj-$(CONFIG_PHY_BRCM_SATA)+= phy-brcm-sata.o
 obj-$(CONFIG_PHY_PISTACHIO_USB)+= phy-pistachio-usb.o
 obj-$(CONFIG_PHY_CYGNUS_PCIE)  += phy-bcm-cygnus-pcie.o
-
 obj-$(CONFIG_ARCH_TEGRA) += tegra/
+obj-$(CONFIG_PHY_NS2_PCIE) += phy-bcm-ns2-pcie.o
diff --git a/drivers/phy/phy-bcm-ns2-pcie.c b/drivers/phy/phy-bcm-ns2-pcie.c
new file mode 100644
index 000..9513f7a
--- /dev/null
+++ b/drivers/phy/phy-bcm-ns2-pcie.c
@@ -0,0 +1,115 @@
+/*
+ * Copyright (C) 2016 Broadcom
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+struct ns2_pci_phy {
+   struct mdio_device *mdiodev;
+   struct phy *phy;
+};
+
+#define BLK_ADDR_REG_OFFSET0x1f
+#define PLL_AFE1_100MHZ_BLK0x2100
+#define PLL_CLK_AMP_OFFSET 0x03
+#define PLL_CLK_AMP_2P05V  0x2b18
+
+static int ns2_pci_phy_init(struct phy *p)
+{
+   struct ns2_pci_phy *phy = phy_get_drvdata(p);
+   int rc;
+
+   /* select the AFE 100MHz block page */
+   rc = mdiobus_write(phy->mdiodev->bus, phy->mdiodev->addr,
+  BLK_ADDR_REG_OFFSET, PLL_AFE1_100MHZ_BLK);
+   if (rc)
+   goto err;
+
+   /* set the 100 MHz reference clock amplitude to 2.05 v */
+   rc = mdiobus_write(phy->mdiodev->bus, phy->mdiodev->addr,
+  PLL_CLK_AMP_OFFSET, PLL_CLK_AMP_2P05V);
+   if (rc)
+   goto err;
+
+   return 0;
+
+err:
+   dev_err(>mdiodev->dev, "Error %d writing to phy\n", rc);
+   return rc;
+}
+
+static struct phy_ops ns2_pci_phy_ops = {
+   .init = ns2_pci_phy_init,
+};
+
+static int ns2_pci_phy_probe(struct mdio_device *mdiodev)
+{
+   struct device *dev = >dev;
+   struct phy_provider *provider;
+   struct ns2_pci_phy *p;
+   struct phy *phy;
+
+   phy = devm_phy_create(dev, dev->of_node, _pci_phy_ops);
+   if (IS_ERR(phy)) {
+   dev_err(dev, "failed to create Phy\n");
+   return PTR_ERR(phy);
+   }
+
+   p = devm_kmalloc(dev, sizeof(struct ns2_pci_phy),
+GFP_KERNEL);
+   if (!p)
+   return -ENOMEM;
+
+   p->mdiodev = mdiodev;
+   dev_set_drvdata(dev, p);
+
+   p->phy = phy;
+   phy_set_drvdata(phy, p);
+
+   provider = devm_of_phy_provider_register(>dev,
+of_phy_simple_xlate);
+   if (IS_ERR(provider)) {
+   dev_err(dev, "failed to register Phy provider\n");
+   return PTR_ERR(provider);
+   }
+
+   dev_info(dev, "%s PHY registered\n", dev_name(dev));
+
+   return 0;
+}
+
+static const struct of_device_id ns2_pci_phy_of_match[] = {
+   { .compatible = "brcm,ns2-pcie-phy", },
+   { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, ns2_pci_phy_of_match);
+
+static struct mdio_driver ns2_pci_phy_driver = {
+   .mdiodrv = {
+   .driver = {
+   .name = "phy-bcm-ns2-pci",
+   .of_match_table = ns2_pci_phy_of_match,
+   },
+   },
+   .probe = ns2_pci_phy_probe,
+};
+mdio_module_driver(ns2_pci_phy_driver);
+
+MODULE_AUTHOR("Broadcom");
+MODULE_DESCRIPTION("Broadcom Northstar2