LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH 1/4] Add DMA sector to Documentation/powerpc/booting-without-of.txt file.
From: Paul Mackerras @ 2007-07-17 11:17 UTC (permalink / raw)
  To: Segher Boessenkool; +Cc: linuxppc-dev, Wei.Zhang
In-Reply-To: <B2EA074C-5D8E-4E13-B6BF-E81FBA52E76E@kernel.crashing.org>

Segher Boessenkool writes:

> The device tree describes _all_ hardware in the system,
> not just the things that are somewhat harder to probe
> for.

Actually, for embedded systems, the device tree is really only
required to describe the things that it's useful for the Linux kernel
to know.

The point of the device tree for embedded systems is to provide
configuration information, not to be able to claim compliance with
some set of legalistic requirements. :)

I think in some cases we have gone a little over the top in trying to
put everything in the device tree, in fact.  Ultimately I think it has
to be up to the more experienced embedded developers to say how much
detail in the device tree is actually helpful and how much is dead
weight.

Paul.

^ permalink raw reply

* Re: [PATCH 3/3] 82xx: SBCPQ2 board platform support
From: Mark Zhan @ 2007-07-17  6:58 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: linuxppc-dev, paulus
In-Reply-To: <200707170327.41648.arnd@arndb.de>

Hi Arnd,

On Tue, 2007-07-17 at 03:27 +0200, Arnd Bergmann wrote:
> > +static struct resource m48t59_resources[] = {
> > +	{
> > +		.start	= SBCPQ2_RTC_BASE,
> > +		.end	= SBCPQ2_RTC_BASE + SBCPQ2_RTC_SIZE - 1,
> > +		.flags	= IORESOURCE_MEM,
> > +	}, {
> > +		.start	= SBCPQ2_M48T59_IRQ,
> > +		.end	= SBCPQ2_M48T59_IRQ,
> > +		.flags	= IORESOURCE_IRQ,
> > +	},
> > +	{ },
> > +};
> 
> This is the kind of information that belongs into the device tree,
> not hardcoded into the board code.
> 

ok, I will move them into device tree.

> > +/**
> > + * sbcpq2_pdev_init - Register the platform device for sbcpq2 board
> > + */
> > +static int __init sbcpq2_platdev_init(void)
> > +{
> > +	struct irq_desc *desc = irq_desc + SBCPQ2_M48T59_IRQ;
> 
> same for the interrupt number. Worse, this looks broken
> because the descriptor array describes virtual interrupt
> numbers, while SBCPQ2_M48T59_IRQ must be a physical number.
> These are often the same, but there is no guarantee.
> 
> In order to get a virtual interrupt number for a given device,
> you need to call irq_of_parse_and_map().
> 
> > +	/* Install a dummy irq chip for M48T59 RTC irq */
> > +	if (desc->chip == &no_irq_chip)
> > +		set_irq_handler(SBCPQ2_M48T59_IRQ, desc->handle_irq);
> > +
> > +	/* Register all platform devices for sbcpq2 */
> > +	platform_add_devices(sbcpq2_devices, ARRAY_SIZE(sbcpq2_devices));
> > +	return 0;
> > +}
> > +arch_initcall(sbcpq2_platdev_init);
> 
> 
> > +/*
> > + * For SBCPQ2 board, the interrupt of M48T59 RTC chip
> > + * will generate a machine check exception. We use a
> > + * fake irq to give the platform machine_check_exception() hook
> > + * a chance to call the driver ISR. If IRQ_HANDLED is returned,
> > + * then we will survive from the machine check exception.
> > + */
> > +static int sbcpq2_mach_check(struct pt_regs *regs)
> > +{
> > +	int recover = 0;
> > +	struct irq_desc *desc = irq_desc + SBCPQ2_M48T59_IRQ;
> > +
> > +	struct irqaction *action = desc->action;
> > +
> > +	while (action && (action->dev_id != &m48t59_rtc))
> > +		action = action->next;
> > +
> > +	/* Try to call m48t59 RTC driver ISR */
> > +	if (action && action->handler)
> > +		recover = action->handler(SBCPQ2_M48T59_IRQ, &m48t59_rtc);
> > +
> > +	return recover;
> > +}
> 
> What you do here looks really scary, but maybe I'm just misunderstanding
> it completely. Why don't you just register your rtc handler function
> as the machine check handler instead of going through various indirections?
> 

The rtc M48T59 driver is not specific to my board, it is probably used
by other board. So I can't register the rtc intr handler as the mcheck
exception handler. And in the other side, there are also other machine
check sources, right?

So here I add a platform mcheck hook for rtc intr handler. Yeah, it is
really scary and confusing:-)

> > +static void __init sbcpq2_init_IRQ(void)
> > +{
> > +	struct device_node *np;
> > +	struct resource res;
> > +
> > +	np = of_find_compatible_node(NULL, "cpm-pic", "CPM2");
> > +	if (np == NULL) {
> > +		printk(KERN_ERR "PIC init: can not find cpm-pic node\n");
> > +		return;
> > +	}
> 
> This looks like your device tree is wrong. Shouldn't the interrupt
> controller have device_type="interrupt-controller" and a specific
> compatible property instead of having the name in the device_type?
> 

Here, I just copy the codes from mpc82xx_ads, is there anything wrong?

> > +static void __init sbcpq2_setup_arch(void)
> > +{
> > +	struct device_node *np;
> > +	volatile memctl_cpm2_t *mc;
> 
> not volatile, but __iomem!
> 

Fixed.

> > +	unsigned char * eeprom_base;
> > +	int i = 0;
> > +
> > +#ifdef CONFIG_CPM2
> > +	cpm2_reset();
> > +#endif
> > +
> > +	/*
> > +	 * Make sure that we have the right CS# setting
> > +	 */
> > +	mc = &cpm2_immr->im_memctl;
> > +
> > +	/* Boot Flash is the on-board flash */
> > +	mc->memc_br0 = (SBCPQ2_BOOT_FLASH_BASE & 0xFFFF8000) | 0x0801;
> > +	mc->memc_or0 = 0xFFE00896;
> 
> consequently, this needs to use out_be32 or similar.
> Where does SBCPQ2_BOOT_FLASH_BASE come from? Shouldn't that be set
> up by the boot loader to match the device tree?

Fixed. out_be32 is used. The reason why they are needed is because some
legacy u-boot for this board probably was setting up the wrong memory
map.

> 
> > +		model = (char *)of_get_property(np, "model", NULL);
> > +		if (!model)
> > +			continue;
> 
> The cast is not needed here.
> > +
> > +		id = of_get_property(np, "device-id", NULL);
> > +		if (!id)
> > +			continue;
> > +
> > +		macaddr = (unsigned char *)of_get_mac_address(np);
> > +		if (!macaddr)
> > +			continue;
> 
> or here.

Both cast are removed.

> 
> > +		if (strstr(model, "FCC"))
> > +			eeprom_ofs = SBCPQ2_FCC1_MACADDR_OFS;
> > +		else if (strstr(model, "SCC"))
> > +			eeprom_ofs = SBCPQ2_SCC1_MACADDR_OFS;
> > +		eeprom_ofs += ((*id) - 1) * 6;
> 
> of_device_is_compatible()
> 
> 
> > +		for (j = 0; j < 6; j++)
> > +			*(macaddr + j) = *(eeprom_base + eeprom_ofs + j);
> 
> in_8().

OK. in_8() will be used.

> 
> > +	}
> > +	iounmap(eeprom_base);
> > +}
> > +
> > +/*
> > + * Called very early, device-tree isn't unflattened
> > + */
> > +static int __init sbcpq2_probe(void)
> > +{
> > +	/* We always match for now, eventually we should look at
> > +	 * the flat dev tree to ensure this is the board we are
> > +	 * supposed to run on
> > +	 */
> > +	return 1;
> > +}
> 
> Don't write why the code is wrong -- just fix it.
> 
> > +/* For our show_cpuinfo hooks. */
> > +#define CPUINFO_VENDOR		"Wind River"
> > +#define CPUINFO_MACHINE		"SBC PowerQUICCII 82xx"
> 
> Not in a header file please.
> 
> > +/*
> > + * Wind River SBC PowerQUICCII 82xx Physical Memory Map (CS0 for OnBoard Flash)
> > + *
> > + *   0x00000000 - 0x07FFFFFF	CS2, 128 MB DIMM SDRAM
> > + *   0x08000000 - 0x0FFFFFFF	CS3, 128 MB DIMM SDRAM
> > + *   0x12000000 - 0x12100000	CS8, ATM
> > + *   0x20000000 - 0x20FFFFFF	CS4, 16 MB Local Bus SDRAM
> > + *   0x21000000 - 0x21001FFF	CS7, Control EPLD
> > + *   0x22000000 - 0x22001FFF	CS5, 8KB EEPROM
> > + *   0x22002000 - 0x22003FFF	CS5, visionPORT
> > + *   0x22004000 - 0x22005FFF	CS5, User Switches
> > + *   0x22006000 - 0x22007FFF	CS5, STATUS
> > + *   0x22008000 - 0x22009FFF	CS5, i8259 interrupt controller
> > + *   0x2200A000 - 0x2200BFFF	CS5, LED (Seven Segment Display)
> > + *   0x80000000 - 0x80001FFF	CS11, RTC
> > + *   0xE0000000 - 0xE3FFFFFF	CS6, 64 MB DIMM Flash
> > + *   0xE4000000 - 0xE7FFFFFF	CS1, 64 MB DIMM Flash
> > + *   0xFE000000 - 0xFFFFFFFF	CS0, 2 MB Boot Flash
> > + *   0xF0000000 - 0xF0020000	MPC82xx Internal Registers Space
> > + */
> > +#define SBCPQ2_SDRAM_BASE		0x00000000
> > +#define SBCPQ2_SDRAM_SIZE		0x10000000
> > +
> > +#define SBCPQ2_LOCAL_SDRAM_BASE		0x20000000
> > +#define SBCPQ2_LOCAL_SDRAM_SIZE		0x1000000
> > +
> > +#define SBCPQ2_EPLD_BASE		0x21000000
> > +#define SBCPQ2_EPLD_SIZE		0x2000
> > +
> > +#define SBCPQ2_EEPROM_BASE		0x22000000
> > +#define SBCPQ2_EEPROM_SIZE		0x2000
> > +
> > +/* User Switches SW5 */
> > +#define SBCPQ2_USER_SW_BASE		0x22004000
> > +#define SBCPQ2_USER_SW_SIZE		0x2000
> > +
> > +#define SBCPQ2_STATUS_BASE		0x22006000
> > +#define SBCPQ2_STATUS_SIZE		0x2000
> > +
> > +#define SBCPQ2_I8259_BASE		0x22008000
> > +#define SBCPQ2_I8259_SIZE		0x2000
> > +
> > +/* Seven Segment Display LED D46 */
> > +#define SBCPQ2_LED_BASE			0x2200A000
> > +#define SBCPQ2_LED_SIZE			0x2000
> > +
> > +#define SBCPQ2_RTC_BASE			0x80000000
> > +#define SBCPQ2_RTC_SIZE			0x2000
> > +
> > +#define SBCPQ2_BOOT_FLASH_BASE		0xFE000000
> > +#define SBCPQ2_BOOT_FLASH_SIZE		0x00200000
> > +
> > +#define SBCPQ2_DIMM_FLASH_BASE		0xE0000000
> > +#define SBCPQ2_DIMM_FLASH_SIZE		0x04000000
> > +
> > +#define CPM_MAP_ADDR			0xF0000000
> > +#define CPM_IRQ_OFFSET			0
> 
> All this is in the device tree already, so don't duplicate it here.
> 
> > +/*
> > + * The offset of ethernet MAC addr within EEPROM
> > + */
> > +#define SBCPQ2_FCC1_MACADDR_OFS		0x60
> > +#define SBCPQ2_FCC2_MACADDR_OFS		0x66
> > +#define SBCPQ2_FCC3_MACADDR_OFS		0x72
> > +#define SBCPQ2_SCC1_MACADDR_OFS		0x78
> 
> Likewise, the mac address is in the device tree, so no need
> to tell the kernel how to read it.
> 
> > +/*
> > + * The following IRQs are routed to i8259 PIC.
> > + *
> > + * NOTE: i8259 PIC is cascaded to SIU_INT_IRQ6 of CPM2 interrupt controller
> > + */
> > +#define SBCPQ2_PC_IRQA		(NR_SIU_INTS+0)
> > +#define SBCPQ2_PC_IRQB		(NR_SIU_INTS+1)
> > +#define SBCPQ2_MPC185_IRQ	(NR_SIU_INTS+2)
> > +#define SBCPQ2_ATM_IRQ		(NR_SIU_INTS+3)
> > +#define SBCPQ2_PIRQA		(NR_SIU_INTS+4)
> > +#define SBCPQ2_PIRQB		(NR_SIU_INTS+5)
> > +#define SBCPQ2_PIRQC		(NR_SIU_INTS+6)
> > +#define SBCPQ2_PIRQD		(NR_SIU_INTS+7)
> 
> Again, these are in the device tree, so don't put them here.
> 
> > +/* cpm serial driver works with constants below */
> > +#define SIU_INT_SMC1		((uint)0x04+CPM_IRQ_OFFSET)
> > +#define SIU_INT_SMC2		((uint)0x05+CPM_IRQ_OFFSET)
> > +#define SIU_INT_SCC1		((uint)0x28+CPM_IRQ_OFFSET)
> > +#define SIU_INT_SCC2		((uint)0x29+CPM_IRQ_OFFSET)
> > +#define SIU_INT_SCC3		((uint)0x2a+CPM_IRQ_OFFSET)
> > +#define SIU_INT_SCC4		((uint)0x2b+CPM_IRQ_OFFSET)
> 
> What are these for? If you need them in the device driver, just put
> them in there, not in a header file. Also, you should make
> sure not to pollute the global name space, so they should
> be named SBCPQ2_SIU_INT_* to make it clear that they are board
> specific.
> 
> > +#ifdef CONFIG_SBCPQ2
> > +#include <platforms/82xx/sbcpq2.h>
> > +#endif
> 
> Never put #ifdef around an #include.
> 
> > +
> >   #ifdef CONFIG_PCI_8260
> >   #include <platforms/82xx/m82xx_pci.h>
> >   #endif
> 
> Kill this #ifdef as well while you're there. If you get name space
> conflicts, just rename the symbols to make them unique.
> 
> > +/ {
> > +	model = "SBCPQ2";
> > +	compatible = "mpc82xx";
> > +	#address-cells = <1>;
> > +	#size-cells = <1>;
> > +	linux,phandle = <100>;
> 
> Don't put explicit phandles here. If you need a reference, do it
> symbolically.
> 
> 	Arnd <><

^ permalink raw reply

* Re: [PATCH 0/3] 82xx: Add the support for Wind River SBC PowerQUICCII
From: Vitaly Bordug @ 2007-07-17  6:41 UTC (permalink / raw)
  To: Mark Zhan; +Cc: linuxppc-dev, paulus
In-Reply-To: <1184649903.10499.49.camel@mark>

On Tue, 17 Jul 2007 13:25:03 +0800
Mark Zhan wrote:

> Hi Vitaly,
> 
> When you will send out your 8xx/82xx series patches?
> 
As I mentioned, they are not mine but Scott's. And should appear shortly according to him...

> Thanks
> Mark Zhan
> On Tue, 2007-07-17 at 04:44 +0400, Vitaly Bordug wrote:
> > On Mon, 16 Jul 2007 17:01:28 +0800
> > Mark Zhan wrote:
> > 
> > > These 3 patches add the powerpc support of Wind River SBC
> > > PowerQUICCII.
> > > 
> > I am sorry, but this would intercept with big 8xx/82xx series from 
> > Scott Wood I am reviewing now. it makes the initial merged code
> > more maintainable and powerpc-friendly, but would have some rework
> > required in upcoming patches for new BSPs.
> > 
> > OTOH, it would make all the changes below very short and apparent.
> > Of course, if something would block those series this one can be
> > considered to merge.
> > 
> > 
> > > 1) Currently, some mpc82xx platform hooks in mpc82xx_ads.c are
> > > actually not specific to ads board. So the 1st patch makes them be
> > > shared by different 82xx boards.
> > > 
> > > 2) The 2nd patch make 'cpm_uart_of_init' be able to parse SMC
> > > uart in DTS
> > > 
> > > 3) The 3rd patch is the platform codes, defconfig and dts for Wind
> > > River SBC PowerQUICCII 82xx board.
> > > 
> > > Any comment is welcome.
> > > 
> > > Thanks
> > > Mark Zhan
> > > 
> > > _______________________________________________
> > > Linuxppc-dev mailing list
> > > Linuxppc-dev@ozlabs.org
> > > https://ozlabs.org/mailman/listinfo/linuxppc-dev
> > 
> > 


-- 
Sincerely, Vitaly

^ permalink raw reply

* Re: [PATCH 10/10] IB/ehca: Support large page MRs
From: Joachim Fenkes @ 2007-07-17  6:29 UTC (permalink / raw)
  To: Roland Dreier
  Cc: LKML, LinuxPPC-Dev, Christoph Raisch, OF-General, Stefan Roscher
In-Reply-To: <adatzs3u1sa.fsf@cisco.com>

Roland Dreier <rdreier@cisco.com> wrote on 17.07.2007 05:50:13:

>  > > Why the module parameter?  Is there any reason a user would want to
>  > > turn this off?  Or conversely, why is it off by default?
>  > 
>  > We're pretty confident this new feature works, but as with all new 
and 
>  > possibly experimental features, there are chances it might explode 
your 
>  > machine when activated. So, like with the scaling code, we want the 
user 
>  > to make the conscious decision of using this code instead of 
activating it 
>  > by default.
> 
> OK, I guess.  So can we expect to, say, change the default to turning
> it on for 2.6.24 and remove the option entirely (so it's always on) in
> 2.6.25?

Deal.

Joachim

^ permalink raw reply

* Re: Linuxppc-embedded Digest, Vol 35, Issue 33
From: suresh suresh @ 2007-07-17  5:59 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-embedded, Siva Prasad
In-Reply-To: <469B9825.6070507@freescale.com>

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

Thanks....

MPC8280 has internal memory space which contains Dualport(Dp) RAM. In the
DpRAM we allocate some tables and these has to store pointer of the buffers
which are allocated in external memory. Core will use this pointer to access
the buffers, basically these buffers are used for DMA.

Using IMMR register we can get physical address of the internal memory. Now
can I store the address return by the kmalloc() function? or I should
convert it into physical?

Please help me how to resove this address translation.


Regards,
Suresh

On 7/16/07, Scott Wood <scottwood@freescale.com> wrote:
>
> Siva Prasad wrote:
> > It returns kernel virtual address. If you use this buffer space for DMA,
> > please use appropriate flags. You may use __pa(address) or
> > virt_to_phys() to convert virtual to physical.
>
> No, you may not -- physical and DMA addresses are not always identical.
>   Use the DMA mapping API.
>
> -Scott
>
>

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

^ permalink raw reply

* Re: [ofa-general] Re: [PATCH 01/10] IB/ehca: Support for multiple event queues
From: Shirley Ma @ 2007-07-17  5:57 UTC (permalink / raw)
  To: Roland Dreier
  Cc: Joachim Fenkes, LKML, LinuxPPC-Dev, general-bounces,
	Hoang-Nam Nguyen, OF-General, Stefan Roscher
In-Reply-To: <ada1wf7vgfb.fsf@cisco.com>

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





Hello Roland,

>I still haven't seen much code using the feature or
>even any anecdotal information about the performance impact.

The multiple links performance has been significant improved according to
the prototype IPoIB-UD mode test for eHCA driver esepcially for two links
on the same adapter. I haven't tried mthca (PCI-X and PCI-E) yet.

Thanks
Shirley

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

^ permalink raw reply

* Re: [PATCH 1/3] 82xx: some 82xx platform hook functions can be shared by different boards
From: Mark Zhan @ 2007-07-17  5:31 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: linuxppc-dev, paulus
In-Reply-To: <200707170259.47098.arnd@arndb.de>

On Tue, 2007-07-17 at 02:59 +0200, Arnd Bergmann wrote:
> On Monday 16 July 2007, Mark Zhan wrote:
> 
> > @@ -96,7 +94,7 @@
> >   	pvid = mfspr(SPRN_PVR);
> >   	svid = mfspr(SPRN_SVR);
> > 
> > -	seq_printf(m, "Vendor\t\t: Freescale Semiconductor\n");
> > +	seq_printf(m, "Vendor\t\t: %s\n", CPUINFO_VENDOR);
> >   	seq_printf(m, "Machine\t\t: %s\n", CPUINFO_MACHINE);
> >   	seq_printf(m, "PVR\t\t: 0x%x\n", pvid);
> >   	seq_printf(m, "SVR\t\t: 0x%x\n", svid);
> 
> This is a step in the wrong direction. CPUINFO_{VENDOR,MACHINE}
> comes from a platform specific header file, so you can not
> use these definitions in platform independent code without
> breaking multiplatform kernels.
> 
> One possible solution would be a platform specific show_cpuinfo()
> function that calls a generic 82xx version and passes in the
> two values. Even better would be to just dump whatever string
> you find in the /model property in the device tree.
> 

OK, Got what you said. I will fix it.


> > +
> > +#define RMR_CSRE 0x00000001
> > +
> > +void mpc82xx_restart(char *cmd)
> > +{
> > +	__volatile__ unsigned char dummy;
> > +
> > +	local_irq_disable();
> > +	((cpm2_map_t *) cpm2_immr)->im_clkrst.car_rmr |= RMR_CSRE;
> > +
> > +	/* Clear the ME,EE,IR & DR bits in MSR to cause checkstop */
> > +	mtmsr(mfmsr() & ~(MSR_ME | MSR_EE | MSR_IR | MSR_DR));
> > +	dummy = ((cpm2_map_t *) cpm2_immr)->im_clkrst.res[0];
> > +	printk("Restart failed\n");
> > +	while (1) ;
> > +}
> 
> I know you're just moving that code, but it looks horribly wrong
> nonetheless. cpm2_immr is an __iomem variable, so you must not
> dereference it but instead should use the in_8() macro to
> access it.
> 
> Once you get that right, you don't need the volatile variable
> any more.

OK. will fix it.

> 
> > +void mpc82xx_halt(void)
> > +{
> > +	local_irq_disable();
> > +	while (1) ;
> > +}
> 
> Here, as in the function above, there should at least be a cpu_relax()
> in the final loop. If the CPU has a nap functionality or something
> similar, that would be even better.

Not sure if mpc82xx has such kind of functionality. Based on the current
definition of cpu_relax(), it is only meaningful for ppc64.

> 
> 	Arnd <><

^ permalink raw reply

* Re: [PATCH 0/3] 82xx: Add the support for Wind River SBC PowerQUICCII
From: Mark Zhan @ 2007-07-17  5:25 UTC (permalink / raw)
  To: Vitaly Bordug; +Cc: linuxppc-dev, paulus
In-Reply-To: <20070717044414.03983375@localhost.localdomain>

Hi Vitaly,

When you will send out your 8xx/82xx series patches?

Thanks
Mark Zhan
On Tue, 2007-07-17 at 04:44 +0400, Vitaly Bordug wrote:
> On Mon, 16 Jul 2007 17:01:28 +0800
> Mark Zhan wrote:
> 
> > These 3 patches add the powerpc support of Wind River SBC
> > PowerQUICCII.
> > 
> I am sorry, but this would intercept with big 8xx/82xx series from 
> Scott Wood I am reviewing now. it makes the initial merged code
> more maintainable and powerpc-friendly, but would have some rework
> required in upcoming patches for new BSPs.
> 
> OTOH, it would make all the changes below very short and apparent.
> Of course, if something would block those series this one can be considered to merge.
> 
> 
> > 1) Currently, some mpc82xx platform hooks in mpc82xx_ads.c are
> > actually not specific to ads board. So the 1st patch makes them be
> > shared by different 82xx boards.
> > 
> > 2) The 2nd patch make 'cpm_uart_of_init' be able to parse SMC uart in
> > DTS
> > 
> > 3) The 3rd patch is the platform codes, defconfig and dts for Wind
> > River SBC PowerQUICCII 82xx board.
> > 
> > Any comment is welcome.
> > 
> > Thanks
> > Mark Zhan
> > 
> > _______________________________________________
> > Linuxppc-dev mailing list
> > Linuxppc-dev@ozlabs.org
> > https://ozlabs.org/mailman/listinfo/linuxppc-dev
> 
> 

