linux-kernel - Re: [PATCH 08/13] media: qcom: camss: csid: Move common code into csid core

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: <8c504081-b0e4-4a64-9e21-e7d040a0aa6c@linaro.org>
Date: Thu, 15 Aug 2024 00:53:17 +0100
From: Bryan O'Donoghue <bryan.odonoghue@...aro.org>
To: Depeng Shao <quic_depengs@...cinc.com>, rfoss@...nel.org,
 todor.too@...il.com, mchehab@...nel.org, robh@...nel.org,
 krzk+dt@...nel.org, conor+dt@...nel.org
Cc: linux-arm-msm@...r.kernel.org, linux-media@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, kernel@...cinc.com
Subject: Re: [PATCH 08/13] media: qcom: camss: csid: Move common code into
 csid core

On 12/08/2024 15:41, Depeng Shao wrote:
> The get hw version and src pad code functions can be common code in csid
> core file, then the csid driver of different hw version can reuse them,
> rather than adding duplicate code in csid driver for each version.
> 
> Suggested-by: Bryan O'Donoghue <bryan.odonoghue@...aro.org>
> Signed-off-by: Depeng Shao <quic_depengs@...cinc.com>
> ---
>   .../platform/qcom/camss/camss-csid-4-1.c      | 19 -----
>   .../platform/qcom/camss/camss-csid-4-7.c      | 42 ----------
>   .../platform/qcom/camss/camss-csid-gen2.c     | 60 ---------------
>   .../media/platform/qcom/camss/camss-csid.c    | 77 +++++++++++++++++++
>   .../media/platform/qcom/camss/camss-csid.h    | 21 +++++
>   5 files changed, 98 insertions(+), 121 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/camss/camss-csid-4-1.c b/drivers/media/platform/qcom/camss/camss-csid-4-1.c
> index c95861420502..6998e1c52895 100644
> --- a/drivers/media/platform/qcom/camss/camss-csid-4-1.c
> +++ b/drivers/media/platform/qcom/camss/camss-csid-4-1.c
> @@ -17,7 +17,6 @@
>   #include "camss-csid-gen1.h"
>   #include "camss.h"
>   
> -#define CAMSS_CSID_HW_VERSION		0x0
>   #define CAMSS_CSID_CORE_CTRL_0		0x004
>   #define CAMSS_CSID_CORE_CTRL_1		0x008
>   #define CAMSS_CSID_RST_CMD		0x00c
> @@ -139,15 +138,6 @@ static int csid_configure_testgen_pattern(struct csid_device *csid, s32 val)
>   	return 0;
>   }
>   
> -static u32 csid_hw_version(struct csid_device *csid)
> -{
> -	u32 hw_version = readl_relaxed(csid->base + CAMSS_CSID_HW_VERSION);
> -
> -	dev_dbg(csid->camss->dev, "CSID HW Version = 0x%08x\n", hw_version);
> -
> -	return hw_version;
> -}
>   
> -static u32 csid_hw_version(struct csid_device *csid)
> -{
> -	u32 hw_version = readl_relaxed(csid->base + CAMSS_CSID_HW_VERSION);
> -
> -	dev_dbg(csid->camss->dev, "CSID HW Version = 0x%08x\n", hw_version);
> -
> -	return hw_version;
> -}

Is it also the case with csid-4-1 and csid-47 that the HW version is in 
the format x.x.x - because you're removing this printout which just 
prints the register in favour of a later function that decodes that 
register into three parts.

Suggest having a csid_hw_version_gen1() for these two and a 
csid_hw_version_gen2() which does the x.x.x print instead.

=> camss-csid-4-1.c and camss-csid-4-7.c will have hw_version = 
csid_hw_version_gen1()

=> camss-csid-gen2, camss-csid-790 will have hw_version = hw_version_gen2()

---
bod