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

[NeilBrown <neil@xxxxxxxxxx>]

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.paracuellos@xxxxxxxxx>
> ---
>  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

Attachment: signature.asc
Description: PGP signature

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