HI Ramon,
On 26 June 2018 at 04:15, Ramon Fried <ramon.fr...@gmail.com> wrote:
>
>
> On Tue, Jun 26, 2018 at 6:59 AM Simon Glass <s...@chromium.org> wrote:
>>
>> Hi Ramon,
>>
>> On 21 June 2018 at 20:11, Ramon Fried <ramon.fr...@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.fr...@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 <s...@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
