Attention is currently required from: Tarun Tuli, Kapil Porwal, Sridhar Siricilla.
Hello Tarun Tuli, Subrata Banik,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/69571
to look at the new patch set (#3).
Change subject: soc/intel/meteorlake: Log CSE RO write protection info for MTL
......................................................................
soc/intel/meteorlake: Log CSE RO write protection info for MTL
The patch logs CSE RO's write protection information for Meteor Lake
platform. As part of write protection information, coreboot logs status
on CSE RO write protection and range. Also, logs error message if EOM
is disabled, and write protection for CSE RO is not enabled.
Port of commit abe0d810f009 ("soc/intel/alderlake: Log CSE RO write
protection info for ADL").
BUG=none
TEST=Verify the write protection details on google/rex.
Excerpt from google/rex coreboot log:
[DEBUG] ME: WP for RO is enabled : YES
[DEBUG] ME: RO write protection scope - Start=0x4000, End=0x396FFF
Signed-off-by: Kapil Porwal <kapilporwal(a)google.com>
Change-Id: Idb072a873a8b8323532799f5fc64f995c9f0a604
---
M src/soc/intel/meteorlake/me.c
1 file changed, 48 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/71/69571/3
--
To view, visit https://review.coreboot.org/c/coreboot/+/69571
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Idb072a873a8b8323532799f5fc64f995c9f0a604
Gerrit-Change-Number: 69571
Gerrit-PatchSet: 3
Gerrit-Owner: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com>
Gerrit-CC: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Tarun Tuli, Subrata Banik.
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69576 )
Change subject: soc/intel/meteorlake: Hide PMC and IOM devices
......................................................................
Patch Set 1:
(2 comments)
Commit Message:
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-163617):
https://review.coreboot.org/c/coreboot/+/69576/comment/caaa5be8_0f9247d6
PS1, Line 11:
Possible unwrapped commit description (prefer a maximum 72 chars per line)
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-163617):
https://review.coreboot.org/c/coreboot/+/69576/comment/631e72ea_caa2d344
PS1, Line 11:
Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 907c85ad48dd ("soc/intel/alderlake: Hide PMC and IOM devices")'
--
To view, visit https://review.coreboot.org/c/coreboot/+/69576
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic62172bee9120d260a3cd60770ef780cb7dce860
Gerrit-Change-Number: 69576
Gerrit-PatchSet: 1
Gerrit-Owner: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com>
Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Comment-Date: Mon, 14 Nov 2022 13:11:29 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Kapil Porwal has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/69576 )
Change subject: soc/intel/meteorlake: Hide PMC and IOM devices
......................................................................
soc/intel/meteorlake: Hide PMC and IOM devices
Hide these ACPI device so Windows does not warn about missing device
drivers.
Port of commit 907c85ad48dd (soc/intel/alderlake: Hide PMC and IOM devices).
BUG=none
TEST=Verified _STA method from ACPI tables in OS.
Signed-off-by: Kapil Porwal <kapilporwal(a)google.com>
Change-Id: Ic62172bee9120d260a3cd60770ef780cb7dce860
---
M src/soc/intel/meteorlake/acpi/tcss.asl
M src/soc/intel/meteorlake/pmc.c
2 files changed, 22 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/76/69576/1
diff --git a/src/soc/intel/meteorlake/acpi/tcss.asl b/src/soc/intel/meteorlake/acpi/tcss.asl
index c2212f0..817afdd 100644
--- a/src/soc/intel/meteorlake/acpi/tcss.asl
+++ b/src/soc/intel/meteorlake/acpi/tcss.asl
@@ -327,6 +327,8 @@
IOM_BASE_ADDR, IOM_BASE_ADDR_MAX, 0x0,
IOM_BASE_SIZE,,,)
})
+ /* Hide the device so that Windows does not complain on missing driver */
+ Name (_STA, 0xB)
}
/*
diff --git a/src/soc/intel/meteorlake/pmc.c b/src/soc/intel/meteorlake/pmc.c
index dbfc4be..bdd7a3b 100644
--- a/src/soc/intel/meteorlake/pmc.c
+++ b/src/soc/intel/meteorlake/pmc.c
@@ -103,6 +103,8 @@
acpigen_write_name_string("_HID", PMC_HID);
acpigen_write_name_string("_DDN", "Intel(R) Meteor Lake IPC Controller");
+ /* Hide the device so that Windows does not complain on missing driver */
+ acpigen_write_STA(ACPI_STATUS_DEVICE_HIDDEN_ON);
/*
* Part of the PCH's reserved 32 MB MMIO range (0xFC800000 - 0xFE7FFFFF).
--
To view, visit https://review.coreboot.org/c/coreboot/+/69576
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic62172bee9120d260a3cd60770ef780cb7dce860
Gerrit-Change-Number: 69576
Gerrit-PatchSet: 1
Gerrit-Owner: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-MessageType: newchange
Attention is currently required from: Mario Scheithauer, Werner Zeh.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69386 )
Change subject: drivers/phy/m88e1512: Provide functionality to customize LED status
......................................................................
Patch Set 10:
(1 comment)
File src/drivers/phy/m88e1512/m88e1512.c:
https://review.coreboot.org/c/coreboot/+/69386/comment/2c316b8b_ae095d0a
PS10, Line 18:
: const struct mdio_operations *mdio_ops;
: struct device *parent = dev->bus->dev;
> This is bit tidious to add to every read/write op.
> Do you think it is possible to write it:
> uint16_t (*mdio_read)(uint8_t reg_adr) = mdio_read_helper(dev);
> void (*mdio_write)(uint8_t reg_adr, uint16_t value) = mdio_write_helper(dev);
> ?
Ehm nvm the function pointer suggestion...
However having helpers is a good idea. See i2c_bus.c as an example. It has the helper functions I'm suggesting for reading and writing in a common place.
--
To view, visit https://review.coreboot.org/c/coreboot/+/69386
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ia71c43f4286f9201f03cb759252ebb405ab81904
Gerrit-Change-Number: 69386
Gerrit-PatchSet: 10
Gerrit-Owner: Mario Scheithauer <mario.scheithauer(a)siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-CC: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Attention: Mario Scheithauer <mario.scheithauer(a)siemens.com>
Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Comment-Date: Mon, 14 Nov 2022 13:04:27 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: comment
Attention is currently required from: Werner Zeh.
Hello Werner Zeh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/69544
to look at the new patch set (#2).
Change subject: mb/siemens/mc_ehl2/devicetree.cb: Use RV3028 bus_speed instead of dummy i2c device
......................................................................
mb/siemens/mc_ehl2/devicetree.cb: Use RV3028 bus_speed instead of dummy i2c device
Instead of creating a dummy I2C device in order to force Linux to
decrease the I2C bus speed, use the own 'bus_speed' field of RV3028
device config structure.
Linux should always set the bus speed to the speed of the slowest
device sitting on the bus. Hence the dummy device is not needed
here anymore.
BUG=none
TEST=See if the RV3028 RTC is visible and working (date/time can
be set/read) in Linux.
Change-Id: I6e269dc67d1fe2a6747fcf3bee224def7b553f08
Signed-off-by: Jan Samek <jan.samek(a)siemens.com>
---
M src/mainboard/siemens/mc_ehl/variants/mc_ehl2/devicetree.cb
1 file changed, 23 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/44/69544/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/69544
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I6e269dc67d1fe2a6747fcf3bee224def7b553f08
Gerrit-Change-Number: 69544
Gerrit-PatchSet: 2
Gerrit-Owner: Jan Samek <jan.samek(a)siemens.com>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Mario Scheithauer, Werner Zeh.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69386 )
Change subject: drivers/phy/m88e1512: Provide functionality to customize LED status
......................................................................
Patch Set 10:
(2 comments)
File src/drivers/phy/m88e1512/m88e1512.c:
https://review.coreboot.org/c/coreboot/+/69386/comment/066ce65c_b2f0a7d2
PS10, Line 18:
: const struct mdio_operations *mdio_ops;
: struct device *parent = dev->bus->dev;
This is bit tidious to add to every read/write op.
Do you think it is possible to write it:
uint16_t (*mdio_read)(uint8_t reg_adr) = mdio_read_helper(dev);
void (*mdio_write)(uint8_t reg_adr, uint16_t value) = mdio_write_helper(dev);
?
https://review.coreboot.org/c/coreboot/+/69386/comment/46fcbcf3_3eccfa1d
PS10, Line 34: path
Check type if it is indeed an MDIO device.
--
To view, visit https://review.coreboot.org/c/coreboot/+/69386
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ia71c43f4286f9201f03cb759252ebb405ab81904
Gerrit-Change-Number: 69386
Gerrit-PatchSet: 10
Gerrit-Owner: Mario Scheithauer <mario.scheithauer(a)siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-CC: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Attention: Mario Scheithauer <mario.scheithauer(a)siemens.com>
Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Comment-Date: Mon, 14 Nov 2022 13:01:06 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Werner Zeh.
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69544 )
Change subject: mb/siemens/mc_ehl2/devicetree.cb: Use RV3028 bus_speed instead of dummy i2c device
......................................................................
Patch Set 1:
(4 comments)
Commit Message:
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-163616):
https://review.coreboot.org/c/coreboot/+/69544/comment/36466f13_dc4fd5ac
PS1, Line 9: Instead of creating a dummy I2C device in order to force Linux to decrease the
Possible unwrapped commit description (prefer a maximum 72 chars per line)
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-163616):
https://review.coreboot.org/c/coreboot/+/69544/comment/9b68e439_e6b943a1
PS1, Line 10: I2C bus speed, use the own 'bus_speed' field of RV3028 device config structure.
Possible unwrapped commit description (prefer a maximum 72 chars per line)
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-163616):
https://review.coreboot.org/c/coreboot/+/69544/comment/9f45c09e_5c85e765
PS1, Line 12: Linux should always set the bus speed to the speed of the slowest device sitting
Possible unwrapped commit description (prefer a maximum 72 chars per line)
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-163616):
https://review.coreboot.org/c/coreboot/+/69544/comment/8e2d471a_b5838274
PS1, Line 16: TEST=See if the RV3028 RTC is visible and working (date/time can be set/read) in
Possible unwrapped commit description (prefer a maximum 72 chars per line)
--
To view, visit https://review.coreboot.org/c/coreboot/+/69544
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I6e269dc67d1fe2a6747fcf3bee224def7b553f08
Gerrit-Change-Number: 69544
Gerrit-PatchSet: 1
Gerrit-Owner: Jan Samek <jan.samek(a)siemens.com>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Comment-Date: Mon, 14 Nov 2022 12:56:46 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Jan Samek has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/69544 )
Change subject: mb/siemens/mc_ehl2/devicetree.cb: Use RV3028 bus_speed instead of dummy i2c device
......................................................................
mb/siemens/mc_ehl2/devicetree.cb: Use RV3028 bus_speed instead of dummy i2c device
Instead of creating a dummy I2C device in order to force Linux to decrease the
I2C bus speed, use the own 'bus_speed' field of RV3028 device config structure.
Linux should always set the bus speed to the speed of the slowest device sitting
on the bus. Hence the dummy device is not needed here anymore.
BUG=none
TEST=See if the RV3028 RTC is visible and working (date/time can be set/read) in
Linux.
Change-Id: I6e269dc67d1fe2a6747fcf3bee224def7b553f08
Signed-off-by: Jan Samek <jan.samek(a)siemens.com>
---
M src/mainboard/siemens/mc_ehl/variants/mc_ehl2/devicetree.cb
1 file changed, 21 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/44/69544/1
diff --git a/src/mainboard/siemens/mc_ehl/variants/mc_ehl2/devicetree.cb b/src/mainboard/siemens/mc_ehl/variants/mc_ehl2/devicetree.cb
index 3b54f24..4dd062c 100644
--- a/src/mainboard/siemens/mc_ehl/variants/mc_ehl2/devicetree.cb
+++ b/src/mainboard/siemens/mc_ehl/variants/mc_ehl2/devicetree.cb
@@ -147,6 +147,7 @@
device pci 15.1 on # I2C1
# Enable external RTC chip
chip drivers/i2c/rv3028c7
+ register "bus_speed" = "I2C_SPEED_STANDARD"
register "set_user_date" = "1"
register "user_year" = "04"
register "user_month" = "07"
@@ -156,12 +157,6 @@
register "cap_charge" = "CHARGE_OFF"
device i2c 0x52 on end # RTC RV3028-C7
end
- # Add dummy I2C device to limit BUS speed to 100 kHz in OS
- chip drivers/i2c/generic
- register "hid" = ""PRP0001""
- register "speed" = "I2C_SPEED_STANDARD"
- device i2c 0x7f on end
- end
end
device pci 15.2 on # I2C2
# Add dummy I2C device to limit BUS speed to 100 kHz in OS
--
To view, visit https://review.coreboot.org/c/coreboot/+/69544
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I6e269dc67d1fe2a6747fcf3bee224def7b553f08
Gerrit-Change-Number: 69544
Gerrit-PatchSet: 1
Gerrit-Owner: Jan Samek <jan.samek(a)siemens.com>
Gerrit-MessageType: newchange
Attention is currently required from: Angel Pons, Arthur Heymans, Lean Sheng Tan, Werner Zeh.
Mario Scheithauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69383 )
Change subject: soc/intel/ehl: Add EHL MDIO operation
......................................................................
Patch Set 7:
(2 comments)
File src/soc/intel/elkhartlake/mdio.c:
https://review.coreboot.org/c/coreboot/+/69383/comment/be400380_f355e626
PS7, Line 82: .read_resources = noop_read_resources,
: .set_resources = noop_set_resources,
> > > This should not be a PCI device, it's a mdio device. […]
static struct mdio_operations ehl_mdio_ops = {
.read = mdio_read,
.write = mdio_write,
};
//struct device_operations mdio_dev_ops = {
// .read_resources = noop_read_resources,
// .set_resources = noop_set_resources,
// .ops_mdio = &mdio_ops,
//};
void mdio_dev_enable(struct device *dev)
{
// dev->ops = &mdio_dev_ops;
dev->ops->ops_mdio = &ehl_mdio_ops;
}
Is that kind of what you mean?
File src/soc/intel/elkhartlake/tsn_gbe.c:
https://review.coreboot.org/c/coreboot/+/69383/comment/52e7ebfc_30509a7b
PS7, Line 93: .init = gbe_tsn_init,
: };
> maybe add . […]
I am not sure what you mean exactly, but mdio_dev_enable(dev) assigns ops_mdio and it is called in gbe_tsn_init().
--
To view, visit https://review.coreboot.org/c/coreboot/+/69383
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I5d1b9dd2f2ba8c18291fff314c13f0c3851784aa
Gerrit-Change-Number: 69383
Gerrit-PatchSet: 7
Gerrit-Owner: Mario Scheithauer <mario.scheithauer(a)siemens.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Comment-Date: Mon, 14 Nov 2022 12:53:24 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Mario Scheithauer <mario.scheithauer(a)siemens.com>
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: comment
Attention is currently required from: Martin L Roth, Subrata Banik, Mario Scheithauer, Angel Pons, Werner Zeh.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69382 )
Change subject: src/device + util/sconfig: Introduce new device 'mdio'
......................................................................
Patch Set 5:
(1 comment)
File src/include/device/device.h:
https://review.coreboot.org/c/coreboot/+/69382/comment/5e8c4fd6_f6e9923f
PS5, Line 23: mdio_operations
Call it mdio_bus_operations similar to other ops?
--
To view, visit https://review.coreboot.org/c/coreboot/+/69382
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I6691f92c4233bc30afc9029840b06f74bb1eb4b2
Gerrit-Change-Number: 69382
Gerrit-PatchSet: 5
Gerrit-Owner: Mario Scheithauer <mario.scheithauer(a)siemens.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Mario Scheithauer <mario.scheithauer(a)siemens.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Comment-Date: Mon, 14 Nov 2022 12:49:16 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment