Message ID | 20220126141340.234125-1-alexander.stein@ew.tq-group.com |
---|---|
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 1/4] usb: dwc3: imx8mp: rename iomem base pointer > > Until now the iomem used is not USB glue as the name suggests, but HSIO BLK_CTL. > Rename the struct member accordingly. This is a preparing patch for when > USB glue is actually used. > > Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com> Reviewed-by: Li Jun <jun.li@nxp.com> > --- > drivers/usb/dwc3/dwc3-imx8mp.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/drivers/usb/dwc3/dwc3-imx8mp.c > b/drivers/usb/dwc3/dwc3-imx8mp.c index d328d20abfbc..1c8fe657b3a9 100644 > --- a/drivers/usb/dwc3/dwc3-imx8mp.c > +++ b/drivers/usb/dwc3/dwc3-imx8mp.c > @@ -39,7 +39,7 @@ > struct dwc3_imx8mp { > struct device *dev; > struct platform_device *dwc3; > - void __iomem *glue_base; > + void __iomem *hsio_blk_base; > struct clk *hsio_clk; > struct clk *suspend_clk; > int irq; > @@ -55,7 +55,7 @@ static void dwc3_imx8mp_wakeup_enable(struct dwc3_imx8mp > *dwc3_imx) > if (!dwc3) > return; > > - val = readl(dwc3_imx->glue_base + USB_WAKEUP_CTRL); > + val = readl(dwc3_imx->hsio_blk_base + USB_WAKEUP_CTRL); > > if ((dwc3->current_dr_role == DWC3_GCTL_PRTCAP_HOST) && dwc3->xhci) > val |= USB_WAKEUP_EN | USB_WAKEUP_SS_CONN | @@ -64,16 +64,16 @@ static > void dwc3_imx8mp_wakeup_enable(struct dwc3_imx8mp *dwc3_imx) > val |= USB_WAKEUP_EN | USB_WAKEUP_VBUS_EN | > USB_WAKEUP_VBUS_SRC_SESS_VAL; > > - writel(val, dwc3_imx->glue_base + USB_WAKEUP_CTRL); > + writel(val, dwc3_imx->hsio_blk_base + USB_WAKEUP_CTRL); > } > > static void dwc3_imx8mp_wakeup_disable(struct dwc3_imx8mp *dwc3_imx) { > u32 val; > > - val = readl(dwc3_imx->glue_base + USB_WAKEUP_CTRL); > + val = readl(dwc3_imx->hsio_blk_base + USB_WAKEUP_CTRL); > val &= ~(USB_WAKEUP_EN | USB_WAKEUP_EN_MASK); > - writel(val, dwc3_imx->glue_base + USB_WAKEUP_CTRL); > + writel(val, dwc3_imx->hsio_blk_base + USB_WAKEUP_CTRL); > } > > static irqreturn_t dwc3_imx8mp_interrupt(int irq, void *_dwc3_imx) @@ > -115,9 +115,9 @@ static int dwc3_imx8mp_probe(struct platform_device *pdev) > > dwc3_imx->dev = dev; > > - dwc3_imx->glue_base = devm_platform_ioremap_resource(pdev, 0); > - if (IS_ERR(dwc3_imx->glue_base)) > - return PTR_ERR(dwc3_imx->glue_base); > + dwc3_imx->hsio_blk_base = devm_platform_ioremap_resource(pdev, 0); > + if (IS_ERR(dwc3_imx->hsio_blk_base)) > + return PTR_ERR(dwc3_imx->hsio_blk_base); > > dwc3_imx->hsio_clk = devm_clk_get(dev, "hsio"); > if (IS_ERR(dwc3_imx->hsio_clk)) { > -- > 2.25.1
> -----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
> -----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 4/4] arm64: dts: imx8mp: Add memory for USB3 glue layer > to usb3 nodes > > The USB3 glue layer has 2 areas in the register set, see RM Rev.1 section > 11.2.5.2.1 GLUE_usb3 memory map: > * USB3 control/status > * PHY control/status > > Provide the memory area to the usb3 nodes for accessing the features in the > USB3 control area. > > Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com> Reviewed-by: Li Jun <jun.li@nxp.com> > --- > arch/arm64/boot/dts/freescale/imx8mp.dtsi | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi > b/arch/arm64/boot/dts/freescale/imx8mp.dtsi > index e61ac5f136ad..d4aadb434d36 100644 > --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi > +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi > @@ -1030,7 +1030,8 @@ usb3_phy0: usb-phy@381f0040 { > > usb3_0: usb@32f10100 { > compatible = "fsl,imx8mp-dwc3"; > - reg = <0x32f10100 0x8>; > + reg = <0x32f10100 0x8>, > + <0x381f0000 0x20>; > clocks = <&clk IMX8MP_CLK_HSIO_ROOT>, > <&clk IMX8MP_CLK_USB_ROOT>; > clock-names = "hsio", "suspend"; > @@ -1070,7 +1071,8 @@ usb3_phy1: usb-phy@382f0040 { > > usb3_1: usb@32f10108 { > compatible = "fsl,imx8mp-dwc3"; > - reg = <0x32f10108 0x8>; > + reg = <0x32f10108 0x8>, > + <0x382f0000 0x20>; > clocks = <&clk IMX8MP_CLK_HSIO_ROOT>, > <&clk IMX8MP_CLK_USB_ROOT>; > clock-names = "hsio", "suspend"; > -- > 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