[PATCH 0/6] ARM: i.MX6: Fix LVDS splash on skov and some others

[PATCH 0/6] ARM: i.MX6: Fix LVDS splash on skov and some others

[Ahmad Fatoum]

The Skov boards exist in multiple variants, among them ones with
displays connected over LVDS and HDMI.
HDMI continues to work for me, but LVDS has regressed due to two issues:

  - LVDS clock calculation was broken by clock framework rework

  - Device tree is fixed up in wrong order in skov board code, which
    became problematic when deep probe was enabled for that board

The first four patches fix these issues and should preferably be
applied to master. The remainder are other patches that resulted
during debugging.

The first problem is generic and I am curious what other issues may
disappear or pop up, now that's fixed.

Cheers,
Ahmad Fatoum (6):
  clk: imx: set CLK_SET_RATE_NO_REPARENT for all muxes
  clk: mux: forward round/set rate to parent if CLK_SET_RATE_PARENT
  ARM: i.MX6: skov: refactor LVDS/parallel device tree fixups
  ARM: i.MX6: skov: fix LVDS deep probe
  video: edid: print debug message on EDID read out error
  ARM: configs: imx_v7_defconfig: enable some useful options

 arch/arm/boards/skov-imx6/board.c | 60 +++++++++++++++++++------------
 arch/arm/configs/imx_v7_defconfig | 16 +++++++++
 drivers/clk/clk-mux.c             |  4 +--
 drivers/clk/imx/clk.h             | 30 ++++++++++------
 drivers/video/edid.c              | 11 ++++--
 5 files changed, 84 insertions(+), 37 deletions(-)

-- 
2.30.2

[PATCH 1/6] clk: imx: set CLK_SET_RATE_NO_REPARENT for all muxes

[Ahmad Fatoum]

The Linux i.MX clk drivers instantiate all muxes with
CLK_SET_RATE_NO_REPARENT. Do likewise in barebox.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/clk/imx/clk.h | 30 +++++++++++++++++++-----------
 1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/drivers/clk/imx/clk.h b/drivers/clk/imx/clk.h
index bc44d0c739e0..041592a06431 100644
--- a/drivers/clk/imx/clk.h
+++ b/drivers/clk/imx/clk.h
@@ -40,11 +40,19 @@ static inline struct clk *imx_clk_divider_table(const char *name,
 				 width, table, 0);
 }
 
+static inline struct clk *__imx_clk_mux(const char *name, void __iomem *reg,
+					u8 shift, u8 width, const char * const *parents,
+					u8 num_parents, unsigned flags, unsigned long clk_mux_flags)
+{
+	return clk_mux(name, CLK_SET_RATE_NO_REPARENT | flags, reg,
+		       shift, width, parents, num_parents, clk_mux_flags);
+}
+
 static inline struct clk *imx_clk_mux_ldb(const char *name, void __iomem *reg,
 		u8 shift, u8 width, const char * const *parents, int num_parents)
 {
-	return clk_mux(name, CLK_SET_RATE_NO_REPARENT | CLK_SET_RATE_PARENT, reg,
-		       shift, width, parents, num_parents, CLK_MUX_READ_ONLY);
+	return __imx_clk_mux(name, reg, shift, width, parents, num_parents,
+			     CLK_SET_RATE_PARENT, CLK_MUX_READ_ONLY);
 }
 
 
