| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <00ec4120-19ca-4b20-85d0-754c05bdc669@norik.com>
Date: Mon, 10 Jun 2024 13:39:20 +0200
From: Primoz Fiser <primoz.fiser@...ik.com>
To: Viresh Kumar <viresh.kumar@...aro.org>
Cc: Viresh Kumar <vireshk@...nel.org>, Nishanth Menon <nm@...com>,
Stephen Boyd <sboyd@...nel.org>, linux-pm@...r.kernel.org,
linux-kernel@...r.kernel.org, upstream@...ts.phytec.de
Subject: Re: [PATCH 2/2] OPP: ti: Use devm_pm_opp_set_config_regulators
Hi Viresh,
On 10. 06. 24 06:22, Viresh Kumar wrote:
> Hi Primoz,
>
> Thanks for your changes, they look exactly as we discussed earlier, but ..
>
> On 06-06-24, 13:33, Primoz Fiser wrote:
>> Function ti_opp_supply_probe() since commit 6baee034cb55 ("OPP: ti:
>> Migrate to dev_pm_opp_set_config_regulators()") returns wrong values
>> when all goes well and hence driver probing eventually fails.
>>
>> Switch to using devm_pm_opp_set_config_regulators() function that
>> correctly handles return values and doesn't require us to handle
>> returned tokens.
>>
>> Fixes: 6baee034cb55 ("OPP: ti: Migrate to dev_pm_opp_set_config_regulators()")
>> Signed-off-by: Primoz Fiser <primoz.fiser@...ik.com>
>> ---
>> drivers/opp/ti-opp-supply.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/opp/ti-opp-supply.c b/drivers/opp/ti-opp-supply.c
>> index e3b97cd1fbbf..8a4bcc5fb9dc 100644
>> --- a/drivers/opp/ti-opp-supply.c
>> +++ b/drivers/opp/ti-opp-supply.c
>> @@ -392,7 +392,7 @@ static int ti_opp_supply_probe(struct platform_device *pdev)
>> return ret;
>> }
>>
>> - ret = dev_pm_opp_set_config_regulators(cpu_dev, ti_opp_config_regulators);
>> + ret = devm_pm_opp_set_config_regulators(cpu_dev, ti_opp_config_regulators);
>> if (ret < 0)
>> _free_optimized_voltages(dev, &opp_data);
>
> -- I made a mistake.
:(
>
> The driver gets probed with a platform device, while
> devm_pm_opp_set_config_regulators() works with cpu device. And so the
> issue related to module insertion/removal/insertion will still be
> there :(.
>
> Did you try that though ?
I didn't because of:
config ARM_TI_CPUFREQ
bool "Texas Instruments CPUFreq support"
is a built-in driver.
Anyway, I guess one could trigger this also with:
$ cd /sys/devices/platform/ocp/4a003b20.opp-supply/driver
$ echo 4a003b20.opp-supply > unbind
$ echo 4a003b20.opp-supply > bind
[ 164.231781] ------------[ cut here ]------------
[ 164.236450] WARNING: CPU: 1 PID: 230 at drivers/opp/core.c:2474
dev_pm_opp_set_config+0x384/0x634
[ 164.245422] Modules linked in: sha256_generic libsha256 sha256_arm
cfg80211 uas usb_storage xhci_plat_hcd xhci_hcd usbcore dwc3 roles
udc_core pru_rproc irq_pruss_intc usb_common rpmsg_ctrl rpmsg_char etnaviv
ti_vpe ti_vip snd_soc_simple_card gpu_sched ti_sc bq27xxx_battery_hdq
bq27xxx_battery snd_soc_simple_card_utils pvrsrvkm(O) snd_soc_omap_hdmi
ahci_dwc ti_csc libahci_platform v4l2_mem2mem ti_vpdma libahci omap_
aes_driver videobuf2_dma_contig libaes videobuf2_memops videobuf2_v4l2
c_can_platform pruss libata videobuf2_common c_can omap_wdt
phy_omap_usb2 can_dev omap_hdq dwc3_omap omap_des
snd_soc_tlv320aic3x_i2c snd_so
c_tlv320aic3x libdes rtc_omap wire at24 omap_crypto palmas_pwrbutton
extcon_palmas rtc_palmas omap_sham crypto_engine omap_remoteproc
virtio_rpmsg_bus rpmsg_ns sch_fq_codel cryptodev(O) cmemk(O)
[ 164.318298] CPU: 1 PID: 230 Comm: sh Tainted: G O
6.1.80-bsp-yocto-ampliphy-am57x-kirkstone #1
[ 164.328369] Hardware name: Generic DRA74X (Flattened Device Tree)
[ 164.334472] unwind_backtrace from show_stack+0x10/0x14
[ 164.339752] show_stack from dump_stack_lvl+0x40/0x4c
[ 164.344818] dump_stack_lvl from __warn+0x94/0xc0
[ 164.349578] __warn from warn_slowpath_fmt+0x1a4/0x1ac
[ 164.354736] warn_slowpath_fmt from dev_pm_opp_set_config+0x384/0x634
[ 164.361236] dev_pm_opp_set_config from devm_pm_opp_set_config+0xc/0x44
[ 164.367889] devm_pm_opp_set_config from ti_opp_supply_probe+0x1c8/0x2f4
[ 164.374633] ti_opp_supply_probe from platform_probe+0x5c/0xbc
[ 164.380493] platform_probe from really_probe+0xc8/0x2ec
[ 164.385833] really_probe from __driver_probe_device+0x88/0x1a0
[ 164.391815] __driver_probe_device from device_driver_attach+0x40/0x98
[ 164.398376] device_driver_attach from bind_store+0x80/0xec
[ 164.403991] bind_store from kernfs_fop_write_iter+0x10c/0x1cc
[ 164.409851] kernfs_fop_write_iter from vfs_write+0x2a0/0x3c8
[ 164.415649] vfs_write from ksys_write+0x5c/0xd4
[ 164.420288] ksys_write from ret_fast_syscall+0x0/0x4c
[ 164.425445] Exception stack(0xf2359fa8 to 0xf2359ff0)
[ 164.430511] 9fa0: 00000014 00539568 00000001
00539568 00000014 00000000
[ 164.438751] 9fc0: 00000014 00539568 b6f5c5a0 00000004 00000014
0050cc08 00000000 00000000
[ 164.446960] 9fe0: 00000004 bed839d8 b6e80827 b6e01ae6
[ 164.452056] ---[ end trace 0000000000000000 ]---
[ 164.456726] ti_opp_supply: probe of 4a003b20.opp-supply failed with
error -16
-sh: echo: write error: Device or resource busy
so the error comes from drivers/opp/core.c block:
/* This should be called before OPPs are initialized */
if (WARN_ON(!list_empty(&opp_table->opp_list))) {
ret = -EBUSY;
goto err;
}
>
> The only way to get this solved is probably by introducing a remove()
> method, which clears the OPP config and stores the token returned by
> dev_pm_opp_set_config_regulators().
>
I did some additional experiments today by adding .remove_new callback
and calling dev_pm_opp_clear_config() in it.
However I still get the same error as above.
Unforunatelly, I cannot test on latest master since it doesn't boot yet
on my board (still using linux-ti 6.1).
Shall we store token and not call dev_pm_opp_set_config_regulators() in
probe() if token has been already aquired once?
BR,
Primoz
Powered by blists - more mailing lists