Re: [PATCH v4 00/15] staging: mt7621-pci: avoid custom pci config read and writes

2018-07-26 Thread Sergio Paracuellos
On Thu, Jul 26, 2018 at 1:24 PM, Sergio Paracuellos
 wrote:
> On Thu, Jul 26, 2018 at 08:02:17PM +1000, NeilBrown wrote:
>> On Thu, Jul 26 2018, Sergio Paracuellos wrote:
>>
>> >
>> > 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(, >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?
>>
>> This one, at least, makes a difference.
>>
>> I now see
>> [2.42] pci :01:00.0: BAR 5: assigned [mem 0x6000-0x61ff]
>>
>> which is promising.
>
> Yes, this and also the dmesg are promising and seems resources are correctly
> assigned now.
>
>>
>> However it gets to:
>>
>> [8.62] pci :00:00.0: enabling device (0004 -> 0006)
>> [8.64] ahci :01:00.0: enabling device ( -> 0002)
>>
>
> So after setup the PCI system topology is the driver responsability to enable
> the device. It means only make the hardware memory space accesible and
> this is the (-> 0002) of the log message (set the memory space bit of PCI 
> COMMAND
> register to 1 to make it accesible to allow the device to respond to memory 
> space acceses).
> So it is correct the behaviour of trying to enable this but it should not 
> hang. This trace is in
> drivers/pci/setup-res.c +495 (function pci_enable_resources). It would be 
> helpful
> a more deeper debug to know exactly where it really hangs.

I was thinking a reason for this to hang could be the access bridge
has not set the "bus master" bit enabled in its
PCI_COMMAND register but it should be set because this is done inside
"pci_assign_unassigned_bridge_resources"
concretely when "pci_enable_bridges(parent);" is called. This function
internally call pci_set_master(dev); that should set the bit active
for each bridge.

Best regards,
Sergio Paracuellos

>
> Thanks in advance.
>
>> and hangs.
>>
>> Full dmesg pasted below.
>>
>> Thanks,
>> NeilBrown
>
> Best regards,
> Sergio Paracuellos
>>
>>
>> [0.00] Linux version 4.18.0-rc5+ (neilb@noble) (gcc version 7.2.0 
>> (GCC)) #231 SMP Thu Jul 26 19:57:52 AEST 2018
>> [0.00] SoC Type: MediaTek MT7621 ver:1 eco:3
>> [0.00] bootconsole [early0] enabled
>> [0.00] CPU0 revision is: 0001992f (MIPS 1004Kc)
>> [0.00] MIPS: machine is GB-PC1
>> [0.00] Determined physical RAM map:
>> [0.00]  memory: 1c00 @  (usable)
>> [0.00]  memory: 0400 @ 2000 (usable)
>> [0.00] Initrd not found or empty - disabling initrd
>> [0.00] VPE topology {2,2} total 4
>> [0.00] Primary instruction cache 32kB, VIPT, 4-way, linesize 32 
>> bytes.
>> [0.00] Primary data cache 32kB, 4-way, PIPT, no aliases, linesize 32 
>> bytes
>> [0.00] MIPS secondary cache 256kB, 8-way, linesize 32 bytes.
>> [0.00] Zone ranges:
>> [0.00]   Normal   [mem 0x-0x1fff]
>> [0.00]   HighMem  [mem 0x2000-0x23ff]
>> [0.00] Movable zone start for each node
>> [0.00] Early memory node ranges
>> [0.00]   node   0: [mem 0x-0x1bff]
>> [0.00]   node   0: [mem 0x2000-0x23ff]
>> [0.00] Initmem setup node 0 [mem 
>> 0x-0x23ff]
>> [0.00] random: get_random_bytes called from start_kernel+0xb4/0x4ec 
>> with crng_init=0
>> [0.00] percpu: Embedded 15 pages/cpu @(ptrval) s30480 r8192 d22768 
>> u61440
>> [0.00] Built 1 zonelists, mobility grouping on.  Total pages: 130048
>> [0.00] Kernel command line: console=ttyS0,57600
>> [0.00] Dentry cache hash table entries: 65536 (order: 6, 262144 
>> bytes)
>> [0.00] Inode-cache hash table entries: 32768 (order: 5, 131072 bytes)
>> [0.00] Writing ErrCtl register=00010882
>> [0.00] Readback ErrCtl register=00010882
>> [0.00] Memory: 504788K/524288K available (6131K kernel code, 232K 
>> rwdata, 1052K rodata, 6524K init, 241K bss, 19500K reserved, 0K 
>> cma-reserved, 65536K highmem)
>> [0.00] SLUB: HWalign=32, Order=0-3, MinObjects=0, CPUs=4, Nodes=1
>> [0.00] Hierarchical RCU implementation.
>> [0.00] NR_IRQS: 256
>> [0.00] clocksource: GIC: mask: 0x max_cycles: 
>> 0xcf914c9718, max_idle_ns: 440795231327 ns
>> [0.00] sched_clock: 32 bits at 100 Hz, resolution 1000ns, wraps 
>> every 2147483647500ns
>> [0.01] Calibrating delay loop... 597.60 BogoMIPS (lpj=2988032)
>> [0.07] pid_max: default: 32768 minimum: 301
>> [0.08] Mount-cache 

Re: [PATCH v4 00/15] staging: mt7621-pci: avoid custom pci config read and writes

2018-07-26 Thread Sergio Paracuellos
On Thu, Jul 26, 2018 at 08:02:17PM +1000, NeilBrown wrote:
> On Thu, Jul 26 2018, Sergio Paracuellos wrote:
> 
> >
> > 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(, >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?
> 
> This one, at least, makes a difference.
> 
> I now see
> [2.42] pci :01:00.0: BAR 5: assigned [mem 0x6000-0x61ff]
> 
> which is promising.

Yes, this and also the dmesg are promising and seems resources are correctly
assigned now.

> 
> However it gets to:
> 
> [8.62] pci :00:00.0: enabling device (0004 -> 0006)
> [8.64] ahci :01:00.0: enabling device ( -> 0002)
> 

So after setup the PCI system topology is the driver responsability to enable
the device. It means only make the hardware memory space accesible and
this is the (-> 0002) of the log message (set the memory space bit of PCI 
COMMAND
register to 1 to make it accesible to allow the device to respond to memory 
space acceses).
So it is correct the behaviour of trying to enable this but it should not hang. 
This trace is in
drivers/pci/setup-res.c +495 (function pci_enable_resources). It would be 
helpful
a more deeper debug to know exactly where it really hangs. 

Thanks in advance.

> and hangs.
> 
> Full dmesg pasted below.
> 
> Thanks,
> NeilBrown

Best regards,
Sergio Paracuellos
> 
> 
> [0.00] Linux version 4.18.0-rc5+ (neilb@noble) (gcc version 7.2.0 
> (GCC)) #231 SMP Thu Jul 26 19:57:52 AEST 2018
> [0.00] SoC Type: MediaTek MT7621 ver:1 eco:3
> [0.00] bootconsole [early0] enabled
> [0.00] CPU0 revision is: 0001992f (MIPS 1004Kc)
> [0.00] MIPS: machine is GB-PC1
> [0.00] Determined physical RAM map:
> [0.00]  memory: 1c00 @  (usable)
> [0.00]  memory: 0400 @ 2000 (usable)
> [0.00] Initrd not found or empty - disabling initrd
> [0.00] VPE topology {2,2} total 4
> [0.00] Primary instruction cache 32kB, VIPT, 4-way, linesize 32 bytes.
> [0.00] Primary data cache 32kB, 4-way, PIPT, no aliases, linesize 32 
> bytes
> [0.00] MIPS secondary cache 256kB, 8-way, linesize 32 bytes.
> [0.00] Zone ranges:
> [0.00]   Normal   [mem 0x-0x1fff]
> [0.00]   HighMem  [mem 0x2000-0x23ff]
> [0.00] Movable zone start for each node
> [0.00] Early memory node ranges
> [0.00]   node   0: [mem 0x-0x1bff]
> [0.00]   node   0: [mem 0x2000-0x23ff]
> [0.00] Initmem setup node 0 [mem 
> 0x-0x23ff]
> [0.00] random: get_random_bytes called from start_kernel+0xb4/0x4ec 
> with crng_init=0
> [0.00] percpu: Embedded 15 pages/cpu @(ptrval) s30480 r8192 d22768 
> u61440
> [0.00] Built 1 zonelists, mobility grouping on.  Total pages: 130048
> [0.00] Kernel command line: console=ttyS0,57600
> [0.00] Dentry cache hash table entries: 65536 (order: 6, 262144 bytes)
> [0.00] Inode-cache hash table entries: 32768 (order: 5, 131072 bytes)
> [0.00] Writing ErrCtl register=00010882
> [0.00] Readback ErrCtl register=00010882
> [0.00] Memory: 504788K/524288K available (6131K kernel code, 232K 
> rwdata, 1052K rodata, 6524K init, 241K bss, 19500K reserved, 0K cma-reserved, 
> 65536K highmem)
> [0.00] SLUB: HWalign=32, Order=0-3, MinObjects=0, CPUs=4, Nodes=1
> [0.00] Hierarchical RCU implementation.
> [0.00] NR_IRQS: 256
> [0.00] clocksource: GIC: mask: 0x max_cycles: 
> 0xcf914c9718, max_idle_ns: 440795231327 ns
> [0.00] sched_clock: 32 bits at 100 Hz, resolution 1000ns, wraps 
> every 2147483647500ns
> [0.01] Calibrating delay loop... 597.60 BogoMIPS (lpj=2988032)
> [0.07] pid_max: default: 32768 minimum: 301
> [0.08] Mount-cache hash table entries: 1024 (order: 0, 4096 bytes)
> [0.09] Mountpoint-cache hash table entries: 1024 (order: 0, 4096 
> bytes)
> [0.10] Hierarchical SRCU implementation.
> [0.11] smp: Bringing up secondary CPUs ...
> [0.12] Primary instruction cache 32kB, VIPT, 4-way, linesize 32 bytes.
> [0.12] Primary data cache 32kB, 4-way, PIPT, no aliases, linesize 32 
> bytes
> [0.12] MIPS secondary cache 256kB, 8-way, linesize 32 bytes.
> [0.12] CPU1 revision is: 0001992f (MIPS 1004Kc)
> [0.18] Synchronize counters for CPU 1: done.
> [0.22] 

Re: [PATCH v4 00/15] staging: mt7621-pci: avoid custom pci config read and writes

2018-07-26 Thread NeilBrown
On Thu, Jul 26 2018, Sergio Paracuellos wrote:

>
> 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(, >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?

This one, at least, makes a difference.

I now see
[2.42] pci :01:00.0: BAR 5: assigned [mem 0x6000-0x61ff]

which is promising.

However it gets to:

[8.62] pci :00:00.0: enabling device (0004 -> 0006)
[8.64] ahci :01:00.0: enabling device ( -> 0002)

and hangs.

Full dmesg pasted below.

Thanks,
NeilBrown


[0.00] Linux version 4.18.0-rc5+ (neilb@noble) (gcc version 7.2.0 
(GCC)) #231 SMP Thu Jul 26 19:57:52 AEST 2018
[0.00] SoC Type: MediaTek MT7621 ver:1 eco:3
[0.00] bootconsole [early0] enabled
[0.00] CPU0 revision is: 0001992f (MIPS 1004Kc)
[0.00] MIPS: machine is GB-PC1
[0.00] Determined physical RAM map:
[0.00]  memory: 1c00 @  (usable)
[0.00]  memory: 0400 @ 2000 (usable)
[0.00] Initrd not found or empty - disabling initrd
[0.00] VPE topology {2,2} total 4
[0.00] Primary instruction cache 32kB, VIPT, 4-way, linesize 32 bytes.
[0.00] Primary data cache 32kB, 4-way, PIPT, no aliases, linesize 32 
bytes
[0.00] MIPS secondary cache 256kB, 8-way, linesize 32 bytes.
[0.00] Zone ranges:
[0.00]   Normal   [mem 0x-0x1fff]
[0.00]   HighMem  [mem 0x2000-0x23ff]
[0.00] Movable zone start for each node
[0.00] Early memory node ranges
[0.00]   node   0: [mem 0x-0x1bff]
[0.00]   node   0: [mem 0x2000-0x23ff]
[0.00] Initmem setup node 0 [mem 0x-0x23ff]
[0.00] random: get_random_bytes called from start_kernel+0xb4/0x4ec 
with crng_init=0
[0.00] percpu: Embedded 15 pages/cpu @(ptrval) s30480 r8192 d22768 
u61440
[0.00] Built 1 zonelists, mobility grouping on.  Total pages: 130048
[0.00] Kernel command line: console=ttyS0,57600
[0.00] Dentry cache hash table entries: 65536 (order: 6, 262144 bytes)
[0.00] Inode-cache hash table entries: 32768 (order: 5, 131072 bytes)
[0.00] Writing ErrCtl register=00010882
[0.00] Readback ErrCtl register=00010882
[0.00] Memory: 504788K/524288K available (6131K kernel code, 232K 
rwdata, 1052K rodata, 6524K init, 241K bss, 19500K reserved, 0K cma-reserved, 
65536K highmem)
[0.00] SLUB: HWalign=32, Order=0-3, MinObjects=0, CPUs=4, Nodes=1
[0.00] Hierarchical RCU implementation.
[0.00] NR_IRQS: 256
[0.00] clocksource: GIC: mask: 0x max_cycles: 
0xcf914c9718, max_idle_ns: 440795231327 ns
[0.00] sched_clock: 32 bits at 100 Hz, resolution 1000ns, wraps 
every 2147483647500ns
[0.01] Calibrating delay loop... 597.60 BogoMIPS (lpj=2988032)
[0.07] pid_max: default: 32768 minimum: 301
[0.08] Mount-cache hash table entries: 1024 (order: 0, 4096 bytes)
[0.09] Mountpoint-cache hash table entries: 1024 (order: 0, 4096 bytes)
[0.10] Hierarchical SRCU implementation.
[0.11] smp: Bringing up secondary CPUs ...
[0.12] Primary instruction cache 32kB, VIPT, 4-way, linesize 32 bytes.
[0.12] Primary data cache 32kB, 4-way, PIPT, no aliases, linesize 32 
bytes
[0.12] MIPS secondary cache 256kB, 8-way, linesize 32 bytes.
[0.12] CPU1 revision is: 0001992f (MIPS 1004Kc)
[0.18] Synchronize counters for CPU 1: done.
[0.22] Primary instruction cache 32kB, VIPT, 4-way, linesize 32 bytes.
[0.22] Primary data cache 32kB, 4-way, PIPT, no aliases, linesize 32 
bytes
[0.22] MIPS secondary cache 256kB, 8-way, linesize 32 bytes.
[0.22] CPU2 revision is: 0001992f (MIPS 1004Kc)
[0.28] Synchronize counters for CPU 2: done.
[0.32] Primary instruction cache 32kB, VIPT, 4-way, linesize 32 bytes.
[0.32] Primary data cache 32kB, 4-way, PIPT, no aliases, linesize 32 
bytes
[0.32] MIPS secondary cache 256kB, 8-way, linesize 32 bytes.
[0.32] CPU3 revision is: 0001992f (MIPS 1004Kc)
[0.38] Synchronize counters for CPU 3: done.
[0.42] smp: Brought up 1 node, 4 CPUs
[0.43] devtmpfs: initialized
[0.48] clocksource: jiffies: mask: 0x max_cycles: 0x, 
max_idle_ns: 1911260446275 ns
[0.49] futex hash table entries: 1024 (order: 3, 32768 bytes)
[0.50] pinctrl core: initialized pinctrl 

Re: [PATCH v4 00/15] staging: mt7621-pci: avoid custom pci config read and writes

2018-07-26 Thread Sergio Paracuellos
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  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.68] ahci: probe of :01:00.0 failed with error -22
>  [8.70] ahci: probe of :02:00.0 failed with error -22
>  [8.71] ahci: probe of :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 = <_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.53] pci :01:00.0: BAR 5: no space for [mem size 
>  0x0200]
>  [2.54] pci :01:00.0: BAR 5: failed to assign [mem size 
>  0x0200]
> >>
> >> 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
> > --- 

Re: [PATCH v4 00/15] staging: mt7621-pci: avoid custom pci config read and writes

2018-07-26 Thread Sergio Paracuellos
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  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.68] ahci: probe of :01:00.0 failed with error -22
>  [8.70] ahci: probe of :02:00.0 failed with error -22
>  [8.71] ahci: probe of :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 = <_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.53] pci :01:00.0: BAR 5: no space for [mem size 
>  0x0200]
>  [2.54] pci :01:00.0: BAR 5: failed to assign [mem size 
>  0x0200]
> >>
> >> 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
> > --- 

Re: [PATCH v4 00/15] staging: mt7621-pci: avoid custom pci config read and writes

2018-07-26 Thread NeilBrown
On Thu, Jul 26 2018, Sergio Paracuellos wrote:

> 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.68] ahci: probe of :01:00.0 failed with error -22
 [8.70] ahci: probe of :02:00.0 failed with error -22
 [8.71] ahci: probe of :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 = <_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.53] pci :01:00.0: BAR 5: no space for [mem size 0x0200]
 [2.54] pci :01:00.0: BAR 5: failed to assign [mem size 
 0x0200]
>>
>> 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);
> 

Re: [PATCH v4 00/15] staging: mt7621-pci: avoid custom pci config read and writes

2018-07-26 Thread Sergio Paracuellos
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.68] ahci: probe of :01:00.0 failed with error -22
>>> [8.70] ahci: probe of :02:00.0 failed with error -22
>>> [8.71] ahci: probe of :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 = <_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.53] pci :01:00.0: BAR 5: no space for [mem size 0x0200]
>>> [2.54] pci :01:00.0: BAR 5: failed to assign [mem size 
>>> 0x0200]
>
> 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, >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 

Re: [PATCH v4 00/15] staging: mt7621-pci: avoid custom pci config read and writes

2018-07-25 Thread NeilBrown
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.68] ahci: probe of :01:00.0 failed with error -22
>> [8.70] ahci: probe of :02:00.0 failed with error -22
>> [8.71] ahci: probe of :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 = <_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.

In the dmesg there is:

>> [2.53] pci :01:00.0: BAR 5: no space for [mem size 0x0200]
>> [2.54] pci :01:00.0: BAR 5: failed to assign [mem size 
>> 0x0200]

I think this is where resource 5 is not getting set up properly.
That is as far as I got today.

Thanks,
NeilBrown


signature.asc
Description: PGP signature
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v4 00/15] staging: mt7621-pci: avoid custom pci config read and writes

2018-07-25 Thread Sergio Paracuellos
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.68] ahci: probe of :01:00.0 failed with error -22
> [8.70] ahci: probe of :02:00.0 failed with error -22
> [8.71] ahci: probe of :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 = <_pins>;

Does this change make the plugged drives to work?

Thanks in advance,

Best regards,
Sergio Paracuellos

> 
> Thanks,
> NeilBrown
> 
> 
> [0.00] Linux version 4.18.0-rc5+ (neilb@noble) (gcc version 7.2.0 
> (GCC)) #205 SMP Wed Jul 25 08:12:13 AEST 2018
> [0.00] SoC Type: MediaTek MT7621 ver:1 eco:3
> [0.00] bootconsole [early0] enabled
> [0.00] CPU0 revision is: 0001992f (MIPS 1004Kc)
> [0.00] MIPS: machine is GB-PC1
> [0.00] Determined physical RAM map:
> [0.00]  memory: 1c00 @  (usable)
> [0.00]  memory: 0400 @ 2000 (usable)
> [0.00] Initrd not found or empty - disabling initrd
> [0.00] VPE topology {2,2} total 4
> [0.00] Primary instruction cache 32kB, VIPT, 4-way, linesize 32 bytes.
> [0.00] Primary data cache 32kB, 4-way, PIPT, no aliases, linesize 32 
> bytes
> [0.00] MIPS secondary cache 256kB, 8-way, linesize 32 bytes.
> [0.00] Zone ranges:
> [0.00]   Normal   [mem 0x-0x1fff]
> [0.00]   HighMem  [mem 0x2000-0x23ff]
> [0.00] Movable zone start for each node
> [0.00] Early memory node ranges
> [0.00]   node   0: [mem 0x-0x1bff]
> [0.00]   node   0: [mem 0x2000-0x23ff]
> [0.00] Initmem setup node 0 [mem 
> 0x-0x23ff]
> [0.00] On node 0 totalpages: 131072
> [0.00]   Normal zone: 1024 pages used for memmap
> [0.00]   Normal zone: 0 pages reserved
> [0.00]   Normal zone: 114688 pages, LIFO batch:31
> [0.00]   HighMem zone: 16384 pages, LIFO batch:3
> [0.00] random: get_random_bytes called from start_kernel+0xb4/0x4ec 
> with crng_init=0
> [0.00] percpu: Embedded 15 pages/cpu @(ptrval) s30480 r8192 d22768 
> u61440
> [0.00] pcpu-alloc: s30480 r8192 d22768 u61440 alloc=15*4096
> [0.00] pcpu-alloc: [0] 

