Message-ID: <B4CFA18F-CE12-4257-AAD0-FA9B744A1E29@gmail.com>
Date: Fri, 31 Oct 2025 13:17:46 -0400
From: Jean-François Lessard <jefflessard3@...il.com>
To: Andy Shevchenko <andriy.shevchenko@...el.com>
CC: Andy Shevchenko <andy@...nel.org>, Geert Uytterhoeven <geert@...ux-m68k.org>,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>, linux-kernel@...r.kernel.org,
 linux-leds@...r.kernel.org, devicetree@...r.kernel.org,
 Paolo Sabatino <paolo.sabatino@...il.com>,
 Christian Hewitt <christianshewitt@...il.com>,
 Martin Blumenstingl <martin.blumenstingl@...glemail.com>
Subject: Re: [PATCH v5 4/7] auxdisplay: Add TM16xx 7-segment LED matrix display controllers driver

Le 31 octobre 2025 05 h 41 min 44 s HAE, Andy Shevchenko <andriy.shevchenko@...el.com> a écrit :
>On Fri, Sep 26, 2025 at 10:19:05AM -0400, Jean-François Lessard wrote:
>> Add driver for TM16xx family LED controllers and compatible chips from
>> multiple vendors including Titan Micro, Fuda Hisi, i-Core, Princeton, and
>> Winrise. These controllers drive 7-segment digits and individual LED icons
>> through either I2C or SPI buses.
>> 
>> Successfully tested on various ARM TV boxes including H96 Max, Magicsee N5,
>> Tanix TX3 Mini, Tanix TX6, X92, and X96 Max across different SoC platforms
>> (Rockchip, Amlogic, Allwinner).
>
>...
>
>> +config TM16XX
>
>Hmm... After applying this patch there will be no compile test coverage.
>
>> +	tristate
>
>IIRC there is a trick how to achieve that by modifying a tristate line to be
>visible depending on the other options.
>
>E.g.,
>drivers/dpll/zl3073x/Kconfig:4: tristate "Microchip Azurite DPLL/PTP/SyncE devices" if COMPILE_TEST
>

Acknowledged. I'll add COMPILE_TEST:
tristate "TM16xx LED matrix display controllers" if COMPILE_TEST

>> +	select LEDS_CLASS
>> +	select LEDS_TRIGGERS
>> +	select LINEDISP
>> +	select NEW_LEDS
>> +	help
>> +	  Core TM16XX-compatible 7-segment LED controllers module
>
>Please, elaborate a bit more here. Usually we expect ~3 lines of description to
>be a minimum.
>

I'll expand to ~3 lines describing driver purpose and supported hardware
families.

>...
>
>> +#ifndef _TM16XX_H
>> +#define _TM16XX_H
>
>+ bits.h
>
>> +#include <linux/bitfield.h>
>> +#include <linux/leds.h>
>
>+ mutex.h
>
>> +#include <linux/workqueue.h>
>
>+ types.h
>

Will add missing includes.

>...
>
>> +#define FD655_CMD_CTRL		0x48
>> +#define FD655_CMD_ADDR		0x66
>> +#define FD655_CTRL_BR_MASK	GENMASK(6, 5)
>> +#define FD655_CTRL_ON		(1 << 0)
>> +
>> +#define FD6551_CMD_CTRL		0x48
>
>Do we need a duplicate? Yes, bitfields can be different, but since the register
>is called the same, I would leave only one register offset definition.
>

Acknowledged. I'll consolidate to single definition since they share the same
offset.

>...
>
>> +/**
>> + * DOC: struct tm16xx_controller - Controller-specific operations and limits
>> + * @max_grids: Maximum number of grids supported by the controller.
>> + * @max_segments: Maximum number of segments supported by the controller.
>> + * @max_brightness: Maximum brightness level supported by the controller.
>> + * @max_key_rows: Maximum number of key input rows supported by the controller.
>> + * @max_key_cols: Maximum number of key input columns supported by the controller.
>> + * @init: Pointer to controller mode/brightness configuration function.
>> + * @data: Pointer to function writing display data to the controller.
>> + * @keys: Pointer to function reading controller key state into bitmap.
>> + *
>> + * Holds function pointers and limits for controller-specific operations.
>> + */
>> +struct tm16xx_controller {
>> +	const u8 max_grids;
>> +	const u8 max_segments;
>> +	const u8 max_brightness;
>> +	const u8 max_key_rows;
>> +	const u8 max_key_cols;
>
>What are const above supposed to achieve?
>

My intent was to mark these as immutable controller characteristics, but I'll
remove them as they don't provide meaningful protection here.

>> +	int (*const init)(struct tm16xx_display *display);
>> +	int (*const data)(struct tm16xx_display *display, u8 index, unsigned int grid);
>> +	int (*const keys)(struct tm16xx_display *display);
>> +};
>
>...
>
>> +struct tm16xx_display {
>> +	struct device *dev;
>
>Missing forward declaration.
>

Will add forward declaration.

>> +	const struct tm16xx_controller *controller;
>> +	struct linedisp linedisp;
>> +	u8 *spi_buffer;
>> +	u8 num_hwgrid;
>> +	u8 num_hwseg;
>> +	struct led_classdev main_led;
>> +	struct tm16xx_led *leds;
>> +	u8 num_leds;
>> +	struct tm16xx_digit *digits;
>> +	u8 num_digits;
>> +	struct work_struct flush_init;
>> +	struct work_struct flush_display;
>> +	int flush_status;
>> +	struct mutex lock; /* prevents concurrent work operations */
>> +	unsigned long *state;
>> +};
>
>> +#endif /* _TM16XX_H */
>
>...
>
>> +#include <linux/bitfield.h>
>> +#include <linux/bitmap.h>
>> +#include <linux/cleanup.h>
>> +#include <linux/container_of.h>
>> +#include <linux/device.h>
>> +#include <linux/leds.h>
>> +#include <linux/map_to_7segment.h>
>> +#include <linux/module.h>
>> +#include <linux/property.h>
>> +#include <linux/sysfs.h>
>
>+ types.h
>

Will add missing include.

>> +#include <linux/workqueue.h>
>
>> +#include "line-display.h"
>
>I would add a blank line here as well.
>

Will add blank line.

>> +#include "tm16xx.h"
>
>...
>
>> +#define linedisp_to_tm16xx(display) \
>> +	container_of(display, struct tm16xx_display, linedisp)
>
>One line, we are using 100 limit here.
>

Will format to single line within 100 char limit.

>...
>
>> +/**
>> + * tm16xx_set_seg() - Set the display state for a specific grid/segment
>> + * @display: pointer to tm16xx_display
>> + * @hwgrid: grid index
>> + * @hwseg: segment index
>> + * @on: true to turn on, false to turn off
>
>Can also be %true and %false. This helps the rendering to use different font
>settings for the constants (where applicable).
>

Will use kernel-doc formatting conventions.

>> + */
>> +static inline void tm16xx_set_seg(const struct tm16xx_display *display,
>> +				  const u8 hwgrid, const u8 hwseg, const bool on)
>> +{
>> +	assign_bit(hwgrid * display->num_hwseg + hwseg, display->state, on);
>
>Do you need an atomic call here? Perhaps __assign_bit() would suffice,
>

Keeping assign_bit(), it's required here. Two distinct concurrency scenarios
exist:
- Bitmap: Multiple LED triggers (network, timer) + userspace write to
  display->state concurrently -> need atomic ops
- Hardware: Mutex serializes different hardware operations (flush_init,
  flush_display, keypad polling) that can race
The mutex doesn't eliminate bitmap concurrency needs, they're orthogonal
concerns.

>> +}
>
>...
>
>> +static inline unsigned int tm16xx_get_grid(const struct tm16xx_display *display,
>> +					   const unsigned int index)
>> +{
>> +	return bitmap_read(display->state, index * display->num_hwseg,
>> +			   display->num_hwseg);
>
>One line.
>

Will format to single line within 100 char limit.

>> +}
>
>...
>
>> +static void tm16xx_display_flush_init(struct work_struct *work)
>> +{
>> +	struct tm16xx_display *display = container_of(work,
>> +						      struct tm16xx_display,
>> +						      flush_init);
>
>I slightly prefer
>
>	struct tm16xx_display *display =
>		container_of(work, struct tm16xx_display, flush_init);
>
>Or even a single line.
>

Will format to single line within 100 char limit.

>
>> +	int ret;
>> +
>> +	if (display->controller->init) {
>> +		scoped_guard(mutex, &display->lock) {
>> +			ret = display->controller->init(display);
>> +			display->flush_status = ret;
>> +		}
>> +		if (ret)
>> +			dev_err(display->dev,
>> +				"Failed to configure controller: %d\n", ret);
>> +	}
>
>First of all, I'm not sure what the lock is protecting. Here you allow "init" to
>be whatever, while in the below code the "data" is protected.
>

The mutex serializes different hardware operation types occurring concurrently:
brightness changes (flush_init), display updates (flush_display), and keypad
polling. The workqueue prevents concurrent execution of the same work, not
different operations.

>Second, I haven't seen changes in this function later in the series, so perhaps
>drop the indentation by negating conditional?
>

Will refactor with early return to reduce indentation.

>> +}
>
>> +/**
>> + * tm16xx_display_flush_data() - Workqueue to update display data to controller
>> + * @work: pointer to work_struct
>
>Perhaps add a small description and explain that this is interrupted if an
>error occurs and that error will be stored for further use by upper layers.
>
>Does the same apply to the above function?
>

Will add descriptions explaining error handling behavior for both functions.

>> + */
>> +static void tm16xx_display_flush_data(struct work_struct *work)
>> +{
>> +	struct tm16xx_display *display = container_of(work,
>> +						      struct tm16xx_display,
>> +						      flush_display);
>> +	unsigned int grid, i;
>> +	int ret = 0;
>
>> +	scoped_guard(mutex, &display->lock) {
>
>As per above, and here AFAICS guard()() will suit better.
>

Will change to guard()() here.

>> +		if (display->controller->data) {
>> +			for (i = 0; i < display->num_hwgrid; i++) {
>> +				grid = tm16xx_get_grid(display, i);
>> +				ret = display->controller->data(display, i, grid);
>> +				if (ret) {
>> +					dev_err(display->dev,
>> +						"Failed to write display data: %d\n",
>> +						ret);
>> +					break;
>> +				}
>> +			}
>> +		}
>> +
>> +		display->flush_status = ret;
>> +	}
>> +}
>
>...
>
>> +static void tm16xx_brightness_set(struct led_classdev *led_cdev,
>> +				  enum led_brightness brightness)
>
>One line
>

