* Re: [patch 12/13] powerpc: fix building without PCI
From: Paul Mackerras @ 2007-06-14 1:44 UTC (permalink / raw)
To: arnd; +Cc: linuxppc-dev, Stephen Rothwell
In-Reply-To: <20070612163751.979776796@arndb.de>
arnd@arndb.de writes:
> Some code looks can be configured to be built without
> PCI support, but does not work properly.
I don't think it makes sense to build a chrp kernel without PCI. I
would rather just always select PCI for chrp rather than uglifying the
code with all those ifdefs.
Paul.
^ permalink raw reply
* Steps needed to get Linux 2.6 Running on Xilinx ML403
From: Leonardo Araujo @ 2007-06-14 1:43 UTC (permalink / raw)
To: linuxppc-embedded
Hi guys I just bouht a ML403 board and started to study how can I get Linux
2.6 working in this board.... My questions are.....
1)The most prepared source tree is the
http://source.mvista.com/git/gitweb.cgi?p=linux-xilinx-26.git;a=log
there I will have the linux prepared to work with Video and network devices
installed on my board?
2)I must use ELDK with this source to provide basic functionality to my
Linux working on my board as gcc, gdb....?
3)Reading some previous posts... I saw some steps used to compile the Linux
... are they right?
3.1) Use EDK 8.2.02 to generate the base system for ml403. EDK generated BSP
files.
3.2)Copy the linux-2.6.20.tar.bz2 into proper location in eldk directory and
extracted there, Then copy all of the BSP files into the root folder of my
kernel source.
3.3)Edit the Makefile and set the ARCH and CROSS_COMPILE variables , also I
set all of the env variables needed by edk.
4) Ok now I got my kernel compiled in my Computer..... how do I transfer
this kernel to my CF card on board?.... I must do some kind o bzimage....?
Thanks
Leonardo Araujo dos Santos
leonardoaraujo.santos@gmail.com
--
View this message in context: http://www.nabble.com/Steps-needed-to-get-Linux-2.6-Running-on-Xilinx-ML403-tf3918874.html#a11112197
Sent from the linuxppc-embedded mailing list archive at Nabble.com.
^ permalink raw reply
* Re: [patch 05/30] PS3: Use ioremap_flags
From: Geoff Levand @ 2007-06-14 1:42 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: Masato Noguchi, linuxppc-dev, Geert Uytterhoeven, paulus
In-Reply-To: <200706131110.48809.arnd@arndb.de>
Arnd Bergmann wrote:
> On Tuesday 12 June 2007, Geoff Levand wrote:
>> - spu->problem = ioremap(spu->problem_phys,
>> - sizeof(struct spu_problem));
>> + spu->problem = (__force void *)ioremap_flags(spu->problem_phys,
>> + sizeof(struct spu_problem), _PAGE_NO_CACHE);
>> +
>
> Unlike the local_store member, of struct spu, ->problem and ->priv2
> members are __iomem, so you should not do a __force cast here.
OK, thanks for checking it, I just did a quick edit without thinking.
-Geoff
^ permalink raw reply
* Re: [patch 29/30] PS3: Device tree source.
From: Geoff Levand @ 2007-06-14 1:31 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: linuxppc-dev, paulus
In-Reply-To: <f7c4ca74d3e2031e01264f31f09e515a@kernel.crashing.org>
Hi.
Segher Boessenkool wrote:
> Hi Geoff,
>
> Not much that can be wrong in this tree, so I'll get really
> pedantic, so you can have the best device tree ever ;-)
>
> There is one real bug though.
>
>> +/ {
>> + model = "PS3 Game Console";
>
> model = "sony,some-model-number" or similar
There is no real model number for this product. Model
numbers specify configuration like PAL video or 60GB disk.
model = "sony,ps3xxxxx"? That doesn't seem any better than
model = "sony,ps3", but then what will be used for compatible?
I will use model = "SonyPS3";
>> + compatible = "PS3";
>
> name = "sony,ps3"
As Ben H. said, the top level 'name' cannot be set, and
I found dtc exits with an error if you include one.
The compatible entry is generally used to test for machine
type, so I think that needs to remain, and I will use
compatible = "sony,ps3".
>> + /*
>> + * Need to keep linux,platform for a while, not used by kernel.
>> + */
>
> Why that?
Newer dtc seems to be OK with this removed, old ones exited with error.
>> + * Seems the only way currently to indicate a processor has multiple
>> + * threads is with an ibm,ppc-interrupt-server#s entry.
>
> Yes, a better way should be defined. Any suggestions?
No, there was some discussion to fix it, but no action.
>> + CBE,PPE {
>
> PowerPC,CBE@0 or cpu@0 -- the important thing is you need
> the @0 unit address.
OK, I set it up as cpu@0, since the actual type seems to be probed for.
>> + device_type = "cpu";
>> + reg = <0>;
>> + ibm,ppc-interrupt-server#s = <0 1>;
>> + clock-frequency = <0>;
>> + timebase-frequency = <0>;
>> + i-cache-size = <8000>;
>> + d-cache-size = <8000>;
>
> I thought DL1 was 64kB, I might be wrong though?
The BE handbook says 32.
-Geoff
^ permalink raw reply
* [patch 29/30 v2] PS3: Device tree source.
From: Geoff Levand @ 2007-06-14 1:31 UTC (permalink / raw)
To: Geoff Levand; +Cc: linuxppc-dev, paulus
In-Reply-To: <466EEC01.6080807@am.sony.com>
Subject: PS3: Device tree source.
The PS3 device tree source.
Signed-off-by: Geoff Levand <geoffrey.levand@am.sony.com>
---
ver2: Here's an updated version based on the discussions.
-Geoff
arch/powerpc/boot/dts/ps3.dts | 68 +++++++++++++++++++++++++++++++++++++
arch/powerpc/platforms/ps3/setup.c | 2 -
2 files changed, 69 insertions(+), 1 deletion(-)
--- /dev/null
+++ b/arch/powerpc/boot/dts/ps3.dts
@@ -0,0 +1,68 @@
+/*
+ * PS3 Game Console device tree.
+ *
+ * Copyright (C) 2007 Sony Computer Entertainment Inc.
+ * Copyright 2007 Sony Corp.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+/ {
+ model = "SonyPS3";
+ compatible = "sony,ps3";
+ #size-cells = <2>;
+ #address-cells = <2>;
+
+ chosen {
+ };
+
+ /*
+ * We'll get the size of the bootmem block from lv1 after startup,
+ * so we'll put a null entry here.
+ */
+
+ memory {
+ device_type = "memory";
+ reg = <0 0 0 0>;
+ };
+
+ /*
+ * The boot cpu is always zero for PS3.
+ *
+ * dtc expects a clock-frequency and timebase-frequency entries, so
+ * we'll put a null entries here. These will be initialized after
+ * startup with data from lv1.
+ *
+ * Seems the only way currently to indicate a processor has multiple
+ * threads is with an ibm,ppc-interrupt-server#s entry. We'll put one
+ * here so we can bring up both of ours. See smp_setup_cpu_maps().
+ */
+
+ cpus {
+ #size-cells = <0>;
+ #address-cells = <1>;
+
+ cpu@0 {
+ device_type = "cpu";
+ reg = <0>;
+ ibm,ppc-interrupt-server#s = <0 1>;
+ clock-frequency = <0>;
+ timebase-frequency = <0>;
+ i-cache-size = <8000>;
+ d-cache-size = <8000>;
+ i-cache-line-size = <80>;
+ d-cache-line-size = <80>;
+ };
+ };
+};
--- a/arch/powerpc/platforms/ps3/setup.c
+++ b/arch/powerpc/platforms/ps3/setup.c
@@ -193,7 +193,7 @@ static int __init ps3_probe(void)
DBG(" -> %s:%d\n", __func__, __LINE__);
dt_root = of_get_flat_dt_root();
- if (!of_flat_dt_is_compatible(dt_root, "PS3"))
+ if (!of_flat_dt_is_compatible(dt_root, "sony,ps3"))
return 0;
powerpc_firmware_features |= FW_FEATURE_PS3_POSSIBLE;
^ permalink raw reply
* New to ML-310
From: biovore @ 2007-06-14 0:31 UTC (permalink / raw)
To: linuxppc-dev
Hello everyone on the list,
I have managed to get my hands on a ML-310 play around with in my spare time
at home. I working on trying to build a linux kernel for the system. I have
ISE 9.1i and EDK 9.1i and am using there system generator to patch a
linux-2.6.22-rc3 kernel I checked out of the montivista kernel git repos. I
am familiar with crosscompiling kernels for fix processors of arm and ppc,
but I am running into problems dealing with the PPC405 in the virtex-II.
The EDK generates a bunch of code and patches the kernel. I then go into the
kernel directory and run ./xmake and it seems to go off and do some stuff to
the .config file and then starts to build the kernel. I all goes nicely
until it hits the following error.
CC arch/ppc/syslib/xilinx_pic.o
arch/ppc/syslib/xilinx_pic.c:30:5: warning: "XPAR_XINTC_USE_DCR" is not
defined
arch/ppc/syslib/xilinx_pic.c:121:5: warning: "XPAR_XINTC_USE_DCR" is not
defined
arch/ppc/syslib/xilinx_pic.c: In function 'ppc4xx_pic_init':
arch/ppc/syslib/xilinx_pic.c:122: error: 'XPAR_INTC_0_BASEADDR' undeclared
(first use in this function)
arch/ppc/syslib/xilinx_pic.c:122: error: (Each undeclared identifier is
reported only once
arch/ppc/syslib/xilinx_pic.c:122: error: for each function it appears in.)
arch/ppc/syslib/xilinx_pic.c:148: error: 'XPAR_INTC_0_KIND_OF_INTR' undeclared
(first use in this function)
make[1]: *** [arch/ppc/syslib/xilinx_pic.o] Error 1
make: *** [arch/ppc/syslib] Error 2
This to me looks like my xparameters.h file is missing something.. Any
ideas?
Peter Fetterer
--
The force is like duct tape. It has a dark side, a light side and binds the
universe together.
^ permalink raw reply
* RE: ucc_geth DPRAM alloc error, 2.6.22-rc3
From: Joakim Tjernlund @ 2007-06-13 22:47 UTC (permalink / raw)
To: 'Timur Tabi'; +Cc: 'linuxppc-dev Development'
In-Reply-To: <467068E1.8060107@freescale.com>
>
> Joakim Tjernlund wrote:
> > Trying to bring up a third ethernet i/f(ifconfig eth2 up) on
> > a Freescale 832x CPU gives me this:
> > ifconfig eth2 up
> > ucc_geth_startup: Can not allocate DPRAM memory for
> p_thread_data_tx.
> > eth2: Cannot configure net device, aborting.
> > SIOCSIFFLAGS: Cannot allocate memory
>
> It's possible that there isn't enough MURAM for three QE
> ethernet interfaces.
Well, it was in 2.6.20 and now it isn't so something is wasting MURAM.
Jocke
^ permalink raw reply
* Re: [patch 29/30] PS3: Device tree source.
From: Benjamin Herrenschmidt @ 2007-06-13 22:46 UTC (permalink / raw)
To: Olaf Hering; +Cc: linuxppc-dev, paulus
In-Reply-To: <20070613181854.GA723@aepfle.de>
On Wed, 2007-06-13 at 20:18 +0200, Olaf Hering wrote:
>
> Ok, so what will be the final values for name, model and compatible?
> Can you add them to 2.6.22 to fixate them?
No name for the root of the DT.
IMHO, model can be whatever he wants, I would say SonyPS3-XX or
something like that, whatever, doens't matter, the most precise the
better.
Then you want compatible which is the really important one. sony,ps3
looks good to me for that, though you could have a list if you wanted.
Ben.
^ permalink raw reply
* Re: [patch 29/30] PS3: Device tree source.
From: Benjamin Herrenschmidt @ 2007-06-13 22:44 UTC (permalink / raw)
To: Geoff Levand; +Cc: linuxppc-dev, Olaf Hering, paulus
In-Reply-To: <46702AFE.8080800@am.sony.com>
On Wed, 2007-06-13 at 10:35 -0700, Geoff Levand wrote:
>
> I specifically changed this because this kernel is
> NOT compatible with the device tree of the legacy
> kernel (2.6.16) that used PS3PF.
>
> If this was set as PS3PF, then there would be no
> way to tell that an incompatible DT has been passed.
No way ? heh, there are ways, you can test for a version property for
example that you add in the new one, or you can use an additional
compatible entry (compatible is a list of strings).
Ben.
^ permalink raw reply
* Re: [patch 29/30] PS3: Device tree source.
From: Benjamin Herrenschmidt @ 2007-06-13 22:42 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: linuxppc-dev, Olaf Hering, paulus
In-Reply-To: <0502b4de346bce374f98c49a99be50c1@kernel.crashing.org>
On Wed, 2007-06-13 at 11:05 +0200, Segher Boessenkool wrote:
> It should include "sony,", in the "name" property at least.
>
> "model" same as "name" cannot make sense either.
Dunno what you are on about the model vs. name but this is the root of
the DT, it has no name.
Ben.
^ permalink raw reply
* Re: ucc_geth DPRAM alloc error, 2.6.22-rc3
From: Timur Tabi @ 2007-06-13 22:00 UTC (permalink / raw)
To: joakim.tjernlund; +Cc: 'linuxppc-dev Development'
In-Reply-To: <1181744529.30670.298.camel@gentoo-jocke.transmode.se>
Joakim Tjernlund wrote:
> Trying to bring up a third ethernet i/f(ifconfig eth2 up) on
> a Freescale 832x CPU gives me this:
> ifconfig eth2 up
> ucc_geth_startup: Can not allocate DPRAM memory for p_thread_data_tx.
> eth2: Cannot configure net device, aborting.
> SIOCSIFFLAGS: Cannot allocate memory
It's possible that there isn't enough MURAM for three QE ethernet interfaces.
--
Timur Tabi
Linux Kernel Developer @ Freescale
^ permalink raw reply
* Re: [PATCH 1/2] ehea: Whitespace cleanup
From: Jeff Garzik @ 2007-06-13 19:47 UTC (permalink / raw)
To: Thomas Klein
Cc: Thomas Klein, Jan-Bernd Themann, netdev, linux-kernel,
Christoph Raisch, Stefan Roscher, linux-ppc, Marcus Eder
In-Reply-To: <200705311354.09992.osstklei@de.ibm.com>
Thomas Klein wrote:
> This patch fixes several whitespace issues.
>
> Signed-off-by: Thomas Klein <tklein@de.ibm.com>
Patch is obviously OK, but needs rediffing since I had to apply fixes
before cleanups (standard patch ordering)
^ permalink raw reply
* Re: [patch 29/30] PS3: Device tree source.
From: Olaf Hering @ 2007-06-13 18:18 UTC (permalink / raw)
To: Geoff Levand; +Cc: linuxppc-dev, paulus
In-Reply-To: <46702AFE.8080800@am.sony.com>
On Wed, Jun 13, Geoff Levand wrote:
> If this was set as PS3PF, then there would be no
> way to tell that an incompatible DT has been passed.
Ok, so what will be the final values for name, model and compatible?
Can you add them to 2.6.22 to fixate them?
^ permalink raw reply
* Re: [PATCH 1/3] [PATCH i386] during VM oom condition, kill all threads in process group
From: Oleg Nesterov @ 2007-06-13 15:51 UTC (permalink / raw)
To: Eric W. Biederman
Cc: linuxppc-dev, Andrew Morton, will_schmidt, Anton Blanchard,
linux-kernel
In-Reply-To: <m1abvarrjq.fsf@ebiederm.dsl.xmission.com>
On 06/08, Eric W. Biederman wrote:
>
> Will Schmidt <will_schmidt@vnet.ibm.com> writes:
>
> > On Fri, 2007-06-08 at 12:32 -0700, Andrew Morton wrote:
> >> On Fri, 08 Jun 2007 14:19:18 -0500
> >> Will Schmidt <will_schmidt@vnet.ibm.com> wrote:
> >>
> >> > > > > zap_other_threads() requires tasklist_lock.
> >> >
> >
> >> In fact, it's probably the case that rcu_read_lock() is now sufficient
> >> locking coverage for zap_other_threads() (cc's people).
> >>
> >> It had better be, because do_group_exit() forgot to take tasklist_lock. It
> >> is perhaps relying upon spin_lock()'s hidden rcu_read_lock() properties
> >> without so much as a code comment, which would be somewhat nasty of it.
> >
> >> You could perhaps just call do_group_exit() from within the fault
> >> handler,
> >> btw.
> >
> > Yup, so looks like I can actually replace the existing do_exit() call
> > with do_group_exit(). I'll sit on this for a bit to give other folks a
> > chance to comment on which lock call is sufficient, read_lock() or
> > rcu_read_lock(), etc; and do_group_exit()'s issue with taking
> > tasklist_lock.
>
> No. The rcu_read_lock is not sufficient.
> Yes. sighand->siglock is enough, and we explicitly take it in
> do_group_exit before calling zap_other_threads.
Yes, we don't need tasklist_lock (or rcu_read_lock).
de_thread() calls zap_other_threads() under tasklist_lock, but this
is because we can change child_reaper.
Oleg.
^ permalink raw reply
* Re: [patch 29/30] PS3: Device tree source.
From: Geoff Levand @ 2007-06-13 17:36 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: linuxppc-dev, Olaf Hering, paulus
In-Reply-To: <184b5119eca8beae9a28453096b6fe26@kernel.crashing.org>
Segher Boessenkool wrote:
>>> Please read up on what those properties mean exactly.
>>
>> Yes, do that before the inital release.
>>
>> currently name contains ''
>> currently compatible contains 'PS3PF'
>> currently model contains 'PLAYSTATION 3'
>>
>> Now, do not break the userspace ABI.
>
> The device tree is not a userspace ABI. Since
> in this case it is built into the kernel image
> (is it, actually?) there is no breakage between
> firmware and kernel versions, either.
>
>> If there is really an urge to break it,
>
> Break what? It's just a bugfix.
This is the first time I am submitting the PS3 device tree
for inclusion in the mainline kernel, so there is no breakage
or even bugfix, just new code.
I understand that Olaf would like to match what the out-of-tree
2.6.16 legacy kernel port did, but that port was never accepted
nor even recognized by the Linux community. I do not plan to
keep compatibility with that work in the mainline linux kernel.
Correctness will always take precedence over compatibility
in the mainline kernel. I will maintain what patches I can to
ease the transition from the legacy kernel to the mainline
kernel in the 'ps3-hacks' directory of my ps3-linux-patches.git
repository. I will not submit those patches for mainline
inclusion though. I'm sorry for any inconvenience.
-Geoff
^ permalink raw reply
* Re: [patch 29/30] PS3: Device tree source.
From: Geoff Levand @ 2007-06-13 17:35 UTC (permalink / raw)
To: Olaf Hering; +Cc: linuxppc-dev, paulus
In-Reply-To: <20070613084023.GA28629@aepfle.de>
Olaf Hering wrote:
> On Wed, Jun 13, Segher Boessenkool wrote:
>
>> There is one real bug though.
>
> Indeed.
>
>> > +/ {
>> > + model = "PS3 Game Console";
>>
>> model = "sony,some-model-number" or similar
>>
>> > + compatible = "PS3";
>>
>> name = "sony,ps3"
>
> Both are wrong.
>
> Stick to the values used right now.
> Which is 'PS3PF'
Hi Olaf.
I specifically changed this because this kernel is
NOT compatible with the device tree of the legacy
kernel (2.6.16) that used PS3PF.
If this was set as PS3PF, then there would be no
way to tell that an incompatible DT has been passed.
See my patch below how I use it to work around the
incompatability:
http://git.kernel.org/?p=linux/kernel/git/geoff/ps3-linux-patches.git;a=blob;f=ps3-hacks/ps3-legacy-bootloader-hack.diff
-Geoff
^ permalink raw reply
* Re: [linux-usb-devel] [PATCH 6/6] [C67x00] Merge c67x00-hub.c into c67x00-hcd.c
From: Alan Stern @ 2007-06-13 16:38 UTC (permalink / raw)
To: Grant Likely; +Cc: Peter Korsgaard, linux-usb-devel, linuxppc-embedded
In-Reply-To: <fa686aa40706130919u602498aw319cff16452079bc@mail.gmail.com>
On Wed, 13 Jun 2007, Grant Likely wrote:
> I think we're crossing wires here. In this particular case, I think
> the hub support code is sufficiently small that it doesn't need to be
> split off into a separate file. (It's only 180 lines) I'm not
> suggesting that the hub support stuff be moved to a .h file.
>
> If the code was larger, I argue that c67x00-hub.c should be compiled
> separately from c67x00-hcd.c and that 'c67x00-hub.c' be added to
> 'c67x00-$(CONFIG_USB_C67X00_HCD)' in the Makefile.
> Heh, 'polluted' is too loaded a term, and it suggests something I
> didn't mean. When the driver is built into the kernel, there are a
> number of c67x00_* symbols which are exported. These symbols are not
> used anywhere other than in the c67x00 driver code. However, this is
> necessary because the overall driver splits the various subsystems
> into separate .c files which are linked together. This approach is
> well supported by convention in the kernel, and all the non-static
> symbols use the c67x00_ prefix to avoid collisions.
>
> For example, see ib_core-y in drivers/infiniband/core/Makefile and
> pcieportdrv-y in drivers/pci/pcie/Makefile.
>
> Since the driver already makes use of this approach, I don't think it
> makes sense to use a difference approach for the root hub support
> code. (Again, I'm specifically talking about the c67x00 driver here;
> I've not looked at the *hci drivers in detail).
But you did say earlier that the way other drivers were written was a
bad idea...
In any case I agree that the root-hub code in c67x00 should be written
to match the style used by the rest of the driver.
> Of course, when it is built as a module, none of those symbols show up
> because they are not exported.
True. And true as well of the other drivers which #include *.c files.
Alan Stern
^ permalink raw reply
* Re: [linux-usb-devel] [PATCH 6/6] [C67x00] Merge c67x00-hub.c into c67x00-hcd.c
From: Grant Likely @ 2007-06-13 16:19 UTC (permalink / raw)
To: Alan Stern; +Cc: Peter Korsgaard, linux-usb-devel, linuxppc-embedded
In-Reply-To: <Pine.LNX.4.44L0.0706131130120.3607-100000@iolanthe.rowland.org>
On 6/13/07, Alan Stern <stern@rowland.harvard.edu> wrote:
> On Wed, 13 Jun 2007, Grant Likely wrote:
>
> > On 6/13/07, Alan Stern <stern@rowland.harvard.edu> wrote:
> > > On Wed, 13 Jun 2007, Grant Likely wrote:
> > >
> > > > On 6/12/07, Peter Korsgaard <jacmet@sunsite.dk> wrote:
> > > > > >>>>> "Grant" == Grant Likely <grant.likely@secretlab.ca> writes:
> > > > >
> > > > > Hi,
> > > > >
> > > > > Grant> Rather than c67x00-hub.c being compiled seperately, the
> > > > > Grant> original code had c67x00-hub.c *included* by c67x00-hcd.c.
> > > > > Grant> This is a very bad idea.
> > >
> > > What's so bad about it? It's an elegant solution to the problem of
> > > breaking a very long driver up into smaller, more digestible pieces
> > > without polluting the kernel's namespace with lots of extra global
> > > symbols.
> >
> > Primarily because it breaks convention. Convention is that you
> > #include .h files, and you compile and link .c files. Convention is
> > important because it reflects the common patterns we use when reading
> > and writing (but mostly reading) code.
>
> The problem is that there are conflicting conventions. You mentioned
> one. But there's another: .h files contain declarations and things
> that should be shared among multiple source files, and .c files contain
> things that generate object code (executable routines, static
> definitions, and so on). The idea is that sharing something which
> generates object code would be a mistake, since every source file which
> included it would generate a copy of those same objects.
I agree 100%
> So if you want to #include a file which generates object code, one
> convention says it should be named .h and the other says it should be
> named .c. A possible solution would be to use yet a different suffix,
> but I think that would only make matters worse.
Right, so I disagree with both approaches. If it generates object
code, it should go into a .c file which is compiled and linked on it's
own.
> (Just to add to the confusion, some people feel that .c files shouldn't
> include much that _doesn't_ generate object code. Hence they put
> top-level declarations in a .h file, even though it is #included in
> only one .c file. This is mainly a matter of taste...)
Heeheehee
> > Yes there are exceptions, and yes it can be done, but there better be
> > a damn good reason for doing so. In this particular case, I really
> > don't think it is warranted.
>
> The reason for doing it is the second convention. IMO that's just as
> good a reason for doing it as the first convention is for not doing it.
I think we're crossing wires here. In this particular case, I think
the hub support code is sufficiently small that it doesn't need to be
split off into a separate file. (It's only 180 lines) I'm not
suggesting that the hub support stuff be moved to a .h file.
If the code was larger, I argue that c67x00-hub.c should be compiled
separately from c67x00-hcd.c and that 'c67x00-hub.c' be added to
'c67x00-$(CONFIG_USB_C67X00_HCD)' in the Makefile.
>
> > We're not talking about a great deal of
> > code, and we're *already* polluting the kernel namespace with c67x00_*
> > function names because the driver is already in multiple pieces.
>
> Sorry, I don't know what you mean. How does the fact that uhci-hcd is
> in multiple pieces create names like c67x00_*? Besides, the fact that
> we are already doing it doesn't justify unnecessarily doing even more.
Heh, 'polluted' is too loaded a term, and it suggests something I
didn't mean. When the driver is built into the kernel, there are a
number of c67x00_* symbols which are exported. These symbols are not
used anywhere other than in the c67x00 driver code. However, this is
necessary because the overall driver splits the various subsystems
into separate .c files which are linked together. This approach is
well supported by convention in the kernel, and all the non-static
symbols use the c67x00_ prefix to avoid collisions.
For example, see ib_core-y in drivers/infiniband/core/Makefile and
pcieportdrv-y in drivers/pci/pcie/Makefile.
Since the driver already makes use of this approach, I don't think it
makes sense to use a difference approach for the root hub support
code. (Again, I'm specifically talking about the c67x00 driver here;
I've not looked at the *hci drivers in detail).
Of course, when it is built as a module, none of those symbols show up
because they are not exported.
Cheers,
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195
^ permalink raw reply
* Re: [PATCH] PHY fixed driver: rework release path and update phy_id notation
From: Joakim Tjernlund @ 2007-06-13 16:06 UTC (permalink / raw)
To: Vitaly Bordug; +Cc: linuxppc-dev, linux-kernel, Jeff Garzik, netdev
In-Reply-To: <20070609162118.2680.99019.stgit@ACA800FC.ipt.aol.com>
On Sat, 2007-06-09 at 20:21 +0400, Vitaly Bordug wrote:
> device_bind_driver() error code returning has been fixed.
> release() function has been written, so that to free resources
> in correct way; the release path is now clean.
>
> Before the rework, it used to cause
> Device 'fixed@100:1' does not have a release() function, it is broken
> and must be fixed.
> BUG: at drivers/base/core.c:104 device_release()
>
> Call Trace:
> [<ffffffff802ec380>] kobject_cleanup+0x53/0x7e
> [<ffffffff802ec3ab>] kobject_release+0x0/0x9
> [<ffffffff802ecf3f>] kref_put+0x74/0x81
> [<ffffffff8035493b>] fixed_mdio_register_device+0x230/0x265
> [<ffffffff80564d31>] fixed_init+0x1f/0x35
> [<ffffffff802071a4>] init+0x147/0x2fb
> [<ffffffff80223b6e>] schedule_tail+0x36/0x92
> [<ffffffff8020a678>] child_rip+0xa/0x12
> [<ffffffff80311714>] acpi_ds_init_one_object+0x0/0x83
> [<ffffffff8020705d>] init+0x0/0x2fb
> [<ffffffff8020a66e>] child_rip+0x0/0x12
>
>
> Also changed the notation of the fixed phy definition on
> mdio bus to the form of <speed>+<duplex> to make it able to be used by
> gianfar and ucc_geth that define phy_id strictly as "%d:%d"
>
> Signed-off-by: Vitaly Bordug <vitb@kernel.crashing.org>
Hi Vitaly
I am about to test this but I am having trouble figuring out how to
make ucc_geth find this PHY. Need some hints/template what to write
in the dts or board init code.
Also, I need several fixed PHYs, maybe you should export
fixed_mdio_register_device() ?
Jocke
^ permalink raw reply
* Re: [patch 29/30] PS3: Device tree source.
From: Olaf Hering @ 2007-06-13 16:04 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: linuxppc-dev, paulus
In-Reply-To: <d225d2e50cd1881a9fe2c015c3602c4c@kernel.crashing.org>
On Wed, Jun 13, Segher Boessenkool wrote:
> What's wrong with uname -m or cat /proc/cpuinfo?
At some point I switched to the device-tree, but I can not remember why
/proc/cpuinfo was not good enough.
^ permalink raw reply
* Re: [linux-usb-devel] [PATCH 6/6] [C67x00] Merge c67x00-hub.c into c67x00-hcd.c
From: Alan Stern @ 2007-06-13 15:43 UTC (permalink / raw)
To: Grant Likely; +Cc: Peter Korsgaard, linux-usb-devel, linuxppc-embedded
In-Reply-To: <fa686aa40706130809q78a875f5u45e458eaa54eae2d@mail.gmail.com>
On Wed, 13 Jun 2007, Grant Likely wrote:
> On 6/13/07, Alan Stern <stern@rowland.harvard.edu> wrote:
> > On Wed, 13 Jun 2007, Grant Likely wrote:
> >
> > > On 6/12/07, Peter Korsgaard <jacmet@sunsite.dk> wrote:
> > > > >>>>> "Grant" == Grant Likely <grant.likely@secretlab.ca> writes:
> > > >
> > > > Hi,
> > > >
> > > > Grant> Rather than c67x00-hub.c being compiled seperately, the
> > > > Grant> original code had c67x00-hub.c *included* by c67x00-hcd.c.
> > > > Grant> This is a very bad idea.
> >
> > What's so bad about it? It's an elegant solution to the problem of
> > breaking a very long driver up into smaller, more digestible pieces
> > without polluting the kernel's namespace with lots of extra global
> > symbols.
>
> Primarily because it breaks convention. Convention is that you
> #include .h files, and you compile and link .c files. Convention is
> important because it reflects the common patterns we use when reading
> and writing (but mostly reading) code.
The problem is that there are conflicting conventions. You mentioned
one. But there's another: .h files contain declarations and things
that should be shared among multiple source files, and .c files contain
things that generate object code (executable routines, static
definitions, and so on). The idea is that sharing something which
generates object code would be a mistake, since every source file which
included it would generate a copy of those same objects.
So if you want to #include a file which generates object code, one
convention says it should be named .h and the other says it should be
named .c. A possible solution would be to use yet a different suffix,
but I think that would only make matters worse.
(Just to add to the confusion, some people feel that .c files shouldn't
include much that _doesn't_ generate object code. Hence they put
top-level declarations in a .h file, even though it is #included in
only one .c file. This is mainly a matter of taste...)
> Yes there are exceptions, and yes it can be done, but there better be
> a damn good reason for doing so. In this particular case, I really
> don't think it is warranted.
The reason for doing it is the second convention. IMO that's just as
good a reason for doing it as the first convention is for not doing it.
> We're not talking about a great deal of
> code, and we're *already* polluting the kernel namespace with c67x00_*
> function names because the driver is already in multiple pieces.
Sorry, I don't know what you mean. How does the fact that uhci-hcd is
in multiple pieces create names like c67x00_*? Besides, the fact that
we are already doing it doesn't justify unnecessarily doing even more.
> This issue has also come up on the LKML also. See this thread:
>
> http://thread.gmane.org/gmane.linux.kernel/498633
I read that thread some time ago. If you look at it carefully, you'll
find that Linus is arguing in favor of the second convention -- mine,
not yours.
> > What's so ugly about breaking a driver up into pieces? Leaving it in
> > one giant piece would be much more ugly IMO.
>
> Breaking into pieces: Good, and I fully agree.
> Doing it in non-standard way: Not so good as it trades one kind of
> ugliness for another.
But what should one do when there are two conflicting standards?
Alan Stern
^ permalink raw reply
* Re: [linux-usb-devel] [PATCH 6/6] [C67x00] Merge c67x00-hub.c into c67x00-hcd.c
From: Grant Likely @ 2007-06-13 15:09 UTC (permalink / raw)
To: Alan Stern; +Cc: Peter Korsgaard, linux-usb-devel, linuxppc-embedded
In-Reply-To: <Pine.LNX.4.44L0.0706131033480.3467-100000@iolanthe.rowland.org>
On 6/13/07, Alan Stern <stern@rowland.harvard.edu> wrote:
> On Wed, 13 Jun 2007, Grant Likely wrote:
>
> > On 6/12/07, Peter Korsgaard <jacmet@sunsite.dk> wrote:
> > > >>>>> "Grant" == Grant Likely <grant.likely@secretlab.ca> writes:
> > >
> > > Hi,
> > >
> > > Grant> Rather than c67x00-hub.c being compiled seperately, the
> > > Grant> original code had c67x00-hub.c *included* by c67x00-hcd.c.
> > > Grant> This is a very bad idea.
>
> What's so bad about it? It's an elegant solution to the problem of
> breaking a very long driver up into smaller, more digestible pieces
> without polluting the kernel's namespace with lots of extra global
> symbols.
Primarily because it breaks convention. Convention is that you
#include .h files, and you compile and link .c files. Convention is
important because it reflects the common patterns we use when reading
and writing (but mostly reading) code.
Yes there are exceptions, and yes it can be done, but there better be
a damn good reason for doing so. In this particular case, I really
don't think it is warranted. We're not talking about a great deal of
code, and we're *already* polluting the kernel namespace with c67x00_*
function names because the driver is already in multiple pieces.
This issue has also come up on the LKML also. See this thread:
http://thread.gmane.org/gmane.linux.kernel/498633
> What's so ugly about breaking a driver up into pieces? Leaving it in
> one giant piece would be much more ugly IMO.
Breaking into pieces: Good, and I fully agree.
Doing it in non-standard way: Not so good as it trades one kind of
ugliness for another.
Cheers,
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195
^ permalink raw reply
* Re: Request for framebuffer full patch
From: Andrei Konovalov @ 2007-06-13 15:08 UTC (permalink / raw)
To: silicom; +Cc: linuxppc-embedded
In-Reply-To: <2073728.680191181708438505.JavaMail.coremail@bj163app51.163.com>
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=x-gbk; format=flowed, Size: 584 bytes --]
silicom wrote:
>
> Hi,
> can anybody send me the full patch for framebuffer driver?
The driver itself is in the mainline:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=147394c8ece44be85d692cc92cc0d047e4d8fb69
The code to register the framebuffer device on the platform bus for ML403 and ML300 boards is here:
http://source.mvista.com/git/gitweb.cgi?p=linux-xilinx-26.git;a=commitdiff;h=32b0133b72f9f03381d6db69745aac4bb1d589b2
http://source.mvista.com/git/gitweb.cgi?p=linux-xilinx-26.git;a=shortlog;h=devel
Thanksd,
Andrei
> thanks,
> sherbin
^ permalink raw reply
* Re: [spi-devel-general] [PATCH] Simple driver for Xilinx SPI controler.
From: Andrei Konovalov @ 2007-06-13 14:55 UTC (permalink / raw)
To: David Brownell; +Cc: spi-devel-general, linuxppc-embedded
In-Reply-To: <200706120928.09176.david-b@pacbell.net>
[-- Attachment #1: Type: text/plain, Size: 509 bytes --]
David Brownell wrote:
> On Saturday 09 June 2007, Andrei Konovalov wrote:
>> The new patch is attached to this message.
>
> Would you send a version that doesn't produce so many
> warnings when run against scripts/checkpatch.pl ??
>
> The issues seem to be mostly whitespace and linelength,
> so that should be easy.
>
> - Dave
Here is the updated patch (attached).
The changes are just
- replace "#include <asm/io.h>" with "#include <linux/io.h>"
- whitespace and linelength mods
Thanks again,
Andrei
[-- Attachment #2: xilinx-spi-driver.patch --]
[-- Type: text/x-patch, Size: 15310 bytes --]
Simple driver for Xilinx SPI controler.
From: Andrei Konovalov <ank@localhost.localdomain>
Only master mode is supported. No support for multiple masters.
Slave mode is not supported either.
Not using level 1 drivers from EDK.
Signed-off-by: Yuri Frolov <yfrolov@ru.mvista.com>
Signed-off-by: Andrei Konovalov <akonovalov@ru.mvista.com>
---
arch/ppc/syslib/virtex_devices.h | 7 +
drivers/spi/Kconfig | 9 +
drivers/spi/Makefile | 1
drivers/spi/xilinx_spi.c | 438 ++++++++++++++++++++++++++++++++++++++
4 files changed, 455 insertions(+), 0 deletions(-)
diff --git a/arch/ppc/syslib/virtex_devices.h b/arch/ppc/syslib/virtex_devices.h
index 3d4be14..9f38d92 100644
--- a/arch/ppc/syslib/virtex_devices.h
+++ b/arch/ppc/syslib/virtex_devices.h
@@ -31,4 +31,11 @@ void __init virtex_early_serial_map(void);
*/
int virtex_device_fixup(struct platform_device *dev);
+/* SPI Controller IP */
+struct xspi_platform_data {
+ s16 bus_num;
+ u16 num_chipselect;
+ u32 speed_hz;
+};
+
#endif /* __ASM_VIRTEX_DEVICES_H__ */
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 5e3f748..851d651 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -156,6 +156,15 @@ config SPI_S3C24XX_GPIO
GPIO lines to provide the SPI bus. This can be used where
the inbuilt hardware cannot provide the transfer mode, or
where the board is using non hardware connected pins.
+
+config SPI_XILINX
+ tristate "Xilinx SPI controller"
+ depends on SPI_MASTER && XILINX_VIRTEX && EXPERIMENTAL
+ select SPI_BITBANG
+ help
+ This enables using the SPI controller IP from Xilinx EDK in master
+ mode. See the DS464, "OPB Serial Peripheral Interface (SPI) (v1.00e)"
+ Product Specification document for the hardware details.
#
# Add new SPI master controllers in alphabetical order above this line
#
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index 5788d86..a2412bd 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -23,6 +23,7 @@ obj-$(CONFIG_SPI_MPC52xx_PSC) += mpc52xx_psc_spi.o
obj-$(CONFIG_SPI_MPC83xx) += spi_mpc83xx.o
obj-$(CONFIG_SPI_S3C24XX_GPIO) += spi_s3c24xx_gpio.o
obj-$(CONFIG_SPI_S3C24XX) += spi_s3c24xx.o
+obj-$(CONFIG_SPI_XILINX) += xilinx_spi.o
# ... add above this line ...
# SPI protocol drivers (device/link on bus)
diff --git a/drivers/spi/xilinx_spi.c b/drivers/spi/xilinx_spi.c
new file mode 100644
index 0000000..b09f7c6
--- /dev/null
+++ b/drivers/spi/xilinx_spi.c
@@ -0,0 +1,438 @@
+/*
+ * xilinx_spi.c
+ *
+ * Xilinx SPI controler driver (master mode only)
+ *
+ * Author: MontaVista Software, Inc.
+ * source@mvista.com
+ *
+ * 2002-2007 (c) MontaVista Software, Inc. This file is licensed under the
+ * terms of the GNU General Public License version 2. This program is licensed
+ * "as is" without any warranty of any kind, whether express or implied.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+#include <linux/spi/spi.h>
+#include <linux/spi/spi_bitbang.h>
+#include <linux/io.h>
+
+#include <syslib/virtex_devices.h>
+
+#define XILINX_SPI_NAME "xspi"
+
+/* Register definitions as per "OPB Serial Peripheral Interface (SPI) (v1.00e)
+ * Product Specification", DS464
+ */
+#define XSPI_CR_OFFSET 0x62 /* 16-bit Control Register */
+
+#define XSPI_CR_ENABLE 0x02
+#define XSPI_CR_MASTER_MODE 0x04
+#define XSPI_CR_CPOL 0x08
+#define XSPI_CR_CPHA 0x10
+#define XSPI_CR_MODE_MASK (XSPI_CR_CPHA | XSPI_CR_CPOL)
+#define XSPI_CR_TXFIFO_RESET 0x20
+#define XSPI_CR_RXFIFO_RESET 0x40
+#define XSPI_CR_MANUAL_SSELECT 0x80
+#define XSPI_CR_TRANS_INHIBIT 0x100
+
+#define XSPI_SR_OFFSET 0x67 /* 8-bit Status Register */
+
+#define XSPI_SR_RX_EMPTY_MASK 0x01 /* Receive FIFO is empty */
+#define XSPI_SR_RX_FULL_MASK 0x02 /* Receive FIFO is full */
+#define XSPI_SR_TX_EMPTY_MASK 0x04 /* Transmit FIFO is empty */
+#define XSPI_SR_TX_FULL_MASK 0x08 /* Transmit FIFO is full */
+#define XSPI_SR_MODE_FAULT_MASK 0x10 /* Mode fault error */
+
+#define XSPI_TXD_OFFSET 0x6b /* 8-bit Data Transmit Register */
+#define XSPI_RXD_OFFSET 0x6f /* 8-bit Data Receive Register */
+
+#define XSPI_SSR_OFFSET 0x70 /* 32-bit Slave Select Register */
+
+/* Register definitions as per "OPB IPIF (v3.01c) Product Specification", DS414
+ * IPIF registers are 32 bit
+ */
+#define XIPIF_V123B_DGIER_OFFSET 0x1c /* IPIF global int enable reg */
+#define XIPIF_V123B_GINTR_ENABLE 0x80000000
+
+#define XIPIF_V123B_IISR_OFFSET 0x20 /* IPIF interrupt status reg */
+#define XIPIF_V123B_IIER_OFFSET 0x28 /* IPIF interrupt enable reg */
+
+#define XSPI_INTR_MODE_FAULT 0x01 /* Mode fault error */
+#define XSPI_INTR_SLAVE_MODE_FAULT 0x02 /* Selected as slave while
+ * disabled */
+#define XSPI_INTR_TX_EMPTY 0x04 /* TxFIFO is empty */
+#define XSPI_INTR_TX_UNDERRUN 0x08 /* TxFIFO was underrun */
+#define XSPI_INTR_RX_FULL 0x10 /* RxFIFO is full */
+#define XSPI_INTR_RX_OVERRUN 0x20 /* RxFIFO was overrun */
+
+#define XIPIF_V123B_RESETR_OFFSET 0x40 /* IPIF reset register */
+#define XIPIF_V123B_RESET_MASK 0x0a /* the value to write */
+
+struct xilinx_spi {
+ /* bitbang has to be first */
+ struct spi_bitbang bitbang;
+ struct completion done;
+
+ u32 regs_phys; /* phys. address of the control registers */
+ void __iomem *regs; /* virt. address of the control registers */
+
+ u32 irq;
+
+ u32 speed_hz; /* SCK has a fixed frequency of speed_hz Hz */
+
+ u8 *rx_ptr; /* pointer in the Tx buffer */
+ const u8 *tx_ptr; /* pointer in the Rx buffer */
+ int remaining_bytes; /* the number of bytes left to transfer */
+};
+
+static void xspi_init_hw(void __iomem *regs_base)
+{
+ /* Reset the SPI device */
+ out_be32(regs_base + XIPIF_V123B_RESETR_OFFSET,
+ XIPIF_V123B_RESET_MASK);
+ /* Disable all the interrupts just in case */
+ out_be32(regs_base + XIPIF_V123B_IIER_OFFSET, 0);
+ /* Enable the global IPIF interrupt */
+ out_be32(regs_base + XIPIF_V123B_DGIER_OFFSET,
+ XIPIF_V123B_GINTR_ENABLE);
+ /* Deselect the slave on the SPI bus */
+ out_be32(regs_base + XSPI_SSR_OFFSET, 0xffff);
+ /* Disable the transmitter, enable Manual Slave Select Assertion,
+ * put SPI controller into master mode, and enable it */
+ out_be16(regs_base + XSPI_CR_OFFSET,
+ XSPI_CR_TRANS_INHIBIT | XSPI_CR_MANUAL_SSELECT
+ | XSPI_CR_MASTER_MODE | XSPI_CR_ENABLE);
+}
+
+static void xilinx_spi_chipselect(struct spi_device *spi, int is_on)
+{
+ struct xilinx_spi *xspi = spi_master_get_devdata(spi->master);
+
+ if (is_on == BITBANG_CS_INACTIVE) {
+ /* Deselect the slave on the SPI bus */
+ out_be32(xspi->regs + XSPI_SSR_OFFSET, 0xffff);
+ } else if (is_on == BITBANG_CS_ACTIVE) {
+ /* Set the SPI clock phase and polarity */
+ u16 cr = in_be16(xspi->regs + XSPI_CR_OFFSET)
+ & ~XSPI_CR_MODE_MASK;
+ if (spi->mode & SPI_CPHA)
+ cr |= XSPI_CR_CPHA;
+ if (spi->mode & SPI_CPOL)
+ cr |= XSPI_CR_CPOL;
+ out_be16(xspi->regs + XSPI_CR_OFFSET, cr);
+
+ /* We do not check spi->max_speed_hz here as the SPI clock
+ * frequency is not software programmable (the IP block design
+ * parameter)
+ */
+
+ /* Activate the chip select */
+ out_be32(xspi->regs + XSPI_SSR_OFFSET,
+ ~(0x0001 << spi->chip_select));
+ }
+}
+
+/* spi_bitbang requires custom setup_transfer() to be defined if there is a
+ * custom txrx_bufs(). We have nothing to setup here as the SPI IP block
+ * supports just 8 bits per word, and SPI clock can't be changed in software.
+ * Check for 8 bits per word. Chip select delay calculations could be
+ * added here as soon as bitbang_work() can be made aware of the delay value.
+ */
+static int xilinx_spi_setup_transfer(struct spi_device *spi,
+ struct spi_transfer *t)
+{
+ u8 bits_per_word;
+ u32 hz;
+ struct xilinx_spi *xspi = spi_master_get_devdata(spi->master);
+
+ bits_per_word = (t) ? t->bits_per_word : spi->bits_per_word;
+ hz = (t) ? t->speed_hz : spi->max_speed_hz;
+ if (bits_per_word != 8) {
+ dev_err(&spi->dev, "%s, unsupported bits_per_word=%d\n",
+ __FUNCTION__, bits_per_word);
+ return -EINVAL;
+ }
+
+ if (hz && xspi->speed_hz > hz) {
+ dev_err(&spi->dev, "%s, unsupported clock rate %uHz\n",
+ __FUNCTION__, hz);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+/* the spi->mode bits understood by this driver: */
+#define MODEBITS (SPI_CPOL | SPI_CPHA)
+
+static int xilinx_spi_setup(struct spi_device *spi)
+{
+ struct spi_bitbang *bitbang;
+ struct xilinx_spi *xspi;
+ int retval;
+
+ xspi = spi_master_get_devdata(spi->master);
+ bitbang = &xspi->bitbang;
+
+ if (!spi->bits_per_word)
+ spi->bits_per_word = 8;
+
+ if (spi->mode & ~MODEBITS) {
+ dev_err(&spi->dev, "%s, unsupported mode bits %x\n",
+ __FUNCTION__, spi->mode & ~MODEBITS);
+ return -EINVAL;
+ }
+
+ retval = xilinx_spi_setup_transfer(spi, NULL);
+ if (retval < 0)
+ return retval;
+
+ dev_dbg(&spi->dev, "%s, mode %d, %u bits/w, %u nsec/bit\n",
+ __FUNCTION__, spi->mode & MODEBITS, spi->bits_per_word, 0);
+
+ return 0;
+}
+
+static inline void xilinx_spi_fill_tx_fifo(struct xilinx_spi *xspi)
+{
+ u8 sr;
+
+ /* Fill the Tx FIFO with as many bytes as possible */
+ sr = in_8(xspi->regs + XSPI_SR_OFFSET);
+ while ((sr & XSPI_SR_TX_FULL_MASK) == 0 && xspi->remaining_bytes > 0) {
+ if (xspi->tx_ptr) {
+ out_8(xspi->regs + XSPI_TXD_OFFSET, *xspi->tx_ptr++);
+ } else {
+ out_8(xspi->regs + XSPI_TXD_OFFSET, 0);
+ }
+ xspi->remaining_bytes--;
+ sr = in_8(xspi->regs + XSPI_SR_OFFSET);
+ }
+}
+
+static int xilinx_spi_txrx_bufs(struct spi_device *spi, struct spi_transfer *t)
+{
+ struct xilinx_spi *xspi = spi_master_get_devdata(spi->master);
+ u32 ipif_ier;
+ u16 cr;
+
+ /* We get here with transmitter inhibited */
+
+ xspi->tx_ptr = t->tx_buf;
+ xspi->rx_ptr = t->rx_buf;
+ xspi->remaining_bytes = t->len;
+ INIT_COMPLETION(xspi->done);
+
+ xilinx_spi_fill_tx_fifo(xspi);
+
+ /* Enable the transmit empty interrupt, which we use to determine
+ * progress on the transmission.
+ */
+ ipif_ier = in_be32(xspi->regs + XIPIF_V123B_IIER_OFFSET);
+ out_be32(xspi->regs + XIPIF_V123B_IIER_OFFSET,
+ ipif_ier | XSPI_INTR_TX_EMPTY);
+
+ /* Start the transfer by not inhibiting the transmitter any longer */
+ cr = in_be16(xspi->regs + XSPI_CR_OFFSET) & ~XSPI_CR_TRANS_INHIBIT;
+ out_be16(xspi->regs + XSPI_CR_OFFSET, cr);
+
+ wait_for_completion(&xspi->done);
+
+ /* Disable the transmit empty interrupt */
+ out_be32(xspi->regs + XIPIF_V123B_IIER_OFFSET, ipif_ier);
+
+ return t->len - xspi->remaining_bytes;
+}
+
+
+/* This driver supports single master mode only. Hence Tx FIFO Empty
+ * is the only interrupt we care about.
+ * Receive FIFO Overrun, Transmit FIFO Underrun, Mode Fault, and Slave Mode
+ * Fault are not to happen.
+ */
+static irqreturn_t xilinx_spi_irq(int irq, void *dev_id)
+{
+ struct xilinx_spi *xspi = dev_id;
+ u32 ipif_isr;
+
+ /* Get the IPIF interrupts, and clear them immediately */
+ ipif_isr = in_be32(xspi->regs + XIPIF_V123B_IISR_OFFSET);
+ out_be32(xspi->regs + XIPIF_V123B_IISR_OFFSET, ipif_isr);
+
+ if (ipif_isr & XSPI_INTR_TX_EMPTY) { /* Transmission completed */
+ u16 cr;
+ u8 sr;
+
+ /* A transmit has just completed. Process received data and
+ * check for more data to transmit. Always inhibit the
+ * transmitter while the Isr refills the transmit register/FIFO,
+ * or make sure it is stopped if we're done.
+ */
+ cr = in_be16(xspi->regs + XSPI_CR_OFFSET);
+ out_be16(xspi->regs + XSPI_CR_OFFSET,
+ cr | XSPI_CR_TRANS_INHIBIT);
+
+ /* Read out all the data from the Rx FIFO */
+ sr = in_8(xspi->regs + XSPI_SR_OFFSET);
+ while ((sr & XSPI_SR_RX_EMPTY_MASK) == 0) {
+ u8 data;
+
+ data = in_8(xspi->regs + XSPI_RXD_OFFSET);
+ if (xspi->rx_ptr) {
+ *xspi->rx_ptr++ = data;
+ }
+ sr = in_8(xspi->regs + XSPI_SR_OFFSET);
+ }
+
+ /* See if there is more data to send */
+ if (xspi->remaining_bytes > 0) {
+ xilinx_spi_fill_tx_fifo(xspi);
+ /* Start the transfer by not inhibiting the
+ * transmitter any longer
+ */
+ out_be16(xspi->regs + XSPI_CR_OFFSET, cr);
+ } else {
+ /* No more data to send.
+ * Indicate the transfer is completed.
+ */
+ complete(&xspi->done);
+ }
+ } else {
+ /* spurious interrupt */
+ }
+
+ return IRQ_HANDLED;
+}
+
+static int __init xilinx_spi_probe(struct platform_device *dev)
+{
+ int ret = 0;
+ struct spi_master *master;
+ struct xilinx_spi *xspi;
+ struct xspi_platform_data *pdata;
+ struct resource *r;
+
+ /* Get resources(memory, IRQ) associated with the device */
+ master = spi_alloc_master(&dev->dev, sizeof(struct xilinx_spi));
+
+ if (master == NULL) {
+ return -ENOMEM;
+ }
+
+ platform_set_drvdata(dev, master);
+ pdata = dev->dev.platform_data;
+
+ if (pdata == NULL) {
+ ret = -ENODEV;
+ goto put_master;
+ }
+
+ r = platform_get_resource(dev, IORESOURCE_MEM, 0);
+ if (r == NULL) {
+ ret = -ENODEV;
+ goto put_master;
+ }
+
+ xspi = spi_master_get_devdata(master);
+ xspi->bitbang.master = spi_master_get(master);
+ xspi->bitbang.chipselect = xilinx_spi_chipselect;
+ xspi->bitbang.setup_transfer = xilinx_spi_setup_transfer;
+ xspi->bitbang.txrx_bufs = xilinx_spi_txrx_bufs;
+ xspi->bitbang.master->setup = xilinx_spi_setup;
+ init_completion(&xspi->done);
+
+ if (!request_mem_region(r->start,
+ r->end - r->start + 1, XILINX_SPI_NAME)) {
+ ret = -ENXIO;
+ goto put_master;
+ }
+
+ xspi->regs = ioremap(r->start, r->end - r->start + 1);
+ if (xspi->regs == NULL) {
+ ret = -ENOMEM;
+ goto put_master;
+ }
+
+ xspi->irq = platform_get_irq(dev, 0);
+ if (xspi->irq < 0) {
+ ret = -ENXIO;
+ goto unmap_io;
+ }
+
+ master->bus_num = pdata->bus_num;
+ master->num_chipselect = pdata->num_chipselect;
+ xspi->speed_hz = pdata->speed_hz;
+
+ /* SPI controller initializations */
+ xspi_init_hw(xspi->regs);
+
+ /* Register for SPI Interrupt */
+ ret = request_irq(xspi->irq, xilinx_spi_irq, 0, XILINX_SPI_NAME, xspi);
+ if (ret != 0)
+ goto unmap_io;
+
+ ret = spi_bitbang_start(&xspi->bitbang);
+ if (ret != 0) {
+ printk(KERN_ALERT "spi_btbang_start FAILED\n");
+ goto free_irq;
+ }
+
+ printk(KERN_INFO "%s: at 0x%08X mapped to 0x%08X, irq=%d\n",
+ dev->dev.bus_id, r->start, (u32)xspi->regs, xspi->irq);
+
+ return ret;
+
+free_irq:
+ free_irq(xspi->irq, xspi);
+unmap_io:
+ iounmap(xspi->regs);
+put_master:
+ spi_master_put(master);
+ return ret;
+}
+
+static int __devexit xilinx_spi_remove(struct platform_device *dev)
+{
+ struct xilinx_spi *xspi;
+ struct spi_master *master;
+
+ master = platform_get_drvdata(dev);
+ xspi = spi_master_get_devdata(master);
+
+ spi_bitbang_stop(&xspi->bitbang);
+ free_irq(xspi->irq, xspi);
+ iounmap(xspi->regs);
+ platform_set_drvdata(dev, 0);
+ spi_master_put(xspi->bitbang.master);
+
+ return 0;
+}
+
+static struct platform_driver xilinx_spi_driver = {
+ .probe = xilinx_spi_probe,
+ .remove = __devexit_p(xilinx_spi_remove),
+ .driver = {
+ .name = XILINX_SPI_NAME,
+ .owner = THIS_MODULE,
+ },
+};
+
+static int __init xilinx_spi_init(void)
+{
+ return platform_driver_register(&xilinx_spi_driver);
+}
+
+static void __exit xilinx_spi_exit(void)
+{
+ platform_driver_unregister(&xilinx_spi_driver);
+}
+
+module_init(xilinx_spi_init);
+module_exit(xilinx_spi_exit);
+
+MODULE_AUTHOR("MontaVista Software, Inc. <source@mvista.com>");
+MODULE_DESCRIPTION("Xilinx SPI driver");
+MODULE_LICENSE("GPL");
^ permalink raw reply related
* Re: [linux-usb-devel] [PATCH 6/6] [C67x00] Merge c67x00-hub.c into c67x00-hcd.c
From: Alan Stern @ 2007-06-13 14:37 UTC (permalink / raw)
To: Grant Likely; +Cc: Peter Korsgaard, linux-usb-devel, linuxppc-embedded
In-Reply-To: <fa686aa40706130554l36fefa13w4e2b1957a9fc5288@mail.gmail.com>
On Wed, 13 Jun 2007, Grant Likely wrote:
> On 6/12/07, Peter Korsgaard <jacmet@sunsite.dk> wrote:
> > >>>>> "Grant" == Grant Likely <grant.likely@secretlab.ca> writes:
> >
> > Hi,
> >
> > Grant> Rather than c67x00-hub.c being compiled seperately, the
> > Grant> original code had c67x00-hub.c *included* by c67x00-hcd.c.
> > Grant> This is a very bad idea.
What's so bad about it? It's an elegant solution to the problem of
breaking a very long driver up into smaller, more digestible pieces
without polluting the kernel's namespace with lots of extra global
symbols.
> > Simplest solution is to merge the
> > Grant> two files into one and be done with it.
> >
> > Yeah, it isn't exactly pretty, but it's what the other hcd drivers do,
> > E.G.:
> >
> > % grep -rs "include.*hub.c" *c
> > ehci-hcd.c:#include "ehci-hub.c"
> > ohci-hcd.c:#include "ohci-hub.c"
> > uhci-hcd.c:#include "uhci-hub.c"
> >
> > I don't quite know why they do it like that though ..
>
> True, but that doesn't mean that it's a good idea to follow the lead.
Why not?
> There are lots of other examples of ugly code in the kernel that is
> tolerated just because nobody has cleaned it up yet, but is still
> unacceptable for new code.
What's so ugly about breaking a driver up into pieces? Leaving it in
one giant piece would be much more ugly IMO.
> We know it's an ugly thing to do, and the fix is simple and easy.
As the Firesign Theater once said, Everything you know is wrong! :-)
Alan Stern
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox