Re: [PATCH 3/4] spi: Add OF binding support for SPI busses

2008-05-24 Thread Grant Likely
On Wed, May 21, 2008 at 7:16 PM, David Brownell [EMAIL PROTECTED] wrote:
 On Friday 16 May 2008, Grant Likely wrote:
 In my mind; platform_data and the device tree are all about the same
 thing: representation.  In other words, how to describe the
 configuration of the hardware independent of the driver itself.

 Platform_data isn't what I'd call independent of drivers.

 The reason the data is there in the first place is that
 the driver needs it ... and chose not to hard-wire it.

Oh, of course the driver needs it!  I'm not claiming otherwise.  More
what I mean is that the driver doesn't need to be loaded or even
configured in for the platform code to make use of pdata.

 One of the things I find rather interesting is just how frequently
 drivers using platform data structures have a big block of code which
 simply copy pdata fields into identically named fields in the device
 private data...

 ... because platform data was designed as a partial template
 for that driver, letting it do that.  (Sometimes without even
 doing scale conversions.)  As drivers grow functionally, they
 sometimes end up needing more platform data fields, to expose
 data that previously didn't matter.

 Whether that data can usefully be stored in flash (or ROM)
 and handed out through the bootloader is something of a
 manufacturing issue.

I do not dispute any of that.  My point, however, is that pdata is
typically used simply as a representation that is convenient for
platform code to pass that data into the driver and that often drivers
don't use that representation directly.  Instead, the data is
explicitly copied explicitly field by field into the driver at probe
time and is not touched again.  That says to me that driver developers
view pdata as somewhat decoupled from the internal workings of the
driver and in the case of many powerpc devices a different
representation is more convenient; namely a device tree node.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH 3/4] spi: Add OF binding support for SPI busses

2008-05-24 Thread Grant Likely
On Thu, May 22, 2008 at 8:05 PM, David Brownell [EMAIL PROTECTED] wrote:
 On Wednesday 21 May 2008, Anton Vorontsov wrote:
  +++ b/drivers/spi/spi_of.c

 I think better placement for this is drivers/of, no?

 Yes please.

Okay, I wasn't sure.  Will do.

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH 3/4] spi: Add OF binding support for SPI busses

2008-05-24 Thread Grant Likely
On Sat, May 24, 2008 at 12:26 AM, Grant Likely
[EMAIL PROTECTED] wrote:
 On Thu, May 22, 2008 at 8:05 PM, David Brownell [EMAIL PROTECTED] wrote:
 On Wednesday 21 May 2008, Anton Vorontsov wrote:
  +++ b/drivers/spi/spi_of.c

 I think better placement for this is drivers/of, no?

 Yes please.

 Okay, I wasn't sure.  Will do.

I'm having second thoughts about this.  I think this code is more SPI
centric than it is OF centric.  ie. it is usable by all spi masters in
an OF enabled system, but it is not usable by all OF devices in an SPI
enabled system.  Or, in other words; it adds OF support to SPI, not
the other way around.  I think drivers/spi is the right place for this
to live.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH 3/4] spi: Add OF binding support for SPI busses

2008-05-24 Thread Jochen Friedrich
Grant Likely schrieb:
 On Sat, May 24, 2008 at 12:26 AM, Grant Likely
 [EMAIL PROTECTED] wrote:
 On Thu, May 22, 2008 at 8:05 PM, David Brownell [EMAIL PROTECTED] wrote:
 On Wednesday 21 May 2008, Anton Vorontsov wrote:
 +++ b/drivers/spi/spi_of.c
 I think better placement for this is drivers/of, no?
 Yes please.
 Okay, I wasn't sure.  Will do.
 
 I'm having second thoughts about this.  I think this code is more SPI
 centric than it is OF centric.  ie. it is usable by all spi masters in
 an OF enabled system, but it is not usable by all OF devices in an SPI
 enabled system.  Or, in other words; it adds OF support to SPI, not
 the other way around.  I think drivers/spi is the right place for this
 to live.

Isn't the same true for drivers/of/gpio.c or drivers/of/of_i2c.c, as well?

Thanks,
Jochen
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH 3/4] spi: Add OF binding support for SPI busses

2008-05-24 Thread Grant Likely
On Sat, May 24, 2008 at 11:14 AM, Jochen Friedrich [EMAIL PROTECTED] wrote:
 Grant Likely schrieb:
 On Sat, May 24, 2008 at 12:26 AM, Grant Likely
 [EMAIL PROTECTED] wrote:
 On Thu, May 22, 2008 at 8:05 PM, David Brownell [EMAIL PROTECTED] wrote:
 On Wednesday 21 May 2008, Anton Vorontsov wrote:
 +++ b/drivers/spi/spi_of.c
 I think better placement for this is drivers/of, no?
 Yes please.
 Okay, I wasn't sure.  Will do.

 I'm having second thoughts about this.  I think this code is more SPI
 centric than it is OF centric.  ie. it is usable by all spi masters in
 an OF enabled system, but it is not usable by all OF devices in an SPI
 enabled system.  Or, in other words; it adds OF support to SPI, not
 the other way around.  I think drivers/spi is the right place for this
 to live.

 Isn't the same true for drivers/of/gpio.c or drivers/of/of_i2c.c, as well?

I would argue 'yes!'

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH 3/4] spi: Add OF binding support for SPI busses

2008-05-24 Thread David Brownell
On Saturday 24 May 2008, Grant Likely wrote:
   +++ b/drivers/spi/spi_of.c
 
  I think better placement for this is drivers/of, no?
 
  Yes please.
 
  Okay, I wasn't sure.  Will do.
 
 I'm having second thoughts about this.  I think this code is more SPI
 centric than it is OF centric.  ie. it is usable by all spi masters in
 an OF enabled system, but it is not usable by all OF devices in an SPI
 enabled system. 

It's not usable by *any* SPI master on a non-OF system though.
So in that sense it's far more about OF setup than it is about SPI.


 Or, in other words; it adds OF support to SPI, not 
 the other way around.  I think drivers/spi is the right place for this
 to live.

I'd still rather see such translations in the OF-specific part of
the source tree.  Like drivers/acpi/pci_*.c code, this has more to
do with the firmware interface than with bus (SPI) interface.

Arguments could be made both ways here, but for the moment it makes
more sense to me to keep this type of platform glue (be it OF, ACPI,
arch-specific setup code, or whatever) together in the source tree
and apart from the bus-specific code.

