LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH] ibm_newemac: Increase number of default rx-/tx-buffers
From: Stefan Roese @ 2008-01-12  7:26 UTC (permalink / raw)
  To: benh; +Cc: netdev, linuxppc-dev
In-Reply-To: <1200089457.6896.65.camel@pasglop>

On Friday 11 January 2008, Benjamin Herrenschmidt wrote:
> On Fri, 2008-01-11 at 09:48 -0800, Eugene Surovegin wrote:
> > On Sat, Jan 05, 2008 at 01:38:17PM +0100, Stefan Roese wrote:
> > > On Saturday 05 January 2008, Benjamin Herrenschmidt wrote:
> > > > On Sat, 2008-01-05 at 10:50 +0100, Stefan Roese wrote:
> > > > > Performance tests done by AMCC have shown that 256 buffer increase
> > > > > the performance of the Linux EMAC driver. So let's update the
> > > > > default values to match this setup.
> > > > >
> > > > > Signed-off-by: Stefan Roese <sr@denx.de>
> > > > > ---
> > > >
> > > > Do we have the numbers ? Did they also measure latency ?
> > >
> > > I hoped this question would not come. ;) No, unfortunately I don't have
> > > any numbers. Just the recommendation from AMCC to always use 256
> > > buffers.
> >
> > This cannot be true for all chips. Default numbers I selected weren't
> > random. In particular, 256 for Tx doesn't make a lot of sense for 405.
> > You just gonna waste memory.

This may be the case with the "old" 405 PPC's. But with the new ones coming 
out right now, like the up to 666MHz 405EX with GBit support, 256 could be an 
improvement. I still owe you figures though. Will try to do some testing in a 
short while.

> > I'd be quite reluctant to follow such advices from AMCC without actual
> > details.
>
> I think we can make defaults based on other config options nowadays. Not
> very nice but we could do things like
>
> 	default 128 if PPC_40x
> 	default 256
>
> Or even more detailed.

We shouldn't make it too complicated. We can always select different settings 
in the defconfig file. My thinking here is to better wast a little memory 
with a potential performance improvement. Just me 0.02$

Best regards,
Stefan

^ permalink raw reply

* Re: PCI Failed to allocate mem for PCI ROM
From: Grant Grundler @ 2008-01-12  7:27 UTC (permalink / raw)
  To: Kumar Gala; +Cc: Greg KH, linux-pci, LKML, linuxppc-dev list
In-Reply-To: <E1025115-6BE2-46B6-8416-A5ADD506E2ED@kernel.crashing.org>

On Fri, Jan 11, 2008 at 02:27:16PM -0600, Kumar Gala wrote:
>>> I'm getting the following message from the kernel on an embedded ppc32
>>> system:
>>>
>>> PCI: Failed to allocate mem resource #9:100000@e0000000 for 0000:00:00.0
>>>
>>> The HW setup is a PCIe host controller and an e1000 NIC card.
...
> I'm happy to debug, is the fact that the resno == 9 ok or does that seem 
> wrong?

That is fine for the Bridge. See include/linux/pci.h :
#define PCI_ROM_RESOURCE        6
#define PCI_BRIDGE_RESOURCES    7
#define PCI_NUM_RESOURCES       11

IIRC, Bridges may have two 32-bit or one 64-bit BAR, Expansion ROM BAR and
three "range" registers: IO Port, MMIO (Prefetchable and non-prefetchable).
The BRIDGE_RESOURCES (7-10) are what failed to be assigned for some reason.

Looking at setup-bus.c:pci_bridge_check_ranges(), I'm concluding that:
[7] is IO Range.
[8] is MMIO
[9] is Prefetchable MMIO
[10] no clue...maybe used by host PCI bus controllers.

0x100000 is 1MB and would be the minimum MMIO range that can be allocated.
So that looks right too. Probably need to find out what is allocating
0xe0000000 instead.

hth,
grant

^ permalink raw reply

* Re: [PATCH] ibm_newemac: Increase number of default rx-/tx-buffers
From: Benjamin Herrenschmidt @ 2008-01-12  7:57 UTC (permalink / raw)
  To: Stefan Roese; +Cc: netdev, linuxppc-dev
In-Reply-To: <200801120826.53091.sr@denx.de>


On Sat, 2008-01-12 at 08:26 +0100, Stefan Roese wrote:
> 
> We shouldn't make it too complicated. We can always select different
> settings 
> in the defconfig file. My thinking here is to better wast a little
> memory 
> with a potential performance improvement. Just me 0.02$

If it gets really critical, then we can move those settings to the
device-tree.

Ben.

^ permalink raw reply

* Re: Device Tree & PCI
From: Benjamin Herrenschmidt @ 2008-01-12  8:27 UTC (permalink / raw)
  To: Rune Torgersen; +Cc: linuxppc-dev
In-Reply-To: <DCEAAC0833DD314AB0B58112AD99B93B03D9A381@ismail.innsys.innovsys.com>


>         pci_bridge@12 {
>     		interrupt-map-mask = <0f800 0 0 7>;
>     		interrupt-map = <
>     		                /* IDSEL 0x10 */
>     		                00008000 0 0 1 &PCI_INT 1
> 
>     		                /* IDSEL 0x11 */
>     		                00008800 0 0 1 &PCI_INT 2
>     		                
>     		                /* IDSEL 0x12 */
>     		                00009000 0 0 1 &PCI_INT 3
>     		                
>     		                /* IDSEL 0x13 */
>     		                00009800 0 0 1 &PCI_INT 4>;
>         };

Your PCI bridge node needs more than those 2 properties. At the very
least it needs device_type "pci" and name "pci" (rather than pci-bridge)
to make the kernel parser happier, it also needs a proper class code and
a "reg" property providing its configuration address (devfn) so that the
kernel can do the matching between what it finds on the PCI bus and that
node.

Basically, you need to make sure the function pci_device_to_OF_node()
works for your bridge or the interrupt resolution will not work. I'd
suggest you add debugging to that function to dump what it's doing so
you can get your tree in order.

Cheers,
Ben.

^ permalink raw reply

* Re: [i2c] [PATCH 1/5] Implement module aliasing for i2c to translate from device tree names
From: Jean Delvare @ 2008-01-12  8:46 UTC (permalink / raw)
  To: Jon Smirl; +Cc: linuxppc-dev, i2c, linux-kernel
In-Reply-To: <20080111202015.65c3c160@hyperion.delvare>

Hi Jon,

On Fri, 11 Jan 2008 20:20:15 +0100, Jean Delvare wrote:
> > +{
> > +	/* only powerpc drivers implement the id_table,
> > +	 * it is empty on other platforms */
> > +	if (id) {
> > +		while (id->name[0]) {
> > +			if (strcmp(client->driver_name, id->name) == 0)
> 
> This doesn't look right to me. You should be comparing client->name,
> not client->driver_name, with id->name. Where id_table is implemented,
> client->driver_name might not even be set. I see that the next patch in
> the series makes use of client->driver_name as well, so your code
> "works"... but this ain't correct still.

Err, scratch this (and related comments), I just realized what you were
trying to do. That's different from what I had in mind and so I read
your code wrong. I'll read it (and test it) again not making this
incorrect assumption and my comments will likely be different after
that. Well, I still think that it needs to be changed a bit, but
probably not in the direction I suggested at first (which, I realize
now, has its own share of issues - so it's not fair to me to point you
there.)

Sorry for the trouble. I'll post updated comments later today, but I'm
also pretty busy with other issues, some of which need to be solved
before 2.6.24 is released so I can't really delay them.

-- 
Jean Delvare

^ permalink raw reply

* Re: [PATCH] ibm_newemac: Increase number of default rx-/tx-buffers
From: Eugene Surovegin @ 2008-01-12  9:08 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: netdev, Stefan Roese, linuxppc-dev
In-Reply-To: <1200124644.6896.99.camel@pasglop>

On Sat, Jan 12, 2008 at 06:57:24PM +1100, Benjamin Herrenschmidt wrote:
> 
> On Sat, 2008-01-12 at 08:26 +0100, Stefan Roese wrote:
> > 
> > We shouldn't make it too complicated. We can always select different
> > settings 
> > in the defconfig file. My thinking here is to better wast a little
> > memory 
> > with a potential performance improvement. Just me 0.02$
> 
> If it gets really critical, then we can move those settings to the
> device-tree.

Come on guys, it's not that critical. I guess I just don't trust a 
certain company :)

-- 
Eugene

^ permalink raw reply

* Re: [i2c] [PATCH 0/5] Version 17, series to add device tree naming to i2c
From: Jean Delvare @ 2008-01-12  9:08 UTC (permalink / raw)
  To: Jon Smirl; +Cc: linuxppc-dev, i2c, linux-kernel
In-Reply-To: <9e4733910801111216g7dfee16ev72ec3c58e5d6c746@mail.gmail.com>

On Fri, 11 Jan 2008 15:16:57 -0500, Jon Smirl wrote:
> On 1/11/08, Jean Delvare <khali@linux-fr.org> wrote:
> > Secondly, it promotes OF device names as acceptable aliases. This I
> > don't think I agree with. While I see some value in moving the OF name
> > -> Linux name translation to the drivers themselves (even though I
> > don't see this as a mandatory move either), this doesn't imply that OF
> > names should be used as aliases. I don't like the idea that different
> > architectures will name the same device differently in a visible way.
> > This could easily break user-space code that makes assumptions on the
> > device names (libsensors comes to mind.) So, I think that this part
> > will need some more discussion.
> 
> They're aliases.  On the x86 my e1000 Ethernet driver loads using this
> alias name:
> "pci:v00008086d00001010sv*sd*bc*sc*i*"
> In fact, the e1000 driver has 63 alias names in addition to "e1000"
> 
> But it's still the e1000 driver after it is loaded.
> jonsmirl@terra:/home/linux/drivers/net/e1000$ lsmod | grep e1000
> e1000                 115968  0
> 
> Loading a I2C driver with an OF alias name is not going to change the
> module name after it is loaded. In fact, once the module is in memory
> there's no way to tell what name was used to load it.

Of course. That's not what I was worried about... what I was worried
about is something your patch set doesn't do but I misread the code and
I thought it was doing. I'll read it again before I make more comments
on this.

> OF device names are set by the Open Firmware committee. It is not
> reasonable to force the Linux names back into Open Firmware since this
> would force the other operating systems using Open Firmware to adopt
> the Linux names.

I never meant to force the Linux names into Open Firmware. It wouldn't
make sense especially when the Linux names are invented by random
contributors with no specific rules, and can even change over time.

What I meant is that the translation from Open Firmware device name to
Linux device name could happen in different ways. Making module aliases
out of the is one possibility but this is not the only one.

I am curious why the translation could not happen "offline". As I
understand it, you're getting the device names from these .dts files.
However you're not parsing them in the kernel directly, are you? I
presume that you have some tool that converts these files into C code
that the kernel can use? This conversion tool could translate the names.

> This issue hasn't been visible before since there was a global table
> in the PowerPC code mapping all known Open Firmware names into linux
> names. Keeping this as a global table doesn't scale. The mapping needs
> to be done by each device individually.

Looking at your patch set, I see only 11 entries in the table (in
arch/powerpc/sysdev/fsl_soc.c) that patch #2 deletes. Are there more in
other files? I'm asking because 11 entries hardly qualifies as "doesn't
scale". I sure hope that you're not doing all this for the sole purpose
of getting rid of this 11-element table.

-- 
Jean Delvare

^ permalink raw reply

* Re: PCI Failed to allocate mem for PCI ROM
From: Ivan Kokshaysky @ 2008-01-12 15:12 UTC (permalink / raw)
  To: Grant Grundler; +Cc: linuxppc-dev list, Greg KH, linux-pci, LKML
In-Reply-To: <20080112072705.GB7234@colo.lackof.org>

On Sat, Jan 12, 2008 at 12:27:05AM -0700, Grant Grundler wrote:
> Looking at setup-bus.c:pci_bridge_check_ranges(), I'm concluding that:
> [7] is IO Range.
> [8] is MMIO
> [9] is Prefetchable MMIO
> [10] no clue...maybe used by host PCI bus controllers.

#10 is for cardbus bridges, IIRC.

> 0x100000 is 1MB and would be the minimum MMIO range that can be allocated.
> So that looks right too. Probably need to find out what is allocating
> 0xe0000000 instead.

I believe that the setup-bus code just tried to allocate 1M ROM of some
device using prefetchable window of the bridge 0000:00:00.0. That failed,
probably because there is no MEM space left on the root bus, which is
perfectly OK. Then, the ROM might be successfully allocated in the
non-prefetchable MMIO space.

So, I don't see any problem in this situation. The only thing I could
suggest is to lower the log level in that message from KERN_ERR to
KERN_WARNING...

Ivan.

^ permalink raw reply

* Re: [i2c] [PATCH 0/5] Version 17, series to add device tree naming to i2c
From: Jon Smirl @ 2008-01-12 16:00 UTC (permalink / raw)
  To: Jean Delvare; +Cc: linuxppc-dev, i2c, linux-kernel
In-Reply-To: <20080112100835.616d6bd9@hyperion.delvare>

On 1/12/08, Jean Delvare <khali@linux-fr.org> wrote:
> On Fri, 11 Jan 2008 15:16:57 -0500, Jon Smirl wrote:
> > On 1/11/08, Jean Delvare <khali@linux-fr.org> wrote:
> > > Secondly, it promotes OF device names as acceptable aliases. This I
> > > don't think I agree with. While I see some value in moving the OF name
> > > -> Linux name translation to the drivers themselves (even though I
> > > don't see this as a mandatory move either), this doesn't imply that OF
> > > names should be used as aliases. I don't like the idea that different
> > > architectures will name the same device differently in a visible way.
> > > This could easily break user-space code that makes assumptions on the
> > > device names (libsensors comes to mind.) So, I think that this part
> > > will need some more discussion.
> >
> > They're aliases.  On the x86 my e1000 Ethernet driver loads using this
> > alias name:
> > "pci:v00008086d00001010sv*sd*bc*sc*i*"
> > In fact, the e1000 driver has 63 alias names in addition to "e1000"
> >
> > But it's still the e1000 driver after it is loaded.
> > jonsmirl@terra:/home/linux/drivers/net/e1000$ lsmod | grep e1000
> > e1000                 115968  0
> >
> > Loading a I2C driver with an OF alias name is not going to change the
> > module name after it is loaded. In fact, once the module is in memory
> > there's no way to tell what name was used to load it.
>
> Of course. That's not what I was worried about... what I was worried
> about is something your patch set doesn't do but I misread the code and
> I thought it was doing. I'll read it again before I make more comments
> on this.
>
> > OF device names are set by the Open Firmware committee. It is not
> > reasonable to force the Linux names back into Open Firmware since this
> > would force the other operating systems using Open Firmware to adopt
> > the Linux names.
>
> I never meant to force the Linux names into Open Firmware. It wouldn't
> make sense especially when the Linux names are invented by random
> contributors with no specific rules, and can even change over time.
>
> What I meant is that the translation from Open Firmware device name to
> Linux device name could happen in different ways. Making module aliases
> out of the is one possibility but this is not the only one.
>
> I am curious why the translation could not happen "offline". As I
> understand it, you're getting the device names from these .dts files.
> However you're not parsing them in the kernel directly, are you? I
> presume that you have some tool that converts these files into C code
> that the kernel can use? This conversion tool could translate the names.

Those dts files are for embedded devices that were specifically
developed for Linux. All of the PowerPC Macs in the world have a
device tree in ROM that was developed by Apple following the Open
Firmware standard. Same thing for Sun boxes, but I'm not working on
those.

The kernel has an existing mechanism for handling translations like
these, the alias scheme.


> > This issue hasn't been visible before since there was a global table
> > in the PowerPC code mapping all known Open Firmware names into linux
> > names. Keeping this as a global table doesn't scale. The mapping needs
> > to be done by each device individually.
>
> Looking at your patch set, I see only 11 entries in the table (in
> arch/powerpc/sysdev/fsl_soc.c) that patch #2 deletes. Are there more in
> other files? I'm asking because 11 entries hardly qualifies as "doesn't
> scale". I sure hope that you're not doing all this for the sole purpose
> of getting rid of this 11-element table.

Currently developers add entries to the table in their private builds
for the i2c devices they are using. Another way to avoid adding a
table entry is to create a platform device in the platform code. But
this support is being extended to audio codecs too. There are hundreds
of audio codecs.

The whole purpose of this code is to dynamically load the correct i2c
and audio drivers by reading the device tree instead of having static
i2s/codec devices for every possible platform combination.


-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply

* Re: [i2c] [PATCH 1/5] Implement module aliasing for i2c to translate from device tree names
From: Jon Smirl @ 2008-01-12 16:26 UTC (permalink / raw)
  To: Jean Delvare; +Cc: linuxppc-dev, i2c, linux-kernel
In-Reply-To: <20080112094658.197acb08@hyperion.delvare>

On 1/12/08, Jean Delvare <khali@linux-fr.org> wrote:
> Hi Jon,
>
> On Fri, 11 Jan 2008 20:20:15 +0100, Jean Delvare wrote:
> > > +{
> > > +   /* only powerpc drivers implement the id_table,
> > > +    * it is empty on other platforms */
> > > +   if (id) {
> > > +           while (id->name[0]) {
> > > +                   if (strcmp(client->driver_name, id->name) == 0)
> >
> > This doesn't look right to me. You should be comparing client->name,
> > not client->driver_name, with id->name. Where id_table is implemented,
> > client->driver_name might not even be set. I see that the next patch in
> > the series makes use of client->driver_name as well, so your code
> > "works"... but this ain't correct still.
>
> Err, scratch this (and related comments), I just realized what you were
> trying to do. That's different from what I had in mind and so I read
> your code wrong. I'll read it (and test it) again not making this
> incorrect assumption and my comments will likely be different after
> that. Well, I still think that it needs to be changed a bit, but
> probably not in the direction I suggested at first (which, I realize
> now, has its own share of issues - so it's not fair to me to point you
> there.)

The common scheme used elsewhere in the kernel for handling more than
one device in a single driver is aliases. The i2c code's existing
driver_name/type combination is a different way of implementing the
same feature. But there is no real need for driver_name/type on any
platform if aliases are used. Back in version 10 or 11 I had code in
there which replaced the two fields with aliases on all platforms but
too many people objected so I removed it..

IMHO, driver_name/type should be removed in new style drivers and
replaced with aliases on all platforms since aliases are the standard
kernel mechanism.



>
> Sorry for the trouble. I'll post updated comments later today, but I'm
> also pretty busy with other issues, some of which need to be solved
> before 2.6.24 is released so I can't really delay them.
>
> --
> Jean Delvare
>


-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply

* Re: [PATCH 1/5] Warp Base Platform
From: Sean MacLennan @ 2008-01-12 20:30 UTC (permalink / raw)
  To: Stefan Roese; +Cc: linuxppc-dev, Stephen Rothwell
In-Reply-To: <200801120723.29393.sr@denx.de>

Stefan Roese wrote:
>
> You are accessing the 440EP GPIO controller here right? Then you really should 
> use big endian access routines. From you code I assume that you have 
> connected the LED signals to GPIO00 and GPIO01. I suggest to use code that 
> looks like this:
>
> #define LED_GREEN	(0x80000000 >> 0)
> #define LED_RED		(0x80000000 >> 1)
>
> 	leds = in_be32(gpio_base);
>
> 	switch (green) {
> 	case 0: leds &= ~LED_GREEN; break;
> 	case 1: leds |=  LED_GREEN; break;
> 	}
> 	switch (red) {
> 	case 0: leds &= ~LED_RED; break;
> 	case 1: leds |=  LED_RED; break;
> 	}
>
>        outbe32(leds, gpio_base);
>
> And when you change the dts to describe both GPIO controllers you should map 
> the 2nd one and remove the 0x100 offset above as I have done above.
>
> Best regards,
> Stefan
>   
Signed-off-by: Sean MacLennan <smaclennan@pikatech.com>
---
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 66a3d8c..b3e4c35 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -469,7 +469,7 @@ config MCA
 config PCI
 	bool "PCI support" if 40x || CPM2 || PPC_83xx || PPC_85xx || PPC_86xx \
 		|| PPC_MPC52xx || (EMBEDDED && (PPC_PSERIES || PPC_ISERIES)) \
-		|| PPC_PS3
+		|| PPC_PS3 || 44x
 	default y if !40x && !CPM2 && !8xx && !PPC_83xx \
 		&& !PPC_85xx && !PPC_86xx
 	default PCI_PERMEDIA if !4xx && !CPM2 && !8xx
diff --git a/arch/powerpc/platforms/44x/Kconfig b/arch/powerpc/platforms/44x/Kconfig
index d248013..a95409e 100644
--- a/arch/powerpc/platforms/44x/Kconfig
+++ b/arch/powerpc/platforms/44x/Kconfig
@@ -53,6 +53,19 @@ config RAINIER
 	help
 	  This option enables support for the AMCC PPC440GRX evaluation board.
 
+config WARP
+	bool "PIKA Warp"
+	depends on 44x
+	default n
+	select 440EP
+	help
+	  This option enables support for the PIKA Warp(tm) Appliance. The Warp
+          is a small computer replacement with up to 9 ports of FXO/FXS plus VOIP
+	  stations and trunks.
+
+	  See http://www.pikatechnologies.com/ and follow the "PIKA for Computer
+	  Telephony Developers" link for more information.
+
 #config LUAN
 #	bool "Luan"
 #	depends on 44x
@@ -75,6 +88,7 @@ config 440EP
 	select PPC_FPU
 	select IBM440EP_ERR42
 	select IBM_NEW_EMAC_ZMII
+	select USB_ARCH_HAS_OHCI
 
 config 440EPX
 	bool
diff --git a/arch/powerpc/platforms/44x/Makefile b/arch/powerpc/platforms/44x/Makefile
index a2a0dc1..7aaee9d 100644
--- a/arch/powerpc/platforms/44x/Makefile
+++ b/arch/powerpc/platforms/44x/Makefile
@@ -5,3 +5,5 @@ obj-$(CONFIG_BAMBOO)	+= bamboo.o
 obj-$(CONFIG_SEQUOIA)	+= sequoia.o
 obj-$(CONFIG_KATMAI)	+= katmai.o
 obj-$(CONFIG_RAINIER)	+= rainier.o
+obj-$(CONFIG_WARP)	+= warp.o
+#obj-$(CONFIG_WARP)	+= warp-nand.o
diff --git a/arch/powerpc/platforms/44x/warp-nand.c b/arch/powerpc/platforms/44x/warp-nand.c
new file mode 100644
index 0000000..0053958
--- /dev/null
+++ b/arch/powerpc/platforms/44x/warp-nand.c
@@ -0,0 +1,92 @@
+/*
+ * PIKA Warp(tm) NAND flash specific routines
+ *
+ * Copyright (c) 2008 PIKA Technologies
+ *   Sean MacLennan <smaclennan@pikatech.com>
+ */
+
+#include <linux/platform_device.h>
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/map.h>
+#include <linux/mtd/partitions.h>
+#include <linux/mtd/nand.h>
+#include <linux/mtd/ndfc.h>
+
+
+#define CS_NAND_0	1	/* use chip select 1 for NAND device 0 */
+
+#define WARP_NAND_FLASH_REG_ADDR	0xD0000000UL
+#define WARP_NAND_FLASH_REG_SIZE	0x2000
+
+static struct resource warp_ndfc = {
+	.start = WARP_NAND_FLASH_REG_ADDR,
+	.end   = WARP_NAND_FLASH_REG_ADDR + WARP_NAND_FLASH_REG_SIZE,
+	.flags = IORESOURCE_MEM,
+};
+
+static struct mtd_partition nand_parts[] = {
+	{
+		.name = "nand",
+		.offset = 0,
+		.size = MTDPART_SIZ_FULL,
+	}
+};
+
+struct ndfc_controller_settings warp_ndfc_settings = {
+	.ccr_settings = (NDFC_CCR_BS(CS_NAND_0) | NDFC_CCR_ARAC1),
+	.ndfc_erpn = 0,
+};
+
+static struct ndfc_chip_settings warp_chip0_settings = {
+	.bank_settings = 0x80002222,
+};
+
+struct platform_nand_ctrl warp_nand_ctrl = {
+	.priv = &warp_ndfc_settings,
+};
+
+static struct platform_device warp_ndfc_device = {
+	.name = "ndfc-nand",
+	.id = 0,
+	.dev = {
+		.platform_data = &warp_nand_ctrl,
+	},
+	.num_resources = 1,
+	.resource = &warp_ndfc,
+};
+
+static struct nand_ecclayout nand_oob_16 = {
+	.eccbytes = 3,
+	.eccpos = { 0, 1, 2, 3, 6, 7 },
+	.oobfree = { {.offset = 8, .length = 16} }
+};
+
+static struct platform_nand_chip warp_nand_chip0 = {
+	.nr_chips = 1,
+	.chip_offset = CS_NAND_0,
+	.nr_partitions = ARRAY_SIZE(nand_parts),
+	.partitions = nand_parts,
+	.chip_delay = 50,
+	.ecclayout = &nand_oob_16,
+	.priv = &warp_chip0_settings,
+};
+
+static struct platform_device warp_nand_device = {
+	.name = "ndfc-chip",
+	.id = 0,
+	.num_resources = 1,
+	.resource = &warp_ndfc,
+	.dev = {
+		.platform_data = &warp_nand_chip0,
+		.parent = &warp_ndfc_device.dev,
+	}
+};
+
+static int warp_setup_nand_flash(void)
+{
+	platform_device_register(&warp_ndfc_device);
+	platform_device_register(&warp_nand_device);
+
+	return 0;
+}
+device_initcall(warp_setup_nand_flash);
diff --git a/arch/powerpc/platforms/44x/warp.c b/arch/powerpc/platforms/44x/warp.c
new file mode 100644
index 0000000..b177de1
--- /dev/null
+++ b/arch/powerpc/platforms/44x/warp.c
@@ -0,0 +1,186 @@
+/*
+ * PIKA Warp(tm) board specific routines
+ *
+ * Copyright (c) 2008 PIKA Technologies
+ *   Sean MacLennan <smaclennan@pikatech.com>
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ */
+#include <linux/init.h>
+#include <linux/of_platform.h>
+#include <linux/kthread.h>
+
+#include <asm/machdep.h>
+#include <asm/prom.h>
+#include <asm/udbg.h>
+#include <asm/time.h>
+#include <asm/uic.h>
+
+#include "44x.h"
+
+
+static __initdata struct of_device_id warp_of_bus[] = {
+	{ .compatible = "ibm,plb4", },
+	{ .compatible = "ibm,opb", },
+	{ .compatible = "ibm,ebc", },
+	{},
+};
+
+static int __init warp_device_probe(void)
+{
+	of_platform_bus_probe(NULL, warp_of_bus, NULL);
+	return 0;
+}
+machine_device_initcall(warp, warp_device_probe);
+
+static int __init warp_probe(void)
+{
+	unsigned long root = of_get_flat_dt_root();
+
+	if (!of_flat_dt_is_compatible(root, "pika,warp"))
+		return 0;
+
+	return 1;
+}
+
+define_machine(warp) {
+	.name		= "Warp",
+	.probe 		= warp_probe,
+	.progress 	= udbg_progress,
+	.init_IRQ 	= uic_init_tree,
+	.get_irq 	= uic_get_irq,
+	.restart	= ppc44x_reset_system,
+	.calibrate_decr = generic_calibrate_decr,
+};
+
+
+#define LED_GREEN (0x80000000 >> 0)
+#define LED_RED   (0x80000000 >> 1)
+
+
+/* This is for the power LEDs 1 = on, 0 = off, -1 = leave alone */
+void warp_set_power_leds(int green, int red)
+{
+	static void __iomem *gpio_base = NULL;
+	unsigned leds;
+
+	if (gpio_base == NULL) {
+		struct device_node *np, *from;
+		int i;
+
+		/* Power LEDS are on the second GPIO controller */
+		for (from = NULL, i = 0; i < 2; ++i, from = np) {
+			np = of_find_compatible_node(from, NULL, "ibm,gpio-440EP");
+			if (np == NULL) {
+				printk(KERN_ERR __FILE__ ": Unable to find gpio\n");
+				return;
+			}
+		}
+
+		gpio_base = of_iomap(np, 0);
+		if (gpio_base == NULL) {
+			printk(KERN_ERR __FILE__ ": Unable to map gpio");
+			return;
+		}
+	}
+
+
+	leds = in_be32(gpio_base);
+
+	switch (green) {
+	case 0: leds &= ~LED_GREEN; break;
+	case 1: leds |=  LED_GREEN; break;
+	}
+	switch (red) {
+	case 0: leds &= ~LED_RED; break;
+	case 1: leds |=  LED_RED; break;
+	}
+
+	out_be32(gpio_base, leds);
+}
+EXPORT_SYMBOL(warp_set_power_leds);
+
+
+#ifdef CONFIG_SENSORS_AD7414y
+static int pika_dtm_thread(void __iomem *fpga)
+{
+	extern int ad7414_get_temp(int index);
+
+	while (!kthread_should_stop()) {
+		int temp = ad7414_get_temp(0);
+
+		out_be32(fpga, temp);
+
+		set_current_state(TASK_INTERRUPTIBLE);
+		schedule_timeout(HZ);
+	}
+
+	return 0;
+}
+
+static int pika_dtm_start(void __iomem *fpga)
+{
+	struct task_struct *dtm_thread;
+
+	dtm_thread = kthread_run(pika_dtm_thread, fpga + 0x20, "pika-dtm");
+	if (IS_ERR(dtm_thread)) {
+		printk(KERN_ERR __FILE__ ": Unable to start PIKA DTM thread\n");
+		return PTR_ERR(dtm_thread);
+	}
+
+	return 0;
+}
+#else
+static int pika_dtm_start(void __iomem *fpga)
+{
+	printk(KERN_WARNING "PIKA DTM disabled\n");
+	return 0;
+}
+#endif
+
+
+static int __devinit warp_fpga_init(void)
+{
+	struct device_node *np;
+	struct resource res;
+	void __iomem *fpga;
+	int irq;
+
+	np = of_find_compatible_node(NULL, NULL, "pika,fpga");
+	if (np == NULL) {
+		printk(KERN_ERR __FILE__ ": Unable to find fpga\n");
+		return -ENOENT;
+	}
+
+	irq = irq_of_parse_and_map(np, 0);
+	if (irq  == NO_IRQ) {
+		printk(KERN_ERR __FILE__ ": irq_of_parse_and_map failed\n");
+		return -EBUSY;
+	}
+
+	/* We do not call of_iomap here since it would map in the entire
+	 * fpga space, which is over 8k.
+	 */
+	if (of_address_to_resource(np, 0, &res)) {
+		printk(KERN_ERR __FILE__ ": Unable to get FPGA address\n");
+		return -ENOENT;
+	}
+	fpga = ioremap(res.start, 0x24);
+	if (fpga == NULL) {
+		printk(KERN_ERR __FILE__ ": Unable to map FPGA\n");
+		return -ENOENT;
+	}
+
+	if (pika_dtm_start(fpga)) {
+		iounmap(fpga);
+		return -ENOENT;
+	}
+
+	/* SAM TODO: Start the watchdog here */
+
+	return 0;
+}
+device_initcall(warp_fpga_init);

^ permalink raw reply related

* Re: [PATCH 2/5] Warp Base Platform - dts
From: Sean MacLennan @ 2008-01-12 20:32 UTC (permalink / raw)
  To: Stefan Roese; +Cc: linuxppc-dev, Sean MacLennan
In-Reply-To: <200801120713.54856.sr@denx.de>

Stefan Roese wrote:
> You define here one gpio node that covers both gpio controllers of the 440EP. 
> I suggest to use two nodes here, like:
>
> 			GPIO0: gpio@ef600b00 {
> 				compatible = "ibm,gpio-440ep";
> 				reg = <ef600b00 48>;
> 			};
>
> 			GPIO1: gpio@ef600c00 {
> 				compatible = "ibm,gpio-440ep";
> 				reg = <ef600c00 48>;
> 			};
>
> Best regards,
> Stefan
>   

Signed-off-by: Sean MacLennan <smaclennan@pikatech.com>
---
diff --git a/arch/powerpc/boot/dts/warp.dts b/arch/powerpc/boot/dts/warp.dts
new file mode 100644
index 0000000..dc1499d
--- /dev/null
+++ b/arch/powerpc/boot/dts/warp.dts
@@ -0,0 +1,239 @@
+/*
+ * Device Tree Source for PIKA Warp
+ *
+ * Copyright (c) 2008 PIKA Technologies
+ *   Sean MacLennan <smaclennan@pikatech.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2.  This program is licensed "as is" without
+ * any warranty of any kind, whether express or implied.
+ */
+
+/ {
+	#address-cells = <2>;
+	#size-cells = <1>;
+	model = "pika,warp";
+	compatible = "pika,warp";
+	dcr-parent = <&/cpus/cpu@0>;
+
+	aliases {
+		ethernet0 = &EMAC0;
+		serial0 = &UART0;
+	};
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		cpu@0 {
+			device_type = "cpu";
+			model = "PowerPC,440EP";
+			reg = <0>;
+			clock-frequency = <0>; /* Filled in by zImage */
+			timebase-frequency = <0>; /* Filled in by zImage */
+			i-cache-line-size = <20>;
+			d-cache-line-size = <20>;
+			i-cache-size = <8000>;
+			d-cache-size = <8000>;
+			dcr-controller;
+			dcr-access-method = "native";
+		};
+	};
+
+	memory {
+		device_type = "memory";
+		reg = <0 0 0>; /* Filled in by zImage */
+	};
+
+	UIC0: interrupt-controller0 {
+		compatible = "ibm,uic-440ep","ibm,uic";
+		interrupt-controller;
+		cell-index = <0>;
+		dcr-reg = <0c0 009>;
+		#address-cells = <0>;
+		#size-cells = <0>;
+		#interrupt-cells = <2>;
+	};
+
+	UIC1: interrupt-controller1 {
+		compatible = "ibm,uic-440ep","ibm,uic";
+		interrupt-controller;
+		cell-index = <1>;
+		dcr-reg = <0d0 009>;
+		#address-cells = <0>;
+		#size-cells = <0>;
+		#interrupt-cells = <2>;
+		interrupts = <1e 4 1f 4>; /* cascade */
+		interrupt-parent = <&UIC0>;
+	};
+
+	SDR0: sdr {
+		compatible = "ibm,sdr-440ep";
+		dcr-reg = <00e 002>;
+	};
+
+	CPR0: cpr {
+		compatible = "ibm,cpr-440ep";
+		dcr-reg = <00c 002>;
+	};
+
+	plb {
+		compatible = "ibm,plb-440ep", "ibm,plb-440gp", "ibm,plb4";
+		#address-cells = <2>;
+		#size-cells = <1>;
+		ranges;
+		clock-frequency = <0>; /* Filled in by zImage */
+
+		SDRAM0: sdram {
+			compatible = "ibm,sdram-440ep", "ibm,sdram-405gp";
+			dcr-reg = <010 2>;
+		};
+
+		DMA0: dma {
+			compatible = "ibm,dma-440ep", "ibm,dma-440gp";
+			dcr-reg = <100 027>;
+		};
+
+		MAL0: mcmal {
+			compatible = "ibm,mcmal-440ep", "ibm,mcmal-440gp", "ibm,mcmal";
+			dcr-reg = <180 62>;
+			num-tx-chans = <4>;
+			num-rx-chans = <2>;
+			interrupt-parent = <&MAL0>;
+			interrupts = <0 1 2 3 4>;
+			#interrupt-cells = <1>;
+			#address-cells = <0>;
+			#size-cells = <0>;
+			interrupt-map = </*TXEOB*/ 0 &UIC0 a 4
+					/*RXEOB*/ 1 &UIC0 b 4
+					/*SERR*/  2 &UIC1 0 4
+					/*TXDE*/  3 &UIC1 1 4
+					/*RXDE*/  4 &UIC1 2 4>;
+		};
+
+		POB0: opb {
+		  	compatible = "ibm,opb-440ep", "ibm,opb-440gp", "ibm,opb";
+			#address-cells = <1>;
+			#size-cells = <1>;
+		  	ranges = <00000000 0 00000000 80000000
+			          80000000 0 80000000 80000000>;
+		  	interrupt-parent = <&UIC1>;
+		  	interrupts = <7 4>;
+		  	clock-frequency = <0>; /* Filled in by zImage */
+
+			EBC0: ebc {
+				compatible = "ibm,ebc-440ep", "ibm,ebc-440gp", "ibm,ebc";
+				dcr-reg = <012 2>;
+				#address-cells = <2>;
+				#size-cells = <1>;
+				clock-frequency = <0>; /* Filled in by zImage */
+				interrupts = <5 1>;
+				interrupt-parent = <&UIC1>;
+
+				fpga@2,0 {
+					compatible = "pika,fpga";
+			   		reg = <2 0 2200>;
+					interrupts = <18 8>;
+					interrupt-parent = <&UIC0>;
+				};
+
+				nor_flash@0,0 {
+					compatible = "amd,s29gl512n", "cfi-flash";
+					bank-width = <2>;
+					reg = <0 0 4000000>;
+					#address-cells = <1>;
+					#size-cells = <1>;
+					partition@0 {
+						label = "kernel";
+						reg = <0 180000>;
+					};
+					partition@180000 {
+						label = "root";
+						reg = <180000 3480000>;
+					};
+					partition@3600000 {
+						label = "user";
+						reg = <3600000 900000>;
+					};
+					partition@3f00000 {
+						label = "fpga";
+						reg = <3f00000 40000>;
+					};
+					partition@3f40000 {
+						label = "env";
+						reg = <3f40000 40000>;
+					};
+					partition@3f80000 {
+						label = "u-boot";
+						reg = <3f80000 80000>;
+					};
+				};
+			};
+
+			UART0: serial@ef600300 {
+		   		device_type = "serial";
+		   		compatible = "ns16550";
+		   		reg = <ef600300 8>;
+		   		virtual-reg = <ef600300>;
+		   		clock-frequency = <0>; /* Filled in by zImage */
+		   		current-speed = <1c200>;
+		   		interrupt-parent = <&UIC0>;
+		   		interrupts = <0 4>;
+	   		};
+
+			IIC0: i2c@ef600700 {
+				compatible = "ibm,iic-440ep", "ibm,iic-440gp", "ibm,iic";
+				reg = <ef600700 14>;
+				interrupt-parent = <&UIC0>;
+				interrupts = <2 4>;
+			};
+
+			GPIO0: gpio@ef600b00 {
+				compatible = "ibm,gpio-440ep";
+				reg = <ef600b00 48>;
+			};
+
+			GPIO1: gpio@ef600c00 {
+				compatible = "ibm,gpio-440ep";
+				reg = <ef600c00 48>;
+			};
+
+			ZMII0: emac-zmii@ef600d00 {
+				compatible = "ibm,zmii-440ep", "ibm,zmii-440gp", "ibm,zmii";
+				reg = <ef600d00 c>;
+			};
+
+			EMAC0: ethernet@ef600e00 {
+				linux,network-index = <0>;
+				device_type = "network";
+				compatible = "ibm,emac-440ep", "ibm,emac-440gp", "ibm,emac";
+				interrupt-parent = <&UIC1>;
+				interrupts = <1c 4 1d 4>;
+				reg = <ef600e00 70>;
+				local-mac-address = [000000000000];
+				mal-device = <&MAL0>;
+				mal-tx-channel = <0 1>;
+				mal-rx-channel = <0>;
+				cell-index = <0>;
+				max-frame-size = <5dc>;
+				rx-fifo-size = <1000>;
+				tx-fifo-size = <800>;
+				phy-mode = "rmii";
+				phy-map = <00000000>;
+				zmii-device = <&ZMII0>;
+				zmii-channel = <0>;
+			};
+
+			usb@ef601000 {
+				compatible = "ohci-be";
+				reg = <ef601000 80>;
+				interrupts = <8 1 9 1>;
+				interrupt-parent = < &UIC1 >;
+			};
+		};
+	};
+
+	chosen {
+		linux,stdout-path = "/plb/opb/serial@ef600300";
+	};
+};

^ permalink raw reply related

* Re: [PATCH 1/5] Warp Base Platform
From: Sean MacLennan @ 2008-01-12 20:42 UTC (permalink / raw)
  Cc: linuxppc-dev, Stefan Roese, Stephen Rothwell
In-Reply-To: <4789235E.1030901@pikatech.com>

Please ignore the last patch. I left in a debug to check 
that the non-ad7414 case works.

Cheers,
   Sean

Signed-off-by: Sean MacLennan <smaclennan@pikatech.com>
---
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 66a3d8c..b3e4c35 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -469,7 +469,7 @@ config MCA
 config PCI
 	bool "PCI support" if 40x || CPM2 || PPC_83xx || PPC_85xx || PPC_86xx \
 		|| PPC_MPC52xx || (EMBEDDED && (PPC_PSERIES || PPC_ISERIES)) \
-		|| PPC_PS3
+		|| PPC_PS3 || 44x
 	default y if !40x && !CPM2 && !8xx && !PPC_83xx \
 		&& !PPC_85xx && !PPC_86xx
 	default PCI_PERMEDIA if !4xx && !CPM2 && !8xx
diff --git a/arch/powerpc/platforms/44x/Kconfig b/arch/powerpc/platforms/44x/Kconfig
index d248013..a95409e 100644
--- a/arch/powerpc/platforms/44x/Kconfig
+++ b/arch/powerpc/platforms/44x/Kconfig
@@ -53,6 +53,19 @@ config RAINIER
 	help
 	  This option enables support for the AMCC PPC440GRX evaluation board.
 
+config WARP
+	bool "PIKA Warp"
+	depends on 44x
+	default n
+	select 440EP
+	help
+	  This option enables support for the PIKA Warp(tm) Appliance. The Warp
+          is a small computer replacement with up to 9 ports of FXO/FXS plus VOIP
+	  stations and trunks.
+
+	  See http://www.pikatechnologies.com/ and follow the "PIKA for Computer
+	  Telephony Developers" link for more information.
+
 #config LUAN
 #	bool "Luan"
 #	depends on 44x
@@ -75,6 +88,7 @@ config 440EP
 	select PPC_FPU
 	select IBM440EP_ERR42
 	select IBM_NEW_EMAC_ZMII
+	select USB_ARCH_HAS_OHCI
 
 config 440EPX
 	bool
diff --git a/arch/powerpc/platforms/44x/Makefile b/arch/powerpc/platforms/44x/Makefile
index a2a0dc1..7aaee9d 100644
--- a/arch/powerpc/platforms/44x/Makefile
+++ b/arch/powerpc/platforms/44x/Makefile
@@ -5,3 +5,5 @@ obj-$(CONFIG_BAMBOO)	+= bamboo.o
 obj-$(CONFIG_SEQUOIA)	+= sequoia.o
 obj-$(CONFIG_KATMAI)	+= katmai.o
 obj-$(CONFIG_RAINIER)	+= rainier.o
+obj-$(CONFIG_WARP)	+= warp.o
+#obj-$(CONFIG_WARP)	+= warp-nand.o
diff --git a/arch/powerpc/platforms/44x/warp-nand.c b/arch/powerpc/platforms/44x/warp-nand.c
new file mode 100644
index 0000000..0053958
--- /dev/null
+++ b/arch/powerpc/platforms/44x/warp-nand.c
@@ -0,0 +1,92 @@
+/*
+ * PIKA Warp(tm) NAND flash specific routines
+ *
+ * Copyright (c) 2008 PIKA Technologies
+ *   Sean MacLennan <smaclennan@pikatech.com>
+ */
+
+#include <linux/platform_device.h>
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/map.h>
+#include <linux/mtd/partitions.h>
+#include <linux/mtd/nand.h>
+#include <linux/mtd/ndfc.h>
+
+
+#define CS_NAND_0	1	/* use chip select 1 for NAND device 0 */
+
+#define WARP_NAND_FLASH_REG_ADDR	0xD0000000UL
+#define WARP_NAND_FLASH_REG_SIZE	0x2000
+
+static struct resource warp_ndfc = {
+	.start = WARP_NAND_FLASH_REG_ADDR,
+	.end   = WARP_NAND_FLASH_REG_ADDR + WARP_NAND_FLASH_REG_SIZE,
+	.flags = IORESOURCE_MEM,
+};
+
+static struct mtd_partition nand_parts[] = {
+	{
+		.name = "nand",
+		.offset = 0,
+		.size = MTDPART_SIZ_FULL,
+	}
+};
+
+struct ndfc_controller_settings warp_ndfc_settings = {
+	.ccr_settings = (NDFC_CCR_BS(CS_NAND_0) | NDFC_CCR_ARAC1),
+	.ndfc_erpn = 0,
+};
+
+static struct ndfc_chip_settings warp_chip0_settings = {
+	.bank_settings = 0x80002222,
+};
+
+struct platform_nand_ctrl warp_nand_ctrl = {
+	.priv = &warp_ndfc_settings,
+};
+
+static struct platform_device warp_ndfc_device = {
+	.name = "ndfc-nand",
+	.id = 0,
+	.dev = {
+		.platform_data = &warp_nand_ctrl,
+	},
+	.num_resources = 1,
+	.resource = &warp_ndfc,
+};
+
+static struct nand_ecclayout nand_oob_16 = {
+	.eccbytes = 3,
+	.eccpos = { 0, 1, 2, 3, 6, 7 },
+	.oobfree = { {.offset = 8, .length = 16} }
+};
+
+static struct platform_nand_chip warp_nand_chip0 = {
+	.nr_chips = 1,
+	.chip_offset = CS_NAND_0,
+	.nr_partitions = ARRAY_SIZE(nand_parts),
+	.partitions = nand_parts,
+	.chip_delay = 50,
+	.ecclayout = &nand_oob_16,
+	.priv = &warp_chip0_settings,
+};
+
+static struct platform_device warp_nand_device = {
+	.name = "ndfc-chip",
+	.id = 0,
+	.num_resources = 1,
+	.resource = &warp_ndfc,
+	.dev = {
+		.platform_data = &warp_nand_chip0,
+		.parent = &warp_ndfc_device.dev,
+	}
+};
+
+static int warp_setup_nand_flash(void)
+{
+	platform_device_register(&warp_ndfc_device);
+	platform_device_register(&warp_nand_device);
+
+	return 0;
+}
+device_initcall(warp_setup_nand_flash);
diff --git a/arch/powerpc/platforms/44x/warp.c b/arch/powerpc/platforms/44x/warp.c
new file mode 100644
index 0000000..97d5c33
--- /dev/null
+++ b/arch/powerpc/platforms/44x/warp.c
@@ -0,0 +1,186 @@
+/*
+ * PIKA Warp(tm) board specific routines
+ *
+ * Copyright (c) 2008 PIKA Technologies
+ *   Sean MacLennan <smaclennan@pikatech.com>
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ */
+#include <linux/init.h>
+#include <linux/of_platform.h>
+#include <linux/kthread.h>
+
+#include <asm/machdep.h>
+#include <asm/prom.h>
+#include <asm/udbg.h>
+#include <asm/time.h>
+#include <asm/uic.h>
+
+#include "44x.h"
+
+
+static __initdata struct of_device_id warp_of_bus[] = {
+	{ .compatible = "ibm,plb4", },
+	{ .compatible = "ibm,opb", },
+	{ .compatible = "ibm,ebc", },
+	{},
+};
+
+static int __init warp_device_probe(void)
+{
+	of_platform_bus_probe(NULL, warp_of_bus, NULL);
+	return 0;
+}
+machine_device_initcall(warp, warp_device_probe);
+
+static int __init warp_probe(void)
+{
+	unsigned long root = of_get_flat_dt_root();
+
+	if (!of_flat_dt_is_compatible(root, "pika,warp"))
+		return 0;
+
+	return 1;
+}
+
+define_machine(warp) {
+	.name		= "Warp",
+	.probe 		= warp_probe,
+	.progress 	= udbg_progress,
+	.init_IRQ 	= uic_init_tree,
+	.get_irq 	= uic_get_irq,
+	.restart	= ppc44x_reset_system,
+	.calibrate_decr = generic_calibrate_decr,
+};
+
+
+#define LED_GREEN (0x80000000 >> 0)
+#define LED_RED   (0x80000000 >> 1)
+
+
+/* This is for the power LEDs 1 = on, 0 = off, -1 = leave alone */
+void warp_set_power_leds(int green, int red)
+{
+	static void __iomem *gpio_base = NULL;
+	unsigned leds;
+
+	if (gpio_base == NULL) {
+		struct device_node *np, *from;
+		int i;
+
+		/* Power LEDS are on the second GPIO controller */
+		for (from = NULL, i = 0; i < 2; ++i, from = np) {
+			np = of_find_compatible_node(from, NULL, "ibm,gpio-440EP");
+			if (np == NULL) {
+				printk(KERN_ERR __FILE__ ": Unable to find gpio\n");
+				return;
+			}
+		}
+
+		gpio_base = of_iomap(np, 0);
+		if (gpio_base == NULL) {
+			printk(KERN_ERR __FILE__ ": Unable to map gpio");
+			return;
+		}
+	}
+
+
+	leds = in_be32(gpio_base);
+
+	switch (green) {
+	case 0: leds &= ~LED_GREEN; break;
+	case 1: leds |=  LED_GREEN; break;
+	}
+	switch (red) {
+	case 0: leds &= ~LED_RED; break;
+	case 1: leds |=  LED_RED; break;
+	}
+
+	out_be32(gpio_base, leds);
+}
+EXPORT_SYMBOL(warp_set_power_leds);
+
+
+#ifdef CONFIG_SENSORS_AD7414
+static int pika_dtm_thread(void __iomem *fpga)
+{
+	extern int ad7414_get_temp(int index);
+
+	while (!kthread_should_stop()) {
+		int temp = ad7414_get_temp(0);
+
+		out_be32(fpga, temp);
+
+		set_current_state(TASK_INTERRUPTIBLE);
+		schedule_timeout(HZ);
+	}
+
+	return 0;
+}
+
+static int pika_dtm_start(void __iomem *fpga)
+{
+	struct task_struct *dtm_thread;
+
+	dtm_thread = kthread_run(pika_dtm_thread, fpga + 0x20, "pika-dtm");
+	if (IS_ERR(dtm_thread)) {
+		printk(KERN_ERR __FILE__ ": Unable to start PIKA DTM thread\n");
+		return PTR_ERR(dtm_thread);
+	}
+
+	return 0;
+}
+#else
+static int pika_dtm_start(void __iomem *fpga)
+{
+	printk(KERN_WARNING "PIKA DTM disabled\n");
+	return 0;
+}
+#endif
+
+
+static int __devinit warp_fpga_init(void)
+{
+	struct device_node *np;
+	struct resource res;
+	void __iomem *fpga;
+	int irq;
+
+	np = of_find_compatible_node(NULL, NULL, "pika,fpga");
+	if (np == NULL) {
+		printk(KERN_ERR __FILE__ ": Unable to find fpga\n");
+		return -ENOENT;
+	}
+
+	irq = irq_of_parse_and_map(np, 0);
+	if (irq  == NO_IRQ) {
+		printk(KERN_ERR __FILE__ ": irq_of_parse_and_map failed\n");
+		return -EBUSY;
+	}
+
+	/* We do not call of_iomap here since it would map in the entire
+	 * fpga space, which is over 8k.
+	 */
+	if (of_address_to_resource(np, 0, &res)) {
+		printk(KERN_ERR __FILE__ ": Unable to get FPGA address\n");
+		return -ENOENT;
+	}
+	fpga = ioremap(res.start, 0x24);
+	if (fpga == NULL) {
+		printk(KERN_ERR __FILE__ ": Unable to map FPGA\n");
+		return -ENOENT;
+	}
+
+	if (pika_dtm_start(fpga)) {
+		iounmap(fpga);
+		return -ENOENT;
+	}
+
+	/* SAM TODO: Start the watchdog here */
+
+	return 0;
+}
+device_initcall(warp_fpga_init);

^ permalink raw reply related

* Re: [PATCH for 2.6.24][NET] fs_enet: check for phydev existence in the ethtool handlers
From: Jeff Garzik @ 2008-01-12 22:45 UTC (permalink / raw)
  To: avorontsov; +Cc: netdev, linuxppc-dev
In-Reply-To: <20080108190555.GA23302@localhost.localdomain>

Anton Vorontsov wrote:
> Otherwise oops will happen if ethernet device has not been opened:
> 
> Unable to handle kernel paging request for data at address 0x0000014c
> Faulting instruction address: 0xc016f7f0
> Oops: Kernel access of bad area, sig: 11 [#1]
> MPC85xx
> NIP: c016f7f0 LR: c01722a0 CTR: 00000000
> REGS: c79ddc70 TRAP: 0300   Not tainted  (2.6.24-rc3-g820a386b)
> MSR: 00029000 <EE,ME>  CR: 20004428  XER: 20000000
> DEAR: 0000014c, ESR: 00000000
> TASK = c789f5e0[999] 'snmpd' THREAD: c79dc000
> GPR00: c01aceb8 c79ddd20 c789f5e0 00000000 c79ddd3c 00000000 c79ddd64 00000000
> GPR08: 00000000 c7845b60 c79dde3c c01ace80 20004422 200249fc 000002a0 100da728
> GPR16: 100c0000 00000000 00000000 00000000 20022078 00000009 200220e0 bfc85558
> GPR24: c79ddd3c 00000000 00000000 c02e0e70 c022fc64 ffffffff c7845800 bfc85498
> NIP [c016f7f0] phy_ethtool_gset+0x0/0x4c
> LR [c01722a0] fs_get_settings+0x18/0x28
> Call Trace:
> [c79ddd20] [c79dde38] 0xc79dde38 (unreliable)
> [c79ddd30] [c01aceb8] dev_ethtool+0x294/0x11ec
> [c79dde30] [c01aaa44] dev_ioctl+0x454/0x6a8
> [c79ddeb0] [c019b9d4] sock_ioctl+0x84/0x230
> [c79dded0] [c007ded8] do_ioctl+0x34/0x8c
> [c79ddee0] [c007dfbc] vfs_ioctl+0x8c/0x41c
> [c79ddf10] [c007e38c] sys_ioctl+0x40/0x74
> [c79ddf40] [c000d4c0] ret_from_syscall+0x0/0x3c
> Instruction dump:
> 81630000 800b0030 2f800000 419e0010 7c0803a6 4e800021 7c691b78 80010014
> 7d234b78 38210010 7c0803a6 4e800020 <8003014c> 7c6b1b78 38600000 90040004
> 
> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> Acked-by: Vitaly Bordug <vitb@kernel.crashing.org>
> ---
> 
> Just resending it, it feels like it got lost during holidays.
> 
>  drivers/net/fs_enet/fs_enet-main.c |   11 +++++++++--
>  1 files changed, 9 insertions(+), 2 deletions(-)

applied

^ permalink raw reply

* Re: RFC: Only deal with apple fix if res is memory
From: Kumar Gala @ 2008-01-12 23:05 UTC (permalink / raw)
  To: benh; +Cc: linuxppc-dev
In-Reply-To: <1200089015.6896.56.camel@pasglop>


On Jan 11, 2008, at 4:03 PM, Benjamin Herrenschmidt wrote:

>
> On Fri, 2008-01-11 at 10:38 -0600, Kumar Gala wrote:
>> Ben,
>>
>> Will this impact the apple fix?  I'm getting some _IO cases hitting  
>> this.
>
> No, your fix is right, it's a bug to compare against pci_mem_offset  
> and
> not check if it's a memory resource in the first place. I would  
> suggest
> that you do the res->flags test first in the if () statement for  
> clarity
> though.

ok, will clean up the patch and resend.

- k

^ permalink raw reply

* Re: [PATCH 1/5] Warp Base Platform
From: Stephen Rothwell @ 2008-01-13  0:35 UTC (permalink / raw)
  To: Sean MacLennan; +Cc: linuxppc-dev, Stefan Roese
In-Reply-To: <47892645.5070003@pikatech.com>

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

Hi Sean,

On Sat, 12 Jan 2008 15:42:45 -0500 Sean MacLennan <smaclennan@pikatech.com> wrote:
>
> +++ b/arch/powerpc/platforms/44x/warp.c

> +static int __init warp_probe(void)
> +{
> +	unsigned long root = of_get_flat_dt_root();
> +
> +	if (!of_flat_dt_is_compatible(root, "pika,warp"))
> +		return 0;
> +
> +	return 1;

I should have noticed this earlier and it is really only a simple cleanup
but it could be done if you do another respin.

	return of_flat_dt_is_compatible(root, "pika,warp");

> +void warp_set_power_leds(int green, int red)
> +{
> +	static void __iomem *gpio_base = NULL;
> +	unsigned leds;
> +
> +	if (gpio_base == NULL) {
> +		struct device_node *np, *from;
> +		int i;
> +
> +		/* Power LEDS are on the second GPIO controller */
> +		for (from = NULL, i = 0; i < 2; ++i, from = np) {
> +			np = of_find_compatible_node(from, NULL, "ibm,gpio-440EP");
> +			if (np == NULL) {
> +				printk(KERN_ERR __FILE__ ": Unable to find gpio\n");
> +				return;
> +			}
> +		}

		struct device_node *np;
		int i = 0;

		for_each_compatible_node(np, NULL, "ibm,gpio-440EP")
			if (i++ == 1)
				break;
		if (i < 2) {
			printk(KERN_ERR __FILE__ ": Unable to find gpio\n");
			return;
		}

or

		struct device_node *np = of_find_compatible_node(NULL, NULL, "ibm,gpio-440EP");

		if (np)
			np = of_find_compatible_node(np, NULL, "ibm,gpio-440EP");
		if (!np) {
			printk(KERN_ERR __FILE__ ": Unable to find gpio\n");
			return;
		}

One small thing - all three solutions assume that the device tree
routines return the nodes in the order that they are in the dts file ...
which is OK for now, but something to consider.

> +		gpio_base = of_iomap(np, 0);

		of_node_put(np);

> +static int __devinit warp_fpga_init(void)

> +	irq = irq_of_parse_and_map(np, 0);
> +	if (irq  == NO_IRQ) {

		of_node_put(np);

> +		printk(KERN_ERR __FILE__ ": irq_of_parse_and_map failed\n");
> +		return -EBUSY;
> +	}
> +
> +	/* We do not call of_iomap here since it would map in the entire
> +	 * fpga space, which is over 8k.
> +	 */
> +	if (of_address_to_resource(np, 0, &res)) {

		of_node_put(np);

> +		printk(KERN_ERR __FILE__ ": Unable to get FPGA address\n");
> +		return -ENOENT;
> +	}

	of_node_put(np);

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: [i2c] [PATCH] update module-init-tools to support the i2c subsystem
From: Jean Delvare @ 2008-01-13 14:27 UTC (permalink / raw)
  To: Jon Smirl; +Cc: linuxppc-dev, i2c, linux-kernel
In-Reply-To: <9e4733910712171839h214e0bc3q179266c86e5987b1@mail.gmail.com>

On Mon, 17 Dec 2007 21:39:37 -0500, Jon Smirl wrote:
> Follow on to: "Series to add device tree naming to i2c"
> Teach module-init-tools about the i2c subsystem.

Can you please explain what this patch does and why it is needed?

-- 
Jean Delvare

^ permalink raw reply

* Re: [i2c] [PATCH 1/5] Implement module aliasing for i2c to translate from device tree names
From: Jean Delvare @ 2008-01-13 14:41 UTC (permalink / raw)
  To: Jon Smirl; +Cc: linuxppc-dev, i2c, linux-kernel
In-Reply-To: <9e4733910801120826haa8905dk863ed1c8e9f420c9@mail.gmail.com>

Hi Jon,

On Sat, 12 Jan 2008 11:26:34 -0500, Jon Smirl wrote:
> The common scheme used elsewhere in the kernel for handling more than
> one device in a single driver is aliases. The i2c code's existing
> driver_name/type combination is a different way of implementing the
> same feature. But there is no real need for driver_name/type on any
> platform if aliases are used. Back in version 10 or 11 I had code in
> there which replaced the two fields with aliases on all platforms but
> too many people objected so I removed it..

While I agree that aliases make i2c_client.driver_name obsolete,
i2c_client.type is still needed. Not for device/driver matching in the
kernel, granted, but for device identification from userspace. This is
a first problem your patch has: when using your aliasing mechanism, the
type string is left empty. i2c-core exports this value to user-space
via the "name" sysfs attribute, and some libraries and applications
make use of it. I know of libsensors at least, but I guess there are
more. I can't apply your patch until this problem is solved, otherwise
we would break some user-space applications.

> IMHO, driver_name/type should be removed in new style drivers and
> replaced with aliases on all platforms since aliases are the standard
> kernel mechanism.

I agree. But we can take your aliasing code now (once you have
addressed the issues I raised) and convert the users of driver_name
later; it doesn't have to be done all at once.

The second problem I have with your patch is that you make use of the
driver_name field, while I ultimately want to get rid of it. I'd rather
see you use a different field for aliases, so that the later removal of
the driver_name field and the associated mechanism is easier.

A third, related problem, is the contents of the modalias file when
using your patch. When I tested on my ADM1032 evaluation board, the
modalias contained "adm1032". This isn't a valid module alias string:
"modprobe adm1032" doesn't work. What works is "modprobe i2c:Nadm1032"
so the modalias file should contain "i2c:Nadm1032". Just take a look at
all modalias files in /sys, they all include the subsystem prefix and a
simple modprobe `cat modalias` loads the required driver. I fail to see
why the i2c subsystem would be different.

I said this is related to the second problem because right now,
i2c-core can't easily differentiate between driver names and aliases,
as both are stored in i2c_client.driver_name. Having separate fields
would make it possible (and relatively easy) to add the required prefix
before aliases but not before driver names. The only drawback is that
it will increase the size of the i2c_client structure, but I do not
care that much given that it is only temporary.

-- 
Jean Delvare

^ permalink raw reply

* Re: [PATCH 1/3] 8xx: Analogue & Micro Adder875 board support.
From: Bryan O'Donoghue @ 2008-01-13 14:26 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev
In-Reply-To: <20080111200705.GA696@loki.buserror.net>

Greetings Scott.

Using u-boot Adder_config 

U-Boot 1.3.1-g5c740711-dirty (Jan 13 2008 - 13:43:30) - using an ELDK
4.1 uclibc toolchain

I can't boot your board - port.

I've applied your code against Linus' git v2.6.26-rc7 as at today.
I have to apply 

diff --git a/arch/powerpc/boot/wrapper b/arch/powerpc/boot/wrapper
index 31147a0..8c060ca 100755
--- a/arch/powerpc/boot/wrapper
+++ b/arch/powerpc/boot/wrapper
@@ -163,7 +163,7 @@ ps3)
     ksection=.kernel:vmlinux.bin
     isection=.kernel:initrd
     ;;
-ep88xc)
+ep88xc|ep405|redboot*)
     platformo="$object/fixed-head.o $object/$platform.o"
     binary=y
     ;;

and

diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
index 18e3271..e426052 100644
--- a/arch/powerpc/boot/Makefile
+++ b/arch/powerpc/boot/Makefile
@@ -56,7 +56,7 @@ src-plat := of.c cuboot-52xx.c cuboot-83xx.c
cuboot-85xx.c holly.c \
                cuboot-ebony.c treeboot-ebony.c prpmc2800.c \
                ps3-head.S ps3-hvcall.S ps3.c treeboot-bamboo.c
cuboot-8xx.c \
                cuboot-pq2.c cuboot-sequoia.c treeboot-walnut.c
cuboot-bamboo.c \
-               fixed-head.S ep88xc.c cuboot-hpc2.c
+               fixed-head.S ep88xc.c cuboot-hpc2.c redboot-8xx.c 
 src-boot := $(src-wlib) $(src-plat) empty.c

manually since those two fail to apply to Linus' tree - presumably
because they depend on other patches - should I be using -mm or some
other tree here instead ?

I then run

source /opt/eldk/eldk_init ppc_8xx

export ARCH=powerpc

make adder87x-uboot_defconfig

make uImage

cp arch/powerpc/boot/uImage /tftpboot - which is my tftpd root
directory.

In U-Boot then.

=> tftpboot 0x400000 uImage
=> bootm 0x400000

Which doesn't boot.

Attaching a BDI2000 to the target board I see that the kernel is
panicing somewhere around line 104 of arch/powerpc/kernel/prom.c in
of_scan_flat_dt - presumably because the kernel cannot find an
OpenFirmware tree..

I admit not understanding everything I should about where the output
of .dts & dtc "go" with respect to booting Linux.... but, didn't want to
ask on the list - lest I display a "lack of knowledge" ...

Seriously though, could you offer a pointer as to how I run this code on
the Adder875 here ?

My kernel command line is

root=/dev/nfs nfsroot=192.168.1.101:/tftpboot/root_fs rw
ip=192.168.1.202:::::eth0:off console=ttyCPM,115200

I've also tried - in ignorance of how OF works I admit to add the
following options to u-boot to no avail.


/* pass open firmware flat tree */
#define CONFIG_OF_LIBFDT      1
#undef  CONFIG_OF_FLAT_TREE
#define CONFIG_OF_BOARD_SETUP 1
#define CONFIG_OF_HAS_BD_T    1
#define CONFIG_OF_HAS_UBOOT_ENV       1

#define OF_CPU                        "PowerPC,875@0"
#define OF_SOC                        "soc875@ff000000"
#define OF_TBCLK              (bd->bi_busfreq / 4)

Have I missed a step with the .dts & dtc - and is this why
of_scan_flat_dt is failing - or - should the board port simply "just
work" with a 

make dder87x-uboot_defconfig && make uImage ?


I'm sure I've done something obscenely obvious with respect to not
getting the whole OF thing right.... but if you could point to what that
is... I'd be highly appreciative.


Regards,
Bryan

^ permalink raw reply related

* Re: [i2c] [PATCH 0/5] Version 17, series to add device tree naming to i2c
From: Jean Delvare @ 2008-01-13 15:09 UTC (permalink / raw)
  To: Jon Smirl; +Cc: linuxppc-dev, i2c, linux-kernel
In-Reply-To: <9e4733910801120800g5b13c84u8720442fc8551339@mail.gmail.com>

Hi Jon,

On Sat, 12 Jan 2008 11:00:31 -0500, Jon Smirl wrote:
> On 1/12/08, Jean Delvare wrote:
> > What I meant is that the translation from Open Firmware device name to
> > Linux device name could happen in different ways. Making module aliases
> > out of the is one possibility but this is not the only one.
> >
> > I am curious why the translation could not happen "offline". As I
> > understand it, you're getting the device names from these .dts files.
> > However you're not parsing them in the kernel directly, are you? I
> > presume that you have some tool that converts these files into C code
> > that the kernel can use? This conversion tool could translate the names.
> 
> Those dts files are for embedded devices that were specifically
> developed for Linux. All of the PowerPC Macs in the world have a
> device tree in ROM that was developed by Apple following the Open
> Firmware standard. Same thing for Sun boxes, but I'm not working on
> those.

OK. So basically we have to handle two different cases here, trees that
come from the .dts files and trees that are read from ROMs, right?
Does this mean that .dts files are compiled to some binary format to
look like what is in the ROMs? Is there kernel code that parses this?
Please explain how both types are handled by the kernel. I need to
understand how this works before I can decide where the OF names ->
Linux names translation can happen.

> The kernel has an existing mechanism for handling translations like
> these, the alias scheme.

That we agree on. My concern here is that you want to replace the Linux
names of i2c devices by OF names, without realizing that the Linux
names have a use outside of the kernel. We can't just replace them like
that, it would break some user-space applications. That's the reason
why I believe that it would make more sense to translate from OF names
to Linux names early in the process, so that the kernel, and thus
user-space applications, always handle and see the Linux names,
independently of the platform. I'm asking questions in order to figure
out whether and how this could be achieved.

> Currently developers add entries to the table in their private builds
> for the i2c devices they are using. Another way to avoid adding a
> table entry is to create a platform device in the platform code. But
> this support is being extended to audio codecs too. There are hundreds
> of audio codecs.
> 
> The whole purpose of this code is to dynamically load the correct i2c
> and audio drivers by reading the device tree instead of having static
> i2s/codec devices for every possible platform combination.

I2C driver autoloading is already implemented, and works. Just not the
way you expected, but it works.

Replacing this mechanism with standard aliases is IMHO a good idea, it
makes the code cleaner and also more similar to what the rest of the
kernel does, which is always nice.

However, having a module aliasing mechanism for i2c drivers does NOT
require that OF names are used. We could implement aliasing using Linux
device names. Note that I have no problem with using OF names for
aliasing, however it should not break applications that currently know
the I2C devices by their Linux name.

-- 
Jean Delvare

^ permalink raw reply

* Re: [i2c] [PATCH] update module-init-tools to support the i2c subsystem
From: Jon Smirl @ 2008-01-13 15:14 UTC (permalink / raw)
  To: Jean Delvare; +Cc: linuxppc-dev, i2c, linux-kernel
In-Reply-To: <20080113152735.5dca46a1@hyperion.delvare>

On 1/13/08, Jean Delvare <khali@linux-fr.org> wrote:
> On Mon, 17 Dec 2007 21:39:37 -0500, Jon Smirl wrote:
> > Follow on to: "Series to add device tree naming to i2c"
> > Teach module-init-tools about the i2c subsystem.
>
> Can you please explain what this patch does and why it is needed?

It generates the entries needed for the user space module tools to
work with the aliases. For example modprobe/depmod. It is a standard
part of the kernel module system.

-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply

* Re: [i2c] [PATCH] update module-init-tools to support the i2c subsystem
From: Jean Delvare @ 2008-01-13 15:35 UTC (permalink / raw)
  To: Jon Smirl; +Cc: linuxppc-dev, i2c, linux-kernel
In-Reply-To: <9e4733910801130714m1be16826jf240171972324d16@mail.gmail.com>

On Sun, 13 Jan 2008 10:14:14 -0500, Jon Smirl wrote:
> On 1/13/08, Jean Delvare <khali@linux-fr.org> wrote:
> > On Mon, 17 Dec 2007 21:39:37 -0500, Jon Smirl wrote:
> > > Follow on to: "Series to add device tree naming to i2c"
> > > Teach module-init-tools about the i2c subsystem.
> >
> > Can you please explain what this patch does and why it is needed?
> 
> It generates the entries needed for the user space module tools to
> work with the aliases. For example modprobe/depmod. It is a standard
> part of the kernel module system.

What entries, where? What can you do after applying your patch that you
couldn't do before?

I'm asking because automatic i2c driver loading works just fine for me
without patching user-space. So I don't get why your want to change
anything.

-- 
Jean Delvare

^ permalink raw reply

* Re: [i2c] [PATCH 1/5] Implement module aliasing for i2c to translate from device tree names
From: Jon Smirl @ 2008-01-13 16:24 UTC (permalink / raw)
  To: Jean Delvare, Greg KH; +Cc: linuxppc-dev, i2c, linux-kernel
In-Reply-To: <20080113154114.4a1c5166@hyperion.delvare>

On 1/13/08, Jean Delvare <khali@linux-fr.org> wrote:
> Hi Jon,
>
> On Sat, 12 Jan 2008 11:26:34 -0500, Jon Smirl wrote:
> > The common scheme used elsewhere in the kernel for handling more than
> > one device in a single driver is aliases. The i2c code's existing
> > driver_name/type combination is a different way of implementing the
> > same feature. But there is no real need for driver_name/type on any
> > platform if aliases are used. Back in version 10 or 11 I had code in
> > there which replaced the two fields with aliases on all platforms but
> > too many people objected so I removed it..
>
> While I agree that aliases make i2c_client.driver_name obsolete,
> i2c_client.type is still needed. Not for device/driver matching in the
> kernel, granted, but for device identification from userspace. This is
> a first problem your patch has: when using your aliasing mechanism, the
> type string is left empty. i2c-core exports this value to user-space
> via the "name" sysfs attribute, and some libraries and applications
> make use of it. I know of libsensors at least, but I guess there are
> more. I can't apply your patch until this problem is solved, otherwise
> we would break some user-space applications.
>
> > IMHO, driver_name/type should be removed in new style drivers and
> > replaced with aliases on all platforms since aliases are the standard
> > kernel mechanism.
>
> I agree. But we can take your aliasing code now (once you have
> addressed the issues I raised) and convert the users of driver_name
> later; it doesn't have to be done all at once.

GregKH, adding a new dynamically loadable subsystem is not something
that happens every day, can you check to make sure all of the standard
kernels mechanisms are being used? I'm not totally sure how the
modalias naming code is supposed to be done. The subsystem core code
in these patches needs review.

Jean, could you take over the i2c core portion of the patch? That will
let you decide exactly how you want the driver_name/name fields to be
dealt with. After you get standard naming support into i2c core I'll
rework the rest of the patch to use your new code.

I don't think driver_name/name fields should be stored in an i2c
structure at all. They are redundant with the standard mechanism.

The kernel automatically exposes modalias as a sysfs attribute so the
string must be recorded further down in the driver support layers. No
need to keep a copy in the i2c structure.

Standard devices don't export a 'name' attribute. To see the driver
name for a device in sysfs look at the 'driver' link.

> The second problem I have with your patch is that you make use of the
> driver_name field, while I ultimately want to get rid of it. I'd rather
> see you use a different field for aliases, so that the later removal of
> the driver_name field and the associated mechanism is easier.
>
> A third, related problem, is the contents of the modalias file when
> using your patch. When I tested on my ADM1032 evaluation board, the
> modalias contained "adm1032". This isn't a valid module alias string:
> "modprobe adm1032" doesn't work. What works is "modprobe i2c:Nadm1032"
> so the modalias file should contain "i2c:Nadm1032". Just take a look at
> all modalias files in /sys, they all include the subsystem prefix and a
> simple modprobe `cat modalias` loads the required driver. I fail to see
> why the i2c subsystem would be different.
>
> I said this is related to the second problem because right now,
> i2c-core can't easily differentiate between driver names and aliases,
> as both are stored in i2c_client.driver_name. Having separate fields
> would make it possible (and relatively easy) to add the required prefix
> before aliases but not before driver names. The only drawback is that
> it will increase the size of the i2c_client structure, but I do not
> care that much given that it is only temporary.
>
> --
> Jean Delvare
>


-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply

* Re: [i2c] [PATCH] update module-init-tools to support the i2c subsystem
From: Jon Smirl @ 2008-01-13 16:26 UTC (permalink / raw)
  To: Jean Delvare; +Cc: linuxppc-dev, i2c, linux-kernel
In-Reply-To: <20080113163543.16e3d26b@hyperion.delvare>

On 1/13/08, Jean Delvare <khali@linux-fr.org> wrote:
> On Sun, 13 Jan 2008 10:14:14 -0500, Jon Smirl wrote:
> > On 1/13/08, Jean Delvare <khali@linux-fr.org> wrote:
> > > On Mon, 17 Dec 2007 21:39:37 -0500, Jon Smirl wrote:
> > > > Follow on to: "Series to add device tree naming to i2c"
> > > > Teach module-init-tools about the i2c subsystem.
> > >
> > > Can you please explain what this patch does and why it is needed?
> >
> > It generates the entries needed for the user space module tools to
> > work with the aliases. For example modprobe/depmod. It is a standard
> > part of the kernel module system.
>
> What entries, where? What can you do after applying your patch that you
> couldn't do before?

The drivers you are testing with don't depend on other drivers.

> I'm asking because automatic i2c driver loading works just fine for me
> without patching user-space. So I don't get why your want to change
> anything.
>
> --
> Jean Delvare
>


-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply

* Re: [PATCH 1/5] Implement module aliasing for i2c to translate from device tree names
From: Jean Delvare @ 2008-01-13 17:40 UTC (permalink / raw)
  To: Jon Smirl; +Cc: Greg KH, i2c, linux-kernel, linuxppc-dev
In-Reply-To: <9e4733910801130824i547b65a9n64e415f9626d6ab5@mail.gmail.com>

On Sun, 13 Jan 2008 11:24:29 -0500, Jon Smirl wrote:
> On 1/13/08, Jean Delvare wrote:
> > On Sat, 12 Jan 2008 11:26:34 -0500, Jon Smirl wrote:
> > > IMHO, driver_name/type should be removed in new style drivers and
> > > replaced with aliases on all platforms since aliases are the standard
> > > kernel mechanism.
> >
> > I agree. But we can take your aliasing code now (once you have
> > addressed the issues I raised) and convert the users of driver_name
> > later; it doesn't have to be done all at once.
> 
> GregKH, adding a new dynamically loadable subsystem is not something
> that happens every day, can you check to make sure all of the standard
> kernels mechanisms are being used? I'm not totally sure how the
> modalias naming code is supposed to be done. The subsystem core code
> in these patches needs review.
> 
> Jean, could you take over the i2c core portion of the patch? That will
> let you decide exactly how you want the driver_name/name fields to be
> dealt with. After you get standard naming support into i2c core I'll
> rework the rest of the patch to use your new code.

Yes, that could be done, and I agree that it will probably be faster
than iterative review/rework cycles between you and me. I'll free some
cycles next week for that.

> I don't think driver_name/name fields should be stored in an i2c
> structure at all. They are redundant with the standard mechanism.
> 
> The kernel automatically exposes modalias as a sysfs attribute so the
> string must be recorded further down in the driver support layers. No
> need to keep a copy in the i2c structure.

Really? I didn't know that. So that's another thing that the i2c
subsystem is not doing like the rest of the kernel? Can you please
point me to the code that does this?

> Standard devices don't export a 'name' attribute. To see the driver
> name for a device in sysfs look at the 'driver' link.

The driver name and the device name are different things! The "name"
attribute that i2c devices have tells user-space the device name, not
the driver name.

You may not like what the i2c subsystem does but you can't ignore its
history. The name attribute of i2c devices has been there pretty much
forever and user-space relies on it, thus we can't remove it.

-- 
Jean Delvare

^ 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