From 4bd8405257da717cd556f99e5fb68693d12c9766 Mon Sep 17 00:00:00 2001 From: Joshua Yeong Date: Wed, 13 Sep 2023 11:17:45 +0800 Subject: i3c: master: cdns: Fix reading status register IBIR_DEPTH and CMDR_DEPTH should read from status0 instead of status1. Cc: stable@vger.kernel.org Fixes: 603f2bee2c54 ("i3c: master: Add driver for Cadence IP") Signed-off-by: Joshua Yeong Reviewed-by: Miquel Raynal Link: https://lore.kernel.org/r/20230913031743.11439-2-joshua.yeong@starfivetech.com Signed-off-by: Alexandre Belloni --- drivers/i3c/master/i3c-master-cdns.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'drivers/i3c') diff --git a/drivers/i3c/master/i3c-master-cdns.c b/drivers/i3c/master/i3c-master-cdns.c index 49551db71bc9..8f1fda3c7ac5 100644 --- a/drivers/i3c/master/i3c-master-cdns.c +++ b/drivers/i3c/master/i3c-master-cdns.c @@ -191,7 +191,7 @@ #define SLV_STATUS1_HJ_DIS BIT(18) #define SLV_STATUS1_MR_DIS BIT(17) #define SLV_STATUS1_PROT_ERR BIT(16) -#define SLV_STATUS1_DA(x) (((s) & GENMASK(15, 9)) >> 9) +#define SLV_STATUS1_DA(s) (((s) & GENMASK(15, 9)) >> 9) #define SLV_STATUS1_HAS_DA BIT(8) #define SLV_STATUS1_DDR_RX_FULL BIT(7) #define SLV_STATUS1_DDR_TX_FULL BIT(6) @@ -1623,13 +1623,13 @@ static int cdns_i3c_master_probe(struct platform_device *pdev) /* Device ID0 is reserved to describe this master. */ master->maxdevs = CONF_STATUS0_DEVS_NUM(val); master->free_rr_slots = GENMASK(master->maxdevs, 1); + master->caps.ibirfifodepth = CONF_STATUS0_IBIR_DEPTH(val); + master->caps.cmdrfifodepth = CONF_STATUS0_CMDR_DEPTH(val); val = readl(master->regs + CONF_STATUS1); master->caps.cmdfifodepth = CONF_STATUS1_CMD_DEPTH(val); master->caps.rxfifodepth = CONF_STATUS1_RX_DEPTH(val); master->caps.txfifodepth = CONF_STATUS1_TX_DEPTH(val); - master->caps.ibirfifodepth = CONF_STATUS0_IBIR_DEPTH(val); - master->caps.cmdrfifodepth = CONF_STATUS0_CMDR_DEPTH(val); spin_lock_init(&master->ibi.lock); master->ibi.num_slots = CONF_STATUS1_IBI_HW_RES(val); -- cgit v1.2.3 From fa7726a4d9b91e0a8fb6fbbd819cd20f116f6c21 Mon Sep 17 00:00:00 2001 From: Justin Stitt Date: Thu, 21 Sep 2023 03:51:04 +0000 Subject: i3c: replace deprecated strncpy `strncpy` is deprecated for use on NUL-terminated destination strings [1]. We should prefer more robust and less ambiguous string interfaces. We expect adap->name to be NUL-terminated based on i2c_adapter name use: | dev_dbg(&adap->dev, "adapter [%s] registered\n", adap->name); NUL-padding does not seem to be required as `master` is zero-allocated and `i3c_master_to_i2c_adapter` simply returns a field from within `master`: | master = devm_kzalloc(dev, sizeof(*master), GFP_KERNEL); ... | struct i2c_adapter *adap = i3c_master_to_i2c_adapter(master); ... | static struct i2c_adapter * | i3c_master_to_i2c_adapter(struct i3c_master_controller *master) | { | return &master->i2c; | } This means that `adap->name` should already be filled with NUL-bytes. Considering the above, a suitable replacement is `strscpy` [2] due to the fact that it guarantees NUL-termination on the destination buffer without unnecessarily NUL-padding. Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1] Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2] Link: https://github.com/KSPP/linux/issues/90 Cc: linux-hardening@vger.kernel.org Signed-off-by: Justin Stitt Reviewed-by: Kees Cook Link: https://lore.kernel.org/r/20230921-strncpy-drivers-i3c-master-c-v1-1-9fdb8d8169e1@google.com Signed-off-by: Alexandre Belloni --- drivers/i3c/master.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/i3c') diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c index 87283e4a4607..8573ca507708 100644 --- a/drivers/i3c/master.c +++ b/drivers/i3c/master.c @@ -2309,7 +2309,7 @@ static int i3c_master_i2c_adapter_init(struct i3c_master_controller *master) adap->dev.parent = master->dev.parent; adap->owner = master->dev.parent->driver->owner; adap->algo = &i3c_master_i2c_algo; - strncpy(adap->name, dev_name(master->dev.parent), sizeof(adap->name)); + strscpy(adap->name, dev_name(master->dev.parent), sizeof(adap->name)); /* FIXME: Should we allow i3c masters to override these values? */ adap->timeout = 1000; -- cgit v1.2.3 From 014c9a0e6f9ff573099051e1e2ff6efc3470d02d Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Fri, 22 Sep 2023 10:50:11 -0700 Subject: i3c: dw: Annotate struct dw_i3c_xfer with __counted_by Prepare for the coming implementation by GCC and Clang of the __counted_by attribute. Flexible array members annotated with __counted_by can have their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family functions). As found with Coccinelle[1], add __counted_by for struct dw_i3c_xfer. [1] https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci Cc: Alexandre Belloni Cc: Jeremy Kerr Cc: Joel Stanley Cc: linux-i3c@lists.infradead.org Signed-off-by: Kees Cook Reviewed-by: Gustavo A. R. Silva Reviewed-by: Jeremy Kerr Link: https://lore.kernel.org/r/20230922175011.work.800-kees@kernel.org Signed-off-by: Alexandre Belloni --- drivers/i3c/master/dw-i3c-master.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/i3c') diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c index 9332ae5f6419..ef5751e91cc9 100644 --- a/drivers/i3c/master/dw-i3c-master.c +++ b/drivers/i3c/master/dw-i3c-master.c @@ -233,7 +233,7 @@ struct dw_i3c_xfer { struct completion comp; int ret; unsigned int ncmds; - struct dw_i3c_cmd cmds[]; + struct dw_i3c_cmd cmds[] __counted_by(ncmds); }; struct dw_i3c_i2c_dev_data { -- cgit v1.2.3 From 49f33846efc079eb94b044bf897e90909fd3aa11 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Fri, 22 Sep 2023 10:50:15 -0700 Subject: i3c: master: cdns: Annotate struct cdns_i3c_xfer with __counted_by MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prepare for the coming implementation by GCC and Clang of the __counted_by attribute. Flexible array members annotated with __counted_by can have their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family functions). As found with Coccinelle[1], add __counted_by for struct cdns_i3c_xfer. [1] https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci Cc: "Przemysław Gaj" Cc: Alexandre Belloni Cc: linux-i3c@lists.infradead.org Signed-off-by: Kees Cook Reviewed-by: Gustavo A. R. Silva Link: https://lore.kernel.org/r/20230922175014.work.637-kees@kernel.org Signed-off-by: Alexandre Belloni --- drivers/i3c/master/i3c-master-cdns.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/i3c') diff --git a/drivers/i3c/master/i3c-master-cdns.c b/drivers/i3c/master/i3c-master-cdns.c index 8f1fda3c7ac5..bcbe8f914149 100644 --- a/drivers/i3c/master/i3c-master-cdns.c +++ b/drivers/i3c/master/i3c-master-cdns.c @@ -387,7 +387,7 @@ struct cdns_i3c_xfer { struct completion comp; int ret; unsigned int ncmds; - struct cdns_i3c_cmd cmds[]; + struct cdns_i3c_cmd cmds[] __counted_by(ncmds); }; struct cdns_i3c_data { -- cgit v1.2.3 From 751d377f0f7a09d6122de0e2232133524568c52b Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Fri, 22 Sep 2023 10:50:19 -0700 Subject: i3c/master/mipi-i3c-hci: Annotate struct hci_rings_data with __counted_by Prepare for the coming implementation by GCC and Clang of the __counted_by attribute. Flexible array members annotated with __counted_by can have their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family functions). As found with Coccinelle[1], add __counted_by for struct hci_rings_data. [1] https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci Cc: Alexandre Belloni Cc: Nicolas Pitre Cc: Len Baker Cc: Boris Brezillon Cc: linux-i3c@lists.infradead.org Signed-off-by: Kees Cook Reviewed-by: Gustavo A. R. Silva Link: https://lore.kernel.org/r/20230922175019.work.129-kees@kernel.org Signed-off-by: Alexandre Belloni --- drivers/i3c/master/mipi-i3c-hci/dma.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/i3c') diff --git a/drivers/i3c/master/mipi-i3c-hci/dma.c b/drivers/i3c/master/mipi-i3c-hci/dma.c index 2990ac9eaade..a1ecdfc35641 100644 --- a/drivers/i3c/master/mipi-i3c-hci/dma.c +++ b/drivers/i3c/master/mipi-i3c-hci/dma.c @@ -139,7 +139,7 @@ struct hci_rh_data { struct hci_rings_data { unsigned int total; - struct hci_rh_data headers[]; + struct hci_rh_data headers[] __counted_by(total); }; struct hci_dma_dev_ibi_data { -- cgit v1.2.3 From a8b163e184dede158c94f6392a406ac40a08fb1b Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Fri, 22 Sep 2023 10:50:23 -0700 Subject: i3c: svc: Annotate struct svc_i3c_xfer with __counted_by Prepare for the coming implementation by GCC and Clang of the __counted_by attribute. Flexible array members annotated with __counted_by can have their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family functions). As found with Coccinelle[1], add __counted_by for struct svc_i3c_xfer. [1] https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci Cc: Miquel Raynal Cc: Conor Culhane Cc: Alexandre Belloni Cc: linux-i3c@lists.infradead.org Signed-off-by: Kees Cook Reviewed-by: Gustavo A. R. Silva Reviewed-by: Miquel Raynal Link: https://lore.kernel.org/r/20230922175023.work.239-kees@kernel.org Signed-off-by: Alexandre Belloni --- drivers/i3c/master/svc-i3c-master.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/i3c') diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c index 8f8295acdadb..32eca2d6caf0 100644 --- a/drivers/i3c/master/svc-i3c-master.c +++ b/drivers/i3c/master/svc-i3c-master.c @@ -143,7 +143,7 @@ struct svc_i3c_xfer { int ret; unsigned int type; unsigned int ncmds; - struct svc_i3c_cmd cmds[]; + struct svc_i3c_cmd cmds[] __counted_by(ncmds); }; struct svc_i3c_regs_save { -- cgit v1.2.3 From 0c35691551387e060e6ae7a6652b4101270c73cf Mon Sep 17 00:00:00 2001 From: Jarkko Nikula Date: Thu, 21 Sep 2023 08:56:53 +0300 Subject: i3c: master: Inherit DMA masks and parameters from parent device Copy the DMA masks and parameters for an I3C master device from parent device so that the master device has them set for the DMA buffer and mapping API. Signed-off-by: Jarkko Nikula Link: https://lore.kernel.org/r/20230921055704.1087277-2-jarkko.nikula@linux.intel.com Signed-off-by: Alexandre Belloni --- drivers/i3c/master.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'drivers/i3c') diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c index 8573ca507708..839eb8b4bbde 100644 --- a/drivers/i3c/master.c +++ b/drivers/i3c/master.c @@ -2629,6 +2629,10 @@ int i3c_master_register(struct i3c_master_controller *master, device_initialize(&master->dev); dev_set_name(&master->dev, "i3c-%d", i3cbus->id); + master->dev.dma_mask = parent->dma_mask; + master->dev.coherent_dma_mask = parent->coherent_dma_mask; + master->dev.dma_parms = parent->dma_parms; + ret = of_populate_i3c_bus(master); if (ret) goto err_put_dev; -- cgit v1.2.3 From f656f6bd22d7cd08b55c6495fcfaa391c2eb933f Mon Sep 17 00:00:00 2001 From: Jarkko Nikula Date: Thu, 21 Sep 2023 08:56:54 +0300 Subject: i3c: mipi-i3c-hci: Add MODULE_ALIAS Add MODULE_ALIAS() in order to be able to autoload this driver when the device is added as a platform device from another glue code driver. Signed-off-by: Jarkko Nikula Link: https://lore.kernel.org/r/20230921055704.1087277-3-jarkko.nikula@linux.intel.com Signed-off-by: Alexandre Belloni --- drivers/i3c/master/mipi-i3c-hci/core.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/i3c') diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c b/drivers/i3c/master/mipi-i3c-hci/core.c index 837af83c85f4..8f43ad81fbfd 100644 --- a/drivers/i3c/master/mipi-i3c-hci/core.c +++ b/drivers/i3c/master/mipi-i3c-hci/core.c @@ -787,6 +787,7 @@ static struct platform_driver i3c_hci_driver = { }, }; module_platform_driver(i3c_hci_driver); +MODULE_ALIAS("platform:mipi-i3c-hci"); MODULE_AUTHOR("Nicolas Pitre "); MODULE_DESCRIPTION("MIPI I3C HCI driver"); -- cgit v1.2.3 From 0676bfebf5766f0a60549f74ba597115028fa39c Mon Sep 17 00:00:00 2001 From: Jarkko Nikula Date: Thu, 21 Sep 2023 08:56:55 +0300 Subject: i3c: mipi-i3c-hci: Fix DAT/DCT entry sizes MIPI I3C HCI specification v1.1 describes the ENTRY_SIZE field for the Device Address Table (DAT) and the Device Characteristics Table (DCT) section offset registers (DAT_SECTION_OFFSET and DCT_SECTION_OFFSET). That field is not documented in earlier version. ENTRY_SIZE value 0 is meant to be backward compatible. For the DAT entry size it is interpreted as 2 DWORDs (8-bytes) and for the DCT entry size as 4 DWORDs (16-bytes). Values 1-15 are reserved for future use. New version I believe fixes also the TABLE_SIZE field description. Before it was defined in DWORDs which I believe is incorrect since the DAT/DCT table entry structures, and sizes, are described having 8-bytes/16-bytes entries. This is more clear in the specification v1.1 which states the TABLE_SIZE fields are interpreted as number of entries in the DAT/DCT tables. I believe this same holds also in earlier version, at least it makes more sense. Fix code accordingly and let the DAT_entry_size and the DCT_entry_size variables carry the size as bytes. Which is how it is already interpreted in the dat_v1.c: hci_dat_v1_init(). Signed-off-by: Jarkko Nikula Link: https://lore.kernel.org/r/20230921055704.1087277-4-jarkko.nikula@linux.intel.com Signed-off-by: Alexandre Belloni --- drivers/i3c/master/mipi-i3c-hci/core.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'drivers/i3c') diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c b/drivers/i3c/master/mipi-i3c-hci/core.c index 8f43ad81fbfd..76a3e6bb3665 100644 --- a/drivers/i3c/master/mipi-i3c-hci/core.c +++ b/drivers/i3c/master/mipi-i3c-hci/core.c @@ -610,17 +610,17 @@ static int i3c_hci_init(struct i3c_hci *hci) offset = FIELD_GET(DAT_TABLE_OFFSET, regval); hci->DAT_regs = offset ? hci->base_regs + offset : NULL; hci->DAT_entries = FIELD_GET(DAT_TABLE_SIZE, regval); - hci->DAT_entry_size = FIELD_GET(DAT_ENTRY_SIZE, regval); + hci->DAT_entry_size = FIELD_GET(DAT_ENTRY_SIZE, regval) ? 0 : 8; dev_info(&hci->master.dev, "DAT: %u %u-bytes entries at offset %#x\n", - hci->DAT_entries, hci->DAT_entry_size * 4, offset); + hci->DAT_entries, hci->DAT_entry_size, offset); regval = reg_read(DCT_SECTION); offset = FIELD_GET(DCT_TABLE_OFFSET, regval); hci->DCT_regs = offset ? hci->base_regs + offset : NULL; hci->DCT_entries = FIELD_GET(DCT_TABLE_SIZE, regval); - hci->DCT_entry_size = FIELD_GET(DCT_ENTRY_SIZE, regval); + hci->DCT_entry_size = FIELD_GET(DCT_ENTRY_SIZE, regval) ? 0 : 16; dev_info(&hci->master.dev, "DCT: %u %u-bytes entries at offset %#x\n", - hci->DCT_entries, hci->DCT_entry_size * 4, offset); + hci->DCT_entries, hci->DCT_entry_size, offset); regval = reg_read(RING_HEADERS_SECTION); offset = FIELD_GET(RING_HEADERS_OFFSET, regval); -- cgit v1.2.3 From 45a832f989e520095429589d5b01b0c65da9b574 Mon Sep 17 00:00:00 2001 From: Jarkko Nikula Date: Thu, 21 Sep 2023 08:56:56 +0300 Subject: i3c: mipi-i3c-hci: Fix out of bounds access in hci_dma_irq_handler Do not loop over ring headers in hci_dma_irq_handler() that are not allocated and enabled in hci_dma_init(). Otherwise out of bounds access will occur from rings->headers[i] access when i >= number of allocated ring headers. Signed-off-by: Jarkko Nikula Link: https://lore.kernel.org/r/20230921055704.1087277-5-jarkko.nikula@linux.intel.com Signed-off-by: Alexandre Belloni --- drivers/i3c/master/mipi-i3c-hci/dma.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/i3c') diff --git a/drivers/i3c/master/mipi-i3c-hci/dma.c b/drivers/i3c/master/mipi-i3c-hci/dma.c index a1ecdfc35641..9897145895c6 100644 --- a/drivers/i3c/master/mipi-i3c-hci/dma.c +++ b/drivers/i3c/master/mipi-i3c-hci/dma.c @@ -734,7 +734,7 @@ static bool hci_dma_irq_handler(struct i3c_hci *hci, unsigned int mask) unsigned int i; bool handled = false; - for (i = 0; mask && i < 8; i++) { + for (i = 0; mask && i < rings->total; i++) { struct hci_rh_data *rh; u32 status; -- cgit v1.2.3 From 361acacaf7c706223968c8186f0d3b6e214e7403 Mon Sep 17 00:00:00 2001 From: Jarkko Nikula Date: Thu, 21 Sep 2023 08:56:57 +0300 Subject: i3c: mipi-i3c-hci: Remove BUG() when Ring Abort request times out Ring Abort request will timeout in case there is an error in the Host Controller interrupt delivery or Ring Header configuration. Using BUG() makes hard to debug those cases. Make it less severe and turn BUG() to WARN_ON(). Signed-off-by: Jarkko Nikula Link: https://lore.kernel.org/r/20230921055704.1087277-6-jarkko.nikula@linux.intel.com Signed-off-by: Alexandre Belloni --- drivers/i3c/master/mipi-i3c-hci/dma.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'drivers/i3c') diff --git a/drivers/i3c/master/mipi-i3c-hci/dma.c b/drivers/i3c/master/mipi-i3c-hci/dma.c index 9897145895c6..e676b47966d2 100644 --- a/drivers/i3c/master/mipi-i3c-hci/dma.c +++ b/drivers/i3c/master/mipi-i3c-hci/dma.c @@ -450,10 +450,9 @@ static bool hci_dma_dequeue_xfer(struct i3c_hci *hci, /* * We're deep in it if ever this condition is ever met. * Hardware might still be writing to memory, etc. - * Better suspend the world than risking silent corruption. */ dev_crit(&hci->master.dev, "unable to abort the ring\n"); - BUG(); + WARN_ON(1); } for (i = 0; i < n; i++) { -- cgit v1.2.3 From e141db842766b1d9af32030a842ceb5eaf389bbb Mon Sep 17 00:00:00 2001 From: Jarkko Nikula Date: Thu, 21 Sep 2023 08:56:58 +0300 Subject: i3c: mipi-i3c-hci: Set ring start request together with enable Set ring start request together with ring enable in hci_dma_init(). This causes the ring abort request in hci_dma_dequeue_xfer() will raise the INTR_RING_OP (RING_OP_STAT in MIPI I3C HCI specification) interrupt in the RH_INTR_STATUS register. Signed-off-by: Jarkko Nikula Link: https://lore.kernel.org/r/20230921055704.1087277-7-jarkko.nikula@linux.intel.com Signed-off-by: Alexandre Belloni --- drivers/i3c/master/mipi-i3c-hci/dma.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'drivers/i3c') diff --git a/drivers/i3c/master/mipi-i3c-hci/dma.c b/drivers/i3c/master/mipi-i3c-hci/dma.c index e676b47966d2..d2dbbcdad1e9 100644 --- a/drivers/i3c/master/mipi-i3c-hci/dma.c +++ b/drivers/i3c/master/mipi-i3c-hci/dma.c @@ -325,7 +325,8 @@ static int hci_dma_init(struct i3c_hci *hci) rh_reg_write(INTR_SIGNAL_ENABLE, regval); ring_ready: - rh_reg_write(RING_CONTROL, RING_CTRL_ENABLE); + rh_reg_write(RING_CONTROL, RING_CTRL_ENABLE | + RING_CTRL_RUN_STOP); } regval = FIELD_PREP(MAX_HEADER_COUNT, rings->total); -- cgit v1.2.3 From 4e40642cdb621c669507d7ef098c93ff98e8747c Mon Sep 17 00:00:00 2001 From: Jarkko Nikula Date: Thu, 21 Sep 2023 08:56:59 +0300 Subject: i3c: mipi-i3c-hci: Fix race between bus cleanup and interrupt If there is a transfer error during i3c_master_bus_init() and code goes doing the bus cleanup in i3c_hci_bus_cleanup() there is possibility that i3c_hci_irq_handler() is running in parallel with hci->io->cleanup() which can be racy. Prevent this by waiting there is no pending interrupt on other CPU before doing the IO cleanup. This was observed with ring headers where first transfer failed and sometimes transfer error or ring transfer abort interrupt was coming simultaneously with the bus cleanup path. Signed-off-by: Jarkko Nikula Link: https://lore.kernel.org/r/20230921055704.1087277-8-jarkko.nikula@linux.intel.com Signed-off-by: Alexandre Belloni --- drivers/i3c/master/mipi-i3c-hci/core.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'drivers/i3c') diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c b/drivers/i3c/master/mipi-i3c-hci/core.c index 76a3e6bb3665..d7fe8e62820a 100644 --- a/drivers/i3c/master/mipi-i3c-hci/core.c +++ b/drivers/i3c/master/mipi-i3c-hci/core.c @@ -161,10 +161,12 @@ static int i3c_hci_bus_init(struct i3c_master_controller *m) static void i3c_hci_bus_cleanup(struct i3c_master_controller *m) { struct i3c_hci *hci = to_i3c_hci(m); + struct platform_device *pdev = to_platform_device(m->dev.parent); DBG(""); reg_clear(HC_CONTROL, HC_CONTROL_BUS_ENABLE); + synchronize_irq(platform_get_irq(pdev, 0)); hci->io->cleanup(hci); if (hci->cmd == &mipi_i3c_hci_cmd_v1) mipi_i3c_hci_dat_v1.cleanup(hci); -- cgit v1.2.3 From 7ccd40edc1f5c43e3f1c7d209336d3981943ab63 Mon Sep 17 00:00:00 2001 From: Jarkko Nikula Date: Thu, 21 Sep 2023 08:57:00 +0300 Subject: i3c: mipi-i3c-hci: Set number of SW enabled Ring Bundles earlier Number of software enabled Ring Bundles must be set before using them. Otherwise Ring will not start and may be power-gated by the Host Controller. Signed-off-by: Jarkko Nikula Link: https://lore.kernel.org/r/20230921055704.1087277-9-jarkko.nikula@linux.intel.com Signed-off-by: Alexandre Belloni --- drivers/i3c/master/mipi-i3c-hci/dma.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'drivers/i3c') diff --git a/drivers/i3c/master/mipi-i3c-hci/dma.c b/drivers/i3c/master/mipi-i3c-hci/dma.c index d2dbbcdad1e9..cb41d0026abd 100644 --- a/drivers/i3c/master/mipi-i3c-hci/dma.c +++ b/drivers/i3c/master/mipi-i3c-hci/dma.c @@ -229,6 +229,9 @@ static int hci_dma_init(struct i3c_hci *hci) hci->io_data = rings; rings->total = nr_rings; + regval = FIELD_PREP(MAX_HEADER_COUNT, rings->total); + rhs_reg_write(CONTROL, regval); + for (i = 0; i < rings->total; i++) { u32 offset = rhs_reg_read(RHn_OFFSET(i)); @@ -329,8 +332,6 @@ ring_ready: RING_CTRL_RUN_STOP); } - regval = FIELD_PREP(MAX_HEADER_COUNT, rings->total); - rhs_reg_write(CONTROL, regval); return 0; err_out: -- cgit v1.2.3 From b8806e0c939f168237593af0056c309bf31022b0 Mon Sep 17 00:00:00 2001 From: Jarkko Nikula Date: Thu, 21 Sep 2023 08:57:01 +0300 Subject: i3c: mipi-i3c-hci: Do not unmap region not mapped for transfer Fix following warning (with CONFIG_DMA_API_DEBUG) which happens with a transfer without a data buffer. DMA-API: i3c mipi-i3c-hci.0: device driver tries to free DMA memory it has not allocated [device address=0x0000000000000000] [size=0 bytes] For those transfers the hci_dma_queue_xfer() doesn't create a mapping and the DMA address pointer xfer->data_dma is not set. Signed-off-by: Jarkko Nikula Link: https://lore.kernel.org/r/20230921055704.1087277-10-jarkko.nikula@linux.intel.com Signed-off-by: Alexandre Belloni --- drivers/i3c/master/mipi-i3c-hci/dma.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'drivers/i3c') diff --git a/drivers/i3c/master/mipi-i3c-hci/dma.c b/drivers/i3c/master/mipi-i3c-hci/dma.c index cb41d0026abd..c8720e6334c9 100644 --- a/drivers/i3c/master/mipi-i3c-hci/dma.c +++ b/drivers/i3c/master/mipi-i3c-hci/dma.c @@ -347,6 +347,8 @@ static void hci_dma_unmap_xfer(struct i3c_hci *hci, for (i = 0; i < n; i++) { xfer = xfer_list + i; + if (!xfer->data) + continue; dma_unmap_single(&hci->master.dev, xfer->data_dma, xfer->data_len, xfer->rnw ? DMA_FROM_DEVICE : DMA_TO_DEVICE); -- cgit v1.2.3 From 4c36f656b7d1fc00643730c5845c19b3e15be856 Mon Sep 17 00:00:00 2001 From: Jarkko Nikula Date: Thu, 21 Sep 2023 08:57:02 +0300 Subject: i3c: mipi-i3c-hci: Fix missing xfer->completion in hci_cmd_v1_daa() Due to missing completion object for the ENTDAA command transfer in the hci_cmd_v1_daa() the wait_for_completion_timeout() will obviously timeout even the transfer itself may succeed. Fix this by setting the xfer->completion to the already initialized completion object. Signed-off-by: Jarkko Nikula Link: https://lore.kernel.org/r/20230921055704.1087277-11-jarkko.nikula@linux.intel.com Signed-off-by: Alexandre Belloni --- drivers/i3c/master/mipi-i3c-hci/cmd_v1.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/i3c') diff --git a/drivers/i3c/master/mipi-i3c-hci/cmd_v1.c b/drivers/i3c/master/mipi-i3c-hci/cmd_v1.c index 6a781f89b0e4..2b2323aa6714 100644 --- a/drivers/i3c/master/mipi-i3c-hci/cmd_v1.c +++ b/drivers/i3c/master/mipi-i3c-hci/cmd_v1.c @@ -332,6 +332,7 @@ static int hci_cmd_v1_daa(struct i3c_hci *hci) CMD_A0_DEV_COUNT(1) | CMD_A0_ROC | CMD_A0_TOC; xfer->cmd_desc[1] = 0; + xfer->completion = &done; hci->io->queue_xfer(hci, xfer, 1); if (!wait_for_completion_timeout(&done, HZ) && hci->io->dequeue_xfer(hci, xfer, 1)) { -- cgit v1.2.3 From 3521fa63c1ee7414e6ba0fdf98b82b07939147d9 Mon Sep 17 00:00:00 2001 From: Jarkko Nikula Date: Thu, 21 Sep 2023 08:57:03 +0300 Subject: i3c: mipi-i3c-hci: Resume controller explicitly On an HW I'm using in enabling work the RESUME bit is not set in the HC_CONTROLLER register when Host Controller goes to halt state. Value 1 should mean controller is suspended when reading and writing 1 resumes it. Because of this erratic behaviour plain HC_CONTROL read and write back won't resume the controller. Therefore do it by setting the RESUME bit explicitly. Signed-off-by: Jarkko Nikula Link: https://lore.kernel.org/r/20230921055704.1087277-12-jarkko.nikula@linux.intel.com Signed-off-by: Alexandre Belloni --- drivers/i3c/master/mipi-i3c-hci/core.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'drivers/i3c') diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c b/drivers/i3c/master/mipi-i3c-hci/core.c index d7fe8e62820a..1ae56a5699c6 100644 --- a/drivers/i3c/master/mipi-i3c-hci/core.c +++ b/drivers/i3c/master/mipi-i3c-hci/core.c @@ -174,8 +174,7 @@ static void i3c_hci_bus_cleanup(struct i3c_master_controller *m) void mipi_i3c_hci_resume(struct i3c_hci *hci) { - /* the HC_CONTROL_RESUME bit is R/W1C so just read and write back */ - reg_write(HC_CONTROL, reg_read(HC_CONTROL)); + reg_set(HC_CONTROL, HC_CONTROL_RESUME); } /* located here rather than pio.c because needed bits are in core reg space */ -- cgit v1.2.3 From fc9176e794d74baccb1e4ef41894ac141f524992 Mon Sep 17 00:00:00 2001 From: Jarkko Nikula Date: Thu, 21 Sep 2023 08:57:04 +0300 Subject: i3c: mipi-i3c-hci: Resume controller after aborted transfer Host Controller goes to halt state after aborted transfer and needs to be resumed by SW. Add this resuming to DMA mode code too. Signed-off-by: Jarkko Nikula Link: https://lore.kernel.org/r/20230921055704.1087277-13-jarkko.nikula@linux.intel.com Signed-off-by: Alexandre Belloni --- drivers/i3c/master/mipi-i3c-hci/dma.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'drivers/i3c') diff --git a/drivers/i3c/master/mipi-i3c-hci/dma.c b/drivers/i3c/master/mipi-i3c-hci/dma.c index c8720e6334c9..c805a8497319 100644 --- a/drivers/i3c/master/mipi-i3c-hci/dma.c +++ b/drivers/i3c/master/mipi-i3c-hci/dma.c @@ -759,9 +759,11 @@ static bool hci_dma_irq_handler(struct i3c_hci *hci, unsigned int mask) if (status & INTR_RING_OP) complete(&rh->op_done); - if (status & INTR_TRANSFER_ABORT) + if (status & INTR_TRANSFER_ABORT) { dev_notice_ratelimited(&hci->master.dev, "ring %d: Transfer Aborted\n", i); + mipi_i3c_hci_resume(hci); + } if (status & INTR_WARN_INS_STOP_MODE) dev_warn_ratelimited(&hci->master.dev, "ring %d: Inserted Stop on Mode Change\n", i); -- cgit v1.2.3 From cab63f64887616e3c4e31cfd8103320be6ebc8d3 Mon Sep 17 00:00:00 2001 From: Dinghao Liu Date: Thu, 21 Sep 2023 16:24:10 +0800 Subject: i3c: Fix potential refcount leak in i3c_master_register_new_i3c_devs put_device() needs to be called on failure of device_register() to give up the reference initialized in it to avoid refcount leak. Fixes: 3a379bbcea0a ("i3c: Add core I3C infrastructure") Signed-off-by: Dinghao Liu Link: https://lore.kernel.org/r/20230921082410.25548-1-dinghao.liu@zju.edu.cn Signed-off-by: Alexandre Belloni --- drivers/i3c/master.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'drivers/i3c') diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c index 839eb8b4bbde..e66a1f84cefb 100644 --- a/drivers/i3c/master.c +++ b/drivers/i3c/master.c @@ -1525,9 +1525,11 @@ i3c_master_register_new_i3c_devs(struct i3c_master_controller *master) desc->dev->dev.of_node = desc->boardinfo->of_node; ret = device_register(&desc->dev->dev); - if (ret) + if (ret) { dev_err(&master->dev, "Failed to add I3C device (err = %d)\n", ret); + put_device(&desc->dev->dev); + } } } -- cgit v1.2.3 From 57ec42b9a1b7e4db4a1c2aa4fcc4eefe6d31bcb8 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Tue, 3 Oct 2023 15:53:39 +0800 Subject: i3c: Fix typo "Provisional ID" to "Provisioned ID" The MIPI I3C spec refers to a Provisioned ID, since it is (sometimes) provisioned at device manufacturing. Signed-off-by: Matt Johnston Acked-by: Rob Herring Reviewed-by: Miquel Raynal Link: https://lore.kernel.org/r/20231003075339.197099-1-matt@codeconstruct.com.au Signed-off-by: Alexandre Belloni --- Documentation/ABI/testing/sysfs-bus-i3c | 4 ++-- Documentation/devicetree/bindings/i3c/i3c.yaml | 4 ++-- Documentation/driver-api/i3c/protocol.rst | 4 ++-- drivers/i3c/master/svc-i3c-master.c | 2 +- include/linux/i3c/device.h | 2 +- include/linux/i3c/master.h | 2 +- 6 files changed, 9 insertions(+), 9 deletions(-) (limited to 'drivers/i3c') diff --git a/Documentation/ABI/testing/sysfs-bus-i3c b/Documentation/ABI/testing/sysfs-bus-i3c index 1f4a2662335b..e5248fd67a56 100644 --- a/Documentation/ABI/testing/sysfs-bus-i3c +++ b/Documentation/ABI/testing/sysfs-bus-i3c @@ -67,7 +67,7 @@ What: /sys/bus/i3c/devices/i3c-/pid KernelVersion: 5.0 Contact: linux-i3c@vger.kernel.org Description: - PID stands for Provisional ID and is used to uniquely identify + PID stands for Provisioned ID and is used to uniquely identify a device on a bus. This PID contains information about the vendor, the part and an instance ID so that several devices of the same type can be connected on the same bus. @@ -123,7 +123,7 @@ What: /sys/bus/i3c/devices/i3c-/-/pid KernelVersion: 5.0 Contact: linux-i3c@vger.kernel.org Description: - PID stands for Provisional ID and is used to uniquely identify + PID stands for Provisioned ID and is used to uniquely identify a device on a bus. This PID contains information about the vendor, the part and an instance ID so that several devices of the same type can be connected on the same bus. diff --git a/Documentation/devicetree/bindings/i3c/i3c.yaml b/Documentation/devicetree/bindings/i3c/i3c.yaml index ab69f4115de4..f8ac7a3e3123 100644 --- a/Documentation/devicetree/bindings/i3c/i3c.yaml +++ b/Documentation/devicetree/bindings/i3c/i3c.yaml @@ -119,12 +119,12 @@ patternProperties: minimum: 0 maximum: 0x7f - description: | - First half of the Provisional ID (following the PID + First half of the Provisioned ID (following the PID definition provided by the I3C specification). Contains the manufacturer ID left-shifted by 1. - description: | - Second half of the Provisional ID (following the PID + Second half of the Provisioned ID (following the PID definition provided by the I3C specification). Contains the ORing of the part ID left-shifted by 16, diff --git a/Documentation/driver-api/i3c/protocol.rst b/Documentation/driver-api/i3c/protocol.rst index 02653defa011..23a0b93c62b1 100644 --- a/Documentation/driver-api/i3c/protocol.rst +++ b/Documentation/driver-api/i3c/protocol.rst @@ -71,8 +71,8 @@ During DAA, each I3C device reports 3 important things: related capabilities * DCR: Device Characteristic Register. This 8-bit register describes the functionalities provided by the device -* Provisional ID: A 48-bit unique identifier. On a given bus there should be no - Provisional ID collision, otherwise the discovery mechanism may fail. +* Provisioned ID: A 48-bit unique identifier. On a given bus there should be no + Provisioned ID collision, otherwise the discovery mechanism may fail. I3C slave events ================ diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c index 32eca2d6caf0..e23d7900c5a1 100644 --- a/drivers/i3c/master/svc-i3c-master.c +++ b/drivers/i3c/master/svc-i3c-master.c @@ -765,7 +765,7 @@ static int svc_i3c_master_do_daa_locked(struct svc_i3c_master *master, u8 data[6]; /* - * We only care about the 48-bit provisional ID yet to + * We only care about the 48-bit provisioned ID yet to * be sure a device does not nack an address twice. * Otherwise, we would just need to flush the RX FIFO. */ diff --git a/include/linux/i3c/device.h b/include/linux/i3c/device.h index 90fa83464f00..84ed77c04940 100644 --- a/include/linux/i3c/device.h +++ b/include/linux/i3c/device.h @@ -96,7 +96,7 @@ enum i3c_dcr { /** * struct i3c_device_info - I3C device information - * @pid: Provisional ID + * @pid: Provisioned ID * @bcr: Bus Characteristic Register * @dcr: Device Characteristic Register * @static_addr: static/I2C address diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h index 0b52da4f2346..4fd6a777150f 100644 --- a/include/linux/i3c/master.h +++ b/include/linux/i3c/master.h @@ -166,7 +166,7 @@ struct i3c_device_ibi_info { * assigned a dynamic address by the master. Will be used during * bus initialization to assign it a specific dynamic address * before starting DAA (Dynamic Address Assignment) - * @pid: I3C Provisional ID exposed by the device. This is a unique identifier + * @pid: I3C Provisioned ID exposed by the device. This is a unique identifier * that may be used to attach boardinfo to i3c_dev_desc when the device * does not have a static address * @of_node: optional DT node in case the device has been described in the DT -- cgit v1.2.3 From 6bf3fc268183816856c96b8794cd66146bc27b35 Mon Sep 17 00:00:00 2001 From: Frank Li Date: Mon, 23 Oct 2023 12:16:53 -0400 Subject: i3c: master: svc: fix race condition in ibi work thread The ibi work thread operates asynchronously with other transfers, such as svc_i3c_master_priv_xfers(). Introduce mutex protection to ensure the completion of the entire i3c/i2c transaction. Fixes: dd3c52846d59 ("i3c: master: svc: Add Silvaco I3C master driver") Cc: Reviewed-by: Miquel Raynal Signed-off-by: Frank Li Link: https://lore.kernel.org/r/20231023161658.3890811-2-Frank.Li@nxp.com Signed-off-by: Alexandre Belloni --- drivers/i3c/master/svc-i3c-master.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) (limited to 'drivers/i3c') diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c index e23d7900c5a1..66d1eaa59bd3 100644 --- a/drivers/i3c/master/svc-i3c-master.c +++ b/drivers/i3c/master/svc-i3c-master.c @@ -175,6 +175,7 @@ struct svc_i3c_regs_save { * @ibi.slots: Available IBI slots * @ibi.tbq_slot: To be queued IBI slot * @ibi.lock: IBI lock + * @lock: Transfer lock, protect between IBI work thread and callbacks from master */ struct svc_i3c_master { struct i3c_master_controller base; @@ -203,6 +204,7 @@ struct svc_i3c_master { /* Prevent races within IBI handlers */ spinlock_t lock; } ibi; + struct mutex lock; }; /** @@ -384,6 +386,7 @@ static void svc_i3c_master_ibi_work(struct work_struct *work) u32 status, val; int ret; + mutex_lock(&master->lock); /* Acknowledge the incoming interrupt with the AUTOIBI mechanism */ writel(SVC_I3C_MCTRL_REQUEST_AUTO_IBI | SVC_I3C_MCTRL_IBIRESP_AUTO, @@ -460,6 +463,7 @@ static void svc_i3c_master_ibi_work(struct work_struct *work) reenable_ibis: svc_i3c_master_enable_interrupts(master, SVC_I3C_MINT_SLVSTART); + mutex_unlock(&master->lock); } static irqreturn_t svc_i3c_master_irq_handler(int irq, void *dev_id) @@ -1204,9 +1208,11 @@ static int svc_i3c_master_send_bdcast_ccc_cmd(struct svc_i3c_master *master, cmd->read_len = 0; cmd->continued = false; + mutex_lock(&master->lock); svc_i3c_master_enqueue_xfer(master, xfer); if (!wait_for_completion_timeout(&xfer->comp, msecs_to_jiffies(1000))) svc_i3c_master_dequeue_xfer(master, xfer); + mutex_unlock(&master->lock); ret = xfer->ret; kfree(buf); @@ -1250,9 +1256,11 @@ static int svc_i3c_master_send_direct_ccc_cmd(struct svc_i3c_master *master, cmd->read_len = read_len; cmd->continued = false; + mutex_lock(&master->lock); svc_i3c_master_enqueue_xfer(master, xfer); if (!wait_for_completion_timeout(&xfer->comp, msecs_to_jiffies(1000))) svc_i3c_master_dequeue_xfer(master, xfer); + mutex_unlock(&master->lock); if (cmd->read_len != xfer_len) ccc->dests[0].payload.len = cmd->read_len; @@ -1309,9 +1317,11 @@ static int svc_i3c_master_priv_xfers(struct i3c_dev_desc *dev, cmd->continued = (i + 1) < nxfers; } + mutex_lock(&master->lock); svc_i3c_master_enqueue_xfer(master, xfer); if (!wait_for_completion_timeout(&xfer->comp, msecs_to_jiffies(1000))) svc_i3c_master_dequeue_xfer(master, xfer); + mutex_unlock(&master->lock); ret = xfer->ret; svc_i3c_master_free_xfer(xfer); @@ -1347,9 +1357,11 @@ static int svc_i3c_master_i2c_xfers(struct i2c_dev_desc *dev, cmd->continued = (i + 1 < nxfers); } + mutex_lock(&master->lock); svc_i3c_master_enqueue_xfer(master, xfer); if (!wait_for_completion_timeout(&xfer->comp, msecs_to_jiffies(1000))) svc_i3c_master_dequeue_xfer(master, xfer); + mutex_unlock(&master->lock); ret = xfer->ret; svc_i3c_master_free_xfer(xfer); @@ -1540,6 +1552,8 @@ static int svc_i3c_master_probe(struct platform_device *pdev) INIT_WORK(&master->hj_work, svc_i3c_master_hj_work); INIT_WORK(&master->ibi_work, svc_i3c_master_ibi_work); + mutex_init(&master->lock); + ret = devm_request_irq(dev, master->irq, svc_i3c_master_irq_handler, IRQF_NO_SUSPEND, "svc-i3c-irq", master); if (ret) -- cgit v1.2.3 From 5e5e3c92e748a6d859190e123b9193cf4911fcca Mon Sep 17 00:00:00 2001 From: Frank Li Date: Mon, 23 Oct 2023 12:16:54 -0400 Subject: i3c: master: svc: fix wrong data return when IBI happen during start frame MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ┌─────┐ ┏──┐ ┏──┐ ┏──┐ ┏──┐ ┏──┐ ┏──┐ ┏──┐ ┏──┐ ┌───── SCL: ┘ └─────┛ └──┛ └──┛ └──┛ └──┛ └──┛ └──┛ └──┛ └──┘ ───┐ ┌─────┐ ┌─────┐ ┌───────────┐ SDA: └───────────────────────┘ └─────┘ └─────┘ └───── xxx╱ ╲╱ ╲╱ ╲╱ ╲╱ ╲ : xxx╲IBI ╱╲ Addr(0x0a) ╱╲ RW ╱╲NACK╱╲ S ╱ If an In-Band Interrupt (IBI) occurs and IBI work thread is not immediately scheduled, when svc_i3c_master_priv_xfers() initiates the I3C transfer and attempts to send address 0x7e, the target interprets it as an IBI handler and returns the target address 0x0a. However, svc_i3c_master_priv_xfers() does not handle this case and proceeds with other transfers, resulting in incorrect data being returned. Add IBIWON check in svc_i3c_master_xfer(). In case this situation occurs, return a failure to the driver. Fixes: dd3c52846d59 ("i3c: master: svc: Add Silvaco I3C master driver") Cc: Reviewed-by: Miquel Raynal Signed-off-by: Frank Li Link: https://lore.kernel.org/r/20231023161658.3890811-3-Frank.Li@nxp.com Signed-off-by: Alexandre Belloni --- drivers/i3c/master/svc-i3c-master.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) (limited to 'drivers/i3c') diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c index 66d1eaa59bd3..2b2b9c948167 100644 --- a/drivers/i3c/master/svc-i3c-master.c +++ b/drivers/i3c/master/svc-i3c-master.c @@ -1011,6 +1011,9 @@ static int svc_i3c_master_xfer(struct svc_i3c_master *master, u32 reg; int ret; + /* clean SVC_I3C_MINT_IBIWON w1c bits */ + writel(SVC_I3C_MINT_IBIWON, master->regs + SVC_I3C_MSTATUS); + writel(SVC_I3C_MCTRL_REQUEST_START_ADDR | xfer_type | SVC_I3C_MCTRL_IBIRESP_NACK | @@ -1029,6 +1032,23 @@ static int svc_i3c_master_xfer(struct svc_i3c_master *master, goto emit_stop; } + /* + * According to I3C spec ver 1.1.1, 5.1.2.2.3 Consequence of Controller Starting a Frame + * with I3C Target Address. + * + * The I3C Controller normally should start a Frame, the Address may be arbitrated, and so + * the Controller shall monitor to see whether an In-Band Interrupt request, a Controller + * Role Request (i.e., Secondary Controller requests to become the Active Controller), or + * a Hot-Join Request has been made. + * + * If missed IBIWON check, the wrong data will be return. When IBIWON happen, return failure + * and yield the above events handler. + */ + if (SVC_I3C_MSTATUS_IBIWON(reg)) { + ret = -ENXIO; + goto emit_stop; + } + if (rnw) ret = svc_i3c_master_read(master, in, xfer_len); else -- cgit v1.2.3 From c85e209b799f12d18a90ae6353b997b1bb1274a5 Mon Sep 17 00:00:00 2001 From: Frank Li Date: Mon, 23 Oct 2023 12:16:55 -0400 Subject: i3c: master: svc: fix ibi may not return mandatory data byte MSTATUS[RXPEND] is only updated after the data transfer cycle started. This creates an issue when the I3C clock is slow, and the CPU is running fast enough that MSTATUS[RXPEND] may not be updated when the code reaches checking point. As a result, mandatory data can be missed. Add a wait for MSTATUS[COMPLETE] to ensure that all mandatory data is already in FIFO. It also works without mandatory data. Fixes: dd3c52846d59 ("i3c: master: svc: Add Silvaco I3C master driver") Cc: Reviewed-by: Miquel Raynal Signed-off-by: Frank Li Link: https://lore.kernel.org/r/20231023161658.3890811-4-Frank.Li@nxp.com Signed-off-by: Alexandre Belloni --- drivers/i3c/master/svc-i3c-master.c | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'drivers/i3c') diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c index 2b2b9c948167..7e83a02573b9 100644 --- a/drivers/i3c/master/svc-i3c-master.c +++ b/drivers/i3c/master/svc-i3c-master.c @@ -333,6 +333,7 @@ static int svc_i3c_master_handle_ibi(struct svc_i3c_master *master, struct i3c_ibi_slot *slot; unsigned int count; u32 mdatactrl; + int ret, val; u8 *buf; slot = i3c_generic_ibi_get_free_slot(data->ibi_pool); @@ -342,6 +343,13 @@ static int svc_i3c_master_handle_ibi(struct svc_i3c_master *master, slot->len = 0; buf = slot->data; + ret = readl_relaxed_poll_timeout(master->regs + SVC_I3C_MSTATUS, val, + SVC_I3C_MSTATUS_COMPLETE(val), 0, 1000); + if (ret) { + dev_err(master->dev, "Timeout when polling for COMPLETE\n"); + return ret; + } + while (SVC_I3C_MSTATUS_RXPEND(readl(master->regs + SVC_I3C_MSTATUS)) && slot->len < SVC_I3C_FIFO_SIZE) { mdatactrl = readl(master->regs + SVC_I3C_MDATACTRL); -- cgit v1.2.3 From 225d5ef048c4ed01a475c95d94833bd7dd61072d Mon Sep 17 00:00:00 2001 From: Frank Li Date: Mon, 23 Oct 2023 12:16:56 -0400 Subject: i3c: master: svc: fix check wrong status register in irq handler svc_i3c_master_irq_handler() wrongly checks register SVC_I3C_MINTMASKED. It should be SVC_I3C_MSTATUS. Fixes: dd3c52846d59 ("i3c: master: svc: Add Silvaco I3C master driver") Cc: Reviewed-by: Miquel Raynal Signed-off-by: Frank Li Link: https://lore.kernel.org/r/20231023161658.3890811-5-Frank.Li@nxp.com Signed-off-by: Alexandre Belloni --- drivers/i3c/master/svc-i3c-master.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/i3c') diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c index 7e83a02573b9..0673ecd1a39d 100644 --- a/drivers/i3c/master/svc-i3c-master.c +++ b/drivers/i3c/master/svc-i3c-master.c @@ -477,7 +477,7 @@ reenable_ibis: static irqreturn_t svc_i3c_master_irq_handler(int irq, void *dev_id) { struct svc_i3c_master *master = (struct svc_i3c_master *)dev_id; - u32 active = readl(master->regs + SVC_I3C_MINTMASKED); + u32 active = readl(master->regs + SVC_I3C_MSTATUS); if (!SVC_I3C_MSTATUS_SLVSTART(active)) return IRQ_NONE; -- cgit v1.2.3 From dfd7cd6aafdb1f5ba93828e97e56b38304b23a05 Mon Sep 17 00:00:00 2001 From: Frank Li Date: Mon, 23 Oct 2023 12:16:57 -0400 Subject: i3c: master: svc: fix SDA keep low when polling IBIWON timeout happen Upon IBIWON timeout, the SDA line will always be kept low if we don't emit a stop. Calling svc_i3c_master_emit_stop() there will let the bus return to idle state. Fixes: dd3c52846d59 ("i3c: master: svc: Add Silvaco I3C master driver") Cc: Reviewed-by: Miquel Raynal Signed-off-by: Frank Li Link: https://lore.kernel.org/r/20231023161658.3890811-6-Frank.Li@nxp.com Signed-off-by: Alexandre Belloni --- drivers/i3c/master/svc-i3c-master.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/i3c') diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c index 0673ecd1a39d..18843e07d175 100644 --- a/drivers/i3c/master/svc-i3c-master.c +++ b/drivers/i3c/master/svc-i3c-master.c @@ -405,6 +405,7 @@ static void svc_i3c_master_ibi_work(struct work_struct *work) SVC_I3C_MSTATUS_IBIWON(val), 0, 1000); if (ret) { dev_err(master->dev, "Timeout when polling for IBIWON\n"); + svc_i3c_master_emit_stop(master); goto reenable_ibis; } -- cgit v1.2.3 From 9aaeef113c55248ecf3ab941c2e4460aaa8b8b9a Mon Sep 17 00:00:00 2001 From: Frank Li Date: Mon, 23 Oct 2023 12:16:58 -0400 Subject: i3c: master: svc: fix random hot join failure since timeout error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit master side report: silvaco-i3c-master 44330000.i3c-master: Error condition: MSTATUS 0x020090c7, MERRWARN 0x00100000 BIT 20: TIMEOUT error The module has stalled too long in a frame. This happens when: - The TX FIFO or RX FIFO is not handled and the bus is stuck in the middle of a message, - No STOP was issued and between messages, - IBI manual is used and no decision was made. The maximum stall period is 100 μs. This can be considered as being just a warning as the system IRQ latency can easily be greater than 100us. Fixes: dd3c52846d59 ("i3c: master: svc: Add Silvaco I3C master driver") Cc: Signed-off-by: Frank Li Reviewed-by: Miquel Raynal Link: https://lore.kernel.org/r/20231023161658.3890811-7-Frank.Li@nxp.com Signed-off-by: Alexandre Belloni --- drivers/i3c/master/svc-i3c-master.c | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'drivers/i3c') diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c index 18843e07d175..b192a8b91e5d 100644 --- a/drivers/i3c/master/svc-i3c-master.c +++ b/drivers/i3c/master/svc-i3c-master.c @@ -93,6 +93,7 @@ #define SVC_I3C_MINTMASKED 0x098 #define SVC_I3C_MERRWARN 0x09C #define SVC_I3C_MERRWARN_NACK BIT(2) +#define SVC_I3C_MERRWARN_TIMEOUT BIT(20) #define SVC_I3C_MDMACTRL 0x0A0 #define SVC_I3C_MDATACTRL 0x0AC #define SVC_I3C_MDATACTRL_FLUSHTB BIT(0) @@ -227,6 +228,14 @@ static bool svc_i3c_master_error(struct svc_i3c_master *master) if (SVC_I3C_MSTATUS_ERRWARN(mstatus)) { merrwarn = readl(master->regs + SVC_I3C_MERRWARN); writel(merrwarn, master->regs + SVC_I3C_MERRWARN); + + /* Ignore timeout error */ + if (merrwarn & SVC_I3C_MERRWARN_TIMEOUT) { + dev_dbg(master->dev, "Warning condition: MSTATUS 0x%08x, MERRWARN 0x%08x\n", + mstatus, merrwarn); + return false; + } + dev_err(master->dev, "Error condition: MSTATUS 0x%08x, MERRWARN 0x%08x\n", mstatus, merrwarn); -- cgit v1.2.3 From 8911eae9c8a947e5c1cc4fcce40473f1f5e475cd Mon Sep 17 00:00:00 2001 From: Frank Li Date: Tue, 17 Oct 2023 15:46:56 -0400 Subject: i3c: master: svc: fix compatibility string mismatch with binding doc In the binding documentation, the compatible string is specified as 'silvaco,i3c-master-v1', but in the driver, it is defined as 'silvaco,i3c-master'. Rename 'silvaco,i3c-master' to 'silvaco,i3c-master-v1' to ensure compatibility with the documentation. Signed-off-by: Frank Li Link: https://lore.kernel.org/r/20231017194657.3199749-1-Frank.Li@nxp.com Signed-off-by: Alexandre Belloni --- drivers/i3c/master/svc-i3c-master.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/i3c') diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c index b192a8b91e5d..cf703c00f633 100644 --- a/drivers/i3c/master/svc-i3c-master.c +++ b/drivers/i3c/master/svc-i3c-master.c @@ -1703,7 +1703,7 @@ static const struct dev_pm_ops svc_i3c_pm_ops = { }; static const struct of_device_id svc_i3c_master_of_match_tbl[] = { - { .compatible = "silvaco,i3c-master" }, + { .compatible = "silvaco,i3c-master-v1"}, { /* sentinel */ }, }; MODULE_DEVICE_TABLE(of, svc_i3c_master_of_match_tbl); -- cgit v1.2.3 From b53e9758a31c683fc8615df930262192ed5f034b Mon Sep 17 00:00:00 2001 From: Billy Tsai Date: Mon, 23 Oct 2023 16:02:37 +0800 Subject: i3c: master: mipi-i3c-hci: Fix a kernel panic for accessing DAT_data. The `i3c_master_bus_init` function may attach the I2C devices before the I3C bus initialization. In this flow, the DAT `alloc_entry`` will be used before the DAT `init`. Additionally, if the `i3c_master_bus_init` fails, the DAT `cleanup` will execute before the device is detached, which will execue DAT `free_entry` function. The above scenario can cause the driver to use DAT_data when it is NULL. Signed-off-by: Billy Tsai Link: https://lore.kernel.org/r/20231023080237.560936-1-billy_tsai@aspeedtech.com Signed-off-by: Alexandre Belloni --- drivers/i3c/master/mipi-i3c-hci/dat_v1.c | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) (limited to 'drivers/i3c') diff --git a/drivers/i3c/master/mipi-i3c-hci/dat_v1.c b/drivers/i3c/master/mipi-i3c-hci/dat_v1.c index 97bb49ff5b53..47b9b4d4ed3f 100644 --- a/drivers/i3c/master/mipi-i3c-hci/dat_v1.c +++ b/drivers/i3c/master/mipi-i3c-hci/dat_v1.c @@ -64,15 +64,17 @@ static int hci_dat_v1_init(struct i3c_hci *hci) return -EOPNOTSUPP; } - /* use a bitmap for faster free slot search */ - hci->DAT_data = bitmap_zalloc(hci->DAT_entries, GFP_KERNEL); - if (!hci->DAT_data) - return -ENOMEM; - - /* clear them */ - for (dat_idx = 0; dat_idx < hci->DAT_entries; dat_idx++) { - dat_w0_write(dat_idx, 0); - dat_w1_write(dat_idx, 0); + if (!hci->DAT_data) { + /* use a bitmap for faster free slot search */ + hci->DAT_data = bitmap_zalloc(hci->DAT_entries, GFP_KERNEL); + if (!hci->DAT_data) + return -ENOMEM; + + /* clear them */ + for (dat_idx = 0; dat_idx < hci->DAT_entries; dat_idx++) { + dat_w0_write(dat_idx, 0); + dat_w1_write(dat_idx, 0); + } } return 0; @@ -87,7 +89,13 @@ static void hci_dat_v1_cleanup(struct i3c_hci *hci) static int hci_dat_v1_alloc_entry(struct i3c_hci *hci) { unsigned int dat_idx; + int ret; + if (!hci->DAT_data) { + ret = hci_dat_v1_init(hci); + if (ret) + return ret; + } dat_idx = find_first_zero_bit(hci->DAT_data, hci->DAT_entries); if (dat_idx >= hci->DAT_entries) return -ENOENT; @@ -103,7 +111,8 @@ static void hci_dat_v1_free_entry(struct i3c_hci *hci, unsigned int dat_idx) { dat_w0_write(dat_idx, 0); dat_w1_write(dat_idx, 0); - __clear_bit(dat_idx, hci->DAT_data); + if (hci->DAT_data) + __clear_bit(dat_idx, hci->DAT_data); } static void hci_dat_v1_set_dynamic_addr(struct i3c_hci *hci, -- cgit v1.2.3 From 9fd00df05e81a2e1080ce6e9abc35533dca99d74 Mon Sep 17 00:00:00 2001 From: Zbigniew Lukwinski Date: Mon, 16 Oct 2023 00:23:34 +0200 Subject: i3c: master: handle IBIs in order they came IBI shall be handled in order they appear on the bus. Otherwise could hit case when order of handling them in device driver will be different. It may lead to invalid assembling fragmented packets or events order broken. Added separate workqueue with option WQ_MEM_RECLAIM for each device driver. This ensures IBI handling order and improves IBI handling performance: IBI handlers for device B are not blocked by IBI handlers for device A. Original solution (single workqueue in main driver) was able to handle also general IBI (not related to specific device) like HJ or MR. So leaving this for such purposes. Signed-off-by: Zbigniew Lukwinski Link: https://lore.kernel.org/r/20231015222334.1652401-2-zbigniew.lukwinski@linux.intel.com Signed-off-by: Alexandre Belloni --- drivers/i3c/master.c | 14 +++++++++++++- include/linux/i3c/master.h | 4 +++- 2 files changed, 16 insertions(+), 2 deletions(-) (limited to 'drivers/i3c') diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c index e66a1f84cefb..0cdc94e4cb77 100644 --- a/drivers/i3c/master.c +++ b/drivers/i3c/master.c @@ -2374,7 +2374,7 @@ static void i3c_master_unregister_i3c_devs(struct i3c_master_controller *master) void i3c_master_queue_ibi(struct i3c_dev_desc *dev, struct i3c_ibi_slot *slot) { atomic_inc(&dev->ibi->pending_ibis); - queue_work(dev->common.master->wq, &slot->work); + queue_work(dev->ibi->wq, &slot->work); } EXPORT_SYMBOL_GPL(i3c_master_queue_ibi); @@ -2819,6 +2819,12 @@ int i3c_dev_request_ibi_locked(struct i3c_dev_desc *dev, if (!ibi) return -ENOMEM; + ibi->wq = alloc_ordered_workqueue(dev_name(i3cdev_to_dev(dev->dev)), WQ_MEM_RECLAIM); + if (!ibi->wq) { + kfree(ibi); + return -ENOMEM; + } + atomic_set(&ibi->pending_ibis, 0); init_completion(&ibi->all_ibis_handled); ibi->handler = req->handler; @@ -2846,6 +2852,12 @@ void i3c_dev_free_ibi_locked(struct i3c_dev_desc *dev) WARN_ON(i3c_dev_disable_ibi_locked(dev)); master->ops->free_ibi(dev); + + if (dev->ibi->wq) { + destroy_workqueue(dev->ibi->wq); + dev->ibi->wq = NULL; + } + kfree(dev->ibi); dev->ibi = NULL; } diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h index 4fd6a777150f..349d7d0dda73 100644 --- a/include/linux/i3c/master.h +++ b/include/linux/i3c/master.h @@ -129,6 +129,7 @@ struct i3c_ibi_slot { * rejected by the master * @num_slots: number of IBI slots reserved for this device * @enabled: reflect the IBI status + * @wq: workqueue used to execute IBI handlers. * @handler: IBI handler specified at i3c_device_request_ibi() call time. This * handler will be called from the controller workqueue, and as such * is allowed to sleep (though it is recommended to process the IBI @@ -151,6 +152,7 @@ struct i3c_device_ibi_info { unsigned int max_payload_len; unsigned int num_slots; unsigned int enabled; + struct workqueue_struct *wq; void (*handler)(struct i3c_device *dev, const struct i3c_ibi_payload *payload); }; @@ -469,7 +471,7 @@ struct i3c_master_controller_ops { * @boardinfo.i2c: list of I2C boardinfo objects * @boardinfo: board-level information attached to devices connected on the bus * @bus: I3C bus exposed by this master - * @wq: workqueue used to execute IBI handlers. Can also be used by master + * @wq: workqueue which can be used by master * drivers if they need to postpone operations that need to take place * in a thread context. Typical examples are Hot Join processing which * requires taking the bus lock in maintenance, which in turn, can only -- cgit v1.2.3