Message ID | 20220124121009.108649-3-alistair@alistair23.me (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | Add support for the silergy,sy7636a | expand |
Hi Alistair, Thanks for your patch, which is now commit bae5a4acef67db88 ("mfd: simple-mfd-i2c: Add a Kconfig name") in mfd/for-mfd-next. On Mon, Jan 24, 2022 at 1:24 PM Alistair Francis <alistair@alistair23.me> wrote: > Add a Kconfig name to the "Simple Multi-Functional Device support (I2C)" > device so that it can be enabled via menuconfig. Which still does not explain why this would be needed... > Signed-off-by: Alistair Francis <alistair@alistair23.me> > Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org> > --- a/drivers/mfd/Kconfig > +++ b/drivers/mfd/Kconfig > @@ -1188,7 +1188,7 @@ config MFD_SI476X_CORE > module will be called si476x-core. > > config MFD_SIMPLE_MFD_I2C > - tristate > + tristate "Simple Multi-Functional Device support (I2C)" > depends on I2C > select MFD_CORE > select REGMAP_I2C The help text states: | This driver creates a single register map with the intention for it | to be shared by all sub-devices. Yes, that's what MFD does? | Once the register map has been successfully initialised, any | sub-devices represented by child nodes in Device Tree will be | subsequently registered. OK...? Still, no clue about what this driver really does, and why and when it would be needed. There is one driver symbol that selects MFD_SIMPLE_MFD_I2C. There are no driver symbols that depend on this symbol. If you have a driver in the pipeline that can make use of this, can't it just select MFD_SIMPLE_MFD_I2C, so the symbol itself can stay invisible? Thanks! 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 Tue, Mar 8, 2022 at 8:53 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Alistair, > > Thanks for your patch, which is now commit bae5a4acef67db88 > ("mfd: simple-mfd-i2c: Add a Kconfig name") in mfd/for-mfd-next. > > On Mon, Jan 24, 2022 at 1:24 PM Alistair Francis <alistair@alistair23.me> wrote: > > Add a Kconfig name to the "Simple Multi-Functional Device support (I2C)" > > device so that it can be enabled via menuconfig. > > Which still does not explain why this would be needed... > > > Signed-off-by: Alistair Francis <alistair@alistair23.me> > > Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org> > > > --- a/drivers/mfd/Kconfig > > +++ b/drivers/mfd/Kconfig > > @@ -1188,7 +1188,7 @@ config MFD_SI476X_CORE > > module will be called si476x-core. > > > > config MFD_SIMPLE_MFD_I2C > > - tristate > > + tristate "Simple Multi-Functional Device support (I2C)" > > depends on I2C > > select MFD_CORE > > select REGMAP_I2C > > The help text states: > > | This driver creates a single register map with the intention for it > | to be shared by all sub-devices. > > Yes, that's what MFD does? > > | Once the register map has been successfully initialised, any > | sub-devices represented by child nodes in Device Tree will be > | subsequently registered. > > OK...? > > Still, no clue about what this driver really does, and why and when > it would be needed. > > There is one driver symbol that selects MFD_SIMPLE_MFD_I2C. > There are no driver symbols that depend on this symbol. > > If you have a driver in the pipeline that can make use of this, > can't it just select MFD_SIMPLE_MFD_I2C, so the symbol itself can > stay invisible? My patch "mfd: simple-mfd-i2c: Enable support for the silergy,sy7636a" allows using this driver for the silergy,sy7636a MFD. So it's nice to be able to enable and disable it as required. Alistair > > Thanks! > > 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
Hi Alistair, On Sat, Mar 19, 2022 at 3:36 AM Alistair Francis <alistair23@gmail.com> wrote: > On Tue, Mar 8, 2022 at 8:53 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Thanks for your patch, which is now commit bae5a4acef67db88 > > ("mfd: simple-mfd-i2c: Add a Kconfig name") in mfd/for-mfd-next. > > > > On Mon, Jan 24, 2022 at 1:24 PM Alistair Francis <alistair@alistair23.me> wrote: > > > Add a Kconfig name to the "Simple Multi-Functional Device support (I2C)" > > > device so that it can be enabled via menuconfig. > > > > Which still does not explain why this would be needed... > > > > > Signed-off-by: Alistair Francis <alistair@alistair23.me> > > > Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org> > > > > > --- a/drivers/mfd/Kconfig > > > +++ b/drivers/mfd/Kconfig > > > @@ -1188,7 +1188,7 @@ config MFD_SI476X_CORE > > > module will be called si476x-core. > > > > > > config MFD_SIMPLE_MFD_I2C > > > - tristate > > > + tristate "Simple Multi-Functional Device support (I2C)" > > > depends on I2C > > > select MFD_CORE > > > select REGMAP_I2C > > > > The help text states: > > > > | This driver creates a single register map with the intention for it > > | to be shared by all sub-devices. > > > > Yes, that's what MFD does? > > > > | Once the register map has been successfully initialised, any > > | sub-devices represented by child nodes in Device Tree will be > > | subsequently registered. > > > > OK...? > > > > Still, no clue about what this driver really does, and why and when > > it would be needed. > > > > There is one driver symbol that selects MFD_SIMPLE_MFD_I2C. > > There are no driver symbols that depend on this symbol. > > > > If you have a driver in the pipeline that can make use of this, > > can't it just select MFD_SIMPLE_MFD_I2C, so the symbol itself can > > stay invisible? > > My patch "mfd: simple-mfd-i2c: Enable support for the silergy,sy7636a" > allows using this driver for the silergy,sy7636a MFD. So it's nice to > be able to enable and disable it as required. So after that patch, enabling MFD_SIMPLE_MFD_I2C will enable support for an ever-growing random bunch of devices, none of which is described in the help text? To me, ghat doesn't look like the way to go forward... 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 3/19/22 02:28, Geert Uytterhoeven wrote: > Hi Alistair, > > On Sat, Mar 19, 2022 at 3:36 AM Alistair Francis <alistair23@gmail.com> wrote: >> On Tue, Mar 8, 2022 at 8:53 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: >>> Thanks for your patch, which is now commit bae5a4acef67db88 >>> ("mfd: simple-mfd-i2c: Add a Kconfig name") in mfd/for-mfd-next. >>> >>> On Mon, Jan 24, 2022 at 1:24 PM Alistair Francis <alistair@alistair23.me> wrote: >>>> Add a Kconfig name to the "Simple Multi-Functional Device support (I2C)" >>>> device so that it can be enabled via menuconfig. >>> >>> Which still does not explain why this would be needed... >>> >>>> Signed-off-by: Alistair Francis <alistair@alistair23.me> >>>> Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org> >>> >>>> --- a/drivers/mfd/Kconfig >>>> +++ b/drivers/mfd/Kconfig >>>> @@ -1188,7 +1188,7 @@ config MFD_SI476X_CORE >>>> module will be called si476x-core. >>>> >>>> config MFD_SIMPLE_MFD_I2C >>>> - tristate >>>> + tristate "Simple Multi-Functional Device support (I2C)" >>>> depends on I2C >>>> select MFD_CORE >>>> select REGMAP_I2C >>> >>> The help text states: >>> >>> | This driver creates a single register map with the intention for it >>> | to be shared by all sub-devices. >>> >>> Yes, that's what MFD does? >>> >>> | Once the register map has been successfully initialised, any >>> | sub-devices represented by child nodes in Device Tree will be >>> | subsequently registered. >>> >>> OK...? >>> >>> Still, no clue about what this driver really does, and why and when >>> it would be needed. >>> >>> There is one driver symbol that selects MFD_SIMPLE_MFD_I2C. >>> There are no driver symbols that depend on this symbol. >>> >>> If you have a driver in the pipeline that can make use of this, >>> can't it just select MFD_SIMPLE_MFD_I2C, so the symbol itself can >>> stay invisible? >> >> My patch "mfd: simple-mfd-i2c: Enable support for the silergy,sy7636a" >> allows using this driver for the silergy,sy7636a MFD. So it's nice to >> be able to enable and disable it as required. > > So after that patch, enabling MFD_SIMPLE_MFD_I2C will enable > support for an ever-growing random bunch of devices, none of which > is described in the help text? > To me, ghat doesn't look like the way to go forward... > I am probably missing something. Why not something like the following ? config MFD_SY7636A tristate "Silergy SY7636A voltage regulator" depends on I2C select MFD_SIMPLE_MFD_I2C help Enable support for Silergy SY7636A voltage regulator. To enable support for building sub-devices as modules, choose M here. This would be quite similar to MFD_SL28CPLD which essentially does the same (and, unless I am missing something, doesn't have its own driver either). Sub-devices would then depend on MFD_SY7636A. Guenter > 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 Sun, Mar 20, 2022 at 12:48 AM Guenter Roeck <linux@roeck-us.net> wrote: > > On 3/19/22 02:28, Geert Uytterhoeven wrote: > > Hi Alistair, > > > > On Sat, Mar 19, 2022 at 3:36 AM Alistair Francis <alistair23@gmail.com> wrote: > >> On Tue, Mar 8, 2022 at 8:53 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > >>> Thanks for your patch, which is now commit bae5a4acef67db88 > >>> ("mfd: simple-mfd-i2c: Add a Kconfig name") in mfd/for-mfd-next. > >>> > >>> On Mon, Jan 24, 2022 at 1:24 PM Alistair Francis <alistair@alistair23.me> wrote: > >>>> Add a Kconfig name to the "Simple Multi-Functional Device support (I2C)" > >>>> device so that it can be enabled via menuconfig. > >>> > >>> Which still does not explain why this would be needed... > >>> > >>>> Signed-off-by: Alistair Francis <alistair@alistair23.me> > >>>> Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org> > >>> > >>>> --- a/drivers/mfd/Kconfig > >>>> +++ b/drivers/mfd/Kconfig > >>>> @@ -1188,7 +1188,7 @@ config MFD_SI476X_CORE > >>>> module will be called si476x-core. > >>>> > >>>> config MFD_SIMPLE_MFD_I2C > >>>> - tristate > >>>> + tristate "Simple Multi-Functional Device support (I2C)" > >>>> depends on I2C > >>>> select MFD_CORE > >>>> select REGMAP_I2C > >>> > >>> The help text states: > >>> > >>> | This driver creates a single register map with the intention for it > >>> | to be shared by all sub-devices. > >>> > >>> Yes, that's what MFD does? > >>> > >>> | Once the register map has been successfully initialised, any > >>> | sub-devices represented by child nodes in Device Tree will be > >>> | subsequently registered. > >>> > >>> OK...? > >>> > >>> Still, no clue about what this driver really does, and why and when > >>> it would be needed. > >>> > >>> There is one driver symbol that selects MFD_SIMPLE_MFD_I2C. > >>> There are no driver symbols that depend on this symbol. > >>> > >>> If you have a driver in the pipeline that can make use of this, > >>> can't it just select MFD_SIMPLE_MFD_I2C, so the symbol itself can > >>> stay invisible? > >> > >> My patch "mfd: simple-mfd-i2c: Enable support for the silergy,sy7636a" > >> allows using this driver for the silergy,sy7636a MFD. So it's nice to > >> be able to enable and disable it as required. > > > > So after that patch, enabling MFD_SIMPLE_MFD_I2C will enable > > support for an ever-growing random bunch of devices, none of which > > is described in the help text? > > To me, ghat doesn't look like the way to go forward... > > > > I am probably missing something. Why not something like the following ? > > config MFD_SY7636A > tristate "Silergy SY7636A voltage regulator" > depends on I2C > select MFD_SIMPLE_MFD_I2C > help > Enable support for Silergy SY7636A voltage regulator. > > To enable support for building sub-devices as modules, > choose M here. > > > This would be quite similar to MFD_SL28CPLD which essentially does > the same (and, unless I am missing something, doesn't have its own > driver either). Sub-devices would then depend on MFD_SY7636A. That's fine with me. As you said this patch is already in the mfd/for-mfd-next tree, should I resend the series? Alistair > > Guenter > > > 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 Mon, 21 Mar 2022, Alistair Francis wrote: > On Sun, Mar 20, 2022 at 12:48 AM Guenter Roeck <linux@roeck-us.net> wrote: > > > > On 3/19/22 02:28, Geert Uytterhoeven wrote: > > > Hi Alistair, > > > > > > On Sat, Mar 19, 2022 at 3:36 AM Alistair Francis <alistair23@gmail.com> wrote: > > >> On Tue, Mar 8, 2022 at 8:53 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > >>> Thanks for your patch, which is now commit bae5a4acef67db88 > > >>> ("mfd: simple-mfd-i2c: Add a Kconfig name") in mfd/for-mfd-next. > > >>> > > >>> On Mon, Jan 24, 2022 at 1:24 PM Alistair Francis <alistair@alistair23.me> wrote: > > >>>> Add a Kconfig name to the "Simple Multi-Functional Device support (I2C)" > > >>>> device so that it can be enabled via menuconfig. > > >>> > > >>> Which still does not explain why this would be needed... > > >>> > > >>>> Signed-off-by: Alistair Francis <alistair@alistair23.me> > > >>>> Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org> > > >>> > > >>>> --- a/drivers/mfd/Kconfig > > >>>> +++ b/drivers/mfd/Kconfig > > >>>> @@ -1188,7 +1188,7 @@ config MFD_SI476X_CORE > > >>>> module will be called si476x-core. > > >>>> > > >>>> config MFD_SIMPLE_MFD_I2C > > >>>> - tristate > > >>>> + tristate "Simple Multi-Functional Device support (I2C)" > > >>>> depends on I2C > > >>>> select MFD_CORE > > >>>> select REGMAP_I2C > > >>> > > >>> The help text states: > > >>> > > >>> | This driver creates a single register map with the intention for it > > >>> | to be shared by all sub-devices. > > >>> > > >>> Yes, that's what MFD does? > > >>> > > >>> | Once the register map has been successfully initialised, any > > >>> | sub-devices represented by child nodes in Device Tree will be > > >>> | subsequently registered. > > >>> > > >>> OK...? > > >>> > > >>> Still, no clue about what this driver really does, and why and when > > >>> it would be needed. > > >>> > > >>> There is one driver symbol that selects MFD_SIMPLE_MFD_I2C. > > >>> There are no driver symbols that depend on this symbol. > > >>> > > >>> If you have a driver in the pipeline that can make use of this, > > >>> can't it just select MFD_SIMPLE_MFD_I2C, so the symbol itself can > > >>> stay invisible? > > >> > > >> My patch "mfd: simple-mfd-i2c: Enable support for the silergy,sy7636a" > > >> allows using this driver for the silergy,sy7636a MFD. So it's nice to > > >> be able to enable and disable it as required. > > > > > > So after that patch, enabling MFD_SIMPLE_MFD_I2C will enable > > > support for an ever-growing random bunch of devices, none of which > > > is described in the help text? > > > To me, ghat doesn't look like the way to go forward... > > > > > > > I am probably missing something. Why not something like the following ? > > > > config MFD_SY7636A > > tristate "Silergy SY7636A voltage regulator" > > depends on I2C > > select MFD_SIMPLE_MFD_I2C > > help > > Enable support for Silergy SY7636A voltage regulator. > > > > To enable support for building sub-devices as modules, > > choose M here. > > > > > > This would be quite similar to MFD_SL28CPLD which essentially does > > the same (and, unless I am missing something, doesn't have its own > > driver either). Sub-devices would then depend on MFD_SY7636A. > > That's fine with me. > > As you said this patch is already in the mfd/for-mfd-next tree, should > I resend the series? Making the symbol selectable-only is fine with me also. Please send a subsequent patch.
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig index ba0b3eb131f1..e0d2fcb10a0c 100644 --- a/drivers/mfd/Kconfig +++ b/drivers/mfd/Kconfig @@ -1188,7 +1188,7 @@ config MFD_SI476X_CORE module will be called si476x-core. config MFD_SIMPLE_MFD_I2C - tristate + tristate "Simple Multi-Functional Device support (I2C)" depends on I2C select MFD_CORE select REGMAP_I2C
Add a Kconfig name to the "Simple Multi-Functional Device support (I2C)" device so that it can be enabled via menuconfig. Signed-off-by: Alistair Francis <alistair@alistair23.me> Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org> --- drivers/mfd/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)