linux-kernel - Re: [PATCH 03/15] mfd: Add the header file of Nuvoton ma35d1 system manager

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 17 Mar 2023 17:28:03 +0800
From:   Jacky Huang <ychuang570808@...il.com>
To:     Arnd Bergmann <arnd@...db.de>, Rob Herring <robh+dt@...nel.org>,
        krzysztof.kozlowski+dt@...aro.org, Lee Jones <lee@...nel.org>,
        Michael Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...nel.org>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jiri Slaby <jirislaby@...nel.org>
Cc:     devicetree@...r.kernel.org, linux-clk@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-serial@...r.kernel.org,
        schung@...oton.com, Jacky Huang <ychuang3@...oton.com>
Subject: Re: [PATCH 03/15] mfd: Add the header file of Nuvoton ma35d1 system
 manager

Dear Arnd,

Thanks for your advice.


On 2023/3/16 下午 10:44, Arnd Bergmann wrote:
> On Wed, Mar 15, 2023, at 08:28, Jacky Huang wrote:
>> From: Jacky Huang <ychuang3@...oton.com>
>>
>> The system manager is a set of registers used for power control,
>> multi-function pin control, USB phy control, IP reset, and other
>> miscellaneous controls. It also contains some registers that
>> provide SoC information and status.
>>
>> Signed-off-by: Jacky Huang <ychuang3@...oton.com>
>> ---
>>   include/linux/mfd/ma35d1-sys.h | 95 ++++++++++++++++++++++++++++++++++
>>   1 file changed, 95 insertions(+)
>>   create mode 100644 include/linux/mfd/ma35d1-sys.h
>>
>> diff --git a/include/linux/mfd/ma35d1-sys.h b/include/linux/mfd/ma35d1-sys.h
>> new file mode 100644
>> index 000000000000..dcd85231125d
>> --- /dev/null
>> +++ b/include/linux/mfd/ma35d1-sys.h
>> +
>> +#define REG_SYS_PDID		(0x000) /* Product and Device Identifier */
>> +#define REG_SYS_PWRONOTP	(0x004) /* Power-on Setting OTP Source */
>> +#define REG_SYS_PWRONPIN	(0x008) /* Power-on Setting Pin Source */
>> +#define REG_SYS_RSTSTS		(0x010) /* Reset Source Active Status */
> ...
>
> It is a bit odd to have a header file in include/linux/mfd/
> but only have the register numbers in there, and not an
> actual drivers/mfd/ driver to go along with them.
>
> I think what we often do is to just list the individual register
> numbers in the drivers that need them and not have the central
> header at all. On the other hand, I can see it's useful to
> have this documented in one place, and we clearly don't want
> to add a driver if none is needed.
>
> Maybe Lee has a suggestion for how he'd like to handle this.

Agree with this.
We will add #define of individual system control registers to the drivers
that have to use them.
So, I will remove this patch from the patchset in the next version.

>> +void ma35d1_reg_lock(void);
>> +void ma35d1_reg_unlock(void);
> These look like they were left over from an earlier version
> of the code. Since you use the regmap framework, I think this
> will take care of the locking for you.
>
>         Arnd

Best regards,

Jacky Huang