Re: [PATCH 2/2] leds: add support for TI LP5860 LED driver chip

[Krzysztof Kozlowski <krzk@kernel.org>]

On 20/02/2025 13:57, Steffen Trumtrar wrote:
> This adds support for the Texas Instruments LP5860 LED driver chip
> via SPI interfaces.
> 

Please do not use "This commit/patch/change", but imperative mood. See
longer explanation here:
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95

....


> +
> +	return 0;
> +}
> +
> +int lp5860_device_init(struct lp5860 *lp)
> +{
> +	int ret;
> +
> +	dev_set_drvdata(lp->dev, lp);
> +
> +	ret = lp5860_enable_disable(lp, 1);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_update_bits(lp->regmap, LP5860_DEV_INITIAL,
> +				 LP5860_MODE_MASK,
> +				 LP5860_MODE_1 << LP5860_MODE_OFFSET);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = sysfs_create_group(&lp->dev->kobj, &lp5860_group);


Where is ABI documentation?

> +	if (ret)
> +		return ret;
> +
> +	return lp5860_probe_dt(lp);
> +}


...

> +static const struct of_device_id lp5860_of_match[] = {
> +	{ .compatible = "ti,lp5860" },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, lp5860_of_match);
> +
> +static struct spi_driver lp5860_driver = {
> +	.driver = {
> +		.name = "lp5860",
> +		.of_match_table = of_match_ptr(lp5860_of_match),


Drop of_match_ptr, you have a warning here.

> +	},
> +	.probe	= lp5860_probe,
> +	.remove = lp5860_remove,
> +};
> +
> +module_spi_driver(lp5860_driver);
> +
> +MODULE_DESCRIPTION("TI leds lp5860");
> +MODULE_AUTHOR("Steffen Trumtrar <kernel@pengutronix.de>");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("spi:leds-lp5860");

You should not need MODULE_ALIAS() in normal cases. If you need it,
usually it means your device ID table is wrong (e.g. misses either
entries or MODULE_DEVICE_TABLE()). MODULE_ALIAS() is not a substitute
for incomplete ID table.

Best regards,
Krzysztof