Hi Shimoda-san,

On Mon, Nov 12, 2018 at 9:53 AM Yoshihiro Shimoda
<[email protected]> wrote:
> > From: Geert Uytterhoeven, Sent: Monday, November 12, 2018 5:19 PM
> > On Mon, Nov 12, 2018 at 8:24 AM Yoshihiro Shimoda
> > <[email protected]> wrote:
> <snip>
> > > --- a/drivers/iommu/ipmmu-vmsa.c
> > > +++ b/drivers/iommu/ipmmu-vmsa.c
> > > @@ -758,10 +758,10 @@ static bool ipmmu_slave_whitelist(struct device 
> > > *dev)
> > >  }
> > >
> > >  static const struct soc_device_attribute soc_rcar_gen3[] = {
> > > -       { .soc_id = "r8a7795", },
> > > -       { .soc_id = "r8a7796", },
> > > +       { .soc_id = "r8a7795", .revision = "ES3.*" },
> > >         { .soc_id = "r8a77965", },
> > >         { .soc_id = "r8a77970", },
> > > +       { .soc_id = "r8a77990", },
> > >         { .soc_id = "r8a77995", },
> > >         { /* sentinel */ }
> > >  };
> >
> > Given the above, I think the time is ripe to convert this from a whitelist 
> > to a
> > blacklist?
>
> About the SoCs, I think so. (I updated example patch below and it seems 
> better :) )
> However, I would like to keep ipmmu_slave_whitelist to avoid any trouble for 
> now...

OK, IC.

> --- a/drivers/iommu/ipmmu-vmsa.c
> +++ b/drivers/iommu/ipmmu-vmsa.c
> @@ -757,12 +757,10 @@ static bool ipmmu_slave_whitelist(struct device *dev)
>         return false;
>  }
>
> -static const struct soc_device_attribute soc_rcar_gen3[] = {
> -       { .soc_id = "r8a7795", },
> -       { .soc_id = "r8a7796", },
> -       { .soc_id = "r8a77965", },
> -       { .soc_id = "r8a77970", },
> -       { .soc_id = "r8a77995", },
> +static const struct soc_device_attribute soc_rcar_gen3_blacklist[] = {
> +       { .soc_id = "r8a7795", .revision = "ES1.*" },
> +       { .soc_id = "r8a7795", .revision = "ES2.*" },

I think you can combine both lines using "ES[12].*", too.
But it may be considered less readable, and not greppable for e.g. "ES1".

> +       { .soc_id = "r8a7796", .revision = "ES1.*" },
>         { /* sentinel */ }
>  };
>
> @@ -770,7 +768,8 @@ static int ipmmu_of_xlate(struct device *dev,
>                           struct of_phandle_args *spec)
>  {
>         /* For R-Car Gen3 use a white list to opt-in slave devices */
> -       if (soc_device_match(soc_rcar_gen3) && !ipmmu_slave_whitelist(dev))
> +       if (!soc_device_match(soc_rcar_gen3_blacklist) &&
> +           !ipmmu_slave_whitelist(dev))
>                 return -ENODEV;

Ah, this has the side effect of applying ipmmu_slave_whitelist() on R-Car
Gen2, too, which is probably not what we want?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

Reply via email to