Re: [PATCH 2/2] leds: ubnt-spi: Add Ubnt AirCube ISP LED driver

[Pavel Machek <pavel@ucw.cz>]

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

On Sat 2019-05-04 14:28:25, list@c-mauderer.de wrote:
> From: Christian Mauderer <oss@c-mauderer.de>
> 
> This driver adds support for the custom LED controller used in Ubiquity
> airCube ISP devices and most likely some other simmilar Ubiquity
> products.

similar.

> +config LEDS_UBNT_SPI
> +	tristate "LED support for Ubnt aircube ISP custom SPI LED controller"
> +	depends on LEDS_CLASS
> +	depends on SPI
> +	depends on OF
> +	help
> +	  This option enables basic support for the LED controller used in
> +	  Ubiquity airCube ISP devices. The controller is microcontroller based
> +	  and uses a single byte on the SPI to set brightness or blink patterns.

> +/*
> + *  Custom controller based LED controller used in Ubiquity airCube ISP.
> + *
> + *  Reverse engineered protocol:
> + *  - The device uses only a single byte sent via SPI.
> + *  - The SPI input doesn't contain any relevant information.
> + *  - Higher two bits set a mode. Lower six bits are a parameter.
> + *  - Mode: 00 -> set brightness between 0x00 (min) and 0x3F (max)
> + *  - Mode: 01 -> pulsing pattern (min -> max -> min) with an interval. From
> + *    some tests, the period is about (50ms + 102ms * parameter). There is a
> + *    slightly different pattern starting from 0x100 (longer gap between the
> + *    pulses) but the time still follows that calculation.
> + *  - Mode: 10 -> same as 01 but with only a ramp from min to max. Again a
> + *    slight jump in the pattern at 0x100.
> + *  - Mode: 11 -> blinking (off -> 25% -> off -> 25% -> ...) with a period of
> + *    (105ms * parameter)
> + *
> + *  NOTE: This driver currently only implements mode 00 (brightness).
> + */

Aha, so this is not as simple as I thought.

"Slightly different pattern starting from 0x100"? What does 0x100 mean
here.

> +	mutex_init(&led->mutex);
> +	led->ldev.name = led->name;
> +	led->ldev.brightness = LED_OFF;
> +	led->ldev.max_brightness = led->max_bright - led->off_bright;

What happens when DTS has off_bright > max_bright? :-).

> +
> +static int ubnt_spi_remove(struct spi_device *spi)
> +{
> +	struct ubnt_spi_led	*led = spi_get_drvdata(spi);
> +
> +	led_classdev_unregister(&led->ldev);
> +
> +	return 0;
> +}

Do you need to do mutex_destroy?
									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 --]