lkml.org 
[lkml]   [2019]   [Feb]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v2 1/8] leds: Add support for AXP20X CHGLED
From
Date

On 2/15/19 8:32 PM, Pavel Machek wrote:
> Hi!
>
>> On Fri, Feb 15, 2019 at 01:50:06PM +0200, Stefan Mavrodiev wrote:
>>> +static ssize_t control_store(struct device *dev, struct device_attribute *attr,
>>> + const char *buf, size_t size)
>>> +{
>>> + struct led_classdev *cdev = dev_get_drvdata(dev);
>>> + struct axp20x_led *priv = to_axp20x_led(cdev);
>>> + unsigned long val;
>>> + int ret;
>>> +
>>> + ret = kstrtoul(buf, 0, &val);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + /**
>>> + * Supported values are:
>>> + * - 0 : Manual control
>>> + * - 1 : Charger control
>>> + */
> ...
>>> +static struct attribute *axp20x_led_attrs[] = {
>>> + &dev_attr_control.attr,
>>> + &dev_attr_mode.attr,
>>> + NULL,
>>> +};
>>> +ATTRIBUTE_GROUPS(axp20x_led);
>> I can't really say whether adding sysfs handles for this is the right
>> thing to do, but if it is you should document the interface.
> It is not. See "Add Intel Cherry Trail Whiskey Cove PMIC LEDs" thread
> in the last few days.

What I understood is that there will be changes in the LED core, exporting
a new sysfs entry "hw_control". Then I should set HW_CONTROL flag for the
device and add hw_control_get/set callback functions. And this can happen
when the LED core is updated. Right?

However I didn't understand how (in my case) hardware controlled pattern
will be selected?

Best regards,
Stefan Mavrodiev

>>> + if (!of_property_read_u8(np, "x-powers,charger-mode", &value)) {
>>> + priv->ctrl = AXP20X_CHGLED_CTRL_CHARGER;
>>> + priv->mode = (value < 2) ? value : 0;
>>> + } else {
>>> + priv->ctrl = AXP20X_CHGLED_CTRL_MANUAL;
>>> + }
>> I'm not sure we want to make this a property of the device
>> tree. Changing the device tree isn't an option for some users, so we
>> need to make sure we can change it even if we can't change the device
>> tree.
> We want this to be configurable at run time. It can get default from
> the device tree.
>
> If we go for the "hardware" trigger, you'll get it for free.
>
> Best regards,
> Pavel

\
 
 \ /
  Last update: 2019-02-19 08:43    [W:0.067 / U:1.436 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site