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@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. > > > I got it. I'll add these information in Kconfig entry briefly next time. Thank you for your advice and detailed explanation.
