Skip to contentSkip to conversation

Make usb device-tree fixup independent of USB controller

PATCH Superseded Sriram Dash 2 participants 18 messages Mar 1, 2016 – Mar 3, 2016

Subsystems (2)
USB USB ehci driver

v3 of 6 Superseded Latest is v6 →

lore
Share & tools
lore

Sriram Dash

author
#0 lore raw

v3 cover Make usb device-tree fixup independent of USB controller

Makes usb device-tree fixup independent of Controller type. This enables the usage of device-tree fixup as a common framework for EHCI and XHCI controllers

Sriram Dash (3):
board:freescale:common: Move device-tree fixup framework to common
file
board:freescale:usb: Remove code duplication for fdt_usb_get_node_type board:freescale:usb: Add device-tree fixup support for xhci controller

 board/freescale/common/Makefile |   3 +
 board/freescale/common/usb.c    | 194 +++++++++++++++++++++++++++++++++++++++
 drivers/usb/host/ehci-fsl.c     | 195 ----------------------------------------
 include/fdt_support.h           |   4 +-
 4 files changed, 199 insertions(+), 197 deletions(-)
 create mode 100644 board/freescale/common/usb.c
Signature · 1 line
2.1.0
author
#1 lore raw

PATCH v3 1/3 board:freescale:common: Move device-tree fixup framework to common file

Move usb device-tree fixup framework from ehci-fsl.c to common place so that it can be used by other drivers as well (xhci-fsl.c).

Signed-off-by Ramneek Mehresh <[email protected]>
Signed-off-by Sriram Dash <[email protected]>
---
 board/freescale/common/Makefile                    |   2 +
 .../ehci-fsl.c => board/freescale/common/usb.c     | 160 +----------------
 drivers/usb/host/ehci-fsl.c                        | 195 ---------------------
 3 files changed, 3 insertions(+), 354 deletions(-)
 copy drivers/usb/host/ehci-fsl.c => board/freescale/common/usb.c (53%)
board/freescale/common/Makefile +2 -0
index be114ce..62de45c 100644
@@ -13,6 +13,8 @@ MINIMAL=y
 endif
 endif
 
+obj-$(CONFIG_USB_EHCI_FSL) += usb.o
+
 ifdef MINIMAL
 # necessary to create built-in.o
 obj- := __dummy__.o
board/freescale/common/usb.c renamed from drivers/usb/host/ehci-fsl.c +1 -159

178 lines not on this page · raw message

drivers/usb/host/ehci-fsl.c +0 -195

198 lines not on this page · raw message

Signature · 1 line
2.1.0
#2 lore raw

PATCH v3 1/3 board:freescale:common: Move device-tree fixup framework to common file

Quoted 11 lines · Sriram Dash

Move usb device-tree fixup framework from ehci-fsl.c to common place so that it can be used by other drivers as well (xhci-fsl.c).

Signed-off-by Ramneek Mehresh <[email protected]>
Signed-off-by Sriram Dash <[email protected]>
---
 board/freescale/common/Makefile                    |   2 +
 .../ehci-fsl.c => board/freescale/common/usb.c     | 160 +----------------
 drivers/usb/host/ehci-fsl.c                        | 195 ---------------------
 3 files changed, 3 insertions(+), 354 deletions(-)
 copy drivers/usb/host/ehci-fsl.c => board/freescale/common/usb.c (53%)

Where is the changelog ?

Quoted 21 lines
board/freescale/common/Makefile +2 -0
index be114ce..62de45c 100644
@@ -13,6 +13,8 @@ MINIMAL=y
 endif
 endif
 
+obj-$(CONFIG_USB_EHCI_FSL) += usb.o
+
 ifdef MINIMAL
 # necessary to create built-in.o
 obj- := __dummy__.o
board/freescale/common/usb.c renamed from drivers/usb/host/ehci-fsl.c +0 -0
similarity index 53%
copy from drivers/usb/host/ehci-fsl.c
copy to board/freescale/common/usb.c
index 97b7f14..85cb1bf 100644

board/freescale/common/usb.c line 2

Quoted 7 lines
board/freescale/common/usb.c renamed from drivers/usb/host/ehci-fsl.c +1 -5
@@ -1,9 +1,5 @@
 /*
- * (C) Copyright 2009, 2011 Freescale Semiconductor, Inc.
- *
- * (C) Copyright 2008, Excito Elektronik i Sk=E5ne AB
- *
- * Author: Tor Krill tor at excito.com
+ * (C) Copyright 2016 Freescale Semiconductor, Inc.

What's with this copyright change here ?

Quoted 7 lines
board/freescale/common/usb.c renamed from drivers/usb/host/ehci-fsl.c +0 -1
  * SPDX-License-Identifier:	GPL-2.0+
  */

@@ -17,164 +13,11 @@
 #include <fsl_usb.h>
 #include <fdt_support.h>
 
-#include "ehci.h"

[...]

Signature · 2 lines
Best regards,
Marek Vasut
author
#3 lore raw

PATCH v3 1/3 board:freescale:common: Move device-tree fixup framework to common file

Quoted 25 lines

-----Original Message-----
From: Marek Vasut [mailto:marex at denx.de]
Sent: Wednesday, March 02, 2016 3:43 AM
To: Sriram Dash <[email protected]>; u-boot at lists.denx.de
Cc: york sun <[email protected]>; Ramneek Mehresh <[email protected]>; Rajesh Bhagat <[email protected]>; Tom Rini <[email protected]>
Subject: Re: [PATCH v3 1/3] board:freescale:common: Move device-tree fixup framework to common file

Quoted 11 lines · Sriram Dash

Move usb device-tree fixup framework from ehci-fsl.c to common place so that it can be used by other drivers as well (xhci-fsl.c).

Signed-off-by Ramneek Mehresh <[email protected]>
Signed-off-by Sriram Dash <[email protected]>
---
 board/freescale/common/Makefile                    |   2 +
 .../ehci-fsl.c => board/freescale/common/usb.c     | 160 +----------------
 drivers/usb/host/ehci-fsl.c                        | 195 ---------------------
 3 files changed, 3 insertions(+), 354 deletions(-)  copy

drivers/usb/host/ehci-fsl.c => board/freescale/common/usb.c (53%)

Quoted 1 line

Where is the changelog ?

Will include changelog for v2 and v3 in v4.

Quoted 22 lines
Quoted 22 lines
(quoted diff) +0 -0

b/board/freescale/common/Makefile index be114ce..62de45c 100644

board/freescale/common/Makefile +2 -0
@@ -13,6 +13,8 @@ MINIMAL=y
 endif
 endif
 
+obj-$(CONFIG_USB_EHCI_FSL) += usb.o
+
 ifdef MINIMAL
 # necessary to create built-in.o
 obj- := __dummy__.o
(quoted diff) +0 -0

b/board/freescale/common/usb.c similarity index 53% copy from drivers/usb/host/ehci-fsl.c copy to board/freescale/common/usb.c index 97b7f14..85cb1bf 100644

board/freescale/common/usb.c renamed from drivers/usb/host/ehci-fsl.c +0 -0
@@ -1,9 +1,5 @@
 /*
Quoted 8 lines

board/freescale/common/usb.c line 2

Quoted 6 lines
board/freescale/common/usb.c renamed from drivers/usb/host/ehci-fsl.c +1 -5
- * (C) Copyright 2009, 2011 Freescale Semiconductor, Inc.
- *
- * (C) Copyright 2008, Excito Elektronik i Sk=E5ne AB
- *
- * Author: Tor Krill tor at excito.com
+ * (C) Copyright 2016 Freescale Semiconductor, Inc.

What's with this copyright change here ?

It is a new file named common/usb.c. Shall I include the complete ehci-fsl.c copyright information in the new file?

Quoted 7 lines
Quoted 7 lines
board/freescale/common/usb.c renamed from drivers/usb/host/ehci-fsl.c +0 -0
  * SPDX-License-Identifier:	GPL-2.0+
  */

@@ -17,164 +13,11 @@
 #include <fsl_usb.h>
 #include <fdt_support.h>
Quoted 6 lines

board/freescale/common/usb.c line 20

Quoted 1 line
board/freescale/common/usb.c renamed from drivers/usb/host/ehci-fsl.c +0 -1
-#include "ehci.h"

[...]

--
Best regards,
Marek Vasut

Best Regards,
Sriram

#4 lore raw

PATCH v3 1/3 board:freescale:common: Move device-tree fixup framework to common file

Quoted 60 lines · Sriram Dash
Quoted 25 lines

-----Original Message-----
From: Marek Vasut [mailto:marex at denx.de]
Sent: Wednesday, March 02, 2016 3:43 AM
To: Sriram Dash <[email protected]>; u-boot at lists.denx.de
Cc: york sun <[email protected]>; Ramneek Mehresh <[email protected]>; Rajesh Bhagat <[email protected]>; Tom Rini <[email protected]>
Subject: Re: [PATCH v3 1/3] board:freescale:common: Move device-tree fixup framework to common file

Quoted 11 lines · Sriram Dash

Move usb device-tree fixup framework from ehci-fsl.c to common place so that it can be used by other drivers as well (xhci-fsl.c).

Signed-off-by Ramneek Mehresh <[email protected]>
Signed-off-by Sriram Dash <[email protected]>
---
 board/freescale/common/Makefile                    |   2 +
 .../ehci-fsl.c => board/freescale/common/usb.c     | 160 +----------------
 drivers/usb/host/ehci-fsl.c                        | 195 ---------------------
 3 files changed, 3 insertions(+), 354 deletions(-)  copy

