[RFC,2/4] hdata/vpd: Parse additional VINI records

Message ID 148825888770.16559.7830135573467674875.stgit@thinktux.in.ibm.com
State Superseded
Headers show

Commit Message

Ananth N Mavinakayanahalli Feb. 28, 2017, 5:14 a.m.
While there, use the helper to add individual records.

Signed-off-by: Ananth N Mavinakayanahalli <ananth@linux.vnet.ibm.com>
---
 hdata/vpd.c |   95 ++++++++++++++++++++++++++++++-----------------------------
 1 file changed, 49 insertions(+), 46 deletions(-)

Comments

Oliver O'Halloran Feb. 28, 2017, 7:49 a.m. | #1
On Tue, Feb 28, 2017 at 4:14 PM, Ananth N Mavinakayanahalli
<ananth@linux.vnet.ibm.com> wrote:
> While there, use the helper to add individual records.
>
> Signed-off-by: Ananth N Mavinakayanahalli <ananth@linux.vnet.ibm.com>
> ---
>  hdata/vpd.c |   95 ++++++++++++++++++++++++++++++-----------------------------
>  1 file changed, 49 insertions(+), 46 deletions(-)
>
> diff --git a/hdata/vpd.c b/hdata/vpd.c
> index 4c787c6..79f2893 100644
> --- a/hdata/vpd.c
> +++ b/hdata/vpd.c
> @@ -225,54 +225,69 @@ static const struct card_info *card_info_lookup(char *ccin)
>         return NULL;
>  }
>
> +static void vpd_add_property_string(struct dt_node *n, const char *name,
> +                                   const void *vpd, unsigned int sz)
> +{
> +       char *str = zalloc(sz + 1);
> +       if (!str)
> +               return;
> +       memcpy(str, vpd, sz);
> +       dt_add_property_string(n, name, str);
> +       free(str);
> +}
> +
This looks the same as dt_add_property_nstr(). Can we remove this
helper entirely and use that instead?

