On 12.05.2022 10:23, Vladimir Zapolskiy wrote:
> To simplify the driver's maintenance it makes sense to escape from
> hardcoded numbers of power domain resources per platform and statical
> allocation of the resources. For instance on a QCOM SM8450 platform
> the number of CAMSS power domains shall be bumped up to 6, also notably
> CAMSS on MSM8916 has only one power domain, however it expects to get 2,
> and thus it should result in a runtime error on driver probe.
>
> The change fixes an issue mentioned above and gives more flexibility
> to support more platforms in future.
>
> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@xxxxxxxxxx>
This patch landed recently in linux next-20220616 as commit f673698ceee5
("media: camss: Allocate power domain resources dynamically").
Unfortunately it causes serious issues on DragonBoard 410c SBC
(arch/arm64/boot/dts/qcom/apq8016-sbc.dts), like multiple 'Unable to
handle kernel NULL pointer dereference at virtual address 0000000000000000'.
The problem originates in camss_configure_pd() function. Old version
assigned 0 to camss->genpd_num on that board, while the new one sets it
to 1 as a result of of_count_phandle_with_args(). When it is set to 1,
it causes issues somewhere later in the code, the stack traces points to
sysfs:
Unable to handle kernel NULL pointer dereference at virtual address
0000000000000000
Mem abort info:
ESR = 0x0000000096000006
EC = 0x25: DABT (current EL), IL = 32 bits
SET = 0, FnV = 0
EA = 0, S1PTW = 0
FSC = 0x06: level 2 translation fault
Data abort info:
ISV = 0, ISS = 0x00000006
CM = 0, WnR = 0
user pgtable: 4k pages, 48-bit VAs, pgdp=0000000080fba000
[0000000000000000] pgd=0800000080fd7003, p4d=0800000080fd7003,
pud=0800000080fdb003, pmd=0000000000000000
Internal error: Oops: 96000006 [#2] PREEMPT SMP
Modules linked in: crct10dif_ce asix(+) qcom_wcnss_pil socinfo adv7511
snd_soc_msm8916_digital snd_soc_lpass_apq8016 snd_soc_lpass_cpu
snd_soc_lpass_platform snd_soc_apq8016_sbc snd_soc_qcom_common qrtr
qcom_q6v5_mss qcom_pil_info qcom_q6v5 qcom_sysmon qcom_common
qcom_glink_smem qmi_helpers snd_soc_msm8916_analog qcom_camss
qnoc_msm8916 icc_smd_rpm videobuf2_dma_sg v4l2_fwnode
qcom_spmi_temp_alarm v4l2_async rtc_pm8xxx videobuf2_memops venus_core
qcom_pon v4l2_mem2mem videobuf2_v4l2 qcom_spmi_vadc qcom_vadc_common
videobuf2_common qcom_rng qcom_stats msm videodev i2c_qcom_cci mc
mdt_loader gpu_sched drm_dp_aux_bus display_connector rmtfs_mem
CPU: 0 PID: 255 Comm: systemd-udevd Tainted: G D W
5.19.0-rc2-next-20220616 #12197
Hardware name: Qualcomm Technologies, Inc. APQ 8016 SBC (DT)
pstate: 00000005 (nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : sysfs_kf_seq_show+0x34/0x140
lr : sysfs_kf_seq_show+0x30/0x140
ng swap..[ 23.849418] sp : ffff80000c643bc0
...
Call trace:
sysfs_kf_seq_show+0x34/0x140
kernfs_seq_show+0x28/0x30
seq_read_iter+0x118/0x440
kernfs_fop_read_iter+0x11c/0x1b0
new_sync_read+0xd0/0x188
vfs_read+0x134/0x1d0
ksys_read+0x64/0xf0
__arm64_sys_read+0x14/0x20
invoke_syscall+0x40/0xf8
el0_svc_common.constprop.3+0x8c/0x120
do_el0_svc_compat+0x18/0x48
el0_svc_compat+0x48/0xd0
el0t_32_sync_handler+0xec/0x140
el0t_32_sync+0x160/0x164
Code: f9401821 f9404437 97fffe37 aa0003f5 (f9400000)
---[ end trace 0000000000000000 ]---
> ---
> drivers/media/platform/qcom/camss/camss.c | 38 +++++++++++++----------
> drivers/media/platform/qcom/camss/camss.h | 7 ++---
> 2 files changed, 24 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
> index 79ad82e233cb..32d72b4f947b 100644
> --- a/drivers/media/platform/qcom/camss/camss.c
> +++ b/drivers/media/platform/qcom/camss/camss.c
> @@ -1452,19 +1452,31 @@ static const struct media_device_ops camss_media_ops = {
>
> static int camss_configure_pd(struct camss *camss)
> {
> - int nbr_pm_domains = 0;
> + struct device *dev = camss->dev;
> int last_pm_domain = 0;
> int i;
> int ret;
>
> - if (camss->version == CAMSS_8x96 ||
> - camss->version == CAMSS_660)
> - nbr_pm_domains = PM_DOMAIN_GEN1_COUNT;
> - else if (camss->version == CAMSS_845 ||
> - camss->version == CAMSS_8250)
> - nbr_pm_domains = PM_DOMAIN_GEN2_COUNT;
> + camss->genpd_num = of_count_phandle_with_args(dev->of_node,
> + "power-domains",
> + "#power-domain-cells");
> + if (camss->genpd_num < 0) {
> + dev_err(dev, "Power domains are not defined for camss\n");
> + return camss->genpd_num;
> + }
> +
> + camss->genpd = devm_kmalloc_array(dev, camss->genpd_num,
> + sizeof(*camss->genpd), GFP_KERNEL);
> + if (!camss->genpd)
> + return -ENOMEM;
>
> - for (i = 0; i < nbr_pm_domains; i++) {
> + camss->genpd_link = devm_kmalloc_array(dev, camss->genpd_num,
> + sizeof(*camss->genpd_link),
> + GFP_KERNEL);
> + if (!camss->genpd_link)
> + return -ENOMEM;
> +
> + for (i = 0; i < camss->genpd_num; i++) {
> camss->genpd[i] = dev_pm_domain_attach_by_id(camss->dev, i);
> if (IS_ERR(camss->genpd[i])) {
> ret = PTR_ERR(camss->genpd[i]);
> @@ -1689,7 +1701,6 @@ static int camss_probe(struct platform_device *pdev)
>
> void camss_delete(struct camss *camss)
> {
> - int nbr_pm_domains = 0;
> int i;
>
> v4l2_device_unregister(&camss->v4l2_dev);
> @@ -1698,14 +1709,7 @@ void camss_delete(struct camss *camss)
>
> pm_runtime_disable(camss->dev);
>
> - if (camss->version == CAMSS_8x96 ||
> - camss->version == CAMSS_660)
> - nbr_pm_domains = PM_DOMAIN_GEN1_COUNT;
> - else if (camss->version == CAMSS_845 ||
> - camss->version == CAMSS_8250)
> - nbr_pm_domains = PM_DOMAIN_GEN2_COUNT;
> -
> - for (i = 0; i < nbr_pm_domains; i++) {
> + for (i = 0; i < camss->genpd_num; i++) {
> device_link_del(camss->genpd_link[i]);
> dev_pm_domain_detach(camss->genpd[i], true);
> }
> diff --git a/drivers/media/platform/qcom/camss/camss.h b/drivers/media/platform/qcom/camss/camss.h
> index c9b3e0df5be8..0db80cadbbaa 100644
> --- a/drivers/media/platform/qcom/camss/camss.h
> +++ b/drivers/media/platform/qcom/camss/camss.h
> @@ -69,9 +69,7 @@ struct resources_icc {
> enum pm_domain {
> PM_DOMAIN_VFE0 = 0,
> PM_DOMAIN_VFE1 = 1,
> - PM_DOMAIN_GEN1_COUNT = 2, /* CAMSS series of ISPs */
> PM_DOMAIN_VFELITE = 2, /* VFELITE / TOP GDSC */
> - PM_DOMAIN_GEN2_COUNT = 3, /* Titan series of ISPs */
> };
>
> enum camss_version {
> @@ -101,8 +99,9 @@ struct camss {
> int vfe_num;
> struct vfe_device *vfe;
> atomic_t ref_count;
> - struct device *genpd[PM_DOMAIN_GEN2_COUNT];
> - struct device_link *genpd_link[PM_DOMAIN_GEN2_COUNT];
> + int genpd_num;
> + struct device **genpd;
> + struct device_link **genpd_link;
> struct icc_path *icc_path[ICC_SM8250_COUNT];
> struct icc_bw_tbl icc_bw_tbl[ICC_SM8250_COUNT];
> };
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
