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

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

[Ramon Fried]

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

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 <ramon.fried@gmail.com>
+ */
+
+#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;
+
+	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.
+ *
+ * Copyright (c) 2018 Ramon Fried <ramon.fried@gmail.com>
+ */
+
+#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
+	 * @item:	smem item handle
+	 * @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);
+
+	/**
+	 * 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);
+
+	/**
+	 * get_free_space() - Set the PWM invert
+	 *
+	 * @host:   the remote processor identifying a partition, or -1
+	 * @return	free space, -ve on error
+	 */
+	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

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

[Simon Glass]

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.

> 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 <ramon.fried@gmail.com>
> + */
> +
> +#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 <ramon.fried@gmail.com>
> + */
> +
> +#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?

> +        * @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] [PATCH v2 1/6] dm: SMEM (Shared memory) uclass

[Ramon Fried]

Hi Simon

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.
>
> > 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 <ramon.fried@gmail.com>
> > + */
> > +
> > +#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.
>
I fixed smem_get_free_space() to look the same as get_free_space().


> > +
> > +       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 <ramon.fried@gmail.com>
> > + */
> > +
> > +#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
>
Done.

>
> > +        * @item:       smem item handle
>
> How does one choose this? What does it mean? Can you expand this comment a
> bit?
>
> > +        * @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?
>
Checkpatch complained, but I agree, this should be 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 ?
>
Yes. Fixed.

>
> > +        */
> > +       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
> >
>
Thanks,
Ramon.

>
> Regards,
> Simon
>

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

[Ramon Fried]

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.
>
> > 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 <ramon.fried@gmail.com>
> > + */
> > +
> > +#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 <ramon.fried@gmail.com>
> > + */
> > +
> > +#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] [PATCH v2 1/6] dm: SMEM (Shared memory) uclass

[Simon Glass]

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.

Regards,
Simon

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

[Ramon Fried]

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
>

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

[Dr. Philipp Tomsich]

> On 22 Jun 2018, at 04: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>

Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>

See below for a requested correction.

> 
> ---
> 
> 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
> 
> 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 <ramon.fried@gmail.com>
> + */
> +
> +#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;
> +
> +	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.
> + *
> + * Copyright (c) 2018 Ramon Fried <ramon.fried@gmail.com>
> + */
> +
> +#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
> +	 * @item:	smem item handle
> +	 * @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);
> +
> +	/**
> +	 * 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);
> +
> +	/**
> +	 * get_free_space() - Set the PWM invert

The “Set the PWM invert” part seems like a mistake here...

> +	 *
> +	 * @host:   the remote processor identifying a partition, or -1
> +	 * @return	free space, -ve on error
> +	 */
> +	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
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 529 bytes
Desc: Message signed with OpenPGP
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180626/ea06cd95/attachment.sig>

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

[Ramon Fried]

On June 26, 2018 9:39:55 AM GMT+03:00, "Dr. Philipp Tomsich" <philipp.tomsich@theobroma-systems.com> wrote:
>
>> On 22 Jun 2018, at 04: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>
>
>Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
>
>See below for a requested correction.
>
>> 
Thanks. Nice catch... 
>> ---
>> 
>> 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
>> 
>> 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 <ramon.fried@gmail.com>
>> + */
>> +
>> +#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;
>> +
>> +	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.
>> + *
>> + * Copyright (c) 2018 Ramon Fried <ramon.fried@gmail.com>
>> + */
>> +
>> +#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
>> +	 * @item:	smem item handle
>> +	 * @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);
>> +
>> +	/**
>> +	 * 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);
>> +
>> +	/**
>> +	 * get_free_space() - Set the PWM invert
>
>The “Set the PWM invert” part seems like a mistake here...
>
>> +	 *
>> +	 * @host:   the remote processor identifying a partition, or -1
>> +	 * @return	free space, -ve on error
>> +	 */
>> +	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
>> 


-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

[U-Boot] [PATCH v2 2/6] soc: qualcomm: Add Shared Memory Manager driver

[Ramon Fried]

The Shared Memory Manager driver implements an interface for allocating
and accessing items in the memory area shared among all of the
processors in a Qualcomm platform.

Adapted from the Linux driver (4.17)

Changes from the original Linux driver:
* Removed HW spinlock mechanism, which is irrelevant
in U-boot particualar use case, which is just reading from the smem.
* adaptaion from Linux driver model to U-boot's.

Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
Signed-off-by: Ramon Fried <ramon.fried@gmail.com>

---

Changes in v2:
- Applied checkpatch fixes (also sent these to Linux upstream)
- Changed UCLASS_SOC to UCLASS_SMEM
- Removed function exports and registered functionality through .ops

 MAINTAINERS             |   1 +
 arch/arm/Kconfig        |   2 +
 drivers/Kconfig         |   2 +
 drivers/smem/Kconfig    |  13 +
 drivers/smem/Makefile   |   1 +
 drivers/smem/msm_smem.c | 941 ++++++++++++++++++++++++++++++++++++++++
 6 files changed, 960 insertions(+)
 create mode 100644 drivers/smem/msm_smem.c

diff --git a/MAINTAINERS b/MAINTAINERS
index b2c9717cb7..b691bae13c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -187,6 +187,7 @@ ARM SNAPDRAGON
 M:	Ramon Fried <ramon.fried@gmail.com>
 S:	Maintained
 F:	arch/arm/mach-snapdragon/
+F:	drivers/smem/msm_smem.c
 
 ARM STI
 M:	Patrice Chotard <patrice.chotard@st.com>
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 22234cde2a..badf4cacb8 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -728,6 +728,8 @@ config ARCH_SNAPDRAGON
 	select SPMI
 	select OF_CONTROL
 	select OF_SEPARATE
+	select SMEM
+	select MSM_SMEM
 
 config ARCH_SOCFPGA
 	bool "Altera SOCFPGA family"
diff --git a/drivers/Kconfig b/drivers/Kconfig
index 9e21b28750..c72abf8932 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -84,6 +84,8 @@ source "drivers/scsi/Kconfig"
 
 source "drivers/serial/Kconfig"
 
+source "drivers/smem/Kconfig"
+
 source "drivers/sound/Kconfig"
 
 source "drivers/spi/Kconfig"
diff --git a/drivers/smem/Kconfig b/drivers/smem/Kconfig
index 64337a8b8e..77ad02e236 100644
--- a/drivers/smem/Kconfig
+++ b/drivers/smem/Kconfig
@@ -1,2 +1,15 @@
 menuconfig SMEM
 	bool  "SMEM (Shared Memory mamanger) support"
+
+if SMEM
+
+config MSM_SMEM
+    bool "Qualcomm Shared Memory Manager (SMEM)"
+    depends on DM
+    depends on ARCH_SNAPDRAGON
+    help
+      enable support for the Qualcomm Shared Memory Manager.
+      The driver provides an interface to items in a heap shared among all
+      processors in a Qualcomm platform.
+
+endif # menu "SMEM Support"
diff --git a/drivers/smem/Makefile b/drivers/smem/Makefile
index ca55c4512d..605b8fc290 100644
--- a/drivers/smem/Makefile
+++ b/drivers/smem/Makefile
@@ -3,3 +3,4 @@
 # Makefile for the U-Boot SMEM interface drivers
 
 obj-$(CONFIG_SMEM) += smem-uclass.o