>  static void vpd_vini_parse(struct dt_node *node,
>                            const void *fruvpd, unsigned int fruvpd_sz)
>  {
>         const void *kw;
>         char *str;
> -       uint8_t kwsz;
> +       uint8_t sz;
>         const struct card_info *cinfo;
>
>         /* FRU Stocking Part Number */
> -       kw = vpd_find(fruvpd, fruvpd_sz, "VINI", "FN", &kwsz);
> -       if (kw) {
> -               str = zalloc(kwsz + 1);
> -               if (!str)
> -                       goto no_memory;
> -               memcpy(str, kw, kwsz);
> -               dt_add_property_string(node, "fru-number", str);
> -               free(str);
> -       }
> +       kw = vpd_find(fruvpd, fruvpd_sz, "VINI", "FN", &sz);
> +       if (kw)
> +               vpd_add_property_string(node, "fru-number", kw, sz);
>
>         /* Serial Number */
> -       kw = vpd_find(fruvpd, fruvpd_sz, "VINI", "SN", &kwsz);
> -       if (kw) {
> -               str = zalloc(kwsz + 1);
> -               if (!str)
> -                       goto no_memory;
> -               memcpy(str, kw, kwsz);
> -               dt_add_property_string(node, "serial-number", str);
> -               free(str);
> -       }
> +       kw = vpd_find(fruvpd, fruvpd_sz, "VINI", "SN", &sz);
> +       if (kw)
> +               vpd_add_property_string(node, "serial-number", kw, sz);
>
>         /* Part Number */
> -       kw = vpd_find(fruvpd, fruvpd_sz, "VINI", "PN", &kwsz);
> -       if (kw) {
> -               str = zalloc(kwsz + 1);
> -               if (!str)
> -                       goto no_memory;
> -               memcpy(str, kw, kwsz);
> -               dt_add_property_string(node, "part-number", str);
> -               free(str);
> -       }
> +       kw = vpd_find(fruvpd, fruvpd_sz, "VINI", "PN", &sz);
> +       if (kw)
> +               vpd_add_property_string(node, "part-number", kw, sz);
> +
> +       /* CCIN Extension */
> +       kw = vpd_find(fruvpd, fruvpd_sz, "VINI", "CE", &sz);
> +       if (kw)
> +               vpd_add_property_string(node, "ccin-extension", kw, sz);
> +
> +       /* HW Version info */
> +       kw = vpd_find(fruvpd, fruvpd_sz, "VINI", "HW", &sz);
> +       if (kw)
> +               vpd_add_property_string(node, "hw-version", kw, sz);
> +
> +       /* Card type info */
> +       kw = vpd_find(fruvpd, fruvpd_sz, "VINI", "CT", &sz);
> +       if (kw)
> +               vpd_add_property_string(node, "card-type", kw, sz);
> +
> +       /* HW characteristics info */
> +       kw = vpd_find(fruvpd, fruvpd_sz, "VINI", "B3", &sz);
> +       if (kw)
> +               vpd_add_property_string(node, "hw-characteristics", kw, sz);
>

>         /* Customer Card Identification Number (CCIN) */
> -       kw = vpd_find(fruvpd, fruvpd_sz, "VINI", "CC", &kwsz);
> +       kw = vpd_find(fruvpd, fruvpd_sz, "VINI", "CC", &sz);
>         if (kw) {
> -               str = zalloc(kwsz + 1);
> -               if (!str)
> -                       goto no_memory;
> -               memcpy(str, kw, kwsz);
> +               str = zalloc(sz + 1);
> +               if (!str) {
> +                       prerror("VPD: memory allocation failure in VINI parsing\n");
> +                       return;
> +               }
> +               memcpy(str, kw, sz);
>                 dt_add_property_string(node, "ccin", str);

We can probably use the helper here too if we looked up what we just
added to the DT, or if card_info_lookup() was tweaked to use
strncmp().


>                 cinfo = card_info_lookup(str);
>                 if (cinfo) {
> @@ -285,9 +300,8 @@ static void vpd_vini_parse(struct dt_node *node,
>                 }
>                 free(str);
>         }
> +
>         return;
> -no_memory:
> -       prerror("VPD: memory allocation failure in VINI parsing\n");
>  }
>
>  static bool valid_child_entry(const struct slca_entry *entry)
> @@ -486,17 +500,6 @@ def_model:
>         dt_add_property_string(dt_root, "model-name", model_name);
>  }
>
> -static void vpd_add_property_string(struct dt_node *n, const char *name,
> -                                   const void *vpd, unsigned int sz)
> -{
> -       char *str = zalloc(sz + 1);
> -       if (!str)
> -               return;
> -       memcpy(str, vpd, sz);
> -       dt_add_property_string(n, name, str);
> -       free(str);
> -}
> -
>  static void sysvpd_parse_opp(const void *sysvpd, unsigned int sysvpd_sz)
>  {
>         const char *v;
>
> _______________________________________________
> Skiboot mailing list
> Skiboot@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/skiboot
Ananth N Mavinakayanahalli Feb. 28, 2017, 8:09 a.m. | #2
On Tue, Feb 28, 2017 at 06:49:46PM +1100, Oliver O'Halloran wrote:
> On Tue, Feb 28, 2017 at 4:14 PM, Ananth N Mavinakayanahalli
> <ananth@linux.vnet.ibm.com> wrote:
> > While there, use the helper to add individual records.
> >
> > Signed-off-by: Ananth N Mavinakayanahalli <ananth@linux.vnet.ibm.com>
> > ---
> >  hdata/vpd.c |   95 ++++++++++++++++++++++++++++++-----------------------------
> >  1 file changed, 49 insertions(+), 46 deletions(-)
> >
> > diff --git a/hdata/vpd.c b/hdata/vpd.c
> > index 4c787c6..79f2893 100644
> > --- a/hdata/vpd.c
> > +++ b/hdata/vpd.c
> > @@ -225,54 +225,69 @@ static const struct card_info *card_info_lookup(char *ccin)
> >         return NULL;
> >  }
> >
> > +static void vpd_add_property_string(struct dt_node *n, const char *name,
> > +                                   const void *vpd, unsigned int sz)
> > +{
> > +       char *str = zalloc(sz + 1);
> > +       if (!str)
> > +               return;
> > +       memcpy(str, vpd, sz);
> > +       dt_add_property_string(n, name, str);
> > +       free(str);
> > +}
> > +
> This looks the same as dt_add_property_nstr(). Can we remove this
> helper entirely and use that instead?

yup.. good point.

...

> >         /* Customer Card Identification Number (CCIN) */
> > -       kw = vpd_find(fruvpd, fruvpd_sz, "VINI", "CC", &kwsz);
> > +       kw = vpd_find(fruvpd, fruvpd_sz, "VINI", "CC", &sz);
> >         if (kw) {
> > -               str = zalloc(kwsz + 1);
> > -               if (!str)
> > -                       goto no_memory;
> > -               memcpy(str, kw, kwsz);
> > +               str = zalloc(sz + 1);
> > +               if (!str) {
> > +                       prerror("VPD: memory allocation failure in VINI parsing\n");
> > +                       return;
> > +               }
> > +               memcpy(str, kw, sz);
> >                 dt_add_property_string(node, "ccin", str);
> 
> We can probably use the helper here too if we looked up what we just
> added to the DT, or if card_info_lookup() was tweaked to use
> strncmp().

agreed..

Ananth

Patch

diff --git a/hdata/vpd.c b/hdata/vpd.c
index 4c787c6..79f2893 100644
--- a/hdata/vpd.c
+++ b/hdata/vpd.c
@@ -225,54 +225,69 @@  static const struct card_info *card_info_lookup(char *ccin)
 	return NULL;
 }
 
+static void vpd_add_property_string(struct dt_node *n, const char *name,
+				    const void *vpd, unsigned int sz)
+{
+	char *str = zalloc(sz + 1);
+	if (!str)
+		return;
+	memcpy(str, vpd, sz);
+	dt_add_property_string(n, name, str);
+	free(str);
+}
+
 static void vpd_vini_parse(struct dt_node *node,
 			   const void *fruvpd, unsigned int fruvpd_sz)
 {
 	const void *kw;
 	char *str;
-	uint8_t kwsz;
+	uint8_t sz;
 	const struct card_info *cinfo;
 
 	/* FRU Stocking Part Number */
-	kw = vpd_find(fruvpd, fruvpd_sz, "VINI", "FN", &kwsz);
-	if (kw) {
-		str = zalloc(kwsz + 1);
-		if (!str)
-			goto no_memory;
-		memcpy(str, kw, kwsz);
-		dt_add_property_string(node, "fru-number", str);
-		free(str);
-	}
+	kw = vpd_find(fruvpd, fruvpd_sz, "VINI", "FN", &sz);
+	if (kw)
+		vpd_add_property_string(node, "fru-number", kw, sz);
 
 	/* Serial Number */
-	kw = vpd_find(fruvpd, fruvpd_sz, "VINI", "SN", &kwsz);
-	if (kw) {
-		str = zalloc(kwsz + 1);
-		if (!str)
-			goto no_memory;
-		memcpy(str, kw, kwsz);
-		dt_add_property_string(node, "serial-number", str);
-		free(str);
-	}
+	kw = vpd_find(fruvpd, fruvpd_sz, "VINI", "SN", &sz);
+	if (kw)
+		vpd_add_property_string(node, "serial-number", kw, sz);
 
 	/* Part Number */
-	kw = vpd_find(fruvpd, fruvpd_sz, "VINI", "PN", &kwsz);
-	if (kw) {
-		str = zalloc(kwsz + 1);
-		if (!str)
-			goto no_memory;
-		memcpy(str, kw, kwsz);
-		dt_add_property_string(node, "part-number", str);
-		free(str);
-	}
+	kw = vpd_find(fruvpd, fruvpd_sz, "VINI", "PN", &sz);
+	if (kw)
+		vpd_add_property_string(node, "part-number", kw, sz);
+
+	/* CCIN Extension */
+	kw = vpd_find(fruvpd, fruvpd_sz, "VINI", "CE", &sz);
+	if (kw)
+		vpd_add_property_string(node, "ccin-extension", kw, sz);
+
+	/* HW Version info */
+	kw = vpd_find(fruvpd, fruvpd_sz, "VINI", "HW", &sz);
+	if (kw)
+		vpd_add_property_string(node, "hw-version", kw, sz);
+
+	/* Card type info */
+	kw = vpd_find(fruvpd, fruvpd_sz, "VINI", "CT", &sz);
+	if (kw)
+		vpd_add_property_string(node, "card-type", kw, sz);
+
+	/* HW characteristics info */
+	kw = vpd_find(fruvpd, fruvpd_sz, "VINI", "B3", &sz);
+	if (kw)
+		vpd_add_property_string(node, "hw-characteristics", kw, sz);
 
 	/* Customer Card Identification Number (CCIN) */
-	kw = vpd_find(fruvpd, fruvpd_sz, "VINI", "CC", &kwsz);
+	kw = vpd_find(fruvpd, fruvpd_sz, "VINI", "CC", &sz);
 	if (kw) {
-		str = zalloc(kwsz + 1);
-		if (!str)
-			goto no_memory;
-		memcpy(str, kw, kwsz);
+		str = zalloc(sz + 1);
+		if (!str) {
+			prerror("VPD: memory allocation failure in VINI parsing\n");
+			return;
+		}
+		memcpy(str, kw, sz);
 		dt_add_property_string(node, "ccin", str);
 		cinfo = card_info_lookup(str);
 		if (cinfo) {
@@ -285,9 +300,8 @@  static void vpd_vini_parse(struct dt_node *node,
 		}
 		free(str);
 	}
+
 	return;
-no_memory:
-	prerror("VPD: memory allocation failure in VINI parsing\n");
 }
 
 static bool valid_child_entry(const struct slca_entry *entry)
@@ -486,17 +500,6 @@  def_model:
 	dt_add_property_string(dt_root, "model-name", model_name);
 }
 
-static void vpd_add_property_string(struct dt_node *n, const char *name,
-				    const void *vpd, unsigned int sz)
-{
-	char *str = zalloc(sz + 1);
-	if (!str)
-		return;
-	memcpy(str, vpd, sz);
-	dt_add_property_string(n, name, str);
-	free(str);
-}
-
 static void sysvpd_parse_opp(const void *sysvpd, unsigned int sysvpd_sz)
 {
 	const char *v;