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. > > > diff --git a/drivers/Makefile b/drivers/Makefile > > index a213ea9671..ba4a561358 100644 > > --- a/drivers/Makefile > > +++ b/drivers/Makefile > > @@ -98,6 +98,7 @@ obj-y += pwm/ > > obj-y += reset/ > > obj-y += input/ > > # SOC specific infrastructure drivers. > > +obj-y += smem/ > > obj-y += soc/ > > obj-$(CONFIG_REMOTEPROC) += remoteproc/ > > obj-y += thermal/ > > diff --git a/drivers/smem/Kconfig b/drivers/smem/Kconfig > > new file mode 100644 > > index 0000000000..64337a8b8e > > --- /dev/null > > +++ b/drivers/smem/Kconfig > > @@ -0,0 +1,2 @@ > > +menuconfig SMEM > > + bool "SMEM (Shared Memory mamanger) support" > > diff --git a/drivers/smem/Makefile b/drivers/smem/Makefile > > new file mode 100644 > > index 0000000000..ca55c4512d > > --- /dev/null > > +++ b/drivers/smem/Makefile > > @@ -0,0 +1,5 @@ > > +# SPDX-License-Identifier: GPL-2.0+ > > +# > > +# Makefile for the U-Boot SMEM interface drivers > > + > > +obj-$(CONFIG_SMEM) += smem-uclass.o > > diff --git a/drivers/smem/smem-uclass.c b/drivers/smem/smem-uclass.c > > new file mode 100644 > > index 0000000000..07ed1a92d8 > > --- /dev/null > > +++ b/drivers/smem/smem-uclass.c > > @@ -0,0 +1,53 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > +/* > > + * Copyright (c) 2018 Ramon Fried <[email protected]> > > + */ > > + > > +#include <common.h> > > +#include <dm.h> > > +#include <smem.h> > > + > > +int smem_alloc(struct udevice *dev, unsigned int host, > > + unsigned int item, size_t size) > > +{ > > + struct smem_ops *ops = smem_get_ops(dev); > > + > > + if (!ops->alloc) > > + return -ENOSYS; > > + > > + return ops->alloc(host, item, size); > > +} > > + > > +void *smem_get(struct udevice *dev, unsigned int host, > > + unsigned int item, size_t *size) > > +{ > > + struct smem_ops *ops = smem_get_ops(dev); > > + > > + if (!ops->get) > > + return NULL; > > + > > + return ops->get(host, item, size); > > +} > > + > > +int smem_get_free_space(struct udevice *dev, unsigned int host, size_t > *free) > > +{ > > + struct smem_ops *ops = smem_get_ops(dev); > > + > > + int ret; > > + > > + if (!ops->get_free_space) > > + return -ENOSYS; > > + > > + ret = ops->get_free_space(host); > > + if (ret > 0) > > + *free = ret; > > + else > > + return ret; > > It seems odd that get_free_space() has a different return value than > smem_get_free_space(). Can you make the latter return the amount of > free space? If not, change the op to return it in a param. You can use > long as the return value if that helps. > > > + > > + return 0; > > +} > > + > > +UCLASS_DRIVER(smem) = { > > + .id = UCLASS_SMEM, > > + .name = "smem", > > +}; > > diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h > > index d7f9df3583..a39643ec5e 100644 > > --- a/include/dm/uclass-id.h > > +++ b/include/dm/uclass-id.h > > @@ -74,6 +74,7 @@ enum uclass_id { > > UCLASS_RTC, /* Real time clock device */ > > UCLASS_SCSI, /* SCSI device */ > > UCLASS_SERIAL, /* Serial UART */ > > + UCLASS_SMEM, /* Shared memory interface */ > > UCLASS_SPI, /* SPI bus */ > > UCLASS_SPMI, /* System Power Management Interface bus > */ > > UCLASS_SPI_FLASH, /* SPI flash */ > > diff --git a/include/smem.h b/include/smem.h > > new file mode 100644 > > index 0000000000..201960232c > > --- /dev/null > > +++ b/include/smem.h > > @@ -0,0 +1,84 @@ > > +/* SPDX-License-Identifier: GPL-2.0+ */ > > +/* > > + * header file for smem driver. > > If you have a README somewhere please point to it here. > > > + * > > + * Copyright (c) 2018 Ramon Fried <[email protected]> > > + */ > > + > > +#ifndef _smemh_ > > +#define _smemh_ > > + > > +/* 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. > > > + * @size: number of bytes to be allocated > > + * @return 0 if OK, -ve on error > > + */ > > + int (*alloc)(unsigned int host, > > + unsigned int item, size_t size); > > Fits on one line? > > + > > + /** > > + * get() - Resolve ptr of size of a smem item > > + * > > + * @host: the remote processor, of -1 > > + * @item: smem item handle > > + * @size: pointer to be filled out with the size of the > item > > + * @return pointer on success, NULL on error > > + */ > > + void *(*get)(unsigned int host, > > + unsigned int item, size_t *size); > > Fits on one line? > > > + > > + /** > > + * get_free_space() - Set the PWM invert > > + * > > + * @host: the remote processor identifying a partition, or -1 > > + * @return free space, -ve on error > > free space in bytes ? > > > + */ > > + int (*get_free_space)(unsigned int host); > > +}; > > + > > +#define smem_get_ops(dev) ((struct smem_ops *)(dev)->driver->ops) > > + > > +/** > > + * smem_alloc() - allocate space for a smem item > > + * @host: remote processor id, or -1 > > + * @item: smem item handle > > + * @size: number of bytes to be allocated > > + * @return 0 if OK, -ve on error > > + * > > + * Allocate space for a given smem item of size @size, given that the > item is > > + * not yet allocated. > > + */ > > +int smem_alloc(struct udevice *dev, unsigned int host, > > + unsigned int item, size_t size); > > + > > +/** > > + * smem_get() - resolve ptr of size of a smem item > > + * @host: the remote processor, or -1 > > + * @item: smem item handle > > + * @size: pointer to be filled out with size of the item > > + * @return pointer on success, NULL on error > > + * > > + * Looks up smem item and returns pointer to it. Size of smem > > + * item is returned in @size. > > + */ > > +void *smem_get(struct udevice *dev, unsigned int host, > > + unsigned int item, size_t *size); > > + > > +/** > > + * smem_get_free_space() - retrieve amount of free space in a partition > > + * @host: the remote processor identifying a partition, or -1 > > + * @free_space: pointer to be filled out with free space > > + * @return 0 if OK, -ve on error > > + * > > + * To be used by smem clients as a quick way to determine if any new > > + * allocations has been made. > > + */ > > +int smem_get_free_space(struct udevice *dev, unsigned int host, size_t > *free_space); > > + > > +#endif /* _smem_h_ */ > > + > > -- > > 2.17.1 > > > > Regards, > Simon > _______________________________________________ U-Boot mailing list [email protected] https://lists.denx.de/listinfo/u-boot
