[PATCH 0/3] iio: add labels with accel-location to bmc150 and kxcjk-1013 drivers

[PATCH 0/3] iio: add labels with accel-location to bmc150 and kxcjk-1013 drivers

[Hans de Goede]

Hi All,

Here is a patch-set implementing the standardized "accel-display"
and "accel-base" label sysfs-attributes defined in my earlier
"[PATCH 1/2] iio: documentation: Document proximity sensor label use"
"[PATCH 2/2] iio: documentation: Document accelerometer label use"
series.

This patch sets adds these labels to the bmc150 and kxcjk-1013 accel
drivers.

Regards,

Hans


Hans de Goede (3):
  iio: core: Allow drivers to specify a label without it coming from of
  iio: accel: bmc150: Set label based on accel-location on 2-accel
    yoga-style 2-in-1s
  iio: accel: kxcjk-1013: Set label based on accel-location on 2-accel
    yoga-style 2-in-1s

 drivers/iio/accel/bmc150-accel-core.c | 18 ++++++++++++------
 drivers/iio/accel/kxcjk-1013.c        | 14 ++++++++++----
 drivers/iio/industrialio-core.c       |  6 ++++--
 3 files changed, 26 insertions(+), 12 deletions(-)

-- 
2.30.0

[PATCH 1/3] iio: core: Allow drivers to specify a label without it coming from of

[Hans de Goede]

Only set indio_dev->label from of/dt if there actually is a label
specified in of.

