Hi,
On 5/22/21 7:37 PM, Jonathan Cameron wrote:
On Fri, 21 May 2021 19:14:13 +0200 Hans de Goede hdegoede@redhat.com wrote:
Move the check for a second ACPI device for BOSC0200 ACPI fwnodes into a new bmc150_acpi_dual_accel_probe() helper function.
This is a preparation patch for adding support for a new "DUAL250E" ACPI Hardware-ID (HID) used on some devices.
Signed-off-by: Hans de Goede hdegoede@redhat.com
A few places I'd like comments rewrapped on basis of still having a minor preference for a 80 chars limit unless there is a reason to do otherwise.
If this is all that turns up in the series, I can do that whilst applying.
Thank for the review.
If you can do the comment rewrapping while applying that would be great, thanks. If a v2 is necessary I will take care of the rewrapping myself.
Regards,
Hans
Thanks,
Jonathan
drivers/iio/accel/bmc150-accel-i2c.c | 80 +++++++++++++++++----------- 1 file changed, 49 insertions(+), 31 deletions(-)
diff --git a/drivers/iio/accel/bmc150-accel-i2c.c b/drivers/iio/accel/bmc150-accel-i2c.c index 2afaae0294ee..e24ce28a4660 100644 --- a/drivers/iio/accel/bmc150-accel-i2c.c +++ b/drivers/iio/accel/bmc150-accel-i2c.c @@ -21,6 +21,51 @@
#include "bmc150-accel.h"
+#ifdef CONFIG_ACPI +static const struct acpi_device_id bmc150_acpi_dual_accel_ids[] = {
- {"BOSC0200"},
- { },
+};
+/*
- Some acpi_devices describe 2 accelerometers in a single ACPI device, try instantiating
80 char wrap still preferred when it doesn't otherwise hurt readability.
- a second i2c_client for an I2cSerialBusV2 ACPI resource with index 1.
- */
+static void bmc150_acpi_dual_accel_probe(struct i2c_client *client) +{
- struct acpi_device *adev = ACPI_COMPANION(&client->dev);
- struct i2c_client *second_dev;
- struct i2c_board_info board_info = {
.type = "bmc150_accel",/** The 2nd accel sits in the base of 2-in-1s. Note this* name is static, as there should never be more then 1* BOSC0200 ACPI node with 2 accelerometers in it.Given the lesser indent after pulling this out into a new function, it would be good to rewrap this text as nearer to 80 chars.
*/.dev_name = "BOSC0200:base",.fwnode = client->dev.fwnode,.irq = -ENOENT,- };
- if (acpi_match_device_ids(adev, bmc150_acpi_dual_accel_ids))
return;- second_dev = i2c_acpi_new_device(&client->dev, 1, &board_info);
- if (!IS_ERR(second_dev))
bmc150_set_second_device(client, second_dev);+}
+static void bmc150_acpi_dual_accel_remove(struct i2c_client *client) +{
- struct i2c_client *second_dev = bmc150_get_second_device(client);
- i2c_unregister_device(second_dev);
+} +#else +static void bmc150_acpi_dual_accel_probe(struct i2c_client *client) {} +static void bmc150_acpi_dual_accel_remove(struct i2c_client *client) {} +#endif
static int bmc150_accel_probe(struct i2c_client *client, const struct i2c_device_id *id) { @@ -30,7 +75,6 @@ static int bmc150_accel_probe(struct i2c_client *client, i2c_check_functionality(client->adapter, I2C_FUNC_I2C) || i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_READ_I2C_BLOCK);
struct acpi_device __maybe_unused *adev; int ret;
regmap = devm_regmap_init_i2c(client, &bmc150_regmap_conf);
@@ -46,42 +90,16 @@ static int bmc150_accel_probe(struct i2c_client *client, if (ret) return ret;
- /*
* Some BOSC0200 acpi_devices describe 2 accelerometers in a single ACPI* device, try instantiating a second i2c_client for an I2cSerialBusV2* ACPI resource with index 1. The !id check avoids recursion when* bmc150_accel_probe() gets called for the second client.*/-#ifdef CONFIG_ACPI
- adev = ACPI_COMPANION(&client->dev);
- if (!id && adev && strcmp(acpi_device_hid(adev), "BOSC0200") == 0) {
struct i2c_board_info board_info = {.type = "bmc150_accel",/** The 2nd accel sits in the base of 2-in-1s. Note this* name is static, as there should never be more then 1* BOSC0200 ACPI node with 2 accelerometers in it.*/.dev_name = "BOSC0200:base",.fwnode = client->dev.fwnode,.irq = -ENOENT,};struct i2c_client *second_dev;second_dev = i2c_acpi_new_device(&client->dev, 1, &board_info);if (!IS_ERR(second_dev))bmc150_set_second_device(client, second_dev);- }
-#endif
- /* The !id check avoids recursion when probe() gets called for the second client. */
Won't hurt readability to wrap this to 80 chars as a multiline comment.
if (!id && has_acpi_companion(&client->dev))
bmc150_acpi_dual_accel_probe(client);return 0;
}
static int bmc150_accel_remove(struct i2c_client *client) {
- struct i2c_client *second_dev = bmc150_get_second_device(client);
- i2c_unregister_device(second_dev);
bmc150_acpi_dual_accel_remove(client);
return bmc150_accel_core_remove(&client->dev);
}