On Wed, Aug 01, 2018 at 07:56:38AM +1000, NeilBrown wrote:
> On Tue, Jul 31 2018, Sergio Paracuellos wrote:
>
> > This patch series include an attempt to avoid the use of custom
> > read and writes in driver code and use PCI subsystem common ones.
> >
> > In order to do this 'map_bus' callback is implemented and also
> > data structures for driver are included. The regs base address
> > ranges and data is being readed from device tree and the driver
> > gets clean a lot of code.
> >
> > This patchet also removes all legacy PCI code using now PCI_DRIVERS_GENERIC
> > kernel api.
> >
> > Changes in v7:
> > - PATCH 1: Store resources in mt7621_pci data structure.
> > - PATCH 1: Change completely function mt7621_pci_parse_request_of_pci_ranges
> > to parse resources from ranges manually instead of use the function
> > devm_of_pci_get_host_bridge_resources. This is closer to the mips pci legacy
> > code.
> > - PATCH 1: Create 'mt7621_pcie_request_resources' function to request resources
> > parsed from ranges property in the DT. Use pci_add_resource_offset and set them
> > manually like the mips pci-legacy code do.
> > - PATCH 1: don't delete function setup_cm_memory_region and call it with memory
> > resource.
> > - Other patches rebased and adapted to this changes.
> >
>
> No good, sorry.
>
> mt7621_pci_parse_request_of_pci_ranges()
> calls of_pci_range_to_resource() which, for IO resources,
> calls
> port = pci_address_to_pio(range->cpu_addr);
> ->cpu_addr is 1e160000
> and pci_address_to_pci sees that this is larger that IO_SPACE_LIMIT
> (0xffff) and returns -1.
> So the pci probe failed.
>
> Maybe mips should have a arch-specific pci_address_to_pio, which does
> the setp_cm_memory_region() thing.... just a random guess really.
>
> Though if I hack pci_address_to_pio() to succeed, I get:
>
> [ 1.990000] mt7621-pci 1e140000.pcie: resource collision: [io 0x1e160000-0x1e16ffff] conflicts with PCI IO [io 0x0000-0xffff]
>
> which looks a little weird ... why do those conflict? Maybe because
> everything has to fit into "PCI IO"..
> I'm getting lost...
Mmmm, So that seems to be the reason about why the result of of_pci_range_to_resource is
not checked in the legacy code:
See arch/mips/pci/pci-legacy.c +137 (pci_load_of_ranges function). We can try if no checking
it change the things and call in the same places the legacy code does change things. Just apply this (diff is against PATCH 1):
diff --git a/drivers/staging/mt7621-pci/pci-mt7621.c b/drivers/staging/mt7621-pci/pci-mt7621.c
index 4e8958b..a7f31cd 100644
--- a/drivers/staging/mt7621-pci/pci-mt7621.c
+++ b/drivers/staging/mt7621-pci/pci-mt7621.c
@@ -545,9 +545,7 @@ static int mt7621_pci_parse_request_of_pci_ranges(struct mt7621_pcie *pcie)
}
for_each_of_pci_range(&parser, &range) {
- err = of_pci_range_to_resource(&range, node, &res);
- if (err < 0)
- return err;
+ of_pci_range_to_resource(&range, node, &res);
switch (res.flags & IORESOURCE_TYPE_BITS) {
case IORESOURCE_IO:
@@ -569,7 +567,6 @@ static int mt7621_pci_parse_request_of_pci_ranges(struct mt7621_pcie *pcie)
memcpy(&pcie->mem, &res, sizeof(res));
pcie->mem.name = "non-prefetchable";
- setup_cm_memory_region(&pcie->mem);
break;
}
}
@@ -610,9 +607,6 @@ static int mt7621_pcie_parse_dt(struct mt7621_pcie *pcie)
if (IS_ERR(pcie->base))
return PTR_ERR(pcie->base);
- err = mt7621_pci_parse_request_of_pci_ranges(pcie);
- if (err)
- return err;
return 0;
}
@@ -876,6 +870,12 @@ pcie(2/1/0) link status pcie2_num pcie1_num pcie0_num
write_config(0, 0, 0, 0x70c, val);
}
+ err = mt7621_pci_parse_request_of_pci_ranges(pcie);
+ if (err)
+ return err;
+
+ setup_cm_memory_region(&pcie->mem);
+
err = mt7621_pcie_request_resources(pcie);
if (err) {
dev_err(dev, "Error requesting resources\n");
>
> Thanks,
> NeilBrown
If this does not work I get completely lost :-(.
Thanks for your time and effort in this.
Best regards,
Sergio Paracuellos
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
