On 7/8/22 10:26, Dmitry Osipenko wrote:
> On 7/8/22 10:19, Viresh Kumar wrote:
>> On 07-07-22, 22:43, Dmitry Osipenko wrote:
>>> This patch breaks Tegra again, please take a look:
>>
>> Damn, not again :(
>>
>>> OPP: Remove dev{m}_pm_opp_of_add_table_noclk()
>>
>> Why did you mention this patch ? This just removed an unused API,
>> Tegra should have broke because of something else, isn't it ?
>
> This patch is the cause.
>
>>> 8<--- cut here ---
>>> Unable to handle kernel paging request at virtual address ffffffff
>>> [ffffffff] *pgd=9effd861, *pte=00000000, *ppte=00000000
>>> Internal error: Oops: 37 [#1] PREEMPT SMP ARM
>>> Modules linked in:
>>> CPU: 3 PID: 8 Comm: kworker/u8:0 Not tainted
>>> 5.19.0-rc1-00040-g30b62d123f4f #82
>>> Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
>>> Workqueue: events_unbound deferred_probe_work_func
>>> PC is at _opp_compare_key+0x40/0xc4
>>> LR is at 0xfffffffb
>>
>> How is LR set to such an address ?
>>
>>> pc : [<c0b91b54>] lr : [<fffffffb>] psr: 20000113
>>> sp : df831b08 ip : c33cd4d0 fp : df831b24
>>> r10: c2586078 r9 : c258606c r8 : 00000000
>>> r7 : 00000000 r6 : 00000001 r5 : c33cd480 r4 : c2586000
>>> r3 : 00000000 r2 : c33cd480 r1 : c258606c r0 : c2586000
>>> Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none
>>> Control: 10c5387d Table: 8000404a DAC: 00000051
>>> ...
>>> Backtrace:
>>> _opp_compare_key from _set_opp+0x80/0x408
>>
>> Whatever happened, happened from _opp_compare_key() and I tried to
>> look at it many times, couldn't figure out what's wrong there.
>>
>> For the device in question, pmc I think, we don't have any "opp-hz"
>> property in the DT, but still the OPP core will fetch its clock and
>> set clk_count to 1. But this was working earlier too, we were
>> comparing the rate anyways. I think one of _opp_compare_rate() or
>> _opp_compare_bw() is broken here, but I just couldn't figure out. The
>> rate one should run one loop and bw one should just return. I don't
>> see why a crash should come out eventually.
>>
>> Can you help debug this a bit ? Also see what are the values of
>> opp_table->path_count and opp_table->clk_count, should be 0 and 1
>> AFAICT.
>
> I see that previously dev_pm_opp_set_config() had "_add_opp_table(dev,
> false)", now it's "_add_opp_table(dev, true)".
>
> Will take a closer look later on.
>
>> Sorry about this Dmitry, I think we are all settled and again went
>> into crap.
>
> No problems :)
BTW, maybe we should consider to start adding kselftests for OPP, like
clk framework did. That will be handy to have given that it's not easy
to test the whole OPP core without having specific devices.
--
Best regards,
Dmitry
