Re: [PATCH v5 2/2] clk: en7523: add support for .set_rate for en7581

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] clk: en7523: misuse of dev_err_probe() in runtime
  en7581_set_rate()
--

commit 5164adf91e05547277b7acad91e9f5b487a3781f
Author: Christian Marangi <ansuelsmth@gmail.com>

clk: en7523: add support for .set_rate for en7581

This commit adds clk_div_table definitions for the BUS, SLIC, and SPI clocks
and implements .determine_rate and .set_rate callbacks for the Airoha EN7581
SoC.

> diff --git a/drivers/clk/clk-en7523.c b/drivers/clk/clk-en7523.c
> index 6640e814d36a4..b0aed88528c5d 100644
> --- a/drivers/clk/clk-en7523.c
> +++ b/drivers/clk/clk-en7523.c

[ ... ]

> @@ -775,11 +864,148 @@ static unsigned long en7581_recalc_rate(struct clk_hw *hw,

[ ... ]

> +static int en7581_set_rate(struct clk_hw *hw, unsigned long rate,
> +			   unsigned long parent_rate)
> +{
> +	struct en_clk *en_clk = container_of(hw, struct en_clk, hw);
> +	const struct en_clk_desc *desc = en_clk->desc;
> +	struct device *dev = clk_hw_get_dev(hw);
> +	struct regmap *map = en_clk->map;
> +	u32 base_val, div_val;
> +	u32 reg, val, mask;
> +	int err;
> +
> +	err = en7581_get_vals_for_rate(hw, en_clk->desc, rate,
> +				       &base_val, &div_val);
> +	if (err)
> +		return err;
> +
> +	if (desc->div_bits) {
> +		reg = desc->div_reg ? desc->div_reg : desc->base_reg;
> +
> +		mask = (BIT(desc->div_bits) - 1) << desc->div_shift;
> +		val = div_val << desc->div_shift;
> +
> +		err = regmap_update_bits(map, reg, mask, val);
> +		if (err)
> +			return dev_err_probe(dev, err, "Failed to update div reg for clock %s\n",
> +					     desc->name);
> +	}
> +
> +	if (desc->base_bits) {
> +		mask = (BIT(desc->base_bits) - 1) << desc->base_shift;
> +		val = base_val << desc->base_shift;
> +
> +		err = regmap_update_bits(map, desc->base_reg, mask, val);
> +		if (err)
> +			return dev_err_probe(dev, err, "Failed to update reg for clock %s\n",
> +					     desc->name);
> +	}
> +
> +	return 0;
>  }

[Severity: Low]
Should en7581_set_rate() use dev_err() instead of dev_err_probe()?
en7581_set_rate() is a runtime clk_ops.set_rate callback, whereas
dev_err_probe() is only meant for probe paths (suppressing -ENOMEM logs and
recording dev->p->deferred_probe_reason on -EPROBE_DEFER), and the sibling
runtime callbacks en7581_recalc_rate() and en7581_get_vals_for_rate() use
dev_err().

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110001128.1041-1-ansuelsmth@gmail.com?part=2