Will format to single line within 100 char limit.

>...
>
>> +static void tm16xx_led_set(struct led_classdev *led_cdev,
>> +			   enum led_brightness value)
>
>Ditto.
>

Will format to single line within 100 char limit.

>...
>
>> +static int tm16xx_display_value(struct tm16xx_display *display, const char *buf, size_t count)
>> +{
>> +	struct linedisp *linedisp = &display->linedisp;
>> +	struct linedisp_map *map = linedisp->map;
>> +	struct tm16xx_digit *digit;
>> +	unsigned int i, j;
>
>> +	int seg_pattern;
>
>Hmm... Should it be signed?
>

Keeping signed, map_to_seg7() returns int per its API contract.

>> +	bool val;
>
>> +	for (i = 0; i < display->num_digits && i < count; i++) {
>
>This means "whatever is smaller", perhaps make it clearer by using min() ?
>
>> +		digit = &display->digits[i];
>> +		seg_pattern = map_to_seg7(&map->map.seg7, buf[i]);
>> +
>> +		for (j = 0; j < TM16XX_DIGIT_SEGMENTS; j++) {
>> +			val = seg_pattern & BIT(j);
>> +			tm16xx_set_seg(display, digit->hwgrids[j], digit->hwsegs[j], val);
>> +		}
>> +	}
>> +
>> +	for (; i < display->num_digits; i++) {
>> +		digit = &display->digits[i];
>> +		for (j = 0; j < TM16XX_DIGIT_SEGMENTS; j++)
>> +			tm16xx_set_seg(display, digit->hwgrids[j], digit->hwsegs[j], 0);
>> +	}
>
>Or unite these two for-loops into a single one with i < count conditional embedded?
>
>		for (j = 0; j < TM16XX_DIGIT_SEGMENTS; j++) {
>			if (i < count)
>				val = seg_pattern & BIT(j);
>			else
>				val = 0;
>			tm16xx_set_seg(display, digit->hwgrids[j], digit->hwsegs[j], val);
>		}
>
>?
>

Will merge loops with embedded conditional as suggested.

>> +	schedule_work(&display->flush_display);
>> +	return 0;
>> +}
>
>...
>
>> +static int tm16xx_parse_fwnode(struct device *dev, struct tm16xx_display *display)
>> +{
>> +	struct tm16xx_led *led;
>> +	struct tm16xx_digit *digit;
>> +	unsigned int max_hwgrid = 0, max_hwseg = 0;
>> +	unsigned int i, j;
>> +	int ret;
>> +	u32 segments[TM16XX_DIGIT_SEGMENTS * 2];
>> +	u32 reg[2];
>> +
>> +	struct fwnode_handle *digits_node __free(fwnode_handle) =
>> +		device_get_named_child_node(dev, "digits");
>> +	struct fwnode_handle *leds_node __free(fwnode_handle) =
>> +		device_get_named_child_node(dev, "leds");
>> +
>> +	/* parse digits */
>> +	if (digits_node) {
>> +		display->num_digits = fwnode_get_child_node_count(digits_node);
>
>> +		if (display->num_digits) {
>
>Drop an indentation level by splitting this to a helper.
>

Will extract digits node/num parsing into helper function.

>> +			display->digits = devm_kcalloc(dev, display->num_digits,
>> +						       sizeof(*display->digits),
>> +						       GFP_KERNEL);
>> +			if (!display->digits)
>> +				return -ENOMEM;
>> +
>> +			i = 0;
>> +			fwnode_for_each_available_child_node_scoped(digits_node, child) {
>> +				digit = &display->digits[i];
>> +
>> +				ret = fwnode_property_read_u32(child, "reg", reg);
>> +				if (ret)
>> +					return ret;
>> +
>> +				ret = fwnode_property_read_u32_array(child,
>> +								     "segments", segments,
>> +								     TM16XX_DIGIT_SEGMENTS * 2);
>
>> +				if (ret < 0)
>> +					return ret;
>
>Why '< 0'? Here it's definitely not a counting call, so it should never return
>positive in this case.
>

Keeping if (ret < 0). While usage with non-NULL buffer won't return positive
values, fwnode_property_read_u32_array() documentation explicitly states it can
return count when buffer is NULL. Using < 0 is the defensive, API-compliant
pattern that matches the function signature.

>> +
>> +				for (j = 0; j < TM16XX_DIGIT_SEGMENTS; ++j) {
>> +					digit->hwgrids[j] = segments[2 * j];
>> +					digit->hwsegs[j] = segments[2 * j + 1];
>> +					max_hwgrid = umax(max_hwgrid, digit->hwgrids[j]);
>> +					max_hwseg = umax(max_hwseg, digit->hwsegs[j]);
>> +				}
>> +				i++;
>> +			}
>> +		}
>> +	}
>> +
>> +	/* parse leds */
>> +	if (leds_node) {
>> +		display->num_leds = fwnode_get_child_node_count(leds_node);
>
>> +		if (display->num_leds) {
>
>Ditto.
>

Will extract leds node/num parsing into helper function.

>> +			display->leds = devm_kcalloc(dev, display->num_leds,
>> +						     sizeof(*display->leds),
>> +						     GFP_KERNEL);
>> +			if (!display->leds)
>> +				return -ENOMEM;
>> +
>> +			i = 0;
>> +			fwnode_for_each_available_child_node_scoped(leds_node, child) {
>> +				led = &display->leds[i];
>> +				ret = fwnode_property_read_u32_array(child, "reg", reg, 2);
>> +				if (ret < 0)
>
>Ditto,.
>

As per above.

>> +					return ret;
>> +
>> +				led->hwgrid = reg[0];
>> +				led->hwseg = reg[1];
>> +				max_hwgrid = umax(max_hwgrid, led->hwgrid);
>> +				max_hwseg = umax(max_hwseg, led->hwseg);
>> +				i++;
>> +			}
>> +		}
>> +	}
>> +
>> +	if (max_hwgrid >= display->controller->max_grids) {
>> +		dev_err(dev, "grid %u exceeds controller max_grids %u\n",
>> +			max_hwgrid, display->controller->max_grids);
>> +		return -EINVAL;
>> +	}
>> +
>> +	if (max_hwseg >= display->controller->max_segments) {
>> +		dev_err(dev, "segment %u exceeds controller max_segments %u\n",
>> +			max_hwseg, display->controller->max_segments);
>> +		return -EINVAL;
>> +	}
>> +
>> +	display->num_hwgrid = max_hwgrid + 1;
>> +	display->num_hwseg = max_hwseg + 1;
>> +
>> +	return 0;
>> +}
>
>...
>
>> +/**
>> + * tm16xx_probe() - Probe and initialize display device, register LEDs
>> + * @display: pointer to tm16xx_display
>> + *
>> + * Return: 0 on success, negative error code on failure
>> + */
>
>Unneeded kernel-doc.
>

