Change in ...coreboot[master]: mb/google/octopus/variants/fleex: Enable EMR function for Grob360S

[Chris Zhou (Code Review)]

Chris Zhou has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/33802

Change subject: mb/google/octopus/variants/fleex: Enable EMR function for Grob360S
......................................................................

mb/google/octopus/variants/fleex: Enable EMR function for Grob360S

Enable EMR function for Grob360S

BUG=b:135968368
BRANCH=octopus
TEST=Work normally with I2C6 which HW reworked.

Change-Id: Ia220dc0d3051b79b110b4df66df108f701776478
Signed-off-by: Chris Zhou 
---
M src/mainboard/google/octopus/variants/fleex/gpio.c
M src/mainboard/google/octopus/variants/fleex/overridetree.cb
2 files changed, 17 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/02/33802/1

diff --git a/src/mainboard/google/octopus/variants/fleex/gpio.c b/src/mainboard/google/octopus/variants/fleex/gpio.c
index 89adfb8..330cc02 100644
--- a/src/mainboard/google/octopus/variants/fleex/gpio.c
+++ b/src/mainboard/google/octopus/variants/fleex/gpio.c
@@ -30,7 +30,7 @@

PAD_NC(GPIO_138, UP_20K),
    PAD_NC(GPIO_139, UP_20K),
-	PAD_NC(GPIO_140, UP_20K),
+	PAD_CFG_GPO_IOSSTATE_IOSTERM(GPIO_140, 0, DEEP, NONE, Tx1RxDCRx0, DISPUPD), /* GPIO_140 -- PEN_RESET */
    PAD_NC(GPIO_143, UP_20K),
    PAD_NC(GPIO_144, UP_20K),
    PAD_NC(GPIO_145, UP_20K),
diff --git a/src/mainboard/google/octopus/variants/fleex/overridetree.cb b/src/mainboard/google/octopus/variants/fleex/overridetree.cb
index 19246e0..3050a7b 100644
--- a/src/mainboard/google/octopus/variants/fleex/overridetree.cb
+++ b/src/mainboard/google/octopus/variants/fleex/overridetree.cb
@@ -61,6 +61,9 @@
    		.speed_mhz = 1,
    		.early_init = 1,
    	},
+		.i2c[0] = {
+			.speed = I2C_SPEED_FAST,
+		},
    	.i2c[5] = {
    		.speed = I2C_SPEED_FAST,
    		.rise_time_ns = 104,
@@ -80,7 +83,19 @@
    }"

device domain 0 on
-		device pci 16.0 off  end	# - I2C 0
+		device pci 16.0 on
+			chip drivers/i2c/hid
+				register "generic.hid" = ""WCOM50C1""
+				register "generic.desc" = ""WCOM Digitizer""
+				register "generic.irq" = "ACPI_IRQ_LEVEL_LOW(GPIO_139_IRQ)"
+				register "generic.reset_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_HIGH(GPIO_140)"
+				register "generic.reset_delay_ms" = "20"
+				register "generic.has_power_resource" = "1"
+				register "generic.probed" = "1"
+				register "hid_desc_reg_offset" = "0x1"
+				device i2c 0x9 on end
+			end
+		end	# - I2C 0
    	device pci 16.1 off  end	# - I2C 1
    	device pci 17.1 on
    		chip drivers/i2c/da7219

[build bot (Jenkins) (Code Review)]

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33802 )

Change subject: mb/google/octopus/variants/fleex: Enable EMR function for Grob360S
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/#/c/33802/1/src/mainboard/google/octopus/variant... 
File src/mainboard/google/octopus/variants/fleex/gpio.c:

https://review.coreboot.org/#/c/33802/1/src/mainboard/google/octopus/variant... 
PS1, Line 33: 	PAD_CFG_GPO_IOSSTATE_IOSTERM(GPIO_140, 0, DEEP, NONE, Tx1RxDCRx0, DISPUPD), /* GPIO_140 -- PEN_RESET */
line over 96 characters

[Chris Zhou (Code Review)]

Hello Ivy Jian, EricR Lai, Furquan Shaikh,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/33802

to look at the new patch set (#2).

Change subject: mb/google/octopus/variants/fleex: Enable EMR function for Grob360S
......................................................................

mb/google/octopus/variants/fleex: Enable EMR function for Grob360S

Enable EMR function for Grob360S

BUG=b:135968368
BRANCH=octopus
TEST=Work normally with I2C6 which HW reworked.

Change-Id: Ia220dc0d3051b79b110b4df66df108f701776478
Signed-off-by: Chris Zhou 
---
M src/mainboard/google/octopus/variants/fleex/gpio.c
M src/mainboard/google/octopus/variants/fleex/overridetree.cb
2 files changed, 18 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/02/33802/2

[Chris Zhou (Code Review)]

Hello Ivy Jian, EricR Lai, build bot (Jenkins), Furquan Shaikh,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/33802

to look at the new patch set (#3).

Change subject: mb/google/octopus/variants/fleex: Enable EMR function for Grob360S
......................................................................

mb/google/octopus/variants/fleex: Enable EMR function for Grob360S

Enable EMR function for Grob360S

BUG=b:135968368
BRANCH=octopus
TEST=Work normally with I2C6 which HW reworked.

Change-Id: Ia220dc0d3051b79b110b4df66df108f701776478
Signed-off-by: Chris Zhou 
---
M src/mainboard/google/octopus/variants/fleex/gpio.c
M src/mainboard/google/octopus/variants/fleex/overridetree.cb
2 files changed, 19 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/02/33802/3

[Chris Zhou (Code Review)]

Chris Zhou has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33802 )

Change subject: mb/google/octopus/variants/fleex: Enable EMR function for Grob360S
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/#/c/33802/1/src/mainboard/google/octopus/variant... 
File src/mainboard/google/octopus/variants/fleex/gpio.c:

https://review.coreboot.org/#/c/33802/1/src/mainboard/google/octopus/variant... 
PS1, Line 33: 	PAD_CFG_GPO_IOSSTATE_IOSTERM(GPIO_140, 0, DEEP, NONE, Tx1RxDCRx0, DISPUPD), /* GPIO_140 -- PEN_RESET */
> line over 96 characters

Done

[Chris Zhou (Code Review)]

Hello Ivy Jian, EricR Lai, build bot (Jenkins), Furquan Shaikh,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/33802

to look at the new patch set (#4).

Change subject: mb/google/octopus/variants/fleex: Enable EMR function for Grob360S
......................................................................

mb/google/octopus/variants/fleex: Enable EMR function for Grob360S

Enable EMR function for Grob360S

BUG=b:135968368
BRANCH=octopus
TEST=EMR function working normally with HW reworked Fleex.

Change-Id: Ia220dc0d3051b79b110b4df66df108f701776478
Signed-off-by: Chris Zhou 
---
M src/mainboard/google/octopus/variants/fleex/gpio.c
M src/mainboard/google/octopus/variants/fleex/overridetree.cb
2 files changed, 19 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/02/33802/4

[Karthik Ramasubramanian (Code Review)]

Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33802 )

Change subject: mb/google/octopus/variants/fleex: Enable EMR function for Grob360S
......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/#/c/33802/4/src/mainboard/google/octopus/variant... 
File src/mainboard/google/octopus/variants/fleex/gpio.c:

https://review.coreboot.org/#/c/33802/4/src/mainboard/google/octopus/variant... 
PS4, Line 34: 0
Do you want to drive '0'? I see that other octopus variants driving '1' for the same digitizer. Also during the IO Standby do you want to Tx 1 or Tx 0?

[Chris Zhou (Code Review)]

Chris Zhou has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33802 )

