[v2] drivers/mfd: simple-mfd-i2c: Add generic compatible

  Some devices may want to use this driver without having a specific
compatible string. Add a generic compatible string to allow this.

Signed-off-by: Jesse Taube <Mr.Bossman075@gmail.com>
---
 drivers/mfd/simple-mfd-i2c.c | 1 +
 1 file changed, 1 insertion(+)
  

Are there any updates?

On 12/2/22 06:32, Jesse Taube wrote:
> Some devices may want to use this driver without having a specific
> compatible string. Add a generic compatible string to allow this.
> 
> Signed-off-by: Jesse Taube <Mr.Bossman075@gmail.com>
> ---
>   drivers/mfd/simple-mfd-i2c.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/mfd/simple-mfd-i2c.c b/drivers/mfd/simple-mfd-i2c.c
> index f4c8fc3ee463..0bda0dd9276e 100644
> --- a/drivers/mfd/simple-mfd-i2c.c
> +++ b/drivers/mfd/simple-mfd-i2c.c
> @@ -73,6 +73,7 @@ static const struct simple_mfd_data silergy_sy7636a = {
>   };
>   
>   static const struct of_device_id simple_mfd_i2c_of_match[] = {
> +	{ .compatible = "simple-mfd-i2c-generic" },
>   	{ .compatible = "kontron,sl28cpld" },
>   	{ .compatible = "silergy,sy7636a", .data = &silergy_sy7636a},
>   	{}
  

On Wed, 14 Dec 2022, Jesse Taube wrote:

> Are there any updates?

Please refrain from top-posting.

I'll get around to it after the merge-window has closed.

> On 12/2/22 06:32, Jesse Taube wrote:
> > Some devices may want to use this driver without having a specific
> > compatible string. Add a generic compatible string to allow this.
> > 
> > Signed-off-by: Jesse Taube <Mr.Bossman075@gmail.com>
> > ---
> >   drivers/mfd/simple-mfd-i2c.c | 1 +
> >   1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/mfd/simple-mfd-i2c.c b/drivers/mfd/simple-mfd-i2c.c
> > index f4c8fc3ee463..0bda0dd9276e 100644
> > --- a/drivers/mfd/simple-mfd-i2c.c
> > +++ b/drivers/mfd/simple-mfd-i2c.c
> > @@ -73,6 +73,7 @@ static const struct simple_mfd_data silergy_sy7636a = {
> >   };
> >   static const struct of_device_id simple_mfd_i2c_of_match[] = {
> > +	{ .compatible = "simple-mfd-i2c-generic" },
> >   	{ .compatible = "kontron,sl28cpld" },
> >   	{ .compatible = "silergy,sy7636a", .data = &silergy_sy7636a},
> >   	{}
  

On Fri, 02 Dec 2022, Jesse Taube wrote:

> Some devices may want to use this driver without having a specific
> compatible string. Add a generic compatible string to allow this.
> 
> Signed-off-by: Jesse Taube <Mr.Bossman075@gmail.com>
> ---
>  drivers/mfd/simple-mfd-i2c.c | 1 +
>  1 file changed, 1 insertion(+)

Sounds like a good idea.  I've changed the subject line a little (no
need to say 'drivers') and applied the patch, thanks.

> diff --git a/drivers/mfd/simple-mfd-i2c.c b/drivers/mfd/simple-mfd-i2c.c
> index f4c8fc3ee463..0bda0dd9276e 100644
> --- a/drivers/mfd/simple-mfd-i2c.c
> +++ b/drivers/mfd/simple-mfd-i2c.c
> @@ -73,6 +73,7 @@ static const struct simple_mfd_data silergy_sy7636a = {
>  };
>  
>  static const struct of_device_id simple_mfd_i2c_of_match[] = {
> +	{ .compatible = "simple-mfd-i2c-generic" },
>  	{ .compatible = "kontron,sl28cpld" },
>  	{ .compatible = "silergy,sy7636a", .data = &silergy_sy7636a},
>  	{}
> -- 
> 2.38.1
>
  

On Fri, Dec 02, 2022 at 06:32:26AM -0500, Jesse Taube wrote:
> Some devices may want to use this driver without having a specific
> compatible string. Add a generic compatible string to allow this.

What devices need this?

Is that no specific compatible string at all or just in the kernel? 
Because the former definitely goes against DT requirements. The latter 
enables the former without a schema.

> 
> Signed-off-by: Jesse Taube <Mr.Bossman075@gmail.com>
> ---
>  drivers/mfd/simple-mfd-i2c.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/mfd/simple-mfd-i2c.c b/drivers/mfd/simple-mfd-i2c.c
> index f4c8fc3ee463..0bda0dd9276e 100644
> --- a/drivers/mfd/simple-mfd-i2c.c
> +++ b/drivers/mfd/simple-mfd-i2c.c
> @@ -73,6 +73,7 @@ static const struct simple_mfd_data silergy_sy7636a = {
>  };
>  
>  static const struct of_device_id simple_mfd_i2c_of_match[] = {
> +	{ .compatible = "simple-mfd-i2c-generic" },

Simple and generic? There is no such device. Anywhere.

This is also not documented which is how I found it (make 
dt_compatible_check). But this should be reverted or dropped rather than 
documented IMO.

Rob
  

On Thu, 19 Jan 2023, Rob Herring wrote:

> On Fri, Dec 02, 2022 at 06:32:26AM -0500, Jesse Taube wrote:
> > Some devices may want to use this driver without having a specific
> > compatible string. Add a generic compatible string to allow this.
> 
> What devices need this?
> 
> Is that no specific compatible string at all or just in the kernel? 
> Because the former definitely goes against DT requirements. The latter 
> enables the former without a schema.
> 
> > 
> > Signed-off-by: Jesse Taube <Mr.Bossman075@gmail.com>
> > ---
> >  drivers/mfd/simple-mfd-i2c.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/mfd/simple-mfd-i2c.c b/drivers/mfd/simple-mfd-i2c.c
> > index f4c8fc3ee463..0bda0dd9276e 100644
> > --- a/drivers/mfd/simple-mfd-i2c.c
> > +++ b/drivers/mfd/simple-mfd-i2c.c
> > @@ -73,6 +73,7 @@ static const struct simple_mfd_data silergy_sy7636a = {
> >  };
> >  
> >  static const struct of_device_id simple_mfd_i2c_of_match[] = {
> > +	{ .compatible = "simple-mfd-i2c-generic" },
> 
> Simple and generic? There is no such device. Anywhere.
> 
> This is also not documented which is how I found it (make 
> dt_compatible_check). But this should be reverted or dropped rather than 
> documented IMO.

I thought it would be better than having a huge list here.

Devices should *also* be allocated a specific compatible string.

$ git grep simple-mfd -- arch
  

On 1/19/23 15:54, Lee Jones wrote:
> On Thu, 19 Jan 2023, Rob Herring wrote:
> 
>> On Fri, Dec 02, 2022 at 06:32:26AM -0500, Jesse Taube wrote:
>>> Some devices may want to use this driver without having a specific
>>> compatible string. Add a generic compatible string to allow this.
>>
>> What devices need this?
>>
>> Is that no specific compatible string at all or just in the kernel?
>> Because the former definitely goes against DT requirements. The latter
>> enables the former without a schema.
>>
>>>
>>> Signed-off-by: Jesse Taube <Mr.Bossman075@gmail.com>
>>> ---
>>>   drivers/mfd/simple-mfd-i2c.c | 1 +
>>>   1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/mfd/simple-mfd-i2c.c b/drivers/mfd/simple-mfd-i2c.c
>>> index f4c8fc3ee463..0bda0dd9276e 100644
>>> --- a/drivers/mfd/simple-mfd-i2c.c
>>> +++ b/drivers/mfd/simple-mfd-i2c.c
>>> @@ -73,6 +73,7 @@ static const struct simple_mfd_data silergy_sy7636a = {
>>>   };
>>>   
>>>   static const struct of_device_id simple_mfd_i2c_of_match[] = {
>>> +	{ .compatible = "simple-mfd-i2c-generic" },
>>
>> Simple and generic? There is no such device. Anywhere.
>>
>> This is also not documented which is how I found it (make
>> dt_compatible_check).
I will write docs if needed.
But this should be reverted or dropped rather than
>> documented IMO.
Hi Rob, sorry for the disturbance. The reason I submitted this patch is 
this driver is generic and can be used by many different devices. Adding 
a generic compatible handle allows device tree writers avoid editing the 
  C source. I also will be submitting device trees that use this in the 
future if added.
Thanks,
Jesse Taube
> 
> I thought it would be better than having a huge list here.
> 
> Devices should *also* be allocated a specific compatible string.
> 
> $ git grep simple-mfd -- arch
>
  

On Thu, Jan 19, 2023 at 2:54 PM Lee Jones <lee@kernel.org> wrote:
>
> On Thu, 19 Jan 2023, Rob Herring wrote:
>
> > On Fri, Dec 02, 2022 at 06:32:26AM -0500, Jesse Taube wrote:
> > > Some devices may want to use this driver without having a specific
> > > compatible string. Add a generic compatible string to allow this.
> >
> > What devices need this?
> >
> > Is that no specific compatible string at all or just in the kernel?
> > Because the former definitely goes against DT requirements. The latter
> > enables the former without a schema.
> >
> > >
> > > Signed-off-by: Jesse Taube <Mr.Bossman075@gmail.com>
> > > ---
> > >  drivers/mfd/simple-mfd-i2c.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/drivers/mfd/simple-mfd-i2c.c b/drivers/mfd/simple-mfd-i2c.c
> > > index f4c8fc3ee463..0bda0dd9276e 100644
> > > --- a/drivers/mfd/simple-mfd-i2c.c
> > > +++ b/drivers/mfd/simple-mfd-i2c.c
> > > @@ -73,6 +73,7 @@ static const struct simple_mfd_data silergy_sy7636a = {
> > >  };
> > >
> > >  static const struct of_device_id simple_mfd_i2c_of_match[] = {
> > > +   { .compatible = "simple-mfd-i2c-generic" },
> >
> > Simple and generic? There is no such device. Anywhere.
> >
> > This is also not documented which is how I found it (make
> > dt_compatible_check). But this should be reverted or dropped rather than
> > documented IMO.
>
> I thought it would be better than having a huge list here.

What indication is there that the list would be huge? We have 2 out of
137 MFD bindings. Usually if the MFD is simple, we'd make it a single
node. It just needs to be clear what the conditions are for using it.

> Devices should *also* be allocated a specific compatible string.
>
> $ git grep simple-mfd -- arch

Why can't simple-mfd be used here?

Rob
  

On Fri, 20 Jan 2023, Rob Herring wrote:

> On Thu, Jan 19, 2023 at 2:54 PM Lee Jones <lee@kernel.org> wrote:
> >
> > On Thu, 19 Jan 2023, Rob Herring wrote:
> >
> > > On Fri, Dec 02, 2022 at 06:32:26AM -0500, Jesse Taube wrote:
> > > > Some devices may want to use this driver without having a specific
> > > > compatible string. Add a generic compatible string to allow this.
> > >
> > > What devices need this?
> > >
> > > Is that no specific compatible string at all or just in the kernel?
> > > Because the former definitely goes against DT requirements. The latter
> > > enables the former without a schema.
> > >
> > > >
> > > > Signed-off-by: Jesse Taube <Mr.Bossman075@gmail.com>
> > > > ---
> > > >  drivers/mfd/simple-mfd-i2c.c | 1 +
> > > >  1 file changed, 1 insertion(+)
> > > >
> > > > diff --git a/drivers/mfd/simple-mfd-i2c.c b/drivers/mfd/simple-mfd-i2c.c
> > > > index f4c8fc3ee463..0bda0dd9276e 100644
> > > > --- a/drivers/mfd/simple-mfd-i2c.c
> > > > +++ b/drivers/mfd/simple-mfd-i2c.c
> > > > @@ -73,6 +73,7 @@ static const struct simple_mfd_data silergy_sy7636a = {
> > > >  };
> > > >
> > > >  static const struct of_device_id simple_mfd_i2c_of_match[] = {
> > > > +   { .compatible = "simple-mfd-i2c-generic" },
> > >
> > > Simple and generic? There is no such device. Anywhere.
> > >
> > > This is also not documented which is how I found it (make
> > > dt_compatible_check). But this should be reverted or dropped rather than
> > > documented IMO.
> >
> > I thought it would be better than having a huge list here.
> 
> What indication is there that the list would be huge? We have 2 out of
> 137 MFD bindings. Usually if the MFD is simple, we'd make it a single
> node. It just needs to be clear what the conditions are for using it.
> 
> > Devices should *also* be allocated a specific compatible string.
> >
> > $ git grep simple-mfd -- arch
> 
> Why can't simple-mfd be used here?

Until this is clarified, agreed and documented, I'm dropping the patch.