Message ID | 1504163557-14432-6-git-send-email-cclaudio@linux.vnet.ibm.com |
---|---|
State | Rejected |
Headers | show |
Series | hdata/i2c: add support to i2c array version 2 in P9 | expand |
On Thu, Aug 31, 2017 at 5:12 PM, Claudio Carvalho <cclaudio@linux.vnet.ibm.com> wrote: > This adds the description field to the i2c array, as specified in the i2c > array version 2. > > When the description field is populated, we should be able to get the > description (name, compat and label) of new i2c devices from it, instead > of having to input these information manually in the list of known i2c > devices. > > Signed-off-by: Claudio Carvalho <cclaudio@linux.vnet.ibm.com> > --- > hdata/i2c.c | 8 +++----- > 1 file changed, 3 insertions(+), 5 deletions(-) > > diff --git a/hdata/i2c.c b/hdata/i2c.c > index 8a2e392..c365a92 100644 > --- a/hdata/i2c.c > +++ b/hdata/i2c.c > @@ -22,6 +22,7 @@ struct i2c_dev { > __be32 purpose; > __be32 i2c_link; > __be16 slca_index; > + char desc[64]; > }; > > #define P9_I2CM_XSCOM_SIZE 0x1000 > @@ -202,11 +203,8 @@ int parse_i2c_devs(const struct HDIF_common_hdr *hdr, int idata_index, > version = be32_to_cpu(ahdr->version); > } > > - if (version == 2) { > - prerror("I2C: v%d found, but not supported. Parsing as v1\n", > - version); > - } else if (version > 2) { > - prerror("I2C: v%d found, but not supported! THIS IS A BUG\n", > + if (version != 1 && version != 2) { > + prerror("I2C: HDAT version %d not supported! THIS IS A BUG\n", > version); > return -1; > } > -- > 2.7.4 > NAK I added that error in the first place since the spec for the v2 strings hadn't been finalised and I didn't want it going off into the weeds if something changed. In hindsight this was a mistake since the V2 format only extends the V1 format so parsing it as V1 is always going to work. This has (almost) always been the case with HDAT so there's no need to make this a hard error. I'd just drop this patch until we have support for parsing the description strings (and they're actually populated). Oliver
On 01/09/2017 01:17, Oliver wrote: > On Thu, Aug 31, 2017 at 5:12 PM, Claudio Carvalho > <cclaudio@linux.vnet.ibm.com> wrote: >> This adds the description field to the i2c array, as specified in the i2c >> array version 2. >> >> When the description field is populated, we should be able to get the >> description (name, compat and label) of new i2c devices from it, instead >> of having to input these information manually in the list of known i2c >> devices. >> >> Signed-off-by: Claudio Carvalho <cclaudio@linux.vnet.ibm.com> >> --- >> hdata/i2c.c | 8 +++----- >> 1 file changed, 3 insertions(+), 5 deletions(-) >> >> diff --git a/hdata/i2c.c b/hdata/i2c.c >> index 8a2e392..c365a92 100644 >> --- a/hdata/i2c.c >> +++ b/hdata/i2c.c >> @@ -22,6 +22,7 @@ struct i2c_dev { >> __be32 purpose; >> __be32 i2c_link; >> __be16 slca_index; >> + char desc[64]; >> }; >> >> #define P9_I2CM_XSCOM_SIZE 0x1000 >> @@ -202,11 +203,8 @@ int parse_i2c_devs(const struct HDIF_common_hdr *hdr, int idata_index, >> version = be32_to_cpu(ahdr->version); >> } >> >> - if (version == 2) { >> - prerror("I2C: v%d found, but not supported. Parsing as v1\n", >> - version); >> - } else if (version > 2) { >> - prerror("I2C: v%d found, but not supported! THIS IS A BUG\n", >> + if (version != 1 && version != 2) { >> + prerror("I2C: HDAT version %d not supported! THIS IS A BUG\n", >> version); >> return -1; >> } >> -- >> 2.7.4 >> > NAK > > I added that error in the first place since the spec for the v2 > strings hadn't been finalised and I didn't want it going off into the > weeds if something changed. In hindsight this was a mistake since the > V2 format only extends the V1 format so parsing it as V1 is always > going to work. This has (almost) always been the case with HDAT so > there's no need to make this a hard error. > > I'd just drop this patch until we have support for parsing the > description strings (and they're actually populated). > > Oliver > Yeah, makes sense. We can drop this patch for now.
diff --git a/hdata/i2c.c b/hdata/i2c.c index 8a2e392..c365a92 100644 --- a/hdata/i2c.c +++ b/hdata/i2c.c @@ -22,6 +22,7 @@ struct i2c_dev { __be32 purpose; __be32 i2c_link; __be16 slca_index; + char desc[64]; }; #define P9_I2CM_XSCOM_SIZE 0x1000 @@ -202,11 +203,8 @@ int parse_i2c_devs(const struct HDIF_common_hdr *hdr, int idata_index, version = be32_to_cpu(ahdr->version); } - if (version == 2) { - prerror("I2C: v%d found, but not supported. Parsing as v1\n", - version); - } else if (version > 2) { - prerror("I2C: v%d found, but not supported! THIS IS A BUG\n", + if (version != 1 && version != 2) { + prerror("I2C: HDAT version %d not supported! THIS IS A BUG\n", version); return -1; }
This adds the description field to the i2c array, as specified in the i2c array version 2. When the description field is populated, we should be able to get the description (name, compat and label) of new i2c devices from it, instead of having to input these information manually in the list of known i2c devices. Signed-off-by: Claudio Carvalho <cclaudio@linux.vnet.ibm.com> --- hdata/i2c.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)