Hi Shimoda-san,
On Mon, Nov 12, 2018 at 9:53 AM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@xxxxxxxxxxx> wrote:
> > From: Geert Uytterhoeven, Sent: Monday, November 12, 2018 5:19 PM
> > On Mon, Nov 12, 2018 at 8:24 AM Yoshihiro Shimoda
> > <yoshihiro.shimoda.uh@xxxxxxxxxxx> 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 -- geert@xxxxxxxxxxxxxx
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
