Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* Re: [PATCH 3/9] Doc/DT: Add DT binding documentation for DVI Connector
From: Tomi Valkeinen @ 2014-03-10 10:32 UTC (permalink / raw)
  To: Philipp Zabel
  Cc: devicetree@vger.kernel.org, Linux Fbdev development list,
	Russell King - ARM Linux, Sascha Hauer, Tomasz Figa,
	DRI Development, Inki Dae, Andrzej Hajda, Rob Clark,
	Thierry Reding, Geert Uytterhoeven, Laurent Pinchart,
	Daniel Vetter, linux-arm-kernel@lists.infradead.org,
	Sebastian Hesselbarth
In-Reply-To: <1394201859.16309.14.camel@paszta.hi.pengutronix.de>

[-- Attachment #1: Type: text/plain, Size: 1919 bytes --]

On 07/03/14 16:17, Philipp Zabel wrote:
> Hi,
> 
> Am Donnerstag, den 06.03.2014, 10:52 +0200 schrieb Tomi Valkeinen:

>> analog;
>> digital-single-link;
>> digital-dual-link;
>>
>> My reasoning to the format I chose was basically that when a connector
>> supports 'digital', it contains TMDS clock and TMDS data for link 1.
>> Adding dual link to that adds only TMDS data for link 2, so the second
>> data link is kind of an additional feature, marked with a flag.
>>
>> Not a very big argument, and I'm fine with other format suggestions.
> 
> I'd prefer the analog / digital / dual-link variant for aesthetic
> reasons. But looking at other connector types, I wonder if this should
> be generalized even more:
> 
> For HDMI/DVI (digital) single-link means one clock pair and 3 TMDS data
> pairs, dual-link means one clock pair and 6 data pairs.
> 
> On LVDS connectors, there usually are one clock pair and three (18-bit)
> or four (24-bit) LVDS data pairs, in dual channel configuration two
> clock pairs and 6 or 8 data pairs are used.
> 
> For DisplayPort there is no separate clock pair, but 1 to 4 data pairs,
> and MIPI DSI again has one clock pair and a one or more data pairs.
> 
> There are already optional endpoint configuration properties
> 'data-lanes' and 'clock-lanes' for MIPI CSI-2 defined in
> Documentation/devicetree/bindings/media/video-interfaces.txt.
> Could/should this be aligned with the same?

Hmm. Well, at least for HDMI and DP we anyway need the connector type,
which tells the form factor, and it also tells the TMDS details. So, we
could define the lanes in a common way, but we'd still need extra
information.

For MIPI DSI and (I believe) LVDS, we don't need connectors. Connectors,
as described in this binding, are meant for standard hotpluggable
connectors to which you can connect any device that has that same connector.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

^ permalink raw reply

* Re: [PATCH v5 1/2] video: ARM CLCD: Add DT support
From: Pawel Moll @ 2014-03-10 12:09 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <5319AF00.5030602@ti.com>

On Fri, 2014-03-07 at 11:35 +0000, Tomi Valkeinen wrote:
> > ... and this makes me try to get out of its way. In other words, I fully
> > agree with you, but I don't think this applies to this particular patch,
> > as I'm not even trying to represent the display pipeline. The (optional)
> 
> Right, you are not, at the moment. My point was, if the driver is
> extended later to support pipelines, or common panel/encoder drivers,
> you (most likely) need to have similar bindings than the other people use.

No argument here.

> Note that the same DT bindings you add here should also work for the DRM
> driver in the future. So, in fact, the question is extended to: will the
> fbdev 

No, I don't think so. It's a end of line for it, I believe. From my
personal point of view the main goal is to fill the last gap between DT
and non-DT support on one of the Versatile Express boards - CLCD works
fine when booted with ATAGs, doesn't work when booted with DT. It looks
bad.

> or drm driver support common panels/encoders?

I would hope so, yes. VE display pipeline is quite complex (more than I
would hope ;-) so to implement KMS in a proper way (with hotplugging,
bells and whistles) it will have to know more than now. But it's out of
scope for me.

Pawel


^ permalink raw reply

* Re: [PATCH v5 1/2] video: ARM CLCD: Add DT support
From: Tomi Valkeinen @ 2014-03-10 12:23 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1394453382.3853.6.camel@hornet>

[-- Attachment #1: Type: text/plain, Size: 824 bytes --]

On 10/03/14 14:09, Pawel Moll wrote:

>> Note that the same DT bindings you add here should also work for the DRM
>> driver in the future. So, in fact, the question is extended to: will the
>> fbdev 
> 
> No, I don't think so. It's a end of line for it, I believe. From my
> personal point of view the main goal is to fill the last gap between DT
> and non-DT support on one of the Versatile Express boards - CLCD works
> fine when booted with ATAGs, doesn't work when booted with DT. It looks
> bad.

The DT describes the hardware, and there should be only one description
of it, no matter what the used SW is. I see compatible = "arm,pl111" in
the dts. PL111 is the LCD controller, right? What will the DRM driver
use, then, if the "arm,pl111" DT data is designed to work only for the
fbdev?

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

^ permalink raw reply

* Re: [PATCHv3 00/41] OMAPDSS: DT support v3
From: Tomi Valkeinen @ 2014-03-10 13:22 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140307164929.GC3789@atomide.com>

[-- Attachment #1: Type: text/plain, Size: 1248 bytes --]

On 07/03/14 18:49, Tony Lindgren wrote:
> * Tomi Valkeinen <tomi.valkeinen@ti.com> [140305 23:32]:
>> Hi Tony,
>>
>> On 21/01/14 12:56, Tomi Valkeinen wrote:
>>> Hi,
>>>
>>> Here's version 3 of the DSS DT series.
>>
>> Can you have a look at the arch/arm/ parts in the series and ack if
>> they're ok, i.e, patches 1, 2, 32.
> 
> Patches 1, 2 & 32 look OK to me, so for those please feel free to add:
> 
> Acked-by: Tony Lindgren <tony@atomide.com>

Thanks.

> How about do a pull request for just the .dts changes against current
> omap-for-v3.15/dt branch ASAP for me so I can pull it in? That is assuming
> that just the .dts changes alone won't break anything.

Unfortunately they do break. At least pinmuxing is moved from the global
definitions to be handled by the respective display drivers, and there
are some regulator_name hacks that the DT patches remove.

I think those changes could be removed from my patches, and then added
back later when everything else has been merged.

The bigger issue is that suddenly there's lots of discussion about the
display DT bindings. If those are not resolved very soon, I guess I have
no choice but to again skip the merge window for the DSS DT changes.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

^ permalink raw reply

* Re: [PATCHv3 00/41] OMAPDSS: DT support v3
From: Tony Lindgren @ 2014-03-10 15:41 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <531DBCA8.80507@ti.com>

* Tomi Valkeinen <tomi.valkeinen@ti.com> [140310 06:26]:
> On 07/03/14 18:49, Tony Lindgren wrote:
> > * Tomi Valkeinen <tomi.valkeinen@ti.com> [140305 23:32]:
> >> Hi Tony,
> >>
> >> On 21/01/14 12:56, Tomi Valkeinen wrote:
> >>> Hi,
> >>>
> >>> Here's version 3 of the DSS DT series.
> >>
> >> Can you have a look at the arch/arm/ parts in the series and ack if
> >> they're ok, i.e, patches 1, 2, 32.
> > 
> > Patches 1, 2 & 32 look OK to me, so for those please feel free to add:
> > 
> > Acked-by: Tony Lindgren <tony@atomide.com>
> 
> Thanks.
> 
> > How about do a pull request for just the .dts changes against current
> > omap-for-v3.15/dt branch ASAP for me so I can pull it in? That is assuming
> > that just the .dts changes alone won't break anything.
> 
> Unfortunately they do break. At least pinmuxing is moved from the global
> definitions to be handled by the respective display drivers, and there
> are some regulator_name hacks that the DT patches remove.

OK. Will that cause regressions for omap3 as that's still also booting
in legacy mode?
 
> I think those changes could be removed from my patches, and then added
> back later when everything else has been merged.

Or you could have a second branch that also merges in omap-for-v3.15/dt
branch that you can send later towards the merge window after the arm-soc
changes have been merged. If you want to do that, then feel free to add
my ack also for the .dts changes:

Acked-by: Tony Lindgren <tony@atomide.com>

If however those changes get postponed to v3.16, it's best that you'll
redo the branch as I'm sure we'll have other merge issues for v3.16.
 
> The bigger issue is that suddenly there's lots of discussion about the
> display DT bindings. If those are not resolved very soon, I guess I have
> no choice but to again skip the merge window for the DSS DT changes.

OK, some of these more bindings can take easily six months to reach
some kind of resolution. You may be able to use TI specific unstable
bindings meanwhile if that makese sense.

Regards,

Tony

^ permalink raw reply

* Re: [PATCH 0/9] Doc/DT: DT bindings for various display components
From: Rob Herring @ 2014-03-10 16:05 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA, Russell King - ARM Linux,
	Sascha Hauer, Tomasz Figa,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Inki Dae,
	Andrzej Hajda, Rob Clark, Thierry Reding, Laurent Pinchart,
	Philipp Zabel,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	Sebastian Hesselbarth
In-Reply-To: <53143796.2050309-l0cyMroinI0@public.gmane.org>

On Mon, Mar 3, 2014 at 2:04 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> Hi Rob, Pawel, Mark, Ian, Kumar,
>
> On 28/02/14 18:56, Russell King - ARM Linux wrote:
>> On Fri, Feb 28, 2014 at 06:48:35PM +0200, Tomi Valkeinen wrote:
>>> This is totally unclear to me. How does it become a public standard?
>>> What's the forum for this?
>>
>> Me too.  That's where I'd hope someone on devicetree-discuss will be
>> able to help us work out what's the right approach here. :)
>
> The story briefly so far: I've implemented DT support for OMAP display,
> and created bindings for various (non-OMAP) display components,
> including generic connector bindings for DVI, HDMI and analog-tv.
>
> Russell's point was that these connector bindings are very generic, i.e.
> they are not for any particular chip from a particular vendor, but for
> any connector for DVI, HDMI or analog-tv. And he's worried that maybe we
> shouldn't define such generic bindings without consulting the whole
> device-tree community (i.e including non-linux users).

So re-work it to be generic and send it out. DT maintainers would
rarely disagree that something shouldn't be made generic.

> So the question is, is there such a community and a forum to bring up
> this kind of things? If yes, should we bring this up there? If yes, what
> kind of things in general should be brought into the attention of
> non-linux users?

devicetree list is just that. It is not just for Linux. There is the
newly created devicetree-spec@vger.kernel.org which is for more
core/common binding discussion.

>
> What I wonder here is that while a thing like DVI connector is, of
> course, more generic than, say, "ti,tfp410" encoder chip, but isn't the
> case still the same: we're defining global bindings for hardware that
> should work for everyone, not only Linux users?

Defining the connectors in DT is a useful thing although mainly when
you have multiple connectors of the same type. Labels for composite,
SVideo, VGA, DVI, HDMI seem less useful to me. Describing position or
printed label (like front vs. rear connections) seem more useful to
me.

Rob

^ permalink raw reply

* Re: [PATCH 0/9] Doc/DT: DT bindings for various display components
From: Tomi Valkeinen @ 2014-03-10 16:40 UTC (permalink / raw)
  To: Rob Herring
  Cc: Mark Rutland, devicetree@vger.kernel.org, linux-fbdev,
	Russell King - ARM Linux, Pawel Moll, Ian Campbell, Sascha Hauer,
	Tomasz Figa, dri-devel, Inki Dae, Andrzej Hajda, Rob Clark,
	Rob Herring, Thierry Reding, Laurent Pinchart, Philipp Zabel,
	Kumar Gala, linux-arm-kernel@lists.infradead.org,
	Sebastian Hesselbarth
In-Reply-To: <CAL_JsqLppZHjND_LFMxscR0WCD4Ox7vF0fDbQFoYg=TENXxY8Q@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2900 bytes --]

Hi Rob,

On 10/03/14 18:05, Rob Herring wrote:

>> Russell's point was that these connector bindings are very generic, i.e.
>> they are not for any particular chip from a particular vendor, but for
>> any connector for DVI, HDMI or analog-tv. And he's worried that maybe we
>> shouldn't define such generic bindings without consulting the whole
>> device-tree community (i.e including non-linux users).
> 
> So re-work it to be generic and send it out. DT maintainers would
> rarely disagree that something shouldn't be made generic.

They (in this series) are already designed to be generic.

I should perhaps re-word the question: we are concerned whether these
bindings are good for all the users, not just us, and whether there
already exists something that overlaps.

Afaik, there's nothing overlapping. And I don't see why they wouldn't be
good for all users (with the few minor modifications that have been
discussed in this thread). But, if I gathered right, Russell would like
some kind of ack from someone who might know better than us.

So is it enough to have posted these, and gotten acks from the people
involved, or should we get acks from DT maintainers also?

Is there a way to get the attention of, say, BSD people, or should we
just presume they'll follow the list?

>> So the question is, is there such a community and a forum to bring up
>> this kind of things? If yes, should we bring this up there? If yes, what
>> kind of things in general should be brought into the attention of
>> non-linux users?
> 
> devicetree list is just that. It is not just for Linux. There is the
> newly created devicetree-spec@vger.kernel.org which is for more
> core/common binding discussion.

Ok.

>> What I wonder here is that while a thing like DVI connector is, of
>> course, more generic than, say, "ti,tfp410" encoder chip, but isn't the
>> case still the same: we're defining global bindings for hardware that
>> should work for everyone, not only Linux users?
> 
> Defining the connectors in DT is a useful thing although mainly when
> you have multiple connectors of the same type. Labels for composite,
> SVideo, VGA, DVI, HDMI seem less useful to me. Describing position or
> printed label (like front vs. rear connections) seem more useful to
> me.

My point above was that it feels mentally easier to define bindings for
one particular IP block or chip, than defining bindings for a more
generic thing like "HDMI connector". But, in the end, I believe they
both should go through similar review, and there's no such difference.

As for the labels, they can be anything that makes sense for that
particular board. It can be "HDMI Front", if such makes sense, or
"HDMI-2" if the connector has such physical printed label. Or, say,
"Main LCD", "Secondary LCD". It's a free form text that is given to the
user.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

^ permalink raw reply

* Re: [PATCH 0/9] Doc/DT: DT bindings for various display components
From: Rob Herring @ 2014-03-10 20:32 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA, Russell King - ARM Linux,
	Sascha Hauer, Tomasz Figa,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Inki Dae,
	Andrzej Hajda, Rob Clark, Thierry Reding, Laurent Pinchart,
	Philipp Zabel,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	Sebastian Hesselbarth
In-Reply-To: <531DEB1A.5090509-l0cyMroinI0@public.gmane.org>

On Mon, Mar 10, 2014 at 11:40 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> Hi Rob,
>
> On 10/03/14 18:05, Rob Herring wrote:
>
>>> Russell's point was that these connector bindings are very generic, i.e.
>>> they are not for any particular chip from a particular vendor, but for
>>> any connector for DVI, HDMI or analog-tv. And he's worried that maybe we
>>> shouldn't define such generic bindings without consulting the whole
>>> device-tree community (i.e including non-linux users).
>>
>> So re-work it to be generic and send it out. DT maintainers would
>> rarely disagree that something shouldn't be made generic.
>
> They (in this series) are already designed to be generic.
>
> I should perhaps re-word the question: we are concerned whether these
> bindings are good for all the users, not just us, and whether there
> already exists something that overlaps.

Given this is tied into the rest of video description with endpoints
and such, you really have to look at the whole picture to answer that
question. I'd guess Linux well ahead of other OSs in terms of dealing
with the plethora of video and graphics h/w. I'd be more concerned
that the bindings are good for all h/w rather than all OS's.

> Afaik, there's nothing overlapping. And I don't see why they wouldn't be
> good for all users (with the few minor modifications that have been
> discussed in this thread). But, if I gathered right, Russell would like
> some kind of ack from someone who might know better than us.
>
> So is it enough to have posted these, and gotten acks from the people
> involved, or should we get acks from DT maintainers also?

We really leave it up to the subsystem maintainers to decide, but for
new common/generic bindings it is a good idea to get DT maintainers
ack. I'll go thru the individual patches.

> Is there a way to get the attention of, say, BSD people, or should we
> just presume they'll follow the list?

devicetree-spec was created specifically to have a lower volume list
without a bunch of Linux driver patches which aren't relevant to
people from other OSs.

>>> So the question is, is there such a community and a forum to bring up
>>> this kind of things? If yes, should we bring this up there? If yes, what
>>> kind of things in general should be brought into the attention of
>>> non-linux users?
>>
>> devicetree list is just that. It is not just for Linux. There is the
>> newly created devicetree-spec@vger.kernel.org which is for more
>> core/common binding discussion.
>
> Ok.
>
>>> What I wonder here is that while a thing like DVI connector is, of
>>> course, more generic than, say, "ti,tfp410" encoder chip, but isn't the
>>> case still the same: we're defining global bindings for hardware that
>>> should work for everyone, not only Linux users?
>>
>> Defining the connectors in DT is a useful thing although mainly when
>> you have multiple connectors of the same type. Labels for composite,
>> SVideo, VGA, DVI, HDMI seem less useful to me. Describing position or
>> printed label (like front vs. rear connections) seem more useful to
>> me.
>
> My point above was that it feels mentally easier to define bindings for
> one particular IP block or chip, than defining bindings for a more
> generic thing like "HDMI connector". But, in the end, I believe they
> both should go through similar review, and there's no such difference.

Unless you need to describe different types of HDMI connectors, I
don't see any issue with it being generic. I certainly don't see the
need for prepending with "linux," in this case.

Rob

^ permalink raw reply

* Re: [PATCH 3/9] Doc/DT: Add DT binding documentation for DVI Connector
From: Rob Herring @ 2014-03-10 21:45 UTC (permalink / raw)
  To: Philipp Zabel
  Cc: Russell King - ARM Linux, Tomi Valkeinen,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	Laurent Pinchart, Sascha Hauer, Sebastian Hesselbarth, Rob Clark,
	Inki Dae, Andrzej Hajda, Tomasz Figa, Thierry Reding
In-Reply-To: <1393604717.3802.61.camel-+qGW7pzALmz7o/J7KWpOmN53zsg1cpMQ@public.gmane.org>

On Fri, Feb 28, 2014 at 10:25 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> Am Freitag, den 28.02.2014, 15:59 +0000 schrieb Russell King - ARM
> Linux:
>> On Fri, Feb 28, 2014 at 02:20:10PM +0200, Tomi Valkeinen wrote:
>> > Add DT binding documentation for DVI Connector.
>> >
>> > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> > Reviewed-by: Archit Taneja <archit@ti.com>
>> > ---
>> >  .../devicetree/bindings/video/dvi-connector.txt    | 26 ++++++++++++++++++++++
>> >  1 file changed, 26 insertions(+)
>> >  create mode 100644 Documentation/devicetree/bindings/video/dvi-connector.txt
>> >
>> > diff --git a/Documentation/devicetree/bindings/video/dvi-connector.txt b/Documentation/devicetree/bindings/video/dvi-connector.txt
>> > new file mode 100644
>> > index 000000000000..6a0aff866c78
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/video/dvi-connector.txt
>> > @@ -0,0 +1,26 @@
>> > +DVI Connector
>> > +=======
>> > +
>> > +Required properties:
>> > +- compatible: "dvi-connector"
>> > +
>> > +Optional properties:
>> > +- label: a symbolic name for the connector
>> > +- i2c-bus: phandle to the i2c bus that is connected to DVI DDC
>> > +
>> > +Required nodes:
>> > +- Video port for DVI input
>> > +
>> > +Example
>> > +-------
>> > +
>> > +dvi0: connector@0 {
>> > +   compatible = "dvi-connector";
>> > +   label = "dvi";
>> > +
>> > +   i2c-bus = <&i2c3>;
>> > +
>> > +   dvi_connector_in: endpoint {
>> > +           remote-endpoint = <&tfp410_out>;
>> > +   };
>> > +};
>>
>> This looks far too simplistic.  There are different classes of DVI
>> connector - there is:
>>
>> DVI A - analogue only
>> DVI D - digital only (single and dual link)
>> DVI I - both (single and dual digital link)
>>
>> DRM at least makes a distinction between these three classes, and this
>> disctinction is part of the user API.  How would a display system know
>> which kind of DVI connector is wired up on the board from this DT
>> description?
>
> Maybe this could be inferred from the sources connected to it. For
> example a i.MX5 board with the SoC internal TV Encoder and an external
> SiI902x HDMI encoder connected to the same DVI I connector:
>
> ipu {
>         port@2 {
>                 ipu_di0_disp0: endpoint {
>                         remote-endpoint = <&sii902x_in>;
>                 };
>         };
>         port@3 {
>                 ipu_di1_tve: endpoint {
>                         remote-endpoint = <&tve_in>;
>                 };
>         };
> };
>
> &sii902x {
>         compatible = "si,sii9022";
>
>         port@0 {
>                 sii902x_in: endpoint {
>                         remote-endpoint = <&ipu_di0>;
>                 };
>         };
>         port@1 {
>                 sii902x_out: endpoint {
>                         remote-endpoint = <&dvi_d_in>;
>                 };
>         };
> };
>
> &tve {
>         compatible = "fsl,imx53-tve";
>         port@0 {
>                 tve_in: endpoint {
>                         remote-endpoint = <&ipu_di1>;
>                 };
>         };
>         port@1 {
>                 tve_out: endpoint {
>                         remote-endpoint = <&dvi_a_in>;
>                 };
>         };
> };
>
> dvi-connector {
>         compatible = "dvi-connector";
>         ddc-i2c-bus = <&i2c3>;
>
>         port {
>                 dvi_d_in: endpoint@0 {
>                         remote-endpoint = <&sii902x_out>;
>                 };
>                 dvi_a_in: endpoint@1 {
>                         remote-endpoint = <&tve_out>;
>                 };
>         };
> };
>
> It should be possible to let the connector know that those two endpoints
> are connected to a TMDS source and to a VGA source, respectively.

I like this proposal over the others. Although, would dual link be a
single endpoint or 2 endpoints? How would you differentiate that?

The port node seems a bit pointless.

Rob

^ permalink raw reply

* Re: [PATCH 3/9] Doc/DT: Add DT binding documentation for DVI Connector
From: Tomi Valkeinen @ 2014-03-11  6:39 UTC (permalink / raw)
  To: Rob Herring, Philipp Zabel
  Cc: devicetree@vger.kernel.org, linux-fbdev, Russell King - ARM Linux,
	Sascha Hauer, Tomasz Figa, dri-devel, Inki Dae, Andrzej Hajda,
	Rob Clark, Thierry Reding, Laurent Pinchart,
	linux-arm-kernel@lists.infradead.org, Sebastian Hesselbarth
In-Reply-To: <CAL_JsqJ00Zr1tMoyR1+qHUhsNoc6Kj6s+nTZTkNTwki9HFHq8g@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1038 bytes --]

On 10/03/14 23:45, Rob Herring wrote:

> I like this proposal over the others. Although, would dual link be a

I don't like inferring the information. With the above, you can't find
out that the DVI connector has digital and analog support before all the
drivers are loaded.

> single endpoint or 2 endpoints? How would you differentiate that?

Hmm, well endpoints for a single port are exclusive. So it's either a
single port and a single endpoint, or two ports and two endpoints. I
think dual link has to be single port & endpoint, as the TMDS links need
to be driven together as a single bus.

And dual-link is not really "two links". DVI dual-link means 1 clock
lane and 6 data lanes, compared to 1 clock lane and 3 data lanes for
single-link.

> The port node seems a bit pointless.

There's another thread discussing the ports and endpoints.

The port node represents, for example, the pins for the connection for
that device. And an endpoint-endpoint link represents wires between two
ports.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

^ permalink raw reply

* Re: [PATCH 3/9] Doc/DT: Add DT binding documentation for DVI Connector
From: Tomi Valkeinen @ 2014-03-11  6:43 UTC (permalink / raw)
  To: Philipp Zabel
  Cc: devicetree, linux-fbdev, Russell King - ARM Linux, Sascha Hauer,
	Tomasz Figa, dri-devel, Inki Dae, Andrzej Hajda, Rob Clark,
	Thierry Reding, Laurent Pinchart, linux-arm-kernel,
	Sebastian Hesselbarth
In-Reply-To: <1393604717.3802.61.camel@paszta.hi.pengutronix.de>

[-- Attachment #1: Type: text/plain, Size: 887 bytes --]

On 28/02/14 18:25, Philipp Zabel wrote:

> dvi-connector {
> 	compatible = "dvi-connector";
> 	ddc-i2c-bus = <&i2c3>;
> 
> 	port {
> 		dvi_d_in: endpoint@0 {
> 			remote-endpoint = <&sii902x_out>;
> 		};
> 		dvi_a_in: endpoint@1 {
> 			remote-endpoint = <&tve_out>;
> 		};
> 	};
> };
> 
> It should be possible to let the connector know that those two endpoints
> are connected to a TMDS source and to a VGA source, respectively.

I have not worked with boards that would have the analog part, so just
wondering about the above.

The above would mean that either digital or analog is in use, but not
both. Was that the intention? From the connector's perspective, the
analog and digital pins are separate, and I think they can be used at
the same time. That kind of sounds like the analog and digital pins
should be represented as separate ports.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

^ permalink raw reply

* Re: [PATCH 3/9] Doc/DT: Add DT binding documentation for DVI Connector
From: Geert Uytterhoeven @ 2014-03-11  8:00 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: devicetree@vger.kernel.org, Linux Fbdev development list,
	Russell King - ARM Linux, DRI Development, Andrzej Hajda,
	Rob Herring, Laurent Pinchart,
	linux-arm-kernel@lists.infradead.org, Sebastian Hesselbarth
In-Reply-To: <531EAF8F.2040400@ti.com>

On Tue, Mar 11, 2014 at 7:39 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On 10/03/14 23:45, Rob Herring wrote:
>> I like this proposal over the others. Although, would dual link be a
>
> I don't like inferring the information. With the above, you can't find
> out that the DVI connector has digital and analog support before all the
> drivers are loaded.
>
>> single endpoint or 2 endpoints? How would you differentiate that?
>
> Hmm, well endpoints for a single port are exclusive. So it's either a
> single port and a single endpoint, or two ports and two endpoints. I
> think dual link has to be single port & endpoint, as the TMDS links need
> to be driven together as a single bus.
>
> And dual-link is not really "two links". DVI dual-link means 1 clock
> lane and 6 data lanes, compared to 1 clock lane and 3 data lanes for
> single-link.

What about having a property for the number of data lanes?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH 3/9] Doc/DT: Add DT binding documentation for DVI Connector
From: Tomi Valkeinen @ 2014-03-11  8:04 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: devicetree@vger.kernel.org, Linux Fbdev development list,
	Russell King - ARM Linux, Sascha Hauer, Tomasz Figa,
	DRI Development, Inki Dae, Andrzej Hajda, Rob Clark,
	Thierry Reding, Laurent Pinchart, Philipp Zabel,
	linux-arm-kernel@lists.infradead.org, Sebastian Hesselbarth
In-Reply-To: <CAMuHMdWqt4M5OEXp8y_DR8UEA8GPxJg99cz-HcmMPQB+h+Rh0A@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1277 bytes --]

