Re: [PATCH 2/2] gpio: xilinx: Add clock support

["Sören Brinkmann" <soren.brinkmann@xilinx.com>]

On Tue, 2016-11-08 at 10:43:10 +0530, Shubhrajyoti Datta wrote:
> Add basic clock support for xilinx gpio.
> 
> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> ---
>  drivers/gpio/gpio-xilinx.c |   22 ++++++++++++++++++++++
>  1 files changed, 22 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-xilinx.c b/drivers/gpio/gpio-xilinx.c
> index 14b2a62..923cab8 100644
> --- a/drivers/gpio/gpio-xilinx.c
> +++ b/drivers/gpio/gpio-xilinx.c
> @@ -13,6 +13,7 @@
>   */
>  
>  #include <linux/bitops.h>
> +#include <linux/clk.h>
>  #include <linux/init.h>
>  #include <linux/errno.h>
>  #include <linux/module.h>
> @@ -45,6 +46,7 @@
>   * @gpio_state: GPIO state shadow register
>   * @gpio_dir: GPIO direction shadow register
>   * @gpio_lock: Lock used for synchronization
> + * @clk: Clock resource for this controller
>   */
>  struct xgpio_instance {
>  	struct of_mm_gpio_chip mmchip;
> @@ -52,6 +54,7 @@ struct xgpio_instance {
>  	u32 gpio_state[2];
>  	u32 gpio_dir[2];
>  	spinlock_t gpio_lock[2];
> +	struct clk *clk;
>  };
>  
>  static inline int xgpio_index(struct xgpio_instance *chip, int gpio)
> @@ -282,6 +285,7 @@ static int xgpio_remove(struct platform_device *pdev)
>  	struct xgpio_instance *chip = platform_get_drvdata(pdev);
>  
>  	of_mm_gpiochip_remove(&chip->mmchip);
> +	clk_disable_unprepare(chip->clk);
>  
>  	return 0;
>  }
> @@ -307,6 +311,23 @@ static int xgpio_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, chip);
>  
> +	/* Retrieve GPIO clock */
> +	chip->clk = devm_clk_get(&pdev->dev, NULL);
> +	if (IS_ERR(chip->clk)) {
> +		if (PTR_ERR(chip->clk) == -ENOENT) {
> +			dev_info(&pdev->dev, "No clocks found for clk\n");
> +			chip->clk = NULL;
> +		} else {
> +			dev_err(&pdev->dev, "axi clock error\n");
> +			return PTR_ERR(chip->clk);
> +		}
> +	}

In the bindings you document 'clocks' as required, but it's handled as
optional here. Things should be consistent and given that the existing
binding doesn't require clocks, we're probably stuck with having the
clock optional.

	Sören