linux-kernel - Re: [PATCH v2 1/1] media: qcom: camss: Restructure camss_link

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1cefe6f0-5d08-41a4-b0c6-2291dd5781ca@linaro.org>
Date: Tue, 12 Nov 2024 23:34:18 +0000
From: Bryan O'Donoghue <bryan.odonoghue@...aro.org>
To: Vikram Sharma <quic_vikramsa@...cinc.com>, rfoss@...nel.org,
 todor.too@...il.com, krzk+dt@...nel.org
Cc: linux-media@...r.kernel.org, linux-arm-msm@...r.kernel.org,
 linux-kernel@...r.kernel.org, kernel@...cinc.com
Subject: Re: [PATCH v2 1/1] media: qcom: camss: Restructure
 camss_link_entities

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@...cinc.com>
> Signed-off-by: Trishansh Bhardwaj <quic_tbhardwa@...cinc.com>
> Signed-off-by: Vikram Sharma <quic_vikramsa@...cinc.com>
> ---
>   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