go ahead, need a sec to comprehend...
in short...
if spm_dev_probe return
per_cpu(cpu_spm_drv, cpu) is nerver set... (line 388)
yeah but from line 250 or after per_cpu?
hold on I'm still trying to keep up... line 388
From my test the function never goes in that for and should be right as we defined only one idle state
The devicetree representation of the idle state should be -
Required properties:
- compatible: Must be one of -
"qcom,idle-state-ret",
"qcom,idle-state-spc",
"qcom,idle-state-pc",
and "arm,idle-state".
from here (android I know but I'm pretty sure I saw this somewhere else). hence I thought that for loop should iterate a least twice.
I can't see what your seeing so go on...
the real idle state is spc... From Documentation idle states for qcom are 4
MH STRANGE... i don't have ANY output from spm_dev_probe
"qcom,idle-state-ret",
"qcom,idle-state-spc",
"qcom,idle-state-pc" AND "arm,idle-state" so i think
"arm,idle-state" is not included
that seems like it could be a bug... now to understand why
I really think that spm is never probed and from the comments... spm MUST BE PROBED BEFORE CPUIDLE INIT
I was thinking that... was trying to use dmesg output to see as there are cpuidle mesages that show up early (I saw where these came from earlier today but then already forgot - stupid spiders)
[ 0.017073] cpuidle: using governor ladder
[ 0.017141] cpuidle: using governor menu
drivers/cpuidle/governor.c prints this... anyway I'm with you about trying to find out about spm probe... but that will have to wait until tomorrow for me...
1 Like
I'm not sure how much time I'll have today to spend on this, but I'll be looking at 0070-qcom-spm-fix-probe-order.patch as this looks like a previous attempt to resolve an issue with spm.c. maybe just need to rework this patch for 4.19...
Also I'm looking at vanilia 4.19.56 and openwrt patched 4.19.56 code. Since your adding debug statements, trying to orient each other via line numbers probably won't work so well...
Well that patch has merged In mainline kernel so... You should take a look at tscr
As I think is the one that probe spm
static const struct of_device_id qcom_idle_state_match[] __initconst = {
{ .compatible = "qcom,idle-state-spc", .data = qcom_cpu_spc },
{ },
};
static int __init qcom_cpuidle_init(struct device_node *cpu_node, int cpu)
{
const struct of_device_id *match_id;
struct device_node *state_node;
int i;
int state_count = 1;
idle_fn idle_fns[CPUIDLE_STATE_MAX];
idle_fn *fns;
cpumask_t mask;
bool use_scm_power_down = false;
if (!qcom_scm_is_available())
return -EPROBE_DEFER;
for (i = 0; ; i++) {
state_node = of_parse_phandle(cpu_node, "cpu-idle-states", i);
if (!state_node)
break;
if (!of_device_is_available(state_node))
continue;
if (i == CPUIDLE_STATE_MAX) {
pr_warn("%s: cpuidle states reached max possible\n",
__func__);
break;
}
match_id = of_match_node(qcom_idle_state_match, state_node);
if (!match_id)
return -ENODEV;
idle_fns[state_count] = match_id->data;
/* Check if any of the states allow power down */
if (match_id->data == qcom_cpu_spc)
use_scm_power_down = true;
state_count++;
}
if (state_count == 1)
goto check_spm;
fns = devm_kcalloc(get_cpu_device(cpu), state_count, sizeof(*fns),
GFP_KERNEL);
if (!fns)
return -ENOMEM;
for (i = 1; i < state_count; i++)
fns[i] = idle_fns[i];
if (use_scm_power_down) {
/* We have atleast one power down mode */
cpumask_clear(&mask);
cpumask_set_cpu(cpu, &mask);
qcom_scm_set_warm_boot_addr(cpu_resume_arm, &mask);
}
per_cpu(qcom_idle_ops, cpu) = fns;
/*
* SPM probe for the cpu should have happened by now, if the
* SPM device does not exist, return -ENXIO to indicate that the
* cpu does not support idle states.
*/
check_spm:
return per_cpu(cpu_spm_drv, cpu) ? 0 : -ENXIO;
}
[snip]
CPUIDLE_METHOD_OF_DECLARE(qcom_idle_v1, "qcom,kpss-acc-v1", &qcom_cpuidle_ops);
CPUIDLE_METHOD_OF_DECLARE(qcom_idle_v2, "qcom,kpss-acc-v2", &qcom_cpuidle_ops);
static struct spm_driver_data *spm_get_drv(struct platform_device *pdev,
int *spm_cpu)
{
struct spm_driver_data *drv = NULL;
struct device_node *cpu_node, *saw_node;
int cpu;
bool found = 0;
for_each_possible_cpu(cpu) {
cpu_node = of_cpu_device_node_get(cpu);
if (!cpu_node)
continue;
saw_node = of_parse_phandle(cpu_node, "qcom,saw", 0);
found = (saw_node == pdev->dev.of_node);
of_node_put(saw_node);
of_node_put(cpu_node);
if (found)
break;
}
if (found) {
drv = devm_kzalloc(&pdev->dev, sizeof(*drv), GFP_KERNEL);
if (drv)
*spm_cpu = cpu;
}
return drv;
}
static const struct of_device_id spm_match_table[] = {
{ .compatible = "qcom,msm8974-saw2-v2.1-cpu",
.data = &spm_reg_8974_8084_cpu },
{ .compatible = "qcom,apq8084-saw2-v2.1-cpu",
.data = &spm_reg_8974_8084_cpu },
{ .compatible = "qcom,apq8064-saw2-v1.1-cpu",
.data = &spm_reg_8064_cpu },
{ },
};
1 Like
not sure I follow (see EDIT 2),
do you observe (or propose) a null driver is returned due to "qcom,saw"?
EDIT:
the same code is in vanilla 4.14 and I don't see a diff in cpu def in the dts. I'd have to check the openwrt patched 4.14 code...
atm i'm trying to trace program execution through the code only... that's pretty hard given my inexperience and the complexity of the kernel code (c function callbacks and an "ops" system that I'm unfamiliar with). Better to do a bit of @Ansuel is doing and put it a few debug statements (in multiple functions) and see what the actual execution is. But this is a "learning" experience for me.
EDIT 2: or are you pointing out that code is doing what its supposed to be doing given the ipq8064.dtsi cpu node saw definitions?
FWIW this is how I think about program execution/flow starting in 4.19 cpuidle-arm.c, ret = arm_cpuidle_init(cpu);
It's my understanding this returns -ENXIO from here. I know the execution gets to cpuidle.c:
if (!ret)
ret = cpuidle_ops[cpu].init(cpu_node, cpu);
since the proceeding function call:
ret = arm_cpuidle_read_ops(cpu_node, cpu);
generates the
Wed Jun 26 02:51:50 2019 kern.notice kernel: [ 1.546629] cpuidle: enable-method property 'qcom,kpss-acc-v1' found operations
messages.
cpuidle_ops[cpu].init(cpu_node, cpu); is a structure c function callback that I'm not sure where it goes... and I expect this is where the ENXIO is generated. I went to spm.c (cpuidle reference and potential for ENXIO return error) looking for a clue, but perhaps that's wrong and I need to keep looking.
HTH
1 Like
The error ENXIO is 100% generated in spm.c by label check_spm that doesn't find probed spm
Spm should be probed by tscr since the normal spm implementation doesn't look supported (the DTS should contain some compatible label but adding them put the router in bootloop) sooo we should find if in 4.14 spm gets probed and why it doesn't in 4.19
idle-states {
CPU_SPC: spc {
compatible = "qcom,idle-state-spc",
"arm,idle-state";
+ arm,psci-suspend-param = <0x40000002>;
entry-latency-us = <400>;
exit-latency-us = <900>;
min-residency-us = <3000>;
~ entry-latency-us = <130>;
~ exit-latency-us = <150>;
~ min-residency-us = <2000>;
+ local-timer-stop;
};
};
why the additional ?
no idea, though it seems that often, idle during init needs some sort of damping parameter....
this seemed close.... i dont think;
no-idle-on-init
is valid.....