From 435c20eed572a95709b1536ff78832836b2f91b1 Mon Sep 17 00:00:00 2001 From: Zichen Xie Date: Thu, 14 Nov 2024 23:43:36 -0600 Subject: [PATCH 01/16] kunit: Fix potential null dereference in kunit_device_driver_test() kunit_kzalloc() may return a NULL pointer, dereferencing it without NULL check may lead to NULL dereference. Add a NULL check for test_state. Link: https://lore.kernel.org/r/20241115054335.21673-1-zichenxie0106@gmail.com Fixes: d03c720e03bd ("kunit: Add APIs for managing devices") Signed-off-by: Zichen Xie Cc: stable@vger.kernel.org Reviewed-by: David Gow Signed-off-by: Shuah Khan --- lib/kunit/kunit-test.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/kunit/kunit-test.c b/lib/kunit/kunit-test.c index 37e02be1e710..d9c781c859fd 100644 --- a/lib/kunit/kunit-test.c +++ b/lib/kunit/kunit-test.c @@ -805,6 +805,8 @@ static void kunit_device_driver_test(struct kunit *test) struct device *test_device; struct driver_test_state *test_state = kunit_kzalloc(test, sizeof(*test_state), GFP_KERNEL); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, test_state); + test->priv = test_state; test_driver = kunit_driver_create(test, "my_driver"); -- 2.50.1 From 95b6d723a00710f129b81556c93fdd994c105ab1 Mon Sep 17 00:00:00 2001 From: Kuan-Wei Chiu Date: Fri, 15 Nov 2024 00:55:58 +0800 Subject: [PATCH 02/16] kunit: debugfs: Use IS_ERR() for alloc_string_stream() error check The alloc_string_stream() function only returns ERR_PTR(-ENOMEM) on failure and never returns NULL. Therefore, switching the error check in the caller from IS_ERR_OR_NULL to IS_ERR improves clarity, indicating that this function will return an error pointer (not NULL) when an error occurs. This change avoids any ambiguity regarding the function's return behavior. Link: https://lore.kernel.org/lkml/Zy9deU5VK3YR+r9N@visitorckw-System-Product-Name Signed-off-by: Kuan-Wei Chiu Reviewed-by: David Gow Signed-off-by: Shuah Khan --- lib/kunit/debugfs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/kunit/debugfs.c b/lib/kunit/debugfs.c index b25d214b93e1..af71911f4a07 100644 --- a/lib/kunit/debugfs.c +++ b/lib/kunit/debugfs.c @@ -181,7 +181,7 @@ void kunit_debugfs_create_suite(struct kunit_suite *suite) * successfully. */ stream = alloc_string_stream(GFP_KERNEL); - if (IS_ERR_OR_NULL(stream)) + if (IS_ERR(stream)) return; string_stream_set_append_newlines(stream, true); @@ -189,7 +189,7 @@ void kunit_debugfs_create_suite(struct kunit_suite *suite) kunit_suite_for_each_test_case(suite, test_case) { stream = alloc_string_stream(GFP_KERNEL); - if (IS_ERR_OR_NULL(stream)) + if (IS_ERR(stream)) goto err; string_stream_set_append_newlines(stream, true); -- 2.50.1 From d28252440428d37076de166c3d576a5d2f4a53e8 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Thomas=20Wei=C3=9Fschuh?= Date: Mon, 11 Nov 2024 06:29:18 +0100 Subject: [PATCH 03/16] kunit: qemu_configs: Add LoongArch config MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Add a basic config to run kunit tests on LoongArch. This requires QEMU 9.1.0 or later for the necessary direct kernel boot support. Link: https://lore.kernel.org/r/20241111-kunit-loongarch-v2-1-7676eb5f2da3@linutronix.de Signed-off-by: Thomas Weißschuh Reviewed-by: Bibo Mao Reviewed-by: David Gow Signed-off-by: Shuah Khan --- tools/testing/kunit/qemu_configs/loongarch.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 tools/testing/kunit/qemu_configs/loongarch.py diff --git a/tools/testing/kunit/qemu_configs/loongarch.py b/tools/testing/kunit/qemu_configs/loongarch.py new file mode 100644 index 000000000000..a874a2156e0f --- /dev/null +++ b/tools/testing/kunit/qemu_configs/loongarch.py @@ -0,0 +1,18 @@ +# SPDX-License-Identifier: GPL-2.0 + +from ..qemu_config import QemuArchParams + +QEMU_ARCH = QemuArchParams(linux_arch='loongarch', + kconfig=''' +CONFIG_EFI_STUB=n +CONFIG_PCI_HOST_GENERIC=y +CONFIG_SERIAL_8250=y +CONFIG_SERIAL_8250_CONSOLE=y +CONFIG_SERIAL_OF_PLATFORM=y +''', + qemu_arch='loongarch64', + kernel_path='arch/loongarch/boot/vmlinux.elf', + kernel_command_line='console=ttyS0', + extra_qemu_params=[ + '-machine', 'virt', + '-cpu', 'max',]) -- 2.50.1 From 0a1111d4cbaf45100e30ebd98d1e1a175b8ce22d Mon Sep 17 00:00:00 2001 From: =?utf8?q?Thomas=20Wei=C3=9Fschuh?= Date: Mon, 11 Nov 2024 06:29:19 +0100 Subject: [PATCH 04/16] kunit: tool: Allow overriding the shutdown mode from qemu config MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Not all platforms support machine reboot. If it a proper reboot is not supported the machine will hang. Allow the QEMU configuration to override the necessary shutdown mode for the specific system under test. Link: https://lore.kernel.org/r/20241111-kunit-loongarch-v2-2-7676eb5f2da3@linutronix.de Signed-off-by: Thomas Weißschuh Reviewed-by: David Gow Signed-off-by: Shuah Khan --- tools/testing/kunit/kunit_kernel.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py index 61931c4926fd..e76d7894b6c5 100644 --- a/tools/testing/kunit/kunit_kernel.py +++ b/tools/testing/kunit/kunit_kernel.py @@ -105,7 +105,9 @@ class LinuxSourceTreeOperationsQemu(LinuxSourceTreeOperations): self._kconfig = qemu_arch_params.kconfig self._qemu_arch = qemu_arch_params.qemu_arch self._kernel_path = qemu_arch_params.kernel_path - self._kernel_command_line = qemu_arch_params.kernel_command_line + ' kunit_shutdown=reboot' + self._kernel_command_line = qemu_arch_params.kernel_command_line + if 'kunit_shutdown=' not in self._kernel_command_line: + self._kernel_command_line += ' kunit_shutdown=reboot' self._extra_qemu_params = qemu_arch_params.extra_qemu_params self._serial = qemu_arch_params.serial -- 2.50.1 From 62adcae479fe5bc04fa3b6c3f93bd340441f8b25 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Thomas=20Wei=C3=9Fschuh?= Date: Mon, 11 Nov 2024 06:29:20 +0100 Subject: [PATCH 05/16] kunit: qemu_configs: loongarch: Enable shutdown MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit QEMU for LoongArch does not yet support shutdown/restart through ACPI. Use the pvpanic driver to enable shutdowns. This requires 9.1.0 for shutdown support in pvpanic, but that is the requirement of kunit on LoongArch anyways. Link: https://lore.kernel.org/r/20241111-kunit-loongarch-v2-3-7676eb5f2da3@linutronix.de Signed-off-by: Thomas Weißschuh Reviewed-by: Bibo Mao Reviewed-by: David Gow Signed-off-by: Shuah Khan --- tools/testing/kunit/qemu_configs/loongarch.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tools/testing/kunit/qemu_configs/loongarch.py b/tools/testing/kunit/qemu_configs/loongarch.py index a874a2156e0f..a92422967d1d 100644 --- a/tools/testing/kunit/qemu_configs/loongarch.py +++ b/tools/testing/kunit/qemu_configs/loongarch.py @@ -6,13 +6,16 @@ QEMU_ARCH = QemuArchParams(linux_arch='loongarch', kconfig=''' CONFIG_EFI_STUB=n CONFIG_PCI_HOST_GENERIC=y +CONFIG_PVPANIC=y +CONFIG_PVPANIC_PCI=y CONFIG_SERIAL_8250=y CONFIG_SERIAL_8250_CONSOLE=y CONFIG_SERIAL_OF_PLATFORM=y ''', qemu_arch='loongarch64', kernel_path='arch/loongarch/boot/vmlinux.elf', - kernel_command_line='console=ttyS0', + kernel_command_line='console=ttyS0 kunit_shutdown=poweroff', extra_qemu_params=[ '-machine', 'virt', + '-device', 'pvpanic-pci', '-cpu', 'max',]) -- 2.50.1 From 45af52e7d3b8560f21d139b3759735eead8b1653 Mon Sep 17 00:00:00 2001 From: guoweikang Date: Wed, 20 Nov 2024 13:27:49 +0800 Subject: [PATCH 06/16] ftrace: Fix regression with module command in stack_trace_filter When executing the following command: # echo "write*:mod:ext3" > /sys/kernel/tracing/stack_trace_filter The current mod command causes a null pointer dereference. While commit 0f17976568b3f ("ftrace: Fix regression with module command in stack_trace_filter") has addressed part of the issue, it left a corner case unhandled, which still results in a kernel crash. Cc: stable@vger.kernel.org Cc: Masami Hiramatsu Cc: Mark Rutland Cc: Mathieu Desnoyers Link: https://lore.kernel.org/20241120052750.275463-1-guoweikang.kernel@gmail.com Fixes: 04ec7bb642b77 ("tracing: Have the trace_array hold the list of registered func probes"); Signed-off-by: guoweikang Signed-off-by: Steven Rostedt (Google) --- kernel/trace/ftrace.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c index 4c28dd177ca6..5ff0822342ac 100644 --- a/kernel/trace/ftrace.c +++ b/kernel/trace/ftrace.c @@ -5076,6 +5076,9 @@ ftrace_mod_callback(struct trace_array *tr, struct ftrace_hash *hash, char *func; int ret; + if (!tr) + return -ENODEV; + /* match_records() modifies func, and we need the original */ func = kstrdup(func_orig, GFP_KERNEL); if (!func) -- 2.50.1 From 02e9bda80d66cdd53364b9b32166ac0be1ced58f Mon Sep 17 00:00:00 2001 From: Stefan Berger Date: Mon, 14 Oct 2024 10:16:43 -0400 Subject: [PATCH 07/16] tpm: ibmvtpm: Set TPM_OPS_AUTO_STARTUP flag on driver Set the TPM_OPS_AUTO_STARTUP on the driver so that the ibmvtpm driver now uses tpm2_auto_startup and tpm1_auto_startup like many other drivers do. Remove tpm_get_timeouts, tpm2_get_cc_attrs_tbl, and tpm2_sessions_init calls from it since these will all be called in tpm2_auto_startup and tpm1_auto_startup. The exporting of the tpm2_session_init symbol was only necessary while the ibmvtpm driver was calling this function. Since this is not the case anymore, remove this symbol from being exported. What is new for the ibmvtpm driver is that now tpm2_do_selftest and tpm1_do_selftest will be called that send commands to the TPM to perform or continue its selftest. However, the firmware should already have sent these commands so that the TPM will not do much work at this time. Signed-off-by: Stefan Berger Reviewed-by: Jarkko Sakkinen Signed-off-by: Jarkko Sakkinen --- drivers/char/tpm/tpm2-sessions.c | 1 - drivers/char/tpm/tpm_ibmvtpm.c | 15 +-------------- 2 files changed, 1 insertion(+), 15 deletions(-) diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c index b0f13c8ea79c..b70165b588ec 100644 --- a/drivers/char/tpm/tpm2-sessions.c +++ b/drivers/char/tpm/tpm2-sessions.c @@ -1390,5 +1390,4 @@ int tpm2_sessions_init(struct tpm_chip *chip) return rc; } -EXPORT_SYMBOL(tpm2_sessions_init); #endif /* CONFIG_TCG_TPM2_HMAC */ diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c index 1e5b107d1f3b..76d048f63d55 100644 --- a/drivers/char/tpm/tpm_ibmvtpm.c +++ b/drivers/char/tpm/tpm_ibmvtpm.c @@ -450,6 +450,7 @@ static bool tpm_ibmvtpm_req_canceled(struct tpm_chip *chip, u8 status) } static const struct tpm_class_ops tpm_ibmvtpm = { + .flags = TPM_OPS_AUTO_STARTUP, .recv = tpm_ibmvtpm_recv, .send = tpm_ibmvtpm_send, .cancel = tpm_ibmvtpm_cancel, @@ -690,20 +691,6 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev, if (!strcmp(id->compat, "IBM,vtpm20")) chip->flags |= TPM_CHIP_FLAG_TPM2; - rc = tpm_get_timeouts(chip); - if (rc) - goto init_irq_cleanup; - - if (chip->flags & TPM_CHIP_FLAG_TPM2) { - rc = tpm2_get_cc_attrs_tbl(chip); - if (rc) - goto init_irq_cleanup; - - rc = tpm2_sessions_init(chip); - if (rc) - goto init_irq_cleanup; - } - return tpm_chip_register(chip); init_irq_cleanup: do { -- 2.50.1 From 932e3a5e1ea31712c91e6b4c64d0c7f675315ab4 Mon Sep 17 00:00:00 2001 From: Jan Dabros Date: Thu, 10 Oct 2024 09:15:58 +0000 Subject: [PATCH 08/16] char: tpm: cr50: Use generic request/relinquish locality ops Instead of using static functions tpm_cr50_request_locality and tpm_cr50_release_locality register callbacks from tpm class chip->ops created for this purpose. Signed-off-by: Jan Dabros Signed-off-by: Grzegorz Bernacki Signed-off-by: Jarkko Sakkinen --- drivers/char/tpm/tpm_tis_i2c_cr50.c | 94 +++++++++++++++++------------ 1 file changed, 54 insertions(+), 40 deletions(-) diff --git a/drivers/char/tpm/tpm_tis_i2c_cr50.c b/drivers/char/tpm/tpm_tis_i2c_cr50.c index adf22992138e..eed1c296a00c 100644 --- a/drivers/char/tpm/tpm_tis_i2c_cr50.c +++ b/drivers/char/tpm/tpm_tis_i2c_cr50.c @@ -17,6 +17,7 @@ */ #include +#include #include #include #include @@ -35,6 +36,7 @@ #define TPM_CR50_I2C_MAX_RETRIES 3 /* Max retries due to I2C errors */ #define TPM_CR50_I2C_RETRY_DELAY_LO 55 /* Min usecs between retries on I2C */ #define TPM_CR50_I2C_RETRY_DELAY_HI 65 /* Max usecs between retries on I2C */ +#define TPM_CR50_I2C_DEFAULT_LOC 0 #define TPM_I2C_ACCESS(l) (0x0000 | ((l) << 4)) #define TPM_I2C_STS(l) (0x0001 | ((l) << 4)) @@ -285,25 +287,26 @@ out: } /** - * tpm_cr50_check_locality() - Verify TPM locality 0 is active. + * tpm_cr50_check_locality() - Verify if required TPM locality is active. * @chip: A TPM chip. + * @loc: Locality to be verified * * Return: - * - 0: Success. + * - loc: Success. * - -errno: A POSIX error code. */ -static int tpm_cr50_check_locality(struct tpm_chip *chip) +static int tpm_cr50_check_locality(struct tpm_chip *chip, int loc) { u8 mask = TPM_ACCESS_VALID | TPM_ACCESS_ACTIVE_LOCALITY; u8 buf; int rc; - rc = tpm_cr50_i2c_read(chip, TPM_I2C_ACCESS(0), &buf, sizeof(buf)); + rc = tpm_cr50_i2c_read(chip, TPM_I2C_ACCESS(loc), &buf, sizeof(buf)); if (rc < 0) return rc; if ((buf & mask) == mask) - return 0; + return loc; return -EIO; } @@ -311,48 +314,57 @@ static int tpm_cr50_check_locality(struct tpm_chip *chip) /** * tpm_cr50_release_locality() - Release TPM locality. * @chip: A TPM chip. - * @force: Flag to force release if set. + * @loc: Locality to be released + * + * Return: + * - 0: Success. + * - -errno: A POSIX error code. */ -static void tpm_cr50_release_locality(struct tpm_chip *chip, bool force) +static int tpm_cr50_release_locality(struct tpm_chip *chip, int loc) { u8 mask = TPM_ACCESS_VALID | TPM_ACCESS_REQUEST_PENDING; - u8 addr = TPM_I2C_ACCESS(0); + u8 addr = TPM_I2C_ACCESS(loc); u8 buf; + int rc; - if (tpm_cr50_i2c_read(chip, addr, &buf, sizeof(buf)) < 0) - return; + rc = tpm_cr50_i2c_read(chip, addr, &buf, sizeof(buf)); + if (rc < 0) + return rc; - if (force || (buf & mask) == mask) { + if ((buf & mask) == mask) { buf = TPM_ACCESS_ACTIVE_LOCALITY; - tpm_cr50_i2c_write(chip, addr, &buf, sizeof(buf)); + rc = tpm_cr50_i2c_write(chip, addr, &buf, sizeof(buf)); } + + return rc; } /** - * tpm_cr50_request_locality() - Request TPM locality 0. + * tpm_cr50_request_locality() - Request TPM locality. * @chip: A TPM chip. + * @loc: Locality to be requested. * * Return: - * - 0: Success. + * - loc: Success. * - -errno: A POSIX error code. */ -static int tpm_cr50_request_locality(struct tpm_chip *chip) +static int tpm_cr50_request_locality(struct tpm_chip *chip, int loc) { u8 buf = TPM_ACCESS_REQUEST_USE; unsigned long stop; int rc; - if (!tpm_cr50_check_locality(chip)) - return 0; + if (tpm_cr50_check_locality(chip, loc) == loc) + return loc; - rc = tpm_cr50_i2c_write(chip, TPM_I2C_ACCESS(0), &buf, sizeof(buf)); + rc = tpm_cr50_i2c_write(chip, TPM_I2C_ACCESS(loc), &buf, sizeof(buf)); if (rc < 0) return rc; stop = jiffies + chip->timeout_a; do { - if (!tpm_cr50_check_locality(chip)) - return 0; + if (tpm_cr50_check_locality(chip, loc) == loc) + return loc; msleep(TPM_CR50_TIMEOUT_SHORT_MS); } while (time_before(jiffies, stop)); @@ -373,7 +385,7 @@ static u8 tpm_cr50_i2c_tis_status(struct tpm_chip *chip) { u8 buf[4]; - if (tpm_cr50_i2c_read(chip, TPM_I2C_STS(0), buf, sizeof(buf)) < 0) + if (tpm_cr50_i2c_read(chip, TPM_I2C_STS(chip->locality), buf, sizeof(buf)) < 0) return 0; return buf[0]; @@ -389,7 +401,7 @@ static void tpm_cr50_i2c_tis_set_ready(struct tpm_chip *chip) { u8 buf[4] = { TPM_STS_COMMAND_READY }; - tpm_cr50_i2c_write(chip, TPM_I2C_STS(0), buf, sizeof(buf)); + tpm_cr50_i2c_write(chip, TPM_I2C_STS(chip->locality), buf, sizeof(buf)); msleep(TPM_CR50_TIMEOUT_SHORT_MS); } @@ -419,7 +431,7 @@ static int tpm_cr50_i2c_get_burst_and_status(struct tpm_chip *chip, u8 mask, stop = jiffies + chip->timeout_b; do { - if (tpm_cr50_i2c_read(chip, TPM_I2C_STS(0), buf, sizeof(buf)) < 0) { + if (tpm_cr50_i2c_read(chip, TPM_I2C_STS(chip->locality), buf, sizeof(buf)) < 0) { msleep(TPM_CR50_TIMEOUT_SHORT_MS); continue; } @@ -453,7 +465,7 @@ static int tpm_cr50_i2c_tis_recv(struct tpm_chip *chip, u8 *buf, size_t buf_len) u8 mask = TPM_STS_VALID | TPM_STS_DATA_AVAIL; size_t burstcnt, cur, len, expected; - u8 addr = TPM_I2C_DATA_FIFO(0); + u8 addr = TPM_I2C_DATA_FIFO(chip->locality); u32 status; int rc; @@ -515,7 +527,6 @@ static int tpm_cr50_i2c_tis_recv(struct tpm_chip *chip, u8 *buf, size_t buf_len) goto out_err; } - tpm_cr50_release_locality(chip, false); return cur; out_err: @@ -523,7 +534,6 @@ out_err: if (tpm_cr50_i2c_tis_status(chip) & TPM_STS_COMMAND_READY) tpm_cr50_i2c_tis_set_ready(chip); - tpm_cr50_release_locality(chip, false); return rc; } @@ -545,10 +555,6 @@ static int tpm_cr50_i2c_tis_send(struct tpm_chip *chip, u8 *buf, size_t len) u32 status; int rc; - rc = tpm_cr50_request_locality(chip); - if (rc < 0) - return rc; - /* Wait until TPM is ready for a command */ stop = jiffies + chip->timeout_b; while (!(tpm_cr50_i2c_tis_status(chip) & TPM_STS_COMMAND_READY)) { @@ -577,7 +583,8 @@ static int tpm_cr50_i2c_tis_send(struct tpm_chip *chip, u8 *buf, size_t len) * that is inserted by tpm_cr50_i2c_write() */ limit = min_t(size_t, burstcnt - 1, len); - rc = tpm_cr50_i2c_write(chip, TPM_I2C_DATA_FIFO(0), &buf[sent], limit); + rc = tpm_cr50_i2c_write(chip, TPM_I2C_DATA_FIFO(chip->locality), + &buf[sent], limit); if (rc < 0) { dev_err(&chip->dev, "Write failed\n"); goto out_err; @@ -598,7 +605,7 @@ static int tpm_cr50_i2c_tis_send(struct tpm_chip *chip, u8 *buf, size_t len) } /* Start the TPM command */ - rc = tpm_cr50_i2c_write(chip, TPM_I2C_STS(0), tpm_go, + rc = tpm_cr50_i2c_write(chip, TPM_I2C_STS(chip->locality), tpm_go, sizeof(tpm_go)); if (rc < 0) { dev_err(&chip->dev, "Start command failed\n"); @@ -611,7 +618,6 @@ out_err: if (tpm_cr50_i2c_tis_status(chip) & TPM_STS_COMMAND_READY) tpm_cr50_i2c_tis_set_ready(chip); - tpm_cr50_release_locality(chip, false); return rc; } @@ -650,6 +656,8 @@ static const struct tpm_class_ops cr50_i2c = { .req_complete_mask = TPM_STS_DATA_AVAIL | TPM_STS_VALID, .req_complete_val = TPM_STS_DATA_AVAIL | TPM_STS_VALID, .req_canceled = &tpm_cr50_i2c_req_canceled, + .request_locality = &tpm_cr50_request_locality, + .relinquish_locality = &tpm_cr50_release_locality, }; #ifdef CONFIG_ACPI @@ -684,6 +692,7 @@ static int tpm_cr50_i2c_probe(struct i2c_client *client) u32 vendor; u8 buf[4]; int rc; + int loc; if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) return -ENODEV; @@ -726,24 +735,30 @@ static int tpm_cr50_i2c_probe(struct i2c_client *client) TPM_CR50_TIMEOUT_NOIRQ_MS); } - rc = tpm_cr50_request_locality(chip); - if (rc < 0) { + loc = tpm_cr50_request_locality(chip, TPM_CR50_I2C_DEFAULT_LOC); + if (loc < 0) { dev_err(dev, "Could not request locality\n"); - return rc; + return loc; } /* Read four bytes from DID_VID register */ - rc = tpm_cr50_i2c_read(chip, TPM_I2C_DID_VID(0), buf, sizeof(buf)); + rc = tpm_cr50_i2c_read(chip, TPM_I2C_DID_VID(loc), buf, sizeof(buf)); if (rc < 0) { dev_err(dev, "Could not read vendor id\n"); - tpm_cr50_release_locality(chip, true); + if (tpm_cr50_release_locality(chip, loc)) + dev_err(dev, "Could not release locality\n"); + return rc; + } + + rc = tpm_cr50_release_locality(chip, loc); + if (rc) { + dev_err(dev, "Could not release locality\n"); return rc; } vendor = le32_to_cpup((__le32 *)buf); if (vendor != TPM_CR50_I2C_DID_VID && vendor != TPM_TI50_I2C_DID_VID) { dev_err(dev, "Vendor ID did not match! ID was %08x\n", vendor); - tpm_cr50_release_locality(chip, true); return -ENODEV; } @@ -772,7 +787,6 @@ static void tpm_cr50_i2c_remove(struct i2c_client *client) } tpm_chip_unregister(chip); - tpm_cr50_release_locality(chip, true); } static SIMPLE_DEV_PM_OPS(cr50_i2c_pm, tpm_pm_suspend, tpm_pm_resume); -- 2.50.1 From 44637b0b40f47629ff78a73744755e6535e0970e Mon Sep 17 00:00:00 2001 From: Jan Dabros Date: Thu, 10 Oct 2024 09:15:59 +0000 Subject: [PATCH 09/16] char: tpm: cr50: Move i2c locking to request/relinquish locality ops Move i2c locking primitives to request_locality and relinquish_locality callbacks, what effectively blocks TPM bus for the whole duration of logical TPM operation. With this in place, cr50-equipped TPM may be shared with external CPUs - assuming that underneath i2c controller driver is aware of this setup (see i2c-designware-amdpsp as an example). Signed-off-by: Jan Dabros Signed-off-by: Jarkko Sakkinen --- drivers/char/tpm/tpm_tis_i2c_cr50.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/drivers/char/tpm/tpm_tis_i2c_cr50.c b/drivers/char/tpm/tpm_tis_i2c_cr50.c index eed1c296a00c..80b0f41ffb5f 100644 --- a/drivers/char/tpm/tpm_tis_i2c_cr50.c +++ b/drivers/char/tpm/tpm_tis_i2c_cr50.c @@ -201,8 +201,6 @@ static int tpm_cr50_i2c_read(struct tpm_chip *chip, u8 addr, u8 *buffer, size_t }; int rc; - i2c_lock_bus(client->adapter, I2C_LOCK_SEGMENT); - /* Prepare for completion interrupt */ tpm_cr50_i2c_enable_tpm_irq(chip); @@ -221,7 +219,6 @@ static int tpm_cr50_i2c_read(struct tpm_chip *chip, u8 addr, u8 *buffer, size_t out: tpm_cr50_i2c_disable_tpm_irq(chip); - i2c_unlock_bus(client->adapter, I2C_LOCK_SEGMENT); if (rc < 0) return rc; @@ -263,8 +260,6 @@ static int tpm_cr50_i2c_write(struct tpm_chip *chip, u8 addr, u8 *buffer, priv->buf[0] = addr; memcpy(priv->buf + 1, buffer, len); - i2c_lock_bus(client->adapter, I2C_LOCK_SEGMENT); - /* Prepare for completion interrupt */ tpm_cr50_i2c_enable_tpm_irq(chip); @@ -278,7 +273,6 @@ static int tpm_cr50_i2c_write(struct tpm_chip *chip, u8 addr, u8 *buffer, out: tpm_cr50_i2c_disable_tpm_irq(chip); - i2c_unlock_bus(client->adapter, I2C_LOCK_SEGMENT); if (rc < 0) return rc; @@ -322,6 +316,7 @@ static int tpm_cr50_check_locality(struct tpm_chip *chip, int loc) */ static int tpm_cr50_release_locality(struct tpm_chip *chip, int loc) { + struct i2c_client *client = to_i2c_client(chip->dev.parent); u8 mask = TPM_ACCESS_VALID | TPM_ACCESS_REQUEST_PENDING; u8 addr = TPM_I2C_ACCESS(loc); u8 buf; @@ -329,13 +324,15 @@ static int tpm_cr50_release_locality(struct tpm_chip *chip, int loc) rc = tpm_cr50_i2c_read(chip, addr, &buf, sizeof(buf)); if (rc < 0) - return rc; + goto unlock_out; if ((buf & mask) == mask) { buf = TPM_ACCESS_ACTIVE_LOCALITY; rc = tpm_cr50_i2c_write(chip, addr, &buf, sizeof(buf)); } +unlock_out: + i2c_unlock_bus(client->adapter, I2C_LOCK_SEGMENT); return rc; } @@ -350,16 +347,19 @@ static int tpm_cr50_release_locality(struct tpm_chip *chip, int loc) */ static int tpm_cr50_request_locality(struct tpm_chip *chip, int loc) { + struct i2c_client *client = to_i2c_client(chip->dev.parent); u8 buf = TPM_ACCESS_REQUEST_USE; unsigned long stop; int rc; + i2c_lock_bus(client->adapter, I2C_LOCK_SEGMENT); + if (tpm_cr50_check_locality(chip, loc) == loc) return loc; rc = tpm_cr50_i2c_write(chip, TPM_I2C_ACCESS(loc), &buf, sizeof(buf)); if (rc < 0) - return rc; + goto unlock_out; stop = jiffies + chip->timeout_a; do { @@ -369,7 +369,11 @@ static int tpm_cr50_request_locality(struct tpm_chip *chip, int loc) msleep(TPM_CR50_TIMEOUT_SHORT_MS); } while (time_before(jiffies, stop)); - return -ETIMEDOUT; + rc = -ETIMEDOUT; + +unlock_out: + i2c_unlock_bus(client->adapter, I2C_LOCK_SEGMENT); + return rc; } /** -- 2.50.1 From 2e1827de1b0f22b420c9446a3c94e6cce8eb4da4 Mon Sep 17 00:00:00 2001 From: Jett Rink Date: Tue, 10 Sep 2024 19:11:14 +0000 Subject: [PATCH 10/16] char: tpm: cr50: Add new device/vendor ID 0x50666666 Accept another DID:VID for the next generation Google TPM. This TPM has the same Ti50 firmware and fulfills the same interface. Suggested-by: Jarkko Sakkinen Signed-off-by: Jett Rink Reviewed-by: Jarkko Sakkinen Signed-off-by: Jarkko Sakkinen --- drivers/char/tpm/tpm_tis_i2c_cr50.c | 32 +++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/drivers/char/tpm/tpm_tis_i2c_cr50.c b/drivers/char/tpm/tpm_tis_i2c_cr50.c index 80b0f41ffb5f..3b55a7b05c46 100644 --- a/drivers/char/tpm/tpm_tis_i2c_cr50.c +++ b/drivers/char/tpm/tpm_tis_i2c_cr50.c @@ -31,8 +31,9 @@ #define TPM_CR50_MAX_BUFSIZE 64 #define TPM_CR50_TIMEOUT_SHORT_MS 2 /* Short timeout during transactions */ #define TPM_CR50_TIMEOUT_NOIRQ_MS 20 /* Timeout for TPM ready without IRQ */ -#define TPM_CR50_I2C_DID_VID 0x00281ae0L /* Device and vendor ID reg value */ -#define TPM_TI50_I2C_DID_VID 0x504a6666L /* Device and vendor ID reg value */ +#define TPM_CR50_I2C_DID_VID 0x00281ae0L /* Device and vendor ID for Cr50 H1 */ +#define TPM_TI50_DT_I2C_DID_VID 0x504a6666L /* Device and vendor ID for Ti50 DT */ +#define TPM_TI50_OT_I2C_DID_VID 0x50666666L /* Device and vendor ID for TI50 OT */ #define TPM_CR50_I2C_MAX_RETRIES 3 /* Max retries due to I2C errors */ #define TPM_CR50_I2C_RETRY_DELAY_LO 55 /* Min usecs between retries on I2C */ #define TPM_CR50_I2C_RETRY_DELAY_HI 65 /* Max usecs between retries on I2C */ @@ -680,6 +681,27 @@ static const struct of_device_id of_cr50_i2c_match[] = { MODULE_DEVICE_TABLE(of, of_cr50_i2c_match); #endif +/** + * tpm_cr50_vid_to_name() - Maps VID to name. + * @vendor: Vendor identifier to map to name + * + * Return: + * A valid string for the vendor or empty string + */ +static const char *tpm_cr50_vid_to_name(u32 vendor) +{ + switch (vendor) { + case TPM_CR50_I2C_DID_VID: + return "cr50"; + case TPM_TI50_DT_I2C_DID_VID: + return "ti50 DT"; + case TPM_TI50_OT_I2C_DID_VID: + return "ti50 OT"; + default: + return "unknown"; + } +} + /** * tpm_cr50_i2c_probe() - Driver probe function. * @client: I2C client information. @@ -761,13 +783,15 @@ static int tpm_cr50_i2c_probe(struct i2c_client *client) } vendor = le32_to_cpup((__le32 *)buf); - if (vendor != TPM_CR50_I2C_DID_VID && vendor != TPM_TI50_I2C_DID_VID) { + if (vendor != TPM_CR50_I2C_DID_VID && + vendor != TPM_TI50_DT_I2C_DID_VID && + vendor != TPM_TI50_OT_I2C_DID_VID) { dev_err(dev, "Vendor ID did not match! ID was %08x\n", vendor); return -ENODEV; } dev_info(dev, "%s TPM 2.0 (i2c 0x%02x irq %d id 0x%x)\n", - vendor == TPM_TI50_I2C_DID_VID ? "ti50" : "cr50", + tpm_cr50_vid_to_name(vendor), client->addr, client->irq, vendor >> 16); return tpm_chip_register(chip); } -- 2.50.1 From 5578b4347bb5d5dfc8eeb8ee2eb8248658707d9b Mon Sep 17 00:00:00 2001 From: "Rob Herring (Arm)" Date: Wed, 6 Nov 2024 20:17:42 +0200 Subject: [PATCH 11/16] tpm: atmel: Drop PPC64 specific MMIO setup The PPC64 specific MMIO setup open codes DT address functions rather than using standard address parsing functions. The open-coded version fails to handle any address translation and is not endian safe. I haven't found any evidence of what platform used this. The only thing that turned up was a PPC405 platform, but that is 32-bit and PPC405 support is being removed as well. CONFIG_TCG_ATMEL is not enabled for any powerpc config and never was. The support was added in 2005 and hasn't been touched since. Rather than try to modernize and fix this code, just remove it. [jarkko: fixed couple of style issues reported by checkpatch.pl --strict and put offset into parentheses in the macro declarations.] Signed-off-by: Rob Herring (Arm) Acked-by: Michael Ellerman Reviewed-by: Jarkko Sakkinen Signed-off-by: Jarkko Sakkinen --- drivers/char/tpm/Kconfig | 2 +- drivers/char/tpm/tpm_atmel.c | 63 +++++++++++++++- drivers/char/tpm/tpm_atmel.h | 140 ----------------------------------- 3 files changed, 61 insertions(+), 144 deletions(-) delete mode 100644 drivers/char/tpm/tpm_atmel.h diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig index cf0be8a7939d..0fc9a510e059 100644 --- a/drivers/char/tpm/Kconfig +++ b/drivers/char/tpm/Kconfig @@ -162,7 +162,7 @@ config TCG_NSC config TCG_ATMEL tristate "Atmel TPM Interface" - depends on PPC64 || HAS_IOPORT_MAP + depends on HAS_IOPORT_MAP depends on HAS_IOPORT help If you have a TPM security chip from Atmel say Yes and it diff --git a/drivers/char/tpm/tpm_atmel.c b/drivers/char/tpm/tpm_atmel.c index 9fb2defa9dc4..54a0360a3c95 100644 --- a/drivers/char/tpm/tpm_atmel.c +++ b/drivers/char/tpm/tpm_atmel.c @@ -15,7 +15,66 @@ */ #include "tpm.h" -#include "tpm_atmel.h" + +struct tpm_atmel_priv { + int region_size; + int have_region; + unsigned long base; + void __iomem *iobase; +}; + +#define atmel_getb(chip, offset) inb(atmel_get_priv(chip)->base + (offset)) +#define atmel_putb(val, chip, offset) \ + outb(val, atmel_get_priv(chip)->base + (offset)) +#define atmel_request_region request_region +#define atmel_release_region release_region +/* Atmel definitions */ +enum tpm_atmel_addr { + TPM_ATMEL_BASE_ADDR_LO = 0x08, + TPM_ATMEL_BASE_ADDR_HI = 0x09 +}; + +static inline int tpm_read_index(int base, int index) +{ + outb(index, base); + return inb(base + 1) & 0xFF; +} + +/* Verify this is a 1.1 Atmel TPM */ +static int atmel_verify_tpm11(void) +{ + /* verify that it is an Atmel part */ + if (tpm_read_index(TPM_ADDR, 4) != 'A' || + tpm_read_index(TPM_ADDR, 5) != 'T' || + tpm_read_index(TPM_ADDR, 6) != 'M' || + tpm_read_index(TPM_ADDR, 7) != 'L') + return 1; + + /* query chip for its version number */ + if (tpm_read_index(TPM_ADDR, 0x00) != 1 || + tpm_read_index(TPM_ADDR, 0x01) != 1) + return 1; + + /* This is an atmel supported part */ + return 0; +} + +/* Determine where to talk to device */ +static void __iomem *atmel_get_base_addr(unsigned long *base, int *region_size) +{ + int lo, hi; + + if (atmel_verify_tpm11() != 0) + return NULL; + + lo = tpm_read_index(TPM_ADDR, TPM_ATMEL_BASE_ADDR_LO); + hi = tpm_read_index(TPM_ADDR, TPM_ATMEL_BASE_ADDR_HI); + + *base = (hi << 8) | lo; + *region_size = 2; + + return ioport_map(*base, *region_size); +} /* write status bits */ enum tpm_atmel_write_status { @@ -142,7 +201,6 @@ static void atml_plat_remove(void) tpm_chip_unregister(chip); if (priv->have_region) atmel_release_region(priv->base, priv->region_size); - atmel_put_base_addr(priv->iobase); platform_device_unregister(pdev); } @@ -211,7 +269,6 @@ static int __init init_atmel(void) err_unreg_dev: platform_device_unregister(pdev); err_rel_reg: - atmel_put_base_addr(iobase); if (have_region) atmel_release_region(base, region_size); diff --git a/drivers/char/tpm/tpm_atmel.h b/drivers/char/tpm/tpm_atmel.h deleted file mode 100644 index 7ac3f69dcf0f..000000000000 --- a/drivers/char/tpm/tpm_atmel.h +++ /dev/null @@ -1,140 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ -/* - * Copyright (C) 2005 IBM Corporation - * - * Authors: - * Kylene Hall - * - * Maintained by: - * - * Device driver for TCG/TCPA TPM (trusted platform module). - * Specifications at www.trustedcomputinggroup.org - * - * These difference are required on power because the device must be - * discovered through the device tree and iomap must be used to get - * around the need for holes in the io_page_mask. This does not happen - * automatically because the tpm is not a normal pci device and lives - * under the root node. - */ - -struct tpm_atmel_priv { - int region_size; - int have_region; - unsigned long base; - void __iomem *iobase; -}; - -#ifdef CONFIG_PPC64 - -#include - -#define atmel_getb(priv, offset) readb(priv->iobase + offset) -#define atmel_putb(val, priv, offset) writeb(val, priv->iobase + offset) -#define atmel_request_region request_mem_region -#define atmel_release_region release_mem_region - -static inline void atmel_put_base_addr(void __iomem *iobase) -{ - iounmap(iobase); -} - -static void __iomem * atmel_get_base_addr(unsigned long *base, int *region_size) -{ - struct device_node *dn; - unsigned long address, size; - const unsigned int *reg; - int reglen; - int naddrc; - int nsizec; - - dn = of_find_node_by_name(NULL, "tpm"); - - if (!dn) - return NULL; - - if (!of_device_is_compatible(dn, "AT97SC3201")) { - of_node_put(dn); - return NULL; - } - - reg = of_get_property(dn, "reg", ®len); - naddrc = of_n_addr_cells(dn); - nsizec = of_n_size_cells(dn); - - of_node_put(dn); - - - if (naddrc == 2) - address = ((unsigned long) reg[0] << 32) | reg[1]; - else - address = reg[0]; - - if (nsizec == 2) - size = - ((unsigned long) reg[naddrc] << 32) | reg[naddrc + 1]; - else - size = reg[naddrc]; - - *base = address; - *region_size = size; - return ioremap(*base, *region_size); -} -#else -#define atmel_getb(chip, offset) inb(atmel_get_priv(chip)->base + offset) -#define atmel_putb(val, chip, offset) \ - outb(val, atmel_get_priv(chip)->base + offset) -#define atmel_request_region request_region -#define atmel_release_region release_region -/* Atmel definitions */ -enum tpm_atmel_addr { - TPM_ATMEL_BASE_ADDR_LO = 0x08, - TPM_ATMEL_BASE_ADDR_HI = 0x09 -}; - -static inline int tpm_read_index(int base, int index) -{ - outb(index, base); - return inb(base+1) & 0xFF; -} - -/* Verify this is a 1.1 Atmel TPM */ -static int atmel_verify_tpm11(void) -{ - - /* verify that it is an Atmel part */ - if (tpm_read_index(TPM_ADDR, 4) != 'A' || - tpm_read_index(TPM_ADDR, 5) != 'T' || - tpm_read_index(TPM_ADDR, 6) != 'M' || - tpm_read_index(TPM_ADDR, 7) != 'L') - return 1; - - /* query chip for its version number */ - if (tpm_read_index(TPM_ADDR, 0x00) != 1 || - tpm_read_index(TPM_ADDR, 0x01) != 1) - return 1; - - /* This is an atmel supported part */ - return 0; -} - -static inline void atmel_put_base_addr(void __iomem *iobase) -{ -} - -/* Determine where to talk to device */ -static void __iomem * atmel_get_base_addr(unsigned long *base, int *region_size) -{ - int lo, hi; - - if (atmel_verify_tpm11() != 0) - return NULL; - - lo = tpm_read_index(TPM_ADDR, TPM_ATMEL_BASE_ADDR_LO); - hi = tpm_read_index(TPM_ADDR, TPM_ATMEL_BASE_ADDR_HI); - - *base = (hi << 8) | lo; - *region_size = 2; - - return ioport_map(*base, *region_size); -} -#endif -- 2.50.1 From ac1f43c03fc91eee53cc95683245350d4d87781e Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Mon, 25 Nov 2024 12:24:46 +0100 Subject: [PATCH 12/16] thermal: gov_power_allocator: Add missing NULL pointer check Commit 0dc23567c206 ("thermal: core: Move lists of thermal instances to trip descriptors") overlooked the case in which the Power Allocator governor attempts to bind to a tripless thermal zone and params->trip_max is NULL in check_power_actors(). No power actors can be found in that case, so check_power_actors() needs to be made return 0 then to restore its previous behavior. Fixes: 0dc23567c206 ("thermal: core: Move lists of thermal instances to trip descriptors") Closes: https://lore.kernel.org/linux-pm/Z0NeGF4ryCe_b5rr@sashalap/ Reported-by: Sasha Levin Signed-off-by: Rafael J. Wysocki Reviewed-by: Lukasz Luba Link: https://patch.msgid.link/2761105.mvXUDI8C0e@rjwysocki.net --- drivers/thermal/gov_power_allocator.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c index ac6fa6b8f99f..3b644de3292e 100644 --- a/drivers/thermal/gov_power_allocator.c +++ b/drivers/thermal/gov_power_allocator.c @@ -588,10 +588,15 @@ static void allow_maximum_power(struct thermal_zone_device *tz) static int check_power_actors(struct thermal_zone_device *tz, struct power_allocator_params *params) { - const struct thermal_trip_desc *td = trip_to_trip_desc(params->trip_max); + const struct thermal_trip_desc *td; struct thermal_instance *instance; int ret = 0; + if (!params->trip_max) + return 0; + + td = trip_to_trip_desc(params->trip_max); + list_for_each_entry(instance, &td->thermal_instances, trip_node) { if (!cdev_is_power_actor(instance->cdev)) { dev_warn(&tz->device, "power_allocator: %s is not a power actor\n", -- 2.50.1 From 69f3aa6ad92447d6e9f50c5b5aea85b56e80b198 Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Thu, 10 Oct 2024 20:06:17 +0200 Subject: [PATCH 13/16] thermal: of: Simplify thermal_of_should_bind with scoped for each OF child Use scoped for_each_child_of_node_scoped() when iterating over device nodes to make code a bit simpler. Reviewed-by: Jonathan Cameron Signed-off-by: Krzysztof Kozlowski Link: https://patch.msgid.link/20241010-b4-cleanup-h-of-node-put-thermal-v4-1-bfbe29ad81f4@linaro.org Signed-off-by: Rafael J. Wysocki --- drivers/thermal/thermal_of.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c index 07e09897165f..e406b9150a2b 100644 --- a/drivers/thermal/thermal_of.c +++ b/drivers/thermal/thermal_of.c @@ -297,7 +297,7 @@ static bool thermal_of_should_bind(struct thermal_zone_device *tz, struct thermal_cooling_device *cdev, struct cooling_spec *c) { - struct device_node *tz_np, *cm_np, *child; + struct device_node *tz_np, *cm_np; bool result = false; tz_np = thermal_of_zone_get_by_name(tz); @@ -311,7 +311,7 @@ static bool thermal_of_should_bind(struct thermal_zone_device *tz, goto out; /* Look up the trip and the cdev in the cooling maps. */ - for_each_child_of_node(cm_np, child) { + for_each_child_of_node_scoped(cm_np, child) { struct device_node *tr_np; int count, i; @@ -330,7 +330,6 @@ static bool thermal_of_should_bind(struct thermal_zone_device *tz, break; } - of_node_put(child); break; } -- 2.50.1 From a094ccfa5277cb9b92040016f3abf22408405820 Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Thu, 10 Oct 2024 20:06:18 +0200 Subject: [PATCH 14/16] thermal: of: Use scoped memory and OF handling to simplify thermal_of_trips_init() Obtain the device node reference and allocate memory with scoped/cleanup.h to reduce error handling and make the code a bit simpler. The code is not equivalent in one minor aspect: outgoing parameter "*ntrips" will not be zeroed on errors of memory allocation. This difference is not important, because code was already not zeroing it in case of earlier errors and the only caller does not rely on ntrips being 0 in case of errors. Signed-off-by: Krzysztof Kozlowski Reviewed-by: Chen-Yu Tsai Reviewed-by: Jonathan Cameron Link: https://patch.msgid.link/20241010-b4-cleanup-h-of-node-put-thermal-v4-2-bfbe29ad81f4@linaro.org [ rjw: Rebase ] Signed-off-by: Rafael J. Wysocki --- drivers/thermal/thermal_of.c | 27 +++++++-------------------- 1 file changed, 7 insertions(+), 20 deletions(-) diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c index e406b9150a2b..9b0a3ec36a90 100644 --- a/drivers/thermal/thermal_of.c +++ b/drivers/thermal/thermal_of.c @@ -95,13 +95,11 @@ static int thermal_of_populate_trip(struct device_node *np, static struct thermal_trip *thermal_of_trips_init(struct device_node *np, int *ntrips) { - struct thermal_trip *tt; - struct device_node *trips; int ret, count; *ntrips = 0; - trips = of_get_child_by_name(np, "trips"); + struct device_node *trips __free(device_node) = of_get_child_by_name(np, "trips"); if (!trips) return NULL; @@ -109,31 +107,20 @@ static struct thermal_trip *thermal_of_trips_init(struct device_node *np, int *n if (!count) return NULL; - tt = kzalloc(sizeof(*tt) * count, GFP_KERNEL); - if (!tt) { - ret = -ENOMEM; - goto out_of_node_put; - } - - *ntrips = count; + struct thermal_trip *tt __free(kfree) = kzalloc(sizeof(*tt) * count, GFP_KERNEL); + if (!tt) + return ERR_PTR(-ENOMEM); count = 0; for_each_child_of_node_scoped(trips, trip) { ret = thermal_of_populate_trip(trip, &tt[count++]); if (ret) - goto out_kfree; + return ERR_PTR(ret); } - of_node_put(trips); - - return tt; - -out_kfree: - kfree(tt); -out_of_node_put: - of_node_put(trips); + *ntrips = count; - return ERR_PTR(ret); + return no_free_ptr(tt); } static struct device_node *of_thermal_zone_find(struct device_node *sensor, int id) -- 2.50.1 From 8309135a39de73af2b4bbaa8385e40ffccbfc42c Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Thu, 10 Oct 2024 20:06:19 +0200 Subject: [PATCH 15/16] thermal: of: Use scoped device node handling to simplify of_thermal_zone_find() Obtain the device node reference with scoped/cleanup.h to reduce error handling and make the code a bit simpler. Reviewed-by: Chen-Yu Tsai Reviewed-by: Jonathan Cameron Signed-off-by: Krzysztof Kozlowski Link: https://patch.msgid.link/20241010-b4-cleanup-h-of-node-put-thermal-v4-3-bfbe29ad81f4@linaro.org Signed-off-by: Rafael J. Wysocki --- drivers/thermal/thermal_of.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c index 9b0a3ec36a90..fab11b98ca49 100644 --- a/drivers/thermal/thermal_of.c +++ b/drivers/thermal/thermal_of.c @@ -125,10 +125,9 @@ static struct thermal_trip *thermal_of_trips_init(struct device_node *np, int *n static struct device_node *of_thermal_zone_find(struct device_node *sensor, int id) { - struct device_node *np, *tz; struct of_phandle_args sensor_specs; - np = of_find_node_by_name(NULL, "thermal-zones"); + struct device_node *np __free(device_node) = of_find_node_by_name(NULL, "thermal-zones"); if (!np) { pr_debug("No thermal zones description\n"); return ERR_PTR(-ENODEV); @@ -146,8 +145,7 @@ static struct device_node *of_thermal_zone_find(struct device_node *sensor, int "#thermal-sensor-cells"); if (count <= 0) { pr_err("%pOFn: missing thermal sensor\n", child); - tz = ERR_PTR(-EINVAL); - goto out; + return ERR_PTR(-EINVAL); } for (i = 0; i < count; i++) { @@ -159,22 +157,18 @@ static struct device_node *of_thermal_zone_find(struct device_node *sensor, int i, &sensor_specs); if (ret < 0) { pr_err("%pOFn: Failed to read thermal-sensors cells: %d\n", child, ret); - tz = ERR_PTR(ret); - goto out; + return ERR_PTR(ret); } if ((sensor == sensor_specs.np) && id == (sensor_specs.args_count ? sensor_specs.args[0] : 0)) { pr_debug("sensor %pOFn id=%d belongs to %pOFn\n", sensor, id, child); - tz = no_free_ptr(child); - goto out; + return no_free_ptr(child); } } } - tz = ERR_PTR(-ENODEV); -out: - of_node_put(np); - return tz; + + return ERR_PTR(-ENODEV); } static int thermal_of_monitor_init(struct device_node *np, int *delay, int *pdelay) -- 2.50.1 From 4dc00afc20ddb676aedc0ab776397c99e4025e30 Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Thu, 10 Oct 2024 20:06:20 +0200 Subject: [PATCH 16/16] thermal: qcom-spmi-adc-tm5: Simplify with scoped for each OF child loop Use scoped for_each_available_child_of_node_scoped() when iterating over device nodes to make code a bit simpler. Reviewed-by: Chen-Yu Tsai Reviewed-by: Jonathan Cameron Signed-off-by: Krzysztof Kozlowski Link: https://patch.msgid.link/20241010-b4-cleanup-h-of-node-put-thermal-v4-4-bfbe29ad81f4@linaro.org Signed-off-by: Rafael J. Wysocki --- drivers/thermal/qcom/qcom-spmi-adc-tm5.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/drivers/thermal/qcom/qcom-spmi-adc-tm5.c b/drivers/thermal/qcom/qcom-spmi-adc-tm5.c index 5e94a45eba3e..d7f2e6ca92c2 100644 --- a/drivers/thermal/qcom/qcom-spmi-adc-tm5.c +++ b/drivers/thermal/qcom/qcom-spmi-adc-tm5.c @@ -938,7 +938,6 @@ static const struct adc_tm5_data adc_tm5_gen2_data_pmic = { static int adc_tm5_get_dt_data(struct adc_tm5_chip *adc_tm, struct device_node *node) { struct adc_tm5_channel *channels; - struct device_node *child; u32 value; int ret; struct device *dev = adc_tm->dev; @@ -982,12 +981,10 @@ static int adc_tm5_get_dt_data(struct adc_tm5_chip *adc_tm, struct device_node * adc_tm->avg_samples = VADC_DEF_AVG_SAMPLES; } - for_each_available_child_of_node(node, child) { + for_each_available_child_of_node_scoped(node, child) { ret = adc_tm5_get_dt_channel_data(adc_tm, channels, child); - if (ret) { - of_node_put(child); + if (ret) return ret; - } channels++; } -- 2.50.1