From: "Maximilian Güntner" <maximilian.guentner@gmail.com>
To: Peter Meerwald <pmeerw@pmeerw.net>
Cc: Bryan Wu <cooloney@gmail.com>, Richard Purdie <rpurdie@rpsys.net>,
linux-kernel <linux-kernel@vger.kernel.org>,
linux-leds <linux-leds@vger.kernel.org>
Subject: Re: [PATCH] leds: Added driver for the NXP PCA9685 I2C chip
Date: Tue, 15 Oct 2013 01:26:16 +0200 [thread overview]
Message-ID: <3285547.78o0xYBaXq@titan> (raw)
In-Reply-To: <alpine.DEB.2.01.1310150015370.26447@pmeerw.net>
On Tuesday 15 October 2013 00:38:13 Peter Meerwald wrote:
> > The NXP PCA9685 supports 16 channels/leds using a 12-bit PWM (4095
> > levels of brightness)
> > This driver supports configuration using platform_data.
>
> some nitpicking inline
Thank you for your comments.
> chip
fixed.
>
>> + * Driver for the NXP PCA9685 12-Bit I2C chip.
> this could be more descriptive: 12-bit PWM LED driver chip
fixed.
> > + /*
> > + * 4095 is the maximum brightness, so we set the ON time to 0x1000
> > + * which disables the PWM generator for that LED
> > + */
> > + if (pca9685->brightness == 4095)
> > + brightness = 4096;
>
> why not check for 4095 instead of 4096 below and save this if statement?
removed.
I left it there in order to improve the code readability, which should be equally
good when we just remove the statement and check for 4095 below.
> > + i2c_smbus_write_byte_data(client, PCA9685_MODE1, 1 << PCA9685_AI);
>
> maybe BIT(PCA9685_AI)
replaced.
next prev parent reply other threads:[~2013-10-14 23:26 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-10-14 17:31 [PATCH] leds: Added driver for the NXP PCA9685 I2C chip Maximilian Güntner
2013-10-14 22:08 ` Bryan Wu
2013-10-14 23:32 ` Maximilian Güntner
2013-10-15 1:19 ` [PATCH v2] " Maximilian Güntner
2013-10-15 17:39 ` Bryan Wu
2013-10-17 1:09 ` [PATCH v3] " Maximilian Güntner
2013-10-17 22:37 ` Bryan Wu
2013-10-17 23:00 ` Maximilian Güntner
2013-10-14 22:38 ` [PATCH] " Peter Meerwald
2013-10-14 23:26 ` Maximilian Güntner [this message]
2013-10-15 0:37 ` Jingoo Han
[not found] <525d89e8.CwG1bICTvwrMEEOp%fengguang.wu@intel.com>
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=3285547.78o0xYBaXq@titan \
--to=maximilian.guentner@gmail.com \
--cc=cooloney@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-leds@vger.kernel.org \
--cc=pmeerw@pmeerw.net \
--cc=rpurdie@rpsys.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox