Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/25019 )
Change subject: lib: Add FIT payload support
......................................................................
Patch Set 37:
(1 comment)
https://review.coreboot.org/c/coreboot/+/25019/37/src/lib/fit_payload.c
File src/lib/fit_payload.c:
https://review.coreboot.org/c/coreboot/+/25019/37/src/lib/fit_payload.c@45
PS37, Line 45: * Returns true on error, false on success.
> Patrick, This was just pointed out in another commit. […]
Do we have a hard and fast rule about how to do this? There are plenty of C functions where a 0 return code means success and non-zero means error (e.g. strcmp()), I assumed this was done in that spirit. (I think some guidelines suggest a pattern where functions that represent an action like extract() return 0 on success and non-zero on error, whereas functions that check if something is the case like islower() return the truth value of the check... but it's not widely standardized among C programmers.)
Anyway, I'm fine with flipping it either way here, just saying that I can see arguments made in favor of both versions. (That's why I kinda dislike a bool return type for functions like this in general... if it's int, people would more commonly expect the 0-means-success pattern.)
--
To view, visit https://review.coreboot.org/c/coreboot/+/25019
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I0f27b92a5e074966f893399eb401eb97d784850d
Gerrit-Change-Number: 25019
Gerrit-PatchSet: 37
Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-CC: Martin Roth <martinroth(a)google.com>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Patrick Rudolph
Gerrit-Comment-Date: Mon, 05 Aug 2019 18:31:45 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Martin Roth <martinroth(a)google.com>
Gerrit-MessageType: comment
Matt DeVillier has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34685 )
Change subject: drivers/fsp1_1/raminit: fix use of mrc_hob
......................................................................
drivers/fsp1_1/raminit: fix use of mrc_hob
Commit 509f469 [drivers/fsp1_1/raminit.c: Always check FSP HOBs]
inadvertently made use of the mrc_hob conditional on
CONFIG_DISPLAY_HOBS, when there is no relation between the two,
leading to MRC cache data not being used / RAM training being
redone on the 3rd boot (and thereafter) following a flash.
On some devices, this causes MRC cache corruption and a bricked device.
Fix this by removing the condition on CONFIG_DISPLAY_HOBS.
Test: boot google/{cyan,edgar}, observe third boot and onward do not
brick device, properly use mrc_hob via cbmem console and timestamps.
Change-Id: I01f6d1d6dfd10297b30de638301c5e0b6545da9c
Signed-off-by: Matt DeVillier <matt.devillier(a)gmail.com>
---
M src/drivers/intel/fsp1_1/raminit.c
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/34685/1
diff --git a/src/drivers/intel/fsp1_1/raminit.c b/src/drivers/intel/fsp1_1/raminit.c
index e71c9a2..21f4ab9 100644
--- a/src/drivers/intel/fsp1_1/raminit.c
+++ b/src/drivers/intel/fsp1_1/raminit.c
@@ -259,7 +259,7 @@
/* Locate the memory configuration data to speed up the next reboot */
mrc_hob = get_next_guid_hob(&mrc_guid, hob_list_ptr);
- if ((mrc_hob == NULL) && CONFIG(DISPLAY_HOBS))
+ if (mrc_hob == NULL)
printk(BIOS_DEBUG,
"Memory Configuration Data Hob not present\n");
else if (!vboot_recovery_mode_enabled()) {
--
To view, visit https://review.coreboot.org/c/coreboot/+/34685
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I01f6d1d6dfd10297b30de638301c5e0b6545da9c
Gerrit-Change-Number: 34685
Gerrit-PatchSet: 1
Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-MessageType: newchange
Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/33170
Change subject: drivers/xpowers/axp209: Remove code
......................................................................
drivers/xpowers/axp209: Remove code
This code was only used by allwinner CPUs which is removed at this
point.
Change-Id: I31a2a502bffdc605cc31127723ed769b8665c314
Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz>
---
D src/drivers/xpowers/axp209/Kconfig
D src/drivers/xpowers/axp209/Makefile.inc
D src/drivers/xpowers/axp209/axp209.c
D src/drivers/xpowers/axp209/axp209.h
D src/drivers/xpowers/axp209/chip.h
5 files changed, 0 insertions(+), 386 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/33170/1
diff --git a/src/drivers/xpowers/axp209/Kconfig b/src/drivers/xpowers/axp209/Kconfig
deleted file mode 100644
index 684873c..0000000
--- a/src/drivers/xpowers/axp209/Kconfig
+++ /dev/null
@@ -1,15 +0,0 @@
-config DRIVER_XPOWERS_AXP209
- bool
- default n
- help
- X-Powers AXP902 Power Management Unit
-
-if DRIVER_XPOWERS_AXP209
-
-config DRIVER_XPOWERS_AXP209_BOOTBLOCK
- bool
- default n
- help
- Make AXP209 functionality available in he bootblock.
-
-endif # DRIVER_XPOWERS_AXP209
diff --git a/src/drivers/xpowers/axp209/Makefile.inc b/src/drivers/xpowers/axp209/Makefile.inc
deleted file mode 100644
index e08a8e2..0000000
--- a/src/drivers/xpowers/axp209/Makefile.inc
+++ /dev/null
@@ -1,7 +0,0 @@
-ifeq ($(CONFIG_DRIVER_XPOWERS_AXP209),y)
-
-bootblock-$(CONFIG_DRIVER_XPOWERS_AXP209_BOOTBLOCK) += axp209.c
-romstage-y += axp209.c
-ramstage-y += axp209.c
-
-endif
diff --git a/src/drivers/xpowers/axp209/axp209.c b/src/drivers/xpowers/axp209/axp209.c
deleted file mode 100644
index da575cc..0000000
--- a/src/drivers/xpowers/axp209/axp209.c
+++ /dev/null
@@ -1,323 +0,0 @@
-/*
- * Driver for X-Powers AXP 209 Power Management Unit
- *
- * Despite axp209_read/write only working on a byte at a time, there is no such
- * limitation in the AXP209.
- *
- * Copyright (C) 2013 Alexandru Gagniuc <mr.nuke.me(a)gmail.com>
- * Subject to the GNU GPL v2, or (at your option) any later version.
- */
-
-#include "axp209.h"
-#include "chip.h"
-
-#include <console/console.h>
-#include <device/device.h>
-#include <device/i2c_simple.h>
-
-/* Hide these definitions from the rest of the source, so keep them here */
-enum registers {
- REG_POWER_STATUS = 0x00,
- REG_POWER_MODE = 0x01,
- REG_OTG_VBUS = 0x02,
- REG_CHIP_ID = 0x03,
- REG_CHIP_PWROUT_CTL = 0x12,
- REG_DCDC2_VOLTAGE = 0x23,
- REG_DCDC2_LDO3_CTL = 0x25,
- REG_DCDC3_VOLTAGE = 0x27,
- REG_LDO24_VOLTAGE = 0x28,
- REG_LDO3_VOLTAGE = 0x29,
- REG_VBUS_IPSOUT = 0x30,
- REG_PWROFF_VOLTAGE = 0x31,
- REG_SHTDWN_SETTING = 0x32,
-};
-
-/* REG_LDO24_VOLTAGE definitions */
-#define REG_LDO24_VOLTAGE_LDO2_MASK (0xf << 4)
-#define REG_LDO24_VOLTAGE_LDO2_VAL(x) ((x << 4) & REG_LDO24_VOLTAGE_LDO2_MASK)
-#define REG_LDO24_VOLTAGE_LDO4_MASK (0xf << 0)
-#define REG_LDO24_VOLTAGE_LDO4_VAL(x) ((x << 0) & REG_LDO24_VOLTAGE_LDO4_MASK)
-
-/*
- * Read and write accessors. We only work on one register at a time, but there
- * is no limitation on the AXP209 as to how many registers we may read or write
- * in one transaction.
- * These return the number of bytes read/written, or an error code. In this
- * case, they return 1 on success, or an error code otherwise. This is done to
- * work with I2C drivers that return either 0 on success or the number of bytes
- * actually transferred.
- */
-static int axp209_read(u8 bus, u8 reg, u8 *val)
-{
- if (i2c_readb(bus, AXP209_I2C_ADDR, reg, val) < 0)
- return CB_ERR;
- return 1;
-}
-
-static int axp209_write(u8 bus, u8 reg, u8 val)
-{
- if (i2c_writeb(bus, AXP209_I2C_ADDR, reg, val) < 0)
- return CB_ERR;
- return 1;
-}
-
-/**
- * \brief Identify and initialize an AXP209 on the I2C bus
- *
- * @param[in] bus I2C bus to which the AXP209 is connected
- * @return CB_SUCCES on if an AXP209 is found, or an error code otherwise.
- */
-enum cb_err axp209_init(u8 bus)
-{
- u8 id;
-
- if (axp209_read(bus, REG_CHIP_ID, &id) != 1)
- return CB_ERR;
-
- /* From U-Boot code : Low 4 bits is chip version */
- if ((id & 0x0f) != 0x1) {
- printk(BIOS_ERR, "[axp209] ID 0x%x does not match\n", id);
- return CB_ERR;
- }
-
- return CB_SUCCESS;
-}
-
-/**
- * \brief Configure the output voltage of DC-DC2 converter
- *
- * If the requested voltage is not available, the next lowest voltage will
- * be applied.
- * Valid values are between 700mV and 2275mV
- *
- * @param[in] millivolts voltage in mV units.
- * @param[in] bus I2C bus to which the AXP209 is connected
- * @return CB_SUCCES on success,
- * CB_ERR_ARG if voltage is out of range, or an error code otherwise.
- */
-enum cb_err axp209_set_dcdc2_voltage(u8 bus, u16 millivolts)
-{
- u8 val;
-
- if (millivolts < 700 || millivolts > 2275)
- return CB_ERR_ARG;
-
- val = (millivolts - 700) / 25;
-
- if (axp209_write(bus, REG_DCDC2_VOLTAGE, val) != 1)
- return CB_ERR;
-
- return CB_SUCCESS;
-}
-
-/**
- * \brief Configure the output voltage of DC-DC3 converter
- *
- * If the requested voltage is not available, the next lowest voltage will
- * be applied.
- * Valid values are between 700mV and 3500mV
- *
- * @param[in] millivolts voltage in mV units.
- * @param[in] bus I2C bus to which the AXP209 is connected
- * @return CB_SUCCES on success,
- * CB_ERR_ARG if voltage is out of range, or an error code otherwise.
- */
-enum cb_err axp209_set_dcdc3_voltage(u8 bus, u16 millivolts)
-{
- u8 val;
-
- if (millivolts < 700 || millivolts > 3500)
- return CB_ERR_ARG;
-
- val = (millivolts - 700) / 25;
-
- if (axp209_write(bus, REG_DCDC3_VOLTAGE, val) != 1)
- return CB_ERR;
-
- return CB_SUCCESS;
-}
-
-/**
- * \brief Configure the output voltage of LDO2 regulator
- *
- * If the requested voltage is not available, the next lowest voltage will
- * be applied.
- * Valid values are between 700mV and 3300mV
- *
- * @param[in] millivolts voltage in mV units.
- * @param[in] bus I2C bus to which the AXP209 is connected
- * @return CB_SUCCES on success,
- * CB_ERR_ARG if voltage is out of range, or an error code otherwise.
- */
-enum cb_err axp209_set_ldo2_voltage(u8 bus, u16 millivolts)
-{
- u8 reg8, val;
-
- if (millivolts < 1800 || millivolts > 3300)
- return CB_ERR_ARG;
-
- /* Try to read the register first, and stop here on error */
- if (axp209_read(bus, REG_LDO24_VOLTAGE, ®8) != 1)
- return CB_ERR;
-
- val = (millivolts - 1800) / 100;
- reg8 &= ~REG_LDO24_VOLTAGE_LDO2_MASK;
- reg8 |= REG_LDO24_VOLTAGE_LDO2_VAL(val);
-
- if (axp209_write(bus, REG_LDO24_VOLTAGE, reg8) != 1)
- return CB_ERR;
-
- return CB_SUCCESS;
-}
-
-/**
- * \brief Configure the output voltage of LDO4 regulator
- *
- * If the requested voltage is not available, the next lowest voltage will
- * be applied.
- * Valid values are between 700mV and 3500mV. Datasheet lists maximum voltage at
- * 2250mV, but hardware samples go as high as 3500mV.
- *
- * @param[in] millivolts voltage in mV units.
- * @param[in] bus I2C bus to which the AXP209 is connected
- * @return CB_SUCCES on success,
- * CB_ERR_ARG if voltage is out of range, or an error code otherwise.
- */
-enum cb_err axp209_set_ldo3_voltage(u8 bus, u16 millivolts)
-{
- u8 val;
-
- /* Datasheet lists 2250 max, but PMU will output up to 3500mV */
- if (millivolts < 700 || millivolts > 3500)
- return CB_ERR_ARG;
-
- val = (millivolts - 700) / 25;
-
- if (axp209_write(bus, REG_LDO3_VOLTAGE, val) != 1)
- return CB_ERR;
-
- return CB_SUCCESS;
-}
-
-/**
- * \brief Configure the output voltage of DC-DC2 converter
- *
- * If the requested voltage is not available, the next lowest voltage will
- * be applied.
- * Valid values are between 1250V and 3300mV
- *
- * @param[in] millivolts voltage in mV units.
- * @param[in] bus I2C bus to which the AXP209 is connected
- * @return CB_SUCCES on success,
- * CB_ERR_ARG if voltage is out of range, or an error code otherwise.
- */
-enum cb_err axp209_set_ldo4_voltage(u8 bus, u16 millivolts)
-{
- u8 reg8, val;
-
- if (millivolts < 1250 || millivolts > 3300)
- return CB_ERR_ARG;
-
- /* Try to read the register first, and stop here on error */
- if (axp209_read(bus, REG_LDO24_VOLTAGE, ®8) != 1)
- return CB_ERR;
-
- if (millivolts <= 2000)
- val = (millivolts - 1200) / 100;
- else if (millivolts <= 2700)
- val = 9 + (millivolts - 2500) / 100;
- else if (millivolts <= 2800)
- val = 11;
- else
- val = 12 + (millivolts - 3000) / 100;
-
- reg8 &= ~REG_LDO24_VOLTAGE_LDO4_MASK;
- reg8 |= REG_LDO24_VOLTAGE_LDO4_VAL(val);
-
- if (axp209_write(bus, REG_LDO24_VOLTAGE, reg8) != 1)
- return CB_ERR;
-
- return CB_SUCCESS;
-}
-
-static const struct {
- enum cb_err (*set_voltage) (u8 bus, u16 mv);
- const char *name;
-} vtable[] = { {
- .set_voltage = axp209_set_dcdc2_voltage,
- .name = "DCDC2",
- }, {
- .set_voltage = axp209_set_dcdc3_voltage,
- .name = "DCDC3",
- }, {
- .set_voltage = axp209_set_ldo2_voltage,
- .name = "LDO2",
- }, {
- .set_voltage = axp209_set_ldo3_voltage,
- .name = "LDO3",
- }, {
- .set_voltage = axp209_set_ldo4_voltage,
- .name = "LDO4",
- }
-};
-
-static enum cb_err set_rail(u8 bus, int idx, u16 mv)
-{
- enum cb_err err;
- const char *name = vtable[idx].name;
-
- /* If voltage isn't specified, don't touch the rail */
- if (mv == 0) {
- printk(BIOS_DEBUG, "[axp209] Skipping %s configuration\n",
- name);
- return CB_SUCCESS;
- }
-
- if ((err = vtable[idx].set_voltage(bus, mv) != CB_SUCCESS)) {
- printk(BIOS_ERR, "[axp209] Failed to set %s to %u mv\n",
- name, mv);
- return err;
- }
-
- return CB_SUCCESS;
-}
-
-/**
- * \brief Configure all voltage rails
- *
- * Configure all converters and regulators from devicetree config. If any of the
- * voltages are not declared (i.e. are zero), the respective rail will not be
- * reconfigured, and retain its powerup voltage.
- *
- * @param[in] cfg pointer to @ref drivers_xpowers_axp209_config structure
- * @param[in] bus I2C bus to which the AXP209 is connected
- * @return CB_SUCCES on success, or an error code otherwise.
- */
-enum cb_err axp209_set_voltages(u8 bus, const struct
- drivers_xpowers_axp209_config *cfg)
-{
- enum cb_err err;
-
- /* Don't worry about what the error is. Console prints that */
- err = set_rail(bus, 0, cfg->dcdc2_voltage_mv);
- err |= set_rail(bus, 1, cfg->dcdc3_voltage_mv);
- err |= set_rail(bus, 2, cfg->ldo2_voltage_mv);
- err |= set_rail(bus, 3, cfg->ldo3_voltage_mv);
- err |= set_rail(bus, 4, cfg->ldo4_voltage_mv);
-
- if (err != CB_SUCCESS)
- return CB_ERR;
-
- return CB_SUCCESS;
-}
-
-/*
- * Usually, the AXP209 is enabled and configured in romstage, so there is no
- * need for a full ramstage driver. Hence .enable_dev is NULL.
- */
-#ifndef __PRE_RAM__
-struct chip_operations drivers_xpowers_axp209_config = {
- CHIP_NAME("X-Powers AXP 209 Power Management Unit")
- .enable_dev = NULL,
-};
-#endif /* __PRE_RAM__ */
diff --git a/src/drivers/xpowers/axp209/axp209.h b/src/drivers/xpowers/axp209/axp209.h
deleted file mode 100644
index c9cdd7e..0000000
--- a/src/drivers/xpowers/axp209/axp209.h
+++ /dev/null
@@ -1,20 +0,0 @@
-/*
- * Definitions for X-Powers AXP 209 Power Management Unit
- *
- * Copyright (C) 2013 Alexandru Gagniuc <mr.nuke.me(a)gmail.com>
- * Subject to the GNU GPL v2, or (at your option) any later version.
- */
-
-#include <types.h>
-#include "chip.h"
-
-#define AXP209_I2C_ADDR (0x68 >> 1)
-
-enum cb_err axp209_init(u8 bus);
-enum cb_err axp209_set_dcdc2_voltage(u8 bus, u16 millivolts);
-enum cb_err axp209_set_dcdc3_voltage(u8 bus, u16 millivolts);
-enum cb_err axp209_set_ldo2_voltage(u8 bus, u16 millivolts);
-enum cb_err axp209_set_ldo3_voltage(u8 bus, u16 millivolts);
-enum cb_err axp209_set_ldo4_voltage(u8 bus, u16 millivolts);
-enum cb_err axp209_set_voltages(u8 bus, const struct
- drivers_xpowers_axp209_config *cfg);
diff --git a/src/drivers/xpowers/axp209/chip.h b/src/drivers/xpowers/axp209/chip.h
deleted file mode 100644
index c19253d..0000000
--- a/src/drivers/xpowers/axp209/chip.h
+++ /dev/null
@@ -1,21 +0,0 @@
-/*
- * X-Powers AXP 209 devicetree.cb interface
- *
- * Copyright (C) 2013 Alexandru Gagniuc <mr.nuke.me(a)gmail.com>
- * Subject to the GNU GPL v2, or (at your option) any later version.
- */
-
-#ifndef AXP209_CHIP_H
-#define AXP209_CHIP_H
-
-#include <types.h>
-
-struct drivers_xpowers_axp209_config {
- u16 dcdc2_voltage_mv; /**< DCDC2 converter voltage output */
- u16 dcdc3_voltage_mv; /**< DCDC3 converter voltage output */
- u16 ldo2_voltage_mv; /**< LDO2 regulator voltage output */
- u16 ldo3_voltage_mv; /**< LDO3 regulator voltage output */
- u16 ldo4_voltage_mv; /**< LDO4 regulator voltage output */
-};
-
-#endif /* AXP209_CHIP_H */
--
To view, visit https://review.coreboot.org/c/coreboot/+/33170
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I31a2a502bffdc605cc31127723ed769b8665c314
Gerrit-Change-Number: 33170
Gerrit-PatchSet: 1
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: newchange