Subject: Re: [PATCH] ARM: dts: imx7: fix USB controller 'size' parameter

From: Peter Chen <[email-masked]>
Date: Fri, 30 Aug 2019 09:33:45 +0000

On 19-08-30 07:11:42, Thomas Schaefer wrote:
> -----Ursprüngliche Nachricht-----
> Von: Peter Chen <[email-masked]> 
> Gesendet: Freitag, 30. August 2019 04:26
> > On 19-08-29 17:49:13, Thomas Schaefer wrote:
> > > Currently the size parameter in the reg property of usbotg and usbh 
> > > nodes in imx7s and imx7d dts includes is set to 0x200 which is wrong 
> > > for the i.MX7 CPU. According to reference manual, spacing of USB 
> > > controller registers is 0x10000 instead.
> > > 
> > > This patch will fix this and set the 'size' to 0x10000.
> > > 
> > > Signed-off-by: Thomas Schaefer <[email-masked]>
> > > ---
> > >  arch/arm/boot/dts/imx7d.dtsi | 2 +-
> > >  arch/arm/boot/dts/imx7s.dtsi | 4 ++--
> > >  2 files changed, 3 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/arch/arm/boot/dts/imx7d.dtsi 
> > > b/arch/arm/boot/dts/imx7d.dtsi index 42528d2812a2..f1b098d28b6e 100644
> > > --- a/arch/arm/boot/dts/imx7d.dtsi
> > > +++ b/arch/arm/boot/dts/imx7d.dtsi
> > > @@ -117,7 +117,7 @@
> > >  &aips3 {
> > >  	usbotg2: usb@30b20000 {
> > >  		compatible = "fsl,imx7d-usb", "fsl,imx27-usb";
> > > -		reg = <0x30b20000 0x200>;
> > > +		reg = <0x30b20000 0x10000>;
> > >  		interrupts = <GIC_SPI 42 IRQ_TYPE_LEVEL_HIGH>;
> > >  		clocks = <&clks IMX7D_USB_CTRL_CLK>;
> > >  		fsl,usbphy = <&usbphynop2>;
> > > diff --git a/arch/arm/boot/dts/imx7s.dtsi 
> > > b/arch/arm/boot/dts/imx7s.dtsi index c1a4fff5ceda..9e25fccf33f0 100644
> > > --- a/arch/arm/boot/dts/imx7s.dtsi
> > > +++ b/arch/arm/boot/dts/imx7s.dtsi
> > > @@ -1088,7 +1088,7 @@
> > >  
> > >  			usbotg1: usb@30b10000 {
> > >  				compatible = "fsl,imx7d-usb", "fsl,imx27-usb";
> > > -				reg = <0x30b10000 0x200>;
> > > +				reg = <0x30b10000 0x10000>;
> > >  				interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
> > >  				clocks = <&clks IMX7D_USB_CTRL_CLK>;
> > >  				fsl,usbphy = <&usbphynop1>;
> > > @@ -1099,7 +1099,7 @@
> > >  
> > >  			usbh: usb@30b30000 {
> > >  				compatible = "fsl,imx7d-usb", "fsl,imx27-usb";
> > > -				reg = <0x30b30000 0x200>;
> > > +				reg = <0x30b30000 0x10000>;
> > >  				interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>;
> > >  				clocks = <&clks IMX7D_USB_CTRL_CLK>;
> > >  				fsl,usbphy = <&usbphynop3>;
> > 
> > Hi Thomos,
> > 
> > The core controller range is 0x200, from offset 0x200, it is non-core register, which is used by usbmisc. Fabio said you met problem at u-boot for this size, what's the problem about it?
> > 
> > Thanks,
> > Peter
> 
> Hi Peter,
> 
> When porting one of our i.MX7 based modules to u-boot v2019.07, I found that scanning USB with 'usb start' crashes when trying to scan the _second_ controller enabled in the device tree (the first controller was detected properly). After some investigation I found that the problem was introduced with Marek Vasuts 'usb: ehci-mx6: Fix bus enumeration for DM case' (u-boot commit 501547cec1f7f0438cae388a104ff60f18576c01). This patch uses the 'reg' property in the usbotg and usbh nodes to calculate the device index number for the driver.
> 
> Actually, controller range on i.MX6 is 0x200, thus the calculation is correct for i.MX6, but on i.MX7 the base addresses of the controller registers differ by 0x10000 and calculation will fail if more than one USB controller is enabled in the device tree. This is why I suggested to fix this problem in the imx7s and imx7d device tree include files.
> 

At dts, each USB controller has its reg property to describe its base
address, we do not need additional calculation.

For controller index, it is only needed for imx6 since we share non-core
register from 0x800 among controllers. From imx7, we have dedicate
non-core region for each controller, it can be described at dts well.

At dts, we use usbmisc to describe the non core part, and the related
Linux code is at drivers/usb/chipidea/usbmisc_imx.c

-- 

Thanks,
Peter Chen
_______________________________________________
linux-arm-kernel mailing list
[email-masked]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel