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

[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/
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.
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.

Please take the lead to fix this issue.

York