diff mbox series

[07/24] imx: update is_imx6ull to include i.MX6ULZ

Message ID 20200422135235.14756-7-peng.fan@nxp.com
State Accepted
Commit 9b4e197d4b836efd3ed753c541d325b147db4c03
Headers show
Series [01/24] imx: fix cpu_type helper | expand

Commit Message

Peng Fan April 22, 2020, 1:52 p.m. UTC
Update is_imx6ull helper to include i.MX6ULZ SoC. i.MX6ULZ could
share same macro, then we no need to add is_imx6ulz in various drivers.

Signed-off-by: Peng Fan <peng.fan at nxp.com>
---
 arch/arm/include/asm/mach-imx/sys_proto.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Michael Heimpold April 22, 2020, 8 p.m. UTC | #1
Am Mittwoch, 22. April 2020, 15:52:18 CEST schrieb Peng Fan:
> Update is_imx6ull helper to include i.MX6ULZ SoC. i.MX6ULZ could
> share same macro, then we no need to add is_imx6ulz in various drivers.
> 
> Signed-off-by: Peng Fan <peng.fan at nxp.com>
> ---
>  arch/arm/include/asm/mach-imx/sys_proto.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/include/asm/mach-imx/sys_proto.h b/arch/arm/include/asm/mach-imx/sys_proto.h
> index a02cd40c7d..2a997f280d 100644
> --- a/arch/arm/include/asm/mach-imx/sys_proto.h
> +++ b/arch/arm/include/asm/mach-imx/sys_proto.h
> @@ -37,7 +37,7 @@
>  #define is_mx6sl() (is_cpu_type(MXC_CPU_MX6SL))
>  #define is_mx6solo() (is_cpu_type(MXC_CPU_MX6SOLO))
>  #define is_mx6ul() (is_cpu_type(MXC_CPU_MX6UL))
> -#define is_mx6ull() (is_cpu_type(MXC_CPU_MX6ULL))
> +#define is_mx6ull() (is_cpu_type(MXC_CPU_MX6ULL) || is_cpu_type(MXC_CPU_MX6ULZ))

While I probably understand your intentions, I fear that it will lead to confusion 
when the helper's name does not reflect that more than
one cpu type can match.
What about introducing is_mx6ulX() to signal that the last letter is "don't care"?

Best regards,
Michael

>  #define is_mx6ulz() (is_cpu_type(MXC_CPU_MX6ULZ))
>  #define is_mx6sll() (is_cpu_type(MXC_CPU_MX6SLL))
>  
>
Peng Fan April 23, 2020, 1:33 a.m. UTC | #2
> Subject: Re: [PATCH 07/24] imx: update is_imx6ull to include i.MX6ULZ
> 
> Am Mittwoch, 22. April 2020, 15:52:18 CEST schrieb Peng Fan:
> > Update is_imx6ull helper to include i.MX6ULZ SoC. i.MX6ULZ could share
> > same macro, then we no need to add is_imx6ulz in various drivers.
> >
> > Signed-off-by: Peng Fan <peng.fan at nxp.com>
> > ---
> >  arch/arm/include/asm/mach-imx/sys_proto.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/arm/include/asm/mach-imx/sys_proto.h
> > b/arch/arm/include/asm/mach-imx/sys_proto.h
> > index a02cd40c7d..2a997f280d 100644
> > --- a/arch/arm/include/asm/mach-imx/sys_proto.h
> > +++ b/arch/arm/include/asm/mach-imx/sys_proto.h
> > @@ -37,7 +37,7 @@
> >  #define is_mx6sl() (is_cpu_type(MXC_CPU_MX6SL))  #define
> is_mx6solo()
> > (is_cpu_type(MXC_CPU_MX6SOLO))  #define is_mx6ul()
> > (is_cpu_type(MXC_CPU_MX6UL)) -#define is_mx6ull()
> > (is_cpu_type(MXC_CPU_MX6ULL))
> > +#define is_mx6ull() (is_cpu_type(MXC_CPU_MX6ULL) ||
> > +is_cpu_type(MXC_CPU_MX6ULZ))
> 
> While I probably understand your intentions, I fear that it will lead to
> confusion when the helper's name does not reflect that more than one cpu
> type can match.
> What about introducing is_mx6ulX() to signal that the last letter is "don't
> care"?

Renaming the macro needs to modify drivers using this macro, this is risk
to easy break existing code.
I prefer to keep as is.

Thanks,
Peng.

> 
> Best regards,
> Michael
> 
> >  #define is_mx6ulz() (is_cpu_type(MXC_CPU_MX6ULZ))  #define
> > is_mx6sll() (is_cpu_type(MXC_CPU_MX6SLL))
> >
> >
> 
> 
>
Michael Heimpold April 24, 2020, 8:14 p.m. UTC | #3
Hi,

Am Donnerstag, 23. April 2020, 03:33:49 CEST schrieb Peng Fan:
> > Subject: Re: [PATCH 07/24] imx: update is_imx6ull to include i.MX6ULZ
> > 
> > Am Mittwoch, 22. April 2020, 15:52:18 CEST schrieb Peng Fan:
> > > Update is_imx6ull helper to include i.MX6ULZ SoC. i.MX6ULZ could share
> > > same macro, then we no need to add is_imx6ulz in various drivers.
> > >
> > > Signed-off-by: Peng Fan <peng.fan at nxp.com>
> > > ---
> > >  arch/arm/include/asm/mach-imx/sys_proto.h | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/arch/arm/include/asm/mach-imx/sys_proto.h
> > > b/arch/arm/include/asm/mach-imx/sys_proto.h
> > > index a02cd40c7d..2a997f280d 100644
> > > --- a/arch/arm/include/asm/mach-imx/sys_proto.h
> > > +++ b/arch/arm/include/asm/mach-imx/sys_proto.h
> > > @@ -37,7 +37,7 @@
> > >  #define is_mx6sl() (is_cpu_type(MXC_CPU_MX6SL))  #define
> > is_mx6solo()
> > > (is_cpu_type(MXC_CPU_MX6SOLO))  #define is_mx6ul()
> > > (is_cpu_type(MXC_CPU_MX6UL)) -#define is_mx6ull()
> > > (is_cpu_type(MXC_CPU_MX6ULL))
> > > +#define is_mx6ull() (is_cpu_type(MXC_CPU_MX6ULL) ||
> > > +is_cpu_type(MXC_CPU_MX6ULZ))
> > 
> > While I probably understand your intentions, I fear that it will lead to
> > confusion when the helper's name does not reflect that more than one cpu
> > type can match.
> > What about introducing is_mx6ulX() to signal that the last letter is "don't
> > care"?
> 
> Renaming the macro needs to modify drivers using this macro, this is risk
> to easy break existing code.
> I prefer to keep as is.

but when you look at the drivers using this is_mx6ull() macro, then you'll
find already a bunch of other is_mx...() macros used in addition. Then it
would also be possible to add the is_mx6ulz() one - it won't make the
situation worse at these points.
In my eyes, this is better than hiding two CPUs behind one macro.

Thanks,
Michael

> 
> Thanks,
> Peng.
> 
> > 
> > Best regards,
> > Michael
> > 
> > >  #define is_mx6ulz() (is_cpu_type(MXC_CPU_MX6ULZ))  #define
> > > is_mx6sll() (is_cpu_type(MXC_CPU_MX6SLL))
> > >
> > >
> > 
> > 
> > 
>
Peng Fan April 27, 2020, 1:25 a.m. UTC | #4
> Subject: Re: [PATCH 07/24] imx: update is_imx6ull to include i.MX6ULZ
> 
> Hi,
> 
> Am Donnerstag, 23. April 2020, 03:33:49 CEST schrieb Peng Fan:
> > > Subject: Re: [PATCH 07/24] imx: update is_imx6ull to include
> > > i.MX6ULZ
> > >
> > > Am Mittwoch, 22. April 2020, 15:52:18 CEST schrieb Peng Fan:
> > > > Update is_imx6ull helper to include i.MX6ULZ SoC. i.MX6ULZ could
> > > > share same macro, then we no need to add is_imx6ulz in various drivers.
> > > >
> > > > Signed-off-by: Peng Fan <peng.fan at nxp.com>
> > > > ---
> > > >  arch/arm/include/asm/mach-imx/sys_proto.h | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/arch/arm/include/asm/mach-imx/sys_proto.h
> > > > b/arch/arm/include/asm/mach-imx/sys_proto.h
> > > > index a02cd40c7d..2a997f280d 100644
> > > > --- a/arch/arm/include/asm/mach-imx/sys_proto.h
> > > > +++ b/arch/arm/include/asm/mach-imx/sys_proto.h
> > > > @@ -37,7 +37,7 @@
> > > >  #define is_mx6sl() (is_cpu_type(MXC_CPU_MX6SL))  #define
> > > is_mx6solo()
> > > > (is_cpu_type(MXC_CPU_MX6SOLO))  #define is_mx6ul()
> > > > (is_cpu_type(MXC_CPU_MX6UL)) -#define is_mx6ull()
> > > > (is_cpu_type(MXC_CPU_MX6ULL))
> > > > +#define is_mx6ull() (is_cpu_type(MXC_CPU_MX6ULL) ||
> > > > +is_cpu_type(MXC_CPU_MX6ULZ))
> > >
> > > While I probably understand your intentions, I fear that it will
> > > lead to confusion when the helper's name does not reflect that more
> > > than one cpu type can match.
> > > What about introducing is_mx6ulX() to signal that the last letter is
> > > "don't care"?
> >
> > Renaming the macro needs to modify drivers using this macro, this is
> > risk to easy break existing code.
> > I prefer to keep as is.
> 
> but when you look at the drivers using this is_mx6ull() macro, then you'll find
> already a bunch of other is_mx...() macros used in addition. Then it would
> also be possible to add the is_mx6ulz() one - it won't make the situation
> worse at these points.
> In my eyes, this is better than hiding two CPUs behind one macro.

That's fine. I'll use ulx in v2.

Thanks,
Peng.

