diff mbox series

[1/5] regulator: dt-bindings: Unify compatible

Message ID 20240507122158.3739291-2-n-francis@ti.com (mailing list archive)
State New, archived
Headers show
Series arm64: ti: Add TPS6287 nodes | expand

Commit Message

Neha Malcom Francis May 7, 2024, 12:21 p.m. UTC
TPS62870/1/2/3 devices have different output currents (6A/9A/12A/15A) of
the TPS6287x family. The I2C addresses are the same between them. There
is no need for different compatibles for each for these devices so drop
them and add a unified "ti,tps6287x" compatible.

Signed-off-by: Neha Malcom Francis <n-francis@ti.com>
---
 .../devicetree/bindings/regulator/ti,tps62870.yaml         | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

Comments

Rob Herring May 7, 2024, 9:11 p.m. UTC | #1
On Tue, May 07, 2024 at 05:51:54PM +0530, Neha Malcom Francis wrote:
> TPS62870/1/2/3 devices have different output currents (6A/9A/12A/15A) of
> the TPS6287x family. The I2C addresses are the same between them. There
> is no need for different compatibles for each for these devices so drop
> them and add a unified "ti,tps6287x" compatible.

And s/w will never need to know what the max output current is?

Same i2c address has no bearing. That's usually not even fixed for 1 
device.

> 
> Signed-off-by: Neha Malcom Francis <n-francis@ti.com>
> ---
>  .../devicetree/bindings/regulator/ti,tps62870.yaml         | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/regulator/ti,tps62870.yaml b/Documentation/devicetree/bindings/regulator/ti,tps62870.yaml
> index 386989544dac..2998773db990 100644
> --- a/Documentation/devicetree/bindings/regulator/ti,tps62870.yaml
> +++ b/Documentation/devicetree/bindings/regulator/ti,tps62870.yaml
> @@ -15,10 +15,7 @@ allOf:
>  properties:
>    compatible:
>      enum:
> -      - ti,tps62870
> -      - ti,tps62871
> -      - ti,tps62872
> -      - ti,tps62873
> +      - ti,tps6287x

You just broke the existing users.

Wildcards in compatible names are generally discouraged. Maybe if this 
was a new binding and had sufficient justification why we don't need to 
distinguish parts, but this is an ABI and we're stuck with them.

If you are doing this to support more versions, then feel free to use 
an existing string. It's just a unique identifier. You have 4 to choose 
from.

Rob
Neha Malcom Francis May 8, 2024, 2:32 a.m. UTC | #2
Hi Rob

On 08/05/24 02:41, Rob Herring wrote:
> On Tue, May 07, 2024 at 05:51:54PM +0530, Neha Malcom Francis wrote:
>> TPS62870/1/2/3 devices have different output currents (6A/9A/12A/15A) of
>> the TPS6287x family. The I2C addresses are the same between them. There
>> is no need for different compatibles for each for these devices so drop
>> them and add a unified "ti,tps6287x" compatible.
> 
> And s/w will never need to know what the max output current is?
> 

Not really, as per understanding from the hardware teams.

> Same i2c address has no bearing. That's usually not even fixed for 1
> device.
> 
>>
>> Signed-off-by: Neha Malcom Francis <n-francis@ti.com>
>> ---
>>   .../devicetree/bindings/regulator/ti,tps62870.yaml         | 7 ++-----
>>   1 file changed, 2 insertions(+), 5 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/regulator/ti,tps62870.yaml b/Documentation/devicetree/bindings/regulator/ti,tps62870.yaml
>> index 386989544dac..2998773db990 100644
>> --- a/Documentation/devicetree/bindings/regulator/ti,tps62870.yaml
>> +++ b/Documentation/devicetree/bindings/regulator/ti,tps62870.yaml
>> @@ -15,10 +15,7 @@ allOf:
>>   properties:
>>     compatible:
>>       enum:
>> -      - ti,tps62870
>> -      - ti,tps62871
>> -      - ti,tps62872
>> -      - ti,tps62873
>> +      - ti,tps6287x
> 
> You just broke the existing users.
> 
> Wildcards in compatible names are generally discouraged. Maybe if this
> was a new binding and had sufficient justification why we don't need to
> distinguish parts, but this is an ABI and we're stuck with them.
> 
> If you are doing this to support more versions, then feel free to use
> an existing string. It's just a unique identifier. You have 4 to choose
> from.

Thanks for the review, Rob! I should have known better than to remove 
compatibles, excuse the noise!

> 
> Rob
>
Mark Brown May 8, 2024, 12:10 p.m. UTC | #3
On Tue, May 07, 2024 at 04:11:12PM -0500, Rob Herring wrote:
> On Tue, May 07, 2024 at 05:51:54PM +0530, Neha Malcom Francis wrote:
> > TPS62870/1/2/3 devices have different output currents (6A/9A/12A/15A) of
> > the TPS6287x family. The I2C addresses are the same between them. There
> > is no need for different compatibles for each for these devices so drop
> > them and add a unified "ti,tps6287x" compatible.

> And s/w will never need to know what the max output current is?

Yes, this seems destructive of information for no gain - if anything it
makes things harder to use since you can't just use the part number and
instead have to know about the wildcard.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/regulator/ti,tps62870.yaml b/Documentation/devicetree/bindings/regulator/ti,tps62870.yaml
index 386989544dac..2998773db990 100644
--- a/Documentation/devicetree/bindings/regulator/ti,tps62870.yaml
+++ b/Documentation/devicetree/bindings/regulator/ti,tps62870.yaml
@@ -15,10 +15,7 @@  allOf:
 properties:
   compatible:
     enum:
-      - ti,tps62870
-      - ti,tps62871
-      - ti,tps62872
-      - ti,tps62873
+      - ti,tps6287x
 
   reg:
     maxItems: 1
@@ -40,7 +37,7 @@  examples:
       #size-cells = <0>;
 
       regulator@41 {
-        compatible = "ti,tps62873";
+        compatible = "ti,tps6287x";
         reg = <0x41>;
         regulator-name = "+0.75V";
         regulator-min-microvolt = <400000>;