Re: [PATCH v2 3/3] clk: qcom: videocc-sm8750: Add video clock controller driver for SM8750

[Taniya Das <taniya.das@oss.qualcomm.com>]

On 9/12/2025 4:38 PM, Konrad Dybcio wrote:
> On 8/29/25 12:15 PM, Taniya Das wrote:
>> Add support for the video clock controller for video clients to be able
>> to request for videocc clocks on SM8750 platform.
>>
>> Signed-off-by: Taniya Das <taniya.das@oss.qualcomm.com>
>> ---
> 
> [...]
> 
>> +static int video_cc_sm8750_probe(struct platform_device *pdev)
>> +{
>> +	struct regmap *regmap;
>> +	int ret;
>> +
>> +	ret = devm_pm_runtime_enable(&pdev->dev);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = pm_runtime_resume_and_get(&pdev->dev);
>> +	if (ret)
>> +		return ret;
>> +
>> +	regmap = qcom_cc_map(pdev, &video_cc_sm8750_desc);
>> +	if (IS_ERR(regmap)) {
>> +		pm_runtime_put(&pdev->dev);
>> +		return PTR_ERR(regmap);
>> +	}
>> +
>> +	clk_taycan_elu_pll_configure(&video_cc_pll0, regmap, &video_cc_pll0_config);
>> +
>> +	/* Update DLY_ACCU_RED_SHIFTER_DONE to 0xF for mvs0, mvs0c */
>> +	regmap_update_bits(regmap, 0x8074, 0x1e00000, 0x1e00000);
> 
> regmap_update_bits(..., GENMASK(x, y) /* full field width */, 0xf)

Sure, Konrad, will update the change.

> 
> would be easier for the next person to check against docs in case this
> needs to ever change or be validated
>> +	regmap_update_bits(regmap, 0x8040, 0x1e00000, 0x1e00000);
>> +
>> +	regmap_update_bits(regmap, 0x9f24, BIT(0), BIT(0));
> 
> The register description mentions a ticket which I believe says this
> is not necessary in production hardware
> 

It is required on production hardware as well.

>> +
>> +	/*
>> +	 * Keep clocks always enabled:
>> +	 *	video_cc_ahb_clk
>> +	 *	video_cc_sleep_clk
>> +	 *	video_cc_xo_clk
>> +	 */
>> +	regmap_update_bits(regmap, 0x80a4, BIT(0), BIT(0));
>> +	regmap_update_bits(regmap, 0x80f8, BIT(0), BIT(0));
>> +	regmap_update_bits(regmap, 0x80d4, BIT(0), BIT(0));
> 
> Please use the new _desc infra
> 
> Konrad

Yes, will migrate to use the _desc infra.

-- 
Thanks,
Taniya Das