On Tue, Jul 31, 2018 at 03:25:43PM +1000, NeilBrown wrote:
> On Tue, Jul 31 2018, Sergio Paracuellos wrote:
>
> > On Tue, Jul 31, 2018 at 08:55:52AM +1000, NeilBrown wrote:
> >> On Mon, Jul 30 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.
> >> >
> >> > Also the driver get removes all legacy PCI code using now PCI_DRIVERS_GENERIC.
> >> >
> >> > Changes in v6:
> >> > - Reorder patches to be each patch correct in itself.
> >> > - PATCH 1 adds also Kconfig to do the step from legacy to generic code
> >> > - PATCH 1 remaps io space using devm_pci_remap_iospace for io resource in
> >> > a new function called 'mt7621_pci_parse_request_of_pci_ranges'.
> >> > - Other patches rebased and adapted with this changes.
> >>
> >> No noticeable difference.
> >> Still hangs after
> >> [ 8.630000] ahci 0000:01:00.0: enabling device (0000 -> 0002)
> >>
> >> the readl() at the start of ahci_enable_ahci() hangs, reading c4017004.
> >>
> >> I built on a merge of
> >> Merge: 527838d470e3 b9f13084580c
> >>
> >> linus' master + staging/staging-testing
> >>
> >> dmesg below.
> >
> > Thanks for this.
> >
> ....
> >> [ 8.430000] bootconsole [early0] disabled
> >> [ 8.430000] bootconsole [early0] disabled
> >> [ 8.450000] cacheinfo: Failed to find cpu0 device node
> >> [ 8.460000] cacheinfo: Unable to detect cache hierarchy for CPU 0
> >> [ 8.550000] loop: module loaded
> >> [ 8.560000] ahci 0000:01:00.0: enabling device (0000 -> 0002)
> >>
> >>
> >
> > So this last dmesg is the new one with last v6, right? Because I can see
>
> Right.
>
>
> > that at least now the assigned resources are pretty much the same of the ones in the dmesg of the
> > original code talking in terms of assigned BAR's and bridge windows. No weird BAR 9 anymore, which I think is
> > good. We can try to get back the hack which I removed at first and see what happend. The hack is
> > this function removed in PATCH 2:
> >
> > -void setup_cm_memory_region(struct resource *mem_resource)
> > -{
> > - resource_size_t mask;
> > - if (mips_cps_numiocu(0)) {
> > - /* FIXME: hardware doesn't accept mask values with 1s after
> > - * 0s (e.g. 0xffef), so it would be great to warn if that's
> > - * about to happen */
> > - mask = ~(mem_resource->end - mem_resource->start);
> > -
> > - write_gcr_reg1_base(mem_resource->start);
> > - write_gcr_reg1_mask(mask | CM_GCR_REGn_MASK_CMTGT_IOCU0);
> > - printk("PCI coherence region base: 0x%08llx, mask/settings: 0x%08llx\n",
> > - (unsigned long long)read_gcr_reg1_base(),
> > - (unsigned long long)read_gcr_reg1_mask());
> > - }
> > -}
> >
> > We can try to add it again and and call it from 'mt7621_pci_parse_request_of_pci_ranges'
> > in the label of the case of the case 'IORESOURCE_MEM' (as you can see this was intentionally
> > without code just to test this if this v6 fails):
> >
> > break;
> > case IORESOURCE_MEM:
> > + setup_cm_memory_region(res);
> > break;
>
> I added setup_cm_memory_region() back in and called it from
> mt7621_pci_parse_request_of_pci_ranges()
> as suggested.
> Now we don't hang at the same place, but crash shortly after.
>
> [ 8.750000] WARNING: CPU: 2 PID: 1 at ../drivers/ata/libahci.c:227 ahci_save_initial_config+0x3c/0x3e0
>
> This is at the end of ahci_enable_ahci(). the HOST_AHCI_EN bit never was
> set.
So it seems this hack is really needed. Does something changed in dmesg with this?
>
>
> >
> > If this also fails we can try to move the call to 'mt7621_pcie_parse_dt(pcie, &res);' after the
> > HW initialization code just before the list_splice_init(&res, &bridge->windows); line:
> >
> > - err = mt7621_pcie_parse_dt(pcie, &res);
> > - if (err) {
> > - dev_err(dev, "Parsing DT failed\n");
> > - return err;
> > - }
> > -
> > - /*
> > iomem_resource.start = 0;
> > iomem_resource.end = ~0;
> > ...
> >
> > write_config(0, 0, 0, 0x70c, val);
> > }
> >
> > + err = mt7621_pcie_parse_dt(pcie, &res);
> > + if (err) {
> > + dev_err(dev, "Parsing DT failed\n");
> > + return err;
> > + }
> > +
> > list_splice_init(&res, &bridge->windows);
>
> If I move the call to mt7621_pcie_parse_dt() anywhere after
> the call to set_phy_for_ssc() I get a crash at the start of
> set_pcie_phy() called from set_phy_for_ssc(), presumably because
> pcie->base isn't set.
>
> Keep trying, I'm sure we'll get there.
True, sorry. Maybe we can get out the mt7621_pci_parse_request_of_pci_ranges
call from mt7621_pcie_parse_dt and put it after the initialization code
just before the list_splice_init to try to reproduce the original code trace.
I never give up :). I only concern if I am not doing you to waste your time
with this tests. I really like to be able to test this by myself but this
board gets its price really increase buying it from Spain :-(.
>
> NeilBrown
Best regards,
Sergio Paracuellos
>
> >
> > (I don't have access now to the laptop with the code and cannot diff properly, sorry).
> >
> > Hopefully it boots now?
> >
> > Thanks in advance.
> >
> > Best regards,
> > Sergio Paracuellos
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
