From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marek Vasut <marex@denx.de>
Date: Wed, 1 May 2013 21:34:29 +0200
Subject: [U-Boot] [PATCH v3 08/11] usb: ehci: add Faraday USB 2.0 EHCI
	support
In-Reply-To: <CAK65tU7QArXgv0YeofinL7QNbPyCfk+Q+ieEEdJDwzh_VNEXMw@mail.gmail.com>
References: <1366963360-2987-1-git-send-email-dantesu@gmail.com>
	<201304300050.14943.marex@denx.de>
	<CAK65tU7QArXgv0YeofinL7QNbPyCfk+Q+ieEEdJDwzh_VNEXMw@mail.gmail.com>
Message-ID: <201305012134.29592.marex@denx.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Dear Kuo-Jung Su,

> 2013/4/30 Marek Vasut <marex@denx.de>:
> > Dear Kuo-Jung Su,
> > 
> >> 2013/4/26 Marek Vasut <marex@denx.de>:
> >> > Dear Kuo-Jung Su,
> >> > 
> >> >> From: Kuo-Jung Su <dantesu@faraday-tech.com>
> >> >> 
> >> >> This patch add supports to both Faraday FUSBH200 and FOTG210,
> >> >> these controllers slightly differ from standard EHCI specification.
> >> > 
> >> > How do they differ?
> >> 
> >> 1. The reserved registers (0x1C - 0x3F) and CONFIGFLAG (0x40) register
> >> 
> >>     are not only un-implemented, but also removed from its register
> >>     address space, which means we have to update the 'struct ehci_hcor'
> >>     of ehci.h
> >> 
> >> 2. FUSBH200/FOTG210 doesn't properly follow the ECHI for ISOC
> >> implementations. (It should be a hardware bug, but no one wants to fix
> >> it.)
> >> 
> >> I'll add these description to commit log at next patch.
> > 
> > Mhmmm ... maybe you can add some hooks into ehci-hcd driver instead of
> > poluting it with ifdefs ? Weak-aliased functions would probably work
> > best to contain your weird stuff in your driver only.
> 
> Did you mean 'Not poluting ehci.h with ifdefs' ?
> 
> It looks to me that the best way is to leave ehci.h untouched, and
> update the ehci_submit_root() as following:
> ----------------------------
> ehci_submit_root()
> {
> ......
> #ifdef CONFIG_USB_EHCI_FARADAY
>     status_reg = (uint32_t *)((uint8_t *)&ctrl->hcor + 0x30);
> #else
>     status_reg = (uint32_t *)&ctrl->hcor->or_portsc[
>                  le16_to_cpu(req->index) - 1];
> #endif
> ......
> }
> ------------------------------
> 
> Would it be acceptable in this way?
> 
> If it's not, I'll make this patch as a separate patch with some hooks
> into ehci-hcd.

What about defining a

__weak uint32_t get_status_register()
{
...
}

function and override it's implementation in your driver?

Best regards,
Marek Vasut