On Wed, Oct 15, 2025 at 4:02 PM Frank Li Frank.li@nxp.com wrote:

On Wed, Oct 15, 2025 at 03:36:05PM -0500, Rob Herring wrote:

...

On Wed, Oct 15, 2025 at 2:39 PM Frank Li Frank.li@nxp.com wrote:

...

On Wed, Oct 15, 2025 at 12:47:40PM -0500, Rob Herring (Arm) wrote:

...

Add a driver for Arm Ethos-U65/U85 NPUs. The Ethos-U NPU has a relatively simple interface with single command stream to describe buffers, operation settings, and network operations. It supports up to 8 memory regions (though no h/w bounds on a region). The Ethos NPUs are designed to use an SRAM for scratch memory. Region 2 is reserved for SRAM (like the downstream driver stack and compiler). Userspace doesn't need access to the SRAM.

...

...

...

+static int ethosu_init(struct ethosu_device *ethosudev) +{

• int ret;
  

• u32 id, config;
  

• ret = devm_pm_runtime_enable(ethosudev->base.dev);
  

• if (ret)
  

•         return ret;
  

• ret = pm_runtime_resume_and_get(ethosudev->base.dev);
  

• if (ret)
  

•         return ret;
  

• pm_runtime_set_autosuspend_delay(ethosudev->base.dev, 50);
  

• pm_runtime_use_autosuspend(ethosudev->base.dev);
  

• /* If PM is disabled, we need to call ethosu_device_resume() manually. */
  

• if (!IS_ENABLED(CONFIG_PM)) {
  

•         ret = ethosu_device_resume(ethosudev->base.dev);
  

•         if (ret)
  

•                 return ret;
  

• }
  

I think it should call ethosu_device_resume() unconditional before devm_pm_runtime_enable();

ethosu_device_resume(); pm_runtime_set_active(); pm_runtime_set_autosuspend_delay(ethosudev->base.dev, 50); devm_pm_runtime_enable();

Why do you think this? Does this do a get?

I don't think it is good to call the resume hook on our own, but we have no choice with !CONFIG_PM. With CONFIG_PM, we should only use the pm_runtime API.

Enable clock and do some init work at probe() is quite common. But I never seen IS_ENABLED(CONFIG_PM) check. It is quite weird and not necessary to check CONFIG_PM flags. The most CONFIG_PM is enabled, so the branch !CONFIG_PM almost never tested.

Okay, I get what you meant.

probe() { devm_clk_bulk_get_all_enabled();

    ... did some init work

    pm_runtime_set_active();
    devm_pm_runtime_enable();

    ...
    pm_runtime_put_autosuspend(ethosudev->base.dev);

}

I think we still need a pm_runtime_get_noresume() in here since we do a put later on. Here's what I have now:

ret = ethosu_device_resume(ethosudev->base.dev); if (ret) return ret;

pm_runtime_set_autosuspend_delay(ethosudev->base.dev, 50); pm_runtime_use_autosuspend(ethosudev->base.dev); ret = devm_pm_runtime_set_active_enabled(ethosudev->base.dev); if (ret) return ret; pm_runtime_get_noresume(ethosudev->base.dev);

Rob