Re: [PATCH v2 03/12] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes

[Sergei Shtylyov]

Hello!

On 1/13/2018 2:14 AM, Laurent Pinchart wrote:

   Here's my (superficial) comments...


The internal LVDS encoders now have their own DT bindings. Before
switching the driver infrastructure to those new bindings, implement
backward-compatibility through live DT patching.

Patching is disabled and will be enabled along with support for the new
DT bindings in the DU driver.

Signed-off-by: Laurent Pinchart 
---
Changes since v1:

- Select OF_FLATTREE
- Compile LVDS DT bindings patch code when DRM_RCAR_LVDS is selected
- Update the SPDX headers to use GPL-2.0 instead of GPL-2.0-only
- Turn __dtb_rcar_du_of_lvds_(begin|end) from u8 to char
- Pass void begin and end pointers to rcar_du_of_get_overlay()
- Use of_get_parent() instead of accessing the parent pointer directly
- Find the LVDS endpoints nodes based on the LVDS node instead of the
   root of the overlay
- Update to the -lvds compatible string format
---
  drivers/gpu/drm/rcar-du/Kconfig |   2 +
  drivers/gpu/drm/rcar-du/Makefile|   3 +-
  drivers/gpu/drm/rcar-du/rcar_du_of.c| 451 
  drivers/gpu/drm/rcar-du/rcar_du_of.h|  16 +
  drivers/gpu/drm/rcar-du/rcar_du_of_lvds.dts |  82 +
  5 files changed, 553 insertions(+), 1 deletion(-)
  create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of.c
  create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of.h
  create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of_lvds.dts

diff --git a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig
index 5d0b4b7119af..3f83352a7313 100644
--- a/drivers/gpu/drm/rcar-du/Kconfig
+++ b/drivers/gpu/drm/rcar-du/Kconfig
@@ -22,6 +22,8 @@ config DRM_RCAR_LVDS
bool "R-Car DU LVDS Encoder Support"
depends on DRM_RCAR_DU
select DRM_PANEL
+   select OF_FLATTREE
+   select OF_OVERLAY
help
  Enable support for the R-Car Display Unit embedded LVDS encoders.
  
diff --git a/drivers/gpu/drm/rcar-du/Makefile b/drivers/gpu/drm/rcar-du/Makefile

index 0cf5c11030e8..8ed569a0f462 100644
--- a/drivers/gpu/drm/rcar-du/Makefile
+++ b/drivers/gpu/drm/rcar-du/Makefile
@@ -5,10 +5,11 @@ rcar-du-drm-y := rcar_du_crtc.o \
 rcar_du_group.o \
 rcar_du_kms.o \
 rcar_du_lvdscon.o \
+rcar_du_of.o \
 rcar_du_plane.o
  
  rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)	+= rcar_du_lvdsenc.o

-
+rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)+= rcar_du_of_lvds.dtb.o
  rcar-du-drm-$(CONFIG_DRM_RCAR_VSP)+= rcar_du_vsp.o
  
  obj-$(CONFIG_DRM_RCAR_DU)		+= rcar-du-drm.o

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_of.c 
b/drivers/gpu/drm/rcar-du/rcar_du_of.c
new file mode 100644
index ..2bf91201fc93
--- /dev/null
+++ b/drivers/gpu/drm/rcar-du/rcar_du_of.c
@@ -0,0 +1,451 @@

[...]

+static struct device_node __init *
+static int __init rcar_du_of_add_property(struct device_node *np,
+ const char *name, const void *value,
+ size_t length)
+{
+   struct property *prop;
+
+   prop = kzalloc(sizeof(*prop), GFP_KERNEL);
+   if (!prop)
+   return -ENOMEM;
+
+   prop->name = kstrdup(name, GFP_KERNEL);
+   prop->value = kmemdup(value, length, GFP_KERNEL);
+   prop->length = length;
+
+   if (!prop->name || !prop->value) {
+   kfree(prop->name);
+   kfree(prop->value);
+   kfree(prop);
+   return -ENOMEM;
+   }
+
+   of_property_set_flag(prop, OF_DYNAMIC);
+
+   prop->next = np->properties;
+   np->properties = prop;
+
+   return 0;
+}
+
+/* Free properties allocated dynamically by rcar_du_of_add_property(). */
+static void __init rcar_du_of_free_properties(struct device_node *np)
+{
+   while (np->properties) {
+   struct property *prop = np->properties;
+
+   np->properties = prop->next;
+
+   if (OF_IS_DYNAMIC(prop)) {
+   kfree(prop->name);
+   kfree(prop->value);
+   kfree(prop);


   Perhaps these kfree() worth factoring out?


+   }
+   }
+}
+

[...]

+extern char __dtb_rcar_du_of_lvds_begin[];
+extern char __dtb_rcar_du_of_lvds_end[];
+
+static void __init rcar_du_of_lvds_patch_one(struct device_node *du,
+unsigned int port_id,
+const struct resource *res,
+const __be32 *reg,
+const struct of_phandle_args 
*clkspec,
+struct device_node *local,
+struct device_node *remote)
+{

[...]

+   /* Apply the overlay, this will resolve phandles. */
+   ovcs_id = 0;
+   ret = of_overlay_apply(overlay.np, 

Re: [PATCH v2 03/12] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes

[Rob Herring]

On Tue, Jan 16, 2018 at 10:32 AM, Laurent Pinchart
 wrote:
> Hi Rob,
>
> On Tuesday, 16 January 2018 16:35:26 EET Rob Herring wrote:
>> On Mon, Jan 15, 2018 at 5:46 PM, Frank Rowand wrote:
>> > On 01/15/18 12:29, Laurent Pinchart wrote:
>> >> On Monday, 15 January 2018 22:12:33 EET Frank Rowand wrote:
>> >>> On 01/15/18 11:22, Laurent Pinchart wrote:
>>  On Monday, 15 January 2018 21:12:44 EET Frank Rowand wrote:
>> > On 01/15/18 09:09, Rob Herring wrote:
>> >> On Fri, Jan 12, 2018 at 5:14 PM, Laurent Pinchart wrote:
>> >>> The internal LVDS encoders now have their own DT bindings. Before
>> >>> switching the driver infrastructure to those new bindings, implement
>> >>> backward-compatibility through live DT patching.

[...]

>> >> How would you like to proceed ? I can try the manual approach again but
>> >> need information about how I could cleanly implement phandle allocation.
>> >> I will likely then run into other issues for which I might need help.
>> >
>> > Here are my first thoughts, based on 4.15-rc7:
>> >
>> > Question to Rob and Frank: should of_attach_node() be called directly, or
>> > should it be called indirectly by creating a change set that adds the
>> > node?
>> >
>> > Frank's off the cuff answer (but would like to think more about it): since
>> > the driver is modifying its own devicetree data, and thus no other entity
>> > needs to be notified about the changes, there is no need to add the
>> > complexity of using a change set.
>>
>> There's exactly 2 users outside of the DT core. That's generally a
>> sign of an API I'd like to make private.
>>
>> > The following is how of_attach_node() could be modified to dynamically
>> > create a phandle on request.
>>
>> How would this work for all the phandle references that need to be fixed up?
>
> As I know which properties contain a phandle that needs to be fixed up, my
> plan is to update those properties manually with the value of the newly
> allocated phandle.

That sounds like more low level, internal detail mucking with than
this current patch.

> What I need here is the ability to insert the following structure in the
> device tree:
>
> lvds@feb9 {
>compatible = "renesas,r8a7796-lvds"; (*)
>reg = <0 0xfeb9 0 0x14>; (*)
>clocks = < CPG_MOD 727>; (*)

I'm still of the opinion that all of this should be in a per SoC overlay.

>
>ports {
> #address-cells = <1>;
> #size-cells = <0>;
>
> port@0 {
> reg = <0>;
> lvds0_in: endpoint {
> remote-endpoint = <_out_lvds0>; (*)
> };
> };
> port@1 {
> reg = <1>;
> lvds0_out: endpoint {
> remote-endpoint = <_in>; (*)

Then do the fixup of just the remote-endpoint properties. While it
would be nice to say overlays are completely static, I'm guessing
that's not going to be the case. After all, we pretty much have
overlays because DT being static has limitations.

> };
> };
> };
> };
>
> with the node unit address and all properties marked with a (*) computed at
> runtime based on information extract from the device tree. Additionally I need
> phandles for the lvds0_in and lvds0_out nodes, and set the value of two
> properties in the tree with those phandles.
>
> I've used overlays as that was the only way I found to allocate phandles, but
> any other API will work for me as well.

I don't think drivers mucking with phandles directly to avoid another
overlay user is an improvement.

Rob

Re: [PATCH v2 03/12] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes

[Laurent Pinchart]

Hi Rob,

On Tuesday, 16 January 2018 16:35:26 EET Rob Herring wrote:
> On Mon, Jan 15, 2018 at 5:46 PM, Frank Rowand wrote:
> > On 01/15/18 12:29, Laurent Pinchart wrote:
> >> On Monday, 15 January 2018 22:12:33 EET Frank Rowand wrote:
> >>> On 01/15/18 11:22, Laurent Pinchart wrote:
>  On Monday, 15 January 2018 21:12:44 EET Frank Rowand wrote:
> > On 01/15/18 09:09, Rob Herring wrote:
> >> On Fri, Jan 12, 2018 at 5:14 PM, Laurent Pinchart wrote:
> >>> The internal LVDS encoders now have their own DT bindings. Before
> >>> switching the driver infrastructure to those new bindings, implement
> >>> backward-compatibility through live DT patching.
> >> 
> >> Uhh, we just got rid of TI's patching and now adding this one. I
> >> guess
> >>> 
> >>> Let me first answer the question that you ask later.  You ask "Can we
> >>> work on this together to find a solution that would suit us both ?"
> >>> 
> >>> My answer to that is emphatically YES.  I will definitely work with you
> >>> to try to find a good solution.
> >> 
> >> \o/
> >> 
> > Please no.  What we just got rid of was making it difficult for me to
> > make changes to the overlay infrastructure.  There are issues with
> > overlays that need to be fixed before overlays become really usable.
> > I am about to propose the next change, which involves removing a
> > chunk of code that I will not be able to remove if this patch is
> > accepted (the proposal is awaiting me collecting some data about
> > the impact of the change, which I expect to complete this week).
> >>> 
> >>> I should have thought just a little bit more before I hit send.  The
> >>> situation is even worse than I wrote.  One of the next steps (in
> >>> addition to what I wrote about above) is to change the overlay apply
> >>> function to accept a flattened device tree (FDT), not an expanded
> >>> device tree.  In this changed model, the unflattened overlay is
> >>> not available to be modified before it is applied.
> >> 
> >> That makes sense if we consider overlays to be immutable objects that we
> >> apply without offering a way to modify them. I won't challenge that API
> >> decision, as my use of an overlay here is a bit of a hack indeed.
> >> 
> >>> It is important for the devicetree infrastructure to have ownership
> >>> of the FDT that is used to create the unflattened tree.  (Notice
> >>> that in the proposed patch, rcar_du_of_get_overlay() follows the
> >>> TI example of doing a kmemdup of the blob (FDT), then uses the
> >>> copy for the unflatten.  The kmemdup() in this case is to create
> >>> a persistent copy of the FDT.)  The driver having ownership of
> >>> this copy, and having the ability to free it is one of the many
> >>> problems with the current overlay implementation.
> >> 
> >> Yes, that's something I've identified as well. Lots of work has been done
> >> to clean up the OF core and we're clearly not done yet. I'm happy to see
> >> all the improvements you're working on.
> >> 
> > Can you please handle both the old and new bindings through driver
> > code instead?
>  
>  I could, but it would be pointless. The point here is to allow cleanups
>  in the driver. The LVDS encoder handling code is very intrusive in its
>  current form and I need to get rid of it. There would be zero point in
>  moving to the new infrastructure, as the main point is to get rid of
>  the old code which prevents moving forward. As a consequence that would
>  block new boards from receiving proper upstream support. An easy option
>  is to break backward compatibility. I'm personally fine with that, but
>  I assume other people would complain :-)
>  
>  I can, on the other hand, work with you to see how live DT patching
>  could be implemented in this driver without blocking your code. When
>  developing this patch series I start by patching the device tree
>  manually without relying on overlays at all, but got blocked by the
>  fact that I need to allocate phandles for new nodes I create. If there
>  was an API to allocate an unused phandle I could avoid using the
>  overlay infrastructure at all. Or there could be other
> >>> 
> >>> It seems reasonable to provide a way to autogenerate a phandle (if
> >>> requested) by the devicetree code that creates a new node.  Were you
> >>> using a function from drivers/of/dynamic.c to create the node?
> >> 
> >> Not to allocate the node, no. I allocated the device_node structure
> >> manually with kzalloc(), and inserted it in the device tree with
> >> of_attach_node(). Is that the right approach ? I haven't been able to
> >> test the code as I stopped when I realized I couldn't allocate phandles.
> >> 
>  options I'm not thinking of as I don't know what the changes you're
>  working on are. Can we work on this together to find a solution that
>  would suit us both ?
> >>> 
> >>> Again, yes, I would be 

Re: [PATCH v2 03/12] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes

[Rob Herring]

On Tue, Jan 16, 2018 at 2:56 AM, Geert Uytterhoeven
 wrote:
> Hi Laurent,
>
> On Mon, Jan 15, 2018 at 7:01 PM, Laurent Pinchart
>  wrote:
>> On Monday, 15 January 2018 19:09:53 EET Rob Herring wrote:
>>> On Fri, Jan 12, 2018 at 5:14 PM, Laurent Pinchart wrote:
>>> > The internal LVDS encoders now have their own DT bindings. Before
>>> > switching the driver infrastructure to those new bindings, implement
>>> > backward-compatibility through live DT patching.
>>>
>>> Uhh, we just got rid of TI's patching and now adding this one. I guess
>>> it's necessary, but I'd like to know how long we need to keep this?
>>
>> That's a good question. How long are we supposed to keep DT backward
>> compatibility for ? I don't think there's a one-size-fits-them-all answer to
>> this question. Geert, any opinion ? How long do you plan to keep the CPG
>> (clocks) DT backward compatibility for instance ?
>
> Good question indeed...
>
> It's not just CPG/MSSR. Over the years we also added or changed APMU (SMP),
> SYSC (power domains), RST (mode pins), CMT (timers), ..., all of which have
> some sort of compatibility support in the kernel.
>
> Hence to avoid having to remember the kernel versions that dropped backwards
> compatibility for each of the above components, I was thinking about an
> R-Car Gen2 DT Flag Day.

There's probably also DT changes that enable new features folks would
want/need? Or maybe carrying for some number of LTS releases is
enough.

> However, that was before I learned about your plans for LVDS (it seems every
> kernel release we learn about something new, postponing the flag day ;-).
> And now I'm quite sure we'll have another change in the future (DU per
> channel device nodes)...
>
> About using DT fixups to implement backwards compatibility: I did my share
> of thinking and experimenting with DT fixups (see e.g. "[PATCH/RFC 0/1]
> soc: renesas: Add DT fixup code for backwards compatibility"
> (https://www.spinics.net/lists/linux-renesas-soc/msg04305.html).
> DT fixups are hard to implement right, and not everything can be done
> that way.  Hence in the end, none of this was ever used upstream, and
> everything was handled in C...

In an ideal world, we would have some tool:

dt-diff-to-overlay old.dts new.dts > my-fixup-overlay.dts

And then in the kernel have infrastructure such you just declare match
tables with overlays to apply:

struct of_device_id dt_match[] = {
  {
.compatible = "vendor,board",
  },
  {},
};
DT_FIXUP(dt_match, my-fixup-overlay.dtbo);

But maybe I'm dreaming...

> So I'm wondering if it would be easier if you would implement backwards
> compatibility in C, using different compatible values for the new bindings?
> I.e. switch from "renesas,du-r8a77*" to "renesas,r8a77*-du" +
> "renesas,r8a77*-lvds"?
> That way it also becomes very clear that there are old and new bindings,
> and that there is backwards compatibility code for the old binding.
>
> I know you're aware (the rest of the audience may not be) that the LVDS
> part is not the only separate hardware block current unified in the single
> DU node: each DU channel has its own hardware block.  Perhaps you can also
> bite the bullet and have a single device node per DT channel, allowing
> Runtime PM for DU channels?
> Of course you have to tie channels together using a "companion" (cfr. USB)
> or "renesas,bonding" (cfr. DRIF) property (unfortunately I learned about
> the former only after the latter was already established).
>
> Finally, implementing backwards compatibility support by DT fixup using
> overlays may complicate backporting to LTSI kernels, due to the dependency
> on DT overlays, which were reworked lately.
>
>>> > --- a/drivers/gpu/drm/rcar-du/Kconfig
>>> > +++ b/drivers/gpu/drm/rcar-du/Kconfig
>>> > @@ -22,6 +22,8 @@ config DRM_RCAR_LVDS
>>> > bool "R-Car DU LVDS Encoder Support"
>>> > depends on DRM_RCAR_DU
>>> > select DRM_PANEL
>>> > +   select OF_FLATTREE
>>> > +   select OF_OVERLAY
>>>
>>> OF_OVERLAY should probably select OF_FLATTREE. I guess in theory, we
>>> could have an overlay from a non-FDT source...
>
> Currently the select is needed for of_fdt_unflatten_tree() only, which is
> not used by the core OF_OVERLAY code. So you could build an overlay in
> memory yourself, and pass that, without using of_fdt_unflatten_tree().
> But that is going to change if I read Frank's reponse well?

Yes, it's currently a 4 or so step process that we plan to make a single call.

Rob

Re: [PATCH v2 03/12] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes

[Rob Herring]

On Mon, Jan 15, 2018 at 5:46 PM, Frank Rowand  wrote:
> On 01/15/18 12:29, Laurent Pinchart wrote:
>> Hi Frank,
>>
>> On Monday, 15 January 2018 22:12:33 EET Frank Rowand wrote:
>>> On 01/15/18 11:22, Laurent Pinchart wrote:
 On Monday, 15 January 2018 21:12:44 EET Frank Rowand wrote:
> On 01/15/18 09:09, Rob Herring wrote:
>> +Frank
>>
>> On Fri, Jan 12, 2018 at 5:14 PM, Laurent Pinchart wrote:
>>> The internal LVDS encoders now have their own DT bindings. Before
>>> switching the driver infrastructure to those new bindings, implement
>>> backward-compatibility through live DT patching.
>>
>> Uhh, we just got rid of TI's patching and now adding this one. I guess
>>>
>>> Let me first answer the question that you ask later.  You ask "Can we work
>>> on this together to find a solution that would suit us both ?"
>>>
>>> My answer to that is emphatically YES.  I will definitely work with you to
>>> try to find a good solution.
>>
>> \o/
>>
> Please no.  What we just got rid of was making it difficult for me to
> make changes to the overlay infrastructure.  There are issues with
> overlays that need to be fixed before overlays become really usable.
> I am about to propose the next change, which involves removing a
> chunk of code that I will not be able to remove if this patch is
> accepted (the proposal is awaiting me collecting some data about
> the impact of the change, which I expect to complete this week).
>>>
>>> I should have thought just a little bit more before I hit send.  The
>>> situation is even worse than I wrote.  One of the next steps (in
>>> addition to what I wrote about above) is to change the overlay apply
>>> function to accept a flattened device tree (FDT), not an expanded
>>> device tree.  In this changed model, the unflattened overlay is
>>> not available to be modified before it is applied.
>>
>> That makes sense if we consider overlays to be immutable objects that we 
>> apply
>> without offering a way to modify them. I won't challenge that API decision, 
>> as
>> my use of an overlay here is a bit of a hack indeed.
>>
>>> It is important for the devicetree infrastructure to have ownership
>>> of the FDT that is used to create the unflattened tree.  (Notice
>>> that in the proposed patch, rcar_du_of_get_overlay() follows the
>>> TI example of doing a kmemdup of the blob (FDT), then uses the
>>> copy for the unflatten.  The kmemdup() in this case is to create
>>> a persistent copy of the FDT.)  The driver having ownership of
>>> this copy, and having the ability to free it is one of the many
>>> problems with the current overlay implementation.
>>
>> Yes, that's something I've identified as well. Lots of work has been done to
>> clean up the OF core and we're clearly not done yet. I'm happy to see all the
>> improvements you're working on.
>>
> Can you please handle both the old and new bindings through driver
> code instead?

 I could, but it would be pointless. The point here is to allow cleanups in
 the driver. The LVDS encoder handling code is very intrusive in its
 current form and I need to get rid of it. There would be zero point in
 moving to the new infrastructure, as the main point is to get rid of the
 old code which prevents moving forward. As a consequence that would block
 new boards from receiving proper upstream support. An easy option is to
 break backward compatibility. I'm personally fine with that, but I assume
 other people would complain :-)

 I can, on the other hand, work with you to see how live DT patching could
 be implemented in this driver without blocking your code. When developing
 this patch series I start by patching the device tree manually without
 relying on overlays at all, but got blocked by the fact that I need to
 allocate phandles for new nodes I create. If there was an API to allocate
 an unused phandle I could avoid using the overlay infrastructure at all.
 Or there could be other
>>>
>>> It seems reasonable to provide a way to autogenerate a phandle (if
>>> requested) by the devicetree code that creates a new node.  Were you using
>>> a function from drivers/of/dynamic.c to create the node?
>>
>> Not to allocate the node, no. I allocated the device_node structure manually
>> with kzalloc(), and inserted it in the device tree with of_attach_node(). Is
>> that the right approach ? I haven't been able to test the code as I stopped
>> when I realized I couldn't allocate phandles.
>>
 options I'm not thinking of as I don't know what the changes you're
 working on are. Can we work on this together to find a solution that
 would suit us both ?
>>>
>>> Again, yes, I would be glad to work with you on this.
>>
>> How would you like to proceed ? I can try the manual approach again but need
>> information about how I could cleanly implement phandle allocation. I will
>> likely 

Re: [PATCH v2 03/12] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes

[Laurent Pinchart]

Hi Geert,

On Tuesday, 16 January 2018 10:56:10 EET Geert Uytterhoeven wrote:
> On Mon, Jan 15, 2018 at 7:01 PM, Laurent Pinchart wrote:
> > On Monday, 15 January 2018 19:09:53 EET Rob Herring wrote:
> >> On Fri, Jan 12, 2018 at 5:14 PM, Laurent Pinchart wrote:
> >>> The internal LVDS encoders now have their own DT bindings. Before
> >>> switching the driver infrastructure to those new bindings, implement
> >>> backward-compatibility through live DT patching.
> >> 
> >> Uhh, we just got rid of TI's patching and now adding this one. I guess
> >> it's necessary, but I'd like to know how long we need to keep this?
> > 
> > That's a good question. How long are we supposed to keep DT backward
> > compatibility for ? I don't think there's a one-size-fits-them-all answer
> > to this question. Geert, any opinion ? How long do you plan to keep the
> > CPG (clocks) DT backward compatibility for instance ?
> 
> Good question indeed...
> 
> It's not just CPG/MSSR. Over the years we also added or changed APMU (SMP),
> SYSC (power domains), RST (mode pins), CMT (timers), ..., all of which have
> some sort of compatibility support in the kernel.
> 
> Hence to avoid having to remember the kernel versions that dropped backwards
> compatibility for each of the above components, I was thinking about an
> R-Car Gen2 DT Flag Day.
> 
> However, that was before I learned about your plans for LVDS (it seems every
> kernel release we learn about something new, postponing the flag day ;-).
> And now I'm quite sure we'll have another change in the future (DU per
> channel device nodes)...

I don't think the DU and LVDS rework should postpone your flag day for all the 
core components.

> About using DT fixups to implement backwards compatibility: I did my share
> of thinking and experimenting with DT fixups (see e.g. "[PATCH/RFC 0/1]
> soc: renesas: Add DT fixup code for backwards compatibility"
> (https://www.spinics.net/lists/linux-renesas-soc/msg04305.html).
> DT fixups are hard to implement right, and not everything can be done
> that way.  Hence in the end, none of this was ever used upstream, and
> everything was handled in C...
> 
> So I'm wondering if it would be easier if you would implement backwards
> compatibility in C, using different compatible values for the new bindings?
> I.e. switch from "renesas,du-r8a77*" to "renesas,r8a77*-du" +
> "renesas,r8a77*-lvds"?
> That way it also becomes very clear that there are old and new bindings,
> and that there is backwards compatibility code for the old binding.

Quoting my reply to Frank,

I could, but it would be pointless. The point here is to allow cleanups in the 
driver. The LVDS encoder handling code is very intrusive in its current form 
and I need to get rid of it. There would be zero point in moving to the new 
infrastructure, as the main point is to get rid of the old code which prevents 
moving forward. As a consequence that would block new boards from receiving 
proper upstream support. An easy option is to break backward compatibility. 
I'm personally fine with that, but I assume other people would complain :-)

> I know you're aware (the rest of the audience may not be) that the LVDS
> part is not the only separate hardware block current unified in the single
> DU node: each DU channel has its own hardware block.  Perhaps you can also
> bite the bullet and have a single device node per DT channel, allowing
> Runtime PM for DU channels?

That's more difficult as the channels have cross-dependencies. I might give it 
a try at some point, or I might not. In any case it's a separate piece of 
work, and backward compatibility for that one might be handled in the driver 
instead of through DT patching.

> Of course you have to tie channels together using a "companion" (cfr. USB)
> or "renesas,bonding" (cfr. DRIF) property (unfortunately I learned about
> the former only after the latter was already established).
> 
> Finally, implementing backwards compatibility support by DT fixup using
> overlays may complicate backporting to LTSI kernels, due to the dependency
> on DT overlays, which were reworked lately.

I can drop backward compatibility completely if you prefer, that would be much 
easier to backport ;-)

As discussed with Frank I will likely try to patch the DT live without using 
overlays, but that will likely also be annoying to backport as the ongoing 
modifications to the OF core are not limited to overlays anyway.

> >>> --- a/drivers/gpu/drm/rcar-du/Kconfig
> >>> +++ b/drivers/gpu/drm/rcar-du/Kconfig
> >>> @@ -22,6 +22,8 @@ config DRM_RCAR_LVDS
> >>> bool "R-Car DU LVDS Encoder Support"
> >>> depends on DRM_RCAR_DU
> >>> select DRM_PANEL
> >>> +   select OF_FLATTREE
> >>> +   select OF_OVERLAY
> >> 
> >> OF_OVERLAY should probably select OF_FLATTREE. I guess in theory, we
> >> could have an overlay from a non-FDT source...
> 
> Currently the select is needed for of_fdt_unflatten_tree() only, which is
> not used by the core 

Re: [PATCH v2 03/12] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes

[Geert Uytterhoeven]

Hi Laurent,

On Mon, Jan 15, 2018 at 7:01 PM, Laurent Pinchart
 wrote:
> On Monday, 15 January 2018 19:09:53 EET Rob Herring wrote:
>> On Fri, Jan 12, 2018 at 5:14 PM, Laurent Pinchart wrote:
>> > The internal LVDS encoders now have their own DT bindings. Before
>> > switching the driver infrastructure to those new bindings, implement
>> > backward-compatibility through live DT patching.
>>
>> Uhh, we just got rid of TI's patching and now adding this one. I guess
>> it's necessary, but I'd like to know how long we need to keep this?
>
> That's a good question. How long are we supposed to keep DT backward
> compatibility for ? I don't think there's a one-size-fits-them-all answer to
> this question. Geert, any opinion ? How long do you plan to keep the CPG
> (clocks) DT backward compatibility for instance ?

Good question indeed...

It's not just CPG/MSSR. Over the years we also added or changed APMU (SMP),
SYSC (power domains), RST (mode pins), CMT (timers), ..., all of which have
some sort of compatibility support in the kernel.

Hence to avoid having to remember the kernel versions that dropped backwards
compatibility for each of the above components, I was thinking about an
R-Car Gen2 DT Flag Day.

However, that was before I learned about your plans for LVDS (it seems every
kernel release we learn about something new, postponing the flag day ;-).
And now I'm quite sure we'll have another change in the future (DU per
channel device nodes)...

About using DT fixups to implement backwards compatibility: I did my share
of thinking and experimenting with DT fixups (see e.g. "[PATCH/RFC 0/1]
soc: renesas: Add DT fixup code for backwards compatibility"
(https://www.spinics.net/lists/linux-renesas-soc/msg04305.html).
DT fixups are hard to implement right, and not everything can be done
that way.  Hence in the end, none of this was ever used upstream, and
everything was handled in C...

So I'm wondering if it would be easier if you would implement backwards
compatibility in C, using different compatible values for the new bindings?
I.e. switch from "renesas,du-r8a77*" to "renesas,r8a77*-du" +
"renesas,r8a77*-lvds"?
That way it also becomes very clear that there are old and new bindings,
and that there is backwards compatibility code for the old binding.

I know you're aware (the rest of the audience may not be) that the LVDS
part is not the only separate hardware block current unified in the single
DU node: each DU channel has its own hardware block.  Perhaps you can also
bite the bullet and have a single device node per DT channel, allowing
Runtime PM for DU channels?
Of course you have to tie channels together using a "companion" (cfr. USB)
or "renesas,bonding" (cfr. DRIF) property (unfortunately I learned about
the former only after the latter was already established).

Finally, implementing backwards compatibility support by DT fixup using
overlays may complicate backporting to LTSI kernels, due to the dependency
on DT overlays, which were reworked lately.

>> > --- a/drivers/gpu/drm/rcar-du/Kconfig
>> > +++ b/drivers/gpu/drm/rcar-du/Kconfig
>> > @@ -22,6 +22,8 @@ config DRM_RCAR_LVDS
>> > bool "R-Car DU LVDS Encoder Support"
>> > depends on DRM_RCAR_DU
>> > select DRM_PANEL
>> > +   select OF_FLATTREE
>> > +   select OF_OVERLAY
>>
>> OF_OVERLAY should probably select OF_FLATTREE. I guess in theory, we
>> could have an overlay from a non-FDT source...

Currently the select is needed for of_fdt_unflatten_tree() only, which is
not used by the core OF_OVERLAY code. So you could build an overlay in
memory yourself, and pass that, without using of_fdt_unflatten_tree().
But that is going to change if I read Frank's reponse well?

Gr{oetje,eeting}s,

Geert

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

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

Re: [PATCH v2 03/12] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes

[Frank Rowand]

On 01/15/18 15:46, Frank Rowand wrote:
> On 01/15/18 12:29, Laurent Pinchart wrote:
>> Hi Frank,
>>
>> On Monday, 15 January 2018 22:12:33 EET Frank Rowand wrote:
>>> On 01/15/18 11:22, Laurent Pinchart wrote:
 On Monday, 15 January 2018 21:12:44 EET Frank Rowand wrote:
> On 01/15/18 09:09, Rob Herring wrote:
>> +Frank
>>
>> On Fri, Jan 12, 2018 at 5:14 PM, Laurent Pinchart wrote:
>>> The internal LVDS encoders now have their own DT bindings. Before
>>> switching the driver infrastructure to those new bindings, implement
>>> backward-compatibility through live DT patching.
>>
>> Uhh, we just got rid of TI's patching and now adding this one. I guess
>>>
>>> Let me first answer the question that you ask later.  You ask "Can we work
>>> on this together to find a solution that would suit us both ?"
>>>
>>> My answer to that is emphatically YES.  I will definitely work with you to
>>> try to find a good solution.
>>
>> \o/
>>
> Please no.  What we just got rid of was making it difficult for me to
> make changes to the overlay infrastructure.  There are issues with
> overlays that need to be fixed before overlays become really usable.
> I am about to propose the next change, which involves removing a
> chunk of code that I will not be able to remove if this patch is
> accepted (the proposal is awaiting me collecting some data about
> the impact of the change, which I expect to complete this week).
>>>
>>> I should have thought just a little bit more before I hit send.  The
>>> situation is even worse than I wrote.  One of the next steps (in
>>> addition to what I wrote about above) is to change the overlay apply
>>> function to accept a flattened device tree (FDT), not an expanded
>>> device tree.  In this changed model, the unflattened overlay is
>>> not available to be modified before it is applied.
>>
>> That makes sense if we consider overlays to be immutable objects that we 
>> apply 
>> without offering a way to modify them. I won't challenge that API decision, 
>> as 
>> my use of an overlay here is a bit of a hack indeed.
>>
>>> It is important for the devicetree infrastructure to have ownership
>>> of the FDT that is used to create the unflattened tree.  (Notice
>>> that in the proposed patch, rcar_du_of_get_overlay() follows the
>>> TI example of doing a kmemdup of the blob (FDT), then uses the
>>> copy for the unflatten.  The kmemdup() in this case is to create
>>> a persistent copy of the FDT.)  The driver having ownership of
>>> this copy, and having the ability to free it is one of the many
>>> problems with the current overlay implementation.
>>
>> Yes, that's something I've identified as well. Lots of work has been done to 
>> clean up the OF core and we're clearly not done yet. I'm happy to see all 
>> the 
>> improvements you're working on.
>>
> Can you please handle both the old and new bindings through driver
> code instead?

 I could, but it would be pointless. The point here is to allow cleanups in
 the driver. The LVDS encoder handling code is very intrusive in its
 current form and I need to get rid of it. There would be zero point in
 moving to the new infrastructure, as the main point is to get rid of the
 old code which prevents moving forward. As a consequence that would block
 new boards from receiving proper upstream support. An easy option is to
 break backward compatibility. I'm personally fine with that, but I assume
 other people would complain :-)

 I can, on the other hand, work with you to see how live DT patching could
 be implemented in this driver without blocking your code. When developing
 this patch series I start by patching the device tree manually without
 relying on overlays at all, but got blocked by the fact that I need to
 allocate phandles for new nodes I create. If there was an API to allocate
 an unused phandle I could avoid using the overlay infrastructure at all.
 Or there could be other
>>>
>>> It seems reasonable to provide a way to autogenerate a phandle (if
>>> requested) by the devicetree code that creates a new node.  Were you using
>>> a function from drivers/of/dynamic.c to create the node?
>>
>> Not to allocate the node, no. I allocated the device_node structure manually 
>> with kzalloc(), and inserted it in the device tree with of_attach_node(). Is 
>> that the right approach ? I haven't been able to test the code as I stopped 
>> when I realized I couldn't allocate phandles.
>>
 options I'm not thinking of as I don't know what the changes you're
 working on are. Can we work on this together to find a solution that
 would suit us both ?
>>>
>>> Again, yes, I would be glad to work with you on this.
>>
>> How would you like to proceed ? I can try the manual approach again but need 
>> information about how I could cleanly implement phandle allocation. I will 
>> likely then run into other 

Re: [PATCH v2 03/12] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes

[Frank Rowand]

On 01/15/18 12:29, Laurent Pinchart wrote:
> Hi Frank,
> 
> On Monday, 15 January 2018 22:12:33 EET Frank Rowand wrote:
>> On 01/15/18 11:22, Laurent Pinchart wrote:
>>> On Monday, 15 January 2018 21:12:44 EET Frank Rowand wrote:
 On 01/15/18 09:09, Rob Herring wrote:
> +Frank
>
> On Fri, Jan 12, 2018 at 5:14 PM, Laurent Pinchart wrote:
>> The internal LVDS encoders now have their own DT bindings. Before
>> switching the driver infrastructure to those new bindings, implement
>> backward-compatibility through live DT patching.
>
> Uhh, we just got rid of TI's patching and now adding this one. I guess
>>
>> Let me first answer the question that you ask later.  You ask "Can we work
>> on this together to find a solution that would suit us both ?"
>>
>> My answer to that is emphatically YES.  I will definitely work with you to
>> try to find a good solution.
> 
> \o/
> 
 Please no.  What we just got rid of was making it difficult for me to
 make changes to the overlay infrastructure.  There are issues with
 overlays that need to be fixed before overlays become really usable.
 I am about to propose the next change, which involves removing a
 chunk of code that I will not be able to remove if this patch is
 accepted (the proposal is awaiting me collecting some data about
 the impact of the change, which I expect to complete this week).
>>
>> I should have thought just a little bit more before I hit send.  The
>> situation is even worse than I wrote.  One of the next steps (in
>> addition to what I wrote about above) is to change the overlay apply
>> function to accept a flattened device tree (FDT), not an expanded
>> device tree.  In this changed model, the unflattened overlay is
>> not available to be modified before it is applied.
> 
> That makes sense if we consider overlays to be immutable objects that we 
> apply 
> without offering a way to modify them. I won't challenge that API decision, 
> as 
> my use of an overlay here is a bit of a hack indeed.
> 
>> It is important for the devicetree infrastructure to have ownership
>> of the FDT that is used to create the unflattened tree.  (Notice
>> that in the proposed patch, rcar_du_of_get_overlay() follows the
>> TI example of doing a kmemdup of the blob (FDT), then uses the
>> copy for the unflatten.  The kmemdup() in this case is to create
>> a persistent copy of the FDT.)  The driver having ownership of
>> this copy, and having the ability to free it is one of the many
>> problems with the current overlay implementation.
> 
> Yes, that's something I've identified as well. Lots of work has been done to 
> clean up the OF core and we're clearly not done yet. I'm happy to see all the 
> improvements you're working on.
> 
 Can you please handle both the old and new bindings through driver
 code instead?
>>>
>>> I could, but it would be pointless. The point here is to allow cleanups in
>>> the driver. The LVDS encoder handling code is very intrusive in its
>>> current form and I need to get rid of it. There would be zero point in
>>> moving to the new infrastructure, as the main point is to get rid of the
>>> old code which prevents moving forward. As a consequence that would block
>>> new boards from receiving proper upstream support. An easy option is to
>>> break backward compatibility. I'm personally fine with that, but I assume
>>> other people would complain :-)
>>>
>>> I can, on the other hand, work with you to see how live DT patching could
>>> be implemented in this driver without blocking your code. When developing
>>> this patch series I start by patching the device tree manually without
>>> relying on overlays at all, but got blocked by the fact that I need to
>>> allocate phandles for new nodes I create. If there was an API to allocate
>>> an unused phandle I could avoid using the overlay infrastructure at all.
>>> Or there could be other
>>
>> It seems reasonable to provide a way to autogenerate a phandle (if
>> requested) by the devicetree code that creates a new node.  Were you using
>> a function from drivers/of/dynamic.c to create the node?
> 
> Not to allocate the node, no. I allocated the device_node structure manually 
> with kzalloc(), and inserted it in the device tree with of_attach_node(). Is 
> that the right approach ? I haven't been able to test the code as I stopped 
> when I realized I couldn't allocate phandles.
> 
>>> options I'm not thinking of as I don't know what the changes you're
>>> working on are. Can we work on this together to find a solution that
>>> would suit us both ?
>>
>> Again, yes, I would be glad to work with you on this.
> 
> How would you like to proceed ? I can try the manual approach again but need 
> information about how I could cleanly implement phandle allocation. I will 
> likely then run into other issues for which I might need help.

Here are my first thoughts, based on 4.15-rc7:

Question to Rob and Frank: should of_attach_node() 

Re: [PATCH v2 03/12] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes

[Laurent Pinchart]

Hi Frank,

On Monday, 15 January 2018 22:12:33 EET Frank Rowand wrote:
> On 01/15/18 11:22, Laurent Pinchart wrote:
> > On Monday, 15 January 2018 21:12:44 EET Frank Rowand wrote:
> >> On 01/15/18 09:09, Rob Herring wrote:
> >>> +Frank
> >>> 
> >>> On Fri, Jan 12, 2018 at 5:14 PM, Laurent Pinchart wrote:
>  The internal LVDS encoders now have their own DT bindings. Before
>  switching the driver infrastructure to those new bindings, implement
>  backward-compatibility through live DT patching.
> >>> 
> >>> Uhh, we just got rid of TI's patching and now adding this one. I guess
> 
> Let me first answer the question that you ask later.  You ask "Can we work
> on this together to find a solution that would suit us both ?"
> 
> My answer to that is emphatically YES.  I will definitely work with you to
> try to find a good solution.

\o/

> >> Please no.  What we just got rid of was making it difficult for me to
> >> make changes to the overlay infrastructure.  There are issues with
> >> overlays that need to be fixed before overlays become really usable.
> >> I am about to propose the next change, which involves removing a
> >> chunk of code that I will not be able to remove if this patch is
> >> accepted (the proposal is awaiting me collecting some data about
> >> the impact of the change, which I expect to complete this week).
> 
> I should have thought just a little bit more before I hit send.  The
> situation is even worse than I wrote.  One of the next steps (in
> addition to what I wrote about above) is to change the overlay apply
> function to accept a flattened device tree (FDT), not an expanded
> device tree.  In this changed model, the unflattened overlay is
> not available to be modified before it is applied.

That makes sense if we consider overlays to be immutable objects that we apply 
without offering a way to modify them. I won't challenge that API decision, as 
my use of an overlay here is a bit of a hack indeed.

> It is important for the devicetree infrastructure to have ownership
> of the FDT that is used to create the unflattened tree.  (Notice
> that in the proposed patch, rcar_du_of_get_overlay() follows the
> TI example of doing a kmemdup of the blob (FDT), then uses the
> copy for the unflatten.  The kmemdup() in this case is to create
> a persistent copy of the FDT.)  The driver having ownership of
> this copy, and having the ability to free it is one of the many
> problems with the current overlay implementation.

Yes, that's something I've identified as well. Lots of work has been done to 
clean up the OF core and we're clearly not done yet. I'm happy to see all the 
improvements you're working on.

> >> Can you please handle both the old and new bindings through driver
> >> code instead?
> > 
> > I could, but it would be pointless. The point here is to allow cleanups in
> > the driver. The LVDS encoder handling code is very intrusive in its
> > current form and I need to get rid of it. There would be zero point in
> > moving to the new infrastructure, as the main point is to get rid of the
> > old code which prevents moving forward. As a consequence that would block
> > new boards from receiving proper upstream support. An easy option is to
> > break backward compatibility. I'm personally fine with that, but I assume
> > other people would complain :-)
> > 
> > I can, on the other hand, work with you to see how live DT patching could
> > be implemented in this driver without blocking your code. When developing
> > this patch series I start by patching the device tree manually without
> > relying on overlays at all, but got blocked by the fact that I need to
> > allocate phandles for new nodes I create. If there was an API to allocate
> > an unused phandle I could avoid using the overlay infrastructure at all.
> > Or there could be other
> 
> It seems reasonable to provide a way to autogenerate a phandle (if
> requested) by the devicetree code that creates a new node.  Were you using
> a function from drivers/of/dynamic.c to create the node?

Not to allocate the node, no. I allocated the device_node structure manually 
with kzalloc(), and inserted it in the device tree with of_attach_node(). Is 
that the right approach ? I haven't been able to test the code as I stopped 
when I realized I couldn't allocate phandles.

> > options I'm not thinking of as I don't know what the changes you're
> > working on are. Can we work on this together to find a solution that
> > would suit us both ?
> 
> Again, yes, I would be glad to work with you on this.

How would you like to proceed ? I can try the manual approach again but need 
information about how I could cleanly implement phandle allocation. I will 
likely then run into other issues for which I might need help.

> >>> it's necessary, but I'd like to know how long we need to keep this?
> >>> 
> >>> How many overlays would you need if everything is static (i.e.
> >>> removing all your fixup code)?
> >>> 
>  Patching is 

Re: [PATCH v2 03/12] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes

[Frank Rowand]

On 01/15/18 11:22, Laurent Pinchart wrote:
> Hi Frank,
> 
> On Monday, 15 January 2018 21:12:44 EET Frank Rowand wrote:
>> On 01/15/18 09:09, Rob Herring wrote:
>>> +Frank
>>>
>>> On Fri, Jan 12, 2018 at 5:14 PM, Laurent Pinchart wrote:
 The internal LVDS encoders now have their own DT bindings. Before
 switching the driver infrastructure to those new bindings, implement
 backward-compatibility through live DT patching.
>>>
>>> Uhh, we just got rid of TI's patching and now adding this one. I guess
>>

Let me first answer the question that you ask later.  You ask "Can we work on
this together to find a solution that would suit us both ?"

My answer to that is emphatically YES.  I will definitely work with you to
try to find a good solution.


>> Please no.  What we just got rid of was making it difficult for me to
>> make changes to the overlay infrastructure.  There are issues with
>> overlays that need to be fixed before overlays become really usable.
>> I am about to propose the next change, which involves removing a
>> chunk of code that I will not be able to remove if this patch is
>> accepted (the proposal is awaiting me collecting some data about
>> the impact of the change, which I expect to complete this week).

I should have thought just a little bit more before I hit send.  The
situation is even worse than I wrote.  One of the next steps (in
addition to what I wrote about above) is to change the overlay apply
function to accept a flattened device tree (FDT), not an expanded
device tree.  In this changed model, the unflattened overlay is
not available to be modified before it is applied.

It is important for the devicetree infrastructure to have ownership
of the FDT that is used to create the unflattened tree.  (Notice
that in the proposed patch, rcar_du_of_get_overlay() follows the
TI example of doing a kmemdup of the blob (FDT), then uses the
copy for the unflatten.  The kmemdup() in this case is to create
a persistent copy of the FDT.)  The driver having ownership of
this copy, and having the ability to free it is one of the many
problems with the current overlay implementation.

>>
>> Can you please handle both the old and new bindings through driver
>> code instead?
> 
> I could, but it would be pointless. The point here is to allow cleanups in 
> the 
> driver. The LVDS encoder handling code is very intrusive in its current form 
> and I need to get rid of it. There would be zero point in moving to the new 
> infrastructure, as the main point is to get rid of the old code which 
> prevents 
> moving forward. As a consequence that would block new boards from receiving 
> proper upstream support. An easy option is to break backward compatibility. 
> I'm personally fine with that, but I assume other people would complain :-)
> 
> I can, on the other hand, work with you to see how live DT patching could be 
> implemented in this driver without blocking your code. When developing this 
> patch series I start by patching the device tree manually without relying on 
> overlays at all, but got blocked by the fact that I need to allocate phandles 
> for new nodes I create. If there was an API to allocate an unused phandle I 
> could avoid using the overlay infrastructure at all. Or there could be other 

It seems reasonable to provide a way to autogenerate a phandle (if requested)
by the devicetree code that creates a new node.  Were you using a function
from drivers/of/dynamic.c to create the node?


> options I'm not thinking of as I don't know what the changes you're working 
> on 
> are. Can we work on this together to find a solution that would suit us both ?

Again, yes, I would be glad to work with you on this.

-Frank

> 
>>> it's necessary, but I'd like to know how long we need to keep this?
>>>
>>> How many overlays would you need if everything is static (i.e.
>>> removing all your fixup code)?
>>>
 Patching is disabled and will be enabled along with support for the new
 DT bindings in the DU driver.

 Signed-off-by: Laurent Pinchart
 
 ---
 Changes since v1:

 - Select OF_FLATTREE
 - Compile LVDS DT bindings patch code when DRM_RCAR_LVDS is selected
 - Update the SPDX headers to use GPL-2.0 instead of GPL-2.0-only
 - Turn __dtb_rcar_du_of_lvds_(begin|end) from u8 to char
 - Pass void begin and end pointers to rcar_du_of_get_overlay()
 - Use of_get_parent() instead of accessing the parent pointer directly
 - Find the LVDS endpoints nodes based on the LVDS node instead of the
   root of the overlay
 - Update to the -lvds compatible string format
 ---

  drivers/gpu/drm/rcar-du/Kconfig |   2 +
  drivers/gpu/drm/rcar-du/Makefile|   3 +-
  drivers/gpu/drm/rcar-du/rcar_du_of.c| 451 ++
  drivers/gpu/drm/rcar-du/rcar_du_of.h|  16 +
  drivers/gpu/drm/rcar-du/rcar_du_of_lvds.dts |  82 +

Re: [PATCH v2 03/12] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes

[Laurent Pinchart]

Hi Frank,

On Monday, 15 January 2018 21:12:44 EET Frank Rowand wrote:
> On 01/15/18 09:09, Rob Herring wrote:
> > +Frank
> > 
> > On Fri, Jan 12, 2018 at 5:14 PM, Laurent Pinchart wrote:
> >> The internal LVDS encoders now have their own DT bindings. Before
> >> switching the driver infrastructure to those new bindings, implement
> >> backward-compatibility through live DT patching.
> > 
> > Uhh, we just got rid of TI's patching and now adding this one. I guess
> 
> Please no.  What we just got rid of was making it difficult for me to
> make changes to the overlay infrastructure.  There are issues with
> overlays that need to be fixed before overlays become really usable.
> I am about to propose the next change, which involves removing a
> chunk of code that I will not be able to remove if this patch is
> accepted (the proposal is awaiting me collecting some data about
> the impact of the change, which I expect to complete this week).
> 
> Can you please handle both the old and new bindings through driver
> code instead?

I could, but it would be pointless. The point here is to allow cleanups in the 
driver. The LVDS encoder handling code is very intrusive in its current form 
and I need to get rid of it. There would be zero point in moving to the new 
infrastructure, as the main point is to get rid of the old code which prevents 
moving forward. As a consequence that would block new boards from receiving 
proper upstream support. An easy option is to break backward compatibility. 
I'm personally fine with that, but I assume other people would complain :-)

I can, on the other hand, work with you to see how live DT patching could be 
implemented in this driver without blocking your code. When developing this 
patch series I start by patching the device tree manually without relying on 
overlays at all, but got blocked by the fact that I need to allocate phandles 
for new nodes I create. If there was an API to allocate an unused phandle I 
could avoid using the overlay infrastructure at all. Or there could be other 
options I'm not thinking of as I don't know what the changes you're working on 
are. Can we work on this together to find a solution that would suit us both ?

> > it's necessary, but I'd like to know how long we need to keep this?
> > 
> > How many overlays would you need if everything is static (i.e.
> > removing all your fixup code)?
> > 
> >> Patching is disabled and will be enabled along with support for the new
> >> DT bindings in the DU driver.
> >> 
> >> Signed-off-by: Laurent Pinchart
> >> 
> >> ---
> >> Changes since v1:
> >> 
> >> - Select OF_FLATTREE
> >> - Compile LVDS DT bindings patch code when DRM_RCAR_LVDS is selected
> >> - Update the SPDX headers to use GPL-2.0 instead of GPL-2.0-only
> >> - Turn __dtb_rcar_du_of_lvds_(begin|end) from u8 to char
> >> - Pass void begin and end pointers to rcar_du_of_get_overlay()
> >> - Use of_get_parent() instead of accessing the parent pointer directly
> >> - Find the LVDS endpoints nodes based on the LVDS node instead of the
> >>   root of the overlay
> >> - Update to the -lvds compatible string format
> >> ---
> >> 
> >>  drivers/gpu/drm/rcar-du/Kconfig |   2 +
> >>  drivers/gpu/drm/rcar-du/Makefile|   3 +-
> >>  drivers/gpu/drm/rcar-du/rcar_du_of.c| 451 ++
> >>  drivers/gpu/drm/rcar-du/rcar_du_of.h|  16 +
> >>  drivers/gpu/drm/rcar-du/rcar_du_of_lvds.dts |  82 +
> >>  5 files changed, 553 insertions(+), 1 deletion(-)
> >>  create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of.c
> >>  create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of.h
> >>  create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of_lvds.dts

[snip]

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 03/12] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes

[Frank Rowand]

On 01/15/18 09:09, Rob Herring wrote:
> +Frank
> 
> On Fri, Jan 12, 2018 at 5:14 PM, Laurent Pinchart
>  wrote:
>> The internal LVDS encoders now have their own DT bindings. Before
>> switching the driver infrastructure to those new bindings, implement
>> backward-compatibility through live DT patching.
> 
> Uhh, we just got rid of TI's patching and now adding this one. I guess

Please no.  What we just got rid of was making it difficult for me to
make changes to the overlay infrastructure.  There are issues with
overlays that need to be fixed before overlays become really usable.
I am about to propose the next change, which involves removing a
chunk of code that I will not be able to remove if this patch is
accepted (the proposal is awaiting me collecting some data about
the impact of the change, which I expect to complete this week).

Can you please handle both the old and new bindings through driver
code instead?

Thanks,

Frank


> it's necessary, but I'd like to know how long we need to keep this?
> 
> How many overlays would you need if everything is static (i.e.
> removing all your fixup code)?
> 
>> Patching is disabled and will be enabled along with support for the new
>> DT bindings in the DU driver.
>>
>> Signed-off-by: Laurent Pinchart 
>> ---
>> Changes since v1:
>>
>> - Select OF_FLATTREE
>> - Compile LVDS DT bindings patch code when DRM_RCAR_LVDS is selected
>> - Update the SPDX headers to use GPL-2.0 instead of GPL-2.0-only
>> - Turn __dtb_rcar_du_of_lvds_(begin|end) from u8 to char
>> - Pass void begin and end pointers to rcar_du_of_get_overlay()
>> - Use of_get_parent() instead of accessing the parent pointer directly
>> - Find the LVDS endpoints nodes based on the LVDS node instead of the
>>   root of the overlay
>> - Update to the -lvds compatible string format
>> ---
>>  drivers/gpu/drm/rcar-du/Kconfig |   2 +
>>  drivers/gpu/drm/rcar-du/Makefile|   3 +-
>>  drivers/gpu/drm/rcar-du/rcar_du_of.c| 451 
>> 
>>  drivers/gpu/drm/rcar-du/rcar_du_of.h|  16 +
>>  drivers/gpu/drm/rcar-du/rcar_du_of_lvds.dts |  82 +
>>  5 files changed, 553 insertions(+), 1 deletion(-)
>>  create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of.c
>>  create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of.h
>>  create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of_lvds.dts
>>
>> diff --git a/drivers/gpu/drm/rcar-du/Kconfig 
>> b/drivers/gpu/drm/rcar-du/Kconfig
>> index 5d0b4b7119af..3f83352a7313 100644
>> --- a/drivers/gpu/drm/rcar-du/Kconfig
>> +++ b/drivers/gpu/drm/rcar-du/Kconfig
>> @@ -22,6 +22,8 @@ config DRM_RCAR_LVDS
>> bool "R-Car DU LVDS Encoder Support"
>> depends on DRM_RCAR_DU
>> select DRM_PANEL
>> +   select OF_FLATTREE
>> +   select OF_OVERLAY
> 
> OF_OVERLAY should probably select OF_FLATTREE. I guess in theory, we
> could have an overlay from a non-FDT source...
> 
>> help
>>   Enable support for the R-Car Display Unit embedded LVDS encoders.
>>
>> diff --git a/drivers/gpu/drm/rcar-du/Makefile 
>> b/drivers/gpu/drm/rcar-du/Makefile
>> index 0cf5c11030e8..8ed569a0f462 100644
>> --- a/drivers/gpu/drm/rcar-du/Makefile
>> +++ b/drivers/gpu/drm/rcar-du/Makefile
>> @@ -5,10 +5,11 @@ rcar-du-drm-y := rcar_du_crtc.o \
>>  rcar_du_group.o \
>>  rcar_du_kms.o \
>>  rcar_du_lvdscon.o \
>> +rcar_du_of.o \
>>  rcar_du_plane.o
>>
>>  rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)+= rcar_du_lvdsenc.o
>> -
>> +rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)+= rcar_du_of_lvds.dtb.o
>>  rcar-du-drm-$(CONFIG_DRM_RCAR_VSP) += rcar_du_vsp.o
>>
>>  obj-$(CONFIG_DRM_RCAR_DU)  += rcar-du-drm.o
>> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_of.c 
>> b/drivers/gpu/drm/rcar-du/rcar_du_of.c
>> new file mode 100644
>> index ..2bf91201fc93
>> --- /dev/null
>> +++ b/drivers/gpu/drm/rcar-du/rcar_du_of.c
>> @@ -0,0 +1,451 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * rcar_du_of.c - Legacy DT bindings compatibility
>> + *
>> + * Copyright (C) 2018 Laurent Pinchart 
>> + *
>> + * Based on work from Jyri Sarha 
>> + * Copyright (C) 2015 Texas Instruments
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#include "rcar_du_crtc.h"
>> +#include "rcar_du_drv.h"
>> +
>> +#ifdef CONFIG_DRM_RCAR_LVDS
> 
> Why not make compiling this file conditional in the Makefile?
> 
>> +
>> +struct rcar_du_of_overlay {
>> +   struct device_node *np;
>> +   void *data;
>> +   void *mem;
>> +};
>> +
>> +static void __init rcar_du_of_free_overlay(struct rcar_du_of_overlay 
>> *overlay)
>> +{
>> +   of_node_put(overlay->np);
>> +   kfree(overlay->data);
>> +   kfree(overlay->mem);
>> +}
>> +
>> +static int 

