| Message ID | 1405921091-28748-1-git-send-email-shawn.guo@freescale.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Jul 21, 2014 at 01:38:11PM +0800, Shawn Guo wrote: > From: Jason Liu <r64343@freescale.com> > > Add more revision support for the new i.MX6DQ tap-out (TO1.5). This > TO1.5 is the Rev 1.3 as documented in i.MX6DQ data sheet, because TO1.3 > and TO1.4 are never revealed. So the chip identifies itself as 1.5 but the data sheet refers to it as 1.3. Is there a way to make that clear somewhere? Otherwise it's really confusing. Sascha > > Signed-off-by: Jason Liu <r64343@freescale.com> > Signed-off-by: Shawn Guo <shawn.guo@freescale.com> > --- > arch/arm/mach-imx/anatop.c | 9 +++++++++ > arch/arm/mach-imx/mxc.h | 2 ++ > 2 files changed, 11 insertions(+) > > diff --git a/arch/arm/mach-imx/anatop.c b/arch/arm/mach-imx/anatop.c > index 4a40bbb46183..459ba290744b 100644 > --- a/arch/arm/mach-imx/anatop.c > +++ b/arch/arm/mach-imx/anatop.c > @@ -104,6 +104,15 @@ void __init imx_init_revision_from_anatop(void) > case 2: > revision = IMX_CHIP_REVISION_1_2; > break; > + case 3: > + revision = IMX_CHIP_REVISION_1_3; > + break; > + case 4: > + revision = IMX_CHIP_REVISION_1_4; > + break; > + case 5: > + revision = IMX_CHIP_REVISION_1_5; > + break; > default: > revision = IMX_CHIP_REVISION_UNKNOWN; > } > diff --git a/arch/arm/mach-imx/mxc.h b/arch/arm/mach-imx/mxc.h > index a39b69ef4301..17a41ca65acf 100644 > --- a/arch/arm/mach-imx/mxc.h > +++ b/arch/arm/mach-imx/mxc.h > @@ -43,6 +43,8 @@ > #define IMX_CHIP_REVISION_1_1 0x11 > #define IMX_CHIP_REVISION_1_2 0x12 > #define IMX_CHIP_REVISION_1_3 0x13 > +#define IMX_CHIP_REVISION_1_4 0x14 > +#define IMX_CHIP_REVISION_1_5 0x15 > #define IMX_CHIP_REVISION_2_0 0x20 > #define IMX_CHIP_REVISION_2_1 0x21 > #define IMX_CHIP_REVISION_2_2 0x22 > -- > 1.9.1 > >
On Mon, Jul 21, 2014 at 08:35:20AM +0200, Sascha Hauer wrote: > On Mon, Jul 21, 2014 at 01:38:11PM +0800, Shawn Guo wrote: > > From: Jason Liu <r64343@freescale.com> > > > > Add more revision support for the new i.MX6DQ tap-out (TO1.5). This Is it "tap-out" or "tape-out"? A quick google request suggests the latter. > > TO1.5 is the Rev 1.3 as documented in i.MX6DQ data sheet, because TO1.3 > > and TO1.4 are never revealed. > > So the chip identifies itself as 1.5 but the data sheet refers to it as > 1.3. Is there a way to make that clear somewhere? Otherwise it's really > confusing. Yeah, from the commit log I would have expected the following patch: + case 5: + revision = IMX_CHIP_REVISION_1_3; + break; maybe with a code comment. Also, the commit log only mentions i.MX6DQ; I wonder about the other imx6 variants. I hope they use the same logic? Best regards Uwe > > diff --git a/arch/arm/mach-imx/anatop.c b/arch/arm/mach-imx/anatop.c > > index 4a40bbb46183..459ba290744b 100644 > > --- a/arch/arm/mach-imx/anatop.c > > +++ b/arch/arm/mach-imx/anatop.c > > @@ -104,6 +104,15 @@ void __init imx_init_revision_from_anatop(void) > > case 2: > > revision = IMX_CHIP_REVISION_1_2; > > break; > > + case 3: > > + revision = IMX_CHIP_REVISION_1_3; > > + break; > > + case 4: > > + revision = IMX_CHIP_REVISION_1_4; > > + break; > > + case 5: > > + revision = IMX_CHIP_REVISION_1_5; > > + break; > > default: > > revision = IMX_CHIP_REVISION_UNKNOWN; > > }
On Mon, Jul 21, 2014 at 08:41:45AM +0200, Uwe Kleine-König wrote: > On Mon, Jul 21, 2014 at 08:35:20AM +0200, Sascha Hauer wrote: > > On Mon, Jul 21, 2014 at 01:38:11PM +0800, Shawn Guo wrote: > > > From: Jason Liu <r64343@freescale.com> > > > > > > Add more revision support for the new i.MX6DQ tap-out (TO1.5). This > Is it "tap-out" or "tape-out"? A quick google request suggests the > latter. Yes, you're right. > > > > TO1.5 is the Rev 1.3 as documented in i.MX6DQ data sheet, because TO1.3 > > > and TO1.4 are never revealed. > > > > So the chip identifies itself as 1.5 but the data sheet refers to it as > > 1.3. Is there a way to make that clear somewhere? Otherwise it's really > > confusing. > Yeah, from the commit log I would have expected the following patch: > > + case 5: > + revision = IMX_CHIP_REVISION_1_3; > + break; > > maybe with a code comment. I did consider such fix-up when I was trying to submit Jason's patch from FSL internal tree. But this TO1.5 numbering has been already used in a couple of FSL BSP releases. We do not really want to maintain two different numbering scheme between FSL BSP and upstream kernel. > Also, the commit log only mentions i.MX6DQ; I > wonder about the other imx6 variants. I hope they use the same logic? This is another reason for that above fix-up may not be good. This revision mismatch between register and document is only with i.MX6DQ. So far, only i.MX6DQ revision goes beyond 1.2, but I'm sure such mismatch/mistake will not be with other imx6 variants. Shawn
On Mon, Jul 21, 2014 at 08:35:20AM +0200, Sascha Hauer wrote: > On Mon, Jul 21, 2014 at 01:38:11PM +0800, Shawn Guo wrote: > > From: Jason Liu <r64343@freescale.com> > > > > Add more revision support for the new i.MX6DQ tap-out (TO1.5). This > > TO1.5 is the Rev 1.3 as documented in i.MX6DQ data sheet, because TO1.3 > > and TO1.4 are never revealed. > > So the chip identifies itself as 1.5 but the data sheet refers to it as > 1.3. Is there a way to make that clear somewhere? Otherwise it's really > confusing. I can add a note about that in the code, but I'm not really sure if there is a better way to pass such info to end users. Suggestions are welcomed. Shawn
On Mon, Jul 21, 2014 at 03:06:48PM +0800, Shawn Guo wrote: > On Mon, Jul 21, 2014 at 08:35:20AM +0200, Sascha Hauer wrote: > > On Mon, Jul 21, 2014 at 01:38:11PM +0800, Shawn Guo wrote: > > > From: Jason Liu <r64343@freescale.com> > > > > > > Add more revision support for the new i.MX6DQ tap-out (TO1.5). This > > > TO1.5 is the Rev 1.3 as documented in i.MX6DQ data sheet, because TO1.3 > > > and TO1.4 are never revealed. > > > > So the chip identifies itself as 1.5 but the data sheet refers to it as > > 1.3. Is there a way to make that clear somewhere? Otherwise it's really > > confusing. > > I can add a note about that in the code, but I'm not really sure if > there is a better way to pass such info to end users. Suggestions are > welcomed. Since the anatop code can be used by other SoCs aswell and you're saying that even the other i.MX6 SoC variants may have a different numbering I think adding a comment to the code is the best we can do. The only other possibility I can think of is to make the info string we print during boot SoC specific again, but I think we are all glad that this isn't the case anymore. So adding a comment should be fine. At least the code would be the place I would look if I remembered that there is some inconsistency and don't know exactly what it was. Sascha
diff --git a/arch/arm/mach-imx/anatop.c b/arch/arm/mach-imx/anatop.c index 4a40bbb46183..459ba290744b 100644 --- a/arch/arm/mach-imx/anatop.c +++ b/arch/arm/mach-imx/anatop.c @@ -104,6 +104,15 @@ void __init imx_init_revision_from_anatop(void) case 2: revision = IMX_CHIP_REVISION_1_2; break; + case 3: + revision = IMX_CHIP_REVISION_1_3; + break; + case 4: + revision = IMX_CHIP_REVISION_1_4; + break; + case 5: + revision = IMX_CHIP_REVISION_1_5; + break; default: revision = IMX_CHIP_REVISION_UNKNOWN; } diff --git a/arch/arm/mach-imx/mxc.h b/arch/arm/mach-imx/mxc.h index a39b69ef4301..17a41ca65acf 100644 --- a/arch/arm/mach-imx/mxc.h +++ b/arch/arm/mach-imx/mxc.h @@ -43,6 +43,8 @@ #define IMX_CHIP_REVISION_1_1 0x11 #define IMX_CHIP_REVISION_1_2 0x12 #define IMX_CHIP_REVISION_1_3 0x13 +#define IMX_CHIP_REVISION_1_4 0x14 +#define IMX_CHIP_REVISION_1_5 0x15 #define IMX_CHIP_REVISION_2_0 0x20 #define IMX_CHIP_REVISION_2_1 0x21 #define IMX_CHIP_REVISION_2_2 0x22