+obj-$(CONFIG_MSM_SMEM) += msm_smem.o
diff --git a/drivers/smem/msm_smem.c b/drivers/smem/msm_smem.c
new file mode 100644
index 0000000000..183ab7b54b
--- /dev/null
+++ b/drivers/smem/msm_smem.c
@@ -0,0 +1,941 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2015, Sony Mobile Communications AB.
+ * Copyright (c) 2012-2013, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2018, Ramon Fried <ramon.fried@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <common.h>
+#include <errno.h>
+#include <dm.h>
+#include <dm/of_access.h>
+#include <dm/of_addr.h>
+#include <asm/io.h>
+#include <linux/ioport.h>
+#include <linux/io.h>
+#include <smem.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+/*
+ * The Qualcomm shared memory system is an allocate only heap structure that
+ * consists of one of more memory areas that can be accessed by the processors
+ * in the SoC.
+ *
+ * All systems contains a global heap, accessible by all processors in the SoC,
+ * with a table of contents data structure (@smem_header) at the beginning of
+ * the main shared memory block.
+ *
+ * The global header contains meta data for allocations as well as a fixed list
+ * of 512 entries (@smem_global_entry) that can be initialized to reference
+ * parts of the shared memory space.
+ *
+ *
+ * In addition to this global heap a set of "private" heaps can be set up at
+ * boot time with access restrictions so that only certain processor pairs can
+ * access the data.
+ *
+ * These partitions are referenced from an optional partition table
+ * (@smem_ptable), that is found 4kB from the end of the main smem region. The
+ * partition table entries (@smem_ptable_entry) lists the involved processors
+ * (or hosts) and their location in the main shared memory region.
+ *
+ * Each partition starts with a header (@smem_partition_header) that identifies
+ * the partition and holds properties for the two internal memory regions. The
+ * two regions are cached and non-cached memory respectively. Each region
+ * contain a link list of allocation headers (@smem_private_entry) followed by
+ * their data.
+ *
+ * Items in the non-cached region are allocated from the start of the partition
+ * while items in the cached region are allocated from the end. The free area
+ * is hence the region between the cached and non-cached offsets. The header of
+ * cached items comes after the data.
+ *
+ * Version 12 (SMEM_GLOBAL_PART_VERSION) changes the item alloc/get procedure
+ * for the global heap. A new global partition is created from the global heap
+ * region with partition type (SMEM_GLOBAL_HOST) and the max smem item count is
+ * set by the bootloader.
+ *
+ */
+
+/*
+ * The version member of the smem header contains an array of versions for the
+ * various software components in the SoC. We verify that the boot loader
+ * version is a valid version as a sanity check.
+ */
+#define SMEM_MASTER_SBL_VERSION_INDEX	7
+#define SMEM_GLOBAL_HEAP_VERSION	11
+#define SMEM_GLOBAL_PART_VERSION	12
+
+/*
+ * The first 8 items are only to be allocated by the boot loader while
+ * initializing the heap.
+ */
+#define SMEM_ITEM_LAST_FIXED	8
+
+/* Highest accepted item number, for both global and private heaps */
+#define SMEM_ITEM_COUNT		512
+
+/* Processor/host identifier for the application processor */
+#define SMEM_HOST_APPS		0
+
+/* Processor/host identifier for the global partition */
+#define SMEM_GLOBAL_HOST	0xfffe
+
+/* Max number of processors/hosts in a system */
+#define SMEM_HOST_COUNT		10
+
+/**
+ * struct smem_proc_comm - proc_comm communication struct (legacy)
+ * @command:	current command to be executed
+ * @status:	status of the currently requested command
+ * @params:	parameters to the command
+ */
+struct smem_proc_comm {
+	__le32 command;
+	__le32 status;
+	__le32 params[2];
+};
+
+/**
+ * struct smem_global_entry - entry to reference smem items on the heap
+ * @allocated:	boolean to indicate if this entry is used
+ * @offset:	offset to the allocated space
+ * @size:	size of the allocated space, 8 byte aligned
+ * @aux_base:	base address for the memory region used by this unit, or 0 for
+ *		the default region. bits 0,1 are reserved
+ */
+struct smem_global_entry {
+	__le32 allocated;
+	__le32 offset;
+	__le32 size;
+	__le32 aux_base; /* bits 1:0 reserved */
+};
+#define AUX_BASE_MASK		0xfffffffc
+
+/**
+ * struct smem_header - header found in beginning of primary smem region
+ * @proc_comm:		proc_comm communication interface (legacy)
+ * @version:		array of versions for the various subsystems
+ * @initialized:	boolean to indicate that smem is initialized
+ * @free_offset:	index of the first unallocated byte in smem
+ * @available:		number of bytes available for allocation
+ * @reserved:		reserved field, must be 0
+ * toc:			array of references to items
+ */
+struct smem_header {
+	struct smem_proc_comm proc_comm[4];
+	__le32 version[32];
+	__le32 initialized;
+	__le32 free_offset;
+	__le32 available;
+	__le32 reserved;
+	struct smem_global_entry toc[SMEM_ITEM_COUNT];
+};
+
+/**
+ * struct smem_ptable_entry - one entry in the @smem_ptable list
+ * @offset:	offset, within the main shared memory region, of the partition
+ * @size:	size of the partition
+ * @flags:	flags for the partition (currently unused)
+ * @host0:	first processor/host with access to this partition
+ * @host1:	second processor/host with access to this partition
+ * @cacheline:	alignment for "cached" entries
+ * @reserved:	reserved entries for later use
+ */
+struct smem_ptable_entry {
+	__le32 offset;
+	__le32 size;
+	__le32 flags;
+	__le16 host0;
+	__le16 host1;
+	__le32 cacheline;
+	__le32 reserved[7];
+};
+
+/**
+ * struct smem_ptable - partition table for the private partitions
+ * @magic:	magic number, must be SMEM_PTABLE_MAGIC
+ * @version:	version of the partition table
+ * @num_entries: number of partitions in the table
+ * @reserved:	for now reserved entries
+ * @entry:	list of @smem_ptable_entry for the @num_entries partitions
+ */
+struct smem_ptable {
+	u8 magic[4];
+	__le32 version;
+	__le32 num_entries;
+	__le32 reserved[5];
+	struct smem_ptable_entry entry[];
+};
+
+static const u8 SMEM_PTABLE_MAGIC[] = { 0x24, 0x54, 0x4f, 0x43 }; /* "$TOC" */
+
+/**
+ * struct smem_partition_header - header of the partitions
+ * @magic:	magic number, must be SMEM_PART_MAGIC
+ * @host0:	first processor/host with access to this partition
+ * @host1:	second processor/host with access to this partition
+ * @size:	size of the partition
+ * @offset_free_uncached: offset to the first free byte of uncached memory in
+ *		this partition
+ * @offset_free_cached: offset to the first free byte of cached memory in this
+ *		partition
+ * @reserved:	for now reserved entries
+ */
+struct smem_partition_header {
+	u8 magic[4];
+	__le16 host0;
+	__le16 host1;
+	__le32 size;
+	__le32 offset_free_uncached;
+	__le32 offset_free_cached;
+	__le32 reserved[3];
+};
+
+static const u8 SMEM_PART_MAGIC[] = { 0x24, 0x50, 0x52, 0x54 };
+
+/**
+ * struct smem_private_entry - header of each item in the private partition
+ * @canary:	magic number, must be SMEM_PRIVATE_CANARY
+ * @item:	identifying number of the smem item
+ * @size:	size of the data, including padding bytes
+ * @padding_data: number of bytes of padding of data
+ * @padding_hdr: number of bytes of padding between the header and the data
+ * @reserved:	for now reserved entry
+ */
+struct smem_private_entry {
+	u16 canary; /* bytes are the same so no swapping needed */
+	__le16 item;
+	__le32 size; /* includes padding bytes */
+	__le16 padding_data;
+	__le16 padding_hdr;
+	__le32 reserved;
+};
+#define SMEM_PRIVATE_CANARY	0xa5a5
+
+/**
+ * struct smem_info - smem region info located after the table of contents
+ * @magic:	magic number, must be SMEM_INFO_MAGIC
+ * @size:	size of the smem region
+ * @base_addr:	base address of the smem region
+ * @reserved:	for now reserved entry
+ * @num_items:	highest accepted item number
+ */
+struct smem_info {
+	u8 magic[4];
+	__le32 size;
+	__le32 base_addr;
+	__le32 reserved;
+	__le16 num_items;
+};
+
+static const u8 SMEM_INFO_MAGIC[] = { 0x53, 0x49, 0x49, 0x49 }; /* SIII */
+
+/**
+ * struct smem_region - representation of a chunk of memory used for smem
+ * @aux_base:	identifier of aux_mem base
+ * @virt_base:	virtual base address of memory with this aux_mem identifier
+ * @size:	size of the memory region
+ */
+struct smem_region {
+	u32 aux_base;
+	void __iomem *virt_base;
+	size_t size;
+};
+
+/**
+ * struct qcom_smem - device data for the smem device
+ * @dev:	device pointer
+ * @global_partition:	pointer to global partition when in use
+ * @global_cacheline:	cacheline size for global partition
+ * @partitions:	list of pointers to partitions affecting the current
+ *		processor/host
+ * @cacheline:	list of cacheline sizes for each host
+ * @item_count: max accepted item number
+ * @num_regions: number of @regions
+ * @regions:	list of the memory regions defining the shared memory
+ */
+struct qcom_smem {
+	struct udevice *dev;
+
+	struct smem_partition_header *global_partition;
+	size_t global_cacheline;
+	struct smem_partition_header *partitions[SMEM_HOST_COUNT];
+	size_t cacheline[SMEM_HOST_COUNT];
+	u32 item_count;
+
+	unsigned int num_regions;
+	struct smem_region regions[0];
+};
+
+static struct smem_private_entry *
+phdr_to_last_uncached_entry(struct smem_partition_header *phdr)
+{
+	void *p = phdr;
+
+	return p + le32_to_cpu(phdr->offset_free_uncached);
+}
+
+static void *phdr_to_first_cached_entry(struct smem_partition_header *phdr,
+					size_t cacheline)
+{
+	void *p = phdr;
+
+	return p + le32_to_cpu(phdr->size) - ALIGN(sizeof(*phdr), cacheline);
+}
+
+static void *phdr_to_last_cached_entry(struct smem_partition_header *phdr)
+{
+	void *p = phdr;
+
+	return p + le32_to_cpu(phdr->offset_free_cached);
+}
+
+static struct smem_private_entry *
+phdr_to_first_uncached_entry(struct smem_partition_header *phdr)
+{
+	void *p = phdr;
+
+	return p + sizeof(*phdr);
+}
+
+static struct smem_private_entry *
+uncached_entry_next(struct smem_private_entry *e)
+{
+	void *p = e;
+
+	return p + sizeof(*e) + le16_to_cpu(e->padding_hdr) +
+	       le32_to_cpu(e->size);
+}
+
+static struct smem_private_entry *
+cached_entry_next(struct smem_private_entry *e, size_t cacheline)
+{
+	void *p = e;
+
+	return p - le32_to_cpu(e->size) - ALIGN(sizeof(*e), cacheline);
+}
+
+static void *uncached_entry_to_item(struct smem_private_entry *e)
+{
+	void *p = e;
+
+	return p + sizeof(*e) + le16_to_cpu(e->padding_hdr);
+}
+
+static void *cached_entry_to_item(struct smem_private_entry *e)
+{
+	void *p = e;
+
+	return p - le32_to_cpu(e->size);
+}
+
+/* Pointer to the one and only smem handle */
+static struct qcom_smem *__smem;
+
+static int qcom_smem_alloc_private(struct qcom_smem *smem,
+				   struct smem_partition_header *phdr,
+				   unsigned int item,
+				   size_t size)
+{
+	struct smem_private_entry *hdr, *end;
+	size_t alloc_size;
+	void *cached;
+
+	hdr = phdr_to_first_uncached_entry(phdr);
+	end = phdr_to_last_uncached_entry(phdr);
+	cached = phdr_to_last_cached_entry(phdr);
+
+	while (hdr < end) {
+		if (hdr->canary != SMEM_PRIVATE_CANARY) {
+			dev_err(smem->dev,
+				"Found invalid canary in hosts %d:%d partition\n",
+				phdr->host0, phdr->host1);
+			return -EINVAL;
+		}
+
+		if (le16_to_cpu(hdr->item) == item)
+			return -EEXIST;
+
+		hdr = uncached_entry_next(hdr);
+	}
+
+	/* Check that we don't grow into the cached region */
+	alloc_size = sizeof(*hdr) + ALIGN(size, 8);
+	if ((void *)hdr + alloc_size >= cached) {
+		dev_err(smem->dev, "Out of memory\n");
+		return -ENOSPC;
+	}
+
+	hdr->canary = SMEM_PRIVATE_CANARY;
+	hdr->item = cpu_to_le16(item);
+	hdr->size = cpu_to_le32(ALIGN(size, 8));
+	hdr->padding_data = cpu_to_le16(le32_to_cpu(hdr->size) - size);
+	hdr->padding_hdr = 0;
+
+	/*
+	 * Ensure the header is written before we advance the free offset, so
+	 * that remote processors that does not take the remote spinlock still
+	 * gets a consistent view of the linked list.
+	 */
+	dmb();
+	le32_add_cpu(&phdr->offset_free_uncached, alloc_size);
+
+	return 0;
+}
+
+static int qcom_smem_alloc_global(struct qcom_smem *smem,
+				  unsigned int item,
+				  size_t size)
+{
+	struct smem_global_entry *entry;
+	struct smem_header *header;
+
+	header = smem->regions[0].virt_base;
+	entry = &header->toc[item];
+	if (entry->allocated)
+		return -EEXIST;
+
+	size = ALIGN(size, 8);
+	if (WARN_ON(size > le32_to_cpu(header->available)))
+		return -ENOMEM;
+
+	entry->offset = header->free_offset;
+	entry->size = cpu_to_le32(size);
+
+	/*
+	 * Ensure the header is consistent before we mark the item allocated,
+	 * so that remote processors will get a consistent view of the item
+	 * even though they do not take the spinlock on read.
+	 */
+	dmb();
+	entry->allocated = cpu_to_le32(1);
+
+	le32_add_cpu(&header->free_offset, size);
+	le32_add_cpu(&header->available, -size);
+
+	return 0;
+}
+
+/**
+ * qcom_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
+ *
+ * Allocate space for a given smem item of size @size, given that the item is
+ * not yet allocated.
+ */
+static int qcom_smem_alloc(unsigned int host, unsigned int item, size_t size)
+{
+	struct smem_partition_header *phdr;
+	int ret;
+
+	if (!__smem)
+		return -EPROBE_DEFER;
+
+	if (item < SMEM_ITEM_LAST_FIXED) {
+		dev_err(__smem->dev,
+			"Rejecting allocation of static entry %d\n", item);
+		return -EINVAL;
+	}
+
+	if (WARN_ON(item >= __smem->item_count))
+		return -EINVAL;
+
+	if (host < SMEM_HOST_COUNT && __smem->partitions[host]) {
+		phdr = __smem->partitions[host];
+		ret = qcom_smem_alloc_private(__smem, phdr, item, size);
+	} else if (__smem->global_partition) {
+		phdr = __smem->global_partition;
+		ret = qcom_smem_alloc_private(__smem, phdr, item, size);
+	} else {
+		ret = qcom_smem_alloc_global(__smem, item, size);
+	}
+
+	return ret;
+}
+
+static void *qcom_smem_get_global(struct qcom_smem *smem,
+				  unsigned int item,
+				  size_t *size)
+{
+	struct smem_header *header;
+	struct smem_region *area;
+	struct smem_global_entry *entry;
+	u32 aux_base;
+	unsigned int i;
+
+	header = smem->regions[0].virt_base;
+	entry = &header->toc[item];
+	if (!entry->allocated)
+		return ERR_PTR(-ENXIO);
+
+	aux_base = le32_to_cpu(entry->aux_base) & AUX_BASE_MASK;
+
+	for (i = 0; i < smem->num_regions; i++) {
+		area = &smem->regions[i];
+
+		if (area->aux_base == aux_base || !aux_base) {
+			if (size != NULL)
+				*size = le32_to_cpu(entry->size);
+			return area->virt_base + le32_to_cpu(entry->offset);
+		}
+	}
+
+	return ERR_PTR(-ENOENT);
+}
+
+static void *qcom_smem_get_private(struct qcom_smem *smem,
+				   struct smem_partition_header *phdr,
+				   size_t cacheline,
+				   unsigned int item,
+				   size_t *size)
+{
+	struct smem_private_entry *e, *end;
+
+	e = phdr_to_first_uncached_entry(phdr);
+	end = phdr_to_last_uncached_entry(phdr);
+
+	while (e < end) {
+		if (e->canary != SMEM_PRIVATE_CANARY)
+			goto invalid_canary;
+
+		if (le16_to_cpu(e->item) == item) {
+			if (size != NULL)
+				*size = le32_to_cpu(e->size) -
+					le16_to_cpu(e->padding_data);
+
+			return uncached_entry_to_item(e);
+		}
+
+		e = uncached_entry_next(e);
+	}
+
+	/* Item was not found in the uncached list, search the cached list */
+
+	e = phdr_to_first_cached_entry(phdr, cacheline);
+	end = phdr_to_last_cached_entry(phdr);
+
+	while (e > end) {
+		if (e->canary != SMEM_PRIVATE_CANARY)
+			goto invalid_canary;
+
+		if (le16_to_cpu(e->item) == item) {
+			if (size != NULL)
+				*size = le32_to_cpu(e->size) -
+					le16_to_cpu(e->padding_data);
+
+			return cached_entry_to_item(e);
+		}
+
+		e = cached_entry_next(e, cacheline);
+	}
+
+	return ERR_PTR(-ENOENT);
+
+invalid_canary:
+	dev_err(smem->dev, "Found invalid canary in hosts %d:%d partition\n",
+			phdr->host0, phdr->host1);
+
+	return ERR_PTR(-EINVAL);
+}
+
+/**
+ * qcom_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
+ *
+ * Looks up smem item and returns pointer to it. Size of smem
+ * item is returned in @size.
+ */
+static void *qcom_smem_get(unsigned int host, unsigned int item, size_t *size)
+{
+	struct smem_partition_header *phdr;
+	size_t cacheln;
+	void *ptr = ERR_PTR(-EPROBE_DEFER);
+
+	if (!__smem)
+		return ptr;
+
+	if (WARN_ON(item >= __smem->item_count))
+		return ERR_PTR(-EINVAL);
+
+	if (host < SMEM_HOST_COUNT && __smem->partitions[host]) {
+		phdr = __smem->partitions[host];
+		cacheln = __smem->cacheline[host];
+		ptr = qcom_smem_get_private(__smem, phdr, cacheln, item, size);
+	} else if (__smem->global_partition) {
+		phdr = __smem->global_partition;
+		cacheln = __smem->global_cacheline;
+		ptr = qcom_smem_get_private(__smem, phdr, cacheln, item, size);
+	} else {
+		ptr = qcom_smem_get_global(__smem, item, size);
+	}
+
+	return ptr;
+
+}
+
+/**
+ * qcom_smem_get_free_space() - retrieve amount of free space in a partition
+ * @host:	the remote processor identifying a partition, or -1
+ *
+ * To be used by smem clients as a quick way to determine if any new
+ * allocations has been made.
+ */
+static int qcom_smem_get_free_space(unsigned int host)
+{
+	struct smem_partition_header *phdr;
+	struct smem_header *header;
+	unsigned int ret;
+
+	if (!__smem)
+		return -EPROBE_DEFER;
+
+	if (host < SMEM_HOST_COUNT && __smem->partitions[host]) {
+		phdr = __smem->partitions[host];
+		ret = le32_to_cpu(phdr->offset_free_cached) -
+		      le32_to_cpu(phdr->offset_free_uncached);
+	} else if (__smem->global_partition) {
+		phdr = __smem->global_partition;
+		ret = le32_to_cpu(phdr->offset_free_cached) -
+		      le32_to_cpu(phdr->offset_free_uncached);
+	} else {
+		header = __smem->regions[0].virt_base;
+		ret = le32_to_cpu(header->available);
+	}
+
+	return ret;
+}
+
+static int qcom_smem_get_sbl_version(struct qcom_smem *smem)
+{
+	struct smem_header *header;
+	__le32 *versions;
+
+	header = smem->regions[0].virt_base;
+	versions = header->version;
+
+	return le32_to_cpu(versions[SMEM_MASTER_SBL_VERSION_INDEX]);
+}
+
+static struct smem_ptable *qcom_smem_get_ptable(struct qcom_smem *smem)
+{
+	struct smem_ptable *ptable;
+	u32 version;
+
+	ptable = smem->regions[0].virt_base + smem->regions[0].size - SZ_4K;
+	if (memcmp(ptable->magic, SMEM_PTABLE_MAGIC, sizeof(ptable->magic)))
+		return ERR_PTR(-ENOENT);
+
+	version = le32_to_cpu(ptable->version);
+	if (version != 1) {
+		dev_err(smem->dev,
+			"Unsupported partition header version %d\n", version);
+		return ERR_PTR(-EINVAL);
+	}
+	return ptable;
+}
+
+static u32 qcom_smem_get_item_count(struct qcom_smem *smem)
+{
+	struct smem_ptable *ptable;
+	struct smem_info *info;
+
+	ptable = qcom_smem_get_ptable(smem);
+	if (IS_ERR_OR_NULL(ptable))
+		return SMEM_ITEM_COUNT;
+
+	info = (struct smem_info *)&ptable->entry[ptable->num_entries];
+	if (memcmp(info->magic, SMEM_INFO_MAGIC, sizeof(info->magic)))
+		return SMEM_ITEM_COUNT;
+
+	return le16_to_cpu(info->num_items);
+}
+
+static int qcom_smem_set_global_partition(struct qcom_smem *smem)
+{
+	struct smem_partition_header *header;
+	struct smem_ptable_entry *entry = NULL;
+	struct smem_ptable *ptable;
+	u32 host0, host1, size;
+	int i;
+
+	ptable = qcom_smem_get_ptable(smem);
+	if (IS_ERR(ptable))
+		return PTR_ERR(ptable);
+
+	for (i = 0; i < le32_to_cpu(ptable->num_entries); i++) {
+		entry = &ptable->entry[i];
+		host0 = le16_to_cpu(entry->host0);
+		host1 = le16_to_cpu(entry->host1);
+
+		if (host0 == SMEM_GLOBAL_HOST && host0 == host1)
+			break;
+	}
+
+	if (!entry) {
+		dev_err(smem->dev, "Missing entry for global partition\n");
+		return -EINVAL;
+	}
+
+	if (!le32_to_cpu(entry->offset) || !le32_to_cpu(entry->size)) {
+		dev_err(smem->dev, "Invalid entry for global partition\n");
+		return -EINVAL;
+	}
+
+	if (smem->global_partition) {
+		dev_err(smem->dev, "Already found the global partition\n");
+		return -EINVAL;
+	}
+
+	header = smem->regions[0].virt_base + le32_to_cpu(entry->offset);
+	host0 = le16_to_cpu(header->host0);
+	host1 = le16_to_cpu(header->host1);
+
+	if (memcmp(header->magic, SMEM_PART_MAGIC, sizeof(header->magic))) {
+		dev_err(smem->dev, "Global partition has invalid magic\n");
+		return -EINVAL;
+	}
+
+	if (host0 != SMEM_GLOBAL_HOST && host1 != SMEM_GLOBAL_HOST) {
+		dev_err(smem->dev, "Global partition hosts are invalid\n");
+		return -EINVAL;
+	}
+
+	if (le32_to_cpu(header->size) != le32_to_cpu(entry->size)) {
+		dev_err(smem->dev, "Global partition has invalid size\n");
+		return -EINVAL;
+	}
+
+	size = le32_to_cpu(header->offset_free_uncached);
+	if (size > le32_to_cpu(header->size)) {
+		dev_err(smem->dev,
+			"Global partition has invalid free pointer\n");
+		return -EINVAL;
+	}
+
+	smem->global_partition = header;
+	smem->global_cacheline = le32_to_cpu(entry->cacheline);
+
+	return 0;
+}
+
+static int qcom_smem_enumerate_partitions(struct qcom_smem *smem,
+					  unsigned int local_host)
+{
+	struct smem_partition_header *header;
+	struct smem_ptable_entry *entry;
+	struct smem_ptable *ptable;
+	unsigned int remote_host;
+	u32 host0, host1;
+	int i;
+
+	ptable = qcom_smem_get_ptable(smem);
+	if (IS_ERR(ptable))
+		return PTR_ERR(ptable);
+
+	for (i = 0; i < le32_to_cpu(ptable->num_entries); i++) {
+		entry = &ptable->entry[i];
+		host0 = le16_to_cpu(entry->host0);
+		host1 = le16_to_cpu(entry->host1);
+
+		if (host0 != local_host && host1 != local_host)
+			continue;
+
+		if (!le32_to_cpu(entry->offset))
+			continue;
+
+		if (!le32_to_cpu(entry->size))
+			continue;
+
+		if (host0 == local_host)
+			remote_host = host1;
+		else
+			remote_host = host0;
+
+		if (remote_host >= SMEM_HOST_COUNT) {
+			dev_err(smem->dev,
+				"Invalid remote host %d\n",
+				remote_host);
+			return -EINVAL;
+		}
+
+		if (smem->partitions[remote_host]) {
+			dev_err(smem->dev,
+				"Already found a partition for host %d\n",
+				remote_host);
+			return -EINVAL;
+		}
+
+		header = smem->regions[0].virt_base + le32_to_cpu(entry->offset);
+		host0 = le16_to_cpu(header->host0);
+		host1 = le16_to_cpu(header->host1);
+
+		if (memcmp(header->magic, SMEM_PART_MAGIC,
+			    sizeof(header->magic))) {
+			dev_err(smem->dev,
+				"Partition %d has invalid magic\n", i);
+			return -EINVAL;
+		}
+
+		if (host0 != local_host && host1 != local_host) {
+			dev_err(smem->dev,
+				"Partition %d hosts are invalid\n", i);
+			return -EINVAL;
+		}
+
+		if (host0 != remote_host && host1 != remote_host) {
+			dev_err(smem->dev,
+				"Partition %d hosts are invalid\n", i);
+			return -EINVAL;
+		}
+
+		if (le32_to_cpu(header->size) != le32_to_cpu(entry->size)) {
+			dev_err(smem->dev,
+				"Partition %d has invalid size\n", i);
+			return -EINVAL;
+		}
+
+		if (le32_to_cpu(header->offset_free_uncached) > le32_to_cpu(header->size)) {
+			dev_err(smem->dev,
+				"Partition %d has invalid free pointer\n", i);
+			return -EINVAL;
+		}
+
+		smem->partitions[remote_host] = header;
+		smem->cacheline[remote_host] = le32_to_cpu(entry->cacheline);
+	}
+
+	return 0;
+}
+
+static int qcom_smem_map_memory(struct qcom_smem *smem, struct udevice *dev,
+				const char *name, int i)
+{
+	struct fdt_resource r;
+	int ret;
+	int node = dev_of_offset(dev);
+
+	ret = fdtdec_lookup_phandle(gd->fdt_blob, node, name);
+	if (ret < 0) {
+		dev_err(dev, "No %s specified\n", name);
+		return -EINVAL;
+	}
+
+	ret = fdt_get_resource(gd->fdt_blob, ret, "reg", 0, &r);
+	if (ret)
+		return ret;
+
+	smem->regions[i].aux_base = (u32)r.start;
+	smem->regions[i].size = fdt_resource_size(&r);
+	smem->regions[i].virt_base = devm_ioremap(dev, r.start, fdt_resource_size(&r));
+	if (!smem->regions[i].virt_base)
+		return -ENOMEM;
+
+	return 0;
+}
+
+static int qcom_smem_probe(struct udevice *dev)
+{
+	struct smem_header *header;
+	struct qcom_smem *smem;
+	size_t array_size;
+	int num_regions;
+	u32 version;
+	int ret;
+	int node = dev_of_offset(dev);
+
+	num_regions = 1;
+	if (fdtdec_lookup_phandle(gd->fdt_blob, node, "qcomrpm-msg-ram") >= 0)
+		num_regions++;
+
+	array_size = num_regions * sizeof(struct smem_region);
+	smem = devm_kzalloc(dev, sizeof(*smem) + array_size, GFP_KERNEL);
+	if (!smem)
+		return -ENOMEM;
+
+	smem->dev = dev;
+	smem->num_regions = num_regions;
+
+	ret = qcom_smem_map_memory(smem, dev, "memory-region", 0);
+	if (ret)
+		return ret;
+
+	if (num_regions > 1) {
+		ret = qcom_smem_map_memory(smem, dev,
+					"qcom,rpm-msg-ram", 1);
+		if (ret)
+			return ret;
+	}
+
+	header = smem->regions[0].virt_base;
+	if (le32_to_cpu(header->initialized) != 1 ||
+	    le32_to_cpu(header->reserved)) {
+		dev_err(&pdev->dev, "SMEM is not initialized by SBL\n");
+		return -EINVAL;
+	}
+
+	version = qcom_smem_get_sbl_version(smem);
+	switch (version >> 16) {
+	case SMEM_GLOBAL_PART_VERSION:
+		ret = qcom_smem_set_global_partition(smem);
+		if (ret < 0)
+			return ret;
+		smem->item_count = qcom_smem_get_item_count(smem);
+		break;
+	case SMEM_GLOBAL_HEAP_VERSION:
+		smem->item_count = SMEM_ITEM_COUNT;
+		break;
+	default:
+		dev_err(dev, "Unsupported SMEM version 0x%x\n", version);
+		return -EINVAL;
+	}
+
+	ret = qcom_smem_enumerate_partitions(smem, SMEM_HOST_APPS);
+	if (ret < 0 && ret != -ENOENT)
+		return ret;
+
+	__smem = smem;
+
+	return 0;
+}
+
+static int qcom_smem_remove(struct udevice *dev)
+{
+	__smem = NULL;
+
+	return 0;
+}
+
+const struct udevice_id qcom_smem_of_match[] = {
+	{ .compatible = "qcom,smem" },
+	{ }
+};
+
+static const struct smem_ops msm_smem_ops = {
+	.alloc = qcom_smem_alloc,
+	.get = qcom_smem_get,
+	.get_free_space = qcom_smem_get_free_space,
+};
+
+U_BOOT_DRIVER(qcom_smem) = {
+	.name	= "qcom_smem",
+	.id	= UCLASS_SMEM,
+	.of_match = qcom_smem_of_match,
+	.ops = &msm_smem_ops,
+	.probe = qcom_smem_probe,
+	.remove = qcom_smem_remove,
+};
-- 
2.17.1

