> Date: Wed, 31 Aug 2005 08:59:44 +0100 (BST)
> From: Mark Underwood <[email protected]>
>
> --- David Brownell wrote:
>
> > The last couple times SPI frameworks came up here, some of the feedback
> > included "make it use the driver model properly; don't be like I2C".
> >
> > In hopes that it'll be useful, here's a small SPI core with driver model
> > support driven from board-specific tables listing devices. I expect the
> > I/O call(s) could stand to change; but at least this one starts out right,
> > based on async I/O. (There's a synchronous call; it's a trivial wrapper.)
> >
> > ...
>
> Well I guess great minds think alike ;-). After
> looking though my SPI core layer I released that it in
> no way reflected the new driver model (not surprising
> as it was a copy of i2c-core.c) and I would probably
> get laughed off the kernel mailing list if I sent it
> as was ;-).
That usually doesn't happen. You'd just be told "make it use the driver
model properly; don't be like I2C." Though maybe there'd be a fiew
other criticisms mixed in. :)
> I am now writing a new spi-core.c which uses the new
> driver model.
How about just merging the code I sent? It's not large, and it solves
that problem. I don't much care about the I/O model issues quite yet,
though requirements for quick sensor captures (RPC-ish) would seem
different from ones like reading bulk SPI flash data.
> For registering an adapter:
> 1) Register an adapter that has a cs table showing
> where devices sit on the adapter.
But how is the adapter driver itself supposed to know that?
That's what I addressed with my patch: the need for the config tables
to be **independent** of controller (and protocol) code. It decouples
all the board-specific tables from the drivers.
(Example shown below.)
The nightmare to avoid is this: EVERY time someone adds a new
SPI-equipped board, working/debugged/stable drivers need to change,
because the board-specific config data was never separated from the
drivers. (And we know it can be, as shown in the patch I posted...)
Ideally adding a new board means adding a source file for just that one
board, with the relevent implementation parameters. Only when hardware
guys do something funky should any driver need to change.
> 2) This causes spi-core to enumerate the devices on
> the cs table and register them.
>
> For un-registering an adapter:
> 1) Unregister an adapter
> 2) This causes spi-core to remove all the children of
> the adapter
Right, that's all exactly as in the patch I posted, though I punted
on the "unregister" path -- an exercise for the reader! -- because I
wanted to focus on (a) the driver model structure, like where things
land in sysfs, and (b) how to keep board-specific initialization code
out of controller and protocol drivers.
- Dave
--- o26.orig/arch/arm/mach-omap1/board-osk.c 2005-08-27 02:11:45.000000000 -0700
+++ o26/arch/arm/mach-omap1/board-osk.c 2005-08-27 18:44:20.000000000 -0700
@@ -193,6 +193,34 @@ static struct omap_board_config_kernel o
#ifdef CONFIG_OMAP_OSK_MISTRAL
+#include <linux/spi.h>
+
+struct ads7864_info { /* FIXME put in standard header */
+ u16 pen_irq, busy; /* GPIO lines */
+ u16 x_ohms, y_ohms;
+};
+
+static struct ads7864_info mistral_ts_info = {
+ .pen_irq = OMAP_GPIO_IRQ(4),
+ .busy = /* GPIO */ 6,
+ .x_ohms = 419,
+ .y_ohms = 486,
+};
+
+static const struct spi_board_info mistral_boardinfo[] = {
+{
+ /* MicroWire CS0 has an ads7846e with touchscreen and
+ * other sensors. It's currently glued into some OMAP
+ * touchscreen support that ignores the driver model.
+ */
+ .driver_name = "ads7846",
+ .platform_data = &mistral_ts_info,
+ .max_speed_hz = 2000000,
+ .bus_num = 2, /* spi2 == microwire */
+ .chip_select = 0,
+},
+};
+
#ifdef CONFIG_PM
static irqreturn_t
osk_mistral_wake_interrupt(int irq, void *ignored, struct pt_regs *regs)
@@ -211,6 +239,9 @@ static void __init osk_mistral_init(void
* But this is too early for that...
*/
+ spi_register_board_info(mistral_boardinfo,
+ ARRAY_SIZE(mistral_boardinfo));
+
/* the sideways button (SW1) is for use as a "wakeup" button */
omap_cfg_reg(N15_1610_MPUIO2);
if (omap_request_gpio(OMAP_MPUIO(2)) == 0) {
--- David Brownell <[email protected]> wrote:
> > Date: Wed, 31 Aug 2005 08:59:44 +0100 (BST)
> > From: Mark Underwood <[email protected]>
> >
> > --- David Brownell wrote:
> >
> > > The last couple times SPI frameworks came up
> here, some of the feedback
> > > included "make it use the driver model properly;
> don't be like I2C".
> > >
> > > In hopes that it'll be useful, here's a small
> SPI core with driver model
> > > support driven from board-specific tables
> listing devices. I expect the
> > > I/O call(s) could stand to change; but at least
> this one starts out right,
> > > based on async I/O. (There's a synchronous
> call; it's a trivial wrapper.)
> > >
> > > ...
> >
> > Well I guess great minds think alike ;-). After
> > looking though my SPI core layer I released that
> it in
> > no way reflected the new driver model (not
> surprising
> > as it was a copy of i2c-core.c) and I would
> probably
> > get laughed off the kernel mailing list if I sent
> it
> > as was ;-).
>
> That usually doesn't happen. You'd just be told
> "make it use the driver
> model properly; don't be like I2C." Though maybe
> there'd be a fiew
> other criticisms mixed in. :)
>
>
> > I am now writing a new spi-core.c which uses the
> new
> > driver model.
>
> How about just merging the code I sent? It's not
> large, and it solves
> that problem. I don't much care about the I/O model
> issues quite yet,
> though requirements for quick sensor captures
> (RPC-ish) would seem
> different from ones like reading bulk SPI flash
> data.
>
>
> > For registering an adapter:
> > 1) Register an adapter that has a cs table showing
> > where devices sit on the adapter.
>
> But how is the adapter driver itself supposed to
> know that?
It gets passed the cs table as part of its platform
data.
>
> That's what I addressed with my patch: the need for
> the config tables
> to be **independent** of controller (and protocol)
> code. It decouples
> all the board-specific tables from the drivers.
>
> (Example shown below.)
>
> The nightmare to avoid is this: EVERY time someone
> adds a new
> SPI-equipped board, working/debugged/stable drivers
> need to change,
> because the board-specific config data was never
> separated from the
> drivers. (And we know it can be, as shown in the
> patch I posted...)
Now I've fixed my version I'll have a more detailed
look.
>
> Ideally adding a new board means adding a source
> file for just that one
> board, with the relevent implementation parameters.
> Only when hardware
> guys do something funky should any driver need to
> change.
>
That's what happens in my SPI subsystem. The adapter
driver only knows how the driver the adapter. When a
adapter gets probed it has platform data passed to it
which contains a pointer to the cs table, the number
of entry?s in the cs table and the pointer to a
function to control some GPIO(s) as cs for adapters
that don?t have any built in.
>
> > 2) This causes spi-core to enumerate the devices
> on
> > the cs table and register them.
> >
> > For un-registering an adapter:
> > 1) Unregister an adapter
> > 2) This causes spi-core to remove all the children
> of
> > the adapter
>
> Right, that's all exactly as in the patch I posted,
> though I punted
> on the "unregister" path -- an exercise for the
> reader! -- because I
> wanted to focus on (a) the driver model structure,
> like where things
> land in sysfs, and (b) how to keep board-specific
> initialization code
> out of controller and protocol drivers.
OK. If you want I could do the same, that is send the
un/registration and sysfs code before I put the
transfer methods in. I have some dummy devices so you
can see what happens in sysfs.
>
> - Dave
>
>
> --- o26.orig/arch/arm/mach-omap1/board-osk.c
> 2005-08-27 02:11:45.000000000 -0700
> +++ o26/arch/arm/mach-omap1/board-osk.c 2005-08-27
> 18:44:20.000000000 -0700
> @@ -193,6 +193,34 @@ static struct
> omap_board_config_kernel o
>
> #ifdef CONFIG_OMAP_OSK_MISTRAL
>
> +#include <linux/spi.h>
> +
> +struct ads7864_info { /* FIXME put in standard
> header */
> + u16 pen_irq, busy; /* GPIO lines */
> + u16 x_ohms, y_ohms;
> +};
> +
> +static struct ads7864_info mistral_ts_info = {
> + .pen_irq = OMAP_GPIO_IRQ(4),
> + .busy = /* GPIO */ 6,
> + .x_ohms = 419,
> + .y_ohms = 486,
> +};
> +
> +static const struct spi_board_info
> mistral_boardinfo[] = {
> +{
> + /* MicroWire CS0 has an ads7846e with touchscreen
> and
> + * other sensors. It's currently glued into some
> OMAP
> + * touchscreen support that ignores the driver
> model.
> + */
> + .driver_name = "ads7846",
> + .platform_data = &mistral_ts_info,
> + .max_speed_hz = 2000000,
> + .bus_num = 2, /* spi2 == microwire */
I did think about doing this but the problem is how do
you know bus 2 is the bus you think it is? This would
work for SPI adapters that are platform devices, but
what about hot-plug devices like PCI and USB (we are
thinking of actually making a USB to SPI converter so
customers can try out some of our SPI devices on a PC
:).
Mark
> + .chip_select = 0,
> +},
> +};
> +
> #ifdef CONFIG_PM
> static irqreturn_t
> osk_mistral_wake_interrupt(int irq, void *ignored,
> struct pt_regs *regs)
> @@ -211,6 +239,9 @@ static void __init
> osk_mistral_init(void
> * But this is too early for that...
> */
>
> + spi_register_board_info(mistral_boardinfo,
> + ARRAY_SIZE(mistral_boardinfo));
> +
> /* the sideways button (SW1) is for use as a
> "wakeup" button */
> omap_cfg_reg(N15_1610_MPUIO2);
> if (omap_request_gpio(OMAP_MPUIO(2)) == 0) {
> -
> To unsubscribe from this list: send the line
> "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at
> http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
___________________________________________________________
Yahoo! Messenger - NEW crystal clear PC to PC calling worldwide with voicemail http://uk.messenger.yahoo.com
Hi Mark,
you've mentioned the code that you're working on several times, but no
one in LKML has ever seen a single line of code from you. Will you
please be so kind to share a piece of you SPI subsystem?
TIA!
Mark Underwood wrote:
>--- David Brownell <[email protected]> wrote:
>
>
>
>>>Date: Wed, 31 Aug 2005 08:59:44 +0100 (BST)
>>>From: Mark Underwood <[email protected]>
>>>
>>>--- David Brownell wrote:
>>>
>>>
>>>
>>>>The last couple times SPI frameworks came up
>>>>
>>>>
>>here, some of the feedback
>>
>>
>>>>included "make it use the driver model properly;
>>>>
>>>>
>>don't be like I2C".
>>
>>
>>>>In hopes that it'll be useful, here's a small
>>>>
>>>>
>>SPI core with driver model
>>
>>
>>>>support driven from board-specific tables
>>>>
>>>>
>>listing devices. I expect the
>>
>>
>>>>I/O call(s) could stand to change; but at least
>>>>
>>>>
>>this one starts out right,
>>
>>
>>>>based on async I/O. (There's a synchronous
>>>>
>>>>
>>call; it's a trivial wrapper.)
>>
>>
>>>> ...
>>>>
>>>>
>>>Well I guess great minds think alike ;-). After
>>>looking though my SPI core layer I released that
>>>
>>>
>>it in
>>
>>
>>>no way reflected the new driver model (not
>>>
>>>
>>surprising
>>
>>
>>>as it was a copy of i2c-core.c) and I would
>>>
>>>
>>probably
>>
>>
>>>get laughed off the kernel mailing list if I sent
>>>
>>>
>>it
>>
>>
>>>as was ;-).
>>>
>>>
>>That usually doesn't happen. You'd just be told
>>"make it use the driver
>>model properly; don't be like I2C." Though maybe
>>there'd be a fiew
>>other criticisms mixed in. :)
>>
>>
>>
>>
>>>I am now writing a new spi-core.c which uses the
>>>
>>>
>>new
>>
>>
>>>driver model.
>>>
>>>
>>How about just merging the code I sent? It's not
>>large, and it solves
>>that problem. I don't much care about the I/O model
>>issues quite yet,
>>though requirements for quick sensor captures
>>(RPC-ish) would seem
>>different from ones like reading bulk SPI flash
>>data.
>>
>>
>>
>>
>>>For registering an adapter:
>>>1) Register an adapter that has a cs table showing
>>>where devices sit on the adapter.
>>>
>>>
>>But how is the adapter driver itself supposed to
>>know that?
>>
>>
>
>It gets passed the cs table as part of its platform
>data.
>
>
>
>>That's what I addressed with my patch: the need for
>>the config tables
>>to be **independent** of controller (and protocol)
>>code. It decouples
>>all the board-specific tables from the drivers.
>>
>>(Example shown below.)
>>
>>The nightmare to avoid is this: EVERY time someone
>>adds a new
>>SPI-equipped board, working/debugged/stable drivers
>>need to change,
>>because the board-specific config data was never
>>separated from the
>>drivers. (And we know it can be, as shown in the
>>patch I posted...)
>>
>>
>
>Now I've fixed my version I'll have a more detailed
>look.
>
>
>
>>Ideally adding a new board means adding a source
>>file for just that one
>>board, with the relevent implementation parameters.
>>Only when hardware
>>guys do something funky should any driver need to
>>change.
>>
>>
>>
>
>That's what happens in my SPI subsystem. The adapter
>driver only knows how the driver the adapter. When a
>adapter gets probed it has platform data passed to it
>which contains a pointer to the cs table, the number
>of entry?s in the cs table and the pointer to a
>function to control some GPIO(s) as cs for adapters
>that don?t have any built in.
>
>
>
>>>2) This causes spi-core to enumerate the devices
>>>
>>>
>>on
>>
>>
>>>the cs table and register them.
>>>
>>>For un-registering an adapter:
>>>1) Unregister an adapter
>>>2) This causes spi-core to remove all the children
>>>
>>>
>>of
>>
>>
>>>the adapter
>>>
>>>
>>Right, that's all exactly as in the patch I posted,
>>though I punted
>>on the "unregister" path -- an exercise for the
>>reader! -- because I
>>wanted to focus on (a) the driver model structure,
>>like where things
>>land in sysfs, and (b) how to keep board-specific
>>initialization code
>>out of controller and protocol drivers.
>>
>>
>
>OK. If you want I could do the same, that is send the
>un/registration and sysfs code before I put the
>transfer methods in. I have some dummy devices so you
>can see what happens in sysfs.
>
>
>
>>- Dave
>>
>>
>>--- o26.orig/arch/arm/mach-omap1/board-osk.c
>>2005-08-27 02:11:45.000000000 -0700
>>+++ o26/arch/arm/mach-omap1/board-osk.c 2005-08-27
>>18:44:20.000000000 -0700
>>@@ -193,6 +193,34 @@ static struct
>>omap_board_config_kernel o
>>
>> #ifdef CONFIG_OMAP_OSK_MISTRAL
>>
>>+#include <linux/spi.h>
>>+
>>+struct ads7864_info { /* FIXME put in standard
>>header */
>>+ u16 pen_irq, busy; /* GPIO lines */
>>+ u16 x_ohms, y_ohms;
>>+};
>>+
>>+static struct ads7864_info mistral_ts_info = {
>>+ .pen_irq = OMAP_GPIO_IRQ(4),
>>+ .busy = /* GPIO */ 6,
>>+ .x_ohms = 419,
>>+ .y_ohms = 486,
>>+};
>>+
>>+static const struct spi_board_info
>>mistral_boardinfo[] = {
>>+{
>>+ /* MicroWire CS0 has an ads7846e with touchscreen
>>and
>>+ * other sensors. It's currently glued into some
>>OMAP
>>+ * touchscreen support that ignores the driver
>>model.
>>+ */
>>+ .driver_name = "ads7846",
>>+ .platform_data = &mistral_ts_info,
>>+ .max_speed_hz = 2000000,
>>+ .bus_num = 2, /* spi2 == microwire */
>>
>>
>
>I did think about doing this but the problem is how do
>you know bus 2 is the bus you think it is? This would
>work for SPI adapters that are platform devices, but
>what about hot-plug devices like PCI and USB (we are
>thinking of actually making a USB to SPI converter so
>customers can try out some of our SPI devices on a PC
>:).
>
>Mark
>
>
>
>>+ .chip_select = 0,
>>+},
>>+};
>>+
>> #ifdef CONFIG_PM
>> static irqreturn_t
>> osk_mistral_wake_interrupt(int irq, void *ignored,
>>struct pt_regs *regs)
>>@@ -211,6 +239,9 @@ static void __init
>>osk_mistral_init(void
>> * But this is too early for that...
>> */
>>
>>+ spi_register_board_info(mistral_boardinfo,
>>+ ARRAY_SIZE(mistral_boardinfo));
>>+
>> /* the sideways button (SW1) is for use as a
>>"wakeup" button */
>> omap_cfg_reg(N15_1610_MPUIO2);
>> if (omap_request_gpio(OMAP_MPUIO(2)) == 0) {
>>-
>>To unsubscribe from this list: send the line
>>"unsubscribe linux-kernel" in
>>the body of a message to [email protected]
>>More majordomo info at
>>http://vger.kernel.org/majordomo-info.html
>>Please read the FAQ at http://www.tux.org/lkml/
>>
>>
>>
>
>
>
>
>
>
>___________________________________________________________
>Yahoo! Messenger - NEW crystal clear PC to PC calling worldwide with voicemail http://uk.messenger.yahoo.com
>-
>To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>the body of a message to [email protected]
>More majordomo info at http://vger.kernel.org/majordomo-info.html
>Please read the FAQ at http://www.tux.org/lkml/
>
>
>
>
> > @@ -193,6 +193,34 @@
> >
> > #ifdef CONFIG_OMAP_OSK_MISTRAL
> >
> > +#include <linux/spi.h>
> > +
> > +struct ads7864_info { /* FIXME put in standard header */
> > + u16 pen_irq, busy; /* GPIO lines */
> > + u16 x_ohms, y_ohms;
> > +};
> > +
> > +static struct ads7864_info mistral_ts_info = {
> > + .pen_irq = OMAP_GPIO_IRQ(4),
> > + .busy = /* GPIO */ 6,
> > + .x_ohms = 419,
> > + .y_ohms = 486,
> > +};
> > +
> > +static const struct spi_board_info mistral_boardinfo[] = {
> > +{
> > + /* MicroWire CS0 has an ads7846e with touchscreen and
> > + * other sensors. It's currently glued into some OMAP
> > + * touchscreen support that ignores the driver model.
> > + */
> > + .driver_name = "ads7846",
> > + .platform_data = &mistral_ts_info,
> > + .max_speed_hz = 2000000,
> > + .bus_num = 2, /* spi2 == microwire */
>
> I did think about doing this but the problem is how do
> you know bus 2 is the bus you think it is?
The numbering is board-specific, but in most cases that can be simplified
to being SOC-specific. In this case I numbered the SPI-capable controllers
(from 1..7, not included in this patch) and this one came out "2". The
consistency matters, not the actual (nonzero) numbers.
Hotpluggable SPI controllers are not common, but that's where that sysfs
API to define new devices would really hit the spot. That would be how the
kernel learns about "struct spi_board_info" structures associated with some
dynamically assigned bus_num. Likely some convention for dynamically
assigned IDs would help, like "they're all negative".
(What I've seen a bit more often is that expansion cards will be wired
for SPI, so the thing that's vaguely hotplug-ish is that once you know
what card's hooked up, you'll know the SPI devices it has. Then the
question is how to tell the kernel about them ... same solution, which
again must work without hardware probing.)
> This would
> work for SPI adapters that are platform devices, but
> what about hot-plug devices like PCI and USB (we are
> thinking of actually making a USB to SPI converter so
> customers can try out some of our SPI devices on a PC
Some of the microcontrollers that talk both USB (slave) and SPI (master)
will even run Linux for you. :)
- Dave
--- David Brownell <[email protected]> wrote:
> > > @@ -193,6 +193,34 @@
> > >
> > > #ifdef CONFIG_OMAP_OSK_MISTRAL
> > >
> > > +#include <linux/spi.h>
> > > +
> > > +struct ads7864_info { /* FIXME put in standard
> header */
> > > + u16 pen_irq, busy; /* GPIO lines */
> > > + u16 x_ohms, y_ohms;
> > > +};
> > > +
> > > +static struct ads7864_info mistral_ts_info = {
> > > + .pen_irq = OMAP_GPIO_IRQ(4),
> > > + .busy = /* GPIO */ 6,
> > > + .x_ohms = 419,
> > > + .y_ohms = 486,
> > > +};
> > > +
> > > +static const struct spi_board_info
> mistral_boardinfo[] = {
> > > +{
> > > + /* MicroWire CS0 has an ads7846e with
> touchscreen and
> > > + * other sensors. It's currently glued into
> some OMAP
> > > + * touchscreen support that ignores the driver
> model.
> > > + */
> > > + .driver_name = "ads7846",
> > > + .platform_data = &mistral_ts_info,
> > > + .max_speed_hz = 2000000,
> > > + .bus_num = 2, /* spi2 == microwire */
> >
> > I did think about doing this but the problem is
> how do
> > you know bus 2 is the bus you think it is?
>
> The numbering is board-specific, but in most cases
> that can be simplified
> to being SOC-specific. In this case I numbered the
> SPI-capable controllers
> (from 1..7, not included in this patch) and this one
> came out "2". The
> consistency matters, not the actual (nonzero)
> numbers.
>
> Hotpluggable SPI controllers are not common, but
> that's where that sysfs
> API to define new devices would really hit the spot.
> That would be how the
> kernel learns about "struct spi_board_info"
> structures associated with some
> dynamically assigned bus_num. Likely some
> convention for dynamically
> assigned IDs would help, like "they're all
> negative".
>
> (What I've seen a bit more often is that expansion
> cards will be wired
> for SPI, so the thing that's vaguely hotplug-ish is
> that once you know
> what card's hooked up, you'll know the SPI devices
> it has. Then the
> question is how to tell the kernel about them ...
> same solution, which
> again must work without hardware probing.)
>
This is why I decided to pass the cs table as platform
data when an adapter is registered. This way you don't
have to try to find out an adapters bus number as the
adapter has the cs table in it, but because it was
passed in as platform data it still abstracts that
from the adapter driver. Simple, yet effective :)
Have you looked at the patch which I sent?
http://www.ussg.iu.edu/hypermail/linux/kernel/0509.0/0817.html
I would appreciate any comments on this approach.
>
> > This would
> > work for SPI adapters that are platform devices,
> but
> > what about hot-plug devices like PCI and USB (we
> are
> > thinking of actually making a USB to SPI converter
> so
> > customers can try out some of our SPI devices on a
> PC
>
> Some of the microcontrollers that talk both USB
> (slave) and SPI (master)
> will even run Linux for you. :)
Yes, but not on a 8051 bassed Cypress USB chip ;)
>
> - Dave
>
> -
> To unsubscribe from this list: send the line
> "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at
> http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
___________________________________________________________
Yahoo! Messenger - NEW crystal clear PC to PC calling worldwide with voicemail http://uk.messenger.yahoo.com
> > > I did think about doing this but the problem is how do
> > > you know bus 2 is the bus you think it is?
> >
> > The numbering is board-specific, but in most cases
> > that can be simplified to being SOC-specific. ...
> >
> > Hotpluggable SPI controllers are not common, but
> > that's where that sysfs API to define new devices
> > would really hit the spot. ...
> >
> > (What I've seen a bit more often is that expansion
> > cards will be wired for SPI, so the thing that's
> > vaguely hotplug-ish is that once you know what
> > card's hooked up, you'll know the SPI devices it
> > has. Then the question is how to tell the kernel
> > about them ... same solution, which again must work
> > without hardware probing.)
>
> This is why I decided to pass the cs table as platform
> data when an adapter is registered. This way you don't
> have to try to find out an adapters bus number as the
> adapter has the cs table in it, but because it was
> passed in as platform data it still abstracts that
> from the adapter driver. Simple, yet effective :)
Except that it doesn't work in that primary case, where the SPI devices
are physically decoupled from any given SPI (master) controller.
One expansion card uses CS1 for a touchscreen; another uses CS3 for
SPI flash ... the same "cs table" can't handle both configurations.
It's got to be segmented, with devices separated from controllers.
Plus, that depends on standardizing platform_data between platforms.
That's really not the model for platform_data! And "struct clk" is
ARM-only for now, too ...
> Have you looked at the patch which I sent?
> http://www.ussg.iu.edu/hypermail/linux/kernel/0509.0/0817.html
>
> I would appreciate any comments on this approach.
Yes, I plan to follow up to that with comments. As with Dmitry's
proposal, it's modeled closely on I2C, and is in consequence larger
than needed for what it does.
One reason I posted this driver-model-only patch was to highlight how
minimal an SPI core can be if it reuses the driver model core. I'm
not a fan of much "mid-layer" infrastructure in driver stacks.
- Dave
--- David Brownell <[email protected]> wrote:
> > > > I did think about doing this but the problem
> is how do
> > > > you know bus 2 is the bus you think it is?
> > >
> > > The numbering is board-specific, but in most
> cases
> > > that can be simplified to being SOC-specific.
> ...
> > >
> > > Hotpluggable SPI controllers are not common, but
> > > that's where that sysfs API to define new
> devices
> > > would really hit the spot. ...
> > >
> > > (What I've seen a bit more often is that
> expansion
> > > cards will be wired for SPI, so the thing that's
> > > vaguely hotplug-ish is that once you know what
> > > card's hooked up, you'll know the SPI devices it
> > > has. Then the question is how to tell the
> kernel
> > > about them ... same solution, which again must
> work
> > > without hardware probing.)
> >
> > This is why I decided to pass the cs table as
> platform
> > data when an adapter is registered. This way you
> don't
> > have to try to find out an adapters bus number as
> the
> > adapter has the cs table in it, but because it was
> > passed in as platform data it still abstracts that
> > from the adapter driver. Simple, yet effective :)
>
> Except that it doesn't work in that primary case,
> where the SPI devices
> are physically decoupled from any given SPI (master)
> controller.
> One expansion card uses CS1 for a touchscreen;
> another uses CS3 for
> SPI flash ... the same "cs table" can't handle both
> configurations.
> It's got to be segmented, with devices separated
> from controllers.
With my subsystem that would look like:
static const struct spi_cs_table
platform_spi1_cs_table[] = {
{
.name = "touchscreen",
.cs_no = 1,
.platform_data = NULL,
.flags = SPI_CS_IDLE_HIGH,
.cs_data = 0,
},
{
.name = "flash",
.cs_no = 3,
.platform_data = NULL,
.flags = SPI_CS_IDLE_HIGH,
.cs_data = 0,
},
};
As far as I can see most SPI devices have fixed
wirering to an adapter as SPI is not really a hotplug
bus.
The subsystem does allow you to add extra devices that
aren't in the cs table if you want by calling
spi_device_register in which case you have to setup
the spi_device with the correct information.
>
> Plus, that depends on standardizing platform_data
> between platforms.
> That's really not the model for platform_data! And
> "struct clk" is
> ARM-only for now, too ...
The struct clk should have been removed, I missed it
on that tidy up.
Why not pass platform data through the platform_data
pointer? I have now provided an extra field now which
lets you pass in any other platform data.
>
>
> > Have you looked at the patch which I sent?
> >
>
http://www.ussg.iu.edu/hypermail/linux/kernel/0509.0/0817.html
> >
> > I would appreciate any comments on this approach.
>
> Yes, I plan to follow up to that with comments. As
> with Dmitry's
> proposal, it's modeled closely on I2C, and is in
> consequence larger
> than needed for what it does.
I hope not, I spent a long time learning about the
features of the 2.6 driver model. I'm happy to trim
down any fat.
>
> One reason I posted this driver-model-only patch was
> to highlight how
> minimal an SPI core can be if it reuses the driver
> model core. I'm
> not a fan of much "mid-layer" infrastructure in
> driver stacks.
>
This is what my SPI core tries to do. I would like to
make at 'as small as possible and no smaller'
Mark
> - Dave
>
> -
> To unsubscribe from this list: send the line
> "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at
> http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
Send instant messages to your online friends http://uk.messenger.yahoo.com
> With my subsystem that would look like:
>
> static const struct spi_cs_table
> platform_spi1_cs_table[] = {
> {
> .name = "touchscreen",
> .cs_no = 1,
> .platform_data = NULL,
> .flags = SPI_CS_IDLE_HIGH,
> .cs_data = 0,
> },
> {
> .name = "flash",
> .cs_no = 3,
> .platform_data = NULL,
> .flags = SPI_CS_IDLE_HIGH,
> .cs_data = 0,
> },
> };
The problem scenario was that only one configuration
is valid at a time ... it would have been clearer if
both the add-on boards used CS1, so that device would
be either ads7864 _or_ at25640a, but not both.
> As far as I can see most SPI devices have fixed
> wirering to an adapter as SPI is not really a hotplug
> bus.
That wiring can be through an expansion connector though, which
is what I meant when I wrote that it's "vaguely hotplug-ish".
Example, the mainboard could have some SPI devices hard-wired,
on CS0 and CS2, while each different plugin board might add very
different devices on CS1 and/or CS3.
In any case, you were the one who also wanted to ship sample USB
peripherals that acted as adapters to various SPI chips... so that
bus adapters would really need to hotplug! :)
> The subsystem does allow you to add extra devices that
> aren't in the cs table if you want by calling
> spi_device_register in which case you have to setup
> the spi_device with the correct information.
Right, but as I had explained, the scenario is that the
SPI devices are on some easily-swapped add-on card.
That's a common physical arrangement for small embeddable
boards, because it takes so few wires per device.
Swapping those SPI connectors shouldn't involve changing
the declaration of the controller on the mainboard ...
> > One reason I posted this driver-model-only patch was
> > to highlight how
> > minimal an SPI core can be if it reuses the driver
> > model core. I'm
> > not a fan of much "mid-layer" infrastructure in
> > driver stacks.
> >
>
> This is what my SPI core tries to do. I would like to
> make at 'as small as possible and no smaller'
I'll post a refresh of my patch that seems to me to be
a much better match for those goals. The refresh includes
some tweaks based on what you sent, but it's still just
one KByte of overhead in the target ROM. :)
- Dave
--- David Brownell <[email protected]> wrote:
> > With my subsystem that would look like:
> >
> > static const struct spi_cs_table
> > platform_spi1_cs_table[] = {
> > {
> > .name = "touchscreen",
> > .cs_no = 1,
> > .platform_data = NULL,
> > .flags = SPI_CS_IDLE_HIGH,
> > .cs_data = 0,
> > },
> > {
> > .name = "flash",
> > .cs_no = 3,
> > .platform_data = NULL,
> > .flags = SPI_CS_IDLE_HIGH,
> > .cs_data = 0,
> > },
> > };
>
> The problem scenario was that only one configuration
> is valid at a time ... it would have been clearer if
> both the add-on boards used CS1, so that device
> would
> be either ads7864 _or_ at25640a, but not both.
>
>
>
> > As far as I can see most SPI devices have fixed
> > wirering to an adapter as SPI is not really a
> hotplug
> > bus.
>
> That wiring can be through an expansion connector
> though, which
> is what I meant when I wrote that it's "vaguely
> hotplug-ish".
> Example, the mainboard could have some SPI devices
> hard-wired,
> on CS0 and CS2, while each different plugin board
> might add very
> different devices on CS1 and/or CS3.
>
> In any case, you were the one who also wanted to
> ship sample USB
> peripherals that acted as adapters to various SPI
> chips... so that
> bus adapters would really need to hotplug! :)
>
I see several posabiltiys of how SPI devices could be
connected to an adapter.
1) All SPI devices are hardwired to the adapter. I
think this would be the most common. In this case you
would register a cs table as part of the platform data
of the SPI adapter like in the example platform in my
patch. Note: this is even the case with a PCI or usb
based device as it is the adapter that is hotpluged
and not the SPI devices on that device.
2) Some SPI devices are hardwired and some are
removable. In this case these the hardwired ones would
be put in the cs table and the other SPI devices would
be registered by calling spi_device_register. I would
add a call in my core layer to which you could pass
the bus_id and it would pass back the adapters pointer
to put in the spi_device structure.
3) All SPI devices are removable. An empty cs table
would be used and SPI devices would be registered by
calling spi_device_register.
>
> > The subsystem does allow you to add extra devices
> that
> > aren't in the cs table if you want by calling
> > spi_device_register in which case you have to
> setup
> > the spi_device with the correct information.
>
> Right, but as I had explained, the scenario is that
> the
> SPI devices are on some easily-swapped add-on card.
> That's a common physical arrangement for small
> embeddable
> boards, because it takes so few wires per device.
>
> Swapping those SPI connectors shouldn't involve
> changing
> the declaration of the controller on the mainboard
> ...
Your not when you use spi_device_register /
spi_device_unregister. You can register an adapter
with an empty cs table if you don't have any hardwired
SPI devices. When you plug a card in you use
spi_device_register to add that device to the system
and when you remove the card you call
spi_device_unregister. You can then do the same for a
different card and at no time have you changed the
declaration of the controller.
>
>
> > > One reason I posted this driver-model-only patch
> was
> > > to highlight how
> > > minimal an SPI core can be if it reuses the
> driver
> > > model core. I'm
> > > not a fan of much "mid-layer" infrastructure in
> > > driver stacks.
> > >
> >
> > This is what my SPI core tries to do. I would like
> to
> > make at 'as small as possible and no smaller'
>
> I'll post a refresh of my patch that seems to me to
> be
> a much better match for those goals. The refresh
> includes
> some tweaks based on what you sent, but it's still
> just
> one KByte of overhead in the target ROM. :)
OK. I will post an updated version of my SPI subsystem
within the next few days with the transfer stuff added
and maybe the interrupt and GPO abstraction as well.
I haven't seen any replies to my SPI patch :( did you
reply to it?
Mark
>
> - Dave
>
>
___________________________________________________________
To help you stay safe and secure online, we've developed the all new Yahoo! Security Centre. http://uk.security.yahoo.com
> Date: Wed, 7 Sep 2005 19:38:43 +0100 (BST)
> From: Mark Underwood <[email protected]>
> ...
>
> I see several posabiltiys of how SPI devices could be
> connected to an adapter.
Certainly, and all are addressed cleanly by the kind of
configuration scheme I showed.
> 1) All SPI devices are hardwired to the adapter. I
> think this would be the most common.
For custom hardware, not designed for expansion, yes. Zaurus Corgi
models, for example, keep three SPI devices busy.
But in that category I'd also include custom hardware that happens to
be packaged by connecting boards, which is also the territory of #2 or
#3 below. "Hard wired" can include connectors that are removable by
breaking the warranty. :)
> 2) Some SPI devices are hardwired and some are
> removable.
Development/Evaluation boards -- the reference implementations in most
environments, not just Linux -- seem to all but universally choose this
option (or, more rarely, #3). There might be some domain-specific chips
hardwired (touchscreen or CAN controller, ADC/DAC, etc), but expansion
connectors WILL expose SPI.
That makes sense; one goal is to support system prototyping, and it's
hard to do that if for any reason one of the major hardware connectivity
options is hard to get at!
> 3) All SPI devices are removable.
This is common for the sort of single board computers that get sold
to run Linux (or whatever) as part of semicustom hardware: maybe not
much more than a few square inches packed with an SOC CPU, FLASH, RAM,
and expansion connectors providing access to a few dozen SOC peripherals.
(There might be 250 or so SOC pins, with expansion connectors providing
access to some big portion of those pins ... including some for SPI.)
It'd be nice to be able to support those SBCs with a core Linux port,
and then just layer support for addon boards on top of that without
needing to touch a line of arch code. And convenient for folk who
are adding value through those addons. :)
> When you plug a card in you use
> spi_device_register to add that device to the system
> and when you remove the card you call
> spi_device_unregister. You can then do the same for a
> different card and at no time have you changed the
> declaration of the controller.
That implies whoever is registering is actually going and creating the
SPI devices ... and doing it AFTER the controller driver is registered.
I actually have issues with each of those implications.
However, I was also aiming to support the model where the controller
drivers are modular, and the "add driver" and "declare hardware" steps
can go in any order. That way things can work "just like other busses"
when you load the controller drivers ... and the approach is like the
familiar "boot firmware gives hardware description tables to the OS"
approach used by lots of _other_ hardware that probes poorly. (Except
that Linux is likely taking over lots of that "boot firmware" role.)
> > I'll post a refresh of my patch that seems to me to be
> > a much better match for those goals. The refresh includes
> > some tweaks based on what you sent, but it's still just
> > one KByte of overhead in the target ROM. :)
Grr. I added sysfs attributes and an I/O utility function,
and now it's a bit bigger than 1KByte. Especially with
debugging enabled, it's nearer 1.5KB. The curse of actually
trying to hook up to hardware and its quirks. :(
> OK. I will post an updated version of my SPI subsystem
> within the next few days with the transfer stuff added
> and maybe the interrupt and GPO abstraction as well.
OK.
> I haven't seen any replies to my SPI patch :( did you
> reply to it?
No, I was going to sent it when I sent that refresh; it's helped
focus my thoughts a bit. :)
Several of the comments (like "get rid of algorithm layer!") you'll have
heard before in response to the RFC from MontaVista. It seems both
approaches are still trying to make SPI seem like I2C, and not taking
the opportunity to _fix_ very much of the I2C oddness.
- Dave
--- David Brownell <[email protected]> wrote:
> > Date: Wed, 7 Sep 2005 19:38:43 +0100 (BST)
> > From: Mark Underwood <[email protected]>
> > ...
> >
> > I see several posabiltiys of how SPI devices could
> be
> > connected to an adapter.
>
> Certainly, and all are addressed cleanly by the kind
> of
> configuration scheme I showed.
And by my scheme :)
>
>
> > 1) All SPI devices are hardwired to the adapter. I
> > think this would be the most common.
>
> For custom hardware, not designed for expansion,
> yes. Zaurus Corgi
> models, for example, keep three SPI devices busy.
>
> But in that category I'd also include custom
> hardware that happens to
> be packaged by connecting boards, which is also the
> territory of #2 or
> #3 below. "Hard wired" can include connectors that
> are removable by
> breaking the warranty. :)
>
>
> > 2) Some SPI devices are hardwired and some are
> > removable.
>
> Development/Evaluation boards -- the reference
> implementations in most
> environments, not just Linux -- seem to all but
> universally choose this
> option (or, more rarely, #3). There might be some
> domain-specific chips
> hardwired (touchscreen or CAN controller, ADC/DAC,
> etc), but expansion
> connectors WILL expose SPI.
>
> That makes sense; one goal is to support system
> prototyping, and it's
> hard to do that if for any reason one of the major
> hardware connectivity
> options is hard to get at!
>
>
> > 3) All SPI devices are removable.
>
> This is common for the sort of single board
> computers that get sold
> to run Linux (or whatever) as part of semicustom
> hardware: maybe not
> much more than a few square inches packed with an
> SOC CPU, FLASH, RAM,
> and expansion connectors providing access to a few
> dozen SOC peripherals.
> (There might be 250 or so SOC pins, with expansion
> connectors providing
> access to some big portion of those pins ...
> including some for SPI.)
>
> It'd be nice to be able to support those SBCs with a
> core Linux port,
> and then just layer support for addon boards on top
> of that without
> needing to touch a line of arch code. And
> convenient for folk who
> are adding value through those addons. :)
>
And with my subsystem you don't can to change any arch
code, yay again! :)
>
>
> > When you plug a card in you use
> > spi_device_register to add that device to the
> system
> > and when you remove the card you call
> > spi_device_unregister. You can then do the same
> for a
> > different card and at no time have you changed the
> > declaration of the controller.
>
> That implies whoever is registering is actually
> going and creating the
> SPI devices ... and doing it AFTER the controller
> driver is registered.
> I actually have issues with each of those
> implications.
But how can you have a device that has no connection
to the system (i.e. no registered adapter) :(. Why
would you want to add SPI devices to adapters which
aren't yet in the system?
>
> However, I was also aiming to support the model
> where the controller
> drivers are modular, and the "add driver" and
> "declare hardware" steps
> can go in any order. That way things can work "just
> like other busses"
My subsystem does that. Once you have registered the
core layer you can add SPI device drivers before or
after registering SPI devices the only restriction is
the you have to register a SPI adapter before
registering any SPI devices which use that adapter. I
think this is sensible as otherwise you have to keep a
list of all SPI devices that have been registered and
didn't have an adapter at that time and go through
this list every time you register an adapter. This
sounds like putting the cart before the horse ;).
> when you load the controller drivers ... and the
> approach is like the
> familiar "boot firmware gives hardware description
> tables to the OS"
> approach used by lots of _other_ hardware that
> probes poorly. (Except
> that Linux is likely taking over lots of that "boot
> firmware" role.)
>
>
> > > I'll post a refresh of my patch that seems to me
> to be
> > > a much better match for those goals. The
> refresh includes
> > > some tweaks based on what you sent, but it's
> still just
> > > one KByte of overhead in the target ROM. :)
>
> Grr. I added sysfs attributes and an I/O utility
> function,
> and now it's a bit bigger than 1KByte. Especially
> with
> debugging enabled, it's nearer 1.5KB. The curse of
> actually
> trying to hook up to hardware and its quirks. :(
>
I have built your spi_init.c for an ARM946EJS and I
get a .ko object of 5.1K this compares to my spi_core,
with transfer routines, of 10.6. I think there is
still some fat to trim from my core layer so I might
be able to get it smaller :).
>
> > OK. I will post an updated version of my SPI
> subsystem
> > within the next few days with the transfer stuff
> added
> > and maybe the interrupt and GPO abstraction as
> well.
>
> OK.
>
>
> > I haven't seen any replies to my SPI patch :( did
> you
> > reply to it?
>
> No, I was going to sent it when I sent that refresh;
> it's helped
> focus my thoughts a bit. :)
Glad to help :).
>
> Several of the comments (like "get rid of algorithm
> layer!") you'll have
> heard before in response to the RFC from MontaVista.
> It seems both
> approaches are still trying to make SPI seem like
> I2C, and not taking
> the opportunity to _fix_ very much of the I2C
> oddness.
OK. The only reason I had keept the algo layer is for
bitbashing, but thinking about it the pin set/clear
routines could be passed in as platform data. So I'll
remove the lago layer, cheers for the input.
Just to let you know I have been, my testing my
subsystem with a simple eeprom driver and a SPI
network device. So my subsystem is being tested under
heavy loads with transfers in size from 32K to 2
bytes!
Mark
>
> - Dave
>
> -
> To unsubscribe from this list: send the line
> "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at
> http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
___________________________________________________________
To help you stay safe and secure online, we've developed the all new Yahoo! Security Centre. http://uk.security.yahoo.com
On 9/8/05, David Brownell <[email protected]> wrote:
> That implies whoever is registering is actually going and creating the
> SPI devices ... and doing it AFTER the controller driver is registered.
> I actually have issues with each of those implications.
>
> However, I was also aiming to support the model where the controller
> drivers are modular, and the "add driver" and "declare hardware" steps
> can go in any order. That way things can work "just like other busses"
> when you load the controller drivers ... and the approach is like the
> familiar "boot firmware gives hardware description tables to the OS"
> approach used by lots of _other_ hardware that probes poorly. (Except
> that Linux is likely taking over lots of that "boot firmware" role.)
To clarify/confirm what your saying:
(I'm going to make liberal use of stars to hilight "devices" and
"drivers" just to make sure that a critical word doesn't get
transposed)
There should be four parts to the SPI model:
1. SPI bus controller *devices*; attached to another bus *instance*
(ie. platform, PCI, etc)
2. SPI bus controller *drivers*; registered with the other bus
*subsystem* (platform, PCI, etc)
3. SPI slave *devices*; attached to a specifiec SPI bus *instance*
4. SPI slave *drivers*; registered with the SPI *subsystem*
a. SPI bus controller *drivers* and slave *drivers* can be registered
at any time in any order
b. SPI bus controller *devices* can be attached to the bus subsystem at any time
c. SPI bus controller *drivers* attach to bus controller *devices* to
create new spi bus instances whenever the driver model makes a 'match'
d. SPI slave devices can be attached to an SPI bus instance only after
that bus instance is created.
e. SPI slave *drivers* attach to SPI slave *devices* when the driver
model makes a match. (let's call it an SPI slave instance)
f. Unregistration of any SPI bus controller *driver* or *device* will
cause attached SPI bus instance(s) and any attached devices to go away
g. Unregistration of any SPI slave *driver* or *device* will cause SPI
slave instance to go away.
[pretty much exactly how the driver model is supposed to work I guess :) ]
Ideally controller drivers, controller devices, slave drivers and
slave devices are all independent of each other. The board setup code
will probably take care of attaching devices to busses independent of
the bus controller drivers and spi slave drivers. Driver code is
responsible for registering itself with the SPI subsystem.
If this is what your saying, then I *strongly* second that opinion.
If not, then what is your view?
I've been dealing with the same problems on my project. Just for
kicks, here's another implementation to look at.
http://ozlabs.org/pipermail/linuxppc-embedded/2005-July/019259.html
http://ozlabs.org/pipermail/linuxppc-embedded/2005-July/019260.html
http://ozlabs.org/pipermail/linuxppc-embedded/2005-July/019261.html
http://ozlabs.org/pipermail/linuxppc-embedded/2005-July/019262.html
It also is not based on i2c in any way and it tries ot follow the
device model. It solves my problem, but I've held off active work on
it while looking at the other options being discussed here.
Thanks,
g.
--- Grant Likely <[email protected]> wrote:
> On 9/8/05, David Brownell <[email protected]>
> wrote:
> > That implies whoever is registering is actually
> going and creating the
> > SPI devices ... and doing it AFTER the controller
> driver is registered.
> > I actually have issues with each of those
> implications.
> >
> > However, I was also aiming to support the model
> where the controller
> > drivers are modular, and the "add driver" and
> "declare hardware" steps
> > can go in any order. That way things can work
> "just like other busses"
> > when you load the controller drivers ... and the
> approach is like the
> > familiar "boot firmware gives hardware description
> tables to the OS"
> > approach used by lots of _other_ hardware that
> probes poorly. (Except
> > that Linux is likely taking over lots of that
> "boot firmware" role.)
> To clarify/confirm what your saying:
>
> (I'm going to make liberal use of stars to hilight
> "devices" and
> "drivers" just to make sure that a critical word
> doesn't get
> transposed)
>
> There should be four parts to the SPI model:
> 1. SPI bus controller *devices*; attached to another
> bus *instance*
> (ie. platform, PCI, etc)
> 2. SPI bus controller *drivers*; registered with the
> other bus
> *subsystem* (platform, PCI, etc)
> 3. SPI slave *devices*; attached to a specifiec SPI
> bus *instance*
> 4. SPI slave *drivers*; registered with the SPI
> *subsystem*
>
> a. SPI bus controller *drivers* and slave *drivers*
> can be registered
> at any time in any order
> b. SPI bus controller *devices* can be attached to
> the bus subsystem at any time
> c. SPI bus controller *drivers* attach to bus
> controller *devices* to
> create new spi bus instances whenever the driver
> model makes a 'match'
> d. SPI slave devices can be attached to an SPI bus
> instance only after
> that bus instance is created.
> e. SPI slave *drivers* attach to SPI slave *devices*
> when the driver
> model makes a match. (let's call it an SPI slave
> instance)
> f. Unregistration of any SPI bus controller *driver*
> or *device* will
> cause attached SPI bus instance(s) and any attached
> devices to go away
> g. Unregistration of any SPI slave *driver* or
> *device* will cause SPI
> slave instance to go away.
>
> [pretty much exactly how the driver model is
> supposed to work I guess :) ]
>
> Ideally controller drivers, controller devices,
> slave drivers and
> slave devices are all independent of each other.
> The board setup code
> will probably take care of attaching devices to
> busses independent of
> the bus controller drivers and spi slave drivers.
> Driver code is
> responsible for registering itself with the SPI
> subsystem.
>
> If this is what your saying, then I *strongly*
> second that opinion.
> If not, then what is your view?
>
> I've been dealing with the same problems on my
> project. Just for
> kicks, here's another implementation to look at.
>
>
http://ozlabs.org/pipermail/linuxppc-embedded/2005-July/019259.html
>
http://ozlabs.org/pipermail/linuxppc-embedded/2005-July/019260.html
>
http://ozlabs.org/pipermail/linuxppc-embedded/2005-July/019261.html
>
http://ozlabs.org/pipermail/linuxppc-embedded/2005-July/019262.html
>
> It also is not based on i2c in any way and it tries
> ot follow the
> device model. It solves my problem, but I've held
> off active work on
> it while looking at the other options being
> discussed here.
Another interesting proposal :), although it doesn't
address the platform abstraction issue along with some
others which people have raised in reply to your
posts. It's good to see other people interested in
this, if we can get up enough people then maybe we can
push a SPI subsystem into the kernel :).
Mark
>
> Thanks,
> g.
> -
> To unsubscribe from this list: send the line
> "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at
> http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
Send instant messages to your online friends http://uk.messenger.yahoo.com
> Date: Fri, 9 Sep 2005 11:40:39 -0600
> From: Grant Likely <[email protected]>
>
> On 9/8/05, David Brownell <[email protected]> wrote:
> > That implies whoever is registering is actually going and creating the
> > SPI devices ... and doing it AFTER the controller driver is registered.
> > I actually have issues with each of those implications.
> >
> > However, I was also aiming to support the model where the controller
> > drivers are modular, and the "add driver" and "declare hardware" steps
> > can go in any order. That way things can work "just like other busses"
> > when you load the controller drivers ... and the approach is like the
> > familiar "boot firmware gives hardware description tables to the OS"
> > approach used by lots of _other_ hardware that probes poorly. (Except
> > that Linux is likely taking over lots of that "boot firmware" role.)
>
> To clarify/confirm what your saying:
>
> (I'm going to make liberal use of stars to hilight "devices" and
> "drivers" just to make sure that a critical word doesn't get
> transposed)
And given that you're not using quite the same terminology I am, I think
we agree here. Good! :)
In my terminology, which should be clear in the Kconfig:
- There are two types of SPI bus controller: master (issues clock)
and slave (receives clock). Linux should plan to handle both.
- On top of either type of controller driver would be a "protocol driver"
that processes messages. (Is there a better name to use for these?)
What you've called a "slave driver" is what I've called a "protocol master"
driver, talking to a slave device/chip. What I'd call a "slave driver"
would be either a controller driver or a protocol driver; but in any case
it'd run on hardware that _receives_ the SPI clock.
A slave protocol driver for the EEPROM protocol could be tricky in Linux,
given the single bit turnaround between request and response; but the
master protocol driver would be simple. Linux should easily be able to
implement data streaming protocol slaves, including things like exchanging
network packets as well as the more conventional streams of sensor data.
> There should be four parts to the SPI model:
> 1. SPI bus controller *devices*; attached to another bus *instance*
> (ie. platform, PCI, etc)
> 2. SPI bus controller *drivers*; registered with the other bus
> *subsystem* (platform, PCI, etc)
> 3. SPI slave *devices*; attached to a specifiec SPI bus *instance*
> 4. SPI slave *drivers*; registered with the SPI *subsystem*
Yes, with the proviso above: "slave driver" here mean the thing that
talks to the slave through the SPI master controller driver. (As you
noted in the PPC patches you sent, you were not addressing SPI slave
side support.)
> a. SPI bus controller *drivers* and slave *drivers* can be registered
> at any time in any order
> b. SPI bus controller *devices* can be attached to the bus subsystem at any time
> c. SPI bus controller *drivers* attach to bus controller *devices* to
> create new spi bus instances whenever the driver model makes a 'match'
> d. SPI slave devices can be attached to an SPI bus instance only after
> that bus instance is created.
Yes, where the "slave device" in this case is the Linux driver model
thing representing a software proxy for the physical slave hardware;
it's never a discrete chip I could point to on a board.
> e. SPI slave *drivers* attach to SPI slave *devices* when the driver
> model makes a match. (let's call it an SPI slave instance)
I'd call that just another "struct device" that happens to be bound to
some "struct device_driver". Being bound is optional in Linux, though
of course a device has limited utility until it's bound (since it's
not accessible to user or kernel code).
> f. Unregistration of any SPI bus controller *driver* or *device* will
> cause attached SPI bus instance(s) and any attached devices to go away
Right. Modulo the usual sysfs/kobject reference counting and memory
management issues; the memory may linger for a while, though things
will not be visible in sysfs any more.
> g. Unregistration of any SPI slave *driver* or *device* will cause SPI
> slave instance to go away.
>
> [pretty much exactly how the driver model is supposed to work I guess :) ]
Exactly so! Except actually for (g) ... unregistering a master protocol
driver (for an SPI slave chip) just leaves an unbound device. Removing
the device implies first unbinding its driver though.
The goal here is that -- unlike I2C -- an SPI driver framework should
leverage the driver model and the "Principle of Least Astonishment", so
that driver concepts (and developer skills!!) from elsewhere in Linux will
transfer as directly as practical.
> Ideally controller drivers, controller devices, slave drivers and
> slave devices are all independent of each other. The board setup code
> will probably take care of attaching devices to busses independent of
> the bus controller drivers and spi slave drivers. Driver code is
> responsible for registering itself with the SPI subsystem.
>
> If this is what your saying, then I *strongly* second that opinion.
Modulo the provisos above, yes -- that's exactly what I'm saying.
(As well as: let's get terminology settled. I didn't make a big deal
of it in my original post, and am not deeply attached to what I used,
but it's important to remember that Linux can handle both master and
slave controllers, and that the drivers talking to each will not be
quite the same ... there are four types of SPI driver involved.)
> I've been dealing with the same problems on my project. Just for
> kicks, here's another implementation to look at.
>
> http://ozlabs.org/pipermail/linuxppc-embedded/2005-July/019259.html
> http://ozlabs.org/pipermail/linuxppc-embedded/2005-July/019260.html
> http://ozlabs.org/pipermail/linuxppc-embedded/2005-July/019261.html
> http://ozlabs.org/pipermail/linuxppc-embedded/2005-July/019262.html
>
> It also is not based on i2c in any way and it tries ot follow the
> device model. It solves my problem, but I've held off active work on
> it while looking at the other options being discussed here.
I noticed that. Also the way your second patch needed some "board
specific hacks" that you didn't much like! :)
Yet another SPI stack, linked from one of those threads, is I2C-ish:
http://www.katix.org/spi.php
For me I think the high order bits involve "normal" driver model support
(which implies avoidng most borrowing from I2C); being sure that both
master and slave sides stay in the picture, and ensuring that the "SPI
master protocol drivers" can issue transfer requests from IRQ handlers
and any other contexts which happen to notice one is needed.
- Dave
> Date: Fri, 9 Sep 2005 11:33:52 +0100 (BST)
> From: Mark Underwood <[email protected]>
>
> ...
> > That implies whoever is registering is actually
> > going and creating the
> > SPI devices ... and doing it AFTER the controller
> > driver is registered.
> > I actually have issues with each of those
> > implications.
>
> But how can you have a device that has no connection
> to the system (i.e. no registered adapter) :(. Why
> would you want to add SPI devices to adapters which
> aren't yet in the system?
The devices and adapters certainly are in the system;
that's hardware! Do you maybe mean "before the driver
for an SPI adapter is bound to its device", and are
you maybe talking about driver model data structures?
The thing which exists before the SPI adapter driver is
bound to its device node is just a table. It lists the
SPI devices, and holds information needed later to set
up the hardware. Way simpler than ACPI or BIOS tables.
> > However, I was also aiming to support the model
> > where the controller
> > drivers are modular, and the "add driver" and
> > "declare hardware" steps
> > can go in any order. That way things can work "just
> > like other busses"
>
> My subsystem does that. Once you have registered the
> core layer you can add SPI device drivers before or
> after registering SPI devices the only restriction is
> the you have to register a SPI adapter before
> registering any SPI devices which use that adapter.
That "only restriction" is the one I was talking about!!
It contorts the normal roles and responsiblities of the
adapter drivers; and it's not necessary.
> I think this is sensible as otherwise you have to keep a
> list of all SPI devices that have been registered and
> didn't have an adapter at that time and go through
> this list every time you register an adapter.
Lots of systems have their earliest boot code provide a
table of devices that exist (but which can't be probed).
That's how my patch approaches SPI.
As for that "every time" ... I don't know about you, but
the systems I've seen will register at most a handful of
devices and adapters; and adapters register just once.
For those numbers, even linear search is just fine.
> I have built your spi_init.c for an ARM946EJS and I
> get a .ko object of 5.1K
... but the ".text" size is MUCH less; and what I sent
was not built as a ".so", so there's other oddness too.
I got something on the order of 0x04d0 bytes with the
refresh I just posted (call it 1200 bytes text).
- Dave
--- David Brownell <[email protected]> wrote:
> > Date: Fri, 9 Sep 2005 11:33:52 +0100 (BST)
> > From: Mark Underwood <[email protected]>
> >
> > ...
> > > That implies whoever is registering is actually
> > > going and creating the
> > > SPI devices ... and doing it AFTER the
> controller
> > > driver is registered.
> > > I actually have issues with each of those
> > > implications.
> >
> > But how can you have a device that has no
> connection
> > to the system (i.e. no registered adapter) :(. Why
> > would you want to add SPI devices to adapters
> which
> > aren't yet in the system?
>
> The devices and adapters certainly are in the
> system;
> that's hardware! Do you maybe mean "before the
> driver
> for an SPI adapter is bound to its device", and are
> you maybe talking about driver model data
> structures?
Yes
>
> The thing which exists before the SPI adapter driver
> is
> bound to its device node is just a table. It lists
> the
> SPI devices, and holds information needed later to
> set
> up the hardware. Way simpler than ACPI or BIOS
> tables.
>
>
> > > However, I was also aiming to support the model
> > > where the controller
> > > drivers are modular, and the "add driver" and
> > > "declare hardware" steps
> > > can go in any order. That way things can work
> "just
> > > like other busses"
> >
> > My subsystem does that. Once you have registered
> the
> > core layer you can add SPI device drivers before
> or
> > after registering SPI devices the only restriction
> is
> > the you have to register a SPI adapter before
> > registering any SPI devices which use that
> adapter.
>
> That "only restriction" is the one I was talking
> about!!
>
> It contorts the normal roles and responsiblities of
> the
> adapter drivers; and it's not necessary.
>
>
> > I think this is sensible as otherwise you have to
> keep a
> > list of all SPI devices that have been registered
> and
> > didn't have an adapter at that time and go through
> > this list every time you register an adapter.
>
> Lots of systems have their earliest boot code
> provide a
> table of devices that exist (but which can't be
> probed).
> That's how my patch approaches SPI.
>
> As for that "every time" ... I don't know about you,
> but
> the systems I've seen will register at most a
> handful of
> devices and adapters; and adapters register just
> once.
> For those numbers, even linear search is just fine.
>
OK. Perhaps it's time to lay out the arguments so far
and see where we go from there :).
I want to pass a cs table in as platform data because
it means you don't have to know what bus number the
adapter is (which you can't easily tell for
hotplugable devices) and for most cases I think the
SPI devices will be hardwire to the adapter. This
approach of defining a structure to be used in
platform_data seems to be used by some serial drivers
so I think it's OK, but your not so sure.
How else could I do it? I could use driver_data but
that seems an even worse solution :(.
You want to pass in SPI devices in a table that gets
registered with the subsystem and when the adapter
that they sit on gets registered (known by the bus
number) all the devices in that table get registered
in the core. I like this solution for registering
hotplug'ish devices :), although I would prefer to use
the bus_id then a bus number. This solution doesn't
work very well for hotplug adapters as you don't know
what bus number they will be given (you would have to
do some undefined sysfs magic).
So how would you feel about the core supporting both
cs tables as platform data and SPI device tables?
>
> > I have built your spi_init.c for an ARM946EJS and
> I
> > get a .ko object of 5.1K
>
> ... but the ".text" size is MUCH less; and what I
> sent
> was not built as a ".so", so there's other oddness
> too.
Sorry, I must be missing something here but isn't a
".so" a shared library not a kernel module? Or where
you only building it as a ".so" to find the text size?
Mark
> I got something on the order of 0x04d0 bytes with
> the
> refresh I just posted (call it 1200 bytes text).
>
> - Dave
>
> -
> To unsubscribe from this list: send the line
> "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at
> http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
___________________________________________________________
To help you stay safe and secure online, we've developed the all new Yahoo! Security Centre. http://uk.security.yahoo.com
> OK. Perhaps it's time to lay out the arguments so far
> and see where we go from there :).
I'd rather see a bit more feedback from other folk about what they
need and want. For one example, Dmitry ([email protected])
posted some SPI code a while back, but we've not heard from him...
Then there are other developers who need or want SPI APIs that are
less chip-specific than linux/arch/arm/mach-pxa/ssp.c (or even
linux/drivers/char/at91_spi.c, interesting since it uses DMA and
handles root-on-DataFlash) and can support development of reusable
drivers for SPI slave devices (Flash, sensors, etc) that aren't
hardwired to a given SOC or board.
> I want to pass a cs table in as platform data because
> it means you don't have to know what bus number the
> adapter is (which you can't easily tell for
> hotplugable devices)
But as you observed earlier, SPI doesn't need to handle
hotplug ... just different common arrangements of boards.
However: Linux uses bus numbers lots of places, even when
the bus can be hotplugged. So that argument's not a win.
> and for most cases I think the
> SPI devices will be hardwire to the adapter. This
> approach of defining a structure to be used in
> platform_data seems to be used by some serial drivers
> so I think it's OK, but your not so sure.
> How else could I do it?
Reread my patches; they ought to even run on x86, though of course
"Real Hardware" is typically embedded. (The folk who've contacted me
run Linux on SOC boards based on ARM, MIPS, and PPC CPUs.)
The basic model is widely used: give the operating system tables that
describe the devices that can't be probed.
> You want to pass in SPI devices in a table that gets
> registered with the subsystem and when the adapter
> that they sit on gets registered (known by the bus
> number) all the devices in that table get registered
> in the core.
Actually there can be several such tables; initialization for each board
in the system might declare a few entries. Those tables are sufficient
to describe the system setup.
That's a difference I'll highlight again: the tables you're talking about
MUST be grouped by adapter. Which means that when the hardware groups
them differently ("by board") your API needs to add another solution.
Why have two, when (as in my patch) just one could suffice?
> I like this solution for registering
> hotplug'ish devices :), although I would prefer to use
> the bus_id then a bus number.
Numbers are just one kind of ID, so I don't see what you're trying to
say here. The kernel uses numbers to identify other bootable devices;
why would you object to using them here?
> This solution doesn't
> work very well for hotplug adapters as you don't know
> what bus number they will be given (you would have to
> do some undefined sysfs magic).
Anyone trying to develop "SPI hotplug" is going to have a wide variety
of problems to solve! There's not even a standard for SPI connectors,
much less cable quality standards or dynamic device identification.
I refuse to consider such a long list of "maybes" in a design.
The scenario you mentioned was development platforms (contrast to
production hardware...) with 8 bit microcontrollers and SPI, connected
over USB. I can think of several ways to implement such devices using
the approach I posted.
> So how would you feel about the core supporting both
> cs tables as platform data and SPI device tables?
I'm not a fan of duplication of function, or code managing platform_data
that's not platform-specific code. (After all, that's why it's called
"platform" data.)
> > > I get a .ko object of 5.1K
> >
> > ... but the ".text" size is MUCH less [1K]; and what I
> > sent was not built as a ".so", so there's other oddness
> > too.
>
> Sorry, I must be missing something here but isn't a
> ".so" a shared library not a kernel module? Or where
> you only building it as a ".so" to find the text size?
Typo; they're both just dynamic linking. My point was that if you built
that core (vs a driver) to get a ".ko", you've played some strange games
already -- and broke at least one thing.
Not that size is the top concern, but on the other hand the folk using
SPI do care about it more than the folk who find bugs like "hardware
DMA fails on memory addresses over 2GB". :)
- Dave