^ permalink raw reply

* Re: [PATCH 2.6.22-rc7 0/4] phy/gianfar: Fixes for gianfar and vitesse RGMII-ID support
From: Greg KH @ 2007-07-17  5:27 UTC (permalink / raw)
  To: Kumar Gala; +Cc: Jeff Garzik, netdev, linuxppc-dev list, Paul Mackerras, stable
In-Reply-To: <CAD6C707-84F6-4355-A514-B9EBB565B4F9@kernel.crashing.org>

On Tue, Jul 17, 2007 at 12:16:27AM -0500, Kumar Gala wrote:
>  Greg,
> 
>  Can you look at pulling these fixes for the next 2.6.22.x stable release.

Can you send them as patches, to the stable@kernel.org address, after
they go into Linus's tree?

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH 2.6.22-rc7 0/4] phy/gianfar: Fixes for gianfar and vitesse RGMII-ID support
From: Kumar Gala @ 2007-07-17  5:16 UTC (permalink / raw)
  To: Greg KH; +Cc: Jeff Garzik, netdev, linuxppc-dev list, Paul Mackerras, stable
In-Reply-To: <469C4E7B.6050401@garzik.org>

Greg,

Can you look at pulling these fixes for the next 2.6.22.x stable  
release.

- k

On Jul 17, 2007, at 12:07 AM, Jeff Garzik wrote:

> Andy Fleming wrote:
>> A few bugs in the Vitesse PHY driver were found on the 8641D HPCN  
>> board.
>> Originally, they were masked by a bug in the PHY Lib which was  
>> fixed by patch 5f708dd91d15876e26d7a57f97a255cedffca463 (Fix  
>> phy_id for Vitesse 824x PHY).
>> That patch allowed the Vitesse PHY to bind on the 8641D HPCN  
>> board, thereby
>> exposing a bug in the interrupt handling and a bug in the  
>> configuration of
>> the PHY.  This sequence of patches fixes the irq handling bug,  
>> then fixes the configuration bug in 3 places:
>> 1) The Vitesse PHY driver
>> 2) The gianfar driver (needs to pass in the mode correctly for  
>> internal delay)
>> 3) The OF device tree for arch/powerpc boards
>> You can apply the following patches, or pull them directly:
>> The following changes since commit  
>> c5e3ae8823693b260ce1f217adca8add1bc0b3de:
>>   Ayaz Abdulla (1):
>>         forcedeth bug fix: realtek phy
>> are found in the git repository at:
>>   http://opensource.freescale.com/pub/scm/linux-2.6-85xx.git netdev
>> Andy Fleming (4):
>>       Fix Vitesse 824x PHY interrupt acking
>>       Add phy-connection-type to gianfar nodes
>>       Fix Vitesse RGMII-ID support
>>       Fix RGMII-ID handling in gianfar
>>  Documentation/powerpc/booting-without-of.txt |    6 +++
>>  arch/powerpc/boot/dts/mpc8641_hpcn.dts       |    4 ++
>>  arch/powerpc/sysdev/fsl_soc.c                |    9 +++++
>>  drivers/net/gianfar.c                        |   12 ++++++-
>>  drivers/net/phy/vitesse.c                    |   46 ++++++++++++++ 
>> +++++++++---
>>  include/linux/fsl_devices.h                  |    1 +
>>  6 files changed, 72 insertions(+), 6 deletions(-)
>
> pulled
>

^ permalink raw reply

* Re: [PATCH 2.6.22-rc7 0/4] phy/gianfar: Fixes for gianfar and vitesse RGMII-ID support
From: Jeff Garzik @ 2007-07-17  5:07 UTC (permalink / raw)
  To: Andy Fleming; +Cc: netdev, paulus, linuxppc-dev
In-Reply-To: <1184635621688-git-send-email-afleming@freescale.com>

Andy Fleming wrote:
> A few bugs in the Vitesse PHY driver were found on the 8641D HPCN board.
> Originally, they were masked by a bug in the PHY Lib which was fixed by patch 
> 5f708dd91d15876e26d7a57f97a255cedffca463 (Fix phy_id for Vitesse 824x PHY).
> That patch allowed the Vitesse PHY to bind on the 8641D HPCN board, thereby
> exposing a bug in the interrupt handling and a bug in the configuration of
> the PHY.  This sequence of patches fixes the irq handling bug, then fixes 
> the configuration bug in 3 places:
> 
> 1) The Vitesse PHY driver
> 2) The gianfar driver (needs to pass in the mode correctly for internal delay)
> 3) The OF device tree for arch/powerpc boards
> 
> You can apply the following patches, or pull them directly:
> 
> The following changes since commit c5e3ae8823693b260ce1f217adca8add1bc0b3de:
>   Ayaz Abdulla (1):
>         forcedeth bug fix: realtek phy
> 
> are found in the git repository at:
> 
>   http://opensource.freescale.com/pub/scm/linux-2.6-85xx.git netdev
> 
> Andy Fleming (4):
>       Fix Vitesse 824x PHY interrupt acking
>       Add phy-connection-type to gianfar nodes
>       Fix Vitesse RGMII-ID support
>       Fix RGMII-ID handling in gianfar
> 
>  Documentation/powerpc/booting-without-of.txt |    6 +++
>  arch/powerpc/boot/dts/mpc8641_hpcn.dts       |    4 ++
>  arch/powerpc/sysdev/fsl_soc.c                |    9 +++++
>  drivers/net/gianfar.c                        |   12 ++++++-
>  drivers/net/phy/vitesse.c                    |   46 +++++++++++++++++++++++---
>  include/linux/fsl_devices.h                  |    1 +
>  6 files changed, 72 insertions(+), 6 deletions(-)

pulled

^ permalink raw reply

* Re: [PATCH 2/3] 82xx: Parse SMC serial device node in DTS
From: Mark Zhan @ 2007-07-17  4:49 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: linuxppc-dev, paulus
In-Reply-To: <200707170302.07469.arnd@arndb.de>

Hello Arnd,

I think, the function of_find_compatible_node(), which is called in the
for loop, has already done that. So definitely, no need to call
of_device_is_compatible() any more.

Thanks
Mark Zhan
On Tue, 2007-07-17 at 03:02 +0200, Arnd Bergmann wrote:
> On Monday 16 July 2007, Mark Zhan wrote:
> > -               cpm_uart_data.uart_clk = ppc_proc_freq;
> > +               if (strstr(model, "SMC")) {
> > +                       cpm_uart_dev = platform_device_register_simple("fsl-cpm-smc:uart",
> > +                                                       i, &r[0], 3);
> > +               } else if (strstr(model, "SCC")) {
> > +                       cpm_uart_dev = platform_device_register_simple("fsl-cpm-scc:uart",
> > +                                                       i, &r[0], 3);
> > +               }
> > 
> 
> You should probably use of_device_is_compatible() to check
> if a given device can be used by a particular driver.
> 
> 	Arnd <><

^ permalink raw reply

* Re: [PATCH 01/10] IB/ehca: Support for multiple event queues
From: Michael S. Tsirkin @ 2007-07-17  4:37 UTC (permalink / raw)
  To: Roland Dreier
  Cc: Joachim Fenkes, LKML, LinuxPPC-Dev, Hoang-Nam Nguyen, OF-General,
	Stefan Roscher
In-Reply-To: <ada1wf7vgfb.fsf@cisco.com>

> I still haven't seen much code using the feature or
> even any anecdotal information about the performance impact.

Here's some anecdotal evidence :)
http://lists.openfabrics.org/pipermail/general/2007-May/035758.html

-- 
MST

^ permalink raw reply

* Re: [PATCH 10/10] IB/ehca: Support large page MRs
From: Roland Dreier @ 2007-07-17  3:50 UTC (permalink / raw)
  To: Joachim Fenkes
  Cc: LKML, LinuxPPC-Dev, Christoph Raisch, OF-General, Stefan Roscher
In-Reply-To: <OFD7499E67.C6B5EA9B-ONC125731A.00715FED-C125731A.007484A7@de.ibm.com>

 > > Why the module parameter?  Is there any reason a user would want to
 > > turn this off?  Or conversely, why is it off by default?
 > 
 > We're pretty confident this new feature works, but as with all new and 
 > possibly experimental features, there are chances it might explode your 
 > machine when activated. So, like with the scaling code, we want the user 
 > to make the conscious decision of using this code instead of activating it 
 > by default.

