linux-kernel - Re: [PATCH v4 2/6] gpio: re-add of_node_to

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAA+hA=SLSk9wMtiFiarVR0t+T6KHSTQkYwPk99Kj75RYFP205g@mail.gmail.com>
Date:	Sat, 26 May 2012 09:15:09 -0700
From:	Dong Aisheng <dongas86@...il.com>
To:	Grant Likely <grant.likely@...retlab.ca>
Cc:	Dong Aisheng <b29396@...escale.com>, linux-kernel@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org, linus.walleij@...ricsson.com,
	swarren@...dotorg.org, devicetree-discuss@...ts.ozlabs.org,
	rob.herring@...xeda.com
Subject: Re: [PATCH v4 2/6] gpio: re-add of_node_to_gpiochip function

On Sat, May 26, 2012 at 8:01 AM, Grant Likely <grant.likely@...retlab.ca> wrote:
> On Fri, 25 May 2012 21:36:16 +0800, Dong Aisheng <b29396@...escale.com> wrote:
>> From: Dong Aisheng <dong.aisheng@...aro.org>
>>
>> Signed-off-by: Dong Aisheng <dong.aisheng@...aro.org>
>
> Nack.  Several problems here.  First and foremost there isn't any
> description of *why* this change is needed or when it was removed.
Sorry i sent it too fast to eager to see people's comments on the main
issue of patch 6.
It should have description on what you pointed out.
It's needed by the patch 6 in this series that it wants to find gpiochip from
node then calculate the gpio number from device tree in a standard way.

> Any patch beyond the most trivial change needs to have a commit
> message the describes it.
>
> Second, of_node_to_gpiochip() is no longer a valid API because gpiolib
> now allows multiple gpiochips to use the same device tree node.  See
> commit 3d0f7cf0f "gpio: Adjust of_xlate API to support multiple GPIO
> chips" to see a description of why this was changed.
>
I noticed that of_node_to_gpiochip() is removed by that patch.
But i did not realized that this API was broken before.
After looking a bit more on the sample code of an banked gpio,
https://lkml.org/lkml/2012/5/18/51
it seems besides the gpio node, we also depend on controller specific
gpio .of_xlate function
to find the correct gpiochip in the same bank for the same node.
So it's correct that of_node_to_gpiochip() is broken now.

I may change it if we finally still find this API is needed.

Thanks for the review.

Regards
Dong Aisheng

>> ---
>>  drivers/gpio/gpiolib-of.c |   11 +++++++++++
>>  include/linux/of_gpio.h   |    5 +++++
>>  2 files changed, 16 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
>> index 8389d4a..b8010a9 100644
>> --- a/drivers/gpio/gpiolib-of.c
>> +++ b/drivers/gpio/gpiolib-of.c
>> @@ -234,3 +234,14 @@ void of_gpiochip_remove(struct gpio_chip *chip)
>>       if (chip->of_node)
>>               of_node_put(chip->of_node);
>>  }
>> +
>> +/* Private function for resolving node pointer to gpio_chip */
>> +static int of_gpiochip_is_match(struct gpio_chip *chip, void *data)
>> +{
>> +     return chip->of_node == data;
>> +}
>> +
>> +struct gpio_chip *of_node_to_gpiochip(struct device_node *np)
>> +{
>> +     return gpiochip_find(np, of_gpiochip_is_match);
>> +}
>> diff --git a/include/linux/of_gpio.h b/include/linux/of_gpio.h
>> index c454f57..880783b 100644
>> --- a/include/linux/of_gpio.h
>> +++ b/include/linux/of_gpio.h
>> @@ -61,6 +61,7 @@ extern void of_gpiochip_remove(struct gpio_chip *gc);
>>  extern int of_gpio_simple_xlate(struct gpio_chip *gc,
>>                               const struct of_phandle_args *gpiospec,
>>                               u32 *flags);
>> +extern struct gpio_chip *of_node_to_gpiochip(struct device_node *np);
>>
>>  #else /* CONFIG_OF_GPIO */
>>
>> @@ -84,6 +85,10 @@ static inline int of_gpio_simple_xlate(struct gpio_chip *gc,
>>       return -ENOSYS;
>>  }
>>
>> +static struct gpio_chip *of_node_to_gpiochip(struct device_node *np)
>> +{
>> +     return NULL;
>> +}
>>  static inline void of_gpiochip_add(struct gpio_chip *gc) { }
>>  static inline void of_gpiochip_remove(struct gpio_chip *gc) { }
>>
>> --
>> 1.7.0.4
>>
>>
>
> --
> Grant Likely, B.Sc, P.Eng.
> Secret Lab Technologies, Ltd.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/