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.
>
>
> >
> > 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.
Neil, I think you notice it actually, but just in case I think it is better
to confirm it. This series should be applicable patch by patch in order
to better debugging the errors. So just applying the first patch
and configuring properly with CONFIG_PCI_MT7621 option should run also...
I suspect this also get the system to hang, right?
Thanks in advance.
Also, hopefully this afternoon/tonigh I give this a new chance and this time
I am going to get resources by myself using pci_add_resource_offset for each
entry in the ranges property of the pcie node (like the mips pci-legacy code does)
instead of using the devm_of_pci_get_host_bridge_resources. It should be the same
but it seems it is something different according the dmesg traces... So just to try
and discard this... v7 is comming soon.
>
> 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
