Re: [PATCH 1/2] leds: aw21024: Add support for Awinic's AW21024

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

On 17/05/2022 20:36, Kyle Swenson wrote:
>>> +static const struct of_device_id of_aw21024_leds_match[] = {
>>> +	{ .compatible = "awinic,aw21024", },
>>> +	{},
>>> +};
>>> +MODULE_DEVICE_TABLE(of, of_aw21024_leds_match);
>>> +
>>> +static struct i2c_driver aw21024_driver = {
>>> +	.driver		= {
>>> +		.name	= "aw21024",
>>> +		.of_match_table = of_match_ptr(of_aw21024_leds_match),
>>
>> of_match_ptr causes this being unused. kbuild robot probably pointed
>> this out... if not - of_match_ptr goes with maybe_unused. You need both
>> or none, depending on intended usage.
>>
> Ah, yes, the kbuild robot did point this out to me, and I had planned on
> fixing by adding 'depends on OF' to the Kconfig.  Perhaps that isn't
> correct or complete (or even relevant)?
> 
> I'll do some investigating and determine if I need to use of_match_ptr
> or not and I'll fix it either by removing it or adding maybe_unused in
> the next version.

Your has i2c_device_id so it could bind without OF, however obviously
aw21024_probe_dt() will do nothing and return 0.

Therefore it is up to you if you want to add dependency on OF. If you
add, please add it with "|| COMPILE_TEST".

Then in both cases you need to handle the case of building (not running)
a driver without OF: using maybe_unused+of_match_ptr() or nothing (thus
always referencing of_device_id). Which one to choose matters less.
Using it causes the code to be smaller for !OF case, which might matter
for some distros which build everything as module. Not using it allows
to match the driver on ACPI systems, although I am not sure if this is
relevant.

I don't have recommendation on that - just be sure there are no warnings.


Best regards,
Krzysztof