On Thu, Oct 07, 2021 at 07:13:25PM +0200, Thierry Reding wrote:
> > @@ -496,6 +506,8 @@ static void tegra_smmu_as_unprepare(struct tegra_smmu *smmu,
> > mutex_unlock(&smmu->lock);
> > }
> >
> > +static const struct file_operations tegra_smmu_debugfs_mappings_fops;
>
> Could the implementation be moved up here to avoid the forward
> declaration?
I thought that keeping all debugfs fops together would be preferable.
But yes, I will move it if you prefer no-additional forward declare.
> > + seq_printf(s, "\nSWGROUP: %s\n", swgrp->name);
> > + seq_printf(s, "as->id: %d\nas->attr: %c|%c|%s\nas->pd_dma: %pad\n", as->id,
> > + as->attr & SMMU_PD_READABLE ? 'R' : '-',
> > + as->attr & SMMU_PD_WRITABLE ? 'W' : '-',
> > + as->attr & SMMU_PD_NONSECURE ? "NS" : "S",
> > + &as->pd_dma);
> > + seq_puts(s, "{\n");
>
> Maybe this can be more compact by putting the name, ID, attributes and
> base address onto a single line? Maybe also use "'-' : 'S'" for the
> non-secure attribute to keep in line with what you've done for readable
> and writable attributes.
Okay. Will change that.
> Then again, this is going to be very verbose output anyway, so maybe it
> isn't worth it.
Are you saying the whole debugfs thing or just attributes? Yet, for
either case, I don't think so, as mappings info would help for sure
from our past experience while the attributes are just one line...
