lkml.org 
[lkml]   [2017]   [Mar]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 4/4] tty/serial: sh-sci: remove uneeded IS_ERR_OR_NULL calls
    Hello Geert,

    On Fri, Mar 03, 2017 at 08:21:05PM +0100, Geert Uytterhoeven wrote:
    > > diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
    > > index 91e7dddbf72c..2f4cdd4e7b4f 100644
    > > --- a/drivers/tty/serial/sh-sci.c
    > > +++ b/drivers/tty/serial/sh-sci.c
    > > @@ -3022,7 +3022,7 @@ static int sci_probe_single(struct platform_device *dev,
    > > return ret;
    > >
    > > sciport->gpios = mctrl_gpio_init(&sciport->port, 0);
    > > - if (IS_ERR(sciport->gpios) && PTR_ERR(sciport->gpios) != -ENOSYS)
    > > + if (IS_ERR(sciport->gpios))
    > > return PTR_ERR(sciport->gpios);
    >
    > Now the sh-sci driver fails to probe on legacy platforms where GPIOLIB=n.
    > The check for -ENOSYS made it succeed before.

    That's right, intended and the only option that's save (for some
    definition of save; the obvious downside is that there is no
    /dev/tty$whatever for you).

    Ignoring -ENOSYS is only ok if your device doesn't have a cts-gpio. If
    it has, ignoring -ENOSYS hides bugs because the driver sends data while
    it shouldn't or cannot signal the other side that it should stop (or
    start) a transmission.

    So the options are:
    - enable GPIOLIB (maybe enforce that by letting the driver select it)
    - introduce a CONFIG_HALFGPIOLIB that makes gpiod_get_optional and
    mctrl_gpio_init return only then -ENOSYS if there is a gpio specified
    and NULL otherwise.

    > > Then mctrl_gpio_to_gpiod isn't called. I don't have a machine to test
    > > this, but I think currently this makes the machine barf to continue
    > > here because with sciport->gpios = ERR_PTR(-ENOSYS) calling
    > >
    > > mctrl_gpio_to_gpiod(sciport->gpios, ...)
    > >
    > > is a bad idea.
    >
    > If sciport->gpios == ERR_PTR(-ENOSYS), CONFIG_GPIOLIB is not enabled, the
    > feature is not available, and mctrl_gpio_to_gpiod() will not
    > dereference the error
    > pointer.

    Ah, makes sense.

    > >> Perhaps mctrl_gpio_to_gpiod() should always return NULL if !CONFIG_GPIOLIB?
    > >
    > > No, mctrl_gpio_to_gpiod is right. You are only supposed to call it if
    > > mctrl_gpio_init succeeded.
    >
    > Then I have to add checks for sciport->gpios == ERR_PTR(-ENOSYS)...

    No, ignoring -ENOSYS is wrong.

    Best regards
    Uwe

    --
    Pengutronix e.K. | Uwe Kleine-König |
    Industrial Linux Solutions | http://www.pengutronix.de/ |

    \
     
     \ /
      Last update: 2017-03-03 20:45    [W:3.262 / U:0.012 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site