coreboot-gerrit January 2021

coreboot-gerrit@coreboot.org

1 participants
6507 discussions

Change in coreboot[master]: mb/clevo/cml-u: Configure IRQ as level triggered for HID over I2C
by Michael Niewöhner (Code Review) 03 Jan '21

03 Jan '21

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47984 ) Change subject: mb/clevo/cml-u: Configure IRQ as level triggered for HID over I2C ...................................................................... Patch Set 9: (2 comments) Given that the touchpad worked before https://review.coreboot.org/c/coreboot/+/47984/6//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/47984/6//COMMIT_MSG@21 PS6, Line 21: successfully > ok ok... […] As discussed on IRQ: Even when the device implements edge, level will work. probably this is done just for supporting shared IRQ lines, where edge wouldn't work reliably. For non-shared lines it probably doesn't make a difference at all, since active-low level also triggers one active-low edge interrupts. https://review.coreboot.org/c/coreboot/+/47984/6/src/mainboard/clevo/cml-u/… File src/mainboard/clevo/cml-u/variants/l140cu/gpio.c: https://review.coreboot.org/c/coreboot/+/47984/6/src/mainboard/clevo/cml-u/… PS6, Line 65: PAD_CFG_GPI_APIC_HIGH > Given the double-inversion story, using _LOW here should work too, […] changed as discussed on irc -- To view, visit https://review.coreboot.org/c/coreboot/+/47984 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia232c0a11546aa6d17614f4cab07c255e58f2fed Gerrit-Change-Number: 47984 Gerrit-PatchSet: 9 Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Sun, 03 Jan 2021 16:54:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/clevo/cml-u: Configure IRQ as level triggered for HID over I2C
by Michael Niewöhner (Code Review) 03 Jan '21

03 Jan '21

1 0

Change in coreboot[master]: mb/clevo/cml-u: Configure IRQ as level triggered for HID over I2C
by Michael Niewöhner (Code Review) 03 Jan '21

03 Jan '21

Hello Felix Singer, build bot (Jenkins), Nico Huber, Furquan Shaikh, Matt DeVillier, Tim Wawrzynczak, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/47984 to look at the new patch set (#9). Change subject: mb/clevo/cml-u: Configure IRQ as level triggered for HID over I2C ...................................................................... mb/clevo/cml-u: Configure IRQ as level triggered for HID over I2C As per HID over I2C Protocol Specification[1] Version 1.00 Section 7.4, the interrupt line used by the device is required to be level triggered. Hence, this change updates the configuration of the HID over I2C devices to be level triggered. References: [1] http://download.microsoft.com/download/7/d/d/7dd44bb7-2a7a-4505-ac1c-7227d3… [2] https://review.coreboot.org/c/coreboot/+/47417/2/src/mainboard/google/hatch… Tested successfully on Clevo L141CU. Change-Id: Ia232c0a11546aa6d17614f4cab07c255e58f2fed Signed-off-by: Michael Niewöhner <foss(a)mniewoehner.de> --- M src/mainboard/clevo/cml-u/variants/l140cu/devicetree.cb M src/mainboard/clevo/cml-u/variants/l140cu/gpio.c 2 files changed, 2 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/47984/9 -- To view, visit https://review.coreboot.org/c/coreboot/+/47984 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia232c0a11546aa6d17614f4cab07c255e58f2fed Gerrit-Change-Number: 47984 Gerrit-PatchSet: 9 Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: mb/clevo/cml-u: Configure IRQ as level triggered for HID over I2C
by Michael Niewöhner (Code Review) 03 Jan '21

03 Jan '21

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47984 ) Change subject: mb/clevo/cml-u: Configure IRQ as level triggered for HID over I2C ...................................................................... Patch Set 8: (1 comment) https://review.coreboot.org/c/coreboot/+/47984/6//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/47984/6//COMMIT_MSG@21 PS6, Line 21: successfully > Wait, are you saying devices are in spec just because they work :D ok ok... you're right, we can't be sure there :D -- To view, visit https://review.coreboot.org/c/coreboot/+/47984 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia232c0a11546aa6d17614f4cab07c255e58f2fed Gerrit-Change-Number: 47984 Gerrit-PatchSet: 8 Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Sun, 03 Jan 2021 16:21:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/clevo/cml-u: Configure IRQ as level triggered for HID over I2C
by Nico Huber (Code Review) 03 Jan '21

