On Tue, Apr 22, 2025 at 01:47:35AM +0300, Laurent Pinchart wrote:
> 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>
I meant
Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@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
