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 <nishadkam...@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 <jo...@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
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to