Message-ID: <6b4645c4-4343-49ca-bfab-ad38443727be@roeck-us.net>
Date: Tue, 22 Apr 2025 07:01:13 -0700
From: Guenter Roeck <linux@...ck-us.net>
To: Chiang Brian <chiang.brian@...entec.com>
Cc: gnoack@...gle.com, gpiccoli@...lia.com, jdelvare@...e.com,
 kees@...nel.org, linux-hardening@...r.kernel.org,
 linux-hwmon@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-security-module@...r.kernel.org, mic@...ikod.net, tony.luck@...el.com
Subject: Re: [PATCH v5] hwmon: (pmbus/tps53679) Add support for TPS53685

On 4/22/25 03:20, Chiang Brian wrote:
> On 3/14/25 10:44, Guenter Roeck wrote:
>> On 3/13/25 20:30, Chiang Brian wrote:
>>> The TPS53685 is a fully AMD SVI3 compliant step down
>>> controller with trans-inductor voltage regulator
>>> (TLVR) topology support, dual channels, built-in
>>> non-volatile memory (NVM), PMBus interface, and
>>> full compatible with TI NexFET smart power
>>> stages.
>>> Add support for it to the tps53679 driver.
>>>
>>> Signed-off-by: Chiang Brian <chiang.brian@...entec.com>
>>> ---
>>> v4 -> V5:
>>>       1. document the compatible of tps53685 into dt-bindings
>>> 	2. add the buffer length as argument for %*ph
>>> 	3. Add Changelog
>>> v3 -> V4:
>>>       1. Add length comparison into the comparison of "id",or it may be true when
>>> 	   the substring of "id" matches device id.
>>> 	2. Restore `return 0;` in `tps53679_identify_chip()`
>>> V2 -> V3:
>>>       1. Remove the length comparsion in the comparison of "id".
>>> V1 -> V2:
>>> 	1. Modify subject and description to meet requirements
>>> 	2. Add "tps53685" into enum chips with numeric order
>>> 	3. Modify the content of marco "TPS53681_DEVICE_ID" from 0x81 to "\x81"
>>> 	   Add marco "TPS53685_DEVICE_ID" with content "TIShP"
>>> 	4. Modify the type of "id" from u16 to char* in `tps53679_identify_chip()`
>>> 	5. Modify the comparison of "id". It will be true if the string "id" matches
>>> 	   device ID and compare with type char*,
>>> 	6. Add the length comparsion into the comparison of "id".
>>> 	7. Modify "len" as return code in `tps53679_identify_chip()`
>>> 	8. Output device error log with %*ph, instead of 0x%x\n"
>>>       9. Use existing tps53679_identify_multiphase() with argument
>>> 	   "TPS53685_DEVICE_ID" in tps53685_identify() rather than creating one
>>> 	   tps53685_identify_multiphase()
>>>
>>> boot-log:
>>
>> This is completely useless noise.
> 
> Sorry for the delay, I've got the approval for posting the boot-log from our
> customer. I was afraid that there's any confidential information in the boot-log

It still does not belong into the commit log. If you absolutely want to,
at least add it after "---".

> of our project. So, I decided to post a boot-log with my laptop then, and I'll
> append a new one once new patch has been finished and uploaded.
> 
> And thanks for the suggestion of adding a buffer length for %*ph.
> The kernel crashes and keeps rebooting without adding that.
> 
> In addition, should I in-reply-to the existing thread or create a new one
> since the dt-bindings should be included in the same thread as well?
> 

You mean for v6 ? Documentation/process/submitting-patches.rst says:

"... However, for a multi-patch series, it is generally
best to avoid using In-Reply-To: to link to older versions of the
series.  This way multiple versions of the patch don't become an
unmanageable forest of references in email clients.  If a link is
helpful, you can use the https://lore.kernel.org/ redirector (e.g., in
the cover email text) to link to an earlier version of the patch series."

Guenter

Powered by blists - more mailing lists