[PATCH v1 15/24] ata: ahci: Add BAR index quirk for Cavium PCI SATA device

[Simon Glass]

Hi Stefan,

On Thu, 30 Jul 2020 at 09:41, Stefan Roese <sr at denx.de> wrote:
>
> Hi Simon,
>
> On 28.07.20 21:01, Simon Glass wrote:
> > Hi Stefan,
> >
> > On Fri, 24 Jul 2020 at 04:09, Stefan Roese <sr at denx.de> wrote:
> >>
> >> From: Suneel Garapati <sgarapati at marvell.com>
> >>
> >> For SATA controller found on OcteonTX SoC's, use non-standard PCI BAR0
> >> instead of BAR5.
> >>
> >> Signed-off-by: Suneel Garapati <sgarapati at marvell.com>
> >> Cc: Simon Glass <sjg at chromium.org>
> >>
> >> Signed-off-by: Stefan Roese <sr at denx.de>
> >> ---
> >>
> >> Changes in v1:
> >> - Change patch subject
> >> - Use constants from pci_ids.h instead of hardcoded values
> >>
> >>   drivers/ata/ahci.c | 8 ++++++++
> >>   1 file changed, 8 insertions(+)
> >
> > Reviewed-by: Simon Glass <sjg at chromium.org>
> >
> >>
> >> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
> >> index 47cdea1f58..28161b5e62 100644
> >> --- a/drivers/ata/ahci.c
> >> +++ b/drivers/ata/ahci.c
> >> @@ -1198,10 +1198,18 @@ int ahci_probe_scsi(struct udevice *ahci_dev, ulong base)
> >>   int ahci_probe_scsi_pci(struct udevice *ahci_dev)
> >>   {
> >>          ulong base;
> >> +       u16 vendor, device;
> >>
> >>          base = (ulong)dm_pci_map_bar(ahci_dev, PCI_BASE_ADDRESS_5,
> >>                                       PCI_REGION_MEM);
> >>
> >> +       dm_pci_read_config16(ahci_dev, PCI_VENDOR_ID, &vendor);
> >> +       dm_pci_read_config16(ahci_dev, PCI_DEVICE_ID, &device);
> >> +
> >> +       if (vendor == PCI_VENDOR_ID_CAVIUM &&
> >> +           device == PCI_DEVICE_ID_CAVIUM_SATA)
> >> +               base = (uintptr_t)dm_pci_map_bar(ahci_dev, PCI_BASE_ADDRESS_0,
> >> +                                                PCI_REGION_MEM);
> >
> > How should we handle this in general? Should we have a Kconfig to
> > enable quirks in ahci?
>
> Perhaps we should wait a bit until, other "quirks" for AHCI get posted.
> Only then we can see, where and how these can be grouped and can
> "extract" them into Kconfig option(s). One quirk is the overhead of
> the additional Kconfig option / handling not worth, IMHO.
>
> What do you think?

Sounds OK to me. You could add a note about it perhaps.

Regards,
SImon