lkml.org 
[lkml]   [2009]   [Mar]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 4/5] lis3: solve dependency between core and ACPI
Daniel Mack schreef:
> This solves the dependency between lis3lv02d.[ch] and ACPI specific
> methods. It introduces a ->bus_priv pointer to the device struct which
> is casted to 'struct acpi_device' in the ACIP layer. Changed hp_accel.c
> accordingly.
>
> This also moves the read_8() and read_16() routines from hp_accel.c to
> lis3lv02d.c as they are not specific to ACPI.
Hello,
I've tried the patch series on my laptop. This particular patch burst
the driver... need some work :-) The values are not read correctly and
the IRQ is not detected. See down in the code...


> @@ -291,23 +284,9 @@ static int lis3lv02d_add(struct acpi_device *device)
> strcpy(acpi_device_class(device), ACPI_MDPS_CLASS);
> device->driver_data = &lis3_dev;
>
> - lis3lv02d_acpi_read(device->handle, WHO_AM_I, &lis3_dev.whoami);
> - switch (lis3_dev.whoami) {
> - case LIS_DOUBLE_ID:
> - printk(KERN_INFO DRIVER_NAME ": 2-byte sensor found\n");
> - lis3_dev.read_data = lis3lv02d_read_16;
> - lis3_dev.mdps_max_val = 2048;
> - break;
> - case LIS_SINGLE_ID:
> - printk(KERN_INFO DRIVER_NAME ": 1-byte sensor found\n");
> - lis3_dev.read_data = lis3lv02d_read_8;
> - lis3_dev.mdps_max_val = 128;
> - break;
> - default:
> - printk(KERN_ERR DRIVER_NAME
> - ": unknown sensor type 0x%X\n", lis3_dev.whoami);
> - return -EINVAL;
> - }
> + ret = lis3lv02d_init_device(&lis3_dev);
> + if (ret)
> + return ret;
>
> /* If possible use a "standard" axes order */
> if (dmi_check_system(lis3lv02d_dmi_ids) == 0) {
> @@ -318,19 +297,16 @@ static int lis3lv02d_add(struct acpi_device *device)
>
> INIT_WORK(&hpled_led.work, delayed_set_status_worker);
> ret = led_classdev_register(NULL, &hpled_led.led_classdev);
> - if (ret)
> - return ret;
> -
> - /* obtain IRQ number of our device from ACPI */
> - lis3lv02d_enum_resources(lis3_dev.device);
> -
> - ret = lis3lv02d_init_device(&lis3_dev);
> if (ret) {
> + lis3lv02d_joystick_disable();
> + lis3lv02d_poweroff(&lis3_dev);
> flush_work(&hpled_led.work);
> - led_classdev_unregister(&hpled_led.led_classdev);
> return ret;
> }
>
> + /* obtain IRQ number of our device from ACPI */
> + lis3lv02d_enum_resources(device);
> +
> return ret;
> }
So now, we first try to set up the IRQ, in lis3lv02d_init_device() and
only after look for a IRQ number with lis3lv02d_enum_resources()... that
doesn't work.

>
> -static s16 lis3lv02d_read_16(acpi_handle handle, int reg)
> -{
> - u8 lo, hi;
> -
> - lis3_dev.read(handle, reg - 1, &lo);
> - lis3_dev.read(handle, reg, &hi);
> - /* In "12 bit right justified" mode, bit 6, bit 7, bit 8 = bit 5 */
> - return (s16)((hi << 8) | lo);
> -}
:
> +static s16 lis3lv02d_read_16(struct lis3lv02d *lis3, int reg)
> {
> u8 lo, hi;
>
> - lis3_dev.read(handle, reg, &lo);
> - lis3_dev.read(handle, reg + 1, &hi);
> + lis3->read(lis3, reg, &lo);
> + lis3->read(lis3, reg + 1, &hi);
> /* In "12 bit right justified" mode, bit 6, bit 7, bit 8 = bit 5 */
> return (s16)((hi << 8) | lo);
> }
Looking at it I cannot beleive we ended up with two versions of
lis3lv02d_read_16()!
Anyway, you want the logic of the above version, the one with "- 1".

Eric



\
 
 \ /
  Last update: 2009-03-03 20:57    [from the cache]
©2003-2014 Jasper Spaans. hosted at Digital Ocean