[2/2] hdata/i2c: Add status property to I2C devices

Message ID 20180921141441.24398-2-hegdevasant@linux.vnet.ibm.com
State New
Headers show
Series
  • [1/2] hdata/i2c: Skip unknown device type
Related show

Checks

Context Check Description
snowpatch_ozlabs/make_check success Test make_check on branch master
snowpatch_ozlabs/apply_patch success master/apply_patch Successfully applied

Commit Message

Vasant Hegde Sept. 21, 2018, 2:14 p.m.
If compatible property is NULL, then Linux cannot do much with those
devices. Lets mark such device as 'reserved'.

Suggested-by: Oliver O'Halloran <oohall@gmail.com>
Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
---
 hdata/i2c.c       | 6 +++++-
 libstb/tpm_chip.c | 1 +
 2 files changed, 6 insertions(+), 1 deletion(-)

Comments

Oliver Sept. 24, 2018, 7:12 a.m. | #1
On Sat, Sep 22, 2018 at 12:14 AM, Vasant Hegde
<hegdevasant@linux.vnet.ibm.com> wrote:
> If compatible property is NULL, then Linux cannot do much with those
> devices. Lets mark such device as 'reserved'.
>
> Suggested-by: Oliver O'Halloran <oohall@gmail.com>
> Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
> ---
>  hdata/i2c.c       | 6 +++++-
>  libstb/tpm_chip.c | 1 +
>  2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/hdata/i2c.c b/hdata/i2c.c
> index 8d4403e4e..81a7695e2 100644
> --- a/hdata/i2c.c
> +++ b/hdata/i2c.c
> @@ -340,8 +340,12 @@ int parse_i2c_devs(const struct HDIF_common_hdr *hdr, int idata_index,
>                 dt_add_property_cells(node, "reg", dev_addr);
>                 dt_add_property_cells(node, "link-id",
>                         be32_to_cpu(dev->i2c_link));
> -               if (compat)
> +               if (compat) {
>                         dt_add_property_string(node, "compatible", compat);
> +                       dt_add_property_string(node, "status", "okay");
> +               } else {
> +                       dt_add_property_string(node, "status", "reserved");

Reserving anything without a compatible string doesn't necessarily do
what we want here. A lot of the devices that we get through HDAT are
owned by firmware so we don't want to pass them to the OS even if they
have a valid compatible string (e.g. a led-controller might have
nxp,pca9551). I was thinking white-listing devices based on the
purpose field we get from HDAT might be a better approach.

> +               }
>                 if (label)
>                         dt_add_property_string(node, "label", label);
>
> diff --git a/libstb/tpm_chip.c b/libstb/tpm_chip.c
> index 2858cafb1..58e5f7530 100644
> --- a/libstb/tpm_chip.c
> +++ b/libstb/tpm_chip.c
> @@ -313,6 +313,7 @@ int tpm_extendl(TPM_Pcr pcr,
>  void tpm_add_status_property(void) {
>         struct tpm_chip *tpm;
>         list_for_each(&tpm_list, tpm, link) {
> +               dt_check_del_prop(tpm->node, "status");
>                 dt_add_property_string(tpm->node, "status",
>                                        tpm->enabled ? "okay" : "disabled");
>         }
> --
> 2.14.3
>

Patch

diff --git a/hdata/i2c.c b/hdata/i2c.c
index 8d4403e4e..81a7695e2 100644
--- a/hdata/i2c.c
+++ b/hdata/i2c.c
@@ -340,8 +340,12 @@  int parse_i2c_devs(const struct HDIF_common_hdr *hdr, int idata_index,
 		dt_add_property_cells(node, "reg", dev_addr);
 		dt_add_property_cells(node, "link-id",
 			be32_to_cpu(dev->i2c_link));
-		if (compat)
+		if (compat) {
 			dt_add_property_string(node, "compatible", compat);
+			dt_add_property_string(node, "status", "okay");
+		} else {
+			dt_add_property_string(node, "status", "reserved");
+		}
 		if (label)
 			dt_add_property_string(node, "label", label);
 
diff --git a/libstb/tpm_chip.c b/libstb/tpm_chip.c
index 2858cafb1..58e5f7530 100644
--- a/libstb/tpm_chip.c
+++ b/libstb/tpm_chip.c
@@ -313,6 +313,7 @@  int tpm_extendl(TPM_Pcr pcr,
 void tpm_add_status_property(void) {
 	struct tpm_chip *tpm;
 	list_for_each(&tpm_list, tpm, link) {
+		dt_check_del_prop(tpm->node, "status");
 		dt_add_property_string(tpm->node, "status",
 				       tpm->enabled ? "okay" : "disabled");
 	}