
On Wed, Jun 27, 2018 at 2:18 AM Simon Glass sjg@chromium.org wrote:
HI Ramon,
On 26 June 2018 at 04:15, Ramon Fried ramon.fried@gmail.com wrote:
On Tue, Jun 26, 2018 at 6:59 AM Simon Glass sjg@chromium.org wrote:
Hi Ramon,
On 21 June 2018 at 20:11, Ramon Fried ramon.fried@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@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@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.
Thanks for the feedback. I'll look in to it soon.
Thanks, Ramon.
Regards, Simon