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