Messages in this thread | | | From | Frank Lee <> | Date | Wed, 8 Jul 2020 19:55:40 +0800 | Subject | Re: [PATCH] thermal: sun8i: Be loud when probe fails |
| |
HI Ondrej, On Wed, Jul 8, 2020 at 6:55 PM Ondrej Jirman <megous@megous.com> wrote: > > I noticed several mobile Linux distributions failing to enable the > thermal regulation correctly, because the kernel is silent > when thermal driver fails to probe. Add enough error reporting > to debug issues and warn users in case thermal sensor is failing > to probe. > > Failing to notify users means, that SoC can easily overheat under > load. > > Signed-off-by: Ondrej Jirman <megous@megous.com> > --- > drivers/thermal/sun8i_thermal.c | 55 ++++++++++++++++++++++++++------- > 1 file changed, 43 insertions(+), 12 deletions(-) > > diff --git a/drivers/thermal/sun8i_thermal.c b/drivers/thermal/sun8i_thermal.c > index 74d73be16496..9065e79ae743 100644 > --- a/drivers/thermal/sun8i_thermal.c > +++ b/drivers/thermal/sun8i_thermal.c > @@ -287,8 +287,12 @@ static int sun8i_ths_calibrate(struct ths_device *tmdev) > > calcell = devm_nvmem_cell_get(dev, "calibration"); > if (IS_ERR(calcell)) { > + dev_err(dev, "Failed to get calibration nvmem cell (%ld)\n", > + PTR_ERR(calcell)); > + > if (PTR_ERR(calcell) == -EPROBE_DEFER) > return -EPROBE_DEFER; > + > /* > * Even if the external calibration data stored in sid is > * not accessible, the THS hardware can still work, although > @@ -308,6 +312,8 @@ static int sun8i_ths_calibrate(struct ths_device *tmdev) > caldata = nvmem_cell_read(calcell, &callen); > if (IS_ERR(caldata)) { > ret = PTR_ERR(caldata); > + dev_err(dev, "Failed to read calibration data (%d)\n", > + ret); > goto out; > } > > @@ -330,23 +336,35 @@ static int sun8i_ths_resource_init(struct ths_device *tmdev) > return PTR_ERR(base); > > tmdev->regmap = devm_regmap_init_mmio(dev, base, &config); > - if (IS_ERR(tmdev->regmap)) > + if (IS_ERR(tmdev->regmap)) { > + dev_err(dev, "Failed to init regmap (%ld)\n", > + PTR_ERR(tmdev->regmap)); > return PTR_ERR(tmdev->regmap); > + } > > if (tmdev->chip->has_bus_clk_reset) { > tmdev->reset = devm_reset_control_get(dev, NULL); > - if (IS_ERR(tmdev->reset)) > + if (IS_ERR(tmdev->reset)) { > + dev_err(dev, "Failed to get reset (%ld)\n", > + PTR_ERR(tmdev->reset)); > return PTR_ERR(tmdev->reset); > + } > > tmdev->bus_clk = devm_clk_get(&pdev->dev, "bus"); > - if (IS_ERR(tmdev->bus_clk)) > + if (IS_ERR(tmdev->bus_clk)) { > + dev_err(dev, "Failed to get bus clock (%ld)\n", > + PTR_ERR(tmdev->bus_clk)); > return PTR_ERR(tmdev->bus_clk); > + } > } > > if (tmdev->chip->has_mod_clk) { > tmdev->mod_clk = devm_clk_get(&pdev->dev, "mod"); > - if (IS_ERR(tmdev->mod_clk)) > + if (IS_ERR(tmdev->mod_clk)) { > + dev_err(dev, "Failed to get mod clock (%ld)\n", > + PTR_ERR(tmdev->mod_clk)); > return PTR_ERR(tmdev->mod_clk); > + } > } > > ret = reset_control_deassert(tmdev->reset); > @@ -471,8 +489,12 @@ static int sun8i_ths_register(struct ths_device *tmdev) > i, > &tmdev->sensor[i], > &ths_ops); > - if (IS_ERR(tmdev->sensor[i].tzd)) > + if (IS_ERR(tmdev->sensor[i].tzd)) { > + dev_err(tmdev->dev, > + "Failed to register sensor %d (%ld)\n", > + i, PTR_ERR(tmdev->sensor[i].tzd)); > return PTR_ERR(tmdev->sensor[i].tzd); > + } > > if (devm_thermal_add_hwmon_sysfs(tmdev->sensor[i].tzd)) > dev_warn(tmdev->dev, > @@ -501,19 +523,21 @@ static int sun8i_ths_probe(struct platform_device *pdev) > > ret = sun8i_ths_resource_init(tmdev); > if (ret) > - return ret; > + goto err_out; > > irq = platform_get_irq(pdev, 0); > - if (irq < 0) > - return irq; > + if (irq < 0) { > + ret = irq; > + goto err_out; > + } > > ret = tmdev->chip->init(tmdev); > if (ret) > - return ret; > + goto err_out; > > ret = sun8i_ths_register(tmdev); > if (ret) > - return ret; > + goto err_out; > > /* > * Avoid entering the interrupt handler, the thermal device is not > @@ -523,10 +547,17 @@ static int sun8i_ths_probe(struct platform_device *pdev) > ret = devm_request_threaded_irq(dev, irq, NULL, > sun8i_irq_thread, > IRQF_ONESHOT, "ths", tmdev); > - if (ret) > - return ret; > + if (ret) { > + dev_err(dev, "Failed to request irq (%d)\n", ret); > + goto err_out; > + } > > + dev_info(dev, "Thermal sensor ready!\n"); > return 0; > + > +err_out: > + dev_err(dev, "Failed to probe thermal sensor (%d)\n", ret);
When the driver fails, there will be this print. Isn't it superfluous for you to add these?
sun8i-thermal: probe of 5070400.thermal-sensor failed with error
Yangtao
| |