Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1672117 > unrolled thread

Re: [RESEND PATCH v4] clk: stm32h7: Add stm32h743 clock driver

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [RESEND PATCH v4] clk: stm32h7: Add stm32h743 clock driver Stephen Boyd <sboyd@codeaurora.org> - 2017-06-22 00:10 +0200
    Re: [RESEND PATCH v4] clk: stm32h7: Add stm32h743 clock driver Gabriel FERNANDEZ <gabriel.fernandez@st.com> - 2017-06-22 16:30 +0200
    Re: [RESEND PATCH v4] clk: stm32h7: Add stm32h743 clock driver Gabriel FERNANDEZ <gabriel.fernandez@st.com> - 2017-06-27 14:20 +0200
      Re: [RESEND PATCH v4] clk: stm32h7: Add stm32h743 clock driver Stephen Boyd <sboyd@codeaurora.org> - 2017-06-28 18:10 +0200
        Re: [RESEND PATCH v4] clk: stm32h7: Add stm32h743 clock driver Gabriel FERNANDEZ <gabriel.fernandez@st.com> - 2017-06-29 15:50 +0200
          Re: [RESEND PATCH v4] clk: stm32h7: Add stm32h743 clock driver Stephen Boyd <sboyd@codeaurora.org> - 2017-06-30 02:30 +0200
            Re: [RESEND PATCH v4] clk: stm32h7: Add stm32h743 clock driver Gabriel FERNANDEZ <gabriel.fernandez@st.com> - 2017-06-30 09:20 +0200
              Re: [RESEND PATCH v4] clk: stm32h7: Add stm32h743 clock driver Stephen Boyd <sboyd@codeaurora.org> - 2017-06-30 21:00 +0200

#1672117 — Re: [RESEND PATCH v4] clk: stm32h7: Add stm32h743 clock driver

On 06/07, gabriel.fernandez@st.com wrote:
> From: Gabriel Fernandez <gabriel.fernandez@st.com>
> 
> This patch enables clocks for STM32H743 boards.
> 
> Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
> 
> for MFD changes:
> Acked-by: Lee Jones <lee.jones@linaro.org>
> 
> for DT-Bindings
> Acked-by: Rob Herring <robh@kernel.org>
> v4:
>   - rename lock into stm32rcc_lock
>   - don't use clk_readl() 
>   - remove useless parentheses with GENMASK
>   - fix parents of timer_x clocks
>   - suppress pll configuration from DT
>   - fix kbuild warning
> 
> v3:
>   - fix compatible string "stm32h7-pll" into "st,stm32h7-pll"
>   - fix bad parent name for mco2 clock
>   - set CLK_SET_RATE_PARENT for ltdc clock
>   - set CLK_IGNORE_UNUSED for pll1
>   - disable power domain write protection on disable ops if needed
> 
> 
> v2:
>   - rename compatible string "stm32,pll" into "stm32h7-pll"
>   - suppress "st,pllrge" property
>   - suppress "st, frac-status" property
>   - change management of "st,frac"  property
> 	0 : enable 0 pll integer mode 
> 	other values : enable pll in fractional mode (value is
> 	the fractional factor)

Please drop the changelog from commit text.

> diff --git a/drivers/clk/clk-stm32h7.c b/drivers/clk/clk-stm32h7.c
> new file mode 100644
> index 0000000..2907c1f
> --- /dev/null
> +++ b/drivers/clk/clk-stm32h7.c
> @@ -0,0 +1,1532 @@
> +/* Power domain helper */
> +static inline void disable_power_domain_write_protection(void)
> +{
> +	if (pdrm)
> +		regmap_update_bits(pdrm, 0x00, (1 << 8), (1 << 8));
> +}
> +
> +static inline void enable_power_domain_write_protection(void)
> +{
> +	if (pdrm)
> +		regmap_update_bits(pdrm, 0x00, (1 << 8), (0 << 8));
> +}
> +
> +static inline int is_enable_power_domain_write_protection(void)

