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