Furquan Shaikh has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/40515 )
Change subject: ec/google/chromeec: Add i2c_tunnel device under Chrome EC ......................................................................
ec/google/chromeec: Add i2c_tunnel device under Chrome EC
This change enables support for generating ACPI nodes for I2C tunnel for any device that is sitting behind the Chrome EC. It accepts a config "remote_bus" which allows mainboard to configure the id of the remote bus that is being tunneled.
BUG=b:154290952 BRANCH=None TEST=Verified that SSDT node for I2C tunnel behind Chrome EC is generated correctly.
Signed-off-by: Furquan Shaikh furquan@google.com Change-Id: Icfc0ec3725d7f1d20bcb5cb43a0a23aac72bf4eb --- M src/ec/google/chromeec/Kconfig M src/ec/google/chromeec/Makefile.inc A src/ec/google/chromeec/i2c_tunnel/Kconfig A src/ec/google/chromeec/i2c_tunnel/Makefile.inc A src/ec/google/chromeec/i2c_tunnel/chip.h A src/ec/google/chromeec/i2c_tunnel/i2c_tunnel.c 6 files changed, 108 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/40515/1
diff --git a/src/ec/google/chromeec/Kconfig b/src/ec/google/chromeec/Kconfig index 554677c..1b269ec 100644 --- a/src/ec/google/chromeec/Kconfig +++ b/src/ec/google/chromeec/Kconfig @@ -1,3 +1,6 @@ + +source "src/ec/google/chromeec/*/Kconfig" + config EC_GOOGLE_CHROMEEC bool select EC_SUPPORTS_DPTF_TEVT diff --git a/src/ec/google/chromeec/Makefile.inc b/src/ec/google/chromeec/Makefile.inc index 590b131..b11f5c8 100644 --- a/src/ec/google/chromeec/Makefile.inc +++ b/src/ec/google/chromeec/Makefile.inc @@ -1,5 +1,7 @@ ifeq ($(CONFIG_EC_GOOGLE_CHROMEEC),y)
+subdirs-y += i2c_tunnel + bootblock-$(CONFIG_EC_GOOGLE_CHROMEEC_BOARDID) += ec_boardid.c verstage-$(CONFIG_EC_GOOGLE_CHROMEEC_BOARDID) += ec_boardid.c romstage-$(CONFIG_EC_GOOGLE_CHROMEEC_BOARDID) += ec_boardid.c diff --git a/src/ec/google/chromeec/i2c_tunnel/Kconfig b/src/ec/google/chromeec/i2c_tunnel/Kconfig new file mode 100644 index 0000000..c90f0d1 --- /dev/null +++ b/src/ec/google/chromeec/i2c_tunnel/Kconfig @@ -0,0 +1,6 @@ +config EC_GOOGLE_CHROMEEC_I2C_TUNNEL + bool + depends on EC_GOOGLE_CHROMEEC && HAVE_ACPI_TABLES + help + This enables the crosec i2c tunnel driver that is required to fill the + SSDT nodes for i2c tunnel used by the mainboard. diff --git a/src/ec/google/chromeec/i2c_tunnel/Makefile.inc b/src/ec/google/chromeec/i2c_tunnel/Makefile.inc new file mode 100644 index 0000000..85e0fba --- /dev/null +++ b/src/ec/google/chromeec/i2c_tunnel/Makefile.inc @@ -0,0 +1 @@ +ramstage-$(CONFIG_EC_GOOGLE_CHROMEEC_I2C_TUNNEL) += i2c_tunnel.c diff --git a/src/ec/google/chromeec/i2c_tunnel/chip.h b/src/ec/google/chromeec/i2c_tunnel/chip.h new file mode 100644 index 0000000..070a59a --- /dev/null +++ b/src/ec/google/chromeec/i2c_tunnel/chip.h @@ -0,0 +1,21 @@ +/* + * This file is part of the coreboot project. + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ + +#ifndef __EC_GOOGLE_CHROMEEC_I2C_TUNNEL__ +#define __EC_GOOGLE_CHROMEEC_I2C_TUNNEL__ + +#include <arch/acpi_device.h> + +struct ec_google_chromeec_i2c_tunnel_config { + /* ACPI device name */ + const char *name; + /* ACPI _UID */ + int uid; + /* EC bus number we tunnel to on the other side. */ + int remote_bus; +}; + +#endif /* __EC_GOOGLE_CHROMEEC_I2C_TUNNEL__ */ diff --git a/src/ec/google/chromeec/i2c_tunnel/i2c_tunnel.c b/src/ec/google/chromeec/i2c_tunnel/i2c_tunnel.c new file mode 100644 index 0000000..764f589 --- /dev/null +++ b/src/ec/google/chromeec/i2c_tunnel/i2c_tunnel.c @@ -0,0 +1,75 @@ +/* + * This file is part of the coreboot project. + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ + +#include <arch/acpi_device.h> +#include <arch/acpigen.h> +#include <console/console.h> +#include <device/device.h> +#include <device/path.h> +#include <string.h> +#include "chip.h" + +#define CROS_EC_I2C_TUNNEL_HID "GOOG0012" +#define CROS_EC_I2C_TUNNEL_DDN "Cros EC I2C Tunnel" + +static void crosec_i2c_tunnel_fill_ssdt(struct device *dev) +{ + const char *scope = acpi_device_scope(dev); + struct ec_google_chromeec_i2c_tunnel_config *cfg = dev->chip_info; + struct acpi_dp *dsd; + + if (!dev->enabled || !scope || !cfg) + return; + + acpigen_write_scope(scope); + + acpigen_write_device(acpi_device_name(dev)); + acpigen_write_name_string("_HID", CROS_EC_I2C_TUNNEL_HID); + acpigen_write_name_integer("_UID", cfg->uid); + acpigen_write_name_string("_DDN", CROS_EC_I2C_TUNNEL_DDN); + acpigen_write_STA(acpi_device_status(dev)); + + dsd = acpi_dp_new_table("_DSD"); + acpi_dp_add_integer(dsd, "google,remote-bus", cfg->remote_bus); + acpi_dp_write(dsd); + + acpigen_pop_len(); /* Device */ + acpigen_pop_len(); /* Scope */ + + printk(BIOS_INFO, "%s: %s at %s\n", acpi_device_path(dev), CROS_EC_I2C_TUNNEL_DDN, + dev_path(dev)); +} + +static const char *crosec_i2c_tunnel_acpi_name(const struct device *dev) +{ + struct ec_google_chromeec_i2c_tunnel_config *cfg = dev->chip_info; + static char name[5]; + + if (cfg->name) + return cfg->name; + + snprintf(name, sizeof(name), "IT%02.2X", dev->path.generic.id); + name[4] = '\0'; + return name; +} + +static struct device_operations crosec_i2c_tunnel_ops = { + .read_resources = noop_read_resources, + .set_resources = noop_set_resources, + .acpi_name = crosec_i2c_tunnel_acpi_name, + .acpi_fill_ssdt_generator = crosec_i2c_tunnel_fill_ssdt, + .scan_bus = scan_static_bus, +}; + +static void crosec_i2c_tunnel_enable(struct device *dev) +{ + dev->ops = &crosec_i2c_tunnel_ops; +} + +struct chip_operations ec_google_chromeec_i2c_tunnel_ops = { + CHIP_NAME("CrosEC I2C Tunnel Device") + .enable_dev = crosec_i2c_tunnel_enable +};
Hello build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40515
to look at the new patch set (#2).
Change subject: ec/google/chromeec: Add driver for i2c_tunnel device under Chrome EC ......................................................................
ec/google/chromeec: Add driver for i2c_tunnel device under Chrome EC
This change enables support for generating ACPI nodes for I2C tunnel for any device that is sitting behind the Chrome EC. It accepts a config "remote_bus" which allows mainboard to configure the id of the remote bus that is being tunneled.
BUG=b:154290952 BRANCH=None TEST=Verified that SSDT node for I2C tunnel behind Chrome EC is generated correctly.
Signed-off-by: Furquan Shaikh furquan@google.com Change-Id: Icfc0ec3725d7f1d20bcb5cb43a0a23aac72bf4eb --- M src/ec/google/chromeec/Kconfig M src/ec/google/chromeec/Makefile.inc A src/ec/google/chromeec/i2c_tunnel/Kconfig A src/ec/google/chromeec/i2c_tunnel/Makefile.inc A src/ec/google/chromeec/i2c_tunnel/chip.h A src/ec/google/chromeec/i2c_tunnel/i2c_tunnel.c 6 files changed, 110 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/40515/2
Hello build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40515
to look at the new patch set (#3).
Change subject: ec/google/chromeec: Add driver for i2c_tunnel device under Chrome EC ......................................................................
ec/google/chromeec: Add driver for i2c_tunnel device under Chrome EC
This change enables support for generating ACPI nodes for I2C tunnel for any device that is sitting behind the Chrome EC. It accepts a config "remote_bus" which allows mainboard to configure the id of the remote bus that is being tunneled.
BUG=b:154290952 BRANCH=None TEST=Verified that SSDT node for I2C tunnel behind Chrome EC is generated correctly.
Signed-off-by: Furquan Shaikh furquan@google.com Change-Id: Icfc0ec3725d7f1d20bcb5cb43a0a23aac72bf4eb --- M src/ec/google/chromeec/Kconfig M src/ec/google/chromeec/Makefile.inc A src/ec/google/chromeec/i2c_tunnel/Kconfig A src/ec/google/chromeec/i2c_tunnel/Makefile.inc A src/ec/google/chromeec/i2c_tunnel/chip.h A src/ec/google/chromeec/i2c_tunnel/i2c_tunnel.c 6 files changed, 109 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/40515/3
Hello build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40515
to look at the new patch set (#4).
Change subject: ec/google/chromeec: Add driver for i2c_tunnel device under Chrome EC ......................................................................
ec/google/chromeec: Add driver for i2c_tunnel device under Chrome EC
This change enables support for generating ACPI nodes for I2C tunnel for any device that is sitting behind the Chrome EC. It accepts a config "remote_bus" which allows mainboard to configure the id of the remote bus that is being tunneled.
BUG=b:154290952 BRANCH=None TEST=Verified that SSDT node for I2C tunnel behind Chrome EC is generated correctly.
Signed-off-by: Furquan Shaikh furquan@google.com Change-Id: Icfc0ec3725d7f1d20bcb5cb43a0a23aac72bf4eb --- M src/ec/google/chromeec/Kconfig M src/ec/google/chromeec/Makefile.inc A src/ec/google/chromeec/i2c_tunnel/Kconfig A src/ec/google/chromeec/i2c_tunnel/Makefile.inc A src/ec/google/chromeec/i2c_tunnel/chip.h A src/ec/google/chromeec/i2c_tunnel/i2c_tunnel.c 6 files changed, 108 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/40515/4
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40515 )
Change subject: ec/google/chromeec: Add driver for i2c_tunnel device under Chrome EC ......................................................................
Patch Set 4: Code-Review+1
(6 comments)
https://review.coreboot.org/c/coreboot/+/40515/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40515/4//COMMIT_MSG@9 PS4, Line 9: This change Normally redundant phrasing in commit messages.
https://review.coreboot.org/c/coreboot/+/40515/4//COMMIT_MSG@10 PS4, Line 10: any device that is sitting behind the Chrome EC. It accepts a config With HID GOOG0012?
https://review.coreboot.org/c/coreboot/+/40515/4/src/ec/google/chromeec/i2c_... File src/ec/google/chromeec/i2c_tunnel/Kconfig:
https://review.coreboot.org/c/coreboot/+/40515/4/src/ec/google/chromeec/i2c_... PS4, Line 6: SSDT nodes for I2C tunnel used by the mainboard. for *the* I2C ?
https://review.coreboot.org/c/coreboot/+/40515/4/src/ec/google/chromeec/i2c_... File src/ec/google/chromeec/i2c_tunnel/chip.h:
https://review.coreboot.org/c/coreboot/+/40515/4/src/ec/google/chromeec/i2c_... PS4, Line 3: * No blank line? Angel?
https://review.coreboot.org/c/coreboot/+/40515/4/src/ec/google/chromeec/i2c_... PS4, Line 14: int unsigned int?
https://review.coreboot.org/c/coreboot/+/40515/4/src/ec/google/chromeec/i2c_... PS4, Line 16: int unsigned int?
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40515 )
Change subject: ec/google/chromeec: Add driver for i2c_tunnel device under Chrome EC ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40515/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40515/4//COMMIT_MSG@9 PS4, Line 9: This change
Normally redundant phrasing in commit messages.
Do you recommend using "Enables support for ..." instead?
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40515 )
Change subject: ec/google/chromeec: Add driver for i2c_tunnel device under Chrome EC ......................................................................
Patch Set 5:
(4 comments)
https://review.coreboot.org/c/coreboot/+/40515/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40515/4//COMMIT_MSG@10 PS4, Line 10: any device that is sitting behind the Chrome EC. It accepts a config
With HID GOOG0012?
Done
https://review.coreboot.org/c/coreboot/+/40515/4/src/ec/google/chromeec/i2c_... File src/ec/google/chromeec/i2c_tunnel/chip.h:
https://review.coreboot.org/c/coreboot/+/40515/4/src/ec/google/chromeec/i2c_... PS4, Line 3: *
No blank line? Angel?
Done
https://review.coreboot.org/c/coreboot/+/40515/4/src/ec/google/chromeec/i2c_... PS4, Line 14: int
unsigned int?
Done
https://review.coreboot.org/c/coreboot/+/40515/4/src/ec/google/chromeec/i2c_... PS4, Line 16: int
unsigned int?
Done
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Paul Menzel, Tim Wawrzynczak, Aaron Durbin, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40515
to look at the new patch set (#6).
Change subject: ec/google/chromeec: Add driver for i2c_tunnel device under Chrome EC ......................................................................
ec/google/chromeec: Add driver for i2c_tunnel device under Chrome EC
This change enables support for generating ACPI nodes for I2C tunnel for any GOOG0012 device that is sitting behind the Chrome EC. It accepts a config "remote_bus" which allows mainboard to configure the id of the remote bus that is being tunneled.
BUG=b:154290952 BRANCH=None TEST=Verified that SSDT node for I2C tunnel behind Chrome EC is generated correctly.
Signed-off-by: Furquan Shaikh furquan@google.com Change-Id: Icfc0ec3725d7f1d20bcb5cb43a0a23aac72bf4eb --- M src/ec/google/chromeec/Kconfig M src/ec/google/chromeec/Makefile.inc A src/ec/google/chromeec/i2c_tunnel/Kconfig A src/ec/google/chromeec/i2c_tunnel/Makefile.inc A src/ec/google/chromeec/i2c_tunnel/chip.h A src/ec/google/chromeec/i2c_tunnel/i2c_tunnel.c 6 files changed, 102 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/40515/6
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40515 )
Change subject: ec/google/chromeec: Add driver for i2c_tunnel device under Chrome EC ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40515/4/src/ec/google/chromeec/i2c_... File src/ec/google/chromeec/i2c_tunnel/Kconfig:
https://review.coreboot.org/c/coreboot/+/40515/4/src/ec/google/chromeec/i2c_... PS4, Line 6: SSDT nodes for I2C tunnel used by the mainboard.
for *the* I2C ?
Done
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Paul Menzel, Tim Wawrzynczak, Aaron Durbin, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40515
to look at the new patch set (#7).
Change subject: ec/google/chromeec: Add driver for i2c_tunnel device under Chrome EC ......................................................................
ec/google/chromeec: Add driver for i2c_tunnel device under Chrome EC
This change enables support for generating ACPI nodes for I2C tunnel for any GOOG0012 device that is sitting behind the Chrome EC. It accepts a config "remote_bus" which allows mainboard to configure the id of the remote bus that is being tunneled.
BUG=b:154290952 BRANCH=None TEST=Verified that SSDT node for I2C tunnel behind Chrome EC is generated correctly.
Signed-off-by: Furquan Shaikh furquan@google.com Change-Id: Icfc0ec3725d7f1d20bcb5cb43a0a23aac72bf4eb --- M src/ec/google/chromeec/Kconfig M src/ec/google/chromeec/Makefile.inc A src/ec/google/chromeec/i2c_tunnel/Kconfig A src/ec/google/chromeec/i2c_tunnel/Makefile.inc A src/ec/google/chromeec/i2c_tunnel/chip.h A src/ec/google/chromeec/i2c_tunnel/i2c_tunnel.c 6 files changed, 102 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/40515/7
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40515 )
Change subject: ec/google/chromeec: Add driver for i2c_tunnel device under Chrome EC ......................................................................
Patch Set 7:
the build failure is just jenkins, seems like it's been really flaky today...
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40515 )
Change subject: ec/google/chromeec: Add driver for i2c_tunnel device under Chrome EC ......................................................................
Patch Set 7:
(1 comment)
Patch Set 7:
the build failure is just jenkins, seems like it's been really flaky today...
Yeah. I just noticed one issue with acpi_fill_ssdt() name being used. I will fix that and push a change. Hopefully, jenkins would be happier by then.
https://review.coreboot.org/c/coreboot/+/40515/7/src/ec/google/chromeec/i2c_... File src/ec/google/chromeec/i2c_tunnel/i2c_tunnel.c:
https://review.coreboot.org/c/coreboot/+/40515/7/src/ec/google/chromeec/i2c_... PS7, Line 59: acpi_fill_ssdt_generator This should be acpi_fill_ssdt since it was recently changed.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40515 )
Change subject: ec/google/chromeec: Add driver for i2c_tunnel device under Chrome EC ......................................................................
Patch Set 7:
(2 comments)
https://review.coreboot.org/c/coreboot/+/40515/7/src/ec/google/chromeec/i2c_... File src/ec/google/chromeec/i2c_tunnel/i2c_tunnel.c:
https://review.coreboot.org/c/coreboot/+/40515/7/src/ec/google/chromeec/i2c_... PS7, Line 39: CROS_EC_I2C_TUNNEL_DDN WDYT about using the acpi_device_name ?
https://review.coreboot.org/c/coreboot/+/40515/7/src/ec/google/chromeec/i2c_... PS7, Line 51: IT%02.2X Just a suggestion, maybe TUNx? Do you think we'd ever use more than 10 tunnels per EC?
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40515 )
Change subject: ec/google/chromeec: Add driver for i2c_tunnel device under Chrome EC ......................................................................
Patch Set 7:
(2 comments)
https://review.coreboot.org/c/coreboot/+/40515/7/src/ec/google/chromeec/i2c_... File src/ec/google/chromeec/i2c_tunnel/i2c_tunnel.c:
https://review.coreboot.org/c/coreboot/+/40515/7/src/ec/google/chromeec/i2c_... PS7, Line 39: CROS_EC_I2C_TUNNEL_DDN
WDYT about using the acpi_device_name ?
I used CROS_EC_I2C_TUNNEL_DDN because other drivers report the device being added to SSDT in a similar way using _DDN.
https://review.coreboot.org/c/coreboot/+/40515/7/src/ec/google/chromeec/i2c_... PS7, Line 51: IT%02.2X
Just a suggestion, maybe TUNx? Do you think we'd ever use more than 10 tunnels per EC?
Sure, I can update that.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40515 )
Change subject: ec/google/chromeec: Add driver for i2c_tunnel device under Chrome EC ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40515/7/src/ec/google/chromeec/i2c_... File src/ec/google/chromeec/i2c_tunnel/i2c_tunnel.c:
https://review.coreboot.org/c/coreboot/+/40515/7/src/ec/google/chromeec/i2c_... PS7, Line 39: CROS_EC_I2C_TUNNEL_DDN
I used CROS_EC_I2C_TUNNEL_DDN because other drivers report the device being added to SSDT in a simil […]
oops I misread it as _HID anyways.
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Tim Wawrzynczak, Paul Menzel, Aaron Durbin, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40515
to look at the new patch set (#8).
Change subject: ec/google/chromeec: Add driver for i2c_tunnel device under Chrome EC ......................................................................
ec/google/chromeec: Add driver for i2c_tunnel device under Chrome EC
This change enables support for generating ACPI nodes for I2C tunnel for any GOOG0012 device that is sitting behind the Chrome EC. It accepts a config "remote_bus" which allows mainboard to configure the id of the remote bus that is being tunneled.
BUG=b:154290952 BRANCH=None TEST=Verified that SSDT node for I2C tunnel behind Chrome EC is generated correctly.
Signed-off-by: Furquan Shaikh furquan@google.com Change-Id: Icfc0ec3725d7f1d20bcb5cb43a0a23aac72bf4eb --- M src/ec/google/chromeec/Kconfig M src/ec/google/chromeec/Makefile.inc A src/ec/google/chromeec/i2c_tunnel/Kconfig A src/ec/google/chromeec/i2c_tunnel/Makefile.inc A src/ec/google/chromeec/i2c_tunnel/chip.h A src/ec/google/chromeec/i2c_tunnel/i2c_tunnel.c 6 files changed, 102 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/40515/8
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40515 )
Change subject: ec/google/chromeec: Add driver for i2c_tunnel device under Chrome EC ......................................................................
Patch Set 8:
(2 comments)
https://review.coreboot.org/c/coreboot/+/40515/7/src/ec/google/chromeec/i2c_... File src/ec/google/chromeec/i2c_tunnel/i2c_tunnel.c:
https://review.coreboot.org/c/coreboot/+/40515/7/src/ec/google/chromeec/i2c_... PS7, Line 51: IT%02.2X
Sure, I can update that.
Done
https://review.coreboot.org/c/coreboot/+/40515/7/src/ec/google/chromeec/i2c_... PS7, Line 59: acpi_fill_ssdt_generator
This should be acpi_fill_ssdt since it was recently changed.
Done
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40515 )
Change subject: ec/google/chromeec: Add driver for i2c_tunnel device under Chrome EC ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40515/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40515/4//COMMIT_MSG@9 PS4, Line 9: This change
Do you recommend using "Enables support for ... […]
Any update?
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40515 )
Change subject: ec/google/chromeec: Add driver for i2c_tunnel device under Chrome EC ......................................................................
Patch Set 8: Code-Review+2
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40515 )
Change subject: ec/google/chromeec: Add driver for i2c_tunnel device under Chrome EC ......................................................................
Patch Set 8: Code-Review+2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40515 )
Change subject: ec/google/chromeec: Add driver for i2c_tunnel device under Chrome EC ......................................................................
Patch Set 9:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40515/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40515/4//COMMIT_MSG@9 PS4, Line 9: This change
Any update?
Haven't heard back anything, so marking resolved.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40515 )
Change subject: ec/google/chromeec: Add driver for i2c_tunnel device under Chrome EC ......................................................................
Patch Set 10:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40515/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40515/4//COMMIT_MSG@9 PS4, Line 9: This change
Haven't heard back anything, so marking resolved.
Done
Furquan Shaikh has submitted this change. ( https://review.coreboot.org/c/coreboot/+/40515 )
Change subject: ec/google/chromeec: Add driver for i2c_tunnel device under Chrome EC ......................................................................
ec/google/chromeec: Add driver for i2c_tunnel device under Chrome EC
This change enables support for generating ACPI nodes for I2C tunnel for any GOOG0012 device that is sitting behind the Chrome EC. It accepts a config "remote_bus" which allows mainboard to configure the id of the remote bus that is being tunneled.
BUG=b:154290952 BRANCH=None TEST=Verified that SSDT node for I2C tunnel behind Chrome EC is generated correctly.
Signed-off-by: Furquan Shaikh furquan@google.com Change-Id: Icfc0ec3725d7f1d20bcb5cb43a0a23aac72bf4eb Reviewed-on: https://review.coreboot.org/c/coreboot/+/40515 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Aaron Durbin adurbin@chromium.org Reviewed-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/ec/google/chromeec/Kconfig M src/ec/google/chromeec/Makefile.inc A src/ec/google/chromeec/i2c_tunnel/Kconfig A src/ec/google/chromeec/i2c_tunnel/Makefile.inc A src/ec/google/chromeec/i2c_tunnel/chip.h A src/ec/google/chromeec/i2c_tunnel/i2c_tunnel.c 6 files changed, 102 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Aaron Durbin: Looks good to me, approved Tim Wawrzynczak: Looks good to me, approved
diff --git a/src/ec/google/chromeec/Kconfig b/src/ec/google/chromeec/Kconfig index 554677c..4615878 100644 --- a/src/ec/google/chromeec/Kconfig +++ b/src/ec/google/chromeec/Kconfig @@ -196,3 +196,9 @@ help Enable support for Chrome OS mode switches provided by the Chrome OS EC. + +if EC_GOOGLE_CHROMEEC + +source "src/ec/google/chromeec/*/Kconfig" + +endif diff --git a/src/ec/google/chromeec/Makefile.inc b/src/ec/google/chromeec/Makefile.inc index 590b131..b11f5c8 100644 --- a/src/ec/google/chromeec/Makefile.inc +++ b/src/ec/google/chromeec/Makefile.inc @@ -1,5 +1,7 @@ ifeq ($(CONFIG_EC_GOOGLE_CHROMEEC),y)
+subdirs-y += i2c_tunnel + bootblock-$(CONFIG_EC_GOOGLE_CHROMEEC_BOARDID) += ec_boardid.c verstage-$(CONFIG_EC_GOOGLE_CHROMEEC_BOARDID) += ec_boardid.c romstage-$(CONFIG_EC_GOOGLE_CHROMEEC_BOARDID) += ec_boardid.c diff --git a/src/ec/google/chromeec/i2c_tunnel/Kconfig b/src/ec/google/chromeec/i2c_tunnel/Kconfig new file mode 100644 index 0000000..20169fd --- /dev/null +++ b/src/ec/google/chromeec/i2c_tunnel/Kconfig @@ -0,0 +1,6 @@ +config EC_GOOGLE_CHROMEEC_I2C_TUNNEL + bool + depends on HAVE_ACPI_TABLES + help + This enables the Cros EC I2C tunnel driver that is required to fill the + SSDT nodes for the I2C tunnel used by the mainboard. diff --git a/src/ec/google/chromeec/i2c_tunnel/Makefile.inc b/src/ec/google/chromeec/i2c_tunnel/Makefile.inc new file mode 100644 index 0000000..85e0fba --- /dev/null +++ b/src/ec/google/chromeec/i2c_tunnel/Makefile.inc @@ -0,0 +1 @@ +ramstage-$(CONFIG_EC_GOOGLE_CHROMEEC_I2C_TUNNEL) += i2c_tunnel.c diff --git a/src/ec/google/chromeec/i2c_tunnel/chip.h b/src/ec/google/chromeec/i2c_tunnel/chip.h new file mode 100644 index 0000000..01d52bd --- /dev/null +++ b/src/ec/google/chromeec/i2c_tunnel/chip.h @@ -0,0 +1,16 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* This file is part of the coreboot project. */ + +#ifndef __EC_GOOGLE_CHROMEEC_I2C_TUNNEL__ +#define __EC_GOOGLE_CHROMEEC_I2C_TUNNEL__ + +struct ec_google_chromeec_i2c_tunnel_config { + /* ACPI device name */ + const char *name; + /* ACPI _UID */ + unsigned int uid; + /* EC I2C bus number we tunnel to on the other side. */ + unsigned int remote_bus; +}; + +#endif /* __EC_GOOGLE_CHROMEEC_I2C_TUNNEL__ */ diff --git a/src/ec/google/chromeec/i2c_tunnel/i2c_tunnel.c b/src/ec/google/chromeec/i2c_tunnel/i2c_tunnel.c new file mode 100644 index 0000000..51375f8 --- /dev/null +++ b/src/ec/google/chromeec/i2c_tunnel/i2c_tunnel.c @@ -0,0 +1,71 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* This file is part of the coreboot project. */ + +#include <arch/acpi_device.h> +#include <arch/acpigen.h> +#include <console/console.h> +#include <device/device.h> +#include <device/path.h> +#include <string.h> +#include "chip.h" + +#define CROS_EC_I2C_TUNNEL_HID "GOOG0012" +#define CROS_EC_I2C_TUNNEL_DDN "Cros EC I2C Tunnel" + +static void crosec_i2c_tunnel_fill_ssdt(struct device *dev) +{ + const char *scope = acpi_device_scope(dev); + struct ec_google_chromeec_i2c_tunnel_config *cfg = dev->chip_info; + struct acpi_dp *dsd; + + if (!dev->enabled || !scope || !cfg) + return; + + acpigen_write_scope(scope); + + acpigen_write_device(acpi_device_name(dev)); + acpigen_write_name_string("_HID", CROS_EC_I2C_TUNNEL_HID); + acpigen_write_name_integer("_UID", cfg->uid); + acpigen_write_name_string("_DDN", CROS_EC_I2C_TUNNEL_DDN); + acpigen_write_STA(acpi_device_status(dev)); + + dsd = acpi_dp_new_table("_DSD"); + acpi_dp_add_integer(dsd, "google,remote-bus", cfg->remote_bus); + acpi_dp_write(dsd); + + acpigen_pop_len(); /* Device */ + acpigen_pop_len(); /* Scope */ + + printk(BIOS_INFO, "%s: %s at %s\n", acpi_device_path(dev), CROS_EC_I2C_TUNNEL_DDN, + dev_path(dev)); +} + +static const char *crosec_i2c_tunnel_acpi_name(const struct device *dev) +{ + struct ec_google_chromeec_i2c_tunnel_config *cfg = dev->chip_info; + static char name[5]; + + if (cfg->name) + return cfg->name; + + snprintf(name, sizeof(name), "TUN%X", dev->path.generic.id); + return name; +} + +static struct device_operations crosec_i2c_tunnel_ops = { + .read_resources = noop_read_resources, + .set_resources = noop_set_resources, + .acpi_name = crosec_i2c_tunnel_acpi_name, + .acpi_fill_ssdt = crosec_i2c_tunnel_fill_ssdt, + .scan_bus = scan_static_bus, +}; + +static void crosec_i2c_tunnel_enable(struct device *dev) +{ + dev->ops = &crosec_i2c_tunnel_ops; +} + +struct chip_operations ec_google_chromeec_i2c_tunnel_ops = { + CHIP_NAME("CrosEC I2C Tunnel Device") + .enable_dev = crosec_i2c_tunnel_enable +};
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40515 )
Change subject: ec/google/chromeec: Add driver for i2c_tunnel device under Chrome EC ......................................................................
Patch Set 11:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40515/11/src/ec/google/chromeec/Kco... File src/ec/google/chromeec/Kconfig:
https://review.coreboot.org/c/coreboot/+/40515/11/src/ec/google/chromeec/Kco... PS11, Line 200: if EC_GOOGLE_CHROMEEC Just a note, I really think this if statement should go inside the sourced files. This makes it look as if the files won't get sourced unless EC_GOOGLE_CHROMEEC is set, which isn't the case.
In the sourced file, there's nothing to show that it won't get run unless EC_GOOGLE_CHROMEEC is set.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40515 )
Change subject: ec/google/chromeec: Add driver for i2c_tunnel device under Chrome EC ......................................................................
Patch Set 11:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40515/11/src/ec/google/chromeec/Kco... File src/ec/google/chromeec/Kconfig:
https://review.coreboot.org/c/coreboot/+/40515/11/src/ec/google/chromeec/Kco... PS11, Line 200: if EC_GOOGLE_CHROMEEC
This makes it look as if the files won't get sourced unless EC_GOOGLE_CHROMEEC is set, which isn't the case.
Maybe I am misunderstanding the syntax here. But, my intent is to ensure that the files don't get sourced unless EC_GOOGLE_CHROMEEC is true. Are you saying that is not what this is doing?
In the sourced file, there's nothing to show that it won't get run unless EC_GOOGLE_CHROMEEC is set.
I put it here because it relieves all the sub-Kconfigs from adding this check. I see that it is also being used in other places like soc/intel/common, soc/amd/common.
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40515 )
Change subject: ec/google/chromeec: Add driver for i2c_tunnel device under Chrome EC ......................................................................
Patch Set 11:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40515/11/src/ec/google/chromeec/Kco... File src/ec/google/chromeec/Kconfig:
https://review.coreboot.org/c/coreboot/+/40515/11/src/ec/google/chromeec/Kco... PS11, Line 200: if EC_GOOGLE_CHROMEEC
This makes it look as if the files won't get sourced unless EC_GOOGLE_CHROMEEC is set, which isn't […]
The 'source' command always pulls in the associated files and cannot be made conditional
The if command behaves the same as if statements anywhere else. configs won't be used, selects inside the if statement won't be selected, etc.
However, For example, something could use a select to enable a config option that's only available inside the if statement.
I've made this same recommendation not to do this in other places, and even put a note about in the kconfig documentation. It generally isn't harmful, but it doesn't do what it looks like it does. It's similar, but not quite the same.
https://doc.coreboot.org/getting_started/kconfig.html?highlight=kconfig#sour...
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40515 )
Change subject: ec/google/chromeec: Add driver for i2c_tunnel device under Chrome EC ......................................................................
Patch Set 11:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40515/11/src/ec/google/chromeec/Kco... File src/ec/google/chromeec/Kconfig:
https://review.coreboot.org/c/coreboot/+/40515/11/src/ec/google/chromeec/Kco... PS11, Line 200: if EC_GOOGLE_CHROMEEC
The 'source' command always pulls in the associated files and cannot be made conditional […]
Done: https://review.coreboot.org/c/coreboot/+/43040
-
Aaron Durbin (Code Review) -
Furquan Shaikh (Code Review)
-
Martin Roth (Code Review)
-
Paul Menzel (Code Review)
-
Tim Wawrzynczak (Code Review)