Message ID | 20200724100856.1482324-16-sr@denx.de |
---|---|
State | Superseded |
Delegated to: | Stefan Roese |
Headers | show |
Series | arm: Introduce Marvell/Cavium OcteonTX/TX2 | expand |
Hi Stefan, On Fri, 24 Jul 2020 at 04:09, Stefan Roese <sr@denx.de> wrote: > > From: Suneel Garapati <sgarapati@marvell.com> > > For SATA controller found on OcteonTX SoC's, use non-standard PCI BAR0 > instead of BAR5. > > Signed-off-by: Suneel Garapati <sgarapati@marvell.com> > Cc: Simon Glass <sjg@chromium.org> > > Signed-off-by: Stefan Roese <sr@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@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? > return ahci_probe_scsi(ahci_dev, base); > } > #endif > -- > 2.27.0 > Regards, Simon
Hi Simon, On 28.07.20 21:01, Simon Glass wrote: > Hi Stefan, > > On Fri, 24 Jul 2020 at 04:09, Stefan Roese <sr@denx.de> wrote: >> >> From: Suneel Garapati <sgarapati@marvell.com> >> >> For SATA controller found on OcteonTX SoC's, use non-standard PCI BAR0 >> instead of BAR5. >> >> Signed-off-by: Suneel Garapati <sgarapati@marvell.com> >> Cc: Simon Glass <sjg@chromium.org> >> >> Signed-off-by: Stefan Roese <sr@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@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? Thanks, Stefan
Hi Stefan, On Thu, 30 Jul 2020 at 09:41, Stefan Roese <sr@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@denx.de> wrote: > >> > >> From: Suneel Garapati <sgarapati@marvell.com> > >> > >> For SATA controller found on OcteonTX SoC's, use non-standard PCI BAR0 > >> instead of BAR5. > >> > >> Signed-off-by: Suneel Garapati <sgarapati@marvell.com> > >> Cc: Simon Glass <sjg@chromium.org> > >> > >> Signed-off-by: Stefan Roese <sr@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@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
Hi Simon, On 31.07.20 20:35, Simon Glass wrote: > Hi Stefan, > > On Thu, 30 Jul 2020 at 09:41, Stefan Roese <sr@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@denx.de> wrote: >>>> >>>> From: Suneel Garapati <sgarapati@marvell.com> >>>> >>>> For SATA controller found on OcteonTX SoC's, use non-standard PCI BAR0 >>>> instead of BAR5. >>>> >>>> Signed-off-by: Suneel Garapati <sgarapati@marvell.com> >>>> Cc: Simon Glass <sjg@chromium.org> >>>> >>>> Signed-off-by: Stefan Roese <sr@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@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. Okay, will do. Thanks, Stefan
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); return ahci_probe_scsi(ahci_dev, base); } #endif