On 02/23/18 01:00, Laurent Pinchart wrote: > Hi Frank, > > On Friday, 23 February 2018 04:38:06 EET Frank Rowand wrote: >> On 02/22/18 14:10, 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 >>>> >>>> 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 >> >> < snip > >> >>> becomes: >>> ret = of_overlay_fdt_apply(dtb->begin, &ovcs_id); >>> >>> 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. >> >> < snip > >> >> I was hoping to be able to convert the .dts files to use sugar syntax >> instead of hand coding the fragment nodes, but for this specific set >> of files I failed, since the labels that would have been required do >> not already exist in the base .dts files that that overlays would be >> applied against. > > And even if they existed in the base .dts in the kernel sources, there's no > guarantee that the .dtb on the systems we want to patch would contain symbol, > so that wouldn't have been an option anyway, would it ? Correct. And even more troublesome is that some of the fragments are targeted at node "/", which dtc does not even allow a label on (at the moment). > >> Oh well. It was an interesting exercise anyway, trying to be crafty. >
