[PATCH 5/5] dma: ti: k3-udma: invalidate prepared buffers before pushing to recv ring
Buffers must not have an unclean cache before being used for DMA - a pending write-back may corrupt the next dev-to-mem transfer otherwise. This was consistently noticeable during long TFTP transfers, when an ARP request is answered by U-Boot in the middle of the transfer: As U-Boot's arp_receive() reuses the receive buffer to prepare its reply packet, the beginning of one of the next incoming TFTP packets is overwritten by the ARP reply. The corrupted packet is ignored, but the TFTP transfer stalls for a few seconds until a timeout is detected and a retransmit is triggered. Signed-off-by: Matthias Schiffer --- drivers/dma/ti/k3-udma.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/dma/ti/k3-udma.c b/drivers/dma/ti/k3-udma.c index 4e6f7f570c5..8f6d396653e 100644 --- a/drivers/dma/ti/k3-udma.c +++ b/drivers/dma/ti/k3-udma.c @@ -2678,6 +2678,9 @@ int udma_prepare_rcv_buf(struct dma *dma, void *dst, size_t size) cppi5_hdesc_set_pktlen(desc_rx, size); cppi5_hdesc_attach_buf(desc_rx, dma_dst, size, dma_dst, size); + invalidate_dcache_range((unsigned long)dma_dst, + (unsigned long)(dma_dst + size)); + flush_dcache_range((unsigned long)desc_rx, ALIGN((unsigned long)desc_rx + uc->config.hdesc_size, ARCH_DMA_MINALIGN)); -- TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany Amtsgericht München, HRB 105018 Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider https://www.tq-group.com/
[PATCH 4/5] dma: ti: k3-udma: add missing include
net.h is needed for PKTBUFSRX. Without this definition, the driver will always use 4 RX buffers, causing am65-cpsw-nuss initialization to fail when a higher number of buffers is requested. Signed-off-by: Matthias Schiffer --- drivers/dma/ti/k3-udma.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/dma/ti/k3-udma.c b/drivers/dma/ti/k3-udma.c index ef3074aa13f..4e6f7f570c5 100644 --- a/drivers/dma/ti/k3-udma.c +++ b/drivers/dma/ti/k3-udma.c @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include -- TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany Amtsgericht München, HRB 105018 Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider https://www.tq-group.com/
[PATCH 3/5] net: ti: am65-cpsw-nuss: fix error handling for "RX dma add buf failed"
The RX DMA channel has been requested at this point already, so it must be freed. Signed-off-by: Matthias Schiffer --- drivers/net/ti/am65-cpsw-nuss.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ti/am65-cpsw-nuss.c b/drivers/net/ti/am65-cpsw-nuss.c index 4a57e945a3a..65ade1afd05 100644 --- a/drivers/net/ti/am65-cpsw-nuss.c +++ b/drivers/net/ti/am65-cpsw-nuss.c @@ -362,7 +362,7 @@ static int am65_cpsw_start(struct udevice *dev) UDMA_RX_BUF_SIZE); if (ret) { dev_err(dev, "RX dma add buf failed %d\n", ret); - goto err_free_tx; + goto err_free_rx; } } -- TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany Amtsgericht München, HRB 105018 Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider https://www.tq-group.com/
[PATCH 2/5] net: ti: am65-cpsw-nuss: avoid errors due to imbalanced start()/stop()
The eth-uclass state machine doesn't prevent imbalanced start()/stop() calls - to the contrary, it even provides eth_init_state_only() and eth_halt_state_only() functions that change the state without any calls into the driver. This means that the driver must be robust against duplicate start() and stop() calls as well as send/recv calls while the interface is down. We decide not to print error messages but just to return an error in the latter case, as trying to send packets on a disabled interface commonly happens when the netconsole is still active after the Ethernet has been halted during bootm. Signed-off-by: Matthias Schiffer --- drivers/net/ti/am65-cpsw-nuss.c | 9 + 1 file changed, 9 insertions(+) diff --git a/drivers/net/ti/am65-cpsw-nuss.c b/drivers/net/ti/am65-cpsw-nuss.c index b151e25d6a4..4a57e945a3a 100644 --- a/drivers/net/ti/am65-cpsw-nuss.c +++ b/drivers/net/ti/am65-cpsw-nuss.c @@ -328,6 +328,9 @@ static int am65_cpsw_start(struct udevice *dev) struct ti_udma_drv_chan_cfg_data *dma_rx_cfg_data; int ret, i; + if (common->started) + return 0; + ret = power_domain_on(&common->pwrdmn); if (ret) { dev_err(dev, "power_domain_on() failed %d\n", ret); @@ -488,6 +491,9 @@ static int am65_cpsw_send(struct udevice *dev, void *packet, int length) struct ti_udma_drv_packet_data packet_data; int ret; + if (!common->started) + return -ENETDOWN; + packet_data.pkt_type = AM65_CPSW_CPPI_PKT_TYPE; packet_data.dest_tag = priv->port_id; ret = dma_send(&common->dma_tx, packet, length, &packet_data); @@ -504,6 +510,9 @@ static int am65_cpsw_recv(struct udevice *dev, int flags, uchar **packetp) struct am65_cpsw_priv *priv = dev_get_priv(dev); struct am65_cpsw_common *common = priv->cpsw_common; + if (!common->started) + return -ENETDOWN; + /* try to receive a new packet */ return dma_receive(&common->dma_rx, (void **)packetp, NULL); } -- TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany Amtsgericht München, HRB 105018 Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider https://www.tq-group.com/
[PATCH 1/5] net: eth-uclass: guard against reentrant eth_init()/eth_halt() calls
With netconsole, any log message can result in an eth_init(), possibly causing an reentrant call into eth_init() if a driver's ops print anything: eth_init() -> driver.start() -> printf() -> netconsole -> eth_init() eth_halt() -> driver.stop() -> printf() -> netconsole -> eth_init() Rather than expecting every single Ethernet driver to handle this case, prevent the reentrant calls in eth_init() and eth_halt(). The issue was noticed on an AM62x board, where a bootm after simultaneous netconsole and TFTP would result in a crash. Signed-off-by: Matthias Schiffer --- net/eth-uclass.c | 40 1 file changed, 32 insertions(+), 8 deletions(-) diff --git a/net/eth-uclass.c b/net/eth-uclass.c index 3d0ec91dfa4..193218a328b 100644 --- a/net/eth-uclass.c +++ b/net/eth-uclass.c @@ -48,6 +48,8 @@ struct eth_uclass_priv { /* eth_errno - This stores the most recent failure code from DM functions */ static int eth_errno; +/* Are we currently in eth_init() or eth_halt()? */ +static bool in_init_halt; /* board-specific Ethernet Interface initializations. */ __weak int board_interface_eth_init(struct udevice *dev, @@ -285,11 +287,19 @@ U_BOOT_ENV_CALLBACK(ethaddr, on_ethaddr); int eth_init(void) { - char *ethact = env_get("ethact"); - char *ethrotate = env_get("ethrotate"); struct udevice *current = NULL; struct udevice *old_current; int ret = -ENODEV; + char *ethrotate; + char *ethact; + + if (in_init_halt) + return -EBUSY; + + in_init_halt = true; + + ethact = env_get("ethact"); + ethrotate = env_get("ethrotate"); /* * When 'ethrotate' variable is set to 'no' and 'ethact' variable @@ -298,8 +308,10 @@ int eth_init(void) if ((ethrotate != NULL) && (strcmp(ethrotate, "no") == 0)) { if (ethact) { current = eth_get_dev_by_name(ethact); - if (!current) - return -EINVAL; + if (!current) { + ret = -EINVAL; + goto end; + } } } @@ -307,7 +319,8 @@ int eth_init(void) current = eth_get_dev(); if (!current) { log_err("No ethernet found.\n"); - return -ENODEV; + ret = -ENODEV; + goto end; } } @@ -324,7 +337,8 @@ int eth_init(void) priv->state = ETH_STATE_ACTIVE; priv->running = true; - return 0; + ret = 0; + goto end; } } else { ret = eth_errno; @@ -344,6 +358,8 @@ int eth_init(void) current = eth_get_dev(); } while (old_current != current); +end: + in_init_halt = false; return ret; } @@ -352,17 +368,25 @@ void eth_halt(void) struct udevice *current; struct eth_device_priv *priv; + if (in_init_halt) + return; + + in_init_halt = true; + current = eth_get_dev(); if (!current) - return; + goto end; priv = dev_get_uclass_priv(current); if (!priv || !priv->running) - return; + goto end; eth_get_ops(current)->stop(current); priv->state = ETH_STATE_PASSIVE; priv->running = false; + +end: + in_init_halt = false; } int eth_is_active(struct udevice *dev) -- TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany Amtsgericht München, HRB 105018 Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider https://www.tq-group.com/
Re: [PATCH 3/4] arm: dts: k3-*-binman.dtsi: Clean up and templatize boot binaries
On Wed, 2024-04-03 at 17:51 +0200, Michael Walle wrote: > Hi, > > > > > > > And on top of that, it will just be a base board and there will > > > > > > likely be some carrier device trees (overlay? I'm not sure yet). > > > > > > > > > > > > As far as I can tell, you've put the memory configuration into the > > > > > > device tree, so I'll probably need to switch between them somehow. > > > > > > > > > > The "k3--ddr.dtsi" file will be present in your k3-r5.dts > > > > > which makes sense, the memory configuration depends on the board. > > > > > > > > > > k3--ddr.dtsi* (e.g J721E EVM vs. SK boards consume different memory > > > configurations. > > Right. > > > > > And one board might have multiple configuration depending on the > > > > variant of the board. Typically, one board is available with > > > > different memory options. i.e. 1GiB, 4GiB and so on. The actual RAM > > > > chips can come from different manufacturers. So all all, I presume > > > > there will be different RAM settings, i.e. different > > > > k3--ddr.dtsi. But I have to switch between the setting during > > > > runtime because there will be only one boot image for that board. > > > > > > This is a runtime dynamic DDR configuration support you are describing > > > correct? This means you would be including all the supported memory option > > > DTSIs in your k3--r5.dts correct and probably do some board magic > > > code in the SPL DDR driver to choose the DTB. How is this affecting the > > > packing of the final bootloader which will anyways pack the whole R5 DTB? > > Correct, the DDR configuration should be chosen at runtime after > reading some board strappings. Unless, it will work with with the > same configuration which seems unlikely to me. But it is not an > unusual configuration I'd say. > > I haven't looked into this in detail, but to me it seems not that > obvious how to do that in a generic/upstreamable way. Multiple > device nodes sounds wrong. Thus, I'd likely need different device > trees for the different memory configurations for the R5 SPL. Not > sure that is yet possible with u-boot, though. If you have any > better idea, I'm all ears. > > > > > > > Also, regarding the board variants, I'll probably need to choose > > > > > > between multiple device trees. That is invisible to the user, > > > > > > because u-boot will choose the correct DTB according a board > > > > > > strapping, which btw. works really fine, see for example > > > > > > (boards/kontron/sl28/spl.c:board_fit_config_name_match). > > > > > > > > > > Again, this is assuming that there is some HW blown register available > > > > > for the board to use (or in our earlier K3 case, the EEPROM) but that > > > > > is > > > > > not necessarily true every time. > > > > > > > > No, that is of course board dependent. It is just an example that > > > > there are boards with more than one DTB. > > > > > > > > Let's step back a bit. Right now, there is > > > >k3---binman.dtsi > > > > which is fine. But it seems, that TI is heading towards a common > > > >k3--binman.dtsi > > > > which is intended to be used by all the boards that are using that > > > > particular SoC, correct me if I'm wrong here. Now the problem with > > > > that is that you hardcode the FIT configuations which are really > > > > board dependent and assume that there will be exactly one DTB per > > > > board, i.e. your "#define SPL_BOARD_DTB" etc. > > > > > > > > > > Correct, but as I mentioned in the earlier message, if your board supports > > > more than 1 FIT configuration, you can easily extend the image and add > > > more > > > configurations. > > > > > > > Thus, what I was trying to say is that you should split all the > > > > board independent configuration (dt fragments) from the board > > > > specific configuration. > > > > > > > > And again, of course I could just ignore the k3--binman.dtsi > > > > and just use a suitable copy "k3---binman.dtsi" for my > > > > board. But as I said, I'm not sure, this is the way to go and I have > > > > a slight feeling I will be asked to reuse the "k3--binman.dtsi" > > > > when I submit my board support. > > > > > > > > > > > > > > > > I don't think it makes much sense to hardcode your generic > > > > > > *-binman.dtsi to just one FIT configuration. I'd rather see a split > > > > > > between generic things which are shared across all boards and board > > > > > > specifics, like the FIT configuration. I mean I could just copy all > > > > > > > > > > Correct me if I'm wrong, but my understanding is that you would want > > > > > to > > > > > add more FDT blobs in the *-binman.dtsi correct? That is still > > > > > possible, > > > > > adding another "fdt-1" and "conf-1" in the > > > > > > > > > > Something like this in your -u-boot.dtsi, > > > > > > > > > > tispl { > > > > > insert-template = <&ti_spl>; > > > > > fit { > > > > > images { > > > > > fdt-1 { > > > > >
Re: [PATCH] boot: add support for fdt_fixup command in environment
On Wed, 2023-12-13 at 19:52 +, Simon Glass wrote: > > Achtung externe E-Mail: Öffnen Sie Anhänge und Links nur, wenn Sie wissen, > dass diese aus einer sicheren Quelle stammen und sicher sind. Leiten Sie die > E-Mail im Zweifelsfall zur Prüfung an den IT-Helpdesk weiter. > Attention external email: Open attachments and links only if you know that > they are from a secure source and are safe. In doubt forward the email to the > IT-Helpdesk to check it. > > > On Mon, 11 Dec 2023 at 04:04, Matthias Schiffer > wrote: > > > > The "fdt" command is convenient for making small changes to the OS FDT, > > especially during development. This is easy when the kernel and FDT are > > loaded separately, but can be cumbersome for FIT images, requiring to > > unpack the image, manually apply overlays, etc. > > > > Add an option to execute a command "fdt_fixup" from the environment at > > the beginning of image_setup_libfdt() (after overlays are applied, and > > before the other fixups). > > > > Signed-off-by: Matthias Schiffer > > --- > > boot/Kconfig | 9 + > > boot/image-fdt.c | 19 +-- > > 2 files changed, 26 insertions(+), 2 deletions(-) > > Reviewed-by: Simon Glass > > How about a test? Makes sense, I'll add a unit test. > > > > > > diff --git a/boot/Kconfig b/boot/Kconfig > > index ef71883a502..7eea935f490 100644 > > --- a/boot/Kconfig > > +++ b/boot/Kconfig > > @@ -1502,6 +1502,15 @@ if OF_LIBFDT > > > > menu "Devicetree fixup" > > > > +config OF_ENV_SETUP > > + bool "Run a command from environment to set up device tree before > > boot" > > + depends on CMD_FDT > > + help > > + This causes U-Boot to run a command from the environment variable > > + fdt_fixup before booting into the operating system, which can use > > the > > + fdt command to modify the device tree. The device tree is then > > passed > > + to the OS. > > + > > config OF_BOARD_SETUP > > bool "Set up board-specific details in device tree before boot" > > help > > diff --git a/boot/image-fdt.c b/boot/image-fdt.c > > index f10200f6474..78b5c639381 100644 > > --- a/boot/image-fdt.c > > +++ b/boot/image-fdt.c > > @@ -9,6 +9,7 @@ > > */ > > > > #include > > +#include > > #include > > #include > > #include > > @@ -608,8 +609,22 @@ int image_setup_libfdt(struct bootm_headers *images, > > void *blob, > > { > > ulong *initrd_start = &images->initrd_start; > > ulong *initrd_end = &images->initrd_end; > > - int ret = -EPERM; > > - int fdt_ret; > > + int ret, fdt_ret; > > + > > + if (IS_ENABLED(CONFIG_OF_ENV_SETUP)) { > > + const char *fdt_fixup; > > + > > + fdt_fixup = env_get("fdt_fixup"); > > + if (fdt_fixup) { > > + set_working_fdt_addr(map_to_sysmem(blob)); > > Is that not already done? Not necessarily in all code paths that lead to image_setup_libfdt(). > > > + ret = run_command_list(fdt_fixup, -1, 0); > > + if (ret) > > + printf("WARNING: fdt_fixup command returned > > %d\n", > > + ret); > > + } > > Would it make sense to put this code near the end of the function, > after other fixups have been done? With separate image and FDT, one would usually run the fdt command to modify the FDT before bootm, so the modifications also happen before other fixups. My intention was to get the same behavior for FIT images. I don't care much either way though, for most fixups the order shouldn't matter too much, and we can always add an fdt_fixup_early or fdt_fixup_late if an actual usecase for such a sequence of fixups comes up. Regards, Matthias > > > + } > > + > > + ret = -EPERM; > > > > if (fdt_root(blob) < 0) { > > printf("ERROR: root node setup failed\n"); > > -- > > TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany > > Amtsgericht München, HRB 105018 > > Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider > > https://www.tq-group.com/ > > > > Regards, > Simon > -- TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany Amtsgericht München, HRB 105018 Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider https://www.tq-group.com/
[PATCH] boot: add support for fdt_fixup command in environment
The "fdt" command is convenient for making small changes to the OS FDT, especially during development. This is easy when the kernel and FDT are loaded separately, but can be cumbersome for FIT images, requiring to unpack the image, manually apply overlays, etc. Add an option to execute a command "fdt_fixup" from the environment at the beginning of image_setup_libfdt() (after overlays are applied, and before the other fixups). Signed-off-by: Matthias Schiffer --- boot/Kconfig | 9 + boot/image-fdt.c | 19 +-- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/boot/Kconfig b/boot/Kconfig index ef71883a502..7eea935f490 100644 --- a/boot/Kconfig +++ b/boot/Kconfig @@ -1502,6 +1502,15 @@ if OF_LIBFDT menu "Devicetree fixup" +config OF_ENV_SETUP + bool "Run a command from environment to set up device tree before boot" + depends on CMD_FDT + help + This causes U-Boot to run a command from the environment variable + fdt_fixup before booting into the operating system, which can use the + fdt command to modify the device tree. The device tree is then passed + to the OS. + config OF_BOARD_SETUP bool "Set up board-specific details in device tree before boot" help diff --git a/boot/image-fdt.c b/boot/image-fdt.c index f10200f6474..78b5c639381 100644 --- a/boot/image-fdt.c +++ b/boot/image-fdt.c @@ -9,6 +9,7 @@ */ #include +#include #include #include #include @@ -608,8 +609,22 @@ int image_setup_libfdt(struct bootm_headers *images, void *blob, { ulong *initrd_start = &images->initrd_start; ulong *initrd_end = &images->initrd_end; - int ret = -EPERM; - int fdt_ret; + int ret, fdt_ret; + + if (IS_ENABLED(CONFIG_OF_ENV_SETUP)) { + const char *fdt_fixup; + + fdt_fixup = env_get("fdt_fixup"); + if (fdt_fixup) { + set_working_fdt_addr(map_to_sysmem(blob)); + ret = run_command_list(fdt_fixup, -1, 0); + if (ret) + printf("WARNING: fdt_fixup command returned %d\n", + ret); + } + } + + ret = -EPERM; if (fdt_root(blob) < 0) { printf("ERROR: root node setup failed\n"); -- TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany Amtsgericht München, HRB 105018 Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider https://www.tq-group.com/
[PATCH] arm: mach-k3: common: fix compile warnings with PHYS_64BIT on 32bit
Use uintptr_t instead of phys_addr_t where appropriate, so passing the addresses to writel() doesn't result in compile warnings when PHYS_64BIT is set for 32bit builds (which is actually a useful configuration, as the K3 SoC family boots from an R5 SPL, which may pass bank information based on gd->bd->bi_dram to fdt_fixup_memory_banks() etc., so PHYS_64BIT is needed for fixing up the upper bank). Signed-off-by: Matthias Schiffer --- arch/arm/mach-k3/common.c | 4 ++-- arch/arm/mach-k3/common.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/arch/arm/mach-k3/common.c b/arch/arm/mach-k3/common.c index a35110429b2..9b90b2fa11c 100644 --- a/arch/arm/mach-k3/common.c +++ b/arch/arm/mach-k3/common.c @@ -83,10 +83,10 @@ void k3_sysfw_print_ver(void) ti_sci->version.firmware_revision, fw_desc); } -void mmr_unlock(phys_addr_t base, u32 partition) +void mmr_unlock(uintptr_t base, u32 partition) { /* Translate the base address */ - phys_addr_t part_base = base + partition * CTRL_MMR0_PARTITION_SIZE; + uintptr_t part_base = base + partition * CTRL_MMR0_PARTITION_SIZE; /* Unlock the requested partition if locked using two-step sequence */ writel(CTRLMMR_LOCK_KICK0_UNLOCK_VAL, part_base + CTRLMMR_LOCK_KICK0); diff --git a/arch/arm/mach-k3/common.h b/arch/arm/mach-k3/common.h index 9bd9ad6d1a0..eabb44f6204 100644 --- a/arch/arm/mach-k3/common.h +++ b/arch/arm/mach-k3/common.h @@ -38,7 +38,7 @@ void remove_fwl_configs(struct fwl_data *fwl_data, size_t fwl_data_size); int load_firmware(char *name_fw, char *name_loadaddr, u32 *loadaddr); void k3_sysfw_print_ver(void); void spl_enable_dcache(void); -void mmr_unlock(phys_addr_t base, u32 partition); +void mmr_unlock(uintptr_t base, u32 partition); bool is_rom_loaded_sysfw(struct rom_extended_boot_data *data); enum k3_device_type get_device_type(void); void ti_secure_image_post_process(void **p_image, size_t *p_size); -- TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany Amtsgericht München, HRB 105018 Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider https://www.tq-group.com/
[PATCH 4/5] pinctrl: single: fix compile warnings with PHYS_64BIT on 32bit
pinctrl-single uses fdt_addr_t and phys_addr_t inconsistently, but both are wrong to be passed to readb() etc., which expect a pointer or pointer-sized integer. Change the driver to use dev_read_addr_size_index_ptr(), so we consistently deal with void* (except for the sandbox case and single_get_pin_muxing()). Signed-off-by: Matthias Schiffer --- Tested on x86 sandbox and TI AM62x. No new unit test failures in sandbox. drivers/pinctrl/pinctrl-single.c | 33 +--- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c index d80281fd3dd..fb34f681740 100644 --- a/drivers/pinctrl/pinctrl-single.c +++ b/drivers/pinctrl/pinctrl-single.c @@ -24,7 +24,7 @@ * @bits_per_mux: true if one register controls more than one pin */ struct single_pdata { - fdt_addr_t base; + void *base; int offset; u32 mask; u32 width; @@ -97,7 +97,7 @@ struct single_fdt_bits_cfg { #if (!IS_ENABLED(CONFIG_SANDBOX)) -static unsigned int single_read(struct udevice *dev, fdt_addr_t reg) +static unsigned int single_read(struct udevice *dev, void *reg) { struct single_pdata *pdata = dev_get_plat(dev); @@ -113,7 +113,7 @@ static unsigned int single_read(struct udevice *dev, fdt_addr_t reg) return readb(reg); } -static void single_write(struct udevice *dev, unsigned int val, fdt_addr_t reg) +static void single_write(struct udevice *dev, unsigned int val, void *reg) { struct single_pdata *pdata = dev_get_plat(dev); @@ -131,18 +131,18 @@ static void single_write(struct udevice *dev, unsigned int val, fdt_addr_t reg) #else /* CONFIG_SANDBOX */ -static unsigned int single_read(struct udevice *dev, fdt_addr_t reg) +static unsigned int single_read(struct udevice *dev, void *reg) { struct single_priv *priv = dev_get_priv(dev); - return priv->sandbox_regs[reg]; + return priv->sandbox_regs[map_to_sysmem(reg)]; } -static void single_write(struct udevice *dev, unsigned int val, fdt_addr_t reg) +static void single_write(struct udevice *dev, unsigned int val, void *reg) { struct single_priv *priv = dev_get_priv(dev); - priv->sandbox_regs[reg] = val; + priv->sandbox_regs[map_to_sysmem(reg)] = val; } #endif /* CONFIG_SANDBOX */ @@ -214,7 +214,8 @@ static int single_get_pin_muxing(struct udevice *dev, unsigned int pin, { struct single_pdata *pdata = dev_get_plat(dev); struct single_priv *priv = dev_get_priv(dev); - fdt_addr_t reg; + phys_addr_t phys_reg; + void *reg; const char *fname; unsigned int val; int offset, pin_shift = 0; @@ -226,13 +227,15 @@ static int single_get_pin_muxing(struct udevice *dev, unsigned int pin, reg = pdata->base + offset; val = single_read(dev, reg); + phys_reg = map_to_sysmem(reg); + if (pdata->bits_per_mux) pin_shift = pin % (pdata->width / priv->bits_per_pin) * priv->bits_per_pin; val &= (pdata->mask << pin_shift); fname = single_get_pin_function(dev, pin); - snprintf(buf, size, "%pa 0x%08x %s", ®, val, + snprintf(buf, size, "%pa 0x%08x %s", &phys_reg, val, fname ? fname : "UNCLAIMED"); return 0; } @@ -243,7 +246,7 @@ static int single_request(struct udevice *dev, int pin, int flags) struct single_pdata *pdata = dev_get_plat(dev); struct single_gpiofunc_range *frange = NULL; struct list_head *pos, *tmp; - phys_addr_t reg; + void *reg; int mux_bytes = 0; u32 data; @@ -321,7 +324,7 @@ static int single_configure_pins(struct udevice *dev, int stride = pdata->args_count + 1; int n, pin, count = size / sizeof(u32); struct single_func *func; - phys_addr_t reg; + void *reg; u32 offset, val, mux; /* If function mask is null, needn't enable it. */ @@ -379,7 +382,7 @@ static int single_configure_bits(struct udevice *dev, int n, pin, count = size / sizeof(struct single_fdt_bits_cfg); int npins_in_reg, pin_num_from_lsb; struct single_func *func; - phys_addr_t reg; + void *reg; u32 offset, val, mask, bit_pos, val_pos, mask_pos, submask; /* If function mask is null, needn't enable it. */ @@ -570,7 +573,7 @@ static int single_probe(struct udevice *dev) static int single_of_to_plat(struct udevice *dev) { - fdt_addr_t addr; + void *addr; fdt_size_t size; struct single_pdata *pdata = dev_get_plat(dev); int ret; @@ -591,8 +594,8 @@ static int single_of_to_plat(struct udevice *dev) return -EINVAL; } - addr = dev_read_addr_size_index(dev, 0, &size); - if (addr == FDT_ADDR_T_NONE) { + addr
[PATCH 5/5] treewide: use dev_read_addr_*_ptr() where appropriate
A follow-up to commit 842fb5de424e ("drivers: use devfdt_get_addr_size_index_ptr when cast to pointer") and commit 320a1938b6f7 ("drivers: use devfdt_get_addr_index_ptr when cast to pointer"). In addition to using the *_ptr variants of these functions where the address is cast to a pointer, this also changes devfdt_get_addr_*() to dev_read_addr_*() in a few places. Some variable and field types are changed from fdt_addr_t or phys_addr_t to void* where the cast was happening later. This patch fixes a number of compile warnings when building a 32bit U-Boot with CONFIG_PHYS_64BIT=y. In some places, it also fixes error handling where the return value of dev_read_addr() etc. was checked for NULL instead of FDT_ADDR_T_NONE. Signed-off-by: Matthias Schiffer --- This seems to work correctly (tested on x86 sandbox and TI AM62x; I have not tested the Tegra, Sun4i and BCM drivers), but I have two questions: It is not entirely clear to me what the difference between dev_read_addr_ptr*() and dev_remap_addr*() etc. is, but some drivers mix both. Should dev_remap_addr*() be used for __iomem? Is __iomem used consistently in U-Boot at all? Furthermore, can devfdt_get_*() be replaced with dev_read_*() unconditionally? Is there any reason why devfdt_get_*() hasn't been dropped entirely in a treewide search-and-replace? The k3-sec-proxy change goes on top of my other patch "mailbox: k3-sec-proxy: fix error handling for missing scfg in FDT" I submitted yesterday. arch/arm/mach-k3/sysfw-loader.c | 16 drivers/dma/ti/k3-udma.c | 5 ++--- drivers/gpio/tegra186_gpio.c | 4 ++-- drivers/mailbox/k3-sec-proxy.c| 18 +- drivers/phy/allwinner/phy-sun4i-usb.c | 12 ++-- drivers/phy/phy-bcm-sr-pcie.c | 4 ++-- drivers/ram/k3-am654-ddrss.c | 20 ++-- drivers/ram/k3-ddrss/k3-ddrss.c | 23 ++- drivers/soc/ti/k3-navss-ringacc.c | 12 ++-- 9 files changed, 55 insertions(+), 59 deletions(-) diff --git a/arch/arm/mach-k3/sysfw-loader.c b/arch/arm/mach-k3/sysfw-loader.c index 9be2d9eaea2..ef245fef9c4 100644 --- a/arch/arm/mach-k3/sysfw-loader.c +++ b/arch/arm/mach-k3/sysfw-loader.c @@ -321,7 +321,7 @@ exit: static void *k3_sysfw_get_spi_addr(void) { struct udevice *dev; - fdt_addr_t addr; + void *addr; int ret; unsigned int sf_bus = spl_spi_boot_bus(); @@ -329,11 +329,11 @@ static void *k3_sysfw_get_spi_addr(void) if (ret) return NULL; - addr = dev_read_addr_index(dev, 1); - if (addr == FDT_ADDR_T_NONE) + addr = dev_read_addr_index_ptr(dev, 1); + if (!addr) return NULL; - return (void *)(addr + CONFIG_K3_SYSFW_IMAGE_SPI_OFFS); + return addr + CONFIG_K3_SYSFW_IMAGE_SPI_OFFS; } static void k3_sysfw_spi_copy(u32 *dst, u32 *src, size_t len) @@ -349,18 +349,18 @@ static void k3_sysfw_spi_copy(u32 *dst, u32 *src, size_t len) static void *get_sysfw_hf_addr(void) { struct udevice *dev; - fdt_addr_t addr; + void *addr; int ret; ret = uclass_find_first_device(UCLASS_MTD, &dev); if (ret) return NULL; - addr = dev_read_addr_index(dev, 1); - if (addr == FDT_ADDR_T_NONE) + addr = dev_read_addr_index_ptr(dev, 1); + if (!addr) return NULL; - return (void *)(addr + CONFIG_K3_SYSFW_IMAGE_SPI_OFFS); + return addr + CONFIG_K3_SYSFW_IMAGE_SPI_OFFS; } #endif diff --git a/drivers/dma/ti/k3-udma.c b/drivers/dma/ti/k3-udma.c index 05c3a4311ce..1847c8889aa 100644 --- a/drivers/dma/ti/k3-udma.c +++ b/drivers/dma/ti/k3-udma.c @@ -1286,7 +1286,7 @@ static int udma_get_mmrs(struct udevice *dev) u32 cap2, cap3, cap4; int i; - ud->mmrs[MMR_GCFG] = (uint32_t *)devfdt_get_addr_name(dev, mmr_names[MMR_GCFG]); + ud->mmrs[MMR_GCFG] = dev_read_addr_name_ptr(dev, mmr_names[MMR_GCFG]); if (!ud->mmrs[MMR_GCFG]) return -EINVAL; @@ -1324,8 +1324,7 @@ static int udma_get_mmrs(struct udevice *dev) if (i == MMR_RCHANRT && ud->rchan_cnt == 0) continue; - ud->mmrs[i] = (uint32_t *)devfdt_get_addr_name(dev, - mmr_names[i]); + ud->mmrs[i] = dev_read_addr_name_ptr(dev, mmr_names[i]); if (!ud->mmrs[i]) return -EINVAL; } diff --git a/drivers/gpio/tegra186_gpio.c b/drivers/gpio/tegra186_gpio.c index 82dcaf96312..94a20d143e1 100644 --- a/drivers/gpio/tegra186_gpio.c +++ b/drivers/gpio/tegra186_gpio.c @@ -176,8 +176,8 @@ static int tegra186_gpio_bind(struct udevice *parent) if (parent_plat) return 0; - regs = (uint32_t *)devfdt_get_addr_name(parent, "gpio"); - if (regs == (u
[PATCH 3/5] core: introduce dev_read_addr_name[_size]_ptr() functions
Same as dev_read_addr_name[_size](), but returns a pointer, cast through map_sysmem(). Signed-off-by: Matthias Schiffer --- drivers/core/fdtaddr.c | 21 + drivers/core/read.c| 21 + include/dm/fdtaddr.h | 31 +++ include/dm/read.h | 41 + 4 files changed, 114 insertions(+) diff --git a/drivers/core/fdtaddr.c b/drivers/core/fdtaddr.c index 426bb762754..560b0b634a2 100644 --- a/drivers/core/fdtaddr.c +++ b/drivers/core/fdtaddr.c @@ -153,6 +153,16 @@ fdt_addr_t devfdt_get_addr_name(const struct udevice *dev, const char *name) #endif } +void *devfdt_get_addr_name_ptr(const struct udevice *dev, const char *name) +{ + fdt_addr_t addr = devfdt_get_addr_name(dev, name); + + if (addr == FDT_ADDR_T_NONE) + return NULL; + + return map_sysmem(addr, 0); +} + fdt_addr_t devfdt_get_addr_size_name(const struct udevice *dev, const char *name, fdt_size_t *size) { @@ -170,6 +180,17 @@ fdt_addr_t devfdt_get_addr_size_name(const struct udevice *dev, #endif } +void *devfdt_get_addr_size_name_ptr(const struct udevice *dev, + const char *name, fdt_size_t *size) +{ + fdt_addr_t addr = devfdt_get_addr_size_name(dev, name, size); + + if (addr == FDT_ADDR_T_NONE) + return NULL; + + return map_sysmem(addr, 0); +} + fdt_addr_t devfdt_get_addr(const struct udevice *dev) { return devfdt_get_addr_index(dev, 0); diff --git a/drivers/core/read.c b/drivers/core/read.c index 49066b59cda..0908321c846 100644 --- a/drivers/core/read.c +++ b/drivers/core/read.c @@ -181,6 +181,16 @@ fdt_addr_t dev_read_addr_name(const struct udevice *dev, const char *name) return dev_read_addr_index(dev, index); } +void *dev_read_addr_name_ptr(const struct udevice *dev, const char *name) +{ + fdt_addr_t addr = dev_read_addr_name(dev, name); + + if (addr == FDT_ADDR_T_NONE) + return NULL; + + return map_sysmem(addr, 0); +} + fdt_addr_t dev_read_addr_size_name(const struct udevice *dev, const char *name, fdt_size_t *size) { @@ -192,6 +202,17 @@ fdt_addr_t dev_read_addr_size_name(const struct udevice *dev, const char *name, return dev_read_addr_size_index(dev, index, size); } +void *dev_read_addr_size_name_ptr(const struct udevice *dev, const char *name, + fdt_size_t *size) +{ + fdt_addr_t addr = dev_read_addr_size_name(dev, name, size); + + if (addr == FDT_ADDR_T_NONE) + return NULL; + + return map_sysmem(addr, 0); +} + void *dev_remap_addr_name(const struct udevice *dev, const char *name) { fdt_addr_t addr = dev_read_addr_name(dev, name); diff --git a/include/dm/fdtaddr.h b/include/dm/fdtaddr.h index d3ad77faebf..750ca0076ed 100644 --- a/include/dm/fdtaddr.h +++ b/include/dm/fdtaddr.h @@ -146,6 +146,19 @@ void *devfdt_get_addr_size_index_ptr(const struct udevice *dev, int index, */ fdt_addr_t devfdt_get_addr_name(const struct udevice *dev, const char *name); +/** + * devfdt_get_addr_name_ptr() - Get the reg property of a device as a pointer, + * indexed by name + * + * @dev: Pointer to a device + * @name: the 'reg' property can hold a list of pairs, with the + * 'reg-names' property providing named-based identification. @name + * indicates the value to search for in 'reg-names'. + * + * Return: Pointer to addr, or NULL if there is no such property + */ +void *devfdt_get_addr_name_ptr(const struct udevice *dev, const char *name); + /** * devfdt_get_addr_size_name() - Get the reg property and its size for a device, * indexed by name @@ -164,6 +177,24 @@ fdt_addr_t devfdt_get_addr_name(const struct udevice *dev, const char *name); fdt_addr_t devfdt_get_addr_size_name(const struct udevice *dev, const char *name, fdt_size_t *size); +/** + * devfdt_get_addr_size_name_ptr() - Get the reg property for a device as a + * pointer, indexed by name + * + * Returns the address and size specified in the 'reg' property of a device. + * + * @dev: Pointer to a device + * @name: the 'reg' property can hold a list of pairs, with the + * 'reg-names' property providing named-based identification. @name + * indicates the value to search for in 'reg-names'. + * @size: Pointer to size variable - this function returns the size + *specified in the 'reg' property here + * + * Return: Pointer to addr, or NULL if there is no such property + */ +void *devfdt_get_addr_size_name_ptr(const struct udevice *dev, + const char *name, fdt_size_t *size); + /*
[PATCH 2/5] core: return FDT_ADDR_T_NONE from devfdt_get_addr_[size_]name() on errors
Checking for the error cast to fdt_addr_t is rather awkward - IS_ERR() can be used, but it's not really made to be used on fdt_addr_t, which may not even be the same size as a native pointer. Most places in U-Boot only check for FDT_ADDR_T_NONE; let's adjust the error return to match the expectation. Signed-off-by: Matthias Schiffer --- drivers/core/fdtaddr.c | 4 ++-- include/dm/fdtaddr.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/core/fdtaddr.c b/drivers/core/fdtaddr.c index 546db675aaf..426bb762754 100644 --- a/drivers/core/fdtaddr.c +++ b/drivers/core/fdtaddr.c @@ -145,7 +145,7 @@ fdt_addr_t devfdt_get_addr_name(const struct udevice *dev, const char *name) index = fdt_stringlist_search(gd->fdt_blob, dev_of_offset(dev), "reg-names", name); if (index < 0) - return index; + return FDT_ADDR_T_NONE; return devfdt_get_addr_index(dev, index); #else @@ -162,7 +162,7 @@ fdt_addr_t devfdt_get_addr_size_name(const struct udevice *dev, index = fdt_stringlist_search(gd->fdt_blob, dev_of_offset(dev), "reg-names", name); if (index < 0) - return index; + return FDT_ADDR_T_NONE; return devfdt_get_addr_size_index(dev, index, size); #else diff --git a/include/dm/fdtaddr.h b/include/dm/fdtaddr.h index ca788dccb39..d3ad77faebf 100644 --- a/include/dm/fdtaddr.h +++ b/include/dm/fdtaddr.h @@ -142,7 +142,7 @@ void *devfdt_get_addr_size_index_ptr(const struct udevice *dev, int index, * 'reg-names' property providing named-based identification. @name * indicates the value to search for in 'reg-names'. * - * Return: addr + * Return: Address, or FDT_ADDR_T_NONE if there is no such property */ fdt_addr_t devfdt_get_addr_name(const struct udevice *dev, const char *name); @@ -159,7 +159,7 @@ fdt_addr_t devfdt_get_addr_name(const struct udevice *dev, const char *name); * @size: Pointer to size variable - this function returns the size *specified in the 'reg' property here * - * Return: addr + * Return: Address, or FDT_ADDR_T_NONE if there is no such property */ fdt_addr_t devfdt_get_addr_size_name(const struct udevice *dev, const char *name, fdt_size_t *size); -- TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany Amtsgericht München, HRB 105018 Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider https://www.tq-group.com/
[PATCH 1/5] core: fix doc comments of dev_read_addr*() and related functions
- The dev_read_addr_name*() family of functions has no "index" argument, doc comments should refer to "name" - Specify the error return for several devfdt_get_addr*() functions Signed-off-by: Matthias Schiffer --- include/dm/fdtaddr.h | 12 ++-- include/dm/read.h| 6 +++--- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/include/dm/fdtaddr.h b/include/dm/fdtaddr.h index dcdc19137cc..ca788dccb39 100644 --- a/include/dm/fdtaddr.h +++ b/include/dm/fdtaddr.h @@ -19,7 +19,7 @@ struct udevice; * * @dev: Pointer to a device * - * Return: addr + * Return: Address, or FDT_ADDR_T_NONE if there is no such property */ fdt_addr_t devfdt_get_addr(const struct udevice *dev); @@ -59,7 +59,7 @@ void *devfdt_remap_addr_index(const struct udevice *dev, int index); * devfdt_remap_addr_name() - Get the reg property of a device, indexed by *name, as a memory-mapped I/O pointer * @name: the 'reg' property can hold a list of pairs, with the - * 'reg-names' property providing named-based identification. @index + * 'reg-names' property providing named-based identification. @name * indicates the value to search for in 'reg-names'. * * @dev: Pointer to a device @@ -87,7 +87,7 @@ void *devfdt_map_physmem(const struct udevice *dev, unsigned long size); * @index: the 'reg' property can hold a list of pairs *and @index is used to select which one is required * - * Return: addr + * Return: Address, or FDT_ADDR_T_NONE if there is no such property */ fdt_addr_t devfdt_get_addr_index(const struct udevice *dev, int index); @@ -114,7 +114,7 @@ void *devfdt_get_addr_index_ptr(const struct udevice *dev, int index); * @size: Pointer to size variable - this function returns the size *specified in the 'reg' property here * - * Return: addr + * Return: Address, or FDT_ADDR_T_NONE if there is no such property */ fdt_addr_t devfdt_get_addr_size_index(const struct udevice *dev, int index, fdt_size_t *size); @@ -139,7 +139,7 @@ void *devfdt_get_addr_size_index_ptr(const struct udevice *dev, int index, * * @dev: Pointer to a device * @name: the 'reg' property can hold a list of pairs, with the - * 'reg-names' property providing named-based identification. @index + * 'reg-names' property providing named-based identification. @name * indicates the value to search for in 'reg-names'. * * Return: addr @@ -154,7 +154,7 @@ fdt_addr_t devfdt_get_addr_name(const struct udevice *dev, const char *name); * * @dev: Pointer to a device * @name: the 'reg' property can hold a list of pairs, with the - * 'reg-names' property providing named-based identification. @index + * 'reg-names' property providing named-based identification. @name * indicates the value to search for in 'reg-names'. * @size: Pointer to size variable - this function returns the size *specified in the 'reg' property here diff --git a/include/dm/read.h b/include/dm/read.h index c2615f72f40..da7732a170f 100644 --- a/include/dm/read.h +++ b/include/dm/read.h @@ -277,7 +277,7 @@ void *dev_remap_addr_index(const struct udevice *dev, int index); * * @dev: Device to read from * @name: the 'reg' property can hold a list of pairs, with the - * 'reg-names' property providing named-based identification. @index + * 'reg-names' property providing named-based identification. @name * indicates the value to search for in 'reg-names'. * * Return: address or FDT_ADDR_T_NONE if not found @@ -289,7 +289,7 @@ fdt_addr_t dev_read_addr_name(const struct udevice *dev, const char *name); * * @dev: Device to read from * @name: the 'reg' property can hold a list of pairs, with the - * 'reg-names' property providing named-based identification. @index + * 'reg-names' property providing named-based identification. @name * indicates the value to search for in 'reg-names'. * @size: place to put size value (on success) * @@ -304,7 +304,7 @@ fdt_addr_t dev_read_addr_size_name(const struct udevice *dev, const char *name, * * @dev: Device to read from * @name: the 'reg' property can hold a list of pairs, with the - * 'reg-names' property providing named-based identification. @index + * 'reg-names' property providing named-based identification. @name * indicates the value to search for in 'reg-names'. * * Return: pointer or NULL if not found -- TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany Amtsgericht München, HRB 105018 Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider https://www.tq-group.com/
[PATCH] mailbox: k3-sec-proxy: fix error handling for missing scfg in FDT
The wrong field was checked. Fixes: f9aa41023bd9 ("mailbox: Introduce K3 Secure Proxy Driver") Signed-off-by: Matthias Schiffer --- drivers/mailbox/k3-sec-proxy.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/mailbox/k3-sec-proxy.c b/drivers/mailbox/k3-sec-proxy.c index 815808498f2..27072610462 100644 --- a/drivers/mailbox/k3-sec-proxy.c +++ b/drivers/mailbox/k3-sec-proxy.c @@ -326,7 +326,7 @@ static int k3_sec_proxy_of_to_priv(struct udevice *dev, } spm->scfg = devfdt_get_addr_name(dev, "scfg"); - if (spm->rt == FDT_ADDR_T_NONE) { + if (spm->scfg == FDT_ADDR_T_NONE) { dev_err(dev, "No reg property for Secure Cfg base\n"); return -EINVAL; } -- TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany Amtsgericht München, HRB 105018 Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider https://www.tq-group.com/
Re: [PATCH 1/2] Revert "lib: string: Fix strlcpy return value", fix callers
On Fri, 2023-07-14 at 13:24 +0200, Matthias Schiffer wrote: > Both the Linux kernel and libbsd agree that strlcpy() should always > return strlen(src) and not include the NUL termination. The incorrect > U-Boot implementation makes it impossible to check the return value for > truncation, and breaks code written with the usual implementation in > mind (for example, fdtdec_add_reserved_memory() was subtly broken). > > I reviewed all callers of strlcpy() and strlcat() and fixed them > according to my understanding of the intended function. > > This reverts commit d3358ecc54be0bc3b4dd11f7a63eab0a2842f772 and adds > related fixes. > > Fixes: d3358ecc54be ("lib: string: Fix strlcpy return value") > Signed-off-by: Matthias Schiffer Ping~ strlcpy and strlcat are now also in glibc and might be added to POSIX, so it would be great if we could get the U-Boot implementation to match the common behaviour. Regards, Matthias > --- > board/amlogic/vim3/vim3.c| 6 +++--- > drivers/fastboot/fb_getvar.c | 2 +- > lib/string.c | 14 +++--- > test/lib/strlcat.c | 4 ++-- > 4 files changed, 13 insertions(+), 13 deletions(-) > > diff --git a/board/amlogic/vim3/vim3.c b/board/amlogic/vim3/vim3.c > index fcd60ab1e05..8bdfb302f72 100644 > --- a/board/amlogic/vim3/vim3.c > +++ b/board/amlogic/vim3/vim3.c > @@ -104,8 +104,8 @@ int meson_ft_board_setup(void *blob, struct bd_info *bd) > } > > /* Update PHY names (mandatory to disable USB3.0) */ > - len = strlcpy(data, "usb2-phy0", 32); > - len += strlcpy(&data[len], "usb2-phy1", 32 - len); > + len = strlcpy(data, "usb2-phy0", 32) + 1; > + len += strlcpy(&data[len], "usb2-phy1", 32 - len) + 1; > ret = fdt_setprop(blob, node, "phy-names", data, len); > if (ret < 0) { > printf("vim3: failed to update usb phy names property > (%d)\n", ret); > @@ -132,7 +132,7 @@ int meson_ft_board_setup(void *blob, struct bd_info *bd) > } > > /* Enable PCIe */ > - len = strlcpy(data, "okay", 32); > + len = strlcpy(data, "okay", 32) + 1; > ret = fdt_setprop(blob, node, "status", data, len); > if (ret < 0) { > printf("vim3: failed to enable pcie node (%d)\n", ret); > diff --git a/drivers/fastboot/fb_getvar.c b/drivers/fastboot/fb_getvar.c > index dd3475e0a8b..d9f0f07b2bc 100644 > --- a/drivers/fastboot/fb_getvar.c > +++ b/drivers/fastboot/fb_getvar.c > @@ -183,7 +183,7 @@ static void __maybe_unused getvar_has_slot(char > *part_name, char *response) > > /* part_name_wslot = part_name + "_a" */ > len = strlcpy(part_name_wslot, part_name, PART_NAME_LEN - 3); > - if (len > PART_NAME_LEN - 3) > + if (len >= PART_NAME_LEN - 3) > goto fail; > strcat(part_name_wslot, "_a"); > > diff --git a/lib/string.c b/lib/string.c > index ecea755f405..f2c61471288 100644 > --- a/lib/string.c > +++ b/lib/string.c > @@ -116,20 +116,18 @@ char * strncpy(char * dest,const char *src,size_t count) > * of course, the buffer size is zero). It does not pad > * out the result like strncpy() does. > * > - * Return: the number of bytes copied > + * Return: strlen(src) > */ > size_t strlcpy(char *dest, const char *src, size_t size) > { > - if (size) { > - size_t srclen = strlen(src); > - size_t len = (srclen >= size) ? size - 1 : srclen; > + size_t ret = strlen(src); > > + if (size) { > + size_t len = (ret >= size) ? size - 1 : ret; > memcpy(dest, src, len); > dest[len] = '\0'; > - return len + 1; > } > - > - return 0; > + return ret; > } > #endif > > @@ -191,6 +189,8 @@ char * strncat(char *dest, const char *src, size_t count) > * Compatible with *BSD: the result is always a valid NUL-terminated string > that > * fits in the buffer (unless, of course, the buffer size is zero). It does > not > * write past @size like strncat() does. > + * > + * Return: min(strlen(dest), size) + strlen(src) > */ > size_t strlcat(char *dest, const char *src, size_t size) > { > diff --git a/test/lib/strlcat.c b/test/lib/strlcat.c > index a0ec037388b..d8453fe78e2 100644 > --- a/test/lib/strlcat.c > +++ b/test/lib/strlcat.c > @@ -43,11 +43,11 @@ static int do_test_strlcat(struct unit_test_state *uts, > int line, size
[PATCH 1/2] Revert "lib: string: Fix strlcpy return value", fix callers
Both the Linux kernel and libbsd agree that strlcpy() should always return strlen(src) and not include the NUL termination. The incorrect U-Boot implementation makes it impossible to check the return value for truncation, and breaks code written with the usual implementation in mind (for example, fdtdec_add_reserved_memory() was subtly broken). I reviewed all callers of strlcpy() and strlcat() and fixed them according to my understanding of the intended function. This reverts commit d3358ecc54be0bc3b4dd11f7a63eab0a2842f772 and adds related fixes. Fixes: d3358ecc54be ("lib: string: Fix strlcpy return value") Signed-off-by: Matthias Schiffer --- board/amlogic/vim3/vim3.c| 6 +++--- drivers/fastboot/fb_getvar.c | 2 +- lib/string.c | 14 +++--- test/lib/strlcat.c | 4 ++-- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/board/amlogic/vim3/vim3.c b/board/amlogic/vim3/vim3.c index fcd60ab1e05..8bdfb302f72 100644 --- a/board/amlogic/vim3/vim3.c +++ b/board/amlogic/vim3/vim3.c @@ -104,8 +104,8 @@ int meson_ft_board_setup(void *blob, struct bd_info *bd) } /* Update PHY names (mandatory to disable USB3.0) */ - len = strlcpy(data, "usb2-phy0", 32); - len += strlcpy(&data[len], "usb2-phy1", 32 - len); + len = strlcpy(data, "usb2-phy0", 32) + 1; + len += strlcpy(&data[len], "usb2-phy1", 32 - len) + 1; ret = fdt_setprop(blob, node, "phy-names", data, len); if (ret < 0) { printf("vim3: failed to update usb phy names property (%d)\n", ret); @@ -132,7 +132,7 @@ int meson_ft_board_setup(void *blob, struct bd_info *bd) } /* Enable PCIe */ - len = strlcpy(data, "okay", 32); + len = strlcpy(data, "okay", 32) + 1; ret = fdt_setprop(blob, node, "status", data, len); if (ret < 0) { printf("vim3: failed to enable pcie node (%d)\n", ret); diff --git a/drivers/fastboot/fb_getvar.c b/drivers/fastboot/fb_getvar.c index dd3475e0a8b..d9f0f07b2bc 100644 --- a/drivers/fastboot/fb_getvar.c +++ b/drivers/fastboot/fb_getvar.c @@ -183,7 +183,7 @@ static void __maybe_unused getvar_has_slot(char *part_name, char *response) /* part_name_wslot = part_name + "_a" */ len = strlcpy(part_name_wslot, part_name, PART_NAME_LEN - 3); - if (len > PART_NAME_LEN - 3) + if (len >= PART_NAME_LEN - 3) goto fail; strcat(part_name_wslot, "_a"); diff --git a/lib/string.c b/lib/string.c index ecea755f405..f2c61471288 100644 --- a/lib/string.c +++ b/lib/string.c @@ -116,20 +116,18 @@ char * strncpy(char * dest,const char *src,size_t count) * of course, the buffer size is zero). It does not pad * out the result like strncpy() does. * - * Return: the number of bytes copied + * Return: strlen(src) */ size_t strlcpy(char *dest, const char *src, size_t size) { - if (size) { - size_t srclen = strlen(src); - size_t len = (srclen >= size) ? size - 1 : srclen; + size_t ret = strlen(src); + if (size) { + size_t len = (ret >= size) ? size - 1 : ret; memcpy(dest, src, len); dest[len] = '\0'; - return len + 1; } - - return 0; + return ret; } #endif @@ -191,6 +189,8 @@ char * strncat(char *dest, const char *src, size_t count) * Compatible with *BSD: the result is always a valid NUL-terminated string that * fits in the buffer (unless, of course, the buffer size is zero). It does not * write past @size like strncat() does. + * + * Return: min(strlen(dest), size) + strlen(src) */ size_t strlcat(char *dest, const char *src, size_t size) { diff --git a/test/lib/strlcat.c b/test/lib/strlcat.c index a0ec037388b..d8453fe78e2 100644 --- a/test/lib/strlcat.c +++ b/test/lib/strlcat.c @@ -43,11 +43,11 @@ static int do_test_strlcat(struct unit_test_state *uts, int line, size_t align1, s2[i] = 32 + 23 * i % (127 - 32); s2[len2 - 1] = '\0'; - expected = len2 < n ? min(len1 + len2 - 1, n) : n; + expected = min(strlen(s2), n) + strlen(s1); actual = strlcat(s2, s1, n); if (expected != actual) { ut_failf(uts, __FILE__, line, __func__, -"strlcat(s2, s1, 2) == len2 < n ? min(len1 + len2, n) : n", +"strlcat(s2, s1, n) == min(len2, n) + len1", "Expected %#zx (%zd), got %#zx (%zd)", expected, expected, actual, actual); return CMD_RET_FAILURE; -- TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany Amtsgericht München, HRB 105018 Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider https://www.tq-group.com/
[PATCH 2/2] lib/charset: fix u16_strlcat() return value
strlcat returns min(strlen(dest), count)+strlen(src). Make u16_strlcat's behaviour the same for consistency. Fixes: eca08ce94ceb ("lib/charset: add u16_strlcat() function") Signed-off-by: Matthias Schiffer --- lib/charset.c | 8 test/unicode_ut.c | 8 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/charset.c b/lib/charset.c index b1842755eb1..5e4c4f948a4 100644 --- a/lib/charset.c +++ b/lib/charset.c @@ -444,14 +444,14 @@ u16 *u16_strdup(const void *src) size_t u16_strlcat(u16 *dest, const u16 *src, size_t count) { - size_t destlen = u16_strlen(dest); + size_t destlen = u16_strnlen(dest, count); size_t srclen = u16_strlen(src); - size_t ret = destlen + srclen + 1; + size_t ret = destlen + srclen; if (destlen >= count) return ret; - if (ret > count) - srclen -= ret - count; + if (ret >= count) + srclen -= (ret - count + 1); memcpy(&dest[destlen], src, 2 * srclen); dest[destlen + srclen] = 0x; diff --git a/test/unicode_ut.c b/test/unicode_ut.c index b27d7116b9e..a9356e2b60d 100644 --- a/test/unicode_ut.c +++ b/test/unicode_ut.c @@ -808,12 +808,12 @@ static int unicode_test_u16_strlcat(struct unit_test_state *uts) /* dest and src are empty string */ memset(buf, 0, sizeof(buf)); ret = u16_strlcat(buf, &null_src, sizeof(buf)); - ut_asserteq(1, ret); + ut_asserteq(0, ret); /* dest is empty string */ memset(buf, 0, sizeof(buf)); ret = u16_strlcat(buf, src, sizeof(buf)); - ut_asserteq(5, ret); + ut_asserteq(4, ret); ut_assert(!unicode_test_u16_strcmp(buf, src, 40)); /* src is empty string */ @@ -821,14 +821,14 @@ static int unicode_test_u16_strlcat(struct unit_test_state *uts) buf[39] = 0; memcpy(buf, dest, sizeof(dest)); ret = u16_strlcat(buf, &null_src, sizeof(buf)); - ut_asserteq(6, ret); + ut_asserteq(5, ret); ut_assert(!unicode_test_u16_strcmp(buf, dest, 40)); for (i = 0; i <= 40; i++) { memset(buf, 0xCD, (sizeof(buf) - sizeof(u16))); buf[39] = 0; memcpy(buf, dest, sizeof(dest)); - expected = 10; + expected = min(5, i) + 4; ret = u16_strlcat(buf, src, i); ut_asserteq(expected, ret); if (i <= 6) { -- TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany Amtsgericht München, HRB 105018 Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider https://www.tq-group.com/
[PATCH] video: backlight: pwm: avoid integer overflow in duty cycle calculation
The intermediate value could overflow for large periods and levels. Signed-off-by: Matthias Schiffer --- drivers/video/pwm_backlight.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/video/pwm_backlight.c b/drivers/video/pwm_backlight.c index d7c096923b3..46c16a8f447 100644 --- a/drivers/video/pwm_backlight.c +++ b/drivers/video/pwm_backlight.c @@ -63,7 +63,7 @@ static int set_pwm(struct pwm_backlight_priv *priv) int ret; if (priv->period_ns) { - duty_cycle = priv->period_ns * (priv->cur_level - priv->min_level) / + duty_cycle = (u64)priv->period_ns * (priv->cur_level - priv->min_level) / (priv->max_level - priv->min_level); ret = pwm_set_config(priv->pwm, priv->channel, priv->period_ns, duty_cycle); -- TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany Amtsgericht München, HRB 105018 Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider https://www.tq-group.com/
Re: [PATCH] ARM: mx7: psci: fix suspend/resume e10133 workaround
On Mon, 2022-09-26 at 10:31 +0200, Matthias Schiffer wrote: > The e10133 workaround was broken in two places: > > - The code intended to temporarily mask all interrupts in GPC_IMRx_CORE0. > While the old register values were saved, the actual masking was > missing. > - imx_udelay() expects the system counter to run at its base frequency, > but the system counter is switched to a lower frequency earlier in > psci_system_suspend(), leading to a much longer delay than intended. > Replace the call with an equivalent loop (linux-imx 5.15 does the same) > > This fixes the SoC hanging forever when there was already a wakeup IRQ > pending while suspending. > > Fixes: 57b620255e ("imx: mx7: add system suspend/resume support") > Signed-off-by: Matthias Schiffer Ping, any feedback on this? I'm not entirely sure anymore if this solution is adequate, as the duration of the loop depends on the CPU clock frequency and cache enable/disable state. Maybe a modified imx_udelay() that accounts for the changed system counter configuration would be necessary after all? > --- > arch/arm/mach-imx/mx7/psci-mx7.c | 9 ++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/arch/arm/mach-imx/mx7/psci-mx7.c > b/arch/arm/mach-imx/mx7/psci-mx7.c > index f32945ea37..699a2569cb 100644 > --- a/arch/arm/mach-imx/mx7/psci-mx7.c > +++ b/arch/arm/mach-imx/mx7/psci-mx7.c > @@ -643,8 +643,10 @@ __secure void psci_system_suspend(u32 __always_unused > function_id, > /* disable GIC distributor */ > writel(0, GIC400_ARB_BASE_ADDR + GIC_DIST_OFFSET); > > - for (i = 0; i < 4; i++) > + for (i = 0; i < 4; i++) { > gpc_mask[i] = readl(GPC_IPS_BASE_ADDR + GPC_IMR1_CORE0 + i * 4); > + writel(~0, GPC_IPS_BASE_ADDR + GPC_IMR1_CORE0 + i * 4); > + } > > /* >* enable the RBC bypass counter here > @@ -668,7 +670,7 @@ __secure void psci_system_suspend(u32 __always_unused > function_id, > writel(gpc_mask[i], GPC_IPS_BASE_ADDR + GPC_IMR1_CORE0 + i * 4); > > /* > - * now delay for a short while (3usec) > + * now delay for a short while (~3usec) >* ARM is at 1GHz at this point >* so a short loop should be enough. >* this delay is required to ensure that > @@ -677,7 +679,8 @@ __secure void psci_system_suspend(u32 __always_unused > function_id, >* or in case an interrupt arrives just >* as ARM is about to assert DSM_request. >*/ > - imx_udelay(3); > + for (i = 0; i < 2000; i++) > + asm volatile(""); > > /* save resume entry and sp in CPU0 GPR registers */ > asm volatile("mov %0, sp" : "=r" (val));
[PATCH] ARM: mx7: psci: fix suspend/resume e10133 workaround
The e10133 workaround was broken in two places: - The code intended to temporarily mask all interrupts in GPC_IMRx_CORE0. While the old register values were saved, the actual masking was missing. - imx_udelay() expects the system counter to run at its base frequency, but the system counter is switched to a lower frequency earlier in psci_system_suspend(), leading to a much longer delay than intended. Replace the call with an equivalent loop (linux-imx 5.15 does the same) This fixes the SoC hanging forever when there was already a wakeup IRQ pending while suspending. Fixes: 57b620255e ("imx: mx7: add system suspend/resume support") Signed-off-by: Matthias Schiffer --- arch/arm/mach-imx/mx7/psci-mx7.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/arch/arm/mach-imx/mx7/psci-mx7.c b/arch/arm/mach-imx/mx7/psci-mx7.c index f32945ea37..699a2569cb 100644 --- a/arch/arm/mach-imx/mx7/psci-mx7.c +++ b/arch/arm/mach-imx/mx7/psci-mx7.c @@ -643,8 +643,10 @@ __secure void psci_system_suspend(u32 __always_unused function_id, /* disable GIC distributor */ writel(0, GIC400_ARB_BASE_ADDR + GIC_DIST_OFFSET); - for (i = 0; i < 4; i++) + for (i = 0; i < 4; i++) { gpc_mask[i] = readl(GPC_IPS_BASE_ADDR + GPC_IMR1_CORE0 + i * 4); + writel(~0, GPC_IPS_BASE_ADDR + GPC_IMR1_CORE0 + i * 4); + } /* * enable the RBC bypass counter here @@ -668,7 +670,7 @@ __secure void psci_system_suspend(u32 __always_unused function_id, writel(gpc_mask[i], GPC_IPS_BASE_ADDR + GPC_IMR1_CORE0 + i * 4); /* -* now delay for a short while (3usec) +* now delay for a short while (~3usec) * ARM is at 1GHz at this point * so a short loop should be enough. * this delay is required to ensure that @@ -677,7 +679,8 @@ __secure void psci_system_suspend(u32 __always_unused function_id, * or in case an interrupt arrives just * as ARM is about to assert DSM_request. */ - imx_udelay(3); + for (i = 0; i < 2000; i++) + asm volatile(""); /* save resume entry and sp in CPU0 GPR registers */ asm volatile("mov %0, sp" : "=r" (val)); -- 2.25.1
[PATCH] common: fdt_support: add support for "partitions" subnode to fdt_fixup_mtdparts()
Listing MTD partitions directly in the flash mode has been deprecated for a while for kernel Device Trees. Look for a node "partitions" in the found flash nodes and use it instead of the flash node itself for the partition list when it exists, so Device Trees following the current best practices can be fixed up. Signed-off-by: Matthias Schiffer --- common/fdt_support.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/common/fdt_support.c b/common/fdt_support.c index daa24d4c10..ea18ea3f04 100644 --- a/common/fdt_support.c +++ b/common/fdt_support.c @@ -988,7 +988,7 @@ void fdt_fixup_mtdparts(void *blob, const struct node_info *node_info, { struct mtd_device *dev; int i, idx; - int noff; + int noff, parts; bool inited = false; for (i = 0; i < node_info_size; i++) { @@ -1014,7 +1014,12 @@ void fdt_fixup_mtdparts(void *blob, const struct node_info *node_info, dev = device_find(node_info[i].type, idx++); if (dev) { - if (fdt_node_set_part_info(blob, noff, dev)) + parts = fdt_subnode_offset(blob, noff, + "partitions"); + if (parts < 0) + parts = noff; + + if (fdt_node_set_part_info(blob, parts, dev)) return; /* return on error */ } } -- 2.25.1
Re: [PATCH] fastboot: only look up real partition names when no alias exists
On Fri, 2021-12-17 at 18:20 -0500, Sean Anderson wrote: > Hi Matthias, > > On 12/16/21 5:26 AM, Matthias Schiffer wrote: > > Having U-Boot look up the passed partition name even though an > > alias > > exists is unexpected, leading to warning messages (when the alias > > name > > doesn't exist as a real partition name) or the use of the wrong > > partition. > > > > Change part_get_info_by_name_or_alias() to consider real partitions > > names only if no alias of the same name exists, allowing to use > > aliases > > to override the configuration for existing partition names. > > Much saner IMO. > > I think the correct move in the long term is to add a "quiet" > parameter to do_get_part_info (and all its helpers). This is OK as an > incremental improvement. > > > Also change one use of strcpy() to strlcpy(). > > > > Signed-off-by: Matthias Schiffer > > > > --- > > drivers/fastboot/fb_mmc.c | 29 - > > 1 file changed, 12 insertions(+), 17 deletions(-) > > > > diff --git a/drivers/fastboot/fb_mmc.c b/drivers/fastboot/fb_mmc.c > > index 2738dc836e..fb7791d9da 100644 > > --- a/drivers/fastboot/fb_mmc.c > > +++ b/drivers/fastboot/fb_mmc.c > > @@ -104,23 +104,18 @@ static int > > part_get_info_by_name_or_alias(struct blk_desc **dev_desc, > > const char *name, > > struct disk_partition *info) > > { > > - int ret; > > - > > - ret = do_get_part_info(dev_desc, name, info); > > - if (ret < 0) { > > - /* strlen("fastboot_partition_alias_") + PART_NAME_LEN > > + 1 */ > > - char env_alias_name[25 + PART_NAME_LEN + 1]; > > - char *aliased_part_name; > > - > > - /* check for alias */ > > - strcpy(env_alias_name, "fastboot_partition_alias_"); > > - strlcat(env_alias_name, name, sizeof(env_alias_name)); > > - aliased_part_name = env_get(env_alias_name); > > - if (aliased_part_name != NULL) > > - ret = do_get_part_info(dev_desc, > > aliased_part_name, > > - info); > > - } > > - return ret; > > + /* strlen("fastboot_partition_alias_") + PART_NAME_LEN + 1 */ > > + char env_alias_name[25 + PART_NAME_LEN + 1]; > > + char *aliased_part_name; > > + > > + /* check for alias */ > > + strlcpy(env_alias_name, "fastboot_partition_alias_", > > sizeof(env_alias_name)); > > + strlcat(env_alias_name, name, sizeof(env_alias_name)); > > + aliased_part_name = env_get(env_alias_name); > > + if (aliased_part_name) > > + name = aliased_part_name; > > + > > + return do_get_part_info(dev_desc, name, info); > > } > > > > /** > > > > Reviewed-by: Sean Anderson > > --Sean Can we get this committed? Kind regards, Matthias
[PATCH] fastboot: only look up real partition names when no alias exists
Having U-Boot look up the passed partition name even though an alias exists is unexpected, leading to warning messages (when the alias name doesn't exist as a real partition name) or the use of the wrong partition. Change part_get_info_by_name_or_alias() to consider real partitions names only if no alias of the same name exists, allowing to use aliases to override the configuration for existing partition names. Also change one use of strcpy() to strlcpy(). Signed-off-by: Matthias Schiffer --- drivers/fastboot/fb_mmc.c | 29 - 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/drivers/fastboot/fb_mmc.c b/drivers/fastboot/fb_mmc.c index 2738dc836e..fb7791d9da 100644 --- a/drivers/fastboot/fb_mmc.c +++ b/drivers/fastboot/fb_mmc.c @@ -104,23 +104,18 @@ static int part_get_info_by_name_or_alias(struct blk_desc **dev_desc, const char *name, struct disk_partition *info) { - int ret; - - ret = do_get_part_info(dev_desc, name, info); - if (ret < 0) { - /* strlen("fastboot_partition_alias_") + PART_NAME_LEN + 1 */ - char env_alias_name[25 + PART_NAME_LEN + 1]; - char *aliased_part_name; - - /* check for alias */ - strcpy(env_alias_name, "fastboot_partition_alias_"); - strlcat(env_alias_name, name, sizeof(env_alias_name)); - aliased_part_name = env_get(env_alias_name); - if (aliased_part_name != NULL) - ret = do_get_part_info(dev_desc, aliased_part_name, - info); - } - return ret; + /* strlen("fastboot_partition_alias_") + PART_NAME_LEN + 1 */ + char env_alias_name[25 + PART_NAME_LEN + 1]; + char *aliased_part_name; + + /* check for alias */ + strlcpy(env_alias_name, "fastboot_partition_alias_", sizeof(env_alias_name)); + strlcat(env_alias_name, name, sizeof(env_alias_name)); + aliased_part_name = env_get(env_alias_name); + if (aliased_part_name) + name = aliased_part_name; + + return do_get_part_info(dev_desc, name, info); } /** -- 2.25.1
Simultaneous support of CONFIG_MX6UL and CONFIG_MX6ULL
Hi everyone, for the submission of support for our TQMa6UL/TQMa6ULL SoM family I've been wondering if it would be desirable to allow U-Boot configs that support both i.MX6UL and i.MX6ULL. This would allow us to reduce the number of required defconfig variants for our SoMs significantly. I had a look at the differences between these configurations, and most of the code already treats both SoCs the same (lots of "#if defined(CONFIG_MX6UL) || defined(CONFIG_MX6ULL)"). The differences are sufficiently small that it seems easy to change them to use runtime detection for the SoC variant (and maybe not even leave CONFIG_MX6UL and CONFIG_MX6ULL as separate config symbols): - MX6UL selects HAS_CAAM. Runtime detection should already work (will double-check) - Fuse support: Easy to switch to runtime detection - mx6ul_pins.h vs. mx6ull_pins.h: Mostly identical. Only definitions for GPIO5 differ (and none of the differing definitions are used at all) I can propose patches for these changes if you think that it is a good idea. Kind regards, Matthias
[PATCH 2/2] board: tq: fix spelling of "TQ-Systems"
"TQ-Systems" is written with a dash. Signed-off-by: Matthias Schiffer --- arch/arm/mach-imx/mx6/Kconfig | 2 +- board/tq/tqma6/MAINTAINERS| 2 +- board/tq/tqma6/README | 6 +++--- board/tq/tqma6/tqma6.c| 2 +- board/tq/tqma6/tqma6_bb.h | 2 +- board/tq/tqma6/tqma6_mba6.c | 2 +- board/tq/tqma6/tqma6_wru4.c | 2 +- include/configs/tqma6.h | 2 +- include/configs/tqma6_mba6.h | 2 +- 9 files changed, 11 insertions(+), 11 deletions(-) diff --git a/arch/arm/mach-imx/mx6/Kconfig b/arch/arm/mach-imx/mx6/Kconfig index d701a46cd6f..62de942a32a 100644 --- a/arch/arm/mach-imx/mx6/Kconfig +++ b/arch/arm/mach-imx/mx6/Kconfig @@ -569,7 +569,7 @@ config TARGET_KP_IMX6Q_TPC imply CMD_SPL config TARGET_TQMA6 - bool "TQ Systems TQMa6 board" + bool "TQ-Systems TQMa6 board" select BOARD_EARLY_INIT_F select BOARD_LATE_INIT select MXC_SPI diff --git a/board/tq/tqma6/MAINTAINERS b/board/tq/tqma6/MAINTAINERS index dae719f00b3..c4fb6ec206d 100644 --- a/board/tq/tqma6/MAINTAINERS +++ b/board/tq/tqma6/MAINTAINERS @@ -1,4 +1,4 @@ -TQ SYSTEMS TQMA6 BOARD +TQ-SYSTEMS TQMA6 BOARD M: Markus Niebel S: Maintained F: board/tq/tqma6/ diff --git a/board/tq/tqma6/README b/board/tq/tqma6/README index c47cb21eebb..bd2466c60a2 100644 --- a/board/tq/tqma6/README +++ b/board/tq/tqma6/README @@ -1,7 +1,7 @@ -U-Boot for the TQ Systems TQMa6 modules +U-Boot for the TQ-Systems TQMa6 modules This file contains information for the port of -U-Boot to the TQ Systems TQMa6 modules. +U-Boot to the TQ-Systems TQMa6 modules. 1. Boot source -- @@ -14,7 +14,7 @@ The following boot source is supported: 2. Building -To build U-Boot for the TQ Systems TQMa6 modules: +To build U-Boot for the TQ-Systems TQMa6 modules: make tqma6___config make diff --git a/board/tq/tqma6/tqma6.c b/board/tq/tqma6/tqma6.c index de9c00174ae..1c2228c77ad 100644 --- a/board/tq/tqma6/tqma6.c +++ b/board/tq/tqma6/tqma6.c @@ -3,7 +3,7 @@ * Copyright (C) 2012 Freescale Semiconductor, Inc. * Author: Fabio Estevam * - * Copyright (C) 2013, 2014 TQ Systems (ported SabreSD to TQMa6x) + * Copyright (C) 2013, 2014 TQ-Systems (ported SabreSD to TQMa6x) * Author: Markus Niebel */ diff --git a/board/tq/tqma6/tqma6_bb.h b/board/tq/tqma6/tqma6_bb.h index b0f1f99a83c..ca81bdf5853 100644 --- a/board/tq/tqma6/tqma6_bb.h +++ b/board/tq/tqma6/tqma6_bb.h @@ -1,6 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0+ */ /* - * Copyright (C) 2013, 2014 TQ Systems + * Copyright (C) 2013, 2014 TQ-Systems * Author: Markus Niebel */ diff --git a/board/tq/tqma6/tqma6_mba6.c b/board/tq/tqma6/tqma6_mba6.c index 801619e80b6..52851dd5b55 100644 --- a/board/tq/tqma6/tqma6_mba6.c +++ b/board/tq/tqma6/tqma6_mba6.c @@ -3,7 +3,7 @@ * Copyright (C) 2012 Freescale Semiconductor, Inc. * Author: Fabio Estevam * - * Copyright (C) 2013, 2014 TQ Systems (ported SabreSD to TQMa6x) + * Copyright (C) 2013, 2014 TQ-Systems (ported SabreSD to TQMa6x) * Author: Markus Niebel */ diff --git a/board/tq/tqma6/tqma6_wru4.c b/board/tq/tqma6/tqma6_wru4.c index 3b1bc603ce8..5d239913fc5 100644 --- a/board/tq/tqma6/tqma6_wru4.c +++ b/board/tq/tqma6/tqma6_wru4.c @@ -3,7 +3,7 @@ * Copyright (C) 2012 Freescale Semiconductor, Inc. * Author: Fabio Estevam * - * Copyright (C) 2013, 2014 TQ Systems (ported SabreSD to TQMa6x) + * Copyright (C) 2013, 2014 TQ-Systems (ported SabreSD to TQMa6x) * Author: Markus Niebel * * Copyright (C) 2015 Stefan Roese diff --git a/include/configs/tqma6.h b/include/configs/tqma6.h index 1efe9d57a84..faa535bbc4e 100644 --- a/include/configs/tqma6.h +++ b/include/configs/tqma6.h @@ -2,7 +2,7 @@ /* * Copyright (C) 2013, 2014, 2017 Markus Niebel * - * Configuration settings for the TQ Systems TQMa6 module. + * Configuration settings for the TQ-Systems TQMa6 module. */ #ifndef __CONFIG_H diff --git a/include/configs/tqma6_mba6.h b/include/configs/tqma6_mba6.h index bee6d2f33bb..a19ea351c27 100644 --- a/include/configs/tqma6_mba6.h +++ b/include/configs/tqma6_mba6.h @@ -2,7 +2,7 @@ /* * Copyright (C) 2013 - 2017 Markus Niebel * - * Configuration settings for the TQ Systems TQMa6 module on + * Configuration settings for the TQ-Systems TQMa6 module on * MBa6 starter kit */ -- 2.17.1
[PATCH 1/2] board: rename "tqc" vendor to "tq"
The subdivision name "TQ Components" hasn't been in use for a long time. Rename the vendor directory to "tq", which also matches our Device Tree vendor prefix. Signed-off-by: Matthias Schiffer --- Take care of the renaming first, in preparation for a larger round of cleanup and new board support we're currently working on. arch/arm/mach-imx/mx6/Kconfig| 2 +- board/{tqc => tq}/tqma6/Kconfig | 8 board/{tqc => tq}/tqma6/MAINTAINERS | 2 +- board/{tqc => tq}/tqma6/Makefile | 0 board/{tqc => tq}/tqma6/README | 0 board/{tqc => tq}/tqma6/clocks.cfg | 0 board/{tqc => tq}/tqma6/tqma6.c | 0 board/{tqc => tq}/tqma6/tqma6_bb.h | 0 board/{tqc => tq}/tqma6/tqma6_mba6.c | 0 board/{tqc => tq}/tqma6/tqma6_wru4.c | 0 board/{tqc => tq}/tqma6/tqma6dl.cfg | 0 board/{tqc => tq}/tqma6/tqma6q.cfg | 0 board/{tqc => tq}/tqma6/tqma6s.cfg | 0 13 files changed, 6 insertions(+), 6 deletions(-) rename board/{tqc => tq}/tqma6/Kconfig (90%) rename board/{tqc => tq}/tqma6/MAINTAINERS (87%) rename board/{tqc => tq}/tqma6/Makefile (100%) rename board/{tqc => tq}/tqma6/README (100%) rename board/{tqc => tq}/tqma6/clocks.cfg (100%) rename board/{tqc => tq}/tqma6/tqma6.c (100%) rename board/{tqc => tq}/tqma6/tqma6_bb.h (100%) rename board/{tqc => tq}/tqma6/tqma6_mba6.c (100%) rename board/{tqc => tq}/tqma6/tqma6_wru4.c (100%) rename board/{tqc => tq}/tqma6/tqma6dl.cfg (100%) rename board/{tqc => tq}/tqma6/tqma6q.cfg (100%) rename board/{tqc => tq}/tqma6/tqma6s.cfg (100%) diff --git a/arch/arm/mach-imx/mx6/Kconfig b/arch/arm/mach-imx/mx6/Kconfig index b4c8511cb87..d701a46cd6f 100644 --- a/arch/arm/mach-imx/mx6/Kconfig +++ b/arch/arm/mach-imx/mx6/Kconfig @@ -688,7 +688,7 @@ source "board/somlabs/visionsom-6ull/Kconfig" source "board/technexion/pico-imx6/Kconfig" source "board/technexion/pico-imx6ul/Kconfig" source "board/tbs/tbs2910/Kconfig" -source "board/tqc/tqma6/Kconfig" +source "board/tq/tqma6/Kconfig" source "board/toradex/apalis_imx6/Kconfig" source "board/toradex/colibri_imx6/Kconfig" source "board/toradex/colibri-imx6ull/Kconfig" diff --git a/board/tqc/tqma6/Kconfig b/board/tq/tqma6/Kconfig similarity index 90% rename from board/tqc/tqma6/Kconfig rename to board/tq/tqma6/Kconfig index a2a5905290c..cb1b8749cea 100644 --- a/board/tqc/tqma6/Kconfig +++ b/board/tq/tqma6/Kconfig @@ -4,7 +4,7 @@ config SYS_BOARD default "tqma6" config SYS_VENDOR - default "tqc" + default "tq" config SYS_CONFIG_NAME default "tqma6" @@ -89,8 +89,8 @@ config SYS_TEXT_BASE default 0x4fc0 if TQMA6Q || TQMA6DL config IMX_CONFIG - default "board/tqc/tqma6/tqma6q.cfg" if TQMA6Q - default "board/tqc/tqma6/tqma6dl.cfg" if TQMA6DL - default "board/tqc/tqma6/tqma6s.cfg" if TQMA6S + default "board/tq/tqma6/tqma6q.cfg" if TQMA6Q + default "board/tq/tqma6/tqma6dl.cfg" if TQMA6DL + default "board/tq/tqma6/tqma6s.cfg" if TQMA6S endif diff --git a/board/tqc/tqma6/MAINTAINERS b/board/tq/tqma6/MAINTAINERS similarity index 87% rename from board/tqc/tqma6/MAINTAINERS rename to board/tq/tqma6/MAINTAINERS index 91cd244499f..dae719f00b3 100644 --- a/board/tqc/tqma6/MAINTAINERS +++ b/board/tq/tqma6/MAINTAINERS @@ -1,6 +1,6 @@ TQ SYSTEMS TQMA6 BOARD M: Markus Niebel S: Maintained -F: board/tqc/tqma6/ +F: board/tq/tqma6/ F: include/configs/tqma6.h F: configs/tqma6*_defconfig diff --git a/board/tqc/tqma6/Makefile b/board/tq/tqma6/Makefile similarity index 100% rename from board/tqc/tqma6/Makefile rename to board/tq/tqma6/Makefile diff --git a/board/tqc/tqma6/README b/board/tq/tqma6/README similarity index 100% rename from board/tqc/tqma6/README rename to board/tq/tqma6/README diff --git a/board/tqc/tqma6/clocks.cfg b/board/tq/tqma6/clocks.cfg similarity index 100% rename from board/tqc/tqma6/clocks.cfg rename to board/tq/tqma6/clocks.cfg diff --git a/board/tqc/tqma6/tqma6.c b/board/tq/tqma6/tqma6.c similarity index 100% rename from board/tqc/tqma6/tqma6.c rename to board/tq/tqma6/tqma6.c diff --git a/board/tqc/tqma6/tqma6_bb.h b/board/tq/tqma6/tqma6_bb.h similarity index 100% rename from board/tqc/tqma6/tqma6_bb.h rename to board/tq/tqma6/tqma6_bb.h diff --git a/board/tqc/tqma6/tqma6_mba6.c b/board/tq/tqma6/tqma6_mba6.c similarity index 100% rename from board/tqc/tqma6/tqma6_mba6.c rename to board/tq/tqma6/tqma6_mba6.c diff --git a/board/tqc/tqma6/tqma6_wru4.c b/board/tq/tqma6/tqma6_wru4.c similarity index 100% rename from board/tqc/tqma6/tqma6_wru4.c rename to board/tq/tqma6/tqma6_wru4.c diff --git a/board/tqc/tqma6/tqma6dl.cfg b/board/tq/tqma6/tqma6dl.cfg similarity index 100% rename from board
Re: [PATCH] fastboot: fix partition name truncation in environment lookup
On Fri, 2021-07-30 at 10:04 -0400, Sean Anderson wrote: > On 7/30/21 8:23 AM, Matthias Schiffer wrote: > > strlcat() need to be passed the full buffer length. The incorrect call > > caused truncation of partition names for fastboot_raw_partition_... and > > fastboot_partition_alias_... env lookup to much less than PART_NAME_LEN. > > > > Fixes: 69a752983171 ("fastboot: Fix possible buffer overrun") > > Signed-off-by: Matthias Schiffer > > --- > > drivers/fastboot/fb_mmc.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/fastboot/fb_mmc.c b/drivers/fastboot/fb_mmc.c > > index 2f3837e559..33fd6c21af 100644 > > --- a/drivers/fastboot/fb_mmc.c > > +++ b/drivers/fastboot/fb_mmc.c > > @@ -40,7 +40,7 @@ static int raw_part_get_info_by_name(struct blk_desc > > *dev_desc, > > > > /* check for raw partition descriptor */ > > strcpy(env_desc_name, "fastboot_raw_partition_"); > > - strlcat(env_desc_name, name, PART_NAME_LEN); > > + strlcat(env_desc_name, name, sizeof(env_desc_name)); > > raw_part_desc = strdup(env_get(env_desc_name)); > > if (raw_part_desc == NULL) > > return -ENODEV; > > @@ -114,7 +114,7 @@ static int part_get_info_by_name_or_alias(struct > > blk_desc **dev_desc, > > > > /* check for alias */ > > strcpy(env_alias_name, "fastboot_partition_alias_"); > > - strlcat(env_alias_name, name, PART_NAME_LEN); > > + strlcat(env_alias_name, name, sizeof(env_alias_name)); > > aliased_part_name = env_get(env_alias_name); > > if (aliased_part_name != NULL) > > ret = do_get_part_info(dev_desc, aliased_part_name, > > > > Reviewed-by: Sean Anderson Hi, what's the status here? It would be great to have this bugfix in the next release. Regards, Matthias
[PATCH] imx: mx7: spl: fix CONFIG_SPL_MAX_SIZE definition
The CONFIG_SPL_MAX_SIZE definition did not account for all areas that are used by the boot ROM according to the manual, causing boot failures due to truncated SPL images when actually hitting this limit. Signed-off-by: Matthias Schiffer --- include/configs/imx7_spl.h | 20 ++-- 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/include/configs/imx7_spl.h b/include/configs/imx7_spl.h index 01d1cd83b23..128f612392f 100644 --- a/include/configs/imx7_spl.h +++ b/include/configs/imx7_spl.h @@ -18,15 +18,23 @@ * - Set the stack at the end of the free area section, at 0x00946BB8. * - The BOOT ROM loads what they consider the firmware image *which consists of a 4K header in front of us that contains the IVT, DCD - *and some padding thus 'our' max size is really 0x00946BB8 - 0x00911000. - *64KB is more then enough for the SPL. + *and some padding. However, the manual also states that the ROM uses the + *OCRAM_EPCD and OCRAM_PXP areas for itself. While the SPL is free to use + *this range for stack and malloc, the SPL itself must fit below 0x92, + *or the image will be truncated in at least some boot modes like USB SDP. + *Thus our max size is really 0x0092 - 0x00912000. If necessary, + *CONFIG_SPL_TEXT_BASE could be moved to 0x00911000 to gain 4KB of space + *for the SPL, but 56KB should be more than enough for the SPL. */ -#define CONFIG_SPL_MAX_SIZE0x1 +#define CONFIG_SPL_MAX_SIZE0xE000 #define CONFIG_SPL_STACK 0x00946BB8 /* - * Pad SPL to 68KB (4KB header + 64KB max size). This allows to write the - * SPL/U-Boot combination generated with u-boot-with-spl.imx directly to a - * boot media (given that boot media specific offset is configured properly). + * Pad SPL to 68KB (4KB header + 56KB max size + 8KB extra padding) + * The extra padding could be removed, but this value was used historically + * based on an incorrect CONFIG_SPL_MAX_SIZE definition. + * This allows to write the SPL/U-Boot combination generated with + * u-boot-with-spl.imx directly to a boot media (given that boot media specific + * offset is configured properly). */ #define CONFIG_SPL_PAD_TO 0x11000 -- 2.17.1
Re: [PATCH 1/2] mmc: add helper to query max enhanced part size
On Thu, 2021-09-23 at 20:58 +0900, Jaehoon Chung wrote: > Hi, > > On 9/22/21 9:30 PM, Matthias Schiffer wrote: > > From: Markus Niebel > > > > This helper will be used later on in an extension of the mmc > > command. > > > > Signed-off-by: Markus Niebel > > Signed-off-by: Matthias Schiffer > > --- > > drivers/mmc/mmc.c | 38 ++ > > include/mmc.h | 1 + > > 2 files changed, 39 insertions(+) > > > > diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c > > index d3babbfeb1c..c1b1ef7eb0b 100644 > > --- a/drivers/mmc/mmc.c > > +++ b/drivers/mmc/mmc.c > > @@ -1039,6 +1039,44 @@ int mmc_switch_part(struct mmc *mmc, unsigned int > > part_num) > > } > > > > #if CONFIG_IS_ENABLED(MMC_HW_PARTITIONING) > > +int mmc_max_enhanced_size_sectors(struct mmc *mmc, u64 *size) > > +{ > > + u64 sz; > > + int err; > > + > > + ALLOC_CACHE_ALIGN_BUFFER(u8, ext_csd, MMC_MAX_BLOCK_LEN); > > + > > + if (IS_SD(mmc) || mmc->version < MMC_VERSION_4_41) { > > + pr_err("eMMC >= 4.4 required for enhanced user data area\n"); > > Error log is considering about only eMMC. It can be SD-card. This check and message were taken from mmc_hwpart_config(). I think it is okay (after all it tells you "eMMC [...] required [...]" if you try the command on an SD card), but I can extend the message if you want. I also noticed another slight difference between the check and the message: The check is for eMMC 4.41, while the message talks about eMMC 4.4. I'd like to make both match, but I don't know whether 4.4 or 4.41 is the correct requirement. > > > + return -EMEDIUMTYPE; > > + } > > + > > + if (!(mmc->part_support & PART_SUPPORT)) { > > + pr_err("Card does not support partitioning\n"); > > + return -EMEDIUMTYPE; > > + } > > + > > + if (!mmc->hc_wp_grp_size) { > > + pr_err("Card does not define HC WP group size\n"); > > + return -EMEDIUMTYPE; > > + } > > + > > + err = mmc_send_ext_csd(mmc, ext_csd); > > + if (err) > > + return err; > > + > > + sz = > > + (ext_csd[EXT_CSD_MAX_ENH_SIZE_MULT + 2] << 16) + > > + (ext_csd[EXT_CSD_MAX_ENH_SIZE_MULT + 1] << 8) + > > + ext_csd[EXT_CSD_MAX_ENH_SIZE_MULT]; > > + sz *= ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE]; > > + sz *= ext_csd[EXT_CSD_HC_WP_GRP_SIZE]; > > + sz *= SZ_1K; > > To use the num of sector, i think better that is adding Comment. > And using not "sz" as variable. It seems to describe real size, not sector. > According to spec, it's 512kByte. It can be confused. Makes sense, I'll change the variable name. > > Best Regards, > Jaehoon Chung > > > + *size = sz; > > + > > + return 0; > > +} > > + > > int mmc_hwpart_config(struct mmc *mmc, > > const struct mmc_hwpart_conf *conf, > > enum mmc_hwpart_conf_mode mode) > > diff --git a/include/mmc.h b/include/mmc.h > > index b92e2553402..3e1fc82d9b4 100644 > > --- a/include/mmc.h > > +++ b/include/mmc.h > > @@ -846,6 +846,7 @@ void print_mmc_devices(char separator); > > */ > > int get_mmc_num(void); > > int mmc_switch_part(struct mmc *mmc, unsigned int part_num); > > +int mmc_max_enhanced_size_sectors(struct mmc *mmc, u64 *size); > > int mmc_hwpart_config(struct mmc *mmc, const struct mmc_hwpart_conf *conf, > > enum mmc_hwpart_conf_mode mode); > > > > > >
[PATCH 2/2] cmd/mmc: add subcommand to query max enhanced partition size
From: Markus Niebel The new command prints the sector count and size in a human-readable format and sets an environment variable for scripted handling. The variable value is set in decimal to match what the 'mmc hwpartition' command expects. The environment variable can be used for automated partitioning scripts, for example the following would convert a whole eMMC to pSLC mode: mmc maxhwpartsectors mmc hwpartition user enh 0 ${maxhwpartsectors} wrrel on complete Signed-off-by: Markus Niebel Signed-off-by: Matthias Schiffer --- The human-readable output of the command could also be added to `mmc info`, but it would still be great to have a separate command that sets an environment variable for scripting, like this patch adds. cmd/mmc.c | 30 ++ 1 file changed, 30 insertions(+) diff --git a/cmd/mmc.c b/cmd/mmc.c index f1e30d0cf64..d0b33cc0494 100644 --- a/cmd/mmc.c +++ b/cmd/mmc.c @@ -593,6 +593,33 @@ static int do_mmc_list(struct cmd_tbl *cmdtp, int flag, } #if CONFIG_IS_ENABLED(MMC_HW_PARTITIONING) +static int do_mmc_maxhwpartsectors(struct cmd_tbl *cmdtp, int flag, int argc, char * const argv[]) +{ + struct mmc *mmc; + u64 sectors; + + mmc = init_mmc_device(curr_device, false); + if (!mmc) + return CMD_RET_FAILURE; + + if (mmc_max_enhanced_size_sectors(mmc, §ors)) + return CMD_RET_FAILURE; + + /* Ensure that the value fits in mmc_hwpart_conf::user.enh_size */ + if (sectors > UINT_MAX) { + puts("ERROR: sector count larger than UINT_MAX\n"); + return CMD_RET_FAILURE; + } + + env_set_ulong("maxhwpartsectors", sectors); + + printf("Maximum size of hardware partition: %u sectors (", + (uint)sectors); + print_size(sectors * 512, ")\n"); + + return 0; +} + static int parse_hwpart_user(struct mmc_hwpart_conf *pconf, int argc, char *const argv[]) { @@ -1021,6 +1048,7 @@ static struct cmd_tbl cmd_mmc[] = { U_BOOT_CMD_MKENT(dev, 4, 0, do_mmc_dev, "", ""), U_BOOT_CMD_MKENT(list, 1, 1, do_mmc_list, "", ""), #if CONFIG_IS_ENABLED(MMC_HW_PARTITIONING) + U_BOOT_CMD_MKENT(maxhwpartsectors, 1, 0, do_mmc_maxhwpartsectors, "", ""), U_BOOT_CMD_MKENT(hwpartition, 28, 0, do_mmc_hwpartition, "", ""), #endif #ifdef CONFIG_SUPPORT_EMMC_BOOT @@ -1084,6 +1112,8 @@ U_BOOT_CMD( "mmc list - lists available devices\n" "mmc wp - power on write protect boot partitions\n" #if CONFIG_IS_ENABLED(MMC_HW_PARTITIONING) + "mmc maxhwpartsectors - shows the maximum number of 512-byte blocks usable for hardware partitioning\n" + " Sets env var maxhwpartsectors on success.\n" "mmc hwpartition- does hardware partitioning\n" " arguments (sizes in 512-byte blocks):\n" " USER - <{on|off}>\n" -- 2.17.1
[PATCH 1/2] mmc: add helper to query max enhanced part size
From: Markus Niebel This helper will be used later on in an extension of the mmc command. Signed-off-by: Markus Niebel Signed-off-by: Matthias Schiffer --- drivers/mmc/mmc.c | 38 ++ include/mmc.h | 1 + 2 files changed, 39 insertions(+) diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c index d3babbfeb1c..c1b1ef7eb0b 100644 --- a/drivers/mmc/mmc.c +++ b/drivers/mmc/mmc.c @@ -1039,6 +1039,44 @@ int mmc_switch_part(struct mmc *mmc, unsigned int part_num) } #if CONFIG_IS_ENABLED(MMC_HW_PARTITIONING) +int mmc_max_enhanced_size_sectors(struct mmc *mmc, u64 *size) +{ + u64 sz; + int err; + + ALLOC_CACHE_ALIGN_BUFFER(u8, ext_csd, MMC_MAX_BLOCK_LEN); + + if (IS_SD(mmc) || mmc->version < MMC_VERSION_4_41) { + pr_err("eMMC >= 4.4 required for enhanced user data area\n"); + return -EMEDIUMTYPE; + } + + if (!(mmc->part_support & PART_SUPPORT)) { + pr_err("Card does not support partitioning\n"); + return -EMEDIUMTYPE; + } + + if (!mmc->hc_wp_grp_size) { + pr_err("Card does not define HC WP group size\n"); + return -EMEDIUMTYPE; + } + + err = mmc_send_ext_csd(mmc, ext_csd); + if (err) + return err; + + sz = + (ext_csd[EXT_CSD_MAX_ENH_SIZE_MULT + 2] << 16) + + (ext_csd[EXT_CSD_MAX_ENH_SIZE_MULT + 1] << 8) + + ext_csd[EXT_CSD_MAX_ENH_SIZE_MULT]; + sz *= ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE]; + sz *= ext_csd[EXT_CSD_HC_WP_GRP_SIZE]; + sz *= SZ_1K; + *size = sz; + + return 0; +} + int mmc_hwpart_config(struct mmc *mmc, const struct mmc_hwpart_conf *conf, enum mmc_hwpart_conf_mode mode) diff --git a/include/mmc.h b/include/mmc.h index b92e2553402..3e1fc82d9b4 100644 --- a/include/mmc.h +++ b/include/mmc.h @@ -846,6 +846,7 @@ void print_mmc_devices(char separator); */ int get_mmc_num(void); int mmc_switch_part(struct mmc *mmc, unsigned int part_num); +int mmc_max_enhanced_size_sectors(struct mmc *mmc, u64 *size); int mmc_hwpart_config(struct mmc *mmc, const struct mmc_hwpart_conf *conf, enum mmc_hwpart_conf_mode mode); -- 2.17.1
[PATCH 3/4] usb: ehci-ci: remove redundant PORTSC flag definitions
These definitions are unused, all boards that define portsc flags use the equivalent PORT_* definitions instead. Signed-off-by: Markus Niebel Signed-off-by: Matthias Schiffer --- include/usb/ehci-ci.h | 11 --- 1 file changed, 11 deletions(-) diff --git a/include/usb/ehci-ci.h b/include/usb/ehci-ci.h index efb2eec5ce7..bf5d26faa53 100644 --- a/include/usb/ehci-ci.h +++ b/include/usb/ehci-ci.h @@ -249,17 +249,6 @@ struct usb_ehci { * For MXC SOCs */ -/* values for portsc field */ -#define MXC_EHCI_PHY_LOW_POWER_SUSPEND (1 << 23) -#define MXC_EHCI_FORCE_FS (1 << 24) -#define MXC_EHCI_UTMI_8BIT (0 << 28) -#define MXC_EHCI_UTMI_16BIT(1 << 28) -#define MXC_EHCI_SERIAL(1 << 29) -#define MXC_EHCI_MODE_UTMI (0 << 30) -#define MXC_EHCI_MODE_PHILIPS (1 << 30) -#define MXC_EHCI_MODE_ULPI (2 << 30) -#define MXC_EHCI_MODE_SERIAL (3 << 30) - /* values for flags field */ #define MXC_EHCI_INTERFACE_DIFF_UNI(0 << 0) #define MXC_EHCI_INTERFACE_DIFF_BI (1 << 0) -- 2.17.1
[PATCH 4/4] usb: ehci-mx6: use phy_type from device tree
Allow using different PHY interfaces for multiple USB controllers. When no value is set in DT, we fall back to CONFIG_MXC_USB_PORTSC for now to stay compatible with current board configurations. This also adds support for the HSIC mode of the i.MX7. Signed-off-by: Markus Niebel Signed-off-by: Matthias Schiffer --- drivers/usb/host/ehci-mx6.c | 25 +++-- include/usb/ehci-ci.h | 1 + 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c index c3e4170513e..1bd6147c76a 100644 --- a/drivers/usb/host/ehci-mx6.c +++ b/drivers/usb/host/ehci-mx6.c @@ -23,6 +23,7 @@ #include #include #include +#include #include "ehci.h" @@ -435,6 +436,7 @@ struct ehci_mx6_priv_data { struct clk clk; struct phy phy; enum usb_init_type init_type; + enum usb_phy_interface phy_type; #if !defined(CONFIG_PHY) int portnr; void __iomem *phy_addr; @@ -443,6 +445,24 @@ struct ehci_mx6_priv_data { #endif }; +static u32 mx6_portsc(enum usb_phy_interface phy_type) +{ + switch (phy_type) { + case USBPHY_INTERFACE_MODE_UTMI: + return PORT_PTS_UTMI; + case USBPHY_INTERFACE_MODE_UTMIW: + return PORT_PTS_UTMI | PORT_PTS_PTW; + case USBPHY_INTERFACE_MODE_ULPI: + return PORT_PTS_ULPI; + case USBPHY_INTERFACE_MODE_SERIAL: + return PORT_PTS_SERIAL; + case USBPHY_INTERFACE_MODE_HSIC: + return PORT_PTS_HSIC; + default: + return CONFIG_MXC_USB_PORTSC; + } +} + static int mx6_init_after_reset(struct ehci_ctrl *dev) { struct ehci_mx6_priv_data *priv = dev->priv; @@ -479,7 +499,7 @@ static int mx6_init_after_reset(struct ehci_ctrl *dev) return 0; setbits_le32(&ehci->usbmode, CM_HOST); - writel(CONFIG_MXC_USB_PORTSC, &ehci->portsc); + writel(mx6_portsc(priv->phy_type), &ehci->portsc); setbits_le32(&ehci->portsc, USB_EN); mdelay(10); @@ -641,6 +661,7 @@ static int ehci_usb_probe(struct udevice *dev) priv->ehci = ehci; priv->init_type = type; + priv->phy_type = usb_get_phy_mode(dev_ofnode(dev)); #if CONFIG_IS_ENABLED(CLK) ret = clk_get_by_index(dev, 0, &priv->clk); @@ -690,7 +711,7 @@ static int ehci_usb_probe(struct udevice *dev) if (priv->init_type == USB_INIT_HOST) { setbits_le32(&ehci->usbmode, CM_HOST); - writel(CONFIG_MXC_USB_PORTSC, &ehci->portsc); + writel(mx6_portsc(priv->phy_type), &ehci->portsc); setbits_le32(&ehci->portsc, USB_EN); } diff --git a/include/usb/ehci-ci.h b/include/usb/ehci-ci.h index bf5d26faa53..2cdb3146e86 100644 --- a/include/usb/ehci-ci.h +++ b/include/usb/ehci-ci.h @@ -23,6 +23,7 @@ #define PORT_PTS_ULPI (2 << 30) #define PORT_PTS_SERIAL(3 << 30) #define PORT_PTS_PTW (1 << 28) +#define PORT_PTS_HSIC (1 << 25) #define PORT_PFSC (1 << 24) /* Defined on Page 39-44 of the mpc5151 ERM */ #define PORT_PTS_PHCD (1 << 23) #define PORT_PP(1 << 12) -- 2.17.1
[PATCH 2/4] include/configs: replace MXC_EHCI_MODE_SERIAL with PORT_PTS_SERIAL
The MXC_EHCI_MODE_ definitions are redundant. Replace MXC_EHCI_MODE_SERIAL with the equivalent PORT_PTS_SERIAL. Only the zmx25 platform is affected. Signed-off-by: Markus Niebel Signed-off-by: Matthias Schiffer --- include/configs/zmx25.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/configs/zmx25.h b/include/configs/zmx25.h index 88a885463d4..8b571da021a 100644 --- a/include/configs/zmx25.h +++ b/include/configs/zmx25.h @@ -57,7 +57,7 @@ #define CONFIG_USB_EHCI_MXC #define CONFIG_EHCI_HCD_INIT_AFTER_RESET #define CONFIG_MXC_USB_PORT1 -#define CONFIG_MXC_USB_PORTSC MXC_EHCI_MODE_SERIAL +#define CONFIG_MXC_USB_PORTSC PORT_PTS_SERIAL #define CONFIG_MXC_USB_FLAGS (MXC_EHCI_INTERNAL_PHY | MXC_EHCI_IPPUE_DOWN) #define CONFIG_EHCI_IS_TDI #endif /* CONFIG_CMD_USB */ -- 2.17.1
[PATCH 1/4] usb: add support for ULPI/SERIAL/HSIC PHY modes
Import usb_phy_interface enum values and DT match strings from the Linux kernel. Signed-off-by: Markus Niebel Signed-off-by: Matthias Schiffer --- drivers/usb/common/common.c | 3 +++ include/linux/usb/phy.h | 3 +++ 2 files changed, 6 insertions(+) diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c index 2a47f40bbab..43564c9fbaf 100644 --- a/drivers/usb/common/common.c +++ b/drivers/usb/common/common.c @@ -80,6 +80,9 @@ static const char *const usbphy_modes[] = { [USBPHY_INTERFACE_MODE_UNKNOWN] = "", [USBPHY_INTERFACE_MODE_UTMI]= "utmi", [USBPHY_INTERFACE_MODE_UTMIW] = "utmi_wide", + [USBPHY_INTERFACE_MODE_ULPI]= "ulpi", + [USBPHY_INTERFACE_MODE_SERIAL] = "serial", + [USBPHY_INTERFACE_MODE_HSIC]= "hsic", }; enum usb_phy_interface usb_get_phy_mode(ofnode node) diff --git a/include/linux/usb/phy.h b/include/linux/usb/phy.h index 1e1217a9583..14b2c7eb2e6 100644 --- a/include/linux/usb/phy.h +++ b/include/linux/usb/phy.h @@ -16,6 +16,9 @@ enum usb_phy_interface { USBPHY_INTERFACE_MODE_UNKNOWN, USBPHY_INTERFACE_MODE_UTMI, USBPHY_INTERFACE_MODE_UTMIW, + USBPHY_INTERFACE_MODE_ULPI, + USBPHY_INTERFACE_MODE_SERIAL, + USBPHY_INTERFACE_MODE_HSIC, }; #if CONFIG_IS_ENABLED(DM_USB) -- 2.17.1
[PATCH] fastboot: fix partition name truncation in environment lookup
strlcat() need to be passed the full buffer length. The incorrect call caused truncation of partition names for fastboot_raw_partition_... and fastboot_partition_alias_... env lookup to much less than PART_NAME_LEN. Fixes: 69a752983171 ("fastboot: Fix possible buffer overrun") Signed-off-by: Matthias Schiffer --- drivers/fastboot/fb_mmc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/fastboot/fb_mmc.c b/drivers/fastboot/fb_mmc.c index 2f3837e559..33fd6c21af 100644 --- a/drivers/fastboot/fb_mmc.c +++ b/drivers/fastboot/fb_mmc.c @@ -40,7 +40,7 @@ static int raw_part_get_info_by_name(struct blk_desc *dev_desc, /* check for raw partition descriptor */ strcpy(env_desc_name, "fastboot_raw_partition_"); - strlcat(env_desc_name, name, PART_NAME_LEN); + strlcat(env_desc_name, name, sizeof(env_desc_name)); raw_part_desc = strdup(env_get(env_desc_name)); if (raw_part_desc == NULL) return -ENODEV; @@ -114,7 +114,7 @@ static int part_get_info_by_name_or_alias(struct blk_desc **dev_desc, /* check for alias */ strcpy(env_alias_name, "fastboot_partition_alias_"); - strlcat(env_alias_name, name, PART_NAME_LEN); + strlcat(env_alias_name, name, sizeof(env_alias_name)); aliased_part_name = env_get(env_alias_name); if (aliased_part_name != NULL) ret = do_get_part_info(dev_desc, aliased_part_name, -- 2.17.1
[PATCH] net: eth-uclass: avoid running start() twice without stop()
Running the start() handler twice without a stop() inbetween completely breaks communication for some ethernet drivers like fec_mxc. eth_halt() is called before each eth_init(). Due to the switch to eth_is_active() in commit 68acb51f442f ("net: Only call halt on a driver that has been init'ed"), this is not sufficient anymore when netconsole is active: eth_init_state_only()/eth_halt_state_only() manipulate the state check that is performed by eth_is_active() without actually calling into the driver. The issue can be triggered by starting a network operation (e.g. ping or tftp) while netconsole is active. Add an additional "running" flag that reflects the actual state of the driver and use it to ensure that eth_halt() actually stops the device as it is supposed to. Fixes: 68acb51f442f ("net: Only call halt on a driver that has been init'ed") Signed-off-by: Matthias Schiffer --- net/eth-uclass.c | 14 ++ 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/net/eth-uclass.c b/net/eth-uclass.c index e14695c0f1..7c9278f3a9 100644 --- a/net/eth-uclass.c +++ b/net/eth-uclass.c @@ -26,6 +26,7 @@ DECLARE_GLOBAL_DATA_PTR; */ struct eth_device_priv { enum eth_state_t state; + bool running; }; /** @@ -293,6 +294,7 @@ int eth_init(void) current->uclass_priv; priv->state = ETH_STATE_ACTIVE; + priv->running = true; return 0; } } else { @@ -322,13 +324,16 @@ void eth_halt(void) struct eth_device_priv *priv; current = eth_get_dev(); - if (!current || !eth_is_active(current)) + if (!current) return; - eth_get_ops(current)->stop(current); priv = current->uclass_priv; - if (priv) - priv->state = ETH_STATE_PASSIVE; + if (!priv || !priv->running) + return; + + eth_get_ops(current)->stop(current); + priv->state = ETH_STATE_PASSIVE; + priv->running = false; } int eth_is_active(struct udevice *dev) @@ -537,6 +542,7 @@ static int eth_post_probe(struct udevice *dev) #endif priv->state = ETH_STATE_INIT; + priv->running = false; /* Check if the device has a valid MAC address in device tree */ if (!eth_dev_get_mac_address(dev, pdata->enetaddr) || -- 2.17.1
[PATCH] tools/imximage: fix DCD Blocks message output order
The correct order is load address, offset, length. The order was accidentally switched a while ago; make it match the HAB Blocks output and what CST expects again. Fixes: e97bdfa5da70 ("tools/imximage: share DCD information via Kconfig") Signed-off-by: Matthias Schiffer --- tools/imximage.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/imximage.c b/tools/imximage.c index d7edd3c52f..5c23fba3b1 100644 --- a/tools/imximage.c +++ b/tools/imximage.c @@ -529,7 +529,7 @@ static void print_hdr_v2(struct imx_header *imx_hdr) (uint32_t)fhdr_v2->self, 0, (uint32_t)(fhdr_v2->csf - fhdr_v2->self)); printf("DCD Blocks: 0x%08x 0x%08x 0x%08x\n", - offs, CONFIG_IMX_DCD_ADDR, be16_to_cpu(dcdlen)); + CONFIG_IMX_DCD_ADDR, offs, be16_to_cpu(dcdlen)); } } else { imx_header_v2_t *next_hdr_v2; -- 2.17.1
Re: [U-Boot] [PATCH] MIPS: fix mips_cache fallback without __builtin_mips_cache
On 03/06/2016 08:38 PM, Daniel Schwierzeck wrote: > > > Am 05.03.2016 um 04:15 schrieb Matthias Schiffer: >> The "R" constraint supplies the address of an variable in a register. Use >> "r" instead and adjust asm to supply the content of addr in a register >> instead. >> >> Fixes: 2b8bcc5a ("MIPS: avoid .set ISA for cache operations") >> Signed-off-by: Matthias Schiffer >> Cc: Paul Burton >> Cc: Daniel Schwierzeck >> --- >> >> Hi, >> I've noticed this when reading the code to understand how the cache >> instruction is used. I'm not sure if this bug had any practical >> consequences, or if nowadays all relevant compilers have >> __builtin_mips_cache anyways. >> >> Please keep me in Cc in follow-up mails, I'm not subscribed to the u-boot >> ML. >> >> Matthias > > I've disabled the builtin code and compared dissaemblies with and without > your patch. Without your patch, gcc adds an additional store instruction > before each cache instruction. > > E.g. for flush_dcache_range(): > > 18: afa20008sw v0,8(sp) > 1c: bfb50008cache 0x15,8(sp) > > vs. > > 14: bc55cache 0x15,0(v0) > > The cache operation works anyway, but with your patch better code is > generated. If I understand this correctly, the code without my patch would rather invalidate the cache for the address 8(sp), which is part of the stack, and not the memory pointed at by v0. > >> >> >> arch/mips/include/asm/cacheops.h | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/arch/mips/include/asm/cacheops.h >> b/arch/mips/include/asm/cacheops.h >> index a3b07c6..002b839 100644 >> --- a/arch/mips/include/asm/cacheops.h >> +++ b/arch/mips/include/asm/cacheops.h >> @@ -16,7 +16,7 @@ static inline void mips_cache(int op, const volatile void >> *addr) >> #ifdef __GCC_HAVE_BUILTIN_MIPS_CACHE >> __builtin_mips_cache(op, addr); >> #else >> -__asm__ __volatile__("cache %0, %1" : : "i"(op), "R"(addr)); >> +__asm__ __volatile__("cache %0, 0(%1)" : : "i"(op), "r"(addr)); >> #endif >> } >> >> > > applied to u-boot-mips/next, thanks! > signature.asc Description: OpenPGP digital signature ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH] MIPS: fix mips_cache fallback without __builtin_mips_cache
The "R" constraint supplies the address of an variable in a register. Use "r" instead and adjust asm to supply the content of addr in a register instead. Fixes: 2b8bcc5a ("MIPS: avoid .set ISA for cache operations") Signed-off-by: Matthias Schiffer Cc: Paul Burton Cc: Daniel Schwierzeck --- Hi, I've noticed this when reading the code to understand how the cache instruction is used. I'm not sure if this bug had any practical consequences, or if nowadays all relevant compilers have __builtin_mips_cache anyways. Please keep me in Cc in follow-up mails, I'm not subscribed to the u-boot ML. Matthias arch/mips/include/asm/cacheops.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/mips/include/asm/cacheops.h b/arch/mips/include/asm/cacheops.h index a3b07c6..002b839 100644 --- a/arch/mips/include/asm/cacheops.h +++ b/arch/mips/include/asm/cacheops.h @@ -16,7 +16,7 @@ static inline void mips_cache(int op, const volatile void *addr) #ifdef __GCC_HAVE_BUILTIN_MIPS_CACHE __builtin_mips_cache(op, addr); #else - __asm__ __volatile__("cache %0, %1" : : "i"(op), "R"(addr)); + __asm__ __volatile__("cache %0, 0(%1)" : : "i"(op), "r"(addr)); #endif } -- 2.7.2 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot