Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1562649 > unrolled thread

[PATCH v2] iio: light: acpi-als: Properly enable on ASUS Zenbooks

Started byJosef Gajdusek <atx@atx.name>
First post2017-01-19 13:00 +0100
Last post2017-01-19 17:50 +0100
Articles 4 — 3 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH v2] iio: light: acpi-als: Properly enable on ASUS Zenbooks Josef Gajdusek <atx@atx.name> - 2017-01-19 13:00 +0100
    Re: [PATCH v2] iio: light: acpi-als: Properly enable on ASUS Zenbooks Marek Vasut <marex@denx.de> - 2017-01-19 16:50 +0100
      Re: [PATCH v2] iio: light: acpi-als: Properly enable on ASUS  Zenbooks atx@atx.name - 2017-01-19 17:30 +0100
        Re: [PATCH v2] iio: light: acpi-als: Properly enable on ASUS Zenbooks Marek Vasut <marex@denx.de> - 2017-01-19 17:50 +0100

#1562649 — [PATCH v2] iio: light: acpi-als: Properly enable on ASUS Zenbooks

FromJosef Gajdusek <atx@atx.name>
Date2017-01-19 13:00 +0100
Subject[PATCH v2] iio: light: acpi-als: Properly enable on ASUS Zenbooks
Message-ID<t1jxM-14i-5@gated-at.bofh.it>
ASUS Zenbooks need several special ACPI calls to enable the ALS peripheral.
Otherwise, reads just return 0.

Signed-off-by: Josef Gajdusek <atx@atx.name>
---
 drivers/iio/light/acpi-als.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/iio/light/acpi-als.c b/drivers/iio/light/acpi-als.c
index f0b47c5..ccd5787 100644
--- a/drivers/iio/light/acpi-als.c
+++ b/drivers/iio/light/acpi-als.c
@@ -39,6 +39,9 @@
 #define ACPI_ALS_DEVICE_NAME		"acpi-als"
 #define ACPI_ALS_NOTIFY_ILLUMINANCE	0x80
 
