Hi Heinrich,
On Mon, 11 Oct 2021 at 09:31, Heinrich Schuchardt <xypron.g...@gmx.de> wrote:
>
>
>
> On 10/11/21 16:54, Simon Glass wrote:
> > Hi Takahiro,
> >
> > On Mon, 11 Oct 2021 at 00:43, AKASHI Takahiro
> > <takahiro.aka...@linaro.org> wrote:
> >>
> >> Simon,
> >>
> >> On Sun, Oct 10, 2021 at 08:14:18AM -0600, Simon Glass wrote:
> >>> Hi Takahiro,
> >>>
> >>> On Thu, 30 Sept 2021 at 23:04, AKASHI Takahiro
> >>> <takahiro.aka...@linaro.org> wrote:
> >>>>
> >>>> This member field in udevice will be used to dereference from udevice
> >>>> to efi_object (or efi_handle).
> >>>>
> >>>> Signed-off-by: AKASHI Takahiro <takahiro.aka...@linaro.org>
> >>>> ---
> >>>> include/dm/device.h | 4 ++++
> >>>> 1 file changed, 4 insertions(+)
> >>>
> >>> I think this should be generalised.
> >>>
> >>> Can we add a simple API for attaching things to devices? Something like:
> >>
> >> Ok.
> >>
> >>
> >>> config DM_TAG
> >>> bool "Support tags attached to devices"
> >>>
> >>> enum dm_tag_t {
> >>> DM_TAG_EFI = 0,
> >>>
> >>> DM_TAG_COUNT,
> >>> };
> >>>
> >>> ret = dev_tag_set_ptr(dev, DM_TAG_EFI, ptr);
> >>>
> >>> void *ptr = dev_tag_get_ptr(dev, DM_TAG_EFI);
> >>>
> >>> ulong val = dev_tag_get_val(dev, DM_TAG_EFI);
> >>>
> >>> Under the hood I think for now we could have a simple list of tags for
> >>> all of DM:
> >>>
> >>> struct dmtag_node {
> >>> struct list_head sibling;
> >>> struct udevice *dev;
> >>> enum dm_tag_t tag;
> >>> union {
> >>> void *ptr;
> >>> ulong val;
> >>> };
> >>> };
> >>
> >> Just let me make sure; Do you intend that we have a *single* list of tags
> >> in the system instead of maintaining a list *per udevice*?
> >
> > Yes I would prefer not to have a list per udevice, although the API
> > could be adjusted to iterate through all tags for a particular
> > udevice, if that is needed (dev_tag_first...() dev_tag_next...().
>
> There will never be more than one UEFI handle for one udevice.
> We need a single field that points to the the handle if such a handle
> exists. But there will be devices for which UEFI protocols don't exist
> and where we need no handle. In this case the value can be NULL.
>
> Why should we complicate the picture with a list of tags?
Let's not talk about complexity while we are discussing UEFI :-)
There are other cases where we need to add info to a device. We cover
almost all the cases with the uclass-private, plat and priv data
attached to each device. But in some cases that is not enough, as with
EFI. I have hit this before in a few other places but have tried to
work around it rather than extending driver model and adding to the
already-large struct udevice. But I think we are at the end of the
road on that.
I'd also like to look at how much (for example) uclass-plat data is
used for devices, in case it would be more efficient to move it to a
tag model.
I should also point out you are talking about the implementation
rather than the API. We can always change the impl later, so long as
we have a suitable API.
> >
> > Looking at some of your other patches I think you might need to
> > support multiple tags for EFI, if there are different things. But
> > perhaps a list is necesary.
> >
> >>
> >> -Takahiro Akashi
> >>
> >>
> >>> This can be useful in other situations, for example I think we need to
> >>> be able to send an event when a device is probed so that other devices
> >>> (with tags attached) can take action. But in any case, it makes the
> >>> API separate from the data structure, so aids refactoring later.
> >>>
> >>> If we find that this is slow we can change the impl, but I doubt it
> >>> will matter fornow.
> >>>
Regards,
Simon