Where do the proposed patches gluing OF to I2C live, or has that
been settled yet?

- Dave


___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH 3/4] spi: Add OF binding support for SPI busses

2008-05-24 Thread David Brownell
On Saturday 24 May 2008, Grant Likely wrote:
  Isn't the same true for drivers/of/gpio.c or drivers/of/of_i2c.c, as well?
 
 I would argue 'yes!'

... all the more reason to have the SPI glue go there too,
matching the ACPI/PCI precedent as well as those others!

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH 3/4] spi: Add OF binding support for SPI busses

2008-05-24 Thread Grant Likely
On Sat, May 24, 2008 at 11:45 AM, David Brownell [EMAIL PROTECTED] wrote:
 On Saturday 24 May 2008, Grant Likely wrote:
  Isn't the same true for drivers/of/gpio.c or drivers/of/of_i2c.c, as well?

 I would argue 'yes!'

 ... all the more reason to have the SPI glue go there too,
 matching the ACPI/PCI precedent as well as those others!

Alright; I give!  I'll put it in drivers/of.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH 3/4] spi: Add OF binding support for SPI busses

2008-05-22 Thread David Brownell
On Wednesday 21 May 2008, Anton Vorontsov wrote:
  +++ b/drivers/spi/spi_of.c
 
 I think better placement for this is drivers/of, no?

Yes please.

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH 3/4] spi: Add OF binding support for SPI busses

2008-05-21 Thread Anton Vorontsov
On Fri, May 16, 2008 at 01:36:13PM -0600, Grant Likely wrote:
 From: Grant Likely [EMAIL PROTECTED]
 
 This patch adds support for populating an SPI bus based on data in the
 OF device tree.  This is useful for powerpc platforms which use the
 device tree instead of discrete code for describing platform layout.
 
 Signed-off-by: Grant Likely [EMAIL PROTECTED]
 ---
[...]
 diff --git a/drivers/spi/spi_of.c b/drivers/spi/spi_of.c
 new file mode 100644
 index 000..b5ae434
 --- /dev/null
 +++ b/drivers/spi/spi_of.c
 @@ -0,0 +1,86 @@
 +/*

I think better placement for this is drivers/of, no?

 + * SPI OF support routines
 + * Copyright (C) 2008 Secret Lab Technologies Ltd.
 + *
 + * Support routines for deriving SPI device attachments from the device
 + * tree.
 + */
 +
 +#include linux/of.h
 +#include linux/device.h
 +#include linux/spi/spi.h
 +#include linux/spi/spi_of.h
 +
 +/**
 + * spi_of_register_devices - Register child devices onto the SPI bus
 + * @master:  Pointer to spi_master device
 + * @np:  parent node of SPI device nodes
 + *
 + * Registers an spi_device for each child node of 'np' which has a 'reg'
 + * property.
 + */
 +void spi_of_register_devices(struct spi_master *master, struct device_node 
 *np)
 +{
 + struct spi_device *spi;
 + struct device_node *nc;
 + const u32 *prop;
 + const char *sprop;
 + int rc;
 + int len;
 +
 + for_each_child_of_node(np, nc) {
 + /* Alloc an spi_device */
 + spi = spi_alloc_device(master);
 + if (!spi) {
 + dev_err(master-dev, spi_device alloc error for %s\n,
 + np-full_name);
 + continue;
 + }
 +
 + /* Device address */
 + prop = of_get_property(nc, reg, len);
 + if (!prop || len  sizeof(*prop)) {
 + dev_err(master-dev, %s has no 'reg' property\n,
 + np-full_name);

Should be nc-full_name.

 + continue;
 + }
[...]

-- 
Anton Vorontsov
email: [EMAIL PROTECTED]
irc://irc.freenode.net/bd2
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH 3/4] spi: Add OF binding support for SPI busses

2008-05-21 Thread Segher Boessenkool

Ok, elegance apart:-) You can use the SPI-bridge construct to also
describe simple SPI-chipselect configurations. But is it really a good
idea? Wouldn't it be better to handle these two cases separately?


It would be best to handle all these things that are specific to
a certain SPI controller (like how CSs work) in the binding for
that SPI controller, and not try to shoehorn all of this into some
artificial generic framework.

If you can have identical addresses on the SPI bus going to different
devices based on which CS is asserted, you'll have to make the CS part
of the reg.  Example:

spi-controller {
#address-cells = 2;
#size-cells = 0;
[EMAIL PROTECTED],f000 { reg =  0 f000 ; } // CS 0, SPI address f000
[EMAIL PROTECTED],f000 { reg =  1 f000 ; } // CS 1, SPI address f000
[EMAIL PROTECTED],ff00 { reg =  1 ff00 ; } // CS 1, SPI address ff00
}

SPI-to-SPI bridges can (and should!) be handled the same way as
anything-to-anything-else bridges are handled as well: either there
is a nice simple one-to-one matching (and you can use ranges) or
you need a driver for that bridge that knows how to make it work (or
both, ranges isn't always enough, the bridge might require some
specific handling for some special situations -- error handling,
suspend, whatever).


Segher

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH 3/4] spi: Add OF binding support for SPI busses

2008-05-21 Thread Grant Likely
On Wed, May 21, 2008 at 1:11 PM, Segher Boessenkool
[EMAIL PROTECTED] wrote:
 Ok, elegance apart:-) You can use the SPI-bridge construct to also
 describe simple SPI-chipselect configurations. But is it really a good
 idea? Wouldn't it be better to handle these two cases separately?

 It would be best to handle all these things that are specific to
 a certain SPI controller (like how CSs work) in the binding for
 that SPI controller, and not try to shoehorn all of this into some
 artificial generic framework.

 If you can have identical addresses on the SPI bus going to different
 devices based on which CS is asserted, you'll have to make the CS part
 of the reg.  Example:

 spi-controller {
#address-cells = 2;
#size-cells = 0;
[EMAIL PROTECTED],f000 { reg =  0 f000 ; } // CS 0, SPI address f000
[EMAIL PROTECTED],f000 { reg =  1 f000 ; } // CS 1, SPI address f000
[EMAIL PROTECTED],ff00 { reg =  1 ff00 ; } // CS 1, SPI address ff00
 }

For SPI the CS # *is* the address.  :-)

