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

[Niklas Cassel <cassel@kernel.org>]

Hello Yulin,

On Thu, Aug 14, 2025 at 05:51:59PM +0800, luyulin@eswincomputing.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