On Mi, 2022-12-21 at 08:45 -0600, Rob Herring wrote: > On Wed, Dec 21, 2022 at 11:48:02AM +0100, 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@xxxxxxxxxxxxxx> > > --- > > .../devicetree/bindings/net/rfkill-gpio.yaml | 60 +++++++++++++++++++ > > 1 file changed, 60 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/net/rfkill-gpio.yaml > > > > 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@xxxxxxxxxxxxxxxx> > > + - Philipp Zabel <p.zabel@xxxxxxxxxxxxxx> > > + > > +properties: > > + compatible: > > + const: rfkill-gpio > > + > > + name: > > Did you test this? Something should complain, but maybe not. The problem > is 'name' is already a property in the unflattened DT (and old FDT > formats). Thank you. Maybe this was hidden by the fact that I set the name property to the same string as the node's name. > 'label' would be appropriate perhaps, but why do we care what the name > is? This is meant to be the identifier of the rfkill API object. It is the content of /sys/class/rfkill/rfkill0/name, and the 'ID' in the rfkill command line tool, that can be used to select a switch, in case a device has multiple radios of the same type. > > + $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'. > > + 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? How should they be called if there are multiple of them? > > + compatible = "rfkill-gpio"; > > + name = "rfkill-pcie-wlan"; > > + type = "wlan"; > > + shutdown-gpios = <&gpio2 25 GPIO_ACTIVE_HIGH>; > > + }; > > -- > > 2.30.2 regards Philipp