| Message ID | 20230621162005.473049-6-ben.dooks@codethink.co.uk (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
| Series | updates for i3c error printing | expand |
Hello, Please fix the typo in the subject. On 21/06/2023 17:20:05+0100, Ben Dooks wrote: > The devm_reset_control_get_optional_exclusive() call does > not print any errors, neiterh does the clk_prepare_enable Also fix this one. > or devm_request_irq() call. > > Add some basic error printing to make the probe failures > easier to debug. I guess all those dev_err could be dev_dbg so we don't litter the driver with strings that will only ever be useful during bring up. > > Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk> > --- > drivers/i3c/master/dw-i3c-master.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c > index 9332ae5f6419..ffc84ff6225c 100644 > --- a/drivers/i3c/master/dw-i3c-master.c > +++ b/drivers/i3c/master/dw-i3c-master.c > @@ -1429,12 +1429,16 @@ int dw_i3c_common_probe(struct dw_i3c_master *master, > > master->core_rst = devm_reset_control_get_optional_exclusive(&pdev->dev, > "core_rst"); > - if (IS_ERR(master->core_rst)) > + if (IS_ERR(master->core_rst)) { > + dev_err(&pdev->dev, "cannot get core_rst\n"); > return PTR_ERR(master->core_rst); > + } > > ret = clk_prepare_enable(master->core_clk); > - if (ret) > + if (ret) { > + dev_err(&pdev->dev, "cannot enable core_clk\n"); > goto err_disable_core_clk; > + } > > reset_control_deassert(master->core_rst); > > @@ -1446,8 +1450,10 @@ int dw_i3c_common_probe(struct dw_i3c_master *master, > ret = devm_request_irq(&pdev->dev, irq, > dw_i3c_master_irq_handler, 0, > dev_name(&pdev->dev), master); > - if (ret) > + if (ret) { > + dev_err(&pdev->dev, "cannot get irq\n"); > goto err_assert_rst; > + } > > platform_set_drvdata(pdev, master); > > -- > 2.40.1 >
On 04/07/2023 22:37, Alexandre Belloni wrote: > Hello, > > Please fix the typo in the subject. > > On 21/06/2023 17:20:05+0100, Ben Dooks wrote: >> The devm_reset_control_get_optional_exclusive() call does >> not print any errors, neiterh does the clk_prepare_enable > Also fix this one. > >> or devm_request_irq() call. >> >> Add some basic error printing to make the probe failures >> easier to debug. > > I guess all those dev_err could be dev_dbg so we don't litter the driver > with strings that will only ever be useful during bring up. I think dev_err_probe() is probably the right one as we aren't going to want to warn on any sort of probe defer errors. However most drivers are printing errors if there are resources missing so I think an error is the best way to report issues. I'll correct the spelling when the discussions about the correct way to print errors is. >> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk> >> --- >> drivers/i3c/master/dw-i3c-master.c | 12 +++++++++--- >> 1 file changed, 9 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c >> index 9332ae5f6419..ffc84ff6225c 100644 >> --- a/drivers/i3c/master/dw-i3c-master.c >> +++ b/drivers/i3c/master/dw-i3c-master.c >> @@ -1429,12 +1429,16 @@ int dw_i3c_common_probe(struct dw_i3c_master *master, >> >> master->core_rst = devm_reset_control_get_optional_exclusive(&pdev->dev, >> "core_rst"); >> - if (IS_ERR(master->core_rst)) >> + if (IS_ERR(master->core_rst)) { >> + dev_err(&pdev->dev, "cannot get core_rst\n"); >> return PTR_ERR(master->core_rst); >> + } >> >> ret = clk_prepare_enable(master->core_clk); >> - if (ret) >> + if (ret) { >> + dev_err(&pdev->dev, "cannot enable core_clk\n"); >> goto err_disable_core_clk; >> + } >> >> reset_control_deassert(master->core_rst); >> >> @@ -1446,8 +1450,10 @@ int dw_i3c_common_probe(struct dw_i3c_master *master, >> ret = devm_request_irq(&pdev->dev, irq, >> dw_i3c_master_irq_handler, 0, >> dev_name(&pdev->dev), master); >> - if (ret) >> + if (ret) { >> + dev_err(&pdev->dev, "cannot get irq\n"); >> goto err_assert_rst; >> + } >> >> platform_set_drvdata(pdev, master); >> >> -- >> 2.40.1 >> >
diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c index 9332ae5f6419..ffc84ff6225c 100644 --- a/drivers/i3c/master/dw-i3c-master.c +++ b/drivers/i3c/master/dw-i3c-master.c @@ -1429,12 +1429,16 @@ int dw_i3c_common_probe(struct dw_i3c_master *master, master->core_rst = devm_reset_control_get_optional_exclusive(&pdev->dev, "core_rst"); - if (IS_ERR(master->core_rst)) + if (IS_ERR(master->core_rst)) { + dev_err(&pdev->dev, "cannot get core_rst\n"); return PTR_ERR(master->core_rst); + } ret = clk_prepare_enable(master->core_clk); - if (ret) + if (ret) { + dev_err(&pdev->dev, "cannot enable core_clk\n"); goto err_disable_core_clk; + } reset_control_deassert(master->core_rst); @@ -1446,8 +1450,10 @@ int dw_i3c_common_probe(struct dw_i3c_master *master, ret = devm_request_irq(&pdev->dev, irq, dw_i3c_master_irq_handler, 0, dev_name(&pdev->dev), master); - if (ret) + if (ret) { + dev_err(&pdev->dev, "cannot get irq\n"); goto err_assert_rst; + } platform_set_drvdata(pdev, master);
The devm_reset_control_get_optional_exclusive() call does not print any errors, neiterh does the clk_prepare_enable or devm_request_irq() call. Add some basic error printing to make the probe failures easier to debug. Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk> --- drivers/i3c/master/dw-i3c-master.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)