Thanks Krzysztof !

On 2/9/2025 5:10 PM, Krzysztof Kozlowski wrote:

On 07/02/2025 13:03, Mukesh Kumar Savaliya wrote:

quoted

quoted

quoted

+	gi3c->se.clk = devm_clk_get(&pdev->dev, "se-clk");
+	if (IS_ERR(gi3c->se.clk)) {
+		ret = PTR_ERR(gi3c->se.clk);
+		dev_err(&pdev->dev, "Error getting SE Core clk %d\n", ret);
+		return ret;
+	}
+
+	ret = device_property_read_u32(&pdev->dev, "se-clock-frequency", &gi3c->clk_src_freq);

You never tested your DTS or this code... Drop

I have tested on SM8550 MTP only. Below entry in my internal/local DTSI.

And how is it supposed to work? Are you going to send us your local
internal DTSI? Is it going to pass any checks?

was saying about code was testing with MTP. DTS was tested using 
dt-bindings check.
I should add "se-clock-frequency" and "dfs-index"

quoted

Do you mean to say i should add this property in yaml too ?

Yes.

You cannot add undocumented ABI. That's a strong NAK.

sure , adding both properties into yaml.

quoted

se-clock-frequency = <100000000>;

quoted

quoted

+	if (ret) {
+		dev_info(&pdev->dev, "SE clk freq not specified, default to 100 MHz.\n");
+		gi3c->clk_src_freq = 100000000;
+	}
+
+	ret = geni_icc_get(&gi3c->se, NULL);
+	if (ret)
+		return ret;
+
+	/* Set the bus quota to a reasonable value for register access */
+	gi3c->se.icc_paths[GENI_TO_CORE].avg_bw = GENI_DEFAULT_BW;
+	gi3c->se.icc_paths[CPU_TO_GENI].avg_bw = GENI_DEFAULT_BW;
+	ret = geni_icc_set_bw(&gi3c->se);
+	if (ret) {
+		dev_err(&pdev->dev, "%s: icc set bw failed ret:%d\n", __func__, ret);
+		return ret;
+	}
+	dev_dbg(&pdev->dev, "%s: GENI_TO_CORE:%d CPU_TO_GENI:%d\n", __func__,
+		gi3c->se.icc_paths[GENI_TO_CORE].avg_bw, gi3c->se.icc_paths[CPU_TO_GENI].avg_bw);
+
+	ret = device_property_read_u32(&pdev->dev, "dfs-index", &gi3c->dfs_idx);

The same. You cannot send us hidden ABI.

This code does not look like ready for upstream. Are you sure it was
internally reviewed?

yes, we have taken 2 rounds internally.

And none of the reviewers spotted undocumented ABI? OK, learning
experience for me.

quoted

Are you saying i should add this into yaml ?  what do you mean by
hiddern ABI ?

Where is the documentation of the ABI?

can you please point to other ABI document so i can prepare and share it.

Best regards,
Krzysztof