@@ -59,36 +67,36 @@ static inline struct clk *imx_clk_mux_flags(const char *name, void __iomem *reg,
 					    const char * const *parents, u8 num_parents,
 					    unsigned long clk_flags)
 {
-	return clk_mux(name, clk_flags, reg, shift, width, parents, num_parents,
-		       0);
+	return __imx_clk_mux(name, reg, shift, width, parents, num_parents,
+			     clk_flags, 0);
 }
 
 static inline struct clk *imx_clk_mux2_flags(const char *name,
 		void __iomem *reg, u8 shift, u8 width, const char * const *parents,
 		int num_parents, unsigned long clk_flags)
 {
-	return clk_mux(name, clk_flags | CLK_OPS_PARENT_ENABLE, reg, shift,
-		       width, parents, num_parents, 0);
+	return __imx_clk_mux(name,reg, shift, width, parents, num_parents,
+			     clk_flags | CLK_OPS_PARENT_ENABLE, 0);
 }
 
 static inline struct clk *imx_clk_mux(const char *name, void __iomem *reg,
 		u8 shift, u8 width, const char * const *parents, u8 num_parents)
 {
-	return clk_mux(name, 0, reg, shift, width, parents, num_parents, 0);
+	return __imx_clk_mux(name, reg, shift, width, parents, num_parents, 0, 0);
 }
 
 static inline struct clk *imx_clk_mux2(const char *name, void __iomem *reg,
 		u8 shift, u8 width, const char * const *parents, u8 num_parents)
 {
-	return clk_mux(name, CLK_OPS_PARENT_ENABLE, reg, shift, width, parents,
-		       num_parents, 0);
+	return __imx_clk_mux(name, reg, shift, width, parents,
+			     num_parents, CLK_OPS_PARENT_ENABLE, 0);
 }
 
 static inline struct clk *imx_clk_mux_p(const char *name, void __iomem *reg,
 		u8 shift, u8 width, const char * const *parents, u8 num_parents)
 {
-	return clk_mux(name, CLK_SET_RATE_PARENT, reg, shift, width, parents,
-		       num_parents, 0);
+	return __imx_clk_mux(name, reg, shift, width, parents, num_parents,
+			     CLK_SET_RATE_PARENT,  0);
 }
 
 static inline struct clk *imx_clk_gate(const char *name, const char *parent,
-- 
2.30.2

Re: [PATCH 1/6] clk: imx: set CLK_SET_RATE_NO_REPARENT for all muxes

[Marco Felsch]

On 22-12-22, Ahmad Fatoum wrote:
> The Linux i.MX clk drivers instantiate all muxes with
> CLK_SET_RATE_NO_REPARENT. Do likewise in barebox.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

Reviewed-by: Marco Felsch <m.felsch@pengutronix.de>

> ---
>  drivers/clk/imx/clk.h | 30 +++++++++++++++++++-----------
>  1 file changed, 19 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/clk/imx/clk.h b/drivers/clk/imx/clk.h
> index bc44d0c739e0..041592a06431 100644
> --- a/drivers/clk/imx/clk.h
> +++ b/drivers/clk/imx/clk.h
> @@ -40,11 +40,19 @@ static inline struct clk *imx_clk_divider_table(const char *name,
>  				 width, table, 0);
>  }
>  
> +static inline struct clk *__imx_clk_mux(const char *name, void __iomem *reg,
> +					u8 shift, u8 width, const char * const *parents,
> +					u8 num_parents, unsigned flags, unsigned long clk_mux_flags)
> +{
> +	return clk_mux(name, CLK_SET_RATE_NO_REPARENT | flags, reg,
> +		       shift, width, parents, num_parents, clk_mux_flags);
> +}
> +
>  static inline struct clk *imx_clk_mux_ldb(const char *name, void __iomem *reg,
>  		u8 shift, u8 width, const char * const *parents, int num_parents)
>  {
> -	return clk_mux(name, CLK_SET_RATE_NO_REPARENT | CLK_SET_RATE_PARENT, reg,
> -		       shift, width, parents, num_parents, CLK_MUX_READ_ONLY);
> +	return __imx_clk_mux(name, reg, shift, width, parents, num_parents,
> +			     CLK_SET_RATE_PARENT, CLK_MUX_READ_ONLY);
>  }
>  
>  
> @@ -59,36 +67,36 @@ static inline struct clk *imx_clk_mux_flags(const char *name, void __iomem *reg,
>  					    const char * const *parents, u8 num_parents,
>  					    unsigned long clk_flags)
>  {
> -	return clk_mux(name, clk_flags, reg, shift, width, parents, num_parents,
> -		       0);
> +	return __imx_clk_mux(name, reg, shift, width, parents, num_parents,
> +			     clk_flags, 0);
>  }
>  
>  static inline struct clk *imx_clk_mux2_flags(const char *name,
>  		void __iomem *reg, u8 shift, u8 width, const char * const *parents,
>  		int num_parents, unsigned long clk_flags)
>  {
> -	return clk_mux(name, clk_flags | CLK_OPS_PARENT_ENABLE, reg, shift,
> -		       width, parents, num_parents, 0);
> +	return __imx_clk_mux(name,reg, shift, width, parents, num_parents,
> +			     clk_flags | CLK_OPS_PARENT_ENABLE, 0);
>  }
>  
>  static inline struct clk *imx_clk_mux(const char *name, void __iomem *reg,
>  		u8 shift, u8 width, const char * const *parents, u8 num_parents)
>  {
> -	return clk_mux(name, 0, reg, shift, width, parents, num_parents, 0);
> +	return __imx_clk_mux(name, reg, shift, width, parents, num_parents, 0, 0);
>  }
>  
>  static inline struct clk *imx_clk_mux2(const char *name, void __iomem *reg,
>  		u8 shift, u8 width, const char * const *parents, u8 num_parents)
>  {
> -	return clk_mux(name, CLK_OPS_PARENT_ENABLE, reg, shift, width, parents,
> -		       num_parents, 0);
> +	return __imx_clk_mux(name, reg, shift, width, parents,
> +			     num_parents, CLK_OPS_PARENT_ENABLE, 0);
>  }
>  
>  static inline struct clk *imx_clk_mux_p(const char *name, void __iomem *reg,
>  		u8 shift, u8 width, const char * const *parents, u8 num_parents)
>  {
> -	return clk_mux(name, CLK_SET_RATE_PARENT, reg, shift, width, parents,
> -		       num_parents, 0);
> +	return __imx_clk_mux(name, reg, shift, width, parents, num_parents,
> +			     CLK_SET_RATE_PARENT,  0);
>  }
>  
>  static inline struct clk *imx_clk_gate(const char *name, const char *parent,
> -- 
> 2.30.2
> 
> 
> 

[PATCH 2/6] clk: mux: forward round/set rate to parent if CLK_SET_RATE_PARENT

[Ahmad Fatoum]

Prior to v2021.07.0, clk mux round/set rate ops were forwarded to
clk_parent_round_rate/clk_parent_set_rate, which would change nothing
unless CLK_SET_RATE_PARENT is set.

Since that release, barebox will instead try to reparent the mux
to arrive at a closer rate, unless CLK_SET_RATE_NO_REPARENT is
specified.

The correct behavior would have been for CLK_SET_RATE_NO_REPARENT
to fall back to the old behavior when NO_REPARENT is specified,
but instead CLK_SET_RATE_PARENT ended up ignored.

Fix this by calling clk_parent_round_rate/clk_parent_set_rate once again
in that case. When CLK_SET_RATE_PARENT is not set, they are equivalent
to the current open-coded behavior.

Fixes: d07c34e116cd ("clk: clk-mux: implement setting rate by reparenting")
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/clk/clk-mux.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c
index ad82d97ea88c..1d94e0916732 100644
--- a/drivers/clk/clk-mux.c
+++ b/drivers/clk/clk-mux.c
@@ -118,7 +118,7 @@ long clk_mux_round_rate(struct clk_hw *hw, unsigned long rate,
 	struct clk *bestparent;
 
 	if (clk->flags & CLK_SET_RATE_NO_REPARENT)
-		return *prate;
+		return clk_parent_round_rate(hw, rate, prate);
 
 	bestparent = clk_mux_best_parent(clk, rate, &rrate);
 
@@ -135,7 +135,7 @@ static int clk_mux_set_rate(struct clk_hw *hw, unsigned long rate,
 	int ret;
 
 	if (clk->flags & CLK_SET_RATE_NO_REPARENT)
-		return 0;
+		return clk_parent_set_rate(hw, rate, parent_rate);
 
 	parent = clk_mux_best_parent(clk, rate, &rrate);
 
-- 
2.30.2

[PATCH 3/6] ARM: i.MX6: skov: refactor LVDS/parallel device tree fixups

[Ahmad Fatoum]

In preparation for a fix in a follow-up commit, refactor the code, so
each fixup is done in its own function. This enables easy early exists
without increasing indentation level. No functional change

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 arch/arm/boards/skov-imx6/board.c | 59 +++++++++++++++++++------------
 1 file changed, 37 insertions(+), 22 deletions(-)

diff --git a/arch/arm/boards/skov-imx6/board.c b/arch/arm/boards/skov-imx6/board.c
index d5d229ae81ac..7ddc6e937b0b 100644
--- a/arch/arm/boards/skov-imx6/board.c
+++ b/arch/arm/boards/skov-imx6/board.c
@@ -451,6 +451,39 @@ static int skov_imx6_fixup(struct device_node *root, void *unused)
 	return 0;
 }
 
+static void skov_init_parallel_lcd(void)
+{
+	struct device_node *lcd;
+
+	lcd = of_find_compatible_node(NULL, NULL, "fsl,imx-parallel-display");
+	if (!lcd) {
+		dev_err(skov_priv->dev, "Cannot find \"fsl,imx-parallel-display\" node\n");
+		return;
+	}
+
+	of_device_enable_and_register(lcd);
+}
+
+static void skov_init_ldb(void)
+{
+	struct device_node *ldb, *chan;
+
+	ldb = of_find_compatible_node(NULL, NULL, "fsl,imx6q-ldb");
+	if (!ldb) {
+		dev_err(skov_priv->dev, "Cannot find \"fsl,imx6q-ldb\" node\n");
+		return;
+	}
+
+	of_device_enable_and_register(ldb);
+
+	/* ... as well as its channel 0 */
+	chan = of_find_node_by_name_address(ldb, "lvds-channel@0");
+	if (chan)
+		of_device_enable(chan);
+	else
+		dev_err(skov_priv->dev, "Cannot find \"lvds-channel@0\" node\n");
+}
+
 /*
  * Some variants need tweaks to make them work
  *
@@ -461,7 +494,6 @@ static int skov_imx6_fixup(struct device_node *root, void *unused)
 static void skov_init_board(const struct board_description *variant)
 {
 	struct device_node *gpio_np = NULL;
-	struct device_node *np;
 	char *environment_path, *envdev;
 	int ret;
 
@@ -524,28 +556,11 @@ static void skov_init_board(const struct board_description *variant)
 		gpio_free(200);
 	}
 
-	if (variant->flags & SKOV_DISPLAY_PARALLEL) {
-		np = of_find_compatible_node(NULL, NULL, "fsl,imx-parallel-display");
-		if (np)
-			of_device_enable_and_register(np);
-		else
-			dev_err(skov_priv->dev, "Cannot find \"fsl,imx-parallel-display\" node\n");
-	}
+	if (variant->flags & SKOV_DISPLAY_PARALLEL)
+		skov_init_parallel_lcd();
 
-	if (variant->flags & SKOV_DISPLAY_LVDS) {
-		np = of_find_compatible_node(NULL, NULL, "fsl,imx6q-ldb");
-		if (np)
-			of_device_enable_and_register(np);
-		else
-			dev_err(skov_priv->dev, "Cannot find \"fsl,imx6q-ldb\" node\n");
-
-		/* ... as well as its channel 0 */
-		np = of_find_node_by_name_address(np, "lvds-channel@0");
-		if (np)
-			of_device_enable(np);
-		else
-			dev_err(skov_priv->dev, "Cannot find \"lvds-channel@0\" node\n");
-	}
+	if (variant->flags & SKOV_DISPLAY_LVDS)
+		skov_init_ldb();
 }
 
 static int skov_set_switch_lan2_mac(struct skov_imx6_priv *priv)
-- 
2.30.2

Re: [PATCH 3/6] ARM: i.MX6: skov: refactor LVDS/parallel device tree fixups

[Marco Felsch]

On 22-12-22, Ahmad Fatoum wrote:
> In preparation for a fix in a follow-up commit, refactor the code, so
> each fixup is done in its own function. This enables easy early exists
> without increasing indentation level. No functional change
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

Reviewed-by: Marco Felsch <m.felsch@pengutronix.de>

> ---
>  arch/arm/boards/skov-imx6/board.c | 59 +++++++++++++++++++------------
>  1 file changed, 37 insertions(+), 22 deletions(-)
> 
> diff --git a/arch/arm/boards/skov-imx6/board.c b/arch/arm/boards/skov-imx6/board.c
> index d5d229ae81ac..7ddc6e937b0b 100644
> --- a/arch/arm/boards/skov-imx6/board.c
> +++ b/arch/arm/boards/skov-imx6/board.c
> @@ -451,6 +451,39 @@ static int skov_imx6_fixup(struct device_node *root, void *unused)
>  	return 0;
>  }
>  
> +static void skov_init_parallel_lcd(void)
> +{
> +	struct device_node *lcd;
> +
> +	lcd = of_find_compatible_node(NULL, NULL, "fsl,imx-parallel-display");
> +	if (!lcd) {
> +		dev_err(skov_priv->dev, "Cannot find \"fsl,imx-parallel-display\" node\n");
> +		return;
> +	}
> +
> +	of_device_enable_and_register(lcd);
> +}
> +
> +static void skov_init_ldb(void)
> +{
> +	struct device_node *ldb, *chan;
> +
> +	ldb = of_find_compatible_node(NULL, NULL, "fsl,imx6q-ldb");
> +	if (!ldb) {
> +		dev_err(skov_priv->dev, "Cannot find \"fsl,imx6q-ldb\" node\n");
> +		return;
> +	}
> +
> +	of_device_enable_and_register(ldb);
> +
> +	/* ... as well as its channel 0 */
> +	chan = of_find_node_by_name_address(ldb, "lvds-channel@0");
> +	if (chan)
> +		of_device_enable(chan);
> +	else
> +		dev_err(skov_priv->dev, "Cannot find \"lvds-channel@0\" node\n");
> +}
> +
>  /*
>   * Some variants need tweaks to make them work
>   *
> @@ -461,7 +494,6 @@ static int skov_imx6_fixup(struct device_node *root, void *unused)
>  static void skov_init_board(const struct board_description *variant)
>  {
>  	struct device_node *gpio_np = NULL;
> -	struct device_node *np;
>  	char *environment_path, *envdev;
>  	int ret;
>  
> @@ -524,28 +556,11 @@ static void skov_init_board(const struct board_description *variant)
>  		gpio_free(200);
>  	}
>  
> -	if (variant->flags & SKOV_DISPLAY_PARALLEL) {
> -		np = of_find_compatible_node(NULL, NULL, "fsl,imx-parallel-display");
> -		if (np)
> -			of_device_enable_and_register(np);
> -		else
> -			dev_err(skov_priv->dev, "Cannot find \"fsl,imx-parallel-display\" node\n");
> -	}
> +	if (variant->flags & SKOV_DISPLAY_PARALLEL)
> +		skov_init_parallel_lcd();
>  
> -	if (variant->flags & SKOV_DISPLAY_LVDS) {
> -		np = of_find_compatible_node(NULL, NULL, "fsl,imx6q-ldb");
> -		if (np)
> -			of_device_enable_and_register(np);
> -		else
> -			dev_err(skov_priv->dev, "Cannot find \"fsl,imx6q-ldb\" node\n");
> -
> -		/* ... as well as its channel 0 */
> -		np = of_find_node_by_name_address(np, "lvds-channel@0");
> -		if (np)
> -			of_device_enable(np);
> -		else
> -			dev_err(skov_priv->dev, "Cannot find \"lvds-channel@0\" node\n");
> -	}
> +	if (variant->flags & SKOV_DISPLAY_LVDS)
> +		skov_init_ldb();
>  }
>  
>  static int skov_set_switch_lan2_mac(struct skov_imx6_priv *priv)
> -- 
> 2.30.2
> 
> 
> 

[PATCH 4/6] ARM: i.MX6: skov: fix LVDS deep probe

[Ahmad Fatoum]

With deep probe, barebox will have registered most drivers by the time
it walks the device tree and will then keep probing devices on demand.

This simplifies the common case of devices having dependencies on each
other, but on the other hand expects board code to be deep probe aware.

The Skov board code already takes care to explicitly probe the GPIO
controllers it requires, but it also optionally enables and registers
devices that were initially disabled in the device tree.

It's paramount that devices are only registered _after_ the relevant
device tree parts are rewritten. This was currently not accounted for,
which led to LDB device being probed before its child lvds-channel node
was enabled: We thus ended up with a LDB device, what the driver
couldn't do anything with:

  mode_name: invalid:0 (type: enum)

Fixes: 31d2289da2f3 ("ARM: boards: skov-imx6: start using deep-probe")
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 arch/arm/boards/skov-imx6/board.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boards/skov-imx6/board.c b/arch/arm/boards/skov-imx6/board.c
index 7ddc6e937b0b..8f4d7efe4278 100644
--- a/arch/arm/boards/skov-imx6/board.c
+++ b/arch/arm/boards/skov-imx6/board.c
@@ -474,14 +474,15 @@ static void skov_init_ldb(void)
 		return;
 	}
 
-	of_device_enable_and_register(ldb);
-
-	/* ... as well as its channel 0 */
+	/* First enable channel 0, prior to enabling parent */
 	chan = of_find_node_by_name_address(ldb, "lvds-channel@0");
 	if (chan)
 		of_device_enable(chan);
 	else
 		dev_err(skov_priv->dev, "Cannot find \"lvds-channel@0\" node\n");
+
+	/* Now probe will see the expected device tree */
+	of_device_enable_and_register(ldb);
 }
 
 /*
-- 
2.30.2

Re: [PATCH 4/6] ARM: i.MX6: skov: fix LVDS deep probe

[Marco Felsch]

On 22-12-22, Ahmad Fatoum wrote:
> With deep probe, barebox will have registered most drivers by the time
> it walks the device tree and will then keep probing devices on demand.
> 
> This simplifies the common case of devices having dependencies on each
> other, but on the other hand expects board code to be deep probe aware.
> 
> The Skov board code already takes care to explicitly probe the GPIO
> controllers it requires, but it also optionally enables and registers
> devices that were initially disabled in the device tree.
> 
> It's paramount that devices are only registered _after_ the relevant
> device tree parts are rewritten. This was currently not accounted for,
> which led to LDB device being probed before its child lvds-channel node
> was enabled: We thus ended up with a LDB device, what the driver
> couldn't do anything with:
> 
>   mode_name: invalid:0 (type: enum)
> 
> Fixes: 31d2289da2f3 ("ARM: boards: skov-imx6: start using deep-probe")
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

Reviewed-by: Marco Felsch <m.felsch@pengutronix.de>

> ---
>  arch/arm/boards/skov-imx6/board.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/boards/skov-imx6/board.c b/arch/arm/boards/skov-imx6/board.c
> index 7ddc6e937b0b..8f4d7efe4278 100644
> --- a/arch/arm/boards/skov-imx6/board.c
> +++ b/arch/arm/boards/skov-imx6/board.c
> @@ -474,14 +474,15 @@ static void skov_init_ldb(void)
>  		return;
>  	}
>  
> -	of_device_enable_and_register(ldb);
> -
> -	/* ... as well as its channel 0 */
> +	/* First enable channel 0, prior to enabling parent */
>  	chan = of_find_node_by_name_address(ldb, "lvds-channel@0");
>  	if (chan)
>  		of_device_enable(chan);
>  	else
>  		dev_err(skov_priv->dev, "Cannot find \"lvds-channel@0\" node\n");
> +
> +	/* Now probe will see the expected device tree */
> +	of_device_enable_and_register(ldb);
>  }
>  
>  /*
> -- 
> 2.30.2
> 
> 
> 

[PATCH 5/6] video: edid: print debug message on EDID read out error

[Ahmad Fatoum]

EDID readout errors happen often, e.g. because the HDMI port doesn't
have a display connected. However, when a monitor is connected, but some
other error occurs, barebox is silent. Add a debug message with an error
code for this.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/video/edid.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/video/edid.c b/drivers/video/edid.c
index 96489f2a372c..7e6747ccd521 100644
--- a/drivers/video/edid.c
+++ b/drivers/video/edid.c
@@ -847,20 +847,27 @@ edid_do_read_i2c(struct i2c_adapter *adapter, unsigned char *buf,
 		ret = i2c_transfer(adapter, &msgs[3 - xfers], xfers);
 	} while (ret != xfers && --retries);
 
-	return ret == xfers ? 0 : -1;
+	if (ret == 0)
+		ret = -EPROTO;
+
+	return ret == xfers ? 0 : ret;
 }
 
 void *edid_read_i2c(struct i2c_adapter *adapter)
 {
 	u8 *block;
+	int ret;
 
 	if (!IS_ENABLED(CONFIG_I2C))
 		return NULL;
 
 	block = xmalloc(EDID_LENGTH);
 
-	if (edid_do_read_i2c(adapter, block, 0, EDID_LENGTH))
+	ret = edid_do_read_i2c(adapter, block, 0, EDID_LENGTH);
+	if (ret) {
+		dev_dbg(&adapter->dev, "EDID readout failed: %pe\n", ERR_PTR(ret));
 		goto out;
+	}
 
 	return block;
 out:
-- 
2.30.2

Re: [PATCH 5/6] video: edid: print debug message on EDID read out error

[Marco Felsch]

Hi Ahmad,

On 22-12-22, Ahmad Fatoum wrote:
> EDID readout errors happen often, e.g. because the HDMI port doesn't
> have a display connected. However, when a monitor is connected, but some
> other error occurs, barebox is silent. Add a debug message with an error
> code for this.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  drivers/video/edid.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/video/edid.c b/drivers/video/edid.c
> index 96489f2a372c..7e6747ccd521 100644
> --- a/drivers/video/edid.c
> +++ b/drivers/video/edid.c
> @@ -847,20 +847,27 @@ edid_do_read_i2c(struct i2c_adapter *adapter, unsigned char *buf,
>  		ret = i2c_transfer(adapter, &msgs[3 - xfers], xfers);
>  	} while (ret != xfers && --retries);
>  
> -	return ret == xfers ? 0 : -1;
> +	if (ret == 0)

When does the i2c_transfer() return 0? According the API documentation
this shouldn't happen and if otherwise is a bug in i2c_transfer().

Regards,
  Marco

> +		ret = -EPROTO;
> +
> +	return ret == xfers ? 0 : ret;
>  }
>  
>  void *edid_read_i2c(struct i2c_adapter *adapter)
>  {
>  	u8 *block;
> +	int ret;
>  
>  	if (!IS_ENABLED(CONFIG_I2C))
>  		return NULL;
>  
>  	block = xmalloc(EDID_LENGTH);
>  
> -	if (edid_do_read_i2c(adapter, block, 0, EDID_LENGTH))
> +	ret = edid_do_read_i2c(adapter, block, 0, EDID_LENGTH);
> +	if (ret) {
> +		dev_dbg(&adapter->dev, "EDID readout failed: %pe\n", ERR_PTR(ret));
>  		goto out;
> +	}
>  
>  	return block;
>  out:
> -- 
> 2.30.2
> 
> 
> 

[PATCH 6/6] ARM: configs: imx_v7_defconfig: enable some useful options

[Ahmad Fatoum]

Give the defconfig some color by enabling console colors during startup,
i.MX6 parallel LCD driver, backlight and some more options that were
noticed missing during debugging a skov-imx6 issue.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 arch/arm/configs/imx_v7_defconfig | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/arm/configs/imx_v7_defconfig b/arch/arm/configs/imx_v7_defconfig
index 3cbec267fc87..6110a52e486f 100644
--- a/arch/arm/configs/imx_v7_defconfig
+++ b/arch/arm/configs/imx_v7_defconfig
@@ -72,11 +72,13 @@ CONFIG_BOOTM_OFTREE=y
 CONFIG_BOOTM_OFTREE_UIMAGE=y
 CONFIG_BLSPEC=y
 CONFIG_CONSOLE_ACTIVATE_NONE=y
+CONFIG_CONSOLE_ALLOW_COLOR=y
 CONFIG_PARTITION_DISK_EFI=y
 CONFIG_DEFAULT_ENVIRONMENT_GENERIC_NEW=y
 CONFIG_STATE=y
 CONFIG_BOOTCHOOSER=y
 CONFIG_RESET_SOURCE=y
+CONFIG_MACHINE_ID=y
 CONFIG_FASTBOOT_CMD_OEM=y
 CONFIG_CMD_DMESG=y
 CONFIG_LONGHELP=y
@@ -86,6 +88,7 @@ CONFIG_CMD_MEMINFO=y
 CONFIG_CMD_ARM_MMUINFO=y
 CONFIG_CMD_REGULATOR=y
 CONFIG_CMD_MMC_EXTCSD=y
+CONFIG_CMD_FCB=y
 # CONFIG_CMD_BOOTU is not set
 CONFIG_CMD_GO=y
 CONFIG_CMD_RESET=y
@@ -117,6 +120,7 @@ CONFIG_CMD_MENU=y
 CONFIG_CMD_MENU_MANAGEMENT=y
 CONFIG_CMD_MENUTREE=y
 CONFIG_CMD_SPLASH=y
+CONFIG_CMD_FBTEST=y
 CONFIG_CMD_READLINE=y
 CONFIG_CMD_TIMEOUT=y
 CONFIG_CMD_CRC=y
@@ -149,9 +153,14 @@ CONFIG_OF_BAREBOX_DRIVERS=y
 CONFIG_DRIVER_NET_FEC_IMX=y
 CONFIG_AT803X_PHY=y
 CONFIG_MICREL_PHY=y
+CONFIG_MDIO_BITBANG=y
+CONFIG_MDIO_GPIO=y
+CONFIG_MDIO_BUS_MUX_GPIO=y
 CONFIG_NET_USB=y
 CONFIG_NET_USB_ASIX=y
 CONFIG_NET_USB_SMSC95XX=y
+CONFIG_I2C_GPIO=y
+CONFIG_I2C_MUX=y
 CONFIG_MTD=y
 CONFIG_MTD_RAW_DEVICE=y
 CONFIG_MTD_DATAFLASH=y
@@ -180,8 +189,12 @@ CONFIG_VIDEO=y
 CONFIG_DRIVER_VIDEO_IMX_IPUV3=y
 CONFIG_DRIVER_VIDEO_IMX_IPUV3_LVDS=y
 CONFIG_DRIVER_VIDEO_IMX_IPUV3_HDMI=y
+CONFIG_DRIVER_VIDEO_IMX_IPUV3_PARALLEL=y
 CONFIG_DRIVER_VIDEO_SIMPLEFB=y
 CONFIG_DRIVER_VIDEO_EDID=y
+CONFIG_DRIVER_VIDEO_BACKLIGHT=y
+CONFIG_DRIVER_VIDEO_BACKLIGHT_PWM=y
+CONFIG_DRIVER_VIDEO_SIMPLE_PANEL=y
 CONFIG_MCI=y
 CONFIG_MCI_MMC_BOOT_PARTITIONS=y
 CONFIG_MCI_IMX_ESDHC=y
@@ -219,4 +232,7 @@ CONFIG_FS_FAT_WRITE=y
 CONFIG_FS_FAT_LFN=y
 CONFIG_FS_UBIFS=y
 CONFIG_FS_UBIFS_COMPRESSION_LZO=y
+CONFIG_FS_UBIFS_COMPRESSION_ZSTD=y
+CONFIG_FS_SQUASHFS=y
 CONFIG_PNG=y
+CONFIG_DIGEST_SHA1_ARM=y
-- 
2.30.2

Re: [PATCH 6/6] ARM: configs: imx_v7_defconfig: enable some useful options

[Marco Felsch]

Hi Ahmad,

On 22-12-22, Ahmad Fatoum wrote:
> Give the defconfig some color by enabling console colors during startup,
> i.MX6 parallel LCD driver, backlight and some more options that were
> noticed missing during debugging a skov-imx6 issue.

I'm okay with enabling a few command and the colored console per default
but the video-output use-case isn't really common and just increases the
size of the barebox image.

Regards,
  Marco

> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  arch/arm/configs/imx_v7_defconfig | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/arch/arm/configs/imx_v7_defconfig b/arch/arm/configs/imx_v7_defconfig
> index 3cbec267fc87..6110a52e486f 100644
> --- a/arch/arm/configs/imx_v7_defconfig
> +++ b/arch/arm/configs/imx_v7_defconfig
> @@ -72,11 +72,13 @@ CONFIG_BOOTM_OFTREE=y
>  CONFIG_BOOTM_OFTREE_UIMAGE=y
>  CONFIG_BLSPEC=y
>  CONFIG_CONSOLE_ACTIVATE_NONE=y
> +CONFIG_CONSOLE_ALLOW_COLOR=y
>  CONFIG_PARTITION_DISK_EFI=y
>  CONFIG_DEFAULT_ENVIRONMENT_GENERIC_NEW=y
>  CONFIG_STATE=y
>  CONFIG_BOOTCHOOSER=y
>  CONFIG_RESET_SOURCE=y
> +CONFIG_MACHINE_ID=y
>  CONFIG_FASTBOOT_CMD_OEM=y
>  CONFIG_CMD_DMESG=y
>  CONFIG_LONGHELP=y
> @@ -86,6 +88,7 @@ CONFIG_CMD_MEMINFO=y
>  CONFIG_CMD_ARM_MMUINFO=y
>  CONFIG_CMD_REGULATOR=y
>  CONFIG_CMD_MMC_EXTCSD=y
> +CONFIG_CMD_FCB=y
>  # CONFIG_CMD_BOOTU is not set
>  CONFIG_CMD_GO=y
>  CONFIG_CMD_RESET=y
> @@ -117,6 +120,7 @@ CONFIG_CMD_MENU=y
>  CONFIG_CMD_MENU_MANAGEMENT=y
>  CONFIG_CMD_MENUTREE=y
>  CONFIG_CMD_SPLASH=y
> +CONFIG_CMD_FBTEST=y
>  CONFIG_CMD_READLINE=y
>  CONFIG_CMD_TIMEOUT=y
>  CONFIG_CMD_CRC=y
> @@ -149,9 +153,14 @@ CONFIG_OF_BAREBOX_DRIVERS=y
>  CONFIG_DRIVER_NET_FEC_IMX=y
>  CONFIG_AT803X_PHY=y
>  CONFIG_MICREL_PHY=y
> +CONFIG_MDIO_BITBANG=y
> +CONFIG_MDIO_GPIO=y
> +CONFIG_MDIO_BUS_MUX_GPIO=y
>  CONFIG_NET_USB=y
>  CONFIG_NET_USB_ASIX=y
>  CONFIG_NET_USB_SMSC95XX=y
> +CONFIG_I2C_GPIO=y
> +CONFIG_I2C_MUX=y
>  CONFIG_MTD=y
>  CONFIG_MTD_RAW_DEVICE=y
>  CONFIG_MTD_DATAFLASH=y
> @@ -180,8 +189,12 @@ CONFIG_VIDEO=y
>  CONFIG_DRIVER_VIDEO_IMX_IPUV3=y
>  CONFIG_DRIVER_VIDEO_IMX_IPUV3_LVDS=y
>  CONFIG_DRIVER_VIDEO_IMX_IPUV3_HDMI=y
> +CONFIG_DRIVER_VIDEO_IMX_IPUV3_PARALLEL=y
>  CONFIG_DRIVER_VIDEO_SIMPLEFB=y
>  CONFIG_DRIVER_VIDEO_EDID=y
> +CONFIG_DRIVER_VIDEO_BACKLIGHT=y
> +CONFIG_DRIVER_VIDEO_BACKLIGHT_PWM=y
> +CONFIG_DRIVER_VIDEO_SIMPLE_PANEL=y
>  CONFIG_MCI=y
>  CONFIG_MCI_MMC_BOOT_PARTITIONS=y
>  CONFIG_MCI_IMX_ESDHC=y
> @@ -219,4 +232,7 @@ CONFIG_FS_FAT_WRITE=y
>  CONFIG_FS_FAT_LFN=y
>  CONFIG_FS_UBIFS=y
>  CONFIG_FS_UBIFS_COMPRESSION_LZO=y
> +CONFIG_FS_UBIFS_COMPRESSION_ZSTD=y
> +CONFIG_FS_SQUASHFS=y
>  CONFIG_PNG=y
> +CONFIG_DIGEST_SHA1_ARM=y
> -- 
> 2.30.2
> 
> 
> 

Re: [PATCH 6/6] ARM: configs: imx_v7_defconfig: enable some useful options

[Sascha Hauer]

On Fri, Dec 23, 2022 at 10:10:48AM +0100, Marco Felsch wrote:
> Hi Ahmad,
> 
> On 22-12-22, Ahmad Fatoum wrote:
> > Give the defconfig some color by enabling console colors during startup,
> > i.MX6 parallel LCD driver, backlight and some more options that were
> > noticed missing during debugging a skov-imx6 issue.
> 
> I'm okay with enabling a few command and the colored console per default
> but the video-output use-case isn't really common and just increases the
> size of the barebox image.

This patch indeed increases the binary size, like for example
barebox-zii-vf610-dev.img from 743788 bytes to 790858 bytes.
However, the enabled video options are only responsible for a small
fraction of it. With disabling the video options again the image
size shrinks to 788738 bytes.

I've always seen the imx_*_defconfig files as mostly full featured
configs to get good compile coverage and a barebox that just works
for as many cases as possible. From that perspective I think it's ok
to add more options.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 6/6] ARM: configs: imx_v7_defconfig: enable some useful options

[Ahmad Fatoum]

On 04.01.23 13:01, Sascha Hauer wrote:
> On Fri, Dec 23, 2022 at 10:10:48AM +0100, Marco Felsch wrote:
>> Hi Ahmad,
>>
>> On 22-12-22, Ahmad Fatoum wrote:
>>> Give the defconfig some color by enabling console colors during startup,
>>> i.MX6 parallel LCD driver, backlight and some more options that were
>>> noticed missing during debugging a skov-imx6 issue.
>>
>> I'm okay with enabling a few command and the colored console per default
>> but the video-output use-case isn't really common and just increases the
>> size of the barebox image.
> 
> This patch indeed increases the binary size, like for example
> barebox-zii-vf610-dev.img from 743788 bytes to 790858 bytes.
> However, the enabled video options are only responsible for a small
> fraction of it. With disabling the video options again the image
> size shrinks to 788738 bytes.
> 
> I've always seen the imx_*_defconfig files as mostly full featured
> configs to get good compile coverage and a barebox that just works
> for as many cases as possible. From that perspective I think it's ok
> to add more options.

Feel free to pick 1-4/6 and I'll look again into 5-6 and resend
as appropriate.

> 
> Sascha
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 6/6] ARM: configs: imx_v7_defconfig: enable some useful options

[Sascha Hauer]

On Wed, Jan 04, 2023 at 01:04:59PM +0100, Ahmad Fatoum wrote:
> On 04.01.23 13:01, Sascha Hauer wrote:
> > On Fri, Dec 23, 2022 at 10:10:48AM +0100, Marco Felsch wrote:
> >> Hi Ahmad,
> >>
> >> On 22-12-22, Ahmad Fatoum wrote:
> >>> Give the defconfig some color by enabling console colors during startup,
> >>> i.MX6 parallel LCD driver, backlight and some more options that were
> >>> noticed missing during debugging a skov-imx6 issue.
> >>
> >> I'm okay with enabling a few command and the colored console per default
> >> but the video-output use-case isn't really common and just increases the
> >> size of the barebox image.
> > 
> > This patch indeed increases the binary size, like for example
> > barebox-zii-vf610-dev.img from 743788 bytes to 790858 bytes.
> > However, the enabled video options are only responsible for a small
> > fraction of it. With disabling the video options again the image
> > size shrinks to 788738 bytes.
> > 
> > I've always seen the imx_*_defconfig files as mostly full featured
> > configs to get good compile coverage and a barebox that just works
> > for as many cases as possible. From that perspective I think it's ok
> > to add more options.
> 
> Feel free to pick 1-4/6 and I'll look again into 5-6 and resend
> as appropriate.

Picked 1-4/6 and 6/6.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |