| Message ID | 20220126141340.234125-4-alexander.stein@ew.tq-group.com |
|---|---|
| State | Accepted |
| Commit | 9d52107185b646d449b682e10e95e6b3037b79cf |
| Headers | show |
| Series | i.MX8MP: more USB3 glue layer feature support | expand |
> -----Original Message----- > From: Alexander Stein <alexander.stein@ew.tq-group.com> > Sent: Wednesday, January 26, 2022 10:14 PM > To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Rob Herring > <robh+dt@kernel.org>; Shawn Guo <shawnguo@kernel.org>; Sascha Hauer > <s.hauer@pengutronix.de>; Fabio Estevam <festevam@gmail.com> > Cc: Alexander Stein <alexander.stein@ew.tq-group.com>; dl-linux-imx > <linux-imx@nxp.com>; linux-usb@vger.kernel.org; > devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; Jun Li > <jun.li@nxp.com> > Subject: [PATCH v4 3/4] usb: dwc3: imx8mp: Add support for setting SOC > specific flags > > The i.MX8MP glue layer has support for the following flags: > * over-current polarity > * PWR pad polarity > * controlling PPC flag in HCCPARAMS register > * permanent port attach for usb2 & usb3 port > > Allow setting these flags by supporting specific flags in the glue node. > In order to get this to work an additional IORESOURCE_MEM and clock is > necessary. For backward compatibility this is purely optional. > > Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com> > --- > drivers/usb/dwc3/dwc3-imx8mp.c | 62 ++++++++++++++++++++++++++++++++++ > 1 file changed, 62 insertions(+) > > diff --git a/drivers/usb/dwc3/dwc3-imx8mp.c > b/drivers/usb/dwc3/dwc3-imx8mp.c > index 1c8fe657b3a9..5a96b66bf237 100644 > --- a/drivers/usb/dwc3/dwc3-imx8mp.c > +++ b/drivers/usb/dwc3/dwc3-imx8mp.c > @@ -36,10 +36,22 @@ > > #define USB_WAKEUP_EN_MASK GENMASK(5, 0) > > +/* USB glue registers */ > +#define USB_CTRL0 0x00 > +#define USB_CTRL1 0x04 > + > +#define USB_CTRL0_PORTPWR_EN BIT(12) /* 1 - PPC enabled (default) */ > +#define USB_CTRL0_USB3_FIXED BIT(22) /* 1 - USB3 permanent attached */ > +#define USB_CTRL0_USB2_FIXED BIT(23) /* 1 - USB2 permanent attached */ > + > +#define USB_CTRL1_OC_POLARITY BIT(16) /* 0 - HIGH / 1 - LOW */ > +#define USB_CTRL1_PWR_POLARITY BIT(17) /* 0 - HIGH / 1 - LOW */ > + > struct dwc3_imx8mp { > struct device *dev; > struct platform_device *dwc3; > void __iomem *hsio_blk_base; > + void __iomem *glue_base; > struct clk *hsio_clk; > struct clk *suspend_clk; > int irq; > @@ -47,6 +59,42 @@ struct dwc3_imx8mp { > bool wakeup_pending; > }; > > +static void imx8mp_configure_glue(struct dwc3_imx8mp *dwc3_imx) > +{ > + struct device *dev = dwc3_imx->dev; > + u32 value; > + > + if (!dwc3_imx->glue_base) > + return; > + > + value = readl(dwc3_imx->glue_base + USB_CTRL0); > + > + if (device_property_read_bool(dev, "fsl,permanently-attached")) > + value |= (USB_CTRL0_USB2_FIXED | USB_CTRL0_USB3_FIXED); > + else > + value &= ~(USB_CTRL0_USB2_FIXED | USB_CTRL0_USB3_FIXED); > + > + if (device_property_read_bool(dev, > "fsl,disable-port-power-control")) > + value &= ~(USB_CTRL0_PORTPWR_EN); > + else > + value |= USB_CTRL0_PORTPWR_EN; > + > + writel(value, dwc3_imx->glue_base + USB_CTRL0); > + > + value = readl(dwc3_imx->glue_base + USB_CTRL1); > + if (device_property_read_bool(dev, "fsl,over-current-active-low")) > + value |= USB_CTRL1_OC_POLARITY; > + else > + value &= ~USB_CTRL1_OC_POLARITY; > + > + if (device_property_read_bool(dev, "fsl,power-active-low")) > + value |= USB_CTRL1_PWR_POLARITY; > + else > + value &= ~USB_CTRL1_PWR_POLARITY; I think you don't need a value update&write if property is not present. > + > + writel(value, dwc3_imx->glue_base + USB_CTRL1); > +} > + > static void dwc3_imx8mp_wakeup_enable(struct dwc3_imx8mp *dwc3_imx) > { > struct dwc3 *dwc3 = platform_get_drvdata(dwc3_imx->dwc3); > @@ -100,6 +148,7 @@ static int dwc3_imx8mp_probe(struct platform_device > *pdev) > struct device *dev = &pdev->dev; > struct device_node *dwc3_np, *node = dev->of_node; > struct dwc3_imx8mp *dwc3_imx; > + struct resource *res; Looks like this is not used anywhere, remove it. > int err, irq; > > if (!node) { > @@ -119,6 +168,15 @@ static int dwc3_imx8mp_probe(struct platform_device > *pdev) > if (IS_ERR(dwc3_imx->hsio_blk_base)) > return PTR_ERR(dwc3_imx->hsio_blk_base); > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 1); > + if (!res) { > + dev_warn(dev, "Base address for glue layer missing. Continuing > without, some features are missing though."); > + } else { > + dwc3_imx->glue_base = devm_ioremap_resource(dev, res); > + if (IS_ERR(dwc3_imx->glue_base)) > + return PTR_ERR(dwc3_imx->glue_base); > + } > + > dwc3_imx->hsio_clk = devm_clk_get(dev, "hsio"); > if (IS_ERR(dwc3_imx->hsio_clk)) { > err = PTR_ERR(dwc3_imx->hsio_clk); > @@ -152,6 +210,8 @@ static int dwc3_imx8mp_probe(struct platform_device > *pdev) > } > dwc3_imx->irq = irq; > > + imx8mp_configure_glue(dwc3_imx); > + > pm_runtime_set_active(dev); > pm_runtime_enable(dev); > err = pm_runtime_get_sync(dev); > @@ -252,6 +312,8 @@ static int __maybe_unused dwc3_imx8mp_resume(struct > dwc3_imx8mp *dwc3_imx, > dwc3_imx8mp_wakeup_disable(dwc3_imx); > dwc3_imx->pm_suspended = false; > > + imx8mp_configure_glue(dwc3_imx); > + I guess this is to restore the settings if we experienced a power lost, Please add some comments. thanks Li Jun > if (dwc3_imx->wakeup_pending) { > dwc3_imx->wakeup_pending = false; > if (dwc->current_dr_role == DWC3_GCTL_PRTCAP_DEVICE) { > -- > 2.25.1
Hi Li Jun, thanks for your feedback. Am Freitag, 18. Februar 2022, 07:57:33 CET schrieb Jun Li: > > -----Original Message----- > > From: Alexander Stein <alexander.stein@ew.tq-group.com> > > Sent: Wednesday, January 26, 2022 10:14 PM > > To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Rob Herring > > <robh+dt@kernel.org>; Shawn Guo <shawnguo@kernel.org>; Sascha Hauer > > <s.hauer@pengutronix.de>; Fabio Estevam <festevam@gmail.com> > > Cc: Alexander Stein <alexander.stein@ew.tq-group.com>; dl-linux-imx > > <linux-imx@nxp.com>; linux-usb@vger.kernel.org; > > devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; Jun Li > > <jun.li@nxp.com> > > Subject: [PATCH v4 3/4] usb: dwc3: imx8mp: Add support for setting SOC > > specific flags > > > > The i.MX8MP glue layer has support for the following flags: > > * over-current polarity > > * PWR pad polarity > > * controlling PPC flag in HCCPARAMS register > > * permanent port attach for usb2 & usb3 port > > > > Allow setting these flags by supporting specific flags in the glue node. > > In order to get this to work an additional IORESOURCE_MEM and clock is > > necessary. For backward compatibility this is purely optional. > > > > Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com> > > --- > > > > drivers/usb/dwc3/dwc3-imx8mp.c | 62 ++++++++++++++++++++++++++++++++++ > > 1 file changed, 62 insertions(+) > > > > diff --git a/drivers/usb/dwc3/dwc3-imx8mp.c > > b/drivers/usb/dwc3/dwc3-imx8mp.c > > index 1c8fe657b3a9..5a96b66bf237 100644 > > --- a/drivers/usb/dwc3/dwc3-imx8mp.c > > +++ b/drivers/usb/dwc3/dwc3-imx8mp.c > > @@ -36,10 +36,22 @@ > > > > #define USB_WAKEUP_EN_MASK GENMASK(5, 0) > > > > +/* USB glue registers */ > > +#define USB_CTRL0 0x00 > > +#define USB_CTRL1 0x04 > > + > > +#define USB_CTRL0_PORTPWR_EN BIT(12) /* 1 - PPC enabled (default) */ > > +#define USB_CTRL0_USB3_FIXED BIT(22) /* 1 - USB3 permanent attached */ > > +#define USB_CTRL0_USB2_FIXED BIT(23) /* 1 - USB2 permanent attached */ > > + > > +#define USB_CTRL1_OC_POLARITY BIT(16) /* 0 - HIGH / 1 - LOW */ > > +#define USB_CTRL1_PWR_POLARITY BIT(17) /* 0 - HIGH / 1 - LOW */ > > + > > > > struct dwc3_imx8mp { > > > > struct device *dev; > > struct platform_device *dwc3; > > void __iomem *hsio_blk_base; > > > > + void __iomem *glue_base; > > > > struct clk *hsio_clk; > > struct clk *suspend_clk; > > int irq; > > > > @@ -47,6 +59,42 @@ struct dwc3_imx8mp { > > > > bool wakeup_pending; > > > > }; > > > > +static void imx8mp_configure_glue(struct dwc3_imx8mp *dwc3_imx) > > +{ > > + struct device *dev = dwc3_imx->dev; > > + u32 value; > > + > > + if (!dwc3_imx->glue_base) > > + return; > > + > > + value = readl(dwc3_imx->glue_base + USB_CTRL0); > > + > > + if (device_property_read_bool(dev, "fsl,permanently-attached")) > > + value |= (USB_CTRL0_USB2_FIXED | USB_CTRL0_USB3_FIXED); > > + else > > + value &= ~(USB_CTRL0_USB2_FIXED | USB_CTRL0_USB3_FIXED); > > + > > + if (device_property_read_bool(dev, > > "fsl,disable-port-power-control")) > > + value &= ~(USB_CTRL0_PORTPWR_EN); > > + else > > + value |= USB_CTRL0_PORTPWR_EN; > > + > > + writel(value, dwc3_imx->glue_base + USB_CTRL0); > > + > > + value = readl(dwc3_imx->glue_base + USB_CTRL1); > > + if (device_property_read_bool(dev, "fsl,over-current-active-low")) > > + value |= USB_CTRL1_OC_POLARITY; > > + else > > + value &= ~USB_CTRL1_OC_POLARITY; > > + > > + if (device_property_read_bool(dev, "fsl,power-active-low")) > > + value |= USB_CTRL1_PWR_POLARITY; > > + else > > + value &= ~USB_CTRL1_PWR_POLARITY; > > I think you don't need a value update&write if property is not present. I get what you intend here, but this assumes some (sane) default which might be true in every case. But I prefer configuring this without any assumption about previous settings. > > + > > + writel(value, dwc3_imx->glue_base + USB_CTRL1); > > +} > > + > > > > static void dwc3_imx8mp_wakeup_enable(struct dwc3_imx8mp *dwc3_imx) > > { > > > > struct dwc3 *dwc3 = platform_get_drvdata(dwc3_imx->dwc3); > > > > @@ -100,6 +148,7 @@ static int dwc3_imx8mp_probe(struct platform_device > > *pdev) > > > > struct device *dev = &pdev->dev; > > struct device_node *dwc3_np, *node = dev->of_node; > > struct dwc3_imx8mp *dwc3_imx; > > > > + struct resource *res; > > Looks like this is not used anywhere, remove it. It actually is, please refer to the next hunk. Both is inside dwc3_imx8mp_probe(). Probably the context of the diff is a bit misleading. > > int err, irq; > > > > if (!node) { > > > > @@ -119,6 +168,15 @@ static int dwc3_imx8mp_probe(struct platform_device > > *pdev) > > > > if (IS_ERR(dwc3_imx->hsio_blk_base)) > > > > return PTR_ERR(dwc3_imx->hsio_blk_base); > > > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 1); > > + if (!res) { > > + dev_warn(dev, "Base address for glue layer missing. Continuing > > without, some features are missing though."); > > + } else { > > + dwc3_imx->glue_base = devm_ioremap_resource(dev, res); > > + if (IS_ERR(dwc3_imx->glue_base)) > > + return PTR_ERR(dwc3_imx->glue_base); > > + } > > + > > > > dwc3_imx->hsio_clk = devm_clk_get(dev, "hsio"); > > if (IS_ERR(dwc3_imx->hsio_clk)) { > > > > err = PTR_ERR(dwc3_imx->hsio_clk); > > > > @@ -152,6 +210,8 @@ static int dwc3_imx8mp_probe(struct platform_device > > *pdev) > > > > } > > dwc3_imx->irq = irq; > > > > + imx8mp_configure_glue(dwc3_imx); > > + > > > > pm_runtime_set_active(dev); > > pm_runtime_enable(dev); > > err = pm_runtime_get_sync(dev); > > > > @@ -252,6 +312,8 @@ static int __maybe_unused dwc3_imx8mp_resume(struct > > dwc3_imx8mp *dwc3_imx, > > > > dwc3_imx8mp_wakeup_disable(dwc3_imx); > > dwc3_imx->pm_suspended = false; > > > > + imx8mp_configure_glue(dwc3_imx); > > + > > I guess this is to restore the settings if we experienced a power lost, > Please add some comments. You are right, makes sense. I'll add a comment. Regards, Alexander > > if (dwc3_imx->wakeup_pending) { > > > > dwc3_imx->wakeup_pending = false; > > if (dwc->current_dr_role == DWC3_GCTL_PRTCAP_DEVICE) { > > > > -- > > 2.25.1
diff --git a/drivers/usb/dwc3/dwc3-imx8mp.c b/drivers/usb/dwc3/dwc3-imx8mp.c index 1c8fe657b3a9..5a96b66bf237 100644 --- a/drivers/usb/dwc3/dwc3-imx8mp.c +++ b/drivers/usb/dwc3/dwc3-imx8mp.c @@ -36,10 +36,22 @@ #define USB_WAKEUP_EN_MASK GENMASK(5, 0) +/* USB glue registers */ +#define USB_CTRL0 0x00 +#define USB_CTRL1 0x04 + +#define USB_CTRL0_PORTPWR_EN BIT(12) /* 1 - PPC enabled (default) */ +#define USB_CTRL0_USB3_FIXED BIT(22) /* 1 - USB3 permanent attached */ +#define USB_CTRL0_USB2_FIXED BIT(23) /* 1 - USB2 permanent attached */ + +#define USB_CTRL1_OC_POLARITY BIT(16) /* 0 - HIGH / 1 - LOW */ +#define USB_CTRL1_PWR_POLARITY BIT(17) /* 0 - HIGH / 1 - LOW */ + struct dwc3_imx8mp { struct device *dev; struct platform_device *dwc3; void __iomem *hsio_blk_base; + void __iomem *glue_base; struct clk *hsio_clk; struct clk *suspend_clk; int irq; @@ -47,6 +59,42 @@ struct dwc3_imx8mp { bool wakeup_pending; }; +static void imx8mp_configure_glue(struct dwc3_imx8mp *dwc3_imx) +{ + struct device *dev = dwc3_imx->dev; + u32 value; + + if (!dwc3_imx->glue_base) + return; + + value = readl(dwc3_imx->glue_base + USB_CTRL0); + + if (device_property_read_bool(dev, "fsl,permanently-attached")) + value |= (USB_CTRL0_USB2_FIXED | USB_CTRL0_USB3_FIXED); + else + value &= ~(USB_CTRL0_USB2_FIXED | USB_CTRL0_USB3_FIXED); + + if (device_property_read_bool(dev, "fsl,disable-port-power-control")) + value &= ~(USB_CTRL0_PORTPWR_EN); + else + value |= USB_CTRL0_PORTPWR_EN; + + writel(value, dwc3_imx->glue_base + USB_CTRL0); + + value = readl(dwc3_imx->glue_base + USB_CTRL1); + if (device_property_read_bool(dev, "fsl,over-current-active-low")) + value |= USB_CTRL1_OC_POLARITY; + else + value &= ~USB_CTRL1_OC_POLARITY; + + if (device_property_read_bool(dev, "fsl,power-active-low")) + value |= USB_CTRL1_PWR_POLARITY; + else + value &= ~USB_CTRL1_PWR_POLARITY; + + writel(value, dwc3_imx->glue_base + USB_CTRL1); +} + static void dwc3_imx8mp_wakeup_enable(struct dwc3_imx8mp *dwc3_imx) { struct dwc3 *dwc3 = platform_get_drvdata(dwc3_imx->dwc3); @@ -100,6 +148,7 @@ static int dwc3_imx8mp_probe(struct platform_device *pdev) struct device *dev = &pdev->dev; struct device_node *dwc3_np, *node = dev->of_node; struct dwc3_imx8mp *dwc3_imx; + struct resource *res; int err, irq; if (!node) { @@ -119,6 +168,15 @@ static int dwc3_imx8mp_probe(struct platform_device *pdev) if (IS_ERR(dwc3_imx->hsio_blk_base)) return PTR_ERR(dwc3_imx->hsio_blk_base); + res = platform_get_resource(pdev, IORESOURCE_MEM, 1); + if (!res) { + dev_warn(dev, "Base address for glue layer missing. Continuing without, some features are missing though."); + } else { + dwc3_imx->glue_base = devm_ioremap_resource(dev, res); + if (IS_ERR(dwc3_imx->glue_base)) + return PTR_ERR(dwc3_imx->glue_base); + } + dwc3_imx->hsio_clk = devm_clk_get(dev, "hsio"); if (IS_ERR(dwc3_imx->hsio_clk)) { err = PTR_ERR(dwc3_imx->hsio_clk); @@ -152,6 +210,8 @@ static int dwc3_imx8mp_probe(struct platform_device *pdev) } dwc3_imx->irq = irq; + imx8mp_configure_glue(dwc3_imx); + pm_runtime_set_active(dev); pm_runtime_enable(dev); err = pm_runtime_get_sync(dev); @@ -252,6 +312,8 @@ static int __maybe_unused dwc3_imx8mp_resume(struct dwc3_imx8mp *dwc3_imx, dwc3_imx8mp_wakeup_disable(dwc3_imx); dwc3_imx->pm_suspended = false; + imx8mp_configure_glue(dwc3_imx); + if (dwc3_imx->wakeup_pending) { dwc3_imx->wakeup_pending = false; if (dwc->current_dr_role == DWC3_GCTL_PRTCAP_DEVICE) {
The i.MX8MP glue layer has support for the following flags: * over-current polarity * PWR pad polarity * controlling PPC flag in HCCPARAMS register * permanent port attach for usb2 & usb3 port Allow setting these flags by supporting specific flags in the glue node. In order to get this to work an additional IORESOURCE_MEM and clock is necessary. For backward compatibility this is purely optional. Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com> --- drivers/usb/dwc3/dwc3-imx8mp.c | 62 ++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+)