Re: [PATCH v4 00/15] staging: mt7621-pci: avoid custom pci config read and writes

2018-07-25 Thread Sergio Paracuellos
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.

Hi Neil,

Thanks for the feedback and your time testing this.

> 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..

Yes, that's the place I put that select also to be able to compile
properly and not get include the PCI_LEGACY configuration. I don't really 
know where this select should be included while this driver is in staging...
Maybe we should add a Kconfig for this PCI controller in driver directory
and just select it, like:

config PCI_MT7621
bool "MT7621 PCI controler driver for MT7621 Mediatek SOCs"
select PCI_DRIVERS_GENERIC

Maybe there is a better approach... What do you think?

> 
> The system boots and can see the three pcie-attached SATA controllers:

We are in the correct way :-).

> 
> # 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.68] ahci: probe of :01:00.0 failed with error -22
> [8.70] ahci: probe of :02:00.0 failed with error -22
> [8.71] ahci: probe of :03:00.0 failed with error -22

Mmmm... So these three seems to be returning EINVAL trying to attach
the driver in 'really_probe' function (drivers/base/dd.c). It is going
through the error path 'pinctrl_bind_failed' because of return error
from pinctrl_bind_pins() function call. I don't really know why this is
happening after the PCI changes... Some debug traces will be helpful.
In the meantime I will check this code more deeply and try to figure out
why this is happening.

