On Fri, Oct 14, 2022 at 11:22:38AM +0200, Anders Roxell wrote:
> On Fri, 14 Oct 2022 at 09:53, Damien Le Moal
> <damien.lemoal@xxxxxxxxxxxxxxxxxx> wrote:
> >
> > On 10/14/22 16:31, Arnd Bergmann wrote:
> > > On Fri, Oct 14, 2022, at 2:22 AM, Damien Le Moal wrote:
> > >> On 10/14/22 07:07, Anders Roxell wrote:
> > >> [...]
> > >>>> 8)
> > >>>>> If reverting these patches restores the eSATA port on this board, then you need
> > >>>>> to fix the defconfig for that board.
> > >>>>
> > >>>> OTOH,
> > >>>> Anders, enabled the new config CONFIG_AHCI_DWC=y and tried but the
> > >>>> device failed to boot.
> > >>>
> > >>> I thought it would work with enabling CONFIG_AHCI_DWC=y, but it didn't...
> > >>
> > >> As mentioned in my previous reply to Naresh, this is a new driver added in
> > >> 6.1. Your board was working before so this should not be the driver needed
> > >> for it.
> > >>
> > >>> However, reverting patch 33629d35090f ("ata: ahci: Add DWC AHCI SATA
> > >>> controller support")
> > >>> from next-20221013 was a success, kernel booted and the 'mkfs.ext4' cmd was
> > >>> successful.
> > >>
> > >> Which is very strange... There is only one hunk in that commit that could
> > >> be considered suspicious:
> > >>
> > >> diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
> > >> index 9b56490ecbc3..8f5572a9f8f1 100644
> > >> --- a/drivers/ata/ahci_platform.c
> > >> +++ b/drivers/ata/ahci_platform.c
> > >> @@ -80,9 +80,7 @@ static SIMPLE_DEV_PM_OPS(ahci_pm_ops, ahci_platform_suspend,
> > >> static const struct of_device_id ahci_of_match[] = {
> > >> { .compatible = "generic-ahci", },
> > >> /* Keep the following compatibles for device tree compatibility */
> > >> - { .compatible = "snps,spear-ahci", },
> > >> { .compatible = "ibm,476gtr-ahci", },
> > >> - { .compatible = "snps,dwc-ahci", },
> > >> { .compatible = "hisilicon,hisi-ahci", },
> > >> { .compatible = "cavium,octeon-7130-ahci", },
> > >> { /* sentinel */ }
> > >>
> > >> Is your board using one of these compatible string ?
> > >
> > > The x15 uses "snps,dwc-ahci". I would expect it to detect the device
> > > with the new driver if that is loaded, but it's possible that the
> > > driver does not work on all versions of the dwc-ahci hardware.
> > >
> > > Anders, can you provide the boot log from a boot with the new driver
> > > built in? There should be some messages from dwc-ahci about finding
> > > the device, but then not ultimately working.
> > >
> > > Depending on which way it goes wrong, the safest fallback for 6.1 is
> > > probably to move the "snps,spear-ahci" and "snps,dwc-ahci" compatible
> > > strings back into the old driver, and leave the new one only for
> > > the "baikal,bt1-ahci" implementation of it, until it has been
> > > successfully verified on TI am5/dra7, spear13xx and exynos.
> >
> > OK. So a fix patch until further tests/debug is completed would be this:
> >
> > diff --git a/drivers/ata/ahci_dwc.c b/drivers/ata/ahci_dwc.c
> > index 8fb66860db31..7a0cbab00843 100644
> > --- a/drivers/ata/ahci_dwc.c
> > +++ b/drivers/ata/ahci_dwc.c
> > @@ -469,8 +469,6 @@ static struct ahci_dwc_plat_data ahci_bt1_plat = {
> > };
> >
> > static const struct of_device_id ahci_dwc_of_match[] = {
> > - { .compatible = "snps,dwc-ahci", &ahci_dwc_plat },
> > - { .compatible = "snps,spear-ahci", &ahci_dwc_plat },
> > { .compatible = "baikal,bt1-ahci", &ahci_bt1_plat },
> > {},
> > };
> > diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
> > index 8f5572a9f8f1..9b56490ecbc3 100644
> > --- a/drivers/ata/ahci_platform.c
> > +++ b/drivers/ata/ahci_platform.c
> > @@ -80,7 +80,9 @@ static SIMPLE_DEV_PM_OPS(ahci_pm_ops, ahci_platform_suspend,
> > static const struct of_device_id ahci_of_match[] = {
> > { .compatible = "generic-ahci", },
> > /* Keep the following compatibles for device tree compatibility */
> > + { .compatible = "snps,spear-ahci", },
> > { .compatible = "ibm,476gtr-ahci", },
> > + { .compatible = "snps,dwc-ahci", },
> > { .compatible = "hisilicon,hisi-ahci", },
> > { .compatible = "cavium,octeon-7130-ahci", },
> > { /* sentinel */ }
> >
> > Anders, Naresh,
> >
> > Can you try this ?
>
> Tested this patch on todays linux-next tag: next-20221014 without enabling
> CONFIG_AHCI_DWC and it worked as expected when booting [1].
> On the other hand I also tried a build/boot with CONFIG_AHCI_DWC enabled
> and it worked as expected to boot [2].
Expected result. The DWC driver will probe the device on our platform
only while your platform falls back to using the generic driver.
Anders, in order understand the root cause of the problem could you please
1. upload the bogus boot log.
2. try what I suggested here
Link: https://lore.kernel.org/linux-ide/20221014133623.l6w4o7onoyhv2q34@mobilestation/
and if the system fails to boot at some point upload the boot log.
-Sergey
> However, during building a warning [3] popped up:
>
> make --silent --keep-going --jobs=8
> O=/home/tuxbuild/.cache/tuxmake/builds/2/build ARCH=arm
> CROSS_COMPILE=arm-linux-gnueabihf- 'CC=sccache
> arm-linux-gnueabihf-gcc' 'HOSTCC=sccache gcc'
> /builds/linux/drivers/ata/ahci_dwc.c:462:34: warning: 'ahci_dwc_plat'
> defined but not used [-Wunused-variable]
> 462 | static struct ahci_dwc_plat_data ahci_dwc_plat = {
> | ^~~~~~~~~~~~~
>
>
> Cheers,
> Anders
> [1] https://lkft.validation.linaro.org/scheduler/job/5678031
> [2] https://lkft.validation.linaro.org/scheduler/job/5678152
> [3] https://builds.tuxbuild.com/2G7PDSV5uzjnQqCCBybK4WpoTxz/build.log
>
