diff mbox series

[1/2] dt-bindings: net: Add rfkill-gpio binding

Message ID 20221221104803.1693874-1-p.zabel@pengutronix.de
State New
Headers show
Series [1/2] dt-bindings: net: Add rfkill-gpio binding | expand

Commit Message

Philipp Zabel Dec. 21, 2022, 10:48 a.m. UTC
Add a device tree binding document for GPIO controlled rfkill switches.
The name, type, shutdown-gpios and reset-gpios properties are the same
as defined for ACPI.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 .../devicetree/bindings/net/rfkill-gpio.yaml  | 60 +++++++++++++++++++
 1 file changed, 60 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/rfkill-gpio.yaml

Comments

Krzysztof Kozlowski Dec. 22, 2022, 10:32 a.m. UTC | #1
On 21/12/2022 16:36, Philipp Zabel wrote:
>>> +    $ref: /schemas/types.yaml#/definitions/string
>>> +    description: rfkill switch name, defaults to node name
>>> +
>>> +  type:
>>
>> Too generic. Property names should ideally have 1 type globally. I think 
>> 'type' is already in use. 'radio-type' instead?
> 
> These values correspond to the 'enum rfkill_type' in Linux UAPI, but I
> think in this context 'radio-type' would be better than 'rfkill-type'.

Do not map Linux driver to DT, but rather describe the actual hardware.

> 
>>> +    description: rfkill radio type
>>> +    enum:
>>> +      - wlan
>>> +      - bluetooth
>>> +      - ultrawideband
>>> +      - wimax
>>> +      - wwan
>>> +      - gps
>>> +      - fm
>>> +      - nfc
>>> +
>>> +  shutdown-gpios:
>>> +    maxItems: 1
>>> +
>>> +  reset-gpios:
>>> +    maxItems: 1
>>
>> I'm lost as to why there are 2 GPIOs.
> 
> I don't know either.  My assumption is that this is for devices that
> are radio silenced by just asserting their reset pin (for example GPS
> chips). The driver handles them the same.
> 
> I could remove reset-gpios and make shutdown-gpios required.
> 
>>> +
>>> +required:
>>> +  - compatible
>>> +  - type
>>> +
>>> +oneOf:
>>> +  - required:
>>> +      - shutdown-gpios
>>> +  - required:
>>> +      - reset-gpios
>>
>> But only 1 can be present? So just define 1 GPIO name.
> 
> The intent was that only one of them would be required.
> 
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> +  - |
>>> +    #include <dt-bindings/gpio/gpio.h>
>>> +
>>> +    rfkill-pcie-wlan {
>>
>> Node names should be generic.
> 
> What could be a generic name for this - is "rfkill" acceptable even
> though it is a Linux subsystem name? Or would "rf-kill-switch" be
> better?

rfkill

> 
> How should they be called if there are multiple of them?

The same as in all other cases (leds, gpios, regulators), so rfkill-1,
rfkill-2...


Best regards,
Krzysztof
Philipp Zabel Jan. 2, 2023, 5:40 p.m. UTC | #2
On Do, 2022-12-22 at 11:20 +0100, Krzysztof Kozlowski wrote:
> On 21/12/2022 11:48, Philipp Zabel wrote:
> > Add a device tree binding document for GPIO controlled rfkill switches.
> > The name, type, shutdown-gpios and reset-gpios properties are the same
> > as defined for ACPI.
> > 
> > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> 
> Please use scripts/get_maintainers.pl to get a list of necessary people
> and lists to CC.  It might happen, that command when run on an older
> kernel, gives you outdated entries.  Therefore please be sure you base
> your patches on recent Linux kernel.
> 
> You missed several maintainers. Resend.

Thank you for the review, I've sent a v2:

https://lore.kernel.org/all/20230102-rfkill-gpio-dt-v2-0-d1b83758c16d@pengutronix.de/T

[...]
> 
> 
> > +  shutdown-gpios:
> > +    maxItems: 1
> > +
> > +  reset-gpios:
> > +    maxItems: 1
> 
> 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.

I've dropped the reset-gpios property. The device I would like to
control with this is the 'wireless disable' pin of an M.2 connector.

regards
Philipp
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/net/rfkill-gpio.yaml b/Documentation/devicetree/bindings/net/rfkill-gpio.yaml
new file mode 100644
index 000000000000..6e62e6c96456
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/rfkill-gpio.yaml
@@ -0,0 +1,60 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/net/rfkill-gpio.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: GPIO controlled rfkill switch
+
+maintainers:
+  - Johannes Berg <johannes@sipsolutions.net>
+  - Philipp Zabel <p.zabel@pengutronix.de>
+
+properties:
+  compatible:
+    const: rfkill-gpio
+
+  name:
+    $ref: /schemas/types.yaml#/definitions/string
+    description: rfkill switch name, defaults to node name
+
+  type:
+    description: rfkill radio type
+    enum:
+      - wlan
+      - bluetooth
+      - ultrawideband
+      - wimax
+      - wwan
+      - gps
+      - fm
+      - nfc
+
+  shutdown-gpios:
+    maxItems: 1
+
+  reset-gpios:
+    maxItems: 1
+
+required:
+  - compatible
+  - type
+
+oneOf:
+  - required:
+      - shutdown-gpios
+  - required:
+      - reset-gpios
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    rfkill-pcie-wlan {
+        compatible = "rfkill-gpio";
+        name = "rfkill-pcie-wlan";
+        type = "wlan";
+        shutdown-gpios = <&gpio2 25 GPIO_ACTIVE_HIGH>;
+    };