linux-kernel - Re: [PATCH] Introduce Tyr

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <C2A539D0-8C07-44A4-93AD-21343396D84F@collabora.com>
Date: Mon, 30 Jun 2025 10:53:13 -0300
From: Daniel Almeida <daniel.almeida@...labora.com>
To: Maíra Canal <mcanal@...lia.com>
Cc: Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
 Maxime Ripard <mripard@...nel.org>,
 Thomas Zimmermann <tzimmermann@...e.de>,
 David Airlie <airlied@...il.com>,
 Simona Vetter <simona@...ll.ch>,
 Miguel Ojeda <ojeda@...nel.org>,
 Alex Gaynor <alex.gaynor@...il.com>,
 Boqun Feng <boqun.feng@...il.com>,
 Gary Guo <gary@...yguo.net>,
 Björn Roy Baron <bjorn3_gh@...tonmail.com>,
 Benno Lossin <lossin@...nel.org>,
 Andreas Hindborg <a.hindborg@...nel.org>,
 Alice Ryhl <aliceryhl@...gle.com>,
 Trevor Gross <tmgross@...ch.edu>,
 Danilo Krummrich <dakr@...nel.org>,
 Daniel Stone <daniels@...labora.com>,
 Rob Herring <robh@...nel.org>,
 Alice Ryhl <alice.ryhl@...gle.com>,
 Beata Michalska <beata.michalska@....com>,
 Carsten Haitzler <carsten.haitzler@...s.arm.com>,
 Boris Brezillon <boris.brezillon@...labora.com>,
 Ashley Smith <ashley.smith@...labora.com>,
 linux-kernel@...r.kernel.org,
 dri-devel@...ts.freedesktop.org,
 rust-for-linux@...r.kernel.org,
 kernel@...labora.com
Subject: Re: [PATCH] Introduce Tyr

Hi Maíra, thanks for chiming in :)

> 
> To enhance readability, consider using a regmap similar to
> panthor_regs.h. This would help avoid 'magic numbers' and make the
> code's intent much clearer.


Are you referring to "struct regmap" itself? Because last I checked, this
abstraction is not available upstream. There was a person working on it, but I
guess it hasn't seen any traction for a few months. I also don't see it being
used in panthor_regs.h?

> 
>> +    regs::GPU_CMD.write(iomem, irq_enable_cmd)?;
>> +
>> +    let op = || regs::GPU_INT_RAWSTAT.read(iomem);
>> +    let cond = |raw_stat: &u32| -> bool { (*raw_stat >> 8) & 1 == 1 };
>> +    let res = io::poll::read_poll_timeout(
>> +        op,
>> +        cond,
>> +        time::Delta::from_millis(100),
>> +        Some(time::Delta::from_micros(20000)),
>> +    );
>> +
>> +    if let Err(e) = res {
>> +        pr_err!("GPU reset failed with errno {}\n", e.to_errno());
>> +        pr_err!(
>> +            "GPU_INT_RAWSTAT is {}\n",
>> +            regs::GPU_INT_RAWSTAT.read(iomem)?
>> +        );
>> +    }
>> +
>> +    Ok(())
>> +}
>> +
>> +kernel::of_device_table!(
>> +    OF_TABLE,
>> +    MODULE_OF_TABLE,
>> +    <TyrDriver as platform::Driver>::IdInfo,
>> +    [
>> +        (of::DeviceId::new(c_str!("rockchip,rk3588-mali")), ()),
>> +        (of::DeviceId::new(c_str!("arm,mali-valhall-csf")), ())
>> +    ]
>> +);
>> +
>> +impl platform::Driver for TyrDriver {
>> +    type IdInfo = ();
>> +    const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = Some(&OF_TABLE);
>> +
>> +    fn probe(
>> +        pdev: &platform::Device<Core>,
>> +        _info: Option<&Self::IdInfo>,
>> +    ) -> Result<Pin<KBox<Self>>> {
>> +        dev_dbg!(pdev.as_ref(), "Probed Tyr\n");
>> +
>> +        let core_clk = Clk::get(pdev.as_ref(), Some(c_str!("core")))?;
>> +        let stacks_clk = Clk::get(pdev.as_ref(), Some(c_str!("stacks")))?;
> 
> Shouldn't it be OptionalClk::get? From the DT schema for "arm,mali-
> valhall-csf", I see that "stacks" and "coregroups" are optional.
> 
>> +        let coregroup_clk = Clk::get(pdev.as_ref(), Some(c_str!("coregroup")))?;
> 
> Same.
> 
> Best Regards,
> - Maíra
> 
> 

Ah yes, you’re right. I will fix that in v2.

— Daniel