diff mbox series

mach-snapdragon: of_fixup: fix condition check in ft_board_setup()

Message ID 20250331104327.321339-1-caleb.connolly@linaro.org
State New
Headers show
Series mach-snapdragon: of_fixup: fix condition check in ft_board_setup() | expand

Commit Message

Caleb Connolly March 31, 2025, 10:43 a.m. UTC 
The fdt_node_check_compatible() function returns 0 on success which is
pretty confusing, and we were using it wrong!

Invert the condition check and refactor things to be more readable.

Additionally, add the check for the RB1 which needs the same fixup as
the RB2.

Reported-by: Sam Day <me@samcday.com>
Fixes: e64503f1fcdf ("mach-snapdragon: implement ft_board_setup() for USB role selection")
Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
---
 arch/arm/mach-snapdragon/of_fixup.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

Comments

Alexey Minnekhanov March 31, 2025, 10:47 a.m. UTC | #1 
On 3/31/25 13:43, Caleb Connolly via groups.io wrote:
> The fdt_node_check_compatible() function returns 0 on success which is
> pretty confusing, and we were using it wrong!
> 
> Invert the condition check and refactor things to be more readable.
> 
> Additionally, add the check for the RB1 which needs the same fixup as
> the RB2.
> 
> Reported-by: Sam Day <me@samcday.com>
> Fixes: e64503f1fcdf ("mach-snapdragon: implement ft_board_setup() for USB role selection")
> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
> ---
>   arch/arm/mach-snapdragon/of_fixup.c | 16 ++++++++--------
>   1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/arm/mach-snapdragon/of_fixup.c b/arch/arm/mach-snapdragon/of_fixup.c
> index 55368dd43b66..588c92c5cc91 100644
> --- a/arch/arm/mach-snapdragon/of_fixup.c
> +++ b/arch/arm/mach-snapdragon/of_fixup.c
> @@ -160,16 +160,16 @@ int ft_board_setup(void *blob, struct bd_info __maybe_unused *bd)
>   {
>   	struct fdt_header *fdt = blob;
>   	int node;
>   
> -	/* We only want to do this fix-up for the RB1 board, quick return for all others */
> -	if (!fdt_node_check_compatible(fdt, 0, "qcom,qrb4210-rb2"))
> -		return 0;
> -
> -	fdt_for_each_node_by_compatible(node, blob, 0, "snps,dwc3") {
> -		log_debug("%s: Setting 'dr_mode' to OTG\n", fdt_get_name(blob, node, NULL));
> -		fdt_setprop_string(fdt, node, "dr_mode", "otg");
> -		break;
> +	/* On RB1/2 we need to fix-up the dr_mode */
> +	if (!fdt_node_check_compatible(fdt, 0, "qcom,qrb4210-rb2") ||
> +	    !fdt_node_check_compatible(fdt, 0, "qcom,qrb2210-rb1")) {
> +		fdt_for_each_node_by_compatible(node, blob, 0, "snps,dwc3") {
> +			log_debug("%s: Setting 'dr_mode' to OTG\n", fdt_get_name(blob, node, NULL));
> +			fdt_setprop_string(fdt, node, "dr_mode", "otg");
> +			break;
> +		}
>   	}
>   
>   	return 0;
>   }

This fixes USB mode on all my sdm660/636 boards that should only use 
gadget mode.

Tested-by: Alexey Minnekhanov <alexeymin@postmarketos.org>

---
Regards,
Alexey Minnekhanov
Sam Day March 31, 2025, 10:50 a.m. UTC | #2 
Hey Caleb,

On Monday, 31 March 2025 at 12:43, Caleb Connolly <caleb.connolly@linaro.org> wrote:

> 
> 
> The fdt_node_check_compatible() function returns 0 on success which is
> pretty confusing, and we were using it wrong!
> 
> Invert the condition check and refactor things to be more readable.
> 
> Additionally, add the check for the RB1 which needs the same fixup as
> the RB2.
> 
> Reported-by: Sam Day me@samcday.com
> 
> Fixes: e64503f1fcdf ("mach-snapdragon: implement ft_board_setup() for USB role selection")
> Signed-off-by: Caleb Connolly caleb.connolly@linaro.org
> 
> ---
> arch/arm/mach-snapdragon/of_fixup.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/arm/mach-snapdragon/of_fixup.c b/arch/arm/mach-snapdragon/of_fixup.c
> index 55368dd43b66..588c92c5cc91 100644
> --- a/arch/arm/mach-snapdragon/of_fixup.c
> +++ b/arch/arm/mach-snapdragon/of_fixup.c
> @@ -160,16 +160,16 @@ int ft_board_setup(void *blob, struct bd_info __maybe_unused *bd)
> {
> struct fdt_header fdt = blob;
> int node;
> 
> - / We only want to do this fix-up for the RB1 board, quick return for all others /
> - if (!fdt_node_check_compatible(fdt, 0, "qcom,qrb4210-rb2"))
> - return 0;
> -
> - fdt_for_each_node_by_compatible(node, blob, 0, "snps,dwc3") {
> - log_debug("%s: Setting 'dr_mode' to OTG\n", fdt_get_name(blob, node, NULL));
> - fdt_setprop_string(fdt, node, "dr_mode", "otg");
> - break;
> + / On RB1/2 we need to fix-up the dr_mode */
> + if (!fdt_node_check_compatible(fdt, 0, "qcom,qrb4210-rb2") ||
> + !fdt_node_check_compatible(fdt, 0, "qcom,qrb2210-rb1")) {
> + fdt_for_each_node_by_compatible(node, blob, 0, "snps,dwc3") {
> + log_debug("%s: Setting 'dr_mode' to OTG\n", fdt_get_name(blob, node, NULL));
> + fdt_setprop_string(fdt, node, "dr_mode", "otg");
> + break;
> + }
> }
> 
> return 0;
> }
> --
> 2.49.0

Thanks, this fixes USB peripheral mode on my fajita.

Tested-by: Sam Day <me@samcday.com>

-Sam
Caleb Connolly March 31, 2025, 1:34 p.m. UTC | #3 
On Mon, 31 Mar 2025 12:43:18 +0200, Caleb Connolly wrote:
> The fdt_node_check_compatible() function returns 0 on success which is
> pretty confusing, and we were using it wrong!
> 
> Invert the condition check and refactor things to be more readable.
> 
> Additionally, add the check for the RB1 which needs the same fixup as
> the RB2.
> 
> [...]

Applied, thanks!

[1/1] mach-snapdragon: of_fixup: fix condition check in ft_board_setup()
      https://source.denx.de/u-boot/custodians/u-boot-snapdragon/-/commit/462471e596e0

Best regards,
diff mbox series

Patch

diff --git a/arch/arm/mach-snapdragon/of_fixup.c b/arch/arm/mach-snapdragon/of_fixup.c
index 55368dd43b66..588c92c5cc91 100644
--- a/arch/arm/mach-snapdragon/of_fixup.c
+++ b/arch/arm/mach-snapdragon/of_fixup.c
@@ -160,16 +160,16 @@  int ft_board_setup(void *blob, struct bd_info __maybe_unused *bd)
 {
 	struct fdt_header *fdt = blob;
 	int node;
 
-	/* We only want to do this fix-up for the RB1 board, quick return for all others */
-	if (!fdt_node_check_compatible(fdt, 0, "qcom,qrb4210-rb2"))
-		return 0;
-
-	fdt_for_each_node_by_compatible(node, blob, 0, "snps,dwc3") {
-		log_debug("%s: Setting 'dr_mode' to OTG\n", fdt_get_name(blob, node, NULL));
-		fdt_setprop_string(fdt, node, "dr_mode", "otg");
-		break;
+	/* On RB1/2 we need to fix-up the dr_mode */
+	if (!fdt_node_check_compatible(fdt, 0, "qcom,qrb4210-rb2") ||
+	    !fdt_node_check_compatible(fdt, 0, "qcom,qrb2210-rb1")) {
+		fdt_for_each_node_by_compatible(node, blob, 0, "snps,dwc3") {
+			log_debug("%s: Setting 'dr_mode' to OTG\n", fdt_get_name(blob, node, NULL));
+			fdt_setprop_string(fdt, node, "dr_mode", "otg");
+			break;
+		}
 	}
 
 	return 0;
 }