Re: [PATCH v7 1/1] leds: LED driver for TI LP3952 6-Channel Color LED

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 07/06/2016 11:15 PM, Rafael J. Wysocki wrote:
On Wednesday, July 06, 2016 03:32:25 PM Jacek Anaszewski wrote:
On 07/05/2016 02:33 PM, Jacek Anaszewski wrote:
On 07/05/2016 02:12 PM, Rafael J. Wysocki wrote:
On Tue, Jul 5, 2016 at 12:47 PM, Jacek Anaszewski
<j.anaszewski@xxxxxxxxxxx> wrote:
PING


On 06/29/2016 12:54 PM, Jacek Anaszewski wrote:

On 06/29/2016 12:07 PM, Tony wrote:


On 29/06/16 07:52, Jacek Anaszewski wrote:

Hi Rafael,


+#ifdef CONFIG_ACPI
+static const struct acpi_device_id lp3952_acpi_match[] = {
+    {LP3952_ACPI_NAME, 0},


No, you can't use "PRP0001" in this list.

+    {}
+};
+
+MODULE_DEVICE_TABLE(acpi, lp3952_acpi_match);


And you don't need this for the "PRP0001" thing to work.  The
core will
take care of it for you then.

+#endif


So the entire ACPI block can be dropped for now.

And the driver doesn't have to depend on CONFIG_ACPI any more,
does it?


The driver currently supports probing only with ACPI.
I have one question BTW: isn't there anything similar to the
device tree
bindings documentation required for ACPI overlays?
Pointer to the discussion which led us to this solution:

http://www.spinics.net/lists/linux-leds/msg06230.html

_DSD is working now. I managed to get "PRP0001" working as
suggested by
Rafael in
http://marc.info/?l=linux-acpi&m=146711623115228&w=2
with _DSD


Thanks for the link.

Rafael, "Package" entries seem to mimic Device Tree properties defined
in the common leds bindings. Would it be possible to make it even
more compatible and define every LED connected to the LED controller
in the form of a child node, similarly as in case of LED DT bindings?

See Documentation/devicetree/bindings/leds/common.txt and other
bindings in Documentation/devicetree/bindings/leds.

I'm not sure what you mean.

If somebody decides to arrange the data in their ACPI tables to follow
that scheme, it will just work IMO.

Is there anything more that needs to be done in the kernel here?

In case of Device Tree based platforms it is customary to define each
LED as a child node of the LED controller node. LED names can then
be obtained from the 'label' property or DT node name.
Tony has encountered some issues [1] while trying to follow this
pattern. Tony, could you please double check that?

[1] http://www.spinics.net/lists/linux-leds/msg06230.html

One more question: now anyone wanting to use the driver would have
to investigate the code to infer what DSD properties need to be
provided for it. Isn't there anything similar to
Documentation/devicetree/bindings/* documentation required in case of
DSD?

There is a plan to set up a database for these, but that's still
under discussion (Al Stone has posted some related documentation for
comments recently).

I've also come across your presentation [1], which describes the
device_property* API for parsing unified DT/ACPI device properties.
IMO it would be good solution for this driver as the device seems not
to be tightly coupled with only ACPI platforms.


[1]
http://events.linuxfoundation.org/sites/events/files/slides/ACPI_vs_DT.pdf

The device_property* API should be used everywhere where (a) properties are
used and (b) the code has no specific need to use a particular platform
firmware interface (ACPI or DT), so I agree.

Tony, could you try switching to device_property* API and make
the DSD properties as much compatible with Device Tree ones as possible?

We haven't tried it in the LED subsystem so far but it is a good
occasion to do so.

--
Best regards,
Jacek Anaszewski
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux