* Re: [Linux-fbdev-devel] [PATCH 1/2] fb: add support for foreign endianness
From: Valdis.Kletnieks @ 2008-02-18 17:30 UTC (permalink / raw)
To: Krzysztof Helt
Cc: linux-fbdev-devel, adaplas, linux-kernel, linuxppc-dev,
Geert Uytterhoeven, Andrew Morton
In-Reply-To: <20080218081847.e9e65f2f.krzysztof.h1@poczta.fm>
[-- Attachment #1: Type: text/plain, Size: 522 bytes --]
On Mon, 18 Feb 2008 08:18:47 +0100, Krzysztof Helt said:
> I know two fb drivers which use endianess information (pm2fb and s3c2410fb).
> Both resolve endianess at driver level. Actually, both handle it by setting special
> bits so the graphics chip itself reorder bytes to transform foreign endianess.
> I understand that this patch is for chips which cannot reorder bytes by themselves.
Does anybody know of such a chip that's actually available in the wild? Or are
we writing drivers for speculative possible chips?
[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]
^ permalink raw reply
* Re: arch_initcall time
From: Grant Likely @ 2008-02-18 18:32 UTC (permalink / raw)
To: Sean MacLennan; +Cc: LinuxPPC-dev
In-Reply-To: <fa686aa40802181031s4ba6ec4eie949040f0bf23ad6@mail.gmail.com>
On Feb 18, 2008 11:31 AM, Grant Likely <grant.likely@secretlab.ca> wrote:
> On Feb 18, 2008 11:28 AM, Sean MacLennan <smaclennan@pikatech.com> wrote:
> > I need to call i2c_register_board_info for the new i2c style ad7414
> > driver. This needs to be called at arch initcall time. Currently I just
> > do this:
> >
> > static int __init warp_arch_init(void)
> > {
> > i2c_register_board_info(0, warp_i2c_info, ARRAY_SIZE(warp_i2c_info));
> > return 0;
> > }
> > arch_initcall(warp_arch_init);
>
> Yes, this is the right thing to do, but use machine_arch_initcall()
> instead so that it doesn't get called if it is not your board.
That being said, I believe there is infrastructure to handle the
creation of your i2c board info from the device tree. Your i2c board
info should not be hard coded.
Cheers,
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: [PATCH 1/3] Fix Unlikely(x) == y
From: Arjan van de Ven @ 2008-02-18 18:33 UTC (permalink / raw)
To: Valdis.Kletnieks
Cc: Adrian Bunk, Roel Kluin, lkml, Willy Tarreau, linuxppc-dev,
Geert Uytterhoeven, cbe-oss-dev
In-Reply-To: <26571.1203358266@turing-police.cc.vt.edu>
On Mon, 18 Feb 2008 13:11:06 -0500
Valdis.Kletnieks@vt.edu wrote:
> On Mon, 18 Feb 2008 14:27:10 GMT, David Howells said:
>
> > __builtin_expect() is useful on FRV where you _have_ to give each
> > branch and conditional branch instruction a measure of probability
> > whether the branch will be taken.
>
> What does gcc do the 99.998% of the time we don't have
> likely/unlikely coded?
see Andi's email.
It gets the exact same hints that 95%+ of the kernels unlikely/likely get you,
because the heuristics in it are usually the same as the kernel programmers
heuristics.
--
If you want to reach me at my work email, use arjan@linux.intel.com
For development, discussion and tips for power savings,
visit http://www.lesswatts.org
^ permalink raw reply
* Re: arch_initcall time
From: Olof Johansson @ 2008-02-18 18:42 UTC (permalink / raw)
To: Grant Likely; +Cc: LinuxPPC-dev, Sean MacLennan
In-Reply-To: <fa686aa40802181032lffff9e2yf079c277c250b430@mail.gmail.com>
On Mon, Feb 18, 2008 at 11:32:14AM -0700, Grant Likely wrote:
> On Feb 18, 2008 11:31 AM, Grant Likely <grant.likely@secretlab.ca> wrote:
> > On Feb 18, 2008 11:28 AM, Sean MacLennan <smaclennan@pikatech.com> wrote:
> > > I need to call i2c_register_board_info for the new i2c style ad7414
> > > driver. This needs to be called at arch initcall time. Currently I just
> > > do this:
> > >
> > > static int __init warp_arch_init(void)
> > > {
> > > i2c_register_board_info(0, warp_i2c_info, ARRAY_SIZE(warp_i2c_info));
> > > return 0;
> > > }
> > > arch_initcall(warp_arch_init);
> >
> > Yes, this is the right thing to do, but use machine_arch_initcall()
> > instead so that it doesn't get called if it is not your board.
>
> That being said, I believe there is infrastructure to handle the
> creation of your i2c board info from the device tree. Your i2c board
> info should not be hard coded.
Jon Smirl's patches? Not yet, unfortunately. It didn't make .25, but
maybe for .26.
(I will need to do it specifically on my platform, like fsl_soc already
does, as a stopgap until then).
-Olof
^ permalink raw reply
* Re: arch_initcall time
From: Josh Boyer @ 2008-02-18 18:49 UTC (permalink / raw)
To: Olof Johansson; +Cc: LinuxPPC-dev, Sean MacLennan
In-Reply-To: <20080218184240.GA17898@lixom.net>
On Mon, 18 Feb 2008 12:42:40 -0600
Olof Johansson <olof@lixom.net> wrote:
> On Mon, Feb 18, 2008 at 11:32:14AM -0700, Grant Likely wrote:
> > On Feb 18, 2008 11:31 AM, Grant Likely <grant.likely@secretlab.ca> wrote:
> > > On Feb 18, 2008 11:28 AM, Sean MacLennan <smaclennan@pikatech.com> wrote:
> > > > I need to call i2c_register_board_info for the new i2c style ad7414
> > > > driver. This needs to be called at arch initcall time. Currently I just
> > > > do this:
> > > >
> > > > static int __init warp_arch_init(void)
> > > > {
> > > > i2c_register_board_info(0, warp_i2c_info, ARRAY_SIZE(warp_i2c_info));
> > > > return 0;
> > > > }
> > > > arch_initcall(warp_arch_init);
> > >
> > > Yes, this is the right thing to do, but use machine_arch_initcall()
> > > instead so that it doesn't get called if it is not your board.
> >
> > That being said, I believe there is infrastructure to handle the
> > creation of your i2c board info from the device tree. Your i2c board
> > info should not be hard coded.
>
> Jon Smirl's patches? Not yet, unfortunately. It didn't make .25, but
> maybe for .26.
>
> (I will need to do it specifically on my platform, like fsl_soc already
> does, as a stopgap until then).
That, and Sean is still working on getting the iic device-tree-compliant driver through as
well :)
josh
^ permalink raw reply
* Re: [Cbe-oss-dev] [PATCH 1/3] Fix Unlikely(x) == y
From: Andrew Pinski @ 2008-02-18 19:22 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Adrian Bunk, Roel Kluin, lkml, cbe-oss-dev, linuxppc-dev,
Willy Tarreau, Arjan van de Ven
In-Reply-To: <Pine.LNX.4.64.0802181500410.13406@vixen.sonytel.be>
On Feb 18, 2008 6:01 AM, Geert Uytterhoeven
<Geert.Uytterhoeven@sonycom.com> wrote:
> > This means it generates faster code with a current gcc for your platform.
> >
> > But a future gcc might e.g. replace the whole loop with a division
> > (gcc SVN head (that will soon become gcc 4.3) already does
> > transformations like replacing loops with divisions [1]).
Yes but the issue is one optimization inside GCC does not take into
account the probability in one case.
And really there is a bug in the linux kernel for not implementing the
long long divide function (or really using libgcc) but that is a
different story and is part of the issue there anyways.
-- Pinski
^ permalink raw reply
* Re: [PATCH] booting-without-of: add Xilinx uart 16550.
From: Sergei Shtylyov @ 2008-02-18 19:47 UTC (permalink / raw)
To: Stephen Neuendorffer; +Cc: linuxppc-dev, Pavel Kiryukhin
In-Reply-To: <20080215214018.EA724CB0046@mail174-sin.bigfish.com>
Hello.
Stephen Neuendorffer wrote:
>>>Instead of attempting to come up with a generic description
>>>of this, I recommend just naming it after the actual device instance;
>>>something like compatible="xlnx,opb-uart16550";
>> Well, that means that we'll need a to add a code which "glues" the chip to
>>8250.c driver... well, of_serial.c could be that glue layer if we add to it
>>the ability to recognize Xilinx UART... well, legacy_serial.c could be taught
>>that trick too...
>> Well, we could also add the new compatible, but still claim "ns16550"
>>compatibility...
> This actually makes more sense to me... I'd rather have the code set
> the reg-shift than have it explicitly set in the device tree anyway.
> The compatibility set should include (at the least):
> opb_uart16550_v1_00_c
> opb_uart16550_v1_00_d
> opb_uart16550_v1_00_e
> plb_uart16550_v1_00_c
> xps_uart16550_v1_00_a
Sounds like too much? Couldn't this be handled via the "model" prop?
> I think this is somewhat independent of Sergei's arguments that generic
> ns16550 devices should allow having a reg-shift set....
> Steve
WBR, Sergei
^ permalink raw reply
* Re: [Patch 0/2] powerpc: avoid userspace poking to legacy ioports
From: Jean Delvare @ 2008-02-18 20:15 UTC (permalink / raw)
To: benh; +Cc: parabelboi, Christian Krafft, linux-kernel, linuxppc-dev
In-Reply-To: <1202935374.7296.44.camel@pasglop>
Hi Ben,
On Thu, 14 Feb 2008 07:42:54 +1100, Benjamin Herrenschmidt wrote:
>
> On Wed, 2008-02-13 at 18:35 +0100, Christian Krafft wrote:
> > sensors_detect crashes kernel on PowerPC, as it pokes directly to memory.
For the records, sensors-detect accesses I/O ports, not memory.
> > This patch adds a check_legacy_ioports to read_port and write_port.
> > It will now return ENXIO, instead of oopsing.
> >
> > Signed-off-by: Christian Krafft <krafft@de.ibm.com>
>
> The problem is that this prevents using /proc/ioports to access PCI
> IO space, which might be useful.
Maybe Christian's patch can be improved to not do the check on these?
As long as /dev/port exists, it seems reasonable that the kernel should
behave, no matter what I/O ports are accessed from user-space.
> I hate that sensors_detect.. or for that matter any other userland code
> that pokes random ports like that. It should die.
What do you propose as a replacement?
And how is userland code poking at random ports different from kernel
code poking at random ports? We could move sensors-detect inside the
kernel (and I have some plan to do that) but I fail to see how this
would solve this particular problem.
> > Index: linux.git/drivers/char/mem.c
> > ===================================================================
> > --- linux.git.orig/drivers/char/mem.c
> > +++ linux.git/drivers/char/mem.c
> > @@ -566,8 +566,13 @@ static ssize_t read_port(struct file * f
> > char __user *tmp = buf;
> >
> > if (!access_ok(VERIFY_WRITE, buf, count))
> > - return -EFAULT;
> > + return -EFAULT;
> > +
> > while (count-- > 0 && i < 65536) {
> > +#ifdef CONFIG_PPC_MERGE
> > + if (check_legacy_ioport(i))
> > + return -ENXIO;
> > +#endif
> > if (__put_user(inb(i),tmp) < 0)
> > return -EFAULT;
> > i++;
> > @@ -585,6 +590,7 @@ static ssize_t write_port(struct file *
> >
> > if (!access_ok(VERIFY_READ,buf,count))
> > return -EFAULT;
> > +
> > while (count-- > 0 && i < 65536) {
> > char c;
> > if (__get_user(c, tmp)) {
> > @@ -592,6 +598,10 @@ static ssize_t write_port(struct file *
> > break;
> > return -EFAULT;
> > }
> > +#ifdef CONFIG_PPC_MERGE
> > + if (check_legacy_ioport(i))
> > + return -ENXIO;
> > +#endif
> > outb(c,i);
> > i++;
> > tmp++;
--
Jean Delvare
^ permalink raw reply
* Re: [Patch 0/2] powerpc: avoid userspace poking to legacy ioports
From: Benjamin Herrenschmidt @ 2008-02-18 20:42 UTC (permalink / raw)
To: Jean Delvare; +Cc: parabelboi, Christian Krafft, linux-kernel, linuxppc-dev
In-Reply-To: <20080218211519.2b159ade@hyperion.delvare>
> Maybe Christian's patch can be improved to not do the check on these?
> As long as /dev/port exists, it seems reasonable that the kernel should
> behave, no matter what I/O ports are accessed from user-space.
nonsense.
/dev/mem exists for example, but you are still not supposed to go
bang all over the place in it.
> > I hate that sensors_detect.. or for that matter any other userland code
> > that pokes random ports like that. It should die.
>
> What do you propose as a replacement?
Dunno, something less scary, like knowing where your sensors are on a
given machine... honestly, it's just scary the risk you guys are taking
by banging random IO ports.
At the very least, that shouldn't be done on non-x86.
> And how is userland code poking at random ports different from kernel
> code poking at random ports? We could move sensors-detect inside the
> kernel (and I have some plan to do that) but I fail to see how this
> would solve this particular problem.
It wouldn't, but at least I could NAK it or make it CONFIG_X86 :-)
Ben.
^ permalink raw reply
* Re: [Patch 0/2] powerpc: avoid userspace poking to legacy ioports
From: Jean Delvare @ 2008-02-18 20:58 UTC (permalink / raw)
To: benh; +Cc: parabelboi, Christian Krafft, linux-kernel, linuxppc-dev
In-Reply-To: <1203367323.6740.21.camel@pasglop>
On Tue, 19 Feb 2008 07:42:03 +1100, Benjamin Herrenschmidt wrote:
>
> > Maybe Christian's patch can be improved to not do the check on these?
> > As long as /dev/port exists, it seems reasonable that the kernel should
> > behave, no matter what I/O ports are accessed from user-space.
>
> nonsense.
>
> /dev/mem exists for example, but you are still not supposed to go
> bang all over the place in it.
You should at least be able to read from it without crashing the
machine. Of course writing is a different story.
> > > I hate that sensors_detect.. or for that matter any other userland code
> > > that pokes random ports like that. It should die.
> >
> > What do you propose as a replacement?
>
> Dunno, something less scary, like knowing where your sensors are on a
> given machine...
You mean, having a complete database for the, what, 4000 PC
motherboards out there? And maintaining it day after day? _This_ sounds
much scarier to me than the current situation.
> honestly, it's just scary the risk you guys are taking
> by banging random IO ports.
I don't remember anyone reporting problems with this in the past 3 or 4
years, so it doesn't seem to be a big problem in practice.
> At the very least, that shouldn't be done on non-x86.
I am surprised that anyone would actually run sensors-detect on
non-x86... Non-PC hardware usually doesn't have sensors driven by
"hwmon" drivers anyway, or people know what they have and do not need
detection. But I would be totally fine with updating sensors-detect to
skip some of the probes on non-x86 hardware. There are basically
3 /dev/port probes that are done currently:
* Super-I/O chips at 0x2e/0x2f and 0x4e/0x4f.
* Legacy PC hardware monitoring chips at 0x290-0x297.
* IPMI interface at 0x0ca3 and 0x0cab (read-only).
Please tell me which ones should be skipped on PowerPC.
Christian, can you tell me which of these probes caused trouble for you?
> > And how is userland code poking at random ports different from kernel
> > code poking at random ports? We could move sensors-detect inside the
> > kernel (and I have some plan to do that) but I fail to see how this
> > would solve this particular problem.
>
> It wouldn't, but at least I could NAK it or make it CONFIG_X86 :-)
The same could be done for user-space (or at the /dev/port level.)
--
Jean Delvare
^ permalink raw reply
* Re: [Patch 0/2] powerpc: avoid userspace poking to legacy ioports
From: Arjan van de Ven @ 2008-02-18 21:04 UTC (permalink / raw)
To: Jean Delvare; +Cc: parabelboi, linux-kernel, linuxppc-dev, Christian Krafft
In-Reply-To: <20080218215842.66bb004f@hyperion.delvare>
On Mon, 18 Feb 2008 21:58:42 +0100
Jean Delvare <khali@linux-fr.org> wrote:
> On Tue, 19 Feb 2008 07:42:03 +1100, Benjamin Herrenschmidt wrote:
> >
> > > Maybe Christian's patch can be improved to not do the check on
> > > these? As long as /dev/port exists, it seems reasonable that the
> > > kernel should behave, no matter what I/O ports are accessed from
> > > user-space.
> >
> > nonsense.
> >
> > /dev/mem exists for example, but you are still not supposed to go
> > bang all over the place in it.
>
> You should at least be able to read from it without crashing the
> machine. Of course writing is a different story.
keep dreaming. This is not how /dev/mem works today, not on x86 and very likely not on ppc either.
^ permalink raw reply
* Re: [Patch 0/2] powerpc: avoid userspace poking to legacy ioports
From: Benjamin Herrenschmidt @ 2008-02-18 21:05 UTC (permalink / raw)
To: Jean Delvare; +Cc: parabelboi, Christian Krafft, linux-kernel, linuxppc-dev
In-Reply-To: <20080218215842.66bb004f@hyperion.delvare>
>
> * Super-I/O chips at 0x2e/0x2f and 0x4e/0x4f.
>
> * Legacy PC hardware monitoring chips at 0x290-0x297.
>
> * IPMI interface at 0x0ca3 and 0x0cab (read-only).
>
> Please tell me which ones should be skipped on PowerPC.
Skip the whole thing. I consider that on a powerpc linux port, the
platform is responsible for telling drivers where things are (via the
device tree generally)
> Christian, can you tell me which of these probes caused trouble for you?
>
> > > And how is userland code poking at random ports different from kernel
> > > code poking at random ports? We could move sensors-detect inside the
> > > kernel (and I have some plan to do that) but I fail to see how this
> > > would solve this particular problem.
> >
> > It wouldn't, but at least I could NAK it or make it CONFIG_X86 :-)
>
> The same could be done for user-space (or at the /dev/port level.)
Well, there are -other- legit usages of /dev/port...
Ben.
^ permalink raw reply
* Re: [PATCH 1/3] Fix Unlikely(x) == y
From: Michael Ellerman @ 2008-02-18 21:46 UTC (permalink / raw)
To: Adrian Bunk
Cc: Roel Kluin, lkml, cbe-oss-dev, linuxppc-dev, Geert Uytterhoeven,
Willy Tarreau, Arjan van de Ven
In-Reply-To: <20080218141340.GB667@cs181133002.pp.htv.fi>
[-- Attachment #1: Type: text/plain, Size: 1357 bytes --]
On Mon, 2008-02-18 at 16:13 +0200, Adrian Bunk wrote:
> On Mon, Feb 18, 2008 at 03:01:35PM +0100, Geert Uytterhoeven wrote:
> > On Mon, 18 Feb 2008, Adrian Bunk wrote:
> > >
> > > This means it generates faster code with a current gcc for your platform.
> > >
> > > But a future gcc might e.g. replace the whole loop with a division
> > > (gcc SVN head (that will soon become gcc 4.3) already does
> > > transformations like replacing loops with divisions [1]).
> >
> > Hence shouldn't we ask the gcc people what's the purpose of __builtin_expect(),
> > if it doesn't live up to its promise?
>
> That's a different issue.
>
> My point here is that we do not know how the latest gcc available in the
> year 2010 might transform this code, and how a likely/unlikely placed
> there might influence gcc's optimizations then.
You're right, we don't know. But if giving the compiler _more_
information causes it to produce vastly inferior code then we should be
filing gcc bugs. After all the unlikely/likely is just a hint, if gcc
knows better it can always ignore it.
cheers
--
Michael Ellerman
OzLabs, IBM Australia Development Lab
wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)
We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [Linux-fbdev-devel] [PATCH 1/2] fb: add support for foreign endianness
From: Clemens Koller @ 2008-02-18 23:35 UTC (permalink / raw)
To: linux-fbdev-devel
Cc: adaplas, Krzysztof Helt, linux-kernel, linuxppc-dev,
Geert Uytterhoeven, Andrew Morton
In-Reply-To: <19805.1203355811@turing-police.cc.vt.edu>
Valdis.Kletnieks@vt.edu schrieb:
> On Mon, 18 Feb 2008 08:18:47 +0100, Krzysztof Helt said:
>> I know two fb drivers which use endianess information (pm2fb and s3c2410fb).
>> Both resolve endianess at driver level. Actually, both handle it by setting special
>> bits so the graphics chip itself reorder bytes to transform foreign endianess.
>> I understand that this patch is for chips which cannot reorder bytes by themselves.
>
> Does anybody know of such a chip that's actually available in the wild? Or are
> we writing drivers for speculative possible chips?
>
I had troubles with the Silicon Motion SM501/SM502 endianess on PowerPC PCI vs. LocalBus.
The chip also has a register to swap endianess, but that seems to only affect some
LocalBus modes.
The current fb and X drivers are working, but when it comes to font
aliasing and hw-acceleration, the problems start to rise again...
Regards,
Clemens
^ permalink raw reply
* Re: reg Philips ISP 1562 usb controller support in linux2.6.23.11
From: Clemens Koller @ 2008-02-18 23:51 UTC (permalink / raw)
To: mahendra varman; +Cc: linuxppc-embedded
In-Reply-To: <4ac2955e0802180845m3287e30bi42f57b0a2522983f@mail.gmail.com>
Hi, Magendra!
- please don't top-post!
- and please keep the list on CC:
mahendra varman schrieb:
> Sir,
> I have enabled the necessary configs for ISP1562
>
> The IRQ number is 39 and while probing the driver correctly assigns the
> IRQ number
>
> but if i insert a device in usb port iam getting error as
>
> Unlink after no-IRQ? "
> "Controller is probably using the wrong IRQ"
>
> Although the interrupt number assignment is correct why it gives the
> above message ?
Well, that seems odd. What kind of cpu / platform / hardware are you using?
Please send the output of `lspci -vv` and `cat /proc/cpuinfo`
If you have the ISP1562 attached to PCI, I guess the interrupt assignments
are wrong or the IRQ line is broken.
Regards,
Clemens
^ permalink raw reply
* Re: Frustrated question with insmod
From: Bruce_Leonard @ 2008-02-19 0:22 UTC (permalink / raw)
To: Wolfgang Grandegger; +Cc: linuxppc-embedded
In-Reply-To: <47B8285C.40204@grandegger.com>
> Bruce_Leonard@selinc.com wrote:
> > Sorry if this is the wrong place to post this question. I'm
developing a
> > NAND flash driver and I need to do some detailed dubugging using GDB
with
> > a BDI2K. According to the Denx web site, to find out the address that
the
> > module is loading at you load it using the -m parameter to insmod
(i.e.,
> > "insmod -m mymodule"). However, every version of insmod I've tried
> > doesn't recognize ANY options much less -m. Can anyone please point
me in
> > the right direction, or give me another way of knowing what the load
> > address of my module is?
>
> # cat /sys/module/<name>/sections/.text
>
> Do not forget to enable CONFIG_KALLSYMS.
>
Well, okay I guess the address I'm getting is the right one because both
the above cat and 'cat /proc/modules' gives me the same thing, 0xe1188000.
I've got CONFIG_KALLSYMS and CONFIG_KALLSYMS_ALL set, though
CONFIG_KALLSYMS_EXTRA_PASS is not set, don't know if that makes a
difference.
So it would seem that there's something wrong with my BDI setup that isn't
allowing address translation in the kernel's dynamically allocated memory
area. I've got PTBASE set to 0xf0 in the BDI config file so it should be
finding the virtual address of the page tables just fine. I've also got
CONFIG_BDI_SWITCH set in .config and I know that works with the BDI,
because I can set breakpoints at places in the kernel code that are called
by my module (like nand_scan_ident() ) and everything works just fine.
It's just when I try to access memory in the dynamic area where my module
is located that the BDI can't do an address translation. At least I
assume it's the BDI, because I can load the module and use it with out
GDB/BDI, so the processor and kernel must be able to handle the addresses
okay. Can anyone think of where I should go dig? I've had this working
exactly once in the past, but I don't know what I've changed to cause it
to stop working.
Thanks.
Bruce
^ permalink raw reply
* Re: [Linux-fbdev-devel] [PATCH 1/2] fb: add support for foreign endianness
From: Benjamin Herrenschmidt @ 2008-02-19 0:35 UTC (permalink / raw)
To: Clemens Koller
Cc: linux-fbdev-devel, adaplas, Krzysztof Helt, linux-kernel,
linuxppc-dev, Geert Uytterhoeven, Andrew Morton
In-Reply-To: <47BA162C.5000807@anagramm.de>
On Tue, 2008-02-19 at 00:35 +0100, Clemens Koller wrote:
> Valdis.Kletnieks@vt.edu schrieb:
> > On Mon, 18 Feb 2008 08:18:47 +0100, Krzysztof Helt said:
> >> I know two fb drivers which use endianess information (pm2fb and s3c2410fb).
> >> Both resolve endianess at driver level. Actually, both handle it by setting special
> >> bits so the graphics chip itself reorder bytes to transform foreign endianess.
> >> I understand that this patch is for chips which cannot reorder bytes by themselves.
> >
> > Does anybody know of such a chip that's actually available in the wild? Or are
> > we writing drivers for speculative possible chips?
> >
>
> I had troubles with the Silicon Motion SM501/SM502 endianess on PowerPC PCI vs. LocalBus.
> The chip also has a register to swap endianess, but that seems to only affect some
> LocalBus modes.
> The current fb and X drivers are working, but when it comes to font
> aliasing and hw-acceleration, the problems start to rise again...
Most "sane" gfx chips nowadays provide configurable surfaces that allow
to perform the swap when writing/reading from regions of the
framebuffer, with the ability to set a different swapper setting (based
on bit depth) per region.
Then there is also the risk that your PCI<->Localbus has been wired
improperly :-)
Ben.
^ permalink raw reply
* Re: [PATCH] next-20080218 build failure at pmac_ide_macio_attach ()
From: Bartlomiej Zolnierkiewicz @ 2008-02-19 0:04 UTC (permalink / raw)
To: Kamalesh Babulal; +Cc: linux-ide, sfr, linux-kernel, linuxppc-dev
In-Reply-To: <20080218140159.GA31310@linux.vnet.ibm.com>
On Monday 18 February 2008, Kamalesh Babulal wrote:
> Hi,
>=20
> The next-20080218 kernel build fails on the powerpc(s)
>=20
> drivers/ide/ppc/pmac.c: In function =E2=80=98pmac_ide_macio_attach=E2=80=
=99:
> drivers/ide/ppc/pmac.c:1094: error: conversion to non-scalar type request=
ed
> drivers/ide/ppc/pmac.c: In function =E2=80=98pmac_ide_pci_attach=E2=80=99:
> drivers/ide/ppc/pmac.c:1232: error: conversion to non-scalar type request=
ed
> make[3]: *** [drivers/ide/ppc/pmac.o] Error 1
> make[2]: *** [drivers/ide/ppc] Error 2
> make[1]: *** [drivers/ide] Error 2
> make: *** [drivers] Error 2
>=20
> I Have tested this patch for build failure only.
>=20
> Signed-off-by: Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>
> ---
> --- linux-2.6.25-rc1/drivers/ide/ppc/pmac.c 2008-02-18 18:41:48.000000000=
+0530
> +++ linux-2.6.25-rc1/drivers/ide/ppc/~pmac.c 2008-02-18 19:20:37.00000000=
0 +0530
> @@ -1091,7 +1091,7 @@ pmac_ide_macio_attach(struct macio_dev *
> int irq, rc;
> hw_regs_t hw;
> =20
> - pmif =3D (struct pmac_ide_hwif)kzalloc(sizeof(*pmif), GFP_KERNEL);
> + pmif =3D (struct pmac_ide_hwif*)kzalloc(sizeof(*pmif), GFP_KERNEL);
> if (pmif =3D=3D NULL)
> return -ENOMEM;
> =20
> @@ -1229,7 +1229,7 @@ pmac_ide_pci_attach(struct pci_dev *pdev
> return -ENODEV;
> }
> =20
> - pmif =3D (struct pmac_ide_hwif)kzalloc(sizeof(*pmif), GFP_KERNEL);
> + pmif =3D (struct pmac_ide_hwif*)kzalloc(sizeof(*pmif), GFP_KERNEL);
> if (pmif =3D=3D NULL)
> return -ENOMEM;
> =20
Thanks, I integrated it with the "guilty" patch to preserve bisectability.
=46rom: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Subject: [PATCH] ide-pmac: dynamically allocate struct pmac_ide_hwif instan=
ces (take 2)
* Dynamically allocate struct pmac_ide_hwif instances in pmac_ide_macio_att=
ach()
and pmac_ide_pci_attach(), then remove no longer needed pmac_ide[].
v2:
* Build fix from Kamalesh Babulal.
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
=2D--
drivers/ide/ppc/pmac.c | 49 +++++++++++++++++++++++++++++++++-----------=
=2D----
1 file changed, 33 insertions(+), 16 deletions(-)
Index: b/drivers/ide/ppc/pmac.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
=2D-- a/drivers/ide/ppc/pmac.c
+++ b/drivers/ide/ppc/pmac.c
@@ -79,8 +79,6 @@ typedef struct pmac_ide_hwif {
=09
} pmac_ide_hwif_t;
=20
=2Dstatic pmac_ide_hwif_t pmac_ide[MAX_HWIFS];
=2D
enum {
controller_ohare, /* OHare based */
controller_heathrow, /* Heathrow/Paddington */
@@ -1094,29 +1092,34 @@ pmac_ide_macio_attach(struct macio_dev *
int i, rc;
hw_regs_t hw;
=20
+ pmif =3D kzalloc(sizeof(*pmif), GFP_KERNEL);
+ if (pmif =3D=3D NULL)
+ return -ENOMEM;
+
i =3D 0;
=2D while (i < MAX_HWIFS && (ide_hwifs[i].io_ports[IDE_DATA_OFFSET] !=3D 0
=2D || pmac_ide[i].node !=3D NULL))
+ while (i < MAX_HWIFS && (ide_hwifs[i].io_ports[IDE_DATA_OFFSET] !=3D 0))
++i;
if (i >=3D MAX_HWIFS) {
printk(KERN_ERR "ide-pmac: MacIO interface attach with no slot\n");
printk(KERN_ERR " %s\n", mdev->ofdev.node->full_name);
=2D return -ENODEV;
+ rc =3D -ENODEV;
+ goto out_free_pmif;
}
=20
=2D pmif =3D &pmac_ide[i];
hwif =3D &ide_hwifs[i];
=20
if (macio_resource_count(mdev) =3D=3D 0) {
printk(KERN_WARNING "ide%d: no address for %s\n",
i, mdev->ofdev.node->full_name);
=2D return -ENXIO;
+ rc =3D -ENXIO;
+ goto out_free_pmif;
}
=20
/* Request memory resource for IO ports */
if (macio_request_resource(mdev, 0, "ide-pmac (ports)")) {
printk(KERN_ERR "ide%d: can't request mmio resource !\n", i);
=2D return -EBUSY;
+ rc =3D -EBUSY;
+ goto out_free_pmif;
}
=09
/* XXX This is bogus. Should be fixed in the registry by checking
@@ -1166,11 +1169,15 @@ pmac_ide_macio_attach(struct macio_dev *
iounmap(pmif->dma_regs);
macio_release_resource(mdev, 1);
}
=2D memset(pmif, 0, sizeof(*pmif));
macio_release_resource(mdev, 0);
+ kfree(pmif);
}
=20
return rc;
+
+out_free_pmif:
+ kfree(pmif);
+ return rc;
}
=20
static int
@@ -1223,30 +1230,36 @@ pmac_ide_pci_attach(struct pci_dev *pdev
printk(KERN_ERR "ide-pmac: cannot find MacIO node for Kauai ATA interfac=
e\n");
return -ENODEV;
}
+
+ pmif =3D kzalloc(sizeof(*pmif), GFP_KERNEL);
+ if (pmif =3D=3D NULL)
+ return -ENOMEM;
+
i =3D 0;
=2D while (i < MAX_HWIFS && (ide_hwifs[i].io_ports[IDE_DATA_OFFSET] !=3D 0
=2D || pmac_ide[i].node !=3D NULL))
+ while (i < MAX_HWIFS && (ide_hwifs[i].io_ports[IDE_DATA_OFFSET] !=3D 0))
++i;
if (i >=3D MAX_HWIFS) {
printk(KERN_ERR "ide-pmac: PCI interface attach with no slot\n");
printk(KERN_ERR " %s\n", np->full_name);
=2D return -ENODEV;
+ rc =3D -ENODEV;
+ goto out_free_pmif;
}
=20
=2D pmif =3D &pmac_ide[i];
hwif =3D &ide_hwifs[i];
=20
if (pci_enable_device(pdev)) {
printk(KERN_WARNING "ide%i: Can't enable PCI device for %s\n",
i, np->full_name);
=2D return -ENXIO;
+ rc =3D -ENXIO;
+ goto out_free_pmif;
}
pci_set_master(pdev);
=09
if (pci_request_regions(pdev, "Kauai ATA")) {
printk(KERN_ERR "ide%d: Cannot obtain PCI resources for %s\n",
i, np->full_name);
=2D return -ENXIO;
+ rc =3D -ENXIO;
+ goto out_free_pmif;
}
=20
hwif->dev =3D &pdev->dev;
@@ -1276,11 +1289,15 @@ pmac_ide_pci_attach(struct pci_dev *pdev
/* The inteface is released to the common IDE layer */
pci_set_drvdata(pdev, NULL);
iounmap(base);
=2D memset(pmif, 0, sizeof(*pmif));
pci_release_regions(pdev);
+ kfree(pmif);
}
=20
return rc;
+
+out_free_pmif:
+ kfree(pmif);
+ return rc;
}
=20
static int
^ permalink raw reply
* Re: [PATCH 2/2] i2c-ibm_iic driver
From: Sean MacLennan @ 2008-02-19 1:42 UTC (permalink / raw)
To: Jean Delvare; +Cc: LinuxPPC-dev, i2c
In-Reply-To: <20080216103123.50a0128b@hyperion.delvare>
An updated version of the patch. This one should answer all of Jean's
concerns.
Cheers,
Sean
Signed-off-by: Sean MacLennan <smaclennan@pikatech.com>
---
--- for-2.6.25/drivers/i2c/busses/orig-i2c-ibm_iic.c 2008-02-18 16:36:30.000000000 -0500
+++ for-2.6.25/drivers/i2c/busses/i2c-ibm_iic.c 2008-02-18 17:39:53.000000000 -0500
@@ -6,6 +6,9 @@
* Copyright (c) 2003, 2004 Zultys Technologies.
* Eugene Surovegin <eugene.surovegin@zultys.com> or <ebs@ebshome.net>
*
+ * Copyright (c) 2008 PIKA Technologies
+ * Sean MacLennan <smaclennan@pikatech.com>
+ *
* Based on original work by
* Ian DaSilva <idasilva@mvista.com>
* Armin Kuster <akuster@mvista.com>
@@ -39,12 +42,17 @@
#include <asm/io.h>
#include <linux/i2c.h>
#include <linux/i2c-id.h>
+
+#ifdef CONFIG_IBM_OCP
#include <asm/ocp.h>
#include <asm/ibm4xx.h>
+#else
+#include <linux/of_platform.h>
+#endif
#include "i2c-ibm_iic.h"
-#define DRIVER_VERSION "2.1"
+#define DRIVER_VERSION "2.2"
MODULE_DESCRIPTION("IBM IIC driver v" DRIVER_VERSION);
MODULE_LICENSE("GPL");
@@ -657,6 +665,7 @@
return (u8)((opb + 9) / 10 - 1);
}
+#ifdef CONFIG_IBM_OCP
/*
* Register single IIC interface
*/
@@ -828,5 +837,182 @@
ocp_unregister_driver(&ibm_iic_driver);
}
+#else /* !CONFIG_IBM_OCP */
+
+static int __devinit iic_request_irq(struct of_device *ofdev,
+ struct ibm_iic_private *dev)
+{
+ struct device_node *np = ofdev->node;
+ int irq;
+
+ if (iic_force_poll)
+ return NO_IRQ;
+
+ irq = irq_of_parse_and_map(np, 0);
+ if (irq == NO_IRQ) {
+ dev_err(&ofdev->dev, "irq_of_parse_and_map failed\n");
+ return NO_IRQ;
+ }
+
+ /* Disable interrupts until we finish initialization, assumes
+ * level-sensitive IRQ setup...
+ */
+ iic_interrupt_mode(dev, 0);
+ if (request_irq(irq, iic_handler, 0, "IBM IIC", dev)) {
+ dev_err(&ofdev->dev, "request_irq %d failed\n", irq);
+ /* Fallback to the polling mode */
+ return NO_IRQ;
+ }
+
+ return irq;
+}
+
+/*
+ * Register single IIC interface
+ */
+static int __devinit iic_probe(struct of_device *ofdev,
+ const struct of_device_id *match)
+{
+ struct device_node *np = ofdev->node;
+ struct ibm_iic_private *dev;
+ struct i2c_adapter *adap;
+ const u32 *indexp, *freq;
+ int ret;
+
+
+ dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+ if (!dev) {
+ dev_err(&ofdev->dev, "failed to allocate device data\n");
+ return -ENOMEM;
+ }
+
+ dev_set_drvdata(&ofdev->dev, dev);
+
+ indexp = of_get_property(np, "index", NULL);
+ if (!indexp) {
+ dev_err(&ofdev->dev, "no index specified.\n");
+ ret = -EINVAL;
+ goto error_cleanup;
+ }
+ dev->idx = *indexp;
+
+ dev->vaddr = of_iomap(np, 0);
+ if (dev->vaddr == NULL) {
+ dev_err(&ofdev->dev, "failed to iomap device\n");
+ ret = -ENXIO;
+ goto error_cleanup;
+ }
+
+ init_waitqueue_head(&dev->wq);
+
+ dev->irq = iic_request_irq(ofdev, dev);
+ if (dev->irq == NO_IRQ)
+ dev_warn(&ofdev->dev, "using polling mode\n");
+
+ /* Board specific settings */
+ if (iic_force_fast || of_get_property(np, "fast-mode", NULL))
+ dev->fast_mode = 1;
+
+ freq = of_get_property(np, "clock-frequency", NULL);
+ if (freq == NULL) {
+ freq = of_get_property(np->parent, "clock-frequency", NULL);
+ if (freq == NULL) {
+ dev_err(&ofdev->dev, "Unable to get bus frequency\n");
+ ret = -EBUSY;
+ goto error_cleanup;
+ }
+ }
+
+ dev->clckdiv = iic_clckdiv(*freq);
+ dev_dbg(&ofdev->dev, "clckdiv = %d\n", dev->clckdiv);
+
+ /* Initialize IIC interface */
+ iic_dev_init(dev);
+
+ /* Register it with i2c layer */
+ adap = &dev->adap;
+ adap->dev.parent = &ofdev->dev;
+ strlcpy(adap->name, "IBM IIC", sizeof(adap->name));
+ i2c_set_adapdata(adap, dev);
+ adap->id = I2C_HW_OCP;
+ adap->class = I2C_CLASS_HWMON;
+ adap->algo = &iic_algo;
+ adap->timeout = 1;
+ adap->nr = dev->idx;
+
+ ret = i2c_add_numbered_adapter(adap);
+ if (ret < 0) {
+ dev_err(&ofdev->dev, "failed to register i2c adapter\n");
+ goto error_cleanup;
+ }
+
+ dev_dbg(&ofdev->dev, "using %s mode\n",
+ dev->fast_mode ? "fast (400 kHz)" : "standard (100 kHz)");
+
+ return 0;
+
+error_cleanup:
+ if (dev->irq != NO_IRQ) {
+ iic_interrupt_mode(dev, 0);
+ free_irq(dev->irq, dev);
+ }
+
+ if (dev->vaddr)
+ iounmap(dev->vaddr);
+
+ dev_set_drvdata(&ofdev->dev, NULL);
+ kfree(dev);
+ return ret;
+}
+
+/*
+ * Cleanup initialized IIC interface
+ */
+static int __devexit iic_remove(struct of_device *ofdev)
+{
+ struct ibm_iic_private *dev = dev_get_drvdata(&ofdev->dev);
+
+ dev_set_drvdata(&ofdev->dev, NULL);
+
+ i2c_del_adapter(&dev->adap);
+
+ if (dev->irq != NO_IRQ) {
+ iic_interrupt_mode(dev, 0);
+ free_irq(dev->irq, dev);
+ }
+
+ iounmap(dev->vaddr);
+ kfree(dev);
+
+ return 0;
+}
+
+static const struct of_device_id ibm_iic_match[] = {
+ { .compatible = "ibm,iic-405ex", },
+ { .compatible = "ibm,iic-405gp", },
+ { .compatible = "ibm,iic-440gp", },
+ { .compatible = "ibm,iic-440gpx", },
+ { .compatible = "ibm,iic-440grx", },
+ {}
+};
+
+static struct of_platform_driver ibm_iic_driver = {
+ .name = "ibm-iic",
+ .match_table = ibm_iic_match,
+ .probe = iic_probe,
+ .remove = __devexit_p(iic_remove),
+};
+
+static int __init iic_init(void)
+{
+ return of_register_platform_driver(&ibm_iic_driver);
+}
+
+static void __exit iic_exit(void)
+{
+ of_unregister_platform_driver(&ibm_iic_driver);
+}
+#endif /* CONFIG_IBM_OCP */
+
module_init(iic_init);
module_exit(iic_exit);
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index b61f56b..44c0984 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -244,7 +244,6 @@ config I2C_PIIX4
config I2C_IBM_IIC
tristate "IBM PPC 4xx on-chip I2C interface"
- depends on IBM_OCP
help
Say Y here if you want to use IIC peripheral found on
embedded IBM PPC 4xx based systems.
^ permalink raw reply related
* Re: [PATCH] i2c-ibm_iic driver bonus patch
From: Sean MacLennan @ 2008-02-19 2:02 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <477EF225.4070505@pikatech.com>
Here is an optional bonus patch that cleans up most of the checkpatch
warnings in the common code. I left in the volatiles, since I don't
understand why they where needed. The memory always seems to be access
with in_8 and out_8, which are declared volatile. But they could be
there to fix a very specific bug.
Cheers,
Sean
Signed-off-by: Sean MacLennan <smaclennan@pikatech.com>
---
--- for-2.6.25/drivers/i2c/busses/submitted-i2c-ibm_iic.c 2008-02-18 20:44:06.000000000 -0500
+++ for-2.6.25/drivers/i2c/busses/i2c-ibm_iic.c 2008-02-18 20:53:53.000000000 -0500
@@ -76,17 +76,17 @@
#endif
#if DBG_LEVEL > 0
-# define DBG(f,x...) printk(KERN_DEBUG "ibm-iic" f, ##x)
+# define DBG(f, x...) printk(KERN_DEBUG "ibm-iic" f, ##x)
#else
-# define DBG(f,x...) ((void)0)
+# define DBG(f, x...) ((void)0)
#endif
#if DBG_LEVEL > 1
-# define DBG2(f,x...) DBG(f, ##x)
+# define DBG2(f, x...) DBG(f, ##x)
#else
-# define DBG2(f,x...) ((void)0)
+# define DBG2(f, x...) ((void)0)
#endif
#if DBG_LEVEL > 2
-static void dump_iic_regs(const char* header, struct ibm_iic_private* dev)
+static void dump_iic_regs(const char *header, struct ibm_iic_private *dev)
{
volatile struct iic_regs __iomem *iic = dev->vaddr;
printk(KERN_DEBUG "ibm-iic%d: %s\n", dev->idx, header);
@@ -98,9 +98,9 @@
in_8(&iic->extsts), in_8(&iic->clkdiv), in_8(&iic->xfrcnt),
in_8(&iic->xtcntlss), in_8(&iic->directcntl));
}
-# define DUMP_REGS(h,dev) dump_iic_regs((h),(dev))
+# define DUMP_REGS(h, dev) dump_iic_regs((h), (dev))
#else
-# define DUMP_REGS(h,dev) ((void)0)
+# define DUMP_REGS(h, dev) ((void)0)
#endif
/* Bus timings (in ns) for bit-banging */
@@ -111,25 +111,26 @@
unsigned int high;
unsigned int buf;
} timings [] = {
-/* Standard mode (100 KHz) */
-{
- .hd_sta = 4000,
- .su_sto = 4000,
- .low = 4700,
- .high = 4000,
- .buf = 4700,
-},
-/* Fast mode (400 KHz) */
-{
- .hd_sta = 600,
- .su_sto = 600,
- .low = 1300,
- .high = 600,
- .buf = 1300,
-}};
+ /* Standard mode (100 KHz) */
+ {
+ .hd_sta = 4000,
+ .su_sto = 4000,
+ .low = 4700,
+ .high = 4000,
+ .buf = 4700,
+ },
+ /* Fast mode (400 KHz) */
+ {
+ .hd_sta = 600,
+ .su_sto = 600,
+ .low = 1300,
+ .high = 600,
+ .buf = 1300,
+ }
+};
/* Enable/disable interrupt generation */
-static inline void iic_interrupt_mode(struct ibm_iic_private* dev, int enable)
+static inline void iic_interrupt_mode(struct ibm_iic_private *dev, int enable)
{
out_8(&dev->vaddr->intmsk, enable ? INTRMSK_EIMTC : 0);
}
@@ -137,7 +138,7 @@
/*
* Initialize IIC interface.
*/
-static void iic_dev_init(struct ibm_iic_private* dev)
+static void iic_dev_init(struct ibm_iic_private *dev)
{
volatile struct iic_regs __iomem *iic = dev->vaddr;
@@ -182,7 +183,7 @@
/*
* Reset IIC interface
*/
-static void iic_dev_reset(struct ibm_iic_private* dev)
+static void iic_dev_reset(struct ibm_iic_private *dev)
{
volatile struct iic_regs __iomem *iic = dev->vaddr;
int i;
@@ -191,19 +192,19 @@
DBG("%d: soft reset\n", dev->idx);
DUMP_REGS("reset", dev);
- /* Place chip in the reset state */
+ /* Place chip in the reset state */
out_8(&iic->xtcntlss, XTCNTLSS_SRST);
/* Check if bus is free */
dc = in_8(&iic->directcntl);
- if (!DIRCTNL_FREE(dc)){
+ if (!DIRCTNL_FREE(dc)) {
DBG("%d: trying to regain bus control\n", dev->idx);
/* Try to set bus free state */
out_8(&iic->directcntl, DIRCNTL_SDAC | DIRCNTL_SCC);
/* Wait until we regain bus control */
- for (i = 0; i < 100; ++i){
+ for (i = 0; i < 100; ++i) {
dc = in_8(&iic->directcntl);
if (DIRCTNL_FREE(dc))
break;
@@ -235,7 +236,7 @@
static int iic_dc_wait(volatile struct iic_regs __iomem *iic, u8 mask)
{
unsigned long x = jiffies + HZ / 28 + 2;
- while ((in_8(&iic->directcntl) & mask) != mask){
+ while ((in_8(&iic->directcntl) & mask) != mask) {
if (unlikely(time_after(jiffies, x)))
return -1;
cond_resched();
@@ -243,15 +244,15 @@
return 0;
}
-static int iic_smbus_quick(struct ibm_iic_private* dev, const struct i2c_msg* p)
+static int iic_smbus_quick(struct ibm_iic_private *dev, const struct i2c_msg *p)
{
volatile struct iic_regs __iomem *iic = dev->vaddr;
- const struct i2c_timings* t = &timings[dev->fast_mode ? 1 : 0];
+ const struct i2c_timings *t = &timings[dev->fast_mode ? 1 : 0];
u8 mask, v, sda;
int i, res;
/* Only 7-bit addresses are supported */
- if (unlikely(p->flags & I2C_M_TEN)){
+ if (unlikely(p->flags & I2C_M_TEN)) {
DBG("%d: smbus_quick - 10 bit addresses are not supported\n",
dev->idx);
return -EINVAL;
@@ -275,7 +276,7 @@
/* Send address */
v = (u8)((p->addr << 1) | ((p->flags & I2C_M_RD) ? 1 : 0));
- for (i = 0, mask = 0x80; i < 8; ++i, mask >>= 1){
+ for (i = 0, mask = 0x80; i < 8; ++i, mask >>= 1) {
out_8(&iic->directcntl, sda);
ndelay(t->low / 2);
sda = (v & mask) ? DIRCNTL_SDAC : 0;
@@ -330,7 +331,7 @@
*/
static irqreturn_t iic_handler(int irq, void *dev_id)
{
- struct ibm_iic_private* dev = (struct ibm_iic_private*)dev_id;
+ struct ibm_iic_private *dev = (struct ibm_iic_private *)dev_id;
volatile struct iic_regs __iomem *iic = dev->vaddr;
DBG2("%d: irq handler, STS = 0x%02x, EXTSTS = 0x%02x\n",
@@ -347,11 +348,11 @@
* Get master transfer result and clear errors if any.
* Returns the number of actually transferred bytes or error (<0)
*/
-static int iic_xfer_result(struct ibm_iic_private* dev)
+static int iic_xfer_result(struct ibm_iic_private *dev)
{
volatile struct iic_regs __iomem *iic = dev->vaddr;
- if (unlikely(in_8(&iic->sts) & STS_ERR)){
+ if (unlikely(in_8(&iic->sts) & STS_ERR)) {
DBG("%d: xfer error, EXTSTS = 0x%02x\n", dev->idx,
in_8(&iic->extsts));
@@ -367,20 +368,19 @@
* IIC interface is usually stuck in some strange
* state, the only way out - soft reset.
*/
- if ((in_8(&iic->extsts) & EXTSTS_BCS_MASK) != EXTSTS_BCS_FREE){
+ if ((in_8(&iic->extsts) & EXTSTS_BCS_MASK) != EXTSTS_BCS_FREE) {
DBG("%d: bus is stuck, resetting\n", dev->idx);
iic_dev_reset(dev);
}
return -EREMOTEIO;
- }
- else
+ } else
return in_8(&iic->xfrcnt) & XFRCNT_MTC_MASK;
}
/*
* Try to abort active transfer.
*/
-static void iic_abort_xfer(struct ibm_iic_private* dev)
+static void iic_abort_xfer(struct ibm_iic_private *dev)
{
volatile struct iic_regs __iomem *iic = dev->vaddr;
unsigned long x;
@@ -394,8 +394,8 @@
* It's not worth to be optimized, just poll (timeout >= 1 tick)
*/
x = jiffies + 2;
- while ((in_8(&iic->extsts) & EXTSTS_BCS_MASK) != EXTSTS_BCS_FREE){
- if (time_after(jiffies, x)){
+ while ((in_8(&iic->extsts) & EXTSTS_BCS_MASK) != EXTSTS_BCS_FREE) {
+ if (time_after(jiffies, x)) {
DBG("%d: abort timeout, resetting...\n", dev->idx);
iic_dev_reset(dev);
return;
@@ -412,19 +412,19 @@
* It puts current process to sleep until we get interrupt or timeout expires.
* Returns the number of transferred bytes or error (<0)
*/
-static int iic_wait_for_tc(struct ibm_iic_private* dev){
-
+static int iic_wait_for_tc(struct ibm_iic_private *dev)
+{
volatile struct iic_regs __iomem *iic = dev->vaddr;
int ret = 0;
- if (dev->irq >= 0){
+ if (dev->irq >= 0) {
/* Interrupt mode */
ret = wait_event_interruptible_timeout(dev->wq,
!(in_8(&iic->sts) & STS_PT), dev->adap.timeout * HZ);
if (unlikely(ret < 0))
DBG("%d: wait interrupted\n", dev->idx);
- else if (unlikely(in_8(&iic->sts) & STS_PT)){
+ else if (unlikely(in_8(&iic->sts) & STS_PT)) {
DBG("%d: wait timeout\n", dev->idx);
ret = -ETIMEDOUT;
}
@@ -433,14 +433,14 @@
/* Polling mode */
unsigned long x = jiffies + dev->adap.timeout * HZ;
- while (in_8(&iic->sts) & STS_PT){
- if (unlikely(time_after(jiffies, x))){
+ while (in_8(&iic->sts) & STS_PT) {
+ if (unlikely(time_after(jiffies, x))) {
DBG("%d: poll timeout\n", dev->idx);
ret = -ETIMEDOUT;
break;
}
- if (unlikely(signal_pending(current))){
+ if (unlikely(signal_pending(current))) {
DBG("%d: poll interrupted\n", dev->idx);
ret = -ERESTARTSYS;
break;
@@ -462,11 +462,11 @@
/*
* Low level master transfer routine
*/
-static int iic_xfer_bytes(struct ibm_iic_private* dev, struct i2c_msg* pm,
+static int iic_xfer_bytes(struct ibm_iic_private *dev, struct i2c_msg *pm,
int combined_xfer)
{
volatile struct iic_regs __iomem *iic = dev->vaddr;
- char* buf = pm->buf;
+ char *buf = pm->buf;
int i, j, loops, ret = 0;
int len = pm->len;
@@ -475,7 +475,7 @@
cntl |= CNTL_RW;
loops = (len + 3) / 4;
- for (i = 0; i < loops; ++i, len -= 4){
+ for (i = 0; i < loops; ++i, len -= 4) {
int count = len > 4 ? 4 : len;
u8 cmd = cntl | ((count - 1) << CNTL_TCT_SHIFT);
@@ -498,7 +498,7 @@
if (unlikely(ret < 0))
break;
- else if (unlikely(ret != count)){
+ else if (unlikely(ret != count)) {
DBG("%d: xfer_bytes, requested %d, transfered %d\n",
dev->idx, count, ret);
@@ -521,7 +521,7 @@
/*
* Set target slave address for master transfer
*/
-static inline void iic_address(struct ibm_iic_private* dev, struct i2c_msg* msg)
+static inline void iic_address(struct ibm_iic_private *dev, struct i2c_msg *msg)
{
volatile struct iic_regs __iomem *iic = dev->vaddr;
u16 addr = msg->addr;
@@ -529,24 +529,24 @@
DBG2("%d: iic_address, 0x%03x (%d-bit)\n", dev->idx,
addr, msg->flags & I2C_M_TEN ? 10 : 7);
- if (msg->flags & I2C_M_TEN){
+ if (msg->flags & I2C_M_TEN) {
out_8(&iic->cntl, CNTL_AMD);
out_8(&iic->lmadr, addr);
out_8(&iic->hmadr, 0xf0 | ((addr >> 7) & 0x06));
- }
- else {
+ } else {
out_8(&iic->cntl, 0);
out_8(&iic->lmadr, addr << 1);
}
}
-static inline int iic_invalid_address(const struct i2c_msg* p)
+static inline int iic_invalid_address(const struct i2c_msg *p)
{
- return (p->addr > 0x3ff) || (!(p->flags & I2C_M_TEN) && (p->addr > 0x7f));
+ return (p->addr > 0x3ff) ||
+ (!(p->flags & I2C_M_TEN) && (p->addr > 0x7f));
}
-static inline int iic_address_neq(const struct i2c_msg* p1,
- const struct i2c_msg* p2)
+static inline int iic_address_neq(const struct i2c_msg *p1,
+ const struct i2c_msg *p2)
{
return (p1->addr != p2->addr)
|| ((p1->flags & I2C_M_TEN) != (p2->flags & I2C_M_TEN));
@@ -558,9 +558,13 @@
*/
static int iic_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
{
- struct ibm_iic_private* dev = (struct ibm_iic_private*)(i2c_get_adapdata(adap));
- volatile struct iic_regs __iomem *iic = dev->vaddr;
+ struct ibm_iic_private *dev;
+ volatile struct iic_regs __iomem *iic;
int i, ret = 0;
+ u8 extsts;
+
+ dev = (struct ibm_iic_private *)(i2c_get_adapdata(adap));
+ iic = dev->vaddr;
DBG2("%d: iic_xfer, %d msg(s)\n", dev->idx, num);
@@ -570,14 +574,14 @@
/* Check the sanity of the passed messages.
* Uhh, generic i2c layer is more suitable place for such code...
*/
- if (unlikely(iic_invalid_address(&msgs[0]))){
+ if (unlikely(iic_invalid_address(&msgs[0]))) {
DBG("%d: invalid address 0x%03x (%d-bit)\n", dev->idx,
msgs[0].addr, msgs[0].flags & I2C_M_TEN ? 10 : 7);
return -EINVAL;
}
- for (i = 0; i < num; ++i){
- if (unlikely(msgs[i].len <= 0)){
- if (num == 1 && !msgs[0].len){
+ for (i = 0; i < num; ++i) {
+ if (unlikely(msgs[i].len <= 0)) {
+ if (num == 1 && !msgs[0].len) {
/* Special case for I2C_SMBUS_QUICK emulation.
* IBM IIC doesn't support 0-length transactions
* so we have to emulate them using bit-banging.
@@ -588,14 +592,15 @@
msgs[i].len, i);
return -EINVAL;
}
- if (unlikely(iic_address_neq(&msgs[0], &msgs[i]))){
+ if (unlikely(iic_address_neq(&msgs[0], &msgs[i]))) {
DBG("%d: invalid addr in msg[%d]\n", dev->idx, i);
return -EINVAL;
}
}
/* Check bus state */
- if (unlikely((in_8(&iic->extsts) & EXTSTS_BCS_MASK) != EXTSTS_BCS_FREE)){
+ extsts = in_8(&iic->extsts);
+ if (unlikely((extsts & EXTSTS_BCS_MASK) != EXTSTS_BCS_FREE)) {
DBG("%d: iic_xfer, bus is not free\n", dev->idx);
/* Usually it means something serious has happend.
@@ -608,12 +613,11 @@
*/
iic_dev_reset(dev);
- if ((in_8(&iic->extsts) & EXTSTS_BCS_MASK) != EXTSTS_BCS_FREE){
+ if ((in_8(&iic->extsts) & EXTSTS_BCS_MASK) != EXTSTS_BCS_FREE) {
DBG("%d: iic_xfer, bus is still not free\n", dev->idx);
return -EREMOTEIO;
}
- }
- else {
+ } else {
/* Flush master data buffer (just in case) */
out_8(&iic->mdcntl, in_8(&iic->mdcntl) | MDCNTL_FMDB);
}
@@ -622,7 +626,7 @@
iic_address(dev, &msgs[0]);
/* Do real transfer */
- for (i = 0; i < num && !ret; ++i)
+ for (i = 0; i < num && !ret; ++i)
ret = iic_xfer_bytes(dev, &msgs[i], i < num - 1);
return ret < 0 ? ret : num;
@@ -648,7 +652,7 @@
* Previous driver version used hardcoded divider value 4,
* it corresponds to OPB frequency from the range (40, 50] MHz
*/
- if (!opb){
+ if (!opb) {
printk(KERN_WARNING "ibm-iic: using compatibility value for OPB freq,"
" fix your board specific setup\n");
opb = 50000000;
@@ -657,9 +661,9 @@
/* Convert to MHz */
opb /= 1000000;
- if (opb < 20 || opb > 150){
- printk(KERN_WARNING "ibm-iic: invalid OPB clock frequency %u MHz\n",
- opb);
+ if (opb < 20 || opb > 150) {
+ printk(KERN_WARNING "ibm-iic: "
+ "invalid OPB clock frequency %u MHz\n", opb);
opb = opb < 20 ? 20 : 150;
}
return (u8)((opb + 9) / 10 - 1);
^ permalink raw reply
* Re: [PATCH 1/3] Fix Unlikely(x) == y
From: Arjan van de Ven @ 2008-02-19 2:40 UTC (permalink / raw)
To: Nick Piggin
Cc: Roel Kluin, lkml, cbe-oss-dev, linuxppc-dev, Andi Kleen,
Willy Tarreau
In-Reply-To: <200802191333.53607.nickpiggin@yahoo.com.au>
On Tue, 19 Feb 2008 13:33:53 +1100
Nick Piggin <nickpiggin@yahoo.com.au> wrote:
>
> Actually one thing I don't like about gcc is that I think it still
> emits cmovs for likely/unlikely branches, which is silly (the gcc
> developers seem to be in love with that instruction). If that goes
> away, then branch hints may be even better.
only for -Os and only if the result is smaller afaik.
(cmov tends to be a performance loss most of the time so for -O2 and such it
isn't used as far as I know.. it does make for nice small code however ;-)
--
If you want to reach me at my work email, use arjan@linux.intel.com
For development, discussion and tips for power savings,
visit http://www.lesswatts.org
^ permalink raw reply
* Re: compile quirk linux-2.6.24 (with workaround)
From: Josh Boyer @ 2008-02-19 2:52 UTC (permalink / raw)
To: Josh Boyer; +Cc: linuxppc-dev, debian-powerpc, paulus, Bernhard Reiter
In-Reply-To: <20080205093820.5918a216@zod.rchland.ibm.com>
On Tue, 5 Feb 2008 09:38:20 -0600
Josh Boyer <jwboyer@linux.vnet.ibm.com> wrote:
> On Tue, 5 Feb 2008 08:24:38 -0700
> "Grant Likely" <grant.likely@secretlab.ca> wrote:
>
> > On 2/5/08, Josh Boyer <jwboyer@linux.vnet.ibm.com> wrote:
> > > > I mean, if you have not included 4xx support in the kernel, as is the
> > > > case here, it does not make sense to add the 4xx bootwrapper code, no ?
> > >
> > > It does, in a manner. There are both generic and platform specific
> > > pieces to the bootwrapper. Having everything always built helps keep
> > > the generic bits from breaking, which is important as they're often
> > > tightly coupled. That's at least the reason I can think of.
> > >
> > > The powerpc maintainers have been over this quite a bit and I don't see
> > > it changing anytime soon.
> >
> > That would mean we're dropping support for compilers which can't build
> > 405/440 specific wrapper bits (or other core specific quirks that need
> > to go in the wrapper) That doesn't sound appropriate to me.
>
> No it doesn't. At least not yet. I said I'd try to come up with a
> patch soon-ish. We haven't failed!(yet) Also, this isn't a
> core-specific quirk. It's an architected instruction of Book III-E in
> the PowerPC ISA. I can't help it if other chips don't implement this
> wonderful control mechanism ;)
>
> Taking a step back though, there will always be odd cases like this as
> we move forward. Toolchain XXX will eventually not support instruction
> YYYY which will eventually be used, etc. I'll try to make this
> specific case work because it's scope is quite limited. But this
> problem as a whole will still remain.
My apologies for taking so long on this. Digging through gcc history
isn't exactly fun :)
Ok, so it seems -mcpu=440 was added in gcc 3.4. The -mcpu=405 option
has been around since 2001. Seeing as how there really isn't anything
440 specific in the files effected, we should be able to pass -mcpu=405
for everything and have it still work.
Bernhard, can you try the patch below? I've compile test it, and
booted it on an Ebony board (PowerPC 440GP). If anyone else cares to
test that would also be welcome.
josh
[POWERPC] Fix bootwrapper builds with older gcc versions
GCC versions before 3.4 did not support the -mcpu=440 option. Use
-mcpu=405 for the 4xx specific bootwrapper files, as that has been
around for much longer.
Signed-off-by: Josh Boyer <jwboyer@linux.vnet.ibm.com>
diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
index 63d07cc..e3993a6 100644
--- a/arch/powerpc/boot/Makefile
+++ b/arch/powerpc/boot/Makefile
@@ -35,10 +35,10 @@ endif
BOOTCFLAGS += -I$(obj) -I$(srctree)/$(obj) -I$(srctree)/$(src)/libfdt
-$(obj)/4xx.o: BOOTCFLAGS += -mcpu=440
-$(obj)/ebony.o: BOOTCFLAGS += -mcpu=440
-$(obj)/cuboot-taishan.o: BOOTCFLAGS += -mcpu=440
-$(obj)/cuboot-katmai.o: BOOTCFLAGS += -mcpu=440
+$(obj)/4xx.o: BOOTCFLAGS += -mcpu=405
+$(obj)/ebony.o: BOOTCFLAGS += -mcpu=405
+$(obj)/cuboot-taishan.o: BOOTCFLAGS += -mcpu=405
+$(obj)/cuboot-katmai.o: BOOTCFLAGS += -mcpu=405
$(obj)/treeboot-walnut.o: BOOTCFLAGS += -mcpu=405
^ permalink raw reply related
* Re: [PATCH] i2c-ibm_iic driver bonus patch
From: Arnd Bergmann @ 2008-02-19 3:27 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Sean MacLennan
In-Reply-To: <47BA38C1.6040609@pikatech.com>
On Tuesday 19 February 2008, Sean MacLennan wrote:
> I left in the volatiles, since I don't
> understand why they where needed. The memory always seems to be access
> with in_8 and out_8, which are declared volatile. But they could be
> there to fix a very specific bug.
It's very unlikely that they were really needed, and you certainly
shouldn't mark data as volatile in new code. It's very common to
mark I/O data structures as volatile when they should be __iomem,
because that's what people learn at university, but that is never
the right solution, even if it can hide other bugs in your code.
Of course, unlike the other changes in your patch, it does impact
code generation, so if you want to change it, that should be a
separate patch.
Arnd <><
^ permalink raw reply
* Re: [PATCH 1/3] Fix Unlikely(x) == y
From: Nick Piggin @ 2008-02-19 2:33 UTC (permalink / raw)
To: Andi Kleen
Cc: Roel Kluin, lkml, Willy Tarreau, linuxppc-dev, cbe-oss-dev,
Arjan van de Ven
In-Reply-To: <p731w7axrat.fsf@bingen.suse.de>
On Tuesday 19 February 2008 01:39, Andi Kleen wrote:
> Arjan van de Ven <arjan@infradead.org> writes:
> > you have more faith in the authors knowledge of how his code actually
> > behaves than I think is warranted :)
>
> iirc there was a mm patch some time ago to keep track of the actual
> unlikely values at runtime and it showed indeed some wrong ones. But the
> far majority of them are probably correct.
>
> > Or faith in that he knows what "unlikely" means.
> > I should write docs about this; but unlikely() means:
> > 1) It happens less than 0.01% of the cases.
> > 2) The compiler couldn't have figured this out by itself
> > (NULL pointer checks are compiler done already, same for some other
> > conditions) 3) It's a hot codepath where shaving 0.5 cycles (less even on
> > x86) matters (and the author is ok with taking a 500 cycles hit if he's
> > wrong)
>
> One more thing unlikely() does is to move the unlikely code out of line.
> So it should conserve some icache in critical functions, which might
> well be worth some more cycles (don't have numbers though).
I actually once measured context switching performance in the scheduler,
and removing the unlikely hint for testing RT tasks IIRC gave about 5%
performance drop.
This was on a P4 which is very different from more modern CPUs both in
terms of branch performance characteristics, and icache characteristics.
However, the P4's branch predictor is pretty good, and it should easily
be able to correctly predict the rt_task check if it has enough entries.
So I think much of the savings came from code transformation and movement.
Anyway, it is definitely worthwhile if used correctly.
Actually one thing I don't like about gcc is that I think it still emits
cmovs for likely/unlikely branches, which is silly (the gcc developers
seem to be in love with that instruction). If that goes away, then
branch hints may be even better.
>
> But overall I agree with you that unlikely is in most cases a bad
> idea (and I submitted the original patch introducing it originally @). That
> is because it is often used in situations where gcc's default branch
> prediction heuristics do would make exactly the same decision
>
> if (unlikely(x == NULL))
>
> is simply totally useless because gcc already assumes all x == NULL
> tests are unlikely. I appended some of the builtin heuristics from
> a recent gcc source so people can see them.
>
> Note in particular the last predictors; assuming branch ending
> with goto, including call, causing early function return or
> returning negative constant are not taken. Just these alone
> are likely 95+% of the unlikelies in the kernel.
Yes, gcc should be able to do pretty good heuristics, considering
the quite good numbers that cold CPU predictors can attain. However
for really performance critical code (or really "never" executed
code), then I think it is OK to have the hints and not have to rely
on gcc heuristics.
>
> -Andi
[snip]
Interesting, thanks!
^ permalink raw reply
* Re: [PATCH 1/3] Fix Unlikely(x) == y
From: Nick Piggin @ 2008-02-19 4:41 UTC (permalink / raw)
To: Arjan van de Ven
Cc: Roel Kluin, lkml, cbe-oss-dev, linuxppc-dev, Andi Kleen,
Willy Tarreau
In-Reply-To: <20080218184027.429cf47b@laptopd505.fenrus.org>
On Tuesday 19 February 2008 13:40, Arjan van de Ven wrote:
> On Tue, 19 Feb 2008 13:33:53 +1100
>
> Nick Piggin <nickpiggin@yahoo.com.au> wrote:
> > Actually one thing I don't like about gcc is that I think it still
> > emits cmovs for likely/unlikely branches, which is silly (the gcc
> > developers seem to be in love with that instruction). If that goes
> > away, then branch hints may be even better.
>
> only for -Os and only if the result is smaller afaik.
What is your evidence for saying this? Because here, with the latest
kernel and recent gcc-4.3 snapshot, it spits out cmov like crazy even
when compiled with -O2.
npiggin@am:~/usr/src/linux-2.6$ grep cmov kernel/sched.s | wc -l
45
And yes it even does for hinted branches and even at -O2/3
npiggin@am:~/tests$ cat cmov.c
int test(int a, int b)
{
if (__builtin_expect(a < b, 0))
return a;
else
return b;
}
npiggin@am:~/tests$ gcc-4.3 -S -O2 cmov.c
npiggin@am:~/tests$ head -13 cmov.s
.file "cmov.c"
.text
.p2align 4,,15
..globl test
.type test, @function
test:
..LFB2:
cmpl %edi, %esi
cmovle %esi, %edi
movl %edi, %eax
ret
..LFE2:
.size test, .-test
This definitely should be a branch, IMO.
> (cmov tends to be a performance loss most of the time so for -O2 and such
> it isn't used as far as I know.. it does make for nice small code however
> ;-)
It shouldn't be hard to work out the cutover point based on how
expensive cmov is, how expensive branch and branch mispredicts are,
and how often the branch is likely to be mispredicted. For an
unpredictable branch, cmov is normally quite a good win even on
modern CPUs. But gcc overuses it I think.
^ 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