* [PATCH v1 2/4] media: camss: vfe: Decrease priority of of VFE HW version to 'dbg'
2021-07-26 8:01 [PATCH v1 1/4] media: camss: vfe: Don't read hardware version needlessly Robert Foss
@ 2021-07-26 8:01 ` Robert Foss
2021-07-26 8:01 ` [PATCH v1 3/4] media: camss: vfe: Remove vfe_hw_version_read() argument Robert Foss
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Robert Foss @ 2021-07-26 8:01 UTC (permalink / raw)
To: robert.foss, todor.too, agross, bjorn.andersson, mchehab,
linux-media, linux-arm-msm, linux-kernel, Andrey Konovalov,
AngeloGioacchino Del Regno
The HW Version can in no case represent an error, so change the print
priority to 'dbg'.
Signed-off-by: Robert Foss <robert.foss@linaro.org>
---
drivers/media/platform/qcom/camss/camss-vfe-170.c | 2 +-
drivers/media/platform/qcom/camss/camss-vfe-4-7.c | 2 +-
drivers/media/platform/qcom/camss/camss-vfe-4-8.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/media/platform/qcom/camss/camss-vfe-170.c b/drivers/media/platform/qcom/camss/camss-vfe-170.c
index 8594d275b41d..1ad502a1c276 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe-170.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe-170.c
@@ -185,7 +185,7 @@ static void vfe_hw_version_read(struct vfe_device *vfe, struct device *dev)
u32 rev = (hw_version >> 16) & 0xFFF;
u32 step = hw_version & 0xFFFF;
- dev_err(dev, "VFE HW Version = %u.%u.%u\n", gen, rev, step);
+ dev_dbg(dev, "VFE HW Version = %u.%u.%u\n", gen, rev, step);
}
static inline void vfe_reg_clr(struct vfe_device *vfe, u32 reg, u32 clr_bits)
diff --git a/drivers/media/platform/qcom/camss/camss-vfe-4-7.c b/drivers/media/platform/qcom/camss/camss-vfe-4-7.c
index a59635217758..13f0fa6a7d60 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe-4-7.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe-4-7.c
@@ -258,7 +258,7 @@ static void vfe_hw_version_read(struct vfe_device *vfe, struct device *dev)
{
u32 hw_version = readl_relaxed(vfe->base + VFE_0_HW_VERSION);
- dev_err(dev, "VFE HW Version = 0x%08x\n", hw_version);
+ dev_dbg(dev, "VFE HW Version = 0x%08x\n", hw_version);
}
static u16 vfe_get_ub_size(u8 vfe_id)
diff --git a/drivers/media/platform/qcom/camss/camss-vfe-4-8.c b/drivers/media/platform/qcom/camss/camss-vfe-4-8.c
index 998429dbb65c..03f92a6737b0 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe-4-8.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe-4-8.c
@@ -251,7 +251,7 @@ static void vfe_hw_version_read(struct vfe_device *vfe, struct device *dev)
{
u32 hw_version = readl_relaxed(vfe->base + VFE_0_HW_VERSION);
- dev_err(dev, "VFE HW Version = 0x%08x\n", hw_version);
+ dev_dbg(dev, "VFE HW Version = 0x%08x\n", hw_version);
}
static inline void vfe_reg_clr(struct vfe_device *vfe, u32 reg, u32 clr_bits)
--
2.29.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v1 3/4] media: camss: vfe: Remove vfe_hw_version_read() argument
2021-07-26 8:01 [PATCH v1 1/4] media: camss: vfe: Don't read hardware version needlessly Robert Foss
2021-07-26 8:01 ` [PATCH v1 2/4] media: camss: vfe: Decrease priority of of VFE HW version to 'dbg' Robert Foss
@ 2021-07-26 8:01 ` Robert Foss
2021-07-26 8:01 ` [PATCH v1 4/4] media: camss: vfe: Rework vfe_hw_version_read() function definition Robert Foss
[not found] ` <CGME20210810214617eucas1p1e007c68c34792a844b6c72dd11d05887@eucas1p1.samsung.com>
3 siblings, 0 replies; 6+ messages in thread
From: Robert Foss @ 2021-07-26 8:01 UTC (permalink / raw)
To: robert.foss, todor.too, agross, bjorn.andersson, mchehab,
linux-media, linux-arm-msm, linux-kernel, Andrey Konovalov,
AngeloGioacchino Del Regno
The device argument is not needed, since it is accessible through
the vfe_device argument already.
Signed-off-by: Robert Foss <robert.foss@linaro.org>
---
drivers/media/platform/qcom/camss/camss-vfe-170.c | 5 +++--
drivers/media/platform/qcom/camss/camss-vfe-4-1.c | 4 ++--
drivers/media/platform/qcom/camss/camss-vfe-4-7.c | 4 ++--
drivers/media/platform/qcom/camss/camss-vfe-4-8.c | 4 ++--
drivers/media/platform/qcom/camss/camss-vfe.c | 2 +-
drivers/media/platform/qcom/camss/camss-vfe.h | 2 +-
6 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/media/platform/qcom/camss/camss-vfe-170.c b/drivers/media/platform/qcom/camss/camss-vfe-170.c
index 1ad502a1c276..8ef4e80a62a2 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe-170.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe-170.c
@@ -177,7 +177,7 @@
#define VFE_BUS_WM_FRAME_INC(n) (0x2258 + (n) * 0x100)
#define VFE_BUS_WM_BURST_LIMIT(n) (0x225c + (n) * 0x100)
-static void vfe_hw_version_read(struct vfe_device *vfe, struct device *dev)
+static void vfe_hw_version_read(struct vfe_device *vfe)
{
u32 hw_version = readl_relaxed(vfe->base + VFE_HW_VERSION);
@@ -185,7 +185,8 @@ static void vfe_hw_version_read(struct vfe_device *vfe, struct device *dev)
u32 rev = (hw_version >> 16) & 0xFFF;
u32 step = hw_version & 0xFFFF;
- dev_dbg(dev, "VFE HW Version = %u.%u.%u\n", gen, rev, step);
+ dev_dbg(vfe->camss->dev, "VFE HW Version = %u.%u.%u\n",
+ gen, rev, step);
}
static inline void vfe_reg_clr(struct vfe_device *vfe, u32 reg, u32 clr_bits)
diff --git a/drivers/media/platform/qcom/camss/camss-vfe-4-1.c b/drivers/media/platform/qcom/camss/camss-vfe-4-1.c
index 53c56a8d4545..614c266e8cd1 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe-4-1.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe-4-1.c
@@ -210,11 +210,11 @@
#define MSM_VFE_VFE0_UB_SIZE 1023
#define MSM_VFE_VFE0_UB_SIZE_RDI (MSM_VFE_VFE0_UB_SIZE / 3)
-static void vfe_hw_version_read(struct vfe_device *vfe, struct device *dev)
+static void vfe_hw_version_read(struct vfe_device *vfe)
{
u32 hw_version = readl_relaxed(vfe->base + VFE_0_HW_VERSION);
- dev_dbg(dev, "VFE HW Version = 0x%08x\n", hw_version);
+ dev_dbg(vfe->camss->dev, "VFE HW Version = 0x%08x\n", hw_version);
}
static u16 vfe_get_ub_size(u8 vfe_id)
diff --git a/drivers/media/platform/qcom/camss/camss-vfe-4-7.c b/drivers/media/platform/qcom/camss/camss-vfe-4-7.c
index 13f0fa6a7d60..aa175e0f6331 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe-4-7.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe-4-7.c
@@ -254,11 +254,11 @@
#define MSM_VFE_VFE1_UB_SIZE 1535
#define MSM_VFE_VFE1_UB_SIZE_RDI (MSM_VFE_VFE1_UB_SIZE / 3)
-static void vfe_hw_version_read(struct vfe_device *vfe, struct device *dev)
+static void vfe_hw_version_read(struct vfe_device *vfe)
{
u32 hw_version = readl_relaxed(vfe->base + VFE_0_HW_VERSION);
- dev_dbg(dev, "VFE HW Version = 0x%08x\n", hw_version);
+ dev_dbg(vfe->camss->dev, "VFE HW Version = 0x%08x\n", hw_version);
}
static u16 vfe_get_ub_size(u8 vfe_id)
diff --git a/drivers/media/platform/qcom/camss/camss-vfe-4-8.c b/drivers/media/platform/qcom/camss/camss-vfe-4-8.c
index 03f92a6737b0..b897f5163c5d 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe-4-8.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe-4-8.c
@@ -247,11 +247,11 @@
#define MSM_VFE_VFE1_UB_SIZE 1535
#define MSM_VFE_VFE1_UB_SIZE_RDI (MSM_VFE_VFE1_UB_SIZE / 3)
-static void vfe_hw_version_read(struct vfe_device *vfe, struct device *dev)
+static void vfe_hw_version_read(struct vfe_device *vfe)
{
u32 hw_version = readl_relaxed(vfe->base + VFE_0_HW_VERSION);
- dev_dbg(dev, "VFE HW Version = 0x%08x\n", hw_version);
+ dev_dbg(vfe->camss->dev, "VFE HW Version = 0x%08x\n", hw_version);
}
static inline void vfe_reg_clr(struct vfe_device *vfe, u32 reg, u32 clr_bits)
diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c
index f6e889c1598e..9b4f8136759c 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe.c
@@ -1299,7 +1299,7 @@ int msm_vfe_subdev_init(struct camss *camss, struct vfe_device *vfe,
return -EINVAL;
}
vfe->ops->subdev_init(dev, vfe);
- vfe->ops->hw_version_read(vfe, dev);
+ vfe->ops->hw_version_read(vfe);
/* Memory */
diff --git a/drivers/media/platform/qcom/camss/camss-vfe.h b/drivers/media/platform/qcom/camss/camss-vfe.h
index 844b9275031d..cca251816d28 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe.h
+++ b/drivers/media/platform/qcom/camss/camss-vfe.h
@@ -103,7 +103,7 @@ struct vfe_device;
struct vfe_hw_ops {
void (*enable_irq_common)(struct vfe_device *vfe);
void (*global_reset)(struct vfe_device *vfe);
- void (*hw_version_read)(struct vfe_device *vfe, struct device *dev);
+ void (*hw_version_read)(struct vfe_device *vfe);
irqreturn_t (*isr)(int irq, void *dev);
void (*isr_read)(struct vfe_device *vfe, u32 *value0, u32 *value1);
void (*pm_domain_off)(struct vfe_device *vfe);
--
2.29.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v1 4/4] media: camss: vfe: Rework vfe_hw_version_read() function definition
2021-07-26 8:01 [PATCH v1 1/4] media: camss: vfe: Don't read hardware version needlessly Robert Foss
2021-07-26 8:01 ` [PATCH v1 2/4] media: camss: vfe: Decrease priority of of VFE HW version to 'dbg' Robert Foss
2021-07-26 8:01 ` [PATCH v1 3/4] media: camss: vfe: Remove vfe_hw_version_read() argument Robert Foss
@ 2021-07-26 8:01 ` Robert Foss
[not found] ` <CGME20210810214617eucas1p1e007c68c34792a844b6c72dd11d05887@eucas1p1.samsung.com>
3 siblings, 0 replies; 6+ messages in thread
From: Robert Foss @ 2021-07-26 8:01 UTC (permalink / raw)
To: robert.foss, todor.too, agross, bjorn.andersson, mchehab,
linux-media, linux-arm-msm, linux-kernel, Andrey Konovalov,
AngeloGioacchino Del Regno
Expose the VFE version by returning the value read from the HW_VERSION
register.
Secondly, change the name of this function to conform with the CSID
equivalent.
Signed-off-by: Robert Foss <robert.foss@linaro.org>
---
drivers/media/platform/qcom/camss/camss-vfe-170.c | 6 ++++--
drivers/media/platform/qcom/camss/camss-vfe-4-1.c | 6 ++++--
drivers/media/platform/qcom/camss/camss-vfe-4-7.c | 6 ++++--
drivers/media/platform/qcom/camss/camss-vfe-4-8.c | 6 ++++--
drivers/media/platform/qcom/camss/camss-vfe.c | 2 +-
drivers/media/platform/qcom/camss/camss-vfe.h | 2 +-
6 files changed, 18 insertions(+), 10 deletions(-)
diff --git a/drivers/media/platform/qcom/camss/camss-vfe-170.c b/drivers/media/platform/qcom/camss/camss-vfe-170.c
index 8ef4e80a62a2..5c083d70d495 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe-170.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe-170.c
@@ -177,7 +177,7 @@
#define VFE_BUS_WM_FRAME_INC(n) (0x2258 + (n) * 0x100)
#define VFE_BUS_WM_BURST_LIMIT(n) (0x225c + (n) * 0x100)
-static void vfe_hw_version_read(struct vfe_device *vfe)
+static u32 vfe_hw_version(struct vfe_device *vfe)
{
u32 hw_version = readl_relaxed(vfe->base + VFE_HW_VERSION);
@@ -187,6 +187,8 @@ static void vfe_hw_version_read(struct vfe_device *vfe)
dev_dbg(vfe->camss->dev, "VFE HW Version = %u.%u.%u\n",
gen, rev, step);
+
+ return hw_version;
}
static inline void vfe_reg_clr(struct vfe_device *vfe, u32 reg, u32 clr_bits)
@@ -772,7 +774,7 @@ static void vfe_subdev_init(struct device *dev, struct vfe_device *vfe)
const struct vfe_hw_ops vfe_ops_170 = {
.global_reset = vfe_global_reset,
- .hw_version_read = vfe_hw_version_read,
+ .hw_version = vfe_hw_version,
.isr_read = vfe_isr_read,
.isr = vfe_isr,
.pm_domain_off = vfe_pm_domain_off,
diff --git a/drivers/media/platform/qcom/camss/camss-vfe-4-1.c b/drivers/media/platform/qcom/camss/camss-vfe-4-1.c
index 614c266e8cd1..7b7c9a0aaab2 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe-4-1.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe-4-1.c
@@ -210,11 +210,13 @@
#define MSM_VFE_VFE0_UB_SIZE 1023
#define MSM_VFE_VFE0_UB_SIZE_RDI (MSM_VFE_VFE0_UB_SIZE / 3)
-static void vfe_hw_version_read(struct vfe_device *vfe)
+static u32 vfe_hw_version(struct vfe_device *vfe)
{
u32 hw_version = readl_relaxed(vfe->base + VFE_0_HW_VERSION);
dev_dbg(vfe->camss->dev, "VFE HW Version = 0x%08x\n", hw_version);
+
+ return hw_version;
}
static u16 vfe_get_ub_size(u8 vfe_id)
@@ -1004,7 +1006,7 @@ static void vfe_subdev_init(struct device *dev, struct vfe_device *vfe)
const struct vfe_hw_ops vfe_ops_4_1 = {
.global_reset = vfe_global_reset,
- .hw_version_read = vfe_hw_version_read,
+ .hw_version = vfe_hw_version,
.isr_read = vfe_isr_read,
.isr = vfe_isr,
.pm_domain_off = vfe_pm_domain_off,
diff --git a/drivers/media/platform/qcom/camss/camss-vfe-4-7.c b/drivers/media/platform/qcom/camss/camss-vfe-4-7.c
index aa175e0f6331..2836b12ec989 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe-4-7.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe-4-7.c
@@ -254,11 +254,13 @@
#define MSM_VFE_VFE1_UB_SIZE 1535
#define MSM_VFE_VFE1_UB_SIZE_RDI (MSM_VFE_VFE1_UB_SIZE / 3)
-static void vfe_hw_version_read(struct vfe_device *vfe)
+static u32 vfe_hw_version(struct vfe_device *vfe)
{
u32 hw_version = readl_relaxed(vfe->base + VFE_0_HW_VERSION);
dev_dbg(vfe->camss->dev, "VFE HW Version = 0x%08x\n", hw_version);
+
+ return hw_version;
}
static u16 vfe_get_ub_size(u8 vfe_id)
@@ -1196,7 +1198,7 @@ static void vfe_subdev_init(struct device *dev, struct vfe_device *vfe)
const struct vfe_hw_ops vfe_ops_4_7 = {
.global_reset = vfe_global_reset,
- .hw_version_read = vfe_hw_version_read,
+ .hw_version = vfe_hw_version,
.isr_read = vfe_isr_read,
.isr = vfe_isr,
.pm_domain_off = vfe_pm_domain_off,
diff --git a/drivers/media/platform/qcom/camss/camss-vfe-4-8.c b/drivers/media/platform/qcom/camss/camss-vfe-4-8.c
index b897f5163c5d..19519234f727 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe-4-8.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe-4-8.c
@@ -247,11 +247,13 @@
#define MSM_VFE_VFE1_UB_SIZE 1535
#define MSM_VFE_VFE1_UB_SIZE_RDI (MSM_VFE_VFE1_UB_SIZE / 3)
-static void vfe_hw_version_read(struct vfe_device *vfe)
+static u32 vfe_hw_version(struct vfe_device *vfe)
{
u32 hw_version = readl_relaxed(vfe->base + VFE_0_HW_VERSION);
dev_dbg(vfe->camss->dev, "VFE HW Version = 0x%08x\n", hw_version);
+
+ return hw_version;
}
static inline void vfe_reg_clr(struct vfe_device *vfe, u32 reg, u32 clr_bits)
@@ -1180,7 +1182,7 @@ static void vfe_subdev_init(struct device *dev, struct vfe_device *vfe)
const struct vfe_hw_ops vfe_ops_4_8 = {
.global_reset = vfe_global_reset,
- .hw_version_read = vfe_hw_version_read,
+ .hw_version = vfe_hw_version,
.isr_read = vfe_isr_read,
.isr = vfe_isr,
.pm_domain_off = vfe_pm_domain_off,
diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c
index 9b4f8136759c..6b2f33fc9be2 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe.c
@@ -1299,7 +1299,7 @@ int msm_vfe_subdev_init(struct camss *camss, struct vfe_device *vfe,
return -EINVAL;
}
vfe->ops->subdev_init(dev, vfe);
- vfe->ops->hw_version_read(vfe);
+ vfe->ops->hw_version(vfe);
/* Memory */
diff --git a/drivers/media/platform/qcom/camss/camss-vfe.h b/drivers/media/platform/qcom/camss/camss-vfe.h
index cca251816d28..f166d176cb77 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe.h
+++ b/drivers/media/platform/qcom/camss/camss-vfe.h
@@ -103,7 +103,7 @@ struct vfe_device;
struct vfe_hw_ops {
void (*enable_irq_common)(struct vfe_device *vfe);
void (*global_reset)(struct vfe_device *vfe);
- void (*hw_version_read)(struct vfe_device *vfe);
+ u32 (*hw_version)(struct vfe_device *vfe);
irqreturn_t (*isr)(int irq, void *dev);
void (*isr_read)(struct vfe_device *vfe, u32 *value0, u32 *value1);
void (*pm_domain_off)(struct vfe_device *vfe);
--
2.29.2
^ permalink raw reply related [flat|nested] 6+ messages in thread[parent not found: <CGME20210810214617eucas1p1e007c68c34792a844b6c72dd11d05887@eucas1p1.samsung.com>]
* Re: [PATCH v1 1/4] media: camss: vfe: Don't read hardware version needlessly
[not found] ` <CGME20210810214617eucas1p1e007c68c34792a844b6c72dd11d05887@eucas1p1.samsung.com>
@ 2021-08-10 21:46 ` Marek Szyprowski
2021-08-11 7:32 ` Robert Foss
0 siblings, 1 reply; 6+ messages in thread
From: Marek Szyprowski @ 2021-08-10 21:46 UTC (permalink / raw)
To: Robert Foss, todor.too, agross, bjorn.andersson, mchehab,
linux-media, linux-arm-msm, linux-kernel, Andrey Konovalov,
AngeloGioacchino Del Regno
Hi,
On 26.07.2021 10:01, Robert Foss wrote:
> vfe_set_power() is invoced a few times, which causes a lot of
> VFE HW version prints.
>
> Secondly there is no real reason for calling hw_version_read() from
> this location specifically, since it does nothing more than print
> the VFE HW version.
>
> A better location for the hw_version_read() call would be somewhere
> which is only executed once per VFE unit.
>
> Signed-off-by: Robert Foss <robert.foss@linaro.org>
This patch landed recently in linux-next as commit b10b5334528a ("media:
camss: vfe: Don't read hardware version needlessly"). Sadly it causes a
following regression on DragonBoard410c
(arch/arm64/boot/dts/qcom/apq8016-sbc.dts):
Unable to handle kernel NULL pointer dereference at virtual address
0000000000000000
Mem abort info:
ESR = 0x96000006
EC = 0x25: DABT (current EL), IL = 32 bits
SET = 0, FnV = 0
EA = 0, S1PTW = 0
FSC = 0x06: level 2 translation fault
input: pm8941_resin as
/devices/platform/soc/200f000.spmi/spmi-0/0-00/200f000.spmi:pmic@0:pon@800/200f000.spmi:pmic@0:pon@800:resin/input/input2
Data abort info:
ISV = 0, ISS = 0x00000006
CM = 0, WnR = 0
user pgtable: 4k pages, 48-bit VAs, pgdp=0000000087ca0000
[0000000000000000] pgd=0800000084c4c003, p4d=0800000084c4c003,
pud=08000000859c2003, pmd=0000000000000000
Internal error: Oops: 96000006 [#1] PREEMPT SMP
Modules linked in: msm(+) qcom_pon rtc_pm8xxx qcom_spmi_vadc
qcom_vadc_common qcom_camss(+) snd_soc_msm8916_analog
qcom_spmi_temp_alarm venus_core(+) videobuf2_dma_sg v4l2_fwnode
snd_soc_lpass_apq8016 v4l2_async videobuf2_memops snd_soc_lpass_cpu
v4l2_mem2mem snd_soc_apq8016_sbc snd_soc_lpass_platform
snd_soc_msm8916_digital videobuf2_v4l2 snd_soc_qcom_common
videobuf2_common qnoc_msm8916 qcom_rng videodev i2c_qcom_cci mdt_loader
mc icc_smd_rpm crct10dif_ce socinfo rmtfs_mem display_connector
adreno 1c00000.gpu: Adding to iommu group 2
CPU: 1 PID: 286 Comm: systemd-udevd Not tainted 5.14.0-rc4+ #3778
Hardware name: Qualcomm Technologies, Inc. APQ 8016 SBC (DT)
pstate: 80000005 (Nzcv daif -PAN -UAO -TCO BTYPE=--)
pc : vfe_hw_version_read+0x4/0x10 [qcom_camss]
platform 1a98000.dsi: Fixing up cyclic dependency with 3-0039
lr : msm_vfe_subdev_init+0x84/0x558 [qcom_camss]
platform 1a98000.dsi: Fixing up cyclic dependency with 1a01000.mdp
sp : ffff8000148337f0
x29: ffff8000148337f0 x28: ffff000006c1d800 x27: 0000000000000001
x26: 0000000000000002 x25: ffff8000091976d0 x24: ffff000006a98080
x23: 0000000000000000 x22: ffff000006c1d800 x21: ffff000000298010
x20: ffff800009197028 x19: ffff000000298000 x18: 0000000000000000
x17: 2e726f74616c7567 x16: 0000000000000028 x15: 0000b5f01da86af8
x14: 0000000000000001 x13: 000000000000001f x12: 0000000000000001
x11: 0000000000000010 x10: 0101010101010101 x9 : ffff8000091922f0
x8 : ffff8000091949f0 x7 : ffff800009194a08 x6 : ffff8000091925c8
x5 : ffff800009191f00 x4 : ffff000006a9ca00 x3 : ffff000006a9c080
x2 : ffff80000918d3a0 x1 : ffff000000298010 x0 : 0000000000000000
msm_mdp 1a01000.mdp: Adding to iommu group 3
Call trace:
vfe_hw_version_read+0x4/0x10 [qcom_camss]
camss_probe+0x370/0xdd0 [qcom_camss]
platform_probe+0x90/0xd8
really_probe+0x138/0x2d0
__driver_probe_device+0x78/0xd8
driver_probe_device+0x40/0x110
__driver_attach+0xcc/0x118
bus_for_each_dev+0x68/0xc8
driver_attach+0x20/0x28
bus_add_driver+0x168/0x1f8
driver_register+0x60/0x110
__platform_driver_register+0x24/0x30
qcom_camss_driver_init+0x1c/0x1000 [qcom_camss]
do_one_initcall+0x84/0x450
do_init_module+0x58/0x200
load_module+0x1f54/0x26a0
__do_sys_finit_module+0xe4/0x100
__arm64_sys_finit_module+0x18/0x20
invoke_syscall+0x40/0xf8
el0_svc_common+0x60/0x100
do_el0_svc_compat+0x1c/0x48
el0_svc_compat+0x20/0x30
el0t_32_sync_handler+0xec/0x140
el0t_32_sync+0x168/0x16c
Code: 88dffc63 88dffc63 00000000 f9400800 (b9400000)
---[ end trace e1e3b303eae40654 ]---
Let me know if I can help somehow to fix this issue.
> ---
> drivers/media/platform/qcom/camss/camss-vfe.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c
> index e0f3a36f3f3f..f6e889c1598e 100644
> --- a/drivers/media/platform/qcom/camss/camss-vfe.c
> +++ b/drivers/media/platform/qcom/camss/camss-vfe.c
> @@ -713,8 +713,6 @@ static int vfe_set_power(struct v4l2_subdev *sd, int on)
> ret = vfe_get(vfe);
> if (ret < 0)
> return ret;
> -
> - vfe->ops->hw_version_read(vfe, vfe->camss->dev);
> } else {
> vfe_put(vfe);
> }
> @@ -1301,6 +1299,7 @@ int msm_vfe_subdev_init(struct camss *camss, struct vfe_device *vfe,
> return -EINVAL;
> }
> vfe->ops->subdev_init(dev, vfe);
> + vfe->ops->hw_version_read(vfe, dev);
>
> /* Memory */
>
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v1 1/4] media: camss: vfe: Don't read hardware version needlessly
2021-08-10 21:46 ` [PATCH v1 1/4] media: camss: vfe: Don't read hardware version needlessly Marek Szyprowski
@ 2021-08-11 7:32 ` Robert Foss
0 siblings, 0 replies; 6+ messages in thread
From: Robert Foss @ 2021-08-11 7:32 UTC (permalink / raw)
To: Marek Szyprowski
Cc: Todor Tomov, Andy Gross, Bjorn Andersson, Mauro Carvalho Chehab,
linux-media, MSM, linux-kernel, Andrey Konovalov,
AngeloGioacchino Del Regno
Hey Marek,
On Tue, 10 Aug 2021 at 23:46, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>
> Hi,
>
> On 26.07.2021 10:01, Robert Foss wrote:
> > vfe_set_power() is invoced a few times, which causes a lot of
> > VFE HW version prints.
> >
> > Secondly there is no real reason for calling hw_version_read() from
> > this location specifically, since it does nothing more than print
> > the VFE HW version.
> >
> > A better location for the hw_version_read() call would be somewhere
> > which is only executed once per VFE unit.
> >
> > Signed-off-by: Robert Foss <robert.foss@linaro.org>
>
> This patch landed recently in linux-next as commit b10b5334528a ("media:
> camss: vfe: Don't read hardware version needlessly"). Sadly it causes a
> following regression on DragonBoard410c
> (arch/arm64/boot/dts/qcom/apq8016-sbc.dts):
>
> Unable to handle kernel NULL pointer dereference at virtual address
> 0000000000000000
> Mem abort info:
> ESR = 0x96000006
> EC = 0x25: DABT (current EL), IL = 32 bits
> SET = 0, FnV = 0
> EA = 0, S1PTW = 0
> FSC = 0x06: level 2 translation fault
> input: pm8941_resin as
> /devices/platform/soc/200f000.spmi/spmi-0/0-00/200f000.spmi:pmic@0:pon@800/200f000.spmi:pmic@0:pon@800:resin/input/input2
> Data abort info:
> ISV = 0, ISS = 0x00000006
> CM = 0, WnR = 0
> user pgtable: 4k pages, 48-bit VAs, pgdp=0000000087ca0000
> [0000000000000000] pgd=0800000084c4c003, p4d=0800000084c4c003,
> pud=08000000859c2003, pmd=0000000000000000
> Internal error: Oops: 96000006 [#1] PREEMPT SMP
> Modules linked in: msm(+) qcom_pon rtc_pm8xxx qcom_spmi_vadc
> qcom_vadc_common qcom_camss(+) snd_soc_msm8916_analog
> qcom_spmi_temp_alarm venus_core(+) videobuf2_dma_sg v4l2_fwnode
> snd_soc_lpass_apq8016 v4l2_async videobuf2_memops snd_soc_lpass_cpu
> v4l2_mem2mem snd_soc_apq8016_sbc snd_soc_lpass_platform
> snd_soc_msm8916_digital videobuf2_v4l2 snd_soc_qcom_common
> videobuf2_common qnoc_msm8916 qcom_rng videodev i2c_qcom_cci mdt_loader
> mc icc_smd_rpm crct10dif_ce socinfo rmtfs_mem display_connector
> adreno 1c00000.gpu: Adding to iommu group 2
> CPU: 1 PID: 286 Comm: systemd-udevd Not tainted 5.14.0-rc4+ #3778
> Hardware name: Qualcomm Technologies, Inc. APQ 8016 SBC (DT)
> pstate: 80000005 (Nzcv daif -PAN -UAO -TCO BTYPE=--)
> pc : vfe_hw_version_read+0x4/0x10 [qcom_camss]
> platform 1a98000.dsi: Fixing up cyclic dependency with 3-0039
> lr : msm_vfe_subdev_init+0x84/0x558 [qcom_camss]
> platform 1a98000.dsi: Fixing up cyclic dependency with 1a01000.mdp
> sp : ffff8000148337f0
> x29: ffff8000148337f0 x28: ffff000006c1d800 x27: 0000000000000001
> x26: 0000000000000002 x25: ffff8000091976d0 x24: ffff000006a98080
> x23: 0000000000000000 x22: ffff000006c1d800 x21: ffff000000298010
> x20: ffff800009197028 x19: ffff000000298000 x18: 0000000000000000
> x17: 2e726f74616c7567 x16: 0000000000000028 x15: 0000b5f01da86af8
> x14: 0000000000000001 x13: 000000000000001f x12: 0000000000000001
> x11: 0000000000000010 x10: 0101010101010101 x9 : ffff8000091922f0
> x8 : ffff8000091949f0 x7 : ffff800009194a08 x6 : ffff8000091925c8
> x5 : ffff800009191f00 x4 : ffff000006a9ca00 x3 : ffff000006a9c080
> x2 : ffff80000918d3a0 x1 : ffff000000298010 x0 : 0000000000000000
> msm_mdp 1a01000.mdp: Adding to iommu group 3
> Call trace:
> vfe_hw_version_read+0x4/0x10 [qcom_camss]
> camss_probe+0x370/0xdd0 [qcom_camss]
> platform_probe+0x90/0xd8
> really_probe+0x138/0x2d0
> __driver_probe_device+0x78/0xd8
> driver_probe_device+0x40/0x110
> __driver_attach+0xcc/0x118
> bus_for_each_dev+0x68/0xc8
> driver_attach+0x20/0x28
> bus_add_driver+0x168/0x1f8
> driver_register+0x60/0x110
> __platform_driver_register+0x24/0x30
> qcom_camss_driver_init+0x1c/0x1000 [qcom_camss]
> do_one_initcall+0x84/0x450
> do_init_module+0x58/0x200
> load_module+0x1f54/0x26a0
> __do_sys_finit_module+0xe4/0x100
> __arm64_sys_finit_module+0x18/0x20
> invoke_syscall+0x40/0xf8
> el0_svc_common+0x60/0x100
> do_el0_svc_compat+0x1c/0x48
> el0_svc_compat+0x20/0x30
> el0t_32_sync_handler+0xec/0x140
> el0t_32_sync+0x168/0x16c
> Code: 88dffc63 88dffc63 00000000 f9400800 (b9400000)
> ---[ end trace e1e3b303eae40654 ]---
>
> Let me know if I can help somehow to fix this issue.
>
> > ---
> > drivers/media/platform/qcom/camss/camss-vfe.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c
> > index e0f3a36f3f3f..f6e889c1598e 100644
> > --- a/drivers/media/platform/qcom/camss/camss-vfe.c
> > +++ b/drivers/media/platform/qcom/camss/camss-vfe.c
> > @@ -713,8 +713,6 @@ static int vfe_set_power(struct v4l2_subdev *sd, int on)
> > ret = vfe_get(vfe);
> > if (ret < 0)
> > return ret;
> > -
> > - vfe->ops->hw_version_read(vfe, vfe->camss->dev);
> > } else {
> > vfe_put(vfe);
> > }
> > @@ -1301,6 +1299,7 @@ int msm_vfe_subdev_init(struct camss *camss, struct vfe_device *vfe,
> > return -EINVAL;
> > }
> > vfe->ops->subdev_init(dev, vfe);
> > + vfe->ops->hw_version_read(vfe, dev);
> >
> > /* Memory */
> >
>
Thanks for reporting this.
I submitted a patch[1] for this yesterday, if you have time to test it
and supply a Tested-by/Reviewed-by, that would be really helpful.
[1] https://lore.kernel.org/linux-arm-msm/20210810103336.114077-1-robert.foss@linaro.org/
^ permalink raw reply [flat|nested] 6+ messages in thread