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] [day] [month] [year] [list]

Message-ID: <5f28693f-a181-485a-aee7-38c5212bdea1@igalia.com>
Date: Thu, 3 Jul 2025 07:45:56 -0300
From: Maíra Canal <mcanal@...lia.com>
To: Daniel Almeida <daniel.almeida@...labora.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 Daniel,

On 30/06/25 10:53, Daniel Almeida wrote:
> 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?

Sorry, I think I didn't express myself clearly. When I say regmap, I
mean using macros to express the register addresses and its fields. From
example, in Panthor, "1 | bit_u32(8)" is expressed as
GPU_IRQ_RESET_COMPLETED, which can make things more readable.

Best Regards,
- Maíra

> 
>>
>>> +    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
>