Linux GPIO subsystem development
 help / color / mirror / Atom feed
* [PATCH AUTOSEL 4.19 144/158] gpiolib: Fix references to gpiod_[gs]et_*value_cansleep() variants
From: Sasha Levin @ 2019-07-15 14:17 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: Geert Uytterhoeven, Linus Walleij, Sasha Levin, linux-gpio
In-Reply-To: <20190715141809.8445-1-sashal@kernel.org>

From: Geert Uytterhoeven <geert+renesas@glider.be>

[ Upstream commit 3285170f28a850638794cdfe712eb6d93e51e706 ]

Commit 372e722ea4dd4ca1 ("gpiolib: use descriptors internally") renamed
the functions to use a "gpiod" prefix, and commit 79a9becda8940deb
("gpiolib: export descriptor-based GPIO interface") introduced the "raw"
variants, but both changes forgot to update the comments.

Readd a similar reference to gpiod_set_value(), which was accidentally
removed by commit 1e77fc82110ac36f ("gpio: Add missing open drain/source
handling to gpiod_set_value_cansleep()").

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Link: https://lore.kernel.org/r/20190701142738.25219-1-geert+renesas@glider.be
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/gpio/gpiolib.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index fd713326dcfc..4a48c7c47709 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2877,7 +2877,7 @@ int gpiod_get_array_value_complex(bool raw, bool can_sleep,
 int gpiod_get_raw_value(const struct gpio_desc *desc)
 {
 	VALIDATE_DESC(desc);
-	/* Should be using gpio_get_value_cansleep() */
+	/* Should be using gpiod_get_raw_value_cansleep() */
 	WARN_ON(desc->gdev->chip->can_sleep);
 	return gpiod_get_raw_value_commit(desc);
 }
@@ -2898,7 +2898,7 @@ int gpiod_get_value(const struct gpio_desc *desc)
 	int value;
 
 	VALIDATE_DESC(desc);
-	/* Should be using gpio_get_value_cansleep() */
+	/* Should be using gpiod_get_value_cansleep() */
 	WARN_ON(desc->gdev->chip->can_sleep);
 
 	value = gpiod_get_raw_value_commit(desc);
@@ -3123,7 +3123,7 @@ int gpiod_set_array_value_complex(bool raw, bool can_sleep,
 void gpiod_set_raw_value(struct gpio_desc *desc, int value)
 {
 	VALIDATE_DESC_VOID(desc);
-	/* Should be using gpiod_set_value_cansleep() */
+	/* Should be using gpiod_set_raw_value_cansleep() */
 	WARN_ON(desc->gdev->chip->can_sleep);
 	gpiod_set_raw_value_commit(desc, value);
 }
@@ -3164,6 +3164,7 @@ static void gpiod_set_value_nocheck(struct gpio_desc *desc, int value)
 void gpiod_set_value(struct gpio_desc *desc, int value)
 {
 	VALIDATE_DESC_VOID(desc);
+	/* Should be using gpiod_set_value_cansleep() */
 	WARN_ON(desc->gdev->chip->can_sleep);
 	gpiod_set_value_nocheck(desc, value);
 }
-- 
2.20.1


^ permalink raw reply related

* [PATCH AUTOSEL 4.14 095/105] gpiolib: Fix references to gpiod_[gs]et_*value_cansleep() variants
From: Sasha Levin @ 2019-07-15 14:28 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: Geert Uytterhoeven, Linus Walleij, Sasha Levin, linux-gpio
In-Reply-To: <20190715142839.9896-1-sashal@kernel.org>

From: Geert Uytterhoeven <geert+renesas@glider.be>

[ Upstream commit 3285170f28a850638794cdfe712eb6d93e51e706 ]

Commit 372e722ea4dd4ca1 ("gpiolib: use descriptors internally") renamed
the functions to use a "gpiod" prefix, and commit 79a9becda8940deb
("gpiolib: export descriptor-based GPIO interface") introduced the "raw"
variants, but both changes forgot to update the comments.

Readd a similar reference to gpiod_set_value(), which was accidentally
removed by commit 1e77fc82110ac36f ("gpio: Add missing open drain/source
handling to gpiod_set_value_cansleep()").

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Link: https://lore.kernel.org/r/20190701142738.25219-1-geert+renesas@glider.be
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/gpio/gpiolib.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 21062cb6b85f..3db0a9b0d259 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2480,7 +2480,7 @@ static int _gpiod_get_raw_value(const struct gpio_desc *desc)
 int gpiod_get_raw_value(const struct gpio_desc *desc)
 {
 	VALIDATE_DESC(desc);
-	/* Should be using gpio_get_value_cansleep() */
+	/* Should be using gpiod_get_raw_value_cansleep() */
 	WARN_ON(desc->gdev->chip->can_sleep);
 	return _gpiod_get_raw_value(desc);
 }
@@ -2501,7 +2501,7 @@ int gpiod_get_value(const struct gpio_desc *desc)
 	int value;
 
 	VALIDATE_DESC(desc);
-	/* Should be using gpio_get_value_cansleep() */
+	/* Should be using gpiod_get_value_cansleep() */
 	WARN_ON(desc->gdev->chip->can_sleep);
 
 	value = _gpiod_get_raw_value(desc);
@@ -2670,7 +2670,7 @@ void gpiod_set_array_value_complex(bool raw, bool can_sleep,
 void gpiod_set_raw_value(struct gpio_desc *desc, int value)
 {
 	VALIDATE_DESC_VOID(desc);
-	/* Should be using gpiod_set_value_cansleep() */
+	/* Should be using gpiod_set_raw_value_cansleep() */
 	WARN_ON(desc->gdev->chip->can_sleep);
 	_gpiod_set_raw_value(desc, value);
 }
-- 
2.20.1


^ permalink raw reply related

* [PATCH AUTOSEL 4.9 67/73] gpiolib: Fix references to gpiod_[gs]et_*value_cansleep() variants
From: Sasha Levin @ 2019-07-15 14:36 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: Geert Uytterhoeven, Linus Walleij, Sasha Levin, linux-gpio
In-Reply-To: <20190715143629.10893-1-sashal@kernel.org>

From: Geert Uytterhoeven <geert+renesas@glider.be>

[ Upstream commit 3285170f28a850638794cdfe712eb6d93e51e706 ]

Commit 372e722ea4dd4ca1 ("gpiolib: use descriptors internally") renamed
the functions to use a "gpiod" prefix, and commit 79a9becda8940deb
("gpiolib: export descriptor-based GPIO interface") introduced the "raw"
variants, but both changes forgot to update the comments.

Readd a similar reference to gpiod_set_value(), which was accidentally
removed by commit 1e77fc82110ac36f ("gpio: Add missing open drain/source
handling to gpiod_set_value_cansleep()").

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Link: https://lore.kernel.org/r/20190701142738.25219-1-geert+renesas@glider.be
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/gpio/gpiolib.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 9e2fe12c2858..a3251faa3ed8 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2411,7 +2411,7 @@ static int _gpiod_get_raw_value(const struct gpio_desc *desc)
 int gpiod_get_raw_value(const struct gpio_desc *desc)
 {
 	VALIDATE_DESC(desc);
-	/* Should be using gpio_get_value_cansleep() */
+	/* Should be using gpiod_get_raw_value_cansleep() */
 	WARN_ON(desc->gdev->chip->can_sleep);
 	return _gpiod_get_raw_value(desc);
 }
@@ -2432,7 +2432,7 @@ int gpiod_get_value(const struct gpio_desc *desc)
 	int value;
 
 	VALIDATE_DESC(desc);
-	/* Should be using gpio_get_value_cansleep() */
+	/* Should be using gpiod_get_value_cansleep() */
 	WARN_ON(desc->gdev->chip->can_sleep);
 
 	value = _gpiod_get_raw_value(desc);
@@ -2608,7 +2608,7 @@ void gpiod_set_array_value_complex(bool raw, bool can_sleep,
 void gpiod_set_raw_value(struct gpio_desc *desc, int value)
 {
 	VALIDATE_DESC_VOID(desc);
-	/* Should be using gpiod_set_value_cansleep() */
+	/* Should be using gpiod_set_raw_value_cansleep() */
 	WARN_ON(desc->gdev->chip->can_sleep);
 	_gpiod_set_raw_value(desc, value);
 }
-- 
2.20.1


^ permalink raw reply related

* [PATCH AUTOSEL 4.9 34/73] gpio: omap: ensure irq is enabled before wakeup
From: Sasha Levin @ 2019-07-15 14:35 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: Russell King, Grygorii Strashko, Tony Lindgren, Linus Walleij,
	Sasha Levin, linux-omap, linux-gpio
In-Reply-To: <20190715143629.10893-1-sashal@kernel.org>

From: Russell King <rmk+kernel@armlinux.org.uk>

[ Upstream commit c859e0d479b3b4f6132fc12637c51e01492f31f6 ]

Documentation states:

  NOTE: There must be a correlation between the wake-up enable and
  interrupt-enable registers. If a GPIO pin has a wake-up configured
  on it, it must also have the corresponding interrupt enabled (on
  one of the two interrupt lines).

Ensure that this condition is always satisfied by enabling the detection
events after enabling the interrupt, and disabling the detection before
disabling the interrupt.  This ensures interrupt/wakeup events can not
happen until both the wakeup and interrupt enables correlate.

If we do any clearing, clear between the interrupt enable/disable and
trigger setting.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
Tested-by: Tony Lindgren <tony@atomide.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/gpio/gpio-omap.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index bd12b433f964..fc841ce24db7 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -786,9 +786,9 @@ static void omap_gpio_irq_shutdown(struct irq_data *d)
 
 	raw_spin_lock_irqsave(&bank->lock, flags);
 	bank->irq_usage &= ~(BIT(offset));
-	omap_set_gpio_irqenable(bank, offset, 0);
-	omap_clear_gpio_irqstatus(bank, offset);
 	omap_set_gpio_triggering(bank, offset, IRQ_TYPE_NONE);
+	omap_clear_gpio_irqstatus(bank, offset);
+	omap_set_gpio_irqenable(bank, offset, 0);
 	if (!LINE_USED(bank->mod_usage, offset))
 		omap_clear_gpio_debounce(bank, offset);
 	omap_disable_gpio_module(bank, offset);
@@ -830,8 +830,8 @@ static void omap_gpio_mask_irq(struct irq_data *d)
 	unsigned long flags;
 
 	raw_spin_lock_irqsave(&bank->lock, flags);
-	omap_set_gpio_irqenable(bank, offset, 0);
 	omap_set_gpio_triggering(bank, offset, IRQ_TYPE_NONE);
+	omap_set_gpio_irqenable(bank, offset, 0);
 	raw_spin_unlock_irqrestore(&bank->lock, flags);
 }
 
@@ -843,9 +843,6 @@ static void omap_gpio_unmask_irq(struct irq_data *d)
 	unsigned long flags;
 
 	raw_spin_lock_irqsave(&bank->lock, flags);
-	if (trigger)
-		omap_set_gpio_triggering(bank, offset, trigger);
-
 	omap_set_gpio_irqenable(bank, offset, 1);
 
 	/*
@@ -853,9 +850,13 @@ static void omap_gpio_unmask_irq(struct irq_data *d)
 	 * is cleared, thus after the handler has run. OMAP4 needs this done
 	 * after enabing the interrupt to clear the wakeup status.
 	 */
-	if (bank->level_mask & BIT(offset))
+	if (bank->regs->leveldetect0 && bank->regs->wkup_en &&
+	    trigger & (IRQ_TYPE_LEVEL_HIGH | IRQ_TYPE_LEVEL_LOW))
 		omap_clear_gpio_irqstatus(bank, offset);
 
+	if (trigger)
+		omap_set_gpio_triggering(bank, offset, trigger);
+
 	raw_spin_unlock_irqrestore(&bank->lock, flags);
 }
 
-- 
2.20.1


^ permalink raw reply related

* [PATCH AUTOSEL 4.9 33/73] gpio: omap: fix lack of irqstatus_raw0 for OMAP4
From: Sasha Levin @ 2019-07-15 14:35 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: Russell King, Grygorii Strashko, Tony Lindgren, Linus Walleij,
	Sasha Levin, linux-omap, linux-gpio
In-Reply-To: <20190715143629.10893-1-sashal@kernel.org>

From: Russell King <rmk+kernel@armlinux.org.uk>

[ Upstream commit 64ea3e9094a1f13b96c33244a3fb3a0f45690bd2 ]

Commit 384ebe1c2849 ("gpio/omap: Add DT support to GPIO driver") added
the register definition tables to the gpio-omap driver. Subsequently to
that commit, commit 4e962e8998cc ("gpio/omap: remove cpu_is_omapxxxx()
checks from *_runtime_resume()") added definitions for irqstatus_raw*
registers to the legacy OMAP4 definitions, but missed the DT
definitions.

This causes an unintentional change of behaviour for the 1.101 errata
workaround on OMAP4 platforms. Fix this oversight.

Fixes: 4e962e8998cc ("gpio/omap: remove cpu_is_omapxxxx() checks from *_runtime_resume()")
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
Tested-by: Tony Lindgren <tony@atomide.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/gpio/gpio-omap.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 038882183bdf..bd12b433f964 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -1585,6 +1585,8 @@ static struct omap_gpio_reg_offs omap4_gpio_regs = {
 	.clr_dataout =		OMAP4_GPIO_CLEARDATAOUT,
 	.irqstatus =		OMAP4_GPIO_IRQSTATUS0,
 	.irqstatus2 =		OMAP4_GPIO_IRQSTATUS1,
+	.irqstatus_raw0 =	OMAP4_GPIO_IRQSTATUSRAW0,
+	.irqstatus_raw1 =	OMAP4_GPIO_IRQSTATUSRAW1,
 	.irqenable =		OMAP4_GPIO_IRQSTATUSSET0,
 	.irqenable2 =		OMAP4_GPIO_IRQSTATUSSET1,
 	.set_irqenable =	OMAP4_GPIO_IRQSTATUSSET0,
-- 
2.20.1


^ permalink raw reply related

* [PATCH AUTOSEL 4.4 27/53] gpio: omap: ensure irq is enabled before wakeup
From: Sasha Levin @ 2019-07-15 14:45 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: Russell King, Grygorii Strashko, Tony Lindgren, Linus Walleij,
	Sasha Levin, linux-omap, linux-gpio
In-Reply-To: <20190715144535.11636-1-sashal@kernel.org>

From: Russell King <rmk+kernel@armlinux.org.uk>

[ Upstream commit c859e0d479b3b4f6132fc12637c51e01492f31f6 ]

Documentation states:

  NOTE: There must be a correlation between the wake-up enable and
  interrupt-enable registers. If a GPIO pin has a wake-up configured
  on it, it must also have the corresponding interrupt enabled (on
  one of the two interrupt lines).

Ensure that this condition is always satisfied by enabling the detection
events after enabling the interrupt, and disabling the detection before
disabling the interrupt.  This ensures interrupt/wakeup events can not
happen until both the wakeup and interrupt enables correlate.

If we do any clearing, clear between the interrupt enable/disable and
trigger setting.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
Tested-by: Tony Lindgren <tony@atomide.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/gpio/gpio-omap.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index f23136825a6e..6e65c02baad1 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -821,9 +821,9 @@ static void omap_gpio_irq_shutdown(struct irq_data *d)
 
 	raw_spin_lock_irqsave(&bank->lock, flags);
 	bank->irq_usage &= ~(BIT(offset));
-	omap_set_gpio_irqenable(bank, offset, 0);
-	omap_clear_gpio_irqstatus(bank, offset);
 	omap_set_gpio_triggering(bank, offset, IRQ_TYPE_NONE);
+	omap_clear_gpio_irqstatus(bank, offset);
+	omap_set_gpio_irqenable(bank, offset, 0);
 	if (!LINE_USED(bank->mod_usage, offset))
 		omap_clear_gpio_debounce(bank, offset);
 	omap_disable_gpio_module(bank, offset);
@@ -865,8 +865,8 @@ static void omap_gpio_mask_irq(struct irq_data *d)
 	unsigned long flags;
 
 	raw_spin_lock_irqsave(&bank->lock, flags);
-	omap_set_gpio_irqenable(bank, offset, 0);
 	omap_set_gpio_triggering(bank, offset, IRQ_TYPE_NONE);
+	omap_set_gpio_irqenable(bank, offset, 0);
 	raw_spin_unlock_irqrestore(&bank->lock, flags);
 }
 
@@ -878,9 +878,6 @@ static void omap_gpio_unmask_irq(struct irq_data *d)
 	unsigned long flags;
 
 	raw_spin_lock_irqsave(&bank->lock, flags);
-	if (trigger)
-		omap_set_gpio_triggering(bank, offset, trigger);
-
 	omap_set_gpio_irqenable(bank, offset, 1);
 
 	/*
@@ -888,9 +885,13 @@ static void omap_gpio_unmask_irq(struct irq_data *d)
 	 * is cleared, thus after the handler has run. OMAP4 needs this done
 	 * after enabing the interrupt to clear the wakeup status.
 	 */
-	if (bank->level_mask & BIT(offset))
+	if (bank->regs->leveldetect0 && bank->regs->wkup_en &&
+	    trigger & (IRQ_TYPE_LEVEL_HIGH | IRQ_TYPE_LEVEL_LOW))
 		omap_clear_gpio_irqstatus(bank, offset);
 
+	if (trigger)
+		omap_set_gpio_triggering(bank, offset, trigger);
+
 	raw_spin_unlock_irqrestore(&bank->lock, flags);
 }
 
-- 
2.20.1


^ permalink raw reply related

* [PATCH AUTOSEL 4.4 26/53] gpio: omap: fix lack of irqstatus_raw0 for OMAP4
From: Sasha Levin @ 2019-07-15 14:45 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: Russell King, Grygorii Strashko, Tony Lindgren, Linus Walleij,
	Sasha Levin, linux-omap, linux-gpio
In-Reply-To: <20190715144535.11636-1-sashal@kernel.org>

From: Russell King <rmk+kernel@armlinux.org.uk>

[ Upstream commit 64ea3e9094a1f13b96c33244a3fb3a0f45690bd2 ]

Commit 384ebe1c2849 ("gpio/omap: Add DT support to GPIO driver") added
the register definition tables to the gpio-omap driver. Subsequently to
that commit, commit 4e962e8998cc ("gpio/omap: remove cpu_is_omapxxxx()
checks from *_runtime_resume()") added definitions for irqstatus_raw*
registers to the legacy OMAP4 definitions, but missed the DT
definitions.

This causes an unintentional change of behaviour for the 1.101 errata
workaround on OMAP4 platforms. Fix this oversight.

Fixes: 4e962e8998cc ("gpio/omap: remove cpu_is_omapxxxx() checks from *_runtime_resume()")
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
Tested-by: Tony Lindgren <tony@atomide.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/gpio/gpio-omap.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index c8c49b1d5f9f..f23136825a6e 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -1611,6 +1611,8 @@ static struct omap_gpio_reg_offs omap4_gpio_regs = {
 	.clr_dataout =		OMAP4_GPIO_CLEARDATAOUT,
 	.irqstatus =		OMAP4_GPIO_IRQSTATUS0,
 	.irqstatus2 =		OMAP4_GPIO_IRQSTATUS1,
+	.irqstatus_raw0 =	OMAP4_GPIO_IRQSTATUSRAW0,
+	.irqstatus_raw1 =	OMAP4_GPIO_IRQSTATUSRAW1,
 	.irqenable =		OMAP4_GPIO_IRQSTATUSSET0,
 	.irqenable2 =		OMAP4_GPIO_IRQSTATUSSET1,
 	.set_irqenable =	OMAP4_GPIO_IRQSTATUSSET0,
-- 
2.20.1


^ permalink raw reply related

* [PATCH AUTOSEL 4.14 044/105] gpio: omap: fix lack of irqstatus_raw0 for OMAP4
From: Sasha Levin @ 2019-07-15 14:27 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: Russell King, Grygorii Strashko, Tony Lindgren, Linus Walleij,
	Sasha Levin, linux-omap, linux-gpio
In-Reply-To: <20190715142839.9896-1-sashal@kernel.org>

From: Russell King <rmk+kernel@armlinux.org.uk>

[ Upstream commit 64ea3e9094a1f13b96c33244a3fb3a0f45690bd2 ]

Commit 384ebe1c2849 ("gpio/omap: Add DT support to GPIO driver") added
the register definition tables to the gpio-omap driver. Subsequently to
that commit, commit 4e962e8998cc ("gpio/omap: remove cpu_is_omapxxxx()
checks from *_runtime_resume()") added definitions for irqstatus_raw*
registers to the legacy OMAP4 definitions, but missed the DT
definitions.

This causes an unintentional change of behaviour for the 1.101 errata
workaround on OMAP4 platforms. Fix this oversight.

Fixes: 4e962e8998cc ("gpio/omap: remove cpu_is_omapxxxx() checks from *_runtime_resume()")
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
Tested-by: Tony Lindgren <tony@atomide.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/gpio/gpio-omap.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 83c6843db50c..abe54659b1f4 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -1604,6 +1604,8 @@ static struct omap_gpio_reg_offs omap4_gpio_regs = {
 	.clr_dataout =		OMAP4_GPIO_CLEARDATAOUT,
 	.irqstatus =		OMAP4_GPIO_IRQSTATUS0,
 	.irqstatus2 =		OMAP4_GPIO_IRQSTATUS1,
+	.irqstatus_raw0 =	OMAP4_GPIO_IRQSTATUSRAW0,
+	.irqstatus_raw1 =	OMAP4_GPIO_IRQSTATUSRAW1,
 	.irqenable =		OMAP4_GPIO_IRQSTATUSSET0,
 	.irqenable2 =		OMAP4_GPIO_IRQSTATUSSET1,
 	.set_irqenable =	OMAP4_GPIO_IRQSTATUSSET0,
-- 
2.20.1


^ permalink raw reply related

* [PATCH AUTOSEL 4.14 045/105] gpio: omap: ensure irq is enabled before wakeup
From: Sasha Levin @ 2019-07-15 14:27 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: Russell King, Grygorii Strashko, Tony Lindgren, Linus Walleij,
	Sasha Levin, linux-omap, linux-gpio
In-Reply-To: <20190715142839.9896-1-sashal@kernel.org>

From: Russell King <rmk+kernel@armlinux.org.uk>

[ Upstream commit c859e0d479b3b4f6132fc12637c51e01492f31f6 ]

Documentation states:

  NOTE: There must be a correlation between the wake-up enable and
  interrupt-enable registers. If a GPIO pin has a wake-up configured
  on it, it must also have the corresponding interrupt enabled (on
  one of the two interrupt lines).

Ensure that this condition is always satisfied by enabling the detection
events after enabling the interrupt, and disabling the detection before
disabling the interrupt.  This ensures interrupt/wakeup events can not
happen until both the wakeup and interrupt enables correlate.

If we do any clearing, clear between the interrupt enable/disable and
trigger setting.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
Tested-by: Tony Lindgren <tony@atomide.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/gpio/gpio-omap.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index abe54659b1f4..47e537a91dd8 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -790,9 +790,9 @@ static void omap_gpio_irq_shutdown(struct irq_data *d)
 
 	raw_spin_lock_irqsave(&bank->lock, flags);
 	bank->irq_usage &= ~(BIT(offset));
-	omap_set_gpio_irqenable(bank, offset, 0);
-	omap_clear_gpio_irqstatus(bank, offset);
 	omap_set_gpio_triggering(bank, offset, IRQ_TYPE_NONE);
+	omap_clear_gpio_irqstatus(bank, offset);
+	omap_set_gpio_irqenable(bank, offset, 0);
 	if (!LINE_USED(bank->mod_usage, offset))
 		omap_clear_gpio_debounce(bank, offset);
 	omap_disable_gpio_module(bank, offset);
@@ -834,8 +834,8 @@ static void omap_gpio_mask_irq(struct irq_data *d)
 	unsigned long flags;
 
 	raw_spin_lock_irqsave(&bank->lock, flags);
-	omap_set_gpio_irqenable(bank, offset, 0);
 	omap_set_gpio_triggering(bank, offset, IRQ_TYPE_NONE);
+	omap_set_gpio_irqenable(bank, offset, 0);
 	raw_spin_unlock_irqrestore(&bank->lock, flags);
 }
 
@@ -847,9 +847,6 @@ static void omap_gpio_unmask_irq(struct irq_data *d)
 	unsigned long flags;
 
 	raw_spin_lock_irqsave(&bank->lock, flags);
-	if (trigger)
-		omap_set_gpio_triggering(bank, offset, trigger);
-
 	omap_set_gpio_irqenable(bank, offset, 1);
 
 	/*
@@ -857,9 +854,13 @@ static void omap_gpio_unmask_irq(struct irq_data *d)
 	 * is cleared, thus after the handler has run. OMAP4 needs this done
 	 * after enabing the interrupt to clear the wakeup status.
 	 */
-	if (bank->level_mask & BIT(offset))
+	if (bank->regs->leveldetect0 && bank->regs->wkup_en &&
+	    trigger & (IRQ_TYPE_LEVEL_HIGH | IRQ_TYPE_LEVEL_LOW))
 		omap_clear_gpio_irqstatus(bank, offset);
 
+	if (trigger)
+		omap_set_gpio_triggering(bank, offset, trigger);
+
 	raw_spin_unlock_irqrestore(&bank->lock, flags);
 }
 
-- 
2.20.1


^ permalink raw reply related

* [PATCH AUTOSEL 4.19 059/158] gpio: omap: fix lack of irqstatus_raw0 for OMAP4
From: Sasha Levin @ 2019-07-15 14:16 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: Russell King, Grygorii Strashko, Tony Lindgren, Linus Walleij,
	Sasha Levin, linux-omap, linux-gpio
In-Reply-To: <20190715141809.8445-1-sashal@kernel.org>

From: Russell King <rmk+kernel@armlinux.org.uk>

[ Upstream commit 64ea3e9094a1f13b96c33244a3fb3a0f45690bd2 ]

Commit 384ebe1c2849 ("gpio/omap: Add DT support to GPIO driver") added
the register definition tables to the gpio-omap driver. Subsequently to
that commit, commit 4e962e8998cc ("gpio/omap: remove cpu_is_omapxxxx()
checks from *_runtime_resume()") added definitions for irqstatus_raw*
registers to the legacy OMAP4 definitions, but missed the DT
definitions.

This causes an unintentional change of behaviour for the 1.101 errata
workaround on OMAP4 platforms. Fix this oversight.

Fixes: 4e962e8998cc ("gpio/omap: remove cpu_is_omapxxxx() checks from *_runtime_resume()")
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
Tested-by: Tony Lindgren <tony@atomide.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/gpio/gpio-omap.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 6fa430d98517..9254bcf7f647 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -1687,6 +1687,8 @@ static struct omap_gpio_reg_offs omap4_gpio_regs = {
 	.clr_dataout =		OMAP4_GPIO_CLEARDATAOUT,
 	.irqstatus =		OMAP4_GPIO_IRQSTATUS0,
 	.irqstatus2 =		OMAP4_GPIO_IRQSTATUS1,
+	.irqstatus_raw0 =	OMAP4_GPIO_IRQSTATUSRAW0,
+	.irqstatus_raw1 =	OMAP4_GPIO_IRQSTATUSRAW1,
 	.irqenable =		OMAP4_GPIO_IRQSTATUSSET0,
 	.irqenable2 =		OMAP4_GPIO_IRQSTATUSSET1,
 	.set_irqenable =	OMAP4_GPIO_IRQSTATUSSET0,
-- 
2.20.1


^ permalink raw reply related

* [PATCH AUTOSEL 5.1 081/219] gpio: omap: Fix lost edge wake-up interrupts
From: Sasha Levin @ 2019-07-15 14:01 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: Tony Lindgren, Aaro Koskinen, Grygorii Strashko, Keerthy,
	Ladislav Michl, Peter Ujfalusi, Russell King, Tero Kristo,
	Linus Walleij, Sasha Levin, linux-omap, linux-gpio
In-Reply-To: <20190715140341.6443-1-sashal@kernel.org>

From: Tony Lindgren <tony@atomide.com>

[ Upstream commit a522f1d0c381c42f3ace13b8bbeeccabdd6d2e5c ]

If an edge interrupt triggers while entering idle just before we save
GPIO datain register to saved_datain, the triggered GPIO will not be
noticed on wake-up. This is because the saved_datain and GPIO datain
are the same on wake-up in omap_gpio_unidle(). Let's fix this by
ignoring any pending edge interrupts for saved_datain.

This issue affects only idle states where the GPIO module internal
wake-up path is operational. For deeper idle states where the GPIO
module gets powered off, Linux generic wakeirqs must be used for
the padconf wake-up events with pinctrl-single driver. For examples,
please see "interrupts-extended" dts usage in many drivers.

This issue can be somewhat easily reproduced by pinging an idle system
with smsc911x Ethernet interface configured IRQ_TYPE_EDGE_FALLING. At
some point the smsc911x interrupts will just stop triggering. Also if
WLCORE WLAN is used with EDGE interrupt like it's documentation specifies,
we can see lost interrupts without this patch.

Note that in the long run we may be able to cancel entering idle by
returning an error in gpio_omap_cpu_notifier() on pending interrupts.
But let's fix the bug first.

Also note that because of the recent clean-up efforts this patch does
not apply directly to older kernels. This does fix a long term issue
though, and can be backported as needed.

Cc: Aaro Koskinen <aaro.koskinen@iki.fi>
Cc: Grygorii Strashko <grygorii.strashko@ti.com>
Cc: Keerthy <j-keerthy@ti.com>
Cc: Ladislav Michl <ladis@linux-mips.org>
Cc: Peter Ujfalusi <peter.ujfalusi@ti.com>
Cc: Russell King <rmk+kernel@armlinux.org.uk>
Cc: Tero Kristo <t-kristo@ti.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/gpio/gpio-omap.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 233245bc693c..1ddc872b4e4b 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -1455,7 +1455,7 @@ static void omap_gpio_idle(struct gpio_bank *bank, bool may_lose_context)
 {
 	struct device *dev = bank->chip.parent;
 	void __iomem *base = bank->base;
-	u32 nowake;
+	u32 mask, nowake;
 
 	bank->saved_datain = readl_relaxed(base + bank->regs->datain);
 
@@ -1465,6 +1465,16 @@ static void omap_gpio_idle(struct gpio_bank *bank, bool may_lose_context)
 	if (!bank->enabled_non_wakeup_gpios)
 		goto update_gpio_context_count;
 
+	/* Check for pending EDGE_FALLING, ignore EDGE_BOTH */
+	mask = bank->enabled_non_wakeup_gpios & bank->context.fallingdetect;
+	mask &= ~bank->context.risingdetect;
+	bank->saved_datain |= mask;
+
+	/* Check for pending EDGE_RISING, ignore EDGE_BOTH */
+	mask = bank->enabled_non_wakeup_gpios & bank->context.risingdetect;
+	mask &= ~bank->context.fallingdetect;
+	bank->saved_datain &= ~mask;
+
 	if (!may_lose_context)
 		goto update_gpio_context_count;
 
-- 
2.20.1


^ permalink raw reply related

* [PATCH AUTOSEL 5.1 079/219] gpio: omap: ensure irq is enabled before wakeup
From: Sasha Levin @ 2019-07-15 14:01 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: Russell King, Grygorii Strashko, Tony Lindgren, Linus Walleij,
	Sasha Levin, linux-omap, linux-gpio
In-Reply-To: <20190715140341.6443-1-sashal@kernel.org>

From: Russell King <rmk+kernel@armlinux.org.uk>

[ Upstream commit c859e0d479b3b4f6132fc12637c51e01492f31f6 ]

Documentation states:

  NOTE: There must be a correlation between the wake-up enable and
  interrupt-enable registers. If a GPIO pin has a wake-up configured
  on it, it must also have the corresponding interrupt enabled (on
  one of the two interrupt lines).

Ensure that this condition is always satisfied by enabling the detection
events after enabling the interrupt, and disabling the detection before
disabling the interrupt.  This ensures interrupt/wakeup events can not
happen until both the wakeup and interrupt enables correlate.

If we do any clearing, clear between the interrupt enable/disable and
trigger setting.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
Tested-by: Tony Lindgren <tony@atomide.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/gpio/gpio-omap.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 0708e50a27f0..233245bc693c 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -838,9 +838,9 @@ static void omap_gpio_irq_shutdown(struct irq_data *d)
 
 	raw_spin_lock_irqsave(&bank->lock, flags);
 	bank->irq_usage &= ~(BIT(offset));
-	omap_set_gpio_irqenable(bank, offset, 0);
-	omap_clear_gpio_irqstatus(bank, offset);
 	omap_set_gpio_triggering(bank, offset, IRQ_TYPE_NONE);
+	omap_clear_gpio_irqstatus(bank, offset);
+	omap_set_gpio_irqenable(bank, offset, 0);
 	if (!LINE_USED(bank->mod_usage, offset))
 		omap_clear_gpio_debounce(bank, offset);
 	omap_disable_gpio_module(bank, offset);
@@ -876,8 +876,8 @@ static void omap_gpio_mask_irq(struct irq_data *d)
 	unsigned long flags;
 
 	raw_spin_lock_irqsave(&bank->lock, flags);
-	omap_set_gpio_irqenable(bank, offset, 0);
 	omap_set_gpio_triggering(bank, offset, IRQ_TYPE_NONE);
+	omap_set_gpio_irqenable(bank, offset, 0);
 	raw_spin_unlock_irqrestore(&bank->lock, flags);
 }
 
@@ -889,9 +889,6 @@ static void omap_gpio_unmask_irq(struct irq_data *d)
 	unsigned long flags;
 
 	raw_spin_lock_irqsave(&bank->lock, flags);
-	if (trigger)
-		omap_set_gpio_triggering(bank, offset, trigger);
-
 	omap_set_gpio_irqenable(bank, offset, 1);
 
 	/*
@@ -899,9 +896,13 @@ static void omap_gpio_unmask_irq(struct irq_data *d)
 	 * is cleared, thus after the handler has run. OMAP4 needs this done
 	 * after enabing the interrupt to clear the wakeup status.
 	 */
-	if (bank->level_mask & BIT(offset))
+	if (bank->regs->leveldetect0 && bank->regs->wkup_en &&
+	    trigger & (IRQ_TYPE_LEVEL_HIGH | IRQ_TYPE_LEVEL_LOW))
 		omap_clear_gpio_irqstatus(bank, offset);
 
+	if (trigger)
+		omap_set_gpio_triggering(bank, offset, trigger);
+
 	raw_spin_unlock_irqrestore(&bank->lock, flags);
 }
 
-- 
2.20.1


^ permalink raw reply related

* [PATCH AUTOSEL 5.1 078/219] gpio: omap: fix lack of irqstatus_raw0 for OMAP4
From: Sasha Levin @ 2019-07-15 14:01 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: Russell King, Grygorii Strashko, Tony Lindgren, Linus Walleij,
	Sasha Levin, linux-omap, linux-gpio
In-Reply-To: <20190715140341.6443-1-sashal@kernel.org>

From: Russell King <rmk+kernel@armlinux.org.uk>

[ Upstream commit 64ea3e9094a1f13b96c33244a3fb3a0f45690bd2 ]

Commit 384ebe1c2849 ("gpio/omap: Add DT support to GPIO driver") added
the register definition tables to the gpio-omap driver. Subsequently to
that commit, commit 4e962e8998cc ("gpio/omap: remove cpu_is_omapxxxx()
checks from *_runtime_resume()") added definitions for irqstatus_raw*
registers to the legacy OMAP4 definitions, but missed the DT
definitions.

This causes an unintentional change of behaviour for the 1.101 errata
workaround on OMAP4 platforms. Fix this oversight.

Fixes: 4e962e8998cc ("gpio/omap: remove cpu_is_omapxxxx() checks from *_runtime_resume()")
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
Tested-by: Tony Lindgren <tony@atomide.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/gpio/gpio-omap.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index fafd79438bbf..0708e50a27f0 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -1728,6 +1728,8 @@ static struct omap_gpio_reg_offs omap4_gpio_regs = {
 	.clr_dataout =		OMAP4_GPIO_CLEARDATAOUT,
 	.irqstatus =		OMAP4_GPIO_IRQSTATUS0,
 	.irqstatus2 =		OMAP4_GPIO_IRQSTATUS1,
+	.irqstatus_raw0 =	OMAP4_GPIO_IRQSTATUSRAW0,
+	.irqstatus_raw1 =	OMAP4_GPIO_IRQSTATUSRAW1,
 	.irqenable =		OMAP4_GPIO_IRQSTATUSSET0,
 	.irqenable2 =		OMAP4_GPIO_IRQSTATUSSET1,
 	.set_irqenable =	OMAP4_GPIO_IRQSTATUSSET0,
-- 
2.20.1


^ permalink raw reply related

* [PATCH AUTOSEL 5.2 093/249] gpio: omap: Fix lost edge wake-up interrupts
From: Sasha Levin @ 2019-07-15 13:44 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: Tony Lindgren, Aaro Koskinen, Grygorii Strashko, Keerthy,
	Ladislav Michl, Peter Ujfalusi, Russell King, Tero Kristo,
	Linus Walleij, Sasha Levin, linux-omap, linux-gpio
In-Reply-To: <20190715134655.4076-1-sashal@kernel.org>

From: Tony Lindgren <tony@atomide.com>

[ Upstream commit a522f1d0c381c42f3ace13b8bbeeccabdd6d2e5c ]

If an edge interrupt triggers while entering idle just before we save
GPIO datain register to saved_datain, the triggered GPIO will not be
noticed on wake-up. This is because the saved_datain and GPIO datain
are the same on wake-up in omap_gpio_unidle(). Let's fix this by
ignoring any pending edge interrupts for saved_datain.

This issue affects only idle states where the GPIO module internal
wake-up path is operational. For deeper idle states where the GPIO
module gets powered off, Linux generic wakeirqs must be used for
the padconf wake-up events with pinctrl-single driver. For examples,
please see "interrupts-extended" dts usage in many drivers.

This issue can be somewhat easily reproduced by pinging an idle system
with smsc911x Ethernet interface configured IRQ_TYPE_EDGE_FALLING. At
some point the smsc911x interrupts will just stop triggering. Also if
WLCORE WLAN is used with EDGE interrupt like it's documentation specifies,
we can see lost interrupts without this patch.

Note that in the long run we may be able to cancel entering idle by
returning an error in gpio_omap_cpu_notifier() on pending interrupts.
But let's fix the bug first.

Also note that because of the recent clean-up efforts this patch does
not apply directly to older kernels. This does fix a long term issue
though, and can be backported as needed.

Cc: Aaro Koskinen <aaro.koskinen@iki.fi>
Cc: Grygorii Strashko <grygorii.strashko@ti.com>
Cc: Keerthy <j-keerthy@ti.com>
Cc: Ladislav Michl <ladis@linux-mips.org>
Cc: Peter Ujfalusi <peter.ujfalusi@ti.com>
Cc: Russell King <rmk+kernel@armlinux.org.uk>
Cc: Tero Kristo <t-kristo@ti.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/gpio/gpio-omap.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 746aa9caf934..8591c410ecaa 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -1275,13 +1275,23 @@ static void omap_gpio_idle(struct gpio_bank *bank, bool may_lose_context)
 {
 	struct device *dev = bank->chip.parent;
 	void __iomem *base = bank->base;
-	u32 nowake;
+	u32 mask, nowake;
 
 	bank->saved_datain = readl_relaxed(base + bank->regs->datain);
 
 	if (!bank->enabled_non_wakeup_gpios)
 		goto update_gpio_context_count;
 
+	/* Check for pending EDGE_FALLING, ignore EDGE_BOTH */
+	mask = bank->enabled_non_wakeup_gpios & bank->context.fallingdetect;
+	mask &= ~bank->context.risingdetect;
+	bank->saved_datain |= mask;
+
+	/* Check for pending EDGE_RISING, ignore EDGE_BOTH */
+	mask = bank->enabled_non_wakeup_gpios & bank->context.risingdetect;
+	mask &= ~bank->context.fallingdetect;
+	bank->saved_datain &= ~mask;
+
 	if (!may_lose_context)
 		goto update_gpio_context_count;
 
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH v4 7/8] dt-bindings: arm: sunxi: add binding for Lichee Zero Plus core board
From: Rob Herring @ 2019-07-15 17:03 UTC (permalink / raw)
  To: Icenowy Zheng
  Cc: Maxime Ripard, Chen-Yu Tsai, Linus Walleij, devicetree,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	linux-kernel@vger.kernel.org, linux-clk, open list:GPIO SUBSYSTEM,
	linux-sunxi
In-Reply-To: <20190713034634.44585-8-icenowy@aosc.io>

On Fri, Jul 12, 2019 at 9:49 PM Icenowy Zheng <icenowy@aosc.io> wrote:
>
> The Lichee Zero Plus is a core board made by Sipeed, with a microUSB
> connector on it, TF slot or WSON8 SD chip, optional eMMC or SPI Flash.
> It has a gold finger connector for expansion, and UART is available from
> reserved pins w/ 2.54mm pitch. The board can use either SoChip S3 or
> Allwinner V3L SoCs.
>
> Add the device tree binding of the basic version of the core board --
> w/o eMMC or SPI Flash, w/ TF slot or WSON8 SD, and use S3 SoC.
>
> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> ---
> No changes since v3.
>
> Patch introduced in v2.
>
>  Documentation/devicetree/bindings/arm/sunxi.yaml | 5 +++++
>  1 file changed, 5 insertions(+)

Reviewed-by: Rob Herring <robh@kernel.org>


Rob

^ permalink raw reply

* [PATCH] Revert "gpio/spi: Fix spi-gpio regression on active high CS"
From: Linus Walleij @ 2019-07-15 20:45 UTC (permalink / raw)
  To: linux-gpio; +Cc: Bartosz Golaszewski, Linus Walleij, linux-spi, stable

This reverts commit fbbf145a0e0a0177e089c52275fbfa55763e7d1d.

It seems I was misguided in my fixup, which was working at the
time but did not work on the final v5.2.

The patch tried to avoid a quirk the gpiolib code not to treat
"spi-gpio" CS gpios "special" by enforcing them to be active
low, in the belief that since the "spi-gpio" driver was
parsing the device tree on its own, it did not care to inspect
the "spi-cs-high" attribute on the device nodes.

That's wrong. The SPI core was inspecting them inside the
of_spi_parse_dt() funtion and setting SPI_CS_HIGH on the
nodes, and the driver inspected this flag when driving the
line.

As of now, the core handles the GPIO and it will consistently
set the GPIO descriptor to 1 to enable CS, strictly requireing
the gpiolib to invert it. And the gpiolib should indeed
enforce active low on the CS line.

Device trees should of course put the right flag on the GPIO
handles, but it used to not matter. If we don't enforce active
low on "gpio-gpio" we may run into ABI backward compatibility
issues, so revert this.

Cc: linux-spi@vger.kernel.org
Cc: stable@vger.kernel.org
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
I am sorry that this at one point fixed a problem for me, it
doesn't anymore and I don't know why it ever did. :(
---
 drivers/gpio/gpiolib-of.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index f974075ff00e..a8f02f551d6b 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -118,15 +118,8 @@ static void of_gpio_flags_quirks(struct device_node *np,
 	 * Legacy handling of SPI active high chip select. If we have a
 	 * property named "cs-gpios" we need to inspect the child node
 	 * to determine if the flags should have inverted semantics.
-	 *
-	 * This does not apply to an SPI device named "spi-gpio", because
-	 * these have traditionally obtained their own GPIOs by parsing
-	 * the device tree directly and did not respect any "spi-cs-high"
-	 * property on the SPI bus children.
 	 */
-	if (IS_ENABLED(CONFIG_SPI_MASTER) &&
-	    !strcmp(propname, "cs-gpios") &&
-	    !of_device_is_compatible(np, "spi-gpio") &&
+	if (IS_ENABLED(CONFIG_SPI_MASTER) && !strcmp(propname, "cs-gpios") &&
 	    of_property_read_bool(np, "cs-gpios")) {
 		struct device_node *child;
 		u32 cs;
-- 
2.21.0


^ permalink raw reply related

* [PATCH] dt-bindings: pinctrl: aspeed: Fix 'compatible' schema errors
From: Rob Herring @ 2019-07-15 22:37 UTC (permalink / raw)
  To: devicetree, Linus Walleij, Andrew Jeffery
  Cc: Joel Stanley, linux-aspeed, linux-gpio, linux-arm-kernel

The Aspeed pinctl schema have errors in the 'compatible' schema:

Documentation/devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.yaml: \
properties:compatible:enum: ['aspeed', 'ast2400-pinctrl', 'aspeed', 'g4-pinctrl'] has non-unique elements
Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.yaml: \
properties:compatible:enum: ['aspeed', 'ast2500-pinctrl', 'aspeed', 'g5-pinctrl'] has non-unique elements

Flow style sequences have to be quoted if the vales contain ','. Fix
this by using the more common one line per entry formatting.

Fixes: 0a617de16730 ("dt-bindings: pinctrl: aspeed: Convert AST2500 bindings to json-schema")
Fixes: 07457937bb5c ("dt-bindings: pinctrl: aspeed: Convert AST2400 bindings to json-schema")
Cc: Andrew Jeffery <andrew@aj.id.au>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Joel Stanley <joel@jms.id.au>
Cc: linux-aspeed@lists.ozlabs.org
Cc: linux-gpio@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Signed-off-by: Rob Herring <robh@kernel.org>
---
 .../devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.yaml   | 4 +++-
 .../devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.yaml   | 4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.yaml
index 61a110a7db8a..125599a2dc5e 100644
--- a/Documentation/devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.yaml
@@ -22,7 +22,9 @@ description: |+
 
 properties:
   compatible:
-    enum: [ aspeed,ast2400-pinctrl, aspeed,g4-pinctrl ]
+    enum:
+      - aspeed,ast2400-pinctrl
+      - aspeed,g4-pinctrl
 
 patternProperties:
   '^.*$':
diff --git a/Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.yaml
index cf561bd55128..a464cfa0cba3 100644
--- a/Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.yaml
@@ -22,7 +22,9 @@ description: |+
 
 properties:
   compatible:
-    enum: [ aspeed,ast2500-pinctrl, aspeed,g5-pinctrl ]
+    enum:
+      - aspeed,ast2500-pinctrl
+      - aspeed,g5-pinctrl
   aspeed,external-nodes:
     minItems: 2
     maxItems: 2
-- 
2.20.1


^ permalink raw reply related

* [PATCH] dt-bindings: pinctrl: aspeed: Fix AST2500 example errors
From: Rob Herring @ 2019-07-15 22:48 UTC (permalink / raw)
  To: devicetree, Linus Walleij, Andrew Jeffery
  Cc: Joel Stanley, linux-aspeed, linux-gpio, linux-arm-kernel

The schema examples are now validated against the schema itself. The
AST2500 pinctrl schema has a couple of errors:

Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.example.dt.yaml: \
example-0: $nodename:0: 'example-0' does not match '^(bus|soc|axi|ahb|apb)(@[0-9a-f]+)?$'
Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.example.dt.yaml: \
pinctrl: aspeed,external-nodes: [[1, 2]] is too short

Fixes: 0a617de16730 ("dt-bindings: pinctrl: aspeed: Convert AST2500 bindings to json-schema")
Cc: Andrew Jeffery <andrew@aj.id.au>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Joel Stanley <joel@jms.id.au>
Cc: linux-aspeed@lists.ozlabs.org
Cc: linux-gpio@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Signed-off-by: Rob Herring <robh@kernel.org>
---
 .../devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.yaml  | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.yaml
index a464cfa0cba3..3e6d85318577 100644
--- a/Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.yaml
@@ -76,9 +76,6 @@ required:
 
 examples:
   - |
-    compatible = "simple-bus";
-    ranges;
-
     apb {
         compatible = "simple-bus";
         #address-cells = <1>;
@@ -91,7 +88,7 @@ examples:
 
             pinctrl: pinctrl {
                 compatible = "aspeed,g5-pinctrl";
-                aspeed,external-nodes = <&gfx &lhc>;
+                aspeed,external-nodes = <&gfx>, <&lhc>;
 
                 pinctrl_i2c3_default: i2c3_default {
                     function = "I2C3";
-- 
2.20.1


^ permalink raw reply related

* [PATCH] dt-bindings: Ensure child nodes are of type 'object'
From: Rob Herring @ 2019-07-15 23:04 UTC (permalink / raw)
  To: devicetree
  Cc: linux-kernel, Maxime Ripard, Chen-Yu Tsai, David Woodhouse,
	Brian Norris, Marek Vasut, Miquel Raynal, Richard Weinberger,
	Vignesh Raghavendra, Linus Walleij, Maxime Coquelin,
	Alexandre Torgue, Mark Brown, linux-mtd, linux-gpio, linux-stm32,
	linux-spi

Properties which are child node definitions need to have an explict
type. Otherwise, a matching (DT) property can silently match when an
error is desired. Fix this up tree-wide. Once this is fixed, the
meta-schema will enforce this on any child node definitions.

Cc: Maxime Ripard <maxime.ripard@bootlin.com>
Cc: Chen-Yu Tsai <wens@csie.org>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Brian Norris <computersforpeace@gmail.com>
Cc: Marek Vasut <marek.vasut@gmail.com>
Cc: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Richard Weinberger <richard@nod.at>
Cc: Vignesh Raghavendra <vigneshr@ti.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: linux-mtd@lists.infradead.org
Cc: linux-gpio@vger.kernel.org
Cc: linux-stm32@st-md-mailman.stormreply.com
Cc: linux-spi@vger.kernel.org
Signed-off-by: Rob Herring <robh@kernel.org>
---
Please ack. I will take this via the DT tree.

Rob

 .../devicetree/bindings/bus/allwinner,sun8i-a23-rsb.yaml       | 1 +
 .../devicetree/bindings/mtd/allwinner,sun4i-a10-nand.yaml      | 1 +
 Documentation/devicetree/bindings/mtd/nand-controller.yaml     | 1 +
 .../devicetree/bindings/pinctrl/st,stm32-pinctrl.yaml          | 3 +++
 .../devicetree/bindings/spi/allwinner,sun4i-a10-spi.yaml       | 1 +
 .../devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml       | 1 +
 6 files changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/bus/allwinner,sun8i-a23-rsb.yaml b/Documentation/devicetree/bindings/bus/allwinner,sun8i-a23-rsb.yaml
index fc2f63860cc8..be32f087c529 100644
--- a/Documentation/devicetree/bindings/bus/allwinner,sun8i-a23-rsb.yaml
+++ b/Documentation/devicetree/bindings/bus/allwinner,sun8i-a23-rsb.yaml
@@ -42,6 +42,7 @@ properties:
 
 patternProperties:
   "^.*@[0-9a-fA-F]+$":
+    type: object
     properties:
       reg:
         maxItems: 1
diff --git a/Documentation/devicetree/bindings/mtd/allwinner,sun4i-a10-nand.yaml b/Documentation/devicetree/bindings/mtd/allwinner,sun4i-a10-nand.yaml
index e5a411518be1..b5b3cf5b1ac2 100644
--- a/Documentation/devicetree/bindings/mtd/allwinner,sun4i-a10-nand.yaml
+++ b/Documentation/devicetree/bindings/mtd/allwinner,sun4i-a10-nand.yaml
@@ -55,6 +55,7 @@ patternProperties:
   "^pinctrl-[0-9]+$": true
 
   "^nand@[a-f0-9]+$":
+    type: object
     properties:
       reg:
         minimum: 0
diff --git a/Documentation/devicetree/bindings/mtd/nand-controller.yaml b/Documentation/devicetree/bindings/mtd/nand-controller.yaml
index 199ba5ac2a06..d261b7096c69 100644
--- a/Documentation/devicetree/bindings/mtd/nand-controller.yaml
+++ b/Documentation/devicetree/bindings/mtd/nand-controller.yaml
@@ -40,6 +40,7 @@ properties:
 
 patternProperties:
   "^nand@[a-f0-9]$":
+    type: object
     properties:
       reg:
         description:
diff --git a/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.yaml
index 06c4b66c3ee6..3ac5d2088e49 100644
--- a/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.yaml
@@ -55,6 +55,7 @@ properties:
 
 patternProperties:
   '^gpio@[0-9a-f]*$':
+    type: object
     properties:
       gpio-controller: true
       '#gpio-cells':
@@ -113,8 +114,10 @@ patternProperties:
       - st,bank-name
 
   '-[0-9]*$':
+    type: object
     patternProperties:
       '^pins':
+        type: object
         description: |
           A pinctrl node should contain at least one subnode representing the
           pinctrl group available on the machine. Each subnode will list the
diff --git a/Documentation/devicetree/bindings/spi/allwinner,sun4i-a10-spi.yaml b/Documentation/devicetree/bindings/spi/allwinner,sun4i-a10-spi.yaml
index c374fd4923a6..6d1329c28170 100644
--- a/Documentation/devicetree/bindings/spi/allwinner,sun4i-a10-spi.yaml
+++ b/Documentation/devicetree/bindings/spi/allwinner,sun4i-a10-spi.yaml
@@ -50,6 +50,7 @@ properties:
 
 patternProperties:
   "^.*@[0-9a-f]+":
+    type: object
     properties:
       reg:
         items:
diff --git a/Documentation/devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml b/Documentation/devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml
index bda7a5befd8b..f36c46d236d7 100644
--- a/Documentation/devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml
+++ b/Documentation/devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml
@@ -55,6 +55,7 @@ properties:
 
 patternProperties:
   "^.*@[0-9a-f]+":
+    type: object
     properties:
       reg:
         items:
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH] dt-bindings: pinctrl: aspeed: Fix 'compatible' schema errors
From: Joel Stanley @ 2019-07-15 23:16 UTC (permalink / raw)
  To: Rob Herring
  Cc: devicetree, Linus Walleij, Andrew Jeffery, linux-aspeed,
	linux-gpio, Linux ARM
In-Reply-To: <20190715223725.12924-1-robh@kernel.org>

On Mon, 15 Jul 2019 at 22:37, Rob Herring <robh@kernel.org> wrote:
>
> The Aspeed pinctl schema have errors in the 'compatible' schema:
>
> Documentation/devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.yaml: \
> properties:compatible:enum: ['aspeed', 'ast2400-pinctrl', 'aspeed', 'g4-pinctrl'] has non-unique elements
> Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.yaml: \
> properties:compatible:enum: ['aspeed', 'ast2500-pinctrl', 'aspeed', 'g5-pinctrl'] has non-unique elements
>
> Flow style sequences have to be quoted if the vales contain ','. Fix
> this by using the more common one line per entry formatting.

>
>  properties:
>    compatible:
> -    enum: [ aspeed,ast2400-pinctrl, aspeed,g4-pinctrl ]
> +    enum:
> +      - aspeed,ast2400-pinctrl
> +      - aspeed,g4-pinctrl

Thanks for the fix. However, we've standardised on the first form for
all of our device trees, so we can drop the second compatible string
from the bindings.

I think Andrew already has a patch cooking to do this.

Cheers,

Joel

^ permalink raw reply

* Re: [PATCH V5 11/18] clk: tegra210: Add support for Tegra210 clocks
From: Sowjanya Komatineni @ 2019-07-16  0:35 UTC (permalink / raw)
  To: Dmitry Osipenko, thierry.reding, jonathanh, tglx, jason,
	marc.zyngier, linus.walleij, stefan, mark.rutland
  Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
	josephl, talho, linux-tegra, linux-kernel, mperttunen, spatra,
	robh+dt, devicetree
In-Reply-To: <e9d4bc0e-fd5d-ae02-2d67-86c7f7c9620f@gmail.com>


On 7/14/19 2:41 PM, Dmitry Osipenko wrote:
> 13.07.2019 8:54, Sowjanya Komatineni пишет:
>> On 6/29/19 8:10 AM, Dmitry Osipenko wrote:
>>> 28.06.2019 5:12, Sowjanya Komatineni пишет:
>>>> This patch adds system suspend and resume support for Tegra210
>>>> clocks.
>>>>
>>>> All the CAR controller settings are lost on suspend when core power
>>>> goes off.
>>>>
>>>> This patch has implementation for saving and restoring all the PLLs
>>>> and clocks context during system suspend and resume to have the
>>>> clocks back to same state for normal operation.
>>>>
>>>> Acked-by: Thierry Reding <treding@nvidia.com>
>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>> ---
>>>>    drivers/clk/tegra/clk-tegra210.c | 115
>>>> ++++++++++++++++++++++++++++++++++++++-
>>>>    drivers/clk/tegra/clk.c          |  14 +++++
>>>>    drivers/clk/tegra/clk.h          |   1 +
>>>>    3 files changed, 127 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/clk/tegra/clk-tegra210.c
>>>> b/drivers/clk/tegra/clk-tegra210.c
>>>> index 1c08c53482a5..1b839544e086 100644
>>>> --- a/drivers/clk/tegra/clk-tegra210.c
>>>> +++ b/drivers/clk/tegra/clk-tegra210.c
>>>> @@ -9,10 +9,12 @@
>>>>    #include <linux/clkdev.h>
>>>>    #include <linux/of.h>
>>>>    #include <linux/of_address.h>
>>>> +#include <linux/of_platform.h>
>>>>    #include <linux/delay.h>
>>>>    #include <linux/export.h>
>>>>    #include <linux/mutex.h>
>>>>    #include <linux/clk/tegra.h>
>>>> +#include <linux/syscore_ops.h>
>>>>    #include <dt-bindings/clock/tegra210-car.h>
>>>>    #include <dt-bindings/reset/tegra210-car.h>
>>>>    #include <linux/iopoll.h>
>>>> @@ -20,6 +22,7 @@
>>>>    #include <soc/tegra/pmc.h>
>>>>      #include "clk.h"
>>>> +#include "clk-dfll.h"
>>>>    #include "clk-id.h"
>>>>      /*
>>>> @@ -225,6 +228,7 @@
>>>>      #define CLK_RST_CONTROLLER_RST_DEV_Y_SET 0x2a8
>>>>    #define CLK_RST_CONTROLLER_RST_DEV_Y_CLR 0x2ac
>>>> +#define CPU_SOFTRST_CTRL 0x380
>>>>      #define LVL2_CLK_GATE_OVRA 0xf8
>>>>    #define LVL2_CLK_GATE_OVRC 0x3a0
>>>> @@ -2820,6 +2824,7 @@ static int tegra210_enable_pllu(void)
>>>>        struct tegra_clk_pll_freq_table *fentry;
>>>>        struct tegra_clk_pll pllu;
>>>>        u32 reg;
>>>> +    int ret;
>>>>          for (fentry = pll_u_freq_table; fentry->input_rate; fentry++) {
>>>>            if (fentry->input_rate == pll_ref_freq)
>>>> @@ -2847,10 +2852,10 @@ static int tegra210_enable_pllu(void)
>>>>        fence_udelay(1, clk_base);
>>>>        reg |= PLL_ENABLE;
>>>>        writel(reg, clk_base + PLLU_BASE);
>>>> +    fence_udelay(1, clk_base);
>>>>    -    readl_relaxed_poll_timeout_atomic(clk_base + PLLU_BASE, reg,
>>>> -                      reg & PLL_BASE_LOCK, 2, 1000);
>>>> -    if (!(reg & PLL_BASE_LOCK)) {
>>>> +    ret = tegra210_wait_for_mask(&pllu, PLLU_BASE, PLL_BASE_LOCK);
>>>> +    if (ret) {
>>>>            pr_err("Timed out waiting for PLL_U to lock\n");
>>>>            return -ETIMEDOUT;
>>>>        }
>>>> @@ -3283,6 +3288,103 @@ static void tegra210_disable_cpu_clock(u32 cpu)
>>>>    }
>>>>      #ifdef CONFIG_PM_SLEEP
>>>> +static u32 cpu_softrst_ctx[3];
>>>> +static struct platform_device *dfll_pdev;
>>>> +#define car_readl(_base, _off) readl_relaxed(clk_base + (_base) +
>>>> ((_off) * 4))
>>>> +#define car_writel(_val, _base, _off) \
>>>> +        writel_relaxed(_val, clk_base + (_base) + ((_off) * 4))
>>>> +
>>>> +static int tegra210_clk_suspend(void)
>>>> +{
>>>> +    unsigned int i;
>>>> +    struct device_node *node;
>>>> +
>>>> +    tegra_cclkg_burst_policy_save_context();
>>>> +
>>>> +    if (!dfll_pdev) {
>>>> +        node = of_find_compatible_node(NULL, NULL,
>>>> +                           "nvidia,tegra210-dfll");
>>>> +        if (node)
>>>> +            dfll_pdev = of_find_device_by_node(node);
>>>> +
>>>> +        of_node_put(node);
>>>> +        if (!dfll_pdev)
>>>> +            pr_err("dfll node not found. no suspend for dfll\n");
>>>> +    }
>>>> +
>>>> +    if (dfll_pdev)
>>>> +        tegra_dfll_suspend(dfll_pdev);
>>>> +
>>>> +    /* Enable PLLP_OUT_CPU after dfll suspend */
>>>> +    tegra_clk_set_pllp_out_cpu(true);
>>>> +
>>>> +    tegra_sclk_cclklp_burst_policy_save_context();
>>>> +
>>>> +    clk_save_context();
>>>> +
>>>> +    for (i = 0; i < ARRAY_SIZE(cpu_softrst_ctx); i++)
>>>> +        cpu_softrst_ctx[i] = car_readl(CPU_SOFTRST_CTRL, i);
>>>> +
>>>> +    return 0;
>>>> +}
>>>> +
>>>> +static void tegra210_clk_resume(void)
>>>> +{
>>>> +    unsigned int i;
>>>> +    struct clk_hw *parent;
>>>> +    struct clk *clk;
>>>> +
>>>> +    /*
>>>> +     * clk_restore_context restores clocks as per the clock tree.
>>>> +     *
>>>> +     * dfllCPU_out is first in the clock tree to get restored and it
>>>> +     * involves programming DFLL controller along with restoring CPUG
>>>> +     * clock burst policy.
>>>> +     *
>>>> +     * DFLL programming needs dfll_ref and dfll_soc peripheral clocks
>>>> +     * to be restores which are part ofthe peripheral clocks.
>                                              ^ white-space
>
> Please use spellchecker to avoid typos.
>
>>>> +     * So, peripheral clocks restore should happen prior to dfll clock
>>>> +     * restore.
>>>> +     */
>>>> +
>>>> +    tegra_clk_osc_resume(clk_base);
>>>> +    for (i = 0; i < ARRAY_SIZE(cpu_softrst_ctx); i++)
>>>> +        car_writel(cpu_softrst_ctx[i], CPU_SOFTRST_CTRL, i);
>>>> +
>>>> +    /* restore all plls and peripheral clocks */
>>>> +    tegra210_init_pllu();
>>>> +    clk_restore_context();
>>>> +
>>>> +    fence_udelay(5, clk_base);
>>>> +
>>>> +    /* resume SCLK and CPULP clocks */
>>>> +    tegra_sclk_cpulp_burst_policy_restore_context();
>>>> +
>>>> +    /*
>>>> +     * restore CPUG clocks:
>>>> +     * - enable DFLL in open loop mode
>>>> +     * - switch CPUG to DFLL clock source
>>>> +     * - close DFLL loop
>>>> +     * - sync PLLX state
>>>> +     */
>>>> +    if (dfll_pdev)
>>>> +        tegra_dfll_resume(dfll_pdev, false);
>>>> +
>>>> +    tegra_cclkg_burst_policy_restore_context();
>>>> +    fence_udelay(2, clk_base);
>>>> +
>>>> +    if (dfll_pdev)
>>>> +        tegra_dfll_resume(dfll_pdev, true);
>>>> +
>>>> +    parent =
>>>> clk_hw_get_parent(__clk_get_hw(clks[TEGRA210_CLK_CCLK_G]));
>>>> +    clk = clks[TEGRA210_CLK_PLL_X];
>>>> +    if (parent != __clk_get_hw(clk))
>>>> +        tegra_clk_sync_state_pll(__clk_get_hw(clk));
>>>> +
>>>> +    /* Disable PLL_OUT_CPU after DFLL resume */
>>>> +    tegra_clk_set_pllp_out_cpu(false);
>>>> +}
>>>> +
>>>>    static void tegra210_cpu_clock_suspend(void)
>>>>    {
>>>>        /* switch coresite to clk_m, save off original source */
>>>> @@ -3298,6 +3400,11 @@ static void tegra210_cpu_clock_resume(void)
>>>>    }
>>>>    #endif
>>>>    +static struct syscore_ops tegra_clk_syscore_ops = {
>>>> +    .suspend = tegra210_clk_suspend,
>>>> +    .resume = tegra210_clk_resume,
>>>> +};
>>>> +
>>>>    static struct tegra_cpu_car_ops tegra210_cpu_car_ops = {
>>>>        .wait_for_reset    = tegra210_wait_cpu_in_reset,
>>>>        .disable_clock    = tegra210_disable_cpu_clock,
>>>> @@ -3583,5 +3690,7 @@ static void __init tegra210_clock_init(struct
>>>> device_node *np)
>>>>        tegra210_mbist_clk_init();
>>>>          tegra_cpu_car_ops = &tegra210_cpu_car_ops;
>>>> +
>>>> +    register_syscore_ops(&tegra_clk_syscore_ops);
>>>>    }
>>> Is it really worthwhile to use syscore_ops for suspend/resume given
>>> that drivers for
>>> won't resume before the CLK driver anyway? Are there any other options
>>> for CLK
>>> suspend/resume?
>>>
>>> I'm also not sure whether PM runtime API could be used at all in the
>>> context of
>>> syscore_ops ..
>>>
>>> Secondly, what about to use generic clk_save_context() /
>>> clk_restore_context()
>>> helpers for the suspend-resume? It looks to me that some other
>>> essential (and proper)
>>> platform driver (soc/tegra/? PMC?) should suspend-resume the clocks
>>> using the generic
>>> CLK Framework API.
>> Clock resume should happen very early to restore peripheral and cpu
>> clocks very early than peripheral drivers resume happens.
> If all peripheral drivers properly requested all of the necessary clocks
> and CLK driver was a platform driver, then I guess the probe should have
> been naturally ordered. But that's not very achievable with the
> currently available infrastructure in the kernel, so I'm not arguing
> that the clocks should be explicitly resumed before the users.
>
>> this patch series uses clk_save_context and clk_restore_context for
>> corresponding divider, pll, pllout.. save and restore context.
> Now I see that indeed this API is utilized in this patch, thank you for
> the clarification.
>
>> But as there is dependency on dfll resume and cpu and pllx clocks
>> restore, couldnt use clk_save_context and clk_restore_context for dfll.
>>
>> So implemented recommended dfll resume sequence in main Tegra210 clock
>> driver along with invoking clk_save_context/clk_restore_context where
>> all other clocks save/restore happens as per clock tree traversal.
> Could you please clarify what part of peripherals clocks is required for
> DFLL's restore? Couldn't DFLL driver be changed to avoid that quirkness
> and thus to make DFLL driver suspend/resume the clock?

DFLL source ref_clk and soc_clk need to be restored prior to dfll.

I see dfllCPU_out parent to CCLK_G first in the clock tree and dfll_ref 
and dfll_soc peripheral clocks are not resumed by the time dfll resume 
happens first.

ref_clk and soc_clk source is from pll_p and clock tree has these 
registered under pll_p which happens later.

tegra210_clock_init registers in order plls, peripheral clocks, 
super_clk init for cclk_g during clock driver probe and dfll probe and 
register happens later.



^ permalink raw reply

* Re: [PATCH] dt-bindings: pinctrl: aspeed: Fix AST2500 example errors
From: Andrew Jeffery @ 2019-07-16  0:37 UTC (permalink / raw)
  To: Rob Herring, devicetree, Linus Walleij
  Cc: Joel Stanley, linux-aspeed, linux-gpio, linux-arm-kernel
In-Reply-To: <20190715224841.6771-1-robh@kernel.org>



On Tue, 16 Jul 2019, at 08:18, Rob Herring wrote:
> The schema examples are now validated against the schema itself. The
> AST2500 pinctrl schema has a couple of errors:
> 
> Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.example.dt.yaml: \
> example-0: $nodename:0: 'example-0' does not match 
> '^(bus|soc|axi|ahb|apb)(@[0-9a-f]+)?$'
> Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.example.dt.yaml: \
> pinctrl: aspeed,external-nodes: [[1, 2]] is too short
> 
> Fixes: 0a617de16730 ("dt-bindings: pinctrl: aspeed: Convert AST2500 
> bindings to json-schema")
> Cc: Andrew Jeffery <andrew@aj.id.au>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Joel Stanley <joel@jms.id.au>
> Cc: linux-aspeed@lists.ozlabs.org
> Cc: linux-gpio@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Signed-off-by: Rob Herring <robh@kernel.org>

Acked-by: Andrew Jeffery <andrew@aj.id.au>

> ---
>  .../devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.yaml  | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git 
> a/Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.yaml 
> b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.yaml
> index a464cfa0cba3..3e6d85318577 100644
> --- 
> a/Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.yaml
> +++ 
> b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.yaml
> @@ -76,9 +76,6 @@ required:
>  
>  examples:
>    - |
> -    compatible = "simple-bus";
> -    ranges;
> -
>      apb {
>          compatible = "simple-bus";
>          #address-cells = <1>;
> @@ -91,7 +88,7 @@ examples:
>  
>              pinctrl: pinctrl {
>                  compatible = "aspeed,g5-pinctrl";
> -                aspeed,external-nodes = <&gfx &lhc>;
> +                aspeed,external-nodes = <&gfx>, <&lhc>;
>  
>                  pinctrl_i2c3_default: i2c3_default {
>                      function = "I2C3";
> -- 
> 2.20.1
> 
>

^ permalink raw reply

* Re: [PATCH] dt-bindings: pinctrl: aspeed: Fix 'compatible' schema errors
From: Andrew Jeffery @ 2019-07-16  0:37 UTC (permalink / raw)
  To: Rob Herring, devicetree, Linus Walleij
  Cc: Joel Stanley, linux-aspeed, linux-gpio, linux-arm-kernel
In-Reply-To: <20190715223725.12924-1-robh@kernel.org>



On Tue, 16 Jul 2019, at 08:07, Rob Herring wrote:
> The Aspeed pinctl schema have errors in the 'compatible' schema:
> 
> Documentation/devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.yaml: \
> properties:compatible:enum: ['aspeed', 'ast2400-pinctrl', 'aspeed', 
> 'g4-pinctrl'] has non-unique elements
> Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.yaml: \
> properties:compatible:enum: ['aspeed', 'ast2500-pinctrl', 'aspeed', 
> 'g5-pinctrl'] has non-unique elements
> 
> Flow style sequences have to be quoted if the vales contain ','. Fix
> this by using the more common one line per entry formatting.
> 
> Fixes: 0a617de16730 ("dt-bindings: pinctrl: aspeed: Convert AST2500 
> bindings to json-schema")
> Fixes: 07457937bb5c ("dt-bindings: pinctrl: aspeed: Convert AST2400 
> bindings to json-schema")
> Cc: Andrew Jeffery <andrew@aj.id.au>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Joel Stanley <joel@jms.id.au>
> Cc: linux-aspeed@lists.ozlabs.org
> Cc: linux-gpio@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Signed-off-by: Rob Herring <robh@kernel.org>

Acked-by: Andrew Jeffery <andrew@aj.id.au>

> ---
>  .../devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.yaml   | 4 +++-
>  .../devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.yaml   | 4 +++-
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git 
> a/Documentation/devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.yaml 
> b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.yaml
> index 61a110a7db8a..125599a2dc5e 100644
> --- 
> a/Documentation/devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.yaml
> +++ 
> b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.yaml
> @@ -22,7 +22,9 @@ description: |+
>  
>  properties:
>    compatible:
> -    enum: [ aspeed,ast2400-pinctrl, aspeed,g4-pinctrl ]
> +    enum:
> +      - aspeed,ast2400-pinctrl
> +      - aspeed,g4-pinctrl
>  
>  patternProperties:
>    '^.*$':
> diff --git 
> a/Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.yaml 
> b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.yaml
> index cf561bd55128..a464cfa0cba3 100644
> --- 
> a/Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.yaml
> +++ 
> b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.yaml
> @@ -22,7 +22,9 @@ description: |+
>  
>  properties:
>    compatible:
> -    enum: [ aspeed,ast2500-pinctrl, aspeed,g5-pinctrl ]
> +    enum:
> +      - aspeed,ast2500-pinctrl
> +      - aspeed,g5-pinctrl
>    aspeed,external-nodes:
>      minItems: 2
>      maxItems: 2
> -- 
> 2.20.1
> 
>

^ permalink raw reply

* Re: [PATCH] dt-bindings: pinctrl: aspeed: Fix 'compatible' schema errors
From: Andrew Jeffery @ 2019-07-16  0:39 UTC (permalink / raw)
  To: Joel Stanley, Rob Herring
  Cc: devicetree, Linus Walleij, linux-aspeed, linux-gpio, Linux ARM
In-Reply-To: <CACPK8Xdz98CQzgE2KCjz8eOhPtx=H8jTe1hVT7LvP77U_gGASQ@mail.gmail.com>



On Tue, 16 Jul 2019, at 08:47, Joel Stanley wrote:
> On Mon, 15 Jul 2019 at 22:37, Rob Herring <robh@kernel.org> wrote:
> >
> > The Aspeed pinctl schema have errors in the 'compatible' schema:
> >
> > Documentation/devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.yaml: \
> > properties:compatible:enum: ['aspeed', 'ast2400-pinctrl', 'aspeed', 'g4-pinctrl'] has non-unique elements
> > Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.yaml: \
> > properties:compatible:enum: ['aspeed', 'ast2500-pinctrl', 'aspeed', 'g5-pinctrl'] has non-unique elements
> >
> > Flow style sequences have to be quoted if the vales contain ','. Fix
> > this by using the more common one line per entry formatting.
> 
> >
> >  properties:
> >    compatible:
> > -    enum: [ aspeed,ast2400-pinctrl, aspeed,g4-pinctrl ]
> > +    enum:
> > +      - aspeed,ast2400-pinctrl
> > +      - aspeed,g4-pinctrl
> 
> Thanks for the fix. However, we've standardised on the first form for
> all of our device trees, so we can drop the second compatible string
> from the bindings.
> 
> I think Andrew already has a patch cooking to do this.

Yes, I have a series in the works. Will send patches soon.

Andrew

^ permalink raw reply

* Re: [PATCH V5 11/18] clk: tegra210: Add support for Tegra210 clocks
From: Sowjanya Komatineni @ 2019-07-16  3:00 UTC (permalink / raw)
  To: Dmitry Osipenko, thierry.reding, jonathanh, tglx, jason,
	marc.zyngier, linus.walleij, stefan, mark.rutland
  Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
	josephl, talho, linux-tegra, linux-kernel, mperttunen, spatra,
	robh+dt, devicetree
In-Reply-To: <3938092a-bbc7-b304-641d-31677539598d@nvidia.com>


On 7/15/19 5:35 PM, Sowjanya Komatineni wrote:
>
> On 7/14/19 2:41 PM, Dmitry Osipenko wrote:
>> 13.07.2019 8:54, Sowjanya Komatineni пишет:
>>> On 6/29/19 8:10 AM, Dmitry Osipenko wrote:
>>>> 28.06.2019 5:12, Sowjanya Komatineni пишет:
>>>>> This patch adds system suspend and resume support for Tegra210
>>>>> clocks.
>>>>>
>>>>> All the CAR controller settings are lost on suspend when core power
>>>>> goes off.
>>>>>
>>>>> This patch has implementation for saving and restoring all the PLLs
>>>>> and clocks context during system suspend and resume to have the
>>>>> clocks back to same state for normal operation.
>>>>>
>>>>> Acked-by: Thierry Reding <treding@nvidia.com>
>>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>>> ---
>>>>>    drivers/clk/tegra/clk-tegra210.c | 115
>>>>> ++++++++++++++++++++++++++++++++++++++-
>>>>>    drivers/clk/tegra/clk.c          |  14 +++++
>>>>>    drivers/clk/tegra/clk.h          |   1 +
>>>>>    3 files changed, 127 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/drivers/clk/tegra/clk-tegra210.c
>>>>> b/drivers/clk/tegra/clk-tegra210.c
>>>>> index 1c08c53482a5..1b839544e086 100644
>>>>> --- a/drivers/clk/tegra/clk-tegra210.c
>>>>> +++ b/drivers/clk/tegra/clk-tegra210.c
>>>>> @@ -9,10 +9,12 @@
>>>>>    #include <linux/clkdev.h>
>>>>>    #include <linux/of.h>
>>>>>    #include <linux/of_address.h>
>>>>> +#include <linux/of_platform.h>
>>>>>    #include <linux/delay.h>
>>>>>    #include <linux/export.h>
>>>>>    #include <linux/mutex.h>
>>>>>    #include <linux/clk/tegra.h>
>>>>> +#include <linux/syscore_ops.h>
>>>>>    #include <dt-bindings/clock/tegra210-car.h>
>>>>>    #include <dt-bindings/reset/tegra210-car.h>
>>>>>    #include <linux/iopoll.h>
>>>>> @@ -20,6 +22,7 @@
>>>>>    #include <soc/tegra/pmc.h>
>>>>>      #include "clk.h"
>>>>> +#include "clk-dfll.h"
>>>>>    #include "clk-id.h"
>>>>>      /*
>>>>> @@ -225,6 +228,7 @@
>>>>>      #define CLK_RST_CONTROLLER_RST_DEV_Y_SET 0x2a8
>>>>>    #define CLK_RST_CONTROLLER_RST_DEV_Y_CLR 0x2ac
>>>>> +#define CPU_SOFTRST_CTRL 0x380
>>>>>      #define LVL2_CLK_GATE_OVRA 0xf8
>>>>>    #define LVL2_CLK_GATE_OVRC 0x3a0
>>>>> @@ -2820,6 +2824,7 @@ static int tegra210_enable_pllu(void)
>>>>>        struct tegra_clk_pll_freq_table *fentry;
>>>>>        struct tegra_clk_pll pllu;
>>>>>        u32 reg;
>>>>> +    int ret;
>>>>>          for (fentry = pll_u_freq_table; fentry->input_rate; 
>>>>> fentry++) {
>>>>>            if (fentry->input_rate == pll_ref_freq)
>>>>> @@ -2847,10 +2852,10 @@ static int tegra210_enable_pllu(void)
>>>>>        fence_udelay(1, clk_base);
>>>>>        reg |= PLL_ENABLE;
>>>>>        writel(reg, clk_base + PLLU_BASE);
>>>>> +    fence_udelay(1, clk_base);
>>>>>    -    readl_relaxed_poll_timeout_atomic(clk_base + PLLU_BASE, reg,
>>>>> -                      reg & PLL_BASE_LOCK, 2, 1000);
>>>>> -    if (!(reg & PLL_BASE_LOCK)) {
>>>>> +    ret = tegra210_wait_for_mask(&pllu, PLLU_BASE, PLL_BASE_LOCK);
>>>>> +    if (ret) {
>>>>>            pr_err("Timed out waiting for PLL_U to lock\n");
>>>>>            return -ETIMEDOUT;
>>>>>        }
>>>>> @@ -3283,6 +3288,103 @@ static void tegra210_disable_cpu_clock(u32 
>>>>> cpu)
>>>>>    }
>>>>>      #ifdef CONFIG_PM_SLEEP
>>>>> +static u32 cpu_softrst_ctx[3];
>>>>> +static struct platform_device *dfll_pdev;
>>>>> +#define car_readl(_base, _off) readl_relaxed(clk_base + (_base) +
>>>>> ((_off) * 4))
>>>>> +#define car_writel(_val, _base, _off) \
>>>>> +        writel_relaxed(_val, clk_base + (_base) + ((_off) * 4))
>>>>> +
>>>>> +static int tegra210_clk_suspend(void)
>>>>> +{
>>>>> +    unsigned int i;
>>>>> +    struct device_node *node;
>>>>> +
>>>>> +    tegra_cclkg_burst_policy_save_context();
>>>>> +
>>>>> +    if (!dfll_pdev) {
>>>>> +        node = of_find_compatible_node(NULL, NULL,
>>>>> +                           "nvidia,tegra210-dfll");
>>>>> +        if (node)
>>>>> +            dfll_pdev = of_find_device_by_node(node);
>>>>> +
>>>>> +        of_node_put(node);
>>>>> +        if (!dfll_pdev)
>>>>> +            pr_err("dfll node not found. no suspend for dfll\n");
>>>>> +    }
>>>>> +
>>>>> +    if (dfll_pdev)
>>>>> +        tegra_dfll_suspend(dfll_pdev);
>>>>> +
>>>>> +    /* Enable PLLP_OUT_CPU after dfll suspend */
>>>>> +    tegra_clk_set_pllp_out_cpu(true);
>>>>> +
>>>>> +    tegra_sclk_cclklp_burst_policy_save_context();
>>>>> +
>>>>> +    clk_save_context();
>>>>> +
>>>>> +    for (i = 0; i < ARRAY_SIZE(cpu_softrst_ctx); i++)
>>>>> +        cpu_softrst_ctx[i] = car_readl(CPU_SOFTRST_CTRL, i);
>>>>> +
>>>>> +    return 0;
>>>>> +}
>>>>> +
>>>>> +static void tegra210_clk_resume(void)
>>>>> +{
>>>>> +    unsigned int i;
>>>>> +    struct clk_hw *parent;
>>>>> +    struct clk *clk;
>>>>> +
>>>>> +    /*
>>>>> +     * clk_restore_context restores clocks as per the clock tree.
>>>>> +     *
>>>>> +     * dfllCPU_out is first in the clock tree to get restored and it
>>>>> +     * involves programming DFLL controller along with restoring 
>>>>> CPUG
>>>>> +     * clock burst policy.
>>>>> +     *
>>>>> +     * DFLL programming needs dfll_ref and dfll_soc peripheral 
>>>>> clocks
>>>>> +     * to be restores which are part ofthe peripheral clocks.
>>                                              ^ white-space
>>
>> Please use spellchecker to avoid typos.
>>
>>>>> +     * So, peripheral clocks restore should happen prior to dfll 
>>>>> clock
>>>>> +     * restore.
>>>>> +     */
>>>>> +
>>>>> +    tegra_clk_osc_resume(clk_base);
>>>>> +    for (i = 0; i < ARRAY_SIZE(cpu_softrst_ctx); i++)
>>>>> +        car_writel(cpu_softrst_ctx[i], CPU_SOFTRST_CTRL, i);
>>>>> +
>>>>> +    /* restore all plls and peripheral clocks */
>>>>> +    tegra210_init_pllu();
>>>>> +    clk_restore_context();
>>>>> +
>>>>> +    fence_udelay(5, clk_base);
>>>>> +
>>>>> +    /* resume SCLK and CPULP clocks */
>>>>> +    tegra_sclk_cpulp_burst_policy_restore_context();
>>>>> +
>>>>> +    /*
>>>>> +     * restore CPUG clocks:
>>>>> +     * - enable DFLL in open loop mode
>>>>> +     * - switch CPUG to DFLL clock source
>>>>> +     * - close DFLL loop
>>>>> +     * - sync PLLX state
>>>>> +     */
>>>>> +    if (dfll_pdev)
>>>>> +        tegra_dfll_resume(dfll_pdev, false);
>>>>> +
>>>>> +    tegra_cclkg_burst_policy_restore_context();
>>>>> +    fence_udelay(2, clk_base);
>>>>> +
>>>>> +    if (dfll_pdev)
>>>>> +        tegra_dfll_resume(dfll_pdev, true);
>>>>> +
>>>>> +    parent =
>>>>> clk_hw_get_parent(__clk_get_hw(clks[TEGRA210_CLK_CCLK_G]));
>>>>> +    clk = clks[TEGRA210_CLK_PLL_X];
>>>>> +    if (parent != __clk_get_hw(clk))
>>>>> +        tegra_clk_sync_state_pll(__clk_get_hw(clk));
>>>>> +
>>>>> +    /* Disable PLL_OUT_CPU after DFLL resume */
>>>>> +    tegra_clk_set_pllp_out_cpu(false);
>>>>> +}
>>>>> +
>>>>>    static void tegra210_cpu_clock_suspend(void)
>>>>>    {
>>>>>        /* switch coresite to clk_m, save off original source */
>>>>> @@ -3298,6 +3400,11 @@ static void tegra210_cpu_clock_resume(void)
>>>>>    }
>>>>>    #endif
>>>>>    +static struct syscore_ops tegra_clk_syscore_ops = {
>>>>> +    .suspend = tegra210_clk_suspend,
>>>>> +    .resume = tegra210_clk_resume,
>>>>> +};
>>>>> +
>>>>>    static struct tegra_cpu_car_ops tegra210_cpu_car_ops = {
>>>>>        .wait_for_reset    = tegra210_wait_cpu_in_reset,
>>>>>        .disable_clock    = tegra210_disable_cpu_clock,
>>>>> @@ -3583,5 +3690,7 @@ static void __init tegra210_clock_init(struct
>>>>> device_node *np)
>>>>>        tegra210_mbist_clk_init();
>>>>>          tegra_cpu_car_ops = &tegra210_cpu_car_ops;
>>>>> +
>>>>> +    register_syscore_ops(&tegra_clk_syscore_ops);
>>>>>    }
>>>> Is it really worthwhile to use syscore_ops for suspend/resume given
>>>> that drivers for
>>>> won't resume before the CLK driver anyway? Are there any other options
>>>> for CLK
>>>> suspend/resume?
>>>>
>>>> I'm also not sure whether PM runtime API could be used at all in the
>>>> context of
>>>> syscore_ops ..
>>>>
>>>> Secondly, what about to use generic clk_save_context() /
>>>> clk_restore_context()
>>>> helpers for the suspend-resume? It looks to me that some other
>>>> essential (and proper)
>>>> platform driver (soc/tegra/? PMC?) should suspend-resume the clocks
>>>> using the generic
>>>> CLK Framework API.
>>> Clock resume should happen very early to restore peripheral and cpu
>>> clocks very early than peripheral drivers resume happens.
>> If all peripheral drivers properly requested all of the necessary clocks
>> and CLK driver was a platform driver, then I guess the probe should have
>> been naturally ordered. But that's not very achievable with the
>> currently available infrastructure in the kernel, so I'm not arguing
>> that the clocks should be explicitly resumed before the users.
>>
>>> this patch series uses clk_save_context and clk_restore_context for
>>> corresponding divider, pll, pllout.. save and restore context.
>> Now I see that indeed this API is utilized in this patch, thank you for
>> the clarification.
>>
>>> But as there is dependency on dfll resume and cpu and pllx clocks
>>> restore, couldnt use clk_save_context and clk_restore_context for dfll.
>>>
>>> So implemented recommended dfll resume sequence in main Tegra210 clock
>>> driver along with invoking clk_save_context/clk_restore_context where
>>> all other clocks save/restore happens as per clock tree traversal.
>> Could you please clarify what part of peripherals clocks is required for
>> DFLL's restore? Couldn't DFLL driver be changed to avoid that quirkness
>> and thus to make DFLL driver suspend/resume the clock?
>
> DFLL source ref_clk and soc_clk need to be restored prior to dfll.
>
> I see dfllCPU_out parent to CCLK_G first in the clock tree and 
> dfll_ref and dfll_soc peripheral clocks are not resumed by the time 
> dfll resume happens first.
>
> ref_clk and soc_clk source is from pll_p and clock tree has these 
> registered under pll_p which happens later.
>
> tegra210_clock_init registers in order plls, peripheral clocks, 
> super_clk init for cclk_g during clock driver probe and dfll probe and 
> register happens later.
>
One more thing, CLDVFS peripheral clock enable is also needed to be 
enabled to program DFLL Controller and all peripheral clock context is 
restored only after their PLL sources are restored.

DFLL restore involves dfll source clock resume along with CLDVFS 
periheral clock enable and reset


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox