On Fri, 2020-04-24 at 15:55 +0200, Paul Kocialkowski wrote:
> Hi Ezequiel,
>
> On Fri 24 Apr 20, 09:54, Ezequiel Garcia wrote:
> > Hey Paul,
> >
> > Thanks for the patch!
> >
> > On Thu, 2020-04-23 at 22:09 +0200, Paul Kocialkowski wrote:
> > > The PX30 SoC has a RGA block, so add the associated compatible to
> > > support it.
> > >
> > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@xxxxxxxxxxx>
> > > ---
> > > drivers/media/platform/rockchip/rga/rga.c | 3 +++
> > > 1 file changed, 3 insertions(+)
> > >
> > > diff --git a/drivers/media/platform/rockchip/rga/rga.c b/drivers/media/platform/rockchip/rga/rga.c
> > > index 9d122429706e..4fb4615662b7 100644
> > > --- a/drivers/media/platform/rockchip/rga/rga.c
> > > +++ b/drivers/media/platform/rockchip/rga/rga.c
> > > @@ -955,6 +955,9 @@ static const struct dev_pm_ops rga_pm = {
> > > };
> > >
> > > static const struct of_device_id rockchip_rga_match[] = {
> > > + {
> > > + .compatible = "rockchip,px30-rga",
> > > + },
> >
> > Please note that if you don't have anything px30-specific,
> > then you don't need the compatible in the driver.
> >
> > You can have something like:
> >
> > compatible = "rockchip,px30-rga", "rockchip,rk3288-rga"
> >
> > so you need to add it to the bindings. See Justin Swartz
> > recent patches for rk3228.
>
> Thanks for the instruction!
>
> I've been a bit confused about that because RK3399 has its own compatible
> (without a 2nd rk3288 compatible) although there's nothing different with it
> either. All of these rockchip platforms come with what they call "RGA2", that
> seems to have no variation across platforms (downstream rockchip even has a
> single compatible for it).
>
Yep, and that's an anti pattern, so you can expect to see that elsewhere.
> Should we add the rk3288 compatible to the rk3399 dtsi? I guess we
> can't remove it from the driver at this point, for backward compatibility
> with previous dts (what a strange idea...).
>
No, we don't necesarily need/have to do anything with rk3399,
not sure if there's any gain at this point.
> > Down the road, if you find something specific for px30,
> > you can make the driver aware.
>
> Makes sense, yes.
>
Cool.
Thanks,
Ezequiel
