RE: [PATCH] dm: soc: Add SoC id for attribute matching
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
> > Reviewed-by: Lad Prabhakar lad...@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
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.
"SOC" could be changed to "SoC", no strong feelings there.
Signed-off-by: Biju Das
Reviewed-by: Lad Prabhakar
---
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
[PATCH] dm: soc: Add SoC id for attribute matching
Add SoC identification string for attribute matching.
Also changed the comments from "an SOC" to "a SoC".
Signed-off-by: Biju Das
Reviewed-by: Lad Prabhakar
---
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)
+{
+ 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
