On Wed, Nov 29, 2023 at 05:23:13PM +0100, Thierry Reding wrote:
> > diff --git a/drivers/memory/tegra/tegra186.c b/drivers/memory/tegra/tegra186.c
> > index 533f85a4b2bdb7..3e4fbe94dd666e 100644
> > --- a/drivers/memory/tegra/tegra186.c
> > +++ b/drivers/memory/tegra/tegra186.c
> > @@ -111,21 +111,21 @@ static void tegra186_mc_client_sid_override(struct tegra_mc *mc,
> > static int tegra186_mc_probe_device(struct tegra_mc *mc, struct device *dev)
> > {
> > #if IS_ENABLED(CONFIG_IOMMU_API)
> > - struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
> > struct of_phandle_args args;
> > unsigned int i, index = 0;
> > + u32 sid;
> >
> > + WARN_ON(!tegra_dev_iommu_get_stream_id(dev, &sid));
>
> I know the code previously didn't check for any errors, but we may want
> to do so now. If tegra_dev_iommu_get_stream_id() ever fails we may end
> up writing some undefined value into the override register.
My assumption was it never fails otherwise this probably already
doesn't work?
> I'm also unsure if WARN_ON() is appropriate here. I vaguely recall that
> ->probe_device() was called for all devices on the bus and not all of
> them may have been associated with the IOMMU. Not all of them may in
> fact access memory in the first place.
So you are thinkin that of_parse_phandle_with_args() is a NOP
sometimes so it will tolerate the failure?
Seems like the best thing to do is just continue to ignore it then?
> Perhaps I'm misremembering and the IOMMU core now takes care of only
> calling this when fwspec is indeed valid?
Can't advise, I have no idea what tegra_mc_ops is for :)
Jason
