Change in coreboot[master]: {soc,drivers}: Enable ISH 'firmware-name' device property on TGL via ...
Hello Jes Klinke,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/39475
to review the following change.
Change subject: {soc,drivers}: Enable ISH 'firmware-name' device property on TGL via ACPI _DSD table ......................................................................
{soc,drivers}: Enable ISH 'firmware-name' device property on TGL via ACPI _DSD table
Upstream of non-mainboard parts of commit c88f7d29565a5f24603bc6cf235d8f3d7d1caf12.
Change-Id: I082c5bdd83a9d9e1a23dbc38b74e5f9752c98c99 Signed-off-by: Hu, Hebo hebo.hu@intel.com --- M src/drivers/intel/ish/Kconfig M src/drivers/intel/ish/ish.c M src/include/device/pci_ids.h A src/soc/intel/tigerlake/acpi/ish.asl M src/soc/intel/tigerlake/acpi/southbridge.asl 5 files changed, 28 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/39475/1
diff --git a/src/drivers/intel/ish/Kconfig b/src/drivers/intel/ish/Kconfig index 635864e..a2828d1 100644 --- a/src/drivers/intel/ish/Kconfig +++ b/src/drivers/intel/ish/Kconfig @@ -1,5 +1,6 @@ config DRIVERS_INTEL_ISH bool + default n help When enabled, chip driver/intel/ish will publish information to the SSDT _DSD table for the ISH device. diff --git a/src/drivers/intel/ish/ish.c b/src/drivers/intel/ish/ish.c index e9d5ae96..d542bd3 100644 --- a/src/drivers/intel/ish/ish.c +++ b/src/drivers/intel/ish/ish.c @@ -65,6 +65,7 @@ static const unsigned short pci_device_ids[] = { PCI_DEVICE_ID_INTEL_CNL_ISHB, PCI_DEVICE_ID_INTEL_CML_ISHB, + PCI_DEVICE_ID_INTEL_TGL_ISHB, 0 };
diff --git a/src/include/device/pci_ids.h b/src/include/device/pci_ids.h index ccbfe40..3da326b 100644 --- a/src/include/device/pci_ids.h +++ b/src/include/device/pci_ids.h @@ -2105,6 +2105,7 @@ #define PCI_DEVICE_ID_INTEL_82439TX 0x7100 #define PCI_DEVICE_ID_INTEL_CNL_ISHB 0x9dfc #define PCI_DEVICE_ID_INTEL_CML_ISHB 0x02fc +#define PCI_DEVICE_ID_INTEL_TGL_ISHB 0xa0fc
/* Intel 82371FB (PIIX) */ #define PCI_DEVICE_ID_INTEL_82371FB_ISA 0x122e diff --git a/src/soc/intel/tigerlake/acpi/ish.asl b/src/soc/intel/tigerlake/acpi/ish.asl new file mode 100644 index 0000000..186a147 --- /dev/null +++ b/src/soc/intel/tigerlake/acpi/ish.asl @@ -0,0 +1,22 @@ +/* + * This file is part of the coreboot project. + * + * Copyright 2019 Google LLC. + * + * 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. + */ + +/* Intel Integrated Sensor Hub Controller 0:12.0 */ + +Device (ISHB) +{ + Name (_ADR, 0x00120000) + Name (_DDN, "Integrated Sensor Hub Controller") +} diff --git a/src/soc/intel/tigerlake/acpi/southbridge.asl b/src/soc/intel/tigerlake/acpi/southbridge.asl index 8593d07..8e17abb 100644 --- a/src/soc/intel/tigerlake/acpi/southbridge.asl +++ b/src/soc/intel/tigerlake/acpi/southbridge.asl @@ -46,6 +46,9 @@ /* SMBus 0:1f.4 */ #include "smbus.asl"
+/* ISH 0:12.0 */ +#include "ish.asl" + /* USB XHCI 0:14.0 */ #include "xhci.asl"
Hello Jes Klinke, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39475
to look at the new patch set (#2).
Change subject: {soc,drivers}: Enable ISH 'firmware-name' device property on TGL via ACPI _DSD table ......................................................................
{soc,drivers}: Enable ISH 'firmware-name' device property on TGL via ACPI _DSD table
Upstream of non-mainboard parts of commit c88f7d29565a5f24603bc6cf235d8f3d7d1caf12.
Change-Id: I082c5bdd83a9d9e1a23dbc38b74e5f9752c98c99 Signed-off-by: Hu, Hebo hebo.hu@intel.com --- M src/drivers/intel/ish/Kconfig M src/drivers/intel/ish/ish.c M src/include/device/pci_ids.h A src/soc/intel/tigerlake/acpi/ish.asl M src/soc/intel/tigerlake/acpi/southbridge.asl 5 files changed, 28 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/39475/2
Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39475 )
Change subject: {soc,drivers}: Enable ISH 'firmware-name' device property on TGL via ACPI _DSD table ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39475/2/src/soc/intel/tigerlake/acp... File src/soc/intel/tigerlake/acpi/ish.asl:
https://review.coreboot.org/c/coreboot/+/39475/2/src/soc/intel/tigerlake/acp... PS2, Line 4: 2019 2020
Hello build bot (Jenkins), Caveh Jalali, Nick Vaccaro, Jes Klinke, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39475
to look at the new patch set (#3).
Change subject: {soc,drivers}: Enable ISH 'firmware-name' device property on TGL via ACPI _DSD table ......................................................................
{soc,drivers}: Enable ISH 'firmware-name' device property on TGL via ACPI _DSD table
Upstream of non-mainboard parts of commit c88f7d29565a5f24603bc6cf235d8f3d7d1caf12.
Change-Id: I082c5bdd83a9d9e1a23dbc38b74e5f9752c98c99 Signed-off-by: Hu, Hebo hebo.hu@intel.com --- M src/drivers/intel/ish/Kconfig M src/drivers/intel/ish/ish.c M src/include/device/pci_ids.h A src/soc/intel/tigerlake/acpi/ish.asl M src/soc/intel/tigerlake/acpi/southbridge.asl 5 files changed, 28 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/39475/3
Jes Klinke has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39475 )
Change subject: {soc,drivers}: Enable ISH 'firmware-name' device property on TGL via ACPI _DSD table ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39475/2/src/soc/intel/tigerlake/acp... File src/soc/intel/tigerlake/acpi/ish.asl:
https://review.coreboot.org/c/coreboot/+/39475/2/src/soc/intel/tigerlake/acp... PS2, Line 4: 2019
2020
Can we really claim 2020 copyright, if this was originally submitted in 2019, and not actually modified in 2020? I guess copyright counts from first "publication" so if the original repo is not publicly available, we may.
Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39475 )
Change subject: {soc,drivers}: Enable ISH 'firmware-name' device property on TGL via ACPI _DSD table ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39475/2/src/soc/intel/tigerlake/acp... File src/soc/intel/tigerlake/acpi/ish.asl:
https://review.coreboot.org/c/coreboot/+/39475/2/src/soc/intel/tigerlake/acp... PS2, Line 4: 2019
Can we really claim 2020 copyright, if this was originally submitted in 2019, and not actually modif […]
Good point, not sure.
Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39475 )
Change subject: {soc,drivers}: Enable ISH 'firmware-name' device property on TGL via ACPI _DSD table ......................................................................
Patch Set 3:
the original patch also updated some device trees. is that in the works as a separate patch?
Jes Klinke has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39475 )
Change subject: {soc,drivers}: Enable ISH 'firmware-name' device property on TGL via ACPI _DSD table ......................................................................
Patch Set 3:
(1 comment)
Please take another look
https://review.coreboot.org/c/coreboot/+/39475/2/src/soc/intel/tigerlake/acp... File src/soc/intel/tigerlake/acpi/ish.asl:
https://review.coreboot.org/c/coreboot/+/39475/2/src/soc/intel/tigerlake/acp... PS2, Line 4: 2019
Can we really claim 2020 copyright, if this was originally submitted in 2019, and not actually modif […]
After offline discussion with Nick, we have agreed that since this is a pure cherry-pick, the safe thing is to not update the copyright year.
Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39475 )
Change subject: {soc,drivers}: Enable ISH 'firmware-name' device property on TGL via ACPI _DSD table ......................................................................
Patch Set 3: Code-Review+1
Jes Klinke has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39475 )
Change subject: {soc,drivers}: Enable ISH 'firmware-name' device property on TGL via ACPI _DSD table ......................................................................
Patch Set 3:
Patch Set 3:
the original patch also updated some device trees. is that in the works as a separate patch?
Yes, that is now available here: https://review.coreboot.org/c/coreboot/+/39482
Jes Klinke has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39475 )
Change subject: {soc,drivers}: Enable ISH 'firmware-name' device property on TGL via ACPI _DSD table ......................................................................
Patch Set 3: Code-Review-1
I have become aware that Intel engineers are also pushing this upstream, see https://review.coreboot.org/c/coreboot/+/39478 https://review.coreboot.org/c/coreboot/+/39480 https://review.coreboot.org/c/coreboot/+/39481
Jes Klinke has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/39475 )
Change subject: {soc,drivers}: Enable ISH 'firmware-name' device property on TGL via ACPI _DSD table ......................................................................
Abandoned
Functionality already implemented
-
Caveh Jalali (Code Review) -
Jes Klinke (Code Review)
-
Nick Vaccaro (Code Review)