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

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: <7206383a.d98.19918c22570.Coremail.luyulin@eswincomputing.com>
Date: Fri, 5 Sep 2025 15:23:20 +0800 (GMT+08:00)
From: luyulin@...incomputing.com
To: "Krzysztof Kozlowski" <krzk@...nel.org>, cassel@...nel.org,
	robh@...nel.org
Cc: "Niklas Cassel" <cassel@...nel.org>, 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,
	vkoul@...nel.org, kishon@...nel.org, linux-phy@...ts.infradead.org,
	ningyu@...incomputing.com, zhengyu@...incomputing.com,
	linmin@...incomputing.com, huangyifeng@...incomputing.com,
	fenglin@...incomputing.com, lianghujun@...incomputing.com
Subject: Re: Re: [PATCH v3 1/3] dt-bindings: ata: eswin: Document for
 EIC7700 SoC ahci

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