Date: Mon, 22 Dec 2014 10:10:46 +0100 From: Hans Petter Selasky <hps@selasky.org> To: Kohji Okuno <okuno.kohji@jp.panasonic.com> Cc: freebsd-usb@freebsd.org Subject: Re: About XHCI_TD_PAGE_SIZE. Message-ID: <5497E016.7020809@selasky.org> In-Reply-To: <20141222.103833.2105768702793613386.okuno.kohji@jp.panasonic.com> References: <20141217.110334.1811096089058017495.okuno.kohji@jp.panasonic.com> <CADoY-6gXxYvw-Raq-LW6q_QmiG8F=W6msZs2Q3vcx3s1LnksLw@mail.gmail.com> <549541BC.6070505@selasky.org> <20141222.103833.2105768702793613386.okuno.kohji@jp.panasonic.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On 12/22/14 02:38, Kohji Okuno wrote:
> From: Hans Petter Selasky <hps@selasky.org>
> Subject: Re: About XHCI_TD_PAGE_SIZE.
> Date: Sat, 20 Dec 2014 10:30:36 +0100
>
>> On 12/17/14 05:42, Nidal Khalil wrote:
>>> I agree. Thanks
>>>
>>> Nidal
>>> On Dec 16, 2014 6:25 PM, "Kohji Okuno" <okuno.kohji@jp.panasonic.com> wrote:
>>>
>>>> Hi Hans,
>>>>
>>>> If we use PAGE_SIZE as USB_PAGE_SIZE, we should use PAGE_SIZE as
>>>> XHCI_TD_PAGE_SIZE, too. I think.
>>>>
>>>> As you know, one TRB can use 1~64kB for the transfer length.
>>>>
>>
>> Hi,
>>
>> We currently only check if 4K pages are supported by the hardware. If you
>> change the value of XHCI_TD_PAGE_SIZE, you will also need to change the checks
>> other places. You know that PAGE_SIZE is not a constant?
>>
>> Do you have a complete patch?
>>
>> --HPS
>>
>
> Hi,
>
> XHCI_TD_PAGE_SIZE is used at only 2-points.
>
> 1. use as XHCI_TD_PAYLOAD_MAX (XHCI_TD_PAGE_NBUF) in xhci.h
> We sholud change as the following, I think.
>
> - #define XHCI_TD_PAGE_NBUF 17 /* units, room enough for 64Kbytes */
> - #define XHCI_TD_PAGE_SIZE 4096 /* bytes */
> - #define XHCI_TD_PAYLOAD_MAX (XHCI_TD_PAGE_SIZE * (XHCI_TD_PAGE_NBUF - 1))
> + #define XHCI_TD_PAYLOAD_MAX (64*1024) /* bytes */
> + #define XHCI_TD_PAGE_SIZE PAGE_SIZE /* bytes */
> + /* units, room enough for 64Kbytes */
> + #define XHCI_TD_PAGE_NBUF (XHCI_TD_PAYLOAD_MAX/XHCI_TD_PAGE_SIZE + 1)
>
> 2. use as the maximum length of TRB.
> If PAGE_SIZE is 8kB, buf_res.length may be 8kB.
> But, we can set 1B~64kB for length of TRB. This is the spcification
> of xHCI. So, we don't need change this point.
>
> xhci.c
> 1807 /* check for maximum length */
> 1808 if (buf_res.length > XHCI_TD_PAGE_SIZE)
> 1809 buf_res.length = XHCI_TD_PAGE_SIZE;
>
Hi Kohji,
I see your points. By doing this you save some memory in the descriptor
layout for 8K page size - right?
BTW: Do you think the following check is OK, or should it be extended to
check also for 8K?
if (!(XREAD4(sc, oper, XHCI_PAGESIZE) & XHCI_PAGESIZE_4K)) {
device_printf(sc->sc_bus.parent, "Controller does "
"not support 4K page size.\n");
return (USB_ERR_IOERROR);
}
I guess your patch is more in the direction of optimisation. Is it very
urgent? Or is it fine if I handle it the beginning of January?
Thank you for your input and review!
--HPS
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?5497E016.7020809>