| Message ID | 1382680943-9258-5-git-send-email-peter.chen@freescale.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Oct 25, 2013 at 04:23:32PM +0800, Shawn Guo wrote: > On Fri, Oct 25, 2013 at 02:02:23PM +0800, Peter Chen wrote: > > Due to imx28 usb has special write request, it is not compatible > > with other imx27 sytle usb controllers. > > > > Signed-off-by: Peter Chen <peter.chen@freescale.com> > > --- > > arch/arm/boot/dts/imx28.dtsi | 4 ++-- > > 1 files changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi > > index 7363fde..6cd2607 100644 > > --- a/arch/arm/boot/dts/imx28.dtsi > > +++ b/arch/arm/boot/dts/imx28.dtsi > > @@ -1041,7 +1041,7 @@ > > ranges; > > > > usb0: usb@80080000 { > > - compatible = "fsl,imx28-usb", "fsl,imx27-usb"; > > + compatible = "fsl,imx28-usb"; > > You shouldn't need the change as long as you put "fsl,imx28-usb" before > "fsl,imx27-usb" in driver ci_hdrc_imx_dt_ids[] table. > I know it can work without this dts change, but that driver change like a workaround. Since the imx28-usb is not compatible with imx27-usb, we'd better only keep "fsl, imx28-usb" compatible string at imx28 dtsi.
On Fri, Oct 25, 2013 at 02:02:23PM +0800, Peter Chen wrote: > Due to imx28 usb has special write request, it is not compatible > with other imx27 sytle usb controllers. > > Signed-off-by: Peter Chen <peter.chen@freescale.com> > --- > arch/arm/boot/dts/imx28.dtsi | 4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi > index 7363fde..6cd2607 100644 > --- a/arch/arm/boot/dts/imx28.dtsi > +++ b/arch/arm/boot/dts/imx28.dtsi > @@ -1041,7 +1041,7 @@ > ranges; > > usb0: usb@80080000 { > - compatible = "fsl,imx28-usb", "fsl,imx27-usb"; > + compatible = "fsl,imx28-usb"; You shouldn't need the change as long as you put "fsl,imx28-usb" before "fsl,imx27-usb" in driver ci_hdrc_imx_dt_ids[] table. Shawn > reg = <0x80080000 0x10000>; > interrupts = <93>; > clocks = <&clks 60>; > @@ -1050,7 +1050,7 @@ > }; > > usb1: usb@80090000 { > - compatible = "fsl,imx28-usb", "fsl,imx27-usb"; > + compatible = "fsl,imx28-usb"; > reg = <0x80090000 0x10000>; > interrupts = <92>; > clocks = <&clks 61>; > -- > 1.7.1 > >
On Fri, Oct 25, 2013 at 04:14:30PM +0800, Peter Chen wrote: > > > @@ -1041,7 +1041,7 @@ > > > ranges; > > > > > > usb0: usb@80080000 { > > > - compatible = "fsl,imx28-usb", "fsl,imx27-usb"; > > > + compatible = "fsl,imx28-usb"; > > > > You shouldn't need the change as long as you put "fsl,imx28-usb" before > > "fsl,imx27-usb" in driver ci_hdrc_imx_dt_ids[] table. > > > > I know it can work without this dts change, but that driver change like > a workaround. No, it's not a workaround. Before of_match_device() gets improved to match the best one, it should just work that way. > Since the imx28-usb is not compatible with imx27-usb, > we'd better only keep "fsl, imx28-usb" compatible string at imx28 dtsi. From hardware point of view, the USB block on imx28 *is* inherited from imx27, and compatible to imx27 USB block, even though we have a little difference to handle in software. The compatible string is written in this way at the first place exactly for the reason we can save such DTS change when we have some little incompatibility to handle. Shawn
On Fri, Oct 25, 2013 at 04:46:09PM +0800, Shawn Guo wrote: > On Fri, Oct 25, 2013 at 04:14:30PM +0800, Peter Chen wrote: > > > > @@ -1041,7 +1041,7 @@ > > > > ranges; > > > > > > > > usb0: usb@80080000 { > > > > - compatible = "fsl,imx28-usb", "fsl,imx27-usb"; > > > > + compatible = "fsl,imx28-usb"; > > > > > > You shouldn't need the change as long as you put "fsl,imx28-usb" before > > > "fsl,imx27-usb" in driver ci_hdrc_imx_dt_ids[] table. > > > > > > > I know it can work without this dts change, but that driver change like > > a workaround. > > No, it's not a workaround. Before of_match_device() gets improved to > match the best one, it should just work that way. > hmm, before of_match_device gets improved or it is well documented, how user knows to organize device_id table. > > Since the imx28-usb is not compatible with imx27-usb, > > we'd better only keep "fsl, imx28-usb" compatible string at imx28 dtsi. > > From hardware point of view, the USB block on imx28 *is* inherited from > imx27, and compatible to imx27 USB block, even though we have a little > difference to handle in software. The compatible string is written in > this way at the first place exactly for the reason we can save such > DTS change when we have some little incompatibility to handle. > OK, I agree, then the of_match_device really should be improved to get above purpose. BTW, any reasons why esdhc has different compatible string for every SoCs?
On Mon, Oct 28, 2013 at 09:59:27AM +0800, Peter Chen wrote: > hmm, before of_match_device gets improved or it is well documented, > how user knows to organize device_id table. Just like you, you found it after you saw that of_match_device() does not return you the expected device :) > > > > Since the imx28-usb is not compatible with imx27-usb, > > > we'd better only keep "fsl, imx28-usb" compatible string at imx28 dtsi. > > > > From hardware point of view, the USB block on imx28 *is* inherited from > > imx27, and compatible to imx27 USB block, even though we have a little > > difference to handle in software. The compatible string is written in > > this way at the first place exactly for the reason we can save such > > DTS change when we have some little incompatibility to handle. > > > > OK, I agree, then the of_match_device really should be improved to get > above purpose. Yes, there is already patch for that but it just hasn't found its way to mainline because it causes some issue on SPARC. > BTW, any reasons why esdhc has different compatible string > for every SoCs? Mostly because the hardware design sucks. There so many register differences and bugs to deal with between difference SoCs. Shawn
diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi index 7363fde..6cd2607 100644 --- a/arch/arm/boot/dts/imx28.dtsi +++ b/arch/arm/boot/dts/imx28.dtsi @@ -1041,7 +1041,7 @@ ranges; usb0: usb@80080000 { - compatible = "fsl,imx28-usb", "fsl,imx27-usb"; + compatible = "fsl,imx28-usb"; reg = <0x80080000 0x10000>; interrupts = <93>; clocks = <&clks 60>; @@ -1050,7 +1050,7 @@ }; usb1: usb@80090000 { - compatible = "fsl,imx28-usb", "fsl,imx27-usb"; + compatible = "fsl,imx28-usb"; reg = <0x80090000 0x10000>; interrupts = <92>; clocks = <&clks 61>;
Due to imx28 usb has special write request, it is not compatible with other imx27 sytle usb controllers. Signed-off-by: Peter Chen <peter.chen@freescale.com> --- arch/arm/boot/dts/imx28.dtsi | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)