Unlike I2C, SPI doesn't impose any protocol on the data.  It is all
anonymous data out, anonymous data in, a clock and a chip select.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH 3/4] spi: Add OF binding support for SPI busses

2008-05-21 Thread David Brownell
On Friday 16 May 2008, Grant Likely wrote:
 In my mind; platform_data and the device tree are all about the same
 thing: representation.  In other words, how to describe the
 configuration of the hardware independent of the driver itself.

Platform_data isn't what I'd call independent of drivers.

The reason the data is there in the first place is that
the driver needs it ... and chose not to hard-wire it.


 One of the things I find rather interesting is just how frequently
 drivers using platform data structures have a big block of code which
 simply copy pdata fields into identically named fields in the device
 private data... 

... because platform data was designed as a partial template
for that driver, letting it do that.  (Sometimes without even
doing scale conversions.)  As drivers grow functionally, they
sometimes end up needing more platform data fields, to expose
data that previously didn't matter.

Whether that data can usefully be stored in flash (or ROM)
and handed out through the bootloader is something of a
manufacturing issue.

- Dave
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH 3/4] spi: Add OF binding support for SPI busses

2008-05-20 Thread Gary Jennejohn
On Mon, 19 May 2008 21:19:50 +0400
Anton Vorontsov [EMAIL PROTECTED] wrote:

 On Mon, May 19, 2008 at 07:09:00PM +0200, Gary Jennejohn wrote:
[snip extraneous content]
  My problem was, and is, that there's no generic GPIO support for powerpc.
  At least, not that I'm aware of.  Please tell me if I'm wrong.
 
 Documentation/powerpc/booting-without-of.txt
 VIII - Specifying GPIO information for devices.
 
 And include/linux/of_gpio.h + drivers/of/gpio.c.
 

Thanks! I'll look into using this.

---
Gary Jennejohn
*
DENX Software Engineering GmbH, MD: Wolfgang Denk  Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: [EMAIL PROTECTED]
*
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH 3/4] spi: Add OF binding support for SPI busses

2008-05-20 Thread Guennadi Liakhovetski
On Mon, 19 May 2008, Grant Likely wrote:

 I'm not so fond of this approach.  cs-parent doesn't seem to make much
 sense to me.  It might be better to have a cs-handler property on the
 SPI bus node instead of on the SPI slave nodes, but even then it
 leaves a number of questions about what it really means.  In some
 cases it would be overkill.  For example, if the SPI node simply had
 multiple GPIO lines then an extra cs-parent node wouldn't be needed at
 all.

Right, it is optional.

 Then there are the complex arrangements.  When setting CS
 requires inserting a special 'set cs' SPI message at the right time.
 Or worse; when setting CS requires /modifying/ the sent SPI message.

Hm, are there actually such SPI _controllers_ that use SPI data to toggle 
chipselects? I.e., you would have to send your SPI client data (for the 
RTC or whatever) plus some extra bytes or with some modifications, and 
this extra information would then be intercepted by the SPI _controller_ 
itself and only client data would be sent out? Isn't what you're 
describing really a case of an SPI bridge, as you also call it below? In 
which case, I think, it might make sense to cleanly differentiate these 
two cases:

1. SPI chipselect. Either controlled by an external (typically a GPIO) 
signal or by the controller itself, in the latter case the controller has 
to be configured with the required address

2. SPI bridge. I don't know such configurations, so, I can only guess: the 
SPI controller has a single SPI client, which acts like a bridge. It 
receives data from the primary host, and in this data the target client 
data and its address are encoded.

Now, I can also imagine case 2 where the bridge is actually a part of the 
host controller... Even though this doesn't make any sense to me.

 Essentially, the binding would need to describe the ability to
 completely intercept and rewrite all SPI messages going through the CS
 scheme.
 
 I'm not saying it's not possible to do, but I am saying that I'd like
 to have a better feel for all the use cases before it is defined.  I'm
 not convinced that adding a cs-parent phandle will do that
 appropriately.  That being said, my gut feel is that the solution will
 be to support spi-bridge nodes that handle the complex CS
 configuration settings; the spi-bridge would be a child of the
 spi-master and the parent of the spi devices; and simple CS settings
 being handled with regular old GPIO bindings.  (Much like the last
 suggestion you make; except that I think that it *does* looks
 elegant.)  :-)

Ok, elegance apart:-) You can use the SPI-bridge construct to also 
describe simple SPI-chipselect configurations. But is it really a good 
idea? Wouldn't it be better to handle these two cases separately? Using 
bridge to describe CS's seems also confusing - imagine someone 
implementing a new DTS, having to describe a bridge not having one doesn't 
seem very intuitive:-)

 example; here's an SPI bus that has 2 GPIOs for two bus CS lines and
 an SPI bridge that uses both CSes; one address for accessing the
 bridge's CS register and one CS to access the downstream devices.
 
 +SPI example for an MPC5200 SPI bus:
 +   [EMAIL PROTECTED] {
 +   #address-cells = 1;
 +   #size-cells = 0;
 +   compatible = fsl,mpc5200b-spi,fsl,mpc5200-spi;
 +   reg = 0xf00 0x20;
 +   interrupts = 2 13 0 2 14 0;
 +   interrupt-parent = mpc5200_pic;
 +   gpios = gpio1 0 0 gpio1 1 0;
 +   [EMAIL PROTECTED] {
 +   compatible = oem,spi-bridge-type;
 +   reg =  0 1 ;  // note: 2 SPI CS addresses; first 
 one to access bridge registers
 +
 +   [EMAIL PROTECTED] {
 +   compatible = micrel,ks8995m;
 +   linux,modalias = ks8995;
 +   max-speed = 100;
 +   reg = 0;
 +   };
 ... // and more SPI child nodes here...
 +   };
 +   };
 
 But even this doesn't reflect the hardware layout well.  What if the
 SS lines are on SPI GPIO expanders on the same bus?  Then does it make
 sense for them to be layed out as spi bridges?

Well, in this case - yes, because addressing clients behind the expander 
and the expander itself is done differently.

On the whole, I think, it begins to look good - I think, it is better to 
implement an imperfect but complete and consistent scheme and modify or 
extend it in the future, than a perfect, but incomplete, and have to use 
auxiliary means (platform bindings) to fill in the gaps.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH 3/4] spi: Add OF binding support for SPI busses