Re: [PATCH v2 03/12] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes

[Laurent Pinchart]

Hi Rob,

(CC'ing Geert)

On Monday, 15 January 2018 19:09:53 EET Rob Herring wrote:
> +Frank
> 
> On Fri, Jan 12, 2018 at 5:14 PM, Laurent Pinchart wrote:
> > The internal LVDS encoders now have their own DT bindings. Before
> > switching the driver infrastructure to those new bindings, implement
> > backward-compatibility through live DT patching.
> 
> Uhh, we just got rid of TI's patching and now adding this one. I guess
> it's necessary, but I'd like to know how long we need to keep this?

That's a good question. How long are we supposed to keep DT backward 
compatibility for ? I don't think there's a one-size-fits-them-all answer to 
this question. Geert, any opinion ? How long do you plan to keep the CPG 
(clocks) DT backward compatibility for instance ?

> How many overlays would you need if everything is static (i.e. removing all
> your fixup code)?

Do you mean if I hardcoded support for SoCs individually instead of handling 
the differences in code ? At least 5 as that's the number of SoCs that are 
impacted, but that wouldn't be enough. Part of the DT structure that is 
patched is board-specific, not SoC-specific. That's 10 boards in mainline, 
plus all the custom boards out there, and even the development boards 
supported in mainline to which a flat panel has been externally connected.

> > Patching is disabled and will be enabled along with support for the new
> > DT bindings in the DU driver.
> > 
> > Signed-off-by: Laurent Pinchart
> > 
> > ---
> > Changes since v1:
> > 
> > - Select OF_FLATTREE
> > - Compile LVDS DT bindings patch code when DRM_RCAR_LVDS is selected
> > - Update the SPDX headers to use GPL-2.0 instead of GPL-2.0-only
> > - Turn __dtb_rcar_du_of_lvds_(begin|end) from u8 to char
> > - Pass void begin and end pointers to rcar_du_of_get_overlay()
> > - Use of_get_parent() instead of accessing the parent pointer directly
> > - Find the LVDS endpoints nodes based on the LVDS node instead of the
> >   root of the overlay
> > - Update to the -lvds compatible string format
> > ---
> >  drivers/gpu/drm/rcar-du/Kconfig |   2 +
> >  drivers/gpu/drm/rcar-du/Makefile|   3 +-
> >  drivers/gpu/drm/rcar-du/rcar_du_of.c| 451 +++
> >  drivers/gpu/drm/rcar-du/rcar_du_of.h|  16 +
> >  drivers/gpu/drm/rcar-du/rcar_du_of_lvds.dts |  82 +
> >  5 files changed, 553 insertions(+), 1 deletion(-)
> >  create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of.c
> >  create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of.h
> >  create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of_lvds.dts
> > 
> > diff --git a/drivers/gpu/drm/rcar-du/Kconfig
> > b/drivers/gpu/drm/rcar-du/Kconfig index 5d0b4b7119af..3f83352a7313 100644
> > --- a/drivers/gpu/drm/rcar-du/Kconfig
> > +++ b/drivers/gpu/drm/rcar-du/Kconfig
> > @@ -22,6 +22,8 @@ config DRM_RCAR_LVDS
> > bool "R-Car DU LVDS Encoder Support"
> > depends on DRM_RCAR_DU
> > select DRM_PANEL
> > +   select OF_FLATTREE
> > +   select OF_OVERLAY
> 
> OF_OVERLAY should probably select OF_FLATTREE. I guess in theory, we
> could have an overlay from a non-FDT source...

Up to you, I'll happily drop OF_FLATTREE if it gets selected automatically. If 
you think it's a good idea I can submit a patch.

> > help
> >   Enable support for the R-Car Display Unit embedded LVDS
> >   encoders.

[snip]

> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_of.c
> > b/drivers/gpu/drm/rcar-du/rcar_du_of.c new file mode 100644
> > index ..2bf91201fc93
> > --- /dev/null
> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_of.c
> > @@ -0,0 +1,451 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * rcar_du_of.c - Legacy DT bindings compatibility
> > + *
> > + * Copyright (C) 2018 Laurent Pinchart
> > 
> > + *
> > + * Based on work from Jyri Sarha 
> > + * Copyright (C) 2015 Texas Instruments
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include "rcar_du_crtc.h"
> > +#include "rcar_du_drv.h"
> > +
> > +#ifdef CONFIG_DRM_RCAR_LVDS
> 
> Why not make compiling this file conditional in the Makefile?

In case I need to patch other DU-related constructs in the future other than 
LVDS. I could compile this conditionally in the Makefile for now and change it 
later if needed. I have no strong preference.

> > +
> > +struct rcar_du_of_overlay {
> > +   struct device_node *np;
> > +   void *data;
> > +   void *mem;
> > +};
> > +
> > +static void __init rcar_du_of_free_overlay(struct rcar_du_of_overlay
> > *overlay)
> > +{
> > +   of_node_put(overlay->np);
> > +   kfree(overlay->data);
> > +   kfree(overlay->mem);
> > +}
> > +
> > +static int __init rcar_du_of_get_overlay(struct rcar_du_of_overlay
> > *overlay,
> > +void *begin, void
> > 

Re: [PATCH v2 03/12] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes

[Rob Herring]

+Frank

On Fri, Jan 12, 2018 at 5:14 PM, Laurent Pinchart
 wrote:
> The internal LVDS encoders now have their own DT bindings. Before
> switching the driver infrastructure to those new bindings, implement
> backward-compatibility through live DT patching.

Uhh, we just got rid of TI's patching and now adding this one. I guess
it's necessary, but I'd like to know how long we need to keep this?

How many overlays would you need if everything is static (i.e.
removing all your fixup code)?

> Patching is disabled and will be enabled along with support for the new
> DT bindings in the DU driver.
>
> Signed-off-by: Laurent Pinchart 
> ---
> Changes since v1:
>
> - Select OF_FLATTREE
> - Compile LVDS DT bindings patch code when DRM_RCAR_LVDS is selected
> - Update the SPDX headers to use GPL-2.0 instead of GPL-2.0-only
> - Turn __dtb_rcar_du_of_lvds_(begin|end) from u8 to char
> - Pass void begin and end pointers to rcar_du_of_get_overlay()
> - Use of_get_parent() instead of accessing the parent pointer directly
> - Find the LVDS endpoints nodes based on the LVDS node instead of the
>   root of the overlay
> - Update to the -lvds compatible string format
> ---
>  drivers/gpu/drm/rcar-du/Kconfig |   2 +
>  drivers/gpu/drm/rcar-du/Makefile|   3 +-
>  drivers/gpu/drm/rcar-du/rcar_du_of.c| 451 
> 
>  drivers/gpu/drm/rcar-du/rcar_du_of.h|  16 +
>  drivers/gpu/drm/rcar-du/rcar_du_of_lvds.dts |  82 +
>  5 files changed, 553 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of.c
>  create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of.h
>  create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of_lvds.dts
>
> diff --git a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig
> index 5d0b4b7119af..3f83352a7313 100644
> --- a/drivers/gpu/drm/rcar-du/Kconfig
> +++ b/drivers/gpu/drm/rcar-du/Kconfig
> @@ -22,6 +22,8 @@ config DRM_RCAR_LVDS
> bool "R-Car DU LVDS Encoder Support"
> depends on DRM_RCAR_DU
> select DRM_PANEL
> +   select OF_FLATTREE
> +   select OF_OVERLAY

OF_OVERLAY should probably select OF_FLATTREE. I guess in theory, we
could have an overlay from a non-FDT source...

> help
>   Enable support for the R-Car Display Unit embedded LVDS encoders.
>
> diff --git a/drivers/gpu/drm/rcar-du/Makefile 
> b/drivers/gpu/drm/rcar-du/Makefile
> index 0cf5c11030e8..8ed569a0f462 100644
> --- a/drivers/gpu/drm/rcar-du/Makefile
> +++ b/drivers/gpu/drm/rcar-du/Makefile
> @@ -5,10 +5,11 @@ rcar-du-drm-y := rcar_du_crtc.o \
>  rcar_du_group.o \
>  rcar_du_kms.o \
>  rcar_du_lvdscon.o \
> +rcar_du_of.o \
>  rcar_du_plane.o
>
>  rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)+= rcar_du_lvdsenc.o
> -
> +rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)+= rcar_du_of_lvds.dtb.o
>  rcar-du-drm-$(CONFIG_DRM_RCAR_VSP) += rcar_du_vsp.o
>
>  obj-$(CONFIG_DRM_RCAR_DU)  += rcar-du-drm.o
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_of.c 
> b/drivers/gpu/drm/rcar-du/rcar_du_of.c
> new file mode 100644
> index ..2bf91201fc93
> --- /dev/null
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_of.c
> @@ -0,0 +1,451 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * rcar_du_of.c - Legacy DT bindings compatibility
> + *
> + * Copyright (C) 2018 Laurent Pinchart 
> + *
> + * Based on work from Jyri Sarha 
> + * Copyright (C) 2015 Texas Instruments
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "rcar_du_crtc.h"
> +#include "rcar_du_drv.h"
> +
> +#ifdef CONFIG_DRM_RCAR_LVDS

Why not make compiling this file conditional in the Makefile?

> +
> +struct rcar_du_of_overlay {
> +   struct device_node *np;
> +   void *data;
> +   void *mem;
> +};
> +
> +static void __init rcar_du_of_free_overlay(struct rcar_du_of_overlay 
> *overlay)
> +{
> +   of_node_put(overlay->np);
> +   kfree(overlay->data);
> +   kfree(overlay->mem);
> +}
> +
> +static int __init rcar_du_of_get_overlay(struct rcar_du_of_overlay *overlay,
> +void *begin, void *end)
> +{
> +   const size_t size = end - begin;
> +
> +   memset(overlay, 0, sizeof(*overlay));
> +
> +   if (!size)
> +   return -EINVAL;
> +
> +   overlay->data = kmemdup(begin, size, GFP_KERNEL);
> +   if (!overlay->data)
> +   return -ENOMEM;
> +
> +   overlay->mem = of_fdt_unflatten_tree(overlay->data, NULL, 
> >np);
> +   if (!overlay->mem) {
> +   rcar_du_of_free_overlay(overlay);
> +   return -ENOMEM;
> +   }
> +
> +   return 0;
> +}
> +
> +static struct device_node __init *
> +rcar_du_of_find_node_by_path(struct device_node 

[PATCH v2 03/12] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes

[Laurent Pinchart]

The internal LVDS encoders now have their own DT bindings. Before
switching the driver infrastructure to those new bindings, implement
backward-compatibility through live DT patching.

Patching is disabled and will be enabled along with support for the new
DT bindings in the DU driver.

Signed-off-by: Laurent Pinchart 
---
Changes since v1:

- Select OF_FLATTREE
- Compile LVDS DT bindings patch code when DRM_RCAR_LVDS is selected
- Update the SPDX headers to use GPL-2.0 instead of GPL-2.0-only
- Turn __dtb_rcar_du_of_lvds_(begin|end) from u8 to char
- Pass void begin and end pointers to rcar_du_of_get_overlay()
- Use of_get_parent() instead of accessing the parent pointer directly
- Find the LVDS endpoints nodes based on the LVDS node instead of the
  root of the overlay
- Update to the -lvds compatible string format
---
 drivers/gpu/drm/rcar-du/Kconfig |   2 +
 drivers/gpu/drm/rcar-du/Makefile|   3 +-
 drivers/gpu/drm/rcar-du/rcar_du_of.c| 451 
 drivers/gpu/drm/rcar-du/rcar_du_of.h|  16 +
 drivers/gpu/drm/rcar-du/rcar_du_of_lvds.dts |  82 +
 5 files changed, 553 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of.c
 create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of.h
 create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of_lvds.dts

diff --git a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig
index 5d0b4b7119af..3f83352a7313 100644
--- a/drivers/gpu/drm/rcar-du/Kconfig
+++ b/drivers/gpu/drm/rcar-du/Kconfig
@@ -22,6 +22,8 @@ config DRM_RCAR_LVDS
bool "R-Car DU LVDS Encoder Support"
depends on DRM_RCAR_DU
select DRM_PANEL
+   select OF_FLATTREE
+   select OF_OVERLAY
help
  Enable support for the R-Car Display Unit embedded LVDS encoders.
 
diff --git a/drivers/gpu/drm/rcar-du/Makefile b/drivers/gpu/drm/rcar-du/Makefile
index 0cf5c11030e8..8ed569a0f462 100644
--- a/drivers/gpu/drm/rcar-du/Makefile
+++ b/drivers/gpu/drm/rcar-du/Makefile
@@ -5,10 +5,11 @@ rcar-du-drm-y := rcar_du_crtc.o \
 rcar_du_group.o \
 rcar_du_kms.o \
 rcar_du_lvdscon.o \
+rcar_du_of.o \
 rcar_du_plane.o
 
 rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)+= rcar_du_lvdsenc.o
