Re: [U-Boot] [PATCH 4/5] rockchip: video: rk_vop: migrate to livetree

[Anatolij Gustschin]

On Fri, 23 Feb 2018 17:38:52 +0100
Philipp Tomsich philipp.toms...@theobroma-systems.com wrote:

> This migrates rk_vop (the shared functions used by multiple VOP
> mini-drivers) to be compatible with a live tree.
> 
> Unfortunately, there's
> (i)  a lot of tree traversal needed for a VOP (as each active VOP
>  vnode references back to the endpoints in the encoders and vice
>  versa) to configure the connection between VOPs and encoders;
> (ii) the DTS binding is not too sane and one needs to walk a node's
>  parents (the original code just assumed that the device would
>  live 3 levels above the property linked through a phandle) until
>  a UCLASS_DISPLAY device can be found.
> 
> As part of the migration, the code for finding the enclosing display
> device has been changed to not assume a specific depth of nesting
> (i.e. we walk until we reach the root or find a matching device) and
> to use the newly introduced (in the same series) ofnode_get_parent()
> function.
> 
> Signed-off-by: Philipp Tomsich 
> Tested-by: Klaus Goger 

Reviewed-by: Anatolij Gustschin 
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH 4/5] rockchip: video: rk_vop: migrate to livetree

[Philipp Tomsich]

This migrates rk_vop (the shared functions used by multiple VOP
mini-drivers) to be compatible with a live tree.

Unfortunately, there's
(i)  a lot of tree traversal needed for a VOP (as each active VOP
 vnode references back to the endpoints in the encoders and vice
 versa) to configure the connection between VOPs and encoders;
(ii) the DTS binding is not too sane and one needs to walk a node's
 parents (the original code just assumed that the device would
 live 3 levels above the property linked through a phandle) until
 a UCLASS_DISPLAY device can be found.

As part of the migration, the code for finding the enclosing display
device has been changed to not assume a specific depth of nesting
(i.e. we walk until we reach the root or find a matching device) and
to use the newly introduced (in the same series) ofnode_get_parent()
function.

Signed-off-by: Philipp Tomsich 
Tested-by: Klaus Goger 
---

 drivers/video/rockchip/rk_vop.c | 85 +++--
 1 file changed, 57 insertions(+), 28 deletions(-)

diff --git a/drivers/video/rockchip/rk_vop.c b/drivers/video/rockchip/rk_vop.c
index c979049..1288608 100644
--- a/drivers/video/rockchip/rk_vop.c
+++ b/drivers/video/rockchip/rk_vop.c
@@ -218,41 +218,67 @@ static void rkvop_mode_set(struct udevice *dev,
  * node within the VOP's 'port' list.
  * @return 0 if OK, -ve if something went wrong
  */
-static int rk_display_init(struct udevice *dev, ulong fbbase, int ep_node)
+static int rk_display_init(struct udevice *dev, ulong fbbase, ofnode ep_node)
 {
struct video_priv *uc_priv = dev_get_uclass_priv(dev);
-   const void *blob = gd->fdt_blob;
struct rk_vop_priv *priv = dev_get_priv(dev);
int vop_id, remote_vop_id;
struct rk3288_vop *regs = priv->regs;
struct display_timing timing;
struct udevice *disp;
-   int ret, remote, i, offset;
+   int ret;
+   u32 remote_phandle;
struct display_plat *disp_uc_plat;
struct clk clk;
enum video_log2_bpp l2bpp;
+   ofnode remote;
 
-   vop_id = fdtdec_get_int(blob, ep_node, "reg", -1);
+   debug("%s(%s, %lu, %s)\n", __func__,
+ dev_read_name(dev), fbbase, ofnode_get_name(ep_node));
+
+   vop_id = ofnode_read_s32_default(ep_node, "reg", -1);
debug("vop_id=%d\n", vop_id);
-   remote = fdtdec_lookup_phandle(blob, ep_node, "remote-endpoint");
-   if (remote < 0)
-   return -EINVAL;
-   remote_vop_id = fdtdec_get_int(blob, remote, "reg", -1);
-   debug("remote vop_id=%d\n", remote_vop_id);
+   ret = ofnode_read_u32(ep_node, "remote-endpoint", _phandle);
+   if (ret)
+   return ret;
 
-   for (i = 0, offset = remote; i < 3 && offset > 0; i++)
-   offset = fdt_parent_offset(blob, offset);
-   if (offset < 0) {
-   debug("%s: Invalid remote-endpoint position\n", dev->name);
+   remote = ofnode_get_by_phandle(remote_phandle);
+   if (!ofnode_valid(remote))
return -EINVAL;
-   }
+   remote_vop_id = ofnode_read_u32_default(remote, "reg", -1);
+   debug("remote vop_id=%d\n", remote_vop_id);
 
-   ret = uclass_find_device_by_of_offset(UCLASS_DISPLAY, offset, );
-   if (ret) {
-   debug("%s: device '%s' display not found (ret=%d)\n", __func__,
- dev->name, ret);
-   return ret;
-   }
+   /*
+* The remote-endpoint references into a subnode of the encoder
+* (i.e. HDMI, MIPI, etc.) with the DTS looking something like
+* the following (assume 'hdmi_in_vopl' to be referenced):
+*
+* hdmi: hdmi@ff94 {
+*   ports {
+* hdmi_in: port {
+*   hdmi_in_vopb: endpoint@0 { ... };
+*   hdmi_in_vopl: endpoint@1 { ... };
+* }
+*   }
+* }
+*
+* The original code had 3 steps of "walking the parent", but
+* a much better (as in: less likely to break if the DTS
+* changes) way of doing this is to "find the enclosing device
+* of UCLASS_DISPLAY".
+*/
+   while (ofnode_valid(remote)) {
+   remote = ofnode_get_parent(remote);
+   if (!ofnode_valid(remote)) {
+   debug("%s(%s): no UCLASS_DISPLAY for remote-endpoint\n",
+ __func__, dev_read_name(dev));
+   return -EINVAL;
+   }
+
+   uclass_find_device_by_ofnode(UCLASS_DISPLAY, remote, );
+   if (disp)
+   break;
+   };
 
disp_uc_plat = dev_get_uclass_platdata(disp);
debug("Found device '%s', disp_uc_priv=%p\n", disp->name, disp_uc_plat);
@@ -334,16 +360,15 @@ void rk_vop_probe_regulators(struct udevice *dev,
 int rk_vop_probe(struct udevice *dev)
 {