From d2098981eb7b7d20edd294a8431908f8a0d2f9c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Draszik?= Date: Tue, 25 Mar 2025 09:46:07 +0000 Subject: [PATCH 1/8] firmware: exynos-acpm: use ktime APIs for timeout detection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit acpm_dequeue_by_polling() uses a loop counter and assumes that each iteration of the loop takes 20us. It may take longer, though, because usleep_range() may sleep a different amount. Switch to using ktime_get() / ktime_before() to detect the timeout condition more reliably. This change also makes the code easier to follow and it allows us to adjust the sleep if necessary, without having to adjust the loop counter exit condition. Reviewed-by: Tudor Ambarus Signed-off-by: André Draszik Link: https://lore.kernel.org/r/20250325-acpm-atomic-v3-1-c66aae7df925@linaro.org Signed-off-by: Krzysztof Kozlowski --- drivers/firmware/samsung/exynos-acpm.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/firmware/samsung/exynos-acpm.c b/drivers/firmware/samsung/exynos-acpm.c index a85b2dbdd9f0..542eaff03f9e 100644 --- a/drivers/firmware/samsung/exynos-acpm.c +++ b/drivers/firmware/samsung/exynos-acpm.c @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -32,8 +33,7 @@ #define ACPM_PROTOCOL_SEQNUM GENMASK(21, 16) -/* The unit of counter is 20 us. 5000 * 20 = 100 ms */ -#define ACPM_POLL_TIMEOUT 5000 +#define ACPM_POLL_TIMEOUT_US (100 * USEC_PER_MSEC) #define ACPM_TX_TIMEOUT_US 500000 #define ACPM_GS101_INITDATA_BASE 0xa000 @@ -284,12 +284,13 @@ static int acpm_dequeue_by_polling(struct acpm_chan *achan, const struct acpm_xfer *xfer) { struct device *dev = achan->acpm->dev; - unsigned int cnt_20us = 0; + ktime_t timeout; u32 seqnum; int ret; seqnum = FIELD_GET(ACPM_PROTOCOL_SEQNUM, xfer->txd[0]); + timeout = ktime_add_us(ktime_get(), ACPM_POLL_TIMEOUT_US); do { ret = acpm_get_rx(achan, xfer); if (ret) @@ -300,11 +301,10 @@ static int acpm_dequeue_by_polling(struct acpm_chan *achan, /* Determined experimentally. */ usleep_range(20, 30); - cnt_20us++; - } while (cnt_20us < ACPM_POLL_TIMEOUT); + } while (ktime_before(ktime_get(), timeout)); - dev_err(dev, "Timeout! ch:%u s:%u bitmap:%lx, cnt_20us = %d.\n", - achan->id, seqnum, achan->bitmap_seqnum[0], cnt_20us); + dev_err(dev, "Timeout! ch:%u s:%u bitmap:%lx.\n", + achan->id, seqnum, achan->bitmap_seqnum[0]); return -ETIME; } From 2d14c680e92f09d18b984cd1a8fae437f9ebc2ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Draszik?= Date: Tue, 25 Mar 2025 09:46:08 +0000 Subject: [PATCH 2/8] firmware: exynos-acpm: allow use during system shutdown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We need to access the PMIC during late system shutdown and at that time we are not allowed to sleep anymore. To make this case work, and since we can't detect this case in a non-racy way, switch to using udelay() unconditionally, instead of usleep_range(). Signed-off-by: André Draszik Link: https://lore.kernel.org/r/20250325-acpm-atomic-v3-2-c66aae7df925@linaro.org Signed-off-by: Krzysztof Kozlowski --- drivers/firmware/samsung/exynos-acpm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/firmware/samsung/exynos-acpm.c b/drivers/firmware/samsung/exynos-acpm.c index 542eaff03f9e..379da420b9eb 100644 --- a/drivers/firmware/samsung/exynos-acpm.c +++ b/drivers/firmware/samsung/exynos-acpm.c @@ -300,7 +300,7 @@ static int acpm_dequeue_by_polling(struct acpm_chan *achan, return 0; /* Determined experimentally. */ - usleep_range(20, 30); + udelay(20); } while (ktime_before(ktime_get(), timeout)); dev_err(dev, "Timeout! ch:%u s:%u bitmap:%lx.\n", From 935e5bd95df2c79404630a691caf42c3d7bc3a93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Draszik?= Date: Wed, 9 Apr 2025 21:37:24 +0100 Subject: [PATCH 3/8] dt-bindings: firmware: google,gs101-acpm-ipc: add PMIC child node MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The PMIC is supposed to be a child of ACPM, add it here to describe the connection. Reviewed-by: Krzysztof Kozlowski Signed-off-by: André Draszik Link: https://lore.kernel.org/r/20250409-s2mpg10-v4-3-d66d5f39b6bf@linaro.org Signed-off-by: Krzysztof Kozlowski --- .../firmware/google,gs101-acpm-ipc.yaml | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/Documentation/devicetree/bindings/firmware/google,gs101-acpm-ipc.yaml b/Documentation/devicetree/bindings/firmware/google,gs101-acpm-ipc.yaml index 2cdad1bbae73..9785aac3b5f3 100644 --- a/Documentation/devicetree/bindings/firmware/google,gs101-acpm-ipc.yaml +++ b/Documentation/devicetree/bindings/firmware/google,gs101-acpm-ipc.yaml @@ -27,6 +27,15 @@ properties: mboxes: maxItems: 1 + pmic: + description: Child node describing the main PMIC. + type: object + additionalProperties: true + + properties: + compatible: + const: samsung,s2mpg10-pmic + shmem: description: List of phandle pointing to the shared memory (SHM) area. The memory @@ -43,8 +52,34 @@ additionalProperties: false examples: - | + #include + power-management { compatible = "google,gs101-acpm-ipc"; mboxes = <&ap2apm_mailbox>; shmem = <&apm_sram>; + + pmic { + compatible = "samsung,s2mpg10-pmic"; + interrupts-extended = <&gpa0 6 IRQ_TYPE_LEVEL_LOW>; + + regulators { + LDO1 { + regulator-name = "vdd_ldo1"; + regulator-min-microvolt = <700000>; + regulator-max-microvolt = <1300000>; + regulator-always-on; + }; + + // ... + + BUCK1 { + regulator-name = "vdd_mif"; + regulator-min-microvolt = <450000>; + regulator-max-microvolt = <1300000>; + regulator-always-on; + regulator-boot-on; + }; + }; + }; }; From 67af3cd813695fd3e6432b0849c453250c4685aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Draszik?= Date: Wed, 19 Mar 2025 05:38:23 +0000 Subject: [PATCH 4/8] firmware: exynos-acpm: fix reading longer results MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ACPM commands that return more than 8 bytes currently don't work correctly, as this driver ignores any such returned bytes. This is evident in at least acpm_pmic_bulk_read(), where up to 8 registers can be read back and those 8 register values are placed starting at &xfer->rxd[8]. The reason is that xfter->rxlen is initialized with the size of a pointer (8 bytes), rather than the size of the byte array that pointer points to (16 bytes) Update the code such that we set the number of bytes expected to be the size of the rx buffer. Note1: While different commands have different lengths rx buffers, we have to specify the same length for all rx buffers since acpm_get_rx() assumes they're all the same length. Note2: The different commands also have different lengths tx buffers, but before switching the code to use the minimum possible length, some more testing would have to be done to ensure this works correctly in all situations. It seems wiser to just apply this fix here without additional logic changes for now. Fixes: a88927b534ba ("firmware: add Exynos ACPM protocol driver") Reviewed-by: Tudor Ambarus Signed-off-by: André Draszik Link: https://lore.kernel.org/r/20250319-acpm-fixes-v2-1-ac2c1bcf322b@linaro.org Signed-off-by: Krzysztof Kozlowski --- drivers/firmware/samsung/exynos-acpm-pmic.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/firmware/samsung/exynos-acpm-pmic.c b/drivers/firmware/samsung/exynos-acpm-pmic.c index 85e90d236da2..39b33a356ebd 100644 --- a/drivers/firmware/samsung/exynos-acpm-pmic.c +++ b/drivers/firmware/samsung/exynos-acpm-pmic.c @@ -43,13 +43,13 @@ static inline u32 acpm_pmic_get_bulk(u32 data, unsigned int i) return (data >> (ACPM_PMIC_BULK_SHIFT * i)) & ACPM_PMIC_BULK_MASK; } -static void acpm_pmic_set_xfer(struct acpm_xfer *xfer, u32 *cmd, +static void acpm_pmic_set_xfer(struct acpm_xfer *xfer, u32 *cmd, size_t cmdlen, unsigned int acpm_chan_id) { xfer->txd = cmd; xfer->rxd = cmd; - xfer->txlen = sizeof(cmd); - xfer->rxlen = sizeof(cmd); + xfer->txlen = cmdlen; + xfer->rxlen = cmdlen; xfer->acpm_chan_id = acpm_chan_id; } @@ -71,7 +71,7 @@ int acpm_pmic_read_reg(const struct acpm_handle *handle, int ret; acpm_pmic_init_read_cmd(cmd, type, reg, chan); - acpm_pmic_set_xfer(&xfer, cmd, acpm_chan_id); + acpm_pmic_set_xfer(&xfer, cmd, sizeof(cmd), acpm_chan_id); ret = acpm_do_xfer(handle, &xfer); if (ret) @@ -104,7 +104,7 @@ int acpm_pmic_bulk_read(const struct acpm_handle *handle, return -EINVAL; acpm_pmic_init_bulk_read_cmd(cmd, type, reg, chan, count); - acpm_pmic_set_xfer(&xfer, cmd, acpm_chan_id); + acpm_pmic_set_xfer(&xfer, cmd, sizeof(cmd), acpm_chan_id); ret = acpm_do_xfer(handle, &xfer); if (ret) @@ -144,7 +144,7 @@ int acpm_pmic_write_reg(const struct acpm_handle *handle, int ret; acpm_pmic_init_write_cmd(cmd, type, reg, chan, value); - acpm_pmic_set_xfer(&xfer, cmd, acpm_chan_id); + acpm_pmic_set_xfer(&xfer, cmd, sizeof(cmd), acpm_chan_id); ret = acpm_do_xfer(handle, &xfer); if (ret) @@ -184,7 +184,7 @@ int acpm_pmic_bulk_write(const struct acpm_handle *handle, return -EINVAL; acpm_pmic_init_bulk_write_cmd(cmd, type, reg, chan, count, buf); - acpm_pmic_set_xfer(&xfer, cmd, acpm_chan_id); + acpm_pmic_set_xfer(&xfer, cmd, sizeof(cmd), acpm_chan_id); ret = acpm_do_xfer(handle, &xfer); if (ret) @@ -214,7 +214,7 @@ int acpm_pmic_update_reg(const struct acpm_handle *handle, int ret; acpm_pmic_init_update_cmd(cmd, type, reg, chan, value, mask); - acpm_pmic_set_xfer(&xfer, cmd, acpm_chan_id); + acpm_pmic_set_xfer(&xfer, cmd, sizeof(cmd), acpm_chan_id); ret = acpm_do_xfer(handle, &xfer); if (ret) From 53734383a73888e6d765aa07f4523802fdf1ee10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Draszik?= Date: Wed, 19 Mar 2025 05:38:24 +0000 Subject: [PATCH 5/8] firmware: exynos-acpm: silence EPROBE_DEFER error on boot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This driver emits error messages when client drivers are trying to get an interface handle to this driver here before this driver has completed _probe(). Given this driver returns -EPROBE_DEFER in that case, this is not an error and shouldn't be emitted to the log, similar to how dev_err_probe() behaves, so just remove them. This change also allows us to simplify the logic around releasing of the acpm_np handle. Fixes: a88927b534ba ("firmware: add Exynos ACPM protocol driver") Signed-off-by: André Draszik Link: https://lore.kernel.org/r/20250319-acpm-fixes-v2-2-ac2c1bcf322b@linaro.org Signed-off-by: Krzysztof Kozlowski --- drivers/firmware/samsung/exynos-acpm.c | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/drivers/firmware/samsung/exynos-acpm.c b/drivers/firmware/samsung/exynos-acpm.c index 379da420b9eb..d1c9ca0b3960 100644 --- a/drivers/firmware/samsung/exynos-acpm.c +++ b/drivers/firmware/samsung/exynos-acpm.c @@ -680,24 +680,17 @@ static const struct acpm_handle *acpm_get_by_phandle(struct device *dev, return ERR_PTR(-ENODEV); pdev = of_find_device_by_node(acpm_np); - if (!pdev) { - dev_err(dev, "Cannot find device node %s\n", acpm_np->name); - of_node_put(acpm_np); - return ERR_PTR(-EPROBE_DEFER); - } - of_node_put(acpm_np); + if (!pdev) + return ERR_PTR(-EPROBE_DEFER); acpm = platform_get_drvdata(pdev); if (!acpm) { - dev_err(dev, "Cannot get drvdata from %s\n", - dev_name(&pdev->dev)); platform_device_put(pdev); return ERR_PTR(-EPROBE_DEFER); } if (!try_module_get(pdev->dev.driver->owner)) { - dev_err(dev, "Cannot get module reference.\n"); platform_device_put(pdev); return ERR_PTR(-EPROBE_DEFER); } From 636baba9489a634c956d6f02076af6bc1725c132 Mon Sep 17 00:00:00 2001 From: Tudor Ambarus Date: Thu, 27 Mar 2025 12:54:27 +0000 Subject: [PATCH 6/8] firmware: exynos-acpm: populate devices from device tree data MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ACPM clients (PMIC, clocks, etc.) will be modeled as children of the ACPM interface. Populate children platform_devices from device tree. Signed-off-by: Tudor Ambarus Signed-off-by: André Draszik Link: https://lore.kernel.org/r/20250327-acpm-children-v1-1-0afe15ee2ff7@linaro.org Signed-off-by: Krzysztof Kozlowski --- drivers/firmware/samsung/exynos-acpm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/firmware/samsung/exynos-acpm.c b/drivers/firmware/samsung/exynos-acpm.c index d1c9ca0b3960..b379f2d9cad0 100644 --- a/drivers/firmware/samsung/exynos-acpm.c +++ b/drivers/firmware/samsung/exynos-acpm.c @@ -633,7 +633,7 @@ static int acpm_probe(struct platform_device *pdev) platform_set_drvdata(pdev, acpm); - return 0; + return devm_of_platform_populate(dev); } /** From a8dc26a0ec43fd416ca781a8030807e65f71cfc5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Draszik?= Date: Thu, 27 Mar 2025 12:54:28 +0000 Subject: [PATCH 7/8] firmware: exynos-acpm: introduce devm_acpm_get_by_node() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit To allow ACPM clients to simply be children of the ACPM node in DT, they need to be able to get the ACPM handle based on that ACPM node directly. Add an API to allow them to do so, devm_acpm_get_by_node(). At the same time, the previous approach of acquiring the ACPM handle via a DT phandle is now obsolete and we can remove devm_acpm_get_by_phandle(), which was there to facilitate that. There are no existing or anticipated upcoming users of that API, because all clients should be children of the ACPM node going forward. Note that no DTs have been merged that use the old approach, so doing this API change in this driver now will not affect any existing DTs or client drivers. Signed-off-by: André Draszik Link: https://lore.kernel.org/r/20250327-acpm-children-v1-2-0afe15ee2ff7@linaro.org Signed-off-by: Krzysztof Kozlowski --- drivers/firmware/samsung/exynos-acpm.c | 23 ++++++++----------- .../firmware/samsung/exynos-acpm-protocol.h | 6 +++-- 2 files changed, 13 insertions(+), 16 deletions(-) diff --git a/drivers/firmware/samsung/exynos-acpm.c b/drivers/firmware/samsung/exynos-acpm.c index b379f2d9cad0..93f87cd90f24 100644 --- a/drivers/firmware/samsung/exynos-acpm.c +++ b/drivers/firmware/samsung/exynos-acpm.c @@ -667,20 +667,14 @@ static void devm_acpm_release(struct device *dev, void *res) * * Return: pointer to handle on success, ERR_PTR(-errno) otherwise. */ -static const struct acpm_handle *acpm_get_by_phandle(struct device *dev, - const char *property) +static const struct acpm_handle *acpm_get_by_node(struct device *dev, + struct device_node *acpm_np) { struct platform_device *pdev; - struct device_node *acpm_np; struct device_link *link; struct acpm_info *acpm; - acpm_np = of_parse_phandle(dev->of_node, property, 0); - if (!acpm_np) - return ERR_PTR(-ENODEV); - pdev = of_find_device_by_node(acpm_np); - of_node_put(acpm_np); if (!pdev) return ERR_PTR(-EPROBE_DEFER); @@ -709,14 +703,14 @@ static const struct acpm_handle *acpm_get_by_phandle(struct device *dev, } /** - * devm_acpm_get_by_phandle() - managed get handle using phandle. - * @dev: device pointer requesting ACPM handle. - * @property: property name containing phandle on ACPM node. + * devm_acpm_get_by_node() - managed get handle using node pointer. + * @dev: device pointer requesting ACPM handle. + * @np: ACPM device tree node. * * Return: pointer to handle on success, ERR_PTR(-errno) otherwise. */ -const struct acpm_handle *devm_acpm_get_by_phandle(struct device *dev, - const char *property) +const struct acpm_handle *devm_acpm_get_by_node(struct device *dev, + struct device_node *np) { const struct acpm_handle **ptr, *handle; @@ -724,7 +718,7 @@ const struct acpm_handle *devm_acpm_get_by_phandle(struct device *dev, if (!ptr) return ERR_PTR(-ENOMEM); - handle = acpm_get_by_phandle(dev, property); + handle = acpm_get_by_node(dev, np); if (!IS_ERR(handle)) { *ptr = handle; devres_add(dev, ptr); @@ -734,6 +728,7 @@ const struct acpm_handle *devm_acpm_get_by_phandle(struct device *dev, return handle; } +EXPORT_SYMBOL_GPL(devm_acpm_get_by_node); static const struct acpm_match_data acpm_gs101 = { .initdata_base = ACPM_GS101_INITDATA_BASE, diff --git a/include/linux/firmware/samsung/exynos-acpm-protocol.h b/include/linux/firmware/samsung/exynos-acpm-protocol.h index 76255b5d06b1..f628bf1862c2 100644 --- a/include/linux/firmware/samsung/exynos-acpm-protocol.h +++ b/include/linux/firmware/samsung/exynos-acpm-protocol.h @@ -11,6 +11,7 @@ #include struct acpm_handle; +struct device_node; struct acpm_pmic_ops { int (*read_reg)(const struct acpm_handle *handle, @@ -44,6 +45,7 @@ struct acpm_handle { struct device; -const struct acpm_handle *devm_acpm_get_by_phandle(struct device *dev, - const char *property); +const struct acpm_handle *devm_acpm_get_by_node(struct device *dev, + struct device_node *np); + #endif /* __EXYNOS_ACPM_PROTOCOL_H */ From 2c2e5e908ea2b53aa0d21fbfe4d1dab527a7703e Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Thu, 24 Apr 2025 22:33:09 +0200 Subject: [PATCH 8/8] firmware: exynos-acpm: Correct kerneldoc and use typical np argument name Correct kerneldoc warnings after commit a8dc26a0ec43 ("firmware: exynos-acpm: introduce devm_acpm_get_by_node()") changed the function prototype: exynos-acpm.c:672: warning: Function parameter or struct member 'acpm_np' not described in 'acpm_get_by_node' exynos-acpm.c:672: warning: expecting prototype for acpm_get_by_phandle(). Prototype was for acpm_get_by_node() instead While touching the lines, change the name of device_node pointer to 'np' to match convention. Fixes: a8dc26a0ec43 ("firmware: exynos-acpm: introduce devm_acpm_get_by_node()") Reported-by: kernel test robot Closes: https://lore.kernel.org/oe-kbuild-all/202504222051.7TqaSQ48-lkp@intel.com/ Reviewed-by: Tudor Ambarus Link: https://lore.kernel.org/r/20250424203308.402168-2-krzysztof.kozlowski@linaro.org Signed-off-by: Krzysztof Kozlowski --- drivers/firmware/samsung/exynos-acpm.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/firmware/samsung/exynos-acpm.c b/drivers/firmware/samsung/exynos-acpm.c index 93f87cd90f24..5266b66a32e1 100644 --- a/drivers/firmware/samsung/exynos-acpm.c +++ b/drivers/firmware/samsung/exynos-acpm.c @@ -661,20 +661,20 @@ static void devm_acpm_release(struct device *dev, void *res) } /** - * acpm_get_by_phandle() - get the ACPM handle using DT phandle. - * @dev: device pointer requesting ACPM handle. - * @property: property name containing phandle on ACPM node. + * acpm_get_by_node() - get the ACPM handle using node pointer. + * @dev: device pointer requesting ACPM handle. + * @np: ACPM device tree node. * * Return: pointer to handle on success, ERR_PTR(-errno) otherwise. */ static const struct acpm_handle *acpm_get_by_node(struct device *dev, - struct device_node *acpm_np) + struct device_node *np) { struct platform_device *pdev; struct device_link *link; struct acpm_info *acpm; - pdev = of_find_device_by_node(acpm_np); + pdev = of_find_device_by_node(np); if (!pdev) return ERR_PTR(-EPROBE_DEFER);