Re: [PATCH 2/2] soc: qcom: Add CDSP power management driver

[Vignesh Viswanathan]

On 5/20/2026 4:20 PM, Krzysztof Kozlowski wrote:
> On Wed, May 20, 2026 at 12:35:10AM +0530, Vignesh Viswanathan wrote:
>> +/**
>> + * cdsp_power_probe() - Probe the CDSP power management driver
>> + * @pdev: Platform device
>> + *
>> + * Acquires the PMIC regulator consumer handles, registers the virtual
>> + * cdsp-vdd-cx (and optionally cdsp-vdd-mx) regulator providers, maps the
>> + * MPM and RSCC register regions, and registers the DCVS and LPM interrupt
>> + * handlers.
>> + *
>> + * Return: 0 on success, negative error code on failure
>> + */
>> +static int cdsp_power_probe(struct platform_device *pdev)
>> +{
>> +	struct regulator_config virt_cfg = {};
>> +	struct cdsp_power_driver *drv;
>> +	struct regulator_dev *rdev;
>> +	void __iomem *rscc_base;
>> +	void __iomem *mpm_base;
>> +	size_t smem_size;
>> +	u32 smem_id;
>> +	int ret;
>> +
>> +	/* Allocate driver context */
>> +	drv = devm_kzalloc(&pdev->dev, sizeof(*drv), GFP_KERNEL);
>> +	if (!drv)
>> +		return -ENOMEM;
>> +
>> +	drv->dev = &pdev->dev;
>> +	mutex_init(&drv->lock);
>> +	atomic_set(&drv->power_state, CDSP_POWER_ON);
>> +
>> +	/* Get SMEM item ID from device tree */
>> +	ret = of_property_read_u32(pdev->dev.of_node, "qcom,smem-item", &smem_id);
>> +	if (ret)
>> +		return dev_err_probe(&pdev->dev, ret, "Failed to get SMEM item ID\n");
>> +
>> +	/* Create SMEM entry for DCVS */
>> +	ret = qcom_smem_alloc(CDSP_SMEM_NSP_HOST_ID, smem_id, CDSP_SMEM_SIZE);
>> +	if (ret && ret != -EEXIST)
>> +		return dev_err_probe(&pdev->dev, ret, "Failed to allocate SMEM\n");
>> +
>> +	/* Get SMEM pointer and validate size */
>> +	drv->smem = qcom_smem_get(CDSP_SMEM_NSP_HOST_ID, smem_id, &smem_size);
>> +	if (IS_ERR(drv->smem))
>> +		return dev_err_probe(&pdev->dev, PTR_ERR(drv->smem),
>> +				     "Failed to get SMEM\n");
>> +
>> +	if (smem_size < CDSP_SMEM_SIZE)
>> +		return dev_err_probe(&pdev->dev, -EINVAL,
>> +				     "SMEM region too small: got %zu, expected %u\n",
>> +				     smem_size, CDSP_SMEM_SIZE);
>> +
>> +	/*
>> +	 * Initialise the SMEM channel header.
>> +	 * Zero the entire region first so all padding and reserved fields
>> +	 * are clean, then fill in the fixed protocol fields.
>> +	 * apss_state is set to 1 last (after wmb) so NSP Q6 only sees a
>> +	 * fully-populated header once APSS is ready.
>> +	 */
>> +	memset(drv->smem, 0, sizeof(*drv->smem));
>> +	drv->smem->hdr.magic           = CDSP_SMEM_MAGIC;
>> +	drv->smem->hdr.version         = CDSP_SMEM_VERSION;
>> +	drv->smem->hdr.request_offset  = CDSP_SMEM_REQUEST_OFFSET;
>> +	drv->smem->hdr.request_size    = CDSP_SMEM_REQUEST_SIZE;
>> +	drv->smem->hdr.response_offset = CDSP_SMEM_RESPONSE_OFFSET;
>> +	drv->smem->hdr.response_size   = CDSP_SMEM_RESPONSE_SIZE;
>> +	/* Signal APSS readiness to NSP Q6 */
>> +	WRITE_ONCE(drv->smem->hdr.apss_state, 1);
>> +	/* Ensure SMEM header is fully written before NSP Q6 reads it */
>> +	wmb();
>> +
>> +	/*
>> +	 * Get voltage regulator consumer handles.
>> +	 * These are the actual NSP_CX and NSP_MX voltage rails.
>> +	 * The virtual regulator ops pass through to these handles.
>> +	 */
>> +	drv->vdd_cx = devm_regulator_get(&pdev->dev, "vdd-cx");
>> +	if (IS_ERR(drv->vdd_cx))
>> +		return dev_err_probe(&pdev->dev, PTR_ERR(drv->vdd_cx),
>> +				     "Failed to get vdd-cx regulator\n");
>> +
>> +	drv->vdd_mx = devm_regulator_get_optional(&pdev->dev, "vdd-mx");
>> +	if (IS_ERR(drv->vdd_mx)) {
>> +		if (PTR_ERR(drv->vdd_mx) != -ENODEV)
>> +			return dev_err_probe(&pdev->dev, PTR_ERR(drv->vdd_mx),
>> +					     "Failed to get vdd-mx regulator\n");
>> +		drv->vdd_mx = NULL;
>> +		dev_dbg(&pdev->dev, "No vdd-mx regulator, MX rail absent on this board\n");
>> +	}
>> +
>> +	/*
>> +	 * Register virtual regulator provider.
>> +	 *
>> +	 * Expose vdd-cx and vdd-mx virtual regulators so that PAS remoteproc
>> +	 * can consume them via cx-supply / mx-supply DTS properties.
>> +	 * The enable/disable ops pass through to vdd_cx / vdd_mx above,
>> +	 * making CDSP the sole hardware power manager for the NSP subsystem.
>> +	 */
>> +	virt_cfg.dev         = &pdev->dev;
>> +	virt_cfg.driver_data = drv;
>> +	virt_cfg.of_node     = pdev->dev.of_node;
>> +
>> +	INIT_WORK(&drv->dcvs_work, cdsp_dcvs_work_fn);
>> +	INIT_WORK(&drv->lpm_work, cdsp_lpm_work_fn);
>> +
>> +	drv->lpm_wq = alloc_ordered_workqueue("cdsp_lpm_wq", 0);
>> +	if (!drv->lpm_wq) {
>> +		mbox_free_channel(drv->dcvs_mbox_chan);
>> +		return dev_err_probe(&pdev->dev,
>> +				     -ENOMEM,
>> +				     "failed to allocate cdsp lpm workqueue\n");
>> +	}
>> +
>> +	rdev = devm_regulator_register(&pdev->dev,
>> +				       &cdsp_virt_reg_descs[CDSP_VIRT_NSP_CX],
>> +				       &virt_cfg);
>> +	if (IS_ERR(rdev))
>> +		return dev_err_probe(&pdev->dev, PTR_ERR(rdev),
>> +				     "Failed to register cdsp-vdd-cx virtual regulator\n");
>> +
>> +	if (drv->vdd_mx) {
>> +		rdev = devm_regulator_register(&pdev->dev,
>> +					       &cdsp_virt_reg_descs[CDSP_VIRT_NSP_MX],
>> +					       &virt_cfg);
>> +		if (IS_ERR(rdev))
>> +			return dev_err_probe(&pdev->dev, PTR_ERR(rdev),
>> +					     "Failed to register cdsp-vdd-mx virtual regulator\n");
>> +	}
>> +
>> +	/* Register DCVS interrupt */
>> +	drv->dcvs_irq = platform_get_irq_byname(pdev, "dcvs");
>> +	if (drv->dcvs_irq < 0)
>> +		return dev_err_probe(&pdev->dev, drv->dcvs_irq,
>> +				     "Failed to get DCVS IRQ\n");
>> +
>> +	ret = devm_request_threaded_irq(&pdev->dev, drv->dcvs_irq,
>> +					NULL, cdsp_dcvs_irq_handler,
>> +					IRQF_ONESHOT, "cdsp-dcvs", drv);
>> +	if (ret)
>> +		return dev_err_probe(&pdev->dev, ret,
>> +				     "Failed to request DCVS IRQ\n");
>> +
>> +	/* Setup MPM for LPM */
>> +	mpm_base = devm_platform_ioremap_resource_byname(pdev, "mpm");
>> +	if (IS_ERR(mpm_base))
>> +		return dev_err_probe(&pdev->dev, PTR_ERR(mpm_base),
>> +				     "Failed to map MPM registers\n");
>> +
>> +	drv->mpm_regmap = devm_regmap_init_mmio(&pdev->dev, mpm_base, &cdsp_regmap_config);
>> +	if (IS_ERR(drv->mpm_regmap))
>> +		return dev_err_probe(&pdev->dev, PTR_ERR(drv->mpm_regmap),
>> +				     "Failed to init MPM regmap\n");
>> +
>> +	/* Setup RSCC for power mode detection */
>> +	rscc_base = devm_platform_ioremap_resource_byname(pdev, "rscc");
>> +	if (IS_ERR(rscc_base))
>> +		return dev_err_probe(&pdev->dev, PTR_ERR(rscc_base),
>> +				     "Failed to map RSCC registers\n");
>> +
>> +	drv->rscc_regmap = devm_regmap_init_mmio(&pdev->dev, rscc_base, &cdsp_rscc_regmap_config);
>> +	if (IS_ERR(drv->rscc_regmap))
>> +		return dev_err_probe(&pdev->dev, PTR_ERR(drv->rscc_regmap),
>> +				     "Failed to init RSCC regmap\n");
>> +
>> +	drv->lpm_irq = platform_get_irq_byname(pdev, "lpm");
>> +	if (drv->lpm_irq < 0)
>> +		return dev_err_probe(&pdev->dev, drv->lpm_irq,
>> +				     "Failed to get LPM IRQ\n");
>> +
>> +	ret = devm_request_threaded_irq(&pdev->dev, drv->lpm_irq,
>> +					NULL, cdsp_lpm_irq_handler,
>> +					IRQF_ONESHOT, "cdsp-lpm", drv);
>> +	if (ret)
>> +		return dev_err_probe(&pdev->dev, ret,
>> +				     "Failed to request LPM IRQ\n");
>> +
>> +	/* Setup mbox for DCVS response */
>> +	drv->dcvs_mbox_client.dev = &pdev->dev;
>> +	drv->dcvs_mbox_client.knows_txdone = true;
>> +	drv->dcvs_mbox_chan = mbox_request_channel(&drv->dcvs_mbox_client, 0);
>> +	if (IS_ERR(drv->dcvs_mbox_chan))
>> +		return dev_err_probe(&pdev->dev, PTR_ERR(drv->dcvs_mbox_chan),
>> +				     "Failed to get dcvs mbox channel\n");
>> +
>> +	platform_set_drvdata(pdev, drv);
>> +
>> +	dev_dbg(&pdev->dev, "CDSP power driver initialized\n");
> 
> Drop. All my comments from your regulator driver apply.

Ack, will address.

Thanks,
Vignesh
> 
>> +
>> +	return 0;
>> +}
> 
> Best regards,
> Krzysztof
>