The PWM Hi-Res allow configuring the PWM resolution from 8 bits PWM
values up to 15 bits values. The current implementation loops through
all possible resolutions (PWM sizes) on top of the already existing
process of determining the prediv, exponent and refclk.
The first issue is that the maximum value used for capping is wrongly
hardcoded.
The second issue is that it uses the wrong maximum possible PWM
value for determining the best matched period.
Fix both.
Signed-off-by: Abel Vesa <abel.vesa(a)linaro.org>
---
Changes in v2:
- Re-worded the commit to drop the details that are not important
w.r.t. what the patch is fixing.
- Added another patch which fixes the resolution used for determining
best matched period and PWM config.
- Link to v1: https://lore.kernel.org/r/20250220-leds-qcom-lpg-fix-max-pwm-on-hi-res-v1-1…
---
Abel Vesa (2):
leds: rgb: leds-qcom-lpg: Fix pwm resolution max for Hi-Res PWMs
leds: rgb: leds-qcom-lpg: Fix calculation of best period Hi-Res PWMs
drivers/leds/rgb/leds-qcom-lpg.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
---
base-commit: 8433c776e1eb1371f5cd40b5fd3a61f9c7b7f3ad
change-id: 20250220-leds-qcom-lpg-fix-max-pwm-on-hi-res-067e8782a79b
Best regards,
--
Abel Vesa <abel.vesa(a)linaro.org>
Hi,
On 24-10-08, Oreoluwa Babatunde wrote:
> Reserved memory regions defined in the devicetree can be broken up into
> two groups:
> i) Statically-placed reserved memory regions
> i.e. regions defined with a static start address and size using the
> "reg" property.
> ii) Dynamically-placed reserved memory regions.
> i.e. regions defined by specifying an address range where they can be
> placed in memory using the "alloc_ranges" and "size" properties.
>
> These regions are processed and set aside at boot time.
> This is done in two stages as seen below:
>
> Stage 1:
> At this stage, fdt_scan_reserved_mem() scans through the child nodes of
> the reserved_memory node using the flattened devicetree and does the
> following:
>
> 1) If the node represents a statically-placed reserved memory region,
> i.e. if it is defined using the "reg" property:
> - Call memblock_reserve() or memblock_mark_nomap() as needed.
> - Add the information for that region into the reserved_mem array
> using fdt_reserved_mem_save_node().
> i.e. fdt_reserved_mem_save_node(node, name, base, size).
>
> 2) If the node represents a dynamically-placed reserved memory region,
> i.e. if it is defined using "alloc-ranges" and "size" properties:
> - Add the information for that region to the reserved_mem array with
> the starting address and size set to 0.
> i.e. fdt_reserved_mem_save_node(node, name, 0, 0).
> Note: This region is saved to the array with a starting address of 0
> because a starting address is not yet allocated for it.
>
> Stage 2:
> After iterating through all the reserved memory nodes and storing their
> relevant information in the reserved_mem array,fdt_init_reserved_mem() is
> called and does the following:
>
> 1) For statically-placed reserved memory regions:
> - Call the region specific init function using
> __reserved_mem_init_node().
> 2) For dynamically-placed reserved memory regions:
> - Call __reserved_mem_alloc_size() which is used to allocate memory
> for each of these regions, and mark them as nomap if they have the
> nomap property specified in the DT.
> - Call the region specific init function.
>
> The current size of the resvered_mem array is 64 as is defined by
> MAX_RESERVED_REGIONS. This means that there is a limitation of 64 for
> how many reserved memory regions can be specified on a system.
> As systems continue to grow more and more complex, the number of
> reserved memory regions needed are also growing and are starting to hit
> this 64 count limit, hence the need to make the reserved_mem array
> dynamically sized (i.e. dynamically allocating memory for the
> reserved_mem array using membock_alloc_*).
>
> On architectures such as arm64, memory allocated using memblock is
> writable only after the page tables have been setup. This means that if
> the reserved_mem array is going to be dynamically allocated, it needs to
> happen after the page tables have been setup, not before.
>
> Since the reserved memory regions are currently being processed and
> added to the array before the page tables are setup, there is a need to
> change the order in which some of the processing is done to allow for
> the reserved_mem array to be dynamically sized.
>
> It is possible to process the statically-placed reserved memory regions
> without needing to store them in the reserved_mem array until after the
> page tables have been setup because all the information stored in the
> array is readily available in the devicetree and can be referenced at
> any time.
> Dynamically-placed reserved memory regions on the other hand get
> assigned a start address only at runtime, and hence need a place to be
> stored once they are allocated since there is no other referrence to the
> start address for these regions.
>
> Hence this patch changes the processing order of the reserved memory
> regions in the following ways:
>
> Step 1:
> fdt_scan_reserved_mem() scans through the child nodes of
> the reserved_memory node using the flattened devicetree and does the
> following:
>
> 1) If the node represents a statically-placed reserved memory region,
> i.e. if it is defined using the "reg" property:
> - Call memblock_reserve() or memblock_mark_nomap() as needed.
>
> 2) If the node represents a dynamically-placed reserved memory region,
> i.e. if it is defined using "alloc-ranges" and "size" properties:
> - Call __reserved_mem_alloc_size() which will:
> i) Allocate memory for the reserved region and call
> memblock_mark_nomap() as needed.
> ii) Call the region specific initialization function using
> fdt_init_reserved_mem_node().
> iii) Save the region information in the reserved_mem array using
> fdt_reserved_mem_save_node().
>
> Step 2:
> 1) This stage of the reserved memory processing is now only used to add
> the statically-placed reserved memory regions into the reserved_mem
> array using fdt_scan_reserved_mem_reg_nodes(), as well as call their
> region specific initialization functions.
>
> 2) This step has also been moved to be after the page tables are
> setup. Moving this will allow us to replace the reserved_mem
> array with a dynamically sized array before storing the rest of
> these regions.
>
> Signed-off-by: Oreoluwa Babatunde <quic_obabatun(a)quicinc.com>
> ---
> drivers/of/fdt.c | 5 +-
> drivers/of/of_private.h | 3 +-
> drivers/of/of_reserved_mem.c | 168 ++++++++++++++++++++++++-----------
> 3 files changed, 122 insertions(+), 54 deletions(-)
this patch got into stable kernel 6.12.13++ as part of Stable-dep-of.
The stable kernel commit is: 9a0fe62f93ede02c27aaca81112af1e59c8c0979.
With the patch applied I see that the cma area pool is misplaced which
cause my 4G device to fail to activate the cma pool. Below are some
logs:
*** Good case (6.12)
root@test:~# dmesg|grep -i cma
[ 0.000000] OF: reserved mem: initialized node linux,cma, compatible id shared-dma-pool
[ 0.000000] OF: reserved mem: 0x0000000044200000..0x00000000541fffff (262144 KiB) map reusable linux,cma
[ 0.056915] Memory: 3695024K/4194304K available (15552K kernel code, 2510K rwdata, 5992K rodata, 6016K init, 489K bss, 231772K reserved, 262144K cma-reserved)
*** Bad (6.12.16)
root@test:~# dmesg|grep -i cma
[ 0.000000] Reserved memory: created CMA memory pool at 0x00000000f2000000, size 256 MiB
[ 0.000000] OF: reserved mem: initialized node linux,cma, compatible id shared-dma-pool
[ 0.000000] OF: reserved mem: 0x00000000f2000000..0x0000000101ffffff (262144 KiB) map reusable linux,cma
[ 0.056968] Memory: 3694896K/4194304K available (15616K kernel code, 2512K rwdata, 6012K rodata, 6080K init, 491K bss, 231900K reserved, 262144K cma-reserved)
[ 0.116920] cma: CMA area linux,cma could not be activated
*** Good (6.12.16, revert 9a0fe62f93ed)
root@test:~# dmesg|grep -i cma
[ 0.000000] OF: reserved mem: initialized node linux,cma, compatible id shared-dma-pool
[ 0.000000] OF: reserved mem: 0x0000000044200000..0x00000000541fffff (262144 KiB) map reusable linux,cma
[ 0.060976] Memory: 3694896K/4194304K available (15616K kernel code, 2512K rwdata, 6012K rodata, 6080K init, 491K bss, 231900K reserved, 262144K cma-reserved)
Below is our reserved-memory dts node:
reserved-memory {
#address-cells = <2>;
#size-cells = <2>;
ranges;
linux,cma {
compatible = "shared-dma-pool";
reusable;
/*
* The CMA area must be in the lower 32-bit address range.
*/
alloc-ranges = <0x0 0x42000000 0 0xc0000000>;
size = <0x0 0x10000000>;
alignment = <0 0x2000>;
linux,cma-default;
};
optee-core@40000000 {
reg = <0 0x40000000 0 0x1e00000>;
no-map;
};
optee-shm@41e00000 {
reg = <0 0x41e00000 0 0x200000>;
no-map;
};
m7_reserved: m7@80000000 {
reg = <0 0x80000000 0 0x1000000>;
no-map;
};
vdev0vring0: vdev0vring0@55000000 {
reg = <0 0x55000000 0 0x8000>;
no-map;
};
vdev0vring1: vdev0vring1@55008000 {
reg = <0 0x55008000 0 0x8000>;
no-map;
};
rsc_table: rsc-table@550ff000 {
reg = <0 0x550ff000 0 0x1000>;
no-map;
};
ram_console_buffer: ram-console-buffer@55100000 {
reg = <0 0x55100000 0 0x1000>;
no-map;
};
vdev0buffer: vdev0buffer@55400000 {
compatible = "shared-dma-pool";
reg = <0 0x55400000 0 0x100000>;
no-map;
};
};
My current workaround is to revert commit 9a0fe62f93ed and the
dep-chain: 2d1d620ff27b444 8de4e5a92282. But I would like to get a
proper solution without having revert commits in my downstream
patchstack.
Regards,
Marco
Currently, for the high resolution PWMs, the resolution, clock,
pre-divider and exponent are being selected based on period. Basically,
the implementation loops over each one of these and tries to find the
closest (higher) period based on the following formula:
period * refclk
prediv_exp = log2 -------------------------------------
NSEC_PER_SEC * pre_div * resolution
Since the resolution is power of 2, the actual period resulting is
usually higher than what the resolution allows. That's why the duty
cycle requested needs to be capped to the maximum value allowed by the
resolution (known as PWM size).
Here is an example of how this can happen:
For a requested period of 5000000, the best clock is 19.2MHz, the best
prediv is 5, the best exponent is 6 and the best resolution is 256.
Then, the pwm value is determined based on requested period and duty
cycle, best prediv, best exponent and best clock, using the following
formula:
duty * refclk
pwm_value = ----------------------------------------------
NSEC_PER_SEC * prediv * (1 << prediv_exp)
So in this specific scenario:
(5000000 * 19200000) / (1000000000 * 5 * (1 << 64)) = 300
With a resolution of 8 bits, this pwm value obviously goes over.
Therefore, the max pwm value allowed needs to be 255.
If not, the PMIC internal logic will only value that is under the set PWM
size, resulting in a wrapped around PWM value.
This has been observed on Lenovo Thinkpad T14s Gen6 (LCD panel version)
which uses one of the PMK8550 to control the LCD backlight.
Fix the value of the PWM by capping to a max based on the chosen
resolution (PWM size).
Cc: stable(a)vger.kernel.org # 6.4
Fixes: b00d2ed37617 ("leds: rgb: leds-qcom-lpg: Add support for high resolution PWM")
Signed-off-by: Abel Vesa <abel.vesa(a)linaro.org>
---
Note: This fix is blocking backlight support on Lenovo Thinkpad T14s
Gen6 (LCD version), for which I have patches ready to send once this
patch is agreed on (review) and merged.
---
drivers/leds/rgb/leds-qcom-lpg.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/leds/rgb/leds-qcom-lpg.c b/drivers/leds/rgb/leds-qcom-lpg.c
index f3c9ef2bfa572f9ee86c8b8aa37deb8231965490..146cd9b447787bf170310321e939022dfb176e9f 100644
--- a/drivers/leds/rgb/leds-qcom-lpg.c
+++ b/drivers/leds/rgb/leds-qcom-lpg.c
@@ -529,7 +529,7 @@ static void lpg_calc_duty(struct lpg_channel *chan, uint64_t duty)
unsigned int clk_rate;
if (chan->subtype == LPG_SUBTYPE_HI_RES_PWM) {
- max = LPG_RESOLUTION_15BIT - 1;
+ max = BIT(lpg_pwm_resolution_hi_res[chan->pwm_resolution_sel]) - 1;
clk_rate = lpg_clk_rates_hi_res[chan->clk_sel];
} else {
max = LPG_RESOLUTION_9BIT - 1;
---
base-commit: 50a0c754714aa3ea0b0e62f3765eb666a1579f24
change-id: 20250220-leds-qcom-lpg-fix-max-pwm-on-hi-res-067e8782a79b
Best regards,
--
Abel Vesa <abel.vesa(a)linaro.org>
From: Steven Rostedt <rostedt(a)goodmis.org>
The following commands causes a crash:
~# cd /sys/kernel/tracing/events/rcu/rcu_callback
~# echo 'hist:name=bad:keys=common_pid:onmax(bogus).save(common_pid)' > trigger
bash: echo: write error: Invalid argument
~# echo 'hist:name=bad:keys=common_pid' > trigger
Because the following occurs:
event_trigger_write() {
trigger_process_regex() {
event_hist_trigger_parse() {
data = event_trigger_alloc(..);
event_trigger_register(.., data) {
cmd_ops->reg(.., data, ..) [hist_register_trigger()] {
data->ops->init() [event_hist_trigger_init()] {
save_named_trigger(name, data) {
list_add(&data->named_list, &named_triggers);
}
}
}
}
ret = create_actions(); (return -EINVAL)
if (ret)
goto out_unreg;
[..]
ret = hist_trigger_enable(data, ...) {
list_add_tail_rcu(&data->list, &file->triggers); <<<---- SKIPPED!!! (this is important!)
[..]
out_unreg:
event_hist_unregister(.., data) {
cmd_ops->unreg(.., data, ..) [hist_unregister_trigger()] {
list_for_each_entry(iter, &file->triggers, list) {
if (!hist_trigger_match(data, iter, named_data, false)) <- never matches
continue;
[..]
test = iter;
}
if (test && test->ops->free) <<<-- test is NULL
test->ops->free(test) [event_hist_trigger_free()] {
[..]
if (data->name)
del_named_trigger(data) {
list_del(&data->named_list); <<<<-- NEVER gets removed!
}
}
}
}
[..]
kfree(data); <<<-- frees item but it is still on list
The next time a hist with name is registered, it causes an u-a-f bug and
the kernel can crash.
Move the code around such that if event_trigger_register() succeeds, the
next thing called is hist_trigger_enable() which adds it to the list.
A bunch of actions is called if get_named_trigger_data() returns false.
But that doesn't need to be called after event_trigger_register(), so it
can be moved up, allowing event_trigger_register() to be called just
before hist_trigger_enable() keeping them together and allowing the
file->triggers to be properly populated.
Cc: stable(a)vger.kernel.org
Cc: Masami Hiramatsu <mhiramat(a)kernel.org>
Cc: Mathieu Desnoyers <mathieu.desnoyers(a)efficios.com>
Link: https://lore.kernel.org/20250227163944.1c37f85f@gandalf.local.home
Fixes: 067fe038e70f6 ("tracing: Add variable reference handling to hist triggers")
Reported-by: Tomas Glozar <tglozar(a)redhat.com>
Tested-by: Tomas Glozar <tglozar(a)redhat.com>
Reviewed-by: Tom Zanussi <zanussi(a)kernel.org>
Closes: https://lore.kernel.org/all/CAP4=nvTsxjckSBTz=Oe_UYh8keD9_sZC4i++4h72mJLic4…
Signed-off-by: Steven Rostedt (Google) <rostedt(a)goodmis.org>
---
kernel/trace/trace_events_hist.c | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 261163b00137..ad7419e24055 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -6724,27 +6724,27 @@ static int event_hist_trigger_parse(struct event_command *cmd_ops,
if (existing_hist_update_only(glob, trigger_data, file))
goto out_free;
- ret = event_trigger_register(cmd_ops, file, glob, trigger_data);
- if (ret < 0)
- goto out_free;
+ if (!get_named_trigger_data(trigger_data)) {
- if (get_named_trigger_data(trigger_data))
- goto enable;
+ ret = create_actions(hist_data);
+ if (ret)
+ goto out_free;
- ret = create_actions(hist_data);
- if (ret)
- goto out_unreg;
+ if (has_hist_vars(hist_data) || hist_data->n_var_refs) {
+ ret = save_hist_vars(hist_data);
+ if (ret)
+ goto out_free;
+ }
- if (has_hist_vars(hist_data) || hist_data->n_var_refs) {
- ret = save_hist_vars(hist_data);
+ ret = tracing_map_init(hist_data->map);
if (ret)
- goto out_unreg;
+ goto out_free;
}
- ret = tracing_map_init(hist_data->map);
- if (ret)
- goto out_unreg;
-enable:
+ ret = event_trigger_register(cmd_ops, file, glob, trigger_data);
+ if (ret < 0)
+ goto out_free;
+
ret = hist_trigger_enable(trigger_data, file);
if (ret)
goto out_unreg;
--
2.47.2
Hi there,
I cannot build the `rtla` tool in the stable branch, version v6.6.80. The
root cause appears to be commit 41955b6c268154f81e34f9b61cf8156eec0730c0
which first appeared in v6.6.78. Here's how the build failure looks like
through Buildroot:
src/timerlat_hist.c: In function â€timerlat_hist_apply_config’:
src/timerlat_hist.c:908:60: error: â€struct timerlat_hist_params’ has
no member named â€kernel_workload’
908 | retval = osnoise_set_workload(tool->context,
params->kernel_workload);
| ^~
make[3]: *** [<builtin>: src/timerlat_hist.o] Error 1
A quick grep shows that that symbol is referenced, but not defined
anywhere:
~/work/prog/linux-kernel[cesnet/2025-02-28] $ git grep kernel_workload
tools/tracing/rtla/src/timerlat_hist.c: retval =
osnoise_set_workload(tool->context, params->kernel_workload);
tools/tracing/rtla/src/timerlat_top.c: retval =
osnoise_set_workload(top->context, params->kernel_workload);
Maybe some prerequisite patch is missing?
With kind regards,
Jan
From: Stefan Eichenberger <stefan.eichenberger(a)toradex.com>
Ensure the PHY reset and perst is asserted during power-off to
guarantee it is in a reset state upon repeated power-on calls. This
resolves an issue where the PHY may not properly initialize during
subsequent power-on cycles. Power-on will deassert the reset at the
appropriate time after tuning the PHY parameters.
During suspend/resume cycles, we observed that the PHY PLL failed to
lock during resume when the CPU temperature increased from 65C to 75C.
The observed errors were:
phy phy-32f00000.pcie-phy.3: phy poweron failed --> -110
imx6q-pcie 33800000.pcie: waiting for PHY ready timeout!
imx6q-pcie 33800000.pcie: PM: dpm_run_callback(): genpd_resume_noirq+0x0/0x80 returns -110
imx6q-pcie 33800000.pcie: PM: failed to resume noirq: error -110
This resulted in a complete CPU freeze, which is resolved by ensuring
the PHY is in reset during power-on, thus preventing PHY PLL failures.
Cc: stable(a)vger.kernel.org
Fixes: 1aa97b002258 ("phy: freescale: pcie: Initialize the imx8 pcie standalone phy driver")
Signed-off-by: Stefan Eichenberger <stefan.eichenberger(a)toradex.com>
---
drivers/phy/freescale/phy-fsl-imx8m-pcie.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/phy/freescale/phy-fsl-imx8m-pcie.c b/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
index 00f957a42d9dc..36bef416618de 100644
--- a/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
+++ b/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
@@ -158,6 +158,17 @@ static int imx8_pcie_phy_power_on(struct phy *phy)
return ret;
}
+static int imx8_pcie_phy_power_off(struct phy *phy)
+{
+ struct imx8_pcie_phy *imx8_phy = phy_get_drvdata(phy);
+
+ reset_control_assert(imx8_phy->reset);
+ if (imx8_phy->perst)
+ reset_control_assert(imx8_phy->perst);
+
+ return 0;
+}
+
static int imx8_pcie_phy_init(struct phy *phy)
{
struct imx8_pcie_phy *imx8_phy = phy_get_drvdata(phy);
@@ -178,6 +189,7 @@ static const struct phy_ops imx8_pcie_phy_ops = {
.init = imx8_pcie_phy_init,
.exit = imx8_pcie_phy_exit,
.power_on = imx8_pcie_phy_power_on,
+ .power_off = imx8_pcie_phy_power_off,
.owner = THIS_MODULE,
};
--
2.45.2