Change subject: mb/google/octopus/variants/fleex: Enable EMR function for Grob360S
......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/#/c/33802/4/src/mainboard/google/octopus/variant... 
File src/mainboard/google/octopus/variants/fleex/gpio.c:

https://review.coreboot.org/#/c/33802/4/src/mainboard/google/octopus/variant... 
PS4, Line 34: 0
> Do you want to drive '0'? I see that other octopus variants driving '1' for the same digitizer. […]

According schematic and specification, normal keep logic “Low”, when active EMR warm reset change to logic “High”.
In this condition, we are the same setting with amption variant.(https://chromium.googlesource.com/chromiumos/third_party/coreboot/+/refs/hea...)
How can I know Tx need to set 1 or 0? I don't see any about Tx/Rx information in schematic and specification.

[EricR Lai (Code Review)]

EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33802 )

Change subject: mb/google/octopus/variants/fleex: Enable EMR function for Grob360S
......................................................................

Patch Set 4:
> Patch Set 4:
>
> (1 comment)

We put an inverter PMZ370UNE at PEN_RESET_ODL.

[EricR Lai (Code Review)]

EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33802 )

Change subject: mb/google/octopus/variants/fleex: Enable EMR function for Grob360S
......................................................................

Patch Set 4:
> Patch Set 4:
> > Patch Set 4:
> >
> > (1 comment)
>
> We put an inverter PMZ370UNE at PEN_RESET_ODL.

correct myself it is a N mos use as inverter.

[EricR Lai (Code Review)]

EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33802 )

Change subject: mb/google/octopus/variants/fleex: Enable EMR function for Grob360S
......................................................................

Patch Set 4: Code-Review+1

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33802 )

Change subject: mb/google/octopus/variants/fleex: Enable EMR function for Grob360S
......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/#/c/33802/4//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/#/c/33802/4//COMMIT_MSG@9 
PS4, Line 9: Enable EMR function for Grob360S
What is EMR?

[EricR Lai (Code Review)]

EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33802 )

Change subject: mb/google/octopus/variants/fleex: Enable EMR function for Grob360S
......................................................................

Patch Set 4:
> Patch Set 4:
>
> (1 comment)

stylus pen

[Karthik Ramasubramanian (Code Review)]

Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33802 )

Change subject: mb/google/octopus/variants/fleex: Enable EMR function for Grob360S
......................................................................

Patch Set 4: Code-Review+2

(2 comments)

https://review.coreboot.org/#/c/33802/4//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/#/c/33802/4//COMMIT_MSG@9 
PS4, Line 9: Enable EMR function for Grob360S
> What is EMR?

Can you please add EMR Pen Stylus for more clarity?

https://review.coreboot.org/#/c/33802/4/src/mainboard/google/octopus/variant... 
File src/mainboard/google/octopus/variants/fleex/gpio.c:

https://review.coreboot.org/#/c/33802/4/src/mainboard/google/octopus/variant... 
PS4, Line 34: 0
> According schematic and specification, normal keep logic “Low”, when active EMR warm reset change to […]

As per the Pen Stylus Datasheet, I can see that if RESET is LOW then the pen stylus stops operating. So both the normal and IoStandby configuration looks fine to me with an inverter logic.

[Chris Zhou (Code Review)]

Hello Ivy Jian, Karthik Ramasubramanian, EricR Lai, Justin TerAvest, build bot (Jenkins), Furquan Shaikh,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/33802

to look at the new patch set (#5).

Change subject: mb/google/octopus/variants/fleex: Enable EMR function for Grob360S
......................................................................

mb/google/octopus/variants/fleex: Enable EMR function for Grob360S

Enable EMR Pen Stylus function for Grob360S

BUG=b:135968368
BRANCH=octopus
TEST=EMR function working normally with HW reworked Fleex.

Change-Id: Ia220dc0d3051b79b110b4df66df108f701776478
Signed-off-by: Chris Zhou 
---
M src/mainboard/google/octopus/variants/fleex/gpio.c
M src/mainboard/google/octopus/variants/fleex/overridetree.cb
2 files changed, 19 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/02/33802/5

[Chris Zhou (Code Review)]

Chris Zhou has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33802 )