Will remove kernel-doc.

>> +int tm16xx_probe(struct tm16xx_display *display)
>> +{
>> +	struct device *dev = display->dev;
>> +	struct led_classdev *main = &display->main_led;
>> +	struct led_init_data led_init = {0};
>
>'0' is not needed.
>

Will use empty braces:
struct led_init_data led_init = {};

>> +	struct fwnode_handle *leds_node;
>> +	struct tm16xx_led *led;
>> +	unsigned int nbits, i;
>> +	int ret;
>> +
>> +	ret = tm16xx_parse_fwnode(dev, display);
>> +	if (ret)
>> +		return dev_err_probe(dev, ret, "Failed to parse device tree\n");
>> +
>> +	nbits = tm16xx_led_nbits(display);
>> +	display->state = devm_bitmap_zalloc(dev, nbits, GFP_KERNEL);
>> +	if (!display->state)
>> +		return -ENOMEM;
>> +
>> +	ret = devm_mutex_init(display->dev, &display->lock);
>> +	if (ret)
>> +		return dev_err_probe(dev, ret, "Failed to initialize mutex\n");
>
>I believe it's ENOMEM here, so we don't need an error message.
>

You are right, underlaying __devm_add_action() only returns -ENOMEM.
Will remove this dev_err_probe().

>> +	INIT_WORK(&display->flush_init, tm16xx_display_flush_init);
>> +	INIT_WORK(&display->flush_display, tm16xx_display_flush_data);
>
>devm-helpers.h have something for this case, I believe.
>

Cannot use devm_work_autocancel(). The shutdown sequence requires specific
ordering: (1) unregister LEDs to stop triggers, (2) clear display state, (3)
flush pending work, (4) turn off display. This sequence prevents hardware
access races when triggers attempt to update the display during removal. Manual
INIT_WORK with explicit flush/cancel in remove() provides this control.

>> +	/* Initialize main LED properties */
>> +	led_init.fwnode = dev_fwnode(dev); /* apply label property */
>
>I didn't get a comment. This not only about label, but for entire set of
>properties that led framework can consume.
>

Will remove /* apply label property */

>> +	main->max_brightness = display->controller->max_brightness;
>> +	device_property_read_u32(dev, "max-brightness", &main->max_brightness);
>> +	main->max_brightness = umin(main->max_brightness,
>> +				    display->controller->max_brightness);
>
>Hmm... Why 'u' variant of macro?
>
>
>> +	main->brightness = main->max_brightness;
>> +	device_property_read_u32(dev, "default-brightness", &main->brightness);
>> +	main->brightness = umin(main->brightness, main->max_brightness);
>
>Ditto.
>

Correct for unsigned brightness values. umin() is the appropriate macro for
unsigned types to avoid type conversion warnings.

>Given a comment about propagating fwnode, why do we need all this? Doesn't led
>core take care of these properties as well?
>

Manual handling is necessary because:
1. default-brightness: Not implemented in LED core
2. max-brightness defaulting: If DT property is absent, default to
   controller->max_brightness
3. Ceiling enforcement: When DT property IS present, clamp to not exceed
   hardware limits (controller->max_brightness)

LED core only reads max-brightness optionally, it doesn't handle defaulting or
hardware ceiling enforcement.

>> +	main->brightness_set = tm16xx_brightness_set;
>> +	main->flags = LED_RETAIN_AT_SHUTDOWN | LED_CORE_SUSPENDRESUME;
>> +
>> +	/* Register individual LEDs from device tree */
>> +	ret = led_classdev_register_ext(dev, main, &led_init);
>> +	if (ret)
>> +		return dev_err_probe(dev, ret, "Failed to register main LED\n");
>> +
>> +	i = 0;
>> +	led_init.devicename = dev_name(main->dev);
>> +	led_init.devname_mandatory = true;
>> +	led_init.default_label = "led";
>> +	leds_node = device_get_named_child_node(dev, "leds");
>> +	fwnode_for_each_available_child_node_scoped(leds_node, child) {
>> +		led_init.fwnode = child;
>> +		led = &display->leds[i];
>
>> +		led->cdev.max_brightness = 1;
>
>That should be set to default by the led core based on the property value, not the case?
>

Individual icons are hardware-constrained to on/off (max_brightness = 1)
regardless of DT properties. This enforces hardware limits, not reads
properties.

>> +		led->cdev.brightness_set = tm16xx_led_set;
>> +		led->cdev.flags = LED_RETAIN_AT_SHUTDOWN | LED_CORE_SUSPENDRESUME;
>> +
>> +		ret = led_classdev_register_ext(dev, &led->cdev, &led_init);
>
>Why not devm_led_*()?
>

Intentional non-devm design documented in commit notes. Explicit unregistration
before removal immediately stops LED triggers, preventing them from accessing
hardware post-removal. devm_led_*() would require complex brightness callback
state tracking to handle trigger activity during remove(). Explicit unregister
is cleaner and eliminates this race.

>> +		if (ret) {
>> +			dev_err_probe(dev, ret, "Failed to register LED %s\n",
>> +				      led->cdev.name);
>> +			goto unregister_leds;
>> +		}
>> +
>> +		i++;
>> +	}
>> +
>> +	ret = tm16xx_display_init(display);
>> +	if (ret) {
>> +		dev_err_probe(dev, ret, "Failed to initialize display\n");
>> +		goto unregister_leds;
>> +	}
>
>> +	ret = linedisp_attach(&display->linedisp, display->main_led.dev,
>> +			      display->num_digits, &tm16xx_linedisp_ops);
>> +	if (ret) {
>> +		dev_err_probe(dev, ret, "Failed to initialize line-display\n");
>> +		goto unregister_leds;
>> +	}
>
>If we haven't yet devm for this, it can be
>1) introduced, OR
>2) wrapped to become a such (see devm_add_action_or_reset() usage).
>

