public inbox for devicetree@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] dt-bindings: regulator: fcs,fan53555: Remove vsel-gpios property
@ 2026-04-27 11:54 Alexander Shiyan
  2026-04-27 19:04 ` Conor Dooley
  2026-04-28  1:00 ` Mark Brown
  0 siblings, 2 replies; 6+ messages in thread
From: Alexander Shiyan @ 2026-04-27 11:54 UTC (permalink / raw)
  To: devicetree
  Cc: linux-kernel, Liam Girdwood, Mark Brown, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner,
	Alexander Shiyan

The vsel-gpios property is currently documented in the binding but
is not used by the driver. The FAN53555 family of regulators supports
two voltage selector registers (VSEL0/VSEL1), and the selection between
them is intended to be controlled by an external hardware pin (VSEL).
However, the driver does not support dynamic toggling of this pin via
a GPIO, it only uses the fcs,suspend-voltage-selector property to
statically assign which register is used for runtime voltage and which
for suspend voltage.
Remove the vsel-gpios property from the binding to prevent incorrect DT
usage and to reflect the actual hardware description supported by the
driver.

Signed-off-by: Alexander Shiyan <eagle.alexander923@gmail.com>
---
 .../devicetree/bindings/regulator/fcs,fan53555.yaml          | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/regulator/fcs,fan53555.yaml b/Documentation/devicetree/bindings/regulator/fcs,fan53555.yaml
index 69bae90fc4b2..7f3b74ccf8db 100644
--- a/Documentation/devicetree/bindings/regulator/fcs,fan53555.yaml
+++ b/Documentation/devicetree/bindings/regulator/fcs,fan53555.yaml
@@ -43,11 +43,6 @@ properties:
   vin-supply:
     description: Supply for the vin pin
 
-  vsel-gpios:
-    description: Voltage Select. When this pin is LOW, VOUT is set by the
-      VSEL0 register. When this pin is HIGH, VOUT is set by the VSEL1 register.
-    maxItems: 1
-
 required:
   - compatible
   - reg
-- 
2.52.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] dt-bindings: regulator: fcs,fan53555: Remove vsel-gpios property
  2026-04-27 11:54 [PATCH] dt-bindings: regulator: fcs,fan53555: Remove vsel-gpios property Alexander Shiyan
@ 2026-04-27 19:04 ` Conor Dooley
  2026-04-27 19:09   ` Alexander Shiyan
  2026-04-28  1:00 ` Mark Brown
  1 sibling, 1 reply; 6+ messages in thread
From: Conor Dooley @ 2026-04-27 19:04 UTC (permalink / raw)
  To: Alexander Shiyan
  Cc: devicetree, linux-kernel, Liam Girdwood, Mark Brown, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner

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

On Mon, Apr 27, 2026 at 02:54:43PM +0300, Alexander Shiyan wrote:
> The vsel-gpios property is currently documented in the binding but
> is not used by the driver. The FAN53555 family of regulators supports
> two voltage selector registers (VSEL0/VSEL1), and the selection between
> them is intended to be controlled by an external hardware pin (VSEL).
> However, the driver does not support dynamic toggling of this pin via
> a GPIO, it only uses the fcs,suspend-voltage-selector property to
> statically assign which register is used for runtime voltage and which
> for suspend voltage.
> Remove the vsel-gpios property from the binding to prevent incorrect DT
> usage and to reflect the actual hardware description supported by the
> driver.

From the wording/justification here, I disagree with this patch. The
binding should document what the hardware can do, not what the driver
can.

Maybe instead you should make fcs,suspend-voltage-selector mutually
exclusive with vsel-gpios?


Cheers,
Conor.
pw-bot: changes-requested
> 
> Signed-off-by: Alexander Shiyan <eagle.alexander923@gmail.com>
> ---
>  .../devicetree/bindings/regulator/fcs,fan53555.yaml          | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/regulator/fcs,fan53555.yaml b/Documentation/devicetree/bindings/regulator/fcs,fan53555.yaml
> index 69bae90fc4b2..7f3b74ccf8db 100644
> --- a/Documentation/devicetree/bindings/regulator/fcs,fan53555.yaml
> +++ b/Documentation/devicetree/bindings/regulator/fcs,fan53555.yaml
> @@ -43,11 +43,6 @@ properties:
>    vin-supply:
>      description: Supply for the vin pin
>  
> -  vsel-gpios:
> -    description: Voltage Select. When this pin is LOW, VOUT is set by the
> -      VSEL0 register. When this pin is HIGH, VOUT is set by the VSEL1 register.
> -    maxItems: 1
> -
>  required:
>    - compatible
>    - reg
> -- 
> 2.52.0
> 

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] dt-bindings: regulator: fcs,fan53555: Remove vsel-gpios property
  2026-04-27 19:04 ` Conor Dooley
@ 2026-04-27 19:09   ` Alexander Shiyan
  2026-04-27 19:59     ` Conor Dooley
  0 siblings, 1 reply; 6+ messages in thread
From: Alexander Shiyan @ 2026-04-27 19:09 UTC (permalink / raw)
  To: Conor Dooley
  Cc: devicetree, linux-kernel, Liam Girdwood, Mark Brown, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner

Hello Comor.

> On Mon, Apr 27, 2026 at 02:54:43PM +0300, Alexander Shiyan wrote:
> > The vsel-gpios property is currently documented in the binding but
> > is not used by the driver. The FAN53555 family of regulators supports
> > two voltage selector registers (VSEL0/VSEL1), and the selection between
> > them is intended to be controlled by an external hardware pin (VSEL).
> > However, the driver does not support dynamic toggling of this pin via
> > a GPIO, it only uses the fcs,suspend-voltage-selector property to
> > statically assign which register is used for runtime voltage and which
> > for suspend voltage.
> > Remove the vsel-gpios property from the binding to prevent incorrect DT
> > usage and to reflect the actual hardware description supported by the
> > driver.
>
> From the wording/justification here, I disagree with this patch. The
> binding should document what the hardware can do, not what the driver
> can.
>
> Maybe instead you should make fcs,suspend-voltage-selector mutually
> exclusive with vsel-gpios?

The main problem here is that this feature (vsel-gpios) was never
implemented in the driver. So, the patch consists solely of removing a
non-existent property.

Thanks!

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] dt-bindings: regulator: fcs,fan53555: Remove vsel-gpios property
  2026-04-27 19:09   ` Alexander Shiyan
@ 2026-04-27 19:59     ` Conor Dooley
  2026-04-28  0:54       ` Mark Brown
  0 siblings, 1 reply; 6+ messages in thread
From: Conor Dooley @ 2026-04-27 19:59 UTC (permalink / raw)
  To: Alexander Shiyan
  Cc: devicetree, linux-kernel, Liam Girdwood, Mark Brown, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner

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

On Mon, Apr 27, 2026 at 10:09:53PM +0300, Alexander Shiyan wrote:
> Hello Comor.
> 
> > On Mon, Apr 27, 2026 at 02:54:43PM +0300, Alexander Shiyan wrote:
> > > The vsel-gpios property is currently documented in the binding but
> > > is not used by the driver. The FAN53555 family of regulators supports
> > > two voltage selector registers (VSEL0/VSEL1), and the selection between
> > > them is intended to be controlled by an external hardware pin (VSEL).
> > > However, the driver does not support dynamic toggling of this pin via
> > > a GPIO, it only uses the fcs,suspend-voltage-selector property to
> > > statically assign which register is used for runtime voltage and which
> > > for suspend voltage.
> > > Remove the vsel-gpios property from the binding to prevent incorrect DT
> > > usage and to reflect the actual hardware description supported by the
> > > driver.
> >
> > From the wording/justification here, I disagree with this patch. The
> > binding should document what the hardware can do, not what the driver
> > can.
> >
> > Maybe instead you should make fcs,suspend-voltage-selector mutually
> > exclusive with vsel-gpios?
> 
> The main problem here is that this feature (vsel-gpios) was never
> implemented in the driver. So, the patch consists solely of removing a
> non-existent property.

I'm not disputing that, but if the functionality is supported by the
hardware the existence of the property is fine. Bindings should be
complete, even if the driver in linux doesn't use something.

Rob's 2023 patch that added the property even says "Add the undocumented
'vsel-gpios' property used to control the VSEL pin.", so having it was
an intentional decision by the main binding maintainer!

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] dt-bindings: regulator: fcs,fan53555: Remove vsel-gpios property
  2026-04-27 19:59     ` Conor Dooley
@ 2026-04-28  0:54       ` Mark Brown
  0 siblings, 0 replies; 6+ messages in thread
From: Mark Brown @ 2026-04-28  0:54 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Alexander Shiyan, devicetree, linux-kernel, Liam Girdwood,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner

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

On Mon, Apr 27, 2026 at 08:59:01PM +0100, Conor Dooley wrote:

> Rob's 2023 patch that added the property even says "Add the undocumented
> 'vsel-gpios' property used to control the VSEL pin.", so having it was
> an intentional decision by the main binding maintainer!

Also sounds like the property was used by the code at some point in the
past...

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] dt-bindings: regulator: fcs,fan53555: Remove vsel-gpios property
  2026-04-27 11:54 [PATCH] dt-bindings: regulator: fcs,fan53555: Remove vsel-gpios property Alexander Shiyan
  2026-04-27 19:04 ` Conor Dooley
@ 2026-04-28  1:00 ` Mark Brown
  1 sibling, 0 replies; 6+ messages in thread
From: Mark Brown @ 2026-04-28  1:00 UTC (permalink / raw)
  To: Alexander Shiyan
  Cc: devicetree, linux-kernel, Liam Girdwood, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner

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

On Mon, Apr 27, 2026 at 02:54:43PM +0300, Alexander Shiyan wrote:

> -  vsel-gpios:
> -    description: Voltage Select. When this pin is LOW, VOUT is set by the
> -      VSEL0 register. When this pin is HIGH, VOUT is set by the VSEL1 register.
> -    maxItems: 1

In addition to Conor's concerns have you checked for existing usage?

Please submit patches using subject lines reflecting the style for the
subsystem, this makes it easier for people to identify relevant patches.
Look at what existing commits in the area you're changing are doing and
make sure your subject lines visually resemble what they're doing.
There's no need to resubmit to fix this alone.

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-04-28  1:00 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-27 11:54 [PATCH] dt-bindings: regulator: fcs,fan53555: Remove vsel-gpios property Alexander Shiyan
2026-04-27 19:04 ` Conor Dooley
2026-04-27 19:09   ` Alexander Shiyan
2026-04-27 19:59     ` Conor Dooley
2026-04-28  0:54       ` Mark Brown
2026-04-28  1:00 ` Mark Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox