From 93cac85d78d9f105ff2e4651c558607acef93b64 Mon Sep 17 00:00:00 2001 From: Frank Wunderlich Date: Tue, 16 Mar 2021 19:05:33 +0100 Subject: ahci: mediatek: fix undefined reference of dev_err building with MTK_AHCI enabled results in implicit declaration and undefined reference of dev_err followed by a segfault of gcc drivers/ata/mtk_ahci.c: In function 'mtk_ahci_parse_property': drivers/ata/mtk_ahci.c:65:4: warning: implicit declaration of function 'dev_err' drivers/ata/mtk_ahci.c:65: undefined reference to `dev_err' in function `mtk_ahci_probe': drivers/ata/mtk_ahci.c:92: undefined reference to `dev_err' Segmentation fault fix this by adding the dm/device_compat.h to includes Signed-off-by: Frank Wunderlich --- drivers/ata/mtk_ahci.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/ata/mtk_ahci.c b/drivers/ata/mtk_ahci.c index 554175bc00..2c5227df30 100644 --- a/drivers/ata/mtk_ahci.c +++ b/drivers/ata/mtk_ahci.c @@ -21,6 +21,7 @@ #include #include #include +#include #define SYS_CFG 0x14 #define SYS_CFG_SATA_MSK GENMASK(31, 30) -- cgit v1.2.1 From 73e553a280b1b0464c44dc912c8e47e5ba379208 Mon Sep 17 00:00:00 2001 From: Michael Walle Date: Wed, 17 Mar 2021 15:01:35 +0100 Subject: board: sl28: disable HS400 mode Since commit 8ee802f899ef ("mmc: fsl_esdhc: make sure delay chain locked for HS400") HS400 mode is unreliable on LS1028A SoCs. Some workarounds are missing for this SoC. Disable HS400 mode for now. Signed-off-by: Michael Walle --- configs/kontron_sl28_defconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configs/kontron_sl28_defconfig b/configs/kontron_sl28_defconfig index 1c781e091c..0c6c1911d9 100644 --- a/configs/kontron_sl28_defconfig +++ b/configs/kontron_sl28_defconfig @@ -70,7 +70,7 @@ CONFIG_I2C_SET_DEFAULT_BUS_NUM=y CONFIG_I2C_DEFAULT_BUS_NUMBER=0 CONFIG_I2C_MUX=y CONFIG_DM_MMC=y -CONFIG_MMC_HS400_SUPPORT=y +CONFIG_MMC_HS200_SUPPORT=y CONFIG_FSL_ESDHC=y CONFIG_FSL_ESDHC_SUPPORT_ADMA2=y CONFIG_DM_SPI_FLASH=y -- cgit v1.2.1 From ccc58b4d324b768d1d629890a218ab32cde5b378 Mon Sep 17 00:00:00 2001 From: Dario Binacchi Date: Tue, 16 Mar 2021 21:51:44 +0100 Subject: bus: ti-sysc: change in a normal driver The module defines a duplicate uclass driver for UCLASS_SIMPLE_BUS, but it is not allowed. This breaks of-platdata and makes the result non-deterministic. The driver does not need to be an uclass driver, so lets remove it. I had turned it into an uclass driver because I thought wrongly it had to call the dm_scan_fdt_dev routine to work properly, but some tests on the board have shown otherwise. Signed-off-by: Dario Binacchi Reviewed-by: Simon Glass --- drivers/bus/ti-sysc.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c index 4e3d610300..778c0654f6 100644 --- a/drivers/bus/ti-sysc.c +++ b/drivers/bus/ti-sysc.c @@ -148,12 +148,6 @@ clocks_err: return err; } -UCLASS_DRIVER(ti_sysc) = { - .id = UCLASS_SIMPLE_BUS, - .name = "ti_sysc", - .post_bind = dm_scan_fdt_dev -}; - U_BOOT_DRIVER(ti_sysc) = { .name = "ti_sysc", .id = UCLASS_SIMPLE_BUS, -- cgit v1.2.1 From d32211ee95a2fd4a322599346e572fc91c1ba9b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Beh=C3=BAn?= Date: Sat, 6 Mar 2021 23:43:22 +0100 Subject: api: fix a potential serious bug caused by undef CONFIG_SYS_64BIT_LBA MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The api_public.h header file undefined macro CONFIG_SYS_64BIT_LBA. But api/api_storage.c includes this header before including part.h, causing the type of lbaint_t and subsequently the type signature of blk_dread() and blk_dwrite() functions to change from the rest of U-Boot (if CONFIG_SYS_64BIT_LBA is defined for the board). This is of course wrong, because the call to blk_dread() / blk_dwrite() will receive mangled arguments. Fix this by removing the undef of macro CONFIG_SYS_64BIT_LBA and instead make the immediate code do what it would do as if the macro was not defined. Add a FIXME to whoever is maintaining this code. CI managed to trigger this bug when compiling for lsxhl_defconfig, which has CONFIG_API selected. The compiler complained about blk_dwrite() and blk_dread() not matching original declarations: include/blk.h:280:15: warning: type of ‘blk_dwrite’ does not match original declaration [-Wlto-type-mismatch] 280 | unsigned long blk_dwrite(struct blk_desc *block_dev, lbaint_t st | ^ drivers/block/blk-uclass.c:456:15: note: type mismatch in parameter 2 456 | unsigned long blk_dwrite(struct blk_desc *block_dev, lbaint_t st | ^ Signed-off-by: Marek Behún Reviewed-by: Simon Glass --- include/api_public.h | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/include/api_public.h b/include/api_public.h index def103ce22..5a4465ea89 100644 --- a/include/api_public.h +++ b/include/api_public.h @@ -70,12 +70,25 @@ struct sys_info { int mr_no; /* number of memory regions */ }; -#undef CONFIG_SYS_64BIT_LBA -#ifdef CONFIG_SYS_64BIT_LBA -typedef u_int64_t lbasize_t; -#else +/* + * FIXME: Previously this code was: + * + * #undef CONFIG_SYS_64BIT_LBA + * #ifdef CONFIG_SYS_64BIT_LBA + * typedef u_int64_t lbasize_t; + * #else + * typedef unsigned long lbasize_t; + * #endif + * + * But we cannot just undefine CONFIG_SYS_64BIT_LBA, because then in + * api/api_storage.c the type signature of lbaint_t will be different if + * CONFIG_SYS_64BIT_LBA is enabled for the board, which can result in various + * bugs. + * So simply define lbasize_t as an unsigned long, since this was what was done + * anyway for at least 13 years, but don't undefine CONFIG_SYS_64BIT_LBA. + */ typedef unsigned long lbasize_t; -#endif + typedef unsigned long lbastart_t; #define DEV_TYP_NONE 0x0000 -- cgit v1.2.1 From d0c04926cd054cf7360ec15913ac17a465f32603 Mon Sep 17 00:00:00 2001 From: Andre Przywara Date: Mon, 8 Feb 2021 13:31:54 +0000 Subject: nvme: Always invalidate whole cqes[] array At the moment nvme_read_completion_status() tries to invalidate a single member of the cqes[] array, which is shady as just a single entry is not cache line aligned. The structure is dictated by hardware, and with 16 bytes is smaller than any cache line we usually deal with. Also multiple entries need to be consecutive in memory, so we can't pad them to cover a whole cache line. As a consequence we can only always invalidate all of them - U-Boot just uses two of them anyway. This is fine, as they are only ever read by the CPU (apart from the initial zeroing), so they can't become dirty. Make this obvious by always invalidating the whole array, regardless of the entry number we are about to read. Also blow up the allocation size to cover whole cache lines, to avoid other heap allocations to sneak in. Signed-off-by: Andre Przywara Reviewed-by: Bin Meng Reviewed-by: Michael Trimarchi Tested-by: Neil Armstrong --- drivers/nvme/nvme.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/drivers/nvme/nvme.c b/drivers/nvme/nvme.c index 5d6331ad34..c9efeff4bc 100644 --- a/drivers/nvme/nvme.c +++ b/drivers/nvme/nvme.c @@ -22,6 +22,8 @@ #define NVME_AQ_DEPTH 2 #define NVME_SQ_SIZE(depth) (depth * sizeof(struct nvme_command)) #define NVME_CQ_SIZE(depth) (depth * sizeof(struct nvme_completion)) +#define NVME_CQ_ALLOCATION ALIGN(NVME_CQ_SIZE(NVME_Q_DEPTH), \ + ARCH_DMA_MINALIGN) #define ADMIN_TIMEOUT 60 #define IO_TIMEOUT 30 #define MAX_PRP_POOL 512 @@ -144,8 +146,14 @@ static __le16 nvme_get_cmd_id(void) static u16 nvme_read_completion_status(struct nvme_queue *nvmeq, u16 index) { - u64 start = (ulong)&nvmeq->cqes[index]; - u64 stop = start + sizeof(struct nvme_completion); + /* + * Single CQ entries are always smaller than a cache line, so we + * can't invalidate them individually. However CQ entries are + * read only by the CPU, so it's safe to always invalidate all of them, + * as the cache line should never become dirty. + */ + ulong start = (ulong)&nvmeq->cqes[0]; + ulong stop = start + NVME_CQ_ALLOCATION; invalidate_dcache_range(start, stop); @@ -241,7 +249,7 @@ static struct nvme_queue *nvme_alloc_queue(struct nvme_dev *dev, return NULL; memset(nvmeq, 0, sizeof(*nvmeq)); - nvmeq->cqes = (void *)memalign(4096, NVME_CQ_SIZE(depth)); + nvmeq->cqes = (void *)memalign(4096, NVME_CQ_ALLOCATION); if (!nvmeq->cqes) goto free_nvmeq; memset((void *)nvmeq->cqes, 0, NVME_CQ_SIZE(depth)); @@ -339,7 +347,7 @@ static void nvme_init_queue(struct nvme_queue *nvmeq, u16 qid) nvmeq->q_db = &dev->dbs[qid * 2 * dev->db_stride]; memset((void *)nvmeq->cqes, 0, NVME_CQ_SIZE(nvmeq->q_depth)); flush_dcache_range((ulong)nvmeq->cqes, - (ulong)nvmeq->cqes + NVME_CQ_SIZE(nvmeq->q_depth)); + (ulong)nvmeq->cqes + NVME_CQ_ALLOCATION); dev->online_queues++; } -- cgit v1.2.1 From 4c498796891a26a7283130f367a346096a6ccce7 Mon Sep 17 00:00:00 2001 From: Andre Przywara Date: Tue, 2 Mar 2021 15:43:43 +0000 Subject: nvme: Elaborate on cache maintenance operation in get/set_features At the moment the nvme_get_features() and nvme_set_features() functions carry a (somewhat misleading) comment about missing cache maintenance. As it turns out, nvme_get_features() has no caller at all in the tree, and nvme_set_features' only user doesn't use a DMA buffer. Mention that in the comment, and leave some breadcrumbs for the future, should those functions attract more users. Signed-off-by: Andre Przywara Reviewed-by: Michael Trimarchi Reviewed-by: Bin Meng --- drivers/nvme/nvme.c | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/drivers/nvme/nvme.c b/drivers/nvme/nvme.c index c9efeff4bc..c61dab20c5 100644 --- a/drivers/nvme/nvme.c +++ b/drivers/nvme/nvme.c @@ -489,6 +489,7 @@ int nvme_get_features(struct nvme_dev *dev, unsigned fid, unsigned nsid, dma_addr_t dma_addr, u32 *result) { struct nvme_command c; + int ret; memset(&c, 0, sizeof(c)); c.features.opcode = nvme_admin_get_features; @@ -496,12 +497,20 @@ int nvme_get_features(struct nvme_dev *dev, unsigned fid, unsigned nsid, c.features.prp1 = cpu_to_le64(dma_addr); c.features.fid = cpu_to_le32(fid); + ret = nvme_submit_admin_cmd(dev, &c, result); + /* - * TODO: add cache invalidate operation when the size of - * the DMA buffer is known + * TODO: Add some cache invalidation when a DMA buffer is involved + * in the request, here and before the command gets submitted. The + * buffer size varies by feature, also some features use a different + * field in the command packet to hold the buffer address. + * Section 5.21.1 (Set Features command) in the NVMe specification + * details the buffer requirements for each feature. + * + * At the moment there is no user of this function. */ - return nvme_submit_admin_cmd(dev, &c, result); + return ret; } int nvme_set_features(struct nvme_dev *dev, unsigned fid, unsigned dword11, @@ -516,8 +525,14 @@ int nvme_set_features(struct nvme_dev *dev, unsigned fid, unsigned dword11, c.features.dword11 = cpu_to_le32(dword11); /* - * TODO: add cache flush operation when the size of - * the DMA buffer is known + * TODO: Add a cache clean (aka flush) operation when a DMA buffer is + * involved in the request. The buffer size varies by feature, also + * some features use a different field in the command packet to hold + * the buffer address. Section 5.21.1 (Set Features command) in the + * NVMe specification details the buffer requirements for each + * feature. + * At the moment the only user of this function is not using + * any DMA buffer at all. */ return nvme_submit_admin_cmd(dev, &c, result); -- cgit v1.2.1