LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH] Add support for binary includes.
From: Jon Loeliger @ 2008-02-22 18:12 UTC (permalink / raw)
  To: Scott Wood, jdl, linuxppc-dev
In-Reply-To: <20080222053450.GH3064@localhost.localdomain>

David Gibson wrote:

>> node {
>> 	prop = /incbin/("path/to/data");
>> };
>>
>> node {
>> 	prop = /incbin/("path/to/data", 8, 16);
>> };
>
> I still dislike the syntax, but haven't thought of a better one yet.
> There are some issues with the implementation too, but I've been a bit
> too busy with ePAPR stuff to review properly.

I'm OK with the syntax, but whatever-ish.

Would these be better?:

    prop = /call/(incbin, "path/to/data", 17, 23);
    prop = /call[incbin]/("path/to/data");
    prop = /call incbin/("path/to/data", 12, 12+10);

What is the aspect of the syntax that you don't like?
I think we essentially need to stick in the /.../ realm
to be consistent with the other non-standard names being
used, like /include/.

I can see a generalized form that allows other pre-defined
or user-defined "functions" to be introduced and called
or used in a similar way:

    prop = <(22 + /fibonacci/(7)) 1000>;

    prop = /directoryof/("/path/to/some/file.doc");

    interrupt-map = /pci_int_map/(8000, 2, 14);

or whatever.  We can paint this bikeshed for a long time
if we need to.  Or, we can get down to some serious issue
if there are any.  Are there?

jdl

^ permalink raw reply

* [PATCH] ps3: Fix "unlikely" incorrect usage
From: Samuel Tardieu @ 2008-02-24  8:06 UTC (permalink / raw)
  To: linux-kernel; +Cc: linuxppc-dev, cbe-oss-dev

Fix unlikely(plug) == NO_IRQ into unlikely(plug == NO_IRQ).

Signed-off-by: Samuel Tardieu <sam@rfc1149.net>
---
 arch/powerpc/platforms/ps3/interrupt.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/platforms/ps3/interrupt.c b/arch/powerpc/platforms/ps3/interrupt.c
index 3a6db04..a14e5cd 100644
--- a/arch/powerpc/platforms/ps3/interrupt.c
+++ b/arch/powerpc/platforms/ps3/interrupt.c
@@ -709,7 +709,7 @@ static unsigned int ps3_get_irq(void)
 	asm volatile("cntlzd %0,%1" : "=r" (plug) : "r" (x));
 	plug &= 0x3f;
 
