diff mbox series

[v2,5/6] gpio: dwapb: Add debounce reference clock support

Message ID 20200323180632.14119-6-Sergey.Semin@baikalelectronics.ru
State New
Headers show
Series gpio: dwapb: Fix reference clocks usage | expand

Commit Message

Serge Semin March 23, 2020, 6:06 p.m. UTC 
From: Serge Semin <Sergey.Semin@baikalelectronics.ru>

Aside from the APB reference clock DW GPIO controller can have a
dedicated clock connected to setup a debounce time interval for
GPIO-based IRQs. Since this functionality is optional the corresponding
clock source is also optional. Due to this lets handle the debounce
clock in the same way as it has been developed for the APB reference
clock, but using the bulk request/enable-disable methods.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>

---

Changelog v2:
- Print error instead of info-message if APB/debounce clocks either
  failed to be acquired or couldn't be enabled.
---
 drivers/gpio/gpio-dwapb.c | 35 ++++++++++++++++++++++-------------
 1 file changed, 22 insertions(+), 13 deletions(-)

Comments

Andy Shevchenko March 23, 2020, 6:38 p.m. UTC | #1 
On Mon, Mar 23, 2020 at 09:06:31PM +0300, Sergey.Semin@baikalelectronics.ru wrote:
> From: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> 
> Aside from the APB reference clock DW GPIO controller can have a
> dedicated clock connected to setup a debounce time interval for
> GPIO-based IRQs. Since this functionality is optional the corresponding
> clock source is also optional. Due to this lets handle the debounce
> clock in the same way as it has been developed for the APB reference
> clock, but using the bulk request/enable-disable methods.

> +	if (err) {
> +		dev_err(gpio->dev, "Cannot reenable APB/Debounce clocks\n");
> +		return err;
> +	}

Yeah, this should be a separate change.

Otherwise looks good.

Also, did I miss the documentation update (bindings)?
Serge Semin March 23, 2020, 7:25 p.m. UTC | #2 
On Mon, Mar 23, 2020 at 08:38:37PM +0200, Andy Shevchenko wrote:
> On Mon, Mar 23, 2020 at 09:06:31PM +0300, Sergey.Semin@baikalelectronics.ru wrote:
> > From: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > 
> > Aside from the APB reference clock DW GPIO controller can have a
> > dedicated clock connected to setup a debounce time interval for
> > GPIO-based IRQs. Since this functionality is optional the corresponding
> > clock source is also optional. Due to this lets handle the debounce
> > clock in the same way as it has been developed for the APB reference
> > clock, but using the bulk request/enable-disable methods.
> 
> > +	if (err) {
> > +		dev_err(gpio->dev, "Cannot reenable APB/Debounce clocks\n");
> > +		return err;
> > +	}
> 
> Yeah, this should be a separate change.
> 

Linus didn't think it was necessary in v1.

> Otherwise looks good.
> 
> Also, did I miss the documentation update (bindings)?
> 

No, it's there: https://www.spinics.net/lists/devicetree/msg342655.html
You just haven't been in Cc there.

-Sergey

> -- 
> With Best Regards,
> Andy Shevchenko
> 
>
Andy Shevchenko March 23, 2020, 9:37 p.m. UTC | #3 
On Mon, Mar 23, 2020 at 9:28 PM Sergey Semin
<Sergey.Semin@baikalelectronics.ru> wrote:
> On Mon, Mar 23, 2020 at 08:38:37PM +0200, Andy Shevchenko wrote:
> > On Mon, Mar 23, 2020 at 09:06:31PM +0300, Sergey.Semin@baikalelectronics.ru wrote:
> > > From: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > >
> > > Aside from the APB reference clock DW GPIO controller can have a
> > > dedicated clock connected to setup a debounce time interval for
> > > GPIO-based IRQs. Since this functionality is optional the corresponding
> > > clock source is also optional. Due to this lets handle the debounce
> > > clock in the same way as it has been developed for the APB reference
> > > clock, but using the bulk request/enable-disable methods.
> >
> > > +   if (err) {
> > > +           dev_err(gpio->dev, "Cannot reenable APB/Debounce clocks\n");
> > > +           return err;
> > > +   }
> >
> > Yeah, this should be a separate change.
> >
>
> Linus didn't think it was necessary in v1.

