RFC [PATCH] acpi: allow SMBus access

RFC [PATCH] acpi: allow SMBus access

[Johan Vromans]

From: Johan Vromans <jvromans@squirrel.nl>

To get battery readings on some laptops it is necessary to interface
with the SMBus that hangs of the EC. However, the current
implementation of the EC driver does not permit other modules
read/write access.

A trivial solution is to change acpi_ec_read/write from static to
nonstatic, and export the symbols so other modules can use them.

This patch is based on the current 2.6.17 kernel sources.

Signed-off-by: Johan Vromans <jvromans@squirrel.nl>
---

--- linux-2.6.17.i686/drivers/acpi/ec.c.orig	2006-07-02 22:46:35.000000000 +0200
+++ linux-2.6.17.i686/drivers/acpi/ec.c	2006-06-27 10:03:19.000000000 +0200
@@ -305,20 +305,22 @@ end:
 }
 #endif /* ACPI_FUTURE_USAGE */
 
-static int acpi_ec_read(union acpi_ec *ec, u8 address, u32 * data)
+int acpi_ec_read(union acpi_ec *ec, u8 address, u32 * data)
 {
 	if (acpi_ec_poll_mode)
 		return acpi_ec_poll_read(ec, address, data);
 	else
 		return acpi_ec_intr_read(ec, address, data);
 }
-static int acpi_ec_write(union acpi_ec *ec, u8 address, u8 data)
+EXPORT_SYMBOL(acpi_ec_read);
+int acpi_ec_write(union acpi_ec *ec, u8 address, u8 data)
 {
 	if (acpi_ec_poll_mode)
 		return acpi_ec_poll_write(ec, address, data);
 	else
 		return acpi_ec_intr_write(ec, address, data);
 }
+EXPORT_SYMBOL(acpi_ec_write);
 static int acpi_ec_poll_read(union acpi_ec *ec, u8 address, u32 * data)
 {
 	acpi_status status = AE_OK;

-- 
Johan

Re: RFC [PATCH] acpi: allow SMBus access

[Yu Luming]

On Monday 03 July 2006 05:12, Johan Vromans wrote:
> From: Johan Vromans <jvromans@squirrel.nl>
>
> To get battery readings on some laptops it is necessary to interface
> with the SMBus that hangs of the EC. However, the current
> implementation of the EC driver does not permit other modules
> read/write access.

why NOT use ec_read/ec_write?

-- 
Thanks,
Luming

Re: RFC [PATCH] acpi: allow SMBus access

[Bruno Ducrot]

On Mon, Jul 03, 2006 at 09:51:29AM +0800, Yu Luming wrote:
> On Monday 03 July 2006 05:12, Johan Vromans wrote:
> > From: Johan Vromans <jvromans@squirrel.nl>
> >
> > To get battery readings on some laptops it is necessary to interface
> > with the SMBus that hangs of the EC. However, the current
> > implementation of the EC driver does not permit other modules
> > read/write access.
> 
> why NOT use ec_read/ec_write?

Because ec_read/ec_write only handle one EC, maybe ?

-- 
Bruno Ducrot

--  Which is worse:  ignorance or apathy?
--  Don't know.  Don't care.

Re: RFC [PATCH] acpi: allow SMBus access

[Bruno Ducrot]

On Sun, Jul 02, 2006 at 11:12:21PM +0200, Johan Vromans wrote:
> From: Johan Vromans <jvromans@squirrel.nl>
> 
> To get battery readings on some laptops it is necessary to interface
> with the SMBus that hangs of the EC. However, the current
> implementation of the EC driver does not permit other modules
> read/write access.
> 
> A trivial solution is to change acpi_ec_read/write from static to
> nonstatic, and export the symbols so other modules can use them.
> 
> This patch is based on the current 2.6.17 kernel sources.

I don't think this patch is correct, or else I would have already
asked this patch being added to mainline.

-- 
Bruno Ducrot

--  Which is worse:  ignorance or apathy?
--  Don't know.  Don't care.

Re: RFC [PATCH] acpi: allow SMBus access

[Johan Vromans]

Bruno Ducrot <ducrot@poupinou.org> writes:

> I don't think this patch is correct, or else I would have already
> asked this patch being added to mainline.

You've been standing at the origins of where this patch came from. Can
you provide a better alternative?

-- Johan

Re: RFC [PATCH] acpi: allow SMBus access

[Bruno Ducrot]

On Mon, Jul 03, 2006 at 01:14:22PM +0200, Johan Vromans wrote:
> Bruno Ducrot <ducrot@poupinou.org> writes:
> 
> > I don't think this patch is correct, or else I would have already
> > asked this patch being added to mainline.
> 
> You've been standing at the origins of where this patch came from. Can
> you provide a better alternative?
> 

If you want this patch to be applied, I think you should at least
mark ec_read/ec_write being obsolete and maybe provide a
solution for drivers who use those functions, as for example
sonypi.  Having two very same kind of access for the EC is
not good IMHO.

In fact why I didn't submitted this patch myself is because
I wanted to provide a real bus access via the EC driver, including
the interrupt driven ones.

Something like that :

int acpi_ec_register(struct acpi_ec_driver *child);
int acpi_ec_unregister(struct acpi_ec_driver *child);

The struct acpi_ec_driver should be something like that:

struct acpi_ec_driver {
	acpi_handle	ec_handle;
	acpi_handle	handle;
	unsigned long	uid;
	unsigned long	query;
	int (*acpi_ec_query_handler) (???);
	...
	/* maybe a private space somewhere */
	void *private;
}


reading/writing may pass perhaps via an exported acpi_ec_(read|write)()
functions, but the real key would have to be able to register a function
in order to trigger the acpi_ec_query_handler function member when the EC
receive the interrupt for the query number ->query instead of the _Qxx
method provided by the OEM.  Please look at ACPI 3.0 specification,
more precisely "5.6.2.2.2 Dispatching to an ACPI-aware Device Driver",
and the whole chapter 12 (ACPI Embedded Controller Interface Specification).

We may have then an access to the ACPI EC HC SMbus with a interrupt
driven driver, which imho is the correct approach: we will be sure
a _Qxx method provided by the bios writer will interferre with our
SMbus driver.

Unfortunately I don't have anymore the time to provide this support
for Linux.

-- 
Bruno Ducrot

--  Which is worse:  ignorance or apathy?
--  Don't know.  Don't care.

Re: RFC [PATCH] acpi: allow SMBus access

[Johan Vromans]

Bruno Ducrot <ducrot@poupinou.org> writes:

> I wanted to provide a real bus access via the EC driver, including
> the interrupt driven ones.

Yes, that would be the right way to go. But it is a longer term
solution and AFAIK noone is currently working on it.

In the mean time, several users can benefit from an intermediate
solution like the one I suggested. Besides, it is not a wrong solution
per se, it is equally wrong as the ac_read/write routines that are
exported.

So I still think that exporting the current acpi_ec_read/write would
be a good thing to so, but I agree it should be marked as an
intermediate solution.

-- Johan

Re: RFC [PATCH] acpi: allow SMBus access

[Bruno Ducrot]

On Tue, Jul 04, 2006 at 10:09:06AM +0200, Johan Vromans wrote:
> Bruno Ducrot <ducrot@poupinou.org> writes:
> 
> > I wanted to provide a real bus access via the EC driver, including
> > the interrupt driven ones.
> 
> Yes, that would be the right way to go. But it is a longer term
> solution and AFAIK noone is currently working on it.

Indeed

> In the mean time, several users can benefit from an intermediate
> solution like the one I suggested. Besides, it is not a wrong solution
> per se, it is equally wrong as the ac_read/write routines that are
> exported.

Yes, but I always considered the ec_read/write functions being an hack
in order to support sonypi at first.  If it's possible to kill them
and to replace them with the acpi_ec_read|write, well this would be
good IMHO.

> So I still think that exporting the current acpi_ec_read/write would
> be a good thing to so, but I agree it should be marked as an
> intermediate solution.

An intermediate solution would be to use the already existing
ec_read|write instead of the one you want to use.  The original
SMBus driver used acpi_ec_read because the author wanted to be
sure that driver will support laptops with more than one EC, but
he never saw such laptops so far.

Cheers,

-- 
Bruno Ducrot

--  Which is worse:  ignorance or apathy?
--  Don't know.  Don't care.

Re: RFC [PATCH] acpi: allow SMBus access

[Johan Vromans]

Bruno Ducrot <ducrot@poupinou.org> writes:

> An intermediate solution would be to use the already existing
> ec_read|write instead of the one you want to use.  The original
> SMBus driver used acpi_ec_read because the author wanted to be
> sure that driver will support laptops with more than one EC, but
> he never saw such laptops so far.

Indeed, if the requirement for multiple ECs can be dropped, this
simplifies the problem sufficiently to use the current ec_read/write
functions.

Unless someone comes up with other ideas I suggest to withdraw this
proposal until laptops with multiple ECs hit the market...

-- Johan

Re: RFC [PATCH] acpi: allow SMBus access

[Johan Vromans]

Johan Vromans <jvromans@squirrel.nl> writes:

Progress!

http://lkml.org/lkml/2006/7/1/76

-- Johan