linux-kernel - Re: [PATCH v2 1/3] leds-pca9633: Add support for PCA9634

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

Date:	Fri, 9 Aug 2013 08:43:16 +0200
From:	Ricardo Ribalda Delgado <ricardo.ribalda@...il.com>
To:	Bryan Wu <cooloney@...il.com>
Cc:	Peter Meerwald <p.meerwald@...-electronic.com>,
	Richard Purdie <rpurdie@...ys.net>,
	Linux LED Subsystem <linux-leds@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 1/3] leds-pca9633: Add support for PCA9634

Hello Bryan

I understand your concerns, but I think that it is more practical to
keep the old names.

First of all there will be some defconfig that we will break with the
new names, the same will happen with the platform data. I know that we
don't have to support out of tree systems, but it wont hurt to make
their live easier.

The patch produced when we rename a file is as big as the file itself,
so it is more difficult to get good reviews, compared to a 10 lines
patch.

Unless the manufacturer makes a statement, there is no way to know if
there will be a pca96339 that will be a completely different chip, and
that will led to many errors.

Finally, there are many examples of files named as the first device
supported, even on the led infrastructure (ie. LEDS_DA9052)

All that said, I have added a new patch to the series that does the
renaming :), therefore we can review the addition 96334 separately
than the other improvements and if we decide to have a generic name we
will have it.

Thanks for your review!

On Fri, Aug 9, 2013 at 12:49 AM, Bryan Wu <cooloney@...il.com> wrote:
> On Thu, Aug 8, 2013 at 3:36 PM, Peter Meerwald
> <p.meerwald@...-electronic.com> wrote:
>> Hello,
>>
>>> > Add support for PCA9634 chip, which belongs to the same family as the
>>> > 9633 but with support for 8 outputs instead of 4.
>>
>>> Basically I like this method to add a new chip supporting. Please find
>>> my comments below.
>>
>> me too :)
>>
>>> What about just rename the whole file to leds-pca963x.c. And rename
>>> some pca9633 to pca963x in the driver.
>>
>> there are other, similar I2C LED driver chips which might be
>> handled with the current pca9633 driver, e.g. the pca9685 (which is
>> supported under pwm/ by the way)
>>
>> people have argued that the numbering scheme of chips is hard to
>> predict; hence, the driver name should be determined by the first
>> device supported to avoid subsequent renaming -- but I have no strong
>> feelings about this
>>
>
> Giving a more generic and meaningful name of this driver should be a
> easier for people to understand. So if pca9685 is coming, we can use
> leds-pca96xx.c and pca96xx for this pca96xx chip family, as long as
> they can share this driver code.
>
> Please take a look at Milo did in leds-lp55xx drivers.
>
> Thanks a lot,
> -Bryan

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