On Friday 17 January 2014, Chen-Yu Tsai wrote: > On Sat, Jan 18, 2014 at 12:47 AM, Arnd Bergmann <arnd@xxxxxxxx> wrote: > > On Friday 17 January 2014, Chen-Yu Tsai wrote: > >> diff --git a/Documentation/devicetree/bindings/rfkill/rfkill-gpio.txt b/Documentation/devicetree/bindings/rfkill/rfkill-gpio.txt > >> new file mode 100644 > >> index 0000000..8a07ea4 > >> --- /dev/null > >> +++ b/Documentation/devicetree/bindings/rfkill/rfkill-gpio.txt > >> @@ -0,0 +1,26 @@ > >> +GPIO controlled RFKILL devices > >> + > >> +Required properties: > >> +- compatible : Must be "rfkill-gpio". > >> +- rfkill-name : Name of RFKILL device > >> +- rfkill-type : Type of RFKILL device: 1 for WiFi, 2 for BlueTooth > >> +- NAME_shutdown-gpios : GPIO phandle to shutdown control > >> + (phandle must be the second) > >> +- NAME_reset-gpios : GPIO phandle to reset control > >> + > >> +NAME must match the rfkill-name property. NAME_shutdown-gpios or > >> +NAME_reset-gpios, or both, must be defined. > >> + > > > > I don't understand this part. Why do you include the name in the > > gpios property, rather than just hardcoding the property strings > > to "shutdown-gpios" and "reset-gpios"? > > This quirk is a result of how gpiod_get_index implements device tree > lookup. You'll also notice that the shutdown GPIO must be the second > phandle, as the driver uses indexed lookup to support ACPI cases. > If con_id is given, it is prepended to "gpios" as the property string. > con_id is also used as the label passed to gpiod_request, which is > then shown in /sys/kernel/debug/gpio. The Linux implementation should not enforce an inferior DT binding. I think it would be better to change the code instead and make this work with a more sensible representation. > I can do a seperate lookup for the device tree case, with or without > fallback to platform lookup tables. Then the names can be "reset-gpio" > and "shutdown-gpio". Need to promote gpiod_request to non-static so > we can register usage of the gpio, to match non-dt code path. > > Personally I prefer adding a "label" parameter to gpiod_get_index, so > we can use a different name than con_id. con_id isn't used in the ACPI > case, and is optional in platform lookup case. However device tree > lookup is dependent on this. What I see is non-uniform behavior > between the three. In my opinion this is undesirable, but I haven't > come up with a good solution yet. (adding the gpio people here). I don't understand enough of the current API to make a good call here, but I agree that we should try to make it more uniform and do it in a way that allows simpler DT bindings for devices using it. > About the property string, is the plural form required, even though we > only want one? I would keep the plural form for consistency. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html