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

[Marek Vasut <marex@denx.de>]

On 05/27/2016 07:14 AM, Sriram Dash wrote:
>> -----Original Message-----
>> From: Marek Vasut [mailto:marex at denx.de]
>> Sent: Thursday, May 26, 2016 5:49 PM
>> To: Sriram Dash <sriram.dash@nxp.com>; u-boot at lists.denx.de
>> Cc: york sun <york.sun@nxp.com>; albert.u.boot at aribaud.net; Rajesh Bhagat
>> <rajesh.bhagat@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@nxp.com>
>>> Signed-off-by: Sriram Dash <sriram.dash@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?

I'm not a fan of huge macros, since they often introduce various subtle
issues and in this case it would most likely reduce size of the u-boot.

>>>  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


-- 
Best regards,
Marek Vasut