[PATCH v2 09/18] media: rzg2l-cru: csi2: Make system clock optional for RZ/V2H(P) SoC

Tommaso Merciai posted 18 patches 9 months, 4 weeks ago
There is a newer version of this series
[PATCH v2 09/18] media: rzg2l-cru: csi2: Make system clock optional for RZ/V2H(P) SoC
Posted by Tommaso Merciai 9 months, 4 weeks ago
From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

The RZ/V2H(P) SoC does not provide a `system` clock for the CSI-2
interface. To accommodate this, use `devm_clk_get_optional()` instead
of `devm_clk_get()` when retrieving the clock.

This patch is in preparation for adding support for RZ/V2H(P) SoC.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
 drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
index 3a4e720ba732..771fa35558be 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
@@ -796,7 +796,7 @@ static int rzg2l_csi2_probe(struct platform_device *pdev)
 		return dev_err_probe(dev, PTR_ERR(csi2->presetn),
 				     "Failed to get cpg presetn\n");
 
-	csi2->sysclk = devm_clk_get(dev, "system");
+	csi2->sysclk = devm_clk_get_optional(dev, "system");
 	if (IS_ERR(csi2->sysclk))
 		return dev_err_probe(dev, PTR_ERR(csi2->sysclk),
 				     "Failed to get system clk\n");
-- 
2.34.1
Re: [PATCH v2 09/18] media: rzg2l-cru: csi2: Make system clock optional for RZ/V2H(P) SoC
Posted by Laurent Pinchart 9 months, 3 weeks ago
Hi Tommaso,

Thank you for the patch.

On Fri, Feb 21, 2025 at 04:55:23PM +0100, Tommaso Merciai wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> The RZ/V2H(P) SoC does not provide a `system` clock for the CSI-2
> interface. To accommodate this, use `devm_clk_get_optional()` instead
> of `devm_clk_get()` when retrieving the clock.

The clock shouldn't be optional. On all SoCs but V2H it should remain
mandatory, and on V2H you shouldn't call clk_get() at all.

I'd recommend adding a flag to the rzg2l_csi2_info structure.

> This patch is in preparation for adding support for RZ/V2H(P) SoC.
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> ---
>  drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
> index 3a4e720ba732..771fa35558be 100644
> --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
> +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
> @@ -796,7 +796,7 @@ static int rzg2l_csi2_probe(struct platform_device *pdev)
>  		return dev_err_probe(dev, PTR_ERR(csi2->presetn),
>  				     "Failed to get cpg presetn\n");
>  
> -	csi2->sysclk = devm_clk_get(dev, "system");
> +	csi2->sysclk = devm_clk_get_optional(dev, "system");
>  	if (IS_ERR(csi2->sysclk))
>  		return dev_err_probe(dev, PTR_ERR(csi2->sysclk),
>  				     "Failed to get system clk\n");

-- 
Regards,

Laurent Pinchart
Re: [PATCH v2 09/18] media: rzg2l-cru: csi2: Make system clock optional for RZ/V2H(P) SoC
Posted by Lad, Prabhakar 9 months, 3 weeks ago
Hi Laurent,

Thank you for the review.

On Sun, Feb 23, 2025 at 6:20 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Tommaso,
>
> Thank you for the patch.
>
> On Fri, Feb 21, 2025 at 04:55:23PM +0100, Tommaso Merciai wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > The RZ/V2H(P) SoC does not provide a `system` clock for the CSI-2
> > interface. To accommodate this, use `devm_clk_get_optional()` instead
> > of `devm_clk_get()` when retrieving the clock.
>
> The clock shouldn't be optional. On all SoCs but V2H it should remain
> mandatory, and on V2H you shouldn't call clk_get() at all.
>
> I'd recommend adding a flag to the rzg2l_csi2_info structure.
>
Agreed, in this case dtbs_check will not catch this issue if we pass
"video" and "apb" clocks.

Cheers,
Prabhakar