[U-Boot] [PATCH 2/5] usb: xhci: fsl: code cleanup for device tree fixup for fsl usb controllers

[Sriram Dash]

>-----Original Message-----
>From: Marek Vasut [mailto:marex at denx.de]
>Sent: Thursday, May 26, 2016 5:49 PM
>To: Sriram Dash <sriram.dash at nxp.com>; u-boot at lists.denx.de
>Cc: york sun <york.sun at nxp.com>; albert.u.boot at aribaud.net; Rajesh Bhagat
><rajesh.bhagat at nxp.com>
>Subject: Re: [PATCH 2/5] usb: xhci: fsl: code cleanup for device tree fixup for fsl usb
>controllers
>
>On 05/26/2016 07:59 AM, Sriram Dash wrote:
>
>Commit message, please.
>

Will take care in v2.

>> Signed-off-by: Rajesh Bhagat <rajesh.bhagat at nxp.com>
>> Signed-off-by: Sriram Dash <sriram.dash at nxp.com>
>> ---
>>  drivers/usb/common/fsl-dt-fixup.c | 51
>> +++++++++++++--------------------------
>>  1 file changed, 17 insertions(+), 34 deletions(-)
>>
>> diff --git a/drivers/usb/common/fsl-dt-fixup.c
>> b/drivers/usb/common/fsl-dt-fixup.c
>> index 6f31932..520130e 100644
>> --- a/drivers/usb/common/fsl-dt-fixup.c
>> +++ b/drivers/usb/common/fsl-dt-fixup.c
>> @@ -99,6 +99,19 @@ static int fdt_fixup_usb_erratum(void *blob, const char
>*prop_erratum,
>>  	return node_offset;
>>  }
>>
>> +#define FDT_FIXUP_ERRATUM(id)						\
>> +do {									\
>> +	if (has_erratum_##id()) {					\
>> +		usb_erratum_##id##_off =  fdt_fixup_usb_erratum		\
>> +					   (blob,			\
>> +					    "fsl,usb-erratum-"#id,	\
>> +					    usb_erratum_##id##_off);	\
>> +		if (usb_erratum_##id##_off < 0)				\
>> +			return;						\
>> +		debug("Adding USB erratum "#id"\n");			\
>> +	}								\
>> +} while (0)
>
>Would it be difficult to turn this into a function ?
>
>You can likely implement some function like int fsl_has_erratum(const unsigned int
>erratum) and then do the string concatenation with snprintf().
>

Do you suspect any possible issue with macro implementation, or your comment is for reducing the size of U boot by making it a function?

>>  void fdt_fixup_dr_usb(void *blob, bd_t *bd)  {
>>  	static const char * const modes[] = { "host", "peripheral", "otg" };
>> @@ -164,39 +177,9 @@ void fdt_fixup_dr_usb(void *blob, bd_t *bd)
>>  		if (usb_phy_off < 0)
>>  			return;
>>
>> -		if (has_erratum_a006261()) {
>> -			usb_erratum_a006261_off =  fdt_fixup_usb_erratum
>> -						   (blob,
>> -						    "fsl,usb-erratum-a006261",
>> -						    usb_erratum_a006261_off);
>> -			if (usb_erratum_a006261_off < 0)
>> -				return;
>> -		}
>> -
>> -		if (has_erratum_a007075()) {
>> -			usb_erratum_a007075_off =  fdt_fixup_usb_erratum
>> -						   (blob,
>> -						    "fsl,usb-erratum-a007075",
>> -						    usb_erratum_a007075_off);
>> -			if (usb_erratum_a007075_off < 0)
>> -				return;
>> -		}
>> -
>> -		if (has_erratum_a007792()) {
>> -			usb_erratum_a007792_off =  fdt_fixup_usb_erratum
>> -						   (blob,
>> -						    "fsl,usb-erratum-a007792",
>> -						    usb_erratum_a007792_off);
>> -			if (usb_erratum_a007792_off < 0)
>> -				return;
>> -		}
>> -		if (has_erratum_a005697()) {
>> -			usb_erratum_a005697_off =  fdt_fixup_usb_erratum
>> -						   (blob,
>> -						    "fsl,usb-erratum-a005697",
>> -						    usb_erratum_a005697_off);
>> -			if (usb_erratum_a005697_off < 0)
>> -				return;
>> -		}
>> +		FDT_FIXUP_ERRATUM(a006261);
>> +		FDT_FIXUP_ERRATUM(a007075);
>> +		FDT_FIXUP_ERRATUM(a007792);
>> +		FDT_FIXUP_ERRATUM(a005697);
>>  	}
>>  }
>>
>
>
>--
>Best regards,
>Marek Vasut