diff mbox series

[v3,01/15] media: qcom: camss: Amalgamate struct resource with struct resource_ispif

Message ID 20230823104444.1954663-2-bryan.odonoghue@linaro.org
State Superseded
Headers show
Series media: qcom: camss: Add parameter passing to remove several outstanding bugs | expand

Commit Message

Bryan O'Donoghue Aug. 23, 2023, 10:44 a.m. UTC
There is no good reason to differentiate the two resource structures
here. As part of a general tidyup of the declaration and passing of
resources within in the CAMSS driver it will be advantageous to have
one unified resource structure.

The two structures are very similar anyway thus leading more credence
still to the argument there should be only one.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/media/platform/qcom/camss/camss-ispif.c |  4 ++--
 drivers/media/platform/qcom/camss/camss-ispif.h |  4 ++--
 drivers/media/platform/qcom/camss/camss.c       | 14 +++++++-------
 drivers/media/platform/qcom/camss/camss.h       |  8 +-------
 4 files changed, 12 insertions(+), 18 deletions(-)

Comments

Konrad Dybcio Aug. 26, 2023, 9:55 a.m. UTC | #1
On 23.08.2023 12:44, Bryan O'Donoghue wrote:
> There is no good reason to differentiate the two resource structures
> here. As part of a general tidyup of the declaration and passing of
> resources within in the CAMSS driver it will be advantageous to have
> one unified resource structure.
> 
> The two structures are very similar anyway thus leading more credence
> still to the argument there should be only one.
> 
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
Acked-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Konrad
Laurent Pinchart Aug. 28, 2023, 5:30 p.m. UTC | #2
Hi Bryan,

Thank you for the patch.

On Wed, Aug 23, 2023 at 11:44:30AM +0100, Bryan O'Donoghue wrote:
> There is no good reason to differentiate the two resource structures
> here. As part of a general tidyup of the declaration and passing of
> resources within in the CAMSS driver it will be advantageous to have
> one unified resource structure.
> 
> The two structures are very similar anyway thus leading more credence
> still to the argument there should be only one.
> 
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>  drivers/media/platform/qcom/camss/camss-ispif.c |  4 ++--
>  drivers/media/platform/qcom/camss/camss-ispif.h |  4 ++--
>  drivers/media/platform/qcom/camss/camss.c       | 14 +++++++-------
>  drivers/media/platform/qcom/camss/camss.h       |  8 +-------
>  4 files changed, 12 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/camss/camss-ispif.c b/drivers/media/platform/qcom/camss/camss-ispif.c
> index 1c7e4b1b49401..61765b874b9a2 100644
> --- a/drivers/media/platform/qcom/camss/camss-ispif.c
> +++ b/drivers/media/platform/qcom/camss/camss-ispif.c
> @@ -1095,7 +1095,7 @@ static int ispif_init_formats(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
>   * Return 0 on success or a negative error code otherwise
>   */
>  int msm_ispif_subdev_init(struct camss *camss,
> -			  const struct resources_ispif *res)
> +			  const struct resources *res)
>  {
>  	struct device *dev = camss->dev;
>  	struct ispif_device *ispif = camss->ispif;
> @@ -1152,7 +1152,7 @@ int msm_ispif_subdev_init(struct camss *camss,
>  
>  	/* Interrupt */
>  
> -	ret = platform_get_irq_byname(pdev, res->interrupt);
> +	ret = platform_get_irq_byname(pdev, res->interrupt[0]);
>  	if (ret < 0)
>  		return ret;
>  
> diff --git a/drivers/media/platform/qcom/camss/camss-ispif.h b/drivers/media/platform/qcom/camss/camss-ispif.h
> index fdf28e68cc7d8..c7c41f7afcaad 100644
> --- a/drivers/media/platform/qcom/camss/camss-ispif.h
> +++ b/drivers/media/platform/qcom/camss/camss-ispif.h
> @@ -66,10 +66,10 @@ struct ispif_device {
>  	struct camss *camss;
>  };
>  
> -struct resources_ispif;
> +struct resources;
>  
>  int msm_ispif_subdev_init(struct camss *camss,
> -			  const struct resources_ispif *res);
> +			  const struct resources *res);
>  
>  int msm_ispif_register_entities(struct ispif_device *ispif,
>  				struct v4l2_device *v4l2_dev);
> diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
> index f4948bdf3f8f9..de39dc987444f 100644
> --- a/drivers/media/platform/qcom/camss/camss.c
> +++ b/drivers/media/platform/qcom/camss/camss.c
> @@ -93,14 +93,14 @@ static const struct resources csid_res_8x16[] = {
>  	},
>  };
>  
> -static const struct resources_ispif ispif_res_8x16 = {
> +static const struct resources ispif_res_8x16 = {
>  	/* ISPIF */
>  	.clock = { "top_ahb", "ahb", "ispif_ahb",
>  		   "csi0", "csi0_pix", "csi0_rdi",
>  		   "csi1", "csi1_pix", "csi1_rdi" },
>  	.clock_for_reset = { "vfe0", "csi_vfe0" },
>  	.reg = { "ispif", "csi_clk_mux" },
> -	.interrupt = "ispif"
> +	.interrupt = { "ispif" }
>  
>  };
>  
> @@ -234,7 +234,7 @@ static const struct resources csid_res_8x96[] = {
>  	}
>  };
>  
> -static const struct resources_ispif ispif_res_8x96 = {
> +static const struct resources ispif_res_8x96 = {
>  	/* ISPIF */
>  	.clock = { "top_ahb", "ahb", "ispif_ahb",
>  		   "csi0", "csi0_pix", "csi0_rdi",
> @@ -243,7 +243,7 @@ static const struct resources_ispif ispif_res_8x96 = {
>  		   "csi3", "csi3_pix", "csi3_rdi" },
>  	.clock_for_reset = { "vfe0", "csi_vfe0", "vfe1", "csi_vfe1" },
>  	.reg = { "ispif", "csi_clk_mux" },
> -	.interrupt = "ispif"
> +	.interrupt = { "ispif" }
>  };
>  
>  static const struct resources vfe_res_8x96[] = {
> @@ -410,7 +410,7 @@ static const struct resources csid_res_660[] = {
>  	}
>  };
>  
> -static const struct resources_ispif ispif_res_660 = {
> +static const struct resources ispif_res_660 = {
>  	/* ISPIF */
>  	.clock = { "top_ahb", "ahb", "ispif_ahb",
>  		   "csi0", "csi0_pix", "csi0_rdi",
> @@ -419,7 +419,7 @@ static const struct resources_ispif ispif_res_660 = {
>  		   "csi3", "csi3_pix", "csi3_rdi" },
>  	.clock_for_reset = { "vfe0", "csi_vfe0", "vfe1", "csi_vfe1" },
>  	.reg = { "ispif", "csi_clk_mux" },
> -	.interrupt = "ispif"
> +	.interrupt = { "ispif" }
>  };
>  
>  static const struct resources vfe_res_660[] = {
> @@ -1122,7 +1122,7 @@ static int camss_init_subdevices(struct camss *camss)
>  {
>  	const struct resources *csiphy_res;
>  	const struct resources *csid_res;
> -	const struct resources_ispif *ispif_res;
> +	const struct resources *ispif_res;
>  	const struct resources *vfe_res;
>  	unsigned int i;
>  	int ret;
> diff --git a/drivers/media/platform/qcom/camss/camss.h b/drivers/media/platform/qcom/camss/camss.h
> index f6c326cb853b8..e95211cdb1fd6 100644
> --- a/drivers/media/platform/qcom/camss/camss.h
> +++ b/drivers/media/platform/qcom/camss/camss.h
> @@ -44,18 +44,12 @@
>  struct resources {

While at it, or in a separate patch, could you rename this to
camss_resources ? resources is a too generic name, too likely to clash.
With this fixed,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  	char *regulators[CAMSS_RES_MAX];
>  	char *clock[CAMSS_RES_MAX];
> +	char *clock_for_reset[CAMSS_RES_MAX];
>  	u32 clock_rate[CAMSS_RES_MAX][CAMSS_RES_MAX];
>  	char *reg[CAMSS_RES_MAX];
>  	char *interrupt[CAMSS_RES_MAX];
>  };
>  
> -struct resources_ispif {
> -	char *clock[CAMSS_RES_MAX];
> -	char *clock_for_reset[CAMSS_RES_MAX];
> -	char *reg[CAMSS_RES_MAX];
> -	char *interrupt;
> -};
> -
>  struct icc_bw_tbl {
>  	u32 avg;
>  	u32 peak;
diff mbox series

Patch

diff --git a/drivers/media/platform/qcom/camss/camss-ispif.c b/drivers/media/platform/qcom/camss/camss-ispif.c
index 1c7e4b1b49401..61765b874b9a2 100644
--- a/drivers/media/platform/qcom/camss/camss-ispif.c
+++ b/drivers/media/platform/qcom/camss/camss-ispif.c
@@ -1095,7 +1095,7 @@  static int ispif_init_formats(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
  * Return 0 on success or a negative error code otherwise
  */
 int msm_ispif_subdev_init(struct camss *camss,
-			  const struct resources_ispif *res)
+			  const struct resources *res)
 {
 	struct device *dev = camss->dev;
 	struct ispif_device *ispif = camss->ispif;
@@ -1152,7 +1152,7 @@  int msm_ispif_subdev_init(struct camss *camss,
 
 	/* Interrupt */
 
-	ret = platform_get_irq_byname(pdev, res->interrupt);
+	ret = platform_get_irq_byname(pdev, res->interrupt[0]);
 	if (ret < 0)
 		return ret;
 
diff --git a/drivers/media/platform/qcom/camss/camss-ispif.h b/drivers/media/platform/qcom/camss/camss-ispif.h
index fdf28e68cc7d8..c7c41f7afcaad 100644
--- a/drivers/media/platform/qcom/camss/camss-ispif.h
+++ b/drivers/media/platform/qcom/camss/camss-ispif.h
@@ -66,10 +66,10 @@  struct ispif_device {
 	struct camss *camss;
 };
 
-struct resources_ispif;
+struct resources;
 
 int msm_ispif_subdev_init(struct camss *camss,
-			  const struct resources_ispif *res);
+			  const struct resources *res);
 
 int msm_ispif_register_entities(struct ispif_device *ispif,
 				struct v4l2_device *v4l2_dev);
diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index f4948bdf3f8f9..de39dc987444f 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -93,14 +93,14 @@  static const struct resources csid_res_8x16[] = {
 	},
 };
 
-static const struct resources_ispif ispif_res_8x16 = {
+static const struct resources ispif_res_8x16 = {
 	/* ISPIF */
 	.clock = { "top_ahb", "ahb", "ispif_ahb",
 		   "csi0", "csi0_pix", "csi0_rdi",
 		   "csi1", "csi1_pix", "csi1_rdi" },
 	.clock_for_reset = { "vfe0", "csi_vfe0" },
 	.reg = { "ispif", "csi_clk_mux" },
-	.interrupt = "ispif"
+	.interrupt = { "ispif" }
 
 };
 
@@ -234,7 +234,7 @@  static const struct resources csid_res_8x96[] = {
 	}
 };
 
-static const struct resources_ispif ispif_res_8x96 = {
+static const struct resources ispif_res_8x96 = {
 	/* ISPIF */
 	.clock = { "top_ahb", "ahb", "ispif_ahb",
 		   "csi0", "csi0_pix", "csi0_rdi",
@@ -243,7 +243,7 @@  static const struct resources_ispif ispif_res_8x96 = {
 		   "csi3", "csi3_pix", "csi3_rdi" },
 	.clock_for_reset = { "vfe0", "csi_vfe0", "vfe1", "csi_vfe1" },
 	.reg = { "ispif", "csi_clk_mux" },
-	.interrupt = "ispif"
+	.interrupt = { "ispif" }
 };
 
 static const struct resources vfe_res_8x96[] = {
@@ -410,7 +410,7 @@  static const struct resources csid_res_660[] = {
 	}
 };
 
-static const struct resources_ispif ispif_res_660 = {
+static const struct resources ispif_res_660 = {
 	/* ISPIF */
 	.clock = { "top_ahb", "ahb", "ispif_ahb",
 		   "csi0", "csi0_pix", "csi0_rdi",
@@ -419,7 +419,7 @@  static const struct resources_ispif ispif_res_660 = {
 		   "csi3", "csi3_pix", "csi3_rdi" },
 	.clock_for_reset = { "vfe0", "csi_vfe0", "vfe1", "csi_vfe1" },
 	.reg = { "ispif", "csi_clk_mux" },
-	.interrupt = "ispif"
+	.interrupt = { "ispif" }
 };
 
 static const struct resources vfe_res_660[] = {
@@ -1122,7 +1122,7 @@  static int camss_init_subdevices(struct camss *camss)
 {
 	const struct resources *csiphy_res;
 	const struct resources *csid_res;
-	const struct resources_ispif *ispif_res;
+	const struct resources *ispif_res;
 	const struct resources *vfe_res;
 	unsigned int i;
 	int ret;
diff --git a/drivers/media/platform/qcom/camss/camss.h b/drivers/media/platform/qcom/camss/camss.h
index f6c326cb853b8..e95211cdb1fd6 100644
--- a/drivers/media/platform/qcom/camss/camss.h
+++ b/drivers/media/platform/qcom/camss/camss.h
@@ -44,18 +44,12 @@ 
 struct resources {
 	char *regulators[CAMSS_RES_MAX];
 	char *clock[CAMSS_RES_MAX];
+	char *clock_for_reset[CAMSS_RES_MAX];
 	u32 clock_rate[CAMSS_RES_MAX][CAMSS_RES_MAX];
 	char *reg[CAMSS_RES_MAX];
 	char *interrupt[CAMSS_RES_MAX];
 };
 
-struct resources_ispif {
-	char *clock[CAMSS_RES_MAX];
-	char *clock_for_reset[CAMSS_RES_MAX];
-	char *reg[CAMSS_RES_MAX];
-	char *interrupt;
-};
-
 struct icc_bw_tbl {
 	u32 avg;
 	u32 peak;