Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [High] Clocks are enabled before power domains (regulators) are powered on. - [Medium] Clock enable state is leaked on early probe failure paths. -- --- Patch [1]: [PATCH v8 1/7] drm/tyr: add resources to RegistrationData --- commit ebb25c59afc71f766bee84e4619205c5e8db839b Author: Deborah Brouwer <[email protected]> drm/tyr: add resources to RegistrationData This commit moves Tyr's device-private resources and GPU information from drm::Driver::Data to drm::Driver::RegistrationData. This ensures safe access to the data during the platform device binding lifetime and while registered with userspace. > diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs > index 8348c6cd39295..46ce5c41e310c 100644 > --- a/drivers/gpu/drm/tyr/driver.rs > +++ b/drivers/gpu/drm/tyr/driver.rs [ ... ] > @@ -134,10 +137,10 @@ fn probe<'bound>( > // other threads of execution. > unsafe { pdev.dma_set_mask_and_coherent(DmaMask::try_new(pa_bits)?)? > }; > > - let platform: ARef<platform::Device> = pdev.into(); > + let unreg_dev = drm::UnregisteredDevice::<TyrDrmDriver>::new(pdev, > Ok(()))?; > > - let data = try_pin_init!(TyrDrmDeviceData { > - pdev: platform.clone(), > + let reg_data = try_pin_init!(TyrDrmRegistrationData { > + pdev, > clks <- new_mutex!(Clocks { [Severity: Medium] This is a pre-existing issue, but I noticed a potential resource leak in probe(). Earlier in this function, core_clk, stacks_clk, and coregroup_clk are enabled: drivers/gpu/drm/tyr/driver.rs:probe() { ... core_clk.prepare_enable()?; stacks_clk.prepare_enable()?; coregroup_clk.prepare_enable()?; let mali_regulator = Regulator::<regulator::Enabled>::get(pdev.as_ref(), c"mali")?; ... } If getting a regulator or another step fails and returns early via ?, does the clock enable state leak? Because the Clocks struct isn't created until this point in the diff, it seems the enable count might remain incremented on those early error paths. [Severity: High] This is also a pre-existing issue, but looking at that same snippet in probe(), the hardware clocks are enabled before the mali and sram regulators are fetched and enabled. Could this sequencing cause problems? Toggling clocks for unpowered IP domains can sometimes lead to bus lockups or undefined hardware behavior during initialization. > core: core_clk, > stacks: stacks_clk, [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
