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

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

Posted by Jesse Taube 2 years, 9 months ago

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},
 	{}
-- 
2.38.1

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

Posted by Rob Herring 2 years, 8 months ago

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

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

Posted by Lee Jones 2 years, 8 months ago

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

-- 
Lee Jones [李琼斯]

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

Posted by Rob Herring 2 years, 8 months ago

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

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

Posted by Lee Jones 2 years, 7 months ago

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.

-- 
Lee Jones [李琼斯]

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

Posted by Jesse Taube 2 years, 8 months ago


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
>

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

Posted by Lee Jones 2 years, 8 months ago

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
> 

-- 
Lee Jones [李琼斯]

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

Posted by Jesse Taube 2 years, 9 months ago

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},
>   	{}

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

Posted by Lee Jones 2 years, 8 months ago

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},
> >   	{}

-- 
Lee Jones [李琼斯]