Ah, okay!

> > Otherwise looks good.
diff mbox series

Patch

diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c
index 0c5abfa361e6..d2ed11510f3c 100644
--- a/drivers/gpio/gpio-dwapb.c
+++ b/drivers/gpio/gpio-dwapb.c
@@ -62,6 +62,8 @@ 
 #define GPIO_INTSTATUS_V2	0x3c
 #define GPIO_PORTA_EOI_V2	0x40
 
+#define DWAPB_NR_CLOCKS		2
+
 struct dwapb_gpio;
 
 #ifdef CONFIG_PM_SLEEP
@@ -97,7 +99,7 @@  struct dwapb_gpio {
 	struct irq_domain	*domain;
 	unsigned int		flags;
 	struct reset_control	*rst;
-	struct clk		*clk;
+	struct clk_bulk_data	clks[DWAPB_NR_CLOCKS];
 };
 
 static inline u32 gpio_reg_v2_convert(unsigned int offset)
@@ -689,16 +691,19 @@  static int dwapb_gpio_probe(struct platform_device *pdev)
 	if (IS_ERR(gpio->regs))
 		return PTR_ERR(gpio->regs);
 
-	/* Optional bus clock */
-	gpio->clk = devm_clk_get_optional(&pdev->dev, "bus");
-	if (IS_ERR(gpio->clk)) {
-		dev_err(&pdev->dev, "Cannot get APB clock\n");
-		return PTR_ERR(gpio->clk);
+	/* Optional bus and debounce clocks */
+	gpio->clks[0].id = "bus";
+	gpio->clks[1].id = "db";
+	err = devm_clk_bulk_get_optional(&pdev->dev, DWAPB_NR_CLOCKS,
+					 gpio->clks);
+	if (err) {
+		dev_err(&pdev->dev, "Cannot get APB/Debounce clocks\n");
+		return err;
 	}
 
-	err = clk_prepare_enable(gpio->clk);
+	err = clk_bulk_prepare_enable(DWAPB_NR_CLOCKS, gpio->clks);
 	if (err) {
-		dev_err(&pdev->dev, "Cannot enable APB clock\n");
+		dev_err(&pdev->dev, "Cannot enable APB/Debounce clocks\n");
 		return err;
 	}
 
@@ -727,7 +732,7 @@  static int dwapb_gpio_probe(struct platform_device *pdev)
 out_unregister:
 	dwapb_gpio_unregister(gpio);
 	dwapb_irq_teardown(gpio);
-	clk_disable_unprepare(gpio->clk);
+	clk_bulk_disable_unprepare(DWAPB_NR_CLOCKS, gpio->clks);
 
 	return err;
 }
@@ -739,7 +744,7 @@  static int dwapb_gpio_remove(struct platform_device *pdev)
 	dwapb_gpio_unregister(gpio);
 	dwapb_irq_teardown(gpio);
 	reset_control_assert(gpio->rst);
-	clk_disable_unprepare(gpio->clk);
+	clk_bulk_disable_unprepare(DWAPB_NR_CLOCKS, gpio->clks);
 
 	return 0;
 }
@@ -784,7 +789,7 @@  static int dwapb_gpio_suspend(struct device *dev)
 	}
 	spin_unlock_irqrestore(&gc->bgpio_lock, flags);
 
-	clk_disable_unprepare(gpio->clk);
+	clk_bulk_disable_unprepare(DWAPB_NR_CLOCKS, gpio->clks);
 
 	return 0;
 }
@@ -794,9 +799,13 @@  static int dwapb_gpio_resume(struct device *dev)
 	struct dwapb_gpio *gpio = dev_get_drvdata(dev);
 	struct gpio_chip *gc	= &gpio->ports[0].gc;
 	unsigned long flags;
-	int i;
+	int i, err;
 
-	clk_prepare_enable(gpio->clk);
+	err = clk_bulk_prepare_enable(DWAPB_NR_CLOCKS, gpio->clks);
+	if (err) {
+		dev_err(gpio->dev, "Cannot reenable APB/Debounce clocks\n");
+		return err;
+	}
 
 	spin_lock_irqsave(&gc->bgpio_lock, flags);
 	for (i = 0; i < gpio->nr_ports; i++) {