[U-Boot] [PATCH v2 2/6] soc: qualcomm: Add Shared Memory Manager driver

[Simon Glass]

Hi Ramon,

On 21 June 2018 at 20:11, Ramon Fried <ramon.fried@gmail.com> wrote:
> The Shared Memory Manager driver implements an interface for allocating
> and accessing items in the memory area shared among all of the
> processors in a Qualcomm platform.
>
> Adapted from the Linux driver (4.17)
>
> Changes from the original Linux driver:
> * Removed HW spinlock mechanism, which is irrelevant
> in U-boot particualar use case, which is just reading from the smem.

U-Boot

> * adaptaion from Linux driver model to U-boot's.

Adapted (I think)

>
> Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
> Signed-off-by: Ramon Fried <ramon.fried@gmail.com>
>
> ---
>
> Changes in v2:
> - Applied checkpatch fixes (also sent these to Linux upstream)
> - Changed UCLASS_SOC to UCLASS_SMEM
> - Removed function exports and registered functionality through .ops
>
>  MAINTAINERS             |   1 +
>  arch/arm/Kconfig        |   2 +
>  drivers/Kconfig         |   2 +
>  drivers/smem/Kconfig    |  13 +
>  drivers/smem/Makefile   |   1 +
>  drivers/smem/msm_smem.c | 941 ++++++++++++++++++++++++++++++++++++++++
>  6 files changed, 960 insertions(+)
>  create mode 100644 drivers/smem/msm_smem.c
>

Reviewed-by: Simon Glass <sjg@chromium.org>

with nits as noted.

> diff --git a/MAINTAINERS b/MAINTAINERS
> index b2c9717cb7..b691bae13c 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -187,6 +187,7 @@ ARM SNAPDRAGON
>  M:     Ramon Fried <ramon.fried@gmail.com>
>  S:     Maintained
>  F:     arch/arm/mach-snapdragon/
> +F:     drivers/smem/msm_smem.c
>
>  ARM STI
>  M:     Patrice Chotard <patrice.chotard@st.com>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 22234cde2a..badf4cacb8 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -728,6 +728,8 @@ config ARCH_SNAPDRAGON
>         select SPMI
>         select OF_CONTROL
>         select OF_SEPARATE
> +       select SMEM
> +       select MSM_SMEM
>
>  config ARCH_SOCFPGA
>         bool "Altera SOCFPGA family"
> diff --git a/drivers/Kconfig b/drivers/Kconfig
> index 9e21b28750..c72abf8932 100644
> --- a/drivers/Kconfig
> +++ b/drivers/Kconfig
> @@ -84,6 +84,8 @@ source "drivers/scsi/Kconfig"
>
>  source "drivers/serial/Kconfig"
>
> +source "drivers/smem/Kconfig"
> +
>  source "drivers/sound/Kconfig"
>
>  source "drivers/spi/Kconfig"
> diff --git a/drivers/smem/Kconfig b/drivers/smem/Kconfig
> index 64337a8b8e..77ad02e236 100644
> --- a/drivers/smem/Kconfig
> +++ b/drivers/smem/Kconfig
> @@ -1,2 +1,15 @@
>  menuconfig SMEM
>         bool  "SMEM (Shared Memory mamanger) support"

manager

> +
> +if SMEM
> +
> +config MSM_SMEM
> +    bool "Qualcomm Shared Memory Manager (SMEM)"
> +    depends on DM
> +    depends on ARCH_SNAPDRAGON
> +    help
> +      enable support for the Qualcomm Shared Memory Manager.

Enable

> +      The driver provides an interface to items in a heap shared among all
> +      processors in a Qualcomm platform.
> +
> +endif # menu "SMEM Support"
> diff --git a/drivers/smem/Makefile b/drivers/smem/Makefile
> index ca55c4512d..605b8fc290 100644
> --- a/drivers/smem/Makefile
> +++ b/drivers/smem/Makefile
> @@ -3,3 +3,4 @@
>  # Makefile for the U-Boot SMEM interface drivers
>
>  obj-$(CONFIG_SMEM) += smem-uclass.o
> +obj-$(CONFIG_MSM_SMEM) += msm_smem.o
> diff --git a/drivers/smem/msm_smem.c b/drivers/smem/msm_smem.c
> new file mode 100644
> index 0000000000..183ab7b54b
> --- /dev/null
> +++ b/drivers/smem/msm_smem.c
> @@ -0,0 +1,941 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2015, Sony Mobile Communications AB.
> + * Copyright (c) 2012-2013, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2018, Ramon Fried <ramon.fried@gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.

Can you drop the license? The SPDX should be enough.

> + */
> +
> +#include <common.h>
> +#include <errno.h>
> +#include <dm.h>
> +#include <dm/of_access.h>
> +#include <dm/of_addr.h>
> +#include <asm/io.h>
> +#include <linux/ioport.h>
> +#include <linux/io.h>
> +#include <smem.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +/*
> + * The Qualcomm shared memory system is an allocate only heap structure that

allocate-only

> + * consists of one of more memory areas that can be accessed by the processors
> + * in the SoC.
> + *
> + * All systems contains a global heap, accessible by all processors in the SoC,
> + * with a table of contents data structure (@smem_header) at the beginning of
> + * the main shared memory block.
> + *
> + * The global header contains meta data for allocations as well as a fixed list
> + * of 512 entries (@smem_global_entry) that can be initialized to reference
> + * parts of the shared memory space.
> + *
> + *
> + * In addition to this global heap a set of "private" heaps can be set up at

heap, a

> + * boot time with access restrictions so that only certain processor pairs can
> + * access the data.
> + *
> + * These partitions are referenced from an optional partition table
> + * (@smem_ptable), that is found 4kB from the end of the main smem region. The
> + * partition table entries (@smem_ptable_entry) lists the involved processors
> + * (or hosts) and their location in the main shared memory region.
> + *
> + * Each partition starts with a header (@smem_partition_header) that identifies
> + * the partition and holds properties for the two internal memory regions. The
> + * two regions are cached and non-cached memory respectively. Each region
> + * contain a link list of allocation headers (@smem_private_entry) followed by
> + * their data.
> + *
> + * Items in the non-cached region are allocated from the start of the partition
> + * while items in the cached region are allocated from the end. The free area
> + * is hence the region between the cached and non-cached offsets. The header of
> + * cached items comes after the data.
> + *
> + * Version 12 (SMEM_GLOBAL_PART_VERSION) changes the item alloc/get procedure
> + * for the global heap. A new global partition is created from the global heap
> + * region with partition type (SMEM_GLOBAL_HOST) and the max smem item count is
> + * set by the bootloader.
> + *
> + */
> +

[...]

Regards,
Simon

[U-Boot] [PATCH v2 3/6] dts: db410c: added smem nodes

[Ramon Fried]

Added necessary nodes for Qualcomm smem driver.

Signed-off-by: Ramon Fried <ramon.fried@gmail.com>
---

Changes in v2: None

 arch/arm/dts/dragonboard410c-uboot.dtsi |  5 +++++
 arch/arm/dts/dragonboard410c.dts        | 16 ++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/arch/arm/dts/dragonboard410c-uboot.dtsi b/arch/arm/dts/dragonboard410c-uboot.dtsi
index 50ac2e6584..773439ab93 100644
--- a/arch/arm/dts/dragonboard410c-uboot.dtsi
+++ b/arch/arm/dts/dragonboard410c-uboot.dtsi
@@ -6,6 +6,11 @@
  */
 
 / {
+
+	smem {
+		u-boot,dm-pre-reloc;
+	};
+
 	soc {
 		u-boot,dm-pre-reloc;
 
diff --git a/arch/arm/dts/dragonboard410c.dts b/arch/arm/dts/dragonboard410c.dts
index 182a865b0a..f4f7c350ec 100644
--- a/arch/arm/dts/dragonboard410c.dts
+++ b/arch/arm/dts/dragonboard410c.dts
@@ -27,18 +27,34 @@
 		#address-cells = <2>;
 		#size-cells = <2>;
 		ranges;
+
+		smem_mem: smem_region at 86300000 {
+			reg = <0x0 0x86300000 0x0 0x100000>;
+			no-map;
+		};
 	};
 
 	chosen {
 		stdout-path = "/soc/serial at 78b0000";
 	};
 
+	smem {
+		compatible = "qcom,smem";
+		memory-region = <&smem_mem>;
+		qcom,rpm-msg-ram = <&rpm_msg_ram>;
+	};
+
 	soc {
 		#address-cells = <0x1>;
 		#size-cells = <0x1>;
 		ranges = <0x0 0x0 0x0 0xffffffff>;
 		compatible = "simple-bus";
 
+		rpm_msg_ram: memory at 60000 {
+			compatible = "qcom,rpm-msg-ram";
+			reg = <0x60000 0x8000>;
+		};
+
 		pinctrl: qcom,tlmm at 1000000 {
 			compatible = "qcom,tlmm-apq8016";
 			reg = <0x1000000 0x400000>;
-- 
2.17.1

[U-Boot] [PATCH v2 3/6] dts: db410c: added smem nodes

[Simon Glass]

On 21 June 2018 at 20:11, Ramon Fried <ramon.fried@gmail.com> wrote:
> Added necessary nodes for Qualcomm smem driver.
>
> Signed-off-by: Ramon Fried <ramon.fried@gmail.com>
> ---
>
> Changes in v2: None
>
>  arch/arm/dts/dragonboard410c-uboot.dtsi |  5 +++++
>  arch/arm/dts/dragonboard410c.dts        | 16 ++++++++++++++++
>  2 files changed, 21 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH v2 4/6] dts: db820c: added smem nodes

[Ramon Fried]

Added necessary nodes for Qualcomm smem driver.

Signed-off-by: Ramon Fried <ramon.fried@gmail.com>
---

Changes in v2: None

 arch/arm/dts/dragonboard820c-uboot.dtsi |  4 ++++
 arch/arm/dts/dragonboard820c.dts        | 16 ++++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/arch/arm/dts/dragonboard820c-uboot.dtsi b/arch/arm/dts/dragonboard820c-uboot.dtsi
index 97394cc5b0..d60aa04494 100644
--- a/arch/arm/dts/dragonboard820c-uboot.dtsi
+++ b/arch/arm/dts/dragonboard820c-uboot.dtsi
@@ -6,6 +6,10 @@
  */
 
 / {
+	smem {
+		u-boot,dm-pre-reloc;
+	};
+
 	soc {
 		u-boot,dm-pre-reloc;
 
diff --git a/arch/arm/dts/dragonboard820c.dts b/arch/arm/dts/dragonboard820c.dts
index 7457d7b7e3..ffad8e0e0a 100644
--- a/arch/arm/dts/dragonboard820c.dts
+++ b/arch/arm/dts/dragonboard820c.dts
@@ -28,11 +28,27 @@
 		reg = <0 0x80000000 0 0xc0000000>;
 	};
 
+	reserved-memory {
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		smem_mem: smem_region at 86300000 {
+			reg = <0x0 0x86300000 0x0 0x200000>;
+			no-map;
+		};
+	};
+
 	psci {
 		compatible = "arm,psci-1.0";
 		method = "smc";
 	};
 
+	smem {
+		compatible = "qcom,smem";
+		memory-region = <&smem_mem>;
+	};
+
 	soc: soc {
 		#address-cells = <1>;
 		#size-cells = <1>;
-- 
2.17.1

[U-Boot] [PATCH v2 4/6] dts: db820c: added smem nodes

[Simon Glass]

On 21 June 2018 at 20:11, Ramon Fried <ramon.fried@gmail.com> wrote:
> Added necessary nodes for Qualcomm smem driver.
>
> Signed-off-by: Ramon Fried <ramon.fried@gmail.com>
> ---
>
> Changes in v2: None
>
>  arch/arm/dts/dragonboard820c-uboot.dtsi |  4 ++++
>  arch/arm/dts/dragonboard820c.dts        | 16 ++++++++++++++++
>  2 files changed, 20 insertions(+)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH v2 5/6] drivers: smem: sandbox

[Ramon Fried]

Add Sandbox driver for SMEM. mostly stub operations.

Signed-off-by: Ramon Fried <ramon.fried@gmail.com>
---

Changes in v2: None

 arch/sandbox/dts/test.dts   |  4 ++++
 configs/sandbox64_defconfig |  2 ++
 configs/sandbox_defconfig   |  2 ++
 drivers/smem/Kconfig        |  9 ++++++++
 drivers/smem/Makefile       |  1 +
 drivers/smem/sandbox_smem.c | 45 +++++++++++++++++++++++++++++++++++++
 6 files changed, 63 insertions(+)
 create mode 100644 drivers/smem/sandbox_smem.c

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index 5752bf547e..01e6bc0367 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -382,6 +382,10 @@
 		remoteproc-name = "remoteproc-test-dev2";
 	};
 
+	smem at 0 {
+		compatible = "sandbox,smem";
+	};
+
 	spi at 0 {
 		#address-cells = <1>;
 		#size-cells = <0>;
diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig
index 20a2ab3ffb..1fa85a819c 100644
--- a/configs/sandbox64_defconfig
+++ b/configs/sandbox64_defconfig
@@ -199,3 +199,5 @@ CONFIG_UT_TIME=y
 CONFIG_UT_DM=y
 CONFIG_UT_ENV=y
 CONFIG_UT_OVERLAY=y
+CONFIG_SMEM=y
+CONFIG_SANDBOX_SMEM=y
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index 2fc84a16c9..47f6bfd87f 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -200,3 +200,5 @@ CONFIG_UT_TIME=y
 CONFIG_UT_DM=y
 CONFIG_UT_ENV=y
 CONFIG_UT_OVERLAY=y
+CONFIG_SMEM=y
+CONFIG_SANDBOX_SMEM=y
diff --git a/drivers/smem/Kconfig b/drivers/smem/Kconfig
index 77ad02e236..3aa32a6c13 100644
--- a/drivers/smem/Kconfig
+++ b/drivers/smem/Kconfig
@@ -3,6 +3,15 @@ menuconfig SMEM
 
 if SMEM
 
+config SANDBOX_SMEM
+    bool "Sandbox Shared Memory Manager (SMEM)"
+    depends on SANDBOX && DM
+    help
+      enable SMEM support for sandbox. This is an emulation of a real SMEM
+      manager.
+      The sandbox driver allocates a shared memory from the heap and
+      initialzies it on start.
+
 config MSM_SMEM
     bool "Qualcomm Shared Memory Manager (SMEM)"
     depends on DM
diff --git a/drivers/smem/Makefile b/drivers/smem/Makefile
index 605b8fc290..af3e9b5088 100644
--- a/drivers/smem/Makefile
+++ b/drivers/smem/Makefile
@@ -2,5 +2,6 @@
 #
 # Makefile for the U-Boot SMEM interface drivers
 
+obj-$(CONFIG_SANDBOX_SMEM) += sandbox_smem.o
 obj-$(CONFIG_SMEM) += smem-uclass.o
 obj-$(CONFIG_MSM_SMEM) += msm_smem.o
diff --git a/drivers/smem/sandbox_smem.c b/drivers/smem/sandbox_smem.c
new file mode 100644
index 0000000000..7397e4407a
--- /dev/null
+++ b/drivers/smem/sandbox_smem.c
@@ -0,0 +1,45 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2018 Ramon Fried <ramon.fried@gmail.com>
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <errno.h>
+#include <smem.h>
+#include <asm/test.h>
+
+static int sandbox_smem_alloc(unsigned int host,
+		unsigned int item, size_t size)
+{
+	return 0;
+}
+
+static void *sandbox_smem_get(unsigned int host,
+		unsigned int item, size_t *size)
+{
+	return NULL;
+}
+
+static int sandbox_smem_get_free_space(unsigned int host)
+{
+	return 0;
+}
+
+static const struct smem_ops sandbox_smem_ops = {
+	.alloc	= sandbox_smem_alloc,
+	.get	= sandbox_smem_get,
+	.get_free_space	= sandbox_smem_get_free_space,
+};
+
+static const struct udevice_id sandbox_smem_ids[] = {
+	{ .compatible = "sandbox,smem" },
+	{ }
+};
+
+U_BOOT_DRIVER(smem_sandbox) = {
+	.name		= "smem_sandbox",
+	.id		= UCLASS_SMEM,
+	.of_match	= sandbox_smem_ids,
+	.ops		= &sandbox_smem_ops,
+};
-- 
2.17.1

[U-Boot] [PATCH v2 5/6] drivers: smem: sandbox

[Simon Glass]

On 21 June 2018 at 20:11, Ramon Fried <ramon.fried@gmail.com> wrote:
> Add Sandbox driver for SMEM. mostly stub operations.
>
> Signed-off-by: Ramon Fried <ramon.fried@gmail.com>
> ---
>
> Changes in v2: None
>
>  arch/sandbox/dts/test.dts   |  4 ++++
>  configs/sandbox64_defconfig |  2 ++
>  configs/sandbox_defconfig   |  2 ++
>  drivers/smem/Kconfig        |  9 ++++++++
>  drivers/smem/Makefile       |  1 +
>  drivers/smem/sandbox_smem.c | 45 +++++++++++++++++++++++++++++++++++++
>  6 files changed, 63 insertions(+)
>  create mode 100644 drivers/smem/sandbox_smem.c

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH v2 6/6] test: smem: add basic smem test

[Ramon Fried]

Add basic smem sandbox testing.

Signed-off-by: Ramon Fried <ramon.fried@gmail.com>

---
Tom, this patch depends on https://patchwork.ozlabs.org/patch/932965/

Changes in v2: None

 test/dm/Makefile |  1 +
 test/dm/smem.c   | 29 +++++++++++++++++++++++++++++
 2 files changed, 30 insertions(+)
 create mode 100644 test/dm/smem.c

diff --git a/test/dm/Makefile b/test/dm/Makefile
index 5511a85700..d2ed96c615 100644
--- a/test/dm/Makefile
+++ b/test/dm/Makefile
@@ -33,6 +33,7 @@ obj-$(CONFIG_DM_RESET) += reset.o
 obj-$(CONFIG_SYSRESET) += sysreset.o
 obj-$(CONFIG_DM_RTC) += rtc.o
 obj-$(CONFIG_DM_SPI_FLASH) += sf.o
+obj-$(CONFIG_SMEM) += smem.o
 obj-$(CONFIG_DM_SPI) += spi.o
 obj-y += syscon.o
 obj-$(CONFIG_DM_USB) += usb.o
diff --git a/test/dm/smem.c b/test/dm/smem.c
new file mode 100644
index 0000000000..4d94ddbe5b
--- /dev/null
+++ b/test/dm/smem.c
@@ -0,0 +1,29 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2018 Ramon Fried <ramon.fried@gmail.com>
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <smem.h>
+#include <dm/test.h>
+#include <test/ut.h>
+
+/* Basic test of the smem uclass */
+static int dm_test_smem_base(struct unit_test_state *uts)
+{
+	struct udevice *dev;
+	size_t size;
+
+	ut_assertok(uclass_get_device(UCLASS_SMEM, 0, &dev));
+	ut_assertnonnull(dev);
+	ut_assertok(smem_alloc(dev, -1, 0, 16));
+	ut_assertok(smem_get_free_space(dev, -1, &size);
+	ut_assertnonnull(smem_get(dev, -1, 0, &size));
+	/* Try getting of bound item */
+	ut_assertnnull(smem_get(dev, -1, 70, &size));
+
+	return 0;
+}
+DM_TEST(dm_test_smem_base, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
+
-- 
2.17.1

[U-Boot] [PATCH v2 6/6] test: smem: add basic smem test

[Simon Glass]

On 21 June 2018 at 20:11, Ramon Fried <ramon.fried@gmail.com> wrote:
> Add basic smem sandbox testing.
>
> Signed-off-by: Ramon Fried <ramon.fried@gmail.com>
>
> ---
> Tom, this patch depends on https://patchwork.ozlabs.org/patch/932965/
>
> Changes in v2: None
>
>  test/dm/Makefile |  1 +
>  test/dm/smem.c   | 29 +++++++++++++++++++++++++++++
>  2 files changed, 30 insertions(+)
>  create mode 100644 test/dm/smem.c

Reviewed-by: Simon Glass <sjg@chromium.org>