| Message ID | 20190810052829.6032-1-tiny.windzz@gmail.com |
|---|---|
| Headers | show |
| Series | add thermal driver for h6 | expand |
On Fri, Aug 9, 2019 at 10:31 PM Yangtao Li <tiny.windzz@gmail.com> wrote: > > H3 has extra clock, so introduce something in ths_thermal_chip/ths_device > and adds the process of the clock. > > This is pre-work for supprt it. > > Signed-off-by: Yangtao Li <tiny.windzz@gmail.com> > --- > drivers/thermal/sun8i_thermal.c | 17 ++++++++++++++++- > 1 file changed, 16 insertions(+), 1 deletion(-) > > diff --git a/drivers/thermal/sun8i_thermal.c b/drivers/thermal/sun8i_thermal.c > index b934bc81eba7..6f4294c2aba7 100644 > --- a/drivers/thermal/sun8i_thermal.c > +++ b/drivers/thermal/sun8i_thermal.c > @@ -54,6 +54,7 @@ struct tsensor { > }; > > struct ths_thermal_chip { > + bool has_mod_clk; > int sensor_num; > int offset; > int scale; > @@ -69,6 +70,7 @@ struct ths_device { > struct regmap *regmap; > struct reset_control *reset; > struct clk *bus_clk; > + struct clk *mod_clk; > struct tsensor sensor[MAX_SENSOR_NUM]; > }; > > @@ -274,6 +276,12 @@ static int sun8i_ths_resource_init(struct ths_device *tmdev) > if (IS_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)) > + return PTR_ERR(tmdev->mod_clk); > + } > + > ret = reset_control_deassert(tmdev->reset); > if (ret) > return ret; > @@ -282,12 +290,18 @@ static int sun8i_ths_resource_init(struct ths_device *tmdev) > if (ret) > goto assert_reset; > > - ret = sun50i_ths_calibrate(tmdev); > + ret = clk_prepare_enable(tmdev->mod_clk); You have to set rate of modclk before enabling it since you can't rely on whatever bootloader left for you. Also I found that parameters you're using for PC_TEMP_PERIOD, ACQ0 and ACQ1 are too aggressive and may result in high interrupt rate to the point when it may stall RCU. I changed driver a bit to use params from Philipp Rossak's work (modclk set to 4MHz, PC_TEMP_PERIOD is 7, ACQ0 is 255, ACQ1 is 63) and it fixed RCU stalls for me, see [1] for details. [1] https://github.com/anarsoul/linux-2.6/commit/46b8bb0fe2ccd1cd88fa9181a2ecbf79e8d513b2 > if (ret) > goto bus_disable; > > + ret = sun50i_ths_calibrate(tmdev); > + if (ret) > + goto mod_disable; > + > return 0; > > +mod_disable: > + clk_disable_unprepare(tmdev->mod_clk); > bus_disable: > clk_disable_unprepare(tmdev->bus_clk); > assert_reset: > @@ -395,6 +409,7 @@ static int sun8i_ths_remove(struct platform_device *pdev) > { > struct ths_device *tmdev = platform_get_drvdata(pdev); > > + clk_disable_unprepare(tmdev->mod_clk); > clk_disable_unprepare(tmdev->bus_clk); > reset_control_assert(tmdev->reset); > > -- > 2.17.1 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi Yangtao, On 10/08/2019 07:28, Yangtao Li wrote: > This patchset add support for A64, H3, H5, H6 and R40 thermal sensor. Could you add the device-tree configuration in the same series? This will allow user to test it. Thanks, Clément > > Thx to Icenowy and Vasily. > > BTY, do a cleanup in thermal makfile. > > Icenowy Zheng (3): > thermal: sun8i: allow to use custom temperature calculation function > thermal: sun8i: add support for Allwinner H5 thermal sensor > thermal: sun8i: add support for Allwinner R40 thermal sensor > > Vasily Khoruzhick (1): > thermal: sun8i: add thermal driver for A64 > > Yangtao Li (14): > thermal: sun8i: add thermal driver for h6 > dt-bindings: thermal: add binding document for h6 thermal controller > thermal: fix indentation in makefile > thermal: sun8i: get ths sensor number from device compatible > thermal: sun8i: rework for sun8i_ths_get_temp() > thermal: sun8i: get ths init func from device compatible > thermal: sun8i: rework for ths irq handler func > thermal: sun8i: support mod clocks > thermal: sun8i: rework for ths calibrate func > dt-bindings: thermal: add binding document for h3 thermal controller > thermal: sun8i: add thermal driver for h3 > dt-bindings: thermal: add binding document for a64 thermal controller > dt-bindings: thermal: add binding document for h5 thermal controller > dt-bindings: thermal: add binding document for r40 thermal controller > > .../bindings/thermal/sun8i-thermal.yaml | 157 +++++ > MAINTAINERS | 7 + > drivers/thermal/Kconfig | 14 + > drivers/thermal/Makefile | 9 +- > drivers/thermal/sun8i_thermal.c | 596 ++++++++++++++++++ > 5 files changed, 779 insertions(+), 4 deletions(-) > create mode 100644 Documentation/devicetree/bindings/thermal/sun8i-thermal.yaml > create mode 100644 drivers/thermal/sun8i_thermal.c > --- > v5: > -add more support > -some trival fix > --- > 2.17.1 > >
Hi, On Sat, Aug 10, 2019 at 05:28:26AM +0000, Yangtao Li wrote: > From: Icenowy Zheng <icenowy@aosc.io> > > The H5 temperature calculation function is strange. Firstly, it's > segmented. Secondly, the formula of two sensors are different in the > second segment. > > Allow to use a custom temperature calculation function, in case of > the function is complex. > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io> When you send a patch on someone else's behalf, you need to put your Signed-off-by as well. > --- > drivers/thermal/sun8i_thermal.c | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/drivers/thermal/sun8i_thermal.c b/drivers/thermal/sun8i_thermal.c > index 3259081da841..a761e2afda08 100644 > --- a/drivers/thermal/sun8i_thermal.c > +++ b/drivers/thermal/sun8i_thermal.c > @@ -76,6 +76,7 @@ struct ths_thermal_chip { > u16 *caldata, int callen); > int (*init)(struct ths_device *tmdev); > int (*irq_ack)(struct ths_device *tmdev); > + int (*calc_temp)(int id, int reg); > }; > > struct ths_device { > @@ -90,9 +91,12 @@ struct ths_device { > > /* Temp Unit: millidegree Celsius */ > static int sun8i_ths_reg2temp(struct ths_device *tmdev, > - int reg) > + int id, int reg) > { > - return (reg + tmdev->chip->offset) * tmdev->chip->scale; > + if (tmdev->chip->calc_temp) > + return tmdev->chip->calc_temp(id, reg); > + else > + return (reg + tmdev->chip->offset) * tmdev->chip->scale; You're not consistent here compared to the other callbacks you have introduced: calibrate, init and irq_ack all need to be set and will fail (hard) if you don't set them, yet this one will have a different behaviour (that behaviour being to use the H6 formula, which is the latest SoC, which is a bit odd in itself). I guess we should either make it mandatory as the rest of the callbacks, or document which callbacks are mandatory and which are optional (and the behaviour when it's optional). Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
On Mon, Aug 12, 2019 at 5:14 AM Clément Péron <peron.clem@gmail.com> wrote: > > Hi Yangtao, > > On 10/08/2019 07:28, Yangtao Li wrote: > > This patchset add support for A64, H3, H5, H6 and R40 thermal sensor. > > Could you add the device-tree configuration in the same series? > This will allow user to test it. Ok, it will be added later. Yangtao > > Thanks, > Clément > > > > > Thx to Icenowy and Vasily. > > > > BTY, do a cleanup in thermal makfile. > > > > Icenowy Zheng (3): > > thermal: sun8i: allow to use custom temperature calculation function > > thermal: sun8i: add support for Allwinner H5 thermal sensor > > thermal: sun8i: add support for Allwinner R40 thermal sensor > > > > Vasily Khoruzhick (1): > > thermal: sun8i: add thermal driver for A64 > > > > Yangtao Li (14): > > thermal: sun8i: add thermal driver for h6 > > dt-bindings: thermal: add binding document for h6 thermal controller > > thermal: fix indentation in makefile > > thermal: sun8i: get ths sensor number from device compatible > > thermal: sun8i: rework for sun8i_ths_get_temp() > > thermal: sun8i: get ths init func from device compatible > > thermal: sun8i: rework for ths irq handler func > > thermal: sun8i: support mod clocks > > thermal: sun8i: rework for ths calibrate func > > dt-bindings: thermal: add binding document for h3 thermal controller > > thermal: sun8i: add thermal driver for h3 > > dt-bindings: thermal: add binding document for a64 thermal controller > > dt-bindings: thermal: add binding document for h5 thermal controller > > dt-bindings: thermal: add binding document for r40 thermal controller > > > > .../bindings/thermal/sun8i-thermal.yaml | 157 +++++ > > MAINTAINERS | 7 + > > drivers/thermal/Kconfig | 14 + > > drivers/thermal/Makefile | 9 +- > > drivers/thermal/sun8i_thermal.c | 596 ++++++++++++++++++ > > 5 files changed, 779 insertions(+), 4 deletions(-) > > create mode 100644 Documentation/devicetree/bindings/thermal/sun8i-thermal.yaml > > create mode 100644 drivers/thermal/sun8i_thermal.c > > --- > > v5: > > -add more support > > -some trival fix > > --- > > 2.17.1 > > > >
HI Vasily, On Sat, Aug 10, 2019 at 2:17 PM Vasily Khoruzhick <anarsoul@gmail.com> wrote: > > On Fri, Aug 9, 2019 at 10:31 PM Yangtao Li <tiny.windzz@gmail.com> wrote: > > > > H3 has extra clock, so introduce something in ths_thermal_chip/ths_device > > and adds the process of the clock. > > > > This is pre-work for supprt it. > > > > Signed-off-by: Yangtao Li <tiny.windzz@gmail.com> > > --- > > drivers/thermal/sun8i_thermal.c | 17 ++++++++++++++++- > > 1 file changed, 16 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/thermal/sun8i_thermal.c b/drivers/thermal/sun8i_thermal.c > > index b934bc81eba7..6f4294c2aba7 100644 > > --- a/drivers/thermal/sun8i_thermal.c > > +++ b/drivers/thermal/sun8i_thermal.c > > @@ -54,6 +54,7 @@ struct tsensor { > > }; > > > > struct ths_thermal_chip { > > + bool has_mod_clk; > > int sensor_num; > > int offset; > > int scale; > > @@ -69,6 +70,7 @@ struct ths_device { > > struct regmap *regmap; > > struct reset_control *reset; > > struct clk *bus_clk; > > + struct clk *mod_clk; > > struct tsensor sensor[MAX_SENSOR_NUM]; > > }; > > > > @@ -274,6 +276,12 @@ static int sun8i_ths_resource_init(struct ths_device *tmdev) > > if (IS_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)) > > + return PTR_ERR(tmdev->mod_clk); > > + } > > + > > ret = reset_control_deassert(tmdev->reset); > > if (ret) > > return ret; > > @@ -282,12 +290,18 @@ static int sun8i_ths_resource_init(struct ths_device *tmdev) > > if (ret) > > goto assert_reset; > > > > - ret = sun50i_ths_calibrate(tmdev); > > + ret = clk_prepare_enable(tmdev->mod_clk); > > You have to set rate of modclk before enabling it since you can't rely > on whatever bootloader left for you. > > Also I found that parameters you're using for PC_TEMP_PERIOD, ACQ0 and > ACQ1 are too aggressive and may result in high interrupt rate to the > point when it may stall RCU. I changed driver a bit to use params from > Philipp Rossak's work (modclk set to 4MHz, PC_TEMP_PERIOD is 7, ACQ0 > is 255, ACQ1 is 63) and it fixed RCU stalls for me, see [1] for > details. Why is the RCU stall happening, is it caused by a deadlock? Can you provide log information and your configuration? I am a bit curious. Thx, Yangtao > > [1] https://github.com/anarsoul/linux-2.6/commit/46b8bb0fe2ccd1cd88fa9181a2ecbf79e8d513b2 > > > > if (ret) > > goto bus_disable; > > > > + ret = sun50i_ths_calibrate(tmdev); > > + if (ret) > > + goto mod_disable; > > + > > return 0; > > > > +mod_disable: > > + clk_disable_unprepare(tmdev->mod_clk); > > bus_disable: > > clk_disable_unprepare(tmdev->bus_clk); > > assert_reset: > > @@ -395,6 +409,7 @@ static int sun8i_ths_remove(struct platform_device *pdev) > > { > > struct ths_device *tmdev = platform_get_drvdata(pdev); > > > > + clk_disable_unprepare(tmdev->mod_clk); > > clk_disable_unprepare(tmdev->bus_clk); > > reset_control_assert(tmdev->reset); > > > > -- > > 2.17.1 > > > > > > _______________________________________________ > > linux-arm-kernel mailing list > > linux-arm-kernel@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Mon, Aug 12, 2019 at 4:46 PM Frank Lee <tiny.windzz@gmail.com> wrote: > > HI Vasily, > > On Sat, Aug 10, 2019 at 2:17 PM Vasily Khoruzhick <anarsoul@gmail.com> wrote: > > > > On Fri, Aug 9, 2019 at 10:31 PM Yangtao Li <tiny.windzz@gmail.com> wrote: > > > > > > H3 has extra clock, so introduce something in ths_thermal_chip/ths_device > > > and adds the process of the clock. > > > > > > This is pre-work for supprt it. > > > > > > Signed-off-by: Yangtao Li <tiny.windzz@gmail.com> > > > --- > > > drivers/thermal/sun8i_thermal.c | 17 ++++++++++++++++- > > > 1 file changed, 16 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/thermal/sun8i_thermal.c b/drivers/thermal/sun8i_thermal.c > > > index b934bc81eba7..6f4294c2aba7 100644 > > > --- a/drivers/thermal/sun8i_thermal.c > > > +++ b/drivers/thermal/sun8i_thermal.c > > > @@ -54,6 +54,7 @@ struct tsensor { > > > }; > > > > > > struct ths_thermal_chip { > > > + bool has_mod_clk; > > > int sensor_num; > > > int offset; > > > int scale; > > > @@ -69,6 +70,7 @@ struct ths_device { > > > struct regmap *regmap; > > > struct reset_control *reset; > > > struct clk *bus_clk; > > > + struct clk *mod_clk; > > > struct tsensor sensor[MAX_SENSOR_NUM]; > > > }; > > > > > > @@ -274,6 +276,12 @@ static int sun8i_ths_resource_init(struct ths_device *tmdev) > > > if (IS_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)) > > > + return PTR_ERR(tmdev->mod_clk); > > > + } > > > + > > > ret = reset_control_deassert(tmdev->reset); > > > if (ret) > > > return ret; > > > @@ -282,12 +290,18 @@ static int sun8i_ths_resource_init(struct ths_device *tmdev) > > > if (ret) > > > goto assert_reset; > > > > > > - ret = sun50i_ths_calibrate(tmdev); > > > + ret = clk_prepare_enable(tmdev->mod_clk); > > > > You have to set rate of modclk before enabling it since you can't rely > > on whatever bootloader left for you. > > > > Also I found that parameters you're using for PC_TEMP_PERIOD, ACQ0 and > > ACQ1 are too aggressive and may result in high interrupt rate to the > > point when it may stall RCU. I changed driver a bit to use params from > > Philipp Rossak's work (modclk set to 4MHz, PC_TEMP_PERIOD is 7, ACQ0 > > is 255, ACQ1 is 63) and it fixed RCU stalls for me, see [1] for > > details. > > Why is the RCU stall happening, is it caused by a deadlock? > Can you provide log information and your configuration? > I am a bit curious. It's not deadlock, I believe it just can't handle that many interrupts when running at lowest CPU frequency. Even with Philipp's settings there's ~20 interrupts a second from ths. I don't remember how many interrupts were there with your settings. Unfortunately there's nothing interesting in backtraces, I'm using Pine64-LTS board. > Thx, > Yangtao > > > > > [1] https://github.com/anarsoul/linux-2.6/commit/46b8bb0fe2ccd1cd88fa9181a2ecbf79e8d513b2 > > > > > > > if (ret) > > > goto bus_disable; > > > > > > + ret = sun50i_ths_calibrate(tmdev); > > > + if (ret) > > > + goto mod_disable; > > > + > > > return 0; > > > > > > +mod_disable: > > > + clk_disable_unprepare(tmdev->mod_clk); > > > bus_disable: > > > clk_disable_unprepare(tmdev->bus_clk); > > > assert_reset: > > > @@ -395,6 +409,7 @@ static int sun8i_ths_remove(struct platform_device *pdev) > > > { > > > struct ths_device *tmdev = platform_get_drvdata(pdev); > > > > > > + clk_disable_unprepare(tmdev->mod_clk); > > > clk_disable_unprepare(tmdev->bus_clk); > > > reset_control_assert(tmdev->reset); > > > > > > -- > > > 2.17.1 > > > > > > > > > _______________________________________________ > > > linux-arm-kernel mailing list > > > linux-arm-kernel@lists.infradead.org > > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Mon, Aug 12, 2019 at 04:54:15PM -0700, Vasily Khoruzhick wrote: > On Mon, Aug 12, 2019 at 4:46 PM Frank Lee <tiny.windzz@gmail.com> wrote: > > > > HI Vasily, > > > > On Sat, Aug 10, 2019 at 2:17 PM Vasily Khoruzhick <anarsoul@gmail.com> wrote: > > > > > > On Fri, Aug 9, 2019 at 10:31 PM Yangtao Li <tiny.windzz@gmail.com> wrote: > > > > > > > > H3 has extra clock, so introduce something in ths_thermal_chip/ths_device > > > > and adds the process of the clock. > > > > > > > > This is pre-work for supprt it. > > > > > > > > Signed-off-by: Yangtao Li <tiny.windzz@gmail.com> > > > > --- > > > > drivers/thermal/sun8i_thermal.c | 17 ++++++++++++++++- > > > > 1 file changed, 16 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/thermal/sun8i_thermal.c b/drivers/thermal/sun8i_thermal.c > > > > index b934bc81eba7..6f4294c2aba7 100644 > > > > --- a/drivers/thermal/sun8i_thermal.c > > > > +++ b/drivers/thermal/sun8i_thermal.c > > > > @@ -54,6 +54,7 @@ struct tsensor { > > > > }; > > > > > > > > struct ths_thermal_chip { > > > > + bool has_mod_clk; > > > > int sensor_num; > > > > int offset; > > > > int scale; > > > > @@ -69,6 +70,7 @@ struct ths_device { > > > > struct regmap *regmap; > > > > struct reset_control *reset; > > > > struct clk *bus_clk; > > > > + struct clk *mod_clk; > > > > struct tsensor sensor[MAX_SENSOR_NUM]; > > > > }; > > > > > > > > @@ -274,6 +276,12 @@ static int sun8i_ths_resource_init(struct ths_device *tmdev) > > > > if (IS_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)) > > > > + return PTR_ERR(tmdev->mod_clk); > > > > + } > > > > + > > > > ret = reset_control_deassert(tmdev->reset); > > > > if (ret) > > > > return ret; > > > > @@ -282,12 +290,18 @@ static int sun8i_ths_resource_init(struct ths_device *tmdev) > > > > if (ret) > > > > goto assert_reset; > > > > > > > > - ret = sun50i_ths_calibrate(tmdev); > > > > + ret = clk_prepare_enable(tmdev->mod_clk); > > > > > > You have to set rate of modclk before enabling it since you can't rely > > > on whatever bootloader left for you. > > > > > > Also I found that parameters you're using for PC_TEMP_PERIOD, ACQ0 and > > > ACQ1 are too aggressive and may result in high interrupt rate to the > > > point when it may stall RCU. I changed driver a bit to use params from > > > Philipp Rossak's work (modclk set to 4MHz, PC_TEMP_PERIOD is 7, ACQ0 > > > is 255, ACQ1 is 63) and it fixed RCU stalls for me, see [1] for > > > details. > > > > Why is the RCU stall happening, is it caused by a deadlock? > > Can you provide log information and your configuration? > > I am a bit curious. > > It's not deadlock, I believe it just can't handle that many interrupts > when running at lowest CPU frequency. Even with Philipp's settings > there's ~20 interrupts a second from ths. I don't remember how many > interrupts were there with your settings. > > Unfortunately there's nothing interesting in backtraces, I'm using > Pine64-LTS board. Recently there was a similar issue, with buggy CCU driver that caused CIR interrupts being fired constantly, and it also resulted in RCU stalls. Looks like a comon cause of RCU stalls. THS timing settings probably need to be made specific to the SoC, because I noticed that the same settings lead to wildly different timings on different SoCs. It would be good to measure how often ths interrupt fires with this driver on various SoCs. 20 times a second and more sounds like overkill. I'd expect a useful range to be at most 5-10 times a second. That should be enough to stop overheating the SoC due to suddenly increased load, even without a heatsink. regards, o. > > Thx, > > Yangtao > > > > > > > > [1] https://github.com/anarsoul/linux-2.6/commit/46b8bb0fe2ccd1cd88fa9181a2ecbf79e8d513b2 > > > > > > > > > > if (ret) > > > > goto bus_disable; > > > > > > > > + ret = sun50i_ths_calibrate(tmdev); > > > > + if (ret) > > > > + goto mod_disable; > > > > + > > > > return 0; > > > > > > > > +mod_disable: > > > > + clk_disable_unprepare(tmdev->mod_clk); > > > > bus_disable: > > > > clk_disable_unprepare(tmdev->bus_clk); > > > > assert_reset: > > > > @@ -395,6 +409,7 @@ static int sun8i_ths_remove(struct platform_device *pdev) > > > > { > > > > struct ths_device *tmdev = platform_get_drvdata(pdev); > > > > > > > > + clk_disable_unprepare(tmdev->mod_clk); > > > > clk_disable_unprepare(tmdev->bus_clk); > > > > reset_control_assert(tmdev->reset); > > > > > > > > -- > > > > 2.17.1 > > > > > > > > > > > > _______________________________________________ > > > > linux-arm-kernel mailing list > > > > linux-arm-kernel@lists.infradead.org > > > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Tue, Aug 13, 2019 at 1:06 PM Ondřej Jirman <megous@megous.com> wrote: > > On Mon, Aug 12, 2019 at 04:54:15PM -0700, Vasily Khoruzhick wrote: > > On Mon, Aug 12, 2019 at 4:46 PM Frank Lee <tiny.windzz@gmail.com> wrote: > > > > > > HI Vasily, > > > > > > On Sat, Aug 10, 2019 at 2:17 PM Vasily Khoruzhick <anarsoul@gmail.com> wrote: > > > > > > > > On Fri, Aug 9, 2019 at 10:31 PM Yangtao Li <tiny.windzz@gmail.com> wrote: > > > > > > > > > > H3 has extra clock, so introduce something in ths_thermal_chip/ths_device > > > > > and adds the process of the clock. > > > > > > > > > > This is pre-work for supprt it. > > > > > > > > > > Signed-off-by: Yangtao Li <tiny.windzz@gmail.com> > > > > > --- > > > > > drivers/thermal/sun8i_thermal.c | 17 ++++++++++++++++- > > > > > 1 file changed, 16 insertions(+), 1 deletion(-) > > > > > > > > > > diff --git a/drivers/thermal/sun8i_thermal.c b/drivers/thermal/sun8i_thermal.c > > > > > index b934bc81eba7..6f4294c2aba7 100644 > > > > > --- a/drivers/thermal/sun8i_thermal.c > > > > > +++ b/drivers/thermal/sun8i_thermal.c > > > > > @@ -54,6 +54,7 @@ struct tsensor { > > > > > }; > > > > > > > > > > struct ths_thermal_chip { > > > > > + bool has_mod_clk; > > > > > int sensor_num; > > > > > int offset; > > > > > int scale; > > > > > @@ -69,6 +70,7 @@ struct ths_device { > > > > > struct regmap *regmap; > > > > > struct reset_control *reset; > > > > > struct clk *bus_clk; > > > > > + struct clk *mod_clk; > > > > > struct tsensor sensor[MAX_SENSOR_NUM]; > > > > > }; > > > > > > > > > > @@ -274,6 +276,12 @@ static int sun8i_ths_resource_init(struct ths_device *tmdev) > > > > > if (IS_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)) > > > > > + return PTR_ERR(tmdev->mod_clk); > > > > > + } > > > > > + > > > > > ret = reset_control_deassert(tmdev->reset); > > > > > if (ret) > > > > > return ret; > > > > > @@ -282,12 +290,18 @@ static int sun8i_ths_resource_init(struct ths_device *tmdev) > > > > > if (ret) > > > > > goto assert_reset; > > > > > > > > > > - ret = sun50i_ths_calibrate(tmdev); > > > > > + ret = clk_prepare_enable(tmdev->mod_clk); > > > > > > > > You have to set rate of modclk before enabling it since you can't rely > > > > on whatever bootloader left for you. > > > > > > > > Also I found that parameters you're using for PC_TEMP_PERIOD, ACQ0 and > > > > ACQ1 are too aggressive and may result in high interrupt rate to the > > > > point when it may stall RCU. I changed driver a bit to use params from > > > > Philipp Rossak's work (modclk set to 4MHz, PC_TEMP_PERIOD is 7, ACQ0 > > > > is 255, ACQ1 is 63) and it fixed RCU stalls for me, see [1] for > > > > details. > > > > > > Why is the RCU stall happening, is it caused by a deadlock? > > > Can you provide log information and your configuration? > > > I am a bit curious. > > > > It's not deadlock, I believe it just can't handle that many interrupts > > when running at lowest CPU frequency. Even with Philipp's settings > > there's ~20 interrupts a second from ths. I don't remember how many > > interrupts were there with your settings. > > > > Unfortunately there's nothing interesting in backtraces, I'm using > > Pine64-LTS board. > > Recently there was a similar issue, with buggy CCU driver that caused > CIR interrupts being fired constantly, and it also resulted in RCU > stalls. Looks like a comon cause of RCU stalls. > > THS timing settings probably need to be made specific to the SoC, because > I noticed that the same settings lead to wildly different timings on > different SoCs. > > It would be good to measure how often ths interrupt fires with this driver > on various SoCs. > > 20 times a second and more sounds like overkill. I'd expect a useful > range to be at most 5-10 times a second. That should be enough to stop > overheating the SoC due to suddenly increased load, even without a > heatsink. Note that A64 has 3 sensors and each sensor has individual interrupt, so technically it's 6-7 interrupts per sensor per second > regards, > o. > > > > Thx, > > > Yangtao > > > > > > > > > > > [1] https://github.com/anarsoul/linux-2.6/commit/46b8bb0fe2ccd1cd88fa9181a2ecbf79e8d513b2 > > > > > > > > > > > > > if (ret) > > > > > goto bus_disable; > > > > > > > > > > + ret = sun50i_ths_calibrate(tmdev); > > > > > + if (ret) > > > > > + goto mod_disable; > > > > > + > > > > > return 0; > > > > > > > > > > +mod_disable: > > > > > + clk_disable_unprepare(tmdev->mod_clk); > > > > > bus_disable: > > > > > clk_disable_unprepare(tmdev->bus_clk); > > > > > assert_reset: > > > > > @@ -395,6 +409,7 @@ static int sun8i_ths_remove(struct platform_device *pdev) > > > > > { > > > > > struct ths_device *tmdev = platform_get_drvdata(pdev); > > > > > > > > > > + clk_disable_unprepare(tmdev->mod_clk); > > > > > clk_disable_unprepare(tmdev->bus_clk); > > > > > reset_control_assert(tmdev->reset); > > > > > > > > > > -- > > > > > 2.17.1 > > > > > > > > > > > > > > > _______________________________________________ > > > > > linux-arm-kernel mailing list > > > > > linux-arm-kernel@lists.infradead.org > > > > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > > > > _______________________________________________ > > linux-arm-kernel mailing list > > linux-arm-kernel@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
HI Vasily, On Wed, Aug 14, 2019 at 11:01 AM Vasily Khoruzhick <anarsoul@gmail.com> wrote: > > On Tue, Aug 13, 2019 at 1:06 PM Ondřej Jirman <megous@megous.com> wrote: > > > > On Mon, Aug 12, 2019 at 04:54:15PM -0700, Vasily Khoruzhick wrote: > > > On Mon, Aug 12, 2019 at 4:46 PM Frank Lee <tiny.windzz@gmail.com> wrote: > > > > > > > > HI Vasily, > > > > > > > > On Sat, Aug 10, 2019 at 2:17 PM Vasily Khoruzhick <anarsoul@gmail.com> wrote: > > > > > > > > > > On Fri, Aug 9, 2019 at 10:31 PM Yangtao Li <tiny.windzz@gmail.com> wrote: > > > > > > > > > > > > H3 has extra clock, so introduce something in ths_thermal_chip/ths_device > > > > > > and adds the process of the clock. > > > > > > > > > > > > This is pre-work for supprt it. > > > > > > > > > > > > Signed-off-by: Yangtao Li <tiny.windzz@gmail.com> > > > > > > --- > > > > > > drivers/thermal/sun8i_thermal.c | 17 ++++++++++++++++- > > > > > > 1 file changed, 16 insertions(+), 1 deletion(-) > > > > > > > > > > > > diff --git a/drivers/thermal/sun8i_thermal.c b/drivers/thermal/sun8i_thermal.c > > > > > > index b934bc81eba7..6f4294c2aba7 100644 > > > > > > --- a/drivers/thermal/sun8i_thermal.c > > > > > > +++ b/drivers/thermal/sun8i_thermal.c > > > > > > @@ -54,6 +54,7 @@ struct tsensor { > > > > > > }; > > > > > > > > > > > > struct ths_thermal_chip { > > > > > > + bool has_mod_clk; > > > > > > int sensor_num; > > > > > > int offset; > > > > > > int scale; > > > > > > @@ -69,6 +70,7 @@ struct ths_device { > > > > > > struct regmap *regmap; > > > > > > struct reset_control *reset; > > > > > > struct clk *bus_clk; > > > > > > + struct clk *mod_clk; > > > > > > struct tsensor sensor[MAX_SENSOR_NUM]; > > > > > > }; > > > > > > > > > > > > @@ -274,6 +276,12 @@ static int sun8i_ths_resource_init(struct ths_device *tmdev) > > > > > > if (IS_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)) > > > > > > + return PTR_ERR(tmdev->mod_clk); > > > > > > + } > > > > > > + > > > > > > ret = reset_control_deassert(tmdev->reset); > > > > > > if (ret) > > > > > > return ret; > > > > > > @@ -282,12 +290,18 @@ static int sun8i_ths_resource_init(struct ths_device *tmdev) > > > > > > if (ret) > > > > > > goto assert_reset; > > > > > > > > > > > > - ret = sun50i_ths_calibrate(tmdev); > > > > > > + ret = clk_prepare_enable(tmdev->mod_clk); > > > > > > > > > > You have to set rate of modclk before enabling it since you can't rely > > > > > on whatever bootloader left for you. > > > > > > > > > > Also I found that parameters you're using for PC_TEMP_PERIOD, ACQ0 and > > > > > ACQ1 are too aggressive and may result in high interrupt rate to the > > > > > point when it may stall RCU. I changed driver a bit to use params from > > > > > Philipp Rossak's work (modclk set to 4MHz, PC_TEMP_PERIOD is 7, ACQ0 > > > > > is 255, ACQ1 is 63) and it fixed RCU stalls for me, see [1] for > > > > > details. > > > > > > > > Why is the RCU stall happening, is it caused by a deadlock? > > > > Can you provide log information and your configuration? > > > > I am a bit curious. > > > > > > It's not deadlock, I believe it just can't handle that many interrupts > > > when running at lowest CPU frequency. Even with Philipp's settings > > > there's ~20 interrupts a second from ths. I don't remember how many > > > interrupts were there with your settings. > > > > > > Unfortunately there's nothing interesting in backtraces, I'm using > > > Pine64-LTS board. > > > > Recently there was a similar issue, with buggy CCU driver that caused > > CIR interrupts being fired constantly, and it also resulted in RCU > > stalls. Looks like a comon cause of RCU stalls. > > > > THS timing settings probably need to be made specific to the SoC, because > > I noticed that the same settings lead to wildly different timings on > > different SoCs. > > > > It would be good to measure how often ths interrupt fires with this driver > > on various SoCs. > > > > 20 times a second and more sounds like overkill. I'd expect a useful > > range to be at most 5-10 times a second. That should be enough to stop > > overheating the SoC due to suddenly increased load, even without a > > heatsink. > > Note that A64 has 3 sensors and each sensor has individual interrupt, > so technically it's 6-7 interrupts per sensor per second You only need to increase the value of the period to reduce the number of interrupts. Can you test the relationship between the period and the number of interrupts when the mod clock does not change and stays 24M? Thx. Yangtao > > > regards, > > o. > > > > > > Thx, > > > > Yangtao > > > > > > > > > > > > > > [1] https://github.com/anarsoul/linux-2.6/commit/46b8bb0fe2ccd1cd88fa9181a2ecbf79e8d513b2 > > > > > > > > > > > > > > > > if (ret) > > > > > > goto bus_disable; > > > > > > > > > > > > + ret = sun50i_ths_calibrate(tmdev); > > > > > > + if (ret) > > > > > > + goto mod_disable; > > > > > > + > > > > > > return 0; > > > > > > > > > > > > +mod_disable: > > > > > > + clk_disable_unprepare(tmdev->mod_clk); > > > > > > bus_disable: > > > > > > clk_disable_unprepare(tmdev->bus_clk); > > > > > > assert_reset: > > > > > > @@ -395,6 +409,7 @@ static int sun8i_ths_remove(struct platform_device *pdev) > > > > > > { > > > > > > struct ths_device *tmdev = platform_get_drvdata(pdev); > > > > > > > > > > > > + clk_disable_unprepare(tmdev->mod_clk); > > > > > > clk_disable_unprepare(tmdev->bus_clk); > > > > > > reset_control_assert(tmdev->reset); > > > > > > > > > > > > -- > > > > > > 2.17.1 > > > > > > > > > > > > > > > > > > _______________________________________________ > > > > > > linux-arm-kernel mailing list > > > > > > linux-arm-kernel@lists.infradead.org > > > > > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > > > > > > _______________________________________________ > > > linux-arm-kernel mailing list > > > linux-arm-kernel@lists.infradead.org > > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Sat, 2019-08-10 at 05:28 +0000, Yangtao Li wrote: > To unify code style. > > Signed-off-by: Yangtao Li <tiny.windzz@gmail.com> the later patches in this series does not change Makefile. So this seems to be a cleanup patch independent of this patch set. It's better to remove this patch from this patch series. thanks, rui > --- > drivers/thermal/Makefile | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile > index fa6f8b206281..d7eafb5ef8ef 100644 > --- a/drivers/thermal/Makefile > +++ b/drivers/thermal/Makefile > @@ -5,7 +5,7 @@ > > obj-$(CONFIG_THERMAL) += thermal_sys.o > thermal_sys-y += thermal_core.o > thermal_sysfs.o \ > - thermal_helpers.o > + thermal_helpers.o > > # interface to/from other layers providing sensors > thermal_sys-$(CONFIG_THERMAL_HWMON) += thermal_hwmon.o > @@ -25,11 +25,11 @@ thermal_sys-$(CONFIG_CPU_THERMAL) += > cpu_cooling.o > thermal_sys-$(CONFIG_CLOCK_THERMAL) += clock_cooling.o > > # devfreq cooling > -thermal_sys-$(CONFIG_DEVFREQ_THERMAL) += devfreq_cooling.o > +thermal_sys-$(CONFIG_DEVFREQ_THERMAL) += devfreq_cooling.o > > # platform thermal drivers > obj-y += broadcom/ > -obj-$(CONFIG_THERMAL_MMIO) += thermal_mmio.o > +obj-$(CONFIG_THERMAL_MMIO) += thermal_mmio.o > obj-$(CONFIG_SPEAR_THERMAL) += spear_thermal.o > obj-$(CONFIG_SUN8I_THERMAL) += sun8i_thermal.o > obj-$(CONFIG_ROCKCHIP_THERMAL) += rockchip_thermal.o > @@ -50,7 +50,7 @@ obj-$(CONFIG_TI_SOC_THERMAL) += ti-soc- > thermal/ > obj-y += st/ > obj-$(CONFIG_QCOM_TSENS) += qcom/ > obj-y += tegra/ > -obj-$(CONFIG_HISI_THERMAL) += hisi_thermal.o > +obj-$(CONFIG_HISI_THERMAL) += hisi_thermal.o > obj-$(CONFIG_MTK_THERMAL) += mtk_thermal.o > obj-$(CONFIG_GENERIC_ADC_THERMAL) += thermal-generic-adc.o > obj-$(CONFIG_ZX2967_THERMAL) += zx2967_thermal.o
On Sat, 2019-08-10 at 05:28 +0000, Yangtao Li wrote: > Here, we do something to prepare for the subsequent > support of multiple platforms. > > 1) rename sun50i_ths_calibrate to sun8i_ths_calibrate, because > this function should be suitable for all platforms now. > > 2) introduce calibrate callback to mask calibration method > differences. > > Signed-off-by: Yangtao Li <tiny.windzz@gmail.com> IMO, patch 4/18 to patch 9/18 are all changes to a new file introduced in patch 1/18, so why not have a prettier patch 1/18 instead of separate patches? thanks, rui > --- > drivers/thermal/sun8i_thermal.c | 86 ++++++++++++++++++------------- > -- > 1 file changed, 48 insertions(+), 38 deletions(-) > > diff --git a/drivers/thermal/sun8i_thermal.c > b/drivers/thermal/sun8i_thermal.c > index 6f4294c2aba7..47c20c4c69e7 100644 > --- a/drivers/thermal/sun8i_thermal.c > +++ b/drivers/thermal/sun8i_thermal.c > @@ -60,6 +60,8 @@ struct ths_thermal_chip { > int scale; > int ft_deviation; > int temp_data_base; > + int (*calibrate)(struct ths_device *tmdev, > + u16 *caldata, int callen); > int (*init)(struct ths_device *tmdev); > int (*irq_ack)(struct ths_device *tmdev); > }; > @@ -152,45 +154,14 @@ static irqreturn_t sun8i_irq_thread(int irq, > void *data) > return IRQ_HANDLED; > } > > -static int sun50i_ths_calibrate(struct ths_device *tmdev) > +static int sun50i_h6_ths_calibrate(struct ths_device *tmdev, > + u16 *caldata, int callen) > { > - struct nvmem_cell *calcell; > struct device *dev = tmdev->dev; > - u16 *caldata; > - size_t callen; > - int ft_temp; > - int i, ret = 0; > - > - calcell = devm_nvmem_cell_get(dev, "calib"); > - if (IS_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 > - * the data won't be so accurate. > - * > - * The default value of calibration register is 0x800 > for > - * every sensor, and the calibration value is usually > 0x7xx > - * or 0x8xx, so they won't be away from the default > value > - * for a lot. > - * > - * So here we do not return error if the calibartion > data is > - * not available, except the probe needs deferring. > - */ > - goto out; > - } > + int i, ft_temp; > > - caldata = nvmem_cell_read(calcell, &callen); > - if (IS_ERR(caldata)) { > - ret = PTR_ERR(caldata); > - goto out; > - } > - > - if (!caldata[0] || callen < 2 + 2 * tmdev->chip->sensor_num) { > - ret = -EINVAL; > - goto out_free; > - } > + if (!caldata[0] || callen < 2 + 2 * tmdev->chip->sensor_num) > + return -EINVAL; > > /* > * efuse layout: > @@ -245,7 +216,45 @@ static int sun50i_ths_calibrate(struct > ths_device *tmdev) > cdata << offset); > } > > -out_free: > + return 0; > +} > + > +static int sun8i_ths_calibrate(struct ths_device *tmdev) > +{ > + struct nvmem_cell *calcell; > + struct device *dev = tmdev->dev; > + u16 *caldata; > + size_t callen; > + int ret = 0; > + > + calcell = devm_nvmem_cell_get(dev, "calib"); > + if (IS_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 > + * the data won't be so accurate. > + * > + * The default value of calibration register is 0x800 > for > + * every sensor, and the calibration value is usually > 0x7xx > + * or 0x8xx, so they won't be away from the default > value > + * for a lot. > + * > + * So here we do not return error if the calibartion > data is > + * not available, except the probe needs deferring. > + */ > + goto out; > + } > + > + caldata = nvmem_cell_read(calcell, &callen); > + if (IS_ERR(caldata)) { > + ret = PTR_ERR(caldata); > + goto out; > + } > + > + tmdev->chip->calibrate(tmdev, caldata, callen); > + > kfree(caldata); > out: > return ret; > @@ -294,7 +303,7 @@ static int sun8i_ths_resource_init(struct > ths_device *tmdev) > if (ret) > goto bus_disable; > > - ret = sun50i_ths_calibrate(tmdev); > + ret = sun8i_ths_calibrate(tmdev); > if (ret) > goto mod_disable; > > @@ -422,6 +431,7 @@ static const struct ths_thermal_chip > sun50i_h6_ths = { > .scale = -67, > .ft_deviation = SUN50I_H6_FT_DEVIATION, > .temp_data_base = SUN50I_H6_THS_TEMP_DATA, > + .calibrate = sun50i_h6_ths_calibrate, > .init = sun50i_h6_thermal_init, > .irq_ack = sun50i_h6_irq_ack, > };
Hello Yangtao, On Sat, Aug 10, 2019 at 05:28:12AM +0000, Yangtao Li wrote: > This patch adds the support for allwinner thermal sensor, within > allwinner SoC. It will register sensors for thermal framework > and use device tree to bind cooling device. > > Signed-off-by: Yangtao Li <tiny.windzz@gmail.com> > --- > MAINTAINERS | 7 + > drivers/thermal/Kconfig | 14 ++ > drivers/thermal/Makefile | 1 + > drivers/thermal/sun8i_thermal.c | 399 ++++++++++++++++++++++++++++++++ > 4 files changed, 421 insertions(+) > create mode 100644 drivers/thermal/sun8i_thermal.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index 47800d32cfbc..89dc43f4064d 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -682,6 +682,13 @@ L: linux-crypto@vger.kernel.org > S: Maintained > F: drivers/crypto/sunxi-ss/ > > +ALLWINNER THERMAL DRIVER > +M: Yangtao Li <tiny.windzz@gmail.com> > +L: linux-pm@vger.kernel.org > +S: Maintained > +F: Documentation/devicetree/bindings/thermal/sun8i-thermal.yaml > +F: drivers/thermal/sun8i_thermal.c > + > ALLWINNER VPU DRIVER > M: Maxime Ripard <maxime.ripard@bootlin.com> > M: Paul Kocialkowski <paul.kocialkowski@bootlin.com> > diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig > index 9966364a6deb..f8b73b32b92d 100644 > --- a/drivers/thermal/Kconfig > +++ b/drivers/thermal/Kconfig > @@ -262,6 +262,20 @@ config SPEAR_THERMAL > Enable this to plug the SPEAr thermal sensor driver into the Linux > thermal framework. > > +config SUN8I_THERMAL > + tristate "Allwinner sun8i thermal driver" > + depends on ARCH_SUNXI || COMPILE_TEST > + depends on HAS_IOMEM > + depends on NVMEM > + depends on OF > + depends on RESET_CONTROLLER > + help > + Support for the sun8i thermal sensor driver into the Linux thermal > + framework. > + > + To compile this driver as a module, choose M here: the > + module will be called sun8i-thermal. > + > config ROCKCHIP_THERMAL > tristate "Rockchip thermal driver" > depends on ARCH_ROCKCHIP || COMPILE_TEST > diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile > index 74a37c7f847a..fa6f8b206281 100644 > --- a/drivers/thermal/Makefile > +++ b/drivers/thermal/Makefile > @@ -31,6 +31,7 @@ thermal_sys-$(CONFIG_DEVFREQ_THERMAL) += devfreq_cooling.o > obj-y += broadcom/ > obj-$(CONFIG_THERMAL_MMIO) += thermal_mmio.o > obj-$(CONFIG_SPEAR_THERMAL) += spear_thermal.o > +obj-$(CONFIG_SUN8I_THERMAL) += sun8i_thermal.o > obj-$(CONFIG_ROCKCHIP_THERMAL) += rockchip_thermal.o > obj-$(CONFIG_RCAR_THERMAL) += rcar_thermal.o > obj-$(CONFIG_RCAR_GEN3_THERMAL) += rcar_gen3_thermal.o > diff --git a/drivers/thermal/sun8i_thermal.c b/drivers/thermal/sun8i_thermal.c > new file mode 100644 > index 000000000000..2ce36fa3fec3 > --- /dev/null > +++ b/drivers/thermal/sun8i_thermal.c > @@ -0,0 +1,399 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Thermal sensor driver for Allwinner SOC > + * Copyright (C) 2019 Yangtao Li > + * > + * Based on the work of Icenowy Zheng <icenowy@aosc.io> > + * Based on the work of Ondrej Jirman <megous@megous.com> > + * Based on the work of Josef Gajdusek <atx@atx.name> > + */ > + > +#include <linux/clk.h> > +#include <linux/device.h> > +#include <linux/interrupt.h> > +#include <linux/module.h> > +#include <linux/nvmem-consumer.h> > +#include <linux/of_device.h> > +#include <linux/platform_device.h> > +#include <linux/regmap.h> > +#include <linux/reset.h> > +#include <linux/slab.h> > +#include <linux/thermal.h> > + > +#define MAX_SENSOR_NUM 4 > + > +#define SUN50I_H6_SENSOR_NUM 2 > +#define SUN50I_H6_OFFSET -2794 > +#define SUN50I_H6_SCALE -67 > + > +#define FT_TEMP_MASK GENMASK(11, 0) > +#define TEMP_CALIB_MASK GENMASK(11, 0) > +#define TEMP_TO_REG 672 > +#define CALIBRATE_DEFAULT 0x800 > + > +#define SUN50I_THS_CTRL0 0x00 > +#define SUN50I_H6_THS_ENABLE 0x04 > +#define SUN50I_H6_THS_PC 0x08 > +#define SUN50I_H6_THS_DIC 0x10 > +#define SUN50I_H6_THS_DIS 0x20 > +#define SUN50I_H6_THS_MFC 0x30 > +#define SUN50I_H6_THS_TEMP_CALIB 0xa0 > +#define SUN50I_H6_THS_TEMP_DATA 0xc0 > + > +#define SUN50I_THS_CTRL0_T_ACQ(x) ((GENMASK(15, 0) & (x)) << 16) > +#define SUN50I_THS_FILTER_EN BIT(2) > +#define SUN50I_THS_FILTER_TYPE(x) (GENMASK(1, 0) & (x)) > +#define SUN50I_H6_THS_PC_TEMP_PERIOD(x) ((GENMASK(19, 0) & (x)) << 12) > +#define SUN50I_H6_THS_DATA_IRQ_STS(x) BIT(x) > + > +/* millidegree celsius */ > +#define SUN50I_H6_FT_DEVIATION 7000 > + > +struct ths_device; > + > +struct tsensor { > + struct ths_device *tmdev; > + struct thermal_zone_device *tzd; > + int id; > +}; > + > +struct ths_device { > + struct device *dev; > + struct regmap *regmap; > + struct reset_control *reset; > + struct clk *bus_clk; > + struct tsensor sensor[MAX_SENSOR_NUM]; > +}; > + > +/* Temp Unit: millidegree Celsius */ > +static int sun8i_ths_reg2temp(struct ths_device *tmdev, > + int reg) > +{ > + return (reg + SUN50I_H6_OFFSET) * SUN50I_H6_SCALE; > +} > + > +static int sun8i_ths_get_temp(void *data, int *temp) > +{ > + struct tsensor *s = data; > + struct ths_device *tmdev = s->tmdev; > + int val; > + > + regmap_read(tmdev->regmap, SUN50I_H6_THS_TEMP_DATA + > + 0x4 * s->id, &val); > + > + /* ths have no data yet */ > + if (!val) > + return -EAGAIN; > + > + *temp = sun8i_ths_reg2temp(tmdev, val); > + /* > + * XX - According to the original sdk, there are some platforms(rarely) > + * that add a fixed offset value after calculating the temperature > + * value. We can't simply put it on the formula for calculating the > + * temperature above, because the formula for calculating the > + * temperature above is also used when the sensor is calibrated. If > + * do this, the correct calibration formula is hard to know. > + */ > + *temp += SUN50I_H6_FT_DEVIATION; > + > + return 0; > +} > + > +static const struct thermal_zone_of_device_ops ths_ops = { > + .get_temp = sun8i_ths_get_temp, > +}; > + > +static const struct regmap_config config = { > + .reg_bits = 32, > + .val_bits = 32, > + .reg_stride = 4, > + .fast_io = true, > +}; > + > +static irqreturn_t sun50i_h6_irq_thread(int irq, void *data) > +{ > + struct ths_device *tmdev = data; > + int i, state; > + > + regmap_read(tmdev->regmap, SUN50I_H6_THS_DIS, &state); > + > + for (i = 0; i < SUN50I_H6_SENSOR_NUM; i++) { > + > + if (state & SUN50I_H6_THS_DATA_IRQ_STS(i)) { > + /* clear data irq pending */ > + regmap_write(tmdev->regmap, SUN50I_H6_THS_DIS, > + SUN50I_H6_THS_DATA_IRQ_STS(i)); > + > + thermal_zone_device_update(tmdev->sensor[i].tzd, > + THERMAL_EVENT_UNSPECIFIED); > + } > + } > + > + return IRQ_HANDLED; > +} > + > +static int sun50i_ths_calibrate(struct ths_device *tmdev) > +{ > + struct nvmem_cell *calcell; > + struct device *dev = tmdev->dev; > + u16 *caldata; > + size_t callen; > + int ft_temp; > + int i, ret = 0; > + > + calcell = devm_nvmem_cell_get(dev, "calib"); > + if (IS_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 > + * the data won't be so accurate. > + * > + * The default value of calibration register is 0x800 for > + * every sensor, and the calibration value is usually 0x7xx > + * or 0x8xx, so they won't be away from the default value > + * for a lot. > + * > + * So here we do not return error if the calibartion data is > + * not available, except the probe needs deferring. > + */ > + goto out; > + } > + > + caldata = nvmem_cell_read(calcell, &callen); > + if (IS_ERR(caldata)) { > + ret = PTR_ERR(caldata); > + goto out; > + } > + > + if (!caldata[0] || callen < 2 + 2 * SUN50I_H6_SENSOR_NUM) { > + ret = -EINVAL; > + goto out_free; > + } > + > + /* > + * efuse layout: > + * > + * 0 11 16 32 > + * +-------+-------+-------+ > + * |temp| |sensor0|sensor1| > + * +-------+-------+-------+ > + * > + * The calibration data on the H6 is the ambient temperature and > + * sensor values that are filled during the factory test stage. > + * > + * The unit of stored FT temperature is 0.1 degreee celusis. > + * Through the stored ambient temperature and the data read > + * by the sensor, after a certain calculation, the calibration > + * value to be compensated can be obtained. > + */ > + ft_temp = caldata[0] & FT_TEMP_MASK; > + > + for (i = 0; i < SUN50I_H6_SENSOR_NUM; i++) { > + int reg = (int)caldata[i + 1]; > + int sensor_temp = sun8i_ths_reg2temp(tmdev, reg); > + int delta, cdata, offset; > + > + /* > + * To calculate the calibration value: > + * > + * X(in Celsius) = Ts - ft_temp > + * delta = X * 10000 / TEMP_TO_REG > + * cdata = CALIBRATE_DEFAULT - delta > + * > + * cdata: calibration value > + */ > + delta = (sensor_temp - ft_temp * 100) * 10 / TEMP_TO_REG; > + cdata = CALIBRATE_DEFAULT - delta; > + if (cdata & ~TEMP_CALIB_MASK) { > + /* > + * Calibration value more than 12-bit, but calibration > + * register is 12-bit. In this case, ths hardware can > + * still work without calibration, although the data > + * won't be so accurate. > + */ > + dev_warn(dev, "sensor%d is not calibrated.\n", i); > + > + continue; > + } > + > + offset = (i % 2) << 4; > + regmap_update_bits(tmdev->regmap, > + SUN50I_H6_THS_TEMP_CALIB + ((i >> 1) * 4), > + 0xfff << offset, > + cdata << offset); > + } > + > +out_free: > + kfree(caldata); > +out: > + return ret; > +} > + > +static int sun8i_ths_resource_init(struct ths_device *tmdev) > +{ > + struct device *dev = tmdev->dev; > + struct platform_device *pdev = to_platform_device(dev); > + struct resource *mem; > + void __iomem *base; > + int ret; > + > + mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + base = devm_ioremap_resource(dev, mem); > + if (IS_ERR(base)) > + return PTR_ERR(base); > + > + tmdev->regmap = devm_regmap_init_mmio(dev, base, &config); > + if (IS_ERR(tmdev->regmap)) > + return PTR_ERR(tmdev->regmap); > + > + tmdev->reset = devm_reset_control_get(dev, 0); > + if (IS_ERR(tmdev->reset)) > + return PTR_ERR(tmdev->reset); > + > + tmdev->bus_clk = devm_clk_get(&pdev->dev, "bus"); > + if (IS_ERR(tmdev->bus_clk)) > + return PTR_ERR(tmdev->bus_clk); > + > + ret = reset_control_deassert(tmdev->reset); > + if (ret) > + return ret; > + > + ret = clk_prepare_enable(tmdev->bus_clk); > + if (ret) > + goto assert_reset; > + > + ret = sun50i_ths_calibrate(tmdev); > + if (ret) > + goto bus_disable; > + > + return 0; > + > +bus_disable: > + clk_disable_unprepare(tmdev->bus_clk); > +assert_reset: > + reset_control_assert(tmdev->reset); > + > + return ret; > +} > + > +static int sun50i_h6_thermal_init(struct ths_device *tmdev) > +{ > + int val; > + > + /* > + * clkin = 24MHz > + * T acquire = clkin / (x + 1) > + * = 20us > + */ I suggest something along the lines of: /* * T_acq = 20us * clkin = 24MHz * * x = T_acq * clkin - 1 * = 479 */ Makes it much more clear how the value in SUN50I_THS_CTRL0_T_ACQ was arrived at and how to calculate a new one. > + regmap_write(tmdev->regmap, SUN50I_THS_CTRL0, > + SUN50I_THS_CTRL0_T_ACQ(479)); > + /* average over 4 samples */ > + regmap_write(tmdev->regmap, SUN50I_H6_THS_MFC, > + SUN50I_THS_FILTER_EN | > + SUN50I_THS_FILTER_TYPE(1)); > + /* period = (x + 1) * 4096 / clkin; ~10ms */ As above, here I suggest: /* * clkin = 24MHz * filter_samples = 4 * period = 0.25s * * x = period * clkin / 4096 / filter_samples - 1 * = 365 */ Also please change this to 365 from 58. I think 4 readings per second are enough. Certainly, 25 are a bit too much. > + regmap_write(tmdev->regmap, SUN50I_H6_THS_PC, > + SUN50I_H6_THS_PC_TEMP_PERIOD(58)); > + /* enable sensor */ > + val = GENMASK(SUN50I_H6_SENSOR_NUM - 1, 0); > + regmap_write(tmdev->regmap, SUN50I_H6_THS_ENABLE, val); > + /* thermal data interrupt enable */ > + val = GENMASK(SUN50I_H6_SENSOR_NUM - 1, 0); > + regmap_write(tmdev->regmap, SUN50I_H6_THS_DIC, val); > + > + return 0; > +} > + > +static int sun8i_ths_register(struct ths_device *tmdev) > +{ > + struct thermal_zone_device *tzd; > + int i; > + > + for (i = 0; i < SUN50I_H6_SENSOR_NUM; i++) { > + tmdev->sensor[i].tmdev = tmdev; > + tmdev->sensor[i].id = i; > + tmdev->sensor[i].tzd = > + devm_thermal_zone_of_sensor_register(tmdev->dev, > + i, > + &tmdev->sensor[i], > + &ths_ops); > + if (IS_ERR(tmdev->sensor[i].tzd)) > + return PTR_ERR(tzd); This should return PTR_ERR(tmdev->sensor[i].tzd). Otherwise this succeeds even if tz registration fails, and you get NULL pointer exception tryin to udpate nonexisting tz from the interrupt handler. regards, Ondrej > + } > + > + return 0; > +} > + > +static int sun8i_ths_probe(struct platform_device *pdev) > +{ > + struct ths_device *tmdev; > + struct device *dev = &pdev->dev; > + int ret, irq; > + > + tmdev = devm_kzalloc(dev, sizeof(*tmdev), GFP_KERNEL); > + if (!tmdev) > + return -ENOMEM; > + > + tmdev->dev = dev; > + platform_set_drvdata(pdev, tmdev); > + > + ret = sun8i_ths_resource_init(tmdev); > + if (ret) > + return ret; > + > + irq = platform_get_irq(pdev, 0); > + if (irq < 0) > + return irq; > + > + ret = sun50i_h6_thermal_init(tmdev); > + if (ret) > + return ret; > + > + ret = sun8i_ths_register(tmdev); > + if (ret) > + return ret; > + > + /* > + * Avoid entering the interrupt handler, the thermal device is not > + * registered yet, we deffer the registration of the interrupt to > + * the end. > + */ > + ret = devm_request_threaded_irq(dev, irq, NULL, > + sun50i_h6_irq_thread, > + IRQF_ONESHOT, "ths", tmdev); > + if (ret) > + return ret; > + > + return ret; > +} > + > +static int sun8i_ths_remove(struct platform_device *pdev) > +{ > + struct ths_device *tmdev = platform_get_drvdata(pdev); > + > + clk_disable_unprepare(tmdev->bus_clk); > + reset_control_assert(tmdev->reset); > + > + return 0; > +} > + > +static const struct of_device_id of_ths_match[] = { > + { .compatible = "allwinner,sun50i-h6-ths"}, > + { /* sentinel */ }, > +}; > +MODULE_DEVICE_TABLE(of, of_ths_match); > + > +static struct platform_driver ths_driver = { > + .probe = sun8i_ths_probe, > + .remove = sun8i_ths_remove, > + .driver = { > + .name = "sun8i-thermal", > + .of_match_table = of_ths_match, > + }, > +}; > +module_platform_driver(ths_driver); > + > +MODULE_DESCRIPTION("Thermal sensor driver for Allwinner SOC"); > +MODULE_LICENSE("GPL v2"); > -- > 2.17.1 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hello, On Sat, Aug 10, 2019 at 05:28:12AM +0000, Yangtao Li wrote: > This patch adds the support for allwinner thermal sensor, within > allwinner SoC. It will register sensors for thermal framework > and use device tree to bind cooling device. I've tested this driver on H6 SoC, and it reports temperatures that are way too high. It overestimates temperature by around 15-25°C. I'm measuring the SoC temperature with IR thermometer (it reports temperatures slightly lower than real ones 2-3°C, when measuring black surfaces). I've found out that ORing 0x2f to SUN50I_THS_CTRL0 will correct this. This value is undocummented, but present in BSP: See: https://megous.com/git/linux/tree/drivers/thermal/sunxi_thermal/sunxi_thermal_sensor/sunxi_ths_driver.h?h=h6-4.9-bsp#n561 With this value set, the driver reports values 7°C above package temperature, which seems about right. regards, o. > Signed-off-by: Yangtao Li <tiny.windzz@gmail.com> > --- > MAINTAINERS | 7 + > drivers/thermal/Kconfig | 14 ++ > drivers/thermal/Makefile | 1 + > drivers/thermal/sun8i_thermal.c | 399 ++++++++++++++++++++++++++++++++ > 4 files changed, 421 insertions(+) > create mode 100644 drivers/thermal/sun8i_thermal.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index 47800d32cfbc..89dc43f4064d 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -682,6 +682,13 @@ L: linux-crypto@vger.kernel.org > S: Maintained > F: drivers/crypto/sunxi-ss/ > > +ALLWINNER THERMAL DRIVER > +M: Yangtao Li <tiny.windzz@gmail.com> > +L: linux-pm@vger.kernel.org > +S: Maintained > +F: Documentation/devicetree/bindings/thermal/sun8i-thermal.yaml > +F: drivers/thermal/sun8i_thermal.c > + > ALLWINNER VPU DRIVER > M: Maxime Ripard <maxime.ripard@bootlin.com> > M: Paul Kocialkowski <paul.kocialkowski@bootlin.com> > diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig > index 9966364a6deb..f8b73b32b92d 100644 > --- a/drivers/thermal/Kconfig > +++ b/drivers/thermal/Kconfig > @@ -262,6 +262,20 @@ config SPEAR_THERMAL > Enable this to plug the SPEAr thermal sensor driver into the Linux > thermal framework. > > +config SUN8I_THERMAL > + tristate "Allwinner sun8i thermal driver" > + depends on ARCH_SUNXI || COMPILE_TEST > + depends on HAS_IOMEM > + depends on NVMEM > + depends on OF > + depends on RESET_CONTROLLER > + help > + Support for the sun8i thermal sensor driver into the Linux thermal > + framework. > + > + To compile this driver as a module, choose M here: the > + module will be called sun8i-thermal. > + > config ROCKCHIP_THERMAL > tristate "Rockchip thermal driver" > depends on ARCH_ROCKCHIP || COMPILE_TEST > diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile > index 74a37c7f847a..fa6f8b206281 100644 > --- a/drivers/thermal/Makefile > +++ b/drivers/thermal/Makefile > @@ -31,6 +31,7 @@ thermal_sys-$(CONFIG_DEVFREQ_THERMAL) += devfreq_cooling.o > obj-y += broadcom/ > obj-$(CONFIG_THERMAL_MMIO) += thermal_mmio.o > obj-$(CONFIG_SPEAR_THERMAL) += spear_thermal.o > +obj-$(CONFIG_SUN8I_THERMAL) += sun8i_thermal.o > obj-$(CONFIG_ROCKCHIP_THERMAL) += rockchip_thermal.o > obj-$(CONFIG_RCAR_THERMAL) += rcar_thermal.o > obj-$(CONFIG_RCAR_GEN3_THERMAL) += rcar_gen3_thermal.o > diff --git a/drivers/thermal/sun8i_thermal.c b/drivers/thermal/sun8i_thermal.c > new file mode 100644 > index 000000000000..2ce36fa3fec3 > --- /dev/null > +++ b/drivers/thermal/sun8i_thermal.c > @@ -0,0 +1,399 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Thermal sensor driver for Allwinner SOC > + * Copyright (C) 2019 Yangtao Li > + * > + * Based on the work of Icenowy Zheng <icenowy@aosc.io> > + * Based on the work of Ondrej Jirman <megous@megous.com> > + * Based on the work of Josef Gajdusek <atx@atx.name> > + */ > + > +#include <linux/clk.h> > +#include <linux/device.h> > +#include <linux/interrupt.h> > +#include <linux/module.h> > +#include <linux/nvmem-consumer.h> > +#include <linux/of_device.h> > +#include <linux/platform_device.h> > +#include <linux/regmap.h> > +#include <linux/reset.h> > +#include <linux/slab.h> > +#include <linux/thermal.h> > + > +#define MAX_SENSOR_NUM 4 > + > +#define SUN50I_H6_SENSOR_NUM 2 > +#define SUN50I_H6_OFFSET -2794 > +#define SUN50I_H6_SCALE -67 > + > +#define FT_TEMP_MASK GENMASK(11, 0) > +#define TEMP_CALIB_MASK GENMASK(11, 0) > +#define TEMP_TO_REG 672 > +#define CALIBRATE_DEFAULT 0x800 > + > +#define SUN50I_THS_CTRL0 0x00 > +#define SUN50I_H6_THS_ENABLE 0x04 > +#define SUN50I_H6_THS_PC 0x08 > +#define SUN50I_H6_THS_DIC 0x10 > +#define SUN50I_H6_THS_DIS 0x20 > +#define SUN50I_H6_THS_MFC 0x30 > +#define SUN50I_H6_THS_TEMP_CALIB 0xa0 > +#define SUN50I_H6_THS_TEMP_DATA 0xc0 > + > +#define SUN50I_THS_CTRL0_T_ACQ(x) ((GENMASK(15, 0) & (x)) << 16) > +#define SUN50I_THS_FILTER_EN BIT(2) > +#define SUN50I_THS_FILTER_TYPE(x) (GENMASK(1, 0) & (x)) > +#define SUN50I_H6_THS_PC_TEMP_PERIOD(x) ((GENMASK(19, 0) & (x)) << 12) > +#define SUN50I_H6_THS_DATA_IRQ_STS(x) BIT(x) > + > +/* millidegree celsius */ > +#define SUN50I_H6_FT_DEVIATION 7000 > + > +struct ths_device; > + > +struct tsensor { > + struct ths_device *tmdev; > + struct thermal_zone_device *tzd; > + int id; > +}; > + > +struct ths_device { > + struct device *dev; > + struct regmap *regmap; > + struct reset_control *reset; > + struct clk *bus_clk; > + struct tsensor sensor[MAX_SENSOR_NUM]; > +}; > + > +/* Temp Unit: millidegree Celsius */ > +static int sun8i_ths_reg2temp(struct ths_device *tmdev, > + int reg) > +{ > + return (reg + SUN50I_H6_OFFSET) * SUN50I_H6_SCALE; > +} > + > +static int sun8i_ths_get_temp(void *data, int *temp) > +{ > + struct tsensor *s = data; > + struct ths_device *tmdev = s->tmdev; > + int val; > + > + regmap_read(tmdev->regmap, SUN50I_H6_THS_TEMP_DATA + > + 0x4 * s->id, &val); > + > + /* ths have no data yet */ > + if (!val) > + return -EAGAIN; > + > + *temp = sun8i_ths_reg2temp(tmdev, val); > + /* > + * XX - According to the original sdk, there are some platforms(rarely) > + * that add a fixed offset value after calculating the temperature > + * value. We can't simply put it on the formula for calculating the > + * temperature above, because the formula for calculating the > + * temperature above is also used when the sensor is calibrated. If > + * do this, the correct calibration formula is hard to know. > + */ > + *temp += SUN50I_H6_FT_DEVIATION; > + > + return 0; > +} > + > +static const struct thermal_zone_of_device_ops ths_ops = { > + .get_temp = sun8i_ths_get_temp, > +}; > + > +static const struct regmap_config config = { > + .reg_bits = 32, > + .val_bits = 32, > + .reg_stride = 4, > + .fast_io = true, > +}; > + > +static irqreturn_t sun50i_h6_irq_thread(int irq, void *data) > +{ > + struct ths_device *tmdev = data; > + int i, state; > + > + regmap_read(tmdev->regmap, SUN50I_H6_THS_DIS, &state); > + > + for (i = 0; i < SUN50I_H6_SENSOR_NUM; i++) { > + > + if (state & SUN50I_H6_THS_DATA_IRQ_STS(i)) { > + /* clear data irq pending */ > + regmap_write(tmdev->regmap, SUN50I_H6_THS_DIS, > + SUN50I_H6_THS_DATA_IRQ_STS(i)); > + > + thermal_zone_device_update(tmdev->sensor[i].tzd, > + THERMAL_EVENT_UNSPECIFIED); > + } > + } > + > + return IRQ_HANDLED; > +} > + > +static int sun50i_ths_calibrate(struct ths_device *tmdev) > +{ > + struct nvmem_cell *calcell; > + struct device *dev = tmdev->dev; > + u16 *caldata; > + size_t callen; > + int ft_temp; > + int i, ret = 0; > + > + calcell = devm_nvmem_cell_get(dev, "calib"); > + if (IS_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 > + * the data won't be so accurate. > + * > + * The default value of calibration register is 0x800 for > + * every sensor, and the calibration value is usually 0x7xx > + * or 0x8xx, so they won't be away from the default value > + * for a lot. > + * > + * So here we do not return error if the calibartion data is > + * not available, except the probe needs deferring. > + */ > + goto out; > + } > + > + caldata = nvmem_cell_read(calcell, &callen); > + if (IS_ERR(caldata)) { > + ret = PTR_ERR(caldata); > + goto out; > + } > + > + if (!caldata[0] || callen < 2 + 2 * SUN50I_H6_SENSOR_NUM) { > + ret = -EINVAL; > + goto out_free; > + } > + > + /* > + * efuse layout: > + * > + * 0 11 16 32 > + * +-------+-------+-------+ > + * |temp| |sensor0|sensor1| > + * +-------+-------+-------+ > + * > + * The calibration data on the H6 is the ambient temperature and > + * sensor values that are filled during the factory test stage. > + * > + * The unit of stored FT temperature is 0.1 degreee celusis. > + * Through the stored ambient temperature and the data read > + * by the sensor, after a certain calculation, the calibration > + * value to be compensated can be obtained. > + */ > + ft_temp = caldata[0] & FT_TEMP_MASK; > + > + for (i = 0; i < SUN50I_H6_SENSOR_NUM; i++) { > + int reg = (int)caldata[i + 1]; > + int sensor_temp = sun8i_ths_reg2temp(tmdev, reg); > + int delta, cdata, offset; > + > + /* > + * To calculate the calibration value: > + * > + * X(in Celsius) = Ts - ft_temp > + * delta = X * 10000 / TEMP_TO_REG > + * cdata = CALIBRATE_DEFAULT - delta > + * > + * cdata: calibration value > + */ > + delta = (sensor_temp - ft_temp * 100) * 10 / TEMP_TO_REG; > + cdata = CALIBRATE_DEFAULT - delta; > + if (cdata & ~TEMP_CALIB_MASK) { > + /* > + * Calibration value more than 12-bit, but calibration > + * register is 12-bit. In this case, ths hardware can > + * still work without calibration, although the data > + * won't be so accurate. > + */ > + dev_warn(dev, "sensor%d is not calibrated.\n", i); > + > + continue; > + } > + > + offset = (i % 2) << 4; > + regmap_update_bits(tmdev->regmap, > + SUN50I_H6_THS_TEMP_CALIB + ((i >> 1) * 4), > + 0xfff << offset, > + cdata << offset); > + } > + > +out_free: > + kfree(caldata); > +out: > + return ret; > +} > + > +static int sun8i_ths_resource_init(struct ths_device *tmdev) > +{ > + struct device *dev = tmdev->dev; > + struct platform_device *pdev = to_platform_device(dev); > + struct resource *mem; > + void __iomem *base; > + int ret; > + > + mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + base = devm_ioremap_resource(dev, mem); > + if (IS_ERR(base)) > + return PTR_ERR(base); > + > + tmdev->regmap = devm_regmap_init_mmio(dev, base, &config); > + if (IS_ERR(tmdev->regmap)) > + return PTR_ERR(tmdev->regmap); > + > + tmdev->reset = devm_reset_control_get(dev, 0); > + if (IS_ERR(tmdev->reset)) > + return PTR_ERR(tmdev->reset); > + > + tmdev->bus_clk = devm_clk_get(&pdev->dev, "bus"); > + if (IS_ERR(tmdev->bus_clk)) > + return PTR_ERR(tmdev->bus_clk); > + > + ret = reset_control_deassert(tmdev->reset); > + if (ret) > + return ret; > + > + ret = clk_prepare_enable(tmdev->bus_clk); > + if (ret) > + goto assert_reset; > + > + ret = sun50i_ths_calibrate(tmdev); > + if (ret) > + goto bus_disable; > + > + return 0; > + > +bus_disable: > + clk_disable_unprepare(tmdev->bus_clk); > +assert_reset: > + reset_control_assert(tmdev->reset); > + > + return ret; > +} > + > +static int sun50i_h6_thermal_init(struct ths_device *tmdev) > +{ > + int val; > + > + /* > + * clkin = 24MHz > + * T acquire = clkin / (x + 1) > + * = 20us > + */ > + regmap_write(tmdev->regmap, SUN50I_THS_CTRL0, > + SUN50I_THS_CTRL0_T_ACQ(479)); > + /* average over 4 samples */ > + regmap_write(tmdev->regmap, SUN50I_H6_THS_MFC, > + SUN50I_THS_FILTER_EN | > + SUN50I_THS_FILTER_TYPE(1)); > + /* period = (x + 1) * 4096 / clkin; ~10ms */ > + regmap_write(tmdev->regmap, SUN50I_H6_THS_PC, > + SUN50I_H6_THS_PC_TEMP_PERIOD(58)); > + /* enable sensor */ > + val = GENMASK(SUN50I_H6_SENSOR_NUM - 1, 0); > + regmap_write(tmdev->regmap, SUN50I_H6_THS_ENABLE, val); > + /* thermal data interrupt enable */ > + val = GENMASK(SUN50I_H6_SENSOR_NUM - 1, 0); > + regmap_write(tmdev->regmap, SUN50I_H6_THS_DIC, val); > + > + return 0; > +} > + > +static int sun8i_ths_register(struct ths_device *tmdev) > +{ > + struct thermal_zone_device *tzd; > + int i; > + > + for (i = 0; i < SUN50I_H6_SENSOR_NUM; i++) { > + tmdev->sensor[i].tmdev = tmdev; > + tmdev->sensor[i].id = i; > + tmdev->sensor[i].tzd = > + devm_thermal_zone_of_sensor_register(tmdev->dev, > + i, > + &tmdev->sensor[i], > + &ths_ops); > + if (IS_ERR(tmdev->sensor[i].tzd)) > + return PTR_ERR(tzd); > + } > + > + return 0; > +} > + > +static int sun8i_ths_probe(struct platform_device *pdev) > +{ > + struct ths_device *tmdev; > + struct device *dev = &pdev->dev; > + int ret, irq; > + > + tmdev = devm_kzalloc(dev, sizeof(*tmdev), GFP_KERNEL); > + if (!tmdev) > + return -ENOMEM; > + > + tmdev->dev = dev; > + platform_set_drvdata(pdev, tmdev); > + > + ret = sun8i_ths_resource_init(tmdev); > + if (ret) > + return ret; > + > + irq = platform_get_irq(pdev, 0); > + if (irq < 0) > + return irq; > + > + ret = sun50i_h6_thermal_init(tmdev); > + if (ret) > + return ret; > + > + ret = sun8i_ths_register(tmdev); > + if (ret) > + return ret; > + > + /* > + * Avoid entering the interrupt handler, the thermal device is not > + * registered yet, we deffer the registration of the interrupt to > + * the end. > + */ > + ret = devm_request_threaded_irq(dev, irq, NULL, > + sun50i_h6_irq_thread, > + IRQF_ONESHOT, "ths", tmdev); > + if (ret) > + return ret; > + > + return ret; > +} > + > +static int sun8i_ths_remove(struct platform_device *pdev) > +{ > + struct ths_device *tmdev = platform_get_drvdata(pdev); > + > + clk_disable_unprepare(tmdev->bus_clk); > + reset_control_assert(tmdev->reset); > + > + return 0; > +} > + > +static const struct of_device_id of_ths_match[] = { > + { .compatible = "allwinner,sun50i-h6-ths"}, > + { /* sentinel */ }, > +}; > +MODULE_DEVICE_TABLE(of, of_ths_match); > + > +static struct platform_driver ths_driver = { > + .probe = sun8i_ths_probe, > + .remove = sun8i_ths_remove, > + .driver = { > + .name = "sun8i-thermal", > + .of_match_table = of_ths_match, > + }, > +}; > +module_platform_driver(ths_driver); > + > +MODULE_DESCRIPTION("Thermal sensor driver for Allwinner SOC"); > +MODULE_LICENSE("GPL v2"); > -- > 2.17.1 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hello Yangtao, On Sat, Aug 10, 2019 at 05:28:11AM +0000, Yangtao Li wrote: > This patchset add support for A64, H3, H5, H6 and R40 thermal sensor. > > Thx to Icenowy and Vasily. > > BTY, do a cleanup in thermal makfile. I've added support for A83T and also some cleanups, according to my feedback: https://megous.com/git/linux/log/?h=ths-5.3 Feel free to pick up whatever you like from that tree. For others, there are also DTS patches in that tree for H3, H5, A83T, and H6, so that shoul make testing of this driver easier. regards, Ondrej > Icenowy Zheng (3): > thermal: sun8i: allow to use custom temperature calculation function > thermal: sun8i: add support for Allwinner H5 thermal sensor > thermal: sun8i: add support for Allwinner R40 thermal sensor > > Vasily Khoruzhick (1): > thermal: sun8i: add thermal driver for A64 > > Yangtao Li (14): > thermal: sun8i: add thermal driver for h6 > dt-bindings: thermal: add binding document for h6 thermal controller > thermal: fix indentation in makefile > thermal: sun8i: get ths sensor number from device compatible > thermal: sun8i: rework for sun8i_ths_get_temp() > thermal: sun8i: get ths init func from device compatible > thermal: sun8i: rework for ths irq handler func > thermal: sun8i: support mod clocks > thermal: sun8i: rework for ths calibrate func > dt-bindings: thermal: add binding document for h3 thermal controller > thermal: sun8i: add thermal driver for h3 > dt-bindings: thermal: add binding document for a64 thermal controller > dt-bindings: thermal: add binding document for h5 thermal controller > dt-bindings: thermal: add binding document for r40 thermal controller > > .../bindings/thermal/sun8i-thermal.yaml | 157 +++++ > MAINTAINERS | 7 + > drivers/thermal/Kconfig | 14 + > drivers/thermal/Makefile | 9 +- > drivers/thermal/sun8i_thermal.c | 596 ++++++++++++++++++ > 5 files changed, 779 insertions(+), 4 deletions(-) > create mode 100644 Documentation/devicetree/bindings/thermal/sun8i-thermal.yaml > create mode 100644 drivers/thermal/sun8i_thermal.c > > --- > v5: > -add more support > -some trival fix > --- > 2.17.1 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi, On Sun, Sep 01, 2019 at 11:52:14PM +0200, Ondřej Jirman wrote: > Hello Yangtao, > > On Sat, Aug 10, 2019 at 05:28:11AM +0000, Yangtao Li wrote: > > This patchset add support for A64, H3, H5, H6 and R40 thermal sensor. > > > > Thx to Icenowy and Vasily. > > > > BTY, do a cleanup in thermal makfile. > > I've added support for A83T and also some cleanups, according to my > feedback: > > https://megous.com/git/linux/log/?h=ths-5.3 > > Feel free to pick up whatever you like from that tree. > > For others, there are also DTS patches in that tree for H3, H5, A83T, and H6, so > that shoul make testing of this driver easier. I'm not convinced that always expanding the number of SoC supported is the best strategy to get this merged. Usually, keeping the same feature set across version, consolidating that, and then once it's in sending the new SoC support works best. Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Hello Maxime, On Mon, Sep 02, 2019 at 09:27:35AM +0200, Maxime Ripard wrote: > Hi, > > On Sun, Sep 01, 2019 at 11:52:14PM +0200, Ondřej Jirman wrote: > > Hello Yangtao, > > > > On Sat, Aug 10, 2019 at 05:28:11AM +0000, Yangtao Li wrote: > > > This patchset add support for A64, H3, H5, H6 and R40 thermal sensor. > > > > > > Thx to Icenowy and Vasily. > > > > > > BTY, do a cleanup in thermal makfile. > > > > I've added support for A83T and also some cleanups, according to my > > feedback: > > > > https://megous.com/git/linux/log/?h=ths-5.3 > > > > Feel free to pick up whatever you like from that tree. > > > > For others, there are also DTS patches in that tree for H3, H5, A83T, and H6, so > > that shoul make testing of this driver easier. > > I'm not convinced that always expanding the number of SoC supported is > the best strategy to get this merged. Usually, keeping the same > feature set across version, consolidating that, and then once it's in > sending the new SoC support works best. That's fine and all, but I've mostly added DT descriptions for already supported SoCs and fixed bugs in the driver, so that people can actually test the existing driver. I think adding DT changes will actually help get needed exposure for this patch series. A83T support that I added, was actually just a small change to the driver. regards, o. > Maxime > > -- > Maxime Ripard, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Sun, Aug 25, 2019 at 9:14 AM Frank Lee <tiny.windzz@gmail.com> wrote: > > HI Vasily, Hi Yangtao, Sorry for the late reply, > On Wed, Aug 14, 2019 at 11:01 AM Vasily Khoruzhick <anarsoul@gmail.com> wrote: > > > > On Tue, Aug 13, 2019 at 1:06 PM Ondřej Jirman <megous@megous.com> wrote: > > > > > > On Mon, Aug 12, 2019 at 04:54:15PM -0700, Vasily Khoruzhick wrote: > > > > On Mon, Aug 12, 2019 at 4:46 PM Frank Lee <tiny.windzz@gmail.com> wrote: > > > > > > > > > > HI Vasily, > > > > > > > > > > On Sat, Aug 10, 2019 at 2:17 PM Vasily Khoruzhick <anarsoul@gmail.com> wrote: > > > > > > > > > > > > On Fri, Aug 9, 2019 at 10:31 PM Yangtao Li <tiny.windzz@gmail.com> wrote: > > > > > > > > > > > > > > H3 has extra clock, so introduce something in ths_thermal_chip/ths_device > > > > > > > and adds the process of the clock. > > > > > > > > > > > > > > This is pre-work for supprt it. > > > > > > > > > > > > > > Signed-off-by: Yangtao Li <tiny.windzz@gmail.com> > > > > > > > --- > > > > > > > drivers/thermal/sun8i_thermal.c | 17 ++++++++++++++++- > > > > > > > 1 file changed, 16 insertions(+), 1 deletion(-) > > > > > > > > > > > > > > diff --git a/drivers/thermal/sun8i_thermal.c b/drivers/thermal/sun8i_thermal.c > > > > > > > index b934bc81eba7..6f4294c2aba7 100644 > > > > > > > --- a/drivers/thermal/sun8i_thermal.c > > > > > > > +++ b/drivers/thermal/sun8i_thermal.c > > > > > > > @@ -54,6 +54,7 @@ struct tsensor { > > > > > > > }; > > > > > > > > > > > > > > struct ths_thermal_chip { > > > > > > > + bool has_mod_clk; > > > > > > > int sensor_num; > > > > > > > int offset; > > > > > > > int scale; > > > > > > > @@ -69,6 +70,7 @@ struct ths_device { > > > > > > > struct regmap *regmap; > > > > > > > struct reset_control *reset; > > > > > > > struct clk *bus_clk; > > > > > > > + struct clk *mod_clk; > > > > > > > struct tsensor sensor[MAX_SENSOR_NUM]; > > > > > > > }; > > > > > > > > > > > > > > @@ -274,6 +276,12 @@ static int sun8i_ths_resource_init(struct ths_device *tmdev) > > > > > > > if (IS_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)) > > > > > > > + return PTR_ERR(tmdev->mod_clk); > > > > > > > + } > > > > > > > + > > > > > > > ret = reset_control_deassert(tmdev->reset); > > > > > > > if (ret) > > > > > > > return ret; > > > > > > > @@ -282,12 +290,18 @@ static int sun8i_ths_resource_init(struct ths_device *tmdev) > > > > > > > if (ret) > > > > > > > goto assert_reset; > > > > > > > > > > > > > > - ret = sun50i_ths_calibrate(tmdev); > > > > > > > + ret = clk_prepare_enable(tmdev->mod_clk); > > > > > > > > > > > > You have to set rate of modclk before enabling it since you can't rely > > > > > > on whatever bootloader left for you. > > > > > > > > > > > > Also I found that parameters you're using for PC_TEMP_PERIOD, ACQ0 and > > > > > > ACQ1 are too aggressive and may result in high interrupt rate to the > > > > > > point when it may stall RCU. I changed driver a bit to use params from > > > > > > Philipp Rossak's work (modclk set to 4MHz, PC_TEMP_PERIOD is 7, ACQ0 > > > > > > is 255, ACQ1 is 63) and it fixed RCU stalls for me, see [1] for > > > > > > details. > > > > > > > > > > Why is the RCU stall happening, is it caused by a deadlock? > > > > > Can you provide log information and your configuration? > > > > > I am a bit curious. > > > > > > > > It's not deadlock, I believe it just can't handle that many interrupts > > > > when running at lowest CPU frequency. Even with Philipp's settings > > > > there's ~20 interrupts a second from ths. I don't remember how many > > > > interrupts were there with your settings. > > > > > > > > Unfortunately there's nothing interesting in backtraces, I'm using > > > > Pine64-LTS board. > > > > > > Recently there was a similar issue, with buggy CCU driver that caused > > > CIR interrupts being fired constantly, and it also resulted in RCU > > > stalls. Looks like a comon cause of RCU stalls. > > > > > > THS timing settings probably need to be made specific to the SoC, because > > > I noticed that the same settings lead to wildly different timings on > > > different SoCs. > > > > > > It would be good to measure how often ths interrupt fires with this driver > > > on various SoCs. > > > > > > 20 times a second and more sounds like overkill. I'd expect a useful > > > range to be at most 5-10 times a second. That should be enough to stop > > > overheating the SoC due to suddenly increased load, even without a > > > heatsink. > > > > Note that A64 has 3 sensors and each sensor has individual interrupt, > > so technically it's 6-7 interrupts per sensor per second > > You only need to increase the value of the period to reduce the number > of interrupts. > Can you test the relationship between the period and the number of interrupts > when the mod clock does not change and stays 24M? I played a bit with your settings and 24M, with PERIOD = 57 I get 26 interrupts / second with 87 - 18 interrupts / second with 116 - 12-15 interrupts / second. I think we should use 116 for A64 since with it we get reasonable number of ths interrupts in a second. Regards, Vasily > Thx. > Yangtao > > > > > > regards, > > > o. > > > > > > > > Thx, > > > > > Yangtao > > > > > > > > > > > > > > > > > [1] https://github.com/anarsoul/linux-2.6/commit/46b8bb0fe2ccd1cd88fa9181a2ecbf79e8d513b2 > > > > > > > > > > > > > > > > > > > if (ret) > > > > > > > goto bus_disable; > > > > > > > > > > > > > > + ret = sun50i_ths_calibrate(tmdev); > > > > > > > + if (ret) > > > > > > > + goto mod_disable; > > > > > > > + > > > > > > > return 0; > > > > > > > > > > > > > > +mod_disable: > > > > > > > + clk_disable_unprepare(tmdev->mod_clk); > > > > > > > bus_disable: > > > > > > > clk_disable_unprepare(tmdev->bus_clk); > > > > > > > assert_reset: > > > > > > > @@ -395,6 +409,7 @@ static int sun8i_ths_remove(struct platform_device *pdev) > > > > > > > { > > > > > > > struct ths_device *tmdev = platform_get_drvdata(pdev); > > > > > > > > > > > > > > + clk_disable_unprepare(tmdev->mod_clk); > > > > > > > clk_disable_unprepare(tmdev->bus_clk); > > > > > > > reset_control_assert(tmdev->reset); > > > > > > > > > > > > > > -- > > > > > > > 2.17.1 > > > > > > > > > > > > > > > > > > > > > _______________________________________________ > > > > > > > linux-arm-kernel mailing list > > > > > > > linux-arm-kernel@lists.infradead.org > > > > > > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > > > > > > > > _______________________________________________ > > > > linux-arm-kernel mailing list > > > > linux-arm-kernel@lists.infradead.org > > > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Mon, Sep 2, 2019 at 3:58 AM Ondřej Jirman <megous@megous.com> wrote: > > Hello Maxime, > > On Mon, Sep 02, 2019 at 09:27:35AM +0200, Maxime Ripard wrote: > > Hi, > > > > On Sun, Sep 01, 2019 at 11:52:14PM +0200, Ondřej Jirman wrote: > > > Hello Yangtao, > > > > > > On Sat, Aug 10, 2019 at 05:28:11AM +0000, Yangtao Li wrote: > > > > This patchset add support for A64, H3, H5, H6 and R40 thermal sensor. > > > > > > > > Thx to Icenowy and Vasily. > > > > > > > > BTY, do a cleanup in thermal makfile. Hey Yangtao, Are there any plans for v6? Regards, Vasily > > > I've added support for A83T and also some cleanups, according to my > > > feedback: > > > > > > https://megous.com/git/linux/log/?h=ths-5.3 > > > > > > Feel free to pick up whatever you like from that tree. > > > > > > For others, there are also DTS patches in that tree for H3, H5, A83T, and H6, so > > > that shoul make testing of this driver easier. > > > > I'm not convinced that always expanding the number of SoC supported is > > the best strategy to get this merged. Usually, keeping the same > > feature set across version, consolidating that, and then once it's in > > sending the new SoC support works best. > > That's fine and all, but I've mostly added DT descriptions for already supported > SoCs and fixed bugs in the driver, so that people can actually test the existing > driver. > > I think adding DT changes will actually help get needed exposure for this > patch series. > > A83T support that I added, was actually just a small change to the driver. > > regards, > o. > > > Maxime > > > > -- > > Maxime Ripard, Bootlin > > Embedded Linux and Kernel engineering > > https://bootlin.com > > > > _______________________________________________ > > linux-arm-kernel mailing list > > linux-arm-kernel@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel