On 06/02/2016 08:54 AM, Sriram Dash wrote:
> Performs code cleanup for device tree fixup for fsl usb controllers by
> making functions to handle these similar errata checking code.
>
> Signed-off-by: Rajesh Bhagat <rajesh.bha...@nxp.com>
> Signed-off-by: Sriram Dash <sriram.d...@nxp.com>
> ---
> Changes in v2:
> - added patch description
> - remove the MACRO and use fdt_fixup_erratum function instead
>
>
> drivers/usb/common/fsl-dt-fixup.c | 58
> +++++++++++++++++----------------------
> 1 file changed, 25 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/usb/common/fsl-dt-fixup.c
> b/drivers/usb/common/fsl-dt-fixup.c
> index 6f31932..cbb008b 100644
> --- a/drivers/usb/common/fsl-dt-fixup.c
> +++ b/drivers/usb/common/fsl-dt-fixup.c
> @@ -99,6 +99,23 @@ static int fdt_fixup_usb_erratum(void *blob, const char
> *prop_erratum,
> return node_offset;
> }
>
> +void fdt_fixup_erratum(int *usb_erratum_off, void *blob,
> + char *str, bool (*has_erratum)(void))
> +{
> + char buf[32] = {0};
> +
> + snprintf(buf, sizeof(buf), "fsl,usb-erratum-%s", str);
> + if (has_erratum()) {
Invert the condition here so you won't have the indentation problems below:
if (!(has_erratum())
return;
... other stuff.
> + *usb_erratum_off = fdt_fixup_usb_erratum
> + (blob,
> + buf,
> + *usb_erratum_off);
> + if (*usb_erratum_off < 0)
> + return;
If fdt_fixup_usb_erratum() fails, this function will return, but the
code below will continue. This changes the logic of the code to the
worse, so fix this.
> + debug("Adding USB erratum %s\n", str);
> + }
> +}
> +
> void fdt_fixup_dr_usb(void *blob, bd_t *bd)
> {
> static const char * const modes[] = { "host", "peripheral", "otg" };
> @@ -164,39 +181,14 @@ 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;
> - }
> + fdt_fixup_erratum(&usb_erratum_a006261_off, blob,
> + "a006261", has_erratum_a006261);
> + fdt_fixup_erratum(&usb_erratum_a007075_off, blob,
> + "a007075", has_erratum_a007075);
> + fdt_fixup_erratum(&usb_erratum_a007792_off, blob,
> + "a007792", has_erratum_a007792);
> + fdt_fixup_erratum(&usb_erratum_a005697_off, blob,
> + "a005697", has_erratum_a005697);
Are these usb_erratum_aNNNNNN_off variables needed at all ? Why are they
even passed into the function ? I think they can be local to the function.
> - 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;
> - }
> }
> }
>
--
Best regards,
Marek Vasut
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
