On Wed, Jun 27, 2018 at 2:18 AM Simon Glass <[email protected]> wrote:
> HI Ramon, > > On 26 June 2018 at 04:15, Ramon Fried <[email protected]> wrote: > > > > > > On Tue, Jun 26, 2018 at 6:59 AM Simon Glass <[email protected]> wrote: > >> > >> Hi Ramon, > >> > >> On 21 June 2018 at 20:11, Ramon Fried <[email protected]> wrote: > >> > This is a uclass for Shared memory manager drivers. > >> > > >> > A Shared Memory Manager driver implements an interface for allocating > >> > and accessing items in the memory area shared among all of the > >> > processors. > >> > > >> > Signed-off-by: Ramon Fried <[email protected]> > >> > > >> > --- > >> > > >> > Changes in v2: > >> > (As suggested by Simon Glass) > >> > - Introduced a new dm class (CLASS_SMEM) instead of CLASS_SOC. > >> > - Added sandbox driver > >> > - Added testing for DM class. > >> > > >> > drivers/Makefile | 1 + > >> > drivers/smem/Kconfig | 2 + > >> > drivers/smem/Makefile | 5 +++ > >> > drivers/smem/smem-uclass.c | 53 ++++++++++++++++++++++++ > >> > include/dm/uclass-id.h | 1 + > >> > include/smem.h | 84 > ++++++++++++++++++++++++++++++++++++++ > >> > 6 files changed, 146 insertions(+) > >> > create mode 100644 drivers/smem/Kconfig > >> > create mode 100644 drivers/smem/Makefile > >> > create mode 100644 drivers/smem/smem-uclass.c > >> > create mode 100644 include/smem.h > >> > > >> > >> Reviewed-by: Simon Glass <[email protected]> > >> > >> A few nits below. > >> > [..] > > >> > +/* struct smem_ops: Operations for the SMEM uclass */ > >> > +struct smem_ops { > >> > + /** > >> > + * alloc() - allocate space for a smem item > >> > + * > >> > + * @host: remote processor id, or -1 > >> > >> What does -1 mean? Please expand commment > >> > >> > + * @item: smem item handle > >> > >> How does one choose this? What does it mean? Can you expand this > comment a > >> bit? > > > > It can be an index (like in QCOM implementation) but honestly, it can be > a > > hash as well. it's up to the implementation > > to decide how to access the items. > > OK that's fine. The key thing is to document this. Your API at present > is pretty vague which is why I am asking for more comments showing > what is going on. Even a comment at the top of your header file with > an example of usage would be useful. > > I just saw your latest v3 patch and it still seems to me that it needs > more details. I had to read the qualcomm driver to try to understand > how the uclass API is supposed to work. > > Thanks for the feedback. I'll look in to it soon. Thanks, Ramon. > Regards, > Simon > _______________________________________________ U-Boot mailing list [email protected] https://lists.denx.de/listinfo/u-boot
