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 <[email protected]> --- drivers/gpu/drm/tyr/driver.rs | 5 ++-- rust/helpers/regulator.c | 5 ++++ rust/kernel/regulator.rs | 45 +++++++++++++++++++++++++++++++++-- 3 files changed, 51 insertions(+), 4 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/helpers/regulator.c b/rust/helpers/regulator.c index 11bc332443bd..5ef45a8adf12 100644 --- a/rust/helpers/regulator.c +++ b/rust/helpers/regulator.c @@ -25,6 +25,11 @@ struct regulator *rust_helper_regulator_get(struct device *dev, const char *id) return regulator_get(dev, id); } +struct regulator *rust_helper_regulator_get_optional(struct device *dev, const char *id) +{ + return regulator_get_optional(dev, id); +} + int rust_helper_regulator_enable(struct regulator *regulator) { return regulator_enable(regulator); diff --git a/rust/kernel/regulator.rs b/rust/kernel/regulator.rs index 2c44827ad0b7..d27ce3f6ef26 100644 --- a/rust/kernel/regulator.rs +++ b/rust/kernel/regulator.rs @@ -232,10 +232,11 @@ pub fn devm_enable_optional(dev: &Device<Bound>, name: &CStr) -> Result { /// /// # Invariants /// -/// - `inner` is a non-null wrapper over a pointer to a `struct -/// regulator` obtained from [`regulator_get()`]. +/// - `inner` is a non-null wrapper over a pointer to a `struct regulator` +/// obtained from [`regulator_get()`] or [`regulator_get_optional()`]. /// /// [`regulator_get()`]: https://docs.kernel.org/driver-api/regulator.html#c.regulator_get +/// [`regulator_get_optional()`]: https://docs.kernel.org/driver-api/regulator.html#c.regulator_get_optional pub struct Regulator<State> where State: RegulatorState, @@ -283,6 +284,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 +324,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 +358,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
