| Message ID | 20240613161045.29606-12-kabel@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Updates for turris-mox-rwtm driver | expand |
On Thu, Jun 13, 2024 at 6:11 PM Marek Behún <kabel@kernel.org> wrote: > > Use dev_err_probe() where possible in the driver's .probe() method. ... > rwtm->mbox = mbox_request_channel(&rwtm->mbox_client, 0); > if (IS_ERR(rwtm->mbox)) { > ret = PTR_ERR(rwtm->mbox); > - if (ret != -EPROBE_DEFER) > - dev_err(dev, "Cannot request mailbox channel: %i\n", > - ret); > - return ret; > + if (ret == -EPROBE_DEFER) > + return ret; What;s the point of this check, please? > + return dev_err_probe(dev, ret, > + "Cannot request mailbox channel!\n"); > }
On Thu, 13 Jun 2024 22:49:18 +0200 Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > On Thu, Jun 13, 2024 at 6:11 PM Marek Behún <kabel@kernel.org> wrote: > > > > Use dev_err_probe() where possible in the driver's .probe() method. > > ... > > > rwtm->mbox = mbox_request_channel(&rwtm->mbox_client, 0); > > if (IS_ERR(rwtm->mbox)) { > > ret = PTR_ERR(rwtm->mbox); > > - if (ret != -EPROBE_DEFER) > > - dev_err(dev, "Cannot request mailbox channel: %i\n", > > - ret); > > - return ret; > > > + if (ret == -EPROBE_DEFER) > > + return ret; > > What;s the point of this check, please? > > > + return dev_err_probe(dev, ret, > > + "Cannot request mailbox channel!\n"); > > } > > The point is to not print the error message if we just need to wait for the mailbox driver.
On Mon, Jun 17, 2024 at 1:14 PM Marek Behún <kabel@kernel.org> wrote: > On Thu, 13 Jun 2024 22:49:18 +0200 > Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > > On Thu, Jun 13, 2024 at 6:11 PM Marek Behún <kabel@kernel.org> wrote: ... > > > rwtm->mbox = mbox_request_channel(&rwtm->mbox_client, 0); > > > if (IS_ERR(rwtm->mbox)) { > > > ret = PTR_ERR(rwtm->mbox); > > > - if (ret != -EPROBE_DEFER) > > > - dev_err(dev, "Cannot request mailbox channel: %i\n", > > > - ret); > > > - return ret; > > > > > + if (ret == -EPROBE_DEFER) > > > + return ret; > > > > What;s the point of this check, please? > > > > > + return dev_err_probe(dev, ret, > > > + "Cannot request mailbox channel!\n"); > > > } > > The point is to not print the error message if we just need to wait for > the mailbox driver. Right, but that was the initial idea behind dev_err_probe(). Have you checked its implementation (and I hope it's documented as well)?
On Mon, 17 Jun 2024 14:26:00 +0200 Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > On Mon, Jun 17, 2024 at 1:14 PM Marek Behún <kabel@kernel.org> wrote: > > On Thu, 13 Jun 2024 22:49:18 +0200 > > Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > > > On Thu, Jun 13, 2024 at 6:11 PM Marek Behún <kabel@kernel.org> wrote: > > ... > > > > > rwtm->mbox = mbox_request_channel(&rwtm->mbox_client, 0); > > > > if (IS_ERR(rwtm->mbox)) { > > > > ret = PTR_ERR(rwtm->mbox); > > > > - if (ret != -EPROBE_DEFER) > > > > - dev_err(dev, "Cannot request mailbox channel: %i\n", > > > > - ret); > > > > - return ret; > > > > > > > + if (ret == -EPROBE_DEFER) > > > > + return ret; > > > > > > What;s the point of this check, please? > > > > > > > + return dev_err_probe(dev, ret, > > > > + "Cannot request mailbox channel!\n"); > > > > } > > > > The point is to not print the error message if we just need to wait for > > the mailbox driver. > > Right, but that was the initial idea behind dev_err_probe(). Have you > checked its implementation (and I hope it's documented as well)? > OK I am blushing now :-) Thanks, I'll drop the check
diff --git a/drivers/firmware/turris-mox-rwtm.c b/drivers/firmware/turris-mox-rwtm.c index 3017a8c19005..81a82b1ef515 100644 --- a/drivers/firmware/turris-mox-rwtm.c +++ b/drivers/firmware/turris-mox-rwtm.c @@ -461,10 +461,11 @@ static int turris_mox_rwtm_probe(struct platform_device *pdev) rwtm->mbox = mbox_request_channel(&rwtm->mbox_client, 0); if (IS_ERR(rwtm->mbox)) { ret = PTR_ERR(rwtm->mbox); - if (ret != -EPROBE_DEFER) - dev_err(dev, "Cannot request mailbox channel: %i\n", - ret); - return ret; + if (ret == -EPROBE_DEFER) + return ret; + + return dev_err_probe(dev, ret, + "Cannot request mailbox channel!\n"); } ret = devm_add_action_or_reset(dev, rwtm_devm_mbox_release, rwtm->mbox); @@ -489,10 +490,8 @@ static int turris_mox_rwtm_probe(struct platform_device *pdev) rwtm->hwrng.priv = (unsigned long) rwtm; ret = devm_hwrng_register(dev, &rwtm->hwrng); - if (ret < 0) { - dev_err(dev, "Cannot register HWRNG: %i\n", ret); - return ret; - } + if (ret < 0) + return dev_err_probe(dev, ret, "Cannot register HWRNG!\n"); rwtm_register_debugfs(rwtm);
Use dev_err_probe() where possible in the driver's .probe() method. Signed-off-by: Marek Behún <kabel@kernel.org> --- drivers/firmware/turris-mox-rwtm.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-)