Hi Frank,
On Friday, 23 February 2018 00:10:17 EET Frank Rowand wrote:
> Hi Laurent, Rob,
>
> Thanks for the prompt spin to address my concerns. There are some small
> technical issues.
>
> I did not read the v3 patch until today. v3 through v6 are still using the
> old overlay apply method which uses an expanded device tree as input.
>
> Rob, I don't see my overlay patches in you for-next branch, and I have
> not seen an "Applied" message from you. What is the status of the
> overlay patches?
>
> Comments in the patch below.
>
> On 02/22/18 05:13, 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.
> >
> > Patching is disabled and will be enabled along with support for the new
> > DT bindings in the DU driver.
> >
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx>
> > ---
> > Changes since v5:
> >
> > - Use a private copy of rcar_du_of_changeset_add_property()
> >
> > Changes since v3:
> >
> > - Use the OF changeset API
> > - Use of_graph_get_endpoint_by_regs()
> > - Replace hardcoded constants by sizeof()
> >
> > Changes since v2:
> >
> > - Update the SPDX headers to use C-style comments in header files
> > - Removed the manually created __local_fixups__ node
> > - Perform manual fixups on live DT instead of overlay
> >
> > 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 <soc>-lvds compatible string format
> > ---
> >
> > drivers/gpu/drm/rcar-du/Kconfig | 2 +
> > drivers/gpu/drm/rcar-du/Makefile | 7 +-
> > drivers/gpu/drm/rcar-du/rcar_du_of.c | 342 ++++++++++++++++
> > drivers/gpu/drm/rcar-du/rcar_du_of.h | 20 ++
> > .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7790.dts | 79 +++++
> > .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7791.dts | 53 ++++
> > .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7793.dts | 53 ++++
> > .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7795.dts | 53 ++++
> > .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7796.dts | 53 ++++
> > 9 files changed, 661 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_r8a7790.dts
> > create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7791.dts
> > create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7793.dts
> > create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7795.dts
> > create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7796.dts
[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 000000000000..ac442ddfed16
> > --- /dev/null
> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_of.c
[snip]
> > +static int __init rcar_du_of_apply_overlay(const struct
> > rcar_du_of_overlay *dtbs,
> > + const char *compatible)
> > +{
> > + const struct rcar_du_of_overlay *dtb = NULL;
> > + struct device_node *node = NULL;
> > + unsigned int i;
> > + int ovcs_id;
> > + void *data;
> > + void *mem;
> > + int ret;
> > +
> > + for (i = 0; dtbs[i].compatible; ++i) {
> > + if (!strcmp(dtbs[i].compatible, compatible)) {
> > + dtb = &dtbs[i];
> > + break;
> > + }
> > + }
> > +
> > + if (!dtb)
> > + return -ENODEV;
>
> __If__ my overlay patches are accepted, this block:
>
> > +
> > + data = kmemdup(dtb->begin, dtb->end - dtb->begin, GFP_KERNEL);
> > + if (!data)
> > + return -ENOMEM;
> > +
> > + mem = of_fdt_unflatten_tree(data, NULL, &node);
> > + if (!mem) {
> > + ret = -ENOMEM;
> > + goto done;
> > + }
> > +
> > + ovcs_id = 0;
> > + ret = of_overlay_apply(node, &ovcs_id);
> > +
> > +done:
> > + of_node_put(node);
> > + kfree(data);
> > + kfree(mem);
>
> becomes:
>
> ret = of_overlay_fdt_apply(dtb->begin, &ovcs_id);
I tried to rework this patch in a way that would make switching to FDT
overlays easy, and I'm glad to hear I haven't done a too bad job :-)
Are your patches scheduled for merge in v4.17 ? If so, is it possible to get
apply them in a stable branch on top of v4.16-rc1 that can be merged as a
dependency for this series ? There are changes to the Renesas DT queued for
merge in v4.17 that would make delaying this patch series to v4.18 quite
painful.
> If my overlay patches do not exist, there are other small errors
> in the code block above. I'll ignore them for the moment.
>
> A quick scan of the rest of the code looks OK. I'll read through it
> more carefully, but wanted to get this reply out without further
> delay.
Thank you.
> > +
> > + return ret;
> > +}
[snip]
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
