Re: [PATCH 1/8] imx8mm: Fix USB reg addresses for i.MX8MM

From: Tim Harvey <tharvey@gateworks.com>
To: Marek Vasut <marex@denx.de>
Cc: Fabio Estevam <festevam@gmail.com>,
	Fabio Estevam <festevam@denx.de>, Stefano Babic <sbabic@denx.de>,
	U-Boot-Denx <u-boot@lists.denx.de>,
	Schrempf Frieder <frieder.schrempf@kontron.de>,
	 dl-uboot-imx <uboot-imx@nxp.com>
Subject: Re: [PATCH 1/8] imx8mm: Fix USB reg addresses for i.MX8MM
Date: Tue, 6 Jul 2021 09:11:17 -0700	[thread overview]
Message-ID: <CAJ+vNU3s0wNtcGMqM3AdgUw9qcPRkZ+tivA0iZfOzBvqyYrj7Q@mail.gmail.com> (raw)
In-Reply-To: <52050ae7-bc10-eeb1-3162-1aa8586042cb@denx.de>

On Sun, Jul 4, 2021 at 12:25 PM Marek Vasut <marex@denx.de> wrote:
>
> On 7/4/21 5:35 PM, Fabio Estevam wrote:
> > Hi Marek,
>
> Hi,
>
> > On Sat, Jul 3, 2021 at 10:04 PM Marek Vasut <marex@denx.de> wrote:
> >
> >>> Retrieving the USB base addresses from DT would be preferred, yes, but
> >>> the current code does not do that.
> >>
> >> I implemented exactly that in mx6_parse_dt_addrs() , see:
> >> 4dcfa3bcbcb ("usb: ehci-mx6: Parse USB PHY and MISC offsets from DT")
> >
> > We are talking about USB_BASE_ADDR, right?
>
> All of the addresses used by the driver, I am trying hard to get rid of
> all that hard-coding in the driver. They should be parsed out of DT.
>

Marek,

I agree with trying to get rid of all the hard-coding but not all of
us are using SPL_DM_USB. I'm not sure how to best tell who is not
using DM in the SPL. I know I ran into issues as I'm supporting a
family of boards with the same U-Boot and could not find a way to
switch dt's early in the SPL after I've detected via EEPROM what board
I have.

Tim

> > imx6/imx7/imxrt provide the USB_BASE_ADDR as define.
>
> That's only because they still need to be fully converted, someone needs
> to write the PHY driver for those. For MX8M, the NOP PHY driver is used.
>
> > If we remove the imx6 definition from arch/arm/include/asm/arch-mx6/imx-regs.h
> > the ehci-mx6: driver fails to build.
> >
> > I didn't want to change ehci-mx6 in this aspect, so that's why I used
> > Frieder's patch that passes
> > USB_BASE_ADDR via define for i.MX8MM too.
> >
> > Is this an acceptable solution?
>
> No, let's not do that, because that exactly un-does the attempt to get
> rid of these hard-coded addresses. Please parse the address out of DT.
> And if you run into something which might still need hard-coded
> addresses, please fix it.
>
> The ehci-mx6 is bad, let's not make it worse, lets fix it instead.
>
> >>> Without providing these defines:
> >>>
> >>> drivers/usb/host/ehci-mx6.c:254:62: error: ‘USB_BASE_ADDR’ undeclared
> >>> (first use in this function); did you mean ‘SRC_BASE_ADDR’?
> >>>     254 |  struct usbnc_regs *usbnc = (struct usbnc_regs
> >>> *)(uintptr_t)(USB_BASE_ADDR +
> >>
> >> I suspect you need CONFIG_PHY for mx8m .
> >
> > CONFIG_PHY is already selected by imx8mm_evk_defconfig.
>
> USBNC is iMX7 specific , so you cannot hit that error on iMX8M, there is
> #elif defined(CONFIG_MX7) around it.