Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1725960

Re: [PATCH 08/13] thermal/drivers/hisi: Fix configuration register setting

Show all headers | View raw

On 04/09/2017 02:58, Leo Yan wrote:
> On Sat, Sep 02, 2017 at 10:34:34AM +0200, Daniel Lezcano wrote:
>> On 02/09/2017 04:54, Leo Yan wrote:
>>> On Wed, Aug 30, 2017 at 10:47:32AM +0200, Daniel Lezcano wrote:
>>>> The TEMP0_CFG configuration register contains different field to set up the
>>>> temperature controller. However in the code, nothing prevents a setup to
>>>> overwrite the previous one: eg. writing the hdak value overwrites the sensor
>>>> selection, the sensor selection overwrites the hdak value.
>>>>
>>>> In order to prevent such thing, use a regmap-like mechanism by reading the
>>>> value before, set the corresponding bits and write the result.
>>>>
>>>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>>>> ---
>>>>  drivers/thermal/hisi_thermal.c | 30 +++++++++++++++++++++++++-----
>>>>  1 file changed, 25 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/drivers/thermal/hisi_thermal.c b/drivers/thermal/hisi_thermal.c
>>>> index d77a938..3e03908 100644
>>>> --- a/drivers/thermal/hisi_thermal.c
>>>> +++ b/drivers/thermal/hisi_thermal.c
>>>> @@ -132,19 +132,39 @@ static inline void hisi_thermal_enable(void __iomem *addr, int value)
>>>>  	writel(value, addr + TEMP0_EN);
>>>>  }
>>>>  
>>>> -static inline void hisi_thermal_sensor_select(void __iomem *addr, int sensor)
>>>> +static inline int hisi_thermal_get_temperature(void __iomem *addr)
>>>>  {
>>>> -	writel((sensor << 12), addr + TEMP0_CFG);
>>>> +	return hisi_thermal_step_to_temp(readl(addr + TEMP0_VALUE));
>>>>  }
>>>>  
>>>> -static inline int hisi_thermal_get_temperature(void __iomem *addr)
>>>> +/*
>>>> + * Temperature configuration register - Sensor selection
>>>> + *
>>>> + * Bits [19:12]
>>>> + *
>>>> + * 0x0: local sensor (default)
>>>> + * 0x1: remote sensor 1 (ACPU cluster 1)
>>>> + * 0x2: remote sensor 2 (ACPU cluster 0)
>>>> + * 0x3: remote sensor 3 (G3D)
>>>> + */
>>>> +static inline void hisi_thermal_sensor_select(void __iomem *addr, int sensor)
>>>>  {
>>>> -	return hisi_thermal_step_to_temp(readl(addr + TEMP0_VALUE));
>>>> +	writel(readl(addr + TEMP0_CFG) | (sensor << 12), addr + TEMP0_CFG);
>>>
>>> nitpick: maybe it's better to firstly clear related bits and then set
>>> value?
>>
>> Sorry, I don't get the comment. Can you elaborate ?
> 
> Sure, here I am bit concern there the mixing old bits value and new
> setting bits. My suggested code likes below:
> 
>   u32 val;
> 
>   val = readl(addr + TEMP0_CFG);
>   val &= ~0xF000;
>   val |= (sensor << 12);
>   writel(val, addr + TEMP0_CFG);

Oh, yes. Good catch.


-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH 01/13] thermal/drivers/hisi: Fix missing interrupt enablement Daniel Lezcano <daniel.lezcano@linaro.org> - 2017-08-30 10:50 +0200
  [PATCH 02/13] thermal/drivers/hisi: Remove the multiple sensors support Daniel Lezcano <daniel.lezcano@linaro.org> - 2017-08-30 10:50 +0200
    Re: [PATCH 02/13] thermal/drivers/hisi: Remove the multiple sensors  support Leo Yan <leo.yan@linaro.org> - 2017-09-01 16:10 +0200
      Re: [PATCH 02/13] thermal/drivers/hisi: Remove the multiple sensors  support Daniel Lezcano <daniel.lezcano@linaro.org> - 2017-09-01 22:50 +0200
  [PATCH 06/13] thermal/drivers/hisi: Remove pointless lock Daniel Lezcano <daniel.lezcano@linaro.org> - 2017-08-30 11:00 +0200
    Re: [PATCH 06/13] thermal/drivers/hisi: Remove pointless lock Leo Yan <leo.yan@linaro.org> - 2017-09-01 16:50 +0200
  [PATCH 03/13] thermal/drivers/hisi: Fix kernel panic on alarm interrupt Daniel Lezcano <daniel.lezcano@linaro.org> - 2017-08-30 11:00 +0200
    Re: [PATCH 03/13] thermal/drivers/hisi: Fix kernel panic on alarm  interrupt Leo Yan <leo.yan@linaro.org> - 2017-09-01 16:20 +0200
  [PATCH 08/13] thermal/drivers/hisi: Fix configuration register setting Daniel Lezcano <daniel.lezcano@linaro.org> - 2017-08-30 11:00 +0200
    Re: [PATCH 08/13] thermal/drivers/hisi: Fix configuration register  setting Leo Yan <leo.yan@linaro.org> - 2017-09-02 05:00 +0200
      Re: [PATCH 08/13] thermal/drivers/hisi: Fix configuration register  setting Daniel Lezcano <daniel.lezcano@linaro.org> - 2017-09-02 10:40 +0200
        Re: [PATCH 08/13] thermal/drivers/hisi: Fix configuration register  setting Leo Yan <leo.yan@linaro.org> - 2017-09-04 03:00 +0200
          Re: [PATCH 08/13] thermal/drivers/hisi: Fix configuration register  setting Daniel Lezcano <daniel.lezcano@linaro.org> - 2017-09-04 11:20 +0200
  [PATCH 09/13] thermal/drivers/hisi: Remove costly sensor inspection Daniel Lezcano <daniel.lezcano@linaro.org> - 2017-08-30 11:00 +0200
    Re: [PATCH 09/13] thermal/drivers/hisi: Remove costly sensor  inspection Leo Yan <leo.yan@linaro.org> - 2017-09-02 05:30 +0200
      Re: [PATCH 09/13] thermal/drivers/hisi: Remove costly sensor  inspection Daniel Lezcano <daniel.lezcano@linaro.org> - 2017-09-02 15:20 +0200
        Re: [PATCH 09/13] thermal/drivers/hisi: Remove costly sensor  inspection Leo Yan <leo.yan@linaro.org> - 2017-09-04 03:00 +0200
          Re: [PATCH 09/13] thermal/drivers/hisi: Remove costly sensor  inspection Daniel Lezcano <daniel.lezcano@linaro.org> - 2017-09-04 13:30 +0200
            Re: [PATCH 09/13] thermal/drivers/hisi: Remove costly sensor  inspection Leo Yan <leo.yan@linaro.org> - 2017-09-04 16:40 +0200
  [PATCH 05/13] thermal/drivers/hisi: Fix multiple alarm interrupts firing Daniel Lezcano <daniel.lezcano@linaro.org> - 2017-08-30 11:00 +0200
    Re: [PATCH 05/13] thermal/drivers/hisi: Fix multiple alarm  interrupts firing Leo Yan <leo.yan@linaro.org> - 2017-09-01 16:50 +0200
  Re: [PATCH 01/13] thermal/drivers/hisi: Fix missing interrupt  enablement Leo Yan <leo.yan@linaro.org> - 2017-09-01 10:40 +0200

csiph-web