On 03/03/2016 12:11 PM, Sriram Dash wrote: > > >> -----Original Message----- >> From: Marek Vasut [mailto:ma...@denx.de] >> Sent: Thursday, March 03, 2016 3:29 PM >> To: Sriram Dash <sriram.d...@nxp.com>; u-boot@lists.denx.de >> Cc: york sun <york....@nxp.com>; Ramneek Mehresh >> <ramneek.mehr...@nxp.com>; Rajesh Bhagat <rajesh.bha...@nxp.com> >> Subject: Re: [PATCH v3 2/3] board:freescale:usb: Remove code duplication for >> fdt_usb_get_node_type >> >> On 03/03/2016 09:30 AM, Sriram Dash wrote: >>> >>>> -----Original Message----- >>>> From: Marek Vasut [mailto:ma...@denx.de] >>>> Sent: Wednesday, March 02, 2016 3:57 AM >>>> To: Sriram Dash <sriram.d...@nxp.com>; u-boot@lists.denx.de >>>> Cc: york sun <york....@nxp.com>; Ramneek Mehresh >>>> <ramneek.mehr...@nxp.com>; Rajesh Bhagat <rajesh.bha...@nxp.com> >>>> Subject: Re: [PATCH v3 2/3] board:freescale:usb: Remove code >>>> duplication for fdt_usb_get_node_type >>>> >>>> On 03/01/2016 08:03 AM, Sriram Dash wrote: >>>>> Call fdt_usb_get_node_type() from fdt_fixup_usb_mode_phy_type() to >>>>> avoid code duplication. >>>>> >>>>> Signed-off-by: Ramneek Mehresh <ramneek.mehr...@nxp.com> >>>>> Signed-off-by: Sriram Dash <sriram.d...@nxp.com> >>>>> --- >>>>> board/freescale/common/usb.c | 72 >>>>> ++++++++++++++++++-------------------------- >>>>> 1 file changed, 29 insertions(+), 43 deletions(-) >>>>> >>>>> diff --git a/board/freescale/common/usb.c >>>>> b/board/freescale/common/usb.c index 85cb1bf..d815dc1 100644 >>>>> --- a/board/freescale/common/usb.c >>>>> +++ b/board/freescale/common/usb.c >>>>> @@ -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; >>>> >>>> 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. >> > > 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. > > ################################################################# > > diff --git a/board/freescale/common/usb.c b/board/freescale/common/usb.c > index 85cb1bf..5553225 100644 > --- a/board/freescale/common/usb.c > +++ b/board/freescale/common/usb.c > @@ -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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
