On Fri, Mar 12, 2021 at 09:35:57PM +0800, Bin Meng wrote:
> At present the tsec driver uses a non-standard DT bindings to get
> its <reg> base / size. The upstream Linux kernel seems to require
> the <reg> base / size to be put under a subnode of the eTSEC node
> with a name prefix "queue-group". This is not documented in the
> kernel DT bindings, but it looks every dtsi file that contains the
> eTSEC node was written like this.
>
> This commit updates the tsec driver to handle this case.
>
> Signed-off-by: Bin Meng <bmeng...@gmail.com>
> Reviewed-by: Ramon Fried <rfried....@gmail.com>
> ---
>
> (no changes since v1)
>
> drivers/net/tsec.c | 26 ++++++++++++++++++++++++--
> 1 file changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/tsec.c b/drivers/net/tsec.c
> index f801d020fb..49bed0c1dd 100644
> --- a/drivers/net/tsec.c
> +++ b/drivers/net/tsec.c
> @@ -827,13 +827,35 @@ int tsec_probe(struct udevice *dev)
> struct tsec_data *data;
> const char *phy_mode;
> fdt_addr_t reg;
> - ofnode parent;
> + ofnode parent, child;
Same comment about the "reverse Christmas tree" notation.
> int ret;
>
> data = (struct tsec_data *)dev_get_driver_data(dev);
>
> pdata->iobase = (phys_addr_t)dev_read_addr(dev);
> - priv->regs = dev_remap_addr(dev);
> + if (pdata->iobase != FDT_ADDR_T_NONE) {
> + priv->regs = dev_remap_addr(dev);
> + } else {
> + ofnode_for_each_subnode(child, dev_ofnode(dev)) {
> + if (!strncmp(ofnode_get_name(child), "queue-group",
> + strlen("queue-group"))) {
I would prefer the syntax:
if (strncmp(ofnode_get_name(child), "queue-group",
strlen("queue-group")))
continue;
which allows us to reduce the indentation level.
> + reg = ofnode_get_addr(child);
> + if (reg == FDT_ADDR_T_NONE) {
> + printf("No 'reg' property of
> <queue-group>\n");
> + return -ENOENT;
> + }
> + pdata->iobase = reg;
> + priv->regs = map_physmem(pdata->iobase, 0,
> + MAP_NOCACHE);
Hmm, can't you just populate pdata->iobase, and call the same
dev_remap_addr in the common code path?
> + break;
Could you please add a comment that if there are multiple queue groups,
we only use the first one?
> + }
> + }
> +
> + if (!ofnode_valid(child)) {
> + printf("No child node for <queue-group>?\n");
> + return -ENOENT;
> + }
> + }