> 
> I haven't dug deeper yet.
> 
> Thanks,
> NeilBrown

Best regards,
Sergio Paracuellos
> 
> 
> [0.00] Linux version 4.18.0-rc5+ (neilb@noble) (gcc version 7.2.0 
> (GCC)) #205 SMP Wed Jul 25 08:12:13 AEST 2018
> [0.00] SoC Type: MediaTek MT7621 ver:1 eco:3
> [0.00] bootconsole [early0] enabled
> [0.00] CPU0 revision is: 0001992f (MIPS 1004Kc)
> [0.00] MIPS: machine is GB-PC1
> [0.00] Determined physical RAM map:
> [0.00]  memory: 1c00 @  (usable)
> [0.00]  memory: 0400 @ 2000 (usable)
> [0.00] Initrd not found or empty - disabling initrd
> [0.00] VPE topology {2,2} total 4
> [0.00] Primary instruction cache 32kB, VIPT, 4-way, linesize 32 bytes.
> [0.00] Primary data cache 32kB, 4-way, PIPT, no aliases, linesize 32 
> bytes
> [0.00] MIPS secondary cache 256kB, 8-way, linesize 32 bytes.
> [0.00] Zone ranges:
> [0.00]   Normal   [mem 0x-0x1fff]
> [0.00]   HighMem  [mem 0x2000-0x23ff]
> [0.00] Movable zone start for each node
> [0.00] Early memory node ranges
> [0.00]   node   0: [mem 0x-0x1bff]
> [0.00]   node  

Re: [PATCH v4 00/15] staging: mt7621-pci: avoid custom pci config read and writes

2018-07-24 Thread NeilBrown
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.68] ahci: probe of :01:00.0 failed with error -22
[8.70] ahci: probe of :02:00.0 failed with error -22
[8.71] ahci: probe of :03:00.0 failed with error -22

I haven't dug deeper yet.

Thanks,
NeilBrown


[0.00] Linux version 4.18.0-rc5+ (neilb@noble) (gcc version 7.2.0 
(GCC)) #205 SMP Wed Jul 25 08:12:13 AEST 2018
[0.00] SoC Type: MediaTek MT7621 ver:1 eco:3
[0.00] bootconsole [early0] enabled
[0.00] CPU0 revision is: 0001992f (MIPS 1004Kc)
[0.00] MIPS: machine is GB-PC1
[0.00] Determined physical RAM map:
[0.00]  memory: 1c00 @  (usable)
[0.00]  memory: 0400 @ 2000 (usable)
[0.00] Initrd not found or empty - disabling initrd
[0.00] VPE topology {2,2} total 4
[0.00] Primary instruction cache 32kB, VIPT, 4-way, linesize 32 bytes.
[0.00] Primary data cache 32kB, 4-way, PIPT, no aliases, linesize 32 
bytes
[0.00] MIPS secondary cache 256kB, 8-way, linesize 32 bytes.
[0.00] Zone ranges:
[0.00]   Normal   [mem 0x-0x1fff]
[0.00]   HighMem  [mem 0x2000-0x23ff]
[0.00] Movable zone start for each node
[0.00] Early memory node ranges
[0.00]   node   0: [mem 0x-0x1bff]
[0.00]   node   0: [mem 0x2000-0x23ff]
[0.00] Initmem setup node 0 [mem 0x-0x23ff]
[0.00] On node 0 totalpages: 131072
[0.00]   Normal zone: 1024 pages used for memmap
[0.00]   Normal zone: 0 pages reserved
[0.00]   Normal zone: 114688 pages, LIFO batch:31
[0.00]   HighMem zone: 16384 pages, LIFO batch:3
[0.00] random: get_random_bytes called from start_kernel+0xb4/0x4ec 
with crng_init=0
[0.00] percpu: Embedded 15 pages/cpu @(ptrval) s30480 r8192 d22768 
u61440
[0.00] pcpu-alloc: s30480 r8192 d22768 u61440 alloc=15*4096
[0.00] pcpu-alloc: [0] 0 [0] 1 [0] 2 [0] 3 
[0.00] Built 1 zonelists, mobility grouping on.  Total pages: 130048
[0.00] Kernel command line: console=ttyS0,57600
[0.00] Dentry cache hash table entries: 65536 (order: 6, 262144 bytes)
[0.00] Inode-cache hash table entries: 32768 (order: 5, 131072 bytes)
[0.00] Writing ErrCtl register=000108a2
[0.00] Readback ErrCtl register=000108a2
[0.00] Memory: 504788K/524288K available (6131K kernel code, 232K 
rwdata, 1052K rodata, 6524K init, 241K bss, 19500K reserved, 0K cma-reserved, 
65536K highmem)
[0.00] SLUB: HWalign=32, Order=0-3, MinObjects=0, CPUs=4, Nodes=1
[0.00] 

