lkml.org 
[lkml]   [2012]   [Sep]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC PATCH 5/17] input: rmidev character driver for RMI4 sensors
On 08/27/2012 11:49 AM, Linus Walleij wrote:
> On Fri, Aug 17, 2012 at 3:17 PM, Christopher Heiny <cheiny@synaptics.com> wrote:
>
>> Driver for Synaptics touchscreens using RMI4 protocol.
>
> Really? This looks more like some custom char driver to get a pipe
> into the device from userspace. Put in a proper description of what this
> is for.
>
> If the purpose is to read/write arbitrary addresses in the device,
> you should use regmap's debugfs interface for this instead,
> it is much better suited for the task.

We'll look into using regmap not only for that, but possibly for general
register access. It has the potential to greatly simply both the
arbitrary register access, as well as the driver itself.

>
> (...)
>> +#define RMI_CHAR_DEV_TMPBUF_SZ 128
>> +#define RMI_REG_ADDR_PAGE_SELECT 0xFF
>> +#define REG_ADDR_LIMIT 0xFFFF
>> +
>> +struct rmidev_data {
>> + /* mutex for file operation*/
>> + struct mutex file_mutex;
>> + /* main char dev structure */
>> + struct cdev main_dev;
>> +
>> + /* pointer to the corresponding RMI4 device. We use this to do */
>> + /* read, write, etc. */
>> + struct rmi_device *rmi_dev;
>> + /* reference count */
>> + int ref_count;
>
> Something tells me you should atleast use <linux/kref.h> for this.
> It also solves a few atomicity problems in a good standard way.
> See Documentation/kref.txt

Roger.


>> +/*store dynamically allocated major number of char device*/
>> +static int rmidev_major_num;
>
> You need to patch your desired major number into
> Documentation/devices.txt'

We were going by the recommendation in Linux Device Drivers (3rd
edition) to use dynamic major number allocation via alloc_chrdev_region.
In particular in section 3.2.3 it says "new numbers are not being
assigned". I guess at this point we need to know whether the info in
LDD3 is authoritative or not. We can always add a number to
Documentation/devices.txt if that's the right thing to do, but I'd like
to make sure our next submission isn't bounced because we did that,
turning the process into Patch Ping-Pong :-).

In any case, it's likely that switching to the regmap interface would
make this question irrelevant.


>> +static struct class *rmidev_device_class;
>
> Last time discussed with Greg, class devices were deprecated,
> and you should just use a bus instead. (But not sure.)

The references I found online weren't clear on this, so more
investigation is required. We'll defer that till we find out if the
regmap changes eliminate the need for this.



\
 
 \ /
  Last update: 2012-09-05 03:22    [W:0.068 / U:0.528 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site