Re: [PATCH v8 1/2] pwm: meson: Add support for Amlogic S4 PWM

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 2024/6/14 15:24, Jerome Brunet wrote:
[ EXTERNAL EMAIL ]

On Thu 13 Jun 2024 at 22:46, Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxx> wrote:

Hello,

On Thu, Jun 13, 2024 at 05:54:31PM +0200, Jerome Brunet wrote:
+  for (i = 0; i < MESON_NUM_PWMS; i++) {
+          meson->channels[i].clk = of_clk_get(np, i);
+          if (IS_ERR(meson->channels[i].clk))
+                  return dev_err_probe(dev,
+                                       PTR_ERR(meson->channels[i].clk),
+                                       "Failed to get clk\n");

here it is ok but ..

+
+          ret = devm_add_action_or_reset(dev, meson_pwm_s4_put_clk,
+                                         meson->channels[i].clk);
+          if (ret)
+                  return dev_err_probe(dev, ret,
+                                       "Failed to add clk_put action\n");

... here, devm_add_action_or_reset cannot defer, so dev_err_probe is not useful.
dev_err() if you must print something. Just "return ret;" would be fine
by me

I don't agree. dev_err_probe() has several purposes. Only one of them is
handling -EPROBE_DEFER. See also commit
532888a59505da2a3fbb4abac6adad381cedb374.

I was stuck on the -EPROBE_DEFER usage. Thanks for the heads up


So yes, please use dev_err_probe() also to handle
devm_add_action_or_reset().

My point here is also that devm_add_action_or_reset() can only fail on
memory allocation, like (devm_)kzalloc. Looking around the kernel, we
tend to not add messages for that and just return the error code,
presumably because those same 'out of memory' messages would proliferate
everywhere.


Best regards
Uwe

--
Jerome

Hi Uwe, I didnt get the clear point.
So, if we need "return ret" directly? or keep "dev_err_probe()" to print?

--
Best regards
Junyi




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux