[PATCH 3/3] dt-bindings: pse-pd: ti,tps23881: Add TPS23881B

[PATCH 3/3] dt-bindings: pse-pd: ti,tps23881: Add TPS23881B

[Thomas Wismer]

From: Thomas Wismer <thomas.wismer@scs.ch>

Add the TPS23881B I2C power sourcing equipment controller to the list of
supported devices.

Signed-off-by: Thomas Wismer <thomas.wismer@scs.ch>
---
 Documentation/devicetree/bindings/net/pse-pd/ti,tps23881.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/net/pse-pd/ti,tps23881.yaml b/Documentation/devicetree/bindings/net/pse-pd/ti,tps23881.yaml
index bb1ee3398655..0b3803f647b7 100644
--- a/Documentation/devicetree/bindings/net/pse-pd/ti,tps23881.yaml
+++ b/Documentation/devicetree/bindings/net/pse-pd/ti,tps23881.yaml
@@ -16,6 +16,7 @@ properties:
   compatible:
     enum:
       - ti,tps23881
+      - ti,tps23881b
 
   reg:
     maxItems: 1
-- 
2.43.0

Re: [PATCH 3/3] dt-bindings: pse-pd: ti,tps23881: Add TPS23881B

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1146 bytes --]

On Sat, Oct 04, 2025 at 08:03:53PM +0200, Thomas Wismer wrote:
> From: Thomas Wismer <thomas.wismer@scs.ch>
> 
> Add the TPS23881B I2C power sourcing equipment controller to the list of
> supported devices.

Missing an explanation for why a fallback compatible is not suitable
here. Seems like it is, if the only difference is that the firmware is
not required to be refreshed, provided that loading the non-B firmware
on a B device would not be problematic.

> 
> Signed-off-by: Thomas Wismer <thomas.wismer@scs.ch>
> ---
>  Documentation/devicetree/bindings/net/pse-pd/ti,tps23881.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/pse-pd/ti,tps23881.yaml b/Documentation/devicetree/bindings/net/pse-pd/ti,tps23881.yaml
> index bb1ee3398655..0b3803f647b7 100644
> --- a/Documentation/devicetree/bindings/net/pse-pd/ti,tps23881.yaml
> +++ b/Documentation/devicetree/bindings/net/pse-pd/ti,tps23881.yaml
> @@ -16,6 +16,7 @@ properties:
>    compatible:
>      enum:
>        - ti,tps23881
> +      - ti,tps23881b
>  
>    reg:
>      maxItems: 1
> -- 
> 2.43.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 3/3] dt-bindings: pse-pd: ti,tps23881: Add TPS23881B

[Thomas Wismer]

Am Tue, 7 Oct 2025 21:40:03 +0100
schrieb Conor Dooley <conor@kernel.org>:

> On Sat, Oct 04, 2025 at 08:03:53PM +0200, Thomas Wismer wrote:
> > From: Thomas Wismer <thomas.wismer@scs.ch>
> > 
> > Add the TPS23881B I2C power sourcing equipment controller to the
> > list of supported devices.  
> 
> Missing an explanation for why a fallback compatible is not suitable
> here. Seems like it is, if the only difference is that the firmware is
> not required to be refreshed, provided that loading the non-B firmware
> on a B device would not be problematic.

Loading the non-B firmware on a B device is indeed problematic. I'll
append the following paragraph to the patch when reposting it after
the current merge window has closed.

Falling back to the TPS23881 predecessor device is not suitable as firmware
loading needs to handled differently by the driver. The TPS23881 and
TPS23881B devices require different firmware. Trying to load the TPS23881
firmware on a TPS23881B device fails and must therefore be omitted.

> > 
> > Signed-off-by: Thomas Wismer <thomas.wismer@scs.ch>
> > ---
> >  Documentation/devicetree/bindings/net/pse-pd/ti,tps23881.yaml | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/net/pse-pd/ti,tps23881.yaml
> > b/Documentation/devicetree/bindings/net/pse-pd/ti,tps23881.yaml
> > index bb1ee3398655..0b3803f647b7 100644 ---
> > a/Documentation/devicetree/bindings/net/pse-pd/ti,tps23881.yaml +++
> > b/Documentation/devicetree/bindings/net/pse-pd/ti,tps23881.yaml @@
> > -16,6 +16,7 @@ properties: compatible: enum:
> >        - ti,tps23881
> > +      - ti,tps23881b
> >  
> >    reg:
> >      maxItems: 1
> > -- 
> > 2.43.0
> >   

Re: [PATCH 3/3] dt-bindings: pse-pd: ti,tps23881: Add TPS23881B

[Andrew Lunn]

On Wed, Oct 08, 2025 at 01:52:43PM +0200, Thomas Wismer wrote:
> Am Tue, 7 Oct 2025 21:40:03 +0100
> schrieb Conor Dooley <conor@kernel.org>:
> 
> > On Sat, Oct 04, 2025 at 08:03:53PM +0200, Thomas Wismer wrote:
> > > From: Thomas Wismer <thomas.wismer@scs.ch>
> > > 
> > > Add the TPS23881B I2C power sourcing equipment controller to the
> > > list of supported devices.  
> > 
> > Missing an explanation for why a fallback compatible is not suitable
> > here. Seems like it is, if the only difference is that the firmware is
> > not required to be refreshed, provided that loading the non-B firmware
> > on a B device would not be problematic.
> 
> Loading the non-B firmware on a B device is indeed problematic. I'll
> append the following paragraph to the patch when reposting it after
> the current merge window has closed.

Is it possible to ask the device what it is?

If you can, maybe you don't even need a new compatible, just load the
appropriate firmware depending on what the device says it is.

	Andrew

Re: [PATCH 3/3] dt-bindings: pse-pd: ti,tps23881: Add TPS23881B

[Thomas Wismer]

Am Wed, 8 Oct 2025 14:38:52 +0200
schrieb Andrew Lunn <andrew@lunn.ch>:

> On Wed, Oct 08, 2025 at 01:52:43PM +0200, Thomas Wismer wrote:
> > Am Tue, 7 Oct 2025 21:40:03 +0100
> > schrieb Conor Dooley <conor@kernel.org>:
> >   
> > > On Sat, Oct 04, 2025 at 08:03:53PM +0200, Thomas Wismer wrote:  
> > > > From: Thomas Wismer <thomas.wismer@scs.ch>
> > > > 
> > > > Add the TPS23881B I2C power sourcing equipment controller to the
> > > > list of supported devices.    
> > > 
> > > Missing an explanation for why a fallback compatible is not
> > > suitable here. Seems like it is, if the only difference is that
> > > the firmware is not required to be refreshed, provided that
> > > loading the non-B firmware on a B device would not be
> > > problematic.  
> > 
> > Loading the non-B firmware on a B device is indeed problematic. I'll
> > append the following paragraph to the patch when reposting it after
> > the current merge window has closed.  
> 
> Is it possible to ask the device what it is?

Yes, the devices allow the silicon revision to be read, which would
enable a driver to correctly handle the case distinctions.

> If you can, maybe you don't even need a new compatible, just load the
> appropriate firmware depending on what the device says it is.

When adapting the driver, I also considered an auto-detection mechanism.
However, it felt safer to rely on the devicetree information than reading
a silicon revision register, which has a totally different meaning on
some other device. I have therefore decided to make the driver behaviour
solely dependent on the devicetree information and to use the silicon
revision only as a sanity check (as already implemented in the driver).
Is there any best practice when to use auto-detection with I2C devices?

Regardless of whether the driver queries the silicon revision, the B
device declaration would look somehow strange to me with a driver having
one single compatible, i.e. compatible = "ti,tps23881b", "ti,tps23881".
The first one specifically names the hardware, the fallback is actually
the name of its predecessor, which is strictly speaking not 100%
compatible but required to have the driver loaded.

Re: [PATCH 3/3] dt-bindings: pse-pd: ti,tps23881: Add TPS23881B

[Andrew Lunn]

> When adapting the driver, I also considered an auto-detection mechanism.
> However, it felt safer to rely on the devicetree information than reading
> a silicon revision register, which has a totally different meaning on
> some other device. I have therefore decided to make the driver behaviour
> solely dependent on the devicetree information and to use the silicon
> revision only as a sanity check (as already implemented in the driver).

So if the silicon and the DT disagree, you get -ENODEV or similar?
That is what i would recommend, so that broken DT blobs get found by
the developer.

> Is there any best practice when to use auto-detection with I2C devices?

Not really. There are devices/drivers where the compatible is just
used to indicate where to find the ID register in the hardware,
nothing else. The ID register is then used by the driver to do the
right thing, we trust the silicon to describe itself. But things like
PHY devices have the ID in a well known location, so we actually don't
require a compatible, but if one is given, we use that instead of the
ID found in the silicon. So the exact opposite.

> Regardless of whether the driver queries the silicon revision, the B
> device declaration would look somehow strange to me with a driver having
> one single compatible, i.e. compatible = "ti,tps23881b", "ti,tps23881".
> The first one specifically names the hardware, the fallback is actually
> the name of its predecessor, which is strictly speaking not 100%
> compatible but required to have the driver loaded.

If it is not compatible, a fallback will not actually work, don't list
a fallback.

	Andrew

Re: [PATCH 3/3] dt-bindings: pse-pd: ti,tps23881: Add TPS23881B

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2558 bytes --]

On Thu, Oct 09, 2025 at 11:43:04PM +0200, Andrew Lunn wrote:
> > When adapting the driver, I also considered an auto-detection mechanism.
> > However, it felt safer to rely on the devicetree information than reading
> > a silicon revision register, which has a totally different meaning on
> > some other device. I have therefore decided to make the driver behaviour
> > solely dependent on the devicetree information and to use the silicon
> > revision only as a sanity check (as already implemented in the driver).
> 
> So if the silicon and the DT disagree, you get -ENODEV or similar?
> That is what i would recommend, so that broken DT blobs get found by
> the developer.

I'm personally not a big fan of this kind of thing, as it prevents using
fallbacks for new devices when done strictly. I only really like it
being done this way if the driver does not produce errors for unknown
part numbers, only if (using this case as an example) a b device is
labeled as a non-b, or vice-versa. IOW, if the driver doesn't recognise
the ID, believe what's in DT.

> > Is there any best practice when to use auto-detection with I2C devices?
> 
> Not really. There are devices/drivers where the compatible is just
> used to indicate where to find the ID register in the hardware,
> nothing else. The ID register is then used by the driver to do the
> right thing, we trust the silicon to describe itself. But things like
> PHY devices have the ID in a well known location, so we actually don't
> require a compatible, but if one is given, we use that instead of the
> ID found in the silicon. So the exact opposite.
> 
> > Regardless of whether the driver queries the silicon revision, the B
> > device declaration would look somehow strange to me with a driver having
> > one single compatible, i.e. compatible = "ti,tps23881b", "ti,tps23881".
> > The first one specifically names the hardware, the fallback is actually
> > the name of its predecessor, which is strictly speaking not 100%
> > compatible but required to have the driver loaded.
> 
> If it is not compatible, a fallback will not actually work, don't list
> a fallback.

Yeah, seconded. I think my original mail about this was maybe a bit
confusingly worded, where I was envisaging a world where a driver that
encountered a b device could load the firmware for the non-b device, and
it would just be a redundant operation. A fallback would be suitable,
but obviously not ideal then. Since that isn't permitted, using a
fallback here does not make sense.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 3/3] dt-bindings: pse-pd: ti,tps23881: Add TPS23881B

[Andrew Lunn]

> > So if the silicon and the DT disagree, you get -ENODEV or similar?
> > That is what i would recommend, so that broken DT blobs get found by
> > the developer.
> 
> I'm personally not a big fan of this kind of thing, as it prevents using
> fallbacks for new devices when done strictly. I only really like it
> being done this way if the driver does not produce errors for unknown
> part numbers, only if (using this case as an example) a b device is
> labeled as a non-b, or vice-versa. IOW, if the driver doesn't recognise
> the ID, believe what's in DT.

The issue we have seen in the past when not strictly checking the
compatible against the hardware, is that a number of DT blob ship with
the wrong compatible. Then sometime later you find you need to key
some feature off the compatible, but you cannot without breaking all
those devices which have the wrong compatible.

In order to be able to use the compatible, it has to be correct.

   Andrew

Re: [PATCH 3/3] dt-bindings: pse-pd: ti,tps23881: Add TPS23881B

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 944 bytes --]

On Sat, Oct 04, 2025 at 08:03:53PM +0200, Thomas Wismer wrote:
> From: Thomas Wismer <thomas.wismer@scs.ch>
> 
> Add the TPS23881B I2C power sourcing equipment controller to the list of
> supported devices.
> 
> Signed-off-by: Thomas Wismer <thomas.wismer@scs.ch>

Acked-by: Conor Dooley <conor.dooley@microchip.com>

> ---
>  Documentation/devicetree/bindings/net/pse-pd/ti,tps23881.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/pse-pd/ti,tps23881.yaml b/Documentation/devicetree/bindings/net/pse-pd/ti,tps23881.yaml
> index bb1ee3398655..0b3803f647b7 100644
> --- a/Documentation/devicetree/bindings/net/pse-pd/ti,tps23881.yaml
> +++ b/Documentation/devicetree/bindings/net/pse-pd/ti,tps23881.yaml
> @@ -16,6 +16,7 @@ properties:
>    compatible:
>      enum:
>        - ti,tps23881
> +      - ti,tps23881b
>  
>    reg:
>      maxItems: 1
> -- 
> 2.43.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]