While devm_add_action_or_reset() could wrap linedisp_detach(), the overall
shutdown still requires explicit ordering across multiple subsystems (linedisp,
LEDs, workqueues, hardware). Using devm for just one component while manually
managing others adds complexity without benefit. The current explicit approach
keeps all cleanup logic together in remove() for clarity.

>> +	return 0;
>> +
>> +unregister_leds:
>> +	while (i--)
>> +		led_classdev_unregister(&display->leds[i].cdev);
>> +
>> +	led_classdev_unregister(main);
>> +	return ret;
>> +}
>> +EXPORT_SYMBOL_NS(tm16xx_probe, "TM16XX");
>
>Needs to be namespaced _GPL variant. Same for all exports.
>

Will change all exports to
EXPORT_SYMBOL_NS_GPL(symbol, "TM16XX").

>> +/**
>> + * tm16xx_remove() - Remove display, unregister LEDs, blank output
>> + * @display: pointer to tm16xx_display
>> + */
>
>Unneeded kernel-doc.
>

Will remove kernel-doc.

>> +void tm16xx_remove(struct tm16xx_display *display)
>> +{
>> +	unsigned int nbits = tm16xx_led_nbits(display);
>> +	struct tm16xx_led *led;
>> +
>> +	linedisp_detach(display->main_led.dev);
>
>> +	/*
>> +	 * Unregister LEDs first to immediately stop trigger activity.
>> +	 * This prevents LED triggers from attempting to access hardware
>> +	 * after it's been disconnected or driver unloaded.
>> +	 */
>
>After switching to devm_*() this comment won't be needed (besides that it will
>come orphaned).
>

Should not switch to devm_*() as explained above.

>> +	for (int i = 0; i < display->num_leds; i++) {
>> +		led = &display->leds[i];
>> +		led_classdev_unregister(&led->cdev);
>> +	}
>> +	led_classdev_unregister(&display->main_led);
>> +
>> +	/* Clear display state */
>> +	bitmap_zero(display->state, nbits);
>> +	schedule_work(&display->flush_display);
>> +	flush_work(&display->flush_display);
>> +
>> +	/* Turn off display */
>> +	display->main_led.brightness = LED_OFF;
>> +	schedule_work(&display->flush_init);
>> +	flush_work(&display->flush_init);
>> +}
>> +EXPORT_SYMBOL_NS(tm16xx_remove, "TM16XX");
>
>_GPL
>

Will change all exports to
EXPORT_SYMBOL_NS_GPL(symbol, "TM16XX").

Best Regards,

Jean-François Lessard
Hi Andy,

Thanks for your feedback.

Powered by blists - more mailing lists