linux-kernel - Question about cleaning up the LP5521/5523 LED driver

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

Message-ID: <A874F61F95741C4A9BA573A70FE3998F41EF514F@DQHE02.ent.ti.com>
Date:	Mon, 17 Sep 2012 11:28:47 +0000
From:	"Kim, Milo" <Milo.Kim@...com>
To:	Bryan Wu <bryan.wu@...onical.com>
CC:	"linux-leds@...r.kernel.org" <linux-leds@...r.kernel.org>,
	"linus.walleij@...aro.org" <linus.walleij@...aro.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Question about cleaning up the LP5521/5523 LED driver

Hi Bryan,

I would like to have your opinion about cleaning up 
LP5521 and LP5523/LP55231 drivers.

Three devices have similar features such like multi channels,
register access via the I2C and internal RAM usage for loading
various LED patterns.
Moreover upcoming IC has also similar layout and interface.

So I would re-design the driver architecture as below.

- Common code which supports shared features.
- Chip code which enables specific features.

For example, leds-lp55xx-common.c is common code.
Chip specific configurations are leds-lp5521.c, leds-lp5523.c and something like that.
Then Makefile will be as following.

obj-$(CONFIG_LEDS_LP5521) 	+= leds-lp5521.o leds-lp55xx-common.o
obj-$(CONFIG_LEDS_LP5523)	+= leds-lp5523.o leds-lp55xx-common.o
obj-$(CONFIG_LEDS_LP55AB)	+= leds-lp55ab.o leds-lp55xx-common.o

Then many duplicated lines can be joined into one file - common code.
Only IC specific code exists each chip dependant file.

But I'm hesitating that some *device attributes* should be kept or not.
For example, The drivers have each 'engine_load' file for loading LED pattern.
LED pattern means blinking LED or ramping up/down the brightness.
To load pattern simply, LP5521/5523/55231 provide programmable memory.

I'll replace these attributes with the firmware interfaces for simplicity.
To load a pattern, hex byte stream is put into the sysfs node.
Then this data is transferred to the internal RAM area of the device.
This is same concept as the firmware interface.
Moreover hex code is dependent on the application or platform.

But it has a problem, point of view of user-space interface.
An application needs to be changed also with the driver version.
The application will failed to run because 'engine_mode' and
'engine_load' don't exist anymore.

Should I keep the sysfs layout of LP5521 and LP5523?
When we change the device attributes, do we have to consider the backward compatibility?

Thank you.

Best Regards,
Milo

--
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/