Re: [RFC 12/22] dm: add a hidden link to efi object

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

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.akashi@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.akashi@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*?

-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.
> 
> >
> > diff --git a/include/dm/device.h b/include/dm/device.h
> > index 0a9718a5b81a..33b09a836f06 100644
> > --- a/include/dm/device.h
> > +++ b/include/dm/device.h
> > @@ -190,6 +190,10 @@ struct udevice {
> >  #if CONFIG_IS_ENABLED(DM_DMA)
> >         ulong dma_offset;
> >  #endif
> > +#if CONFIG_IS_ENABLED(EFI_LOADER)
> > +       /* link to efi_object */
> > +       void *efi_obj;
> > +#endif
> >  };
> >
> >  /**
> > --
> > 2.33.0
> >
> 
> Regards,
> Simon