* [RFC] iio: light: acpi-als: Enable the light sensor on the Zenbook UX430UQ
@ 2017-11-16 3:27 Kiernan Hager
2017-11-19 17:03 ` Jonathan Cameron
0 siblings, 1 reply; 5+ messages in thread
From: Kiernan Hager @ 2017-11-16 3:27 UTC (permalink / raw)
To: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, linux-iio, linux-kernel
Cc: Kiernan Hager
This makes acpi-als properly enable the light sensor on the Zenbook UX430UQ. I don't know if the checking that I do to make sure that the ACPI method exists is sufficient or if it should disable the sensor when the module is unloaded, so input is appreciated on those matters.
Signed-off-by: Kiernan Hager <kah.listaddress@gmail.com>
---
drivers/iio/light/acpi-als.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/drivers/iio/light/acpi-als.c b/drivers/iio/light/acpi-als.c
index c35e2f8df339..8fd66166f19f 100644
--- a/drivers/iio/light/acpi-als.c
+++ b/drivers/iio/light/acpi-als.c
@@ -179,6 +179,10 @@ static int acpi_als_add(struct acpi_device *device)
struct acpi_als *als;
struct iio_dev *indio_dev;
struct iio_buffer *buffer;
+ unsigned long long temp_val;
+ acpi_status status;
+ struct acpi_object_list arg_list;
+ union acpi_object arg;
indio_dev = devm_iio_device_alloc(&device->dev, sizeof(*als));
if (!indio_dev)
@@ -203,6 +207,18 @@ static int acpi_als_add(struct acpi_device *device)
iio_device_attach_buffer(indio_dev, buffer);
+ arg_list.count = 1;
+ arg_list.pointer = &arg;
+ arg.type = ACPI_TYPE_INTEGER;
+ arg.integer.value = 1;
+
+ if (acpi_has_method(als->device->handle, "\\_SB.PCI0.LPCB.EC0.ALSC")) {
+ status = acpi_evaluate_integer(als->device->handle,
+ "\\_SB.PCI0.LPCB.EC0.ALSC",
+ &arg_list,
+ &temp_val);
+ }
+
return devm_iio_device_register(&device->dev, indio_dev);
}
--
2.15.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [RFC] iio: light: acpi-als: Enable the light sensor on the Zenbook UX430UQ
2017-11-16 3:27 [RFC] iio: light: acpi-als: Enable the light sensor on the Zenbook UX430UQ Kiernan Hager
@ 2017-11-19 17:03 ` Jonathan Cameron
2017-11-19 17:38 ` Gabriele Mazzotta
0 siblings, 1 reply; 5+ messages in thread
From: Jonathan Cameron @ 2017-11-19 17:03 UTC (permalink / raw)
To: Kiernan Hager
Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
linux-iio, linux-kernel, Gabriele Mazzotta, Marek Vasut,
linux-acpi
On Wed, 15 Nov 2017 20:27:54 -0700
Kiernan Hager <kah.listaddress@gmail.com> wrote:
> This makes acpi-als properly enable the light sensor on the Zenbook UX430UQ. I don't know if the checking that I do to make sure that the ACPI method exists is sufficient or if it should disable the sensor when the module is unloaded, so input is appreciated on those matters.
Please wrap descriptions to around 72 characters (leaves room for indentation
in the email thread :)
This seems very much like a board specific hack. I've cc'd people
who have worked on the driver recently and the acpi list.
>
> Signed-off-by: Kiernan Hager <kah.listaddress@gmail.com>
> ---
> drivers/iio/light/acpi-als.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/drivers/iio/light/acpi-als.c b/drivers/iio/light/acpi-als.c
> index c35e2f8df339..8fd66166f19f 100644
> --- a/drivers/iio/light/acpi-als.c
> +++ b/drivers/iio/light/acpi-als.c
> @@ -179,6 +179,10 @@ static int acpi_als_add(struct acpi_device *device)
> struct acpi_als *als;
> struct iio_dev *indio_dev;
> struct iio_buffer *buffer;
> + unsigned long long temp_val;
> + acpi_status status;
> + struct acpi_object_list arg_list;
> + union acpi_object arg;
>
> indio_dev = devm_iio_device_alloc(&device->dev, sizeof(*als));
> if (!indio_dev)
> @@ -203,6 +207,18 @@ static int acpi_als_add(struct acpi_device *device)
>
> iio_device_attach_buffer(indio_dev, buffer);
>
> + arg_list.count = 1;
> + arg_list.pointer = &arg;
> + arg.type = ACPI_TYPE_INTEGER;
> + arg.integer.value = 1;
> +
> + if (acpi_has_method(als->device->handle, "\\_SB.PCI0.LPCB.EC0.ALSC")) {
So this is poking something on
PCI bus 0
LPC bus B
Somethingbus 0
Ambient light controller.
I assume there is a better way of establishing this needs to be set?
> + status = acpi_evaluate_integer(als->device->handle,
> + "\\_SB.PCI0.LPCB.EC0.ALSC",
> + &arg_list,
> + &temp_val);
> + }
> +
> return devm_iio_device_register(&device->dev, indio_dev);
> }
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC] iio: light: acpi-als: Enable the light sensor on the Zenbook UX430UQ
2017-11-19 17:03 ` Jonathan Cameron
@ 2017-11-19 17:38 ` Gabriele Mazzotta
2017-11-19 18:11 ` Marek Vasut
0 siblings, 1 reply; 5+ messages in thread
From: Gabriele Mazzotta @ 2017-11-19 17:38 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Kiernan Hager, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, linux-iio, linux-kernel, Marek Vasut,
linux-acpi
2017-11-19 18:03 GMT+01:00 Jonathan Cameron <jic23@kernel.org>:
> On Wed, 15 Nov 2017 20:27:54 -0700
> Kiernan Hager <kah.listaddress@gmail.com> wrote:
>
>> This makes acpi-als properly enable the light sensor on the Zenbook UX430UQ. I don't know if the checking that I do to make sure that the ACPI method exists is sufficient or if it should disable the sensor when the module is unloaded, so input is appreciated on those matters.
>
> Please wrap descriptions to around 72 characters (leaves room for indentation
> in the email thread :)
>
> This seems very much like a board specific hack. I've cc'd people
> who have worked on the driver recently and the acpi list.
This is indeed board specific. I know many other laptops of the
Zenbook line need something similar and if you look at
drivers/platform/x86/asus-wmi.c or any other asus-* driver you'll
find several quirks to enable various features.
Kiernan, check commit aca234f6378864d85514be558746c0ea6eabfa8e
("asus-wmi: provide access to ALS control") in particular.
>>
>> Signed-off-by: Kiernan Hager <kah.listaddress@gmail.com>
>> ---
>> drivers/iio/light/acpi-als.c | 16 ++++++++++++++++
>> 1 file changed, 16 insertions(+)
>>
>> diff --git a/drivers/iio/light/acpi-als.c b/drivers/iio/light/acpi-als.c
>> index c35e2f8df339..8fd66166f19f 100644
>> --- a/drivers/iio/light/acpi-als.c
>> +++ b/drivers/iio/light/acpi-als.c
>> @@ -179,6 +179,10 @@ static int acpi_als_add(struct acpi_device *device)
>> struct acpi_als *als;
>> struct iio_dev *indio_dev;
>> struct iio_buffer *buffer;
>> + unsigned long long temp_val;
>> + acpi_status status;
>> + struct acpi_object_list arg_list;
>> + union acpi_object arg;
>>
>> indio_dev = devm_iio_device_alloc(&device->dev, sizeof(*als));
>> if (!indio_dev)
>> @@ -203,6 +207,18 @@ static int acpi_als_add(struct acpi_device *device)
>>
>> iio_device_attach_buffer(indio_dev, buffer);
>>
>> + arg_list.count = 1;
>> + arg_list.pointer = &arg;
>> + arg.type = ACPI_TYPE_INTEGER;
>> + arg.integer.value = 1;
>> +
>> + if (acpi_has_method(als->device->handle, "\\_SB.PCI0.LPCB.EC0.ALSC")) {
> So this is poking something on
> PCI bus 0
> LPC bus B
> Somethingbus 0
> Ambient light controller.
>
> I assume there is a better way of establishing this needs to be set?
>
>> + status = acpi_evaluate_integer(als->device->handle,
>> + "\\_SB.PCI0.LPCB.EC0.ALSC",
>> + &arg_list,
>> + &temp_val);
>> + }
>> +
>> return devm_iio_device_register(&device->dev, indio_dev);
>> }
>>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC] iio: light: acpi-als: Enable the light sensor on the Zenbook UX430UQ
2017-11-19 17:38 ` Gabriele Mazzotta
@ 2017-11-19 18:11 ` Marek Vasut
2017-11-20 16:51 ` Kiernan Hager
0 siblings, 1 reply; 5+ messages in thread
From: Marek Vasut @ 2017-11-19 18:11 UTC (permalink / raw)
To: Gabriele Mazzotta, Jonathan Cameron
Cc: Kiernan Hager, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, linux-iio, linux-kernel, linux-acpi
On 11/19/2017 06:38 PM, Gabriele Mazzotta wrote:
> 2017-11-19 18:03 GMT+01:00 Jonathan Cameron <jic23@kernel.org>:
>> On Wed, 15 Nov 2017 20:27:54 -0700
>> Kiernan Hager <kah.listaddress@gmail.com> wrote:
>>
>>> This makes acpi-als properly enable the light sensor on the Zenbook UX430UQ. I don't know if the checking that I do to make sure that the ACPI method exists is sufficient or if it should disable the sensor when the module is unloaded, so input is appreciated on those matters.
>>
>> Please wrap descriptions to around 72 characters (leaves room for indentation
>> in the email thread :)
>>
>> This seems very much like a board specific hack. I've cc'd people
>> who have worked on the driver recently and the acpi list.
>
> This is indeed board specific. I know many other laptops of the
> Zenbook line need something similar and if you look at
> drivers/platform/x86/asus-wmi.c or any other asus-* driver you'll
> find several quirks to enable various features.
Maybe that's where it should go . EC0 in the ACPI path seems like
Embedded Controller to me .
> Kiernan, check commit aca234f6378864d85514be558746c0ea6eabfa8e
> ("asus-wmi: provide access to ALS control") in particular.
>
>>>
>>> Signed-off-by: Kiernan Hager <kah.listaddress@gmail.com>
>>> ---
>>> drivers/iio/light/acpi-als.c | 16 ++++++++++++++++
>>> 1 file changed, 16 insertions(+)
>>>
>>> diff --git a/drivers/iio/light/acpi-als.c b/drivers/iio/light/acpi-als.c
>>> index c35e2f8df339..8fd66166f19f 100644
>>> --- a/drivers/iio/light/acpi-als.c
>>> +++ b/drivers/iio/light/acpi-als.c
>>> @@ -179,6 +179,10 @@ static int acpi_als_add(struct acpi_device *device)
>>> struct acpi_als *als;
>>> struct iio_dev *indio_dev;
>>> struct iio_buffer *buffer;
>>> + unsigned long long temp_val;
>>> + acpi_status status;
>>> + struct acpi_object_list arg_list;
>>> + union acpi_object arg;
>>>
>>> indio_dev = devm_iio_device_alloc(&device->dev, sizeof(*als));
>>> if (!indio_dev)
>>> @@ -203,6 +207,18 @@ static int acpi_als_add(struct acpi_device *device)
>>>
>>> iio_device_attach_buffer(indio_dev, buffer);
>>>
>>> + arg_list.count = 1;
>>> + arg_list.pointer = &arg;
>>> + arg.type = ACPI_TYPE_INTEGER;
>>> + arg.integer.value = 1;
>>> +
>>> + if (acpi_has_method(als->device->handle, "\\_SB.PCI0.LPCB.EC0.ALSC")) {
>> So this is poking something on
>> PCI bus 0
>> LPC bus B
>> Somethingbus 0
>> Ambient light controller.
>>
>> I assume there is a better way of establishing this needs to be set?
>>
>>> + status = acpi_evaluate_integer(als->device->handle,
>>> + "\\_SB.PCI0.LPCB.EC0.ALSC",
>>> + &arg_list,
>>> + &temp_val);
>>> + }
>>> +
>>> return devm_iio_device_register(&device->dev, indio_dev);
>>> }
>>>
>>
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC] iio: light: acpi-als: Enable the light sensor on the Zenbook UX430UQ
2017-11-19 18:11 ` Marek Vasut
@ 2017-11-20 16:51 ` Kiernan Hager
0 siblings, 0 replies; 5+ messages in thread
From: Kiernan Hager @ 2017-11-20 16:51 UTC (permalink / raw)
To: Marek Vasut
Cc: Gabriele Mazzotta, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler, linux-iio,
linux-kernel, linux-acpi
On Sun, Nov 19, 2017 at 11:11 AM, Marek Vasut <marex@denx.de> wrote:
> On 11/19/2017 06:38 PM, Gabriele Mazzotta wrote:
>> 2017-11-19 18:03 GMT+01:00 Jonathan Cameron <jic23@kernel.org>:
>>> On Wed, 15 Nov 2017 20:27:54 -0700
>>> Kiernan Hager <kah.listaddress@gmail.com> wrote:
>>>
>>>> This makes acpi-als properly enable the light sensor on the Zenbook UX430UQ. I don't know if the checking that I do to make sure that the ACPI method exists is sufficient or if it should disable the sensor when the module is unloaded, so input is appreciated on those matters.
>>>
>>> Please wrap descriptions to around 72 characters (leaves room for indentation
>>> in the email thread :)
>>>
>>> This seems very much like a board specific hack. I've cc'd people
>>> who have worked on the driver recently and the acpi list.
>>
>> This is indeed board specific. I know many other laptops of the
>> Zenbook line need something similar and if you look at
>> drivers/platform/x86/asus-wmi.c or any other asus-* driver you'll
>> find several quirks to enable various features.
>
> Maybe that's where it should go . EC0 in the ACPI path seems like
> Embedded Controller to me .
>
>> Kiernan, check commit aca234f6378864d85514be558746c0ea6eabfa8e
>> ("asus-wmi: provide access to ALS control") in particular.
>>
>>>>
>>>> Signed-off-by: Kiernan Hager <kah.listaddress@gmail.com>
>>>> ---
>>>> drivers/iio/light/acpi-als.c | 16 ++++++++++++++++
>>>> 1 file changed, 16 insertions(+)
>>>>
>>>> diff --git a/drivers/iio/light/acpi-als.c b/drivers/iio/light/acpi-als.c
>>>> index c35e2f8df339..8fd66166f19f 100644
>>>> --- a/drivers/iio/light/acpi-als.c
>>>> +++ b/drivers/iio/light/acpi-als.c
>>>> @@ -179,6 +179,10 @@ static int acpi_als_add(struct acpi_device *device)
>>>> struct acpi_als *als;
>>>> struct iio_dev *indio_dev;
>>>> struct iio_buffer *buffer;
>>>> + unsigned long long temp_val;
>>>> + acpi_status status;
>>>> + struct acpi_object_list arg_list;
>>>> + union acpi_object arg;
>>>>
>>>> indio_dev = devm_iio_device_alloc(&device->dev, sizeof(*als));
>>>> if (!indio_dev)
>>>> @@ -203,6 +207,18 @@ static int acpi_als_add(struct acpi_device *device)
>>>>
>>>> iio_device_attach_buffer(indio_dev, buffer);
>>>>
>>>> + arg_list.count = 1;
>>>> + arg_list.pointer = &arg;
>>>> + arg.type = ACPI_TYPE_INTEGER;
>>>> + arg.integer.value = 1;
>>>> +
>>>> + if (acpi_has_method(als->device->handle, "\\_SB.PCI0.LPCB.EC0.ALSC")) {
>>> So this is poking something on
>>> PCI bus 0
>>> LPC bus B
>>> Somethingbus 0
>>> Ambient light controller.
>>>
>>> I assume there is a better way of establishing this needs to be set?
>>>
>>>> + status = acpi_evaluate_integer(als->device->handle,
>>>> + "\\_SB.PCI0.LPCB.EC0.ALSC",
>>>> + &arg_list,
>>>> + &temp_val);
>>>> + }
>>>> +
>>>> return devm_iio_device_register(&device->dev, indio_dev);
>>>> }
>>>>
>>>
>
>
> --
> Best regards,
> Marek Vasut
That sounds like a good idea. I'll have a look at that file and
possibly apply my patch to that instead.
Kiernan
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-11-20 16:51 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-16 3:27 [RFC] iio: light: acpi-als: Enable the light sensor on the Zenbook UX430UQ Kiernan Hager
2017-11-19 17:03 ` Jonathan Cameron
2017-11-19 17:38 ` Gabriele Mazzotta
2017-11-19 18:11 ` Marek Vasut
2017-11-20 16:51 ` Kiernan Hager
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).