[PATCH v2 0/2] Renesas R-Car gen3 PCIe PHY driver/bindings

[PATCH v2 0/2] Renesas R-Car gen3 PCIe PHY driver/bindings

[Sergei Shtylyov]

Hello!

Here's the set of 2 patches against Kishon Vijay Abraham's 'linux.git' repo's
'next' branch. We're adding the Renesas R-Car gen3 (namely R-Car V3H) PCIe PHY
driver and the device tree bindings for it...

[1/2] dt-bindings: phy: Renesas R-Car Gen3 PCIe PHY bindings
[2/2] phy: Renesas R-Car gen3 PCIe PHY driver

MBR, Sergei

[PATCH v2 1/2] dt-bindings: phy: Renesas R-Car Gen3 PCIe PHY bindings

[Sergei Shtylyov]

This PHY is  still  mostly undocumented --  the only documented registers
exist on R-Car V3H (R8A77980) SoC.  Add the corresponding device tree
bindings.

Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

---
Changes in version 2:
- split from the big driver/bindings patch;
- got rid of the generic R-Car gen3 "compatible" prop value.

 Documentation/devicetree/bindings/phy/rcar-gen3-phy-pcie.txt |   24 +++++++++++
 1 file changed, 24 insertions(+)

Index: linux-phy/Documentation/devicetree/bindings/phy/rcar-gen3-phy-pcie.txt
===================================================================
--- /dev/null
+++ linux-phy/Documentation/devicetree/bindings/phy/rcar-gen3-phy-pcie.txt
@@ -0,0 +1,24 @@
+* Renesas R-Car generation 3 PCIe PHY
+
+This file provides information on what the device node for the R-Car
+generation 3 PCIe PHY contains.
+
+Required properties:
+- compatible: "renesas,r8a77980-pcie-phy" if the device is a part of the
+	      R8A77980 SoC.
+- reg: offset and length of the register block.
+- clocks: clock phandle and specifier pair.
+- power-domains: power domain phandle and specifier pair.
+- resets: reset phandle and specifier pair.
+- #phy-cells: see phy-bindings.txt in the same directory, must be <0>.
+
+Example (R-Car V3H):
+
+	pcie-phy@e65d0000 {
+		compatible = "renesas,r8a77980-pcie-phy";
+		reg = <0 0xe65d0000 0 0x8000>;
+		#phy-cells = <0>;
+		clocks = <&cpg CPG_MOD 319>;
+		power-domains = <&sysc 32>;
+		resets = <&cpg 319>;
+	};

[PATCH v2 2/2] phy: Renesas R-Car gen3 PCIe PHY driver

[Sergei Shtylyov]

This PHY is  still  mostly undocumented -- the only documented registers
exist on R-Car V3H (R8A77980) SoC where this PHY stays in a powered-down
state after a reset and thus  we  must power it up for PCIe to work...

Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

---
Changes in version 2:
- split the bindings into their own patch;
- fixed the PHY name in the heading comment;
- removed 'rcar_gen3_phy_pcie_ops' and the generic R-Car gen3 PCIe PHY entry in
  rcar_gen3_phy_pcie_match_table[];
- replaced the 'of_device_id::data' initializer with an explicit argument to
  devm_phy_create();
- uppercased the acronym in the comment;
- cleaned up the patch description.

 drivers/phy/renesas/Kconfig              |    7 +
 drivers/phy/renesas/Makefile             |    1 
 drivers/phy/renesas/phy-rcar-gen3-pcie.c |  151 +++++++++++++++++++++++++++++++
 3 files changed, 159 insertions(+)

Index: linux-phy/drivers/phy/renesas/Kconfig
===================================================================
--- linux-phy.orig/drivers/phy/renesas/Kconfig
+++ linux-phy/drivers/phy/renesas/Kconfig
@@ -8,6 +8,13 @@ config PHY_RCAR_GEN2
 	help
 	  Support for USB PHY found on Renesas R-Car generation 2 SoCs.
 
+config PHY_RCAR_GEN3_PCIE
+	tristate "Renesas R-Car generation 3 PCIe PHY driver"
+	depends on ARCH_RENESAS
+	select GENERIC_PHY
+	help
+	  Support for the PCIe PHY found on Renesas R-Car generation 3 SoCs.
+
 config PHY_RCAR_GEN3_USB2
 	tristate "Renesas R-Car generation 3 USB 2.0 PHY driver"
 	depends on ARCH_RENESAS
Index: linux-phy/drivers/phy/renesas/Makefile
===================================================================
--- linux-phy.orig/drivers/phy/renesas/Makefile
+++ linux-phy/drivers/phy/renesas/Makefile
@@ -1,3 +1,4 @@
 obj-$(CONFIG_PHY_RCAR_GEN2)		+= phy-rcar-gen2.o
+obj-$(CONFIG_PHY_RCAR_GEN3_PCIE)	+= phy-rcar-gen3-pcie.o
 obj-$(CONFIG_PHY_RCAR_GEN3_USB2)	+= phy-rcar-gen3-usb2.o
 obj-$(CONFIG_PHY_RCAR_GEN3_USB3)	+= phy-rcar-gen3-usb3.o
Index: linux-phy/drivers/phy/renesas/phy-rcar-gen3-pcie.c
===================================================================
--- /dev/null
+++ linux-phy/drivers/phy/renesas/phy-rcar-gen3-pcie.c
@@ -0,0 +1,151 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Renesas R-Car Gen3 PCIe PHY driver
+ *
+ * Copyright (C) 2018 Cogent Embedded, Inc.
+ */
+
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/phy/phy.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/spinlock.h>
+
+#define PHY_CTRL		0x4000		/* R8A77980 only */
+
+/* PHY control register (PHY_CTRL) */
+#define PHY_CTRL_PHY_PWDN	0x00000004
+
+struct rcar_gen3_phy {
+	struct phy *phy;
+	spinlock_t lock;
+	void __iomem *base;
+};
+
+static void rcar_gen3_phy_pcie_modify_reg(struct phy *p, unsigned int reg,
+					  u32 clear, u32 set)
+{
+	struct rcar_gen3_phy *phy = phy_get_drvdata(p);
+	void __iomem *base = phy->base;
+	unsigned long flags;
+	u32 value;
+
+	spin_lock_irqsave(&phy->lock, flags);
+
+	value = readl(base + reg);
+	value &= ~clear;
+	value |= set;
+	writel(value, base + reg);
+
+	spin_unlock_irqrestore(&phy->lock, flags);
+}
+
+static int r8a77980_phy_pcie_power_on(struct phy *p)
+{
+	/* Power on the PCIe PHY */
+	rcar_gen3_phy_pcie_modify_reg(p, PHY_CTRL, PHY_CTRL_PHY_PWDN, 0);
+
+	return 0;
+}
+
+static int r8a77980_phy_pcie_power_off(struct phy *p)
+{
+	/* Power off the PCIe PHY */
+	rcar_gen3_phy_pcie_modify_reg(p, PHY_CTRL, 0, PHY_CTRL_PHY_PWDN);
+
+	return 0;
+}
+
+static const struct phy_ops r8a77980_phy_pcie_ops = {
+	.power_on	= r8a77980_phy_pcie_power_on,
+	.power_off	= r8a77980_phy_pcie_power_off,
+	.owner		= THIS_MODULE,
+};
+
+static const struct of_device_id rcar_gen3_phy_pcie_match_table[] = {
+	{ .compatible = "renesas,r8a77980-pcie-phy" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, rcar_gen3_phy_pcie_match_table);
+
+static int rcar_gen3_phy_pcie_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct phy_provider *provider;
+	struct rcar_gen3_phy *phy;
+	struct resource *res;
+	void __iomem *base;
+	int error;
+
+	if (!dev->of_node) {
+		dev_err(dev,
+			"This driver must only be instantiated from the device tree\n");
+		return -EINVAL;
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	phy = devm_kzalloc(dev, sizeof(*phy), GFP_KERNEL);
+	if (!phy)
+		return -ENOMEM;
+
+	spin_lock_init(&phy->lock);
+
+	phy->base = base;
+
+	/*
+	 * devm_phy_create() will call pm_runtime_enable(&phy->dev);
+	 * And then, phy-core will manage runtime PM for this device.
+	 */
+	pm_runtime_enable(dev);
+
+	phy->phy = devm_phy_create(dev, NULL, &r8a77980_phy_pcie_ops);
+	if (IS_ERR(phy->phy)) {
+		dev_err(dev, "Failed to create PCIe PHY\n");
+		error = PTR_ERR(phy->phy);
+		goto error;
+	}
+	phy_set_drvdata(phy->phy, phy);
+
+	provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+	if (IS_ERR(provider)) {
+		dev_err(dev, "Failed to register PHY provider\n");
+		error = PTR_ERR(provider);
+		goto error;
+	}
+
+	return 0;
+
+error:
+	pm_runtime_disable(dev);
+
+	return error;
+}
+
+static int rcar_gen3_phy_pcie_remove(struct platform_device *pdev)
+{
+	pm_runtime_disable(&pdev->dev);
+
+	return 0;
+};
+
+static struct platform_driver rcar_gen3_phy_driver = {
+	.driver = {
+		.name		= "phy_rcar_gen3_pcie",
+		.of_match_table	= rcar_gen3_phy_pcie_match_table,
+	},
+	.probe	= rcar_gen3_phy_pcie_probe,
+	.remove = rcar_gen3_phy_pcie_remove,
+};
+
+module_platform_driver(rcar_gen3_phy_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Renesas R-Car Gen3 PCIe PHY");
+MODULE_AUTHOR("Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>");

Re: [PATCH v2 2/2] phy: Renesas R-Car gen3 PCIe PHY driver

[Simon Horman]

On Sun, Jun 10, 2018 at 09:24:13PM +0300, Sergei Shtylyov wrote:
> This PHY is  still  mostly undocumented -- the only documented registers
> exist on R-Car V3H (R8A77980) SoC where this PHY stays in a powered-down
> state after a reset and thus  we  must power it up for PCIe to work...
> 
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> 
> ---
> Changes in version 2:
> - split the bindings into their own patch;
> - fixed the PHY name in the heading comment;
> - removed 'rcar_gen3_phy_pcie_ops' and the generic R-Car gen3 PCIe PHY entry in
>   rcar_gen3_phy_pcie_match_table[];
> - replaced the 'of_device_id::data' initializer with an explicit argument to
>   devm_phy_create();
> - uppercased the acronym in the comment;
> - cleaned up the patch description.
> 
>  drivers/phy/renesas/Kconfig              |    7 +
>  drivers/phy/renesas/Makefile             |    1 
>  drivers/phy/renesas/phy-rcar-gen3-pcie.c |  151 +++++++++++++++++++++++++++++++
>  3 files changed, 159 insertions(+)
> 
> Index: linux-phy/drivers/phy/renesas/Kconfig
> ===================================================================
> --- linux-phy.orig/drivers/phy/renesas/Kconfig
> +++ linux-phy/drivers/phy/renesas/Kconfig
> @@ -8,6 +8,13 @@ config PHY_RCAR_GEN2
>  	help
>  	  Support for USB PHY found on Renesas R-Car generation 2 SoCs.
>  
> +config PHY_RCAR_GEN3_PCIE
> +	tristate "Renesas R-Car generation 3 PCIe PHY driver"
> +	depends on ARCH_RENESAS
> +	select GENERIC_PHY
> +	help
> +	  Support for the PCIe PHY found on Renesas R-Car generation 3 SoCs.
> +
>  config PHY_RCAR_GEN3_USB2
>  	tristate "Renesas R-Car generation 3 USB 2.0 PHY driver"
>  	depends on ARCH_RENESAS
> Index: linux-phy/drivers/phy/renesas/Makefile
> ===================================================================
> --- linux-phy.orig/drivers/phy/renesas/Makefile
> +++ linux-phy/drivers/phy/renesas/Makefile
> @@ -1,3 +1,4 @@
>  obj-$(CONFIG_PHY_RCAR_GEN2)		+= phy-rcar-gen2.o
> +obj-$(CONFIG_PHY_RCAR_GEN3_PCIE)	+= phy-rcar-gen3-pcie.o
>  obj-$(CONFIG_PHY_RCAR_GEN3_USB2)	+= phy-rcar-gen3-usb2.o
>  obj-$(CONFIG_PHY_RCAR_GEN3_USB3)	+= phy-rcar-gen3-usb3.o
> Index: linux-phy/drivers/phy/renesas/phy-rcar-gen3-pcie.c
> ===================================================================
> --- /dev/null
> +++ linux-phy/drivers/phy/renesas/phy-rcar-gen3-pcie.c
> @@ -0,0 +1,151 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Renesas R-Car Gen3 PCIe PHY driver
> + *
> + * Copyright (C) 2018 Cogent Embedded, Inc.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/phy/phy.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/spinlock.h>
> +
> +#define PHY_CTRL		0x4000		/* R8A77980 only */
> +
> +/* PHY control register (PHY_CTRL) */
> +#define PHY_CTRL_PHY_PWDN	0x00000004

nit: Can you use BIT(2) ?

> +
> +struct rcar_gen3_phy {
> +	struct phy *phy;
> +	spinlock_t lock;
> +	void __iomem *base;
> +};
> +
> +static void rcar_gen3_phy_pcie_modify_reg(struct phy *p, unsigned int reg,
> +					  u32 clear, u32 set)
> +{
> +	struct rcar_gen3_phy *phy = phy_get_drvdata(p);
> +	void __iomem *base = phy->base;
> +	unsigned long flags;
> +	u32 value;
> +
> +	spin_lock_irqsave(&phy->lock, flags);
> +
> +	value = readl(base + reg);
> +	value &= ~clear;
> +	value |= set;
> +	writel(value, base + reg);
> +
> +	spin_unlock_irqrestore(&phy->lock, flags);
> +}
> +
> +static int r8a77980_phy_pcie_power_on(struct phy *p)
> +{
> +	/* Power on the PCIe PHY */
> +	rcar_gen3_phy_pcie_modify_reg(p, PHY_CTRL, PHY_CTRL_PHY_PWDN, 0);
> +
> +	return 0;
> +}
> +
> +static int r8a77980_phy_pcie_power_off(struct phy *p)
> +{
> +	/* Power off the PCIe PHY */
> +	rcar_gen3_phy_pcie_modify_reg(p, PHY_CTRL, 0, PHY_CTRL_PHY_PWDN);
> +
> +	return 0;
> +}
> +
> +static const struct phy_ops r8a77980_phy_pcie_ops = {
> +	.power_on	= r8a77980_phy_pcie_power_on,
> +	.power_off	= r8a77980_phy_pcie_power_off,
> +	.owner		= THIS_MODULE,
> +};
> +
> +static const struct of_device_id rcar_gen3_phy_pcie_match_table[] = {
> +	{ .compatible = "renesas,r8a77980-pcie-phy" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, rcar_gen3_phy_pcie_match_table);
> +
> +static int rcar_gen3_phy_pcie_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct phy_provider *provider;
> +	struct rcar_gen3_phy *phy;
> +	struct resource *res;
> +	void __iomem *base;
> +	int error;
> +
> +	if (!dev->of_node) {
> +		dev_err(dev,
> +			"This driver must only be instantiated from the device tree\n");
> +		return -EINVAL;
> +	}
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(base))
> +		return PTR_ERR(base);
> +
> +	phy = devm_kzalloc(dev, sizeof(*phy), GFP_KERNEL);
> +	if (!phy)
> +		return -ENOMEM;
> +
> +	spin_lock_init(&phy->lock);
> +
> +	phy->base = base;
> +
> +	/*
> +	 * devm_phy_create() will call pm_runtime_enable(&phy->dev);
> +	 * And then, phy-core will manage runtime PM for this device.
> +	 */
> +	pm_runtime_enable(dev);
> +
> +	phy->phy = devm_phy_create(dev, NULL, &r8a77980_phy_pcie_ops);
> +	if (IS_ERR(phy->phy)) {
> +		dev_err(dev, "Failed to create PCIe PHY\n");
> +		error = PTR_ERR(phy->phy);
> +		goto error;
> +	}
> +	phy_set_drvdata(phy->phy, phy);
> +
> +	provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> +	if (IS_ERR(provider)) {
> +		dev_err(dev, "Failed to register PHY provider\n");
> +		error = PTR_ERR(provider);
> +		goto error;
> +	}
> +
> +	return 0;
> +
> +error:
> +	pm_runtime_disable(dev);
> +
> +	return error;
> +}
> +
> +static int rcar_gen3_phy_pcie_remove(struct platform_device *pdev)
> +{
> +	pm_runtime_disable(&pdev->dev);
> +
> +	return 0;
> +};
> +
> +static struct platform_driver rcar_gen3_phy_driver = {
> +	.driver = {
> +		.name		= "phy_rcar_gen3_pcie",
> +		.of_match_table	= rcar_gen3_phy_pcie_match_table,
> +	},
> +	.probe	= rcar_gen3_phy_pcie_probe,
> +	.remove = rcar_gen3_phy_pcie_remove,
> +};
> +
> +module_platform_driver(rcar_gen3_phy_driver);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("Renesas R-Car Gen3 PCIe PHY");
> +MODULE_AUTHOR("Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>");
> 

Re: [PATCH v2 1/2] dt-bindings: phy: Renesas R-Car Gen3 PCIe PHY bindings

[Simon Horman]

On Sun, Jun 10, 2018 at 09:22:46PM +0300, Sergei Shtylyov wrote:
> This PHY is  still  mostly undocumented --  the only documented registers
> exist on R-Car V3H (R8A77980) SoC.  Add the corresponding device tree
> bindings.
> 
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> 
> ---
> Changes in version 2:
> - split from the big driver/bindings patch;
> - got rid of the generic R-Car gen3 "compatible" prop value.
> 
>  Documentation/devicetree/bindings/phy/rcar-gen3-phy-pcie.txt |   24 +++++++++++
>  1 file changed, 24 insertions(+)

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>

> 
> Index: linux-phy/Documentation/devicetree/bindings/phy/rcar-gen3-phy-pcie.txt
> ===================================================================
> --- /dev/null
> +++ linux-phy/Documentation/devicetree/bindings/phy/rcar-gen3-phy-pcie.txt
> @@ -0,0 +1,24 @@
> +* Renesas R-Car generation 3 PCIe PHY
> +
> +This file provides information on what the device node for the R-Car
> +generation 3 PCIe PHY contains.
> +
> +Required properties:
> +- compatible: "renesas,r8a77980-pcie-phy" if the device is a part of the
> +	      R8A77980 SoC.
> +- reg: offset and length of the register block.
> +- clocks: clock phandle and specifier pair.
> +- power-domains: power domain phandle and specifier pair.
> +- resets: reset phandle and specifier pair.
> +- #phy-cells: see phy-bindings.txt in the same directory, must be <0>.
> +
> +Example (R-Car V3H):
> +
> +	pcie-phy@e65d0000 {
> +		compatible = "renesas,r8a77980-pcie-phy";
> +		reg = <0 0xe65d0000 0 0x8000>;
> +		#phy-cells = <0>;
> +		clocks = <&cpg CPG_MOD 319>;
> +		power-domains = <&sysc 32>;
> +		resets = <&cpg 319>;
> +	};
> 

Re: [PATCH v2 1/2] dt-bindings: phy: Renesas R-Car Gen3 PCIe PHY bindings

[Rob Herring]

On Sun, Jun 10, 2018 at 09:22:46PM +0300, Sergei Shtylyov wrote:
> This PHY is  still  mostly undocumented --  the only documented registers
> exist on R-Car V3H (R8A77980) SoC.  Add the corresponding device tree
> bindings.
> 
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> 
> ---
> Changes in version 2:
> - split from the big driver/bindings patch;
> - got rid of the generic R-Car gen3 "compatible" prop value.
> 
>  Documentation/devicetree/bindings/phy/rcar-gen3-phy-pcie.txt |   24 +++++++++++
>  1 file changed, 24 insertions(+)

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v2 2/2] phy: Renesas R-Car gen3 PCIe PHY driver

