linux-kernel - Re: [PATCH 3/3] i2c / ACPI: add ACPI enumeration support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20121104095031.5e6465ae@endymion.delvare>
Date:	Sun, 4 Nov 2012 09:50:31 +0100
From:	Jean Delvare <khali@...ux-fr.org>
To:	Mika Westerberg <mika.westerberg@...ux.intel.com>
Cc:	linux-kernel@...r.kernel.org, lenb@...nel.org,
	rafael.j.wysocki@...el.com, broonie@...nsource.wolfsonmicro.com,
	grant.likely@...retlab.ca, linus.walleij@...aro.org,
	ben-linux@...ff.org, w.sang@...gutronix.de,
	mathias.nyman@...ux.intel.com, linux-acpi@...r.kernel.org
Subject: Re: [PATCH 3/3] i2c / ACPI: add ACPI enumeration support

On Sun, 4 Nov 2012 09:23:17 +0200, Mika Westerberg wrote:
> On Sat, Nov 03, 2012 at 10:52:46PM +0100, Jean Delvare wrote:
> > On Sat,  3 Nov 2012 09:46:33 +0200, Mika Westerberg wrote:
> > > --- /dev/null
> > > +++ b/drivers/acpi/acpi_i2c.c
> > > @@ -0,0 +1,234 @@
> > > +/*
> > > + * ACPI I2C enumeration support
> > > + *
> > > + * Copyright (C) 2012, Intel Corporation
> > > + * Author: Mika Westerberg <mika.westerberg@...ux.intel.com>
> > > + *
> > > + * This program is free software; you can redistribute it and/or modify
> > > + * it under the terms of the GNU General Public License version 2 as
> > > + * published by the Free Software Foundation.
> > > + */
> > > +
> > > +#include <linux/acpi.h>
> > > +#include <linux/i2c.h>
> > > +#include <linux/module.h>
> > 
> > You also need <linux/device.h> for dev_err() etc., and <linux/err.h> for
> > ENODEV etc.
> 
> I think <acpi.h> already includes <device.h> but I'll double check. At
> least this compiles without those headers in place :)

That's not really the point. You never know which header inclusions
will be removed from other header files in the future, so you should
include what you need explicitly. This avoids future build breakages as
well as build breakages on other architectures.

> > > (...)
> > > +static acpi_status acpi_i2c_add_device(struct acpi_device *adev, void *data)
> > > +{
> > > +	struct acpi_i2c_device_info info;
> > > +	struct i2c_adapter *adapter = data;
> > > +	acpi_status status;
> > > +
> > > +	memset(&info, 0, sizeof(info));
> > > +	info.gsi = -1;
> > > +
> > > +	status = acpi_walk_resources(adev->handle, METHOD_NAME__CRS,
> > > +				     acpi_i2c_add_resources, &info);
> > > +	if (ACPI_FAILURE(status) || !info.valid)
> > > +		return status;
> > > +
> > > +	strlcpy(info.board.type, acpi_device_hid(adev),
> > > +		sizeof(info.board.type));
> > 
> > I very much doubt the ACPI HID names will match the Linux i2c device
> > names. In other words you are instantiating devices no driver will want
> > to bind to. How do you plan to solve this issue?
> 
> We use ACPI IDs (_HID, _CID) for matching in a similar way than the Device
> Tree does. Typicaly you add following code to the existing I2C driver:
> 
> #ifdef CONFIG_ACPI
> static struct acpi_device_id mydrv_acpi_match[] = {
> 	{ "CHRGR00", 0 },
> 	...
> 	{ }
> };
> MODULE_DEVICE_TABLE(acpi, mydrv_acpi_match);
> #endif
> 
> static struct i2c_driver mydrv = {
> 	.driver = {
> 		.acpi_match_table = ACPI_PTR(mydrv_acpi_match),
> 	},
> 	...
> };
> 
> in order to get the driver matched to the device. If the driver needs to
> perform some ACPI specific things, like call _DSM method - it gets the ACPI
> handle from dev->acpi_handle (analoguous to Device Tree dev->of_node).
> 
> The same thing applies to platform and SPI busses as well.

OK, very nice, that should work just fine :)

Thanks,
-- 
Jean Delvare
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/