Re: [PATCH v2 3/3] media: exynos4-is: fimc-is: replace duplicate pmu node with phandle

On 08/08/2023 13:42, Andi Shyti wrote:

quoted

quoted

quoted

+static void __iomem *fimc_is_get_pmu_regs(struct device *dev)
+{
+	struct device_node *node;
+	void __iomem *regs;
+
+	node = of_parse_phandle(dev->of_node, "samsung,pmu-syscon", 0);
+	if (!node) {
+		dev_warn(dev, "Finding PMU node via deprecated method, update your DTB\n");
+		node = of_get_child_by_name(dev->of_node, "pmu");
+		if (!node)
+			return IOMEM_ERR_PTR(-ENODEV);

in my opinion this should be:

		...
		if (!node)
			return IOMEM_ERR_PTR(-ENODEV);

		dev_warn(dev, "Finding PMU node via deprecated method, update your DTB\n");

Because if you don't have both "samsung,pmu-syscon and "pmu" then
the warning should not be printed and you need to return -ENODEV.

Why not? Warning is correct - the driver is trying to find, thus
continuous tense "Finding", PMU node via old method.

Alright, I'll go along with what you're suggesting, but I have to
say, I find it misleading.

From what I understand, you're requesting an update to the dtb
because it's using deprecated methods. However, the reality might 
be that the node is not present in any method at all.

Your statement would be accurate if you failed to find the
previous method but then did end up finding it.

Relying on the present continuous tense for clarity is a bold
move, don't you think? :)

I just don't think it matters and is not worth resending.

Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel