2024-04-02 19:35:39

by Andreas Kemnade

[permalink] [raw]
Subject: [PATCH 0/2] mfd: Add ROHM BD71879

Add software-compatible variant of the BD71828.

Andreas Kemnade (2):
dt-bindings: mfd: Add ROHM BD71879
mfd: rohm-bd71828: Add software-compatible variant BD71879

Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml | 4 +++-
drivers/mfd/rohm-bd71828.c | 4 ++++
2 files changed, 7 insertions(+), 1 deletion(-)

--
2.39.2



2024-04-02 19:35:39

by Andreas Kemnade

[permalink] [raw]
Subject: [PATCH 2/2] mfd: rohm-bd71828: Add software-compatible variant BD71879

Add the BD71879 PMIC which is software-compatible to the BD71828, so reuse
the same device_type enum.

Signed-off-by: Andreas Kemnade <[email protected]>
Suggested-by: Matti Vaittinen <[email protected]>
---
drivers/mfd/rohm-bd71828.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/mfd/rohm-bd71828.c b/drivers/mfd/rohm-bd71828.c
index 4a1fa8a0d76a..f0b444690d4d 100644
--- a/drivers/mfd/rohm-bd71828.c
+++ b/drivers/mfd/rohm-bd71828.c
@@ -585,6 +585,10 @@ static const struct of_device_id bd71828_of_match[] = {
{
.compatible = "rohm,bd71828",
.data = (void *)ROHM_CHIP_TYPE_BD71828,
+ }, {
+ .compatible = "rohm,bd71879",
+ /* equivalent from a software point of view */
+ .data = (void *)ROHM_CHIP_TYPE_BD71828,
}, {
.compatible = "rohm,bd71815",
.data = (void *)ROHM_CHIP_TYPE_BD71815,
--
2.39.2


2024-04-02 19:38:49

by Andreas Kemnade

[permalink] [raw]
Subject: [PATCH 1/2] dt-bindings: mfd: Add ROHM BD71879

As this chip was seen in several devices in the wild, add it.

Signed-off-by: Andreas Kemnade <[email protected]>
Suggested-by: Matti Vaittinen <[email protected]>
---
Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml b/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml
index 0b62f854bf6b..e4df09e8961c 100644
--- a/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml
+++ b/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml
@@ -17,7 +17,9 @@ description: |

properties:
compatible:
- const: rohm,bd71828
+ enum:
+ - rohm,bd71828
+ - rohm,bd71879

reg:
description:
--
2.39.2


2024-04-03 08:28:54

by Matti Vaittinen

[permalink] [raw]
Subject: Re: [PATCH 2/2] mfd: rohm-bd71828: Add software-compatible variant BD71879

On 4/2/24 22:35, Andreas Kemnade wrote:
> Add the BD71879 PMIC which is software-compatible to the BD71828, so reuse
> the same device_type enum.
>
> Signed-off-by: Andreas Kemnade <[email protected]>
> Suggested-by: Matti Vaittinen <[email protected]>

Acked-by: Matti Vaittinen <[email protected]>

> ---
> drivers/mfd/rohm-bd71828.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/mfd/rohm-bd71828.c b/drivers/mfd/rohm-bd71828.c
> index 4a1fa8a0d76a..f0b444690d4d 100644
> --- a/drivers/mfd/rohm-bd71828.c
> +++ b/drivers/mfd/rohm-bd71828.c
> @@ -585,6 +585,10 @@ static const struct of_device_id bd71828_of_match[] = {
> {
> .compatible = "rohm,bd71828",
> .data = (void *)ROHM_CHIP_TYPE_BD71828,
> + }, {
> + .compatible = "rohm,bd71879",
> + /* equivalent from a software point of view */
> + .data = (void *)ROHM_CHIP_TYPE_BD71828,
> }, {
> .compatible = "rohm,bd71815",
> .data = (void *)ROHM_CHIP_TYPE_BD71815,

--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~


2024-04-03 08:33:03

by Matti Vaittinen

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: mfd: Add ROHM BD71879

On 4/2/24 22:35, Andreas Kemnade wrote:
> As this chip was seen in several devices in the wild, add it.
>
> Signed-off-by: Andreas Kemnade <[email protected]>
> Suggested-by: Matti Vaittinen <[email protected]>


Acked-by: Matti Vaittinen <[email protected]>

> ---
> Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml b/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml
> index 0b62f854bf6b..e4df09e8961c 100644
> --- a/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml
> +++ b/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml
> @@ -17,7 +17,9 @@ description: |
>
> properties:
> compatible:
> - const: rohm,bd71828
> + enum:
> + - rohm,bd71828
> + - rohm,bd71879
>
> reg:
> description:

--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~


2024-04-04 07:00:09

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: mfd: Add ROHM BD71879

On 02/04/2024 21:35, Andreas Kemnade wrote:
> As this chip was seen in several devices in the wild, add it.
>
> Signed-off-by: Andreas Kemnade <[email protected]>
> Suggested-by: Matti Vaittinen <[email protected]>
> ---
> Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml b/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml
> index 0b62f854bf6b..e4df09e8961c 100644
> --- a/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml
> +++ b/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml
> @@ -17,7 +17,9 @@ description: |
>
> properties:
> compatible:
> - const: rohm,bd71828
> + enum:
> + - rohm,bd71828
> + - rohm,bd71879

In your second commit you claim they are compatible, so why they are not
marked as such?

Best regards,
Krzysztof


2024-04-04 10:30:38

by Andreas Kemnade

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: mfd: Add ROHM BD71879

On Thu, 4 Apr 2024 08:59:54 +0200
Krzysztof Kozlowski <[email protected]> wrote:

> On 02/04/2024 21:35, Andreas Kemnade wrote:
> > As this chip was seen in several devices in the wild, add it.
> >
> > Signed-off-by: Andreas Kemnade <[email protected]>
> > Suggested-by: Matti Vaittinen <[email protected]>
> > ---
> > Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml b/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml
> > index 0b62f854bf6b..e4df09e8961c 100644
> > --- a/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml
> > +++ b/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml
> > @@ -17,7 +17,9 @@ description: |
> >
> > properties:
> > compatible:
> > - const: rohm,bd71828
> > + enum:
> > + - rohm,bd71828
> > + - rohm,bd71879
>
> In your second commit you claim they are compatible, so why they are not
> marked as such?
>
so you mean allowing

compatible = "rohm,bd71828"
and
compatible = "rohm,bd71879", "rohm,bd71828"

Regards,
Andreas
> Best regards,
> Krzysztof
>
>


2024-04-04 12:04:25

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: mfd: Add ROHM BD71879

On 04/04/2024 12:30, Andreas Kemnade wrote:
> On Thu, 4 Apr 2024 08:59:54 +0200
> Krzysztof Kozlowski <[email protected]> wrote:
>
>> On 02/04/2024 21:35, Andreas Kemnade wrote:
>>> As this chip was seen in several devices in the wild, add it.
>>>
>>> Signed-off-by: Andreas Kemnade <[email protected]>
>>> Suggested-by: Matti Vaittinen <[email protected]>
>>> ---
>>> Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml | 4 +++-
>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml b/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml
>>> index 0b62f854bf6b..e4df09e8961c 100644
>>> --- a/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml
>>> +++ b/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml
>>> @@ -17,7 +17,9 @@ description: |
>>>
>>> properties:
>>> compatible:
>>> - const: rohm,bd71828
>>> + enum:
>>> + - rohm,bd71828
>>> + - rohm,bd71879
>>
>> In your second commit you claim they are compatible, so why they are not
>> marked as such?
>>
> so you mean allowing
>
> compatible = "rohm,bd71828"
> and
> compatible = "rohm,bd71879", "rohm,bd71828"

Yes. If there are reasons against, please briefly mention them in commit
msg.

Best regards,
Krzysztof


2024-04-05 06:01:37

by Matti Vaittinen

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: mfd: Add ROHM BD71879

On 4/4/24 15:04, Krzysztof Kozlowski wrote:
> On 04/04/2024 12:30, Andreas Kemnade wrote:
>> On Thu, 4 Apr 2024 08:59:54 +0200
>> Krzysztof Kozlowski <[email protected]> wrote:
>>
>>> On 02/04/2024 21:35, Andreas Kemnade wrote:
>>>> As this chip was seen in several devices in the wild, add it.
>>>>
>>>> Signed-off-by: Andreas Kemnade <[email protected]>
>>>> Suggested-by: Matti Vaittinen <[email protected]>
>>>> ---
>>>> Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml | 4 +++-
>>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml b/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml
>>>> index 0b62f854bf6b..e4df09e8961c 100644
>>>> --- a/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml
>>>> +++ b/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml
>>>> @@ -17,7 +17,9 @@ description: |
>>>>
>>>> properties:
>>>> compatible:
>>>> - const: rohm,bd71828
>>>> + enum:
>>>> + - rohm,bd71828
>>>> + - rohm,bd71879
>>>
>>> In your second commit you claim they are compatible, so why they are not
>>> marked as such?
>>>
>> so you mean allowing
>>
>> compatible = "rohm,bd71828"
>> and
>> compatible = "rohm,bd71879", "rohm,bd71828"


This makes me slightly nervous. It wouldn't be the first time when I've
been told "they are similar", and later the reality has turned out to be
"they are similar, except...". Furthermore, even if these devices seem
similar to software (which is what the comment in the MFD driver is
referring to), it does not mean these devices are 100% electrically
compatible so that they could be used as a "drop-in" replacement to each
others. I wouldn't guarantee that.

Furthermore, my current understanding is that the BD71828 was a model
that was used for a limited purposes. So, maybe creating an dt-entry like:
compatible = "rohm,bd71879", "rohm,bd71828"

might not prove to be too useful. (But I'm not 100% certain on this).

> Yes. If there are reasons against, please briefly mention them in commit
> msg.

I would like to understand the rationale for allowing:
compatible = "rohm,bd71879", "rohm,bd71828".

Is the intention to:
1) allow boards which tell the software that "the hardware may be
bd71828 or bd71879", or
2) to tell a binding reader that these ICs are likely to be usable as
replacements to each others?
(Or, is there some other rationale beyond these?)

If it's 1), then I see limited sense in doing so, while I expect that
not so many bd71828 variants will be seen out there - and at least not
in that many different products. If it's the 2), then I wouldn't say we
have the facts to do this.
And, as always, if there is 3), 4), ... - I am keen to learn :)

