[U-Boot] [RFC PATCH v1] usb: spl: fix USB errata for FSL SPL targets

[Sriram Dash]

>From: york sun
>On 06/28/2016 12:02 AM, Sriram Dash wrote:
>>> From: Marek Vasut [mailto:marex at denx.de] On 06/28/2016 01:02 AM, York
>>> Sun wrote:
>>>> Commit 9262367 moved USB errata workaround to a C file but didn't
>>>> build it for SPL targets.
>>>>
>>>> Signed-off-by: York Sun <york.sun at nxp.com>
>>>> CC: Sriram Dash <sriram.dash at nxp.com>
>>>> CC: Rajesh Bhagat <rajesh.bhagat at nxp.com>
>>>>
>>>> ---
>>>> Please review this patch. It fixed the compiling errors introduced
>>>> by 9262367. Not sure if this is the way USB errata should be handled.
>>>>
>>>>   drivers/Makefile                    | 7 +++++++
>>>>   drivers/usb/common/Makefile         | 8 ++++++--
>>>>   include/configs/km/kmp204x-common.h | 1 +
>>>>   3 files changed, 14 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/Makefile b/drivers/Makefile index
>>>> 1723958..88774ba 100644
>>>> --- a/drivers/Makefile
>>>> +++ b/drivers/Makefile
>>>> @@ -39,6 +39,13 @@ obj-$(CONFIG_OMAP_USB_PHY) += usb/phy/
>>>>   obj-$(CONFIG_SPL_SATA_SUPPORT) += block/
>>>>   obj-$(CONFIG_SPL_USB_HOST_SUPPORT) += block/
>>>>   obj-$(CONFIG_SPL_MMC_SUPPORT) += block/
>>>> +ifdef CONFIG_USB_EHCI_FSL
>>>> +CONFIG_SPL_USB_ERRATA = y
>>>> +endif
>>>> +ifdef CONFIG_USB_XHCI_FSL
>>>> +CONFIG_SPL_USB_ERRATA = y
>>>> +endif
>>>> +obj-$(CONFIG_SPL_USB_ERRATA) += usb/common/
>>>
>>> I really dislike the naming here, I'd say just do
>>>
>>> obj-$(CONFIG_USB_EHCI_FSL) += usb/common/
>>> obj-$(CONFIG_USB_XHCI_FSL) += usb/common/
>
>Better.
>
>>>
>>
>> Hello York/Marek,
>>
>> IMO, the build for SPL is failing in PPC as the cmd_errata is not
>> getting the definition of the has_erratum_aNNNNNN functions. So,
>> instead of EHCI or XHCI flags, i think we can use CONFIG_CMD_ERRATA
>> for SPL build for the errata applicability.
>>
>> +obj-$(CONFIG_CMD_ERRATA) += usb/common/fsl-errata.o
>>
>> What is your opinion?
>>
>
>Sriram,
>
>I think it is not a good idea. CONFIG_CMD_ERRATA has nothing to do with USB
>errata specifically. It means to enable the command "errata", and nothing more.
>
>What's broken here is the mechanism to detect if an erratum applies to a particular
>SoC version. Before your change, the functions were included from the header file.
>You moved those functions into a C file. That's fine. We need to build this file for
>both normal image and SPL. I don't think the workarounds are optional, unless USB
>is not used at all for SPL.
>
>I think Marek's comments make sense.
>1) obj-$(CONFIG_USB_EHCI_FSL) += usb/common/
>    obj-$(CONFIG_USB_XHCI_FSL) += usb/common/

OK. This looks generic also.

> For this to work, we need to fix
>kmp204x-common.h, which I noticed CONFIG_USB_EHCI_FSL is not defined. Please
>examine if this macro should be defined at SoC level, instead of board level.

The kmp204x-common does not need to define CONFIG_USB_EHCI_FSL.
Currently, the board is not using the USB. But, they are using
CONFIG_CMD_ERRATA. So, instead of having a flag of CONFIG_USB_EHCI_FSL
in the board specific file, drivers/usb/common/Makefile should include
fsl-errata.o for the config CONFIG_CMD_ERRATA. What is your opinion?

>2) Use a proper macro for fsl-dt-fixup.o The entry point is fdt_fixup_dr_usb(), which
>is only called by
>ft_board_setup() when CONFIG_OF_BOARD_SETUP is defined.
>

Yes. You are correct. fdt_fixup_dr_usb is only called when CONFIG_OF_BOARD_SETUP
is defined. So, I guess it will be fine for drivers/usb/common/Makefile

obj-$(CONFIG_USB_EHCI_FSL) += fsl-errata.o
obj-$(CONFIG_USB_XHCI_FSL) += fsl-errata.o
obj-$( CONFIG_CMD_ERRATA) += fsl-errata.o
obj-$(CONFIG_OF_BOARD_SETUP) += fsl-dt-fixup.o

What is your opinion?

>Please take the lead to fix this issue.
>
>York