03 Jan '21

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47984 ) Change subject: mb/clevo/cml-u: Configure IRQ as level triggered for HID over I2C ...................................................................... Patch Set 8: (1 comment) https://review.coreboot.org/c/coreboot/+/47984/6//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/47984/6//COMMIT_MSG@21 PS6, Line 21: successfully > if it didn't they would produce non-working devices Wait, are you saying devices are in spec just because they work :D -- To view, visit https://review.coreboot.org/c/coreboot/+/47984 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia232c0a11546aa6d17614f4cab07c255e58f2fed Gerrit-Change-Number: 47984 Gerrit-PatchSet: 8 Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Sun, 03 Jan 2021 16:02:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/clevo/cml-u: Configure IRQ as level triggered for HID over I2C
by Michael Niewöhner (Code Review) 03 Jan '21

03 Jan '21

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47984 ) Change subject: mb/clevo/cml-u: Configure IRQ as level triggered for HID over I2C ...................................................................... Patch Set 8: (1 comment) https://review.coreboot.org/c/coreboot/+/47984/6//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/47984/6//COMMIT_MSG@21 PS6, Line 21: successfully > But shouldn't the device comply with the specification too? How do you know that it does? if it didn't they would produce non-working devices -- To view, visit https://review.coreboot.org/c/coreboot/+/47984 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia232c0a11546aa6d17614f4cab07c255e58f2fed Gerrit-Change-Number: 47984 Gerrit-PatchSet: 8 Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Sun, 03 Jan 2021 15:58:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/clevo/cml-u: Configure IRQ as level triggered for HID over I2C
by Michael Niewöhner (Code Review) 03 Jan '21

03 Jan '21

Hello Felix Singer, build bot (Jenkins), Nico Huber, Furquan Shaikh, Matt DeVillier, Tim Wawrzynczak, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/47984 to look at the new patch set (#8). Change subject: mb/clevo/cml-u: Configure IRQ as level triggered for HID over I2C ...................................................................... mb/clevo/cml-u: Configure IRQ as level triggered for HID over I2C As per HID over I2C Protocol Specification[1] Version 1.00 Section 7.4, the interrupt line used by the device is required to be level triggered. Hence, this change updates the configuration of the HID over I2C devices to be level triggered. Note: The signal from the pad gets passed to the ITSS (Interrupt and Timer Subsystem) as-is. There is no need to invert the signal. References: [1] http://download.microsoft.com/download/7/d/d/7dd44bb7-2a7a-4505-ac1c-7227d3… [2] https://review.coreboot.org/c/coreboot/+/47417/2/src/mainboard/google/hatch… Tested successfully on Clevo L141CU. Change-Id: Ia232c0a11546aa6d17614f4cab07c255e58f2fed Signed-off-by: Michael Niewöhner <foss(a)mniewoehner.de> --- M src/mainboard/clevo/cml-u/variants/l140cu/devicetree.cb M src/mainboard/clevo/cml-u/variants/l140cu/gpio.c 2 files changed, 2 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/47984/8 -- To view, visit https://review.coreboot.org/c/coreboot/+/47984 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia232c0a11546aa6d17614f4cab07c255e58f2fed Gerrit-Change-Number: 47984 Gerrit-PatchSet: 8 Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: mb/clevo/cml-u: Configure IRQ as level triggered for HID over I2C
by Nico Huber (Code Review) 03 Jan '21

03 Jan '21

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47984 ) Change subject: mb/clevo/cml-u: Configure IRQ as level triggered for HID over I2C ...................................................................... Patch Set 7: (2 comments) https://review.coreboot.org/c/coreboot/+/47984/6//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/47984/6//COMMIT_MSG@15 PS6, Line 15: PAD_CFG_GPI_APIC_HIGH. There is no need to invert the signal. There is no need because the SoC supports active-low signals in the ITSS, AIUI. But you are using a macro that is named such that all signals going through the ITSS would be active high. Using this macro for other purposes makes the code incredibly hard to follow. Even with this note. https://review.coreboot.org/c/coreboot/+/47984/6/src/mainboard/clevo/cml-u/… File src/mainboard/clevo/cml-u/variants/l140cu/gpio.c: https://review.coreboot.org/c/coreboot/+/47984/6/src/mainboard/clevo/cml-u/… PS6, Line 65: PAD_CFG_GPI_APIC_HIGH Given the double-inversion story, using _LOW here should work too, would be more consistent and much less confusing. -- To view, visit https://review.coreboot.org/c/coreboot/+/47984 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia232c0a11546aa6d17614f4cab07c255e58f2fed Gerrit-Change-Number: 47984 Gerrit-PatchSet: 7 Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Sun, 03 Jan 2021 15:56:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/clevo/cml-u: Configure IRQ as level triggered for HID over I2C
by Michael Niewöhner (Code Review) 03 Jan '21

03 Jan '21

Hello Felix Singer, build bot (Jenkins), Nico Huber, Furquan Shaikh, Matt DeVillier, Tim Wawrzynczak, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/47984 to look at the new patch set (#7). Change subject: mb/clevo/cml-u: Configure IRQ as level triggered for HID over I2C ...................................................................... mb/clevo/cml-u: Configure IRQ as level triggered for HID over I2C As per HID over I2C Protocol Specification[1] Version 1.00 Section 7.4, the interrupt line used by the device is required to be level triggered. Hence, this change updates the configuration of the HID over I2C devices to be level triggered. Note: The signal from the pad gets passed to the ITSS (Interrupt and Timer Subsystem) as-is. There is no need to invert the signal. References: [1] http://download.microsoft.com/download/7/d/d/7dd44bb7-2a7a-4505-ac1c-7227d3… [2] https://review.coreboot.org/c/coreboot/+/47417/2/src/mainboard/google/hatch… Tested successfully on Clevo L141CU. Change-Id: Ia232c0a11546aa6d17614f4cab07c255e58f2fed Signed-off-by: Michael Niewöhner <foss(a)mniewoehner.de> --- M src/mainboard/clevo/cml-u/variants/l140cu/devicetree.cb M src/mainboard/clevo/cml-u/variants/l140cu/gpio.c 2 files changed, 2 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/47984/7 -- To view, visit https://review.coreboot.org/c/coreboot/+/47984 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia232c0a11546aa6d17614f4cab07c255e58f2fed Gerrit-Change-Number: 47984 Gerrit-PatchSet: 7 Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: mb/clevo/cml-u: Configure IRQ as level triggered for HID over I2C
by Felix Singer (Code Review) 03 Jan '21

03 Jan '21

Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47984 ) Change subject: mb/clevo/cml-u: Configure IRQ as level triggered for HID over I2C ...................................................................... Patch Set 6: (1 comment) https://review.coreboot.org/c/coreboot/+/47984/6//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/47984/6//COMMIT_MSG@21 PS6, Line 21: successfully > Well, in this case success is "it doesn't break and doesn't cause dmesg warnings/errors". […] But shouldn't the device comply with the specification too? How do you know that it does? -- To view, visit https://review.coreboot.org/c/coreboot/+/47984 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia232c0a11546aa6d17614f4cab07c255e58f2fed Gerrit-Change-Number: 47984 Gerrit-PatchSet: 6 Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Sun, 03 Jan 2021 15:40:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit January 2021