All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Danilo Krummrich" <dakr@kernel.org>
To: "Matthew Maurer" <mmaurer@google.com>
Cc: "Miguel Ojeda" <ojeda@kernel.org>,
	"Alex Gaynor" <alex.gaynor@gmail.com>,
	"Boqun Feng" <boqun.feng@gmail.com>,
	"Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	"Sami Tolvanen" <samitolvanen@google.com>,
	"Timur Tabi" <ttabi@nvidia.com>,
	"Benno Lossin" <lossin@kernel.org>,
	linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org
Subject: Re: [PATCH v9 5/5] rust: samples: Add debugfs sample
Date: Wed, 09 Jul 2025 23:56:55 +0200	[thread overview]
Message-ID: <DB7UZCKSQ4G2.ZZBIWS6YJAOF@kernel.org> (raw)
In-Reply-To: <20250709-debugfs-rust-v9-5-92b9eab5a951@google.com>

On Wed Jul 9, 2025 at 9:09 PM CEST, Matthew Maurer wrote:
> +// This data structure would be unlikely to be there in a real driver - it's to hook up mutation
> +// that would normally be driven by whatever the driver was actually servicing and show how that
> +// would work. We're assuming here that those methods would have access to a `&RustDebugFs`.

Please see also [1]. I think you're making this too complicated, and due to the
missing write() support you have to create workarounds because of that, which
serve as a bad reference.

Keep it simple, Create some driver private data in probe() and export a couple
of fields of this driver private data through debugfs.

If you really want to showcase that those values can change, queue some work and
modify the counter and / or the Inner type that is protected by a mutex.

[1] https://lore.kernel.org/lkml/DB7US8G7ISG0.20430M3P7I0K0@kernel.org/

> +#[pin_data]
> +struct Wrapper {
> +    _dir: Dir,
> +    #[pin]
> +    _wrapped: File<File<RustDebugFs>>,
> +}
> +
> +#[pin_data]
> +struct RustDebugFs {
> +    pdev: ARef<platform::Device>,
> +    // As we only hold these for drop effect (to remove the directory/files) we have a leading
> +    // underscore to indicate to the compiler that we don't expect to use this field directly.
> +    _debugfs: Dir,
> +    #[pin]
> +    _compatible: File<CString>,
> +    #[pin]
> +    counter: File<File<AtomicUsize>>,
> +    #[pin]
> +    inner: File<Mutex<Inner>>,
> +}
> +
> +#[derive(Debug)]
> +struct Inner {
> +    x: u32,
> +    y: u32,
> +}
> +
> +kernel::of_device_table!(
> +    OF_TABLE,
> +    MODULE_OF_TABLE,
> +    <Wrapper as platform::Driver>::IdInfo,
> +    [(of::DeviceId::new(c_str!("test,rust-debugfs-device")), ())]
> +);

I don't think we need both, ACPI should be much simpler with QEMU.

> +kernel::acpi_device_table!(
> +    ACPI_TABLE,
> +    MODULE_ACPI_TABLE,
> +    <Wrapper as platform::Driver>::IdInfo,
> +    [(acpi::DeviceId::new(c_str!("LNUXDEBF")), ())]
> +);
> +
> +impl platform::Driver for Wrapper {
> +    type IdInfo = ();
> +    const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = Some(&OF_TABLE);
> +    const ACPI_ID_TABLE: Option<acpi::IdTable<Self::IdInfo>> = Some(&ACPI_TABLE);
> +
> +    fn probe(
> +        pdev: &platform::Device<Core>,
> +        _info: Option<&Self::IdInfo>,
> +    ) -> Result<Pin<KBox<Self>>> {
> +        KBox::try_pin_init(Wrapper::new(RustDebugFs::new(pdev)), GFP_KERNEL)
> +    }
> +}

  reply	other threads:[~2025-07-09 21:57 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-09 19:09 [PATCH v9 0/5] rust: DebugFS Bindings Matthew Maurer
2025-07-09 19:09 ` [PATCH v9 1/5] rust: debugfs: Bind DebugFS directory creation Matthew Maurer
2025-07-09 19:09 ` [PATCH v9 2/5] rust: debugfs: Bind file creation for long-lived Display Matthew Maurer
2025-07-09 19:09 ` [PATCH v9 3/5] rust: debugfs: Support `PinInit` backing for `File`s Matthew Maurer
2025-08-19  5:51   ` Dirk Behme
2025-08-19 14:33     ` Matthew Maurer
2025-08-19 14:47       ` Miguel Ojeda
2025-08-19 23:22         ` Matthew Maurer
2025-07-09 19:09 ` [PATCH v9 4/5] rust: debugfs: Support format hooks Matthew Maurer
2025-07-09 19:09 ` [PATCH v9 5/5] rust: samples: Add debugfs sample Matthew Maurer
2025-07-09 21:56   ` Danilo Krummrich [this message]
2025-07-09 23:35     ` Matthew Maurer
2025-07-09 21:47 ` [PATCH v9 0/5] rust: DebugFS Bindings Danilo Krummrich
2025-07-09 21:53   ` Matthew Maurer
2025-07-09 21:59     ` Danilo Krummrich
2025-07-09 22:04       ` Matthew Maurer
2025-07-09 22:12         ` Danilo Krummrich
2025-07-09 22:21           ` Matthew Maurer
2025-07-09 22:33             ` Danilo Krummrich
2025-07-10  5:27           ` Greg Kroah-Hartman
2025-07-10  9:36             ` Danilo Krummrich
2025-07-10 11:09               ` Benno Lossin
2025-07-10 11:11                 ` Benno Lossin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DB7UZCKSQ4G2.ZZBIWS6YJAOF@kernel.org \
    --to=dakr@kernel.org \
    --cc=a.hindborg@kernel.org \
    --cc=alex.gaynor@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=gary@garyguo.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=mmaurer@google.com \
    --cc=ojeda@kernel.org \
    --cc=rafael@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=samitolvanen@google.com \
    --cc=tmgross@umich.edu \
    --cc=ttabi@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.