On Wed, Jul 13, 2022 at 10:22 PM Thierry Reding
<thierry.reding@xxxxxxxxx> wrote:
> On Wed, Jul 13, 2022 at 02:14:27PM +0200, Arnd Bergmann wrote:
> >
> > It's not a 100% strict rule, I've just tried to limit it as much as possible,
> > and sometimes missed drivers doing it anyway. My main goal here is
> > to make things consistent between SoC families, so if one piece of
> > information is provided by a number of them, I'd rather have a standard
> > attribute, or a common way of encoding this in the existing attributes
> > than to have too many custom attributes with similar names.
>
> The major/minor attributes that we have on Tegra SoCs should be easy to
> standardize. It seems like those could be fairly common.
I think these can just be folded into one of the other attributes, probably
either revision or soc_id dependending on what they actually refer to.
These properties are intentionally free-text fields that you can match
using wildcards with the soc_device_match() function. If I read this
part right, the information is already available in the soc_id field,
so we don't even need to change anything here.
> The other one
> that we have is the "platform" one, which I suppose is not as easy to
> standardize. I don't recall the exact details, but I think we're mostly
> interested in whether or not the platform is simulation or silicon. The
> exact simulation value is not something that userspace scripts will look
> at, as far as I recall.
This also looks like it's part of the chip_id.
> > > > > YueHaibing (1):
> > > > > soc/tegra: fuse: Add missing DMADEVICES dependency
> > > >
> > > > This one fixes the warning the wrong way: we don't 'select' random
> > > > drivers from other subsystems, and selecting the entire
> > > > subsystem makes it worse. Just drop the 'select' here and
> > > > enable the drivers in the defconfig.
> > >
> > > This doesn't actually select the DMADEVICES property. It adds a
> > > dependency on DMADEVICES and if that is met it will select
> > > TEGRA20_APB_DMA.
> >
> > My mistake. However, I still think it's wrong to select
> > TEGRA20_APB_DMA here, unless there is a build-time
> > dependency that prevents it from being compiled otherwise.
> >
> > The dmaengine subsystem is meant to abstract the relation
> > between the drivers using DMA and those providing the feature,
> > the same way we abstract all the other subsystems. The
> > fuse driver may only be used on machines that use
> > TEGRA20_APB_DMA, but neither the driver code nor
> > Kconfig should care about that.
>
> This dependency has existed for quite a while and my recollection is
> that we wanted to make this very explicit because the lack of the
> TEGRA20_APB_DMA driver makes the FUSE driver completely useless on
> Tegra20 and that in turn has a very negative impact on the rest of the
> system, so we deemed a default configuration change insufficient.
>
> Perhaps a better way to solve this would be to make TEGRA20_APB_DMA
> default to "y" if ARCH_TEGRA_2x_SOC. And then perhaps make the FUSE
> driver depend on DMADEVICES. That still wouldn't ensure that we get
> SOC_TEGRA_FUSE enabled automatically all the time, but perhaps it'd
> document the dependency a bit more explicitly.
Ok, this sounds good to me.
Arnd
