On 07/07/16 15:34, Jacek Anaszewski wrote:
Why actually do you need this array? Couldn't you provide the LED
identifier in the DSD entry corresponding to a LED? You can follow
the scheme used in Device Tree by introducing "reg" property.
The names in the array are the 6 identifiers in the DSD entry. For
example
the DSD property for blue2 led used is
"Package ()
{
Package (2) {"blue2", "led_blue2"},
"
acpi_dev_get_property will return "led_blue2" in obj->string.pointer.
There isn't separate DSD for individual leds. There is only 1 DSD for
the led controller identified by "Name (_HID, "TXNW3952")". The
individual led names are separate properties withing th DSD (eg:
led_blue2 identified by blue2).
OK, thanks for the explanation. BTW LED name pattern is
devicename:colour:function.
+ for (i = 0; i < LP3952_LED_ALL; i++) {
+ priv->leds[i].cdev.brightness = LED_OFF;
+ priv->leds[i].cdev.max_brightness = LP3952_BRIGHT_MAX;
+ priv->leds[i].cdev.brightness_set_blocking =
+ lp3952_set_brightness;
+ priv->leds[i].channel = i;
+ priv->leds[i].priv = priv;
+
+ ret = lp3952_get_label(priv->leds[i].name, led_name_hdl[i],
+ &priv->client->dev);
+ if (ret)
+ break;
You're assuming here that all LEDs will always be present, which
doesn't necessarily have to be true. How about just skipping
the LED and moving to the next one if given label was not found?
You could move this check to the beginning of the loop then.
If we skip leds, there could be problem, if all the leds were
skipped(For example, no ACPI name data present).
If all the LEDs were skipped, then no LED class device would be
registered. In such a case you could return an error from
lp3952_probe().
At present, if a label is not present, the probe will fail.
How about reverting to a default name (eg: "lp3952:blue2") if led name
read fails?
There is no point in registering the LED if its DSD entry is not
valid. In case of Device Tree we fail LED registration in such
cases.
Sorry, I thought we were skipping LEDs due to invalid DSD. If that is
not the case, isn't the assumption all LEDS will be present true?
I am guessing leds might be missing, if some of the leds are not
physically connected to controller output. But the controller would
have 6 led controls all the time.
Right, but there is no point in exposing corresponding
LED class devices to user space if the user will be unable
to notice any effect because the LED is not connected.
Got it, Thank you :) Will skip registration if led not found.
This would also cover cases in which none of the led names
were read properly.
Whole driver probing should fail then.
+
+ priv->leds[i].cdev.name = priv->leds[i].name;
+
+ ret = devm_led_classdev_register(&priv->client->dev,
+ &priv->leds[i].cdev);
+ if (ret < 0) {
+ dev_err(&priv->client->dev,
+ "couldn't register LED %s\n",
+ priv->leds[i].cdev.name);
+ break;
+ }
+ }
+ return ret;
+}
+
--
To unsubscribe from this list: send the line "unsubscribe
linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-leds" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
