On Wed, Aug 1, 2018 at 6:37 AM, Sergio Paracuellos
<sergio.paracuellos@xxxxxxxxx> wrote:
> 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
>
v8 sent already including this changes and setting resources limits also.
Best regards,
Sergio Paracuellos
> 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
