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....
> >
> > 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.
Actually I did not get notice but this was being done inside
pci_assign_unassigned_bus_resources function, sorry.
I am a bit lost now. I am going to go again through the code and others controllers
code to see what could be wrong because it seems correct at first glance. Also the dmesg
related pci traces seems to be correct also before this "cannot assign" stuff messages...
>
> 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??
Sorry, Neil. Yes, you are right, I have changes but not relevant for this. I have some changes just parsing the nested
nodes of the pcie in device tree and filling the port's data structures. But we have to make this work before :-)
>
> Thanks,
> NeilBrown
Thanks for your time and patience.
Best regards,
Sergio Paracuellos
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
