Re: [PATCH v6 01/33] staging: mt7621-pci: parse and init port data from device tree

[NeilBrown]

On Sun, Nov 04 2018, Sergio Paracuellos wrote:

> Add initialization of each PCIe port reading and initializing
> data using device tree.
>
> Signed-off-by: Sergio Paracuellos <sergio.paracuel...@gmail.com>
> ---
>  drivers/staging/mt7621-pci/pci-mt7621.c | 75 
> +++++++++++++++++++++++++++++++--
>  1 file changed, 71 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/mt7621-pci/pci-mt7621.c 
> b/drivers/staging/mt7621-pci/pci-mt7621.c
> index 8371a9c..b7cb273 100644
> --- a/drivers/staging/mt7621-pci/pci-mt7621.c
> +++ b/drivers/staging/mt7621-pci/pci-mt7621.c
> @@ -126,16 +126,20 @@ static int pcie_link_status;
>  
>  /**
>   * struct mt7621_pcie_port - PCIe port information
> - * @base: IO mapped register base
> + * @base: I/O mapped register base
>   * @list: port list
>   * @pcie: pointer to PCIe host info
> - * @reset: pointer to port reset control
> + * @pcie_rst: pointer to port reset control
> + * @pcie_clk: PCIe clock
> + * @slot: port slot
>   */
>  struct mt7621_pcie_port {
>       void __iomem *base;
>       struct list_head list;
>       struct mt7621_pcie *pcie;
> -     struct reset_control *reset;
> +     struct reset_control *pcie_rst;
> +     struct clk *pcie_clk;
> +     u32 slot;
>  };
>  
>  /**
> @@ -382,10 +386,57 @@ static int 
> mt7621_pci_parse_request_of_pci_ranges(struct mt7621_pcie *pcie)
>       return 0;
>  }
>  
> +static int mt7621_pcie_parse_port(struct mt7621_pcie *pcie,
> +                               struct device_node *node,
> +                               int slot)
> +{
> +     struct mt7621_pcie_port *port;
> +     struct device *dev = pcie->dev;
> +     struct device_node *pnode = dev->of_node;
> +     struct resource regs;
> +     char name[6];
> +     int err;
> +
> +     port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
> +     if (!port)
> +             return -ENOMEM;
> +
> +     err = of_address_to_resource(pnode, slot + 1, &regs);
> +     if (err) {
> +             dev_err(dev, "missing \"reg\" property\n");
> +             return err;
> +     }
> +
> +     port->base = devm_ioremap_resource(dev, &regs);
> +     if (IS_ERR(port->base))
> +             return PTR_ERR(port->base);
> +
> +     snprintf(name, sizeof(name), "pcie%d", slot);
> +     port->pcie_clk = devm_clk_get(dev, name);
> +     if (IS_ERR(port->pcie_clk)) {
> +             dev_err(dev, "failed to get pcie%d clock\n", slot);
> +             return PTR_ERR(port->pcie_clk);
> +     }

This is problematic.
The clocks are defined in the dtsi as:

                clocks = <&clkctrl 24 &clkctrl 25 &clkctrl 26>;
                clock-names = "pcie0", "pcie1", "pcie2";
and clkctrl is
        clkctrl: clkctrl {
                compatible = "ralink,rt2880-clock";
                #clock-cells = <1>;
        };

but there is no driver that declares compatibility with
ralink,rt2880-clock.
So devm_clk_get() cannot find any clocks, and returns an error.
I worked around this by just ignoring the error, but then something
else went wrong in a subsequent patch - haven't worked out what yet.

Probably the *right* way to fix this is to add a driver for
ralink,rt2880-clock but I don't know what such a driver would do.

In any case, I don't plan to pursue this particular issue further until
I understand the other things that are happening with this patch set.

Thanks,
NeilBrown



> +
> +     port->pcie_rst = devm_reset_control_get_exclusive(dev, name);
> +     if (PTR_ERR(port->pcie_rst) == -EPROBE_DEFER) {
> +             dev_err(dev, "failed to get pcie%d reset control\n", slot);
> +             return PTR_ERR(port->pcie_rst);
> +     }
> +
> +     port->slot = slot;
> +     port->pcie = pcie;
> +
> +     INIT_LIST_HEAD(&port->list);
> +     list_add_tail(&port->list, &pcie->ports);
> +
> +     return 0;
> +}
> +
>  static int mt7621_pcie_parse_dt(struct mt7621_pcie *pcie)
>  {
>       struct device *dev = pcie->dev;
> -     struct device_node *node = dev->of_node;
> +     struct device_node *node = dev->of_node, *child;
>       struct resource regs;
>       int err;
>  
> @@ -399,6 +450,22 @@ static int mt7621_pcie_parse_dt(struct mt7621_pcie *pcie)
>       if (IS_ERR(pcie->base))
>               return PTR_ERR(pcie->base);
>  
> +     for_each_available_child_of_node(node, child) {
> +             int slot;
> +
> +             err = of_pci_get_devfn(child);
> +             if (err < 0) {
> +                     dev_err(dev, "failed to parse devfn: %d\n", err);
> +                     return err;
> +             }
> +
> +             slot = PCI_SLOT(err);
> +
> +             err = mt7621_pcie_parse_port(pcie, child, slot);
> +             if (err)
> +                     return err;
> +     }
> +
>       return 0;
>  }
>  
> -- 
> 2.7.4

signature.asc
Description: PGP signature

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