{"thread":{"id":"17709","subject":"[PATCHv2] lis3lv02d: support both one- and two-byte sensors","startedAt":"2009-02-10T23:40:49Z","lastAt":"2009-02-10T23:54:53Z","messageCount":4,"participants":["Giuseppe Bilotta","Éric Piel"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"104115","messageId":"1234309249-13672-1-git-send-email-giuseppe.bilotta@gmail.com","threadId":"17709","inReplyTo":null,"subject":"[PATCHv2] lis3lv02d: support both one- and two-byte sensors","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2009-02-10T23:40:49Z","receivedAt":"2009-02-10T23:40:49Z","isPatch":false,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"Sensors responding with 0x3B to WHO_AM_I only have one data register per\ndirection, thus returning a signed byte from the position which is\noccupied by the MSB in sensors responding with 0x3A.\n\nWe support both kind of sensors by checking for the sensor type on init\nand defining appropriate data-access routines and sensor limits (for the\njoystick) depending on what we find.\n\nSigned-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>\n---\n\nThis is the other version of the patch, and it changes access to use the\nbase address from the 8-bit, so the 16-bit routine is changed to access\nthe PREVIOUS byte for the LSB.\n\nChoose whichever of the patches is deemed more correct 8-)\n\nNote that both patches are based off Linus tree, so I don't know how\nthey cope with Pavel's \"don't touch too much on init\" one. I'll try to\nwork out a patch on top of that too.\n\n drivers/hwmon/lis3lv02d.c |   42 ++++++++++++++++++++++++++++++------------\n drivers/hwmon/lis3lv02d.h |   12 +++++++++++-\n 2 files changed, 41 insertions(+), 13 deletions(-)\n\ndiff --git a/drivers/hwmon/lis3lv02d.c b/drivers/hwmon/lis3lv02d.c\nindex 219d2d0..b6b2d4a 100644\n--- a/drivers/hwmon/lis3lv02d.c\n+++ b/drivers/hwmon/lis3lv02d.c\n@@ -52,9 +52,6 @@\n  * joystick.\n  */\n \n-/* Maximum value our axis may get for the input device (signed 12 bits) */\n-#define MDPS_MAX_VAL 2048\n-\n struct acpi_lis3lv02d adev;\n EXPORT_SYMBOL_GPL(adev);\n \n@@ -64,12 +61,19 @@ static s16 lis3lv02d_read_16(acpi_handle handle, int reg)\n {\n \tu8 lo, hi;\n \n-\tadev.read(handle, reg, &lo);\n-\tadev.read(handle, reg + 1, &hi);\n+\tadev.read(handle, reg - 1, &lo);\n+\tadev.read(handle, reg, &hi);\n \t/* In \"12 bit right justified\" mode, bit 6, bit 7, bit 8 = bit 5 */\n \treturn (s16)((hi << 8) | lo);\n }\n \n+static s16 lis3lv02d_read_8(acpi_handle handle, int reg)\n+{\n+\tu8 lo;\n+\tadev.read(handle, reg, &lo);\n+\treturn *((s8*)(&lo));\n+}\n+\n /**\n  * lis3lv02d_get_axis - For the given axis, give the value converted\n  * @axis:      1,2,3 - can also be negative\n@@ -98,9 +102,9 @@ static void lis3lv02d_get_xyz(acpi_handle handle, int *x, int *y, int *z)\n {\n \tint position[3];\n \n-\tposition[0] = lis3lv02d_read_16(handle, OUTX_L);\n-\tposition[1] = lis3lv02d_read_16(handle, OUTY_L);\n-\tposition[2] = lis3lv02d_read_16(handle, OUTZ_L);\n+\tposition[0] = adev.lis3lv02d_read(handle, OUTX);\n+\tposition[1] = adev.lis3lv02d_read(handle, OUTY);\n+\tposition[2] = adev.lis3lv02d_read(handle, OUTZ);\n \n \t*x = lis3lv02d_get_axis(adev.ac.x, position);\n \t*y = lis3lv02d_get_axis(adev.ac.y, position);\n@@ -121,7 +125,21 @@ void lis3lv02d_poweron(acpi_handle handle)\n \n \tadev.is_on = 1;\n \tadev.init(handle);\n-\tadev.write(handle, FF_WU_CFG, 0);\n+\tadev.read(handle, WHO_AM_I, &adev.whoami);\n+\t/* Tell apart LISxLV02Dy from LISx02Dy family by checking the LSB\n+\t * (0x3A vs 0x3B). TODO More sophisticated checks on other registers\n+\t * could be implemented, for example to see if we have 2 or 3 axes, and\n+\t * configure the joystick accordingly.\n+\t */\n+\tif (adev.whoami & 1) {\n+\t\tprintk(KERN_INFO DRIVER_NAME \": 1-byte sensor found\\n\");\n+\t\tadev.lis3lv02d_read = lis3lv02d_read_8;\n+\t\tadev.mdps_max_val = 128;\n+\t} else {\n+\t\tprintk(KERN_INFO DRIVER_NAME \": 2-byte sensor found\\n\");\n+\t\tadev.lis3lv02d_read = lis3lv02d_read_16;\n+\t\tadev.mdps_max_val = 2048;\n+\t}\n \t/*\n \t * BDU: LSB and MSB values are not updated until both have been read.\n \t *      So the value read will always be correct.\n@@ -231,9 +249,9 @@ int lis3lv02d_joystick_enable(void)\n \tadev.idev->close      = lis3lv02d_joystick_close;\n \n \tset_bit(EV_ABS, adev.idev->evbit);\n-\tinput_set_abs_params(adev.idev, ABS_X, -MDPS_MAX_VAL, MDPS_MAX_VAL, 3, 3);\n-\tinput_set_abs_params(adev.idev, ABS_Y, -MDPS_MAX_VAL, MDPS_MAX_VAL, 3, 3);\n-\tinput_set_abs_params(adev.idev, ABS_Z, -MDPS_MAX_VAL, MDPS_MAX_VAL, 3, 3);\n+\tinput_set_abs_params(adev.idev, ABS_X, -adev.mdps_max_val, adev.mdps_max_val, 3, 3);\n+\tinput_set_abs_params(adev.idev, ABS_Y, -adev.mdps_max_val, adev.mdps_max_val, 3, 3);\n+\tinput_set_abs_params(adev.idev, ABS_Z, -adev.mdps_max_val, adev.mdps_max_val, 3, 3);\n \n \terr = input_register_device(adev.idev);\n \tif (err) {\ndiff --git a/drivers/hwmon/lis3lv02d.h b/drivers/hwmon/lis3lv02d.h\nindex 223f1c0..9d7d4a8 100644\n--- a/drivers/hwmon/lis3lv02d.h\n+++ b/drivers/hwmon/lis3lv02d.h\n@@ -22,11 +22,14 @@\n /*\n  * The actual chip is STMicroelectronics LIS3LV02DL or LIS3LV02DQ that seems to\n  * be connected via SPI. There exists also several similar chips (such as LIS302DL or\n- * LIS3L02DQ) but not in the HP laptops and they have slightly different registers.\n+ * LIS3L02DQ) and they have slightly different registers, but we can provide a\n+ * common interface for all of them.\n  * They can also be connected via I²C.\n  */\n \n+/* 2-byte registers */\n #define LIS3LV02DL_ID\t0x3A /* Also the LIS3LV02DQ */\n+/* 1-byte registers */\n #define LIS302DL_ID\t0x3B /* Also the LIS202DL! */\n \n enum lis3lv02d_reg {\n@@ -44,10 +47,13 @@ enum lis3lv02d_reg {\n \tSTATUS_REG\t= 0x27,\n \tOUTX_L\t\t= 0x28,\n \tOUTX_H\t\t= 0x29,\n+\tOUTX\t\t= 0x29,\n \tOUTY_L\t\t= 0x2A,\n \tOUTY_H\t\t= 0x2B,\n+\tOUTY\t\t= 0x2B,\n \tOUTZ_L\t\t= 0x2C,\n \tOUTZ_H\t\t= 0x2D,\n+\tOUTZ\t\t= 0x2D,\n \tFF_WU_CFG\t= 0x30,\n \tFF_WU_SRC\t= 0x31,\n \tFF_WU_ACK\t= 0x32,\n@@ -159,6 +165,10 @@ struct acpi_lis3lv02d {\n \tacpi_status (*write) (acpi_handle handle, int reg, u8 val);\n \tacpi_status (*read) (acpi_handle handle, int reg, u8 *ret);\n \n+\tu8\t\t\twhoami;    /* 3Ah: 2-byte registries, 3Bh: 1-byte registries */\n+\ts16 (*lis3lv02d_read) (acpi_handle handle, int reg);\n+\tint\t\t\tmdps_max_val;\n+\n \tstruct input_dev\t*idev;     /* input device */\n \tstruct task_struct\t*kthread;  /* kthread for input */\n \tstruct mutex            lock;\n-- \n1.6.2.rc0.173.g5e148\n"},{"id":"104117","messageId":"499211D0.3090705@tremplin-utc.net","threadId":"17709","inReplyTo":"1234309249-13672-1-git-send-email-giuseppe.bilotta@gmail.com","subject":"Re: [PATCHv2] lis3lv02d: support both one- and two-byte sensors","fromName":"Éric Piel","fromEmail":"eric.piel@tremplin-utc.net","sentAt":"2009-02-10T23:46:24Z","receivedAt":"2009-02-10T23:46:24Z","isPatch":false,"sender":{"key":"eric.piel@tremplin-utc.net","avatar":null},"body":"Giuseppe Bilotta schreef:\n> Sensors responding with 0x3B to WHO_AM_I only have one data register per\n> direction, thus returning a signed byte from the position which is\n> occupied by the MSB in sensors responding with 0x3A.\n> \n> We support both kind of sensors by checking for the sensor type on init\n> and defining appropriate data-access routines and sensor limits (for the\n> joystick) depending on what we find.\n> \n> Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>\n> ---\n> \n> This is the other version of the patch, and it changes access to use the\n> base address from the 8-bit, so the 16-bit routine is changed to access\n> the PREVIOUS byte for the LSB.\n> \n> Choose whichever of the patches is deemed more correct 8-)\n> \n> Note that both patches are based off Linus tree, so I don't know how\n> they cope with Pavel's \"don't touch too much on init\" one. I'll try to\n> work out a patch on top of that too.\nYes, great, and this one looks even better than v1 :-)\nHowever I'm not sure about the conversion between s8 and s16:\n\n> +static s16 lis3lv02d_read_8(acpi_handle handle, int reg)\n> +{\n> +\tu8 lo;\n> +\tadev.read(handle, reg, &lo);\n> +\treturn *((s8*)(&lo));\n> +}\nDoes it really extend the sign to 16 bits? I would have written it this way:\n+static s16 lis3lv02d_read_8(acpi_handle handle, int reg)\n+{\n+\ts8 lo;\n+\tadev.read(handle, reg, &lo);\n+\treturn (s16)lo;\n+}\nDoesn't it work better?\n\nEric\n"},{"id":"104118","messageId":"cb7bb73a0902101551o30dde0aesaeba399f1ab732af@mail.gmail.com","threadId":"17709","inReplyTo":"499211D0.3090705@tremplin-utc.net","subject":"Re: [PATCHv2] lis3lv02d: support both one- and two-byte sensors","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2009-02-10T23:51:25Z","receivedAt":"2009-02-10T23:51:25Z","isPatch":false,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"On Wed, Feb 11, 2009 at 12:46 AM, Éric Piel <Eric.Piel@tremplin-utc.net> wrote:\n> Giuseppe Bilotta schreef:\n>>\n>> Sensors responding with 0x3B to WHO_AM_I only have one data register per\n>> direction, thus returning a signed byte from the position which is\n>> occupied by the MSB in sensors responding with 0x3A.\n>>\n>> We support both kind of sensors by checking for the sensor type on init\n>> and defining appropriate data-access routines and sensor limits (for the\n>> joystick) depending on what we find.\n>>\n>> Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>\n>> ---\n>>\n>> This is the other version of the patch, and it changes access to use the\n>> base address from the 8-bit, so the 16-bit routine is changed to access\n>> the PREVIOUS byte for the LSB.\n>>\n>> Choose whichever of the patches is deemed more correct 8-)\n>>\n>> Note that both patches are based off Linus tree, so I don't know how\n>> they cope with Pavel's \"don't touch too much on init\" one. I'll try to\n>> work out a patch on top of that too.\n>\n> Yes, great, and this one looks even better than v1 :-)\n\nAnd don't forget about the v3 for -mm ;-)\n\n> However I'm not sure about the conversion between s8 and s16:\n>\n>> +static s16 lis3lv02d_read_8(acpi_handle handle, int reg)\n>> +{\n>> +       u8 lo;\n>> +       adev.read(handle, reg, &lo);\n>> +       return *((s8*)(&lo));\n>> +}\n>\n> Does it really extend the sign to 16 bits? I would have written it this way:\n> +static s16 lis3lv02d_read_8(acpi_handle handle, int reg)\n> +{\n> +       s8 lo;\n> +       adev.read(handle, reg, &lo);\n> +       return (s16)lo;\n> +}\n> Doesn't it work better?\n\nWell, the previous one works, but I admit it's horribly convoluted.\nI'll double check with this one and resubmit (I'll only resubmit the\none based on Pavel's \"don't touch anything on init\" probably though,\nhope it's ok)\n\n\n-- \nGiuseppe \"Oblomov\" Bilotta\n"},{"id":"104121","messageId":"499213CD.6080903@tremplin-utc.net","threadId":"17709","inReplyTo":"cb7bb73a0902101551o30dde0aesaeba399f1ab732af@mail.gmail.com","subject":"Re: [PATCHv2] lis3lv02d: support both one- and two-byte sensors","fromName":"Éric Piel","fromEmail":"eric.piel@tremplin-utc.net","sentAt":"2009-02-10T23:54:53Z","receivedAt":"2009-02-10T23:54:53Z","isPatch":false,"sender":{"key":"eric.piel@tremplin-utc.net","avatar":null},"body":"Giuseppe Bilotta schreef:\n\n> Well, the previous one works, but I admit it's horribly convoluted.\n> I'll double check with this one and resubmit (I'll only resubmit the\n> one based on Pavel's \"don't touch anything on init\" probably though,\n> hope it's ok)\nSure, only modified v3 is ok.\nBTW, now that this is working. Could you double check the correct axes \nset up. In particular, is it x_inverted or y_inverted, and what about z \naxis...\n\nThanks,\nEric\n"}]}