From 4ecc672c3ee684cff6af414ca16a4609cb95ce15 Mon Sep 17 00:00:00 2001 From: Vedashree Vidwans Date: Mon, 27 Feb 2023 10:56:15 -0800 Subject: [PATCH] tegra: hwpm: clean up code and add bug fixes - Create tegra_hwpm_element_enable() instead of directly using perfmon_enable() HAL. This will allow us to expand tegra_hwpm_element_enable in future. - Update log messages in ip structure init code and floorsweep info function. - It is possible that IP instances and elements to have 0 start range address. So, modify check for available elements to use range end instead. - Use tegra_hwpm_fake_readl() and tegra_hwpm_fake_writel() macros instead of fake_readl() and fake_write() functions. That way we have similar implementation of IO functions and macros can be used across OSes. - Check that reserve perfmon function is invoked only for HWPM components. This check will be useful for expansion in types of components in the future. - Clean up and rearrange tegra_hwpm_regops_readl_impl() and tegra_hwpm_regops_writel_impl() to have designated code corresponding to the element type. - Currently, device open and release functions are incorrectly using clock enable/disable functions instead of using HALs. Correct open and close functions to use lock HALs. - Currently, tegra_hwpm_update_mem_bytes() doesn't validate mem_mgmt structure allocation before accessing mem_bytes_kernel pointer. This can lead to kernel crash. Update tegra_hwpm_update_mem_bytes() to return error if mem_mgmt structure s not allocated. Jira THWPM-74 Change-Id: Ia40bd51187e5ea08572dbee81e577dacf5fb66b6 Signed-off-by: Vedashree Vidwans (cherry picked from commit 411f07484d68dfde0d350a5c67f2748e876b11b8) Reviewed-on: https://git-master.nvidia.com/r/c/linux-hwpm/+/2888553 Reviewed-by: Adeel Raza GVS: Gerrit_Virtual_Submit --- drivers/tegra/hwpm/common/aperture.c | 74 ++++++++++++++---- drivers/tegra/hwpm/include/tegra_hwpm_io.h | 6 ++ drivers/tegra/hwpm/os/linux/aperture_utils.c | 13 +++- drivers/tegra/hwpm/os/linux/io_utils.c | 80 +++++++++++--------- drivers/tegra/hwpm/os/linux/io_utils.h | 4 + drivers/tegra/hwpm/os/linux/ioctl.c | 4 +- drivers/tegra/hwpm/os/linux/ip_utils.c | 6 +- drivers/tegra/hwpm/os/linux/mem_mgmt_utils.c | 7 ++ 8 files changed, 137 insertions(+), 57 deletions(-) diff --git a/drivers/tegra/hwpm/common/aperture.c b/drivers/tegra/hwpm/common/aperture.c index b8522ae..fb836d2 100644 --- a/drivers/tegra/hwpm/common/aperture.c +++ b/drivers/tegra/hwpm/common/aperture.c @@ -67,6 +67,38 @@ fail: return err; } +static int tegra_hwpm_element_enable(struct tegra_soc_hwpm *hwpm, + struct hwpm_ip_aperture *element) +{ + int err = 0; + + tegra_hwpm_fn(hwpm, " "); + + switch (element->element_type) { + case HWPM_ELEMENT_PERFMON: + err = hwpm->active_chip->perfmon_enable(hwpm, element); + if (err != 0) { + tegra_hwpm_err(hwpm, "Element mask 0x%x disable failed", + element->element_index_mask); + goto fail; + } + break; + case HWPM_ELEMENT_PERFMUX: + case IP_ELEMENT_PERFMUX: + case IP_ELEMENT_BROADCAST: + /* Nothing to do here */ + break; + case HWPM_ELEMENT_INVALID: + default: + tegra_hwpm_err(hwpm, "Invalid element type %d", + element->element_type); + return -EINVAL; + } + +fail: + return err; +} + static int tegra_hwpm_element_disable(struct tegra_soc_hwpm *hwpm, struct hwpm_ip_aperture *element) { @@ -175,7 +207,7 @@ static int tegra_hwpm_alloc_dynamic_inst_element_array( tegra_hwpm_fn(hwpm, " "); - if (inst_a_info->range_start == 0ULL) { + if (inst_a_info->range_end == 0ULL) { tegra_hwpm_dbg(hwpm, hwpm_dbg_driver_init, "No a_type = %d elements in IP", a_type); return 0; @@ -199,6 +231,11 @@ static int tegra_hwpm_alloc_dynamic_inst_element_array( inst_a_info->inst_arr[idx] = NULL; } + tegra_hwpm_dbg(hwpm, hwpm_dbg_driver_init, + "IP inst range(0x%llx-0x%llx) a_type = %d inst_slots %d", + inst_a_info->range_start, inst_a_info->range_end, + a_type, inst_a_info->inst_slots); + return 0; } @@ -387,14 +424,13 @@ static int tegra_hwpm_func_single_element(struct tegra_soc_hwpm *hwpm, ip_idx, a_type, static_aperture_idx); goto fail; } - if (element->element_type == HWPM_ELEMENT_PERFMON) { - err = hwpm->active_chip->perfmon_enable(hwpm, element); - if (err != 0) { - tegra_hwpm_err(hwpm, "IP %d element" - " type %d idx %d enable failed", - ip_idx, a_type, static_aperture_idx); - goto fail; - } + + err = tegra_hwpm_element_enable(hwpm, element); + if (err != 0) { + tegra_hwpm_err(hwpm, "IP %d element" + " type %d idx %d enable failed", + ip_idx, a_type, static_aperture_idx); + goto fail; } break; case TEGRA_HWPM_UNBIND_RESOURCES: @@ -465,8 +501,8 @@ static int tegra_hwpm_func_all_elements_of_type(struct tegra_soc_hwpm *hwpm, if (e_info->num_element_per_inst == 0U) { /* no a_type elements in this IP */ tegra_hwpm_dbg(hwpm, hwpm_dbg_driver_init, - "No a_type = %d elements in IP %d", - a_type, ip_idx); + "No a_type = %d elements in IP %d stat inst %d", + a_type, ip_idx, static_inst_idx); return 0; } @@ -488,6 +524,14 @@ static int tegra_hwpm_func_all_elements_of_type(struct tegra_soc_hwpm *hwpm, for (idx = 0U; idx < e_info->element_slots; idx++) { e_info->element_arr[idx] = NULL; } + + tegra_hwpm_dbg(hwpm, hwpm_dbg_driver_init, + "iia_func %d IP %d static inst %d a_type %d" + " element range(0x%llx-0x%llx) element_slots %d " + "num_element_per_inst %d", + iia_func, ip_idx, static_inst_idx, a_type, + e_info->range_start, e_info->range_end, + e_info->element_slots, e_info->num_element_per_inst); } if (iia_func == TEGRA_HWPM_UPDATE_IP_INST_MASK) { @@ -569,7 +613,7 @@ static int tegra_hwpm_func_single_inst(struct tegra_soc_hwpm *hwpm, inst_a_info = &chip_ip->inst_aperture_info[a_type]; e_info = &ip_inst->element_info[a_type]; - if (inst_a_info->range_start == 0ULL) { + if (inst_a_info->range_end == 0ULL) { tegra_hwpm_dbg(hwpm, hwpm_dbg_driver_init, "No a_type = %d elements in IP %d", a_type, ip_idx); @@ -585,9 +629,11 @@ static int tegra_hwpm_func_single_inst(struct tegra_soc_hwpm *hwpm, inst_offset / inst_a_info->inst_stride); tegra_hwpm_dbg(hwpm, hwpm_dbg_driver_init, - "IP %d a_type %d " + "IP %d a_type %d inst range start 0x%llx" + "element range start 0x%llx" " static inst idx %d == dynamic idx %d", - ip_idx, a_type, static_inst_idx, idx); + ip_idx, a_type, inst_a_info->range_start, + e_info->range_start, static_inst_idx, idx); /* Set perfmux slot pointer */ inst_a_info->inst_arr[idx] = ip_inst; diff --git a/drivers/tegra/hwpm/include/tegra_hwpm_io.h b/drivers/tegra/hwpm/include/tegra_hwpm_io.h index 8096b9f..4c6084d 100644 --- a/drivers/tegra/hwpm/include/tegra_hwpm_io.h +++ b/drivers/tegra/hwpm/include/tegra_hwpm_io.h @@ -68,6 +68,12 @@ static inline u32 get_field(u32 input_data, u32 mask) #define tegra_hwpm_read_sticky_bits(hwpm, reg_base, reg_offset, val) \ tegra_hwpm_read_sticky_bits_impl(hwpm, reg_base, reg_offset, val) +#define tegra_hwpm_fake_readl(hwpm, aperture, addr, val) \ + tegra_hwpm_fake_readl_impl(hwpm, aperture, addr, val) + +#define tegra_hwpm_fake_writel(hwpm, aperture, addr, val) \ + tegra_hwpm_fake_writel_impl(hwpm, aperture, addr, val) + #define tegra_hwpm_readl(hwpm, aperture, addr, val) \ tegra_hwpm_readl_impl(hwpm, aperture, addr, val) diff --git a/drivers/tegra/hwpm/os/linux/aperture_utils.c b/drivers/tegra/hwpm/os/linux/aperture_utils.c index 67c5ef4..7d4f4f6 100644 --- a/drivers/tegra/hwpm/os/linux/aperture_utils.c +++ b/drivers/tegra/hwpm/os/linux/aperture_utils.c @@ -40,9 +40,16 @@ int tegra_hwpm_perfmon_reserve_impl(struct tegra_soc_hwpm *hwpm, } /* Reserve */ - /* Make sure that resource exists in device node */ - res = platform_get_resource(hwpm_linux->pdev, - IORESOURCE_MEM, perfmon->device_index); + if ((perfmon->element_type == HWPM_ELEMENT_PERFMON) || + (perfmon->element_type == HWPM_ELEMENT_PERFMUX)) { + /* Make sure that resource exists in device node */ + res = platform_get_resource(hwpm_linux->pdev, + IORESOURCE_MEM, perfmon->device_index); + } else { + tegra_hwpm_err(hwpm, + "Unknown perfmon type, execution shouldn't reach here"); + return -EINVAL; + } if ((!res) || (res->start == 0) || (res->end == 0)) { tegra_hwpm_err(hwpm, "Failed to get perfmon %s", perfmon->name); return -ENOMEM; diff --git a/drivers/tegra/hwpm/os/linux/io_utils.c b/drivers/tegra/hwpm/os/linux/io_utils.c index 2bbddb1..fd4dfcb 100644 --- a/drivers/tegra/hwpm/os/linux/io_utils.c +++ b/drivers/tegra/hwpm/os/linux/io_utils.c @@ -44,7 +44,7 @@ int tegra_hwpm_read_sticky_bits_impl(struct tegra_soc_hwpm *hwpm, return 0; } -static int fake_readl(struct tegra_soc_hwpm *hwpm, +int tegra_hwpm_fake_readl_impl(struct tegra_soc_hwpm *hwpm, struct hwpm_ip_aperture *aperture, u64 offset, u32 *val) { if (!hwpm->fake_registers_enabled) { @@ -56,7 +56,7 @@ static int fake_readl(struct tegra_soc_hwpm *hwpm, return 0; } -static int fake_writel(struct tegra_soc_hwpm *hwpm, +int tegra_hwpm_fake_writel_impl(struct tegra_soc_hwpm *hwpm, struct hwpm_ip_aperture *aperture, u64 offset, u32 val) { if (!hwpm->fake_registers_enabled) { @@ -80,7 +80,7 @@ static int ip_readl(struct tegra_soc_hwpm *hwpm, struct hwpm_ip_inst *ip_inst, aperture->start_abs_pa, aperture->end_abs_pa, offset); if (hwpm->fake_registers_enabled) { - return fake_readl(hwpm, aperture, offset, val); + return tegra_hwpm_fake_readl(hwpm, aperture, offset, val); } else { struct tegra_hwpm_ip_ops *ip_ops_ptr = &ip_inst->ip_ops; if (ip_ops_ptr->hwpm_ip_reg_op != NULL) { @@ -128,7 +128,7 @@ static int ip_writel(struct tegra_soc_hwpm *hwpm, struct hwpm_ip_inst *ip_inst, aperture->start_abs_pa, aperture->end_abs_pa, offset, val); if (hwpm->fake_registers_enabled) { - return fake_writel(hwpm, aperture, offset, val); + return tegra_hwpm_fake_writel(hwpm, aperture, offset, val); } else { struct tegra_hwpm_ip_ops *ip_ops_ptr = &ip_inst->ip_ops; if (ip_ops_ptr->hwpm_ip_reg_op != NULL) { @@ -176,7 +176,7 @@ static int hwpm_readl(struct tegra_soc_hwpm *hwpm, aperture->start_abs_pa, aperture->end_abs_pa, offset); if (hwpm->fake_registers_enabled) { - return fake_readl(hwpm, aperture, offset, val); + return tegra_hwpm_fake_readl(hwpm, aperture, offset, val); } else { if (aperture->dt_mmio == NULL) { tegra_hwpm_err(hwpm, @@ -197,14 +197,12 @@ static int hwpm_readl(struct tegra_soc_hwpm *hwpm, static int hwpm_writel(struct tegra_soc_hwpm *hwpm, struct hwpm_ip_aperture *aperture, u64 offset, u32 val) { - int err = 0; - tegra_hwpm_dbg(hwpm, hwpm_register, "Aperture (0x%llx-0x%llx) offset(0x%llx) val(0x%x)", aperture->start_abs_pa, aperture->end_abs_pa, offset, val); if (hwpm->fake_registers_enabled) { - err = fake_writel(hwpm, aperture, offset, val); + return tegra_hwpm_fake_writel(hwpm, aperture, offset, val); } else { if (aperture->dt_mmio == NULL) { tegra_hwpm_err(hwpm, @@ -215,7 +213,7 @@ static int hwpm_writel(struct tegra_soc_hwpm *hwpm, writel(val, aperture->dt_mmio + offset); } - return err; + return 0; } /* @@ -290,9 +288,6 @@ int tegra_hwpm_regops_readl_impl(struct tegra_soc_hwpm *hwpm, struct hwpm_ip_inst *ip_inst, struct hwpm_ip_aperture *aperture, u64 addr, u32 *val) { - u64 reg_offset = 0ULL; - int err = 0; - tegra_hwpm_fn(hwpm, " "); if (!aperture) { @@ -300,20 +295,29 @@ int tegra_hwpm_regops_readl_impl(struct tegra_soc_hwpm *hwpm, return -ENODEV; } - /* - * Register address passed to this function always belong to - * virtual address range of the aperture. - * Hence, subtract start_abs_pa from given addr for offset. - */ - reg_offset = tegra_hwpm_safe_sub_u64(addr, aperture->start_abs_pa); - if ((aperture->element_type == HWPM_ELEMENT_PERFMON) || (aperture->element_type == HWPM_ELEMENT_PERFMUX)) { - err = hwpm_readl(hwpm, aperture, reg_offset, val); + /* + * Register address passed to this function always belong to + * virtual address range of the aperture. + * Hence, subtract start_abs_pa from given addr for offset. + */ + u64 reg_offset = tegra_hwpm_safe_sub_u64(addr, + aperture->start_abs_pa); + + return hwpm_readl(hwpm, aperture, reg_offset, val); } else { - err = ip_readl(hwpm, ip_inst, aperture, reg_offset, val); + /* + * Register address passed to this function always belong to + * virtual address range of the aperture. + * Hence, subtract start_abs_pa from given addr for offset. + */ + u64 reg_offset = tegra_hwpm_safe_sub_u64(addr, + aperture->start_abs_pa); + + return ip_readl(hwpm, ip_inst, aperture, reg_offset, val); } - return err; + return 0; } /* @@ -324,9 +328,6 @@ int tegra_hwpm_regops_writel_impl(struct tegra_soc_hwpm *hwpm, struct hwpm_ip_inst *ip_inst, struct hwpm_ip_aperture *aperture, u64 addr, u32 val) { - u64 reg_offset = 0ULL; - int err = 0; - tegra_hwpm_fn(hwpm, " "); if (!aperture) { @@ -334,18 +335,27 @@ int tegra_hwpm_regops_writel_impl(struct tegra_soc_hwpm *hwpm, return -ENODEV; } - /* - * Register address passed to this function always belong to - * virtual address range of the aperture. - * Hence, subtract start_abs_pa from given addr for offset. - */ - reg_offset = tegra_hwpm_safe_sub_u64(addr, aperture->start_abs_pa); - if ((aperture->element_type == HWPM_ELEMENT_PERFMON) || (aperture->element_type == HWPM_ELEMENT_PERFMUX)) { - err = hwpm_writel(hwpm, aperture, reg_offset, val); + /* + * Register address passed to this function always belong to + * virtual address range of the aperture. + * Hence, subtract start_abs_pa from given addr for offset. + */ + u64 reg_offset = tegra_hwpm_safe_sub_u64(addr, + aperture->start_abs_pa); + + return hwpm_writel(hwpm, aperture, reg_offset, val); } else { - err = ip_writel(hwpm, ip_inst, aperture, reg_offset, val); + /* + * Register address passed to this function always belong to + * virtual address range of the aperture. + * Hence, subtract start_abs_pa from given addr for offset. + */ + u64 reg_offset = tegra_hwpm_safe_sub_u64(addr, + aperture->start_abs_pa); + + return ip_writel(hwpm, ip_inst, aperture, reg_offset, val); } - return err; + return 0; } diff --git a/drivers/tegra/hwpm/os/linux/io_utils.h b/drivers/tegra/hwpm/os/linux/io_utils.h index f6979b8..cce342b 100644 --- a/drivers/tegra/hwpm/os/linux/io_utils.h +++ b/drivers/tegra/hwpm/os/linux/io_utils.h @@ -23,6 +23,10 @@ struct hwpm_ip_aperture; int tegra_hwpm_read_sticky_bits_impl(struct tegra_soc_hwpm *hwpm, u64 reg_base, u64 reg_offset, u32 *val); +int tegra_hwpm_fake_readl_impl(struct tegra_soc_hwpm *hwpm, + struct hwpm_ip_aperture *aperture, u64 offset, u32 *val); +int tegra_hwpm_fake_writel_impl(struct tegra_soc_hwpm *hwpm, + struct hwpm_ip_aperture *aperture, u64 offset, u32 val); int tegra_hwpm_readl_impl(struct tegra_soc_hwpm *hwpm, struct hwpm_ip_aperture *aperture, u64 addr, u32 *val); int tegra_hwpm_writel_impl(struct tegra_soc_hwpm *hwpm, diff --git a/drivers/tegra/hwpm/os/linux/ioctl.c b/drivers/tegra/hwpm/os/linux/ioctl.c index 3d2f5fb..bff0cd2 100644 --- a/drivers/tegra/hwpm/os/linux/ioctl.c +++ b/drivers/tegra/hwpm/os/linux/ioctl.c @@ -405,7 +405,7 @@ static int tegra_hwpm_open(struct inode *inode, struct file *filp) } if (hwpm->active_chip->clk_rst_set_rate_enable) { - ret = tegra_hwpm_clk_rst_set_rate_enable(hwpm_linux); + ret = hwpm->active_chip->clk_rst_set_rate_enable(hwpm_linux); if (ret != 0) { goto fail; } @@ -514,7 +514,7 @@ static int tegra_hwpm_release(struct inode *inode, struct file *filp) } if (hwpm->active_chip->clk_rst_disable) { - ret = tegra_hwpm_clk_rst_disable(hwpm_linux); + ret = hwpm->active_chip->clk_rst_disable(hwpm_linux); if (ret != 0) { tegra_hwpm_err(hwpm, "Failed to release clock"); err = ret; diff --git a/drivers/tegra/hwpm/os/linux/ip_utils.c b/drivers/tegra/hwpm/os/linux/ip_utils.c index 14311db..1d1b8a7 100644 --- a/drivers/tegra/hwpm/os/linux/ip_utils.c +++ b/drivers/tegra/hwpm/os/linux/ip_utils.c @@ -114,9 +114,9 @@ int tegra_hwpm_obtain_floorsweep_info(struct tegra_soc_hwpm *hwpm, hwpm, fs_info->ip_fsinfo[i].ip), &fs_info->ip_fsinfo[i].ip_inst_mask, &fs_info->ip_fsinfo[i].status); - if (ret < 0) { - /* Print error for debug purpose. */ - tegra_hwpm_err(hwpm, "Failed to get fs_info"); + if (ret != 0) { + tegra_hwpm_err(hwpm, + "Failed to get fs_info query %d", i); } tegra_hwpm_dbg(hwpm, hwpm_info | hwpm_dbg_floorsweep_info, diff --git a/drivers/tegra/hwpm/os/linux/mem_mgmt_utils.c b/drivers/tegra/hwpm/os/linux/mem_mgmt_utils.c index 3c66098..aa88356 100644 --- a/drivers/tegra/hwpm/os/linux/mem_mgmt_utils.c +++ b/drivers/tegra/hwpm/os/linux/mem_mgmt_utils.c @@ -366,6 +366,13 @@ int tegra_hwpm_update_mem_bytes(struct tegra_soc_hwpm *hwpm, tegra_hwpm_fn(hwpm, " "); + if (hwpm->mem_mgmt == NULL) { + /* Memory buffer was not initialized */ + tegra_hwpm_dbg(hwpm, hwpm_dbg_alloc_pma_stream, + "mem_mgmt struct was uninitialized in this sesion"); + return -ENXIO; + } + if (!hwpm->mem_mgmt->mem_bytes_kernel) { tegra_hwpm_err(hwpm, "mem_bytes buffer is not mapped in the driver");