| Message ID | 20200422135235.14756-7-peng.fan@nxp.com |
|---|---|
| State | Accepted |
| Commit | 9b4e197d4b836efd3ed753c541d325b147db4c03 |
| Delegated to: | Stefano Babic |
| Headers | show |
| Series | [01/24] imx: fix cpu_type helper | expand |
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@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)) > >
> 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@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)) > > > > > > >
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@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)) > > > > > > > > > > > > >
> 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@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)) > > > > > > > > > > > > > > > > > > > > > >
> 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@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 --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))
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@nxp.com> --- arch/arm/include/asm/mach-imx/sys_proto.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)