Hello Mark,
On Thu, 9 Jan 2020 20:38:19 +0000, Mark Brown broonie@kernel.org wrote:
On Fri, Dec 27, 2019 at 04:20:56PM +0100, Peter Seiderer wrote:
@@ -338,7 +338,8 @@ static unsigned long clk_aic32x4_div_recalc_rate(struct clk_hw *hw,
unsigned int val;
- regmap_read(div->regmap, div->reg, &val);
- if (regmap_read(div->regmap, div->reg, &val))
Unrelated to your question, but I would change this line (on next patch iteration) to (as all other return value checked places for regmap_read in the same file):
if (regmap_read(div->regmap, div->reg, &val) < 0)
return 0;Is this the best fix - shouldn't we be returning an error here? We don't know what the value programmed into the device actually is so zero might be wrong, and we still have the risk that the value we read from the device may be zero if the device is misprogrammed.
clk_aic32x4_div_recalc_rate() is used for clk_ops aic32x4_div_ops.recalc_rate, did not check/or see on first sight if there is a error handling on the usage of recalc_rate, but did take a look at some other places where the error handling seems to be to return zero, e.g. sound/soc/codecs/da7219.c sound/soc/intel/skylake/skl-ssp-clk.c, etc.
The error occurred with linux-5.3.18, with the earlier versions on regmap_read failure val was (by chance) near 2^31 and evaluated with (val & AIC32X4_DIV_MASK) to 96 (or similar)...but with 5.3.18 evaluated to 0 and the line
return DIV_ROUND_UP(parent_rate, val & AIC32X4_DIV_MASK);
produced the 'Division by zero'...
Regards, Peter