Re: [PATCH] Signed-off-by: Runa Guo-oc <RunaGuo-oc@zhaoxin.com>

[Runa Guo-oc <RunaGuo-oc@zhaoxin.com>]

On 2023/6/27 16:02, Damien Le Moal wrote:
> 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).
> 
For more details, take ZX-200 as an example to illustrate.
ZX-200 is an TSMC 40nm chip that integrates PCIE EP/RC and internal SB
controllers
(Serial ATA, Universal Serial Bus Controller, Network Interface
Controller, SPI, and so on).

And information about SATA is as follows:
•	Serial ATA
•	Compliant with Serial ATA Specification Revision 3.2
•	Support AHCI compliant with AHCI Specification 1.3.1
•	Internal SATA PHY supports 1.5G, 3G and 6G speed. Up to 4 ports
•	Support 4 SATA ports for one SATA bus master, primary channel and
secondary channel
•	Support Partial/Slumber/Auto Partial to Slumber/HIPM/DIPM, support DEVSLP
•	Support Esata/SATA Express
•	Support M.2
•	Support Listen Mode (both AHCI controller and legacy SATA controller )
•	Support MPS/DevSleep(AHCI controller only)
•	Do not support compatible mode
•	Do not support port multiplier
•	Do not support SGPIO
•	Do not support CPD
•	Support Hot-plug and MSI-X
•	Support FLR

>>
>>>>
>>>>> And what is "CUPs" ? Please spell this out.
>>>>>
>>>>
>>>> Yes, this is  a spelling error.
>>>>
>>>>>>
>>>>>> Signed-off-by: Runa Guo-oc <RunaGuo-oc@zhaoxin.com>
>>>>>> ---
>>>>>>  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.
> 
> 
> 

I got it. I'll add these information in Kconfig entry briefly next time.
Thank you for your  advice and detailed explanation.