[PATCH v3 0/2] ACPI serdev support

Add ACPI support for serial attached devices.

Currently, serial devices are not set as enumerated during ACPI scan for SPI
or i2c buses (but not for UART). This should also be done for UART serial
devices.
I renamed *spi_i2c_slave* to *serial_bus_slave* to reflect this.

This needs Johan Hovold's "serdev: fix registration of second slave" patch.

Tested on T100TA with Broadcom BCM2E39.

Since v2:
  - Remove ctrl->serdev set to NULL in acpi_serdev_register_device() in favor
    of Johan's patch
  - Fallback to ctrl->serdev check when acpi_walk_namespace() returns an error
    to prevent memory leak
  - Remove a change in dev_dbg() call in serdev_controller_add(), this will
    be done in separate patch
Since v1:
  - Check if a serdev device as been allocated during acpi_walk_namespace() to
    prevent serdev controller registration instead of the tty-class device.
  - Reword dev_dbg() strings replacing Serial by serdev
  - Removing redundant "serdev%d" in dev_dbg() calls in serdev_controller_add()
Since RFC:
  - Add or reword commit messages
  - Rename *serial_slave* to *serial_bus_slave*
  - Add specific check for Apple in acpi_is_serial_bus_slave(), thanks to
    Lukas Wunner
  - Update comment in acpi_default_enumeration()
  - Remove patch 3 "Bluetooth: hci_bcm: Add ACPI serdev support for BCM2E39"
    in favor of patches from Hans de Goede

Frédéric Danis (2):
  serdev: Add ACPI support
  ACPI / scan: Fix enumeration for special UART devices

 drivers/acpi/scan.c       |  37 ++++++++---------
 drivers/tty/serdev/core.c | 100 +++++++++++++++++++++++++++++++++++++++++++---
 include/acpi/acpi_bus.h   |   2 +-
 3 files changed, 113 insertions(+), 26 deletions(-)

-- 
2.7.4

UART devices is expected to be enumerated by SerDev subsystem.

During ACPI scan, serial devices behind SPI, I2C or UART buses are not
enumerated, allowing them to be enumerated by their respective parents.

Rename *spi_i2c_slave* to *serial_bus_slave* as this will be used for serial
devices on serial buses (SPI, I2C or UART).

On Macs an empty ResourceTemplate is returned for uart slaves.
Instead the device properties "baud", "parity", "dataBits", "stopBits" are
provided. Add a check for "baud" in acpi_is_serial_bus_slave().

Signed-off-by: Frédéric Danis <redacted>
Reviewed-by: Sebastian Reichel <redacted>
---
 drivers/acpi/scan.c     | 37 +++++++++++++++++--------------------
 include/acpi/acpi_bus.h |  2 +-
 2 files changed, 18 insertions(+), 21 deletions(-)

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 602f8ff..860b698 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c

@@ -1505,41 +1505,38 @@ static void acpi_init_coherency(struct acpi_device *adev)
 	adev->flags.coherent_dma = cca;
 }
 
-static int acpi_check_spi_i2c_slave(struct acpi_resource *ares, void *data)
+static int acpi_check_serial_bus_slave(struct acpi_resource *ares, void *data)
 {
-	bool *is_spi_i2c_slave_p = data;
+	bool *is_serial_bus_slave_p = data;
 
 	if (ares->type != ACPI_RESOURCE_TYPE_SERIAL_BUS)
 		return 1;
 
-	/*
-	 * devices that are connected to UART still need to be enumerated to
-	 * platform bus
-	 */
-	if (ares->data.common_serial_bus.type != ACPI_RESOURCE_SERIAL_TYPE_UART)
-		*is_spi_i2c_slave_p = true;
+	*is_serial_bus_slave_p = true;
 
 	 /* no need to do more checking */
 	return -1;
 }
 
-static bool acpi_is_spi_i2c_slave(struct acpi_device *device)
+static bool acpi_is_serial_bus_slave(struct acpi_device *device)
 {
 	struct list_head resource_list;
-	bool is_spi_i2c_slave = false;
+	bool is_serial_bus_slave = false;
 
 	/* Macs use device properties in lieu of _CRS resources */
 	if (x86_apple_machine &&
 	    (fwnode_property_present(&device->fwnode, "spiSclkPeriod") ||
-	     fwnode_property_present(&device->fwnode, "i2cAddress")))
+	     fwnode_property_present(&device->fwnode, "i2cAddress") ||
+	     fwnode_property_present(&device->fwnode, "baud")))
 		return true;
 
 	INIT_LIST_HEAD(&resource_list);
-	acpi_dev_get_resources(device, &resource_list, acpi_check_spi_i2c_slave,
-			       &is_spi_i2c_slave);
+	acpi_dev_get_resources(device, &resource_list,
+			       acpi_check_serial_bus_slave,
+			       &is_serial_bus_slave);
 	acpi_dev_free_resource_list(&resource_list);
 
-	return is_spi_i2c_slave;
+	return is_serial_bus_slave;
 }
 
 void acpi_init_device_object(struct acpi_device *device, acpi_handle handle,

@@ -1557,7 +1554,7 @@ void acpi_init_device_object(struct acpi_device *device, acpi_handle handle,
 	acpi_bus_get_flags(device);
 	device->flags.match_driver = false;
 	device->flags.initialized = true;
-	device->flags.spi_i2c_slave = acpi_is_spi_i2c_slave(device);
+	device->flags.serial_bus_slave = acpi_is_serial_bus_slave(device);
 	acpi_device_clear_enumerated(device);
 	device_initialize(&device->dev);
 	dev_set_uevent_suppress(&device->dev, true);

@@ -1841,10 +1838,10 @@ static acpi_status acpi_bus_check_add(acpi_handle handle, u32 lvl_not_used,
 static void acpi_default_enumeration(struct acpi_device *device)
 {
 	/*
-	 * Do not enumerate SPI/I2C slaves as they will be enumerated by their
-	 * respective parents.
+	 * Do not enumerate SPI/I2C/UART slaves as they will be enumerated by
+	 * their respective parents.
 	 */
-	if (!device->flags.spi_i2c_slave) {
+	if (!device->flags.serial_bus_slave) {
 		acpi_create_platform_device(device, NULL);
 		acpi_device_set_enumerated(device);
 	} else {

@@ -1941,7 +1938,7 @@ static void acpi_bus_attach(struct acpi_device *device)
 		return;
 
 	device->flags.match_driver = true;
-	if (ret > 0 && !device->flags.spi_i2c_slave) {
+	if (ret > 0 && !device->flags.serial_bus_slave) {
 		acpi_device_set_enumerated(device);
 		goto ok;
 	}

@@ -1950,7 +1947,7 @@ static void acpi_bus_attach(struct acpi_device *device)
 	if (ret < 0)
 		return;
 
-	if (!device->pnp.type.platform_id && !device->flags.spi_i2c_slave)
+	if (!device->pnp.type.platform_id && !device->flags.serial_bus_slave)
 		acpi_device_set_enumerated(device);
 	else
 		acpi_default_enumeration(device);

diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index fa15052..f849be2 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h

@@ -211,7 +211,7 @@ struct acpi_device_flags {
 	u32 of_compatible_ok:1;
 	u32 coherent_dma:1;
 	u32 cca_seen:1;
-	u32 spi_i2c_slave:1;
+	u32 serial_bus_slave:1;
 	u32 reserved:19;
 };
 

-- 
2.7.4

On Wed, Oct 11, 2017 at 10:32:12AM +0200, Frédéric Danis wrote:

Add ACPI support for serial attached devices.

Currently, serial devices are not set as enumerated during ACPI scan for SPI
or i2c buses (but not for UART). This should also be done for UART serial
devices.
I renamed *spi_i2c_slave* to *serial_bus_slave* to reflect this.

This needs Johan Hovold's "serdev: fix registration of second slave" patch.

In theory this series could go in through the acpi-tree without my
fix. It would only affect an error case where an unlikely failure to
register an ACPI serdev device, would prevent the tty-class device from
being registered instead of the controller. That is, something we can
live with until this all converges in 4.15-rc1 if needed.

That said, I think we should consider taking all serdev changes, and
therefore also the ACPI patch, through the tty tree instead in order to
avoid merge conflicts. Rafael?

Johan

On Wed, Oct 11, 2017 at 11:03 AM, Johan Hovold [off-list ref] wrote:

On Wed, Oct 11, 2017 at 10:32:12AM +0200, Frédéric Danis wrote:

quoted

Add ACPI support for serial attached devices.

Currently, serial devices are not set as enumerated during ACPI scan for SPI
or i2c buses (but not for UART). This should also be done for UART serial
devices.
I renamed *spi_i2c_slave* to *serial_bus_slave* to reflect this.

This needs Johan Hovold's "serdev: fix registration of second slave" patch.

In theory this series could go in through the acpi-tree without my
fix. It would only affect an error case where an unlikely failure to
register an ACPI serdev device, would prevent the tty-class device from
being registered instead of the controller. That is, something we can
live with until this all converges in 4.15-rc1 if needed.

That said, I think we should consider taking all serdev changes, and
therefore also the ACPI patch, through the tty tree instead in order to
avoid merge conflicts. Rafael?

OK

Please feel free to add

Acked-by: Rafael J. Wysocki <redacted>

to the ACPI core change.

And I will assume that this series will go in via the tty tree.

Thanks,
Rafael

Hi Greg,

quoted

quoted

Add ACPI support for serial attached devices.

Currently, serial devices are not set as enumerated during ACPI scan for SPI
or i2c buses (but not for UART). This should also be done for UART serial
devices.
I renamed *spi_i2c_slave* to *serial_bus_slave* to reflect this.

This needs Johan Hovold's "serdev: fix registration of second slave" patch.

In theory this series could go in through the acpi-tree without my
fix. It would only affect an error case where an unlikely failure to
register an ACPI serdev device, would prevent the tty-class device from
being registered instead of the controller. That is, something we can
live with until this all converges in 4.15-rc1 if needed.

That said, I think we should consider taking all serdev changes, and
therefore also the ACPI patch, through the tty tree instead in order to
avoid merge conflicts. Rafael?

OK

Please feel free to add

Acked-by: Rafael J. Wysocki <redacted>

to the ACPI core change.

And I will assume that this series will go in via the tty tree.

you have to take these two patches now via the TTY tree now. In case you already marked them as someone else problem ;)

Regards

Marcel

On Wed, Oct 11, 2017 at 10:32:14AM +0200, Frédéric Danis wrote:

UART devices is expected to be enumerated by SerDev subsystem.

During ACPI scan, serial devices behind SPI, I2C or UART buses are not
enumerated, allowing them to be enumerated by their respective parents.

Rename *spi_i2c_slave* to *serial_bus_slave* as this will be used for serial
devices on serial buses (SPI, I2C or UART).

On Macs an empty ResourceTemplate is returned for uart slaves.
Instead the device properties "baud", "parity", "dataBits", "stopBits" are
provided. Add a check for "baud" in acpi_is_serial_bus_slave().

Tested-by: Ronald Tschalär <redacted>
Tested-by: Peter Y. Chuang <redacted>

Ronald and Peter both report success for the above-mentioned Mac-specific
change on GitHub: https://github.com/l1k/linux/pull/1#issuecomment-336126330

Thanks,

Lukas

Hi Greg, Rafael, Marcel,

Le 11/10/2017 à 20:32, Marcel Holtmann a écrit :

Hi Greg,

quoted

quoted

quoted

Add ACPI support for serial attached devices.

Currently, serial devices are not set as enumerated during ACPI scan for SPI
or i2c buses (but not for UART). This should also be done for UART serial
devices.
I renamed *spi_i2c_slave* to *serial_bus_slave* to reflect this.

This needs Johan Hovold's "serdev: fix registration of second slave" patch.

In theory this series could go in through the acpi-tree without my
fix. It would only affect an error case where an unlikely failure to
register an ACPI serdev device, would prevent the tty-class device from
being registered instead of the controller. That is, something we can
live with until this all converges in 4.15-rc1 if needed.

That said, I think we should consider taking all serdev changes, and
therefore also the ACPI patch, through the tty tree instead in order to
avoid merge conflicts. Rafael?

OK

Please feel free to add

Acked-by: Rafael J. Wysocki <redacted>

to the ACPI core change.

And I will assume that this series will go in via the tty tree.

you have to take these two patches now via the TTY tree now. In case you already marked them as someone else problem ;)

Regards

Marcel

Is there any problem I missed with those patches?
Do I have to re-send them?

Regards,

Fred

On Wed, Oct 18, 2017 at 04:46:05PM +0200, Frédéric Danis wrote:

Hi Greg, Rafael, Marcel,

Le 11/10/2017 à 20:32, Marcel Holtmann a écrit :

quoted

Hi Greg,

quoted

quoted

quoted

Add ACPI support for serial attached devices.

Currently, serial devices are not set as enumerated during ACPI scan for SPI
or i2c buses (but not for UART). This should also be done for UART serial
devices.
I renamed *spi_i2c_slave* to *serial_bus_slave* to reflect this.

This needs Johan Hovold's "serdev: fix registration of second slave" patch.

In theory this series could go in through the acpi-tree without my
fix. It would only affect an error case where an unlikely failure to
register an ACPI serdev device, would prevent the tty-class device from
being registered instead of the controller. That is, something we can
live with until this all converges in 4.15-rc1 if needed.

That said, I think we should consider taking all serdev changes, and
therefore also the ACPI patch, through the tty tree instead in order to
avoid merge conflicts. Rafael?

OK

Please feel free to add

Acked-by: Rafael J. Wysocki <redacted>

to the ACPI core change.

And I will assume that this series will go in via the tty tree.

you have to take these two patches now via the TTY tree now. In case you already marked them as someone else problem ;)

Regards

Marcel

Is there any problem I missed with those patches?
Do I have to re-send them?

No, they are in my queue, still catching up...

thanks,

greg k-h