From: gregory.clement@free-electrons•com (Gregory CLEMENT)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH v3 06/11] thermal: armada: Add support for Armada CP110
Date: Thu, 14 Dec 2017 12:37:32 +0100 [thread overview]
Message-ID: <87a7ylo803.fsf@free-electrons.com> (raw)
In-Reply-To: <20171214123301.1729df4f@xps13> (Miquel RAYNAL's message of "Thu, 14 Dec 2017 12:33:01 +0100")
Hi Miquel,
On jeu., d?c. 14 2017, Miquel RAYNAL <miquel.raynal@free-electrons•com> wrote:
> On Thu, 14 Dec 2017 12:11:49 +0100
> Gregory CLEMENT <gregory.clement@free-electrons•com> wrote:
>
>> Hi Miquel,
>>
>> On jeu., d?c. 14 2017, Miquel Raynal
>> <miquel.raynal@free-electrons•com> wrote:
>>
>> > From: Baruch Siach <baruch@tkos•co.il>
>> >
>> > The CP110 component is integrated in the Armada 8k and 7k lines of
>> > processors.
>> >
>> > Signed-off-by: Baruch Siach <baruch@tkos•co.il>
>> > [<miquel.raynal@free-electrons•com>: renamed the register
>> > pointers]
>>
>> Actually you did more thant this see below
>>
>>
>> > Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons•com>
>> > ---
>> > drivers/thermal/armada_thermal.c | 30
>> > ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6
>> > deletions(-)
>> >
>> > diff --git a/drivers/thermal/armada_thermal.c
>> > b/drivers/thermal/armada_thermal.c index 279d01937bb8..f5c911524656
>> > 100644 --- a/drivers/thermal/armada_thermal.c
>> > +++ b/drivers/thermal/armada_thermal.c
>> > @@ -37,7 +37,6 @@
>> > #define A375_UNIT_CONTROL_MASK 0x7
>> > #define A375_READOUT_INVERT BIT(15)
>> > #define A375_HW_RESETn BIT(8)
>> > -#define A380_HW_RESET BIT(8)
>> >
>> > /* Legacy bindings */
>> > #define LEGACY_CONTROL_MEM_LEN 0x4
>> > @@ -52,6 +51,10 @@
>> > #define CONTROL0_TSEN_RESET BIT(1)
>> > #define CONTROL0_TSEN_ENABLE BIT(2)
>> >
>> > +/* EXT_TSEN refers to the external temperature sensors, out of the
>> > AP */ +#define CONTROL1_EXT_TSEN_SW_RESET BIT(7)
>> > +#define CONTROL1_EXT_TSEN_HW_RESETn BIT(8)
>> You added or rename these values
>>
>> > +
>> > struct armada_thermal_data;
>> >
>> > /* Marvell EBU Thermal Sensor Dev Structure */
>> > @@ -153,11 +156,10 @@ static void armada380_init_sensor(struct
>> > platform_device *pdev, u32 reg = readl_relaxed(priv->control1);
>> >
>> > /* Reset hardware once */
>> > - if (!(reg & A380_HW_RESET)) {
>> > - reg |= A380_HW_RESET;
>> > - writel(reg, priv->control1);
>> > - msleep(10);
>> > - }
>> > + reg |= CONTROL1_EXT_TSEN_HW_RESETn;
>> > + reg &= ~CONTROL1_EXT_TSEN_SW_RESET;
>> > + writel(reg, priv->control1);
>>
>> And here you modified the behavior of this function.
>> Did you checked that it is valid for Armada 38x?
>
> There is nothing about it the documentation and anyway this register
> can be accessed later, so writing it is harmless ayway.
>
>>
>> Given the comment we had, I thought we should not do anything if
>> CONTROL1_EXT_TSEN_HW_RESETn was not set.
>
> That is the opposite, if it is not set (ie. reset is active), you have
> to set it (reset is then disabled).
Actually I was concerned by the "once" for me it means "only one time",
but maybe it just meant it was useless to reset it again but not
harmful.
Gregory
>
>>
>> By the way, if the new sequence is valid, this comment should be
>> removed or at least updated.
>
> That's right, I will in v4.
>
> Thanks for reviewing,
> Miqu?l
>
>
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
next prev parent reply other threads:[~2017-12-14 11:37 UTC|newest]
Thread overview: 45+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-12-14 10:30 [PATCH v3 00/11] Armada thermal: improvements and A7K/A8K SoCs support Miquel Raynal
2017-12-14 10:30 ` [PATCH v3 01/11] dt-bindings: thermal: Describe Armada AP806 and CP110 Miquel Raynal
2017-12-15 8:27 ` Baruch Siach
2017-12-15 8:32 ` Miquel RAYNAL
2017-12-15 8:44 ` Baruch Siach
2017-12-15 8:44 ` Gregory CLEMENT
2017-12-15 10:52 ` Miquel RAYNAL
2017-12-15 23:28 ` Rob Herring
2017-12-16 12:50 ` Miquel RAYNAL
2017-12-14 10:30 ` [PATCH v3 02/11] thermal: armada: Use msleep for long delays Miquel Raynal
2017-12-14 10:51 ` Gregory CLEMENT
2017-12-14 10:30 ` [PATCH v3 03/11] thermal: armada: Simplify the check of the validity bit Miquel Raynal
2017-12-14 10:52 ` Gregory CLEMENT
2017-12-15 8:33 ` Baruch Siach
2017-12-15 10:56 ` Miquel RAYNAL
2017-12-14 10:30 ` [PATCH v3 04/11] thermal: armada: Rationalize register accesses Miquel Raynal
2017-12-14 10:55 ` Gregory CLEMENT
2017-12-15 8:56 ` Baruch Siach
2017-12-18 13:48 ` Miquel RAYNAL
2017-12-16 22:18 ` Baruch Siach
2017-12-17 22:02 ` Baruch Siach
2017-12-18 12:37 ` Miquel RAYNAL
2017-12-14 10:30 ` [PATCH v3 05/11] thermal: armada: Add support for Armada AP806 Miquel Raynal
2017-12-14 11:05 ` Gregory CLEMENT
2017-12-18 9:41 ` Miquel RAYNAL
2017-12-18 11:11 ` Baruch Siach
2017-12-18 12:25 ` Miquel RAYNAL
2017-12-16 22:22 ` Baruch Siach
2017-12-14 10:30 ` [PATCH v3 06/11] thermal: armada: Add support for Armada CP110 Miquel Raynal
2017-12-14 11:11 ` Gregory CLEMENT
2017-12-14 11:33 ` Miquel RAYNAL
2017-12-14 11:37 ` Gregory CLEMENT [this message]
2017-12-14 12:24 ` Miquel RAYNAL
2017-12-14 10:30 ` [PATCH v3 07/11] thermal: armada: Update Kconfig and module description Miquel Raynal
2017-12-14 11:13 ` Gregory CLEMENT
2017-12-14 11:17 ` Miquel RAYNAL
2017-12-14 11:30 ` Gregory CLEMENT
2017-12-14 11:36 ` Miquel RAYNAL
2017-12-14 13:10 ` Thomas Petazzoni
2017-12-14 10:30 ` [PATCH v3 08/11] thermal: armada: Change sensors trim default value Miquel Raynal
2017-12-14 10:30 ` [PATCH v3 09/11] thermal: armada: Wait sensors validity before exiting the init callback Miquel Raynal
2017-12-14 11:23 ` Gregory CLEMENT
2017-12-14 11:27 ` Miquel RAYNAL
2017-12-14 10:30 ` [PATCH v3 10/11] thermal: armada: Give useful names to the thermal zone Miquel Raynal
2017-12-14 10:30 ` [PATCH v3 11/11] ARM64: dts: marvell: Add thermal support for A7K/A8K Miquel Raynal
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87a7ylo803.fsf@free-electrons.com \
--to=gregory.clement@free-electrons$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox