LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Xilinx devicetrees
From: David H. Lynch Jr. @ 2007-11-24 11:37 UTC (permalink / raw)
  To: linuxppc-embedded

    I am following developments regarding device trees for xilinx boards
both here and on the microblaze list.

    I am trying to get a grasp on what they will really do for me and
what using them will demand.

    Please correct any misperceptions:

     As I understand it devicetrees are basically just a tree structured
binary  database describing the hardware.
    They have some heritage in OpenFirmware.
    There are tools to convert  some human readable representations into
the binary form.
    There appear to be tools to take xilinx firmware projects and create
a devicetree database from it
    A BSP using devicetree's configures its hardware, drivers etc, by
querying the devicetree database.  
    It it possible to pass the device tree database independent of the
kernel itself some what similar to the way many bootloaders pass initrd
filesystems.
   
    So in the end I write a BSP that could support a wide variety of
hardware and compile a single kernel that could be passed different
devicetree databases representing different xilinx firmware, and still
hope to work.
    But in return for making the BSP more generic (sort of), I now have
to somehow get the correct devicetree database passed for each different
firmware set that I load.

    I am having some difficulty grasping the significant advantages to
this.
    If the firmware for a given target is not fairly static - and I load
different firmware depending on what I am doing all the time, then I
know have to manage both a bit file for the firmware, and a devicetree
file describing it to the kernel.
    Currently for base hardware we maintain as much design consistancy
as possible accross all our different cards/firmware.
    particular hardware/firmware blocks/IP's may or may not be present -
but if present they are always the same - the Same Uartlite at the same
location, on the same irq, same for PIC's, TEMAC's ...
    For the most part it makes the most sense for us to use code to
detect the presence/absence of specific baseline hardware and then to
load non-base custom drivers after boot.

    What am  missing about devicetrees that would make me more
interested in them ?
      
  




-- 
Dave Lynch 					  	    DLA Systems
Software Development:  				         Embedded Linux
717.627.3770 	       dhlii@dlasys.net 	  http://www.dlasys.net
fax: 1.253.369.9244 			           Cell: 1.717.587.7774
Over 25 years' experience in platforms, languages, and technologies too numerous to list.

"Any intelligent fool can make things bigger and more complex... It takes a touch of genius - and a lot of courage to move in the opposite direction."
Albert Einstein

^ permalink raw reply

* Re: oops trying to execute sh
From: John Tyner @ 2007-11-24 14:26 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <Pine.LNX.4.64.0711211149210.3338@eon.cs.ucr.edu>

This is ppc. I'm in the midst of trying to get powerpc to boot, but our 
boards are running an old version of ppcboot that can't be upgraded, so 
I'm having to figure out the translation to the open firmware stuff.

By the way, this is an 860t, not c... typo.

Do you have any suggestions about getting ppc to boot? I'd like to try to 
at least get the board booting so I can hand the user space stuff off to 
someone else while I do the powerpc port.

Thanks,
John

> On Wed, 21 Nov 2007 11:54:01 -0800 (PST)
> John Charles Tyner wrote:
> 
> > I'm trying to boot linux 2.6.22.9 on an mpc860c rev d4.
> > 
> ppc or powerpc?
> 
> 
> > When init trys to spawn sh, during the exec, the kernel oopses as
> > seen below:
> This looks like coherency problem, or kernel picks wrong entry off 
> cputable.
> I think I recall something similar when I lost a hunk applying patch for 
> new e300 core.
> ... or not. The game across that ff8.. value is very confusing.
> 
> -- 
> Sincerely, Vitaly

On Wed, Nov 21, 2007 at 11:54:01AM -0800, John Charles Tyner wrote:
> I'm trying to boot linux 2.6.22.9 on an mpc860c rev d4.
> 
> When init trys to spawn sh, during the exec, the kernel oopses as seen 
> below:
> 
> ## Starting application at 0x00400000 ...
> 
> loaded at:     00400000 004EF15C
> board data at: 03F9FBC0 03F9FBFC
> relocated to:  00404044 00404080
> zimage at:     00404E74 004EC662
> avail ram:     004F0000 04000000
> 
> Linux/PPC load: console=ttyCPM,38400
> Uncompressing Linux...done.
> Now booting the kernel
> Linux version 2.6.22.9 (jtyner@johnnyedge) (gcc version 4.2.1) #113 Wed Nov 
> 21 10:49:36 PST 2007
> Zone PFN ranges:
>   DMA             0 ->    16384
>   Normal      16384 ->    16384
> early_node_map[1] active PFN ranges
>     0:        0 ->    16384
> Built 1 zonelists.  Total pages: 16256
> Kernel command line: console=ttyCPM,38400
> PID hash table entries: 256 (order: 8, 1024 bytes)
> Decrementer Frequency = 183750000/60
> Console: colour dummy device 80x25
> cpm_uart: console: compat mode
> Dentry cache hash table entries: 8192 (order: 3, 32768 bytes)
> Inode-cache hash table entries: 4096 (order: 2, 16384 bytes)
> Memory: 63244k available (880k kernel code, 268k data, 444k init, 0k 
> highmem)
> Mount-cache hash table entries: 512
> ADDSI: Init
> io scheduler noop registered (default)
> Serial: CPM driver $Revision: 0.02 $
> ttyCPM0 at MMIO 0xc5000a80 (irq = 20) is a CPM UART
> mice: PS/2 mouse device common for all mice
> Freeing unused kernel memory: 444k init
> init started: BusyBox v1.8.0 (2007-11-16 14:24:51 PST)
> starting pid 103, tty '': '/bin/sh'
> Oops: kernel access of bad area, sig: 11 [#1]
> NIP: c0044ed0 LR: c0044ff0 CTR: 00000001
> REGS: c3c0bd00 TRAP: 0300   Not tainted  (2.6.22.9)
> MSR: 00009032 <EE,ME,IR,DR>  CR: 30099099  XER: a0008c7f
> DAR: ff80103f, DSISR: c0000000
> TASK = c0288070[103] 'init' THREAD: c3c0a000
> GPR00: c0044ff0 c3c0bdb0 c0288070 ff800fff 00000000 7faf8000 00000000 
> 00000000
> GPR08: c01a8f58 c017d91c 00000002 c0179cd0 30099093 1007687c 00000002 
> c00f8744
> GPR16: 00000000 c00f0a64 c011d1ac c00f0aa4 c00f0a90 c0120000 00000001 
> 00000003
> GPR24: c3c1ce00 00000000 c0180000 c0247550 00000000 c3c0bdc8 c0179cd0 
> ff800fff
> NIP [c0044ed0] remove_vma+0x14/0x70
> LR [c0044ff0] exit_mmap+0xc4/0xf0
> Call Trace:
> [c3c0bdb0] [c3c0bdc8] 0xc3c0bdc8 (unreliable)
> [c3c0bdc0] [c0044ff0] exit_mmap+0xc4/0xf0
> [c3c0bdf0] [c000f74c] mmput+0x50/0xd4
> [c3c0be00] [c00591f4] flush_old_exec+0x3b8/0x7a8
> [c3c0be50] [c0086cc0] load_elf_binary+0x2e8/0x1454
> [c3c0bee0] [c005892c] search_binary_handler+0x58/0x12c
> [c3c0bf00] [c0059d64] do_execve+0x13c/0x1f0
> [c3c0bf20] [c00089b4] sys_execve+0x50/0x90
> [c3c0bf40] [c0002a40] ret_from_syscall+0x0/0x38
> Instruction dump:
> 7d808120 38210040 4e800020 83c30000 4bffff18 38a00000 4bffff9c 7c0802a6
> 9421fff0 bfc10008 90010014 7c7f1b78 <81230040> 83c3000c 2f890000 419e0018
> 
> The interesting thing is that r3 points to something funny. While tracing 
> this problem down, I replaced the remove_vma function with the following:
> 
> /*
>  * Close a vm structure and free it, returning the next.
>  */
> static struct vm_area_struct * __attribute__((__noinline__)) 
> __remove_vma(struct vm_area_struct *vma)
> {
> 
> 	struct vm_area_struct *next = vma->vm_next;
> 
> 	might_sleep();
> 	if (vma->vm_ops && vma->vm_ops->close)
> 		vma->vm_ops->close(vma);
> 	if (vma->vm_file)
> 		fput(vma->vm_file);
> 	mpol_free(vma_policy(vma));
> 	kmem_cache_free(vm_area_cachep, vma);
> 	return next;
> }
> 
> static struct vm_area_struct *remove_vma(struct vm_area_struct *vma)
> {
>         asm volatile (
>                 "lis  4,-128\n"
>                 "ori  4,4,4095\n"
>                 "tweq 3,4\n"
>                 "lwz  5,0(1)\n"
>                 "tweq 3,4\n"
>                 );
>         return __remove_vma( vma );
> }
> 
> With this code, the kernel oopses on the *second* tweq instruction:
> 
> Kernel BUG at c0045fd4 [verbose debug info unavailable]
> Oops: Exception in kernel mode, sig: 5 [#1]
> NIP: c0045fd4 LR: c00460a0 CTR: 00000001
> REGS: c3c0bd10 TRAP: 0700   Not tainted  (2.6.22.9)
> MSR: 00029032 <EE,ME,IR,DR>  CR: 30099099  XER: a0008c7f
> TASK = c0292b40[103] 'init' THREAD: c3c0a000
> GPR00: 00000001 c3c0bdc0 c0292b40 ff800fff ff800fff c3c0bdf0 00000000 
> 00000000
> GPR08: c0219398 c017d91c 00000002 c0179cd0 30099093 1007687c 00000002 
> c00f8744
> GPR16: 00000000 c00f0a64 c011d1ac c00f0aa4 c00f0a90 c0120000 00000001 
> 00000003
> GPR24: c3c32e00 00000000 c0180000 c0247080 00000000 c3c0bdc8 c0179cd0 
> c017641c
> NIP [c0045fd4] remove_vma+0x10/0x18
> LR [c00460a0] exit_mmap+0xc4/0xf0
> Call Trace:
> [c3c0bdc0] [c0046074] exit_mmap+0x98/0xf0 (unreliable)
> [c3c0bdf0] [c000f74c] mmput+0x50/0xd4
> [c3c0be00] [c005920c] flush_old_exec+0x3b8/0x7a8
> [c3c0be50] [c0086cd8] load_elf_binary+0x2e8/0x1454
> [c3c0bee0] [c0058944] search_binary_handler+0x58/0x12c
> [c3c0bf00] [c0059d7c] do_execve+0x13c/0x1f0
> [c3c0bf20] [c00089b4] sys_execve+0x50/0x90
> [c3c0bf40] [c0002a40] ret_from_syscall+0x0/0x38
> Instruction dump:
> 7fe4fb78 4800a0ed 80010014 7fc3f378 7c0803a6 bbc10008 38210010 4e800020
> 3c80ff80 60840fff 7c832008 80a10000 <7c832008> 4bffff7c 7c0802a6 9421ffd0
> 
> The access of memory through r1 seems to corrupt r3, and always with the 
> same value. The problem isn't necessarily here, though. If I modify my 
> remove_vma function to cause and correct the problem (by saving r3 prior 
> to the memory access and restoring it afterwards), I just get the same 
> problem in some other part of the code, but the oops is always caused 
> because the base register for some memory access is set to ff800fff.
> 
> I applied a recent patch I found that corrects the address returned by 
> cpm_dpram_addr and its use in cpu_uart_cpm1.h, and I've created my own 
> platform setup file by copying the mpc866ads setup enough to get the 
> console uart (SMC1) to work.
> 
> If there is any other information I can or need to provide, let me 
> know. Any help would be greatly appreciated.
> 
> Thanks,
> John

-- 
John Tyner
jtyner@cs.ucr.edu

^ permalink raw reply

* shekr06@lzu.cn;puyq@lzu.cn;zhang_wei@lzu.cn
From: Bai Shuwei @ 2007-11-24 15:19 UTC (permalink / raw)
  To: linuxppc-embedded, linuxppc-dev

[-- Attachment #1: Type: text/plain, Size: 718 bytes --]

hi, all
    I bought a SMC1500 stepper motor card. And it can connect with host
through parallel port. My target board is PowerPC 440, which hasn't
parrallel port. So I bought a PCI to Parallel line for SMC1500. But when I
run the stepper motor, I find it's not stable. I  doubt there are somthing
wrong with my PCI to Parallel line. So I beg somebody can tell me where I
can bought the  appropriate conversion line from PCI to parallel, and does
somebody give me some idea about how to control my stepper motor
through PowerPC 440? thx all

Best Regards!

Buroc

-- 

Add: Tianshui South Road 222, Lanzhou, P.R.China
Tel: +86-931-8912025
Zip Code: 730000
URL: oss.lzu.edu.cn
Email: baishuwei@gmail.com, buroc@126.com

[-- Attachment #2: Type: text/html, Size: 1016 bytes --]

^ permalink raw reply

* Re: Xilinx devicetrees
From: Grant Likely @ 2007-11-24 17:12 UTC (permalink / raw)
  To: David H. Lynch Jr.; +Cc: linuxppc-embedded
In-Reply-To: <47480CF0.7090105@dlasys.net>

On 11/24/07, David H. Lynch Jr. <dhlii@dlasys.net> wrote:
>     I am following developments regarding device trees for xilinx boards
> both here and on the microblaze list.
>
>     I am trying to get a grasp on what they will really do for me and
> what using them will demand.
>
>     Please correct any misperceptions:
>
>      As I understand it devicetrees are basically just a tree structured
> binary  database describing the hardware.
>     They have some heritage in OpenFirmware.
>     There are tools to convert  some human readable representations into
> the binary form.
>     There appear to be tools to take xilinx firmware projects and create
> a devicetree database from it
>     A BSP using devicetree's configures its hardware, drivers etc, by
> querying the devicetree database.
>     It it possible to pass the device tree database independent of the
> kernel itself some what similar to the way many bootloaders pass initrd
> filesystems.

Yes, you are correct in all of the above.

One more point; it is also possible to bind the device tree up with
the kernel so you've got a single image just like old times.  :-)

>
>     So in the end I write a BSP that could support a wide variety of
> hardware and compile a single kernel that could be passed different
> devicetree databases representing different xilinx firmware, and still
> hope to work.
>     But in return for making the BSP more generic (sort of), I now have
> to somehow get the correct devicetree database passed for each different
> firmware set that I load.

Yes; either by changes the device tree blob; or having a different
kernel+device tree image for each FPGA bitstream.  In the later case,
the kernel can be compiled once and then bound to multiple dt blobs
(creating multiple images)

>
>     I am having some difficulty grasping the significant advantages to
> this.
>     If the firmware for a given target is not fairly static - and I load
> different firmware depending on what I am doing all the time, then I
> know have to manage both a bit file for the firmware, and a devicetree
> file describing it to the kernel.
>     Currently for base hardware we maintain as much design consistancy
> as possible accross all our different cards/firmware.
>     particular hardware/firmware blocks/IP's may or may not be present -
> but if present they are always the same - the Same Uartlite at the same
> location, on the same irq, same for PIC's, TEMAC's ...
>     For the most part it makes the most sense for us to use code to
> detect the presence/absence of specific baseline hardware and then to
> load non-base custom drivers after boot.

The board description has to live *somewhere*.  For powerpc (and
microblaze) we've decided that for generic stuff, it makes sense to
use the device tree data structure to describe the hardware.  It makes
the platform code simpler because the platform code no longer needs to
explicitly instantiate drivers for each device on the board.  Instead
it registers part of the device tree with the of_platform bus and lets
the drivers handle binding themselves.

That being said, using the device tree does not preclude using
platform code to setup devices *where it makes sense to do so*.  Many
things are one-off board specific things that are not well described
with the device tree.  For example, we've been debating how to handle
on board sound which use common codec chips, but have custom wireup.
The codec chip can use a common driver, but the wireup is handled with
an ALSA 'fabric' driver that is pretty much a one-off for the board.
It probably makes more sense for the fabric driver to be instantiated
by the platform code rather than trying to do a full device tree
representation.

>
>     What am  missing about devicetrees that would make me more
> interested in them ?

To put it all in perspective, for Virtex support in arch/ppc
registration of devices is data driven.  Look at
arch/ppc/syslib/virtex_devices.c which contains a table of platform
devices which the Virtex platform code iterates over.  In arch/powerpc
we're *still* data driven; it's just that the data is no longer
compiled into a static structure.  Plus, in the transition we've moved
from needed per-device platform data structures to a more expressive
format.  Also, per-board platform support code has become much simpler
(compare ml300.c in arch/ppc with platforms/40x/virtex.c)

Cheers,
g.


-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

^ permalink raw reply

* Re: [linux-usb-devel] [PATCH 1/5] USB: Make usb_hcd_irq work for multi-role USB controllers w/ shared irq
From: Alan Cox @ 2007-11-24 17:03 UTC (permalink / raw)
  To: Greg KH; +Cc: akpm, dbrownell, linux-usb-devel, linuxppc-dev
In-Reply-To: <20071124051039.GA11029@suse.de>

On Fri, 23 Nov 2007 21:10:39 -0800
Greg KH <gregkh@suse.de> wrote:

> On Fri, Nov 23, 2007 at 05:24:31PM -0700, Grant Likely wrote:
> > From: Peter Korsgaard <peter.korsgaard@barco.com>
> > 
> > Some multi-role (host/peripheral) USB controllers use a shared interrupt
> > line for all parts of the chip. Export usb_hcd_irq so drivers can call it
> > from their interrupt handler instead of duplicating code.
> > Drivers pass an irqnum of 0 to usb_add_hcd to signal that the interrupt handler
> > shouldn't be registerred by the core.
> 
> What about for platforms where irq 0 is a valid irq?

There are no such platforms. Linus made that absolutely clear every time
this came up before

	0	-	No IRQ

A platform with a physical or bus IRQ of 0 needs to remap it to a
different constant.

Alan

^ permalink raw reply

* Re: [linux-usb-devel] [PATCH 1/5] USB: Make usb_hcd_irq work for multi-role USB controllers w/ shared irq
From: Grant Likely @ 2007-11-24 17:15 UTC (permalink / raw)
  To: Alan Cox; +Cc: akpm, linuxppc-dev, Greg KH, linux-usb-devel, dbrownell
In-Reply-To: <20071124170307.773d7317@the-village.bc.nu>

On 11/24/07, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> On Fri, 23 Nov 2007 21:10:39 -0800
> Greg KH <gregkh@suse.de> wrote:
>
> > On Fri, Nov 23, 2007 at 05:24:31PM -0700, Grant Likely wrote:
> > > From: Peter Korsgaard <peter.korsgaard@barco.com>
> > >
> > > Some multi-role (host/peripheral) USB controllers use a shared interrupt
> > > line for all parts of the chip. Export usb_hcd_irq so drivers can call it
> > > from their interrupt handler instead of duplicating code.
> > > Drivers pass an irqnum of 0 to usb_add_hcd to signal that the interrupt handler
> > > shouldn't be registerred by the core.
> >
> > What about for platforms where irq 0 is a valid irq?
>
> There are no such platforms. Linus made that absolutely clear every time
> this came up before
>
>         0       -       No IRQ
>
> A platform with a physical or bus IRQ of 0 needs to remap it to a
> different constant.

Regardless, I should probably use the NO_IRQ macro instead.

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

^ permalink raw reply

* Re: [linux-usb-devel] [PATCH 1/5] USB: Make usb_hcd_irq work for multi-role USB controllers w/ shared irq
From: David Brownell @ 2007-11-24 17:50 UTC (permalink / raw)
  To: Alan Cox; +Cc: akpm, Greg KH, linux-usb-devel, linuxppc-dev
In-Reply-To: <20071124170307.773d7317@the-village.bc.nu>

On Saturday 24 November 2007, Alan Cox wrote:
> > 
> > What about for platforms where irq 0 is a valid irq?
> 
> There are no such platforms. Linus made that absolutely clear every time
> this came up before
> 
>         0       -       No IRQ
> 
> A platform with a physical or bus IRQ of 0 needs to remap it to a
> different constant.

However it's also common practice to use negative numbers to
flag "this is no IRQ" ... avoiding all confusions with zero.

  - platform_get_irq(), platform_get_irq_byname() ... never
    return zero, they return irq (positive) or errno

  - PNP initializes invalid IRQs to "-1", and pnp_check_irq()
    handles irq zero as in-range

  - I'm sure I've seen negative numbers used elsewhere too

Something like

  #define is_valid_irq(x) ((x) >= 0)

would work better than expecting sudden agreement everywhere
about a single number representing "this is not an IRQ".

- Dave

^ permalink raw reply

* Re: [RFC/PATCH] powerpc: Move CPM command handling into the cpm drivers
From: Jochen Friedrich @ 2007-11-24 17:53 UTC (permalink / raw)
  To: Vitaly Bordug; +Cc: linuxppc-dev
In-Reply-To: <20071123005121.4d38d877@kernel.crashing.org>

Hi Vitaly,

>>> +       printk(KERN_ERR "%s(): Not able to issue CPM command\n",
>>> +               __FUNCTION__);
>>> +       return -EIO;  
>>>       
>> Do these need to be protected with a spin lock?
>>     
> Even that might be not enough - we may have simultaneous call of this func in non-smp case...
> I was thinking of some kind of refcount, so one that is going to issue CPM command, must do say pq_cpmp_get()
> and another driver won't be able to mangle with cpcr while it's not done with previous request.
>
> Yet I am not telling it was better the way it used to be - this approach looks okay but needs some efforts to defend against
> deadlocks while we are at it

Wouldn't spin_lock_irqsave() prevent a deadlock?

Thanks,
Jochen

^ permalink raw reply

* Re: [PATCH 4/5] PowerPC 74xx: Katana Qp base support
From: Arnd Bergmann @ 2007-11-24 18:51 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <20071116163116.GE25062@ru.mvista.com>

On Friday 16 November 2007, Andrei Dolnikov wrote:

> +static int __init katanaqp_is_monarch(void)
> +{
> +	return !(in_8((volatile char *)(cpld_base + KATANAQP_CPLD_PSR)) &
> +		 KATANAQP_CPLD_PSR_PMCM);
> +}

The pointer here needs to be __iomem, not volatile. Same in other places.
Please use 'sparse' to check your code for bugs like this.

> +
> +static void __init katanaqp_setup_arch(void)
> +{
> +	struct device_node *cpld;
> +	const unsigned int *reg;
> +
> +	/*
> +	 * ioremap cpld registers in case they are later
> +	 * needed by katanaqp_reset_board().
> +	 */
> +	cpld = of_find_node_by_path("/mv64x60@f8100000/cpld@f8200000");

It doesn't sounds good to hardcode the path for this device.
Instead, it would be much better to look for the 'compatible' property
here.

> +static int __init katanaqp_of_init(void)
> +{
> +	struct device_node *np;
> +
> +	np = of_find_compatible_node(NULL, NULL, "cfi-flash");
> +	if (np)
> +		of_platform_device_create(np, "of-flash", NULL);
> +
> +	return 0;
> +}
> +
> +device_initcall(katanaqp_of_init);

This should be done automatically using of_platform_bus_probe().

	Arnd <><

^ permalink raw reply

* Re: [PATCH 0/5] Review request: Cypress c67x00 OTG controller
From: David Brownell @ 2007-11-24 17:57 UTC (permalink / raw)
  To: Grant Likely; +Cc: akpm, linuxppc-dev, gregkh, linux-usb-devel
In-Reply-To: <20071124001203.25361.99294.stgit@trillian.cg.shawcable.net>

On Friday 23 November 2007, Grant Likely wrote:
> 
> This patch series is based on the c67x00 work done by Peter Korsgaard and
> posted back in April this year.

What's changed since that version?  Were the comments sent
at that time addressed?

^ permalink raw reply

* Re: [PATCH 1/5] USB: Make usb_hcd_irq work for multi-role USB controllers w/ shared irq
From: David Brownell @ 2007-11-24 19:06 UTC (permalink / raw)
  To: Grant Likely; +Cc: akpm, linuxppc-dev, gregkh, linux-usb-devel
In-Reply-To: <20071124002431.25361.23974.stgit@trillian.cg.shawcable.net>

On Friday 23 November 2007, Grant Likely wrote:
> Some multi-role (host/peripheral) USB controllers use a shared interrupt
> line for all parts of the chip.

Like the musb_hdrc code ... soonish to go upstream (it needs some
updates to catch up to usbcore urb->status changes), this is used
by the Nokia 800 and 810.  In terms of chips with Linux support:
DaVinci, TUSB60x0, OMAP 2430, OMAP 3430, Blackfin BF527; and ISTR
a few less-publicised ones (including, yes, some PPC SOCs).

That driver hasn't needed to change usbcore for IRQ handling though.


> Export usb_hcd_irq so drivers can call it 
> from their interrupt handler instead of duplicating code.

This seems to be the main point of this patch.  I'd rather just
make that "static" though; it should already be marked that way.

That routine doesn't do enough to make me like it any more; and
with dual-role controllers, the driver lifecycle is more complex
than usbcore can be expected to mediate.  Best to just call the
host side IRQ logic directly from your toplevel IRQ handler.


> Drivers pass an irqnum of 0 to usb_add_hcd to signal that the interrupt handler
> shouldn't be registerred by the core.

The current way to get that behavior is to leave hcd->driver->irq
as zero; then "irqnum" is ignored, and your dual role driver can
register its own handler.

- Dave

^ permalink raw reply

* [RFC/PATCHv2] powerpc: Move CPM command handling into the cpm drivers
From: Jochen Friedrich @ 2007-11-24 19:16 UTC (permalink / raw)
  To: linuxppc-dev

This patch moves the CPM command handling into commproc.c
for CPM1 and cpm2_common.c. This is yet another preparation
to get rid of drivers accessing the CPM via the global cpmp.

Signed-off-by: Jochen Friedrich <jochen@scram.de>
---
 arch/powerpc/sysdev/commproc.c          |   27 +++++++++++++++++++++++++++
 arch/powerpc/sysdev/cpm2_common.c       |   24 ++++++++++++++++++++++++
 drivers/net/fs_enet/mac-fcc.c           |   10 +---------
 drivers/net/fs_enet/mac-scc.c           |   11 +----------
 drivers/serial/cpm_uart/cpm_uart_cpm1.c |    6 +-----
 drivers/serial/cpm_uart/cpm_uart_cpm2.c |    8 +-------
 include/asm-powerpc/cpm.h               |    1 +
 7 files changed, 56 insertions(+), 31 deletions(-)

diff --git a/arch/powerpc/sysdev/commproc.c b/arch/powerpc/sysdev/commproc.c
index f6a6378..4cc245f 100644
--- a/arch/powerpc/sysdev/commproc.c
+++ b/arch/powerpc/sysdev/commproc.c
@@ -240,6 +240,33 @@ void __init cpm_reset(void)
 #endif
 }
 
+DEFINE_SPINLOCK(cmd_lock);
+
+#define MAX_CR_CMD_LOOPS        10000
+
+int cpm_command(u32 command, u8 opcode)
+{
+	int i;
+	unsigned long flags;
+
+	if (command & 0xffffff0f)
+		return -EINVAL;
+
+	spin_lock_irqsave(&cmd_lock, flags);
+
+	out_be16(&cpmp->cp_cpcr, command | CPM_CR_FLG | (opcode << 8));
+	for (i = 0; i < MAX_CR_CMD_LOOPS; i++)
+		if ((in_be16(&cpmp->cp_cpcr) & CPM_CR_FLG) == 0) {
+			spin_unlock_irqrestore(&cmd_lock, flags);
+			return 0;
+		}
+
+	printk(KERN_ERR "%s(): Not able to issue CPM command\n", __FUNCTION__);
+	spin_unlock_irqrestore(&cmd_lock, flags);
+	return -EIO;
+}
+EXPORT_SYMBOL(cpm_command);
+
 /* We used to do this earlier, but have to postpone as long as possible
  * to ensure the kernel VM is now running.
  */
diff --git a/arch/powerpc/sysdev/cpm2_common.c b/arch/powerpc/sysdev/cpm2_common.c
index 859362f..db282bc 100644
--- a/arch/powerpc/sysdev/cpm2_common.c
+++ b/arch/powerpc/sysdev/cpm2_common.c
@@ -83,6 +83,30 @@ cpm2_reset(void)
 	cpmp = &cpm2_immr->im_cpm;
 }
 
+DEFINE_SPINLOCK(cmd_lock);
+
+#define MAX_CR_CMD_LOOPS        10000
+
+int cpm_command(u32 command, u8 opcode)
+{
+	int i;
+	unsigned long flags;
+
+	spin_lock_irqsave(&cmd_lock, flags);
+
+	out_be32(&cpmp->cp_cpcr, command | opcode | CPM_CR_FLG);
+	for (i = 0; i < MAX_CR_CMD_LOOPS; i++)
+		if ((in_be32(&cpmp->cp_cpcr) & CPM_CR_FLG) == 0) {
+			spin_unlock_irqrestore(&cmd_lock, flags);
+			return 0;
+		}
+
+	printk(KERN_ERR "%s(): Not able to issue CPM command\n", __FUNCTION__);
+	spin_unlock_irqrestore(&cmd_lock, flags);
+	return -EIO;
+}
+EXPORT_SYMBOL(cpm_command);
+
 /* Set a baud rate generator.  This needs lots of work.  There are
  * eight BRGs, which can be connected to the CPM channels or output
  * as clocks.  The BRGs are in two different block of internal
diff --git a/drivers/net/fs_enet/mac-fcc.c b/drivers/net/fs_enet/mac-fcc.c
index da4efbc..e363211 100644
--- a/drivers/net/fs_enet/mac-fcc.c
+++ b/drivers/net/fs_enet/mac-fcc.c
@@ -81,16 +81,8 @@
 static inline int fcc_cr_cmd(struct fs_enet_private *fep, u32 op)
 {
 	const struct fs_platform_info *fpi = fep->fpi;
-	int i;
-
-	W32(cpmp, cp_cpcr, fpi->cp_command | op | CPM_CR_FLG);
-	for (i = 0; i < MAX_CR_CMD_LOOPS; i++)
-		if ((R32(cpmp, cp_cpcr) & CPM_CR_FLG) == 0)
-			return 0;
 
-	printk(KERN_ERR "%s(): Not able to issue CPM command\n",
-	       __FUNCTION__);
-	return 1;
+	return cpm_command(fpi->cp_command, op);
 }
 
 static int do_pd_setup(struct fs_enet_private *fep)
diff --git a/drivers/net/fs_enet/mac-scc.c b/drivers/net/fs_enet/mac-scc.c
index 03134f4..5ff856d 100644
--- a/drivers/net/fs_enet/mac-scc.c
+++ b/drivers/net/fs_enet/mac-scc.c
@@ -89,21 +89,12 @@
  * Delay to wait for SCC reset command to complete (in us)
  */
 #define SCC_RESET_DELAY		50
-#define MAX_CR_CMD_LOOPS	10000
 
 static inline int scc_cr_cmd(struct fs_enet_private *fep, u32 op)
 {
 	const struct fs_platform_info *fpi = fep->fpi;
-	int i;
-
-	W16(cpmp, cp_cpcr, fpi->cp_command | CPM_CR_FLG | (op << 8));
-	for (i = 0; i < MAX_CR_CMD_LOOPS; i++)
-		if ((R16(cpmp, cp_cpcr) & CPM_CR_FLG) == 0)
-			return 0;
 
-	printk(KERN_ERR "%s(): Not able to issue CPM command\n",
-		__FUNCTION__);
-	return 1;
+	return cpm_command(fpi->cp_command, op);
 }
 
 static int do_pd_setup(struct fs_enet_private *fep)
diff --git a/drivers/serial/cpm_uart/cpm_uart_cpm1.c b/drivers/serial/cpm_uart/cpm_uart_cpm1.c
index 52fb044..6ea0366 100644
--- a/drivers/serial/cpm_uart/cpm_uart_cpm1.c
+++ b/drivers/serial/cpm_uart/cpm_uart_cpm1.c
@@ -52,11 +52,7 @@
 #ifdef CONFIG_PPC_CPM_NEW_BINDING
 void cpm_line_cr_cmd(struct uart_cpm_port *port, int cmd)
 {
-	u16 __iomem *cpcr = &cpmp->cp_cpcr;
-
-	out_be16(cpcr, port->command | (cmd << 8) | CPM_CR_FLG);
-	while (in_be16(cpcr) & CPM_CR_FLG)
-		;
+	cpm_command(port->command, cmd);
 }
 #else
 void cpm_line_cr_cmd(struct uart_cpm_port *port, int cmd)
diff --git a/drivers/serial/cpm_uart/cpm_uart_cpm2.c b/drivers/serial/cpm_uart/cpm_uart_cpm2.c
index 882dbc1..def0158 100644
--- a/drivers/serial/cpm_uart/cpm_uart_cpm2.c
+++ b/drivers/serial/cpm_uart/cpm_uart_cpm2.c
@@ -52,13 +52,7 @@
 #ifdef CONFIG_PPC_CPM_NEW_BINDING
 void cpm_line_cr_cmd(struct uart_cpm_port *port, int cmd)
 {
-	cpm_cpm2_t __iomem *cp = cpm2_map(im_cpm);
-
-	out_be32(&cp->cp_cpcr, port->command | cmd | CPM_CR_FLG);
-	while (in_be32(&cp->cp_cpcr) & CPM_CR_FLG)
-		;
-
-	cpm2_unmap(cp);
+	cpm_command(port->command, cmd);
 }
 #else
 void cpm_line_cr_cmd(struct uart_cpm_port *port, int cmd)
diff --git a/include/asm-powerpc/cpm.h b/include/asm-powerpc/cpm.h
index 48df9f3..fae83b1 100644
--- a/include/asm-powerpc/cpm.h
+++ b/include/asm-powerpc/cpm.h
@@ -10,5 +10,6 @@ int cpm_muram_free(unsigned long offset);
 unsigned long cpm_muram_alloc_fixed(unsigned long offset, unsigned long size);
 void __iomem *cpm_muram_addr(unsigned long offset);
 dma_addr_t cpm_muram_dma(void __iomem *addr);
+int cpm_command(u32 command, u8 opcode);
 
 #endif
-- 
1.5.3.5

^ permalink raw reply related

* Re: [PATCH 0/5] Review request: Cypress c67x00 OTG controller
From: Grant Likely @ 2007-11-24 19:27 UTC (permalink / raw)
  To: David Brownell; +Cc: akpm, linuxppc-dev, gregkh, linux-usb-devel
In-Reply-To: <200711240957.35385.david-b@pacbell.net>

On 11/24/07, David Brownell <david-b@pacbell.net> wrote:
> On Friday 23 November 2007, Grant Likely wrote:
> >
> > This patch series is based on the c67x00 work done by Peter Korsgaard and
> > posted back in April this year.
>
> What's changed since that version?  Were the comments sent
> at that time addressed?

A lot has changes since that version.  I'm not sure if all the
comments at that time were addressed (as in it's been a while since
I've looked at them; I'll go back and take another look).

You can see my progression from Peter's version and my version here:

http://git.secretlab.ca/git/gitweb.cgi?p=linux-2.6-virtex.git;a=shortlog;h=virtex-c67x00-dev

The patches I posted are the consolidation of the series in that git tree.

Cheers,
g.



-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

^ permalink raw reply

* Re: [RFC/PATCHv2] powerpc: Move CPM command handling into the cpm drivers
From: Arnd Bergmann @ 2007-11-24 19:27 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <4748788C.7020706@scram.de>

On Saturday 24 November 2007, Jochen Friedrich wrote:
> This patch moves the CPM command handling into commproc.c
> for CPM1 and cpm2_common.c. This is yet another preparation
> to get rid of drivers accessing the CPM via the global cpmp.

good stuff, just a little nitpicking below:

> +DEFINE_SPINLOCK(cmd_lock);

This should probably be a static variable. The name is too generic
for a global identifier. If every use of the command register
goes through the cpm_command function, you could even make it
a static variable in that function.

> +int cpm_command(u32 command, u8 opcode)
> +{
> +	int i;
> +	unsigned long flags;
> +
> +	if (command & 0xffffff0f)
> +		return -EINVAL;
> +
> +	spin_lock_irqsave(&cmd_lock, flags);
> +
> +	out_be16(&cpmp->cp_cpcr, command | CPM_CR_FLG | (opcode << 8));
> +	for (i = 0; i < MAX_CR_CMD_LOOPS; i++)
> +		if ((in_be16(&cpmp->cp_cpcr) & CPM_CR_FLG) == 0) {
> +			spin_unlock_irqrestore(&cmd_lock, flags);
> +			return 0;
> +		}

Error handling in here is nicer if you do a goto that jumps to the
unlock below. If the code ever gets more complex, this makes it easier
to check if it's correct regarding locking.

> +	printk(KERN_ERR "%s(): Not able to issue CPM command\n", __FUNCTION__);
> +	spin_unlock_irqrestore(&cmd_lock, flags);
> +	return -EIO;
> +}
> +EXPORT_SYMBOL(cpm_command);

Why not EXPORT_SYMBOL_GPL?
  
> +DEFINE_SPINLOCK(cmd_lock);

see above

> +
> +#define MAX_CR_CMD_LOOPS        10000
> +
> +int cpm_command(u32 command, u8 opcode)
> +{
> +	int i;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&cmd_lock, flags);
> +
> +	out_be32(&cpmp->cp_cpcr, command | opcode | CPM_CR_FLG);
> +	for (i = 0; i < MAX_CR_CMD_LOOPS; i++)
> +		if ((in_be32(&cpmp->cp_cpcr) & CPM_CR_FLG) == 0) {
> +			spin_unlock_irqrestore(&cmd_lock, flags);
> +			return 0;
> +		}
> +
> +	printk(KERN_ERR "%s(): Not able to issue CPM command\n", __FUNCTION__);
> +	spin_unlock_irqrestore(&cmd_lock, flags);
> +	return -EIO;
> +}
> +EXPORT_SYMBOL(cpm_command);

see above

	Arnd <><

^ permalink raw reply

* Re: [PATCH 1/5] USB: Make usb_hcd_irq work for multi-role USB controllers w/ shared irq
From: Grant Likely @ 2007-11-24 19:28 UTC (permalink / raw)
  To: David Brownell; +Cc: akpm, linuxppc-dev, gregkh, linux-usb-devel
In-Reply-To: <200711241106.46515.david-b@pacbell.net>

On 11/24/07, David Brownell <david-b@pacbell.net> wrote:
> On Friday 23 November 2007, Grant Likely wrote:
> > Some multi-role (host/peripheral) USB controllers use a shared interrupt
> > line for all parts of the chip.
>
> Like the musb_hdrc code ... soonish to go upstream (it needs some
> updates to catch up to usbcore urb->status changes), this is used
> by the Nokia 800 and 810.  In terms of chips with Linux support:
> DaVinci, TUSB60x0, OMAP 2430, OMAP 3430, Blackfin BF527; and ISTR
> a few less-publicised ones (including, yes, some PPC SOCs).
>
> That driver hasn't needed to change usbcore for IRQ handling though.
>
>
> > Export usb_hcd_irq so drivers can call it
> > from their interrupt handler instead of duplicating code.
>
> This seems to be the main point of this patch.  I'd rather just
> make that "static" though; it should already be marked that way.
>
> That routine doesn't do enough to make me like it any more; and
> with dual-role controllers, the driver lifecycle is more complex
> than usbcore can be expected to mediate.  Best to just call the
> host side IRQ logic directly from your toplevel IRQ handler.
>
>
> > Drivers pass an irqnum of 0 to usb_add_hcd to signal that the interrupt handler
> > shouldn't be registerred by the core.
>
> The current way to get that behavior is to leave hcd->driver->irq
> as zero; then "irqnum" is ignored, and your dual role driver can
> register its own handler.

Okay, I'll make that change.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

^ permalink raw reply

* Re: [PATCH 2/5] USB: Add Cypress c67x00 low level interface code
From: David Brownell @ 2007-11-24 19:39 UTC (permalink / raw)
  To: Grant Likely; +Cc: akpm, linuxppc-dev, gregkh, linux-usb-devel
In-Reply-To: <20071124002436.25361.50445.stgit@trillian.cg.shawcable.net>

On Friday 23 November 2007, Grant Likely wrote:
> +/* These functions could also be implemented with SPI of HSS.
> + * This is currently not supported */

Give that this "HPI" interface seems to use a parallel bus
with irq-safe synchronous accesses, and SPI is a serial bus
where synchronous accesses can't be done in IRQ context ...
that comment doesn't seem accurate.

Surely a more correct statement would be that while the chip
can also be accessed using SPI, that would require updating
the driver structure to stop assuming register accesses can
always be done from IRQ context?

If that's not the case, then those spinlocks are overkill,
and they should become mutexes.  :)

- Dave

^ permalink raw reply

* Re: [PATCH 5/5] USB: Add Cypress c67x00 OTG controller driver to Kconfig and Makefiles
From: David Brownell @ 2007-11-24 20:12 UTC (permalink / raw)
  To: Grant Likely; +Cc: akpm, linuxppc-dev, gregkh, linux-usb-devel
In-Reply-To: <20071124002451.25361.90102.stgit@trillian.cg.shawcable.net>

On Friday 23 November 2007, Grant Likely wrote:
> +config USB_C67X00_DRV
> +       tristate "Cypress C67x00 support"
> +       # only allowed to be =y if both USB!=m and USB_GADGET!=m

This is wrong.  Remember that since this is a dual-role driver,
there are exactly three possible driver modes ... and the driver
must support only one of them at a time:

  - Host-only ... only allowed if host side USB is enabled.  The
    controller driver can be statically linked iff usbcore is too.

  - Peripheral-only ... only allowed if peripheral side USB is
    enabled.  Only one port may be used as the peripheral; the
    controller driver can be linked statically or as a module.

  - OTG ... only allowed if both host and peripheral side USB
    are enabled.  Only one port can be the OTG port, but other
    ports can be used for host functionality.  Static/modular
    linkage follows (more restrictive) the host-only rule.

And of course, what a given board supports is controlled by the
connectors it has ... A, B, or AB.  (Possibly AB plus n*A.)  So
the driver should probably be able to make sense of having OTG
support, but needing to cope with a non-OTG board ... or having
an OTG board, a driver that only copes with one role.

Hmmm ... your patches don't include peripheral mode support yet.

Either all these gadget hooks should vanish, or you should try
to get the controller mode stuff right from the beginning.

I've appended the relevant Kconfig bits from the musb_hdrc
driver, which (despite some glitches) are pretty much correct.

- Dave



> +       depends on (!USB && USB_GADGET) || (!USB_GADGET && USB) || (USB && USB_GADGET)
> +       default n
> +       help
> +         The Cypress C67x00 (EZ-Host/EZ-OTG) chips are dual-role
> +         host/peripheral USB controllers.
> +
> +         To compile this driver as a module, choose M here: the
> +         module will be called c67x00.
> +
> +config USB_C67X00_HCD
> +       bool "Cypress C67X00 Host support"
> +       depends on USB_C67X00_DRV
> +       depends on USB
> +       default y
> +       help
> +         This enables the host functionality of the Cypress C67X00.
> 


================
#
# USB Dual Role (OTG-ready) Controller Drivers
# for silicon based on Mentor Graphics INVENTRA designs
#

comment "Enable Host or Gadget support to see Inventra options"
	depends on !USB && USB_GADGET=n

# (M)HDRC = (Multipoint) Highspeed Dual-Role Controller
config USB_MUSB_HDRC
	depends on USB || USB_GADGET
	tristate 'Inventra Highspeed Dual Role Controller (TI, ...)'
	help
	  Say Y here if your system has a dual role high speed USB
	  controller based on the Mentor Graphics silicon IP.  Then
	  configure options to match your silicon and the board
	  it's being used with, including the USB peripheral role,
	  or the USB host role, or both.

	  Texas Instruments parts using this IP include DaVinci 644x,
	  OMAP 243x, OMAP 343x, and TUSB 6010.

	  If you do not know what this is, please say N.

	  To compile this driver as a module, choose M here; the
	  module will be called "musb_hdrc".

config USB_MUSB_SOC
	boolean
	depends on USB_MUSB_HDRC
	default y if ARCH_DAVINCI
	default y if ARCH_OMAP2430
	default y if ARCH_OMAP343X
	help
	  Use a static <asm/arch/hdrc_cnf.h> file to describe how the
	  controller is configured (endpoints, mechanisms, etc) on the
	  current iteration of a given system-on-chip.

comment "DaVinci 644x USB support"
	depends on USB_MUSB_HDRC && ARCH_DAVINCI

comment "OMAP 243x high speed USB support"
	depends on USB_MUSB_HDRC && ARCH_OMAP2430

comment "OMAP 343x high speed USB support"
	depends on USB_MUSB_HDRC && ARCH_OMAP343X

config USB_TUSB6010
	boolean "TUSB 6010 support"
	depends on USB_MUSB_HDRC && !USB_MUSB_SOC
	default y
	help
	  The TUSB 6010 chip, from Texas Instruments, connects a discrete
	  HDRC core using a 16-bit parallel bus.  It can use system-specific
	  external DMA controllers.

choice
	prompt "Driver Mode"
	depends on USB_MUSB_HDRC
	help
	  Dual-Role devices can support both host and peripheral roles,
	  as well as a the special "OTG Device" role which can switch
	  between both roles as needed.

# use USB_MUSB_HDRC_HCD not USB_MUSB_HOST to #ifdef host side support;
# OTG needs both roles, not just USB_MUSB_HOST.
config USB_MUSB_HOST
	depends on USB
	bool "USB Host"
	help
	  Say Y here if your system supports the USB host role.
	  If it has a USB "A" (rectangular), "Mini-A" (uncommon),
	  or "Mini-AB" connector, it supports the host role.
	  (With a "Mini-AB" connector, you should enable USB OTG.)

# use USB_GADGET_MUSB_HDRC not USB_MUSB_PERIPHERAL to #ifdef peripheral
# side support ... OTG needs both roles
config USB_MUSB_PERIPHERAL
	depends on USB_GADGET
	bool "USB Peripheral (gadget stack)"
	select USB_GADGET_MUSB_HDRC
	help
	  Say Y here if your system supports the USB peripheral role.
	  If it has a USB "B" (squarish), "Mini-B", or "Mini-AB"
	  connector, it supports the peripheral role.
	  (With a "Mini-AB" connector, you should enable USB OTG.)

config USB_MUSB_OTG
	depends on USB && USB_GADGET && EXPERIMENTAL
	bool "Both host and peripheral:  USB OTG (On The Go) Device"
	select USB_GADGET_MUSB_HDRC
	select USB_OTG
	select PM
	help
	   The most notable feature of USB OTG is support for a
	   "Dual-Role" device, which can act as either a device
	   or a host.  The initial role choice can be changed
	   later, when two dual-role devices talk to each other.

	   Select this if your system has a Mini-AB connector, or
	   to simplify certain kinds of configuration.

	   To implement your OTG Targeted Peripherals List (TPL), enable
	   USB_OTG_WHITELIST and update "drivers/usb/core/otg_whitelist.h"
	   to match your requirements.

endchoice

# enable peripheral support (including with OTG)
config USB_GADGET_MUSB_HDRC
	bool
	depends on USB_MUSB_HDRC && (USB_MUSB_PERIPHERAL || USB_MUSB_OTG)

# enables host support (including with OTG)
config USB_MUSB_HDRC_HCD
	bool
	depends on USB_MUSB_HDRC && (USB_MUSB_HOST || USB_MUSB_OTG)
	select USB_OTG if USB_GADGET_MUSB_HDRC
	default y

^ permalink raw reply

* [RFC/PATCHv3] powerpc: Move CPM command handling into the cpm drivers
From: Jochen Friedrich @ 2007-11-24 20:17 UTC (permalink / raw)
  To: linuxppc-dev

This patch moves the CPM command handling into commproc.c
for CPM1 and cpm2_common.c. This is yet another preparation
to get rid of drivers accessing the CPM via the global cpmp.

Signed-off-by: Jochen Friedrich <jochen@scram.de>
---
 arch/powerpc/sysdev/commproc.c          |   28 ++++++++++++++++++++++++++++
 arch/powerpc/sysdev/cpm2_common.c       |   25 +++++++++++++++++++++++++
 drivers/net/fs_enet/mac-fcc.c           |   10 +---------
 drivers/net/fs_enet/mac-scc.c           |   11 +----------
 drivers/serial/cpm_uart/cpm_uart_cpm1.c |    6 +-----
 drivers/serial/cpm_uart/cpm_uart_cpm2.c |    8 +-------
 include/asm-powerpc/cpm.h               |    1 +
 7 files changed, 58 insertions(+), 31 deletions(-)

diff --git a/arch/powerpc/sysdev/commproc.c b/arch/powerpc/sysdev/commproc.c
index f6a6378..2426bcb 100644
--- a/arch/powerpc/sysdev/commproc.c
+++ b/arch/powerpc/sysdev/commproc.c
@@ -240,6 +240,34 @@ void __init cpm_reset(void)
 #endif
 }
 
+static DEFINE_SPINLOCK(cmd_lock);
+
+#define MAX_CR_CMD_LOOPS        10000
+
+int cpm_command(u32 command, u8 opcode)
+{
+	int i, ret;
+	unsigned long flags;
+
+	if (command & 0xffffff0f)
+		return -EINVAL;
+
+	spin_lock_irqsave(&cmd_lock, flags);
+
+	ret = 0;
+	out_be16(&cpmp->cp_cpcr, command | CPM_CR_FLG | (opcode << 8));
+	for (i = 0; i < MAX_CR_CMD_LOOPS; i++)
+		if ((in_be16(&cpmp->cp_cpcr) & CPM_CR_FLG) == 0)
+			goto out;
+
+	printk(KERN_ERR "%s(): Not able to issue CPM command\n", __FUNCTION__);
+	ret = -EIO;
+out:
+	spin_unlock_irqrestore(&cmd_lock, flags);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(cpm_command);
+
 /* We used to do this earlier, but have to postpone as long as possible
  * to ensure the kernel VM is now running.
  */
diff --git a/arch/powerpc/sysdev/cpm2_common.c b/arch/powerpc/sysdev/cpm2_common.c
index 859362f..e2acc60 100644
--- a/arch/powerpc/sysdev/cpm2_common.c
+++ b/arch/powerpc/sysdev/cpm2_common.c
@@ -83,6 +83,31 @@ cpm2_reset(void)
 	cpmp = &cpm2_immr->im_cpm;
 }
 