> 
> Thanks,
> Michael
> 
> >
> > Thanks,
> > Peng.
> >
> > >
> > > Best regards,
> > > Michael
> > >
> > > >  #define is_mx6ulz() (is_cpu_type(MXC_CPU_MX6ULZ))  #define
> > > > is_mx6sll() (is_cpu_type(MXC_CPU_MX6SLL))
> > > >
> > > >
> > >
> > >
> > >
> >
> 
> 
>
Peng Fan April 27, 2020, 7:57 a.m. UTC | #5
> Subject: RE: [PATCH 07/24] imx: update is_imx6ull to include i.MX6ULZ
> 
> > Subject: Re: [PATCH 07/24] imx: update is_imx6ull to include i.MX6ULZ
> >
> > Hi,
> >
> > Am Donnerstag, 23. April 2020, 03:33:49 CEST schrieb Peng Fan:
> > > > Subject: Re: [PATCH 07/24] imx: update is_imx6ull to include
> > > > i.MX6ULZ
> > > >
> > > > Am Mittwoch, 22. April 2020, 15:52:18 CEST schrieb Peng Fan:
> > > > > Update is_imx6ull helper to include i.MX6ULZ SoC. i.MX6ULZ could
> > > > > share same macro, then we no need to add is_imx6ulz in various
> drivers.
> > > > >
> > > > > Signed-off-by: Peng Fan <peng.fan at nxp.com>
> > > > > ---
> > > > >  arch/arm/include/asm/mach-imx/sys_proto.h | 2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/arch/arm/include/asm/mach-imx/sys_proto.h
> > > > > b/arch/arm/include/asm/mach-imx/sys_proto.h
> > > > > index a02cd40c7d..2a997f280d 100644
> > > > > --- a/arch/arm/include/asm/mach-imx/sys_proto.h
> > > > > +++ b/arch/arm/include/asm/mach-imx/sys_proto.h
> > > > > @@ -37,7 +37,7 @@
> > > > >  #define is_mx6sl() (is_cpu_type(MXC_CPU_MX6SL))  #define
> > > > is_mx6solo()
> > > > > (is_cpu_type(MXC_CPU_MX6SOLO))  #define is_mx6ul()
> > > > > (is_cpu_type(MXC_CPU_MX6UL)) -#define is_mx6ull()
> > > > > (is_cpu_type(MXC_CPU_MX6ULL))
> > > > > +#define is_mx6ull() (is_cpu_type(MXC_CPU_MX6ULL) ||
> > > > > +is_cpu_type(MXC_CPU_MX6ULZ))
> > > >
> > > > While I probably understand your intentions, I fear that it will
> > > > lead to confusion when the helper's name does not reflect that
> > > > more than one cpu type can match.
> > > > What about introducing is_mx6ulX() to signal that the last letter
> > > > is "don't care"?
> > >
> > > Renaming the macro needs to modify drivers using this macro, this is
> > > risk to easy break existing code.
> > > I prefer to keep as is.
> >
> > but when you look at the drivers using this is_mx6ull() macro, then
> > you'll find already a bunch of other is_mx...() macros used in
> > addition. Then it would also be possible to add the is_mx6ulz() one -
> > it won't make the situation worse at these points.
> > In my eyes, this is better than hiding two CPUs behind one macro.
> 
> That's fine. I'll use ulx in v2.

After a check, there is lots places to change. I would keep v1.

There will no more ull variants. ull is superset of ulz,
only when need to handle ulz specific things, need to use
is_mx6ulz.

Thanks,
Peng.

> 
> Thanks,
> Peng.
> 
> >
> > Thanks,
> > Michael
> >
> > >
> > > Thanks,
> > > Peng.
> > >
> > > >
> > > > Best regards,
> > > > Michael
> > > >
> > > > >  #define is_mx6ulz() (is_cpu_type(MXC_CPU_MX6ULZ))  #define
> > > > > is_mx6sll() (is_cpu_type(MXC_CPU_MX6SLL))
> > > > >
> > > > >
> > > >
> > > >
> > > >
> > >
> >
> >
> >
diff mbox series

Patch

diff --git a/arch/arm/include/asm/mach-imx/sys_proto.h b/arch/arm/include/asm/mach-imx/sys_proto.h
index a02cd40c7d..2a997f280d 100644
--- a/arch/arm/include/asm/mach-imx/sys_proto.h
+++ b/arch/arm/include/asm/mach-imx/sys_proto.h
@@ -37,7 +37,7 @@ 
 #define is_mx6sl() (is_cpu_type(MXC_CPU_MX6SL))
 #define is_mx6solo() (is_cpu_type(MXC_CPU_MX6SOLO))
 #define is_mx6ul() (is_cpu_type(MXC_CPU_MX6UL))
-#define is_mx6ull() (is_cpu_type(MXC_CPU_MX6ULL))
+#define is_mx6ull() (is_cpu_type(MXC_CPU_MX6ULL) || is_cpu_type(MXC_CPU_MX6ULZ))
 #define is_mx6ulz() (is_cpu_type(MXC_CPU_MX6ULZ))
 #define is_mx6sll() (is_cpu_type(MXC_CPU_MX6SLL))