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

On 21/12/2022 16:36, Philipp Zabel wrote:

quoted

quoted

+    $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.

quoted

quoted

+    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.

quoted

quoted

+
+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.

quoted

quoted

+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