| Message ID | 20170524080912.19870-1-oohall@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
On 05/24/2017 01:39 PM, Oliver O'Halloran wrote: > Currently this is not populated on FSP machines which causes some > obnoxious errors to appear in the boot log. We also only want to > parse version 1 of this structure since future versions will completely Latest HDAT spec says 0x02 for P9. > change the array item format. > > Cc: Vasant Hegde <hegdevasant@linux.vnet.ibm.com> > Signed-off-by: Oliver O'Halloran <oohall@gmail.com> > --- > hdata/i2c.c | 33 +++++++++++++++++++++++++++++++++ > 1 file changed, 33 insertions(+) > > diff --git a/hdata/i2c.c b/hdata/i2c.c > index f5c5bafe758b..4c3f920e2fda 100644 > --- a/hdata/i2c.c > +++ b/hdata/i2c.c > @@ -149,6 +149,11 @@ static bool is_zeros(const void *p, size_t size) > return true; > } > > +struct host_i2c_hdr { > + const struct HDIF_array_hdr hdr; > + __be32 version; May be we should define macro for each version instead of hardcoding below. -Vasant
On Thu, May 25, 2017 at 4:46 PM, Vasant Hegde <hegdevasant@linux.vnet.ibm.com> wrote: > On 05/24/2017 01:39 PM, Oliver O'Halloran wrote: >> >> Currently this is not populated on FSP machines which causes some >> obnoxious errors to appear in the boot log. We also only want to >> parse version 1 of this structure since future versions will completely > > > Latest HDAT spec says 0x02 for P9. The latest specs (I think it changed in j) changed the format, but existing hostboot still provide the v1 format. I don't think there are any immediate plans to start using the v2 format for now so we need to tolerate getting either. I've been asking for some change to the v2 format so I don't want to support it yet, but I think skiboot needs to cleanly handle the unsupported format. >> change the array item format. >> >> Cc: Vasant Hegde <hegdevasant@linux.vnet.ibm.com> >> Signed-off-by: Oliver O'Halloran <oohall@gmail.com> >> --- >> hdata/i2c.c | 33 +++++++++++++++++++++++++++++++++ >> 1 file changed, 33 insertions(+) >> >> diff --git a/hdata/i2c.c b/hdata/i2c.c >> index f5c5bafe758b..4c3f920e2fda 100644 >> --- a/hdata/i2c.c >> +++ b/hdata/i2c.c >> @@ -149,6 +149,11 @@ static bool is_zeros(const void *p, size_t size) >> return true; >> } >> >> +struct host_i2c_hdr { >> + const struct HDIF_array_hdr hdr; >> + __be32 version; > > > May be we should define macro for each version instead of hardcoding below. I don't really see the point. It's only ever going to be 1, 2, 3, etc > > -Vasant >
Oliver O'Halloran <oohall@gmail.com> writes: > Currently this is not populated on FSP machines which causes some > obnoxious errors to appear in the boot log. We also only want to > parse version 1 of this structure since future versions will completely > change the array item format. > > Cc: Vasant Hegde <hegdevasant@linux.vnet.ibm.com> > Signed-off-by: Oliver O'Halloran <oohall@gmail.com> Merged to master as of 41dc3eb4495c451a405974570f604622a3f829ef
diff --git a/hdata/i2c.c b/hdata/i2c.c index f5c5bafe758b..4c3f920e2fda 100644 --- a/hdata/i2c.c +++ b/hdata/i2c.c @@ -149,6 +149,11 @@ static bool is_zeros(const void *p, size_t size) return true; } +struct host_i2c_hdr { + const struct HDIF_array_hdr hdr; + __be32 version; +}; + int parse_i2c_devs(const struct HDIF_common_hdr *hdr, int idata_index, struct dt_node *xscom) { @@ -156,7 +161,9 @@ int parse_i2c_devs(const struct HDIF_common_hdr *hdr, int idata_index, const struct hdat_i2c_type *type; const struct i2c_dev *dev; const char *label, *name, *compat; + const struct host_i2c_hdr *ahdr; uint32_t i2c_addr; + uint32_t version; uint32_t size; int i, count; @@ -166,6 +173,32 @@ int parse_i2c_devs(const struct HDIF_common_hdr *hdr, int idata_index, */ assert(proc_gen == proc_gen_p9); + /* + * Emit an error if we get a newer version. This is an interim measure + * until the new version format is finalised. + */ + ahdr = HDIF_get_idata(hdr, idata_index, &size); + if (!ahdr || !size) + return 1; + + /* + * Some hostboots don't correctly fill the version field. On these + * the offset from the start of the header to the start of the array + * is 16 bytes. + */ + if (be32_to_cpu(ahdr->hdr.offset) == 16) { + version = 1; + prerror("I2C: HDAT device array has no version! Assuming v1\n"); + } else { + version = be32_to_cpu(hdr->version); + } + + if (version != 1) { + prerror("I2C: HDAT version %d not supported! THIS IS A BUG\n", + version); + return 1; + } + count = HDIF_get_iarray_size(hdr, idata_index); for (i = 0; i < count; i++) { dev = HDIF_get_iarray_item(hdr, idata_index, i, &size);
Currently this is not populated on FSP machines which causes some obnoxious errors to appear in the boot log. We also only want to parse version 1 of this structure since future versions will completely change the array item format. Cc: Vasant Hegde <hegdevasant@linux.vnet.ibm.com> Signed-off-by: Oliver O'Halloran <oohall@gmail.com> --- hdata/i2c.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+)