Re: [RFC PATCH 02/06] input/rmi4: Core files

From: Benjamin Tissoires
Date: Tue Nov 27 2012 - 04:33:39 EST


On Mon, Nov 26, 2012 at 11:54 PM, Christopher Heiny
<cheiny@xxxxxxxxxxxxx> wrote:
> On 11/26/2012 10:41 AM, Benjamin Tissoires wrote:
>>
>> Hi Christopher,
>>
>> On Sat, Nov 17, 2012 at 4:58 AM, Christopher Heiny <cheiny@xxxxxxxxxxxxx>
>> wrote:
>>>
>>> rmi_bus.c implements the basic functionality of the RMI bus. This file
>>> is
>>> greatly simplified compared to the previous patch - we've switched from
>>> "do it yourself" device/driver binding to using device_type to
>>> distinguish
>>> between the two kinds of devices on the bus (sensor devices and function
>>> specific devices) and using the standard bus implementation to manage
>>> devices
>>> and drivers.
>>>
>>>
>>> rmi_driver.c is a driver for the general functionality of the RMI sensor
>>> as a
>>> whole, managing those behaviors (including IRQ handling) that are not
>>> specific
>>> to any RMI4 function. It has some unavoidable dependencies on F01
>>> behavior,
>>> though we have worked to minimize those as far as possible.
>>>
>>>
>>> The header file rmi_driver.h provides definitions that are shared among
>>> the modules of the RMI implementation, but not thought to be necessary
>>> outside it.
>>>
>>>
>>> Greg KH - Linus Walleij recommended that we seek your input on these core
>>> files, particularly the bus implementation.
>>>
>>>
>>> Signed-off-by: Christopher Heiny <cheiny@xxxxxxxxxxxxx>
>>>
>>> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
>>> Cc: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
>>> Cc: Linus Walleij <linus.walleij@xxxxxxxxxxxxxx>
>>> Cc: Naveen Kumar Gaddipati <naveen.gaddipati@xxxxxxxxxxxxxx>
>>> Cc: Joeri de Gram <j.de.gram@xxxxxxxxx>
>>>
>>> ---
>>>
>>> drivers/input/rmi4/rmi_bus.c | 248 ++++++
>>> drivers/input/rmi4/rmi_driver.c | 1663
>>> +++++++++++++++++++++++++++++++++++++++
>>> drivers/input/rmi4/rmi_driver.h | 139 ++++
>>> include/uapi/linux/input.h | 1 +
>>> 4 files changed, 2051 insertions(+), 0 deletions(-)
>>>
>> [snipped]
>>>
>>> diff --git a/drivers/input/rmi4/rmi_driver.c
>>> b/drivers/input/rmi4/rmi_driver.c
>>> new file mode 100644
>>> index 0000000..05a73ae
>>> --- /dev/null
>>> +++ b/drivers/input/rmi4/rmi_driver.c
>>
>> [snipped]
>>>
>>> +/* extract product ID */
>>> +void get_prod_id(struct rmi_device *rmi_dev, struct rmi_driver_data
>>> *drvdata)
>>> +{
>>> + struct device *dev = &rmi_dev->dev;
>>> + int retval;
>>> + int board = 0, rev = 0;
>>> + int i;
>>> + static const char * const pattern[] = {
>>> + "tm%4d-%d", "s%4d-%d", "s%4d-ver%1d"};
>>> + u8 product_id[RMI_PRODUCT_ID_LENGTH+1];
>>> +
>>> + retval = rmi_read_block(rmi_dev,
>>> + drvdata->f01_container->fd.query_base_addr+
>>> + sizeof(struct f01_basic_queries),
>>> + product_id, RMI_PRODUCT_ID_LENGTH);
>>> + if (retval < 0) {
>>> + dev_err(dev, "Failed to read product id, code=%d!",
>>> retval);
>>> + return;
>>> + }
>>> + product_id[RMI_PRODUCT_ID_LENGTH] = '\0';
>>> +
>>> + for (i = 0; i < sizeof(product_id); i++)
>>> + product_id[i] = tolower(product_id[i]);
>>> +
>>> + for (i = 0; i < sizeof(pattern); i++) {
>>
>>
>> This should be ARRAY_SIZE(pattern).
>> It gave me a wonderful kernel oops :)
>
>
> Yep, that's a bug! Oddly enough, it runs without barfing on my systems
> (though who knows what horrible things are happening under the hood). Thanks
> for letting us know.

That's because the product id advertised by the touchpad you sent to
me is 'DS4 R3.0'. So it's not compliant with the patterns that are
advertised: "tm%4d-%d", "s%4d-%d", "s%4d-ver%1d".

If you present one of these patterns, thanks to the break, you leave
the for loop. But as I'm not in this optimal case, I'm having i > 2,
which introduced segfault.

Cheers,
Benjamin

>
> Chris
>
>>
>> Cheers,
>> Benjamin
>
>
> [snip]
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/