Message-ID: <CADYN=9JiX-=PcKMzAcSm=p7Dh6kYT7Kbv-8kcNF0MQ4=1hFS5g@mail.gmail.com>
Date:   Fri, 14 Oct 2022 11:22:38 +0200
From:   Anders Roxell <anders.roxell@...aro.org>
To:     Damien Le Moal <damien.lemoal@...nsource.wdc.com>
Cc:     Arnd Bergmann <arnd@...db.de>,
        Naresh Kamboju <naresh.kamboju@...aro.org>,
        Sergey.Semin@...kalelectronics.ru,
        open list <linux-kernel@...r.kernel.org>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        regressions@...ts.linux.dev,
        "open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers)" 
        <linux-ide@...r.kernel.org>, lkft-triage@...ts.linaro.org,
        Lukas Bulwahn <lukas.bulwahn@...il.com>,
        Niklas Cassel <niklas.cassel@....com>
Subject: Re: TI: X15 the connected SSD is not detected on Linux next 20221006 tag

On Fri, 14 Oct 2022 at 09:53, Damien Le Moal
<damien.lemoal@...nsource.wdc.com> 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]. 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

Powered by blists - more mailing lists