OK, I guess.  So can we expect to, say, change the default to turning
it on for 2.6.24 and remove the option entirely (so it's always on) in
2.6.25?

 - R.

^ permalink raw reply

* Re: [PATCH 01/10] IB/ehca: Support for multiple event queues
From: Roland Dreier @ 2007-07-17  3:48 UTC (permalink / raw)
  To: Hoang-Nam Nguyen
  Cc: Joachim Fenkes, LKML, LinuxPPC-Dev, Christoph Raisch, OF-General,
	Stefan Roscher
In-Reply-To: <OF2377E85C.B05BDE94-ONC125731A.006E64A7-C125731A.00714F9E@de.ibm.com>

 > No, I've no figures to provide here. The background of this dist_eqs
 > option is actually to allow us testing across all event queues
 > without to change the testcases resp consumers to use certain
 > event queue number. Thus, I should comment it as EXPERIMENTAL?

Seems like it's just development/testing code that shouldn't escape
into the wild?

 > > I think I would rather hold off on multiple EQs for this merge window
 > > and plan on having something really solid and thought-out for 2.6.24.

 > Fair enough. However why don't let us gather experience with this
 > feature now? Should we remove dist_eqs option for more consistency?

As I said I definitely think the dist_eqs switch doesn't sound like
something we want to expose to people.

With that said I still am not sure about putting the multiple EQs
feature in this release.  All the infrastructure is there to make
experimenting with it fairly painless (just the low-level driver needs
to change), and I still haven't seen much code using the feature or
even any anecdotal information about the performance impact.

^ permalink raw reply

* [PATCH] Change the pci express controller node name from pci to pcie
From: Zang Roy-r61911 @ 2007-07-17  3:30 UTC (permalink / raw)
  To: Kumar Gala, Paul Mackerras; +Cc: linuxppc-dev list

From: Roy Zang <tie-fei.zang@freescale.com>

Change the pci express controller node name from pci
to pcie in device tree.

Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
---
Based on Kumar tree and my previous patch for 8544DS
board.

 arch/powerpc/boot/dts/mpc8544ds.dts    |    6 +++---
 arch/powerpc/boot/dts/mpc8548cds.dts   |    2 +-
 arch/powerpc/boot/dts/mpc8641_hpcn.dts |    4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/boot/dts/mpc8544ds.dts b/arch/powerpc/boot/dts/mpc8544ds.dts
index aab0bc6..3f33af6 100644
--- a/arch/powerpc/boot/dts/mpc8544ds.dts
+++ b/arch/powerpc/boot/dts/mpc8544ds.dts
@@ -174,7 +174,7 @@
 			reg = <8000 1000>;
 		};
 
