Changes since v1:
- Removed the gpio entries from arch/arm/mach-tegra/board-paz00.c
Hi,
I was waiting for the DT support from Chen-Yu before sending these,
but decided it makes no difference when I send them. I'm dropping the
con ID in the second patch because Dan noticed the warning, but of
course it will mean the "gpios" property can be used with DT.
The two last patches just add ACPI IDs for some new Baytrail based
boards.
Heikki Krogerus (5):
ARM: tegra: remove obsolete gpio entries
net: rfkill: gpio: remove unused and obsolete platform parameters
net: rfkill: gpio: remove gpio names
net: rfkill: gpio: add ACPI ID for GPS module on Lenove Miix2
net: rfkill: gpio: add ACPI IDs for a Broadcom bluetooth chip
arch/arm/mach-tegra/board-paz00.c | 4 +---
include/linux/rfkill-gpio.h | 10 ----------
net/rfkill/rfkill-gpio.c | 40 +++++++++------------------------------
3 files changed, 10 insertions(+), 44 deletions(-)
--
1.9.0.rc3
On Tue, 2014-02-25 at 14:22 +0200, Heikki Krogerus wrote:
> I was waiting for the DT support from Chen-Yu before sending these,
> but decided it makes no difference when I send them. I'm dropping the
> con ID in the second patch because Dan noticed the warning, but of
> course it will mean the "gpios" property can be used with DT.
>
> The two last patches just add ACPI IDs for some new Baytrail based
> boards.
Do these patches have any dependencies, or should I just add them to the
mac80211-next tree (since I don't keep a separate rfkill tree)?
Or does anyone else want to take them? In that case, it's all fine with
me, I'm really just nominally the rfkill maintainer and have no idea
about these patches... :)
johannes
On 02/25/2014 05:22 AM, Heikki Krogerus wrote:
> There is no use for them in this driver. This will fix a
> static checker warning..
>
> net/rfkill/rfkill-gpio.c:144 rfkill_gpio_probe()
> warn: variable dereferenced before check 'rfkill->name'
>
> This will also make sure that when DT support is added,
> "gpios" property can be used as no con_id labels are
> provided.
> diff --git a/net/rfkill/rfkill-gpio.c b/net/rfkill/rfkill-gpio.c
> - gpio = devm_gpiod_get_index(&pdev->dev, rfkill->reset_name, 0);
> + gpio = devm_gpiod_get_index(&pdev->dev, NULL, 0);
I think the correct fix here is to look up the GPIO by name rather than
by index, but simply hard-code the name rather than generating it with
sprintf(). Index lookups are hard to expand compatibly, but named-based
lookups scale much better.
In other words, I rather specifically disagree with using a plain
"gpios" property in any future DT binding, but would strongly prefer
e.g. reset-gpios/shutdown-gpios or gpios/gpio-names.
Still, I guess I don't object too much.
Hi Johannes,
>> I was waiting for the DT support from Chen-Yu before sending these,
>> but decided it makes no difference when I send them. I'm dropping the
>> con ID in the second patch because Dan noticed the warning, but of
>> course it will mean the "gpios" property can be used with DT.
>>
>> The two last patches just add ACPI IDs for some new Baytrail based
>> boards.
>
> Do these patches have any dependencies, or should I just add them to the
> mac80211-next tree (since I don't keep a separate rfkill tree)?
>
> Or does anyone else want to take them? In that case, it's all fine with
> me, I'm really just nominally the rfkill maintainer and have no idea
> about these patches... :)
someone has to take them. So I say go for it.
Regards
Marcel
On Lenovo Miix 2 8", BCM4752 is renamed LNV4752.
Signed-off-by: Heikki Krogerus <[email protected]>
---
net/rfkill/rfkill-gpio.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/rfkill/rfkill-gpio.c b/net/rfkill/rfkill-gpio.c
index ad5e354..ec38884 100644
--- a/net/rfkill/rfkill-gpio.c
+++ b/net/rfkill/rfkill-gpio.c
@@ -158,6 +158,7 @@ static int rfkill_gpio_remove(struct platform_device *pdev)
static const struct acpi_device_id rfkill_acpi_match[] = {
{ "BCM4752", RFKILL_TYPE_GPS },
+ { "LNV4752", RFKILL_TYPE_GPS },
{ },
};
--
1.9.0
After moving to description based gpio interface in
rfkill-gpio, the gpio numbers are not used any more.
Signed-off-by: Heikki Krogerus <[email protected]>
---
arch/arm/mach-tegra/board-paz00.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/arch/arm/mach-tegra/board-paz00.c b/arch/arm/mach-tegra/board-paz00.c
index e4dec9f..9c6029b 100644
--- a/arch/arm/mach-tegra/board-paz00.c
+++ b/arch/arm/mach-tegra/board-paz00.c
@@ -23,9 +23,7 @@
#include "board.h"
static struct rfkill_gpio_platform_data wifi_rfkill_platform_data = {
- .name = "wifi_rfkill",
- .reset_gpio = 25, /* PD1 */
- .shutdown_gpio = 85, /* PK5 */
+ .name = "wifi_rfkill",
.type = RFKILL_TYPE_WLAN,
};
--
1.9.0.rc3
This adds ACPI IDs for Broadcom bluetooth chip BCM43241 used
on various Baytrail based boards such as Lenovo Miix 2 and
Asus Transformer Book T100TA.
Signed-off-by: Heikki Krogerus <[email protected]>
---
net/rfkill/rfkill-gpio.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/net/rfkill/rfkill-gpio.c b/net/rfkill/rfkill-gpio.c
index ec38884..c4d7c6b 100644
--- a/net/rfkill/rfkill-gpio.c
+++ b/net/rfkill/rfkill-gpio.c
@@ -156,11 +156,16 @@ static int rfkill_gpio_remove(struct platform_device *pdev)
return 0;
}
+#ifdef CONFIG_ACPI
static const struct acpi_device_id rfkill_acpi_match[] = {
+ { "BCM2E1A", RFKILL_TYPE_BLUETOOTH },
+ { "BCM2E39", RFKILL_TYPE_BLUETOOTH },
+ { "BCM2E3D", RFKILL_TYPE_BLUETOOTH },
{ "BCM4752", RFKILL_TYPE_GPS },
{ "LNV4752", RFKILL_TYPE_GPS },
{ },
};
+#endif
static struct platform_driver rfkill_gpio_driver = {
.probe = rfkill_gpio_probe,
--
1.9.0.rc3
Hi Heikki,
Am Dienstag, 25. Februar 2014, 14:22:26 schrieb Heikki Krogerus:
> After moving to description based gpio interface in
> rfkill-gpio, the gpio numbers are not used any more.
>
> Signed-off-by: Heikki Krogerus <[email protected]>
Reviewed-by: Marc Dietrich <[email protected]>
Thanks for taking care!
Marc
> ---
> arch/arm/mach-tegra/board-paz00.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/arch/arm/mach-tegra/board-paz00.c
> b/arch/arm/mach-tegra/board-paz00.c index e4dec9f..9c6029b 100644
> --- a/arch/arm/mach-tegra/board-paz00.c
> +++ b/arch/arm/mach-tegra/board-paz00.c
> @@ -23,9 +23,7 @@
> #include "board.h"
>
> static struct rfkill_gpio_platform_data wifi_rfkill_platform_data = {
> - .name = "wifi_rfkill",
> - .reset_gpio = 25, /* PD1 */
> - .shutdown_gpio = 85, /* PK5 */
> + .name = "wifi_rfkill",
> .type = RFKILL_TYPE_WLAN,
> };
After upgrading to descriptor based gpios, the gpio numbers
are not used anymore. The power_clk_name and the platform
specific setup and close hooks are not used by anybody, and
we should not encourage use of such things, so removing them.
Signed-off-by: Heikki Krogerus <[email protected]>
---
include/linux/rfkill-gpio.h | 10 ----------
net/rfkill/rfkill-gpio.c | 15 +--------------
2 files changed, 1 insertion(+), 24 deletions(-)
diff --git a/include/linux/rfkill-gpio.h b/include/linux/rfkill-gpio.h
index 4d09f6e..20bcb55 100644
--- a/include/linux/rfkill-gpio.h
+++ b/include/linux/rfkill-gpio.h
@@ -27,21 +27,11 @@
* struct rfkill_gpio_platform_data - platform data for rfkill gpio device.
* for unused gpio's, the expected value is -1.
* @name: name for the gpio rf kill instance
- * @reset_gpio: GPIO which is used for reseting rfkill switch
- * @shutdown_gpio: GPIO which is used for shutdown of rfkill switch
- * @power_clk_name: [optional] name of clk to turn off while blocked
- * @gpio_runtime_close: clean up platform specific gpio configuration
- * @gpio_runtime_setup: set up platform specific gpio configuration
*/
struct rfkill_gpio_platform_data {
char *name;
- int reset_gpio;
- int shutdown_gpio;
- const char *power_clk_name;
enum rfkill_type type;
- void (*gpio_runtime_close)(struct platform_device *);
- int (*gpio_runtime_setup)(struct platform_device *);
};
#endif /* __RFKILL_GPIO_H */
diff --git a/net/rfkill/rfkill-gpio.c b/net/rfkill/rfkill-gpio.c
index bd2a5b9..0adda44 100644
--- a/net/rfkill/rfkill-gpio.c
+++ b/net/rfkill/rfkill-gpio.c
@@ -87,7 +87,6 @@ static int rfkill_gpio_probe(struct platform_device *pdev)
{
struct rfkill_gpio_platform_data *pdata = pdev->dev.platform_data;
struct rfkill_gpio_data *rfkill;
- const char *clk_name = NULL;
struct gpio_desc *gpio;
int ret;
int len;
@@ -101,7 +100,6 @@ static int rfkill_gpio_probe(struct platform_device *pdev)
if (ret)
return ret;
} else if (pdata) {
- clk_name = pdata->power_clk_name;
rfkill->name = pdata->name;
rfkill->type = pdata->type;
} else {
@@ -120,7 +118,7 @@ static int rfkill_gpio_probe(struct platform_device *pdev)
snprintf(rfkill->reset_name, len + 6 , "%s_reset", rfkill->name);
snprintf(rfkill->shutdown_name, len + 9, "%s_shutdown", rfkill->name);
- rfkill->clk = devm_clk_get(&pdev->dev, clk_name);
+ rfkill->clk = devm_clk_get(&pdev->dev, NULL);
gpio = devm_gpiod_get_index(&pdev->dev, rfkill->reset_name, 0);
if (!IS_ERR(gpio)) {
@@ -146,14 +144,6 @@ static int rfkill_gpio_probe(struct platform_device *pdev)
return -EINVAL;
}
- if (pdata && pdata->gpio_runtime_setup) {
- ret = pdata->gpio_runtime_setup(pdev);
- if (ret) {
- dev_err(&pdev->dev, "can't set up gpio\n");
- return ret;
- }
- }
-
rfkill->rfkill_dev = rfkill_alloc(rfkill->name, &pdev->dev,
rfkill->type, &rfkill_gpio_ops,
rfkill);
@@ -174,10 +164,7 @@ static int rfkill_gpio_probe(struct platform_device *pdev)
static int rfkill_gpio_remove(struct platform_device *pdev)
{
struct rfkill_gpio_data *rfkill = platform_get_drvdata(pdev);
- struct rfkill_gpio_platform_data *pdata = pdev->dev.platform_data;
- if (pdata && pdata->gpio_runtime_close)
- pdata->gpio_runtime_close(pdev);
rfkill_unregister(rfkill->rfkill_dev);
rfkill_destroy(rfkill->rfkill_dev);
--
1.9.0.rc3
On Lenovo Miix 2 8", BCM4752 is renamed LNV4752.
Signed-off-by: Heikki Krogerus <[email protected]>
---
net/rfkill/rfkill-gpio.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/rfkill/rfkill-gpio.c b/net/rfkill/rfkill-gpio.c
index ad5e354..ec38884 100644
--- a/net/rfkill/rfkill-gpio.c
+++ b/net/rfkill/rfkill-gpio.c
@@ -158,6 +158,7 @@ static int rfkill_gpio_remove(struct platform_device *pdev)
static const struct acpi_device_id rfkill_acpi_match[] = {
{ "BCM4752", RFKILL_TYPE_GPS },
+ { "LNV4752", RFKILL_TYPE_GPS },
{ },
};
--
1.9.0.rc3
On Tue, Feb 25, 2014 at 08:40:54PM +0400, Sergei Shtylyov wrote:
> On 25-02-2014 16:22, Heikki Krogerus wrote:
>
> >On Lenovo Miix 2 8", BCM4752 is renamed LNV4752.
>
> You have typoed in the subject, s/Lenove/Lenovo/.
True. I'll send a fixed version.
Thanks,
--
heikki
There is no use for them in this driver. This will fix a
static checker warning..
net/rfkill/rfkill-gpio.c:144 rfkill_gpio_probe()
warn: variable dereferenced before check 'rfkill->name'
This will also make sure that when DT support is added,
"gpios" property can be used as no con_id labels are
provided.
Reported-by: Dan Carpenter <[email protected]>
Signed-off-by: Heikki Krogerus <[email protected]>
---
net/rfkill/rfkill-gpio.c | 19 ++-----------------
1 file changed, 2 insertions(+), 17 deletions(-)
diff --git a/net/rfkill/rfkill-gpio.c b/net/rfkill/rfkill-gpio.c
index 0adda44..ad5e354 100644
--- a/net/rfkill/rfkill-gpio.c
+++ b/net/rfkill/rfkill-gpio.c
@@ -36,8 +36,6 @@ struct rfkill_gpio_data {
struct gpio_desc *shutdown_gpio;
struct rfkill *rfkill_dev;
- char *reset_name;
- char *shutdown_name;
struct clk *clk;
bool clk_enabled;
@@ -89,7 +87,6 @@ static int rfkill_gpio_probe(struct platform_device *pdev)
struct rfkill_gpio_data *rfkill;
struct gpio_desc *gpio;
int ret;
- int len;
rfkill = devm_kzalloc(&pdev->dev, sizeof(*rfkill), GFP_KERNEL);
if (!rfkill)
@@ -106,21 +103,9 @@ static int rfkill_gpio_probe(struct platform_device *pdev)
return -ENODEV;
}
- len = strlen(rfkill->name);
- rfkill->reset_name = devm_kzalloc(&pdev->dev, len + 7, GFP_KERNEL);
- if (!rfkill->reset_name)
- return -ENOMEM;
-
- rfkill->shutdown_name = devm_kzalloc(&pdev->dev, len + 10, GFP_KERNEL);
- if (!rfkill->shutdown_name)
- return -ENOMEM;
-
- snprintf(rfkill->reset_name, len + 6 , "%s_reset", rfkill->name);
- snprintf(rfkill->shutdown_name, len + 9, "%s_shutdown", rfkill->name);
-
rfkill->clk = devm_clk_get(&pdev->dev, NULL);
- gpio = devm_gpiod_get_index(&pdev->dev, rfkill->reset_name, 0);
+ gpio = devm_gpiod_get_index(&pdev->dev, NULL, 0);
if (!IS_ERR(gpio)) {
ret = gpiod_direction_output(gpio, 0);
if (ret)
@@ -128,7 +113,7 @@ static int rfkill_gpio_probe(struct platform_device *pdev)
rfkill->reset_gpio = gpio;
}
- gpio = devm_gpiod_get_index(&pdev->dev, rfkill->shutdown_name, 1);
+ gpio = devm_gpiod_get_index(&pdev->dev, NULL, 1);
if (!IS_ERR(gpio)) {
ret = gpiod_direction_output(gpio, 0);
if (ret)
--
1.9.0.rc3
On Tue, 2014-02-25 at 14:22 +0200, Heikki Krogerus wrote:
> After moving to description based gpio interface in
> rfkill-gpio, the gpio numbers are not used any more.
>
> Signed-off-by: Heikki Krogerus <[email protected]>
> ---
> arch/arm/mach-tegra/board-paz00.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
Hm, ok, I'm not sure I should take a tegra patch through the mac80211 ->
wireless -> networking trees. OTOH, I suppose that the second patch
depends on this. I can take it all I suppose, Thierry? Stephen?
johannes
On 02/25/2014 05:22 AM, Heikki Krogerus wrote:
> After moving to description based gpio interface in
> rfkill-gpio, the gpio numbers are not used any more.
Acked-by: Stephen Warren <[email protected]>
This shouldn't cause any conflicts since I really don't anticipate
changing board-paz00.c in this kernel cycle other than this patch.
If you want though, I can prepare a branch and signed tag in the Tegra
tree, containing this patch, and you can merge it. I'm fine either way.
Hello.
On 25-02-2014 16:22, Heikki Krogerus wrote:
> On Lenovo Miix 2 8", BCM4752 is renamed LNV4752.
You have typoed in the subject, s/Lenove/Lenovo/.
> Signed-off-by: Heikki Krogerus <[email protected]>
WBR, Sergei
On Wed, Feb 26, 2014 at 7:04 AM, Stephen Warren <[email protected]> wrote:
>> - gpio = devm_gpiod_get_index(&pdev->dev, rfkill->reset_name, 0);
>> + gpio = devm_gpiod_get_index(&pdev->dev, NULL, 0);
>
> I think the correct fix here is to look up the GPIO by name rather than
> by index, but simply hard-code the name rather than generating it with
> sprintf(). Index lookups are hard to expand compatibly, but named-based
> lookups scale much better.
>
> In other words, I rather specifically disagree with using a plain
> "gpios" property in any future DT binding, but would strongly prefer
> e.g. reset-gpios/shutdown-gpios or gpios/gpio-names.
If I understand the situation correctly it's like ACPI does not have named
GPIOs so keeping specifying this in DT GPIO bindings is counter-productive
to the work of abstracting the access to GPIO handlers so that drivers
need not know whether ACPI or DT is used for describing the hardware.
We could do worse. Like putting the GPIOs in a differently indexed order
in DT vs ACPI.
I have no strong opinion really, I just see that people doing DT and ACPI
HW descriptions need to cooperate if they want to share infrastructure
or we have to give up that pipe dream and let each HW description
method have its own unique probe()-runpath. Which would be the result
in this driver if we persist on using named GPIOs.
Yours,
Linus Walleij
On 03/04/2014 06:43 PM, Linus Walleij wrote:
> On Wed, Feb 26, 2014 at 7:04 AM, Stephen Warren <[email protected]> wrote:
>
>>> - gpio = devm_gpiod_get_index(&pdev->dev, rfkill->reset_name, 0);
>>> + gpio = devm_gpiod_get_index(&pdev->dev, NULL, 0);
>>
>> I think the correct fix here is to look up the GPIO by name rather than
>> by index, but simply hard-code the name rather than generating it with
>> sprintf(). Index lookups are hard to expand compatibly, but named-based
>> lookups scale much better.
>>
>> In other words, I rather specifically disagree with using a plain
>> "gpios" property in any future DT binding, but would strongly prefer
>> e.g. reset-gpios/shutdown-gpios or gpios/gpio-names.
>
> If I understand the situation correctly it's like ACPI does not have named
> GPIOs so keeping specifying this in DT GPIO bindings is counter-productive
> to the work of abstracting the access to GPIO handlers so that drivers
> need not know whether ACPI or DT is used for describing the hardware.
For devices that already have both ACPI and DT bindings, we can't
pretend they can be the same; they are already potentially different. We
simply need to parse DT and ACPI differently, since that's the sway
their bindings are defined.
For any devices that don't have both ACPI and DT bindings, I agree we
should certainly strive to make any new bindings aligned so we don't
have to deal with this for them.
However, we can't change the past.
On Fri, Mar 7, 2014 at 11:41 AM, Linus Walleij <[email protected]> wrote:
> On Wed, Mar 5, 2014 at 10:59 AM, Stephen Warren <[email protected]> wrote:
>> On 03/04/2014 07:37 PM, Linus Walleij wrote:
>>> On Wed, Mar 5, 2014 at 10:18 AM, Stephen Warren <[email protected]> wrote:
>>>> On 03/04/2014 06:43 PM, Linus Walleij wrote:
>>>
>>>>> If I understand the situation correctly it's like ACPI does not have named
>>>>> GPIOs so keeping specifying this in DT GPIO bindings is counter-productive
>>>>> to the work of abstracting the access to GPIO handlers so that drivers
>>>>> need not know whether ACPI or DT is used for describing the hardware.
>>>>
>>>> For devices that already have both ACPI and DT bindings, we can't
>>>> pretend they can be the same; they are already potentially different. We
>>>> simply need to parse DT and ACPI differently, since that's the sway
>>>> their bindings are defined.
>>>>
>>>> For any devices that don't have both ACPI and DT bindings, I agree we
>>>> should certainly strive to make any new bindings aligned so we don't
>>>> have to deal with this for them.
>>>>
>>>> However, we can't change the past.
>>>
>>> Yeah, right, so for this very driver there are no bindings defined (yet)
>>> and the only device tree I can find referencing it is the Tegra20-paz00
>>> and it just use gpios = <>;
>>>
>>> So in this case I think this patch is the right way forward, but I admit
>>> I'm really uncertain in the general case.
>>
>> If there are no bindings defined at all yet, then we can define both DT
>> and ACPI bindings to use name-based GPIOs. Index-based lookups aren't a
>> good way forward.
>
> After Mark clarifying that ACPI is going to have named GPIOs I'm
> totally aligned on this, so OK!
Glad to hear this, but is it possible to get rid of the index in current
drivers? Or change the behavior to name-based OR index-based lookups.
This might break any DTs that have multiple GPIOs defined under one
property though.
Cheers
ChenYu
On Wed, Mar 5, 2014 at 10:18 AM, Stephen Warren <[email protected]> wrote:
> On 03/04/2014 06:43 PM, Linus Walleij wrote:
>> If I understand the situation correctly it's like ACPI does not have named
>> GPIOs so keeping specifying this in DT GPIO bindings is counter-productive
>> to the work of abstracting the access to GPIO handlers so that drivers
>> need not know whether ACPI or DT is used for describing the hardware.
>
> For devices that already have both ACPI and DT bindings, we can't
> pretend they can be the same; they are already potentially different. We
> simply need to parse DT and ACPI differently, since that's the sway
> their bindings are defined.
>
> For any devices that don't have both ACPI and DT bindings, I agree we
> should certainly strive to make any new bindings aligned so we don't
> have to deal with this for them.
>
> However, we can't change the past.
Yeah, right, so for this very driver there are no bindings defined (yet)
and the only device tree I can find referencing it is the Tegra20-paz00
and it just use gpios = <>;
So in this case I think this patch is the right way forward, but I admit
I'm really uncertain in the general case.
Yours,
Linus Walleij
On 03/04/2014 07:37 PM, Linus Walleij wrote:
> On Wed, Mar 5, 2014 at 10:18 AM, Stephen Warren <[email protected]> wrote:
>> On 03/04/2014 06:43 PM, Linus Walleij wrote:
>
>>> If I understand the situation correctly it's like ACPI does not have named
>>> GPIOs so keeping specifying this in DT GPIO bindings is counter-productive
>>> to the work of abstracting the access to GPIO handlers so that drivers
>>> need not know whether ACPI or DT is used for describing the hardware.
>>
>> For devices that already have both ACPI and DT bindings, we can't
>> pretend they can be the same; they are already potentially different. We
>> simply need to parse DT and ACPI differently, since that's the sway
>> their bindings are defined.
>>
>> For any devices that don't have both ACPI and DT bindings, I agree we
>> should certainly strive to make any new bindings aligned so we don't
>> have to deal with this for them.
>>
>> However, we can't change the past.
>
> Yeah, right, so for this very driver there are no bindings defined (yet)
> and the only device tree I can find referencing it is the Tegra20-paz00
> and it just use gpios = <>;
>
> So in this case I think this patch is the right way forward, but I admit
> I'm really uncertain in the general case.
If there are no bindings defined at all yet, then we can define both DT
and ACPI bindings to use name-based GPIOs. Index-based lookups aren't a
good way forward.
On Wed, Mar 5, 2014 at 10:59 AM, Stephen Warren <[email protected]> wrote:
> On 03/04/2014 07:37 PM, Linus Walleij wrote:
>> On Wed, Mar 5, 2014 at 10:18 AM, Stephen Warren <[email protected]> wrote:
>>> On 03/04/2014 06:43 PM, Linus Walleij wrote:
>>
>>>> If I understand the situation correctly it's like ACPI does not have named
>>>> GPIOs so keeping specifying this in DT GPIO bindings is counter-productive
>>>> to the work of abstracting the access to GPIO handlers so that drivers
>>>> need not know whether ACPI or DT is used for describing the hardware.
>>>
>>> For devices that already have both ACPI and DT bindings, we can't
>>> pretend they can be the same; they are already potentially different. We
>>> simply need to parse DT and ACPI differently, since that's the sway
>>> their bindings are defined.
>>>
>>> For any devices that don't have both ACPI and DT bindings, I agree we
>>> should certainly strive to make any new bindings aligned so we don't
>>> have to deal with this for them.
>>>
>>> However, we can't change the past.
>>
>> Yeah, right, so for this very driver there are no bindings defined (yet)
>> and the only device tree I can find referencing it is the Tegra20-paz00
>> and it just use gpios = <>;
>>
>> So in this case I think this patch is the right way forward, but I admit
>> I'm really uncertain in the general case.
>
> If there are no bindings defined at all yet, then we can define both DT
> and ACPI bindings to use name-based GPIOs. Index-based lookups aren't a
> good way forward.
After Mark clarifying that ACPI is going to have named GPIOs I'm
totally aligned on this, so OK!
Yours,
Linus Walleij
On 03/06/2014 08:43 PM, Chen-Yu Tsai wrote:
> On Fri, Mar 7, 2014 at 11:41 AM, Linus Walleij <[email protected]> wrote:
>> On Wed, Mar 5, 2014 at 10:59 AM, Stephen Warren <[email protected]> wrote:
>>> On 03/04/2014 07:37 PM, Linus Walleij wrote:
>>>> On Wed, Mar 5, 2014 at 10:18 AM, Stephen Warren <[email protected]> wrote:
>>>>> On 03/04/2014 06:43 PM, Linus Walleij wrote:
>>>>
>>>>>> If I understand the situation correctly it's like ACPI does not have named
>>>>>> GPIOs so keeping specifying this in DT GPIO bindings is counter-productive
>>>>>> to the work of abstracting the access to GPIO handlers so that drivers
>>>>>> need not know whether ACPI or DT is used for describing the hardware.
>>>>>
>>>>> For devices that already have both ACPI and DT bindings, we can't
>>>>> pretend they can be the same; they are already potentially different. We
>>>>> simply need to parse DT and ACPI differently, since that's the sway
>>>>> their bindings are defined.
>>>>>
>>>>> For any devices that don't have both ACPI and DT bindings, I agree we
>>>>> should certainly strive to make any new bindings aligned so we don't
>>>>> have to deal with this for them.
>>>>>
>>>>> However, we can't change the past.
>>>>
>>>> Yeah, right, so for this very driver there are no bindings defined (yet)
>>>> and the only device tree I can find referencing it is the Tegra20-paz00
>>>> and it just use gpios = <>;
>>>>
>>>> So in this case I think this patch is the right way forward, but I admit
>>>> I'm really uncertain in the general case.
>>>
>>> If there are no bindings defined at all yet, then we can define both DT
>>> and ACPI bindings to use name-based GPIOs. Index-based lookups aren't a
>>> good way forward.
>>
>> After Mark clarifying that ACPI is going to have named GPIOs I'm
>> totally aligned on this, so OK!
>
> Glad to hear this, but is it possible to get rid of the index in current
> drivers? Or change the behavior to name-based OR index-based lookups.
> This might break any DTs that have multiple GPIOs defined under one
> property though.
For any bindings that are already defined to use index-based lookups, I
think we have to continue using them, for backwards-compatibility with
old DTs (and I assume old ACPI databases need the same thing).