Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/30957
Change subject: superio/ite: Add and use it8528e ......................................................................
superio/ite: Add and use it8528e
* Add SuperIO ITE8528E * Use ITE8528E to configure serial on wedge100s
TODO: Add support for accessing EC space.
Tested on wedge100s. The serial works without CONFIG_CONSOLE_SERIAL.
Change-Id: I72aa756e123d6f99d9ef4fe955c4b7f1be25d547 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/mainboard/ocp/wedge100s/Kconfig M src/mainboard/ocp/wedge100s/devicetree.cb M src/superio/ite/Makefile.inc A src/superio/ite/it8528e/Kconfig A src/superio/ite/it8528e/Makefile.inc A src/superio/ite/it8528e/chip.h A src/superio/ite/it8528e/it8528e.h A src/superio/ite/it8528e/superio.c 8 files changed, 226 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/30957/1
diff --git a/src/mainboard/ocp/wedge100s/Kconfig b/src/mainboard/ocp/wedge100s/Kconfig index ce9c097..6224340 100644 --- a/src/mainboard/ocp/wedge100s/Kconfig +++ b/src/mainboard/ocp/wedge100s/Kconfig @@ -16,6 +16,7 @@ select MAINBOARD_HAS_LPC_TPM select MAINBOARD_HAS_TPM1 select DRIVERS_UART_8250IO + select SUPERIO_ITE_IT8528E
config VBOOT select VBOOT_VBNV_CMOS diff --git a/src/mainboard/ocp/wedge100s/devicetree.cb b/src/mainboard/ocp/wedge100s/devicetree.cb index 3d66d0d..fc6dccc 100644 --- a/src/mainboard/ocp/wedge100s/devicetree.cb +++ b/src/mainboard/ocp/wedge100s/devicetree.cb @@ -11,6 +11,58 @@ chip drivers/pc80/tpm device pnp 0c31.0 on end end + chip superio/ite/it8528e + # COM1, routed to COM-e header + device pnp 6e.1 on + io 0x60 = 0x3f8 + irq 0x70 = 4 + end + # COM2, routed to COM-e header + device pnp 6e.2 on + io 0x60 = 0x2f8 + irq 0x70 = 3 + end + device pnp 6e.4 off end + device pnp 6e.5 off end + device pnp 6e.6 off end + device pnp 6e.a off end + device pnp 6e.f off end + device pnp 6e.10 off + io 0x60 = 0x70 + io 0x62 = 0x72 + irq 0x70 = 8 + end + device pnp 6e.11 off + io 0x60 = 0x620 + io 0x62 = 0x660 + irq 0x70 = 1 + end + device pnp 6e.12 off + io 0x60 = 0x680 + io 0x62 = 0x6c0 + irq 0x70 = 1 + end + device pnp 6e.13 off + io 0x60 = 0x300 + irq 0x70 = 2 + end + device pnp 6e.14 off end + device pnp 6e.17 off + io 0x60 = 0x6a0 + io 0x62 = 0x6e0 + irq 0x70 = 1 + end + device pnp 6e.18 off + io 0x60 = 0x740 + io 0x62 = 0x780 + irq 0x70 = 1 + end + device pnp 6e.19 off + io 0x60 = 0x7a0 + io 0x62 = 0x7c0 + irq 0x70 = 1 + end + end #superio/ite/it8528e end # LPC Bridge device pci 1f.2 on end # SATA Controller device pci 1f.3 on end # SMBus Controller diff --git a/src/superio/ite/Makefile.inc b/src/superio/ite/Makefile.inc index e73fd71..551abe9 100644 --- a/src/superio/ite/Makefile.inc +++ b/src/superio/ite/Makefile.inc @@ -20,6 +20,7 @@ ## include generic ite environment controller driver ramstage-$(CONFIG_SUPERIO_ITE_ENV_CTRL) += common/env_ctrl.c
+subdirs-y += it8528e subdirs-y += it8623e subdirs-y += it8671f subdirs-y += it8712f diff --git a/src/superio/ite/it8528e/Kconfig b/src/superio/ite/it8528e/Kconfig new file mode 100644 index 0000000..3815c28 --- /dev/null +++ b/src/superio/ite/it8528e/Kconfig @@ -0,0 +1,18 @@ +## +## This file is part of the coreboot project. +## +## Copyright (C) 2019 Patrick Rudolph patrick.rudolph@9elements.com +## +## This program is free software; you can redistribute it and/or modify +## it under the terms of the GNU General Public License as published by +## the Free Software Foundation; version 2 of the License. +## +## This program is distributed in the hope that it will be useful, +## but WITHOUT ANY WARRANTY; without even the implied warranty of +## MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +## GNU General Public License for more details. +## + +config SUPERIO_ITE_IT8528E + bool + select SUPERIO_ITE_COMMON_PRE_RAM diff --git a/src/superio/ite/it8528e/Makefile.inc b/src/superio/ite/it8528e/Makefile.inc new file mode 100644 index 0000000..def04e5 --- /dev/null +++ b/src/superio/ite/it8528e/Makefile.inc @@ -0,0 +1,17 @@ +## +## This file is part of the coreboot project. +## +## Copyright (C) 2019 Patrick Rudolph patrick.rudolph@9elements.com +## +## This program is free software; you can redistribute it and/or modify +## it under the terms of the GNU General Public License as published by +## the Free Software Foundation; either version 2 of the License, or +## (at your option) any later version. +## +## This program is distributed in the hope that it will be useful, +## but WITHOUT ANY WARRANTY; without even the implied warranty of +## MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +## GNU General Public License for more details. +## + +ramstage-$(CONFIG_SUPERIO_ITE_IT8528E) += superio.c diff --git a/src/superio/ite/it8528e/chip.h b/src/superio/ite/it8528e/chip.h new file mode 100644 index 0000000..ef7e14f --- /dev/null +++ b/src/superio/ite/it8528e/chip.h @@ -0,0 +1,26 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2019 Patrick Rudolph patrick.rudolph@9elements.com + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#ifndef SUPERIO_ITE_IT8528E_CHIP_H +#define SUPERIO_ITE_IT8528E_CHIP_H + +#include <superio/ite/common/env_ctrl_chip.h> + +struct superio_ite_it8528e_config { + // FIXME: Add support for EC +}; + +#endif /* SUPERIO_ITE_IT8528E_CHIP_H */ diff --git a/src/superio/ite/it8528e/it8528e.h b/src/superio/ite/it8528e/it8528e.h new file mode 100644 index 0000000..7b19349 --- /dev/null +++ b/src/superio/ite/it8528e/it8528e.h @@ -0,0 +1,37 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2019 Patrick Rudolph patrick.rudolph@9elements.com + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#ifndef SUPERIO_ITE_IT8528E_H +#define SUPERIO_ITE_IT8528E_H + +#define IT8528E_SP1 0x01 /* Com1 */ +#define IT8528E_SP2 0x02 /* Com2 */ +#define IT8528E_SWUC 0x04 /* System Wake-Up */ +#define IT8528E_KBCM 0x05 /* PS/2 mouse */ +#define IT8528E_KBCK 0x06 /* PS/2 keyboard */ +#define IT8528E_IR 0x0a /* Consumer IR */ +#define IT8528E_SMFI 0x0f /* Shared Memory/Flash Interface */ +#define IT8528E_RTCT 0x10 /* RTC-like Timer */ +#define IT8528E_PMC1 0x11 /* Power Management Channel 1 */ +#define IT8528E_PMC2 0x12 /* Power Management Channel 2 */ +#define IT8528E_SSPI 0x13 /* Serial Periphial Interface */ +#define IT8528E_PECI 0x14 /* Platform EC Interface */ +#define IT8528E_PMC3 0x17 /* Power Management Channel 3 */ +#define IT8528E_PMC4 0x18 /* Power Management Channel 4 */ +#define IT8528E_PMC5 0x19 /* Power Management Channel 5 */ + + +#endif /* SUPERIO_ITE_IT8528E_H */ diff --git a/src/superio/ite/it8528e/superio.c b/src/superio/ite/it8528e/superio.c new file mode 100644 index 0000000..ae2a8e9 --- /dev/null +++ b/src/superio/ite/it8528e/superio.c @@ -0,0 +1,74 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2006 Uwe Hermann uwe@hermann-uwe.de + * Copyright (C) 2007 Philipp Degler pdegler@rumms.uni-mannheim.de + * Copyright (C) 2017 Gergely Kiss mail.gery@gmail.com + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <device/device.h> +#include <device/pnp.h> +#include <arch/io.h> +#include <stdlib.h> +#include <superio/conf_mode.h> + +#include "chip.h" +#include "it8528e.h" + +static void it8528e_init(struct device *dev) +{ + // FIXME: Init EC +} + +static struct device_operations ops = { + .read_resources = pnp_read_resources, + .set_resources = pnp_set_resources, + .enable_resources = pnp_enable_resources, + .enable = pnp_alt_enable, + .init = it8528e_init, + .ops_pnp_mode = &pnp_conf_mode_870155_aa, +}; + +static struct pnp_info pnp_dev_info[] = { + { NULL, IT8528E_SP1, PNP_IO0 | PNP_IRQ0, 0x0ff8, }, + { NULL, IT8528E_SP2, PNP_IO0 | PNP_IRQ0, 0x0ff8, }, + { NULL, IT8528E_SWUC, PNP_IO0 | PNP_IRQ0, 0xfff0, }, + { NULL, IT8528E_KBCM, PNP_IRQ0, }, + /* Documentation: Programm io0 = 0x60 and io1 = 0x64 */ + { NULL, IT8528E_KBCK, PNP_IO0 | PNP_IO1 | PNP_IRQ0, 0x07ff, 0x07ff, }, + { NULL, IT8528E_IR, PNP_IO0 | PNP_IRQ0, 0xfff8, }, + { NULL, IT8528E_SMFI, PNP_IO0 | PNP_IRQ0, 0xfff0, }, + /* Documentation: Programm io0 = 0x70 and io1 = 0x272 */ + { NULL, IT8528E_RTCT, PNP_IO0 | PNP_IO1 | PNP_IO2 | PNP_IO3 | PNP_IRQ0, + 0xfffe, 0xfffe, 0xfffe, 0xfffe}, + /* Documentation: Programm io0 = 0x62 and io1 = 0x66 */ + { NULL, IT8528E_PMC1, PNP_IO0 | PNP_IO1 | PNP_IRQ0 , 0x07ff, 0x07ff }, + { NULL, IT8528E_PMC2, PNP_IO0 | PNP_IO1 | PNP_IO2 | PNP_IRQ0 , 0x07fc, + 0x07fc, 0xfff0 }, + /* Documentation is unclear if PMC3-5 have LPC I/O decoding support */ + { NULL, IT8528E_PMC3, PNP_IO0 | PNP_IO1 | PNP_IRQ0 , 0x07ff, 0x07ff }, + { NULL, IT8528E_PMC4, PNP_IO0 | PNP_IO1 | PNP_IRQ0 , 0x07ff, 0x07ff }, + { NULL, IT8528E_PMC5, PNP_IO0 | PNP_IO1 | PNP_IRQ0 , 0x07ff, 0x07ff }, + { NULL, IT8528E_SSPI, PNP_IO0 | PNP_IRQ0, 0xfff8 }, + { NULL, IT8528E_PECI, PNP_IO0 , 0xfff8 }, +}; + +static void enable_dev(struct device *dev) +{ + pnp_enable_devices(dev, &ops, ARRAY_SIZE(pnp_dev_info), pnp_dev_info); +} + +struct chip_operations superio_ite_it8528e_ops = { + CHIP_NAME("ITE IT8528E Super I/O") + .enable_dev = enable_dev, +};
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30957 )
Change subject: superio/ite: Add and use it8528e ......................................................................
Patch Set 1:
(9 comments)
https://review.coreboot.org/#/c/30957/1/src/superio/ite/it8528e/superio.c File src/superio/ite/it8528e/superio.c:
https://review.coreboot.org/#/c/30957/1/src/superio/ite/it8528e/superio.c@47 PS1, Line 47: /* Documentation: Programm io0 = 0x60 and io1 = 0x64 */ 'Programm' may be misspelled - perhaps 'Program'?
https://review.coreboot.org/#/c/30957/1/src/superio/ite/it8528e/superio.c@51 PS1, Line 51: /* Documentation: Programm io0 = 0x70 and io1 = 0x272 */ 'Programm' may be misspelled - perhaps 'Program'?
https://review.coreboot.org/#/c/30957/1/src/superio/ite/it8528e/superio.c@54 PS1, Line 54: /* Documentation: Programm io0 = 0x62 and io1 = 0x66 */ 'Programm' may be misspelled - perhaps 'Program'?
https://review.coreboot.org/#/c/30957/1/src/superio/ite/it8528e/superio.c@55 PS1, Line 55: { NULL, IT8528E_PMC1, PNP_IO0 | PNP_IO1 | PNP_IRQ0 , 0x07ff, 0x07ff }, space prohibited before that ',' (ctx:WxW)
https://review.coreboot.org/#/c/30957/1/src/superio/ite/it8528e/superio.c@56 PS1, Line 56: { NULL, IT8528E_PMC2, PNP_IO0 | PNP_IO1 | PNP_IO2 | PNP_IRQ0 , 0x07fc, space prohibited before that ',' (ctx:WxW)
https://review.coreboot.org/#/c/30957/1/src/superio/ite/it8528e/superio.c@59 PS1, Line 59: { NULL, IT8528E_PMC3, PNP_IO0 | PNP_IO1 | PNP_IRQ0 , 0x07ff, 0x07ff }, space prohibited before that ',' (ctx:WxW)
https://review.coreboot.org/#/c/30957/1/src/superio/ite/it8528e/superio.c@60 PS1, Line 60: { NULL, IT8528E_PMC4, PNP_IO0 | PNP_IO1 | PNP_IRQ0 , 0x07ff, 0x07ff }, space prohibited before that ',' (ctx:WxW)
https://review.coreboot.org/#/c/30957/1/src/superio/ite/it8528e/superio.c@61 PS1, Line 61: { NULL, IT8528E_PMC5, PNP_IO0 | PNP_IO1 | PNP_IRQ0 , 0x07ff, 0x07ff }, space prohibited before that ',' (ctx:WxW)
https://review.coreboot.org/#/c/30957/1/src/superio/ite/it8528e/superio.c@63 PS1, Line 63: { NULL, IT8528E_PECI, PNP_IO0 , 0xfff8 }, space prohibited before that ',' (ctx:WxW)
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30957 )
Change subject: superio/ite: Add and use it8528e ......................................................................
Patch Set 1:
please split the superio support and the board support in two patches. also what jenkins said
Hello Felix Held, Philipp Deppenwiese, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30957
to look at the new patch set (#2).
Change subject: superio/ite: Add and use it8528e ......................................................................
superio/ite: Add and use it8528e
* Add SuperIO ITE8528E
TODO: Add support for accessing EC space.
Tested on wedge100s. The serial works without CONFIG_CONSOLE_SERIAL.
Change-Id: I72aa756e123d6f99d9ef4fe955c4b7f1be25d547 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/superio/ite/Makefile.inc A src/superio/ite/it8528e/Kconfig A src/superio/ite/it8528e/Makefile.inc A src/superio/ite/it8528e/chip.h A src/superio/ite/it8528e/it8528e.h A src/superio/ite/it8528e/superio.c 6 files changed, 173 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/30957/2
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30957 )
Change subject: superio/ite: Add and use it8528e ......................................................................
Patch Set 2: Code-Review+1
(1 comment)
https://review.coreboot.org/#/c/30957/2/src/superio/ite/it8528e/superio.c File src/superio/ite/it8528e/superio.c:
https://review.coreboot.org/#/c/30957/2/src/superio/ite/it8528e/superio.c@53 PS2, Line 53: indent with one tab and not one space? same below
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30957 )
Change subject: superio/ite: Add and use it8528e ......................................................................
Patch Set 2:
Isn't this an EC? I wouldn't mind if we handle their PnP interface separately as SIO. Not sure how to avoid confusion, though.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30957 )
Change subject: superio/ite: Add and use it8528e ......................................................................
Patch Set 2:
(1 comment)
Patch Set 2:
Isn't this an EC? I wouldn't mind if we handle their PnP interface separately as SIO. Not sure how to avoid confusion, though.
haven't found a datasheet, so I winder if it has its own firmware. for me the difference between sio and ec is mainly that the ec has firmware (and in the laptop case keyboard matrix scanning support); usually an ec is a superset of a sio
https://review.coreboot.org/#/c/30957/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/30957/2//COMMIT_MSG@9 PS2, Line 9: ITE8528E IT8528E
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30957 )
Change subject: superio/ite: Add and use it8528e ......................................................................
Patch Set 2:
IT8516E is an ec and the pnp stuff is implemented there; splitting the chip into a sio and an ec part is probably a bad idea though. since sio and ec aren't that clearly distinguishable and have an overlapping feature set, it might be worth a thought to merge them into one directory though
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30957 )
Change subject: superio/ite: Add and use it8528e ......................................................................
Patch Set 2:
Yeah, the whole IT85xx line seems to be ECs. We have some in `ec/<possible firmware vendor>/it85*`. Their PnP interface seems to be independent from the programmable part, though. So maybe treating the interfaces separately would be feasible in this case.
We can't share the EC firmware interface parts ofc, but we could share the PnP definitions. Though, if this is going to be another driver that doesn't make this distinction (and gets firmware interface parts added later), it should probably reside in `ec/`.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30957 )
Change subject: superio/ite: Add and use it8528e ......................................................................
Patch Set 2:
Patch Set 2:
Yeah, the whole IT85xx line seems to be ECs. We have some in `ec/<possible firmware vendor>/it85*`. Their PnP interface seems to be independent from the programmable part, though. So maybe treating the interfaces separately would be feasible in this case.
We can't share the EC firmware interface parts ofc, but we could share the PnP definitions. Though, if this is going to be another driver that doesn't make this distinction (and gets firmware interface parts added later), it should probably reside in `ec/`.
Ok, yes, it seems to make sense to split those parts then.
Hello Felix Held, Philipp Deppenwiese, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30957
to look at the new patch set (#3).
Change subject: superio/ite: Add and use it8528e ......................................................................
superio/ite: Add and use it8528e
* Add SuperIO ITE8528E
TODO: Add support for accessing EC space.
Tested on wedge100s. The serial works without CONFIG_CONSOLE_SERIAL.
Change-Id: I72aa756e123d6f99d9ef4fe955c4b7f1be25d547 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/superio/ite/Makefile.inc A src/superio/ite/it8528e/Kconfig A src/superio/ite/it8528e/Makefile.inc A src/superio/ite/it8528e/chip.h A src/superio/ite/it8528e/it8528e.h A src/superio/ite/it8528e/superio.c 6 files changed, 173 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/30957/3
Hello Felix Held, Philipp Deppenwiese, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30957
to look at the new patch set (#4).
Change subject: superio/ite: Add and use it8528e ......................................................................
superio/ite: Add and use it8528e
* Add SuperIO IT8528E
TODO: Add support for accessing EC space.
Tested on wedge100s. The serial works without CONFIG_CONSOLE_SERIAL.
Change-Id: I72aa756e123d6f99d9ef4fe955c4b7f1be25d547 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/superio/ite/Makefile.inc A src/superio/ite/it8528e/Kconfig A src/superio/ite/it8528e/Makefile.inc A src/superio/ite/it8528e/chip.h A src/superio/ite/it8528e/it8528e.h A src/superio/ite/it8528e/superio.c 6 files changed, 173 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/30957/4
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30957 )
Change subject: superio/ite: Add and use it8528e ......................................................................
Patch Set 4:
(2 comments)
Patch Set 2:
Patch Set 2:
Yeah, the whole IT85xx line seems to be ECs. We have some in `ec/<possible firmware vendor>/it85*`. Their PnP interface seems to be independent from the programmable part, though. So maybe treating the interfaces separately would be feasible in this case.
We can't share the EC firmware interface parts ofc, but we could share the PnP definitions. Though, if this is going to be another driver that doesn't make this distinction (and gets firmware interface parts added later), it should probably reside in `ec/`.
Ok, yes, it seems to make sense to split those parts then.
Oh, I didn't know that there's IT85xx support already.
The EC is mostly independent of the SuperIO configuration as it has it's own address space. Looks like a good idea to place it in ec/
https://review.coreboot.org/#/c/30957/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/30957/2//COMMIT_MSG@9 PS2, Line 9: ITE8528E
IT8528E
Done
https://review.coreboot.org/#/c/30957/2/src/superio/ite/it8528e/superio.c File src/superio/ite/it8528e/superio.c:
https://review.coreboot.org/#/c/30957/2/src/superio/ite/it8528e/superio.c@53 PS2, Line 53:
indent with one tab and not one space? same below
Done
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30957 )
Change subject: superio/ite: Add and use it8528e ......................................................................
Patch Set 4: Code-Review+1
(5 comments)
https://review.coreboot.org/#/c/30957/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/30957/4//COMMIT_MSG@7 PS4, Line 7: and use Can be removed?
https://review.coreboot.org/#/c/30957/4//COMMIT_MSG@9 PS4, Line 9: * Add SuperIO IT8528E Could you extend the message with the discussion in the comments, that it’s actually an EC?
https://review.coreboot.org/#/c/30957/4//COMMIT_MSG@10 PS4, Line 10: Could you please also add the datasheet name/URL?
https://review.coreboot.org/#/c/30957/4//COMMIT_MSG@13 PS4, Line 13: The serial works without CONFIG_CONSOLE_SERIAL. Is that good or bad? Or do you mean, that it works under the OS?
https://review.coreboot.org/#/c/30957/4/src/superio/ite/it8528e/superio.c File src/superio/ite/it8528e/superio.c:
https://review.coreboot.org/#/c/30957/4/src/superio/ite/it8528e/superio.c@36 PS4, Line 36: .enable_resources = pnp_enable_resources, Use tabs for alignment in the whole block?
Hello Felix Held, Paul Menzel, Philipp Deppenwiese, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30957
to look at the new patch set (#5).
Change subject: superio/ite: Add it8528e ......................................................................
superio/ite: Add it8528e
* Add support for the SuperIO part of IT8528E * Based on the IT8528E datasheet and tests on vendor firmware
TODO: Add support for accessing EC space.
No datasheet is publicy available.
Tested on wedge100s. The serial works under the OS without CONFIG_CONSOLE_SERIAL.
Change-Id: I72aa756e123d6f99d9ef4fe955c4b7f1be25d547 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/superio/ite/Makefile.inc A src/superio/ite/it8528e/Kconfig A src/superio/ite/it8528e/Makefile.inc A src/superio/ite/it8528e/chip.h A src/superio/ite/it8528e/it8528e.h A src/superio/ite/it8528e/superio.c 6 files changed, 173 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/30957/5
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30957 )
Change subject: superio/ite: Add it8528e ......................................................................
Patch Set 5:
(4 comments)
https://review.coreboot.org/#/c/30957/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/30957/4//COMMIT_MSG@7 PS4, Line 7: and use
Can be removed?
Done
https://review.coreboot.org/#/c/30957/4//COMMIT_MSG@9 PS4, Line 9: * Add SuperIO IT8528E
Could you extend the message with the discussion in the comments, that it’s actually an EC?
Done
https://review.coreboot.org/#/c/30957/4//COMMIT_MSG@10 PS4, Line 10:
Could you please also add the datasheet name/URL?
Done
https://review.coreboot.org/#/c/30957/4//COMMIT_MSG@13 PS4, Line 13: The serial works without CONFIG_CONSOLE_SERIAL.
Is that good or bad? Or do you mean, that it works under the OS?
Done
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30957 )
Change subject: superio/ite: Add it8528e ......................................................................
Patch Set 5: Code-Review+1
(3 comments)
https://review.coreboot.org/#/c/30957/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/30957/5//COMMIT_MSG@12 PS5, Line 12: TODO: Add support for accessing EC space. Update with your plans where to put that?
https://review.coreboot.org/#/c/30957/5/src/superio/ite/it8528e/chip.h File src/superio/ite/it8528e/chip.h:
PS5: If you are going to treat the EC part as a (logically) separate chip, you won't need this.
https://review.coreboot.org/#/c/30957/5/src/superio/ite/it8528e/chip.h@4 PS5, Line 4: * Copyright (C) 2019 Patrick Rudolph patrick.rudolph@9elements.com There is a tendency to only add copyright notes when they make sense. Basically, when you add some creative part (e.g. design a new compo- sition or start something new in general, maybe also when you add some loops or branches to existing code). This also means that copy- right years should only be updated on such creative changes. AIUI, not when you just touch some lines.
Also are you sure about the attribution? I would have expected 9elements here. But it's hard to map the German "Nutzungsrechte" to copyright and the former depends on your contract with them anyway.
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30957 )
Change subject: superio/ite: Add it8528e ......................................................................
Patch Set 5:
(2 comments)
https://review.coreboot.org/#/c/30957/5/src/superio/ite/it8528e/chip.h File src/superio/ite/it8528e/chip.h:
PS5:
If you are going to treat the EC part as a (logically) separate chip, […]
I agree.
https://review.coreboot.org/#/c/30957/5/src/superio/ite/it8528e/chip.h@4 PS5, Line 4: * Copyright (C) 2019 Patrick Rudolph patrick.rudolph@9elements.com
There is a tendency to only add copyright notes when they make sense. […]
Well I came up with the super brilliant name "SUPERIO_ITE_IT8528E_CHIP_H" ! That needs to be copyright protected !
I'll change it to 9elements.
Hello Felix Held, Paul Menzel, Philipp Deppenwiese, build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30957
to look at the new patch set (#6).
Change subject: superio/ite: Add it8528e ......................................................................
superio/ite: Add it8528e
* Add support for the SuperIO part of IT8528E * Based on the IT8528E datasheet and tests on vendor firmware
TODO: Add support for accessing EC space, which should be implemented in src/ec/ instead, as it's a separate logical unit.
No datasheet is publicy available.
Tested on wedge100s. The serial works under the OS without CONFIG_CONSOLE_SERIAL.
Change-Id: I72aa756e123d6f99d9ef4fe955c4b7f1be25d547 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/superio/ite/Makefile.inc A src/superio/ite/it8528e/Kconfig A src/superio/ite/it8528e/Makefile.inc A src/superio/ite/it8528e/it8528e.h A src/superio/ite/it8528e/superio.c 5 files changed, 146 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/30957/6
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30957 )
Change subject: superio/ite: Add it8528e ......................................................................
Patch Set 6:
(2 comments)
https://review.coreboot.org/#/c/30957/5/src/superio/ite/it8528e/chip.h File src/superio/ite/it8528e/chip.h:
PS5:
I agree.
Done
https://review.coreboot.org/#/c/30957/5/src/superio/ite/it8528e/chip.h@4 PS5, Line 4: * Copyright (C) 2019 Patrick Rudolph patrick.rudolph@9elements.com
Well I came up with the super brilliant name "SUPERIO_ITE_IT8528E_CHIP_H" ! That needs to be copyrig […]
Done
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30957 )
Change subject: superio/ite: Add it8528e ......................................................................
Patch Set 6: Code-Review+2
Felix Held has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/30957 )
Change subject: superio/ite: Add it8528e ......................................................................
superio/ite: Add it8528e
* Add support for the SuperIO part of IT8528E * Based on the IT8528E datasheet and tests on vendor firmware
TODO: Add support for accessing EC space, which should be implemented in src/ec/ instead, as it's a separate logical unit.
No datasheet is publicy available.
Tested on wedge100s. The serial works under the OS without CONFIG_CONSOLE_SERIAL.
Change-Id: I72aa756e123d6f99d9ef4fe955c4b7f1be25d547 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com Reviewed-on: https://review.coreboot.org/c/30957 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Felix Held felix-coreboot@felixheld.de --- M src/superio/ite/Makefile.inc A src/superio/ite/it8528e/Kconfig A src/superio/ite/it8528e/Makefile.inc A src/superio/ite/it8528e/it8528e.h A src/superio/ite/it8528e/superio.c 5 files changed, 146 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Felix Held: Looks good to me, approved
diff --git a/src/superio/ite/Makefile.inc b/src/superio/ite/Makefile.inc index e73fd71..551abe9 100644 --- a/src/superio/ite/Makefile.inc +++ b/src/superio/ite/Makefile.inc @@ -20,6 +20,7 @@ ## include generic ite environment controller driver ramstage-$(CONFIG_SUPERIO_ITE_ENV_CTRL) += common/env_ctrl.c
+subdirs-y += it8528e subdirs-y += it8623e subdirs-y += it8671f subdirs-y += it8712f diff --git a/src/superio/ite/it8528e/Kconfig b/src/superio/ite/it8528e/Kconfig new file mode 100644 index 0000000..86f4048 --- /dev/null +++ b/src/superio/ite/it8528e/Kconfig @@ -0,0 +1,18 @@ +## +## This file is part of the coreboot project. +## +## Copyright (C) 2019 9Elements GmbH patrick.rudolph@9elements.com +## +## This program is free software; you can redistribute it and/or modify +## it under the terms of the GNU General Public License as published by +## the Free Software Foundation; version 2 of the License. +## +## This program is distributed in the hope that it will be useful, +## but WITHOUT ANY WARRANTY; without even the implied warranty of +## MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +## GNU General Public License for more details. +## + +config SUPERIO_ITE_IT8528E + bool + select SUPERIO_ITE_COMMON_PRE_RAM diff --git a/src/superio/ite/it8528e/Makefile.inc b/src/superio/ite/it8528e/Makefile.inc new file mode 100644 index 0000000..deb1bc0 --- /dev/null +++ b/src/superio/ite/it8528e/Makefile.inc @@ -0,0 +1,17 @@ +## +## This file is part of the coreboot project. +## +## Copyright (C) 2019 9Elements GmbH patrick.rudolph@9elements.com +## +## This program is free software; you can redistribute it and/or modify +## it under the terms of the GNU General Public License as published by +## the Free Software Foundation; either version 2 of the License, or +## (at your option) any later version. +## +## This program is distributed in the hope that it will be useful, +## but WITHOUT ANY WARRANTY; without even the implied warranty of +## MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +## GNU General Public License for more details. +## + +ramstage-$(CONFIG_SUPERIO_ITE_IT8528E) += superio.c diff --git a/src/superio/ite/it8528e/it8528e.h b/src/superio/ite/it8528e/it8528e.h new file mode 100644 index 0000000..d9d07d0 --- /dev/null +++ b/src/superio/ite/it8528e/it8528e.h @@ -0,0 +1,37 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2019 9Elements GmbH patrick.rudolph@9elements.com + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#ifndef SUPERIO_ITE_IT8528E_H +#define SUPERIO_ITE_IT8528E_H + +#define IT8528E_SP1 0x01 /* Com1 */ +#define IT8528E_SP2 0x02 /* Com2 */ +#define IT8528E_SWUC 0x04 /* System Wake-Up */ +#define IT8528E_KBCM 0x05 /* PS/2 mouse */ +#define IT8528E_KBCK 0x06 /* PS/2 keyboard */ +#define IT8528E_IR 0x0a /* Consumer IR */ +#define IT8528E_SMFI 0x0f /* Shared Memory/Flash Interface */ +#define IT8528E_RTCT 0x10 /* RTC-like Timer */ +#define IT8528E_PMC1 0x11 /* Power Management Channel 1 */ +#define IT8528E_PMC2 0x12 /* Power Management Channel 2 */ +#define IT8528E_SSPI 0x13 /* Serial Periphial Interface */ +#define IT8528E_PECI 0x14 /* Platform EC Interface */ +#define IT8528E_PMC3 0x17 /* Power Management Channel 3 */ +#define IT8528E_PMC4 0x18 /* Power Management Channel 4 */ +#define IT8528E_PMC5 0x19 /* Power Management Channel 5 */ + + +#endif /* SUPERIO_ITE_IT8528E_H */ diff --git a/src/superio/ite/it8528e/superio.c b/src/superio/ite/it8528e/superio.c new file mode 100644 index 0000000..72e2870 --- /dev/null +++ b/src/superio/ite/it8528e/superio.c @@ -0,0 +1,73 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2006 Uwe Hermann uwe@hermann-uwe.de + * Copyright (C) 2007 Philipp Degler pdegler@rumms.uni-mannheim.de + * Copyright (C) 2017 Gergely Kiss mail.gery@gmail.com + * Copyright (C) 2019 9Elements GmbH patrick.rudolph@9elements.com + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <device/device.h> +#include <device/pnp.h> +#include <arch/io.h> +#include <stdlib.h> +#include <superio/conf_mode.h> + +#include "it8528e.h" + +static void it8528e_init(struct device *dev) +{ +} + +static struct device_operations ops = { + .read_resources = pnp_read_resources, + .set_resources = pnp_set_resources, + .enable_resources = pnp_enable_resources, + .enable = pnp_alt_enable, + .init = it8528e_init, + .ops_pnp_mode = &pnp_conf_mode_870155_aa, +}; + +static struct pnp_info pnp_dev_info[] = { + { NULL, IT8528E_SP1, PNP_IO0 | PNP_IRQ0, 0x0ff8, }, + { NULL, IT8528E_SP2, PNP_IO0 | PNP_IRQ0, 0x0ff8, }, + { NULL, IT8528E_SWUC, PNP_IO0 | PNP_IRQ0, 0xfff0, }, + { NULL, IT8528E_KBCM, PNP_IRQ0, }, + /* Documentation: Program io0 = 0x60 and io1 = 0x64 */ + { NULL, IT8528E_KBCK, PNP_IO0 | PNP_IO1 | PNP_IRQ0, 0x07ff, 0x07ff, }, + { NULL, IT8528E_IR, PNP_IO0 | PNP_IRQ0, 0xfff8, }, + { NULL, IT8528E_SMFI, PNP_IO0 | PNP_IRQ0, 0xfff0, }, + /* Documentation: Program io0 = 0x70 and io1 = 0x272 */ + { NULL, IT8528E_RTCT, PNP_IO0 | PNP_IO1 | PNP_IO2 | PNP_IO3 | PNP_IRQ0, + 0xfffe, 0xfffe, 0xfffe, 0xfffe}, + /* Documentation: Program io0 = 0x62 and io1 = 0x66 */ + { NULL, IT8528E_PMC1, PNP_IO0 | PNP_IO1 | PNP_IRQ0, 0x07ff, 0x07ff }, + { NULL, IT8528E_PMC2, PNP_IO0 | PNP_IO1 | PNP_IO2 | PNP_IRQ0, 0x07fc, + 0x07fc, 0xfff0 }, + /* Documentation is unclear if PMC3-5 have LPC I/O decoding support */ + { NULL, IT8528E_PMC3, PNP_IO0 | PNP_IO1 | PNP_IRQ0, 0x07ff, 0x07ff }, + { NULL, IT8528E_PMC4, PNP_IO0 | PNP_IO1 | PNP_IRQ0, 0x07ff, 0x07ff }, + { NULL, IT8528E_PMC5, PNP_IO0 | PNP_IO1 | PNP_IRQ0, 0x07ff, 0x07ff }, + { NULL, IT8528E_SSPI, PNP_IO0 | PNP_IRQ0, 0xfff8 }, + { NULL, IT8528E_PECI, PNP_IO0, 0xfff8 }, +}; + +static void enable_dev(struct device *dev) +{ + pnp_enable_devices(dev, &ops, ARRAY_SIZE(pnp_dev_info), pnp_dev_info); +} + +struct chip_operations superio_ite_it8528e_ops = { + CHIP_NAME("ITE IT8528E Super I/O") + .enable_dev = enable_dev, +};
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30957 )
Change subject: superio/ite: Add it8528e ......................................................................
Patch Set 7:
(3 comments)
https://review.coreboot.org/c/coreboot/+/30957/6/src/superio/ite/it8528e/sup... File src/superio/ite/it8528e/superio.c:
https://review.coreboot.org/c/coreboot/+/30957/6/src/superio/ite/it8528e/sup... PS6, Line 44: 0xfff0 0xffe0
https://review.coreboot.org/c/coreboot/+/30957/6/src/superio/ite/it8528e/sup... PS6, Line 60: 0x07ff, 0x07ff this differes from PMC1-4: 0x0fff, 0x0fff
https://review.coreboot.org/c/coreboot/+/30957/6/src/superio/ite/it8528e/sup... PS6, Line 61: 0xfff8 should be 0x0ffc according to datasheet
-
build bot (Jenkins) (Code Review) -
Felix Held (Code Review)
-
Michael Niewöhner (Code Review)
-
Nico Huber (Code Review)
-
Patrick Rudolph (Code Review)
-
Paul Menzel (Code Review)