[PATCH] dm: soc: Add SoC id for attribute matching
Dave Gerlach
d-gerlach at ti.com
Fri Oct 30 15:19:57 CET 2020
Hi,
On 10/30/20 9:07 AM, Biju Das wrote:
> Add SoC identification string for attribute matching.
> Also changed the comments from "an SOC" to "a SoC".
This is not a correct change, "an" should be used if the word that
follows starts with a vowel sound, which "SoC" does.
"SOC" could be changed to "SoC", no strong feelings there.
>
> Signed-off-by: Biju Das <biju.das.jz at bp.renesas.com>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj at bp.renesas.com>
> ---
> drivers/soc/soc-uclass.c | 19 ++++++++++++++++++-
> drivers/soc/soc_sandbox.c | 8 ++++++++
> include/soc.h | 39 +++++++++++++++++++++++++++++++++------
> test/dm/soc.c | 8 ++++++++
> 4 files changed, 67 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/soc/soc-uclass.c b/drivers/soc/soc-uclass.c
> index c32d647864..a3f8be841b 100644
> --- a/drivers/soc/soc-uclass.c
> +++ b/drivers/soc/soc-uclass.c
> @@ -46,6 +46,16 @@ int soc_get_revision(struct udevice *dev, char *buf, int size)
> return ops->get_revision(dev, buf, size);
> }
>
> +int soc_get_soc_id(struct udevice *dev, char *buf, int size)
Is there any additional background here? Why is soc_id needed? Can
"machine" not meet the same purpose?
My thought was "family" would cover a range of parts and "machine" was
to be a specific SoC. Is a more specific ID needed?
> +{
> + struct soc_ops *ops = soc_get_ops(dev);
> +
> + if (!ops->get_soc_id)
> + return -ENOSYS;
> +
> + return ops->get_soc_id(dev, buf, size);
> +}
> +
> const struct soc_attr *
> soc_device_match(const struct soc_attr *matches)
> {
> @@ -61,7 +71,7 @@ soc_device_match(const struct soc_attr *matches)
>
> while (1) {
> if (!(matches->machine || matches->family ||
> - matches->revision))
> + matches->revision || matches->soc_id))
> break;
>
> match = true;
> @@ -87,6 +97,13 @@ soc_device_match(const struct soc_attr *matches)
> }
> }
>
> + if (matches->soc_id) {
> + if (!soc_get_soc_id(soc, str, SOC_MAX_STR_SIZE)) {
> + if (strcmp(matches->soc_id, str))
> + match = false;
> + }
> + }
> +
> if (match)
> return matches;
>
> diff --git a/drivers/soc/soc_sandbox.c b/drivers/soc/soc_sandbox.c
> index 5c82ad84fc..1a81d3562a 100644
> --- a/drivers/soc/soc_sandbox.c
> +++ b/drivers/soc/soc_sandbox.c
> @@ -31,10 +31,18 @@ int soc_sandbox_get_revision(struct udevice *dev, char *buf, int size)
> return 0;
> }
>
> +int soc_sandbox_get_soc_id(struct udevice *dev, char *buf, int size)
> +{
> + snprintf(buf, size, "r8a774a1");
> +
> + return 0;
> +}
> +
> static const struct soc_ops soc_sandbox_ops = {
> .get_family = soc_sandbox_get_family,
> .get_revision = soc_sandbox_get_revision,
> .get_machine = soc_sandbox_get_machine,
> + .get_soc_id = soc_sandbox_get_soc_id,
> };
>
> int soc_sandbox_probe(struct udevice *dev)
> diff --git a/include/soc.h b/include/soc.h
> index a55eb1b572..ae7b36846f 100644
> --- a/include/soc.h
> +++ b/include/soc.h
> @@ -20,18 +20,20 @@
> * variants. Example: am33
> * @machine - Name of a specific SoC. Example: am3352
> * @revision - Name of a specific SoC revision. Example: SR1.1
> + * @soc_id - SoC identification String. Example: r8a774a1
> * @data - A pointer to user data for the SoC variant
> */
> struct soc_attr {
> const char *family;
> const char *machine;
> const char *revision;
> + const char *soc_id;
> const void *data;
> };
>
> struct soc_ops {
> /**
> - * get_machine() - Get machine name of an SOC
> + * get_machine() - Get machine name of a SoC
> *
> * @dev: Device to check (UCLASS_SOC)
> * @buf: Buffer to place string
> @@ -41,7 +43,7 @@ struct soc_ops {
> int (*get_machine)(struct udevice *dev, char *buf, int size);
>
> /**
> - * get_revision() - Get revision name of a SOC
> + * get_revision() - Get revision name of a SoC
> *
> * @dev: Device to check (UCLASS_SOC)
> * @buf: Buffer to place string
> @@ -51,7 +53,7 @@ struct soc_ops {
> int (*get_revision)(struct udevice *dev, char *buf, int size);
>
> /**
> - * get_family() - Get family name of an SOC
> + * get_family() - Get family name of a SoC
> *
> * @dev: Device to check (UCLASS_SOC)
> * @buf: Buffer to place string
> @@ -59,6 +61,16 @@ struct soc_ops {
> * @return 0 if OK, -ENOSPC if buffer is too small, other -ve on error
> */
> int (*get_family)(struct udevice *dev, char *buf, int size);
> +
> + /**
> + * get_soc_id() - Get SoC identification name of a SoC
> + *
> + * @dev: Device to check (UCLASS_SOC)
> + * @buf: Buffer to place string
> + * @size: Size of string space
> + * @return 0 if OK, -ENOSPC if buffer is too small, other -ve on error
> + */
> + int (*get_soc_id)(struct udevice *dev, char *buf, int size);
> };
>
> #define soc_get_ops(dev) ((struct soc_ops *)(dev)->driver->ops)
> @@ -76,7 +88,7 @@ struct soc_ops {
> int soc_get(struct udevice **devp);
>
> /**
> - * soc_get_machine() - Get machine name of an SOC
> + * soc_get_machine() - Get machine name of a SoC
> * @dev: Device to check (UCLASS_SOC)
> * @buf: Buffer to place string
> * @size: Size of string space
> @@ -86,7 +98,7 @@ int soc_get(struct udevice **devp);
> int soc_get_machine(struct udevice *dev, char *buf, int size);
>
> /**
> - * soc_get_revision() - Get revision name of an SOC
> + * soc_get_revision() - Get revision name of a SoC
> * @dev: Device to check (UCLASS_SOC)
> * @buf: Buffer to place string
> * @size: Size of string space
> @@ -96,7 +108,7 @@ int soc_get_machine(struct udevice *dev, char *buf, int size);
> int soc_get_revision(struct udevice *dev, char *buf, int size);
>
> /**
> - * soc_get_family() - Get family name of an SOC
> + * soc_get_family() - Get family name of a SoC
> * @dev: Device to check (UCLASS_SOC)
> * @buf: Buffer to place string
> * @size: Size of string space
> @@ -105,6 +117,16 @@ int soc_get_revision(struct udevice *dev, char *buf, int size);
> */
> int soc_get_family(struct udevice *dev, char *buf, int size);
>
> +/**
> + * soc_get_soc_id() - Get SoC identification name of a SoC
> + * @dev: Device to check (UCLASS_SOC)
> + * @buf: Buffer to place string
> + * @size: Size of string space
> + *
> + * Return: 0 if OK, -ENOSPC if buffer is too small, other -ve on error
> + */
> +int soc_get_soc_id(struct udevice *dev, char *buf, int size);
> +
> /**
> * soc_device_match() - Return match from an array of soc_attr
> * @matches: Array with any combination of family, revision or machine set
> @@ -136,6 +158,11 @@ static inline int soc_get_family(struct udevice *dev, char *buf, int size)
> return -ENOSYS;
> }
>
> +static inline int soc_get_soc_id(struct udevice *dev, char *buf, int size)
> +{
> + return -ENOSYS;
> +}
> +
> static inline const struct soc_attr *
> soc_device_match(const struct soc_attr *matches)
> {
> diff --git a/test/dm/soc.c b/test/dm/soc.c
> index 17e1b5ba01..82cd53ff29 100644
> --- a/test/dm/soc.c
> +++ b/test/dm/soc.c
> @@ -32,24 +32,28 @@ static int dm_test_soc(struct unit_test_state *uts)
> .family = "SANDBOX0xx",
> .machine = "SANDBOX012",
> .revision = "1.0",
> + .soc_id = "r8a774a1",
Some more thought will need to be given here, you can't just add the new
value everywhere, some should not match.
Regards,
Dave
> .data = NULL,
> },
> {
> .family = "SANDBOX1xx",
> .machine = "SANDBOX107",
> .revision = "1.0",
> + .soc_id = "r8a774a1",
> .data = NULL,
> },
> {
> .family = "SANDBOX1xx",
> .machine = "SANDBOX123",
> .revision = "1.0",
> + .soc_id = "r8a774a1",
> .data = &soc_sandbox123_data,
> },
> {
> .family = "SANDBOX1xx",
> .machine = "SANDBOX131",
> .revision = "2.0",
> + .soc_id = "r8a774a1",
> .data = NULL,
> },
> { /* sentinel */ }
> @@ -78,6 +82,7 @@ static int dm_test_soc(struct unit_test_state *uts)
> {
> .family = "SANDBOX0xx",
> .revision = "1.0",
> + .soc_id = "r8a774b1",
> .data = NULL,
> },
> {
> @@ -99,6 +104,9 @@ static int dm_test_soc(struct unit_test_state *uts)
> ut_assertok(soc_get_revision(dev, text, sizeof(text)));
> ut_asserteq_str(text, "1.0");
>
> + ut_assertok(soc_get_soc_id(dev, text, sizeof(text)));
> + ut_asserteq_str(text, "r8a774a1");
> +
> soc_data = soc_device_match(sb_soc_devices_full);
> ut_assert(soc_data);
>
>
More information about the U-Boot
mailing list