[U-Boot] [PATCH] ahci-pci: Update call to ahci_scsi_probe()

Bin Meng bmeng.cn at gmail.com
Thu Aug 3 01:42:03 UTC 2017


Hi Tom,

On Thu, Aug 3, 2017 at 9:32 AM, Tom Rini <trini at konsulko.com> wrote:
> On Thu, Aug 03, 2017 at 09:23:51AM +0800, Bin Meng wrote:
>> Hi Tom,
>>
>> On Thu, Aug 3, 2017 at 8:37 AM, Tom Rini <trini at konsulko.com> wrote:
>> > The function now takes a 'base' argument, and we can provide that by
>> > having dev_read_addr() get it from the struct uclass dev that we have
>> > been given.
>> >
>>
>> Oops. Thanks for catching this!
>>
>> > Cc: Bin Meng <bmeng.cn at gmail.com>
>> > Signed-off-by: Tom Rini <trini at konsulko.com>
>> > ---
>> >  drivers/ata/ahci-pci.c | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/ata/ahci-pci.c b/drivers/ata/ahci-pci.c
>> > index f46fad899e5a..beec14f46d66 100644
>> > --- a/drivers/ata/ahci-pci.c
>> > +++ b/drivers/ata/ahci-pci.c
>> > @@ -18,7 +18,7 @@ static int ahci_pci_bind(struct udevice *dev)
>> >
>> >  static int ahci_pci_probe(struct udevice *dev)
>> >  {
>> > -       return ahci_probe_scsi(dev);
>> > +       return ahci_probe_scsi(dev, dev_read_addr(dev));
>>
>> But dev_read_addr(dev) does not work here as the AHCI device is on PCI
>> bus. The base address should be read from PCI configuration space.
>> There is already an API that does this, see ahci_probe_scsi_pci().
>
> OK, so that calls dm_pci_map_bar for PCI_BASE_ADDRESS_5, which at first
> glance, I would have to ask why we're hard-coding that value in.  And
> then, I wonder why we have this code in ahci-pci.c but then
> ahci_probe_scsi_pci() as you note in drivers/ata/ahci.c.  Wouldn't both
> of these calls be the same code?  Can you please provide a patch that
> fixes ahci-pci.c?  It's breaking qemu-x86 and I let travis-ci (and then
> my lab, sigh, funky power issues) be broken long enough that some
> problems have slipped in to master :(, thanks!
>

PCI_BASE_ADDRESS_5 is hardcoded because that's defined by the AHCI
controller spec. ahci_probe_scsi_pci() is put in drivers/ata/ahci.c
because arch/x86/cpu/ivybridge/sata.c calls it too. The broken was not
that long enough, as it was just exposed when
http://patchwork.ozlabs.org/patch/796095/ and
http://patchwork.ozlabs.org/patch/796209/ were merged altogether at
the same day :)

I will prepare a patch ASAP. Thanks!

Regards,
Bin


More information about the U-Boot mailing list