All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Hans Verkuil <hverkuil@xs4all.nl>
To: Robert Foss <robert.foss@linaro.org>,
	agross@kernel.org, bjorn.andersson@linaro.org,
	todor.too@gmail.com, mchehab@kernel.org, robh+dt@kernel.org,
	angelogioacchino.delregno@somainline.org,
	linux-arm-msm@vger.kernel.org, linux-media@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	AngeloGioacchino Del Regno <kholk11@gmail.com>,
	Sakari Ailus <sakari.ailus@iki.fi>,
	Andrey Konovalov <andrey.konovalov@linaro.org>
Cc: Rob Herring <robh@kernel.org>, Tomasz Figa <tfiga@chromium.org>,
	Azam Sadiq Pasha Kapatrala Syed <akapatra@quicinc.com>,
	Sarvesh Sridutt <Sarvesh.Sridutt@smartwirelesscompute.com>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Jonathan Marek <jonathan@marek.ca>
Subject: Re: [PATCH v8 09/22] media: camss: Refactor CSID HW version support
Date: Tue, 16 Mar 2021 10:36:49 +0100	[thread overview]
Message-ID: <b06ce7af-4449-fb5c-2920-09ebd5abdf75@xs4all.nl> (raw)
In-Reply-To: <20210315155942.640889-10-robert.foss@linaro.org>

On 15/03/2021 16:59, Robert Foss wrote:
> In order to support Qualcomm ISP hardware architectures that diverge
> from older architectures, the CSID subdevice drivers needs to be refactored
> to better abstract the different ISP hardware architectures.
> 
> Signed-off-by: Robert Foss <robert.foss@linaro.org>
> Reviewed-by: Andrey Konovalov <andrey.konovalov@linaro.org>
> ---
> 
> 
> Changes since v1:
>  - kernel test robot: Add missing include, interrupt.h
> 
> Changes since v4:
>  - Andrey: Removed whitespace from some includes
>  - Andrey: Removed unused enum
> 
> Changes since v5:
>  - Andrey: Fixed test pattern selection logic
>  - Andrey: Align test mode enum values with v4l mode selection return values
>  - Andrey: r-b
>  - Move Titan 170 test modes to the the Titan 170 commit
>  - Fixed test pattern boundary check
> 
> Changes since v7:
>  - Hans: Fix checkpatch.pl --strict warnings
> 
> 
> 
>  drivers/media/platform/qcom/camss/Makefile    |   2 +
>  .../platform/qcom/camss/camss-csid-4-1.c      | 328 ++++++++++
>  .../platform/qcom/camss/camss-csid-4-7.c      | 404 ++++++++++++
>  .../media/platform/qcom/camss/camss-csid.c    | 608 +-----------------
>  .../media/platform/qcom/camss/camss-csid.h    | 129 +++-
>  5 files changed, 885 insertions(+), 586 deletions(-)
>  create mode 100644 drivers/media/platform/qcom/camss/camss-csid-4-1.c
>  create mode 100644 drivers/media/platform/qcom/camss/camss-csid-4-7.c
> 

<snip>

> diff --git a/drivers/media/platform/qcom/camss/camss-csid.h b/drivers/media/platform/qcom/camss/camss-csid.h
> index 479ac1f83836..613ef377b051 100644
> --- a/drivers/media/platform/qcom/camss/camss-csid.h
> +++ b/drivers/media/platform/qcom/camss/camss-csid.h
> @@ -11,6 +11,7 @@
>  #define QC_MSM_CAMSS_CSID_H
>  
>  #include <linux/clk.h>
> +#include <linux/interrupt.h>
>  #include <media/media-entity.h>
>  #include <media/v4l2-ctrls.h>
>  #include <media/v4l2-device.h>
> @@ -44,18 +45,42 @@
>  #define DATA_TYPE_RAW_16BIT		0x2e
>  #define DATA_TYPE_RAW_20BIT		0x2f
>  
> -enum csid_payload_mode {
> -	CSID_PAYLOAD_MODE_INCREMENTING = 0,
> -	CSID_PAYLOAD_MODE_ALTERNATING_55_AA = 1,
> -	CSID_PAYLOAD_MODE_ALL_ZEROES = 2,
> -	CSID_PAYLOAD_MODE_ALL_ONES = 3,
> -	CSID_PAYLOAD_MODE_RANDOM = 4,
> -	CSID_PAYLOAD_MODE_USER_SPECIFIED = 5,
> +#define CSID_RESET_TIMEOUT_MS 500
> +
> +enum csid_testgen_mode {
> +	CSID_PAYLOAD_MODE_DISABLED = 0,
> +	CSID_PAYLOAD_MODE_INCREMENTING = 1,
> +	CSID_PAYLOAD_MODE_ALTERNATING_55_AA = 2,
> +	CSID_PAYLOAD_MODE_ALL_ZEROES = 3,
> +	CSID_PAYLOAD_MODE_ALL_ONES = 4,
> +	CSID_PAYLOAD_MODE_RANDOM = 5,
> +	CSID_PAYLOAD_MODE_USER_SPECIFIED = 6,
> +	CSID_PAYLOAD_MODE_NUM_SUPPORTED_GEN1 = 6, /* excluding disabled */
> +};
> +
> +static const char * const csid_testgen_modes[] = {
> +	"Disabled",
> +	"Incrementing",
> +	"Alternating 0x55/0xAA",
> +	"All Zeros 0x00",
> +	"All Ones 0xFF",
> +	"Pseudo-random Data",
> +	"User Specified",
> +};

This gives this sparse warning:

'csid_testgen_modes' defined but not used [-Wunused-const-variable=]

This array needs to be moved to camss-csid.c and declared as an extern
here. Also, this menu array needs to be terminated with a NULL, and the
right capitalization needs to be used (first character of each word must
be a capital). This is a suggested patch I made to verify that this solves
this issue, but really both patch 9 and 10 need to be modified.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/platform/qcom/camss/camss-csid.c | 14 ++++++++++++++
 drivers/media/platform/qcom/camss/camss-csid.h | 13 +------------
 2 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/drivers/media/platform/qcom/camss/camss-csid.c b/drivers/media/platform/qcom/camss/camss-csid.c
index fb94dc03ccd4..1513b3d47fc2 100644
--- a/drivers/media/platform/qcom/camss/camss-csid.c
+++ b/drivers/media/platform/qcom/camss/camss-csid.c
@@ -27,6 +27,20 @@

 #define MSM_CSID_NAME "msm_csid"

+const char * const csid_testgen_modes[] = {
+	"Disabled",
+	"Incrementing",
+	"Alternating 0x55/0xAA",
+	"All Zeros 0x00",
+	"All Ones 0xFF",
+	"Pseudo-Random Data",
+	"User Specified",
+	"Complex Pattern",
+	"Color Box",
+	"Color Bars",
+	NULL
+};
+
 u32 csid_find_code(u32 *codes, unsigned int ncodes,
 		   unsigned int match_format_idx, u32 match_code)
 {
diff --git a/drivers/media/platform/qcom/camss/camss-csid.h b/drivers/media/platform/qcom/camss/camss-csid.h
index c2a025f6846b..81a3704ac0e3 100644
--- a/drivers/media/platform/qcom/camss/camss-csid.h
+++ b/drivers/media/platform/qcom/camss/camss-csid.h
@@ -62,18 +62,7 @@ enum csid_testgen_mode {
 	CSID_PAYLOAD_MODE_NUM_SUPPORTED_GEN2 = 9, /* excluding disabled */
 };

-static const char * const csid_testgen_modes[] = {
-	"Disabled",
-	"Incrementing",
-	"Alternating 0x55/0xAA",
-	"All Zeros 0x00",
-	"All Ones 0xFF",
-	"Pseudo-random Data",
-	"User Specified",
-	"Complex pattern",
-	"Color box",
-	"Color bars",
-};
+extern const char * const csid_testgen_modes[];

 struct csid_format {
 	u32 code;
-- 
2.30.1

Regards,

	Hans

  reply	other threads:[~2021-03-16  9:37 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-15 15:59 [PATCH v8 00/22] Add support for the SDM845 Camera Subsystem Robert Foss
2021-03-15 15:59 ` [PATCH v8 01/22] media: camss: Fix vfe_isr_comp_done() documentation Robert Foss
2021-03-15 15:59 ` [PATCH v8 02/22] media: camss: Fix vfe_isr comment typo Robert Foss
2021-03-15 15:59 ` [PATCH v8 03/22] media: camss: Replace trace_printk() with dev_dbg() Robert Foss
2021-03-15 15:59 ` [PATCH v8 04/22] media: camss: Add CAMSS_845 camss version Robert Foss
2021-03-15 15:59 ` [PATCH v8 05/22] media: camss: Make ISPIF subdevice optional Robert Foss
2021-03-15 15:59 ` [PATCH v8 06/22] media: camss: Refactor VFE HW version support Robert Foss
2021-03-16  9:40   ` Hans Verkuil
2021-03-16 11:02     ` Robert Foss
2021-03-15 15:59 ` [PATCH v8 07/22] media: camss: Add support for VFE hardware version Titan 170 Robert Foss
2021-03-15 15:59 ` [PATCH v8 08/22] media: camss: Add missing format identifiers Robert Foss
2021-03-15 15:59 ` [PATCH v8 09/22] media: camss: Refactor CSID HW version support Robert Foss
2021-03-16  9:36   ` Hans Verkuil [this message]
2021-03-16 11:12     ` Robert Foss
2021-03-15 15:59 ` [PATCH v8 10/22] media: camss: Add support for CSID hardware version Titan 170 Robert Foss
2021-03-15 15:59 ` [PATCH v8 11/22] media: camss: Add support for CSIPHY " Robert Foss
2021-03-15 15:59 ` [PATCH v8 12/22] media: camss: Refactor VFE power domain toggling Robert Foss
2021-03-15 15:59 ` [PATCH v8 13/22] media: camss: Enable SDM845 Robert Foss
2021-03-15 15:59 ` [PATCH v8 14/22] dt-bindings: media: camss: Add qcom,msm8916-camss binding Robert Foss
2021-03-15 15:59 ` [PATCH v8 15/22] dt-bindings: media: camss: Add qcom,msm8996-camss binding Robert Foss
2021-03-15 15:59 ` [PATCH v8 16/22] dt-bindings: media: camss: Add qcom,sdm660-camss binding Robert Foss
2021-03-15 15:59 ` [PATCH v8 17/22] dt-bindings: media: camss: Add qcom,sdm845-camss binding Robert Foss
2021-03-15 15:59 ` [PATCH v8 18/22] MAINTAINERS: Change CAMSS documentation to use dtschema bindings Robert Foss
2021-03-15 15:59 ` [PATCH v8 19/22] media: dt-bindings: media: Remove qcom,camss documentation Robert Foss
2021-03-15 15:59 ` [PATCH v8 20/22] arm64: dts: sdm845: Add CAMSS ISP node Robert Foss
2021-03-15 15:59 ` [PATCH v8 21/22] arm64: dts: sdm845-db845c: Configure regulators for camss node Robert Foss
2021-03-15 15:59 ` [PATCH v8 22/22] arm64: dts: sdm845-db845c: Enable ov8856 sensor and connect to ISP Robert Foss

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b06ce7af-4449-fb5c-2920-09ebd5abdf75@xs4all.nl \
    --to=hverkuil@xs4all.nl \
    --cc=Sarvesh.Sridutt@smartwirelesscompute.com \
    --cc=agross@kernel.org \
    --cc=akapatra@quicinc.com \
    --cc=andrey.konovalov@linaro.org \
    --cc=angelogioacchino.delregno@somainline.org \
    --cc=bjorn.andersson@linaro.org \
    --cc=devicetree@vger.kernel.org \
    --cc=jonathan@marek.ca \
    --cc=kholk11@gmail.com \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=robert.foss@linaro.org \
    --cc=robh+dt@kernel.org \
    --cc=robh@kernel.org \
    --cc=sakari.ailus@iki.fi \
    --cc=tfiga@chromium.org \
    --cc=todor.too@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.