Re: [PATCH v3 08/19] dm: add tag support

[AKASHI Takahiro <takahiro.akashi@linaro.org>]

On Wed, Mar 09, 2022 at 01:41:04PM +0200, Ilias Apalodimas wrote:
> Hi Akashi-san,
> 
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > Reviewed-by: Simon Glass <sjg@chromium.org>
> > ---
> >  drivers/core/Makefile             |   2 +-
> >  drivers/core/root.c               |   2 +
> >  drivers/core/tag.c                | 139 ++++++++++++++++++++++++++++++
> >  include/asm-generic/global_data.h |   4 +
> >  include/dm/tag.h                  | 110 +++++++++++++++++++++++
> >  5 files changed, 256 insertions(+), 1 deletion(-)
> >  create mode 100644 drivers/core/tag.c
> >  create mode 100644 include/dm/tag.h
> > 
> > diff --git a/drivers/core/Makefile b/drivers/core/Makefile
> > index 5edd4e413576..3742e7574525 100644
> > --- a/drivers/core/Makefile
> > +++ b/drivers/core/Makefile
> > @@ -2,7 +2,7 @@
> >  #
> >  # Copyright (c) 2013 Google, Inc
> >  
> > -obj-y	+= device.o fdtaddr.o lists.o root.o uclass.o util.o
> > +obj-y	+= device.o fdtaddr.o lists.o root.o uclass.o util.o tag.o
> >  obj-$(CONFIG_$(SPL_TPL_)ACPIGEN) += acpi.o
> >  obj-$(CONFIG_DEVRES) += devres.o
> >  obj-$(CONFIG_$(SPL_)DM_DEVICE_REMOVE)	+= device-remove.o
> > diff --git a/drivers/core/root.c b/drivers/core/root.c
> > index 8efb4256b27e..86b3884fc674 100644
> > --- a/drivers/core/root.c
> > +++ b/drivers/core/root.c
> > @@ -199,6 +199,8 @@ int dm_init(bool of_live)
> >  			return ret;
> >  	}
> >  
> > +	INIT_LIST_HEAD((struct list_head *)&gd->dmtag_list);
> > +
> >  	return 0;
> >  }
> >  
> > diff --git a/drivers/core/tag.c b/drivers/core/tag.c
> > new file mode 100644
> > index 000000000000..6829bcd8806c
> > --- /dev/null
> > +++ b/drivers/core/tag.c
> > @@ -0,0 +1,139 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (c) 2021 Linaro Limited
> > + *			Author: AKASHI Takahiro
> > + */
> > +
> > +#include <malloc.h>
> > +#include <asm/global_data.h>
> > +#include <dm/tag.h>
> > +#include <linux/err.h>
> > +#include <linux/list.h>
> > +#include <linux/types.h>
> > +
> > +struct udevice;
> > +
> > +DECLARE_GLOBAL_DATA_PTR;
> > +
> > +int dev_tag_set_ptr(struct udevice *dev, enum dm_tag_t tag, void *ptr)
> > +{
> > +	struct dmtag_node *node;
> > +
> > +	if (!dev || tag >= DM_TAG_COUNT)
> > +		return -EINVAL;
> > +
> > +	list_for_each_entry(node, &gd->dmtag_list, sibling) {
> > +		if (node->dev == dev && node->tag == tag)
> > +			return -EEXIST;
> > +	}
> > +
> > +	node = calloc(sizeof(*node), 1);
> > +	if (!node)
> > +		return -ENOSPC;
> 
> Isn't -ENOMEM better here?

Ah, yes.

> > +
> > +	node->dev = dev;
> > +	node->tag = tag;
> > +	node->ptr = ptr;
> > +	list_add_tail(&node->sibling, (struct list_head *)&gd->dmtag_list);
> > +
> > +	return 0;
> > +}
> > +
> > +int dev_tag_set_val(struct udevice *dev, enum dm_tag_t tag, ulong val)
> 
> Is this used anywhere else apart from selftests?

Currently, no. It exists just for future simpler use cases.

-Takahiro Akashi

> > +{
> > +	struct dmtag_node *node;
> 
> 
> Thanks
> /Ilias