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@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxxx>
> 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;
--
Regards,
Laurent Pinchart
