Change in coreboot[master]: mb/kontron/986lcd-m: Use `winbond_enable_serial()`

List overview All Threads
Download

newer

older

[XS] Change in coreboot[master]:...

Change in coreboot[master]:...

Paul Menzel (Code Review)

25 Feb 2020 25 Feb '20

10:40 p.m.

Paul Menzel has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/39125 )

Change subject: mb/kontron/986lcd-m: Use `winbond_enable_serial()` ......................................................................

mb/kontron/986lcd-m: Use `winbond_enable_serial()`

Note, the common function does not set the IRQ.

pnp_set_irq(dev, PNP_IDX_IRQ0, 4);

Change-Id: Ia437aa31ae92a299d0d0b35db15715997ee9bad3 Signed-off-by: Paul Menzel pmenzel@molgen.mpg.de --- M src/mainboard/kontron/986lcd-m/early_init.c 1 file changed, 6 insertions(+), 10 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/25/39125/1

diff --git a/src/mainboard/kontron/986lcd-m/early_init.c b/src/mainboard/kontron/986lcd-m/early_init.c index 827f792..467d907 100644 --- a/src/mainboard/kontron/986lcd-m/early_init.c +++ b/src/mainboard/kontron/986lcd-m/early_init.c @@ -52,19 +52,15 @@ pnp_write_config(dev, 0x29, 0x43); /* GPIO settings */ pnp_write_config(dev, 0x2a, 0x40); /* GPIO settings */

+ pnp_exit_conf_state(dev); + dev = PNP_DEV(0x2e, W83627THG_SP1); - pnp_set_logical_device(dev); - pnp_set_enable(dev, 0); - pnp_set_iobase(dev, PNP_IDX_IO0, 0x3f8); - pnp_set_irq(dev, PNP_IDX_IRQ0, 4); - pnp_set_enable(dev, 1); + winbond_enable_serial(dev, 0x3f8);

dev = PNP_DEV(0x2e, W83627THG_SP2); - pnp_set_logical_device(dev); - pnp_set_enable(dev, 0); - pnp_set_iobase(dev, PNP_IDX_IO0, 0x2f8); - pnp_set_irq(dev, PNP_IDX_IRQ0, 3); - pnp_set_enable(dev, 1); + winbond_enable_serial(dev, 0x2f8); + + pnp_enter_conf_state(dev);

dev = PNP_DEV(0x2e, W83627THG_KBC); pnp_set_logical_device(dev);

Attachments:

attachment.htm (text/html — 3.7 KB)

Show replies by date

Paul Menzel (Code Review)

26 Feb 26 Feb

10:44 a.m.

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

Change subject: mb/kontron/986lcd-m: Use `winbond_enable_serial()` ......................................................................

Patch Set 1:

Felix, any suggestions regarding the `pnp_set_irq()` integration?

-- To view, visit https://review.coreboot.org/c/coreboot/+/39125 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia437aa31ae92a299d0d0b35db15715997ee9bad3 Gerrit-Change-Number: 39125 Gerrit-PatchSet: 1 Gerrit-Owner: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Wed, 26 Feb 2020 10:44:41 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

Angel Pons (Code Review)

3:45 p.m.

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

Change subject: mb/kontron/986lcd-m: Use `winbond_enable_serial()` ......................................................................

Patch Set 1: Code-Review+1

...

Patch Set 1:

Felix, any suggestions regarding the `pnp_set_irq()` integration?

What's the default value according to the SIO datasheet? If it's the same, it's probably not a big deal.

-- To view, visit https://review.coreboot.org/c/coreboot/+/39125 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia437aa31ae92a299d0d0b35db15715997ee9bad3 Gerrit-Change-Number: 39125 Gerrit-PatchSet: 1 Gerrit-Owner: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Wed, 26 Feb 2020 15:45:04 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

HAOUAS Elyes (Code Review)

28 Feb 28 Feb

10:28 a.m.

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39125 )

Change subject: mb/kontron/986lcd-m: Use `winbond_enable_serial()` ......................................................................

Patch Set 1:

(2 comments)

https://review.coreboot.org/c/coreboot/+/39125/1/src/mainboard/kontron/986lc... File src/mainboard/kontron/986lcd-m/early_init.c:

https://review.coreboot.org/c/coreboot/+/39125/1/src/mainboard/kontron/986lc... PS1, Line 107: pnp_enter_conf_state(dev); : : pnp_set_logical_device(dev); /* Set COM3 to sane non-conflicting values */ : pnp_set_enable(dev, 0); : pnp_set_iobase(dev, PNP_IDX_IO0, 0x3e8); : pnp_set_irq(dev, PNP_IDX_IRQ0, 11); : pnp_set_enable(dev, 1); same here ?

https://review.coreboot.org/c/coreboot/+/39125/1/src/mainboard/kontron/986lc... PS1, Line 116: pnp_set_logical_device(dev); /* Set COM4 to sane non-conflicting values */ : pnp_set_enable(dev, 0); : pnp_set_iobase(dev, PNP_IDX_IO0, 0x2e8); : pnp_set_irq(dev, PNP_IDX_IRQ0, 10); : pnp_set_enable(dev, 1); ditto?

-- To view, visit https://review.coreboot.org/c/coreboot/+/39125 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia437aa31ae92a299d0d0b35db15715997ee9bad3 Gerrit-Change-Number: 39125 Gerrit-PatchSet: 1 Gerrit-Owner: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: HAOUAS Elyes ehaouas@noos.fr Gerrit-Comment-Date: Fri, 28 Feb 2020 10:28:36 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Nico Huber (Code Review)

8 Mar 8 Mar

3:04 p.m.

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39125 )

Change subject: mb/kontron/986lcd-m: Use `winbond_enable_serial()` ......................................................................

Patch Set 1:

winbond_enable_serial() is used to enable the port for console purposes, I assume? Not setting the IRQ shouldn't be a problem, but... there is a lot going on here that shouldn't be necessary beside these IRQ settings. ramstage should take care of most of it.

So, I don't see what this fixes. It doesn't clean up all the early_init code, only parts of it, and only by accident. Which might make the whole story more confusing (currently, it's just configuring the whole SIO, with this change, console setup is mixed in) if somebody wants to clean it up in the future.

If somebody wants to really clean it up (look into each line and decide if it needs to be done early), I'd be happy to test that.

-- To view, visit https://review.coreboot.org/c/coreboot/+/39125 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia437aa31ae92a299d0d0b35db15715997ee9bad3 Gerrit-Change-Number: 39125 Gerrit-PatchSet: 1 Gerrit-Owner: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: HAOUAS Elyes ehaouas@noos.fr Gerrit-Comment-Date: Sun, 08 Mar 2020 15:04:35 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

HAOUAS Elyes (Code Review)

3:37 p.m.

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39125 )

Change subject: mb/kontron/986lcd-m: Use `winbond_enable_serial()` ......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39125/1/src/mainboard/kontron/986lc... File src/mainboard/kontron/986lcd-m/early_init.c:

https://review.coreboot.org/c/coreboot/+/39125/1/src/mainboard/kontron/986lc... PS1, Line 60: dev = PNP_DEV(0x2e, W83627THG_SP2); : winbond_enable_serial(dev, 0x2f8); : : pnp_enter_conf_state(dev); : : dev = PNP_DEV(0x2e, W83627THG_KBC); : pnp_set_logical_device(dev); : pnp_set_enable(dev, 0); : pnp_set_iobase(dev, PNP_IDX_IO0, 0x60); : pnp_set_iobase(dev, PNP_IDX_IO1, 0x64); : pnp_set_enable(dev, 1); : : dev = PNP_DEV(0x2e, W83627THG_GAME_MIDI_GPIO1); : pnp_set_logical_device(dev); : pnp_set_enable(dev, 0); : pnp_write_config(dev, 0xf5, 0xff); /* invert all GPIOs */ : pnp_set_enable(dev, 1); : : dev = PNP_DEV(0x2e, W83627THG_GPIO2); : pnp_set_logical_device(dev); : pnp_set_enable(dev, 1); /* Just enable it */ : : dev = PNP_DEV(0x2e, W83627THG_GPIO3); : pnp_set_logical_device(dev); : pnp_set_enable(dev, 0); : pnp_write_config(dev, 0xf0, 0xfb); /* GPIO bit 2 is output */ : pnp_write_config(dev, 0xf1, 0x00); /* GPIO bit 2 is 0 */ : pnp_write_config(dev, 0x30, 0x03); /* Enable GPIO3+4. pnp_set_enable is not sufficient */ : : dev = PNP_DEV(0x2e, W83627THG_FDC); : pnp_set_logical_device(dev); : pnp_set_enable(dev, 0); : : dev = PNP_DEV(0x2e, W83627THG_PP); : pnp_set_logical_device(dev); : pnp_set_enable(dev, 0); : : /* Enable HWM */ : dev = PNP_DEV(0x2e, W83627THG_HWM); : pnp_set_logical_device(dev); : pnp_set_enable(dev, 0); : pnp_set_iobase(dev, PNP_IDX_IO0, 0xa00); : pnp_set_enable(dev, 1); : : pnp_exit_conf_state(dev); : : dev = PNP_DEV(0x4e, W83627THG_SP1); : pnp_enter_conf_state(dev); : : pnp_set_logical_device(dev); /* Set COM3 to sane non-conflicting values */ : pnp_set_enable(dev, 0); : pnp_set_iobase(dev, PNP_IDX_IO0, 0x3e8); : pnp_set_irq(dev, PNP_IDX_IRQ0, 11); : pnp_set_enable(dev, 1); : : dev = PNP_DEV(0x4e, W83627THG_SP2); : pnp_set_logical_device(dev); /* Set COM4 to sane non-conflicting values */ : pnp_set_enable(dev, 0); : pnp_set_iobase(dev, PNP_IDX_IO0, 0x2e8); : pnp_set_irq(dev, PNP_IDX_IRQ0, 10); : pnp_set_enable(dev, 1); : : dev = PNP_DEV(0x4e, W83627THG_FDC); : pnp_set_logical_device(dev); : pnp_set_enable(dev, 0); : : dev = PNP_DEV(0x4e, W83627THG_PP); : pnp_set_logical_device(dev); : pnp_set_enable(dev, 0); : : dev = PNP_DEV(0x4e, W83627THG_KBC); : pnp_set_logical_device(dev); : pnp_set_enable(dev, 0); : pnp_set_iobase(dev, PNP_IDX_IO0, 0x00); : pnp_set_iobase(dev, PNP_IDX_IO1, 0x00); : : pnp_exit_conf_state(dev); I think you can remove all of this ...

-- To view, visit https://review.coreboot.org/c/coreboot/+/39125 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia437aa31ae92a299d0d0b35db15715997ee9bad3 Gerrit-Change-Number: 39125 Gerrit-PatchSet: 1 Gerrit-Owner: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: HAOUAS Elyes ehaouas@noos.fr Gerrit-Comment-Date: Sun, 08 Mar 2020 15:37:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Paul Menzel (Code Review)

9 Mar 9 Mar

1:28 p.m.

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

Change subject: mb/kontron/986lcd-m: Use `winbond_enable_serial()` ......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39125/1/src/mainboard/kontron/986lc... File src/mainboard/kontron/986lcd-m/early_init.c:

...

I think you can remove all of this ...

Please elaborate? Because it’s done somewhere else, and not needed in bootblock?

-- To view, visit https://review.coreboot.org/c/coreboot/+/39125 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia437aa31ae92a299d0d0b35db15715997ee9bad3 Gerrit-Change-Number: 39125 Gerrit-PatchSet: 1 Gerrit-Owner: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: HAOUAS Elyes ehaouas@noos.fr Gerrit-Comment-Date: Mon, 09 Mar 2020 13:28:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: HAOUAS Elyes ehaouas@noos.fr Gerrit-MessageType: comment

Stefan Reinauer (Code Review)

10 Jun 10 Jun

3:07 a.m.

New subject: [S] Change in coreboot[master]: mb/kontron/986lcd-m: Use `winbond_enable_serial()`

Stefan Reinauer has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/39125?usp=email )

Change subject: mb/kontron/986lcd-m: Use `winbond_enable_serial()` ......................................................................

Abandoned

-- To view, visit https://review.coreboot.org/c/coreboot/+/39125?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia437aa31ae92a299d0d0b35db15715997ee9bad3 Gerrit-Change-Number: 39125 Gerrit-PatchSet: 1 Gerrit-Owner: Paul Menzel paulepanter@mailbox.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Paul Menzel paulepanter@mailbox.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-MessageType: abandon

328

days inactive

1529

days old

coreboot-gerrit@coreboot.org

7 comments

5 participants

tags (0)

participants (5)

Angel Pons (Code Review)
HAOUAS Elyes (Code Review)
Nico Huber (Code Review)
Paul Menzel (Code Review)
Stefan Reinauer (Code Review)