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

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]

Message-ID: <abe231fb-6891-4008-92a9-9c3525ea0bcf@kernel.org>
Date: Fri, 21 Feb 2025 09:42:41 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Steffen Trumtrar <s.trumtrar@...gutronix.de>, Pavel Machek
 <pavel@....cz>, Lee Jones <lee@...nel.org>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Steffen Trumtrar <kernel@...gutronix.de>
Cc: linux-leds@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] leds: add support for TI LP5860 LED driver chip

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@...gutronix.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