On Mon, Jul 30, 2018 at 03:20:47PM +1000, NeilBrown wrote:
> On Fri, Jul 27 2018, Sergio Paracuellos wrote:
>
> > map_bus callback is called before every .read/.write operation.
> > Implement it and change custom read write operations for the
> > pci subsystem generics. Make the probe function to don't use
> > legacy stuff and request bus resources directly. Get pci register
> > base from device tree.
> >
> > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@xxxxxxxxx>
>
> Thanks for your persistence!
> Something still isn't right ... weird hangs, though my serial connector
> seems to be causing problems, which doesn't help.
>
> I tried one patch at a time, and this first patch doesn't work.
>
> It gets to pcibios_align_resource() and crashes because in:
> if (start < PCIBIOS_MIN_MEM + hose->mem_resource->start)
> start = PCIBIOS_MIN_MEM + hose->mem_resource->start;
>
> hose->mem_resource (which is pcie->mem_resource in mt7621_pci_probe())
> is NULL.
>
> The call stack is:
>
> [ 2.490000] Call Trace:
> [ 2.500000] [<804a5948>] pcibios_align_resource+0xa0/0xb4
> [ 2.510000] [<8002ceb4>] __find_resource+0x150/0x230
> [ 2.520000] [<8002d17c>] allocate_resource+0x1e8/0x224
> [ 2.530000] [<80365e94>] pci_bus_alloc_resource+0x168/0x1c8
> [ 2.540000] [<80374450>] _pci_assign_resource+0xa8/0x180
> [ 2.550000] [<803749a4>] pci_assign_resource+0xf4/0x25c
> [ 2.560000] [<80376820>] assign_requested_resources_sorted+0x90/0xe4
> [ 2.570000] [<803771ec>] __assign_resources_sorted+0x110/0x4f8
> [ 2.580000] [<80378618>] __pci_bus_assign_resources+0x74/0x240
> [ 2.590000] [<803790c8>] pci_assign_unassigned_bus_resources+0x70/0xe8
> [ 2.610000] [<804998f8>] mt7621_pci_probe+0x9e8/0xa44
> [ 2.620000] [<803b1708>] platform_drv_probe+0x40/0x7c
> [ 2.630000] [<803afc88>] driver_probe_device+0x31c/0x474
> [ 2.640000] [<803afe94>] __driver_attach+0xb4/0x138
> [ 2.650000] [<803ad730>] bus_for_each_dev+0x6c/0xb0
> [ 2.650000] [<803ae968>] bus_add_driver+0x204/0x24c
> [ 2.660000] [<803b07d8>] driver_register+0xd0/0x118
> [ 2.670000] [<80001618>] do_one_initcall+0x84/0x19c
> [ 2.680000] [<80741ed0>] kernel_init_freeable+0x248/0x250
> [ 2.690000] [<805f549c>] kernel_init+0x14/0x110
>
> So presumably the resources aren't getting initialised properly?
>
> Thanks,
> NeilBrown
>
>
> > ---
> > drivers/staging/mt7621-pci/pci-mt7621.c | 143 ++++++++++++++++++++++++++++++--
> > 1 file changed, 134 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/staging/mt7621-pci/pci-mt7621.c b/drivers/staging/mt7621-pci/pci-mt7621.c
> > index 650e49b..b4f6de2 100644
> > --- a/drivers/staging/mt7621-pci/pci-mt7621.c
> > +++ b/drivers/staging/mt7621-pci/pci-mt7621.c
> > @@ -53,11 +53,16 @@
> > #include <linux/delay.h>
> > #include <linux/of.h>
> > #include <linux/of_pci.h>
> > +#include <linux/of_platform.h>
> > +#include <linux/of_address.h>
> > +#include <linux/of_irq.h>
> > #include <linux/platform_device.h>
> >
> > #include <ralink_regs.h>
> > #include <mt7621.h>
> >
> > +#include "../../pci/pci.h"
> > +
> > /*
> > * These functions and structures provide the BIOS scan and mapping of the PCI
> > * devices.
> > @@ -178,6 +183,32 @@ static int pcie_link_status = 0;
> > #define PCI_ACCESS_WRITE_2 4
> > #define PCI_ACCESS_WRITE_4 5
> >
> > +/**
> > + * struct mt7621_pcie_port - PCIe port information
> > + * @base: IO mapped register base
> > + * @list: port list
> > + * @pcie: pointer to PCIe host info
> > + * @reset: pointer to port reset control
> > + */
> > +struct mt7621_pcie_port {
> > + void __iomem *base;
> > + struct list_head list;
> > + struct mt7621_pcie *pcie;
> > + struct reset_control *reset;
> > +};
> > +
> > +/**
> > + * struct mt7621_pcie - PCIe host information
> > + * @base: IO Mapped Register Base
> > + * @dev: Pointer to PCIe device
> > + * @ports: pointer to PCIe port information
> > + */
> > +struct mt7621_pcie {
> > + void __iomem *base;
> > + struct device *dev;
> > + struct list_head ports;
> > +};
> > +
> > static inline u32 mt7621_pci_get_cfgaddr(unsigned int bus, unsigned int slot,
> > unsigned int func, unsigned int where)
> > {
> > @@ -297,15 +328,27 @@ pci_config_write(struct pci_bus *bus, unsigned int devfn, int where, int size, u
> > }
> > }
> >
> > +static void __iomem *mt7621_pcie_map_bus(struct pci_bus *bus,
> > + unsigned int devfn, int where)
> > +{
> > + struct mt7621_pcie *pcie = bus->sysdata;
> > + u32 address = mt7621_pci_get_cfgaddr(bus->number, PCI_SLOT(devfn),
> > + PCI_FUNC(devfn), where);
> > +
> > + writel(address, pcie->base + RALINK_PCI_CONFIG_ADDR);
> > +
> > + return pcie->base + RALINK_PCI_CONFIG_DATA_VIRTUAL_REG + (where & 3);
> > +}
> > +
> > struct pci_ops mt7621_pci_ops = {
> > - .read = pci_config_read,
> > - .write = pci_config_write,
> > + .map_bus = mt7621_pcie_map_bus,
> > + .read = pci_generic_config_read,
> > + .write = pci_generic_config_write,
> > };
> >
> > static struct resource mt7621_res_pci_mem1;
> > static struct resource mt7621_res_pci_io1;
> > static struct pci_controller mt7621_controller = {
> > - .pci_ops = &mt7621_pci_ops,
> > .mem_resource = &mt7621_res_pci_mem1,
> > .io_resource = &mt7621_res_pci_io1,
> > };
> > @@ -480,14 +523,67 @@ void setup_cm_memory_region(struct resource *mem_resource)
> > }
> > }
> >
> > +static int mt7621_pcie_parse_dt(struct mt7621_pcie *pcie)
> > +{
> > + struct device *dev = pcie->dev;
> > + struct device_node *node = dev->of_node;
> > + struct resource regs;
> > + const char *type;
> > + int err;
> > +
> > + type = of_get_property(node, "device_type", NULL);
> > + if (!type || strcmp(type, "pci") != 0) {
> > + dev_err(dev, "invalid \"device_type\" %s\n", type);
> > + return -EINVAL;
> > + }
> > +
> > + err = of_address_to_resource(node, 0, ®s);
> > + if (err) {
> > + dev_err(dev, "missing \"reg\" property\n");
> > + return err;
> > + }
> > +
> > + pcie->base = devm_pci_remap_cfg_resource(dev, ®s);
> > + if (IS_ERR(pcie->base))
> > + return PTR_ERR(pcie->base);
> > +
> > + return 0;
> > +}
> > +
> > static int mt7621_pci_probe(struct platform_device *pdev)
> > {
> > + struct device *dev = &pdev->dev;
> > + struct mt7621_pcie *pcie;
> > + struct pci_host_bridge *bridge;
> > + struct pci_bus *bus, *child;
> > + int err;
> > + resource_size_t iobase = 0;
> > unsigned long val = 0;
> > + LIST_HEAD(res);
> > +
> > + if (!dev->of_node)
> > + return -ENODEV;
> > +
> > + bridge = devm_pci_alloc_host_bridge(dev, sizeof(*pcie));
> > + if (!bridge)
> > + return -ENODEV;
> > +
> > + pcie = pci_host_bridge_priv(bridge);
> > + pcie->dev = dev;
> > + INIT_LIST_HEAD(&pcie->ports);
> > +
> > + err = mt7621_pcie_parse_dt(pcie);
> > + if (err) {
> > + dev_err(dev, "Parsing DT failed\n");
> > + return err;
> > + }
> >
> > + /*
> > iomem_resource.start = 0;
> > iomem_resource.end = ~0;
> > ioport_resource.start = 0;
> > ioport_resource.end = ~0;
> > + */
> >
> > val = RALINK_PCIE0_RST;
> > val |= RALINK_PCIE1_RST;
> > @@ -612,8 +708,8 @@ pcie(2/1/0) link status pcie2_num pcie1_num pcie0_num
> > ioport_resource.end = mt7621_res_pci_io1.end;
> > */
> >
> > - RALINK_PCI_MEMBASE = 0xffffffff; //RALINK_PCI_MM_MAP_BASE;
> > - RALINK_PCI_IOBASE = RALINK_PCI_IO_MAP_BASE;
> > + //RALINK_PCI_MEMBASE = 0xffffffff; //RALINK_PCI_MM_MAP_BASE;
> > + //RALINK_PCI_IOBASE = RALINK_PCI_IO_MAP_BASE;
> >
> > //PCIe0
> > if ((pcie_link_status & 0x1) != 0) {
> > @@ -665,11 +761,40 @@ pcie(2/1/0) link status pcie2_num pcie1_num pcie0_num
> > write_config(0, 0, 0, 0x70c, val);
> > }
> >
> > - pci_load_of_ranges(&mt7621_controller, pdev->dev.of_node);
> > - setup_cm_memory_region(mt7621_controller.mem_resource);
> > - register_pci_controller(&mt7621_controller);
> > - return 0;
> > + err = devm_of_pci_get_host_bridge_resources(dev, 0, 0xff, &res,
> > + &iobase);
> > + if (err) {
> > + dev_err(dev, "Getting bridge resources failed\n");
> > + return err;
> > + }
> > +
> > + err = devm_request_pci_bus_resources(dev, &res);
> > + if (err)
> > + return err;
> > +
> > + list_splice_init(&res, &bridge->windows);
> > + bridge->busnr = 0;
> > + bridge->dev.parent = dev;
> > + bridge->sysdata = pcie;
> > + bridge->ops = &mt7621_pci_ops;
> > + bridge->map_irq = of_irq_parse_and_map_pci;
> > + bridge->swizzle_irq = pci_common_swizzle;
> >
> > + err = pci_scan_root_bus_bridge(bridge);
> > + if (err < 0)
> > + return err;
> > +
> > + bus = bridge->bus;
> > +
> > + pci_assign_unassigned_bus_resources(bus);
> > + list_for_each_entry(child, &bus->children, node)
> > + pcie_bus_configure_settings(child);
> > +
> > + pci_bus_add_devices(bus);
> > + //pci_load_of_ranges(&mt7621_controller, pdev->dev.of_node);
> > + //setup_cm_memory_region(mt7621_controller.mem_resource);
Another concern is this function which it's a hack with memory
resources... We just deleted this assuming that if all is being
properly assigned it should not be need at all... But it is a very
deep hack... Does this function really being executed and the trace
printk("PCI coherence region base: 0x%08llx, mask/settings: 0x%08llx\n"
displayed in the original code?
> > + //register_pci_controller(&mt7621_controller);
> > + return 0;
> > }
> >
> > int pcibios_plat_dev_init(struct pci_dev *dev)
> > --
> > 2.7.4
Best regards,
Sergio Paracuellos
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
