From: Anton Vorontsov <hidden> Date: 2007-07-26 13:58:00
mmc_spi already tested to work. When it will hit mainline
the only change that would be needed is replacing "spidev"
by "mmc_spi", and adding trivial platform data to mmc_spi
driver.
Signed-off-by: Anton Vorontsov <redacted>
---
arch/powerpc/boot/dts/mpc832x_rdb.dts | 33 ++++++++++++++++++++-
arch/powerpc/platforms/83xx/mpc832x_rdb.c | 46 +++++++++++++++++++++++++++++
2 files changed, 78 insertions(+), 1 deletions(-)
-----Original Message-----
From:
linuxppc-dev-bounces+joakim.tjernlund=transmode.se@ozlabs.org
[mailto:linuxppc-dev-bounces+joakim.tjernlund=transmode.se@ozl
abs.org] On Behalf Of Anton Vorontsov
Sent: den 26 juli 2007 15:58
To: linuxppc-dev@ozlabs.org
Subject: [RFC][PATCH] MPC832x_RDB: update dts to use spi,
register mmc_spi stub
mmc_spi already tested to work. When it will hit mainline
the only change that would be needed is replacing "spidev"
by "mmc_spi", and adding trivial platform data to mmc_spi
driver.
Signed-off-by: Anton Vorontsov <redacted>
---
arch/powerpc/boot/dts/mpc832x_rdb.dts | 33
++++++++++++++++++++-
arch/powerpc/platforms/83xx/mpc832x_rdb.c | 46
+++++++++++++++++++++++++++++
2 files changed, 78 insertions(+), 1 deletions(-)
The driver is still using "cpu" mode. It just happens that the cpu mode for QE
enabled CPUs isn't 100% compatible with the defacto cpu mode used by other CPU:s.
So mode should probably be "cpu_qe" or, even better, if the drver could
autodetect that it is running on a QE enabled CPU and adjust accordinly.
Maybe the Freescale guys have some ideas how to do that.
Jocke
PS.
Will be on vacation for a week as of tmw, won't have access
during this time.
The driver is still using "cpu" mode. It just happens that the cpu mode for QE
enabled CPUs isn't 100% compatible with the defacto cpu mode used by other CPU:s.
Yup, indeed.
So mode should probably be "cpu_qe" or, even better, if the drver could
That's easy to change, thanks.
autodetect that it is running on a QE enabled CPU and adjust accordinly.
Maybe the Freescale guys have some ideas how to do that.
Ok. Well, for now could we use machine_is() for kind of autodetect?
Or it's better leave it as is, and just rename mode to cpu_qe?
Jocke
PS.
Will be on vacation for a week as of tmw, won't have access
during this time.
Thanks,
--
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2
-----Original Message-----
From: Anton Vorontsov [mailto:avorontsov@ru.mvista.com]
Sent: den 26 juli 2007 17:48
To: Joakim Tjernlund
Cc: linuxppc-dev@ozlabs.org
Subject: Re: [RFC][PATCH] MPC832x_RDB: update dts to use spi,
registermmc_spi stub
On Thu, Jul 26, 2007 at 05:36:06PM +0200, Joakim Tjernlund wrote:
[...]
From: Kumar Gala <hidden> Date: 2007-07-27 07:54:29
On Jul 26, 2007, at 2:40 PM, Joakim Tjernlund wrote:
quoted
-----Original Message-----
From: Anton Vorontsov [mailto:avorontsov@ru.mvista.com]
Sent: den 26 juli 2007 17:48
To: Joakim Tjernlund
Cc: linuxppc-dev@ozlabs.org
Subject: Re: [RFC][PATCH] MPC832x_RDB: update dts to use spi,
registermmc_spi stub
On Thu, Jul 26, 2007 at 05:36:06PM +0200, Joakim Tjernlund wrote:
[...]
From: Kumar Gala <hidden> Date: 2007-07-27 08:13:11
On Jul 26, 2007, at 8:57 AM, Anton Vorontsov wrote:
quoted hunk
mmc_spi already tested to work. When it will hit mainline
the only change that would be needed is replacing "spidev"
by "mmc_spi", and adding trivial platform data to mmc_spi
driver.
Signed-off-by: Anton Vorontsov <redacted>
---
arch/powerpc/boot/dts/mpc832x_rdb.dts | 33 ++++++++++++++++++
++-
arch/powerpc/platforms/83xx/mpc832x_rdb.c | 46 ++++++++++++++++++
+++++++++++
2 files changed, 78 insertions(+), 1 deletions(-)
Can we just use the reg value for bus_num in the kernel.
Sure, technically nothing prevents this. But, QE specs names
SPIs by these ids. Plus, from the kernel side spi name will be
not pretty, it will be spi1216.1. Reg value making little sense
to the userspace (or kernel-side SPI layer). Still want get
rid of device-id?
we don't need this in the spi node, its just the system clock frequency
which we can get other ways.
Will try, I guess "bus-frequency" property of soc8323 is what I need.
quoted
+ max-chipselect = <1>;
I'm not sure how I feel about this in here, I'm thinking it should go.
It's board-specific, i.e. how much chips connected to this SPI bus.
SPI layer needs this. Otherwise I have to pass it from board file,
but isn't it fits nicely in the DT?
we should do this in board code and not the device tree.
Well, I've done this initially. But Vitaly hinted that this could
be done in the DT instead, which made sense to me - mmc is the child
device of SPI bus. Why do you think it shouldn't be in the DT? I'm
not arguing, just want understand this.
Thanks!
--
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2
Can we just use the reg value for bus_num in the kernel.
Sure, technically nothing prevents this. But, QE specs names
SPIs by these ids. Plus, from the kernel side spi name will be
not pretty, it will be spi1216.1. Reg value making little sense
to the userspace (or kernel-side SPI layer). Still want get
rid of device-id?
we don't need this in the spi node, its just the system clock
frequency
which we can get other ways.
Will try, I guess "bus-frequency" property of soc8323 is what I need.
quoted
quoted
+ max-chipselect = <1>;
I'm not sure how I feel about this in here, I'm thinking it should
go.
It's board-specific, i.e. how much chips connected to this SPI bus.
SPI layer needs this. Otherwise I have to pass it from board file,
but isn't it fits nicely in the DT?
we should do this in board code and not the device tree.
Well, I've done this initially. But Vitaly hinted that this could
be done in the DT instead, which made sense to me - mmc is the child
device of SPI bus. Why do you think it shouldn't be in the DT? I'm
not arguing, just want understand this.
I understand. Look at the dts/device tree from a non-Linux
perspective. From that point of view its not always clear how one
would interpret some of the fields or if they are necessary. I get
very concerned when we start trying to describe any devices connect
to a bus when the bus doesn't have any specific for things that are
just software convention for a given OS.
The "id" concept and number is from Linux and another OS may have
come up w/some other way of describe this.
- k
From: Scott Wood <hidden> Date: 2007-07-27 16:40:57
On Fri, Jul 27, 2007 at 03:45:59PM +0400, Anton Vorontsov wrote:
Sure, technically nothing prevents this. But, QE specs names
SPIs by these ids. Plus, from the kernel side spi name will be
not pretty, it will be spi1216.1. Reg value making little sense
to the userspace (or kernel-side SPI layer). Still want get
rid of device-id?
Naming devices for human consumption is a more general problem than
device-id addresses; I'd like to see a standard "label" property added to
the device tree spec that software could use to present a human-friendly
label that corresponds to markings on the case, position on the board,
etc.
-Scott
Unless the DTS guys thinks otherwise, just rename to "cpu_qe".
"qe" would then be reserved for true QE mode.
If I understand it correctly, this is a property of how the
driver works, not how the hardware works / is connected; so
there should be _no_ such property in the device tree.
If I understand that wrong :-), just keep mode = "cpu", but
add an extra (empty) property representing that you need some
quirky mode special thing.
Either way, don't use underscores.
Segher
Naming devices for human consumption is a more general problem than
device-id addresses; I'd like to see a standard "label" property added
to
the device tree spec that software could use to present a
human-friendly
label that corresponds to markings on the case, position on the board,
etc.
For some buses, there is a "slot-names" property; some (non-core)
bindings seem to define a "location" property.
For "random" human-readable labelling, i.e. not corresponding to
physical markings on the hardware, I recommend you look for a
matching entry in /aliases. It won't ever be _exactly_ what you
want though, the Linux device namespace is separate from the
device tree.
Segher
Can we just use the reg value for bus_num in the kernel.
Sure, technically nothing prevents this. But, QE specs names
SPIs by these ids.
As a minimum the property name should start with "fsl," then.
Plus, from the kernel side spi name will be
not pretty, it will be spi1216.1.
What, the kernel cannot implement a counter itself?
quoted
quoted
+ max-chipselect = <1>;
I'm not sure how I feel about this in here, I'm thinking it should go.
It's board-specific, i.e. how much chips connected to this SPI bus.
It is misnamed then. It should be automatically derived from
the child nodes, though.
quoted
quoted
+ mmc@01 {
@01 should be @1. Except that it is wrong, since there is
no "reg" property.
quoted
quoted
+ device_type = "mmc";
No device_type please.
quoted
quoted
+ compatible = "mmc-spi";
Needs to be more specific.
quoted
quoted
+ device-id = <1>;
Get rid of this.
quoted
quoted
+ max-speed-hz = <bebc20>; /* 12500000 Hz */
Just max-speed.
quoted
quoted
+ chip-select = <0>;
This should be named "reg". And the parent needs #address-cells
and #size-cells properties.
quoted
quoted
+ pio-handle = <&mmc1pio>;
What is this for?
quoted
we should do this in board code and not the device tree.
Well, I've done this initially. But Vitaly hinted that this could
be done in the DT instead, which made sense to me - mmc is the child
device of SPI bus. Why do you think it shouldn't be in the DT? I'm
not arguing, just want understand this.
The hardware should be described in the device tree. This isn't
the same as simply copying all your Linux code into it ;-)
Segher
From: Anton Vorontsov <hidden> Date: 2007-08-01 12:35:06
On Wed, Aug 01, 2007 at 12:10:28AM +0200, Segher Boessenkool wrote:
quoted
+ spi1pio:spi_pin@01 {
There should be whitespace after the label. @01 should be
spelled @1. Except there is no "reg" property.
Hm. I've just tried to keep original style in this particular dts.
Want to ack patch below?
What is prefered style of <&label> vs. < &label > usage, btw?
arch/powerpc/boot/dts$ grep "<&" -r . | wc -l
327
arch/powerpc/boot/dts$ grep "< &" -r . | wc -l
92
I can only guess - the first?
What is this
stuff, anyway?
Which one? pio-map for spi? This is GPIO pins configuration, to use
dedicated functions (SPI) for these pins, otherwise SPI will not work.
From: Anton Vorontsov <hidden> Date: 2007-08-01 13:30:51
On Wed, Aug 01, 2007 at 12:06:46AM +0200, Segher Boessenkool wrote:
quoted
quoted
quoted
spi@4c0 {
device_type = "spi";
+ device-id = <1>;
Can we just use the reg value for bus_num in the kernel.
Sure, technically nothing prevents this. But, QE specs names
SPIs by these ids.
As a minimum the property name should start with "fsl," then.
fsl,device-id = <1>;, correct?
quoted
Plus, from the kernel side spi name will be
not pretty, it will be spi1216.1.
What, the kernel cannot implement a counter itself?
Just counter is especially meaningless and confusing. It will
work in that particular case, though. But then SPI bus number will
depend on definition order in the dts file. This isn't how SPI
bus numbers should be assigned. SPI bus numbers taken from specs,
this is how people know which SPI is which.
quoted
quoted
quoted
+ max-chipselect = <1>;
I'm not sure how I feel about this in here, I'm thinking it should go.
It's board-specific, i.e. how much chips connected to this SPI bus.
It is misnamed then. It should be automatically derived from
the child nodes, though.
quoted
quoted
quoted
+ mmc@01 {
@01 should be @1. Except that it is wrong, since there is
no "reg" property.
quoted
quoted
quoted
+ device_type = "mmc";
No device_type please.
quoted
quoted
quoted
+ compatible = "mmc-spi";
Needs to be more specific.
Um.. for example? I can't imagine anything specific for this. ;-)
quoted
quoted
quoted
+ device-id = <1>;
Get rid of this.
quoted
quoted
quoted
+ max-speed-hz = <bebc20>; /* 12500000 Hz */
Just max-speed.
quoted
quoted
quoted
+ chip-select = <0>;
This should be named "reg". And the parent needs #address-cells
and #size-cells properties.
quoted
quoted
quoted
+ pio-handle = <&mmc1pio>;
What is this for?
To set up output function of GPIO pin for MMC chip select.
And well, I've just looked into par_io_of_config(), and I've found
that pio-handle is mandatory (obviously), and thus let's back to:
quoted
quoted
we should do this in board code and not the device tree.
Well, I've done this initially. But Vitaly hinted that this could
be done in the DT instead, which made sense to me - mmc is the child
device of SPI bus. Why do you think it shouldn't be in the DT? I'm
not arguing, just want understand this.
The hardware should be described in the device tree. This isn't
the same as simply copying all your Linux code into it ;-)
Ugh. SD/MMC slot is the hardware, isn't it? It have wired SPI pins,
and chip select pin. To set up this pin, I need mmc node, which means
that I can't completely move mmc definitions to the board file, as
suggested by Kumar Gala.
Advices?
Segher
Thanks!
--
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2
From: Scott Wood <hidden> Date: 2007-08-01 17:43:09
Segher Boessenkool wrote:
For some buses, there is a "slot-names" property; some (non-core)
bindings seem to define a "location" property.
For "random" human-readable labelling, i.e. not corresponding to
physical markings on the hardware, I recommend you look for a
matching entry in /aliases.
Aliases could work, but are awkward to use for the purposes I'm thinking
of (giving the OS a name to present to the user in association with a
device). They're more suited to interactive OF use where the device
tree is being directly referenced by the user.
Plus, you're then restricted to valid property names for the alias,
whereas with a label property you could use any string, including spaces
and such.
It won't ever be _exactly_ what you
want though, the Linux device namespace is separate from the
device tree.
That's Linux's choice. Nothing stops it from showing device tree labels
to the user in various situations -- what got me thinking about this
was that apparently ALSA lets the driver pass an arbitrary string to
identify the device, and it seemed that such a device-tree-derived label
would be the most useful to the user. To use aliases for that, it'd
have to get the full path to the audio node, compare it to each alias,
and hope it finds one and only one, and that that alias was intended to
be a user label and not something else.
-Scott
There should be whitespace after the label. @01 should be
spelled @1. Except there is no "reg" property.
Hm. I've just tried to keep original style in this particular dts.
Want to ack patch below?
Not unless you get rid of the extra zeroes, too :-)
What is prefered style of <&label> vs. < &label > usage, btw?
arch/powerpc/boot/dts$ grep "<&" -r . | wc -l
327
arch/powerpc/boot/dts$ grep "< &" -r . | wc -l
92
I can only guess - the first?
I think it looks neater, yes. I wouldn't worry about it
too much though.
quoted
What is this
stuff, anyway?
Which one? pio-map for spi? This is GPIO pins configuration, to use
dedicated functions (SPI) for these pins, otherwise SPI will not work.
The weird pseudo-nodes, yes. This really should be handled in
platform code, not in the device tree; if there is a need to
describe what GPIOs are used for what, that should be handled
differently.
p.s. mpc8272ads.dts is broken wrt spaces/tabs, very.
Care to do a cleanup patch? Good for your karma ;-)
Segher
Can we just use the reg value for bus_num in the kernel.
Sure, technically nothing prevents this. But, QE specs names
SPIs by these ids.
As a minimum the property name should start with "fsl," then.
fsl,device-id = <1>;, correct?
Fine with me. Someone more familiar with the FSL SoCs might
have a different opinion about polluting their namespace though.
quoted
quoted
Plus, from the kernel side spi name will be
not pretty, it will be spi1216.1.
What, the kernel cannot implement a counter itself?
Just counter is especially meaningless and confusing. It will
work in that particular case, though. But then SPI bus number will
depend on definition order in the dts file. This isn't how SPI
bus numbers should be assigned. SPI bus numbers taken from specs,
this is how people know which SPI is which.
Right, so the kernel platform code should number the SPI busses
based on their position in the device tree, etc.; that doesn't
mean you should put a Linux-specific "device name" property in
there.
quoted
quoted
quoted
quoted
+ compatible = "mmc-spi";
Needs to be more specific.
Um.. for example? I can't imagine anything specific for this. ;-)
It should include a vendor name, a device name, and/or a board
name. Something that uniquely defines the hardware programming
model for the device.
quoted
quoted
quoted
quoted
+ pio-handle = <&mmc1pio>;
What is this for?
To set up output function of GPIO pin for MMC chip select.
And well, I've just looked into par_io_of_config(), and I've found
that pio-handle is mandatory (obviously), and thus let's back to:
quoted
quoted
quoted
we should do this in board code and not the device tree.
Well, I've done this initially. But Vitaly hinted that this could
be done in the DT instead, which made sense to me - mmc is the child
device of SPI bus. Why do you think it shouldn't be in the DT? I'm
not arguing, just want understand this.
The hardware should be described in the device tree. This isn't
the same as simply copying all your Linux code into it ;-)
Ugh. SD/MMC slot is the hardware, isn't it? It have wired SPI pins,
and chip select pin. To set up this pin, I need mmc node, which means
that I can't completely move mmc definitions to the board file, as
suggested by Kumar Gala.
Advices?
You need to declare in the SPI node which GPIOs it uses for
what. You shouldn't have the actual values to put into the
GPIO registers in the device tree; the kernel driver can
figure it out.
Hope this helps,
Segher
For some buses, there is a "slot-names" property; some (non-core)
bindings seem to define a "location" property.
For "random" human-readable labelling, i.e. not corresponding to
physical markings on the hardware, I recommend you look for a
matching entry in /aliases.
Aliases could work, but are awkward to use for the purposes I'm
thinking of (giving the OS a name to present to the user in
association with a device).
Sure; it's just one thing a platform driver could use.
Plus, you're then restricted to valid property names for the alias,
whereas with a label property you could use any string, including
spaces and such.
Spaces in a device name are a bad idea anyway ;-)
quoted
It won't ever be _exactly_ what you
want though, the Linux device namespace is separate from the
device tree.
That's Linux's choice. Nothing stops it from showing device tree
labels to the user in various situations -- what got me thinking
about this was that apparently ALSA lets the driver pass an arbitrary
string to identify the device, and it seemed that such a
device-tree-derived label would be the most useful to the user.
Yeah. The platform code has the final responsibility for those
names, it can use whatever mechanism is appropriate for that
platform.
To use aliases for that, it'd have to get the full path to the audio
node, compare it to each alias, and hope it finds one and only one,
This would of course be split off into a nice prom.c utility
function, so it isn't much code, just a function call.
and that that alias was intended to be a user label and not something
else.
Not sure what you mean. I'm just saying that "speed-hz" is a
terrible name, I'm not saying that "max-speed" is perfect at all.
yet you suggest a /more/ generic name, contrary to your prior comments.
My interpretation of your recent comments is that 'max-speed' is now a
valid property name for devices such as ucc_geth. Do I have that right?
Kim
Not sure what you mean. I'm just saying that "speed-hz" is a
terrible name, I'm not saying that "max-speed" is perfect at all.
yet you suggest a /more/ generic name, contrary to your prior comments.
Uh, you mean "hz" doesn't mean "Hertz"? What a great name,
then</sarcasm>.
My interpretation of your recent comments is that 'max-speed' is now a
valid property name for devices such as ucc_geth. Do I have that
right?
It is (and always was) a _valid_ name. Whether it is a _good_
name depends on the context; if there is only one speed it can
be (reasonably) referring to, it is okay; if not (or even if so),
you're better off being a bit more verbose in your property names.
No need to go over the top though, it's all a tradeoff.
Segher
From: Anton Vorontsov <hidden> Date: 2007-08-07 10:55:31
On Mon, Aug 06, 2007 at 08:18:47PM +0200, Segher Boessenkool wrote:
[...]
quoted
quoted
The hardware should be described in the device tree. This isn't
the same as simply copying all your Linux code into it ;-)
Ugh. SD/MMC slot is the hardware, isn't it? It have wired SPI pins,
and chip select pin. To set up this pin, I need mmc node, which means
that I can't completely move mmc definitions to the board file, as
suggested by Kumar Gala.
Advices?
You need to declare in the SPI node which GPIOs it uses for
what. You shouldn't have the actual values to put into the
GPIO registers in the device tree; the kernel driver can
figure it out.
Well, how SPI differs from UCC in that regard? This is how mpc832x_mds.dts,
mpc832x_rdb.dts, mpc836x_mds.dts, ... doing things already for UCC pins.
And then what pio-map exists for?.. In my understanding pio-map tries to
describe hardware (GPIO) wiring, exactly how SPI (and UCC) nodes trying
to use it.
Heh.. anyway, it's really hard to find proper logic around device tree,
do's and don'ts, so I'll just follow your suggestions in hope that I'll
get it as time goes by. ;-)
Hope this helps,
Thanks!
--
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2
The hardware should be described in the device tree. This isn't
the same as simply copying all your Linux code into it ;-)
Ugh. SD/MMC slot is the hardware, isn't it? It have wired SPI pins,
and chip select pin. To set up this pin, I need mmc node, which means
that I can't completely move mmc definitions to the board file, as
suggested by Kumar Gala.
Advices?
You need to declare in the SPI node which GPIOs it uses for
what. You shouldn't have the actual values to put into the
GPIO registers in the device tree; the kernel driver can
figure it out.
Well, how SPI differs from UCC in that regard?
Not at all, I think. You just copied a bad example that
shouldn't exist like that at all :-(
This is how mpc832x_mds.dts,
mpc832x_rdb.dts, mpc836x_mds.dts, ... doing things already for UCC
pins.
And then what pio-map exists for?.. In my understanding pio-map tries
to
describe hardware (GPIO) wiring, exactly how SPI (and UCC) nodes trying
to use it.
Yeah. It however contains lots more information that really
should be implicit in the driver using it (like, GPIO output
lines that are marked "no interrupt" -- what a surprise, duh).
As far as I can see the devices that need some GPIOs should
just say which GPIOs (on what GPIO controller) they use, and
that's about it, the kernel device drivers can handle the
rest (since they need to know lots more implicit information
_anyway_, there is no point in describing more details in the
device tree, esp. since many of those details describe only
how a device is used, not what it _is_).
Anyway, not your fault, I'd prefer not to see the madness
spread though :-)
Heh.. anyway, it's really hard to find proper logic around device tree,
do's and don'ts, so I'll just follow your suggestions in hope that I'll
get it as time goes by. ;-)