drivers/usb/host/ehci-fsl.c => board/freescale/common/usb.c (53%)

Quoted 1 line

Where is the changelog ?

Will include changelog for v2 and v3 in v4.

Quoted 22 lines
Quoted 22 lines
(quoted diff) +0 -0

b/board/freescale/common/Makefile index be114ce..62de45c 100644

board/freescale/common/Makefile +2 -0
@@ -13,6 +13,8 @@ MINIMAL=y
 endif
 endif
 
+obj-$(CONFIG_USB_EHCI_FSL) += usb.o
+
 ifdef MINIMAL
 # necessary to create built-in.o
 obj- := __dummy__.o
(quoted diff) +0 -0

b/board/freescale/common/usb.c similarity index 53% copy from drivers/usb/host/ehci-fsl.c copy to board/freescale/common/usb.c index 97b7f14..85cb1bf 100644

board/freescale/common/usb.c renamed from drivers/usb/host/ehci-fsl.c +0 -0
@@ -1,9 +1,5 @@
 /*
Quoted 8 lines

board/freescale/common/usb.c line 2

Quoted 6 lines
board/freescale/common/usb.c renamed from drivers/usb/host/ehci-fsl.c +1 -5
- * (C) Copyright 2009, 2011 Freescale Semiconductor, Inc.
- *
- * (C) Copyright 2008, Excito Elektronik i Sk=E5ne AB
- *
- * Author: Tor Krill tor at excito.com
+ * (C) Copyright 2016 Freescale Semiconductor, Inc.

What's with this copyright change here ?

Quoted 1 line

It is a new file named common/usb.c. Shall I include the complete ehci-fsl.c copyright information in the new file?

There is already a file named common/usb.c , you surely mean board/freescale/common/usb.c , yes ?

According to git, it's not a new file:

b/board/freescale/common/usb.c similarity index 53% copy from drivers/usb/host/ehci-fsl.c copy to board/freescale/common/usb.c

so yes, it should retain all copyright info.

And now that I am looking at it, I would much rather see the fixup bits in drivers/usb/host/ than some board-specific file. You can very well put those into fsl-dt-fixup.c or whatever there.

Quoted 17 lines
Quoted 7 lines
Quoted 7 lines
board/freescale/common/usb.c renamed from drivers/usb/host/ehci-fsl.c +0 -0
  * SPDX-License-Identifier:	GPL-2.0+
  */

@@ -17,164 +13,11 @@
 #include <fsl_usb.h>
 #include <fdt_support.h>
Quoted 6 lines

board/freescale/common/usb.c line 20

Quoted 1 line
board/freescale/common/usb.c renamed from drivers/usb/host/ehci-fsl.c +0 -1
-#include "ehci.h"

[...]

--
Best regards,
Marek Vasut

Best Regards,
Sriram

Signature · 2 lines
Best regards,
Marek Vasut
author
#5 lore raw

PATCH v3 1/3 board:freescale:common: Move device-tree fixup framework to common file

Quoted 73 lines

-----Original Message-----
From: Marek Vasut [mailto:marex at denx.de]
Sent: Thursday, March 03, 2016 3:26 PM
To: Sriram Dash <[email protected]>; u-boot at lists.denx.de
Cc: york sun <[email protected]>; Ramneek Mehresh <[email protected]>; Rajesh Bhagat <[email protected]>; Tom Rini <[email protected]>
Subject: Re: [PATCH v3 1/3] board:freescale:common: Move device-tree fixup framework to common file

Quoted 60 lines · Sriram Dash
Quoted 25 lines

-----Original Message-----
From: Marek Vasut [mailto:marex at denx.de]
Sent: Wednesday, March 02, 2016 3:43 AM
To: Sriram Dash <[email protected]>; u-boot at lists.denx.de
Cc: york sun <[email protected]>; Ramneek Mehresh <[email protected]>; Rajesh Bhagat <[email protected]>; Tom Rini <[email protected]>
Subject: Re: [PATCH v3 1/3] board:freescale:common: Move device-tree fixup framework to common file

Quoted 11 lines · Sriram Dash

Move usb device-tree fixup framework from ehci-fsl.c to common place so that it can be used by other drivers as well (xhci-fsl.c).

Signed-off-by Ramneek Mehresh <[email protected]>
Signed-off-by Sriram Dash <[email protected]>
---
 board/freescale/common/Makefile                    |   2 +
 .../ehci-fsl.c => board/freescale/common/usb.c     | 160 +----------------
 drivers/usb/host/ehci-fsl.c                        | 195 ---------------------
 3 files changed, 3 insertions(+), 354 deletions(-)  copy

drivers/usb/host/ehci-fsl.c => board/freescale/common/usb.c (53%)

Quoted 1 line

Where is the changelog ?

Will include changelog for v2 and v3 in v4.

Quoted 30 lines
Quoted 21 lines
(quoted diff) +0 -0

b/board/freescale/common/Makefile index be114ce..62de45c 100644

board/freescale/common/Makefile +2 -0
@@ -13,6 +13,8 @@ MINIMAL=y
 endif
 endif
 
+obj-$(CONFIG_USB_EHCI_FSL) += usb.o
+
 ifdef MINIMAL
 # necessary to create built-in.o
 obj- := __dummy__.o
(quoted diff) +0 -0

b/board/freescale/common/usb.c similarity index 53% copy from drivers/usb/host/ehci-fsl.c copy to board/freescale/common/usb.c index 97b7f14..85cb1bf 100644

board/freescale/common/usb.c line 2

Quoted 7 lines
board/freescale/common/usb.c renamed from drivers/usb/host/ehci-fsl.c +1 -5
@@ -1,9 +1,5 @@
 /*
- * (C) Copyright 2009, 2011 Freescale Semiconductor, Inc.
- *
- * (C) Copyright 2008, Excito Elektronik i Sk=E5ne AB
- *
- * Author: Tor Krill tor at excito.com
+ * (C) Copyright 2016 Freescale Semiconductor, Inc.

What's with this copyright change here ?

Quoted 6 lines
Quoted 1 line

It is a new file named common/usb.c. Shall I include the complete ehci-fsl.c

copyright information in the new file?

There is already a file named common/usb.c , you surely mean board/freescale/common/usb.c , yes ?

Yes

Quoted 7 lines

According to git, it's not a new file:

b/board/freescale/common/usb.c similarity index 53% copy from drivers/usb/host/ehci-fsl.c copy to board/freescale/common/usb.c

so yes, it should retain all copyright info.

Ok, We will retain copyright info in v4.

Quoted 4 lines

And now that I am looking at it, I would much rather see the fixup bits in drivers/usb/host/ than some board-specific file. You can very well put those into fsl-dt-fixup.c or whatever there.

drivers/usb/host/ was a good option, but we want to make it independent of host and gadget. So, whenever there is a specific requirement for freescale boards, it will use the same from board: freescale: common: usb

Else, another option is to have drivers/usb/common/fsl-dt-fixup.c. What do you say?

Quoted 22 lines
Quoted 10 lines
Quoted 10 lines
Quoted 8 lines
board/freescale/common/usb.c renamed from drivers/usb/host/ehci-fsl.c +0 -1
  * SPDX-License-Identifier:	GPL-2.0+
  */

@@ -17,164 +13,11 @@
 #include <fsl_usb.h>
 #include <fdt_support.h>
 
-#include "ehci.h"

[...]

Quoted 7 lines
Quoted 3 lines

--
Best regards,
Marek Vasut

Best Regards,
Sriram

--
Best regards,
Marek Vasut

#6 lore raw

PATCH v3 1/3 board:freescale:common: Move device-tree fixup framework to common file

Quoted 97 lines · Sriram Dash
Quoted 73 lines

-----Original Message-----
From: Marek Vasut [mailto:marex at denx.de]
Sent: Thursday, March 03, 2016 3:26 PM
To: Sriram Dash <[email protected]>; u-boot at lists.denx.de
Cc: york sun <[email protected]>; Ramneek Mehresh <[email protected]>; Rajesh Bhagat <[email protected]>; Tom Rini <[email protected]>
Subject: Re: [PATCH v3 1/3] board:freescale:common: Move device-tree fixup framework to common file

Quoted 60 lines · Sriram Dash
Quoted 25 lines

-----Original Message-----
From: Marek Vasut [mailto:marex at denx.de]
Sent: Wednesday, March 02, 2016 3:43 AM
To: Sriram Dash <[email protected]>; u-boot at lists.denx.de
Cc: york sun <[email protected]>; Ramneek Mehresh <[email protected]>; Rajesh Bhagat <[email protected]>; Tom Rini <[email protected]>
Subject: Re: [PATCH v3 1/3] board:freescale:common: Move device-tree fixup framework to common file

Quoted 11 lines · Sriram Dash

Move usb device-tree fixup framework from ehci-fsl.c to common place so that it can be used by other drivers as well (xhci-fsl.c).

Signed-off-by Ramneek Mehresh <[email protected]>
Signed-off-by Sriram Dash <[email protected]>
---
 board/freescale/common/Makefile                    |   2 +
 .../ehci-fsl.c => board/freescale/common/usb.c     | 160 +----------------
 drivers/usb/host/ehci-fsl.c                        | 195 ---------------------
 3 files changed, 3 insertions(+), 354 deletions(-)  copy

drivers/usb/host/ehci-fsl.c => board/freescale/common/usb.c (53%)

Quoted 1 line

Where is the changelog ?

Will include changelog for v2 and v3 in v4.

Quoted 30 lines
Quoted 21 lines
(quoted diff) +0 -0

b/board/freescale/common/Makefile index be114ce..62de45c 100644

board/freescale/common/Makefile +2 -0
@@ -13,6 +13,8 @@ MINIMAL=y
 endif
 endif
 
+obj-$(CONFIG_USB_EHCI_FSL) += usb.o
+
 ifdef MINIMAL
 # necessary to create built-in.o
 obj- := __dummy__.o
(quoted diff) +0 -0

b/board/freescale/common/usb.c similarity index 53% copy from drivers/usb/host/ehci-fsl.c copy to board/freescale/common/usb.c index 97b7f14..85cb1bf 100644

board/freescale/common/usb.c line 2

Quoted 7 lines
board/freescale/common/usb.c renamed from drivers/usb/host/ehci-fsl.c +1 -5
@@ -1,9 +1,5 @@
 /*
- * (C) Copyright 2009, 2011 Freescale Semiconductor, Inc.
- *
- * (C) Copyright 2008, Excito Elektronik i Sk=E5ne AB
- *
- * Author: Tor Krill tor at excito.com
+ * (C) Copyright 2016 Freescale Semiconductor, Inc.

What's with this copyright change here ?

Quoted 6 lines
Quoted 1 line

It is a new file named common/usb.c. Shall I include the complete ehci-fsl.c

copyright information in the new file?

There is already a file named common/usb.c , you surely mean board/freescale/common/usb.c , yes ?

Yes

Quoted 7 lines

According to git, it's not a new file:

b/board/freescale/common/usb.c similarity index 53% copy from drivers/usb/host/ehci-fsl.c copy to board/freescale/common/usb.c

so yes, it should retain all copyright info.

Ok, We will retain copyright info in v4.

Quoted 4 lines

And now that I am looking at it, I would much rather see the fixup bits in drivers/usb/host/ than some board-specific file. You can very well put those into fsl-dt-fixup.c or whatever there.

Quoted 6 lines

drivers/usb/host/ was a good option, but we want to make it independent of host and gadget. So, whenever there is a specific requirement for freescale boards, it will use the same from board: freescale: common: usb

Else, another option is to have drivers/usb/common/fsl-dt-fixup.c. What do you say?

That is fine. Moving it to board code would make it problematic to convert to DM afterward, so I want to prevent that.

Quoted 22 lines
Quoted 22 lines
Quoted 10 lines
Quoted 10 lines
Quoted 8 lines
board/freescale/common/usb.c renamed from drivers/usb/host/ehci-fsl.c +0 -1
  * SPDX-License-Identifier:	GPL-2.0+
  */

@@ -17,164 +13,11 @@
 #include <fsl_usb.h>
 #include <fdt_support.h>
 
-#include "ehci.h"

[...]

Quoted 7 lines
Quoted 3 lines

--
Best regards,
Marek Vasut

Best Regards,
Sriram

--
Best regards,
Marek Vasut

Signature · 2 lines
Best regards,
Marek Vasut
author
#7 lore raw

PATCH v3 2/3 board:freescale:usb: Remove code duplication for fdt_usb_get_node_type

In reply to Sriram Dash

Call fdt_usb_get_node_type() from fdt_fixup_usb_mode_phy_type() to avoid code duplication.

Signed-off-by Ramneek Mehresh <[email protected]>
Signed-off-by Sriram Dash <[email protected]>
---
 board/freescale/common/usb.c | 72 ++++++++++++++++++--------------------------
 1 file changed, 29 insertions(+), 43 deletions(-)
board/freescale/common/usb.c +29 -43
index 85cb1bf..d815dc1 100644
@@ -18,33 +18,45 @@
 #define CONFIG_USB_MAX_CONTROLLER_COUNT 1
 #endif
 
-static int fdt_fixup_usb_mode_phy_type(void *blob, const char *mode,
-				       const char *phy_type, int start_offset)
+static const char *fdt_usb_get_node_type(void *blob, int start_offset,
+					 int *node_offset)
 {
 	const char *compat_dr = "fsl-usb2-dr";
 	const char *compat_mph = "fsl-usb2-mph";
-	const char *prop_mode = "dr_mode";
-	const char *prop_type = "phy_type";
 	const char *node_type = NULL;
-	int node_offset;
-	int err;
 
-	node_offset = fdt_node_offset_by_compatible(blob,
-						    start_offset, compat_mph);
-	if (node_offset < 0) {
-		node_offset = fdt_node_offset_by_compatible(blob,
-							    start_offset,
-							    compat_dr);
-		if (node_offset < 0) {
-			printf("WARNING: could not find compatible node: %s",
-			       fdt_strerror(node_offset));
-			return -1;
+	*node_offset = fdt_node_offset_by_compatible(blob, start_offset,
+						     compat_mph);
+	if (*node_offset < 0) {
+		*node_offset = fdt_node_offset_by_compatible(blob,
+							     start_offset,
+							     compat_dr);
+		if (*node_offset < 0) {
+			printf("ERROR: could not find compatible node: %s\n",
+			       fdt_strerror(*node_offset));
+		} else {
+			node_type = compat_dr;
 		}
-		node_type = compat_dr;
 	} else {
 		node_type = compat_mph;
 	}
 
+	return node_type;
+}
+
+static int fdt_fixup_usb_mode_phy_type(void *blob, const char *mode,
+					const char *phy_type, int start_offset)
+{
+	const char *prop_mode = "dr_mode";
+	const char *prop_type = "phy_type";
+	const char *node_type = NULL;
+	int node_offset;
+	int err;
+
+	node_type = fdt_usb_get_node_type(blob, start_offset, &node_offset);
+	if (!node_type)
+		return -1;
+
 	if (mode) {
 		err = fdt_setprop(blob, node_offset, prop_mode, mode,
 				  strlen(mode) + 1);

@@ -64,32 +76,6 @@ static int fdt_fixup_usb_mode_phy_type(void *blob, const char *mode,
 	return node_offset;
 }
 
-static const char *fdt_usb_get_node_type(void *blob, int start_offset,
-					 int *node_offset)
-{
-	const char *compat_dr = "fsl-usb2-dr";
-	const char *compat_mph = "fsl-usb2-mph";
-	const char *node_type = NULL;
-
-	*node_offset = fdt_node_offset_by_compatible(blob, start_offset,
-						     compat_mph);
-	if (*node_offset < 0) {
-		*node_offset = fdt_node_offset_by_compatible(blob,
-							     start_offset,
-							     compat_dr);
-		if (*node_offset < 0) {
-			printf("ERROR: could not find compatible node: %s\n",
-			       fdt_strerror(*node_offset));
-		} else {
-			node_type = compat_dr;
-		}
-	} else {
-		node_type = compat_mph;
-	}
-
-	return node_type;
-}
-
 static int fdt_fixup_usb_erratum(void *blob, const char *prop_erratum,
 				 int start_offset)
 {
Signature · 1 line
2.1.0
#8 lore raw

PATCH v3 2/3 board:freescale:usb: Remove code duplication for fdt_usb_get_node_type

Quoted 48 lines · Sriram Dash

Call fdt_usb_get_node_type() from fdt_fixup_usb_mode_phy_type() to avoid code duplication.

Signed-off-by Ramneek Mehresh <[email protected]>
Signed-off-by Sriram Dash <[email protected]>
---
 board/freescale/common/usb.c | 72 ++++++++++++++++++--------------------------
 1 file changed, 29 insertions(+), 43 deletions(-)
board/freescale/common/usb.c +10 -16
index 85cb1bf..d815dc1 100644
@@ -18,33 +18,45 @@
 #define CONFIG_USB_MAX_CONTROLLER_COUNT 1
 #endif
 
-static int fdt_fixup_usb_mode_phy_type(void *blob, const char *mode,
-				       const char *phy_type, int start_offset)
+static const char *fdt_usb_get_node_type(void *blob, int start_offset,
+					 int *node_offset)
 {
 	const char *compat_dr = "fsl-usb2-dr";
 	const char *compat_mph = "fsl-usb2-mph";
-	const char *prop_mode = "dr_mode";
-	const char *prop_type = "phy_type";
 	const char *node_type = NULL;
-	int node_offset;
-	int err;
 
-	node_offset = fdt_node_offset_by_compatible(blob,
-						    start_offset, compat_mph);
-	if (node_offset < 0) {
-		node_offset = fdt_node_offset_by_compatible(blob,
-							    start_offset,
-							    compat_dr);
-		if (node_offset < 0) {
-			printf("WARNING: could not find compatible node: %s",
-			       fdt_strerror(node_offset));
-			return -1;
+	*node_offset = fdt_node_offset_by_compatible(blob, start_offset,
+						     compat_mph);
+	if (*node_offset < 0) {
+		*node_offset = fdt_node_offset_by_compatible(blob,
+							     start_offset,
+							     compat_dr);
+		if (*node_offset < 0) {
+			printf("ERROR: could not find compatible node: %s\n",

board/freescale/common/usb.c line 44

Quoted 10 lines
board/freescale/common/usb.c +4 -1
+			       fdt_strerror(*node_offset));
+		} else {
+			node_type = compat_dr;
 		}
-		node_type = compat_dr;
 	} else {
 		node_type = compat_mph;
 	}
 
+	return node_type;

The function should be able to return error code. If you need to return some more stuff from the function, return it via reference.

Quoted 52 lines
board/freescale/common/usb.c +15 -26
+}
+
+static int fdt_fixup_usb_mode_phy_type(void *blob, const char *mode,
+					const char *phy_type, int start_offset)
+{
+	const char *prop_mode = "dr_mode";
+	const char *prop_type = "phy_type";
+	const char *node_type = NULL;
+	int node_offset;
+	int err;
+
+	node_type = fdt_usb_get_node_type(blob, start_offset, &node_offset);
+	if (!node_type)
+		return -1;
+
 	if (mode) {
 		err = fdt_setprop(blob, node_offset, prop_mode, mode,
 				  strlen(mode) + 1);

@@ -64,32 +76,6 @@ static int fdt_fixup_usb_mode_phy_type(void *blob, const char *mode,
 	return node_offset;
 }
 
-static const char *fdt_usb_get_node_type(void *blob, int start_offset,
-					 int *node_offset)
-{
-	const char *compat_dr = "fsl-usb2-dr";
-	const char *compat_mph = "fsl-usb2-mph";
-	const char *node_type = NULL;
-
-	*node_offset = fdt_node_offset_by_compatible(blob, start_offset,
-						     compat_mph);
-	if (*node_offset < 0) {
-		*node_offset = fdt_node_offset_by_compatible(blob,
-							     start_offset,
-							     compat_dr);
-		if (*node_offset < 0) {
-			printf("ERROR: could not find compatible node: %s\n",
-			       fdt_strerror(*node_offset));
-		} else {
-			node_type = compat_dr;
-		}
-	} else {
-		node_type = compat_mph;
-	}
-
-	return node_type;
-}
-
 static int fdt_fixup_usb_erratum(void *blob, const char *prop_erratum,
 				 int start_offset)
 {
Signature · 2 lines
Best regards,
Marek Vasut
author
#9 lore raw

PATCH v3 2/3 board:freescale:usb: Remove code duplication for fdt_usb_get_node_type

Quoted 68 lines

-----Original Message-----
From: Marek Vasut [mailto:marex at denx.de]
Sent: Wednesday, March 02, 2016 3:57 AM
To: Sriram Dash <[email protected]>; u-boot at lists.denx.de
Cc: york sun <[email protected]>; Ramneek Mehresh <[email protected]>; Rajesh Bhagat <[email protected]>
Subject: Re: [PATCH v3 2/3] board:freescale:usb: Remove code duplication for fdt_usb_get_node_type

Quoted 57 lines · Sriram Dash

Call fdt_usb_get_node_type() from fdt_fixup_usb_mode_phy_type() to avoid code duplication.

Signed-off-by Ramneek Mehresh <[email protected]>
Signed-off-by Sriram Dash <[email protected]>
---
 board/freescale/common/usb.c | 72
++++++++++++++++++--------------------------
 1 file changed, 29 insertions(+), 43 deletions(-)
(quoted diff) +0 -0

b/board/freescale/common/usb.c index 85cb1bf..d815dc1 100644

board/freescale/common/usb.c +13 -17
@@ -18,33 +18,45 @@
 #define CONFIG_USB_MAX_CONTROLLER_COUNT 1  #endif
 
-static int fdt_fixup_usb_mode_phy_type(void *blob, const char *mode,
-				       const char *phy_type, int start_offset)
+static const char *fdt_usb_get_node_type(void *blob, int start_offset,
+					 int *node_offset)
 {
 	const char *compat_dr = "fsl-usb2-dr";
 	const char *compat_mph = "fsl-usb2-mph";
-	const char *prop_mode = "dr_mode";
-	const char *prop_type = "phy_type";
 	const char *node_type = NULL;
-	int node_offset;
-	int err;
 
-	node_offset = fdt_node_offset_by_compatible(blob,
-						    start_offset, compat_mph);
-	if (node_offset < 0) {
-		node_offset = fdt_node_offset_by_compatible(blob,
-							    start_offset,
-							    compat_dr);
-		if (node_offset < 0) {
-			printf("WARNING: could not find compatible node: %s",
-			       fdt_strerror(node_offset));
-			return -1;
+	*node_offset = fdt_node_offset_by_compatible(blob, start_offset,
+						     compat_mph);
+	if (*node_offset < 0) {
+		*node_offset = fdt_node_offset_by_compatible(blob,
+							     start_offset,
+							     compat_dr);
+		if (*node_offset < 0) {
+			printf("ERROR: could not find compatible node: %s\n",
+			       fdt_strerror(*node_offset));
+		} else {
+			node_type = compat_dr;
 		}
-		node_type = compat_dr;
 	} else {
 		node_type = compat_mph;
 	}
Quoted 5 lines

board/freescale/common/usb.c line 44

Quoted 1 line
board/freescale/common/usb.c +1 -0
+	return node_type;

The function should be able to return error code. If you need to return some more stuff from the function, return it via reference.

This patch is not altering the fdt_usb_get_node_type(). It is only calling the function from fdt_fixup_usb_mode_phy_type(), to avoid code duplication.

Quoted 57 lines

board/freescale/common/usb.c static int fdt_fixup_usb_mode_phy_type(void *blob, const

Quoted 18 lines
board/freescale/common/usb.c +14 -0
+}
+
+static int fdt_fixup_usb_mode_phy_type(void *blob, const char *mode,
+					const char *phy_type, int start_offset) {
+	const char *prop_mode = "dr_mode";
+	const char *prop_type = "phy_type";
+	const char *node_type = NULL;
+	int node_offset;
+	int err;
+
+	node_type = fdt_usb_get_node_type(blob, start_offset, &node_offset);
+	if (!node_type)
+		return -1;
+
 	if (mode) {
 		err = fdt_setprop(blob, node_offset, prop_mode, mode,
 				  strlen(mode) + 1);

@@ -64,32 +76,6 @@ static int fdt_fixup_usb_mode_phy_type(void *blob, const

char *mode,

Quoted 23 lines
board/freescale/common/usb.c +0 -19
 	return node_offset;
 }
 
-static const char *fdt_usb_get_node_type(void *blob, int start_offset,
-					 int *node_offset)
-{
-	const char *compat_dr = "fsl-usb2-dr";
-	const char *compat_mph = "fsl-usb2-mph";
-	const char *node_type = NULL;
-
-	*node_offset = fdt_node_offset_by_compatible(blob, start_offset,
-						     compat_mph);
-	if (*node_offset < 0) {
-		*node_offset = fdt_node_offset_by_compatible(blob,
-							     start_offset,
-							     compat_dr);
-		if (*node_offset < 0) {
-			printf("ERROR: could not find compatible node: %s\n",
-			       fdt_strerror(*node_offset));
-		} else {
-			node_type = compat_dr;
-		}

board/freescale/common/usb.c static int fdt_fixup_usb_mode_phy_type(void *blob, const char *mode, · line 81

Quoted 10 lines
board/freescale/common/usb.c +0 -7
-	} else {
-		node_type = compat_mph;
-	}
-
-	return node_type;
-}
-
 static int fdt_fixup_usb_erratum(void *blob, const char *prop_erratum,
 				 int start_offset)
 {

--
Best regards,
Marek Vasut

#10 lore raw

PATCH v3 2/3 board:freescale:usb: Remove code duplication for fdt_usb_get_node_type

Quoted 68 lines · Sriram Dash
Quoted 68 lines

-----Original Message-----
From: Marek Vasut [mailto:marex at denx.de]
Sent: Wednesday, March 02, 2016 3:57 AM
To: Sriram Dash <[email protected]>; u-boot at lists.denx.de
Cc: york sun <[email protected]>; Ramneek Mehresh <[email protected]>; Rajesh Bhagat <[email protected]>
Subject: Re: [PATCH v3 2/3] board:freescale:usb: Remove code duplication for fdt_usb_get_node_type

Quoted 57 lines · Sriram Dash

Call fdt_usb_get_node_type() from fdt_fixup_usb_mode_phy_type() to avoid code duplication.

Signed-off-by Ramneek Mehresh <[email protected]>
Signed-off-by Sriram Dash <[email protected]>
---
 board/freescale/common/usb.c | 72
++++++++++++++++++--------------------------
 1 file changed, 29 insertions(+), 43 deletions(-)
(quoted diff) +0 -0

b/board/freescale/common/usb.c index 85cb1bf..d815dc1 100644

board/freescale/common/usb.c +13 -17
@@ -18,33 +18,45 @@
 #define CONFIG_USB_MAX_CONTROLLER_COUNT 1  #endif
 
-static int fdt_fixup_usb_mode_phy_type(void *blob, const char *mode,
-				       const char *phy_type, int start_offset)
+static const char *fdt_usb_get_node_type(void *blob, int start_offset,
+					 int *node_offset)
 {
 	const char *compat_dr = "fsl-usb2-dr";
 	const char *compat_mph = "fsl-usb2-mph";
-	const char *prop_mode = "dr_mode";
-	const char *prop_type = "phy_type";
 	const char *node_type = NULL;
-	int node_offset;
-	int err;
 
-	node_offset = fdt_node_offset_by_compatible(blob,
-						    start_offset, compat_mph);
-	if (node_offset < 0) {
-		node_offset = fdt_node_offset_by_compatible(blob,
-							    start_offset,
-							    compat_dr);
-		if (node_offset < 0) {
-			printf("WARNING: could not find compatible node: %s",
-			       fdt_strerror(node_offset));
-			return -1;
+	*node_offset = fdt_node_offset_by_compatible(blob, start_offset,
+						     compat_mph);
+	if (*node_offset < 0) {
+		*node_offset = fdt_node_offset_by_compatible(blob,
+							     start_offset,
+							     compat_dr);
+		if (*node_offset < 0) {
+			printf("ERROR: could not find compatible node: %s\n",
+			       fdt_strerror(*node_offset));
+		} else {
+			node_type = compat_dr;
 		}
-		node_type = compat_dr;
 	} else {
 		node_type = compat_mph;
 	}
Quoted 8 lines
Quoted 5 lines

board/freescale/common/usb.c line 44

Quoted 1 line
board/freescale/common/usb.c +1 -0
+	return node_type;

The function should be able to return error code. If you need to return some more stuff from the function, return it via reference.

This patch is not altering the fdt_usb_get_node_type(). It is only calling the function from fdt_fixup_usb_mode_phy_type(), to avoid code duplication.

I am not complaining about that part. I am complaining about the new function, fdt_usb_get_node_type(), which returns pointer as a return value instead of returning proper error code as a return value.

Quoted 57 lines
Quoted 57 lines

board/freescale/common/usb.c static int fdt_fixup_usb_mode_phy_type(void *blob, const

Quoted 18 lines
board/freescale/common/usb.c +14 -0
+}
+
+static int fdt_fixup_usb_mode_phy_type(void *blob, const char *mode,
+					const char *phy_type, int start_offset) {
+	const char *prop_mode = "dr_mode";
+	const char *prop_type = "phy_type";
+	const char *node_type = NULL;
+	int node_offset;
+	int err;
+
+	node_type = fdt_usb_get_node_type(blob, start_offset, &node_offset);
+	if (!node_type)
+		return -1;
+
 	if (mode) {
 		err = fdt_setprop(blob, node_offset, prop_mode, mode,
 				  strlen(mode) + 1);

@@ -64,32 +76,6 @@ static int fdt_fixup_usb_mode_phy_type(void *blob, const

char *mode,

Quoted 23 lines
board/freescale/common/usb.c +0 -19
 	return node_offset;
 }
 
-static const char *fdt_usb_get_node_type(void *blob, int start_offset,
-					 int *node_offset)
-{
-	const char *compat_dr = "fsl-usb2-dr";
-	const char *compat_mph = "fsl-usb2-mph";
-	const char *node_type = NULL;
-
-	*node_offset = fdt_node_offset_by_compatible(blob, start_offset,
-						     compat_mph);
-	if (*node_offset < 0) {
-		*node_offset = fdt_node_offset_by_compatible(blob,
-							     start_offset,
-							     compat_dr);
-		if (*node_offset < 0) {
-			printf("ERROR: could not find compatible node: %s\n",
-			       fdt_strerror(*node_offset));
-		} else {
-			node_type = compat_dr;
-		}

board/freescale/common/usb.c static int fdt_fixup_usb_mode_phy_type(void *blob, const char *mode, · line 81

Quoted 10 lines
board/freescale/common/usb.c +0 -7
-	} else {
-		node_type = compat_mph;
-	}
-
-	return node_type;
-}
-
 static int fdt_fixup_usb_erratum(void *blob, const char *prop_erratum,
 				 int start_offset)
 {

--
Best regards,
Marek Vasut

Signature · 2 lines
Best regards,
Marek Vasut
author
#11 lore raw

PATCH v3 2/3 board:freescale:usb: Remove code duplication for fdt_usb_get_node_type

Quoted 86 lines

-----Original Message-----
From: Marek Vasut [mailto:marex at denx.de]
Sent: Thursday, March 03, 2016 3:29 PM
To: Sriram Dash <[email protected]>; u-boot at lists.denx.de
Cc: york sun <[email protected]>; Ramneek Mehresh <[email protected]>; Rajesh Bhagat <[email protected]>
Subject: Re: [PATCH v3 2/3] board:freescale:usb: Remove code duplication for fdt_usb_get_node_type

Quoted 74 lines · Sriram Dash
Quoted 73 lines

-----Original Message-----
From: Marek Vasut [mailto:marex at denx.de]
Sent: Wednesday, March 02, 2016 3:57 AM
To: Sriram Dash <[email protected]>; u-boot at lists.denx.de
Cc: york sun <[email protected]>; Ramneek Mehresh <[email protected]>; Rajesh Bhagat <[email protected]>
Subject: Re: [PATCH v3 2/3] board:freescale:usb: Remove code duplication for fdt_usb_get_node_type

Quoted 48 lines · Sriram Dash

Call fdt_usb_get_node_type() from fdt_fixup_usb_mode_phy_type() to avoid code duplication.

Signed-off-by Ramneek Mehresh <[email protected]>
Signed-off-by Sriram Dash <[email protected]>
---
 board/freescale/common/usb.c | 72
++++++++++++++++++--------------------------
 1 file changed, 29 insertions(+), 43 deletions(-)
(quoted diff) +0 -0

b/board/freescale/common/usb.c index 85cb1bf..d815dc1 100644

board/freescale/common/usb.c +10 -16
@@ -18,33 +18,45 @@
 #define CONFIG_USB_MAX_CONTROLLER_COUNT 1  #endif
 
-static int fdt_fixup_usb_mode_phy_type(void *blob, const char *mode,
-				       const char *phy_type, int start_offset)
+static const char *fdt_usb_get_node_type(void *blob, int start_offset,
+					 int *node_offset)
 {
 	const char *compat_dr = "fsl-usb2-dr";
 	const char *compat_mph = "fsl-usb2-mph";
-	const char *prop_mode = "dr_mode";
-	const char *prop_type = "phy_type";
 	const char *node_type = NULL;
-	int node_offset;
-	int err;
 
-	node_offset = fdt_node_offset_by_compatible(blob,
-						    start_offset, compat_mph);
-	if (node_offset < 0) {
-		node_offset = fdt_node_offset_by_compatible(blob,
-							    start_offset,
-							    compat_dr);
-		if (node_offset < 0) {
-			printf("WARNING: could not find compatible node: %s",
-			       fdt_strerror(node_offset));
-			return -1;
+	*node_offset = fdt_node_offset_by_compatible(blob, start_offset,
+						     compat_mph);
+	if (*node_offset < 0) {
+		*node_offset = fdt_node_offset_by_compatible(blob,
+							     start_offset,
+							     compat_dr);
+		if (*node_offset < 0) {
+			printf("ERROR: could not find compatible node: %s\n",

board/freescale/common/usb.c line 43

Quoted 10 lines
board/freescale/common/usb.c +4 -1
+			       fdt_strerror(*node_offset));
+		} else {
+			node_type = compat_dr;
 		}
-		node_type = compat_dr;
 	} else {
 		node_type = compat_mph;
 	}
 
+	return node_type;

The function should be able to return error code. If you need to return some more stuff from the function, return it via reference.

Quoted 8 lines
Quoted 2 lines

This patch is not altering the fdt_usb_get_node_type(). It is only calling the function from fdt_fixup_usb_mode_phy_type(), to avoid code

duplication.

I am not complaining about that part. I am complaining about the new function, fdt_usb_get_node_type(), which returns pointer as a return value instead of returning proper error code as a return value.

I think git diff tool is creating confusion here, I have just moved function fdt_usb_get_node_type above fdt_fixup_usb_mode_phy_type which created all the diff and looked as if new function is added.

Please check below created patch where declaration is added to call fdt_usb_get_node_type in fdt_usb_get_mode_phy_type. We will be sending below in V4.

#################################################################

board/freescale/common/usb.c +6 -17
index 85cb1bf..5553225 100644
@@ -18,32 +18,21 @@
 #define CONFIG_USB_MAX_CONTROLLER_COUNT 1
 #endif
 
+const char *fdt_usb_get_node_type(void *blob, int start_offset,
+                                        int *node_offset);
+
 static int fdt_fixup_usb_mode_phy_type(void *blob, const char *mode,
                                       const char *phy_type, int start_offset)
 {
-       const char *compat_dr = "fsl-usb2-dr";
-       const char *compat_mph = "fsl-usb2-mph";
        const char *prop_mode = "dr_mode";
        const char *prop_type = "phy_type";
        const char *node_type = NULL;
        int node_offset;
        int err;
 
-       node_offset = fdt_node_offset_by_compatible(blob,
-                                                   start_offset, compat_mph);
-       if (node_offset < 0) {
-               node_offset = fdt_node_offset_by_compatible(blob,
-                                                           start_offset,
-                                                           compat_dr);
-               if (node_offset < 0) {
-                       printf("WARNING: could not find compatible node: %s",
-                              fdt_strerror(node_offset));
-                       return -1;
-               }
-               node_type = compat_dr;
-       } else {
-               node_type = compat_mph;
-       }
+       node_type = fdt_usb_get_node_type(blob, start_offset, &node_offset);
+       if (!node_type)
+               return -1;
 
        if (mode) {
                err = fdt_setprop(blob, node_offset, prop_mode, mode,

#################################################################

Quoted 63 lines
Quoted 50 lines
Quoted 50 lines
Quoted 18 lines
board/freescale/common/usb.c +14 -0
+}
+
+static int fdt_fixup_usb_mode_phy_type(void *blob, const char *mode,
+					const char *phy_type, int start_offset) {
+	const char *prop_mode = "dr_mode";
+	const char *prop_type = "phy_type";
+	const char *node_type = NULL;
+	int node_offset;
+	int err;
+
+	node_type = fdt_usb_get_node_type(blob, start_offset, &node_offset);
+	if (!node_type)
+		return -1;
+
 	if (mode) {
 		err = fdt_setprop(blob, node_offset, prop_mode, mode,
 				  strlen(mode) + 1);

board/freescale/common/usb.c static int fdt_fixup_usb_mode_phy_type(void · line 76

Quoted 1 line
board/freescale/common/usb.c +0 -0
@@ -64,32 +76,6 @@ static int fdt_fixup_usb_mode_phy_type(void
 *blob, const

char *mode,

Quoted 30 lines
board/freescale/common/usb.c +0 -26
 	return node_offset;
 }
 
-static const char *fdt_usb_get_node_type(void *blob, int start_offset,
-					 int *node_offset)
-{
-	const char *compat_dr = "fsl-usb2-dr";
-	const char *compat_mph = "fsl-usb2-mph";
-	const char *node_type = NULL;
-
-	*node_offset = fdt_node_offset_by_compatible(blob, start_offset,
-						     compat_mph);
-	if (*node_offset < 0) {
-		*node_offset = fdt_node_offset_by_compatible(blob,
-							     start_offset,
-							     compat_dr);
-		if (*node_offset < 0) {
-			printf("ERROR: could not find compatible node: %s\n",
-			       fdt_strerror(*node_offset));
-		} else {
-			node_type = compat_dr;
-		}
-	} else {
-		node_type = compat_mph;
-	}
-
-	return node_type;
-}
-
 static int fdt_fixup_usb_erratum(void *blob, const char *prop_erratum,
Quoted 8 lines
Quoted 8 lines
Quoted 3 lines
board/freescale/common/usb.c +0 -0
 				 int start_offset)

{

--
Best regards,
Marek Vasut

--
Best regards,
Marek Vasut

#12 lore raw

PATCH v3 2/3 board:freescale:usb: Remove code duplication for fdt_usb_get_node_type

Quoted 109 lines · Sriram Dash
Quoted 86 lines

-----Original Message-----
From: Marek Vasut [mailto:marex at denx.de]
Sent: Thursday, March 03, 2016 3:29 PM
To: Sriram Dash <[email protected]>; u-boot at lists.denx.de
Cc: york sun <[email protected]>; Ramneek Mehresh <[email protected]>; Rajesh Bhagat <[email protected]>
Subject: Re: [PATCH v3 2/3] board:freescale:usb: Remove code duplication for fdt_usb_get_node_type

Quoted 74 lines · Sriram Dash
Quoted 73 lines

-----Original Message-----
From: Marek Vasut [mailto:marex at denx.de]
Sent: Wednesday, March 02, 2016 3:57 AM
To: Sriram Dash <[email protected]>; u-boot at lists.denx.de
Cc: york sun <[email protected]>; Ramneek Mehresh <[email protected]>; Rajesh Bhagat <[email protected]>
Subject: Re: [PATCH v3 2/3] board:freescale:usb: Remove code duplication for fdt_usb_get_node_type

Quoted 48 lines · Sriram Dash

Call fdt_usb_get_node_type() from fdt_fixup_usb_mode_phy_type() to avoid code duplication.

Signed-off-by Ramneek Mehresh <[email protected]>
Signed-off-by Sriram Dash <[email protected]>
---
 board/freescale/common/usb.c | 72
++++++++++++++++++--------------------------
 1 file changed, 29 insertions(+), 43 deletions(-)
(quoted diff) +0 -0

b/board/freescale/common/usb.c index 85cb1bf..d815dc1 100644

board/freescale/common/usb.c +10 -16
@@ -18,33 +18,45 @@
 #define CONFIG_USB_MAX_CONTROLLER_COUNT 1  #endif
 
-static int fdt_fixup_usb_mode_phy_type(void *blob, const char *mode,
-				       const char *phy_type, int start_offset)
+static const char *fdt_usb_get_node_type(void *blob, int start_offset,
+					 int *node_offset)
 {
 	const char *compat_dr = "fsl-usb2-dr";
 	const char *compat_mph = "fsl-usb2-mph";
-	const char *prop_mode = "dr_mode";
-	const char *prop_type = "phy_type";
 	const char *node_type = NULL;
-	int node_offset;
-	int err;
 
-	node_offset = fdt_node_offset_by_compatible(blob,
-						    start_offset, compat_mph);
-	if (node_offset < 0) {
-		node_offset = fdt_node_offset_by_compatible(blob,
-							    start_offset,
-							    compat_dr);
-		if (node_offset < 0) {
-			printf("WARNING: could not find compatible node: %s",
-			       fdt_strerror(node_offset));
-			return -1;
+	*node_offset = fdt_node_offset_by_compatible(blob, start_offset,
+						     compat_mph);
+	if (*node_offset < 0) {
+		*node_offset = fdt_node_offset_by_compatible(blob,
+							     start_offset,
+							     compat_dr);
+		if (*node_offset < 0) {
+			printf("ERROR: could not find compatible node: %s\n",

board/freescale/common/usb.c line 43

Quoted 10 lines
board/freescale/common/usb.c +4 -1
+			       fdt_strerror(*node_offset));
+		} else {
+			node_type = compat_dr;
 		}
-		node_type = compat_dr;
 	} else {
 		node_type = compat_mph;
 	}
 
+	return node_type;

The function should be able to return error code. If you need to return some more stuff from the function, return it via reference.

Quoted 8 lines
Quoted 2 lines

This patch is not altering the fdt_usb_get_node_type(). It is only calling the function from fdt_fixup_usb_mode_phy_type(), to avoid code

duplication.

I am not complaining about that part. I am complaining about the new function, fdt_usb_get_node_type(), which returns pointer as a return value instead of returning proper error code as a return value.

I think git diff tool is creating confusion here, I have just moved function fdt_usb_get_node_type above fdt_fixup_usb_mode_phy_type which created all the diff and looked as if new function is added.

Please check below created patch where declaration is added to call fdt_usb_get_node_type in fdt_usb_get_mode_phy_type. We will be sending below in V4.

#################################################################

board/freescale/common/usb.c +0 -0
index 85cb1bf..5553225 100644

board/freescale/common/usb.c line 23

Quoted 6 lines
board/freescale/common/usb.c +3 -0
@@ -18,32 +18,21 @@
 #define CONFIG_USB_MAX_CONTROLLER_COUNT 1
 #endif
 
+const char *fdt_usb_get_node_type(void *blob, int start_offset,
+                                        int *node_offset);
+

Ah I see, I am wrong, sorry.

btw can you fix the fdt_usb_get_node_type() in another patch to return int ?

Thanks!
Best regards,
Marek Vasut

author
#13 lore raw

PATCH v3 3/3 board:freescale:usb: Add device-tree fixup support for xhci controller

In reply to Sriram Dash

Enables usb device-tree fixup code to incorporate xhci controller

Signed-off-by Ramneek Mehresh <[email protected]>
Signed-off-by Sriram Dash <[email protected]>
---
 board/freescale/common/Makefile |  1 +
 board/freescale/common/usb.c    | 30 +++++++++++++-----------------
 include/fdt_support.h           |  4 ++--
 3 files changed, 16 insertions(+), 19 deletions(-)
board/freescale/common/Makefile +1 -0
index 62de45c..c644896 100644
@@ -14,6 +14,7 @@ endif
 endif
 
 obj-$(CONFIG_USB_EHCI_FSL) += usb.o
+obj-$(CONFIG_USB_XHCI_FSL) += usb.o
 
 ifdef MINIMAL
 # necessary to create built-in.o
board/freescale/common/usb.c +13 -17
index d815dc1..8e423be 100644
@@ -18,29 +18,25 @@
 #define CONFIG_USB_MAX_CONTROLLER_COUNT 1
 #endif
 
+const char compat_usb_fsl[][16] = {"fsl-usb2-mph",
+				   "fsl-usb2-dr",
+				   "snps,dwc3"};
+
 static const char *fdt_usb_get_node_type(void *blob, int start_offset,
 					 int *node_offset)
 {
-	const char *compat_dr = "fsl-usb2-dr";
-	const char *compat_mph = "fsl-usb2-mph";
 	const char *node_type = NULL;
-
-	*node_offset = fdt_node_offset_by_compatible(blob, start_offset,
-						     compat_mph);
-	if (*node_offset < 0) {
-		*node_offset = fdt_node_offset_by_compatible(blob,
-							     start_offset,
-							     compat_dr);
-		if (*node_offset < 0) {
-			printf("ERROR: could not find compatible node: %s\n",
-			       fdt_strerror(*node_offset));
-		} else {
-			node_type = compat_dr;
+	int size = sizeof(compat_usb_fsl)/sizeof(compat_usb_fsl[0]);
+	int i;
+
+	for (i = 0; i < size ; i++) {
+		*node_offset = fdt_node_offset_by_compatible
+				(blob, start_offset, compat_usb_fsl[i]);
+		if (*node_offset >= 0) {
+			node_type = compat_usb_fsl[i];
+			break;
 		}
-	} else {
-		node_type = compat_mph;
 	}
-
 	return node_type;
 }
include/fdt_support.h +2 -2
index 296add0..d34e959 100644
@@ -113,11 +113,11 @@ void fdt_fixup_qe_firmware(void *fdt);
  */
 int fdt_fixup_display(void *blob, const char *path, const char *display);
 
-#if defined(CONFIG_HAS_FSL_DR_USB) || defined(CONFIG_HAS_FSL_MPH_USB)
+#if defined(CONFIG_USB_EHCI_FSL) || defined(CONFIG_USB_XHCI_FSL)
 void fdt_fixup_dr_usb(void *blob, bd_t *bd);
 #else
 static inline void fdt_fixup_dr_usb(void *blob, bd_t *bd) {}
-#endif /* defined(CONFIG_HAS_FSL_DR_USB) || defined(CONFIG_HAS_FSL_MPH_USB) */
+#endif /* defined(CONFIG_USB_EHCI_FSL) || defined(CONFIG_USB_XHCI_FSL) */
 
 #if defined(CONFIG_SYS_FSL_SEC_COMPAT)
 void fdt_fixup_crypto_node(void *blob, int sec_rev);
Signature · 1 line
2.1.0
#14 lore raw

PATCH v3 3/3 board:freescale:usb: Add device-tree fixup support for xhci controller

Quoted 4 lines · Sriram Dash

Enables usb device-tree fixup code to incorporate xhci controller

Signed-off-by Ramneek Mehresh <[email protected]>
Signed-off-by Sriram Dash <[email protected]>

Changelog ?

Quoted 23 lines
---
 board/freescale/common/Makefile |  1 +
 board/freescale/common/usb.c    | 30 +++++++++++++-----------------
 include/fdt_support.h           |  4 ++--
 3 files changed, 16 insertions(+), 19 deletions(-)
board/freescale/common/Makefile +1 -0
index 62de45c..c644896 100644
@@ -14,6 +14,7 @@ endif
 endif
 
 obj-$(CONFIG_USB_EHCI_FSL) += usb.o
+obj-$(CONFIG_USB_XHCI_FSL) += usb.o
 
 ifdef MINIMAL
 # necessary to create built-in.o
board/freescale/common/usb.c +0 -0
index d815dc1..8e423be 100644

board/freescale/common/usb.c line 23

Quoted 6 lines
board/freescale/common/usb.c +3 -0
@@ -18,29 +18,25 @@
 #define CONFIG_USB_MAX_CONTROLLER_COUNT 1
 #endif
 
+const char compat_usb_fsl[][16] = {"fsl-usb2-mph",
+				   "fsl-usb2-dr",
+				   "snps,dwc3"};

This is const char *foo[].

Quoted 9 lines
board/freescale/common/usb.c +0 -5
 static const char *fdt_usb_get_node_type(void *blob, int start_offset,
 					 int *node_offset)
 {
-	const char *compat_dr = "fsl-usb2-dr";
-	const char *compat_mph = "fsl-usb2-mph";
 	const char *node_type = NULL;
-
-	*node_offset = fdt_node_offset_by_compatible(blob, start_offset,
-						     compat_mph);

board/freescale/common/usb.c line 29

Quoted 10 lines
board/freescale/common/usb.c +1 -9
-	if (*node_offset < 0) {
-		*node_offset = fdt_node_offset_by_compatible(blob,
-							     start_offset,
-							     compat_dr);
-		if (*node_offset < 0) {
-			printf("ERROR: could not find compatible node: %s\n",
-			       fdt_strerror(*node_offset));
-		} else {
-			node_type = compat_dr;
+	int size = sizeof(compat_usb_fsl)/sizeof(compat_usb_fsl[0]);

Oh the art of counting. Firstly, what you did here is reimplementation of ARRAY_SIZE(), but that's wrong in this context. Each one of the array elements is differently sized, so to avoid problems with this crap, the code hard-codes random constant defining the element size, which is another crap workaround as it will break once a longer element is added. And it wastes space. No, instead, use a terminating entry in the array.

btw use checkpatch before your next submission.

Quoted 35 lines
board/freescale/common/usb.c +8 -3
+	int i;
+
+	for (i = 0; i < size ; i++) {
+		*node_offset = fdt_node_offset_by_compatible
+				(blob, start_offset, compat_usb_fsl[i]);
+		if (*node_offset >= 0) {
+			node_type = compat_usb_fsl[i];
+			break;
 		}
-	} else {
-		node_type = compat_mph;
 	}
-
 	return node_type;
 }
include/fdt_support.h +2 -2
index 296add0..d34e959 100644
@@ -113,11 +113,11 @@ void fdt_fixup_qe_firmware(void *fdt);
  */
 int fdt_fixup_display(void *blob, const char *path, const char *display);
 
-#if defined(CONFIG_HAS_FSL_DR_USB) || defined(CONFIG_HAS_FSL_MPH_USB)
+#if defined(CONFIG_USB_EHCI_FSL) || defined(CONFIG_USB_XHCI_FSL)
 void fdt_fixup_dr_usb(void *blob, bd_t *bd);
 #else
 static inline void fdt_fixup_dr_usb(void *blob, bd_t *bd) {}
-#endif /* defined(CONFIG_HAS_FSL_DR_USB) || defined(CONFIG_HAS_FSL_MPH_USB) */
+#endif /* defined(CONFIG_USB_EHCI_FSL) || defined(CONFIG_USB_XHCI_FSL) */
 
 #if defined(CONFIG_SYS_FSL_SEC_COMPAT)
 void fdt_fixup_crypto_node(void *blob, int sec_rev);
Signature · 2 lines
Best regards,
Marek Vasut
author
#15 lore raw

PATCH v3 3/3 board:freescale:usb: Add device-tree fixup support for xhci controller

Quoted 9 lines

-----Original Message-----
From: Marek Vasut [mailto:marex at denx.de]
Sent: Wednesday, March 02, 2016 3:55 AM
To: Sriram Dash <[email protected]>; u-boot at lists.denx.de
Cc: york sun <[email protected]>; Ramneek Mehresh <[email protected]>; Rajesh Bhagat <[email protected]>
Subject: Re: [PATCH v3 3/3] board:freescale:usb: Add device-tree fixup support for xhci controller

Quoted 8 lines
Quoted 4 lines · Sriram Dash

Enables usb device-tree fixup code to incorporate xhci controller

Signed-off-by Ramneek Mehresh <[email protected]>
Signed-off-by Sriram Dash <[email protected]>

Changelog ?

Will include changelog for v2 and v3 in v4.

Quoted 25 lines
Quoted 25 lines
---
 board/freescale/common/Makefile |  1 +
 board/freescale/common/usb.c    | 30 +++++++++++++-----------------
 include/fdt_support.h           |  4 ++--
 3 files changed, 16 insertions(+), 19 deletions(-)
(quoted diff) +0 -0

b/board/freescale/common/Makefile index 62de45c..c644896 100644

board/freescale/common/Makefile +1 -0
@@ -14,6 +14,7 @@ endif
 endif
 
 obj-$(CONFIG_USB_EHCI_FSL) += usb.o
+obj-$(CONFIG_USB_XHCI_FSL) += usb.o
 
 ifdef MINIMAL
 # necessary to create built-in.o
(quoted diff) +0 -0

b/board/freescale/common/usb.c index d815dc1..8e423be 100644

board/freescale/common/usb.c +0 -0
@@ -18,29 +18,25 @@
 #define CONFIG_USB_MAX_CONTROLLER_COUNT 1  #endif
Quoted 6 lines

board/freescale/common/usb.c line 23

Quoted 3 lines
board/freescale/common/usb.c +3 -0
+const char compat_usb_fsl[][16] = {"fsl-usb2-mph",
+				   "fsl-usb2-dr",
+				   "snps,dwc3"};

This is const char *foo[].

Reference from "char compat[][16] = { "cfi-flash", "jedec-flash" };" I will change to const char *compat_usb_fsl[] in v4.

Quoted 18 lines
Quoted 18 lines
board/freescale/common/usb.c +0 -14
 static const char *fdt_usb_get_node_type(void *blob, int start_offset,
 					 int *node_offset)
 {
-	const char *compat_dr = "fsl-usb2-dr";
-	const char *compat_mph = "fsl-usb2-mph";
 	const char *node_type = NULL;
-
-	*node_offset = fdt_node_offset_by_compatible(blob, start_offset,
-						     compat_mph);
-	if (*node_offset < 0) {
-		*node_offset = fdt_node_offset_by_compatible(blob,
-							     start_offset,
-							     compat_dr);
-		if (*node_offset < 0) {
-			printf("ERROR: could not find compatible node: %s\n",
-			       fdt_strerror(*node_offset));
-		} else {
-			node_type = compat_dr;
Quoted 9 lines

board/freescale/common/usb.c line 29

Quoted 1 line
board/freescale/common/usb.c +1 -0
+	int size = sizeof(compat_usb_fsl)/sizeof(compat_usb_fsl[0]);

Oh the art of counting. Firstly, what you did here is reimplementation of ARRAY_SIZE(), but that's wrong in this context. Each one of the array elements is differently sized, so to avoid problems with this crap, the code hard-codes random constant defining the element size, which is another crap workaround as it will break once a longer element is added.
And it wastes space. No, instead, use a terminating entry in the array.

Will change compat_usb_fsl[][16] into const char *compat_usb_fsl[], and use ARRAY_SIZE(compat_usb_fsl) . What is your opinion?

Quoted 43 lines

btw use checkpatch before your next submission.

Quoted 31 lines
board/freescale/common/usb.c +8 -3
+	int i;
+
+	for (i = 0; i < size ; i++) {
+		*node_offset = fdt_node_offset_by_compatible
+				(blob, start_offset, compat_usb_fsl[i]);
+		if (*node_offset >= 0) {
+			node_type = compat_usb_fsl[i];
+			break;
 		}
-	} else {
-		node_type = compat_mph;
 	}
-
 	return node_type;
 }
include/fdt_support.h index renamed from include/fdt_support.h +0 -0

296add0..d34e959 100644

include/fdt_support.h +1 -1
@@ -113,11 +113,11 @@ void fdt_fixup_qe_firmware(void *fdt);
  */
 int fdt_fixup_display(void *blob, const char *path, const char
 *display);
 
-#if defined(CONFIG_HAS_FSL_DR_USB) || defined(CONFIG_HAS_FSL_MPH_USB)
+#if defined(CONFIG_USB_EHCI_FSL) || defined(CONFIG_USB_XHCI_FSL)
 void fdt_fixup_dr_usb(void *blob, bd_t *bd);  #else  static inline

void fdt_fixup_dr_usb(void *blob, bd_t *bd) {} -#endif /* defined(CONFIG_HAS_FSL_DR_USB) || defined(CONFIG_HAS_FSL_MPH_USB) */

include/fdt_support.h void fdt_fixup_qe_firmware(void *fdt); · line 123

Quoted 5 lines
include/fdt_support.h +2 -0
+#endif /* defined(CONFIG_USB_EHCI_FSL) ||
+defined(CONFIG_USB_XHCI_FSL) */
 
 #if defined(CONFIG_SYS_FSL_SEC_COMPAT)
 void fdt_fixup_crypto_node(void *blob, int sec_rev);

--
Best regards,
Marek Vasut

#16 lore raw

PATCH v3 3/3 board:freescale:usb: Add device-tree fixup support for xhci controller

Quoted 83 lines · Sriram Dash
Quoted 9 lines

-----Original Message-----
From: Marek Vasut [mailto:marex at denx.de]
Sent: Wednesday, March 02, 2016 3:55 AM
To: Sriram Dash <[email protected]>; u-boot at lists.denx.de
Cc: york sun <[email protected]>; Ramneek Mehresh <[email protected]>; Rajesh Bhagat <[email protected]>
Subject: Re: [PATCH v3 3/3] board:freescale:usb: Add device-tree fixup support for xhci controller

Quoted 8 lines
Quoted 4 lines · Sriram Dash

Enables usb device-tree fixup code to incorporate xhci controller

Signed-off-by Ramneek Mehresh <[email protected]>
Signed-off-by Sriram Dash <[email protected]>

Changelog ?

Will include changelog for v2 and v3 in v4.

Quoted 25 lines
Quoted 25 lines
---
 board/freescale/common/Makefile |  1 +
 board/freescale/common/usb.c    | 30 +++++++++++++-----------------
 include/fdt_support.h           |  4 ++--
 3 files changed, 16 insertions(+), 19 deletions(-)
(quoted diff) +0 -0

b/board/freescale/common/Makefile index 62de45c..c644896 100644

board/freescale/common/Makefile +1 -0
@@ -14,6 +14,7 @@ endif
 endif
 
 obj-$(CONFIG_USB_EHCI_FSL) += usb.o
+obj-$(CONFIG_USB_XHCI_FSL) += usb.o
 
 ifdef MINIMAL
 # necessary to create built-in.o
(quoted diff) +0 -0

b/board/freescale/common/usb.c index d815dc1..8e423be 100644

board/freescale/common/usb.c +0 -0
@@ -18,29 +18,25 @@
 #define CONFIG_USB_MAX_CONTROLLER_COUNT 1  #endif
Quoted 6 lines

board/freescale/common/usb.c line 23

Quoted 3 lines
board/freescale/common/usb.c +3 -0
+const char compat_usb_fsl[][16] = {"fsl-usb2-mph",
+				   "fsl-usb2-dr",
+				   "snps,dwc3"};

This is const char *foo[].

Reference from "char compat[][16] = { "cfi-flash", "jedec-flash" };" I will change to const char *compat_usb_fsl[] in v4.

Quoted 18 lines
Quoted 18 lines
board/freescale/common/usb.c +0 -14
 static const char *fdt_usb_get_node_type(void *blob, int start_offset,
 					 int *node_offset)
 {
-	const char *compat_dr = "fsl-usb2-dr";
-	const char *compat_mph = "fsl-usb2-mph";
 	const char *node_type = NULL;
-
-	*node_offset = fdt_node_offset_by_compatible(blob, start_offset,
-						     compat_mph);
-	if (*node_offset < 0) {
-		*node_offset = fdt_node_offset_by_compatible(blob,
-							     start_offset,
-							     compat_dr);
-		if (*node_offset < 0) {
-			printf("ERROR: could not find compatible node: %s\n",
-			       fdt_strerror(*node_offset));
-		} else {
-			node_type = compat_dr;
Quoted 9 lines

board/freescale/common/usb.c line 29

Quoted 1 line
board/freescale/common/usb.c +1 -0
+	int size = sizeof(compat_usb_fsl)/sizeof(compat_usb_fsl[0]);

Oh the art of counting. Firstly, what you did here is reimplementation of ARRAY_SIZE(), but that's wrong in this context. Each one of the array elements is differently sized, so to avoid problems with this crap, the code hard-codes random constant defining the element size, which is another crap workaround as it will break once a longer element is added.
And it wastes space. No, instead, use a terminating entry in the array.

Quoted 2 lines

Will change compat_usb_fsl[][16] into const char *compat_usb_fsl[], and use ARRAY_SIZE(compat_usb_fsl) . What is your opinion?

My opinion is to use a terminating NULL entry and iterate over the array until you reach it.

Signature · 2 lines
Best regards,
Marek Vasut
author
#17 lore raw

PATCH v3 3/3 board:freescale:usb: Add device-tree fixup support for xhci controller

Quoted 93 lines

-----Original Message-----
From: Marek Vasut [mailto:marex at denx.de]
Sent: Thursday, March 03, 2016 3:33 PM
To: Sriram Dash <[email protected]>; u-boot at lists.denx.de
Cc: york sun <[email protected]>; Ramneek Mehresh <[email protected]>; Rajesh Bhagat <[email protected]>
Subject: Re: [PATCH v3 3/3] board:freescale:usb: Add device-tree fixup support for xhci controller

Quoted 52 lines · Sriram Dash
Quoted 9 lines

-----Original Message-----
From: Marek Vasut [mailto:marex at denx.de]
Sent: Wednesday, March 02, 2016 3:55 AM
To: Sriram Dash <[email protected]>; u-boot at lists.denx.de
Cc: york sun <[email protected]>; Ramneek Mehresh <[email protected]>; Rajesh Bhagat <[email protected]>
Subject: Re: [PATCH v3 3/3] board:freescale:usb: Add device-tree fixup support for xhci controller

Quoted 8 lines
Quoted 4 lines · Sriram Dash

Enables usb device-tree fixup code to incorporate xhci controller

Signed-off-by Ramneek Mehresh <[email protected]>
Signed-off-by Sriram Dash <[email protected]>

Changelog ?

Will include changelog for v2 and v3 in v4.

Quoted 25 lines
Quoted 25 lines
---
 board/freescale/common/Makefile |  1 +
 board/freescale/common/usb.c    | 30 +++++++++++++-----------------
 include/fdt_support.h           |  4 ++--
 3 files changed, 16 insertions(+), 19 deletions(-)
(quoted diff) +0 -0

b/board/freescale/common/Makefile index 62de45c..c644896 100644

board/freescale/common/Makefile +1 -0
@@ -14,6 +14,7 @@ endif
 endif
 
 obj-$(CONFIG_USB_EHCI_FSL) += usb.o
+obj-$(CONFIG_USB_XHCI_FSL) += usb.o
 
 ifdef MINIMAL
 # necessary to create built-in.o
(quoted diff) +0 -0

b/board/freescale/common/usb.c index d815dc1..8e423be 100644

board/freescale/common/usb.c +0 -0
@@ -18,29 +18,25 @@
 #define CONFIG_USB_MAX_CONTROLLER_COUNT 1  #endif
Quoted 6 lines

board/freescale/common/usb.c line 23

Quoted 3 lines
board/freescale/common/usb.c +3 -0
+const char compat_usb_fsl[][16] = {"fsl-usb2-mph",
+				   "fsl-usb2-dr",
+				   "snps,dwc3"};

This is const char *foo[].

Quoted 28 lines

Reference from "char compat[][16] = { "cfi-flash", "jedec-flash" };" I will change to const char *compat_usb_fsl[] in v4.

Quoted 25 lines
Quoted 9 lines
board/freescale/common/usb.c +0 -5
 static const char *fdt_usb_get_node_type(void *blob, int start_offset,
 					 int *node_offset)
 {
-	const char *compat_dr = "fsl-usb2-dr";
-	const char *compat_mph = "fsl-usb2-mph";
 	const char *node_type = NULL;
-
-	*node_offset = fdt_node_offset_by_compatible(blob, start_offset,
-						     compat_mph);

board/freescale/common/usb.c line 29

Quoted 10 lines
board/freescale/common/usb.c +1 -9
-	if (*node_offset < 0) {
-		*node_offset = fdt_node_offset_by_compatible(blob,
-							     start_offset,
-							     compat_dr);
-		if (*node_offset < 0) {
-			printf("ERROR: could not find compatible node: %s\n",
-			       fdt_strerror(*node_offset));
-		} else {
-			node_type = compat_dr;
+	int size = sizeof(compat_usb_fsl)/sizeof(compat_usb_fsl[0]);

Oh the art of counting. Firstly, what you did here is reimplementation of ARRAY_SIZE(), but that's wrong in this context. Each one of the array elements is differently sized, so to avoid problems with this crap, the code hard-codes random constant defining the element size, which is another crap workaround as it will break once a longer

element is added.

Quoted 9 lines
Quoted 5 lines
Quoted 2 lines

And it wastes space. No, instead, use a terminating entry in the array.

Will change compat_usb_fsl[][16] into const char *compat_usb_fsl[], and use ARRAY_SIZE(compat_usb_fsl) . What is your opinion?

My opinion is to use a terminating NULL entry and iterate over the array until you reach it.

Accepted. Will do it in v4.

Quoted 3 lines

--
Best regards,
Marek Vasut