| Message ID | 1385460350-17543-2-git-send-email-mika.westerberg@linux.intel.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On 11/26/2013 03:05 AM, Mika Westerberg wrote: > From: Heikki Krogerus <heikki.krogerus@linux.intel.com> > > This makes it possible to request the gpio descriptors in > rfkill_gpio driver regardless of the platform. V2 of the series did the following: static struct rfkill_gpio_platform_data wifi_rfkill_platform_data = { .name = "wifi_rfkill", - .reset_gpio = 25, /* PD1 */ - .shutdown_gpio = 85, /* PK5 */ .type = RFKILL_TYPE_WLAN, }; I assume that will happen sometime, along with removing those two fields from the struct definition. -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 11/27/2013 05:33 AM, Stephen Warren wrote: > On 11/26/2013 03:05 AM, Mika Westerberg wrote: >> From: Heikki Krogerus <heikki.krogerus@linux.intel.com> >> >> This makes it possible to request the gpio descriptors in >> rfkill_gpio driver regardless of the platform. > > V2 of the series did the following: > > static struct rfkill_gpio_platform_data wifi_rfkill_platform_data = { > .name = "wifi_rfkill", > - .reset_gpio = 25, /* PD1 */ > - .shutdown_gpio = 85, /* PK5 */ > .type = RFKILL_TYPE_WLAN, > }; > > I assume that will happen sometime, along with removing those two fields > from the struct definition. This needs to happen after patch 2 of this series, but would be nice to have indeed. Would suggest to do it in a separate patch if this series needs no more revision. Reviewed-by: Alexandre Courbot <acourbot@nvidia.com> -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 11/26/2013 5:05 AM, Mika Westerberg wrote: > From: Heikki Krogerus <heikki.krogerus@linux.intel.com> > > This makes it possible to request the gpio descriptors in > rfkill_gpio driver regardless of the platform. > > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> > Tested-by: Stephen Warren <swarren@nvidia.com> > --- > arch/arm/mach-tegra/board-paz00.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/arch/arm/mach-tegra/board-paz00.c b/arch/arm/mach-tegra/board-paz00.c > index 06f024070dab..a309795da665 100644 > --- a/arch/arm/mach-tegra/board-paz00.c > +++ b/arch/arm/mach-tegra/board-paz00.c > @@ -18,6 +18,7 @@ > */ > > #include <linux/platform_device.h> > +#include <linux/gpio/driver.h> > #include <linux/rfkill-gpio.h> > #include "board.h" > > @@ -36,7 +37,13 @@ static struct platform_device wifi_rfkill_device = { > }, > }; > > +static struct gpiod_lookup wifi_gpio_lookup[] = { > + GPIO_LOOKUP_IDX("tegra-gpio", 25, "rfkill_gpio", NULL, 0, 0), > + GPIO_LOOKUP_IDX("tegra-gpio", 85, "rfkill_gpio", NULL, 1, 0), > +}; I wouldn't think this table would match for the gpios as the driver currently is. From what I see, the driver calls into gpiod_get_index, which will try 1 of 3 ways of getting the gpios: of-enabled: of_find_gpio - which I believe wouldn't work for paz00, since rfkill doesn't support dt? acpi: acpi_find_gpio - I assume this does work, but I didn't dive into it gpiod lookup table: gpiod_find - I think this is the path we expect to be taken, given the addition of the lookup table here, but I don't think it would actually match. Looking at the code for gpiod_find, it seems like it would try to match the con_id, but would fail. Patch 2/6 is passing the reset_name and shutdown_name for the con_ids, which isn't what is registered in this table. Shouldn't it look more like this? +static struct gpiod_lookup wifi_gpio_lookup[] = { + GPIO_LOOKUP_IDX("tegra-gpio", 25, "rfkill_gpio_reset", NULL, 0, 0), + GPIO_LOOKUP_IDX("tegra-gpio", 85, "rfkill_gpio_shutdown", NULL, 1, 0), +}; Sorry if I am missing something... -rhyland
On Thu, Nov 28, 2013 at 1:47 AM, Rhyland Klein <rklein@nvidia.com> wrote: > On 11/26/2013 5:05 AM, Mika Westerberg wrote: >> From: Heikki Krogerus <heikki.krogerus@linux.intel.com> >> >> This makes it possible to request the gpio descriptors in >> rfkill_gpio driver regardless of the platform. >> >> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> >> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> >> Tested-by: Stephen Warren <swarren@nvidia.com> >> --- >> arch/arm/mach-tegra/board-paz00.c | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/arch/arm/mach-tegra/board-paz00.c b/arch/arm/mach-tegra/board-paz00.c >> index 06f024070dab..a309795da665 100644 >> --- a/arch/arm/mach-tegra/board-paz00.c >> +++ b/arch/arm/mach-tegra/board-paz00.c >> @@ -18,6 +18,7 @@ >> */ >> >> #include <linux/platform_device.h> >> +#include <linux/gpio/driver.h> >> #include <linux/rfkill-gpio.h> >> #include "board.h" >> >> @@ -36,7 +37,13 @@ static struct platform_device wifi_rfkill_device = { >> }, >> }; >> >> +static struct gpiod_lookup wifi_gpio_lookup[] = { >> + GPIO_LOOKUP_IDX("tegra-gpio", 25, "rfkill_gpio", NULL, 0, 0), >> + GPIO_LOOKUP_IDX("tegra-gpio", 85, "rfkill_gpio", NULL, 1, 0), >> +}; > > I wouldn't think this table would match for the gpios as the driver > currently is. From what I see, the driver calls into gpiod_get_index, > which will try 1 of 3 ways of getting the gpios: > > of-enabled: of_find_gpio > - which I believe wouldn't work for paz00, since rfkill > doesn't support dt? > acpi: acpi_find_gpio > - I assume this does work, but I didn't dive into it > gpiod lookup table: gpiod_find > - I think this is the path we expect to be taken, given the addition of > the lookup table here, but I don't think it would actually match. > Looking at the code for gpiod_find, it seems like it would try to match > the con_id, but would fail. Patch 2/6 is passing the reset_name and > shutdown_name for the con_ids, which isn't what is registered in this > table. > > Shouldn't it look more like this? > > +static struct gpiod_lookup wifi_gpio_lookup[] = { > + GPIO_LOOKUP_IDX("tegra-gpio", 25, "rfkill_gpio_reset", NULL, 0, 0), > + GPIO_LOOKUP_IDX("tegra-gpio", 85, "rfkill_gpio_shutdown", NULL, 1, 0), > +}; The original GPIO lookup table specifies the device name ("rfkill_gpio"), no con_id, and an index. gpiod_find() will ensure that the device names match, skip the con_id (as it is null in the table), and again require that the indexes are the same. So provided the instanciated device's dev_name() is "rfkill_gpio" (which it seems to be according to the driver - not sure if it could become "rfkill_gpio.0"), then I'd say the GPIOs will match. The con_id passed to gpiod_get() will only be used as a label for debugfs. I am not sure where the "rfkill_gpio_reset" you mention comes from? One could wonder why the names of the GPIO are not hardcoded in the driver instead of being defined as platform data, but that point could be improved in a future patch. It is true however that the platform GPIO lookup mechanism is confusing at best, on top of being inefficient (one big linked list). I have a patch in the pipe that should improve both points by making GPIO lookup tables defined per-device - don't hesitate to merge this series first if it works though. Alex. -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, Am Donnerstag, 28. November 2013, 11:47:34 schrieb Alexandre Courbot: > On Thu, Nov 28, 2013 at 1:47 AM, Rhyland Klein <rklein@nvidia.com> wrote: > > On 11/26/2013 5:05 AM, Mika Westerberg wrote: > >> From: Heikki Krogerus <heikki.krogerus@linux.intel.com> > >> > >> This makes it possible to request the gpio descriptors in > >> rfkill_gpio driver regardless of the platform. > >> > >> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> > >> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> > >> Tested-by: Stephen Warren <swarren@nvidia.com> > >> --- > >> > >> arch/arm/mach-tegra/board-paz00.c | 7 +++++++ > >> 1 file changed, 7 insertions(+) > >> > >> diff --git a/arch/arm/mach-tegra/board-paz00.c > >> b/arch/arm/mach-tegra/board-paz00.c index 06f024070dab..a309795da665 > >> 100644 > >> --- a/arch/arm/mach-tegra/board-paz00.c > >> +++ b/arch/arm/mach-tegra/board-paz00.c > >> @@ -18,6 +18,7 @@ > >> > >> */ > >> > >> #include <linux/platform_device.h> > >> > >> +#include <linux/gpio/driver.h> > >> > >> #include <linux/rfkill-gpio.h> > >> #include "board.h" > >> > >> @@ -36,7 +37,13 @@ static struct platform_device wifi_rfkill_device = { > >> > >> }, > >> > >> }; > >> > >> +static struct gpiod_lookup wifi_gpio_lookup[] = { > >> + GPIO_LOOKUP_IDX("tegra-gpio", 25, "rfkill_gpio", NULL, 0, 0), > >> + GPIO_LOOKUP_IDX("tegra-gpio", 85, "rfkill_gpio", NULL, 1, 0), > >> +}; > > > > I wouldn't think this table would match for the gpios as the driver > > currently is. From what I see, the driver calls into gpiod_get_index, > > which will try 1 of 3 ways of getting the gpios: > > > > of-enabled: of_find_gpio > > > > - which I believe wouldn't work for paz00, since rfkill > > > > doesn't support dt? > > > > acpi: acpi_find_gpio > > > > - I assume this does work, but I didn't dive into it > > > > gpiod lookup table: gpiod_find > > > > - I think this is the path we expect to be taken, given the > > addition of > > > > the lookup table here, but I don't think it would actually match. > > Looking at the code for gpiod_find, it seems like it would try to match > > the con_id, but would fail. Patch 2/6 is passing the reset_name and > > shutdown_name for the con_ids, which isn't what is registered in this > > table. > > > > Shouldn't it look more like this? > > > > +static struct gpiod_lookup wifi_gpio_lookup[] = { > > + GPIO_LOOKUP_IDX("tegra-gpio", 25, "rfkill_gpio_reset", NULL, 0, > > 0), > > + GPIO_LOOKUP_IDX("tegra-gpio", 85, "rfkill_gpio_shutdown", NULL, 1, > > 0), +}; > > The original GPIO lookup table specifies the device name > ("rfkill_gpio"), no con_id, and an index. gpiod_find() will ensure > that the device names match, skip the con_id (as it is null in the > table), and again require that the indexes are the same. So provided > the instanciated device's dev_name() is "rfkill_gpio" (which it seems > to be according to the driver - not sure if it could become > "rfkill_gpio.0"), then I'd say the GPIOs will match. The con_id passed > to gpiod_get() will only be used as a label for debugfs. I am not sure > where the "rfkill_gpio_reset" you mention comes from? > > One could wonder why the names of the GPIO are not hardcoded in the > driver instead of being defined as platform data, but that point could > be improved in a future patch. > > It is true however that the platform GPIO lookup mechanism is > confusing at best, on top of being inefficient (one big linked list). > I have a patch in the pipe that should improve both points by making > GPIO lookup tables defined per-device - don't hesitate to merge this > series first if it works though. I'll try to apply the patches and check on the actual hw. rfkill userspace command should be able to enable/disable the gpio. No idea how it finds the required gpio names. The real problem with the rfkill driver is that it does not support DT. A naive attempt to convert it was made some year ago, but got rejected because rfkill wasn't seen as isolated device which can be represented in the device- tree. Also it can not be added under some existing device node (e.g. the wifi driver) because those devices sit normally on an "enumeratable" bus (e.g. usb, pci), which is not listed in the device tree at all. This is why it still requires a board file and platform_data. I wish we could find a solution for this. Marc -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Nov 28, 2013 at 10:09:14AM +0100, Marc Dietrich wrote: > Hi, > > Am Donnerstag, 28. November 2013, 11:47:34 schrieb Alexandre Courbot: > > On Thu, Nov 28, 2013 at 1:47 AM, Rhyland Klein <rklein@nvidia.com> wrote: > > > On 11/26/2013 5:05 AM, Mika Westerberg wrote: > > >> From: Heikki Krogerus <heikki.krogerus@linux.intel.com> > > >> > > >> This makes it possible to request the gpio descriptors in > > >> rfkill_gpio driver regardless of the platform. > > >> > > >> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> > > >> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> > > >> Tested-by: Stephen Warren <swarren@nvidia.com> > > >> --- > > >> > > >> arch/arm/mach-tegra/board-paz00.c | 7 +++++++ > > >> 1 file changed, 7 insertions(+) > > >> > > >> diff --git a/arch/arm/mach-tegra/board-paz00.c > > >> b/arch/arm/mach-tegra/board-paz00.c index 06f024070dab..a309795da665 > > >> 100644 > > >> --- a/arch/arm/mach-tegra/board-paz00.c > > >> +++ b/arch/arm/mach-tegra/board-paz00.c > > >> @@ -18,6 +18,7 @@ > > >> > > >> */ > > >> > > >> #include <linux/platform_device.h> > > >> > > >> +#include <linux/gpio/driver.h> > > >> > > >> #include <linux/rfkill-gpio.h> > > >> #include "board.h" > > >> > > >> @@ -36,7 +37,13 @@ static struct platform_device wifi_rfkill_device = { > > >> > > >> }, > > >> > > >> }; > > >> > > >> +static struct gpiod_lookup wifi_gpio_lookup[] = { > > >> + GPIO_LOOKUP_IDX("tegra-gpio", 25, "rfkill_gpio", NULL, 0, 0), > > >> + GPIO_LOOKUP_IDX("tegra-gpio", 85, "rfkill_gpio", NULL, 1, 0), > > >> +}; > > > > > > I wouldn't think this table would match for the gpios as the driver > > > currently is. From what I see, the driver calls into gpiod_get_index, > > > which will try 1 of 3 ways of getting the gpios: > > > > > > of-enabled: of_find_gpio > > > > > > - which I believe wouldn't work for paz00, since rfkill > > > > > > doesn't support dt? > > > > > > acpi: acpi_find_gpio > > > > > > - I assume this does work, but I didn't dive into it > > > > > > gpiod lookup table: gpiod_find > > > > > > - I think this is the path we expect to be taken, given the > > > addition of > > > > > > the lookup table here, but I don't think it would actually match. > > > Looking at the code for gpiod_find, it seems like it would try to match > > > the con_id, but would fail. Patch 2/6 is passing the reset_name and > > > shutdown_name for the con_ids, which isn't what is registered in this > > > table. > > > > > > Shouldn't it look more like this? > > > > > > +static struct gpiod_lookup wifi_gpio_lookup[] = { > > > + GPIO_LOOKUP_IDX("tegra-gpio", 25, "rfkill_gpio_reset", NULL, 0, > > > 0), > > > + GPIO_LOOKUP_IDX("tegra-gpio", 85, "rfkill_gpio_shutdown", NULL, 1, > > > 0), +}; > > > > The original GPIO lookup table specifies the device name > > ("rfkill_gpio"), no con_id, and an index. gpiod_find() will ensure > > that the device names match, skip the con_id (as it is null in the > > table), and again require that the indexes are the same. So provided > > the instanciated device's dev_name() is "rfkill_gpio" (which it seems > > to be according to the driver - not sure if it could become > > "rfkill_gpio.0"), then I'd say the GPIOs will match. The con_id passed > > to gpiod_get() will only be used as a label for debugfs. I am not sure > > where the "rfkill_gpio_reset" you mention comes from? > > > > One could wonder why the names of the GPIO are not hardcoded in the > > driver instead of being defined as platform data, but that point could > > be improved in a future patch. > > > > It is true however that the platform GPIO lookup mechanism is > > confusing at best, on top of being inefficient (one big linked list). > > I have a patch in the pipe that should improve both points by making > > GPIO lookup tables defined per-device - don't hesitate to merge this > > series first if it works though. > > I'll try to apply the patches and check on the actual hw. rfkill userspace > command should be able to enable/disable the gpio. No idea how it finds the > required gpio names. > > The real problem with the rfkill driver is that it does not support DT. A > naive attempt to convert it was made some year ago, but got rejected because > rfkill wasn't seen as isolated device which can be represented in the device- > tree. Also it can not be added under some existing device node (e.g. the wifi > driver) because those devices sit normally on an "enumeratable" bus (e.g. usb, > pci), which is not listed in the device tree at all. This is why it still > requires a board file and platform_data. I wish we could find a solution for > this. There is a solution at least for PCI. You can list PCI devices within the device tree, which is really handy (required even) if for instance one of the PCI devices is an SPI or I2C controller, each providing a bus that cannot be probed. What you usually do is describe the PCI hierarchy at least up to the controller and then list slaves as child nodes. I'm not aware of anything similar for USB, but it should certainly be possible to come up with a standard binding for the USB bus. It has a topology that's similar enough to that of PCI so that the same general rules could be applied. If that's really the only thing that keeps rfkill from gaining DT support then it's something worth tackling in my opinion. Thierry
Am Donnerstag, 28. November 2013, 10:32:41 schrieb Thierry Reding: > On Thu, Nov 28, 2013 at 10:09:14AM +0100, Marc Dietrich wrote: > > The real problem with the rfkill driver is that it does not support DT. A > > naive attempt to convert it was made some year ago, but got rejected > > because rfkill wasn't seen as isolated device which can be represented in > > the device- tree. Also it can not be added under some existing device > > node (e.g. the wifi driver) because those devices sit normally on an > > "enumeratable" bus (e.g. usb, pci), which is not listed in the device > > tree at all. This is why it still requires a board file and > > platform_data. I wish we could find a solution for this. > > There is a solution at least for PCI. You can list PCI devices within > the device tree, which is really handy (required even) if for instance > one of the PCI devices is an SPI or I2C controller, each providing a bus > that cannot be probed. What you usually do is describe the PCI hierarchy > at least up to the controller and then list slaves as child nodes. > > I'm not aware of anything similar for USB, but it should certainly be > possible to come up with a standard binding for the USB bus. It has a > topology that's similar enough to that of PCI so that the same general > rules could be applied. > > If that's really the only thing that keeps rfkill from gaining DT > support then it's something worth tackling in my opinion. it's not so simple I fear. The wifi driver needs to learn about the rfkill "device". As mentioned above, it's not really a device so the question is what needs to be added and where? The wifi driver just polls his own gpio lines to check the status of rfkill. Be we want to modify the "other side", so maybe this isn't related to the wifi driver at all. It's more a "virtual rfkill device". No idea if something like this exists already in device tree. Marc -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Nov 28, 2013 at 11:20:19AM +0100, Marc Dietrich wrote: > Am Donnerstag, 28. November 2013, 10:32:41 schrieb Thierry Reding: > > On Thu, Nov 28, 2013 at 10:09:14AM +0100, Marc Dietrich wrote: > > > The real problem with the rfkill driver is that it does not support DT. A > > > naive attempt to convert it was made some year ago, but got rejected > > > because rfkill wasn't seen as isolated device which can be represented in > > > the device- tree. Also it can not be added under some existing device > > > node (e.g. the wifi driver) because those devices sit normally on an > > > "enumeratable" bus (e.g. usb, pci), which is not listed in the device > > > tree at all. This is why it still requires a board file and > > > platform_data. I wish we could find a solution for this. > > > > There is a solution at least for PCI. You can list PCI devices within > > the device tree, which is really handy (required even) if for instance > > one of the PCI devices is an SPI or I2C controller, each providing a bus > > that cannot be probed. What you usually do is describe the PCI hierarchy > > at least up to the controller and then list slaves as child nodes. > > > > I'm not aware of anything similar for USB, but it should certainly be > > possible to come up with a standard binding for the USB bus. It has a > > topology that's similar enough to that of PCI so that the same general > > rules could be applied. > > > > If that's really the only thing that keeps rfkill from gaining DT > > support then it's something worth tackling in my opinion. > > it's not so simple I fear. The wifi driver needs to learn about the rfkill > "device". Why does the WiFi driver need to know about it? You say below that the WiFi driver polls a separate set of GPIO lines (internal to the WiFi module I assume?). In that case rfkill is merely a way to export control of that functionality so that it can be used from some other part of the kernel or from userspace. That's very similar to things such as backlight control or fan control. Still we manage to describe those in DT as well. > As mentioned above, it's not really a device so the question is what > needs to be added and where? The wifi driver just polls his own gpio lines to > check the status of rfkill. Be we want to modify the "other side", so maybe > this isn't related to the wifi driver at all. It's more a "virtual rfkill > device". No idea if something like this exists already in device tree. But it's a part of some other device. Or rather, it's always associated with another device, right? So I don't see anything particularily wrong with something like this: usb-controller { /* USB hub */ usb@0,0 { ... /* USB device */ usb@1,0 { ... rfkill { shutdown-gpio = <&gpio ...>; reset-gpio = <&gpio ...>; }; ... }; ... }; }; You said that the main objection was that it needed to be represented as a "subdevice" of whatever device it controls. If the only reason is that we have no means to represent those devices because they are on a bus that can be enumerated and therefore can't be listed in DT, then my suggestion is to fix things so that we can describe those devices in DT. The goal is to get rid of board files, right? board-paz00.c is the only one left for Tegra, and rfkill is an actual hardware component, so there is no reason why it can't be described in DT. Thierry
Am Donnerstag, 28. November 2013, 12:06:37 schrieb Thierry Reding: > On Thu, Nov 28, 2013 at 11:20:19AM +0100, Marc Dietrich wrote: > > Am Donnerstag, 28. November 2013, 10:32:41 schrieb Thierry Reding: > > > On Thu, Nov 28, 2013 at 10:09:14AM +0100, Marc Dietrich wrote: > > > > The real problem with the rfkill driver is that it does not support > > > > DT. A > > > > naive attempt to convert it was made some year ago, but got rejected > > > > because rfkill wasn't seen as isolated device which can be represented > > > > in > > > > the device- tree. Also it can not be added under some existing device > > > > node (e.g. the wifi driver) because those devices sit normally on an > > > > "enumeratable" bus (e.g. usb, pci), which is not listed in the device > > > > tree at all. This is why it still requires a board file and > > > > platform_data. I wish we could find a solution for this. > > > > > > There is a solution at least for PCI. You can list PCI devices within > > > the device tree, which is really handy (required even) if for instance > > > one of the PCI devices is an SPI or I2C controller, each providing a bus > > > that cannot be probed. What you usually do is describe the PCI hierarchy > > > at least up to the controller and then list slaves as child nodes. > > > > > > I'm not aware of anything similar for USB, but it should certainly be > > > possible to come up with a standard binding for the USB bus. It has a > > > topology that's similar enough to that of PCI so that the same general > > > rules could be applied. > > > > > > If that's really the only thing that keeps rfkill from gaining DT > > > support then it's something worth tackling in my opinion. > > > > it's not so simple I fear. The wifi driver needs to learn about the rfkill > > "device". > > Why does the WiFi driver need to know about it? You say below that the > WiFi driver polls a separate set of GPIO lines (internal to the WiFi > module I assume?). In that case rfkill is merely a way to export control > of that functionality so that it can be used from some other part of the > kernel or from userspace. You are right. We just need some device to bind this driver to. It doesn't need to be the wifi driver. > That's very similar to things such as backlight control or fan control. > Still we manage to describe those in DT as well. Yes, rfkill is just an interface for userspace to able to control the gpio. E.g. backlight of medcom-wide seems to be related to the pwm controller, but is not a subnode of it. Instead it is a device of its own without parent. Hence, we may include rfkill in a similar, "free-standing" node. But this approch was rejected in the past. Maybe this changed now. > > As mentioned above, it's not really a device so the question is what > > needs to be added and where? The wifi driver just polls his own gpio lines > > to check the status of rfkill. Be we want to modify the "other side", so > > maybe this isn't related to the wifi driver at all. It's more a "virtual > > rfkill device". No idea if something like this exists already in device > > tree. > But it's a part of some other device. Or rather, it's always associated > with another device, right? So I don't see anything particularily wrong > with something like this: > > usb-controller { > /* USB hub */ > usb@0,0 { > ... > > /* USB device */ > usb@1,0 { > ... > > rfkill { > shutdown-gpio = <&gpio ...>; > reset-gpio = <&gpio ...>; > }; > > ... > }; > > ... > }; > }; > > You said that the main objection was that it needed to be represented as > a "subdevice" of whatever device it controls. If the only reason is that > we have no means to represent those devices because they are on a bus > that can be enumerated and therefore can't be listed in DT, then my > suggestion is to fix things so that we can describe those devices in DT. > > The goal is to get rid of board files, right? board-paz00.c is the only > one left for Tegra, and rfkill is an actual hardware component, so there > is no reason why it can't be described in DT. Thinking a bit more about it, rfkill is neither a hw block nor a function of the wifi driver, so I guess it cannot be added to the usb controller (or a usb device). Anyway, I think this discussion deserves a new mail thread, but I don't have enough background information to start one. We can bring this topic back when all turkeys who deserve it, are dead. Marc -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Nov 28, 2013 at 01:54:10PM +0100, Marc Dietrich wrote: [...] > Yes, rfkill is just an interface for userspace to able to control the gpio. > E.g. backlight of medcom-wide seems to be related to the pwm controller, but > is not a subnode of it. Instead it is a device of its own without parent. > Hence, we may include rfkill in a similar, "free-standing" node. But this > approch was rejected in the past. Maybe this changed now. Indeed. I think we really do need ways to represent this kind of device. If we can't then how are we supposed to get rid of board files? > Thinking a bit more about it, rfkill is neither a hw block nor a function of > the wifi driver, so I guess it cannot be added to the usb controller (or a usb > device). I guess it depends a bit. Some rfkill-type devices may control more than just WiFi (bluetooth, NFC, ...). If it really only controls WiFi, I think it would still be reasonable to represent it as a child of the WiFi device. It is, after all, a mechanism to control the WiFi hardware. Alternatively I suppose we wouldn't really need to have an explicit node. The WiFi driver could simply instantiate an rfkill device based on its own properties. Thierry
diff --git a/arch/arm/mach-tegra/board-paz00.c b/arch/arm/mach-tegra/board-paz00.c index 06f024070dab..a309795da665 100644 --- a/arch/arm/mach-tegra/board-paz00.c +++ b/arch/arm/mach-tegra/board-paz00.c @@ -18,6 +18,7 @@ */ #include <linux/platform_device.h> +#include <linux/gpio/driver.h> #include <linux/rfkill-gpio.h> #include "board.h" @@ -36,7 +37,13 @@ static struct platform_device wifi_rfkill_device = { }, }; +static struct gpiod_lookup wifi_gpio_lookup[] = { + GPIO_LOOKUP_IDX("tegra-gpio", 25, "rfkill_gpio", NULL, 0, 0), + GPIO_LOOKUP_IDX("tegra-gpio", 85, "rfkill_gpio", NULL, 1, 0), +}; + void __init tegra_paz00_wifikill_init(void) { + gpiod_add_table(wifi_gpio_lookup, ARRAY_SIZE(wifi_gpio_lookup)); platform_device_register(&wifi_rfkill_device); }