OMAP2+: optional clock handling fixes
Subsystems (6)
cover OMAP2+: optional clock handling fixes
The patches fix some opt clock handling in gpio and in hwmod.
Rajendra Nayak (2):
gpio: omap: prepare and unprepare the debounce clock
ARM: OMAP2+: hwmod: Don't leave the optional clocks in
clk_prepare()ed state
arch/arm/mach-omap2/omap_hwmod.c | 13 ++----------- drivers/gpio/gpio-omap.c | 10 +++++----- 2 files changed, 7 insertions(+), 16 deletions(-)
Signature · 1 line
1.7.9.5
PATCH 1/2 gpio: omap: prepare and unprepare the debounce clock
Replace the clk_enable()s with a clk_prepare_enable() and the clk_disables()s with a clk_disable_unprepare()
This never showed issues due to the OMAP platform code (hwmod) leaving these clocks in clk_prepare()ed state by default.
--- drivers/gpio/gpio-omap.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
drivers/gpio/gpio-omap.c +5 -5
@@ -180,7 +180,7 @@ static inline void _gpio_rmw(void __iomem *base, u32 reg, u32 mask, bool set)static inline void _gpio_dbck_enable (struct gpio_bank *bank) {if (bank->dbck_enable_mask && !bank->dbck_enabled) { -clk_enable (bank->dbck); +clk_prepare_enable (bank->dbck); bank->dbck_enabled = true; writel_relaxed(bank->dbck_enable_mask,
@@ -198,7 +198,7 @@ static inline void _gpio_dbck_disable(struct gpio_bank *bank)*/ writel_relaxed (0 , bank->base + bank->regs->debounce_en); -clk_disable (bank->dbck); +clk_disable_unprepare (bank->dbck); bank->dbck_enabled = false; } }
@@ -231,7 +231,7 @@ static void _set_gpio_debounce(struct gpio_bank *bank, unsigned gpio, l =GPIO_BIT (bank, gpio); -clk_enable (bank->dbck); +clk_prepare_enable (bank->dbck); reg = bank->base + bank->regs->debounce;writel_relaxed (debounce, reg);
@@ -245,7 +245,7 @@ static void _set_gpio_debounce(struct gpio_bank *bank, unsigned gpio, bank->dbck_enable_mask = val;writel_relaxed (val, reg); -clk_disable (bank->dbck); +clk_disable_unprepare (bank->dbck);/* * Enable debounce clock per module. * This call is mandatory because in omap_gpio_request() when
@@ -290,7 +290,7 @@ static void _clear_gpio_debounce(struct gpio_bank *bank, unsigned gpio) bank->context.debounce =0 ;writel_relaxed (bank->context.debounce, bank->base + bank->regs->debounce); -clk_disable (bank->dbck); +clk_disable_unprepare (bank->dbck); bank->dbck_enabled = false; } }
Signature · 1 line
1.7.9.5
PATCH 1/2 gpio: omap: prepare and unprepare the debounce clock
Quoted 11 lines · Rajendra Nayak
Replace the clk_enable()s with a
clk_prepare_enable()and the clk_disables()s with aclk_disable_unprepare()This never showed issues due to the OMAP platform code (hwmod) leaving these clocks in clk_prepare()ed state by default.
Reported-by Kishon Vijay Abraham I <[email protected]>Signed-off-by Rajendra Nayak <[email protected]>Cc linux-gpio at vger.kernel.orgCc Santosh Shilimkar <[email protected]>Cc Kevin Hilman <[email protected]>
Linus,
Do you mind picking this fix up via the GPIO tree? Alternatively you could Ack this if you are fine and we can take both Patch 1/2 and Patch 2/2 from this series via the OMAP tree.
Patch 2/2 has a dependency on Patch 1/2 and they need to go in in that order else gpio would break. More discussions are here [1].
Let us know what you think. Thanks.
regards,
Rajendra
[1] http://www.mail-archive.com/linux-gpio at vger.kernel.org/msg02801.html
Quoted 54 lines
--- drivers/gpio/gpio-omap.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)drivers/gpio/gpio-omap.c +5
-5@@ -180,7 +180,7 @@ static inline void _gpio_rmw(void __iomem *base, u32 reg, u32 mask, bool set)static inline void _gpio_dbck_enable (struct gpio_bank *bank) {if (bank->dbck_enable_mask && !bank->dbck_enabled) { -clk_enable (bank->dbck); +clk_prepare_enable (bank->dbck); bank->dbck_enabled = true; writel_relaxed(bank->dbck_enable_mask,@@ -198,7 +198,7 @@ static inline void _gpio_dbck_disable(struct gpio_bank *bank)*/ writel_relaxed (0 , bank->base + bank->regs->debounce_en); -clk_disable (bank->dbck); +clk_disable_unprepare (bank->dbck); bank->dbck_enabled = false; } }@@ -231,7 +231,7 @@ static void _set_gpio_debounce(struct gpio_bank *bank, unsigned gpio, l =GPIO_BIT (bank, gpio); -clk_enable (bank->dbck); +clk_prepare_enable (bank->dbck); reg = bank->base + bank->regs->debounce;writel_relaxed (debounce, reg);@@ -245,7 +245,7 @@ static void _set_gpio_debounce(struct gpio_bank *bank, unsigned gpio, bank->dbck_enable_mask = val;writel_relaxed (val, reg); -clk_disable (bank->dbck); +clk_disable_unprepare (bank->dbck);/* * Enable debounce clock per module. * This call is mandatory because in omap_gpio_request() when @@ -290,7 +290,7 @@ static void _clear_gpio_debounce(struct gpio_bank *bank, unsigned gpio) bank->context.debounce =0 ;writel_relaxed (bank->context.debounce, bank->base + bank->regs->debounce); -clk_disable (bank->dbck); +clk_disable_unprepare (bank->dbck); bank->dbck_enabled = false; } }
PATCH 1/2 gpio: omap: prepare and unprepare the debounce clock
Hello Rajendra,
Quoted 15 lines · Rajendra Nayak
Quoted 11 lines · Rajendra Nayak
Replace the clk_enable()s with a
clk_prepare_enable()and the clk_disables()s with aclk_disable_unprepare()This never showed issues due to the OMAP platform code (hwmod) leaving these clocks in clk_prepare()ed state by default.
Reported-by Kishon Vijay Abraham I <[email protected]>Signed-off-by Rajendra Nayak <[email protected]>Cc linux-gpio at vger.kernel.orgCc Santosh Shilimkar <[email protected]>Cc Kevin Hilman <[email protected]>Linus,
Quoted 8 lines
Do you mind picking this fix up via the GPIO tree? Alternatively you could Ack this if you are fine and we can take both Patch 1/2 and Patch 2/2 from this series via the OMAP tree.
Patch 2/2 has a dependency on Patch 1/2 and they need to go in in that order else gpio would break. More discussions are here [1].
Let us know what you think. Thanks.
I wonder if that is really the case. Your Patch 2/2 removes the call to clk_prepare on _init_opt_clks() but it also replaces clk_{enable,disable} with clk_prepare_enable()/clk_disable_unprepare() on _enable_optional_clocks() and _disable_optional_clocks() respectively.
And GPIO banks are reset by hwmod on init which as far as I know happen very early before the GPIO OMAP driver is even probed so by the time clk_enable() is called on the GPIO driver the clock will already be prepared by _enable_optional_clocks(). I tested linux-gpio/devel branch + only your Patch 2/2 and the GPIOs were working correctly on a OMAP3 board.
So I think that there isn't a strict dependency between these two patches or am I missing something?
In fact now that I think about it I wonder what's the functional change of your Patch 2/2 since hwmod is still calling clk_prepare() before the driver. If the clocks should actually be controlled by the drivers like you said then I think that we should remove _{enable,disable}_optional_clocks() completely and let the drivers do the clock prepare and enable like is made on your Patch 1/2 for the GPIO driver.
What do you think about it?
Best regards,
Javier
Quoted 64 lines
regards,
Rajendra[1] http://www.mail-archive.com/linux-gpio at vger.kernel.org/msg02801.html
Quoted 46 lines
--- drivers/gpio/gpio-omap.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)drivers/gpio/gpio-omap.c +4
-4@@ -180,7 +180,7 @@ static inline void _gpio_rmw(void __iomem *base, u32 reg, u32 mask, bool set)static inline void _gpio_dbck_enable (struct gpio_bank *bank) {if (bank->dbck_enable_mask && !bank->dbck_enabled) { -clk_enable (bank->dbck); +clk_prepare_enable (bank->dbck); bank->dbck_enabled = true; writel_relaxed(bank->dbck_enable_mask,@@ -198,7 +198,7 @@ static inline void _gpio_dbck_disable(struct gpio_bank *bank)*/ writel_relaxed (0 , bank->base + bank->regs->debounce_en); -clk_disable (bank->dbck); +clk_disable_unprepare (bank->dbck); bank->dbck_enabled = false; } }@@ -231,7 +231,7 @@ static void _set_gpio_debounce(struct gpio_bank *bank, unsigned gpio, l =GPIO_BIT (bank, gpio); -clk_enable (bank->dbck); +clk_prepare_enable (bank->dbck); reg = bank->base + bank->regs->debounce;writel_relaxed (debounce, reg);@@ -245,7 +245,7 @@ static void _set_gpio_debounce(struct gpio_bank *bank, unsigned gpio, bank->dbck_enable_mask = val;writel_relaxed (val, reg); -clk_disable (bank->dbck); +clk_disable_unprepare (bank->dbck);/* * Enable debounce clock per module. * This call is mandatory because in omap_gpio_request() when drivers/gpio/gpio-omap.c static void _clear_gpio_debounce(struct gpio_bank *bank, unsigned gpio) · line 296
Quoted 8 lines
drivers/gpio/gpio-omap.c +1
-1@@ -290,7 +290,7 @@ static void _clear_gpio_debounce(struct gpio_bank *bank, unsigned gpio) bank->context.debounce =0 ;writel_relaxed (bank->context.debounce, bank->base + bank->regs->debounce); -clk_disable (bank->dbck); +clk_disable_unprepare (bank->dbck); bank->dbck_enabled = false; } }--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH 1/2 gpio: omap: prepare and unprepare the debounce clock
Quoted 27 lines · Javier Martinez Canillas
Hello Rajendra,
Quoted 15 lines · Rajendra Nayak
Quoted 11 lines · Rajendra Nayak
Replace the clk_enable()s with a
clk_prepare_enable()and the clk_disables()s with aclk_disable_unprepare()This never showed issues due to the OMAP platform code (hwmod) leaving these clocks in clk_prepare()ed state by default.
Reported-by Kishon Vijay Abraham I <[email protected]>Signed-off-by Rajendra Nayak <[email protected]>Cc linux-gpio at vger.kernel.orgCc Santosh Shilimkar <[email protected]>Cc Kevin Hilman <[email protected]>Linus,
Quoted 8 lines
Do you mind picking this fix up via the GPIO tree? Alternatively you could Ack this if you are fine and we can take both Patch 1/2 and Patch 2/2 from this series via the OMAP tree.
Patch 2/2 has a dependency on Patch 1/2 and they need to go in in that order else gpio would break. More discussions are here [1].
Let us know what you think. Thanks.
Quoted 5 lines
I wonder if that is really the case. Your Patch 2/2 removes the call to
clk_prepareon_init_opt_clks()but it also replaces clk_{enable,disable} with clk_prepare_enable()/clk_disable_unprepare() on_enable_optional_clocks()and_disable_optional_clocks()respectively.
Right, the difference being, by the time hwmod is done enabling/disabling the opt clocks, without patch 2/2, the prepare count is 1, with patch 2/2 prepare count is 0.
Quoted 5 lines
And GPIO banks are reset by hwmod on init which as far as I know happen very early before the GPIO OMAP driver is even probed so by the time
clk_enable()is called on the GPIO driver the clock will already be prepared by_enable_optional_clocks(). I tested linux-gpio/devel
and unprepared by _disable_optional_clocks()?
Quoted 2 lines
branch + only your Patch 2/2 and the GPIOs were working correctly on a OMAP3 board.
Did gpio_debounce() ever get called for any of the gpios?
Quoted 7 lines
So I think that there isn't a strict dependency between these two patches or am I missing something?
In fact now that I think about it I wonder what's the functional change of your Patch 2/2 since hwmod is still calling
clk_prepare()before the driver. If the clocks should actually be controlled by the
I don't understand why you say 'before the driver'. Hwmod needs to control optional clocks for some devices in order to do a ocp reset. So it does touch these optional clocks, but if you look at the code it subsequently also disables (and unprepares with patch 2/2) these clocks before returning the control to the driver.
Quoted 74 lines
drivers like you said then I think that we should remove _{enable,disable}_optional_clocks() completely and let the drivers do the clock prepare and enable like is made on your Patch 1/2 for the GPIO driver.
What do you think about it?
Best regards,
JavierQuoted 64 lines
regards,
Rajendra[1] http://www.mail-archive.com/linux-gpio at vger.kernel.org/msg02801.html
Quoted 46 lines
--- drivers/gpio/gpio-omap.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)drivers/gpio/gpio-omap.c +4
-4@@ -180,7 +180,7 @@ static inline void _gpio_rmw(void __iomem *base, u32 reg, u32 mask, bool set)static inline void _gpio_dbck_enable (struct gpio_bank *bank) {if (bank->dbck_enable_mask && !bank->dbck_enabled) { -clk_enable (bank->dbck); +clk_prepare_enable (bank->dbck); bank->dbck_enabled = true; writel_relaxed(bank->dbck_enable_mask,@@ -198,7 +198,7 @@ static inline void _gpio_dbck_disable(struct gpio_bank *bank)*/ writel_relaxed (0 , bank->base + bank->regs->debounce_en); -clk_disable (bank->dbck); +clk_disable_unprepare (bank->dbck); bank->dbck_enabled = false; } }@@ -231,7 +231,7 @@ static void _set_gpio_debounce(struct gpio_bank *bank, unsigned gpio, l =GPIO_BIT (bank, gpio); -clk_enable (bank->dbck); +clk_prepare_enable (bank->dbck); reg = bank->base + bank->regs->debounce;writel_relaxed (debounce, reg);@@ -245,7 +245,7 @@ static void _set_gpio_debounce(struct gpio_bank *bank, unsigned gpio, bank->dbck_enable_mask = val;writel_relaxed (val, reg); -clk_disable (bank->dbck); +clk_disable_unprepare (bank->dbck);/* * Enable debounce clock per module. * This call is mandatory because in omap_gpio_request() when drivers/gpio/gpio-omap.c static void _clear_gpio_debounce(struct gpio_bank *bank, unsigned gpio) · line 296
Quoted 8 lines
drivers/gpio/gpio-omap.c +1
-1@@ -290,7 +290,7 @@ static void _clear_gpio_debounce(struct gpio_bank *bank, unsigned gpio) bank->context.debounce =0 ;writel_relaxed (bank->context.debounce, bank->base + bank->regs->debounce); -clk_disable (bank->dbck); +clk_disable_unprepare (bank->dbck); bank->dbck_enabled = false; } }--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH 1/2 gpio: omap: prepare and unprepare the debounce clock
Hello Rajendra,
Quoted 34 lines · Rajendra Nayak
Quoted 27 lines · Javier Martinez Canillas
Hello Rajendra,
Quoted 15 lines · Rajendra Nayak
Quoted 11 lines · Rajendra Nayak
Replace the clk_enable()s with a
clk_prepare_enable()and the clk_disables()s with aclk_disable_unprepare()This never showed issues due to the OMAP platform code (hwmod) leaving these clocks in clk_prepare()ed state by default.
Reported-by Kishon Vijay Abraham I <[email protected]>Signed-off-by Rajendra Nayak <[email protected]>Cc linux-gpio at vger.kernel.orgCc Santosh Shilimkar <[email protected]>Cc Kevin Hilman <[email protected]>Linus,
Quoted 8 lines
Do you mind picking this fix up via the GPIO tree? Alternatively you could Ack this if you are fine and we can take both Patch 1/2 and Patch 2/2 from this series via the OMAP tree.
Patch 2/2 has a dependency on Patch 1/2 and they need to go in in that order else gpio would break. More discussions are here [1].
Let us know what you think. Thanks.Quoted 5 lines
I wonder if that is really the case. Your Patch 2/2 removes the call to
clk_prepareon_init_opt_clks()but it also replaces clk_{enable,disable} with clk_prepare_enable()/clk_disable_unprepare() on_enable_optional_clocks()and_disable_optional_clocks()respectively.
Quoted 4 lines
Right, the difference being, by the time hwmod is done enabling/disabling the opt clocks, without patch 2/2, the prepare count is 1, with patch 2/2 prepare count is 0.
Ok, got it now.
Quoted 8 lines
Quoted 5 lines
And GPIO banks are reset by hwmod on init which as far as I know happen very early before the GPIO OMAP driver is even probed so by the time
clk_enable()is called on the GPIO driver the clock will already be prepared by_enable_optional_clocks(). I tested linux-gpio/develand unprepared by
_disable_optional_clocks()?
I see that _disable_optional_clocks() is called as well so the clock is left unprepared as you said.
Quoted 5 lines
Quoted 2 lines
branch + only your Patch 2/2 and the GPIOs were working correctly on a OMAP3 board.
Did
gpio_debounce()ever get called for any of the gpios?
I don't see gpio_debounce() to be called indeed.
omap_gpio_runtime_resume() is executed and calls _gpio_dbck_enable(bank) but clk_enable(bank->dbck) is not called since bank->dbck_enable_mask is 0, that was my confusion since I thought that clk_enable() was called.
Now I understand the dependency between the two patches.
Quoted 8 lines
Quoted 7 lines
So I think that there isn't a strict dependency between these two patches or am I missing something?
In fact now that I think about it I wonder what's the functional change of your Patch 2/2 since hwmod is still calling
clk_prepare()before the driver. If the clocks should actually be controlled by the
Quoted 6 lines
I don't understand why you say 'before the driver'. Hwmod needs to control optional clocks for some devices in order to do a ocp reset. So it does touch these optional clocks, but if you look at the code it subsequently also disables (and unprepares with patch 2/2) these clocks before returning the control to the driver.
Right, it was just me getting confused by the interaction between hwmod and the GPIO driver. Thanks a lot for the explanation and sorry for the noise.
Feel free to add my:
Best regards,
Javier
Quoted 75 lines
Quoted 74 lines
drivers like you said then I think that we should remove _{enable,disable}_optional_clocks() completely and let the drivers do the clock prepare and enable like is made on your Patch 1/2 for the GPIO driver.
What do you think about it?
Best regards,
JavierQuoted 64 lines
regards,
Rajendra[1] http://www.mail-archive.com/linux-gpio at vger.kernel.org/msg02801.html
Quoted 46 lines
--- drivers/gpio/gpio-omap.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)drivers/gpio/gpio-omap.c +4
-4@@ -180,7 +180,7 @@ static inline void _gpio_rmw(void __iomem *base, u32 reg, u32 mask, bool set)static inline void _gpio_dbck_enable (struct gpio_bank *bank) {if (bank->dbck_enable_mask && !bank->dbck_enabled) { -clk_enable (bank->dbck); +clk_prepare_enable (bank->dbck); bank->dbck_enabled = true; writel_relaxed(bank->dbck_enable_mask,@@ -198,7 +198,7 @@ static inline void _gpio_dbck_disable(struct gpio_bank *bank)*/ writel_relaxed (0 , bank->base + bank->regs->debounce_en); -clk_disable (bank->dbck); +clk_disable_unprepare (bank->dbck); bank->dbck_enabled = false; } }@@ -231,7 +231,7 @@ static void _set_gpio_debounce(struct gpio_bank *bank, unsigned gpio, l =GPIO_BIT (bank, gpio); -clk_enable (bank->dbck); +clk_prepare_enable (bank->dbck); reg = bank->base + bank->regs->debounce;writel_relaxed (debounce, reg);@@ -245,7 +245,7 @@ static void _set_gpio_debounce(struct gpio_bank *bank, unsigned gpio, bank->dbck_enable_mask = val;writel_relaxed (val, reg); -clk_disable (bank->dbck); +clk_disable_unprepare (bank->dbck);/* * Enable debounce clock per module. * This call is mandatory because in omap_gpio_request() when drivers/gpio/gpio-omap.c static void _clear_gpio_debounce(struct gpio_bank *bank, unsigned gpio) · line 296
Quoted 8 lines
drivers/gpio/gpio-omap.c +1
-1@@ -290,7 +290,7 @@ static void _clear_gpio_debounce(struct gpio_bank *bank, unsigned gpio) bank->context.debounce =0 ;writel_relaxed (bank->context.debounce, bank->base + bank->regs->debounce); -clk_disable (bank->dbck); +clk_disable_unprepare (bank->dbck); bank->dbck_enabled = false; } }--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH 1/2 gpio: omap: prepare and unprepare the debounce clock
Quoted 86 lines · Javier Martinez Canillas
Hello Rajendra,
Quoted 34 lines · Rajendra Nayak
Quoted 27 lines · Javier Martinez Canillas
Hello Rajendra,
Quoted 15 lines · Rajendra Nayak
Quoted 11 lines · Rajendra Nayak
Replace the clk_enable()s with a
clk_prepare_enable()and the clk_disables()s with aclk_disable_unprepare()This never showed issues due to the OMAP platform code (hwmod) leaving these clocks in clk_prepare()ed state by default.
Reported-by Kishon Vijay Abraham I <[email protected]>Signed-off-by Rajendra Nayak <[email protected]>Cc linux-gpio at vger.kernel.orgCc Santosh Shilimkar <[email protected]>Cc Kevin Hilman <[email protected]>Linus,
Quoted 8 lines
Do you mind picking this fix up via the GPIO tree? Alternatively you could Ack this if you are fine and we can take both Patch 1/2 and Patch 2/2 from this series via the OMAP tree.
Patch 2/2 has a dependency on Patch 1/2 and they need to go in in that order else gpio would break. More discussions are here [1].
Let us know what you think. Thanks.Quoted 5 lines
I wonder if that is really the case. Your Patch 2/2 removes the call to
clk_prepareon_init_opt_clks()but it also replaces clk_{enable,disable} with clk_prepare_enable()/clk_disable_unprepare() on_enable_optional_clocks()and_disable_optional_clocks()respectively.Quoted 4 lines
Right, the difference being, by the time hwmod is done enabling/disabling the opt clocks, without patch 2/2, the prepare count is 1, with patch 2/2 prepare count is 0.
Ok, got it now.
Quoted 8 lines
Quoted 5 lines
And GPIO banks are reset by hwmod on init which as far as I know happen very early before the GPIO OMAP driver is even probed so by the time
clk_enable()is called on the GPIO driver the clock will already be prepared by_enable_optional_clocks(). I tested linux-gpio/develand unprepared by
_disable_optional_clocks()?I see that
_disable_optional_clocks()is called as well so the clock is left unprepared as you said.Quoted 5 lines
Quoted 2 lines
branch + only your Patch 2/2 and the GPIOs were working correctly on a OMAP3 board.
Did
gpio_debounce()ever get called for any of the gpios?I don't see
gpio_debounce()to be called indeed.
omap_gpio_runtime_resume()is executed and calls_gpio_dbck_enable(bank)butclk_enable(bank->dbck)is not called sincebank->dbck_enable_maskis 0, that was my confusion since I thought thatclk_enable()was called.Now I understand the dependency between the two patches.
Quoted 8 lines
Quoted 7 lines
So I think that there isn't a strict dependency between these two patches or am I missing something?
In fact now that I think about it I wonder what's the functional change of your Patch 2/2 since hwmod is still calling
clk_prepare()before the driver. If the clocks should actually be controlled by theQuoted 6 lines
I don't understand why you say 'before the driver'. Hwmod needs to control optional clocks for some devices in order to do a ocp reset. So it does touch these optional clocks, but if you look at the code it subsequently also disables (and unprepares with patch 2/2) these clocks before returning the control to the driver.
Quoted 3 lines
Right, it was just me getting confused by the interaction between hwmod and the GPIO driver. Thanks a lot for the explanation and sorry for the noise.
No issues, thanks for the review and ack.
Quoted 83 lines
Feel free to add my:
Best regards,
JavierQuoted 75 lines
Quoted 74 lines
drivers like you said then I think that we should remove _{enable,disable}_optional_clocks() completely and let the drivers do the clock prepare and enable like is made on your Patch 1/2 for the GPIO driver.
What do you think about it?
Best regards,
JavierQuoted 64 lines
regards,
Rajendra[1] http://www.mail-archive.com/linux-gpio at vger.kernel.org/msg02801.html
Quoted 46 lines
--- drivers/gpio/gpio-omap.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)drivers/gpio/gpio-omap.c +4
-4@@ -180,7 +180,7 @@ static inline void _gpio_rmw(void __iomem *base, u32 reg, u32 mask, bool set)static inline void _gpio_dbck_enable (struct gpio_bank *bank) {if (bank->dbck_enable_mask && !bank->dbck_enabled) { -clk_enable (bank->dbck); +clk_prepare_enable (bank->dbck); bank->dbck_enabled = true; writel_relaxed(bank->dbck_enable_mask,@@ -198,7 +198,7 @@ static inline void _gpio_dbck_disable(struct gpio_bank *bank)*/ writel_relaxed (0 , bank->base + bank->regs->debounce_en); -clk_disable (bank->dbck); +clk_disable_unprepare (bank->dbck); bank->dbck_enabled = false; } }@@ -231,7 +231,7 @@ static void _set_gpio_debounce(struct gpio_bank *bank, unsigned gpio, l =GPIO_BIT (bank, gpio); -clk_enable (bank->dbck); +clk_prepare_enable (bank->dbck); reg = bank->base + bank->regs->debounce;writel_relaxed (debounce, reg);@@ -245,7 +245,7 @@ static void _set_gpio_debounce(struct gpio_bank *bank, unsigned gpio, bank->dbck_enable_mask = val;writel_relaxed (val, reg); -clk_disable (bank->dbck); +clk_disable_unprepare (bank->dbck);/* * Enable debounce clock per module. * This call is mandatory because in omap_gpio_request() when drivers/gpio/gpio-omap.c static void _clear_gpio_debounce(struct gpio_bank *bank, unsigned gpio) · line 296
Quoted 8 lines
drivers/gpio/gpio-omap.c +1
-1@@ -290,7 +290,7 @@ static void _clear_gpio_debounce(struct gpio_bank *bank, unsigned gpio) bank->context.debounce =0 ;writel_relaxed (bank->context.debounce, bank->base + bank->regs->debounce); -clk_disable (bank->dbck); +clk_disable_unprepare (bank->dbck); bank->dbck_enabled = false; } }--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH 1/2 gpio: omap: prepare and unprepare the debounce clock
In reply to Rajendra NayakQuoted 1 line · Rajendra Nayak
Do you mind picking this fix up via the GPIO tree?
Yes, it's merged to my devel branch now with the ACKs.
Quoted 3 lines
Alternatively you could
Ack this if you are fine and we can take both Patch 1/2 and Patch 2/2 from this series via the OMAP tree.
This probably will not work as I have a set of other changes to this driver in my tree.
Quoted 2 lines
Patch 2/2 has a dependency on Patch 1/2 and they need to go in in that order else gpio would break. More discussions are here [1].
Tell me if I should prepare an immutable tag on my branch that you can pull in. I want an explicit handshake with the platform maintainer for this kind of stuff.
Yours,
Linus Walleij
PATCH 1/2 gpio: omap: prepare and unprepare the debounce clock
In reply to Rajendra NayakQuoted 6 lines · Rajendra Nayak
Replace the clk_enable()s with a
clk_prepare_enable()and the clk_disables()s with aclk_disable_unprepare()This never showed issues due to the OMAP platform code (hwmod) leaving these clocks in clk_prepare()ed state by default.
Quoted 9 lines
Reported-by Kishon Vijay Abraham I <[email protected]>Signed-off-by Rajendra Nayak <[email protected]>Cc linux-gpio at vger.kernel.orgCc Santosh Shilimkar <[email protected]>Cc Kevin Hilman <[email protected]>--- drivers/gpio/gpio-omap.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
PATCH 1/2 gpio: omap: prepare and unprepare the debounce clock
In reply to Rajendra NayakQuoted 6 lines · Rajendra Nayak
Replace the clk_enable()s with a
clk_prepare_enable()and the clk_disables()s with aclk_disable_unprepare()This never showed issues due to the OMAP platform code (hwmod) leaving these clocks in clk_prepare()ed state by default.
Quoted 6 lines
Reported-by Kishon Vijay Abraham I <[email protected]>Signed-off-by Rajendra Nayak <[email protected]>Cc linux-gpio at vger.kernel.orgCc Santosh Shilimkar <[email protected]>Cc Kevin Hilman <[email protected]>---
$subject patch looks fine but I don't see patch 2/2 assuming this is series of two patches.
PATCH 2/2 ARM: OMAP2+: hwmod: Don't leave the optional clocks in clk_prepare()ed state
In reply to Rajendra NayakAt hwmod init, theres no reason why optional clocks should be left in clk_prepare()ed state as these are actually directly controlled by the drivers themselves. Let the drivers prepare/unprepare as well as enable/ disable them.
--- arch/arm/mach-omap2/omap_hwmod.c | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-)
arch/arm/mach-omap2/omap_hwmod.c +2 -11
@@ -857,15 +857,6 @@ static int _init_opt_clks(struct omap_hwmod *oh)continue ; } oc->_clk = c; -/* -* HACK: This needs a re-visit once clk_prepare() is implemented -* to do something meaningful. Today its just a no-op. -* If clk_prepare() is used at some point to do things like -* voltage scaling etc, then this would have to be moved to -* some point where subsystems like i2c and pmic become -* available. -*/ -clk_prepare (oc->_clk); }return ret;
@@ -945,7 +936,7 @@ static void _enable_optional_clocks(struct omap_hwmod *oh)if (oc->_clk) {pr_debug ("omap_hwmod: enable %s:%s\n" , oc->role,__clk_get_name (oc->_clk)); -clk_enable (oc->_clk); +clk_prepare_enable (oc->_clk); } }
@@ -960,7 +951,7 @@ static void _disable_optional_clocks(struct omap_hwmod *oh)if (oc->_clk) {pr_debug ("omap_hwmod: disable %s:%s\n" , oc->role,__clk_get_name (oc->_clk)); -clk_disable (oc->_clk); +clk_disable_unprepare (oc->_clk); } }
Signature · 1 line
1.7.9.5
Paul Walmsley
maintainercover OMAP2+: optional clock handling fixes
In reply to Rajendra NayakHi Rajendra,
Quoted 11 lines · Rajendra Nayak
The patches fix some opt clock handling in gpio and in hwmod.
Rajendra Nayak (2):
gpio: omap: prepare and unprepare the debounce clock
ARM: OMAP2+: hwmod: Don't leave the optional clocks in
clk_prepare()ed statearch/arm/mach-omap2/omap_hwmod.c | 13 ++----------- drivers/gpio/gpio-omap.c | 10 +++++----- 2 files changed, 7 insertions(+), 16 deletions(-)
Can these patches be merged separately? Looks to me that the two options are either to:
A. to merge them together, or
B. to merge patch 1 first, then patch 2
Or will things break if only patch 1 is merged?
If we merge them together, I'd say the best situation would be to take them through the OMAP tree, since the changes are all OMAP-specific. In that case we'll want an ack for the first patch from the GPIO maintainers, Linus Walleij <[email protected]> and Alexandre Courbot <[email protected]>.
Otherwise the path of least resistance would be (B) - you can get patch 1 merged via the GPIO tree. The GPIO maintainers can then provide a stable branch for us to base our changes on, or we can wait until the change reaches Linus. Then we can subsequently merge patch 2 via the OMAP side.
Thoughts?
- Paul
cover OMAP2+: optional clock handling fixes
Quoted 16 lines · Paul Walmsley
Hi Rajendra,
Quoted 11 lines · Rajendra Nayak
The patches fix some opt clock handling in gpio and in hwmod.
Rajendra Nayak (2):
gpio: omap: prepare and unprepare the debounce clock
ARM: OMAP2+: hwmod: Don't leave the optional clocks in
clk_prepare()ed statearch/arm/mach-omap2/omap_hwmod.c | 13 ++----------- drivers/gpio/gpio-omap.c | 10 +++++----- 2 files changed, 7 insertions(+), 16 deletions(-)
Quoted 6 lines
Can these patches be merged separately? Looks to me that the two options are either to:
A. to merge them together, or
B. to merge patch 1 first, then patch 2
Thats right.
Quoted 2 lines
Or will things break if only patch 1 is merged?
Things will break if only patch 2 is merged as gpios clk_enable() request would fail. Merging only patch 1 has no issues.
Quoted 8 lines
If we merge them together, I'd say the best situation would be to take them through the OMAP tree, since the changes are all OMAP-specific. In that case we'll want an ack for the first patch from the GPIO maintainers, Linus Walleij <[email protected]> and Alexandre Courbot <[email protected]>.
Quoted 7 lines
Otherwise the path of least resistance would be (B) - you can get patch 1 merged via the GPIO tree. The GPIO maintainers can then provide a stable branch for us to base our changes on, or we can wait until the change reaches Linus. Then we can subsequently merge patch 2 via the OMAP side.
Thoughts?
I am fine either way. I will check with Linus W. what he prefers. Thanks.
regards,
Rajendra
Quoted 4 lines
- Paul