On 2/13/24 22:07, Niklas Cassel wrote:
> Most quirks are applied using a specific board type board_ahci_no*
> (e.g. board_ahci_nomsi, board_ahci_noncq), which then sets a flag
> representing the specific quirk.
>
> ahci_pci_tbl (which is the table of all supported PCI devices), then
> uses that board type for the PCI vendor and device IDs which need to
> be quirked.
>
> The ahci_broken_devslp quirk is not implemented in this standard way.
>
> Modify the ahci_broken_devslp quirk to be implemented like the other
> quirks. This way, we will not have the same PCI device and vendor ID
> scattered over ahci.c. It will simply be defined in a single location.
>
> Suggested-by: Dan Williams <dan.j.williams@xxxxxxxxx>
> Signed-off-by: Niklas Cassel <cassel@xxxxxxxxxx>
> ---
> drivers/ata/ahci.c | 26 ++++++++++----------------
> 1 file changed, 10 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
> index c28ad3f4b59e..1e1533d01803 100644
> --- a/drivers/ata/ahci.c
> +++ b/drivers/ata/ahci.c
> @@ -51,6 +51,7 @@ enum board_ids {
> board_ahci_43bit_dma,
> board_ahci_ign_iferr,
> board_ahci_no_debounce_delay,
> + board_ahci_no_devslp_pcs_quirk,
> board_ahci_nomsi,
> board_ahci_noncq,
> board_ahci_nosntf_pcs_quirk,
> @@ -151,6 +152,14 @@ static const struct ata_port_info ahci_port_info[] = {
> .udma_mask = ATA_UDMA6,
> .port_ops = &ahci_ops,
> },
> + [board_ahci_no_devslp_pcs_quirk] = {
> + AHCI_HFLAGS (AHCI_HFLAG_NO_DEVSLP |
> + AHCI_HFLAG_INTEL_PCS_QUIRK),
> + .flags = AHCI_FLAG_COMMON,
> + .pio_mask = ATA_PIO4,
> + .udma_mask = ATA_UDMA6,
> + .port_ops = &ahci_ops,
> + },
> [board_ahci_nomsi] = {
> AHCI_HFLAGS (AHCI_HFLAG_NO_MSI),
> .flags = AHCI_FLAG_COMMON,
> @@ -420,7 +429,7 @@ static const struct pci_device_id ahci_pci_tbl[] = {
> { PCI_VDEVICE(INTEL, 0x06d7), board_ahci_pcs_quirk }, /* Comet Lake-H RAID */
> { PCI_VDEVICE(INTEL, 0xa386), board_ahci_pcs_quirk }, /* Comet Lake PCH-V RAID */
> { PCI_VDEVICE(INTEL, 0x0f22), board_ahci_pcs_quirk }, /* Bay Trail AHCI */
> - { PCI_VDEVICE(INTEL, 0x0f23), board_ahci_pcs_quirk }, /* Bay Trail AHCI */
> + { PCI_VDEVICE(INTEL, 0x0f23), board_ahci_no_devslp_pcs_quirk }, /* Bay Trail AHCI */
Same nit as the previous patch: let's name this board_ahci_pcs_quirk_no_devslp.
With that, looks OK to me.
Reviewed-by: Damien Le Moal <dlemoal@xxxxxxxxxx>
> { PCI_VDEVICE(INTEL, 0x22a3), board_ahci_pcs_quirk }, /* Cherry Tr. AHCI */
> { PCI_VDEVICE(INTEL, 0x5ae3), board_ahci_pcs_quirk }, /* ApolloLake AHCI */
> { PCI_VDEVICE(INTEL, 0x34d3), board_ahci_pcs_quirk }, /* Ice Lake LP AHCI */
> @@ -1420,17 +1429,6 @@ static bool ahci_broken_online(struct pci_dev *pdev)
> return pdev->bus->number == (val >> 8) && pdev->devfn == (val & 0xff);
> }
>
> -static bool ahci_broken_devslp(struct pci_dev *pdev)
> -{
> - /* device with broken DEVSLP but still showing SDS capability */
> - static const struct pci_device_id ids[] = {
> - { PCI_VDEVICE(INTEL, 0x0f23)}, /* Valleyview SoC */
> - {}
> - };
> -
> - return pci_match_id(ids, pdev);
> -}
> -
> #ifdef CONFIG_ATA_ACPI
> static void ahci_gtf_filter_workaround(struct ata_host *host)
> {
> @@ -1823,10 +1821,6 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
> &dev_attr_remapped_nvme.attr,
> NULL);
>
> - /* must set flag prior to save config in order to take effect */
> - if (ahci_broken_devslp(pdev))
> - hpriv->flags |= AHCI_HFLAG_NO_DEVSLP;
> -
> #ifdef CONFIG_ARM64
> if (pdev->vendor == PCI_VENDOR_ID_HUAWEI &&
> pdev->device == 0xa235 &&
--
Damien Le Moal
Western Digital Research
