[v2,4/5] hdata/i2c: ignore the secure window and physical presence i2c devs

Message ID 1504163557-14432-5-git-send-email-cclaudio@linux.vnet.ibm.com
State Changes Requested
Headers show
Series
  • hdata/i2c: add support to i2c array version 2 in P9
Related show

Commit Message

Claudio Carvalho Aug. 31, 2017, 7:12 a.m.
This ignores the secure window and physical presence i2c devices since
they are failing the HDAT parsing and they won't be used for now.

In P9, the secure window and physical presence are gpio pins in an i2c
gpio expander. In the HDAT the secure window and physical presence are
duplicates of the associated i2c gpio expander device, however
distinguished by the gpio port. That causes a failure when dt_new_addr()
is called because skiboot is not able to distinguish nodes by port.

Signed-off-by: Claudio Carvalho <cclaudio@linux.vnet.ibm.com>
---
 hdata/i2c.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

Comments

Oliver Sept. 1, 2017, 4:14 a.m. | #1
On Thu, Aug 31, 2017 at 5:12 PM, Claudio Carvalho
<cclaudio@linux.vnet.ibm.com> wrote:
> This ignores the secure window and physical presence i2c devices since
> they are failing the HDAT parsing and they won't be used for now.
>
> In P9, the secure window and physical presence are gpio pins in an i2c
> gpio expander. In the HDAT the secure window and physical presence are
> duplicates of the associated i2c gpio expander device, however
> distinguished by the gpio port. That causes a failure when dt_new_addr()
> is called because skiboot is not able to distinguish nodes by port.
>
> Signed-off-by: Claudio Carvalho <cclaudio@linux.vnet.ibm.com>
> ---
>  hdata/i2c.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/hdata/i2c.c b/hdata/i2c.c
> index 14650b6..8a2e392 100644
> --- a/hdata/i2c.c
> +++ b/hdata/i2c.c
> @@ -259,6 +259,20 @@ int parse_i2c_devs(const struct HDIF_common_hdr *hdr, int idata_index,
>                         compat = NULL;
>                 }
>
> +               /*
> +                * TODO: Create devtree entries for the secure window and
> +                * physical presence. In the hdat they are i2c gpio-expander
> +                * devices distinguished only by the port property.
> +                */
> +               if (be32_to_cpu(dev->purpose) == 0xd ||
> +                   be32_to_cpu(dev->purpose) == 0xe) {
> +                       prlog(PR_INFO, "HDAT I2C: ignoring e%dp%d - %s@%x "
> +                             "(port 0x%x) - %s\n", dev->i2cm_engine,
> +                             dev->i2cm_port, name, i2c_addr, dev->i2c_port,
> +                             label);
> +                       continue;
> +               }
> +
>                 node = dt_new_addr(bus, name, i2c_addr);
>                 if (!node)
>                         continue;
> --
> 2.7.4
>

NAK

You can use dt_find_by_name_addr() to check if that device node exists
and keep going. We always want the I2C device node to exists so that
we know the device exists. For now we can just ignore the port, but we
will need to work out how to use that information at some point in the
future.

Oliver
Claudio Carvalho Sept. 4, 2017, 3:04 p.m. | #2
On 01/09/2017 01:14, Oliver wrote:
> On Thu, Aug 31, 2017 at 5:12 PM, Claudio Carvalho
> <cclaudio@linux.vnet.ibm.com> wrote:
>> This ignores the secure window and physical presence i2c devices since
>> they are failing the HDAT parsing and they won't be used for now.
>>
>> In P9, the secure window and physical presence are gpio pins in an i2c
>> gpio expander. In the HDAT the secure window and physical presence are
>> duplicates of the associated i2c gpio expander device, however
>> distinguished by the gpio port. That causes a failure when dt_new_addr()
>> is called because skiboot is not able to distinguish nodes by port.
>>
>> Signed-off-by: Claudio Carvalho <cclaudio@linux.vnet.ibm.com>
>> ---
>>   hdata/i2c.c | 14 ++++++++++++++
>>   1 file changed, 14 insertions(+)
>>
>> diff --git a/hdata/i2c.c b/hdata/i2c.c
>> index 14650b6..8a2e392 100644
>> --- a/hdata/i2c.c
>> +++ b/hdata/i2c.c
>> @@ -259,6 +259,20 @@ int parse_i2c_devs(const struct HDIF_common_hdr *hdr, int idata_index,
>>                          compat = NULL;
>>                  }
>>
>> +               /*
>> +                * TODO: Create devtree entries for the secure window and
>> +                * physical presence. In the hdat they are i2c gpio-expander
>> +                * devices distinguished only by the port property.
>> +                */
>> +               if (be32_to_cpu(dev->purpose) == 0xd ||
>> +                   be32_to_cpu(dev->purpose) == 0xe) {
>> +                       prlog(PR_INFO, "HDAT I2C: ignoring e%dp%d - %s@%x "
>> +                             "(port 0x%x) - %s\n", dev->i2cm_engine,
>> +                             dev->i2cm_port, name, i2c_addr, dev->i2c_port,
>> +                             label);
>> +                       continue;
>> +               }
>> +
>>                  node = dt_new_addr(bus, name, i2c_addr);
>>                  if (!node)
>>                          continue;
>> --
>> 2.7.4
>>
> NAK
>
> You can use dt_find_by_name_addr() to check if that device node exists
> and keep going. We always want the I2C device node to exists so that
> we know the device exists. For now we can just ignore the port, but we
> will need to work out how to use that information at some point in the
> future.
>
> Oliver
>

Right. I will change that. Thanks.

Claudio

Patch

diff --git a/hdata/i2c.c b/hdata/i2c.c
index 14650b6..8a2e392 100644
--- a/hdata/i2c.c
+++ b/hdata/i2c.c
@@ -259,6 +259,20 @@  int parse_i2c_devs(const struct HDIF_common_hdr *hdr, int idata_index,
 			compat = NULL;
 		}
 
+		/*
+		 * TODO: Create devtree entries for the secure window and
+		 * physical presence. In the hdat they are i2c gpio-expander
+		 * devices distinguished only by the port property.
+		 */
+		if (be32_to_cpu(dev->purpose) == 0xd ||
+		    be32_to_cpu(dev->purpose) == 0xe) {
+			prlog(PR_INFO, "HDAT I2C: ignoring e%dp%d - %s@%x "
+			      "(port 0x%x) - %s\n", dev->i2cm_engine,
+			      dev->i2cm_port, name, i2c_addr, dev->i2c_port,
+			      label);
+			continue;
+		}
+
 		node = dt_new_addr(bus, name, i2c_addr);
 		if (!node)
 			continue;