Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* Re: [PATCH v12 0/6] of: add display helper
From: Thierry Reding @ 2012-11-20 19:35 UTC (permalink / raw)
  To: Robert Schwebel
  Cc: Laurent Pinchart, Steffen Trumtrar, devicetree-discuss,
	Rob Herring, linux-fbdev, dri-devel, Guennady Liakhovetski,
	linux-media, Tomi Valkeinen, Stephen Warren, kernel,
	Florian Tobias Schandinat, David Airlie
In-Reply-To: <20121120181129.GI23204@pengutronix.de>

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

On Tue, Nov 20, 2012 at 07:11:29PM +0100, Robert Schwebel wrote:
> On Tue, Nov 20, 2012 at 05:13:19PM +0100, Laurent Pinchart wrote:
> > On Tuesday 20 November 2012 16:54:50 Steffen Trumtrar wrote:
> > > Hi!
> > > 
> > > Changes since v11:
> > > 	- make pointers const where applicable
> > > 	- add reviewed-by Laurent Pinchart
> > 
> > Looks good to me.
> > 
> > Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > 
> > Through which tree do you plan to push this ?
> 
> We have no idea yet, and none of the people on Cc: have volunteered
> so far... what do you think?

The customary approach would be to take the patches through separate
trees, but I think this particular series is sufficiently interwoven to
warrant taking them all through one tree. However the least that we
should do is collect Acked-by's from the other tree maintainers.

Given that most of the patches modify files in drivers/video, Florian's
fbdev tree would be the most obvious candidate, right? If Florian agrees
to take the patches, all we would need is David's Acked-by.

How does that sound?

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCHv7] video: Add support for the Solomon SSD1307 OLED Controller
From: Andrew Morton @ 2012-11-20 19:43 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1353421237-4100-1-git-send-email-maxime.ripard@free-electrons.com>

On Tue, 20 Nov 2012 15:20:37 +0100
Maxime Ripard <maxime.ripard@free-electrons.com> wrote:

> This patch adds support for the Solomon SSD1307 OLED
> controller found on the Crystalfontz CFA10036 board.
> 
> This controller can drive a display with a resolution up
> to 128x39 and can operate over I2C or SPI.
> 
> The current driver has only been tested on the CFA-10036,
> that is using this controller over I2C to driver a 96x16
> OLED screen.

Looks OK to my little eye.  I merged it into -mm.  If this
patch (or a variant of it) turns up in linux-next (via Florian's
tree) then I shall autodrop it again.

^ permalink raw reply

* Re: [PATCHv9 1/3] Runtime Interpreted Power Sequences
From: Grant Likely @ 2012-11-20 21:54 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1353149747-31871-2-git-send-email-acourbot@nvidia.com>

On Sat, 17 Nov 2012 19:55:45 +0900, Alexandre Courbot <acourbot@nvidia.com> wrote:
> Some device drivers (e.g. panel or backlights) need to follow precise
> sequences for powering on and off, involving GPIOs, regulators, PWMs
> and other power-related resources, with a defined powering order and
> delays to respect between steps. These sequences are device-specific,
> and do not belong to a particular driver - therefore they have been
> performed by board-specific hook functions so far.

I must be honest, this series really makes me nervous...

> With the advent of the device tree and of ARM kernels that are not
> board-tied, we cannot rely on these board-specific hooks anymore but

This isn't strictly true. It is still perfectly fine to have board
specific code when necessary. However, there is strong encouragement to
enable that code in device drivers as much as possible and new board
files need to have very strong justification.

> need a way to implement these sequences in a portable manner. This patch
> introduces a simple interpreter that can execute such power sequences
> encoded either as platform data or within the device tree. It also
> introduces first support for regulator, GPIO and PWM resources.

This is where I start getting nervous. Up to now I've strongly resisted
adding any kind of interpreted code to the device tree. The model is to
identify hardware, but require the driver to know how to control it. (as
compared to ACPI which is entirely designed around executable
bytecode).

While the power sequences described here certainly cannot be confused
with a Turing complete bytecode, it is a step in that direction. Plus,
there will always be that new use case that needs just a "little new
feature" to make this work for that too. Without thinking about how to
handle that ahead of time is just asking for something to turn into a
maintenance nightmare. It's just as important to specify what the limits
of this approach are and when to punt to real driver code to handle a
device.

> +Power Sequences Steps
> +---------------------
> +Steps of a sequence describe an action to be performed on a resource. They
> +always include a "type" property which indicates what kind of resource this
> +step works on. Depending on the resource type, additional properties are defined
> +to control the action to be performed.
> +
> +"delay" type required properties:
> +  - delay: delay to wait (in microseconds)
> +
> +"regulator" type required properties:
> +  - id: name of the regulator to use.
> +  - enable / disable: one of these two empty properties must be present to
> +                      enable or disable the resource
> +
> +"pwm" type required properties:
> +  - id: name of the PWM to use.
> +  - enable / disable: one of these two empty properties must be present to
> +                      enable or disable the resource
> +
> +"gpio" type required properties:
> +  - gpio: phandle of the GPIO to use.
> +  - value: value this GPIO should take. Must be 0 or 1.
> +
> +Example
> +-------
> +Here are example sequences declared within a backlight device that use all the
> +supported resources types:
> +
> +	backlight {
> +		compatible = "pwm-backlight";
> +		...
> +
> +		/* resources used by the power sequences */
> +		pwms = <&pwm 2 5000000>;
> +		pwm-names = "backlight";
> +		power-supply = <&backlight_reg>;
> +
> +		power-sequences {
> +			power-on {
> +				step0 {
> +					type = "regulator";
> +					id = "power";
> +					enable;
> +				};
> +				step1 {
> +					type = "delay";
> +					delay = <10000>;
> +				};
> +				step2 {
> +					type = "pwm";
> +					id = "backlight";
> +					enable;
> +				};
> +				step3 {
> +					type = "gpio";
> +					gpio = <&gpio 28 0>;
> +					value = <1>;
> +				};
> +			};
> +
> +			power-off {
> +				step0 {
> +					type = "gpio";
> +					gpio = <&gpio 28 0>;
> +					value = <0>;
> +				};
> +				step1 {
> +					type = "pwm";
> +					id = "backlight";
> +					disable;
> +				};
> +				step2 {
> +					type = "delay";
> +					delay = <10000>;
> +				};
> +				step3 {
> +					type = "regulator";
> +					id = "power";
> +					disable;
> +				};
> +			};
> +		};
> +	};

I think this will get very verbose in a hurry. Already this simple
example is 45 lines long. Using the device tree structure to encode the
language doesn't look like a very good fit. Not to mention that the
order of operations is entirely based on the node name. Want to insert
an operation between step0 and step1? Need to rename step1, step2, and
step3 to do so.

This implementation also isn't very consistent. The gpio is referenced
with a phandle in step3/step0, but the regulator and pwm are referenced
by id.

As an alternative, what about something like the following?

	backlight {
		compatible = "pwm-backlight";
		...

		/* resources used by the power sequences */
		pwms = <&pwm 2 5000000>;
		pwm-names = "backlight";
		regulators = <&backlight_reg>;
		gpios = <&gpio 28 0>;

		power-on-sequence = "r0e;d10000m;p0e;g0s";
		power-off-sequence = "g0c;p0d;d10000m;r0d";
	};

I'm thinking about the scripts as trivial-to-parse ascii strings that
have a very simple set of commands. The commands use resources already
defined in the node. ie. 'g0' refers to the first entry in the gpios
property. 'r0' for the regulator, 'p0' for the pwms, 'd' means delay. By
no means take this as the format to use, it is just an example off the
top of my head, but it is already way easier to work with than putting
each command into a node.

The trick is still to define a syntax that doesn't fall apart when it
needs to be extended. I would also like to get opinions on whether or
not conditionals or loops should be supported (ie. loop waiting for a
status to change). If they should then we need to be a lot more careful
about the design (and due to my aforementioned nervousness, somebody may
need to get me therapy).

(Mitch, I'll let you make the argument for using Forth here. To be
honest, I'm not keen on defining any kind of new language, however
simple, but neither am I keen to pull in Forth).

> +Platform Data Format
> +--------------------
> +All relevant data structures for declaring power sequences are located in
> +include/linux/power_seq.h.

Hmm... If sequences are switched to a string instead, then platform_data
should also use it. The power sequence data structures can be created at
runtime by parsing the string.

g.

^ permalink raw reply

* Re: [PATCHv9 0/3] Runtime Interpreted Power Sequences
From: Grant Likely @ 2012-11-20 21:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1353149747-31871-1-git-send-email-acourbot@nvidia.com>

On Sat, 17 Nov 2012 19:55:44 +0900, Alexandre Courbot <acourbot@nvidia.com> wrote:
> Apologies for sending two patchsets in two days - the main purpose
> of this new revision is to add the linux-arm-kernel list to the
> audience. A few suggestions from v8 have also been added.

Just to be clear from my previous reply, don't merge this series. The DT
binding is not good.

g.

^ permalink raw reply

* Re: [PATCH 0/5] OMAPFB: use dma_alloc instead of omap's vram
From: Tony Lindgren @ 2012-11-20 22:14 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <50A5E801.3000100@ti.com>

* Tomi Valkeinen <tomi.valkeinen@ti.com> [121115 23:17]:
> 
> I added your acks, and pushed:
> 
> git://gitorious.org/linux-omap-dss2/linux.git 3.8/vram-conversion
> 
> It's based on -rc4 as my other branches are based on that.

OK thanks!

Tony

^ permalink raw reply

* efifb swapping Red/Blue color
From: Bruno Prémont @ 2012-11-20 23:14 UTC (permalink / raw)
  To: linux-fbdev

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="windows-1254", Size: 749 bytes --]

On a MacBookAir2,1 with 9400M C79 nVidia GPU fbcon on efifb has
red and blue colors swapped.

Extract from kernel log:
efifb: probing for efifb
efifb: framebuffer at 0x80010000, mapped to 0xffffc90004580000, using 6400k, total 6400k
efifb: mode is 1280x800x32, linelength92, pages=1
efifb: scrolling: redraw
efifb: Truecolor: size=8:8:8:8, shift$:0:8:16

On another system with radeon GPU and externally connected DVI monitor
colors are set properly.

Probably efifb should be looking at something to determine if display
is RGB or BGR...

Thanks,
Bruno
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCHv9 1/3] Runtime Interpreted Power Sequences
From: Mark Brown @ 2012-11-21  1:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20121120215429.B621F3E1821@localhost>

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

On Tue, Nov 20, 2012 at 09:54:29PM +0000, Grant Likely wrote:
> On Sat, 17 Nov 2012 19:55:45 +0900, Alexandre Courbot <acourbot@nvidia.com> wrote:

> > With the advent of the device tree and of ARM kernels that are not
> > board-tied, we cannot rely on these board-specific hooks anymore but

> This isn't strictly true. It is still perfectly fine to have board
> specific code when necessary. However, there is strong encouragement to
> enable that code in device drivers as much as possible and new board
> files need to have very strong justification.

This isn't the message that's gone over, and even for device drivers
everyone seems to be taking the whole device tree thing as a move to
pull all data out of the kernel.  In some cases there are some real
practical advantages to doing this but a lot of the people making these
changes seem to view having things in DT as a goal in itself.

> I'm thinking about the scripts as trivial-to-parse ascii strings that
> have a very simple set of commands. The commands use resources already
> defined in the node. ie. 'g0' refers to the first entry in the gpios
> property. 'r0' for the regulator, 'p0' for the pwms, 'd' means delay. By
> no means take this as the format to use, it is just an example off the
> top of my head, but it is already way easier to work with than putting
> each command into a node.

It does appear to have some legibility challenges, especially with using
the indexes into the arrays of resources.  On the other hand the arrays
should be fairly small.

> > +Platform Data Format
> > +--------------------
> > +All relevant data structures for declaring power sequences are located in
> > +include/linux/power_seq.h.

> Hmm... If sequences are switched to a string instead, then platform_data
> should also use it. The power sequence data structures can be created at
> runtime by parsing the string.

Seems like a step backwards to me - why not let people store the more
parsed version of the data?  It's going to be less convenient for users
on non-DT systems.

As I said in my earlier reviews I think this is a useful thing to have
as a utility library for drivers independantly of the DT bindings, it
would allow drivers to become more data driven.  Perhaps we can rework
the series so that the DT bindings are added as a final patch?  All the
rest of the code seems OK.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCHv9 1/3] Runtime Interpreted Power Sequences
From: Alex Courbot @ 2012-11-21  1:56 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <50AB9832.90709@ti.com>

Hi Tomi,

On Tuesday 20 November 2012 22:48:18 Tomi Valkeinen wrote:
> I guess there's a reason, but the above looks a bit inconsistent. For
> gpio you define the gpio resource inside the step. For power and pwm the
> resource is defined before the steps. Why wouldn't "pwm = <&pwm 2
> 5000000>;" work in step2?

That's mostly a framework issue. Most frameworks do not export a function that 
allow to dereference a phandle - they expect resources to be declared right 
under the device node and accessed by name through foo_get(device, name). So 
using phandles in power sequences would require to export these additional 
functions and also opens the door to some inconsistencies - for instance, your 
PWM phandle could be referenced a second time in the sequence with a different 
period - how do you know that these are actually referring the same PWM 
device?

> > +When a power sequence is run, its steps is executed one after the other
> > until +one step fails or the end of the sequence is reached.
> 
> The document doesn't give any hint of what the driver should do if
> running the power sequence fails. Run the "opposite" power sequence?
> Will that work for all resources? I'm mainly thinking of a case where
> each enable of the resource should be matched by a disable, i.e. you
> can't call disable if no enable was called.

We discussed that issue already (around v5 I think) and the conclusion was 
that it should be up to the driver. When we simply enable/disable resources it 
is easy to revert, but in the future non-boolean properties will likely be 
introduced, and these cannot easily be reverted. Moreover some drivers might 
have more complex recovery needs. This deserves more discussion I think, as 
I'd like to have some "generic" recovery mechanism that covers most of the 
cases.

Alex.


^ permalink raw reply

* Re: [PATCHv9 1/3] Runtime Interpreted Power Sequences
From: Alex Courbot @ 2012-11-21  4:23 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20121120215429.B621F3E1821@localhost>

Hi Grant,

On Wednesday 21 November 2012 05:54:29 Grant Likely wrote:
> > With the advent of the device tree and of ARM kernels that are not
> > board-tied, we cannot rely on these board-specific hooks anymore but
> 
> This isn't strictly true. It is still perfectly fine to have board
> specific code when necessary. However, there is strong encouragement to
> enable that code in device drivers as much as possible and new board
> files need to have very strong justification.

But doesn't introducing board-specific code into the kernel just defeats the 
purpose of the DT? If we extend this logic, we are heading straight back to 
board-definition files. To a lesser extent than before I agree, but the problem 
is fundamentally the same.

> > need a way to implement these sequences in a portable manner. This patch
> > introduces a simple interpreter that can execute such power sequences
> > encoded either as platform data or within the device tree. It also
> > introduces first support for regulator, GPIO and PWM resources.
> 
> This is where I start getting nervous. Up to now I've strongly resisted
> adding any kind of interpreted code to the device tree. The model is to
> identify hardware, but require the driver to know how to control it. (as
> compared to ACPI which is entirely designed around executable
> bytecode).
> 
> While the power sequences described here certainly cannot be confused
> with a Turing complete bytecode, it is a step in that direction.

Technically speaking power sequences are a step towards an interpreter, but it 
is a very small one and it should not go much further than the current state. 
I understand the concern of having "code" into the DT but I really think it 
should be viewed from a different angle.

Powering sequences are special in that they can be affected by the board design 
or the devices variations. For instance hundreds of different panels with 
backlights are currently compatible with the pwm-backlight driver. The only 
thing that differenciates them is how the backlight is powered on and off. If 
you are to build a kernel that is supposed to support all these panels, you 
would need to embed all the powering sequences in the kernel even though only 
one of them will be used by one specific board. Power sequences in the DT help 
preventing that.

With that stated, it is clear that we should not need to define more than the 
short, simple sequences of actions that cannot be elegantly handled by the 
driver. Anything beyond that should be handled by the driver itself. In 
particular, here are a few things I do *not* want to see included in power 
seqs:

- conditionals/jumps (or it's not a sequence anymore).
- direct access to hardware. Resources must at least be abstracted in some 
way. You shall not e.g. access the address space directly.
- support for non-power related resources - that is out of the special case of 
powering sequences and should be done by the driver

That should keep the "grammar" simple, and the sequences short enough to that 
we can consider then as data belonging to the device, and not as code that is 
interpreted.

> I think this will get very verbose in a hurry. Already this simple
> example is 45 lines long. Using the device tree structure to encode the
> language doesn't look like a very good fit. Not to mention that the
> order of operations is entirely based on the node name. Want to insert
> an operation between step0 and step1? Need to rename step1, step2, and
> step3 to do so.

I don't like that steps numbering thing neither, but it seems to be the best 
way to do it so far.

As for the DT structure not being adapted for this - I would agree if we 
wanted to implement a complete interpreter, but that's precisely not the case. 
More about this later.

> This implementation also isn't very consistent. The gpio is referenced
> with a phandle in step3/step0, but the regulator and pwm are referenced
> by id.

Tomi made the same remark - the reason for using the phandle in GPIO is 
because GPIO framework does not support referencing GPIOs by name yet. I 
wanted to DT bindings to reflect the underlying framework as much as possible 
until we have a function like gpio_get(device, id).

However I agree that this makes things inconsistent at the moment and would 
require a bindings change. And in the case of the DT this is actually easy to 
implement (I did it in some previous versions). I'll make sure to do it.

> As an alternative, what about something like the following?
> 
> 	backlight {
> 		compatible = "pwm-backlight";
> 		...
> 
> 		/* resources used by the power sequences */
> 		pwms = <&pwm 2 5000000>;
> 		pwm-names = "backlight";
> 		regulators = <&backlight_reg>;
> 		gpios = <&gpio 28 0>;
> 
> 		power-on-sequence = "r0e;d10000m;p0e;g0s";
> 		power-off-sequence = "g0c;p0d;d10000m;r0d";
> 	};

Well, *now* it really looks like bytecode. :)

> I'm thinking about the scripts as trivial-to-parse ascii strings that
> have a very simple set of commands. The commands use resources already
> defined in the node. ie. 'g0' refers to the first entry in the gpios
> property. 'r0' for the regulator, 'p0' for the pwms, 'd' means delay. By
> no means take this as the format to use, it is just an example off the
> top of my head, but it is already way easier to work with than putting
> each command into a node.

I'd argue that this opens the door wide open towards having a complete 
interpreter in the device tree. At least DT nodes restrict what we can do 
conveniently.

> The trick is still to define a syntax that doesn't fall apart when it
> needs to be extended. I would also like to get opinions on whether or
> not conditionals or loops should be supported (ie. loop waiting for a
> status to change). If they should then we need to be a lot more careful
> about the design (and due to my aforementioned nervousness, somebody may
> need to get me therapy).

That's IMHO the main argument in favor of using DT nodes here (the syntax 
extension, not your therapy). They can be extended simply by adding properties 
to them, and I was relying on this to make power seqs extensible while keeping 
things consistent (and backward-compatible). For instance, when we want to 
support voltage setting in regulators we can just add a "voltage" property for 
that.

So to sum up the pros of the current node-base representation:
- give a "data like" representation of powering sequences (what they should 
be, actually)
- puts sanity barriers for not turning power seqs into a real code interpreter
- easily extensible

There are probably a few cons, the numbering of steps being one, but at least 
we don't need to design a new DSL and care too much about extensibility which 
is, in the nodes case, built-in and free.

I also like to make it clear that I don't want to see this going out of 
control and that any extension proposal would have to be thoroughly justified 
and reviewed - and better not contradict any of the 3 points listed above.

If that makes you feel better, maybe we can try and fix what is wrong with the 
current bindings instead of introducing a new language that will be, by 
nature, more complex to handle and difficult to extend without breaking things?

Alex.


^ permalink raw reply

* Re: [PATCH v12 0/6] of: add display helper
From: Steffen Trumtrar @ 2012-11-21  7:41 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA, Florian Tobias Schandinat,
	David Airlie, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Tomi Valkeinen,
	kernel-bIcnvbaLZ9MEGnE8C9+IrQ, Guennady Liakhovetski,
	linux-media-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1501232.SOApmW1MhU@avalon>

On Tue, Nov 20, 2012 at 05:13:19PM +0100, Laurent Pinchart wrote:
> On Tuesday 20 November 2012 16:54:50 Steffen Trumtrar wrote:
> > Hi!
> > 
> > Changes since v11:
> > 	- make pointers const where applicable
> > 	- add reviewed-by Laurent Pinchart
> 
> Looks good to me.
> 
> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 

Thanks.

Regards,
Steffen

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply

* Re: [PATCHv9 1/3] Runtime Interpreted Power Sequences
From: Tomi Valkeinen @ 2012-11-21  8:13 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <4316169.5QXVzv7peZ@percival>

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

On 2012-11-21 03:56, Alex Courbot wrote:
> Hi Tomi,
> 
> On Tuesday 20 November 2012 22:48:18 Tomi Valkeinen wrote:
>> I guess there's a reason, but the above looks a bit inconsistent. For
>> gpio you define the gpio resource inside the step. For power and pwm the
>> resource is defined before the steps. Why wouldn't "pwm = <&pwm 2
>> 5000000>;" work in step2?
> 
> That's mostly a framework issue. Most frameworks do not export a function that 
> allow to dereference a phandle - they expect resources to be declared right 
> under the device node and accessed by name through foo_get(device, name). So 
> using phandles in power sequences would require to export these additional 

Right, I expected something like that.

> functions and also opens the door to some inconsistencies - for instance, your 
> PWM phandle could be referenced a second time in the sequence with a different 
> period - how do you know that these are actually referring the same PWM 
> device?

This I didn't understand. Doesn't "<&pwm 2 xyz>" point to a single
device, no matter where and how many times it's used?

>>> +When a power sequence is run, its steps is executed one after the other
>>> until +one step fails or the end of the sequence is reached.
>>
>> The document doesn't give any hint of what the driver should do if
>> running the power sequence fails. Run the "opposite" power sequence?
>> Will that work for all resources? I'm mainly thinking of a case where
>> each enable of the resource should be matched by a disable, i.e. you
>> can't call disable if no enable was called.
> 
> We discussed that issue already (around v5 I think) and the conclusion was 
> that it should be up to the driver. When we simply enable/disable resources it 
> is easy to revert, but in the future non-boolean properties will likely be 
> introduced, and these cannot easily be reverted. Moreover some drivers might 
> have more complex recovery needs. This deserves more discussion I think, as 
> I'd like to have some "generic" recovery mechanism that covers most of the 
> cases.

Ok. I'll need to dig up the conversation. Did you consider any examples
of how some driver could handle the error cases?

What I'm worried about is that, as far as I understand, the power
sequence is kinda like black box to the driver. The driver just does
"power-up", without knowing what really goes on in there.

And if it doesn't know what goes on in there, nor what's in "power-down"
sequence, how can it do anything when an error happens? The only option
I see is that the driver doesn't do anything, which will leave some
resources enabled, or it can run the power-down sequence, which may or
may not work.

 Tomi



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

^ permalink raw reply

* Re: [PATCH v12 0/6] of: add display helper
From: Steffen Trumtrar @ 2012-11-21  8:28 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Robert Schwebel, Laurent Pinchart, devicetree-discuss,
	Rob Herring, linux-fbdev, dri-devel, Guennady Liakhovetski,
	linux-media, Tomi Valkeinen, Stephen Warren, kernel,
	Florian Tobias Schandinat, David Airlie
In-Reply-To: <20121120193531.GB27186@avionic-0098.adnet.avionic-design.de>

On Tue, Nov 20, 2012 at 08:35:31PM +0100, Thierry Reding wrote:
> On Tue, Nov 20, 2012 at 07:11:29PM +0100, Robert Schwebel wrote:
> > On Tue, Nov 20, 2012 at 05:13:19PM +0100, Laurent Pinchart wrote:
> > > On Tuesday 20 November 2012 16:54:50 Steffen Trumtrar wrote:
> > > > Hi!
> > > > 
> > > > Changes since v11:
> > > > 	- make pointers const where applicable
> > > > 	- add reviewed-by Laurent Pinchart
> > > 
> > > Looks good to me.
> > > 
> > > Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > 
> > > Through which tree do you plan to push this ?
> > 
> > We have no idea yet, and none of the people on Cc: have volunteered
> > so far... what do you think?
> 
> The customary approach would be to take the patches through separate
> trees, but I think this particular series is sufficiently interwoven to
> warrant taking them all through one tree. However the least that we
> should do is collect Acked-by's from the other tree maintainers.
> 
> Given that most of the patches modify files in drivers/video, Florian's
> fbdev tree would be the most obvious candidate, right? If Florian agrees
> to take the patches, all we would need is David's Acked-by.
> 
> How does that sound?
> 
> Thierry

Hi!

That is exactly the way I thought this could happen.

Regards
Steffen

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply

* Re: [PATCHv9 1/3] Runtime Interpreted Power Sequences
From: Alex Courbot @ 2012-11-21  8:32 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <50AC8D3B.6040300@ti.com>

On Wednesday 21 November 2012 16:13:47 Tomi Valkeinen wrote:
> * PGP Signed by an unknown key
> 
> On 2012-11-21 03:56, Alex Courbot wrote:
> > Hi Tomi,
> > 
> > On Tuesday 20 November 2012 22:48:18 Tomi Valkeinen wrote:
> >> I guess there's a reason, but the above looks a bit inconsistent. For
> >> gpio you define the gpio resource inside the step. For power and pwm the
> >> resource is defined before the steps. Why wouldn't "pwm = <&pwm 2
> >> 5000000>;" work in step2?
> > 
> > That's mostly a framework issue. Most frameworks do not export a function
> > that allow to dereference a phandle - they expect resources to be
> > declared right under the device node and accessed by name through
> > foo_get(device, name). So using phandles in power sequences would require
> > to export these additional
> Right, I expected something like that.
> 
> > functions and also opens the door to some inconsistencies - for instance,
> > your PWM phandle could be referenced a second time in the sequence with a
> > different period - how do you know that these are actually referring the
> > same PWM device?
> 
> This I didn't understand. Doesn't "<&pwm 2 xyz>" point to a single
> device, no matter where and how many times it's used?
> 
> >>> +When a power sequence is run, its steps is executed one after the other
> >>> until +one step fails or the end of the sequence is reached.
> >> 
> >> The document doesn't give any hint of what the driver should do if
> >> running the power sequence fails. Run the "opposite" power sequence?
> >> Will that work for all resources? I'm mainly thinking of a case where
> >> each enable of the resource should be matched by a disable, i.e. you
> >> can't call disable if no enable was called.
> > 
> > We discussed that issue already (around v5 I think) and the conclusion was
> > that it should be up to the driver. When we simply enable/disable
> > resources it is easy to revert, but in the future non-boolean properties
> > will likely be introduced, and these cannot easily be reverted. Moreover
> > some drivers might have more complex recovery needs. This deserves more
> > discussion I think, as I'd like to have some "generic" recovery mechanism
> > that covers most of the cases.
> 
> Ok. I'll need to dig up the conversation

IIRC it was somewhere around here:

https://lkml.org/lkml/2012/9/7/662

See the parent messages too.

> Did you consider any examples
> of how some driver could handle the error cases?

For all the (limited) use cases I considered, playing the power-off sequence 
when power-on fails just works. If power-off also fails you are potentially in 
more trouble though. Maybe we could have another "run" function that does not 
stop on errors for handling such cases where you want to "stop everything you 
can".

> What I'm worried about is that, as far as I understand, the power
> sequence is kinda like black box to the driver. The driver just does
> "power-up", without knowing what really goes on in there.

The driver could always inspect the sequence, but you are right in that this 
is not how it is intended to be done.

> And if it doesn't know what goes on in there, nor what's in "power-down"
> sequence, how can it do anything when an error happens? The only option
> I see is that the driver doesn't do anything, which will leave some
> resources enabled, or it can run the power-down sequence, which may or
> may not work.

Failures might be better handled if sequences have some "recovery policy" 
about what to do when they fail, as mentioned in the link above. As you 
pointed out, the driver might not always know enough about the resources 
involved to do the right thing.

Alex.


^ permalink raw reply

* Re: [PATCHv9 1/3] Runtime Interpreted Power Sequences
From: Tomi Valkeinen @ 2012-11-21  8:48 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1699753.ZQsWMHINxd@percival>

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

On 2012-11-21 10:32, Alex Courbot wrote:

>> Ok. I'll need to dig up the conversation
> 
> IIRC it was somewhere around here:
> 
> https://lkml.org/lkml/2012/9/7/662
> 
> See the parent messages too.

Thanks.

>> Did you consider any examples
>> of how some driver could handle the error cases?
> 
> For all the (limited) use cases I considered, playing the power-off sequence 
> when power-on fails just works. If power-off also fails you are potentially in 
> more trouble though. Maybe we could have another "run" function that does not 
> stop on errors for handling such cases where you want to "stop everything you 
> can".

If the power-off sequence disables a regulator that was supposed to be
enabled by the power-on sequence (but wasn't enabled because of an
error), the regulator_disable is still called when the driver runs the
power-off sequence, isn't it? Regulator enables and disables are ref
counted, and the enables should match the disables.

> Failures might be better handled if sequences have some "recovery policy" 
> about what to do when they fail, as mentioned in the link above. As you 
> pointed out, the driver might not always know enough about the resources 
> involved to do the right thing.

Yes, I think such recovery policy would be needed.

 Tomi



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

^ permalink raw reply

* Re: [PATCH 2/2] ARM: dts: mxs: add oled support for the cfa-10036
From: Maxime Ripard @ 2012-11-21  8:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1351674774-2891-3-git-send-email-maxime.ripard@free-electrons.com>

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi Shawn,

Le 31/10/2012 10:12, Maxime Ripard a écrit :
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> Cc:
> Brian Lilly <brian@crystalfontz.com> --- 
> arch/arm/boot/dts/imx28-cfa10036.dts |   19 +++++++++++++++++++ 1
> file changed, 19 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/imx28-cfa10036.dts
> b/arch/arm/boot/dts/imx28-cfa10036.dts index c03a577..816cae9
> 100644 --- a/arch/arm/boot/dts/imx28-cfa10036.dts +++
> b/arch/arm/boot/dts/imx28-cfa10036.dts @@ -33,11 +33,30 @@ };
> 
> apbx@80040000 { +			pwm: pwm@80064000 { +				pinctrl-names > "default"; +				pinctrl-0 = <&pwm4_pins_a>; +				status = "okay"; +
> }; + duart: serial@80074000 { pinctrl-names = "default"; pinctrl-0
> = <&duart_pins_b>; status = "okay"; }; + +			i2c0: i2c@80058000 { +
> pinctrl-names = "default"; +				pinctrl-0 = <&i2c0_pins_b>; +
> status = "okay"; + +				ssd1307: oled@3c { +					compatible > "solomon,ssd1307fb-i2c"; +					reg = <0x3c>; +					pwms = <&pwm 4
> 3000>; +					reset-gpios = <&gpio2 7 0>; +				}; +			}; }; };