Return bool, not int?

> +{
> +	if (pdrm) {
> +		u32 val;
> +
> +		regmap_read(pdrm, 0x00, &val);
> +
> +		return !(val & 0x100);
> +	}
> +	return -1;

Returning -1 looks odd.

> +}
> +
> +/* Gate clock with ready bit and backup domain management */
> +struct stm32_ready_gate {
> +	struct	clk_gate gate;
> +	u8	bit_rdy;
> +	u8	backup_domain;
> +};
> +
> +#define to_ready_gate_clk(_rgate) container_of(_rgate, struct stm32_ready_gate,\
> +		gate)
> +
> +#define RGATE_TIMEOUT 600000
> +
> +static int ready_gate_clk_is_enabled(struct clk_hw *hw)
> +{
> +	return clk_gate_ops.is_enabled(hw);
> +}

Perhaps we should expose clk_gate_ops::is_enabled as functions
that can be directly called and assigned in places like this so
we don't need wrapper functions that do nothing besides forward
the call.

> +
> +static int ready_gate_clk_enable(struct clk_hw *hw)
> +{
> +	struct clk_gate *gate = to_clk_gate(hw);
> +	struct stm32_ready_gate *rgate = to_ready_gate_clk(gate);
> +	int dbp_status;
> +	int bit_status;
> +	unsigned int timeout = RGATE_TIMEOUT;
> +
> +	if (clk_gate_ops.is_enabled(hw))
> +		return 0;
> +
> +	dbp_status = is_enable_power_domain_write_protection();
> +
> +	if (rgate->backup_domain && dbp_status)
> +		disable_power_domain_write_protection();
> +
> +	clk_gate_ops.enable(hw);
> +
> +	do {
> +		bit_status = !(readl(gate->reg) & BIT(rgate->bit_rdy));
> +
> +		if (bit_status)
> +			udelay(1000);
> +
> +	} while (bit_status && --timeout);

readl_poll_timeout?

> +
> +/* RTC clock */
> +static u8 rtc_mux_get_parent(struct clk_hw *hw)
> +{
> +	return clk_mux_ops.get_parent(hw);
> +}
> +
> +static int rtc_mux_set_parent(struct clk_hw *hw, u8 index)
> +{
> +	int dbp_status;
> +	int err;
> +
> +	dbp_status = is_enable_power_domain_write_protection();
> +
> +	if (dbp_status)
> +		disable_power_domain_write_protection();
> +
> +	err = clk_mux_ops.set_parent(hw, index);
> +
> +	if (dbp_status)
> +		enable_power_domain_write_protection();
> +
> +	return err;
> +}
> +
> +static int rtc_mux_determine_rate(struct clk_hw *hw,
> +		struct clk_rate_request *req)
> +{
> +	return clk_mux_ops.determine_rate(hw, req);
> +}

In this case we have that function exposed already so it could
be assigned.

> +
> +static const struct clk_ops rtc_mux_ops = {
> +	.get_parent	= rtc_mux_get_parent,
> +	.set_parent	= rtc_mux_set_parent,
> +	.determine_rate = rtc_mux_determine_rate,
> +};
> +
> +/* Clock gate with backup domain protection management */
> +static int bd_gate_is_enabled(struct clk_hw *hw)
> +{
> +	return clk_gate_ops.is_enabled(hw);
> +}
> +
> +static int bd_gate_enable(struct clk_hw *hw)
> +{
> +	int dbp_status;
> +	int err;
> +
> +	if (bd_gate_is_enabled(hw))
> +		return 0;
> +
[...]
> +
> +	return;
> +
> +err_free_clks:
> +	kfree(clk_data);
> +}
> +
> +CLK_OF_DECLARE_DRIVER(stm32h7_rcc, "st,stm32h743-rcc", stm32h7_rcc_init);

Can you add a comment above this why we can't do a split design
with a platform driver and a CLK_OF_DECLARE_DRIVER() routine here
and also mention the other driver that's probing against the same
compatible?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[toc] | [next] | [standalone]

#1672688

Hi Stephen,

Thanks for reviewing.

On 06/22/2017 12:07 AM, Stephen Boyd wrote:
> On 06/07, gabriel.fernandez@st.com wrote:
>> From: Gabriel Fernandez <gabriel.fernandez@st.com>
>>
>> This patch enables clocks for STM32H743 boards.
>>
>> Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
>>
>> for MFD changes:
>> Acked-by: Lee Jones <lee.jones@linaro.org>
>>
>> for DT-Bindings
>> Acked-by: Rob Herring <robh@kernel.org>
>> v4:
>>    - rename lock into stm32rcc_lock
>>    - don't use clk_readl()
>>    - remove useless parentheses with GENMASK
>>    - fix parents of timer_x clocks
>>    - suppress pll configuration from DT
>>    - fix kbuild warning
>>
>> v3:
>>    - fix compatible string "stm32h7-pll" into "st,stm32h7-pll"
>>    - fix bad parent name for mco2 clock
>>    - set CLK_SET_RATE_PARENT for ltdc clock
>>    - set CLK_IGNORE_UNUSED for pll1
>>    - disable power domain write protection on disable ops if needed
>>
>>
>> v2:
>>    - rename compatible string "stm32,pll" into "stm32h7-pll"
>>    - suppress "st,pllrge" property
>>    - suppress "st, frac-status" property
>>    - change management of "st,frac"  property
>> 	0 : enable 0 pll integer mode
>> 	other values : enable pll in fractional mode (value is
>> 	the fractional factor)
> Please drop the changelog from commit text.

strange, i added the changelog after 'git format-patch'

>
>> diff --git a/drivers/clk/clk-stm32h7.c b/drivers/clk/clk-stm32h7.c
>> new file mode 100644
>> index 0000000..2907c1f
>> --- /dev/null
>> +++ b/drivers/clk/clk-stm32h7.c
>> @@ -0,0 +1,1532 @@
>> +/* Power domain helper */
>> +static inline void disable_power_domain_write_protection(void)
>> +{
>> +	if (pdrm)
>> +		regmap_update_bits(pdrm, 0x00, (1 << 8), (1 << 8));
>> +}
>> +
>> +static inline void enable_power_domain_write_protection(void)
>> +{
>> +	if (pdrm)
>> +		regmap_update_bits(pdrm, 0x00, (1 << 8), (0 << 8));
>> +}
>> +
>> +static inline int is_enable_power_domain_write_protection(void)
> Return bool, not int?

ok

>
>> +{
>> +	if (pdrm) {
>> +		u32 val;
>> +
>> +		regmap_read(pdrm, 0x00, &val);
>> +
>> +		return !(val & 0x100);
>> +	}
>> +	return -1;
> Returning -1 looks odd.

ok i will change it

>
>> +}
>> +
>> +/* Gate clock with ready bit and backup domain management */
>> +struct stm32_ready_gate {
>> +	struct	clk_gate gate;
>> +	u8	bit_rdy;
>> +	u8	backup_domain;
>> +};
>> +
>> +#define to_ready_gate_clk(_rgate) container_of(_rgate, struct stm32_ready_gate,\
>> +		gate)
>> +
>> +#define RGATE_TIMEOUT 600000
>> +
>> +static int ready_gate_clk_is_enabled(struct clk_hw *hw)
>> +{
>> +	return clk_gate_ops.is_enabled(hw);
>> +}
> Perhaps we should expose clk_gate_ops::is_enabled as functions
> that can be directly called and assigned in places like this so
> we don't need wrapper functions that do nothing besides forward
> the call.

