.../devicetree/bindings/regulator/fcs,fan53555.yaml | 5 ----- 1 file changed, 5 deletions(-)
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
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.
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 >
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!
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!
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...
© 2016 - 2026 Red Hat, Inc.