Re: [PATCH v4 3/6] drm/tyr: Use register! macro for JOB_CONTROL

[Deborah Brouwer <deborah.brouwer@collabora.com>]

On Fri, Apr 03, 2026 at 09:27:59AM +0200, Boris Brezillon wrote:
> On Thu, 02 Apr 2026 16:35:33 -0700
> Deborah Brouwer <deborah.brouwer@collabora.com> wrote:
> 
> > Convert the JOB_CONTROL register definitions to use the `register!` macro.
> > 
> > Using the `register!` macro allows us to replace manual bit masks and
> > shifts with typed register and field accessors, which makes the code
> > easier to read and avoids errors from bit manipulation.
> > 
> > Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
> > Co-developed-by: Daniel Almeida <daniel.almeida@collabora.com>
> > Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
> > Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
> > Signed-off-by: Deborah Brouwer <deborah.brouwer@collabora.com>
> > ---
> >  drivers/gpu/drm/tyr/regs.rs | 58 ++++++++++++++++++++++++++++++++++++++-------
> >  1 file changed, 50 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/tyr/regs.rs b/drivers/gpu/drm/tyr/regs.rs
> > index d0f99a433dfd1f55b855744abfe26ff9b429f179..54145d90d4922b895a6ebbbd2cda4fddb8488e1a 100644
> > --- a/drivers/gpu/drm/tyr/regs.rs
> > +++ b/drivers/gpu/drm/tyr/regs.rs
> > @@ -28,7 +28,6 @@
> >  #![allow(dead_code)]
> >  
> >  use kernel::{
> > -    bits::bit_u32,
> >      device::{
> >          Bound,
> >          Device, //
> > @@ -893,14 +892,57 @@ fn from(status: McuStatus) -> Self {
> >      }
> >  }
> >  
> > -pub(crate) const JOB_IRQ_RAWSTAT: Register<0x1000> = Register;
> > -pub(crate) const JOB_IRQ_CLEAR: Register<0x1004> = Register;
> > -pub(crate) const JOB_IRQ_MASK: Register<0x1008> = Register;
> > -pub(crate) const JOB_IRQ_STAT: Register<0x100c> = Register;
> > -
> > -pub(crate) const JOB_IRQ_GLOBAL_IF: u32 = bit_u32(31);
> > -
> >  pub(crate) const MMU_IRQ_RAWSTAT: Register<0x2000> = Register;
> >  pub(crate) const MMU_IRQ_CLEAR: Register<0x2004> = Register;
> >  pub(crate) const MMU_IRQ_MASK: Register<0x2008> = Register;
> >  pub(crate) const MMU_IRQ_STAT: Register<0x200c> = Register;
> > +
> > +/// These registers correspond to the JOB_CONTROL register page.
> > +/// They are involved in communication between the firmware running on the MCU and the host.
> > +pub(crate) mod job_control {
> > +    use kernel::register;
> > +
> > +    register! {
> > +        /// Raw status of job interrupts.
> > +        ///
> > +        /// Write to this register to trigger these interrupts.
> > +        /// Writing a 1 to a bit forces that bit on.
> > +        pub(crate) JOB_IRQ_RAWSTAT(u32) @ 0x1000 {
> > +            /// CSG request. These bits indicate that CSGn requires attention from the host.
> > +            30:0    csg;
> > +            /// GLB request. Indicates that the GLB interface requires attention from the host.
> > +            31:31   glb;
> 
> Any particular reason you didn't go for
> 
> 		31:31   glb => bool;
> 
> here?

No, just an oversight, I'll fix it in v5.

> 
> > +        }
> > +
> > +        /// Clear job interrupts. Write only.
> > +        ///
> > +        /// Write a 1 to a bit to clear the corresponding bit in [`JOB_IRQ_RAWSTAT`].
> > +        pub(crate) JOB_IRQ_CLEAR(u32) @ 0x1004 {
> > +            /// Clear CSG request interrupts.
> > +            30:0    csg;
> > +            /// Clear GLB request interrupt.
> > +            31:31   glb;
> > +        }
> > +
> > +        /// Mask for job interrupts.
> > +        ///
> > +        /// Set each bit to 1 to enable the corresponding interrupt source or to 0 to disable it.
> > +        pub(crate) JOB_IRQ_MASK(u32) @ 0x1008 {
> > +            /// Enable CSG request interrupts.
> > +            30:0    csg;
> > +            /// Enable GLB request interrupt.
> > +            31:31   glb;
> > +        }
> > +
> > +        /// Active job interrupts. Read only.
> > +        ///
> > +        /// This register contains the result of ANDing together [`JOB_IRQ_RAWSTAT`] and
> > +        /// [`JOB_IRQ_MASK`].
> > +        pub(crate) JOB_IRQ_STATUS(u32) @ 0x100c {
> > +            /// CSG request interrupt status.
> > +            30:0    csg;
> > +            /// GLB request interrupt status.
> > +            31:31   glb;
> > +        }
> > +    }
> > +}
> > 
>