Re: [PATCH v5 3/4] drivers: tee: sandbox: add rpc test ta emulation
Hi Igor, On Wed, 20 Jan 2021 at 18:56, Igor Opaniuk wrote: > > From: Igor Opaniuk > > This adds support for RPC test trusted application emulation, which > permits to test reverse RPC calls to TEE supplicant. Currently it covers > requests to the I2C bus from TEE. > > Signed-off-by: Igor Opaniuk > Reviewed-by: Simon Glass > --- > > (no changes since v1) > > drivers/tee/Makefile| 2 + > drivers/tee/optee/Kconfig | 9 ++ > drivers/tee/sandbox.c | 143 +++- > include/tee/optee_ta_rpc_test.h | 28 +++ > 4 files changed, 178 insertions(+), 4 deletions(-) > create mode 100644 include/tee/optee_ta_rpc_test.h > > diff --git a/drivers/tee/Makefile b/drivers/tee/Makefile > index 5c8ffdbce8..ff844195ae 100644 > --- a/drivers/tee/Makefile > +++ b/drivers/tee/Makefile > @@ -2,5 +2,7 @@ > > obj-y += tee-uclass.o > obj-$(CONFIG_SANDBOX) += sandbox.o > +obj-$(CONFIG_OPTEE_TA_RPC_TEST) += optee/supplicant.o > +obj-$(CONFIG_OPTEE_TA_RPC_TEST) += optee/i2c.o I think this line should move to drivers/tee/optee/Makefile for consistency. > obj-$(CONFIG_OPTEE) += optee/ > obj-y += broadcom/ > diff --git a/drivers/tee/optee/Kconfig b/drivers/tee/optee/Kconfig > index d489834df9..65622f30b1 100644 > --- a/drivers/tee/optee/Kconfig > +++ b/drivers/tee/optee/Kconfig > @@ -22,6 +22,15 @@ config OPTEE_TA_AVB > The TA can support the "avb" subcommands "read_rb", "write"rb" > and "is_unlocked". > > +config OPTEE_TA_RPC_TEST > + bool "Support RPC TEST TA" > + depends on SANDBOX_TEE > + default y > + help > + Enables support for RPC test trusted application emulation, which > + permits to test reverse RPC calls to TEE supplicant. Should > + be used only in sandbox env. > + > endmenu > > endif > diff --git a/drivers/tee/sandbox.c b/drivers/tee/sandbox.c > index e1ba027fd6..d075701b4e 100644 > --- a/drivers/tee/sandbox.c > +++ b/drivers/tee/sandbox.c > @@ -7,11 +7,15 @@ > #include > #include > #include > +#include > + > +#include "optee/optee_msg.h" > +#include "optee/optee_private.h" > > /* > * The sandbox tee driver tries to emulate a generic Trusted Exectution > - * Environment (TEE) with the Trusted Application (TA) OPTEE_TA_AVB > - * available. > + * Environment (TEE) with the Trusted Applications (TA) OPTEE_TA_AVB and > + * OPTEE_TA_RPC_TEST available. > */ > > static const u32 pstorage_max = 16; > @@ -32,7 +36,38 @@ struct ta_entry { >struct tee_param *params); > }; > > -#ifdef CONFIG_OPTEE_TA_AVB > +static int get_msg_arg(struct udevice *dev, uint num_params, > + struct tee_shm **shmp, struct optee_msg_arg **msg_arg) > +{ > + int rc; > + struct optee_msg_arg *ma; > + > + rc = __tee_shm_add(dev, OPTEE_MSG_NONCONTIG_PAGE_SIZE, NULL, > + OPTEE_MSG_GET_ARG_SIZE(num_params), TEE_SHM_ALLOC, > + shmp); > + if (rc) > + return rc; > + > + ma = (*shmp)->addr; > + memset(ma, 0, OPTEE_MSG_GET_ARG_SIZE(num_params)); > + ma->num_params = num_params; > + *msg_arg = ma; > + > + return 0; > +} > + > +void *optee_alloc_and_init_page_list(void *buf, ulong len, > +u64 *phys_buf_ptr) > +{ > + /* > +* An empty stub is added just to fix linking issues. > +* This function isn't supposed to be called in sandbox > +* setup, otherwise replace this with a proper > +* implementation from optee/core.c > +*/ > + return NULL; > +} > + > static u32 get_attr(uint n, uint num_params, struct tee_param *params) > { > if (n >= num_params) > @@ -63,6 +98,7 @@ bad_params: > return TEE_ERROR_BAD_PARAMETERS; > } > > +#ifdef CONFIG_OPTEE_TA_AVB > static u32 ta_avb_open_session(struct udevice *dev, uint num_params, >struct tee_param *params) > { > @@ -214,7 +250,100 @@ static u32 ta_avb_invoke_func(struct udevice *dev, u32 > func, uint num_params, > return TEE_ERROR_NOT_SUPPORTED; > } > } > -#endif /*OPTEE_TA_AVB*/ > +#endif /* OPTEE_TA_AVB */ > + > +#ifdef CONFIG_OPTEE_TA_RPC_TEST > +static u32 ta_rpc_test_open_session(struct udevice *dev, uint num_params, > + struct tee_param *params) > +{ > + /* > +* We don't expect additional parameters when opening a session to > +* this TA. > +*/ > + return check_params(TEE_PARAM_ATTR_TYPE_NONE, > TEE_PARAM_ATTR_TYPE_NONE, > + TEE_PARAM_ATTR_TYPE_NONE, > TEE_PARAM_ATTR_TYPE_NONE, > + num_params, params); > +} > + > +static void fill_i2c_rpc_params(struct optee_msg_arg *msg_arg, u64 bus_num, > + u64 chip_addr, u64 op, > + struct tee_param_memref memref) > +{ > + ms
Re: [PATCH v2 12/12] smbios: Allow a few values to come from sysinfo
On Thu, Jan 21, 2021 at 10:07 AM Simon Glass wrote: > > While static configuration is useful it cannot cover every case. Sometimes > board revisions are encoded in resistor straps and must be read at > runtime. > > The easiest way to provide this information is via sysinfo, since the > board can then provide a driver to read whatever is needed. > > Add some standard sysinfo options for this, and use them to obtain the > required information. > > Signed-off-by: Simon Glass > --- > > (no changes since v1) > > include/sysinfo.h | 11 +++ > lib/smbios.c | 32 +--- > 2 files changed, 40 insertions(+), 3 deletions(-) > > diff --git a/include/sysinfo.h b/include/sysinfo.h > index 6e021253524..743f3554659 100644 > --- a/include/sysinfo.h > +++ b/include/sysinfo.h > @@ -31,6 +31,17 @@ > * to read the serial number. > */ > > +/** enum sysinfo_id - Standard IDs defined by U-Boot */ > +enum sysinfo_id { > + SYSINFO_ID_NONE, > + > + SYSINFO_ID_SMBIOS_SYSTEM_VERSION, > + SYSINFO_ID_SMBIOS_BASEBOARD_VERSION, > + > + /* First value available for downstream/board used */ > + SYSINFO_ID_USER = 0x1000, > +}; > + > struct sysinfo_ops { > /** > * detect() - Run the hardware info detection procedure for this > diff --git a/lib/smbios.c b/lib/smbios.c > index d46569b09f4..9bdde0b953f 100644 > --- a/lib/smbios.c > +++ b/lib/smbios.c > @@ -10,6 +10,7 @@ > #include > #include > #include > +#include > #include > #include > #ifdef CONFIG_CPU > @@ -106,15 +107,26 @@ static int smbios_add_string(struct smbios_ctx *ctx, > const char *str) > } > > /** > - * smbios_add_prop() - Add a property from the device tree > + * smbios_add_prop_si() - Add a property from the devicetree or sysinfo > + * > + * Sysinfo is used if available, with a fallback to devicetree > * > * @start: string area start address > * @node: node containing the information to write (ofnode_null() if > none) > * @prop: property to write > * @return 0 if not found, else SMBIOS string number (1 or more) > */ > -static int smbios_add_prop(struct smbios_ctx *ctx, const char *prop) > +static int smbios_add_prop_si(struct smbios_ctx *ctx, const char *prop, > + int sysinfo_id) > { > + if (sysinfo_id && ctx->dev) { > + char val[80]; Is 80 the limitation defined by sysinfo drivers? Can this be a macro? > + int ret; > + > + ret = sysinfo_get_str(ctx->dev, sysinfo_id, sizeof(val), val); > + if (!ret) > + return smbios_add_string(ctx, val); > + } > if (IS_ENABLED(CONFIG_OF_CONTROL)) { > const char *str; > > @@ -126,6 +138,17 @@ static int smbios_add_prop(struct smbios_ctx *ctx, const > char *prop) > return 0; > } > > +/** > + * smbios_add_prop() - Add a property from the devicetree > + * > + * @prop: property to write > + * @return 0 if not found, else SMBIOS string number (1 or more) > + */ > +static int smbios_add_prop(struct smbios_ctx *ctx, const char *prop) > +{ > + return smbios_add_prop_si(ctx, prop, SYSINFO_ID_NONE); > +} > + > static void set_eos(struct smbios_ctx *ctx, char *eos) > { > ctx->eos = eos; > @@ -239,7 +262,8 @@ static int smbios_write_type1(ulong *current, int handle, > set_eos(ctx, t->eos); > t->manufacturer = smbios_add_prop(ctx, "manufacturer"); > t->product_name = smbios_add_prop(ctx, "product"); > - t->version = smbios_add_prop(ctx, "version"); > + t->version = smbios_add_prop_si(ctx, "version", > + SYSINFO_ID_SMBIOS_SYSTEM_VERSION); > if (serial_str) { > t->serial_number = smbios_add_string(ctx, serial_str); > strncpy((char *)t->uuid, serial_str, sizeof(t->uuid)); > @@ -268,6 +292,8 @@ static int smbios_write_type2(ulong *current, int handle, > set_eos(ctx, t->eos); > t->manufacturer = smbios_add_prop(ctx, "manufacturer"); > t->product_name = smbios_add_prop(ctx, "product"); > + t->version = smbios_add_prop_si(ctx, "version", > + SYSINFO_ID_SMBIOS_BASEBOARD_VERSION); > t->asset_tag_number = smbios_add_prop(ctx, "asset-tag"); > t->feature_flags = SMBIOS_BOARD_FEATURE_HOSTING; > t->board_type = SMBIOS_BOARD_MOTHERBOARD; > -- Reviewed-by: Bin Meng
Re: [PATCH v2 11/12] x86: coral: Add sysinfo ops
Hi Simon, On Thu, Jan 21, 2021 at 10:07 AM Simon Glass wrote: > > These ops are missing at present which is not permitted. Add an empty > operation struct. > > Signed-off-by: Simon Glass > --- > > Changes in v2: > - Add new patch to fix crash on coral > > board/google/chromebook_coral/coral.c | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/board/google/chromebook_coral/coral.c > b/board/google/chromebook_coral/coral.c > index 34b2c2ac5d5..f9fb3f163f0 100644 > --- a/board/google/chromebook_coral/coral.c > +++ b/board/google/chromebook_coral/coral.c > @@ -8,6 +8,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -143,6 +144,9 @@ struct acpi_ops coral_acpi_ops = { > .inject_dsdt= chromeos_acpi_gpio_generate, > }; > > +struct sysinfo_ops coral_sysinfo_ops = { > +}; > + > #if !CONFIG_IS_ENABLED(OF_PLATDATA) > static const struct udevice_id coral_ids[] = { > { .compatible = "google,coral" }, > @@ -154,5 +158,6 @@ U_BOOT_DRIVER(coral_drv) = { > .name = "coral", > .id = UCLASS_SYSINFO, > .of_match = of_match_ptr(coral_ids), > + .ops= &coral_sysinfo_ops, > ACPI_OPS_PTR(&coral_acpi_ops) > }; Shouldn't we fix sysinfo-uclass to test op against NULL? That way we relax the driver a little bit. Regards, Bin
Re: [PATCH v2 10/12] sysinfo: Move #ifdef so that operations are always defined
On Thu, Jan 21, 2021 at 10:07 AM Simon Glass wrote: > > At present the struct is not available unless SYSINFO is enabled. This is > annoying since code it is not possible to use compile-time checks like > CONFIG_IS_ENABLED(SYSINFO) with this header. > > Fix it by moving the #ifdef. > > Signed-off-by: Simon Glass > --- > > Changes in v2: > - Add new patch to fix sysinfo with CONFIG_IS_ENABLED() > > include/sysinfo.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > Reviewed-by: Bin Meng
Re: [PATCH v2 09/12] smbios: Add more options for the BIOS version string
On Thu, Jan 21, 2021 at 10:07 AM Simon Glass wrote: > > At present the version string is obtained from PLAIN_VERSION. Some boards > may want to configure this using the device tree, since the build system > can more easily insert things there after U-Boot itself is built. Add this > option to the code. > > Also in some cases the version needs to be generated programmatically, > such as when it is stored elsewhere in the ROM and must be read first. > To handle this, keep a pointer around so that it can be updated later. > This works by storing the last string in the context, since it is easier > than passing out a little-used extra parameter. > > Provide a function to update the version string. > > Signed-off-by: Simon Glass > --- > > Changes in v2: > - Correct documentation format > > include/asm-generic/global_data.h | 6 > include/smbios.h | 12 +++ > lib/smbios.c | 56 +-- > 3 files changed, 71 insertions(+), 3 deletions(-) > > diff --git a/include/asm-generic/global_data.h > b/include/asm-generic/global_data.h > index 19f70393b45..750e998d885 100644 > --- a/include/asm-generic/global_data.h > +++ b/include/asm-generic/global_data.h > @@ -439,6 +439,12 @@ struct global_data { > */ > struct acpi_ctx *acpi_ctx; > #endif > +#if CONFIG_IS_ENABLED(GENERATE_SMBIOS_TABLE) > + /** > +* @smbios_version: Points to SMBIOS type 0 version > +*/ > + char *smbios_version; > +#endif > }; > > /** > diff --git a/include/smbios.h b/include/smbios.h > index 1cbeabf9522..ecc4fd1de3b 100644 > --- a/include/smbios.h > +++ b/include/smbios.h > @@ -257,4 +257,16 @@ const struct smbios_header *smbios_header(const struct > smbios_entry *entry, int > */ > const char *smbios_string(const struct smbios_header *header, int index); > > +/** > + * smbios_update_version() - Update the version string > + * > + * This can be called after the SMBIOS tables are written (e.g. after the > U-Boot > + * main loop has started) to update the BIOS version string (SMBIOS table 0). > + * > + * @version: New version string to use > + * @return 0 if OK, -ENOENT if no version string was previously written, > + * -ENOSPC if the new string is too large to fit > + */ > +int smbios_update_version(const char *version); > + > #endif /* _SMBIOS_H_ */ > diff --git a/lib/smbios.c b/lib/smbios.c > index 7090460bc02..d46569b09f4 100644 > --- a/lib/smbios.c > +++ b/lib/smbios.c > @@ -17,6 +17,10 @@ > #include > #endif > > +enum { > + SMBIOS_STR_MAX = 64, /* Maximum length allowed for a string */ > +}; > + > /** > * struct smbios_ctx - context for writing SMBIOS tables > * > @@ -27,12 +31,15 @@ > * @next_ptr: pointer to the start of the next string to be added. When the > * table is nopt empty, this points to the byte after the \0 of > the > * previous string. > + * @last_str: points to the last string that was written to the table, or > NULL > + * if none > */ > struct smbios_ctx { > ofnode node; > struct udevice *dev; > char *eos; > char *next_ptr; > + char *last_str; > }; > > /** > @@ -78,6 +85,7 @@ static int smbios_add_string(struct smbios_ctx *ctx, const > char *str) > > for (;;) { > if (!*p) { > + ctx->last_str = p; > strcpy(p, str); > p += strlen(str); > *p++ = '\0'; > @@ -87,8 +95,10 @@ static int smbios_add_string(struct smbios_ctx *ctx, const > char *str) > return i; > } > > - if (!strcmp(p, str)) > + if (!strcmp(p, str)) { > + ctx->last_str = p; > return i; > + } > > p += strlen(p) + 1; > i++; > @@ -120,6 +130,35 @@ static void set_eos(struct smbios_ctx *ctx, char *eos) > { > ctx->eos = eos; > ctx->next_ptr = eos; > + ctx->last_str = NULL; > +} > + > +int smbios_update_version(const char *version) > +{ > + char *ptr = gd->smbios_version; Missing DECLARE_GLOBAL_DATA_PTR? > + uint old_len, len; > + > + if (!ptr) > + return log_ret(-ENOENT); > + > + /* > +* This string is supposed to have at least enough bytes and is > +* padded with spaces. Update it, taking care not to move the > +* \0 terminator, so that other strings in the string table > +* are not disturbed. See smbios_add_string() > +*/ > + old_len = strnlen(ptr, SMBIOS_STR_MAX); > + len = strnlen(version, SMBIOS_STR_MAX); > + if (len > old_len) > + return log_ret(-ENOSPC); > + > + log_debug("Replacing SMBIOS type 0 version string '%s'\n", ptr); > + memcpy(ptr, version, len); > +#ifdef LOG_DEBUG > + print_buffer((ulong)ptr, ptr, 1, old
Re: [PATCH] smcc: fix sign bit expansion
On Tue, Jan 05, 2021 at 08:03:11PM +, Volodymyr Babchuk wrote: > Signed ARM_SMCCC_FAST_CALL value is shifted to 31'st bit. Then, it is expanded > to 64 bit value, which results in 1s in higher 32 bits. > > This causes corrupted values in 64-bit SMC IDs and issues in buggy handlers of > 32-bit calls. > > We need to make ARM_SMCCC_FAST_CALL unsigned long, so it would work properly > on 32 bit architectures. > > Signed-off-by: Volodymyr Babchuk > --- > include/linux/arm-smccc.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Jens Wiklander You may want to make the prefix "smccc:" (note the extra c) instead. Cheers, Jens > > diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h > index 2d1e6cc156..7f2be23394 100644 > --- a/include/linux/arm-smccc.h > +++ b/include/linux/arm-smccc.h > @@ -11,8 +11,8 @@ > * http://infocenter.arm.com/help/topic/com.arm.doc.den0028a/index.html > */ > > -#define ARM_SMCCC_STD_CALL 0 > -#define ARM_SMCCC_FAST_CALL 1 > +#define ARM_SMCCC_STD_CALL 0UL > +#define ARM_SMCCC_FAST_CALL 1UL > #define ARM_SMCCC_TYPE_SHIFT 31 > > #define ARM_SMCCC_SMC_32 0 > -- > 2.29.2
Re: [PATCH v2 07/12] smbios: Drop the eos parameter
Hi Simon, On Thu, Jan 21, 2021 at 10:07 AM Simon Glass wrote: > > We can store this in the context and avoid passing it to each function. > This makes it easier to follow and will also allow keeping track of the > end of the string table (in future patches). > > Add an 'eos' field to the context and create a function to set it up. > > Signed-off-by: Simon Glass > --- > > (no changes since v1) > > lib/smbios.c | 61 +++- > 1 file changed, 37 insertions(+), 24 deletions(-) > > diff --git a/lib/smbios.c b/lib/smbios.c > index 4d2cb0f85e2..1d9bde0c3c2 100644 > --- a/lib/smbios.c > +++ b/lib/smbios.c > @@ -22,10 +22,13 @@ > * > * @node: node containing the information to write (ofnode_null() if > none) > * @dev: sysinfo device to use (NULL if none) > + * @eos: end-of-string pointer for the table being processed. This is > set > + * up when we start processing a table > */ > struct smbios_ctx { > ofnode node; > struct udevice *dev; > + char *eos; > }; > > /** > @@ -57,14 +60,15 @@ struct smbios_write_method { > * This adds a string to the string area which is appended directly after > * the formatted portion of an SMBIOS structure. > * > - * @start: string area start address > + * @ctx: SMBIOS context > * @str: string to add > * @return:string number in the string area (1 or more) > */ > -static int smbios_add_string(char *start, const char *str) > +static int smbios_add_string(struct smbios_ctx *ctx, const char *str) > { > int i = 1; > - char *p = start; > + char *p = ctx->eos; > + > if (!*str) > str = "Unknown"; > > @@ -90,25 +94,28 @@ static int smbios_add_string(char *start, const char *str) > * smbios_add_prop() - Add a property from the device tree > * > * @start: string area start address > - * @ctx: context for writing the tables > + * @node: node containing the information to write (ofnode_null() if > none) Instead of adding @node, we should remove @start. > * @prop: property to write > * @return 0 if not found, else SMBIOS string number (1 or more) > */ > -static int smbios_add_prop(char *start, struct smbios_ctx *ctx, > - const char *prop) > +static int smbios_add_prop(struct smbios_ctx *ctx, const char *prop) > { > - > if (IS_ENABLED(CONFIG_OF_CONTROL)) { > const char *str; > > str = ofnode_read_string(ctx->node, prop); > if (str) > - return smbios_add_string(start, str); > + return smbios_add_string(ctx, str); > } > > return 0; > } > > +static void set_eos(struct smbios_ctx *ctx, char *eos) nits: call it smbios_ctx_set_eos()? > +{ > + ctx->eos = eos; > +} > + > /** > * smbios_string_table_len() - compute the string area size > * > @@ -140,9 +147,10 @@ static int smbios_write_type0(ulong *current, int handle, > t = map_sysmem(*current, len); > memset(t, 0, sizeof(struct smbios_type0)); > fill_smbios_header(t, SMBIOS_BIOS_INFORMATION, len, handle); > - t->vendor = smbios_add_string(t->eos, "U-Boot"); > - t->bios_ver = smbios_add_string(t->eos, PLAIN_VERSION); > - t->bios_release_date = smbios_add_string(t->eos, U_BOOT_DMI_DATE); > + set_eos(ctx, t->eos); > + t->vendor = smbios_add_string(ctx, "U-Boot"); > + t->bios_ver = smbios_add_string(ctx, PLAIN_VERSION); > + t->bios_release_date = smbios_add_string(ctx, U_BOOT_DMI_DATE); > #ifdef CONFIG_ROM_SIZE > t->bios_rom_size = (CONFIG_ROM_SIZE / 65536) - 1; > #endif > @@ -180,17 +188,18 @@ static int smbios_write_type1(ulong *current, int > handle, > t = map_sysmem(*current, len); > memset(t, 0, sizeof(struct smbios_type1)); > fill_smbios_header(t, SMBIOS_SYSTEM_INFORMATION, len, handle); > - t->manufacturer = smbios_add_prop(t->eos, ctx, "manufacturer"); > - t->product_name = smbios_add_prop(t->eos, ctx, "product"); > - t->version = smbios_add_prop(t->eos, ctx, "version"); > + set_eos(ctx, t->eos); > + t->manufacturer = smbios_add_prop(ctx, "manufacturer"); > + t->product_name = smbios_add_prop(ctx, "product"); > + t->version = smbios_add_prop(ctx, "version"); > if (serial_str) { > - t->serial_number = smbios_add_string(t->eos, serial_str); > + t->serial_number = smbios_add_string(ctx, serial_str); > strncpy((char *)t->uuid, serial_str, sizeof(t->uuid)); > } else { > - t->serial_number = smbios_add_prop(t->eos, ctx, "serial"); > + t->serial_number = smbios_add_prop(ctx, "serial"); > } > - t->sku_number = smbios_add_prop(t->eos, ctx, "sku"); > - t->family = smbios_add_prop(t->eos, ctx, "family"); > + t->sku_number = smbios_add_
Re: [PATCH v2 08/12] smbios: Track the end of the string table
On Thu, Jan 21, 2021 at 10:07 AM Simon Glass wrote: > > Add a new member to the context struct which tracks the end of the string > table. This allows us to avoid recalculating this at the end. > > Signed-off-by: Simon Glass > --- > > (no changes since v1) > > lib/smbios.c | 32 +++- > 1 file changed, 15 insertions(+), 17 deletions(-) > Reviewed-by: Bin Meng
Re: [PATCH v2 06/12] smbios: Use a struct to keep track of context
On Thu, Jan 21, 2021 at 10:07 AM Simon Glass wrote: > > At present we pass the ofnode to each function. We also pass the 'eos' > pointer for adding new strings. We don't track the current end of the > string table, so have smbios_string_table_len() to find that. > > The code can be made more efficient if it keeps information in a > context struct. This also makes it easier to add more features. > > As a first step, switch the ofnode parameter to be a context pointer. > Update smbios_add_prop() at the same time to avoid changing the same > lines of code in consecutive patches. > > Signed-off-by: Simon Glass > --- > > Changes in v2: > - Zero the context's dev pointer if not used > > lib/smbios.c | 87 +--- > 1 file changed, 55 insertions(+), 32 deletions(-) > Reviewed-by: Bin Meng
Re: [PATCH v2 05/12] smbios: Set BIOS release version
On Thu, Jan 21, 2021 at 10:07 AM Simon Glass wrote: > > We may as well include the U-Boot release information in the type-0 table > since it is designed for that purpose. > > U-Boot uses release versions based on the year and month. The year cannot > fit in a byte, so drop the century. > > Signed-off-by: Simon Glass > --- > > Changes in v2: > - Add a comment about dropping the century > > lib/smbios.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > Reviewed-by: Bin Meng
Re: [PATCH v2 04/12] smbios: Use char consistently for the eos member
On Thu, Jan 21, 2021 at 10:07 AM Simon Glass wrote: > > At present a few of the structs use u8 instead of char. This is a string, > so char is better. Update them. > > Signed-off-by: Simon Glass > Reviewed-by: Christian Gmeiner > --- > > (no changes since v1) > > include/smbios.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > Reviewed-by: Bin Meng
Re: [PATCH v2 03/12] smbios: Move smbios_write_type to the C file
On Thu, Jan 21, 2021 at 10:07 AM Simon Glass wrote: > > This type is not used outside the smbios.c file so there is no need for it > to be in the header file. Move it. > > Signed-off-by: Simon Glass > Reviewed-by: Christian Gmeiner > --- > > (no changes since v1) > > include/smbios.h | 10 -- > lib/smbios.c | 10 ++ > 2 files changed, 10 insertions(+), 10 deletions(-) > Reviewed-by: Bin Meng
Re: [PATCH v2 02/12] Makefile: Provide numeric versions
On Thu, Jan 21, 2021 at 10:06 AM Simon Glass wrote: > > For SMBIOS we want to store the numeric version numbers in the tables. It > does not make sense to parse the strings. Instead, add new #defines with > the version and patchlevel. > > Signed-off-by: Simon Glass > --- > > (no changes since v1) > > Makefile | 4 > README | 8 > 2 files changed, 12 insertions(+) > Reviewed-by: Bin Meng
Re: [PATCH v2 01/12] README: Add doumentation for version information
Hi Simon, On Thu, Jan 21, 2021 at 10:06 AM Simon Glass wrote: > > There are quite a few available version options in U-Boot. Add a list of > the available Makefile variables and #defines, along with examples. > > Signed-off-by: Simon Glass > --- > > (no changes since v1) > > README | 84 ++ > 1 file changed, 84 insertions(+) > Good write-up. However I believe going forward we should write documents using reST. Please consider moving this version to something like develop/version.rst. Regards, Bin
Re: [PATCH 1/1] cmd: CMD_ACPI depends on ACPIGEN
On Thu, Jan 21, 2021 at 11:13 AM Bin Meng wrote: > > On Thu, Jan 21, 2021 at 4:38 AM Heinrich Schuchardt > wrote: > > > > Trying to compile qemu-x86_64_defconfig with CONFIG_CMD_ACPI=y and > > CONFIG_ACPIGEN=n fails with > > > > ld.bfd: cmd/built-in.o: in function `do_acpi_items': > > cmd/acpi.c:162: undefined reference to `acpi_dump_items' > > > > Add the missing configuration dependency. > > > > Signed-off-by: Heinrich Schuchardt > > --- > > cmd/Kconfig | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > Reviewed-by: Bin Meng applied to u-boot-x86, thanks!
Re: [PATCH 12/12] x86: coral: Show memory config and SKU ID on startup
On Sun, Jan 17, 2021 at 5:54 AM Simon Glass wrote: > > Provide the model information through sysinfo so that it shows up on > boot. For memconfig 4 pins are provided, for 16 combinations. For SKU > ID there are two options: > >- two pins provided in a ternary arrangement, for 9 combinations. >- reading from the EC > > Add a binding doc and drop the unused #defines as well. > > Example: > >U-Boot 2021.01-rc5 > >CPU: Intel(R) Celeron(R) CPU N3450 @ 1.10GHz >DRAM: 3.9 GiB >MMC: sdmmc@1b,0: 1, emmc@1c,0: 2 >Video: 1024x768x32 @ b000 >Model: Google Coral (memconfig 5, SKU 3) > > Signed-off-by: Simon Glass > --- > > arch/x86/dts/chromebook_coral.dts | 11 ++ > board/google/chromebook_coral/coral.c | 139 +- > board/google/chromebook_coral/variant_gpio.h | 6 - > .../sysinfo/google,coral.txt | 37 + > 4 files changed, 179 insertions(+), 14 deletions(-) > create mode 100644 doc/device-tree-bindings/sysinfo/google,coral.txt > > diff --git a/arch/x86/dts/chromebook_coral.dts > b/arch/x86/dts/chromebook_coral.dts > index bfbdd517d1f..f4e408bb751 100644 > --- a/arch/x86/dts/chromebook_coral.dts > +++ b/arch/x86/dts/chromebook_coral.dts > @@ -54,6 +54,17 @@ > recovery-gpios = <&gpio_nw (-1) GPIO_ACTIVE_LOW>; > write-protect-gpios = <&gpio_nw GPIO_75 GPIO_ACTIVE_HIGH>; > phase-enforce-gpios = <&gpio_n GPIO_10 GPIO_ACTIVE_HIGH>; > + memconfig-gpios = <&gpio_nw GPIO_101 GPIO_ACTIVE_HIGH > + &gpio_nw GPIO_102 GPIO_ACTIVE_HIGH > + &gpio_n GPIO_38 GPIO_ACTIVE_HIGH > + &gpio_n GPIO_45 GPIO_ACTIVE_HIGH>; > + > + /* > +* This is used for reef only: > +* > +* skuconfig-gpios = <&gpio_nw GPIO_16 GPIO_ACTIVE_HIGH > + &gpio_nw GPIO_17 GPIO_ACTIVE_HIGH>; missing * > +*/ > smbios { > /* Type 1 table */ > system { > diff --git a/board/google/chromebook_coral/coral.c > b/board/google/chromebook_coral/coral.c > index f9fb3f163f0..77a6cca4497 100644 > --- a/board/google/chromebook_coral/coral.c > +++ b/board/google/chromebook_coral/coral.c > @@ -3,9 +3,12 @@ > * Copyright 2019 Google LLC > */ > > +#define LOG_CATEGORY UCLASS_SYSINFO > + > #include > #include > #include > +#include > #include > #include > #include > @@ -15,6 +18,7 @@ > #include > #include > #include > +#include > #include "variant_gpio.h" > > struct cros_gpio_info { > @@ -29,10 +33,125 @@ int arch_misc_init(void) > return 0; > } > > -/* This function is needed if CONFIG_CMDLINE is not enabled */ > -int board_run_command(const char *cmdline) > +static int get_memconfig(struct udevice *dev) > { > - printf("No command line\n"); > + struct gpio_desc gpios[4]; > + int cfg; > + int ret; > + > + ret = gpio_request_list_by_name(dev, "memconfig-gpios", gpios, > + ARRAY_SIZE(gpios), > + GPIOD_IS_IN | GPIOD_PULL_UP); > + if (ret < 0) { > + log_debug("Cannot get GPIO list '%s' (%d)\n", dev->name, ret); > + return ret; > + } > + > + /* Give the lines time to settle */ > + udelay(10); > + > + ret = dm_gpio_get_values_as_int(gpios, ARRAY_SIZE(gpios)); > + if (ret < 0) > + return log_msg_ret("get", ret); > + cfg = ret; > + > + ret = gpio_free_list(dev, gpios, ARRAY_SIZE(gpios)); > + if (ret) > + return log_msg_ret("free", ret); > + > + return cfg; > +} > + > +/** > + * get_skuconfig() - Get the SKU number either from pins or the EC > + * > + * Two options are supported: > + * skuconfig-gpios - two pins in the device tree (tried first) > + * EC - reading from the EC (backup) > + * > + * @dev: sysinfo device to use > + * @return SKU ID, or -ve error if not found > + */ > +static int get_skuconfig(struct udevice *dev) > +{ > + struct gpio_desc gpios[2]; > + int cfg; > + int ret; > + > + ret = gpio_request_list_by_name(dev, "skuconfig-gpios", gpios, > + ARRAY_SIZE(gpios), > + GPIOD_IS_IN); > + if (ret != ARRAY_SIZE(gpios)) { > + struct udevice *cros_ec; > + > + log_debug("Cannot get GPIO list '%s' (%d)\n", dev->name, ret); > + > + /* Try the EC */ > + ret = uclass_first_device_err(UCLASS_CROS_EC, &cros_ec); > + if (ret < 0) { > + log_err("Cannot find EC for SKU details\n"); > + return log_msg_ret("sku", ret); > + } > + ret = cros_ec_get_sku_id(cros_ec); > + if (re
Re: [PATCH 11/12] sysinfo: Allow showing model info from sysinfo
On Sun, Jan 17, 2021 at 5:54 AM Simon Glass wrote: > > Some boards may want to show the SKU ID or other information obtained at > runtime. Allow this to come from sysinfo. The board can then provide a > sysinfo driver to provide it. > > Signed-off-by: Simon Glass > --- > > common/board_info.c | 37 + > include/sysinfo.h | 4 > 2 files changed, 33 insertions(+), 8 deletions(-) > Reviewed-by: Bin Meng
Re: [PATCH 10/12] x86: tpl: Show next stage being booted
On Sun, Jan 17, 2021 at 5:54 AM Simon Glass wrote: > > Enhance the debugging to show the next stage being booted as well as a > dump of the start of the image. > > Signed-off-by: Simon Glass > --- > > arch/x86/lib/tpl.c | 7 ++- > 1 file changed, 6 insertions(+), 1 deletion(-) > Reviewed-by: Bin Meng
Re: [PATCH 09/12] x86: spl: Clear BSS unconditionally
On Sun, Jan 17, 2021 at 5:54 AM Simon Glass wrote: > > This should be done even if not using TPL, since BSS may be in use or > boards that only use SPL. Fix it. > > Signed-off-by: Simon Glass > --- > > arch/x86/lib/spl.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > Reviewed-by: Bin Meng
Re: [PATCH 08/12] x86: zimage: Improve command-line debug handling
On Sun, Jan 17, 2021 at 5:54 AM Simon Glass wrote: > > At present if the command line is very long it is truncated by the > printf() statement, which works within a limited buffer. Use puts() > instead. Also show better debugging with the command-line setup > fails. > > Signed-off-by: Simon Glass > --- > > arch/x86/lib/zimage.c | 10 +++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > Reviewed-by: Bin Meng
Re: [PATCH 07/12] x86: zimage: Allow dumping the image from outside the module
On Sun, Jan 17, 2021 at 5:54 AM Simon Glass wrote: > > At present it is possible to dump an image within the zimage command, but > it is also useful to be able to dump it from elsewhere, for example in a > loader that has special handling for the different zimage stages. > > Export this feature as a new function. > > Signed-off-by: Simon Glass > --- > > arch/x86/include/asm/zimage.h | 10 ++ > arch/x86/lib/zimage.c | 23 +++ > 2 files changed, 25 insertions(+), 8 deletions(-) > Reviewed-by: Bin Meng
Re: [PATCH 06/12] x86: Update Chromium OS GNVS names
On Sun, Jan 17, 2021 at 5:54 AM Simon Glass wrote: > > The Global Non-Voltatile Storage struct has some fields with particular typo: Volatile > meanings. Rename these to make things easier to follow. Also add a few > more boot flags. > > GNVS should not be confused with GNVQ (Going Nowhere Very Quickly). > > Signed-off-by: Simon Glass > --- > > arch/x86/include/asm/intel_gnvs.h | 34 +-- > 1 file changed, 28 insertions(+), 6 deletions(-) > Reviewed-by: Bin Meng
Re: [PATCH 05/12] x86: spl: Make moving BSS conditional
On Sun, Jan 17, 2021 at 5:54 AM Simon Glass wrote: > > At present BSS is always placed in SDRAM. If a separate BSS is not in use > this means that BSS doesn't work as expected. Make the setting conditional > on the SEPARATE_BSS option. > > Signed-off-by: Simon Glass > --- > > arch/x86/cpu/u-boot-spl.lds | 2 ++ > 1 file changed, 2 insertions(+) > Reviewed-by: Bin Meng
Re: [PATCH 04/12] x86: Make sure the SPL image ends on a suitable boundary
Hi Simon, On Sun, Jan 17, 2021 at 5:54 AM Simon Glass wrote: > > The part of U-Boot that actually ends up in u-boot-nodtb.bin is not built > with any particular alignment. It ends at the start of the BSS section. > The BSS section selects its own alignment, which may larger. I don't see start of the BSS section has alignment in the linker script. > This means that there can be a gap of a few bytes between the image > ending and BSS starting. > > Since u-boot.bin is build by joining u-boot-nodtb.bin and u-boot.dtb (with > perhaps some padding for BSS), the expected result is not obtained. U-Boot > uses the end of BSS to find the devicetree, so this means that it cannot > be found. Please explain this in more details, maybe showing a failure case with exact bss start/size and where U-Boot expects to get the device tree but it's not > > Add 32-byte alignment of BSS so that the image size is correct and > appending the devicetree will place it at the end of BSS. > > Signed-off-by: Simon Glass > --- > > arch/x86/cpu/u-boot-spl.lds | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/x86/cpu/u-boot-spl.lds b/arch/x86/cpu/u-boot-spl.lds > index e6c22895b35..e0c70b076b8 100644 > --- a/arch/x86/cpu/u-boot-spl.lds > +++ b/arch/x86/cpu/u-boot-spl.lds > @@ -43,6 +43,7 @@ SECTIONS > __binman_sym_start = .; > KEEP(*(SORT(.binman_sym*))); > __binman_sym_end = .; > + . = ALIGN(32); Is 32 safe enough? > } > > _image_binary_end = .; > -- Regards, Bin
Re: [PATCH v2 01/21] mmc: sdhci: Add helper functions for UHS modes
Hi Aswath, On 1/21/21 1:13 PM, Aswath Govindraju wrote: > Hi Jaehoon, > > On 21/01/21 4:26 am, Jaehoon Chung wrote: >> Hi Aswath, >> >> On 1/19/21 9:35 PM, Aswath Govindraju wrote: >>> Hi Jaehoon, >>> >>> On 05/11/20 4:03 am, Jaehoon Chung wrote: On 11/5/20 4:05 AM, Faiz Abbas wrote: > Jaehoon, > > On 21/10/20 5:08 pm, Jaehoon Chung wrote: >> Hi Faiz, >> >> On 10/16/20 8:08 PM, Faiz Abbas wrote: >>> Add a set_voltage() function which handles the switch from 3.3V to 1.8V >>> for SD card UHS modes. >>> >>> Signed-off-by: Faiz Abbas >>> --- >>> drivers/mmc/sdhci.c | 51 + >>> include/sdhci.h | 1 + >>> 2 files changed, 52 insertions(+) >>> >>> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c >>> index 7673219fb3..a69f058191 100644 >>> --- a/drivers/mmc/sdhci.c >>> +++ b/drivers/mmc/sdhci.c >>> @@ -20,6 +20,7 @@ >>> #include >>> #include >>> #include >>> +#include >>> >>> static void sdhci_reset(struct sdhci_host *host, u8 mask) >>> { >>> @@ -556,6 +557,56 @@ void sdhci_set_uhs_timing(struct sdhci_host *host) >>> sdhci_writew(host, reg, SDHCI_HOST_CONTROL2); >>> } >>> >>> +#if CONFIG_IS_ENABLED(MMC_IO_VOLTAGE) >>> +static void sdhci_set_voltage(struct sdhci_host *host) >>> +{ >>> + struct mmc *mmc = (struct mmc *)host->mmc; >>> + u32 ctrl; >>> + >>> + ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2); >>> + >>> + switch (mmc->signal_voltage) { >>> + case MMC_SIGNAL_VOLTAGE_330: >>> +#if CONFIG_IS_ENABLED(DM_REGULATOR) >>> + if (mmc->vqmmc_supply) { >>> + regulator_set_enable(mmc->vqmmc_supply, false); >>> + regulator_set_value(mmc->vqmmc_supply, 330); >> >> Doesn't need to consider about fail to set its value? > > You're right. I'll handle the failure case in v3. > >> >>> + regulator_set_enable(mmc->vqmmc_supply, true); >>> + } >>> +#endif >>> + mdelay(5); >> >> For what purpose about mdelay(5)? > > I'm following this from the kernel implementation here: > https://protect2.fireeye.com/v1/url?k=8b617b16-d4fa420c-8b60f059-0cc47a6cba04-71d56f4128587abb&q=1&e=02f975f3-5191-4501-a554-a58145bd6ac1&u=https%3A%2F%2Fgithub.com%2Ftorvalds%2Flinux%2Fblob%2Fmaster%2Fdrivers%2Fmmc%2Fhost%2Fsdhci.c%23L2547 > > Not sure if this a part of the spec or not though. Thanks for sharing info. :) > >> >> >>> + if (IS_SD(mmc)) { >>> + ctrl &= ~SDHCI_CTRL_VDD_180; >>> + sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2); >>> + } >>> + break; >>> + case MMC_SIGNAL_VOLTAGE_180: >>> +#if CONFIG_IS_ENABLED(DM_REGULATOR) >>> + if (mmc->vqmmc_supply) { >>> + regulator_set_enable(mmc->vqmmc_supply, false); >>> + regulator_set_value(mmc->vqmmc_supply, 180); >>> + regulator_set_enable(mmc->vqmmc_supply, true); >>> + } >>> +#endif >>> + if (IS_SD(mmc)) { >>> + ctrl |= SDHCI_CTRL_VDD_180; >>> + sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2); >>> + } >>> + break; >>> + default: >>> + /* No signal voltage switch required */ >>> + return; >>> + } >>> +} >>> +#else >>> +static void sdhci_set_voltage(struct sdhci_host *host) { } >>> +#endif >>> +void sdhci_set_control_reg(struct sdhci_host *host) >> >> this function is called as callback function in sdhci_set_ios(). >> it's strange... set_control_reg callback is for host specific control >> register. >> >> I think that it doesn't need to assign to callback. > > This is the default set_control_reg() implementation which is defined > in the sdhci spec. Any host that that wants default implementation > case assign this as their callback. > > Hosts that have custom implementations can add in their own drivers. Yes..but when i have checked your code. It doesn't need to assign to callback for your driver. If sdhci_set_control_reg() is common function about all sdhci-xx driver, then it can be just added in sdhci_set_ios(). > callback is for sdhci-xx specific progress. In my opinion, static int sdhci_set_ios() { ... sdhci_set_control_reg(); if (host->ops && host->ops->set_control_reg) host->ops->set_control_reg(); } if sdhci_set_control_reg() is no
Re: [PATCH v2 01/21] mmc: sdhci: Add helper functions for UHS modes
Hi Jaehoon, On 21/01/21 4:26 am, Jaehoon Chung wrote: > Hi Aswath, > > On 1/19/21 9:35 PM, Aswath Govindraju wrote: >> Hi Jaehoon, >> >> On 05/11/20 4:03 am, Jaehoon Chung wrote: >>> On 11/5/20 4:05 AM, Faiz Abbas wrote: Jaehoon, On 21/10/20 5:08 pm, Jaehoon Chung wrote: > Hi Faiz, > > On 10/16/20 8:08 PM, Faiz Abbas wrote: >> Add a set_voltage() function which handles the switch from 3.3V to 1.8V >> for SD card UHS modes. >> >> Signed-off-by: Faiz Abbas >> --- >> drivers/mmc/sdhci.c | 51 + >> include/sdhci.h | 1 + >> 2 files changed, 52 insertions(+) >> >> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c >> index 7673219fb3..a69f058191 100644 >> --- a/drivers/mmc/sdhci.c >> +++ b/drivers/mmc/sdhci.c >> @@ -20,6 +20,7 @@ >> #include >> #include >> #include >> +#include >> >> static void sdhci_reset(struct sdhci_host *host, u8 mask) >> { >> @@ -556,6 +557,56 @@ void sdhci_set_uhs_timing(struct sdhci_host *host) >> sdhci_writew(host, reg, SDHCI_HOST_CONTROL2); >> } >> >> +#if CONFIG_IS_ENABLED(MMC_IO_VOLTAGE) >> +static void sdhci_set_voltage(struct sdhci_host *host) >> +{ >> +struct mmc *mmc = (struct mmc *)host->mmc; >> +u32 ctrl; >> + >> +ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2); >> + >> +switch (mmc->signal_voltage) { >> +case MMC_SIGNAL_VOLTAGE_330: >> +#if CONFIG_IS_ENABLED(DM_REGULATOR) >> +if (mmc->vqmmc_supply) { >> +regulator_set_enable(mmc->vqmmc_supply, false); >> +regulator_set_value(mmc->vqmmc_supply, 330); > > Doesn't need to consider about fail to set its value? You're right. I'll handle the failure case in v3. > >> +regulator_set_enable(mmc->vqmmc_supply, true); >> +} >> +#endif >> +mdelay(5); > > For what purpose about mdelay(5)? I'm following this from the kernel implementation here: https://protect2.fireeye.com/v1/url?k=8b617b16-d4fa420c-8b60f059-0cc47a6cba04-71d56f4128587abb&q=1&e=02f975f3-5191-4501-a554-a58145bd6ac1&u=https%3A%2F%2Fgithub.com%2Ftorvalds%2Flinux%2Fblob%2Fmaster%2Fdrivers%2Fmmc%2Fhost%2Fsdhci.c%23L2547 Not sure if this a part of the spec or not though. >>> >>> Thanks for sharing info. :) >>> > > >> +if (IS_SD(mmc)) { >> +ctrl &= ~SDHCI_CTRL_VDD_180; >> +sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2); >> +} >> +break; >> +case MMC_SIGNAL_VOLTAGE_180: >> +#if CONFIG_IS_ENABLED(DM_REGULATOR) >> +if (mmc->vqmmc_supply) { >> +regulator_set_enable(mmc->vqmmc_supply, false); >> +regulator_set_value(mmc->vqmmc_supply, 180); >> +regulator_set_enable(mmc->vqmmc_supply, true); >> +} >> +#endif >> +if (IS_SD(mmc)) { >> +ctrl |= SDHCI_CTRL_VDD_180; >> +sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2); >> +} >> +break; >> +default: >> +/* No signal voltage switch required */ >> +return; >> +} >> +} >> +#else >> +static void sdhci_set_voltage(struct sdhci_host *host) { } >> +#endif >> +void sdhci_set_control_reg(struct sdhci_host *host) > > this function is called as callback function in sdhci_set_ios(). > it's strange... set_control_reg callback is for host specific control > register. > > I think that it doesn't need to assign to callback. This is the default set_control_reg() implementation which is defined in the sdhci spec. Any host that that wants default implementation case assign this as their callback. Hosts that have custom implementations can add in their own drivers. >>> >>> Yes..but when i have checked your code. It doesn't need to assign to >>> callback for your driver. >>> If sdhci_set_control_reg() is common function about all sdhci-xx driver, >>> then it can be just added in sdhci_set_ios(). callback is for sdhci-xx specific progress. >>> >>> In my opinion, >>> >>> static int sdhci_set_ios() >>> { >>> ... >>> sdhci_set_control_reg(); >>> if (host->ops && host->ops->set_control_reg) >>> host->ops->set_control_reg(); >>> >>> } >>> >>> if sdhci_set_control_reg() is not generic sequence, it has to be located >>> into am654_sdhci.c >>> It seems to add sdhci_set_ctonrol_reg() in sdhci.c for your am654_sdhci >>> driver. >>> >>
Re: Contributor meeting notes 19-Jan-21
Hi Michal, On Wed, 20 Jan 2021 at 08:50, Michal Simek wrote: > > > > On 1/20/21 4:44 PM, Simon Glass wrote: > > Hi Michal, > > > > On Wed, 20 Jan 2021 at 00:46, Michal Simek wrote: > >> > >> Hi, > >> > >> On 1/19/21 7:54 PM, Simon Glass wrote: > >>> Hi, > >>> > >>> Thank you for attending! > >>> > >>> Full notes at [1] > >>> > >>> Tuesday 19 January 2021 > >>> > >>> Present: Daniel Schwierzeck, Heinrich Schuchardt, Michal Simek, Sean > >>> Anderson, Simon Glass, Walter Lozano > >>> > >>> Notes: > >>> [all] Introductions > >>> [all] Timing of call > >>> - Current time is the best across US, Europe, India. But not any good > >>> for East Asia > >>> - Simon might send out survey > >>> - Send invitation to maintainer group - DONE > >>> [Simon] New sequence numbers > >> > >> I can't see any issue with it on zynqmp platform. There is only issue > >> with i2c which was there for quite a long time that i2c mux is > >> considered as chip on the subbus as is visible below. > > > > Do you mean the naming of the device, or something else? What happens > > if you type 'i2c bus'? > > If look at log below. mux is at address 74 which is i2c_mux_bus_drv > and when you run i2c dev and run i2c probe i2c mux > itself is assigned to to i2c_generic_chip_drv. > Below you see it multiple times as generic_74 with ID 2/8/12/14/20. All > of them are just targeting i2c mux. That's strange. I wonder if it is failing to find it and creating multiple copies? +Heiko Schocher too Regards, Simon
Re: Contributor meeting notes 19-Jan-21
Hi, On 1/21/21 12:26 PM, Simon Glass wrote: > Hi Sean & Jaehoon, > > On Wed, 20 Jan 2021 at 16:25, Sean Anderson wrote: >> >> On 1/20/21 6:18 PM, Jaehoon Chung wrote: >>> Hi, >>> >>> On 1/20/21 3:54 AM, Simon Glass wrote: Hi, Thank you for attending! Full notes at [1] Tuesday 19 January 2021 Present: Daniel Schwierzeck, Heinrich Schuchardt, Michal Simek, Sean Anderson, Simon Glass, Walter Lozano Notes: [all] Introductions [all] Timing of call - Current time is the best across US, Europe, India. But not any good for East Asia >>> >>> In South Korea, it will be almost am 01:30. >>> Frankly, i can't speak English very well..If it doesn't matter, i hope to >>> attend sometime. :)\ >> >> One option could be to alternate times. E.g. one meeting pick a time >> best for one hemisphere, and another call pick a time best for the >> other. For example, the next meeting could be at 4:30 UTC (since we just >> met at 16:30 UTC) >> >> --Sean > > Yes I think that is the best option. I'll figure this out. > > Re speaking English, U-Boot is a global project so we have to > accommodate different people. We have live notes during the meeting so > if it is easier you can type things into the notes. Sometimes I find > that helps to get points across. It's helpful to me. :) Thaks! Best Regards, Jaehoon Chung > > Regards, > Simon > > >> >>> >>> Best Regards, >>> Jaehoon Chung >>> - Simon might send out survey - Send invitation to maintainer group - DONE [Simon] New sequence numbers [Heinrich] Online docs - https://protect2.fireeye.com/v1/url?k=93d7f10b-cc4cc86e-93d67a44-000babff32e3-8edc69528000b522&q=1&e=ae252977-3a7b-4c4a-ab06-1c4711467fc0&u=https%3A%2F%2Fu-boot.readthedocs.io%2F - Accept a merge request only with docs included? Next meeting Tuesday 2 Feb. [1] https://protect2.fireeye.com/v1/url?k=030f1752-5c942e37-030e9c1d-000babff32e3-e989b91a6d0ade29&q=1&e=ae252977-3a7b-4c4a-ab06-1c4711467fc0&u=https%3A%2F%2Fdocs.google.com%2Fdocument%2Fd%2F1YBOMsbM19uSFyoJWnt7-PsOLBaevzQUgV-hiR88a5-o%2Fedit%23 Regards, Simon >>> >> >
Re: [PATCH v2 2/2] pci: Add Rockchip dwc based PCIe controller driver
On 2021/1/15 下午6:01, Shawn Lin wrote: Add Rockchip dwc based PCIe controller driver for rk356x platform. Driver support Gen3 by operating as a Root complex. Signed-off-by: Shawn Lin Reviewed-by: Kever Yang Thanks, - Kever --- Changes in v2: - reorder the header file - add more comment - use clrsetbits_le32 and setbits_le32 - fix other various suggestions from Simon drivers/pci/Kconfig| 9 + drivers/pci/Makefile | 1 + drivers/pci/pcie_dw_rockchip.c | 877 + drivers/phy/rockchip/Kconfig | 3 + 4 files changed, 890 insertions(+) create mode 100644 drivers/pci/pcie_dw_rockchip.c diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig index 65498bce1d..b1de38f766 100644 --- a/drivers/pci/Kconfig +++ b/drivers/pci/Kconfig @@ -281,6 +281,15 @@ config PCIE_ROCKCHIP Say Y here if you want to enable PCIe controller support on Rockchip SoCs. +config PCIE_DW_ROCKCHIP + bool "Rockchip DesignWare based PCIe controller" + depends on ARCH_ROCKCHIP + select DM_PCI + select PHY_ROCKCHIP_SNPS_PCIE3 + help + Say Y here if you want to enable DW PCIe controller support on + Rockchip SoCs. + config PCI_BRCMSTB bool "Broadcom STB PCIe controller" depends on DM_PCI diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile index 8b4d49a590..5ed94bc95c 100644 --- a/drivers/pci/Makefile +++ b/drivers/pci/Makefile @@ -48,5 +48,6 @@ obj-$(CONFIG_PCIE_INTEL_FPGA) += pcie_intel_fpga.o obj-$(CONFIG_PCI_KEYSTONE) += pcie_dw_ti.o obj-$(CONFIG_PCIE_MEDIATEK) += pcie_mediatek.o obj-$(CONFIG_PCIE_ROCKCHIP) += pcie_rockchip.o +obj-$(CONFIG_PCIE_DW_ROCKCHIP) += pcie_dw_rockchip.o obj-$(CONFIG_PCI_BRCMSTB) += pcie_brcmstb.o obj-$(CONFIG_PCI_OCTEONTX) += pci_octeontx.o diff --git a/drivers/pci/pcie_dw_rockchip.c b/drivers/pci/pcie_dw_rockchip.c new file mode 100644 index 00..15270627b0 --- /dev/null +++ b/drivers/pci/pcie_dw_rockchip.c @@ -0,0 +1,877 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Rockchip DesignWare based PCIe host controller driver + * + * Copyright (c) 2021 Rockchip, Inc. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +DECLARE_GLOBAL_DATA_PTR; + +/** + * struct rk_pcie - RK DW PCIe controller state + * + * @vpcie3v3: The 3.3v power supply for slot + * @dbi_base: The base address of dwc core regs + * @apb_base: The base address of vendor regs + * @cfg_base: The base address of config header space + * @cfg_size: The size of the configuration space which is needed + *as it gets written into the PCIE_ATU_LIMIT register + * @first_busno: This driver supports multiple PCIe controllers. + * first_busno stores the bus number of the PCIe root-port + * number which may vary depending on the PCIe setup + * (PEX switches etc). + * @rst_gpio: The #PERST signal for slot + * @io:The IO space for EP's BAR + * @mem: The memory space for EP's BAR + */ +struct rk_pcie { + struct udevice *dev; + struct udevice *vpcie3v3; + void*dbi_base; + void*apb_base; + void*cfg_base; + fdt_size_t cfg_size; + struct phy phy; + struct clk_bulk clks; + int first_busno; + struct reset_ctl_bulk rsts; + struct gpio_descrst_gpio; + struct pci_region io; + struct pci_region mem; +}; + +/* Parameters for the waiting for iATU enabled routine */ +#define PCIE_CLIENT_GENERAL_DEBUG 0x104 +#define PCIE_CLIENT_HOT_RESET_CTRL 0x180 +#define PCIE_LTSSM_ENABLE_ENHANCE BIT(4) +#define PCIE_CLIENT_LTSSM_STATUS 0x300 +#define SMLH_LINKUPBIT(16) +#define RDLH_LINKUPBIT(17) +#define PCIE_CLIENT_DBG_FIFO_MODE_CON 0x310 +#define PCIE_CLIENT_DBG_FIFO_PTN_HIT_D0 0x320 +#define PCIE_CLIENT_DBG_FIFO_PTN_HIT_D1 0x324 +#define PCIE_CLIENT_DBG_FIFO_TRN_HIT_D0 0x328 +#define PCIE_CLIENT_DBG_FIFO_TRN_HIT_D1 0x32c +#define PCIE_CLIENT_DBG_FIFO_STATUS0x350 +#define PCIE_CLIENT_DBG_TRANSITION_DATA0x +#define PCIE_CLIENT_DBF_EN 0x0003 + +/* PCI DBICS registers */ +#define PCIE_LINK_STATUS_REG 0x80 +#define PCIE_LINK_STATUS_SPEED_OFF 16 +#define PCIE_LINK_STATUS_SPEED_MASK(0xf << PCIE_LINK_STATUS_SPEED_OFF) +#define PCIE_LINK_STATUS_WIDTH_OFF 20 +#define PCIE_LINK_STATUS_WIDTH_MASK(0xf << PCIE_LINK_STATUS_WIDTH_OFF) + +#define PCIE_LINK_CAPABILITY 0x7c +#define PCIE_LINK_CTL_20xa0 +#define TARGET_LINK_SPEED_MASK 0xf +#define LINK_SPEED_GEN_1 0x1 +#define LINK_SPEED_GEN_2 0x2 +#define LINK_SPEED_GEN_3 0x3 + +#define PCIE_MISC_CONTROL_1_OFF0x8bc +#de
Re: [PATCH v2 1/2] phy: rockchip: Add Rockchip Synopsys PCIe 3.0 PHY
On 2021/1/15 下午6:01, Shawn Lin wrote: Add the Rockchip Synopsys based PCIe 3.0 PHY driver as part of Generic PHY framework. Signed-off-by: Shawn Lin Reviewed-by: Simon Glass Reviewed-by: Kever Yang Thanks, - Kever --- Changes in v2: - reoder header file - add comment drivers/phy/rockchip/Kconfig | 6 + drivers/phy/rockchip/Makefile | 1 + .../phy/rockchip/phy-rockchip-snps-pcie3.c| 157 ++ 3 files changed, 164 insertions(+) create mode 100644 drivers/phy/rockchip/phy-rockchip-snps-pcie3.c diff --git a/drivers/phy/rockchip/Kconfig b/drivers/phy/rockchip/Kconfig index 2318e71f35..b794cdaf6a 100644 --- a/drivers/phy/rockchip/Kconfig +++ b/drivers/phy/rockchip/Kconfig @@ -18,6 +18,12 @@ config PHY_ROCKCHIP_PCIE help Enable this to support the Rockchip PCIe PHY. +config PHY_ROCKCHIP_SNPS_PCIE3 + bool "Rockchip Snps PCIe3 PHY Driver" + depends on PHY && ARCH_ROCKCHIP + help + Support for Rockchip PCIe3 PHY with Synopsys IP block. + config PHY_ROCKCHIP_TYPEC bool "Rockchip TYPEC PHY Driver" depends on ARCH_ROCKCHIP diff --git a/drivers/phy/rockchip/Makefile b/drivers/phy/rockchip/Makefile index 44049154f9..f6ad3bf59a 100644 --- a/drivers/phy/rockchip/Makefile +++ b/drivers/phy/rockchip/Makefile @@ -5,4 +5,5 @@ obj-$(CONFIG_PHY_ROCKCHIP_INNO_USB2) += phy-rockchip-inno-usb2.o obj-$(CONFIG_PHY_ROCKCHIP_PCIE) += phy-rockchip-pcie.o +obj-$(CONFIG_PHY_ROCKCHIP_SNPS_PCIE3) += phy-rockchip-snps-pcie3.o obj-$(CONFIG_PHY_ROCKCHIP_TYPEC) += phy-rockchip-typec.o diff --git a/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c b/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c new file mode 100644 index 00..5ae41fbeee --- /dev/null +++ b/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c @@ -0,0 +1,157 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Rockchip PCIE3.0 phy driver + * + * Copyright (C) 2021 Rockchip Electronics Co., Ltd. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define GRF_PCIE30PHY_CON1 0x4 +#define GRF_PCIE30PHY_CON6 0x18 +#define GRF_PCIE30PHY_CON9 0x24 + +/** + * struct rockchip_p3phy_priv - RK DW PCIe PHY state + * + * @mmio: The base address of PHY internal registers + * @phy_grf: The regmap for controlling pipe signal + * @p30phy: The reset signal for PHY + * @ref_clk_m: The reference clock of M for PHY + * @ref_clk_n: The reference clock of N for PHY + * @pclk: The clock for accessing PHY blocks + */ +struct rockchip_p3phy_priv { + void __iomem *mmio; + struct regmap *phy_grf; + struct reset_ctl p30phy; + struct clk ref_clk_m; + struct clk ref_clk_n; + struct clk pclk; +}; + +static int rochchip_p3phy_init(struct phy *phy) +{ + struct rockchip_p3phy_priv *priv = dev_get_priv(phy->dev); + int ret; + + ret = clk_enable(&priv->ref_clk_m); + if (ret < 0 && ret != -ENOSYS) + return ret; + + ret = clk_enable(&priv->ref_clk_n); + if (ret < 0 && ret != -ENOSYS) + goto err_ref; + + ret = clk_enable(&priv->pclk); + if (ret < 0 && ret != -ENOSYS) + goto err_pclk; + + reset_assert(&priv->p30phy); + udelay(1); + + /* Deassert PCIe PMA output clamp mode */ + regmap_write(priv->phy_grf, GRF_PCIE30PHY_CON9, +(0x1 << 15) | (0x1 << 31)); + + reset_deassert(&priv->p30phy); + udelay(1); + + return 0; +err_pclk: + clk_disable(&priv->ref_clk_n); +err_ref: + clk_disable(&priv->ref_clk_m); + + return ret; +} + +static int rochchip_p3phy_exit(struct phy *phy) +{ + struct rockchip_p3phy_priv *priv = dev_get_priv(phy->dev); + + clk_disable(&priv->ref_clk_m); + clk_disable(&priv->ref_clk_n); + clk_disable(&priv->pclk); + reset_assert(&priv->p30phy); + + return 0; +} + +static int rockchip_p3phy_probe(struct udevice *dev) +{ + struct rockchip_p3phy_priv *priv = dev_get_priv(dev); + struct udevice *syscon; + int ret; + + priv->mmio = (void __iomem *)dev_read_addr(dev); + if ((fdt_addr_t)priv->mmio == FDT_ADDR_T_NONE) + return -EINVAL; + + ret = uclass_get_device_by_phandle(UCLASS_SYSCON, dev, + "rockchip,phy-grf", &syscon); + if (ret) { + pr_err("unable to find syscon device for rockchip,phy-grf\n"); + return ret; + } + + priv->phy_grf = syscon_get_regmap(syscon); + if (IS_ERR(priv->phy_grf)) { + dev_err(dev, "failed to find rockchip,phy_grf regmap\n"); + return PTR_ERR(priv->phy_grf); + } + + ret = reset_get_by_name(dev, "phy", &priv->p30phy); + if (ret) { + dev_err(dev, "no phy reset control specified\n"); + return ret; + } +
Re: [PATCH v2] rockchip: rk3328: Add support for FriendlyARM NanoPi R2S
On 2021/1/7 上午7:06, David Bauer wrote: This adds support for the NanoPi R2S from FriendlyArm. Rockchip RK3328 SoC 1GB DDR4 RAM Gigabit Ethernet (WAN) Gigabit Ethernet (USB3) (LAN) USB 2.0 Host Port MicroSD slot Reset button WAN - LAN - SYS LED Signed-off-by: David Bauer Reviewed-by: Kever Yang Thanks, - Kever --- arch/arm/dts/Makefile | 1 + arch/arm/dts/rk3328-nanopi-r2s-u-boot.dtsi | 40 +++ arch/arm/dts/rk3328-nanopi-r2s.dts | 370 + board/rockchip/evb_rk3328/MAINTAINERS | 7 + configs/nanopi-r2s-rk3328_defconfig| 98 ++ 5 files changed, 516 insertions(+) create mode 100644 arch/arm/dts/rk3328-nanopi-r2s-u-boot.dtsi create mode 100644 arch/arm/dts/rk3328-nanopi-r2s.dts create mode 100644 configs/nanopi-r2s-rk3328_defconfig diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile index fd47e408f8..ea5b5586cc 100644 --- a/arch/arm/dts/Makefile +++ b/arch/arm/dts/Makefile @@ -110,6 +110,7 @@ dtb-$(CONFIG_ROCKCHIP_RK3308) += \ dtb-$(CONFIG_ROCKCHIP_RK3328) += \ rk3328-evb.dtb \ + rk3328-nanopi-r2s.dtb \ rk3328-roc-cc.dtb \ rk3328-rock64.dtb \ rk3328-rock-pi-e.dtb diff --git a/arch/arm/dts/rk3328-nanopi-r2s-u-boot.dtsi b/arch/arm/dts/rk3328-nanopi-r2s-u-boot.dtsi new file mode 100644 index 00..9e2ced1541 --- /dev/null +++ b/arch/arm/dts/rk3328-nanopi-r2s-u-boot.dtsi @@ -0,0 +1,40 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * (C) Copyright 2018-2019 Rockchip Electronics Co., Ltd + * (C) Copyright 2020 David Bauer + */ + +#include "rk3328-u-boot.dtsi" +#include "rk3328-sdram-ddr4-666.dtsi" +/ { + chosen { + u-boot,spl-boot-order = "same-as-spl", &sdmmc, &emmc; + }; +}; + +&gpio0 { + u-boot,dm-spl; +}; + +&pinctrl { + u-boot,dm-spl; +}; + +&sdmmc0m1_gpio { + u-boot,dm-spl; +}; + +&pcfg_pull_up_4ma { + u-boot,dm-spl; +}; + +/* Need this and all the pinctrl/gpio stuff above to set pinmux */ +&vcc_sd { + u-boot,dm-spl; +}; + +&gmac2io { + snps,reset-gpio = <&gpio1 RK_PC2 GPIO_ACTIVE_LOW>; + snps,reset-active-low; + snps,reset-delays-us = <0 1 5>; +}; diff --git a/arch/arm/dts/rk3328-nanopi-r2s.dts b/arch/arm/dts/rk3328-nanopi-r2s.dts new file mode 100644 index 00..5445c5cb3d --- /dev/null +++ b/arch/arm/dts/rk3328-nanopi-r2s.dts @@ -0,0 +1,370 @@ +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) +/* + * Copyright (c) 2020 David Bauer + */ + +/dts-v1/; + +#include +#include +#include "rk3328.dtsi" + +/ { + model = "FriendlyElec NanoPi R2S"; + compatible = "friendlyarm,nanopi-r2s", "rockchip,rk3328"; + + chosen { + stdout-path = "serial2:150n8"; + }; + + gmac_clk: gmac-clock { + compatible = "fixed-clock"; + clock-frequency = <12500>; + clock-output-names = "gmac_clkin"; + #clock-cells = <0>; + }; + + keys { + compatible = "gpio-keys"; + pinctrl-0 = <&reset_button_pin>; + pinctrl-names = "default"; + + reset { + label = "reset"; + gpios = <&gpio0 RK_PA0 GPIO_ACTIVE_LOW>; + linux,code = ; + debounce-interval = <50>; + }; + }; + + leds { + compatible = "gpio-leds"; + pinctrl-0 = <&lan_led_pin>, <&sys_led_pin>, <&wan_led_pin>; + pinctrl-names = "default"; + + lan_led: led-0 { + gpios = <&gpio2 RK_PB7 GPIO_ACTIVE_HIGH>; + label = "nanopi-r2s:green:lan"; + }; + + sys_led: led-1 { + gpios = <&gpio0 RK_PA2 GPIO_ACTIVE_HIGH>; + label = "nanopi-r2s:red:sys"; + }; + + wan_led: led-2 { + gpios = <&gpio2 RK_PC2 GPIO_ACTIVE_HIGH>; + label = "nanopi-r2s:green:wan"; + }; + }; + + vcc_io_sdio: sdmmcio-regulator { + compatible = "regulator-gpio"; + enable-active-high; + gpios = <&gpio1 RK_PD4 GPIO_ACTIVE_HIGH>; + pinctrl-0 = <&sdio_vcc_pin>; + pinctrl-names = "default"; + regulator-name = "vcc_io_sdio"; + regulator-always-on; + regulator-min-microvolt = <180>; + regulator-max-microvolt = <330>; + regulator-settling-time-us = <5000>; + regulator-type = "voltage"; + startup-delay-us = <2000>; + states = <180 0x1 + 330 0x0>; + vin-supply = <&vcc_io_33>; + }; + + vcc_sd: sdmmc-regulator { + compatible = "regulator-fixed"; + gpio = <&gpio0 RK_PD6 GPIO_ACTIVE_LOW>; +
RE: [PATCH 1/1] cmd/riscv/sbi: support System Reset Extension
>-Original Message- >From: Heinrich Schuchardt >Sent: 18 January 2021 02:57 >To: Rick Chen >Cc: Atish Patra ; Bin Meng ; >Pragnesh Patel ; u-boot@lists.denx.de; Heinrich >Schuchardt >Subject: [PATCH 1/1] cmd/riscv/sbi: support System Reset Extension > >[External Email] Do not click links or attachments unless you recognize the >sender and know the content is safe > >Let the sbi command detect the 'System Reset Extension' >(EID #0x53525354 "SRST"). > >Cf. https://github.com/riscv/riscv-sbi-doc > >Signed-off-by: Heinrich Schuchardt >--- > cmd/riscv/sbi.c | 1 + > 1 file changed, 1 insertion(+) Reviewed-by: Pragnesh Patel
Re: [PATCH] rockchip: pinebook-pro: fix SPI flash detection
On 2021/1/8 下午6:34, Marcin Juszkiewicz wrote: Copy changes done to rockpro64 in commit c180e2939d3ccb43f89565d6660a0d6f912712b6 ("rockchip: rockpro64: fix boot from SPI flash on spi1") Remove the spi0 alias, set the default bus for SPI flash to 1, and enable support for numbered aliases in SPL so that it uses the same bus numbering as U-Boot proper. This fixes detection of SPI flash on the pinebook-pro board. Signed-off-by: Marcin Juszkiewicz Reviewed-by: Kever Yang Thanks, - Kever --- arch/arm/dts/rk3399-pinebook-pro-u-boot.dtsi | 4 configs/pinebook-pro-rk3399_defconfig| 2 ++ 2 files changed, 2 insertions(+), 4 deletions(-) diff --git arch/arm/dts/rk3399-pinebook-pro-u-boot.dtsi arch/arm/dts/rk3399-pinebook-pro-u-boot.dtsi index ded7db0aef..ee3b98698e 100644 --- arch/arm/dts/rk3399-pinebook-pro-u-boot.dtsi +++ arch/arm/dts/rk3399-pinebook-pro-u-boot.dtsi @@ -7,10 +7,6 @@ #include "rk3399-sdram-lpddr4-100.dtsi" / { - aliases { - spi0 = &spi1; - }; - chosen { u-boot,spl-boot-order = "same-as-spl", &sdhci, &spiflash, &sdmmc; }; diff --git configs/pinebook-pro-rk3399_defconfig configs/pinebook-pro-rk3399_defconfig index 8fbd7280ac..a471c3e06a 100644 --- configs/pinebook-pro-rk3399_defconfig +++ configs/pinebook-pro-rk3399_defconfig @@ -37,6 +37,7 @@ CONFIG_SPL_OF_CONTROL=y CONFIG_OF_SPL_REMOVE_PROPS="pinctrl-0 pinctrl-names clock-names interrupt-parent assigned-clocks assigned-clock-rates assigned-clock-parents" CONFIG_ENV_IS_IN_SPI_FLASH=y CONFIG_SYS_RELOC_GD_ENV_ADDR=y +CONFIG_SPL_DM_SEQ_ALIAS=y CONFIG_ROCKCHIP_GPIO=y CONFIG_SYS_I2C_ROCKCHIP=y CONFIG_DM_KEYBOARD=y @@ -49,6 +50,7 @@ CONFIG_MMC_DW_ROCKCHIP=y CONFIG_MMC_SDHCI=y CONFIG_MMC_SDHCI_SDMA=y CONFIG_MMC_SDHCI_ROCKCHIP=y +CONFIG_SF_DEFAULT_BUS=1 CONFIG_SF_DEFAULT_SPEED=2000 CONFIG_SPI_FLASH_GIGADEVICE=y CONFIG_SPI_FLASH_WINBOND=y
Re: arm: rk3399: add support nanopi r4s
Hi Xiaobo, Please add commit message for this patch, eg. you can introduce this board. Does this board support by mainline kernel? If yes, please share the commit number in the commit message. Thanks, - Kever On 2021/1/19 下午9:30, alex tian wrote: From e4f4c74b1f0e2bb7205a7b083fab6faf0b65c1ba Mon Sep 17 00:00:00 2001 From: Xiaobo Tian Date: Sat, 26 Dec 2020 00:13:37 +0800 Subject: [PATCH] arm: rk3399: add support nanopi r4s Signed-off-by: Xiaobo Tian --- arch/arm/dts/Makefile | 1 + arch/arm/dts/rk3399-nanopi-r4s-u-boot.dtsi | 7 ++ arch/arm/dts/rk3399-nanopi-r4s.dts | 120 + configs/nanopi-r4s-rk3399_defconfig| 62 +++ 4 files changed, 190 insertions(+) create mode 100644 arch/arm/dts/rk3399-nanopi-r4s-u-boot.dtsi create mode 100644 arch/arm/dts/rk3399-nanopi-r4s.dts create mode 100644 configs/nanopi-r4s-rk3399_defconfig diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile index fd47e408f8..8d42c37fcb 100644 --- a/arch/arm/dts/Makefile +++ b/arch/arm/dts/Makefile @@ -92,6 +92,7 @@ dtb-$(CONFIG_ROCKCHIP_RK3288) += \ rk3288-evb.dtb \ rk3288-firefly.dtb \ rk3288-miqi.dtb \ + rk3399-nanopi-r4s.dtb \ rk3288-phycore-rdk.dtb \ rk3288-popmetal.dtb \ rk3288-rock2-square.dtb \ diff --git a/arch/arm/dts/rk3399-nanopi-r4s-u-boot.dtsi b/arch/arm/dts/rk3399-nanopi-r4s-u-boot.dtsi new file mode 100644 index 00..05f785e662 --- /dev/null +++ b/arch/arm/dts/rk3399-nanopi-r4s-u-boot.dtsi @@ -0,0 +1,7 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (C) 2020 Xiaobo + */ + +#include "rk3399-nanopi4-u-boot.dtsi" +#include "rk3399-sdram-lpddr4-100.dtsi" diff --git a/arch/arm/dts/rk3399-nanopi-r4s.dts b/arch/arm/dts/rk3399-nanopi-r4s.dts new file mode 100644 index 00..5c65447f0e --- /dev/null +++ b/arch/arm/dts/rk3399-nanopi-r4s.dts @@ -0,0 +1,120 @@ +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) +/* + * Copyright (c) 2020 FriendlyElec Computer Tech. Co., Ltd. + * (http://www.friendlyarm.com) + * + * Copyright (C) 2020 Xiaobo + */ + +/dts-v1/; +#include "rk3399-nanopi4.dtsi" + +/ { + model = "FriendlyElec NanoPi R4S"; + compatible = "friendlyarm,nanopi-r4s", "rockchip,rk3399"; + + vdd_5v: vdd-5v { + compatible = "regulator-fixed"; + regulator-name = "vdd_5v"; + regulator-always-on; + regulator-boot-on; + }; + + fan: pwm-fan { + compatible = "pwm-fan"; + /* FIXME: adjust leveles for the connected fan */ + cooling-levels = <0 12 18 255>; + #cooling-cells = <2>; + fan-supply = <&vdd_5v>; + pwms = <&pwm1 0 5 0>; + }; +}; + +&cpu_thermal { + trips { + cpu_warm: cpu_warm { + temperature = <55000>; + hysteresis = <2000>; + type = "active"; + }; + + cpu_hot: cpu_hot { + temperature = <65000>; + hysteresis = <2000>; + type = "active"; + }; + }; + + cooling-maps { + map2 { + trip = <&cpu_warm>; + cooling-device = <&fan THERMAL_NO_LIMIT 1>; + }; + + map3 { + trip = <&cpu_hot>; + cooling-device = <&fan 2 THERMAL_NO_LIMIT>; + }; + }; +}; + +&emmc_phy { + status = "disabled"; +}; + +&fusb0 { + status = "disabled"; +}; + +&leds { + lan_led: led-1 { + gpios = <&gpio1 RK_PA1 GPIO_ACTIVE_HIGH>; + label = "nanopi-r4s:green:lan"; + }; + + wan_led: led-2 { + gpios = <&gpio1 RK_PA0 GPIO_ACTIVE_HIGH>; + label = "nanopi-r4s:green:wan"; + }; +}; + +&leds_gpio { + rockchip,pins = + <0 RK_PB5 RK_FUNC_GPIO &pcfg_pull_none>, + <1 RK_PA0 RK_FUNC_GPIO &pcfg_pull_none>, + <1 RK_PA1 RK_FUNC_GPIO &pcfg_pull_none>; +}; + +&pcie0 { + max-link-speed = <1>; + num-lanes = <1>; + vpcie3v3-supply = <&vcc3v3_sys>; +}; + +&sdhci { + status = "disabled"; +}; + +&sdio0 { + status = "disabled"; +}; + +&sdmmc { + host-index-min = <1>; +}; + +&u2phy0_host { + phy-supply = <&vdd_5v>; +}; + +&u2phy1_host { + status = "disabled"; +}; + +&usbdrd_dwc3_0 { + dr_mode = "host"; +}; + +&vcc3v3_sys { + vin-supply = <&vcc5v0_sys>; +}; diff --git a/configs/nanopi-r4s-rk3399_defconfig b/configs/nanopi-r4s-rk3399_defconfig new file mode 100644 index 00..0a3c28b012 --- /dev/null +++ b/configs/nanopi-r4s-rk3399_defconfig @@ -0,0 +1,62 @@ +CONFIG_ARM=y +CONFIG_ARCH_ROCKCHIP=y +CONFIG_SYS_TEXT_BASE=0x0020 +CONFIG_ENV_OFFSET=0x3F8000 +CONFIG_ROCKCHIP_RK3399=y +CONFIG_TARGET_EVB_RK3399=y +CONFIG_NR_DRAM_BANKS=1 +CONFIG_DEBUG_UART_BASE=0xFF1A +CONFIG_DEBUG_UART_CLOCK=2400 +CONFIG_DEBUG_UART=y +CONFIG_DEFAULT_FDT_FILE="rockchip/rk3399-nanopi-r4s.dtb" +CONFIG_DISPLAY_BOARDINFO_LATE=y +# CONFIG_SPL_RAW_IMAGE_SUPPORT is not set +CONFIG_SPL_STACK_R=y +CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN=0x1 +CONFIG_TPL=y +CONFIG_CMD_BOOTZ=y +CONFIG_CMD_GPT=y +CONFIG_CMD_MMC=y +CONFIG_CMD_USB=y +# CONFIG_CMD_SETEXPR is not set +CONFIG_CMD_TIME=y +CONFIG_SPL_OF_CONTROL=y +CONFIG_DEFAULT_DEVICE_TREE="rk3399-nanopi-r4s" +CONFIG_OF_SPL_REMOVE_PROPS="pinctrl-0 pinctrl-names clock-names interrupt-parent assigned-clocks assigned-clock-rates assigned-clock-parents" +CONFIG_ENV_IS_IN_MMC=y +CONFIG_SYS_RELOC_GD_ENV_ADDR=y +CONFIG_ROCKCHIP_GPIO=y
Re: Boot failure triggered by USB on rockpro64-rk3399 and pinebook-pro-rk3399
Hi Vagrant, Do you know which version is the last version that works in this case? The firmware is from eMMC and it's wired for USB to affect the boot process. Thanks, - Kever On 2021/1/21 上午8:08, Vagrant Cascadian wrote: It seems rockpro64-rk3399 and pinebook-pro-rk3399 fail to boot when usb is started. It hangs indefinitely at: ## Flattened Device Tree blob at 01f0 Booting using the fdt blob at 0x1f0 I have observed this also using 2020.10 on rockpro64-rk3399, though on pinebook-pro-rk3399 usb does not work and so it basically avoids triggering the issue. Setting CONFIG_USE_PREBOOT=n in the config works around the problem, though obviously by breaking usb keyboard support or booting from USB devices. Related bugs in Debian and manjaro: https://bugs.debian.org/973323 https://bugs.debian.org/980434 https://gitlab.manjaro.org/manjaro-arm/packages/core/uboot-rockpro64/-/issues/4 Boot log: U-Boot 2021.01+dfsg-1 (Jan 17 2021 - 03:50:13 +) SoC: Rockchip rk3399 Reset cause: POR Model: Pine64 RockPro64 v2.1 DRAM: 3.9 GiB PMIC: RK808 MMC: mmc@fe31: 2, mmc@fe32: 1, sdhci@fe33: 0 Loading Environment from SPIFlash... SF: Detected gd25q128 with page size 256 Bytes, erase size 4 KiB, total 16 MiB *** Warning - bad CRC, using default environment In:serial Out: serial Err: serial Model: Pine64 RockPro64 v2.1 Net: eth0: ethernet@fe30 starting USB... Bus usb@fe38: USB EHCI 1.00 Bus usb@fe3a: USB OHCI 1.0 Bus usb@fe3c: USB EHCI 1.00 Bus usb@fe3e: USB OHCI 1.0 Bus dwc3: usb maximum-speed not found Register 2000140 NbrPorts 2 Starting the controller USB XHCI 1.10 scanning bus usb@fe38 for devices... 1 USB Device(s) found scanning bus usb@fe3a for devices... 1 USB Device(s) found scanning bus usb@fe3c for devices... 1 USB Device(s) found scanning bus usb@fe3e for devices... 1 USB Device(s) found scanning bus dwc3 for devices... 1 USB Device(s) found scanning usb for storage devices... 0 Storage Device(s) found Hit any key to stop autoboot: 0 => printenv preboot preboot=usb start => usb reset resetting USB... Bus usb@fe38: USB EHCI 1.00 Bus usb@fe3a: USB OHCI 1.0 Bus usb@fe3c: USB EHCI 1.00 Bus usb@fe3e: USB OHCI 1.0 Bus dwc3: usb maximum-speed not found Register 2000140 NbrPorts 2 Starting the controller USB XHCI 1.10 scanning bus usb@fe38 for devices... 1 USB Device(s) found scanning bus usb@fe3a for devices... 1 USB Device(s) found scanning bus usb@fe3c for devices... 1 USB Device(s) found scanning bus usb@fe3e for devices... 1 USB Device(s) found scanning bus dwc3 for devices... 1 USB Device(s) found scanning usb for storage devices... 0 Storage Device(s) found => boot Card did not respond to voltage select! : -110 switch to partitions #0, OK mmc1 is current device Scanning mmc 1:1... Found /extlinux/extlinux.conf Retrieving file: /extlinux/extlinux.conf 144 bytes read in 5 ms (27.3 KiB/s) 1: Debian-Installer Retrieving file: /initrd.gz 28995285 bytes read in 1287 ms (21.5 MiB/s) Retrieving file: /vmlinuz 26922864 bytes read in 1195 ms (21.5 MiB/s) Retrieving file: /dtbs/rockchip/rk3399-rockpro64.dtb 56849 bytes read in 13 ms (4.2 MiB/s) Moving Image from 0x208 to 0x220, end=3c5 ## Flattened Device Tree blob at 01f0 Booting using the fdt blob at 0x1f0 live well, vagrant
Re: [PATCH 01/12] x86: acpi_gpe: Update driver name to match devicetree
On Thu, Jan 21, 2021 at 11:26 AM Simon Glass wrote: > > Hi Bin, > > On Wed, 20 Jan 2021 at 20:19, Bin Meng wrote: > > > > Hi Simon, > > > > On Sun, Jan 17, 2021 at 5:54 AM Simon Glass wrote: > > > > > > Use a driver name in line with the compatible string so that of-platdata > > > can use this driver. > > > > > > Signed-off-by: Simon Glass > > > --- > > > > > > arch/x86/cpu/acpi_gpe.c | 6 -- > > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > > > diff --git a/arch/x86/cpu/acpi_gpe.c b/arch/x86/cpu/acpi_gpe.c > > > index 83128c33c2c..da01e71335f 100644 > > > --- a/arch/x86/cpu/acpi_gpe.c > > > +++ b/arch/x86/cpu/acpi_gpe.c > > > @@ -4,6 +4,8 @@ > > > * Written by Simon Glass > > > */ > > > > > > +#define LOG_CATEGORY UCLASS_IRQ > > > + > > > #include > > > #include > > > #include > > > @@ -102,8 +104,8 @@ static const struct udevice_id acpi_gpe_ids[] = { > > > { } > > > }; > > > > > > -U_BOOT_DRIVER(acpi_gpe_drv) = { > > > - .name = "acpi_gpe", > > > +U_BOOT_DRIVER(intel_acpi_gpe) = { > > > > Why adding an intel_ prefix? > > This is to match the compatible string intel,acpi-gpe so that > of--platdata works correctly. Ah, yes, so, Reviewed-by: Bin Meng
Re: [PATCH 03/12] x86: apl: Enhance debugging in the SPL loader
On Sun, Jan 17, 2021 at 5:54 AM Simon Glass wrote: > > Move to log_debug() and make use of the new SPL function to find the > text base. > > Signed-off-by: Simon Glass > --- > > arch/x86/cpu/apollolake/spl.c | 12 +++- > 1 file changed, 7 insertions(+), 5 deletions(-) > Reviewed-by: Bin Meng
Re: [PATCH 01/12] x86: acpi_gpe: Update driver name to match devicetree
Hi Bin, On Wed, 20 Jan 2021 at 20:19, Bin Meng wrote: > > Hi Simon, > > On Sun, Jan 17, 2021 at 5:54 AM Simon Glass wrote: > > > > Use a driver name in line with the compatible string so that of-platdata > > can use this driver. > > > > Signed-off-by: Simon Glass > > --- > > > > arch/x86/cpu/acpi_gpe.c | 6 -- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/arch/x86/cpu/acpi_gpe.c b/arch/x86/cpu/acpi_gpe.c > > index 83128c33c2c..da01e71335f 100644 > > --- a/arch/x86/cpu/acpi_gpe.c > > +++ b/arch/x86/cpu/acpi_gpe.c > > @@ -4,6 +4,8 @@ > > * Written by Simon Glass > > */ > > > > +#define LOG_CATEGORY UCLASS_IRQ > > + > > #include > > #include > > #include > > @@ -102,8 +104,8 @@ static const struct udevice_id acpi_gpe_ids[] = { > > { } > > }; > > > > -U_BOOT_DRIVER(acpi_gpe_drv) = { > > - .name = "acpi_gpe", > > +U_BOOT_DRIVER(intel_acpi_gpe) = { > > Why adding an intel_ prefix? This is to match the compatible string intel,acpi-gpe so that of--platdata works correctly. > > > + .name = "intel_acpi_gpe", > > .id = UCLASS_IRQ, > > .of_match = acpi_gpe_ids, > > .ops= &acpi_gpe_ops, > > -- Regards, Simon
Re: Contributor meeting notes 19-Jan-21
Hi Sean & Jaehoon, On Wed, 20 Jan 2021 at 16:25, Sean Anderson wrote: > > On 1/20/21 6:18 PM, Jaehoon Chung wrote: > > Hi, > > > > On 1/20/21 3:54 AM, Simon Glass wrote: > >> Hi, > >> > >> Thank you for attending! > >> > >> Full notes at [1] > >> > >> Tuesday 19 January 2021 > >> > >> Present: Daniel Schwierzeck, Heinrich Schuchardt, Michal Simek, Sean > >> Anderson, Simon Glass, Walter Lozano > >> > >> Notes: > >> [all] Introductions > >> [all] Timing of call > >> - Current time is the best across US, Europe, India. But not any good > >> for East Asia > > > > In South Korea, it will be almost am 01:30. > > Frankly, i can't speak English very well..If it doesn't matter, i hope to > > attend sometime. :)\ > > One option could be to alternate times. E.g. one meeting pick a time > best for one hemisphere, and another call pick a time best for the > other. For example, the next meeting could be at 4:30 UTC (since we just > met at 16:30 UTC) > > --Sean Yes I think that is the best option. I'll figure this out. Re speaking English, U-Boot is a global project so we have to accommodate different people. We have live notes during the meeting so if it is easier you can type things into the notes. Sometimes I find that helps to get points across. Regards, Simon > > > > > Best Regards, > > Jaehoon Chung > > > >> - Simon might send out survey > >> - Send invitation to maintainer group - DONE > >> [Simon] New sequence numbers > >> [Heinrich] Online docs > >> - > >> https://protect2.fireeye.com/v1/url?k=93d7f10b-cc4cc86e-93d67a44-000babff32e3-8edc69528000b522&q=1&e=ae252977-3a7b-4c4a-ab06-1c4711467fc0&u=https%3A%2F%2Fu-boot.readthedocs.io%2F > >> - Accept a merge request only with docs included? > >> > >> Next meeting Tuesday 2 Feb. > >> > >> [1] > >> https://protect2.fireeye.com/v1/url?k=030f1752-5c942e37-030e9c1d-000babff32e3-e989b91a6d0ade29&q=1&e=ae252977-3a7b-4c4a-ab06-1c4711467fc0&u=https%3A%2F%2Fdocs.google.com%2Fdocument%2Fd%2F1YBOMsbM19uSFyoJWnt7-PsOLBaevzQUgV-hiR88a5-o%2Fedit%23 > >> > >> Regards, > >> Simon > >> > > >
Re: [PATCH 1/1] cmd: change suppress newline in echo command
Hi Heinrich, On Wed, 20 Jan 2021 at 09:43, Heinrich Schuchardt wrote: > > By default the echo command emits its arguments followed by a line feed. > > If any of the arguments contains the sub-string "\c", the line feed is > suppressed. > > This does not match shells used in Linux and BSD where the first argument > has to be -n to suppress the line feed. > > The hush shell interferes with the parsing of backslashes. E.g. in the > following command line quadruple backslashes are required for suppressing > the line feed: > > for i in 1 2 3; do for j in 4 5; do echo c ${i}${j}; done; echo; done; > > To avoid unexpected behavior the patch changes echo to use -n as first > argument to suppress the line feed. > > Signed-off-by: Heinrich Schuchardt > --- > cmd/echo.c | 49 ++--- > 1 file changed, 18 insertions(+), 31 deletions(-) Reviewed-by: Simon Glass This could be a good oppty to add a test for the echo command.
Re: [PATCH 02/12] x86: spl: Add a function to find the text base
On Sun, Jan 17, 2021 at 5:54 AM Simon Glass wrote: > > It is useful to know the TEXT_BASE value for the image being loaded in > TPL/SPL. Add a new spl_get_image_text_base() function to handle this. > > Make use of this in the x86 SPL handler, instead of having the logic > there. > > Signed-off-by: Simon Glass > --- > > common/spl/spl.c | 6 ++ > include/spl.h| 10 ++ > 2 files changed, 16 insertions(+) > Reviewed-by: Bin Meng
Re: [PATCH 01/12] x86: acpi_gpe: Update driver name to match devicetree
Hi Simon, On Sun, Jan 17, 2021 at 5:54 AM Simon Glass wrote: > > Use a driver name in line with the compatible string so that of-platdata > can use this driver. > > Signed-off-by: Simon Glass > --- > > arch/x86/cpu/acpi_gpe.c | 6 -- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/cpu/acpi_gpe.c b/arch/x86/cpu/acpi_gpe.c > index 83128c33c2c..da01e71335f 100644 > --- a/arch/x86/cpu/acpi_gpe.c > +++ b/arch/x86/cpu/acpi_gpe.c > @@ -4,6 +4,8 @@ > * Written by Simon Glass > */ > > +#define LOG_CATEGORY UCLASS_IRQ > + > #include > #include > #include > @@ -102,8 +104,8 @@ static const struct udevice_id acpi_gpe_ids[] = { > { } > }; > > -U_BOOT_DRIVER(acpi_gpe_drv) = { > - .name = "acpi_gpe", > +U_BOOT_DRIVER(intel_acpi_gpe) = { Why adding an intel_ prefix? > + .name = "intel_acpi_gpe", > .id = UCLASS_IRQ, > .of_match = acpi_gpe_ids, > .ops= &acpi_gpe_ops, > -- Regards, Bin
Re: [PATCH 1/1] cmd: CMD_ACPI depends on ACPIGEN
On Thu, Jan 21, 2021 at 4:38 AM Heinrich Schuchardt wrote: > > Trying to compile qemu-x86_64_defconfig with CONFIG_CMD_ACPI=y and > CONFIG_ACPIGEN=n fails with > > ld.bfd: cmd/built-in.o: in function `do_acpi_items': > cmd/acpi.c:162: undefined reference to `acpi_dump_items' > > Add the missing configuration dependency. > > Signed-off-by: Heinrich Schuchardt > --- > cmd/Kconfig | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > Reviewed-by: Bin Meng
[PATCH v2 09/11] tpm: Add TPM2 support for read/write values
Implement this API function for TPM2. Signed-off-by: Simon Glass --- (no changes since v1) include/tpm-common.h | 3 ++ include/tpm-v2.h | 38 lib/tpm-v2.c | 84 lib/tpm_api.c| 4 +-- 4 files changed, 127 insertions(+), 2 deletions(-) diff --git a/include/tpm-common.h b/include/tpm-common.h index e29b10b1766..080183779b3 100644 --- a/include/tpm-common.h +++ b/include/tpm-common.h @@ -53,6 +53,8 @@ enum tpm_version { * @buf: Buffer used during the exchanges with the chip * @pcr_count: Number of PCR per bank * @pcr_select_min:Minimum size in bytes of the pcrSelect array + * @plat_hier_disabled:Platform hierarchy has been disabled (TPM is locked + * down until next reboot) */ struct tpm_chip_priv { enum tpm_version version; @@ -64,6 +66,7 @@ struct tpm_chip_priv { /* TPM v2 specific data */ uint pcr_count; uint pcr_select_min; + bool plat_hier_disabled; }; /** diff --git a/include/tpm-v2.h b/include/tpm-v2.h index 2766dc72a65..6a400771af1 100644 --- a/include/tpm-v2.h +++ b/include/tpm-v2.h @@ -240,6 +240,7 @@ enum tpm2_command_codes { TPM2_CC_HIERCHANGEAUTH = 0x0129, TPM2_CC_NV_DEFINE_SPACE = 0x012a, TPM2_CC_PCR_SETAUTHPOL = 0x012C, + TPM2_CC_NV_WRITE= 0x0137, TPM2_CC_DAM_RESET = 0x0139, TPM2_CC_DAM_PARAMETERS = 0x013A, TPM2_CC_NV_READ = 0x014E, @@ -354,6 +355,20 @@ enum { TPM_MAX_BUF_SIZE= 1260, }; +enum { + /* Secure storage for firmware settings */ + TPM_HT_PCR = 0, + TPM_HT_NV_INDEX, + TPM_HT_HMAC_SESSION, + TPM_HT_POLICY_SESSION, + + HR_SHIFT= 24, + HR_PCR = TPM_HT_PCR << HR_SHIFT, + HR_HMAC_SESSION = TPM_HT_HMAC_SESSION << HR_SHIFT, + HR_POLICY_SESSION = TPM_HT_POLICY_SESSION << HR_SHIFT, + HR_NV_INDEX = TPM_HT_NV_INDEX << HR_SHIFT, +}; + /** * Issue a TPM2_Startup command. * @@ -418,6 +433,29 @@ u32 tpm2_nv_define_space(struct udevice *dev, u32 space_index, u32 tpm2_pcr_extend(struct udevice *dev, u32 index, u32 algorithm, const u8 *digest, u32 digest_len); +/** + * Read data from the secure storage + * + * @devTPM device + * @index Index of data to read + * @data Place to put data + * @count Number of bytes of data + * @return code of the operation + */ +u32 tpm2_nv_read_value(struct udevice *dev, u32 index, void *data, u32 count); + +/** + * Write data to the secure storage + * + * @devTPM device + * @index Index of data to write + * @data Data to write + * @count Number of bytes of data + * @return code of the operation + */ +u32 tpm2_nv_write_value(struct udevice *dev, u32 index, const void *data, + u32 count); + /** * Issue a TPM2_PCR_Read command. * diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c index 87d5e126b06..40633b63a94 100644 --- a/lib/tpm-v2.c +++ b/lib/tpm-v2.c @@ -166,6 +166,90 @@ u32 tpm2_pcr_extend(struct udevice *dev, u32 index, u32 algorithm, return tpm_sendrecv_command(dev, command_v2, NULL, NULL); } +u32 tpm2_nv_read_value(struct udevice *dev, u32 index, void *data, u32 count) +{ + u8 command_v2[COMMAND_BUFFER_SIZE] = { + /* header 10 bytes */ + tpm_u16(TPM2_ST_SESSIONS), /* TAG */ + tpm_u32(10 + 8 + 4 + 9 + 4),/* Length */ + tpm_u32(TPM2_CC_NV_READ), /* Command code */ + + /* handles 8 bytes */ + tpm_u32(TPM2_RH_PLATFORM), /* Primary platform seed */ + tpm_u32(HR_NV_INDEX + index), /* Password authorisation */ + + /* AUTH_SESSION */ + tpm_u32(9), /* Authorization size */ + tpm_u32(TPM2_RS_PW),/* Session handle */ + tpm_u16(0), /* Size of */ + /* (if any) */ + 0, /* Attributes: Cont/Excl/Rst */ + tpm_u16(0), /* Size of */ + /* (if any) */ + + tpm_u16(count), /* Number of bytes */ + tpm_u16(0), /* Offset */ + }; + size_t response_len = COMMAND_BUFFER_SIZE; + u8 response[COMMAND_BUFFER_SIZE]; + int ret; + u16 tag; + u32 size, code; + + ret = tpm_sendrecv_command(dev, command_v2, response, &response_len); + if (ret) + return log_msg_ret("read", ret); + if (unpack_byte_string(response, response_len, "wdds", + 0, &tag, 2, &size, 6, &code, + 16, data, count)) +
[PATCH v2 08/11] tpm: Add an implementation of define_space
Add support for this so that the TPM can be set up for use with Chromium OS verified boot. Signed-off-by: Simon Glass --- (no changes since v1) include/tpm-v2.h | 18 ++ lib/tpm-v2.c | 44 2 files changed, 62 insertions(+) diff --git a/include/tpm-v2.h b/include/tpm-v2.h index fab6b86ca2f..2766dc72a65 100644 --- a/include/tpm-v2.h +++ b/include/tpm-v2.h @@ -238,6 +238,7 @@ enum tpm2_command_codes { TPM2_CC_CLEAR = 0x0126, TPM2_CC_CLEARCONTROL= 0x0127, TPM2_CC_HIERCHANGEAUTH = 0x0129, + TPM2_CC_NV_DEFINE_SPACE = 0x012a, TPM2_CC_PCR_SETAUTHPOL = 0x012C, TPM2_CC_DAM_RESET = 0x0139, TPM2_CC_DAM_PARAMETERS = 0x013A, @@ -386,6 +387,23 @@ u32 tpm2_self_test(struct udevice *dev, enum tpm2_yes_no full_test); u32 tpm2_clear(struct udevice *dev, u32 handle, const char *pw, const ssize_t pw_sz); +/** + * Issue a TPM_NV_DefineSpace command + * + * This allows a space to be defined with given attributes and policy + * + * @devTPM device + * @space_indexindex of the area + * @space_size size of area in bytes + * @nv_attributes TPM_NV_ATTRIBUTES of the area + * @nv_policy policy to use + * @nv_policy_size size of the policy + * @return return code of the operation + */ +u32 tpm2_nv_define_space(struct udevice *dev, u32 space_index, +size_t space_size, u32 nv_attributes, +const u8 *nv_policy, size_t nv_policy_size); + /** * Issue a TPM2_PCR_Extend command. * diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c index c4e869ec5b5..87d5e126b06 100644 --- a/lib/tpm-v2.c +++ b/lib/tpm-v2.c @@ -81,6 +81,50 @@ u32 tpm2_clear(struct udevice *dev, u32 handle, const char *pw, return tpm_sendrecv_command(dev, command_v2, NULL, NULL); } +u32 tpm2_nv_define_space(struct udevice *dev, u32 space_index, +size_t space_size, u32 nv_attributes, +const u8 *nv_policy, size_t nv_policy_size) +{ + uint offset = 10 + 8 + 9 + 14; + u8 command_v2[COMMAND_BUFFER_SIZE] = { + /* header 10 bytes */ + tpm_u16(TPM2_ST_SESSIONS), /* TAG */ + tpm_u32(offset + nv_policy_size),/* Length */ + tpm_u32(TPM2_CC_NV_DEFINE_SPACE),/* Command code */ + + /* handles 8 bytes */ + tpm_u32(TPM2_RH_PLATFORM), /* Primary platform seed */ + + /* session header 9 bytes */ + tpm_u32(9), /* Header size */ + tpm_u32(TPM2_RS_PW),/* Password authorisation */ + tpm_u16(0), /* nonce_size */ + 0, /* session_attrs */ + tpm_u16(0), /* auth_size */ + + /* message 14 bytes + policy */ + tpm_u16(12 + nv_policy_size), /* size */ + tpm_u32(space_index), + tpm_u16(TPM2_ALG_SHA256), + tpm_u32(nv_attributes), + tpm_u16(nv_policy_size), + /* nv_policy */ + }; + int ret; + + /* +* Fill the command structure starting from the first buffer: +* - the password (if any) +*/ + ret = pack_byte_string(command_v2, sizeof(command_v2), "s", + offset, nv_policy, nv_policy_size); + offset += nv_policy_size; + if (ret) + return TPM_LIB_ERROR; + + return tpm_sendrecv_command(dev, command_v2, NULL, NULL); +} + u32 tpm2_pcr_extend(struct udevice *dev, u32 index, u32 algorithm, const u8 *digest, u32 digest_len) { -- 2.30.0.296.g2bfb1c46d8-goog
[PATCH v2 11/11] tpm: Allow disabling platform hierarchy with TPM2
With TPM2 we don't actually lock the TPM once verified boot is finished. Instead we disable the platform hierarchy which serves the same purpose. Add an implementation of this so we can safely boot into the kernel. Signed-off-by: Simon Glass --- Changes in v2: - Add definition of TPM2_RC_NV_DEFINED return code include/tpm-v2.h | 13 + lib/tpm-v2.c | 35 +++ 2 files changed, 48 insertions(+) diff --git a/include/tpm-v2.h b/include/tpm-v2.h index 1ca1e7e2011..e18c8b1ccca 100644 --- a/include/tpm-v2.h +++ b/include/tpm-v2.h @@ -235,6 +235,7 @@ enum tpm2_handles { enum tpm2_command_codes { TPM2_CC_STARTUP = 0x0144, TPM2_CC_SELF_TEST = 0x0143, + TPM2_CC_HIER_CONTROL= 0x0121, TPM2_CC_CLEAR = 0x0126, TPM2_CC_CLEARCONTROL= 0x0127, TPM2_CC_HIERCHANGEAUTH = 0x0129, @@ -272,6 +273,7 @@ enum tpm2_return_codes { TPM2_RC_COMMAND_CODE= TPM2_RC_VER1 + 0x0043, TPM2_RC_AUTHSIZE= TPM2_RC_VER1 + 0x0044, TPM2_RC_AUTH_CONTEXT= TPM2_RC_VER1 + 0x0045, + TPM2_RC_NV_DEFINED = TPM2_RC_VER1 + 0x004c, TPM2_RC_NEEDS_TEST = TPM2_RC_VER1 + 0x0053, TPM2_RC_WARN= 0x0900, TPM2_RC_TESTING = TPM2_RC_WARN + 0x000A, @@ -582,4 +584,15 @@ u32 tpm2_get_random(struct udevice *dev, void *data, u32 count); */ u32 tpm2_write_lock(struct udevice *dev, u32 index); +/** + * Disable access to any platform data + * + * This can be called to close off access to the firmware data in the data, + * before calling the kernel. + * + * @devTPM device + * @return code of the operation + */ +u32 tpm2_disable_platform_hierarchy(struct udevice *dev); + #endif /* __TPM_V2_H */ diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c index 15e12bc2f41..8ef9a492423 100644 --- a/lib/tpm-v2.c +++ b/lib/tpm-v2.c @@ -621,3 +621,38 @@ u32 tpm2_write_lock(struct udevice *dev, u32 index) return tpm_sendrecv_command(dev, command_v2, NULL, NULL); } + +u32 tpm2_disable_platform_hierarchy(struct udevice *dev) +{ + struct tpm_chip_priv *priv = dev_get_uclass_priv(dev); + u8 command_v2[COMMAND_BUFFER_SIZE] = { + /* header 10 bytes */ + tpm_u16(TPM2_ST_SESSIONS), /* TAG */ + tpm_u32(10 + 4 + 13 + 5), /* Length */ + tpm_u32(TPM2_CC_HIER_CONTROL), /* Command code */ + + /* 4 bytes */ + tpm_u32(TPM2_RH_PLATFORM), /* Primary platform seed */ + + /* session header 9 bytes */ + tpm_u32(9), /* Header size */ + tpm_u32(TPM2_RS_PW),/* Password authorisation */ + tpm_u16(0), /* nonce_size */ + 0, /* session_attrs */ + tpm_u16(0), /* auth_size */ + + /* payload 5 bytes */ + tpm_u32(TPM2_RH_PLATFORM), /* Hierarchy to disable */ + 0, /* 0=disable */ + }; + int ret; + + ret = tpm_sendrecv_command(dev, command_v2, NULL, NULL); + log_info("ret=%s, %x\n", dev->name, ret); + if (ret) + return ret; + + priv->plat_hier_disabled = true; + + return 0; +} -- 2.30.0.296.g2bfb1c46d8-goog
[PATCH v2 10/11] tpm: Add TPM2 support for write_lock
Implement this API function for TPM2. Signed-off-by: Simon Glass --- (no changes since v1) include/tpm-v2.h | 12 lib/tpm-v2.c | 23 +++ lib/tpm_api.c| 2 +- 3 files changed, 36 insertions(+), 1 deletion(-) diff --git a/include/tpm-v2.h b/include/tpm-v2.h index 6a400771af1..1ca1e7e2011 100644 --- a/include/tpm-v2.h +++ b/include/tpm-v2.h @@ -241,6 +241,7 @@ enum tpm2_command_codes { TPM2_CC_NV_DEFINE_SPACE = 0x012a, TPM2_CC_PCR_SETAUTHPOL = 0x012C, TPM2_CC_NV_WRITE= 0x0137, + TPM2_CC_NV_WRITELOCK= 0x0138, TPM2_CC_DAM_RESET = 0x0139, TPM2_CC_DAM_PARAMETERS = 0x013A, TPM2_CC_NV_READ = 0x014E, @@ -570,4 +571,15 @@ u32 tpm2_pcr_setauthvalue(struct udevice *dev, const char *pw, */ u32 tpm2_get_random(struct udevice *dev, void *data, u32 count); +/** + * Lock data in the TPM + * + * Once locked the data cannot be written until after a reboot + * + * @devTPM device + * @index Index of data to lock + * @return code of the operation + */ +u32 tpm2_write_lock(struct udevice *dev, u32 index); + #endif /* __TPM_V2_H */ diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c index 40633b63a94..15e12bc2f41 100644 --- a/lib/tpm-v2.c +++ b/lib/tpm-v2.c @@ -598,3 +598,26 @@ u32 tpm2_get_random(struct udevice *dev, void *data, u32 count) return 0; } + +u32 tpm2_write_lock(struct udevice *dev, u32 index) +{ + u8 command_v2[COMMAND_BUFFER_SIZE] = { + /* header 10 bytes */ + tpm_u16(TPM2_ST_SESSIONS), /* TAG */ + tpm_u32(10 + 8 + 13), /* Length */ + tpm_u32(TPM2_CC_NV_WRITELOCK), /* Command code */ + + /* handles 8 bytes */ + tpm_u32(TPM2_RH_PLATFORM), /* Primary platform seed */ + tpm_u32(HR_NV_INDEX + index), /* Password authorisation */ + + /* session header 9 bytes */ + tpm_u32(9), /* Header size */ + tpm_u32(TPM2_RS_PW),/* Password authorisation */ + tpm_u16(0), /* nonce_size */ + 0, /* session_attrs */ + tpm_u16(0), /* auth_size */ + }; + + return tpm_sendrecv_command(dev, command_v2, NULL, NULL); +} diff --git a/lib/tpm_api.c b/lib/tpm_api.c index 687fc8bc7ee..4c662640a92 100644 --- a/lib/tpm_api.c +++ b/lib/tpm_api.c @@ -144,7 +144,7 @@ u32 tpm_write_lock(struct udevice *dev, u32 index) if (is_tpm1(dev)) return -ENOSYS; else if (is_tpm2(dev)) - return -ENOSYS; + return tpm2_write_lock(dev, index); else return -ENOSYS; } -- 2.30.0.296.g2bfb1c46d8-goog
[PATCH v2 07/11] tpm: Reduce duplication in a few functions
Update tpm2_clear() and tpm2_pcr_extend() so that the command size is not repeated twice. Add a small comment to the latter. Signed-off-by: Simon Glass --- Changes in v2: - Add comments for the offset value lib/tpm-v2.c | 13 - 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c index 1f3deb06e48..c4e869ec5b5 100644 --- a/lib/tpm-v2.c +++ b/lib/tpm-v2.c @@ -47,9 +47,11 @@ u32 tpm2_self_test(struct udevice *dev, enum tpm2_yes_no full_test) u32 tpm2_clear(struct udevice *dev, u32 handle, const char *pw, const ssize_t pw_sz) { + /* Length of the message header, up to start of password */ + uint offset = 27; u8 command_v2[COMMAND_BUFFER_SIZE] = { tpm_u16(TPM2_ST_SESSIONS), /* TAG */ - tpm_u32(27 + pw_sz),/* Length */ + tpm_u32(offset + pw_sz),/* Length */ tpm_u32(TPM2_CC_CLEAR), /* Command code */ /* HANDLE */ @@ -64,7 +66,6 @@ u32 tpm2_clear(struct udevice *dev, u32 handle, const char *pw, tpm_u16(pw_sz), /* Size of */ /* STRING(pw) (if any) */ }; - unsigned int offset = 27; int ret; /* @@ -83,9 +84,11 @@ u32 tpm2_clear(struct udevice *dev, u32 handle, const char *pw, u32 tpm2_pcr_extend(struct udevice *dev, u32 index, u32 algorithm, const u8 *digest, u32 digest_len) { + /* Length of the message header, up to start of digest */ + uint offset = 33; u8 command_v2[COMMAND_BUFFER_SIZE] = { tpm_u16(TPM2_ST_SESSIONS), /* TAG */ - tpm_u32(33 + digest_len), /* Length */ + tpm_u32(offset + digest_len), /* Length */ tpm_u32(TPM2_CC_PCR_EXTEND),/* Command code */ /* HANDLE */ @@ -99,11 +102,12 @@ u32 tpm2_pcr_extend(struct udevice *dev, u32 index, u32 algorithm, 0, /* Attributes: Cont/Excl/Rst */ tpm_u16(0), /* Size of */ /* (if any) */ + + /* hashes */ tpm_u32(1), /* Count (number of hashes) */ tpm_u16(algorithm), /* Algorithm of the hash */ /* STRING(digest) Digest */ }; - unsigned int offset = 33; int ret; /* @@ -112,7 +116,6 @@ u32 tpm2_pcr_extend(struct udevice *dev, u32 index, u32 algorithm, */ ret = pack_byte_string(command_v2, sizeof(command_v2), "s", offset, digest, digest_len); - offset += digest_len; if (ret) return TPM_LIB_ERROR; -- 2.30.0.296.g2bfb1c46d8-goog
[PATCH v2 05/11] tpm: Switch TPMv1 over to use the new API
Take over the plain 'tpm_...' functions for use by the new TPM API. Rename all the TPMv1 functions so they are called from the API. Update the TPMv1 functions so that they are called from the API. Change existing users to use the tpm1_ prefix so they don't need to go through the API, which might introduce uncertainty. Signed-off-by: Simon Glass --- (no changes since v1) board/gdsys/a38x/controlcenterdc.c| 4 +- board/gdsys/a38x/hre.c| 28 +++ board/gdsys/a38x/keyprogram.c | 8 +- board/gdsys/mpc8308/gazerbeam.c | 4 +- board/gdsys/p1022/controlcenterd-id.c | 36 cmd/tpm-v1.c | 25 +++--- cmd/tpm_test.c| 40 + include/tpm-v1.h | 76 - lib/Makefile | 1 + lib/tpm-v1.c | 115 -- 10 files changed, 168 insertions(+), 169 deletions(-) diff --git a/board/gdsys/a38x/controlcenterdc.c b/board/gdsys/a38x/controlcenterdc.c index a2287f9deb1..187ac8c4f9f 100644 --- a/board/gdsys/a38x/controlcenterdc.c +++ b/board/gdsys/a38x/controlcenterdc.c @@ -286,8 +286,8 @@ int last_stage_init(void) ccdc_eth_init(); #endif ret = get_tpm(&tpm); - if (ret || tpm_init(tpm) || tpm_startup(tpm, TPM_ST_CLEAR) || - tpm_continue_self_test(tpm)) { + if (ret || tpm_init(tpm) || tpm1_startup(tpm, TPM_ST_CLEAR) || + tpm1_continue_self_test(tpm)) { return 1; } diff --git a/board/gdsys/a38x/hre.c b/board/gdsys/a38x/hre.c index 699241b3e62..de5411a6b93 100644 --- a/board/gdsys/a38x/hre.c +++ b/board/gdsys/a38x/hre.c @@ -107,8 +107,8 @@ static int get_tpm_nv_size(struct udevice *tpm, uint32_t index, uint32_t *size) uint8_t *ptr; uint16_t v16; - err = tpm_get_capability(tpm, TPM_CAP_NV_INDEX, index, -info, sizeof(info)); + err = tpm1_get_capability(tpm, TPM_CAP_NV_INDEX, index, info, + sizeof(info)); if (err) { printf("tpm_get_capability(CAP_NV_INDEX, %08x) failed: %u\n", index, err); @@ -150,8 +150,8 @@ static int find_key(struct udevice *tpm, const uint8_t auth[20], unsigned int i; /* fetch list of already loaded keys in the TPM */ - err = tpm_get_capability(tpm, TPM_CAP_HANDLE, TPM_RT_KEY, buf, -sizeof(buf)); + err = tpm1_get_capability(tpm, TPM_CAP_HANDLE, TPM_RT_KEY, buf, + sizeof(buf)); if (err) return -1; key_count = get_unaligned_be16(buf); @@ -162,8 +162,8 @@ static int find_key(struct udevice *tpm, const uint8_t auth[20], /* now search a(/ the) key which we can access with the given auth */ for (i = 0; i < key_count; ++i) { buf_len = sizeof(buf); - err = tpm_get_pub_key_oiap(tpm, key_handles[i], auth, buf, - &buf_len); + err = tpm1_get_pub_key_oiap(tpm, key_handles[i], auth, buf, + &buf_len); if (err && err != TPM_AUTHFAIL) return -1; if (err) @@ -192,8 +192,8 @@ static int read_common_data(struct udevice *tpm) if (get_tpm_nv_size(tpm, NV_COMMON_DATA_INDEX, &size) || size < NV_COMMON_DATA_MIN_SIZE) return 1; - err = tpm_nv_read_value(tpm, NV_COMMON_DATA_INDEX, - buf, min(sizeof(buf), size)); + err = tpm1_nv_read_value(tpm, NV_COMMON_DATA_INDEX, buf, +min(sizeof(buf), size)); if (err) { printf("tpm_nv_read_value() failed: %u\n", err); return 1; @@ -270,8 +270,8 @@ static struct h_reg *access_hreg(struct udevice *tpm, uint8_t spec, if (mode & HREG_RD) { if (!result->valid) { if (IS_PCR_HREG(spec)) { - hre_tpm_err = tpm_pcr_read(tpm, HREG_IDX(spec), - result->digest, 20); + hre_tpm_err = tpm1_pcr_read(tpm, HREG_IDX(spec), + result->digest, 20); result->valid = (hre_tpm_err == TPM_SUCCESS); } else if (IS_FIX_HREG(spec)) { switch (HREG_IDX(spec)) { @@ -357,8 +357,8 @@ static int hre_op_loadkey(struct udevice *tpm, struct h_reg *src_reg, return -1; if (find_key(tpm, src_reg->digest, dst_reg->digest, &parent_handle)) return -1; - hre_tpm_err = tpm_load_key2_oiap(tpm, parent_handle, key, key_size, -src_reg->digest, &key_handle); + hre_tpm_err = tpm1_load_
[PATCH v2 06/11] tpm: Add a basic API implementation for TPMv2
Add support for TPMv2 versions of API functions. So far this is not complete as the standard is quite large, but it implements everything currently available for TPMv2 in U-Boot. Signed-off-by: Simon Glass --- (no changes since v1) lib/tpm_api.c | 84 ++- 1 file changed, 77 insertions(+), 7 deletions(-) diff --git a/lib/tpm_api.c b/lib/tpm_api.c index 758350bd18d..f1553512cc5 100644 --- a/lib/tpm_api.c +++ b/lib/tpm_api.c @@ -16,18 +16,41 @@ static bool is_tpm1(struct udevice *dev) return IS_ENABLED(CONFIG_TPM_V1) && tpm_get_version(dev) == TPM_V1; } +static bool is_tpm2(struct udevice *dev) +{ + return IS_ENABLED(CONFIG_TPM_V2) && tpm_get_version(dev) == TPM_V2; +} + u32 tpm_startup(struct udevice *dev, enum tpm_startup_type mode) { - if (is_tpm1(dev)) + if (is_tpm1(dev)) { return tpm1_startup(dev, mode); - else + } else if (is_tpm2(dev)) { + enum tpm2_startup_types type; + + switch (mode) { + case TPM_ST_CLEAR: + type = TPM2_SU_CLEAR; + break; + case TPM_ST_STATE: + type = TPM2_SU_STATE; + break; + default: + case TPM_ST_DEACTIVATED: + return -EINVAL; + } + return tpm2_startup(dev, type); + } else { return -ENOSYS; + } } u32 tpm_resume(struct udevice *dev) { if (is_tpm1(dev)) return tpm1_startup(dev, TPM_ST_STATE); + else if (is_tpm2(dev)) + return tpm2_startup(dev, TPM2_SU_STATE); else return -ENOSYS; } @@ -36,6 +59,8 @@ u32 tpm_self_test_full(struct udevice *dev) { if (is_tpm1(dev)) return tpm1_self_test_full(dev); + else if (is_tpm2(dev)) + return tpm2_self_test(dev, TPMI_YES); else return -ENOSYS; } @@ -44,6 +69,8 @@ u32 tpm_continue_self_test(struct udevice *dev) { if (is_tpm1(dev)) return tpm1_continue_self_test(dev); + else if (is_tpm2(dev)) + return tpm2_self_test(dev, TPMI_NO); else return -ENOSYS; } @@ -71,8 +98,6 @@ u32 tpm_clear_and_reenable(struct udevice *dev) log_err("TPM: Can't set deactivated state\n"); return ret; } - } else { - return -ENOSYS; } return TPM_SUCCESS; @@ -82,6 +107,8 @@ u32 tpm_nv_enable_locking(struct udevice *dev) { if (is_tpm1(dev)) return tpm1_nv_define_space(dev, TPM_NV_INDEX_LOCK, 0, 0); + else if (is_tpm2(dev)) + return -ENOSYS; else return -ENOSYS; } @@ -90,6 +117,8 @@ u32 tpm_nv_read_value(struct udevice *dev, u32 index, void *data, u32 count) { if (is_tpm1(dev)) return tpm1_nv_read_value(dev, index, data, count); + else if (is_tpm2(dev)) + return -ENOSYS; else return -ENOSYS; } @@ -99,6 +128,8 @@ u32 tpm_nv_write_value(struct udevice *dev, u32 index, const void *data, { if (is_tpm1(dev)) return tpm1_nv_write_value(dev, index, data, count); + else if (is_tpm2(dev)) + return -ENOSYS; else return -ENOSYS; } @@ -112,6 +143,8 @@ u32 tpm_write_lock(struct udevice *dev, u32 index) { if (is_tpm1(dev)) return -ENOSYS; + else if (is_tpm2(dev)) + return -ENOSYS; else return -ENOSYS; } @@ -121,6 +154,9 @@ u32 tpm_pcr_extend(struct udevice *dev, u32 index, const void *in_digest, { if (is_tpm1(dev)) return tpm1_extend(dev, index, in_digest, out_digest); + else if (is_tpm2(dev)) + return tpm2_pcr_extend(dev, index, TPM2_ALG_SHA256, in_digest, + TPM2_DIGEST_LEN); else return -ENOSYS; } @@ -129,6 +165,8 @@ u32 tpm_pcr_read(struct udevice *dev, u32 index, void *data, size_t count) { if (is_tpm1(dev)) return tpm1_pcr_read(dev, index, data, count); + else if (is_tpm2(dev)) + return -ENOSYS; else return -ENOSYS; } @@ -137,6 +175,13 @@ u32 tpm_tsc_physical_presence(struct udevice *dev, u16 presence) { if (is_tpm1(dev)) return tpm1_tsc_physical_presence(dev, presence); + + /* +* Nothing to do on TPM2 for this; use platform hierarchy availability +* instead. +*/ + else if (is_tpm2(dev)) + return 0; else return -ENOSYS; } @@ -145,6 +190,10 @@ u32 tpm_finalise_physical_presence(struct udevice *dev) { if (is_tpm1(dev)) return tpm1_finalise_physica
[PATCH v2 04/11] tpm: Add an API that can support v1.2 and v2
There are two different TPM standards. U-Boot supports both but each has its own set of functions. We really need a single TPM API that can call one or the other. This is not always possible as there are some differences between the two standards, but it is mostly possible. Add an API to handle this. So far it is not plumbed into the build and only supports TPMv1. Signed-off-by: Simon Glass --- (no changes since v1) include/tpm_api.h | 322 ++ lib/tpm_api.c | 215 +++ 2 files changed, 537 insertions(+) create mode 100644 include/tpm_api.h create mode 100644 lib/tpm_api.c diff --git a/include/tpm_api.h b/include/tpm_api.h new file mode 100644 index 000..f13d98cae47 --- /dev/null +++ b/include/tpm_api.h @@ -0,0 +1,322 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* + * Copyright (c) 2013 The Chromium OS Authors. + * Coypright (c) 2013 Guntermann & Drunck GmbH + */ + +#ifndef __TPM_API_H +#define __TPM_API_H + +#include +#include +#include + +/** + * Issue a TPM_Startup command. + * + * @param dev TPM device + * @param mode TPM startup mode + * @return return code of the operation + */ +u32 tpm_startup(struct udevice *dev, enum tpm_startup_type mode); + +/** + * Issue a TPM_SelfTestFull command. + * + * @param dev TPM device + * @return return code of the operation + */ +u32 tpm_self_test_full(struct udevice *dev); + +/** + * Issue a TPM_ContinueSelfTest command. + * + * @param dev TPM device + * @return return code of the operation + */ +u32 tpm_continue_self_test(struct udevice *dev); + +/** + * Issue a TPM_NV_DefineSpace command. The implementation is limited + * to specify TPM_NV_ATTRIBUTES and size of the area. The area index + * could be one of the special value listed in enum tpm_nv_index. + * + * @param dev TPM device + * @param indexindex of the area + * @param perm TPM_NV_ATTRIBUTES of the area + * @param size size of the area + * @return return code of the operation + */ +u32 tpm_nv_define_space(struct udevice *dev, u32 index, u32 perm, u32 size); + +/** + * Issue a TPM_NV_ReadValue command. This implementation is limited + * to read the area from offset 0. The area index could be one of + * the special value listed in enum tpm_nv_index. + * + * @param dev TPM device + * @param indexindex of the area + * @param data output buffer of the area contents + * @param countsize of output buffer + * @return return code of the operation + */ +u32 tpm_nv_read_value(struct udevice *dev, u32 index, void *data, u32 count); + +/** + * Issue a TPM_NV_WriteValue command. This implementation is limited + * to write the area from offset 0. The area index could be one of + * the special value listed in enum tpm_nv_index. + * + * @param dev TPM device + * @param indexindex of the area + * @param data input buffer to be wrote to the area + * @param length length of data bytes of input buffer + * @return return code of the operation + */ +u32 tpm_nv_write_value(struct udevice *dev, u32 index, const void *data, + u32 length); + +/** + * Issue a TPM_Extend command. + * + * @param dev TPM device + * @param indexindex of the PCR + * @param in_digest160-bit value representing the event to be + * recorded + * @param out_digest 160-bit PCR value after execution of the + * command + * @return return code of the operation + */ +u32 tpm_pcr_extend(struct udevice *dev, u32 index, const void *in_digest, + void *out_digest); + +/** + * Issue a TPM_PCRRead command. + * + * @param dev TPM device + * @param indexindex of the PCR + * @param data output buffer for contents of the named PCR + * @param countsize of output buffer + * @return return code of the operation + */ +u32 tpm_pcr_read(struct udevice *dev, u32 index, void *data, size_t count); + +/** + * Issue a TSC_PhysicalPresence command. TPM physical presence flag + * is bit-wise OR'ed of flags listed in enum tpm_physical_presence. + * + * @param dev TPM device + * @param presence TPM physical presence flag + * @return return code of the operation + */ +u32 tpm_tsc_physical_presence(struct udevice *dev, u16 presence); + +/** + * Issue a TPM_ReadPubek command. + * + * @param dev TPM device + * @param data output buffer for the public endorsement key + * @param countsize of output buffer + * @return return code of the operation + */ +u32 tpm_read_pubek(struct udevice *dev, void *data, size_t count); + +/** + * Issue a TPM_ForceClear command. + * + * @param dev TPM device + * @return return code of the operation + */ +u32 tpm_force_clear(struct udevice *dev); + +/** + * Issue a TPM_PhysicalEnable command. + * +
[PATCH v2 03/11] tpm: Add debugging of request in tpm_sendrecv_command()
The response is shown but not the request. Update the code to show both if debugging is enabled. Signed-off-by: Simon Glass --- (no changes since v1) lib/tpm-common.c | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/lib/tpm-common.c b/lib/tpm-common.c index e4af87f76aa..0255d3bd9cf 100644 --- a/lib/tpm-common.c +++ b/lib/tpm-common.c @@ -165,7 +165,7 @@ u32 tpm_sendrecv_command(struct udevice *dev, const void *command, int err, ret; u8 response_buffer[COMMAND_BUFFER_SIZE]; size_t response_length; - int i; + int i, size; if (response) { response_length = *size_ptr; @@ -174,8 +174,13 @@ u32 tpm_sendrecv_command(struct udevice *dev, const void *command, response_length = sizeof(response_buffer); } - err = tpm_xfer(dev, command, tpm_command_size(command), - response, &response_length); + size = tpm_command_size(command); + log_debug("TPM request [size:%d]: ", size); + for (i = 0; i < size; i++) + log_debug("%02x ", ((u8 *)command)[i]); + log_debug("\n"); + + err = tpm_xfer(dev, command, size, response, &response_length); if (err < 0) return err; -- 2.30.0.296.g2bfb1c46d8-goog
[PATCH v2 02/11] tpm: Use logging in the uclass
Update this to use log_debug() instead of the old debug(). Signed-off-by: Simon Glass --- (no changes since v1) drivers/tpm/tpm-uclass.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/tpm/tpm-uclass.c b/drivers/tpm/tpm-uclass.c index beb0fa3f93c..35774a6289e 100644 --- a/drivers/tpm/tpm-uclass.c +++ b/drivers/tpm/tpm-uclass.c @@ -4,6 +4,8 @@ * Written by Simon Glass */ +#define LOG_CATEGORY UCLASS_TPM + #include #include #include @@ -87,15 +89,15 @@ int tpm_xfer(struct udevice *dev, const uint8_t *sendbuf, size_t send_size, ordinal = get_unaligned_be32(sendbuf + TPM_CMD_ORDINAL_BYTE); if (count == 0) { - debug("no data\n"); + log_debug("no data\n"); return -ENODATA; } if (count > send_size) { - debug("invalid count value %x %zx\n", count, send_size); + log_debug("invalid count value %x %zx\n", count, send_size); return -E2BIG; } - debug("%s: Calling send\n", __func__); + log_debug("%s: Calling send\n", __func__); ret = ops->send(dev, sendbuf, send_size); if (ret < 0) return ret; -- 2.30.0.296.g2bfb1c46d8-goog
[PATCH v2 01/11] tpm: Don't include cr50 in TPL/SPL
At present the security chip is not used in these U-Boot phases. Update the Makefile to exclude it. Fix a few logging statements while we are here. Signed-off-by: Simon Glass --- (no changes since v1) drivers/tpm/Makefile | 2 +- drivers/tpm/cr50_i2c.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/tpm/Makefile b/drivers/tpm/Makefile index 8f075b9f45f..f64d20067f8 100644 --- a/drivers/tpm/Makefile +++ b/drivers/tpm/Makefile @@ -10,7 +10,7 @@ obj-$(CONFIG_TPM_TIS_SANDBOX) += tpm_tis_sandbox.o obj-$(CONFIG_TPM_ST33ZP24_I2C) += tpm_tis_st33zp24_i2c.o obj-$(CONFIG_TPM_ST33ZP24_SPI) += tpm_tis_st33zp24_spi.o -obj-$(CONFIG_TPM2_CR50_I2C) += cr50_i2c.o +obj-$(CONFIG_$(SPL_TPL_)TPM2_CR50_I2C) += cr50_i2c.o obj-$(CONFIG_TPM2_TIS_SANDBOX) += tpm2_tis_sandbox.o obj-$(CONFIG_TPM2_TIS_SPI) += tpm2_tis_spi.o obj-$(CONFIG_TPM2_FTPM_TEE) += tpm2_ftpm_tee.o diff --git a/drivers/tpm/cr50_i2c.c b/drivers/tpm/cr50_i2c.c index b103a6fdc39..76432bdec1f 100644 --- a/drivers/tpm/cr50_i2c.c +++ b/drivers/tpm/cr50_i2c.c @@ -309,7 +309,7 @@ static int cr50_i2c_recv(struct udevice *dev, u8 *buf, size_t buf_len) int status; int ret; - log_debug("%s: len=%x\n", __func__, buf_len); + log_debug("%s: buf_len=%x\n", __func__, buf_len); if (buf_len < TPM_HEADER_SIZE) return -E2BIG; @@ -386,7 +386,7 @@ static int cr50_i2c_send(struct udevice *dev, const u8 *buf, size_t len) ulong timeout; int ret; - log_debug("%s: len=%x\n", __func__, len); + log_debug("len=%x\n", len); timeout = timer_get_us() + TIMEOUT_LONG_US; do { ret = cr50_i2c_status(dev); -- 2.30.0.296.g2bfb1c46d8-goog
[PATCH v2 00/11] tpm: Support using TPM1 and TPM2 from a single API
At present if an application wants to be written so it can work with both TPMv1.2 and TPM2 it must use two different APIs. This is inconvenient since it requires adding code to deal with the mismatch between the two. It would be better to have a common API that all boards could share. This series provides a simple API the covers some basic features and implements them for both TPM standards. Changes in v2: - Add comments for the offset value - Add definition of TPM2_RC_NV_DEFINED return code Simon Glass (11): tpm: Don't include cr50 in TPL/SPL tpm: Use logging in the uclass tpm: Add debugging of request in tpm_sendrecv_command() tpm: Add an API that can support v1.2 and v2 tpm: Switch TPMv1 over to use the new API tpm: Add a basic API implementation for TPMv2 tpm: Reduce duplication in a few functions tpm: Add an implementation of define_space tpm: Add TPM2 support for read/write values tpm: Add TPM2 support for write_lock tpm: Allow disabling platform hierarchy with TPM2 board/gdsys/a38x/controlcenterdc.c| 4 +- board/gdsys/a38x/hre.c| 28 +-- board/gdsys/a38x/keyprogram.c | 8 +- board/gdsys/mpc8308/gazerbeam.c | 4 +- board/gdsys/p1022/controlcenterd-id.c | 36 +-- cmd/tpm-v1.c | 25 +- cmd/tpm_test.c| 40 ++-- drivers/tpm/Makefile | 2 +- drivers/tpm/cr50_i2c.c| 4 +- drivers/tpm/tpm-uclass.c | 8 +- include/tpm-common.h | 3 + include/tpm-v1.h | 76 +++--- include/tpm-v2.h | 81 +++ include/tpm_api.h | 322 ++ lib/Makefile | 1 + lib/tpm-common.c | 11 +- lib/tpm-v1.c | 115 + lib/tpm-v2.c | 199 +++- lib/tpm_api.c | 285 +++ 19 files changed, 1069 insertions(+), 183 deletions(-) create mode 100644 include/tpm_api.h create mode 100644 lib/tpm_api.c -- 2.30.0.296.g2bfb1c46d8-goog
Re: [PATCH v4 01/23] mips: dts: switch to board defines for dtb for mtmips
On Wed, 2021-01-20 at 16:51 +0100, Daniel Schwierzeck wrote: > Hi Weijie, > > Am Dienstag, den 19.01.2021, 08:58 +0800 schrieb Weijie Gao: > > > > > > > > Hi Daniel, > > > > > > > > Gentle ping > > > > > > > > I'm just curious when can the patch series be merged into u-boot/master? > > > > > > > > > > Hi Weijie, > > > > > > thanks for the reminder, I missed the opening of the merge window ;) > > > > > > I rebased the MIPS next branch and triggered all CI builds. If all is > > > ok, I'll prepare a pull request. > > > > > > > got it. thanks. > > in mainline there was a lot of renaming in DM core. I created fixup > commits in u-boot-mips/next to fix all build errors. Could you give it > short look and test? Thanks. > I have tested and all works fine. Regards
[PATCH v2 15/15] gpio: Add a way to read 3-way strapping pins
Using the internal vs. external pull resistors it is possible to get 27 different combinations from 3 strapping pins. Add an implementation of this. This involves updating the sandbox GPIO driver to model external and (weaker) internal pull resistors. The get_value() method now takes account of what is driving a pin: sandbox: GPIOD_EXT_DRIVEN - in which case GPIO_EXT_HIGH provides the value outside source - in which case GPIO_EXT_PULL_UP/DOWN indicates the external state and we work the final state using those flags and the internal GPIOD_PULL_UP/DOWN flags Of course the outside source does not really exist in sandbox. We are just modelling it for test purpose. Signed-off-by: Simon Glass --- (no changes since v1) arch/sandbox/include/asm/gpio.h | 5 +- drivers/gpio/gpio-uclass.c | 78 ++ drivers/gpio/sandbox.c | 13 +++-- include/asm-generic/gpio.h | 37 + test/dm/gpio.c | 98 + 5 files changed, 226 insertions(+), 5 deletions(-) diff --git a/arch/sandbox/include/asm/gpio.h b/arch/sandbox/include/asm/gpio.h index edf78cb4131..097abfb299c 100644 --- a/arch/sandbox/include/asm/gpio.h +++ b/arch/sandbox/include/asm/gpio.h @@ -26,8 +26,11 @@ /* Our own private GPIO flags, which musn't conflict with GPIOD_... */ #define GPIOD_EXT_HIGH BIT(20) /* external source is high (else low) */ #define GPIOD_EXT_DRIVEN BIT(21) /* external source is driven */ +#define GPIOD_EXT_PULL_UP BIT(22) /* GPIO has external pull-up */ +#define GPIOD_EXT_PULL_DOWNBIT(23) /* GPIO has external pull-down */ -#define GPIOD_SANDBOX_MASK GENMASK(21, 20) +#define GPIOD_EXT_PULL (BIT(21) | BIT(22)) +#define GPIOD_SANDBOX_MASK GENMASK(23, 20) /** * Return the simulated value of a GPIO (used only in sandbox test code) diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c index 2406947ed1c..7f358438527 100644 --- a/drivers/gpio/gpio-uclass.c +++ b/drivers/gpio/gpio-uclass.c @@ -3,6 +3,8 @@ * Copyright (c) 2013 Google, Inc */ +#define LOG_CATEGORY UCLASS_GPIO + #include #include #include @@ -20,6 +22,7 @@ #include #include #include +#include DECLARE_GLOBAL_DATA_PTR; @@ -708,6 +711,21 @@ int dm_gpio_set_dir_flags(struct gpio_desc *desc, ulong flags) return dm_gpio_clrset_flags(desc, 0, flags); } +int dm_gpios_clrset_flags(struct gpio_desc *desc, int count, ulong clr, + ulong set) +{ + int ret; + int i; + + for (i = 0; i < count; i++) { + ret = dm_gpio_clrset_flags(&desc[i], clr, set); + if (ret) + return log_ret(ret); + } + + return 0; +} + int dm_gpio_get_flags(struct gpio_desc *desc, ulong *flagsp) { struct udevice *dev = desc->dev; @@ -974,6 +992,66 @@ int dm_gpio_get_values_as_int(const struct gpio_desc *desc_list, int count) return vector; } +int dm_gpio_get_values_as_int_base3(struct gpio_desc *desc_list, + int count) +{ + static const char tristate[] = "01z"; + enum { + PULLUP, + PULLDOWN, + + NUM_OPTIONS, + }; + int vals[NUM_OPTIONS]; + uint mask; + uint vector = 0; + int ret, i; + + for (i = 0; i < NUM_OPTIONS; i++) { + uint flags = GPIOD_IS_IN; + + flags |= (i == PULLDOWN) ? GPIOD_PULL_DOWN : GPIOD_PULL_UP; + ret = dm_gpios_clrset_flags(desc_list, count, GPIOD_MASK_PULL, + flags); + if (ret) + return log_msg_ret("pu", ret); + + /* Give the lines time to settle */ + udelay(10); + + ret = dm_gpio_get_values_as_int(desc_list, count); + if (ret < 0) + return log_msg_ret("get1", ret); + vals[i] = ret; + } + + log_debug("values: %x %x, count = %d\n", vals[0], vals[1], count); + for (i = count - 1, mask = 1 << i; i >= 0; i--, mask >>= 1) { + uint pd = vals[PULLDOWN] & mask ? 1 : 0; + uint pu = vals[PULLUP] & mask ? 1 : 0; + uint digit; + + /* +* Get value with internal pulldown active. If this is 1 then +* there is a stronger external pullup, which we call 1. If not +* then call it 0. +*/ + digit = pd; + + /* +* If the values differ then the pin is floating so we call +* this a 2. +*/ + if (pu != pd) + digit |= 2; + log_debug("%c ", tristate[digit]); + vector = 3 * vector + digit; + } + log_debug("vector=%d\n", vector); + + return vector; +} + /** * gp
[PATCH v2 14/15] gpio: sandbox: Track whether a GPIO is driven
Add a new flag to keep track of whether sandbox is driving the pin, or whether it is expecting an input signal. If it is driving, then the value of the pin is the value being driven (0 or 1). If not driving, then we consider the value 0, since we don't currently handle things like pull-ups yet. Signed-off-by: Simon Glass --- (no changes since v1) arch/sandbox/include/asm/gpio.h | 3 ++- drivers/gpio/sandbox.c | 21 +++-- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/arch/sandbox/include/asm/gpio.h b/arch/sandbox/include/asm/gpio.h index 3f267797644..edf78cb4131 100644 --- a/arch/sandbox/include/asm/gpio.h +++ b/arch/sandbox/include/asm/gpio.h @@ -25,8 +25,9 @@ /* Our own private GPIO flags, which musn't conflict with GPIOD_... */ #define GPIOD_EXT_HIGH BIT(20) /* external source is high (else low) */ +#define GPIOD_EXT_DRIVEN BIT(21) /* external source is driven */ -#define GPIOD_SANDBOX_MASK BIT(20) +#define GPIOD_SANDBOX_MASK GENMASK(21, 20) /** * Return the simulated value of a GPIO (used only in sandbox test code) diff --git a/drivers/gpio/sandbox.c b/drivers/gpio/sandbox.c index b446d0dcc81..02ffc76bfed 100644 --- a/drivers/gpio/sandbox.c +++ b/drivers/gpio/sandbox.c @@ -76,16 +76,22 @@ static int set_gpio_flag(struct udevice *dev, unsigned int offset, ulong flag, int sandbox_gpio_get_value(struct udevice *dev, unsigned offset) { struct gpio_state *state = get_gpio_state(dev, offset); + bool val; if (get_gpio_flag(dev, offset, GPIOD_IS_OUT)) debug("sandbox_gpio: get_value on output gpio %u\n", offset); - return state->flags & GPIOD_EXT_HIGH ? true : false; + if (state->flags & GPIOD_EXT_DRIVEN) + val = state->flags & GPIOD_EXT_HIGH; + else + val = false; + + return val; } int sandbox_gpio_set_value(struct udevice *dev, unsigned offset, int value) { - set_gpio_flag(dev, offset, GPIOD_EXT_HIGH, value); + set_gpio_flag(dev, offset, GPIOD_EXT_DRIVEN | GPIOD_EXT_HIGH, value); return 0; } @@ -142,8 +148,8 @@ static int sb_gpio_direction_output(struct udevice *dev, unsigned offset, ret = sandbox_gpio_set_direction(dev, offset, 1); if (ret) return ret; - ret = set_gpio_flag(dev, offset, GPIOD_IS_OUT_ACTIVE | GPIOD_EXT_HIGH, - value); + ret = set_gpio_flag(dev, offset, GPIOD_IS_OUT_ACTIVE | + GPIOD_EXT_DRIVEN | GPIOD_EXT_HIGH, value); if (ret) return ret; @@ -171,8 +177,8 @@ static int sb_gpio_set_value(struct udevice *dev, unsigned offset, int value) return -1; } - ret = set_gpio_flag(dev, offset, GPIOD_IS_OUT_ACTIVE | GPIOD_EXT_HIGH, - value); + ret = set_gpio_flag(dev, offset, GPIOD_IS_OUT_ACTIVE | + GPIOD_EXT_DRIVEN | GPIOD_EXT_HIGH, value); if (ret) return ret; @@ -218,10 +224,13 @@ static int sb_gpio_set_flags(struct udevice *dev, unsigned int offset, struct gpio_state *state = get_gpio_state(dev, offset); if (flags & GPIOD_IS_OUT) { + flags |= GPIOD_EXT_DRIVEN; if (flags & GPIOD_IS_OUT_ACTIVE) flags |= GPIOD_EXT_HIGH; else flags &= ~GPIOD_EXT_HIGH; + } else { + flags |= state->flags & GPIOD_SANDBOX_MASK; } state->flags = flags; -- 2.30.0.296.g2bfb1c46d8-goog
[PATCH v2 10/15] dm: gpio: Add a way to update flags
It is convenient to be able to adjust some of the flags for a GPIO while leaving others alone. Add a function for this. Update dm_gpio_set_dir_flags() to make use of this. Also update dm_gpio_set_value() to use this also, since this allows the open-drain / open-source features to be implemented directly in the driver, rather than using the uclass workaround. Update the sandbox tests accordingly. This involves a lot of changes to dm_test_gpio_opendrain_opensource() since we no-longer have the direciion being reported differently depending on the open drain/open source flags. Signed-off-by: Simon Glass --- (no changes since v1) drivers/gpio/gpio-uclass.c | 65 ++- include/asm-generic/gpio.h | 25 test/dm/gpio.c | 125 - 3 files changed, 184 insertions(+), 31 deletions(-) diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c index 8931c420845..3492086725e 100644 --- a/drivers/gpio/gpio-uclass.c +++ b/drivers/gpio/gpio-uclass.c @@ -568,6 +568,7 @@ int dm_gpio_get_value(const struct gpio_desc *desc) int dm_gpio_set_value(const struct gpio_desc *desc, int value) { + const struct dm_gpio_ops *ops; int ret; ret = check_reserved(desc, "set_value"); @@ -577,21 +578,33 @@ int dm_gpio_set_value(const struct gpio_desc *desc, int value) if (desc->flags & GPIOD_ACTIVE_LOW) value = !value; + /* GPIOD_ are directly managed by driver in set_flags */ + ops = gpio_get_ops(desc->dev); + if (ops->set_flags) { + ulong flags = desc->flags; + + if (value) + flags |= GPIOD_IS_OUT_ACTIVE; + else + flags &= ~GPIOD_IS_OUT_ACTIVE; + return ops->set_flags(desc->dev, desc->offset, flags); + } + /* * Emulate open drain by not actively driving the line high or * Emulate open source by not actively driving the line low */ if ((desc->flags & GPIOD_OPEN_DRAIN && value) || (desc->flags & GPIOD_OPEN_SOURCE && !value)) - return gpio_get_ops(desc->dev)->direction_input(desc->dev, - desc->offset); + return ops->direction_input(desc->dev, desc->offset); else if (desc->flags & GPIOD_OPEN_DRAIN || desc->flags & GPIOD_OPEN_SOURCE) - return gpio_get_ops(desc->dev)->direction_output(desc->dev, - desc->offset, - value); + return ops->direction_output(desc->dev, desc->offset, value); + + ret = ops->set_value(desc->dev, desc->offset, value); + if (ret) + return ret; - gpio_get_ops(desc->dev)->set_value(desc->dev, desc->offset, value); return 0; } @@ -619,6 +632,17 @@ static int check_dir_flags(ulong flags) return 0; } +/** + * _dm_gpio_set_flags() - Send flags to the driver + * + * This uses the best available method to send the given flags to the driver. + * Note that if flags & GPIOD_ACTIVE_LOW, the driver sees the opposite value + * of GPIOD_IS_OUT_ACTIVE. + * + * @desc: GPIO description + * @flags: flags value to set + * @return 0 if OK, -ve on error + */ static int _dm_gpio_set_flags(struct gpio_desc *desc, ulong flags) { struct udevice *dev = desc->dev; @@ -637,6 +661,11 @@ static int _dm_gpio_set_flags(struct gpio_desc *desc, ulong flags) return ret; } + /* If active low, invert the output state */ + if ((flags & (GPIOD_IS_OUT | GPIOD_ACTIVE_LOW)) == + (GPIOD_IS_OUT | GPIOD_ACTIVE_LOW)) + flags ^= GPIOD_IS_OUT_ACTIVE; + /* GPIOD_ are directly managed by driver in set_flags */ if (ops->set_flags) { ret = ops->set_flags(dev, desc->offset, flags); @@ -649,26 +678,34 @@ static int _dm_gpio_set_flags(struct gpio_desc *desc, ulong flags) } } - /* save the flags also in descriptor */ - if (!ret) - desc->flags = flags; - return ret; } -int dm_gpio_set_dir_flags(struct gpio_desc *desc, ulong flags) +int dm_gpio_clrset_flags(struct gpio_desc *desc, ulong clr, ulong set) { + ulong flags; int ret; ret = check_reserved(desc, "set_dir_flags"); if (ret) return ret; - /* combine the requested flags (for IN/OUT) and the descriptor flags */ - flags |= desc->flags; + flags = (desc->flags & ~clr) | set; + ret = _dm_gpio_set_flags(desc, flags); + if (ret) + return ret; - return ret; + /* save the flags also in descriptor */ + desc->flags = flags; + + return 0; +} + +int dm_gpio_set_dir_flags(struct gpio_desc *desc, ulong flags)
[PATCH v2 13/15] gpio: x86: Drop the deprecated methods in intel_gpio
We don't need to implement direction_input() and direction_output() anymore. Drop them and use update_flags() instead. Signed-off-by: Simon Glass --- (no changes since v1) arch/x86/include/asm/intel_pinctrl_defs.h | 5 ++ drivers/gpio/intel_gpio.c | 72 --- 2 files changed, 43 insertions(+), 34 deletions(-) diff --git a/arch/x86/include/asm/intel_pinctrl_defs.h b/arch/x86/include/asm/intel_pinctrl_defs.h index 1ea141f082f..5d83d24bae2 100644 --- a/arch/x86/include/asm/intel_pinctrl_defs.h +++ b/arch/x86/include/asm/intel_pinctrl_defs.h @@ -11,6 +11,11 @@ /* This file is included by device trees, so avoid BIT() macros */ +#define GPIO_DW_SIZE(x)(sizeof(u32) * (x)) +#define PAD_CFG_OFFSET(x, dw_num) ((x) + GPIO_DW_SIZE(dw_num)) +#define PAD_CFG0_OFFSET(x) PAD_CFG_OFFSET(x, 0) +#define PAD_CFG1_OFFSET(x) PAD_CFG_OFFSET(x, 1) + #define PAD_CFG0_TX_STATE_BIT 0 #define PAD_CFG0_TX_STATE (1 << PAD_CFG0_TX_STATE_BIT) #define PAD_CFG0_RX_STATE_BIT 1 diff --git a/drivers/gpio/intel_gpio.c b/drivers/gpio/intel_gpio.c index eda95485c93..ab46a94dbc1 100644 --- a/drivers/gpio/intel_gpio.c +++ b/drivers/gpio/intel_gpio.c @@ -3,6 +3,8 @@ * Copyright 2019 Google LLC */ +#define LOG_CATEGORY UCLASS_GPIO + #include #include #include @@ -23,38 +25,6 @@ #include #include -static int intel_gpio_direction_input(struct udevice *dev, uint offset) -{ - struct udevice *pinctrl = dev_get_parent(dev); - uint config_offset; - - config_offset = intel_pinctrl_get_config_reg_offset(pinctrl, offset); - - pcr_clrsetbits32(pinctrl, config_offset, -PAD_CFG0_MODE_MASK | PAD_CFG0_TX_STATE | - PAD_CFG0_RX_DISABLE, -PAD_CFG0_MODE_GPIO | PAD_CFG0_TX_DISABLE); - - return 0; -} - -static int intel_gpio_direction_output(struct udevice *dev, uint offset, - int value) -{ - struct udevice *pinctrl = dev_get_parent(dev); - uint config_offset; - - config_offset = intel_pinctrl_get_config_reg_offset(pinctrl, offset); - - pcr_clrsetbits32(pinctrl, config_offset, -PAD_CFG0_MODE_MASK | PAD_CFG0_RX_STATE | - PAD_CFG0_TX_DISABLE | PAD_CFG0_TX_STATE, -PAD_CFG0_MODE_GPIO | PAD_CFG0_RX_DISABLE | - (value ? PAD_CFG0_TX_STATE : 0)); - - return 0; -} - static int intel_gpio_get_value(struct udevice *dev, uint offset) { struct udevice *pinctrl = dev_get_parent(dev); @@ -130,6 +100,41 @@ static int intel_gpio_xlate(struct udevice *orig_dev, struct gpio_desc *desc, return 0; } +static int intel_gpio_set_flags(struct udevice *dev, unsigned int offset, + ulong flags) +{ + struct udevice *pinctrl = dev_get_parent(dev); + u32 bic0 = 0, bic1 = 0; + u32 or0, or1; + uint config_offset; + + config_offset = intel_pinctrl_get_config_reg_offset(pinctrl, offset); + + if (flags & GPIOD_IS_OUT) { + bic0 |= PAD_CFG0_MODE_MASK | PAD_CFG0_RX_STATE | + PAD_CFG0_TX_DISABLE; + or0 |= PAD_CFG0_MODE_GPIO | PAD_CFG0_RX_DISABLE; + } else if (flags & GPIOD_IS_IN) { + bic0 |= PAD_CFG0_MODE_MASK | PAD_CFG0_TX_STATE | + PAD_CFG0_RX_DISABLE; + or0 |= PAD_CFG0_MODE_GPIO | PAD_CFG0_TX_DISABLE; + } + if (flags & GPIOD_PULL_UP) { + bic1 |= PAD_CFG1_PULL_MASK; + or1 |= PAD_CFG1_PULL_UP_20K; + } else if (flags & GPIOD_PULL_DOWN) { + bic1 |= PAD_CFG1_PULL_MASK; + or1 |= PAD_CFG1_PULL_DN_20K; + } + + pcr_clrsetbits32(pinctrl, PAD_CFG0_OFFSET(config_offset), bic0, or0); + pcr_clrsetbits32(pinctrl, PAD_CFG1_OFFSET(config_offset), bic1, or1); + log_debug("%s: flags=%lx, offset=%x, config_offset=%x, %x/%x %x/%x\n", + dev->name, flags, offset, config_offset, bic0, or0, bic1, or1); + + return 0; +} + #if CONFIG_IS_ENABLED(ACPIGEN) static int intel_gpio_get_acpi(const struct gpio_desc *desc, struct acpi_gpio *gpio) @@ -177,12 +182,11 @@ static int intel_gpio_of_to_plat(struct udevice *dev) } static const struct dm_gpio_ops gpio_intel_ops = { - .direction_input= intel_gpio_direction_input, - .direction_output = intel_gpio_direction_output, .get_value = intel_gpio_get_value, .set_value = intel_gpio_set_value, .get_function = intel_gpio_get_function, .xlate = intel_gpio_xlate, + .set_flags = intel_gpio_set_flags, #if CONFIG_IS_ENABLED(ACPIGEN) .get_acpi = intel_gpio_get_acpi, #endif
[PATCH v2 12/15] gpio: Use an 'ops' variable everywhere
Update this driver to use the common method of putting the driver operations in an 'ops' variable install of calling gpio_get_ops() repeatedly. Make it const since operations do not change. Signed-off-by: Simon Glass --- (no changes since v1) drivers/gpio/gpio-uclass.c | 25 ++--- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c index 352435e4f06..2406947ed1c 100644 --- a/drivers/gpio/gpio-uclass.c +++ b/drivers/gpio/gpio-uclass.c @@ -219,7 +219,7 @@ int gpio_xlate_offs_flags(struct udevice *dev, struct gpio_desc *desc, static int gpio_find_and_xlate(struct gpio_desc *desc, struct ofnode_phandle_args *args) { - struct dm_gpio_ops *ops = gpio_get_ops(desc->dev); + const struct dm_gpio_ops *ops = gpio_get_ops(desc->dev); if (ops->xlate) return ops->xlate(desc->dev, desc, args); @@ -352,6 +352,7 @@ int gpio_hog_lookup_name(const char *name, struct gpio_desc **desc) int dm_gpio_request(struct gpio_desc *desc, const char *label) { + const struct dm_gpio_ops *ops = gpio_get_ops(desc->dev); struct udevice *dev = desc->dev; struct gpio_dev_priv *uc_priv; char *str; @@ -363,8 +364,8 @@ int dm_gpio_request(struct gpio_desc *desc, const char *label) str = strdup(label); if (!str) return -ENOMEM; - if (gpio_get_ops(dev)->request) { - ret = gpio_get_ops(dev)->request(dev, desc->offset, label); + if (ops->request) { + ret = ops->request(dev, desc->offset, label); if (ret) { free(str); return ret; @@ -441,14 +442,15 @@ int gpio_requestf(unsigned gpio, const char *fmt, ...) int _dm_gpio_free(struct udevice *dev, uint offset) { + const struct dm_gpio_ops *ops = gpio_get_ops(dev); struct gpio_dev_priv *uc_priv; int ret; uc_priv = dev_get_uclass_priv(dev); if (!uc_priv->name[offset]) return -ENXIO; - if (gpio_get_ops(dev)->rfree) { - ret = gpio_get_ops(dev)->rfree(dev, offset); + if (ops->rfree) { + ret = ops->rfree(dev, offset); if (ret) return ret; } @@ -545,9 +547,10 @@ int gpio_direction_output(unsigned gpio, int value) static int _gpio_get_value(const struct gpio_desc *desc) { + const struct dm_gpio_ops *ops = gpio_get_ops(desc->dev); int value; - value = gpio_get_ops(desc->dev)->get_value(desc->dev, desc->offset); + value = ops->get_value(desc->dev, desc->offset); return desc->flags & GPIOD_ACTIVE_LOW ? !value : value; } @@ -643,7 +646,7 @@ static int check_dir_flags(ulong flags) static int _dm_gpio_set_flags(struct gpio_desc *desc, ulong flags) { struct udevice *dev = desc->dev; - struct dm_gpio_ops *ops = gpio_get_ops(dev); + const struct dm_gpio_ops *ops = gpio_get_ops(dev); struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev); int ret = 0; @@ -709,7 +712,7 @@ int dm_gpio_get_flags(struct gpio_desc *desc, ulong *flagsp) { struct udevice *dev = desc->dev; int ret, value; - struct dm_gpio_ops *ops = gpio_get_ops(dev); + const struct dm_gpio_ops *ops = gpio_get_ops(dev); ulong flags; ret = check_reserved(desc, "get_flags"); @@ -807,7 +810,7 @@ static int get_function(struct udevice *dev, int offset, bool skip_unused, const char **namep) { struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev); - struct dm_gpio_ops *ops = gpio_get_ops(dev); + const struct dm_gpio_ops *ops = gpio_get_ops(dev); BUILD_BUG_ON(GPIOF_COUNT != ARRAY_SIZE(gpio_function)); if (!device_active(dev)) @@ -844,7 +847,7 @@ int gpio_get_raw_function(struct udevice *dev, int offset, const char **namep) int gpio_get_status(struct udevice *dev, int offset, char *buf, int buffsize) { - struct dm_gpio_ops *ops = gpio_get_ops(dev); + const struct dm_gpio_ops *ops = gpio_get_ops(dev); struct gpio_dev_priv *priv; char *str = buf; int func; @@ -884,7 +887,7 @@ int gpio_get_status(struct udevice *dev, int offset, char *buf, int buffsize) #if CONFIG_IS_ENABLED(ACPIGEN) int gpio_get_acpi(const struct gpio_desc *desc, struct acpi_gpio *gpio) { - struct dm_gpio_ops *ops; + const struct dm_gpio_ops *ops; memset(gpio, '\0', sizeof(*gpio)); if (!dm_gpio_is_valid(desc)) { -- 2.30.0.296.g2bfb1c46d8-goog
[PATCH v2 11/15] gpio: Replace direction_input() and direction_output()
The new update_flags() method is more flexible since it allows the driver to see the full flags all at once. Use that in preference to these two functions. Add comments to that effect. Signed-off-by: Simon Glass --- (no changes since v1) drivers/gpio/gpio-uclass.c | 15 ++- include/asm-generic/gpio.h | 26 +- 2 files changed, 31 insertions(+), 10 deletions(-) diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c index 3492086725e..352435e4f06 100644 --- a/drivers/gpio/gpio-uclass.c +++ b/drivers/gpio/gpio-uclass.c @@ -512,13 +512,10 @@ int gpio_direction_input(unsigned gpio) int ret; ret = gpio_to_device(gpio, &desc); - if (ret) - return ret; - ret = check_reserved(&desc, "dir_input"); if (ret) return ret; - return gpio_get_ops(desc.dev)->direction_input(desc.dev, desc.offset); + return dm_gpio_clrset_flags(&desc, GPIOD_MASK_DIR, GPIOD_IS_IN); } /** @@ -533,17 +530,17 @@ int gpio_direction_input(unsigned gpio) int gpio_direction_output(unsigned gpio, int value) { struct gpio_desc desc; + ulong flags; int ret; ret = gpio_to_device(gpio, &desc); - if (ret) - return ret; - ret = check_reserved(&desc, "dir_output"); if (ret) return ret; - return gpio_get_ops(desc.dev)->direction_output(desc.dev, - desc.offset, value); + flags = GPIOD_IS_OUT; + if (value) + flags |= GPIOD_IS_OUT_ACTIVE; + return dm_gpio_clrset_flags(&desc, GPIOD_MASK_DIR, flags); } static int _gpio_get_value(const struct gpio_desc *desc) diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h index 947b65d9d0d..c0c3360aee4 100644 --- a/include/asm-generic/gpio.h +++ b/include/asm-generic/gpio.h @@ -266,10 +266,32 @@ int gpio_xlate_offs_flags(struct udevice *dev, struct gpio_desc *desc, struct dm_gpio_ops { int (*request)(struct udevice *dev, unsigned offset, const char *label); int (*rfree)(struct udevice *dev, unsigned int offset); + + /** +* direction_input() - deprecated +* +* Equivalent to set_flags(...GPIOD_IS_IN) +*/ int (*direction_input)(struct udevice *dev, unsigned offset); + + /** +* direction_output() - deprecated +* +* Equivalent to set_flags(...GPIOD_IS_OUT) with GPIOD_IS_OUT_ACTIVE +* also set if @value +*/ int (*direction_output)(struct udevice *dev, unsigned offset, int value); + int (*get_value)(struct udevice *dev, unsigned offset); + + /** +* set_value() - Sets the GPIO value of an output +* +* If the driver provides an @set_flags() method then that is used +* in preference to this, with GPIOD_IS_OUT_ACTIVE set according to +* @value. +*/ int (*set_value)(struct udevice *dev, unsigned offset, int value); /** * get_function() Get the GPIO function @@ -326,7 +348,9 @@ struct dm_gpio_ops { * uclass, so the driver always sees the value that should be set at the * pin (1=high, 0=low). * -* This method is optional. +* This method is required and should be implemented by new drivers. At +* some point, it will supersede direction_input() and +* direction_output(), which wil be removed. * * @dev:GPIO device * @offset: GPIO offset within that device -- 2.30.0.296.g2bfb1c46d8-goog
[PATCH v2 09/15] gpio: sandbox: Make sandbox_gpio_set_flags() set all flags
Allow this function to see all flags, including the internal sandbox ones. This allows the tests to fully control the behaviour of the driver. To make this work, move the setting of GPIOD_EXT_HIGH -to where the flags are updated via driver model, rather than the sandbox 'back door'. Signed-off-by: Simon Glass --- (no changes since v1) drivers/gpio/sandbox.c | 17 - 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/drivers/gpio/sandbox.c b/drivers/gpio/sandbox.c index ae9ebd141d9..b446d0dcc81 100644 --- a/drivers/gpio/sandbox.c +++ b/drivers/gpio/sandbox.c @@ -114,9 +114,7 @@ int sandbox_gpio_set_flags(struct udevice *dev, uint offset, ulong flags) { struct gpio_state *state = get_gpio_state(dev, offset); - if (flags & GPIOD_IS_OUT_ACTIVE) - flags |= GPIOD_EXT_HIGH; - state->flags = (state->flags & GPIOD_SANDBOX_MASK) | flags; + state->flags = flags; return 0; } @@ -217,14 +215,23 @@ static int sb_gpio_set_flags(struct udevice *dev, unsigned int offset, ulong flags) { debug("%s: offset:%u, flags = %lx\n", __func__, offset, flags); + struct gpio_state *state = get_gpio_state(dev, offset); + + if (flags & GPIOD_IS_OUT) { + if (flags & GPIOD_IS_OUT_ACTIVE) + flags |= GPIOD_EXT_HIGH; + else + flags &= ~GPIOD_EXT_HIGH; + } + state->flags = flags; - return sandbox_gpio_set_flags(dev, offset, flags); + return 0; } static int sb_gpio_get_flags(struct udevice *dev, uint offset, ulong *flagsp) { debug("%s: offset:%u\n", __func__, offset); - *flagsp = *get_gpio_flags(dev, offset); + *flagsp = *get_gpio_flags(dev, offset) & ~GPIOD_SANDBOX_MASK; return 0; } -- 2.30.0.296.g2bfb1c46d8-goog
[PATCH v2 06/15] gpio: sandbox: Rename GPIO dir_flags to flags
Adjust the terminology in this driver to reflect that fact that all flags are handled, not just direction flags. Create a new access function to get the full GPIO state, not just the direction flags. Drop the static invalid_dir_flags since we can rely on a segfault if something is wrong. Signed-off-by: Simon Glass --- Changes in v2: - Swap newf and flags in sb_gpio_set_flags() arch/sandbox/include/asm/gpio.h | 8 ++--- drivers/gpio/sandbox.c | 60 +++-- test/dm/gpio.c | 18 +- 3 files changed, 47 insertions(+), 39 deletions(-) diff --git a/arch/sandbox/include/asm/gpio.h b/arch/sandbox/include/asm/gpio.h index df4ba4fb5f3..20d78296551 100644 --- a/arch/sandbox/include/asm/gpio.h +++ b/arch/sandbox/include/asm/gpio.h @@ -69,17 +69,17 @@ int sandbox_gpio_set_direction(struct udevice *dev, unsigned int offset, * @param offset GPIO offset within bank * @return dir_flags: bitfield accesses by GPIOD_ defines */ -ulong sandbox_gpio_get_dir_flags(struct udevice *dev, unsigned int offset); +ulong sandbox_gpio_get_flags(struct udevice *dev, unsigned int offset); /** * Set the simulated flags of a GPIO (used only in sandbox test code) * * @param dev device to use * @param offset GPIO offset within bank - * @param flagsdir_flags: bitfield accesses by GPIOD_ defines + * @param flagsbitfield accesses by GPIOD_ defines * @return -1 on error, 0 if ok */ -int sandbox_gpio_set_dir_flags(struct udevice *dev, unsigned int offset, - ulong flags); +int sandbox_gpio_set_flags(struct udevice *dev, unsigned int offset, + ulong flags); #endif diff --git a/drivers/gpio/sandbox.c b/drivers/gpio/sandbox.c index 38dc34ee910..cc144bccbad 100644 --- a/drivers/gpio/sandbox.c +++ b/drivers/gpio/sandbox.c @@ -22,34 +22,44 @@ struct gpio_state { const char *label; /* label given by requester */ - ulong dir_flags;/* dir_flags (GPIOD_...) */ + ulong flags;/* flags (GPIOD_...) */ }; -/* Access routines for GPIO dir flags */ -static ulong *get_gpio_dir_flags(struct udevice *dev, unsigned int offset) +/* Access routines for GPIO info */ +static struct gpio_state *get_gpio_state(struct udevice *dev, uint offset) { struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev); struct gpio_state *state = dev_get_priv(dev); if (offset >= uc_priv->gpio_count) { - static ulong invalid_dir_flags; printf("sandbox_gpio: error: invalid gpio %u\n", offset); - return &invalid_dir_flags; + return NULL; } - return &state[offset].dir_flags; + return &state[offset]; +} + +/* Access routines for GPIO dir flags */ +static ulong *get_gpio_flags(struct udevice *dev, unsigned int offset) +{ + struct gpio_state *state = get_gpio_state(dev, offset); + + if (!state) + return NULL; + + return &state->flags; } static int get_gpio_flag(struct udevice *dev, unsigned int offset, ulong flag) { - return (*get_gpio_dir_flags(dev, offset) & flag) != 0; + return (*get_gpio_flags(dev, offset) & flag) != 0; } static int set_gpio_flag(struct udevice *dev, unsigned int offset, ulong flag, int value) { - ulong *gpio = get_gpio_dir_flags(dev, offset); + ulong *gpio = get_gpio_flags(dev, offset); if (value) *gpio |= flag; @@ -88,15 +98,14 @@ int sandbox_gpio_set_direction(struct udevice *dev, unsigned offset, int output) return 0; } -ulong sandbox_gpio_get_dir_flags(struct udevice *dev, unsigned int offset) +ulong sandbox_gpio_get_flags(struct udevice *dev, uint offset) { - return *get_gpio_dir_flags(dev, offset); + return *get_gpio_flags(dev, offset); } -int sandbox_gpio_set_dir_flags(struct udevice *dev, unsigned int offset, - ulong flags) +int sandbox_gpio_set_flags(struct udevice *dev, uint offset, ulong flags) { - *get_gpio_dir_flags(dev, offset) = flags; + *get_gpio_flags(dev, offset) = flags; return 0; } @@ -180,30 +189,29 @@ static int sb_gpio_xlate(struct udevice *dev, struct gpio_desc *desc, static int sb_gpio_set_flags(struct udevice *dev, unsigned int offset, ulong flags) { - ulong *dir_flags; + ulong *newf; - debug("%s: offset:%u, dir_flags = %lx\n", __func__, offset, flags); + debug("%s: offset:%u, flags = %lx\n", __func__, offset, flags); - dir_flags = get_gpio_dir_flags(dev, offset); + newf = get_gpio_flags(dev, offset); /* * For testing purposes keep the output value when switching to input. * This allows us to manipulate the input value via the gpio command. */ if (flags & GPIOD_IS_IN) - *dir_fla
[PATCH v2 08/15] gpio: sandbox: Fully separate pin value from output value
At present we have the concept of a pin's external value. This is what is used when getting the value of a pin. But we still set the GPIOD_IS_OUT_ACTIVE flag when changing the value. This is not actually correct, since if the pin changes from output to input, the external value need not change. Adjust the logic for this difference. Signed-off-by: Simon Glass --- (no changes since v1) drivers/gpio/sandbox.c | 24 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/drivers/gpio/sandbox.c b/drivers/gpio/sandbox.c index 86d9162aa75..ae9ebd141d9 100644 --- a/drivers/gpio/sandbox.c +++ b/drivers/gpio/sandbox.c @@ -85,7 +85,7 @@ int sandbox_gpio_get_value(struct udevice *dev, unsigned offset) int sandbox_gpio_set_value(struct udevice *dev, unsigned offset, int value) { - set_gpio_flag(dev, offset, GPIOD_IS_OUT_ACTIVE | GPIOD_EXT_HIGH, value); + set_gpio_flag(dev, offset, GPIOD_EXT_HIGH, value); return 0; } @@ -137,10 +137,19 @@ static int sb_gpio_direction_input(struct udevice *dev, unsigned offset) static int sb_gpio_direction_output(struct udevice *dev, unsigned offset, int value) { + int ret; + debug("%s: offset:%u, value = %d\n", __func__, offset, value); - return sandbox_gpio_set_direction(dev, offset, 1) | - sandbox_gpio_set_value(dev, offset, value); + ret = sandbox_gpio_set_direction(dev, offset, 1); + if (ret) + return ret; + ret = set_gpio_flag(dev, offset, GPIOD_IS_OUT_ACTIVE | GPIOD_EXT_HIGH, + value); + if (ret) + return ret; + + return 0; } /* read GPIO IN value of port 'offset' */ @@ -154,6 +163,8 @@ static int sb_gpio_get_value(struct udevice *dev, unsigned offset) /* write GPIO OUT value to port 'offset' */ static int sb_gpio_set_value(struct udevice *dev, unsigned offset, int value) { + int ret; + debug("%s: offset:%u, value = %d\n", __func__, offset, value); if (!sandbox_gpio_get_direction(dev, offset)) { @@ -162,7 +173,12 @@ static int sb_gpio_set_value(struct udevice *dev, unsigned offset, int value) return -1; } - return sandbox_gpio_set_value(dev, offset, value); + ret = set_gpio_flag(dev, offset, GPIOD_IS_OUT_ACTIVE | GPIOD_EXT_HIGH, + value); + if (ret) + return ret; + + return 0; } static int sb_gpio_get_function(struct udevice *dev, unsigned offset) -- 2.30.0.296.g2bfb1c46d8-goog
[PATCH v2 07/15] gpio: sandbox: Use a separate flag for the value
At present with the sandbox GPIO driver it is not possible to change the value of GPIOD_IS_OUT_ACTIVE unless the GPIO is an output. This makes it hard to test changing the flags since we need to be aware of the internal workings of the driver. The feature is designed to aid testing. Split this feature out into a separate sandbox-specific flag, so that the flags can change unimpeded. This will make it easier to allow updating the flags in a future patch. Signed-off-by: Simon Glass --- (no changes since v1) arch/sandbox/include/asm/gpio.h | 5 drivers/gpio/sandbox.c | 43 +++-- 2 files changed, 25 insertions(+), 23 deletions(-) diff --git a/arch/sandbox/include/asm/gpio.h b/arch/sandbox/include/asm/gpio.h index 20d78296551..3f267797644 100644 --- a/arch/sandbox/include/asm/gpio.h +++ b/arch/sandbox/include/asm/gpio.h @@ -23,6 +23,11 @@ */ #include +/* Our own private GPIO flags, which musn't conflict with GPIOD_... */ +#define GPIOD_EXT_HIGH BIT(20) /* external source is high (else low) */ + +#define GPIOD_SANDBOX_MASK BIT(20) + /** * Return the simulated value of a GPIO (used only in sandbox test code) * diff --git a/drivers/gpio/sandbox.c b/drivers/gpio/sandbox.c index cc144bccbad..86d9162aa75 100644 --- a/drivers/gpio/sandbox.c +++ b/drivers/gpio/sandbox.c @@ -59,12 +59,12 @@ static int get_gpio_flag(struct udevice *dev, unsigned int offset, ulong flag) static int set_gpio_flag(struct udevice *dev, unsigned int offset, ulong flag, int value) { - ulong *gpio = get_gpio_flags(dev, offset); + struct gpio_state *state = get_gpio_state(dev, offset); if (value) - *gpio |= flag; + state->flags |= flag; else - *gpio &= ~flag; + state->flags &= ~flag; return 0; } @@ -75,14 +75,19 @@ static int set_gpio_flag(struct udevice *dev, unsigned int offset, ulong flag, int sandbox_gpio_get_value(struct udevice *dev, unsigned offset) { + struct gpio_state *state = get_gpio_state(dev, offset); + if (get_gpio_flag(dev, offset, GPIOD_IS_OUT)) debug("sandbox_gpio: get_value on output gpio %u\n", offset); - return get_gpio_flag(dev, offset, GPIOD_IS_OUT_ACTIVE); + + return state->flags & GPIOD_EXT_HIGH ? true : false; } int sandbox_gpio_set_value(struct udevice *dev, unsigned offset, int value) { - return set_gpio_flag(dev, offset, GPIOD_IS_OUT_ACTIVE, value); + set_gpio_flag(dev, offset, GPIOD_IS_OUT_ACTIVE | GPIOD_EXT_HIGH, value); + + return 0; } int sandbox_gpio_get_direction(struct udevice *dev, unsigned offset) @@ -93,19 +98,25 @@ int sandbox_gpio_get_direction(struct udevice *dev, unsigned offset) int sandbox_gpio_set_direction(struct udevice *dev, unsigned offset, int output) { set_gpio_flag(dev, offset, GPIOD_IS_OUT, output); - set_gpio_flag(dev, offset, GPIOD_IS_IN, !(output)); + set_gpio_flag(dev, offset, GPIOD_IS_IN, !output); return 0; } ulong sandbox_gpio_get_flags(struct udevice *dev, uint offset) { - return *get_gpio_flags(dev, offset); + ulong flags = *get_gpio_flags(dev, offset); + + return flags & ~GPIOD_SANDBOX_MASK; } int sandbox_gpio_set_flags(struct udevice *dev, uint offset, ulong flags) { - *get_gpio_flags(dev, offset) = flags; + struct gpio_state *state = get_gpio_state(dev, offset); + + if (flags & GPIOD_IS_OUT_ACTIVE) + flags |= GPIOD_EXT_HIGH; + state->flags = (state->flags & GPIOD_SANDBOX_MASK) | flags; return 0; } @@ -189,23 +200,9 @@ static int sb_gpio_xlate(struct udevice *dev, struct gpio_desc *desc, static int sb_gpio_set_flags(struct udevice *dev, unsigned int offset, ulong flags) { - ulong *newf; - debug("%s: offset:%u, flags = %lx\n", __func__, offset, flags); - newf = get_gpio_flags(dev, offset); - - /* -* For testing purposes keep the output value when switching to input. -* This allows us to manipulate the input value via the gpio command. -*/ - if (flags & GPIOD_IS_IN) - *newf = (flags & ~GPIOD_IS_OUT_ACTIVE) | - (*newf & GPIOD_IS_OUT_ACTIVE); - else - *newf = flags; - - return 0; + return sandbox_gpio_set_flags(dev, offset, flags); } static int sb_gpio_get_flags(struct udevice *dev, uint offset, ulong *flagsp) -- 2.30.0.296.g2bfb1c46d8-goog
[PATCH v2 05/15] gpio: Drop dm_gpio_set_dir()
This function is not used. Drop it. Signed-off-by: Simon Glass --- (no changes since v1) drivers/gpio/gpio-uclass.c | 11 --- include/asm-generic/gpio.h | 11 --- 2 files changed, 22 deletions(-) diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c index 87254b0781b..8931c420845 100644 --- a/drivers/gpio/gpio-uclass.c +++ b/drivers/gpio/gpio-uclass.c @@ -671,17 +671,6 @@ int dm_gpio_set_dir_flags(struct gpio_desc *desc, ulong flags) return ret; } -int dm_gpio_set_dir(struct gpio_desc *desc) -{ - int ret; - - ret = check_reserved(desc, "set_dir"); - if (ret) - return ret; - - return _dm_gpio_set_flags(desc, desc->flags); -} - int dm_gpio_get_flags(struct gpio_desc *desc, ulong *flagsp) { struct udevice *dev = desc->dev; diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h index 4f8d1938da9..f7f10c469c0 100644 --- a/include/asm-generic/gpio.h +++ b/include/asm-generic/gpio.h @@ -657,17 +657,6 @@ int dm_gpio_get_value(const struct gpio_desc *desc); int dm_gpio_set_value(const struct gpio_desc *desc, int value); -/** - * dm_gpio_set_dir() - Set the direction for a GPIO - * - * This sets up the direction according to the GPIO flags: desc->flags. - * - * @desc: GPIO description containing device, offset and flags, - * previously returned by gpio_request_by_name() - * @return 0 if OK, -ve on error - */ -int dm_gpio_set_dir(struct gpio_desc *desc); - /** * dm_gpio_set_dir_flags() - Set direction using description and added flags * -- 2.30.0.296.g2bfb1c46d8-goog
[PATCH v2 03/15] dm: gpio: Rename get_dir_flags() method to get_flags()
It is more useful to be able to read all the flags, not just the direction ones. In fact this is what the STM32 driver does. Update the method name to reflect this. Tweak the docs a little and use 'flagsp' as the return argument, as is common in driver model, to indicate it returns a value. Signed-off-by: Simon Glass Reviewed-by: Pratyush Yadav --- (no changes since v1) drivers/gpio/gpio-uclass.c | 30 +++--- drivers/gpio/sandbox.c | 8 drivers/gpio/stm32_gpio.c | 8 drivers/pinctrl/pinctrl-stmfx.c | 8 include/asm-generic/gpio.h | 11 ++- 5 files changed, 33 insertions(+), 32 deletions(-) diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c index d59e5df4b4a..c5cb9b92b36 100644 --- a/drivers/gpio/gpio-uclass.c +++ b/drivers/gpio/gpio-uclass.c @@ -682,39 +682,39 @@ int dm_gpio_set_dir(struct gpio_desc *desc) return _dm_gpio_set_flags(desc, desc->flags); } -int dm_gpio_get_dir_flags(struct gpio_desc *desc, ulong *flags) +int dm_gpio_get_dir_flags(struct gpio_desc *desc, ulong *flagsp) { struct udevice *dev = desc->dev; int ret, value; struct dm_gpio_ops *ops = gpio_get_ops(dev); - ulong dir_flags; + ulong flags; - ret = check_reserved(desc, "get_dir_flags"); + ret = check_reserved(desc, "get_flags"); if (ret) return ret; /* GPIOD_ are directly provided by driver except GPIOD_ACTIVE_LOW */ - if (ops->get_dir_flags) { - ret = ops->get_dir_flags(dev, desc->offset, &dir_flags); + if (ops->get_flags) { + ret = ops->get_flags(dev, desc->offset, &flags); if (ret) return ret; /* GPIOD_ACTIVE_LOW is saved in desc->flags */ - value = dir_flags & GPIOD_IS_OUT_ACTIVE ? 1 : 0; + value = flags & GPIOD_IS_OUT_ACTIVE ? 1 : 0; if (desc->flags & GPIOD_ACTIVE_LOW) value = !value; - dir_flags &= ~(GPIOD_ACTIVE_LOW | GPIOD_IS_OUT_ACTIVE); - dir_flags |= (desc->flags & GPIOD_ACTIVE_LOW); + flags &= ~(GPIOD_ACTIVE_LOW | GPIOD_IS_OUT_ACTIVE); + flags |= (desc->flags & GPIOD_ACTIVE_LOW); if (value) - dir_flags |= GPIOD_IS_OUT_ACTIVE; + flags |= GPIOD_IS_OUT_ACTIVE; } else { - dir_flags = desc->flags; + flags = desc->flags; /* only GPIOD_IS_OUT_ACTIVE is provided by uclass */ - dir_flags &= ~GPIOD_IS_OUT_ACTIVE; + flags &= ~GPIOD_IS_OUT_ACTIVE; if ((desc->flags & GPIOD_IS_OUT) && _gpio_get_value(desc)) - dir_flags |= GPIOD_IS_OUT_ACTIVE; + flags |= GPIOD_IS_OUT_ACTIVE; } - *flags = dir_flags; + *flagsp = flags; return 0; } @@ -1309,8 +1309,8 @@ static int gpio_post_bind(struct udevice *dev) ops->xlate += gd->reloc_off; if (ops->set_flags) ops->set_flags += gd->reloc_off; - if (ops->get_dir_flags) - ops->get_dir_flags += gd->reloc_off; + if (ops->get_flags) + ops->get_flags += gd->reloc_off; reloc_done++; } diff --git a/drivers/gpio/sandbox.c b/drivers/gpio/sandbox.c index 05f17928f0a..38dc34ee910 100644 --- a/drivers/gpio/sandbox.c +++ b/drivers/gpio/sandbox.c @@ -199,11 +199,11 @@ static int sb_gpio_set_flags(struct udevice *dev, unsigned int offset, return 0; } -static int sb_gpio_get_dir_flags(struct udevice *dev, unsigned int offset, -ulong *flags) +static int sb_gpio_get_flags(struct udevice *dev, unsigned int offset, +ulong *flagsp) { debug("%s: offset:%u\n", __func__, offset); - *flags = *get_gpio_dir_flags(dev, offset); + *flagsp = *get_gpio_dir_flags(dev, offset); return 0; } @@ -273,7 +273,7 @@ static const struct dm_gpio_ops gpio_sandbox_ops = { .get_function = sb_gpio_get_function, .xlate = sb_gpio_xlate, .set_flags = sb_gpio_set_flags, - .get_dir_flags = sb_gpio_get_dir_flags, + .get_flags = sb_gpio_get_flags, #if CONFIG_IS_ENABLED(ACPIGEN) .get_acpi = sb_gpio_get_acpi, #endif diff --git a/drivers/gpio/stm32_gpio.c b/drivers/gpio/stm32_gpio.c index 6d1bef63c36..c2d7046c0dd 100644 --- a/drivers/gpio/stm32_gpio.c +++ b/drivers/gpio/stm32_gpio.c @@ -223,8 +223,8 @@ static int stm32_gpio_set_flags(struct udevice *dev, unsigned int offset, return 0; } -static int stm32_gpio_get_dir_flags(struct udevice *dev, unsigned int offset, - ulong *flags) +stati
[PATCH v2 04/15] gpio: Rename dm_gpio_get_dir_flags() to dm_gpio_get_flags()
This function can be used to get any flags, not just direction flags. Rename it to avoid confusion. Signed-off-by: Simon Glass Reviewed-by: Pratyush Yadav --- (no changes since v1) drivers/gpio/gpio-uclass.c | 2 +- include/asm-generic/gpio.h | 6 +++--- test/dm/gpio.c | 12 ++-- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c index c5cb9b92b36..87254b0781b 100644 --- a/drivers/gpio/gpio-uclass.c +++ b/drivers/gpio/gpio-uclass.c @@ -682,7 +682,7 @@ int dm_gpio_set_dir(struct gpio_desc *desc) return _dm_gpio_set_flags(desc, desc->flags); } -int dm_gpio_get_dir_flags(struct gpio_desc *desc, ulong *flagsp) +int dm_gpio_get_flags(struct gpio_desc *desc, ulong *flagsp) { struct udevice *dev = desc->dev; int ret, value; diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h index 153312d8af4..4f8d1938da9 100644 --- a/include/asm-generic/gpio.h +++ b/include/asm-generic/gpio.h @@ -683,16 +683,16 @@ int dm_gpio_set_dir(struct gpio_desc *desc); int dm_gpio_set_dir_flags(struct gpio_desc *desc, ulong flags); /** - * dm_gpio_get_dir_flags() - Get direction flags + * dm_gpio_get_flags() - Get flags * - * read the current direction flags + * Read the current flags * * @desc: GPIO description containing device, offset and flags, * previously returned by gpio_request_by_name() * @flags: place to put the used flags * @return 0 if OK, -ve on error, in which case desc->flags is not updated */ -int dm_gpio_get_dir_flags(struct gpio_desc *desc, ulong *flags); +int dm_gpio_get_flags(struct gpio_desc *desc, ulong *flags); /** * gpio_get_number() - Get the global GPIO number of a GPIO diff --git a/test/dm/gpio.c b/test/dm/gpio.c index 36bbaedb01c..c583d2b3447 100644 --- a/test/dm/gpio.c +++ b/test/dm/gpio.c @@ -397,22 +397,22 @@ static int dm_test_gpio_get_dir_flags(struct unit_test_state *uts) ut_asserteq(6, gpio_request_list_by_name(dev, "test3-gpios", desc_list, ARRAY_SIZE(desc_list), 0)); - ut_assertok(dm_gpio_get_dir_flags(&desc_list[0], &flags)); + ut_assertok(dm_gpio_get_flags(&desc_list[0], &flags)); ut_asserteq(GPIOD_IS_OUT | GPIOD_OPEN_DRAIN, flags); - ut_assertok(dm_gpio_get_dir_flags(&desc_list[1], &flags)); + ut_assertok(dm_gpio_get_flags(&desc_list[1], &flags)); ut_asserteq(GPIOD_IS_OUT | GPIOD_OPEN_SOURCE, flags); - ut_assertok(dm_gpio_get_dir_flags(&desc_list[2], &flags)); + ut_assertok(dm_gpio_get_flags(&desc_list[2], &flags)); ut_asserteq(GPIOD_IS_OUT, flags); - ut_assertok(dm_gpio_get_dir_flags(&desc_list[3], &flags)); + ut_assertok(dm_gpio_get_flags(&desc_list[3], &flags)); ut_asserteq(GPIOD_IS_IN | GPIOD_PULL_UP, flags); - ut_assertok(dm_gpio_get_dir_flags(&desc_list[4], &flags)); + ut_assertok(dm_gpio_get_flags(&desc_list[4], &flags)); ut_asserteq(GPIOD_IS_IN | GPIOD_PULL_DOWN, flags); - ut_assertok(dm_gpio_get_dir_flags(&desc_list[5], &flags)); + ut_assertok(dm_gpio_get_flags(&desc_list[5], &flags)); ut_asserteq(GPIOD_IS_IN, flags); ut_assertok(gpio_free_list(dev, desc_list, 6)); -- 2.30.0.296.g2bfb1c46d8-goog
[PATCH v2 02/15] dm: gpio: Rename set_dir_flags() method to update_flags()
The current method is a misnomer since it is also used (e.g. by stm32) to update pull settings and open source/open drain. Rename it and expand the documentation to cover a few more details. Signed-off-by: Simon Glass Reviewed-by: Pratyush Yadav --- Changes in v2: - Use set_flags() instead of update_flags() - Fix 'provide' typo while we are here - Make operation of set_flags() deterministic drivers/gpio/gpio-uclass.c | 16 drivers/gpio/sandbox.c | 6 +++--- drivers/gpio/stm32_gpio.c | 6 +++--- drivers/pinctrl/pinctrl-stmfx.c | 6 +++--- include/asm-generic/gpio.h | 28 ++-- test/dm/gpio.c | 8 6 files changed, 43 insertions(+), 27 deletions(-) diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c index e84b68db772..d59e5df4b4a 100644 --- a/drivers/gpio/gpio-uclass.c +++ b/drivers/gpio/gpio-uclass.c @@ -619,7 +619,7 @@ static int check_dir_flags(ulong flags) return 0; } -static int _dm_gpio_set_dir_flags(struct gpio_desc *desc, ulong flags) +static int _dm_gpio_set_flags(struct gpio_desc *desc, ulong flags) { struct udevice *dev = desc->dev; struct dm_gpio_ops *ops = gpio_get_ops(dev); @@ -637,9 +637,9 @@ static int _dm_gpio_set_dir_flags(struct gpio_desc *desc, ulong flags) return ret; } - /* GPIOD_ are directly managed by driver in set_dir_flags*/ - if (ops->set_dir_flags) { - ret = ops->set_dir_flags(dev, desc->offset, flags); + /* GPIOD_ are directly managed by driver in set_flags */ + if (ops->set_flags) { + ret = ops->set_flags(dev, desc->offset, flags); } else { if (flags & GPIOD_IS_OUT) { ret = ops->direction_output(dev, desc->offset, @@ -666,7 +666,7 @@ int dm_gpio_set_dir_flags(struct gpio_desc *desc, ulong flags) /* combine the requested flags (for IN/OUT) and the descriptor flags */ flags |= desc->flags; - ret = _dm_gpio_set_dir_flags(desc, flags); + ret = _dm_gpio_set_flags(desc, flags); return ret; } @@ -679,7 +679,7 @@ int dm_gpio_set_dir(struct gpio_desc *desc) if (ret) return ret; - return _dm_gpio_set_dir_flags(desc, desc->flags); + return _dm_gpio_set_flags(desc, desc->flags); } int dm_gpio_get_dir_flags(struct gpio_desc *desc, ulong *flags) @@ -1307,8 +1307,8 @@ static int gpio_post_bind(struct udevice *dev) ops->get_function += gd->reloc_off; if (ops->xlate) ops->xlate += gd->reloc_off; - if (ops->set_dir_flags) - ops->set_dir_flags += gd->reloc_off; + if (ops->set_flags) + ops->set_flags += gd->reloc_off; if (ops->get_dir_flags) ops->get_dir_flags += gd->reloc_off; diff --git a/drivers/gpio/sandbox.c b/drivers/gpio/sandbox.c index dc8d506e8d4..05f17928f0a 100644 --- a/drivers/gpio/sandbox.c +++ b/drivers/gpio/sandbox.c @@ -177,8 +177,8 @@ static int sb_gpio_xlate(struct udevice *dev, struct gpio_desc *desc, return 0; } -static int sb_gpio_set_dir_flags(struct udevice *dev, unsigned int offset, -ulong flags) +static int sb_gpio_set_flags(struct udevice *dev, unsigned int offset, +ulong flags) { ulong *dir_flags; @@ -272,7 +272,7 @@ static const struct dm_gpio_ops gpio_sandbox_ops = { .set_value = sb_gpio_set_value, .get_function = sb_gpio_get_function, .xlate = sb_gpio_xlate, - .set_dir_flags = sb_gpio_set_dir_flags, + .set_flags = sb_gpio_set_flags, .get_dir_flags = sb_gpio_get_dir_flags, #if CONFIG_IS_ENABLED(ACPIGEN) .get_acpi = sb_gpio_get_acpi, diff --git a/drivers/gpio/stm32_gpio.c b/drivers/gpio/stm32_gpio.c index 7184db3c527..6d1bef63c36 100644 --- a/drivers/gpio/stm32_gpio.c +++ b/drivers/gpio/stm32_gpio.c @@ -191,8 +191,8 @@ static int stm32_gpio_get_function(struct udevice *dev, unsigned int offset) return GPIOF_FUNC; } -static int stm32_gpio_set_dir_flags(struct udevice *dev, unsigned int offset, - ulong flags) +static int stm32_gpio_set_flags(struct udevice *dev, unsigned int offset, + ulong flags) { struct stm32_gpio_priv *priv = dev_get_priv(dev); struct stm32_gpio_regs *regs = priv->regs; @@ -270,7 +270,7 @@ static const struct dm_gpio_ops gpio_stm32_ops = { .get_value = stm32_gpio_get_value, .set_value = stm32_gpio_set_value, .get_function = stm32_gpio_get_function, - .set_dir_flags = stm32_gpio_set_dir_flags, + .set_flags = stm32_gpio_set_fla
[PATCH v2 01/15] gpio: Disable functions not used with of-platdata
These functions use devicetree and cannot wprl with of-platdata, which has no runtime devicetree. If they are used, the current linker error is confusing, since it talks about missing functions in the bowels of driver model. Avoid compiling these functions at all with of-platdata, so that a straightforward link error points to the problem. Signed-off-by: Simon Glass --- (no changes since v1) drivers/gpio/gpio-uclass.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c index bad6b71e0c3..e84b68db772 100644 --- a/drivers/gpio/gpio-uclass.c +++ b/drivers/gpio/gpio-uclass.c @@ -1023,6 +1023,7 @@ err: return ret; } +#if !CONFIG_IS_ENABLED(OF_PLATDATA) static int _gpio_request_by_name_nodev(ofnode node, const char *list_name, int index, struct gpio_desc *desc, int flags, bool add_index) @@ -1109,6 +1110,7 @@ int gpio_get_list_count(struct udevice *dev, const char *list_name) return ret; } +#endif /* OF_PLATDATA */ int dm_gpio_free(struct udevice *dev, struct gpio_desc *desc) { -- 2.30.0.296.g2bfb1c46d8-goog
[PATCH v2 00/15] gpio: Update and simplify the uclass API
At present the GPIO uclass mirrors what was in U-Boot before driver model. It works well in most cases but is becoming cumbersome with things like pull-up/down and drive strength. In those cases it is easier for the driver to deal with all the flags at one, rather than piece by piece. In fact the current API does not officially have a method for adjusting anything other than the direction flags. While set_dir_flags() and get_dir_flags() do in fact support changing other flags, this is not documented. Secondly, set_dir_flags actually ORs the current flags with the new ones so it is not possible to clear flags. It seems better to use a clr/set interface (bit clear followed by OR) to provide more flexibility. Finally, direction_input() and direction_output() are really just the same thing as set_dir_flags(), with a different name. We currently use the latter if available, failing back to the former. But it makes sense to deprecate the old methods. This series makes the above changes. Existing drivers are mostly left alone, since they should continue to operate as is. The sandbox driver is updated to add the required new tests and the x86 driver is switched over to the new API. The STM32 driver should be checked to make sure the open source/open drain features still work as intended. Changes in v2: - Use set_flags() instead of update_flags() - Fix 'provide' typo while we are here - Make operation of set_flags() deterministic - Swap newf and flags in sb_gpio_set_flags() Simon Glass (15): gpio: Disable functions not used with of-platdata dm: gpio: Rename set_dir_flags() method to update_flags() dm: gpio: Rename get_dir_flags() method to get_flags() gpio: Rename dm_gpio_get_dir_flags() to dm_gpio_get_flags() gpio: Drop dm_gpio_set_dir() gpio: sandbox: Rename GPIO dir_flags to flags gpio: sandbox: Use a separate flag for the value gpio: sandbox: Fully separate pin value from output value gpio: sandbox: Make sandbox_gpio_set_flags() set all flags dm: gpio: Add a way to update flags gpio: Replace direction_input() and direction_output() gpio: Use an 'ops' variable everywhere gpio: x86: Drop the deprecated methods in intel_gpio gpio: sandbox: Track whether a GPIO is driven gpio: Add a way to read 3-way strapping pins arch/sandbox/include/asm/gpio.h | 17 +- arch/x86/include/asm/intel_pinctrl_defs.h | 5 + drivers/gpio/gpio-uclass.c| 228 ++- drivers/gpio/intel_gpio.c | 72 +++--- drivers/gpio/sandbox.c| 138 drivers/gpio/stm32_gpio.c | 14 +- drivers/pinctrl/pinctrl-stmfx.c | 14 +- include/asm-generic/gpio.h| 130 +-- test/dm/gpio.c| 261 +++--- 9 files changed, 663 insertions(+), 216 deletions(-) -- 2.30.0.296.g2bfb1c46d8-goog
[PATCH v2 3/6] log: Add return-checking macros for 0 being success
The existing log_ret() and log_msg_ret() macros consider an error to be less than zero. But some function may return a positive number to indicate a different kind of failure. Add macros to check for that also. Signed-off-by: Simon Glass --- (no changes since v1) doc/develop/logging.rst | 15 ++- include/log.h | 20 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/doc/develop/logging.rst b/doc/develop/logging.rst index 482c17f7800..b6c6b45049f 100644 --- a/doc/develop/logging.rst +++ b/doc/develop/logging.rst @@ -119,11 +119,24 @@ can be used whenever your function returns an error value: .. code-block:: c - return log_ret(uclass_first_device(UCLASS_MMC, &dev)); + return log_ret(uclass_first_device_err(UCLASS_MMC, &dev)); This will write a log record when an error code is detected (a value < 0). This can make it easier to trace errors that are generated deep in the call stack. +The log_msg_ret() variant will print a short string if CONFIG_LOG_ERROR_RETURN +is enabled. So long as the string is unique within the function you can normally +determine exactly which call failed: + +.. code-block:: c + + ret = gpio_request_by_name(dev, "cd-gpios", 0, &desc, GPIOD_IS_IN); + if (ret) + return log_msg_ret("gpio", ret); + +Some functions return 0 for success and any other value is an error. For these, +log_retz() and log_msg_retz() are available. + Convenience functions ~ diff --git a/include/log.h b/include/log.h index c0453d2f97c..6ef891d4d2d 100644 --- a/include/log.h +++ b/include/log.h @@ -316,10 +316,30 @@ void __assert_fail(const char *assertion, const char *file, unsigned int line, __ret); \ __ret; \ }) + +/* + * Similar to the above, but any non-zero value is consider an error, not just + * values less than 0. + */ +#define log_retz(_ret) ({ \ + int __ret = (_ret); \ + if (__ret) \ + log(LOG_CATEGORY, LOGL_ERR, "returning err=%d\n", __ret); \ + __ret; \ + }) +#define log_msg_retz(_msg, _ret) ({ \ + int __ret = (_ret); \ + if (__ret) \ + log(LOG_CATEGORY, LOGL_ERR, "%s: returning err=%d\n", _msg, \ + __ret); \ + __ret; \ + }) #else /* Non-logging versions of the above which just return the error code */ #define log_ret(_ret) (_ret) #define log_msg_ret(_msg, _ret) ((void)(_msg), _ret) +#define log_retz(_ret) (_ret) +#define log_msg_retz(_msg, _ret) ((void)(_msg), _ret) #endif /** * enum log_rec_flags - Flags for a log record */ -- 2.30.0.296.g2bfb1c46d8-goog
[PATCH v2 6/6] log: Convert log values to printf() if not enabled
At present if logging not enabled, log_info() becomes a nop. But we want log output at the 'info' level to be akin to printf(). Update the macro to pass the output straight to printf() in this case. This mimics the behaviour for the log_...() macros like log_debug() and log_info(), so we can drop the special case for these. Add new tests to cover this case. Signed-off-by: Simon Glass --- Changes in v2: - Update commit message and cover letter to mention log_...() macros - Add a test for !CONFIG_LOG - Update log() to (effectively) call debug() for log_level == LOGL_DEBUG doc/develop/logging.rst | 6 -- include/log.h | 33 ++--- test/log/Makefile | 1 + test/log/nolog_ndebug.c | 38 ++ test/log/nolog_test.c | 3 +++ 5 files changed, 60 insertions(+), 21 deletions(-) create mode 100644 test/log/nolog_ndebug.c diff --git a/doc/develop/logging.rst b/doc/develop/logging.rst index b6c6b45049f..55cef42664d 100644 --- a/doc/develop/logging.rst +++ b/doc/develop/logging.rst @@ -54,6 +54,10 @@ If CONFIG_LOG is not set, then no logging will be available. The above have SPL and TPL versions also, e.g. CONFIG_SPL_LOG_MAX_LEVEL and CONFIG_TPL_LOG_MAX_LEVEL. +If logging is disabled, the default behaviour is to output any message at +level LOGL_INFO and below. If logging is disabled and DEBUG is defined (at +the very top of a C file) then any message at LOGL_DEBUG will be written. + Temporary logging within a single file -- @@ -293,8 +297,6 @@ More logging destinations: Convert debug() statements in the code to log() statements -Support making printf() emit log statements at L_INFO level - Convert error() statements in the code to log() statements Figure out what to do with BUG(), BUG_ON() and warn_non_spl() diff --git a/include/log.h b/include/log.h index 6ef891d4d2d..748e34d5a26 100644 --- a/include/log.h +++ b/include/log.h @@ -156,6 +156,10 @@ static inline int _log_nop(enum log_category_t cat, enum log_level_t level, */ #if CONFIG_IS_ENABLED(LOG) #define _LOG_MAX_LEVEL CONFIG_VAL(LOG_MAX_LEVEL) +#else +#define _LOG_MAX_LEVEL LOGL_INFO +#endif + #define log_emer(_fmt...) log(LOG_CATEGORY, LOGL_EMERG, ##_fmt) #define log_alert(_fmt...) log(LOG_CATEGORY, LOGL_ALERT, ##_fmt) #define log_crit(_fmt...) log(LOG_CATEGORY, LOGL_CRIT, ##_fmt) @@ -167,41 +171,32 @@ static inline int _log_nop(enum log_category_t cat, enum log_level_t level, #define log_content(_fmt...) log(LOG_CATEGORY, LOGL_DEBUG_CONTENT, ##_fmt) #define log_io(_fmt...)log(LOG_CATEGORY, LOGL_DEBUG_IO, ##_fmt) #define log_cont(_fmt...) log(LOGC_CONT, LOGL_CONT, ##_fmt) -#else -#define _LOG_MAX_LEVEL LOGL_INFO -#define log_emerg(_fmt, ...) printf(_fmt, ##__VA_ARGS__) -#define log_alert(_fmt, ...) printf(_fmt, ##__VA_ARGS__) -#define log_crit(_fmt, ...)printf(_fmt, ##__VA_ARGS__) -#define log_err(_fmt, ...) printf(_fmt, ##__VA_ARGS__) -#define log_warning(_fmt, ...) printf(_fmt, ##__VA_ARGS__) -#define log_notice(_fmt, ...) printf(_fmt, ##__VA_ARGS__) -#define log_info(_fmt, ...)printf(_fmt, ##__VA_ARGS__) -#define log_cont(_fmt, ...)printf(_fmt, ##__VA_ARGS__) -#define log_debug(_fmt, ...) debug(_fmt, ##__VA_ARGS__) -#define log_content(_fmt...) log_nop(LOG_CATEGORY, \ - LOGL_DEBUG_CONTENT, ##_fmt) -#define log_io(_fmt...)log_nop(LOG_CATEGORY, LOGL_DEBUG_IO, ##_fmt) -#endif -#if CONFIG_IS_ENABLED(LOG) #ifdef LOG_DEBUG #define _LOG_DEBUG LOGL_FORCE_DEBUG #else #define _LOG_DEBUG 0 #endif +#if CONFIG_IS_ENABLED(LOG) + /* Emit a log record if the level is less that the maximum */ #define log(_cat, _level, _fmt, _args...) ({ \ int _l = _level; \ - if (CONFIG_IS_ENABLED(LOG) && \ - (_LOG_DEBUG != 0 || _l <= _LOG_MAX_LEVEL)) \ + if (_LOG_DEBUG != 0 || _l <= _LOG_MAX_LEVEL) \ _log((enum log_category_t)(_cat), \ (enum log_level_t)(_l | _LOG_DEBUG), __FILE__, \ __LINE__, __func__, \ pr_fmt(_fmt), ##_args); \ }) #else -#define log(_cat, _level, _fmt, _args...) +/* Note: _LOG_DEBUG != 0 avoids a warning with clang */ +#define log(_cat, _level, _fmt, _args...) ({ \ + int _l = _level; \ + if (_LOG_DEBUG != 0 || _l <= LOGL_INFO || \ + (_DEBUG && _l == LOGL_DEBUG)) \ + printf(_fmt, ##_args); \ + }) #endif #define log_nop(_cat, _level, _fmt, _args...) ({ \ diff --git a/test/log/Makefile b/test/log/Makefile index 0e900363595..b63064e8d02 100644 --- a/test/log/Makefile +++ b/test/log/Makefile @@ -19,6 +19,7 @@ obj-$(CONFIG_CONSOLE_RECORD) += cont_test.o obj-y += pr_cont_test.o else obj-$(CONFIG_CONSOLE_RECORD) += nolog_test.o +obj-$(CONFIG_CONSOLE_RECORD) += nolog_ndebug.o endif endif # CONFIG_UT_LOG diff --git a/test/lo
[PATCH v2 5/6] tpm: Don't select LOG
We don't need to enable logging to run this command since the output will still appear. Drop the 'select'. Signed-off-by: Simon Glass --- (no changes since v1) cmd/Kconfig | 1 - 1 file changed, 1 deletion(-) diff --git a/cmd/Kconfig b/cmd/Kconfig index 0625ee4050f..7097cc1a145 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -2025,7 +2025,6 @@ config CMD_TPM_V1 config CMD_TPM_V2 bool - select CMD_LOG config CMD_TPM bool "Enable the 'tpm' command" -- 2.30.0.296.g2bfb1c46d8-goog
[PATCH v2 4/6] sandbox: log: Avoid build error with !CONFIG_LOG
The pr_cont_test.c test requires CONFIG_LOG since it directly accesses fields in global_data that require it. Move the test into the CONFIG_LOG condition to avoid build errors. Enable CONFIG_LOG on sandbox (not sandbox_spl, etc.) so that we still run this test. This requires resyncing of the configs. Signed-off-by: Simon Glass --- (no changes since v1) configs/sandbox_defconfig | 15 +++ test/log/Makefile | 2 +- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index 86dc603667c..788d22d56e6 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -20,8 +20,8 @@ CONFIG_BOOTSTAGE_STASH=y CONFIG_BOOTSTAGE_STASH_SIZE=0x4096 CONFIG_CONSOLE_RECORD=y CONFIG_CONSOLE_RECORD_OUT_SIZE=0x1000 -CONFIG_SILENT_CONSOLE=y CONFIG_PRE_CONSOLE_BUFFER=y +CONFIG_LOG=y CONFIG_LOG_SYSLOG=y CONFIG_LOG_ERROR_RETURN=y CONFIG_DISPLAY_BOARDINFO_LATE=y @@ -50,6 +50,7 @@ CONFIG_CMD_MEMTEST=y CONFIG_CMD_BIND=y CONFIG_CMD_DEMO=y CONFIG_CMD_GPIO=y +CONFIG_CMD_PWM=y CONFIG_CMD_GPT=y CONFIG_CMD_GPT_RENAME=y CONFIG_CMD_IDE=y @@ -58,7 +59,6 @@ CONFIG_CMD_LSBLK=y CONFIG_CMD_MUX=y CONFIG_CMD_OSD=y CONFIG_CMD_PCI=y -CONFIG_CMD_PWM=y CONFIG_CMD_READ=y CONFIG_CMD_REMOTEPROC=y CONFIG_CMD_SPI=y @@ -132,6 +132,7 @@ CONFIG_CPU=y CONFIG_DM_DEMO=y CONFIG_DM_DEMO_SIMPLE=y CONFIG_DM_DEMO_SHAPE=y +CONFIG_DFU_SF=y CONFIG_DMA=y CONFIG_DMA_CHANNELS=y CONFIG_SANDBOX_DMA=y @@ -270,14 +271,12 @@ CONFIG_CMD_DHRYSTONE=y CONFIG_TPM=y CONFIG_LZ4=y CONFIG_ERRNO_STR=y +CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y +CONFIG_EFI_CAPSULE_ON_DISK=y +CONFIG_EFI_CAPSULE_FIRMWARE_FIT=y +CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y CONFIG_EFI_SECURE_BOOT=y CONFIG_TEST_FDTDEC=y CONFIG_UNIT_TEST=y CONFIG_UT_TIME=y CONFIG_UT_DM=y -# -CONFIG_DFU_SF=y -CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y -CONFIG_EFI_CAPSULE_ON_DISK=y -CONFIG_EFI_CAPSULE_FIRMWARE_FIT=y -CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y diff --git a/test/log/Makefile b/test/log/Makefile index afdafa502ac..0e900363595 100644 --- a/test/log/Makefile +++ b/test/log/Makefile @@ -8,7 +8,6 @@ obj-$(CONFIG_CMD_LOG) += log_filter.o ifdef CONFIG_UT_LOG obj-y += test-main.o -obj-y += pr_cont_test.o ifdef CONFIG_SANDBOX obj-$(CONFIG_LOG_SYSLOG) += syslog_test.o @@ -17,6 +16,7 @@ endif ifdef CONFIG_LOG obj-$(CONFIG_CONSOLE_RECORD) += cont_test.o +obj-y += pr_cont_test.o else obj-$(CONFIG_CONSOLE_RECORD) += nolog_test.o endif -- 2.30.0.296.g2bfb1c46d8-goog
[PATCH v2 2/6] log: Handle line continuation
When multiple log() calls are used which don't end in newline, the log prefix is prepended multiple times in the same line. This makes the output look strange. Fix this by detecting when the previous log record did not end in newline. In that case, setting a flag. Drop the unused BUFFSIZE in the test while we are here. As an example implementation, update log_console to check the flag and produce the expected output. Signed-off-by: Simon Glass --- Changes in v2: - Move the newline check into log_dispatch() common/log.c | 7 ++- common/log_console.c | 26 +++--- doc/develop/logging.rst | 16 include/asm-generic/global_data.h | 6 ++ include/log.h | 2 ++ test/log/cont_test.c | 19 +++ 6 files changed, 60 insertions(+), 16 deletions(-) diff --git a/common/log.c b/common/log.c index 02f4cbe3fdb..1709c6ed192 100644 --- a/common/log.c +++ b/common/log.c @@ -217,8 +217,11 @@ static int log_dispatch(struct log_rec *rec, const char *fmt, va_list args) if ((ldev->flags & LOGDF_ENABLE) && log_passes_filters(ldev, rec)) { if (!rec->msg) { - vsnprintf(buf, sizeof(buf), fmt, args); + int len; + + len = vsnprintf(buf, sizeof(buf), fmt, args); rec->msg = buf; + gd->log_cont = len && buf[len - 1] != '\n'; } ldev->drv->emit(ldev, rec); } @@ -247,6 +250,8 @@ int _log(enum log_category_t cat, enum log_level_t level, const char *file, rec.flags = 0; if (level & LOGL_FORCE_DEBUG) rec.flags |= LOGRECF_FORCE_DEBUG; + if (gd->log_cont) + rec.flags |= LOGRECF_CONT; rec.file = file; rec.line = line; rec.func = func; diff --git a/common/log_console.c b/common/log_console.c index 8776fd47039..76bc9524202 100644 --- a/common/log_console.c +++ b/common/log_console.c @@ -14,6 +14,7 @@ DECLARE_GLOBAL_DATA_PTR; static int log_console_emit(struct log_device *ldev, struct log_rec *rec) { int fmt = gd->log_fmt; + bool add_space = false; /* * The output format is designed to give someone a fighting chance of @@ -25,18 +26,21 @@ static int log_console_emit(struct log_device *ldev, struct log_rec *rec) *- function is an identifier and ends with () *- message has a space before it unless it is on its own */ - if (fmt & BIT(LOGF_LEVEL)) - printf("%s.", log_get_level_name(rec->level)); - if (fmt & BIT(LOGF_CAT)) - printf("%s,", log_get_cat_name(rec->cat)); - if (fmt & BIT(LOGF_FILE)) - printf("%s:", rec->file); - if (fmt & BIT(LOGF_LINE)) - printf("%d-", rec->line); - if (fmt & BIT(LOGF_FUNC)) - printf("%s()", rec->func); + if (!(rec->flags & LOGRECF_CONT) && fmt != BIT(LOGF_MSG)) { + add_space = true; + if (fmt & BIT(LOGF_LEVEL)) + printf("%s.", log_get_level_name(rec->level)); + if (fmt & BIT(LOGF_CAT)) + printf("%s,", log_get_cat_name(rec->cat)); + if (fmt & BIT(LOGF_FILE)) + printf("%s:", rec->file); + if (fmt & BIT(LOGF_LINE)) + printf("%d-", rec->line); + if (fmt & BIT(LOGF_FUNC)) + printf("%s()", rec->func); + } if (fmt & BIT(LOGF_MSG)) - printf("%s%s", fmt != BIT(LOGF_MSG) ? " " : "", rec->msg); + printf("%s%s", add_space ? " " : "", rec->msg); return 0; } diff --git a/doc/develop/logging.rst b/doc/develop/logging.rst index 7fdd1132efe..482c17f7800 100644 --- a/doc/develop/logging.rst +++ b/doc/develop/logging.rst @@ -98,6 +98,22 @@ Also debug() and error() will generate log records - these use LOG_CATEGORY as the category, so you should #define this right at the top of the source file to ensure the category is correct. +Generally each log format_string ends with a newline. If it does not, then the +next log statement will have the LOGRECF_CONT flag set. This can be used to +continue the statement on the same line as the previous one without emitting +new header information (such as category/level). This behaviour is implemented +with log_console. Here is an example that prints a list all on one line with +the tags at the start: + +.. code-block:: c + + log_debug("Here is a list:"); + for (i = 0; i < count; i++) + log_debug(" item %d", i); + log_debug("\n"); + +Also see the special category LOGL_CONT and level LOGC_CONT. + You can also define CONFIG_LOG_ERROR_RETURN to enable the log_ret() macro. This can be us
[PATCH v2 1/6] log: Set up a flag byte for log records
At present only a single flag (force_debug) is used in log records. Before adding more, convert this into a bitfield, so more can be added without using more space. To avoid expanding the log_record struct itself (which some drivers may wish to store in memory) reduce the line-number field to 16 bits. This provides for up to 64K lines which should be enough for anyone. Signed-off-by: Simon Glass --- (no changes since v1) common/log.c | 9 ++--- include/log.h | 14 ++ 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/common/log.c b/common/log.c index 767f0febc51..02f4cbe3fdb 100644 --- a/common/log.c +++ b/common/log.c @@ -152,7 +152,7 @@ static bool log_passes_filters(struct log_device *ldev, struct log_rec *rec) { struct log_filter *filt; - if (rec->force_debug) + if (rec->flags & LOGRECF_FORCE_DEBUG) return true; /* If there are no filters, filter on the default log level */ @@ -244,7 +244,9 @@ int _log(enum log_category_t cat, enum log_level_t level, const char *file, rec.cat = cat; rec.level = level & LOGL_LEVEL_MASK; - rec.force_debug = level & LOGL_FORCE_DEBUG; + rec.flags = 0; + if (level & LOGL_FORCE_DEBUG) + rec.flags |= LOGRECF_FORCE_DEBUG; rec.file = file; rec.line = line; rec.func = func; @@ -254,7 +256,8 @@ int _log(enum log_category_t cat, enum log_level_t level, const char *file, gd->log_drop_count++; /* display dropped traces with console puts and DEBUG_UART */ - if (rec.level <= CONFIG_LOG_DEFAULT_LEVEL || rec.force_debug) { + if (rec.level <= CONFIG_LOG_DEFAULT_LEVEL || + rec.flags & LOGRECF_FORCE_DEBUG) { char buf[CONFIG_SYS_CBSIZE]; va_start(args, fmt); diff --git a/include/log.h b/include/log.h index 2d27f9f657e..da053b0a6e8 100644 --- a/include/log.h +++ b/include/log.h @@ -322,6 +322,12 @@ void __assert_fail(const char *assertion, const char *file, unsigned int line, #define log_msg_ret(_msg, _ret) ((void)(_msg), _ret) #endif +/** * enum log_rec_flags - Flags for a log record */ +enum log_rec_flags { + /** @LOGRECF_FORCE_DEBUG: Force output of debug record */ + LOGRECF_FORCE_DEBUG = BIT(0), +}; + /** * struct log_rec - a single log record * @@ -337,18 +343,18 @@ void __assert_fail(const char *assertion, const char *file, unsigned int line, * * @cat: Category, representing a uclass or part of U-Boot * @level: Severity level, less severe is higher - * @force_debug: Force output of debug - * @file: Name of file where the log record was generated (not allocated) * @line: Line number where the log record was generated + * @flags: Flags for log record (enum log_rec_flags) + * @file: Name of file where the log record was generated (not allocated) * @func: Function where the log record was generated (not allocated) * @msg: Log message (allocated) */ struct log_rec { enum log_category_t cat; enum log_level_t level; - bool force_debug; + u16 line; + u8 flags; const char *file; - int line; const char *func; const char *msg; }; -- 2.30.0.296.g2bfb1c46d8-goog
[PATCH v2 0/6] log: Allow multiple lines and conversion to printf()
At present when logging is not enabled, all log() calls become nops. This does not seem right, since if the log level is high enough then there should be some sort of message. So in that case, this series updates it to print the message if the log level is above LOGL_INFO. This mimics the behaviour for the log_...() macros like log_debug() and log_info(), so we can drop the special case for these. Also the current implementation does not support multiple log calls on the same line nicely. The tags are repeated so the line is very hard to read. This series adds that as a new feature. Changes in v2: - Move the newline check into log_dispatch() - Update commit message and cover letter to mention log_...() macros - Add a test for !CONFIG_LOG - Update log() to (effectively) call debug() for log_level == LOGL_DEBUG Simon Glass (6): log: Set up a flag byte for log records log: Handle line continuation log: Add return-checking macros for 0 being success sandbox: log: Avoid build error with !CONFIG_LOG tpm: Don't select LOG log: Convert log values to printf() if not enabled cmd/Kconfig | 1 - common/log.c | 16 +-- common/log_console.c | 26 +++- configs/sandbox_defconfig | 15 --- doc/develop/logging.rst | 37 +++-- include/asm-generic/global_data.h | 6 +++ include/log.h | 69 --- test/log/Makefile | 3 +- test/log/cont_test.c | 19 +++-- test/log/nolog_ndebug.c | 38 + test/log/nolog_test.c | 3 ++ 11 files changed, 178 insertions(+), 55 deletions(-) create mode 100644 test/log/nolog_ndebug.c -- 2.30.0.296.g2bfb1c46d8-goog
Re: [PATCH v2] sysboot: add zboot support to boot x86 Linux kernel image
On Sat, Dec 26, 2020 at 5:25 AM Kory Maincent wrote: > > Add "zboot" command to the list of supported boot in the label_boot function. > > Signed-off-by: Kory Maincent > --- > > Change since v1: > - Modify comment > > cmd/pxe_utils.c | 4 > include/command.h | 2 ++ > 2 files changed, 6 insertions(+) > applied to u-boot-x86, thanks!
Re: [PATCH v3 0/7] mtd: spi-nor: Add support for Cypress s25hl-t/s25hs-t
Hi Jagan, On 11/4/2020 5:10 PM, tkuw584...@gmail.com wrote: > From: Takahiro Kuwano > > The S25HL-T/S25HS-T family is the Cypress Semper Flash with Quad SPI. > The datasheet can be found in https://community.cypress.com/docs/DOC-15165 > Tested on Xilinx Zynq-7000 FPGA board. > > Takahiro Kuwano (7): > mtd: spi-nor: Add Cypress manufacturer ID > mtd: spi-nor-ids: Add Cypress s25hl-t/s25hs-t > mtd: spi-nor: Add support for volatile QE bit > mtd: spi-nor-core: Add Cypress manufacturer ID in set_4byte > mtd: spi-nor-core: Add overlaid sector erase feature > mtd: spi-nor-core: Add fixups for Cypress s25hl-t/s25hs-t > mtd: spi-nor-tiny: Add fixups for Cypress s25hl-t/s25hs-t > > drivers/mtd/spi/spi-nor-core.c | 178 + > drivers/mtd/spi/spi-nor-ids.c | 24 + > drivers/mtd/spi/spi-nor-tiny.c | 73 ++ > include/linux/mtd/spi-nor.h| 3 + > 4 files changed, 278 insertions(+) > > --- > Changes since v3: > - Split into multiple patches > > Changes since v2: > - Fixed typo in comment for spansion_overlaid_erase() > - Fixed expressions for addr and len check in spansion_overlaid_erase() > - Added device ID check to make the changes effective for S25 only > - Added nor->setup() and fixup hooks based on the following patches > > https://patchwork.ozlabs.org/project/uboot/patch/20200904153500.3569-7-p.ya...@ti.com/ > > https://patchwork.ozlabs.org/project/uboot/patch/20200904153500.3569-8-p.ya...@ti.com/ > > https://patchwork.ozlabs.org/project/uboot/patch/20200904153500.3569-9-p.ya...@ti.com/ > Please let me revise this series. I would like to add 2Gb (dual die package) and 4Gb (quad die package) pats support. Thanks, Takahiro
[PATCH v2 05/12] smbios: Set BIOS release version
We may as well include the U-Boot release information in the type-0 table since it is designed for that purpose. U-Boot uses release versions based on the year and month. The year cannot fit in a byte, so drop the century. Signed-off-by: Simon Glass --- Changes in v2: - Add a comment about dropping the century lib/smbios.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/smbios.c b/lib/smbios.c index b1171f544a8..a072d9ec10b 100644 --- a/lib/smbios.c +++ b/lib/smbios.c @@ -143,8 +143,9 @@ static int smbios_write_type0(ulong *current, int handle, ofnode node) #endif t->bios_characteristics_ext2 = BIOS_CHARACTERISTICS_EXT2_TARGET; - t->bios_major_release = 0xff; - t->bios_minor_release = 0xff; + /* bios_major_release has only one byte, so drop century */ + t->bios_major_release = U_BOOT_VERSION_NUM % 100; + t->bios_minor_release = U_BOOT_VERSION_NUM_PATCH; t->ec_major_release = 0xff; t->ec_minor_release = 0xff; -- 2.30.0.296.g2bfb1c46d8-goog
Re: [scan-ad...@coverity.com: New Defects reported by Coverity Scan for Das U-Boot]
Tom, Regarding EFI capsule update, On Wed, Jan 20, 2021 at 09:43:57PM +0100, Heinrich Schuchardt wrote: > On 1/20/21 8:04 PM, Tom Rini wrote: > > CC: Takahiro > > > I decided to run Coverity part-way through the merge window this time > > and here's what's been found so far. > > > > - Forwarded message from scan-ad...@coverity.com - > > > > Date: Mon, 18 Jan 2021 17:53:19 + (UTC) > > From: scan-ad...@coverity.com > > To: tom.r...@gmail.com > > Subject: New Defects reported by Coverity Scan for Das U-Boot > > > > Hi, > > > > Please find the latest report on new defect(s) introduced to Das U-Boot > > found with Coverity Scan. > > > > 23 new defect(s) introduced to Das U-Boot found with Coverity Scan. > > 2 defect(s), reported by Coverity Scan earlier, were marked fixed in the > > recent build analyzed by Coverity Scan. > > > > New defect(s) Reported-by: Coverity Scan > > Showing 20 of 23 defect(s) > > > > > > ** CID 316365: Memory - corruptions (STRING_OVERFLOW) > > /tools/sunxi_egon.c: 96 in egon_set_header() > > > > > > > > *** CID 316365: Memory - corruptions (STRING_OVERFLOW) > > /tools/sunxi_egon.c: 96 in egon_set_header() > > 90 > > 91 /* If an image name has been provided, use it as the DT name. */ > > 92 if (params->imagename && params->imagename[0]) { > > 93 if (strlen(params->imagename) > > > sizeof(header->string_pool) - 1) > > 94 printf("WARNING: DT name too long for SPL > > header!\n"); > > 95 else { > > > > > CID 316365: Memory - corruptions (STRING_OVERFLOW) > > > > > You might overrun the 13-character destination string > > > > > "header->string_pool" by writing 51 characters from > > > > > "params->imagename". > > 96 strcpy((char *)header->string_pool, > > params->imagename); > > 97 value = offsetof(struct boot_file_head, > > string_pool); > > 98 header->dt_name_offset = cpu_to_le32(value); > > 99 header->spl_signature[3] = > > SPL_DT_HEADER_VERSION; > > 100 } > > 101 } > > > > ** CID 316364: Null pointer dereferences (FORWARD_NULL) > > /cmd/efidebug.c: 202 in do_efi_capsule_res() > > > > > > > > *** CID 316364: Null pointer dereferences (FORWARD_NULL) > > /cmd/efidebug.c: 202 in do_efi_capsule_res() > > 196 printf("Failed to get %ls\n", var_name16); > > 197 > > 198 return CMD_RET_FAILURE; > > 199 } > > 200 } > > 201 > > > > > CID 316364: Null pointer dereferences (FORWARD_NULL) > > > > > Dereferencing null pointer "result". > > 202 printf("Result total size: 0x%x\n", > > result->variable_total_size); This is basically safe because a buffer for "result" is allocated by malloc(). (The second "get_variable" fails any way if the allocation has failed.) But there may be a chance (unlikely though) that the first "get_variable" will return neither EFI_SUCCESS or EFI_BUFFER_TOO_SMALL. I will modify the code a bit to address that. > > 203 printf("Capsule guid: %pUl\n", &result->capsule_guid); > > 204 printf("Time processed: %04d-%02d-%02d %02d:%02d:%02d\n", > > 205result->capsule_processed.year, > > result->capsule_processed.month, > > 206result->capsule_processed.day, > > result->capsule_processed.hour, > > 207result->capsule_processed.minute, > > > > ** CID 316363: Null pointer dereferences (REVERSE_INULL) > > /lib/efi_loader/efi_boottime.c: 1993 in efi_load_image_from_path() > > > > > > > > *** CID 316363: Null pointer dereferences (REVERSE_INULL) > > /lib/efi_loader/efi_boottime.c: 1993 in efi_load_image_from_path() > > 1987ret = EFI_CALL(load_file_protocol->load_file( > > 1988load_file_protocol, dp, > > boot_policy, > > 1989&buffer_size, (void > > *)(uintptr_t)addr)); > > 1990if (ret != EFI_SUCCESS) > > 1991efi_free_pages(addr, pages); > > 1992 out: > > > > > CID 316363: Null pointer dereferences (REVERSE_INULL) > > > > > Null-checking "load_file_protocol" suggests that it may be null, > > > > > but it has already been dereferenced on all paths leading to the > > > > > check. > > 1993if (load_file_protocol) > > 1994EFI_CALL(efi_close_protocol(device, > > 1995 > > &efi_guid_load_file2_protocol, > > 1996efi_root, NULL)); > > 19
[PATCH v2 10/12] sysinfo: Move #ifdef so that operations are always defined
At present the struct is not available unless SYSINFO is enabled. This is annoying since code it is not possible to use compile-time checks like CONFIG_IS_ENABLED(SYSINFO) with this header. Fix it by moving the #ifdef. Signed-off-by: Simon Glass --- Changes in v2: - Add new patch to fix sysinfo with CONFIG_IS_ENABLED() include/sysinfo.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/sysinfo.h b/include/sysinfo.h index c045d316b07..6e021253524 100644 --- a/include/sysinfo.h +++ b/include/sysinfo.h @@ -31,7 +31,6 @@ * to read the serial number. */ -#if CONFIG_IS_ENABLED(SYSINFO) struct sysinfo_ops { /** * detect() - Run the hardware info detection procedure for this @@ -102,6 +101,7 @@ struct sysinfo_ops { #define sysinfo_get_ops(dev) ((struct sysinfo_ops *)(dev)->driver->ops) +#if CONFIG_IS_ENABLED(SYSINFO) /** * sysinfo_detect() - Run the hardware info detection procedure for this device. * -- 2.30.0.296.g2bfb1c46d8-goog
[PATCH v2 12/12] smbios: Allow a few values to come from sysinfo
While static configuration is useful it cannot cover every case. Sometimes board revisions are encoded in resistor straps and must be read at runtime. The easiest way to provide this information is via sysinfo, since the board can then provide a driver to read whatever is needed. Add some standard sysinfo options for this, and use them to obtain the required information. Signed-off-by: Simon Glass --- (no changes since v1) include/sysinfo.h | 11 +++ lib/smbios.c | 32 +--- 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/include/sysinfo.h b/include/sysinfo.h index 6e021253524..743f3554659 100644 --- a/include/sysinfo.h +++ b/include/sysinfo.h @@ -31,6 +31,17 @@ * to read the serial number. */ +/** enum sysinfo_id - Standard IDs defined by U-Boot */ +enum sysinfo_id { + SYSINFO_ID_NONE, + + SYSINFO_ID_SMBIOS_SYSTEM_VERSION, + SYSINFO_ID_SMBIOS_BASEBOARD_VERSION, + + /* First value available for downstream/board used */ + SYSINFO_ID_USER = 0x1000, +}; + struct sysinfo_ops { /** * detect() - Run the hardware info detection procedure for this diff --git a/lib/smbios.c b/lib/smbios.c index d46569b09f4..9bdde0b953f 100644 --- a/lib/smbios.c +++ b/lib/smbios.c @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #ifdef CONFIG_CPU @@ -106,15 +107,26 @@ static int smbios_add_string(struct smbios_ctx *ctx, const char *str) } /** - * smbios_add_prop() - Add a property from the device tree + * smbios_add_prop_si() - Add a property from the devicetree or sysinfo + * + * Sysinfo is used if available, with a fallback to devicetree * * @start: string area start address * @node: node containing the information to write (ofnode_null() if none) * @prop: property to write * @return 0 if not found, else SMBIOS string number (1 or more) */ -static int smbios_add_prop(struct smbios_ctx *ctx, const char *prop) +static int smbios_add_prop_si(struct smbios_ctx *ctx, const char *prop, + int sysinfo_id) { + if (sysinfo_id && ctx->dev) { + char val[80]; + int ret; + + ret = sysinfo_get_str(ctx->dev, sysinfo_id, sizeof(val), val); + if (!ret) + return smbios_add_string(ctx, val); + } if (IS_ENABLED(CONFIG_OF_CONTROL)) { const char *str; @@ -126,6 +138,17 @@ static int smbios_add_prop(struct smbios_ctx *ctx, const char *prop) return 0; } +/** + * smbios_add_prop() - Add a property from the devicetree + * + * @prop: property to write + * @return 0 if not found, else SMBIOS string number (1 or more) + */ +static int smbios_add_prop(struct smbios_ctx *ctx, const char *prop) +{ + return smbios_add_prop_si(ctx, prop, SYSINFO_ID_NONE); +} + static void set_eos(struct smbios_ctx *ctx, char *eos) { ctx->eos = eos; @@ -239,7 +262,8 @@ static int smbios_write_type1(ulong *current, int handle, set_eos(ctx, t->eos); t->manufacturer = smbios_add_prop(ctx, "manufacturer"); t->product_name = smbios_add_prop(ctx, "product"); - t->version = smbios_add_prop(ctx, "version"); + t->version = smbios_add_prop_si(ctx, "version", + SYSINFO_ID_SMBIOS_SYSTEM_VERSION); if (serial_str) { t->serial_number = smbios_add_string(ctx, serial_str); strncpy((char *)t->uuid, serial_str, sizeof(t->uuid)); @@ -268,6 +292,8 @@ static int smbios_write_type2(ulong *current, int handle, set_eos(ctx, t->eos); t->manufacturer = smbios_add_prop(ctx, "manufacturer"); t->product_name = smbios_add_prop(ctx, "product"); + t->version = smbios_add_prop_si(ctx, "version", + SYSINFO_ID_SMBIOS_BASEBOARD_VERSION); t->asset_tag_number = smbios_add_prop(ctx, "asset-tag"); t->feature_flags = SMBIOS_BOARD_FEATURE_HOSTING; t->board_type = SMBIOS_BOARD_MOTHERBOARD; -- 2.30.0.296.g2bfb1c46d8-goog
[PATCH v2 11/12] x86: coral: Add sysinfo ops
These ops are missing at present which is not permitted. Add an empty operation struct. Signed-off-by: Simon Glass --- Changes in v2: - Add new patch to fix crash on coral board/google/chromebook_coral/coral.c | 5 + 1 file changed, 5 insertions(+) diff --git a/board/google/chromebook_coral/coral.c b/board/google/chromebook_coral/coral.c index 34b2c2ac5d5..f9fb3f163f0 100644 --- a/board/google/chromebook_coral/coral.c +++ b/board/google/chromebook_coral/coral.c @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -143,6 +144,9 @@ struct acpi_ops coral_acpi_ops = { .inject_dsdt= chromeos_acpi_gpio_generate, }; +struct sysinfo_ops coral_sysinfo_ops = { +}; + #if !CONFIG_IS_ENABLED(OF_PLATDATA) static const struct udevice_id coral_ids[] = { { .compatible = "google,coral" }, @@ -154,5 +158,6 @@ U_BOOT_DRIVER(coral_drv) = { .name = "coral", .id = UCLASS_SYSINFO, .of_match = of_match_ptr(coral_ids), + .ops= &coral_sysinfo_ops, ACPI_OPS_PTR(&coral_acpi_ops) }; -- 2.30.0.296.g2bfb1c46d8-goog
[PATCH v2 09/12] smbios: Add more options for the BIOS version string
At present the version string is obtained from PLAIN_VERSION. Some boards may want to configure this using the device tree, since the build system can more easily insert things there after U-Boot itself is built. Add this option to the code. Also in some cases the version needs to be generated programmatically, such as when it is stored elsewhere in the ROM and must be read first. To handle this, keep a pointer around so that it can be updated later. This works by storing the last string in the context, since it is easier than passing out a little-used extra parameter. Provide a function to update the version string. Signed-off-by: Simon Glass --- Changes in v2: - Correct documentation format include/asm-generic/global_data.h | 6 include/smbios.h | 12 +++ lib/smbios.c | 56 +-- 3 files changed, 71 insertions(+), 3 deletions(-) diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h index 19f70393b45..750e998d885 100644 --- a/include/asm-generic/global_data.h +++ b/include/asm-generic/global_data.h @@ -439,6 +439,12 @@ struct global_data { */ struct acpi_ctx *acpi_ctx; #endif +#if CONFIG_IS_ENABLED(GENERATE_SMBIOS_TABLE) + /** +* @smbios_version: Points to SMBIOS type 0 version +*/ + char *smbios_version; +#endif }; /** diff --git a/include/smbios.h b/include/smbios.h index 1cbeabf9522..ecc4fd1de3b 100644 --- a/include/smbios.h +++ b/include/smbios.h @@ -257,4 +257,16 @@ const struct smbios_header *smbios_header(const struct smbios_entry *entry, int */ const char *smbios_string(const struct smbios_header *header, int index); +/** + * smbios_update_version() - Update the version string + * + * This can be called after the SMBIOS tables are written (e.g. after the U-Boot + * main loop has started) to update the BIOS version string (SMBIOS table 0). + * + * @version: New version string to use + * @return 0 if OK, -ENOENT if no version string was previously written, + * -ENOSPC if the new string is too large to fit + */ +int smbios_update_version(const char *version); + #endif /* _SMBIOS_H_ */ diff --git a/lib/smbios.c b/lib/smbios.c index 7090460bc02..d46569b09f4 100644 --- a/lib/smbios.c +++ b/lib/smbios.c @@ -17,6 +17,10 @@ #include #endif +enum { + SMBIOS_STR_MAX = 64, /* Maximum length allowed for a string */ +}; + /** * struct smbios_ctx - context for writing SMBIOS tables * @@ -27,12 +31,15 @@ * @next_ptr: pointer to the start of the next string to be added. When the * table is nopt empty, this points to the byte after the \0 of the * previous string. + * @last_str: points to the last string that was written to the table, or NULL + * if none */ struct smbios_ctx { ofnode node; struct udevice *dev; char *eos; char *next_ptr; + char *last_str; }; /** @@ -78,6 +85,7 @@ static int smbios_add_string(struct smbios_ctx *ctx, const char *str) for (;;) { if (!*p) { + ctx->last_str = p; strcpy(p, str); p += strlen(str); *p++ = '\0'; @@ -87,8 +95,10 @@ static int smbios_add_string(struct smbios_ctx *ctx, const char *str) return i; } - if (!strcmp(p, str)) + if (!strcmp(p, str)) { + ctx->last_str = p; return i; + } p += strlen(p) + 1; i++; @@ -120,6 +130,35 @@ static void set_eos(struct smbios_ctx *ctx, char *eos) { ctx->eos = eos; ctx->next_ptr = eos; + ctx->last_str = NULL; +} + +int smbios_update_version(const char *version) +{ + char *ptr = gd->smbios_version; + uint old_len, len; + + if (!ptr) + return log_ret(-ENOENT); + + /* +* This string is supposed to have at least enough bytes and is +* padded with spaces. Update it, taking care not to move the +* \0 terminator, so that other strings in the string table +* are not disturbed. See smbios_add_string() +*/ + old_len = strnlen(ptr, SMBIOS_STR_MAX); + len = strnlen(version, SMBIOS_STR_MAX); + if (len > old_len) + return log_ret(-ENOSPC); + + log_debug("Replacing SMBIOS type 0 version string '%s'\n", ptr); + memcpy(ptr, version, len); +#ifdef LOG_DEBUG + print_buffer((ulong)ptr, ptr, 1, old_len + 1, 0); +#endif + + return 0; } /** @@ -147,7 +186,18 @@ static int smbios_write_type0(ulong *current, int handle, fill_smbios_header(t, SMBIOS_BIOS_INFORMATION, len, handle); set_eos(ctx, t->eos); t->vendor = smbios_add_string(ctx, "U-Boot"); - t->bios_ver = smbios_add_string(ctx, PLAIN_VERSION); + + t->bios_ver = smb
[PATCH v2 08/12] smbios: Track the end of the string table
Add a new member to the context struct which tracks the end of the string table. This allows us to avoid recalculating this at the end. Signed-off-by: Simon Glass --- (no changes since v1) lib/smbios.c | 32 +++- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/lib/smbios.c b/lib/smbios.c index 1d9bde0c3c2..7090460bc02 100644 --- a/lib/smbios.c +++ b/lib/smbios.c @@ -24,11 +24,15 @@ * @dev: sysinfo device to use (NULL if none) * @eos: end-of-string pointer for the table being processed. This is set * up when we start processing a table + * @next_ptr: pointer to the start of the next string to be added. When the + * table is nopt empty, this points to the byte after the \0 of the + * previous string. */ struct smbios_ctx { ofnode node; struct udevice *dev; char *eos; + char *next_ptr; }; /** @@ -77,6 +81,7 @@ static int smbios_add_string(struct smbios_ctx *ctx, const char *str) strcpy(p, str); p += strlen(str); *p++ = '\0'; + ctx->next_ptr = p; *p++ = '\0'; return i; @@ -114,6 +119,7 @@ static int smbios_add_prop(struct smbios_ctx *ctx, const char *prop) static void set_eos(struct smbios_ctx *ctx, char *eos) { ctx->eos = eos; + ctx->next_ptr = eos; } /** @@ -121,21 +127,13 @@ static void set_eos(struct smbios_ctx *ctx, char *eos) * * This computes the size of the string area including the string terminator. * - * @start: string area start address + * @ctx: SMBIOS context * @return:string area size */ -static int smbios_string_table_len(char *start) +static int smbios_string_table_len(const struct smbios_ctx *ctx) { - char *p = start; - int i, len = 0; - - while (*p) { - i = strlen(p) + 1; - p += i; - len += i; - } - - return len + 1; + /* Allow for the final \0 after all strings */ + return (ctx->next_ptr + 1) - ctx->eos; } static int smbios_write_type0(ulong *current, int handle, @@ -171,7 +169,7 @@ static int smbios_write_type0(ulong *current, int handle, t->ec_major_release = 0xff; t->ec_minor_release = 0xff; - len = t->length + smbios_string_table_len(t->eos); + len = t->length + smbios_string_table_len(ctx); *current += len; unmap_sysmem(t); @@ -201,7 +199,7 @@ static int smbios_write_type1(ulong *current, int handle, t->sku_number = smbios_add_prop(ctx, "sku"); t->family = smbios_add_prop(ctx, "family"); - len = t->length + smbios_string_table_len(t->eos); + len = t->length + smbios_string_table_len(ctx); *current += len; unmap_sysmem(t); @@ -224,7 +222,7 @@ static int smbios_write_type2(ulong *current, int handle, t->feature_flags = SMBIOS_BOARD_FEATURE_HOSTING; t->board_type = SMBIOS_BOARD_MOTHERBOARD; - len = t->length + smbios_string_table_len(t->eos); + len = t->length + smbios_string_table_len(ctx); *current += len; unmap_sysmem(t); @@ -248,7 +246,7 @@ static int smbios_write_type3(ulong *current, int handle, t->thermal_state = SMBIOS_STATE_SAFE; t->security_status = SMBIOS_SECURITY_NONE; - len = t->length + smbios_string_table_len(t->eos); + len = t->length + smbios_string_table_len(ctx); *current += len; unmap_sysmem(t); @@ -307,7 +305,7 @@ static int smbios_write_type4(ulong *current, int handle, t->l3_cache_handle = 0x; t->processor_family2 = t->processor_family; - len = t->length + smbios_string_table_len(t->eos); + len = t->length + smbios_string_table_len(ctx); *current += len; unmap_sysmem(t); -- 2.30.0.296.g2bfb1c46d8-goog
[PATCH v2 07/12] smbios: Drop the eos parameter
We can store this in the context and avoid passing it to each function. This makes it easier to follow and will also allow keeping track of the end of the string table (in future patches). Add an 'eos' field to the context and create a function to set it up. Signed-off-by: Simon Glass --- (no changes since v1) lib/smbios.c | 61 +++- 1 file changed, 37 insertions(+), 24 deletions(-) diff --git a/lib/smbios.c b/lib/smbios.c index 4d2cb0f85e2..1d9bde0c3c2 100644 --- a/lib/smbios.c +++ b/lib/smbios.c @@ -22,10 +22,13 @@ * * @node: node containing the information to write (ofnode_null() if none) * @dev: sysinfo device to use (NULL if none) + * @eos: end-of-string pointer for the table being processed. This is set + * up when we start processing a table */ struct smbios_ctx { ofnode node; struct udevice *dev; + char *eos; }; /** @@ -57,14 +60,15 @@ struct smbios_write_method { * This adds a string to the string area which is appended directly after * the formatted portion of an SMBIOS structure. * - * @start: string area start address + * @ctx: SMBIOS context * @str: string to add * @return:string number in the string area (1 or more) */ -static int smbios_add_string(char *start, const char *str) +static int smbios_add_string(struct smbios_ctx *ctx, const char *str) { int i = 1; - char *p = start; + char *p = ctx->eos; + if (!*str) str = "Unknown"; @@ -90,25 +94,28 @@ static int smbios_add_string(char *start, const char *str) * smbios_add_prop() - Add a property from the device tree * * @start: string area start address - * @ctx: context for writing the tables + * @node: node containing the information to write (ofnode_null() if none) * @prop: property to write * @return 0 if not found, else SMBIOS string number (1 or more) */ -static int smbios_add_prop(char *start, struct smbios_ctx *ctx, - const char *prop) +static int smbios_add_prop(struct smbios_ctx *ctx, const char *prop) { - if (IS_ENABLED(CONFIG_OF_CONTROL)) { const char *str; str = ofnode_read_string(ctx->node, prop); if (str) - return smbios_add_string(start, str); + return smbios_add_string(ctx, str); } return 0; } +static void set_eos(struct smbios_ctx *ctx, char *eos) +{ + ctx->eos = eos; +} + /** * smbios_string_table_len() - compute the string area size * @@ -140,9 +147,10 @@ static int smbios_write_type0(ulong *current, int handle, t = map_sysmem(*current, len); memset(t, 0, sizeof(struct smbios_type0)); fill_smbios_header(t, SMBIOS_BIOS_INFORMATION, len, handle); - t->vendor = smbios_add_string(t->eos, "U-Boot"); - t->bios_ver = smbios_add_string(t->eos, PLAIN_VERSION); - t->bios_release_date = smbios_add_string(t->eos, U_BOOT_DMI_DATE); + set_eos(ctx, t->eos); + t->vendor = smbios_add_string(ctx, "U-Boot"); + t->bios_ver = smbios_add_string(ctx, PLAIN_VERSION); + t->bios_release_date = smbios_add_string(ctx, U_BOOT_DMI_DATE); #ifdef CONFIG_ROM_SIZE t->bios_rom_size = (CONFIG_ROM_SIZE / 65536) - 1; #endif @@ -180,17 +188,18 @@ static int smbios_write_type1(ulong *current, int handle, t = map_sysmem(*current, len); memset(t, 0, sizeof(struct smbios_type1)); fill_smbios_header(t, SMBIOS_SYSTEM_INFORMATION, len, handle); - t->manufacturer = smbios_add_prop(t->eos, ctx, "manufacturer"); - t->product_name = smbios_add_prop(t->eos, ctx, "product"); - t->version = smbios_add_prop(t->eos, ctx, "version"); + set_eos(ctx, t->eos); + t->manufacturer = smbios_add_prop(ctx, "manufacturer"); + t->product_name = smbios_add_prop(ctx, "product"); + t->version = smbios_add_prop(ctx, "version"); if (serial_str) { - t->serial_number = smbios_add_string(t->eos, serial_str); + t->serial_number = smbios_add_string(ctx, serial_str); strncpy((char *)t->uuid, serial_str, sizeof(t->uuid)); } else { - t->serial_number = smbios_add_prop(t->eos, ctx, "serial"); + t->serial_number = smbios_add_prop(ctx, "serial"); } - t->sku_number = smbios_add_prop(t->eos, ctx, "sku"); - t->family = smbios_add_prop(t->eos, ctx, "family"); + t->sku_number = smbios_add_prop(ctx, "sku"); + t->family = smbios_add_prop(ctx, "family"); len = t->length + smbios_string_table_len(t->eos); *current += len; @@ -208,9 +217,10 @@ static int smbios_write_type2(ulong *current, int handle, t = map_sysmem(*current, len); memset(t, 0, sizeof(struct smbios_type2)); fill_smbios_header(t, SMBIOS_BOARD_INFORMATION, len, handle); -
[PATCH v2 06/12] smbios: Use a struct to keep track of context
At present we pass the ofnode to each function. We also pass the 'eos' pointer for adding new strings. We don't track the current end of the string table, so have smbios_string_table_len() to find that. The code can be made more efficient if it keeps information in a context struct. This also makes it easier to add more features. As a first step, switch the ofnode parameter to be a context pointer. Update smbios_add_prop() at the same time to avoid changing the same lines of code in consecutive patches. Signed-off-by: Simon Glass --- Changes in v2: - Zero the context's dev pointer if not used lib/smbios.c | 87 +--- 1 file changed, 55 insertions(+), 32 deletions(-) diff --git a/lib/smbios.c b/lib/smbios.c index a072d9ec10b..4d2cb0f85e2 100644 --- a/lib/smbios.c +++ b/lib/smbios.c @@ -17,15 +17,27 @@ #include #endif +/** + * struct smbios_ctx - context for writing SMBIOS tables + * + * @node: node containing the information to write (ofnode_null() if none) + * @dev: sysinfo device to use (NULL if none) + */ +struct smbios_ctx { + ofnode node; + struct udevice *dev; +}; + /** * Function prototype to write a specific type of SMBIOS structure * * @addr: start address to write the structure * @handle:the structure's handle, a unique 16-bit number - * @node: node containing the information to write (ofnode_null() if none) + * @ctx: context for writing the tables * @return:size of the structure */ -typedef int (*smbios_write_type)(ulong *addr, int handle, ofnode node); +typedef int (*smbios_write_type)(ulong *addr, int handle, +struct smbios_ctx *ctx); /** * struct smbios_write_method - Information about a table-writing function @@ -78,17 +90,18 @@ static int smbios_add_string(char *start, const char *str) * smbios_add_prop() - Add a property from the device tree * * @start: string area start address - * @node: node containing the information to write (ofnode_null() if none) + * @ctx: context for writing the tables * @prop: property to write * @return 0 if not found, else SMBIOS string number (1 or more) */ -static int smbios_add_prop(char *start, ofnode node, const char *prop) +static int smbios_add_prop(char *start, struct smbios_ctx *ctx, + const char *prop) { if (IS_ENABLED(CONFIG_OF_CONTROL)) { const char *str; - str = ofnode_read_string(node, prop); + str = ofnode_read_string(ctx->node, prop); if (str) return smbios_add_string(start, str); } @@ -118,7 +131,8 @@ static int smbios_string_table_len(char *start) return len + 1; } -static int smbios_write_type0(ulong *current, int handle, ofnode node) +static int smbios_write_type0(ulong *current, int handle, + struct smbios_ctx *ctx) { struct smbios_type0 *t; int len = sizeof(struct smbios_type0); @@ -156,7 +170,8 @@ static int smbios_write_type0(ulong *current, int handle, ofnode node) return len; } -static int smbios_write_type1(ulong *current, int handle, ofnode node) +static int smbios_write_type1(ulong *current, int handle, + struct smbios_ctx *ctx) { struct smbios_type1 *t; int len = sizeof(struct smbios_type1); @@ -165,17 +180,17 @@ static int smbios_write_type1(ulong *current, int handle, ofnode node) t = map_sysmem(*current, len); memset(t, 0, sizeof(struct smbios_type1)); fill_smbios_header(t, SMBIOS_SYSTEM_INFORMATION, len, handle); - t->manufacturer = smbios_add_prop(t->eos, node, "manufacturer"); - t->product_name = smbios_add_prop(t->eos, node, "product"); - t->version = smbios_add_prop(t->eos, node, "version"); + t->manufacturer = smbios_add_prop(t->eos, ctx, "manufacturer"); + t->product_name = smbios_add_prop(t->eos, ctx, "product"); + t->version = smbios_add_prop(t->eos, ctx, "version"); if (serial_str) { t->serial_number = smbios_add_string(t->eos, serial_str); strncpy((char *)t->uuid, serial_str, sizeof(t->uuid)); } else { - t->serial_number = smbios_add_prop(t->eos, node, "serial"); + t->serial_number = smbios_add_prop(t->eos, ctx, "serial"); } - t->sku_number = smbios_add_prop(t->eos, node, "sku"); - t->family = smbios_add_prop(t->eos, node, "family"); + t->sku_number = smbios_add_prop(t->eos, ctx, "sku"); + t->family = smbios_add_prop(t->eos, ctx, "family"); len = t->length + smbios_string_table_len(t->eos); *current += len; @@ -184,7 +199,8 @@ static int smbios_write_type1(ulong *current, int handle, ofnode node) return len; } -static int smbios_write_type2(ulong *current, int handle, ofnode node) +static int smbios_w
[PATCH v2 04/12] smbios: Use char consistently for the eos member
At present a few of the structs use u8 instead of char. This is a string, so char is better. Update them. Signed-off-by: Simon Glass Reviewed-by: Christian Gmeiner --- (no changes since v1) include/smbios.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/smbios.h b/include/smbios.h index fc69188a8fe..1cbeabf9522 100644 --- a/include/smbios.h +++ b/include/smbios.h @@ -183,14 +183,14 @@ struct __packed smbios_type32 { u16 handle; u8 reserved[6]; u8 boot_status; - u8 eos[SMBIOS_STRUCT_EOS_BYTES]; + char eos[SMBIOS_STRUCT_EOS_BYTES]; }; struct __packed smbios_type127 { u8 type; u8 length; u16 handle; - u8 eos[SMBIOS_STRUCT_EOS_BYTES]; + char eos[SMBIOS_STRUCT_EOS_BYTES]; }; struct __packed smbios_header { -- 2.30.0.296.g2bfb1c46d8-goog
[PATCH v2 03/12] smbios: Move smbios_write_type to the C file
This type is not used outside the smbios.c file so there is no need for it to be in the header file. Move it. Signed-off-by: Simon Glass Reviewed-by: Christian Gmeiner --- (no changes since v1) include/smbios.h | 10 -- lib/smbios.c | 10 ++ 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/include/smbios.h b/include/smbios.h index 1846607c3cf..fc69188a8fe 100644 --- a/include/smbios.h +++ b/include/smbios.h @@ -219,16 +219,6 @@ static inline void fill_smbios_header(void *table, int type, header->handle = handle; } -/** - * Function prototype to write a specific type of SMBIOS structure - * - * @addr: start address to write the structure - * @handle:the structure's handle, a unique 16-bit number - * @node: node containing the information to write (ofnode_null() if none) - * @return:size of the structure - */ -typedef int (*smbios_write_type)(ulong *addr, int handle, ofnode node); - /** * write_smbios_table() - Write SMBIOS table * diff --git a/lib/smbios.c b/lib/smbios.c index 1e10fa84207..b1171f544a8 100644 --- a/lib/smbios.c +++ b/lib/smbios.c @@ -17,6 +17,16 @@ #include #endif +/** + * Function prototype to write a specific type of SMBIOS structure + * + * @addr: start address to write the structure + * @handle:the structure's handle, a unique 16-bit number + * @node: node containing the information to write (ofnode_null() if none) + * @return:size of the structure + */ +typedef int (*smbios_write_type)(ulong *addr, int handle, ofnode node); + /** * struct smbios_write_method - Information about a table-writing function * -- 2.30.0.296.g2bfb1c46d8-goog
[PATCH v2 02/12] Makefile: Provide numeric versions
For SMBIOS we want to store the numeric version numbers in the tables. It does not make sense to parse the strings. Instead, add new #defines with the version and patchlevel. Signed-off-by: Simon Glass --- (no changes since v1) Makefile | 4 README | 8 2 files changed, 12 insertions(+) diff --git a/Makefile b/Makefile index cef149dec97..67972dbdb9b 100644 --- a/Makefile +++ b/Makefile @@ -1849,9 +1849,13 @@ prepare: prepare0 # Generate some files # --- +# Use sed to remove leading zeros from PATCHLEVEL to avoid using octal numbers define filechk_version.h (echo \#define PLAIN_VERSION \"$(UBOOTRELEASE)\"; \ echo \#define U_BOOT_VERSION \"U-Boot \" PLAIN_VERSION; \ + echo \#define U_BOOT_VERSION_NUM $(VERSION); \ + echo \#define U_BOOT_VERSION_NUM_PATCH $$(echo $(PATCHLEVEL) | \ + sed -e "s/^0*//"); \ echo \#define CC_VERSION_STRING \"$$(LC_ALL=C $(CC) --version | head -n 1)\"; \ echo \#define LD_VERSION_STRING \"$$(LC_ALL=C $(LD) --version | head -n 1)\"; ) endef diff --git a/README b/README index f32c69c8d77..37d21f18ca6 100644 --- a/README +++ b/README @@ -1898,6 +1898,14 @@ The following options need to be configured: U-Boot 2020.10 (Jan 06 2021 - 08:50:36 -0700) U-Boot 2021.01-rc5-00248-g60dd854f3ba-dirty (Jan 06 2021 - 08:50:36 -0700) for spring + U_BOOT_VERSION_NUM (integer #define) + Release year, e.g. 2021 for release 2021.01. Note + this is an integer, not a string. + + U_BOOT_VERSION_NUM_PATCH (integer #define) + Patch number, e.g. 1 for release 2020.01. Note + this is an integer, not a string. + Build date/time is also included. See the generated file include/generated/timestamp_autogenerated.h for the available fields. For example: -- 2.30.0.296.g2bfb1c46d8-goog
[PATCH v2 01/12] README: Add doumentation for version information
There are quite a few available version options in U-Boot. Add a list of the available Makefile variables and #defines, along with examples. Signed-off-by: Simon Glass --- (no changes since v1) README | 84 ++ 1 file changed, 84 insertions(+) diff --git a/README b/README index 89606c8adde..f32c69c8d77 100644 --- a/README +++ b/README @@ -1830,6 +1830,90 @@ The following options need to be configured: Time to wait after FPGA configuration. The default is 200 ms. +- Version identification: + + U-Boot releases are named by year and patch level, for example + 2020.10 means the release that came out in October 2020. + Release candidates are tagged every few weeks as the project + heads to the next release. So 2020.10-rc1 was the first + release candidate (RC), tagged soon after 2020.07 was released. + + See here for full details: + https://www.denx.de/wiki/view/U-Boot/ReleaseCycle + + Within the build system, various Makefile variables are created, + making use of VERSION, PATCHLEVEL and EXTRAVERSION defined at + the top of 'Makefile'. There is also SUBLEVEL available for + downstream use. See also CONFIG_IDENT_STRING. + + Some variables end up in a generated header file at + include/generated/version_autogenerated.h and can be accessed + from C source by including + + The following are available: + + UBOOTRELEASE (Makefile) + Full release version as a string. If this is not a + tagged release, it also includes the number of commits + since the last tag as well as the the git hash. If + there are uncommitted changes a '-dirty' suffix is + added too. + This is written by scripts/setlocalversion + (maintained by Linux) to include/config/uboot.release + and ends up in the UBOOTRELEASE Makefile variable. + + Examples: +2020.10-rc3 +2021.01-rc5-00248-g60dd854f3ba-dirty + + PLAIN_VERSION (string #define) + This is UBOOTRELEASE but available in C source. + + Examples: +2020.10 +2021.01-rc5-00248-g60dd854f3ba-dirty + + UBOOTVERSION (Makefile) + This holds just the first three components of + UBOOTRELEASE (i.e. not the git hash, etc.) + + Examples: +2020.10 +2021.01-rc5 + + U_BOOT_VERSION (string #define) + "U-Boot " followed by UBOOTRELEASE, for example: + +U-Boot 2020.10 +U-Boot 2021.01-rc5 + + This is used as part of the banner string when U-Boot + starts. + + U_BOOT_VERSION_STRING (string #define) + U_BOOT_VERSION followed by build-time information + and CONFIG_IDENT_STRING. + + Examples: +U-Boot 2020.10 (Jan 06 2021 - 08:50:36 -0700) + U-Boot 2021.01-rc5-00248-g60dd854f3ba-dirty (Jan 06 2021 - 08:50:36 -0700) for spring + + Build date/time is also included. See the generated file + include/generated/timestamp_autogenerated.h for the available + fields. For example: + + #define U_BOOT_DATE "Jan 06 2021" (US format only) + #define U_BOOT_TIME "08:50:36"(24-hour clock) + #define U_BOOT_TZ "-0700" (Time zone in hours) + #define U_BOOT_DMI_DATE "01/06/2021" (US format only) + #define U_BOOT_BUILD_DATE 0x20210106 (hex mmdd format) + #define U_BOOT_EPOCH 1609948236 + + The Epoch is the number of seconds since midnight on 1/1/70. + Every time you build U-Boot this will update based on the time + on your build machine. See 'Reproducible builds' if you want to + avoid that. + - Configuration Management: CONFIG_IDENT_STRING -- 2.30.0.296.g2bfb1c46d8-goog
[PATCH v2 00/12] smbios: Enhancements for more flexibility
This series includes various patches to allow more flexibility as to where the data for SMBIOS tables comes from: - introduces some standard sysinfo options as a source, e.g. to read strapping pins to determine the board revision - allows the U-Boot version number to be included - allows the version number to be provided programmatically, e.g. to support the build system adding information after U-Boot is built Documentation is added for how to obtain version information. The code is also refactored a little to make it easier to maintain. Changes in v2: - Add a comment about dropping the century - Zero the context's dev pointer if not used - Correct documentation format - Add new patch to fix sysinfo with CONFIG_IS_ENABLED() - Add new patch to fix crash on coral Simon Glass (12): README: Add doumentation for version information Makefile: Provide numeric versions smbios: Move smbios_write_type to the C file smbios: Use char consistently for the eos member smbios: Set BIOS release version smbios: Use a struct to keep track of context smbios: Drop the eos parameter smbios: Track the end of the string table smbios: Add more options for the BIOS version string sysinfo: Move #ifdef so that operations are always defined x86: coral: Add sysinfo ops smbios: Allow a few values to come from sysinfo Makefile | 4 + README| 92 ++ board/google/chromebook_coral/coral.c | 5 + include/asm-generic/global_data.h | 6 + include/smbios.h | 26 +-- include/sysinfo.h | 13 +- lib/smbios.c | 243 +++--- 7 files changed, 315 insertions(+), 74 deletions(-) -- 2.30.0.296.g2bfb1c46d8-goog
Re: [PATCH 1/1] cmd: CMD_ACPI depends on ACPIGEN
Am 20. Januar 2021 21:47:45 MEZ schrieb Andy Shevchenko : >On Wed, Jan 20, 2021 at 10:46:23PM +0200, Andy Shevchenko wrote: >> On Wed, Jan 20, 2021 at 09:37:56PM +0100, Heinrich Schuchardt wrote: >> > Trying to compile qemu-x86_64_defconfig with CONFIG_CMD_ACPI=y and >> > CONFIG_ACPIGEN=n fails with >> > >> > ld.bfd: cmd/built-in.o: in function `do_acpi_items': >> > cmd/acpi.c:162: undefined reference to `acpi_dump_items' >> > >> > Add the missing configuration dependency. > >... > >> > config CMD_ACPI >> >bool "acpi" >> > - default y if ACPIGEN >> > + depends on ACPIGEN >> > + default y >> >> Shouldn't be rather >> >> default ACPIGEN >> >> ? Thanks for reviewing. You cannot drop the depends line. If you only want to change the last line only, 'default y' is easier to read and matches the style of the rest of our code. Best regards Heinrich > >Actually it makes no difference...
Boot failure triggered by USB on rockpro64-rk3399 and pinebook-pro-rk3399
It seems rockpro64-rk3399 and pinebook-pro-rk3399 fail to boot when usb is started. It hangs indefinitely at: ## Flattened Device Tree blob at 01f0 Booting using the fdt blob at 0x1f0 I have observed this also using 2020.10 on rockpro64-rk3399, though on pinebook-pro-rk3399 usb does not work and so it basically avoids triggering the issue. Setting CONFIG_USE_PREBOOT=n in the config works around the problem, though obviously by breaking usb keyboard support or booting from USB devices. Related bugs in Debian and manjaro: https://bugs.debian.org/973323 https://bugs.debian.org/980434 https://gitlab.manjaro.org/manjaro-arm/packages/core/uboot-rockpro64/-/issues/4 Boot log: U-Boot 2021.01+dfsg-1 (Jan 17 2021 - 03:50:13 +) SoC: Rockchip rk3399 Reset cause: POR Model: Pine64 RockPro64 v2.1 DRAM: 3.9 GiB PMIC: RK808 MMC: mmc@fe31: 2, mmc@fe32: 1, sdhci@fe33: 0 Loading Environment from SPIFlash... SF: Detected gd25q128 with page size 256 Bytes, erase size 4 KiB, total 16 MiB *** Warning - bad CRC, using default environment In:serial Out: serial Err: serial Model: Pine64 RockPro64 v2.1 Net: eth0: ethernet@fe30 starting USB... Bus usb@fe38: USB EHCI 1.00 Bus usb@fe3a: USB OHCI 1.0 Bus usb@fe3c: USB EHCI 1.00 Bus usb@fe3e: USB OHCI 1.0 Bus dwc3: usb maximum-speed not found Register 2000140 NbrPorts 2 Starting the controller USB XHCI 1.10 scanning bus usb@fe38 for devices... 1 USB Device(s) found scanning bus usb@fe3a for devices... 1 USB Device(s) found scanning bus usb@fe3c for devices... 1 USB Device(s) found scanning bus usb@fe3e for devices... 1 USB Device(s) found scanning bus dwc3 for devices... 1 USB Device(s) found scanning usb for storage devices... 0 Storage Device(s) found Hit any key to stop autoboot: 0 => printenv preboot preboot=usb start => usb reset resetting USB... Bus usb@fe38: USB EHCI 1.00 Bus usb@fe3a: USB OHCI 1.0 Bus usb@fe3c: USB EHCI 1.00 Bus usb@fe3e: USB OHCI 1.0 Bus dwc3: usb maximum-speed not found Register 2000140 NbrPorts 2 Starting the controller USB XHCI 1.10 scanning bus usb@fe38 for devices... 1 USB Device(s) found scanning bus usb@fe3a for devices... 1 USB Device(s) found scanning bus usb@fe3c for devices... 1 USB Device(s) found scanning bus usb@fe3e for devices... 1 USB Device(s) found scanning bus dwc3 for devices... 1 USB Device(s) found scanning usb for storage devices... 0 Storage Device(s) found => boot Card did not respond to voltage select! : -110 switch to partitions #0, OK mmc1 is current device Scanning mmc 1:1... Found /extlinux/extlinux.conf Retrieving file: /extlinux/extlinux.conf 144 bytes read in 5 ms (27.3 KiB/s) 1: Debian-Installer Retrieving file: /initrd.gz 28995285 bytes read in 1287 ms (21.5 MiB/s) Retrieving file: /vmlinuz 26922864 bytes read in 1195 ms (21.5 MiB/s) Retrieving file: /dtbs/rockchip/rk3399-rockpro64.dtb 56849 bytes read in 13 ms (4.2 MiB/s) Moving Image from 0x208 to 0x220, end=3c5 ## Flattened Device Tree blob at 01f0 Booting using the fdt blob at 0x1f0 live well, vagrant signature.asc Description: PGP signature
[PATCH 1/1] fs: fat: structure for name and extension
The short name and extension of FAT files are stored in adjacent fields of the directory entry. For some operations like calculating a checksum or copying both fields it is preferable to treat both as one structure. Change the definition of the directory entry structure to include a structure comprising the name and the extension field. This resolves Coverity CID 316357, CID 316350, CID 316348. Signed-off-by: Heinrich Schuchardt --- fs/fat/fat.c | 32 fs/fat/fat_write.c | 20 +--- include/fat.h | 7 ++- 3 files changed, 31 insertions(+), 28 deletions(-) diff --git a/fs/fat/fat.c b/fs/fat/fat.c index 157dad60a4..fb6ce094ac 100644 --- a/fs/fat/fat.c +++ b/fs/fat/fat.c @@ -123,16 +123,16 @@ static void get_name(dir_entry *dirent, char *s_name) { char *ptr; - memcpy(s_name, dirent->name, 8); + memcpy(s_name, dirent->nameext.name, 8); s_name[8] = '\0'; ptr = s_name; while (*ptr && *ptr != ' ') ptr++; if (dirent->lcase & CASE_LOWER_BASE) downcase(s_name, (unsigned)(ptr - s_name)); - if (dirent->ext[0] && dirent->ext[0] != ' ') { + if (dirent->nameext.ext[0] && dirent->nameext.ext[0] != ' ') { *ptr++ = '.'; - memcpy(ptr, dirent->ext, 3); + memcpy(ptr, dirent->nameext.ext, 3); if (dirent->lcase & CASE_LOWER_EXT) downcase(ptr, 3); ptr[3] = '\0'; @@ -472,16 +472,15 @@ static int slot2str(dir_slot *slotptr, char *l_name, int *idx) } /* Calculate short name checksum */ -static __u8 mkcksum(const char name[8], const char ext[3]) +static __u8 mkcksum(struct nameext *nameext) { int i; + u8 *pos = (void *)nameext; __u8 ret = 0; - for (i = 0; i < 8; i++) - ret = (((ret & 1) << 7) | ((ret & 0xfe) >> 1)) + name[i]; - for (i = 0; i < 3; i++) - ret = (((ret & 1) << 7) | ((ret & 0xfe) >> 1)) + ext[i]; + for (i = 0; i < 11; i++) + ret = (((ret & 1) << 7) | ((ret & 0xfe) >> 1)) + pos[i]; return ret; } @@ -896,7 +895,7 @@ static dir_entry *next_dent(fat_itr *itr) } /* have we reached the last valid entry? */ - if (itr->dent->name[0] == 0) + if (itr->dent->nameext.name[0] == 0) return NULL; return itr->dent; @@ -905,7 +904,7 @@ static dir_entry *next_dent(fat_itr *itr) static dir_entry *extract_vfat_name(fat_itr *itr) { struct dir_entry *dent = itr->dent; - int seqn = itr->dent->name[0] & ~LAST_LONG_ENTRY_MASK; + int seqn = itr->dent->nameext.name[0] & ~LAST_LONG_ENTRY_MASK; u8 chksum, alias_checksum = ((dir_slot *)dent)->alias_checksum; int n = 0; @@ -932,18 +931,19 @@ static dir_entry *extract_vfat_name(fat_itr *itr) * We are now at the short file name entry. * If it is marked as deleted, just skip it. */ - if (dent->name[0] == DELETED_FLAG || - dent->name[0] == aRING) + if (dent->nameext.name[0] == DELETED_FLAG || + dent->nameext.name[0] == aRING) return NULL; itr->l_name[n] = '\0'; - chksum = mkcksum(dent->name, dent->ext); + chksum = mkcksum(&dent->nameext); /* checksum mismatch could mean deleted file, etc.. skip it: */ if (chksum != alias_checksum) { debug("** chksum=%x, alias_checksum=%x, l_name=%s, s_name=%8s.%3s\n", - chksum, alias_checksum, itr->l_name, dent->name, dent->ext); + chksum, alias_checksum, itr->l_name, dent->nameext.name, + dent->nameext.ext); return NULL; } @@ -984,12 +984,12 @@ static int fat_itr_next(fat_itr *itr) itr->dent_rem = itr->remaining; itr->dent_start = itr->dent; itr->dent_clust = itr->clust; - if (dent->name[0] == DELETED_FLAG) + if (dent->nameext.name[0] == DELETED_FLAG) continue; if (dent->attr & ATTR_VOLUME) { if ((dent->attr & ATTR_VFAT) == ATTR_VFAT && - (dent->name[0] & LAST_LONG_ENTRY_MASK)) { + (dent->nameext.name[0] & LAST_LONG_ENTRY_MASK)) { /* long file name */ dent = extract_vfat_name(itr); /* diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c index 2f4391e86f..a17e51750c 100644 --- a/fs/fat/fat_write.c +++ b/fs/fat/fat_write.c @@ -275,7 +275,7 @@ static int fat_find_empty_dentries(fat_itr *itr, int count) log_debug("Not enough directory entries available\n"); return -ENOSPC; } - switch (itr->dent->name[0]) { + switch (itr->dent->nameext.nam
Re: Contributor meeting notes 19-Jan-21
On 1/20/21 6:18 PM, Jaehoon Chung wrote: Hi, On 1/20/21 3:54 AM, Simon Glass wrote: Hi, Thank you for attending! Full notes at [1] Tuesday 19 January 2021 Present: Daniel Schwierzeck, Heinrich Schuchardt, Michal Simek, Sean Anderson, Simon Glass, Walter Lozano Notes: [all] Introductions [all] Timing of call - Current time is the best across US, Europe, India. But not any good for East Asia In South Korea, it will be almost am 01:30. Frankly, i can't speak English very well..If it doesn't matter, i hope to attend sometime. :)\ One option could be to alternate times. E.g. one meeting pick a time best for one hemisphere, and another call pick a time best for the other. For example, the next meeting could be at 4:30 UTC (since we just met at 16:30 UTC) --Sean Best Regards, Jaehoon Chung - Simon might send out survey - Send invitation to maintainer group - DONE [Simon] New sequence numbers [Heinrich] Online docs - https://protect2.fireeye.com/v1/url?k=93d7f10b-cc4cc86e-93d67a44-000babff32e3-8edc69528000b522&q=1&e=ae252977-3a7b-4c4a-ab06-1c4711467fc0&u=https%3A%2F%2Fu-boot.readthedocs.io%2F - Accept a merge request only with docs included? Next meeting Tuesday 2 Feb. [1] https://protect2.fireeye.com/v1/url?k=030f1752-5c942e37-030e9c1d-000babff32e3-e989b91a6d0ade29&q=1&e=ae252977-3a7b-4c4a-ab06-1c4711467fc0&u=https%3A%2F%2Fdocs.google.com%2Fdocument%2Fd%2F1YBOMsbM19uSFyoJWnt7-PsOLBaevzQUgV-hiR88a5-o%2Fedit%23 Regards, Simon
Re: Contributor meeting notes 19-Jan-21
Hi, On 1/20/21 3:54 AM, Simon Glass wrote: > Hi, > > Thank you for attending! > > Full notes at [1] > > Tuesday 19 January 2021 > > Present: Daniel Schwierzeck, Heinrich Schuchardt, Michal Simek, Sean > Anderson, Simon Glass, Walter Lozano > > Notes: > [all] Introductions > [all] Timing of call > - Current time is the best across US, Europe, India. But not any good > for East Asia In South Korea, it will be almost am 01:30. Frankly, i can't speak English very well..If it doesn't matter, i hope to attend sometime. :) Best Regards, Jaehoon Chung > - Simon might send out survey > - Send invitation to maintainer group - DONE > [Simon] New sequence numbers > [Heinrich] Online docs > - > https://protect2.fireeye.com/v1/url?k=93d7f10b-cc4cc86e-93d67a44-000babff32e3-8edc69528000b522&q=1&e=ae252977-3a7b-4c4a-ab06-1c4711467fc0&u=https%3A%2F%2Fu-boot.readthedocs.io%2F > - Accept a merge request only with docs included? > > Next meeting Tuesday 2 Feb. > > [1] > https://protect2.fireeye.com/v1/url?k=030f1752-5c942e37-030e9c1d-000babff32e3-e989b91a6d0ade29&q=1&e=ae252977-3a7b-4c4a-ab06-1c4711467fc0&u=https%3A%2F%2Fdocs.google.com%2Fdocument%2Fd%2F1YBOMsbM19uSFyoJWnt7-PsOLBaevzQUgV-hiR88a5-o%2Fedit%23 > > Regards, > Simon >