On 11/03/14 10:00, Geert Uytterhoeven wrote:
> On Tue, Mar 11, 2014 at 7:39 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>> On 10/03/14 23:45, Rob Herring wrote:
>>> I like this proposal over the others. Although, would dual link be a
>>
>> I don't like inferring the information. With the above, you can't find
>> out that the DVI connector has digital and analog support before all the
>> drivers are loaded.
>>
>>> single endpoint or 2 endpoints? How would you differentiate that?
>>
>> Hmm, well endpoints for a single port are exclusive. So it's either a
>> single port and a single endpoint, or two ports and two endpoints. I
>> think dual link has to be single port & endpoint, as the TMDS links need
>> to be driven together as a single bus.
>>
>> And dual-link is not really "two links". DVI dual-link means 1 clock
>> lane and 6 data lanes, compared to 1 clock lane and 3 data lanes for
>> single-link.
> 
> What about having a property for the number of data lanes?

That was already suggested by Philipp in this thread. I don't see
anything wrong with that, but I don't really see benefit either.
"dual-link" is a standard term for 6 data lanes for the DVI connector.
And the choices are 3 or 6 data lanes, nothing else.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

^ permalink raw reply

* Re: [PATCHv3 00/41] OMAPDSS: DT support v3
From: Tomi Valkeinen @ 2014-03-11 10:15 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140310154105.GA4882@atomide.com>

