MIME-Version: 1.0
References: <20230224-rust-iopt-rtkit-v1-0-49ced3391295@asahilina.net>
 <20230224-rust-iopt-rtkit-v1-5-49ced3391295@asahilina.net>
 <Y/idMBIOfFZxXnVM@kroah.com> <CANeycqoHwp3URSPGvnNZx+9PdbC90UVFWLwg4w=JBHQnjnGUPA@mail.gmail.com>
 <ZAnB/DozWsir1cIE@kroah.com> <CANeycqrtjKMfpae_DUp-VrDZugJVO7mcbBvUBB+zAc=E6aU4tw@mail.gmail.com>
 <ZAoTNlF+bHyyGs7x@kroah.com>
In-Reply-To: <ZAoTNlF+bHyyGs7x@kroah.com>
From:   Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
Date:   Thu, 9 Mar 2023 20:43:22 +0100
Message-ID: <CANiq72nkqu+o5QvmLKvk82rJ6b6O2KdC-iYq9zFZFwVyf_Fv9A@mail.gmail.com>
Subject: Re: [PATCH 5/5] rust: device: Add a stub abstraction for devices
To:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc:     Wedson Almeida Filho <wedsonaf@gmail.com>,
        Asahi Lina <lina@asahilina.net>,
        Miguel Ojeda <ojeda@kernel.org>,
        Alex Gaynor <alex.gaynor@gmail.com>,
        Boqun Feng <boqun.feng@gmail.com>, Gary Guo <gary@garyguo.net>,
        =?UTF-8?Q?Bj=C3=B6rn_Roy_Baron?= <bjorn3_gh@protonmail.com>,
        Will Deacon <will@kernel.org>,
        Robin Murphy <robin.murphy@arm.com>,
        Joerg Roedel <joro@8bytes.org>,
        Hector Martin <marcan@marcan.st>,
        Sven Peter <sven@svenpeter.dev>, Arnd Bergmann <arnd@arndb.de>,
        "Rafael J. Wysocki" <rafael@kernel.org>,
        Alyssa Rosenzweig <alyssa@rosenzweig.io>,
        Neal Gompa <neal@gompa.dev>, rust-for-linux@vger.kernel.org,
        linux-kernel@vger.kernel.org, asahi@lists.linux.dev
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Thu, Mar 9, 2023 at 6:11=E2=80=AFPM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> I don't understand, why do you need both of these?  Why can't one just
> do?  Why would you need one without the other?  I would think that
> "Device" and "RawDevice" here would be the same thing, that is a way to
> refer to a "larger" underlying struct device memory chunk in a way that
> can be passed around without knowing, or caring, what the "real" device
> type is.
>
> That sounds like a question, and would return a boolean, not a structure =
:)
>
> I don't really understand that, sorry.

To complement what Wedson wrote, perhaps this helps a bit, since there
are a few `*Device` things around (I will prefix things with the
module they are in for clarity):

  - `device::RawDevice` is a trait, not a struct. This means it is
just an interface/property/tag/feature. Then types (like structs) can
say "I support/provide/implement this trait". In this case, it
requires those types implementing `device::RawDevice` to provide a
suitable `raw_device()` method.

  - Then, you have `device::Device`. This is an actual struct,
containing a `struct device *`. It implements `device::RawDevice`, by
returning the pointer.

  - Then there are others like `platform::Device` or `amba::Device`.
They are also structs, containing e.g. `struct platform_device *` or
anything they may need. They implement `device::RawDevice` too if it
makes sense, say, by returning a pointer to the `->dev` in their C
struct.

So you have two different kinds of entities here. One is the trait,
and the others are structs that promise to implement that trait
correctly, regardless of how each struct manages to do it.

The trait can also then provide extra default functionality for those
that have implemented it. For instance, the `clk_get()` method Wedson
mentioned can be defined in the trait, and in its body we can call the
`raw_device()` to do so. `raw_device()` is available because we are
inside the trait, even if each struct implementing the trait provides
a different implementation.

Traits are sometimes named after the "main" method they contain, e.g.
`ToString` may be a trait with a `to_string()` method. Here
`device::RawDevice` has a `raw_device()` method, so it makes sense in
that way.

Wedson was suggesting `device::IsDevice` as an alternative, because
`device::Device` is taken for the ref-counted device. Of course, other
arrangements of names could be possible.

`IsDevice` uses Pascal case, so by convention it would be fairly clear
that it would not be a function returning a boolean. But one may
expect that it is a trait that implements a `is_device()` method,
though. (And if that was the case, which it isn't, one could then
indeed expect that such a method would return the boolean you
expected).

Cheers,
Miguel