Hi,

On Mon, Jun 29, 2015 at 06:44:23PM +0200, Dr. H. Nikolaus Schaller wrote:
[...]

quoted

quoted

+	list_for_each_entry(uart, &uart_list, head) {
+		if (node != uart->dev->of_node)
+			continue;
+
+		return uart;

We can easily save three lines here :)

Hm. We have copied from here:

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/usb/phy/phy.c?id=refs/tags/v4.1#n65

So please let us know how you want to save 3 lines.

Sorry for not being specific, I just meant that it could be written as

if (node == uart->dev->of_node)
	return uart;

quoted

quoted

+/**
+ * devm_serial_get_uart_by_phandle - find the uart by phandle
+ * @dev - device that requests this uart
+ * @phandle - name of the property holding the uart phandle value
+ * @index - the index of the uart
+ *
+ * Returns the uart_port associated with the given phandle value,
+ * after getting a refcount to it, -ENODEV if there is no such uart or
+ * -EPROBE_DEFER if there is a phandle to the uart, but the device is
+ * not yet loaded. While at that, it also associates the device with
+ * the uart using devres. On driver detach, release function is invoked
+ * on the devres data, then, devres data is freed.

Add -ENOMEM and -EINVAL, remove -EPROBE_DEFER?

Well, if the device is not loaded it means the caller must return -EPROBE_DEFER
anyways since it can’t complete it’s probe function.

That was my mistake, from the first sight I hadn't found where
-EPROBE_DEFER is actually returned from the code and thus decided that
kernel-doc has an error. But now I see it, thanks.

quoted

quoted

+
+	*ptr = uart;
+	devres_add(dev, ptr);

What is the point of assigning value to *ptr?

Good question. I think it is necessary to store a copy of the found uart/phy instead of a reference.
Therefore the assignment to *ptr copies into the new memory area allocated above by 

ptr = devres_alloc(devm_serial_uart_release, sizeof(*ptr), GFP_KERNEL);

This makes the dev the owner of the data - instead of unknown ownership before.

It is the same as here (but it might be wrong/unnecessary there as well):

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/usb/phy/phy.c?id=refs/tags/v4.1#n209

Maybe it has something to do with the unfinished devm_serial_uart_release().

Indeed. I haven't noticed this through the first quick look into the
code. Thank you for explanation.