On 2/28/25 11:06 AM, Alexander Stein wrote:
Hi,
diff --git a/drivers/gpu/drm/panthor/panthor_device.c b/drivers/gpu/drm/panthor/panthor_device.c
index a9da1d1eeb707..51ee9cae94504 100644
--- a/drivers/gpu/drm/panthor/panthor_device.c
+++ b/drivers/gpu/drm/panthor/panthor_device.c
@@ -64,6 +64,17 @@ static int panthor_clk_init(struct panthor_device *ptdev)
return 0;
}
+static int panthor_reset_init(struct panthor_device *ptdev)
+{
+ ptdev->resets = devm_reset_control_get_optional_exclusive_deasserted(ptdev->base.dev, NULL);
If the description as a write-once register is true, wouldn't this
already write to it?
It would. I believe the reset handling in the GPU driver should be
generic. The reset has to be deasserted here to access the GPU registers.
The question is, whether the GPUMIX GPURESET bit should be modeled as
simple reset, even if it really behaves as a
set-and-never-can-be-cleared latched bit.
+ if (IS_ERR(ptdev->resets))
+ return dev_err_probe(ptdev->base.dev,
+ PTR_ERR(ptdev->resets),
+ "get reset failed");
+
+ return 0;
+}
+
void panthor_device_unplug(struct panthor_device *ptdev)
{
/* This function can be called from two different path: the reset work
@@ -217,6 +228,10 @@ int panthor_device_init(struct panthor_device *ptdev)
if (ret)
return ret;
+ ret = panthor_reset_init(ptdev);
+ if (ret)
+ return ret;
+
ret = panthor_devfreq_init(ptdev);
if (ret)
return ret;
@@ -470,6 +485,10 @@ int panthor_device_resume(struct device *dev)
if (ret)
goto err_disable_stacks_clk;
+ ret = reset_control_deassert(ptdev->resets);
+ if (ret)
+ goto err_disable_coregroup_clk;
+
This wouldn't work at all on a write-once register, no? Same for resume.
See above and also my reply to 2/9 .
