From: arnd@arndb.de (Arnd Bergmann)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 3.9 1/3] misc: new driver for GPIO-connected 7-segment displays
Date: Mon, 7 Jan 2013 21:41:48 +0000 [thread overview]
Message-ID: <201301072141.48487.arnd@arndb.de> (raw)
In-Reply-To: <20130107205854.1f019816@skate>
On Monday 07 January 2013, Thomas Petazzoni wrote:
> Dear Arnd Bergmann,
>
> On Mon, 7 Jan 2013 19:43:39 +0000, Arnd Bergmann wrote:
>
> > > .../devicetree/bindings/misc/gpio-7seg.txt | 18 +++
> > > drivers/misc/Kconfig | 13 ++
> > > drivers/misc/Makefile | 1 +
> > > drivers/misc/gpio-7seg.c | 168 ++++++++++++++++++++
> >
> > I wonder if it would make sense to merge this into the LED subsystem
> > rather than having it as a standalone driver.
>
> Hum, maybe. How do you see it fitting inside the LED subsystem? The
> purpose of the device is quite different in its userspace to kernel
> interface, no?
The LED subsystem can be used by both in-kernel and by user space
front-ends. It currently supports on/off, blinking and variable
brightness LEDs. Adding 7-segment LEDs may be a stretch but also
would fit into the same basic interface I think.
In addition, it would not have to be GPIO based, although that may
be the only implementation we need for the foreseeable future.
Well, unless you make a driver for PC-style "port-0x80" PCI
cards.
> > > + sdev->dev_attr.attr.name = "value";
> > > + sdev->dev_attr.attr.mode = S_IRUGO | S_IWUGO;
> > > + sdev->dev_attr.show = gpio_7seg_show;
> > > + sdev->dev_attr.store = gpio_7seg_store;
> >
> > Any reason why you are not using the DEVICE_ATTR macro?
>
> DEVICE_ATTR declares the structure, and I wanted one per gpio_7seg
> device, which is dynamically allocated, so I thought that using
> DEVICE_ATTR was not possible. But now that you point this, I realize
> that I was stupid. I can perfectly live with one single global 'struct
> device_attribute' that I register to several devices using
> device_create_file(), no?
Correct.
> That said, is it worth the effort to continue polishing this driver,
> after seeing the feedback from Greg about it? I don't mind working on
> it more and fix the problems of course, but if the general answer is
> that we don't want such a driver in the kernel, then I'd better not
> spend more time on this.
I only saw the discussion after I had sent my reply. I think that
this driver in the form of a drivers/misc sysfs driver is not worth
pursuing, but it may still be possible to integrate it nicely into
the LED framework.
Arnd
WARNING: multiple messages have this Message-ID (diff)
From: Arnd Bergmann <arnd@arndb.de>
To: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Lior Amsalem <alior@marvell.com>, Andrew Lunn <andrew@lunn.ch>,
Jason Cooper <jason@lakedaemon.net>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
devicetree-discuss@lists.ozlabs.org,
Gregory Clement <gregory.clement@free-electrons.com>,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 3.9 1/3] misc: new driver for GPIO-connected 7-segment displays
Date: Mon, 7 Jan 2013 21:41:48 +0000 [thread overview]
Message-ID: <201301072141.48487.arnd@arndb.de> (raw)
In-Reply-To: <20130107205854.1f019816@skate>
On Monday 07 January 2013, Thomas Petazzoni wrote:
> Dear Arnd Bergmann,
>
> On Mon, 7 Jan 2013 19:43:39 +0000, Arnd Bergmann wrote:
>
> > > .../devicetree/bindings/misc/gpio-7seg.txt | 18 +++
> > > drivers/misc/Kconfig | 13 ++
> > > drivers/misc/Makefile | 1 +
> > > drivers/misc/gpio-7seg.c | 168 ++++++++++++++++++++
> >
> > I wonder if it would make sense to merge this into the LED subsystem
> > rather than having it as a standalone driver.
>
> Hum, maybe. How do you see it fitting inside the LED subsystem? The
> purpose of the device is quite different in its userspace to kernel
> interface, no?
The LED subsystem can be used by both in-kernel and by user space
front-ends. It currently supports on/off, blinking and variable
brightness LEDs. Adding 7-segment LEDs may be a stretch but also
would fit into the same basic interface I think.
In addition, it would not have to be GPIO based, although that may
be the only implementation we need for the foreseeable future.
Well, unless you make a driver for PC-style "port-0x80" PCI
cards.
> > > + sdev->dev_attr.attr.name = "value";
> > > + sdev->dev_attr.attr.mode = S_IRUGO | S_IWUGO;
> > > + sdev->dev_attr.show = gpio_7seg_show;
> > > + sdev->dev_attr.store = gpio_7seg_store;
> >
> > Any reason why you are not using the DEVICE_ATTR macro?
>
> DEVICE_ATTR declares the structure, and I wanted one per gpio_7seg
> device, which is dynamically allocated, so I thought that using
> DEVICE_ATTR was not possible. But now that you point this, I realize
> that I was stupid. I can perfectly live with one single global 'struct
> device_attribute' that I register to several devices using
> device_create_file(), no?
Correct.
> That said, is it worth the effort to continue polishing this driver,
> after seeing the feedback from Greg about it? I don't mind working on
> it more and fix the problems of course, but if the general answer is
> that we don't want such a driver in the kernel, then I'd better not
> spend more time on this.
I only saw the discussion after I had sent my reply. I think that
this driver in the form of a drivers/misc sysfs driver is not worth
pursuing, but it may still be possible to integrate it nicely into
the LED framework.
Arnd
next prev parent reply other threads:[~2013-01-07 21:41 UTC|newest]
Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-01-07 16:42 [PATCH 3.9] Driver for 7-segment displays connected over GPIOs Thomas Petazzoni
2013-01-07 16:42 ` Thomas Petazzoni
2013-01-07 16:42 ` [PATCH 3.9 1/3] misc: new driver for GPIO-connected 7-segment displays Thomas Petazzoni
2013-01-07 16:42 ` Thomas Petazzoni
2013-01-07 19:43 ` Arnd Bergmann
2013-01-07 19:43 ` Arnd Bergmann
2013-01-07 19:58 ` Thomas Petazzoni
2013-01-07 19:58 ` Thomas Petazzoni
2013-01-07 21:41 ` Arnd Bergmann [this message]
2013-01-07 21:41 ` Arnd Bergmann
2013-02-08 22:38 ` Grant Likely
2013-02-08 22:38 ` Grant Likely
2013-02-08 22:35 ` Grant Likely
2013-02-08 22:35 ` Grant Likely
2013-01-07 16:42 ` [PATCH 3.9 2/3] arm: mvebu: add the 7-segment display DT informations to Armada 370 DB Thomas Petazzoni
2013-01-07 16:42 ` Thomas Petazzoni
2013-01-07 16:42 ` [PATCH 3.9 3/3] arm: mvebu: add the 7-segment display DT informations to Armada XP DB Thomas Petazzoni
2013-01-07 16:42 ` Thomas Petazzoni
2013-02-08 22:39 ` Grant Likely
2013-02-08 22:39 ` Grant Likely
2013-01-07 16:48 ` [PATCH 3.9] Driver for 7-segment displays connected over GPIOs Greg Kroah-Hartman
2013-01-07 16:48 ` Greg Kroah-Hartman
2013-01-07 17:07 ` Thomas Petazzoni
2013-01-07 17:07 ` Thomas Petazzoni
2013-01-07 17:23 ` Greg Kroah-Hartman
2013-01-07 17:23 ` Greg Kroah-Hartman
2013-01-07 17:40 ` Thomas Petazzoni
2013-01-07 17:40 ` Thomas Petazzoni
2013-01-07 17:43 ` Thomas Petazzoni
2013-01-07 17:43 ` Thomas Petazzoni
2013-01-07 17:56 ` H Hartley Sweeten
2013-01-07 17:56 ` H Hartley Sweeten
2013-01-07 18:02 ` Jason Cooper
2013-01-07 18:02 ` Jason Cooper
2013-01-07 18:06 ` Russell King - ARM Linux
2013-01-07 18:06 ` Russell King - ARM Linux
2013-01-07 17:53 ` Thomas Petazzoni
2013-01-07 17:53 ` Thomas Petazzoni
2013-01-07 18:36 ` Greg Kroah-Hartman
2013-01-07 18:36 ` Greg Kroah-Hartman
2013-01-07 18:38 ` Thomas Petazzoni
2013-01-07 18:38 ` Thomas Petazzoni
2013-01-07 23:05 ` Linus Walleij
2013-01-07 23:05 ` Linus Walleij
2013-02-09 16:28 ` Pavel Machek
2013-02-09 16:28 ` Pavel Machek
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=201301072141.48487.arnd@arndb.de \
--to=arnd@arndb.de \
--cc=linux-arm-kernel@lists.infradead.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.