lkml.org 
[lkml]   [2016]   [Dec]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH 10/20] gpio: pca953x: Add optional reset gpio control
    On Thu, Dec 29, 2016 at 11:27 PM, Steve Longerbeam
    <slongerbeam@gmail.com> wrote:

    > Add optional reset-gpios pin control. If present, de-assert the
    > specified reset gpio pin to bring the chip out of reset.
    >
    > Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
    > Cc: Linus Walleij <linus.walleij@linaro.org>
    > Cc: Alexandre Courbot <gnurou@gmail.com>
    > Cc: linux-gpio@vger.kernel.org
    > Cc: linux-kernel@vger.kernel.org

    This seems like a separate patch from the other 19 patches so please send it
    separately so I can handle logistics easier in the future.


    > @@ -133,6 +134,7 @@ struct pca953x_chip {
    > const char *const *names;
    > unsigned long driver_data;
    > struct regulator *regulator;
    > + struct gpio_desc *reset_gpio;

    Why do you even keep this around in the device state container?

    As you only use it in the probe() function, use a local variable.

    The descriptor will be free():ed by the devm infrastructure anyways.

    > + /* see if we need to de-assert a reset pin */
    > + chip->reset_gpio = devm_gpiod_get_optional(&client->dev,
    > + "reset",
    > + GPIOD_OUT_LOW);
    > + if (IS_ERR(chip->reset_gpio)) {
    > + dev_err(&client->dev, "request for reset pin failed\n");
    > + return PTR_ERR(chip->reset_gpio);
    > + }

    Nice.

    > + if (chip->reset_gpio) {
    > + /* bring chip out of reset */
    > + dev_info(&client->dev, "releasing reset\n");
    > + gpiod_set_value(chip->reset_gpio, 0);
    > + }

    Is this really needed given that you set it low in the
    devm_gpiod_get_optional()?

    Yours,
    Linus Walleij

    \
     
     \ /
      Last update: 2016-12-30 14:18    [W:2.221 / U:0.252 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site