Re: svn commit: r331306 - head/sys/dev/usb/controller

[Hans Petter Selasky]

On 03/22/18 13:02, Andrew Turner wrote:

On 21 Mar 2018, at 21:54, Hans Petter Selasky  wrote:

On 03/21/18 17:45, Andrew Turner wrote:

On 21 Mar 2018, at 15:37, Hans Petter Selasky  wrote:

On 03/21/18 16:24, Kyle Evans wrote:

+   {
+   .ep_end = 7,
+   .ep_fifosz_shift = 9,
+   .ep_fifosz_reg = MUSB2_VAL_FIFOSZ_512 | MUSB2_MASK_FIFODB,
+   },

I'm afraid I'm not familiar with this- why did .ep_fifosz_shift for
this case drop to 9? frx = 10 in the temp < 8 case in the removals of
the following hunks. Mostly curious because the others seemed to stay
the same.


Hi Andrew,

It might be an idea to keep the fifosz_shift at 10, else high-speed BULK 
traffic won't be double buffered, and this might affect performance.

Should the endpoint 1 size also be fixed? The register has it at 4k, but it 
wasn’t an 8k buffer.


No, because High-Speed BULK will only use 512 byte packets, and 4k is reserved 
for isochronous, which doesn't need double buffering at the moment.


Ok, in that case should we remove the double buffer flag on endpoint 1? The 
Linux driver seems to always use a 512 byte buffer there, and always doubles 
the offset increment when the double buffer flag is set.


Hi Andrew,

That's a good question. I think you should leave the double buffer flag 
on for this register. Double buffering is then later on re-programmed by:



static void
musbotg_clear_stall_sub(struct musbotg_softc *sc, uint16_t wMaxPacket,
uint8_t ep_no, uint8_t ep_type, uint8_t ep_dir)


If you look at the writes to MUSB2_REG_TXDBDIS and MUSB2_REG_RXDBDIS.

I think those registers override the "master" register. You might want 
to check the PRM to be absolutely sure.


--HPS
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r331306 - head/sys/dev/usb/controller

[Andrew Turner]

> On 21 Mar 2018, at 21:54, Hans Petter Selasky  wrote:
> 
> On 03/21/18 17:45, Andrew Turner wrote:
>>> On 21 Mar 2018, at 15:37, Hans Petter Selasky  wrote:
>>> 
>>> On 03/21/18 16:24, Kyle Evans wrote:
> +   {
> +   .ep_end = 7,
> +   .ep_fifosz_shift = 9,
> +   .ep_fifosz_reg = MUSB2_VAL_FIFOSZ_512 | MUSB2_MASK_FIFODB,
> +   },
 I'm afraid I'm not familiar with this- why did .ep_fifosz_shift for
 this case drop to 9? frx = 10 in the temp < 8 case in the removals of
 the following hunks. Mostly curious because the others seemed to stay
 the same.
>>> 
>>> Hi Andrew,
>>> 
>>> It might be an idea to keep the fifosz_shift at 10, else high-speed BULK 
>>> traffic won't be double buffered, and this might affect performance.
>> Should the endpoint 1 size also be fixed? The register has it at 4k, but it 
>> wasn’t an 8k buffer.
> 
> No, because High-Speed BULK will only use 512 byte packets, and 4k is 
> reserved for isochronous, which doesn't need double buffering at the moment.

Ok, in that case should we remove the double buffer flag on endpoint 1? The 
Linux driver seems to always use a 512 byte buffer there, and always doubles 
the offset increment when the double buffer flag is set.

Andrew

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r331306 - head/sys/dev/usb/controller

[Hans Petter Selasky]

On 03/21/18 17:45, Andrew Turner wrote:




On 21 Mar 2018, at 15:37, Hans Petter Selasky  wrote:

On 03/21/18 16:24, Kyle Evans wrote:

+   {
+   .ep_end = 7,
+   .ep_fifosz_shift = 9,
+   .ep_fifosz_reg = MUSB2_VAL_FIFOSZ_512 | MUSB2_MASK_FIFODB,
+   },

I'm afraid I'm not familiar with this- why did .ep_fifosz_shift for
this case drop to 9? frx = 10 in the temp < 8 case in the removals of
the following hunks. Mostly curious because the others seemed to stay
the same.


Hi Andrew,

It might be an idea to keep the fifosz_shift at 10, else high-speed BULK 
traffic won't be double buffered, and this might affect performance.


Should the endpoint 1 size also be fixed? The register has it at 4k, but it 
wasn’t an 8k buffer.



No, because High-Speed BULK will only use 512 byte packets, and 4k is 
reserved for isochronous, which doesn't need double buffering at the moment.


--HPS

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r331306 - head/sys/dev/usb/controller

[Andrew Turner]

> On 21 Mar 2018, at 15:37, Hans Petter Selasky  wrote:
> 
> On 03/21/18 16:24, Kyle Evans wrote:
>>> +   {
>>> +   .ep_end = 7,
>>> +   .ep_fifosz_shift = 9,
>>> +   .ep_fifosz_reg = MUSB2_VAL_FIFOSZ_512 | MUSB2_MASK_FIFODB,
>>> +   },
>> I'm afraid I'm not familiar with this- why did .ep_fifosz_shift for
>> this case drop to 9? frx = 10 in the temp < 8 case in the removals of
>> the following hunks. Mostly curious because the others seemed to stay
>> the same.
> 
> Hi Andrew,
> 
> It might be an idea to keep the fifosz_shift at 10, else high-speed BULK 
> traffic won't be double buffered, and this might affect performance.

Should the endpoint 1 size also be fixed? The register has it at 4k, but it 
wasn’t an 8k buffer.

Andrew

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r331306 - head/sys/dev/usb/controller

[Hans Petter Selasky]

On 03/21/18 16:24, Kyle Evans wrote:

+   {
+   .ep_end = 7,
+   .ep_fifosz_shift = 9,
+   .ep_fifosz_reg = MUSB2_VAL_FIFOSZ_512 | MUSB2_MASK_FIFODB,
+   },

I'm afraid I'm not familiar with this- why did .ep_fifosz_shift for
this case drop to 9? frx = 10 in the temp < 8 case in the removals of
the following hunks. Mostly curious because the others seemed to stay
the same.



Hi Andrew,

It might be an idea to keep the fifosz_shift at 10, else high-speed BULK 
traffic won't be double buffered, and this might affect performance.


--HPS
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r331306 - head/sys/dev/usb/controller

[Kyle Evans]

On Wed, Mar 21, 2018 at 10:17 AM, Andrew Turner  wrote:
> Author: andrew
> Date: Wed Mar 21 15:17:54 2018
> New Revision: 331306
> URL: https://svnweb.freebsd.org/changeset/base/331306
>
> Log:
>   Use a table to find the endpoint configuration
>
>   On the Allwinner SoCs we need to set a custom endpoint configuration. To
>   allow for this use a table to store the configuration so the attachment
>   can override it.
>
>   Reviewed by:  hselasky
>   Sponsored by: DARPA, AFRL
>   Differential Revision:https://reviews.freebsd.org/D14783
>
> Modified:
>   head/sys/dev/usb/controller/musb_otg.c
>   head/sys/dev/usb/controller/musb_otg.h
>
> Modified: head/sys/dev/usb/controller/musb_otg.c
> ==
> --- head/sys/dev/usb/controller/musb_otg.c  Wed Mar 21 15:13:47 2018  
>   (r331305)
> +++ head/sys/dev/usb/controller/musb_otg.c  Wed Mar 21 15:17:54 2018  
>   (r331306)
> @@ -149,6 +149,27 @@ static const struct usb_hw_ep_profile musbotg_ep_profi
> }
>  };
>
> +static const struct musb_otg_ep_cfg musbotg_ep_default[] = {
> +   {
> +   .ep_end = 1,
> +   .ep_fifosz_shift = 12,
> +   .ep_fifosz_reg = MUSB2_VAL_FIFOSZ_4096 | MUSB2_MASK_FIFODB,
> +   },
> +   {
> +   .ep_end = 7,
> +   .ep_fifosz_shift = 9,
> +   .ep_fifosz_reg = MUSB2_VAL_FIFOSZ_512 | MUSB2_MASK_FIFODB,
> +   },

I'm afraid I'm not familiar with this- why did .ep_fifosz_shift for
this case drop to 9? frx = 10 in the temp < 8 case in the removals of
the following hunks. Mostly curious because the others seemed to stay
the same.

> [..snip..]

> @@ -3193,20 +3220,15 @@ musbotg_init(struct musbotg_softc *sc)
>
> if (dynfifo) {
> if (frx && (temp <= nrx)) {
> -   if (temp == 1) {
> -   frx = 12;   /* 4K */
> -   MUSB2_WRITE_1(sc, MUSB2_REG_RXFIFOSZ,
> -   MUSB2_VAL_FIFOSZ_4096 |
> -   MUSB2_MASK_FIFODB);
> -   } else if (temp < 8) {
> -   frx = 10;   /* 1K */
> -   MUSB2_WRITE_1(sc, MUSB2_REG_RXFIFOSZ,
> -   MUSB2_VAL_FIFOSZ_512 |
> -   MUSB2_MASK_FIFODB);
> -   } else {
> -   frx = 7;/* 128 bytes */
> -   MUSB2_WRITE_1(sc, MUSB2_REG_RXFIFOSZ,
> -   MUSB2_VAL_FIFOSZ_128);
> +   for (i = 0; sc->sc_ep_cfg[i].ep_end >= 0; 
> i++) {
> +   cfg = >sc_ep_cfg[i];
> +   if (temp <= cfg->ep_end) {
> +   frx = cfg->ep_fifosz_shift;
> +   MUSB2_WRITE_1(sc,
> +   MUSB2_REG_RXFIFOSZ,
> +   cfg->ep_fifosz_reg);
> +   break;
> +   }
> }
>
> MUSB2_WRITE_2(sc, MUSB2_REG_RXFIFOADD,
> @@ -3215,20 +3237,15 @@ musbotg_init(struct musbotg_softc *sc)
> offset += (1 << frx);
> }
> if (ftx && (temp <= ntx)) {
> -   if (temp == 1) {
> -   ftx = 12;   /* 4K */
> -   MUSB2_WRITE_1(sc, MUSB2_REG_TXFIFOSZ,
> -   MUSB2_VAL_FIFOSZ_4096 |
> -   MUSB2_MASK_FIFODB);
> -   } else if (temp < 8) {
> -   ftx = 10;   /* 1K */
> -   MUSB2_WRITE_1(sc, MUSB2_REG_TXFIFOSZ,
> -   MUSB2_VAL_FIFOSZ_512 |
> -   MUSB2_MASK_FIFODB);
> -   } else {
> -   ftx = 7;/* 128 bytes */
> -   MUSB2_WRITE_1(sc, MUSB2_REG_TXFIFOSZ,
> -   MUSB2_VAL_FIFOSZ_128);
> +   for (i = 0; sc->sc_ep_cfg[i].ep_end >= 0; 
> i++) {
> +   cfg = >sc_ep_cfg[i];
> +   if (temp <= cfg->ep_end) {
> +   ftx = cfg->ep_fifosz_shift;
> +   

svn commit: r331306 - head/sys/dev/usb/controller

[Andrew Turner]

Author: andrew
Date: Wed Mar 21 15:17:54 2018
New Revision: 331306
URL: https://svnweb.freebsd.org/changeset/base/331306

Log:
  Use a table to find the endpoint configuration
  
  On the Allwinner SoCs we need to set a custom endpoint configuration. To
  allow for this use a table to store the configuration so the attachment
  can override it.
  
  Reviewed by:  hselasky
  Sponsored by: DARPA, AFRL
  Differential Revision:https://reviews.freebsd.org/D14783

Modified:
  head/sys/dev/usb/controller/musb_otg.c
  head/sys/dev/usb/controller/musb_otg.h

Modified: head/sys/dev/usb/controller/musb_otg.c
==
--- head/sys/dev/usb/controller/musb_otg.c  Wed Mar 21 15:13:47 2018
(r331305)
+++ head/sys/dev/usb/controller/musb_otg.c  Wed Mar 21 15:17:54 2018
(r331306)
@@ -149,6 +149,27 @@ static const struct usb_hw_ep_profile musbotg_ep_profi
}
 };
 
+static const struct musb_otg_ep_cfg musbotg_ep_default[] = {
+   {
+   .ep_end = 1,
+   .ep_fifosz_shift = 12,
+   .ep_fifosz_reg = MUSB2_VAL_FIFOSZ_4096 | MUSB2_MASK_FIFODB,
+   },
+   {
+   .ep_end = 7,
+   .ep_fifosz_shift = 9,
+   .ep_fifosz_reg = MUSB2_VAL_FIFOSZ_512 | MUSB2_MASK_FIFODB,
+   },
+   {
+   .ep_end = 15,
+   .ep_fifosz_shift = 7,
+   .ep_fifosz_reg = MUSB2_VAL_FIFOSZ_128,
+   },
+   {
+   .ep_end = -1,
+   },
+};
+
 static int
 musbotg_channel_alloc(struct musbotg_softc *sc, struct musbotg_td *td, uint8_t 
is_tx)
 {
@@ -3059,7 +3080,9 @@ musbotg_clear_stall(struct usb_device *udev, struct us
 usb_error_t
 musbotg_init(struct musbotg_softc *sc)
 {
+   const struct musb_otg_ep_cfg *cfg;
struct usb_hw_ep_profile *pf;
+   int i;
uint16_t offset;
uint8_t nrx;
uint8_t ntx;
@@ -3075,6 +3098,10 @@ musbotg_init(struct musbotg_softc *sc)
sc->sc_bus.usbrev = USB_REV_2_0;
sc->sc_bus.methods = _bus_methods;
 
+   /* Set a default endpoint configuration */
+   if (sc->sc_ep_cfg == NULL)
+   sc->sc_ep_cfg = musbotg_ep_default;
+
USB_BUS_LOCK(>sc_bus);
 
/* turn on clocks */
@@ -3193,20 +3220,15 @@ musbotg_init(struct musbotg_softc *sc)
 
if (dynfifo) {
if (frx && (temp <= nrx)) {
-   if (temp == 1) {
-   frx = 12;   /* 4K */
-   MUSB2_WRITE_1(sc, MUSB2_REG_RXFIFOSZ, 
-   MUSB2_VAL_FIFOSZ_4096 |
-   MUSB2_MASK_FIFODB);
-   } else if (temp < 8) {
-   frx = 10;   /* 1K */
-   MUSB2_WRITE_1(sc, MUSB2_REG_RXFIFOSZ, 
-   MUSB2_VAL_FIFOSZ_512 |
-   MUSB2_MASK_FIFODB);
-   } else {
-   frx = 7;/* 128 bytes */
-   MUSB2_WRITE_1(sc, MUSB2_REG_RXFIFOSZ, 
-   MUSB2_VAL_FIFOSZ_128);
+   for (i = 0; sc->sc_ep_cfg[i].ep_end >= 0; i++) {
+   cfg = >sc_ep_cfg[i];
+   if (temp <= cfg->ep_end) {
+   frx = cfg->ep_fifosz_shift;
+   MUSB2_WRITE_1(sc,
+   MUSB2_REG_RXFIFOSZ,
+   cfg->ep_fifosz_reg);
+   break;
+   }
}
 
MUSB2_WRITE_2(sc, MUSB2_REG_RXFIFOADD,
@@ -3215,20 +3237,15 @@ musbotg_init(struct musbotg_softc *sc)
offset += (1 << frx);
}
if (ftx && (temp <= ntx)) {
-   if (temp == 1) {
-   ftx = 12;   /* 4K */
-   MUSB2_WRITE_1(sc, MUSB2_REG_TXFIFOSZ,
-   MUSB2_VAL_FIFOSZ_4096 |
-   MUSB2_MASK_FIFODB);
-   } else if (temp < 8) {
-   ftx = 10;   /* 1K */
-   MUSB2_WRITE_1(sc, MUSB2_REG_TXFIFOSZ,
-   MUSB2_VAL_FIFOSZ_512 |
-   MUSB2_MASK_FIFODB);
-   } else {
-