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

[luyulin@eswincomputing.com]

Hello Krzysztof, Niklas, Rob,

Thank you very much for your suggestions and reply.

> 
> On 04/09/2025 11:14, Niklas Cassel wrote:
> > Hello Krzysztof, Rob,
> > 
> > On Thu, Sep 04, 2025 at 09:10:34AM +0200, Krzysztof Kozlowski wrote:
> >>> +
> >>> +  ports-implemented:
> >>> +    const: 1
> >>
> >> I do not see how you addressed request about firmware. Nothing changed
> >> here, no explanation in the commit msg.
> > 

...

> > 
> > Anyway, I provided my 50 cents here:
> > https://lore.kernel.org/linux-ide/aLBUC116MdJqDGIJ@flawful.org/
> > 
> > (I would like to add that I think it is the disabling of clocks and
> > regulators that causes the register to be cleared, since we do call
> > ahci_platform_assert_rsts() during the first probe, so if it was the reset
> > that cleared the register, the first probe should also not have worked.)
> > 

Thank you very much for your explanation. To add some context:
In our system, the ports-implemented register has already been configured by the firmware
(which is U-Boot on the HiFive Premier P550 board).
Therefore, when entering the kernel, the value of this register is correctly set to 0x1.

During probe, ahci_platform_enable_resources → ahci_platform_deassert_rsts is called.
And when the driver is removed, ahci_platform_disable_resources
→ ahci_platform_assert_rsts is triggered.
This reset operation causes the register to be restored to 0.
According to the IP databook, this register is indeed set to 0 after reset.

This is my understanding. I'd greatly appreciate it if you point out any issues.

> > 
> > Not sure if it relevant to mention this reply to Rob's review comment in the
> > commit message, but perhaps it should have been mentioned in the change log.
> 
> Reviewer questions for more serious stuff happen for a reason, so when
> discussion is resolved somehow differently than reviewer suggested, it
> pretty often deserves explanation in commit msg.
> 
> Well, in changelog as absolute minimum. No explanation happened here in
> the changelog, nor in the commit msg.

Thank you very much for your suggestion.
I'll add explanations in the commit message and changelogs for the next patch.

Best regards,
Yulin