-	if (unlikely(plug) == NO_IRQ) {
+	if (unlikely(plug == NO_IRQ)) {
 		pr_debug("%s:%d: no plug found: thread_id %lu\n", __func__,
 			__LINE__, pd->thread_id);
 		dump_bmp(&per_cpu(ps3_private, 0));
-- 
1.5.4.2.197.g22c43

^ permalink raw reply related

* Status of 5200B and 440 Kernel Support
From: Albrecht Dreß @ 2008-02-24 10:17 UTC (permalink / raw)
  To: LinuxPPC Embedded

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

Hi all,

sorry if this is a dumb question, I'm new in the list...

For a new project, I am looking through the available PPC based SoC's.   
Good candidates are probably the Freescale 5200B or the AMCC 440EP (I  
need a FPU).

For a decision, I need some further information about the Kernel  
support (source code available, if possible in stock kernels, *not*  
only vendor binary modules!) of the included devices, in particular
- MTD/NAND controller
- Ethernet
- UART, IIC and SPI
- USB host
- local external bus (not PCI) DMA

Any information or pointers would be really appreciated!

Thanks in advance,
Albrecht.

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

^ permalink raw reply

* Re: Status of 5200B and 440 Kernel Support
From: Josh Boyer @ 2008-02-24 13:38 UTC (permalink / raw)
  To: Albrecht Dreß; +Cc: LinuxPPC Embedded
In-Reply-To: <1203848259l.2313l.1l@antares.localdomain>

On Sun, 24 Feb 2008 11:17:39 +0100
Albrecht Dre=C3=9F <albrecht.dress@arcor.de> wrote:

> For a decision, I need some further information about the Kernel =20
> support (source code available, if possible in stock kernels, *not* =20
> only vendor binary modules!) of the included devices, in particular

For 440EP in arch/powerpc:

> - MTD/NAND controller

NOR flash works.  NAND needs some additional code.

> - Ethernet

Works.

> - UART, IIC and SPI

UART works.  IIC has recent patches.  No idea about SPI.

> - USB host

Works.

> - local external bus (not PCI) DMA

DMA to the EBC for what?

josh

^ permalink raw reply

* Re: How to dynamically disable/enable CPU features?
From: Gerhard Pircher @ 2008-02-24 14:47 UTC (permalink / raw)
  To: benh; +Cc: linuxppc-dev, miltonm
In-Reply-To: <1203719521.6976.30.camel@pasglop>


-------- Original-Nachricht --------
> Datum: Sat, 23 Feb 2008 09:32:01 +1100
> Von: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> An: Gerhard Pircher <gerhard_pircher@gmx.net>
> CC: Milton Miller <miltonm@bga.com>, linuxppc-dev@ozlabs.org
> Betreff: Re: How to dynamically disable/enable CPU features?

> > The flag is in POSSIBLE. I now use this code in the platform probe
> > function to nop out the code affected by the flag:
> > 
> > cur_cpu_spec->cpu_features &= ~CPU_FTR_NEED_COHERENT;
> > /* Patch out unwanted feature. */
> > do_feature_fixups(cur_cpu_spec->cpu_features,
> > 		  PTRRELOC(&__start___ftr_fixup),
> > 		  PTRRELOC(&__stop___ftr_fixup));
> > 
> > It seems to work so far, but I would like to know if this is the right
> > way to do it, or if calling do_feature_fixups() more than once can have
> > any side effects.
> 
> It's a bit hairy... Things -could- have been nop'ed out by the first
> call as a result of CPU_FTR_NEED_COHERENT being set and the second
> call will not be able to put them back in... now that may not be the
> case (depends what kind of patching is done with that flag) and so
> 'happen' to work for this specific bit but it isn't a nice solution...
I checked this now. Looks like it only needs to nop out some code (mainly
in the hash table code).

> A better long term approach is to look at moving the fixup to after
> the machine probe() after carefully checking whether that can cause
> any problem...
Well, that's a job for an more experienced kernel developer. :)

Thanks!

Gerhard
-- 
Psssst! Schon vom neuen GMX MultiMessenger gehört?
Der kann`s mit allen: http://www.gmx.net/de/go/multimessenger

^ permalink raw reply

* Re: [PATCHv4 2.6.25] i2c: adds support for i2c bus on Freescale CPM1/CPM2 controllers
From: Jochen Friedrich @ 2008-02-24 15:16 UTC (permalink / raw)
  To: Olof Johansson
  Cc: Jean Delvare, linuxppc-dev list, linux-kernel, Scott Wood, i2c
In-Reply-To: <20080223212823.GA22131@lixom.net>

Hi Olof,

>> 2. record the I2c name in the dts tree, either as seperate tag (like linux,i2c-name="<i2c-name>")
>>    or as additional compatible entry (like compatible="...", "linux,<i2c-name>").
> 
> I have to say no on this one. The device tree is not supposed to know
> about how linux uses devices, there are firmwares out there that don't
> use DTS for thier device trees, etc.

I still believe this this could be done for embedded devices which are usually booted
via wrapper or U-Boot as those devices will most probably use the most exotic I2c devices
out there (e.g. home-grown devices used by stbs). However, I'm not an device tree expert.
 
>> 3. use a glue layer with a translation map.
> 
> In my opinion this is an OK solution since the same information has to
> be added somewhere already anyway -- eiither to the drivers or to this
> translation table. It should of course be an abstacted shared table,
> preferrably contained under the i2c source directories since several
> platforms and architectures might share them.

I could think of a mixture between 2. and 3.:

Using the compatible attribute with the manufacturer stripped off as I2c name by default
and using an exception table. For now, the struct i2c_driver_device would currently only
need one entry ("dallas,ds1374", "rtc-ds1374").

Thanks,
Jochen

^ permalink raw reply

* Re: [PATCH] PowerPC 44x: add missing define TARGET_4xx to cuboot-taishan.c
From: Josh Boyer @ 2008-02-24 15:32 UTC (permalink / raw)
  To: Valentine Barshak; +Cc: linuxppc-dev
In-Reply-To: <20080221144317.GA19395@ru.mvista.com>

On Thu, 21 Feb 2008 17:43:17 +0300
Valentine Barshak <vbarshak@ru.mvista.com> wrote:

> In order to get the proper bd_info structure for PowerPC 440,
> both TARGET_4xx and TARGET_44x should be defined.

Could you explain what this adds or why it's needed in the changelog?
Also, is this needed for other 440 boards?

josh

^ permalink raw reply

* Re: [PATCHv4 2.6.25] i2c: adds support for i2c bus on Freescale CPM1/CPM2 controllers
From: Jon Smirl @ 2008-02-24 16:19 UTC (permalink / raw)
  To: Jean Delvare; +Cc: Scott Wood, linuxppc-dev list, i2c, linux-kernel
In-Reply-To: <47BEAF00.50106@scram.de>

On 2/22/08, Jochen Friedrich <jochen@scram.de> wrote:
> Hi Jean,
>
>
>  >> +/*
>  >> + * Wait for patch from Jon Smirl
>  >> + * #include "powerpc-common.h"
>  >> + */
>  >
>  > It doesn't make sense to merge this comment upstream.
>
>
> I know you don't like the patch from Jon Smirl and you also explained your reasons.
>  Fortunately, I2c no longer uses numeric device IDs but names. So what are the alternatives?
>
>  1. modify the I2c subsystem to accept OF names additionally to I2c names (proposed by Jon smirl).

The correct statement is: modify the i2c subsystem to support the
standard kernel driver aliasing mechanism. Leaving powerpc and OF out
of the argument for the moment, i2c has a custom aliasing scheme on
the x86 too.

So as a first step, can we remove the custom i2c aliasing scheme and
change i2c to use standard module aliases on the x86? Patches for this
already exist.

On 2/23/08, Jean Delvare <khali@linux-fr.org> wrote:
> The problem I have with this is that it breaks compatibility. The chip
>  name is not only used for device/driver matching, it is also exported
>  to userspace as a sysfs attribute ("name"). Applications might rely on
>  it. At least libsensors does.

I think there is some confusion here. The OF aliases are only used by
the kernel to load the correct driver. Would doing something like this
help?

static struct i2c_device_id pcf8563_id[] = {
	{"pcf8563", 0, "sysfs_legacy_name"},
	{"rtc8564", 0, "sysfs_legacy_name"},
	OF_ID("phillips,pcf8563", &pcf8563_id[0], 0)
	OF_ID("epson,rtc8564", &pcf8563_id[1], 0)
	{},
};
MODULE_DEVICE_TABLE(i2c, pcf8563_id);

Then in the probe function you can use the pointer to find the base id
entry and i2c never has to be aware that the OF alias exists.

>  2. record the I2c name in the dts tree, either as separate tag (like linux,i2c-name="<i2c-name>")

Not really practical for the millions of machines (all PowerPC Macs)
already shipped.

>    or as additional compatible entry (like compatible="...", "linux,<i2c-name>").
>  3. use a glue layer with a translation map.

Audio codecs have exactly the same problem. There are probably other
devices that also need mapping.

This mapping table will need to contain a map from the OF names to the
kernel driver names.  It will need to stored permanently in RAM and
contain all possible mappings. This table will only grow in size.

The kernel has a widely used mechanism for mapping -- aliases in the
device drivers. Why do we want to build a new, parallel one?

What we are doing now is option 4.

4. Use kconfig to build custom kernels for each target system. Don't
load drivers automatically based on what the BIOS tells us.

-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply

* Re: [PATCHv4 2.6.25] i2c: adds support for i2c bus on Freescale CPM1/CPM2 controllers
From: Olof Johansson @ 2008-02-24 18:15 UTC (permalink / raw)
  To: Jochen Friedrich
  Cc: Jean Delvare, linuxppc-dev list, linux-kernel, Scott Wood, i2c
In-Reply-To: <47C18A4E.5080804@scram.de>

Hi,

On Sun, Feb 24, 2008 at 04:16:30PM +0100, Jochen Friedrich wrote:
> Hi Olof,
> 
> >> 2. record the I2c name in the dts tree, either as seperate tag (like linux,i2c-name="<i2c-name>")
> >>    or as additional compatible entry (like compatible="...", "linux,<i2c-name>").
> > 
> > I have to say no on this one. The device tree is not supposed to know
> > about how linux uses devices, there are firmwares out there that don't
> > use DTS for thier device trees, etc.
> 
> I still believe this this could be done for embedded devices which are usually booted
> via wrapper or U-Boot as those devices will most probably use the most exotic I2c devices
> out there (e.g. home-grown devices used by stbs). However, I'm not an device tree expert.

Sorry, but you're wrong in your assumptions. Not all embedded devices
use U-boot, and not all use the wrapper. It's just a bad idea to encode
linux-specific things in the device tree, period.

And even if you DO decide to go that route, guess what? You need a
translation table just as with (3) anyway!

> >> 3. use a glue layer with a translation map.
> > 
> > In my opinion this is an OK solution since the same information has to
> > be added somewhere already anyway -- eiither to the drivers or to this
> > translation table. It should of course be an abstacted shared table,
> > preferrably contained under the i2c source directories since several
> > platforms and architectures might share them.
> 
> I could think of a mixture between 2. and 3.:
> 
> Using the compatible attribute with the manufacturer stripped off as I2c name by default
> and using an exception table. For now, the struct i2c_driver_device would currently only
> need one entry ("dallas,ds1374", "rtc-ds1374").

You still need the translation table, you're just flattening the
namespace to one string instead of two, the same information still has
to be encoded. I can't see what the benefit of this approach compared to
the other one is. "dallas,ds1374" already only has one translation entry
in the table?


-Olof

^ permalink raw reply

* Re: [PATCH] PowerPC 44x: add missing define TARGET_4xx to cuboot-taishan.c
From: Josh Boyer @ 2008-02-24 18:36 UTC (permalink / raw)
  To: Valentine Barshak; +Cc: linuxppc-dev
In-Reply-To: <20080224093255.7c47a1e2@zod.rchland.ibm.com>

On Sun, 24 Feb 2008 09:32:55 -0600
Josh Boyer <jwboyer@linux.vnet.ibm.com> wrote:

> On Thu, 21 Feb 2008 17:43:17 +0300
> Valentine Barshak <vbarshak@ru.mvista.com> wrote:
> 
> > In order to get the proper bd_info structure for PowerPC 440,
> > both TARGET_4xx and TARGET_44x should be defined.
> 
> Could you explain what this adds or why it's needed in the changelog?
> Also, is this needed for other 440 boards?

And actually, looking at this more, does taishan also need TARGET_440GX?

josh

^ permalink raw reply

* Re: [PATCH] ps3: Fix "unlikely" incorrect usage
From: Geoff Levand @ 2008-02-24 18:49 UTC (permalink / raw)
  To: Samuel Tardieu; +Cc: linuxppc-dev, linux-kernel, cbe-oss-dev
In-Reply-To: <20080224081037.D492320AFF@arrakis.enst.fr>

On 02/24/2008 12:06 AM, Samuel Tardieu wrote:
> Fix unlikely(plug) == NO_IRQ into unlikely(plug == NO_IRQ).
> 
> Signed-off-by: Samuel Tardieu <sam@rfc1149.net>
> ---
>  arch/powerpc/platforms/ps3/interrupt.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)

Thanks for the submission, but this fix is already in
ps3-linux.git.  I was waiting to collect a few other bugs
before sending it to Paul.

-Geoff

^ permalink raw reply

* PIKA Warp Rev B
From: Sean MacLennan @ 2008-02-24 19:50 UTC (permalink / raw)
  To: LinuxPPC-dev

For those who like seeing pictures of embedded boards, here's a pic of 
rev B of the PIKA taco, official name Warp. As always, this is a 
development version. I only put one module on so you could see the 440EP 
in all it's glory.

The dangley thing in the front is a combination button (the metal pad) 
and power leds.

ftp://ftp.seanm.ca/stuff/taco-revb.jpg

Also available at http:// same url.

Cheers,
   Sean

^ permalink raw reply

* Re: [PATCH 1/2][OF] Add of_device_is_disabled function
From: Nathan Lynch @ 2008-02-24 20:12 UTC (permalink / raw)
  To: Josh Boyer; +Cc: linuxppc-dev, sfr, davem
In-Reply-To: <20080223185904.757c2884@zod.rchland.ibm.com>

Josh Boyer wrote:
> On Sat, 23 Feb 2008 15:58:23 -0600
> Josh Boyer <jwboyer@linux.vnet.ibm.com> wrote:
> 
> > IEEE 1275 defined a standard "status" property to indicate the operational
> > status of a device.  The property has four possible values: okay, disabled,
> > fail, fail-xxx.  The absence of this property means the operational status
> > of the device is unknown or okay.
> > 
> > This adds a function called of_device_is_disabled that checks to see if a
> > node has the status property set to "disabled".  This can be quite useful
> > for devices that may be present but disabled due to pin sharing, etc.
> > 
> 
> Talking with Ben H a bit, he suggested to reverse this API.  Basically,
> create an of_device_is_available that returns 1 if the status property
> is completely missing, or if it's set to "okay" or "ok".  The latter is
> to cope with some broken firmwares.

I agree with Ben's suggestion.  The rtas_pci and eeh code could be
converted to use this, which gives a net savings of a few bytes with
ppc64_defconfig:


diff --git a/arch/powerpc/kernel/rtas_pci.c b/arch/powerpc/kernel/rtas_pci.c
index 433a0a0..39a752b 100644
--- a/arch/powerpc/kernel/rtas_pci.c
+++ b/arch/powerpc/kernel/rtas_pci.c
@@ -56,21 +56,6 @@ static inline int config_access_valid(struct pci_dn *dn, int where)
 	return 0;
 }
 
-static int of_device_available(struct device_node * dn)
-{
-        const char *status;
-
-        status = of_get_property(dn, "status", NULL);
-
-        if (!status)
-                return 1;
-
-        if (!strcmp(status, "okay"))
-                return 1;
-
-        return 0;
-}
-
 int rtas_read_config(struct pci_dn *pdn, int where, int size, u32 *val)
 {
 	int returnval = -1;
@@ -117,7 +102,7 @@ static int rtas_pci_read_config(struct pci_bus *bus,
 	for (dn = busdn->child; dn; dn = dn->sibling) {
 		struct pci_dn *pdn = PCI_DN(dn);
 		if (pdn && pdn->devfn == devfn
-		    && of_device_available(dn))
+		    && of_device_is_available(dn))
 			return rtas_read_config(pdn, where, size, val);
 	}
 
@@ -164,7 +149,7 @@ static int rtas_pci_write_config(struct pci_bus *bus,
 	for (dn = busdn->child; dn; dn = dn->sibling) {
 		struct pci_dn *pdn = PCI_DN(dn);
 		if (pdn && pdn->devfn == devfn
-		    && of_device_available(dn))
+		    && of_device_is_available(dn))
 			return rtas_write_config(pdn, where, size, val);
 	}
 	return PCIBIOS_DEVICE_NOT_FOUND;
diff --git a/arch/powerpc/platforms/pseries/eeh.c b/arch/powerpc/platforms/pseries/eeh.c
index 9eb539e..550b2f7 100644
--- a/arch/powerpc/platforms/pseries/eeh.c
+++ b/arch/powerpc/platforms/pseries/eeh.c
@@ -945,7 +945,6 @@ static void *early_enable_eeh(struct device_node *dn, void *data)
 	unsigned int rets[3];
 	struct eeh_early_enable_info *info = data;
 	int ret;
-	const char *status = of_get_property(dn, "status", NULL);
 	const u32 *class_code = of_get_property(dn, "class-code", NULL);
 	const u32 *vendor_id = of_get_property(dn, "vendor-id", NULL);
 	const u32 *device_id = of_get_property(dn, "device-id", NULL);
@@ -959,8 +958,8 @@ static void *early_enable_eeh(struct device_node *dn, void *data)
 	pdn->eeh_freeze_count = 0;
 	pdn->eeh_false_positives = 0;
 
-	if (status && strncmp(status, "ok", 2) != 0)
-		return NULL;	/* ignore devices with bad status */
+	if (!of_device_is_available(dn))
+		return NULL;
 
 	/* Ignore bad nodes. */
 	if (!class_code || !vendor_id || !device_id)

^ permalink raw reply related

* RE: [PATCH] Xilinx: hwicap: cleanup
From: Stephen Neuendorffer @ 2008-02-24 23:21 UTC (permalink / raw)
  To: Grant Likely; +Cc: linuxppc-dev, git-dev, jirislaby, linux-kernel
In-Reply-To: <fa686aa40802232216s47b7751bi384352697dbf4ce2@mail.gmail.com>



> -----Original Message-----
> From: glikely@secretlab.ca [mailto:glikely@secretlab.ca] On Behalf Of
Grant Likely
> Sent: Saturday, February 23, 2008 10:17 PM
> To: Stephen Neuendorffer
> Cc: linuxppc-dev@ozlabs.org; git-dev; jirislaby@gmail.com;
linux.kernel@vger.kernel.org
> Subject: Re: [PATCH] Xilinx: hwicap: cleanup
>=20
> On Mon, Feb 11, 2008 at 11:24 AM, Stephen Neuendorffer
> <stephen.neuendorffer@xilinx.com> wrote:
> > Fix some missing __user tags and incorrect section tags.
> >  Convert semaphores to mutexes.
> >  Make probed_devices re-entrancy and error condition safe.
> >  Fix some backwards memcpys.
> >  Some other minor cleanups.
> >  Use kerneldoc format.
> >
> >  Signed-off-by: Stephen Neuendorffer
<stephen.neuendorffer@xilinx.com>
>=20
> Thanks Steven, some more comments below.
>=20
> >
> >  ---
> >
> >  Grant, Since it appears that the driver will stay in as-is, here
are
> >  the updates against mainline, based on Jiri's comments.
> >  ---
> >   drivers/char/xilinx_hwicap/buffer_icap.c   |   80
++++++++++----------
> >   drivers/char/xilinx_hwicap/fifo_icap.c     |   60 +++++++-------
> >   drivers/char/xilinx_hwicap/xilinx_hwicap.c |  113
++++++++++++++++------------
> >   drivers/char/xilinx_hwicap/xilinx_hwicap.h |   24 +++---
> >   4 files changed, 148 insertions(+), 129 deletions(-)
> >
> >  diff --git a/drivers/char/xilinx_hwicap/buffer_icap.c
b/drivers/char/xilinx_hwicap/buffer_icap.c
> >  index dfea2bd..2c5d17d 100644
> >  --- a/drivers/char/xilinx_hwicap/buffer_icap.c
> >  +++ b/drivers/char/xilinx_hwicap/buffer_icap.c
> >  @@ -148,9 +148,9 @@ static inline void buffer_icap_set_size(void
__iomem *base_address,
> >   }
> >
> >   /**
> >  - * buffer_icap_mSetoffsetReg: Set the bram offset register.
> >  - * @parameter base_address: contains the base address of the
device.
> >  - * @parameter data: is the value to be written to the data
register.
> >  + * buffer_icap_mSetoffsetReg - Set the bram offset register.
>=20
> This is the only function that is still in camel case; it should
> probably be changed also... In fact, this functions doesn't seem to be
> used at all.  Can it just be removed?  Are there any other unused
> functions in this driver?

Actually, it's just the comment that still had the old name.. Fixed it.

-Wall reports one unused static:

drivers/char/xilinx_hwicap/xilinx_hwicap.c:240: warning:
'hwicap_command_capture' defined but not used

I'd intended to leave this in, but I'm thinking it can be done by
userspace code using this driver, so I took it out too.

In verifying this, I discovered that I had inserted a variable names
'register', which doesn't work...  Fixed that too.

> >  diff --git a/drivers/char/xilinx_hwicap/xilinx_hwicap.c
> b/drivers/char/xilinx_hwicap/xilinx_hwicap.c
> >  index 24f6aef..eddaa26 100644
> >  --- a/drivers/char/xilinx_hwicap/xilinx_hwicap.c
> >  +++ b/drivers/char/xilinx_hwicap/xilinx_hwicap.c
> >  @@ -344,7 +345,7 @@ int hwicap_initialize_hwicap(struct
hwicap_drvdata *drvdata)
> >   }
> >
> >   static ssize_t
> >  -hwicap_read(struct file *file, char *buf, size_t count, loff_t
*ppos)
> >  +hwicap_read(struct file *file, __user char *buf, size_t count,
loff_t *ppos)
>=20
> This looks like it should be 'char __user *buf' instead of '__user
char *buf'.

Fixed.
=20
> >   {
> >         struct hwicap_drvdata *drvdata =3D file->private_data;
> >         ssize_t bytes_to_read =3D 0;
> >   static ssize_t
> >  -hwicap_write(struct file *file, const char *buf,
> >  +hwicap_write(struct file *file, const __user char *buf,
> >                 size_t count, loff_t *ppos)
>=20
> Ditto on placement of __user

Fixed.

>=20
> >  @@ -549,8 +556,7 @@ static int hwicap_release(struct inode *inode,
struct file *file)
> >         int i;
> >         int status =3D 0;
> >
> >  -       if (down_interruptible(&drvdata->sem))
> >  -               return -ERESTARTSYS;
> >  +       mutex_lock(&drvdata->sem);
>=20
> Why not mutex_lock_interruptible()? (goes for all cases of
mutex_lock())

It's not clear to me how to get 'correct' behavior in these functions if
the interrupt happens.  For instance in probe/setup, if the mutex_lock
is interrupted, it doesn't appear that there is anything to do other
than return an error code that no device is present?  I think this was
suggested by Jiri...

Steve

^ permalink raw reply

* [PATCH] [POWERPC] [v2] Xilinx: hwicap: cleanup
From: Stephen Neuendorffer @ 2008-02-24 23:34 UTC (permalink / raw)
  To: grant.likely, git-dev, linuxppc-dev, linux-kernel, jirislaby
In-Reply-To: <20080224232152.CEC5F12C8057@mail99-dub.bigfish.com>

Fix some missing __user tags and incorrect section tags.
Convert semaphores to mutexes.
Make probed_devices re-entrancy and error condition safe.
Fix some backwards memcpys.
Some other minor cleanups.
Use kerneldoc format.

[v2]
__user char => char __user
removed unused hwicap_command_capture
Fixed a comment that didn't match the function name
fixed argument with 'register' keyword.

Signed-off-by: Stephen Neuendorffer <stephen.neuendorffer@xilinx.com>

---

Grant, Since it appears that the driver will stay in as-is, here are
the updates against mainline, based on Jiri's comments.
---
 drivers/char/xilinx_hwicap/buffer_icap.c   |   80 ++++++++--------
 drivers/char/xilinx_hwicap/fifo_icap.c     |   60 ++++++------
 drivers/char/xilinx_hwicap/xilinx_hwicap.c |  138 +++++++++++++---------------
 drivers/char/xilinx_hwicap/xilinx_hwicap.h |   24 +++---
 4 files changed, 144 insertions(+), 158 deletions(-)

diff --git a/drivers/char/xilinx_hwicap/buffer_icap.c b/drivers/char/xilinx_hwicap/buffer_icap.c
index dfea2bd..f577dae 100644
--- a/drivers/char/xilinx_hwicap/buffer_icap.c
+++ b/drivers/char/xilinx_hwicap/buffer_icap.c
@@ -73,8 +73,8 @@
 #define XHI_BUFFER_START 0
 
 /**
- * buffer_icap_get_status: Get the contents of the status register.
- * @parameter base_address: is the base address of the device
+ * buffer_icap_get_status - Get the contents of the status register.
+ * @base_address: is the base address of the device
  *
  * The status register contains the ICAP status and the done bit.
  *
@@ -94,9 +94,9 @@ static inline u32 buffer_icap_get_status(void __iomem *base_address)
 }
 
 /**
- * buffer_icap_get_bram: Reads data from the storage buffer bram.
- * @parameter base_address: contains the base address of the component.
- * @parameter offset: The word offset from which the data should be read.
+ * buffer_icap_get_bram - Reads data from the storage buffer bram.
+ * @base_address: contains the base address of the component.
+ * @offset: The word offset from which the data should be read.
  *
  * A bram is used as a configuration memory cache.  One frame of data can
  * be stored in this "storage buffer".
@@ -108,8 +108,8 @@ static inline u32 buffer_icap_get_bram(void __iomem *base_address,
 }
 
 /**
- * buffer_icap_busy: Return true if the icap device is busy
- * @parameter base_address: is the base address of the device
+ * buffer_icap_busy - Return true if the icap device is busy
+ * @base_address: is the base address of the device
  *
  * The queries the low order bit of the status register, which
  * indicates whether the current configuration or readback operation
@@ -121,8 +121,8 @@ static inline bool buffer_icap_busy(void __iomem *base_address)
 }
 
 /**
- * buffer_icap_busy: Return true if the icap device is not busy
- * @parameter base_address: is the base address of the device
+ * buffer_icap_busy - Return true if the icap device is not busy
+ * @base_address: is the base address of the device
  *
  * The queries the low order bit of the status register, which
  * indicates whether the current configuration or readback operation
@@ -134,9 +134,9 @@ static inline bool buffer_icap_done(void __iomem *base_address)
 }
 
 /**
- * buffer_icap_set_size: Set the size register.
- * @parameter base_address: is the base address of the device
- * @parameter data: The size in bytes.
+ * buffer_icap_set_size - Set the size register.
+ * @base_address: is the base address of the device
+ * @data: The size in bytes.
  *
  * The size register holds the number of 8 bit bytes to transfer between
  * bram and the icap (or icap to bram).
@@ -148,9 +148,9 @@ static inline void buffer_icap_set_size(void __iomem *base_address,
 }
 
 /**
- * buffer_icap_mSetoffsetReg: Set the bram offset register.
- * @parameter base_address: contains the base address of the device.
- * @parameter data: is the value to be written to the data register.
+ * buffer_icap_set_offset - Set the bram offset register.
+ * @base_address: contains the base address of the device.
+ * @data: is the value to be written to the data register.
  *
  * The bram offset register holds the starting bram address to transfer
  * data from during configuration or write data to during readback.
@@ -162,9 +162,9 @@ static inline void buffer_icap_set_offset(void __iomem *base_address,
 }
 
 /**
- * buffer_icap_set_rnc: Set the RNC (Readback not Configure) register.
- * @parameter base_address: contains the base address of the device.
- * @parameter data: is the value to be written to the data register.
+ * buffer_icap_set_rnc - Set the RNC (Readback not Configure) register.
+ * @base_address: contains the base address of the device.
+ * @data: is the value to be written to the data register.
  *
  * The RNC register determines the direction of the data transfer.  It
  * controls whether a configuration or readback take place.  Writing to
@@ -178,10 +178,10 @@ static inline void buffer_icap_set_rnc(void __iomem *base_address,
 }
 
 /**
- * buffer_icap_set_bram: Write data to the storage buffer bram.
- * @parameter base_address: contains the base address of the component.
- * @parameter offset: The word offset at which the data should be written.
- * @parameter data: The value to be written to the bram offset.
+ * buffer_icap_set_bram - Write data to the storage buffer bram.
+ * @base_address: contains the base address of the component.
+ * @offset: The word offset at which the data should be written.
+ * @data: The value to be written to the bram offset.
  *
  * A bram is used as a configuration memory cache.  One frame of data can
  * be stored in this "storage buffer".
@@ -193,10 +193,10 @@ static inline void buffer_icap_set_bram(void __iomem *base_address,
 }
 
 /**
- * buffer_icap_device_read: Transfer bytes from ICAP to the storage buffer.
- * @parameter drvdata: a pointer to the drvdata.
- * @parameter offset: The storage buffer start address.
- * @parameter count: The number of words (32 bit) to read from the
+ * buffer_icap_device_read - Transfer bytes from ICAP to the storage buffer.
+ * @drvdata: a pointer to the drvdata.
+ * @offset: The storage buffer start address.
+ * @count: The number of words (32 bit) to read from the
  *           device (ICAP).
  **/
 static int buffer_icap_device_read(struct hwicap_drvdata *drvdata,
@@ -227,10 +227,10 @@ static int buffer_icap_device_read(struct hwicap_drvdata *drvdata,
 };
 
 /**
- * buffer_icap_device_write: Transfer bytes from ICAP to the storage buffer.
- * @parameter drvdata: a pointer to the drvdata.
- * @parameter offset: The storage buffer start address.
- * @parameter count: The number of words (32 bit) to read from the
+ * buffer_icap_device_write - Transfer bytes from ICAP to the storage buffer.
+ * @drvdata: a pointer to the drvdata.
+ * @offset: The storage buffer start address.
+ * @count: The number of words (32 bit) to read from the
  *           device (ICAP).
  **/
 static int buffer_icap_device_write(struct hwicap_drvdata *drvdata,
@@ -261,8 +261,8 @@ static int buffer_icap_device_write(struct hwicap_drvdata *drvdata,
 };
 
 /**
- * buffer_icap_reset: Reset the logic of the icap device.
- * @parameter drvdata: a pointer to the drvdata.
+ * buffer_icap_reset - Reset the logic of the icap device.
+ * @drvdata: a pointer to the drvdata.
  *
  * Writing to the status register resets the ICAP logic in an internal
  * version of the core.  For the version of the core published in EDK,
@@ -274,10 +274,10 @@ void buffer_icap_reset(struct hwicap_drvdata *drvdata)
 }
 
 /**
- * buffer_icap_set_configuration: Load a partial bitstream from system memory.
- * @parameter drvdata: a pointer to the drvdata.
- * @parameter data: Kernel address of the partial bitstream.
- * @parameter size: the size of the partial bitstream in 32 bit words.
+ * buffer_icap_set_configuration - Load a partial bitstream from system memory.
+ * @drvdata: a pointer to the drvdata.
+ * @data: Kernel address of the partial bitstream.
+ * @size: the size of the partial bitstream in 32 bit words.
  **/
 int buffer_icap_set_configuration(struct hwicap_drvdata *drvdata, u32 *data,
 			     u32 size)
@@ -333,10 +333,10 @@ int buffer_icap_set_configuration(struct hwicap_drvdata *drvdata, u32 *data,
 };
 
 /**
- * buffer_icap_get_configuration: Read configuration data from the device.
- * @parameter drvdata: a pointer to the drvdata.
- * @parameter data: Address of the data representing the partial bitstream
- * @parameter size: the size of the partial bitstream in 32 bit words.
+ * buffer_icap_get_configuration - Read configuration data from the device.
+ * @drvdata: a pointer to the drvdata.
+ * @data: Address of the data representing the partial bitstream
+ * @size: the size of the partial bitstream in 32 bit words.
  **/
 int buffer_icap_get_configuration(struct hwicap_drvdata *drvdata, u32 *data,
 			     u32 size)
diff --git a/drivers/char/xilinx_hwicap/fifo_icap.c b/drivers/char/xilinx_hwicap/fifo_icap.c
index 0988314..6f45dbd 100644
--- a/drivers/char/xilinx_hwicap/fifo_icap.c
+++ b/drivers/char/xilinx_hwicap/fifo_icap.c
@@ -94,9 +94,9 @@
 
 
 /**
- * fifo_icap_fifo_write: Write data to the write FIFO.
- * @parameter drvdata: a pointer to the drvdata.
- * @parameter data: the 32-bit value to be written to the FIFO.
+ * fifo_icap_fifo_write - Write data to the write FIFO.
+ * @drvdata: a pointer to the drvdata.
+ * @data: the 32-bit value to be written to the FIFO.
  *
  * This function will silently fail if the fifo is full.
  **/
@@ -108,8 +108,8 @@ static inline void fifo_icap_fifo_write(struct hwicap_drvdata *drvdata,
 }
 
 /**
- * fifo_icap_fifo_read: Read data from the Read FIFO.
- * @parameter drvdata: a pointer to the drvdata.
+ * fifo_icap_fifo_read - Read data from the Read FIFO.
+ * @drvdata: a pointer to the drvdata.
  *
  * This function will silently fail if the fifo is empty.
  **/
@@ -121,9 +121,9 @@ static inline u32 fifo_icap_fifo_read(struct hwicap_drvdata *drvdata)
 }
 
 /**
- * fifo_icap_set_read_size: Set the the size register.
- * @parameter drvdata: a pointer to the drvdata.
- * @parameter data: the size of the following read transaction, in words.
+ * fifo_icap_set_read_size - Set the the size register.
+ * @drvdata: a pointer to the drvdata.
+ * @data: the size of the following read transaction, in words.
  **/
 static inline void fifo_icap_set_read_size(struct hwicap_drvdata *drvdata,
 		u32 data)
@@ -132,8 +132,8 @@ static inline void fifo_icap_set_read_size(struct hwicap_drvdata *drvdata,
 }
 
 /**
- * fifo_icap_start_config: Initiate a configuration (write) to the device.
- * @parameter drvdata: a pointer to the drvdata.
+ * fifo_icap_start_config - Initiate a configuration (write) to the device.
+ * @drvdata: a pointer to the drvdata.
  **/
 static inline void fifo_icap_start_config(struct hwicap_drvdata *drvdata)
 {
@@ -142,8 +142,8 @@ static inline void fifo_icap_start_config(struct hwicap_drvdata *drvdata)
 }
 
 /**
- * fifo_icap_start_readback: Initiate a readback from the device.
- * @parameter drvdata: a pointer to the drvdata.
+ * fifo_icap_start_readback - Initiate a readback from the device.
+ * @drvdata: a pointer to the drvdata.
  **/
 static inline void fifo_icap_start_readback(struct hwicap_drvdata *drvdata)
 {
@@ -152,8 +152,8 @@ static inline void fifo_icap_start_readback(struct hwicap_drvdata *drvdata)
 }
 
 /**
- * fifo_icap_busy: Return true if the ICAP is still processing a transaction.
- * @parameter drvdata: a pointer to the drvdata.
+ * fifo_icap_busy - Return true if the ICAP is still processing a transaction.
+ * @drvdata: a pointer to the drvdata.
  **/
 static inline u32 fifo_icap_busy(struct hwicap_drvdata *drvdata)
 {
@@ -163,8 +163,8 @@ static inline u32 fifo_icap_busy(struct hwicap_drvdata *drvdata)
 }
 
 /**
- * fifo_icap_write_fifo_vacancy: Query the write fifo available space.
- * @parameter drvdata: a pointer to the drvdata.
+ * fifo_icap_write_fifo_vacancy - Query the write fifo available space.
+ * @drvdata: a pointer to the drvdata.
  *
  * Return the number of words that can be safely pushed into the write fifo.
  **/
@@ -175,8 +175,8 @@ static inline u32 fifo_icap_write_fifo_vacancy(
 }
 
 /**
- * fifo_icap_read_fifo_occupancy: Query the read fifo available data.
- * @parameter drvdata: a pointer to the drvdata.
+ * fifo_icap_read_fifo_occupancy - Query the read fifo available data.
+ * @drvdata: a pointer to the drvdata.
  *
  * Return the number of words that can be safely read from the read fifo.
  **/
@@ -187,11 +187,11 @@ static inline u32 fifo_icap_read_fifo_occupancy(
 }
 
 /**
- * fifo_icap_set_configuration: Send configuration data to the ICAP.
- * @parameter drvdata: a pointer to the drvdata.
- * @parameter frame_buffer: a pointer to the data to be written to the
+ * fifo_icap_set_configuration - Send configuration data to the ICAP.
+ * @drvdata: a pointer to the drvdata.
+ * @frame_buffer: a pointer to the data to be written to the
  *		ICAP device.
- * @parameter num_words: the number of words (32 bit) to write to the ICAP
+ * @num_words: the number of words (32 bit) to write to the ICAP
  *		device.
 
  * This function writes the given user data to the Write FIFO in
@@ -266,10 +266,10 @@ int fifo_icap_set_configuration(struct hwicap_drvdata *drvdata,
 }
 
 /**
- * fifo_icap_get_configuration: Read configuration data from the device.
- * @parameter drvdata: a pointer to the drvdata.
- * @parameter data: Address of the data representing the partial bitstream
- * @parameter size: the size of the partial bitstream in 32 bit words.
+ * fifo_icap_get_configuration - Read configuration data from the device.
+ * @drvdata: a pointer to the drvdata.
+ * @data: Address of the data representing the partial bitstream
+ * @size: the size of the partial bitstream in 32 bit words.
  *
  * This function reads the specified number of words from the ICAP device in
  * the polled mode.
@@ -335,8 +335,8 @@ int fifo_icap_get_configuration(struct hwicap_drvdata *drvdata,
 }
 
 /**
- * buffer_icap_reset: Reset the logic of the icap device.
- * @parameter drvdata: a pointer to the drvdata.
+ * buffer_icap_reset - Reset the logic of the icap device.
+ * @drvdata: a pointer to the drvdata.
  *
  * This function forces the software reset of the complete HWICAP device.
  * All the registers will return to the default value and the FIFO is also
@@ -360,8 +360,8 @@ void fifo_icap_reset(struct hwicap_drvdata *drvdata)
 }
 
 /**
- * fifo_icap_flush_fifo: This function flushes the FIFOs in the device.
- * @parameter drvdata: a pointer to the drvdata.
+ * fifo_icap_flush_fifo - This function flushes the FIFOs in the device.
+ * @drvdata: a pointer to the drvdata.
  */
 void fifo_icap_flush_fifo(struct hwicap_drvdata *drvdata)
 {
diff --git a/drivers/char/xilinx_hwicap/xilinx_hwicap.c b/drivers/char/xilinx_hwicap/xilinx_hwicap.c
index 24f6aef..2284fa2 100644
--- a/drivers/char/xilinx_hwicap/xilinx_hwicap.c
+++ b/drivers/char/xilinx_hwicap/xilinx_hwicap.c
@@ -84,7 +84,7 @@
 #include <linux/init.h>
 #include <linux/poll.h>
 #include <linux/proc_fs.h>
-#include <asm/semaphore.h>
+#include <linux/mutex.h>
 #include <linux/sysctl.h>
 #include <linux/version.h>
 #include <linux/fs.h>
@@ -119,6 +119,7 @@ module_param(xhwicap_minor, int, S_IRUGO);
 
 /* An array, which is set to true when the device is registered. */
 static bool probed_devices[HWICAP_DEVICES];
+static struct mutex icap_sem;
 
 static struct class *icap_class;
 
@@ -199,14 +200,14 @@ static const struct config_registers v5_config_registers = {
 };
 
 /**
- * hwicap_command_desync: Send a DESYNC command to the ICAP port.
- * @parameter drvdata: a pointer to the drvdata.
+ * hwicap_command_desync - Send a DESYNC command to the ICAP port.
+ * @drvdata: a pointer to the drvdata.
  *
  * This command desynchronizes the ICAP After this command, a
  * bitstream containing a NULL packet, followed by a SYNCH packet is
  * required before the ICAP will recognize commands.
  */
-int hwicap_command_desync(struct hwicap_drvdata *drvdata)
+static int hwicap_command_desync(struct hwicap_drvdata *drvdata)
 {
 	u32 buffer[4];
 	u32 index = 0;
@@ -228,51 +229,18 @@ int hwicap_command_desync(struct hwicap_drvdata *drvdata)
 }
 
 /**
- * hwicap_command_capture: Send a CAPTURE command to the ICAP port.
- * @parameter drvdata: a pointer to the drvdata.
- *
- * This command captures all of the flip flop states so they will be
- * available during readback.  One can use this command instead of
- * enabling the CAPTURE block in the design.
- */
-int hwicap_command_capture(struct hwicap_drvdata *drvdata)
-{
-	u32 buffer[7];
-	u32 index = 0;
-
-	/*
-	 * Create the data to be written to the ICAP.
-	 */
-	buffer[index++] = XHI_DUMMY_PACKET;
-	buffer[index++] = XHI_SYNC_PACKET;
-	buffer[index++] = XHI_NOOP_PACKET;
-	buffer[index++] = hwicap_type_1_write(drvdata->config_regs->CMD) | 1;
-	buffer[index++] = XHI_CMD_GCAPTURE;
-	buffer[index++] = XHI_DUMMY_PACKET;
-	buffer[index++] = XHI_DUMMY_PACKET;
-
-	/*
-	 * Write the data to the FIFO and intiate the transfer of data
-	 * present in the FIFO to the ICAP device.
-	 */
-	return drvdata->config->set_configuration(drvdata,
-			&buffer[0], index);
-
-}
-
-/**
- * hwicap_get_configuration_register: Query a configuration register.
- * @parameter drvdata: a pointer to the drvdata.
- * @parameter reg: a constant which represents the configuration
+ * hwicap_get_configuration_register - Query a configuration register.
+ * @drvdata: a pointer to the drvdata.
+ * @reg: a constant which represents the configuration
  *		register value to be returned.
  * 		Examples:  XHI_IDCODE, XHI_FLR.
- * @parameter RegData: returns the value of the register.
+ * @reg_data: returns the value of the register.
  *
  * Sends a query packet to the ICAP and then receives the response.
  * The icap is left in Synched state.
  */
-int hwicap_get_configuration_register(struct hwicap_drvdata *drvdata,
-		u32 reg, u32 *RegData)
+static int hwicap_get_configuration_register(struct hwicap_drvdata *drvdata,
+		u32 reg, u32 *reg_data)
 {
 	int status;
 	u32 buffer[6];
@@ -300,14 +268,14 @@ int hwicap_get_configuration_register(struct hwicap_drvdata *drvdata,
 	/*
 	 * Read the configuration register
 	 */
-	status = drvdata->config->get_configuration(drvdata, RegData, 1);
+	status = drvdata->config->get_configuration(drvdata, reg_data, 1);
 	if (status)
 		return status;
 
 	return 0;
 }
 
-int hwicap_initialize_hwicap(struct hwicap_drvdata *drvdata)
+static int hwicap_initialize_hwicap(struct hwicap_drvdata *drvdata)
 {
 	int status;
 	u32 idcode;
@@ -344,7 +312,7 @@ int hwicap_initialize_hwicap(struct hwicap_drvdata *drvdata)
 }
 
 static ssize_t
-hwicap_read(struct file *file, char *buf, size_t count, loff_t *ppos)
+hwicap_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
 {
 	struct hwicap_drvdata *drvdata = file->private_data;
 	ssize_t bytes_to_read = 0;
@@ -353,8 +321,9 @@ hwicap_read(struct file *file, char *buf, size_t count, loff_t *ppos)
 	u32 bytes_remaining;
 	int status;
 
-	if (down_interruptible(&drvdata->sem))
-		return -ERESTARTSYS;
+	status = mutex_lock_interruptible(&drvdata->sem);
+	if (status)
+		return status;
 
 	if (drvdata->read_buffer_in_use) {
 		/* If there are leftover bytes in the buffer, just */
@@ -370,8 +339,9 @@ hwicap_read(struct file *file, char *buf, size_t count, loff_t *ppos)
 			goto error;
 		}
 		drvdata->read_buffer_in_use -= bytes_to_read;
-		memcpy(drvdata->read_buffer + bytes_to_read,
-				drvdata->read_buffer, 4 - bytes_to_read);
+		memmove(drvdata->read_buffer,
+		       drvdata->read_buffer + bytes_to_read,
+		       4 - bytes_to_read);
 	} else {
 		/* Get new data from the ICAP, and return was was requested. */
 		kbuf = (u32 *) get_zeroed_page(GFP_KERNEL);
@@ -414,18 +384,20 @@ hwicap_read(struct file *file, char *buf, size_t count, loff_t *ppos)
 			status = -EFAULT;
 			goto error;
 		}
-		memcpy(kbuf, drvdata->read_buffer, bytes_remaining);
+		memcpy(drvdata->read_buffer,
+		       kbuf,
+		       bytes_remaining);
 		drvdata->read_buffer_in_use = bytes_remaining;
 		free_page((unsigned long)kbuf);
 	}
 	status = bytes_to_read;
  error:
-	up(&drvdata->sem);
+	mutex_unlock(&drvdata->sem);
 	return status;
 }
 
 static ssize_t
-hwicap_write(struct file *file, const char *buf,
+hwicap_write(struct file *file, const char __user *buf,
 		size_t count, loff_t *ppos)
 {
 	struct hwicap_drvdata *drvdata = file->private_data;
@@ -435,8 +407,9 @@ hwicap_write(struct file *file, const char *buf,
 	ssize_t len;
 	ssize_t status;
 
-	if (down_interruptible(&drvdata->sem))
-		return -ERESTARTSYS;
+	status = mutex_lock_interruptible(&drvdata->sem);
+	if (status)
+		return status;
 
 	left += drvdata->write_buffer_in_use;
 
@@ -465,7 +438,7 @@ hwicap_write(struct file *file, const char *buf,
 			memcpy(kbuf, drvdata->write_buffer,
 					drvdata->write_buffer_in_use);
 			if (copy_from_user(
-			    (((char *)kbuf) + (drvdata->write_buffer_in_use)),
+			    (((char *)kbuf) + drvdata->write_buffer_in_use),
 			    buf + written,
 			    len - (drvdata->write_buffer_in_use))) {
 				free_page((unsigned long)kbuf);
@@ -508,7 +481,7 @@ hwicap_write(struct file *file, const char *buf,
 	free_page((unsigned long)kbuf);
 	status = written;
  error:
-	up(&drvdata->sem);
+	mutex_unlock(&drvdata->sem);
 	return status;
 }
 
@@ -519,8 +492,9 @@ static int hwicap_open(struct inode *inode, struct file *file)
 
 	drvdata = container_of(inode->i_cdev, struct hwicap_drvdata, cdev);
 
-	if (down_interruptible(&drvdata->sem))
-		return -ERESTARTSYS;
+	status = mutex_lock_interruptible(&drvdata->sem);
+	if (status)
+		return status;
 
 	if (drvdata->is_open) {
 		status = -EBUSY;
@@ -539,7 +513,7 @@ static int hwicap_open(struct inode *inode, struct file *file)
 	drvdata->is_open = 1;
 
  error:
-	up(&drvdata->sem);
+	mutex_unlock(&drvdata->sem);
 	return status;
 }
 
@@ -549,8 +523,7 @@ static int hwicap_release(struct inode *inode, struct file *file)
 	int i;
 	int status = 0;
 
-	if (down_interruptible(&drvdata->sem))
-		return -ERESTARTSYS;
+	mutex_lock(&drvdata->sem);
 
 	if (drvdata->write_buffer_in_use) {
 		/* Flush write buffer. */
@@ -569,7 +542,7 @@ static int hwicap_release(struct inode *inode, struct file *file)
 
  error:
 	drvdata->is_open = 0;
-	up(&drvdata->sem);
+	mutex_unlock(&drvdata->sem);
 	return status;
 }
 
@@ -592,31 +565,36 @@ static int __devinit hwicap_setup(struct device *dev, int id,
 
 	dev_info(dev, "Xilinx icap port driver\n");
 
+	mutex_lock(&icap_sem);
+
 	if (id < 0) {
 		for (id = 0; id < HWICAP_DEVICES; id++)
 			if (!probed_devices[id])
 				break;
 	}
 	if (id < 0 || id >= HWICAP_DEVICES) {
+		mutex_unlock(&icap_sem);
 		dev_err(dev, "%s%i too large\n", DRIVER_NAME, id);
 		return -EINVAL;
 	}
 	if (probed_devices[id]) {
+		mutex_unlock(&icap_sem);
 		dev_err(dev, "cannot assign to %s%i; it is already in use\n",
 			DRIVER_NAME, id);
 		return -EBUSY;
 	}
 
 	probed_devices[id] = 1;
+	mutex_unlock(&icap_sem);
 
 	devt = MKDEV(xhwicap_major, xhwicap_minor + id);
 
-	drvdata = kmalloc(sizeof(struct hwicap_drvdata), GFP_KERNEL);
+	drvdata = kzalloc(sizeof(struct hwicap_drvdata), GFP_KERNEL);
 	if (!drvdata) {
 		dev_err(dev, "Couldn't allocate device private record\n");
-		return -ENOMEM;
+		retval = -ENOMEM;
+		goto failed0;
 	}
-	memset((void *)drvdata, 0, sizeof(struct hwicap_drvdata));
 	dev_set_drvdata(dev, (void *)drvdata);
 
 	if (!regs_res) {
@@ -648,7 +626,7 @@ static int __devinit hwicap_setup(struct device *dev, int id,
 	drvdata->config = config;
 	drvdata->config_regs = config_regs;
 
-	init_MUTEX(&drvdata->sem);
+	mutex_init(&drvdata->sem);
 	drvdata->is_open = 0;
 
 	dev_info(dev, "ioremap %lx to %p with size %x\n",
@@ -663,7 +641,7 @@ static int __devinit hwicap_setup(struct device *dev, int id,
 		goto failed3;
 	}
 	/*  devfs_mk_cdev(devt, S_IFCHR|S_IRUGO|S_IWUGO, DRIVER_NAME); */
-	class_device_create(icap_class, NULL, devt, NULL, DRIVER_NAME);
+	device_create(icap_class, dev, devt, "%s%d", DRIVER_NAME, id);
 	return 0;		/* success */
 
  failed3:
@@ -675,6 +653,11 @@ static int __devinit hwicap_setup(struct device *dev, int id,
  failed1:
 	kfree(drvdata);
 
+ failed0:
+	mutex_lock(&icap_sem);
+	probed_devices[id] = 0;
+	mutex_unlock(&icap_sem);
+
 	return retval;
 }
 
@@ -699,14 +682,16 @@ static int __devexit hwicap_remove(struct device *dev)
 	if (!drvdata)
 		return 0;
 
-	class_device_destroy(icap_class, drvdata->devt);
+	device_destroy(icap_class, drvdata->devt);
 	cdev_del(&drvdata->cdev);
 	iounmap(drvdata->base_address);
 	release_mem_region(drvdata->mem_start, drvdata->mem_size);
 	kfree(drvdata);
 	dev_set_drvdata(dev, NULL);
-	probed_devices[MINOR(dev->devt)-xhwicap_minor] = 0;
 
+	mutex_lock(&icap_sem);
+	probed_devices[MINOR(dev->devt)-xhwicap_minor] = 0;
+	mutex_unlock(&icap_sem);
 	return 0;		/* success */
 }
 
@@ -821,28 +806,29 @@ static struct of_platform_driver hwicap_of_driver = {
 };
 
 /* Registration helpers to keep the number of #ifdefs to a minimum */
-static inline int __devinit hwicap_of_register(void)
+static inline int __init hwicap_of_register(void)
 {
 	pr_debug("hwicap: calling of_register_platform_driver()\n");
 	return of_register_platform_driver(&hwicap_of_driver);
 }
 
-static inline void __devexit hwicap_of_unregister(void)
+static inline void __exit hwicap_of_unregister(void)
 {
 	of_unregister_platform_driver(&hwicap_of_driver);
 }
 #else /* CONFIG_OF */
 /* CONFIG_OF not enabled; do nothing helpers */
-static inline int __devinit hwicap_of_register(void) { return 0; }
-static inline void __devexit hwicap_of_unregister(void) { }
+static inline int __init hwicap_of_register(void) { return 0; }
+static inline void __exit hwicap_of_unregister(void) { }
 #endif /* CONFIG_OF */
 
-static int __devinit hwicap_module_init(void)
+static int __init hwicap_module_init(void)
 {
 	dev_t devt;
 	int retval;
 
 	icap_class = class_create(THIS_MODULE, "xilinx_config");
+	mutex_init(&icap_sem);
 
 	if (xhwicap_major) {
 		devt = MKDEV(xhwicap_major, xhwicap_minor);
@@ -883,7 +869,7 @@ static int __devinit hwicap_module_init(void)
 	return retval;
 }
 
-static void __devexit hwicap_module_cleanup(void)
+static void __exit hwicap_module_cleanup(void)
 {
 	dev_t devt = MKDEV(xhwicap_major, xhwicap_minor);
 
diff --git a/drivers/char/xilinx_hwicap/xilinx_hwicap.h b/drivers/char/xilinx_hwicap/xilinx_hwicap.h
index ae771ca..405fee7 100644
--- a/drivers/char/xilinx_hwicap/xilinx_hwicap.h
+++ b/drivers/char/xilinx_hwicap/xilinx_hwicap.h
@@ -48,9 +48,9 @@ struct hwicap_drvdata {
 	u8 write_buffer[4];
 	u32 read_buffer_in_use;	  /* Always in [0,3] */
 	u8 read_buffer[4];
-	u32 mem_start;		  /* phys. address of the control registers */
-	u32 mem_end;		  /* phys. address of the control registers */
-	u32 mem_size;
+	resource_size_t mem_start;/* phys. address of the control registers */
+	resource_size_t mem_end;  /* phys. address of the control registers */
+	resource_size_t mem_size;
 	void __iomem *base_address;/* virt. address of the control registers */
 
 	struct device *dev;
@@ -61,7 +61,7 @@ struct hwicap_drvdata {
 	const struct config_registers *config_regs;
 	void *private_data;
 	bool is_open;
-	struct semaphore sem;
+	struct mutex sem;
 };
 
 struct hwicap_driver_config {
@@ -164,29 +164,29 @@ struct config_registers {
 #define XHI_DISABLED_AUTO_CRC       0x0000DEFCUL
 
 /**
- * hwicap_type_1_read: Generates a Type 1 read packet header.
- * @parameter: Register is the address of the register to be read back.
+ * hwicap_type_1_read - Generates a Type 1 read packet header.
+ * @reg: is the address of the register to be read back.
  *
  * Generates a Type 1 read packet header, which is used to indirectly
  * read registers in the configuration logic.  This packet must then
  * be sent through the icap device, and a return packet received with
  * the information.
  **/
-static inline u32 hwicap_type_1_read(u32 Register)
+static inline u32 hwicap_type_1_read(u32 reg)
 {
 	return (XHI_TYPE_1 << XHI_TYPE_SHIFT) |
-		(Register << XHI_REGISTER_SHIFT) |
+		(reg << XHI_REGISTER_SHIFT) |
 		(XHI_OP_READ << XHI_OP_SHIFT);
 }
 
 /**
- * hwicap_type_1_write: Generates a Type 1 write packet header
- * @parameter: Register is the address of the register to be read back.
+ * hwicap_type_1_write - Generates a Type 1 write packet header
+ * @reg: is the address of the register to be read back.
  **/
-static inline u32 hwicap_type_1_write(u32 Register)
+static inline u32 hwicap_type_1_write(u32 reg)
 {
 	return (XHI_TYPE_1 << XHI_TYPE_SHIFT) |
-		(Register << XHI_REGISTER_SHIFT) |
+		(reg << XHI_REGISTER_SHIFT) |
 		(XHI_OP_WRITE << XHI_OP_SHIFT);
 }
 
-- 
1.5.3.4-dirty

^ permalink raw reply related

* Use aliases instead of linux,network-index on Ebony
From: David Gibson @ 2008-02-25  0:33 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: linuxppc-dev

This patch alters the Ebony bootwrapper to use the new preferred
method of using aliases to work out which MAC address to attach to
which ethernet device node, rather than the old method based on the
linux,network-index property.

The now obsolete linux,network-index properties are removed from the
ebony device tree as well.  This won't break backwards compatiblity,
because in cases where this fixup code is relevant, the device tree is
part of the kernel image.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

For 2.6.26, tested on an Ebony board with treeboot.

Index: working-2.6/arch/powerpc/boot/ebony.c
===================================================================
--- working-2.6.orig/arch/powerpc/boot/ebony.c	2008-02-25 11:22:31.000000000 +1100
+++ working-2.6/arch/powerpc/boot/ebony.c	2008-02-25 11:23:20.000000000 +1100
@@ -75,7 +75,8 @@ static void ebony_fixups(void)
 
 	ibm440gp_fixup_clocks(sysclk, 6 * 1843200);
 	ibm4xx_sdram_fixup_memsize();
-	dt_fixup_mac_addresses(ebony_mac0, ebony_mac1);
+	dt_fixup_mac_address_by_alias("ethernet0", ebony_mac0);
+	dt_fixup_mac_address_by_alias("ethernet1", ebony_mac1);
 	ibm4xx_fixup_ebc_ranges("/plb/opb/ebc");
 	ebony_flashsel_fixup();
 }
Index: working-2.6/arch/powerpc/boot/dts/ebony.dts
===================================================================
--- working-2.6.orig/arch/powerpc/boot/dts/ebony.dts	2008-02-25 11:25:28.000000000 +1100
+++ working-2.6/arch/powerpc/boot/dts/ebony.dts	2008-02-25 11:25:37.000000000 +1100
@@ -243,7 +243,6 @@
 			};
 
 			EMAC0: ethernet@40000800 {
-				linux,network-index = <0>;
 				device_type = "network";
 				compatible = "ibm,emac-440gp", "ibm,emac";
 				interrupt-parent = <&UIC1>;
@@ -263,7 +262,6 @@
 				zmii-channel = <0>;
 			};
 			EMAC1: ethernet@40000900 {
-				linux,network-index = <1>;
 				device_type = "network";
 				compatible = "ibm,emac-440gp", "ibm,emac";
 				interrupt-parent = <&UIC1>;

-- 
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: [PATCH] Add support for binary includes.
From: David Gibson @ 2008-02-25  3:38 UTC (permalink / raw)
  To: Jon Loeliger; +Cc: Scott Wood, linuxppc-dev, jdl
In-Reply-To: <47BF1089.8060204@freescale.com>

On Fri, Feb 22, 2008 at 12:12:25PM -0600, Jon Loeliger wrote:
> David Gibson wrote:
> 
> >> node {
> >> 	prop = /incbin/("path/to/data");
> >> };
> >>
> >> node {
> >> 	prop = /incbin/("path/to/data", 8, 16);
> >> };
> >
> > I still dislike the syntax, but haven't thought of a better one yet.
> > There are some issues with the implementation too, but I've been a bit
> > too busy with ePAPR stuff to review properly.
> 
> I'm OK with the syntax, but whatever-ish.
> 
> Would these be better?:
> 
>     prop = /call/(incbin, "path/to/data", 17, 23);
>     prop = /call[incbin]/("path/to/data");
>     prop = /call incbin/("path/to/data", 12, 12+10);

Ugh, I like those even less.

> What is the aspect of the syntax that you don't like?

Well, when I first objected, I didn't really know why.  But
contemplating your discussion below has helped by crystallise why this
syntax is making my ugly sense tingle.

1) I've no problem with using /word/ for "reserved words".  It's what
we're already doing, and importantly it's lexically distinct in all
contexts, even in proximity to the lexically troublesome node and
property names.

2) I've no problem with <something>(arg, arg, arg) function syntax.
Functions are handy, and this is consistent with our "least suprise to
C programmers" design principle.

What I don't like is the combination of the two.  Using the /word/
form in (1) suggests that each /word/ is a lexically distinct symbol
with functions in different contexts: consider /dts-v1/, /include/,
/memreserve/ - they're all used only in their own distinct context.
Use of /word/s in (2) would suggest that each /word/ is just an
identifier for a different function, and should all be usable in a
similar grammtical context - which won't be true of /memreserve/,
/dts-v1/ and any other truly lexically distinct symbols we need to
add.

> I think we essentially need to stick in the /.../ realm
> to be consistent with the other non-standard names being
> used, like /include/.

So, with the thoughts about, I come to the conclusion that, yes, we
should stick to /.../ for things which really act like reserved words,
but binary includes aren't one of those things.

Unlike source includes, binary includes only make sense inside a
property *value* (or to put it another way, an a context suitable for
an expression).  Which means the possible lexical confusion with
node/property names that motivated the /foo/ form in the first place
disappears.


So, I see two good options for the binary include.  Once is to treat
binary includes as a new special operator.  That operator (as it's
own, lexically distinct symbol) can be represented by a /word/, but in
that case, using it shouldn't look like a general function call.

Or, we can treat binary includes as a built-in function, see below.

> I can see a generalized form that allows other pre-defined
> or user-defined "functions" to be introduced and called
> or used in a similar way:
> 
>     prop = <(22 + /fibonacci/(7)) 1000>;
> 
>     prop = /directoryof/("/path/to/some/file.doc");
> 
>     interrupt-map = /pci_int_map/(8000, 2, 14);
> 
> or whatever.  We can paint this bikeshed for a long time
> if we need to.  Or, we can get down to some serious issue
> if there are any.  Are there?

So, I like the notion of functions like this, but with identifiers
that aren't /word/s.  Re-invoking the "least surprise to C
programmers" principle, in general I think the identifiers should be
as C identifiers (i.e. [a-zA-Z_][a-zA-Z0-9_]*).  With one caveat, it's
not essential but it might be worthwhile to make built-in function
identifiers obviously distinct from user-defined ones (if we add those
in future).


PS. I do see one potential gotcha with C-like function syntax.  As we
add expression support, the obvious way to handle constructs like:
	prop = <0x1 0x2>, "string";
Is to treat ',' as a bytestring concatenation operator.  There's
potentialy ambiguity between that usage and it's use as a separator
for function arguments.  Of course the same issue exists in C with its
comma operator, so it's not a showstopper.

-- 
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: Use aliases instead of linux,network-index on Ebony
From: Josh Boyer @ 2008-02-25  4:34 UTC (permalink / raw)
  To: David Gibson; +Cc: linuxppc-dev, Paul Mackerras
In-Reply-To: <20080225003339.GB16071@localhost.localdomain>

On Mon, 25 Feb 2008 11:33:39 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> This patch alters the Ebony bootwrapper to use the new preferred
> method of using aliases to work out which MAC address to attach to
> which ethernet device node, rather than the old method based on the
> linux,network-index property.

I like it.  But do we have this new preferred method documented
somewhere?  Doesn't seem to be in booting-without-of.txt.  It probably
should be added there, and reference to the linux,network-index
property removed if this is really preferred now.  Can you add
something to this patch for that?

josh

^ permalink raw reply

* Re: Use aliases instead of linux,network-index on Ebony
From: David Gibson @ 2008-02-25  6:49 UTC (permalink / raw)
  To: Josh Boyer; +Cc: linuxppc-dev, Paul Mackerras
In-Reply-To: <20080224223443.6f84d073@zod.rchland.ibm.com>

On Sun, Feb 24, 2008 at 10:34:43PM -0600, Josh Boyer wrote:
> On Mon, 25 Feb 2008 11:33:39 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > This patch alters the Ebony bootwrapper to use the new preferred
> > method of using aliases to work out which MAC address to attach to
> > which ethernet device node, rather than the old method based on the
> > linux,network-index property.
> 
> I like it.  But do we have this new preferred method documented
> somewhere?  Doesn't seem to be in booting-without-of.txt.  It probably
> should be added there, and reference to the linux,network-index
> property removed if this is really preferred now.  Can you add
> something to this patch for that?

Hrm.  linux,network-index was mentioned in b-w-o.txt, but to be honest
I don't think it ever belonged there.  It describes a bootloader to
kernel interface, whereas network-index was always a bootwrapper
internal hack, applicable only when the device tree and the fixup code
were built into the one image.

aliases, on the other hand do warrent wider mention, and aren't
mentioned at all in b-w-o.txt, but that's a matter of larger scope
than just getting rid of the network-index hack.

-- 
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: [PATCH] Xilinx: hwicap: cleanup
From: Jiri Slaby @ 2008-02-25  8:16 UTC (permalink / raw)
  To: Stephen Neuendorffer; +Cc: linuxppc-dev, git-dev, linux-kernel
In-Reply-To: <20080224232152.CEC5F12C8057@mail99-dub.bigfish.com>

On 02/25/2008 12:21 AM, Stephen Neuendorffer wrote:
>>>  @@ -549,8 +556,7 @@ static int hwicap_release(struct inode *inode,
> struct file *file)
>>>         int i;
>>>         int status = 0;
>>>
>>>  -       if (down_interruptible(&drvdata->sem))
>>>  -               return -ERESTARTSYS;
>>>  +       mutex_lock(&drvdata->sem);
>> Why not mutex_lock_interruptible()? (goes for all cases of
> mutex_lock())
> 
> It's not clear to me how to get 'correct' behavior in these functions if
> the interrupt happens.  For instance in probe/setup, if the mutex_lock
> is interrupted, it doesn't appear that there is anything to do other
> than return an error code that no device is present?  I think this was
> suggested by Jiri...

Yeah, since ERESTARTSYS would be ignored from f_op->release (see __fput()), 
drv->probe (see really_probe() and probe_failed label); not even talking about 
void return value functions. In those cases, the device won't be de/initialized 
and might result in unwanted behaviour (multiple modprobes for one device, 
rmmod/insmod need if you hit the path in release etc.).

^ permalink raw reply

* Re: 2.6.24 for mpc8458amc
From: maxime louvel @ 2008-02-25 10:42 UTC (permalink / raw)
  To: Kumar Gala; +Cc: linuxppc-embedded
In-Reply-To: <A0EDC255-48E2-4CDC-B768-C41D8274FFA0@kernel.crashing.org>

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

Hi,

I have updated the uboot on the cards and get the same result

AMC=> bootm 0x1000000
## Booting image at 01000000 ...
  Image Name:   Linux-2.6.24
  Image Type:   PowerPC Linux Kernel Image (gzip compressed)
  Data Size:    1802038 Bytes =  1.7 MB
  Load Address: 00000000
  Entry Point:  00000000
  Verifying Checksum ... OK
  Uncompressing Kernel Image ... OK

No further messages...

However I have try to boot an image (2.6.23-rc6 from freescale) which has
been patched for my platform and it works.
Thus the problem is not coming from the uboot anymore, but from something
else.
I can't get the source from freescale to see what they have changed because
their is some other stuff (unrelated with my problem or even my platform)
that are confidential to them...

So what I am looking for here is a hint of where to look, an idea of what I
might have done wrong...

regards,
Maxime

On Wed, Feb 20, 2008 at 1:53 PM, Kumar Gala <galak@kernel.crashing.org>
wrote:

>
> On Feb 20, 2008, at 7:00 AM, maxime louvel wrote:
>
> > Hi,
> >
> > yes It has something like 16550 UART.
> > the compiler version is gcc-3.4.3 with some specific stuff for the
> > platform.
> > I also have a gcc-4.1.2 vanilla which has been compiled with the
> > previous one. The 4.1.2 works if you tell it to emulate the floating
> > point instructions.
> >
> > thanks Scott,
> > without the treeboot-walnut it compiled
> >
> > I have still my first problem though:
> >
> > AMC=> bootm 0x1000000
> > ## Booting image at 01000000 ...
> >   Image Name:   Linux-2.6.24
> >   Image Type:   PowerPC Linux Kernel Image (gzip compressed)
> >   Data Size:    1802038 Bytes =  1.7 MB
> >   Load Address: 00000000
> >   Entry Point:  00000000
> >   Verifying Checksum ... OK
> >   Uncompressing Kernel Image ... OK
> >
> > and nothing else...
> >
> > Any idea ?
> >
> > This may become from the fact that the uboot installed on the cards
> > is old and it apparently doesn't support the newer version of the
> > kernel image... (I have got that from an another mailing list)
>
> Yes, you'll either need to look at updating u-boot and including
> support for the device tree in it or look at the cuImage wrappers in
> arch/powerpc/boot as a mechanism to boot a kernel w/an old fw.
>
> - k
>



-- 
Maxime Louvel
0044 7964 5555 80
43 Allen road
Whitemore reans
WV60AW Wolverhampton
United Kingdom

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

^ permalink raw reply

* Re: compile quirk linux-2.6.24 (with workaround)
From: Bernhard Reiter @ 2008-02-25 11:56 UTC (permalink / raw)
  To: debian-powerpc; +Cc: linuxppc-dev, paulus
In-Reply-To: <200802221550.27093.bernhard@intevation.de>

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

On Friday 22 February 2008 15:50, Bernhard Reiter wrote:
> > Ok, so it seems -mcpu=440 was added in gcc 3.4.  The -mcpu=405 option
> > has been around since 2001.  Seeing as how there really isn't anything
> > 440 specific in the files effected, we should be able to pass -mcpu=405
> > for everything and have it still work.
> >
> > Bernhard, can you try the patch below?
>
> I will test it in the next days.

Done. Looks good.
(I did _not_ do a full rebuild and installation, only a build test.
I will do a full blown test with 2.6.24.3.)

> The short test for my sarge compiler makes me optimistic:
>
> dpkg -l gcc | grep gcc
> ii  gcc            3.3.5-3        The GNU C compiler
> bernhard@burn:~/tmp$ gcc -mcpu=440 x.c
> cc1: error: bad value (440) for -mcpu= switch
> bernhard@burn:~/tmp$ gcc -mcpu=405 x.c
> bernhard@burn:~/tmp$
>
> > -$(obj)/cuboot-katmai.o: BOOTCFLAGS += -mcpu=440
> > +$(obj)/4xx.o: BOOTCFLAGS += -mcpu=405

Note: Your original did not fully apply, I think it had lines like
-$(obj)/cuboot-taishan.o: BOOTCFLAGS += -mcpu=440
-$(obj)/cuboot-katmai.o: BOOTCFLAGS += -mcpu=440
which I did not have in my 2.6.24.
Probably because you've used a git version of linux.
What I did was to change the similiar occurances from 440 to 405.

Bernhard

-- 
Managing Director - Owner: www.intevation.net       (Free Software Company)
Germany Coordinator: fsfeurope.org. Coordinator: www.Kolab-Konsortium.com.
Intevation GmbH, Osnabrück, DE; Amtsgericht Osnabrück, HRB 18998
Geschäftsführer Frank Koormann, Bernhard Reiter, Dr. Jan-Oliver Wagner

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

^ permalink raw reply

* Re: Use aliases instead of linux,network-index on Ebony
From: Josh Boyer @ 2008-02-25 11:56 UTC (permalink / raw)
  To: David Gibson; +Cc: linuxppc-dev, Paul Mackerras
In-Reply-To: <20080225064925.GA18527@localhost.localdomain>

On Mon, 25 Feb 2008 17:49:25 +1100
David Gibson <dwg@au1.ibm.com> wrote:

> On Sun, Feb 24, 2008 at 10:34:43PM -0600, Josh Boyer wrote:
> > On Mon, 25 Feb 2008 11:33:39 +1100
> > David Gibson <david@gibson.dropbear.id.au> wrote:
> > 
> > > This patch alters the Ebony bootwrapper to use the new preferred
> > > method of using aliases to work out which MAC address to attach to
> > > which ethernet device node, rather than the old method based on the
> > > linux,network-index property.
> > 
> > I like it.  But do we have this new preferred method documented
> > somewhere?  Doesn't seem to be in booting-without-of.txt.  It probably
> > should be added there, and reference to the linux,network-index
> > property removed if this is really preferred now.  Can you add
> > something to this patch for that?
> 
> Hrm.  linux,network-index was mentioned in b-w-o.txt, but to be honest
> I don't think it ever belonged there.  It describes a bootloader to
> kernel interface, whereas network-index was always a bootwrapper
> internal hack, applicable only when the device tree and the fixup code
> were built into the one image.

That's fine.  But can it you remove reference to it then?

> aliases, on the other hand do warrent wider mention, and aren't
> mentioned at all in b-w-o.txt, but that's a matter of larger scope
> than just getting rid of the network-index hack.

My concern is that people writing new ports will continue to copy DTS
files with linux,network-index in it.  Particularly since it's
recommended in b-w-of.txt, and there is no mentioned alternative.

josh

^ permalink raw reply

* E500 linux : are the 64-bit GPRs context-switched ?
From: Philippe De Muyter @ 2008-02-25 14:47 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <20080222095022.GA635@ingate.macqel>

Dear ppclinux gurus,

I have just compiled linux-2.6.24 for a MPC8540 target using a MPC8540
specific gcc.

After my first attempt using ARCH=ppc, leading to an infinity of messages :
"SPE used in kernel", I recompiled the whole kernel sources using
the default ARCH (ARCH=powerpc).  I now have a kernel that does not complain
about "SPE used in kernel", but user processes still crash ramdomly.

Searching around, I learned that the E500 GPR registers are 64-bits wide,
and gcc targetted for powerpc-linuxspe uses them sometimes.  In the other
PPC32 targets, those registers are 32-bits wide.

The specific E500 64-bit move instructions are `evstdd' and `evldd'.
I searched in the linux kernel sources (2.6.24) but did not find where
those GPR registers could be saved in 64-bit mode for context-switch.

Is there a patch pending somewhere to preserve the E500 GPR's in 64-bit mode
for context-switch ?

Philippe

^ permalink raw reply

* Re: E500 linux : are the 64-bit GPRs context-switched ?
From: Scott Wood @ 2008-02-25 16:58 UTC (permalink / raw)
  To: Philippe De Muyter; +Cc: linuxppc-dev
In-Reply-To: <20080225144722.GA5011@ingate.macqel>

On Mon, Feb 25, 2008 at 03:47:22PM +0100, Philippe De Muyter wrote:
> Searching around, I learned that the E500 GPR registers are 64-bits wide,
> and gcc targetted for powerpc-linuxspe uses them sometimes.  In the other
> PPC32 targets, those registers are 32-bits wide.
> 
> The specific E500 64-bit move instructions are `evstdd' and `evldd'.
> I searched in the linux kernel sources (2.6.24) but did not find where
> those GPR registers could be saved in 64-bit mode for context-switch.

giveup_spe and load_up_spe.

-Scott

^ 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