On 6/27/23 16:17, Runa Guo-oc wrote: > On 2023/6/26 20:40, Damien Le Moal wrote: >> On 6/26/23 20:29, Runa Guo-oc wrote: >>> On 2023/6/16 16:34, Damien Le Moal wrote: >>>> On 6/16/23 16:49, Runa Guo-oc wrote: >>>>> [PATCH] ata:sata_zhaoxin: Add support for ZhaoXin Serial ATA >>>> >>>> Broken patch: the email subject is your SoB instead of the above line, which >>>> should not be part of the message (it should be the subject). Please reformat >>>> and resend. >>>> >>> >>> Okay. >>> >>>>> >>>>> Add ZhaoXin Serial ATA support for ZhaoXin CUPs. >>>> >>>> What is "ZhaoXin" ? >>> >>> Zhaoxin is a Chinese company that has mastered the core technology >>> of independent general-purpose processors and its system platform chips, >>> and is committed to providing users with efficient, compatible and secure >>> independent general-purpose processors, chipsets and other products. >>> for more information, you can visit here: https://www.zhaoxin.com/. >> >> A company marketing message is not very informative, technically speaking. What >> is this chipset and on what board/machine can it be found ? That is the more >> relevant information we need in this commit message. >> > > The reason why it is called Zhaoxin SATA is actually to express that it > applies > to all Zhaoxin support of its separate chipset/SOC, for example, > ZX-100S/ZX-200 > chipsets. That is fine. I do not need a reason for the name. what I would like to see is information about which product/board/soc this driver will be needed for. So something like the above is actually fine (may be a little more details if you have). > >>> >>>> And what is "CUPs" ? Please spell this out. >>>> >>> >>> Yes, this is a spelling error. >>> >>>>> >>>>> Signed-off-by: Runa Guo-oc <RunaGuo-oc@xxxxxxxxxxx> >>>>> --- >>>>> drivers/ata/Kconfig | 8 + >>>>> drivers/ata/Makefile | 1 + >>>>> drivers/ata/sata_zhaoxin.c | 384 +++++++++++++++++++++++++++++++++++++++++++++ >>>>> 3 files changed, 393 insertions(+) >>>>> create mode 100644 drivers/ata/sata_zhaoxin.c >>>>> >>>>> diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig >>>>> index 42b51c9..ae327f3 100644 >>>>> --- a/drivers/ata/Kconfig >>>>> +++ b/drivers/ata/Kconfig >>>>> @@ -553,6 +553,14 @@ config SATA_VITESSE >>>>> >>>>> If unsure, say N. >>>>> >>>>> +config SATA_ZHAOXIN >>>>> + tristate "ZhaoXin SATA support" >> >> Same here. If ZhaoXin is only a company name, we need also a chipset model to be >> informative regarding which HW this driver serves. >> > > As mentioned before, the chipset models this driver serves are ZX-100S > and ZX-200 > currently. Fine. Just say so in the Kconfig entry then. If in the future your company produces a different chipset model that needs a different driver, then the entries can be clearly differentiated even if they use the same company name (ZhaoXin). E.g. "ZhaoXin ZX-100S and ZX-200 chipset support" vs "ZhaoXin XYZ-newgen chipset support". Adding entries should not require modifying existing entries. -- Damien Le Moal Western Digital Research