| Message ID | 20220329152926.50958-10-andriy.shevchenko@linux.intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | gpiolib: Two new helpers and way toward fwnode | expand |
On 29/03/2022 17:29, Andy Shevchenko wrote: > Rename REG_* to MREG_* as a prerequisite for enabling COMPILE_TEST. What error do you hit ? MREG_ is rather ugly, something like PINCONF_REG_ or more simpler MESON_REG_ would be more appropriate. Neil > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > drivers/pinctrl/meson/pinctrl-meson.c | 24 ++++++++++++------------ > drivers/pinctrl/meson/pinctrl-meson.h | 24 ++++++++++++------------ > 2 files changed, 24 insertions(+), 24 deletions(-) > > diff --git a/drivers/pinctrl/meson/pinctrl-meson.c b/drivers/pinctrl/meson/pinctrl-meson.c > index 49851444a6e3..64da61ba2bb9 100644 > --- a/drivers/pinctrl/meson/pinctrl-meson.c > +++ b/drivers/pinctrl/meson/pinctrl-meson.c > @@ -218,13 +218,13 @@ static int meson_pinconf_set_output(struct meson_pinctrl *pc, > unsigned int pin, > bool out) > { > - return meson_pinconf_set_gpio_bit(pc, pin, REG_DIR, !out); > + return meson_pinconf_set_gpio_bit(pc, pin, MREG_DIR, !out); > } > > static int meson_pinconf_get_output(struct meson_pinctrl *pc, > unsigned int pin) > { > - int ret = meson_pinconf_get_gpio_bit(pc, pin, REG_DIR); > + int ret = meson_pinconf_get_gpio_bit(pc, pin, MREG_DIR); > > if (ret < 0) > return ret; > @@ -236,13 +236,13 @@ static int meson_pinconf_set_drive(struct meson_pinctrl *pc, > unsigned int pin, > bool high) > { > - return meson_pinconf_set_gpio_bit(pc, pin, REG_OUT, high); > + return meson_pinconf_set_gpio_bit(pc, pin, MREG_OUT, high); > } > > static int meson_pinconf_get_drive(struct meson_pinctrl *pc, > unsigned int pin) > { > - return meson_pinconf_get_gpio_bit(pc, pin, REG_OUT); > + return meson_pinconf_get_gpio_bit(pc, pin, MREG_OUT); > } > > static int meson_pinconf_set_output_drive(struct meson_pinctrl *pc, > @@ -269,7 +269,7 @@ static int meson_pinconf_disable_bias(struct meson_pinctrl *pc, > if (ret) > return ret; > > - meson_calc_reg_and_bit(bank, pin, REG_PULLEN, ®, &bit); > + meson_calc_reg_and_bit(bank, pin, MREG_PULLEN, ®, &bit); > ret = regmap_update_bits(pc->reg_pullen, reg, BIT(bit), 0); > if (ret) > return ret; > @@ -288,7 +288,7 @@ static int meson_pinconf_enable_bias(struct meson_pinctrl *pc, unsigned int pin, > if (ret) > return ret; > > - meson_calc_reg_and_bit(bank, pin, REG_PULL, ®, &bit); > + meson_calc_reg_and_bit(bank, pin, MREG_PULL, ®, &bit); > if (pull_up) > val = BIT(bit); > > @@ -296,7 +296,7 @@ static int meson_pinconf_enable_bias(struct meson_pinctrl *pc, unsigned int pin, > if (ret) > return ret; > > - meson_calc_reg_and_bit(bank, pin, REG_PULLEN, ®, &bit); > + meson_calc_reg_and_bit(bank, pin, MREG_PULLEN, ®, &bit); > ret = regmap_update_bits(pc->reg_pullen, reg, BIT(bit), BIT(bit)); > if (ret) > return ret; > @@ -321,7 +321,7 @@ static int meson_pinconf_set_drive_strength(struct meson_pinctrl *pc, > if (ret) > return ret; > > - meson_calc_reg_and_bit(bank, pin, REG_DS, ®, &bit); > + meson_calc_reg_and_bit(bank, pin, MREG_DS, ®, &bit); > > if (drive_strength_ua <= 500) { > ds_val = MESON_PINCONF_DRV_500UA; > @@ -407,7 +407,7 @@ static int meson_pinconf_get_pull(struct meson_pinctrl *pc, unsigned int pin) > if (ret) > return ret; > > - meson_calc_reg_and_bit(bank, pin, REG_PULLEN, ®, &bit); > + meson_calc_reg_and_bit(bank, pin, MREG_PULLEN, ®, &bit); > > ret = regmap_read(pc->reg_pullen, reg, &val); > if (ret) > @@ -416,7 +416,7 @@ static int meson_pinconf_get_pull(struct meson_pinctrl *pc, unsigned int pin) > if (!(val & BIT(bit))) { > conf = PIN_CONFIG_BIAS_DISABLE; > } else { > - meson_calc_reg_and_bit(bank, pin, REG_PULL, ®, &bit); > + meson_calc_reg_and_bit(bank, pin, MREG_PULL, ®, &bit); > > ret = regmap_read(pc->reg_pull, reg, &val); > if (ret) > @@ -447,7 +447,7 @@ static int meson_pinconf_get_drive_strength(struct meson_pinctrl *pc, > if (ret) > return ret; > > - meson_calc_reg_and_bit(bank, pin, REG_DS, ®, &bit); > + meson_calc_reg_and_bit(bank, pin, MREG_DS, ®, &bit); > > ret = regmap_read(pc->reg_ds, reg, &val); > if (ret) > @@ -595,7 +595,7 @@ static int meson_gpio_get(struct gpio_chip *chip, unsigned gpio) > if (ret) > return ret; > > - meson_calc_reg_and_bit(bank, gpio, REG_IN, ®, &bit); > + meson_calc_reg_and_bit(bank, gpio, MREG_IN, ®, &bit); > regmap_read(pc->reg_gpio, reg, &val); > > return !!(val & BIT(bit)); > diff --git a/drivers/pinctrl/meson/pinctrl-meson.h b/drivers/pinctrl/meson/pinctrl-meson.h > index ff5372e0a475..c00d9ad27843 100644 > --- a/drivers/pinctrl/meson/pinctrl-meson.h > +++ b/drivers/pinctrl/meson/pinctrl-meson.h > @@ -63,12 +63,12 @@ struct meson_reg_desc { > * enum meson_reg_type - type of registers encoded in @meson_reg_desc > */ > enum meson_reg_type { > - REG_PULLEN, > - REG_PULL, > - REG_DIR, > - REG_OUT, > - REG_IN, > - REG_DS, > + MREG_PULLEN, > + MREG_PULL, > + MREG_DIR, > + MREG_OUT, > + MREG_IN, > + MREG_DS, > NUM_REG, > }; > > @@ -150,12 +150,12 @@ struct meson_pinctrl { > .irq_first = fi, \ > .irq_last = li, \ > .regs = { \ > - [REG_PULLEN] = { per, peb }, \ > - [REG_PULL] = { pr, pb }, \ > - [REG_DIR] = { dr, db }, \ > - [REG_OUT] = { or, ob }, \ > - [REG_IN] = { ir, ib }, \ > - [REG_DS] = { dsr, dsb }, \ > + [MREG_PULLEN] = { per, peb }, \ > + [MREG_PULL] = { pr, pb }, \ > + [MREG_DIR] = { dr, db }, \ > + [MREG_OUT] = { or, ob }, \ > + [MREG_IN] = { ir, ib }, \ > + [MREG_DS] = { dsr, dsb }, \ > }, \ > } >
On Tue, Mar 29, 2022 at 06:13:19PM +0200, Neil Armstrong wrote: > On 29/03/2022 17:29, Andy Shevchenko wrote: > > Rename REG_* to * as a prerequisite for enabling COMPILE_TEST. > > What error do you hit ? arch/x86/include/asm/arch_hweight.h:9:17: error: expected identifier before string constant 9 | #define REG_OUT "a" | ^~~ drivers/pinctrl/meson/pinctrl-meson.h:69:9: note: in expansion of macro ‘REG_OUT’ 69 | REG_OUT, And some more. > MREG_ is rather ugly, something like PINCONF_REG_ or more simpler MESON_REG_ would be more appropriate. Fine with me.
Hi Andy, On Tue, Mar 29, 2022 at 6:47 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > On Tue, Mar 29, 2022 at 06:13:19PM +0200, Neil Armstrong wrote: > > On 29/03/2022 17:29, Andy Shevchenko wrote: > > > Rename REG_* to * as a prerequisite for enabling COMPILE_TEST. > > > > What error do you hit ? > > arch/x86/include/asm/arch_hweight.h:9:17: error: expected identifier before string constant > 9 | #define REG_OUT "a" > | ^~~ Perhaps REG_{OUT,IN} in arch/x86/include/asm/arch_hweight.h should be renamed instead, as this is a generic header file that can be included anywhere, while the REG_{OUT,IN} definitions are only used locally, in the header file? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On 30/03/2022 10:54, Geert Uytterhoeven wrote: > Hi Andy, > > On Tue, Mar 29, 2022 at 6:47 PM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: >> On Tue, Mar 29, 2022 at 06:13:19PM +0200, Neil Armstrong wrote: >>> On 29/03/2022 17:29, Andy Shevchenko wrote: >>>> Rename REG_* to * as a prerequisite for enabling COMPILE_TEST. >>> >>> What error do you hit ? >> >> arch/x86/include/asm/arch_hweight.h:9:17: error: expected identifier before string constant >> 9 | #define REG_OUT "a" >> | ^~~ > > Perhaps REG_{OUT,IN} in arch/x86/include/asm/arch_hweight.h should be > renamed instead, as this is a generic header file that can be included > anywhere, while the REG_{OUT,IN} definitions are only used locally, > in the header file? Even better, those REG_OUT/REG_IN should be undefined at the end of the header since only used in the headers inline functions: ==============><================================== diff --git a/arch/x86/include/asm/arch_hweight.h b/arch/x86/include/asm/arch_hweight.h index ba88edd0d58b..139a4b0a2a14 100644 --- a/arch/x86/include/asm/arch_hweight.h +++ b/arch/x86/include/asm/arch_hweight.h @@ -52,4 +52,7 @@ static __always_inline unsigned long __arch_hweight64(__u64 w) } #endif /* CONFIG_X86_32 */ +#undef REG_IN +#undef REG_OUT + #endif ==============><================================== Neil > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds
On Wed, Mar 30, 2022 at 11:09:11AM +0200, Neil Armstrong wrote: > On 30/03/2022 10:54, Geert Uytterhoeven wrote: > > On Tue, Mar 29, 2022 at 6:47 PM Andy Shevchenko > > <andriy.shevchenko@linux.intel.com> wrote: > > > On Tue, Mar 29, 2022 at 06:13:19PM +0200, Neil Armstrong wrote: > > > > On 29/03/2022 17:29, Andy Shevchenko wrote: ... > > > > What error do you hit ? > > > > > > arch/x86/include/asm/arch_hweight.h:9:17: error: expected identifier before string constant > > > 9 | #define REG_OUT "a" > > > | ^~~ > > > > Perhaps REG_{OUT,IN} in arch/x86/include/asm/arch_hweight.h should be > > renamed instead, as this is a generic header file that can be included > > anywhere, while the REG_{OUT,IN} definitions are only used locally, > > in the header file? > > Even better, those REG_OUT/REG_IN should be undefined at the end of the header since only > used in the headers inline functions: > ==============><================================== > diff --git a/arch/x86/include/asm/arch_hweight.h b/arch/x86/include/asm/arch_hweight.h > index ba88edd0d58b..139a4b0a2a14 100644 > --- a/arch/x86/include/asm/arch_hweight.h > +++ b/arch/x86/include/asm/arch_hweight.h > @@ -52,4 +52,7 @@ static __always_inline unsigned long __arch_hweight64(__u64 w) > } > #endif /* CONFIG_X86_32 */ > > +#undef REG_IN > +#undef REG_OUT > + > #endif > ==============><================================== Can you submit a formal patch, please? And I think it would be good to have my patch as well, so we do not depend on the fate of the other one.
On 30/03/2022 11:18, Andy Shevchenko wrote: > On Wed, Mar 30, 2022 at 11:09:11AM +0200, Neil Armstrong wrote: >> On 30/03/2022 10:54, Geert Uytterhoeven wrote: >>> On Tue, Mar 29, 2022 at 6:47 PM Andy Shevchenko >>> <andriy.shevchenko@linux.intel.com> wrote: >>>> On Tue, Mar 29, 2022 at 06:13:19PM +0200, Neil Armstrong wrote: >>>>> On 29/03/2022 17:29, Andy Shevchenko wrote: > > ... > >>>>> What error do you hit ? >>>> >>>> arch/x86/include/asm/arch_hweight.h:9:17: error: expected identifier before string constant >>>> 9 | #define REG_OUT "a" >>>> | ^~~ >>> >>> Perhaps REG_{OUT,IN} in arch/x86/include/asm/arch_hweight.h should be >>> renamed instead, as this is a generic header file that can be included >>> anywhere, while the REG_{OUT,IN} definitions are only used locally, >>> in the header file? >> >> Even better, those REG_OUT/REG_IN should be undefined at the end of the header since only >> used in the headers inline functions: >> ==============><================================== >> diff --git a/arch/x86/include/asm/arch_hweight.h b/arch/x86/include/asm/arch_hweight.h >> index ba88edd0d58b..139a4b0a2a14 100644 >> --- a/arch/x86/include/asm/arch_hweight.h >> +++ b/arch/x86/include/asm/arch_hweight.h >> @@ -52,4 +52,7 @@ static __always_inline unsigned long __arch_hweight64(__u64 w) >> } >> #endif /* CONFIG_X86_32 */ >> >> +#undef REG_IN >> +#undef REG_OUT >> + >> #endif >> ==============><================================== > > Can you submit a formal patch, please? I'll submit it separately > > > And I think it would be good to have my patch as well, so we do not depend on > the fate of the other one. > Yes sure Thanks, Neil
On Wed, Mar 30, 2022 at 05:22:56PM +0200, Neil Armstrong wrote: > On 30/03/2022 11:18, Andy Shevchenko wrote: ... > > > > > > What error do you hit ? > > > > > > > > > > arch/x86/include/asm/arch_hweight.h:9:17: error: expected identifier before string constant > > > > > 9 | #define REG_OUT "a" > > > > > | ^~~ > > > > > > > > Perhaps REG_{OUT,IN} in arch/x86/include/asm/arch_hweight.h should be > > > > renamed instead, as this is a generic header file that can be included > > > > anywhere, while the REG_{OUT,IN} definitions are only used locally, > > > > in the header file? > > > > > > Even better, those REG_OUT/REG_IN should be undefined at the end of the header since only > > > used in the headers inline functions: > > > ==============><================================== > > > diff --git a/arch/x86/include/asm/arch_hweight.h b/arch/x86/include/asm/arch_hweight.h > > > index ba88edd0d58b..139a4b0a2a14 100644 > > > --- a/arch/x86/include/asm/arch_hweight.h > > > +++ b/arch/x86/include/asm/arch_hweight.h > > > @@ -52,4 +52,7 @@ static __always_inline unsigned long __arch_hweight64(__u64 w) > > > } > > > #endif /* CONFIG_X86_32 */ > > > > > > +#undef REG_IN > > > +#undef REG_OUT > > > + > > > #endif > > > ==============><================================== > > > > Can you submit a formal patch, please? > > I'll submit it separately Sure! > > And I think it would be good to have my patch as well, so we do not depend on > > the fate of the other one. > > Yes sure Thanks for acknowledging and review!
diff --git a/drivers/pinctrl/meson/pinctrl-meson.c b/drivers/pinctrl/meson/pinctrl-meson.c index 49851444a6e3..64da61ba2bb9 100644 --- a/drivers/pinctrl/meson/pinctrl-meson.c +++ b/drivers/pinctrl/meson/pinctrl-meson.c @@ -218,13 +218,13 @@ static int meson_pinconf_set_output(struct meson_pinctrl *pc, unsigned int pin, bool out) { - return meson_pinconf_set_gpio_bit(pc, pin, REG_DIR, !out); + return meson_pinconf_set_gpio_bit(pc, pin, MREG_DIR, !out); } static int meson_pinconf_get_output(struct meson_pinctrl *pc, unsigned int pin) { - int ret = meson_pinconf_get_gpio_bit(pc, pin, REG_DIR); + int ret = meson_pinconf_get_gpio_bit(pc, pin, MREG_DIR); if (ret < 0) return ret; @@ -236,13 +236,13 @@ static int meson_pinconf_set_drive(struct meson_pinctrl *pc, unsigned int pin, bool high) { - return meson_pinconf_set_gpio_bit(pc, pin, REG_OUT, high); + return meson_pinconf_set_gpio_bit(pc, pin, MREG_OUT, high); } static int meson_pinconf_get_drive(struct meson_pinctrl *pc, unsigned int pin) { - return meson_pinconf_get_gpio_bit(pc, pin, REG_OUT); + return meson_pinconf_get_gpio_bit(pc, pin, MREG_OUT); } static int meson_pinconf_set_output_drive(struct meson_pinctrl *pc, @@ -269,7 +269,7 @@ static int meson_pinconf_disable_bias(struct meson_pinctrl *pc, if (ret) return ret; - meson_calc_reg_and_bit(bank, pin, REG_PULLEN, ®, &bit); + meson_calc_reg_and_bit(bank, pin, MREG_PULLEN, ®, &bit); ret = regmap_update_bits(pc->reg_pullen, reg, BIT(bit), 0); if (ret) return ret; @@ -288,7 +288,7 @@ static int meson_pinconf_enable_bias(struct meson_pinctrl *pc, unsigned int pin, if (ret) return ret; - meson_calc_reg_and_bit(bank, pin, REG_PULL, ®, &bit); + meson_calc_reg_and_bit(bank, pin, MREG_PULL, ®, &bit); if (pull_up) val = BIT(bit); @@ -296,7 +296,7 @@ static int meson_pinconf_enable_bias(struct meson_pinctrl *pc, unsigned int pin, if (ret) return ret; - meson_calc_reg_and_bit(bank, pin, REG_PULLEN, ®, &bit); + meson_calc_reg_and_bit(bank, pin, MREG_PULLEN, ®, &bit); ret = regmap_update_bits(pc->reg_pullen, reg, BIT(bit), BIT(bit)); if (ret) return ret; @@ -321,7 +321,7 @@ static int meson_pinconf_set_drive_strength(struct meson_pinctrl *pc, if (ret) return ret; - meson_calc_reg_and_bit(bank, pin, REG_DS, ®, &bit); + meson_calc_reg_and_bit(bank, pin, MREG_DS, ®, &bit); if (drive_strength_ua <= 500) { ds_val = MESON_PINCONF_DRV_500UA; @@ -407,7 +407,7 @@ static int meson_pinconf_get_pull(struct meson_pinctrl *pc, unsigned int pin) if (ret) return ret; - meson_calc_reg_and_bit(bank, pin, REG_PULLEN, ®, &bit); + meson_calc_reg_and_bit(bank, pin, MREG_PULLEN, ®, &bit); ret = regmap_read(pc->reg_pullen, reg, &val); if (ret) @@ -416,7 +416,7 @@ static int meson_pinconf_get_pull(struct meson_pinctrl *pc, unsigned int pin) if (!(val & BIT(bit))) { conf = PIN_CONFIG_BIAS_DISABLE; } else { - meson_calc_reg_and_bit(bank, pin, REG_PULL, ®, &bit); + meson_calc_reg_and_bit(bank, pin, MREG_PULL, ®, &bit); ret = regmap_read(pc->reg_pull, reg, &val); if (ret) @@ -447,7 +447,7 @@ static int meson_pinconf_get_drive_strength(struct meson_pinctrl *pc, if (ret) return ret; - meson_calc_reg_and_bit(bank, pin, REG_DS, ®, &bit); + meson_calc_reg_and_bit(bank, pin, MREG_DS, ®, &bit); ret = regmap_read(pc->reg_ds, reg, &val); if (ret) @@ -595,7 +595,7 @@ static int meson_gpio_get(struct gpio_chip *chip, unsigned gpio) if (ret) return ret; - meson_calc_reg_and_bit(bank, gpio, REG_IN, ®, &bit); + meson_calc_reg_and_bit(bank, gpio, MREG_IN, ®, &bit); regmap_read(pc->reg_gpio, reg, &val); return !!(val & BIT(bit)); diff --git a/drivers/pinctrl/meson/pinctrl-meson.h b/drivers/pinctrl/meson/pinctrl-meson.h index ff5372e0a475..c00d9ad27843 100644 --- a/drivers/pinctrl/meson/pinctrl-meson.h +++ b/drivers/pinctrl/meson/pinctrl-meson.h @@ -63,12 +63,12 @@ struct meson_reg_desc { * enum meson_reg_type - type of registers encoded in @meson_reg_desc */ enum meson_reg_type { - REG_PULLEN, - REG_PULL, - REG_DIR, - REG_OUT, - REG_IN, - REG_DS, + MREG_PULLEN, + MREG_PULL, + MREG_DIR, + MREG_OUT, + MREG_IN, + MREG_DS, NUM_REG, }; @@ -150,12 +150,12 @@ struct meson_pinctrl { .irq_first = fi, \ .irq_last = li, \ .regs = { \ - [REG_PULLEN] = { per, peb }, \ - [REG_PULL] = { pr, pb }, \ - [REG_DIR] = { dr, db }, \ - [REG_OUT] = { or, ob }, \ - [REG_IN] = { ir, ib }, \ - [REG_DS] = { dsr, dsb }, \ + [MREG_PULLEN] = { per, peb }, \ + [MREG_PULL] = { pr, pb }, \ + [MREG_DIR] = { dr, db }, \ + [MREG_OUT] = { or, ob }, \ + [MREG_IN] = { ir, ib }, \ + [MREG_DS] = { dsr, dsb }, \ }, \ }
Rename REG_* to MREG_* as a prerequisite for enabling COMPILE_TEST. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/pinctrl/meson/pinctrl-meson.c | 24 ++++++++++++------------ drivers/pinctrl/meson/pinctrl-meson.h | 24 ++++++++++++------------ 2 files changed, 24 insertions(+), 24 deletions(-)