Brandon Breitenstein has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/40245 )
Change subject: mainboard/volteer: Update Aux settings for Port 0 ......................................................................
mainboard/volteer: Update Aux settings for Port 0
On Volteer port 0 (MB PORT) does not have a retimer so the port needs to be configured for the SOC to handle Aux orientation flipping. This requires 2 changes setting the TcssAuxOri UPD to 1 for port 0 (Bit 0) and configuring AUXP and AUXN GPIOs to Native Function 6 so SOC can control the orientation
BUG=b:145220205 BRANCH=NONE TEST=booted Volteer proto 2 and verified that the AUX channels flip when the cable is flipped
Change-Id: Ic81adc24d10322cc305bf0fa4c38514468ea0942 Signed-off-by: Brandon Breitenstein brandon.breitenstein@intel.com --- M src/mainboard/google/volteer/variants/baseboard/devicetree.cb M src/mainboard/google/volteer/variants/baseboard/gpio.c 2 files changed, 12 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/45/40245/1
diff --git a/src/mainboard/google/volteer/variants/baseboard/devicetree.cb b/src/mainboard/google/volteer/variants/baseboard/devicetree.cb index 361e563..df7e90b 100644 --- a/src/mainboard/google/volteer/variants/baseboard/devicetree.cb +++ b/src/mainboard/google/volteer/variants/baseboard/devicetree.cb @@ -127,7 +127,16 @@
# TCSS USB3 register "TcssXhciEn" = "1" - register "TcssAuxOri" = "0" + register "TcssAuxOri" = "1" + register "IomTypeCPortPadCfg[0]" = "0x090E000A" + register "IomTypeCPortPadCfg[1]" = "0x090E000D" + register "IomTypeCPortPadCfg[2]" = "0x0" + register "IomTypeCPortPadCfg[3]" = "0x0" + register "IomTypeCPortPadCfg[4]" = "0x0" + register "IomTypeCPortPadCfg[5]" = "0x0" + register "IomTypeCPortPadCfg[6]" = "0x0" + register "IomTypeCPortPadCfg[7]" = "0x0" +
# DP port register "DdiPortAConfig" = "1" # eDP diff --git a/src/mainboard/google/volteer/variants/baseboard/gpio.c b/src/mainboard/google/volteer/variants/baseboard/gpio.c index fff0381..09417d7 100644 --- a/src/mainboard/google/volteer/variants/baseboard/gpio.c +++ b/src/mainboard/google/volteer/variants/baseboard/gpio.c @@ -216,13 +216,13 @@ /* E9 : USB2_OC0# ==> USB_C1_OC_ODL */ PAD_CFG_NF(GPP_E9, NONE, DEEP, NF1), /* E10 : SPI1_CS# ==> USB_C0_AUXP_DC */ - PAD_CFG_GPO(GPP_E10, 0, DEEP), + PAD_CFG_NF(GPP_E10, NONE, DEEP, NF6), /* E11 : SPI1_CLK ==> SD_PE_WAKE_ODL */ PAD_CFG_GPI(GPP_E11, NONE, DEEP), /* E12 : SPI1_MISO_IO1 ==> NOT USED */ PAD_NC(GPP_E12, NONE), /* E13 : SPI1_MOSI_IO0 ==> USB_C0_AUXN_DC */ - PAD_CFG_GPO(GPP_E13, 0, DEEP), + PAD_CFG_NF(GPP_E13, NONE, DEEP, NF6), /* E14 : DDPC_HPDA ==> SOC_EDP_HPD */ PAD_CFG_NF(GPP_E14, NONE, DEEP, NF1), /* E15 : ISH_GP6 ==> TRACKPAD_INT_ODL */
Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40245 )
Change subject: mainboard/volteer: Update Aux settings for Port 0 ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/40245/1/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/40245/1/src/mainboard/google/voltee... PS1, Line 131: 0x090E000A putting the pad encoding here is pretty obscure. do we really need more than a boolean here? perhaps we can do the pad encoding (lookup) in fsp_params.c or something?
it would take a bit of digging to figure out the values for the rest of the IomTypeCPortPadCfg[...] should the need arise.
https://review.coreboot.org/c/coreboot/+/40245/1/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/40245/1/src/mainboard/google/voltee... PS1, Line 219: NF6 doesn't the FSP automatically do this when IomTypeCPortPadCfg[0], IomTypeCPortPadCfg[1] are non-zero?
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40245 )
Change subject: mainboard/volteer: Update Aux settings for Port 0 ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/40245/1/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/40245/1/src/mainboard/google/voltee... PS1, Line 130: 1 Brandon - have you tried doing only the pad configuration in gpio.c and not setting this UPD? Instead set IomTypeCPortPadCfg[0-7] as 0. It looks like all that FSP is doing with this is configure the pad which we should be able to do in coreboot and not rely on FSP to do that.
https://review.coreboot.org/c/coreboot/+/40245/1/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/40245/1/src/mainboard/google/voltee... PS1, Line 219: NF6
doesn't the FSP automatically do this when […]
We should not rely on FSP to configure the pads for us.
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40245 )
Change subject: mainboard/volteer: Update Aux settings for Port 0 ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40245/1/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/40245/1/src/mainboard/google/voltee... PS1, Line 130: 1
Brandon - have you tried doing only the pad configuration in gpio. […]
Tested this out setting everything to the default it was already setting prior to this patch and can confirm this part does nothing flip is working without the IomTypeCPortPadCfg being set. Will update this patch accordingly and abandon the other patch that adds this setting as configurable
Hello build bot (Jenkins), Furquan Shaikh, Wonkyu Kim, Caveh Jalali, Ravishankar Sarawadi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40245
to look at the new patch set (#2).
Change subject: mainboard/volteer: Update Aux settings for Port 0 ......................................................................
mainboard/volteer: Update Aux settings for Port 0
On Volteer port 0 (MB PORT) does not have a retimer so the port needs to be configured for the SOC to handle Aux orientation flipping. This requires 2 changes setting the TcssAuxOri UPD to 1 for port 0 (Bit 0) and configuring AUXP and AUXN GPIOs to Native Function 6 so SOC can control the orientation
BUG=b:145220205 BRANCH=NONE TEST=booted Volteer proto 2 and verified that the AUX channels flip when the cable is flipped
Change-Id: Ic81adc24d10322cc305bf0fa4c38514468ea0942 Signed-off-by: Brandon Breitenstein brandon.breitenstein@intel.com --- M src/mainboard/google/volteer/variants/baseboard/devicetree.cb M src/mainboard/google/volteer/variants/baseboard/gpio.c 2 files changed, 3 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/45/40245/2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40245 )
Change subject: mainboard/volteer: Update Aux settings for Port 0 ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/40245/1/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/40245/1/src/mainboard/google/voltee... PS1, Line 130: 1
Tested this out setting everything to the default it was already setting prior to this patch and can […]
Thanks for confirming this Brandon. I think we need one additional change. Please see my comment here: https://review.coreboot.org/c/coreboot/+/40244/1/src/soc/intel/tigerlake/fsp...
https://review.coreboot.org/c/coreboot/+/40245/2/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/40245/2/src/mainboard/google/voltee... PS2, Line 129: 1 What exactly is this doing? Disabling Port1 retimer?
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40245 )
Change subject: mainboard/volteer: Update Aux settings for Port 0 ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40245/2/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/40245/2/src/mainboard/google/voltee... PS2, Line 129: 1
What exactly is this doing? Disabling Port1 retimer?
Correct this is disabling retimer and going to SOC configured orientation
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40245 )
Change subject: mainboard/volteer: Update Aux settings for Port 0 ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40245/2/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/40245/2/src/mainboard/google/voltee... PS2, Line 129: 1
Correct this is disabling retimer and going to SOC configured orientation
Can you please add a comment here indicating that.
Hello build bot (Jenkins), Furquan Shaikh, Wonkyu Kim, Caveh Jalali, Ravishankar Sarawadi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40245
to look at the new patch set (#3).
Change subject: mainboard/volteer: Update Aux settings for Port 0 ......................................................................
mainboard/volteer: Update Aux settings for Port 0
On Volteer port 0 (MB PORT) does not have a retimer so the port needs to be configured for the SOC to handle Aux orientation flipping. This requires 2 changes setting the TcssAuxOri UPD to 1 for port 0 (Bit 0) and configuring AUXP and AUXN GPIOs to Native Function 6 so SOC can control the orientation
BUG=b:145220205 BRANCH=NONE TEST=booted Volteer proto 2 and verified that the AUX channels flip when the cable is flipped
Change-Id: Ic81adc24d10322cc305bf0fa4c38514468ea0942 Signed-off-by: Brandon Breitenstein brandon.breitenstein@intel.com --- M src/mainboard/google/volteer/variants/baseboard/devicetree.cb M src/mainboard/google/volteer/variants/baseboard/gpio.c 2 files changed, 5 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/45/40245/3
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40245 )
Change subject: mainboard/volteer: Update Aux settings for Port 0 ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40245/2/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/40245/2/src/mainboard/google/voltee... PS2, Line 129: 1
Can you please add a comment here indicating that.
Done
Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40245 )
Change subject: mainboard/volteer: Update Aux settings for Port 0 ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40245/1/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/40245/1/src/mainboard/google/voltee... PS1, Line 219: NF6
We should not rely on FSP to configure the pads for us.
quick question - which runs first?
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40245 )
Change subject: mainboard/volteer: Update Aux settings for Port 0 ......................................................................
Patch Set 3: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/40245/1/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/40245/1/src/mainboard/google/voltee... PS1, Line 219: NF6
quick question - which runs first?
coreboot GPIO initialization happens before FSP-S runs.
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40245 )
Change subject: mainboard/volteer: Update Aux settings for Port 0 ......................................................................
Patch Set 3:
(3 comments)
https://review.coreboot.org/c/coreboot/+/40245/1/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/40245/1/src/mainboard/google/voltee... PS1, Line 130: 1
Thanks for confirming this Brandon. I think we need one additional change. […]
Ack
https://review.coreboot.org/c/coreboot/+/40245/1/src/mainboard/google/voltee... PS1, Line 131: 0x090E000A
putting the pad encoding here is pretty obscure. […]
removed this and will instead rely on baseboard gpio
https://review.coreboot.org/c/coreboot/+/40245/1/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/40245/1/src/mainboard/google/voltee... PS1, Line 219: NF6
coreboot GPIO initialization happens before FSP-S runs.
Ack
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40245 )
Change subject: mainboard/volteer: Update Aux settings for Port 0 ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40245/1/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/40245/1/src/mainboard/google/voltee... PS1, Line 130: 1
Ack
After some more testing we have found that this might be unstable without the UPD patch if you look in src/third_party/fsp/tgl/fsp/ClientOneSiliconPkg/IpBlock/Tcss/LibraryPrivate/PeiTcssInitLib/TcssInit.c there appears to be some communication to IOM about what GPIOs to use so it seems sometimes the detection isn't working without the UPD set
Wonkyu Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40245 )
Change subject: mainboard/volteer: Update Aux settings for Port 0 ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40245/1/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/40245/1/src/mainboard/google/voltee... PS1, Line 130: 1
After some more testing we have found that this might be unstable without the UPD patch if you look […]
IOM should know the which GPIOs to use for aux orientation and this is done by IomTypeCPortPadCfg parameters.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40245 )
Change subject: mainboard/volteer: Update Aux settings for Port 0 ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40245/1/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/40245/1/src/mainboard/google/voltee... PS1, Line 130: 1
After some more testing we have found that this might be unstable without the UPD patch if you look […]
Are you referring to VW configuration?
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40245 )
Change subject: mainboard/volteer: Update Aux settings for Port 0 ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40245/1/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/40245/1/src/mainboard/google/voltee... PS1, Line 130: 1
Are you referring to VW configuration?
Also, did you try setting IomTypeCPortPadCfg to all 0s instead of 0x9000000? Does that make a difference?
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40245 )
Change subject: mainboard/volteer: Update Aux settings for Port 0 ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40245/1/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/40245/1/src/mainboard/google/voltee... PS1, Line 130: 1
Also, did you try setting IomTypeCPortPadCfg to all 0s instead of 0x9000000? Does that make a differ […]
Let me verify on 0s I believe I tested this but want to double check as I've tested a ton of things the last few days
Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40245 )
Change subject: mainboard/volteer: Update Aux settings for Port 0 ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40245/3/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/40245/3/src/mainboard/google/voltee... PS3, Line 219: NF6 i'm having trouble finding references to NF6 - which intel docID describes what NF6 means for these pins?
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40245 )
Change subject: mainboard/volteer: Update Aux settings for Port 0 ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40245/3/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/40245/3/src/mainboard/google/voltee... PS3, Line 219: NF6
i'm having trouble finding references to NF6 - which intel docID describes what NF6 means for these […]
I set it to Native Function 6 based on this document https://docs.google.com/document/d/1nhWN1rzMFXBOb6XVSvQeNwPanKTq_Iai_kLdKfFi... perhaps Will or Aline would know more about what this actually means
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40245 )
Change subject: mainboard/volteer: Update Aux settings for Port 0 ......................................................................
Patch Set 3: Code-Review+1
(2 comments)
https://review.coreboot.org/c/coreboot/+/40245/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40245/3//COMMIT_MSG@7 PS3, Line 7: mainboard/volteer mb/google/volteer
https://review.coreboot.org/c/coreboot/+/40245/3//COMMIT_MSG@13 PS3, Line 13: control the orientation Please add a dot/period at the end of sentences please.
Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40245 )
Change subject: mainboard/volteer: Update Aux settings for Port 0 ......................................................................
Patch Set 3:
thanks for the pointer to the doc.
the doc describes a 2nd required configuration step for AUX DC biasing:
tell the IOM which GPIOs are used for the type-C pull{up,down} function for a particular port. where do we configure the SoC to use GPP_E10 and GPP_E13 for port 0?
i'm sorry, but i'm not following how that's happening. wonkyu also mentioned this needs to happen in his comment.
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40245 )
Change subject: mainboard/volteer: Update Aux settings for Port 0 ......................................................................
Patch Set 3:
Patch Set 3:
thanks for the pointer to the doc.
the doc describes a 2nd required configuration step for AUX DC biasing:
tell the IOM which GPIOs are used for the type-C pull{up,down} function for a particular port. where do we configure the SoC to use GPP_E10 and GPP_E13 for port 0?
i'm sorry, but i'm not following how that's happening. wonkyu also mentioned this needs to happen in his comment.
The IomTypeCPortPadCfg UPD takes care of this in FSP if you take a look at the file I pointed out it is also configuring IOM to use the GPIOs as pull up and pull down
Hello build bot (Jenkins), Furquan Shaikh, Wonkyu Kim, Caveh Jalali, Ravishankar Sarawadi, Paul Menzel,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40245
to look at the new patch set (#4).
Change subject: mb/google/volteer: Update Aux settings for Port 0 ......................................................................
mb/google/volteer: Update Aux settings for Port 0
On Volteer port 0 (MB PORT) does not have a retimer so the port needs to be configured for the SOC to handle Aux orientation flipping. This requires 2 changes setting the TcssAuxOri UPD to 1 for port 0 (Bit 0) and configuring AUXP and AUXN GPIOs to Native Function 6 so SOC can control the orientation.
BUG=b:145220205 BRANCH=NONE TEST=booted Volteer proto 2 and verified that the AUX channels flip when the cable is flipped
Change-Id: Ic81adc24d10322cc305bf0fa4c38514468ea0942 Signed-off-by: Brandon Breitenstein brandon.breitenstein@intel.com --- M src/mainboard/google/volteer/variants/baseboard/devicetree.cb M src/mainboard/google/volteer/variants/baseboard/gpio.c 2 files changed, 5 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/45/40245/4
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40245 )
Change subject: mb/google/volteer: Update Aux settings for Port 0 ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/c/coreboot/+/40245/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40245/3//COMMIT_MSG@7 PS3, Line 7: mainboard/volteer
mb/google/volteer
Done
https://review.coreboot.org/c/coreboot/+/40245/3//COMMIT_MSG@13 PS3, Line 13: control the orientation
Please add a dot/period at the end of sentences please.
Done
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40245 )
Change subject: mb/google/volteer: Update Aux settings for Port 0 ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40245/1/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/40245/1/src/mainboard/google/voltee... PS1, Line 130: 1
Let me verify on 0s I believe I tested this but want to double check as I've tested a ton of things […]
I see what you are saying. Humm.. TcssAuxOri still needs to be set at least until coreboot can configure all these registers.
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40245 )
Change subject: mb/google/volteer: Update Aux settings for Port 0 ......................................................................
Patch Set 4:
returning this patch to patchset 1 due to some weirdness occuring when the IOM setting is not correctly assigned to a GPIO
Hello build bot (Jenkins), Furquan Shaikh, Wonkyu Kim, Caveh Jalali, Ravishankar Sarawadi, Paul Menzel,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40245
to look at the new patch set (#5).
Change subject: mainboard/volteer: Update Aux settings for Port 0 ......................................................................
mainboard/volteer: Update Aux settings for Port 0
On Volteer port 0 (MB PORT) does not have a retimer so the port needs to be configured for the SOC to handle Aux orientation flipping. This requires 2 changes setting the TcssAuxOri UPD to 1 for port 0 (Bit 0) and configuring AUXP and AUXN GPIOs to Native Function 6 so SOC can control the orientation
BUG=b:145220205 BRANCH=NONE TEST=booted Volteer proto 2 and verified that the AUX channels flip when the cable is flipped
Change-Id: Ic81adc24d10322cc305bf0fa4c38514468ea0942 Signed-off-by: Brandon Breitenstein brandon.breitenstein@intel.com --- M src/mainboard/google/volteer/variants/baseboard/devicetree.cb M src/mainboard/google/volteer/variants/baseboard/gpio.c 2 files changed, 12 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/45/40245/5
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40245 )
Change subject: mainboard/volteer: Update Aux settings for Port 0 ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40245/1/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/40245/1/src/mainboard/google/voltee... PS1, Line 130: 1
I see what you are saying. Humm.. […]
so all 0 works for the ports not being used. however for the port with no retimer there is some inconsistency with functionality when the register is not set to the correct GPIO pad. reverting this back to the original patchset so that IOM can correctly tell SOC which GPIOs to switch for the orientation flip of the cable
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40245 )
Change subject: mainboard/volteer: Update Aux settings for Port 0 ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40245/5/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/40245/5/src/mainboard/google/voltee... PS5, Line 128: 0x090E000A These values are hardcoded in an FSP header, but could we build it up from a macro so we don't need magic values in devicetree?
#define FSP_PAD(group, pad) 0x09000000 | (group) << 16 | (pad)
register "IomTypeCPortPadCfg[0]" = "FSP_PAD(GPP_E, GPP_E10-GPP_E0)" (or something similar)
This breaks down if the group numbers don't match, and something seems to be off between FSP and coreboot so we might need a new table:
coreboot: #define GPP_E 0xC
FSP: #define GPIO_VER4_LP_GROUP_GPP_E 0x090E
The GPP_E10-GPP_E0 isn't very readable either, but overall this is more informative than the magic value from a closed source header.
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40245 )
Change subject: mainboard/volteer: Update Aux settings for Port 0 ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40245/5/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/40245/5/src/mainboard/google/voltee... PS5, Line 128: 0x090E000A
These values are hardcoded in an FSP header, but could we build it up from a macro so we don't need […]
Yeah this makes sense I'll work on a better implementation that is more easily readable
Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40245 )
Change subject: mainboard/volteer: Update Aux settings for Port 0 ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40245/5/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/40245/5/src/mainboard/google/voltee... PS5, Line 242: /* E22 : DDPA_CTRLCLK ==> USB_C1_AUXP_DC: Retimer FW drives this pin */ : PAD_NC(GPP_E22, NONE), : /* E23 : DDPA_CTRLDATA ==> USB_C1_AUXN_DC: Retimer FW drives this pin */ : PAD_NC(GPP_E23, NONE), don't these need to be NF6 as well? the only difference from C0 is that high/low settings don't change with cable orientation because the retimer takes care of the orientation for AUX/SBU lanes.
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40245 )
Change subject: mainboard/volteer: Update Aux settings for Port 0 ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40245/5/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/40245/5/src/mainboard/google/voltee... PS5, Line 242: /* E22 : DDPA_CTRLCLK ==> USB_C1_AUXP_DC: Retimer FW drives this pin */ : PAD_NC(GPP_E22, NONE), : /* E23 : DDPA_CTRLDATA ==> USB_C1_AUXN_DC: Retimer FW drives this pin */ : PAD_NC(GPP_E23, NONE),
don't these need to be NF6 as well? […]
If these are set to NF6 they will be SOC controlled instead of Retimer controller. The GPIO only needs to be set to NF6 in cases where there is no retimer as specified in https://docs.google.com/document/d/1nhWN1rzMFXBOb6XVSvQeNwPanKTq_Iai_kLdKfFi...
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40245 )
Change subject: mainboard/volteer: Update Aux settings for Port 0 ......................................................................
Patch Set 5:
(2 comments)
https://review.coreboot.org/c/coreboot/+/40245/5/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/40245/5/src/mainboard/google/voltee... PS5, Line 128: 0x090E000A
Yeah this makes sense I'll work on a better implementation that is more easily readable
I will be pushing the non hard coded method as a separate patch to avoid issues on getting this merged so AUX flipping works on Volteer
https://review.coreboot.org/c/coreboot/+/40245/5/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/40245/5/src/mainboard/google/voltee... PS5, Line 242: /* E22 : DDPA_CTRLCLK ==> USB_C1_AUXP_DC: Retimer FW drives this pin */ : PAD_NC(GPP_E22, NONE), : /* E23 : DDPA_CTRLDATA ==> USB_C1_AUXN_DC: Retimer FW drives this pin */ : PAD_NC(GPP_E23, NONE),
If these are set to NF6 they will be SOC controlled instead of Retimer controller. […]
Ack
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40245 )
Change subject: mainboard/volteer: Update Aux settings for Port 0 ......................................................................
Patch Set 5:
(2 comments)
https://review.coreboot.org/c/coreboot/+/40245/1/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/40245/1/src/mainboard/google/voltee... PS1, Line 130: 1
so all 0 works for the ports not being used. […]
Done
https://review.coreboot.org/c/coreboot/+/40245/3/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/40245/3/src/mainboard/google/voltee... PS3, Line 219: NF6
I set it to Native Function 6 based on this document https://docs.google. […]
Done
Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40245 )
Change subject: mainboard/volteer: Update Aux settings for Port 0 ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40245/5/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/40245/5/src/mainboard/google/voltee... PS5, Line 242: /* E22 : DDPA_CTRLCLK ==> USB_C1_AUXP_DC: Retimer FW drives this pin */ : PAD_NC(GPP_E22, NONE), : /* E23 : DDPA_CTRLDATA ==> USB_C1_AUXN_DC: Retimer FW drives this pin */ : PAD_NC(GPP_E23, NONE),
Ack
hmmm... which bit is the "aux orientation override" bit mentioned in the EDS? does that refer to the adjacent "retimer disabled" bit?
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40245 )
Change subject: mainboard/volteer: Update Aux settings for Port 0 ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40245/5/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/40245/5/src/mainboard/google/voltee... PS5, Line 242: /* E22 : DDPA_CTRLCLK ==> USB_C1_AUXP_DC: Retimer FW drives this pin */ : PAD_NC(GPP_E22, NONE), : /* E23 : DDPA_CTRLDATA ==> USB_C1_AUXN_DC: Retimer FW drives this pin */ : PAD_NC(GPP_E23, NONE),
hmmm... […]
I assume you are referencing the values for the TcssAuxOri variable? the retimer disable and aux orientation override bit are the same this is just a mix up in the EDS referring to the same bit with different names
Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40245 )
Change subject: mainboard/volteer: Update Aux settings for Port 0 ......................................................................
Patch Set 5: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/40245/5/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/40245/5/src/mainboard/google/voltee... PS5, Line 242: /* E22 : DDPA_CTRLCLK ==> USB_C1_AUXP_DC: Retimer FW drives this pin */ : PAD_NC(GPP_E22, NONE), : /* E23 : DDPA_CTRLDATA ==> USB_C1_AUXN_DC: Retimer FW drives this pin */ : PAD_NC(GPP_E23, NONE),
I assume you are referencing the values for the TcssAuxOri variable? the retimer disable and aux ori […]
Ack
Wonkyu Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40245 )
Change subject: mainboard/volteer: Update Aux settings for Port 0 ......................................................................
Patch Set 5: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/40245 )
Change subject: mainboard/volteer: Update Aux settings for Port 0 ......................................................................
mainboard/volteer: Update Aux settings for Port 0
On Volteer port 0 (MB PORT) does not have a retimer so the port needs to be configured for the SOC to handle Aux orientation flipping. This requires 2 changes setting the TcssAuxOri UPD to 1 for port 0 (Bit 0) and configuring AUXP and AUXN GPIOs to Native Function 6 so SOC can control the orientation
BUG=b:145220205 BRANCH=NONE TEST=booted Volteer proto 2 and verified that the AUX channels flip when the cable is flipped
Change-Id: Ic81adc24d10322cc305bf0fa4c38514468ea0942 Signed-off-by: Brandon Breitenstein brandon.breitenstein@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/40245 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Caveh Jalali caveh@chromium.org Reviewed-by: Wonkyu Kim wonkyu.kim@intel.com --- M src/mainboard/google/volteer/variants/baseboard/devicetree.cb M src/mainboard/google/volteer/variants/baseboard/gpio.c 2 files changed, 12 insertions(+), 3 deletions(-)
Approvals: build bot (Jenkins): Verified Caveh Jalali: Looks good to me, but someone else must approve Wonkyu Kim: Looks good to me, approved
diff --git a/src/mainboard/google/volteer/variants/baseboard/devicetree.cb b/src/mainboard/google/volteer/variants/baseboard/devicetree.cb index 273b7a8..fa99de8 100644 --- a/src/mainboard/google/volteer/variants/baseboard/devicetree.cb +++ b/src/mainboard/google/volteer/variants/baseboard/devicetree.cb @@ -125,7 +125,16 @@
# TCSS USB3 register "TcssXhciEn" = "1" - register "TcssAuxOri" = "0" + register "TcssAuxOri" = "1" + register "IomTypeCPortPadCfg[0]" = "0x090E000A" + register "IomTypeCPortPadCfg[1]" = "0x090E000D" + register "IomTypeCPortPadCfg[2]" = "0x0" + register "IomTypeCPortPadCfg[3]" = "0x0" + register "IomTypeCPortPadCfg[4]" = "0x0" + register "IomTypeCPortPadCfg[5]" = "0x0" + register "IomTypeCPortPadCfg[6]" = "0x0" + register "IomTypeCPortPadCfg[7]" = "0x0" +
# DP port register "DdiPortAConfig" = "1" # eDP diff --git a/src/mainboard/google/volteer/variants/baseboard/gpio.c b/src/mainboard/google/volteer/variants/baseboard/gpio.c index 2ca59aa..8cbda1c 100644 --- a/src/mainboard/google/volteer/variants/baseboard/gpio.c +++ b/src/mainboard/google/volteer/variants/baseboard/gpio.c @@ -215,13 +215,13 @@ /* E9 : USB2_OC0# ==> USB_C1_OC_ODL */ PAD_CFG_NF(GPP_E9, NONE, DEEP, NF1), /* E10 : SPI1_CS# ==> USB_C0_AUXP_DC */ - PAD_CFG_GPO(GPP_E10, 0, DEEP), + PAD_CFG_NF(GPP_E10, NONE, DEEP, NF6), /* E11 : SPI1_CLK ==> SD_PE_WAKE_ODL */ PAD_CFG_GPI(GPP_E11, NONE, DEEP), /* E12 : SPI1_MISO_IO1 ==> NOT USED */ PAD_NC(GPP_E12, NONE), /* E13 : SPI1_MOSI_IO0 ==> USB_C0_AUXN_DC */ - PAD_CFG_GPO(GPP_E13, 0, DEEP), + PAD_CFG_NF(GPP_E13, NONE, DEEP, NF6), /* E14 : DDPC_HPDA ==> SOC_EDP_HPD */ PAD_CFG_NF(GPP_E14, NONE, DEEP, NF1), /* E15 : ISH_GP6 ==> TRACKPAD_INT_ODL */
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40245 )
Change subject: mainboard/volteer: Update Aux settings for Port 0 ......................................................................
Patch Set 6:
Automatic boot test returned (PASS/FAIL/TOTAL): 4/0/4 Emulation targets: "QEMU x86 q35/ich9" using payload TianoCore : SUCCESS : https://lava.9esec.io/r/3650 "QEMU x86 q35/ich9" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/3649 "QEMU x86 i440fx/piix4" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/3648 "QEMU AArch64" using payload LinuxBoot_u-root_kexec : SUCCESS : https://lava.9esec.io/r/3647
Please note: This test is under development and might not be accurate at all!
Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40245 )
Change subject: mainboard/volteer: Update Aux settings for Port 0 ......................................................................
Patch Set 6:
Hi Brandon, Where did you find definition of NF6 for GPP_E10 and GPP_E13? I don't see an NF6 defined for either of those GPIOs in the "607038-intel-500-chipset-pch-gpio-implementsummary-rev1p2.xlsx" document. Does GPP_E22 and GPP_E23 also have an NF6 that serves this same purpose?
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40245 )
Change subject: mainboard/volteer: Update Aux settings for Port 0 ......................................................................
Patch Set 6:
Patch Set 6:
Hi Brandon, Where did you find definition of NF6 for GPP_E10 and GPP_E13? I don't see an NF6 defined for either of those GPIOs in the "607038-intel-500-chipset-pch-gpio-implementsummary-rev1p2.xlsx" document. Does GPP_E22 and GPP_E23 also have an NF6 that serves this same purpose?
It is specified here that any GPIO that is used for AUX has to meet certain criteria and be set to NF6 https://docs.google.com/document/d/1nhWN1rzMFXBOb6XVSvQeNwPanKTq_Iai_kLdKfFi...
Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40245 )
Change subject: mainboard/volteer: Update Aux settings for Port 0 ......................................................................
Patch Set 6:
Patch Set 6:
Patch Set 6:
Hi Brandon, Where did you find definition of NF6 for GPP_E10 and GPP_E13? I don't see an NF6 defined for either of those GPIOs in the "607038-intel-500-chipset-pch-gpio-implementsummary-rev1p2.xlsx" document. Does GPP_E22 and GPP_E23 also have an NF6 that serves this same purpose?
It is specified here that any GPIO that is used for AUX has to meet certain criteria and be set to NF6 https://docs.google.com/document/d/1nhWN1rzMFXBOb6XVSvQeNwPanKTq_Iai_kLdKfFi...
I can't view that document. I requested permission yesterday, but it hasn't been granted yet.
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40245 )
Change subject: mainboard/volteer: Update Aux settings for Port 0 ......................................................................
Patch Set 6:
Patch Set 6:
Patch Set 6:
Patch Set 6:
Hi Brandon, Where did you find definition of NF6 for GPP_E10 and GPP_E13? I don't see an NF6 defined for either of those GPIOs in the "607038-intel-500-chipset-pch-gpio-implementsummary-rev1p2.xlsx" document. Does GPP_E22 and GPP_E23 also have an NF6 that serves this same purpose?
It is specified here that any GPIO that is used for AUX has to meet certain criteria and be set to NF6 https://docs.google.com/document/d/1nhWN1rzMFXBOb6XVSvQeNwPanKTq_Iai_kLdKfFi...
I can't view that document. I requested permission yesterday, but it hasn't been granted yet.
I pinged Aline to grant the permission. I assume you are inquiring here for Port 1 with Sku 1? I'm working on that implementation as well currently the same settings but with GPIOS E22 and E23 isn't showing the same behavior I'm going to do some more debug on that today and tomorrow
Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40245 )
Change subject: mainboard/volteer: Update Aux settings for Port 0 ......................................................................
Patch Set 6:
Patch Set 6:
Patch Set 6:
Patch Set 6:
Patch Set 6:
Hi Brandon, Where did you find definition of NF6 for GPP_E10 and GPP_E13? I don't see an NF6 defined for either of those GPIOs in the "607038-intel-500-chipset-pch-gpio-implementsummary-rev1p2.xlsx" document. Does GPP_E22 and GPP_E23 also have an NF6 that serves this same purpose?
It is specified here that any GPIO that is used for AUX has to meet certain criteria and be set to NF6 https://docs.google.com/document/d/1nhWN1rzMFXBOb6XVSvQeNwPanKTq_Iai_kLdKfFi...
I can't view that document. I requested permission yesterday, but it hasn't been granted yet.
I pinged Aline to grant the permission. I assume you are inquiring here for Port 1 with Sku 1? I'm working on that implementation as well currently the same settings but with GPIOS E22 and E23 isn't showing the same behavior I'm going to do some more debug on that today and tomorrow
Is there a bug for it? I came up with an implementation, but didn't realize someone else was working on it. No need for both of us to be implementing it, so I will step back and let you finish driving this one unless you'd prefer me to drive this. I'd still like access to that document, however.
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40245 )
Change subject: mainboard/volteer: Update Aux settings for Port 0 ......................................................................
Patch Set 6:
Patch Set 6:
Patch Set 6:
Patch Set 6:
Hi Brandon, Where did you find definition of NF6 for GPP_E10 and GPP_E13? I don't see an NF6 defined for either of those GPIOs in the "607038-intel-500-chipset-pch-gpio-implementsummary-rev1p2.xlsx" document. Does GPP_E22 and GPP_E23 also have an NF6 that serves this same purpose?
It is specified here that any GPIO that is used for AUX has to meet certain criteria and be set to NF6 https://docs.google.com/document/d/1nhWN1rzMFXBOb6XVSvQeNwPanKTq_Iai_kLdKfFi...
I can't view that document. I requested permission yesterday, but it hasn't been granted yet.
Seems Aline Copied this to a shared location as well...https://docs.google.com/file/d/1uG_pEWKArAoRyrcqa2oQPjyC1XZDwRrm/edit?ts=5e7... try this link
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40245 )
Change subject: mainboard/volteer: Update Aux settings for Port 0 ......................................................................
Patch Set 6:
Patch Set 6:
Patch Set 6:
Patch Set 6:
Patch Set 6:
Patch Set 6:
Hi Brandon, Where did you find definition of NF6 for GPP_E10 and GPP_E13? I don't see an NF6 defined for either of those GPIOs in the "607038-intel-500-chipset-pch-gpio-implementsummary-rev1p2.xlsx" document. Does GPP_E22 and GPP_E23 also have an NF6 that serves this same purpose?
It is specified here that any GPIO that is used for AUX has to meet certain criteria and be set to NF6 https://docs.google.com/document/d/1nhWN1rzMFXBOb6XVSvQeNwPanKTq_Iai_kLdKfFi...
I can't view that document. I requested permission yesterday, but it hasn't been granted yet.
I pinged Aline to grant the permission. I assume you are inquiring here for Port 1 with Sku 1? I'm working on that implementation as well currently the same settings but with GPIOS E22 and E23 isn't showing the same behavior I'm going to do some more debug on that today and tomorrow
Is there a bug for it? I came up with an implementation, but didn't realize someone else was working on it. No need for both of us to be implementing it, so I will step back and let you finish driving this one unless you'd prefer me to drive this. I'd still like access to that document, however.
No bug on this as we don't have many SKU 1 and are treating it as a bring up item for SKU 1. The main issue I see with this is we need to be able to dynamically set this or unset it based on SKU and I'm not sure I see the path forward there.
Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40245 )
Change subject: mainboard/volteer: Update Aux settings for Port 0 ......................................................................
Patch Set 6:
Patch Set 6:
Patch Set 6:
Patch Set 6:
Patch Set 6:
Patch Set 6:
Patch Set 6:
Hi Brandon, Where did you find definition of NF6 for GPP_E10 and GPP_E13? I don't see an NF6 defined for either of those GPIOs in the "607038-intel-500-chipset-pch-gpio-implementsummary-rev1p2.xlsx" document. Does GPP_E22 and GPP_E23 also have an NF6 that serves this same purpose?
It is specified here that any GPIO that is used for AUX has to meet certain criteria and be set to NF6 https://docs.google.com/document/d/1nhWN1rzMFXBOb6XVSvQeNwPanKTq_Iai_kLdKfFi...
I can't view that document. I requested permission yesterday, but it hasn't been granted yet.
I pinged Aline to grant the permission. I assume you are inquiring here for Port 1 with Sku 1? I'm working on that implementation as well currently the same settings but with GPIOS E22 and E23 isn't showing the same behavior I'm going to do some more debug on that today and tomorrow
Is there a bug for it? I came up with an implementation, but didn't realize someone else was working on it. No need for both of us to be implementing it, so I will step back and let you finish driving this one unless you'd prefer me to drive this. I'd still like access to that document, however.
No bug on this as we don't have many SKU 1 and are treating it as a bring up item for SKU 1. The main issue I see with this is we need to be able to dynamically set this or unset it based on SKU and I'm not sure I see the path forward there.
re: "The main issue I see with this is we need to be able to dynamically set this or unset it based on SKU and I'm not sure I see the path forward there."
I've got that path, coded it up last night. Let's chat offline.
-
9elements QA (Code Review) -
Brandon Breitenstein (Code Review)
-
Caveh Jalali (Code Review)
-
Duncan Laurie (Code Review)
-
Furquan Shaikh (Code Review)
-
Nick Vaccaro (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)
-
Wonkyu Kim (Code Review)