On 05/05/2019 22:09, Jacek Anaszewski wrote:
> Christian,
>
> Thank you for the update. One thing left after switching
> to devm API. Please refer below to the remove op.
>
> On 5/5/19 10:00 PM, oss@xxxxxxxxxxxxx wrote:
>> From: Christian Mauderer <oss@xxxxxxxxxxxxx>
>>
>> This driver adds support for simple SPI based LED controller which use
>> only one byte for setting the brightness.
>>
>> Signed-off-by: Christian Mauderer <oss@xxxxxxxxxxxxx>
>> ---
>>
>> Changes compared to v2:
>> - use "if (ret)" instead of "if (ret != 0)"
>> - don't initialize ldev-fields with zero
>> - use devm_led_classdev_register instead of led_classdev_register
>> - check for error instead of good case with the last if in spi_byte_probe
>>
>> Changes compared to v1:
>> - rename ubnt-spi to leds-spi-byte
>> - rework probe to get all parameters before allocating anything ->
>> error checks
>> all collected together and initializing all fields of the device
>> structure is
>> more obvious
>> - fix some unsteady indentations during variable declaration
>> - rework comment with protocol explanation
>> - handle case of off_bright > max_bright
>> - fix spelling in commit message
>> - mutex_destroy in remove
>> - change label to use either use the given one without a prefix or a
>> default one
>>
>>
>> drivers/leds/Kconfig | 12 ++++
>> drivers/leds/Makefile | 1 +
>> drivers/leds/leds-spi-byte.c | 134 +++++++++++++++++++++++++++++++++++
>> 3 files changed, 147 insertions(+)
>> create mode 100644 drivers/leds/leds-spi-byte.c
>
> [...]
>> +
>> +static int spi_byte_remove(struct spi_device *spi)
>> +{
>> + struct spi_byte_led *led = spi_get_drvdata(spi);
>> +
>> + led_classdev_unregister(&led->ldev);
>
> This is now not needed - devm, means "device managed", i.e.
> all resources claimed with it will be automatically reclaimed
> on device destruction.
>
Thanks for the hint. I've read that the devm functions are some "free
automatically" functions. But I haven't drawn the conclusion that I have
to remove this function. Seems I still need to learn a lot about the
Linux API.
I'll wait for further feedback over night and then remove it in a v4
tomorrow. Otherwise I'll spam the list with lots of new versions.
>> + mutex_destroy(&led->mutex);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct of_device_id spi_byte_dt_ids[] = {
>> + { .compatible = "leds-spi-byte", },
>> + {},
>> +};
>> +
>> +MODULE_DEVICE_TABLE(of, spi_byte_dt_ids);
>> +
>> +static struct spi_driver spi_byte_driver = {
>> + .probe = spi_byte_probe,
>> + .remove = spi_byte_remove,
>> + .driver = {
>> + .name = KBUILD_MODNAME,
>> + .of_match_table = spi_byte_dt_ids,
>> + },
>> +};
>> +
>> +module_spi_driver(spi_byte_driver);
>> +
>> +MODULE_AUTHOR("Christian Mauderer <oss@xxxxxxxxxxxxx>");
>> +MODULE_DESCRIPTION("single byte SPI LED driver");
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_ALIAS("spi:leds-spi-byte");
>>
>