+#define ACPI_ALS_ASUS_TALS		"\\_SB.PCI0.LPCB.EC0.TALS"
+#define ACPI_ALS_ASUS_ALSC		"\\_SB.ATKD.ALSC"
+
 ACPI_MODULE_NAME("acpi-als");
 
 /*
@@ -170,6 +173,16 @@ static int acpi_als_read_raw(struct iio_dev *indio_dev,
 	return IIO_VAL_INT;
 }
 
+static void acpi_als_quirk_asus(struct acpi_als *als)
+{
+	acpi_execute_simple_method(NULL, ACPI_ALS_ASUS_TALS, 1);
+	acpi_execute_simple_method(NULL, ACPI_ALS_ASUS_ALSC, 1);
+}
+
+static void (*acpi_als_quirks[])(struct acpi_als *als) = {
+	acpi_als_quirk_asus,
+};
+
 static const struct iio_info acpi_als_info = {
 	.driver_module		= THIS_MODULE,
 	.read_raw		= acpi_als_read_raw,
@@ -180,6 +193,7 @@ static int acpi_als_add(struct acpi_device *device)
 	struct acpi_als *als;
 	struct iio_dev *indio_dev;
 	struct iio_buffer *buffer;
+	int i;
 
 	indio_dev = devm_iio_device_alloc(&device->dev, sizeof(*als));
 	if (!indio_dev)
@@ -191,6 +205,9 @@ static int acpi_als_add(struct acpi_device *device)
 	als->device = device;
 	mutex_init(&als->lock);
 
+	for (i = 0; i < ARRAY_SIZE(acpi_als_quirks); i++)
+		acpi_als_quirks[i](als);
+
 	indio_dev->name = ACPI_ALS_DEVICE_NAME;
 	indio_dev->dev.parent = &device->dev;
 	indio_dev->info = &acpi_als_info;
-- 
2.10.2

[toc] | [next] | [standalone]

#1562835

FromMarek Vasut <marex@denx.de>
Date2017-01-19 16:50 +0100
Message-ID<t1n8m-3mv-11@gated-at.bofh.it>
In reply to#1562649
On 01/19/2017 12:48 PM, Josef Gajdusek wrote:
> ASUS Zenbooks need several special ACPI calls to enable the ALS peripheral.
> Otherwise, reads just return 0.
> 
> Signed-off-by: Josef Gajdusek <atx@atx.name>
> ---
>  drivers/iio/light/acpi-als.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/iio/light/acpi-als.c b/drivers/iio/light/acpi-als.c
> index f0b47c5..ccd5787 100644
> --- a/drivers/iio/light/acpi-als.c
> +++ b/drivers/iio/light/acpi-als.c
> @@ -39,6 +39,9 @@
>  #define ACPI_ALS_DEVICE_NAME		"acpi-als"
>  #define ACPI_ALS_NOTIFY_ILLUMINANCE	0x80
>  
> +#define ACPI_ALS_ASUS_TALS		"\\_SB.PCI0.LPCB.EC0.TALS"
> +#define ACPI_ALS_ASUS_ALSC		"\\_SB.ATKD.ALSC"
> +
>  ACPI_MODULE_NAME("acpi-als");
>  
>  /*
> @@ -170,6 +173,16 @@ static int acpi_als_read_raw(struct iio_dev *indio_dev,
>  	return IIO_VAL_INT;
>  }
>  
> +static void acpi_als_quirk_asus(struct acpi_als *als)
> +{
> +	acpi_execute_simple_method(NULL, ACPI_ALS_ASUS_TALS, 1);
> +	acpi_execute_simple_method(NULL, ACPI_ALS_ASUS_ALSC, 1);

This will run on ALL systems, right ? Also, error checking disappeared ?

> +}
> +
> +static void (*acpi_als_quirks[])(struct acpi_als *als) = {
> +	acpi_als_quirk_asus,
> +};
> +
>  static const struct iio_info acpi_als_info = {
>  	.driver_module		= THIS_MODULE,
>  	.read_raw		= acpi_als_read_raw,
> @@ -180,6 +193,7 @@ static int acpi_als_add(struct acpi_device *device)
>  	struct acpi_als *als;
>  	struct iio_dev *indio_dev;
>  	struct iio_buffer *buffer;
> +	int i;
>  
>  	indio_dev = devm_iio_device_alloc(&device->dev, sizeof(*als));
>  	if (!indio_dev)
> @@ -191,6 +205,9 @@ static int acpi_als_add(struct acpi_device *device)
>  	als->device = device;
>  	mutex_init(&als->lock);
>  
> +	for (i = 0; i < ARRAY_SIZE(acpi_als_quirks); i++)
> +		acpi_als_quirks[i](als);
> +
>  	indio_dev->name = ACPI_ALS_DEVICE_NAME;
>  	indio_dev->dev.parent = &device->dev;
>  	indio_dev->info = &acpi_als_info;
> 


-- 
Best regards,
Marek Vasut

[toc] | [prev] | [next] | [standalone]

#1562874 — Re: [PATCH v2] iio: light: acpi-als: Properly enable on ASUS Zenbooks

Fromatx@atx.name
Date2017-01-19 17:30 +0100
SubjectRe: [PATCH v2] iio: light: acpi-als: Properly enable on ASUS Zenbooks
Message-ID<t1nL3-3PP-1@gated-at.bofh.it>
In reply to#1562835
January 19 2017 4:49 PM, "Marek Vasut" <marex@denx.de> wrote:
> On 01/19/2017 12:48 PM, Josef Gajdusek wrote:
> 
>> ASUS Zenbooks need several special ACPI calls to enable the ALS peripheral.
>> Otherwise, reads just return 0.
>> 
>> Signed-off-by: Josef Gajdusek <atx@atx.name>
>> ---
>> drivers/iio/light/acpi-als.c | 17 +++++++++++++++++
>> 1 file changed, 17 insertions(+)
>> 
>> diff --git a/drivers/iio/light/acpi-als.c b/drivers/iio/light/acpi-als.c
>> index f0b47c5..ccd5787 100644
>> --- a/drivers/iio/light/acpi-als.c
>> +++ b/drivers/iio/light/acpi-als.c
>> @@ -39,6 +39,9 @@
>> #define ACPI_ALS_DEVICE_NAME "acpi-als"
>> #define ACPI_ALS_NOTIFY_ILLUMINANCE 0x80
>> 
>> +#define ACPI_ALS_ASUS_TALS "\\_SB.PCI0.LPCB.EC0.TALS"
>> +#define ACPI_ALS_ASUS_ALSC "\\_SB.ATKD.ALSC"
>> +
>> ACPI_MODULE_NAME("acpi-als");
>> 
>> /*
>> @@ -170,6 +173,16 @@ static int acpi_als_read_raw(struct iio_dev *indio_dev,
>> return IIO_VAL_INT;
>> }
>> 
>> +static void acpi_als_quirk_asus(struct acpi_als *als)
>> +{
>> + acpi_execute_simple_method(NULL, ACPI_ALS_ASUS_TALS, 1);
>> + acpi_execute_simple_method(NULL, ACPI_ALS_ASUS_ALSC, 1);
> 
> This will run on ALL systems, right ? Also, error checking disappeared ?
> 

Yes, that's intended. Should I add some DMI checks to only run on
ASUS/ASUS Zenbooks?

As for error checking, this code behaves exactly like v1, except
that the xor warning is removed, which means that it is not needed
to explicitly get the handles. The acpi_exceute_simple_method call just
fails if it can't find the object.

Distinguishing between the "object not found" and "failed to call method" scenarios
seemed a bit overly verbose to me especially as it would probably be overkill to abort
the driver initialization because of that.

>> +}
>> +
>> +static void (*acpi_als_quirks[])(struct acpi_als *als) = {
>> + acpi_als_quirk_asus,
>> +};
>> +
>> static const struct iio_info acpi_als_info = {
>> .driver_module = THIS_MODULE,
>> .read_raw = acpi_als_read_raw,
>> @@ -180,6 +193,7 @@ static int acpi_als_add(struct acpi_device *device)
>> struct acpi_als *als;
>> struct iio_dev *indio_dev;
>> struct iio_buffer *buffer;
>> + int i;
>> 
>> indio_dev = devm_iio_device_alloc(&device->dev, sizeof(*als));
>> if (!indio_dev)
>> @@ -191,6 +205,9 @@ static int acpi_als_add(struct acpi_device *device)
>> als->device = device;
>> mutex_init(&als->lock);
>> 
>> + for (i = 0; i < ARRAY_SIZE(acpi_als_quirks); i++)
>> + acpi_als_quirks[i](als);
>> +
>> indio_dev->name = ACPI_ALS_DEVICE_NAME;
>> indio_dev->dev.parent = &device->dev;
>> indio_dev->info = &acpi_als_info;
> 
> --
> Best regards,
> Marek Vasut

[toc] | [prev] | [next] | [standalone]

#1562913

FromMarek Vasut <marex@denx.de>
Date2017-01-19 17:50 +0100
Message-ID<t1o4r-3Xc-69@gated-at.bofh.it>
In reply to#1562874
On 01/19/2017 05:19 PM, atx@atx.name wrote:
> January 19 2017 4:49 PM, "Marek Vasut" <marex@denx.de> wrote:
>> On 01/19/2017 12:48 PM, Josef Gajdusek wrote:
>>
>>> ASUS Zenbooks need several special ACPI calls to enable the ALS peripheral.
>>> Otherwise, reads just return 0.
>>>
>>> Signed-off-by: Josef Gajdusek <atx@atx.name>
>>> ---
>>> drivers/iio/light/acpi-als.c | 17 +++++++++++++++++
>>> 1 file changed, 17 insertions(+)
>>>
>>> diff --git a/drivers/iio/light/acpi-als.c b/drivers/iio/light/acpi-als.c
>>> index f0b47c5..ccd5787 100644
>>> --- a/drivers/iio/light/acpi-als.c
>>> +++ b/drivers/iio/light/acpi-als.c
>>> @@ -39,6 +39,9 @@
>>> #define ACPI_ALS_DEVICE_NAME "acpi-als"
>>> #define ACPI_ALS_NOTIFY_ILLUMINANCE 0x80
>>>
>>> +#define ACPI_ALS_ASUS_TALS "\\_SB.PCI0.LPCB.EC0.TALS"
>>> +#define ACPI_ALS_ASUS_ALSC "\\_SB.ATKD.ALSC"
>>> +
>>> ACPI_MODULE_NAME("acpi-als");
>>>
>>> /*
>>> @@ -170,6 +173,16 @@ static int acpi_als_read_raw(struct iio_dev *indio_dev,
>>> return IIO_VAL_INT;
>>> }
>>>
>>> +static void acpi_als_quirk_asus(struct acpi_als *als)
>>> +{
>>> + acpi_execute_simple_method(NULL, ACPI_ALS_ASUS_TALS, 1);
>>> + acpi_execute_simple_method(NULL, ACPI_ALS_ASUS_ALSC, 1);
>>
>> This will run on ALL systems, right ? Also, error checking disappeared ?
>>
> 
> Yes, that's intended. Should I add some DMI checks to only run on
> ASUS/ASUS Zenbooks?

Definitely, it doesn't seem right to execute random stuff on hardware
where such stuff is undefined.

> As for error checking, this code behaves exactly like v1, except
> that the xor warning is removed, which means that it is not needed
> to explicitly get the handles. The acpi_exceute_simple_method call just
> fails if it can't find the object.

But the driver will register even though the call fails , that's wrong.
The probe must stop if there is a failure.

> Distinguishing between the "object not found" and "failed to call method" scenarios
> seemed a bit overly verbose to me especially as it would probably be overkill to abort
> the driver initialization because of that.

If something which should be present (like the acpi method) is not
present and/or fails, the driver probe should abort.

>>> +}
>>> +
>>> +static void (*acpi_als_quirks[])(struct acpi_als *als) = {
>>> + acpi_als_quirk_asus,
>>> +};
>>> +
>>> static const struct iio_info acpi_als_info = {
>>> .driver_module = THIS_MODULE,
>>> .read_raw = acpi_als_read_raw,
>>> @@ -180,6 +193,7 @@ static int acpi_als_add(struct acpi_device *device)
>>> struct acpi_als *als;
>>> struct iio_dev *indio_dev;
>>> struct iio_buffer *buffer;
>>> + int i;
>>>
>>> indio_dev = devm_iio_device_alloc(&device->dev, sizeof(*als));
>>> if (!indio_dev)
>>> @@ -191,6 +205,9 @@ static int acpi_als_add(struct acpi_device *device)
>>> als->device = device;
>>> mutex_init(&als->lock);
>>>
>>> + for (i = 0; i < ARRAY_SIZE(acpi_als_quirks); i++)
>>> + acpi_als_quirks[i](als);
>>> +
>>> indio_dev->name = ACPI_ALS_DEVICE_NAME;
>>> indio_dev->dev.parent = &device->dev;
>>> indio_dev->info = &acpi_als_info;
>>
>> --
>> Best regards,
>> Marek Vasut


-- 
Best regards,
Marek Vasut

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web