| Message ID | 20240830080103.12811-1-chenyufan@vivo.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v1] clk: meson: axg-audio: Use dev_err_probe() to simplfy code | expand |
On Fri 30 Aug 2024 at 16:01, Chen Yufan <chenyufan@vivo.com> wrote: > Use dev_err_probe() can make code a bit simpler. It surely does but ... > > Signed-off-by: Chen Yufan <chenyufan@vivo.com> > --- > drivers/clk/meson/axg-audio.c | 12 ++++-------- Why this driver alone ? I have nothing against the change you are doing but I would not want to get a single patch for each and every amlogic clock drivers. > 1 file changed, 4 insertions(+), 8 deletions(-) > > diff --git a/drivers/clk/meson/axg-audio.c b/drivers/clk/meson/axg-audio.c > index e03a5bf899c0..0637b05a4c89 100644 > --- a/drivers/clk/meson/axg-audio.c > +++ b/drivers/clk/meson/axg-audio.c > @@ -1761,10 +1761,8 @@ static int axg_audio_clkc_probe(struct platform_device *pdev) > return PTR_ERR(regs); > > map = devm_regmap_init_mmio(dev, regs, &axg_audio_regmap_cfg); > - if (IS_ERR(map)) { > - dev_err(dev, "failed to init regmap: %ld\n", PTR_ERR(map)); > - return PTR_ERR(map); > - } > + if (IS_ERR(map)) > + return dev_err_probe(dev, PTR_ERR(map), "failed to init regmap: %ld\n"); This does not inspire a lot of confidence. Did you perfom any test before sending this ? I know you sent a v2 fixing this, but still ... > > /* Get the mandatory peripheral clock */ > clk = devm_clk_get_enabled(dev, "pclk"); > @@ -1772,10 +1770,8 @@ static int axg_audio_clkc_probe(struct platform_device *pdev) > return PTR_ERR(clk); > > ret = device_reset(dev); > - if (ret) { > - dev_err_probe(dev, ret, "failed to reset device\n"); > - return ret; > - } > + if (ret) > + return dev_err_probe(dev, ret, "failed to reset device\n"); > > /* Populate regmap for the regmap backed clocks */ > for (i = 0; i < data->regmap_clk_num; i++) Finally, it does not apply on my tree. Check the PR sent to Stephen, you'll get all the details. https://lore.kernel.org/linux-clk/1jjzfyrsbh.fsf@starbuckisacylon.baylibre.com/
On 30/08/2024 16:46, Jerome Brunet wrote: > On Fri 30 Aug 2024 at 16:01, Chen Yufan <chenyufan@vivo.com> wrote: > >> Use dev_err_probe() can make code a bit simpler. > > It surely does but ... > >> >> Signed-off-by: Chen Yufan <chenyufan@vivo.com> >> --- >> drivers/clk/meson/axg-audio.c | 12 ++++-------- > > Why this driver alone ? > > I have nothing against the change you are doing but I would not want to get a > single patch for each and every amlogic clock drivers. > Look here: https://lore.kernel.org/all/?q=f:@vivo.com There is a like a flood of trivial patches started last two weeks. Not sure if there is any coordination, any oversight or review. Few trivial ones were introducing bugs, so I suggest caution. Best regards, Krzysztof
diff --git a/drivers/clk/meson/axg-audio.c b/drivers/clk/meson/axg-audio.c index e03a5bf899c0..0637b05a4c89 100644 --- a/drivers/clk/meson/axg-audio.c +++ b/drivers/clk/meson/axg-audio.c @@ -1761,10 +1761,8 @@ static int axg_audio_clkc_probe(struct platform_device *pdev) return PTR_ERR(regs); map = devm_regmap_init_mmio(dev, regs, &axg_audio_regmap_cfg); - if (IS_ERR(map)) { - dev_err(dev, "failed to init regmap: %ld\n", PTR_ERR(map)); - return PTR_ERR(map); - } + if (IS_ERR(map)) + return dev_err_probe(dev, PTR_ERR(map), "failed to init regmap: %ld\n"); /* Get the mandatory peripheral clock */ clk = devm_clk_get_enabled(dev, "pclk"); @@ -1772,10 +1770,8 @@ static int axg_audio_clkc_probe(struct platform_device *pdev) return PTR_ERR(clk); ret = device_reset(dev); - if (ret) { - dev_err_probe(dev, ret, "failed to reset device\n"); - return ret; - } + if (ret) + return dev_err_probe(dev, ret, "failed to reset device\n"); /* Populate regmap for the regmap backed clocks */ for (i = 0; i < data->regmap_clk_num; i++)
Use dev_err_probe() can make code a bit simpler. Signed-off-by: Chen Yufan <chenyufan@vivo.com> --- drivers/clk/meson/axg-audio.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-)