Krishna P Bhat D has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31178
Change subject: mb/google/hatch: Reconfigure SD card detect GPIO host software ownership ......................................................................
mb/google/hatch: Reconfigure SD card detect GPIO host software ownership
This implementation reconfigures SD card detect GPIO(GPP_G5) host ownership to GPIO driver mode.
BUG=b:123432607 BRANCH=None TEST=Check SD card gets detected and data transfer working.
Change-Id: Id214184f3e183f6feb6887484cd4ccd3bab65b1a Signed-off-by: Krishna Prasad Bhat krishna.p.bhat.d@intel.com --- M src/mainboard/google/hatch/variants/baseboard/gpio.c 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/78/31178/1
diff --git a/src/mainboard/google/hatch/variants/baseboard/gpio.c b/src/mainboard/google/hatch/variants/baseboard/gpio.c index a078997..4b923829 100644 --- a/src/mainboard/google/hatch/variants/baseboard/gpio.c +++ b/src/mainboard/google/hatch/variants/baseboard/gpio.c @@ -138,7 +138,7 @@ /* SD_DATA3 */ PAD_CFG_NF(GPP_G4, NONE, DEEP, NF1), /* SD_CD# */ - PAD_CFG_NF(GPP_G5, NONE, DEEP, NF1), + PAD_CFG_GPI_GPIO_DRIVER(GPP_G5, NONE, DEEP), /* SD_CLK */ PAD_CFG_NF(GPP_G6, NONE, DEEP, NF1), /* SD_WP => NC */
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31178 )
Change subject: mb/google/hatch: Reconfigure SD card detect GPIO host software ownership ......................................................................
Patch Set 1: Code-Review+2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31178 )
Change subject: mb/google/hatch: Reconfigure SD card detect GPIO host software ownership ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/31178/1/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/baseboard/gpio.c:
https://review.coreboot.org/#/c/31178/1/src/mainboard/google/hatch/variants/... PS1, Line 141: PAD_CFG_GPI_GPIO_DRIVER Does it not result in any issues if CD# is not configured as native function for the SD controller?
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31178 )
Change subject: mb/google/hatch: Reconfigure SD card detect GPIO host software ownership ......................................................................
Patch Set 1: Code-Review-1
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31178 )
Change subject: mb/google/hatch: Reconfigure SD card detect GPIO host software ownership ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/31178/1/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/baseboard/gpio.c:
https://review.coreboot.org/#/c/31178/1/src/mainboard/google/hatch/variants/... PS1, Line 141: PAD_CFG_GPI_GPIO_DRIVER
Does it not result in any issues if CD# is not configured as native function for the SD controller?
I want to understand this better before the change gets pushed in. Why does this pin have to be configured in GPIO mode? Wouldn't the SD controller want it to be in native mode?
Rizwan Qureshi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31178 )
Change subject: mb/google/hatch: Reconfigure SD card detect GPIO host software ownership ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/31178/1/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/baseboard/gpio.c:
https://review.coreboot.org/#/c/31178/1/src/mainboard/google/hatch/variants/... PS1, Line 141: PAD_CFG_GPI_GPIO_DRIVER
I want to understand this better before the change gets pushed in. […]
this was done at the whim of the kernel driver. If this is set to native, the pinctrlr driver in kernel could not cofigure IRQ for this pin, "INT34BB:00: pin 56 cannot be used as IRQ", changing the host SW ownership to driver will fix this issue.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31178 )
Change subject: mb/google/hatch: Reconfigure SD card detect GPIO host software ownership ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/31178/1/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/baseboard/gpio.c:
https://review.coreboot.org/#/c/31178/1/src/mainboard/google/hatch/variants/... PS1, Line 141: PAD_CFG_GPI_GPIO_DRIVER
this was done at the whim of the kernel driver. […]
This brings up two questions to mind: 1. What is the impact of doing this on the SD controller? Wouldn't it want the pin to be in native mode?
2. Since the kernel driver wants to configure this in a certain way, would it make sense to dual-route this signal?
Rizwan Qureshi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31178 )
Change subject: mb/google/hatch: Reconfigure SD card detect GPIO host software ownership ......................................................................
Patch Set 1:
(1 comment)
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/31178/1/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/baseboard/gpio.c:
https://review.coreboot.org/#/c/31178/1/src/mainboard/google/hatch/variants/... PS1, Line 141: PAD_CFG_GPI_GPIO_DRIVER
- What is the impact of doing this on the SD controller? Wouldn't it want the pin to be in native mode?
We are checking this.
- Since the kernel driver wants to configure this in a certain way, would it make sense to dual-route this signal?
Or, does the kernel really need to program this pin for IRQ?
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31178 )
Change subject: mb/google/hatch: Reconfigure SD card detect GPIO host software ownership ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/31178/1/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/baseboard/gpio.c:
https://review.coreboot.org/#/c/31178/1/src/mainboard/google/hatch/variants/... PS1, Line 141: PAD_CFG_GPI_GPIO_DRIVER
- Since the kernel driver wants to configure this in a certain way, would it make sense to dual-route this signal?
Or, does the kernel really need to program this pin for IRQ?
I believe the kernel needs it to enable/disable wake from SD card insert. I will have to go back and check the kernel driver to see its use. Rajat might be able to provide more input on this.
Ronak Kanabar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31178 )
Change subject: mb/google/hatch: Reconfigure SD card detect GPIO host software ownership ......................................................................
Patch Set 1: Code-Review+1
Rajat Jain has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31178 )
Change subject: mb/google/hatch: Reconfigure SD card detect GPIO host software ownership ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/31178/1/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/baseboard/gpio.c:
https://review.coreboot.org/#/c/31178/1/src/mainboard/google/hatch/variants/... PS1, Line 141: PAD_CFG_GPI_GPIO_DRIVER
- […]
Lets move this discussion to the bug. I am putting my comments there.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31178 )
Change subject: mb/google/hatch: Reconfigure SD card detect GPIO host software ownership ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/31178/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/31178/1//COMMIT_MSG@9 PS1, Line 9: GPIO(GPP_G5) Please add a space before the (.
Hello Rajat Jain, Subrata Banik, Ronak Kanabar, Aamir Bohra, Rizwan Qureshi, Shelley Chen, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31178
to look at the new patch set (#2).
Change subject: mb/google/hatch: Reconfigure SD card detect GPIO host software ownership ......................................................................
mb/google/hatch: Reconfigure SD card detect GPIO host software ownership
This implementation reconfigures SD card detect GPIO (GPP_G5) host ownership to GPIO driver mode.
BUG=b:123432607 BRANCH=None TEST=Check SD card gets detected and data transfer working.
Change-Id: Id214184f3e183f6feb6887484cd4ccd3bab65b1a Signed-off-by: Krishna Prasad Bhat krishna.p.bhat.d@intel.com --- M src/mainboard/google/hatch/variants/baseboard/gpio.c 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/78/31178/2
Krishna P Bhat D has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31178 )
Change subject: mb/google/hatch: Reconfigure SD card detect GPIO host software ownership ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/#/c/31178/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/31178/1//COMMIT_MSG@9 PS1, Line 9: GPIO(GPP_G5)
Please add a space before the (.
Done
Ronak Kanabar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31178 )
Change subject: mb/google/hatch: Reconfigure SD card detect GPIO host software ownership ......................................................................
Patch Set 2: Code-Review+1
Krishna P Bhat D has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/31178 )
Change subject: mb/google/hatch: Reconfigure SD card detect GPIO host software ownership ......................................................................
Abandoned