Sathya Prakash M R has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/33932
Change subject: mb/google/helios: Add ALC1011 in Device tree ......................................................................
mb/google/helios: Add ALC1011 in Device tree
Following changes are done to enable ALC1011 codec on Helios 1. ACL1011 4 devices to I2C4 2. Invert GPIO H3 as required for SPKR_PA_EN 3. GPIO H13 is NC as per recommendation Verified SSDT table and i2cdetect from kernel
Signed-off-by: Naveen Manohar naveen.m@intel.com
Change-Id: I0d71e3bd2d4493d059a33023c1afe1b630181d4f Signed-off-by: Sathya Prakash M R sathya.prakash.m.r@intel.com --- M src/mainboard/google/hatch/variants/helios/gpio.c M src/mainboard/google/hatch/variants/helios/overridetree.cb 2 files changed, 38 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/32/33932/1
diff --git a/src/mainboard/google/hatch/variants/helios/gpio.c b/src/mainboard/google/hatch/variants/helios/gpio.c index 4353fc0..7a35053 100644 --- a/src/mainboard/google/hatch/variants/helios/gpio.c +++ b/src/mainboard/google/hatch/variants/helios/gpio.c @@ -87,8 +87,8 @@ PAD_NC(GPP_H4, NONE), /* H5 : I2C2_SCL ==> NC */ PAD_NC(GPP_H5, NONE), - /* H13 : M2_SKT2_CFG1 ==> SPKR_RST_L */ - PAD_CFG_GPO(GPP_H13, 0, PLTRST), + /* H13 : M2_SKT2_CFG1 ==> NC*/ + PAD_NC(GPP_H13, NONE), /* H14 : M2_SKT2_CFG2 ==> TOUCHSCREEN_STOP_L */ PAD_CFG_GPO(GPP_H14, 0, PLTRST), /* H19 : TIMESYNC[0] ==> MEM_STRAP_0 */ diff --git a/src/mainboard/google/hatch/variants/helios/overridetree.cb b/src/mainboard/google/hatch/variants/helios/overridetree.cb index 405f85f..2086b7e 100644 --- a/src/mainboard/google/hatch/variants/helios/overridetree.cb +++ b/src/mainboard/google/hatch/variants/helios/overridetree.cb @@ -124,6 +124,42 @@ register "property_list[0].integer" = "1" device i2c 1a on end end + chip drivers/i2c/generic + register "hid" = ""10EC1011"" + register "desc" = ""RT1011 Tweeter Left Speaker Amp"" + register "uid" = "0" + register "name" = ""TL"" + register "device_present_gpio" = "GPP_H3" + register "device_present_gpio_invert" = "1" + device i2c 38 on end + end + chip drivers/i2c/generic + register "hid" = ""10EC1011"" + register "desc" = ""RT1011 Tweeter Right Speaker Amp"" + register "uid" = "1" + register "name" = ""TR"" + register "device_present_gpio" = "GPP_H3" + register "device_present_gpio_invert" = "1" + device i2c 39 on end + end + chip drivers/i2c/generic + register "hid" = ""10EC1011"" + register "desc" = ""RT1011 Woofer Left Speaker Amp"" + register "uid" = "2" + register "name" = ""WL"" + register "device_present_gpio" = "GPP_H3" + register "device_present_gpio_invert" = "1" + device i2c 3a on end + end + chip drivers/i2c/generic + register "hid" = ""10EC1011"" + register "desc" = ""RT1011 Woofer Right Speaker Amp"" + register "uid" = "3" + register "name" = ""WR"" + register "device_present_gpio" = "GPP_H3" + register "device_present_gpio_invert" = "1" + device i2c 3b on end + end end #I2C #4 end end
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33932 )
Change subject: mb/google/helios: Add ALC1011 in Device tree ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/33932/1/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/helios/gpio.c:
https://review.coreboot.org/#/c/33932/1/src/mainboard/google/hatch/variants/... PS1, Line 91: PAD_NC(GPP_H13, NONE), from schematics I see this is interfaced from SOC to Spkr amp. Can you try adding the register "reset_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_LOW(GPP_H13)" config from devicetree
Sathya Prakash M R has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33932 )
Change subject: mb/google/helios: Add ALC1011 in Device tree ......................................................................
Patch Set 1:
Patch Set 1:
(1 comment)
I tried the suggestion and it fails to detect the codec.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33932 )
Change subject: mb/google/helios: Add ALC1011 in Device tree ......................................................................
Patch Set 1:
(3 comments)
https://review.coreboot.org/#/c/33932/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/33932/1//COMMIT_MSG@9 PS1, Line 9: Following changes are done to enable ALC1011 codec on Helios Add a blank line below?
https://review.coreboot.org/#/c/33932/1//COMMIT_MSG@13 PS1, Line 13: Verified SSDT table and i2cdetect from kernel Add blank line above.
https://review.coreboot.org/#/c/33932/1//COMMIT_MSG@16 PS1, Line 16: Please remove this blank line.
Sathya Prakash M R has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33932 )
Change subject: mb/google/helios: Add ALC1011 in Device tree ......................................................................
Patch Set 1:
(3 comments)
will address them in v2 ( mostly there might be )
https://review.coreboot.org/#/c/33932/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/33932/1//COMMIT_MSG@9 PS1, Line 9: Following changes are done to enable ALC1011 codec on Helios
Add a blank line below?
Ack
https://review.coreboot.org/#/c/33932/1//COMMIT_MSG@13 PS1, Line 13: Verified SSDT table and i2cdetect from kernel
Add blank line above.
Ack
https://review.coreboot.org/#/c/33932/1//COMMIT_MSG@16 PS1, Line 16:
Please remove this blank line.
Ack
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33932 )
Change subject: mb/google/helios: Add ALC1011 in Device tree ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33932/1/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/helios/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/33932/1/src/mainboard/google/hatch/... PS1, Line 132: register "device_present_gpio" = "GPP_H3" Why is this being used?
Sathya Prakash M R has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33932 )
Change subject: mb/google/helios: Add ALC1011 in Device tree ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33932/1/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/helios/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/33932/1/src/mainboard/google/hatch/... PS1, Line 132: register "device_present_gpio" = "GPP_H3"
Why is this being used?
This is the SPKR_PA_EN signal to the codec.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33932 )
Change subject: mb/google/helios: Add ALC1011 in Device tree ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33932/1/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/helios/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/33932/1/src/mainboard/google/hatch/... PS1, Line 132: register "device_present_gpio" = "GPP_H3"
This is the SPKR_PA_EN signal to the codec.
device_present_gpio is used for detecting if a device is present based on the value of the gpio. I don't think that is what you intend to use it for.
Sathya Prakash M R has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33932 )
Change subject: mb/google/helios: Add ALC1011 in Device tree ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33932/1/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/helios/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/33932/1/src/mainboard/google/hatch/... PS1, Line 132: register "device_present_gpio" = "GPP_H3"
device_present_gpio is used for detecting if a device is present based on the value of the gpio. […]
Thanks, let me look into this further and get back.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33932 )
Change subject: mb/google/helios: Add ALC1011 in Device tree ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/33932/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/33932/1//COMMIT_MSG@7 PS1, Line 7: Device tree devicetree
https://review.coreboot.org/c/coreboot/+/33932/1//COMMIT_MSG@7 PS1, Line 7: in to?
Sathya Prakash M R has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33932 )
Change subject: mb/google/helios: Add ALC1011 in Device tree ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/33932/1/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/helios/gpio.c:
https://review.coreboot.org/c/coreboot/+/33932/1/src/mainboard/google/hatch/... PS1, Line 91: PAD_NC(GPP_H13, NONE),
from schematics I see this is interfaced from SOC to Spkr amp. […]
This did not help. only NC worked for me.
https://review.coreboot.org/c/coreboot/+/33932/1/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/helios/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/33932/1/src/mainboard/google/hatch/... PS1, Line 132: register "device_present_gpio" = "GPP_H3"
Thanks, let me look into this further and get back.
Thanks for pointer Furquan. I removed these 2 lines on the GPIO and speaker works as expected. Hence will push the patchset#2 removing these changes
Hello Aamir Bohra, build bot (Jenkins), Naveen M, Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33932
to look at the new patch set (#2).
Change subject: mb/google/helios: Add ALC1011 in device tree to enable speaker amps ......................................................................
mb/google/helios: Add ALC1011 in device tree to enable speaker amps
Following changes are done to enable ALC1011 codec on Helios
1. ACL1011 4 devices to I2C4 2. GPIO H13 is NC as per recommendation
Verified SSDT table and i2cdetect from kernel.
Signed-off-by: Naveen Manohar naveen.m@intel.com Change-Id: I0d71e3bd2d4493d059a33023c1afe1b630181d4f Signed-off-by: Sathya Prakash M R sathya.prakash.m.r@intel.com --- M src/mainboard/google/hatch/variants/helios/gpio.c M src/mainboard/google/hatch/variants/helios/overridetree.cb 2 files changed, 30 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/32/33932/2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33932 )
Change subject: mb/google/helios: Add ALC1011 in device tree to enable speaker amps ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/33932/2/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/helios/gpio.c:
https://review.coreboot.org/c/coreboot/+/33932/2/src/mainboard/google/hatch/... PS2, Line 90: So this is no longer required?
https://review.coreboot.org/c/coreboot/+/33932/2/src/mainboard/google/hatch/... PS2, Line 91: PAD_NC Baseboard gpio.c configures GPP_H13 as PAD_NC. So, you just need to remove this entry from here.
Sathya Prakash M R has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33932 )
Change subject: mb/google/helios: Add ALC1011 in device tree to enable speaker amps ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/33932/2/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/helios/gpio.c:
https://review.coreboot.org/c/coreboot/+/33932/2/src/mainboard/google/hatch/... PS2, Line 90:
So this is no longer required?
this is not needed for Helios based on my experiments.
https://review.coreboot.org/c/coreboot/+/33932/2/src/mainboard/google/hatch/... PS2, Line 91: PAD_NC
Baseboard gpio.c configures GPP_H13 as PAD_NC. So, you just need to remove this entry from here.
yes. thats right. will remove
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33932 )
Change subject: mb/google/helios: Add ALC1011 in device tree to enable speaker amps ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33932/2/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/helios/gpio.c:
https://review.coreboot.org/c/coreboot/+/33932/2/src/mainboard/google/hatch/... PS2, Line 91: PAD_NC
yes. thats right. […]
Wait. I was just looking at the schematics and it looks like this line is connected to RSTB on the amp. Shouldn't you be doing something like this here:
PAD_CFG_GPO(GPP_H13, 1, DEEP),
Sathya Prakash M R has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33932 )
Change subject: mb/google/helios: Add ALC1011 in device tree to enable speaker amps ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33932/2/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/helios/gpio.c:
https://review.coreboot.org/c/coreboot/+/33932/2/src/mainboard/google/hatch/... PS2, Line 91: PAD_NC
Wait. […]
I will try this. But 1 change Aamir suggested on Patch set#1 and that didnt work. let me try this and get back.
Sathya Prakash M R has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33932 )
Change subject: mb/google/helios: Add ALC1011 in device tree to enable speaker amps ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33932/2/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/helios/gpio.c:
https://review.coreboot.org/c/coreboot/+/33932/2/src/mainboard/google/hatch/... PS2, Line 91: PAD_NC
I will try this. But 1 change Aamir suggested on Patch set#1 and that didnt work. […]
ok. your suggestions works. will update patch set #3.
Hello Aamir Bohra, build bot (Jenkins), Naveen M, Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33932
to look at the new patch set (#3).
Change subject: mb/google/helios: Add ALC1011 in device tree to enable speaker amps ......................................................................
mb/google/helios: Add ALC1011 in device tree to enable speaker amps
Following changes are done to enable ALC1011 codec on Helios
1. ACL1011 4 devices to I2C4 2. GPIO H13 is set to GPO as per schematics
Verified SSDT table and i2cdetect from kernel.
Signed-off-by: Naveen Manohar naveen.m@intel.com Change-Id: I0d71e3bd2d4493d059a33023c1afe1b630181d4f Signed-off-by: Sathya Prakash M R sathya.prakash.m.r@intel.com --- M src/mainboard/google/hatch/variants/helios/gpio.c M src/mainboard/google/hatch/variants/helios/overridetree.cb 2 files changed, 29 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/32/33932/3
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33932 )
Change subject: mb/google/helios: Add ALC1011 in device tree to enable speaker amps ......................................................................
Patch Set 3: Code-Review+1
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33932 )
Change subject: mb/google/helios: Add ALC1011 in device tree to enable speaker amps ......................................................................
Patch Set 3: Code-Review+2
Sathya Prakash M R has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33932 )
Change subject: mb/google/helios: Add ALC1011 in device tree to enable speaker amps ......................................................................
Patch Set 3:
Can we merge the patch ?
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33932 )
Change subject: mb/google/helios: Add ALC1011 in device tree to enable speaker amps ......................................................................
Patch Set 3:
Sathya -- you need to mark all the comments as resolved. Only then it is possible to now merge the commit: https://mail.coreboot.org/hyperkitty/list/coreboot@coreboot.org/thread/FESJX...
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33932 )
Change subject: mb/google/helios: Add ALC1011 in device tree to enable speaker amps ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/33932/2/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/helios/gpio.c:
https://review.coreboot.org/c/coreboot/+/33932/2/src/mainboard/google/hatch/... PS2, Line 90:
this is not needed for Helios based on my experiments.
Done
https://review.coreboot.org/c/coreboot/+/33932/2/src/mainboard/google/hatch/... PS2, Line 91: PAD_NC
ok. your suggestions works. will update patch set #3.
Done
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33932 )
Change subject: mb/google/helios: Add ALC1011 in device tree to enable speaker amps ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33932/1/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/helios/gpio.c:
https://review.coreboot.org/c/coreboot/+/33932/1/src/mainboard/google/hatch/... PS1, Line 91: PAD_NC(GPP_H13, NONE),
This did not help. only NC worked for me.
Done
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33932 )
Change subject: mb/google/helios: Add ALC1011 in device tree to enable speaker amps ......................................................................
Patch Set 3:
(5 comments)
https://review.coreboot.org/c/coreboot/+/33932/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/33932/1//COMMIT_MSG@7 PS1, Line 7: in
to?
Done
https://review.coreboot.org/c/coreboot/+/33932/1//COMMIT_MSG@7 PS1, Line 7: Device tree
devicetree
Done
https://review.coreboot.org/c/coreboot/+/33932/1//COMMIT_MSG@9 PS1, Line 9: Following changes are done to enable ALC1011 codec on Helios
Ack
Done
https://review.coreboot.org/c/coreboot/+/33932/1//COMMIT_MSG@13 PS1, Line 13: Verified SSDT table and i2cdetect from kernel
Ack
Done
https://review.coreboot.org/c/coreboot/+/33932/1//COMMIT_MSG@16 PS1, Line 16:
Ack
Done
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33932 )
Change subject: mb/google/helios: Add ALC1011 in device tree to enable speaker amps ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33932/1/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/helios/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/33932/1/src/mainboard/google/hatch/... PS1, Line 132: register "device_present_gpio" = "GPP_H3"
Thanks for pointer Furquan. I removed these 2 lines on the GPIO and speaker works as expected. […]
Done
Furquan Shaikh has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/33932 )
Change subject: mb/google/helios: Add ALC1011 in device tree to enable speaker amps ......................................................................
mb/google/helios: Add ALC1011 in device tree to enable speaker amps
Following changes are done to enable ALC1011 codec on Helios
1. ACL1011 4 devices to I2C4 2. GPIO H13 is set to GPO as per schematics
Verified SSDT table and i2cdetect from kernel.
Signed-off-by: Naveen Manohar naveen.m@intel.com Change-Id: I0d71e3bd2d4493d059a33023c1afe1b630181d4f Signed-off-by: Sathya Prakash M R sathya.prakash.m.r@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/33932 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Paul Menzel paulepanter@users.sourceforge.net Reviewed-by: Furquan Shaikh furquan@google.com --- M src/mainboard/google/hatch/variants/helios/gpio.c M src/mainboard/google/hatch/variants/helios/overridetree.cb 2 files changed, 29 insertions(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified Paul Menzel: Looks good to me, but someone else must approve Furquan Shaikh: Looks good to me, approved
diff --git a/src/mainboard/google/hatch/variants/helios/gpio.c b/src/mainboard/google/hatch/variants/helios/gpio.c index 4353fc0..12801de 100644 --- a/src/mainboard/google/hatch/variants/helios/gpio.c +++ b/src/mainboard/google/hatch/variants/helios/gpio.c @@ -88,7 +88,7 @@ /* H5 : I2C2_SCL ==> NC */ PAD_NC(GPP_H5, NONE), /* H13 : M2_SKT2_CFG1 ==> SPKR_RST_L */ - PAD_CFG_GPO(GPP_H13, 0, PLTRST), + PAD_CFG_GPO(GPP_H13, 1, DEEP), /* H14 : M2_SKT2_CFG2 ==> TOUCHSCREEN_STOP_L */ PAD_CFG_GPO(GPP_H14, 0, PLTRST), /* H19 : TIMESYNC[0] ==> MEM_STRAP_0 */ diff --git a/src/mainboard/google/hatch/variants/helios/overridetree.cb b/src/mainboard/google/hatch/variants/helios/overridetree.cb index 1907289..35f8ad5 100644 --- a/src/mainboard/google/hatch/variants/helios/overridetree.cb +++ b/src/mainboard/google/hatch/variants/helios/overridetree.cb @@ -124,6 +124,34 @@ register "property_list[0].integer" = "1" device i2c 1a on end end + chip drivers/i2c/generic + register "hid" = ""10EC1011"" + register "desc" = ""RT1011 Tweeter Left Speaker Amp"" + register "uid" = "0" + register "name" = ""TL"" + device i2c 38 on end + end + chip drivers/i2c/generic + register "hid" = ""10EC1011"" + register "desc" = ""RT1011 Tweeter Right Speaker Amp"" + register "uid" = "1" + register "name" = ""TR"" + device i2c 39 on end + end + chip drivers/i2c/generic + register "hid" = ""10EC1011"" + register "desc" = ""RT1011 Woofer Left Speaker Amp"" + register "uid" = "2" + register "name" = ""WL"" + device i2c 3a on end + end + chip drivers/i2c/generic + register "hid" = ""10EC1011"" + register "desc" = ""RT1011 Woofer Right Speaker Amp"" + register "uid" = "3" + register "name" = ""WR"" + device i2c 3b on end + end end #I2C #4 end end
-
Aamir Bohra (Code Review) -
Furquan Shaikh (Code Review)
-
Paul Menzel (Code Review)
-
Sathya Prakash M R (Code Review)