Change in coreboot[master]: lenovo/t440p: fix keyboard backlight

[Iru Cai (vimacs) (Code Review)]

Hello Iru Cai,

I'd like you to do a code review. Please visit

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

to review the following change.


Change subject: lenovo/t440p: fix keyboard backlight
......................................................................

lenovo/t440p: fix keyboard backlight

Change-Id: Ib4c2b1591d26e2bb33f9549e3933efe9a6e0b043
Signed-off-by: Iru Cai <[e-mail]>
---
M src/mainboard/lenovo/t440p/cmos.layout
M src/mainboard/lenovo/t440p/devicetree.cb
2 files changed, 5 insertions(+), 3 deletions(-)



  git pull ssh://review.coreboot.org:29418/coreboot refs/changes/50/38650/1

diff --git a/src/mainboard/lenovo/t440p/cmos.layout b/src/mainboard/lenovo/t440p/cmos.layout
index 8c5fb0b..e61886b 100644
--- a/src/mainboard/lenovo/t440p/cmos.layout
+++ b/src/mainboard/lenovo/t440p/cmos.layout
@@ -103,9 +103,10 @@
 8     1     Primary
 9     0     AHCI
 9     1     Compatible
-10    0     Both
-10    1     Keyboard only
-10    2     Thinklight only
+# Haswell ThinkPads has no Thinklight
+#10    0     Both
+10    1     Keyboard
+#10    2     Thinklight only
 10    3     None
 #12    0     Integrated Only
 #12    1     Discrete Only
diff --git a/src/mainboard/lenovo/t440p/devicetree.cb b/src/mainboard/lenovo/t440p/devicetree.cb
index 7e5e616..b63767e 100644
--- a/src/mainboard/lenovo/t440p/devicetree.cb
+++ b/src/mainboard/lenovo/t440p/devicetree.cb
@@ -83,6 +83,7 @@
 					register "config1" = "0x0d"
 					register "config2" = "0xa8"
 					register "config3" = "0xc4"
+					register "has_keyboard_backlight" = "1"
 					register "event2_enable" = "0xff"
 					register "event3_enable" = "0xff"
 					register "event4_enable" = "0xd0"

-- 
To view, visit https://review.coreboot.org/c/coreboot/+/38650
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib4c2b1591d26e2bb33f9549e3933efe9a6e0b043
Gerrit-Change-Number: 38650
Gerrit-PatchSet: 1
Gerrit-Owner: Iru Cai (vimacs) <[e-mail]>
Gerrit-Reviewer: Iru Cai <[e-mail]>
Gerrit-MessageType: newchange

[Paul Menzel (Code Review)]

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

Change subject: lenovo/t440p: fix keyboard backlight
......................................................................


Patch Set 1:

(1 comment)

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

https://review.coreboot.org/c/coreboot/+/38650/1//COMMIT_MSG@8 
PS1, Line 8: 
Please elaborate in the commit message body. What is broken, and how do you fix it.



-- 
To view, visit https://review.coreboot.org/c/coreboot/+/38650
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib4c2b1591d26e2bb33f9549e3933efe9a6e0b043
Gerrit-Change-Number: 38650
Gerrit-PatchSet: 1
Gerrit-Owner: Iru Cai (vimacs) <[e-mail]>
Gerrit-Reviewer: Alexander Couzens <[e-mail]>
Gerrit-Reviewer: Iru Cai (vimacs) <[e-mail]>
Gerrit-Reviewer: Iru Cai <[e-mail]>
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-Comment-Date: Thu, 30 Jan 2020 11:32:48 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

[Angel Pons (Code Review)]

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

Change subject: lenovo/t440p: fix keyboard backlight
......................................................................


Patch Set 1: Code-Review+1

(1 comment)

https://review.coreboot.org/c/coreboot/+/38650/1/src/mainboard/lenovo/t440p/... 
File src/mainboard/lenovo/t440p/cmos.layout:

https://review.coreboot.org/c/coreboot/+/38650/1/src/mainboard/lenovo/t440p/... 
PS1, Line 106: has
have



-- 
To view, visit https://review.coreboot.org/c/coreboot/+/38650
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib4c2b1591d26e2bb33f9549e3933efe9a6e0b043
Gerrit-Change-Number: 38650
Gerrit-PatchSet: 1
Gerrit-Owner: Iru Cai (vimacs) <[e-mail]>
Gerrit-Reviewer: Alexander Couzens <[e-mail]>
Gerrit-Reviewer: Angel Pons <[e-mail]>
Gerrit-Reviewer: Iru Cai (vimacs) <[e-mail]>
Gerrit-Reviewer: Iru Cai <[e-mail]>
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-Comment-Date: Thu, 30 Jan 2020 11:36:57 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

[Dennis Witzig (Code Review)]

Dennis Witzig has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38650 )

Change subject: lenovo/t440p: fix keyboard backlight
......................................................................


Patch Set 1: Code-Review+1

(1 comment)

When changing it to "Keyboard" manually, it builds successful and the backlight works again on my T440p. Deactivating it with nvramtool also works.

https://review.coreboot.org/c/coreboot/+/38650/1/src/mainboard/lenovo/t440p/... 
File src/mainboard/lenovo/t440p/cmos.layout:

https://review.coreboot.org/c/coreboot/+/38650/1/src/mainboard/lenovo/t440p/... 
PS1, Line 107: Both
cmos.default is still set to "Both" which gives an error while building because it now doesn't exist anymore



-- 
To view, visit https://review.coreboot.org/c/coreboot/+/38650
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib4c2b1591d26e2bb33f9549e3933efe9a6e0b043
Gerrit-Change-Number: 38650
Gerrit-PatchSet: 1
Gerrit-Owner: Iru Cai (vimacs) <[e-mail]>
Gerrit-Reviewer: Alexander Couzens <[e-mail]>
Gerrit-Reviewer: Angel Pons <[e-mail]>
Gerrit-Reviewer: Dennis Witzig <[e-mail]>
Gerrit-Reviewer: Iru Cai (vimacs) <[e-mail]>
Gerrit-Reviewer: Iru Cai <[e-mail]>
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-Comment-Date: Thu, 30 Jan 2020 12:06:14 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

[Iru Cai (vimacs) (Code Review)]

Hello Alexander Couzens, Dennis Witzig, Patrick Rudolph, Iru Cai, Angel Pons, build bot (Jenkins), 

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

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

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

Change subject: lenovo/t440p: fix keyboard backlight
......................................................................

lenovo/t440p: fix keyboard backlight

It is found that keyboard backlight in T440p is enabled by clearing
bit 3 of EC RAM 0x01. This patch sets has_keyboard_backlight in
devicetree.cb and also corrects the CMOS configuration.

Change-Id: Ib4c2b1591d26e2bb33f9549e3933efe9a6e0b043
Signed-off-by: Iru Cai <[e-mail]>
---
M src/mainboard/lenovo/t440p/cmos.default
M src/mainboard/lenovo/t440p/cmos.layout
M src/mainboard/lenovo/t440p/devicetree.cb
3 files changed, 6 insertions(+), 4 deletions(-)


  git pull ssh://review.coreboot.org:29418/coreboot refs/changes/50/38650/2
-- 
To view, visit https://review.coreboot.org/c/coreboot/+/38650
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib4c2b1591d26e2bb33f9549e3933efe9a6e0b043
Gerrit-Change-Number: 38650
Gerrit-PatchSet: 2
Gerrit-Owner: Iru Cai (vimacs) <[e-mail]>
Gerrit-Reviewer: Alexander Couzens <[e-mail]>
Gerrit-Reviewer: Angel Pons <[e-mail]>
Gerrit-Reviewer: Dennis Witzig <[e-mail]>
Gerrit-Reviewer: Iru Cai (vimacs) <[e-mail]>
Gerrit-Reviewer: Iru Cai <[e-mail]>
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-MessageType: newpatchset

[Iru Cai (vimacs) (Code Review)]

Iru Cai (vimacs) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38650 )

Change subject: lenovo/t440p: fix keyboard backlight
......................................................................


Patch Set 2:

(3 comments)

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

https://review.coreboot.org/c/coreboot/+/38650/1//COMMIT_MSG@8 
PS1, Line 8:
> Please elaborate in the commit message body. What is broken, and how do you fix it.
> Done

https://review.coreboot.org/c/coreboot/+/38650/1/src/mainboard/lenovo/t440p/... 
File src/mainboard/lenovo/t440p/cmos.layout:

https://review.coreboot.org/c/coreboot/+/38650/1/src/mainboard/lenovo/t440p/... 
PS1, Line 106: has
> have
> Done

https://review.coreboot.org/c/coreboot/+/38650/1/src/mainboard/lenovo/t440p/... 
PS1, Line 107: Both
> cmos. […]
> Done

[Angel Pons (Code Review)]

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

Change subject: lenovo/t440p: fix keyboard backlight
......................................................................


Patch Set 2: Code-Review+2


-- 
To view, visit https://review.coreboot.org/c/coreboot/+/38650
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib4c2b1591d26e2bb33f9549e3933efe9a6e0b043
Gerrit-Change-Number: 38650
Gerrit-PatchSet: 2
Gerrit-Owner: Iru Cai (vimacs) <[e-mail]>
Gerrit-Reviewer: Alexander Couzens <[e-mail]>
Gerrit-Reviewer: Angel Pons <[e-mail]>
Gerrit-Reviewer: Dennis Witzig <[e-mail]>
Gerrit-Reviewer: Iru Cai (vimacs) <[e-mail]>
Gerrit-Reviewer: Iru Cai <[e-mail]>
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-Comment-Date: Thu, 30 Jan 2020 21:56:32 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

[Dennis Witzig (Code Review)]

Dennis Witzig has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38650 )

Change subject: lenovo/t440p: fix keyboard backlight
......................................................................


Patch Set 2: Code-Review+1


-- 
To view, visit https://review.coreboot.org/c/coreboot/+/38650
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib4c2b1591d26e2bb33f9549e3933efe9a6e0b043
Gerrit-Change-Number: 38650
Gerrit-PatchSet: 2
Gerrit-Owner: Iru Cai (vimacs) <[e-mail]>
Gerrit-Reviewer: Alexander Couzens <[e-mail]>
Gerrit-Reviewer: Angel Pons <[e-mail]>
Gerrit-Reviewer: Dennis Witzig <[e-mail]>
Gerrit-Reviewer: Iru Cai (vimacs) <[e-mail]>
Gerrit-Reviewer: Iru Cai <[e-mail]>
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-Comment-Date: Thu, 30 Jan 2020 22:24:01 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

[Alexander Couzens (Code Review)]

Alexander Couzens has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38650 )

Change subject: lenovo/t440p: fix keyboard backlight
......................................................................


Patch Set 2: Code-Review+2


-- 
To view, visit https://review.coreboot.org/c/coreboot/+/38650
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib4c2b1591d26e2bb33f9549e3933efe9a6e0b043
Gerrit-Change-Number: 38650
Gerrit-PatchSet: 2
Gerrit-Owner: Iru Cai (vimacs) <[e-mail]>
Gerrit-Reviewer: Alexander Couzens <[e-mail]>
Gerrit-Reviewer: Angel Pons <[e-mail]>
Gerrit-Reviewer: Dennis Witzig <[e-mail]>
Gerrit-Reviewer: Iru Cai (vimacs) <[e-mail]>
Gerrit-Reviewer: Iru Cai <[e-mail]>
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-Comment-Date: Fri, 31 Jan 2020 13:10:43 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

[Alexander Couzens (Code Review)]

Alexander Couzens has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38650 )

Change subject: lenovo/t440p: fix keyboard backlight
......................................................................


Patch Set 2: Code-Review-1

