From: Pavel Machek <pavel@ucw.cz>
To: Dan Murphy <dmurphy@ti.com>
Cc: jacek.anaszewski@gmail.com, linux-leds@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v20 03/17] leds: multicolor: Introduce a multicolor class definition
Date: Tue, 28 Apr 2020 10:41:59 +0200 [thread overview]
Message-ID: <20200428084159.GB20640@amd> (raw)
In-Reply-To: <80e20291-0ff2-87e6-8f93-2f37f588b148@ti.com>
[-- Attachment #1: Type: text/plain, Size: 1927 bytes --]
Hi!
> >>+cat /sys/class/leds/multicolor:status/multi_led_index
> >>+green blue red
> >Hmm. We should really make sure LEDs are ordered as "red green
> >blue". Yes, userspace should support any order, but...
>
> Ordering is not guaranteed since it is based on the DT ordering. I don't
> think we can mandate that these LEDs be put in order in the DT.
>
> Besides the framework and the device driver do not care what color is where
> only the user space needs to care. The FW and device driver only care about
> the brightness, intensity and channel.
Ok, lets keep it like this.
> >>+ offset += nrchars;
> >>+ }
> >This checks for "not enough" intensities. Do we need check for "too
> >many" intensities?
>
> We ignore anything greater then mcled_cdev->num_colors. So if this is set
> to 3 we only read the first 3 values.
>
> So we cannot read more then what is set by the DT.
Please make it return an error if extra values are passed in.
> >>+static ssize_t multi_led_intensity_show(struct device *dev,
> >>+ struct device_attribute *intensity_attr,
> >>+ char *buf)
> >>+{
> >>+ struct led_classdev *led_cdev = dev_get_drvdata(dev);
> >>+ struct led_classdev_mc *mcled_cdev = lcdev_to_mccdev(led_cdev);
> >>+ int len = 0;
> >>+ int i;
> >>+
> >>+ for (i = 0; i < mcled_cdev->num_colors; i++)
> >>+ len += sprintf(buf + len, "%d ",
> >>+ mcled_cdev->multicolor_info[i].color_led_intensity);
> >>+
> >>+ len += sprintf(buf + len, "%s", "\n");
> >This will result in extra " " before end of line.
> >
> >Please don't use "%s", "\n" to add single character. "\n" would be enough.
> Ack changed to just sprintf(buf + len, "\n");
Also note the extra space before end of line.
Best regards,
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 --]
next prev parent reply other threads:[~2020-04-28 8:42 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-23 15:55 [PATCH v20 00/17] Multicolor Framework (array edition) Dan Murphy
2020-04-23 15:55 ` [PATCH v20 01/17] dt: bindings: Add multicolor class dt bindings documention Dan Murphy
2020-04-23 15:55 ` [PATCH v20 02/17] leds: Add multicolor ID to the color ID list Dan Murphy
2020-04-25 19:52 ` Pavel Machek
2020-04-27 17:12 ` Dan Murphy
2020-04-28 8:43 ` Pavel Machek
2020-04-28 12:02 ` Dan Murphy
2020-04-23 15:55 ` [PATCH v20 03/17] leds: multicolor: Introduce a multicolor class definition Dan Murphy
2020-04-25 19:53 ` Pavel Machek
2020-04-27 17:12 ` Dan Murphy
2020-04-25 20:23 ` Pavel Machek
2020-04-26 16:24 ` Jacek Anaszewski
2020-04-26 19:48 ` Pavel Machek
2020-04-27 19:33 ` Dan Murphy
2020-04-28 8:41 ` Pavel Machek [this message]
2020-04-26 16:46 ` Jacek Anaszewski
2020-04-23 15:55 ` [PATCH v20 04/17] dt: bindings: lp50xx: Introduce the lp50xx family of RGB drivers Dan Murphy
2020-04-25 20:24 ` Pavel Machek
2020-04-27 19:02 ` Dan Murphy
2020-04-23 15:55 ` [PATCH v20 05/17] leds: lp50xx: Add the LP50XX family of the RGB LED driver Dan Murphy
2020-04-23 15:55 ` [PATCH v20 06/17] dt: bindings: lp55xx: Be consistent in the document with LED acronym Dan Murphy
2020-04-23 15:55 ` [PATCH v20 07/17] dt: bindings: lp55xx: Update binding for Multicolor Framework Dan Murphy
2020-04-23 15:55 ` [PATCH v20 08/17] ARM: dts: n900: Add reg property to the LP5523 channel node Dan Murphy
2020-04-23 15:55 ` [PATCH v20 09/17] ARM: dts: imx6dl-yapp4: Add reg property to the lp5562 " Dan Murphy
2020-04-25 20:31 ` Pavel Machek
2020-04-23 15:55 ` [PATCH v20 10/17] ARM: dts: ste-href: Add reg property to the LP5521 channel nodes Dan Murphy
2020-04-27 19:06 ` Dan Murphy
2020-04-23 15:55 ` [PATCH v20 11/17] leds: lp55xx: Convert LED class registration to devm_* Dan Murphy
2020-04-23 15:55 ` [PATCH v20 12/17] leds: lp55xx: Add multicolor framework support to lp55xx Dan Murphy
2020-04-25 20:37 ` Pavel Machek
2020-04-26 16:07 ` Jacek Anaszewski
2020-04-27 18:17 ` Dan Murphy
2020-04-27 19:42 ` Jacek Anaszewski
2020-04-27 19:58 ` Dan Murphy
2020-04-23 15:55 ` [PATCH v20 13/17] leds: lp5523: Update the lp5523 code to add multicolor brightness function Dan Murphy
2020-04-25 20:38 ` Pavel Machek
2020-04-23 15:55 ` [PATCH v20 14/17] leds: lp5521: Add multicolor framework multicolor brightness support Dan Murphy
2020-04-25 20:39 ` Pavel Machek
2020-04-23 15:55 ` [PATCH v20 15/17] leds: lp55xx: Fix checkpatch file permissions issues Dan Murphy
2020-04-23 15:55 ` [PATCH v20 16/17] leds: lp5523: Fix checkpatch issues in the code Dan Murphy
2020-04-23 15:55 ` [PATCH v20 17/17] dt: bindings: Update lp55xx binding to recommended LED naming Dan Murphy
2020-04-25 20:48 ` [PATCH v20 00/17] Multicolor Framework (array edition) Pavel Machek
2020-04-27 17:18 ` Dan Murphy
2020-04-27 19:05 ` Dan Murphy
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=20200428084159.GB20640@amd \
--to=pavel@ucw.cz \
--cc=dmurphy@ti.com \
--cc=jacek.anaszewski@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-leds@vger.kernel.org \
/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;
as well as URLs for NNTP newsgroup(s).