[Simon Horman]

On Mon, Jun 11, 2018 at 10:06:51AM +0200, Simon Horman wrote:
> On Sun, Jun 10, 2018 at 09:24:13PM +0300, Sergei Shtylyov wrote:
> > This PHY is  still  mostly undocumented -- the only documented registers
> > exist on R-Car V3H (R8A77980) SoC where this PHY stays in a powered-down
> > state after a reset and thus  we  must power it up for PCIe to work...
> > 
> > Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

Nit below notwithstanding

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>

> > 
> > ---
> > Changes in version 2:
> > - split the bindings into their own patch;
> > - fixed the PHY name in the heading comment;
> > - removed 'rcar_gen3_phy_pcie_ops' and the generic R-Car gen3 PCIe PHY entry in
> >   rcar_gen3_phy_pcie_match_table[];
> > - replaced the 'of_device_id::data' initializer with an explicit argument to
> >   devm_phy_create();
> > - uppercased the acronym in the comment;
> > - cleaned up the patch description.
> > 
> >  drivers/phy/renesas/Kconfig              |    7 +
> >  drivers/phy/renesas/Makefile             |    1 
> >  drivers/phy/renesas/phy-rcar-gen3-pcie.c |  151 +++++++++++++++++++++++++++++++
> >  3 files changed, 159 insertions(+)
> > 
> > Index: linux-phy/drivers/phy/renesas/Kconfig
> > ===================================================================
> > --- linux-phy.orig/drivers/phy/renesas/Kconfig
> > +++ linux-phy/drivers/phy/renesas/Kconfig
> > @@ -8,6 +8,13 @@ config PHY_RCAR_GEN2
> >  	help
> >  	  Support for USB PHY found on Renesas R-Car generation 2 SoCs.
> >  
> > +config PHY_RCAR_GEN3_PCIE
> > +	tristate "Renesas R-Car generation 3 PCIe PHY driver"
> > +	depends on ARCH_RENESAS
> > +	select GENERIC_PHY
> > +	help
> > +	  Support for the PCIe PHY found on Renesas R-Car generation 3 SoCs.
> > +
> >  config PHY_RCAR_GEN3_USB2
> >  	tristate "Renesas R-Car generation 3 USB 2.0 PHY driver"
> >  	depends on ARCH_RENESAS
> > Index: linux-phy/drivers/phy/renesas/Makefile
> > ===================================================================
> > --- linux-phy.orig/drivers/phy/renesas/Makefile
> > +++ linux-phy/drivers/phy/renesas/Makefile
> > @@ -1,3 +1,4 @@
> >  obj-$(CONFIG_PHY_RCAR_GEN2)		+= phy-rcar-gen2.o
> > +obj-$(CONFIG_PHY_RCAR_GEN3_PCIE)	+= phy-rcar-gen3-pcie.o
> >  obj-$(CONFIG_PHY_RCAR_GEN3_USB2)	+= phy-rcar-gen3-usb2.o
> >  obj-$(CONFIG_PHY_RCAR_GEN3_USB3)	+= phy-rcar-gen3-usb3.o
> > Index: linux-phy/drivers/phy/renesas/phy-rcar-gen3-pcie.c
> > ===================================================================
> > --- /dev/null
> > +++ linux-phy/drivers/phy/renesas/phy-rcar-gen3-pcie.c
> > @@ -0,0 +1,151 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Renesas R-Car Gen3 PCIe PHY driver
> > + *
> > + * Copyright (C) 2018 Cogent Embedded, Inc.
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/io.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/phy/phy.h>
> > +#include <linux/of_device.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/spinlock.h>
> > +
> > +#define PHY_CTRL		0x4000		/* R8A77980 only */
> > +
> > +/* PHY control register (PHY_CTRL) */
> > +#define PHY_CTRL_PHY_PWDN	0x00000004
> 
> nit: Can you use BIT(2) ?
> 
> > +
> > +struct rcar_gen3_phy {
> > +	struct phy *phy;
> > +	spinlock_t lock;
> > +	void __iomem *base;
> > +};
> > +
> > +static void rcar_gen3_phy_pcie_modify_reg(struct phy *p, unsigned int reg,
> > +					  u32 clear, u32 set)
> > +{
> > +	struct rcar_gen3_phy *phy = phy_get_drvdata(p);
> > +	void __iomem *base = phy->base;
> > +	unsigned long flags;
> > +	u32 value;
> > +
> > +	spin_lock_irqsave(&phy->lock, flags);
> > +
> > +	value = readl(base + reg);
> > +	value &= ~clear;
> > +	value |= set;
> > +	writel(value, base + reg);
> > +
> > +	spin_unlock_irqrestore(&phy->lock, flags);
> > +}
> > +
> > +static int r8a77980_phy_pcie_power_on(struct phy *p)
> > +{
> > +	/* Power on the PCIe PHY */
> > +	rcar_gen3_phy_pcie_modify_reg(p, PHY_CTRL, PHY_CTRL_PHY_PWDN, 0);
> > +
> > +	return 0;
> > +}
> > +
> > +static int r8a77980_phy_pcie_power_off(struct phy *p)
> > +{
> > +	/* Power off the PCIe PHY */
> > +	rcar_gen3_phy_pcie_modify_reg(p, PHY_CTRL, 0, PHY_CTRL_PHY_PWDN);
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct phy_ops r8a77980_phy_pcie_ops = {
> > +	.power_on	= r8a77980_phy_pcie_power_on,
> > +	.power_off	= r8a77980_phy_pcie_power_off,
> > +	.owner		= THIS_MODULE,
> > +};
> > +
> > +static const struct of_device_id rcar_gen3_phy_pcie_match_table[] = {
> > +	{ .compatible = "renesas,r8a77980-pcie-phy" },
> > +	{ }
> > +};
> > +MODULE_DEVICE_TABLE(of, rcar_gen3_phy_pcie_match_table);
> > +
> > +static int rcar_gen3_phy_pcie_probe(struct platform_device *pdev)
> > +{
> > +	struct device *dev = &pdev->dev;
> > +	struct phy_provider *provider;
> > +	struct rcar_gen3_phy *phy;
> > +	struct resource *res;
> > +	void __iomem *base;
> > +	int error;
> > +
> > +	if (!dev->of_node) {
> > +		dev_err(dev,
> > +			"This driver must only be instantiated from the device tree\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +	base = devm_ioremap_resource(dev, res);
> > +	if (IS_ERR(base))
> > +		return PTR_ERR(base);
> > +
> > +	phy = devm_kzalloc(dev, sizeof(*phy), GFP_KERNEL);
> > +	if (!phy)
> > +		return -ENOMEM;
> > +
> > +	spin_lock_init(&phy->lock);
> > +
> > +	phy->base = base;
> > +
> > +	/*
> > +	 * devm_phy_create() will call pm_runtime_enable(&phy->dev);
> > +	 * And then, phy-core will manage runtime PM for this device.
> > +	 */
> > +	pm_runtime_enable(dev);
> > +
> > +	phy->phy = devm_phy_create(dev, NULL, &r8a77980_phy_pcie_ops);
> > +	if (IS_ERR(phy->phy)) {
> > +		dev_err(dev, "Failed to create PCIe PHY\n");
> > +		error = PTR_ERR(phy->phy);
> > +		goto error;
> > +	}
> > +	phy_set_drvdata(phy->phy, phy);
> > +
> > +	provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> > +	if (IS_ERR(provider)) {
> > +		dev_err(dev, "Failed to register PHY provider\n");
> > +		error = PTR_ERR(provider);
> > +		goto error;
> > +	}
> > +
> > +	return 0;
> > +
> > +error:
> > +	pm_runtime_disable(dev);
> > +
> > +	return error;
> > +}
> > +
> > +static int rcar_gen3_phy_pcie_remove(struct platform_device *pdev)
> > +{
> > +	pm_runtime_disable(&pdev->dev);
> > +
> > +	return 0;
> > +};
> > +
> > +static struct platform_driver rcar_gen3_phy_driver = {
> > +	.driver = {
> > +		.name		= "phy_rcar_gen3_pcie",
> > +		.of_match_table	= rcar_gen3_phy_pcie_match_table,
> > +	},
> > +	.probe	= rcar_gen3_phy_pcie_probe,
> > +	.remove = rcar_gen3_phy_pcie_remove,
> > +};
> > +
> > +module_platform_driver(rcar_gen3_phy_driver);
> > +
> > +MODULE_LICENSE("GPL v2");
> > +MODULE_DESCRIPTION("Renesas R-Car Gen3 PCIe PHY");
> > +MODULE_AUTHOR("Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>");
> > 
> 

Re: [PATCH v2 2/2] phy: Renesas R-Car gen3 PCIe PHY driver

[Eugeniu Rosca]

Hello Sergei,
Cc: Geert, Shimoda-san, Marek

On Sun, Jun 10, 2018 at 09:24:13PM +0300, Sergei Shtylyov wrote:
> This PHY is  still  mostly undocumented -- the only documented registers
> exist on R-Car V3H (R8A77980) SoC where this PHY stays in a powered-down
> state after a reset and thus  we  must power it up for PCIe to work...

Indeed, this [1] PCIE PHY driver looks entirely V3H-focused and looking
at the "Table 54.5 PCIE Controller Phy Register Configuration" in Rcar3
HW User’s Manual Rev.2.00 Jul 2019, _all_ except one PCIE PHY register
(PHY_CLK_RST) exist on V3H and no other Rcar3 SoC.

So, except PHY_CLK_RST, this driver appears to be doomed to R8A77980.
Ironically, PHY_CLK_RST is exactly the register we now need to manage
to implement "Internal Reference Clock Supply" (HW man Chapter 54.3.14).

Just to avoid any surprises on our side, do you see any issues in
extending the driver to the whole R-Car3 family, even if only for the
sake of controlling the non-V3H PHY_CLK_RST register?

[1] 2ce7f2f425ef74 ("phy: Renesas R-Car gen3 PCIe PHY driver")

-- 
Best Regards,
Eugeniu

Re: [PATCH v2 2/2] phy: Renesas R-Car gen3 PCIe PHY driver

[Sergei Shtylyov]

Hello!

On 11/04/2019 04:27 PM, Eugeniu Rosca wrote:

[...]

>> This PHY is  still  mostly undocumented -- the only documented registers
>> exist on R-Car V3H (R8A77980) SoC where this PHY stays in a powered-down
>> state after a reset and thus  we  must power it up for PCIe to work...
> 
> Indeed, this [1] PCIE PHY driver looks entirely V3H-focused and looking
> at the "Table 54.5 PCIE Controller Phy Register Configuration" in Rcar3
> HW User’s Manual Rev.2.00 Jul 2019, _all_ except one PCIE PHY register
> (PHY_CLK_RST) exist on V3H and no other Rcar3 SoC.
> 
> So, except PHY_CLK_RST, this driver appears to be doomed to R8A77980.
> Ironically, PHY_CLK_RST is exactly the register we now need to manage
> to implement "Internal Reference Clock Supply" (HW man Chapter 54.3.14).

   Do you have in mind a working approach to switch internal/external clocks?
phy_set_mode()?

> Just to avoid any surprises on our side, do you see any issues in
> extending the driver to the whole R-Car3 family, even if only for the
> sake of controlling the non-V3H PHY_CLK_RST register?

   Depends on the previous question...

> [1] 2ce7f2f425ef74 ("phy: Renesas R-Car gen3 PCIe PHY driver")

MBR, Sergei

Re: [PATCH v2 2/2] phy: Renesas R-Car gen3 PCIe PHY driver

[Eugeniu Rosca]

Hi Sergei,

On Tue, Nov 05, 2019 at 11:46:18PM +0300, Sergei Shtylyov wrote:
> Hello!
> 
> On 11/04/2019 04:27 PM, Eugeniu Rosca wrote:
> 
> [...]
> 
> >> This PHY is  still  mostly undocumented -- the only documented registers
> >> exist on R-Car V3H (R8A77980) SoC where this PHY stays in a powered-down
> >> state after a reset and thus  we  must power it up for PCIe to work...
> > 
> > Indeed, this [1] PCIE PHY driver looks entirely V3H-focused and looking
> > at the "Table 54.5 PCIE Controller Phy Register Configuration" in Rcar3
> > HW User’s Manual Rev.2.00 Jul 2019, _all_ except one PCIE PHY register
> > (PHY_CLK_RST) exist on V3H and no other Rcar3 SoC.
> > 
> > So, except PHY_CLK_RST, this driver appears to be doomed to R8A77980.
> > Ironically, PHY_CLK_RST is exactly the register we now need to manage
> > to implement "Internal Reference Clock Supply" (HW man Chapter 54.3.14).
> 
>    Do you have in mind a working approach to switch internal/external clocks?
> phy_set_mode()?

Thanks to Andrew (CC), the approach is to implement a new binding
"use-internal-reference-clock" and, based on its setting in DT,
write the appropriate value (0) into the PHY_REF_USE_PAD bit of
PHY_CLK_RST register, just as described in Chapter "54.3.14 Internal
Reference Clock Supply" of R-Car3 HW manual. We don't employ the
generic phy_set_mode().

If you are fine with this approach, then we will try to find some time
to push Andrew's patches to you in the following days.

> > Just to avoid any surprises on our side, do you see any issues in
> > extending the driver to the whole R-Car3 family, even if only for the
> > sake of controlling the non-V3H PHY_CLK_RST register?
> 
>    Depends on the previous question...
> 
> > [1] 2ce7f2f425ef74 ("phy: Renesas R-Car gen3 PCIe PHY driver")

-- 
Best Regards,
Eugeniu

Re: [PATCH v2 2/2] phy: Renesas R-Car gen3 PCIe PHY driver

[Sergei Shtylyov]

Hello!

On 11/06/2019 03:10 PM, Eugeniu Rosca wrote:

   Sorry for a late reply -- I'm on vacation.

[...]
>>>> This PHY is  still  mostly undocumented -- the only documented registers
>>>> exist on R-Car V3H (R8A77980) SoC where this PHY stays in a powered-down
>>>> state after a reset and thus  we  must power it up for PCIe to work...
>>>
>>> Indeed, this [1] PCIE PHY driver looks entirely V3H-focused and looking
>>> at the "Table 54.5 PCIE Controller Phy Register Configuration" in Rcar3
>>> HW User’s Manual Rev.2.00 Jul 2019, _all_ except one PCIE PHY register
>>> (PHY_CLK_RST) exist on V3H and no other Rcar3 SoC.
>>>
>>> So, except PHY_CLK_RST, this driver appears to be doomed to R8A77980.
>>> Ironically, PHY_CLK_RST is exactly the register we now need to manage
>>> to implement "Internal Reference Clock Supply" (HW man Chapter 54.3.14).
>>
>>    Do you have in mind a working approach to switch internal/external clocks?
>> phy_set_mode()?
> 
> Thanks to Andrew (CC), the approach is to implement a new binding
> "use-internal-reference-clock" and, based on its setting in DT,

   So boolean prop... should be OK.

> write the appropriate value (0) into the PHY_REF_USE_PAD bit of
> PHY_CLK_RST register, just as described in Chapter "54.3.14 Internal
> Reference Clock Supply" of R-Car3 HW manual. We don't employ the
> generic phy_set_mode().
> 
> If you are fine with this approach, then we will try to find some time
> to push Andrew's patches to you in the following days.

>>> Just to avoid any surprises on our side, do you see any issues in
>>> extending the driver to the whole R-Car3 family, even if only for the
>>> sake of controlling the non-V3H PHY_CLK_RST register?
>>
>>    Depends on the previous question...

   Wait, I don't see why we should support all the family. The PCIe PHY
registers don't exist on each member of the family, according to the
manual. Please continue using the chip spoecific "compatible" prop.

>>> [1] 2ce7f2f425ef74 ("phy: Renesas R-Car gen3 PCIe PHY driver")

MBR, Sergei

Re: [PATCH v2 2/2] phy: Renesas R-Car gen3 PCIe PHY driver

[Eugeniu Rosca]

Hi Sergei,

On Wed, Nov 13, 2019 at 02:51:48PM +0300, Sergei Shtylyov wrote:
> Hello!
> 
> On 11/06/2019 03:10 PM, Eugeniu Rosca wrote:
> 
>    Sorry for a late reply -- I'm on vacation.

No worries. Please, enjoy your vacation :)

> [...]
> >>>> This PHY is  still  mostly undocumented -- the only documented registers
> >>>> exist on R-Car V3H (R8A77980) SoC where this PHY stays in a powered-down
> >>>> state after a reset and thus  we  must power it up for PCIe to work...
> >>>
> >>> Indeed, this [1] PCIE PHY driver looks entirely V3H-focused and looking
> >>> at the "Table 54.5 PCIE Controller Phy Register Configuration" in Rcar3
> >>> HW User’s Manual Rev.2.00 Jul 2019, _all_ except one PCIE PHY register
> >>> (PHY_CLK_RST) exist on V3H and no other Rcar3 SoC.
> >>>
> >>> So, except PHY_CLK_RST, this driver appears to be doomed to R8A77980.
> >>> Ironically, PHY_CLK_RST is exactly the register we now need to manage
> >>> to implement "Internal Reference Clock Supply" (HW man Chapter 54.3.14).
> >>
> >>    Do you have in mind a working approach to switch internal/external clocks?
> >> phy_set_mode()?
> > 
> > Thanks to Andrew (CC), the approach is to implement a new binding
> > "use-internal-reference-clock" and, based on its setting in DT,
> 
>    So boolean prop... should be OK.
> 
> > write the appropriate value (0) into the PHY_REF_USE_PAD bit of
> > PHY_CLK_RST register, just as described in Chapter "54.3.14 Internal
> > Reference Clock Supply" of R-Car3 HW manual. We don't employ the
> > generic phy_set_mode().
> > 
> > If you are fine with this approach, then we will try to find some time
> > to push Andrew's patches to you in the following days.
> 
> >>> Just to avoid any surprises on our side, do you see any issues in
> >>> extending the driver to the whole R-Car3 family, even if only for the
> >>> sake of controlling the non-V3H PHY_CLK_RST register?
> >>
> >>    Depends on the previous question...
> 
>    Wait, I don't see why we should support all the family. The PCIe PHY
> registers don't exist on each member of the family, according to the
> manual. Please continue using the chip spoecific "compatible" prop.

Given the "phy-rcar-gen3-pcie.c" driver name, I would expect that
enriching its behavior with Gen3 non-V3H functionality is legitimate.
Hope to see your review comments when the series is submitted. TIA.

> 
> >>> [1] 2ce7f2f425ef74 ("phy: Renesas R-Car gen3 PCIe PHY driver")
> 
> MBR, Sergei

-- 
Best Regards,
Eugeniu