This allows drivers to set a label without this being overwritten with
NULL when there is no label specified in of. This is esp. useful on
devices where of is not used at all, such as your typical x86/ACPI device.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/iio/industrialio-core.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index e9ee9363fed0..b409e076818b 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -1755,6 +1755,7 @@ static const struct iio_buffer_setup_ops noop_ring_setup_ops;
 
 int __iio_device_register(struct iio_dev *indio_dev, struct module *this_mod)
 {
+	const char *label;
 	int ret;
 
 	if (!indio_dev->info)
@@ -1765,8 +1766,9 @@ int __iio_device_register(struct iio_dev *indio_dev, struct module *this_mod)
 	if (!indio_dev->dev.of_node && indio_dev->dev.parent)
 		indio_dev->dev.of_node = indio_dev->dev.parent->of_node;
 
-	indio_dev->label = of_get_property(indio_dev->dev.of_node, "label",
-					   NULL);
+	label = of_get_property(indio_dev->dev.of_node, "label", NULL);
+	if (label)
+		indio_dev->label = label;
 
 	ret = iio_check_unique_scan_index(indio_dev);
 	if (ret < 0)
-- 
2.30.0

Re: [PATCH 1/3] iio: core: Allow drivers to specify a label without it coming from of

[Alexandru Ardelean]

On Sun, Feb 7, 2021 at 6:11 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Only set indio_dev->label from of/dt if there actually is a label
> specified in of.
>
> This allows drivers to set a label without this being overwritten with
> NULL when there is no label specified in of. This is esp. useful on
> devices where of is not used at all, such as your typical x86/ACPI device.

Reviewed-by: Alexandru Ardelean <alexandru.ardelean@analog.com>


>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/iio/industrialio-core.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index e9ee9363fed0..b409e076818b 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -1755,6 +1755,7 @@ static const struct iio_buffer_setup_ops noop_ring_setup_ops;
>
>  int __iio_device_register(struct iio_dev *indio_dev, struct module *this_mod)
>  {
> +       const char *label;
>         int ret;
>
>         if (!indio_dev->info)
> @@ -1765,8 +1766,9 @@ int __iio_device_register(struct iio_dev *indio_dev, struct module *this_mod)
>         if (!indio_dev->dev.of_node && indio_dev->dev.parent)
>                 indio_dev->dev.of_node = indio_dev->dev.parent->of_node;
>
> -       indio_dev->label = of_get_property(indio_dev->dev.of_node, "label",
> -                                          NULL);
> +       label = of_get_property(indio_dev->dev.of_node, "label", NULL);
> +       if (label)
> +               indio_dev->label = label;
>
>         ret = iio_check_unique_scan_index(indio_dev);
>         if (ret < 0)
> --
> 2.30.0
>

[PATCH 2/3] iio: accel: bmc150: Set label based on accel-location on 2-accel yoga-style 2-in-1s

[Hans de Goede]

Some 2-in-1 laptops / convertibles with 360° (yoga-style) hinges,
use 2 bmc150 accelerometers, defined by a single BOSC0200 ACPI
device node (1 in their base and 1 in their display).

Since in this case we know the location of each accelerometer,
set the label for the accelerometers to the standardized
"accel-display" resp. "accel-base" labels. This way userspace
can use the labels to get the location.

This was tested on a Lenovo ThinkPad Yoga 11e 4th gen (N3450 CPU).

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/iio/accel/bmc150-accel-core.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/accel/bmc150-accel-core.c b/drivers/iio/accel/bmc150-accel-core.c
index 7e425ebcd7ea..b0dbd12cbf42 100644
--- a/drivers/iio/accel/bmc150-accel-core.c
+++ b/drivers/iio/accel/bmc150-accel-core.c
@@ -443,26 +443,32 @@ static bool bmc150_apply_acpi_orientation(struct device *dev,
 					  struct iio_mount_matrix *orientation)
 {
 	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct acpi_device *adev = ACPI_COMPANION(dev);
+	char *name, *alt_name, *label, *str;
 	union acpi_object *obj, *elements;
-	char *name, *alt_name, *str;
 	acpi_status status;
 	int i, j, val[3];
 
 	if (!adev || !acpi_dev_hid_uid_match(adev, "BOSC0200", NULL))
 		return false;
 
-	if (strcmp(dev_name(dev), "i2c-BOSC0200:base") == 0)
+	if (strcmp(dev_name(dev), "i2c-BOSC0200:base") == 0) {
 		alt_name = "ROMK";
-	else
+		label = "accel-base";
+	} else {
 		alt_name = "ROMS";
+		label = "accel-display";
+	}
 
-	if (acpi_has_method(adev->handle, "ROTM"))
+	if (acpi_has_method(adev->handle, "ROTM")) {
 		name = "ROTM";
-	else if (acpi_has_method(adev->handle, alt_name))
+	} else if (acpi_has_method(adev->handle, alt_name)) {
 		name = alt_name;
-	else
+		indio_dev->label = label;
+	} else {
 		return false;
+	}
 
 	status = acpi_evaluate_object(adev->handle, name, NULL, &buffer);
 	if (ACPI_FAILURE(status)) {
-- 
2.30.0

[PATCH 3/3] iio: accel: kxcjk-1013: Set label based on accel-location on 2-accel yoga-style 2-in-1s

[Hans de Goede]

Some 2-in-1 laptops / convertibles with 360° (yoga-style) hinges,
use 2 KXCJ91008 accelerometers:
1 in their display using an ACPI HID of "KIOX010A"; and
1 in their base    using an ACPI HID of "KIOX020A"

Since in this case we know the location of each accelerometer,
set the label for the accelerometers to the standardized
"accel-display" resp. "accel-base" labels. This way userspace
can use the labels to get the location.

This was tested on a Medion Akoya E2228T MD60250.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/iio/accel/kxcjk-1013.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
index 849eb79fc537..1e7ca666c77a 100644
--- a/drivers/iio/accel/kxcjk-1013.c
+++ b/drivers/iio/accel/kxcjk-1013.c
@@ -1331,7 +1331,8 @@ static irqreturn_t kxcjk1013_data_rdy_trig_poll(int irq, void *private)
 
 static const char *kxcjk1013_match_acpi_device(struct device *dev,
 					       enum kx_chipset *chipset,
-					       enum kx_acpi_type *acpi_type)
+					       enum kx_acpi_type *acpi_type,
+					       const char **label)
 {
 	const struct acpi_device_id *id;
 
@@ -1339,10 +1340,14 @@ static const char *kxcjk1013_match_acpi_device(struct device *dev,
 	if (!id)
 		return NULL;
 
-	if (strcmp(id->id, "SMO8500") == 0)
+	if (strcmp(id->id, "SMO8500") == 0) {
 		*acpi_type = ACPI_SMO8500;
-	else if (strcmp(id->id, "KIOX010A") == 0)
+	} else if (strcmp(id->id, "KIOX010A") == 0) {
 		*acpi_type = ACPI_KIOX010A;
+		*label = "accel-display";
+	} else if (strcmp(id->id, "KIOX020A") == 0) {
+		*label = "accel-base";
+	}
 
 	*chipset = (enum kx_chipset)id->driver_data;
 
@@ -1385,7 +1390,8 @@ static int kxcjk1013_probe(struct i2c_client *client,
 	} else if (ACPI_HANDLE(&client->dev)) {
 		name = kxcjk1013_match_acpi_device(&client->dev,
 						   &data->chipset,
-						   &data->acpi_type);
+						   &data->acpi_type,
+						   &indio_dev->label);
 	} else
 		return -ENODEV;
 
-- 
2.30.0

Re: [PATCH 0/3] iio: add labels with accel-location to bmc150 and kxcjk-1013 drivers

[Jonathan Cameron]

On Sun,  7 Feb 2021 17:08:58 +0100
Hans de Goede <hdegoede@redhat.com> wrote:

> Hi All,
> 
> Here is a patch-set implementing the standardized "accel-display"
> and "accel-base" label sysfs-attributes defined in my earlier
> "[PATCH 1/2] iio: documentation: Document proximity sensor label use"
> "[PATCH 2/2] iio: documentation: Document accelerometer label use"
> series.
> 
> This patch sets adds these labels to the bmc150 and kxcjk-1013 accel
> drivers.
Series applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to poke at it.

Note we are almost certainly too late for coming merge window so this
will be next time around.

Thanks,

Jonathan

> 
> Regards,
> 
> Hans
> 
> 
> Hans de Goede (3):
>   iio: core: Allow drivers to specify a label without it coming from of
>   iio: accel: bmc150: Set label based on accel-location on 2-accel
>     yoga-style 2-in-1s
>   iio: accel: kxcjk-1013: Set label based on accel-location on 2-accel
>     yoga-style 2-in-1s
> 
>  drivers/iio/accel/bmc150-accel-core.c | 18 ++++++++++++------
>  drivers/iio/accel/kxcjk-1013.c        | 14 ++++++++++----
>  drivers/iio/industrialio-core.c       |  6 ++++--
>  3 files changed, 26 insertions(+), 12 deletions(-)
> 

Re: [PATCH 0/3] iio: add labels with accel-location to bmc150 and kxcjk-1013 drivers

[Hans de Goede]

Hi,

On 2/12/21 7:33 PM, Jonathan Cameron wrote:
> On Sun,  7 Feb 2021 17:08:58 +0100
> Hans de Goede <hdegoede@redhat.com> wrote:
> 
>> Hi All,
>>
>> Here is a patch-set implementing the standardized "accel-display"
>> and "accel-base" label sysfs-attributes defined in my earlier
>> "[PATCH 1/2] iio: documentation: Document proximity sensor label use"
>> "[PATCH 2/2] iio: documentation: Document accelerometer label use"
>> series.
>>
>> This patch sets adds these labels to the bmc150 and kxcjk-1013 accel
>> drivers.
> Series applied to the togreg branch of iio.git and pushed out as testing
> for the autobuilders to poke at it.

Thanks, much appreciated.

But this was sorta a follow-up to:

[PATCH 1/2] iio: documentation: Document proximity sensor label use
[PATCH 2/2] iio: documentation: Document accelerometer label use

Where 2/2 defines the standardized label values which this series uses
and AFAIK those have not been merged yet?

I guess we can always fix the labels if discussion surrounding those
leads to standardizing on different label contents for this. But if
the discussion surrounding those results in the conclusion to not
use labels for this at all, while we already have this merged, then
we probably need to revert 2/3 and 3/3 of this series...

But maybe I just missed the 2 documentation patches getting merged ?

> Note we are almost certainly too late for coming merge window so this
> will be next time around.

That is fine.

Regards,

Hans

Re: [PATCH 0/3] iio: add labels with accel-location to bmc150 and kxcjk-1013 drivers

[Jonathan Cameron]

On Fri, 12 Feb 2021 19:39:11 +0100
Hans de Goede <hdegoede@redhat.com> wrote:

> Hi,
> 
> On 2/12/21 7:33 PM, Jonathan Cameron wrote:
> > On Sun,  7 Feb 2021 17:08:58 +0100
> > Hans de Goede <hdegoede@redhat.com> wrote:
> >   
> >> Hi All,
> >>
> >> Here is a patch-set implementing the standardized "accel-display"
> >> and "accel-base" label sysfs-attributes defined in my earlier
> >> "[PATCH 1/2] iio: documentation: Document proximity sensor label use"
> >> "[PATCH 2/2] iio: documentation: Document accelerometer label use"
> >> series.
> >>
> >> This patch sets adds these labels to the bmc150 and kxcjk-1013 accel
> >> drivers.  
> > Series applied to the togreg branch of iio.git and pushed out as testing
> > for the autobuilders to poke at it.  
> 
> Thanks, much appreciated.
> 
> But this was sorta a follow-up to:
> 
> [PATCH 1/2] iio: documentation: Document proximity sensor label use
> [PATCH 2/2] iio: documentation: Document accelerometer label use
> 
> Where 2/2 defines the standardized label values which this series uses
> and AFAIK those have not been merged yet?
> 
> I guess we can always fix the labels if discussion surrounding those
> leads to standardizing on different label contents for this. But if
> the discussion surrounding those results in the conclusion to not
> use labels for this at all, while we already have this merged, then
> we probably need to revert 2/3 and 3/3 of this series...
> 
> But maybe I just missed the 2 documentation patches getting merged ?

Understood.   The changes for the docs patch that I was looking for
are just editorial, so I wasn't that fussed if it goes in slightly out of
order.

Jonathan

> 
> > Note we are almost certainly too late for coming merge window so this
> > will be next time around.  
> 
> That is fine.
> 
> Regards,
> 
> Hans
>