-
+rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)+= rcar_du_of_lvds.dtb.o
 rcar-du-drm-$(CONFIG_DRM_RCAR_VSP) += rcar_du_vsp.o
 
 obj-$(CONFIG_DRM_RCAR_DU)  += rcar-du-drm.o
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_of.c 
b/drivers/gpu/drm/rcar-du/rcar_du_of.c
new file mode 100644
index ..2bf91201fc93
--- /dev/null
+++ b/drivers/gpu/drm/rcar-du/rcar_du_of.c
@@ -0,0 +1,451 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * rcar_du_of.c - Legacy DT bindings compatibility
+ *
+ * Copyright (C) 2018 Laurent Pinchart 
+ *
+ * Based on work from Jyri Sarha 
+ * Copyright (C) 2015 Texas Instruments
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "rcar_du_crtc.h"
+#include "rcar_du_drv.h"
+
+#ifdef CONFIG_DRM_RCAR_LVDS
+
+struct rcar_du_of_overlay {
+   struct device_node *np;
+   void *data;
+   void *mem;
+};
+
+static void __init rcar_du_of_free_overlay(struct rcar_du_of_overlay *overlay)
+{
+   of_node_put(overlay->np);
+   kfree(overlay->data);
+   kfree(overlay->mem);
+}
+
+static int __init rcar_du_of_get_overlay(struct rcar_du_of_overlay *overlay,
+void *begin, void *end)
+{
+   const size_t size = end - begin;
+
+   memset(overlay, 0, sizeof(*overlay));
+
+   if (!size)
+   return -EINVAL;
+
+   overlay->data = kmemdup(begin, size, GFP_KERNEL);
+   if (!overlay->data)
+   return -ENOMEM;
+
+   overlay->mem = of_fdt_unflatten_tree(overlay->data, NULL, >np);
+   if (!overlay->mem) {
+   rcar_du_of_free_overlay(overlay);
+   return -ENOMEM;
+   }
+
+   return 0;
+}
+
+static struct device_node __init *
+rcar_du_of_find_node_by_path(struct device_node *parent, const char *path)
+{
+   parent = of_node_get(parent);
+   if (!parent)
+   return NULL;
+
+   while (parent && *path == '/') {
+   struct device_node *child = NULL;
+   struct device_node *node;
+   const char *next;
+   size_t len;
+
+   /* Skip the initial '/' delimiter and find the next one. */
+   path++;
+   next = strchrnul(path, '/');
+   len = next - path;
+   if (!len)
+   goto error;
+
+   for_each_child_of_node(parent, node) {
+   const char *name = kbasename(node->full_name);
+
+   if (strncmp(path, name, len) == 0 &&
+   strlen(name)