[PATCH v1] drm/tyr: make SRAM supply optional like panthor

[PATCH v1] drm/tyr: make SRAM supply optional like panthor

[Onur Özkan]

On rk3588s, `dmesg | grep 'tyr'` logs:

  tyr fb000000.gpu: supply SRAM not found, using dummy regulator

This happens because Tyr calls Regulator<Enabled>::get() for SRAM,
which goes through the non-optional regulator_get() path. If the
device tree doesn't provide sram-supply, regulator core falls back
to a dummy regulator and writes that log.

Panthor handles SRAM as optional and tolerates missing sram-supply.
This patch matches that behavior in Tyr by using optional regulator
lookup and storing SRAM as Option<Regulator<Enabled>> which avoids
dummy-regulator fallback/noise when SRAM is not described inside
the device tree.

Link: https://rust-for-linux.zulipchat.com/#narrow/stream/x/topic/x/near/573210018
Signed-off-by: Onur Özkan <work@onurozkan.dev>
---
 drivers/gpu/drm/tyr/driver.rs |  5 +++--
 rust/kernel/regulator.rs      | 40 +++++++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs
index 0389c558c036..e0856deb83ec 100644
--- a/drivers/gpu/drm/tyr/driver.rs
+++ b/drivers/gpu/drm/tyr/driver.rs
@@ -113,7 +113,8 @@ fn probe(
         coregroup_clk.prepare_enable()?;
 
         let mali_regulator = Regulator::<regulator::Enabled>::get(pdev.as_ref(), c_str!("mali"))?;
-        let sram_regulator = Regulator::<regulator::Enabled>::get(pdev.as_ref(), c_str!("sram"))?;
+        let sram_regulator =
+            Regulator::<regulator::Enabled>::get_optional(pdev.as_ref(), c_str!("sram"))?;
 
         let request = pdev.io_request_by_index(0).ok_or(ENODEV)?;
         let iomem = Arc::pin_init(request.iomap_sized::<SZ_2M>(), GFP_KERNEL)?;
@@ -201,5 +202,5 @@ struct Clocks {
 #[pin_data]
 struct Regulators {
     mali: Regulator<regulator::Enabled>,
-    sram: Regulator<regulator::Enabled>,
+    sram: Option<Regulator<regulator::Enabled>>,
 }
diff --git a/rust/kernel/regulator.rs b/rust/kernel/regulator.rs
index 2c44827ad0b7..8d95e5e80051 100644
--- a/rust/kernel/regulator.rs
+++ b/rust/kernel/regulator.rs
@@ -283,6 +283,29 @@ fn get_internal(dev: &Device, name: &CStr) -> Result<Regulator<T>> {
         })
     }
 
+    fn get_optional_internal(dev: &Device, name: &CStr) -> Result<Option<Regulator<T>>> {
+        // SAFETY: It is safe to call `regulator_get_optional()`, on a
+        // device pointer received from the C code.
+        let inner = from_err_ptr(unsafe {
+            bindings::regulator_get_optional(dev.as_raw(), name.as_char_ptr())
+        });
+
+        let inner = match inner {
+            Ok(inner) => inner,
+            Err(ENODEV) => return Ok(None),
+            Err(err) => return Err(err),
+        };
+
+        // SAFETY: We can safely trust `inner` to be a pointer to a valid
+        // regulator if `ERR_PTR` was not returned.
+        let inner = unsafe { NonNull::new_unchecked(inner) };
+
+        Ok(Some(Self {
+            inner,
+            _phantom: PhantomData,
+        }))
+    }
+
     fn enable_internal(&self) -> Result {
         // SAFETY: Safe as per the type invariants of `Regulator`.
         to_result(unsafe { bindings::regulator_enable(self.inner.as_ptr()) })
@@ -300,6 +323,11 @@ pub fn get(dev: &Device, name: &CStr) -> Result<Self> {
         Regulator::get_internal(dev, name)
     }
 
+    /// Obtains an optional [`Regulator`] instance from the system.
+    pub fn get_optional(dev: &Device, name: &CStr) -> Result<Option<Self>> {
+        Regulator::get_optional_internal(dev, name)
+    }
+
     /// Attempts to convert the regulator to an enabled state.
     pub fn try_into_enabled(self) -> Result<Regulator<Enabled>, Error<Disabled>> {
         // We will be transferring the ownership of our `regulator_get()` count to
@@ -329,6 +357,18 @@ pub fn get(dev: &Device, name: &CStr) -> Result<Self> {
             .map_err(|error| error.error)
     }
 
+    /// Obtains an optional [`Regulator`] instance from the system and enables it.
+    pub fn get_optional(dev: &Device, name: &CStr) -> Result<Option<Self>> {
+        match Regulator::<Disabled>::get_optional_internal(dev, name)? {
+            Some(regulator) => {
+                let enabled_regulator =
+                    regulator.try_into_enabled().map_err(|error| error.error)?;
+                Ok(Some(enabled_regulator))
+            }
+            None => Ok(None),
+        }
+    }
+
     /// Attempts to convert the regulator to a disabled state.
     pub fn try_into_disabled(self) -> Result<Regulator<Disabled>, Error<Enabled>> {
         // We will be transferring the ownership of our `regulator_get()` count
-- 
2.51.2

Re: [PATCH v1] drm/tyr: make SRAM supply optional like panthor

[Charalampos Mitrodimas]

Onur Özkan <work@onurozkan.dev> writes:

> On rk3588s, `dmesg | grep 'tyr'` logs:
>
>   tyr fb000000.gpu: supply SRAM not found, using dummy regulator
>
> This happens because Tyr calls Regulator<Enabled>::get() for SRAM,
> which goes through the non-optional regulator_get() path. If the
> device tree doesn't provide sram-supply, regulator core falls back
> to a dummy regulator and writes that log.
>
> Panthor handles SRAM as optional and tolerates missing sram-supply.
> This patch matches that behavior in Tyr by using optional regulator
> lookup and storing SRAM as Option<Regulator<Enabled>> which avoids
> dummy-regulator fallback/noise when SRAM is not described inside
> the device tree.
>
> Link: https://rust-for-linux.zulipchat.com/#narrow/stream/x/topic/x/near/573210018
> Signed-off-by: Onur Özkan <work@onurozkan.dev>
> ---
>  drivers/gpu/drm/tyr/driver.rs |  5 +++--
>  rust/kernel/regulator.rs      | 40 +++++++++++++++++++++++++++++++++++
>  2 files changed, 43 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs
> index 0389c558c036..e0856deb83ec 100644
> --- a/drivers/gpu/drm/tyr/driver.rs
> +++ b/drivers/gpu/drm/tyr/driver.rs
> @@ -113,7 +113,8 @@ fn probe(
>          coregroup_clk.prepare_enable()?;
>  
>          let mali_regulator = Regulator::<regulator::Enabled>::get(pdev.as_ref(), c_str!("mali"))?;
> -        let sram_regulator = Regulator::<regulator::Enabled>::get(pdev.as_ref(), c_str!("sram"))?;
> +        let sram_regulator =
> +            Regulator::<regulator::Enabled>::get_optional(pdev.as_ref(), c_str!("sram"))?;
>  
>          let request = pdev.io_request_by_index(0).ok_or(ENODEV)?;
>          let iomem = Arc::pin_init(request.iomap_sized::<SZ_2M>(), GFP_KERNEL)?;
> @@ -201,5 +202,5 @@ struct Clocks {
>  #[pin_data]
>  struct Regulators {
>      mali: Regulator<regulator::Enabled>,
> -    sram: Regulator<regulator::Enabled>,
> +    sram: Option<Regulator<regulator::Enabled>>,
>  }
> diff --git a/rust/kernel/regulator.rs b/rust/kernel/regulator.rs
> index 2c44827ad0b7..8d95e5e80051 100644
> --- a/rust/kernel/regulator.rs
> +++ b/rust/kernel/regulator.rs
> @@ -283,6 +283,29 @@ fn get_internal(dev: &Device, name: &CStr) -> Result<Regulator<T>> {
>          })
>      }
>  
> +    fn get_optional_internal(dev: &Device, name: &CStr) -> Result<Option<Regulator<T>>> {
> +        // SAFETY: It is safe to call `regulator_get_optional()`, on a
> +        // device pointer received from the C code.
> +        let inner = from_err_ptr(unsafe {
> +            bindings::regulator_get_optional(dev.as_raw(), name.as_char_ptr())
> +        });

Hello,

When CONFIG_REGULATOR is disabled, regulator_get_optional() becomes a
static inline stub in consumer.h, and bindgen cannot export it as a
symbol. The other regulator functions all have C helpers for this but
regulator_get_optional() is missing one.

So it causes a E0425 with CONFIG_REGULATOR not set.

  error[E0425]: cannot find function `regulator_get_optional` in crate `bindings`
      --> rust/kernel/regulator.rs:290:23
       |
   290 |             bindings::regulator_get_optional(dev.as_raw(), name.as_char_ptr())
       |                       ^^^^^^^^^^^^^^^^^^^^^^ help: a function with a similar name exists: `regulator_get_voltage`

> +
> +        let inner = match inner {
> +            Ok(inner) => inner,
> +            Err(ENODEV) => return Ok(None),
> +            Err(err) => return Err(err),
> +        };
> +
> +        // SAFETY: We can safely trust `inner` to be a pointer to a valid
> +        // regulator if `ERR_PTR` was not returned.
> +        let inner = unsafe { NonNull::new_unchecked(inner) };
> +
> +        Ok(Some(Self {
> +            inner,
> +            _phantom: PhantomData,
> +        }))
> +    }

The Regulator struct invariant currently says:

  /// - `inner` is a non-null wrapper over a pointer to a `struct
  ///   regulator` obtained from [`regulator_get()`].

Since get_optional_internal() creates a Regulator from
regulator_get_optional(), should we also update it to mention it?


Cheers,
C. Mitrodimas

> +
>      fn enable_internal(&self) -> Result {
>          // SAFETY: Safe as per the type invariants of `Regulator`.
>          to_result(unsafe { bindings::regulator_enable(self.inner.as_ptr()) })
> @@ -300,6 +323,11 @@ pub fn get(dev: &Device, name: &CStr) -> Result<Self> {
>          Regulator::get_internal(dev, name)
>      }
>  
> +    /// Obtains an optional [`Regulator`] instance from the system.
> +    pub fn get_optional(dev: &Device, name: &CStr) -> Result<Option<Self>> {
> +        Regulator::get_optional_internal(dev, name)
> +    }
> +
>      /// Attempts to convert the regulator to an enabled state.
>      pub fn try_into_enabled(self) -> Result<Regulator<Enabled>, Error<Disabled>> {
>          // We will be transferring the ownership of our `regulator_get()` count to
> @@ -329,6 +357,18 @@ pub fn get(dev: &Device, name: &CStr) -> Result<Self> {
>              .map_err(|error| error.error)
>      }
>  
> +    /// Obtains an optional [`Regulator`] instance from the system and enables it.
> +    pub fn get_optional(dev: &Device, name: &CStr) -> Result<Option<Self>> {
> +        match Regulator::<Disabled>::get_optional_internal(dev, name)? {
> +            Some(regulator) => {
> +                let enabled_regulator =
> +                    regulator.try_into_enabled().map_err(|error| error.error)?;
> +                Ok(Some(enabled_regulator))
> +            }
> +            None => Ok(None),
> +        }
> +    }
> +
>      /// Attempts to convert the regulator to a disabled state.
>      pub fn try_into_disabled(self) -> Result<Regulator<Disabled>, Error<Enabled>> {
>          // We will be transferring the ownership of our `regulator_get()` count

Re: [PATCH v1] drm/tyr: make SRAM supply optional like panthor

[kernel test robot]

Hi Onur,

kernel test robot noticed the following build errors:

[auto build test ERROR on broonie-regulator/for-next]
[also build test ERROR on drm-i915/for-linux-next drm-i915/for-linux-next-fixes v6.19]
[cannot apply to drm-misc/drm-misc-next daeinki-drm-exynos/exynos-drm-next drm/drm-next drm-tip/drm-tip linus/master next-20260211]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Onur-zkan/drm-tyr-make-SRAM-supply-optional-like-panthor/20260212-035718
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next
patch link:    https://lore.kernel.org/r/20260211195406.289634-1-work%40onurozkan.dev
patch subject: [PATCH v1] drm/tyr: make SRAM supply optional like panthor
config: x86_64-rhel-9.4-rust (https://download.01.org/0day-ci/archive/20260212/202602121116.499PMsGK-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
rustc: rustc 1.88.0 (6b00bc388 2025-06-23)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260212/202602121116.499PMsGK-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202602121116.499PMsGK-lkp@intel.com/

All errors (new ones prefixed by >>):

>> error[E0425]: cannot find function `regulator_get_optional` in crate `bindings`
   --> rust/kernel/regulator.rs:287:23
   |
   287  |             bindings::regulator_get_optional(dev.as_raw(), name.as_char_ptr())
   |                       ^^^^^^^^^^^^^^^^^^^^^^ help: a function with a similar name exists: `regulator_get_voltage`
   |
   ::: rust/bindings/bindings_helpers_generated.rs:1306:5
   |
   1306 |     pub fn regulator_get_voltage(regulator: *mut regulator) -> ffi::c_int;
   |     ---------------------------------------------------------------------- similarly named function `regulator_get_voltage` defined here

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v1] drm/tyr: make SRAM supply optional like panthor

[Onur Özkan]

On Thu, 12 Feb 2026 02:04:23 +0000
Charalampos Mitrodimas <charmitro@posteo.net> wrote:

> Onur Özkan <work@onurozkan.dev> writes:
> 
> > On rk3588s, `dmesg | grep 'tyr'` logs:
> >
> >   tyr fb000000.gpu: supply SRAM not found, using dummy regulator
> >
> > This happens because Tyr calls Regulator<Enabled>::get() for SRAM,
> > which goes through the non-optional regulator_get() path. If the
> > device tree doesn't provide sram-supply, regulator core falls back
> > to a dummy regulator and writes that log.
> >
> > Panthor handles SRAM as optional and tolerates missing sram-supply.
> > This patch matches that behavior in Tyr by using optional regulator
> > lookup and storing SRAM as Option<Regulator<Enabled>> which avoids
> > dummy-regulator fallback/noise when SRAM is not described inside
> > the device tree.
> >
> > Link:
> > https://rust-for-linux.zulipchat.com/#narrow/stream/x/topic/x/near/573210018
> > Signed-off-by: Onur Özkan <work@onurozkan.dev> ---
> >  drivers/gpu/drm/tyr/driver.rs |  5 +++--
> >  rust/kernel/regulator.rs      | 40
> > +++++++++++++++++++++++++++++++++++ 2 files changed, 43
> > insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/tyr/driver.rs
> > b/drivers/gpu/drm/tyr/driver.rs index 0389c558c036..e0856deb83ec
> > 100644 --- a/drivers/gpu/drm/tyr/driver.rs
> > +++ b/drivers/gpu/drm/tyr/driver.rs
> > @@ -113,7 +113,8 @@ fn probe(
> >          coregroup_clk.prepare_enable()?;
> >  
> >          let mali_regulator =
> > Regulator::<regulator::Enabled>::get(pdev.as_ref(),
> > c_str!("mali"))?;
> > -        let sram_regulator =
> > Regulator::<regulator::Enabled>::get(pdev.as_ref(),
> > c_str!("sram"))?;
> > +        let sram_regulator =
> > +
> > Regulator::<regulator::Enabled>::get_optional(pdev.as_ref(),
> > c_str!("sram"))?; let request =
> > pdev.io_request_by_index(0).ok_or(ENODEV)?; let iomem =
> > Arc::pin_init(request.iomap_sized::<SZ_2M>(), GFP_KERNEL)?; @@
> > -201,5 +202,5 @@ struct Clocks { #[pin_data]
> >  struct Regulators {
> >      mali: Regulator<regulator::Enabled>,
> > -    sram: Regulator<regulator::Enabled>,
> > +    sram: Option<Regulator<regulator::Enabled>>,
> >  }
> > diff --git a/rust/kernel/regulator.rs b/rust/kernel/regulator.rs
> > index 2c44827ad0b7..8d95e5e80051 100644
> > --- a/rust/kernel/regulator.rs
> > +++ b/rust/kernel/regulator.rs
> > @@ -283,6 +283,29 @@ fn get_internal(dev: &Device, name: &CStr) ->
> > Result<Regulator<T>> { })
> >      }
> >  
> > +    fn get_optional_internal(dev: &Device, name: &CStr) ->
> > Result<Option<Regulator<T>>> {
> > +        // SAFETY: It is safe to call `regulator_get_optional()`,
> > on a
> > +        // device pointer received from the C code.
> > +        let inner = from_err_ptr(unsafe {
> > +            bindings::regulator_get_optional(dev.as_raw(),
> > name.as_char_ptr())
> > +        });
> 
> Hello,
> 
> When CONFIG_REGULATOR is disabled, regulator_get_optional() becomes a
> static inline stub in consumer.h, and bindgen cannot export it as a
> symbol. The other regulator functions all have C helpers for this but
> regulator_get_optional() is missing one.
> 
> So it causes a E0425 with CONFIG_REGULATOR not set.
> 
>   error[E0425]: cannot find function `regulator_get_optional` in
> crate `bindings` --> rust/kernel/regulator.rs:290:23
>        |
>    290 |             bindings::regulator_get_optional(dev.as_raw(),
> name.as_char_ptr()) |                       ^^^^^^^^^^^^^^^^^^^^^^
> help: a function with a similar name exists: `regulator_get_voltage`

Yeah, I missed that.

> 
> > +
> > +        let inner = match inner {
> > +            Ok(inner) => inner,
> > +            Err(ENODEV) => return Ok(None),
> > +            Err(err) => return Err(err),
> > +        };
> > +
> > +        // SAFETY: We can safely trust `inner` to be a pointer to
> > a valid
> > +        // regulator if `ERR_PTR` was not returned.
> > +        let inner = unsafe { NonNull::new_unchecked(inner) };
> > +
> > +        Ok(Some(Self {
> > +            inner,
> > +            _phantom: PhantomData,
> > +        }))
> > +    }
> 
> The Regulator struct invariant currently says:
> 
>   /// - `inner` is a non-null wrapper over a pointer to a `struct
>   ///   regulator` obtained from [`regulator_get()`].
> 
> Since get_optional_internal() creates a Regulator from
> regulator_get_optional(), should we also update it to mention it?

I think we should, will send v2 and cover both changes.

Thanks,
Onur

> 
> 
> Cheers,
> C. Mitrodimas
> 
> > +
> >      fn enable_internal(&self) -> Result {
> >          // SAFETY: Safe as per the type invariants of `Regulator`.
> >          to_result(unsafe {
> > bindings::regulator_enable(self.inner.as_ptr()) }) @@ -300,6
> > +323,11 @@ pub fn get(dev: &Device, name: &CStr) -> Result<Self> {
> > Regulator::get_internal(dev, name) }
> >  
> > +    /// Obtains an optional [`Regulator`] instance from the system.
> > +    pub fn get_optional(dev: &Device, name: &CStr) ->
> > Result<Option<Self>> {
> > +        Regulator::get_optional_internal(dev, name)
> > +    }
> > +
> >      /// Attempts to convert the regulator to an enabled state.
> >      pub fn try_into_enabled(self) -> Result<Regulator<Enabled>,
> > Error<Disabled>> { // We will be transferring the ownership of our
> > `regulator_get()` count to @@ -329,6 +357,18 @@ pub fn get(dev:
> > &Device, name: &CStr) -> Result<Self> { .map_err(|error|
> > error.error) }
> >  
> > +    /// Obtains an optional [`Regulator`] instance from the system
> > and enables it.
> > +    pub fn get_optional(dev: &Device, name: &CStr) ->
> > Result<Option<Self>> {
> > +        match Regulator::<Disabled>::get_optional_internal(dev,
> > name)? {
> > +            Some(regulator) => {
> > +                let enabled_regulator =
> > +                    regulator.try_into_enabled().map_err(|error|
> > error.error)?;
> > +                Ok(Some(enabled_regulator))
> > +            }
> > +            None => Ok(None),
> > +        }
> > +    }
> > +
> >      /// Attempts to convert the regulator to a disabled state.
> >      pub fn try_into_disabled(self) -> Result<Regulator<Disabled>,
> > Error<Enabled>> { // We will be transferring the ownership of our
> > `regulator_get()` count