Re: [PATCH 2/4] platform/x86: Add new get_serdev_controller() helper

[Tony Lindgren <tony@atomide.com>]

* Hans de Goede <hdegoede@redhat.com> [240216 20:17]:
> +static inline struct device *
> +get_serdev_controller(const char *serial_ctrl_hid,
> +		      const char *serial_ctrl_uid,
> +		      int serial_ctrl_port,
> +		      const char *serdev_ctrl_name)
> +{
> +	struct device *ctrl_dev, *child;
> +	struct acpi_device *ctrl_adev;
> +	char name[32];
> +	int i;
> +
> +	ctrl_adev = acpi_dev_get_first_match_dev(serial_ctrl_hid, serial_ctrl_uid, -1);
> +	if (!ctrl_adev) {
> +		pr_err("error could not get %s/%s serial-ctrl adev\n",
> +		       serial_ctrl_hid, serial_ctrl_uid);
> +		return ERR_PTR(-ENODEV);
> +	}

Maybe split get_serdev_controller() into two functions, an acpi specific
function, and a serdev specific function? And I also think these should
not be in the header file, maybe splitting will also help with that :)

> +	/* get_first_physical_node() returns a weak ref */
> +	ctrl_dev = get_device(acpi_get_first_physical_node(ctrl_adev));
> +	if (!ctrl_dev) {
> +		pr_err("error could not get %s/%s serial-ctrl physical node\n",
> +		       serial_ctrl_hid, serial_ctrl_uid);
> +		ctrl_dev = ERR_PTR(-ENODEV);
> +		goto put_ctrl_adev;
> +	}
> +
> +	/* Walk host -> uart-ctrl -> port -> serdev-ctrl */
> +	for (i = 0; i < 3; i++) {
> +		switch (i) {
> +		case 0:
> +			snprintf(name, sizeof(name), "%s:0", dev_name(ctrl_dev));

Note that in theory it's possible we will encounter a device that has
multiple serial core controller instances as noted by Jiri earlier.

And each controller may have one or more ports. For the multiport test
case, you can use qemu with options like below FYI:

-chardev null,id=s1 -chardev null,id=s2 \
-device pci-serial-2x,chardev1=s1,chardev2=s2

Regards,

Tony