Hi Niklas,
Thank you for the patch.
On Mon, Apr 21, 2025 at 01:12:39PM +0200, Niklas Söderlund wrote:
> Prepare for extending the driver to in addition to support the channel
s/support/supporting/
> selector (CS) also support the core ISP. The two different functions
> have different base addresses so the driver needs to distinguish between
> them.
>
> Prepare for this by marking existing base address variable and
> read/write functions to make it clear it operates on the CS portion of
s/it operates/they operate/
> the driver. There is no functional change.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>
> Reviewed-by: Jacopo Mondi <jacopo.mondi@xxxxxxxxxxxxxxxx>
> ---
> .../media/platform/renesas/rcar-isp/csisp.c | 46 +++++++++----------
> 1 file changed, 23 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/media/platform/renesas/rcar-isp/csisp.c b/drivers/media/platform/renesas/rcar-isp/csisp.c
> index 4bc89d4757fa..f36d43c2e0a2 100644
> --- a/drivers/media/platform/renesas/rcar-isp/csisp.c
> +++ b/drivers/media/platform/renesas/rcar-isp/csisp.c
> @@ -159,7 +159,7 @@ enum rcar_isp_pads {
>
> struct rcar_isp {
> struct device *dev;
> - void __iomem *base;
> + void __iomem *csbase;
> struct reset_control *rstc;
>
> enum rcar_isp_input csi_input;
> @@ -184,14 +184,14 @@ static inline struct rcar_isp *notifier_to_isp(struct v4l2_async_notifier *n)
> return container_of(n, struct rcar_isp, notifier);
> }
>
> -static void risp_write(struct rcar_isp *isp, u32 offset, u32 value)
> +static void risp_write_cs(struct rcar_isp *isp, u32 offset, u32 value)
Not sure what the other write function will be called, but I would have
called this risp_cs_write(). Up to you.
Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> {
> - iowrite32(value, isp->base + offset);
> + iowrite32(value, isp->csbase + offset);
> }
>
> -static u32 risp_read(struct rcar_isp *isp, u32 offset)
> +static u32 risp_read_cs(struct rcar_isp *isp, u32 offset)
> {
> - return ioread32(isp->base + offset);
> + return ioread32(isp->csbase + offset);
> }
>
> static int risp_power_on(struct rcar_isp *isp)
> @@ -245,31 +245,31 @@ static int risp_start(struct rcar_isp *isp, struct v4l2_subdev_state *state)
> if (isp->csi_input == RISP_CSI_INPUT1)
> sel_csi = ISPINPUTSEL0_SEL_CSI0;
>
> - risp_write(isp, ISPINPUTSEL0_REG,
> - risp_read(isp, ISPINPUTSEL0_REG) | sel_csi);
> + risp_write_cs(isp, ISPINPUTSEL0_REG,
> + risp_read_cs(isp, ISPINPUTSEL0_REG) | sel_csi);
>
> /* Configure Channel Selector. */
> for (vc = 0; vc < 4; vc++) {
> u8 ch = vc + 4;
> u8 dt = format->datatype;
>
> - risp_write(isp, ISPCS_FILTER_ID_CH_REG(ch), BIT(vc));
> - risp_write(isp, ISPCS_DT_CODE03_CH_REG(ch),
> - ISPCS_DT_CODE03_EN3 | ISPCS_DT_CODE03_DT3(dt) |
> - ISPCS_DT_CODE03_EN2 | ISPCS_DT_CODE03_DT2(dt) |
> - ISPCS_DT_CODE03_EN1 | ISPCS_DT_CODE03_DT1(dt) |
> - ISPCS_DT_CODE03_EN0 | ISPCS_DT_CODE03_DT0(dt));
> + risp_write_cs(isp, ISPCS_FILTER_ID_CH_REG(ch), BIT(vc));
> + risp_write_cs(isp, ISPCS_DT_CODE03_CH_REG(ch),
> + ISPCS_DT_CODE03_EN3 | ISPCS_DT_CODE03_DT3(dt) |
> + ISPCS_DT_CODE03_EN2 | ISPCS_DT_CODE03_DT2(dt) |
> + ISPCS_DT_CODE03_EN1 | ISPCS_DT_CODE03_DT1(dt) |
> + ISPCS_DT_CODE03_EN0 | ISPCS_DT_CODE03_DT0(dt));
> }
>
> /* Setup processing method. */
> - risp_write(isp, ISPPROCMODE_DT_REG(format->datatype),
> - ISPPROCMODE_DT_PROC_MODE_VC3(format->procmode) |
> - ISPPROCMODE_DT_PROC_MODE_VC2(format->procmode) |
> - ISPPROCMODE_DT_PROC_MODE_VC1(format->procmode) |
> - ISPPROCMODE_DT_PROC_MODE_VC0(format->procmode));
> + risp_write_cs(isp, ISPPROCMODE_DT_REG(format->datatype),
> + ISPPROCMODE_DT_PROC_MODE_VC3(format->procmode) |
> + ISPPROCMODE_DT_PROC_MODE_VC2(format->procmode) |
> + ISPPROCMODE_DT_PROC_MODE_VC1(format->procmode) |
> + ISPPROCMODE_DT_PROC_MODE_VC0(format->procmode));
>
> /* Start ISP. */
> - risp_write(isp, ISPSTART_REG, ISPSTART_START);
> + risp_write_cs(isp, ISPSTART_REG, ISPSTART_START);
>
> ret = v4l2_subdev_enable_streams(isp->remote, isp->remote_pad,
> BIT_ULL(0));
> @@ -284,7 +284,7 @@ static void risp_stop(struct rcar_isp *isp)
> v4l2_subdev_disable_streams(isp->remote, isp->remote_pad, BIT_ULL(0));
>
> /* Stop ISP. */
> - risp_write(isp, ISPSTART_REG, ISPSTART_STOP);
> + risp_write_cs(isp, ISPSTART_REG, ISPSTART_STOP);
>
> risp_power_off(isp);
> }
> @@ -465,9 +465,9 @@ static const struct media_entity_operations risp_entity_ops = {
> static int risp_probe_resources(struct rcar_isp *isp,
> struct platform_device *pdev)
> {
> - isp->base = devm_platform_get_and_ioremap_resource(pdev, 0, NULL);
> - if (IS_ERR(isp->base))
> - return PTR_ERR(isp->base);
> + isp->csbase = devm_platform_get_and_ioremap_resource(pdev, 0, NULL);
> + if (IS_ERR(isp->csbase))
> + return PTR_ERR(isp->csbase);
>
> isp->rstc = devm_reset_control_get(&pdev->dev, NULL);
>
--
Regards,
Laurent Pinchart
