[U-Boot] [PATCH v2 1/6] dm: SMEM (Shared memory) uclass
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_ */ +
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;elsereturn 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
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;elsereturn 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 -1What does -1 mean? Please expand commment
Done.
* @item: smem item handleHow 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 theitem
* @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 errorfree 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
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;elsereturn 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 -1What does -1 mean? Please expand commment
* @item: smem item handleHow 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 theitem
* @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 errorfree 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
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 -1What does -1 mean? Please expand commment
* @item: smem item handleHow 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
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 -1What does -1 mean? Please expand commment
* @item: smem item handleHow 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
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
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 invertThe “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
participants (3)
-
Dr. Philipp Tomsich -
Ramon Fried -
Simon Glass