| Message ID | 20240319-rfkill-reset-gpio-binding-v1-1-a0e3f1767c87@solid-run.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | dt-bindings: net: rfkill-gpio: add reset-gpio property | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On 19/03/2024 11:13, Josua Mayer wrote: > rfkill-gpio driver supports management of two gpios: reset, shutdown. > Reset seems to have been missed when bindings were added. No, you do not add properties just because driver supports them. Bindings are for hardware, not for drivers. You now revert all the comments from v1, without actually addressing them. NAK. Or provide rationale why my previous comments, from adding the binding, should be reversed/ignored/overruled. Best regards, Krzysztof
Am 19.03.24 um 11:25 schrieb Krzysztof Kozlowski: > On 19/03/2024 11:13, Josua Mayer wrote: >> rfkill-gpio driver supports management of two gpios: reset, shutdown. >> Reset seems to have been missed when bindings were added. > No, you do not add properties just because driver supports them. > Bindings are for hardware, not for drivers. > > You now revert all the comments from v1, without actually addressing them. "Reset of rfkill? It seems entire binding is a workaround of missing reset in your device. I don't think this is suitable for binding." > NAK. Ack. I provide rationale below, but ultimately agree with your NAK. > Or provide rationale why my previous comments, from adding the > binding, should be reversed/ignored/overruled. As mentioned by Philipp, reset signal might be used to radio-silence a device in case more appropriate signals are not available or wired. M.2 connector has two interesting signals that fit surprisingly well to the rfkill-gpio driver's signal names: FULL_CARD_POWER_OFF# and RESET#. If for some reason both signals are under software control, and a specific radio card is known to be connected, then it would seem correct to describe both reset and shutdown gpios. However your remark ("It seems entire binding is a workaround of missing reset in your device") still stands: When we have a standard m.2 connector, users might connect any compatible card, and we know cards are often composite devices utilizing multiple buses. After thinking this through I agree that combining reset- and shutdown-gpio in rfkill is a workaround to a different problem: We are unable to describe control signals shared by multiple components from different buses, i.e. usb, i2c, pci-e, sata. Perhaps an m.2 connector should be its own entity, with various gpios and buses linked to it. Such a structural change is currently beyond my capabilities. sincerely Josua Mayer
diff --git a/Documentation/devicetree/bindings/net/rfkill-gpio.yaml b/Documentation/devicetree/bindings/net/rfkill-gpio.yaml index 9630c8466fac..d01cefef6115 100644 --- a/Documentation/devicetree/bindings/net/rfkill-gpio.yaml +++ b/Documentation/devicetree/bindings/net/rfkill-gpio.yaml @@ -29,6 +29,9 @@ properties: - wlan - wwan + reset-gpios: + maxItems: 1 + shutdown-gpios: maxItems: 1
rfkill-gpio driver supports management of two gpios: reset, shutdown. Reset seems to have been missed when bindings were added. Add binding for the supported reset-gpios property. Signed-off-by: Josua Mayer <josua@solid-run.com> --- Documentation/devicetree/bindings/net/rfkill-gpio.yaml | 3 +++ 1 file changed, 3 insertions(+) --- base-commit: e8f897f4afef0031fe618a8e94127a0934896aba change-id: 20240319-rfkill-reset-gpio-binding-5e9bfefb5f76 Sincerely,