[PATCH v3] ata: ahci: Skip 200 ms debounce delay for Marvell 88SE9235

Paul Menzel posted 1 patch 4 years, 4 months ago
drivers/ata/ahci.c | 2 ++
1 file changed, 2 insertions(+)
[PATCH v3] ata: ahci: Skip 200 ms debounce delay for Marvell 88SE9235
Posted by Paul Menzel 4 years, 4 months ago
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

Re: [PATCH v3] ata: ahci: Skip 200 ms debounce delay for Marvell 88SE9235
Posted by Damien Le Moal 4 years, 4 months ago
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
Re: [PATCH v3] ata: ahci: Skip 200 ms debounce delay for Marvell 88SE9235
Posted by Damien Le Moal 4 years, 4 months ago
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
Re: [PATCH v3] ata: ahci: Skip 200 ms debounce delay for Marvell 88SE9235
Posted by Paul Menzel 4 years, 4 months ago
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
Re: [PATCH v3] ata: ahci: Skip 200 ms debounce delay for Marvell 88SE9235
Posted by Damien Le Moal 4 years, 4 months ago
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
Re: [PATCH v3] ata: ahci: Skip 200 ms debounce delay for Marvell 88SE9235
Posted by Damien Le Moal 4 years, 4 months ago
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