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

[Krzysztof Kozlowski]

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