[U-Boot] [PATCH v3 2/5] usb: xhci: fsl: code cleanup for device tree fixup for fsl usb controllers
Sriram Dash
sriram.dash at nxp.com
Fri Jun 10 05:47:17 CEST 2016
>-----Original Message-----
>From: Marek Vasut [mailto:marex at denx.de]
>Sent: Thursday, June 09, 2016 7:01 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 v3 2/5] usb: xhci: fsl: code cleanup for device tree fixup for fsl
>usb controllers
>
>On 06/09/2016 02:21 PM, 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.bhagat at nxp.com>
>> Signed-off-by: Sriram Dash <sriram.dash at nxp.com>
>> ---
>>
>> Changes in v3:
>> - Inverted the condition for has_erratum for better readability
>> - If fdt_fixup_usb_erratum fails, return ENOSPC and exit the fixup
>> - Added logic for handling the condition with different controllers
>> with different erratas
>> - first check if the errata is applicable for the SoC
>> - then check if it is applicable for the controller
>> - if both are successful, then fix dt.
>>
>> Changes in v2:
>> - Added patch description
>> - Removed the MACRO and use fdt_fixup_erratum function instead
>>
>> drivers/usb/common/fsl-dt-fixup.c | 88 ++++++++++++++++++++++-----------------
>> include/fsl_usb.h | 6 +++
>> 2 files changed, 56 insertions(+), 38 deletions(-)
>>
>> diff --git a/drivers/usb/common/fsl-dt-fixup.c
>> b/drivers/usb/common/fsl-dt-fixup.c
>> index 6f31932..9cbd9d2 100644
>> --- a/drivers/usb/common/fsl-dt-fixup.c
>> +++ b/drivers/usb/common/fsl-dt-fixup.c
>> @@ -20,9 +20,9 @@
>> #endif
>>
>> static const char * const compat_usb_fsl[] = {
>> - "fsl-usb2-mph",
>> - "fsl-usb2-dr",
>> - "snps,dwc3",
>> + FSL_USB2_MPH,
>> + FSL_USB2_DR,
>> + SNPS_DWC3,
>> NULL
>> };
>>
>> @@ -80,16 +80,24 @@ static int fdt_fixup_usb_mode_phy_type(void *blob,
>> const char *mode, }
>>
>> static int fdt_fixup_usb_erratum(void *blob, const char *prop_erratum,
>> - int start_offset)
>> + const char *controller_type, int start_offset)
>> {
>> int node_offset, err;
>> const char *node_type = NULL;
>> + const char *node_name = NULL;
>>
>> err = fdt_usb_get_node_type(blob, start_offset,
>> &node_offset, &node_type);
>> if (err < 0)
>> return err;
>>
>> + if (!strcmp(node_type, FSL_USB2_MPH) || !strcmp(node_type,
>FSL_USB2_DR))
>> + node_name = USB2_CI;
>> + else
>> + node_name = node_type;
>> + if (strcmp(node_name, controller_type))
>> + return err;
>> +
>> err = fdt_setprop(blob, node_offset, prop_erratum, NULL, 0);
>> if (err < 0) {
>> printf("ERROR: could not set %s for %s: %s.\n", @@ -99,6 +107,23
>@@
>> static int fdt_fixup_usb_erratum(void *blob, const char *prop_erratum,
>> return node_offset;
>> }
>>
>> +static int fdt_fixup_erratum(int *usb_erratum_off, void *blob,
>> + const char *controller_type, char *str,
>> + bool (*has_erratum)(void))
>> +{
>> + char buf[32] = {0};
>> +
>> + snprintf(buf, sizeof(buf), "fsl,usb-erratum-%s", str);
>> + if (!(has_erratum()))
>
>No need for the extra parenthesis around has_erratum() .
>
Ok. Will remove parenthesis in v4.
>> + return -EINVAL;
>> + *usb_erratum_off = fdt_fixup_usb_erratum(blob, buf, controller_type,
>> + *usb_erratum_off);
>> + if (*usb_erratum_off < 0)
>> + return -ENOSPC;
>> + debug("Adding USB erratum %s\n", str);
>> + return 0;
>> +}
>> +
>> void fdt_fixup_dr_usb(void *blob, bd_t *bd) {
>> static const char * const modes[] = { "host", "peripheral", "otg" };
>> @@ -111,6 +136,7 @@ void fdt_fixup_dr_usb(void *blob, bd_t *bd)
>> int usb_phy_off = -1;
>> char str[5];
>> int i, j;
>> + int ret;
>>
>> for (i = 1; i <= CONFIG_USB_MAX_CONTROLLER_COUNT; i++) {
>> const char *dr_mode_type = NULL;
>> @@ -164,39 +190,25 @@ 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;
>> - }
>> + ret = fdt_fixup_erratum(&usb_erratum_a006261_off, blob,
>> + USB2_CI, "a006261",
>> + has_erratum_a006261);
>> + if (ret == -ENOSPC)
>> + return;
>> + ret = fdt_fixup_erratum(&usb_erratum_a007075_off, blob,
>> + USB2_CI, "a007075",
>> + has_erratum_a007075);
>> + if (ret == -ENOSPC)
>> + return;
>> + ret = fdt_fixup_erratum(&usb_erratum_a007792_off, blob,
>> + USB2_CI, "a007792",
>> + has_erratum_a007792);
>> + if (ret == -ENOSPC)
>> + return;
>> + ret = fdt_fixup_erratum(&usb_erratum_a005697_off, blob,
>> + USB2_CI, "a005697",
>> + has_erratum_a005697);
>> + if (ret == -ENOSPC)
>> + return;
>> }
>> }
>> diff --git a/include/fsl_usb.h b/include/fsl_usb.h index
>> 187e384..882a5f5 100644
>> --- a/include/fsl_usb.h
>> +++ b/include/fsl_usb.h
>> @@ -85,6 +85,12 @@ struct ccsr_usb_phy { #define
>> CONFIG_SYS_FSL_USB_SQUELCH_PROG_MASK 0x07 #endif
>>
>> +/* USB Controllers */
>> +#define FSL_USB2_MPH "fsl-usb2-mph"
>> +#define FSL_USB2_DR "fsl-usb2-dr"
>> +#define USB2_CI "usb2-ci"
>> +#define SNPS_DWC3 "snps,dwc3"
>
>Is this needed as a global macro or can it be local to fsl-dt-fixup.c ?
>
Currently, the macro are only being used for dt fixup for fsl usb.
However, in the near future, they might be needed for some other purpose,
for example, board specific settings.
So, i think i will stick with the global macro.
>> /* USB Erratum Checking code */
>> #ifdef CONFIG_PPC
>> static inline bool has_dual_phy(void)
>>
>
>
>--
>Best regards,
>Marek Vasut
More information about the U-Boot
mailing list