Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1602527 > unrolled thread
| Started by | Linus Walleij <linus.walleij@linaro.org> |
|---|---|
| First post | 2017-03-16 16:30 +0100 |
| Last post | 2017-03-16 17:40 +0100 |
| Articles | 10 on this page of 30 — 4 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: [PATCH 4/4] tty/serial: sh-sci: remove uneeded IS_ERR_OR_NULL calls Linus Walleij <linus.walleij@linaro.org> - 2017-03-16 16:30 +0100
Re: [PATCH 4/4] tty/serial: sh-sci: remove uneeded IS_ERR_OR_NULL calls Geert Uytterhoeven <geert@linux-m68k.org> - 2017-03-16 17:40 +0100
Re: [PATCH 4/4] tty/serial: sh-sci: remove uneeded IS_ERR_OR_NULL calls Geert Uytterhoeven <geert@linux-m68k.org> - 2017-03-20 11:00 +0100
Re: [PATCH 4/4] tty/serial: sh-sci: remove uneeded IS_ERR_OR_NULL calls Geert Uytterhoeven <geert@linux-m68k.org> - 2017-03-20 11:20 +0100
Re: [PATCH 4/4] tty/serial: sh-sci: remove uneeded IS_ERR_OR_NULL calls Uwe Kleine-König <u.kleine-koenig@pengutronix.de> - 2017-03-20 11:40 +0100
Re: [PATCH 4/4] tty/serial: sh-sci: remove uneeded IS_ERR_OR_NULL calls Geert Uytterhoeven <geert@linux-m68k.org> - 2017-03-20 12:00 +0100
Re: [PATCH 4/4] tty/serial: sh-sci: remove uneeded IS_ERR_OR_NULL calls Uwe Kleine-König <u.kleine-koenig@pengutronix.de> - 2017-03-20 12:10 +0100
Re: [PATCH 4/4] tty/serial: sh-sci: remove uneeded IS_ERR_OR_NULL calls Linus Walleij <linus.walleij@linaro.org> - 2017-03-23 10:40 +0100
Re: [PATCH 4/4] tty/serial: sh-sci: remove uneeded IS_ERR_OR_NULL calls Uwe Kleine-König <u.kleine-koenig@pengutronix.de> - 2017-03-23 11:20 +0100
Re: [PATCH 4/4] tty/serial: sh-sci: remove uneeded IS_ERR_OR_NULL calls Geert Uytterhoeven <geert@linux-m68k.org> - 2017-03-23 11:30 +0100
Re: [PATCH 4/4] tty/serial: sh-sci: remove uneeded IS_ERR_OR_NULL calls Uwe Kleine-König <u.kleine-koenig@pengutronix.de> - 2017-03-23 12:20 +0100
Re: [PATCH 4/4] tty/serial: sh-sci: remove uneeded IS_ERR_OR_NULL calls Geert Uytterhoeven <geert@linux-m68k.org> - 2017-03-23 13:10 +0100
Re: [PATCH 4/4] tty/serial: sh-sci: remove uneeded IS_ERR_OR_NULL calls Uwe Kleine-König <u.kleine-koenig@pengutronix.de> - 2017-03-23 13:40 +0100
Re: [PATCH 4/4] tty/serial: sh-sci: remove uneeded IS_ERR_OR_NULL calls Geert Uytterhoeven <geert@linux-m68k.org> - 2017-03-23 13:50 +0100
Re: [PATCH 4/4] tty/serial: sh-sci: remove uneeded IS_ERR_OR_NULL calls Linus Walleij <linus.walleij@linaro.org> - 2017-03-23 14:50 +0100
Re: [PATCH 4/4] tty/serial: sh-sci: remove uneeded IS_ERR_OR_NULL calls Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2017-03-23 15:50 +0100
Re: [PATCH 4/4] tty/serial: sh-sci: remove uneeded IS_ERR_OR_NULL calls Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2017-03-23 16:50 +0100
Re: [PATCH 4/4] tty/serial: sh-sci: remove uneeded IS_ERR_OR_NULL calls Uwe Kleine-König <u.kleine-koenig@pengutronix.de> - 2017-03-23 20:20 +0100
Re: [PATCH 4/4] tty/serial: sh-sci: remove uneeded IS_ERR_OR_NULL calls Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2017-03-23 21:00 +0100
Re: [PATCH 4/4] tty/serial: sh-sci: remove uneeded IS_ERR_OR_NULL calls Uwe Kleine-König <u.kleine-koenig@pengutronix.de> - 2017-03-24 09:30 +0100
Re: [PATCH 4/4] tty/serial: sh-sci: remove uneeded IS_ERR_OR_NULL calls Uwe Kleine-König <u.kleine-koenig@pengutronix.de> - 2017-03-24 09:40 +0100
Re: [PATCH 4/4] tty/serial: sh-sci: remove uneeded IS_ERR_OR_NULL calls Geert Uytterhoeven <geert@linux-m68k.org> - 2017-03-24 10:00 +0100
Re: [PATCH 4/4] tty/serial: sh-sci: remove uneeded IS_ERR_OR_NULL calls Uwe Kleine-König <u.kleine-koenig@pengutronix.de> - 2017-03-24 10:20 +0100
Re: [PATCH 4/4] tty/serial: sh-sci: remove uneeded IS_ERR_OR_NULL calls Geert Uytterhoeven <geert@linux-m68k.org> - 2017-03-24 10:50 +0100
Re: [PATCH 4/4] tty/serial: sh-sci: remove uneeded IS_ERR_OR_NULL calls Uwe Kleine-König <u.kleine-koenig@pengutronix.de> - 2017-03-24 11:10 +0100
Re: [PATCH 4/4] tty/serial: sh-sci: remove uneeded IS_ERR_OR_NULL calls Geert Uytterhoeven <geert@linux-m68k.org> - 2017-03-24 09:40 +0100
Re: [PATCH 4/4] tty/serial: sh-sci: remove uneeded IS_ERR_OR_NULL calls Linus Walleij <linus.walleij@linaro.org> - 2017-03-24 10:00 +0100
Re: [PATCH 4/4] tty/serial: sh-sci: remove uneeded IS_ERR_OR_NULL calls Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2017-03-23 17:00 +0100
Re: [PATCH 4/4] tty/serial: sh-sci: remove uneeded IS_ERR_OR_NULL calls Linus Walleij <linus.walleij@linaro.org> - 2017-03-23 14:40 +0100
Re: [PATCH 4/4] tty/serial: sh-sci: remove uneeded IS_ERR_OR_NULL calls Uwe Kleine-König <u.kleine-koenig@pengutronix.de> - 2017-03-16 17:40 +0100
Page 2 of 2 — ← Prev page 1 [2]
| From | Uwe Kleine-König <u.kleine-koenig@pengutronix.de> |
|---|---|
| Date | 2017-03-24 09:40 +0100 |
| Subject | Re: [PATCH 4/4] tty/serial: sh-sci: remove uneeded IS_ERR_OR_NULL calls |
| Message-ID | <tosVj-7gO-5@gated-at.bofh.it> |
| In reply to | #1608204 |
Hello Geert,
On Fri, Mar 24, 2017 at 09:29:02AM +0100, Geert Uytterhoeven wrote:
> On Fri, Mar 24, 2017 at 9:00 AM, Uwe Kleine-König
> <u.kleine-koenig@pengutronix.de> wrote:
> > From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > Subject: [PATCH] gpiod: let get_optional return NULL in some cases with GPIOLIB disabled
> >
> > People disagree if gpiod_get_optional should return NULL or
> > ERR_PTR(-ENOSYS) if GPIOLIB is disabled. The argument for NULL is that
> > the person who decided to disable GPIOLIB is assumed to know that there
> > is no GPIO. The reason to stick to ERR_PTR(-ENOSYS) is that it might
> > introduce hard to debug problems if that decision is wrong.
> >
> > So this patch introduces a compromise and let gpiod_get_optional (and
> > its variants) return NULL if the device in question cannot have an
> > associated GPIO because it is neither instantiated by a device tree nor
> > by ACPI.
> >
> > This should handle most cases that are argued about.
> >
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > ---
> > include/linux/gpio/consumer.h | 55 ++++++++++++++++++++++++++++++++++++-------
> > 1 file changed, 46 insertions(+), 9 deletions(-)
> >
> > diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
> > index fb0fde686cb1..0ca29889290d 100644
> > --- a/include/linux/gpio/consumer.h
> > +++ b/include/linux/gpio/consumer.h
> > @@ -161,20 +161,48 @@ gpiod_get_index(struct device *dev,
> > return ERR_PTR(-ENOSYS);
> > }
> >
> > -static inline struct gpio_desc *__must_check
> > -gpiod_get_optional(struct device *dev, const char *con_id,
> > - enum gpiod_flags flags)
> > +static inline bool __gpiod_no_optional_possible(struct device *dev)
> > {
> > - return ERR_PTR(-ENOSYS);
> > + /*
> > + * gpiod_get_optional et al can only provide a GPIO if at least one of
> > + * the backends for specifing a GPIO is available. These are device
> > + * tree, ACPI and gpiolib's lookup tables. The latter isn't available if
> > + * GPIOLIB is disabled (which is the case here).
> > + * So if the provided device is unrelated to device tree and ACPI, we
> > + * can be sure that there is no optional GPIO and let gpiod_get_optional
> > + * safely return NULL.
> > + * Otherwise there is still a chance that there is no GPIO but we cannot
> > + * be sure without having to enable a part of GPIOLIB (i.e. the lookup
> > + * part). So lets play safe and return an error. (Though there are also
> > + * arguments that returning NULL then would be beneficial.)
> > + */
> > +
> > + if (IS_ENABLED(CONFIG_OF) && dev && dev->of_node)
> > + return false;
>
> At first sight, I though this was OK:
>
> 1. On ARM with DT, we can assume CONFIG_GPIOLOB=y.
>
> 2. I managed to configure an SH kernel with CONFIG_GPIOLOB=n, CONFIG_OF=y,
> and CONFIG_SERIAL_SH_SCI=y, but since SH boards with SH-SCI UARTs do
> not use DT (yet), the check for dev->of_node (false) should handle
> that.
>
> 3. However, I managed to do the same for h8300, which does use DT. Hence
> if mctrl_gpio would start relying on gpiod_get_optional(), this would
> break the sh-sci driver on h8300 :-(
> Note that h8300 doesn't have any GPIO drivers (yet?), so
> CONFIG_GPIPOLIB=n makes perfect sense!
Thanks for your efforts.
> So I'm afraid the only option is to always return NULL, and put the
> responsability on the shoulders of the system integrator...
The gpio lines could be provided by an i2c gpio adapter, right? So IMHO
you don't need platform gpios to justify -ENODEV. So I guess that's a
case where we don't come to an agreement.
> > + if (IS_ENABLED(CONFIG_ACPI) && dev && ACPI_COMPANION(dev))
> > + return false;
>
> No comments about the ACPI case.
>
> > static inline struct gpio_desc *__must_check
> > gpiod_get_index_optional(struct device *dev, const char *con_id,
> > unsigned int index, enum gpiod_flags flags)
> > {
> > + if (__gpiod_no_optional_possible(dev))
> > + return NULL;
> > +
> > return ERR_PTR(-ENOSYS);
>
> Regardless of the above, given you use the exact same construct in four
> locations, what about letting __gpiod_no_optional_possible() return the NULL
> or ERR_PTR itself, and renaming it to e.g. __gpiod_no_optional_return_value()?
I thought about that but didn't find a good name and so considered it
more clear this way. Another optimisation would be to unconditionally
define get_optional in terms of get_index_optional which would simplify
my patch a bit.
I'd consider __gpiod_optional_return_value a better name than
__gpiod_no_optional_return_value but I'm still not convinced.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
[toc] | [prev] | [next] | [standalone]
| From | Geert Uytterhoeven <geert@linux-m68k.org> |
|---|---|
| Date | 2017-03-24 10:00 +0100 |
| Message-ID | <toteH-7pT-37@gated-at.bofh.it> |
| In reply to | #1608209 |
Hi Uwe,
On Fri, Mar 24, 2017 at 9:39 AM, Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
> On Fri, Mar 24, 2017 at 09:29:02AM +0100, Geert Uytterhoeven wrote:
>> On Fri, Mar 24, 2017 at 9:00 AM, Uwe Kleine-König
>> <u.kleine-koenig@pengutronix.de> wrote:
>> > From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>> > Subject: [PATCH] gpiod: let get_optional return NULL in some cases with GPIOLIB disabled
>> >
>> > People disagree if gpiod_get_optional should return NULL or
>> > ERR_PTR(-ENOSYS) if GPIOLIB is disabled. The argument for NULL is that
>> > the person who decided to disable GPIOLIB is assumed to know that there
>> > is no GPIO. The reason to stick to ERR_PTR(-ENOSYS) is that it might
>> > introduce hard to debug problems if that decision is wrong.
>> >
>> > So this patch introduces a compromise and let gpiod_get_optional (and
>> > its variants) return NULL if the device in question cannot have an
>> > associated GPIO because it is neither instantiated by a device tree nor
>> > by ACPI.
>> >
>> > This should handle most cases that are argued about.
>> >
>> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>> > ---
>> > include/linux/gpio/consumer.h | 55 ++++++++++++++++++++++++++++++++++++-------
>> > 1 file changed, 46 insertions(+), 9 deletions(-)
>> >
>> > diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
>> > index fb0fde686cb1..0ca29889290d 100644
>> > --- a/include/linux/gpio/consumer.h
>> > +++ b/include/linux/gpio/consumer.h
>> > @@ -161,20 +161,48 @@ gpiod_get_index(struct device *dev,
>> > return ERR_PTR(-ENOSYS);
>> > }
>> >
>> > -static inline struct gpio_desc *__must_check
>> > -gpiod_get_optional(struct device *dev, const char *con_id,
>> > - enum gpiod_flags flags)
>> > +static inline bool __gpiod_no_optional_possible(struct device *dev)
>> > {
>> > - return ERR_PTR(-ENOSYS);
>> > + /*
>> > + * gpiod_get_optional et al can only provide a GPIO if at least one of
>> > + * the backends for specifing a GPIO is available. These are device
>> > + * tree, ACPI and gpiolib's lookup tables. The latter isn't available if
>> > + * GPIOLIB is disabled (which is the case here).
>> > + * So if the provided device is unrelated to device tree and ACPI, we
>> > + * can be sure that there is no optional GPIO and let gpiod_get_optional
>> > + * safely return NULL.
>> > + * Otherwise there is still a chance that there is no GPIO but we cannot
>> > + * be sure without having to enable a part of GPIOLIB (i.e. the lookup
>> > + * part). So lets play safe and return an error. (Though there are also
>> > + * arguments that returning NULL then would be beneficial.)
>> > + */
>> > +
>> > + if (IS_ENABLED(CONFIG_OF) && dev && dev->of_node)
>> > + return false;
>>
>> At first sight, I though this was OK:
>>
>> 1. On ARM with DT, we can assume CONFIG_GPIOLOB=y.
>>
>> 2. I managed to configure an SH kernel with CONFIG_GPIOLOB=n, CONFIG_OF=y,
>> and CONFIG_SERIAL_SH_SCI=y, but since SH boards with SH-SCI UARTs do
>> not use DT (yet), the check for dev->of_node (false) should handle
>> that.
>>
>> 3. However, I managed to do the same for h8300, which does use DT. Hence
>> if mctrl_gpio would start relying on gpiod_get_optional(), this would
>> break the sh-sci driver on h8300 :-(
>> Note that h8300 doesn't have any GPIO drivers (yet?), so
>> CONFIG_GPIPOLIB=n makes perfect sense!
>
> Thanks for your efforts.
You're welcome.
>> So I'm afraid the only option is to always return NULL, and put the
>> responsability on the shoulders of the system integrator...
>
> The gpio lines could be provided by an i2c gpio adapter, right? So IMHO
> you don't need platform gpios to justify -ENODEV. So I guess that's a
> case where we don't come to an agreement.
While you can enable I2C without further dependencies, no I2C GPIO expander
will be offered... unless you have enabled CONFIG_GPIOLIB first.
>> > static inline struct gpio_desc *__must_check
>> > gpiod_get_index_optional(struct device *dev, const char *con_id,
>> > unsigned int index, enum gpiod_flags flags)
>> > {
>> > + if (__gpiod_no_optional_possible(dev))
>> > + return NULL;
>> > +
>> > return ERR_PTR(-ENOSYS);
>>
>> Regardless of the above, given you use the exact same construct in four
>> locations, what about letting __gpiod_no_optional_possible() return the NULL
>> or ERR_PTR itself, and renaming it to e.g. __gpiod_no_optional_return_value()?
>
> I thought about that but didn't find a good name and so considered it
> more clear this way. Another optimisation would be to unconditionally
> define get_optional in terms of get_index_optional which would simplify
> my patch a bit.
>
> I'd consider __gpiod_optional_return_value a better name than
> __gpiod_no_optional_return_value but I'm still not convinced.
No hard feelings about the name from my side.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
[toc] | [prev] | [next] | [standalone]
| From | Uwe Kleine-König <u.kleine-koenig@pengutronix.de> |
|---|---|
| Date | 2017-03-24 10:20 +0100 |
| Subject | Re: [PATCH 4/4] tty/serial: sh-sci: remove uneeded IS_ERR_OR_NULL calls |
| Message-ID | <toty2-7Nu-11@gated-at.bofh.it> |
| In reply to | #1608243 |
On Fri, Mar 24, 2017 at 09:59:04AM +0100, Geert Uytterhoeven wrote:
> Hi Uwe,
>
> On Fri, Mar 24, 2017 at 9:39 AM, Uwe Kleine-König
> <u.kleine-koenig@pengutronix.de> wrote:
> > On Fri, Mar 24, 2017 at 09:29:02AM +0100, Geert Uytterhoeven wrote:
> >> On Fri, Mar 24, 2017 at 9:00 AM, Uwe Kleine-König
> >> <u.kleine-koenig@pengutronix.de> wrote:
> >> > From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> >> > Subject: [PATCH] gpiod: let get_optional return NULL in some cases with GPIOLIB disabled
> >> >
> >> > People disagree if gpiod_get_optional should return NULL or
> >> > ERR_PTR(-ENOSYS) if GPIOLIB is disabled. The argument for NULL is that
> >> > the person who decided to disable GPIOLIB is assumed to know that there
> >> > is no GPIO. The reason to stick to ERR_PTR(-ENOSYS) is that it might
> >> > introduce hard to debug problems if that decision is wrong.
> >> >
> >> > So this patch introduces a compromise and let gpiod_get_optional (and
> >> > its variants) return NULL if the device in question cannot have an
> >> > associated GPIO because it is neither instantiated by a device tree nor
> >> > by ACPI.
> >> >
> >> > This should handle most cases that are argued about.
> >> >
> >> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> >> > ---
> >> > include/linux/gpio/consumer.h | 55 ++++++++++++++++++++++++++++++++++++-------
> >> > 1 file changed, 46 insertions(+), 9 deletions(-)
> >> >
> >> > diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
> >> > index fb0fde686cb1..0ca29889290d 100644
> >> > --- a/include/linux/gpio/consumer.h
> >> > +++ b/include/linux/gpio/consumer.h
> >> > @@ -161,20 +161,48 @@ gpiod_get_index(struct device *dev,
> >> > return ERR_PTR(-ENOSYS);
> >> > }
> >> >
> >> > -static inline struct gpio_desc *__must_check
> >> > -gpiod_get_optional(struct device *dev, const char *con_id,
> >> > - enum gpiod_flags flags)
> >> > +static inline bool __gpiod_no_optional_possible(struct device *dev)
> >> > {
> >> > - return ERR_PTR(-ENOSYS);
> >> > + /*
> >> > + * gpiod_get_optional et al can only provide a GPIO if at least one of
> >> > + * the backends for specifing a GPIO is available. These are device
> >> > + * tree, ACPI and gpiolib's lookup tables. The latter isn't available if
> >> > + * GPIOLIB is disabled (which is the case here).
> >> > + * So if the provided device is unrelated to device tree and ACPI, we
> >> > + * can be sure that there is no optional GPIO and let gpiod_get_optional
> >> > + * safely return NULL.
> >> > + * Otherwise there is still a chance that there is no GPIO but we cannot
> >> > + * be sure without having to enable a part of GPIOLIB (i.e. the lookup
> >> > + * part). So lets play safe and return an error. (Though there are also
> >> > + * arguments that returning NULL then would be beneficial.)
> >> > + */
> >> > +
> >> > + if (IS_ENABLED(CONFIG_OF) && dev && dev->of_node)
> >> > + return false;
> >>
> >> At first sight, I though this was OK:
> >>
> >> 1. On ARM with DT, we can assume CONFIG_GPIOLOB=y.
> >>
> >> 2. I managed to configure an SH kernel with CONFIG_GPIOLOB=n, CONFIG_OF=y,
> >> and CONFIG_SERIAL_SH_SCI=y, but since SH boards with SH-SCI UARTs do
> >> not use DT (yet), the check for dev->of_node (false) should handle
> >> that.
> >>
> >> 3. However, I managed to do the same for h8300, which does use DT. Hence
> >> if mctrl_gpio would start relying on gpiod_get_optional(), this would
> >> break the sh-sci driver on h8300 :-(
> >> Note that h8300 doesn't have any GPIO drivers (yet?), so
> >> CONFIG_GPIPOLIB=n makes perfect sense!
> >
> > Thanks for your efforts.
>
> You're welcome.
>
> >> So I'm afraid the only option is to always return NULL, and put the
> >> responsability on the shoulders of the system integrator...
> >
> > The gpio lines could be provided by an i2c gpio adapter, right? So IMHO
> > you don't need platform gpios to justify -ENODEV. So I guess that's a
> > case where we don't come to an agreement.
>
> While you can enable I2C without further dependencies, no I2C GPIO expander
> will be offered... unless you have enabled CONFIG_GPIOLIB first.
And that is expected, still the device tree could reference such a GPIO
and thus create a situation where Dmitry's and my judgement disagree.
So I think my suggestion is the best we could do now. It minimizes the
number of cases where we disagree. The next best thing would be to
implement that half gpiolib stuff (i.e. do the full lookup to be sure
there is no gpio) but this comes at a price: We need some time to
implement it and it adds a bit to the kernel size.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
[toc] | [prev] | [next] | [standalone]
| From | Geert Uytterhoeven <geert@linux-m68k.org> |
|---|---|
| Date | 2017-03-24 10:50 +0100 |
| Message-ID | <tou14-80L-21@gated-at.bofh.it> |
| In reply to | #1608259 |
Hi Uwe,
On Fri, Mar 24, 2017 at 10:15 AM, Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
> On Fri, Mar 24, 2017 at 09:59:04AM +0100, Geert Uytterhoeven wrote:
>> On Fri, Mar 24, 2017 at 9:39 AM, Uwe Kleine-König
>> <u.kleine-koenig@pengutronix.de> wrote:
>> > On Fri, Mar 24, 2017 at 09:29:02AM +0100, Geert Uytterhoeven wrote:
>> >> On Fri, Mar 24, 2017 at 9:00 AM, Uwe Kleine-König
>> >> <u.kleine-koenig@pengutronix.de> wrote:
>> >> > From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>> >> > Subject: [PATCH] gpiod: let get_optional return NULL in some cases with GPIOLIB disabled
>> >> >
>> >> > People disagree if gpiod_get_optional should return NULL or
>> >> > ERR_PTR(-ENOSYS) if GPIOLIB is disabled. The argument for NULL is that
>> >> > the person who decided to disable GPIOLIB is assumed to know that there
>> >> > is no GPIO. The reason to stick to ERR_PTR(-ENOSYS) is that it might
>> >> > introduce hard to debug problems if that decision is wrong.
>> >> >
>> >> > So this patch introduces a compromise and let gpiod_get_optional (and
>> >> > its variants) return NULL if the device in question cannot have an
>> >> > associated GPIO because it is neither instantiated by a device tree nor
>> >> > by ACPI.
>> >> >
>> >> > This should handle most cases that are argued about.
>> >> >
>> >> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>> >> > ---
>> >> > include/linux/gpio/consumer.h | 55 ++++++++++++++++++++++++++++++++++++-------
>> >> > 1 file changed, 46 insertions(+), 9 deletions(-)
>> >> >
>> >> > diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
>> >> > index fb0fde686cb1..0ca29889290d 100644
>> >> > --- a/include/linux/gpio/consumer.h
>> >> > +++ b/include/linux/gpio/consumer.h
>> >> > @@ -161,20 +161,48 @@ gpiod_get_index(struct device *dev,
>> >> > return ERR_PTR(-ENOSYS);
>> >> > }
>> >> >
>> >> > -static inline struct gpio_desc *__must_check
>> >> > -gpiod_get_optional(struct device *dev, const char *con_id,
>> >> > - enum gpiod_flags flags)
>> >> > +static inline bool __gpiod_no_optional_possible(struct device *dev)
>> >> > {
>> >> > - return ERR_PTR(-ENOSYS);
>> >> > + /*
>> >> > + * gpiod_get_optional et al can only provide a GPIO if at least one of
>> >> > + * the backends for specifing a GPIO is available. These are device
>> >> > + * tree, ACPI and gpiolib's lookup tables. The latter isn't available if
>> >> > + * GPIOLIB is disabled (which is the case here).
>> >> > + * So if the provided device is unrelated to device tree and ACPI, we
>> >> > + * can be sure that there is no optional GPIO and let gpiod_get_optional
>> >> > + * safely return NULL.
>> >> > + * Otherwise there is still a chance that there is no GPIO but we cannot
>> >> > + * be sure without having to enable a part of GPIOLIB (i.e. the lookup
>> >> > + * part). So lets play safe and return an error. (Though there are also
>> >> > + * arguments that returning NULL then would be beneficial.)
>> >> > + */
>> >> > +
>> >> > + if (IS_ENABLED(CONFIG_OF) && dev && dev->of_node)
>> >> > + return false;
>> >>
>> >> At first sight, I though this was OK:
>> >>
>> >> 1. On ARM with DT, we can assume CONFIG_GPIOLOB=y.
>> >>
>> >> 2. I managed to configure an SH kernel with CONFIG_GPIOLOB=n, CONFIG_OF=y,
>> >> and CONFIG_SERIAL_SH_SCI=y, but since SH boards with SH-SCI UARTs do
>> >> not use DT (yet), the check for dev->of_node (false) should handle
>> >> that.
>> >>
>> >> 3. However, I managed to do the same for h8300, which does use DT. Hence
>> >> if mctrl_gpio would start relying on gpiod_get_optional(), this would
>> >> break the sh-sci driver on h8300 :-(
>> >> Note that h8300 doesn't have any GPIO drivers (yet?), so
>> >> CONFIG_GPIPOLIB=n makes perfect sense!
>> >
>> > Thanks for your efforts.
>>
>> You're welcome.
>>
>> >> So I'm afraid the only option is to always return NULL, and put the
>> >> responsability on the shoulders of the system integrator...
>> >
>> > The gpio lines could be provided by an i2c gpio adapter, right? So IMHO
>> > you don't need platform gpios to justify -ENODEV. So I guess that's a
>> > case where we don't come to an agreement.
>>
>> While you can enable I2C without further dependencies, no I2C GPIO expander
>> will be offered... unless you have enabled CONFIG_GPIOLIB first.
>
> And that is expected, still the device tree could reference such a GPIO
> and thus create a situation where Dmitry's and my judgement disagree.
If the device tree references such a GPIO, and CONFIG_GPIOLIB is not set,
the I2C GPIO expander device will not be bound.
Frank Rowand's DT scripts (http://elinux.org/Device_Tree_frowand) will come
to the rescue, and inform the user which driver(s) to enable.
> So I think my suggestion is the best we could do now. It minimizes the
> number of cases where we disagree. The next best thing would be to
> implement that half gpiolib stuff (i.e. do the full lookup to be sure
> there is no gpio) but this comes at a price: We need some time to
> implement it and it adds a bit to the kernel size.
So I still have to handle -ENOSYS in sh-sci.c, to avoid regressions...
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
[toc] | [prev] | [next] | [standalone]
| From | Uwe Kleine-König <u.kleine-koenig@pengutronix.de> |
|---|---|
| Date | 2017-03-24 11:10 +0100 |
| Subject | Re: [PATCH 4/4] tty/serial: sh-sci: remove uneeded IS_ERR_OR_NULL calls |
| Message-ID | <toukq-8qR-45@gated-at.bofh.it> |
| In reply to | #1608286 |
Hello Geert,
On Fri, Mar 24, 2017 at 10:44:50AM +0100, Geert Uytterhoeven wrote:
> On Fri, Mar 24, 2017 at 10:15 AM, Uwe Kleine-König
> <u.kleine-koenig@pengutronix.de> wrote:
> > On Fri, Mar 24, 2017 at 09:59:04AM +0100, Geert Uytterhoeven wrote:
> >> On Fri, Mar 24, 2017 at 9:39 AM, Uwe Kleine-König
> >> <u.kleine-koenig@pengutronix.de> wrote:
> >> > On Fri, Mar 24, 2017 at 09:29:02AM +0100, Geert Uytterhoeven wrote:
> >> >> On Fri, Mar 24, 2017 at 9:00 AM, Uwe Kleine-König
> >> >> <u.kleine-koenig@pengutronix.de> wrote:
> >> >> > From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> >> >> > Subject: [PATCH] gpiod: let get_optional return NULL in some cases with GPIOLIB disabled
> >> >> >
> >> >> > People disagree if gpiod_get_optional should return NULL or
> >> >> > ERR_PTR(-ENOSYS) if GPIOLIB is disabled. The argument for NULL is that
> >> >> > the person who decided to disable GPIOLIB is assumed to know that there
> >> >> > is no GPIO. The reason to stick to ERR_PTR(-ENOSYS) is that it might
> >> >> > introduce hard to debug problems if that decision is wrong.
> >> >> >
> >> >> > So this patch introduces a compromise and let gpiod_get_optional (and
> >> >> > its variants) return NULL if the device in question cannot have an
> >> >> > associated GPIO because it is neither instantiated by a device tree nor
> >> >> > by ACPI.
> >> >> >
> >> >> > This should handle most cases that are argued about.
> >> >> >
> >> >> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> >> >> > ---
> >> >> > include/linux/gpio/consumer.h | 55 ++++++++++++++++++++++++++++++++++++-------
> >> >> > 1 file changed, 46 insertions(+), 9 deletions(-)
> >> >> >
> >> >> > diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
> >> >> > index fb0fde686cb1..0ca29889290d 100644
> >> >> > --- a/include/linux/gpio/consumer.h
> >> >> > +++ b/include/linux/gpio/consumer.h
> >> >> > @@ -161,20 +161,48 @@ gpiod_get_index(struct device *dev,
> >> >> > return ERR_PTR(-ENOSYS);
> >> >> > }
> >> >> >
> >> >> > -static inline struct gpio_desc *__must_check
> >> >> > -gpiod_get_optional(struct device *dev, const char *con_id,
> >> >> > - enum gpiod_flags flags)
> >> >> > +static inline bool __gpiod_no_optional_possible(struct device *dev)
> >> >> > {
> >> >> > - return ERR_PTR(-ENOSYS);
> >> >> > + /*
> >> >> > + * gpiod_get_optional et al can only provide a GPIO if at least one of
> >> >> > + * the backends for specifing a GPIO is available. These are device
> >> >> > + * tree, ACPI and gpiolib's lookup tables. The latter isn't available if
> >> >> > + * GPIOLIB is disabled (which is the case here).
> >> >> > + * So if the provided device is unrelated to device tree and ACPI, we
> >> >> > + * can be sure that there is no optional GPIO and let gpiod_get_optional
> >> >> > + * safely return NULL.
> >> >> > + * Otherwise there is still a chance that there is no GPIO but we cannot
> >> >> > + * be sure without having to enable a part of GPIOLIB (i.e. the lookup
> >> >> > + * part). So lets play safe and return an error. (Though there are also
> >> >> > + * arguments that returning NULL then would be beneficial.)
> >> >> > + */
> >> >> > +
> >> >> > + if (IS_ENABLED(CONFIG_OF) && dev && dev->of_node)
> >> >> > + return false;
> >> >>
> >> >> At first sight, I though this was OK:
> >> >>
> >> >> 1. On ARM with DT, we can assume CONFIG_GPIOLOB=y.
> >> >>
> >> >> 2. I managed to configure an SH kernel with CONFIG_GPIOLOB=n, CONFIG_OF=y,
> >> >> and CONFIG_SERIAL_SH_SCI=y, but since SH boards with SH-SCI UARTs do
> >> >> not use DT (yet), the check for dev->of_node (false) should handle
> >> >> that.
> >> >>
> >> >> 3. However, I managed to do the same for h8300, which does use DT. Hence
> >> >> if mctrl_gpio would start relying on gpiod_get_optional(), this would
> >> >> break the sh-sci driver on h8300 :-(
> >> >> Note that h8300 doesn't have any GPIO drivers (yet?), so
> >> >> CONFIG_GPIPOLIB=n makes perfect sense!
> >>
> >> >> So I'm afraid the only option is to always return NULL, and put the
> >> >> responsability on the shoulders of the system integrator...
> >> >
> >> > The gpio lines could be provided by an i2c gpio adapter, right? So IMHO
> >> > you don't need platform gpios to justify -ENODEV. So I guess that's a
> >> > case where we don't come to an agreement.
> >>
> >> While you can enable I2C without further dependencies, no I2C GPIO expander
> >> will be offered... unless you have enabled CONFIG_GPIOLIB first.
> >
> > And that is expected, still the device tree could reference such a GPIO
> > and thus create a situation where Dmitry's and my judgement disagree.
>
> If the device tree references such a GPIO, and CONFIG_GPIOLIB is not set,
> the I2C GPIO expander device will not be bound.
> Frank Rowand's DT scripts (http://elinux.org/Device_Tree_frowand) will come
> to the rescue, and inform the user which driver(s) to enable.
>
> > So I think my suggestion is the best we could do now. It minimizes the
> > number of cases where we disagree. The next best thing would be to
> > implement that half gpiolib stuff (i.e. do the full lookup to be sure
> > there is no gpio) but this comes at a price: We need some time to
> > implement it and it adds a bit to the kernel size.
>
> So I still have to handle -ENOSYS in sh-sci.c, to avoid regressions...
How much would it hurt to enable GPIOLIB there now? I assume that the
platform has GPIOs and it's only an intermediate step that there is no
driver for it at present? At some point you have to bite the bullet.
Given that mctrl_gpio is a usage of GPIOs that might be now?
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
[toc] | [prev] | [next] | [standalone]
| From | Geert Uytterhoeven <geert@linux-m68k.org> |
|---|---|
| Date | 2017-03-24 09:40 +0100 |
| Message-ID | <tosVj-7gO-7@gated-at.bofh.it> |
| In reply to | #1608204 |
Hi Uwe,
On Fri, Mar 24, 2017 at 9:00 AM, Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
> From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> Subject: [PATCH] gpiod: let get_optional return NULL in some cases with GPIOLIB disabled
>
> People disagree if gpiod_get_optional should return NULL or
> ERR_PTR(-ENOSYS) if GPIOLIB is disabled. The argument for NULL is that
> the person who decided to disable GPIOLIB is assumed to know that there
> is no GPIO. The reason to stick to ERR_PTR(-ENOSYS) is that it might
> introduce hard to debug problems if that decision is wrong.
>
> So this patch introduces a compromise and let gpiod_get_optional (and
> its variants) return NULL if the device in question cannot have an
> associated GPIO because it is neither instantiated by a device tree nor
> by ACPI.
>
> This should handle most cases that are argued about.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> include/linux/gpio/consumer.h | 55 ++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 46 insertions(+), 9 deletions(-)
>
> diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
> index fb0fde686cb1..0ca29889290d 100644
> --- a/include/linux/gpio/consumer.h
> +++ b/include/linux/gpio/consumer.h
> @@ -161,20 +161,48 @@ gpiod_get_index(struct device *dev,
> return ERR_PTR(-ENOSYS);
> }
>
> -static inline struct gpio_desc *__must_check
> -gpiod_get_optional(struct device *dev, const char *con_id,
> - enum gpiod_flags flags)
> +static inline bool __gpiod_no_optional_possible(struct device *dev)
> {
> - return ERR_PTR(-ENOSYS);
> + /*
> + * gpiod_get_optional et al can only provide a GPIO if at least one of
> + * the backends for specifing a GPIO is available. These are device
> + * tree, ACPI and gpiolib's lookup tables. The latter isn't available if
> + * GPIOLIB is disabled (which is the case here).
> + * So if the provided device is unrelated to device tree and ACPI, we
> + * can be sure that there is no optional GPIO and let gpiod_get_optional
> + * safely return NULL.
> + * Otherwise there is still a chance that there is no GPIO but we cannot
> + * be sure without having to enable a part of GPIOLIB (i.e. the lookup
> + * part). So lets play safe and return an error. (Though there are also
> + * arguments that returning NULL then would be beneficial.)
> + */
> +
> + if (IS_ENABLED(CONFIG_OF) && dev && dev->of_node)
> + return false;
At first sight, I though this was OK:
1. On ARM with DT, we can assume CONFIG_GPIOLOB=y.
2. I managed to configure an SH kernel with CONFIG_GPIOLOB=n, CONFIG_OF=y,
and CONFIG_SERIAL_SH_SCI=y, but since SH boards with SH-SCI UARTs do
not use DT (yet), the check for dev->of_node (false) should handle
that.
3. However, I managed to do the same for h8300, which does use DT. Hence
if mctrl_gpio would start relying on gpiod_get_optional(), this would
break the sh-sci driver on h8300 :-(
Note that h8300 doesn't have any GPIO drivers (yet?), so
CONFIG_GPIPOLIB=n makes perfect sense!
So I'm afraid the only option is to always return NULL, and put the
responsability on the shoulders of the system integrator...
> + if (IS_ENABLED(CONFIG_ACPI) && dev && ACPI_COMPANION(dev))
> + return false;
No comments about the ACPI case.
> static inline struct gpio_desc *__must_check
> gpiod_get_index_optional(struct device *dev, const char *con_id,
> unsigned int index, enum gpiod_flags flags)
> {
> + if (__gpiod_no_optional_possible(dev))
> + return NULL;
> +
> return ERR_PTR(-ENOSYS);
Regardless of the above, given you use the exact same construct in four
locations, what about letting __gpiod_no_optional_possible() return the NULL
or ERR_PTR itself, and renaming it to e.g. __gpiod_no_optional_return_value()?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
[toc] | [prev] | [next] | [standalone]
| From | Linus Walleij <linus.walleij@linaro.org> |
|---|---|
| Date | 2017-03-24 10:00 +0100 |
| Message-ID | <toteG-7pT-29@gated-at.bofh.it> |
| In reply to | #1607823 |
On Thu, Mar 23, 2017 at 8:10 PM, Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
> On Thu, Mar 23, 2017 at 08:44:41AM -0700, Dmitry Torokhov wrote:
>> On Thu, Mar 23, 2017 at 07:43:25AM -0700, Dmitry Torokhov wrote:
>> > On Thu, Mar 23, 2017 at 02:41:53PM +0100, Linus Walleij wrote:
>> > > On Thu, Mar 23, 2017 at 1:34 PM, Uwe Kleine-König
>> > > <u.kleine-koenig@pengutronix.de> wrote:
>> > >
>> > > > Maybe we can make gpiod_get_optional look like this:
>> > > >
>> > > > if (!dev->of_node && isnt_a_acpi_device(dev) && !IS_ENABLED(GPIOLIB))
>> > > > return NULL;
>> > > > else
>> > > > return -ENOSYS;
>> > > >
>> > > > I don't know how isnt_a_acpi_device looks like, probably it involves
>> > > > CONFIG_ACPI and/or dev->acpi_node.
>> > > >
>> > > > This should be safe and still comfortable for legacy platforms, isn't it?
>> > >
>> > > I like the looks of this.
>> > >
>> > > Can we revert Dmitry's patch and apply something like this instead?
>> > >
>> > > Dmitry, how do you feel about this?
>> >
>> > I frankly do not see the point. It still makes driver code more complex
>
> Note that this code is in the gpio header, and not in driver code. So
> the driver just does
>
> gpiod = gpiod_get_optional(...)
> if (IS_ERR(gpiod))
> return PTR_ERR(gpiod);
>
> (as it is supposed to do now). I think that's nice.
It does look nice. Compare this to what we must do for optional regulators:
st->reg = devm_regulator_get_optional(&spi->dev, "vref");
if (IS_ERR(st->reg)) {
/* Any other error indicates that the regulator does exist */
if (PTR_ERR(st->reg) != -ENODEV)
return PTR_ERR(st->reg);
/* Use internal reference */
}
So for optional regulators we get -ENODEV if we don't have it,
and then proceed on an alternate path, such as using an internal
reference voltage.
However the fact that regulators and GPIO optionals are already
handled differently is creating "cognitive dissonance" or what I
should call it.
Yours,
Linus Walleij
[toc] | [prev] | [next] | [standalone]
| From | Dmitry Torokhov <dmitry.torokhov@gmail.com> |
|---|---|
| Date | 2017-03-23 17:00 +0100 |
| Subject | Re: [PATCH 4/4] tty/serial: sh-sci: remove uneeded IS_ERR_OR_NULL calls |
| Message-ID | <todjz-4ib-15@gated-at.bofh.it> |
| In reply to | #1607376 |
On Thu, Mar 23, 2017 at 12:11:06PM +0100, Uwe Kleine-König wrote: > Hello, > > On Thu, Mar 23, 2017 at 11:20:39AM +0100, Geert Uytterhoeven wrote: > > But having the error breaks setups where the GPIO is optional and does > > not exist. > > so the right way forward is to check harder in the situation where > -ENOSYS was returned before to determine if there is really no GPIO to > be used. "Oh, there are hints that there is no GPIO (GPIOLIB=n), so lets > assume there isn't." is wrong. > > Can we please properly fix the problem instead of papering over it? I think I once already said what would need to _attempt_ to fix it "properly". You would need to implement custom parsing of ACPI tables in GPIOLIB (what if they disable ACPI by mistake?), do the same for OF, call board's manufacturer hotline to ensure that they indeed did not forget to describe GPIOs, etc, etc. Or you could trust that person responsible for selecting kernel configuration has a clue, and if GPIOLIB is disabled it was disabled for a reason. -- Dmitry
[toc] | [prev] | [next] | [standalone]
| From | Linus Walleij <linus.walleij@linaro.org> |
|---|---|
| Date | 2017-03-23 14:40 +0100 |
| Message-ID | <tob85-2Oe-1@gated-at.bofh.it> |
| In reply to | #1607320 |
On Thu, Mar 23, 2017 at 11:10 AM, Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
> So you exchanged many obvious and easy to fix problems with a few hard
> ones. I don't agree that's a good idea, but you seem to be willing to
> try it. Good luck.
I think instead of going to sarcastic remarks you can say you NACK the
patch and suggest that it be reverted?
The problem I have here as maintainer is that both you and Dmitry are
very smart people and I have a great deal of trust invested in both of you.
When two valued contributors give me very different advice I get a bit
confused and maybe the best option is not to change anything at all
right now, and just revert Dmitry's patch.
git grep -e 'gpio.*optional(' | wc -l
gives 154 use sites outside drivers/gpio, so it is not impossible to fix
this if we want a good and strict order to it. I'm just a bit overworked to
do it myself right now.
What do you all say, is it better to revert Dmitry's patch and instead go
around and fix the consumers to do it correctly everywhere, after
hammering down the exact semantics?
Yours,
Linus Walleij
[toc] | [prev] | [next] | [standalone]
| From | Uwe Kleine-König <u.kleine-koenig@pengutronix.de> |
|---|---|
| Date | 2017-03-16 17:40 +0100 |
| Subject | Re: [PATCH 4/4] tty/serial: sh-sci: remove uneeded IS_ERR_OR_NULL calls |
| Message-ID | <tlGBt-10m-43@gated-at.bofh.it> |
| In reply to | #1602527 |
On Thu, Mar 16, 2017 at 04:18:52PM +0100, Linus Walleij wrote: > On Mon, Mar 6, 2017 at 11:02 AM, Uwe Kleine-König > <u.kleine-koenig@pengutronix.de> wrote: > > On Mon, Mar 06, 2017 at 10:53:27AM +0100, Geert Uytterhoeven wrote: > >> On Mon, Mar 6, 2017 at 10:30 AM, Uwe Kleine-König > > >> > I wouldn't want to code this in each driver (something like: > >> > > >> > if (IS_ENABLED(GPIOLIB) || device_is_instantiated_by_dt(dev) || device_is_instantiated_by_acpi(dev)) > >> > gpios = mctrl_gpio_init(...); > >> > else > >> > gpios = NULL; > >> > > >> > ). Putting this into GPIOLIB is the right approach, and so this is > >> > another argument for HALFGPIOLIB. This would fix mctrl_gpio_init en > >> > passant. > >> > >> Do we have platforms where DT=y || ACPI=y, but GPIOLIB=n? > >> Ah, x86 ;-) > > > > Yeah, and I think rm -r arch/x86 won't be acceptable :-) I assume you > > can also configure some arm or powerpc systems without GPIOLIB. > > > >> Anyway, for sh-sci.c, platforms either have DT and GPIOLIB, or they do not > >> need mctrl-gpio. > > > > So we're in agreement now that HALFGPIOLIB is the way to go? > > Linus, what do you think? > > OK modem lines over GPIO. > > So the problem is that GPIOLIB is needed (obviously) for mctrl_gpio_init() to > work properly, and then there are some stubs in > drivers/tty/serial/serial_mctrl_gpio.h > for !GPIOLIB. > > And this whole discussion is all about that !GPIOLIB case really, > whether DT, ACPI, SFI or board files machine data is used doesn't > really matter. > > We're talking about: > > > git grep mctrl_gpio_init > drivers/tty/serial/atmel_serial.c: atmel_port->gpios = > mctrl_gpio_init(&atmel_port->uart, 0); > drivers/tty/serial/clps711x.c: s->gpios = > mctrl_gpio_init_noauto(&pdev->dev, 0); > drivers/tty/serial/etraxfs-uart.c: up->gpios = > mctrl_gpio_init_noauto(&pdev->dev, 0); > drivers/tty/serial/imx.c: sport->gpios = mctrl_gpio_init(&sport->port, 0); > drivers/tty/serial/mxs-auart.c: s->gpios = mctrl_gpio_init_noauto(dev, 0); > drivers/tty/serial/sh-sci.c: sciport->gpios = > mctrl_gpio_init(&sciport->port, 0); > > Atmel, ARM, ETRAX, ARM, ARM, Super-H, all have GPIOLIB. > Right now no x86, correct? > > They actually all even do things like this in Kconfig: > > config SERIAL_ATMEL > (...) > select SERIAL_MCTRL_GPIO if GPIOLIB > > What stops us from removing all the stubs in > drivers/tty/serial/serial_mctrl_gpio.h > and just make SERIAL_MCTRL_GPIO depends on GPIOLIB? Would be ok for me, too. People seem to object to that though. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ |
[toc] | [prev] | [standalone]
Page 2 of 2 — ← Prev page 1 [2]
Back to top | Article view | linux.kernel
csiph-web