Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/52723 )
Change subject: drivers/i2c/designware: Use safe defaults for SCL parameters
......................................................................
drivers/i2c/designware: Use safe defaults for SCL parameters
Inspired by discussion in CB:22822.
If I2C bus step response has not been measured, assume the layout to
have been designed with a minimal capacitance and SCL rise and fall
times of 0 ns. The calculations will add the required amount of
reference clocks for the host to drive SCL high or low, such that the
maximum bus frequency specification is met.
Change-Id: Icbafae22c83ffbc16c179fb5412fb4fd6b70813a
Signed-off-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/52723
Reviewed-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Reviewed-by: Furquan Shaikh <furquan(a)google.com>
Reviewed-by: Werner Zeh <werner.zeh(a)siemens.com>
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
---
M src/drivers/i2c/designware/dw_i2c.c
1 file changed, 2 insertions(+), 30 deletions(-)
Approvals:
build bot (Jenkins): Verified
Werner Zeh: Looks good to me, approved
Furquan Shaikh: Looks good to me, but someone else must approve
Tim Wawrzynczak: Looks good to me, approved
diff --git a/src/drivers/i2c/designware/dw_i2c.c b/src/drivers/i2c/designware/dw_i2c.c
index c0212c3..2e2d20d 100644
--- a/src/drivers/i2c/designware/dw_i2c.c
+++ b/src/drivers/i2c/designware/dw_i2c.c
@@ -621,7 +621,6 @@
{
const int ic_clk = CONFIG_DRIVERS_I2C_DESIGNWARE_CLOCK_MHZ;
struct dw_i2c_regs *regs;
- uint16_t hcnt_min, lcnt_min;
int i;
regs = (struct dw_i2c_regs *)dw_i2c_addr;
@@ -637,35 +636,8 @@
return 0;
}
- /* If rise time is set use the time calculation. */
- if (bcfg->rise_time_ns)
- return dw_i2c_gen_config_rise_fall_time(regs, speed, bcfg,
- ic_clk, config);
-
- if (speed >= I2C_SPEED_HIGH) {
- /* High speed */
- hcnt_min = MIN_HS_SCL_HIGHTIME;
- lcnt_min = MIN_HS_SCL_LOWTIME;
- } else if (speed >= I2C_SPEED_FAST_PLUS) {
- /* Fast-Plus speed */
- hcnt_min = MIN_FP_SCL_HIGHTIME;
- lcnt_min = MIN_FP_SCL_LOWTIME;
- } else if (speed >= I2C_SPEED_FAST) {
- /* Fast speed */
- hcnt_min = MIN_FS_SCL_HIGHTIME;
- lcnt_min = MIN_FS_SCL_LOWTIME;
- } else {
- /* Standard speed */
- hcnt_min = MIN_SS_SCL_HIGHTIME;
- lcnt_min = MIN_SS_SCL_LOWTIME;
- }
-
- config->speed = speed;
- config->scl_hcnt = ic_clk * hcnt_min / KHz;
- config->scl_lcnt = ic_clk * lcnt_min / KHz;
- config->sda_hold = ic_clk * DEFAULT_SDA_HOLD_TIME / KHz;
-
- return 0;
+ /* Use the time calculation. */
+ return dw_i2c_gen_config_rise_fall_time(regs, speed, bcfg, ic_clk, config);
}
static int dw_i2c_set_speed(unsigned int bus, enum i2c_speed speed,
--
To view, visit https://review.coreboot.org/c/coreboot/+/52723
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Icbafae22c83ffbc16c179fb5412fb4fd6b70813a
Gerrit-Change-Number: 52723
Gerrit-PatchSet: 3
Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Duncan Laurie <duncan(a)iceblink.org>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: merged
Attention is currently required from: Felix Singer, Raul Rangel, Philipp Hug, Patrick Rudolph, Paul Menzel, Angel Pons, Aaron Durbin, Lance Zhao, Jason Glenesk, Nico Huber, Marshall Dawson, Lee Leahy, Christian Walter, Tim Wawrzynczak, Huang Jin, Alexander Couzens, Werner Zeh, Ron Minnich, Felix Held.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52809 )
Change subject: treewide: Kconfig: replace `def_bool n` by `bool`
......................................................................
Patch Set 1:
(6 comments)
File src/mainboard/google/beltino/Kconfig:
https://review.coreboot.org/c/coreboot/+/52809/comment/cd67d4e8_27a1542a
PS1, Line 63: def_bool n
default n required to override the options `default y` in src/southbridge/intel/lynxpoint/Kconfig
File src/mainboard/google/jecht/Kconfig:
https://review.coreboot.org/c/coreboot/+/52809/comment/5356e5cb_b333ea33
PS1, Line 53: def_bool n
default n required to override the options `default y` in src/soc/intel/broadwell/Kconfig
File src/mainboard/purism/librem_bdw/Kconfig:
https://review.coreboot.org/c/coreboot/+/52809/comment/f41112d3_40a646e7
PS1, Line 25: def_bool n
default n required to override the options `default y` in src/drivers/uart/Kconfig
https://review.coreboot.org/c/coreboot/+/52809/comment/4784f166_faf0dcb4
PS1, Line 30: config PCIEXP_L1_SUB_STATE
: def_bool n
:
default n required to override the options `default y` in src/soc/intel/broadwell/Kconfig
https://review.coreboot.org/c/coreboot/+/52809/comment/0e45a86a_19875e67
PS1, Line 34: def_bool n
default n required to override the options `default y` in src/soc/intel/broadwell/Kconfig
File src/soc/qualcomm/ipq40xx/Kconfig:
https://review.coreboot.org/c/coreboot/+/52809/comment/94b239b0_26e0bafb
PS1, Line 18: def_bool n
required to override default y in src/Kconfig
--
To view, visit https://review.coreboot.org/c/coreboot/+/52809
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I27e89c66d18b7b3a5ac425eacea10df8c013814f
Gerrit-Change-Number: 52809
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Alexander Couzens <lynxis(a)fe80.eu>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Huang Jin <huang.jin(a)intel.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Lance Zhao
Gerrit-Reviewer: Lee Leahy <leroy.p.leahy(a)intel.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Philipp Hug <philipp(a)hug.cx>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Ron Minnich <rminnich(a)gmail.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Philipp Hug <philipp(a)hug.cx>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Attention: Lance Zhao
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: Lee Leahy <leroy.p.leahy(a)intel.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Huang Jin <huang.jin(a)intel.com>
Gerrit-Attention: Alexander Couzens <lynxis(a)fe80.eu>
Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Attention: Ron Minnich <rminnich(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Fri, 30 Apr 2021 22:59:32 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Raul Rangel, Furquan Shaikh, Matt DeVillier, Angel Pons, Michael Niewöhner, Patrick Rudolph, Felix Held.
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52493 )
Change subject: [RFC] soc/intel/skylake: Introduce new method for setting device states
......................................................................
Patch Set 13:
(1 comment)
File src/soc/intel/skylake/fspdevmap.c:
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-118138):
https://review.coreboot.org/c/coreboot/+/52493/comment/1e95123d_53906a4d
PS13, Line 14: if (dev) {
braces {} are not necessary for single statement blocks
--
To view, visit https://review.coreboot.org/c/coreboot/+/52493
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I70fde306c65a8881f565c5f923be20f380ea64d3
Gerrit-Change-Number: 52493
Gerrit-PatchSet: 13
Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Fri, 30 Apr 2021 22:58:09 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Raul Rangel, Furquan Shaikh, Matt DeVillier, Angel Pons, Michael Niewöhner, Patrick Rudolph, Felix Held.
Hello build bot (Jenkins), Raul Rangel, Nico Huber, Furquan Shaikh, Matt DeVillier, Tim Wawrzynczak, Angel Pons, Michael Niewöhner, Patrick Rudolph, Felix Held,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/52493
to look at the new patch set (#13).
Change subject: [RFC] soc/intel/skylake: Introduce new method for setting device states
......................................................................
[RFC] soc/intel/skylake: Introduce new method for setting device states
We mostly use the same mechanism for setting the state of internal
devices, but we copy it over again and again. Thus, introduce a new
method trying to reduce redundant code.
Basically, this uses an array of structs containing the DEVFN number,
its corresponding FSP option and an indicator if the FSP option is
inverted. The struct maps the device to its related FSP option and
`set_dev_state_by_devicetree()` iterates over the array using the usual
mechanism.
Unresolved issues:
- Where should fspdevmap{.c,.h} be implemented?
Change-Id: I70fde306c65a8881f565c5f923be20f380ea64d3
Signed-off-by: Felix Singer <felixsinger(a)posteo.net>
---
M src/soc/intel/skylake/Makefile.inc
M src/soc/intel/skylake/chip.c
A src/soc/intel/skylake/fspdevmap.c
A src/soc/intel/skylake/fspdevmap.h
M src/soc/intel/skylake/romstage/Makefile.inc
M src/soc/intel/skylake/romstage/fsp_params.c
6 files changed, 69 insertions(+), 63 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/52493/13
--
To view, visit https://review.coreboot.org/c/coreboot/+/52493
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I70fde306c65a8881f565c5f923be20f380ea64d3
Gerrit-Change-Number: 52493
Gerrit-PatchSet: 13
Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: newpatchset