Would you consider merging this patch?
Andrew Morton merged the driver into its tree
(http://marc.info/?l=linux-fbdev&m\x135336761530731&w=2)

Thanks,
Maxime


- -- 
Maxime Ripard, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://www.enigmail.net/

iEYEARECAAYFAlCsl7MACgkQGxsu9jQV9nZtJwCfR9c2SnwK1hzAU2F+xptFqg9x
2+0An3YrhNMXSZaPwPcJwc3Bnwy+FSqe
=mjpV
-----END PGP SIGNATURE-----

^ permalink raw reply

* Re: [PATCHv9 1/3] Runtime Interpreted Power Sequences
From: Alex Courbot @ 2012-11-21 10:00 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <50AC956D.7020303@ti.com>

On Wednesday 21 November 2012 16:48:45 Tomi Valkeinen wrote:
> If the power-off sequence disables a regulator that was supposed to be
> enabled by the power-on sequence (but wasn't enabled because of an
> error), the regulator_disable is still called when the driver runs the
> power-off sequence, isn't it? Regulator enables and disables are ref
> counted, and the enables should match the disables.

And there collapses my theory.

> > Failures might be better handled if sequences have some "recovery policy"
> > about what to do when they fail, as mentioned in the link above. As you
> > pointed out, the driver might not always know enough about the resources
> > involved to do the right thing.
> 
> Yes, I think such recovery policy would be needed.

Indeed, from your last paragraph this makes even more sense now.

Oh, and I noticed I forgot to reply to this:

> This I didn't understand. Doesn't "<&pwm 2 xyz>" point to a single
> device, no matter where and how many times it's used?

That's true - however when dereferencing the phandle, the underlying framework 
will try to acquire the PWM, which will result in failure if the same resource 
is referenced several times.

One could compare the phandles to avoid this, but in your example you must 
know that for PWMs the "xyz" part is not relevant for comparison.

This makes referencing of resources by name much easier to implement and more 
elegant with respect to frameworks leveraging.

Alex.


^ permalink raw reply

* RE: [PATCH v12 3/6] fbmon: add videomode helpers
From: Manjunathappa, Prakash @ 2012-11-21 10:09 UTC (permalink / raw)
  To: Steffen Trumtrar, devicetree-discuss@lists.ozlabs.org
  Cc: Rob Herring, linux-fbdev@vger.kernel.org,
	dri-devel@lists.freedesktop.org, Laurent Pinchart, Thierry Reding,
	Guennady Liakhovetski, linux-media@vger.kernel.org,
	Valkeinen, Tomi, Stephen Warren, kernel@pengutronix.de,
	Florian Tobias Schandinat, David Airlie
In-Reply-To: <1353426896-6045-4-git-send-email-s.trumtrar@pengutronix.de>

Hi Steffen,

I am trying to add DT support for da8xx-fb driver on top of your patches.
Encountered below build error. Sorry for reporting it late.

On Tue, Nov 20, 2012 at 21:24:53, Steffen Trumtrar wrote:
> Add a function to convert from the generic videomode to a fb_videomode.
> 
> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
> Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
> Tested-by: Thierry Reding <thierry.reding@avionic-design.de>
> Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  drivers/video/fbmon.c |   46 ++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/fb.h    |    6 ++++++
>  2 files changed, 52 insertions(+)
> 
> diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
> index cef6557..c1939a6 100644
> --- a/drivers/video/fbmon.c
> +++ b/drivers/video/fbmon.c
> @@ -31,6 +31,7 @@
>  #include <linux/pci.h>
>  #include <linux/slab.h>
>  #include <video/edid.h>
> +#include <linux/videomode.h>
>  #ifdef CONFIG_PPC_OF
>  #include <asm/prom.h>
>  #include <asm/pci-bridge.h>
> @@ -1373,6 +1374,51 @@ int fb_get_mode(int flags, u32 val, struct fb_var_screeninfo *var, struct fb_inf
>  	kfree(timings);
>  	return err;
>  }
> +
> +#if IS_ENABLED(CONFIG_VIDEOMODE)
> +int fb_videomode_from_videomode(const struct videomode *vm,
> +				struct fb_videomode *fbmode)
> +{
> +	unsigned int htotal, vtotal;
> +
> +	fbmode->xres = vm->hactive;
> +	fbmode->left_margin = vm->hback_porch;
> +	fbmode->right_margin = vm->hfront_porch;
> +	fbmode->hsync_len = vm->hsync_len;
> +
> +	fbmode->yres = vm->vactive;
> +	fbmode->upper_margin = vm->vback_porch;
> +	fbmode->lower_margin = vm->vfront_porch;
> +	fbmode->vsync_len = vm->vsync_len;
> +
> +	fbmode->pixclock = KHZ2PICOS(vm->pixelclock / 1000);
> +
> +	fbmode->sync = 0;
> +	fbmode->vmode = 0;
> +	if (vm->hah)
> +		fbmode->sync |= FB_SYNC_HOR_HIGH_ACT;
> +	if (vm->vah)
> +		fbmode->sync |= FB_SYNC_VERT_HIGH_ACT;
> +	if (vm->interlaced)
> +		fbmode->vmode |= FB_VMODE_INTERLACED;
> +	if (vm->doublescan)
> +		fbmode->vmode |= FB_VMODE_DOUBLE;
> +	if (vm->de)
> +		fbmode->sync |= FB_SYNC_DATA_ENABLE_HIGH_ACT;

"FB_SYNC_DATA_ENABLE_HIGH_ACT" seems to be mxsfb specific flag, I am getting
build error on this. Please let me know if I am missing something.

Thanks,
Prakash

> +	fbmode->flag = 0;
> +
> +	htotal = vm->hactive + vm->hfront_porch + vm->hback_porch +
> +		 vm->hsync_len;
> +	vtotal = vm->vactive + vm->vfront_porch + vm->vback_porch +
> +		 vm->vsync_len;
> +	fbmode->refresh = (vm->pixelclock * 1000) / (htotal * vtotal);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(fb_videomode_from_videomode);
> +#endif
> +
> +
>  #else
>  int fb_parse_edid(unsigned char *edid, struct fb_var_screeninfo *var)
>  {
> diff --git a/include/linux/fb.h b/include/linux/fb.h
> index c7a9571..920cbe3 100644
> --- a/include/linux/fb.h
> +++ b/include/linux/fb.h
> @@ -14,6 +14,7 @@
>  #include <linux/backlight.h>
>  #include <linux/slab.h>
>  #include <asm/io.h>
> +#include <linux/videomode.h>
>  
>  struct vm_area_struct;
>  struct fb_info;
> @@ -714,6 +715,11 @@ extern void fb_destroy_modedb(struct fb_videomode *modedb);
>  extern int fb_find_mode_cvt(struct fb_videomode *mode, int margins, int rb);
>  extern unsigned char *fb_ddc_read(struct i2c_adapter *adapter);
>  
> +#if IS_ENABLED(CONFIG_VIDEOMODE)
> +extern int fb_videomode_from_videomode(const struct videomode *vm,
> +				       struct fb_videomode *fbmode);
> +#endif
> +
>  /* drivers/video/modedb.c */
>  #define VESA_MODEDB_SIZE 34
>  extern void fb_var_to_videomode(struct fb_videomode *mode,
> -- 
> 1.7.10.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


^ permalink raw reply

* RE: [PATCH v12 2/6] video: add of helper for videomode
From: Manjunathappa, Prakash @ 2012-11-21 10:12 UTC (permalink / raw)
  To: Steffen Trumtrar,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
  Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org, David Airlie,
	Florian Tobias Schandinat,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org,
	Valkeinen, Tomi, Laurent Pinchart, Philipp Zabel,
	Guennady Liakhovetski,
	linux-media-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <1353426896-6045-3-git-send-email-s.trumtrar-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

SGkgU3RlZmZlbiwNCg0KT24gVHVlLCBOb3YgMjAsIDIwMTIgYXQgMjE6MjQ6NTIsIFN0ZWZmZW4g
VHJ1bXRyYXIgd3JvdGU6DQo+IFRoaXMgYWRkcyBzdXBwb3J0IGZvciByZWFkaW5nIGRpc3BsYXkg
dGltaW5ncyBmcm9tIERUIG9yL2FuZCBjb252ZXJ0IG9uZSBvZiB0aG9zZQ0KPiB0aW1pbmdzIHRv
IGEgdmlkZW9tb2RlLg0KPiBUaGUgb2ZfZGlzcGxheV90aW1pbmcgaW1wbGVtZW50YXRpb24gc3Vw
cG9ydHMgbXVsdGlwbGUgY2hpbGRyZW4gd2hlcmUgZWFjaA0KPiBwcm9wZXJ0eSBjYW4gaGF2ZSB1
cCB0byAzIHZhbHVlcy4gQWxsIGNoaWxkcmVuIGFyZSByZWFkIGludG8gYW4gYXJyYXksIHRoYXQN
Cj4gY2FuIGJlIHF1ZXJpZWQuDQo+IG9mX2dldF92aWRlb21vZGUgY29udmVydHMgZXhhY3RseSBv
bmUgb2YgdGhhdCB0aW1pbmdzIHRvIGEgc3RydWN0IHZpZGVvbW9kZS4NCj4gDQo+IFNpZ25lZC1v
ZmYtYnk6IFN0ZWZmZW4gVHJ1bXRyYXIgPHMudHJ1bXRyYXJAcGVuZ3V0cm9uaXguZGU+DQo+IFNp
Z25lZC1vZmYtYnk6IFBoaWxpcHAgWmFiZWwgPHAuemFiZWxAcGVuZ3V0cm9uaXguZGU+DQo+IEFj
a2VkLWJ5OiBTdGVwaGVuIFdhcnJlbiA8c3dhcnJlbkBudmlkaWEuY29tPg0KPiBSZXZpZXdlZC1i
eTogVGhpZXJyeSBSZWRpbmcgPHRoaWVycnkucmVkaW5nQGF2aW9uaWMtZGVzaWduLmRlPg0KPiBB
Y2tlZC1ieTogVGhpZXJyeSBSZWRpbmcgPHRoaWVycnkucmVkaW5nQGF2aW9uaWMtZGVzaWduLmRl
Pg0KPiBUZXN0ZWQtYnk6IFRoaWVycnkgUmVkaW5nIDx0aGllcnJ5LnJlZGluZ0BhdmlvbmljLWRl
c2lnbi5kZT4NCj4gVGVzdGVkLWJ5OiBQaGlsaXBwIFphYmVsIDxwLnphYmVsQHBlbmd1dHJvbml4
LmRlPg0KPiBSZXZpZXdlZC1ieTogTGF1cmVudCBQaW5jaGFydCA8bGF1cmVudC5waW5jaGFydEBp
ZGVhc29uYm9hcmQuY29tPg0KPiAtLS0NCj4gIC4uLi9kZXZpY2V0cmVlL2JpbmRpbmdzL3ZpZGVv
L2Rpc3BsYXktdGltaW5ncy50eHQgIHwgIDEwNyArKysrKysrKysrDQo+ICBkcml2ZXJzL3ZpZGVv
L0tjb25maWcgICAgICAgICAgICAgICAgICAgICAgICAgICAgICB8ICAgMTMgKysNCj4gIGRyaXZl
cnMvdmlkZW8vTWFrZWZpbGUgICAgICAgICAgICAgICAgICAgICAgICAgICAgIHwgICAgMiArDQo+
ICBkcml2ZXJzL3ZpZGVvL29mX2Rpc3BsYXlfdGltaW5nLmMgICAgICAgICAgICAgICAgICB8ICAy
MTYgKysrKysrKysrKysrKysrKysrKysNCj4gIGRyaXZlcnMvdmlkZW8vb2ZfdmlkZW9tb2RlLmMg
ICAgICAgICAgICAgICAgICAgICAgIHwgICA0OCArKysrKw0KPiAgaW5jbHVkZS9saW51eC9vZl9k
aXNwbGF5X3RpbWluZ3MuaCAgICAgICAgICAgICAgICAgfCAgIDIwICsrDQo+ICBpbmNsdWRlL2xp
bnV4L29mX3ZpZGVvbW9kZS5oICAgICAgICAgICAgICAgICAgICAgICB8ICAgMTggKysNCj4gIDcg
ZmlsZXMgY2hhbmdlZCwgNDI0IGluc2VydGlvbnMoKykNCj4gIGNyZWF0ZSBtb2RlIDEwMDY0NCBE
b2N1bWVudGF0aW9uL2RldmljZXRyZWUvYmluZGluZ3MvdmlkZW8vZGlzcGxheS10aW1pbmdzLnR4
dA0KPiAgY3JlYXRlIG1vZGUgMTAwNjQ0IGRyaXZlcnMvdmlkZW8vb2ZfZGlzcGxheV90aW1pbmcu
Yw0KPiAgY3JlYXRlIG1vZGUgMTAwNjQ0IGRyaXZlcnMvdmlkZW8vb2ZfdmlkZW9tb2RlLmMNCj4g
IGNyZWF0ZSBtb2RlIDEwMDY0NCBpbmNsdWRlL2xpbnV4L29mX2Rpc3BsYXlfdGltaW5ncy5oDQo+
ICBjcmVhdGUgbW9kZSAxMDA2NDQgaW5jbHVkZS9saW51eC9vZl92aWRlb21vZGUuaA0KPiANCj4g
ZGlmZiAtLWdpdCBhL0RvY3VtZW50YXRpb24vZGV2aWNldHJlZS9iaW5kaW5ncy92aWRlby9kaXNw
bGF5LXRpbWluZ3MudHh0IGIvRG9jdW1lbnRhdGlvbi9kZXZpY2V0cmVlL2JpbmRpbmdzL3ZpZGVv
L2Rpc3BsYXktdGltaW5ncy50eHQNCj4gbmV3IGZpbGUgbW9kZSAxMDA2NDQNCj4gaW5kZXggMDAw
MDAwMC4uYTA1Y2FkZQ0KPiAtLS0gL2Rldi9udWxsDQo+ICsrKyBiL0RvY3VtZW50YXRpb24vZGV2
aWNldHJlZS9iaW5kaW5ncy92aWRlby9kaXNwbGF5LXRpbWluZ3MudHh0DQo+IEBAIC0wLDAgKzEs
MTA3IEBADQo+ICtkaXNwbGF5LXRpbWluZ3MgYmluZGluZ3MNCj4gKz09PT09PT09PT09PT09PT09
PT09PT09PQ0KPiArDQo+ICtkaXNwbGF5LXRpbWluZ3Mgbm9kZQ0KPiArLS0tLS0tLS0tLS0tLS0t
LS0tLS0NCj4gKw0KPiArcmVxdWlyZWQgcHJvcGVydGllczoNCj4gKyAtIG5vbmUNCj4gKw0KPiAr
b3B0aW9uYWwgcHJvcGVydGllczoNCj4gKyAtIG5hdGl2ZS1tb2RlOiBUaGUgbmF0aXZlIG1vZGUg
Zm9yIHRoZSBkaXNwbGF5LCBpbiBjYXNlIG11bHRpcGxlIG1vZGVzIGFyZQ0KPiArCQlwcm92aWRl
ZC4gV2hlbiBvbWl0dGVkLCBhc3N1bWUgdGhlIGZpcnN0IG5vZGUgaXMgdGhlIG5hdGl2ZS4NCj4g
Kw0KPiArdGltaW5ncyBzdWJub2RlDQo+ICstLS0tLS0tLS0tLS0tLS0NCj4gKw0KPiArcmVxdWly
ZWQgcHJvcGVydGllczoNCj4gKyAtIGhhY3RpdmUsIHZhY3RpdmU6IERpc3BsYXkgcmVzb2x1dGlv
bg0KPiArIC0gaGZyb250LXBvcmNoLCBoYmFjay1wb3JjaCwgaHN5bmMtbGVuOiBIb3Jpem9udGFs
IERpc3BsYXkgdGltaW5nIHBhcmFtZXRlcnMNCj4gKyAgIGluIHBpeGVscw0KPiArICAgdmZyb250
LXBvcmNoLCB2YmFjay1wb3JjaCwgdnN5bmMtbGVuOiBWZXJ0aWNhbCBkaXNwbGF5IHRpbWluZyBw
YXJhbWV0ZXJzIGluDQo+ICsgICBsaW5lcw0KPiArIC0gY2xvY2stZnJlcXVlbmN5OiBkaXNwbGF5
IGNsb2NrIGluIEh6DQo+ICsNCj4gK29wdGlvbmFsIHByb3BlcnRpZXM6DQo+ICsgLSBoc3luYy1h
Y3RpdmU6IEhzeW5jIHB1bHNlIGlzIGFjdGl2ZSBsb3cvaGlnaC9pZ25vcmVkDQo+ICsgLSB2c3lu
Yy1hY3RpdmU6IFZzeW5jIHB1bHNlIGlzIGFjdGl2ZSBsb3cvaGlnaC9pZ25vcmVkDQo+ICsgLSBk
ZS1hY3RpdmU6IERhdGEtRW5hYmxlIHB1bHNlIGlzIGFjdGl2ZSBsb3cvaGlnaC9pZ25vcmVkDQo+
ICsgLSBwaXhlbGNsay1pbnZlcnRlZDogcGl4ZWxjbG9jayBpcyBpbnZlcnRlZC9ub24taW52ZXJ0
ZWQvaWdub3JlZA0KPiArIC0gaW50ZXJsYWNlZCAoYm9vbCkNCj4gKyAtIGRvdWJsZXNjYW4gKGJv
b2wpDQo+ICsNCj4gK0FsbCB0aGUgb3B0aW9uYWwgcHJvcGVydGllcyB0aGF0IGFyZSBub3QgYm9v
bCBmb2xsb3cgdGhlIGZvbGxvd2luZyBsb2dpYzoNCj4gKyAgICA8MT46IGhpZ2ggYWN0aXZlDQo+
ICsgICAgPDA+OiBsb3cgYWN0aXZlDQo+ICsgICAgb21pdHRlZDogbm90IHVzZWQgb24gaGFyZHdh
cmUNCj4gKw0KPiArVGhlcmUgYXJlIGRpZmZlcmVudCB3YXlzIG9mIGRlc2NyaWJpbmcgdGhlIGNh
cGFiaWxpdGllcyBvZiBhIGRpc3BsYXkuIFRoZSBkZXZpY2V0cmVlDQo+ICtyZXByZXNlbnRhdGlv
biBjb3JyZXNwb25kcyB0byB0aGUgb25lIGNvbW1vbmx5IGZvdW5kIGluIGRhdGFzaGVldHMgZm9y
IGRpc3BsYXlzLg0KPiArSWYgYSBkaXNwbGF5IHN1cHBvcnRzIG11bHRpcGxlIHNpZ25hbCB0aW1p
bmdzLCB0aGUgbmF0aXZlLW1vZGUgY2FuIGJlIHNwZWNpZmllZC4NCj4gKw0KPiArVGhlIHBhcmFt
ZXRlcnMgYXJlIGRlZmluZWQgYXMNCj4gKw0KPiArc3RydWN0IGRpc3BsYXlfdGltaW5nDQo+ICs9
PT09PT09PT09PT09PT09PT09PT0NCj4gKw0KPiArICArLS0tLS0tLS0tLSstLS0tLS0tLS0tLS0t
LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0rLS0tLS0tLS0tLSstLS0tLS0tKw0KPiAr
ICB8ICAgICAgICAgIHwgICAgICAgICAgICAgICAg4oaRICAgICAgICAgICAgICAgICAgICAgICAg
ICAgIHwgICAgICAgICAgfCAgICAgICB8DQo+ICsgIHwgICAgICAgICAgfCAgICAgICAgICAgICAg
ICB8dmJhY2tfcG9yY2ggICAgICAgICAgICAgICAgIHwgICAgICAgICAgfCAgICAgICB8DQo+ICsg
IHwgICAgICAgICAgfCAgICAgICAgICAgICAgICDihpMgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgfCAgICAgICAgICB8ICAgICAgIHwNCj4gKyAgKy0tLS0tLS0tLS0jIyMjIyMjIyMjIyMjIyMj
IyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIy0tLS0tLS0tLS0rLS0tLS0tLSsNCj4gKyAg
fCAgICAgICAgICAjICAgICAgICAgICAgICAgIOKGkSAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAjICAgICAgICAgIHwgICAgICAgfA0KPiArICB8ICAgICAgICAgICMgICAgICAgICAgICAgICAg
fCAgICAgICAgICAgICAgICAgICAgICAgICAgICAjICAgICAgICAgIHwgICAgICAgfA0KPiArICB8
ICBoYmFjayAgICMgICAgICAgICAgICAgICAgfCAgICAgICAgICAgICAgICAgICAgICAgICAgICAj
ICBoZnJvbnQgIHwgaHN5bmMgfA0KPiArICB8ICAgcG9yY2ggICMgICAgICAgICAgICAgICAgfCAg
ICAgICBoYWN0aXZlICAgICAgICAgICAgICAjICBwb3JjaCAgIHwgIGxlbiAgfA0KPiArICB8PC0t
LS0tLS0tPiM8LS0tLS0tLS0tLS0tLS0tKy0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLT4jPC0t
LS0tLS0tPnw8LS0tLS0+fA0KPiArICB8ICAgICAgICAgICMgICAgICAgICAgICAgICAgfCAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAjICAgICAgICAgIHwgICAgICAgfA0KPiArICB8ICAgICAg
ICAgICMgICAgICAgICAgICAgICAgfHZhY3RpdmUgICAgICAgICAgICAgICAgICAgICAjICAgICAg
ICAgIHwgICAgICAgfA0KPiArICB8ICAgICAgICAgICMgICAgICAgICAgICAgICAgfCAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAjICAgICAgICAgIHwgICAgICAgfA0KPiArICB8ICAgICAgICAg
ICMgICAgICAgICAgICAgICAg4oaTICAgICAgICAgICAgICAgICAgICAgICAgICAgICMgICAgICAg
ICAgfCAgICAgICB8DQo+ICsgICstLS0tLS0tLS0tIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMj
IyMjIyMjIyMjIyMjIyMjIyMjIyMtLS0tLS0tLS0tKy0tLS0tLS0rDQo+ICsgIHwgICAgICAgICAg
fCAgICAgICAgICAgICAgICDihpEgICAgICAgICAgICAgICAgICAgICAgICAgICAgfCAgICAgICAg
ICB8ICAgICAgIHwNCj4gKyAgfCAgICAgICAgICB8ICAgICAgICAgICAgICAgIHx2ZnJvbnRfcG9y
Y2ggICAgICAgICAgICAgICAgfCAgICAgICAgICB8ICAgICAgIHwNCj4gKyAgfCAgICAgICAgICB8
ICAgICAgICAgICAgICAgIOKGkyAgICAgICAgICAgICAgICAgICAgICAgICAgICB8ICAgICAgICAg
IHwgICAgICAgfA0KPiArICArLS0tLS0tLS0tLSstLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t
LS0tLS0tLS0tLS0tLS0tLS0rLS0tLS0tLS0tLSstLS0tLS0tKw0KPiArICB8ICAgICAgICAgIHwg
ICAgICAgICAgICAgICAg4oaRICAgICAgICAgICAgICAgICAgICAgICAgICAgIHwgICAgICAgICAg
fCAgICAgICB8DQo+ICsgIHwgICAgICAgICAgfCAgICAgICAgICAgICAgICB8dnN5bmNfbGVuICAg
ICAgICAgICAgICAgICAgIHwgICAgICAgICAgfCAgICAgICB8DQo+ICsgIHwgICAgICAgICAgfCAg
ICAgICAgICAgICAgICDihpMgICAgICAgICAgICAgICAgICAgICAgICAgICAgfCAgICAgICAgICB8
ICAgICAgIHwNCj4gKyAgKy0tLS0tLS0tLS0rLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t
LS0tLS0tLS0tLS0tLS0tKy0tLS0tLS0tLS0rLS0tLS0tLSsNCj4gKw0KPiArDQo+ICtFeGFtcGxl
Og0KPiArDQo+ICsJZGlzcGxheS10aW1pbmdzIHsNCj4gKwkJbmF0aXZlLW1vZGUgPSA8JnRpbWlu
ZzA+Ow0KPiArCQl0aW1pbmcwOiAxOTIwcDI0IHsNCj4gKwkJCS8qIDE5MjB4MTA4MHAyNCAqLw0K
PiArCQkJY2xvY2stZnJlcXVlbmN5ID0gPDUyMDAwMDAwPjsNCj4gKwkJCWhhY3RpdmUgPSA8MTky
MD47DQo+ICsJCQl2YWN0aXZlID0gPDEwODA+Ow0KPiArCQkJaGZyb250LXBvcmNoID0gPDI1PjsN
Cj4gKwkJCWhiYWNrLXBvcmNoID0gPDI1PjsNCj4gKwkJCWhzeW5jLWxlbiA9IDwyNT47DQo+ICsJ
CQl2YmFjay1wb3JjaCA9IDwyPjsNCj4gKwkJCXZmcm9udC1wb3JjaCA9IDwyPjsNCj4gKwkJCXZz
eW5jLWxlbiA9IDwyPjsNCj4gKwkJCWhzeW5jLWFjdGl2ZSA9IDwxPjsNCj4gKwkJfTsNCj4gKwl9
Ow0KPiArDQo+ICtFdmVyeSByZXF1aXJlZCBwcm9wZXJ0eSBhbHNvIHN1cHBvcnRzIHRoZSB1c2Ug
b2YgcmFuZ2VzLCBzbyB0aGUgY29tbW9ubHkgdXNlZA0KPiArZGF0YXNoZWV0IGRlc2NyaXB0aW9u
IHdpdGggPG1pbiB0eXAgbWF4Pi10dXBsZXMgY2FuIGJlIHVzZWQuDQo+ICsNCj4gK0V4YW1wbGU6
DQo+ICsNCj4gKwl0aW1pbmcxOiB0aW1pbmcgew0KPiArCQkvKiAxOTIweDEwODBwMjQgKi8NCj4g
KwkJY2xvY2stZnJlcXVlbmN5ID0gPDE0ODUwMDAwMD47DQo+ICsJCWhhY3RpdmUgPSA8MTkyMD47
DQo+ICsJCXZhY3RpdmUgPSA8MTA4MD47DQo+ICsJCWhzeW5jLWxlbiA9IDwwIDQ0IDYwPjsNCj4g
KwkJaGZyb250LXBvcmNoID0gPDgwIDg4IDk1PjsNCj4gKwkJaGJhY2stcG9yY2ggPSA8MTAwIDE0
OCAxNjA+Ow0KPiArCQl2ZnJvbnQtcG9yY2ggPSA8MCA0IDY+Ow0KPiArCQl2YmFjay1wb3JjaCA9
IDwwIDM2IDUwPjsNCj4gKwkJdnN5bmMtbGVuID0gPDAgNSA2PjsNCj4gKwl9Ow0KPiBkaWZmIC0t
Z2l0IGEvZHJpdmVycy92aWRlby9LY29uZmlnIGIvZHJpdmVycy92aWRlby9LY29uZmlnDQo+IGlu
ZGV4IDJhMjNiMTguLjgwN2ZlZGQgMTAwNjQ0DQo+IC0tLSBhL2RyaXZlcnMvdmlkZW8vS2NvbmZp
Zw0KPiArKysgYi9kcml2ZXJzL3ZpZGVvL0tjb25maWcNCj4gQEAgLTM5LDYgKzM5LDE5IEBAIGNv
bmZpZyBESVNQTEFZX1RJTUlORw0KPiAgY29uZmlnIFZJREVPTU9ERQ0KPiAgICAgICAgIGJvb2wN
Cj4gIA0KPiArY29uZmlnIE9GX0RJU1BMQVlfVElNSU5HDQo+ICsJYm9vbCAiRW5hYmxlIE9GIGRp
c3BsYXkgdGltaW5nIHN1cHBvcnQiDQo+ICsJc2VsZWN0IERJU1BMQVlfVElNSU5HDQo+ICsJaGVs
cA0KPiArCSAgaGVscGVyIHRvIHBhcnNlIGRpc3BsYXkgdGltaW5ncyBmcm9tIHRoZSBkZXZpY2V0
cmVlDQo+ICsNCj4gK2NvbmZpZyBPRl9WSURFT01PREUNCj4gKwlib29sICJFbmFibGUgT0Ygdmlk
ZW9tb2RlIHN1cHBvcnQiDQo+ICsJc2VsZWN0IFZJREVPTU9ERQ0KPiArCXNlbGVjdCBPRl9ESVNQ
TEFZX1RJTUlORw0KPiArCWhlbHANCj4gKwkgIGhlbHBlciB0byBnZXQgdmlkZW9tb2RlcyBmcm9t
IHRoZSBkZXZpY2V0cmVlDQo+ICsNCj4gIG1lbnVjb25maWcgRkINCj4gIAl0cmlzdGF0ZSAiU3Vw
cG9ydCBmb3IgZnJhbWUgYnVmZmVyIGRldmljZXMiDQo+ICAJLS0taGVscC0tLQ0KPiBkaWZmIC0t
Z2l0IGEvZHJpdmVycy92aWRlby9NYWtlZmlsZSBiL2RyaXZlcnMvdmlkZW8vTWFrZWZpbGUNCj4g
aW5kZXggZmMzMDQzOS4uYjkzNmIwMCAxMDA2NDQNCj4gLS0tIGEvZHJpdmVycy92aWRlby9NYWtl
ZmlsZQ0KPiArKysgYi9kcml2ZXJzL3ZpZGVvL01ha2VmaWxlDQo+IEBAIC0xNjgsNCArMTY4LDYg
QEAgb2JqLSQoQ09ORklHX0ZCX1ZJUlRVQUwpICAgICAgICAgICs9IHZmYi5vDQo+ICAjdmlkZW8g
b3V0cHV0IHN3aXRjaCBzeXNmcyBkcml2ZXINCj4gIG9iai0kKENPTkZJR19WSURFT19PVVRQVVRf
Q09OVFJPTCkgKz0gb3V0cHV0Lm8NCj4gIG9iai0kKENPTkZJR19ESVNQTEFZX1RJTUlORykgKz0g
ZGlzcGxheV90aW1pbmcubw0KPiArb2JqLSQoQ09ORklHX09GX0RJU1BMQVlfVElNSU5HKSArPSBv
Zl9kaXNwbGF5X3RpbWluZy5vDQo+ICBvYmotJChDT05GSUdfVklERU9NT0RFKSArPSB2aWRlb21v
ZGUubw0KPiArb2JqLSQoQ09ORklHX09GX1ZJREVPTU9ERSkgKz0gb2ZfdmlkZW9tb2RlLm8NCj4g
ZGlmZiAtLWdpdCBhL2RyaXZlcnMvdmlkZW8vb2ZfZGlzcGxheV90aW1pbmcuYyBiL2RyaXZlcnMv
dmlkZW8vb2ZfZGlzcGxheV90aW1pbmcuYw0KPiBuZXcgZmlsZSBtb2RlIDEwMDY0NA0KPiBpbmRl
eCAwMDAwMDAwLi4zZWI3MzFmDQo+IC0tLSAvZGV2L251bGwNCj4gKysrIGIvZHJpdmVycy92aWRl
by9vZl9kaXNwbGF5X3RpbWluZy5jDQo+IEBAIC0wLDAgKzEsMjE2IEBADQo+ICsvKg0KPiArICog
T0YgaGVscGVycyBmb3IgcGFyc2luZyBkaXNwbGF5IHRpbWluZ3MNCj4gKyAqDQo+ICsgKiBDb3B5
cmlnaHQgKGMpIDIwMTIgU3RlZmZlbiBUcnVtdHJhciA8cy50cnVtdHJhckBwZW5ndXRyb25peC5k
ZT4sIFBlbmd1dHJvbml4DQo+ICsgKg0KPiArICogYmFzZWQgb24gb2ZfdmlkZW9tb2RlLmMgYnkg
U2FzY2hhIEhhdWVyIDxzLmhhdWVyQHBlbmd1dHJvbml4LmRlPg0KPiArICoNCj4gKyAqIFRoaXMg
ZmlsZSBpcyByZWxlYXNlZCB1bmRlciB0aGUgR1BMdjINCj4gKyAqLw0KPiArI2luY2x1ZGUgPGxp
bnV4L29mLmg+DQo+ICsjaW5jbHVkZSA8bGludXgvc2xhYi5oPg0KPiArI2luY2x1ZGUgPGxpbnV4
L2V4cG9ydC5oPg0KPiArI2luY2x1ZGUgPGxpbnV4L29mX2Rpc3BsYXlfdGltaW5ncy5oPg0KPiAr
DQo+ICsvKioNCj4gKyAqIHBhcnNlX3Byb3BlcnR5IC0gcGFyc2UgdGltaW5nX2VudHJ5IGZyb20g
ZGV2aWNlX25vZGUNCj4gKyAqIEBucDogZGV2aWNlX25vZGUgd2l0aCB0aGUgcHJvcGVydHkNCj4g
KyAqIEBuYW1lOiBuYW1lIG9mIHRoZSBwcm9wZXJ0eQ0KPiArICogQHJlc3VsdDogd2lsbCBiZSBz
ZXQgdG8gdGhlIHJldHVybiB2YWx1ZQ0KPiArICoNCj4gKyAqIERFU0NSSVBUSU9OOg0KPiArICog
RXZlcnkgZGlzcGxheV90aW1pbmcgY2FuIGJlIHNwZWNpZmllZCB3aXRoIGVpdGhlciBqdXN0IHRo
ZSB0eXBpY2FsIHZhbHVlIG9yDQo+ICsgKiBhIHJhbmdlIGNvbnNpc3Rpbmcgb2YgbWluL3R5cC9t
YXguIFRoaXMgZnVuY3Rpb24gaGVscHMgaGFuZGxpbmcgdGhpcw0KPiArICoqLw0KPiArc3RhdGlj
IGludCBwYXJzZV9wcm9wZXJ0eShjb25zdCBzdHJ1Y3QgZGV2aWNlX25vZGUgKm5wLCBjb25zdCBj
aGFyICpuYW1lLA0KPiArCQkJICBzdHJ1Y3QgdGltaW5nX2VudHJ5ICpyZXN1bHQpDQo+ICt7DQo+
ICsJc3RydWN0IHByb3BlcnR5ICpwcm9wOw0KPiArCWludCBsZW5ndGgsIGNlbGxzLCByZXQ7DQo+
ICsNCj4gKwlwcm9wID0gb2ZfZmluZF9wcm9wZXJ0eShucCwgbmFtZSwgJmxlbmd0aCk7DQo+ICsJ
aWYgKCFwcm9wKSB7DQo+ICsJCXByX2VycigiJXM6IGNvdWxkIG5vdCBmaW5kIHByb3BlcnR5ICVz
XG4iLCBfX2Z1bmNfXywgbmFtZSk7DQo+ICsJCXJldHVybiAtRUlOVkFMOw0KPiArCX0NCj4gKw0K
PiArCWNlbGxzID0gbGVuZ3RoIC8gc2l6ZW9mKHUzMik7DQo+ICsJaWYgKGNlbGxzID09IDEpIHsN
Cj4gKwkJcmV0ID0gb2ZfcHJvcGVydHlfcmVhZF91MzIobnAsIG5hbWUsICZyZXN1bHQtPnR5cCk7
DQo+ICsJCXJlc3VsdC0+bWluID0gcmVzdWx0LT50eXA7DQo+ICsJCXJlc3VsdC0+bWF4ID0gcmVz
dWx0LT50eXA7DQo+ICsJfSBlbHNlIGlmIChjZWxscyA9PSAzKSB7DQo+ICsJCXJldCA9IG9mX3By
b3BlcnR5X3JlYWRfdTMyX2FycmF5KG5wLCBuYW1lLCAmcmVzdWx0LT5taW4sIGNlbGxzKTsNCj4g
Kwl9IGVsc2Ugew0KPiArCQlwcl9lcnIoIiVzOiBpbGxlZ2FsIHRpbWluZyBzcGVjaWZpY2F0aW9u
IGluICVzXG4iLCBfX2Z1bmNfXywgbmFtZSk7DQo+ICsJCXJldHVybiAtRUlOVkFMOw0KPiArCX0N
Cj4gKw0KPiArCXJldHVybiByZXQ7DQo+ICt9DQo+ICsNCj4gKy8qKg0KPiArICogb2ZfZ2V0X2Rp
c3BsYXlfdGltaW5nIC0gcGFyc2UgZGlzcGxheV90aW1pbmcgZW50cnkgZnJvbSBkZXZpY2Vfbm9k
ZQ0KPiArICogQG5wOiBkZXZpY2Vfbm9kZSB3aXRoIHRoZSBwcm9wZXJ0aWVzDQo+ICsgKiovDQo+
ICtzdGF0aWMgc3RydWN0IGRpc3BsYXlfdGltaW5nICpvZl9nZXRfZGlzcGxheV90aW1pbmcoY29u
c3Qgc3RydWN0IGRldmljZV9ub2RlICpucCkNCj4gK3sNCj4gKwlzdHJ1Y3QgZGlzcGxheV90aW1p
bmcgKmR0Ow0KPiArCWludCByZXQgPSAwOw0KPiArDQo+ICsJZHQgPSBremFsbG9jKHNpemVvZigq
ZHQpLCBHRlBfS0VSTkVMKTsNCj4gKwlpZiAoIWR0KSB7DQo+ICsJCXByX2VycigiJXM6IGNvdWxk
IG5vdCBhbGxvY2F0ZSBkaXNwbGF5X3RpbWluZyBzdHJ1Y3RcbiIsIF9fZnVuY19fKTsNCj4gKwkJ
cmV0dXJuIE5VTEw7DQo+ICsJfQ0KPiArDQo+ICsJcmV0IHw9IHBhcnNlX3Byb3BlcnR5KG5wLCAi
aGJhY2stcG9yY2giLCAmZHQtPmhiYWNrX3BvcmNoKTsNCj4gKwlyZXQgfD0gcGFyc2VfcHJvcGVy
dHkobnAsICJoZnJvbnQtcG9yY2giLCAmZHQtPmhmcm9udF9wb3JjaCk7DQo+ICsJcmV0IHw9IHBh
cnNlX3Byb3BlcnR5KG5wLCAiaGFjdGl2ZSIsICZkdC0+aGFjdGl2ZSk7DQo+ICsJcmV0IHw9IHBh
cnNlX3Byb3BlcnR5KG5wLCAiaHN5bmMtbGVuIiwgJmR0LT5oc3luY19sZW4pOw0KPiArCXJldCB8
PSBwYXJzZV9wcm9wZXJ0eShucCwgInZiYWNrLXBvcmNoIiwgJmR0LT52YmFja19wb3JjaCk7DQo+
ICsJcmV0IHw9IHBhcnNlX3Byb3BlcnR5KG5wLCAidmZyb250LXBvcmNoIiwgJmR0LT52ZnJvbnRf
cG9yY2gpOw0KPiArCXJldCB8PSBwYXJzZV9wcm9wZXJ0eShucCwgInZhY3RpdmUiLCAmZHQtPnZh
Y3RpdmUpOw0KPiArCXJldCB8PSBwYXJzZV9wcm9wZXJ0eShucCwgInZzeW5jLWxlbiIsICZkdC0+
dnN5bmNfbGVuKTsNCj4gKwlyZXQgfD0gcGFyc2VfcHJvcGVydHkobnAsICJjbG9jay1mcmVxdWVu
Y3kiLCAmZHQtPnBpeGVsY2xvY2spOw0KPiArDQo+ICsJb2ZfcHJvcGVydHlfcmVhZF91MzIobnAs
ICJ2c3luYy1hY3RpdmUiLCAmZHQtPnZzeW5jX3BvbF9hY3RpdmUpOw0KPiArCW9mX3Byb3BlcnR5
X3JlYWRfdTMyKG5wLCAiaHN5bmMtYWN0aXZlIiwgJmR0LT5oc3luY19wb2xfYWN0aXZlKTsNCj4g
KwlvZl9wcm9wZXJ0eV9yZWFkX3UzMihucCwgImRlLWFjdGl2ZSIsICZkdC0+ZGVfcG9sX2FjdGl2
ZSk7DQo+ICsJb2ZfcHJvcGVydHlfcmVhZF91MzIobnAsICJwaXhlbGNsay1pbnZlcnRlZCIsICZk
dC0+cGl4ZWxjbGtfcG9sKTsNCj4gKwlkdC0+aW50ZXJsYWNlZCA9IG9mX3Byb3BlcnR5X3JlYWRf
Ym9vbChucCwgImludGVybGFjZWQiKTsNCj4gKwlkdC0+ZG91Ymxlc2NhbiA9IG9mX3Byb3BlcnR5
X3JlYWRfYm9vbChucCwgImRvdWJsZXNjYW4iKTsNCj4gKw0KPiArCWlmIChyZXQpIHsNCj4gKwkJ
cHJfZXJyKCIlczogZXJyb3IgcmVhZGluZyB0aW1pbmcgcHJvcGVydGllc1xuIiwgX19mdW5jX18p
Ow0KPiArCQlrZnJlZShkdCk7DQo+ICsJCXJldHVybiBOVUxMOw0KPiArCX0NCj4gKw0KPiArCXJl
dHVybiBkdDsNCj4gK30NCj4gKw0KPiArLyoqDQo+ICsgKiBvZl9nZXRfZGlzcGxheV90aW1pbmdz
IC0gcGFyc2UgYWxsIGRpc3BsYXlfdGltaW5nIGVudHJpZXMgZnJvbSBhIGRldmljZV9ub2RlDQo+
ICsgKiBAbnA6IGRldmljZV9ub2RlIHdpdGggdGhlIHN1Ym5vZGVzDQo+ICsgKiovDQo+ICtzdHJ1
Y3QgZGlzcGxheV90aW1pbmdzICpvZl9nZXRfZGlzcGxheV90aW1pbmdzKGNvbnN0IHN0cnVjdCBk
ZXZpY2Vfbm9kZSAqbnApDQo+ICt7DQo+ICsJc3RydWN0IGRldmljZV9ub2RlICp0aW1pbmdzX25w
Ow0KPiArCXN0cnVjdCBkZXZpY2Vfbm9kZSAqZW50cnk7DQo+ICsJc3RydWN0IGRldmljZV9ub2Rl
ICpuYXRpdmVfbW9kZTsNCj4gKwlzdHJ1Y3QgZGlzcGxheV90aW1pbmdzICpkaXNwOw0KPiArDQo+
ICsJaWYgKCFucCkgew0KPiArCQlwcl9lcnIoIiVzOiBubyBkZXZpY2Vub2RlIGdpdmVuXG4iLCBf
X2Z1bmNfXyk7DQo+ICsJCXJldHVybiBOVUxMOw0KPiArCX0NCj4gKw0KPiArCXRpbWluZ3NfbnAg
PSBvZl9maW5kX25vZGVfYnlfbmFtZShucCwgImRpc3BsYXktdGltaW5ncyIpOw0KDQpJIGdldCBi
ZWxvdyBidWlsZCB3YXJuaW5ncyBvbiB0aGlzIGxpbmUNCmRyaXZlcnMvdmlkZW8vb2ZfZGlzcGxh
eV90aW1pbmcuYzogSW4gZnVuY3Rpb24gJ29mX2dldF9kaXNwbGF5X3RpbWluZ3MnOg0KZHJpdmVy
cy92aWRlby9vZl9kaXNwbGF5X3RpbWluZy5jOjEwOToyOiB3YXJuaW5nOiBwYXNzaW5nIGFyZ3Vt
ZW50IDEgb2YgJ29mX2ZpbmRfbm9kZV9ieV9uYW1lJyBkaXNjYXJkcyBxdWFsaWZpZXJzIGZyb20g
cG9pbnRlciB0YXJnZXQgdHlwZQ0KaW5jbHVkZS9saW51eC9vZi5oOjE2NzoyODogbm90ZTogZXhw
ZWN0ZWQgJ3N0cnVjdCBkZXZpY2Vfbm9kZSAqJyBidXQgYXJndW1lbnQgaXMgb2YgdHlwZSAnY29u
c3Qgc3RydWN0IGRldmljZV9ub2RlIConDQoNCj4gKwlpZiAoIXRpbWluZ3NfbnApIHsNCj4gKwkJ
cHJfZXJyKCIlczogY291bGQgbm90IGZpbmQgZGlzcGxheS10aW1pbmdzIG5vZGVcbiIsIF9fZnVu
Y19fKTsNCj4gKwkJcmV0dXJuIE5VTEw7DQo+ICsJfQ0KPiArDQo+ICsJZGlzcCA9IGt6YWxsb2Mo
c2l6ZW9mKCpkaXNwKSwgR0ZQX0tFUk5FTCk7DQo+ICsJaWYgKCFkaXNwKSB7DQo+ICsJCXByX2Vy
cigiJXM6IGNvdWxkIG5vdCBhbGxvY2F0ZSBzdHJ1Y3QgZGlzcCdcbiIsIF9fZnVuY19fKTsNCj4g
KwkJZ290byBkaXNwZmFpbDsNCj4gKwl9DQo+ICsNCj4gKwllbnRyeSA9IG9mX3BhcnNlX3BoYW5k
bGUodGltaW5nc19ucCwgIm5hdGl2ZS1tb2RlIiwgMCk7DQo+ICsJLyogYXNzdW1lIGZpcnN0IGNo
aWxkIGFzIG5hdGl2ZSBtb2RlIGlmIG5vbmUgcHJvdmlkZWQgKi8NCj4gKwlpZiAoIWVudHJ5KQ0K
PiArCQllbnRyeSA9IG9mX2dldF9uZXh0X2NoaWxkKG5wLCBOVUxMKTsNCj4gKwkvKiBpZiB0aGVy
ZSBpcyBubyBjaGlsZCwgaXQgaXMgdXNlbGVzcyB0byBnbyBvbiAqLw0KPiArCWlmICghZW50cnkp
IHsNCj4gKwkJcHJfZXJyKCIlczogbm8gdGltaW5nIHNwZWNpZmljYXRpb25zIGdpdmVuXG4iLCBf
X2Z1bmNfXyk7DQo+ICsJCWdvdG8gZW50cnlmYWlsOw0KPiArCX0NCj4gKw0KPiArCXByX2luZm8o
IiVzOiB1c2luZyAlcyBhcyBkZWZhdWx0IHRpbWluZ1xuIiwgX19mdW5jX18sIGVudHJ5LT5uYW1l
KTsNCj4gKw0KPiArCW5hdGl2ZV9tb2RlID0gZW50cnk7DQo+ICsNCj4gKwlkaXNwLT5udW1fdGlt
aW5ncyA9IG9mX2dldF9jaGlsZF9jb3VudCh0aW1pbmdzX25wKTsNCj4gKwlpZiAoZGlzcC0+bnVt
X3RpbWluZ3MgPT0gMCkgew0KPiArCQkvKiBzaG91bGQgbmV2ZXIgaGFwcGVuLCBhcyBlbnRyeSB3
YXMgYWxyZWFkeSBmb3VuZCBhYm92ZSAqLw0KPiArCQlwcl9lcnIoIiVzOiBubyB0aW1pbmdzIHNw
ZWNpZmllZFxuIiwgX19mdW5jX18pOw0KPiArCQlnb3RvIGVudHJ5ZmFpbDsNCj4gKwl9DQo+ICsN
Cj4gKwlkaXNwLT50aW1pbmdzID0ga3phbGxvYyhzaXplb2Yoc3RydWN0IGRpc3BsYXlfdGltaW5n
ICopICogZGlzcC0+bnVtX3RpbWluZ3MsDQo+ICsJCQkJR0ZQX0tFUk5FTCk7DQo+ICsJaWYgKCFk
aXNwLT50aW1pbmdzKSB7DQo+ICsJCXByX2VycigiJXM6IGNvdWxkIG5vdCBhbGxvY2F0ZSB0aW1p
bmdzIGFycmF5XG4iLCBfX2Z1bmNfXyk7DQo+ICsJCWdvdG8gZW50cnlmYWlsOw0KPiArCX0NCj4g
Kw0KPiArCWRpc3AtPm51bV90aW1pbmdzID0gMDsNCj4gKwlkaXNwLT5uYXRpdmVfbW9kZSA9IDA7
DQo+ICsNCj4gKwlmb3JfZWFjaF9jaGlsZF9vZl9ub2RlKHRpbWluZ3NfbnAsIGVudHJ5KSB7DQo+
ICsJCXN0cnVjdCBkaXNwbGF5X3RpbWluZyAqZHQ7DQo+ICsNCj4gKwkJZHQgPSBvZl9nZXRfZGlz
cGxheV90aW1pbmcoZW50cnkpOw0KPiArCQlpZiAoIWR0KSB7DQo+ICsJCQkvKg0KPiArCQkJICog
dG8gbm90IGVuY291cmFnZSB3cm9uZyBkZXZpY2V0cmVlcywgZmFpbCBpbiBjYXNlIG9mDQo+ICsJ
CQkgKiBhbiBlcnJvcg0KPiArCQkJICovDQo+ICsJCQlwcl9lcnIoIiVzOiBlcnJvciBpbiB0aW1p
bmcgJWRcbiIsIF9fZnVuY19fLA0KPiArCQkJICAgICAgIGRpc3AtPm51bV90aW1pbmdzICsgMSk7
DQo+ICsJCQlnb3RvIHRpbWluZ2ZhaWw7DQo+ICsJCX0NCj4gKw0KPiArCQlpZiAobmF0aXZlX21v
ZGUgPT0gZW50cnkpDQo+ICsJCQlkaXNwLT5uYXRpdmVfbW9kZSA9IGRpc3AtPm51bV90aW1pbmdz
Ow0KPiArDQo+ICsJCWRpc3AtPnRpbWluZ3NbZGlzcC0+bnVtX3RpbWluZ3NdID0gZHQ7DQo+ICsJ
CWRpc3AtPm51bV90aW1pbmdzKys7DQo+ICsJfQ0KPiArCW9mX25vZGVfcHV0KHRpbWluZ3NfbnAp
Ow0KPiArCW9mX25vZGVfcHV0KG5hdGl2ZV9tb2RlKTsNCj4gKw0KPiArCWlmIChkaXNwLT5udW1f
dGltaW5ncyA+IDApDQo+ICsJCXByX2luZm8oIiVzOiBnb3QgJWQgdGltaW5ncy4gVXNpbmcgdGlt
aW5nICMlZCBhcyBkZWZhdWx0XG4iLA0KPiArCQkJX19mdW5jX18sIGRpc3AtPm51bV90aW1pbmdz
LCBkaXNwLT5uYXRpdmVfbW9kZSArIDEpOw0KPiArCWVsc2Ugew0KPiArCQlwcl9lcnIoIiVzOiBu
byB2YWxpZCB0aW1pbmdzIHNwZWNpZmllZFxuIiwgX19mdW5jX18pOw0KPiArCQlkaXNwbGF5X3Rp
bWluZ3NfcmVsZWFzZShkaXNwKTsNCj4gKwkJcmV0dXJuIE5VTEw7DQo+ICsJfQ0KPiArCXJldHVy
biBkaXNwOw0KPiArDQo+ICt0aW1pbmdmYWlsOg0KPiArCWlmIChuYXRpdmVfbW9kZSkNCj4gKwkJ
b2Zfbm9kZV9wdXQobmF0aXZlX21vZGUpOw0KPiArCWRpc3BsYXlfdGltaW5nc19yZWxlYXNlKGRp
c3ApOw0KPiArZW50cnlmYWlsOg0KPiArCWlmIChkaXNwKQ0KPiArCQlrZnJlZShkaXNwKTsNCj4g
K2Rpc3BmYWlsOg0KPiArCW9mX25vZGVfcHV0KHRpbWluZ3NfbnApOw0KPiArCXJldHVybiBOVUxM
Ow0KPiArfQ0KPiArRVhQT1JUX1NZTUJPTF9HUEwob2ZfZ2V0X2Rpc3BsYXlfdGltaW5ncyk7DQo+
ICsNCj4gKy8qKg0KPiArICogb2ZfZGlzcGxheV90aW1pbmdzX2V4aXN0cyAtIGNoZWNrIGlmIGEg
ZGlzcGxheS10aW1pbmdzIG5vZGUgaXMgcHJvdmlkZWQNCj4gKyAqIEBucDogZGV2aWNlX25vZGUg
d2l0aCB0aGUgdGltaW5nDQo+ICsgKiovDQo+ICtpbnQgb2ZfZGlzcGxheV90aW1pbmdzX2V4aXN0
cyhjb25zdCBzdHJ1Y3QgZGV2aWNlX25vZGUgKm5wKQ0KPiArew0KPiArCXN0cnVjdCBkZXZpY2Vf
bm9kZSAqdGltaW5nc19ucDsNCj4gKw0KPiArCWlmICghbnApDQo+ICsJCXJldHVybiAtRUlOVkFM
Ow0KPiArDQo+ICsJdGltaW5nc19ucCA9IG9mX3BhcnNlX3BoYW5kbGUobnAsICJkaXNwbGF5LXRp
bWluZ3MiLCAwKTsNCg0KQWxzbyBoZXJlOg0KZHJpdmVycy92aWRlby9vZl9kaXNwbGF5X3RpbWlu
Zy5jOiBJbiBmdW5jdGlvbiAnb2ZfZGlzcGxheV90aW1pbmdzX2V4aXN0cyc6DQpkcml2ZXJzL3Zp
ZGVvL29mX2Rpc3BsYXlfdGltaW5nLmM6MjA5OjI6IHdhcm5pbmc6IHBhc3NpbmcgYXJndW1lbnQg
MSBvZiAnb2ZfcGFyc2VfcGhhbmRsZScgZGlzY2FyZHMgcXVhbGlmaWVycyBmcm9tIHBvaW50ZXIg
dGFyZ2V0IHR5cGUNCmluY2x1ZGUvbGludXgvb2YuaDoyNTg6Mjg6IG5vdGU6IGV4cGVjdGVkICdz
dHJ1Y3QgZGV2aWNlX25vZGUgKicgYnV0IGFyZ3VtZW50IGlzIG9mIHR5cGUgJ2NvbnN0IHN0cnVj
dCBkZXZpY2Vfbm9kZSAqJw0KDQpUaGFua3MsDQpQcmFrYXNoDQoNCj4gKwlpZiAoIXRpbWluZ3Nf
bnApDQo+ICsJCXJldHVybiAtRUlOVkFMOw0KPiArDQo+ICsJb2Zfbm9kZV9wdXQodGltaW5nc19u
cCk7DQo+ICsJcmV0dXJuIDE7DQo+ICt9DQo+ICtFWFBPUlRfU1lNQk9MX0dQTChvZl9kaXNwbGF5
X3RpbWluZ3NfZXhpc3RzKTsNCj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvdmlkZW8vb2ZfdmlkZW9t
b2RlLmMgYi9kcml2ZXJzL3ZpZGVvL29mX3ZpZGVvbW9kZS5jDQo+IG5ldyBmaWxlIG1vZGUgMTAw
NjQ0DQo+IGluZGV4IDAwMDAwMDAuLmM1NzNmOTINCj4gLS0tIC9kZXYvbnVsbA0KPiArKysgYi9k
cml2ZXJzL3ZpZGVvL29mX3ZpZGVvbW9kZS5jDQo+IEBAIC0wLDAgKzEsNDggQEANCj4gKy8qDQo+
ICsgKiBnZW5lcmljIHZpZGVvbW9kZSBoZWxwZXINCj4gKyAqDQo+ICsgKiBDb3B5cmlnaHQgKGMp
IDIwMTIgU3RlZmZlbiBUcnVtdHJhciA8cy50cnVtdHJhckBwZW5ndXRyb25peC5kZT4sIFBlbmd1
dHJvbml4DQo+ICsgKg0KPiArICogVGhpcyBmaWxlIGlzIHJlbGVhc2VkIHVuZGVyIHRoZSBHUEx2
Mg0KPiArICovDQo+ICsjaW5jbHVkZSA8bGludXgvb2YuaD4NCj4gKyNpbmNsdWRlIDxsaW51eC9v
Zl9kaXNwbGF5X3RpbWluZ3MuaD4NCj4gKyNpbmNsdWRlIDxsaW51eC9vZl92aWRlb21vZGUuaD4N
Cj4gKyNpbmNsdWRlIDxsaW51eC9leHBvcnQuaD4NCj4gKw0KPiArLyoqDQo+ICsgKiBvZl9nZXRf
dmlkZW9tb2RlIC0gZ2V0IHRoZSB2aWRlb21vZGUgIzxpbmRleD4gZnJvbSBkZXZpY2V0cmVlDQo+
ICsgKiBAbnAgLSBkZXZpY2Vub2RlIHdpdGggdGhlIGRpc3BsYXlfdGltaW5ncw0KPiArICogQHZt
IC0gc2V0IHRvIHJldHVybiB2YWx1ZQ0KPiArICogQGluZGV4IC0gaW5kZXggaW50byBsaXN0IG9m
IGRpc3BsYXlfdGltaW5ncw0KPiArICogREVTQ1JJUFRJT046DQo+ICsgKiBHZXQgYSBsaXN0IG9m
IGFsbCBkaXNwbGF5IHRpbWluZ3MgYW5kIHB1dCB0aGUgb25lDQo+ICsgKiBzcGVjaWZpZWQgYnkg
aW5kZXggaW50byAqdm0uIFRoaXMgZnVuY3Rpb24gc2hvdWxkIG9ubHkgYmUgdXNlZCwgaWYNCj4g
KyAqIG9ubHkgb25lIHZpZGVvbW9kZSBpcyB0byBiZSByZXRyaWV2ZWQuIEEgZHJpdmVyIHRoYXQg
bmVlZHMgdG8gd29yaw0KPiArICogd2l0aCBtdWx0aXBsZS9hbGwgdmlkZW9tb2RlcyBzaG91bGQg
d29yayB3aXRoDQo+ICsgKiBvZl9nZXRfZGlzcGxheV90aW1pbmdzIGluc3RlYWQuDQo+ICsgKiov
DQo+ICtpbnQgb2ZfZ2V0X3ZpZGVvbW9kZShjb25zdCBzdHJ1Y3QgZGV2aWNlX25vZGUgKm5wLCBz
dHJ1Y3QgdmlkZW9tb2RlICp2bSwNCj4gKwkJICAgICBpbnQgaW5kZXgpDQo+ICt7DQo+ICsJc3Ry
dWN0IGRpc3BsYXlfdGltaW5ncyAqZGlzcDsNCj4gKwlpbnQgcmV0Ow0KPiArDQo+ICsJZGlzcCA9
IG9mX2dldF9kaXNwbGF5X3RpbWluZ3MobnApOw0KPiArCWlmICghZGlzcCkgew0KPiArCQlwcl9l
cnIoIiVzOiBubyB0aW1pbmdzIHNwZWNpZmllZFxuIiwgX19mdW5jX18pOw0KPiArCQlyZXR1cm4g
LUVJTlZBTDsNCj4gKwl9DQo+ICsNCj4gKwlpZiAoaW5kZXggPT0gT0ZfVVNFX05BVElWRV9NT0RF
KQ0KPiArCQlpbmRleCA9IGRpc3AtPm5hdGl2ZV9tb2RlOw0KPiArDQo+ICsJcmV0ID0gdmlkZW9t
b2RlX2Zyb21fdGltaW5nKGRpc3AsIHZtLCBpbmRleCk7DQo+ICsJaWYgKHJldCkNCj4gKwkJcmV0
dXJuIHJldDsNCj4gKw0KPiArCWRpc3BsYXlfdGltaW5nc19yZWxlYXNlKGRpc3ApOw0KPiArDQo+
ICsJcmV0dXJuIDA7DQo+ICt9DQo+ICtFWFBPUlRfU1lNQk9MX0dQTChvZl9nZXRfdmlkZW9tb2Rl
KTsNCj4gZGlmZiAtLWdpdCBhL2luY2x1ZGUvbGludXgvb2ZfZGlzcGxheV90aW1pbmdzLmggYi9p
bmNsdWRlL2xpbnV4L29mX2Rpc3BsYXlfdGltaW5ncy5oDQo+IG5ldyBmaWxlIG1vZGUgMTAwNjQ0
DQo+IGluZGV4IDAwMDAwMDAuLjJiNGZhMGENCj4gLS0tIC9kZXYvbnVsbA0KPiArKysgYi9pbmNs
dWRlL2xpbnV4L29mX2Rpc3BsYXlfdGltaW5ncy5oDQo+IEBAIC0wLDAgKzEsMjAgQEANCj4gKy8q
DQo+ICsgKiBDb3B5cmlnaHQgMjAxMiBTdGVmZmVuIFRydW10cmFyIDxzLnRydW10cmFyQHBlbmd1
dHJvbml4LmRlPg0KPiArICoNCj4gKyAqIGRpc3BsYXkgdGltaW5ncyBvZiBoZWxwZXJzDQo+ICsg
Kg0KPiArICogVGhpcyBmaWxlIGlzIHJlbGVhc2VkIHVuZGVyIHRoZSBHUEx2Mg0KPiArICovDQo+
ICsNCj4gKyNpZm5kZWYgX19MSU5VWF9PRl9ESVNQTEFZX1RJTUlOR1NfSA0KPiArI2RlZmluZSBf
X0xJTlVYX09GX0RJU1BMQVlfVElNSU5HU19IDQo+ICsNCj4gKyNpbmNsdWRlIDxsaW51eC9kaXNw
bGF5X3RpbWluZy5oPg0KPiArI2luY2x1ZGUgPGxpbnV4L29mLmg+DQo+ICsNCj4gKyNkZWZpbmUg
T0ZfVVNFX05BVElWRV9NT0RFIC0xDQo+ICsNCj4gK3N0cnVjdCBkaXNwbGF5X3RpbWluZ3MgKm9m
X2dldF9kaXNwbGF5X3RpbWluZ3MoY29uc3Qgc3RydWN0IGRldmljZV9ub2RlICpucCk7DQo+ICtp
bnQgb2ZfZGlzcGxheV90aW1pbmdzX2V4aXN0cyhjb25zdCBzdHJ1Y3QgZGV2aWNlX25vZGUgKm5w
KTsNCj4gKw0KPiArI2VuZGlmDQo+IGRpZmYgLS1naXQgYS9pbmNsdWRlL2xpbnV4L29mX3ZpZGVv
bW9kZS5oIGIvaW5jbHVkZS9saW51eC9vZl92aWRlb21vZGUuaA0KPiBuZXcgZmlsZSBtb2RlIDEw
MDY0NA0KPiBpbmRleCAwMDAwMDAwLi40ZGU1ZmNjDQo+IC0tLSAvZGV2L251bGwNCj4gKysrIGIv
aW5jbHVkZS9saW51eC9vZl92aWRlb21vZGUuaA0KPiBAQCAtMCwwICsxLDE4IEBADQo+ICsvKg0K
PiArICogQ29weXJpZ2h0IDIwMTIgU3RlZmZlbiBUcnVtdHJhciA8cy50cnVtdHJhckBwZW5ndXRy
b25peC5kZT4NCj4gKyAqDQo+ICsgKiB2aWRlb21vZGUgb2YtaGVscGVycw0KPiArICoNCj4gKyAq
IFRoaXMgZmlsZSBpcyByZWxlYXNlZCB1bmRlciB0aGUgR1BMdjINCj4gKyAqLw0KPiArDQo+ICsj
aWZuZGVmIF9fTElOVVhfT0ZfVklERU9NT0RFX0gNCj4gKyNkZWZpbmUgX19MSU5VWF9PRl9WSURF
T01PREVfSA0KPiArDQo+ICsjaW5jbHVkZSA8bGludXgvdmlkZW9tb2RlLmg+DQo+ICsjaW5jbHVk
ZSA8bGludXgvb2YuaD4NCj4gKw0KPiAraW50IG9mX2dldF92aWRlb21vZGUoY29uc3Qgc3RydWN0
IGRldmljZV9ub2RlICpucCwgc3RydWN0IHZpZGVvbW9kZSAqdm0sDQo+ICsJCSAgICAgaW50IGlu
ZGV4KTsNCj4gKw0KPiArI2VuZGlmIC8qIF9fTElOVVhfT0ZfVklERU9NT0RFX0ggKi8NCj4gLS0g
DQo+IDEuNy4xMC40DQo+IA0KPiAtLQ0KPiBUbyB1bnN1YnNjcmliZSBmcm9tIHRoaXMgbGlzdDog
c2VuZCB0aGUgbGluZSAidW5zdWJzY3JpYmUgbGludXgtZmJkZXYiIGluDQo+IHRoZSBib2R5IG9m
IGEgbWVzc2FnZSB0byBtYWpvcmRvbW9Admdlci5rZXJuZWwub3JnDQo+IE1vcmUgbWFqb3Jkb21v
IGluZm8gYXQgIGh0dHA6Ly92Z2VyLmtlcm5lbC5vcmcvbWFqb3Jkb21vLWluZm8uaHRtbA0KPiAN
Cg0K

^ permalink raw reply

* Re: [PATCHv9 1/3] Runtime Interpreted Power Sequences
From: Tomi Valkeinen @ 2012-11-21 11:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <13540495.epaCf4JVn9@percival>

On 2012-11-21 06:23, Alex Courbot wrote:
> Hi Grant,
> 
> On Wednesday 21 November 2012 05:54:29 Grant Likely wrote:
>>> With the advent of the device tree and of ARM kernels that are not
>>> board-tied, we cannot rely on these board-specific hooks anymore but
>>
>> This isn't strictly true. It is still perfectly fine to have board
>> specific code when necessary. However, there is strong encouragement to
>> enable that code in device drivers as much as possible and new board
>> files need to have very strong justification.
> 
> But doesn't introducing board-specific code into the kernel just defeats the 
> purpose of the DT? If we extend this logic, we are heading straight back to 
> board-definition files. To a lesser extent than before I agree, but the problem 
> is fundamentally the same.

I don't think so. I'll reiterate my opinion on this subject, as a
summary and for those who haven't read the discussions of the earlier
versions of the series. And perhaps I'll even manage to say something
new =).


First about the board specific code. I think we may need some board
specific code, even if this series was merged. Let's call them board
drivers. These board drivers would only exist for boards with some weird
setups that cannot be expressed or managed with DT and normal drivers.

I think these cases would be quite rare, as I can't even come up with a
very good example. I guess most likely these cases would involve some
small trivial chips for muxing or such, for which it doesn't really make
sense to have a real driver.

Say, perhaps a board with two LCDs connected to one video output, and
only one LCD can be enabled at a time, and you need to set some mux chip
to route the signals to the correct LCD. In this case I'd see we should
have hotplug support in the display framework, and the board driver
would act on user input (sysfs file, perhaps), plugging in/out the LCD
device depending on the user input.


As for expressing device internal details in the DT data, as done in
this series, I don't like it very much. I think the DT data or the board
file should just describe which device is the one in question, and how
it's connected to other components on the board. The driver for the
device should handle everything else.

As Alex pointed out, there may be lots of devices that work the same way
when enabled, but require slightly different power-on/off sequences. We
could have these sequences in the driver itself, either as plain code,
or in a table of some sort, possibly using the power sequence framework
presented in this series. The correct code or sequence would be ran
depending on the particular model of the device.

I think this approach is correct in the sense that this is what drivers
are supposed to do: handle all the device internal matters. But this
raises the problem of bloating the kernel with possibly lots of
different power sequences, of which only a few would be used by a board,
and all the rest would just be waste of memory.

Regarding this problem I have the following questions, to which I don't
have clear answers:

- How much of this device specific data is too much? If a driver
supports 10 different models, and the sequences for each model take 100
bytes, this 1000 bytes doesn't sound too much. But where's the limit?
And even if one driver only has 1kB of this data, what if we have lots
of similar drivers?

- How many bytes does a power sequence presented in this series take, if
the sequence contains, say, 5 steps, with gpio, delay, pwm, delay and
regulator?

- How likely is it that we'll get lots of different models? A hundred
different models for a backlight PWM with different power-on/off
sequences already sounds a really big number. If we're only going to
have each driver supporting, say, no more than 10 models, perhaps the
problem is not even an issue in practice.

- Are there ways to limit this bloat in the driver? One obvious way
would be to discard the unused sequences after driver probe, but that
only works for platform devices. Although, I guess these sequences would
mostly be used by platform drivers? If so, then the problem could be
solved by discarding the data after probe. Another would be to load the
sequences from a file as firmware, but that feels quite an awful
solution. Anybody have other ideas?

One clear positive side with in-driver approach is that it's totally
inside the kernel, and can be easily reworked in the future, or even
changed to a DT-based approach as presented in this series if that seems
like a better solution. The DT based approach, on the other hand, will
be more or less written to stone after it's merged.


So, I like the framework of expressing the sequences presented in this
series (except there's a problem with error cases, as I pointed out in
another post), as it can be used inside the drivers. But I'm not so
enthusiastic about presenting the sequences in DT.

My suggestion would be to go forward with an in-driver solution, and
look at the DT based solution later if we are seeing an increasing bloat
in the drivers.

 Tomi


^ permalink raw reply

* Re: [PATCH v12 3/6] fbmon: add videomode helpers
From: Leela Krishna Amudala @ 2012-11-21 11:21 UTC (permalink / raw)
  To: Manjunathappa, Prakash
  Cc: Steffen Trumtrar, devicetree-discuss@lists.ozlabs.org,
	linux-fbdev@vger.kernel.org, David Airlie,
	Florian Tobias Schandinat, dri-devel@lists.freedesktop.org,
	Valkeinen, Tomi, Laurent Pinchart, kernel@pengutronix.de,
	Guennady Liakhovetski, linux-media@vger.kernel.org
In-Reply-To: <A73F36158E33644199EB82C5EC81C7BC3E9FA769@DBDE01.ent.ti.com>

Yes,
Even I got the same build error.
later I fixed it by including "#include <linux/mxsfb.h>"

Best Wishes,
Leela Krishna.

On Wed, Nov 21, 2012 at 3:39 PM, Manjunathappa, Prakash
<prakash.pm@ti.com> wrote:
> Hi Steffen,
>
> I am trying to add DT support for da8xx-fb driver on top of your patches.
> Encountered below build error. Sorry for reporting it late.
>
> On Tue, Nov 20, 2012 at 21:24:53, Steffen Trumtrar wrote:
>> Add a function to convert from the generic videomode to a fb_videomode.
>>
>> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
>> Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
>> Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
>> Tested-by: Thierry Reding <thierry.reding@avionic-design.de>
>> Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> ---
>>  drivers/video/fbmon.c |   46 ++++++++++++++++++++++++++++++++++++++++++++++
>>  include/linux/fb.h    |    6 ++++++
>>  2 files changed, 52 insertions(+)
>>
>> diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
>> index cef6557..c1939a6 100644
>> --- a/drivers/video/fbmon.c
>> +++ b/drivers/video/fbmon.c
>> @@ -31,6 +31,7 @@
>>  #include <linux/pci.h>
>>  #include <linux/slab.h>
>>  #include <video/edid.h>
>> +#include <linux/videomode.h>
>>  #ifdef CONFIG_PPC_OF
>>  #include <asm/prom.h>
>>  #include <asm/pci-bridge.h>
>> @@ -1373,6 +1374,51 @@ int fb_get_mode(int flags, u32 val, struct fb_var_screeninfo *var, struct fb_inf
>>       kfree(timings);
>>       return err;
>>  }
>> +
>> +#if IS_ENABLED(CONFIG_VIDEOMODE)
>> +int fb_videomode_from_videomode(const struct videomode *vm,
>> +                             struct fb_videomode *fbmode)
>> +{
>> +     unsigned int htotal, vtotal;
>> +
>> +     fbmode->xres = vm->hactive;
>> +     fbmode->left_margin = vm->hback_porch;
>> +     fbmode->right_margin = vm->hfront_porch;
>> +     fbmode->hsync_len = vm->hsync_len;
>> +
>> +     fbmode->yres = vm->vactive;
>> +     fbmode->upper_margin = vm->vback_porch;
>> +     fbmode->lower_margin = vm->vfront_porch;
>> +     fbmode->vsync_len = vm->vsync_len;
>> +
>> +     fbmode->pixclock = KHZ2PICOS(vm->pixelclock / 1000);
>> +
>> +     fbmode->sync = 0;
>> +     fbmode->vmode = 0;
>> +     if (vm->hah)
>> +             fbmode->sync |= FB_SYNC_HOR_HIGH_ACT;
>> +     if (vm->vah)
>> +             fbmode->sync |= FB_SYNC_VERT_HIGH_ACT;
>> +     if (vm->interlaced)
>> +             fbmode->vmode |= FB_VMODE_INTERLACED;
>> +     if (vm->doublescan)
>> +             fbmode->vmode |= FB_VMODE_DOUBLE;
>> +     if (vm->de)
>> +             fbmode->sync |= FB_SYNC_DATA_ENABLE_HIGH_ACT;
>
> "FB_SYNC_DATA_ENABLE_HIGH_ACT" seems to be mxsfb specific flag, I am getting
> build error on this. Please let me know if I am missing something.
>
> Thanks,
> Prakash
>
>> +     fbmode->flag = 0;
>> +
>> +     htotal = vm->hactive + vm->hfront_porch + vm->hback_porch +
>> +              vm->hsync_len;
>> +     vtotal = vm->vactive + vm->vfront_porch + vm->vback_porch +
>> +              vm->vsync_len;
>> +     fbmode->refresh = (vm->pixelclock * 1000) / (htotal * vtotal);
>> +
>> +     return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(fb_videomode_from_videomode);
>> +#endif
>> +
>> +
>>  #else
>>  int fb_parse_edid(unsigned char *edid, struct fb_var_screeninfo *var)
>>  {
>> diff --git a/include/linux/fb.h b/include/linux/fb.h
>> index c7a9571..920cbe3 100644
>> --- a/include/linux/fb.h
>> +++ b/include/linux/fb.h
>> @@ -14,6 +14,7 @@
>>  #include <linux/backlight.h>
>>  #include <linux/slab.h>
>>  #include <asm/io.h>
>> +#include <linux/videomode.h>
>>
>>  struct vm_area_struct;
>>  struct fb_info;
>> @@ -714,6 +715,11 @@ extern void fb_destroy_modedb(struct fb_videomode *modedb);
>>  extern int fb_find_mode_cvt(struct fb_videomode *mode, int margins, int rb);
>>  extern unsigned char *fb_ddc_read(struct i2c_adapter *adapter);
>>
>> +#if IS_ENABLED(CONFIG_VIDEOMODE)
>> +extern int fb_videomode_from_videomode(const struct videomode *vm,
>> +                                    struct fb_videomode *fbmode);
>> +#endif
>> +
>>  /* drivers/video/modedb.c */
>>  #define VESA_MODEDB_SIZE 34
>>  extern void fb_var_to_videomode(struct fb_videomode *mode,
>> --
>> 1.7.10.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>
> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss

^ permalink raw reply

* Re: [PATCH v12 1/6] video: add display_timing and videomode
From: Tomi Valkeinen @ 2012-11-21 11:37 UTC (permalink / raw)
  To: Steffen Trumtrar
  Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA, David Airlie,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	Florian Tobias Schandinat,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Laurent Pinchart,
	kernel-bIcnvbaLZ9MEGnE8C9+IrQ, Guennady Liakhovetski,
	linux-media-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1353426896-6045-2-git-send-email-s.trumtrar-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

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

Hi,

On 2012-11-20 17:54, Steffen Trumtrar wrote:
> Add display_timing structure and the according helper functions. This allows
> the description of a display via its supported timing parameters.
> 
> Every timing parameter can be specified as a single value or a range
> <min typ max>.
> 
> Also, add helper functions to convert from display timings to a generic videomode
> structure. This videomode can then be converted to the corresponding subsystem
> mode representation (e.g. fb_videomode).

Sorry for reviewing this so late.

One thing I'd like to see is some explanation of the structs involved.
For example, in this patch you present structs videomode, display_timing
and display_timings without giving any hint what they represent.

I'm not asking for you to write a long documentation, but perhaps the
header files could include a few lines of comments above the structs,
explaining the idea.

> +void display_timings_release(struct display_timings *disp)
> +{
> +	if (disp->timings) {
> +		unsigned int i;
> +
> +		for (i = 0; i < disp->num_timings; i++)
> +			kfree(disp->timings[i]);
> +		kfree(disp->timings);
> +	}
> +	kfree(disp);
> +}
> +EXPORT_SYMBOL_GPL(display_timings_release);

Perhaps this will become clearer after reading the following patches,
but it feels a bit odd to add a release function, without anything in
this patch that would actually allocate the timings.

> diff --git a/drivers/video/videomode.c b/drivers/video/videomode.c
> new file mode 100644
> index 0000000..e24f879
> --- /dev/null
> +++ b/drivers/video/videomode.c
> @@ -0,0 +1,46 @@
> +/*
> + * generic display timing functions
> + *
> + * Copyright (c) 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>, Pengutronix
> + *
> + * This file is released under the GPLv2
> + */
> +
> +#include <linux/export.h>
> +#include <linux/errno.h>
> +#include <linux/display_timing.h>
> +#include <linux/kernel.h>
> +#include <linux/videomode.h>
> +
> +int videomode_from_timing(const struct display_timings *disp,
> +			  struct videomode *vm, unsigned int index)
> +{
> +	struct display_timing *dt;
> +
> +	dt = display_timings_get(disp, index);
> +	if (!dt)
> +		return -EINVAL;
> +
> +	vm->pixelclock = display_timing_get_value(&dt->pixelclock, 0);
> +	vm->hactive = display_timing_get_value(&dt->hactive, 0);
> +	vm->hfront_porch = display_timing_get_value(&dt->hfront_porch, 0);
> +	vm->hback_porch = display_timing_get_value(&dt->hback_porch, 0);
> +	vm->hsync_len = display_timing_get_value(&dt->hsync_len, 0);
> +
> +	vm->vactive = display_timing_get_value(&dt->vactive, 0);
> +	vm->vfront_porch = display_timing_get_value(&dt->vfront_porch, 0);
> +	vm->vback_porch = display_timing_get_value(&dt->vback_porch, 0);
> +	vm->vsync_len = display_timing_get_value(&dt->vsync_len, 0);

Shouldn't all these calls get the typical value, with index 1?

> +
> +	vm->vah = dt->vsync_pol_active;
> +	vm->hah = dt->hsync_pol_active;
> +	vm->de = dt->de_pol_active;
> +	vm->pixelclk_pol = dt->pixelclk_pol;
> +
> +	vm->interlaced = dt->interlaced;
> +	vm->doublescan = dt->doublescan;
> +
> +	return 0;
> +}
> +
> +EXPORT_SYMBOL_GPL(videomode_from_timing);
> diff --git a/include/linux/display_timing.h b/include/linux/display_timing.h
> new file mode 100644
> index 0000000..d5bf03f
> --- /dev/null
> +++ b/include/linux/display_timing.h
> @@ -0,0 +1,70 @@
> +/*
> + * Copyright 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>
> + *
> + * description of display timings
> + *
> + * This file is released under the GPLv2
> + */
> +
> +#ifndef __LINUX_DISPLAY_TIMINGS_H
> +#define __LINUX_DISPLAY_TIMINGS_H
> +
> +#include <linux/types.h>

What is this needed for? u32? I don't see it defined in types.h

> +
> +struct timing_entry {
> +	u32 min;
> +	u32 typ;
> +	u32 max;
> +};
> +
> +struct display_timing {
> +	struct timing_entry pixelclock;
> +
> +	struct timing_entry hactive;
> +	struct timing_entry hfront_porch;
> +	struct timing_entry hback_porch;
> +	struct timing_entry hsync_len;
> +
> +	struct timing_entry vactive;
> +	struct timing_entry vfront_porch;
> +	struct timing_entry vback_porch;
> +	struct timing_entry vsync_len;
> +
> +	unsigned int vsync_pol_active;
> +	unsigned int hsync_pol_active;
> +	unsigned int de_pol_active;
> +	unsigned int pixelclk_pol;
> +	bool interlaced;
> +	bool doublescan;
> +};
> +
> +struct display_timings {
> +	unsigned int num_timings;
> +	unsigned int native_mode;
> +
> +	struct display_timing **timings;
> +};
> +
> +/*
> + * placeholder function until ranges are really needed
> + * the index parameter should then be used to select one of [min typ max]
> + */
> +static inline u32 display_timing_get_value(const struct timing_entry *te,
> +					   unsigned int index)
> +{
> +	return te->typ;
> +}

Why did you opt for a placeholder here? It feels trivial to me to have
support to get the min/typ/max value properly.

> +static inline struct display_timing *display_timings_get(const struct
> +							 display_timings *disp,
> +							 unsigned int index)
> +{
> +	if (disp->num_timings > index)
> +		return disp->timings[index];
> +	else
> +		return NULL;
> +}
> +
> +void display_timings_release(struct display_timings *disp);
> +
> +#endif
> diff --git a/include/linux/videomode.h b/include/linux/videomode.h
> new file mode 100644
> index 0000000..5d3e796
> --- /dev/null
> +++ b/include/linux/videomode.h
> @@ -0,0 +1,40 @@
> +/*
> + * Copyright 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>
> + *
> + * generic videomode description
> + *
> + * This file is released under the GPLv2
> + */
> +
> +#ifndef __LINUX_VIDEOMODE_H
> +#define __LINUX_VIDEOMODE_H
> +
> +#include <linux/display_timing.h>

This is not needed, just add:

struct display_timings;

> +struct videomode {
> +	u32 pixelclock;
> +	u32 refreshrate;
> +
> +	u32 hactive;
> +	u32 hfront_porch;
> +	u32 hback_porch;
> +	u32 hsync_len;
> +
> +	u32 vactive;
> +	u32 vfront_porch;
> +	u32 vback_porch;
> +	u32 vsync_len;
> +
> +	u32 hah;
> +	u32 vah;
> +	u32 de;
> +	u32 pixelclk_pol;
> +
> +	bool interlaced;
> +	bool doublescan;
> +};
> +
> +int videomode_from_timing(const struct display_timings *disp,
> +			  struct videomode *vm, unsigned int index);
> +

Are this and the few other functions above meant to be used from
drivers? If so, some explanation of the parameters here would be nice.
If they are just framework internal, they don't probably need that.

 Tomi



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

^ permalink raw reply

* Re: [PATCHv9 1/3] Runtime Interpreted Power Sequences
From: Thierry Reding @ 2012-11-21 11:40 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <50ACB59B.4090404@iki.fi>

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

On Wed, Nov 21, 2012 at 01:06:03PM +0200, Tomi Valkeinen wrote:
> On 2012-11-21 06:23, Alex Courbot wrote:
> > Hi Grant,
> > 
> > On Wednesday 21 November 2012 05:54:29 Grant Likely wrote:
> >>> With the advent of the device tree and of ARM kernels that are not
> >>> board-tied, we cannot rely on these board-specific hooks anymore but
> >>
> >> This isn't strictly true. It is still perfectly fine to have board
> >> specific code when necessary. However, there is strong encouragement to
> >> enable that code in device drivers as much as possible and new board
> >> files need to have very strong justification.
> > 
> > But doesn't introducing board-specific code into the kernel just defeats the 
> > purpose of the DT? If we extend this logic, we are heading straight back to 
> > board-definition files. To a lesser extent than before I agree, but the problem 
> > is fundamentally the same.
> 
> I don't think so. I'll reiterate my opinion on this subject, as a
> summary and for those who haven't read the discussions of the earlier
> versions of the series. And perhaps I'll even manage to say something
> new =).
> 
> 
> First about the board specific code. I think we may need some board
> specific code, even if this series was merged. Let's call them board
> drivers. These board drivers would only exist for boards with some weird
> setups that cannot be expressed or managed with DT and normal drivers.
> 
> I think these cases would be quite rare, as I can't even come up with a
> very good example. I guess most likely these cases would involve some
> small trivial chips for muxing or such, for which it doesn't really make
> sense to have a real driver.
> 
> Say, perhaps a board with two LCDs connected to one video output, and
> only one LCD can be enabled at a time, and you need to set some mux chip
> to route the signals to the correct LCD. In this case I'd see we should
> have hotplug support in the display framework, and the board driver
> would act on user input (sysfs file, perhaps), plugging in/out the LCD
> device depending on the user input.
> 
> 
> As for expressing device internal details in the DT data, as done in
> this series, I don't like it very much. I think the DT data or the board
> file should just describe which device is the one in question, and how
> it's connected to other components on the board. The driver for the
> device should handle everything else.
> 
> As Alex pointed out, there may be lots of devices that work the same way
> when enabled, but require slightly different power-on/off sequences. We
> could have these sequences in the driver itself, either as plain code,
> or in a table of some sort, possibly using the power sequence framework
> presented in this series. The correct code or sequence would be ran
> depending on the particular model of the device.
> 
> I think this approach is correct in the sense that this is what drivers
> are supposed to do: handle all the device internal matters. But this
> raises the problem of bloating the kernel with possibly lots of
> different power sequences, of which only a few would be used by a board,
> and all the rest would just be waste of memory.
> 
> Regarding this problem I have the following questions, to which I don't
> have clear answers:
> 
> - How much of this device specific data is too much? If a driver
> supports 10 different models, and the sequences for each model take 100
> bytes, this 1000 bytes doesn't sound too much. But where's the limit?
> And even if one driver only has 1kB of this data, what if we have lots
> of similar drivers?
> 
> - How many bytes does a power sequence presented in this series take, if
> the sequence contains, say, 5 steps, with gpio, delay, pwm, delay and
> regulator?
> 
> - How likely is it that we'll get lots of different models? A hundred
> different models for a backlight PWM with different power-on/off
> sequences already sounds a really big number. If we're only going to
> have each driver supporting, say, no more than 10 models, perhaps the
> problem is not even an issue in practice.
> 
> - Are there ways to limit this bloat in the driver? One obvious way
> would be to discard the unused sequences after driver probe, but that
> only works for platform devices. Although, I guess these sequences would
> mostly be used by platform drivers? If so, then the problem could be
> solved by discarding the data after probe. Another would be to load the
> sequences from a file as firmware, but that feels quite an awful
> solution. Anybody have other ideas?
> 
> One clear positive side with in-driver approach is that it's totally
> inside the kernel, and can be easily reworked in the future, or even
> changed to a DT-based approach as presented in this series if that seems
> like a better solution. The DT based approach, on the other hand, will
> be more or less written to stone after it's merged.
> 
> 
> So, I like the framework of expressing the sequences presented in this
> series (except there's a problem with error cases, as I pointed out in
> another post), as it can be used inside the drivers. But I'm not so
> enthusiastic about presenting the sequences in DT.
> 
> My suggestion would be to go forward with an in-driver solution, and
> look at the DT based solution later if we are seeing an increasing bloat
> in the drivers.

Assuming we go with your approach, what's the plan? We're actually
facing this problem right now for Tegra. Basically we have a DRM driver
that can drive the panel, but we're still missing a way to hook up the
backlight and panel enabling code. So we effectively can't support any
of the LVDS devices out there without this series.

As I understand it, what you propose is similar to what ASoC does. For a
specific board, you'd have to write a driver, presumably for the new
panel/display framework, that provides code to power the panel on and
off. That means we'll have to have a driver for each panel out there
basically, or we'd need to write generic drivers that can be configured
to some degree (via platform data or DT). This is similar to how ASoC
works, where we have a driver that provides support for a specific codec
connected to the Tegra SoC. For the display framework things could be
done in a similar way I suppose, so that Tegra could have one display
driver to handle all aspects of powering on and off the various panels
for the various boards out there.

Obviously, a lot of the code will be similar for other SoCs, but maybe
that's just the way things are if we choose that approach. There's also
the potential for factoring out large chunks of common code later on
once we start to see common patterns.

One thing that's not very clear is how the backlight subsystem should be
wired up with the display framework. I have a patch on top of the Tegra
DRM driver which adds some ad-hoc display support using this power
sequences series and the pwm-backlight.

From reading the proposal for the panel/display framework, it sounds
like a lot more is planned than just enabling or disabling panels, but
it also seems like a lot of code needs to be written to support things
like DSI, DBI or other control busses.

At least for Tegra, and I think the same holds for a wide variety of
other SoCs, dumb panels would be enough for a start. In the interest of
getting a working solution for those setups, maybe we can start small
and add just enough framework to register dumb panel drivers to along
with code to wire up a backlight to light up the display. Then we could
possibly still make it to have a proper solution to support the various
LVDS panels for Tegra with 3.9.

I'm adding Laurent on Cc since he's probably busy working on a new
proposal for the panel/display framework. Maybe he can share his thought
on this.

All of the above said, I'm willing to help out with the coding if that's
what is required to reach a solution that everybody can be happy with.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH v12 2/6] video: add of helper for videomode
From: Steffen Trumtrar @ 2012-11-21 11:48 UTC (permalink / raw)
  To: Manjunathappa, Prakash
  Cc: devicetree-discuss@lists.ozlabs.org, Philipp Zabel, Rob Herring,
	linux-fbdev@vger.kernel.org, dri-devel@lists.freedesktop.org,
	Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
	linux-media@vger.kernel.org, Valkeinen, Tomi, Stephen Warren,
	kernel@pengutronix.de, Florian Tobias Schandinat, David Airlie
In-Reply-To: <A73F36158E33644199EB82C5EC81C7BC3E9FA7A0@DBDE01.ent.ti.com>

Hi!

On Wed, Nov 21, 2012 at 10:12:43AM +0000, Manjunathappa, Prakash wrote:
> Hi Steffen,
> 
> On Tue, Nov 20, 2012 at 21:24:52, Steffen Trumtrar wrote:
> > +/**
> > + * of_get_display_timings - parse all display_timing entries from a device_node
> > + * @np: device_node with the subnodes
> > + **/
> > +struct display_timings *of_get_display_timings(const struct device_node *np)
> > +{
> > +	struct device_node *timings_np;
> > +	struct device_node *entry;
> > +	struct device_node *native_mode;
> > +	struct display_timings *disp;
> > +
> > +	if (!np) {
> > +		pr_err("%s: no devicenode given\n", __func__);
> > +		return NULL;
> > +	}
> > +
> > +	timings_np = of_find_node_by_name(np, "display-timings");
> 
> I get below build warnings on this line
> drivers/video/of_display_timing.c: In function 'of_get_display_timings':
> drivers/video/of_display_timing.c:109:2: warning: passing argument 1 of 'of_find_node_by_name' discards qualifiers from pointer target type
> include/linux/of.h:167:28: note: expected 'struct device_node *' but argument is of type 'const struct device_node *'
> 
> > + * of_display_timings_exists - check if a display-timings node is provided
> > + * @np: device_node with the timing
> > + **/
> > +int of_display_timings_exists(const struct device_node *np)
> > +{
> > +	struct device_node *timings_np;
> > +
> > +	if (!np)
> > +		return -EINVAL;
> > +
> > +	timings_np = of_parse_phandle(np, "display-timings", 0);
> 
> Also here:
> drivers/video/of_display_timing.c: In function 'of_display_timings_exists':
> drivers/video/of_display_timing.c:209:2: warning: passing argument 1 of 'of_parse_phandle' discards qualifiers from pointer target type
> include/linux/of.h:258:28: note: expected 'struct device_node *' but argument is of type 'const struct device_node *'
> 

The warnings are because the of-functions do not use const pointers where they
should. I had two options: don't use const pointers even if they should be and
have no warnings or use const pointers and have a correct API. (Third option:
send patches for of-functions). I chose the second option.

Regards,
Steffen


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply

* Re: [PATCH v12 2/6] video: add of helper for videomode
From: Thierry Reding @ 2012-11-21 11:52 UTC (permalink / raw)
  To: Steffen Trumtrar
  Cc: Manjunathappa, Prakash, devicetree-discuss@lists.ozlabs.org,
	Philipp Zabel, Rob Herring, linux-fbdev@vger.kernel.org,
	dri-devel@lists.freedesktop.org, Laurent Pinchart,
	Guennady Liakhovetski, linux-media@vger.kernel.org,
	Valkeinen, Tomi, Stephen Warren, kernel@pengutronix.de,
	Florian Tobias Schandinat, David Airlie
In-Reply-To: <20121121114843.GC14013@pengutronix.de>

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

On Wed, Nov 21, 2012 at 12:48:43PM +0100, Steffen Trumtrar wrote:
> Hi!
> 
> On Wed, Nov 21, 2012 at 10:12:43AM +0000, Manjunathappa, Prakash wrote:
> > Hi Steffen,
> > 
> > On Tue, Nov 20, 2012 at 21:24:52, Steffen Trumtrar wrote:
> > > +/**
> > > + * of_get_display_timings - parse all display_timing entries from a device_node
> > > + * @np: device_node with the subnodes
> > > + **/
> > > +struct display_timings *of_get_display_timings(const struct device_node *np)
> > > +{
> > > +	struct device_node *timings_np;
> > > +	struct device_node *entry;
> > > +	struct device_node *native_mode;
> > > +	struct display_timings *disp;
> > > +
> > > +	if (!np) {
> > > +		pr_err("%s: no devicenode given\n", __func__);
> > > +		return NULL;
> > > +	}
> > > +
> > > +	timings_np = of_find_node_by_name(np, "display-timings");
> > 
> > I get below build warnings on this line
> > drivers/video/of_display_timing.c: In function 'of_get_display_timings':
> > drivers/video/of_display_timing.c:109:2: warning: passing argument 1 of 'of_find_node_by_name' discards qualifiers from pointer target type
> > include/linux/of.h:167:28: note: expected 'struct device_node *' but argument is of type 'const struct device_node *'
> > 
> > > + * of_display_timings_exists - check if a display-timings node is provided
> > > + * @np: device_node with the timing
> > > + **/
> > > +int of_display_timings_exists(const struct device_node *np)
> > > +{
> > > +	struct device_node *timings_np;
> > > +
> > > +	if (!np)
> > > +		return -EINVAL;
> > > +
> > > +	timings_np = of_parse_phandle(np, "display-timings", 0);
> > 
> > Also here:
> > drivers/video/of_display_timing.c: In function 'of_display_timings_exists':
> > drivers/video/of_display_timing.c:209:2: warning: passing argument 1 of 'of_parse_phandle' discards qualifiers from pointer target type
> > include/linux/of.h:258:28: note: expected 'struct device_node *' but argument is of type 'const struct device_node *'
> > 
> 
> The warnings are because the of-functions do not use const pointers where they
> should. I had two options: don't use const pointers even if they should be and
> have no warnings or use const pointers and have a correct API. (Third option:
> send patches for of-functions). I chose the second option.

Maybe a better approach would be a combination of 1 and 3: don't use
const pointers for struct device_node for now and bring the issue up
with the OF maintainers, possibly with patches attached that fix the
problematic functions.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH v12 3/6] fbmon: add videomode helpers
From: Steffen Trumtrar @ 2012-11-21 11:58 UTC (permalink / raw)
  To: Leela Krishna Amudala
  Cc: Manjunathappa, Prakash, devicetree-discuss@lists.ozlabs.org,
	linux-fbdev@vger.kernel.org, David Airlie,
	Florian Tobias Schandinat, dri-devel@lists.freedesktop.org,
	Valkeinen, Tomi, Laurent Pinchart, kernel@pengutronix.de,
	Guennady Liakhovetski, linux-media@vger.kernel.org
In-Reply-To: <CAL1wa8dQ4QL0SzbXdo8nogBfBjQ8GpaJ134v6zu_iMkWQeXefA@mail.gmail.com>

Hi!

On Wed, Nov 21, 2012 at 04:39:01PM +0530, Leela Krishna Amudala wrote:
> Yes,
> Even I got the same build error.
> later I fixed it by including "#include <linux/mxsfb.h>"
> 
> Best Wishes,
> Leela Krishna.
> 
> On Wed, Nov 21, 2012 at 3:39 PM, Manjunathappa, Prakash
> <prakash.pm@ti.com> wrote:
> > Hi Steffen,
> >
> > I am trying to add DT support for da8xx-fb driver on top of your patches.
> > Encountered below build error. Sorry for reporting it late.
> >
> > On Tue, Nov 20, 2012 at 21:24:53, Steffen Trumtrar wrote:
> >> Add a function to convert from the generic videomode to a fb_videomode.
> >>
> >> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> >> Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
> >> Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
> >> Tested-by: Thierry Reding <thierry.reding@avionic-design.de>
> >> Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
> >> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >> ---
> >>  drivers/video/fbmon.c |   46 ++++++++++++++++++++++++++++++++++++++++++++++
> >>  include/linux/fb.h    |    6 ++++++
> >>  2 files changed, 52 insertions(+)
> >>
> >> diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
> >> index cef6557..c1939a6 100644
> >> --- a/drivers/video/fbmon.c
> >> +++ b/drivers/video/fbmon.c
> >> @@ -31,6 +31,7 @@
> >>  #include <linux/pci.h>
> >>  #include <linux/slab.h>
> >>  #include <video/edid.h>
> >> +#include <linux/videomode.h>
> >>  #ifdef CONFIG_PPC_OF
> >>  #include <asm/prom.h>
> >>  #include <asm/pci-bridge.h>
> >> @@ -1373,6 +1374,51 @@ int fb_get_mode(int flags, u32 val, struct fb_var_screeninfo *var, struct fb_inf
> >>       kfree(timings);
> >>       return err;
> >>  }
> >> +
> >> +#if IS_ENABLED(CONFIG_VIDEOMODE)
> >> +int fb_videomode_from_videomode(const struct videomode *vm,
> >> +                             struct fb_videomode *fbmode)
> >> +{
> >> +     unsigned int htotal, vtotal;
> >> +
> >> +     fbmode->xres = vm->hactive;
> >> +     fbmode->left_margin = vm->hback_porch;
> >> +     fbmode->right_margin = vm->hfront_porch;
> >> +     fbmode->hsync_len = vm->hsync_len;
> >> +
> >> +     fbmode->yres = vm->vactive;
> >> +     fbmode->upper_margin = vm->vback_porch;
> >> +     fbmode->lower_margin = vm->vfront_porch;
> >> +     fbmode->vsync_len = vm->vsync_len;
> >> +
> >> +     fbmode->pixclock = KHZ2PICOS(vm->pixelclock / 1000);
> >> +
> >> +     fbmode->sync = 0;
> >> +     fbmode->vmode = 0;
> >> +     if (vm->hah)
> >> +             fbmode->sync |= FB_SYNC_HOR_HIGH_ACT;
> >> +     if (vm->vah)
> >> +             fbmode->sync |= FB_SYNC_VERT_HIGH_ACT;
> >> +     if (vm->interlaced)
> >> +             fbmode->vmode |= FB_VMODE_INTERLACED;
> >> +     if (vm->doublescan)
> >> +             fbmode->vmode |= FB_VMODE_DOUBLE;
> >> +     if (vm->de)
> >> +             fbmode->sync |= FB_SYNC_DATA_ENABLE_HIGH_ACT;
> >
> > "FB_SYNC_DATA_ENABLE_HIGH_ACT" seems to be mxsfb specific flag, I am getting
> > build error on this. Please let me know if I am missing something.
> >
> > Thanks,
> > Prakash
> >

I compile-tested the series and didn't have that error. But obviously I should
have. As this is a mxsfs flag, I will throw it out.

Regards,
Steffen

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ 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