[PATCH 2/2] Input: nomadik-ske-keypad - start using the apb_pclk
From: Ulf Hansson ulf.hans...@linaro.org
Previously this clock was handled internally by the clockdriver,
but now this is separate clk. So we need take care of it.
Signed-off-by: Ulf Hansson ulf.hans...@linaro.org
---
drivers/input/keyboard/nomadik-ske-keypad.c | 22 --
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/drivers/input/keyboard/nomadik-ske-keypad.c
b/drivers/input/keyboard/nomadik-ske-keypad.c
index 95dcc9b..a1a9375 100644
--- a/drivers/input/keyboard/nomadik-ske-keypad.c
+++ b/drivers/input/keyboard/nomadik-ske-keypad.c
@@ -67,6 +67,7 @@ struct ske_keypad {
const struct ske_keypad_platform_data *board;
unsigned short keymap[SKE_KPD_NUM_ROWS * SKE_KPD_NUM_COLS];
struct clk *clk;
+ struct clk *pclk;
spinlock_t ske_keypad_lock;
};
@@ -271,11 +272,18 @@ static int __init ske_keypad_probe(struct platform_device
*pdev)
goto err_free_mem_region;
}
+ keypad-pclk = clk_get(pdev-dev, apb_pclk);
+ if (IS_ERR(keypad-pclk)) {
+ dev_err(pdev-dev, failed to get pclk\n);
+ error = PTR_ERR(keypad-pclk);
+ goto err_iounmap;
+ }
+
keypad-clk = clk_get(pdev-dev, NULL);
if (IS_ERR(keypad-clk)) {
dev_err(pdev-dev, failed to get clk\n);
error = PTR_ERR(keypad-clk);
- goto err_iounmap;
+ goto err_pclk;
}
input-id.bustype = BUS_HOST;
@@ -294,10 +302,16 @@ static int __init ske_keypad_probe(struct platform_device
*pdev)
if (!plat-no_autorepeat)
__set_bit(EV_REP, input-evbit);
+ error = clk_prepare_enable(keypad-pclk);
+ if (error) {
+ dev_err(pdev-dev, Failed to prepare/enable pclk\n);
+ goto err_clk;
+ }
+
error = clk_prepare_enable(keypad-clk);
if (error) {
dev_err(pdev-dev, Failed to prepare/enable clk\n);
- goto err_clk;
+ goto err_pclk_disable;
}
@@ -336,8 +350,12 @@ err_free_irq:
free_irq(keypad-irq, keypad);
err_clk_disable:
clk_disable_unprepare(keypad-clk);
+err_pclk_disable:
+ clk_disable_unprepare(keypad-pclk);
err_clk:
clk_put(keypad-clk);
+err_pclk:
+ clk_put(keypad-pclk);
err_iounmap:
iounmap(keypad-reg_base);
err_free_mem_region:
--
1.7.10
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] Input: nomadik-ske-keypad - fixup use of clk
From: Ulf Hansson ulf.hans...@linaro.org
Do proper error handling for clk and make sure clocks are being
prepared|unprepared as well as enabled|disabled.
Signed-off-by: Ulf Hansson ulf.hans...@linaro.org
---
drivers/input/keyboard/nomadik-ske-keypad.c | 14 ++
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/input/keyboard/nomadik-ske-keypad.c
b/drivers/input/keyboard/nomadik-ske-keypad.c
index 49f5fa6..95dcc9b 100644
--- a/drivers/input/keyboard/nomadik-ske-keypad.c
+++ b/drivers/input/keyboard/nomadik-ske-keypad.c
@@ -287,14 +287,19 @@ static int __init ske_keypad_probe(struct platform_device
*pdev)
keypad-keymap, input);
if (error) {
dev_err(pdev-dev, Failed to build keymap\n);
- goto err_iounmap;
+ goto err_clk;
}
input_set_capability(input, EV_MSC, MSC_SCAN);
if (!plat-no_autorepeat)
__set_bit(EV_REP, input-evbit);
- clk_enable(keypad-clk);
+ error = clk_prepare_enable(keypad-clk);
+ if (error) {
+ dev_err(pdev-dev, Failed to prepare/enable clk\n);
+ goto err_clk;
+ }
+
/* go through board initialization helpers */
if (keypad-board-init)
@@ -330,7 +335,8 @@ static int __init ske_keypad_probe(struct platform_device
*pdev)
err_free_irq:
free_irq(keypad-irq, keypad);
err_clk_disable:
- clk_disable(keypad-clk);
+ clk_disable_unprepare(keypad-clk);
+err_clk:
clk_put(keypad-clk);
err_iounmap:
iounmap(keypad-reg_base);
@@ -351,7 +357,7 @@ static int __devexit ske_keypad_remove(struct
platform_device *pdev)
input_unregister_device(keypad-input);
- clk_disable(keypad-clk);
+ clk_disable_unprepare(keypad-clk);
clk_put(keypad-clk);
if (keypad-board-exit)
--
1.7.10
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 0/2] Input: nomadik-ske-keypad - clk fixups
From: Ulf Hansson ulf.hans...@linaro.org Due to the convert to the common clk driver these changes for clks are needed. Ulf Hansson (2): Input: nomadik-ske-keypad - fixup use of clk Input: nomadik-ske-keypad - start using the apb_pclk drivers/input/keyboard/nomadik-ske-keypad.c | 34 +++ 1 file changed, 29 insertions(+), 5 deletions(-) -- 1.7.10 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 0/3] clk: ux500: Set sdmmc clock to 100MHz
From: Ulf Hansson ulf.hans...@linaro.org The reason behind this change is that we would like to enable all the mmc/sd/sdio features from a hardware perspective (ARM PL18x). It is then required that the frequency must be 100MHz. Ulf Hansson (3): mfd: dbx500: Export prmcu_request_ape_opp_100_voltage clk: ux500: Support prcmu ape opp voltage clock clk: ux500: Update sdmmc clock to 100MHz for u8500 drivers/clk/ux500/clk-prcmu.c| 55 ++ drivers/clk/ux500/clk.h |6 + drivers/clk/ux500/u8500_clk.c|5 ++-- drivers/mfd/db8500-prcmu.c |4 +-- include/linux/mfd/db8500-prcmu.h |4 +-- include/linux/mfd/dbx500-prcmu.h | 10 +++ 6 files changed, 78 insertions(+), 6 deletions(-) -- 1.7.10 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/3] mfd: dbx500: Export prmcu_request_ape_opp_100_voltage
From: Ulf Hansson ulf.hans...@linaro.org
This function needs to be exported to let clients be able to
request the ape opp 100 voltage.
Cc: Samuel Ortiz sa...@linux.intel.com
Signed-off-by: Ulf Hansson ulf.hans...@linaro.org
---
drivers/mfd/db8500-prcmu.c |4 ++--
include/linux/mfd/db8500-prcmu.h |4 ++--
include/linux/mfd/dbx500-prcmu.h | 10 ++
3 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/mfd/db8500-prcmu.c b/drivers/mfd/db8500-prcmu.c
index e7f9539..0b8e0a0 100644
--- a/drivers/mfd/db8500-prcmu.c
+++ b/drivers/mfd/db8500-prcmu.c
@@ -1167,12 +1167,12 @@ int db8500_prcmu_get_ape_opp(void)
}
/**
- * prcmu_request_ape_opp_100_voltage - Request APE OPP 100% voltage
+ * db8500_prcmu_request_ape_opp_100_voltage - Request APE OPP 100% voltage
* @enable: true to request the higher voltage, false to drop a request.
*
* Calls to this function to enable and disable requests must be balanced.
*/
-int prcmu_request_ape_opp_100_voltage(bool enable)
+int db8500_prcmu_request_ape_opp_100_voltage(bool enable)
{
int r = 0;
u8 header;
diff --git a/include/linux/mfd/db8500-prcmu.h b/include/linux/mfd/db8500-prcmu.h
index b82f6ee..6ee4247 100644
--- a/include/linux/mfd/db8500-prcmu.h
+++ b/include/linux/mfd/db8500-prcmu.h
@@ -515,7 +515,6 @@ enum romcode_read prcmu_get_rc_p2a(void);
enum ap_pwrst prcmu_get_xp70_current_state(void);
bool prcmu_has_arm_maxopp(void);
struct prcmu_fw_version *prcmu_get_fw_version(void);
-int prcmu_request_ape_opp_100_voltage(bool enable);
int prcmu_release_usb_wakeup_state(void);
void prcmu_configure_auto_pm(struct prcmu_auto_pm_config *sleep,
struct prcmu_auto_pm_config *idle);
@@ -564,6 +563,7 @@ int db8500_prcmu_set_arm_opp(u8 opp);
int db8500_prcmu_get_arm_opp(void);
int db8500_prcmu_set_ape_opp(u8 opp);
int db8500_prcmu_get_ape_opp(void);
+int db8500_prcmu_request_ape_opp_100_voltage(bool enable);
int db8500_prcmu_set_ddr_opp(u8 opp);
int db8500_prcmu_get_ddr_opp(void);
@@ -610,7 +610,7 @@ static inline int db8500_prcmu_get_ape_opp(void)
return APE_100_OPP;
}
-static inline int prcmu_request_ape_opp_100_voltage(bool enable)
+static inline int db8500_prcmu_request_ape_opp_100_voltage(bool enable)
{
return 0;
}
diff --git a/include/linux/mfd/dbx500-prcmu.h b/include/linux/mfd/dbx500-prcmu.h
index c410d99..c202d6c 100644
--- a/include/linux/mfd/dbx500-prcmu.h
+++ b/include/linux/mfd/dbx500-prcmu.h
@@ -336,6 +336,11 @@ static inline int prcmu_get_ape_opp(void)
return db8500_prcmu_get_ape_opp();
}
+static inline int prcmu_request_ape_opp_100_voltage(bool enable)
+{
+ return db8500_prcmu_request_ape_opp_100_voltage(enable);
+}
+
static inline void prcmu_system_reset(u16 reset_code)
{
return db8500_prcmu_system_reset(reset_code);
@@ -507,6 +512,11 @@ static inline int prcmu_get_ape_opp(void)
return APE_100_OPP;
}
+static inline int prcmu_request_ape_opp_100_voltage(bool enable)
+{
+ return 0;
+}
+
static inline int prcmu_set_arm_opp(u8 opp)
{
return 0;
--
1.7.10
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/3] clk: ux500: Support prcmu ape opp voltage clock
From: Ulf Hansson ulf.hans...@linaro.org
Some scalable prcmu clocks needs to be handled in conjuction with the
ape opp 100 voltage. A new prcmu clock type clk_prcmu_opp_volt_scalable
is implemented to handle this.
Signed-off-by: Ulf Hansson ulf.hans...@linaro.org
---
drivers/clk/ux500/clk-prcmu.c | 55 +
drivers/clk/ux500/clk.h |6 +
2 files changed, 61 insertions(+)
diff --git a/drivers/clk/ux500/clk-prcmu.c b/drivers/clk/ux500/clk-prcmu.c
index 930cdfe..04577ca 100644
--- a/drivers/clk/ux500/clk-prcmu.c
+++ b/drivers/clk/ux500/clk-prcmu.c
@@ -133,6 +133,40 @@ out_error:
hw-init-name);
}
+static int clk_prcmu_opp_volt_prepare(struct clk_hw *hw)
+{
+ int err;
+ struct clk_prcmu *clk = to_clk_prcmu(hw);
+
+ err = prcmu_request_ape_opp_100_voltage(true);
+ if (err) {
+ pr_err(clk_prcmu: %s failed to request APE OPP VOLT for %s.\n,
+ __func__, hw-init-name);
+ return err;
+ }
+
+ err = prcmu_request_clock(clk-cg_sel, true);
+ if (err)
+ prcmu_request_ape_opp_100_voltage(false);
+
+ return err;
+}
+
+static void clk_prcmu_opp_volt_unprepare(struct clk_hw *hw)
+{
+ struct clk_prcmu *clk = to_clk_prcmu(hw);
+
+ if (prcmu_request_clock(clk-cg_sel, false))
+ goto out_error;
+ if (prcmu_request_ape_opp_100_voltage(false))
+ goto out_error;
+ return;
+
+out_error:
+ pr_err(clk_prcmu: %s failed to disable %s.\n, __func__,
+ hw-init-name);
+}
+
static struct clk_ops clk_prcmu_scalable_ops = {
.prepare = clk_prcmu_prepare,
.unprepare = clk_prcmu_unprepare,
@@ -167,6 +201,17 @@ static struct clk_ops clk_prcmu_opp_gate_ops = {
.recalc_rate = clk_prcmu_recalc_rate,
};
+static struct clk_ops clk_prcmu_opp_volt_scalable_ops = {
+ .prepare = clk_prcmu_opp_volt_prepare,
+ .unprepare = clk_prcmu_opp_volt_unprepare,
+ .enable = clk_prcmu_enable,
+ .disable = clk_prcmu_disable,
+ .is_enabled = clk_prcmu_is_enabled,
+ .recalc_rate = clk_prcmu_recalc_rate,
+ .round_rate = clk_prcmu_round_rate,
+ .set_rate = clk_prcmu_set_rate,
+};
+
static struct clk *clk_reg_prcmu(const char *name,
const char *parent_name,
u8 cg_sel,
@@ -250,3 +295,13 @@ struct clk *clk_reg_prcmu_opp_gate(const char *name,
return clk_reg_prcmu(name, parent_name, cg_sel, 0, flags,
clk_prcmu_opp_gate_ops);
}
+
+struct clk *clk_reg_prcmu_opp_volt_scalable(const char *name,
+ const char *parent_name,
+ u8 cg_sel,
+ unsigned long rate,
+ unsigned long flags)
+{
+ return clk_reg_prcmu(name, parent_name, cg_sel, rate, flags,
+ clk_prcmu_opp_volt_scalable_ops);
+}
diff --git a/drivers/clk/ux500/clk.h b/drivers/clk/ux500/clk.h
index 836d7d1..f36eeed 100644
--- a/drivers/clk/ux500/clk.h
+++ b/drivers/clk/ux500/clk.h
@@ -45,4 +45,10 @@ struct clk *clk_reg_prcmu_opp_gate(const char *name,
u8 cg_sel,
unsigned long flags);
+struct clk *clk_reg_prcmu_opp_volt_scalable(const char *name,
+ const char *parent_name,
+ u8 cg_sel,
+ unsigned long rate,
+ unsigned long flags);
+
#endif /* __UX500_CLK_H */
--
1.7.10
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 3/3] clk: ux500: Update sdmmc clock to 100MHz for u8500
From: Ulf Hansson ulf.hans...@linaro.org For u8500 and using 100MHz as the frequency also requires the ape opp 100 voltage, thus use the prcmu_opp_volt_scalable clock type. Signed-off-by: Ulf Hansson ulf.hans...@linaro.org --- drivers/clk/ux500/u8500_clk.c |5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/clk/ux500/u8500_clk.c b/drivers/clk/ux500/u8500_clk.c index ca4a25e..7bebf1f 100644 --- a/drivers/clk/ux500/u8500_clk.c +++ b/drivers/clk/ux500/u8500_clk.c @@ -170,10 +170,11 @@ void u8500_clk_init(void) clk_register_clkdev(clk, NULL, mtu0); clk_register_clkdev(clk, NULL, mtu1); - clk = clk_reg_prcmu_gate(sdmmcclk, NULL, PRCMU_SDMMCCLK, CLK_IS_ROOT); + clk = clk_reg_prcmu_opp_volt_scalable(sdmmcclk, NULL, PRCMU_SDMMCCLK, + 1, + CLK_IS_ROOT|CLK_SET_RATE_GATE); clk_register_clkdev(clk, NULL, sdmmc); - clk = clk_reg_prcmu_scalable(dsi_pll, hdmiclk, PRCMU_PLLDSI, 0, CLK_SET_RATE_GATE); clk_register_clkdev(clk, dsihs2, mcde); -- 1.7.10 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/3] clk: ux500: Support prcmu ape opp voltage clock
Hi Mike,
Thanks for your input!
On 24 September 2012 17:35, Mike Turquette mturque...@ti.com wrote:
Quoting Ulf Hansson (2012-09-24 07:43:18)
From: Ulf Hansson ulf.hans...@linaro.org
Some scalable prcmu clocks needs to be handled in conjuction with the
ape opp 100 voltage. A new prcmu clock type clk_prcmu_opp_volt_scalable
is implemented to handle this.
Signed-off-by: Ulf Hansson ulf.hans...@linaro.org
---
drivers/clk/ux500/clk-prcmu.c | 55
+
drivers/clk/ux500/clk.h |6 +
2 files changed, 61 insertions(+)
diff --git a/drivers/clk/ux500/clk-prcmu.c b/drivers/clk/ux500/clk-prcmu.c
index 930cdfe..04577ca 100644
--- a/drivers/clk/ux500/clk-prcmu.c
+++ b/drivers/clk/ux500/clk-prcmu.c
@@ -133,6 +133,40 @@ out_error:
hw-init-name);
}
+static int clk_prcmu_opp_volt_prepare(struct clk_hw *hw)
+{
+ int err;
+ struct clk_prcmu *clk = to_clk_prcmu(hw);
+
+ err = prcmu_request_ape_opp_100_voltage(true);
+ if (err) {
+ pr_err(clk_prcmu: %s failed to request APE OPP VOLT for
%s.\n,
+ __func__, hw-init-name);
+ return err;
+ }
+
+ err = prcmu_request_clock(clk-cg_sel, true);
+ if (err)
+ prcmu_request_ape_opp_100_voltage(false);
+
+ return err;
+}
+
+static void clk_prcmu_opp_volt_unprepare(struct clk_hw *hw)
+{
+ struct clk_prcmu *clk = to_clk_prcmu(hw);
+
+ if (prcmu_request_clock(clk-cg_sel, false))
+ goto out_error;
+ if (prcmu_request_ape_opp_100_voltage(false))
+ goto out_error;
+ return;
+
+out_error:
+ pr_err(clk_prcmu: %s failed to disable %s.\n, __func__,
+ hw-init-name);
+}
Hello Ulf,
I was hoping to use the rate-change notifiers for voltage scaling, as in
my clk reentrancy/dvfs RFCs. Using prepare/unprepare to set voltage for
non-scalable clocks is probably OK; do you plan to scale voltage when
your clocks change rate? If so then you will need more than just
prepare/unprepare.
I see, you have a good point here. Although right now this clock will
never change rate except during init. Prepare/unprepare is thus
enough.
For the u8500 platform the sdmmc clock is the parent for all the
mmc/sd/sdio controllers. So, to no introduce a very complex
clockhandling mechanism (using rate-change notifiers would help a bit,
but is not enough), the clock is not supposed to be changed after
init. Proper dividing of the clock is handled internally by each
controller instead.
Regards,
Mike
+
static struct clk_ops clk_prcmu_scalable_ops = {
.prepare = clk_prcmu_prepare,
.unprepare = clk_prcmu_unprepare,
@@ -167,6 +201,17 @@ static struct clk_ops clk_prcmu_opp_gate_ops = {
.recalc_rate = clk_prcmu_recalc_rate,
};
+static struct clk_ops clk_prcmu_opp_volt_scalable_ops = {
+ .prepare = clk_prcmu_opp_volt_prepare,
+ .unprepare = clk_prcmu_opp_volt_unprepare,
+ .enable = clk_prcmu_enable,
+ .disable = clk_prcmu_disable,
+ .is_enabled = clk_prcmu_is_enabled,
+ .recalc_rate = clk_prcmu_recalc_rate,
+ .round_rate = clk_prcmu_round_rate,
+ .set_rate = clk_prcmu_set_rate,
+};
+
static struct clk *clk_reg_prcmu(const char *name,
const char *parent_name,
u8 cg_sel,
@@ -250,3 +295,13 @@ struct clk *clk_reg_prcmu_opp_gate(const char *name,
return clk_reg_prcmu(name, parent_name, cg_sel, 0, flags,
clk_prcmu_opp_gate_ops);
}
+
+struct clk *clk_reg_prcmu_opp_volt_scalable(const char *name,
+ const char *parent_name,
+ u8 cg_sel,
+ unsigned long rate,
+ unsigned long flags)
+{
+ return clk_reg_prcmu(name, parent_name, cg_sel, rate, flags,
+ clk_prcmu_opp_volt_scalable_ops);
+}
diff --git a/drivers/clk/ux500/clk.h b/drivers/clk/ux500/clk.h
index 836d7d1..f36eeed 100644
--- a/drivers/clk/ux500/clk.h
+++ b/drivers/clk/ux500/clk.h
@@ -45,4 +45,10 @@ struct clk *clk_reg_prcmu_opp_gate(const char *name,
u8 cg_sel,
unsigned long flags);
+struct clk *clk_reg_prcmu_opp_volt_scalable(const char *name,
+ const char *parent_name,
+ u8 cg_sel,
+ unsigned long rate,
+ unsigned long flags);
+
#endif /* __UX500_CLK_H */
--
1.7.10
Kind regards
Ulf Hansson
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
Re: [PATCH 2/3] clk: ux500: Support prcmu ape opp voltage clock
Hi Mike,
On 26 September 2012 21:25, Mike Turquette mturque...@ti.com wrote:
Quoting Ulf Hansson (2012-09-25 00:56:56)
Hi Mike,
Thanks for your input!
On 24 September 2012 17:35, Mike Turquette mturque...@ti.com wrote:
Quoting Ulf Hansson (2012-09-24 07:43:18)
From: Ulf Hansson ulf.hans...@linaro.org
Some scalable prcmu clocks needs to be handled in conjuction with the
ape opp 100 voltage. A new prcmu clock type clk_prcmu_opp_volt_scalable
is implemented to handle this.
Signed-off-by: Ulf Hansson ulf.hans...@linaro.org
---
drivers/clk/ux500/clk-prcmu.c | 55
+
drivers/clk/ux500/clk.h |6 +
2 files changed, 61 insertions(+)
diff --git a/drivers/clk/ux500/clk-prcmu.c b/drivers/clk/ux500/clk-prcmu.c
index 930cdfe..04577ca 100644
--- a/drivers/clk/ux500/clk-prcmu.c
+++ b/drivers/clk/ux500/clk-prcmu.c
@@ -133,6 +133,40 @@ out_error:
hw-init-name);
}
+static int clk_prcmu_opp_volt_prepare(struct clk_hw *hw)
+{
+ int err;
+ struct clk_prcmu *clk = to_clk_prcmu(hw);
+
+ err = prcmu_request_ape_opp_100_voltage(true);
+ if (err) {
+ pr_err(clk_prcmu: %s failed to request APE OPP VOLT for
%s.\n,
+ __func__, hw-init-name);
+ return err;
+ }
+
+ err = prcmu_request_clock(clk-cg_sel, true);
+ if (err)
+ prcmu_request_ape_opp_100_voltage(false);
+
+ return err;
+}
+
+static void clk_prcmu_opp_volt_unprepare(struct clk_hw *hw)
+{
+ struct clk_prcmu *clk = to_clk_prcmu(hw);
+
+ if (prcmu_request_clock(clk-cg_sel, false))
+ goto out_error;
+ if (prcmu_request_ape_opp_100_voltage(false))
+ goto out_error;
+ return;
+
+out_error:
+ pr_err(clk_prcmu: %s failed to disable %s.\n, __func__,
+ hw-init-name);
+}
Hello Ulf,
I was hoping to use the rate-change notifiers for voltage scaling, as in
my clk reentrancy/dvfs RFCs. Using prepare/unprepare to set voltage for
non-scalable clocks is probably OK; do you plan to scale voltage when
your clocks change rate? If so then you will need more than just
prepare/unprepare.
I see, you have a good point here. Although right now this clock will
never change rate except during init. Prepare/unprepare is thus
enough.
For the u8500 platform the sdmmc clock is the parent for all the
mmc/sd/sdio controllers. So, to no introduce a very complex
clockhandling mechanism (using rate-change notifiers would help a bit,
but is not enough), the clock is not supposed to be changed after
init. Proper dividing of the clock is handled internally by each
controller instead.
The internal dividers could be modeled as clk nodes, which themselves
could have notifier handlers that scale voltage. So perhaps the MMC
controller driver for your platform should define the appropriate leaf
clks?
I am not sure I completely follow you here, but let me try to answer
your suggestions.
The internal dividers are supposed to be handled only by the mmc host
driver since it is IP specific. I don't think it is viable to add
another clock type/node for this, but maybe that is not what you are
suggesting either. :-)
In my case for ux500 we use the mmci host driver. It is an ARM cross
SoC device driver, and I believe adding clock notifiers in the driver
to handle the voltage scale would complicate things quite a bit. It
seems a lot easier to let the clock driver handle this, especially
since the frequency will remain fixed after initialization. So in the
end I see now need and benefit for adding the clock notifiers in the
mmci host driver.
Kind regards
Ulf Hansson
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] mfd: dbx500: Export prmcu_request_ape_opp_100_voltage
Hi Sam and Linus,
Do you see any issues with this patch? Can we advise Mike to merge it
though his clock tree?
Kind regards
Ulf Hansson
On 24 September 2012 16:43, Ulf Hansson ulf.hans...@stericsson.com wrote:
From: Ulf Hansson ulf.hans...@linaro.org
This function needs to be exported to let clients be able to
request the ape opp 100 voltage.
Cc: Samuel Ortiz sa...@linux.intel.com
Signed-off-by: Ulf Hansson ulf.hans...@linaro.org
---
drivers/mfd/db8500-prcmu.c |4 ++--
include/linux/mfd/db8500-prcmu.h |4 ++--
include/linux/mfd/dbx500-prcmu.h | 10 ++
3 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/mfd/db8500-prcmu.c b/drivers/mfd/db8500-prcmu.c
index e7f9539..0b8e0a0 100644
--- a/drivers/mfd/db8500-prcmu.c
+++ b/drivers/mfd/db8500-prcmu.c
@@ -1167,12 +1167,12 @@ int db8500_prcmu_get_ape_opp(void)
}
/**
- * prcmu_request_ape_opp_100_voltage - Request APE OPP 100% voltage
+ * db8500_prcmu_request_ape_opp_100_voltage - Request APE OPP 100% voltage
* @enable: true to request the higher voltage, false to drop a request.
*
* Calls to this function to enable and disable requests must be balanced.
*/
-int prcmu_request_ape_opp_100_voltage(bool enable)
+int db8500_prcmu_request_ape_opp_100_voltage(bool enable)
{
int r = 0;
u8 header;
diff --git a/include/linux/mfd/db8500-prcmu.h
b/include/linux/mfd/db8500-prcmu.h
index b82f6ee..6ee4247 100644
--- a/include/linux/mfd/db8500-prcmu.h
+++ b/include/linux/mfd/db8500-prcmu.h
@@ -515,7 +515,6 @@ enum romcode_read prcmu_get_rc_p2a(void);
enum ap_pwrst prcmu_get_xp70_current_state(void);
bool prcmu_has_arm_maxopp(void);
struct prcmu_fw_version *prcmu_get_fw_version(void);
-int prcmu_request_ape_opp_100_voltage(bool enable);
int prcmu_release_usb_wakeup_state(void);
void prcmu_configure_auto_pm(struct prcmu_auto_pm_config *sleep,
struct prcmu_auto_pm_config *idle);
@@ -564,6 +563,7 @@ int db8500_prcmu_set_arm_opp(u8 opp);
int db8500_prcmu_get_arm_opp(void);
int db8500_prcmu_set_ape_opp(u8 opp);
int db8500_prcmu_get_ape_opp(void);
+int db8500_prcmu_request_ape_opp_100_voltage(bool enable);
int db8500_prcmu_set_ddr_opp(u8 opp);
int db8500_prcmu_get_ddr_opp(void);
@@ -610,7 +610,7 @@ static inline int db8500_prcmu_get_ape_opp(void)
return APE_100_OPP;
}
-static inline int prcmu_request_ape_opp_100_voltage(bool enable)
+static inline int db8500_prcmu_request_ape_opp_100_voltage(bool enable)
{
return 0;
}
diff --git a/include/linux/mfd/dbx500-prcmu.h
b/include/linux/mfd/dbx500-prcmu.h
index c410d99..c202d6c 100644
--- a/include/linux/mfd/dbx500-prcmu.h
+++ b/include/linux/mfd/dbx500-prcmu.h
@@ -336,6 +336,11 @@ static inline int prcmu_get_ape_opp(void)
return db8500_prcmu_get_ape_opp();
}
+static inline int prcmu_request_ape_opp_100_voltage(bool enable)
+{
+ return db8500_prcmu_request_ape_opp_100_voltage(enable);
+}
+
static inline void prcmu_system_reset(u16 reset_code)
{
return db8500_prcmu_system_reset(reset_code);
@@ -507,6 +512,11 @@ static inline int prcmu_get_ape_opp(void)
return APE_100_OPP;
}
+static inline int prcmu_request_ape_opp_100_voltage(bool enable)
+{
+ return 0;
+}
+
static inline int prcmu_set_arm_opp(u8 opp)
{
return 0;
--
1.7.10
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] Input: nomadik-ske-keypad - start using the apb_pclk
On 4 November 2012 19:12, Linus Walleij linus.wall...@linaro.org wrote: On Thu, Nov 1, 2012 at 3:20 PM, Ulf Hansson ulf.hans...@stericsson.com wrote: From: Ulf Hansson ulf.hans...@linaro.org Previously this clock was handled internally by the clockdriver, but now this is separate clk. So we need take care of it. Signed-off-by: Ulf Hansson ulf.hans...@linaro.org So this is a silicon block clock and falls into the category of things we've been discussing... If I understand correctly, the only real solution is to implement the PM domains and have these enable the clocks. Agree. Although, since the pm_domain not yet exist, this as a way forward for now - to fix what is broken. When the pm_domain is in place and when we decide to fold in the clock handling in there, we can move it. An alternative may be to move this driver over to the AMBA bus, because I think this device actually has primecell registers. Then the bus will take care of the pclk for starters. You are definitely right, this driver can be converted into using the AMBA bus. Although, do you think that should be done _instead_ of going ahead with this patch or do you want that to be handled as a next and a separate step? Kind regards Ulf Hansson -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] ASoC: Ux500: Fixup use of clocks
On 22 October 2012 15:30, Mark Brown broo...@opensource.wolfsonmicro.com wrote: On Mon, Oct 22, 2012 at 02:32:04PM +0200, Ulf Hansson wrote: From: Ulf Hansson ulf.hans...@linaro.org Make sure clocks are being prepared and unprepared as well as enabled and disabled. Applied, thanks. I can not find this patch on any next-tree yet. Same goes for: [PATCH 2/2] ASoC: Ux500: Control apb clock. Maybe I should be more patient, but thought it make sense to send a ping on this. :-) Kind regards Ulf Hansson -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RESEND PATCH 2/2] ASoC: Ux500: Control apb clock
From: Ulf Hansson ulf.hans...@linaro.org
When switching to common clock driver for ux500 this clock needs to
be handled as well. Before this clock was internally managed by the
clock driver itself.
Acked-by: Linus Walleij linus.wall...@linaro.org
Signed-off-by: Ulf Hansson ulf.hans...@linaro.org
---
sound/soc/ux500/ux500_msp_dai.c | 38 --
sound/soc/ux500/ux500_msp_dai.h |1 +
2 files changed, 33 insertions(+), 6 deletions(-)
diff --git a/sound/soc/ux500/ux500_msp_dai.c b/sound/soc/ux500/ux500_msp_dai.c
index e11187f..74bb3c0 100644
--- a/sound/soc/ux500/ux500_msp_dai.c
+++ b/sound/soc/ux500/ux500_msp_dai.c
@@ -398,13 +398,28 @@ static int ux500_msp_dai_startup(struct snd_pcm_substream
*substream,
return ret;
}
- /* Prepare and enable clock */
- dev_dbg(dai-dev, %s: Enabling MSP-clock.\n, __func__);
+ /* Prepare and enable clocks */
+ dev_dbg(dai-dev, %s: Enabling MSP-clocks.\n, __func__);
+ ret = clk_prepare_enable(drvdata-pclk);
+ if (ret) {
+ dev_err(drvdata-msp-dev,
+ %s: Failed to prepare/enable pclk!\n, __func__);
+ goto err_pclk;
+ }
+
ret = clk_prepare_enable(drvdata-clk);
- if (ret)
- regulator_disable(drvdata-reg_vape);
+ if (ret) {
+ dev_err(drvdata-msp-dev,
+ %s: Failed to prepare/enable clk!\n, __func__);
+ goto err_clk;
+ }
return ret;
+err_clk:
+ clk_disable_unprepare(drvdata-pclk);
+err_pclk:
+ regulator_disable(drvdata-reg_vape);
+ return ret;
}
static void ux500_msp_dai_shutdown(struct snd_pcm_substream *substream,
@@ -430,8 +445,9 @@ static void ux500_msp_dai_shutdown(struct snd_pcm_substream
*substream,
__func__, dai-id, snd_pcm_stream_str(substream));
}
- /* Disable and unprepare clock */
+ /* Disable and unprepare clocks */
clk_disable_unprepare(drvdata-clk);
+ clk_disable_unprepare(drvdata-pclk);
/* Disable regulator */
ret = regulator_disable(drvdata-reg_vape);
@@ -782,6 +798,14 @@ static int __devinit ux500_msp_drv_probe(struct
platform_device *pdev)
}
prcmu_qos_add_requirement(PRCMU_QOS_APE_OPP, (char *)pdev-name, 50);
+ drvdata-pclk = clk_get(pdev-dev, apb_pclk);
+ if (IS_ERR(drvdata-pclk)) {
+ ret = (int)PTR_ERR(drvdata-pclk);
+ dev_err(pdev-dev, %s: ERROR: clk_get of pclk failed (%d)!\n,
+ __func__, ret);
+ goto err_pclk;
+ }
+
drvdata-clk = clk_get(pdev-dev, NULL);
if (IS_ERR(drvdata-clk)) {
ret = (int)PTR_ERR(drvdata-clk);
@@ -812,8 +836,9 @@ static int __devinit ux500_msp_drv_probe(struct
platform_device *pdev)
err_init_msp:
clk_put(drvdata-clk);
-
err_clk:
+ clk_put(drvdata-pclk);
+err_pclk:
devm_regulator_put(drvdata-reg_vape);
return ret;
@@ -829,6 +854,7 @@ static int __devexit ux500_msp_drv_remove(struct
platform_device *pdev)
prcmu_qos_remove_requirement(PRCMU_QOS_APE_OPP, ux500_msp_i2s);
clk_put(drvdata-clk);
+ clk_put(drvdata-pclk);
ux500_msp_i2s_cleanup_msp(pdev, drvdata-msp);
diff --git a/sound/soc/ux500/ux500_msp_dai.h b/sound/soc/ux500/ux500_msp_dai.h
index 98202a3..9c778d9 100644
--- a/sound/soc/ux500/ux500_msp_dai.h
+++ b/sound/soc/ux500/ux500_msp_dai.h
@@ -69,6 +69,7 @@ struct ux500_msp_i2s_drvdata {
/* Clocks */
unsigned int master_clk;
struct clk *clk;
+ struct clk *pclk;
/* Regulators */
int vape_opp_constraint;
--
1.7.10
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[RESEND PATCH 1/2] ASoC: Ux500: Fixup use of clocks
From: Ulf Hansson ulf.hans...@linaro.org Make sure clocks are being prepared and unprepared as well as enabled and disabled. Acked-by: Linus Walleij linus.wall...@linaro.org Signed-off-by: Ulf Hansson ulf.hans...@linaro.org --- sound/soc/ux500/ux500_msp_dai.c | 12 +++- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/sound/soc/ux500/ux500_msp_dai.c b/sound/soc/ux500/ux500_msp_dai.c index be94bf9..e11187f 100644 --- a/sound/soc/ux500/ux500_msp_dai.c +++ b/sound/soc/ux500/ux500_msp_dai.c @@ -398,11 +398,13 @@ static int ux500_msp_dai_startup(struct snd_pcm_substream *substream, return ret; } - /* Enable clock */ + /* Prepare and enable clock */ dev_dbg(dai-dev, %s: Enabling MSP-clock.\n, __func__); - clk_enable(drvdata-clk); + ret = clk_prepare_enable(drvdata-clk); + if (ret) + regulator_disable(drvdata-reg_vape); - return 0; + return ret; } static void ux500_msp_dai_shutdown(struct snd_pcm_substream *substream, @@ -428,8 +430,8 @@ static void ux500_msp_dai_shutdown(struct snd_pcm_substream *substream, __func__, dai-id, snd_pcm_stream_str(substream)); } - /* Disable clock */ - clk_disable(drvdata-clk); + /* Disable and unprepare clock */ + clk_disable_unprepare(drvdata-clk); /* Disable regulator */ ret = regulator_disable(drvdata-reg_vape); -- 1.7.10 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] ASoC: Ux500: Fixup complile errors due to merge
From: Ulf Hansson ulf.hans...@linaro.org
Likely during merge of the below commits ended up breaking compilation:
ASoC: Ux500: Enable ux500 MSP driver for Device Tree
ASoC: ux500_msp_i2s: better use devm functions and fix error return code
Signed-off-by: Ulf Hansson ulf.hans...@linaro.org
---
sound/soc/ux500/ux500_msp_i2s.c | 18 +++---
1 file changed, 7 insertions(+), 11 deletions(-)
diff --git a/sound/soc/ux500/ux500_msp_i2s.c b/sound/soc/ux500/ux500_msp_i2s.c
index b7c996e..e6ff328 100644
--- a/sound/soc/ux500/ux500_msp_i2s.c
+++ b/sound/soc/ux500/ux500_msp_i2s.c
@@ -692,19 +692,15 @@ int ux500_msp_i2s_init_msp(struct platform_device *pdev,
if (!msp)
return -ENOMEM;
- if (np) {
- if (!platform_data) {
- platform_data = devm_kzalloc(pdev-dev,
- sizeof(struct msp_i2s_platform_data),
GFP_KERNEL);
- if (!platform_data)
- ret = -ENOMEM;
- }
- } else
+ if (np !platform_data) {
+ platform_data = devm_kzalloc(pdev-dev,
+ sizeof(struct msp_i2s_platform_data), GFP_KERNEL);
if (!platform_data)
- ret = -EINVAL;
+ return -ENOMEM;
+ }
- if (ret)
- goto err_res;
+ if (!platform_data)
+ return -EINVAL;
dev_dbg(pdev-dev, %s: Enter (name: %s, id: %d).\n, __func__,
pdev-name, platform_data-id);
--
1.7.10
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2] ASoC: Ux500: Fixup compile error
From: Ulf Hansson ulf.hans...@linaro.org The below commit introduced a compile error for a missing include file. ASoC: ux500_msp_i2s: better use devm functions and fix error return code Signed-off-by: Ulf Hansson ulf.hans...@linaro.org --- sound/soc/ux500/ux500_msp_i2s.c |1 + 1 file changed, 1 insertion(+) diff --git a/sound/soc/ux500/ux500_msp_i2s.c b/sound/soc/ux500/ux500_msp_i2s.c index e6ff328..ba15351 100644 --- a/sound/soc/ux500/ux500_msp_i2s.c +++ b/sound/soc/ux500/ux500_msp_i2s.c @@ -18,6 +18,7 @@ #include linux/pinctrl/consumer.h #include linux/delay.h #include linux/slab.h +#include linux/io.h #include linux/of.h #include mach/hardware.h -- 1.7.10 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/6] mmc: core: sdio_io.c: Fixed lines with 80 chars
Hi Sangho,
If you really must do this kind of clean up patches; from my point of
view I would suggest you to collect them in one mmc: core: Code
cleanup patch instead.
Kind regards
Ulf Hansson
On 18 October 2012 18:58, Sangho Yi antir...@gmail.com wrote:
I fixed lines over 80 characters per line.
Signed-off-by: Sangho Yi antir...@gmail.com
---
drivers/mmc/core/sdio_io.c |6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/core/sdio_io.c b/drivers/mmc/core/sdio_io.c
index 8f6f5ac..c5f53d8 100644
--- a/drivers/mmc/core/sdio_io.c
+++ b/drivers/mmc/core/sdio_io.c
@@ -80,7 +80,8 @@ int sdio_enable_func(struct sdio_func *func)
timeout = jiffies + msecs_to_jiffies(func-enable_timeout);
while (1) {
- ret = mmc_io_rw_direct(func-card, 0, 0, SDIO_CCCR_IORx, 0,
reg);
+ ret = mmc_io_rw_direct(func-card, 0, 0, SDIO_CCCR_IORx, 0,
+ reg);
if (ret)
goto err;
if (reg (1 func-num))
@@ -664,7 +665,8 @@ void sdio_f0_writeb(struct sdio_func *func, unsigned char
b, unsigned int addr,
BUG_ON(!func);
- if ((addr 0xF0 || addr 0xFF)
(!mmc_card_lenient_fn0(func-card))) {
+ if ((addr 0xF0 || addr 0xFF)
+ (!mmc_card_lenient_fn0(func-card))) {
if (err_ret)
*err_ret = -EINVAL;
return;
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2] ASoC: Ux500: Control apb clock
From: Ulf Hansson ulf.hans...@linaro.org
When switching to common clock driver for ux500 this clock needs to
be handled as well. Before this clock was internally managed by the
clock driver itself.
Signed-off-by: Ulf Hansson ulf.hans...@linaro.org
---
sound/soc/ux500/ux500_msp_dai.c | 38 --
sound/soc/ux500/ux500_msp_dai.h |1 +
2 files changed, 33 insertions(+), 6 deletions(-)
diff --git a/sound/soc/ux500/ux500_msp_dai.c b/sound/soc/ux500/ux500_msp_dai.c
index e11187f..74bb3c0 100644
--- a/sound/soc/ux500/ux500_msp_dai.c
+++ b/sound/soc/ux500/ux500_msp_dai.c
@@ -398,13 +398,28 @@ static int ux500_msp_dai_startup(struct snd_pcm_substream
*substream,
return ret;
}
- /* Prepare and enable clock */
- dev_dbg(dai-dev, %s: Enabling MSP-clock.\n, __func__);
+ /* Prepare and enable clocks */
+ dev_dbg(dai-dev, %s: Enabling MSP-clocks.\n, __func__);
+ ret = clk_prepare_enable(drvdata-pclk);
+ if (ret) {
+ dev_err(drvdata-msp-dev,
+ %s: Failed to prepare/enable pclk!\n, __func__);
+ goto err_pclk;
+ }
+
ret = clk_prepare_enable(drvdata-clk);
- if (ret)
- regulator_disable(drvdata-reg_vape);
+ if (ret) {
+ dev_err(drvdata-msp-dev,
+ %s: Failed to prepare/enable clk!\n, __func__);
+ goto err_clk;
+ }
return ret;
+err_clk:
+ clk_disable_unprepare(drvdata-pclk);
+err_pclk:
+ regulator_disable(drvdata-reg_vape);
+ return ret;
}
static void ux500_msp_dai_shutdown(struct snd_pcm_substream *substream,
@@ -430,8 +445,9 @@ static void ux500_msp_dai_shutdown(struct snd_pcm_substream
*substream,
__func__, dai-id, snd_pcm_stream_str(substream));
}
- /* Disable and unprepare clock */
+ /* Disable and unprepare clocks */
clk_disable_unprepare(drvdata-clk);
+ clk_disable_unprepare(drvdata-pclk);
/* Disable regulator */
ret = regulator_disable(drvdata-reg_vape);
@@ -782,6 +798,14 @@ static int __devinit ux500_msp_drv_probe(struct
platform_device *pdev)
}
prcmu_qos_add_requirement(PRCMU_QOS_APE_OPP, (char *)pdev-name, 50);
+ drvdata-pclk = clk_get(pdev-dev, apb_pclk);
+ if (IS_ERR(drvdata-pclk)) {
+ ret = (int)PTR_ERR(drvdata-pclk);
+ dev_err(pdev-dev, %s: ERROR: clk_get of pclk failed (%d)!\n,
+ __func__, ret);
+ goto err_pclk;
+ }
+
drvdata-clk = clk_get(pdev-dev, NULL);
if (IS_ERR(drvdata-clk)) {
ret = (int)PTR_ERR(drvdata-clk);
@@ -812,8 +836,9 @@ static int __devinit ux500_msp_drv_probe(struct
platform_device *pdev)
err_init_msp:
clk_put(drvdata-clk);
-
err_clk:
+ clk_put(drvdata-pclk);
+err_pclk:
devm_regulator_put(drvdata-reg_vape);
return ret;
@@ -829,6 +854,7 @@ static int __devexit ux500_msp_drv_remove(struct
platform_device *pdev)
prcmu_qos_remove_requirement(PRCMU_QOS_APE_OPP, ux500_msp_i2s);
clk_put(drvdata-clk);
+ clk_put(drvdata-pclk);
ux500_msp_i2s_cleanup_msp(pdev, drvdata-msp);
diff --git a/sound/soc/ux500/ux500_msp_dai.h b/sound/soc/ux500/ux500_msp_dai.h
index 98202a3..9c778d9 100644
--- a/sound/soc/ux500/ux500_msp_dai.h
+++ b/sound/soc/ux500/ux500_msp_dai.h
@@ -69,6 +69,7 @@ struct ux500_msp_i2s_drvdata {
/* Clocks */
unsigned int master_clk;
struct clk *clk;
+ struct clk *pclk;
/* Regulators */
int vape_opp_constraint;
--
1.7.10
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] ASoC: Ux500: Fixup use of clocks
From: Ulf Hansson ulf.hans...@linaro.org Make sure clocks are being prepared and unprepared as well as enabled and disabled. Signed-off-by: Ulf Hansson ulf.hans...@linaro.org --- sound/soc/ux500/ux500_msp_dai.c | 12 +++- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/sound/soc/ux500/ux500_msp_dai.c b/sound/soc/ux500/ux500_msp_dai.c index be94bf9..e11187f 100644 --- a/sound/soc/ux500/ux500_msp_dai.c +++ b/sound/soc/ux500/ux500_msp_dai.c @@ -398,11 +398,13 @@ static int ux500_msp_dai_startup(struct snd_pcm_substream *substream, return ret; } - /* Enable clock */ + /* Prepare and enable clock */ dev_dbg(dai-dev, %s: Enabling MSP-clock.\n, __func__); - clk_enable(drvdata-clk); + ret = clk_prepare_enable(drvdata-clk); + if (ret) + regulator_disable(drvdata-reg_vape); - return 0; + return ret; } static void ux500_msp_dai_shutdown(struct snd_pcm_substream *substream, @@ -428,8 +430,8 @@ static void ux500_msp_dai_shutdown(struct snd_pcm_substream *substream, __func__, dai-id, snd_pcm_stream_str(substream)); } - /* Disable clock */ - clk_disable(drvdata-clk); + /* Disable and unprepare clock */ + clk_disable_unprepare(drvdata-clk); /* Disable regulator */ ret = regulator_disable(drvdata-reg_vape); -- 1.7.10 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ASoC: Ux500: Control apb clock
On 22 October 2012 15:32, Mark Brown broo...@opensource.wolfsonmicro.com wrote: On Mon, Oct 22, 2012 at 02:32:05PM +0200, Ulf Hansson wrote: From: Ulf Hansson ulf.hans...@linaro.org When switching to common clock driver for ux500 this clock needs to be handled as well. Before this clock was internally managed by the clock driver itself. Applied but why isn't this a deficiency in the common clock implementation for the platform? Good question. :-) Patches have just been sent to Mike Turquette clk tree for adding the clock lookups. First, I thought I might wanted to group them all in a series but decided that splitting them are probably easier to handle. Kind regards Ulf Hansson -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ASoC: ux500_msp_i2s: Fix devm_* and return code merge error
Acked-by: Ulf Hansson ulf.hans...@linaro.org I assume this shall go into 3.7, right? Kind regards Ulf Hansson On 15 October 2012 15:13, Lee Jones lee.jo...@linaro.org wrote: Some ux500_msp_i2s patches clashed with: b18e93a493626c1446f9788ebd5844d008bbf71c ASoC: ux500_msp_i2s: better use devm functions and fix error return code ... leaving the driver uncompilable. This patch fixes the issues encountered. Cc: alsa-de...@alsa-project.org Cc: Liam Girdwood l...@ti.com Cc: Mark Brown broo...@opensource.wolfsonmicro.com Acked-by: Linus Walleij linus.wall...@linaro.org Signed-off-by: Lee Jones lee.jo...@linaro.org --- sound/soc/ux500/ux500_msp_i2s.c |8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/sound/soc/ux500/ux500_msp_i2s.c b/sound/soc/ux500/ux500_msp_i2s.c index b7c996e..a26c6bf 100644 --- a/sound/soc/ux500/ux500_msp_i2s.c +++ b/sound/soc/ux500/ux500_msp_i2s.c @@ -18,6 +18,7 @@ #include linux/pinctrl/consumer.h #include linux/delay.h #include linux/slab.h +#include linux/io.h #include linux/of.h #include mach/hardware.h @@ -697,14 +698,11 @@ int ux500_msp_i2s_init_msp(struct platform_device *pdev, platform_data = devm_kzalloc(pdev-dev, sizeof(struct msp_i2s_platform_data), GFP_KERNEL); if (!platform_data) - ret = -ENOMEM; + return -ENOMEM; } } else if (!platform_data) - ret = -EINVAL; - - if (ret) - goto err_res; + return -EINVAL; dev_dbg(pdev-dev, %s: Enter (name: %s, id: %d).\n, __func__, pdev-name, platform_data-id); -- 1.7.9.5 ___ linux-arm-kernel mailing list linux-arm-ker...@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/6] pinctrl: Update clock handling for the pinctrl-nomadik GPIO driver
On 25 October 2012 10:23, Lee Jones lee.jo...@linaro.org wrote: On Thu, 25 Oct 2012, Linus Walleij wrote: On 10/25/2012 09:31 AM, Lee Jones wrote: This certainly doesn't fix the bug we spoke about. I believe Ulf is still working on that one. So do you want me to remove this patch? Yeah drop it for now. Actually, a quick question before I do: If it's better/faster to prepare the clock and keep it prepared while you do clk_enable/clk_disable, why don't we do that in all drivers? Why do we bother to prepare/unprepare each time if all it does is take up cycles? Depending on clock type, a clk_disable is actually not going to gate the clock, that might happen only in unprepare. This depends on if the clock is a fast or slow clock. To save as much power as possible, in general, we should do both disable and unprepare. Although it will be device driver dependent were it is most convenient to do this things. Sometimes it is possible to group them sometimes not. Kind regards Ulf Hansson -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ASoC: ux500_msp_i2s: Fix devm_* and return code merge error
On 24 October 2012 12:27, Mark Brown broo...@opensource.wolfsonmicro.com wrote: On Wed, Oct 24, 2012 at 11:34:00AM +0200, Ulf Hansson wrote: Acked-by: Ulf Hansson ulf.hans...@linaro.org I assume this shall go into 3.7, right? Kind regards Ulf Hansson You may recall that only yesterday I reminded you not to top post... Sorry, I have a short memory. :-) I will try to improve. Kind regards Ulf Hansson -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] mmc: core: Support all MMC capabilities when booting from Device Tree
On 17 October 2012 15:38, Arnd Bergmann a...@arndb.de wrote: On Monday 15 October 2012, Lee Jones wrote: and so on. What are you actually missing in the properties that are already there? MMC_CAP_ERASE This one seems to be set unconditionally on some controllers but not on others. Why would it need to be configurable? MMC_CAP_UHS_SDR12 MMC_CAP_UHS_SDR25 MMC_CAP_UHS_DDR50 Could this be derived from max-frequency? No, this is likely depending on what the hw controller supports. Not connected to the freq. UHS also means 1.8 V I/O voltage. MMC_CAP_1_8V_DDR Right, I suppose we need this. Should we have a minimum and maximum voltage added to the common properties for this? MMC_CAP2_DETECT_ON_ERR MMC_CAP2_NO_SLEEP_CMD I don't see these ones being set anywhere, but they were both added by Ulf. Maybe he can comment on if or why they are needed in devicetree, rather than being set by the driver unconditionally or for specific versions of the host controller. From ux500 perspective there are patches not been up-streamed yet which are using these host caps, for whatever it is worth for you to know and consider. Actually, I think quite a few of the host caps in mmc could be debated whether those should exist at all. Some are directly mapped to what the host controller hw support, some are purely what the host driver (sw) support, but then there are others kind of mmc/sd/sdio software support configuration which are kind of strange host caps to me. For example MMC_CAP2_DETECT_ON_ERR which I invented. :-). I think it especially these software support configuration caps that might be causing this dt issues. Would be very interesting to hear if someone is sharing my thoughts around the host caps. Or if I am totally wrong here. Kind regards Ulf Hansson -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] mmc: card: Adding support for sanitize in eMMC 4.5
;
}
- if (mmc_can_sanitize(card))
- err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
-EXT_CSD_SANITIZE_START, 1, 0);
out_retry:
if (err !mmc_blk_reset(md, card-host, type))
goto retry;
diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c
index 9447a0e..fa9632e 100644
--- a/drivers/mmc/card/queue.c
+++ b/drivers/mmc/card/queue.c
@@ -173,7 +173,7 @@ static void mmc_queue_setup_discard(struct request_queue
*q,
/* granularity must not be greater than max. discard */
if (card-pref_erase max_discard)
q-limits.discard_granularity = 0;
- if (mmc_can_secure_erase_trim(card) || mmc_can_sanitize(card))
+ if (mmc_can_secure_erase_trim(card))
queue_flag_set_unlocked(QUEUE_FLAG_SECDISCARD, q);
}
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 37c5dad..dea9381 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -402,6 +402,27 @@ static int mmc_wait_for_data_req_done(struct mmc_host
*host,
context_info-is_done_rcv = false;
context_info-is_new_req = false;
cmd = mrq-cmd;
+
+ /*
+* If host has timed out waiting for the sanitize
+* to complete, card might be still in programming
state
+* so let's try to bring the card out of programming
+* state.
+*/
+ if (cmd-sanitize_busy cmd-error == -ETIMEDOUT) {
+ if (!mmc_interrupt_hpi(host-card)) {
+ pr_warning(%s: %s: Interrupted
+ sanitize\n,
+ mmc_hostname(host),
+ __func__);
+ cmd-error = 0;
+ break;
+ } else {
+ pr_err(%s: %s: Failed to interrupt
+ sanitize\n,
+ mmc_hostname(host), __func__);
+ }
+ }
if (!cmd-error || !cmd-retries ||
mmc_card_removed(host-card)) {
err = host-areq-err_check(host-card,
diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
index 39613b9..bd06ff5 100644
--- a/include/linux/mmc/core.h
+++ b/include/linux/mmc/core.h
@@ -96,6 +96,8 @@ struct mmc_command {
*/
unsigned intcmd_timeout_ms; /* in milliseconds */
+ /* Set this flag only for blocking sanitize request */
+ boolsanitize_busy;
struct mmc_data *data; /* data segment associated
with cmd */
struct mmc_request *mrq; /* associated request */
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index e326ae2..00c10be 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -281,6 +281,7 @@ struct mmc_host {
#define MMC_CAP2_PACKED_CMD(MMC_CAP2_PACKED_RD | \
MMC_CAP2_PACKED_WR)
#define MMC_CAP2_NO_PRESCAN_POWERUP (1 14) /* Don't power up before scan
*/
+#define MMC_CAP2_SANITIZE (1 15) /* Support Sanitize */
Is there a specific reason to why a cap is needed? Would it be okay to
use this feature just if the card supports it?
mmc_pm_flag_t pm_caps;/* supported pm features */
--
1.7.3.3
--
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Kind regards
Ulf Hansson
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mmc: mmci: Allow MMCI to request channels with information acquired from DT
On 18 April 2013 10:02, Lee Jones lee.jo...@linaro.org wrote:
I think you can further simplify this, given that in the DT case we always
allocate a zeroed mmci_platform_data for host-plat, so !plat cannot happen
when we get here.
Okay, third time lucky. :)
diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 372e921..3260ab4 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -298,14 +298,16 @@ static void mmci_init_sg(struct mmci_host *host, struct
mmc_data *data)
* no custom DMA interfaces are supported.
*/
#ifdef CONFIG_DMA_ENGINE
-static void mmci_dma_setup(struct mmci_host *host)
+static void mmci_dma_setup(struct amba_device *dev,
+ struct mmci_host *host)
Hi Lee,
You don't need to pass the amba_device, this can be fetched from the
mmci_host. Check how other functions converts the mmci host it in this
driver.
Kind regards
Ulf Hansson
{
+ struct device_node *np = dev-dev.of_node;
struct mmci_platform_data *plat = host-plat;
const char *rxname, *txname;
dma_cap_mask_t mask;
- if (!plat || !plat-dma_filter) {
- dev_info(mmc_dev(host-mmc), no DMA platform data\n);
+ if (!(plat-dma_filter || np)) {
+ dev_info(mmc_dev(host-mmc), no DMA platform data or DT\n);
return;
}
@@ -321,19 +323,21 @@ static void mmci_dma_setup(struct mmci_host *host)
* attempt to use it bidirectionally, however if it is
* is specified but cannot be located, DMA will be disabled.
*/
- if (plat-dma_rx_param) {
- host-dma_rx_channel = dma_request_channel(mask,
- plat-dma_filter,
-
plat-dma_rx_param);
+ if (plat-dma_rx_param || np) {
+ host-dma_rx_channel = dma_request_slave_channel_compat(mask,
+
plat-dma_filter,
+
plat-dma_rx_param,
+ dev-dev,
rx);
/* E.g if no DMA hardware is present */
if (!host-dma_rx_channel)
dev_err(mmc_dev(host-mmc), no RX DMA channel\n);
}
- if (plat-dma_tx_param) {
- host-dma_tx_channel = dma_request_channel(mask,
- plat-dma_filter,
-
plat-dma_tx_param);
+ if (plat-dma_tx_param || np) {
+ host-dma_tx_channel = dma_request_slave_channel_compat(mask,
+
plat-dma_filter,
+
plat-dma_tx_param,
+ dev-dev,
tx);
if (!host-dma_tx_channel)
dev_warn(mmc_dev(host-mmc), no TX DMA channel\n);
} else {
@@ -1538,7 +1542,7 @@ static int mmci_probe(struct amba_device *dev,
amba_rev(dev), (unsigned long long)dev-res.start,
dev-irq[0], dev-irq[1]);
- mmci_dma_setup(host);
+ mmci_dma_setup(dev, host);
pm_runtime_set_autosuspend_delay(dev-dev, 50);
pm_runtime_use_autosuspend(dev-dev);
--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] clk: abstract locking out into helper functions
On 28 March 2013 21:59, Mike Turquette mturque...@linaro.org wrote:
Create locking helpers for the global mutex and global spinlock. The
definitions of these helpers will be expanded upon in the next patch
which introduces reentrancy into the locking scheme.
Signed-off-by: Mike Turquette mturque...@linaro.org
Cc: Rajagopal Venkat rajagopal.ven...@linaro.org
Cc: David Brown dav...@codeaurora.org
Cc: Ulf Hansson ulf.hans...@linaro.org
Tested-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
Reviewed-by: Thomas Gleixner t...@linutronix.de
---
Changes since v5:
* pass flags by value instead of by reference in clk_enable_{un}lock
drivers/clk/clk.c | 99
+
1 file changed, 61 insertions(+), 38 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 5e8..0b5d612 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -27,6 +27,29 @@ static HLIST_HEAD(clk_root_list);
static HLIST_HEAD(clk_orphan_list);
static LIST_HEAD(clk_notifier_list);
+/*** locking ***/
+static void clk_prepare_lock(void)
+{
+ mutex_lock(prepare_lock);
+}
+
+static void clk_prepare_unlock(void)
+{
+ mutex_unlock(prepare_lock);
+}
+
+static unsigned long clk_enable_lock(void)
+{
+ unsigned long flags;
+ spin_lock_irqsave(enable_lock, flags);
+ return flags;
+}
+
+static void clk_enable_unlock(unsigned long flags)
+{
+ spin_unlock_irqrestore(enable_lock, flags);
+}
+
/***debugfs support***/
#ifdef CONFIG_COMMON_CLK_DEBUG
@@ -69,7 +92,7 @@ static int clk_summary_show(struct seq_file *s, void *data)
seq_printf(s,clockenable_cnt
prepare_cnt rate\n);
seq_printf(s,
-\n);
- mutex_lock(prepare_lock);
+ clk_prepare_lock();
hlist_for_each_entry(c, clk_root_list, child_node)
clk_summary_show_subtree(s, c, 0);
@@ -77,7 +100,7 @@ static int clk_summary_show(struct seq_file *s, void *data)
hlist_for_each_entry(c, clk_orphan_list, child_node)
clk_summary_show_subtree(s, c, 0);
- mutex_unlock(prepare_lock);
+ clk_prepare_unlock();
return 0;
}
@@ -130,7 +153,7 @@ static int clk_dump(struct seq_file *s, void *data)
seq_printf(s, {);
- mutex_lock(prepare_lock);
+ clk_prepare_lock();
hlist_for_each_entry(c, clk_root_list, child_node) {
if (!first_node)
@@ -144,7 +167,7 @@ static int clk_dump(struct seq_file *s, void *data)
clk_dump_subtree(s, c, 0);
}
- mutex_unlock(prepare_lock);
+ clk_prepare_unlock();
seq_printf(s, });
return 0;
@@ -316,7 +339,7 @@ static int __init clk_debug_init(void)
if (!orphandir)
return -ENOMEM;
- mutex_lock(prepare_lock);
+ clk_prepare_lock();
hlist_for_each_entry(clk, clk_root_list, child_node)
clk_debug_create_subtree(clk, rootdir);
@@ -326,7 +349,7 @@ static int __init clk_debug_init(void)
inited = 1;
- mutex_unlock(prepare_lock);
+ clk_prepare_unlock();
return 0;
}
@@ -372,7 +395,7 @@ static void clk_disable_unused_subtree(struct clk *clk)
hlist_for_each_entry(child, clk-children, child_node)
clk_disable_unused_subtree(child);
- spin_lock_irqsave(enable_lock, flags);
+ flags = clk_enable_lock();
if (clk-enable_count)
goto unlock_out;
@@ -393,7 +416,7 @@ static void clk_disable_unused_subtree(struct clk *clk)
}
unlock_out:
- spin_unlock_irqrestore(enable_lock, flags);
+ clk_enable_unlock(flags);
out:
return;
@@ -403,7 +426,7 @@ static int clk_disable_unused(void)
{
struct clk *clk;
- mutex_lock(prepare_lock);
+ clk_prepare_lock();
hlist_for_each_entry(clk, clk_root_list, child_node)
clk_disable_unused_subtree(clk);
@@ -417,7 +440,7 @@ static int clk_disable_unused(void)
hlist_for_each_entry(clk, clk_orphan_list, child_node)
clk_unprepare_unused_subtree(clk);
- mutex_unlock(prepare_lock);
+ clk_prepare_unlock();
return 0;
}
@@ -600,9 +623,9 @@ void __clk_unprepare(struct clk *clk)
*/
void clk_unprepare(struct clk *clk)
{
- mutex_lock(prepare_lock);
+ clk_prepare_lock();
__clk_unprepare(clk);
- mutex_unlock(prepare_lock);
+ clk_prepare_unlock();
}
EXPORT_SYMBOL_GPL(clk_unprepare);
@@ -648,9 +671,9 @@ int clk_prepare(struct clk *clk)
{
int ret;
- mutex_lock(prepare_lock);
+ clk_prepare_lock();
ret = __clk_prepare(clk);
- mutex_unlock(prepare_lock);
+ clk_prepare_unlock();
return
Re: [PATCH 2/2] clk: allow reentrant calls into the clk framework
On 28 March 2013 21:59, Mike Turquette mturque...@linaro.org wrote:
Reentrancy into the clock framework is necessary for clock operations
that result in nested calls to the clk api. A common example is a clock
that is prepared via an i2c transaction, such as a clock inside of a
discrete audio chip or a power management IC. The i2c subsystem itself
will use the clk api resulting in a deadlock:
clk_prepare(audio_clk)
i2c_transfer(..)
clk_prepare(i2c_controller_clk)
The ability to reenter the clock framework prevents this deadlock.
Other use cases exist such as allowing .set_rate callbacks to call
clk_set_parent to achieve the best rate, or to save power in certain
configurations. Yet another example is performing pinctrl operations
from a clk_ops callback. Calls into the pinctrl subsystem may call
clk_{un}prepare on an unrelated clock. Allowing for nested calls to
reenter the clock framework enables both of these use cases.
Reentrancy is implemented by two global pointers that track the owner
currently holding a global lock. One pointer tracks the owner during
sleepable, mutex-protected operations and the other one tracks the owner
during non-interruptible, spinlock-protected operations.
When the clk framework is entered we try to hold the global lock. If it
is held we compare the current task against the current owner; a match
implies a nested call and we reenter. If the values do not match then
we block on the lock until it is released.
Signed-off-by: Mike Turquette mturque...@linaro.org
Cc: Rajagopal Venkat rajagopal.ven...@linaro.org
Cc: David Brown dav...@codeaurora.org
Cc: Ulf Hansson ulf.hans...@linaro.org
Tested-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
Reviewed-by: Thomas Gleixner t...@linutronix.de
---
Changes since v5:
* fixed up typo in changelog
Changes since v4:
* remove uneccesary atomic operations
* remove casting bugs
* place reentrancy logic into locking helper functions
* improve debugging with reference counting and WARNs
drivers/clk/clk.c | 44 ++--
1 file changed, 42 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 0b5d612..0230c9d 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -19,10 +19,17 @@
#include linux/of.h
#include linux/device.h
#include linux/init.h
+#include linux/sched.h
static DEFINE_SPINLOCK(enable_lock);
static DEFINE_MUTEX(prepare_lock);
+static struct task_struct *prepare_owner;
+static struct task_struct *enable_owner;
+
+static int prepare_refcnt;
+static int enable_refcnt;
+
static HLIST_HEAD(clk_root_list);
static HLIST_HEAD(clk_orphan_list);
static LIST_HEAD(clk_notifier_list);
@@ -30,23 +37,56 @@ static LIST_HEAD(clk_notifier_list);
/*** locking ***/
static void clk_prepare_lock(void)
{
- mutex_lock(prepare_lock);
+ if (!mutex_trylock(prepare_lock)) {
+ if (prepare_owner == current) {
+ prepare_refcnt++;
+ return;
+ }
+ mutex_lock(prepare_lock);
+ }
+ WARN_ON_ONCE(prepare_owner != NULL);
+ WARN_ON_ONCE(prepare_refcnt != 0);
+ prepare_owner = current;
+ prepare_refcnt = 1;
}
static void clk_prepare_unlock(void)
{
+ WARN_ON_ONCE(prepare_owner != current);
+ WARN_ON_ONCE(prepare_refcnt == 0);
+
+ if (--prepare_refcnt)
+ return;
+ prepare_owner = NULL;
mutex_unlock(prepare_lock);
}
static unsigned long clk_enable_lock(void)
{
unsigned long flags;
- spin_lock_irqsave(enable_lock, flags);
+
+ if (!spin_trylock_irqsave(enable_lock, flags)) {
+ if (enable_owner == current) {
+ enable_refcnt++;
+ return flags;
+ }
+ spin_lock_irqsave(enable_lock, flags);
+ }
+ WARN_ON_ONCE(enable_owner != NULL);
+ WARN_ON_ONCE(enable_refcnt != 0);
+ enable_owner = current;
+ enable_refcnt = 1;
return flags;
}
static void clk_enable_unlock(unsigned long flags)
{
+ WARN_ON_ONCE(enable_owner != current);
+ WARN_ON_ONCE(enable_refcnt == 0);
+
+ if (--enable_refcnt)
+ return;
+ enable_owner = NULL;
spin_unlock_irqrestore(enable_lock, flags);
}
--
1.7.10.4
Great piece of work!
Reviewed-by: Ulf Hansson ulf.hans...@linaro.org
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] wait while adding MMC host to ensure root mounts
On 27 March 2013 13:28, Сергей Янович ynv...@gmail.com wrote: Hi Ulf, On 27 March 2013 15:13, Ulf Hansson ulf.hans...@linaro.org wrote: Moreover, this patch will have bad impact on booting the kernel, since every host device that has scheduled a detect work from it's probe function will also wait for it to finish. Even if it is the boot device of not. If this is needed, I would prefer that a host cap is used. Do you have any profiling data supporting bad impact on boot? It should be it in the range of dozens us, if any. Without the patch, approx. 30% of boots succeeded with CONFIG_PREEMT and aprox. 95% without CONFIG_PREEMT. mmc_rescan is just a few instructions, if there is no card present. On boot and with a card present, it might only sleep in the host implementation. Anyway, something had to be done about mmc boot. rootdelay will not report error if the card is absent or its partition table is damaged. rootdelay is a workaround by definition, so it may occasionally fail if it is too small. Big rootdealy has a clear bad impact on boot time. Consider a platform having two eMMCs and one SD-card. Each eMMC card will take around 400 ms to initialize and the SD-card 700 ms. The card initialization times are real examples from eMMCs and SD-cards, moreover those are typical values not worth cases. In total we have around 1.5 s to initialize the cards. Now, suppose you boot using an initrd image. Thus neither of the cards needs to be accessible immediately after the kernel has booted. It all depends what the init process decides to do. With this patch the init process will always be delayed to wait for each and every card to be initialized. I would prefer a solution where this can be configurable somehow, since certainly this is not the scenario you want for all cases. Kind regards Ulf Hansson -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] wait while adding MMC host to ensure root mounts
On 2 April 2013 12:35, Sergey Yanovich ynv...@gmail.com wrote: On Tue, 2013-04-02 at 12:13 +0200, Ulf Hansson wrote: Consider a platform having two eMMCs and one SD-card. Each eMMC card will take around 400 ms to initialize and the SD-card 700 ms. The card initialization times are real examples from eMMCs and SD-cards, moreover those are typical values not worth cases. In total we have around 1.5 s to initialize the cards. We have a separate workqueue per host, so all probes go in parallel. They also go in parallel with probing for other devices. So the actual delay is 700 ms minus maximum probing time for other devices. The time is either zero or small (dozen us) on the hardware I have access to (intel laptop, arm controller). Now, suppose you boot using an initrd image. Thus neither of the cards needs to be accessible immediately after the kernel has booted. It all depends what the init process decides to do. With this patch the init process will always be delayed to wait for each and every card to be initialized. I would prefer a solution where this can be configurable somehow, since certainly this is not the scenario you want for all cases. If the system is booted using initrd and root is not on an mmc card, then mmc modules can be omitted from initrd. The probing will happen only after root is mounted. This will not solve the problem when having one device intended for rootfs and some other for something else. Of course, as long as the devices uses the same mmc module. Once inserted, all devices will be probed. If root is on an mmc, kernel needs to wait for the mmc probe. True, although your patch is preventing the parallelism and instead doing things in synchronized manner. I think we must discuss alternative solutions instead. Like an mmc detect flush mechanism or a new card device notification event. Kind regards Ulf Hansson -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC 1/1] clk: Add notifier support in clk_prepare_enable/clk_disable_unprepare
On 15 March 2013 10:39, Peter De Schrijver pdeschrij...@nvidia.com wrote: On Fri, Mar 15, 2013 at 06:22:47AM +0100, Stephen Warren wrote: On 03/14/2013 07:20 PM, Bill Huang wrote: On Fri, 2013-03-15 at 01:54 +0800, Stephen Warren wrote: On 03/14/2013 03:28 AM, Bill Huang wrote: On Thu, 2013-03-14 at 17:21 +0800, Peter De Schrijver wrote: On Thu, Mar 14, 2013 at 03:15:11AM +0100, Bill Huang wrote: I don't think deferring will work either, considering the usage of DVFS, device voltage is tightly coupled with frequency, when clock rate is about to increase, we have to boost voltage first and we can lower the voltage after the clock rate has decreased. All the above sequence have to be guaranteed or you might crash, so deferring not only make thing complicated in controlling the order but also hurt performance. But we could use notifiers in clk_prepare/clk_unprepare to set the voltage no? As clk_prepare/clk_unprepare have to be called before clk_enable or after clk_disable, the voltage can be raised to a safe level, before the clock becomes active. Thanks Peter, actually I'm just about to propose my v2 RFC which add notifier in clk_prepare/clk_unprepare. Can't clk_set_rate() be called while the clock is prepared, or even enabled? I don't see how your proposal would work. I think it works with just a little sacrifice on saving more power but that's related minor. Taking clk_prepare as an indicator on that clock will be enabled later, so we can raise the voltage to a safe level (according to the current rate or maybe default rate when clk_prepare is called, some time late when clk_set_rate() is called we can adjust again according to the requested rate change) Is clk_set_rate() only legal to call in non-atomic contexts then? The header file doesn't say, although I guess since many other functions explicitly say they can't, then by omission it can... Yes. Only clk_enable() and clk_disable() can be called in an atomic context. Cheers, Peter. ___ linaro-dev mailing list linaro-...@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev Just wanted to add my reflection on this topic; Some prerequisites; I think am in favor of using the clk API to trigger DVFS changes and then I agree on that clk_prepare|unprepare needs to be possible to track from a DVFS perspective. clk_set_rate is not enough. So if we decide to do the above (using the clk API to trigger DVFS changes), I believe we should discuss two possible solutions; - clk notifiers or.. - dvfs clock type. I am trying to make up my mind of what I think is the best solution. Have you considered dvfs clock type? I put some comments about this for [PATCH 2/5] clk: notifier handler for dynamic voltage scaling recently as well. What could the advantages/disadvantages be between the two options? Kind regards Ulf Hansson -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC 1/1] clk: Add notifier support in clk_prepare_enable/clk_disable_unprepare
On 15 March 2013 13:06, Bill Huang bilhu...@nvidia.com wrote: On Fri, 2013-03-15 at 18:08 +0800, Ulf Hansson wrote: On 15 March 2013 10:39, Peter De Schrijver pdeschrij...@nvidia.com wrote: On Fri, Mar 15, 2013 at 06:22:47AM +0100, Stephen Warren wrote: On 03/14/2013 07:20 PM, Bill Huang wrote: On Fri, 2013-03-15 at 01:54 +0800, Stephen Warren wrote: On 03/14/2013 03:28 AM, Bill Huang wrote: On Thu, 2013-03-14 at 17:21 +0800, Peter De Schrijver wrote: On Thu, Mar 14, 2013 at 03:15:11AM +0100, Bill Huang wrote: I don't think deferring will work either, considering the usage of DVFS, device voltage is tightly coupled with frequency, when clock rate is about to increase, we have to boost voltage first and we can lower the voltage after the clock rate has decreased. All the above sequence have to be guaranteed or you might crash, so deferring not only make thing complicated in controlling the order but also hurt performance. But we could use notifiers in clk_prepare/clk_unprepare to set the voltage no? As clk_prepare/clk_unprepare have to be called before clk_enable or after clk_disable, the voltage can be raised to a safe level, before the clock becomes active. Thanks Peter, actually I'm just about to propose my v2 RFC which add notifier in clk_prepare/clk_unprepare. Can't clk_set_rate() be called while the clock is prepared, or even enabled? I don't see how your proposal would work. I think it works with just a little sacrifice on saving more power but that's related minor. Taking clk_prepare as an indicator on that clock will be enabled later, so we can raise the voltage to a safe level (according to the current rate or maybe default rate when clk_prepare is called, some time late when clk_set_rate() is called we can adjust again according to the requested rate change) Is clk_set_rate() only legal to call in non-atomic contexts then? The header file doesn't say, although I guess since many other functions explicitly say they can't, then by omission it can... Yes. Only clk_enable() and clk_disable() can be called in an atomic context. Cheers, Peter. ___ linaro-dev mailing list linaro-...@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev Just wanted to add my reflection on this topic; Some prerequisites; I think am in favor of using the clk API to trigger DVFS changes and then I agree on that clk_prepare|unprepare needs to be possible to track from a DVFS perspective. clk_set_rate is not enough. So if we decide to do the above (using the clk API to trigger DVFS changes), I believe we should discuss two possible solutions; - clk notifiers or.. - dvfs clock type. I am trying to make up my mind of what I think is the best solution. Have you considered dvfs clock type? I put some comments about this for [PATCH 2/5] clk: notifier handler for dynamic voltage scaling recently as well. What could the advantages/disadvantages be between the two options? I personally prefer clk notifiers since that's easy and all the existing device drivers don't need to be modified, a new clock or API might be more thoroughly considered (and hence maybe more graceful) but that means we need more time to cook and many drivers need to plug into that API when it comes out, a lot of test/verification or maybe chaos follows, I'm not sure will that be a little overkill. I guess you did not fully got what I meant with dvfs clock type. It will not affect the clock API. But instead the dvfs is handled by implementing a specific clk hw type. So the same thing is accomplished as with clk notifiers, no changes should be needed to device drivers. The difference is only that no notifiers will be needed, and all the dvfs stuff will be handled in the clk hw instead. It will mean that we will bundle dvfs stuff into the clock drivers, instead of separating the code outside the clock drivers. But, on the other hand no notifiers will be needed. I'm not against designing a more robust API, I'm just suggesting before we have that defined we should at least let clk notifiers work. Kind regards Ulf Hansson Kind regards Ulf Hansson -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC 1/1] clk: Add notifier support in clk_prepare_enable/clk_disable_unprepare
On 15 March 2013 20:38, Stephen Warren swar...@wwwdotorg.org wrote: On 03/15/2013 06:33 AM, Ulf Hansson wrote: On 15 March 2013 13:06, Bill Huang bilhu...@nvidia.com wrote: On Fri, 2013-03-15 at 18:08 +0800, Ulf Hansson wrote: ... Some prerequisites; I think am in favor of using the clk API to trigger DVFS changes and then I agree on that clk_prepare|unprepare needs to be possible to track from a DVFS perspective. clk_set_rate is not enough. So if we decide to do the above (using the clk API to trigger DVFS changes), I believe we should discuss two possible solutions; - clk notifiers or.. - dvfs clock type. I am trying to make up my mind of what I think is the best solution. Have you considered dvfs clock type? I put some comments about this for [PATCH 2/5] clk: notifier handler for dynamic voltage scaling recently as well. What could the advantages/disadvantages be between the two options? I personally prefer clk notifiers since that's easy and all the existing device drivers don't need to be modified, a new clock or API might be more thoroughly considered (and hence maybe more graceful) but that means we need more time to cook and many drivers need to plug into that API when it comes out, a lot of test/verification or maybe chaos follows, I'm not sure will that be a little overkill. I guess you did not fully got what I meant with dvfs clock type. It will not affect the clock API. But instead the dvfs is handled by implementing a specific clk hw type. So the same thing is accomplished as with clk notifiers, no changes should be needed to device drivers. The difference is only that no notifiers will be needed, and all the dvfs stuff will be handled in the clk hw instead. It will mean that we will bundle dvfs stuff into the clock drivers, instead of separating the code outside the clock drivers. But, on the other hand no notifiers will be needed. The advantage here is that I assume that a notifier would continually have to check whether the clock being modified was one that the DVFS notifier cared about. By integrating the CVFS logic into the clk_hw itself, it'll only ever get executed for clocks that really care about DVFS. Presumably, the code that implements the clk_hw could also use some common DVFS library as part of the implementation, and still share code. Or perhaps, what about putting DVFS ops into a clk_hw alongside any other existing ops, and having the clock core call them whenever appropriate? Thanks for your comment Stephen. I agree to your reflections as well. It will probably be a more optimized solution going this direction and we don't have to add more clk notifier code to the clk API, which I guess is good. It would be interesting to get some input from some of the maintainers to this discussion as well, let's see. Kind regards Ulf Hansson -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V3 5/7] mmc: queue work on any cpu
On 18 March 2013 16:23, Viresh Kumar viresh.ku...@linaro.org wrote:
mmc uses workqueues for running mmc_rescan(). There is no real dependency of
scheduling these on the cpu which scheduled them.
On a idle system, it is observed that and idle cpu wakes up many times just to
service this work. It would be better if we can schedule it on a cpu which
isn't
idle to save on power.
By idle cpu (from scheduler's perspective) we mean:
- Current task is idle task
- nr_running == 0
- wake_list is empty
This patch replaces the queue_delayed_work() with
queue_delayed_work_on_any_cpu() siblings.
This routine would look for the closest (via scheduling domains) non-idle cpu
(non-idle from schedulers perspective). If the current cpu is not idle or all
cpus are idle, work will be scheduled on local cpu.
Cc: Chris Ball c...@laptop.org
Cc: linux-...@vger.kernel.org
Signed-off-by: Viresh Kumar viresh.ku...@linaro.org
---
drivers/mmc/core/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 9290bb5..adf331a 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -85,7 +85,7 @@ MODULE_PARM_DESC(
static int mmc_schedule_delayed_work(struct delayed_work *work,
unsigned long delay)
{
- return queue_delayed_work(workqueue, work, delay);
+ return queue_delayed_work_on_any_cpu(workqueue, work, delay);
}
/*
--
1.7.12.rc2.18.g61b472e
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Acked-by: Ulf Hansson ulf.hans...@linaro.org
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] clk: Add notifier support in clk_prepare/clk_unprepare
On 20 March 2013 15:47, Mike Turquette mturque...@linaro.org wrote: Quoting Bill Huang (2013-03-19 21:39:44) On Wed, 2013-03-20 at 11:31 +0800, Mike Turquette wrote: Quoting Bill Huang (2013-03-19 19:55:49) On Wed, 2013-03-20 at 01:01 +0800, Mike Turquette wrote: Quoting Bill Huang (2013-03-19 06:28:32) Add notifier calls in clk_prepare and clk_unprepare so drivers which are interested in knowing that clk_prepare/unprepare call can act accordingly. The existing clk_set_rate notifier is not enough for normal DVFS inplementation since clock might be enabled/disabled at runtime. Adding these notifiers is useful on DVFS core which take clk_prepare as a hint on that the notified clock might be enabled later so it can raise voltage to a safe level before enabling the clock, and take clk_unprepare as a hint that the clock has been disabled and is safe to lower the voltage. The added notifier events are: PRE_CLK_PREPARE POST_CLK_PREPARE ABORT_CLK_PREPARE PRE_CLK_UNPREPARE POST_CLK_UNPREPARE Signed-off-by: Bill Huang bilhu...@nvidia.com I'm still not sure about this approach. Based on feedback I got from Linaro Connect I am not convinced that scaling voltage through clk rate-change notifiers is the right way to go. As I understand it this patch only exists for that single purpose, so if the voltage-notifier idea gets dropped then I will not take this patch in. Thanks Mike, actually we won't use your clk: notifier handler for dynamic voltage scaling patch instead we are trying to port our DVFS into Non-CPU DVFS framework devfreq which will need to hook those notifiers, without the clock notifiers been extended the framework is useless for us since we cannot do polling due to the fact that polling is not in real time. If it ended up extending the notifiers cannot happen then the only choice for us I think would be giving up devfreq and implement them in Tegra's clk_hw. I'm familiar with the devfreq framework. Can you explain further how you plan to use devfreq with the clock notifiers? What does the call graph look like? The call graph will look like this, when any DVFS interested clock rate changes (including enable and disable) happen - Tegra devfreq clock notifier is called - call into update_devfreq if needed - in update_devfreq it will call into .get_target_freq in Tegra devfreq_governor - and then call into .target of tegra devfreq_dev_profile to set voltage and done. More details are as below. We'll create devfreq driver for Tegra VDD_CORE rail, and the safe voltage level of the rail is determined by tens of clocks (2d, 3d, mpe,...), all the frequency ladders of those clocks are defined in DT also the operating points for VDD_CORE is declared in DT where its frequency will be more of a virtual clock or index. operating-points = /* virtual-kHz uV */ 0 95 1 100 2 105 3 110 4 115 5 120 6 125 7 130 8 135 Register a Tegra governor where the callback .get_target_freq is the function to determine the overall frequency it can go to, and the .target callback in devfreq_dev_profile will be the function really do the voltage scaling. Tegra devfreq driver will register clock notifiers on all its interested clock and hence when any of those clock rate changes, disabled, enabled, we'll specifically call update_devfreq in the notifier. Thank you for the explanation. Do you plan to use actual devfreq governors (like simple-ondemand, or something custom) for changing OPPs, or do you just plan to use the clock framework as a trigger for DVFS? Regards, Mike At a recent discussion regarding a previous version of this patch [RFC 1/1] clk: Add notifier support in clk_prepare_enable/clk_disable_unprepare, we also discussed whether to use clk notifiers or to use a clk hw to implement DVFS. Stephen Warren an myself, kind of pointed out that there could be benefits of not using notifers. I would just like to add that to this discussion as well. The idea in principle, could be as an option to Bill's idea, using devfreq with notifiers, to implement a clk hw which possibly makes use of the opp libary and do implements the DVFS functionallity that is needed for each SoC. Kind regards Ulf Hansson ___ linaro-dev mailing list linaro-...@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4] clk: allow reentrant calls into the clk framework
On 27 March 2013 10:55, Viresh Kumar viresh.ku...@linaro.org wrote: On 27 March 2013 15:10, Thomas Gleixner t...@linutronix.de wrote: On Wed, 27 Mar 2013, Mike Turquette wrote: Reentrancy into the clock framework from the clk.h api is necessary for clocks that are prepared and unprepared via i2c_transfer (which includes many PMICs and discrete audio chips) as well as for several other use cases. That explanation sucks. Why does an i2c clock need reentrancy? Just because it's i2c or what? I am noway connected to this development but was just going through your mail and i think i might know the answer why is this required. Consider an example where an external chip has clock controller and has bits which can be programmed to enable/disable clock. And this chip is connected via spi/i2c to SoC. clk_prepare(peripheral on external chip) - i2c_xfer(to write to external chips register) - clk_enable(i2c controller) -controller-xfer-routine.. and finally we enable clk here... Sorry if i am on the wrong side :) I agree with you Viresh. I guess Mike should update the commit message. I would also like add another reason to why this is needed. For some clks you would like to do pinctrl operations from a clk hw. But since a pinctrl driver likely requires a clk to be prepared|enabled, we run into a clk reentrant issue. Kind regards Ulf Hansson -- viresh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] wait while adding MMC host to ensure root mounts
On 22 March 2013 18:03, Chris Ball c...@laptop.org wrote: Hi Sergey, On Wed, Mar 13 2013, Sergey Yanovich wrote: MMC hosts are added asynchronously. We need to wait until detect returns to avoid failed root filesystem mounts. ---8--- VFS: Cannot open root device mmcblk0p1 or unknown-block(0,0): error -6 Please append a correct root= boot option; here are the available partitions: mmc0: host does not support reading read-only switch. assuming write-enable. 1f00 256 mtdblock0 (driver?) 1f01 256 mtdblock1 (driver?) 1f022560 mtdblock2 mmc0: new SDHC card at address b368 (driver?) 1f03 29696 mtdblock3 (driver?) 1f04 16384 mtdblock4 mmcblk0: mmc0:b368 USD 3.72 GiB (driver?) mmcblk0: p1 b300 3910656 mmcblk0 driver: mmcblk b301 3906560 mmcblk0p1 -01 Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(0,0) ---8--- Signed-off-by: Sergey Yanovich ynv...@gmail.com --- changes for v2: - removed exporting as symbol is in the same file drivers/mmc/core/core.c |1 + 1 file changed, 1 insertion(+) diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index aaed768..7196888 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -2225,6 +2225,7 @@ void mmc_start_host(struct mmc_host *host) host-rescan_disable = 0; mmc_power_up(host); mmc_detect_change(host, 0); + mmc_flush_scheduled_work(); } void mmc_stop_host(struct mmc_host *host) Thanks, this looks okay to me, I've pushed it to mmc-next for 3.10. - Chris. -- Chris Ball c...@laptop.org http://printf.net/ One Laptop Per Child Hi Chris, I noticed you merged this. I thought the idea was to use the rootwait or rootdelay? Moreover, this patch will have bad impact on booting the kernel, since every host device that has scheduled a detect work from it's probe function will also wait for it to finish. Even if it is the boot device of not. If this is needed, I would prefer that a host cap is used. Kind regards Ulf Hansson -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/5] clk: notifier handler for dynamic voltage scaling
. I like the direction of were this idea is going. In principle that will mean that you actually can do DVFS handling from clk_prepare|unprepare as well, which I think is a wanted feature as well. Moreover, drivers do in general not need to bother about DVFS which in the end should be a good, right? For reference, from a ux500 SoC perspective, we have internal code - not upstreamed yet, which implements a specific OPP clock type. No changes has been done to the common clk framework for this. The OPP clock clk hw, utilizes the OPP library to find out what opp to chose for a certain frequency and then request a change if needed. To make this solution really fly we do require the clk API to be re-entrant since that is needed to be able to update the OPP. Moreover, it would be possible to make use of the clk_prepare|unprepare callbacks for this clk hw to also handle OPP changes. None of the above solutions are related to your point about scaling voltage from clk_set_rate. Voltage may still be scaled from the clock rate-change notifier despite the method chose to scale groups of clocks. Regards, Mike Thanks Richard ___ linaro-dev mailing list linaro-...@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev Kind regards Ulf Hansson -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3] mmc: core: Add support for idle time BKOPS
mmc_wait_for_app_cmd(struct mmc_host *, struct mmc_card *, struct mmc_command *, int); extern void mmc_start_bkops(struct mmc_card *card, bool from_exception); +extern void mmc_start_delayed_bkops(struct mmc_card *card); +extern void mmc_start_idle_time_bkops(struct work_struct *work); +extern void mmc_bkops_completion_polling(struct work_struct *work); extern int __mmc_switch(struct mmc_card *, u8, u8, u8, unsigned int, bool); extern int mmc_switch(struct mmc_card *, u8, u8, u8, unsigned int); -- QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ Finally some overall thoughts. What I would like to understand is how we decide that the card has become idle. I belive two values should be considered, but are they? 1. The card need BKOPS to be performed for some status level. 2. Request inactivity for a certain timeout has occured. Have you considered to use runtime PM for the card device instead of the workqueue? Kind regards Ulf Hansson -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/5] clk: allow reentrant calls into the clk framework
= clk_calc_new_rates(clk, rate);
+ if (!top)
+ return -EINVAL;
+
+ /* notify that we are about to change rates */
+ fail_clk = clk_propagate_rate_change(top, PRE_RATE_CHANGE);
+ if (fail_clk) {
+ pr_warn(%s: failed to set %s rate\n, __func__,
+ fail_clk-name);
+ clk_propagate_rate_change(top, ABORT_RATE_CHANGE);
+ return -EBUSY;
+ }
+
+ /* change the rates */
+ clk_change_rate(top);
+
+ return ret;
+}
+
/**
* clk_set_rate - specify a new rate for clk
* @clk: the clk whose rate is being changed
@@ -1059,44 +1191,18 @@ static void clk_change_rate(struct clk *clk)
*/
int clk_set_rate(struct clk *clk, unsigned long rate)
{
- struct clk *top, *fail_clk;
int ret = 0;
- /* prevent racing with updates to the clock topology */
- mutex_lock(prepare_lock);
-
- /* bail early if nothing to do */
- if (rate == clk-rate)
- goto out;
-
- if ((clk-flags CLK_SET_RATE_GATE) clk-prepare_count) {
- ret = -EBUSY;
- goto out;
- }
-
- /* calculate new rates and get the topmost changed clock */
- top = clk_calc_new_rates(clk, rate);
- if (!top) {
- ret = -EINVAL;
- goto out;
- }
-
- /* notify that we are about to change rates */
- fail_clk = clk_propagate_rate_change(top, PRE_RATE_CHANGE);
- if (fail_clk) {
- pr_warn(%s: failed to set %s rate\n, __func__,
- fail_clk-name);
- clk_propagate_rate_change(top, ABORT_RATE_CHANGE);
- ret = -EBUSY;
+ if (clk_is_reentrant()) {
+ ret = __clk_set_rate(clk, rate);
goto out;
}
- /* change the rates */
- clk_change_rate(top);
+ clk_fwk_lock();
+ ret = __clk_set_rate(clk, rate);
+ clk_fwk_unlock();
out:
- mutex_unlock(prepare_lock);
-
return ret;
}
EXPORT_SYMBOL_GPL(clk_set_rate);
@@ -,10 +1217,16 @@ struct clk *clk_get_parent(struct clk *clk)
{
struct clk *parent;
- mutex_lock(prepare_lock);
+ if (clk_is_reentrant()) {
+ parent = __clk_get_parent(clk);
+ goto out;
+ }
+
+ clk_fwk_lock();
parent = __clk_get_parent(clk);
- mutex_unlock(prepare_lock);
+ clk_fwk_unlock();
+out:
return parent;
}
EXPORT_SYMBOL_GPL(clk_get_parent);
@@ -1293,6 +1405,7 @@ out:
int clk_set_parent(struct clk *clk, struct clk *parent)
{
int ret = 0;
+ bool reenter;
if (!clk || !clk-ops)
return -EINVAL;
@@ -1300,8 +1413,10 @@ int clk_set_parent(struct clk *clk, struct clk *parent)
if (!clk-ops-set_parent)
return -ENOSYS;
- /* prevent racing with updates to the clock topology */
- mutex_lock(prepare_lock);
+ reenter = clk_is_reentrant();
+
+ if (!reenter)
+ clk_fwk_lock();
if (clk-parent == parent)
goto out;
@@ -1330,7 +1445,8 @@ int clk_set_parent(struct clk *clk, struct clk *parent)
__clk_reparent(clk, parent);
out:
- mutex_unlock(prepare_lock);
+ if (!reenter)
+ clk_fwk_unlock();
return ret;
}
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Kind regards
Ulf Hansson
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v1] mmc: core: move the cache disabling operation to mmc_suspend
On 20 January 2013 12:29, Maya Erez me...@codeaurora.org wrote:
Cache control is an eMMC feature and in therefore should be
part of MMC's bus resume operations, performed in mmc_suspend,
rather than in the generic mmc_suspend_host().
Signed-off-by: Maya Erez me...@codeaurora.org
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index aaed768..b438bb2 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2388,6 +2388,7 @@ EXPORT_SYMBOL(mmc_flush_cache);
* Turn the cache ON/OFF.
* Turning the cache OFF shall trigger flushing of the data
* to the non-volatile storage.
+ * This function should be called with host claimed
*/
int mmc_cache_ctrl(struct mmc_host *host, u8 enable)
{
@@ -2399,7 +2400,6 @@ int mmc_cache_ctrl(struct mmc_host *host, u8 enable)
mmc_card_is_removable(host))
return err;
- mmc_claim_host(host);
if (card mmc_card_mmc(card)
(card-ext_csd.cache_size 0)) {
enable = !!enable;
@@ -2417,7 +2417,6 @@ int mmc_cache_ctrl(struct mmc_host *host, u8 enable)
card-ext_csd.cache_ctrl = enable;
}
}
- mmc_release_host(host);
return err;
}
@@ -2436,10 +2435,6 @@ int mmc_suspend_host(struct mmc_host *host)
cancel_delayed_work(host-detect);
mmc_flush_scheduled_work();
- err = mmc_cache_ctrl(host, 0);
- if (err)
- goto out;
-
mmc_bus_get(host);
if (host-bus_ops !host-bus_dead) {
if (host-bus_ops-suspend) {
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index e6e3911..dc17d40 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1379,6 +1379,11 @@ static int mmc_suspend(struct mmc_host *host)
BUG_ON(!host-card);
mmc_claim_host(host);
+
+ err = mmc_cache_ctrl(host, 0);
+ if (err)
+ goto out;
+
if (mmc_can_poweroff_notify(host-card))
err = mmc_poweroff_notify(host-card,
EXT_CSD_POWER_OFF_SHORT);
else if (mmc_card_can_sleep(host))
@@ -1386,8 +1391,9 @@ static int mmc_suspend(struct mmc_host *host)
else if (!mmc_host_is_spi(host))
err = mmc_deselect_cards(host);
host-card-state = ~(MMC_STATE_HIGHSPEED | MMC_STATE_HIGHSPEED_200);
- mmc_release_host(host);
+out:
+ mmc_release_host(host);
return err;
}
--
1.7.3.3
--
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Acked-by: Ulf Hansson ulf.hans...@linaro.org
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/8] mmc: mmci: Move ios_handler functionality into the driver
On 21 January 2013 15:06, Lee Jones lee.jo...@linaro.org wrote:
On Thu, 13 Dec 2012, Lee Jones wrote:
There are currently two instances of the ios_handler being used.
Both of which mearly toy with some regulator settings. Now there
is a GPIO regulator API, we can use that instead, and lessen the
per platform burden. By doing this, we also become more Device
Tree compatible.
Cc: Chris Ball c...@laptop.org
Cc: Russell King rmk+ker...@arm.linux.org.uk
Acked-by: Ulf Hansson ulf.hans...@stericsson.com
Signed-off-by: Lee Jones lee.jo...@linaro.org
Any more news on this one?
Not exactly this version of the patch, but another one has has been
merged through Russell's patch tracker. Available for 3.9.
drivers/mmc/host/mmci.c | 22 ++
drivers/mmc/host/mmci.h |1 +
2 files changed, 23 insertions(+)
diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index edc3e9b..d45c931 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -1091,6 +1091,16 @@ static void mmci_set_ios(struct mmc_host *mmc, struct
mmc_ios *ios)
case MMC_POWER_OFF:
if (host-vcc)
ret = mmc_regulator_set_ocr(mmc, host-vcc, 0);
+
+ if (host-vqmmc) {
+ if (regulator_is_enabled(host-vqmmc)) {
+ ret = regulator_disable(host-vqmmc);
+ if (ret 0)
+ dev_warn(mmc_dev(mmc),
+ unable to disable
vmmc-ios\n);
+ }
+ }
+
break;
case MMC_POWER_UP:
if (host-vcc) {
@@ -1115,6 +1125,14 @@ static void mmci_set_ios(struct mmc_host *mmc, struct
mmc_ios *ios)
break;
case MMC_POWER_ON:
+ if (host-vqmmc)
+ if (!regulator_is_enabled(host-vqmmc)) {
+ ret = regulator_enable(host-vqmmc);
+ if (ret 0)
+ dev_warn(mmc_dev(mmc),
+ unable to enable
vmmc-ios\n);
+ }
+
pwr |= MCI_PWR_ON;
break;
}
@@ -1379,6 +1397,10 @@ static int __devinit mmci_probe(struct amba_device
*dev,
(using regulator instead)\n);
}
}
+
+ host-vqmmc = regulator_get(dev-dev, vqmmc);
+ if (IS_ERR(host-vqmmc))
+ host-vqmmc = NULL;
#endif
/* Fall back to platform data if no regulator is found */
if (host-vcc == NULL)
diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index d437ccf..b87d9e2 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -194,6 +194,7 @@ struct mmci_host {
struct sg_mapping_iter sg_miter;
unsigned intsize;
struct regulator*vcc;
+ struct regulator*vqmmc;
#ifdef CONFIG_DMA_ENGINE
/* DMA stuff */
--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 06/15] clk: export __clk_get_name
On 21 January 2013 18:15, Arnd Bergmann a...@arndb.de wrote:
The __clk_get_name is used by drivers that provide their
own clocks, which have traditionally all been built-in.
The new imx drm driver however provides clocks but can
be built as a module, so we need to export __clk_get_name.
While this is only a staging driver, it's likely that
there will be others like it in the future.
Without this patch, building ARM allmodconfig results in:
ERROR: __clk_get_name [drivers/staging/imx-drm/ipu-v3/imx-ipu-v3.ko]
undefined!
Signed-off-by: Arnd Bergmann a...@arndb.de
Cc: Sascha Hauer s.ha...@pengutronix.de
Cc: Mike Turquette mturque...@linaro.org
---
drivers/clk/clk.c |1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 251e45d..13b793e 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -263,6 +263,7 @@ inline const char *__clk_get_name(struct clk *clk)
{
return !clk ? NULL : clk-name;
}
+EXPORT_SYMBOL_GPL(__clk_get_name);
inline struct clk_hw *__clk_get_hw(struct clk *clk)
{
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Acked-by: Ulf Hansson ulf.hans...@linaro.org
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mmc: core: disable the cache before suspend only after stopping BKOPS
On 13 January 2013 22:47, Maya Erez me...@codeaurora.org wrote:
-Original Message-
From: Subhash Jadavani [mailto:subha...@codeaurora.org]
Sent: Saturday, January 12, 2013 9:07 AM
To: Maya Erez
Cc: linux-...@vger.kernel.org; linux-arm-...@vger.kernel.org; open list
Subject: Re: [PATCH] mmc: core: disable the cache before suspend only after
stopping BKOPS
On 1/12/2013 2:12 AM, Maya Erez wrote:
mmc_cache_ctrl was called in runtime suspend before MMC interrupted
BKOPS in case it is still running on the card. This caused the cache
disable to timeout.
I guess even if the idle time bkops polling is not implemented, this patch
is good to have. cache control is the eMMC feature and in that sense, it
should have been part of MMC's bus resume (mmc_resume) rather than generic
mmc_suspend_host().
Patch as such is fine and if you agree, you may want to remove the
mentioning of bkops as part of commit text and may just want to mention
above reason as the main motivation for this patch.
Agreed, I will change the commit text in the next uploaded version.
Therefore, mmc_cache_ctrl has to move to mmc_suspend where we are sure
that the card can go into suspend and there is no pending activity.
Signed-off-by: Maya Erez me...@codeaurora.org
---
drivers/mmc/core/core.c |7 +--
drivers/mmc/core/mmc.c |8 +++-
2 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index
aaed768..b438bb2 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2388,6 +2388,7 @@ EXPORT_SYMBOL(mmc_flush_cache);
* Turn the cache ON/OFF.
* Turning the cache OFF shall trigger flushing of the data
* to the non-volatile storage.
+ * This function should be called with host claimed
*/
int mmc_cache_ctrl(struct mmc_host *host, u8 enable)
{
@@ -2399,7 +2400,6 @@ int mmc_cache_ctrl(struct mmc_host *host, u8 enable)
mmc_card_is_removable(host))
return err;
- mmc_claim_host(host);
if (card mmc_card_mmc(card)
(card-ext_csd.cache_size 0)) {
enable = !!enable;
@@ -2417,7 +2417,6 @@ int mmc_cache_ctrl(struct mmc_host *host, u8 enable)
card-ext_csd.cache_ctrl = enable;
}
}
- mmc_release_host(host);
return err;
}
@@ -2436,10 +2435,6 @@ int mmc_suspend_host(struct mmc_host *host)
cancel_delayed_work(host-detect);
mmc_flush_scheduled_work();
- err = mmc_cache_ctrl(host, 0);
- if (err)
- goto out;
-
mmc_bus_get(host);
if (host-bus_ops !host-bus_dead) {
if (host-bus_ops-suspend) {
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index
e6e3911..dc17d40 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1379,6 +1379,11 @@ static int mmc_suspend(struct mmc_host *host)
BUG_ON(!host-card);
mmc_claim_host(host);
+
+ err = mmc_cache_ctrl(host, 0);
+ if (err)
+ goto out;
+
if (mmc_can_poweroff_notify(host-card))
err = mmc_poweroff_notify(host-card,
EXT_CSD_POWER_OFF_SHORT);
else if (mmc_card_can_sleep(host))
@@ -1386,8 +1391,9 @@ static int mmc_suspend(struct mmc_host *host)
else if (!mmc_host_is_spi(host))
err = mmc_deselect_cards(host);
host-card-state = ~(MMC_STATE_HIGHSPEED |
MMC_STATE_HIGHSPEED_200);
- mmc_release_host(host);
+out:
+ mmc_release_host(host);
return err;
}
--
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
I agree that some update to the commit msg is wanted; since this fixup
is not related to runtime suspend as such.
Anyway, you have my ack!
Acked-by: Ulf Hansson ulf.hans...@linaro.org
Kind regards
Ulf Hansson
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3 RESEND] mmc: sdhci: check voltage range only on regulators aware of voltage value
On 14 February 2013 09:05, Marek Szyprowski m.szyprow...@samsung.com wrote: On 2/13/2013 12:35 PM, Mark Brown wrote: On Wed, Feb 13, 2013 at 08:45:56AM +0100, Guennadi Liakhovetski wrote: On Wed, 13 Feb 2013, Marek Szyprowski wrote: BTW, mmc_regulator_get_ocrmask() won't work with continuous range regulators. This seems like a problem, that has to be fixed... Indeed, what's the issue? There are probably 2 issues: 1. mmc_regulator_get_ocrmask() works only with regulators which support regulator_count_voltages() and regulator_list_voltage(). Recently support for continuous regulators have been merged. Such regulators doesn't provide regulator_list_voltage() method, but are able to change/set voltage to the given value. I agree that they are not very common, so right now we can probably ignore them until the first board, which uses them arrives. 2. The second issue might be related to the testing of precise voltage values in the ocr mask, not the whole allowed ranges. Such issues in sdhci.c driver has been recently fixed by commit cec2e216f72c6b5ccdadb60aadbe99821d744503 (mmc: sdhci: Use regulator min/max voltage range according to spec), but I don't know MMC core code to judge if ocr mask is used for exact voltage checking or only for checking the voltage ranges. However someone with good mmc subsystem knowledge should check it. Not 100% sure what your problem relates too here, but I am aware of an issue for how the mmc protocol layer are handling ocr_masks. Let me try to describe it here: 1. During card init mmc_power_up will be called for telling the host driver to provide power to the card. The level of voltage will be set to ocr_avail which means the highest supported voltage by the host. 2. At the protocol layer the card init sequence tries to negotiate to lowest possible ocr value from what the card and the host together supports. Once done, the ocr mask value will be cached into the host struct. 3. The host will informed about the new ocr mask from the protocol layer with mmc_select_voltage and it somewhere here the problems starts. No host are actually changing the voltage level at this state (MMC_POWER_ON) which is correct since it would likely mean violation of the spec. At the same time the protocol layer still believes the host has switched to operate at the new voltage level. 4. So the host and the protocol layer are out of sync with regards to the ocr mask, which is why the cached ocr_mask in the host struct is reset when doing mmc_power_off. Otherwise the suspend/resume sequence would have been broken. I have been looking into a solution for the above problem, but has not yet been able to finalize the task. Hope this did not become more fussy now. :-) Best regards -- Marek Szyprowski Samsung Poland RD Center -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Kind regards Ulf Hansson -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 01/35] mfd: ab8500-gpadc: Implemented suspend/resume
On 20 February 2013 14:19, Mark Brown
broo...@opensource.wolfsonmicro.com wrote:
On Fri, Feb 15, 2013 at 12:56:32PM +, Lee Jones wrote:
+static int ab8500_gpadc_suspend(struct device *dev)
+{
+ struct ab8500_gpadc *gpadc = dev_get_drvdata(dev);
+
+ mutex_lock(gpadc-ab8500_gpadc_lock);
+
+ pm_runtime_get_sync(dev);
+
+ regulator_disable(gpadc-regu);
+ return 0;
+}
This doesn't look especially sane... You're doing a runtime get, taking
the lock without releasing it and disabling the regulator. This is
*very* odd, both the changelog and the code need to explain what's going
on and why it's safe in a lot more detail here.
You need to do pm_runtime_get_sync to be able to make sure resources
(which seems to be only the regulator) are safe to switch off. To my
understanding this is a generic way to use for being able to switch
off resources at a device suspend when runtime pm is used in
conjunction.
Regarding the mutex, I can't tell the reason behind it. It seems
strange but not sure.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Kind regards
Ulf Hansson
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 01/35] mfd: ab8500-gpadc: Implemented suspend/resume
On 22 February 2013 11:38, Mark Brown broo...@opensource.wolfsonmicro.com wrote: On Thu, Feb 21, 2013 at 11:45:08PM +0100, Ulf Hansson wrote: On 20 February 2013 14:19, Mark Brown This doesn't look especially sane... You're doing a runtime get, taking the lock without releasing it and disabling the regulator. This is *very* odd, both the changelog and the code need to explain what's going on and why it's safe in a lot more detail here. You need to do pm_runtime_get_sync to be able to make sure resources (which seems to be only the regulator) are safe to switch off. To my understanding this is a generic way to use for being able to switch off resources at a device suspend when runtime pm is used in conjunction. Are you sure this actually does what you think it does, especially when run on modern kernels? Not sure, what you are thinking of more precisely here. Runtime pm has been in the kernel for quite some time now. Anyway, to make it a bit clearer, we switch the regulator on/off at the runtime suspend/resume callbacks. We want to take similar actions in device suspend/resume. To accomplish this a pm_runtime_get_sync is done in suspend and vice verse in resume, otherwise you can not safely handle the regulator. Kind regards Ulf Hansson -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 01/35] mfd: ab8500-gpadc: Implemented suspend/resume
On 25 February 2013 14:51, Mark Brown broo...@opensource.wolfsonmicro.com wrote: On Mon, Feb 25, 2013 at 10:27:36AM +0100, Ulf Hansson wrote: On 22 February 2013 11:38, Mark Brown Are you sure this actually does what you think it does, especially when run on modern kernels? Not sure, what you are thinking of more precisely here. Runtime pm has been in the kernel for quite some time now. Yes, thanks - I was aware of that. The integration between runtime and system PM has been an area that's had some development though. Anyway, to make it a bit clearer, we switch the regulator on/off at the runtime suspend/resume callbacks. We want to take similar actions in device suspend/resume. To accomplish this a pm_runtime_get_sync is done in suspend and vice verse in resume, otherwise you can not safely handle the regulator. Are you absolutely positive that with modern kernels your get actually resumes the device? Yes, runtime resume is always ok, But you can not runtime suspend the device, since the device suspend layer prevent this with a pm_runtime_get_noresume before calling the device suspend callback. Kind regards Ulf Hansson -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] clk: Provide option for clk_get_rate to issue hw for new rate
Hi Mike,
Thanks for your input, and sorry for my late reply!
On 31 August 2012 21:29, Mike Turquette mturque...@ti.com wrote:
Quoting Ulf Hansson (2012-08-31 05:21:28)
From: Ulf Hansson ulf.hans...@linaro.org
By using CLK_GET_RATE_NOCACHE flag, we tell the clk_get_rate API to
issue the hw for an updated clock rate. This can be used for a clock
which rate may be updated without a client necessary modifying it.
I'm glad to see this. We discussed whether the default behavior should
be cached or from the hardware at length some time back, so having a
flag to support the non-default is great.
Signed-off-by: Ulf Hansson ulf.hans...@linaro.org
---
drivers/clk/clk.c| 43
+++---
include/linux/clk-provider.h |1 +
2 files changed, 25 insertions(+), 19 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index efdfd00..d9cbae0 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -558,25 +558,6 @@ int clk_enable(struct clk *clk)
EXPORT_SYMBOL_GPL(clk_enable);
/**
- * clk_get_rate - return the rate of clk
- * @clk: the clk whose rate is being returned
- *
- * Simply returns the cached rate of the clk. Does not query the hardware.
If
- * clk is NULL then returns 0.
- */
-unsigned long clk_get_rate(struct clk *clk)
-{
- unsigned long rate;
-
- mutex_lock(prepare_lock);
- rate = __clk_get_rate(clk);
- mutex_unlock(prepare_lock);
-
- return rate;
-}
-EXPORT_SYMBOL_GPL(clk_get_rate);
-
-/**
* __clk_round_rate - round the given rate for a clk
* @clk: round the rate of this clock
*
@@ -702,6 +683,30 @@ static void __clk_recalc_rates(struct clk *clk,
unsigned long msg)
}
/**
+ * clk_get_rate - return the rate of clk
+ * @clk: the clk whose rate is being returned
+ *
+ * Simply returns the cached rate of the clk, unless CLK_GET_RATE_NOCACHE
flag
+ * is set, which means a recalc_rate will be issued.
+ * If clk is NULL then returns 0.
+ */
+unsigned long clk_get_rate(struct clk *clk)
+{
+ unsigned long rate;
+
+ mutex_lock(prepare_lock);
+
+ if (clk (clk-flags CLK_GET_RATE_NOCACHE))
+ __clk_recalc_rates(clk, 0);
This is a bit subtle. Calling __clk_recalc_rates will walk the subtree
of children recalculating rates as well as firing off notifiers. Is
this what you want? If your clock changes rates behind your back AND
has chilren then this is probably the right thing to do. However you
might be better off with:
if (clk (clk-flags CLK_GET_RATE_NOCACHE))
rate = clk-ops-recalc_rate(clk-hw, clk-parent-rate);
This doesn't update children or fire off notifiers. What is best for
your platform?
For my platform, ux500 and for the clock connected to this
patchseries, your suggesting above is enough. (Well some additional
error handling is needed in your code proposal though :-) )
The reason for why I used __clk_recalc_rates was because I think it
could make sense to have a more generic approach, not sure if it is
needed as you mention. Additionally, using __clk_recalc_rates with
0 as the notification argument, should prevent notifications from
happen, right?
So basically, I wanted the clock rates for the children to be updated
as well as the parent clock rate, but no notifications.
I can happily update the patch according to your proposal if you still
think it is the best way to do it, just tell me again then. :-)
Kind regards
Ulf Hansson
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] clk: Provide option for clk_get_rate to issue hw for new rate
On 7 September 2012 10:21, Linus Walleij linus.wall...@linaro.org wrote: On Fri, Sep 7, 2012 at 3:00 AM, Turquette, Mike mturque...@ti.com wrote: On Thu, Sep 6, 2012 at 5:19 PM, Mike Turquette mturque...@ti.com wrote: I'll take this series into clk-next. Oops, I forgot to ask about patch #3. Which tree do you want that to go through? Just take it all through your tree if there are no major conflicts. Acked-by etc. Agree, it makes it simpler to go through Mike's clock tree. At least let's try it out. I will likely have similar patch series with cross dependencies later on, so this can be a good first test and hopefully it works. Although, don't we need an ack by Samuel Ortiz for the patch on mfd? mfd: dbx500: Provide a more accurate smp_twd clock Or, is it enough with Linus ack since he is the maintainer of that specific mfd file I have patched? Kind regards Ulf Hansson -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 0/4] clk: Support for smp_twd clock for ux500
From: Ulf Hansson ulf.hans...@linaro.org To implement support for the smp_twd clock for ux500 several steps was needed. This patchseries has also proposed some new changes in the common clock core, which the ux500 smp_twd clock definition are relying on. Moreover some crossdepency exist to the ux500 prcmu mfd driver. My idea is that the corresponding maintainers may be able to ack these changes, if they like them of course, so we can merge this through Mike Turquette's clk git tree. Otherwise I can happily set up some other way forward, if that suits better. Patches is based upon Linux 3.6 rc2. The clock patches specific for ux500, is based upon the series [PATCH V2 0/4] clk: Convert ARM ux500 to common clock. Michel Jaouen (1): mfd: dbx500: Provide a more accurate smp_twd clock Ulf Hansson (3): clk: Provide option for clk_get_rate to issue hw for new rate clk: ux500: Support for prmcu_rate clock clk: ux500: Define smp_twd clock for u8500 drivers/clk/clk.c| 43 +- drivers/clk/ux500/clk-prcmu.c| 14 + drivers/clk/ux500/clk.h |5 + drivers/clk/ux500/u8500_clk.c| 12 +++ drivers/mfd/db8500-prcmu.c | 42 + drivers/mfd/dbx500-prcmu-regs.h |4 +++- include/linux/clk-provider.h |1 + include/linux/mfd/dbx500-prcmu.h |1 + 8 files changed, 98 insertions(+), 24 deletions(-) -- 1.7.10 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/4] clk: Provide option for clk_get_rate to issue hw for new rate
From: Ulf Hansson ulf.hans...@linaro.org
By using CLK_GET_RATE_NOCACHE flag, we tell the clk_get_rate API to
issue the hw for an updated clock rate. This can be used for a clock
which rate may be updated without a client necessary modifying it.
Signed-off-by: Ulf Hansson ulf.hans...@linaro.org
---
drivers/clk/clk.c| 43 +++---
include/linux/clk-provider.h |1 +
2 files changed, 25 insertions(+), 19 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index efdfd00..d9cbae0 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -558,25 +558,6 @@ int clk_enable(struct clk *clk)
EXPORT_SYMBOL_GPL(clk_enable);
/**
- * clk_get_rate - return the rate of clk
- * @clk: the clk whose rate is being returned
- *
- * Simply returns the cached rate of the clk. Does not query the hardware. If
- * clk is NULL then returns 0.
- */
-unsigned long clk_get_rate(struct clk *clk)
-{
- unsigned long rate;
-
- mutex_lock(prepare_lock);
- rate = __clk_get_rate(clk);
- mutex_unlock(prepare_lock);
-
- return rate;
-}
-EXPORT_SYMBOL_GPL(clk_get_rate);
-
-/**
* __clk_round_rate - round the given rate for a clk
* @clk: round the rate of this clock
*
@@ -702,6 +683,30 @@ static void __clk_recalc_rates(struct clk *clk, unsigned
long msg)
}
/**
+ * clk_get_rate - return the rate of clk
+ * @clk: the clk whose rate is being returned
+ *
+ * Simply returns the cached rate of the clk, unless CLK_GET_RATE_NOCACHE flag
+ * is set, which means a recalc_rate will be issued.
+ * If clk is NULL then returns 0.
+ */
+unsigned long clk_get_rate(struct clk *clk)
+{
+ unsigned long rate;
+
+ mutex_lock(prepare_lock);
+
+ if (clk (clk-flags CLK_GET_RATE_NOCACHE))
+ __clk_recalc_rates(clk, 0);
+
+ rate = __clk_get_rate(clk);
+ mutex_unlock(prepare_lock);
+
+ return rate;
+}
+EXPORT_SYMBOL_GPL(clk_get_rate);
+
+/**
* __clk_speculate_rates
* @clk: first clk in the subtree
* @parent_rate: the future rate of clk's parent
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 77335fa..1b15307 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -26,6 +26,7 @@
#define CLK_IGNORE_UNUSED BIT(3) /* do not gate even if unused */
#define CLK_IS_ROOTBIT(4) /* root clk, has no parent */
#define CLK_IS_BASIC BIT(5) /* Basic clk, can't do a to_clk_foo() */
+#define CLK_GET_RATE_NOCACHE BIT(6) /* do not use the cached clk rate */
struct clk_hw;
--
1.7.10
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 4/4] clk: ux500: Define smp_twd clock for u8500
From: Ulf Hansson ulf.hans...@linaro.org The smp_twd clock is based upon a prcmu_rate clock type for the PRCMU_ARMSS clock. Signed-off-by: Ulf Hansson ulf.hans...@linaro.org --- drivers/clk/ux500/u8500_clk.c | 12 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/drivers/clk/ux500/u8500_clk.c b/drivers/clk/ux500/u8500_clk.c index 5c1fca1..ca4a25e 100644 --- a/drivers/clk/ux500/u8500_clk.c +++ b/drivers/clk/ux500/u8500_clk.c @@ -205,12 +205,16 @@ void u8500_clk_init(void) clk_register_clkdev(clk, dsilp2, dsilink.2); clk_register_clkdev(clk, dsilp2, mcde); + clk = clk_reg_prcmu_rate(smp_twd, NULL, PRCMU_ARMSS, + CLK_IS_ROOT|CLK_GET_RATE_NOCACHE| + CLK_IGNORE_UNUSED); + clk_register_clkdev(clk, NULL, smp_twd); + /* * FIXME: Add special handled PRCMU clocks here: -* 1. smp_twd, use PRCMU_ARMSS. -* 2. clk_arm, use PRCMU_ARMCLK. -* 3. clkout0yuv, use PRCMU as parent + need regulator + pinctrl. -* 4. ab9540_clkout1yuv, see clkout0yuv +* 1. clk_arm, use PRCMU_ARMCLK. +* 2. clkout0yuv, use PRCMU as parent + need regulator + pinctrl. +* 3. ab9540_clkout1yuv, see clkout0yuv */ /* PRCC P-clocks */ -- 1.7.10 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 3/4] mfd: dbx500: Provide a more accurate smp_twd clock
From: Michel Jaouen michel.jao...@stericsson.com
The local timer clock is based on ARM subsystem clock. This patch
obtains a more exact value of that clock by reading PRCMU registers.
Using this increases the accuracy of the local timer events.
Signed-off-by: Ulf Hansson ulf.hans...@linaro.org
Signed-off-by: Rickard Andersson rickard.anders...@stericsson.com
Signed-off-by: Michel Jaouen michel.jao...@stericsson.com
---
drivers/mfd/db8500-prcmu.c | 42 ++
drivers/mfd/dbx500-prcmu-regs.h |4 +++-
include/linux/mfd/dbx500-prcmu.h |1 +
3 files changed, 46 insertions(+), 1 deletion(-)
diff --git a/drivers/mfd/db8500-prcmu.c b/drivers/mfd/db8500-prcmu.c
index 4f74529..e7f9539 100644
--- a/drivers/mfd/db8500-prcmu.c
+++ b/drivers/mfd/db8500-prcmu.c
@@ -418,6 +418,9 @@ static struct {
static atomic_t ac_wake_req_state = ATOMIC_INIT(0);
+/* Functions definition */
+static void compute_armss_rate(void);
+
/* Spinlocks */
static DEFINE_SPINLOCK(prcmu_lock);
static DEFINE_SPINLOCK(clkout_lock);
@@ -517,6 +520,7 @@ static struct dsiescclk dsiescclk[3] = {
}
};
+
/*
* Used by MCDE to setup all necessary PRCMU registers
*/
@@ -1013,6 +1017,7 @@ int db8500_prcmu_set_arm_opp(u8 opp)
(mb1_transfer.ack.arm_opp != opp))
r = -EIO;
+ compute_armss_rate();
mutex_unlock(mb1_transfer.lock);
return r;
@@ -1612,6 +1617,7 @@ static unsigned long pll_rate(void __iomem *reg, unsigned
long src_rate,
if ((branch == PLL_FIX) || ((branch == PLL_DIV)
(val PRCM_PLL_FREQ_DIV2EN)
((reg == PRCM_PLLSOC0_FREQ) ||
+(reg == PRCM_PLLARM_FREQ) ||
(reg == PRCM_PLLDDR_FREQ
div *= 2;
@@ -1661,6 +1667,39 @@ static unsigned long clock_rate(u8 clock)
else
return 0;
}
+static unsigned long latest_armss_rate;
+static unsigned long armss_rate(void)
+{
+ return latest_armss_rate;
+}
+
+static void compute_armss_rate(void)
+{
+ u32 r;
+ unsigned long rate;
+
+ r = readl(PRCM_ARM_CHGCLKREQ);
+
+ if (r PRCM_ARM_CHGCLKREQ_PRCM_ARM_CHGCLKREQ) {
+ /* External ARMCLKFIX clock */
+
+ rate = pll_rate(PRCM_PLLDDR_FREQ, ROOT_CLOCK_RATE, PLL_FIX);
+
+ /* Check PRCM_ARM_CHGCLKREQ divider */
+ if (!(r PRCM_ARM_CHGCLKREQ_PRCM_ARM_DIVSEL))
+ rate /= 2;
+
+ /* Check PRCM_ARMCLKFIX_MGT divider */
+ r = readl(PRCM_ARMCLKFIX_MGT);
+ r = PRCM_CLK_MGT_CLKPLLDIV_MASK;
+ rate /= r;
+
+ } else {/* ARM PLL */
+ rate = pll_rate(PRCM_PLLARM_FREQ, ROOT_CLOCK_RATE, PLL_DIV);
+ }
+
+ latest_armss_rate = rate;
+}
static unsigned long dsiclk_rate(u8 n)
{
@@ -1707,6 +1746,8 @@ unsigned long prcmu_clock_rate(u8 clock)
return pll_rate(PRCM_PLLSOC0_FREQ, ROOT_CLOCK_RATE, PLL_RAW);
else if (clock == PRCMU_PLLSOC1)
return pll_rate(PRCM_PLLSOC1_FREQ, ROOT_CLOCK_RATE, PLL_RAW);
+ else if (clock == PRCMU_ARMSS)
+ return armss_rate();
else if (clock == PRCMU_PLLDDR)
return pll_rate(PRCM_PLLDDR_FREQ, ROOT_CLOCK_RATE, PLL_RAW);
else if (clock == PRCMU_PLLDSI)
@@ -2693,6 +2734,7 @@ void __init db8500_prcmu_early_init(void)
handle_simple_irq);
set_irq_flags(irq, IRQF_VALID);
}
+ compute_armss_rate();
}
static void __init init_prcm_registers(void)
diff --git a/drivers/mfd/dbx500-prcmu-regs.h b/drivers/mfd/dbx500-prcmu-regs.h
index 23108a6..79c76eb 100644
--- a/drivers/mfd/dbx500-prcmu-regs.h
+++ b/drivers/mfd/dbx500-prcmu-regs.h
@@ -61,7 +61,8 @@
#define PRCM_PLLARM_LOCKP_PRCM_PLLARM_LOCKP3 0x2
#define PRCM_ARM_CHGCLKREQ (_PRCMU_BASE + 0x114)
-#define PRCM_ARM_CHGCLKREQ_PRCM_ARM_CHGCLKREQ 0x1
+#define PRCM_ARM_CHGCLKREQ_PRCM_ARM_CHGCLKREQ BIT(0)
+#define PRCM_ARM_CHGCLKREQ_PRCM_ARM_DIVSEL BIT(16)
#define PRCM_PLLARM_ENABLE (_PRCMU_BASE + 0x98)
#define PRCM_PLLARM_ENABLE_PRCM_PLLARM_ENABLE 0x1
@@ -140,6 +141,7 @@
/* PRCMU clock/PLL/reset registers */
#define PRCM_PLLSOC0_FREQ (_PRCMU_BASE + 0x080)
#define PRCM_PLLSOC1_FREQ (_PRCMU_BASE + 0x084)
+#define PRCM_PLLARM_FREQ (_PRCMU_BASE + 0x088)
#define PRCM_PLLDDR_FREQ (_PRCMU_BASE + 0x08C)
#define PRCM_PLL_FREQ_D_SHIFT 0
#define PRCM_PLL_FREQ_D_MASK BITS(0, 7)
diff --git a/include/linux/mfd/dbx500-prcmu.h b/include/linux/mfd/dbx500-prcmu.h
index 5b90e94..c410d99 100644
--- a/include/linux/mfd/dbx500-prcmu.h
+++ b/include/linux/mfd/dbx500-prcmu.h
@@ -136,6 +136,7 @@ enum prcmu_clock {
PRCMU_TIMCLK,
PRCMU_PLLSOC0,
PRCMU_PLLSOC1,
+ PRCMU_ARMSS,
PRCMU_PLLDDR,
PRCMU_PLLDSI,
PRCMU_DSI0CLK,
--
1.7.10
--
To unsubscribe from
[PATCH 2/4] clk: ux500: Support for prmcu_rate clock
From: Ulf Hansson ulf.hans...@linaro.org
The prmcu_rate clock is not gateable and has a rate which
only can be fetched.
Signed-off-by: Ulf Hansson ulf.hans...@linaro.org
---
drivers/clk/ux500/clk-prcmu.c | 14 ++
drivers/clk/ux500/clk.h |5 +
2 files changed, 19 insertions(+)
diff --git a/drivers/clk/ux500/clk-prcmu.c b/drivers/clk/ux500/clk-prcmu.c
index 1d779ad..930cdfe 100644
--- a/drivers/clk/ux500/clk-prcmu.c
+++ b/drivers/clk/ux500/clk-prcmu.c
@@ -153,6 +153,11 @@ static struct clk_ops clk_prcmu_gate_ops = {
.recalc_rate = clk_prcmu_recalc_rate,
};
+static struct clk_ops clk_prcmu_rate_ops = {
+ .is_enabled = clk_prcmu_is_enabled,
+ .recalc_rate = clk_prcmu_recalc_rate,
+};
+
static struct clk_ops clk_prcmu_opp_gate_ops = {
.prepare = clk_prcmu_opp_prepare,
.unprepare = clk_prcmu_opp_unprepare,
@@ -228,6 +233,15 @@ struct clk *clk_reg_prcmu_gate(const char *name,
clk_prcmu_gate_ops);
}
+struct clk *clk_reg_prcmu_rate(const char *name,
+ const char *parent_name,
+ u8 cg_sel,
+ unsigned long flags)
+{
+ return clk_reg_prcmu(name, parent_name, cg_sel, 0, flags,
+ clk_prcmu_rate_ops);
+}
+
struct clk *clk_reg_prcmu_opp_gate(const char *name,
const char *parent_name,
u8 cg_sel,
diff --git a/drivers/clk/ux500/clk.h b/drivers/clk/ux500/clk.h
index 32085aa..836d7d1 100644
--- a/drivers/clk/ux500/clk.h
+++ b/drivers/clk/ux500/clk.h
@@ -35,6 +35,11 @@ struct clk *clk_reg_prcmu_gate(const char *name,
u8 cg_sel,
unsigned long flags);
+struct clk *clk_reg_prcmu_rate(const char *name,
+ const char *parent_name,
+ u8 cg_sel,
+ unsigned long flags);
+
struct clk *clk_reg_prcmu_opp_gate(const char *name,
const char *parent_name,
u8 cg_sel,
--
1.7.10
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] dma/ste_dma40: Fixup clock usage during probe
From: Ulf Hansson ulf.hans...@linaro.org
Fixup some errorhandling for clocks during probe and make sure
to use clk_prepare as well as clk_enable.
Signed-off-by: Ulf Hansson ulf.hans...@linaro.org
Acked-by: Linus Walleij linus.wall...@linaro.org
---
drivers/dma/ste_dma40.c | 14 +-
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index 000d309..826d0d5 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -2920,19 +2920,23 @@ static struct d40_base * __init
d40_hw_detect_init(struct platform_device *pdev)
struct d40_base *base = NULL;
int num_log_chans = 0;
int num_phy_chans;
+ int clk_ret = -EINVAL;
int i;
u32 pid;
u32 cid;
u8 rev;
clk = clk_get(pdev-dev, NULL);
-
if (IS_ERR(clk)) {
d40_err(pdev-dev, No matching clock found\n);
goto failure;
}
- clk_enable(clk);
+ clk_ret = clk_prepare_enable(clk);
+ if (clk_ret) {
+ d40_err(pdev-dev, Failed to prepare/enable clock\n);
+ goto failure;
+ }
/* Get IO for DMAC base address */
res = platform_get_resource_byname(pdev, IORESOURCE_MEM, base);
@@ -3062,10 +3066,10 @@ static struct d40_base * __init
d40_hw_detect_init(struct platform_device *pdev)
return base;
failure:
- if (!IS_ERR(clk)) {
- clk_disable(clk);
+ if (!clk_ret)
+ clk_disable_unprepare(clk);
+ if (!IS_ERR(clk))
clk_put(clk);
- }
if (virtbase)
iounmap(virtbase);
if (res)
--
1.7.10
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 7/13] drivers/mmc/host/mmci.c: use clk_prepare_enable and clk_disable_unprepare
Hi Julia, Patches for mmci should normally be sent on the arm list as well. mmci is maintained by Russell King, even if the MAINTAINER file does not say so. Some minor comment below. On 26 August 2012 18:00, Julia Lawall julia.law...@lip6.fr wrote: From: Julia Lawall julia.law...@lip6.fr I think can simplify the commit msg a lot. Just say something with simplify clock code in probe. Clk_prepare_enable and clk_disable_unprepare combine clk_prepare and clk_enable, and clk_disable and clk_unprepare. They make the code more concise, and ensure that clk_unprepare is called when clk_enable fails. A simplified version of the semantic patch that introduces calls to these functions is as follows: (http://coccinelle.lip6.fr/) // smpl @@ expression e; @@ - clk_prepare(e); - clk_enable(e); + clk_prepare_enable(e); @@ expression e; @@ - clk_disable(e); - clk_unprepare(e); + clk_disable_unprepare(e); // /smpl Signed-off-by: Julia Lawall julia.law...@lip6.fr --- drivers/mmc/host/mmci.c | 13 +++-- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index 50ff19a..edc3e9b 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -1309,14 +1309,10 @@ static int __devinit mmci_probe(struct amba_device *dev, goto host_free; } - ret = clk_prepare(host-clk); + ret = clk_prepare_enable(host-clk); white space? Did you run checkpatch? if (ret) goto clk_free; - ret = clk_enable(host-clk); - if (ret) - goto clk_unprep; - host-plat = plat; host-variant = variant; host-mclk = clk_get_rate(host-clk); @@ -1515,9 +1511,7 @@ static int __devinit mmci_probe(struct amba_device *dev, err_gpio_cd: iounmap(host-base); clk_disable: - clk_disable(host-clk); - clk_unprep: - clk_unprepare(host-clk); + clk_disable_unprepare(host-clk); white space? Did you run checkpatch? clk_free: clk_put(host-clk); host_free: @@ -1564,8 +1558,7 @@ static int __devexit mmci_remove(struct amba_device *dev) gpio_free(host-gpio_cd); iounmap(host-base); - clk_disable(host-clk); - clk_unprepare(host-clk); + clk_disable_unprepare(host-clk); white space? Did you run checkpatch? clk_put(host-clk); if (host-vcc) -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Kind regards Ulf Hansson -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3] mmc: core: Add support for idle time BKOPS
Hi Maya, On 4 December 2012 22:17, me...@codeaurora.org wrote: Hi Ulf, Let me try to better explain: The idea behind the periodic BKOPS is to check the card's need for BKOPS periodically in order to prevent an urgent BKOPS need by the card. In order to actually manage to prevent the urgent BKOPS need, the host should give the card enough time to perform the BKOPS (when it recognizes BKOPS need), otherwise there is no point in having the periodic BKOPS. The above results in the following: 1. After starting non-urgent BKOPS we delay getting into suspend by polling on the card's status (explanation below), in order to give the card time to perform the BKOPS. This has no effect on the power consumption since the same BKOPS operations that were performed after the foregound operation just moved to another location, meaning before going into suspend. I am not sure what you are talking about here, runtime suspend or system suspend? Polling the card's status will not prevent any of this. So you have got this wrong. 2. Using PM_SUSPEND_PREPARE instead of the workqueue would not be efficient since we don't want to wait until the host is ready to get into suspend and then prevent him from suspending by doing BKOPS operations that can take a long time. It is better to start the BKOPS earlier. I did not suggest to use PM_SUSPEND_PREPARE, but to use runtime PM for the card device. It can be an option to implement this feature on top of a workqueue. At least worth to consider. I am not too familiar with the controllers code and also my understanding in runtime suspend is very basic, so feel free to correct me if I'm wrong here or the behavior in other controllers is different from msm_sdcc. mmc_claim_host calls host-ops-enable. This API is implemented per controller but as far as I understand it, this API must prevent suspend, otherwise we might go into suspend while there is bus activity. By doing get_card_status we call mmc_claim_host and this call is the one that delays getting into suspend. host-ops-enable is the old way of implementing runtime power save for host drivers. Nowadays most drivers is using runtime PM instead. When you say that mmc_claim_host will prevent suspend, I suppose you mean that host-ops-disable wont be called? That is definitely not the same as preventing a system suspend, and moreover it should not. If you think that the host must be prevented from entering runtime power save (runtime_supend or host-ops-disable), you must elaborate more on this, because I don't understand why this is needed. If this is not the case in other controllers than the BKOPS will not prevent the suspend and BKOPS will be interrupted. As for the effect on the battery consumption, this is probably something specific to our controller, so sorry if I created a confusion. Additional comments inline. Thanks, Maya On Tue, December 4, 2012 1:52 am, Ulf Hansson wrote: On 3 December 2012 10:49, me...@codeaurora.org wrote: Hi Jaehoon, With this patch we don't expect to see any degradation. Thanks for verifying that. The test plan would be to run the lmdd and iozone benchmarks with this patch and verify that the performance is not degraded. I verified it with the msm_sdcc controller. Chris - We do expect it to influence the battery consumption, since we now delay getting into suspend (since mmc_start_bkops which is called after the delayed work is executed claims the host). The solution for that should be done by the controller which can shorten the timeout given to pm_schedule_suspend by the periodic BKOPS idle time. Does it make sense to you? Thanks, Maya On Thu, November 29, 2012 4:40 am, Jaehoon Chung wrote: Hi Maya, Thank you a lot for working idle time BKOPS. I tested with this patch. It's working fine.(Suspend/resume is also working well.) Test controller is sdhci controller. When i tested the performance with iozone, i didn't find that performance is decreased. Well, as Chris is mentioned, do you have any test plan? So I will test more with this patch, because i want to test with dw-mmc controller, too. On 11/25/2012 08:56 PM, Maya Erez wrote: Devices have various maintenance operations need to perform internally. In order to reduce latencies during time critical operations like read and write, it is better to execute maintenance operations in other times - when the host is not being serviced. Such operations are called Background operations (BKOPS). The device notifies the status of the BKOPS need by updating BKOPS_STATUS (EXT_CSD byte [246]). According to the standard a host that supports BKOPS shall check the status periodically and start background operations as needed, so that the device has enough time for its maintenance operations. This patch adds support for this periodic check of the BKOPS status. Since foreground operations are of higher priority than background operations the host will check the need
Re: [PATCH 11/12] ARM: ux500: Use the GPIO regulator framework for SDI0's 'en' and 'vsel'
On 10 December 2012 09:56, Lee Jones lee.jo...@linaro.org wrote:
To prevent lots of unnecessary call-backs into platform code, we're
now using the GPIO regulator framework to control the 'enable' (en)
and 'voltage select' (vsel) GPIO pins which in turn control the
MMCI's secondary regulator settings. This already works with Device
Tree, but when booting with ATAGs we need to register it as a
platform device.
Signed-off-by: Lee Jones lee.jo...@linaro.org
---
arch/arm/mach-ux500/board-mop500.c | 61
1 file changed, 61 insertions(+)
diff --git a/arch/arm/mach-ux500/board-mop500.c
b/arch/arm/mach-ux500/board-mop500.c
index daa4237..9f9fe7f 100644
--- a/arch/arm/mach-ux500/board-mop500.c
+++ b/arch/arm/mach-ux500/board-mop500.c
@@ -24,6 +24,8 @@
#include linux/mfd/abx500/ab8500.h
#include linux/regulator/ab8500.h
#include linux/regulator/fixed.h
+#include linux/regulator/driver.h
+#include linux/regulator/gpio-regulator.h
#include linux/mfd/tc3589x.h
#include linux/mfd/tps6105x.h
#include linux/mfd/abx500/ab8500-gpio.h
@@ -91,6 +93,54 @@ static struct platform_device
snowball_gpio_en_3v3_regulator_dev = {
},
};
+static struct regulator_consumer_supply sdi0_reg_consumers[] = {
+REGULATOR_SUPPLY(vqmmc, NULL),
REGULATOR_SUPPLY(vqmmc, sdi0),
+};
+
+static struct regulator_init_data sdi0_reg_init_data = {
The levelshifter uses the ab8500-ext-supply3. Reflect that here.
.supply_regulator = ab8500-ext-supply3
+.constraints = {
+.min_uV = 180,
2.9V, not 3.3V
+.max_uV = 330,
+.valid_ops_mask =
REGULATOR_CHANGE_VOLTAGE|REGULATOR_CHANGE_STATUS,
+},
+.num_consumer_supplies = ARRAY_SIZE(sdi0_reg_consumers),
+.consumer_supplies = sdi0_reg_consumers,
+};
+
+/* Dynamically populated. */
+static struct gpio sdi0_reg_gpios[] = {
+{ 0, GPIOF_OUT_INIT_LOW, mmci_vsel },
+};
+
+static struct gpio_regulator_state sdi0_reg_states[] = {
+{ .value = 330, .gpios = (0 0) },
2.9V, not 3.3V
+{ .value = 180, .gpios = (1 0) },
+};
+
+static struct gpio_regulator_config sdi0_reg_info = {
+.supply_name = mmci-reg,
+
+.enable_high = 1,
+.enabled_at_boot = 0,
+
+.gpios = sdi0_reg_gpios,
+.nr_gpios = ARRAY_SIZE(sdi0_reg_gpios),
+
+.states = sdi0_reg_states,
+.nr_states = ARRAY_SIZE(sdi0_reg_states),
+
+.type = REGULATOR_VOLTAGE,
+.init_data = sdi0_reg_init_data,
+};
+
+static struct platform_device sdi0_regulator = {
+.name = gpio-regulator,
+.id = -1,
+.dev = {
+.platform_data = sdi0_reg_info,
+},
+};
+
static struct ab8500_gpio_platform_data ab8500_gpio_pdata = {
.gpio_base = MOP500_AB8500_PIN_GPIO(1),
.irq_base = MOP500_AB8500_VIR_GPIO_IRQ_BASE,
@@ -440,6 +490,7 @@ static struct hash_platform_data
u8500_hash1_platform_data = {
/* add any platform devices here - TODO */
static struct platform_device *mop500_platform_devs[] __initdata = {
mop500_gpio_keys_device,
+ sdi0_regulator,
};
#ifdef CONFIG_STE_DMA40
@@ -581,6 +632,7 @@ static struct platform_device *snowball_platform_devs[]
__initdata = {
snowball_key_dev,
snowball_sbnet_dev,
snowball_gpio_en_3v3_regulator_dev,
+ sdi0_regulator,
};
static void __init mop500_init_machine(void)
@@ -591,6 +643,9 @@ static void __init mop500_init_machine(void)
mop500_gpio_keys[0].gpio = GPIO_PROX_SENSOR;
+ sdi0_reg_info.enable_gpio = GPIO_SDMMC_EN;
+ sdi0_reg_info.gpios[0].gpio = GPIO_SDMMC_1V8_3V_SEL;
+
mop500_pinmaps_init();
parent = u8500_init_devices(ab8500_platdata);
@@ -623,6 +678,9 @@ static void __init snowball_init_machine(void)
struct device *parent = NULL;
int i;
+ sdi0_reg_info.enable_gpio = SNOWBALL_SDMMC_EN_GPIO;
+ sdi0_reg_info.gpios[0].gpio = SNOWBALL_SDMMC_1V8_3V_GPIO;
+
snowball_pinmaps_init();
parent = u8500_init_devices(ab8500_platdata);
@@ -655,6 +713,9 @@ static void __init hrefv60_init_machine(void)
*/
mop500_gpio_keys[0].gpio = HREFV60_PROX_SENSE_GPIO;
+ sdi0_reg_info.enable_gpio = HREFV60_SDMMC_EN_GPIO;
+ sdi0_reg_info.gpios[0].gpio = HREFV60_SDMMC_1V8_3V_GPIO;
+
hrefv60_pinmaps_init();
parent = u8500_init_devices(ab8500_platdata);
--
1.7.9.5
Finally, I suppose regulator inits i located in
board-mop500-regulators.c, so I would suggest to move most of this
code in there instead.
Kind regards
Ulf Hansson
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 11/12] ARM: ux500: Use the GPIO regulator framework for SDI0's 'en' and 'vsel'
On 10 December 2012 11:18, Ulf Hansson ulf.hans...@linaro.org wrote:
On 10 December 2012 09:56, Lee Jones lee.jo...@linaro.org wrote:
To prevent lots of unnecessary call-backs into platform code, we're
now using the GPIO regulator framework to control the 'enable' (en)
and 'voltage select' (vsel) GPIO pins which in turn control the
MMCI's secondary regulator settings. This already works with Device
Tree, but when booting with ATAGs we need to register it as a
platform device.
Signed-off-by: Lee Jones lee.jo...@linaro.org
---
arch/arm/mach-ux500/board-mop500.c | 61
1 file changed, 61 insertions(+)
diff --git a/arch/arm/mach-ux500/board-mop500.c
b/arch/arm/mach-ux500/board-mop500.c
index daa4237..9f9fe7f 100644
--- a/arch/arm/mach-ux500/board-mop500.c
+++ b/arch/arm/mach-ux500/board-mop500.c
@@ -24,6 +24,8 @@
#include linux/mfd/abx500/ab8500.h
#include linux/regulator/ab8500.h
#include linux/regulator/fixed.h
+#include linux/regulator/driver.h
+#include linux/regulator/gpio-regulator.h
#include linux/mfd/tc3589x.h
#include linux/mfd/tps6105x.h
#include linux/mfd/abx500/ab8500-gpio.h
@@ -91,6 +93,54 @@ static struct platform_device
snowball_gpio_en_3v3_regulator_dev = {
},
};
+static struct regulator_consumer_supply sdi0_reg_consumers[] = {
+REGULATOR_SUPPLY(vqmmc, NULL),
REGULATOR_SUPPLY(vqmmc, sdi0),
+};
+
+static struct regulator_init_data sdi0_reg_init_data = {
The levelshifter uses the ab8500-ext-supply3. Reflect that here.
.supply_regulator = ab8500-ext-supply3
+.constraints = {
+.min_uV = 180,
2.9V, not 3.3V
+.max_uV = 330,
+.valid_ops_mask =
REGULATOR_CHANGE_VOLTAGE|REGULATOR_CHANGE_STATUS,
+},
+.num_consumer_supplies = ARRAY_SIZE(sdi0_reg_consumers),
+.consumer_supplies = sdi0_reg_consumers,
+};
+
+/* Dynamically populated. */
+static struct gpio sdi0_reg_gpios[] = {
+{ 0, GPIOF_OUT_INIT_LOW, mmci_vsel },
+};
+
+static struct gpio_regulator_state sdi0_reg_states[] = {
+{ .value = 330, .gpios = (0 0) },
2.9V, not 3.3V
+{ .value = 180, .gpios = (1 0) },
+};
+
+static struct gpio_regulator_config sdi0_reg_info = {
+.supply_name = mmci-reg,
+
+.enable_high = 1,
+.enabled_at_boot = 0,
+
+.gpios = sdi0_reg_gpios,
+.nr_gpios = ARRAY_SIZE(sdi0_reg_gpios),
+
I think the settling time is missing here. 100us according to
levelshifter spec.
+.states = sdi0_reg_states,
+.nr_states = ARRAY_SIZE(sdi0_reg_states),
+
+.type = REGULATOR_VOLTAGE,
+.init_data = sdi0_reg_init_data,
+};
+
+static struct platform_device sdi0_regulator = {
+.name = gpio-regulator,
+.id = -1,
+.dev = {
+.platform_data = sdi0_reg_info,
+},
+};
+
static struct ab8500_gpio_platform_data ab8500_gpio_pdata = {
.gpio_base = MOP500_AB8500_PIN_GPIO(1),
.irq_base = MOP500_AB8500_VIR_GPIO_IRQ_BASE,
@@ -440,6 +490,7 @@ static struct hash_platform_data
u8500_hash1_platform_data = {
/* add any platform devices here - TODO */
static struct platform_device *mop500_platform_devs[] __initdata = {
mop500_gpio_keys_device,
+ sdi0_regulator,
};
#ifdef CONFIG_STE_DMA40
@@ -581,6 +632,7 @@ static struct platform_device *snowball_platform_devs[]
__initdata = {
snowball_key_dev,
snowball_sbnet_dev,
snowball_gpio_en_3v3_regulator_dev,
+ sdi0_regulator,
};
static void __init mop500_init_machine(void)
@@ -591,6 +643,9 @@ static void __init mop500_init_machine(void)
mop500_gpio_keys[0].gpio = GPIO_PROX_SENSOR;
+ sdi0_reg_info.enable_gpio = GPIO_SDMMC_EN;
+ sdi0_reg_info.gpios[0].gpio = GPIO_SDMMC_1V8_3V_SEL;
+
mop500_pinmaps_init();
parent = u8500_init_devices(ab8500_platdata);
@@ -623,6 +678,9 @@ static void __init snowball_init_machine(void)
struct device *parent = NULL;
int i;
+ sdi0_reg_info.enable_gpio = SNOWBALL_SDMMC_EN_GPIO;
+ sdi0_reg_info.gpios[0].gpio = SNOWBALL_SDMMC_1V8_3V_GPIO;
+
snowball_pinmaps_init();
parent = u8500_init_devices(ab8500_platdata);
@@ -655,6 +713,9 @@ static void __init hrefv60_init_machine(void)
*/
mop500_gpio_keys[0].gpio = HREFV60_PROX_SENSE_GPIO;
+ sdi0_reg_info.enable_gpio = HREFV60_SDMMC_EN_GPIO;
+ sdi0_reg_info.gpios[0].gpio = HREFV60_SDMMC_1V8_3V_GPIO;
+
hrefv60_pinmaps_init();
parent = u8500_init_devices(ab8500_platdata);
--
1.7.9.5
Finally, I suppose regulator inits i located in
board-mop500-regulators.c, so I would suggest to move most of this
code in there instead.
Kind regards
Ulf Hansson
--
To unsubscribe from this list
Re: [PATCH 11/12] ARM: ux500: Use the GPIO regulator framework for SDI0's 'en' and 'vsel'
On 10 December 2012 11:30, Lee Jones lee.jo...@linaro.org wrote:
+.constraints = {
+.min_uV = 180,
2.9V, not 3.3V
+static struct gpio_regulator_state sdi0_reg_states[] = {
+{ .value = 330, .gpios = (0 0) },
2.9V, not 3.3V
I'm still a little unsure about this. I know the actual
voltage is v2.9, but the supported/requested MMC voltage
from the driver is v3.3.
Will it still work if I set it all up as v2.9?
So that is if course important to consider. Although the regulator
must nu lie about what it actually supports.
In this case mmci driver will have to decide what to do when 3.3V is
requested, but only 2.9 is supported.
--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
Kind regards
Ulf Hansson
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 11/12 v2] ARM: ux500: Use the GPIO regulator framework for SDI0's 'en' and 'vsel'
On 10 December 2012 12:08, Lee Jones lee.jo...@linaro.org wrote:
To prevent lots of unnecessary call-backs into platform code, we're
now using the GPIO regulator framework to control the 'enable' (en)
and 'voltage select' (vsel) GPIO pins which in turn control the
MMCI's secondary regulator settings. This already works with Device
Tree, but when booting with ATAGs we need to register it as a
platform device.
Signed-off-by: Lee Jones lee.jo...@linaro.org
---
arch/arm/mach-ux500/board-mop500-regulators.c | 14
arch/arm/mach-ux500/board-mop500-regulators.h |1 +
arch/arm/mach-ux500/board-mop500.c| 44
+
3 files changed, 59 insertions(+)
diff --git a/arch/arm/mach-ux500/board-mop500-regulators.c
b/arch/arm/mach-ux500/board-mop500-regulators.c
index 2a17bc5..cb75405 100644
--- a/arch/arm/mach-ux500/board-mop500-regulators.c
+++ b/arch/arm/mach-ux500/board-mop500-regulators.c
@@ -28,6 +28,20 @@ struct regulator_init_data gpio_en_3v3_regulator = {
.consumer_supplies = gpio_en_3v3_consumers,
};
+static struct regulator_consumer_supply sdi0_reg_consumers[] = {
+REGULATOR_SUPPLY(vqmmc, sdi0),
+};
+
+struct regulator_init_data sdi0_reg_init_data = {
You missed this:
The levelshifter uses the ab8500-ext-supply3. Reflect that here.
.supply_regulator = ab8500-ext-supply3
+.constraints = {
+.min_uV = 180,
+.max_uV = 290,
+.valid_ops_mask =
REGULATOR_CHANGE_VOLTAGE|REGULATOR_CHANGE_STATUS,
+},
+.num_consumer_supplies = ARRAY_SIZE(sdi0_reg_consumers),
+.consumer_supplies = sdi0_reg_consumers,
+};
+
/*
* TPS61052 regulator
*/
diff --git a/arch/arm/mach-ux500/board-mop500-regulators.h
b/arch/arm/mach-ux500/board-mop500-regulators.h
index 78a0642..0c79d90 100644
--- a/arch/arm/mach-ux500/board-mop500-regulators.h
+++ b/arch/arm/mach-ux500/board-mop500-regulators.h
@@ -19,5 +19,6 @@ ab8500_regulator_reg_init[AB8500_NUM_REGULATOR_REGISTERS];
extern struct regulator_init_data ab8500_regulators[AB8500_NUM_REGULATORS];
extern struct regulator_init_data tps61052_regulator;
extern struct regulator_init_data gpio_en_3v3_regulator;
+extern struct regulator_init_data sdi0_reg_init_data;
#endif
diff --git a/arch/arm/mach-ux500/board-mop500.c
b/arch/arm/mach-ux500/board-mop500.c
index daa4237..1d721ea 100644
--- a/arch/arm/mach-ux500/board-mop500.c
+++ b/arch/arm/mach-ux500/board-mop500.c
@@ -24,6 +24,8 @@
#include linux/mfd/abx500/ab8500.h
#include linux/regulator/ab8500.h
#include linux/regulator/fixed.h
+#include linux/regulator/driver.h
+#include linux/regulator/gpio-regulator.h
#include linux/mfd/tc3589x.h
#include linux/mfd/tps6105x.h
#include linux/mfd/abx500/ab8500-gpio.h
@@ -91,6 +93,37 @@ static struct platform_device
snowball_gpio_en_3v3_regulator_dev = {
},
};
+/* Dynamically populated. */
+static struct gpio sdi0_reg_gpios[] = {t
+{ 0, GPIOF_OUT_INIT_LOW, mmci_vsel },
+};
+
+static struct gpio_regulator_state sdi0_reg_states[] = {
+{ .value = 290, .gpios = (0 0) },
+{ .value = 180, .gpios = (1 0) },
+};
+
+static struct gpio_regulator_config sdi0_reg_info = {
+ .supply_name = ab8500-ext-supply3,
No, this is not what I think you want. This regulator can be called
something with sdcard or levelshifter...
+ .gpios = sdi0_reg_gpios,
+ .nr_gpios = ARRAY_SIZE(sdi0_reg_gpios),
+ .states= sdi0_reg_states,
+ .nr_states = ARRAY_SIZE(sdi0_reg_states),
+ .type = REGULATOR_VOLTAGE,
+ .enable_high = 1,
+ .enabled_at_boot = 0,
+ .init_data = sdi0_reg_init_data,
+ .startup_delay = 100,
+};
+
+static struct platform_device sdi0_regulator = {
+.name = gpio-regulator,
+.id = -1,
+.dev = {
+.platform_data = sdi0_reg_info,
+},
+};
+
static struct ab8500_gpio_platform_data ab8500_gpio_pdata = {
.gpio_base = MOP500_AB8500_PIN_GPIO(1),
.irq_base = MOP500_AB8500_VIR_GPIO_IRQ_BASE,
@@ -440,6 +473,7 @@ static struct hash_platform_data
u8500_hash1_platform_data = {
/* add any platform devices here - TODO */
static struct platform_device *mop500_platform_devs[] __initdata = {
mop500_gpio_keys_device,
+ sdi0_regulator,
};
#ifdef CONFIG_STE_DMA40
@@ -581,6 +615,7 @@ static struct platform_device *snowball_platform_devs[]
__initdata = {
snowball_key_dev,
snowball_sbnet_dev,
snowball_gpio_en_3v3_regulator_dev,
+ sdi0_regulator,
};
static void __init mop500_init_machine(void)
@@ -591,6 +626,9 @@ static void __init mop500_init_machine(void)
Re: [PATCH 06/12] ARM: ux500: Set correct MMCI regulator voltages in the ux5x0 Device Tree
On 11 December 2012 10:17, Linus Walleij linus.wall...@linaro.org wrote: On Mon, Dec 10, 2012 at 9:55 AM, Lee Jones lee.jo...@linaro.org wrote: The voltages for the MMCI GPIO controlled voltage regulation differ depending on who you believe and where you pick the information up from. More recent internal code suggests that the true upper voltage from the shifter is actually v3.3, instead of the previously indicated v2.9. Let's mirror that change in the Device Tree file. This is wrong. The value should be 2.9V nothing else! Signed-off-by: Lee Jones lee.jo...@linaro.org Acked-by: Linus Walleij linus.wall...@linaro.org Yours, Linus Walleij Kind regards Ulf Hansson -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/2] Input: nomadik-ske-keypad - clk fixups
On 1 November 2012 15:20, Ulf Hansson ulf.hans...@stericsson.com wrote: From: Ulf Hansson ulf.hans...@linaro.org Due to the convert to the common clk driver these changes for clks are needed. Ulf Hansson (2): Input: nomadik-ske-keypad - fixup use of clk Input: nomadik-ske-keypad - start using the apb_pclk drivers/input/keyboard/nomadik-ske-keypad.c | 34 +++ 1 file changed, 29 insertions(+), 5 deletions(-) -- 1.7.10 Just a kind reminder here. Do we see any issues with merging this? Kind regards Ulf Hansson -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Resend][PATCH] PM: Move disabling/enabling runtime PM to late suspend/early resume
On 16 December 2012 16:29, Alan Stern st...@rowland.harvard.edu wrote: On Sun, 16 Dec 2012, Rafael J. Wysocki wrote: On Saturday, December 15, 2012 10:16:29 PM Jiri Kosina wrote: On Sat, 15 Dec 2012, Rafael J. Wysocki wrote: From: Rafael J. Wysocki rafael.j.wyso...@intel.com Currently, the PM core disables runtime PM for all devices right after executing subsystem/driver .suspend() callbacks for them and re-enables it right before executing subsystem/driver .resume() callbacks for them. This may lead to problems when there are two devices such that the .suspend() callback executed for one of them depends on runtime PM working for the other. In that case, if runtime PM has already been disabled for the second device, the first one's .suspend() won't work correctly (and analogously for resume). To make those issues go away, make the PM core disable runtime PM for devices right before executing subsystem/driver .suspend_late() callbacks for them and enable runtime PM for them right after executing subsystem/driver .resume_early() callbacks for them. This way the potential conflitcs between .suspend_late()/.resume_early() and their runtime PM counterparts are still prevented from happening, but the subtle ordering issues related to disabling/enabling runtime PM for devices during system suspend/resume are much easier to avoid. Reported-and-tested-by: Jan-Matthias Braun jan_br...@gmx.net Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com Hi Rafael, just curious what is the reason for resend? Do you want to gather more Acks before pushing this upstream? Well, I thought that some people might actually look at it when they found it again in their mailboxes. :-) I did look at it the first time it appeared. It seemed to be okay, but I haven't tried any testing. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-pm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html I believe this should work fine for ux500 platforms, so: Reviewed-by: Ulf Hansson ulf.hans...@linaro.org Kind regards Ulf Hansson -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3] mmc: core: Add support for idle time BKOPS
On 21 December 2012 09:35, Maya Erez me...@codeaurora.org wrote: Hi Ulf, Thanks for the info. We will consider our controller behavior in suspend. Would it be acceptable by you to keep the polling for BKOPS completion under a special CAPS2 flag? Not sure what you propose here. I guess some host cap would be needed to be able to cope with those drivers which uses runtime PM for normal suspend. But, the polling method as such shall not be done just because of preventing those drivers entering runtime susend state. pm_runtime_get* must be used here, I think. Then of course you need to poll for the BKOPS status. Thanks, Maya -Original Message- From: linux-mmc-ow...@vger.kernel.org [mailto:linux-mmc-ow...@vger.kernel.org] On Behalf Of Ulf Hansson Sent: Thursday, December 13, 2012 12:18 PM To: me...@codeaurora.org Cc: Jaehoon Chung; linux-...@vger.kernel.org; linux-arm-...@vger.kernel.org; open list Subject: Re: [PATCH v3] mmc: core: Add support for idle time BKOPS On 12 December 2012 13:32, me...@codeaurora.org wrote: Hi Ulf, Sorry for the late response. See my reply below. Thanks, Maya On Thu, December 6, 2012 2:18 am, Ulf Hansson wrote: Hi Maya, On 4 December 2012 22:17, me...@codeaurora.org wrote: Hi Ulf, Let me try to better explain: The idea behind the periodic BKOPS is to check the card's need for BKOPS periodically in order to prevent an urgent BKOPS need by the card. In order to actually manage to prevent the urgent BKOPS need, the host should give the card enough time to perform the BKOPS (when it recognizes BKOPS need), otherwise there is no point in having the periodic BKOPS. The above results in the following: 1. After starting non-urgent BKOPS we delay getting into suspend by polling on the card's status (explanation below), in order to give the card time to perform the BKOPS. This has no effect on the power consumption since the same BKOPS operations that were performed after the foregound operation just moved to another location, meaning before going into suspend. I am not sure what you are talking about here, runtime suspend or system suspend? Polling the card's status will not prevent any of this. So you have got this wrong. I am referring to the runtime suspend. Our controller implements the runtime suspend and is not using the default implementation of core/bus.c. This is not the default runtime suspend for a host device, but for the card device. Do not mix this up with runtime pm for a host device. Right now runtime pm for the card _device_ is only enabled for SDIO. Thus SDIO drivers can use runtime pm to actually trigger an SDIO card to be fully suspended when it is not needed and thus save a lot of power. For example when a WLAN interface goes up/down. This is the reason why in our implementation polling the card status delays the runtime suspend while it is not the case when using the default runtime suspend implementation. I can try to explain here what our controller is doing but since it is specific to us then I guess it is not relevant to the discussion. Our controller is calling mmc_suspend_host in runtime suspend, which issues an HPI to stop the BKOPS. So, doing mmc_suspend_host in you runtime_suspend callback, is that really what you want to do? 1. You will introduce significant latencies (I have seen SD-cards which needs more than 1 s to initialize, eMMC is better but we are anyway talking several 100 ms) once new requests arrives after the host as entered the runtime suspend state. 2. SD cards has no power off notification, so you will actually stress test the SD cards internal FTL to be crash safe by cutting the power to it more often. 3. You will prevent SD-cards from doing it's back ground operations, which is done automatically and not like in a controlled manner as for eMMC. So of course, you save some power, but is the consequences worth it? :-) Now that I understand that this code is not needed for all the host drivers I will add a flag to decide if polling is required when doing an unblocking BKOPS. You must not poll to prevent this! Instead you need to prevent the host from going into runtime suspend state, which is simply done by pm_runtime_get_sync for the host device. Although, it _must_ not be done for drivers not doing mmc_suspend_host in their runtime suspend callbacks. Since then it will prevent these from doing runtime power save actions, which is not ok. Other host drivers that actually suspend on runtime suspend can enable this flag and allow BKOPS to be active for a longer period. I will prepare a new patch and send it for review. 2. Using PM_SUSPEND_PREPARE instead of the workqueue would not be efficient since we don't want to wait until the host is ready to get into suspend and then prevent him from suspending by doing BKOPS operations that can take a long time. It is better to start the BKOPS earlier. I did not suggest to use
Re: [PATCH v3] mmc: core: Add support for idle time BKOPS
On 12 December 2012 13:32, me...@codeaurora.org wrote: Hi Ulf, Sorry for the late response. See my reply below. Thanks, Maya On Thu, December 6, 2012 2:18 am, Ulf Hansson wrote: Hi Maya, On 4 December 2012 22:17, me...@codeaurora.org wrote: Hi Ulf, Let me try to better explain: The idea behind the periodic BKOPS is to check the card's need for BKOPS periodically in order to prevent an urgent BKOPS need by the card. In order to actually manage to prevent the urgent BKOPS need, the host should give the card enough time to perform the BKOPS (when it recognizes BKOPS need), otherwise there is no point in having the periodic BKOPS. The above results in the following: 1. After starting non-urgent BKOPS we delay getting into suspend by polling on the card's status (explanation below), in order to give the card time to perform the BKOPS. This has no effect on the power consumption since the same BKOPS operations that were performed after the foregound operation just moved to another location, meaning before going into suspend. I am not sure what you are talking about here, runtime suspend or system suspend? Polling the card's status will not prevent any of this. So you have got this wrong. I am referring to the runtime suspend. Our controller implements the runtime suspend and is not using the default implementation of core/bus.c. This is not the default runtime suspend for a host device, but for the card device. Do not mix this up with runtime pm for a host device. Right now runtime pm for the card _device_ is only enabled for SDIO. Thus SDIO drivers can use runtime pm to actually trigger an SDIO card to be fully suspended when it is not needed and thus save a lot of power. For example when a WLAN interface goes up/down. This is the reason why in our implementation polling the card status delays the runtime suspend while it is not the case when using the default runtime suspend implementation. I can try to explain here what our controller is doing but since it is specific to us then I guess it is not relevant to the discussion. Our controller is calling mmc_suspend_host in runtime suspend, which issues an HPI to stop the BKOPS. So, doing mmc_suspend_host in you runtime_suspend callback, is that really what you want to do? 1. You will introduce significant latencies (I have seen SD-cards which needs more than 1 s to initialize, eMMC is better but we are anyway talking several 100 ms) once new requests arrives after the host as entered the runtime suspend state. 2. SD cards has no power off notification, so you will actually stress test the SD cards internal FTL to be crash safe by cutting the power to it more often. 3. You will prevent SD-cards from doing it's back ground operations, which is done automatically and not like in a controlled manner as for eMMC. So of course, you save some power, but is the consequences worth it? :-) Now that I understand that this code is not needed for all the host drivers I will add a flag to decide if polling is required when doing an unblocking BKOPS. You must not poll to prevent this! Instead you need to prevent the host from going into runtime suspend state, which is simply done by pm_runtime_get_sync for the host device. Although, it _must_ not be done for drivers not doing mmc_suspend_host in their runtime suspend callbacks. Since then it will prevent these from doing runtime power save actions, which is not ok. Other host drivers that actually suspend on runtime suspend can enable this flag and allow BKOPS to be active for a longer period. I will prepare a new patch and send it for review. 2. Using PM_SUSPEND_PREPARE instead of the workqueue would not be efficient since we don't want to wait until the host is ready to get into suspend and then prevent him from suspending by doing BKOPS operations that can take a long time. It is better to start the BKOPS earlier. I did not suggest to use PM_SUSPEND_PREPARE, but to use runtime PM for the card device. It can be an option to implement this feature on top of a workqueue. At least worth to consider. We consider to call mmc_start_bkops every time MMC becomes idle, to check the need for BKOPS. I will test it and include it in the next patch. I am not too familiar with the controllers code and also my understanding in runtime suspend is very basic, so feel free to correct me if I'm wrong here or the behavior in other controllers is different from msm_sdcc. mmc_claim_host calls host-ops-enable. This API is implemented per controller but as far as I understand it, this API must prevent suspend, otherwise we might go into suspend while there is bus activity. By doing get_card_status we call mmc_claim_host and this call is the one that delays getting into suspend. host-ops-enable is the old way of implementing runtime power save for host drivers. Nowadays most drivers is using runtime PM instead. When you say that mmc_claim_host
Re: [PATCH 7/8] ARM: ux500: Use the GPIO regulator framework for SDI0's 'en' and 'vsel'
); @@ -623,6 +661,9 @@ static void __init snowball_init_machine(void) struct device *parent = NULL; int i; + sdi0_reg_info.enable_gpio = SNOWBALL_SDMMC_EN_GPIO; + sdi0_reg_info.gpios[0].gpio = SNOWBALL_SDMMC_1V8_3V_GPIO; + snowball_pinmaps_init(); parent = u8500_init_devices(ab8500_platdata); @@ -655,6 +696,9 @@ static void __init hrefv60_init_machine(void) */ mop500_gpio_keys[0].gpio = HREFV60_PROX_SENSE_GPIO; + sdi0_reg_info.enable_gpio = HREFV60_SDMMC_EN_GPIO; + sdi0_reg_info.gpios[0].gpio = HREFV60_SDMMC_1V8_3V_GPIO; + hrefv60_pinmaps_init(); parent = u8500_init_devices(ab8500_platdata); -- 1.7.9.5 Acked-by: Ulf Hansson ulf.hans...@linaro.org -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 8/8] ARM: ux500: Remove traces of the ios_handler from platform code
On 13 December 2012 14:22, Lee Jones lee.jo...@linaro.org wrote:
Now MMCI on/off functionality is using the regulator framework
from the MMCI driver, there is no need to keep the ios_handler
laying around, duplicating functionality. So we're removing it.
Acked-by: Linus Walleij linus.wall...@linaro.org
Signed-off-by: Lee Jones lee.jo...@linaro.org
---
arch/arm/mach-ux500/board-mop500-sdi.c | 52
1 file changed, 52 deletions(-)
diff --git a/arch/arm/mach-ux500/board-mop500-sdi.c
b/arch/arm/mach-ux500/board-mop500-sdi.c
index 9c8e4a9..5a798d6 100644
--- a/arch/arm/mach-ux500/board-mop500-sdi.c
+++ b/arch/arm/mach-ux500/board-mop500-sdi.c
@@ -31,35 +31,6 @@
* SDI 0 (MicroSD slot)
*/
-/* GPIO pins used by the sdi0 level shifter */
-static int sdi0_en = -1;
-static int sdi0_vsel = -1;
-
-static int mop500_sdi0_ios_handler(struct device *dev, struct mmc_ios *ios)
-{
- switch (ios-power_mode) {
- case MMC_POWER_UP:
- case MMC_POWER_ON:
- /*
-* Level shifter voltage should depend on vdd to when deciding
-* on either 1.8V or 2.9V. Once the decision has been made the
-* level shifter must be disabled and re-enabled with a
changed
-* select signal in order to switch the voltage. Since there
is
-* no framework support yet for indicating 1.8V in vdd, use
the
-* default 2.9V.
-*/
- gpio_direction_output(sdi0_vsel, 0);
- gpio_direction_output(sdi0_en, 1);
- break;
- case MMC_POWER_OFF:
- gpio_direction_output(sdi0_vsel, 0);
- gpio_direction_output(sdi0_en, 0);
- break;
- }
-
- return 0;
-}
-
#ifdef CONFIG_STE_DMA40
struct stedma40_chan_cfg mop500_sdi0_dma_cfg_rx = {
.mode = STEDMA40_MODE_LOGICAL,
@@ -81,7 +52,6 @@ static struct stedma40_chan_cfg mop500_sdi0_dma_cfg_tx = {
#endif
struct mmci_platform_data mop500_sdi0_data = {
- .ios_handler= mop500_sdi0_ios_handler,
.ocr_mask = MMC_VDD_29_30,
.f_max = 5000,
.capabilities = MMC_CAP_4_BIT_DATA |
@@ -101,22 +71,6 @@ struct mmci_platform_data mop500_sdi0_data = {
static void sdi0_configure(struct device *parent)
{
- int ret;
-
- ret = gpio_request(sdi0_en, level shifter enable);
- if (!ret)
- ret = gpio_request(sdi0_vsel,
- level shifter 1v8-3v select);
-
- if (ret) {
- pr_warning(unable to config sdi0 gpios for level
shifter.\n);
- return;
- }
-
- /* Select the default 2.9V and enable level shifter */
- gpio_direction_output(sdi0_vsel, 0);
- gpio_direction_output(sdi0_en, 1);
-
/* Add the device, force v2 to subrevision 1 */
db8500_add_sdi0(parent, mop500_sdi0_data, U8500_SDI_V2_PERIPHID);
}
@@ -124,8 +78,6 @@ static void sdi0_configure(struct device *parent)
void mop500_sdi_tc35892_init(struct device *parent)
{
mop500_sdi0_data.gpio_cd = GPIO_SDMMC_CD;
- sdi0_en = GPIO_SDMMC_EN;
- sdi0_vsel = GPIO_SDMMC_1V8_3V_SEL;
sdi0_configure(parent);
}
@@ -264,8 +216,6 @@ void __init snowball_sdi_init(struct device *parent)
/* External Micro SD slot */
mop500_sdi0_data.gpio_cd = SNOWBALL_SDMMC_CD_GPIO;
mop500_sdi0_data.cd_invert = true;
- sdi0_en = SNOWBALL_SDMMC_EN_GPIO;
- sdi0_vsel = SNOWBALL_SDMMC_1V8_3V_GPIO;
sdi0_configure(parent);
}
@@ -277,8 +227,6 @@ void __init hrefv60_sdi_init(struct device *parent)
db8500_add_sdi4(parent, mop500_sdi4_data, U8500_SDI_V2_PERIPHID);
/* External Micro SD slot */
mop500_sdi0_data.gpio_cd = HREFV60_SDMMC_CD_GPIO;
- sdi0_en = HREFV60_SDMMC_EN_GPIO;
- sdi0_vsel = HREFV60_SDMMC_1V8_3V_GPIO;
sdi0_configure(parent);
/* WLAN SDIO channel */
db8500_add_sdi1(parent, mop500_sdi1_data, U8500_SDI_V2_PERIPHID);
--
1.7.9.5
Acked-by: Ulf Hansson ulf.hans...@linaro.org
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 1/2] mmc: core: Add support for idle time BKOPS
;
}
- mmc_card_clr_doing_bkops(host-card);
}
spin_lock_irqsave(host-lock, flags);
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index e6e3911..f68624a 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1275,6 +1275,26 @@ static int mmc_init_card(struct mmc_host *host,
u32 ocr,
}
}
+ if (!oldcard) {
+ if (card-ext_csd.bkops_en) {
+ INIT_DELAYED_WORK(card-bkops_info.dw,
+ mmc_start_idle_time_bkops);
+
+ /*
+* Calculate the time to start the BKOPs
checking.
+* The host controller can set this time in
order to
+* prevent a race condition before starting
BKOPs
+* and going into suspend.
+* If the host controller didn't set this time,
+* a default value is used.
+*/
+ card-bkops_info.delay_ms =
MMC_IDLE_BKOPS_TIME_MS;
+ if (card-bkops_info.host_delay_ms)
+ card-bkops_info.delay_ms =
+ card-bkops_info.host_delay_ms;
+ }
+ }
+
if (!oldcard)
host-card = card;
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index 5c69315..ca4cf19 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -210,6 +210,30 @@ struct mmc_part {
#define MMC_BLK_DATA_AREA_RPMB (13)
};
+/**
+ * struct mmc_bkops_info - BKOPS data
+ * @dw:Idle time bkops delayed work
+ * @host_delay_ms: The host controller time to start bkops
+ * @delay_ms: The time to start the BKOPS
+ *delayed work once MMC thread is idle
+ * @cancel_delayed_work: A flag to indicate if the delayed work
+ *should be cancelled
+ * @started_delayed_bkops: A flag to indicate if the delayed
+ *work was scheduled
+ */
+struct mmc_bkops_info {
+ struct delayed_work dw;
+ unsigned inthost_delay_ms;
+ unsigned intdelay_ms;
+/*
+ * A default time for checking the need for non urgent BKOPS once mmcqd
+ * is idle.
+ */
+#define MMC_IDLE_BKOPS_TIME_MS 2000
+ boolcancel_delayed_work;
+ boolstarted_delayed_bkops;
+};
+
/*
* MMC device
*/
@@ -278,6 +302,8 @@ struct mmc_card {
struct dentry *debugfs_root;
struct mmc_part part[MMC_NUM_PHY_PARTITION]; /* physical
partitions */
unsigned intnr_parts;
+
+ struct mmc_bkops_info bkops_info;
};
/*
diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
index 5bf7c22..c6426c6 100644
--- a/include/linux/mmc/core.h
+++ b/include/linux/mmc/core.h
@@ -145,6 +145,8 @@ extern int mmc_app_cmd(struct mmc_host *, struct
mmc_card *);
extern int mmc_wait_for_app_cmd(struct mmc_host *, struct mmc_card *,
struct mmc_command *, int);
extern void mmc_start_bkops(struct mmc_card *card, bool
from_exception);
+extern void mmc_start_delayed_bkops(struct mmc_card *card);
+extern void mmc_start_idle_time_bkops(struct work_struct *work);
extern int __mmc_switch(struct mmc_card *, u8, u8, u8, unsigned int,
bool);
extern int mmc_switch(struct mmc_card *, u8, u8, u8, unsigned int);
--
1.7.3.3
--
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a
member
of Code Aurora Forum, hosted by The Linux Foundation
--
To unsubscribe from this list: send the line unsubscribe linux-kernel
in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Kind regards
Ulf Hansson
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Kind regards
Ulf Hansson
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 1/2] mmc: core: Add support for idle time BKOPS
.
+* If the host controller didn't set this time,
+* a default value is used.
+*/
+ card-bkops_info.delay_ms = MMC_IDLE_BKOPS_TIME_MS;
+ if (card-bkops_info.host_delay_ms)
+ card-bkops_info.delay_ms =
+ card-bkops_info.host_delay_ms;
+ }
+ }
+
if (!oldcard)
host-card = card;
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index 5c69315..ca4cf19 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -210,6 +210,30 @@ struct mmc_part {
#define MMC_BLK_DATA_AREA_RPMB (13)
};
+/**
+ * struct mmc_bkops_info - BKOPS data
+ * @dw:Idle time bkops delayed work
+ * @host_delay_ms: The host controller time to start bkops
+ * @delay_ms: The time to start the BKOPS
+ *delayed work once MMC thread is idle
+ * @cancel_delayed_work: A flag to indicate if the delayed work
+ *should be cancelled
+ * @started_delayed_bkops: A flag to indicate if the delayed
+ *work was scheduled
+ */
+struct mmc_bkops_info {
+ struct delayed_work dw;
+ unsigned inthost_delay_ms;
+ unsigned intdelay_ms;
+/*
+ * A default time for checking the need for non urgent BKOPS once mmcqd
+ * is idle.
+ */
+#define MMC_IDLE_BKOPS_TIME_MS 2000
+ boolcancel_delayed_work;
+ boolstarted_delayed_bkops;
+};
+
/*
* MMC device
*/
@@ -278,6 +302,8 @@ struct mmc_card {
struct dentry *debugfs_root;
struct mmc_part part[MMC_NUM_PHY_PARTITION]; /* physical partitions */
unsigned intnr_parts;
+
+ struct mmc_bkops_info bkops_info;
};
/*
diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
index 5bf7c22..c6426c6 100644
--- a/include/linux/mmc/core.h
+++ b/include/linux/mmc/core.h
@@ -145,6 +145,8 @@ extern int mmc_app_cmd(struct mmc_host *, struct mmc_card
*);
extern int mmc_wait_for_app_cmd(struct mmc_host *, struct mmc_card *,
struct mmc_command *, int);
extern void mmc_start_bkops(struct mmc_card *card, bool from_exception);
+extern void mmc_start_delayed_bkops(struct mmc_card *card);
+extern void mmc_start_idle_time_bkops(struct work_struct *work);
extern int __mmc_switch(struct mmc_card *, u8, u8, u8, unsigned int, bool);
extern int mmc_switch(struct mmc_card *, u8, u8, u8, unsigned int);
--
1.7.3.3
--
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Kind regards
Ulf Hansson
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/4] clk: ux500: Provide an alias for the SMSC911x Ethernet chip
On 9 January 2013 09:56, Lee Jones lee.jo...@linaro.org wrote: On Wed, 19 Dec 2012, Lee Jones wrote: In the case of some of the ux500 platforms, an Ethernet chip is placed on an extended bus which is traditionally used as a NAND flash chip placeholder. The p3_pclk0 clock is used to control it, so we are required to provide and easy way to access it from the SMSC911x driver. We do this using an alias provided by this patch. Cc: Mike Turquette mturque...@linaro.org Cc: Ulf Hansson ulf.hans...@linaro.org Signed-off-by: Lee Jones lee.jo...@linaro.org --- drivers/clk/ux500/u8500_clk.c |1 + 1 file changed, 1 insertion(+) diff --git a/drivers/clk/ux500/u8500_clk.c b/drivers/clk/ux500/u8500_clk.c index a601802..9d9add1 100644 --- a/drivers/clk/ux500/u8500_clk.c +++ b/drivers/clk/ux500/u8500_clk.c @@ -325,6 +325,7 @@ void u8500_clk_init(void) clk = clk_reg_prcc_pclk(p3_pclk0, per3clk, U8500_CLKRST3_BASE, BIT(0), 0); clk_register_clkdev(clk, fsmc, NULL); + clk_register_clkdev(clk, NULL, smsc911x); clk = clk_reg_prcc_pclk(p3_pclk1, per3clk, U8500_CLKRST3_BASE, BIT(1), 0); -- 1.7.9.5 I still need a maintiner Ack for this before I can push it. Mike? -- Lee Jones Linaro ST-Ericsson Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog Acked-by: Ulf Hansson ulf.hans...@linaro.org -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/4 v3] net/smsc911x: Provide common clock functionality
On 9 January 2013 09:55, Lee Jones lee.jo...@linaro.org wrote: On Thu, 03 Jan 2013, Linus Walleij wrote: On Thu, Jan 3, 2013 at 12:14 PM, Lee Jones lee.jo...@linaro.org wrote: Some platforms provide clocks which require enabling before the SMSC911x chip will power on. This patch uses the new common clk framework to do just that. If no clock is provided, it will just be ignored and the driver will continue to assume that no clock is required for the chip to run successfully. Cc: Steve Glendinning steve.glendinn...@shawell.net Cc: net...@vger.kernel.org Signed-off-by: Lee Jones lee.jo...@linaro.org Looks all right to me now: Reviewed-by: Linus Walleij linus.wall...@linaro.org I still need a maintiner Ack for this before I can push it. Anyone? -- Lee Jones Linaro ST-Ericsson Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ Reviewed-by: Ulf Hansson ulf.hans...@linaro.org -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] mmc: Enable wakeup_sources for mmc core
On 22 August 2013 19:08, Zoran Markovic zoran.marko...@linaro.org wrote: Ulf, I got confirmation from Broadcom that all cell phone reference designs have card insert/removal configured as a wakeup IRQ. Unless our customers change that - which I doubt - this results in a considerable number of products implementing this feature. Please let me know how you wish to proceed. I think this patch would be useful for all mobile applications. What are the chances of getting this in the next kernel version? Hi Zoran, Recently got back from vacation, sorry for the delayed response. I am about to set up test using this patch, to validate this for a ux500 Android build. Please give me a few days to provide you with some results and input. Kind regards Ulf Hansson Thanks, Zoran -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] mmc: Enable wakeup_sources for mmc core
Hi Zoran,
On 13 June 2013 19:56, Zoran Markovic zoran.marko...@linaro.org wrote:
This is a reworked implementation of wakelocks for the MMC core from
Android kernel, originally authored by Colin Cross and San Mehat.
The patch makes sure that whenever a MMC device is inserted/removed,
the system stays awake until it's reconfigured for the new state.
It is assumed that 1/2 second is sufficient for the system to start
the configuration action for the newly detected MMC device, which might
include e.g. mounting the hosted file system(s).
The implementation uses wakeup_sources instead of wake_locks.
Feedback on the approach is greatly appreciated, in particular for the
1/2 second extension peroid.
Cc: San Mehat s...@google.com
Cc: Colin Cross ccr...@android.com
Cc: John Stultz john.stu...@linaro.org
Cc: Chris Ball c...@laptop.org
Cc: Ulf Hansson ulf.hans...@linaro.org
Cc: Johan Rudholm johan.rudh...@stericsson.com
Cc: Jaehoon Chung jh80.ch...@samsung.com
Cc: Konstantin Dorfman kdorf...@codeaurora.org
Cc: Guennadi Liakhovetski g.liakhovet...@gmx.de
Cc: Tejun Heo t...@kernel.org
Cc: Andrew Morton a...@linux-foundation.org
Signed-off-by: John Stultz john.stu...@linaro.org
[zoran.marko...@linaro.org: tweaked commit message, reworked to use
wakeup_source_register/unregister instead of wakeup_source_init/trash,
added the missing __pm_relax() for non-removable devices in mmc_rescan().]
Signed-off-by: Zoran Markovic zoran.marko...@linaro.org
---
drivers/mmc/core/core.c | 31 +--
drivers/mmc/core/host.c |7 +++
include/linux/mmc/host.h |2 ++
3 files changed, 34 insertions(+), 6 deletions(-)
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index c40396f..d5230c7 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -23,6 +23,7 @@
#include linux/log2.h
#include linux/regulator/consumer.h
#include linux/pm_runtime.h
+#include linux/pm_wakeup.h
#include linux/suspend.h
#include linux/fault-inject.h
#include linux/random.h
@@ -1656,6 +1657,7 @@ void mmc_detect_change(struct mmc_host *host, unsigned
long delay)
spin_unlock_irqrestore(host-lock, flags);
#endif
host-detect_change = 1;
+ __pm_stay_awake(host-ws);
There are some scenarios I want to be sure you have thought of. Please
comment on them.
1. mmc_detect_change does obviously not have to be run the same number
of times as the mmc_rescan function. In other words, the calls to
__pm_stay_awake is not paired with __pm_relay, I suppose this does not
matter?
2. mmc_detect_change can for example be called while the device
suspend sequence is progressing. At this point the rescan work is
disabled, thus __pm_relax will not be called, until the next rescan
work as executed which is after the complete resume cycle
(mmc_pm_notify:PM_POST_SUSPEND). Is that an issue?
mmc_schedule_delayed_work(host-detect, delay);
}
@@ -2351,13 +2353,16 @@ void mmc_rescan(struct work_struct *work)
struct mmc_host *host =
container_of(work, struct mmc_host, detect.work);
int i;
+ bool extend_wakeup = false;
if (host-rescan_disable)
return;
/* If there is a non-removable card registered, only scan once */
- if ((host-caps MMC_CAP_NONREMOVABLE) host-rescan_entered)
+ if ((host-caps MMC_CAP_NONREMOVABLE) host-rescan_entered) {
+ __pm_relax(host-ws);
By calling __pm_relax here, this indicates to me that is seems like
you might have prevented, even for a very small timeslot, with a
MMC_CAP_NONREMOVABLE card/host from the system to suspend.
For sure, you must not prevent the suspend even for small timeslots,
when MMC_CAP_NONREMOVABLE is set.
return;
+ }
host-rescan_entered = 1;
mmc_bus_get(host);
@@ -2400,16 +2405,27 @@ void mmc_rescan(struct work_struct *work)
mmc_claim_host(host);
for (i = 0; i ARRAY_SIZE(freqs); i++) {
- if (!mmc_rescan_try_freq(host, max(freqs[i], host-f_min)))
+ if (!mmc_rescan_try_freq(host, max(freqs[i], host-f_min))) {
+ /* stay awake extra time to process detected device */
+ extend_wakeup = true;
break;
+ }
if (freqs[i] = host-f_min)
break;
}
mmc_release_host(host);
out:
- if (host-caps MMC_CAP_NEEDS_POLL)
+ if (extend_wakeup)
+ /* extra 1/2 second should be enough, hopefully */
+ __pm_wakeup_event(host-ws, MSEC_PER_SEC/2);
+ else
+ __pm_relax(host-ws);
+
+ if (host-caps MMC_CAP_NEEDS_POLL) {
+ __pm_stay_awake(host-ws);
This does not make sense.
So when using polling mode to detect card insert/remove, you will
prevent suspend forever? Maybe I missed a point
Re: [RFC PATCH] mmc: Enable wakeup_sources for mmc core
On 17 June 2013 20:33, Colin Cross ccr...@android.com wrote: On Mon, Jun 17, 2013 at 7:22 AM, Ulf Hansson ulf.hans...@linaro.org wrote: On 14 June 2013 22:52, Colin Cross ccr...@android.com wrote: On Fri, Jun 14, 2013 at 11:42 AM, Zoran Markovic zoran.marko...@linaro.org wrote: I am not sure I understand why this patch is needed. When a new card is inserted/removed and the upper levels gets notification about the new card, triggering the mounting/un-mounting of the file system, why should it be the lowest layer (mmc) that prevents the platform from enter suspend/sleep? Why do we need to prevent it at all? Note that notifier handling in mmc_pm_notify, was if I remember correctly, not completely developed when the original version of this patch was being discussed. mmc_pm_notify prevents cards from being inserted/removed in the middle of suspend-resume sequence, is that not enough? I will try to speak on behalf of the original implementers in a hope they would provide the original motivation for the patch. My understanding is that any preemption in the procedure could be an opportunity to suspend, as there may be a suspend request racing with this code. This is why the calls to __pm_stay_awake() and queue_delayed_work() are so tightly coupled. It would be up to the delayed work procedure (mmc_rescan()) to decide whether or not it is safe to suspend. If there are no changes in the MMC state or all changes can be handled by mmc_rescan(), it is safe to call __pm_relax(). Otherwise, userland may take over processing of this event, and this is why the awake state needs to be extended by 1/2 second. The __pm_stay_awake() is required to prevent autosleep during the time between the card detect interrupt and when the userspace process that gets the notification runs. The 1/2 second delay is used because it is easier than trying to detect when the userspace process has received the notification, at which time it should hold its own wakelock and the mmc subsystem can call __pm_relax(). Hi Colin, I don't have the in-depth knowledge about how the userspace deamons handles the event notifications, so please bare with me while I am trying to understand more here. First of all, are we trying to solve an issue here or just improving some specific situation, that is not clear to me. I might have misunderstood this patch, but it seems like your concern is that you believe the event notification can get lost - if userspace are about to trigger a suspend while a card is being inserted/removed. If that is the case, could you elaborate on what level the notification can get lost? Kind regards Ulf Hansson This is a generic requirement for using a kernel with autosleep enabled. Autosleep will enter suspend whenever there is no wakeup source/wakelock held. Consider the following sequence: Kernel is suspended Card is inserted, triggering a wakeup interrupt, which is an implicit wakeup source until it is handled I don't think a card insert/remove irq need to be configured as a wakeup interrupt. As you say, it will force a resume to detect the card, but for what reason? Instead, I think it it better to leave the card detection to be handled at the next resume, thus not resuming the system when not needed. Kernel starts resuming, resumes the mmc driver The mmc driver enables its interrupt, which is immediately handled and queues an event to be handled by userspace At this point the wakeup interrupt is handled and gone, and no wakeup sources are being held, so the kernel can choose to go back to suspend, so userspace can't handle the insertion event until the kernel wakes up for another reason. Is this a problem? From my point of view it should be perfectly acceptable to let userspace handle the event at the next resume/wakeup instead. Don't you think so? In general, an event that is triggered by a wakeup interrupt that is being passed from the kernel to userspace needs to have a wakeup source held while the event is queued. That's sounds reasonable. Would it then make sense to hold a generic wakeup source in the suspend/resume core, once a wakeup interrupt is fetched? Kind regards Ulf Hansson -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] mmc: Enable wakeup_sources for mmc core
On 18 June 2013 19:15, Colin Cross ccr...@android.com wrote: On Tue, Jun 18, 2013 at 6:17 AM, Ulf Hansson ulf.hans...@linaro.org wrote: On 17 June 2013 20:33, Colin Cross ccr...@android.com wrote: This is a generic requirement for using a kernel with autosleep enabled. Autosleep will enter suspend whenever there is no wakeup source/wakelock held. Consider the following sequence: Kernel is suspended Card is inserted, triggering a wakeup interrupt, which is an implicit wakeup source until it is handled I don't think a card insert/remove irq need to be configured as a wakeup interrupt. As you say, it will force a resume to detect the card, but for what reason? Instead, I think it it better to leave the card detection to be handled at the next resume, thus not resuming the system when not needed. That decision is going to be different on each device. On Android devices it has been configured as a wakeup interrupt. This patch is necessary on devices that want to handle the event as a wakeup event, and has negligible impact on those that do not. Kernel starts resuming, resumes the mmc driver The mmc driver enables its interrupt, which is immediately handled and queues an event to be handled by userspace At this point the wakeup interrupt is handled and gone, and no wakeup sources are being held, so the kernel can choose to go back to suspend, so userspace can't handle the insertion event until the kernel wakes up for another reason. Is this a problem? From my point of view it should be perfectly acceptable to let userspace handle the event at the next resume/wakeup instead. Don't you think so? Depends what userspace is doing. If userspace would like to provide the user some feedback on the event, then it has to be a wakeup interrupt, and it has to hold a wakelock until it has passed the event to userspace. It seems like a bad idea that an insertion of an SD card should trigger the display to be light up. That is indirectly in principle what you suggest should happen from user space once a new SD card is found. Right? I have been working with Android for several years, we never used this kind of setup. Instead we wait for the user to press the display on button. At that time the confirmation will be received. Not saying that this is the only way of doing it, but it seems to be an accepted solution for all our customers. I agree to that this patch should have negligible impact though - if we get things right. I will try to review the code in more detail soon. Kind regards Ulf Hansson In general, an event that is triggered by a wakeup interrupt that is being passed from the kernel to userspace needs to have a wakeup source held while the event is queued. That's sounds reasonable. Would it then make sense to hold a generic wakeup source in the suspend/resume core, once a wakeup interrupt is fetched? No, the suspend/resume core can only hold a wakeup source until it has handed the event off to the driver, at which point it is the driver's responsibility to hold a wakeup source. The suspend/resume core cannot tell if the event was handled by the driver or will be passed to userspace. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Async runtime put in __device_release_driver()
On 23 October 2013 12:11, Tomi Valkeinen tomi.valkei...@ti.com wrote: Hi, I was debugging why clocks were left enabled after removing omapdss driver, and I found this commit: fa180eb448fa263cf18dd930143b515d27d70d7b (PM / Runtime: Idle devices asynchronously after probe|release) I don't understand how that is supposed to work. When a driver is removed, instead of using pm_runtime_put_sync() the commit uses pm_runtime_put(), so the runtime_suspend call is queued. But who is going to handle the queued suspend call, as the driver is already removed? At least in my case, obviously nobody, as I only get runtime_resume call in my driver, never the runtime_suspend. Is there something I need to add to my driver to make this work, or should that part of the patch be reverted? I believe it is quite common that a device driver calls pm_runtime_get_sync as a part of it's remove callback, then it explicitly returns it's resources that has been fetched during probe. Like a clk_disable_unprepare for example. The idea behind the change in __device_release_driver, was to try to prevent devices from going active-idle-active and instead just remain active (if possible). In your case, which seems like a more modern way of implementing remove, you shall call pm_runtime_suspend to make sure the runtime_suspend callbacks gets called. Kind regards Ulf Hansson Tomi -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Async runtime put in __device_release_driver()
Rafael J. Wysocki r...@rjwysocki.net skrev: On Wednesday, November 06, 2013 05:02:12 PM Alan Stern wrote: On Wed, 6 Nov 2013, Rafael J. Wysocki wrote: On Wednesday, November 06, 2013 09:51:42 AM Tomi Valkeinen wrote: On 2013-11-05 23:29, Ulf Hansson wrote: On 23 October 2013 12:11, Tomi Valkeinen tomi.valkei...@ti.com wrote: Hi, I was debugging why clocks were left enabled after removing omapdss driver, and I found this commit: fa180eb448fa263cf18dd930143b515d27d70d7b (PM / Runtime: Idle devices asynchronously after probe|release) I don't understand how that is supposed to work. When a driver is removed, instead of using pm_runtime_put_sync() the commit uses pm_runtime_put(), so the runtime_suspend call is queued. But who is going to handle the queued suspend call, as the driver is already removed? At least in my case, obviously nobody, as I only get runtime_resume call in my driver, never the runtime_suspend. Is there something I need to add to my driver to make this work, or should that part of the patch be reverted? I believe it is quite common that a device driver calls pm_runtime_get_sync as a part of it's remove callback, then it explicitly returns it's resources that has been fetched during probe. Like a clk_disable_unprepare for example. I guess you mean the driver calls pm_runtime_get_sync _and_ pm_runtime_put_sync as part of its remove callback? Probably bus drivers need to do that, but for memory mapped devices in a SoC, I don't think there's normally any need to do pm_runtime_get/put_sync during the remove callback. The idea behind the change in __device_release_driver, was to try to prevent devices from going active-idle-active and instead just remain active (if possible). In your case, which seems like a more modern way of implementing remove, you shall call pm_runtime_suspend to make sure the runtime_suspend callbacks gets called. And as far as I understand, the change creates an explicit requirement to do either pm_runtime_get/put_sync or pm_runtime_suspend inside driver's remove callback. If so, that should be mentioned in big red letters in the pm-runtime documentation. The runtime_pm.txt doc does mention something related to this (and btw, the doc says pm_runtime_put_sync is being called, which is no longer true), but nothing clear about how the driver remove callback must be implemented. That's correct. I tried grepping the kernel sources to find out if pm_runtime_suspend is widely used to get SoC platform devices to suspend, but it doesn't seem like it is. I didn't see pm_runtime_get/put_sync being used in remove callbacks widely either, but that was more difficult one to grep. I think your observations are valid, which unfortunately means that we'll need to revert the commit in question, because it has changed the behavior that drivers are perfectly fine to expect given the existing documentation etc. It looks like the change was premature at least. Greg, I wonder if you can queue up a revert of fa180eb448fa for 3.13, or do you want me to do that? Would it be better to leave the runtime-idle callbacks (invoked during probe) async and revert only the change to __device_release_driver()? Having an async callback after probe shouldn't cause problems, because the driver will then be bound (assuming the probe succeeded). Right. OK, I'll prepare a patch. That seems like a good way forward. Also I appoligize for not updating the doc as part of the original patch. Kind regards Ulf Hansson Thanks, Rafael -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Async runtime put in __device_release_driver()
On 7 November 2013 02:05, Rafael J. Wysocki r...@rjwysocki.net wrote: On Wednesday, November 06, 2013 04:21:48 PM Kevin Hilman wrote: On Wed, Nov 6, 2013 at 4:16 PM, Rafael J. Wysocki r...@rjwysocki.net wrote: On Wednesday, November 06, 2013 11:48:24 PM Ulf Hansson wrote: Rafael J. Wysocki r...@rjwysocki.net skrev: On Wednesday, November 06, 2013 05:02:12 PM Alan Stern wrote: On Wed, 6 Nov 2013, Rafael J. Wysocki wrote: On Wednesday, November 06, 2013 09:51:42 AM Tomi Valkeinen wrote: On 2013-11-05 23:29, Ulf Hansson wrote: On 23 October 2013 12:11, Tomi Valkeinen tomi.valkei...@ti.com wrote: Hi, I was debugging why clocks were left enabled after removing omapdss driver, and I found this commit: fa180eb448fa263cf18dd930143b515d27d70d7b (PM / Runtime: Idle devices asynchronously after probe|release) I don't understand how that is supposed to work. When a driver is removed, instead of using pm_runtime_put_sync() the commit uses pm_runtime_put(), so the runtime_suspend call is queued. But who is going to handle the queued suspend call, as the driver is already removed? At least in my case, obviously nobody, as I only get runtime_resume call in my driver, never the runtime_suspend. Is there something I need to add to my driver to make this work, or should that part of the patch be reverted? I believe it is quite common that a device driver calls pm_runtime_get_sync as a part of it's remove callback, then it explicitly returns it's resources that has been fetched during probe. Like a clk_disable_unprepare for example. I guess you mean the driver calls pm_runtime_get_sync _and_ pm_runtime_put_sync as part of its remove callback? Probably bus drivers need to do that, but for memory mapped devices in a SoC, I don't think there's normally any need to do pm_runtime_get/put_sync during the remove callback. The idea behind the change in __device_release_driver, was to try to prevent devices from going active-idle-active and instead just remain active (if possible). In your case, which seems like a more modern way of implementing remove, you shall call pm_runtime_suspend to make sure the runtime_suspend callbacks gets called. And as far as I understand, the change creates an explicit requirement to do either pm_runtime_get/put_sync or pm_runtime_suspend inside driver's remove callback. If so, that should be mentioned in big red letters in the pm-runtime documentation. The runtime_pm.txt doc does mention something related to this (and btw, the doc says pm_runtime_put_sync is being called, which is no longer true), but nothing clear about how the driver remove callback must be implemented. That's correct. I tried grepping the kernel sources to find out if pm_runtime_suspend is widely used to get SoC platform devices to suspend, but it doesn't seem like it is. I didn't see pm_runtime_get/put_sync being used in remove callbacks widely either, but that was more difficult one to grep. I think your observations are valid, which unfortunately means that we'll need to revert the commit in question, because it has changed the behavior that drivers are perfectly fine to expect given the existing documentation etc. It looks like the change was premature at least. Greg, I wonder if you can queue up a revert of fa180eb448fa for 3.13, or do you want me to do that? Would it be better to leave the runtime-idle callbacks (invoked during probe) async and revert only the change to __device_release_driver()? Having an async callback after probe shouldn't cause problems, because the driver will then be bound (assuming the probe succeeded). Right. OK, I'll prepare a patch. That seems like a good way forward. There you go. --- From: Rafael J. Wysocki rafael.j.wyso...@intel.com Subject: PM / runtime: Use pm_runtime_put_sync() in __device_release_driver() Commit fa180eb448fa (PM / Runtime: Idle devices asynchronously after probe|release) modified __device_release_driver() to call pm_runtime_put(dev) instead of pm_runtime_put_sync(dev) before detaching the driver from the device. However, that was a mistake, because pm_runtime_put(dev) causes rpm_idle() to be queued up and the driver may be gone already when that function is executed. That breaks the assumptions the drivers have the right to make about the core's behavior on the basis of the existing documentation and actually causes problems to happen, so revert that part of commit fa180eb448fa and restore the previous behavior of __device_release_driver(). Reported-by: Tomi Valkeinen tomi.valkei...@ti.com
Re: [PATCH 3/7] ARM: ux500: Remove ATAG support for SDI (MMC)
|
- MMC_CAP_8_BIT_DATA |
- MMC_CAP_NONREMOVABLE |
- MMC_CAP_MMC_HIGHSPEED |
- MMC_CAP_ERASE |
- MMC_CAP_CMD23,
Dito
- .gpio_cd= -1,
- .gpio_wp= -1,
-#ifdef CONFIG_STE_DMA40
- .dma_filter = stedma40_filter,
- .dma_rx_param = mop500_sdi2_dma_cfg_rx,
- .dma_tx_param = mop500_sdi2_dma_cfg_tx,
-#endif
-};
-
-/*
- * SDI 4 (on-board eMMC)
- */
-
-#ifdef CONFIG_STE_DMA40
-struct stedma40_chan_cfg mop500_sdi4_dma_cfg_rx = {
- .mode = STEDMA40_MODE_LOGICAL,
- .dir = DMA_DEV_TO_MEM,
- .dev_type = DB8500_DMA_DEV42_SD_MM4,
-};
-
-static struct stedma40_chan_cfg mop500_sdi4_dma_cfg_tx = {
- .mode = STEDMA40_MODE_LOGICAL,
- .dir = DMA_MEM_TO_DEV,
- .dev_type = DB8500_DMA_DEV42_SD_MM4,
-};
-#endif
-
-struct mmci_platform_data mop500_sdi4_data = {
- .f_max = 1,
- .capabilities = MMC_CAP_4_BIT_DATA |
- MMC_CAP_8_BIT_DATA |
- MMC_CAP_NONREMOVABLE |
- MMC_CAP_MMC_HIGHSPEED |
- MMC_CAP_ERASE |
- MMC_CAP_CMD23,
Dito
- .gpio_cd= -1,
- .gpio_wp= -1,
-#ifdef CONFIG_STE_DMA40
- .dma_filter = stedma40_filter,
- .dma_rx_param = mop500_sdi4_dma_cfg_rx,
- .dma_tx_param = mop500_sdi4_dma_cfg_tx,
-#endif
-};
diff --git a/arch/arm/mach-ux500/board-mop500.h
b/arch/arm/mach-ux500/board-mop500.h
index d48e866..78861f6 100644
--- a/arch/arm/mach-ux500/board-mop500.h
+++ b/arch/arm/mach-ux500/board-mop500.h
@@ -79,10 +79,6 @@
#define SNOWBALL_EN_3V3_ETH_GPIO MOP500_AB8500_PIN_GPIO(26) /*
GPIO26 */
struct device;
-extern struct mmci_platform_data mop500_sdi0_data;
-extern struct mmci_platform_data mop500_sdi1_data;
-extern struct mmci_platform_data mop500_sdi2_data;
-extern struct mmci_platform_data mop500_sdi4_data;
extern struct msp_i2s_platform_data msp0_platform_data;
extern struct msp_i2s_platform_data msp1_platform_data;
extern struct msp_i2s_platform_data msp2_platform_data;
--
1.8.1.2
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Kind regards
Ulf Hansson
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mmc: atmel-mci: add vmmc-supply support
On 16 October 2013 16:32, Alexandre Belloni
alexandre.bell...@free-electrons.com wrote:
Other MMC hosts handle a regulator named vmmc-supply that allows to power the
MMC card or SDIO device before communicating on the bus.
Signed-off-by: Alexandre Belloni alexandre.bell...@free-electrons.com
---
drivers/mmc/host/atmel-mci.c | 12
1 file changed, 12 insertions(+)
diff --git a/drivers/mmc/host/atmel-mci.c b/drivers/mmc/host/atmel-mci.c
index 69e438e..deffa1f 100644
--- a/drivers/mmc/host/atmel-mci.c
+++ b/drivers/mmc/host/atmel-mci.c
@@ -23,6 +23,7 @@
#include linux/of_device.h
#include linux/of_gpio.h
#include linux/platform_device.h
+#include linux/regulator/consumer.h
#include linux/scatterlist.h
#include linux/seq_file.h
#include linux/slab.h
@@ -2355,6 +2356,7 @@ static int __init atmci_probe(struct platform_device
*pdev)
struct mci_platform_data*pdata;
struct atmel_mci*host;
struct resource *regs;
+ struct regulator*reg_vmmc;
unsigned intnr_slots;
int irq;
int ret;
@@ -2371,6 +2373,16 @@ static int __init atmci_probe(struct platform_device
*pdev)
}
}
+ reg_vmmc = devm_regulator_get(pdev-dev, vmmc);
Use mmc_regulator_get_supply instead. This will also try to setup
the ocr_avail mask, which is likely what you need as well.
Kind regards
Ulf Hansson
+ if (!IS_ERR(reg_vmmc)) {
+ ret = regulator_enable(reg_vmmc);
+ if (ret) {
+ dev_err(pdev-dev,
+ Failed to enable vmmc regulator: %d\n, ret);
+ return ret;
+ }
+ }
+
irq = platform_get_irq(pdev, 0);
if (irq 0)
return irq;
--
1.8.1.2
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] MMC: Detect execution mode errors after r/w command
. -*/ - } while (!(status R1_READY_FOR_DATA) || -(R1_CURRENT_STATE(status) == R1_STATE_PRG)); + } } /* if general error occurs, retry the write operation. */ -- 1.8.1.5 Kind regards Ulf Hansson -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv2] mmc: atmel-mci: add vmmc-supply support
On 17 October 2013 00:19, Alexandre Belloni
alexandre.bell...@free-electrons.com wrote:
Other MMC hosts handle a regulator named vmmc-supply that allows to power the
MMC card or SDIO device before communicating on the bus.
Signed-off-by: Alexandre Belloni alexandre.bell...@free-electrons.com
---
Changes in v2:
- use mmc_regulator_get_supply instead of devm_regulator_get
drivers/mmc/host/atmel-mci.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/mmc/host/atmel-mci.c b/drivers/mmc/host/atmel-mci.c
index 69e438e..4ea5333 100644
--- a/drivers/mmc/host/atmel-mci.c
+++ b/drivers/mmc/host/atmel-mci.c
@@ -23,6 +23,7 @@
#include linux/of_device.h
#include linux/of_gpio.h
#include linux/platform_device.h
+#include linux/regulator/consumer.h
#include linux/scatterlist.h
#include linux/seq_file.h
#include linux/slab.h
@@ -2198,6 +2199,14 @@ static int __init atmci_init_slot(struct atmel_mci
*host,
host-slot[id] = slot;
mmc_add_host(mmc);
+ mmc_regulator_get_supply(mmc);
+ if (!IS_ERR(mmc-supply.vmmc)) {
+ int ret = regulator_enable(mmc-supply.vmmc);
You should not enable the regulator here, instead handle that from the
.set_ios function. This complete code chunk must also be moved prior
to mmc_add_host above.
When enabling/disabling the regulator from the .set_ios, I suppose you
want to use mmc_regulator_set_ocr API instead.
Kind regards
Ulf Hansson
+ if (ret)
+ dev_err(mmc-class_dev,
+ failed to enable regulator: %d\n, ret);
+ }
+
if (gpio_is_valid(slot-detect_pin)) {
int ret;
--
1.8.1.2
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv3] mmc: atmel-mci: add vmmc-supply support
On 17 October 2013 12:46, Alexandre Belloni
alexandre.bell...@free-electrons.com wrote:
Other MMC hosts handle a regulator named vmmc-supply that allows to power the
MMC card or SDIO device before communicating on the bus.
Signed-off-by: Alexandre Belloni alexandre.bell...@free-electrons.com
Acked-by: Ulf Hansson ulf.hans...@linaro.org
---
Changes in v2:
- use mmc_regulator_get_supply instead of devm_regulator_get
Changes in v3:
- en/disable the regulator in .set_ios using mmc_regulator_set_ocr
drivers/mmc/host/atmel-mci.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/mmc/host/atmel-mci.c b/drivers/mmc/host/atmel-mci.c
index 69e438e..a9e1ba6 100644
--- a/drivers/mmc/host/atmel-mci.c
+++ b/drivers/mmc/host/atmel-mci.c
@@ -1385,8 +1385,14 @@ static void atmci_set_ios(struct mmc_host *mmc, struct
mmc_ios *ios)
clk_unprepare(host-mck);
switch (ios-power_mode) {
+ case MMC_POWER_OFF:
+ if (!IS_ERR(mmc-supply.vmmc))
+ mmc_regulator_set_ocr(mmc, mmc-supply.vmmc, 0);
+ break;
case MMC_POWER_UP:
set_bit(ATMCI_CARD_NEED_INIT, slot-flags);
+ if (!IS_ERR(mmc-supply.vmmc))
+ mmc_regulator_set_ocr(mmc, mmc-supply.vmmc,
ios-vdd);
break;
default:
/*
@@ -2196,6 +2202,7 @@ static int __init atmci_init_slot(struct atmel_mci
*host,
}
host-slot[id] = slot;
+ mmc_regulator_get_supply(mmc);
mmc_add_host(mmc);
if (gpio_is_valid(slot-detect_pin)) {
--
1.8.1.2
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] i2c-mux-pca954x: Disable mux after 200ms timeout
On 26 November 2013 13:28, Wolfram Sang w...@the-dreams.de wrote: CCing linux-pm, maybe they know more... The extra I2C traffic consumes extra power. If the bus is terminated using 2k resistors, approximately 1mA of current (assuming ~2V signals) is flowing when the bus is pulled low. On low power designs, this extra power consumption is noticable. There is no way to turn the mux off from either user or kernel space. The signals going through the mux to a place where no I2C device is actually listening are only wasting power. I only have an overview of current linux pm mechanisms. I wonder if those can't be used somehow. Like if devices on the multiplexed bus are idle (well, only regarding transfers), then we can switch off the muxer. The I2C signals are used to control sensitive codecs. When looking at the sampled data, the I2C traffic is visible in the captured analog signal. To prevent this from happening, the mux can be I wonder: Is this really a feature of sensitive codecs or an issue of the board design? disabled whenever not communicating with the codec. This could be accomplished with the deselect_on_exit boolean, but because audio driver sends the codec parameters in dozens of small transactions, this will result in a lot more needless I2C traffic, constantly switching the mux for each codec setting. Has this been looked at? ASoC supports grouping of tranfers IIRC. Maybe your I2C driver is only missing I2C_M_NOSTART?. Would it be acceptable if I made the timeout optional, by making the deselect_on_exit boolean into a three-state value (off, on, timeout)? Or, alternatively, implement deselect_on_exit using the timeout scheme (probably with a very short timeout)? I have a number of concerns designwise. First, if we do something like shutting-down-a-bus-if-there-are-no-transfers-expected, it definately should be in the core, not the driver. As said before, I have the assumption it should even be connected to the runtime pm core via some callback. And if we have that for I2C, we surely want that for other buses as well, at least SPI. Also, the timeout thing sounds too heuristic to me. Later, people might want to change the timeout value depending on workloads or so, and then a governor, etc... No, thanks. It very much sounds like runtime PM should help you here. Then you get the reference counting and the so called runtime_autosuspend feature for free. :-) BTW is it feasible to shut down the whole I2C bus at controller level after transfers? No needless transfers and maybe even more power savings. For sure this should be possible. Some controller drivers have started to adapt some runtime PM code for this is already, nomadik and omap for example. I think typically clocks, pins and if there are a power domain regulator for the controller, those should be handled through runtime PM to save power at request inactivity. Kind regards Ulf Hansson Anyway, thanks for letting me know about your requirements (they should have been in the original commit message, though ;)) -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 5/5] drm/exynos: Convert to suspend_late/resume_early callbacks for fimc
To simplify code for system suspend, convert the .suspend|resume
callbacks into .suspend_late|resume_early. In general this could be
convenient for any driver that supports both system PM and runtime PM.
Move the runtime PM callbacks to be implemented within CONFIG_PM, to
make them available for system PM.
Use pm_generic_suspend_late_runtime and pm_generic_resume_early_runtime
as the new callbacks.
Cc: David Airlie airl...@linux.ie
Cc: Kukjin Kim kgene@samsung.com
Cc: linux-samsung-...@vger.kernel.org
Cc: Kevin Hilman khil...@linaro.org
Cc: Alan Stern st...@rowland.harvard.edu
Cc: Rafael J. Wysocki r...@rjwysocki.net
Cc: Pavel Machek pa...@ucw.cz
Cc: Len Brown len.br...@intel.com
Signed-off-by: Ulf Hansson ulf.hans...@linaro.org
---
drivers/gpu/drm/exynos/exynos_drm_fimc.c | 33 --
1 file changed, 4 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
index 8adfc8f..7aa17a3 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
@@ -1888,33 +1888,7 @@ static int fimc_remove(struct platform_device *pdev)
return 0;
}
-#ifdef CONFIG_PM_SLEEP
-static int fimc_suspend(struct device *dev)
-{
- struct fimc_context *ctx = get_fimc_context(dev);
-
- DRM_DEBUG_KMS(id[%d]\n, ctx-id);
-
- if (pm_runtime_suspended(dev))
- return 0;
-
- return fimc_clk_ctrl(ctx, false);
-}
-
-static int fimc_resume(struct device *dev)
-{
- struct fimc_context *ctx = get_fimc_context(dev);
-
- DRM_DEBUG_KMS(id[%d]\n, ctx-id);
-
- if (!pm_runtime_suspended(dev))
- return fimc_clk_ctrl(ctx, true);
-
- return 0;
-}
-#endif
-
-#ifdef CONFIG_PM_RUNTIME
+#ifdef CONFIG_PM
static int fimc_runtime_suspend(struct device *dev)
{
struct fimc_context *ctx = get_fimc_context(dev);
@@ -1935,8 +1909,9 @@ static int fimc_runtime_resume(struct device *dev)
#endif
static const struct dev_pm_ops fimc_pm_ops = {
- SET_SYSTEM_SLEEP_PM_OPS(fimc_suspend, fimc_resume)
- SET_RUNTIME_PM_OPS(fimc_runtime_suspend, fimc_runtime_resume, NULL)
+ SET_LATE_SYSTEM_SLEEP_PM_OPS(pm_generic_suspend_late_runtime,
+ pm_generic_resume_early_runtime)
+ SET_PM_RUNTIME_PM_OPS(fimc_runtime_suspend, fimc_runtime_resume, NULL)
};
static const struct of_device_id fimc_of_match[] = {
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 4/5] PM / Sleep: Add macro to define common late/early system PM callbacks
We use the same approach as for the existing SET_SYSTEM_SLEEP_PM_OPS,
but for the late and early callbacks instead.
The new SET_LATE_SYSTEM_SLEEP_PM_OPS, defined for CONFIG_PM_SLEEP, will
point -suspend_late, -freeze_late and -poweroff_late to the same
function. Vice verse happens for -resume_early, -thaw_early and
-restore_early.
Cc: Kevin Hilman khil...@linaro.org
Cc: Alan Stern st...@rowland.harvard.edu
Signed-off-by: Ulf Hansson ulf.hans...@linaro.org
---
include/linux/pm.h | 12
1 file changed, 12 insertions(+)
diff --git a/include/linux/pm.h b/include/linux/pm.h
index 529657c..fb0f1db 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -311,6 +311,18 @@ struct dev_pm_ops {
#define SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn)
#endif
+#ifdef CONFIG_PM_SLEEP
+#define SET_LATE_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
+ .suspend_late = suspend_fn, \
+ .resume_early = resume_fn, \
+ .freeze_late = suspend_fn, \
+ .thaw_early = resume_fn, \
+ .poweroff_late = suspend_fn, \
+ .restore_early = resume_fn,
+#else
+#define SET_LATE_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn)
+#endif
+
#ifdef CONFIG_PM_RUNTIME
#define SET_RUNTIME_PM_OPS(suspend_fn, resume_fn, idle_fn) \
.runtime_suspend = suspend_fn, \
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/5] PM / Sleep: Add pm_generic functions to re-use runtime PM callbacks
To put devices into low power state during sleep, it sometimes makes
sense at subsystem-level to re-use the runtime PM callbacks.
The PM core will at device_suspend_late disable runtime PM, after that
we can safely operate on these callbacks. At suspend_late the device
will be put into low power state by invoking the runtime_suspend
callbacks, unless the runtime status is already suspended. At
resume_early the state is restored by invoking the runtime_resume
callbacks. Soon after the PM core will re-enable runtime PM before
returning from device_resume_early.
The new pm_generic functions are named pm_generic_suspend_late_runtime
and pm_generic_resume_early_runtime, they are supposed to be used in
pairs.
Do note that these new generic callbacks will work smothly even with
and without CONFIG_PM_RUNTIME, as long as the runtime PM callbacks are
implemented inside CONFIG_PM instead of CONFIG_PM_RUNTIME.
A special thanks to Alan Stern who came up with this idea.
Cc: Kevin Hilman khil...@linaro.org
Cc: Alan Stern st...@rowland.harvard.edu
Signed-off-by: Ulf Hansson ulf.hans...@linaro.org
---
drivers/base/power/generic_ops.c | 86 ++
include/linux/pm.h |2 +
2 files changed, 88 insertions(+)
diff --git a/drivers/base/power/generic_ops.c b/drivers/base/power/generic_ops.c
index 5ee030a..8e78ad1 100644
--- a/drivers/base/power/generic_ops.c
+++ b/drivers/base/power/generic_ops.c
@@ -93,6 +93,49 @@ int pm_generic_suspend_late(struct device *dev)
EXPORT_SYMBOL_GPL(pm_generic_suspend_late);
/**
+ * pm_generic_suspend_late_runtime - Generic suspend_late callback for drivers
+ * @dev: Device to suspend.
+ * Use to invoke runtime_suspend callbacks at suspend_late.
+ */
+int pm_generic_suspend_late_runtime(struct device *dev)
+{
+ int (*callback)(struct device *);
+ int ret = 0;
+
+ /*
+* PM core has disabled runtime PM in device_suspend_late, thus we can
+* safely check the device's runtime status and decide whether
+* additional actions are needed to put the device into low power state.
+* If so, we invoke the runtime_suspend callbacks.
+* For the !CONFIG_PM_RUNTIME case, pm_runtime_status_suspended() always
+* returns false and therefore the runtime_suspend callback will be
+* invoked.
+*/
+ if (pm_runtime_status_suspended(dev))
+ return 0;
+
+ if (dev-pm_domain)
+ callback = dev-pm_domain-ops.runtime_suspend;
+ else if (dev-type dev-type-pm)
+ callback = dev-type-pm-runtime_suspend;
+ else if (dev-class dev-class-pm)
+ callback = dev-class-pm-runtime_suspend;
+ else if (dev-bus dev-bus-pm)
+ callback = dev-bus-pm-runtime_suspend;
+ else
+ callback = NULL;
+
+ if (!callback dev-driver dev-driver-pm)
+ callback = dev-driver-pm-runtime_suspend;
+
+ if (callback)
+ ret = callback(dev);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(pm_generic_suspend_late_runtime);
+
+/**
* pm_generic_suspend - Generic suspend callback for subsystems.
* @dev: Device to suspend.
*/
@@ -237,6 +280,49 @@ int pm_generic_resume_early(struct device *dev)
EXPORT_SYMBOL_GPL(pm_generic_resume_early);
/**
+ * pm_generic_resume_early_runtime - Generic resume_early callback for drivers
+ * @dev: Device to resume.
+ * Use to invoke runtime_resume callbacks at resume_early.
+ */
+int pm_generic_resume_early_runtime(struct device *dev)
+{
+ int (*callback)(struct device *);
+ int ret = 0;
+
+ /*
+* PM core has not yet enabled runtime PM in device_resume_early,
+* thus we can safely check the device's runtime status and restore the
+* previous state we had in device_suspend_late. If restore is needed
+* we invoke the runtime_resume callbacks.
+* For the !CONFIG_PM_RUNTIME case, pm_runtime_status_suspended() always
+* returns false and therefore the runtime_resume callback will be
+* invoked.
+*/
+ if (pm_runtime_status_suspended(dev))
+ return 0;
+
+ if (dev-pm_domain)
+ callback = dev-pm_domain-ops.runtime_resume;
+ else if (dev-type dev-type-pm)
+ callback = dev-type-pm-runtime_resume;
+ else if (dev-class dev-class-pm)
+ callback = dev-class-pm-runtime_resume;
+ else if (dev-bus dev-bus-pm)
+ callback = dev-bus-pm-runtime_resume;
+ else
+ callback = NULL;
+
+ if (!callback dev-driver dev-driver-pm)
+ callback = dev-driver-pm-runtime_resume;
+
+ if (callback)
+ ret = callback(dev);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(pm_generic_resume_early_runtime);
+
+/**
* pm_generic_resume - Generic resume callback for subsystems.
* @dev: Device to resume.
*/
diff --git a/include/linux/pm.h b
[PATCH 2/5] PM / Runtime: Implement the pm_generic_runtime functions for CONFIG_PM
The pm_generic_runtime_suspend|resume functions were implemented within
CONFIG_PM_RUNTIME.
As we also may use runtime PM callbacks during system suspend, to put
devices into low power state, we need to move the implementation of
pm_generic_runtime_suspend|resume to CONFIG_PM.
This change prevents the platform bus from by-passing driver's runtime
PM callbacks while only CONFIG_PM_SLEEP and not CONFIG_PM_RUNTIME is
being used.
Cc: Kevin Hilman khil...@linaro.org
Cc: Alan Stern st...@rowland.harvard.edu
Signed-off-by: Ulf Hansson ulf.hans...@linaro.org
---
drivers/base/power/generic_ops.c |4 ++--
include/linux/pm_runtime.h | 12
2 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/base/power/generic_ops.c b/drivers/base/power/generic_ops.c
index 8e78ad1..a4c6cb0 100644
--- a/drivers/base/power/generic_ops.c
+++ b/drivers/base/power/generic_ops.c
@@ -10,7 +10,7 @@
#include linux/pm_runtime.h
#include linux/export.h
-#ifdef CONFIG_PM_RUNTIME
+#ifdef CONFIG_PM
/**
* pm_generic_runtime_suspend - Generic runtime suspend callback for
subsystems.
* @dev: Device to suspend.
@@ -48,7 +48,7 @@ int pm_generic_runtime_resume(struct device *dev)
return ret;
}
EXPORT_SYMBOL_GPL(pm_generic_runtime_resume);
-#endif /* CONFIG_PM_RUNTIME */
+#endif /* CONFIG_PM */
#ifdef CONFIG_PM_SLEEP
/**
diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h
index 6fa7cea..16c9a62 100644
--- a/include/linux/pm_runtime.h
+++ b/include/linux/pm_runtime.h
@@ -23,6 +23,14 @@
usage_count */
#define RPM_AUTO 0x08/* Use autosuspend_delay */
+#ifdef CONFIG_PM
+extern int pm_generic_runtime_suspend(struct device *dev);
+extern int pm_generic_runtime_resume(struct device *dev);
+#else
+static inline int pm_generic_runtime_suspend(struct device *dev) { return 0; }
+static inline int pm_generic_runtime_resume(struct device *dev) { return 0; }
+#endif
+
#ifdef CONFIG_PM_RUNTIME
extern struct workqueue_struct *pm_wq;
@@ -37,8 +45,6 @@ extern void pm_runtime_enable(struct device *dev);
extern void __pm_runtime_disable(struct device *dev, bool check_resume);
extern void pm_runtime_allow(struct device *dev);
extern void pm_runtime_forbid(struct device *dev);
-extern int pm_generic_runtime_suspend(struct device *dev);
-extern int pm_generic_runtime_resume(struct device *dev);
extern void pm_runtime_no_callbacks(struct device *dev);
extern void pm_runtime_irq_safe(struct device *dev);
extern void __pm_runtime_use_autosuspend(struct device *dev, bool use);
@@ -142,8 +148,6 @@ static inline bool pm_runtime_active(struct device *dev) {
return true; }
static inline bool pm_runtime_status_suspended(struct device *dev) { return
false; }
static inline bool pm_runtime_enabled(struct device *dev) { return false; }
-static inline int pm_generic_runtime_suspend(struct device *dev) { return 0; }
-static inline int pm_generic_runtime_resume(struct device *dev) { return 0; }
static inline void pm_runtime_no_callbacks(struct device *dev) {}
static inline void pm_runtime_irq_safe(struct device *dev) {}
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 3/5] PM / Runtime: Add second macro for definition of runtime PM callbacks
It is allowed and will in many cases make sense to have the runtime PM
callbacks to be defined for CONFIG_PM instead of CONFIG_PM_RUNTIME.
Since the PM core disables runtime PM during system suspend, before the
.suspend_late callbacks are invoked, drivers could at this point
directly invoke it's runtime PM callbacks or even walk through it's bus
or power domain, to put it's device into low power state.
Use the new macro SET_PM_RUNTIME_PM_OPS in cases were the above makes
sense. Make sure the callbacks are encapsulated within CONFIG_PM
instead of CONFIG_PM_RUNTIME.
Do note that the old macro SET_RUNTIME_PM_OPS, which is being quite
widely used right now, requires the callbacks to be defined for
CONFIG_PM_RUNTIME. In many cases it will certainly be convenient to
convert to the new macro above, but still some cases are optimal for
only CONFIG_PM_RUNTIME.
Cc: Kevin Hilman khil...@linaro.org
Cc: Alan Stern st...@rowland.harvard.edu
Signed-off-by: Ulf Hansson ulf.hans...@linaro.org
---
include/linux/pm.h |9 +
1 file changed, 9 insertions(+)
diff --git a/include/linux/pm.h b/include/linux/pm.h
index 5bce0d4..529657c 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -320,6 +320,15 @@ struct dev_pm_ops {
#define SET_RUNTIME_PM_OPS(suspend_fn, resume_fn, idle_fn)
#endif
+#ifdef CONFIG_PM
+#define SET_PM_RUNTIME_PM_OPS(suspend_fn, resume_fn, idle_fn) \
+ .runtime_suspend = suspend_fn, \
+ .runtime_resume = resume_fn, \
+ .runtime_idle = idle_fn,
+#else
+#define SET_PM_RUNTIME_PM_OPS(suspend_fn, resume_fn, idle_fn)
+#endif
+
/*
* Use this if you want to use the same suspend and resume callbacks for
suspend
* to RAM and hibernation.
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 0/5] PM: Enable option of re-use runtime PM callbacks at system suspend
To put devices into low power state during system suspend, it may be convenient for runtime PM supported subsystems, power domains and drivers to have the option of re-using the runtime PM callbacks. At the moment, quite complex solutions exist for power domains that tries to handle the above, like for OMAP2. The idea here, is to make it possible for drivers, who should know best, how to easiest put their devices into low power state. The intent is thus not only to simplify drivers but also power domains. Additionally, some drivers seems to have messed up things when combining runtime PM with system PM. While we enable the option of re-using the runtime PM callbacks during system PM, we also intend to clarify the way forward for how these scenarios could be resolved. Some new helper macros for defining PM callbacks and two new pm_generic* functions has been implemented in this patch set. These are provided to make it easier for those who wants to enable the option of re-using the runtime PM callbacks during system suspend. A minor fix was needed for the platform bus, which runtime PM callbacks are set to the pm_genric_runtime_suspend|resume functions. These were implemented only for CONFIG_PM_RUNTIME and thus the platform bus prevented driver's runtime PM callbacks to be invoked when only CONFIG_PM_SLEEP was used. We move them into CONFIG_PM to resolve the problem and then let drivers decide how to handle this themselves. Patch 5 is folded into this patch set, to visualize how a driver that uses both runtime PM and system PM, can benefit from re-using the runtime PM callbacks during system suspend. Also note that, patch 1 to 4 has been submitted to linux-pm as standalone patches recently. I thought it make sense to collect them into a patchset since those are closely related. Ulf Hansson (5): PM / Sleep: Add pm_generic functions to re-use runtime PM callbacks PM / Runtime: Implement the pm_generic_runtime functions for CONFIG_PM PM / Runtime: Add second macro for definition of runtime PM callbacks PM / Sleep: Add macro to define common late/early system PM callbacks drm/exynos: Convert to suspend_late/resume_early callbacks for fimc drivers/base/power/generic_ops.c | 90 +- drivers/gpu/drm/exynos/exynos_drm_fimc.c | 33 ++- include/linux/pm.h | 23 include/linux/pm_runtime.h | 12 ++-- 4 files changed, 123 insertions(+), 35 deletions(-) -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/5] PM: Enable option of re-use runtime PM callbacks at system suspend
On 27 November 2013 21:42, Rafael J. Wysocki r...@rjwysocki.net wrote: On Wednesday, November 27, 2013 04:34:55 PM Ulf Hansson wrote: To put devices into low power state during system suspend, it may be convenient for runtime PM supported subsystems, power domains and drivers to have the option of re-using the runtime PM callbacks. At the moment, quite complex solutions exist for power domains that tries to handle the above, like for OMAP2. The idea here, is to make it possible for drivers, who should know best, how to easiest put their devices into low power state. The intent is thus not only to simplify drivers but also power domains. So this is not entirely correct and stems from the fact that you are only considering one platform. Bad wording from my side, sorry. I guess you realize, that am mostly working on ARM SoCs, but that involve many platforms and drivers. In these cases it is quite common that drivers are the best decision makers for PM. It is also very common to have power domain regulators. This combination is almost impossible to support without hacks as of today, especially if you have a more complex situation were the driver is attached to both a bus and a power domain, and were those also are handling runtime PM resources. On some other platforms, like x86 PC for one example, device drivers have no idea how to put their devices into low power states at all, because that depends on what's there in the ACPI tables. This becomes clearly visible when you try to use the same driver on two different platforms that have different board layouts and power configurations. And if one of them uses ACPI by chance, the driver shouldn't really fiddle with its little knobs for clocks and power rails directly. Again, this patch set will only provide the option of being able to re-use runtime PM callbacks during system suspend; could also be considered as fundamental building blocks for those SoCs who needs this. Additionally and important, it wont break nor interfere with any other platforms like x86, which of course is very important. On the other hand, if it makes sense for other platforms to use these new building blocks they have the opportunity to do so. Additionally, some drivers seems to have messed up things when combining runtime PM with system PM. While we enable the option of re-using the runtime PM callbacks during system PM, we also intend to clarify the way forward for how these scenarios could be resolved. Some new helper macros for defining PM callbacks and two new pm_generic* functions has been implemented in this patch set. These are provided to make it easier for those who wants to enable the option of re-using the runtime PM callbacks during system suspend. I'm generally opposed to re-using callbacks like this, because it adds confusion to the picture. It may seem to be clever, but in fact it leads to bad design choices in the drivers in my opinion. In my world of the kernel, it will clearly resolve confusions and simplify a significant amount of code in power domains, buses and drivers. So I guess it depends on from what point you look at this. And, as you stated previously during these discussions, we have the opportunity to update the documentation around this topic, I will happily do it, if needed. Let's talk about specific examples, though. Why exactly do you need what patch [5/5] does in the exynos_drm_fimc driver? This was a simple example, I wanted to visualize how the new building blocks were going to be used. Anyway, this we achieve with the patch: 1. The PM part in the driver becomes simplified, we don't need the wrapper functions for the runtime PM callbacks any more. 2. Previously the driver did not make sure runtime PM was disabled, before it put the device into low power state at .suspend. From a runtime PM point of view, this is not a nice behaviour and opens up for confusions, even if it likely would work in most cases. 3. The power domain runtime PM callbacks were by-passed during system suspend, my patch fixes this. Why do I want this? Because the power domain can have runtime PM resources it need to handle at this phase. Potentially, it could handle that from it's .suspend_late callback instead, but then it gets unnecessary complicated, which is what clearly happened to the power domain for OMAP2, for example. If you want additional proof of concepts, we can have a look at more complex example. Typically I am thinking of cases were a cross SoC driver is attached to a bus and for some SoCs a power domain as well. Then, the bus, the power domain and the driver - all have runtime PM resources to handle. In these cases using the new building blocks will not only significantly simplify code, but also fix immediate bugs. One example are drivers attached to the AMBA bus, at drivers/amba/bus.c. Kind regards Ulf Hansson Rafael -- To unsubscribe from this list: send the line unsubscribe linux-kernel
Re: [PATCH 0/5] PM: Enable option of re-use runtime PM callbacks at system suspend
The lack of specificity here doesn't make the discussion any easier. It usually is better to talk about specific problems to address than using general terms which may mean slightly different things for different people. During these discussions, I have tried to point at existing code for drivers and existing code for power domains. Those which at the moment either have got things wrong or are unnecessary complicated, in regards to their PM implementation. I suppose I could provide some more patches for proof of concept, will that be a way forward? Additionally, some drivers seems to have messed up things when combining runtime PM with system PM. While we enable the option of re-using the runtime PM callbacks during system PM, we also intend to clarify the way forward for how these scenarios could be resolved. Some new helper macros for defining PM callbacks and two new pm_generic* functions has been implemented in this patch set. These are provided to make it easier for those who wants to enable the option of re-using the runtime PM callbacks during system suspend. I'm generally opposed to re-using callbacks like this, because it adds confusion to the picture. It may seem to be clever, but in fact it leads to bad design choices in the drivers in my opinion. In my world of the kernel, it will clearly resolve confusions and simplify a significant amount of code in power domains, buses and drivers. So I guess it depends on from what point you look at this. This is so vague that I don't even know how to respond. :-) So let me say instead that what you did in patch [5/5] is a layering violation which always is a bug, even if it doesn't break things outright. After that patch the driver would call into a layer that is supposed to call into it under normal conditions. Moreover, it would expect that layer to call back into it again in a specific way, which may or may not happen depending on how exactly that layer is implemented. So even if it works today, it will add constraints on how that other layer may be implmented which *is* confusing and wrong in my not so humble opinion. I'll never apply any patches that lead to situations like that, don't even bother to send them to me. Pretty please. After all these good discussions which clearly pointed the solution into this direction, you decide to bring up this argument now? It makes me wonder. Indirectly what you are saying is that, the PM core should at device_prepare, do pm_runtime_disable() instead of pm_runtime_get_noresume(), to prevent drivers/subsystems from triggering layering violations by invoking pm_runtime_get_sync(). Because, this is exactly the same kind of layering violation you refer to while neglecting my approach, which at the moment drivers/subsystems not only are allowed to, but also encouraged to do during system suspend. Now, obviously I don't think we shall change the behaviour of PM core, that would just not be convenient for subsystems and drivers, right? So, the PM core allows layering violations for the .runtime_resume callbacks to be invoked during system suspend. It can do so, because it trust drivers/subsystems to act responsibly and to what suites them best. For the same reasons, I believe we should trust drivers/subsystems, to understand when it makes sense for them to re-use all of the runtime PM callbacks during system suspend and not just the .runtime_suspend callback. That is in principle what I and Alan, who came up with this idea, are suggesting. And, as you stated previously during these discussions, we have the opportunity to update the documentation around this topic, I will happily do it, if needed. That's always welcome. :-) Let's talk about specific examples, though. Why exactly do you need what patch [5/5] does in the exynos_drm_fimc driver? This was a simple example, I wanted to visualize how the new building blocks were going to be used. Anyway, this we achieve with the patch: 1. The PM part in the driver becomes simplified, we don't need the wrapper functions for the runtime PM callbacks any more. No, it is not simplified. It becomes *far* more complicated conceptually instead, although that is hidden by moving the complexity into the functions added by patch [1/5]. So whoever doesn't look into those functions will not actually realize how complicated the code really is. 2. Previously the driver did not make sure runtime PM was disabled, before it put the device into low power state at .suspend. From a runtime PM point of view, this is not a nice behaviour and opens up for confusions, even if it likely would work in most cases. So the proper fix, in my opinion, would be to point .suspend_late and .resume_early in that driver to fimc_suspend() and fimc_resume(), respectively, and leave the .suspend and .resume pointers unpopulated. Wouldn't that actually work? If we decide to ignore the
Re: [PATCH 0/5] PM: Enable option of re-use runtime PM callbacks at system suspend
On 29 November 2013 10:32, Ulf Hansson ulf.hans...@linaro.org wrote: The lack of specificity here doesn't make the discussion any easier. It usually is better to talk about specific problems to address than using general terms which may mean slightly different things for different people. During these discussions, I have tried to point at existing code for drivers and existing code for power domains. Those which at the moment either have got things wrong or are unnecessary complicated, in regards to their PM implementation. I suppose I could provide some more patches for proof of concept, will that be a way forward? Additionally, some drivers seems to have messed up things when combining runtime PM with system PM. While we enable the option of re-using the runtime PM callbacks during system PM, we also intend to clarify the way forward for how these scenarios could be resolved. Some new helper macros for defining PM callbacks and two new pm_generic* functions has been implemented in this patch set. These are provided to make it easier for those who wants to enable the option of re-using the runtime PM callbacks during system suspend. I'm generally opposed to re-using callbacks like this, because it adds confusion to the picture. It may seem to be clever, but in fact it leads to bad design choices in the drivers in my opinion. In my world of the kernel, it will clearly resolve confusions and simplify a significant amount of code in power domains, buses and drivers. So I guess it depends on from what point you look at this. This is so vague that I don't even know how to respond. :-) So let me say instead that what you did in patch [5/5] is a layering violation which always is a bug, even if it doesn't break things outright. After that patch the driver would call into a layer that is supposed to call into it under normal conditions. Moreover, it would expect that layer to call back into it again in a specific way, which may or may not happen depending on how exactly that layer is implemented. So even if it works today, it will add constraints on how that other layer may be implmented which *is* confusing and wrong in my not so humble opinion. I'll never apply any patches that lead to situations like that, don't even bother to send them to me. Pretty please. After all these good discussions which clearly pointed the solution into this direction, you decide to bring up this argument now? It makes me wonder. Indirectly what you are saying is that, the PM core should at device_prepare, do pm_runtime_disable() instead of pm_runtime_get_noresume(), to prevent drivers/subsystems from triggering layering violations by invoking pm_runtime_get_sync(). Because, this is exactly the same kind of layering violation you refer to while neglecting my approach, which at the moment drivers/subsystems not only are allowed to, but also encouraged to do during system suspend. Now, obviously I don't think we shall change the behaviour of PM core, that would just not be convenient for subsystems and drivers, right? So, the PM core allows layering violations for the .runtime_resume callbacks to be invoked during system suspend. It can do so, because it trust drivers/subsystems to act responsibly and to what suites them best. For the same reasons, I believe we should trust drivers/subsystems, to understand when it makes sense for them to re-use all of the runtime PM callbacks during system suspend and not just the .runtime_suspend callback. Sorry, another typo: not just the .runtime_suspend - not just the .runtime_resume. That is in principle what I and Alan, who came up with this idea, are suggesting. And, as you stated previously during these discussions, we have the opportunity to update the documentation around this topic, I will happily do it, if needed. That's always welcome. :-) Let's talk about specific examples, though. Why exactly do you need what patch [5/5] does in the exynos_drm_fimc driver? This was a simple example, I wanted to visualize how the new building blocks were going to be used. Anyway, this we achieve with the patch: 1. The PM part in the driver becomes simplified, we don't need the wrapper functions for the runtime PM callbacks any more. No, it is not simplified. It becomes *far* more complicated conceptually instead, although that is hidden by moving the complexity into the functions added by patch [1/5]. So whoever doesn't look into those functions will not actually realize how complicated the code really is. 2. Previously the driver did not make sure runtime PM was disabled, before it put the device into low power state at .suspend. From a runtime PM point of view, this is not a nice behaviour and opens up for confusions, even if it likely would work in most cases. So the proper fix, in my opinion, would be to point .suspend_late
Re: [PATCH 0/5] PM: Enable option of re-use runtime PM callbacks at system suspend
specific examples, though. Why exactly do you need what patch [5/5] does in the exynos_drm_fimc driver? This was a simple example, I wanted to visualize how the new building blocks were going to be used. Anyway, this we achieve with the patch: 1. The PM part in the driver becomes simplified, we don't need the wrapper functions for the runtime PM callbacks any more. No, it is not simplified. It becomes *far* more complicated conceptually instead, although that is hidden by moving the complexity into the functions added by patch [1/5]. So whoever doesn't look into those functions will not actually realize how complicated the code really is. 2. Previously the driver did not make sure runtime PM was disabled, before it put the device into low power state at .suspend. From a runtime PM point of view, this is not a nice behaviour and opens up for confusions, even if it likely would work in most cases. So the proper fix, in my opinion, would be to point .suspend_late and .resume_early in that driver to fimc_suspend() and fimc_resume(), respectively, and leave the .suspend and .resume pointers unpopulated. Wouldn't that actually work? If we decide to ignore the power domain issue below, yes. 3. The power domain runtime PM callbacks were by-passed during system suspend, my patch fixes this. I don't exactly understand this. Why would they be bypassed? Why do I want this? Because the power domain can have runtime PM resources it need to handle at this phase. Potentially, it could handle that from it's .suspend_late callback instead, but then it gets unnecessary complicated, which is what clearly happened to the power domain for OMAP2, for example. I'd like to understand this. What exactly do you mean by unnecessary complicated? Please, have a deeper look into the OMAP power domain implementation. What files exactly do I need to look at? Start with this: arch/arm/mach-omap2/omap_device.c If each SoC (for those that have power domain regulators) needs their own version of such a power domain, then I certainly think it is more complicated that in needs to be. They surely can share implementations. If you want additional proof of concepts, we can have a look at more complex example. Typically I am thinking of cases were a cross SoC driver is attached to a bus and for some SoCs a power domain as well. Then, the bus, the power domain and the driver - all have runtime PM resources to handle. Sure. OK, I consider resending the patch set, including some additional proof of concept patches. In these cases using the new building blocks will not only significantly simplify code, but also fix immediate bugs. One example are drivers attached to the AMBA bus, at drivers/amba/bus.c. Again, what drivers and what's the bug you're talking about? I will use some of these as examples, it will be more visible to you then. Well, I think I know what you're trying to do. The problem is that in my opinion the way you're trying to achieve that is not the right one. I am not so sure I have managed to describe the scenarios in detail which I am trying to solve. Hopefully the information in this reply adds some more insight though. It sounds like you think the direction of where my solution is heading is totally wrong. I would certainly appreciate some advise from you at this point, because I am kind of out ideas at the moment. Kind regards Ulf Hansson Thanks! -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/5] PM: Enable option of re-use runtime PM callbacks at system suspend
That would be kind of OK, if the driver's .suspend_late() only invoked its own .runtime_suspend(), what you did below. But, in the Ulf's approach the driver calls .runtime_suspend() from its *subsystem* in the hope that it will all work out properly (or perhaps based on the knowledge about this particular subsystem). Correct assumption. As I said, that may lead to problems when the same driver is attempted to be used with a different subsystem-level code (for example, a different PM domain). I don't think this is an issue we need to consider. I assume the reason for having a PM domain is typically because you have a PM domain regulator to handle. Certainly the PM domain, controls it by using the runtime PM callbacks. Thus it does not make sense to bypass the PM domain callbacks, and just letting the driver call it's own runtime PM callbacks during system suspend. It will only do parts of the work then. The amount of code needed is quite small; basically nothing more than: if (!pm_runtime_status_suspended(dev)) dev-driver-pm-runtime_suspend(dev); The problems are: 1. Which callback does this code belong in? 2. Which runtime_suspend callback should be invoked? The example above uses dev-driver-pm-runtime_suspend, but this may not always be the right one. Well, none of them may always be the right one, which is my point. I don't think we can come up with a really generic set of routines that will be generally useful regardless of the subsystem/driver combination in question, so in my opinion the PM core is not the right place to put such routines into. I wouldn't have problems with them being defined in the OMAP PM code. As part of that code they are free to do whatever they like if that code's maintainers are fine with that. But I don't want to see such confusing stuff in the core. As I just replied in my other email, having the code to be available and used in the PM domain code is not an option that we will gain from. The OMAP PM domain already does this, though in a slightly other form - and side effects is not what we want. The drivers need to re-use and invoke the runtime PM callbacks, that is to me the only option that makes sense. At the same time, I somewhat agree with you that the new functions I propose to be added is not so generic. What if we think of them as pm_runtime helper functions instead, would that make more sense? Kind regards Uffe Anyway, I'd like to understand what problems in particular those things would be useful to solve, as I suspect that they may be addressed differently and more cleanly. Thanks, Rafael -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] MAINTAINERS: Add maintainer for the ARM Ux500 clock driver
Cc: Linus Walleij linus.wall...@linaro.org Cc: Mike Turquette mturque...@linaro.org Signed-off-by: Ulf Hansson ulf.hans...@linaro.org --- MAINTAINERS |8 1 file changed, 8 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 284969f..1d0ae30 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1296,6 +1296,14 @@ F: drivers/rtc/rtc-ab8500.c F: drivers/rtc/rtc-pl031.c T: git git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-stericsson.git +ARM/Ux500 CLOCK FRAMEWORK SUPPORT +M: Ulf Hansson ulf.hans...@linaro.org +L: linux-arm-ker...@lists.infradead.org (moderated for non-subscribers) +T: git git://git.linaro.org/people/ulfh/clk.git +S: Maintained +F: drivers/clk/ux500/ +F: include/linux/platform_data/clk-ux500.h + ARM/VFP SUPPORT M: Russell King li...@arm.linux.org.uk L: linux-arm-ker...@lists.infradead.org (moderated for non-subscribers) -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V3] ARM: OMAP2+: omap_device: maintain sane runtime pm status around suspend/resume
On 15 November 2013 23:03, Kevin Hilman khil...@linaro.org wrote: Tony Lindgren t...@atomide.com writes: * Nishanth Menon n...@ti.com [131115 05:30]: On 11/15/2013 02:07 AM, Paul Walmsley wrote: On Thu, 14 Nov 2013, Nishanth Menon wrote: OMAP device hooks around suspend|resume_noirq ensures that hwmod devices are forced to idle using omap_device_idle/enable as part of the last stage of suspend activity. For a device such as i2c who uses autosuspend, it is possible to enter the suspend path with dev-power.runtime_status = RPM_ACTIVE. As part of the suspend flow, the generic runtime logic would increment it's dev-power.disable_depth to 1. This should prevent further pm_runtime_get_sync from succeeding once the runtime_status has been set to RPM_SUSPENDED. Now, as part of the suspend_noirq handler in omap_device, we force the following: if the device status is !suspended, we force the device to idle using omap_device_idle (clocks are cut etc..). This ensures that from a hardware perspective, the device is suspended. However, runtime_status is left to be active. *if* an operation is attempted after this point to pm_runtime_get_sync, runtime framework depends on runtime_status to indicate accurately the device status, and since it sees it to be ACTIVE, it assumes the module is functional and returns a non-error value. As a result the user will see pm_runtime_get succeed, however a register access will crash due to the lack of clocks. To prevent this from happening, we should ensure that runtime_status exactly indicates the device status. As a result of this change any further calls to pm_runtime_get* would return -EACCES (since disable_depth is 1). On resume, we restore the clocks and runtime status exactly as we suspended with. These operations are not expected to fail as we update the states after the core runtime framework has suspended itself and restore before the core runtime framework has resumed. Reported-by: J Keerthy j-keer...@ti.com Signed-off-by: Nishanth Menon n...@ti.com Acked-by: Rajendra Nayak rna...@ti.com Acked-by: Kevin Hilman khil...@linaro.org Hi Kevin, I am wondering if OMAP would benefit from letting the PM core allow runtime suspends during system suspend, which is not the case as of now. My impression is that it could simplify OMAP drivers and the power domain, but it would be interesting to hear your opinion in this. It somewhat touches this patch. I guess. The reason for bringing this up here, is that I wanted to highlight that we are at the moment discussing the following RFC on the linux-pm mailing list: [RFC PATCH] PM / Runtime: Allow to inactivate devices during system suspend. Kind regards Ulf Hansson Looks reasonable to me. Looks like this should be considered for -stable - Nishanth, what do you think? Every product kernel since 3.4 needed to be hacked (we have hacked in different ways so far) to work around this (since we never spend time digging deeper :( ), So, I do agree with your view that a -stable tag will be most beneficial. Tony or Kevin, do you want to take this one, or want me to? I can take it unless you have other fixes pending right now. This version looks good to me. Reviewed-by: Kevin Hilman khil...@linaro.org -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/5] PM / Sleep: Add pm_generic functions to re-use runtime PM callbacks
On 4 December 2013 00:41, Rafael J. Wysocki r...@rjwysocki.net wrote:
On Wednesday, December 04, 2013 12:15:13 AM Rafael J. Wysocki wrote:
On Wednesday, November 27, 2013 04:34:56 PM Ulf Hansson wrote:
To put devices into low power state during sleep, it sometimes makes
sense at subsystem-level to re-use the runtime PM callbacks.
The PM core will at device_suspend_late disable runtime PM, after that
we can safely operate on these callbacks. At suspend_late the device
will be put into low power state by invoking the runtime_suspend
callbacks, unless the runtime status is already suspended. At
resume_early the state is restored by invoking the runtime_resume
callbacks. Soon after the PM core will re-enable runtime PM before
returning from device_resume_early.
The new pm_generic functions are named pm_generic_suspend_late_runtime
and pm_generic_resume_early_runtime, they are supposed to be used in
pairs.
Do note that these new generic callbacks will work smothly even with
and without CONFIG_PM_RUNTIME, as long as the runtime PM callbacks are
implemented inside CONFIG_PM instead of CONFIG_PM_RUNTIME.
A special thanks to Alan Stern who came up with this idea.
Cc: Kevin Hilman khil...@linaro.org
Cc: Alan Stern st...@rowland.harvard.edu
Signed-off-by: Ulf Hansson ulf.hans...@linaro.org
---
drivers/base/power/generic_ops.c | 86
++
include/linux/pm.h |2 +
2 files changed, 88 insertions(+)
diff --git a/drivers/base/power/generic_ops.c
b/drivers/base/power/generic_ops.c
index 5ee030a..8e78ad1 100644
--- a/drivers/base/power/generic_ops.c
+++ b/drivers/base/power/generic_ops.c
@@ -93,6 +93,49 @@ int pm_generic_suspend_late(struct device *dev)
EXPORT_SYMBOL_GPL(pm_generic_suspend_late);
After a bit of reconsideration it appears to me that the only benefit of that
would be to make it possible for drivers to work around incomplete PM domains
implementations. Which would be of questionable value.
/**
+ * pm_generic_suspend_late_runtime - Generic suspend_late callback for
drivers
+ * @dev: Device to suspend.
+ * Use to invoke runtime_suspend callbacks at suspend_late.
+ */
+int pm_generic_suspend_late_runtime(struct device *dev)
+{
+ int (*callback)(struct device *);
+ int ret = 0;
+
+ /*
+* PM core has disabled runtime PM in device_suspend_late, thus we can
+* safely check the device's runtime status and decide whether
+* additional actions are needed to put the device into low power
state.
+* If so, we invoke the runtime_suspend callbacks.
+* For the !CONFIG_PM_RUNTIME case, pm_runtime_status_suspended()
always
+* returns false and therefore the runtime_suspend callback will be
+* invoked.
+*/
+ if (pm_runtime_status_suspended(dev))
+ return 0;
+
+ if (dev-pm_domain)
+ callback = dev-pm_domain-ops.runtime_suspend;
First of all, for this to work you need to assume that the type/bus/class
will
not exercise hardware in any way by itself (or you can look at the PCI bus
type
for an example why it won't generally work otherwise), so you could simply
skip
the rest of this conditional.
For the bus types you are concerned about (platform and AMBA) that actually
is
the case as far as I can say.
+ else if (dev-type dev-type-pm)
+ callback = dev-type-pm-runtime_suspend;
+ else if (dev-class dev-class-pm)
+ callback = dev-class-pm-runtime_suspend;
+ else if (dev-bus dev-bus-pm)
+ callback = dev-bus-pm-runtime_suspend;
+ else
+ callback = NULL;
+
+ if (!callback dev-driver dev-driver-pm)
+ callback = dev-driver-pm-runtime_suspend;
+
+ if (callback)
+ ret = callback(dev);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(pm_generic_suspend_late_runtime);
After that you're left with something like this:
int prototype_suspend_late(struct device *dev)
{
int (*callback)(struct device *);
if (pm_runtime_status_suspended(dev))
return 0;
if (dev-pm_domain)
callback = dev-pm_domain-ops.runtime_suspend;
else if (dev-driver dev-driver-pm)
callback = dev-driver-pm-runtime_suspend;
else
callback = NULL;
return callback ? callback(dev) : 0;
}
Moreover, if a driver's .suspend_late is pointed to the above, it can be
called
in two ways. The first way is via type/bus/class or the PM core itself which
means that all PM handling at this stage is going to be done by
prototype_suspend_late(). The other way is via a PM domain, in which case
there may be some additional PM handling in the domain code in principle
(before or after calling the driver's .suspend_late()). However, in the
latter case it generally wouldn't be OK to call PM domain's
