[PATCH] dm: soc: Add SoC id for attribute matching
Biju Das
biju.das.jz at bp.renesas.com
Fri Oct 30 15:54:16 CET 2020
Hi Dave,
Thanks for the feedback.
> Subject: Re: [PATCH] dm: soc: Add SoC id for attribute matching
>
> 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.
Agreed. Will change back to an SoC on V2.
The below one still uses "a SOC". Will change to "an SoC" as well.
- * get_revision() - Get revision name of a SOC
+ * get_revision() - Get revision name of a SoC
> "SOC" could be changed to "SoC", no strong feelings there.
OK.
> >
> > 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?
Renesas SoC and other SoC vendors use SoC_ID for SoC identification.See the details here [1]
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/soc/renesas/renesas-soc.c?h=v5.10-rc1
Please see other SoC vendors as well [2] which uses SoC_ID for identification.
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/soc/samsung/exynos-chipid.c?h=v5.10-rc1
> My thought was "family" would cover a range of parts and "machine" was to
> be a specific SoC. Is a more specific ID needed?
Yes, Please see above.
Family in over case is: RCar Gen3 SoC family or RZ/G2 SoC family
Machine is Name of machine for eg;- "HopeRun HiHope RZ/G2M with sub board"
SoC_ID is some thing SoC specific for eg:- r8a774a1
Revision is : ES x.y.
I think in your case, it is different??
>
> > +{
> > + 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.
I have ran the ut_dm test case and is passing.
./test/py/test.py --bd sandbox --build -k ut_dm.
Please let me know, if I should remove the same.
Regards,
Biju
>
> > .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