* [PATCH v3] ata: ahci: Skip 200 ms debounce delay for Marvell 88SE9235
@ 2022-02-01 7:12 Paul Menzel
2022-02-01 8:27 ` Damien Le Moal
2022-02-02 8:14 ` Damien Le Moal
0 siblings, 2 replies; 6+ messages in thread
From: Paul Menzel @ 2022-02-01 7:12 UTC (permalink / raw)
To: Damien Le Moal; +Cc: Paul Menzel, linux-ide, linux-kernel
The 200 ms delay before debouncing the PHY in `sata_link_resume()` is
not needed for the Marvell 88SE9235.
$ lspci -nn -s 0021:0e:00.0
0021:0e:00.0 SATA controller [0106]: Marvell Technology Group Ltd. 88SE9235 PCIe 2.0 x2 4-port SATA 6 Gb/s Controller [1b4b:9235] (rev 11)
So, remove it. Tested on IBM S822LC with current Linux 5.17-rc1:
Currently, without this patch (with 200 ms delay), device probe for ata1
takes 485 ms:
[ 3.358158] ata1: SATA max UDMA/133 abar m2048@0x3fe881000000 port 0x3fe881000100 irq 39
[ 3.358175] ata2: SATA max UDMA/133 abar m2048@0x3fe881000000 port 0x3fe881000180 irq 39
[ 3.358191] ata3: SATA max UDMA/133 abar m2048@0x3fe881000000 port 0x3fe881000200 irq 39
[ 3.358207] ata4: SATA max UDMA/133 abar m2048@0x3fe881000000 port 0x3fe881000280 irq 39
[…]
[ 3.677542] ata3: SATA link down (SStatus 0 SControl 300)
[ 3.677719] ata4: SATA link down (SStatus 0 SControl 300)
[ 3.839242] ata2: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
[ 3.839828] ata2.00: ATA-10: ST1000NX0313 00LY266 00LY265IBM, BE33, max UDMA/133
[ 3.840029] ata2.00: 1953525168 sectors, multi 0: LBA48 NCQ (depth 32), AA
[ 3.841796] ata2.00: configured for UDMA/133
[ 3.843231] ata1: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
[ 3.844083] ata1.00: ATA-10: ST1000NX0313 00LY266 00LY265IBM, BE33, max UDMA/133
[ 3.844313] ata1.00: 1953525168 sectors, multi 0: LBA48 NCQ (depth 32), AA
[ 3.846043] ata1.00: configured for UDMA/133
With this patch (no delay) device probe for ata1 takes 273 ms:
[ 3.624259] ata1: SATA max UDMA/133 abar m2048@0x3fe881000000 port 0x3f e881000100 irq 39
[ 3.624436] ata2: SATA max UDMA/133 abar m2048@0x3fe881000000 port 0x3f e881000180 irq 39
[ 3.624452] ata3: SATA max UDMA/133 abar m2048@0x3fe881000000 port 0x3f e881000200 irq 39
[ 3.624468] ata4: SATA max UDMA/133 abar m2048@0x3fe881000000 port 0x3f e881000280 irq 39
[…]
[ 3.731966] ata3: SATA link down (SStatus 0 SControl 300)
[ 3.732069] ata4: SATA link down (SStatus 0 SControl 300)
[ 3.897448] ata1: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
[ 3.897678] ata2: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
[ 3.898140] ata1.00: ATA-10: ST1000NX0313 00LY266 00LY265IBM, BE33, max UDMA/133
[ 3.898175] ata2.00: ATA-10: ST1000NX0313 00LY266 00LY265IBM, BE33, max UDMA/133
[ 3.898287] ata1.00: 1953525168 sectors, multi 0: LBA48 NCQ (depth 32), AA
[ 3.898349] ata2.00: 1953525168 sectors, multi 0: LBA48 NCQ (depth 32), AA
[ 3.900070] ata1.00: configured for UDMA/133
[ 3.900166] ata2.00: configured for UDMA/133
Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
---
v2: address comments for commit message (but forgot v2 tag)
v3: resend with v3 tag in subject line/commit message summary
drivers/ata/ahci.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index ab5811ef5a53..edca4e8fd44e 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -582,6 +582,8 @@ static const struct pci_device_id ahci_pci_tbl[] = {
.driver_data = board_ahci_yes_fbs },
{ PCI_DEVICE(PCI_VENDOR_ID_MARVELL_EXT, 0x9230),
.driver_data = board_ahci_yes_fbs },
+ { PCI_DEVICE(PCI_VENDOR_ID_MARVELL_EXT, 0x9235),
+ .driver_data = board_ahci_no_debounce_delay },
{ PCI_DEVICE(PCI_VENDOR_ID_TTI, 0x0642), /* highpoint rocketraid 642L */
.driver_data = board_ahci_yes_fbs },
{ PCI_DEVICE(PCI_VENDOR_ID_TTI, 0x0645), /* highpoint rocketraid 644L */
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3] ata: ahci: Skip 200 ms debounce delay for Marvell 88SE9235
2022-02-01 7:12 [PATCH v3] ata: ahci: Skip 200 ms debounce delay for Marvell 88SE9235 Paul Menzel
@ 2022-02-01 8:27 ` Damien Le Moal
2022-02-02 8:14 ` Damien Le Moal
1 sibling, 0 replies; 6+ messages in thread
From: Damien Le Moal @ 2022-02-01 8:27 UTC (permalink / raw)
To: Paul Menzel; +Cc: linux-ide, linux-kernel
On 2022/02/01 16:12, Paul Menzel wrote:
> The 200 ms delay before debouncing the PHY in `sata_link_resume()` is
> not needed for the Marvell 88SE9235.
>
> $ lspci -nn -s 0021:0e:00.0
> 0021:0e:00.0 SATA controller [0106]: Marvell Technology Group Ltd. 88SE9235 PCIe 2.0 x2 4-port SATA 6 Gb/s Controller [1b4b:9235] (rev 11)
>
> So, remove it. Tested on IBM S822LC with current Linux 5.17-rc1:
>
> Currently, without this patch (with 200 ms delay), device probe for ata1
> takes 485 ms:
>
> [ 3.358158] ata1: SATA max UDMA/133 abar m2048@0x3fe881000000 port 0x3fe881000100 irq 39
> [ 3.358175] ata2: SATA max UDMA/133 abar m2048@0x3fe881000000 port 0x3fe881000180 irq 39
> [ 3.358191] ata3: SATA max UDMA/133 abar m2048@0x3fe881000000 port 0x3fe881000200 irq 39
> [ 3.358207] ata4: SATA max UDMA/133 abar m2048@0x3fe881000000 port 0x3fe881000280 irq 39
> […]
> [ 3.677542] ata3: SATA link down (SStatus 0 SControl 300)
> [ 3.677719] ata4: SATA link down (SStatus 0 SControl 300)
> [ 3.839242] ata2: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
> [ 3.839828] ata2.00: ATA-10: ST1000NX0313 00LY266 00LY265IBM, BE33, max UDMA/133
> [ 3.840029] ata2.00: 1953525168 sectors, multi 0: LBA48 NCQ (depth 32), AA
> [ 3.841796] ata2.00: configured for UDMA/133
> [ 3.843231] ata1: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
> [ 3.844083] ata1.00: ATA-10: ST1000NX0313 00LY266 00LY265IBM, BE33, max UDMA/133
> [ 3.844313] ata1.00: 1953525168 sectors, multi 0: LBA48 NCQ (depth 32), AA
> [ 3.846043] ata1.00: configured for UDMA/133
>
> With this patch (no delay) device probe for ata1 takes 273 ms:
>
> [ 3.624259] ata1: SATA max UDMA/133 abar m2048@0x3fe881000000 port 0x3f e881000100 irq 39
> [ 3.624436] ata2: SATA max UDMA/133 abar m2048@0x3fe881000000 port 0x3f e881000180 irq 39
> [ 3.624452] ata3: SATA max UDMA/133 abar m2048@0x3fe881000000 port 0x3f e881000200 irq 39
> [ 3.624468] ata4: SATA max UDMA/133 abar m2048@0x3fe881000000 port 0x3f e881000280 irq 39
> […]
> [ 3.731966] ata3: SATA link down (SStatus 0 SControl 300)
> [ 3.732069] ata4: SATA link down (SStatus 0 SControl 300)
> [ 3.897448] ata1: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
> [ 3.897678] ata2: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
> [ 3.898140] ata1.00: ATA-10: ST1000NX0313 00LY266 00LY265IBM, BE33, max UDMA/133
> [ 3.898175] ata2.00: ATA-10: ST1000NX0313 00LY266 00LY265IBM, BE33, max UDMA/133
> [ 3.898287] ata1.00: 1953525168 sectors, multi 0: LBA48 NCQ (depth 32), AA
> [ 3.898349] ata2.00: 1953525168 sectors, multi 0: LBA48 NCQ (depth 32), AA
> [ 3.900070] ata1.00: configured for UDMA/133
> [ 3.900166] ata2.00: configured for UDMA/133
>
> Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
> ---
> v2: address comments for commit message (but forgot v2 tag)
> v3: resend with v3 tag in subject line/commit message summary
>
> drivers/ata/ahci.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
> index ab5811ef5a53..edca4e8fd44e 100644
> --- a/drivers/ata/ahci.c
> +++ b/drivers/ata/ahci.c
> @@ -582,6 +582,8 @@ static const struct pci_device_id ahci_pci_tbl[] = {
> .driver_data = board_ahci_yes_fbs },
> { PCI_DEVICE(PCI_VENDOR_ID_MARVELL_EXT, 0x9230),
> .driver_data = board_ahci_yes_fbs },
> + { PCI_DEVICE(PCI_VENDOR_ID_MARVELL_EXT, 0x9235),
> + .driver_data = board_ahci_no_debounce_delay },
> { PCI_DEVICE(PCI_VENDOR_ID_TTI, 0x0642), /* highpoint rocketraid 642L */
> .driver_data = board_ahci_yes_fbs },
> { PCI_DEVICE(PCI_VENDOR_ID_TTI, 0x0645), /* highpoint rocketraid 644L */
Looks good. Will apply to for-5.18.
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] ata: ahci: Skip 200 ms debounce delay for Marvell 88SE9235
2022-02-01 7:12 [PATCH v3] ata: ahci: Skip 200 ms debounce delay for Marvell 88SE9235 Paul Menzel
2022-02-01 8:27 ` Damien Le Moal
@ 2022-02-02 8:14 ` Damien Le Moal
2022-02-02 14:29 ` Paul Menzel
1 sibling, 1 reply; 6+ messages in thread
From: Damien Le Moal @ 2022-02-02 8:14 UTC (permalink / raw)
To: Paul Menzel; +Cc: linux-ide, linux-kernel
On 2/1/22 16:12, Paul Menzel wrote:
> The 200 ms delay before debouncing the PHY in `sata_link_resume()` is
> not needed for the Marvell 88SE9235.
>
> $ lspci -nn -s 0021:0e:00.0
> 0021:0e:00.0 SATA controller [0106]: Marvell Technology Group Ltd. 88SE9235 PCIe 2.0 x2 4-port SATA 6 Gb/s Controller [1b4b:9235] (rev 11)
>
> So, remove it. Tested on IBM S822LC with current Linux 5.17-rc1:
>
> Currently, without this patch (with 200 ms delay), device probe for ata1
> takes 485 ms:
>
> [ 3.358158] ata1: SATA max UDMA/133 abar m2048@0x3fe881000000 port 0x3fe881000100 irq 39
> [ 3.358175] ata2: SATA max UDMA/133 abar m2048@0x3fe881000000 port 0x3fe881000180 irq 39
> [ 3.358191] ata3: SATA max UDMA/133 abar m2048@0x3fe881000000 port 0x3fe881000200 irq 39
> [ 3.358207] ata4: SATA max UDMA/133 abar m2048@0x3fe881000000 port 0x3fe881000280 irq 39
> […]
> [ 3.677542] ata3: SATA link down (SStatus 0 SControl 300)
> [ 3.677719] ata4: SATA link down (SStatus 0 SControl 300)
> [ 3.839242] ata2: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
> [ 3.839828] ata2.00: ATA-10: ST1000NX0313 00LY266 00LY265IBM, BE33, max UDMA/133
> [ 3.840029] ata2.00: 1953525168 sectors, multi 0: LBA48 NCQ (depth 32), AA
> [ 3.841796] ata2.00: configured for UDMA/133
> [ 3.843231] ata1: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
> [ 3.844083] ata1.00: ATA-10: ST1000NX0313 00LY266 00LY265IBM, BE33, max UDMA/133
> [ 3.844313] ata1.00: 1953525168 sectors, multi 0: LBA48 NCQ (depth 32), AA
> [ 3.846043] ata1.00: configured for UDMA/133
>
> With this patch (no delay) device probe for ata1 takes 273 ms:
>
> [ 3.624259] ata1: SATA max UDMA/133 abar m2048@0x3fe881000000 port 0x3f e881000100 irq 39
> [ 3.624436] ata2: SATA max UDMA/133 abar m2048@0x3fe881000000 port 0x3f e881000180 irq 39
> [ 3.624452] ata3: SATA max UDMA/133 abar m2048@0x3fe881000000 port 0x3f e881000200 irq 39
> [ 3.624468] ata4: SATA max UDMA/133 abar m2048@0x3fe881000000 port 0x3f e881000280 irq 39
> […]
> [ 3.731966] ata3: SATA link down (SStatus 0 SControl 300)
> [ 3.732069] ata4: SATA link down (SStatus 0 SControl 300)
> [ 3.897448] ata1: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
> [ 3.897678] ata2: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
> [ 3.898140] ata1.00: ATA-10: ST1000NX0313 00LY266 00LY265IBM, BE33, max UDMA/133
> [ 3.898175] ata2.00: ATA-10: ST1000NX0313 00LY266 00LY265IBM, BE33, max UDMA/133
> [ 3.898287] ata1.00: 1953525168 sectors, multi 0: LBA48 NCQ (depth 32), AA
> [ 3.898349] ata2.00: 1953525168 sectors, multi 0: LBA48 NCQ (depth 32), AA
> [ 3.900070] ata1.00: configured for UDMA/133
> [ 3.900166] ata2.00: configured for UDMA/133
>
> Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
> ---
> v2: address comments for commit message (but forgot v2 tag)
> v3: resend with v3 tag in subject line/commit message summary
>
> drivers/ata/ahci.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
> index ab5811ef5a53..edca4e8fd44e 100644
> --- a/drivers/ata/ahci.c
> +++ b/drivers/ata/ahci.c
> @@ -582,6 +582,8 @@ static const struct pci_device_id ahci_pci_tbl[] = {
> .driver_data = board_ahci_yes_fbs },
> { PCI_DEVICE(PCI_VENDOR_ID_MARVELL_EXT, 0x9230),
> .driver_data = board_ahci_yes_fbs },
> + { PCI_DEVICE(PCI_VENDOR_ID_MARVELL_EXT, 0x9235),
> + .driver_data = board_ahci_no_debounce_delay },
> { PCI_DEVICE(PCI_VENDOR_ID_TTI, 0x0642), /* highpoint rocketraid 642L */
> .driver_data = board_ahci_yes_fbs },
> { PCI_DEVICE(PCI_VENDOR_ID_TTI, 0x0645), /* highpoint rocketraid 644L */
Applied to for-5.18 with commit title and message changes. The title is now:
ata: ahci: Add support for Marvell 88SE9235 adapter
Since it is exactly what this patch is doing by adding a PCI ID.
The comments about the 200ms debounce delay not being needed is kept as
a description of how this new adapter support is defined, using the
board_ahci_no_debounce_delay board definition.
Thanks !
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] ata: ahci: Skip 200 ms debounce delay for Marvell 88SE9235
2022-02-02 8:14 ` Damien Le Moal
@ 2022-02-02 14:29 ` Paul Menzel
2022-02-02 23:05 ` Damien Le Moal
0 siblings, 1 reply; 6+ messages in thread
From: Paul Menzel @ 2022-02-02 14:29 UTC (permalink / raw)
To: Damien Le Moal; +Cc: linux-ide, linux-kernel
Dear Damien,
Am 02.02.22 um 09:14 schrieb Damien Le Moal:
> On 2/1/22 16:12, Paul Menzel wrote:
>> The 200 ms delay before debouncing the PHY in `sata_link_resume()` is
>> not needed for the Marvell 88SE9235.
>>
>> $ lspci -nn -s 0021:0e:00.0
>> 0021:0e:00.0 SATA controller [0106]: Marvell Technology Group Ltd. 88SE9235 PCIe 2.0 x2 4-port SATA 6 Gb/s Controller [1b4b:9235] (rev 11)
>>
>> So, remove it. Tested on IBM S822LC with current Linux 5.17-rc1:
>>
>> Currently, without this patch (with 200 ms delay), device probe for ata1
>> takes 485 ms:
>>
>> [ 3.358158] ata1: SATA max UDMA/133 abar m2048@0x3fe881000000 port 0x3fe881000100 irq 39
>> [ 3.358175] ata2: SATA max UDMA/133 abar m2048@0x3fe881000000 port 0x3fe881000180 irq 39
>> [ 3.358191] ata3: SATA max UDMA/133 abar m2048@0x3fe881000000 port 0x3fe881000200 irq 39
>> [ 3.358207] ata4: SATA max UDMA/133 abar m2048@0x3fe881000000 port 0x3fe881000280 irq 39
>> […]
>> [ 3.677542] ata3: SATA link down (SStatus 0 SControl 300)
>> [ 3.677719] ata4: SATA link down (SStatus 0 SControl 300)
>> [ 3.839242] ata2: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
>> [ 3.839828] ata2.00: ATA-10: ST1000NX0313 00LY266 00LY265IBM, BE33, max UDMA/133
>> [ 3.840029] ata2.00: 1953525168 sectors, multi 0: LBA48 NCQ (depth 32), AA
>> [ 3.841796] ata2.00: configured for UDMA/133
>> [ 3.843231] ata1: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
>> [ 3.844083] ata1.00: ATA-10: ST1000NX0313 00LY266 00LY265IBM, BE33, max UDMA/133
>> [ 3.844313] ata1.00: 1953525168 sectors, multi 0: LBA48 NCQ (depth 32), AA
>> [ 3.846043] ata1.00: configured for UDMA/133
>>
>> With this patch (no delay) device probe for ata1 takes 273 ms:
>>
>> [ 3.624259] ata1: SATA max UDMA/133 abar m2048@0x3fe881000000 port 0x3f e881000100 irq 39
>> [ 3.624436] ata2: SATA max UDMA/133 abar m2048@0x3fe881000000 port 0x3f e881000180 irq 39
>> [ 3.624452] ata3: SATA max UDMA/133 abar m2048@0x3fe881000000 port 0x3f e881000200 irq 39
>> [ 3.624468] ata4: SATA max UDMA/133 abar m2048@0x3fe881000000 port 0x3f e881000280 irq 39
>> […]
>> [ 3.731966] ata3: SATA link down (SStatus 0 SControl 300)
>> [ 3.732069] ata4: SATA link down (SStatus 0 SControl 300)
>> [ 3.897448] ata1: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
>> [ 3.897678] ata2: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
>> [ 3.898140] ata1.00: ATA-10: ST1000NX0313 00LY266 00LY265IBM, BE33, max UDMA/133
>> [ 3.898175] ata2.00: ATA-10: ST1000NX0313 00LY266 00LY265IBM, BE33, max UDMA/133
>> [ 3.898287] ata1.00: 1953525168 sectors, multi 0: LBA48 NCQ (depth 32), AA
>> [ 3.898349] ata2.00: 1953525168 sectors, multi 0: LBA48 NCQ (depth 32), AA
>> [ 3.900070] ata1.00: configured for UDMA/133
>> [ 3.900166] ata2.00: configured for UDMA/133
>>
>> Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
>> ---
>> v2: address comments for commit message (but forgot v2 tag)
>> v3: resend with v3 tag in subject line/commit message summary
>>
>> drivers/ata/ahci.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
>> index ab5811ef5a53..edca4e8fd44e 100644
>> --- a/drivers/ata/ahci.c
>> +++ b/drivers/ata/ahci.c
>> @@ -582,6 +582,8 @@ static const struct pci_device_id ahci_pci_tbl[] = {
>> .driver_data = board_ahci_yes_fbs },
>> { PCI_DEVICE(PCI_VENDOR_ID_MARVELL_EXT, 0x9230),
>> .driver_data = board_ahci_yes_fbs },
>> + { PCI_DEVICE(PCI_VENDOR_ID_MARVELL_EXT, 0x9235),
>> + .driver_data = board_ahci_no_debounce_delay },
>> { PCI_DEVICE(PCI_VENDOR_ID_TTI, 0x0642), /* highpoint rocketraid 642L */
>> .driver_data = board_ahci_yes_fbs },
>> { PCI_DEVICE(PCI_VENDOR_ID_TTI, 0x0645), /* highpoint rocketraid 644L */
>
> Applied to for-5.18 with commit title and message changes. The title is now:
>
> ata: ahci: Add support for Marvell 88SE9235 adapter
>
> Since it is exactly what this patch is doing by adding a PCI ID.
Thank you for applying the patch. I saw the summary/title change also
with the other patch. I am sorry, but I totally disagree. Reading that
summary/title in `git log --oneline`, it’s not clear at all, what the
patch does, and the full description or diff has to be read. “Add
support” for me means, that it was unsupported before, which is not true
at all as the defaults were used.
> The comments about the 200ms debounce delay not being needed is kept as
> a description of how this new adapter support is defined, using the
> board_ahci_no_debounce_delay board definition.
Kind regards,
Paul
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] ata: ahci: Skip 200 ms debounce delay for Marvell 88SE9235
2022-02-02 14:29 ` Paul Menzel
@ 2022-02-02 23:05 ` Damien Le Moal
2022-02-02 23:49 ` Damien Le Moal
0 siblings, 1 reply; 6+ messages in thread
From: Damien Le Moal @ 2022-02-02 23:05 UTC (permalink / raw)
To: Paul Menzel; +Cc: linux-ide, linux-kernel
On 2/2/22 23:29, Paul Menzel wrote:
> Dear Damien,
>
>
> Am 02.02.22 um 09:14 schrieb Damien Le Moal:
>> On 2/1/22 16:12, Paul Menzel wrote:
>>> The 200 ms delay before debouncing the PHY in `sata_link_resume()` is
>>> not needed for the Marvell 88SE9235.
>>>
>>> $ lspci -nn -s 0021:0e:00.0
>>> 0021:0e:00.0 SATA controller [0106]: Marvell Technology Group Ltd. 88SE9235 PCIe 2.0 x2 4-port SATA 6 Gb/s Controller [1b4b:9235] (rev 11)
>>>
>>> So, remove it. Tested on IBM S822LC with current Linux 5.17-rc1:
>>>
>>> Currently, without this patch (with 200 ms delay), device probe for ata1
>>> takes 485 ms:
>>>
>>> [ 3.358158] ata1: SATA max UDMA/133 abar m2048@0x3fe881000000 port 0x3fe881000100 irq 39
>>> [ 3.358175] ata2: SATA max UDMA/133 abar m2048@0x3fe881000000 port 0x3fe881000180 irq 39
>>> [ 3.358191] ata3: SATA max UDMA/133 abar m2048@0x3fe881000000 port 0x3fe881000200 irq 39
>>> [ 3.358207] ata4: SATA max UDMA/133 abar m2048@0x3fe881000000 port 0x3fe881000280 irq 39
>>> […]
>>> [ 3.677542] ata3: SATA link down (SStatus 0 SControl 300)
>>> [ 3.677719] ata4: SATA link down (SStatus 0 SControl 300)
>>> [ 3.839242] ata2: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
>>> [ 3.839828] ata2.00: ATA-10: ST1000NX0313 00LY266 00LY265IBM, BE33, max UDMA/133
>>> [ 3.840029] ata2.00: 1953525168 sectors, multi 0: LBA48 NCQ (depth 32), AA
>>> [ 3.841796] ata2.00: configured for UDMA/133
>>> [ 3.843231] ata1: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
>>> [ 3.844083] ata1.00: ATA-10: ST1000NX0313 00LY266 00LY265IBM, BE33, max UDMA/133
>>> [ 3.844313] ata1.00: 1953525168 sectors, multi 0: LBA48 NCQ (depth 32), AA
>>> [ 3.846043] ata1.00: configured for UDMA/133
>>>
>>> With this patch (no delay) device probe for ata1 takes 273 ms:
>>>
>>> [ 3.624259] ata1: SATA max UDMA/133 abar m2048@0x3fe881000000 port 0x3f e881000100 irq 39
>>> [ 3.624436] ata2: SATA max UDMA/133 abar m2048@0x3fe881000000 port 0x3f e881000180 irq 39
>>> [ 3.624452] ata3: SATA max UDMA/133 abar m2048@0x3fe881000000 port 0x3f e881000200 irq 39
>>> [ 3.624468] ata4: SATA max UDMA/133 abar m2048@0x3fe881000000 port 0x3f e881000280 irq 39
>>> […]
>>> [ 3.731966] ata3: SATA link down (SStatus 0 SControl 300)
>>> [ 3.732069] ata4: SATA link down (SStatus 0 SControl 300)
>>> [ 3.897448] ata1: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
>>> [ 3.897678] ata2: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
>>> [ 3.898140] ata1.00: ATA-10: ST1000NX0313 00LY266 00LY265IBM, BE33, max UDMA/133
>>> [ 3.898175] ata2.00: ATA-10: ST1000NX0313 00LY266 00LY265IBM, BE33, max UDMA/133
>>> [ 3.898287] ata1.00: 1953525168 sectors, multi 0: LBA48 NCQ (depth 32), AA
>>> [ 3.898349] ata2.00: 1953525168 sectors, multi 0: LBA48 NCQ (depth 32), AA
>>> [ 3.900070] ata1.00: configured for UDMA/133
>>> [ 3.900166] ata2.00: configured for UDMA/133
>>>
>>> Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
>>> ---
>>> v2: address comments for commit message (but forgot v2 tag)
>>> v3: resend with v3 tag in subject line/commit message summary
>>>
>>> drivers/ata/ahci.c | 2 ++
>>> 1 file changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
>>> index ab5811ef5a53..edca4e8fd44e 100644
>>> --- a/drivers/ata/ahci.c
>>> +++ b/drivers/ata/ahci.c
>>> @@ -582,6 +582,8 @@ static const struct pci_device_id ahci_pci_tbl[] = {
>>> .driver_data = board_ahci_yes_fbs },
>>> { PCI_DEVICE(PCI_VENDOR_ID_MARVELL_EXT, 0x9230),
>>> .driver_data = board_ahci_yes_fbs },
>>> + { PCI_DEVICE(PCI_VENDOR_ID_MARVELL_EXT, 0x9235),
>>> + .driver_data = board_ahci_no_debounce_delay },
>>> { PCI_DEVICE(PCI_VENDOR_ID_TTI, 0x0642), /* highpoint rocketraid 642L */
>>> .driver_data = board_ahci_yes_fbs },
>>> { PCI_DEVICE(PCI_VENDOR_ID_TTI, 0x0645), /* highpoint rocketraid 644L */
>>
>> Applied to for-5.18 with commit title and message changes. The title is now:
>>
>> ata: ahci: Add support for Marvell 88SE9235 adapter
>>
>> Since it is exactly what this patch is doing by adding a PCI ID.
>
> Thank you for applying the patch. I saw the summary/title change also
> with the other patch. I am sorry, but I totally disagree. Reading that
> summary/title in `git log --oneline`, it’s not clear at all, what the
> patch does, and the full description or diff has to be read. “Add
> support” for me means, that it was unsupported before, which is not true
> at all as the defaults were used.
Right... I did not consider the final entry in the ahci_pci_tbl table
which matches against the device against the PCI AHCI storage class.
So right you are. I will restore your original commit title/message.
Thanks for checking.
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] ata: ahci: Skip 200 ms debounce delay for Marvell 88SE9235
2022-02-02 23:05 ` Damien Le Moal
@ 2022-02-02 23:49 ` Damien Le Moal
0 siblings, 0 replies; 6+ messages in thread
From: Damien Le Moal @ 2022-02-02 23:49 UTC (permalink / raw)
To: Paul Menzel; +Cc: linux-ide, linux-kernel
On 2/3/22 08:05, Damien Le Moal wrote:
>>> ata: ahci: Add support for Marvell 88SE9235 adapter
>>>
>>> Since it is exactly what this patch is doing by adding a PCI ID.
>>
>> Thank you for applying the patch. I saw the summary/title change also
>> with the other patch. I am sorry, but I totally disagree. Reading that
>> summary/title in `git log --oneline`, it’s not clear at all, what the
>> patch does, and the full description or diff has to be read. “Add
>> support” for me means, that it was unsupported before, which is not true
>> at all as the defaults were used.
>
> Right... I did not consider the final entry in the ahci_pci_tbl table
> which matches against the device against the PCI AHCI storage class.
> So right you are. I will restore your original commit title/message.
>
> Thanks for checking.
Fixed now !
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-02-02 23:49 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-01 7:12 [PATCH v3] ata: ahci: Skip 200 ms debounce delay for Marvell 88SE9235 Paul Menzel
2022-02-01 8:27 ` Damien Le Moal
2022-02-02 8:14 ` Damien Le Moal
2022-02-02 14:29 ` Paul Menzel
2022-02-02 23:05 ` Damien Le Moal
2022-02-02 23:49 ` Damien Le Moal
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.