| Message ID | 20231002152142.76516-7-detlev.casanova@collabora.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | Marek Vasut |
| Headers | show |
| Series | Introduce the sysinfo command | expand |
On 10/2/23 17:20, Detlev Casanova wrote: > Expose that information to the sysinfo command to let scripts make > decisions based on the board id and revision. > > Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com> > --- > drivers/sysinfo/rcar3.c | 89 +++++++++++++++++++++++++++++------------ > 1 file changed, 63 insertions(+), 26 deletions(-) > > diff --git a/drivers/sysinfo/rcar3.c b/drivers/sysinfo/rcar3.c > index 633e80bc19b..a554323506a 100644 > --- a/drivers/sysinfo/rcar3.c > +++ b/drivers/sysinfo/rcar3.c > @@ -32,6 +32,10 @@ > */ > struct sysinfo_rcar_priv { > char boardmodel[64]; > + u8 id; > + u8 rev_major; > + u8 rev_minor; > + bool has_rev; > u8 val; > }; > > @@ -56,9 +60,33 @@ static int sysinfo_rcar_get_str(struct udevice *dev, int id, size_t size, char * > }; > } > > +static int sysinfo_rcar_get_int(struct udevice *dev, int id, int *val) > +{ > + struct sysinfo_rcar_priv *priv = dev_get_priv(dev); > + > + switch (id) { > + case SYSINFO_ID_BOARD_ID: > + *val = priv->id; > + return 0; > + case SYSINFO_ID_BOARD_REV_MAJOR: > + if (!priv->has_rev) > + return -EINVAL; > + *val = priv->rev_major; > + return 0; > + case SYSINFO_ID_BOARD_REV_MINOR: > + if (!priv->has_rev) > + return -EINVAL; > + *val = priv->rev_minor; > + return 0; > + default: > + return -EINVAL; > + }; > +} > + > static const struct sysinfo_ops sysinfo_rcar_ops = { > .detect = sysinfo_rcar_detect, > .get_str = sysinfo_rcar_get_str, > + .get_int = sysinfo_rcar_get_int, > }; > > static void sysinfo_rcar_parse(struct sysinfo_rcar_priv *priv) > @@ -69,8 +97,9 @@ static void sysinfo_rcar_parse(struct sysinfo_rcar_priv *priv) > bool ebisu_4d = false; > bool condor_i = false; > char rev[4] = "?.?"; > - u8 rev_major = 0; > - u8 rev_minor = 0; > + > + priv->id = board_id; > + priv->has_rev = false; Would it make more sense to assign priv->rev_major = 1; priv->rev_minor = 0; And get rid of priv->has_rev entirely ? Basically say that the default case is rev. 1.0 board. [...]
On Saturday, October 7, 2023 5:35:27 P.M. EDT Marek Vasut wrote: > On 10/2/23 17:20, Detlev Casanova wrote: > > Expose that information to the sysinfo command to let scripts make > > decisions based on the board id and revision. > > > > Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com> > > --- > > > > drivers/sysinfo/rcar3.c | 89 +++++++++++++++++++++++++++++------------ > > 1 file changed, 63 insertions(+), 26 deletions(-) > > > > diff --git a/drivers/sysinfo/rcar3.c b/drivers/sysinfo/rcar3.c > > index 633e80bc19b..a554323506a 100644 > > --- a/drivers/sysinfo/rcar3.c > > +++ b/drivers/sysinfo/rcar3.c > > @@ -32,6 +32,10 @@ > > > > */ > > > > struct sysinfo_rcar_priv { > > > > char boardmodel[64]; > > > > + u8 id; > > + u8 rev_major; > > + u8 rev_minor; > > + bool has_rev; > > > > u8 val; > > > > }; > > > > @@ -56,9 +60,33 @@ static int sysinfo_rcar_get_str(struct udevice *dev, > > int id, size_t size, char *> > > }; > > > > } > > > > +static int sysinfo_rcar_get_int(struct udevice *dev, int id, int *val) > > +{ > > + struct sysinfo_rcar_priv *priv = dev_get_priv(dev); > > + > > + switch (id) { > > + case SYSINFO_ID_BOARD_ID: > > + *val = priv->id; > > + return 0; > > + case SYSINFO_ID_BOARD_REV_MAJOR: > > + if (!priv->has_rev) > > + return -EINVAL; > > + *val = priv->rev_major; > > + return 0; > > + case SYSINFO_ID_BOARD_REV_MINOR: > > + if (!priv->has_rev) > > + return -EINVAL; > > + *val = priv->rev_minor; > > + return 0; > > + default: > > + return -EINVAL; > > + }; > > +} > > + > > > > static const struct sysinfo_ops sysinfo_rcar_ops = { > > > > .detect = sysinfo_rcar_detect, > > .get_str = sysinfo_rcar_get_str, > > > > + .get_int = sysinfo_rcar_get_int, > > > > }; > > > > static void sysinfo_rcar_parse(struct sysinfo_rcar_priv *priv) > > > > @@ -69,8 +97,9 @@ static void sysinfo_rcar_parse(struct sysinfo_rcar_priv > > *priv)> > > bool ebisu_4d = false; > > bool condor_i = false; > > char rev[4] = "?.?"; > > > > - u8 rev_major = 0; > > - u8 rev_minor = 0; > > + > > + priv->id = board_id; > > + priv->has_rev = false; > > Would it make more sense to assign > > priv->rev_major = 1; > priv->rev_minor = 0; > > And get rid of priv->has_rev entirely ? > > Basically say that the default case is rev. 1.0 board. > > [...] I'm not really sure about this has it doesn't differentiate between rev 1.0 and unknown revision.
On 10/16/23 19:03, Detlev Casanova wrote: > On Saturday, October 7, 2023 5:35:27 P.M. EDT Marek Vasut wrote: >> On 10/2/23 17:20, Detlev Casanova wrote: >>> Expose that information to the sysinfo command to let scripts make >>> decisions based on the board id and revision. >>> >>> Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com> >>> --- >>> >>> drivers/sysinfo/rcar3.c | 89 +++++++++++++++++++++++++++++------------ >>> 1 file changed, 63 insertions(+), 26 deletions(-) >>> >>> diff --git a/drivers/sysinfo/rcar3.c b/drivers/sysinfo/rcar3.c >>> index 633e80bc19b..a554323506a 100644 >>> --- a/drivers/sysinfo/rcar3.c >>> +++ b/drivers/sysinfo/rcar3.c >>> @@ -32,6 +32,10 @@ >>> >>> */ >>> >>> struct sysinfo_rcar_priv { >>> >>> char boardmodel[64]; >>> >>> + u8 id; >>> + u8 rev_major; >>> + u8 rev_minor; >>> + bool has_rev; >>> >>> u8 val; >>> >>> }; >>> >>> @@ -56,9 +60,33 @@ static int sysinfo_rcar_get_str(struct udevice *dev, >>> int id, size_t size, char *> >>> }; >>> >>> } >>> >>> +static int sysinfo_rcar_get_int(struct udevice *dev, int id, int *val) >>> +{ >>> + struct sysinfo_rcar_priv *priv = dev_get_priv(dev); >>> + >>> + switch (id) { >>> + case SYSINFO_ID_BOARD_ID: >>> + *val = priv->id; >>> + return 0; >>> + case SYSINFO_ID_BOARD_REV_MAJOR: >>> + if (!priv->has_rev) >>> + return -EINVAL; >>> + *val = priv->rev_major; >>> + return 0; >>> + case SYSINFO_ID_BOARD_REV_MINOR: >>> + if (!priv->has_rev) >>> + return -EINVAL; >>> + *val = priv->rev_minor; >>> + return 0; >>> + default: >>> + return -EINVAL; >>> + }; >>> +} >>> + >>> >>> static const struct sysinfo_ops sysinfo_rcar_ops = { >>> >>> .detect = sysinfo_rcar_detect, >>> .get_str = sysinfo_rcar_get_str, >>> >>> + .get_int = sysinfo_rcar_get_int, >>> >>> }; >>> >>> static void sysinfo_rcar_parse(struct sysinfo_rcar_priv *priv) >>> >>> @@ -69,8 +97,9 @@ static void sysinfo_rcar_parse(struct sysinfo_rcar_priv >>> *priv)> >>> bool ebisu_4d = false; >>> bool condor_i = false; >>> char rev[4] = "?.?"; >>> >>> - u8 rev_major = 0; >>> - u8 rev_minor = 0; >>> + >>> + priv->id = board_id; >>> + priv->has_rev = false; >> >> Would it make more sense to assign >> >> priv->rev_major = 1; >> priv->rev_minor = 0; >> >> And get rid of priv->has_rev entirely ? >> >> Basically say that the default case is rev. 1.0 board. >> >> [...] > > I'm not really sure about this has it doesn't differentiate between rev 1.0 and > unknown revision. Good point, please document this in the commit message.
diff --git a/drivers/sysinfo/rcar3.c b/drivers/sysinfo/rcar3.c index 633e80bc19b..a554323506a 100644 --- a/drivers/sysinfo/rcar3.c +++ b/drivers/sysinfo/rcar3.c @@ -32,6 +32,10 @@ */ struct sysinfo_rcar_priv { char boardmodel[64]; + u8 id; + u8 rev_major; + u8 rev_minor; + bool has_rev; u8 val; }; @@ -56,9 +60,33 @@ static int sysinfo_rcar_get_str(struct udevice *dev, int id, size_t size, char * }; } +static int sysinfo_rcar_get_int(struct udevice *dev, int id, int *val) +{ + struct sysinfo_rcar_priv *priv = dev_get_priv(dev); + + switch (id) { + case SYSINFO_ID_BOARD_ID: + *val = priv->id; + return 0; + case SYSINFO_ID_BOARD_REV_MAJOR: + if (!priv->has_rev) + return -EINVAL; + *val = priv->rev_major; + return 0; + case SYSINFO_ID_BOARD_REV_MINOR: + if (!priv->has_rev) + return -EINVAL; + *val = priv->rev_minor; + return 0; + default: + return -EINVAL; + }; +} + static const struct sysinfo_ops sysinfo_rcar_ops = { .detect = sysinfo_rcar_detect, .get_str = sysinfo_rcar_get_str, + .get_int = sysinfo_rcar_get_int, }; static void sysinfo_rcar_parse(struct sysinfo_rcar_priv *priv) @@ -69,8 +97,9 @@ static void sysinfo_rcar_parse(struct sysinfo_rcar_priv *priv) bool ebisu_4d = false; bool condor_i = false; char rev[4] = "?.?"; - u8 rev_major = 0; - u8 rev_minor = 0; + + priv->id = board_id; + priv->has_rev = false; switch (board_id) { case BOARD_SALVATOR_XS: @@ -78,9 +107,10 @@ static void sysinfo_rcar_parse(struct sysinfo_rcar_priv *priv) fallthrough; case BOARD_SALVATOR_X: if (!(board_rev & ~1)) { /* Only rev 0 and 1 is valid */ - rev_major = 1; - rev_minor = board_rev & BIT(0); - snprintf(rev, sizeof(rev), "%u.%u", rev_major, rev_minor); + priv->rev_major = 1; + priv->rev_minor = board_rev & BIT(0); + priv->has_rev = true; + snprintf(rev, sizeof(rev), "%u.%u", priv->rev_major, priv->rev_minor); } snprintf(priv->boardmodel, sizeof(priv->boardmodel), "Renesas Salvator-X%s board rev %s", @@ -89,27 +119,30 @@ static void sysinfo_rcar_parse(struct sysinfo_rcar_priv *priv) return; case BOARD_STARTER_KIT: if (!(board_rev & ~1)) { /* Only rev 0 and 1 is valid */ - rev_major = (board_rev & BIT(0)) ? 3 : 1; - rev_minor = 0; - snprintf(rev, sizeof(rev), "%u.%u", rev_major, rev_minor); + priv->rev_major = (board_rev & BIT(0)) ? 3 : 1; + priv->rev_minor = 0; + snprintf(rev, sizeof(rev), "%u.%u", priv->rev_major, priv->rev_minor); + priv->has_rev = true; } snprintf(priv->boardmodel, sizeof(priv->boardmodel), "Renesas Starter Kit board rev %s", rev); return; case BOARD_STARTER_KIT_PRE: if (!(board_rev & ~3)) { /* Only rev 0..3 is valid */ - rev_major = (board_rev & BIT(1)) ? 2 : 1; - rev_minor = (board_rev == 3) ? 1 : 0; - snprintf(rev, sizeof(rev), "%u.%u", rev_major, rev_minor); + priv->rev_major = (board_rev & BIT(1)) ? 2 : 1; + priv->rev_minor = (board_rev == 3) ? 1 : 0; + snprintf(rev, sizeof(rev), "%u.%u", priv->rev_major, priv->rev_minor); + priv->has_rev = true; } snprintf(priv->boardmodel, sizeof(priv->boardmodel), "Renesas Starter Kit Premier board rev %s", rev); return; case BOARD_EAGLE: if (!board_rev) { /* Only rev 0 is valid */ - rev_major = 1; - rev_minor = 0; - snprintf(rev, sizeof(rev), "%u.%u", rev_major, rev_minor); + priv->rev_major = 1; + priv->rev_minor = 0; + snprintf(rev, sizeof(rev), "%u.%u", priv->rev_major, priv->rev_minor); + priv->has_rev = true; } snprintf(priv->boardmodel, sizeof(priv->boardmodel), "Renesas Eagle board rev %s", rev); @@ -119,9 +152,10 @@ static void sysinfo_rcar_parse(struct sysinfo_rcar_priv *priv) fallthrough; case BOARD_EBISU: if (!board_rev) { /* Only rev 0 is valid */ - rev_major = 1; - rev_minor = 0; - snprintf(rev, sizeof(rev), "%u.%u", rev_major, rev_minor); + priv->rev_major = 1; + priv->rev_minor = 0; + snprintf(rev, sizeof(rev), "%u.%u", priv->rev_major, priv->rev_minor); + priv->has_rev = true; } snprintf(priv->boardmodel, sizeof(priv->boardmodel), "Renesas Ebisu%s board rev %s", @@ -129,18 +163,20 @@ static void sysinfo_rcar_parse(struct sysinfo_rcar_priv *priv) return; case BOARD_DRAAK: if (!board_rev) { /* Only rev 0 is valid */ - rev_major = 1; - rev_minor = 0; - snprintf(rev, sizeof(rev), "%u.%u", rev_major, rev_minor); + priv->rev_major = 1; + priv->rev_minor = 0; + snprintf(rev, sizeof(rev), "%u.%u", priv->rev_major, priv->rev_minor); + priv->has_rev = true; } snprintf(priv->boardmodel, sizeof(priv->boardmodel), "Renesas Draak board rev %s", rev); return; case BOARD_KRIEK: if (!board_rev) { /* Only rev 0 is valid */ - rev_major = 1; - rev_minor = 0; - snprintf(rev, sizeof(rev), "%u.%u", rev_major, rev_minor); + priv->rev_major = 1; + priv->rev_minor = 0; + snprintf(rev, sizeof(rev), "%u.%u", priv->rev_major, priv->rev_minor); + priv->has_rev = true; } snprintf(priv->boardmodel, sizeof(priv->boardmodel), "Renesas Kriek board rev %s", rev); @@ -150,9 +186,10 @@ static void sysinfo_rcar_parse(struct sysinfo_rcar_priv *priv) fallthrough; case BOARD_CONDOR: if (!board_rev) { /* Only rev 0 is valid */ - rev_major = 1; - rev_minor = 0; - snprintf(rev, sizeof(rev), "%u.%u", rev_major, rev_minor); + priv->rev_major = 1; + priv->rev_minor = 0; + snprintf(rev, sizeof(rev), "%u.%u", priv->rev_major, priv->rev_minor); + priv->has_rev = true; } snprintf(priv->boardmodel, sizeof(priv->boardmodel), "Renesas Condor%s board rev %s",
Expose that information to the sysinfo command to let scripts make decisions based on the board id and revision. Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com> --- drivers/sysinfo/rcar3.c | 89 +++++++++++++++++++++++++++++------------ 1 file changed, 63 insertions(+), 26 deletions(-)