On Thu, Jul 26, 2018 at 04:45:41PM +1000, NeilBrown wrote:
> On Thu, Jul 26 2018, Sergio Paracuellos wrote:
>
> > On Thu, Jul 26, 2018 at 6:50 AM, NeilBrown <neil@xxxxxxxxxx> wrote:
> >> On Wed, Jul 25 2018, Sergio Paracuellos wrote:
> >>
> >>> On Wed, Jul 25, 2018 at 08:21:35AM +1000, NeilBrown wrote:
> >>>> On Mon, Jul 16 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
> >>>> > is being readed from device tree and the driver gets clean a lot
> >>>> > of code.
> >>>> >
> >>>> > Changes in v4:
> >>>> > - Rebased onto staging-next.
> >>>> >
> >>>> > Changes in v3:
> >>>> > - Include new patches to delete all RALINK_BASE definition
> >>>> > dependant code and be able to avoid use of pci_legacy code.
> >>>> > - use devm_of_pci_get_host_bridge_resources,
> >>>> > devm_request_pci_bus_resources and pci_scan_root_bus_bridge
> >>>> > and pci_bus_add_devices
> >>>> >
> >>>> > Changes in v2:
> >>>> > - squash PATCH 1 and PATCH 2 of previous series in only PATCH 1
> >>>> > - Change name for host structure.
> >>>> > - Create a new port structure (platform has 3 pcie controllers)
> >>>> > - Replace the use of pci_generic_config_[read|write]32 in favour
> >>>> > of pci_generic_config_[read|write] and change map_bus implemen-
> >>>> > tation for hopefully the right one.
> >>>> >
> >>>> > Best regards,
> >>>>
> >>>> Thanks for these.
> >>>> I added
> >>>> diff --git a/arch/mips/ralink/Kconfig b/arch/mips/ralink/Kconfig
> >>>> index 1f9cb0e3c79a..50779b3379db 100644
> >>>> --- a/arch/mips/ralink/Kconfig
> >>>> +++ b/arch/mips/ralink/Kconfig
> >>>> @@ -51,6 +51,7 @@ choice
> >>>> select COMMON_CLK
> >>>> select CLKSRC_MIPS_GIC
> >>>> select HW_HAS_PCI
> >>>> + select PCI_DRIVERS_GENERIC
> >>>> endchoice
> >>>>
> >>>> choice
> >>>>
> >>>> so that I could build and test it - maybe there is somewhere else that
> >>>> "select" can go while this is still in staging..
> >>>>
> >>>> The system boots and can see the three pcie-attached SATA controllers:
> >>>>
> >>>> # lspci
> >>>> 00:00.0 PCI bridge: Device 0e8d:0801 (rev 01)
> >>>> 00:01.0 PCI bridge: Device 0e8d:0801 (rev 01)
> >>>> 00:02.0 PCI bridge: Device 0e8d:0801 (rev 01)
> >>>> 01:00.0 IDE interface: ASMedia Technology Inc. ASM1061 SATA IDE Controller (rev 02)
> >>>> 02:00.0 IDE interface: ASMedia Technology Inc. ASM1061 SATA IDE Controller (rev 02)
> >>>> 03:00.0 IDE interface: ASMedia Technology Inc. ASM1061 SATA IDE Controller (rev 02)
> >>>>
> >>>> but it cannot see the drive that is plugged into one of these.
> >>>> Below is the first 10 seconds of dmesg.
> >>>> I suspect the relevant bit is
> >>>>
> >>>> [ 8.680000] ahci: probe of 0000:01:00.0 failed with error -22
> >>>> [ 8.700000] ahci: probe of 0000:02:00.0 failed with error -22
> >>>> [ 8.710000] ahci: probe of 0000:03:00.0 failed with error -22
> >>>>
> >>>> I haven't dug deeper yet.
> >>>
> >>> Hi Neil,
> >>>
> >>> Can you please make a test for me? Can you comment lines about pinctrl in the pcie
> >>> node of the device tree? I am not sure we have to use the reset pin there but just
> >>> use the reset_control in a proper way. These two:
> >>>
> >>> pinctrl-names = "default";
> >>> pinctrl-0 = <&pcie_pins>;
> >>>
> >>> Does this change make the plugged drives to work?
> >>
> >> Thanks for the suggestion. No, this does change anything.
> >> I had a go at sprinking printks to see what exactly was failing.
> >> It is pcim_iomap_regions().
> >> "mask" is 0x20, is t wants to map region 5.
> >> However region 5 has size zero - hence -EINVAL.
> >
> > Thanks for this analysis. It is really helpful.
> >
> >>
> >> In the dmesg there is:
> >>
> >>>> [ 2.530000] pci 0000:01:00.0: BAR 5: no space for [mem size 0x00000200]
> >>>> [ 2.540000] pci 0000:01:00.0: BAR 5: failed to assign [mem size 0x00000200]
> >>
> >> I think this is where resource 5 is not getting set up properly.
> >> That is as far as I got today.
> >
> > It seems there are not space in bridges to assign to the downstream devices...
> >
> > As far as I know Linux assigns space for endpoint BARs, but it doesn't
> > automatically
> > reassign bridge windows to make space for downstream devices. We can
> > try two things.
> >
> > 1) Call pci_bus_size_bridges as follows (if you prefer I can send v5
> > including this change in the
> > first PATCH of the series, but I think it is better to test this
> > before sending anything):
> >
> > diff --git a/drivers/staging/mt7621-pci/pci-mt7621.c
> > b/drivers/staging/mt7621-pci/pci-mt7621.c
> > index f8e81aa..1f329d6 100644
> > --- a/drivers/staging/mt7621-pci/pci-mt7621.c
> > +++ b/drivers/staging/mt7621-pci/pci-mt7621.c
> > @@ -629,6 +629,7 @@ pcie(2/1/0) link status pcie2_num
> > pcie1_num pcie0_num
> >
> > bus = bridge->bus;
> >
> > + pci_bus_size_bridges(bridge->bus);
> > pci_assign_unassigned_bus_resources(bridge->bus);
> > list_for_each_entry(child, &bus->children, node)
> > pcie_bus_configure_settings(child);
> >
> > I think after this change the problem should disappear but if that is
> > not the case....
Ok, I think the problem is we are not setting the bridge->windows retrieved
with devm_request_pci_bus_resources in "res". So we have to set those properly
to the bridge to get all correctly assigned. So I think adding this should make
the system to work:
+ list_splice_init(&res, &bridge->windows);
bridge->busnr = 0;
bridge->dev.parent = dev;
bridge->sysdata = pcie;
(Sorry don't access to code now and cannot diff).
Let me know if this works. Is this the hopefully good one?
Best regards,
Sergio Paracuellos
> >
> > 2) if that does not work, we can try also booting with "pci=realloc"
> > and see if there is any difference in dmesg.
> >
> > Thanks in advance.
>
> Neither of these suggestions make any noticeable difference.
>
> I'm a bit confused by something thought - your patch above puts the new
> line at line 633 of pci-mt7621.c. It is line 584 in my code.
> There are only 614 lines.
> Do you have other code in there that might be relevant??
>
> Thanks,
> NeilBrown
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
