Re: [PATCH v2 1/1] media: qcom: camss: Restructure camss_link_entities

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12/11/2024 13:38, Vikram Sharma wrote:
Refactor the camss_link_entities function by breaking it down into
three distinct functions. Each function will handle the linking of
a specific entity separately, enhancing readability.

Signed-off-by: Suresh Vankadara <quic_svankada@xxxxxxxxxxx>
Signed-off-by: Trishansh Bhardwaj <quic_tbhardwa@xxxxxxxxxxx>
Signed-off-by: Vikram Sharma <quic_vikramsa@xxxxxxxxxxx>
---
  drivers/media/platform/qcom/camss/camss-vfe.c |   6 +-
  drivers/media/platform/qcom/camss/camss.c     | 196 ++++++++++++------
  drivers/media/platform/qcom/camss/camss.h     |   4 +
  3 files changed, 138 insertions(+), 68 deletions(-)

diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c
index 83c5a36d071f..446604cc7ef6 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe.c
@@ -1794,9 +1794,9 @@ int msm_vfe_register_entities(struct vfe_device *vfe,
  				&video_out->vdev.entity, 0,
  				MEDIA_LNK_FL_IMMUTABLE | MEDIA_LNK_FL_ENABLED);
  		if (ret < 0) {
-			dev_err(dev, "Failed to link %s->%s entities: %d\n",
-				sd->entity.name, video_out->vdev.entity.name,
-				ret);
+			camss_link_err(vfe->camss, sd->entity.name,
+				       video_out->vdev.entity.name,
+				       ret);

So you're doing the right thing reusing camss_link_err here however

1. The commit log no-longer matches
2. I generally suggest patches should be as granular as possible
3. That means if you want to use camss_link_err in camss-vfe.c
   and BTW I think that's, correct then

a) Refactor this to be two patches
b) First patch is about reducing the repitious string and introducing
   the reduction in camss.c and camss-vfe.c
c) The second patch is about restructiring link_entities in camss.c

Basically this patch now does two things and instead of havin those two things contained in the one patch, you should split those two things into two separate patches.

---
bod




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux