On Wed, Nov 9, 2016 at 11:57 AM, Sören Brinkmann
<soren.brinkmann@xxxxxxxxxx> wrote:
> On Wed, 2016-11-09 at 10:39:51 +0530, Shubhrajyoti Datta wrote:
>> Add basic clock support for xilinx gpio.
>>
>> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xxxxxxxxxx>
>> ---
>> v2 :
>> no change
>>
>> 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);
>
> The driver should use the clock-name documented in the binding to do the
> clock lookup.
My idea was to keep the clk name optional since there is only one clock.
Or do you think we should mandate the name if clk is provided.
>
>> + if (IS_ERR(chip->clk)) {
>> + if (PTR_ERR(chip->clk) == -ENOENT) {
>> + dev_info(&pdev->dev, "No clocks found for clk\n");
>
> This is pretty much just noise. The clocks property is optional. No need
> to be too verbose about that. It would be quite a lot of printing if
> every driver would report absent optional DT properties.
Ok will remove the print
>
> Sören
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
