On Sat, Dec 22, 2018 at 08:22:09PM +0530, Nishad Kamdar wrote:
> Use the gpiod interface instead of the deprecated old non-descriptor
> interface.
> 
> Signed-off-by: Nishad Kamdar <nishadkam...@gmail.com>
> ---
> 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.

Please also mention this in the commit message (i.e. that we continue to
ignore gpio flags from device tree for now).

>  - 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.
> ---

> @@ -75,11 +74,10 @@ static int coldboot_seq(struct platform_device *pdev)
>               return 0;
>  
>       /* Hold APB in reset state */
> -     assert_reset(apb->resetn_gpio);
> +     assert_reset(apb->resetn);
>  
> -     if (apb->state == ARCHE_PLATFORM_STATE_FW_FLASHING &&
> -         gpio_is_valid(apb->spi_en_gpio))
> -             devm_gpio_free(dev, apb->spi_en_gpio);
> +     if (apb->state == ARCHE_PLATFORM_STATE_FW_FLASHING && apb->spi_en)
> +             devm_gpiod_put(dev, apb->spi_en);
>  
>       /* Enable power to APB */
>       if (!IS_ERR(apb->vcore)) {
> @@ -101,13 +99,13 @@ static int coldboot_seq(struct platform_device *pdev)
>       apb_bootret_deassert(dev);
>  
>       /* On DB3 clock was not mandatory */
> -     if (gpio_is_valid(apb->clk_en_gpio))
> -             gpio_set_value(apb->clk_en_gpio, 1);
> +     if (apb->clk_en)
> +             gpiod_set_value(apb->clk_en, 1);
>  
>       usleep_range(100, 200);
>  
>       /* deassert reset to APB : Active-low signal */
> -     deassert_reset(apb->resetn_gpio);
> +     deassert_reset(apb->resetn);
>  
>       apb->state = ARCHE_PLATFORM_STATE_ACTIVE;
>  
> @@ -136,25 +134,25 @@ static int fw_flashing_seq(struct platform_device *pdev)
>               return ret;
>       }
>  
> -     if (gpio_is_valid(apb->spi_en_gpio)) {
> +     if (apb->spi_en) {
>               unsigned long flags;
>  
>               if (apb->spi_en_polarity_high)
> -                     flags = GPIOF_OUT_INIT_HIGH;
> +                     flags = GPIOD_OUT_HIGH;
>               else
> -                     flags = GPIOF_OUT_INIT_LOW;
> +                     flags = GPIOD_OUT_LOW;
>  
> -             ret = devm_gpio_request_one(dev, apb->spi_en_gpio,
> -                                         flags, "apb_spi_en");
> -             if (ret) {
> -                     dev_err(dev, "Failed requesting SPI bus en gpio %d\n",
> -                             apb->spi_en_gpio);
> +             apb->spi_en = devm_gpiod_get(dev, "gb,spi-en-gpio", flags);

I just noticed that you change the name of the device-tree property here
(and later in apb_ctrl_get_devtree_data()). How is that expected to
work without breaking current systems? This will be unavoidable at some
point, but must not be snuck into a patch like this without any comment.
Please keep the current names for now.

I do think you need to drop the "-gpio" suffix when requesting the gpio
though. Please double check to make sure.

Johan
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to