[PATCH 0/5] Add Cygnus PCIe PHY support

[PATCH 0/5] Add Cygnus PCIe PHY support

[Ray Jui]

This patch series adds support for the Cygnus PCIe PHY and support of MDC/MDIO
interface found in various iProc based of SoCs. The iProc MDC/MDIO interface
can be used by the host processor to communicate with various internal
Serdes/PHYs including Ethernet, PCIe, USB, etc.

This patch series is based on Linux v4.1-rc4 and is avaliable in:
https://github.com/Broadcom/cygnus-linux/tree/cygnus-pcie-phy-v1

Ray Jui (5):
  phy: iproc-mdio: Define DT binding
  phy: iproc-mdio: Initial iProc MDC/MDIO support
  phy: cygnus: pcie: Define DT binding
  phy: cygnus: pcie: Add Cygnus PCIe PHY support
  ARM: dts: enable PCIe PHY support for Cygnus

 .../bindings/phy/brcm,cygnus-pcie-phy.txt          |   31 ++
 .../devicetree/bindings/phy/brcm,iproc-mdio.txt    |   15 +
 arch/arm/boot/dts/bcm-cygnus.dtsi                  |   17 +
 drivers/phy/Kconfig                                |   27 ++
 drivers/phy/Makefile                               |    2 +
 drivers/phy/phy-cygnus-pcie.c                      |  340 ++++++++++++++++++++
 drivers/phy/phy-iproc-mdio.c                       |  249 ++++++++++++++
 include/linux/phy/iproc_mdio_phy.h                 |   22 ++
 8 files changed, 703 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/brcm,cygnus-pcie-phy.txt
 create mode 100644 Documentation/devicetree/bindings/phy/brcm,iproc-mdio.txt
 create mode 100644 drivers/phy/phy-cygnus-pcie.c
 create mode 100644 drivers/phy/phy-iproc-mdio.c
 create mode 100644 include/linux/phy/iproc_mdio_phy.h

-- 
1.7.9.5

[PATCH 1/5] phy: iproc-mdio: Define DT binding

[Ray Jui]

Define device tree bindings for iProc MDC/MDIO interface

Signed-off-by: Ray Jui <rjui@broadcom.com>
Reviewed-by: Arun Parameswaran <aparames@broadcom.com>
Reviewed-by: JD (Jiandong) Zheng <jdzheng@broadcom.com>
Reviewed-by: Scott Branden <sbranden@broadcom.com>
---
 .../devicetree/bindings/phy/brcm,iproc-mdio.txt    |   15 +++++++++++++++
 1 file changed, 15 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/brcm,iproc-mdio.txt

diff --git a/Documentation/devicetree/bindings/phy/brcm,iproc-mdio.txt b/Documentation/devicetree/bindings/phy/brcm,iproc-mdio.txt
new file mode 100644
index 0000000..dce7644
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/brcm,iproc-mdio.txt
@@ -0,0 +1,15 @@
+Broadcom iProc MDC/MDIO interface
+
+The iProc MDC/MDIO interface is found on various iProc based SoCs and used to
+communicate with various types of Serdes/PHYs including Ethernet, PCIe, USB,
+and etc.
+
+Required properties:
+- compatible: Must be "brcm,iproc-mdio"
+- reg: base address and length of the MDC/MDIO block
+
+Example:
+	mdio: mdio@18002000 {
+		compatible = "brcm,iproc-mdio";
+		reg = <0x18002000 0x8>;
+	};
-- 
1.7.9.5

Re: [PATCH 1/5] phy: iproc-mdio: Define DT binding

[Kishon Vijay Abraham I]

Hi,

On Wednesday 20 May 2015 06:53 AM, Ray Jui wrote:
> Define device tree bindings for iProc MDC/MDIO interface


Fix $subject to something like 'dt_bindings: Add iProc MDC/MDIO PHY
bindings'

>
> Signed-off-by: Ray Jui <rjui@broadcom.com>
> Reviewed-by: Arun Parameswaran <aparames@broadcom.com>
> Reviewed-by: JD (Jiandong) Zheng <jdzheng@broadcom.com>
> Reviewed-by: Scott Branden <sbranden@broadcom.com>
> ---
>   .../devicetree/bindings/phy/brcm,iproc-mdio.txt    |   15 +++++++++++++++
>   1 file changed, 15 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/phy/brcm,iproc-mdio.txt
>
> diff --git a/Documentation/devicetree/bindings/phy/brcm,iproc-mdio.txt b/Documentation/devicetree/bindings/phy/brcm,iproc-mdio.txt
> new file mode 100644
> index 0000000..dce7644
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/brcm,iproc-mdio.txt
> @@ -0,0 +1,15 @@
> +Broadcom iProc MDC/MDIO interface
> +
> +The iProc MDC/MDIO interface is found on various iProc based SoCs and used to
> +communicate with various types of Serdes/PHYs including Ethernet, PCIe, USB,
> +and etc.

This sounds like it isn't a PHY exactly but a bus to which other PHYs are 
connected?

Thanks
Kishon

Re: [PATCH 1/5] phy: iproc-mdio: Define DT binding

[Ray Jui]

Hi Kishon,

On 5/21/2015 6:07 AM, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On Wednesday 20 May 2015 06:53 AM, Ray Jui wrote:
>> Define device tree bindings for iProc MDC/MDIO interface
> 
> 
> Fix $subject to something like 'dt_bindings: Add iProc MDC/MDIO PHY
> bindings'
> 
Will do.

>>
>> Signed-off-by: Ray Jui <rjui@broadcom.com>
>> Reviewed-by: Arun Parameswaran <aparames@broadcom.com>
>> Reviewed-by: JD (Jiandong) Zheng <jdzheng@broadcom.com>
>> Reviewed-by: Scott Branden <sbranden@broadcom.com>
>> ---
>>   .../devicetree/bindings/phy/brcm,iproc-mdio.txt    |   15
>> +++++++++++++++
>>   1 file changed, 15 insertions(+)
>>   create mode 100644
>> Documentation/devicetree/bindings/phy/brcm,iproc-mdio.txt
>>
>> diff --git a/Documentation/devicetree/bindings/phy/brcm,iproc-mdio.txt
>> b/Documentation/devicetree/bindings/phy/brcm,iproc-mdio.txt
>> new file mode 100644
>> index 0000000..dce7644
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/phy/brcm,iproc-mdio.txt
>> @@ -0,0 +1,15 @@
>> +Broadcom iProc MDC/MDIO interface
>> +
>> +The iProc MDC/MDIO interface is found on various iProc based SoCs and
>> used to
>> +communicate with various types of Serdes/PHYs including Ethernet,
>> PCIe, USB,
>> +and etc.
> 
> This sounds like it isn't a PHY exactly but a bus to which other PHYs
> are connected?

Yes it's an internal MDIO interface connected to various types of
internal Serdes or PHYs.

> 
> Thanks
> Kishon

[PATCH 2/5] phy: iproc-mdio: Initial iProc MDC/MDIO support

[Ray Jui]

This adds the support for the iProc MDC/MDIO interface. Multiple iProc
SoCs contain the MDC/MDIO interface that can be used for the host to
communicate with various Serdes/PHYs including Ethernet, PCIe, USB, etc.

Signed-off-by: Ray Jui <rjui@broadcom.com>
Reviewed-by: Arun Parameswaran <aparames@broadcom.com>
Reviewed-by: JD (Jiandong) Zheng <jdzheng@broadcom.com>
Reviewed-by: Scott Branden <sbranden@broadcom.com>
---
 drivers/phy/Kconfig                |   12 ++
 drivers/phy/Makefile               |    1 +
 drivers/phy/phy-iproc-mdio.c       |  249 ++++++++++++++++++++++++++++++++++++
 include/linux/phy/iproc_mdio_phy.h |   22 ++++
 4 files changed, 284 insertions(+)
 create mode 100644 drivers/phy/phy-iproc-mdio.c
 create mode 100644 include/linux/phy/iproc_mdio_phy.h

diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index a53bd5b..2664285 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -52,6 +52,18 @@ config PHY_EXYNOS_MIPI_VIDEO
 	  Support for MIPI CSI-2 and MIPI DSI DPHY found on Samsung S5P
 	  and EXYNOS SoCs.
 
+config PHY_IPROC_MDIO
+	bool "Broadcom iProc MDC/MDIO driver"
+	depends on ARCH_BCM_IPROC
+	default ARCH_BCM_IPROC
+	help
+	  Enable this to support the iProc generic MDC/MDIO interface found
+	  in various iProc based SoCs. The generic MDC/MDIO interface can be
+	  used to communicate with various types of Serdes/PHYs including
+	  Ethernet, PCIe, USB, and etc.
+
+	  If unsure, say N.
+
 config PHY_MVEBU_SATA
 	def_bool y
 	depends on ARCH_DOVE || MACH_DOVE || MACH_KIRKWOOD
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index f126251..d989dd7b 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -13,6 +13,7 @@ obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO)	+= phy-exynos-mipi-video.o
 obj-$(CONFIG_PHY_MVEBU_SATA)		+= phy-mvebu-sata.o
 obj-$(CONFIG_PHY_MIPHY28LP) 		+= phy-miphy28lp.o
 obj-$(CONFIG_PHY_MIPHY365X)		+= phy-miphy365x.o
+obj-$(CONFIG_PHY_IPROC_MDIO)		+= phy-iproc-mdio.o
 obj-$(CONFIG_PHY_RCAR_GEN2)		+= phy-rcar-gen2.o
 obj-$(CONFIG_OMAP_CONTROL_PHY)		+= phy-omap-control.o
 obj-$(CONFIG_OMAP_USB2)			+= phy-omap-usb2.o
