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. 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
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
