Re: [PATCH v3 05/11] leds: led-class: Add generic [devm_]led_get()

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 12/16/22 14:43, Andy Shevchenko wrote:
> On Fri, Dec 16, 2022 at 12:30:07PM +0100, Hans de Goede wrote:
>> Add a generic [devm_]led_get() method which can be used on both devicetree
>> and non devicetree platforms to get a LED classdev associated with
>> a specific function on a specific device, e.g. the privacy LED associated
>> with a specific camera sensor.
>>
>> Note unlike of_led_get() this takes a string describing the function
>> rather then an index. This is done because e.g. camera sensors might
>> have a privacy LED, or a flash LED, or both and using an index
>> approach leaves it unclear what the function of index 0 is if there is
>> only 1 LED. The existing of support is extended to also support
>> getting a LED by function-name using the standard devicetree pattern
>> of adding a -names string array to map names to the indexes.
>>
>> For non devicetree platforms a lookup-table mechanism is added to
>> allow the platform code to map specific LED class_dev-s to specific
>> device,function combinations this way.
> 
> ...
> 
>> +	list_for_each_entry(lookup, &leds_lookup_list, list) {
>> +		if (!strcmp(lookup->consumer_dev_name, dev_name(dev)) &&
>> +		    !strcmp(lookup->consumer_function, function)) {
>> +			led_name = kstrdup(lookup->led_name, GFP_KERNEL);
> 
> kstrdup_const() ?

Ack.

> 
>> +			break;
>> +		}
>> +	}
> 
>> +	if (!led_name)
>> +		return ERR_PTR(-ENOENT);
>> +
>> +	led_dev = class_find_device_by_name(leds_class, led_name);
>> +	kfree(led_name);
> 
> kfree_const() ?

Ack, will change for v4.

> 
>> +	return __led_get(led_dev);
>> +}
> 
> ...
> 
>> +EXPORT_SYMBOL_GPL(led_add_lookup);
> 
>> +EXPORT_SYMBOL_GPL(led_remove_lookup);
> 
> Wondering why we can't make at least those two to be namespaced from day 1,

switching the LED subsystem code to use module namespaces is
a RFE which is independent of / orthogonal to this patchset.

Regards,

Hans