2008-05-19 Thread Guennadi Liakhovetski
On Fri, 16 May 2008, Grant Likely wrote:

 +However, the binding does not attempt to define the specific method for
 +assigning chip select numbers.  Since SPI chip select configuration is
 +flexible and non-standardized, it is left out of this binding with the
 +assumption that board specific platform code will be used to manage
 +chip selects.  Individual drivers can define additional properties to
 +support describing the chip select layout.

Yes, this looks like a problem to me. This means, SPI devices will need 
two bindings - OF and platform?... Maybe define an spi_chipselect 
OF-binding?

Thanks
Guennadi
---
Guennadi Liakhovetski
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH 3/4] spi: Add OF binding support for SPI busses

2008-05-19 Thread Grant Likely
On Mon, May 19, 2008 at 7:17 AM, Guennadi Liakhovetski
[EMAIL PROTECTED] wrote:
 On Fri, 16 May 2008, Grant Likely wrote:

 +However, the binding does not attempt to define the specific method for
 +assigning chip select numbers.  Since SPI chip select configuration is
 +flexible and non-standardized, it is left out of this binding with the
 +assumption that board specific platform code will be used to manage
 +chip selects.  Individual drivers can define additional properties to
 +support describing the chip select layout.

 Yes, this looks like a problem to me. This means, SPI devices will need
 two bindings - OF and platform?... Maybe define an spi_chipselect
 OF-binding?

Actually, spi devices have *neither*.  :-)  They bind to the SPI bus.
Not the platform bus or of_platform bus.  But that is Linux internal
details; this discussion is about device tree bindings.

Note that I did say that drivers can define additional properties for
supporting chip select changes as needed.  I'm just not attempting to
encode them into the formal binding.  There is simply just too many
different ways to manipulate chip select signals and so I don't feel
confident trying to define a *common* binding at this moment in time.
At some point in the future when we have a number of examples to
choose from then we can extend this binding with chip select related
properties.

As for the Linux internals, the 5200 SPI bus driver that I posted
exports a function that allows another driver to call in and
manipulated the CS lines before the transfer.  It isn't the prettiest
solution, but I'm not locked into the approach and that gives some
time to consider cleaner interfaces.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH 3/4] spi: Add OF binding support for SPI busses

2008-05-19 Thread Guennadi Liakhovetski
On Mon, 19 May 2008, Grant Likely wrote:

 On Mon, May 19, 2008 at 7:17 AM, Guennadi Liakhovetski
 [EMAIL PROTECTED] wrote:
  On Fri, 16 May 2008, Grant Likely wrote:
 
  +However, the binding does not attempt to define the specific method 
  for
  +assigning chip select numbers.  Since SPI chip select configuration is
  +flexible and non-standardized, it is left out of this binding with the
  +assumption that board specific platform code will be used to manage
  +chip selects.  Individual drivers can define additional properties to
  +support describing the chip select layout.
 
  Yes, this looks like a problem to me. This means, SPI devices will need
  two bindings - OF and platform?... Maybe define an spi_chipselect
  OF-binding?
 
 Actually, spi devices have *neither*.  :-)  They bind to the SPI bus.
 Not the platform bus or of_platform bus.

Right, sorry, your SPI bus driver scans the bus device bindings and 
registers devices on it using spi_of_register_devices().

 But that is Linux internal
 details; this discussion is about device tree bindings.
 
 Note that I did say that drivers can define additional properties for
 supporting chip select changes as needed.  I'm just not attempting to
 encode them into the formal binding.  There is simply just too many
 different ways to manipulate chip select signals and so I don't feel
 confident trying to define a *common* binding at this moment in time.

Yes, I understand, that physically there can be many ways SPI chipselects 
can be controlled. But I thought there could be a generic way to cover 
them all by defining a separate entry on your SPI bus. Like

+SPI example for an MPC5200 SPI bus:
+   [EMAIL PROTECTED] {
+   #address-cells = 1;
+   #size-cells = 0;
+   compatible = fsl,mpc5200b-spi,fsl,mpc5200-spi;
+   reg = 0xf00 0x20;
+   interrupts = 2 13 0 2 14 0;
+   interrupt-parent = mpc5200_pic;
+   [EMAIL PROTECTED] {
+   compatible = oem,cs-type;
+   };
+
+   [EMAIL PROTECTED] {
+   compatible = micrel,ks8995m;
+   linux,modalias = ks8995;
+   max-speed = 100;
+   reg = 0;
+   cs-parent = /.../[EMAIL PROTECTED]/[EMAIL 
PROTECTED];
+   };
...
+   };

Then whatever method is used to actually switch the CS, a driver should be 
registered to handle [EMAIL PROTECTED], providing the required calls. 
Without such a driver [EMAIL PROTECTED] will not probe successfully. 
Wouldn't this cover all possible cases? One could even consider actually 
putting SPI devices on SPI chipselect busses, but that won't look very 
elegant:-)

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH 3/4] spi: Add OF binding support for SPI busses

2008-05-19 Thread Gary Jennejohn
On Mon, 19 May 2008 09:57:21 -0600
Grant Likely [EMAIL PROTECTED] wrote:

 On Mon, May 19, 2008 at 7:17 AM, Guennadi Liakhovetski
 [EMAIL PROTECTED] wrote:
  On Fri, 16 May 2008, Grant Likely wrote:
 
  +However, the binding does not attempt to define the specific method 
  for
  +assigning chip select numbers.  Since SPI chip select configuration is
  +flexible and non-standardized, it is left out of this binding with the
  +assumption that board specific platform code will be used to manage
  +chip selects.  Individual drivers can define additional properties to
  +support describing the chip select layout.
 
  Yes, this looks like a problem to me. This means, SPI devices will need
  two bindings - OF and platform?... Maybe define an spi_chipselect
  OF-binding?
 
 Actually, spi devices have *neither*.  :-)  They bind to the SPI bus.
 Not the platform bus or of_platform bus.  But that is Linux internal
 details; this discussion is about device tree bindings.
 
 Note that I did say that drivers can define additional properties for
 supporting chip select changes as needed.  I'm just not attempting to
 encode them into the formal binding.  There is simply just too many
 different ways to manipulate chip select signals and so I don't feel
 confident trying to define a *common* binding at this moment in time.
 At some point in the future when we have a number of examples to
 choose from then we can extend this binding with chip select related
 properties.
 
 As for the Linux internals, the 5200 SPI bus driver that I posted
 exports a function that allows another driver to call in and
 manipulated the CS lines before the transfer.  It isn't the prettiest
 solution, but I'm not locked into the approach and that gives some
 time to consider cleaner interfaces.
 

