diff mbox series

[V4,02/13] phy: renesas: phy-rcar-gen2: Add support for r8a77470

Message ID 1554883715-41725-3-git-send-email-biju.das@bp.renesas.com (mailing list archive)
State Superseded
Delegated to: Geert Uytterhoeven
Headers show
Series Add USB2.0 support | expand

Commit Message

Biju Das April 10, 2019, 8:08 a.m. UTC 
This patch adds support for RZ/G1C (r8a77470) SoC. RZ/G1C SoC has a
PLL register shared between hsusb0 and hsusb1. Compared to other RZ/G1
and R-Car Gen2/3, USB Host needs to deassert the pll reset.

Signed-off-by: Biju Das <biju.das@bp.renesas.com>
Reviewed-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
V3-->V4
  * No Change
V2-->V3
 * No change
V1-->V2
 * Incorporated shimoda-san's review comment
 Ref: https://patchwork.kernel.org/patch/10655855/
---
 drivers/phy/renesas/phy-rcar-gen2.c | 86 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 84 insertions(+), 2 deletions(-)

Comments

Yoshihiro Shimoda April 10, 2019, 9:49 a.m. UTC | #1 
Hi Biju-san,

Thank you for the patch!

> From: Biju Das, Sent: Wednesday, April 10, 2019 5:08 PM
<snip>
>  static int rcar_gen2_phy_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -238,6 +313,8 @@ static int rcar_gen2_phy_probe(struct platform_device *pdev)
>  	struct resource *res;
>  	void __iomem *base;
>  	struct clk *clk;
> +	const struct phy_ops *gen2_phy_ops = &rcar_gen2_phy_ops;
> +	const u32 (*select_value)[PHYS_PER_CHANNEL]  = pci_select_value;
>  	int i = 0;
> 
>  	if (!dev->of_node) {
> @@ -266,6 +343,11 @@ static int rcar_gen2_phy_probe(struct platform_device *pdev)
>  	drv->clk = clk;
>  	drv->base = base;
> 
> +	if (soc_device_match(soc_r8a77470)) {

I'm not sure, but this driver also should not use the soc_device_match() like phy-rcar-gen3-usb2 driver?
If we add a special data struct and has phy_ops and select_value members, we can achieve not to use the soc_device_match().

Best regards,
Yoshihiro Shimoda
Biju Das April 10, 2019, 10:08 a.m. UTC | #2 
Hi Shimoda-San,

Thanks for the feedback.

> Subject: RE: [PATCH V4 02/13] phy: renesas: phy-rcar-gen2: Add support for
> r8a77470
> 
> Hi Biju-san,
> 
> Thank you for the patch!
> 
> > From: Biju Das, Sent: Wednesday, April 10, 2019 5:08 PM
> <snip>
> >  static int rcar_gen2_phy_probe(struct platform_device *pdev)  {
> >  	struct device *dev = &pdev->dev;
> > @@ -238,6 +313,8 @@ static int rcar_gen2_phy_probe(struct
> platform_device *pdev)
> >  	struct resource *res;
> >  	void __iomem *base;
> >  	struct clk *clk;
> > +	const struct phy_ops *gen2_phy_ops = &rcar_gen2_phy_ops;
> > +	const u32 (*select_value)[PHYS_PER_CHANNEL]  = pci_select_value;
> >  	int i = 0;
> >
> >  	if (!dev->of_node) {
> > @@ -266,6 +343,11 @@ static int rcar_gen2_phy_probe(struct
> platform_device *pdev)
> >  	drv->clk = clk;
> >  	drv->base = base;
> >
> > +	if (soc_device_match(soc_r8a77470)) {
> 
> I'm not sure, but this driver also should not use the soc_device_match() like
> phy-rcar-gen3-usb2 driver?
> If we add a special data struct and has phy_ops and select_value members,
> we can achieve not to use the soc_device_match().

There is a difference compared to phy-rcar-gen3-usb2 driver.
Here we are using generic compatible string for RZ/G1C. 

With the  new proposal, I need to make  following changes.

1)  Add r8a77470 compatible in phy_match table, fill the driver_data structure and remove soc_device_match
2)  Modify the SoC dtsi to remove generic compatible string.

I will send V5 based on  new proposal, if you are ok for removing generic compatibility.
Please let me know.

Regards,
Biju
Yoshihiro Shimoda April 10, 2019, 11:35 a.m. UTC | #3 
Hi Biju-san,

> From: Biju Das, Sent: Wednesday, April 10, 2019 7:08 PM
> 
> Hi Shimoda-San,
> 
> Thanks for the feedback.
> 
> > Subject: RE: [PATCH V4 02/13] phy: renesas: phy-rcar-gen2: Add support for
> > r8a77470
> >
> > Hi Biju-san,
> >
> > Thank you for the patch!
> >
> > > From: Biju Das, Sent: Wednesday, April 10, 2019 5:08 PM
> > <snip>
> > >  static int rcar_gen2_phy_probe(struct platform_device *pdev)  {
> > >  	struct device *dev = &pdev->dev;
> > > @@ -238,6 +313,8 @@ static int rcar_gen2_phy_probe(struct
> > platform_device *pdev)
> > >  	struct resource *res;
> > >  	void __iomem *base;
> > >  	struct clk *clk;
> > > +	const struct phy_ops *gen2_phy_ops = &rcar_gen2_phy_ops;
> > > +	const u32 (*select_value)[PHYS_PER_CHANNEL]  = pci_select_value;
> > >  	int i = 0;
> > >
> > >  	if (!dev->of_node) {
> > > @@ -266,6 +343,11 @@ static int rcar_gen2_phy_probe(struct
> > platform_device *pdev)
> > >  	drv->clk = clk;
> > >  	drv->base = base;
> > >
> > > +	if (soc_device_match(soc_r8a77470)) {
> >
> > I'm not sure, but this driver also should not use the soc_device_match() like
> > phy-rcar-gen3-usb2 driver?
> > If we add a special data struct and has phy_ops and select_value members,
> > we can achieve not to use the soc_device_match().
> 
> There is a difference compared to phy-rcar-gen3-usb2 driver.
> Here we are using generic compatible string for RZ/G1C.
> 
> With the  new proposal, I need to make  following changes.
> 
> 1)  Add r8a77470 compatible in phy_match table, fill the driver_data structure and remove soc_device_match

I think so.

> 2)  Modify the SoC dtsi to remove generic compatible string.

I don't think so. For example, r8a77990.dtsi has compatible with both "renesas,usbhs-r8a77990" and "renesas,rcar-gen3-usbhs",
and then the renesas_usbhs driver has .data for renesas,usbhs-r8a77990, but it is not compatible
with .data of "renesas,rcar-gen3-usbhs".

Simon-san, what do you think?

Best regards,
Yoshihiro Shimoda

> I will send V5 based on  new proposal, if you are ok for removing generic compatibility.
> Please let me know.
> 
> Regards,
> Biju
> 
> 
>
diff mbox series

Patch

diff --git a/drivers/phy/renesas/phy-rcar-gen2.c b/drivers/phy/renesas/phy-rcar-gen2.c
index 72eeb06..5897386 100644
--- a/drivers/phy/renesas/phy-rcar-gen2.c
+++ b/drivers/phy/renesas/phy-rcar-gen2.c
@@ -4,6 +4,7 @@ 
  *
  * Copyright (C) 2014 Renesas Solutions Corp.
  * Copyright (C) 2014 Cogent Embedded, Inc.
+ * Copyright (C) 2019 Renesas Electronics Corp.
  */
 
 #include <linux/clk.h>
@@ -15,6 +16,7 @@ 
 #include <linux/platform_device.h>
 #include <linux/spinlock.h>
 #include <linux/atomic.h>
+#include <linux/sys_soc.h>
 
 #define USBHS_LPSTS			0x02
 #define USBHS_UGCTRL			0x80
@@ -35,6 +37,8 @@ 
 #define USBHS_UGCTRL2_USB0SEL		0x00000030
 #define USBHS_UGCTRL2_USB0SEL_PCI	0x00000010
 #define USBHS_UGCTRL2_USB0SEL_HS_USB	0x00000030
+#define USBHS_UGCTRL2_USB0SEL_USB20	0x00000010
+#define USBHS_UGCTRL2_USB0SEL_HS_USB20	0x00000020
 
 /* USB General status register (UGSTS) */
 #define USBHS_UGSTS_LOCK		0x00000100 /* From technical update */
@@ -180,6 +184,60 @@  static int rcar_gen2_phy_power_off(struct phy *p)
 	return 0;
 }
 
+static int rz_g1c_phy_power_on(struct phy *p)
+{
+	struct rcar_gen2_phy *phy = phy_get_drvdata(p);
+	struct rcar_gen2_phy_driver *drv = phy->channel->drv;
+	void __iomem *base = drv->base;
+	unsigned long flags;
+	u32 value;
+
+	spin_lock_irqsave(&drv->lock, flags);
+
+	/* Power on USBHS PHY */
+	value = readl(base + USBHS_UGCTRL);
+	value &= ~USBHS_UGCTRL_PLLRESET;
+	writel(value, base + USBHS_UGCTRL);
+
+	/* As per the data sheet wait 340 micro sec for power stable */
+	udelay(340);
+
+	if (phy->select_value == USBHS_UGCTRL2_USB0SEL_HS_USB20) {
+		value = readw(base + USBHS_LPSTS);
+		value |= USBHS_LPSTS_SUSPM;
+		writew(value, base + USBHS_LPSTS);
+	}
+
+	spin_unlock_irqrestore(&drv->lock, flags);
+
+	return 0;
+}
+
+static int rz_g1c_phy_power_off(struct phy *p)
+{
+	struct rcar_gen2_phy *phy = phy_get_drvdata(p);
+	struct rcar_gen2_phy_driver *drv = phy->channel->drv;
+	void __iomem *base = drv->base;
+	unsigned long flags;
+	u32 value;
+
+	spin_lock_irqsave(&drv->lock, flags);
+	/* Power off USBHS PHY */
+	if (phy->select_value == USBHS_UGCTRL2_USB0SEL_HS_USB20) {
+		value = readw(base + USBHS_LPSTS);
+		value &= ~USBHS_LPSTS_SUSPM;
+		writew(value, base + USBHS_LPSTS);
+	}
+
+	value = readl(base + USBHS_UGCTRL);
+	value |= USBHS_UGCTRL_PLLRESET;
+	writel(value, base + USBHS_UGCTRL);
+
+	spin_unlock_irqrestore(&drv->lock, flags);
+
+	return 0;
+}
+
 static const struct phy_ops rcar_gen2_phy_ops = {
 	.init		= rcar_gen2_phy_init,
 	.exit		= rcar_gen2_phy_exit,
@@ -188,6 +246,14 @@  static const struct phy_ops rcar_gen2_phy_ops = {
 	.owner		= THIS_MODULE,
 };
 
+static const struct phy_ops rz_g1c_phy_ops = {
+	.init		= rcar_gen2_phy_init,
+	.exit		= rcar_gen2_phy_exit,
+	.power_on	= rz_g1c_phy_power_on,
+	.power_off	= rz_g1c_phy_power_off,
+	.owner		= THIS_MODULE,
+};
+
 static const struct of_device_id rcar_gen2_phy_match_table[] = {
 	{ .compatible = "renesas,usb-phy-r8a7790" },
 	{ .compatible = "renesas,usb-phy-r8a7791" },
@@ -224,11 +290,20 @@  static const u32 select_mask[] = {
 	[2]	= USBHS_UGCTRL2_USB2SEL,
 };
 
-static const u32 select_value[][PHYS_PER_CHANNEL] = {
+static const u32 pci_select_value[][PHYS_PER_CHANNEL] = {
 	[0]	= { USBHS_UGCTRL2_USB0SEL_PCI, USBHS_UGCTRL2_USB0SEL_HS_USB },
 	[2]	= { USBHS_UGCTRL2_USB2SEL_PCI, USBHS_UGCTRL2_USB2SEL_USB30 },
 };
 
+static const u32 usb20_select_value[][PHYS_PER_CHANNEL] = {
+	{ USBHS_UGCTRL2_USB0SEL_USB20, USBHS_UGCTRL2_USB0SEL_HS_USB20 },
+};
+
+static struct soc_device_attribute soc_r8a77470[] = {
+	{ .soc_id = "r8a77470" },
+	{ /* sentinel */ }
+};
+
 static int rcar_gen2_phy_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -238,6 +313,8 @@  static int rcar_gen2_phy_probe(struct platform_device *pdev)
 	struct resource *res;
 	void __iomem *base;
 	struct clk *clk;
+	const struct phy_ops *gen2_phy_ops = &rcar_gen2_phy_ops;
+	const u32 (*select_value)[PHYS_PER_CHANNEL]  = pci_select_value;
 	int i = 0;
 
 	if (!dev->of_node) {
@@ -266,6 +343,11 @@  static int rcar_gen2_phy_probe(struct platform_device *pdev)
 	drv->clk = clk;
 	drv->base = base;
 
+	if (soc_device_match(soc_r8a77470)) {
+		select_value = usb20_select_value;
+		gen2_phy_ops = &rz_g1c_phy_ops;
+	}
+
 	drv->num_channels = of_get_child_count(dev->of_node);
 	drv->channels = devm_kcalloc(dev, drv->num_channels,
 				     sizeof(struct rcar_gen2_channel),
@@ -297,7 +379,7 @@  static int rcar_gen2_phy_probe(struct platform_device *pdev)
 			phy->select_value = select_value[channel_num][n];
 
 			phy->phy = devm_phy_create(dev, NULL,
-						   &rcar_gen2_phy_ops);
+						   gen2_phy_ops);
 			if (IS_ERR(phy->phy)) {
 				dev_err(dev, "Failed to create PHY\n");
 				return PTR_ERR(phy->phy);