[-- Attachment #1: Type: text/plain, Size: 3067 bytes --]

On 10/03/14 17:41, Tony Lindgren wrote:

>>> How about do a pull request for just the .dts changes against current
>>> omap-for-v3.15/dt branch ASAP for me so I can pull it in? That is assuming
>>> that just the .dts changes alone won't break anything.
>>
>> Unfortunately they do break. At least pinmuxing is moved from the global
>> definitions to be handled by the respective display drivers, and there
>> are some regulator_name hacks that the DT patches remove.
> 
> OK. Will that cause regressions for omap3 as that's still also booting
> in legacy mode?

No, I don't think so. The problems revolve around the pdata-quirks, with
current DSS support when booting with DT. It's rather difficult to split
the series so that it could be merged freely in multiple parts.

If I split the series into three parts: 1) .dts changes 2) main DSS DT
changes 3) removal of pdata-quirks etc, I run into problems. Both 1) and
2) work fine individually, but when both are merged, there are two
competing systems, the proper DT stuff and the pdata-quirks stuff. And I
haven't found out a simple way to manage that, as the whole display
support is split into multiple independent devices.

One option would be to combine 1) and 3), so that when they are merged,
there would be proper DT data, and the pdata-quirks would be removed so
that it wouldn't be messing everything up. But that would, of course,
mean that after merging 1+3, the display wouldn't work on those boards
that rely on pdata-quirks, until 2) is merged.

>> I think those changes could be removed from my patches, and then added
>> back later when everything else has been merged.
> 
> Or you could have a second branch that also merges in omap-for-v3.15/dt
> branch that you can send later towards the merge window after the arm-soc
> changes have been merged. If you want to do that, then feel free to add
> my ack also for the .dts changes:
> 
> Acked-by: Tony Lindgren <tony@atomide.com>
> 
> If however those changes get postponed to v3.16, it's best that you'll
> redo the branch as I'm sure we'll have other merge issues for v3.16.

Ok. At the moment I feel that the easiest option would be to keep the
DSS DT series as it is, but merge omap-for-v3.15/dt to it and solve the
conflicts. I'd keep that branch separate from the fbdev changes, so that
I could send the DSS DT pull request later, when arm-soc has been pulled.

>> The bigger issue is that suddenly there's lots of discussion about the
>> display DT bindings. If those are not resolved very soon, I guess I have
>> no choice but to again skip the merge window for the DSS DT changes.
> 
> OK, some of these more bindings can take easily six months to reach
> some kind of resolution. You may be able to use TI specific unstable
> bindings meanwhile if that makese sense.

Yep. I don't know... If the whole port/endpoint system that I currently
use is changed totally, it might be painful to support both the TI
specific bindings with the old format and the newer format.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

^ permalink raw reply

* Re: [PATCH 3/9] Doc/DT: Add DT binding documentation for DVI Connector
From: Philipp Zabel @ 2014-03-11 11:19 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: devicetree@vger.kernel.org, Linux Fbdev development list,
	Russell King - ARM Linux, DRI Development, Andrzej Hajda,
	Rob Herring, Laurent Pinchart, Geert Uytterhoeven,
	linux-arm-kernel@lists.infradead.org, Sebastian Hesselbarth
In-Reply-To: <531EC385.7090001@ti.com>

Am Dienstag, den 11.03.2014, 10:04 +0200 schrieb Tomi Valkeinen:
> On 11/03/14 10:00, Geert Uytterhoeven wrote:
> > On Tue, Mar 11, 2014 at 7:39 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> >> On 10/03/14 23:45, Rob Herring wrote:
> >>> I like this proposal over the others. Although, would dual link be a
> >>
> >> I don't like inferring the information. With the above, you can't find
> >> out that the DVI connector has digital and analog support before all the
> >> drivers are loaded.
> >>
> >>> single endpoint or 2 endpoints? How would you differentiate that?
> >>
> >> Hmm, well endpoints for a single port are exclusive. So it's either a
> >> single port and a single endpoint, or two ports and two endpoints. I
> >> think dual link has to be single port & endpoint, as the TMDS links need
> >> to be driven together as a single bus.
> >>
> >> And dual-link is not really "two links". DVI dual-link means 1 clock
> >> lane and 6 data lanes, compared to 1 clock lane and 3 data lanes for
> >> single-link.
> > 
> > What about having a property for the number of data lanes?
> 
> That was already suggested by Philipp in this thread. I don't see
> anything wrong with that, but I don't really see benefit either.
> "dual-link" is a standard term for 6 data lanes for the DVI connector.
> And the choices are 3 or 6 data lanes, nothing else.

The number of lanes of a DisplayPort connector could be 1 to 4. Also,
there's dual-mode DP which can use four lanes to drive
somewhat-like-HDMI single link TMDS signals.

regards
Philipp


^ permalink raw reply

* Fwd: Addition to https://www.kernel.org/doc/Documentation/fb/deferred_io.txt
From: Geert Uytterhoeven @ 2014-03-11 16:05 UTC (permalink / raw)
  To: linux-fbdev

--------- Forwarded message ----------
From: Conor O <falling174fps@gmail.com>
Date: Tue, Mar 11, 2014 at 4:36 PM
Subject: Addition to https://www.kernel.org/doc/Documentation/fb/deferred_io.txt
To: Geert Uytterhoeven <geert@linux-m68k.org>


Dear Geert,

Having run into this problem myself
(http://stackoverflow.com/questions/22285151/kernel-panic-using-deferred-io-on-kmalloced-buffer)
I'd like to suggest an addendum to the deferred_io.txt document to
note the difference in treatment between a framebuffer allocated with
vmalloc and one allocated with, say, kmalloc. I've pasted in the diff
below.

Thanks,

Conor O'Rourke.

$ diff deferred_io.txt deferred_io_changes.txt
75a76,89
>
> 5. Bear in mind the following:
>
>   - deferred_io changes info->fp_ops->fb_mmap to its own function that
>     sets up fault handlers etc. In the fault handler it checks bounds
>     against info->fix.smem_len
>
>   - when it gets the page written, it checks the address in info->screen_base
>     to see if that address is one from vmalloc. If so it uses that address
>     as the one to get the page number. If not, the assumption is that the
>     address of interest is the physical framebuffer address stored in
>     info->fix.smem_start. You can get an physical address from, say, a
>     kmalloced pointer by using virt_to_phys().
>

^ permalink raw reply

* Re: [PATCHv3 00/41] OMAPDSS: DT support v3
From: Tony Lindgren @ 2014-03-11 16:28 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <531EE25B.9060201@ti.com>

* Tomi Valkeinen <tomi.valkeinen@ti.com> [140311 03:19]:
> On 10/03/14 17:41, Tony Lindgren wrote:
> 
> >>> How about do a pull request for just the .dts changes against current
> >>> omap-for-v3.15/dt branch ASAP for me so I can pull it in? That is assuming
> >>> that just the .dts changes alone won't break anything.
> >>
> >> Unfortunately they do break. At least pinmuxing is moved from the global
> >> definitions to be handled by the respective display drivers, and there
> >> are some regulator_name hacks that the DT patches remove.
> > 
> > OK. Will that cause regressions for omap3 as that's still also booting
> > in legacy mode?
> 
> No, I don't think so. The problems revolve around the pdata-quirks, with
> current DSS support when booting with DT. It's rather difficult to split
> the series so that it could be merged freely in multiple parts.
> 
> If I split the series into three parts: 1) .dts changes 2) main DSS DT
> changes 3) removal of pdata-quirks etc, I run into problems. Both 1) and
> 2) work fine individually, but when both are merged, there are two
> competing systems, the proper DT stuff and the pdata-quirks stuff. And I
> haven't found out a simple way to manage that, as the whole display
> support is split into multiple independent devices.
> 
> One option would be to combine 1) and 3), so that when they are merged,
> there would be proper DT data, and the pdata-quirks would be removed so
> that it wouldn't be messing everything up. But that would, of course,
> mean that after merging 1+3, the display wouldn't work on those boards
> that rely on pdata-quirks, until 2) is merged.

OK, best to keep the series together then.
 
> >> I think those changes could be removed from my patches, and then added
> >> back later when everything else has been merged.
> > 
> > Or you could have a second branch that also merges in omap-for-v3.15/dt
> > branch that you can send later towards the merge window after the arm-soc
> > changes have been merged. If you want to do that, then feel free to add
> > my ack also for the .dts changes:
> > 
> > Acked-by: Tony Lindgren <tony@atomide.com>
> > 
> > If however those changes get postponed to v3.16, it's best that you'll
> > redo the branch as I'm sure we'll have other merge issues for v3.16.
> 
> Ok. At the moment I feel that the easiest option would be to keep the
> DSS DT series as it is, but merge omap-for-v3.15/dt to it and solve the
> conflicts. I'd keep that branch separate from the fbdev changes, so that
> I could send the DSS DT pull request later, when arm-soc has been pulled.

OK makes sense to me.
 
> >> The bigger issue is that suddenly there's lots of discussion about the
> >> display DT bindings. If those are not resolved very soon, I guess I have
> >> no choice but to again skip the merge window for the DSS DT changes.
> > 
> > OK, some of these more bindings can take easily six months to reach
> > some kind of resolution. You may be able to use TI specific unstable
> > bindings meanwhile if that makese sense.
> 
> Yep. I don't know... If the whole port/endpoint system that I currently
> use is changed totally, it might be painful to support both the TI
> specific bindings with the old format and the newer format.

OK that's up you guys in the display land, I have not followed the
latest bindings discussion.

Regards,

Tony

^ permalink raw reply

* Re: [PATCH 0/9] Doc/DT: DT bindings for various display components
From: Tomi Valkeinen @ 2014-03-12  8:15 UTC (permalink / raw)
  To: Grant Likely
  Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Philipp Zabel,
	Laurent Pinchart, Russell King - ARM Linux, Sascha Hauer,
	Sebastian Hesselbarth, Rob Clark, Inki Dae, Andrzej Hajda,
	Tomasz Figa, Thierry Reding
