linux-kernel - Re: Re: [PATCH v1 1/2] dt-bindings: sata: eswin: Document for EIC7700 SoC

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aJ3ziEwmGF-KLPuT@ryzen>
Date: Thu, 14 Aug 2025 16:32:40 +0200
From: Niklas Cassel <cassel@...nel.org>
To: luyulin@...incomputing.com
Cc: hehuan1@...incomputing.com, dlemoal@...nel.org, robh@...nel.org,
	krzk+dt@...nel.org, conor+dt@...nel.org, linux-ide@...r.kernel.org,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
	p.zabel@...gutronix.de, ningyu@...incomputing.com,
	linmin@...incomputing.com, Serge Semin <fancer.lancer@...il.com>
Subject: Re: Re: [PATCH v1 1/2] dt-bindings: sata: eswin: Document for
 EIC7700 SoC

Hello Yulin,

On Thu, Aug 14, 2025 at 05:51:59PM +0800, luyulin@...incomputing.com wrote:
> Thank you very much for your constructive suggestions. Based on your advice,
> I have optimized both the driver and the YAML program.
> I sincerely apologize for the delayed response to your suggestions.

No need to apologize for anything :)


> > The good news is that snps,dwc-ahci-common.yaml has defined and documented
> > all the SATA clocks and resets for your board already (a lot of them which
> > you missed to include in this binding).
> > 
> > 
> > Looking quickly at:
> > eswin,hsp_sp_csr = <&hsp_sp_csr 0x1050>;
> > 
> > I can't help to wonder if these regs shouldn't be in a SATA PHY binding
> > instead.
> > 
> > Do e.g. a
> > $ git grep -A 20 snps,dwc-ahci arch/
> > 
> > There are multiple examples that use a PHY driver.
> > 
> > If you were to implement a PHY driver, it is possible that you would
> > not need to create a new (AHCI) DT binding at all, you could probably
> > just add your compatible string to snps,dwc-ahci.yaml, as (from a quick)
> > glance, all the only platform specific things appear to be PHY related.
> > 
> Thank you very much for your expert advice. I have already implemented a 
> independent PHY driver, while the controller driver utilizes ahci_dwc.c.
> Due to our hardware platform's SATA controller has specific constraints on clock, reset
> and port resources, I think adding these to snps,dwc-ahci.yaml might compromise its readability.
> Following reference implementations from other vendors in the Linux kernel, 
> such as rockchip,dwc-ahci.yaml, amlogic,axg-pcie.yaml and others, I plan to create 
> a new eswin,eic7700-ahci.yaml to describe these specifications.
> Based on your professional experience, would you consider this approach acceptable?

That sounds like a good approach.

When you create your device tree binding, make sure to reference
snps,dwc-ahci-common.yaml, like the other DWC based bindings:

$ git grep snps,dwc-ahci-common.yaml
baikal,bt1-ahci.yaml:  - $ref: snps,dwc-ahci-common.yaml#
baikal,bt1-ahci.yaml:    $ref: /schemas/ata/snps,dwc-ahci-common.yaml#/$defs/dwc-ahci-port
rockchip,dwc-ahci.yaml:    $ref: /schemas/ata/snps,dwc-ahci-common.yaml#/$defs/dwc-ahci-port
rockchip,dwc-ahci.yaml:  - $ref: snps,dwc-ahci-common.yaml#
snps,dwc-ahci.yaml:  - $ref: snps,dwc-ahci-common.yaml#
snps,dwc-ahci.yaml:    $ref: /schemas/ata/snps,dwc-ahci-common.yaml#/$defs/dwc-ahci-port


Kind regards,
Niklas