Yours,
-- Matti

--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~


2024-04-05 06:31:55

by Andreas Kemnade

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: mfd: Add ROHM BD71879

On Fri, 5 Apr 2024 09:01:25 +0300
Matti Vaittinen <[email protected]> wrote:

> > Yes. If there are reasons against, please briefly mention them in commit
> > msg.
>
> I would like to understand the rationale for allowing:
> compatible = "rohm,bd71879", "rohm,bd71828".
>
> Is the intention to:
> 1) allow boards which tell the software that "the hardware may be
> bd71828 or bd71879", or
> 2) to tell a binding reader that these ICs are likely to be usable as
> replacements to each others?
> (Or, is there some other rationale beyond these?)

As far as I understand the second compatible can be a fallback for
software which does not know the bd71879 (remember the devicetree is
not only for linux). Especially if that software does not use the full
functionality of the device, like drivers in bootloaders do. E.g.
mmc drivers in bootloaders might not use high speed modes at all, so
differences in them do not matter.

Regards,
Andreas

2024-04-05 06:35:50

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: mfd: Add ROHM BD71879

On 05/04/2024 08:01, Matti Vaittinen wrote:
> On 4/4/24 15:04, Krzysztof Kozlowski wrote:
>> On 04/04/2024 12:30, Andreas Kemnade wrote:
>>> On Thu, 4 Apr 2024 08:59:54 +0200
>>> Krzysztof Kozlowski <[email protected]> wrote:
>>>
>>>> On 02/04/2024 21:35, Andreas Kemnade wrote:
>>>>> As this chip was seen in several devices in the wild, add it.
>>>>>
>>>>> Signed-off-by: Andreas Kemnade <[email protected]>
>>>>> Suggested-by: Matti Vaittinen <[email protected]>
>>>>> ---
>>>>> Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml | 4 +++-
>>>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml b/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml
>>>>> index 0b62f854bf6b..e4df09e8961c 100644
>>>>> --- a/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml
>>>>> +++ b/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml
>>>>> @@ -17,7 +17,9 @@ description: |
>>>>>
>>>>> properties:
>>>>> compatible:
>>>>> - const: rohm,bd71828
>>>>> + enum:
>>>>> + - rohm,bd71828
>>>>> + - rohm,bd71879
>>>>
>>>> In your second commit you claim they are compatible, so why they are not
>>>> marked as such?
>>>>
>>> so you mean allowing
>>>
>>> compatible = "rohm,bd71828"
>>> and
>>> compatible = "rohm,bd71879", "rohm,bd71828"
>
>
> This makes me slightly nervous. It wouldn't be the first time when I've
> been told "they are similar", and later the reality has turned out to be
> "they are similar, except...". Furthermore, even if these devices seem
> similar to software (which is what the comment in the MFD driver is
> referring to), it does not mean these devices are 100% electrically
> compatible so that they could be used as a "drop-in" replacement to each
> others. I wouldn't guarantee that.

compatibility does not mean that. It is only about software interface.

>
> Furthermore, my current understanding is that the BD71828 was a model
> that was used for a limited purposes. So, maybe creating an dt-entry like:
> compatible = "rohm,bd71879", "rohm,bd71828"
>
> might not prove to be too useful. (But I'm not 100% certain on this).
>
>> Yes. If there are reasons against, please briefly mention them in commit
>> msg.
>
> I would like to understand the rationale for allowing:
> compatible = "rohm,bd71879", "rohm,bd71828".
>
> Is the intention to:
> 1) allow boards which tell the software that "the hardware may be
> bd71828 or bd71879", or
> 2) to tell a binding reader that these ICs are likely to be usable as
> replacements to each others?
> (Or, is there some other rationale beyond these?)

None of these. Compatibility means that software can use fallback
compatible as matching mechanism and everything will work fine or with
reduced set of features, but still working.

>
> If it's 1), then I see limited sense in doing so, while I expect that
> not so many bd71828 variants will be seen out there - and at least not
> in that many different products. If it's the 2), then I wouldn't say we
> have the facts to do this.
> And, as always, if there is 3), 4), ... - I am keen to learn :)
>


Best regards,
Krzysztof


2024-04-05 06:36:14

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH 2/2] mfd: rohm-bd71828: Add software-compatible variant BD71879

On Tue, 02 Apr 2024, Andreas Kemnade wrote:

> Add the BD71879 PMIC which is software-compatible to the BD71828, so reuse
> the same device_type enum.
>
> Signed-off-by: Andreas Kemnade <[email protected]>
> Suggested-by: Matti Vaittinen <[email protected]>
> ---
> drivers/mfd/rohm-bd71828.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/mfd/rohm-bd71828.c b/drivers/mfd/rohm-bd71828.c
> index 4a1fa8a0d76a..f0b444690d4d 100644
> --- a/drivers/mfd/rohm-bd71828.c
> +++ b/drivers/mfd/rohm-bd71828.c
> @@ -585,6 +585,10 @@ static const struct of_device_id bd71828_of_match[] = {
> {
> .compatible = "rohm,bd71828",
> .data = (void *)ROHM_CHIP_TYPE_BD71828,
> + }, {
> + .compatible = "rohm,bd71879",
> + /* equivalent from a software point of view */

Nit: Sentences start with capital letters.

> + .data = (void *)ROHM_CHIP_TYPE_BD71828,
> }, {
> .compatible = "rohm,bd71815",
> .data = (void *)ROHM_CHIP_TYPE_BD71815,
> --
> 2.39.2
>

--
Lee Jones [李琼斯]

2024-04-05 06:58:41

by Matti Vaittinen

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: mfd: Add ROHM BD71879

On 4/5/24 09:35, Krzysztof Kozlowski wrote:
> On 05/04/2024 08:01, Matti Vaittinen wrote:
>> On 4/4/24 15:04, Krzysztof Kozlowski wrote:
>>> On 04/04/2024 12:30, Andreas Kemnade wrote:
>>>> On Thu, 4 Apr 2024 08:59:54 +0200
>>>> Krzysztof Kozlowski <[email protected]> wrote:
>>>>
>>>>> On 02/04/2024 21:35, Andreas Kemnade wrote:
>>>>>> As this chip was seen in several devices in the wild, add it.
>>>>>>
>>>>>> Signed-off-by: Andreas Kemnade <[email protected]>
>>>>>> Suggested-by: Matti Vaittinen <[email protected]>
>>>>>> ---
>>>>>> Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml | 4 +++-
>>>>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml b/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml
>>>>>> index 0b62f854bf6b..e4df09e8961c 100644
>>>>>> --- a/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml
>>>>>> +++ b/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml
>>>>>> @@ -17,7 +17,9 @@ description: |
>>>>>>

..

Compatibility means that software can use fallback
> compatible as matching mechanism and everything will work fine or with
> reduced set of features, but still working.

Thanks to both of you Andreas and Krzysztof!

Then I'm fine with this.

Yours,
-- Matti

--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~