linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Christopher Heiny <cheiny@synaptics.com>
To: Linus Walleij <linus.walleij@linaro.org>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>,
	Jean Delvare <khali@linux-fr.org>,
	Linux Kernel <linux-kernel@vger.kernel.org>,
	Linux Input <linux-input@vger.kernel.org>,
	Allie Xiong <axiong@synaptics.com>,
	William Manson <wmanson@synaptics.com>,
	Peichen Chang <peichen.chang@synaptics.com>,
	Joerie de Gram <j.de.gram@gmail.com>,
	Wolfram Sang <w.sang@pengutronix.de>,
	Mathieu Poirier <mathieu.poirier@linaro.org>,
	Linus Walleij <linus.walleij@stericsson.com>,
	Naveen Kumar Gaddipati <naveen.gaddipati@stericsson.com>
Subject: Re: [RFC PATCH 5/17] input: rmidev character driver for RMI4 sensors
Date: Tue, 4 Sep 2012 17:26:23 -0700	[thread overview]
Message-ID: <50469C2F.2060008@synaptics.com> (raw)
In-Reply-To: <CACRpkdYGR=YtgyvDjfH4w8OMnn3iZKuRpQKrTVCE=i=gZ1hPuw@mail.gmail.com>

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.


  reply	other threads:[~2012-09-05  0:26 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-17 22:17 [RFC PATCH 00/11] input: Synaptics RMI4 Touchscreen Driver Christopher Heiny
2012-08-17 22:17 ` [RFC PATCH 1/17] input: RMI4 public header file and documentation Christopher Heiny
2012-08-22 19:08   ` Linus Walleij
2012-08-22 21:45     ` Dmitry Torokhov
2012-08-23  0:26       ` Christopher Heiny
2012-08-22 23:35     ` Rafael J. Wysocki
2012-08-17 22:17 ` [RFC PATCH 2/17] input: RMI4 core bus and sensor drivers Christopher Heiny
2012-08-23  8:55   ` Linus Walleij
2012-09-25 23:53     ` Christopher Heiny
2012-09-26 11:39       ` Linus Walleij
2012-08-17 22:17 ` [RFC PATCH 3/17] input: RMI4 physical layer drivers for I2C and SPI Christopher Heiny
2012-08-23 13:21   ` Linus Walleij
2012-08-17 22:17 ` [RFC PATCH 4/17] input: RMI4 configs and makefiles Christopher Heiny
2012-08-27 18:39   ` Linus Walleij
2012-08-17 22:17 ` [RFC PATCH 5/17] input: rmidev character driver for RMI4 sensors Christopher Heiny
2012-08-27 18:49   ` Linus Walleij
2012-09-05  0:26     ` Christopher Heiny [this message]
2012-09-05  8:29       ` Linus Walleij
2012-08-17 22:17 ` [RFC PATCH 6/17] input: RMI4 firmware update Christopher Heiny
2012-08-27 21:01   ` Linus Walleij
2012-08-17 22:17 ` [RFC PATCH 7/17] input: RMI4 F01 device control Christopher Heiny
2012-08-27 21:59   ` Linus Walleij
2012-08-17 22:17 ` [RFC PATCH 8/17] input: RMI4 F09 Built-In Self Test Christopher Heiny
2012-08-27 22:07   ` Linus Walleij
2012-09-05  0:21     ` Christopher Heiny
2012-08-17 22:17 ` [RFC PATCH 9/17] input: RMI4 F11 multitouch sensing Christopher Heiny
2012-08-27 22:50   ` Linus Walleij
2012-08-17 22:17 ` [RFC PATCH 10/17] input: RM4 F17 Pointing sticks Christopher Heiny
2012-08-17 22:17 ` [RFC PATCH 11/17] input: RMI4 F19 capacitive buttons Christopher Heiny
2012-08-17 22:17 ` [RFC PATCH 12/17] input: RMI4 F1A simple " Christopher Heiny
2012-08-17 22:17 ` [RFC PATCH 13/17] input: RMI4 F21 Force sensing Christopher Heiny
2012-08-17 22:17 ` [RFC PATCH 14/17] input: RMI4 F30 GPIO/LED control Christopher Heiny
2012-08-27 22:58   ` Linus Walleij
2012-09-05  0:28     ` Christopher Heiny
2012-08-17 22:17 ` [RFC PATCH 15/17] input: RMI4 F34 device reflash Christopher Heiny
2012-08-17 22:17 ` [RFC PATCH 16/17] input: RMI4 F41 Active pen 2D input Christopher Heiny
2012-08-17 22:17 ` [RFC PATCH 17/17] input: RMI4 F54 analog data reporting Christopher Heiny
2012-08-27 23:01   ` Linus Walleij
2012-09-05  0:38     ` Christopher Heiny
2012-08-22 12:50 ` [RFC PATCH 00/11] input: Synaptics RMI4 Touchscreen Driver Linus Walleij
2012-08-22 21:29   ` Christopher Heiny
2012-08-27 23:20   ` Christopher Heiny
2012-08-28  0:12     ` Linus Walleij
2012-08-27 23:05 ` Linus Walleij

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=50469C2F.2060008@synaptics.com \
    --to=cheiny@synaptics.com \
    --cc=axiong@synaptics.com \
    --cc=dmitry.torokhov@gmail.com \
    --cc=j.de.gram@gmail.com \
    --cc=khali@linux-fr.org \
    --cc=linus.walleij@linaro.org \
    --cc=linus.walleij@stericsson.com \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mathieu.poirier@linaro.org \
    --cc=naveen.gaddipati@stericsson.com \
    --cc=peichen.chang@synaptics.com \
    --cc=w.sang@pengutronix.de \
    --cc=wmanson@synaptics.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).