Re: [PATCH] i2c: move acpi code back into the core

From: Mika Westerberg
Date: Tue Sep 23 2014 - 05:16:58 EST


On Mon, Sep 22, 2014 at 11:06:46PM +0200, Wolfram Sang wrote:
> Commit 5d98e61d337c ("I2C/ACPI: Add i2c ACPI operation region support") renamed
> the i2c-core module. This may cause regressions for distributions, so put the
> ACPI code back into the core. No code was changed.

Thanks Wolfram for doing this!

I tested this already and it of course works but there are few things
that you are missing so this probably needs an additional iteration.

> Reported-by: Jean Delvare <jdelvare@xxxxxxx>
> Signed-off-by: Wolfram Sang <wsa@xxxxxxxxxxxxx>
> Cc: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx>
> Cc: Lan Tianyu <tianyu.lan@xxxxxxxxx>
> Cc: Jean Delvare <jdelvare@xxxxxxx>
> ---
>
> Lan Tianyi, this is a patch I expected to get from you, fully tested. Since we
> are approaching the final release soon, I did it now. Please do test this
> patch. All I can do is build test (it works for me, build robots are still
> running). If nobody tests this, I have to revert the ACPI changes of this
> cycle.
>
> Jean, can you have a look, too?
>
> We can think of something better for the next release, but for now, this should be it.
>
> MAINTAINERS | 1 -
> drivers/i2c/Makefile | 5 +-
> drivers/i2c/i2c-acpi.c | 364 -------------------------------------------------
> drivers/i2c/i2c-core.c | 345 ++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 346 insertions(+), 369 deletions(-)
> delete mode 100644 drivers/i2c/i2c-acpi.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 809ecd680d88..e3682d0dea1e 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4477,7 +4477,6 @@ M: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx>
> L: linux-i2c@xxxxxxxxxxxxxxx
> L: linux-acpi@xxxxxxxxxxxxxxx
> S: Maintained
> -F: drivers/i2c/i2c-acpi.c

You can remove my name from MAINTANERS as well. I'm not I2C subsystem
maintainer :) Of course I will still assist reviewing ACPI related
changes in i2c-core.c.

> I2C-TAOS-EVM DRIVER
> M: Jean Delvare <jdelvare@xxxxxxx>

...

> +/**
> + * acpi_i2c_register_devices - enumerate I2C slave devices behind adapter
> + * @adap: pointer to adapter
> + *
> + * Enumerate all I2C slave devices behind this adapter by walking the ACPI
> + * namespace. When a device is found it will be added to the Linux device
> + * model and bound to the corresponding ACPI handle.
> + */
> +void acpi_i2c_register_devices(struct i2c_adapter *adap)

static

> +{
> + acpi_handle handle;
> + acpi_status status;
> +
> + if (!adap->dev.parent)
> + return;
> +
> + handle = ACPI_HANDLE(adap->dev.parent);
> + if (!handle)
> + return;
> +
> + status = acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, 1,
> + acpi_i2c_add_device, NULL,
> + adap, NULL);
> + if (ACPI_FAILURE(status))
> + dev_warn(&adap->dev, "failed to enumerate I2C slaves\n");
> +}
> +

...

> +int acpi_i2c_install_space_handler(struct i2c_adapter *adapter)

static

> +{
> + acpi_handle handle = ACPI_HANDLE(adapter->dev.parent);
> + struct acpi_i2c_handler_data *data;
> + acpi_status status;
> +
> + if (!handle)
> + return -ENODEV;
> +
> + data = kzalloc(sizeof(struct acpi_i2c_handler_data),
> + GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + data->adapter = adapter;
> + status = acpi_bus_attach_private_data(handle, (void *)data);
> + if (ACPI_FAILURE(status)) {
> + kfree(data);
> + return -ENOMEM;
> + }
> +
> + status = acpi_install_address_space_handler(handle,
> + ACPI_ADR_SPACE_GSBUS,
> + &acpi_i2c_space_handler,
> + NULL,
> + data);
> + if (ACPI_FAILURE(status)) {
> + dev_err(&adapter->dev, "Error installing i2c space handler\n");
> + acpi_bus_detach_private_data(handle);
> + kfree(data);
> + return -ENOMEM;
> + }
> +
> + return 0;
> +}
> +
> +void acpi_i2c_remove_space_handler(struct i2c_adapter *adapter)

static

> +{
> + acpi_handle handle = ACPI_HANDLE(adapter->dev.parent);
> + struct acpi_i2c_handler_data *data;
> + acpi_status status;
> +
> + if (!handle)
> + return;
> +
> + acpi_remove_address_space_handler(handle,
> + ACPI_ADR_SPACE_GSBUS,
> + &acpi_i2c_space_handler);
> +
> + status = acpi_bus_get_private_data(handle, (void **)&data);
> + if (ACPI_SUCCESS(status))
> + kfree(data);
> +
> + acpi_bus_detach_private_data(handle);
> +}
> +#endif

I would put there

#endif /* CONFIG_ACPI_I2C_OPREGION */

> +#endif /* CONFIG_ACPI */

Also please remove all ACPI related stuff in include/linux/i2c.h. That
is not needed anymore.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/