Re: [PATCH 2/6] clk: qcom: Add support for GPU Clock Controller on QCS8300

From: Imran Shaik
Date: Tue Oct 22 2024 - 02:34:40 EST




On 10/21/2024 8:41 PM, Krzysztof Kozlowski wrote:
On 21/10/2024 12:56, Dmitry Baryshkov wrote:
{ }
};
@@ -596,6 +635,14 @@ static int gpu_cc_sa8775p_probe(struct platform_device *pdev)
if (IS_ERR(regmap))
return PTR_ERR(regmap);
+ if (of_device_is_compatible(pdev->dev.of_node, "qcom,qcs8300-gpucc")) {

Why we cannot use match data? Seeing compatibles in the code is
unexpected and does not scale.

Because using match data doesn't scale in such cases. We have been using

I don't understand how it could not scale. That's the entire point of
match data - scaling.

compatibles to patch clock trees for the platforms for quite a while.
You can see that each of the "tunings" is slightly different. From my


You have one driver, where are these tunings which are supposed to be
different? You need here only enum or define, in the simplest choice.

point of view, this approach provides a nice balance between having a
completely duplicate driver and having a driver which self-patches the
tree.

How duplicate driver got into this? I don't think we talk about the
same. I meant ID table match data.


I agree with Dmitry. If I understand correctly, to add match data support, we need to define the gpu_cc_qcs8300_clocks struct by duplicating the entries from gpu_cc_sa8775p_clocks and then adding the additional qcs8300 clocks. The compatible approach is simpler and used across most existing platforms.

Thanks,
Imran


Best regards,
Krzysztof