In-Reply-To: <1393590016-9361-1-git-send-email-tomi.valkeinen-l0cyMroinI0@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 2104 bytes --]

Hi Grant,

On 28/02/14 14:20, Tomi Valkeinen wrote:
> Hi,
> 
> This series is a re-send of
> http://article.gmane.org/gmane.linux.drivers.devicetree/61739
> 
> I'm cc'ing more people, and I want to clarify the contents of the series:
> 
> While this has been developed for OMAP, only the first patch is about OMAP
> bindings. The rest are generic bindings for video components, which can be used
> on any platform.
> 
> The bindings use the V4L2 style video port/endpoint system, described in
> Documentation/devicetree/bindings/media/video-interfaces.txt, to connect the
> components. The same port/endpoint bindings are used by Philipp Zabel in his
> imx-drm patch series.

This series is a piece of bigger series, which brings DT support for
OMAP display subsystem. This uses the same V4L2 style ports/endpoints as
has been discussed recently regarding the series from Philipp. It
doesn't use the helper code from Philipp, but a custom one as Philipp's
code didn't exist when I made this, and also because I needed extra
functionality not present in Philipp's series (which aimed to just move
the current V4L2 code to a common place).

The main concerns with the ports/endpoints has been the optional 'port'
node for the case where we have a single endpoint, and the
double-linking of the endpoints.

If I remove the optional 'port' usage from my series, are you ok with me
proceeding with this series for 3.15, with the double-linked endpoints?

As far as I see, when we come to a conclusion how the linking should be
made, it's trivial to change the bindings in this series to match it,
even without needing any compatibility code. I just need to remove the
other link, and old dts files having double-linking will still work fine.

The reason I want to push this forward asap is that omap4 and omap5 are
already DT only, and for omap5 we don't have working display without DT
support for display. For omap4, we have a really hacky way to add
display support for a few boards, but that is causing major headaches
and I want to get rid of it.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

^ permalink raw reply

* Re: [PATCH] video: atmel_lcdfb: ensure the hardware is initialized with the correct mode
From: Nicolas Ferre @ 2014-03-12  9:02 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1394209254-23797-1-git-send-email-antoine.tenart@free-electrons.com>

On 07/03/2014 17:20, Antoine Ténart :
> If no driver takeover the atmel_lcdfb, the lcd won't be in a working state
> since atmel_lcdfb_set_par() will never be called. Enabling a driver which does,
> like fbcon, will call the function and put atmel_lcdfb in a working state.
> 
> Fixes: b985172b328a (video: atmel_lcdfb: add device tree suport)
> 
> Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com>
> Reported-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>

Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>

Thanks for having fixing this.

Bye,

