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

[Jacky Huang <ychuang570808@gmail.com>]

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@nuvoton.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@nuvoton.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