linux-kernel - Re: [PATCH] leds: Add DT support for leds-pca9532

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] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 09 Sep 2014 20:08:53 +0200
From:	Arnd Bergmann <arnd@...db.de>
To:	Roland Stigge <stigge@...com.de>
Cc:	robh+dt@...nel.org, riku.voipio@....fi, cooloney@...il.com,
	rpurdie@...ys.net, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-leds@...r.kernel.org
Subject: Re: [PATCH] leds: Add DT support for leds-pca9532

On Tuesday 09 September 2014 15:28:00 Roland Stigge wrote:
> +#ifdef CONFIG_OF
> +static struct pca9532_platform_data *pca9532_parse_dt(struct device *dev)
> +{
...
> +
>  static int pca9532_probe(struct i2c_client *client,
>         const struct i2c_device_id *id)
>  {
> @@ -449,6 +489,11 @@ static int pca9532_probe(struct i2c_clie
>         struct pca9532_platform_data *pca9532_pdata =
>                         dev_get_platdata(&client->dev);
>  
> +#ifdef CONFIG_OF
> +       if (!pca9532_pdata && client->dev.of_node)
> +               pca9532_pdata = pca9532_parse_dt(&client->dev);
> +#endif
> +

It would be better to remove the two #ifdef statements and instead
do this in C language as

	if (IS_ENABLED(CONFIG_OF) && !pca9532_pdata && client->dev.of_node)
		pca9532_pdata = pca9532_parse_dt(&client->dev);

This gives you extra compile-time coverage when CONFIG_OF is not
enabled.

I would probably also remove the !pca9532_pdata check there, since
we rarely do auxdata for new devices these days, but that is a separate
matter, and the check does not hurt.

> +       if (!of_property_read_u32_array(np, "nxp,pwm", &pwm[0], 2)) {
> +               for (i = 0; i < 2; i++)
> +                       pca9532_pdata->pwm[i] = pwm[i];
> +       }

Is this a full PWM controller? If it is, it would be better to also
provide a #pwm-cells property and register the device with the PWM
subsystem to allow arbitrary users.

If the PWM is only really usable for LED, it's probably enough to
leave this used by the LED subsystem.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/