Re: [PATCH 2/2] leds: add Panasonic AN30259A support

[Pavel Machek <pavel@ucw.cz>]

[-- Attachment #1: Type: text/plain, Size: 2123 bytes --]

Hi!

> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index 3e763d2a0cb3..80bed557cc6b 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -57,6 +57,16 @@ config LEDS_AAT1290
>  	depends on PINCTRL
>  	help
>  	 This option enables support for the LEDs on the AAT1290.
> +
> +config LEDS_AN30259A
> +    tristate "LED support for Panasonic AN30259A"
> +    depends on OF
> +    depends on I2C
> +    depends on LEDS_CLASS
> +    help
> +     This option enables support for the AN30259A 3-channel
> +     LED driver.

Something funny goes on with the indentation here. And normally we'd
do depends on A && B && ... and have note how the module will be
called.


> +++ b/drivers/leds/leds-an30259a.c
> @@ -0,0 +1,338 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <linux/i2c.h>

This would be suitable place for author name, and very short hw
description. http link for datasheet would be preffered here over the
dts binding.

> +		duty_max = an30259a_get_duty_max(brightness & 0xff);
> +		ret = regmap_write(led->chip->regmap,
> +				REG_LEDCNT1(led->num),
> +				LED_DUTYMAX(duty_max) | LED_DUTYMID(duty_max));
> +		if (ret)
> +			return ret;
> +		break;
> +	}
> +	ret = regmap_write(led->chip->regmap, REG_LEDON,
> +				ledon);

You can fit this on one line.

> +		chip->leds[i].cdev.brightness_set_blocking = an30259a_led_set;
> +		chip->leds[i].cdev.blink_set = an30259a_blink_set;
> +
> +		err = led_classdev_register(&client->dev, &chip->leds[i].cdev);
> +		if (err < 0)
> +			goto exit;
> +	}

Do ve have devm version of led class register?

> +static int an30259a_remove(struct i2c_client *client)
> +{
> +	struct an30259a *chip = i2c_get_clientdata(client);
> +	int i;
> +
> +	for (i = 0; i < MAX_LEDS; i++)
> +		led_classdev_unregister(&chip->leds[i].cdev);

You always unregister 3 leds; what if only 1 is described in the
device tree?

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]