Re: Re: [PATCH v2 1/3] dt-bindings: ata: eswin: Document for EIC7700 SoC ahci

[luyulin@eswincomputing.com]

Hi, Rob
Thank you very much for your professional response and suggestions.
Among the suggestions you mentioned, 
one point is not entirely clear, and I would like to seek your advice on it.
> 
> On Tue, Aug 19, 2025 at 8:54 AM Yulin Lu <luyulin@eswincomputing.com> wrote:
> >
> > From: luyulin <luyulin@eswincomputing.com>
> 
> Please fix your name.
> 
> >
> > Add document for the SATA AHCI controller on the EIC7700 SoC platform,
> > including descriptions of its hardware configurations.
> >
> > Signed-off-by: luyulin <luyulin@eswincomputing.com>
> 
> And here.
> 
> > ---
> >  .../bindings/ata/eswin,eic7700-ahci.yaml      | 92 +++++++++++++++++++
> >  1 file changed, 92 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/ata/eswin,eic7700-ahci.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/ata/eswin,eic7700-ahci.yaml b/Documentation/devicetree/bindings/ata/eswin,eic7700-ahci.yaml
> > new file mode 100644
> > index 000000000000..9ef58c9c2f28
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/ata/eswin,eic7700-ahci.yaml
> > @@ -0,0 +1,92 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/ata/eswin,eic7700-ahci.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Eswin EIC7700 SoC SATA Controller
> > +
> > +maintainers:
> > +  - Yulin Lu <luyulin@eswincomputing.com>
> > +  - Huan He <hehuan1@eswincomputing.com>
> > +
> > +description:
> > +  This document defines device tree bindings for the Synopsys DWC
> > +  implementation of the AHCI SATA controller found in Eswin's
> > +  Eic7700 SoC platform.
> > +
> > +select:
> > +  properties:
> > +    compatible:
> > +      const: eswin,eic7700-ahci
> > +  required:
> > +    - compatible
> > +
> > +allOf:
> > +  - $ref: snps,dwc-ahci-common.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    items:
> > +      - const: eswin,eic7700-ahci
> > +      - const: snps,dwc-ahci
> > +
> > +  reg:
> > +    maxItems: 1
> 
> Drop. snps,dwc-ahci-common.yaml already defines this.
> 
> > +
> > +  interrupts:
> > +    maxItems: 1
> 
> Drop. snps,dwc-ahci-common.yaml already defines this.
> 
> > +
> > +  ports-implemented:
> > +    const: 1
> 
> Really, your firmware should initialize the DWC specific register that
> sets this and is discoverable via a standard AHCI register.
> 
Yes, I initialized the relevant registers during the uboot stage,
and they worked correctly after entering the Linux system.
However, after unloading and then reloading the ko driver,
a reset operation causes these registers to be initialized to 0,
leading to initialization errors when reloading the ko driver.
If I want to implement it this way, I need to modify the ahci_dwc.c driver
and add program handling tailored to our SoC design.

I searched the kernel for "snps,dwc-ahci" and found that
in manufacturers' dts files, such as rk3588-base.dtsi, dra7-l4.dtsi,
exynos5250.dtsi, etc., all include the ports-implemented field in their sata nodes. 
only the sata node in amd-seattle-soc.dtsi lacks the ports-implemented field
and does not have reset resources.
Additionally, in the YAML files under the /Documentation/ata directory that
integrate the DWC AHCI controller, the ports-implemented field has been implemented,
as seen in examples such as rockchip,dwc-ahci.yaml and baikal,bt1-ahci.yaml.

Therefore, I have questions I would like to confirm with you:
1. In your previous feedback, were you suggesting that I remove the ports-implemented field?
And what is the reason for doing so?
Based on your professional advice and considering our SoC design, do you think
it is acceptable to retain the ports-implemented field in the dts instead of
removing it and modifying the ahci_dwc.c driver?
2. I noticed that in the arch/ directory, chips like dra7-l4.dtsi and
exynos5250.dtsi integrate the DWC AHCI controller and include corresponding
hardware resource designs such as clocks and resets.
Why do they not have independent yaml files implemented in the kernel?

Best regards,
Yulin Lu