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@xxxxxxxxxxx/ > > > > (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
