RFC [PATCH] acpi: allow SMBus access

From: Johan Vromans <[email protected]>

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.

---

@@ -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

why NOT use ec_read/ec_write?

Thanks,
Luming

Because ec_read/ec_write only handle one EC, maybe ?

Bruno Ducrot

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

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.

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

-- Johan

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.

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

Indeed

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.

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.

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

Johan Vromans <[email protected]> writes:

Progress!

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

-- Johan