I sort of hesitate to hijack this thread, but since we're discussing SPI
and chip selects...

I have a driver for the SPI controller in the 440EPx.  This controller
is very simple and does not have any internal support for a chip select.
The controller seems to also be in the 440GR and 440EP, and may be in
other AMCC CPUs too.

All chip selects must be done using GPIO.  In fact, the board for which
I developed this driver, a modified sequoia, actually uses 2 chip selects.

My problem was, and is, that there's no generic GPIO support for powerpc.
At least, not that I'm aware of.  Please tell me if I'm wrong.

So the driver has great gobs of GPIO code in it, most of which I took
from u-boot.  The code is pretty generic, but some 440EPx-specific
stuff may have crept in without my being aware of it.

My real question is - should this code be in a platform-specific file
such as sequoia.c, which could result in lots of duplicated code, or is
it better to leave it in the driver for now until some day we hopefully
get generic GPIO support for powerpc?

I want to get this driver upstream ASAP.

---
Gary Jennejohn
*
DENX Software Engineering GmbH, MD: Wolfgang Denk  Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: [EMAIL PROTECTED]
*
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH 3/4] spi: Add OF binding support for SPI busses

2008-05-19 Thread Anton Vorontsov
On Mon, May 19, 2008 at 07:09:00PM +0200, Gary Jennejohn wrote:
 On Mon, 19 May 2008 09:57:21 -0600
 Grant Likely [EMAIL PROTECTED] wrote:
 
  On Mon, May 19, 2008 at 7:17 AM, Guennadi Liakhovetski
  [EMAIL PROTECTED] wrote:
   On Fri, 16 May 2008, Grant Likely wrote:
  
   +However, the binding does not attempt to define the specific method 
   for
   +assigning chip select numbers.  Since SPI chip select configuration 
   is
   +flexible and non-standardized, it is left out of this binding with 
   the
   +assumption that board specific platform code will be used to manage
   +chip selects.  Individual drivers can define additional properties 
   to
   +support describing the chip select layout.
  
   Yes, this looks like a problem to me. This means, SPI devices will need
   two bindings - OF and platform?... Maybe define an spi_chipselect
   OF-binding?
  
  Actually, spi devices have *neither*.  :-)  They bind to the SPI bus.
  Not the platform bus or of_platform bus.  But that is Linux internal
  details; this discussion is about device tree bindings.
  
  Note that I did say that drivers can define additional properties for
  supporting chip select changes as needed.  I'm just not attempting to
  encode them into the formal binding.  There is simply just too many
  different ways to manipulate chip select signals and so I don't feel
  confident trying to define a *common* binding at this moment in time.
  At some point in the future when we have a number of examples to
  choose from then we can extend this binding with chip select related
  properties.
  
  As for the Linux internals, the 5200 SPI bus driver that I posted
  exports a function that allows another driver to call in and
  manipulated the CS lines before the transfer.  It isn't the prettiest
  solution, but I'm not locked into the approach and that gives some
  time to consider cleaner interfaces.
  
 
 I sort of hesitate to hijack this thread, but since we're discussing SPI
 and chip selects...
 
 I have a driver for the SPI controller in the 440EPx.  This controller
 is very simple and does not have any internal support for a chip select.
 The controller seems to also be in the 440GR and 440EP, and may be in
 other AMCC CPUs too.
 
 All chip selects must be done using GPIO.  In fact, the board for which
 I developed this driver, a modified sequoia, actually uses 2 chip selects.
 
 My problem was, and is, that there's no generic GPIO support for powerpc.
 At least, not that I'm aware of.  Please tell me if I'm wrong.

Documentation/powerpc/booting-without-of.txt
VIII - Specifying GPIO information for devices.

And include/linux/of_gpio.h + drivers/of/gpio.c.

Soon I'll post some patches for mpc83xx_spi showing how to use GPIOs
for the SPI chip selects.

-- 
Anton Vorontsov
email: [EMAIL PROTECTED]
irc://irc.freenode.net/bd2
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH 3/4] spi: Add OF binding support for SPI busses

2008-05-19 Thread Grant Likely
On Mon, May 19, 2008 at 10:30 AM, Guennadi Liakhovetski
[EMAIL PROTECTED] wrote:
 On Mon, 19 May 2008, Grant Likely wrote:
 But that is Linux internal
 details; this discussion is about device tree bindings.

 Note that I did say that drivers can define additional properties for
 supporting chip select changes as needed.  I'm just not attempting to
 encode them into the formal binding.  There is simply just too many
 different ways to manipulate chip select signals and so I don't feel
 confident trying to define a *common* binding at this moment in time.

 Yes, I understand, that physically there can be many ways SPI chipselects
 can be controlled. But I thought there could be a generic way to cover
 them all by defining a separate entry on your SPI bus. Like

 +SPI example for an MPC5200 SPI bus:
 +   [EMAIL PROTECTED] {
 +   #address-cells = 1;
 +   #size-cells = 0;
 +   compatible = fsl,mpc5200b-spi,fsl,mpc5200-spi;
 +   reg = 0xf00 0x20;
 +   interrupts = 2 13 0 2 14 0;
 +   interrupt-parent = mpc5200_pic;
 +   [EMAIL PROTECTED] {
 +   compatible = oem,cs-type;
 +   };
 +
 +   [EMAIL PROTECTED] {
 +   compatible = micrel,ks8995m;
 +   linux,modalias = ks8995;
 +   max-speed = 100;
 +   reg = 0;
 +   cs-parent = /.../[EMAIL PROTECTED]/[EMAIL 
 PROTECTED];
 +   };
 ...
 +   };

 Then whatever method is used to actually switch the CS, a driver should be
 registered to handle [EMAIL PROTECTED], providing the required calls.
 Without such a driver [EMAIL PROTECTED] will not probe successfully.
 Wouldn't this cover all possible cases? One could even consider actually
 putting SPI devices on SPI chipselect busses, but that won't look very
 elegant:-)

Hurr...