> ---
>  drivers/video/atmel_lcdfb.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
> index cd96162..b74e5f5d 100644
> --- a/drivers/video/atmel_lcdfb.c
> +++ b/drivers/video/atmel_lcdfb.c
> @@ -1298,6 +1298,12 @@ static int __init atmel_lcdfb_probe(struct platform_device *pdev)
>  		goto unregister_irqs;
>  	}
>  
> +	ret = atmel_lcdfb_set_par(info);
> +	if (ret < 0) {
> +		dev_err(dev, "set par failed: %d\n", ret);
> +		goto unregister_irqs;
> +	}
> +
>  	dev_set_drvdata(dev, info);
>  
>  	/*
> 


-- 
Nicolas Ferre

^ permalink raw reply

* Re: [PATCH] video: atmel_lcdfb: ensure the hardware is initialized with the correct mode
From: Tomi Valkeinen @ 2014-03-12 11:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1394209254-23797-1-git-send-email-antoine.tenart@free-electrons.com>

[-- Attachment #1: Type: text/plain, Size: 1143 bytes --]

On 07/03/14 18:20, Antoine Ténart wrote:
> If no driver takeover the atmel_lcdfb, the lcd won't be in a working state
> since atmel_lcdfb_set_par() will never be called. Enabling a driver which does,
> like fbcon, will call the function and put atmel_lcdfb in a working state.
> 
> Fixes: b985172b328a (video: atmel_lcdfb: add device tree suport)
> 
> Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com>
> Reported-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> ---
>  drivers/video/atmel_lcdfb.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
> index cd96162..b74e5f5d 100644
> --- a/drivers/video/atmel_lcdfb.c
> +++ b/drivers/video/atmel_lcdfb.c
> @@ -1298,6 +1298,12 @@ static int __init atmel_lcdfb_probe(struct platform_device *pdev)
>  		goto unregister_irqs;
>  	}
>  
> +	ret = atmel_lcdfb_set_par(info);
> +	if (ret < 0) {
> +		dev_err(dev, "set par failed: %d\n", ret);
> +		goto unregister_irqs;
> +	}
> +
>  	dev_set_drvdata(dev, info);
>  
>  	/*
> 

Thanks, queued for 3.15.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

^ permalink raw reply

* Re: [PATCH] backlight: add new LP8860 backlight driver
From: Daniel Jeong @ 2014-03-13  5:01 UTC (permalink / raw)
  To: Jingoo Han
  Cc: linux-fbdev, linux-kernel,
	'Jean-Christophe Plagniol-Villard',
	'Tomi Valkeinen', 'Bryan Wu', 'Lee Jones'
In-Reply-To: <001b01cf3c40$a4b347e0$ee19d7a0$%han@samsung.com>

Thank you for your comments

> On Monday, March 03, 2014 6:15 PM, Daniel Jeong wrote:
> (+CC Bryan Wu, Lee Jones)
>
> Please add Bryan Wu, Lee Jones to CC list, when you send
> patches for backlight.
>
>>   This patch adds LP8860 backlight device driver.
>> LP8860 is a low EMI and High performance 4 channel LED Driver of TI.
>> This device driver provide the way to control brightness and currnet
> (+CC Bryan Wu, Lee Jones)
>
> s/provide/provides
> s/currnet/current
>
>> of each channel and provide the way to write eeprom.
> s/provide/provides
>
>> To support dt structure, another patch file will be sent.
>>
>> Signed-off-by: Daniel Jeong <gshark.jeong@gmail.com>
>> ---
> 'To support dt structure, another patch file will be sent.' is
> NOT appropriate for the commit message. So, please move it as below.
> Then, this message will not be included to the commit message, when
> this patch will be merged to maintainer's tree.
>
> Signed-off-by: Daniel Jeong <gshark.jeong@gmail.com>
> ---
> To support dt structure, another patch file will be sent.
>
>
>>   drivers/video/backlight/Kconfig         |    7 +
>>   drivers/video/backlight/Makefile        |    1 +
>>   drivers/video/backlight/lp8860_bl.c     |  528 +++++++++++++++++++++++++++++++
>>   include/linux/platform_data/lp8860_bl.h |   54 ++++
>>   4 files changed, 590 insertions(+)
>>   create mode 100644 drivers/video/backlight/lp8860_bl.c
>>   create mode 100644 include/linux/platform_data/lp8860_bl.h
>>
>> diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
>> index 5a3eb2e..908048f 100644
>> --- a/drivers/video/backlight/Kconfig
>> +++ b/drivers/video/backlight/Kconfig
>> @@ -397,6 +397,13 @@ config BACKLIGHT_LP8788
>>   	help
>>   	  This supports TI LP8788 backlight driver.
>>
>> +config BACKLIGHT_LP8860
>> +	tristate "Backlight Driver for LP8860"
>> +	depends on BACKLIGHT_CLASS_DEVICE && I2C
>> +	select REGMAP_I2C
>> +	help
>> +	  This supports TI LP8860 Backlight Driver
>> +
>>   config BACKLIGHT_OT200
>>   	tristate "Backlight driver for ot200 visualisation device"
>>   	depends on BACKLIGHT_CLASS_DEVICE && CS5535_MFGPT && GPIO_CS5535
>> diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
>> index bb82002..cbc5ac3 100644
>> --- a/drivers/video/backlight/Makefile
>> +++ b/drivers/video/backlight/Makefile
>> @@ -42,6 +42,7 @@ obj-$(CONFIG_BACKLIGHT_LM3639)		+= lm3639_bl.o
>>   obj-$(CONFIG_BACKLIGHT_LOCOMO)		+= locomolcd.o
>>   obj-$(CONFIG_BACKLIGHT_LP855X)		+= lp855x_bl.o
>>   obj-$(CONFIG_BACKLIGHT_LP8788)		+= lp8788_bl.o
>> +obj-$(CONFIG_BACKLIGHT_LP8860)		+= lp8860_bl.o
>>   obj-$(CONFIG_BACKLIGHT_LV5207LP)	+= lv5207lp.o
>>   obj-$(CONFIG_BACKLIGHT_MAX8925)		+= max8925_bl.o
>>   obj-$(CONFIG_BACKLIGHT_OMAP1)		+= omap1_bl.o
>> diff --git a/drivers/video/backlight/lp8860_bl.c b/drivers/video/backlight/lp8860_bl.c
>> new file mode 100644
>> index 0000000..4712e84
>> --- /dev/null
>> +++ b/drivers/video/backlight/lp8860_bl.c
>> @@ -0,0 +1,528 @@
>> +/*
>> +* Simple driver for Texas Instruments lp8860 Backlight driver chip
>> +*
>> +* Copyright (C) 2014 Texas Instruments
>> +* Author: Daniel Jeong  <gshark.jeong@gmail.com>
>> +*		  Ldd Mlp <ldd-mlp@list.ti.com>
>> +*
>> +* This program is free software; you can redistribute it and/or modify
>> +* it under the terms of the GNU General Public License version 2 as
>> +* published by the Free Software Foundation.
>> +*
>> +*/
>> +#include <linux/module.h>
> Please move this header in alphabetical order.
>
>> +#include <linux/backlight.h>
>> +#include <linux/delay.h>
>> +#include <linux/err.h>
>> +#include <linux/i2c.h>
>> +#include <linux/platform_data/lp8860_bl.h>
>> +#include <linux/regmap.h>
>> +#include <linux/slab.h>
>> +#include <linux/uaccess.h>
>> +
>> +#define REG_CL0_BRT_H	0x00
>> +#define REG_CL0_BRT_L	0x01
>> +#define REG_CL0_I_H		0x02
>> +#define REG_CL0_I_L		0x03
>> +
>> +#define REG_CL1_BRT_H	0x04
>> +#define REG_CL1_BRT_L	0x05
>> +#define REG_CL1_I		0x06
>> +
>> +#define REG_CL2_BRT_H	0x07
>> +#define REG_CL2_BRT_L	0x08
>> +#define REG_CL2_I		0x09
>> +
>> +#define REG_CL3_BRT_H	0x0a
>> +#define REG_CL3_BRT_L	0x0b
>> +#define REG_CL3_I		0x0c
>> +
>> +#define REG_CONF	0x0d
>> +#define REG_STATUS	0x0e
>> +#define REG_ID		0x12
>> +
>> +#define REG_ROM_CTRL	0x19
>> +#define REG_ROM_UNLOCK	0x1a
>> +#define REG_ROM_START	0x60
>> +#define REG_ROM_END		0x78
>> +
>> +#define REG_EEPROM_START	0x60
>> +#define REG_EEPROM_END		0x78
>> +#define REG_MAX	0xFF
>> +
>> +struct lp8860_chip {
>> +	struct device *dev;
>> +	struct lp8860_platform_data *pdata;
>> +	struct backlight_device *bled[LP8860_LED_MAX];
>> +	struct regmap *regmap;
>> +};
>> +
>> +/* brightness control */
>> +static int lp8860_bled_update_status(struct backlight_device *bl,
>> +				     enum lp8860_leds nsr)
>> +{
>> +	int ret = -EINVAL;
>> +	struct lp8860_chip *pchip = bl_get_data(bl);
>> +
>> +	if (pchip->pdata->mode)
>> +		return 0;
>> +
>> +	if (bl->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK))
>> +		bl->props.brightness = 0;
>> +
>> +	switch (nsr) {
>> +	case LP8860_LED0:
>> +		ret = regmap_write(pchip->regmap,
>> +				   REG_CL0_BRT_H, bl->props.brightness >> 8);
>> +		ret |= regmap_write(pchip->regmap,
>> +				    REG_CL0_BRT_L, bl->props.brightness & 0xff);
>> +		break;
>> +	case LP8860_LED1:
>> +		ret = regmap_write(pchip->regmap,
>> +				   REG_CL1_BRT_H,
>> +				   (bl->props.brightness >> 8) & 0x1f);
>> +		ret |>> +		    regmap_write(pchip->regmap, REG_CL1_BRT_L,
>> +				 bl->props.brightness & 0xff);
>> +		break;
>> +	case LP8860_LED2:
>> +		ret = regmap_write(pchip->regmap,
>> +				   REG_CL2_BRT_H,
>> +				   (bl->props.brightness >> 8) & 0x1f);
>> +		ret |>> +		    regmap_write(pchip->regmap, REG_CL2_BRT_L,
>> +				 bl->props.brightness & 0xff);
>> +		break;
>> +	case LP8860_LED3:
>> +		ret = regmap_write(pchip->regmap,
>> +				   REG_CL3_BRT_H,
>> +				   (bl->props.brightness >> 8) & 0x1f);
>> +		ret |>> +		    regmap_write(pchip->regmap, REG_CL3_BRT_L,
>> +				 bl->props.brightness & 0xff);
>> +		break;
>> +	default:
>> +		BUG();
>> +	}
>> +	if (ret < 0)
>> +		dev_err(pchip->dev, "fail : i2c access to register.\n");
>> +	else
>> +		ret = bl->props.brightness;
>> +
>> +	return ret;
>> +}
>> +
>> +static int lp8860_bled_get_brightness(struct backlight_device *bl,
>> +				      enum lp8860_leds nsr)
>> +{
>> +	struct lp8860_chip *pchip = bl_get_data(bl);
>> +	unsigned int rval_h, rval_l;
>> +	int ret = -EINVAL;
>> +
>> +	switch (nsr) {
>> +	case LP8860_LED0:
>> +		ret = regmap_read(pchip->regmap, REG_CL0_BRT_H, &rval_h);
>> +		ret |= regmap_read(pchip->regmap, REG_CL0_BRT_L, &rval_l);
>> +		break;
>> +	case LP8860_LED1:
>> +		ret = regmap_read(pchip->regmap, REG_CL1_BRT_H, &rval_h);
>> +		ret |= regmap_read(pchip->regmap, REG_CL1_BRT_L, &rval_l);
>> +		break;
>> +	case LP8860_LED2:
>> +		ret = regmap_read(pchip->regmap, REG_CL2_BRT_H, &rval_h);
>> +		ret |= regmap_read(pchip->regmap, REG_CL2_BRT_L, &rval_l);
>> +		break;
>> +	case LP8860_LED3:
>> +		ret = regmap_read(pchip->regmap, REG_CL3_BRT_H, &rval_h);
>> +		ret |= regmap_read(pchip->regmap, REG_CL3_BRT_L, &rval_l);
>> +		break;
>> +	default:
>> +		BUG();
>> +	}
>> +	if (ret < 0) {
>> +		dev_err(pchip->dev, "fail : i2c access to register.\n");
>> +		return ret;
>> +	}
>> +	bl->props.brightness = (rval_h << 8) | rval_l;
>> +	return bl->props.brightness;
>> +}
>> +
>> +static int lp8860_update_status_bled0(struct backlight_device *bl)
>> +{
>> +	return lp8860_bled_update_status(bl, LP8860_LED0);
>> +}
>> +
>> +static int lp8860_get_brightness_bled0(struct backlight_device *bl)
>> +{
>> +	return lp8860_bled_get_brightness(bl, LP8860_LED0);
>> +}
>> +
>> +static int lp8860_update_status_bled1(struct backlight_device *bl)
>> +{
>> +	return lp8860_bled_update_status(bl, LP8860_LED1);
>> +}
>> +
>> +static int lp8860_get_brightness_bled1(struct backlight_device *bl)
>> +{
>> +	return lp8860_bled_get_brightness(bl, LP8860_LED1);
>> +}
>> +
>> +static int lp8860_update_status_bled2(struct backlight_device *bl)
>> +{
>> +	return lp8860_bled_update_status(bl, LP8860_LED2);
>> +}
>> +
>> +static int lp8860_get_brightness_bled2(struct backlight_device *bl)
>> +{
>> +	return lp8860_bled_get_brightness(bl, LP8860_LED2);
>> +}
>> +
>> +static int lp8860_update_status_bled3(struct backlight_device *bl)
>> +{
>> +	return lp8860_bled_update_status(bl, LP8860_LED3);
>> +}
>> +
>> +static int lp8860_get_brightness_bled3(struct backlight_device *bl)
>> +{
>> +	return lp8860_bled_get_brightness(bl, LP8860_LED3);
>> +}
>> +
>> +#define lp8860_bled_ops(_id)\
>> +{\
>> +	.options = BL_CORE_SUSPENDRESUME,\
>> +	.update_status = lp8860_update_status_bled##_id,\
>> +	.get_brightness = lp8860_get_brightness_bled##_id,\
>> +}
>> +
>> +static const struct backlight_ops lp8860_bled_ops[LP8860_LED_MAX] = {
>> +	[LP8860_LED0] = lp8860_bled_ops(0),
>> +	[LP8860_LED1] = lp8860_bled_ops(1),
>> +	[LP8860_LED2] = lp8860_bled_ops(2),
>> +	[LP8860_LED3] = lp8860_bled_ops(3),
>> +};
>> +
>> +/* current control */
>> +static int lp8860_set_current(struct device *dev,
>> +			      const char *buf, enum lp8860_leds nsr)
>> +{
>> +	struct lp8860_chip *pchip = dev_get_drvdata(dev);
>> +	unsigned int ival;
>> +	ssize_t ret;
>> +
>> +	ret = kstrtouint(buf, 10, &ival);
>> +	if (ret)
>> +		return ret;
>> +
>> +	switch (nsr) {
>> +	case LP8860_LED0:
>> +		ival = min_t(unsigned int, ival, LP8860_LED0_BR_MAX);
>> +		ret = regmap_write(pchip->regmap, REG_CL0_I_H, ival >> 8);
>> +		ret |= regmap_write(pchip->regmap, REG_CL0_I_L, ival & 0xff);
>> +		break;
>> +	case LP8860_LED1:
>> +		ival = min_t(unsigned int, ival, LP8860_LED1_BR_MAX);
>> +		ret = regmap_write(pchip->regmap, REG_CL1_I, ival & 0xff);
>> +		break;
>> +	case LP8860_LED2:
>> +		ival = min_t(unsigned int, ival, LP8860_LED2_BR_MAX);
>> +		ret = regmap_write(pchip->regmap, REG_CL2_I, ival & 0xff);
>> +		break;
>> +	case LP8860_LED3:
>> +		ival = min_t(unsigned int, ival, LP8860_LED3_BR_MAX);
>> +		ret = regmap_write(pchip->regmap, REG_CL3_I, ival & 0xff);
>> +		break;
>> +	default:
>> +		BUG();
>> +	}
>> +	if (ret < 0)
>> +		dev_err(pchip->dev, "fail : i2c access error.\n");
>> +
>> +	return ret;
>> +}
>> +
>> +static ssize_t lp8860_current_store_bled0(struct device *dev,
>> +					  struct device_attribute *devAttr,
>> +					  const char *buf, size_t size)
>> +{
>> +	int ret;
>> +
>> +	ret = lp8860_set_current(dev, buf, LP8860_LED0);
>> +	if (ret < 0)
>> +		return ret;
>> +	return size;
>> +}
>> +
>> +static ssize_t lp8860_current_store_bled1(struct device *dev,
>> +					  struct device_attribute *devAttr,
>> +					  const char *buf, size_t size)
>> +{
>> +	int ret;
>> +
>> +	ret = lp8860_set_current(dev, buf, LP8860_LED1);
>> +	if (ret < 0)
>> +		return ret;
>> +	return size;
>> +}
>> +
>> +static ssize_t lp8860_current_store_bled2(struct device *dev,
>> +					  struct device_attribute *devAttr,
>> +					  const char *buf, size_t size)
>> +{
>> +	int ret;
>> +
>> +	ret = lp8860_set_current(dev, buf, LP8860_LED2);
>> +	if (ret < 0)
>> +		return ret;
>> +	return size;
>> +}
>> +
>> +static ssize_t lp8860_current_store_bled3(struct device *dev,
>> +					  struct device_attribute *devAttr,
>> +					  const char *buf, size_t size)
>> +{
>> +	int ret;
>> +
>> +	ret = lp8860_set_current(dev, buf, LP8860_LED3);
>> +	if (ret < 0)
>> +		return ret;
>> +	return size;
>> +}
>> +
>> +#define lp8860_attr(_name, _show, _store)\
>> +{\
>> +	.attr = {\
>> +		.name = _name,\
>> +		.mode = S_IWUSR,\
>> +	},\
>> +	.show = _show,\
>> +	.store = _store,\
>> +}
>> +
>> +static struct device_attribute lp8860_dev_attr[LP8860_LED_MAX] = {
>> +	[LP8860_LED0] = lp8860_attr("current", NULL,
>> +				    lp8860_current_store_bled0),
>> +	[LP8860_LED1] = lp8860_attr("current", NULL,
>> +				    lp8860_current_store_bled1),
>> +	[LP8860_LED2] = lp8860_attr("current", NULL,
>> +				    lp8860_current_store_bled2),
>> +	[LP8860_LED3] = lp8860_attr("current", NULL,
>> +				    lp8860_current_store_bled3),
>> +};
>> +
>> +/*
>> + * eeprom write and readback to check.
>> + * eeprom register range is from 60h to 78h
>> + * buffer value to write data [reg] [data]
>> + * e.g) to change the register 0x60 value to 0xff
>> + *      buffer value should be 60 ff
>> + */
>> +static ssize_t lp8860_eeprom_store(struct device *dev,
>> +				   struct device_attribute *devAttr,
>> +				   const char *buf, size_t size)
>> +{
>> +	struct lp8860_chip *pchip = dev_get_drvdata(dev);
>> +	unsigned int reg, data, rval;
>> +	char *tok;
>> +	int ret;
>> +
>> +	/* register no. */
>> +	tok = strsep((char **)&buf, " ,\n");
>> +	if (tok = NULL)
>> +		goto err_input;
>> +	ret = kstrtouint(tok, 16, &reg);
>> +	if (ret)
>> +		goto err_input;
>> +
>> +	/* register value */
>> +	tok = strsep((char **)&buf, " ,\n");
>> +	if (tok = NULL)
>> +		goto err_input;
>> +	ret = kstrtouint(tok, 16, &data);
>> +	if (ret)
>> +		goto err_input;
>> +	/*
>> +	 * EEPROM Programming sequence
>> +	 *    (program data permanently from registers to NVM)
>> +	 * 1. Unlock EEPROM by writing
>> +	 *    the unlock codes to register 1Ah(08, BA, EF)
>> +	 * 2. Write data to EEPROM registers (address 60h...78h)
>> +	 * 3. Write EE_PROG to 1 in address 19h. (02h to address 19h)
>> +	 * 4. Wait 100ms
> If possible, please add the reason why 100ms is necessary.
> 100ms is very huge delay.

These sequences are based on the datasheet.

>
>> +	 * 5. Write EE_PROG to 0 in address 19h. (00h to address 19h)
>> +	 */
>> +	if (reg < REG_EEPROM_START || reg > REG_EEPROM_END || data > 0xff)
>> +		goto err_input;
>> +	ret = regmap_write(pchip->regmap, REG_ROM_UNLOCK, 0x08);
>> +	ret |= regmap_write(pchip->regmap, REG_ROM_UNLOCK, 0xba);
>> +	ret |= regmap_write(pchip->regmap, REG_ROM_UNLOCK, 0xef);
>> +	ret |= regmap_write(pchip->regmap, reg, data);
>> +	ret |= regmap_write(pchip->regmap, REG_ROM_CTRL, 0x02);
>> +	msleep(100);
>> +	ret |= regmap_write(pchip->regmap, REG_ROM_CTRL, 0x00);
>> +	if (ret < 0)
>> +		goto err_i2c;
>> +
>> +	/* read back */
>> +	ret = regmap_write(pchip->regmap, REG_ROM_UNLOCK, 0x08);
>> +	ret |= regmap_write(pchip->regmap, REG_ROM_UNLOCK, 0xba);
>> +	ret |= regmap_write(pchip->regmap, REG_ROM_UNLOCK, 0xef);
>> +	ret |= regmap_write(pchip->regmap, REG_ROM_CTRL, 0x01);
>> +	msleep(100);
>> +	ret |= regmap_write(pchip->regmap, REG_ROM_CTRL, 0x00);
>> +	ret |= regmap_read(pchip->regmap, reg, &rval);
>> +	if (ret < 0)
>> +		goto err_i2c;
>> +	if (rval != data)
>> +		dev_err(pchip->dev, "fail : eeprom did not change.\n");
>> +
>> +	return size;
>> +
>> +err_i2c:
>> +	dev_err(pchip->dev, "fail : i2c access error.\n");
>> +	return ret;
>> +
>> +err_input:
>> +	dev_err(pchip->dev, "fail : input fail.\n");
>> +	return -EINVAL;
>> +}
>> +
>> +static DEVICE_ATTR(eeprom, S_IWUSR, NULL, lp8860_eeprom_store);
>> +
>> +/* backlight register and remove */
>> +static char *lp8860_bled_name[LP8860_LED_MAX] = {
>> +	[LP8860_LED0] = "bled0",
>> +	[LP8860_LED1] = "bled1",
>> +	[LP8860_LED2] = "bled2",
>> +	[LP8860_LED3] = "bled3",
>> +};
>> +
>> +static int lp8860_backlight_remove(struct lp8860_chip *pchip)
>> +{
>> +	int icnt;
>> +
>> +	device_remove_file(&(pchip->bled[0]->dev), &dev_attr_eeprom);
>> +	for (icnt = LP8860_LED0; icnt < LP8860_LED_MAX; icnt++) {
>> +		if (pchip->bled[icnt]) {
>> +			backlight_device_unregister(pchip->bled[icnt]);
>> +			device_remove_file(&(pchip->bled[icnt]->dev),
>> +					   &lp8860_dev_attr[icnt]);
>> +		}
>> +	}
>> +	return 0;
>> +}
>> +
>> +static int lp8860_backlight_registers(struct lp8860_chip *pchip)
>> +{
>> +	struct backlight_properties props;
>> +	struct lp8860_platform_data *pdata = pchip->pdata;
>> +	int icnt, ret;
>> +
>> +	props.type = BACKLIGHT_RAW;
>> +	for (icnt = LP8860_LED0; icnt < LP8860_LED_MAX; icnt++) {
>> +		props.max_brightness = pdata->max_brt[icnt];
>> +		pchip->bled[icnt] >> +		    backlight_device_register(lp8860_bled_name[icnt],
> devm_* functions will make the code simpler.
> Thus, please use devm_backlight_device_register(), instead of
> backlight_device_register().
>
>> +					      pchip->dev, pchip,
>> +					      &lp8860_bled_ops[icnt], &props);
>> +		if (IS_ERR(pchip->bled[icnt])) {
>> +			dev_err(pchip->dev, "fail : backlight register.\n");
>> +			ret = PTR_ERR(pchip->bled[icnt]);
>> +			goto err_out;
>> +		}
>> +
>> +		ret = device_create_file(&(pchip->bled[icnt]->dev),
>> +					 &lp8860_dev_attr[icnt]);
>> +		if (ret < 0) {
>> +			dev_err(pchip->dev, "fail : to add sysfs entries.\n");
>> +			goto err_out;
>> +		}
>> +	}
>> +	/* access eeprom */
>> +	ret = device_create_file(&(pchip->bled[LP8860_LED0]->dev),
>> +				 &dev_attr_eeprom);
>> +	if (ret < 0) {
>> +		dev_err(pchip->dev, "fail : to add sysfs entries.\n");
>> +		goto err_out;
>> +	}
>> +	return 0;
>> +
>> +err_out:
>> +	lp8860_backlight_remove(pchip);
>> +	return ret;
>> +}
>> +
>> +static const struct regmap_config lp8860_regmap = {
>> +	.reg_bits = 8,
>> +	.val_bits = 8,
>> +	.max_register = REG_MAX,
>> +};
>> +
>> +static int lp8860_probe(struct i2c_client *client,
>> +			const struct i2c_device_id *id)
>> +{
>> +	struct lp8860_chip *pchip;
>> +	struct lp8860_platform_data *pdata = dev_get_platdata(&client->dev);
>> +	int ret, icnt;
>> +
>> +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
>> +		dev_err(&client->dev, "fail : i2c functionality check.\n");
>> +		return -EOPNOTSUPP;
>> +	}
>> +
>> +	pchip = devm_kzalloc(&client->dev,
>> +			     sizeof(struct lp8860_chip), GFP_KERNEL);
>> +	if (!pchip)
>> +		return -ENOMEM;
>> +	pchip->dev = &client->dev;
>> +
>> +	pchip->regmap = devm_regmap_init_i2c(client, &lp8860_regmap);
>> +	if (IS_ERR(pchip->regmap)) {
>> +		ret = PTR_ERR(pchip->regmap);
>> +		dev_err(pchip->dev, "fail : allocate i2c register map.\n");
>> +		return ret;
>> +	}
>> +
>> +	if (pdata = NULL) {
>> +		pdata = devm_kzalloc(pchip->dev,
>> +				     sizeof(struct lp8860_platform_data),
>> +				     GFP_KERNEL);
>> +		if (pdata = NULL)
>> +			return -ENOMEM;
>> +		pdata->max_brt[LP8860_LED0] = 65535;
>> +		for (icnt = LP8860_LED1; icnt < LP8860_LED_MAX; icnt++)
>> +			pdata->max_brt[icnt] = 8191;
>> +		pchip->pdata = pdata;
>> +	} else {
>> +		pchip->pdata = pdata;
>> +	}
>> +	i2c_set_clientdata(client, pchip);
>> +	ret = lp8860_backlight_registers(pchip);
>> +	return ret;
>> +}
>> +
>> +static int lp8860_remove(struct i2c_client *client)
>> +{
>> +	return lp8860_backlight_remove(i2c_get_clientdata(client));
>> +}
>> +
>> +static const struct i2c_device_id lp8860_id[] = {
>> +	{LP8860_NAME, 0},
>> +	{}
>> +};
>> +
>> +MODULE_DEVICE_TABLE(i2c, lp8860_id);
>> +static struct i2c_driver lp8860_i2c_driver = {
>> +	.driver = {
>> +		   .name = LP8860_NAME,
>> +		   },
> Fix the indent style as below.
>
>   +	   },
>
>> +	.probe = lp8860_probe,
>> +	.remove = lp8860_remove,
>> +	.id_table = lp8860_id,
>> +};
>> +
>> +module_i2c_driver(lp8860_i2c_driver);
>> +
>> +MODULE_DESCRIPTION("Texas Instruments LP8860 Backlight Driver");
>> +MODULE_AUTHOR("Daniel Jeong <gshark.jeong@gmail.com>");
>> +MODULE_AUTHOR("Ldd Mlp <ldd-mlp@list.ti.com>");
>> +MODULE_LICENSE("GPL v2");
>> diff --git a/include/linux/platform_data/lp8860_bl.h b/include/linux/platform_data/lp8860_bl.h
>> new file mode 100644
>> index 0000000..61bd0f5
>> --- /dev/null
>> +++ b/include/linux/platform_data/lp8860_bl.h
>> @@ -0,0 +1,54 @@
>> +/*
>> + * Simple driver for Texas Instruments LM3642 LED Flash driver chip
>> + * Copyright (C) 2014 Texas Instruments
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + */
>> +
>> +#ifndef __LINUX_LP8860_H
>> +#define __LINUX_LP8860_H
> Prefix 'LINUX' looks redundant.
> Please replace '__LINUX_LP8860_H' with '__LP8860_H'.
>
> If the datasheet of 'lp8860' is not closed, please let us
> know where the datasheet is. It would be helpful for reviewing
> your patch.

Officially the datasheet will be opened next month but as of now I can't open it.

> Best regards,
> Jingoo Han
>
>> +
>> +#define LP8860_NAME "lp8860"
>> +#define LP8860_ADDR 0x2d
>> +
>> +#define LP8860_LED0_BR_MAX 65535
>> +#define LP8860_LED1_BR_MAX 8191
>> +#define LP8860_LED2_BR_MAX 8191
>> +#define LP8860_LED3_BR_MAX 8191
>> +
>> +#define LP8860_LED0_I_MAX 4095
>> +#define LP8860_LED1_I_MAX 255
>> +#define LP8860_LED2_I_MAX 255
>> +#define LP8860_LED3_I_MAX 255
>> +
>> +enum lp8860_leds {
>> +	LP8860_LED0 = 0,
>> +	LP8860_LED1,
>> +	LP8860_LED2,
>> +	LP8860_LED3,
>> +	LP8860_LED_MAX
>> +};
>> +
>> +enum lp8860_ctrl_mode {
>> +	LP8860_CTRL_I2C = 0,
>> +	LP8860_CTRL_I2C_PWM,
>> +};
>> +
>> +/* struct lp8860 platform data
>> + * @mode : control mode
>> + * @max_brt : maximum brightness.
>> + *		LED0 0 ~ 65535
>> + *		LED1 0 ~ 8191
>> + *		LED2 0 ~ 8191
>> + *		LED3 0 ~ 8191
>> + */
>> +struct lp8860_platform_data {
>> +
>> +	enum lp8860_ctrl_mode mode;
>> +	int max_brt[LP8860_LED_MAX];
>> +};
>> +
>> +#endif /* __LINUX_LP8860_H */
>> --
>> 1.7.9.5


^ permalink raw reply

* [PATCH v2] backlight: add new LP8860 backlight driver
From: Daniel Jeong @ 2014-03-13  5:03 UTC (permalink / raw)
  To: Jingoo Han
  Cc: Daniel Jeong, linux-fbdev, linux-kernel,
	Jean-Christophe Plagniol-Villard, Tomi Valkeinen, Bryan Wu,
	Lee Jones

 This patch adds LP8860 backlight device driver.
LP8860 is a low EMI and High performance 4 channel LED Driver of TI.
This device driver provides the way to control brightness and current
of each channel and provides the way to change values in the eeprom.

Signed-off-by: Daniel Jeong <gshark.jeong@gmail.com>
---
 drivers/video/backlight/Kconfig         |    7 +
 drivers/video/backlight/Makefile        |    1 +
 drivers/video/backlight/lp8860_bl.c     |  525 +++++++++++++++++++++++++++++++
 include/linux/platform_data/lp8860_bl.h |   54 ++++
 4 files changed, 587 insertions(+)
 create mode 100644 drivers/video/backlight/lp8860_bl.c
 create mode 100644 include/linux/platform_data/lp8860_bl.h

diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index 5a3eb2e..908048f 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -397,6 +397,13 @@ config BACKLIGHT_LP8788
 	help
 	  This supports TI LP8788 backlight driver.
 
+config BACKLIGHT_LP8860
+	tristate "Backlight Driver for LP8860"
+	depends on BACKLIGHT_CLASS_DEVICE && I2C
+	select REGMAP_I2C
+	help
+	  This supports TI LP8860 Backlight Driver
+
 config BACKLIGHT_OT200
 	tristate "Backlight driver for ot200 visualisation device"
 	depends on BACKLIGHT_CLASS_DEVICE && CS5535_MFGPT && GPIO_CS5535
diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
index bb82002..cbc5ac3 100644
--- a/drivers/video/backlight/Makefile
+++ b/drivers/video/backlight/Makefile
@@ -42,6 +42,7 @@ obj-$(CONFIG_BACKLIGHT_LM3639)		+= lm3639_bl.o
 obj-$(CONFIG_BACKLIGHT_LOCOMO)		+= locomolcd.o
 obj-$(CONFIG_BACKLIGHT_LP855X)		+= lp855x_bl.o
 obj-$(CONFIG_BACKLIGHT_LP8788)		+= lp8788_bl.o
+obj-$(CONFIG_BACKLIGHT_LP8860)		+= lp8860_bl.o
 obj-$(CONFIG_BACKLIGHT_LV5207LP)	+= lv5207lp.o
 obj-$(CONFIG_BACKLIGHT_MAX8925)		+= max8925_bl.o
 obj-$(CONFIG_BACKLIGHT_OMAP1)		+= omap1_bl.o
diff --git a/drivers/video/backlight/lp8860_bl.c b/drivers/video/backlight/lp8860_bl.c
new file mode 100644
index 0000000..d2be950
--- /dev/null
+++ b/drivers/video/backlight/lp8860_bl.c
@@ -0,0 +1,525 @@
+/*
+* Simple driver for Texas Instruments lp8860 Backlight driver chip
+*
+* Copyright (C) 2014 Texas Instruments
+* Author: Daniel Jeong  <gshark.jeong@gmail.com>
+*		  Ldd Mlp <ldd-mlp@list.ti.com>
+*
+* This program is free software; you can redistribute it and/or modify
+* it under the terms of the GNU General Public License version 2 as
+* published by the Free Software Foundation.
+*
+*/
+#include <linux/backlight.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/platform_data/lp8860_bl.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <linux/uaccess.h>
+
+#define REG_CL0_BRT_H	0x00
+#define REG_CL0_BRT_L	0x01
+#define REG_CL0_I_H		0x02
+#define REG_CL0_I_L		0x03
+
+#define REG_CL1_BRT_H	0x04
+#define REG_CL1_BRT_L	0x05
+#define REG_CL1_I		0x06
+
+#define REG_CL2_BRT_H	0x07
+#define REG_CL2_BRT_L	0x08
+#define REG_CL2_I		0x09
+
+#define REG_CL3_BRT_H	0x0a
+#define REG_CL3_BRT_L	0x0b
+#define REG_CL3_I		0x0c
+
+#define REG_CONF	0x0d
+#define REG_STATUS	0x0e
+#define REG_ID		0x12
+
+#define REG_ROM_CTRL	0x19
+#define REG_ROM_UNLOCK	0x1a
+#define REG_ROM_START	0x60
+#define REG_ROM_END		0x78
+
+#define REG_EEPROM_START	0x60
+#define REG_EEPROM_END		0x78
+#define REG_MAX	0xFF
+
+struct lp8860_chip {
+	struct device *dev;
+	struct lp8860_platform_data *pdata;
+	struct backlight_device *bled[LP8860_LED_MAX];
+	struct regmap *regmap;
+};
+
+/* brightness control */
+static int lp8860_bled_update_status(struct backlight_device *bl,
+				     enum lp8860_leds nsr)
+{
+	int ret = -EINVAL;
+	struct lp8860_chip *pchip = bl_get_data(bl);
+
+	if (pchip->pdata->mode)
+		return 0;
+
+	if (bl->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK))
+		bl->props.brightness = 0;
+
+	switch (nsr) {
+	case LP8860_LED0:
+		ret = regmap_write(pchip->regmap,
+				   REG_CL0_BRT_H, bl->props.brightness >> 8);
+		ret |= regmap_write(pchip->regmap,
+				    REG_CL0_BRT_L, bl->props.brightness & 0xff);
+		break;
+	case LP8860_LED1:
+		ret = regmap_write(pchip->regmap,
+				   REG_CL1_BRT_H,
+				   (bl->props.brightness >> 8) & 0x1f);
+		ret |= regmap_write(pchip->regmap,
+				    REG_CL1_BRT_L, bl->props.brightness & 0xff);
+		break;
+	case LP8860_LED2:
+		ret = regmap_write(pchip->regmap,
+				   REG_CL2_BRT_H,
+				   (bl->props.brightness >> 8) & 0x1f);
+		ret |= regmap_write(pchip->regmap,
+				    REG_CL2_BRT_L, bl->props.brightness & 0xff);
+		break;
+	case LP8860_LED3:
+		ret = regmap_write(pchip->regmap,
+				   REG_CL3_BRT_H,
+				   (bl->props.brightness >> 8) & 0x1f);
+		ret |= regmap_write(pchip->regmap,
+				    REG_CL3_BRT_L, bl->props.brightness & 0xff);
+		break;
+	default:
+		BUG();
+	}
+	if (ret < 0)
+		dev_err(pchip->dev, "fail : i2c access to register.\n");
+	else
+		ret = bl->props.brightness;
+
+	return ret;
+}
+
+static int lp8860_bled_get_brightness(struct backlight_device *bl,
+				      enum lp8860_leds nsr)
+{
+	struct lp8860_chip *pchip = bl_get_data(bl);
+	unsigned int rval_h, rval_l;
+	int ret = -EINVAL;
+
+	switch (nsr) {
+	case LP8860_LED0:
+		ret = regmap_read(pchip->regmap, REG_CL0_BRT_H, &rval_h);
+		ret |= regmap_read(pchip->regmap, REG_CL0_BRT_L, &rval_l);
+		break;
+	case LP8860_LED1:
+		ret = regmap_read(pchip->regmap, REG_CL1_BRT_H, &rval_h);
+		ret |= regmap_read(pchip->regmap, REG_CL1_BRT_L, &rval_l);
+		break;
+	case LP8860_LED2:
+		ret = regmap_read(pchip->regmap, REG_CL2_BRT_H, &rval_h);
+		ret |= regmap_read(pchip->regmap, REG_CL2_BRT_L, &rval_l);
+		break;
+	case LP8860_LED3:
+		ret = regmap_read(pchip->regmap, REG_CL3_BRT_H, &rval_h);
+		ret |= regmap_read(pchip->regmap, REG_CL3_BRT_L, &rval_l);
+		break;
+	default:
+		BUG();
+	}
+	if (ret < 0) {
+		dev_err(pchip->dev, "fail : i2c access to register.\n");
+		return ret;
+	}
+	bl->props.brightness = (rval_h << 8) | rval_l;
+	return bl->props.brightness;
+}
+
+static int lp8860_update_status_bled0(struct backlight_device *bl)
+{
+	return lp8860_bled_update_status(bl, LP8860_LED0);
+}
+
+static int lp8860_get_brightness_bled0(struct backlight_device *bl)
+{
+	return lp8860_bled_get_brightness(bl, LP8860_LED0);
+}
+
+static int lp8860_update_status_bled1(struct backlight_device *bl)
+{
+	return lp8860_bled_update_status(bl, LP8860_LED1);
+}
+
+static int lp8860_get_brightness_bled1(struct backlight_device *bl)
+{
+	return lp8860_bled_get_brightness(bl, LP8860_LED1);
+}
+
+static int lp8860_update_status_bled2(struct backlight_device *bl)
+{
+	return lp8860_bled_update_status(bl, LP8860_LED2);
+}
+
+static int lp8860_get_brightness_bled2(struct backlight_device *bl)
+{
+	return lp8860_bled_get_brightness(bl, LP8860_LED2);
+}
+
+static int lp8860_update_status_bled3(struct backlight_device *bl)
+{
+	return lp8860_bled_update_status(bl, LP8860_LED3);
+}
+
+static int lp8860_get_brightness_bled3(struct backlight_device *bl)
+{
+	return lp8860_bled_get_brightness(bl, LP8860_LED3);
+}
+
+#define lp8860_bled_ops(_id)\
+{\
+	.options = BL_CORE_SUSPENDRESUME,\
+	.update_status = lp8860_update_status_bled##_id,\
+	.get_brightness = lp8860_get_brightness_bled##_id,\
+}
+
+static const struct backlight_ops lp8860_bled_ops[LP8860_LED_MAX] = {
+	[LP8860_LED0] = lp8860_bled_ops(0),
+	[LP8860_LED1] = lp8860_bled_ops(1),
+	[LP8860_LED2] = lp8860_bled_ops(2),
+	[LP8860_LED3] = lp8860_bled_ops(3),
+};
+
+/* current control */
+static int lp8860_set_current(struct device *dev,
+			      const char *buf, enum lp8860_leds nsr)
+{
+	struct lp8860_chip *pchip = dev_get_drvdata(dev);
+	unsigned int ival;
+	ssize_t ret;
+
+	ret = kstrtouint(buf, 10, &ival);
+	if (ret)
+		return ret;
+
+	switch (nsr) {
+	case LP8860_LED0:
+		ival = min_t(unsigned int, ival, LP8860_LED0_BR_MAX);
+		ret = regmap_write(pchip->regmap, REG_CL0_I_H, ival >> 8);
+		ret |= regmap_write(pchip->regmap, REG_CL0_I_L, ival & 0xff);
+		break;
+	case LP8860_LED1:
+		ival = min_t(unsigned int, ival, LP8860_LED1_BR_MAX);
+		ret = regmap_write(pchip->regmap, REG_CL1_I, ival & 0xff);
+		break;
+	case LP8860_LED2:
+		ival = min_t(unsigned int, ival, LP8860_LED2_BR_MAX);
+		ret = regmap_write(pchip->regmap, REG_CL2_I, ival & 0xff);
+		break;
+	case LP8860_LED3:
+		ival = min_t(unsigned int, ival, LP8860_LED3_BR_MAX);
+		ret = regmap_write(pchip->regmap, REG_CL3_I, ival & 0xff);
+		break;
+	default:
+		BUG();
+	}
+	if (ret < 0)
+		dev_err(pchip->dev, "fail : i2c access error.\n");
+
+	return ret;
+}
+
+static ssize_t lp8860_current_store_bled0(struct device *dev,
+					  struct device_attribute *devAttr,
+					  const char *buf, size_t size)
+{
+	int ret;
+
+	ret = lp8860_set_current(dev, buf, LP8860_LED0);
+	if (ret < 0)
+		return ret;
+	return size;
+}
+
+static ssize_t lp8860_current_store_bled1(struct device *dev,
+					  struct device_attribute *devAttr,
+					  const char *buf, size_t size)
+{
+	int ret;
+
+	ret = lp8860_set_current(dev, buf, LP8860_LED1);
+	if (ret < 0)
+		return ret;
+	return size;
+}
+
+static ssize_t lp8860_current_store_bled2(struct device *dev,
+					  struct device_attribute *devAttr,
+					  const char *buf, size_t size)
+{
+	int ret;
+
+	ret = lp8860_set_current(dev, buf, LP8860_LED2);
+	if (ret < 0)
+		return ret;
+	return size;
+}
+
+static ssize_t lp8860_current_store_bled3(struct device *dev,
+					  struct device_attribute *devAttr,
+					  const char *buf, size_t size)
+{
+	int ret;
+
+	ret = lp8860_set_current(dev, buf, LP8860_LED3);
+	if (ret < 0)
+		return ret;
+	return size;
+}
+
+#define lp8860_attr(_name, _show, _store)\
+{\
+	.attr = {\
+		.name = _name,\
+		.mode = S_IWUSR,\
+	},\
+	.show = _show,\
+	.store = _store,\
+}
+
+static struct device_attribute lp8860_dev_attr[LP8860_LED_MAX] = {
+	[LP8860_LED0] = lp8860_attr("current", NULL,
+				    lp8860_current_store_bled0),
+	[LP8860_LED1] = lp8860_attr("current", NULL,
+				    lp8860_current_store_bled1),
+	[LP8860_LED2] = lp8860_attr("current", NULL,
+				    lp8860_current_store_bled2),
+	[LP8860_LED3] = lp8860_attr("current", NULL,
+				    lp8860_current_store_bled3),
+};
+
+/*
+ * eeprom write and readback to check.
+ * eeprom register range is from 60h to 78h
+ * buffer value to write data [reg] [data]
+ * e.g) to change the register 0x60 value to 0xff
+ *      buffer value should be 60 ff
+ */
+static ssize_t lp8860_eeprom_store(struct device *dev,
+				   struct device_attribute *devAttr,
+				   const char *buf, size_t size)
+{
+	struct lp8860_chip *pchip = dev_get_drvdata(dev);
+	unsigned int reg, data, rval;
+	char *tok;
+	int ret;
+
+	/* register no. */
+	tok = strsep((char **)&buf, " ,\n");
+	if (tok = NULL)
+		goto err_input;
+	ret = kstrtouint(tok, 16, &reg);
+	if (ret)
+		goto err_input;
+
+	/* register value */
+	tok = strsep((char **)&buf, " ,\n");
+	if (tok = NULL)
+		goto err_input;
+	ret = kstrtouint(tok, 16, &data);
+	if (ret)
+		goto err_input;
+	/*
+	 * EEPROM Programming sequence
+	 *    (program data permanently from registers to NVM)
+	 * 1. Unlock EEPROM by writing
+	 *    the unlock codes to register 1Ah(08, BA, EF)
+	 * 2. Write data to EEPROM registers (address 60h...78h)
+	 * 3. Write EE_PROG to 1 in address 19h. (02h to address 19h)
+	 * 4. Wait 100ms for chip interanl dealy to access the eeprom
+	 * 5. Write EE_PROG to 0 in address 19h. (00h to address 19h)
+	 */
+	if (reg < REG_EEPROM_START || reg > REG_EEPROM_END || data > 0xff)
+		goto err_input;
+	ret = regmap_write(pchip->regmap, REG_ROM_UNLOCK, 0x08);
+	ret |= regmap_write(pchip->regmap, REG_ROM_UNLOCK, 0xba);
+	ret |= regmap_write(pchip->regmap, REG_ROM_UNLOCK, 0xef);
+	ret |= regmap_write(pchip->regmap, reg, data);
+	ret |= regmap_write(pchip->regmap, REG_ROM_CTRL, 0x02);
+	msleep(100);
+	ret |= regmap_write(pchip->regmap, REG_ROM_CTRL, 0x00);
+	if (ret < 0)
+		goto err_i2c;
+
+	/* read back */
+	ret = regmap_write(pchip->regmap, REG_ROM_UNLOCK, 0x08);
+	ret |= regmap_write(pchip->regmap, REG_ROM_UNLOCK, 0xba);
+	ret |= regmap_write(pchip->regmap, REG_ROM_UNLOCK, 0xef);
+	ret |= regmap_write(pchip->regmap, REG_ROM_CTRL, 0x01);
+	msleep(100);
+	ret |= regmap_write(pchip->regmap, REG_ROM_CTRL, 0x00);
+	ret |= regmap_read(pchip->regmap, reg, &rval);
+	if (ret < 0)
+		goto err_i2c;
+	if (rval != data)
+		dev_err(pchip->dev, "fail : eeprom did not change.\n");
+
+	return size;
+
+err_i2c:
+	dev_err(pchip->dev, "fail : i2c access error.\n");
+	return ret;
+
+err_input:
+	dev_err(pchip->dev, "fail : input fail.\n");
+	return -EINVAL;
+}
+
+static DEVICE_ATTR(eeprom, S_IWUSR, NULL, lp8860_eeprom_store);
+
+/* backlight register and remove */
+static char *lp8860_bled_name[LP8860_LED_MAX] = {
+	[LP8860_LED0] = "bled0",
+	[LP8860_LED1] = "bled1",
+	[LP8860_LED2] = "bled2",
+	[LP8860_LED3] = "bled3",
+};
+
+static int lp8860_backlight_remove(struct lp8860_chip *pchip)
+{
+	int icnt;
+
+	device_remove_file(&(pchip->bled[0]->dev), &dev_attr_eeprom);
+	for (icnt = LP8860_LED0; icnt < LP8860_LED_MAX; icnt++) {
+		if (pchip->bled[icnt])
+			device_remove_file(&(pchip->bled[icnt]->dev),
+					   &lp8860_dev_attr[icnt]);
+	}
+	return 0;
+}
+
+static int lp8860_backlight_registers(struct lp8860_chip *pchip)
+{
+	struct backlight_properties props;
+	struct lp8860_platform_data *pdata = pchip->pdata;
+	int icnt, ret;
+
+	props.type = BACKLIGHT_RAW;
+	for (icnt = LP8860_LED0; icnt < LP8860_LED_MAX; icnt++) {
+		props.max_brightness = pdata->max_brt[icnt];
+		pchip->bled[icnt] +		    devm_backlight_device_register(pchip->dev,
+						   lp8860_bled_name[icnt],
+						   pchip->dev, pchip,
+						   &lp8860_bled_ops[icnt],
+						   &props);
+		if (IS_ERR(pchip->bled[icnt])) {
+			dev_err(pchip->dev, "fail : backlight register.\n");
+			ret = PTR_ERR(pchip->bled[icnt]);
+			goto err_out;
+		}
+		/* to control current */
+		ret = device_create_file(&(pchip->bled[icnt]->dev),
+					 &lp8860_dev_attr[icnt]);
+		if (ret < 0) {
+			dev_err(pchip->dev, "fail : to add sysfs entries.\n");
+			goto err_out;
+		}
+	}
+	/* to access eeprom */
+	ret = device_create_file(&(pchip->bled[LP8860_LED0]->dev),
+				 &dev_attr_eeprom);
+	if (ret < 0) {
+		dev_err(pchip->dev, "fail : to add sysfs entries.\n");
+		goto err_out;
+	}
+	return 0;
+
+err_out:
+	lp8860_backlight_remove(pchip);
+	return ret;
+}
+
+static const struct regmap_config lp8860_regmap = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = REG_MAX,
+};
+
+static int lp8860_probe(struct i2c_client *client,
+			const struct i2c_device_id *id)
+{
+	struct lp8860_chip *pchip;
+	struct lp8860_platform_data *pdata = dev_get_platdata(&client->dev);
+	int ret, icnt;
+
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
+		dev_err(&client->dev, "fail : i2c functionality check.\n");
+		return -EOPNOTSUPP;
+	}
+
+	pchip = devm_kzalloc(&client->dev,
+			     sizeof(struct lp8860_chip), GFP_KERNEL);
+	if (!pchip)
+		return -ENOMEM;
+	pchip->dev = &client->dev;
+
+	pchip->regmap = devm_regmap_init_i2c(client, &lp8860_regmap);
+	if (IS_ERR(pchip->regmap)) {
+		ret = PTR_ERR(pchip->regmap);
+		dev_err(pchip->dev, "fail : allocate i2c register map.\n");
+		return ret;
+	}
+
+	if (pdata = NULL) {
+		pdata = devm_kzalloc(pchip->dev,
+				     sizeof(struct lp8860_platform_data),
+				     GFP_KERNEL);
+		if (pdata = NULL)
+			return -ENOMEM;
+		pdata->max_brt[LP8860_LED0] = 65535;
+		for (icnt = LP8860_LED1; icnt < LP8860_LED_MAX; icnt++)
+			pdata->max_brt[icnt] = 8191;
+		pchip->pdata = pdata;
+	} else {
+		pchip->pdata = pdata;
+	}
+	i2c_set_clientdata(client, pchip);
+	ret = lp8860_backlight_registers(pchip);
+	return ret;
+}
+
+static int lp8860_remove(struct i2c_client *client)
+{
+	return lp8860_backlight_remove(i2c_get_clientdata(client));
+}
+
+static const struct i2c_device_id lp8860_id[] = {
+	{LP8860_NAME, 0},
+	{}
+};
+
+MODULE_DEVICE_TABLE(i2c, lp8860_id);
+static struct i2c_driver lp8860_i2c_driver = {
+	.driver = {
+		.name = LP8860_NAME,
+	},
+	.probe = lp8860_probe,
+	.remove = lp8860_remove,
+	.id_table = lp8860_id,
+};
+
+module_i2c_driver(lp8860_i2c_driver);
+
+MODULE_DESCRIPTION("Texas Instruments LP8860 Backlight Driver");
+MODULE_AUTHOR("Daniel Jeong <gshark.jeong@gmail.com>");
+MODULE_AUTHOR("Ldd Mlp <ldd-mlp@list.ti.com>");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/platform_data/lp8860_bl.h b/include/linux/platform_data/lp8860_bl.h
new file mode 100644
index 0000000..9edd7cf
--- /dev/null
+++ b/include/linux/platform_data/lp8860_bl.h
@@ -0,0 +1,54 @@
+/*
+ * Simple driver for Texas Instruments LP8860 Backlight driver chip
+ * Copyright (C) 2014 Texas Instruments
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#ifndef __LP8860_H
+#define __LP8860_H
+
+#define LP8860_NAME "lp8860"
+#define LP8860_ADDR 0x2d
+
+#define LP8860_LED0_BR_MAX 65535
+#define LP8860_LED1_BR_MAX 8191
+#define LP8860_LED2_BR_MAX 8191
+#define LP8860_LED3_BR_MAX 8191
+
+#define LP8860_LED0_I_MAX 4095
+#define LP8860_LED1_I_MAX 255
+#define LP8860_LED2_I_MAX 255
+#define LP8860_LED3_I_MAX 255
+
+enum lp8860_leds {
+	LP8860_LED0 = 0,
+	LP8860_LED1,
+	LP8860_LED2,
+	LP8860_LED3,
+	LP8860_LED_MAX
+};
+
+enum lp8860_ctrl_mode {
+	LP8860_CTRL_I2C = 0,
+	LP8860_CTRL_I2C_PWM,
+};
+
+/* struct lp8860 platform data
+ * @mode : control mode
+ * @max_brt : maximum brightness.
+ *		LED0 0 ~ 65535
+ *		LED1 0 ~ 8191
+ *		LED2 0 ~ 8191
+ *		LED3 0 ~ 8191
+ */
+struct lp8860_platform_data {
+
+	enum lp8860_ctrl_mode mode;
+	int max_brt[LP8860_LED_MAX];
+};
+
+#endif /* __LP8860_H */
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH] lcd: Provide dummy functions if CONFIG_LCD_CLASS_DEVICE is not set
From: Alexander Shiyan @ 2014-03-13  7:31 UTC (permalink / raw)
  To: linux-fbdev

Provide dummy functions for LCD register()/unregister() if
CONFIG_LCD_CLASS_DEVICE is not set.

Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
---
 include/linux/lcd.h | 37 ++++++++++++++++++++++++++++++-------
 1 file changed, 30 insertions(+), 7 deletions(-)

diff --git a/include/linux/lcd.h b/include/linux/lcd.h
index 504f624..f1c94fd 100644
--- a/include/linux/lcd.h
+++ b/include/linux/lcd.h
@@ -110,14 +110,37 @@ static inline void lcd_set_power(struct lcd_device *ld, int power)
 	mutex_unlock(&ld->update_lock);
 }
 
-extern struct lcd_device *lcd_device_register(const char *name,
-	struct device *parent, void *devdata, struct lcd_ops *ops);
-extern struct lcd_device *devm_lcd_device_register(struct device *dev,
-	const char *name, struct device *parent,
+#if defined(CONFIG_LCD_CLASS_DEVICE) || defined(CONFIG_LCD_CLASS_DEVICE_MODULE)
+struct lcd_device *lcd_device_register(const char *name, struct device *parent,
 	void *devdata, struct lcd_ops *ops);
-extern void lcd_device_unregister(struct lcd_device *ld);
-extern void devm_lcd_device_unregister(struct device *dev,
-	struct lcd_device *ld);
+struct lcd_device *devm_lcd_device_register(struct device *dev,
+	const char *name, struct device *parent, void *devdata,
+	struct lcd_ops *ops);
+void lcd_device_unregister(struct lcd_device *ld);
+void devm_lcd_device_unregister(struct device *dev, struct lcd_device *ld);
+#else
+static inline struct lcd_device *lcd_device_register(const char *name,
+	struct device *parent, void *devdata, struct lcd_ops *ops)
+{
+	return ERR_PTR(-ENOSYS);
+}
+
+static inline struct lcd_device *devm_lcd_device_register(struct device *dev,
+	const char *name, struct device *parent, void *devdata,
+	struct lcd_ops *ops)
+{
+	return ERR_PTR(-ENOSYS);
+}
+
+static inline void lcd_device_unregister(struct lcd_device *ld)
+{
+}
+
+static inline void devm_lcd_device_unregister(struct device *dev,
+	struct lcd_device *ld)
+{
+}
+#endif
 
 #define to_lcd_device(obj) container_of(obj, struct lcd_device, dev)
 
-- 
1.8.3.2


^ permalink raw reply related

* Re: [PATCH] lcd: Provide dummy functions if CONFIG_LCD_CLASS_DEVICE is not set
From: Tomi Valkeinen @ 2014-03-13  8:23 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1394695879-22845-1-git-send-email-shc_work@mail.ru>

[-- Attachment #1: Type: text/plain, Size: 262 bytes --]

On 13/03/14 09:31, Alexander Shiyan wrote:
> Provide dummy functions for LCD register()/unregister() if
> CONFIG_LCD_CLASS_DEVICE is not set.

Hmm, why do you want to do that? If a driver needs the LCD class, it
should depend on or select it.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

^ permalink raw reply

* Re: [PATCH] lcd: Provide dummy functions if C =?UTF-8?B?T05GSUdfTENEX0NMQ
From: Alexander Shiyan @ 2014-03-13  8:48 UTC (permalink / raw)
  To: linux-fbdev

0KfQtdGC0LLQtdGA0LMsIDEzINC80LDRgNGC0LAgMjAxNCwgMTA6MjMgKzAyOjAwINC+0YIgVG9t
aSBWYWxrZWluZW4gPHRvbWkudmFsa2VpbmVuQHRpLmNvbT46Cj4gT24gMTMvMDMvMTQgMDk6MzEs
IEFsZXhhbmRlciBTaGl5YW4gd3JvdGU6Cj4gPiBQcm92aWRlIGR1bW15IGZ1bmN0aW9ucyBmb3Ig
TENEIHJlZ2lzdGVyKCkvdW5yZWdpc3RlcigpIGlmCj4gPiBDT05GSUdfTENEX0NMQVNTX0RFVklD
RSBpcyBub3Qgc2V0Lgo+IAo+IEhtbSwgd2h5IGRvIHlvdSB3YW50IHRvIGRvIHRoYXQ/IElmIGEg
ZHJpdmVyIG5lZWRzIHRoZSBMQ0QgY2xhc3MsIGl0Cj4gc2hvdWxkIGRlcGVuZCBvbiBvciBzZWxl
Y3QgaXQuCgpJIGluc3BlY3QgbXkgcmVjZW50IGNoYW5nZXMgZm9yIHRoZSBpbXhmYiBkcml2ZXIu
CkkgdXNlIHRoZSBMQ0QgY2xhc3MgZm9yIHBvd2VyIG1hbmFnZW1lbnQgYW5kIGNvbnRyYXN0LCBu
ZXZlcnRoZWxlc3MsCmlmIGl0IGxhY2sgaW4gdGhlIGtlcm5lbCB0aGlzIGxlYWRzIHRvIGFuIGVy
cm9yLgpZZXMsIHdlIGNhbiBjaG9vc2UgdGhlIExDRF9DTEFTU19ERVZJQ0Ugc3ltYm9sIGZvciB0
aGUgaW14ZmIgZHJpdmVyLApidXQgYXQgdGhlIHNhbWUgdGltZSB3ZSBtdXN0IGNob29zZSBCQUNL
TElHSFRfTENEX1NVUFBPUlQuCkkgZG8gbm90IHRoaW5rIGl0J3MgYSBnb29kIHdheS4KSW4gYW55
IGNhc2UsIEkgd291bGQgbGlrZSB0byByZXZpc2UgdGhlIHBhdGNoIHRvIHVzZSBOVUxMIGFzIGEg
cmVzdWx0CmlmIHRoZXJlIGlzIG5vIHN1cHBvcnQgTENEX0NMQVNTX0RFVklDRSBpbiB0aGUga2Vy
bmVsLgpBZGRpdGlvbmFsbHksIEkgaGF2ZSBhIHBsYW4gdG8gY29udmVydCAibWVudWNvbmZpZyIg
ZW50cnkgZm9yCkJBQ0tMSUdIVF9MQ0RfU1VQUE9SVCB0byB0aGUgIm1lbnUiLgpPSz8KCi0tLQo

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox