Re: [PATCH v2 2/3] i3c: master: Add Qualcomm I3C controller driver

From: Mukesh Kumar Savaliya
Date: Fri Mar 28 2025 - 10:35:38 EST


Thanks Krzysztof for correcting !

On 3/26/2025 7:58 PM, Krzysztof Kozlowski wrote:
On 26/03/2025 15:16, Mukesh Kumar Savaliya wrote:
+
+static int i3c_geni_resources_init(struct geni_i3c_dev *gi3c, struct platform_device *pdev)
+{
+ int ret;
+
+ gi3c->se.base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(gi3c->se.base))
+ return PTR_ERR(gi3c->se.base);
+
+ gi3c->se.clk = devm_clk_get(&pdev->dev, "se-clk");

Never tested.

sorry, i ran and i got below error now:
i3c@884000: Unevaluated properties are not allowed ('clock-names', 'interrupts' were unexpected).

so i have made below change and ran dt_binding_check + dtbs_check, i could fix the issue. Let me have this review internally specific to dt-binding and will post next patch after internal review. So i can avoid such misses.

yaml :
+ clock-names:
+ const: se
+
- - interrupts-extended
+ - interrupts

example:
clock-names = "se";

+ if (IS_ERR(gi3c->se.clk))
+ return dev_err_probe(&pdev->dev, PTR_ERR(gi3c->se.clk),
+ "Unable to get serial engine core clock: %pe\n",
+ gi3c->se.clk);
+
+ ret = device_property_read_u32(&pdev->dev, "se-clock-frequency", &gi3c->clk_src_freq);

I don't see previous comments implemented. Comment was: "Drop".

You did not test the DTS - again - even though I asked for that.

Sorry for miss. I did test for this but what happened is while i worked for multiple other changes, i did miss merging of removal of se-clock-frequency from the driver code. I had hard-coded it instead of removing because we don't give this frequency selection to user right now.

You claim you did internal review, but I have doubts because internal
review would tell you how to test it (there is comprehensive internal
guide - see go/upstream). Then the testing would point out this issue.

Such trivial things should not be in v1 even. But they happened so you
got the review. Now you sent v2 ignoring that public review.

Sorry guys, please improve internal processes instead of wasting
reviewers time.

Sure, i agree there are misses. will have internal review with dt-bindings and related changes in driver, then will post V3.
NAK.

Best regards,
Krzysztof