Re: [PATCH 3/3] media: rcar-csi2: Add support for C-PHY on R-Car V4H

["Niklas Söderlund" <niklas.soderlund+renesas@ragnatech.se>]

Hi Geert,

Thanks for your feedback.

On 2023-02-13 16:45:18 +0100, Geert Uytterhoeven wrote:
> Hi Niklas,
> 
> On Sat, Feb 11, 2023 at 4:06 PM Niklas Söderlund
> <niklas.soderlund+renesas@ragnatech.se> wrote:
> > Add support for C-PHY on R-Car V4H. While the V4H supports both D-PHY
> > and C-PHY this patch only adds support for the C-PHY mode due to lack of
> > documentation and hardware to test on.
> >
> > The V4H is the first Gen4 device that is enabled in the rcar-csi2
> > driver. There is much overlap with the Gen3 driver, the primary
> > difference is in how the receiver is started. The V4H have a much larger
> > register space and some addresses overlap with Gen3.
> >
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> 
> Thanks for your patch!
> 
> > --- a/drivers/media/platform/renesas/rcar-vin/rcar-csi2.c
> > +++ b/drivers/media/platform/renesas/rcar-vin/rcar-csi2.c
> 
> > @@ -1503,6 +1781,12 @@ static const struct rcar_csi2_info rcar_csi2_info_r8a779a0 = {
> >         .support_dphy = true,
> >  };
> >
> > +static const struct rcar_csi2_info rcar_csi2_info_r8a779g0 = {
> > +       .start_receiver = rcsi2_start_receiver_v4h,
> 
> The description of "[PATCH 1/3] media: rcar-csi2: Prepare for Gen4
> support" suggests you also need a different procedure for
> .enter_standby()?  But perhaps not doing the procedure is just the right
> thing to do?

They are indeed different, but doing nothing in enter_standby() for V4H 
is the correct thing.

Maybe the function enter_standby(), could have been better named as 
enter_standby_extra() or pre_enter_standby(). As for both on Gen3 and 
V4H after enter_standby(), if present, have been called the rest 
controller is asserted and the module is powers off.

Only that on Gen3 which have more documentation an extra step to poke 
some registers before asserting the reset controller is documented, so 
this is what happens in rcsi2_enter_standby_gen3(). I would not be 
surprised as we get more documentation for V4H we will add a 
rcsi2_enter_standby_gen4() to match updates in the datasheet.


> 
> > +       .use_isp = true,
> > +       .support_cphy = true,
> > +};
> > +
> >  static const struct of_device_id rcar_csi2_of_table[] = {
> >         {
> >                 .compatible = "renesas,r8a774a1-csi2",
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

-- 
Kind Regards,
Niklas Söderlund