lkml.org 
[lkml]   [2019]   [Jan]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v5 2/3] staging: greybus: arche-apb-ctrl.c: Switch to the gpio descriptor interface
    On Fri, Jan 11, 2019 at 09:41:34AM +0100, Johan Hovold wrote:
    > On Thu, Jan 10, 2019 at 11:21:04PM +0530, Nishad Kamdar wrote:
    > > Use the gpiod interface instead of the deprecated old non-descriptor
    > > interface while continuing to ignore gpio flags from device tree in
    > > functions "deassert_reset()" and "assert_reset()" for now.
    > >
    > > Signed-off-by: Nishad Kamdar <nishadkamdar@gmail.com>
    > > ---
    > > Changes in v5:
    > > - Change the commit message.
    > > - Restore the names of the gpio device-tree properties without
    > > the "-gpio" suffix.
    > > Changes in v4:
    > > - Use gpiod_set_raw_value() for deassert_reset() and
    > > assert_reset() as gpiod_set_value() will change the
    > > sematics of these calls by taking any gpio flags
    > > into account.
    > > - Remove some unnecesssary line breaks.
    > > - Restore 'spi_en' gpio check in fw_flashing_seq()
    > > as it is currently optional.
    > > Changes in v3:
    > > - Add this patch in a patchset.
    > > Changes in v2:
    > > - Resolved compilation errors.
    > > ---
    >
    > Also looks good now. You can add my
    >
    > Reviewed-by: Johan Hovold <johan@kernel.org>
    >
    > Found one really minor nit below, which doesn't really need to be fixed,
    > but since you may need to update the third patch, you might as well
    > consider this too.
    >
    Ok, I'll do that.
    > > /* It's not mandatory to support power management interface */
    > > - apb->pwroff_gpio = of_get_named_gpio(np, "pwr-off-gpios", 0);
    > > - if (apb->pwroff_gpio < 0) {
    > > - dev_err(dev, "failed to get power off gpio\n");
    > > - return apb->pwroff_gpio;
    > > - }
    > > - ret = devm_gpio_request_one(dev, apb->pwroff_gpio,
    > > - GPIOF_IN, "pwroff_n");
    > > - if (ret) {
    > > - dev_err(dev, "Failed requesting pwroff_n gpio %d\n",
    > > - apb->pwroff_gpio);
    > > + apb->pwroff = devm_gpiod_get_optional(dev, "pwr-off",
    > > + GPIOD_IN);
    >
    > Looks like you don't need to break the above statement any more either.
    >
    > > + if (IS_ERR(apb->pwroff)) {
    > > + ret = PTR_ERR(apb->pwroff);
    > > + dev_err(dev, "Failed requesting pwroff_n GPIO: %d\n", ret);
    > > return ret;
    > > }
    > >
    > > /* Do not make clock mandatory as of now (for DB3) */
    > > - apb->clk_en_gpio = of_get_named_gpio(np, "clock-en-gpio", 0);
    > > - if (apb->clk_en_gpio < 0) {
    > > - dev_warn(dev, "failed to get clock en gpio\n");
    > > - } else if (gpio_is_valid(apb->clk_en_gpio)) {
    > > - ret = devm_gpio_request_one(dev, apb->clk_en_gpio,
    > > - GPIOF_OUT_INIT_LOW, "apb_clk_en");
    > > - if (ret) {
    > > - dev_warn(dev, "Failed requesting APB clock en gpio %d\n",
    > > - apb->clk_en_gpio);
    > > - return ret;
    > > - }
    > > + apb->clk_en = devm_gpiod_get_optional(dev, "clock-en",
    > > + GPIOD_OUT_LOW);
    >
    > Same here?
    >
    Ok, I'll do that.

    Thanks and regards,
    Nishad
    > > + if (IS_ERR(apb->clk_en)) {
    > > + ret = PTR_ERR(apb->clk_en);
    > > + dev_err(dev, "Failed requesting APB clock en GPIO: %d\n", ret);
    > > + return ret;
    > > }
    >
    > Johan

    \
     
     \ /
      Last update: 2019-01-13 13:55    [W:3.186 / U:0.156 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site