ok i will add a patch in clk.c and clk-provider.h to export 'clk_gate_is_enabled'

>
>> +
>> +static int ready_gate_clk_enable(struct clk_hw *hw)
>> +{
>> +	struct clk_gate *gate = to_clk_gate(hw);
>> +	struct stm32_ready_gate *rgate = to_ready_gate_clk(gate);
>> +	int dbp_status;
>> +	int bit_status;
>> +	unsigned int timeout = RGATE_TIMEOUT;
>> +
>> +	if (clk_gate_ops.is_enabled(hw))
>> +		return 0;
>> +
>> +	dbp_status = is_enable_power_domain_write_protection();
>> +
>> +	if (rgate->backup_domain && dbp_status)
>> +		disable_power_domain_write_protection();
>> +
>> +	clk_gate_ops.enable(hw);
>> +
>> +	do {
>> +		bit_status = !(readl(gate->reg) & BIT(rgate->bit_rdy));
>> +
>> +		if (bit_status)
>> +			udelay(1000);
>> +
>> +	} while (bit_status && --timeout);
> readl_poll_timeout?

last time it didn't work, i will investigate again

>> +
>> +/* RTC clock */
>> +static u8 rtc_mux_get_parent(struct clk_hw *hw)
>> +{
>> +	return clk_mux_ops.get_parent(hw);
>> +}
>> +
>> +static int rtc_mux_set_parent(struct clk_hw *hw, u8 index)
>> +{
>> +	int dbp_status;
>> +	int err;
>> +
>> +	dbp_status = is_enable_power_domain_write_protection();
>> +
>> +	if (dbp_status)
>> +		disable_power_domain_write_protection();
>> +
>> +	err = clk_mux_ops.set_parent(hw, index);
>> +
>> +	if (dbp_status)
>> +		enable_power_domain_write_protection();
>> +
>> +	return err;
>> +}
>> +
>> +static int rtc_mux_determine_rate(struct clk_hw *hw,
>> +		struct clk_rate_request *req)
>> +{
>> +	return clk_mux_ops.determine_rate(hw, req);
>> +}
> In this case we have that function exposed already so it could
> be assigned.

ok i will use __clk_mux_determine_rate

>> +
>> +static const struct clk_ops rtc_mux_ops = {
>> +	.get_parent	= rtc_mux_get_parent,
>> +	.set_parent	= rtc_mux_set_parent,
>> +	.determine_rate = rtc_mux_determine_rate,
>> +};
>> +
>> +/* Clock gate with backup domain protection management */
>> +static int bd_gate_is_enabled(struct clk_hw *hw)
>> +{
>> +	return clk_gate_ops.is_enabled(hw);
>> +}
>> +
>> +static int bd_gate_enable(struct clk_hw *hw)
>> +{
>> +	int dbp_status;
>> +	int err;
>> +
>> +	if (bd_gate_is_enabled(hw))
>> +		return 0;
>> +
> [...]
>> +
>> +	return;
>> +
>> +err_free_clks:
>> +	kfree(clk_data);
>>   +}
>> +
>> +CLK_OF_DECLARE_DRIVER(stm32h7_rcc, "st,stm32h743-rcc", stm32h7_rcc_init);
> Can you add a comment above this why we can't do a split design
> with a platform driver and a CLK_OF_DECLARE_DRIVER() routine here
> and also mention the other driver that's probing against the same
> compatible?
>

ok

[toc] | [prev] | [next] | [standalone]

#1675612

On 06/22/2017 12:07 AM, Stephen Boyd wrote:
> On 06/07, gabriel.fernandez@st.com wrote:
>> From: Gabriel Fernandez <gabriel.fernandez@st.com>
>>
>> This patch enables clocks for STM32H743 boards.
>>
>> Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
>>
>> for MFD changes:
>> Acked-by: Lee Jones <lee.jones@linaro.org>
>>
>> for DT-Bindings
>> Acked-by: Rob Herring <robh@kernel.org>
>> v4:
>>    - rename lock into stm32rcc_lock
>>    - don't use clk_readl()
>>    - remove useless parentheses with GENMASK
>>    - fix parents of timer_x clocks
>>    - suppress pll configuration from DT
>>    - fix kbuild warning
>>
>> v3:
>>    - fix compatible string "stm32h7-pll" into "st,stm32h7-pll"
>>    - fix bad parent name for mco2 clock
>>    - set CLK_SET_RATE_PARENT for ltdc clock
>>    - set CLK_IGNORE_UNUSED for pll1
>>    - disable power domain write protection on disable ops if needed
>>
>>
>> v2:
>>    - rename compatible string "stm32,pll" into "stm32h7-pll"
>>    - suppress "st,pllrge" property
>>    - suppress "st, frac-status" property
>>    - change management of "st,frac"  property
>> 	0 : enable 0 pll integer mode
>> 	other values : enable pll in fractional mode (value is
>> 	the fractional factor)
> Please drop the changelog from commit text.
>
>> diff --git a/drivers/clk/clk-stm32h7.c b/drivers/clk/clk-stm32h7.c
>> new file mode 100644
>> index 0000000..2907c1f
>> --- /dev/null
>> +++ b/drivers/clk/clk-stm32h7.c
>> @@ -0,0 +1,1532 @@
>> +/* Power domain helper */
>> +static inline void disable_power_domain_write_protection(void)
>> +{
>> +	if (pdrm)
>> +		regmap_update_bits(pdrm, 0x00, (1 << 8), (1 << 8));
>> +}
>> +
>> +static inline void enable_power_domain_write_protection(void)
>> +{
>> +	if (pdrm)
>> +		regmap_update_bits(pdrm, 0x00, (1 << 8), (0 << 8));
>> +}
>> +
>> +static inline int is_enable_power_domain_write_protection(void)
> Return bool, not int?
>
>> +{
>> +	if (pdrm) {
>> +		u32 val;
>> +
>> +		regmap_read(pdrm, 0x00, &val);
>> +
>> +		return !(val & 0x100);
>> +	}
>> +	return -1;
> Returning -1 looks odd.
>
>> +}
>> +
>> +/* Gate clock with ready bit and backup domain management */
>> +struct stm32_ready_gate {
>> +	struct	clk_gate gate;
>> +	u8	bit_rdy;
>> +	u8	backup_domain;
>> +};
>> +
>> +#define to_ready_gate_clk(_rgate) container_of(_rgate, struct stm32_ready_gate,\
>> +		gate)
>> +
>> +#define RGATE_TIMEOUT 600000
>> +
>> +static int ready_gate_clk_is_enabled(struct clk_hw *hw)
>> +{
>> +	return clk_gate_ops.is_enabled(hw);
>> +}
> Perhaps we should expose clk_gate_ops::is_enabled as functions
> that can be directly called and assigned in places like this so
> we don't need wrapper functions that do nothing besides forward
> the call.
>
>> +
>> +static int ready_gate_clk_enable(struct clk_hw *hw)
>> +{
>> +	struct clk_gate *gate = to_clk_gate(hw);
>> +	struct stm32_ready_gate *rgate = to_ready_gate_clk(gate);
>> +	int dbp_status;
>> +	int bit_status;
>> +	unsigned int timeout = RGATE_TIMEOUT;
>> +
>> +	if (clk_gate_ops.is_enabled(hw))
>> +		return 0;
>> +
>> +	dbp_status = is_enable_power_domain_write_protection();
>> +
>> +	if (rgate->backup_domain && dbp_status)
>> +		disable_power_domain_write_protection();
>> +
>> +	clk_gate_ops.enable(hw);
>> +
>> +	do {
>> +		bit_status = !(readl(gate->reg) & BIT(rgate->bit_rdy));
>> +
>> +		if (bit_status)
>> +			udelay(1000);
>> +
>> +	} while (bit_status && --timeout);
> readl_poll_timeout?
if i use readl_poll_timeout (wich use 'ktime_get()') it can be 
operational only after the selection of clocksource ? (device_initcall).
And then if a driver turn on a clock before, it could blocked the linux 
console ?

>
>> +
>> +/* RTC clock */
>> +static u8 rtc_mux_get_parent(struct clk_hw *hw)
>> +{
>> +	return clk_mux_ops.get_parent(hw);
>> +}
>> +
>> +static int rtc_mux_set_parent(struct clk_hw *hw, u8 index)
>> +{
>> +	int dbp_status;
>> +	int err;
>> +
>> +	dbp_status = is_enable_power_domain_write_protection();
>> +
>> +	if (dbp_status)
>> +		disable_power_domain_write_protection();
>> +
>> +	err = clk_mux_ops.set_parent(hw, index);
>> +
>> +	if (dbp_status)
>> +		enable_power_domain_write_protection();
>> +
>> +	return err;
>> +}
>> +
>> +static int rtc_mux_determine_rate(struct clk_hw *hw,
>> +		struct clk_rate_request *req)
>> +{
>> +	return clk_mux_ops.determine_rate(hw, req);
>> +}
> In this case we have that function exposed already so it could
> be assigned.
>
>> +
>> +static const struct clk_ops rtc_mux_ops = {
>> +	.get_parent	= rtc_mux_get_parent,
>> +	.set_parent	= rtc_mux_set_parent,
>> +	.determine_rate = rtc_mux_determine_rate,
>> +};
>> +
>> +/* Clock gate with backup domain protection management */
>> +static int bd_gate_is_enabled(struct clk_hw *hw)
>> +{
>> +	return clk_gate_ops.is_enabled(hw);
>> +}
>> +
>> +static int bd_gate_enable(struct clk_hw *hw)
>> +{
>> +	int dbp_status;
>> +	int err;
>> +
>> +	if (bd_gate_is_enabled(hw))
>> +		return 0;
>> +
> [...]
>> +
>> +	return;
>> +
>> +err_free_clks:
>> +	kfree(clk_data);
>> +}
>> +
>> +CLK_OF_DECLARE_DRIVER(stm32h7_rcc, "st,stm32h743-rcc", stm32h7_rcc_init);
> Can you add a comment above this why we can't do a split design
> with a platform driver and a CLK_OF_DECLARE_DRIVER() routine here
> and also mention the other driver that's probing against the same
> compatible?
>

[toc] | [prev] | [next] | [standalone]

#1676887

On 06/27, Gabriel FERNANDEZ wrote:
> 
> 
> On 06/22/2017 12:07 AM, Stephen Boyd wrote:
> > readl_poll_timeout?
> >
> if i use readl_poll_timeout (wich use 'ktime_get()') it can be 
> operational only after the selection of clocksource ? (device_initcall).
> And then if a driver turn on a clock before, it could blocked the linux 
> console ?
> 

Ok. I wonder if we could add some sort of starting check to
readl_poll_timeout() that tests system_state for booting vs.
scheduling? That should be sufficient to handle this case?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[toc] | [prev] | [next] | [standalone]

#1677769

On 06/28/2017 05:59 PM, Stephen Boyd wrote:
> On 06/27, Gabriel FERNANDEZ wrote:
>>
>> On 06/22/2017 12:07 AM, Stephen Boyd wrote:
>>> readl_poll_timeout?
>>>
>> if i use readl_poll_timeout (wich use 'ktime_get()') it can be
>> operational only after the selection of clocksource ? (device_initcall).
>> And then if a driver turn on a clock before, it could blocked the linux
>> console ?
>>
> Ok. I wonder if we could add some sort of starting check to
> readl_poll_timeout() that tests system_state for booting vs.
> scheduling? That should be sufficient to handle this case?
>
Oops i think i understood my problem...
i used readl_poll_timeout in atomic context.
I have to move my code in the .prepare ops.

If you are ok with that i will send a v5

Thanks

Gabriel

[toc] | [prev] | [next] | [standalone]

#1678339

On 06/29, Gabriel FERNANDEZ wrote:
> 
> 
> On 06/28/2017 05:59 PM, Stephen Boyd wrote:
> > On 06/27, Gabriel FERNANDEZ wrote:
> >>
> >> On 06/22/2017 12:07 AM, Stephen Boyd wrote:
> >>> readl_poll_timeout?
> >>>
> >> if i use readl_poll_timeout (wich use 'ktime_get()') it can be
> >> operational only after the selection of clocksource ? (device_initcall).
> >> And then if a driver turn on a clock before, it could blocked the linux
> >> console ?
> >>
> > Ok. I wonder if we could add some sort of starting check to
> > readl_poll_timeout() that tests system_state for booting vs.
> > scheduling? That should be sufficient to handle this case?
> >
> Oops i think i understood my problem...
> i used readl_poll_timeout in atomic context.
> I have to move my code in the .prepare ops.
> 
> If you are ok with that i will send a v5
> 

There's readl_poll_timeout_atomic() for those modes.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[toc] | [prev] | [next] | [standalone]

#1678549

On 06/30/2017 02:20 AM, Stephen Boyd wrote:
> On 06/29, Gabriel FERNANDEZ wrote:
>>
>> On 06/28/2017 05:59 PM, Stephen Boyd wrote:
>>> On 06/27, Gabriel FERNANDEZ wrote:
>>>> On 06/22/2017 12:07 AM, Stephen Boyd wrote:
>>>>> readl_poll_timeout?
>>>>>
>>>> if i use readl_poll_timeout (wich use 'ktime_get()') it can be
>>>> operational only after the selection of clocksource ? (device_initcall).
>>>> And then if a driver turn on a clock before, it could blocked the linux
>>>> console ?
>>>>
>>> Ok. I wonder if we could add some sort of starting check to
>>> readl_poll_timeout() that tests system_state for booting vs.
>>> scheduling? That should be sufficient to handle this case?
>>>
>> Oops i think i understood my problem...
>> i used readl_poll_timeout in atomic context.
>> I have to move my code in the .prepare ops.
>>
>> If you are ok with that i will send a v5
>>
> There's readl_poll_timeout_atomic() for those modes.
>
yes it's exactly the test i made (use 'readl_poll_timeout()_atomic' in 
.enable ops) but i'm blocked.

if i do the same in .prepare ops with 'readl_poll_timeout()' it's ok.

[toc] | [prev] | [next] | [standalone]

#1679064

On 06/30, Gabriel FERNANDEZ wrote:
> 
> 
> On 06/30/2017 02:20 AM, Stephen Boyd wrote:
> > On 06/29, Gabriel FERNANDEZ wrote:
> >>
> >> On 06/28/2017 05:59 PM, Stephen Boyd wrote:
> >>> On 06/27, Gabriel FERNANDEZ wrote:
> >>>> On 06/22/2017 12:07 AM, Stephen Boyd wrote:
> >>>>> readl_poll_timeout?
> >>>>>
> >>>> if i use readl_poll_timeout (wich use 'ktime_get()') it can be
> >>>> operational only after the selection of clocksource ? (device_initcall).
> >>>> And then if a driver turn on a clock before, it could blocked the linux
> >>>> console ?
> >>>>
> >>> Ok. I wonder if we could add some sort of starting check to
> >>> readl_poll_timeout() that tests system_state for booting vs.
> >>> scheduling? That should be sufficient to handle this case?
> >>>
> >> Oops i think i understood my problem...
> >> i used readl_poll_timeout in atomic context.
> >> I have to move my code in the .prepare ops.
> >>
> >> If you are ok with that i will send a v5
> >>
> > There's readl_poll_timeout_atomic() for those modes.
> >
> yes it's exactly the test i made (use 'readl_poll_timeout()_atomic' in 
> .enable ops) but i'm blocked.
> 
> if i do the same in .prepare ops with 'readl_poll_timeout()' it's ok.

I'm still confused. readl_poll_timeout_atomic() uses ktime_get(),
and so does readl_poll_timeout(), so how does moving to the
prepare op fix the problem? What's the actual problem?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web