+static DEFINE_SPINLOCK(cmd_lock);
+
+#define MAX_CR_CMD_LOOPS        10000
+
+int cpm_command(u32 command, u8 opcode)
+{
+	int i, ret;
+	unsigned long flags;
+
+	spin_lock_irqsave(&cmd_lock, flags);
+
+	ret = 0;
+	out_be32(&cpmp->cp_cpcr, command | opcode | CPM_CR_FLG);
+	for (i = 0; i < MAX_CR_CMD_LOOPS; i++)
+		if ((in_be32(&cpmp->cp_cpcr) & CPM_CR_FLG) == 0)
+			goto out;
+
+	printk(KERN_ERR "%s(): Not able to issue CPM command\n", __FUNCTION__);
+	ret = -EIO;
+out:
+	spin_unlock_irqrestore(&cmd_lock, flags);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(cpm_command);
+
 /* Set a baud rate generator.  This needs lots of work.  There are
  * eight BRGs, which can be connected to the CPM channels or output
  * as clocks.  The BRGs are in two different block of internal
diff --git a/drivers/net/fs_enet/mac-fcc.c b/drivers/net/fs_enet/mac-fcc.c
index da4efbc..e363211 100644
--- a/drivers/net/fs_enet/mac-fcc.c
+++ b/drivers/net/fs_enet/mac-fcc.c
@@ -81,16 +81,8 @@
 static inline int fcc_cr_cmd(struct fs_enet_private *fep, u32 op)
 {
 	const struct fs_platform_info *fpi = fep->fpi;
-	int i;
-
-	W32(cpmp, cp_cpcr, fpi->cp_command | op | CPM_CR_FLG);
-	for (i = 0; i < MAX_CR_CMD_LOOPS; i++)
-		if ((R32(cpmp, cp_cpcr) & CPM_CR_FLG) == 0)
-			return 0;
 
-	printk(KERN_ERR "%s(): Not able to issue CPM command\n",
-	       __FUNCTION__);
-	return 1;
+	return cpm_command(fpi->cp_command, op);
 }
 
 static int do_pd_setup(struct fs_enet_private *fep)
diff --git a/drivers/net/fs_enet/mac-scc.c b/drivers/net/fs_enet/mac-scc.c
index 03134f4..5ff856d 100644
--- a/drivers/net/fs_enet/mac-scc.c
+++ b/drivers/net/fs_enet/mac-scc.c
@@ -89,21 +89,12 @@
  * Delay to wait for SCC reset command to complete (in us)
  */
 #define SCC_RESET_DELAY		50
-#define MAX_CR_CMD_LOOPS	10000
 
 static inline int scc_cr_cmd(struct fs_enet_private *fep, u32 op)
 {
 	const struct fs_platform_info *fpi = fep->fpi;
-	int i;
-
-	W16(cpmp, cp_cpcr, fpi->cp_command | CPM_CR_FLG | (op << 8));
-	for (i = 0; i < MAX_CR_CMD_LOOPS; i++)
-		if ((R16(cpmp, cp_cpcr) & CPM_CR_FLG) == 0)
-			return 0;
 
-	printk(KERN_ERR "%s(): Not able to issue CPM command\n",
-		__FUNCTION__);
-	return 1;
+	return cpm_command(fpi->cp_command, op);
 }
 
 static int do_pd_setup(struct fs_enet_private *fep)
diff --git a/drivers/serial/cpm_uart/cpm_uart_cpm1.c b/drivers/serial/cpm_uart/cpm_uart_cpm1.c
index 52fb044..6ea0366 100644
--- a/drivers/serial/cpm_uart/cpm_uart_cpm1.c
+++ b/drivers/serial/cpm_uart/cpm_uart_cpm1.c
@@ -52,11 +52,7 @@
 #ifdef CONFIG_PPC_CPM_NEW_BINDING
 void cpm_line_cr_cmd(struct uart_cpm_port *port, int cmd)
 {
-	u16 __iomem *cpcr = &cpmp->cp_cpcr;
-
-	out_be16(cpcr, port->command | (cmd << 8) | CPM_CR_FLG);
-	while (in_be16(cpcr) & CPM_CR_FLG)
-		;
+	cpm_command(port->command, cmd);
 }
 #else
 void cpm_line_cr_cmd(struct uart_cpm_port *port, int cmd)
diff --git a/drivers/serial/cpm_uart/cpm_uart_cpm2.c b/drivers/serial/cpm_uart/cpm_uart_cpm2.c
index 882dbc1..def0158 100644
--- a/drivers/serial/cpm_uart/cpm_uart_cpm2.c
+++ b/drivers/serial/cpm_uart/cpm_uart_cpm2.c
@@ -52,13 +52,7 @@
 #ifdef CONFIG_PPC_CPM_NEW_BINDING
 void cpm_line_cr_cmd(struct uart_cpm_port *port, int cmd)
 {
-	cpm_cpm2_t __iomem *cp = cpm2_map(im_cpm);
-
-	out_be32(&cp->cp_cpcr, port->command | cmd | CPM_CR_FLG);
-	while (in_be32(&cp->cp_cpcr) & CPM_CR_FLG)
-		;
-
-	cpm2_unmap(cp);
+	cpm_command(port->command, cmd);
 }
 #else
 void cpm_line_cr_cmd(struct uart_cpm_port *port, int cmd)
diff --git a/include/asm-powerpc/cpm.h b/include/asm-powerpc/cpm.h
index 48df9f3..fae83b1 100644
--- a/include/asm-powerpc/cpm.h
+++ b/include/asm-powerpc/cpm.h
@@ -10,5 +10,6 @@ int cpm_muram_free(unsigned long offset);
 unsigned long cpm_muram_alloc_fixed(unsigned long offset, unsigned long size);
 void __iomem *cpm_muram_addr(unsigned long offset);
 dma_addr_t cpm_muram_dma(void __iomem *addr);
+int cpm_command(u32 command, u8 opcode);
 
 #endif
-- 
1.5.3.5

^ permalink raw reply related

* Re: [PATCH 5/5] USB: Add Cypress c67x00 OTG controller driver to Kconfig and Makefiles
From: Grant Likely @ 2007-11-24 20:20 UTC (permalink / raw)
  To: David Brownell; +Cc: akpm, linuxppc-dev, gregkh, linux-usb-devel
In-Reply-To: <200711241212.27787.david-b@pacbell.net>

On 11/24/07, David Brownell <david-b@pacbell.net> wrote:
> On Friday 23 November 2007, Grant Likely wrote:
> > +config USB_C67X00_DRV
> > +tristate "Cypress C67x00 support"
> > +# only allowed to be =y if both USB!=m and USB_GADGET!=m
>
> This is wrong.  Remember that since this is a dual-role driver,
> there are exactly three possible driver modes ... and the driver
> must support only one of them at a time:

Actually, this part has 2 serial in interface engines; either of which
can be in host, peripheral or OTG mode.  So; it is entirely relevant
for the driver to be doing both host and peripheral at the same time
with a single device.

>
>   - Host-only ... only allowed if host side USB is enabled.  The
>     controller driver can be statically linked iff usbcore is too.
>
>   - Peripheral-only ... only allowed if peripheral side USB is
>     enabled.  Only one port may be used as the peripheral; the
>     controller driver can be linked statically or as a module.
>
>   - OTG ... only allowed if both host and peripheral side USB
>     are enabled.  Only one port can be the OTG port, but other
>     ports can be used for host functionality.  Static/modular
>     linkage follows (more restrictive) the host-only rule.
>
> And of course, what a given board supports is controlled by the
> connectors it has ... A, B, or AB.  (Possibly AB plus n*A.)  So
> the driver should probably be able to make sense of having OTG
> support, but needing to cope with a non-OTG board ... or having
> an OTG board, a driver that only copes with one role.
>
> Hmmm ... your patches don't include peripheral mode support yet.

No, nobody has written it yet.

>
> Either all these gadget hooks should vanish, or you should try
> to get the controller mode stuff right from the beginning.
>
> I've appended the relevant Kconfig bits from the musb_hdrc
> driver, which (despite some glitches) are pretty much correct.

I'll drop the gadget hooks for now.  Most likely gadget and OTG
support will require more changes to this driver regardless, so it's
not a big deal.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

^ permalink raw reply

* Re: [RFC][PATCH 0/3] OF-platform PATA driver
From: Arnd Bergmann @ 2007-11-24 20:50 UTC (permalink / raw)
  To: linuxppc-dev, avorontsov; +Cc: linux-ide, Paul Mundt, Jeff Garzik
In-Reply-To: <20071123175229.GA27143@localhost.localdomain>

On Friday 23 November 2007, Anton Vorontsov wrote:
> Here is the PATA Platform driver using OF infrastructure.
> 
> Mostly it's just a wrapper around a bit modified pata_platform
> driver.

Thanks a lot for doing this. Patches 2/3 are what I tried to get
people to do for some time now but was too lazy to do myself.

As a further thought, do the drivers now still need to be
pata specific, or should the OF part be called ata_of_platform
instead and also be used for sata devices?

	Arnd <><

^ permalink raw reply

* Re: [PATCH 3/3] [POWERPC] MPC8349E-mITX: introduce pata node, make use pata_of_platform driver
From: Arnd Bergmann @ 2007-11-24 20:57 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: linux-ide
In-Reply-To: <20071123175356.GC27338@localhost.localdomain>

On Friday 23 November 2007, Anton Vorontsov wrote:
>=20
> +static struct of_device_id mpc834x_ids[] =3D {
> +=A0=A0=A0=A0=A0=A0=A0{ .compatible =3D "pata-platform", },
> +=A0=A0=A0=A0=A0=A0=A0{},
> +};
> +
> +static int __init mpc834x_declare_of_platform_devices(void)
> +{
> +=A0=A0=A0=A0=A0=A0=A0if (!machine_is(mpc834x_itx))
> +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0return 0;
> +
> +=A0=A0=A0=A0=A0=A0=A0of_platform_bus_probe(NULL, mpc834x_ids, NULL);
> +
> +=A0=A0=A0=A0=A0=A0=A0return 0;
> +}
> +device_initcall(mpc834x_declare_of_platform_devices);

This is not really how of_platform_bus_probe was meant to be used.
Instead of listing the device you want to probe, you should list
all buses that potentially contain a device that you are probing.

Normally, an ata controller is not a top-level device but instead
the child of an SOC bus device, and then you just probe all
SOC devices, which means their children get added to the device
tree.

In your case, that would probably mean that you have to another
entry in the "ranges" of the soc8349 node, or add a second socXXXX
node that has the 0xf0000000 ranges, depending on the actual layout
of the SOC.

	Arnd <><

^ permalink raw reply

* Re: [PATCH 5/5] USB: Add Cypress c67x00 OTG controller driver to Kconfig and Makefiles
From: David Brownell @ 2007-11-24 21:03 UTC (permalink / raw)
  To: Grant Likely; +Cc: akpm, linuxppc-dev, gregkh, linux-usb-devel
In-Reply-To: <fa686aa40711241220r4e808ad8obb27ae8bf5890048@mail.gmail.com>

On Saturday 24 November 2007, Grant Likely wrote:
> On 11/24/07, David Brownell <david-b@pacbell.net> wrote:
> > On Friday 23 November 2007, Grant Likely wrote:
> > > +config USB_C67X00_DRV
> > > +tristate "Cypress C67x00 support"
> > > +# only allowed to be =y if both USB!=m and USB_GADGET!=m
> >
> > This is wrong.  Remember that since this is a dual-role driver,
> > there are exactly three possible driver modes ... and the driver
> > must support only one of them at a time:
> 
> Actually, this part has 2 serial in interface engines; either of which
> can be in host, peripheral or OTG mode.  So; it is entirely relevant
> for the driver to be doing both host and peripheral at the same time
> with a single device.

While conforming with USB specs?  Has that changed recently?

In the not-too-distant past I know there was explicit language
saying that such configurations are not allowed.  (At least,
in terms of external connectors.  A board using USB for internal
peripheral connections would be OK.)

Those issues can be deferred until peripheral support exists.
At that point you'll need to get better solutions to those config
issues...


> > Hmmm ... your patches don't include peripheral mode support yet.
> 
> No, nobody has written it yet.

Then what's it doing in that GIT tree you pointed me at?  :)


> > Either all these gadget hooks should vanish, or you should try
> > to get the controller mode stuff right from the beginning.
> >
> > I've appended the relevant Kconfig bits from the musb_hdrc
> > driver, which (despite some glitches) are pretty much correct.
> 
> I'll drop the gadget hooks for now.  Most likely gadget and OTG
> support will require more changes to this driver regardless, so it's
> not a big deal.

OK.  Then the first incaration of this will be as a host-only
driver, in its own directory, and adding peripheral support will
cause minor lunacy.  :)

- Dave

^ permalink raw reply

* Re: [PATCH 5/5] USB: Add Cypress c67x00 OTG controller driver to Kconfig and Makefiles
From: Grant Likely @ 2007-11-24 21:13 UTC (permalink / raw)
  To: David Brownell; +Cc: akpm, linuxppc-dev, gregkh, linux-usb-devel
In-Reply-To: <200711241303.31700.david-b@pacbell.net>

On 11/24/07, David Brownell <david-b@pacbell.net> wrote:
> On Saturday 24 November 2007, Grant Likely wrote:
> > On 11/24/07, David Brownell <david-b@pacbell.net> wrote:
> > > On Friday 23 November 2007, Grant Likely wrote:
> > > > +config USB_C67X00_DRV
> > > > +tristate "Cypress C67x00 support"
> > > > +# only allowed to be =y if both USB!=m and USB_GADGET!=m
> > >
> > > This is wrong.  Remember that since this is a dual-role driver,
> > > there are exactly three possible driver modes ... and the driver
> > > must support only one of them at a time:
> >
> > Actually, this part has 2 serial in interface engines; either of which
> > can be in host, peripheral or OTG mode.  So; it is entirely relevant
> > for the driver to be doing both host and peripheral at the same time
> > with a single device.
>
> While conforming with USB specs?  Has that changed recently?

Let me rephrase; the silicon implements 2 USB engines with separate
pinouts.  They can be configured as 2 host controllers; 2 peripheral
controllers or 1 of each.  They share registers and the IRQ line from
the software interface, but they are distinctly separate USB busses.

This driver actually implements 2 sub devices.  Each sub device is
either bound to the host part of the driver or the gadget part.

>
> In the not-too-distant past I know there was explicit language
> saying that such configurations are not allowed.  (At least,
> in terms of external connectors.  A board using USB for internal
> peripheral connections would be OK.)
>
> Those issues can be deferred until peripheral support exists.
> At that point you'll need to get better solutions to those config
> issues...

ok

>
>
> > > Hmmm ... your patches don't include peripheral mode support yet.
> >
> > No, nobody has written it yet.
>
> Then what's it doing in that GIT tree you pointed me at?  :)

Sitting there being useless.  :-)  That driver doesn't actually work;
it's just a proof of concept placeholder (which is why I didn't
include it in my series)

>
>
> > > Either all these gadget hooks should vanish, or you should try
> > > to get the controller mode stuff right from the beginning.
> > >
> > > I've appended the relevant Kconfig bits from the musb_hdrc
> > > driver, which (despite some glitches) are pretty much correct.
> >
> > I'll drop the gadget hooks for now.  Most likely gadget and OTG
> > support will require more changes to this driver regardless, so it's
> > not a big deal.
>
> OK.  Then the first incaration of this will be as a host-only
> driver, in its own directory, and adding peripheral support will
> cause minor lunacy.  :)

:-)

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

^ permalink raw reply

* Re: [RFC/PATCH] powerpc: Move CPM command handling into the cpm drivers
From: Vitaly Bordug @ 2007-11-24 21:47 UTC (permalink / raw)
  To: Jochen Friedrich; +Cc: linuxppc-dev
In-Reply-To: <4748651E.2020806@scram.de>

On Sat, 24 Nov 2007 18:53:34 +0100
Jochen Friedrich wrote:

> Hi Vitaly,
> 
> >>> +       printk(KERN_ERR "%s(): Not able to issue CPM command\n",
> >>> +               __FUNCTION__);
> >>> +       return -EIO;  
> >>>       
> >> Do these need to be protected with a spin lock?
> >>     
> > Even that might be not enough - we may have simultaneous call of
> > this func in non-smp case... I was thinking of some kind of
> > refcount, so one that is going to issue CPM command, must do say
> > pq_cpmp_get() and another driver won't be able to mangle with cpcr
> > while it's not done with previous request.
> >
> > Yet I am not telling it was better the way it used to be - this
> > approach looks okay but needs some efforts to defend against
> > deadlocks while we are at it
> 
> Wouldn't spin_lock_irqsave() prevent a deadlock?
> 
yes, I believe it is OK for now.

> Thanks,
> Jochen


-- 
Sincerely, Vitaly

^ permalink raw reply

* Re: [PATCH] IB/ehca: Fix static rate regression
From: Roland Dreier @ 2007-11-24 21:48 UTC (permalink / raw)
  To: Joachim Fenkes
  Cc: LKML, OF-EWG, LinuxPPC-Dev, Christoph Raisch, Marcus Eder,
	OF-General, Stefan Roscher
In-Reply-To: <200711221126.27465.fenkes@de.ibm.com>

thanks, applied.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox