[PATCH 1/2] media: qcom: camss: Fix csid clock configuration and IRQ offset for 8775p

Wenmeng Liu posted 2 patches 3 weeks, 4 days ago
[PATCH 1/2] media: qcom: camss: Fix csid clock configuration and IRQ offset for 8775p
Posted by Wenmeng Liu 3 weeks, 4 days ago
Fix two issues in csid driver for 8775p platform:

1. Simplify clock configuration for csid lite by removing unused clocks
   and correcting clock rates. Only vfe_lite_csid and vfe_lite_cphy_rx
   clocks are actually needed.
2. Fix BUF_DONE_IRQ_STATUS_RDI_OFFSET calculation for csid lite on
   sa8775p platform. The offset should be 0 for csid lite on sa8775p,

Fixes: ed03e99de0fa ("media: qcom: camss: Add support for CSID 690")
Signed-off-by: Wenmeng Liu <wenmeng.liu@oss.qualcomm.com>
---
 .../media/platform/qcom/camss/camss-csid-gen3.c    |  6 ++--
 drivers/media/platform/qcom/camss/camss.c          | 40 +++++++++-------------
 2 files changed, 20 insertions(+), 26 deletions(-)

diff --git a/drivers/media/platform/qcom/camss/camss-csid-gen3.c b/drivers/media/platform/qcom/camss/camss-csid-gen3.c
index 664245cf6eb0cac662b02f8b920cd1c72db0aeb2..bd059243790edeb045080905eb76fef3b12caae1 100644
--- a/drivers/media/platform/qcom/camss/camss-csid-gen3.c
+++ b/drivers/media/platform/qcom/camss/camss-csid-gen3.c
@@ -48,9 +48,9 @@
 #define IS_CSID_690(csid)	((csid->camss->res->version == CAMSS_8775P) \
 				 || (csid->camss->res->version == CAMSS_8300))
 #define CSID_BUF_DONE_IRQ_STATUS	0x8C
-#define BUF_DONE_IRQ_STATUS_RDI_OFFSET  (csid_is_lite(csid) ?\
-						1 : (IS_CSID_690(csid) ?\
-						13 : 14))
+#define BUF_DONE_IRQ_STATUS_RDI_OFFSET  (csid_is_lite(csid) ? \
+						((IS_CSID_690(csid) ? 0 : 1)) : \
+						((IS_CSID_690(csid) ? 13 : 14)))
 #define CSID_BUF_DONE_IRQ_MASK		0x90
 #define CSID_BUF_DONE_IRQ_CLEAR		0x94
 #define CSID_BUF_DONE_IRQ_SET		0x98
diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index 00b87fd9afbd89871ffaee9cb2b2db6538e1d70d..4a0bf8acd7645f8cd8c1b4cb9b6ff6f3a54d42e8 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -3598,9 +3598,11 @@ static const struct camss_subdev_resources csid_res_8775p[] = {
 	/* CSID2 (lite) */
 	{
 		.regulators = {},
-		.clock = { "cpas_vfe_lite", "vfe_lite_ahb",
-			   "vfe_lite_csid", "vfe_lite_cphy_rx",
-			   "vfe_lite"},
+		.clock = { "vfe_lite_csid", "vfe_lite_cphy_rx" },
+		.clock_rate = {
+			{ 400000000, 480000000 },
+			{ 400000000, 480000000 }
+		},
 		.clock_rate = {
 			{ 0, 0, 400000000, 400000000, 0},
 			{ 0, 0, 400000000, 480000000, 0}
@@ -3617,12 +3619,10 @@ static const struct camss_subdev_resources csid_res_8775p[] = {
 	/* CSID3 (lite) */
 	{
 		.regulators = {},
-		.clock = { "cpas_vfe_lite", "vfe_lite_ahb",
-			   "vfe_lite_csid", "vfe_lite_cphy_rx",
-			   "vfe_lite"},
+		.clock = { "vfe_lite_csid", "vfe_lite_cphy_rx" },
 		.clock_rate = {
-			{ 0, 0, 400000000, 400000000, 0},
-			{ 0, 0, 400000000, 480000000, 0}
+			{ 400000000, 480000000 },
+			{ 400000000, 480000000 }
 		},
 		.reg = { "csid_lite1" },
 		.interrupt = { "csid_lite1" },
@@ -3636,12 +3636,10 @@ static const struct camss_subdev_resources csid_res_8775p[] = {
 	/* CSID4 (lite) */
 	{
 		.regulators = {},
-		.clock = { "cpas_vfe_lite", "vfe_lite_ahb",
-			   "vfe_lite_csid", "vfe_lite_cphy_rx",
-			   "vfe_lite"},
+		.clock = { "vfe_lite_csid", "vfe_lite_cphy_rx" },
 		.clock_rate = {
-			{ 0, 0, 400000000, 400000000, 0},
-			{ 0, 0, 400000000, 480000000, 0}
+			{ 400000000, 480000000 },
+			{ 400000000, 480000000 }
 		},
 		.reg = { "csid_lite2" },
 		.interrupt = { "csid_lite2" },
@@ -3655,12 +3653,10 @@ static const struct camss_subdev_resources csid_res_8775p[] = {
 	/* CSID5 (lite) */
 	{
 		.regulators = {},
-		.clock = { "cpas_vfe_lite", "vfe_lite_ahb",
-			   "vfe_lite_csid", "vfe_lite_cphy_rx",
-			   "vfe_lite"},
+		.clock = { "vfe_lite_csid", "vfe_lite_cphy_rx" },
 		.clock_rate = {
-			{ 0, 0, 400000000, 400000000, 0},
-			{ 0, 0, 400000000, 480000000, 0}
+			{ 400000000, 480000000 },
+			{ 400000000, 480000000 }
 		},
 		.reg = { "csid_lite3" },
 		.interrupt = { "csid_lite3" },
@@ -3674,12 +3670,10 @@ static const struct camss_subdev_resources csid_res_8775p[] = {
 	/* CSID6 (lite) */
 	{
 		.regulators = {},
-		.clock = { "cpas_vfe_lite", "vfe_lite_ahb",
-			   "vfe_lite_csid", "vfe_lite_cphy_rx",
-			   "vfe_lite"},
+		.clock = { "vfe_lite_csid", "vfe_lite_cphy_rx" },
 		.clock_rate = {
-			{ 0, 0, 400000000, 400000000, 0},
-			{ 0, 0, 400000000, 480000000, 0}
+			{ 400000000, 480000000 },
+			{ 400000000, 480000000 }
 		},
 		.reg = { "csid_lite4" },
 		.interrupt = { "csid_lite4" },

-- 
2.34.1
Re: [PATCH 1/2] media: qcom: camss: Fix csid clock configuration and IRQ offset for 8775p
Posted by Bryan O'Donoghue 3 weeks, 4 days ago
On 13/03/2026 09:42, Wenmeng Liu wrote:
> Fix two issues in csid driver for 8775p platform:
> 
> 1. Simplify clock configuration for csid lite by removing unused clocks
>     and correcting clock rates. Only vfe_lite_csid and vfe_lite_cphy_rx
>     clocks are actually needed.

This should be its own patch and should that patch have a Fixes: ?

Simplification != fixing a bug.

> 2. Fix BUF_DONE_IRQ_STATUS_RDI_OFFSET calculation for csid lite on
>     sa8775p platform. The offset should be 0 for csid lite on sa8775p,
> 
> Fixes: ed03e99de0fa ("media: qcom: camss: Add support for CSID 690")
> Signed-off-by: Wenmeng Liu <wenmeng.liu@oss.qualcomm.com>

Yeah this should be standalone then.

My general rule here is if your patch requires line items to explain 
various things being done, then those line-items deserve their own patch.

---
bod
Re: [PATCH 1/2] media: qcom: camss: Fix csid clock configuration and IRQ offset for 8775p
Posted by Wenmeng Liu 3 weeks, 4 days ago
Hi Bryan,

On 3/13/2026 5:46 PM, Bryan O'Donoghue wrote:
> On 13/03/2026 09:42, Wenmeng Liu wrote:
>> Fix two issues in csid driver for 8775p platform:
>>
>> 1. Simplify clock configuration for csid lite by removing unused clocks
>>     and correcting clock rates. Only vfe_lite_csid and vfe_lite_cphy_rx
>>     clocks are actually needed.
> 
> This should be its own patch and should that patch have a Fixes: ?
> 
> Simplification != fixing a bug.

Sure, There is also an issue with the clock here, I will also modify the 
description.

> 
>> 2. Fix BUF_DONE_IRQ_STATUS_RDI_OFFSET calculation for csid lite on
>>     sa8775p platform. The offset should be 0 for csid lite on sa8775p,
>>
>> Fixes: ed03e99de0fa ("media: qcom: camss: Add support for CSID 690")
>> Signed-off-by: Wenmeng Liu <wenmeng.liu@oss.qualcomm.com>
> 
> Yeah this should be standalone then.
> 
> My general rule here is if your patch requires line items to explain 
> various things being done, then those line-items deserve their own patch.
ACK.
> 
> ---
> bod

Thanks,
Wenmeng