Change subject: mb/google/octopus/variants/fleex: Enable EMR function for Grob360S
......................................................................

Patch Set 5:

(1 comment)

https://review.coreboot.org/#/c/33802/4//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/#/c/33802/4//COMMIT_MSG@9 
PS4, Line 9: Enable EMR function for Grob360S
> Can you please add EMR Pen Stylus for more clarity?

Done

[Chris Zhou (Code Review)]

Chris Zhou has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33802 )

Change subject: mb/google/octopus/variants/fleex: Enable EMR function for Grob360S
......................................................................

Patch Set 5:

Hi goolge,

Could you help us merge this CL?

[Angel Pons (Code Review)]

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

Change subject: mb/google/octopus/variants/fleex: Enable EMR function for Grob360S
......................................................................

Patch Set 5:

(1 comment)

https://review.coreboot.org/#/c/33802/5//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/#/c/33802/5//COMMIT_MSG@7 
PS5, Line 7: Grob360S
What does this refer to?

[EricR Lai (Code Review)]

EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33802 )

Change subject: mb/google/octopus/variants/fleex: Enable EMR function for Grob360S
......................................................................

Patch Set 5:

(1 comment)

https://review.coreboot.org/#/c/33802/5//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/#/c/33802/5//COMMIT_MSG@7 
PS5, Line 7: Grob360S
> What does this refer to?

we file the hotlist name for this project.

[Frank Wu (Code Review)]

Frank Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33802 )

Change subject: mb/google/octopus/variants/fleex: Enable EMR function for Grob360S
......................................................................

Patch Set 5:

Google@,

Could you help merge the CL since it had code-revivew +2?
Thank you.

[Furquan Shaikh (Code Review)]

Furquan Shaikh has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/33802 )

Change subject: mb/google/octopus/variants/fleex: Enable EMR function for Grob360S
......................................................................

mb/google/octopus/variants/fleex: Enable EMR function for Grob360S

Enable EMR Pen Stylus function for Grob360S

BUG=b:135968368
BRANCH=octopus
TEST=EMR function working normally with HW reworked Fleex.

Change-Id: Ia220dc0d3051b79b110b4df66df108f701776478
Signed-off-by: Chris Zhou 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/33802
Reviewed-by: EricR Lai 
Reviewed-by: Karthik Ramasubramanian 
Tested-by: build bot (Jenkins) 
---
M src/mainboard/google/octopus/variants/fleex/gpio.c
M src/mainboard/google/octopus/variants/fleex/overridetree.cb
2 files changed, 19 insertions(+), 2 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Karthik Ramasubramanian: Looks good to me, approved
  EricR Lai: Looks good to me, but someone else must approve

diff --git a/src/mainboard/google/octopus/variants/fleex/gpio.c b/src/mainboard/google/octopus/variants/fleex/gpio.c
index 89adfb8..d2d0d64 100644
--- a/src/mainboard/google/octopus/variants/fleex/gpio.c
+++ b/src/mainboard/google/octopus/variants/fleex/gpio.c
@@ -30,7 +30,8 @@

PAD_NC(GPIO_138, UP_20K),
    PAD_NC(GPIO_139, UP_20K),
-	PAD_NC(GPIO_140, UP_20K),
+	/* GPIO_140 -- PEN_RESET */
+	PAD_CFG_GPO_IOSSTATE_IOSTERM(GPIO_140, 0, DEEP, NONE, Tx1RxDCRx0, DISPUPD),
    PAD_NC(GPIO_143, UP_20K),
    PAD_NC(GPIO_144, UP_20K),
    PAD_NC(GPIO_145, UP_20K),
diff --git a/src/mainboard/google/octopus/variants/fleex/overridetree.cb b/src/mainboard/google/octopus/variants/fleex/overridetree.cb
index 19246e0..da4e701 100644
--- a/src/mainboard/google/octopus/variants/fleex/overridetree.cb
+++ b/src/mainboard/google/octopus/variants/fleex/overridetree.cb
@@ -61,6 +61,9 @@
    		.speed_mhz = 1,
    		.early_init = 1,
    	},
+		.i2c[0] = {
+			.speed = I2C_SPEED_FAST,
+		},
    	.i2c[5] = {
    		.speed = I2C_SPEED_FAST,
    		.rise_time_ns = 104,
@@ -80,7 +83,20 @@
    }"

device domain 0 on
-		device pci 16.0 off  end	# - I2C 0
+		device pci 16.0 on
+			chip drivers/i2c/hid
+				register "generic.hid" = ""WCOM50C1""
+				register "generic.desc" = ""WCOM Digitizer""
+				register "generic.irq" = "ACPI_IRQ_LEVEL_LOW(GPIO_139_IRQ)"
+				register "generic.reset_gpio" =
+							"ACPI_GPIO_OUTPUT_ACTIVE_HIGH(GPIO_140)"
+				register "generic.reset_delay_ms" = "20"
+				register "generic.has_power_resource" = "1"
+				register "generic.probed" = "1"
+				register "hid_desc_reg_offset" = "0x1"
+				device i2c 0x9 on end
+			end
+		end	# - I2C 0
    	device pci 16.1 off  end	# - I2C 1
    	device pci 17.1 on
    		chip drivers/i2c/da7219

[Frank Wu (Code Review)]

Frank Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33802 )

Change subject: mb/google/octopus/variants/fleex: Enable EMR function for Grob360S
......................................................................

Patch Set 6:

Furquan@,

Thank you for merging the CL.
Could I cherry-pick this to octopus firmware branch directly before it landed on ToT?

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33802 )

Change subject: mb/google/octopus/variants/fleex: Enable EMR function for Grob360S
......................................................................

Patch Set 6:
> Patch Set 6:
>
> Furquan@,
>
> Thank you for merging the CL.
> Could I cherry-pick this to octopus firmware branch directly before it landed on ToT?

Frank -- It should be fine. Karthik should be able to help with that.

Change in coreboot[master]: mb/google/octopus/variants/fleex: Enable EMR function for Grob360S

[Karthik Ramasubramanian (Code Review)]

Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33802 )

Change subject: mb/google/octopus/variants/fleex: Enable EMR function for Grob360S
......................................................................

Patch Set 6:
> Patch Set 6:
> > Patch Set 6:
> >
> > Furquan@,
> >
> > Thank you for merging the CL.
> > Could I cherry-pick this to octopus firmware branch directly before it landed on ToT?
>
> Frank -- It should be fine. Karthik should be able to help with that.

Cherry-picked the change to chromium ToT. Please follow the CL.

Change in coreboot[master]: mb/google/octopus/variants/fleex: Enable EMR function for Grob360S

[Frank Wu (Code Review)]

Frank Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33802 )

Change subject: mb/google/octopus/variants/fleex: Enable EMR function for Grob360S
......................................................................

Patch Set 6:
> Patch Set 6:
> > Patch Set 6:
> > > Patch Set 6:
> > >
> > > Furquan@,
> > >
> > > Thank you for merging the CL.
> > > Could I cherry-pick this to octopus firmware branch directly before it landed on ToT?
> >
> > Frank -- It should be fine. Karthik should be able to help with that.
>
> Cherry-picked the change to chromium ToT. Please follow the CL.

Furquan@, Karthik@,

I am appreciate your kind assistance.
The CL is merged into octopus firmware branch.
Waiting for it to land on CPFE FW.
Thank you for very much.