linux-kernel - Re: [PATCH v7 2/2] leds: lm3601x: Introduce the lm3601x LED driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Tue, 22 May 2018 07:17:08 -0500
From:   Dan Murphy <dmurphy@...com>
To:     Andy Shevchenko <andy.shevchenko@...il.com>
CC:     Jacek Anaszewski <jacek.anaszewski@...il.com>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        devicetree <devicetree@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linux LED Subsystem <linux-leds@...r.kernel.org>
Subject: Re: [PATCH v7 2/2] leds: lm3601x: Introduce the lm3601x LED driver

Andy

On 05/21/2018 06:05 PM, Andy Shevchenko wrote:
> On Tue, May 22, 2018 at 12:44 AM, Dan Murphy <dmurphy@...com> wrote:
> 
> 
>>>> +    child = device_get_next_child_node(&led->client->dev, child);
>>>> +    if (!child) {
>>>> +        dev_err(&led->client->dev, "No LED Child node\n");
>>>> +        return ret;
>>>> +    }
>>>> +
>>>> +    ret = fwnode_property_read_u32(child, "reg", &led->led_mode);
>>>> +    if (ret) {
>>>> +        dev_err(&led->client->dev, "reg DT property missing\n");
>>>> +        goto out_err;
>>>> +    }
>>>> +
>>>> +    if (led->led_mode > LM3601X_LED_TORCH ||
>>>> +        led->led_mode < LM3601X_LED_IR) {
>>>> +        dev_warn(&led->client->dev, "Invalid led mode requested\n");
>>>> +        ret = -EINVAL;
>>>> +        goto out_err;
>>>> +    }
>>>> +
>>>> +    ret = fwnode_property_read_string(child, "label", &name);
>>>> +    if (ret) {
>>>> +        if (led->led_mode == LM3601X_LED_TORCH)
>>>> +            name = "torch";
>>>> +        else
>>>> +            name = "infrared";
>>>> +    }
>>>> +
>>>> +    snprintf(led->led_name, sizeof(led->led_name),
>>>> +        "%s:%s", node->name, name);
>>>
>>> Reading once again my recent explanation regarding this I realized
>>> that I didn't provide clear conclusion, which is: we no longer
>>> use child node name for LED class device name if label is absent.
>>> (apart from that - you're using parent DT node now, i.e.
>>> led-controller).
>>>
>>> Please follow what was done for drivers/leds/leds-cr0014114.c.
>>
>> Hmmm.  If this is calling dev->of_node->name to store the name will this
>> work in non-DT configurations?
> 
> I didn't found this kind of use in linux-next, perhaps I missed something?
> 

Linux next has a different version then what I found doing a search for the driver.

I will change the code to match what is in linux-next.

Dan

> In the driver Jacek referred to I found, though, use of of_node, which
> at some point should be changed to fwnode.
> 
> For now you can fill that if you want to using something like this
> (IIRC it should work):
> 
> if (is_of_node(fwnode))
>  ...->of_node = to_of_node(...);
> 
>> I have not dug to deeply into the fwnode code to find out how the nodes
>> get populated.  So my question may not even be valid.
> 


-- 
------------------
Dan Murphy