From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: MIME-Version: 1.0 In-Reply-To: <87a7qeob0w.fsf@notabene.neil.brown.name> References: <1531756403-7197-1-git-send-email-sergio.paracuellos@gmail.com> <874lgop95c.fsf@notabene.neil.brown.name> <20180725074756.GA28389@foobar> <87a7qeob0w.fsf@notabene.neil.brown.name> From: Sergio Paracuellos Date: Thu, 26 Jul 2018 07:59:53 +0200 Message-ID: Subject: Re: [PATCH v4 00/15] staging: mt7621-pci: avoid custom pci config read and writes Content-Type: text/plain; charset="UTF-8" List-ID: To: NeilBrown Cc: Greg KH , driverdev-devel@linuxdriverproject.org On Thu, Jul 26, 2018 at 6:50 AM, NeilBrown 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. > > Thanks, > NeilBrown Best regards, Sergio Paracuellos