cmos default still says it's defaulting to "Both".


-- 
To view, visit https://review.coreboot.org/c/coreboot/+/38650
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib4c2b1591d26e2bb33f9549e3933efe9a6e0b043
Gerrit-Change-Number: 38650
Gerrit-PatchSet: 2
Gerrit-Owner: Iru Cai (vimacs) <[e-mail]>
Gerrit-Reviewer: Alexander Couzens <[e-mail]>
Gerrit-Reviewer: Angel Pons <[e-mail]>
Gerrit-Reviewer: Dennis Witzig <[e-mail]>
Gerrit-Reviewer: Iru Cai (vimacs) <[e-mail]>
Gerrit-Reviewer: Iru Cai <[e-mail]>
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-Comment-Date: Fri, 31 Jan 2020 13:14:03 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

[Angel Pons (Code Review)]

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

Change subject: lenovo/t440p: fix keyboard backlight
......................................................................


Patch Set 2:
> Patch Set 2: Code-Review-1
>
> cmos default still says it's defaulting to "Both".

What do you mean?


-- 
To view, visit https://review.coreboot.org/c/coreboot/+/38650
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib4c2b1591d26e2bb33f9549e3933efe9a6e0b043
Gerrit-Change-Number: 38650
Gerrit-PatchSet: 2
Gerrit-Owner: Iru Cai (vimacs) <[e-mail]>
Gerrit-Reviewer: Alexander Couzens <[e-mail]>
Gerrit-Reviewer: Angel Pons <[e-mail]>
Gerrit-Reviewer: Dennis Witzig <[e-mail]>
Gerrit-Reviewer: Iru Cai (vimacs) <[e-mail]>
Gerrit-Reviewer: Iru Cai <[e-mail]>
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-Comment-Date: Fri, 31 Jan 2020 19:16:00 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

[Alexander Couzens (Code Review)]

Alexander Couzens has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38650 )

Change subject: lenovo/t440p: fix keyboard backlight
......................................................................


Patch Set 2:
> Patch Set 2:
> > Patch Set 2: Code-Review-1
> >
> > cmos default still says it's defaulting to "Both".
>
> What do you mean?

The file src/mainboard/lenovo/t440p/cmos.default contains "backlight=Both"


-- 
To view, visit https://review.coreboot.org/c/coreboot/+/38650
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib4c2b1591d26e2bb33f9549e3933efe9a6e0b043
Gerrit-Change-Number: 38650
Gerrit-PatchSet: 2
Gerrit-Owner: Iru Cai (vimacs) <[e-mail]>
Gerrit-Reviewer: Alexander Couzens <[e-mail]>
Gerrit-Reviewer: Angel Pons <[e-mail]>
Gerrit-Reviewer: Dennis Witzig <[e-mail]>
Gerrit-Reviewer: Iru Cai (vimacs) <[e-mail]>
Gerrit-Reviewer: Iru Cai <[e-mail]>
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-Comment-Date: Sat, 01 Feb 2020 01:11:27 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

[Angel Pons (Code Review)]

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

Change subject: lenovo/t440p: fix keyboard backlight
......................................................................


Patch Set 2:
> Patch Set 2:
> > Patch Set 2:
> > > Patch Set 2: Code-Review-1
> > >
> > > cmos default still says it's defaulting to "Both".
> >
> > What do you mean?
>
> The file src/mainboard/lenovo/t440p/cmos.default contains "backlight=Both"

Does it?

https://review.coreboot.org/c/coreboot/+/38650/2/src/mainboard/lenovo/t440p/...


-- 
To view, visit https://review.coreboot.org/c/coreboot/+/38650
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib4c2b1591d26e2bb33f9549e3933efe9a6e0b043
Gerrit-Change-Number: 38650
Gerrit-PatchSet: 2
Gerrit-Owner: Iru Cai (vimacs) <[e-mail]>
Gerrit-Reviewer: Alexander Couzens <[e-mail]>
Gerrit-Reviewer: Angel Pons <[e-mail]>
Gerrit-Reviewer: Dennis Witzig <[e-mail]>
Gerrit-Reviewer: Iru Cai (vimacs) <[e-mail]>
Gerrit-Reviewer: Iru Cai <[e-mail]>
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-Comment-Date: Sat, 01 Feb 2020 11:25:01 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

[Alexander Couzens (Code Review)]

Alexander Couzens has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38650 )

Change subject: lenovo/t440p: fix keyboard backlight
......................................................................


Patch Set 2: -Code-Review
> Patch Set 2:
> > Patch Set 2:
> > > Patch Set 2:
> > > > Patch Set 2: Code-Review-1
> > > >
> > > > cmos default still says it's defaulting to "Both".
> > >
> > > What do you mean?
> >
> > The file src/mainboard/lenovo/t440p/cmos.default contains "backlight=Both"
>
> Does it?
>
> https://review.coreboot.org/c/coreboot/+/38650/2/src/mainboard/lenovo/t440p/...

Sorry, maybe I downloaded by accident the v1 of the change via git review.


-- 
To view, visit https://review.coreboot.org/c/coreboot/+/38650
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib4c2b1591d26e2bb33f9549e3933efe9a6e0b043
Gerrit-Change-Number: 38650
Gerrit-PatchSet: 2
Gerrit-Owner: Iru Cai (vimacs) <[e-mail]>
Gerrit-Reviewer: Alexander Couzens <[e-mail]>
Gerrit-Reviewer: Angel Pons <[e-mail]>
Gerrit-Reviewer: Dennis Witzig <[e-mail]>
Gerrit-Reviewer: Iru Cai (vimacs) <[e-mail]>
Gerrit-Reviewer: Iru Cai <[e-mail]>
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-Comment-Date: Mon, 03 Feb 2020 04:01:58 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

[Alexander Couzens (Code Review)]

Alexander Couzens has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38650 )

Change subject: lenovo/t440p: fix keyboard backlight
......................................................................


Patch Set 2: Code-Review+2


-- 
To view, visit https://review.coreboot.org/c/coreboot/+/38650
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib4c2b1591d26e2bb33f9549e3933efe9a6e0b043
Gerrit-Change-Number: 38650
Gerrit-PatchSet: 2
Gerrit-Owner: Iru Cai (vimacs) <[e-mail]>
Gerrit-Reviewer: Alexander Couzens <[e-mail]>
Gerrit-Reviewer: Angel Pons <[e-mail]>
Gerrit-Reviewer: Dennis Witzig <[e-mail]>
Gerrit-Reviewer: Iru Cai (vimacs) <[e-mail]>
Gerrit-Reviewer: Iru Cai <[e-mail]>
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-Comment-Date: Mon, 03 Feb 2020 04:02:14 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

[Alexander Couzens (Code Review)]

Alexander Couzens has submitted this change. ( https://review.coreboot.org/c/coreboot/+/38650 )

Change subject: lenovo/t440p: fix keyboard backlight
......................................................................

lenovo/t440p: fix keyboard backlight

It is found that keyboard backlight in T440p is enabled by clearing
bit 3 of EC RAM 0x01. This patch sets has_keyboard_backlight in
devicetree.cb and also corrects the CMOS configuration.

Change-Id: Ib4c2b1591d26e2bb33f9549e3933efe9a6e0b043
Signed-off-by: Iru Cai <[e-mail]>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/38650
Tested-by: build bot (Jenkins) <[e-mail]>
Reviewed-by: Angel Pons <[e-mail]>
Reviewed-by: Dennis Witzig <[e-mail]>
Reviewed-by: Alexander Couzens <[e-mail]>
---
M src/mainboard/lenovo/t440p/cmos.default
M src/mainboard/lenovo/t440p/cmos.layout
M src/mainboard/lenovo/t440p/devicetree.cb
3 files changed, 6 insertions(+), 4 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Alexander Couzens: Looks good to me, approved
  Angel Pons: Looks good to me, approved
  Dennis Witzig: Looks good to me, but someone else must approve



diff --git a/src/mainboard/lenovo/t440p/cmos.default b/src/mainboard/lenovo/t440p/cmos.default
index b104ae1..0949e7b 100644
--- a/src/mainboard/lenovo/t440p/cmos.default
+++ b/src/mainboard/lenovo/t440p/cmos.default
@@ -9,5 +9,5 @@
 f1_to_f12_as_primary=Enable
 sticky_fn=Disable
 trackpoint=Enable
-backlight=Both
+backlight=Keyboard
 usb_always_on=Disable
diff --git a/src/mainboard/lenovo/t440p/cmos.layout b/src/mainboard/lenovo/t440p/cmos.layout
index 8c5fb0b..f65933a 100644
--- a/src/mainboard/lenovo/t440p/cmos.layout
+++ b/src/mainboard/lenovo/t440p/cmos.layout
@@ -103,9 +103,10 @@
 8     1     Primary
 9     0     AHCI
 9     1     Compatible
-10    0     Both
-10    1     Keyboard only
-10    2     Thinklight only
+# Haswell ThinkPads have no Thinklight
+#10    0     Both
+10    1     Keyboard
+#10    2     Thinklight only
 10    3     None
 #12    0     Integrated Only
 #12    1     Discrete Only
diff --git a/src/mainboard/lenovo/t440p/devicetree.cb b/src/mainboard/lenovo/t440p/devicetree.cb
index 7e5e616..b63767e 100644
--- a/src/mainboard/lenovo/t440p/devicetree.cb
+++ b/src/mainboard/lenovo/t440p/devicetree.cb
@@ -83,6 +83,7 @@
 					register "config1" = "0x0d"
 					register "config2" = "0xa8"
 					register "config3" = "0xc4"
+					register "has_keyboard_backlight" = "1"
 					register "event2_enable" = "0xff"
 					register "event3_enable" = "0xff"
 					register "event4_enable" = "0xd0"

-- 
To view, visit https://review.coreboot.org/c/coreboot/+/38650
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib4c2b1591d26e2bb33f9549e3933efe9a6e0b043
Gerrit-Change-Number: 38650
Gerrit-PatchSet: 3
Gerrit-Owner: Iru Cai (vimacs) <[e-mail]>
Gerrit-Reviewer: Alexander Couzens <[e-mail]>
Gerrit-Reviewer: Angel Pons <[e-mail]>
Gerrit-Reviewer: Dennis Witzig <[e-mail]>
Gerrit-Reviewer: Iru Cai (vimacs) <[e-mail]>
Gerrit-Reviewer: Iru Cai <[e-mail]>
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-MessageType: merged

[9elements QA (Code Review)]

9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38650 )

Change subject: lenovo/t440p: fix keyboard backlight
......................................................................


Patch Set 3:

Automatic boot test returned (PASS/FAIL/TOTAL): 3/0/3
Emulation targets:
EMULATION_QEMU_X86_Q35 using payload TianoCore : SUCCESS : https://lava.9esec.io/r/402
EMULATION_QEMU_X86_Q35 using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/401
EMULATION_QEMU_X86_I440FX using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/400

Please note: This test is under development and might not be accurate at all!


-- 
To view, visit https://review.coreboot.org/c/coreboot/+/38650
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib4c2b1591d26e2bb33f9549e3933efe9a6e0b043
Gerrit-Change-Number: 38650
Gerrit-PatchSet: 3
Gerrit-Owner: Iru Cai (vimacs) <[e-mail]>
Gerrit-Reviewer: Alexander Couzens <[e-mail]>
Gerrit-Reviewer: Angel Pons <[e-mail]>
Gerrit-Reviewer: Dennis Witzig <[e-mail]>
Gerrit-Reviewer: Iru Cai (vimacs) <[e-mail]>
Gerrit-Reviewer: Iru Cai <[e-mail]>
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: 9elements QA <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-Comment-Date: Mon, 03 Feb 2020 04:39:00 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment