Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 20 Dec 2022 12:03:24 -0800
From:   Luis Chamberlain <mcgrof@kernel.org>
To:     Allen Webb <allenwebb@google.com>
Cc:     Nick Alcock <nick.alcock@oracle.com>,
        Masahiro Yamada <masahiroy@kernel.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        "linux-modules@vger.kernel.org" <linux-modules@vger.kernel.org>,
        "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "Rafael J. Wysocki" <rafael@kernel.org>, stable@vger.kernel.org,
        kernel test robot <lkp@intel.com>
Subject: Re: [PATCH v9 02/10] rockchip-mailbox: Fix typo
Message-ID: <Y6IVDE3NEE6teggy@bombadil.infradead.org>
References: <20221219191855.2010466-1-allenwebb@google.com>
 <20221219204619.2205248-1-allenwebb@google.com>
 <20221219204619.2205248-3-allenwebb@google.com>
 <Y6FaUynXTrYD6OYT@kroah.com>
 <CAJzde04Hbd2+s-Bqog2V81dBEeZD7WWaFCf2BkesQS4yUAKiNA@mail.gmail.com>
 <Y6H6/U0w96Z4kpDn@bombadil.infradead.org>
 <CAJzde04igO0LJ46Hsbcm-hJBFtPdqJC6svaoMkb3WBG0e1fGBw@mail.gmail.com>
 <Y6IDOwxOxZpsdtiu@bombadil.infradead.org>
 <CAJzde06q3w7CHd8FSs-bwS3EeVv6xrBzCwerQVqps49V=_voQQ@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAJzde06q3w7CHd8FSs-bwS3EeVv6xrBzCwerQVqps49V=_voQQ@mail.gmail.com>
Sender: Luis Chamberlain <mcgrof@infradead.org>
Precedence: bulk

On Tue, Dec 20, 2022 at 01:49:04PM -0600, Allen Webb wrote:
> I took another stab at clarifying (and also dropped the ifdev since
> the same macro works both for separate and built-in modules:
> 
> /*
>  * Creates an alias so file2alias.c can find device table.
>  *
>  * Use this in cases where a device table is used to match devices because it
>  * surfaces match-id based module aliases to userspace for:
>  *   - Automatic module loading.
>  *   - Tools like USBGuard which allow or block devices based on policy such as
>  *     which modules match a device.
>  *
>  * The module name is included in the alias for two reasons:
>  *   - It avoids creating two aliases with the same name for built-in modules.
>  *     Historically MODULE_DEVICE_TABLE was a no-op for built-in modules, so
>  *     there was nothing to stop different modules from having the same device
>  *     table name and consequently the same alias when building as a module.
>  *   - The module name is needed by files2alias.c to associate a particular
>  *     device table with its associated module for built-in modules since
>  *     files2alias would otherwise see the module name as `vmlinuz.o`.
>  */

This is still weak in light of the questions I had. It does not make it
easy for a driver developer who is going to support only built-in only
if they need to define this or not. And it seems we're still discussing
the merits of this, so I'd wait until this is fleshed out, but I think
we are on the right track finally.

> The deciding factor in whether it makes sense to remove these vs fix
> them seems to be, "How complete do we want modules.builtin.alias to
> be?"
> 
> Arguably we should just drop these in cases where there isn't an
> "authorized" sysfs attribute but following that logic there is not any
> reason to generate built-in aliases for anything except USB and
> thunderbolt.

There we go, now we have a *real* use case for this for built-in stuff
to consider. Is USBGuard effective even for built-in stuff?

Given everything discussed so far I'd like to get clarification if it
even help for built-in USB / thunderbolt. Does it? If so how? What could
userspace do with this information if the driver is already built-in?

> On the flip side, if we are going to the effort to make this a generic
> solution that covers everything, the built-in aliases are only as
> useful as they are complete, so we would want everything that defines
> a device table to call the macro correctly.

It is the ambiguity which is terrible to add. If the only use case is
for USB and Thunderbolt then we can spell it out, then only those driver
developers would care to consider it if the driver is bool. And, a
respective tooling would scrape only those drivers to verify if the
table is missing for built-in too.

> It definitely is needed for never-tristate modules that match devices
> in subsystems that surface the authorized attribute.

What is this "authorized attribute" BTW exactly? Do have some
documentation reference?

  Luis