[PATCH RFC RFT v2 2/3] clk: divider: add CLK_DIVIDER_INVALID_ZERO flag

[Jerome Brunet]

> When a divider register reads zero, divider_recalc_rate() currently
> emits a warning (unless CLK_DIVIDER_ALLOW_ZERO is set) and returns
> the parent rate.
> 
> This behaviour assumes a zero divisor acts as a pass-through, which
> does not apply to all hardware. Some dividers, for example PLL
> pre-dividers on Amlogic SoCs, come out of reset with a zero value
> that does not produce a usable clock. Returning the parent rate
> hides this fault behind an incorrect, plausible value.
> 
> CLK_DIVIDER_ALLOW_ZERO only suppresses the warning while still
> reporting the parent rate, so it cannot represent this hardware
> behaviour.
> 
> Add CLK_DIVIDER_INVALID_ZERO for dividers where a zero divisor value
> is invalid. When this flag is set, recalc_rate() returns 0, making
> the faulty state visible to the clock framework rather than hiding it
> as a pass-through.
> 
> Signed-off-by: Jian Hu <jian.hu at amlogic.com>
> ---
>  drivers/clk/clk-divider.c    | 3 +++
>  include/linux/clk-provider.h | 6 ++++++
>  2 files changed, 9 insertions(+)
>
> diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
> index f68216c65d3d..6c1523d22477 100644
> --- a/drivers/clk/clk-divider.c
> +++ b/drivers/clk/clk-divider.c
> @@ -143,6 +143,9 @@ unsigned long divider_recalc_rate(struct clk_hw *hw, unsigned long parent_rate,
>  
>  	div = _get_div(table, val, flags, width);
>  	if (!div) {
> +		if (flags & CLK_DIVIDER_INVALID_ZERO)
> +			return 0;

While I think CLK_DIVIDER_INVALID_ZERO really makes sense for a writeable divider
only, to gracefully handle a reset value at zero that we would later correct,
we should still handle all paths correctly. This include a RO divider and its
determine rate path. I'll try to send something for this soon.

> +
>  		WARN(!(flags & CLK_DIVIDER_ALLOW_ZERO),
>  			"%s: Zero divisor and CLK_DIVIDER_ALLOW_ZERO not set\n",
>  			clk_hw_get_name(hw));
> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> index dc8e161f9c2c..fe2dbbba49e3 100644
> --- a/include/linux/clk-provider.h
> +++ b/include/linux/clk-provider.h
> @@ -744,6 +744,11 @@ struct clk_div_table {
>   *	big endian.
>   * CLK_DIVIDER_EVEN_INTEGERS - clock divisor is 2, 4, 6, 8, 10, etc.
>   *	Formula is 2 * (value read from hardware + 1).
> + * CLK_DIVIDER_INVALID_ZERO - a zero value read from the register is not a
> + *	divide by one / bypass but an invalid setting which does not produce
> + *	a usable clock.  recalc_rate() returns 0 so that the broken state is
> + *	reported to the framework instead of the parent rate, without
> + *	triggering the CLK_DIVIDER_ALLOW_ZERO warning.


 CLK_DIVIDER_INVALID_ZERO - a zero divisor is an invalid setting with no
 	usable output. recalc_rate() returns 0 instead of the parent rate.
	Takes precedence over CLK_DIVIDER_ALLOW_ZERO.

-- 
Jerome