-		pci@a000 {
+		pcie@a000 {
 			compatible = "fsl,mpc8548-pcie";
 			device_type = "pci";
 			#interrupt-cells = <1>;
@@ -197,7 +197,7 @@
 				>;
 		};
 
-		pci@9000 {
+		pcie@9000 {
 			compatible = "fsl,mpc8548-pcie";
 			device_type = "pci";
 			#interrupt-cells = <1>;
@@ -220,7 +220,7 @@
 				>;
 		};
 
-		pci@b000 {
+		pcie@b000 {
 			compatible = "fsl,mpc8548-pcie";
 			device_type = "pci";
 			#interrupt-cells = <1>;
diff --git a/arch/powerpc/boot/dts/mpc8548cds.dts b/arch/powerpc/boot/dts/mpc8548cds.dts
index c241e4f..4770a5b 100644
--- a/arch/powerpc/boot/dts/mpc8548cds.dts
+++ b/arch/powerpc/boot/dts/mpc8548cds.dts
@@ -310,7 +310,7 @@
 			device_type = "pci";
 		};
 		/* PCI Express */
-		pci@a000 {
+		pcie@a000 {
 			interrupt-map-mask = <f800 0 0 7>;
 			interrupt-map = <
 
diff --git a/arch/powerpc/boot/dts/mpc8641_hpcn.dts b/arch/powerpc/boot/dts/mpc8641_hpcn.dts
index e3cc97a..d1a8e0b 100644
--- a/arch/powerpc/boot/dts/mpc8641_hpcn.dts
+++ b/arch/powerpc/boot/dts/mpc8641_hpcn.dts
@@ -207,7 +207,7 @@
 			interrupt-parent = <&mpic>;
 		};
 
-		pci@8000 {
+		pcie@8000 {
 			compatible = "fsl,mpc8641-pcie";
 			device_type = "pci";
 			#interrupt-cells = <1>;
@@ -395,7 +395,7 @@
 
 		};
 
-		pci@9000 {
+		pcie@9000 {
 			compatible = "fsl,mpc8641-pcie";
 			device_type = "pci";
 			#interrupt-cells = <1>;
-- 
1.5.1

^ permalink raw reply related

* Re: [PATCH] Allow exec on 32-bit from readable, non-exec pages, with a warning.
From: David Gibson @ 2007-07-17  2:42 UTC (permalink / raw)
  To: Segher Boessenkool; +Cc: linuxppc-dev, Paul Mackerras, Arnd Bergmann
In-Reply-To: <89E626D4-D48D-4062-A996-E2DFFE8248E3@kernel.crashing.org>

On Wed, Jul 11, 2007 at 02:03:24AM +0200, Segher Boessenkool wrote:
> >> Yeah.  Giving the warning is a good thing though.
> >
> > No, it isn't; it's just noise, if we're not ever going to do anything
> > to prevent the behaviour - and we can't.
> 
> The same userland code will not run correctly on PPC64 or BookE
> systems.  Is that not a reason to warn?

Way back when, I distinctly recall aborting my plans to implement
per-page exec on 40x, precisely because of executables like this.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

^ permalink raw reply

* Re: [RFC][PATCH 6/8] Walnut DTS
From: Josh Boyer @ 2007-07-17  2:39 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, Yoder Stuart-B08248
In-Reply-To: <1184622446.25235.89.camel@localhost.localdomain>

On Tue, Jul 17, 2007 at 07:47:26AM +1000, Benjamin Herrenschmidt wrote:
> > That "typical practice" is inspired by the need to explicitly
> > put #address-cells and #size-cells into the device tree if you
> > want Linux to properly parse the device tree, even if the default
> > values would work perfectly (if Linux would work correctly,
> > that is).
> 
> Linux does handle default values in some areas. The problem with default
> values is that they are badly defined and the spec contains gray areas
> and contradictions as to what the default values should be in some
> circumstances. As a general matter, I dislike default values because
> they somewhat require background knowledge of what default values should
> be in different contexts to "read" a device-tree. To be simple, I
> believe default values are a bad idea.

Right.  See, there are people like me that don't know what the default values
are/should be.  Having them explicitly listed, even if it's redundant, serves
as a good learning aid.

Now, realistically I do know what the default is in this case.  But I only
learned that recently.  With hopefully more people starting to port things
over to arch/powerpc it might be a good idea to document them at least.
Otherwise, I fear we'd wind up repeating ourselves over and over.

Could we get a 'thou shalt not rely on defaults' added to
booting-without-of.txt?  Or maybe something less draconian ;).

josh

^ permalink raw reply

* Re: [PATCH 1/2] adb: create class devices for each adb device
From: Benjamin Herrenschmidt @ 2007-07-17  2:06 UTC (permalink / raw)
  To: Aristeu Rozanski; +Cc: linuxppc-dev, Paul Mackerras
In-Reply-To: <20070717013022.GB20841@cathedrallabs.org>


> or maybe generate disconnect/connect events so udev rules can reconfigure
> each keyboard?

Possibly... That would be obtained by just removing/re-adding the
devices (CONFIG_HOTPLUG is necessary to get the infrastructure for that
afaik).

Ben.

^ permalink raw reply

* Re: [patch 3/6] Add 8548 CDS PCI express controller node and PCI-X device node
From: Zang Roy-r61911 @ 2007-07-17  1:43 UTC (permalink / raw)
  To: Segher Boessenkool; +Cc: linuxppc-dev list, Paul Mackerras
In-Reply-To: <A76ECE36-4B72-4522-8EB0-A57359D0F938@kernel.crashing.org>

On Mon, 2007-07-16 at 23:00, Segher Boessenkool wrote:
> >>>> is this the only fix, I've already applied a version of this to
> my
> >>>> tree (with the irq sense fix) and just want to make sure there  
> >>>> isn't
> >>>> anything else.
> >>> I do not get more.
> >>> I'd like to enroll Segher's suggestion together with VIA chip  
> >>> function
> >>> in next step.
> >>> We need to make basic pcie work on 8548 CDS board first.
> >>
> >> If you can, please make the "compatible" properties correct
> >> ("pcie" instead of "pciex") before the patch goes in; it'll
> >> be less work total and less confusing in the end.
> >
> > I've taken care of that as well.
> >
> > They are now 'fsl,mpc8548-pcie, and 'fsl,mpc8641-pcie'
> 
> Perfect, thanks!
I noticed Kumar made some modifications according to the feedback, when
he applied the patch.
Thanks!
Roy

^ permalink raw reply

* Re: [PATCH 1/2] adb: create class devices for each adb device
From: Aristeu Rozanski @ 2007-07-17  1:30 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, Paul Mackerras
In-Reply-To: <1184628153.25235.111.camel@localhost.localdomain>

Hi Benjamin,
> I don't think class device is the right approach. Classes are supposed
> to be a functional binding, ie, in this case, input devices. ADB is a
> bus, and as such, you should create an adb bus type and and adb_device
> that is an extension of struct device, not struct class device.
ok, I'll work on that.

> There's also the question of what to do on ADB resets (such as when
> doing a sleep/wakeup cycle). Should we wipe out all devices and
> re-created them, thus loosing any settings ? Or should we have some way
> to attempt at re-matching existing ones ?
or maybe generate disconnect/connect events so udev rules can reconfigure
each keyboard?

-- 
Aristeu

^ permalink raw reply

* Re: [PATCH 3/3] 82xx: SBCPQ2 board platform support
From: Arnd Bergmann @ 2007-07-17  1:27 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: paulus
In-Reply-To: <469B3401.9030602@windriver.com>

On Monday 16 July 2007, Mark Zhan wrote:
> This patch addes the powerpc support to Wind River SBC PowerQUICCII 82xx board.

Hi Mark!

I've got lots of small comments about your code, but mostly it comes
down to one problem: your new platform code is not able to coexist
with other platforms because you hardcode information.

> +static struct resource m48t59_resources[] = {
> +	{
> +		.start	= SBCPQ2_RTC_BASE,
> +		.end	= SBCPQ2_RTC_BASE + SBCPQ2_RTC_SIZE - 1,
> +		.flags	= IORESOURCE_MEM,
> +	}, {
> +		.start	= SBCPQ2_M48T59_IRQ,
> +		.end	= SBCPQ2_M48T59_IRQ,
> +		.flags	= IORESOURCE_IRQ,
> +	},
> +	{ },
> +};

This is the kind of information that belongs into the device tree,
not hardcoded into the board code.

> +/**
> + * sbcpq2_pdev_init - Register the platform device for sbcpq2 board
> + */
> +static int __init sbcpq2_platdev_init(void)
> +{
> +	struct irq_desc *desc = irq_desc + SBCPQ2_M48T59_IRQ;

same for the interrupt number. Worse, this looks broken
because the descriptor array describes virtual interrupt
numbers, while SBCPQ2_M48T59_IRQ must be a physical number.
These are often the same, but there is no guarantee.

In order to get a virtual interrupt number for a given device,
you need to call irq_of_parse_and_map().

> +	/* Install a dummy irq chip for M48T59 RTC irq */
> +	if (desc->chip == &no_irq_chip)
> +		set_irq_handler(SBCPQ2_M48T59_IRQ, desc->handle_irq);
> +
> +	/* Register all platform devices for sbcpq2 */
> +	platform_add_devices(sbcpq2_devices, ARRAY_SIZE(sbcpq2_devices));
> +	return 0;
> +}
> +arch_initcall(sbcpq2_platdev_init);


> +/*
> + * For SBCPQ2 board, the interrupt of M48T59 RTC chip
> + * will generate a machine check exception. We use a
> + * fake irq to give the platform machine_check_exception() hook
> + * a chance to call the driver ISR. If IRQ_HANDLED is returned,
> + * then we will survive from the machine check exception.
> + */
> +static int sbcpq2_mach_check(struct pt_regs *regs)
> +{
> +	int recover = 0;
> +	struct irq_desc *desc = irq_desc + SBCPQ2_M48T59_IRQ;
> +
> +	struct irqaction *action = desc->action;
> +
> +	while (action && (action->dev_id != &m48t59_rtc))
> +		action = action->next;
> +
> +	/* Try to call m48t59 RTC driver ISR */
> +	if (action && action->handler)
> +		recover = action->handler(SBCPQ2_M48T59_IRQ, &m48t59_rtc);
> +
> +	return recover;
> +}

What you do here looks really scary, but maybe I'm just misunderstanding
it completely. Why don't you just register your rtc handler function
as the machine check handler instead of going through various indirections?

> +static void __init sbcpq2_init_IRQ(void)
> +{
> +	struct device_node *np;
> +	struct resource res;
> +
> +	np = of_find_compatible_node(NULL, "cpm-pic", "CPM2");
> +	if (np == NULL) {
> +		printk(KERN_ERR "PIC init: can not find cpm-pic node\n");
> +		return;
> +	}

This looks like your device tree is wrong. Shouldn't the interrupt
controller have device_type="interrupt-controller" and a specific
compatible property instead of having the name in the device_type?

> +static void __init sbcpq2_setup_arch(void)
> +{
> +	struct device_node *np;
> +	volatile memctl_cpm2_t *mc;

not volatile, but __iomem!

> +	unsigned char * eeprom_base;
> +	int i = 0;
> +
> +#ifdef CONFIG_CPM2
> +	cpm2_reset();
> +#endif
> +
> +	/*
> +	 * Make sure that we have the right CS# setting
> +	 */
> +	mc = &cpm2_immr->im_memctl;
> +
> +	/* Boot Flash is the on-board flash */
> +	mc->memc_br0 = (SBCPQ2_BOOT_FLASH_BASE & 0xFFFF8000) | 0x0801;
> +	mc->memc_or0 = 0xFFE00896;

consequently, this needs to use out_be32 or similar.
Where does SBCPQ2_BOOT_FLASH_BASE come from? Shouldn't that be set
up by the boot loader to match the device tree?

> +		model = (char *)of_get_property(np, "model", NULL);
> +		if (!model)
> +			continue;

The cast is not needed here.
> +
> +		id = of_get_property(np, "device-id", NULL);
> +		if (!id)
> +			continue;
> +
> +		macaddr = (unsigned char *)of_get_mac_address(np);
> +		if (!macaddr)
> +			continue;

or here.

> +		if (strstr(model, "FCC"))
> +			eeprom_ofs = SBCPQ2_FCC1_MACADDR_OFS;
> +		else if (strstr(model, "SCC"))
> +			eeprom_ofs = SBCPQ2_SCC1_MACADDR_OFS;
> +		eeprom_ofs += ((*id) - 1) * 6;

of_device_is_compatible()


> +		for (j = 0; j < 6; j++)
> +			*(macaddr + j) = *(eeprom_base + eeprom_ofs + j);

in_8().

> +	}
> +	iounmap(eeprom_base);
> +}
> +
> +/*
> + * Called very early, device-tree isn't unflattened
> + */
> +static int __init sbcpq2_probe(void)
> +{
> +	/* We always match for now, eventually we should look at
> +	 * the flat dev tree to ensure this is the board we are
> +	 * supposed to run on
> +	 */
> +	return 1;
> +}

Don't write why the code is wrong -- just fix it.

> +/* For our show_cpuinfo hooks. */
> +#define CPUINFO_VENDOR		"Wind River"
> +#define CPUINFO_MACHINE		"SBC PowerQUICCII 82xx"

Not in a header file please.

> +/*
> + * Wind River SBC PowerQUICCII 82xx Physical Memory Map (CS0 for OnBoard Flash)
> + *
> + *   0x00000000 - 0x07FFFFFF	CS2, 128 MB DIMM SDRAM
> + *   0x08000000 - 0x0FFFFFFF	CS3, 128 MB DIMM SDRAM
> + *   0x12000000 - 0x12100000	CS8, ATM
> + *   0x20000000 - 0x20FFFFFF	CS4, 16 MB Local Bus SDRAM
> + *   0x21000000 - 0x21001FFF	CS7, Control EPLD
> + *   0x22000000 - 0x22001FFF	CS5, 8KB EEPROM
> + *   0x22002000 - 0x22003FFF	CS5, visionPORT
> + *   0x22004000 - 0x22005FFF	CS5, User Switches
> + *   0x22006000 - 0x22007FFF	CS5, STATUS
> + *   0x22008000 - 0x22009FFF	CS5, i8259 interrupt controller
> + *   0x2200A000 - 0x2200BFFF	CS5, LED (Seven Segment Display)
> + *   0x80000000 - 0x80001FFF	CS11, RTC
> + *   0xE0000000 - 0xE3FFFFFF	CS6, 64 MB DIMM Flash
> + *   0xE4000000 - 0xE7FFFFFF	CS1, 64 MB DIMM Flash
> + *   0xFE000000 - 0xFFFFFFFF	CS0, 2 MB Boot Flash
> + *   0xF0000000 - 0xF0020000	MPC82xx Internal Registers Space
> + */
> +#define SBCPQ2_SDRAM_BASE		0x00000000
> +#define SBCPQ2_SDRAM_SIZE		0x10000000
> +
> +#define SBCPQ2_LOCAL_SDRAM_BASE		0x20000000
> +#define SBCPQ2_LOCAL_SDRAM_SIZE		0x1000000
> +
> +#define SBCPQ2_EPLD_BASE		0x21000000
> +#define SBCPQ2_EPLD_SIZE		0x2000
> +
> +#define SBCPQ2_EEPROM_BASE		0x22000000
> +#define SBCPQ2_EEPROM_SIZE		0x2000
> +
> +/* User Switches SW5 */
> +#define SBCPQ2_USER_SW_BASE		0x22004000
> +#define SBCPQ2_USER_SW_SIZE		0x2000
> +
> +#define SBCPQ2_STATUS_BASE		0x22006000
> +#define SBCPQ2_STATUS_SIZE		0x2000
> +
> +#define SBCPQ2_I8259_BASE		0x22008000
> +#define SBCPQ2_I8259_SIZE		0x2000
> +
> +/* Seven Segment Display LED D46 */
> +#define SBCPQ2_LED_BASE			0x2200A000
> +#define SBCPQ2_LED_SIZE			0x2000
> +
> +#define SBCPQ2_RTC_BASE			0x80000000
> +#define SBCPQ2_RTC_SIZE			0x2000
> +
> +#define SBCPQ2_BOOT_FLASH_BASE		0xFE000000
> +#define SBCPQ2_BOOT_FLASH_SIZE		0x00200000
> +
> +#define SBCPQ2_DIMM_FLASH_BASE		0xE0000000
> +#define SBCPQ2_DIMM_FLASH_SIZE		0x04000000
> +
> +#define CPM_MAP_ADDR			0xF0000000
> +#define CPM_IRQ_OFFSET			0

All this is in the device tree already, so don't duplicate it here.

> +/*
> + * The offset of ethernet MAC addr within EEPROM
> + */
> +#define SBCPQ2_FCC1_MACADDR_OFS		0x60
> +#define SBCPQ2_FCC2_MACADDR_OFS		0x66
> +#define SBCPQ2_FCC3_MACADDR_OFS		0x72
> +#define SBCPQ2_SCC1_MACADDR_OFS		0x78

Likewise, the mac address is in the device tree, so no need
to tell the kernel how to read it.

> +/*
> + * The following IRQs are routed to i8259 PIC.
> + *
> + * NOTE: i8259 PIC is cascaded to SIU_INT_IRQ6 of CPM2 interrupt controller
> + */
> +#define SBCPQ2_PC_IRQA		(NR_SIU_INTS+0)
> +#define SBCPQ2_PC_IRQB		(NR_SIU_INTS+1)
> +#define SBCPQ2_MPC185_IRQ	(NR_SIU_INTS+2)
> +#define SBCPQ2_ATM_IRQ		(NR_SIU_INTS+3)
> +#define SBCPQ2_PIRQA		(NR_SIU_INTS+4)
> +#define SBCPQ2_PIRQB		(NR_SIU_INTS+5)
> +#define SBCPQ2_PIRQC		(NR_SIU_INTS+6)
> +#define SBCPQ2_PIRQD		(NR_SIU_INTS+7)

Again, these are in the device tree, so don't put them here.

> +/* cpm serial driver works with constants below */
> +#define SIU_INT_SMC1		((uint)0x04+CPM_IRQ_OFFSET)
> +#define SIU_INT_SMC2		((uint)0x05+CPM_IRQ_OFFSET)
> +#define SIU_INT_SCC1		((uint)0x28+CPM_IRQ_OFFSET)
> +#define SIU_INT_SCC2		((uint)0x29+CPM_IRQ_OFFSET)
> +#define SIU_INT_SCC3		((uint)0x2a+CPM_IRQ_OFFSET)
> +#define SIU_INT_SCC4		((uint)0x2b+CPM_IRQ_OFFSET)

What are these for? If you need them in the device driver, just put
them in there, not in a header file. Also, you should make
sure not to pollute the global name space, so they should
be named SBCPQ2_SIU_INT_* to make it clear that they are board
specific.

> +#ifdef CONFIG_SBCPQ2
> +#include <platforms/82xx/sbcpq2.h>
> +#endif

Never put #ifdef around an #include.

> +
>   #ifdef CONFIG_PCI_8260
>   #include <platforms/82xx/m82xx_pci.h>
>   #endif

Kill this #ifdef as well while you're there. If you get name space
conflicts, just rename the symbols to make them unique.

> +/ {
> +	model = "SBCPQ2";
> +	compatible = "mpc82xx";
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +	linux,phandle = <100>;

Don't put explicit phandles here. If you need a reference, do it
symbolically.

	Arnd <><

^ permalink raw reply

* [PATCH 3/4] phy: Fix Vitesse RGMII-ID support
From: Andy Fleming @ 2007-07-17  1:27 UTC (permalink / raw)
  To: jgarzik, paulus, galak; +Cc: netdev, linuxppc-dev
In-Reply-To: <11846356233423-git-send-email-afleming@freescale.com>

The Vitesse PHY on the 8641D needs to be set up with internal delay to
work in RGMII mode.  So we add skew when it is set to RGMII_ID mode.

Signed-off-by: Andy Fleming <afleming@freescale.com>
Signed-off-by: Haruki Dai <Dai.Haruki@freescale.com>
Signed-off-by: Haiying Wang <Haiying.Wang@freescale.com>
---
 drivers/net/phy/vitesse.c |   23 ++++++++++++++++++++---
 1 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/drivers/net/phy/vitesse.c b/drivers/net/phy/vitesse.c
index f39ab76..6a53856 100644
--- a/drivers/net/phy/vitesse.c
+++ b/drivers/net/phy/vitesse.c
@@ -21,6 +21,10 @@
 /* Vitesse Extended Control Register 1 */
 #define MII_VSC8244_EXT_CON1           0x17
 #define MII_VSC8244_EXTCON1_INIT       0x0000
+#define MII_VSC8244_EXTCON1_TX_SKEW_MASK	0x0c00
+#define MII_VSC8244_EXTCON1_RX_SKEW_MASK	0x0300
+#define MII_VSC8244_EXTCON1_TX_SKEW	0x0800
+#define MII_VSC8244_EXTCON1_RX_SKEW	0x0200
 
 /* Vitesse Interrupt Mask Register */
 #define MII_VSC8244_IMASK		0x19
@@ -39,7 +43,7 @@
 
 /* Vitesse Auxiliary Control/Status Register */
 #define MII_VSC8244_AUX_CONSTAT        	0x1c
-#define MII_VSC8244_AUXCONSTAT_INIT    	0x0004
+#define MII_VSC8244_AUXCONSTAT_INIT    	0x0000
 #define MII_VSC8244_AUXCONSTAT_DUPLEX  	0x0020
 #define MII_VSC8244_AUXCONSTAT_SPEED   	0x0018
 #define MII_VSC8244_AUXCONSTAT_GBIT    	0x0010
@@ -51,6 +55,7 @@ MODULE_LICENSE("GPL");
 
 static int vsc824x_config_init(struct phy_device *phydev)
 {
+	int extcon;
 	int err;
 
 	err = phy_write(phydev, MII_VSC8244_AUX_CONSTAT,
@@ -58,8 +63,20 @@ static int vsc824x_config_init(struct phy_device *phydev)
 	if (err < 0)
 		return err;
 
-	err = phy_write(phydev, MII_VSC8244_EXT_CON1,
-			MII_VSC8244_EXTCON1_INIT);
+	extcon = phy_read(phydev, MII_VSC8244_EXT_CON1);
+
+	if (extcon < 0)
+		return err;
+
+	extcon &= ~(MII_VSC8244_EXTCON1_TX_SKEW_MASK |
+			MII_VSC8244_EXTCON1_RX_SKEW_MASK);
+
+	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
+		extcon |= (MII_VSC8244_EXTCON1_TX_SKEW |
+				MII_VSC8244_EXTCON1_RX_SKEW);
+
+	err = phy_write(phydev, MII_VSC8244_EXT_CON1, extcon);
+
 	return err;
 }
 
-- 
1.5.0.2.230.gfbe3d-dirty

^ permalink raw reply related

* [PATCH 2/4] gianfar: Add phy-connection-type to gianfar nodes
From: Andy Fleming @ 2007-07-17  1:26 UTC (permalink / raw)
  To: jgarzik, paulus, galak; +Cc: netdev, linuxppc-dev
In-Reply-To: <1184635622653-git-send-email-afleming@freescale.com>

The TSEC/eTSEC automatically detect their PHY interface type, unless
the type is RGMII-ID (RGMII with internal delay).  In that situation,
it just detects RGMII.  In order to fix this, we need to pass in rgmii-id
if that is the connection type.

Signed-off-by: Andy Fleming <afleming@freescale.com>
---
 Documentation/powerpc/booting-without-of.txt |    6 ++++++
 arch/powerpc/boot/dts/mpc8641_hpcn.dts       |    4 ++++
 2 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/Documentation/powerpc/booting-without-of.txt b/Documentation/powerpc/booting-without-of.txt
index d42d981..a41b6b1 100644
--- a/Documentation/powerpc/booting-without-of.txt
+++ b/Documentation/powerpc/booting-without-of.txt
@@ -1241,6 +1241,12 @@ platforms are moved over to use the flattened-device-tree model.
       network device.  This is used by the bootwrapper to interpret
       MAC addresses passed by the firmware when no information other
       than indices is available to associate an address with a device.
+    - phy-connection-type : a string naming the controller/PHY interface type,
+      i.e., "mii" (default), "rmii", "gmii", "rgmii", "rgmii-id", "sgmii",
+      "tbi", or "rtbi".  This property is only really needed if the connection
+      is of type "rgmii-id", as all other connection types are detected by
+      hardware.
+
 
   Example:
 
diff --git a/arch/powerpc/boot/dts/mpc8641_hpcn.dts b/arch/powerpc/boot/dts/mpc8641_hpcn.dts
index 260b264..9849073 100644
--- a/arch/powerpc/boot/dts/mpc8641_hpcn.dts
+++ b/arch/powerpc/boot/dts/mpc8641_hpcn.dts
@@ -121,6 +121,7 @@
 			interrupts = <1d 2 1e 2 22 2>;
 			interrupt-parent = <&mpic>;
 			phy-handle = <&phy0>;
+			phy-connection-type = "rgmii-id";
 		};
 
 		ethernet@25000 {
@@ -134,6 +135,7 @@
 			interrupts = <23 2 24 2 28 2>;
 			interrupt-parent = <&mpic>;
 			phy-handle = <&phy1>;
+			phy-connection-type = "rgmii-id";
 		};
 		
 		ethernet@26000 {
@@ -147,6 +149,7 @@
 			interrupts = <1F 2 20 2 21 2>;
 			interrupt-parent = <&mpic>;
 			phy-handle = <&phy2>;
+			phy-connection-type = "rgmii-id";
 		};
 
 		ethernet@27000 {
@@ -160,6 +163,7 @@
 			interrupts = <25 2 26 2 27 2>;
 			interrupt-parent = <&mpic>;
 			phy-handle = <&phy3>;
+			phy-connection-type = "rgmii-id";
 		};
 		serial@4500 {
 			device_type = "serial";
-- 
1.5.0.2.230.gfbe3d-dirty

^ permalink raw reply related

* [PATCH 4/4] gianfar: Fix RGMII-ID handling in gianfar
From: Andy Fleming @ 2007-07-17  1:27 UTC (permalink / raw)
  To: jgarzik, paulus, galak; +Cc: netdev, linuxppc-dev
In-Reply-To: <11846356241295-git-send-email-afleming@freescale.com>

The TSEC/eTSEC can detect the interface to the PHY automatically,
but it isn't able to detect whether the RGMII connection needs internal
delay.  So we need to detect that change in the device tree, propagate
it to the platform data, and then check it if we're in RGMII.  This fixes
a bug on the 8641D HPCN board where the Vitesse PHY doesn't use the delay
for RGMII.

Signed-off-by: Andy Fleming <afleming@freescale.com>
---
 arch/powerpc/sysdev/fsl_soc.c |    9 +++++++++
 drivers/net/gianfar.c         |   12 +++++++++++-
 include/linux/fsl_devices.h   |    1 +
 3 files changed, 21 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/fsl_soc.c
index cad1757..64efee6 100644
--- a/arch/powerpc/sysdev/fsl_soc.c
+++ b/arch/powerpc/sysdev/fsl_soc.c
@@ -197,6 +197,7 @@ static int __init gfar_of_init(void)
 		struct gianfar_platform_data gfar_data;
 		const unsigned int *id;
 		const char *model;
+		const char *ctype;
 		const void *mac_addr;
 		const phandle *ph;
 		int n_res = 2;
@@ -254,6 +255,14 @@ static int __init gfar_of_init(void)
 			    FSL_GIANFAR_DEV_HAS_VLAN |
 			    FSL_GIANFAR_DEV_HAS_EXTENDED_HASH;
 
+		ctype = of_get_property(np, "phy-connection-type", NULL);
+
+		/* We only care about rgmii-id.  The rest are autodetected */
+		if (ctype && !strcmp(ctype, "rgmii-id"))
+			gfar_data.interface = PHY_INTERFACE_MODE_RGMII_ID;
+		else
+			gfar_data.interface = PHY_INTERFACE_MODE_MII;
+
 		ph = of_get_property(np, "phy-handle", NULL);
 		phy = of_find_node_by_phandle(*ph);
 
diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
index d7a1a58..f926905 100644
--- a/drivers/net/gianfar.c
+++ b/drivers/net/gianfar.c
@@ -420,8 +420,18 @@ static phy_interface_t gfar_get_interface(struct net_device *dev)
 	if (ecntrl & ECNTRL_REDUCED_MODE) {
 		if (ecntrl & ECNTRL_REDUCED_MII_MODE)
 			return PHY_INTERFACE_MODE_RMII;
-		else
+		else {
+			phy_interface_t interface = priv->einfo->interface;
+
+			/*
+			 * This isn't autodetected right now, so it must
+			 * be set by the device tree or platform code.
+			 */
+			if (interface == PHY_INTERFACE_MODE_RGMII_ID)
+				return PHY_INTERFACE_MODE_RGMII_ID;
+
 			return PHY_INTERFACE_MODE_RGMII;
+		}
 	}
 
 	if (priv->einfo->device_flags & FSL_GIANFAR_DEV_HAS_GIGABIT)
diff --git a/include/linux/fsl_devices.h b/include/linux/fsl_devices.h
index 73710d6..2de358f 100644
--- a/include/linux/fsl_devices.h
+++ b/include/linux/fsl_devices.h
@@ -53,6 +53,7 @@ struct gianfar_platform_data {
 	u32	bus_id;
 	u32	phy_id;
 	u8	mac_addr[6];
+	phy_interface_t interface;
 };
 
 struct gianfar_mdio_data {
-- 
1.5.0.2.230.gfbe3d-dirty

^ permalink raw reply related


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