Skip to contentSkip to conversation

iio: light: acpi-als: Properly enable on ASUS Zenbooks

PATCH Superseded Josef Gajdusek 2 participants 4 messages Jan 19, 2017 IIO

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Josef Gajdusek

author
#0 lore raw

PATCH v2 1/1 iio: light: acpi-als: Properly enable on ASUS Zenbooks

ASUS Zenbooks need several special ACPI calls to enable the ALS peripheral. Otherwise, reads just return 0.

Signed-off-by Josef Gajdusek <[email protected]>
---
 drivers/iio/light/acpi-als.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
drivers/iio/light/acpi-als.c +17 -0
index f0b47c5..ccd5787 100644
@@ -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;
Signature · 1 line
2.10.2
#1 lore raw
Quoted 23 lines · Josef Gajdusek

ASUS Zenbooks need several special ACPI calls to enable the ALS peripheral. Otherwise, reads just return 0.

Signed-off-by Josef Gajdusek <[email protected]>
---
 drivers/iio/light/acpi-als.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
drivers/iio/light/acpi-als.c +3 -0
index f0b47c5..ccd5787 100644
@@ -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");
 
 /*

drivers/iio/light/acpi-als.c static int acpi_als_read_raw(struct iio_dev *indio_dev, · line 179

Quoted 7 lines
drivers/iio/light/acpi-als.c +4 -0
@@ -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 ?

Quoted 28 lines
drivers/iio/light/acpi-als.c +10 -0
+}
+
+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;
Signature · 2 lines
Best regards,
Marek Vasut
author
#2 lore raw
Quoted 28 lines · Marek Vasut
Quoted 26 lines · Josef Gajdusek

ASUS Zenbooks need several special ACPI calls to enable the ALS peripheral. Otherwise, reads just return 0.

Signed-off-by Josef Gajdusek <[email protected]>

---
drivers/iio/light/acpi-als.c | 17 +++++++++++++++++

1 file changed, 17 insertions(+)
drivers/iio/light/acpi-als.c +3 -0
index f0b47c5..ccd5787 100644
@@ -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;
 }
Quoted 7 lines

drivers/iio/light/acpi-als.c static int acpi_als_read_raw(struct iio_dev *indio_dev, · line 179

Quoted 4 lines
drivers/iio/light/acpi-als.c +4 -0
+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.

Quoted 31 lines
Quoted 18 lines
drivers/iio/light/acpi-als.c +7 -0
+}
+
+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)

drivers/iio/light/acpi-als.c static int acpi_als_add(struct acpi_device *device) · line 213

Quoted 9 lines
drivers/iio/light/acpi-als.c +3 -0
@@ -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

#3 lore raw
Quoted 37 lines · On 01/19/2017 05:19 PM, [email protected] wrote:
Quoted 28 lines · Marek Vasut
Quoted 26 lines · Josef Gajdusek

ASUS Zenbooks need several special ACPI calls to enable the ALS peripheral. Otherwise, reads just return 0.

Signed-off-by Josef Gajdusek <[email protected]>

---
drivers/iio/light/acpi-als.c | 17 +++++++++++++++++

1 file changed, 17 insertions(+)
drivers/iio/light/acpi-als.c +3 -0
index f0b47c5..ccd5787 100644
@@ -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;
 }
Quoted 7 lines

drivers/iio/light/acpi-als.c static int acpi_als_read_raw(struct iio_dev *indio_dev, · line 179

Quoted 4 lines
drivers/iio/light/acpi-als.c +4 -0
+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 ?

Quoted 2 lines

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.

Quoted 4 lines

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.

Quoted 3 lines

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.

Quoted 31 lines
Quoted 31 lines
Quoted 18 lines
drivers/iio/light/acpi-als.c +7 -0
+}
+
+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)

drivers/iio/light/acpi-als.c static int acpi_als_add(struct acpi_device *device) · line 213

Quoted 9 lines
drivers/iio/light/acpi-als.c +3 -0
@@ -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

Signature · 2 lines
Best regards,
Marek Vasut