On 10/02/2025 10:07, Nas Chung wrote:

+
+int wave6_vpu_ctrl_resume_and_get(struct device *dev, struct wave6_vpu_entity *entity)
+{
+	struct vpu_ctrl *ctrl = dev_get_drvdata(dev);
+	bool boot;
+	int ret;
+
+	if (!ctrl)
+		return -EINVAL;
+
+	if (!entity || !entity->dev || !entity->read_reg || !entity->write_reg)
+		return -EINVAL;
+
+	mutex_lock(&ctrl->ctrl_lock);
+
+	ret = pm_runtime_resume_and_get(ctrl->dev);
+	if (ret) {
+		dev_err(dev, "pm runtime resume fail, ret = %d\n", ret);
+		mutex_unlock(&ctrl->ctrl_lock);
+		return ret;
+	}
+
+	entity->booted = false;
+
+	if (ctrl->current_entity)
+		boot = false;
+	else
+		boot = list_empty(&ctrl->entities) ? true : false;
+
+	list_add_tail(&entity->list, &ctrl->entities);
+	if (boot)
+		ret = wave6_vpu_ctrl_try_boot(ctrl, entity);
+
+	if (ctrl->state == WAVE6_VPU_STATE_ON)
+		wave6_vpu_ctrl_on_boot(entity);
+
+	if (ret)
+		pm_runtime_put_sync(ctrl->dev);
+
+	mutex_unlock(&ctrl->ctrl_lock);
+
+	return ret;
+}

Drop entire function.

+EXPORT_SYMBOL_GPL(wave6_vpu_ctrl_resume_and_get);

Drop.

+
+void wave6_vpu_ctrl_put_sync(struct device *dev, struct wave6_vpu_entity *entity)
+{
+	struct vpu_ctrl *ctrl = dev_get_drvdata(dev);
+
+	if (!ctrl)
+		return;
+
+	if (entity == ctrl->current_entity)
+		wave6_vpu_ctrl_wait_done(dev);
+
+	mutex_lock(&ctrl->ctrl_lock);
+
+	if (!wave6_vpu_ctrl_find_entity(ctrl, entity))
+		goto exit;
+
+	list_del_init(&entity->list);
+	if (list_empty(&ctrl->entities)) {
+		if (!entity->read_reg(entity->dev, W6_VPU_VCPU_CUR_PC))
+			wave6_vpu_ctrl_set_state(ctrl, WAVE6_VPU_STATE_OFF);
+		else
+			wave6_vpu_ctrl_sleep(ctrl, entity);
+	}
+
+	if (!pm_runtime_suspended(ctrl->dev))
+		pm_runtime_put_sync(ctrl->dev);
+exit:
+	mutex_unlock(&ctrl->ctrl_lock);
+}
+EXPORT_SYMBOL_GPL(wave6_vpu_ctrl_put_sync);

Drop entire function. Dead code.

... or you arranged your patches really incorrectly, but this I can't
really judge.

+
+int wave6_vpu_ctrl_wait_done(struct device *dev)
+{
+	struct vpu_ctrl *ctrl = dev_get_drvdata(dev);
+	int ret;
+
+	if (!ctrl)
+		return -EINVAL;
+
+	if (ctrl->state == WAVE6_VPU_STATE_OFF)
+		return -EINVAL;
+
+	if (ctrl->state == WAVE6_VPU_STATE_ON)
+		return 0;
+
+	ret = wave6_wait_event_freezable_timeout(ctrl->load_fw_wq,
+						 wave6_vpu_ctrl_get_state(dev) ==
+						 WAVE6_VPU_STATE_ON,
+						 msecs_to_jiffies(W6_BOOT_WAIT_TIMEOUT));
+	if (ret == -ERESTARTSYS || !ret) {
+		dev_err(ctrl->dev, "fail to wait vcpu boot done,ret %d\n", ret);
+		mutex_lock(&ctrl->ctrl_lock);
+		wave6_vpu_ctrl_set_state(ctrl, WAVE6_VPU_STATE_OFF);
+		mutex_unlock(&ctrl->ctrl_lock);
+		return -EINVAL;
+	}
+
+	mutex_lock(&ctrl->ctrl_lock);
+	wave6_vpu_ctrl_boot_done(ctrl, 0);
+	mutex_unlock(&ctrl->ctrl_lock);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(wave6_vpu_ctrl_wait_done);

There is no user of this outside. Drop.

+
+int wave6_vpu_ctrl_get_state(struct device *dev)
+{
+	struct vpu_ctrl *ctrl = dev_get_drvdata(dev);
+
+	if (!ctrl)
+		return -EINVAL;
+
+	return ctrl->state;
+}
+EXPORT_SYMBOL_GPL(wave6_vpu_ctrl_get_state);

There is no user of this outside. Drop.

Whatever export stays, must be used by a following patch. You *must* add
the kerneldoc for it.

+
+static void wave6_vpu_ctrl_init_reserved_boot_region(struct vpu_ctrl *ctrl)
+{
+	if (ctrl->boot_mem.size < WAVE6_CODE_BUF_SIZE) {
+		dev_warn(ctrl->dev, "boot memory size (%zu) is too small\n", ctrl->boot_mem.size);
+		ctrl->boot_mem.phys_addr = 0;
+		ctrl->boot_mem.size = 0;
+		memset(&ctrl->boot_mem, 0, sizeof(ctrl->boot_mem));
+		return;
+	}
+
+	ctrl->boot_mem.vaddr = devm_memremap(ctrl->dev,
+					     ctrl->boot_mem.phys_addr,
+					     ctrl->boot_mem.size,
+					     MEMREMAP_WC);
+	if (!ctrl->boot_mem.vaddr) {
+		memset(&ctrl->boot_mem, 0, sizeof(ctrl->boot_mem));
+		return;
+	}
+
+	ctrl->boot_mem.dma_addr = dma_map_resource(ctrl->dev,
+						   ctrl->boot_mem.phys_addr,
+						   ctrl->boot_mem.size,
+						   DMA_BIDIRECTIONAL,
+						   0);
+	if (!ctrl->boot_mem.dma_addr) {
+		memset(&ctrl->boot_mem, 0, sizeof(ctrl->boot_mem));
+		return;
+	}
+
+	dev_info(ctrl->dev, "boot phys_addr: %pad, dma_addr: %pad, size: 0x%zx\n",
+		 &ctrl->boot_mem.phys_addr,
+		 &ctrl->boot_mem.dma_addr,
+		 ctrl->boot_mem.size);

No, drop. Reasoning further.

+}

...

+
+	ctrl->num_clks = ret;
+
+	np = of_parse_phandle(pdev->dev.of_node, "boot", 0);
+	if (np) {
+		struct resource mem;
+
+		ret = of_address_to_resource(np, 0, &mem);
+		of_node_put(np);
+		if (!ret) {
+			ctrl->boot_mem.phys_addr = mem.start;
+			ctrl->boot_mem.size = resource_size(&mem);
+			wave6_vpu_ctrl_init_reserved_boot_region(ctrl);
+		} else {
+			dev_warn(&pdev->dev, "boot resource is not available.\n");
+		}
+	}
+
+	ctrl->sram_pool = of_gen_pool_get(pdev->dev.of_node, "sram", 0);
+	if (ctrl->sram_pool) {
+		ctrl->sram_buf.size = ctrl->res->sram_size;
+		ctrl->sram_buf.vaddr = gen_pool_dma_alloc(ctrl->sram_pool,
+							  ctrl->sram_buf.size,
+							  &ctrl->sram_buf.phys_addr);
+		if (!ctrl->sram_buf.vaddr)
+			ctrl->sram_buf.size = 0;
+		else
+			ctrl->sram_buf.dma_addr = dma_map_resource(&pdev->dev,
+								   ctrl->sram_buf.phys_addr,
+								   ctrl->sram_buf.size,
+								   DMA_BIDIRECTIONAL,
+								   0);
+
+		dev_info(&pdev->dev, "sram 0x%pad, 0x%pad, size 0x%lx\n",
+			 &ctrl->sram_buf.phys_addr, &ctrl->sram_buf.dma_addr, ctrl->sram_buf.size);

You are not supposed to print addresses. This look like fixed hardware
mappings, so probably harmless but dma_addr not. Drop entire dev_info,
this is really discouraged practice.

+	}
+
+	if (of_find_property(pdev->dev.of_node, "support-follower", NULL))
+		ctrl->support_follower = true;
+
+	wave6_cooling_init(ctrl);
+
+	pm_runtime_enable(&pdev->dev);
+
+	return 0;
+}
+
+static void wave6_vpu_ctrl_remove(struct platform_device *pdev)
+{
+	struct vpu_ctrl *ctrl = dev_get_drvdata(&pdev->dev);
+
+	pm_runtime_disable(&pdev->dev);
+
+	wave6_vpu_ctrl_clear_buffers(ctrl);
+	wave6_cooling_remove(ctrl);
+	if (ctrl->sram_pool && ctrl->sram_buf.vaddr) {
+		dma_unmap_resource(&pdev->dev,
+				   ctrl->sram_buf.dma_addr,
+				   ctrl->sram_buf.size,
+				   DMA_BIDIRECTIONAL,
+				   0);
+		gen_pool_free(ctrl->sram_pool,
+			      (unsigned long)ctrl->sram_buf.vaddr,
+			      ctrl->sram_buf.size);
+	}
+	if (ctrl->boot_mem.dma_addr)
+		dma_unmap_resource(&pdev->dev,
+				   ctrl->boot_mem.dma_addr,
+				   ctrl->boot_mem.size,
+				   DMA_BIDIRECTIONAL,
+				   0);
+	mutex_destroy(&ctrl->ctrl_lock);
+}
+
+#ifdef CONFIG_PM
+static int wave6_vpu_ctrl_runtime_suspend(struct device *dev)
+{
+	struct vpu_ctrl *ctrl = dev_get_drvdata(dev);
+
+	clk_bulk_disable_unprepare(ctrl->num_clks, ctrl->clks);
+	return 0;
+}
+
+static int wave6_vpu_ctrl_runtime_resume(struct device *dev)
+{
+	struct vpu_ctrl *ctrl = dev_get_drvdata(dev);
+
+	return clk_bulk_prepare_enable(ctrl->num_clks, ctrl->clks);
+}
+#endif
+
+#ifdef CONFIG_PM_SLEEP
+static int wave6_vpu_ctrl_suspend(struct device *dev)
+{
+	return 0;

Why do you need empty callbacks?

Best regards,
Krzysztof