[U-Boot] [PATCH v2 1/6] dm: SMEM (Shared memory) uclass

[Simon Glass]

HI Ramon,

On 26 June 2018 at 04:15, Ramon Fried <ramon.fried at gmail.com> wrote:
>
>
> On Tue, Jun 26, 2018 at 6:59 AM Simon Glass <sjg at chromium.org> wrote:
>>
>> Hi Ramon,
>>
>> On 21 June 2018 at 20:11, Ramon Fried <ramon.fried at gmail.com> 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 <ramon.fried at gmail.com>
>> >
>> > ---
>> >
>> > 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 <sjg at chromium.org>
>>
>> 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.

Regards,
Simon