Hello,
On Wed, Feb 13, 2013 at 04:03:00PM +0900, Alexandre Courbot wrote:
> From: Alexandre Courbot <[email protected]>
>
> Constify descriptor parameter of gpiod_* functions for those that
> should obviously not modify it. This includes value or direction get,
> cansleep, and IRQ number query.
>
> Signed-off-by: Alexandre Courbot <[email protected]>
This patch is applied as def634338d3ffb32fbe9b0a2d70cc24ef909cd4f since
v3.9-rc2.
> drivers/gpio/gpiolib.c | 29 ++++++++++++++++-------------
> 1 file changed, 16 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 8a2cf9c..ad6df6b 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -207,7 +208,7 @@ static int gpiochip_find_base(int ngpio)
> }
>
> /* caller ensures gpio is valid and requested, chip->get_direction may sleep */
> -static int gpiod_get_direction(struct gpio_desc *desc)
> +static int gpiod_get_direction(const struct gpio_desc *desc)
> {
> struct gpio_chip *chip;
> unsigned offset;
> @@ -223,11 +224,13 @@ static int gpiod_get_direction(struct gpio_desc *desc)
> if (status > 0) {
> /* GPIOF_DIR_IN, or other positive */
> status = 1;
> - clear_bit(FLAG_IS_OUT, &desc->flags);
> + /* FLAG_IS_OUT is just a cache of the result of get_direction(),
> + * so it does not affect constness per se */
> + clear_bit(FLAG_IS_OUT, &((struct gpio_desc *)desc)->flags);
I think this is worse than not having gpiod_get_direction take a const
descriptor.
I don't know an example for this particular case where this could break,
but usually casting away a const is bad and can break in various ways.
Maybe even drop the public function gpiod_get_direction completely.
As of next-20141114 there are only two users of this function (apart
from drivers/gpio/gpiolib*) and both are wrong (as discussed in
http://thread.gmane.org/gmane.linux.serial/16808/focus=4783).
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
On Mon, Nov 17, 2014 at 6:09 PM, Uwe Kleine-König
<[email protected]> wrote:
> Hello,
>
> On Wed, Feb 13, 2013 at 04:03:00PM +0900, Alexandre Courbot wrote:
>> From: Alexandre Courbot <[email protected]>
>>
>> Constify descriptor parameter of gpiod_* functions for those that
>> should obviously not modify it. This includes value or direction get,
>> cansleep, and IRQ number query.
>>
>> Signed-off-by: Alexandre Courbot <[email protected]>
> This patch is applied as def634338d3ffb32fbe9b0a2d70cc24ef909cd4f since
> v3.9-rc2.
>
>> drivers/gpio/gpiolib.c | 29 ++++++++++++++++-------------
>> 1 file changed, 16 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
>> index 8a2cf9c..ad6df6b 100644
>> --- a/drivers/gpio/gpiolib.c
>> +++ b/drivers/gpio/gpiolib.c
>> @@ -207,7 +208,7 @@ static int gpiochip_find_base(int ngpio)
>> }
>>
>> /* caller ensures gpio is valid and requested, chip->get_direction may sleep */
>> -static int gpiod_get_direction(struct gpio_desc *desc)
>> +static int gpiod_get_direction(const struct gpio_desc *desc)
>> {
>> struct gpio_chip *chip;
>> unsigned offset;
>> @@ -223,11 +224,13 @@ static int gpiod_get_direction(struct gpio_desc *desc)
>> if (status > 0) {
>> /* GPIOF_DIR_IN, or other positive */
>> status = 1;
>> - clear_bit(FLAG_IS_OUT, &desc->flags);
>> + /* FLAG_IS_OUT is just a cache of the result of get_direction(),
>> + * so it does not affect constness per se */
>> + clear_bit(FLAG_IS_OUT, &((struct gpio_desc *)desc)->flags);
> I think this is worse than not having gpiod_get_direction take a const
> descriptor.
Yeah, this kind of sucks but ultimately gpiod_get_direction() is
without side-effect to the caller and the const parameter helps
highlighting that fact.
> I don't know an example for this particular case where this could break,
> but usually casting away a const is bad and can break in various ways.
>
> Maybe even drop the public function gpiod_get_direction completely.
> As of next-20141114 there are only two users of this function (apart
> from drivers/gpio/gpiolib*) and both are wrong (as discussed in
> http://thread.gmane.org/gmane.linux.serial/16808/focus=4783).
Or maybe gpiod_direction_*() should register the last set direction to
be used by gpiod_get_direction() if the controller does not implement
the get_direction op? On the other hand callers can also track the
direction themselves (since they presumably set it), so this function
is really only useful for users who want to read the *hardware* state
of a GPIO. Which might still be a useful feature?
Hello Alexandre,
On Wed, Nov 19, 2014 at 05:33:42PM +0900, Alexandre Courbot wrote:
> On Mon, Nov 17, 2014 at 6:09 PM, Uwe Kleine-K?nig
> <[email protected]> wrote:
> > Hello,
> >
> > On Wed, Feb 13, 2013 at 04:03:00PM +0900, Alexandre Courbot wrote:
> >> From: Alexandre Courbot <[email protected]>
> >>
> >> Constify descriptor parameter of gpiod_* functions for those that
> >> should obviously not modify it. This includes value or direction get,
> >> cansleep, and IRQ number query.
> >>
> >> Signed-off-by: Alexandre Courbot <[email protected]>
> > This patch is applied as def634338d3ffb32fbe9b0a2d70cc24ef909cd4f since
> > v3.9-rc2.
> >
> >> drivers/gpio/gpiolib.c | 29 ++++++++++++++++-------------
> >> 1 file changed, 16 insertions(+), 13 deletions(-)
> >>
> >> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> >> index 8a2cf9c..ad6df6b 100644
> >> --- a/drivers/gpio/gpiolib.c
> >> +++ b/drivers/gpio/gpiolib.c
> >> @@ -207,7 +208,7 @@ static int gpiochip_find_base(int ngpio)
> >> }
> >>
> >> /* caller ensures gpio is valid and requested, chip->get_direction may sleep */
> >> -static int gpiod_get_direction(struct gpio_desc *desc)
> >> +static int gpiod_get_direction(const struct gpio_desc *desc)
> >> {
> >> struct gpio_chip *chip;
> >> unsigned offset;
> >> @@ -223,11 +224,13 @@ static int gpiod_get_direction(struct gpio_desc *desc)
> >> if (status > 0) {
> >> /* GPIOF_DIR_IN, or other positive */
> >> status = 1;
> >> - clear_bit(FLAG_IS_OUT, &desc->flags);
> >> + /* FLAG_IS_OUT is just a cache of the result of get_direction(),
> >> + * so it does not affect constness per se */
> >> + clear_bit(FLAG_IS_OUT, &((struct gpio_desc *)desc)->flags);
> > I think this is worse than not having gpiod_get_direction take a const
> > descriptor.
>
> Yeah, this kind of sucks but ultimately gpiod_get_direction() is
> without side-effect to the caller and the const parameter helps
> highlighting that fact.
Does that mean you want to keep the const and take the risk that it
might introduce some hard to debug problem when the compiler changes its
optimisation strategies? I hope you don't.
> > I don't know an example for this particular case where this could break,
> > but usually casting away a const is bad and can break in various ways.
> >
> > Maybe even drop the public function gpiod_get_direction completely.
> > As of next-20141114 there are only two users of this function (apart
> > from drivers/gpio/gpiolib*) and both are wrong (as discussed in
> > http://thread.gmane.org/gmane.linux.serial/16808/focus=4783).
>
> Or maybe gpiod_direction_*() should register the last set direction to
> be used by gpiod_get_direction() if the controller does not implement
> the get_direction op? On the other hand callers can also track the
> direction themselves (since they presumably set it), so this function
> is really only useful for users who want to read the *hardware* state
> of a GPIO. Which might still be a useful feature?
I think it's useful to be able to check how a given gpio is configured
in hardware. So you really want to look into the register and not cache
the direction in gpiod_direction_*.
And I'd say having /sys/kernel/debug/gpio is good enough. So IMHO there
is no reason to hide gpiod_get_direction from general misuse.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
On Wed, Nov 19, 2014 at 5:44 PM, Uwe Kleine-König
<[email protected]> wrote:
> Hello Alexandre,
>
> On Wed, Nov 19, 2014 at 05:33:42PM +0900, Alexandre Courbot wrote:
>> On Mon, Nov 17, 2014 at 6:09 PM, Uwe Kleine-König
>> <[email protected]> wrote:
>> > Hello,
>> >
>> > On Wed, Feb 13, 2013 at 04:03:00PM +0900, Alexandre Courbot wrote:
>> >> From: Alexandre Courbot <[email protected]>
>> >>
>> >> Constify descriptor parameter of gpiod_* functions for those that
>> >> should obviously not modify it. This includes value or direction get,
>> >> cansleep, and IRQ number query.
>> >>
>> >> Signed-off-by: Alexandre Courbot <[email protected]>
>> > This patch is applied as def634338d3ffb32fbe9b0a2d70cc24ef909cd4f since
>> > v3.9-rc2.
>> >
>> >> drivers/gpio/gpiolib.c | 29 ++++++++++++++++-------------
>> >> 1 file changed, 16 insertions(+), 13 deletions(-)
>> >>
>> >> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
>> >> index 8a2cf9c..ad6df6b 100644
>> >> --- a/drivers/gpio/gpiolib.c
>> >> +++ b/drivers/gpio/gpiolib.c
>> >> @@ -207,7 +208,7 @@ static int gpiochip_find_base(int ngpio)
>> >> }
>> >>
>> >> /* caller ensures gpio is valid and requested, chip->get_direction may sleep */
>> >> -static int gpiod_get_direction(struct gpio_desc *desc)
>> >> +static int gpiod_get_direction(const struct gpio_desc *desc)
>> >> {
>> >> struct gpio_chip *chip;
>> >> unsigned offset;
>> >> @@ -223,11 +224,13 @@ static int gpiod_get_direction(struct gpio_desc *desc)
>> >> if (status > 0) {
>> >> /* GPIOF_DIR_IN, or other positive */
>> >> status = 1;
>> >> - clear_bit(FLAG_IS_OUT, &desc->flags);
>> >> + /* FLAG_IS_OUT is just a cache of the result of get_direction(),
>> >> + * so it does not affect constness per se */
>> >> + clear_bit(FLAG_IS_OUT, &((struct gpio_desc *)desc)->flags);
>> > I think this is worse than not having gpiod_get_direction take a const
>> > descriptor.
>>
>> Yeah, this kind of sucks but ultimately gpiod_get_direction() is
>> without side-effect to the caller and the const parameter helps
>> highlighting that fact.
> Does that mean you want to keep the const and take the risk that it
> might introduce some hard to debug problem when the compiler changes its
> optimisation strategies? I hope you don't.
Nope, I would like to keep that const but correctness is more
important than prototype aesthetics. I will fix that.
>> > I don't know an example for this particular case where this could break,
>> > but usually casting away a const is bad and can break in various ways.
>> >
>> > Maybe even drop the public function gpiod_get_direction completely.
>> > As of next-20141114 there are only two users of this function (apart
>> > from drivers/gpio/gpiolib*) and both are wrong (as discussed in
>> > http://thread.gmane.org/gmane.linux.serial/16808/focus=4783).
>>
>> Or maybe gpiod_direction_*() should register the last set direction to
>> be used by gpiod_get_direction() if the controller does not implement
>> the get_direction op? On the other hand callers can also track the
>> direction themselves (since they presumably set it), so this function
>> is really only useful for users who want to read the *hardware* state
>> of a GPIO. Which might still be a useful feature?
> I think it's useful to be able to check how a given gpio is configured
> in hardware. So you really want to look into the register and not cache
> the direction in gpiod_direction_*.
> And I'd say having /sys/kernel/debug/gpio is good enough. So IMHO there
> is no reason to hide gpiod_get_direction from general misuse.
Mmm so are you suggesting that we should keep gpiod_get_direction() in the end?
Hello Alexandre,
On Wed, Nov 19, 2014 at 05:57:46PM +0900, Alexandre Courbot wrote:
> On Wed, Nov 19, 2014 at 5:44 PM, Uwe Kleine-K?nig
> <[email protected]> wrote:
> > Hello Alexandre,
> >
> > On Wed, Nov 19, 2014 at 05:33:42PM +0900, Alexandre Courbot wrote:
> >> On Mon, Nov 17, 2014 at 6:09 PM, Uwe Kleine-K?nig
> >> <[email protected]> wrote:
> >> > Hello,
> >> >
> >> > On Wed, Feb 13, 2013 at 04:03:00PM +0900, Alexandre Courbot wrote:
> >> >> From: Alexandre Courbot <[email protected]>
> >> >>
> >> >> Constify descriptor parameter of gpiod_* functions for those that
> >> >> should obviously not modify it. This includes value or direction get,
> >> >> cansleep, and IRQ number query.
> >> >>
> >> >> Signed-off-by: Alexandre Courbot <[email protected]>
> >> > This patch is applied as def634338d3ffb32fbe9b0a2d70cc24ef909cd4f since
> >> > v3.9-rc2.
> >> >
> >> >> drivers/gpio/gpiolib.c | 29 ++++++++++++++++-------------
> >> >> 1 file changed, 16 insertions(+), 13 deletions(-)
> >> >>
> >> >> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> >> >> index 8a2cf9c..ad6df6b 100644
> >> >> --- a/drivers/gpio/gpiolib.c
> >> >> +++ b/drivers/gpio/gpiolib.c
> >> >> @@ -207,7 +208,7 @@ static int gpiochip_find_base(int ngpio)
> >> >> }
> >> >>
> >> >> /* caller ensures gpio is valid and requested, chip->get_direction may sleep */
> >> >> -static int gpiod_get_direction(struct gpio_desc *desc)
> >> >> +static int gpiod_get_direction(const struct gpio_desc *desc)
> >> >> {
> >> >> struct gpio_chip *chip;
> >> >> unsigned offset;
> >> >> @@ -223,11 +224,13 @@ static int gpiod_get_direction(struct gpio_desc *desc)
> >> >> if (status > 0) {
> >> >> /* GPIOF_DIR_IN, or other positive */
> >> >> status = 1;
> >> >> - clear_bit(FLAG_IS_OUT, &desc->flags);
> >> >> + /* FLAG_IS_OUT is just a cache of the result of get_direction(),
> >> >> + * so it does not affect constness per se */
> >> >> + clear_bit(FLAG_IS_OUT, &((struct gpio_desc *)desc)->flags);
> >> > I think this is worse than not having gpiod_get_direction take a const
> >> > descriptor.
> >>
> >> Yeah, this kind of sucks but ultimately gpiod_get_direction() is
> >> without side-effect to the caller and the const parameter helps
> >> highlighting that fact.
> > Does that mean you want to keep the const and take the risk that it
> > might introduce some hard to debug problem when the compiler changes its
> > optimisation strategies? I hope you don't.
>
> Nope, I would like to keep that const but correctness is more
> important than prototype aesthetics. I will fix that.
Fine.
>
> >> > I don't know an example for this particular case where this could break,
> >> > but usually casting away a const is bad and can break in various ways.
> >> >
> >> > Maybe even drop the public function gpiod_get_direction completely.
> >> > As of next-20141114 there are only two users of this function (apart
> >> > from drivers/gpio/gpiolib*) and both are wrong (as discussed in
> >> > http://thread.gmane.org/gmane.linux.serial/16808/focus=4783).
> >>
> >> Or maybe gpiod_direction_*() should register the last set direction to
> >> be used by gpiod_get_direction() if the controller does not implement
> >> the get_direction op? On the other hand callers can also track the
> >> direction themselves (since they presumably set it), so this function
> >> is really only useful for users who want to read the *hardware* state
> >> of a GPIO. Which might still be a useful feature?
> > I think it's useful to be able to check how a given gpio is configured
> > in hardware. So you really want to look into the register and not cache
> > the direction in gpiod_direction_*.
> > And I'd say having /sys/kernel/debug/gpio is good enough. So IMHO there
> > is no reason to hide gpiod_get_direction from general misuse.
>
> Mmm so are you suggesting that we should keep gpiod_get_direction() in the end?
I'd make gpiod_get_direction static and only use it to fill in
/sys/kernel/debug/gpio.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
On Wed, Nov 19, 2014 at 6:02 PM, Uwe Kleine-König
<[email protected]> wrote:
> Hello Alexandre,
>
> On Wed, Nov 19, 2014 at 05:57:46PM +0900, Alexandre Courbot wrote:
>> On Wed, Nov 19, 2014 at 5:44 PM, Uwe Kleine-König
>> <[email protected]> wrote:
>> > Hello Alexandre,
>> >
>> > On Wed, Nov 19, 2014 at 05:33:42PM +0900, Alexandre Courbot wrote:
>> >> On Mon, Nov 17, 2014 at 6:09 PM, Uwe Kleine-König
>> >> <[email protected]> wrote:
>> >> > Hello,
>> >> >
>> >> > On Wed, Feb 13, 2013 at 04:03:00PM +0900, Alexandre Courbot wrote:
>> >> >> From: Alexandre Courbot <[email protected]>
>> >> >>
>> >> >> Constify descriptor parameter of gpiod_* functions for those that
>> >> >> should obviously not modify it. This includes value or direction get,
>> >> >> cansleep, and IRQ number query.
>> >> >>
>> >> >> Signed-off-by: Alexandre Courbot <[email protected]>
>> >> > This patch is applied as def634338d3ffb32fbe9b0a2d70cc24ef909cd4f since
>> >> > v3.9-rc2.
>> >> >
>> >> >> drivers/gpio/gpiolib.c | 29 ++++++++++++++++-------------
>> >> >> 1 file changed, 16 insertions(+), 13 deletions(-)
>> >> >>
>> >> >> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
>> >> >> index 8a2cf9c..ad6df6b 100644
>> >> >> --- a/drivers/gpio/gpiolib.c
>> >> >> +++ b/drivers/gpio/gpiolib.c
>> >> >> @@ -207,7 +208,7 @@ static int gpiochip_find_base(int ngpio)
>> >> >> }
>> >> >>
>> >> >> /* caller ensures gpio is valid and requested, chip->get_direction may sleep */
>> >> >> -static int gpiod_get_direction(struct gpio_desc *desc)
>> >> >> +static int gpiod_get_direction(const struct gpio_desc *desc)
>> >> >> {
>> >> >> struct gpio_chip *chip;
>> >> >> unsigned offset;
>> >> >> @@ -223,11 +224,13 @@ static int gpiod_get_direction(struct gpio_desc *desc)
>> >> >> if (status > 0) {
>> >> >> /* GPIOF_DIR_IN, or other positive */
>> >> >> status = 1;
>> >> >> - clear_bit(FLAG_IS_OUT, &desc->flags);
>> >> >> + /* FLAG_IS_OUT is just a cache of the result of get_direction(),
>> >> >> + * so it does not affect constness per se */
>> >> >> + clear_bit(FLAG_IS_OUT, &((struct gpio_desc *)desc)->flags);
>> >> > I think this is worse than not having gpiod_get_direction take a const
>> >> > descriptor.
>> >>
>> >> Yeah, this kind of sucks but ultimately gpiod_get_direction() is
>> >> without side-effect to the caller and the const parameter helps
>> >> highlighting that fact.
>> > Does that mean you want to keep the const and take the risk that it
>> > might introduce some hard to debug problem when the compiler changes its
>> > optimisation strategies? I hope you don't.
>>
>> Nope, I would like to keep that const but correctness is more
>> important than prototype aesthetics. I will fix that.
> Fine.
>
>>
>> >> > I don't know an example for this particular case where this could break,
>> >> > but usually casting away a const is bad and can break in various ways.
>> >> >
>> >> > Maybe even drop the public function gpiod_get_direction completely.
>> >> > As of next-20141114 there are only two users of this function (apart
>> >> > from drivers/gpio/gpiolib*) and both are wrong (as discussed in
>> >> > http://thread.gmane.org/gmane.linux.serial/16808/focus=4783).
>> >>
>> >> Or maybe gpiod_direction_*() should register the last set direction to
>> >> be used by gpiod_get_direction() if the controller does not implement
>> >> the get_direction op? On the other hand callers can also track the
>> >> direction themselves (since they presumably set it), so this function
>> >> is really only useful for users who want to read the *hardware* state
>> >> of a GPIO. Which might still be a useful feature?
>> > I think it's useful to be able to check how a given gpio is configured
>> > in hardware. So you really want to look into the register and not cache
>> > the direction in gpiod_direction_*.
>> > And I'd say having /sys/kernel/debug/gpio is good enough. So IMHO there
>> > is no reason to hide gpiod_get_direction from general misuse.
>>
>> Mmm so are you suggesting that we should keep gpiod_get_direction() in the end?
> I'd make gpiod_get_direction static and only use it to fill in
> /sys/kernel/debug/gpio.
That's very tempting. I see only atmel_serial.c using this function,
and there is no gpio_get_direction() declared anywhere so no user of
this either. I'm not sure what I was thinking when I decided to export
it?
So yeah, if you can fix the current users, I agree with your proposal.
Hello Alexandre,
On Wed, Nov 19, 2014 at 06:07:50PM +0900, Alexandre Courbot wrote:
> On Wed, Nov 19, 2014 at 6:02 PM, Uwe Kleine-K?nig
> <[email protected]> wrote:
> > I'd make gpiod_get_direction static and only use it to fill in
> > /sys/kernel/debug/gpio.
>
> That's very tempting. I see only atmel_serial.c using this function,
> and there is no gpio_get_direction() declared anywhere so no user of
> this either. I'm not sure what I was thinking when I decided to export
> it?
In next there is also drivers/tty/serial/mxs-auart.c.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
On Wed, Nov 19, 2014 at 7:09 PM, Uwe Kleine-König
<[email protected]> wrote:
> Hello Alexandre,
>
> On Wed, Nov 19, 2014 at 06:07:50PM +0900, Alexandre Courbot wrote:
>> On Wed, Nov 19, 2014 at 6:02 PM, Uwe Kleine-König
>> <[email protected]> wrote:
>> > I'd make gpiod_get_direction static and only use it to fill in
>> > /sys/kernel/debug/gpio.
>>
>> That's very tempting. I see only atmel_serial.c using this function,
>> and there is no gpio_get_direction() declared anywhere so no user of
>> this either. I'm not sure what I was thinking when I decided to export
>> it?
> In next there is also drivers/tty/serial/mxs-auart.c.
Ok. I think we can get rid of gpiod_get_direction() if
serial_mctrl_gpio implements its own way to query the direction (maybe
using a bitmap in the mctrl_gpios and a mctrl_gpio_get_direction()
function that queries that map). Using gpiod_get_direction() is just
too unreliable.
Janusz, since your change for mxs-auart is in -next, would you mind
amending it to do this? Then we could do the same for atmel_serial and
remove gpiod_get_direction() from the public GPIO interface. This
function would do more harm than good anyway.
Thanks!
Hello,
W dniu 2014-11-25 o 08:37, Alexandre Courbot pisze:
> On Wed, Nov 19, 2014 at 7:09 PM, Uwe Kleine-König
> <[email protected]> wrote:
>> Hello Alexandre,
>>
>> On Wed, Nov 19, 2014 at 06:07:50PM +0900, Alexandre Courbot wrote:
>>> On Wed, Nov 19, 2014 at 6:02 PM, Uwe Kleine-König
>>> <[email protected]> wrote:
>>>> I'd make gpiod_get_direction static and only use it to fill in
>>>> /sys/kernel/debug/gpio.
>>> That's very tempting. I see only atmel_serial.c using this function,
>>> and there is no gpio_get_direction() declared anywhere so no user of
>>> this either. I'm not sure what I was thinking when I decided to export
>>> it?
>> In next there is also drivers/tty/serial/mxs-auart.c.
> Ok. I think we can get rid of gpiod_get_direction() if
> serial_mctrl_gpio implements its own way to query the direction (maybe
> using a bitmap in the mctrl_gpios and a mctrl_gpio_get_direction()
> function that queries that map). Using gpiod_get_direction() is just
> too unreliable.
good idea
>
> Janusz, since your change for mxs-auart is in -next, would you mind
> amending it to do this? Then we could do the same for atmel_serial and
> remove gpiod_get_direction() from the public GPIO interface. This
> function would do more harm than good anyway.
After a discussion I prepared RFC patch set to avoid the function.
You will find it here [1]:
("[RFC PATCH 1/2] serial: mctrl-gpio: Add irqs helpers for input lines")
("[RFC PATCH 2/2] serial: mxs-auart: use helpers for gpio irqs")
and the function was moved for debug only:
("[PATCH v3] gpio: mxs: implement get_direction callback")
Affected drivers by the patch set in the next are:
- atmel_serial
- mxs-auart
- clps711x (very basic usage)
The RFC patch set removes the inconvenient function from mxs-auart as
example.
I didn't prepared a patch for atmel_serial because I wait for comments about
mctrl_gpio_is_gpio() function to differentiate if line is GPIO or native in
enable/disable_ms() callbacks.
I also can't test it for other devices than mxs (mx28).
[1] http://www.spinics.net/lists/arm-kernel/msg378444.html
best regards
Janusz