diff --git a/drivers/phy/phy-iproc-mdio.c b/drivers/phy/phy-iproc-mdio.c
new file mode 100644
index 0000000..a37e92f
--- /dev/null
+++ b/drivers/phy/phy-iproc-mdio.c
@@ -0,0 +1,249 @@
+/*
+ * Copyright (C) 2015 Broadcom Corporation
+ *
+ * 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 <linux/clk.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/phy/iproc_mdio_phy.h>
+
+#define MDIO_TIMEOUT_MSEC             10
+
+#define MDC_CTRL_OFFSET               0x000
+
+#define MDC_CTRL_DIV_SHIFT            0
+#define MDC_CTRL_DIV_WIDTH            7
+#define MDC_MAX_DIV                   (BIT(MDC_CTRL_DIV_WIDTH) - 1)
+#define MDC_CTRL_PRE_SHIFT            7
+#define MDC_CTRL_BUSY_SHIFT           8
+#define MDC_CTRL_EXT_SHIFT            9
+#define MDC_CTRL_BTP_SHIFT            10
+
+#define MDC_DATA_OFFSET               0x004
+
+#define MDC_DATA_SHIFT                0
+#define MDC_DATA_MASK                 0xffff
+
+#define MDC_DATA_TA_SHIFT             16
+#define MDC_DATA_TA_VAL               2
+
+#define MDC_DATA_RA_SHIFT             18
+#define MDC_DATA_RA_WIDTH             5
+#define MDC_MAX_RA                    (BIT(MDC_DATA_RA_WIDTH) - 1)
+
+#define MDC_DATA_PA_SHIFT             23
+#define MDC_DATA_PA_WIDTH             5
+#define MDC_MAX_PA                    (BIT(MDC_DATA_PA_WIDTH) - 1)
+
+#define MDC_DATA_OP_SHIFT             28
+#define MDC_DATA_OP_WRITE             1
+#define MDC_DATA_OP_READ              2
+
+#define MDC_DATA_SB_SHIFT             30
+
+/**
+ * struct iproc_mdc - iProc MDC/MDIO device
+ * @dev: pointer to device
+ * @base: MDC controller register base pointer
+ * @lock: mutex to protect access to the MDC device
+ * @is_ready: flag to indicate the driver has been initialized and is ready for
+ * use
+ */
+struct iproc_mdc {
+	struct device *dev;
+	void __iomem *base;
+	struct mutex lock;
+	bool is_ready;
+};
+
+static struct iproc_mdc iproc_mdc;
+
+static inline int iproc_mdio_check_params(unsigned clk_div, unsigned pa,
+					  unsigned ra)
+{
+	if (clk_div > MDC_MAX_DIV || pa > MDC_MAX_PA || ra > MDC_MAX_RA)
+		return -EINVAL;
+
+	return 0;
+}
+
+static inline int iproc_mdio_wait_for_idle(void __iomem *base)
+{
+	u32 val;
+	unsigned long timeout = jiffies + msecs_to_jiffies(MDIO_TIMEOUT_MSEC);
+
+	while (!time_after(jiffies, timeout)) {
+		val = readl(base + MDC_CTRL_OFFSET);
+		if ((val & BIT(MDC_CTRL_BUSY_SHIFT)) == 0)
+			return 0;
+
+		cpu_relax();
+	}
+
+	return -ETIMEDOUT;
+}
+
+static inline void iproc_mdio_config_clk(void __iomem *base, unsigned clk_div)
+{
+	u32 val;
+
+	val = (clk_div << MDC_CTRL_DIV_SHIFT) | BIT(MDC_CTRL_PRE_SHIFT);
+	writel(val, base + MDC_CTRL_OFFSET);
+}
+
+static int iproc_mdio_read_write(void __iomem *base, bool dir_is_write,
+				 unsigned clk_div, unsigned pa, unsigned ra,
+				 u16 *data)
+{
+	int ret;
+	u32 val;
+
+	ret = iproc_mdio_check_params(clk_div, pa, ra);
+	if (ret)
+		return ret;
+
+	ret = iproc_mdio_wait_for_idle(base);
+	if (ret)
+		return ret;
+
+	iproc_mdio_config_clk(base, clk_div);
+
+	ret = iproc_mdio_wait_for_idle(base);
+	if (ret)
+		return ret;
+
+	val = (MDC_DATA_TA_VAL << MDC_DATA_TA_SHIFT) |
+		(ra << MDC_DATA_RA_SHIFT) |
+		(pa << MDC_DATA_PA_SHIFT) |
+		BIT(MDC_DATA_SB_SHIFT);
+
+	if (dir_is_write) {
+		val |= (MDC_DATA_OP_WRITE << MDC_DATA_OP_SHIFT);
+		val |= ((u32)(*data) & MDC_DATA_MASK);
+	} else {
+		val |= (MDC_DATA_OP_READ << MDC_DATA_OP_SHIFT);
+	}
+
+	writel(val, base + MDC_DATA_OFFSET);
+
+	ret = iproc_mdio_wait_for_idle(base);
+	if (ret)
+		return ret;
+
+	if (!dir_is_write) {
+		val = readl(base + MDC_DATA_OFFSET) & MDC_DATA_MASK;
+		*data = (u16)val;
+	}
+
+	return 0;
+}
+
+
+/**
+ * iproc_mdio_read() - read from a PHY register through the MDC interface
+ *
+ * @clk_div: MDC clock divisor
+ * @phy_addr: MDC PHY address
+ * @reg_addr: PHY register address
+ * @data: pointer to the memory where data will be stored
+ */
+int iproc_mdio_read(unsigned clk_div, unsigned phy_addr, unsigned reg_addr,
+		    u16 *data)
+{
+	int ret;
+	struct iproc_mdc *mdc = &iproc_mdc;
+
+	if (!mdc->is_ready)
+		return -ENODEV;
+
+	mutex_lock(&mdc->lock);
+	ret = iproc_mdio_read_write(mdc->base, false, clk_div, phy_addr,
+				    reg_addr, data);
+	if (ret)
+		dev_err(mdc->dev, "mdio read failed\n");
+	mutex_unlock(&mdc->lock);
+
+	return ret;
+}
+EXPORT_SYMBOL(iproc_mdio_read);
+
+/**
+ * iproc_mdio_write() - write to a PHY register through the MDC interface
+ *
+ * @clk_div: MDC clock divisor
+ * @phy_addr: MDC PHY address
+ * @reg_addr: PHY register address
+ * @data: data value to be written to the PHY register
+ */
+int iproc_mdio_write(unsigned clk_div, unsigned phy_addr, unsigned reg_addr,
+		     u16 data)
+{
+	int ret;
+	struct iproc_mdc *mdc = &iproc_mdc;
+
+	if (!mdc->is_ready)
+		return -ENODEV;
+
+	mutex_lock(&mdc->lock);
+	ret = iproc_mdio_read_write(mdc->base, true, clk_div, phy_addr,
+				    reg_addr, &data);
+	if (ret)
+		dev_err(mdc->dev, "mdio write failed\n");
+	mutex_unlock(&mdc->lock);
+
+	return ret;
+}
+EXPORT_SYMBOL(iproc_mdio_write);
+
+static int iproc_mdio_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct iproc_mdc *mdc = &iproc_mdc;
+	struct resource *res;
+
+	mdc->dev = dev;
+	mutex_init(&mdc->lock);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	mdc->base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(mdc->base))
+		return PTR_ERR(mdc->base);
+
+	mdc->is_ready = true;
+	return 0;
+}
+
+static const struct of_device_id iproc_mdio_match_table[] = {
+	{ .compatible = "brcm,iproc-mdio" },
+	{ }
+};
+
+static struct platform_driver iproc_mdio_driver = {
+	.driver = {
+		.name = "iproc-mdio",
+		.of_match_table = iproc_mdio_match_table,
+		.suppress_bind_attrs = true,
+	},
+	.probe	= iproc_mdio_probe,
+};
+
+static int __init iproc_mdio_init(void)
+{
+	return platform_driver_register(&iproc_mdio_driver);
+}
+arch_initcall_sync(iproc_mdio_init);
+
+MODULE_AUTHOR("Ray Jui <rjui@broadcom.com>");
+MODULE_DESCRIPTION("Broadcom iPROC MDC/MDIO driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/phy/iproc_mdio_phy.h b/include/linux/phy/iproc_mdio_phy.h
new file mode 100644
index 0000000..d3e88ce
--- /dev/null
+++ b/include/linux/phy/iproc_mdio_phy.h
@@ -0,0 +1,22 @@
+/*
+ * Copyright (C) 2015 Broadcom Corporation
+ *
+ * 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.
+ */
+
+#ifndef __IPROC_MDIO_PHY_H
+#define __IPROC_MDIO_PHY_H
+
+int iproc_mdio_read(unsigned clk_div, unsigned phy_addr, unsigned reg_addr,
+		    u16 *data);
+int iproc_mdio_write(unsigned clk_div, unsigned phy_addr, unsigned reg_addr,
+		     u16 data);
+
+#endif /* __IPROC_MDIO_PHY_H */
-- 
1.7.9.5

Re: [PATCH 2/5] phy: iproc-mdio: Initial iProc MDC/MDIO support

[Paul Bolle]

On Tue, 2015-05-19 at 18:23 -0700, Ray Jui wrote:
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
>  
> +config PHY_IPROC_MDIO
> +	bool "Broadcom iProc MDC/MDIO driver"
> +	depends on ARCH_BCM_IPROC
> +	default ARCH_BCM_IPROC
> +	help
> +	  Enable this to support the iProc generic MDC/MDIO interface found
> +	  in various iProc based SoCs. The generic MDC/MDIO interface can be
> +	  used to communicate with various types of Serdes/PHYs including
> +	  Ethernet, PCIe, USB, and etc.
> +
> +	  If unsure, say N.

> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile

> +obj-$(CONFIG_PHY_IPROC_MDIO)		+= phy-iproc-mdio.o

> --- /dev/null
> +++ b/drivers/phy/phy-iproc-mdio.c

> +#include <linux/module.h>

> +static int __init iproc_mdio_init(void)
> +{
> +	return platform_driver_register(&iproc_mdio_driver);
> +}
> +arch_initcall_sync(iproc_mdio_init);
> +
> +MODULE_AUTHOR("Ray Jui <rjui@broadcom.com>");
> +MODULE_DESCRIPTION("Broadcom iPROC MDC/MDIO driver");
> +MODULE_LICENSE("GPL v2");

I guess phy-iproc-mdio.o is intended to be built-in only. If that's
correct the above three MODULE_ macros (and, probably, the include of
linux/module.h) can safely be dropped.

Thanks,


Paul Bolle

Re: [PATCH 2/5] phy: iproc-mdio: Initial iProc MDC/MDIO support

[Ray Jui]

On 5/21/2015 12:41 AM, Paul Bolle wrote:
> On Tue, 2015-05-19 at 18:23 -0700, Ray Jui wrote:
>> --- a/drivers/phy/Kconfig
>> +++ b/drivers/phy/Kconfig
>>  
>> +config PHY_IPROC_MDIO
>> +	bool "Broadcom iProc MDC/MDIO driver"
>> +	depends on ARCH_BCM_IPROC
>> +	default ARCH_BCM_IPROC
>> +	help
>> +	  Enable this to support the iProc generic MDC/MDIO interface found
>> +	  in various iProc based SoCs. The generic MDC/MDIO interface can be
>> +	  used to communicate with various types of Serdes/PHYs including
>> +	  Ethernet, PCIe, USB, and etc.
>> +
>> +	  If unsure, say N.
> 
>> --- a/drivers/phy/Makefile
>> +++ b/drivers/phy/Makefile
> 
>> +obj-$(CONFIG_PHY_IPROC_MDIO)		+= phy-iproc-mdio.o
> 
>> --- /dev/null
>> +++ b/drivers/phy/phy-iproc-mdio.c
> 
>> +#include <linux/module.h>
> 
>> +static int __init iproc_mdio_init(void)
>> +{
>> +	return platform_driver_register(&iproc_mdio_driver);
>> +}
>> +arch_initcall_sync(iproc_mdio_init);
>> +
>> +MODULE_AUTHOR("Ray Jui <rjui@broadcom.com>");
>> +MODULE_DESCRIPTION("Broadcom iPROC MDC/MDIO driver");
>> +MODULE_LICENSE("GPL v2");
> 
> I guess phy-iproc-mdio.o is intended to be built-in only. If that's
> correct the above three MODULE_ macros (and, probably, the include of
> linux/module.h) can safely be dropped.
> 
> Thanks,
> 
> 
> Paul Bolle
> 

I think we had previous discussions on this for the pinctrl patches.
People including Linus and me think this is good to be there at least
for information purpose.

I do not want to go over the same argument again. I'll leave this to the
subsystem maintainer. If the subsystem maintainer wants this out, I can
take it out. Otherwise, I'll leave it as it is.

Thanks,

Ray

Re: [PATCH 2/5] phy: iproc-mdio: Initial iProc MDC/MDIO support

[Paul Bolle]

On Thu, 2015-05-21 at 16:52 -0700, Ray Jui wrote:
> I think we had previous discussions on this for the pinctrl patches.
> People including Linus and me think this is good to be there at least
> for information purpose.

Maybe I just forgot we already discussed this. (I only remember PCI as
valuing consistency with other drivers more. But even there I remember
the maintainer wanting to clean things up, eventually.) 

> I do not want to go over the same argument again. I'll leave this to the
> subsystem maintainer. If the subsystem maintainer wants this out, I can
> take it out. Otherwise, I'll leave it as it is.

Sure. It's the maintainer's decision, at the end of the day.

Thanks,


Paul Bolle

Re: [PATCH 2/5] phy: iproc-mdio: Initial iProc MDC/MDIO support

[Kishon Vijay Abraham I]

Hi,

On Wednesday 20 May 2015 06:53 AM, Ray Jui wrote:
> This adds the support for the iProc MDC/MDIO interface. Multiple iProc
> SoCs contain the MDC/MDIO interface that can be used for the host to
> communicate with various Serdes/PHYs including Ethernet, PCIe, USB, etc.

the term phy used in this driver is misleading. It's not a PHY actually.
This sounds more like a bus driver to me and should be present in drivers/bus?

Thanks
Kishon
>
> Signed-off-by: Ray Jui <rjui@broadcom.com>
> Reviewed-by: Arun Parameswaran <aparames@broadcom.com>
> Reviewed-by: JD (Jiandong) Zheng <jdzheng@broadcom.com>
> Reviewed-by: Scott Branden <sbranden@broadcom.com>
> ---
>   drivers/phy/Kconfig                |   12 ++
>   drivers/phy/Makefile               |    1 +
>   drivers/phy/phy-iproc-mdio.c       |  249 ++++++++++++++++++++++++++++++++++++
>   include/linux/phy/iproc_mdio_phy.h |   22 ++++
>   4 files changed, 284 insertions(+)
>   create mode 100644 drivers/phy/phy-iproc-mdio.c
>   create mode 100644 include/linux/phy/iproc_mdio_phy.h
>
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index a53bd5b..2664285 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -52,6 +52,18 @@ config PHY_EXYNOS_MIPI_VIDEO
>   	  Support for MIPI CSI-2 and MIPI DSI DPHY found on Samsung S5P
>   	  and EXYNOS SoCs.
>
> +config PHY_IPROC_MDIO
> +	bool "Broadcom iProc MDC/MDIO driver"
> +	depends on ARCH_BCM_IPROC
> +	default ARCH_BCM_IPROC
> +	help
> +	  Enable this to support the iProc generic MDC/MDIO interface found
> +	  in various iProc based SoCs. The generic MDC/MDIO interface can be
> +	  used to communicate with various types of Serdes/PHYs including
> +	  Ethernet, PCIe, USB, and etc.
> +
> +	  If unsure, say N.
> +
>   config PHY_MVEBU_SATA
>   	def_bool y
>   	depends on ARCH_DOVE || MACH_DOVE || MACH_KIRKWOOD
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index f126251..d989dd7b 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -13,6 +13,7 @@ obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO)	+= phy-exynos-mipi-video.o
>   obj-$(CONFIG_PHY_MVEBU_SATA)		+= phy-mvebu-sata.o
>   obj-$(CONFIG_PHY_MIPHY28LP) 		+= phy-miphy28lp.o
>   obj-$(CONFIG_PHY_MIPHY365X)		+= phy-miphy365x.o
> +obj-$(CONFIG_PHY_IPROC_MDIO)		+= phy-iproc-mdio.o
>   obj-$(CONFIG_PHY_RCAR_GEN2)		+= phy-rcar-gen2.o
>   obj-$(CONFIG_OMAP_CONTROL_PHY)		+= phy-omap-control.o
>   obj-$(CONFIG_OMAP_USB2)			+= phy-omap-usb2.o
> diff --git a/drivers/phy/phy-iproc-mdio.c b/drivers/phy/phy-iproc-mdio.c
> new file mode 100644
> index 0000000..a37e92f
> --- /dev/null
> +++ b/drivers/phy/phy-iproc-mdio.c
> @@ -0,0 +1,249 @@
> +/*
> + * Copyright (C) 2015 Broadcom Corporation
> + *
> + * 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 <linux/clk.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/phy/iproc_mdio_phy.h>
> +
> +#define MDIO_TIMEOUT_MSEC             10
> +
> +#define MDC_CTRL_OFFSET               0x000
> +
> +#define MDC_CTRL_DIV_SHIFT            0
> +#define MDC_CTRL_DIV_WIDTH            7
> +#define MDC_MAX_DIV                   (BIT(MDC_CTRL_DIV_WIDTH) - 1)
> +#define MDC_CTRL_PRE_SHIFT            7
> +#define MDC_CTRL_BUSY_SHIFT           8
> +#define MDC_CTRL_EXT_SHIFT            9
> +#define MDC_CTRL_BTP_SHIFT            10
> +
> +#define MDC_DATA_OFFSET               0x004
> +
> +#define MDC_DATA_SHIFT                0
> +#define MDC_DATA_MASK                 0xffff
> +
> +#define MDC_DATA_TA_SHIFT             16
> +#define MDC_DATA_TA_VAL               2
> +
> +#define MDC_DATA_RA_SHIFT             18
> +#define MDC_DATA_RA_WIDTH             5
> +#define MDC_MAX_RA                    (BIT(MDC_DATA_RA_WIDTH) - 1)
> +
> +#define MDC_DATA_PA_SHIFT             23
> +#define MDC_DATA_PA_WIDTH             5
> +#define MDC_MAX_PA                    (BIT(MDC_DATA_PA_WIDTH) - 1)
> +
> +#define MDC_DATA_OP_SHIFT             28
> +#define MDC_DATA_OP_WRITE             1
> +#define MDC_DATA_OP_READ              2
> +
> +#define MDC_DATA_SB_SHIFT             30
> +
> +/**
> + * struct iproc_mdc - iProc MDC/MDIO device
> + * @dev: pointer to device
> + * @base: MDC controller register base pointer
> + * @lock: mutex to protect access to the MDC device
> + * @is_ready: flag to indicate the driver has been initialized and is ready for
> + * use
> + */
> +struct iproc_mdc {
> +	struct device *dev;
> +	void __iomem *base;
> +	struct mutex lock;
> +	bool is_ready;
> +};
> +
> +static struct iproc_mdc iproc_mdc;
> +
> +static inline int iproc_mdio_check_params(unsigned clk_div, unsigned pa,
> +					  unsigned ra)
> +{
> +	if (clk_div > MDC_MAX_DIV || pa > MDC_MAX_PA || ra > MDC_MAX_RA)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +static inline int iproc_mdio_wait_for_idle(void __iomem *base)
> +{
> +	u32 val;
> +	unsigned long timeout = jiffies + msecs_to_jiffies(MDIO_TIMEOUT_MSEC);
> +
> +	while (!time_after(jiffies, timeout)) {
> +		val = readl(base + MDC_CTRL_OFFSET);
> +		if ((val & BIT(MDC_CTRL_BUSY_SHIFT)) == 0)
> +			return 0;
> +
> +		cpu_relax();
> +	}
> +
> +	return -ETIMEDOUT;
> +}
> +
> +static inline void iproc_mdio_config_clk(void __iomem *base, unsigned clk_div)
> +{
> +	u32 val;
> +
> +	val = (clk_div << MDC_CTRL_DIV_SHIFT) | BIT(MDC_CTRL_PRE_SHIFT);
> +	writel(val, base + MDC_CTRL_OFFSET);
> +}
> +
> +static int iproc_mdio_read_write(void __iomem *base, bool dir_is_write,
> +				 unsigned clk_div, unsigned pa, unsigned ra,
> +				 u16 *data)
> +{
> +	int ret;
> +	u32 val;
> +
> +	ret = iproc_mdio_check_params(clk_div, pa, ra);
> +	if (ret)
> +		return ret;
> +
> +	ret = iproc_mdio_wait_for_idle(base);
> +	if (ret)
> +		return ret;
> +
> +	iproc_mdio_config_clk(base, clk_div);
> +
> +	ret = iproc_mdio_wait_for_idle(base);
> +	if (ret)
> +		return ret;
> +
> +	val = (MDC_DATA_TA_VAL << MDC_DATA_TA_SHIFT) |
> +		(ra << MDC_DATA_RA_SHIFT) |
> +		(pa << MDC_DATA_PA_SHIFT) |
> +		BIT(MDC_DATA_SB_SHIFT);
> +
> +	if (dir_is_write) {
> +		val |= (MDC_DATA_OP_WRITE << MDC_DATA_OP_SHIFT);
> +		val |= ((u32)(*data) & MDC_DATA_MASK);
> +	} else {
> +		val |= (MDC_DATA_OP_READ << MDC_DATA_OP_SHIFT);
> +	}
> +
> +	writel(val, base + MDC_DATA_OFFSET);
> +
> +	ret = iproc_mdio_wait_for_idle(base);
> +	if (ret)
> +		return ret;
> +
> +	if (!dir_is_write) {
> +		val = readl(base + MDC_DATA_OFFSET) & MDC_DATA_MASK;
> +		*data = (u16)val;
> +	}
> +
> +	return 0;
> +}
> +
> +
> +/**
> + * iproc_mdio_read() - read from a PHY register through the MDC interface
> + *
> + * @clk_div: MDC clock divisor
> + * @phy_addr: MDC PHY address
> + * @reg_addr: PHY register address
> + * @data: pointer to the memory where data will be stored
> + */
> +int iproc_mdio_read(unsigned clk_div, unsigned phy_addr, unsigned reg_addr,
> +		    u16 *data)
> +{
> +	int ret;
> +	struct iproc_mdc *mdc = &iproc_mdc;
> +
> +	if (!mdc->is_ready)
> +		return -ENODEV;
> +
> +	mutex_lock(&mdc->lock);
> +	ret = iproc_mdio_read_write(mdc->base, false, clk_div, phy_addr,
> +				    reg_addr, data);
> +	if (ret)
> +		dev_err(mdc->dev, "mdio read failed\n");
> +	mutex_unlock(&mdc->lock);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(iproc_mdio_read);
> +
> +/**
> + * iproc_mdio_write() - write to a PHY register through the MDC interface
> + *
> + * @clk_div: MDC clock divisor
> + * @phy_addr: MDC PHY address
> + * @reg_addr: PHY register address
> + * @data: data value to be written to the PHY register
> + */
> +int iproc_mdio_write(unsigned clk_div, unsigned phy_addr, unsigned reg_addr,
> +		     u16 data)
> +{
> +	int ret;
> +	struct iproc_mdc *mdc = &iproc_mdc;
> +
> +	if (!mdc->is_ready)
> +		return -ENODEV;
> +
> +	mutex_lock(&mdc->lock);
> +	ret = iproc_mdio_read_write(mdc->base, true, clk_div, phy_addr,
> +				    reg_addr, &data);
> +	if (ret)
> +		dev_err(mdc->dev, "mdio write failed\n");
> +	mutex_unlock(&mdc->lock);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(iproc_mdio_write);
> +
> +static int iproc_mdio_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct iproc_mdc *mdc = &iproc_mdc;
> +	struct resource *res;
> +
> +	mdc->dev = dev;
> +	mutex_init(&mdc->lock);
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	mdc->base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(mdc->base))
> +		return PTR_ERR(mdc->base);
> +
> +	mdc->is_ready = true;
> +	return 0;
> +}
> +
> +static const struct of_device_id iproc_mdio_match_table[] = {
> +	{ .compatible = "brcm,iproc-mdio" },
> +	{ }
> +};
> +
> +static struct platform_driver iproc_mdio_driver = {
> +	.driver = {
> +		.name = "iproc-mdio",
> +		.of_match_table = iproc_mdio_match_table,
> +		.suppress_bind_attrs = true,
> +	},
> +	.probe	= iproc_mdio_probe,
> +};
> +
> +static int __init iproc_mdio_init(void)
> +{
> +	return platform_driver_register(&iproc_mdio_driver);
> +}
> +arch_initcall_sync(iproc_mdio_init);
> +
> +MODULE_AUTHOR("Ray Jui <rjui@broadcom.com>");
> +MODULE_DESCRIPTION("Broadcom iPROC MDC/MDIO driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/phy/iproc_mdio_phy.h b/include/linux/phy/iproc_mdio_phy.h
> new file mode 100644
> index 0000000..d3e88ce
> --- /dev/null
> +++ b/include/linux/phy/iproc_mdio_phy.h
> @@ -0,0 +1,22 @@
> +/*
> + * Copyright (C) 2015 Broadcom Corporation
> + *
> + * 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.
> + */
> +
> +#ifndef __IPROC_MDIO_PHY_H
> +#define __IPROC_MDIO_PHY_H
> +
> +int iproc_mdio_read(unsigned clk_div, unsigned phy_addr, unsigned reg_addr,
> +		    u16 *data);
> +int iproc_mdio_write(unsigned clk_div, unsigned phy_addr, unsigned reg_addr,
> +		     u16 data);
> +
> +#endif /* __IPROC_MDIO_PHY_H */
>

Re: [PATCH 2/5] phy: iproc-mdio: Initial iProc MDC/MDIO support

[Ray Jui]

On 5/21/2015 6:12 AM, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On Wednesday 20 May 2015 06:53 AM, Ray Jui wrote:
>> This adds the support for the iProc MDC/MDIO interface. Multiple iProc
>> SoCs contain the MDC/MDIO interface that can be used for the host to
>> communicate with various Serdes/PHYs including Ethernet, PCIe, USB, etc.
> 
> the term phy used in this driver is misleading. It's not a PHY actually.
> This sounds more like a bus driver to me and should be present in
> drivers/bus?

Sure I can move it to drivers/bus/* if that's more appropriate.
Typically MDIO is used with Ethernet PHYs and most people register it to
the mii bus under drivers/net/ethernet. Our case is rare, where the same
MDIO interface is shared by the Ethernet PHY and other types of PHYs.
But yeah that does not change the fact that this is more of a bus type
of driver than a PHY driver.

I checked the maintainers' list and found apparently there's no
maintainer for drivers/bus/*? In this case, who is supposed to ack and
send a pull request for the patch?

Thanks,

Ray

Re: [PATCH 2/5] phy: iproc-mdio: Initial iProc MDC/MDIO support

[Florian Fainelli]

On 21/05/15 14:35, Ray Jui wrote:
> On 5/21/2015 6:12 AM, Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On Wednesday 20 May 2015 06:53 AM, Ray Jui wrote:
>>> This adds the support for the iProc MDC/MDIO interface. Multiple iProc
>>> SoCs contain the MDC/MDIO interface that can be used for the host to
>>> communicate with various Serdes/PHYs including Ethernet, PCIe, USB, etc.
>>
>> the term phy used in this driver is misleading. It's not a PHY actually.
>> This sounds more like a bus driver to me and should be present in
>> drivers/bus?
> 
> Sure I can move it to drivers/bus/* if that's more appropriate.
> Typically MDIO is used with Ethernet PHYs and most people register it to
> the mii bus under drivers/net/ethernet. Our case is rare, where the same
> MDIO interface is shared by the Ethernet PHY and other types of PHYs.
> But yeah that does not change the fact that this is more of a bus type
> of driver than a PHY driver.
> 
> I checked the maintainers' list and found apparently there's no
> maintainer for drivers/bus/*? In this case, who is supposed to ack and
> send a pull request for the patch?

In the case of drivers/bus/brcmstb_gisb.c I sent change via an arm-soc
pull request relating to driver/SoC code changes, and Arnd merged these
changes the same way he did merge changes for code in arch/arm/
-- 
Florian

Re: [PATCH 2/5] phy: iproc-mdio: Initial iProc MDC/MDIO support

[Ray Jui]

On 5/21/2015 2:51 PM, Florian Fainelli wrote:
> On 21/05/15 14:35, Ray Jui wrote:
>> On 5/21/2015 6:12 AM, Kishon Vijay Abraham I wrote:
>>> Hi,
>>>
>>> On Wednesday 20 May 2015 06:53 AM, Ray Jui wrote:
>>>> This adds the support for the iProc MDC/MDIO interface. Multiple iProc
>>>> SoCs contain the MDC/MDIO interface that can be used for the host to
>>>> communicate with various Serdes/PHYs including Ethernet, PCIe, USB, etc.
>>>
>>> the term phy used in this driver is misleading. It's not a PHY actually.
>>> This sounds more like a bus driver to me and should be present in
>>> drivers/bus?
>>
>> Sure I can move it to drivers/bus/* if that's more appropriate.
>> Typically MDIO is used with Ethernet PHYs and most people register it to
>> the mii bus under drivers/net/ethernet. Our case is rare, where the same
>> MDIO interface is shared by the Ethernet PHY and other types of PHYs.
>> But yeah that does not change the fact that this is more of a bus type
>> of driver than a PHY driver.
>>
>> I checked the maintainers' list and found apparently there's no
>> maintainer for drivers/bus/*? In this case, who is supposed to ack and
>> send a pull request for the patch?
> 
> In the case of drivers/bus/brcmstb_gisb.c I sent change via an arm-soc
> pull request relating to driver/SoC code changes, and Arnd merged these
> changes the same way he did merge changes for code in arch/arm/
> 

Thanks for the tip, Florian!

Ray

[PATCH 3/5] phy: cygnus: pcie: Define DT binding

[Ray Jui]

Add DT binding document for Broadcom Cygnus PCIe PHY driver

Signed-off-by: Ray Jui <rjui@broadcom.com>
Reviewed-by: Arun Parameswaran <aparames@broadcom.com>
Reviewed-by: JD (Jiandong) Zheng <jdzheng@broadcom.com>
Reviewed-by: Scott Branden <sbranden@broadcom.com>
---
 .../bindings/phy/brcm,cygnus-pcie-phy.txt          |   31 ++++++++++++++++++++
 1 file changed, 31 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/brcm,cygnus-pcie-phy.txt

diff --git a/Documentation/devicetree/bindings/phy/brcm,cygnus-pcie-phy.txt b/Documentation/devicetree/bindings/phy/brcm,cygnus-pcie-phy.txt
new file mode 100644
index 0000000..36745ee
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/brcm,cygnus-pcie-phy.txt
@@ -0,0 +1,31 @@
+Broadcom Cygnus PCIe PHY
+
+Required properties:
+- compatible: Must be "brcm,cygnus-pcie-phy"
+- reg: base address and length of the CRMU block
+- #phy-cells: must be <2>
+The first cell is the PHY ID:
+0 - PCIe RC 0
+1 - PCIe RC 1
+The second cell is the internal PHY address
+
+Example:
+	phy: phy@0301d0a0 {
+		compatible = "brcm,cygnus-pcie-phy";
+		reg = <0x0301d0a0 0x14>;
+		#phy-cells = <2>;
+	};
+
+	pcie0: pcie@18012000 {
+		...
+		...
+		phys = <&phy 0 5>;
+		phy-names = "pcie-phy";
+	};
+
+	pcie1: pcie@18013000 {
+		...
+		...
+		phys = <&phy 1 6>;
+		phy-names = "pcie-phy";
+	};
-- 
1.7.9.5

Re: [PATCH 3/5] phy: cygnus: pcie: Define DT binding

[Kishon Vijay Abraham I]

Hi,

On Wednesday 20 May 2015 06:53 AM, Ray Jui wrote:
> Add DT binding document for Broadcom Cygnus PCIe PHY driver
>
> Signed-off-by: Ray Jui <rjui@broadcom.com>
> Reviewed-by: Arun Parameswaran <aparames@broadcom.com>
> Reviewed-by: JD (Jiandong) Zheng <jdzheng@broadcom.com>
> Reviewed-by: Scott Branden <sbranden@broadcom.com>
> ---
>   .../bindings/phy/brcm,cygnus-pcie-phy.txt          |   31 ++++++++++++++++++++
>   1 file changed, 31 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/phy/brcm,cygnus-pcie-phy.txt
>
> diff --git a/Documentation/devicetree/bindings/phy/brcm,cygnus-pcie-phy.txt b/Documentation/devicetree/bindings/phy/brcm,cygnus-pcie-phy.txt
> new file mode 100644
> index 0000000..36745ee
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/brcm,cygnus-pcie-phy.txt

brcm-cygnus-pcie-phy.txt?
> @@ -0,0 +1,31 @@
> +Broadcom Cygnus PCIe PHY
> +
> +Required properties:
> +- compatible: Must be "brcm,cygnus-pcie-phy"
> +- reg: base address and length of the CRMU block
> +- #phy-cells: must be <2>
> +The first cell is the PHY ID:
> +0 - PCIe RC 0
> +1 - PCIe RC 1
> +The second cell is the internal PHY address
> +
> +Example:
> +	phy: phy@0301d0a0 {
> +		compatible = "brcm,cygnus-pcie-phy";
> +		reg = <0x0301d0a0 0x14>;
> +		#phy-cells = <2>;
> +	};

IMHO these nodes should be child nodes of the mdio node. Are they modelled that 
way?

Thanks
Kishon

Re: [PATCH 3/5] phy: cygnus: pcie: Define DT binding

[Ray Jui]

On 5/21/2015 6:14 AM, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On Wednesday 20 May 2015 06:53 AM, Ray Jui wrote:
>> Add DT binding document for Broadcom Cygnus PCIe PHY driver
>>
>> Signed-off-by: Ray Jui <rjui@broadcom.com>
>> Reviewed-by: Arun Parameswaran <aparames@broadcom.com>
>> Reviewed-by: JD (Jiandong) Zheng <jdzheng@broadcom.com>
>> Reviewed-by: Scott Branden <sbranden@broadcom.com>
>> ---
>>   .../bindings/phy/brcm,cygnus-pcie-phy.txt          |   31
>> ++++++++++++++++++++
>>   1 file changed, 31 insertions(+)
>>   create mode 100644
>> Documentation/devicetree/bindings/phy/brcm,cygnus-pcie-phy.txt
>>
>> diff --git
>> a/Documentation/devicetree/bindings/phy/brcm,cygnus-pcie-phy.txt
>> b/Documentation/devicetree/bindings/phy/brcm,cygnus-pcie-phy.txt
>> new file mode 100644
>> index 0000000..36745ee
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/phy/brcm,cygnus-pcie-phy.txt
> 
> brcm-cygnus-pcie-phy.txt?
>> @@ -0,0 +1,31 @@
>> +Broadcom Cygnus PCIe PHY
>> +
>> +Required properties:
>> +- compatible: Must be "brcm,cygnus-pcie-phy"
>> +- reg: base address and length of the CRMU block
>> +- #phy-cells: must be <2>
>> +The first cell is the PHY ID:
>> +0 - PCIe RC 0
>> +1 - PCIe RC 1
>> +The second cell is the internal PHY address
>> +
>> +Example:
>> +    phy: phy@0301d0a0 {
>> +        compatible = "brcm,cygnus-pcie-phy";
>> +        reg = <0x0301d0a0 0x14>;
>> +        #phy-cells = <2>;
>> +    };
> 
> IMHO these nodes should be child nodes of the mdio node. Are they
> modelled that way?
> 
> Thanks
> Kishon

Currently they are not. But you are right. It makes more sense for the
PHY nodes to be declared under the MDIO bus node. I will make that change.

Thanks,

Ray

Re: [PATCH 3/5] phy: cygnus: pcie: Define DT binding

[Ray Jui]

On 5/21/2015 2:59 PM, Ray Jui wrote:
> 
> 
> On 5/21/2015 6:14 AM, Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On Wednesday 20 May 2015 06:53 AM, Ray Jui wrote:
>>> Add DT binding document for Broadcom Cygnus PCIe PHY driver
>>>
>>> Signed-off-by: Ray Jui <rjui@broadcom.com>
>>> Reviewed-by: Arun Parameswaran <aparames@broadcom.com>
>>> Reviewed-by: JD (Jiandong) Zheng <jdzheng@broadcom.com>
>>> Reviewed-by: Scott Branden <sbranden@broadcom.com>
>>> ---
>>>   .../bindings/phy/brcm,cygnus-pcie-phy.txt          |   31
>>> ++++++++++++++++++++
>>>   1 file changed, 31 insertions(+)
>>>   create mode 100644
>>> Documentation/devicetree/bindings/phy/brcm,cygnus-pcie-phy.txt
>>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/phy/brcm,cygnus-pcie-phy.txt
>>> b/Documentation/devicetree/bindings/phy/brcm,cygnus-pcie-phy.txt
>>> new file mode 100644
>>> index 0000000..36745ee
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/phy/brcm,cygnus-pcie-phy.txt
>>
>> brcm-cygnus-pcie-phy.txt?

Sorry, I missed this comment on my previous reply.

We try to keep the device tree file name the same format as the
compatible string. That's why I use "brcm,...." instead of "brcm-...".

Thanks,

Ray

[PATCH 4/5] phy: cygnus: pcie: Add Cygnus PCIe PHY support

[Ray Jui]

This patch adds the PCIe PHY support for the Broadcom PCIe RC interface

Signed-off-by: Ray Jui <rjui@broadcom.com>
Reviewed-by: Arun Parameswaran <aparames@broadcom.com>
Reviewed-by: JD (Jiandong) Zheng <jdzheng@broadcom.com>
Reviewed-by: Scott Branden <sbranden@broadcom.com>
---
 drivers/phy/Kconfig           |   15 ++
 drivers/phy/Makefile          |    1 +
 drivers/phy/phy-cygnus-pcie.c |  340 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 356 insertions(+)
 create mode 100644 drivers/phy/phy-cygnus-pcie.c

diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 2664285..8250169 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -35,6 +35,21 @@ config ARMADA375_USBCLUSTER_PHY
 	depends on OF
 	select GENERIC_PHY
 
+config PHY_CYGNUS_PCIE
+	bool "Broadcom Cygnus PCIe PHY driver"
+	depends on ARCH_BCM_CYGNUS
+	select GENERIC_PHY
+	select PHY_IPROC_MDIO
+	default ARCH_BCM_CYGNUS
+	help
+	  Enable this to support the Broadcom Cygnus PCIe PHY.
+
+	  The host communicates with the PHY through the iProc MDC/MDIO
+	  interface.
+
+	  If unsure, say N.
+
+
 config PHY_DM816X_USB
 	tristate "TI dm816x USB PHY driver"
 	depends on ARCH_OMAP2PLUS
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index d989dd7b..6545950 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -5,6 +5,7 @@
 obj-$(CONFIG_GENERIC_PHY)		+= phy-core.o
 obj-$(CONFIG_PHY_BERLIN_USB)		+= phy-berlin-usb.o
 obj-$(CONFIG_PHY_BERLIN_SATA)		+= phy-berlin-sata.o
+obj-$(CONFIG_PHY_CYGNUS_PCIE)		+= phy-cygnus-pcie.o
 obj-$(CONFIG_PHY_DM816X_USB)		+= phy-dm816x-usb.o
 obj-$(CONFIG_ARMADA375_USBCLUSTER_PHY)	+= phy-armada375-usb2.o
 obj-$(CONFIG_BCM_KONA_USB2_PHY)		+= phy-bcm-kona-usb2.o
diff --git a/drivers/phy/phy-cygnus-pcie.c b/drivers/phy/phy-cygnus-pcie.c
new file mode 100644
index 0000000..5817cd4
--- /dev/null
+++ b/drivers/phy/phy-cygnus-pcie.c
@@ -0,0 +1,340 @@
+/*
+ * Copyright (C) 2015 Broadcom Corporation
+ *
+ * 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 <linux/delay.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/phy/iproc_mdio_phy.h>
+
+#define MAX_PHY_ADDR                  0x1f
+
+#define PCIE_MDCDIV_VAL               0x3e
+
+#define PCIE_BLK_ADDR_OFFSET          0x1f
+#define PCIE_BLK_ADDR_MASK            0xff00
+#define PCIE_REG_ADDR_MASK            0x1f
+
+#define PCIE_SW_CTRL0_OFFSET          0x1000
+#define PCIE_SW_PWRDOWN_SHIFT         0
+
+#define PCIE_AFE1_100MHZ_C3_OFFSET    0x2103
+#define PCIE_AFE1_100MHZ_C3_VAL       0x2b1c
+
+#define CRMU_PCIE_CFG_OFFSET          0x00
+#define CRMU_PCIE1_PHY_IDDQ_SHIFT     10
+#define CRMU_PCIE0_PHY_IDDQ_SHIFT     2
+
+enum cygnus_pcie_phy_id {
+	CYGNUS_PHY_PCIE0 = 0,
+	CYGNUS_PHY_PCIE1,
+	MAX_NUM_PHYS,
+};
+
+struct cygnus_pcie_phy_core;
+
+/**
+ * struct cygnus_pcie_phy - Cygnus PCIe PHY device
+ * @core: pointer to the Cygnus PCIe PHY core control
+ * @id: internal ID to identify the Cygnus PCIe PHY
+ * @addr: PHY address used by MDC to communicate with the PHY
+ * @phy: pointer to the kernel PHY device
+ *
+ */
+struct cygnus_pcie_phy {
+	struct cygnus_pcie_phy_core *core;
+	enum cygnus_pcie_phy_id id;
+	unsigned addr;
+	struct phy *phy;
+};
+
+/**
+ * struct cygnus_pcie_phy_core - Cygnus PCIe PHY core control
+ * @dev: pointer to device
+ * @crmu: CRMU register base
+ * @lock: mutex to protect access to individual PHYs
+ * @phys: pointer to Cygnus PHY device
+ *
+ */
+struct cygnus_pcie_phy_core {
+	struct device *dev;
+	void __iomem *crmu;
+	struct mutex lock;
+	struct cygnus_pcie_phy phys[MAX_NUM_PHYS];
+};
+
+static int cygnus_pcie_phy_reg_read(unsigned phy_addr, unsigned reg_addr,
+				    u16 *val)
+{
+	int ret;
+	u16 addr = (u16)(reg_addr & PCIE_BLK_ADDR_MASK);
+
+	ret = iproc_mdio_write(PCIE_MDCDIV_VAL, phy_addr, PCIE_BLK_ADDR_OFFSET,
+			       addr);
+	if (ret)
+		return ret;
+
+	ret = iproc_mdio_read(PCIE_MDCDIV_VAL, phy_addr,
+			      reg_addr & PCIE_REG_ADDR_MASK, val);
+	return ret;
+}
+
+static int cygnus_pcie_phy_reg_write(unsigned phy_addr, unsigned reg_addr,
+				     u16 val)
+{
+	int ret;
+	u16 addr = (u16)(reg_addr & PCIE_BLK_ADDR_MASK);
+
+	ret = iproc_mdio_write(PCIE_MDCDIV_VAL, phy_addr, PCIE_BLK_ADDR_OFFSET,
+			       addr);
+	if (ret)
+		return ret;
+
+	ret = iproc_mdio_write(PCIE_MDCDIV_VAL, phy_addr,
+			       reg_addr & PCIE_REG_ADDR_MASK, val);
+	return ret;
+}
+
+static void cygnus_pcie_afe_enable_disable(void __iomem *reg,
+					   enum cygnus_pcie_phy_id id,
+					   bool enable)
+{
+	unsigned shift;
+	u32 val;
+
+	if (id == CYGNUS_PHY_PCIE0)
+		shift = CRMU_PCIE0_PHY_IDDQ_SHIFT;
+	else
+		shift = CRMU_PCIE1_PHY_IDDQ_SHIFT;
+
+	if (enable) {
+		val = readl(reg + CRMU_PCIE_CFG_OFFSET);
+		val &= ~BIT(shift);
+		writel(val, reg + CRMU_PCIE_CFG_OFFSET);
+	} else {
+		val = readl(reg + CRMU_PCIE_CFG_OFFSET);
+		val |= BIT(shift);
+		writel(val, reg + CRMU_PCIE_CFG_OFFSET);
+	}
+
+	/* wait for AFE to come up or shut down */
+	msleep(100);
+}
+
+static int cygnus_pcie_power_config(struct cygnus_pcie_phy *phy, bool on)
+{
+	struct cygnus_pcie_phy_core *core = phy->core;
+	u16 val;
+	int ret;
+
+	if (phy->id != CYGNUS_PHY_PCIE0 && phy->id != CYGNUS_PHY_PCIE1)
+		return -EINVAL;
+
+	if (on) {
+		/* enable AFE through the CRMU interface */
+		cygnus_pcie_afe_enable_disable(core->crmu, phy->id, true);
+
+		/* to get the reference clock configured */
+		ret = cygnus_pcie_phy_reg_write(phy->addr,
+						PCIE_AFE1_100MHZ_C3_OFFSET,
+						PCIE_AFE1_100MHZ_C3_VAL);
+		if (ret)
+			goto err_disable_afe;
+		cygnus_pcie_phy_reg_read(phy->addr,
+					 PCIE_AFE1_100MHZ_C3_OFFSET, &val);
+		if (ret)
+			goto err_disable_afe;
+
+		msleep(20);
+
+		/* now toggle AFE */
+		cygnus_pcie_afe_enable_disable(core->crmu, phy->id, false);
+		cygnus_pcie_afe_enable_disable(core->crmu, phy->id, true);
+
+		dev_info(core->dev,
+			 "pcie phy on, addr:0x%02x off:0x%04x val:0x%04x\n",
+			 phy->addr, PCIE_AFE1_100MHZ_C3_OFFSET, val);
+	} else {
+		/* disable AFE through the CRMU interface */
+		cygnus_pcie_afe_enable_disable(core->crmu, phy->id, false);
+		dev_info(core->dev, "pcie phy off\n");
+	}
+
+	return 0;
+
+err_disable_afe:
+	cygnus_pcie_afe_enable_disable(core->crmu, phy->id, false);
+	return ret;
+}
+
+static int cygnus_pcie_phy_init(struct phy *p)
+{
+	return 0;
+}
+
+static int cygnus_pcie_phy_exit(struct phy *p)
+{
+	return 0;
+}
+
+static int cygnus_pcie_phy_power_on(struct phy *p)
+{
+	struct cygnus_pcie_phy *phy = phy_get_drvdata(p);
+	struct cygnus_pcie_phy_core *core = phy->core;
+	int ret = 0;
+
+	mutex_lock(&core->lock);
+
+	switch (phy->id) {
+	case CYGNUS_PHY_PCIE0:
+	case CYGNUS_PHY_PCIE1:
+		ret = cygnus_pcie_power_config(phy, true);
+		if (ret)
+			dev_err(core->dev, "unable to power on PCIe PHY\n");
+		break;
+
+	default:
+		dev_err(core->dev, "PHY id not supported\n");
+		mutex_unlock(&core->lock);
+		return -EINVAL;
+	}
+
+	mutex_unlock(&core->lock);
+
+	return ret;
+}
+
+static int cygnus_pcie_phy_power_off(struct phy *p)
+{
+	struct cygnus_pcie_phy *phy = phy_get_drvdata(p);
+	struct cygnus_pcie_phy_core *core = phy->core;
+	int ret = 0;
+
+	mutex_lock(&core->lock);
+
+	switch (phy->id) {
+	case CYGNUS_PHY_PCIE0:
+	case CYGNUS_PHY_PCIE1:
+		ret = cygnus_pcie_power_config(phy, false);
+		if (ret)
+			dev_err(core->dev, "unable to power off PCIe PHY\n");
+		break;
+
+	default:
+		dev_err(core->dev, "PHY id not supported\n");
+		mutex_unlock(&core->lock);
+		break;
+	}
+
+	mutex_unlock(&core->lock);
+
+	return ret;
+}
+
+static struct phy_ops cygnus_pcie_phy_ops = {
+	.init = cygnus_pcie_phy_init,
+	.exit = cygnus_pcie_phy_exit,
+	.power_on = cygnus_pcie_phy_power_on,
+	.power_off = cygnus_pcie_phy_power_off,
+};
+
+static struct phy *cygnus_pcie_phy_xlate(struct device *dev,
+					 struct of_phandle_args *args)
+{
+	struct cygnus_pcie_phy_core *core;
+	int id, phy_addr;
+
+	core = dev_get_drvdata(dev);
+	if (!core)
+		return ERR_PTR(-EINVAL);
+
+	id = args->args[0];
+	phy_addr = args->args[1];
+
+	if (WARN_ON(id >= MAX_NUM_PHYS))
+		return ERR_PTR(-ENODEV);
+
+	if (WARN_ON(phy_addr > MAX_PHY_ADDR))
+		return ERR_PTR(-ENODEV);
+
+	core->phys[id].addr = phy_addr;
+
+	return core->phys[id].phy;
+}
+
+static int cygnus_pcie_phy_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct cygnus_pcie_phy_core *core;
+	struct phy_provider *provider;
+	struct resource *res;
+	int i = 0;
+
+	core = devm_kzalloc(dev, sizeof(*core), GFP_KERNEL);
+	if (!core)
+		return -ENOMEM;
+
+	core->dev = dev;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	core->crmu = devm_ioremap_resource(dev, res);
+	if (IS_ERR(core->crmu))
+		return PTR_ERR(core->crmu);
+
+	mutex_init(&core->lock);
+
+	for (i = 0; i < MAX_NUM_PHYS; i++) {
+		struct cygnus_pcie_phy *p = &core->phys[i];
+
+		p->phy = devm_phy_create(dev, NULL, &cygnus_pcie_phy_ops);
+		if (IS_ERR(p->phy)) {
+			dev_err(dev, "failed to create PHY\n");
+			return PTR_ERR(p->phy);
+		}
+
+		p->core = core;
+		p->id = i;
+		phy_set_drvdata(p->phy, p);
+	}
+
+	dev_set_drvdata(dev, core);
+
+	provider = devm_of_phy_provider_register(dev, cygnus_pcie_phy_xlate);
+	if (IS_ERR(provider)) {
+		dev_err(dev, "failed to register PHY provider\n");
+		return PTR_ERR(provider);
+	}
+
+	return 0;
+}
+
+static const struct of_device_id cygnus_pcie_phy_match_table[] = {
+	{ .compatible = "brcm,cygnus-pcie-phy" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, cygnus_pcie_phy_match_table);
+
+static struct platform_driver cygnus_pcie_phy_driver = {
+	.driver = {
+		.name		= "cygnus-pcie-phy",
+		.of_match_table	= cygnus_pcie_phy_match_table,
+	},
+	.probe	= cygnus_pcie_phy_probe,
+};
+module_platform_driver(cygnus_pcie_phy_driver);
+
+MODULE_AUTHOR("Ray Jui <rjui@broadcom.com>");
+MODULE_DESCRIPTION("Broadcom Cygnus PCIe PHY driver");
+MODULE_LICENSE("GPL v2");
-- 
1.7.9.5

Re: [PATCH 4/5] phy: cygnus: pcie: Add Cygnus PCIe PHY support

[Paul Bolle]

On Tue, 2015-05-19 at 18:23 -0700, Ray Jui wrote:
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
 
> +config PHY_CYGNUS_PCIE
> +	bool "Broadcom Cygnus PCIe PHY driver"
> +	depends on ARCH_BCM_CYGNUS
> +	select GENERIC_PHY
> +	select PHY_IPROC_MDIO
> +	default ARCH_BCM_CYGNUS
> +	help
> +	  Enable this to support the Broadcom Cygnus PCIe PHY.
> +
> +	  The host communicates with the PHY through the iProc MDC/MDIO
> +	  interface.
> +
> +	  If unsure, say N.
> +
> +

> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile

> +obj-$(CONFIG_PHY_CYGNUS_PCIE)		+= phy-cygnus-pcie.o

> --- /dev/null
> +++ b/drivers/phy/phy-cygnus-pcie.c

> +#include <linux/module.h>

> +static const struct of_device_id cygnus_pcie_phy_match_table[] = {
> +	{ .compatible = "brcm,cygnus-pcie-phy" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, cygnus_pcie_phy_match_table);
> +
> +static struct platform_driver cygnus_pcie_phy_driver = {
> +	.driver = {
> +		.name		= "cygnus-pcie-phy",
> +		.of_match_table	= cygnus_pcie_phy_match_table,
> +	},
> +	.probe	= cygnus_pcie_phy_probe,
> +};
> +module_platform_driver(cygnus_pcie_phy_driver);
> +
> +MODULE_AUTHOR("Ray Jui <rjui@broadcom.com>");
> +MODULE_DESCRIPTION("Broadcom Cygnus PCIe PHY driver");
> +MODULE_LICENSE("GPL v2");

And here I guess you intend this to be buildable as a module. If that's
the intention I think PHY_CYGNUS_PCIE needs to be tristate.

Thanks,


Paul Bolle

Re: [PATCH 4/5] phy: cygnus: pcie: Add Cygnus PCIe PHY support

[Ray Jui]

On 5/21/2015 12:52 AM, Paul Bolle wrote:
> On Tue, 2015-05-19 at 18:23 -0700, Ray Jui wrote:
>> --- a/drivers/phy/Kconfig
>> +++ b/drivers/phy/Kconfig
>  
>> +config PHY_CYGNUS_PCIE
>> +	bool "Broadcom Cygnus PCIe PHY driver"
>> +	depends on ARCH_BCM_CYGNUS
>> +	select GENERIC_PHY
>> +	select PHY_IPROC_MDIO
>> +	default ARCH_BCM_CYGNUS
>> +	help
>> +	  Enable this to support the Broadcom Cygnus PCIe PHY.
>> +
>> +	  The host communicates with the PHY through the iProc MDC/MDIO
>> +	  interface.
>> +
>> +	  If unsure, say N.
>> +
>> +
> 
>> --- a/drivers/phy/Makefile
>> +++ b/drivers/phy/Makefile
> 
>> +obj-$(CONFIG_PHY_CYGNUS_PCIE)		+= phy-cygnus-pcie.o
> 
>> --- /dev/null
>> +++ b/drivers/phy/phy-cygnus-pcie.c
> 
>> +#include <linux/module.h>
> 
>> +static const struct of_device_id cygnus_pcie_phy_match_table[] = {
>> +	{ .compatible = "brcm,cygnus-pcie-phy" },
>> +	{ }
>> +};
>> +MODULE_DEVICE_TABLE(of, cygnus_pcie_phy_match_table);
>> +
>> +static struct platform_driver cygnus_pcie_phy_driver = {
>> +	.driver = {
>> +		.name		= "cygnus-pcie-phy",
>> +		.of_match_table	= cygnus_pcie_phy_match_table,
>> +	},
>> +	.probe	= cygnus_pcie_phy_probe,
>> +};
>> +module_platform_driver(cygnus_pcie_phy_driver);
>> +
>> +MODULE_AUTHOR("Ray Jui <rjui@broadcom.com>");
>> +MODULE_DESCRIPTION("Broadcom Cygnus PCIe PHY driver");
>> +MODULE_LICENSE("GPL v2");
> 
> And here I guess you intend this to be buildable as a module. If that's
> the intention I think PHY_CYGNUS_PCIE needs to be tristate.

Yes, I'll change the PCIe PHY driver to tristate in the Kconfig. Thanks!

> 
> Thanks,
> 
> 
> Paul Bolle
> 

Re: [PATCH 4/5] phy: cygnus: pcie: Add Cygnus PCIe PHY support

[Paul Bolle]

On Thu, 2015-05-21 at 16:53 -0700, Ray Jui wrote:
> Yes, I'll change the PCIe PHY driver to tristate in the Kconfig. Thanks!

To continue the discussion started in 2/5: this is my main beef with
built-in only code using module specific macros. It forces the reader of
the patch, and the code if it's already in the tree, to know the
intentions of the author. Did the author intend the code to be built-in
only or not?

And since most people appear not to be clairvoyant that's kind of hard
to do for most of us.


Paul Bolle

Re: [PATCH 4/5] phy: cygnus: pcie: Add Cygnus PCIe PHY support

[Kishon Vijay Abraham I]

On Wednesday 20 May 2015 06:53 AM, Ray Jui wrote:
> This patch adds the PCIe PHY support for the Broadcom PCIe RC interface
>
> Signed-off-by: Ray Jui <rjui@broadcom.com>
> Reviewed-by: Arun Parameswaran <aparames@broadcom.com>
> Reviewed-by: JD (Jiandong) Zheng <jdzheng@broadcom.com>
> Reviewed-by: Scott Branden <sbranden@broadcom.com>
> ---
>   drivers/phy/Kconfig           |   15 ++
>   drivers/phy/Makefile          |    1 +
>   drivers/phy/phy-cygnus-pcie.c |  340 +++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 356 insertions(+)
>   create mode 100644 drivers/phy/phy-cygnus-pcie.c
>
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index 2664285..8250169 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -35,6 +35,21 @@ config ARMADA375_USBCLUSTER_PHY
>   	depends on OF
>   	select GENERIC_PHY
>
> +config PHY_CYGNUS_PCIE
> +	bool "Broadcom Cygnus PCIe PHY driver"
> +	depends on ARCH_BCM_CYGNUS
> +	select GENERIC_PHY
> +	select PHY_IPROC_MDIO
> +	default ARCH_BCM_CYGNUS
> +	help
> +	  Enable this to support the Broadcom Cygnus PCIe PHY.
> +
> +	  The host communicates with the PHY through the iProc MDC/MDIO
> +	  interface.
> +
> +	  If unsure, say N.
> +
> +

trailing blank line
>   config PHY_DM816X_USB
>   	tristate "TI dm816x USB PHY driver"
>   	depends on ARCH_OMAP2PLUS
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index d989dd7b..6545950 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -5,6 +5,7 @@
>   obj-$(CONFIG_GENERIC_PHY)		+= phy-core.o
>   obj-$(CONFIG_PHY_BERLIN_USB)		+= phy-berlin-usb.o
>   obj-$(CONFIG_PHY_BERLIN_SATA)		+= phy-berlin-sata.o
> +obj-$(CONFIG_PHY_CYGNUS_PCIE)		+= phy-cygnus-pcie.o
>   obj-$(CONFIG_PHY_DM816X_USB)		+= phy-dm816x-usb.o
>   obj-$(CONFIG_ARMADA375_USBCLUSTER_PHY)	+= phy-armada375-usb2.o
>   obj-$(CONFIG_BCM_KONA_USB2_PHY)		+= phy-bcm-kona-usb2.o
> diff --git a/drivers/phy/phy-cygnus-pcie.c b/drivers/phy/phy-cygnus-pcie.c
> new file mode 100644
> index 0000000..5817cd4
> --- /dev/null
> +++ b/drivers/phy/phy-cygnus-pcie.c
> @@ -0,0 +1,340 @@
> +/*
> + * Copyright (C) 2015 Broadcom Corporation
> + *
> + * 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 <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/phy/iproc_mdio_phy.h>
> +
> +#define MAX_PHY_ADDR                  0x1f
> +
> +#define PCIE_MDCDIV_VAL               0x3e
> +
> +#define PCIE_BLK_ADDR_OFFSET          0x1f
> +#define PCIE_BLK_ADDR_MASK            0xff00
> +#define PCIE_REG_ADDR_MASK            0x1f
> +
> +#define PCIE_SW_CTRL0_OFFSET          0x1000
> +#define PCIE_SW_PWRDOWN_SHIFT         0
> +
> +#define PCIE_AFE1_100MHZ_C3_OFFSET    0x2103
> +#define PCIE_AFE1_100MHZ_C3_VAL       0x2b1c
> +
> +#define CRMU_PCIE_CFG_OFFSET          0x00
> +#define CRMU_PCIE1_PHY_IDDQ_SHIFT     10
> +#define CRMU_PCIE0_PHY_IDDQ_SHIFT     2
> +
> +enum cygnus_pcie_phy_id {
> +	CYGNUS_PHY_PCIE0 = 0,
> +	CYGNUS_PHY_PCIE1,
> +	MAX_NUM_PHYS,
> +};
> +
> +struct cygnus_pcie_phy_core;
> +
> +/**
> + * struct cygnus_pcie_phy - Cygnus PCIe PHY device
> + * @core: pointer to the Cygnus PCIe PHY core control
> + * @id: internal ID to identify the Cygnus PCIe PHY
> + * @addr: PHY address used by MDC to communicate with the PHY
> + * @phy: pointer to the kernel PHY device
> + *
> + */
> +struct cygnus_pcie_phy {
> +	struct cygnus_pcie_phy_core *core;
> +	enum cygnus_pcie_phy_id id;
> +	unsigned addr;
> +	struct phy *phy;
> +};
> +
> +/**
> + * struct cygnus_pcie_phy_core - Cygnus PCIe PHY core control
> + * @dev: pointer to device
> + * @crmu: CRMU register base
> + * @lock: mutex to protect access to individual PHYs
> + * @phys: pointer to Cygnus PHY device
> + *
> + */
> +struct cygnus_pcie_phy_core {
> +	struct device *dev;
> +	void __iomem *crmu;
> +	struct mutex lock;
> +	struct cygnus_pcie_phy phys[MAX_NUM_PHYS];
> +};
> +
> +static int cygnus_pcie_phy_reg_read(unsigned phy_addr, unsigned reg_addr,
> +				    u16 *val)
> +{
> +	int ret;
> +	u16 addr = (u16)(reg_addr & PCIE_BLK_ADDR_MASK);
> +
> +	ret = iproc_mdio_write(PCIE_MDCDIV_VAL, phy_addr, PCIE_BLK_ADDR_OFFSET,
> +			       addr);
> +	if (ret)
> +		return ret;
> +
> +	ret = iproc_mdio_read(PCIE_MDCDIV_VAL, phy_addr,
> +			      reg_addr & PCIE_REG_ADDR_MASK, val);
> +	return ret;
> +}
> +
> +static int cygnus_pcie_phy_reg_write(unsigned phy_addr, unsigned reg_addr,
> +				     u16 val)
> +{
> +	int ret;
> +	u16 addr = (u16)(reg_addr & PCIE_BLK_ADDR_MASK);
> +
> +	ret = iproc_mdio_write(PCIE_MDCDIV_VAL, phy_addr, PCIE_BLK_ADDR_OFFSET,
> +			       addr);
> +	if (ret)
> +		return ret;
> +
> +	ret = iproc_mdio_write(PCIE_MDCDIV_VAL, phy_addr,
> +			       reg_addr & PCIE_REG_ADDR_MASK, val);
> +	return ret;
> +}
> +
> +static void cygnus_pcie_afe_enable_disable(void __iomem *reg,
> +					   enum cygnus_pcie_phy_id id,
> +					   bool enable)
> +{
> +	unsigned shift;
> +	u32 val;
> +
> +	if (id == CYGNUS_PHY_PCIE0)
> +		shift = CRMU_PCIE0_PHY_IDDQ_SHIFT;
> +	else
> +		shift = CRMU_PCIE1_PHY_IDDQ_SHIFT;
> +
> +	if (enable) {
> +		val = readl(reg + CRMU_PCIE_CFG_OFFSET);
> +		val &= ~BIT(shift);
> +		writel(val, reg + CRMU_PCIE_CFG_OFFSET);
> +	} else {
> +		val = readl(reg + CRMU_PCIE_CFG_OFFSET);
> +		val |= BIT(shift);
> +		writel(val, reg + CRMU_PCIE_CFG_OFFSET);
> +	}
> +
> +	/* wait for AFE to come up or shut down */
> +	msleep(100);
> +}
> +
> +static int cygnus_pcie_power_config(struct cygnus_pcie_phy *phy, bool on)
> +{
> +	struct cygnus_pcie_phy_core *core = phy->core;
> +	u16 val;
> +	int ret;
> +
> +	if (phy->id != CYGNUS_PHY_PCIE0 && phy->id != CYGNUS_PHY_PCIE1)
> +		return -EINVAL;
> +
> +	if (on) {
> +		/* enable AFE through the CRMU interface */
> +		cygnus_pcie_afe_enable_disable(core->crmu, phy->id, true);
> +
> +		/* to get the reference clock configured */
> +		ret = cygnus_pcie_phy_reg_write(phy->addr,
> +						PCIE_AFE1_100MHZ_C3_OFFSET,
> +						PCIE_AFE1_100MHZ_C3_VAL);
> +		if (ret)
> +			goto err_disable_afe;
> +		cygnus_pcie_phy_reg_read(phy->addr,
> +					 PCIE_AFE1_100MHZ_C3_OFFSET, &val);
> +		if (ret)
> +			goto err_disable_afe;
> +
> +		msleep(20);
> +
> +		/* now toggle AFE */
> +		cygnus_pcie_afe_enable_disable(core->crmu, phy->id, false);
> +		cygnus_pcie_afe_enable_disable(core->crmu, phy->id, true);
> +
> +		dev_info(core->dev,
> +			 "pcie phy on, addr:0x%02x off:0x%04x val:0x%04x\n",
> +			 phy->addr, PCIE_AFE1_100MHZ_C3_OFFSET, val);
> +	} else {
> +		/* disable AFE through the CRMU interface */
> +		cygnus_pcie_afe_enable_disable(core->crmu, phy->id, false);
> +		dev_info(core->dev, "pcie phy off\n");
> +	}
> +
> +	return 0;
> +
> +err_disable_afe:
> +	cygnus_pcie_afe_enable_disable(core->crmu, phy->id, false);
> +	return ret;
> +}
> +
> +static int cygnus_pcie_phy_init(struct phy *p)
> +{
> +	return 0;
> +}
> +
> +static int cygnus_pcie_phy_exit(struct phy *p)
> +{
> +	return 0;
> +}

empty callbacks are not required.
> +
> +static int cygnus_pcie_phy_power_on(struct phy *p)
> +{
> +	struct cygnus_pcie_phy *phy = phy_get_drvdata(p);
> +	struct cygnus_pcie_phy_core *core = phy->core;
> +	int ret = 0;
> +
> +	mutex_lock(&core->lock);
> +
> +	switch (phy->id) {
> +	case CYGNUS_PHY_PCIE0:
> +	case CYGNUS_PHY_PCIE1:
> +		ret = cygnus_pcie_power_config(phy, true);
> +		if (ret)
> +			dev_err(core->dev, "unable to power on PCIe PHY\n");
> +		break;
> +
> +	default:
> +		dev_err(core->dev, "PHY id not supported\n");
> +		mutex_unlock(&core->lock);
> +		return -EINVAL;
> +	}
> +
> +	mutex_unlock(&core->lock);
> +
> +	return ret;
> +}
> +
> +static int cygnus_pcie_phy_power_off(struct phy *p)
> +{
> +	struct cygnus_pcie_phy *phy = phy_get_drvdata(p);
> +	struct cygnus_pcie_phy_core *core = phy->core;
> +	int ret = 0;
> +
> +	mutex_lock(&core->lock);
> +
> +	switch (phy->id) {
> +	case CYGNUS_PHY_PCIE0:
> +	case CYGNUS_PHY_PCIE1:
> +		ret = cygnus_pcie_power_config(phy, false);
> +		if (ret)
> +			dev_err(core->dev, "unable to power off PCIe PHY\n");
> +		break;
> +
> +	default:
> +		dev_err(core->dev, "PHY id not supported\n");
> +		mutex_unlock(&core->lock);
> +		break;
> +	}
> +
> +	mutex_unlock(&core->lock);
> +
> +	return ret;
> +}
> +
> +static struct phy_ops cygnus_pcie_phy_ops = {
> +	.init = cygnus_pcie_phy_init,
> +	.exit = cygnus_pcie_phy_exit,
> +	.power_on = cygnus_pcie_phy_power_on,
> +	.power_off = cygnus_pcie_phy_power_off,
> +};
> +
> +static struct phy *cygnus_pcie_phy_xlate(struct device *dev,
> +					 struct of_phandle_args *args)
> +{
> +	struct cygnus_pcie_phy_core *core;
> +	int id, phy_addr;
> +
> +	core = dev_get_drvdata(dev);
> +	if (!core)
> +		return ERR_PTR(-EINVAL);
> +
> +	id = args->args[0];
> +	phy_addr = args->args[1];
> +
> +	if (WARN_ON(id >= MAX_NUM_PHYS))
> +		return ERR_PTR(-ENODEV);
> +
> +	if (WARN_ON(phy_addr > MAX_PHY_ADDR))
> +		return ERR_PTR(-ENODEV);
> +
> +	core->phys[id].addr = phy_addr;
> +
> +	return core->phys[id].phy;
> +}
> +
> +static int cygnus_pcie_phy_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct cygnus_pcie_phy_core *core;
> +	struct phy_provider *provider;
> +	struct resource *res;
> +	int i = 0;
> +
> +	core = devm_kzalloc(dev, sizeof(*core), GFP_KERNEL);
> +	if (!core)
> +		return -ENOMEM;
> +
> +	core->dev = dev;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	core->crmu = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(core->crmu))
> +		return PTR_ERR(core->crmu);
> +
> +	mutex_init(&core->lock);
> +
> +	for (i = 0; i < MAX_NUM_PHYS; i++) {
> +		struct cygnus_pcie_phy *p = &core->phys[i];
> +
> +		p->phy = devm_phy_create(dev, NULL, &cygnus_pcie_phy_ops);
> +		if (IS_ERR(p->phy)) {
> +			dev_err(dev, "failed to create PHY\n");
> +			return PTR_ERR(p->phy);
> +		}
> +
> +		p->core = core;
> +		p->id = i;
> +		phy_set_drvdata(p->phy, p);
> +	}
> +
> +	dev_set_drvdata(dev, core);
> +
> +	provider = devm_of_phy_provider_register(dev, cygnus_pcie_phy_xlate);
> +	if (IS_ERR(provider)) {
> +		dev_err(dev, "failed to register PHY provider\n");
> +		return PTR_ERR(provider);
> +	}

PTR_ERR_OR_ZERO(phy_provider)?

Thanks
Kishon

[PATCH 5/5] ARM: dts: enable PCIe PHY support for Cygnus

[Ray Jui]

Added MDIO and PCIe PHY device nodes for Cygnus. The PCIe PHY is
referenced by the iProc PCIe device

Signed-off-by: Ray Jui <rjui@broadcom.com>
Reviewed-by: Arun Parameswaran <aparames@broadcom.com>
Reviewed-by: JD (Jiandong) Zheng <jdzheng@broadcom.com>
Reviewed-by: Scott Branden <sbranden@broadcom.com>
---
 arch/arm/boot/dts/bcm-cygnus.dtsi |   17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/arch/arm/boot/dts/bcm-cygnus.dtsi b/arch/arm/boot/dts/bcm-cygnus.dtsi
index 7b52c33..2710747 100644
--- a/arch/arm/boot/dts/bcm-cygnus.dtsi
+++ b/arch/arm/boot/dts/bcm-cygnus.dtsi
@@ -54,6 +54,12 @@
 
 	/include/ "bcm-cygnus-clock.dtsi"
 
+	pcie_phy: pcie_phy@0301d0a0 {
+		compatible = "brcm,cygnus-pcie-phy";
+		reg = <0x0301d0a0 0x14>;
+		#phy-cells = <2>;
+	};
+
 	pinctrl: pinctrl@0x0301d0c8 {
 		compatible = "brcm,cygnus-pinmux";
 		reg = <0x0301d0c8 0x30>,
@@ -106,6 +112,11 @@
 		};
 	};
 
+	mdio: mdio@18002000 {
+		compatible = "brcm,iproc-mdio";
+		reg = <0x18002000 0x8>;
+	};
+
 	i2c0: i2c@18008000 {
 		compatible = "brcm,cygnus-iproc-i2c", "brcm,iproc-i2c";
 		reg = <0x18008000 0x100>;
@@ -144,6 +155,9 @@
 		ranges = <0x81000000 0 0	  0x28000000 0 0x00010000
 			  0x82000000 0 0x20000000 0x20000000 0 0x04000000>;
 
+		phys = <&pcie_phy 0 5>;
+		phy-names = "pcie-phy";
+
 		status = "disabled";
 	};
 
@@ -165,6 +179,9 @@
 		ranges = <0x81000000 0 0	  0x48000000 0 0x00010000
 			  0x82000000 0 0x40000000 0x40000000 0 0x04000000>;
 
+		phys = <&pcie_phy 1 6>;
+		phy-names = "pcie-phy";
+
 		status = "disabled";
 	};
 
-- 
1.7.9.5