I'm not so fond of this approach.  cs-parent doesn't seem to make much
sense to me.  It might be better to have a cs-handler property on the
SPI bus node instead of on the SPI slave nodes, but even then it
leaves a number of questions about what it really means.  In some
cases it would be overkill.  For example, if the SPI node simply had
multiple GPIO lines then an extra cs-parent node wouldn't be needed at
all.  Then there are the complex arrangements.  When setting CS
requires inserting a special 'set cs' SPI message at the right time.
Or worse; when setting CS requires /modifying/ the sent SPI message.
Essentially, the binding would need to describe the ability to
completely intercept and rewrite all SPI messages going through the CS
scheme.

I'm not saying it's not possible to do, but I am saying that I'd like
to have a better feel for all the use cases before it is defined.  I'm
not convinced that adding a cs-parent phandle will do that
appropriately.  That being said, my gut feel is that the solution will
be to support spi-bridge nodes that handle the complex CS
configuration settings; the spi-bridge would be a child of the
spi-master and the parent of the spi devices; and simple CS settings
being handled with regular old GPIO bindings.  (Much like the last
suggestion you make; except that I think that it *does* looks
elegant.)  :-)

example; here's an SPI bus that has 2 GPIOs for two bus CS lines and
an SPI bridge that uses both CSes; one address for accessing the
bridge's CS register and one CS to access the downstream devices.

+SPI example for an MPC5200 SPI bus:
+   [EMAIL PROTECTED] {
+   #address-cells = 1;
+   #size-cells = 0;
+   compatible = fsl,mpc5200b-spi,fsl,mpc5200-spi;
+   reg = 0xf00 0x20;
+   interrupts = 2 13 0 2 14 0;
+   interrupt-parent = mpc5200_pic;
+   gpios = gpio1 0 0 gpio1 1 0;
+   [EMAIL PROTECTED] {
+   compatible = oem,spi-bridge-type;
+   reg =  0 1 ;  // note: 2 SPI CS addresses;
first one to access bridge registers
+
+   [EMAIL PROTECTED] {
+   compatible = micrel,ks8995m;
+   linux,modalias = ks8995;
+   max-speed = 100;
+   reg = 0;
+   };
... // and more SPI child nodes here...
+   };
+   };

But even this doesn't reflect the hardware layout well.  What if the
SS lines are on SPI GPIO expanders on the same bus?  Then does it make
sense for them to be layed out as spi bridges?

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org

Re: [PATCH 3/4] spi: Add OF binding support for SPI busses

2008-05-16 Thread Grant Likely
On Fri, May 16, 2008 at 2:47 PM, Randy Dunlap [EMAIL PROTECTED] wrote:
 On Fri, 16 May 2008 13:36:13 -0600 Grant Likely wrote:

 diff --git a/Documentation/powerpc/booting-without-of.txt 
 b/Documentation/powerpc/booting-without-of.txt
 index 1d2a772..452c242 100644
 --- a/Documentation/powerpc/booting-without-of.txt
 +++ b/Documentation/powerpc/booting-without-of.txt
 @@ -2870,6 +2871,66 @@ platforms are moved over to use the 
 flattened-device-tree model.
   reg = 0xe800 32;
   };

 +s) SPI (Serial Peripheral Interface) busses
 +
 +SPI busses can be described with a node for the SPI master device
 +and a set of child nodes for each SPI slave on the bus.  For this
 +discussion, it is assumed that the system's SPI controller is in
 +SPI master mode.  This binding does not describe SPI controllers
 +in slave mode.
 +
 +The SPI master node requires the following properties:
 +- #address-cells  - number of cells required to define a chip select
 + address on the SPI bus.
 +- #size-cells - should be zero.
 +- compatible  - name of SPI bus controller following generic names
 + recommended practice.
 +No other properties are required in the spi bus node.  It is assumed
   ~~~

 +that a driver for an SPI bus device will understand that it is an SPI 
 bus.
 +However, the binding does not attempt to define the specific method for
 +assigning chip select numbers.  Since SPI chip select configuration is
 +flexible and non-standardized, it is left out of this binding with the
 +assumption that board specific platform code will be used to manage
 +chip selects.  Individual drivers can define additional properties to
 +support describing the chip select layout.
 +
 +SPI slave nodes must be children of the spi master node and can
   ~~~

 +contain the following properties.
 +- reg - (required) chip select address of device.
 +- compatible  - (required) name of SPI device following generic 
 names
 + recommended practice
 +- max-speed   - (optional) Maximum SPI clocking speed of device in 
 Hz
 +- spi,cpol- (optional) Device requires inverse clock polarity
 +- spi,cpha- (optional) Device requires shifted clock phase
 +- linux,modalias  - (optional, Linux specific) Force binding of SPI 
 device
 + to a particular spi_device driver.  Useful for changing
 + driver binding between spidev and a kernel spi driver.
   ~~~

 Hi,
 You mostly capitalize SPI in sentences (i.e., when it's not part of
 a function name or OF data), so could the 3 underlined instances of it
 also be all caps?

No problem.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH 3/4] spi: Add OF binding support for SPI busses

2008-05-16 Thread Randy Dunlap
On Fri, 16 May 2008 13:36:13 -0600 Grant Likely wrote:

 diff --git a/Documentation/powerpc/booting-without-of.txt 
 b/Documentation/powerpc/booting-without-of.txt
 index 1d2a772..452c242 100644
 --- a/Documentation/powerpc/booting-without-of.txt
 +++ b/Documentation/powerpc/booting-without-of.txt
 @@ -2870,6 +2871,66 @@ platforms are moved over to use the 
 flattened-device-tree model.
   reg = 0xe800 32;
   };
  
 +s) SPI (Serial Peripheral Interface) busses
 +
 +SPI busses can be described with a node for the SPI master device
 +and a set of child nodes for each SPI slave on the bus.  For this
 +discussion, it is assumed that the system's SPI controller is in
 +SPI master mode.  This binding does not describe SPI controllers
 +in slave mode.
 +
 +The SPI master node requires the following properties:
 +- #address-cells  - number of cells required to define a chip select
 + address on the SPI bus.
 +- #size-cells - should be zero.
 +- compatible  - name of SPI bus controller following generic names
 + recommended practice.
 +No other properties are required in the spi bus node.  It is assumed
   ~~~

 +that a driver for an SPI bus device will understand that it is an SPI 
 bus.
 +However, the binding does not attempt to define the specific method for
 +assigning chip select numbers.  Since SPI chip select configuration is
 +flexible and non-standardized, it is left out of this binding with the
 +assumption that board specific platform code will be used to manage
 +chip selects.  Individual drivers can define additional properties to
 +support describing the chip select layout.
 +
 +SPI slave nodes must be children of the spi master node and can
   ~~~

 +contain the following properties.
 +- reg - (required) chip select address of device.
 +- compatible  - (required) name of SPI device following generic names
 + recommended practice
 +- max-speed   - (optional) Maximum SPI clocking speed of device in Hz
 +- spi,cpol- (optional) Device requires inverse clock polarity
 +- spi,cpha- (optional) Device requires shifted clock phase
 +- linux,modalias  - (optional, Linux specific) Force binding of SPI 
 device
 + to a particular spi_device driver.  Useful for changing
 + driver binding between spidev and a kernel spi driver.
   ~~~

Hi,
You mostly capitalize SPI in sentences (i.e., when it's not part of
a function name or OF data), so could the 3 underlined instances of it
also be all caps?

Thanks,
---
~Randy
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH 3/4] spi: Add OF binding support for SPI busses

2008-05-16 Thread Anton Vorontsov
On Fri, May 16, 2008 at 01:36:13PM -0600, Grant Likely wrote:
 From: Grant Likely [EMAIL PROTECTED]
 
 This patch adds support for populating an SPI bus based on data in the
 OF device tree.  This is useful for powerpc platforms which use the
 device tree instead of discrete code for describing platform layout.
 
 Signed-off-by: Grant Likely [EMAIL PROTECTED]
 ---
[...]
 +void spi_of_register_devices(struct spi_master *master, struct device_node 
 *np)
 +{
 + struct spi_device *spi;
 + struct device_node *nc;
 + const u32 *prop;
 + const char *sprop;
 + int rc;
 + int len;
 +
 + for_each_child_of_node(np, nc) {
 + /* Alloc an spi_device */
 + spi = spi_alloc_device(master);
 + if (!spi) {
 + dev_err(master-dev, spi_device alloc error for %s\n,
 + np-full_name);
 + continue;
 + }
 +
 + /* Device address */
 + prop = of_get_property(nc, reg, len);
 + if (!prop || len  sizeof(*prop)) {
 + dev_err(master-dev, %s has no 'reg' property\n,
 + np-full_name);
 + continue;
 + }
 + spi-chip_select = *prop;
 +
 + /* Mode (clock phase/polarity/etc. */
 + if (of_find_property(nc, spi,cpha, NULL))
 + spi-mode |= SPI_CPHA;
 + if (of_find_property(nc, spi,cpol, NULL))
 + spi-mode |= SPI_CPOL;
 +
 + /* Device speed */
 + prop = of_get_property(nc, max-speed, len);
 + if (prop  len = sizeof(*prop))
 + spi-max_speed_hz = *prop;
 + else
 + spi-max_speed_hz = 10;
 +
 + /* IRQ */
 + spi-irq = irq_of_parse_and_map(nc, 0);
 +
 + /* Select device driver */
 + sprop = of_get_property(nc, linux,modalias, len);
 + if (sprop  len  0)
 + strncpy(spi-modalias, sprop, KOBJ_NAME_LEN);
 + else
 + strncpy(spi-modalias, spidev, KOBJ_NAME_LEN);
 +
 + /* Store a pointer to the node in the device structure */
 + of_node_get(nc);
 + spi-dev.archdata.of_node = nc;
 +
 + /* Register the new device */
 + rc = spi_register_device(spi);
 + if (rc) {
 + dev_err(master-dev, spi_device register error %s\n,
 + np-full_name);
 + spi_device_release(spi);
 + }

No way to pass platform data... can you suggest any idea to use
this for things like
[POWERPC] 86xx: mpc8610_hpcd: add support for SPI and MMC-over-SPI
I've sent just recently...?

Maybe this code could do something like
spi-dev.platform_data = nc-data;
and board code would fill nc-data at early stages? This needs to be a
convention, not just random use though.. Maybe we can expand the struct
device_node to explicitly include .platform_data for such cases?

Thanks,

-- 
Anton Vorontsov
email: [EMAIL PROTECTED]
irc://irc.freenode.net/bd2
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH 3/4] spi: Add OF binding support for SPI busses

2008-05-16 Thread Grant Likely
On Fri, May 16, 2008 at 4:03 PM, Anton Vorontsov [EMAIL PROTECTED] wrote:
 On Fri, May 16, 2008 at 01:36:13PM -0600, Grant Likely wrote:
 + /* Store a pointer to the node in the device structure */
 + of_node_get(nc);
 + spi-dev.archdata.of_node = nc;
 +
 + /* Register the new device */
 + rc = spi_register_device(spi);
 + if (rc) {
 + dev_err(master-dev, spi_device register error %s\n,
 + np-full_name);
 + spi_device_release(spi);
 + }

 No way to pass platform data... can you suggest any idea to use
 this for things like
 [POWERPC] 86xx: mpc8610_hpcd: add support for SPI and MMC-over-SPI
 I've sent just recently...?

That's right.  platform_data being a very driver specific thing there
is no way to generically extract a pdata structure from the device
tree.  Instead, I'm storing the device node in archdata.of_node (line
immediately above spi_register_device) so that drivers can read the
device node themselves to populate a platform_device structure.
(Protected by CONFIG_OF of course).

 Maybe this code could do something like
 spi-dev.platform_data = nc-data;
 and board code would fill nc-data at early stages? This needs to be a
 convention, not just random use though.. Maybe we can expand the struct
 device_node to explicitly include .platform_data for such cases?

Hmmm, as you say, this could end up being rather messy.  However, by
passing the device node pointer, the driver could extract that data on
a per case basis.  (ie. it would be decided on a per driver basis
where to get the platform data).  I'm not sure; this bears more
thought...

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH 3/4] spi: Add OF binding support for SPI busses

2008-05-16 Thread Anton Vorontsov
On Fri, May 16, 2008 at 04:14:23PM -0600, Grant Likely wrote:
 On Fri, May 16, 2008 at 4:03 PM, Anton Vorontsov [EMAIL PROTECTED] wrote:
  On Fri, May 16, 2008 at 01:36:13PM -0600, Grant Likely wrote:
  + /* Store a pointer to the node in the device structure */
  + of_node_get(nc);
  + spi-dev.archdata.of_node = nc;
  +
  + /* Register the new device */
  + rc = spi_register_device(spi);
  + if (rc) {
  + dev_err(master-dev, spi_device register error 
  %s\n,
  + np-full_name);
  + spi_device_release(spi);
  + }
 
  No way to pass platform data... can you suggest any idea to use
  this for things like
  [POWERPC] 86xx: mpc8610_hpcd: add support for SPI and MMC-over-SPI
  I've sent just recently...?
 
 That's right.  platform_data being a very driver specific thing there
 is no way to generically extract a pdata structure from the device
 tree.  Instead, I'm storing the device node in archdata.of_node (line
 immediately above spi_register_device) so that drivers can read the
 device node themselves to populate a platform_device structure.
 (Protected by CONFIG_OF of course).
 
  Maybe this code could do something like
  spi-dev.platform_data = nc-data;
  and board code would fill nc-data at early stages? This needs to be a
  convention, not just random use though.. Maybe we can expand the struct
  device_node to explicitly include .platform_data for such cases?
 
 Hmmm, as you say, this could end up being rather messy.  However, by
 passing the device node pointer, the driver could extract that data on
 a per case basis.  (ie. it would be decided on a per driver basis
 where to get the platform data).  I'm not sure; this bears more
 thought...

Sometimes it's not worth powder and shot adding OF functionality to
the drivers, I2C and SPI are major examples. Another [not mmc_spi]
example is drivers/input/touchscreen/ads7846.c, which is SPI driver
and needs platform data. There is a board that needs this (touchscreen
controller on a MPC8360E-RDK).

Also there is no way to pass functions via device tree, we're
always end up doing board-specific hooks in the generic drivers...

Finally, let's call this platform_data and be done with it. Then we
can use this for things like drivers/video/fsl-diu-fb.c (see diu_ops,
which is global struct, filled by
arch/powerpc/platforms/86xx/mpc8610_hpcd.c).

-- 
Anton Vorontsov
email: [EMAIL PROTECTED]
irc://irc.freenode.net/bd2
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH 3/4] spi: Add OF binding support for SPI busses

2008-05-16 Thread Grant Likely
On Fri, May 16, 2008 at 4:49 PM, Anton Vorontsov [EMAIL PROTECTED] wrote:
 On Fri, May 16, 2008 at 04:14:23PM -0600, Grant Likely wrote:
  Maybe this code could do something like
  spi-dev.platform_data = nc-data;
  and board code would fill nc-data at early stages? This needs to be a
  convention, not just random use though.. Maybe we can expand the struct
  device_node to explicitly include .platform_data for such cases?

 Hmmm, as you say, this could end up being rather messy.  However, by
 passing the device node pointer, the driver could extract that data on
 a per case basis.  (ie. it would be decided on a per driver basis
 where to get the platform data).  I'm not sure; this bears more
 thought...

 Sometimes it's not worth powder and shot adding OF functionality to
 the drivers, I2C and SPI are major examples. Another [not mmc_spi]
 example is drivers/input/touchscreen/ads7846.c, which is SPI driver
 and needs platform data. There is a board that needs this (touchscreen
 controller on a MPC8360E-RDK).

In my mind; platform_data and the device tree are all about the same
thing: representation.  In other words, how to describe the
configuration of the hardware independent of the driver itself.  The
device tree and platform data structures both solve the same problem.
In both cases, the representation data must be
translated/decoded/interpreted and stored in the drivers own private
data structure so it can be of use.

One of the things I find rather interesting is just how frequently
drivers using platform data structures have a big block of code which
simply copy pdata fields into identically named fields in the device
private data... specifically: copied discretely instead of being a
verbatim block that can be memcopied, or instead of maintaining a
pointer and using the pdata itself.  It highlights for me just how
much pdata structures are decoupled from the driver itself (just like
how the device tree data is decoupled from the driver)... but I
digress.

The point is that the translation of data from the device tree (and
from pdata for that matter) to a form usable by the driver has to live
*somewhere*.  Does it belong in the platform code?  Or should it live
with the driver itself?  I argue that it really belongs as much as
feasibly possible with the driver code.  Even if a pdata structure is
chosen to be used as an intermediary representation, the code is only
relevant to the driver and therefore shouldn't appear anywhere else in
the kernel tree.  Putting it with the driver also has the added
advantage that it can be lumped in with the driver module and
therefore will only get compiled into the kernel if the driver is
present.  Putting driver specific (not platform specific) translation
code anywhere other than with the device driver it is intended for is
just wrong.

In addition, I'd really like to avoid a situation where the same block
of translation code (or at least calls to it) is duplicated all over
the platform code directories.  It's that sort of duplication that the
device tree (and similar schemes) is intended to solve.  I agree that
using platform code is often the best solution, especially when
dealing with one-off board ports that won't appear anywhere else.
However, I strongly believe that the platform code approach should be
the exception, not the rule.  If it is a common data property that all
instantiations of the device must have, then encode it in the device
tree and be done with it.  Doing so keeps platform code straight
forward and reduces duplication.

 Also there is no way to pass functions via device tree, we're
 always end up doing board-specific hooks in the generic drivers...

 Finally, let's call this platform_data and be done with it. Then we
 can use this for things like drivers/video/fsl-diu-fb.c (see diu_ops,
 which is global struct, filled by
 arch/powerpc/platforms/86xx/mpc8610_hpcd.c).

Yes, I agree, there are always going to be platform/board specific
hooks and I'm not saying that we should try to eliminate them.  But
(as expressed in the argument above) I don't like the idea of making
the platform code fill in all the necessary pdata structures.  How
about this as an alternative:

Instead of allowing platform code to fill in platform_data pointers at
early platform setup, let it register a driver-specific callback hook
instead.  If the hook it populated, the driver will call it at an
appropriate time to allow the platform code to manipulate the driver
configuration.  The signature of the hook can be driver dependent
(just like how the pdata hook is platform dependent).  Doing this
ensure that the translation code stays where it belongs: with the
driver itself, and it defers execution of the code to a point to
driver initialization time instead of earlier in the platform setup
which should improve boot times in certain circumstances if the
drivers are loaded as modules.

As for adding OF support to the drivers not worth powder and shot; I
must disagree.