Change in coreboot[master]: soc/intel/tigerlake: Set default USB3 de-emphasis to -3.5dB

[Duncan Laurie (Code Review)]

Duncan Laurie has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/43831 )

Change subject: soc/intel/tigerlake: Set default USB3 de-emphasis to -3.5dB
......................................................................

soc/intel/tigerlake: Set default USB3 de-emphasis to -3.5dB

The HSIO tuning guide recommendation for the default USB3 settings is to
have de-emphasis set to -3.5dB with the equation 20*log(X/64).  0x29 results
in a value close to -3.5dB and it is the value that was used for the default
on past platforms so I used it here as well.

BUG=b:160721468
TEST=Ensure WWAN device does not disconnect during use.

Change-Id: Ia594996cb55523dacce0d4bef98cc217321c62de
Signed-off-by: Duncan Laurie 
---
M src/soc/intel/tigerlake/include/soc/usb.h
1 file changed, 1 insertion(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/31/43831/1

diff --git a/src/soc/intel/tigerlake/include/soc/usb.h b/src/soc/intel/tigerlake/include/soc/usb.h
index 247b0ba..d4836c9 100644
--- a/src/soc/intel/tigerlake/include/soc/usb.h
+++ b/src/soc/intel/tigerlake/include/soc/usb.h
@@ -132,7 +132,7 @@
 #define USB3_PORT_DEFAULT(pin) { \
    .enable           = 1, \
    .ocpin            = (pin), \
-	.tx_de_emp        = 0x0, \
+	.tx_de_emp        = 0x29, \
    .tx_downscale_amp = 0x00, \
 }

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43831 )

Change subject: soc/intel/tigerlake: Set default USB3 de-emphasis to -3.5dB
......................................................................

Patch Set 1: Code-Review+1

(1 comment)

https://review.coreboot.org/c/coreboot/+/43831/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/43831/1//COMMIT_MSG@10 
PS1, Line 10: log
Hmm, using 0x29 = 41 results in -3.86 dB, which isn't very accurate (0x2a or 0x2b would be closer to -3.5 dB)

Also, Only SKL seems to use that de-emphasis value. Other platforms just set it to zero, which is treated as a no-op when copying the parameters to the FSP UPDs. However, the UPDs seem to default to 0x29. Maybe the TGL FSP has some problem?

[Duncan Laurie (Code Review)]

Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43831 )

Change subject: soc/intel/tigerlake: Set default USB3 de-emphasis to -3.5dB
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/43831/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/43831/1//COMMIT_MSG@10 
PS1, Line 10: log
> Hmm, using 0x29 = 41 results in -3. […]

Yeah I can't remember where the original 0x29 value came from, but that is what was used to validate on Volteer before I found this algorithm in a doc.  I can't easily repro the original issue (no cell service at home) so I can't test 0x2a myself with the original issue but I imagine it wouldn't make much difference.

I suspect other platforms that are setting 0 for the default may have the same issue that TGL has, but it is going to be hard to verify without USB test equipment or a reproduction case like we had on Volteer.

As far as I can tell FSP is checking Usb3HsioTx*Enable==TRUE before applying any settings, and these UPDs all default to 0 so nothing will get applied unless coreboot sets it up.

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43831 )

Change subject: soc/intel/tigerlake: Set default USB3 de-emphasis to -3.5dB
......................................................................

Patch Set 1: Code-Review+2

(1 comment)

https://review.coreboot.org/c/coreboot/+/43831/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/43831/1//COMMIT_MSG@10 
PS1, Line 10: log
> Yeah I can't remember where the original 0x29 value came from, but that is what was used to validate […]

Ack. I can see that default value of 0x29 in FSP UPDs anyway.

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/43831 )

Change subject: soc/intel/tigerlake: Set default USB3 de-emphasis to -3.5dB
......................................................................

soc/intel/tigerlake: Set default USB3 de-emphasis to -3.5dB

The HSIO tuning guide recommendation for the default USB3 settings is to
have de-emphasis set to -3.5dB with the equation 20*log(X/64).  0x29 results
in a value close to -3.5dB and it is the value that was used for the default
on past platforms so I used it here as well.

BUG=b:160721468
TEST=Ensure WWAN device does not disconnect during use.

Change-Id: Ia594996cb55523dacce0d4bef98cc217321c62de
Signed-off-by: Duncan Laurie 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/43831
Tested-by: build bot (Jenkins) 
Reviewed-by: Angel Pons 
---
M src/soc/intel/tigerlake/include/soc/usb.h
1 file changed, 1 insertion(+), 1 deletion(-)

Approvals:
  build bot (Jenkins): Verified
  Angel Pons: Looks good to me, approved

diff --git a/src/soc/intel/tigerlake/include/soc/usb.h b/src/soc/intel/tigerlake/include/soc/usb.h
index 247b0ba..d4836c9 100644
--- a/src/soc/intel/tigerlake/include/soc/usb.h
+++ b/src/soc/intel/tigerlake/include/soc/usb.h
@@ -132,7 +132,7 @@
 #define USB3_PORT_DEFAULT(pin) { \
    .enable           = 1, \
    .ocpin            = (pin), \
-	.tx_de_emp        = 0x0, \
+	.tx_de_emp        = 0x29, \
    .tx_downscale_amp = 0x00, \
 }