[PATCH v4 00/15] staging: mt7621-pci: avoid custom pci config read and writes

2018-07-16 Thread Sergio Paracuellos
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,


Sergio Paracuellos (15):
  staging: mt7621-pci: use generic kernel pci subsystem read and write
  staging: mt7621-pci: remove dead code derived to not use custom reads
and writes
  staging: mt7621-pci: add pcie_write and pcie_read helpers
  staging: mt7621-pci: use pcie_[read|write] in [write|read]_config
  staging: mt7621-pci: simplify read_config function
  staging: mt7621-pci: simplify write_config function
  staging: mt7621-pci: remove unused macros
  staging: mt7621-pci: avoid register duplication per controller using
pcie_[read|write]
  staging: mt7621-pci: remove unused includes
  staging: mt7621-pci: use pcie_[read|write] in RALINK_PCI_PCICFG_ADDR
and RALINK_PCI_PCIMSK_ADDR
  staging: mt7621-pci: remove RALINK_PCI_BASE from remaining definitions
  staging: mt7621-pci: use BIT macro in preprocessor definitions
  staging: mt7621-pci: rename RALINK_PCI_CONFIG_DATA_VIRTUAL_REG
definition
  staging: mt7621-pci: remove duplicated include
  staging: mt7621-pci: remove remaining pci_legacy dependant code

 drivers/staging/mt7621-pci/pci-mt7621.c | 675 ++--
 1 file changed, 294 insertions(+), 381 deletions(-)

-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel