LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [patch 1/3] ps3: Disk Storage Driver
From: Andrew Morton @ 2007-07-19  9:10 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Jens Axboe, James E.J. Bottomley, linux-scsi, Jens Axboe,
	linux-kernel, Alessandro Rubini, linuxppc-dev, Paul Mackerras
In-Reply-To: <Pine.LNX.4.62.0707191039430.8661@pademelon.sonytel.be>

On Thu, 19 Jul 2007 10:57:53 +0200 (CEST) Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com> wrote:

> 	Hi Andrew,
> 
> On Wed, 18 Jul 2007, Andrew Morton wrote:
> > On Mon, 16 Jul 2007 18:15:40 +0200
> > Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com> wrote:
> > 
> > > From: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
> > > 
> > > Add a Disk Storage Driver for the PS3:
> > 
> > Your patchset significantly hits powerpc, scsi and block.  So who gets to
> > merge this?  Jens?  James?  Paul?
> > 
> > Me, I guess ;)
> 
> As Paul is on holidays, please take it.

OK.

> The PS3 storage driver core support is
> already in mainline, but the actual drivers aren't, as Paul was waiting for
> acks from the maintainers.
> 
> BTW, do you prefer incremental patches for the comments below, or an update of
> the full patchset?

Incremental is preferred, but I convert replacement patches into
incremental patches at about 10Hz nowadays.  Whatever's easier.

(Actually, if it's a replacement patch then only I get to see the
incremental patch, and the incremental patch is more reviewer-friendly).

> I didn't have much choice, as most of it was static and I don't need the full
> libata core anyway.
> 
> If I would factor it out, any good suggestion where to put the factored out
> code?

Take it up with Jeff, please.  If you're keen.  It isn't a lot of code.

> 
> > > +static int ps3disk_remove(struct ps3_system_bus_device *_dev)
> > > +{
> > > +	struct ps3_storage_device *dev = to_ps3_storage_device(&_dev->core);
> > > +	struct ps3disk_private *priv = ps3disk_priv(dev);
> > > +
> > > +	__clear_bit(priv->gendisk->first_minor / PS3DISK_MINORS,
> > > +		    &ps3disk_mask);
> > 
> > I see no locking here which makes this __clear_bit and the above __set_bit
> > non-racy?
> 
> Were .probe()/.remove() made concurrent again? I thought that idea was dropped
> because it caused too many problems?

I don't _think_ there's any global exclusion on ->probe calls.  For a
particular driver instance it's hard to see how these thigns can run
concurrently, dunno.  I guess two hotunplugs coud happen concurrently.

^ permalink raw reply

* Re: [patch 2/3] ps3: BD/DVD/CD-ROM Storage Driver
From: Andrew Morton @ 2007-07-19  9:17 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Jens Axboe, James E.J. Bottomley, linux-scsi, linux-kernel,
	Alessandro Rubini, linuxppc-dev, Paul Mackerras
In-Reply-To: <Pine.LNX.4.62.0707191100270.8661@pademelon.sonytel.be>

On Thu, 19 Jul 2007 11:02:07 +0200 (CEST) Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com> wrote:

> On Wed, 18 Jul 2007, Andrew Morton wrote:
> > > +struct ps3rom_private {
> > > +	struct ps3_storage_device *dev;
> > > +	struct scsi_cmnd *curr_cmd;
> > > +};
> > > +#define ps3rom_priv(dev)	((dev)->sbd.core.driver_data)
> > > +
> > 
> > Someone should invent a keyboard which delivers an electric shock when the
> > operator types "#define".   In the meanwhile, I get to do the honours.
> > 
> > Please don't implement in a macro anything which can be implemented in C.
> 
> All I needed was a shorthand to access driver_data, for both read and write
> access (you cannot do the latter with C, unless you decouple read and write).

Oh dear.

	ps3rom_priv(dev) = host;

that's 'orrid.  We have an identifier pretending to be a function, only we
go and treat it as an lvalue.

I mean, C code should look like C code, and the above just doesn't.

Sigh.

^ permalink raw reply

* Re: [patch 1/3] ps3: Disk Storage Driver
From: Cornelia Huck @ 2007-07-19  9:26 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Jens Axboe, James E.J. Bottomley, linux-scsi, Jens Axboe,
	linux-kernel, Alessandro Rubini, linuxppc-dev, Paul Mackerras,
	Andrew Morton
In-Reply-To: <Pine.LNX.4.62.0707191039430.8661@pademelon.sonytel.be>

On Thu, 19 Jul 2007 10:57:53 +0200 (CEST),
Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com> wrote:

> Were .probe()/.remove() made concurrent again? I thought that idea was dropped
> because it caused too many problems?

Well, for a given device, ->probe()/->remove() are locked by dev->sem,
so that there cannot be two probes/removes for the same device at the
same time. However, if you have multiple hotplug events pending at the
same time, it depends on your bus whether it does some serialization or
whether it allows multiple probes/removes running for different devices.
(Initial probing of a bus and probing of all devices with a new driver
is done serialized again, I think that's what you're referring to.)

^ permalink raw reply

* Re: [patch 1/3] ps3: Disk Storage Driver
From: Geert Uytterhoeven @ 2007-07-19  9:36 UTC (permalink / raw)
  To: Cornelia Huck
  Cc: Jens Axboe, James E.J. Bottomley, linux-scsi, Jens Axboe,
	Linux Kernel Development, Linux/PPC Development, Paul Mackerras,
	Andrew Morton
In-Reply-To: <20070719112602.5e3cb21a@gondolin.boeblingen.de.ibm.com>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1663 bytes --]

On Thu, 19 Jul 2007, Cornelia Huck wrote:
> On Thu, 19 Jul 2007 10:57:53 +0200 (CEST),
> Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com> wrote:
> 
> > Were .probe()/.remove() made concurrent again? I thought that idea was dropped
> > because it caused too many problems?
> 
> Well, for a given device, ->probe()/->remove() are locked by dev->sem,
> so that there cannot be two probes/removes for the same device at the
> same time. However, if you have multiple hotplug events pending at the

OK.

> same time, it depends on your bus whether it does some serialization or
> whether it allows multiple probes/removes running for different devices.
> (Initial probing of a bus and probing of all devices with a new driver
> is done serialized again, I think that's what you're referring to.)

We have a probe thread that checks for new storage devices and adds them to the
bus with ps3_system_bus_device_register(), which calls device_register().

I guess the actual bus probe() routine gets called through the notifier call
chain? That's where I got lost...

With kind regards,
 
Geert Uytterhoeven
Software Architect

Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
 
Phone:    +32 (0)2 700 8453	
Fax:      +32 (0)2 700 8622	
E-mail:   Geert.Uytterhoeven@sonycom.com	
Internet: http://www.sony-europe.com/
 	
Sony Network and Software Technology Center Europe	
A division of Sony Service Centre (Europe) N.V.	
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium	
VAT BE 0413.825.160 · RPR Brussels	
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619

^ permalink raw reply

* Re: [patch 2/3] ps3: BD/DVD/CD-ROM Storage Driver
From: Geert Uytterhoeven @ 2007-07-19  9:39 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Jens Axboe, James E.J. Bottomley, linux-scsi,
	Linux Kernel Development, Linux/PPC Development, Paul Mackerras
In-Reply-To: <20070719021752.35cb7daa.akpm@linux-foundation.org>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1948 bytes --]

On Thu, 19 Jul 2007, Andrew Morton wrote:
> On Thu, 19 Jul 2007 11:02:07 +0200 (CEST) Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com> wrote:
> 
> > On Wed, 18 Jul 2007, Andrew Morton wrote:
> > > > +struct ps3rom_private {
> > > > +	struct ps3_storage_device *dev;
> > > > +	struct scsi_cmnd *curr_cmd;
> > > > +};
> > > > +#define ps3rom_priv(dev)	((dev)->sbd.core.driver_data)
> > > > +
> > > 
> > > Someone should invent a keyboard which delivers an electric shock when the
> > > operator types "#define".   In the meanwhile, I get to do the honours.
> > > 
> > > Please don't implement in a macro anything which can be implemented in C.
> > 
> > All I needed was a shorthand to access driver_data, for both read and write
> > access (you cannot do the latter with C, unless you decouple read and write).
> 
> Oh dear.
> 
> 	ps3rom_priv(dev) = host;
> 
> that's 'orrid.  We have an identifier pretending to be a function, only we
> go and treat it as an lvalue.
> 
> I mean, C code should look like C code, and the above just doesn't.
> 
> Sigh.

Do you prefer

static inline struct ps3rom_private *ps3rom_priv_get(struct ps3_storage_device
*dev)
{
	return dev->sbd.core.driver_data;
}

static inline void ps3rom_priv_set(struct ps3_storage_device *dev,
				   struct ps3rom_private *priv)
{
	dev->sbd.core.driver_data = priv;
}

instead?

With kind regards,
 
Geert Uytterhoeven
Software Architect

Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
 
Phone:    +32 (0)2 700 8453	
Fax:      +32 (0)2 700 8622	
E-mail:   Geert.Uytterhoeven@sonycom.com	
Internet: http://www.sony-europe.com/
 	
Sony Network and Software Technology Center Europe	
A division of Sony Service Centre (Europe) N.V.	
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium	
VAT BE 0413.825.160 · RPR Brussels	
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619

^ permalink raw reply

* Re: [patch 2/3] ps3: BD/DVD/CD-ROM Storage Driver
From: Jens Axboe @ 2007-07-19  9:41 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: James E.J. Bottomley, linux-scsi, Linux Kernel Development,
	Linux/PPC Development, Paul Mackerras, Andrew Morton
In-Reply-To: <Pine.LNX.4.62.0707191137470.8661@pademelon.sonytel.be>

On Thu, Jul 19 2007, Geert Uytterhoeven wrote:
> On Thu, 19 Jul 2007, Andrew Morton wrote:
> > On Thu, 19 Jul 2007 11:02:07 +0200 (CEST) Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com> wrote:
> > 
> > > On Wed, 18 Jul 2007, Andrew Morton wrote:
> > > > > +struct ps3rom_private {
> > > > > +	struct ps3_storage_device *dev;
> > > > > +	struct scsi_cmnd *curr_cmd;
> > > > > +};
> > > > > +#define ps3rom_priv(dev)	((dev)->sbd.core.driver_data)
> > > > > +
> > > > 
> > > > Someone should invent a keyboard which delivers an electric shock when the
> > > > operator types "#define".   In the meanwhile, I get to do the honours.
> > > > 
> > > > Please don't implement in a macro anything which can be implemented in C.
> > > 
> > > All I needed was a shorthand to access driver_data, for both read and write
> > > access (you cannot do the latter with C, unless you decouple read and write).
> > 
> > Oh dear.
> > 
> > 	ps3rom_priv(dev) = host;
> > 
> > that's 'orrid.  We have an identifier pretending to be a function, only we
> > go and treat it as an lvalue.
> > 
> > I mean, C code should look like C code, and the above just doesn't.
> > 
> > Sigh.
> 
> Do you prefer
> 
> static inline struct ps3rom_private *ps3rom_priv_get(struct ps3_storage_device
> *dev)
> {
> 	return dev->sbd.core.driver_data;
> }
> 
> static inline void ps3rom_priv_set(struct ps3_storage_device *dev,
> 				   struct ps3rom_private *priv)
> {
> 	dev->sbd.core.driver_data = priv;
> }

how about just killing them? it makes the code harder to read, what is
the point of abstracting something like that out?

-- 
Jens Axboe

^ permalink raw reply

* Re: [patch 2/3] ps3: BD/DVD/CD-ROM Storage Driver
From: Andrew Morton @ 2007-07-19  9:47 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Jens Axboe, James E.J. Bottomley, linux-scsi, Linux/PPC,
	Linux Kernel Development, Development, Paul Mackerras
In-Reply-To: <Pine.LNX.4.62.0707191137470.8661@pademelon.sonytel.be>

On Thu, 19 Jul 2007 11:39:32 +0200 (CEST) Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com> wrote:

> > Oh dear.
> > 
> > 	ps3rom_priv(dev) = host;
> > 
> > that's 'orrid.  We have an identifier pretending to be a function, only we
> > go and treat it as an lvalue.
> > 
> > I mean, C code should look like C code, and the above just doesn't.
> > 
> > Sigh.
> 
> Do you prefer
> 
> static inline struct ps3rom_private *ps3rom_priv_get(struct ps3_storage_device
> *dev)
> {
> 	return dev->sbd.core.driver_data;
> }
> 
> static inline void ps3rom_priv_set(struct ps3_storage_device *dev,
> 				   struct ps3rom_private *priv)
> {
> 	dev->sbd.core.driver_data = priv;
> }
> 
> instead?

Yes.  Not that it's a terribly important issue, particularly down in the
dark and dusty corners where this code lurks.  The "function" as an lvalue
thing would be more obnoxious in some top-level interface.  We probably
already have some :(

If it were mine I'd open-code the "set" and do the "get" as a static
inline.  Or you could leave it as-is and go off and do something more
important ;)

^ permalink raw reply

* Re: [patch 3/3] ps3: FLASH ROM Storage Driver
From: Geert Uytterhoeven @ 2007-07-19 11:25 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Jens Axboe, James E.J. Bottomley, linux-scsi,
	Linux Kernel Development, Linux/PPC Development, Paul Mackerras
In-Reply-To: <20070718165222.e504d92e.akpm@linux-foundation.org>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 4687 bytes --]

	Hi Andrew,

On Wed, 18 Jul 2007, Andrew Morton wrote:
> > +struct ps3flash_private {
> > +	struct mutex mutex;	/* Bounce buffer mutex */
> > +};
> > +#define ps3flash_priv(dev)	((dev)->sbd.core.driver_data)
> 
> bzzzt!

Same defense as ps3rom ;-)

> > +static loff_t ps3flash_llseek(struct file *file, loff_t offset, int origin)
> > +{
> > +	struct ps3_storage_device *dev = ps3flash_dev;
> > +	u64 size = dev->regions[dev->region_idx].size*dev->blk_size;
> > +
> > +	switch (origin) {
> > +	case 1:
> > +		offset += file->f_pos;
> > +		break;
> > +	case 2:
> > +		offset += size;
> > +		break;
> > +	}
> > +	if (offset < 0)
> > +		return -EINVAL;
> > +
> > +	file->f_pos = offset;
> > +	return file->f_pos;
> > +}
> 
> lseek implementations usually need locking.  file->f_mapping->host->i_mutex
> would be typical.
> 
> That locking mostly protects 64-bit f_pos on 32-bit architectures so you
> can perahps kinda get away with it here.  However the code is a bit racy
> even on 64-bit, for silly userspace.
> 
> That being said, I'd have thought that you could use one of our many
> generic lseek library fucntions here, or even an newly-exported
> block_llseek().  That assumes that i_size is correct, which I trust is the
> case.

You mean generic_file_llseek()?

Who is responsible for setting i_size in that case? This is not a file, but a
character device, so currently i_size is always zero.

So I'll just add the missing locks.

> > +static ssize_t ps3flash_read(struct file *file, char __user *buf, size_t count,
> > +			     loff_t *pos)
> > +{
> > +	struct ps3_storage_device *dev = ps3flash_dev;
> > +	struct ps3flash_private *priv = ps3flash_priv(dev);
> > +	u64 size, start_sector, end_sector, offset;
> > +	ssize_t sectors_read;
> > +	size_t remaining, n;
> > +
> > +	dev_dbg(&dev->sbd.core,
> > +		"%s:%u: Reading %zu bytes at position %lld to user 0x%p\n",
> > +		__func__, __LINE__, count, *pos, buf);
> > +
> > +	size = dev->regions[dev->region_idx].size*dev->blk_size;
> > +	if (*pos >= size || !count)
> > +		return 0;
> > +
> > +	if (*pos+count > size) {
> > +		dev_dbg(&dev->sbd.core,
> > +			"%s:%u Truncating count from %zu to %llu\n", __func__,
> > +			__LINE__, count, size - *pos);
> > +		count = size - *pos;
> > +	}
> > +
> > +	start_sector = do_div_llr(*pos, dev->blk_size, &offset);
> > +	end_sector = DIV_ROUND_UP(*pos+count, dev->blk_size);
> > +
> > +	remaining = count;
> > +	do {
> > +		mutex_lock(&priv->mutex);
> > +
> > +		sectors_read = ps3flash_read_sectors(dev, start_sector,
> > +						     end_sector-start_sector,
> > +						     0);
> > +		if (sectors_read < 0) {
> > +			mutex_unlock(&priv->mutex);
> > +			return sectors_read;
> > +		}
> > +
> > +		n = min(remaining, sectors_read*dev->blk_size-offset);
> > +		dev_dbg(&dev->sbd.core,
> > +			"%s:%u: copy %lu bytes from 0x%p to user 0x%p\n",
> > +			__func__, __LINE__, n, dev->bounce_buf+offset, buf);
> > +		if (copy_to_user(buf, dev->bounce_buf+offset, n)) {
> > +			mutex_unlock(&priv->mutex);
> > +			return -EFAULT;
> > +		}
> > +
> > +		mutex_unlock(&priv->mutex);
> > +
> > +		*pos += n;
> > +		buf += n;
> > +		remaining -= n;
> > +		start_sector += sectors_read;
> > +		offset = 0;
> > +	} while (remaining > 0);
> > +
> > +	return count;
> > +}
> 
> There are several nasty deeply-embedded return points in this function.

Will change to `goto fail' and common error return.

> > +	if (*pos+count > size) {
> 
> checkpatch missed stuff here.

Checkpatch is silent...

> > +		dev_dbg(&dev->sbd.core,
> > +			"%s:%u Truncating count from %zu to %llu\n", __func__,
> > +			__LINE__, count, size - *pos);
> > +		count = size - *pos;
> > +	}
> > +
> > +	chunk_sectors = dev->bounce_size / dev->blk_size;
> > +
> > +	start_write_sector = do_div_llr(*pos, dev->bounce_size, &offset) *
> > +			     chunk_sectors;
> 
> It's strange to see a do_div_llr() in the middle of all this 64-bit-only
> code.  Could use / and %?

Sure.

Note that in theory do_div_llr() could do the division and modulo in one
instruction on architectures that support that (hmm, that's div_long_long_rem).

With kind regards,
 
Geert Uytterhoeven
Software Architect

Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
 
Phone:    +32 (0)2 700 8453	
Fax:      +32 (0)2 700 8622	
E-mail:   Geert.Uytterhoeven@sonycom.com	
Internet: http://www.sony-europe.com/
 	
Sony Network and Software Technology Center Europe	
A division of Sony Service Centre (Europe) N.V.	
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium	
VAT BE 0413.825.160 · RPR Brussels	
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619

^ permalink raw reply

* Re: [patch 1/3] ps3: Disk Storage Driver
From: Cornelia Huck @ 2007-07-19 12:00 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Jens Axboe, James E.J. Bottomley, Jens Axboe, linux-scsi,
	Development, Linux/PPC Development, Paul Mackerras, Linux,
	Andrew Morton
In-Reply-To: <Pine.LNX.4.62.0707191132150.8661@pademelon.sonytel.be>

On Thu, 19 Jul 2007 11:36:31 +0200 (CEST),
Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com> wrote:

> We have a probe thread that checks for new storage devices and adds them to the
> bus with ps3_system_bus_device_register(), which calls device_register().
> 
> I guess the actual bus probe() routine gets called through the notifier call
> chain? That's where I got lost...

No, ->probe() is called from device_register() directly. If you just
have one probe thread, you should have enough serialization.

(Unless you're doing something interesting from the bus_notifier, which
is called via the notifier chain before ->probe()...)

^ permalink raw reply

* Re: [patch 1/3] ps3: Disk Storage Driver
From: Geert Uytterhoeven @ 2007-07-19 12:13 UTC (permalink / raw)
  To: Cornelia Huck
  Cc: Jens Axboe, James E.J. Bottomley, linux-scsi, Jens Axboe,
	Linux Kernel Development, Linux/PPC Development, Paul Mackerras,
	Andrew Morton
In-Reply-To: <20070719140038.606abd44@gondolin.boeblingen.de.ibm.com>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1316 bytes --]

On Thu, 19 Jul 2007, Cornelia Huck wrote:
> On Thu, 19 Jul 2007 11:36:31 +0200 (CEST),
> Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com> wrote:
> 
> > We have a probe thread that checks for new storage devices and adds them to the
> > bus with ps3_system_bus_device_register(), which calls device_register().
> > 
> > I guess the actual bus probe() routine gets called through the notifier call
> > chain? That's where I got lost...
> 
> No, ->probe() is called from device_register() directly. If you just
> have one probe thread, you should have enough serialization.

IC, through bus_attach_device() in device_add().

Any chance this will change? I already added a mutex to ps3disk to protect
against this.

With kind regards,
 
Geert Uytterhoeven
Software Architect

Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
 
Phone:    +32 (0)2 700 8453	
Fax:      +32 (0)2 700 8622	
E-mail:   Geert.Uytterhoeven@sonycom.com	
Internet: http://www.sony-europe.com/
 	
Sony Network and Software Technology Center Europe	
A division of Sony Service Centre (Europe) N.V.	
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium	
VAT BE 0413.825.160 · RPR Brussels	
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619

^ permalink raw reply

* Re: [patch 1/3] ps3: Disk Storage Driver
From: Cornelia Huck @ 2007-07-19 12:52 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Jens Axboe, James E.J. Bottomley, Jens Axboe, linux-scsi,
	Development, Linux/PPC Development, Paul Mackerras, Linux,
	Andrew Morton
In-Reply-To: <Pine.LNX.4.62.0707191411060.8661@pademelon.sonytel.be>

On Thu, 19 Jul 2007 14:13:33 +0200 (CEST),
Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com> wrote:

> Any chance this will change? I already added a mutex to ps3disk to protect
> against this.

Probably not in the near future. A mutex looks like a good idea though,
since one never knows (and the driver core generally doesn't make many
guarantees regarding serialization).

^ permalink raw reply

* Re: [PATCH 1/4] ibmveth: Enable TCP checksum offload
From: Brian King @ 2007-07-19 13:35 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: linuxppc-dev, netdev, santil, rcjenn
In-Reply-To: <469E9510.7060703@garzik.org>

Jeff Garzik wrote:
> Brian King wrote:
>> This patchset enables TCP checksum offload support for IPV4
>> on ibmveth. This completely eliminates the generation and checking of
>> the checksum for packets that are completely virtual and never
>> touch a physical network. A simple TCP_STREAM netperf run on
>> a virtual network with maximum mtu set yielded a ~30% increase
>> in throughput. This feature is enabled by default on systems that
>> support it, but can be disabled with a module option.
>>
>> Signed-off-by: Brian King <brking@linux.vnet.ibm.com>
> 
> this should be controllable purely via ethtool...

I'll remove the module parameter and resend.

>>  struct ibmveth_rx_q_entry {
>>      u16 toggle : 1;
>>      u16 valid : 1;
>> -    u16 reserved : 14;
>> +    u16 reserved : 4;
>> +    u16 no_csum : 1;
>> +    u16 csum_good : 1;
>> +    u16 reserved2 : 8;
>>      u16 offset;
> 
> As a general rule, it would be nice to start moving away from bitfields 
> in this driver

I'll take a look at that for a future cleanup patch.

Thanks,

Brian

-- 
Brian King
Linux on Power Virtualization
IBM Linux Technology Center

^ permalink raw reply

* [patch 0/4] PS3 storage driver updates (was: Re: [patch 1/3] ps3: Disk Storage Driver)
From: Geert Uytterhoeven @ 2007-07-19 13:52 UTC (permalink / raw)
  To: Andrew Morton
  Cc: James E.J. Bottomley, linux-scsi, Jens Axboe,
	Linux Kernel Development, Linux/PPC Development, Paul Mackerras
In-Reply-To: <20070719021045.c0b994ca.akpm@linux-foundation.org>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1912 bytes --]

	Hi Andrew,

On Thu, 19 Jul 2007, Andrew Morton wrote:
> On Thu, 19 Jul 2007 10:57:53 +0200 (CEST) Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com> wrote:
> > On Wed, 18 Jul 2007, Andrew Morton wrote:
> > > Your patchset significantly hits powerpc, scsi and block.  So who gets to
> > > merge this?  Jens?  James?  Paul?
> > > 
> > > Me, I guess ;)
> > 
> > As Paul is on holidays, please take it.
> 
> OK.
> 
> > The PS3 storage driver core support is
> > already in mainline, but the actual drivers aren't, as Paul was waiting for
> > acks from the maintainers.
> > 
> > BTW, do you prefer incremental patches for the comments below, or an update of
> > the full patchset?
> 
> Incremental is preferred, but I convert replacement patches into
> incremental patches at about 10Hz nowadays.  Whatever's easier.
> 
> (Actually, if it's a replacement patch then only I get to see the
> incremental patch, and the incremental patch is more reviewer-friendly).

Here are the updates:
  [1] ps3disk: use correct bio vector size
  [2] ps3disk: updates after final review
  [3] ps3rom: updates after final review
  [4] ps3flash: updates after final review

The first one is the bugfix for O_DIRECT, the other 3 patches are the updates
in response to your review.

Thanks for integrating (and forwarding to Linus)!

With kind regards,
 
Geert Uytterhoeven
Software Architect

Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
 
Phone:    +32 (0)2 700 8453	
Fax:      +32 (0)2 700 8622	
E-mail:   Geert.Uytterhoeven@sonycom.com	
Internet: http://www.sony-europe.com/
 	
Sony Network and Software Technology Center Europe	
A division of Sony Service Centre (Europe) N.V.	
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium	
VAT BE 0413.825.160 · RPR Brussels	
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619

^ permalink raw reply

* [patch 1/4] ps3disk: use correct bio vector size
From: Geert Uytterhoeven @ 2007-07-19 13:53 UTC (permalink / raw)
  To: Andrew Morton
  Cc: James E.J. Bottomley, Olaf Hering, linux-scsi, Jens Axboe,
	Linux Kernel Development, Linux/PPC Development, Paul Mackerras
In-Reply-To: <Pine.LNX.4.62.0707191548070.8661@pademelon.sonytel.be>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1322 bytes --]

ps3disk: use correct bio vector size

This fixes the O_DIRECT corruptions, as reported by Olaf Hering (triggered by
e.g. parted >= 1.8.0).

Signed-off-by: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
---
 drivers/block/ps3disk.c |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/block/ps3disk.c
+++ b/drivers/block/ps3disk.c
@@ -108,7 +108,7 @@ static void ps3disk_scatter_gather(struc
 			__func__, __LINE__, i, bio_segments(bio),
 			bio_sectors(bio), sector);
 		bio_for_each_segment(bvec, bio, j) {
-			size = bio_cur_sectors(bio)*KERNEL_SECTOR_SIZE;
+			size = bvec->bv_len;
 			buf = __bio_kmap_atomic(bio, j, KM_IRQ0);
 			if (gather)
 				memcpy(dev->bounce_buf+offset, buf, size);

With kind regards,
 
Geert Uytterhoeven
Software Architect

Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
 
Phone:    +32 (0)2 700 8453	
Fax:      +32 (0)2 700 8622	
E-mail:   Geert.Uytterhoeven@sonycom.com	
Internet: http://www.sony-europe.com/
 	
Sony Network and Software Technology Center Europe	
A division of Sony Service Centre (Europe) N.V.	
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium	
VAT BE 0413.825.160 · RPR Brussels	
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619

^ permalink raw reply

* [patch 2/4] ps3disk: updates after final review
From: Geert Uytterhoeven @ 2007-07-19 13:54 UTC (permalink / raw)
  To: Andrew Morton
  Cc: James E.J. Bottomley, linux-scsi, Jens Axboe,
	Linux Kernel Development, Linux/PPC Development, Paul Mackerras
In-Reply-To: <Pine.LNX.4.62.0707191548070.8661@pademelon.sonytel.be>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 7440 bytes --]

ps3disk: updates after final review:
  o Kill KERNEL_SECTOR_SIZE, just hardcode `512' or `>> 9'
  o Kill confusing ps3disk_priv() macro, open code it instead
  o ps3disk_scatter_gather(): Kill unused variable  `sectors'
  o Introduce ps3disk_mask_mutex to protect ps3disk_mask
  o Minor coding style fixes not reported by checkpatch

Signed-off-by: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
---
 drivers/block/ps3disk.c |   44 +++++++++++++++++++++++++-------------------
 1 files changed, 25 insertions(+), 19 deletions(-)

--- a/drivers/block/ps3disk.c
+++ b/drivers/block/ps3disk.c
@@ -33,8 +33,6 @@
 #define PS3DISK_MAX_DISKS	16
 #define PS3DISK_MINORS		16
 
-#define KERNEL_SECTOR_SIZE	512
-
 
 #define PS3DISK_NAME		"ps3d%c"
 
@@ -48,7 +46,6 @@ struct ps3disk_private {
 	u64 raw_capacity;
 	unsigned char model[ATA_ID_PROD_LEN+1];
 };
-#define ps3disk_priv(dev)	((dev)->sbd.core.driver_data)
 
 
 #define LV1_STORAGE_SEND_ATA_COMMAND	(2)
@@ -93,7 +90,7 @@ static struct block_device_operations ps
 static void ps3disk_scatter_gather(struct ps3_storage_device *dev,
 				   struct request *req, int gather)
 {
-	unsigned int sectors = 0, offset = 0;
+	unsigned int offset = 0;
 	struct bio *bio;
 	sector_t sector;
 	struct bio_vec *bvec;
@@ -118,7 +115,6 @@ static void ps3disk_scatter_gather(struc
 			flush_kernel_dcache_page(bio_iovec_idx(bio, j)->bv_page);
 			__bio_kunmap_atomic(bio, KM_IRQ0);
 		}
-		sectors += bio_sectors(bio);
 		i++;
 	}
 }
@@ -126,7 +122,7 @@ static void ps3disk_scatter_gather(struc
 static int ps3disk_submit_request_sg(struct ps3_storage_device *dev,
 				     struct request *req)
 {
-	struct ps3disk_private *priv = ps3disk_priv(dev);
+	struct ps3disk_private *priv = dev->sbd.core.driver_data;
 	int write = rq_data_dir(req), res;
 	const char *op = write ? "write" : "read";
 	u64 start_sector, sectors;
@@ -135,6 +131,7 @@ static int ps3disk_submit_request_sg(str
 #ifdef DEBUG
 	unsigned int n = 0;
 	struct bio *bio;
+
 	rq_for_each_bio(bio, req)
 		n++;
 	dev_dbg(&dev->sbd.core,
@@ -143,8 +140,8 @@ static int ps3disk_submit_request_sg(str
 		req->hard_nr_sectors);
 #endif
 
-	start_sector = req->sector*priv->blocking_factor;
-	sectors = req->nr_sectors*priv->blocking_factor;
+	start_sector = req->sector * priv->blocking_factor;
+	sectors = req->nr_sectors * priv->blocking_factor;
 	dev_dbg(&dev->sbd.core, "%s:%u: %s %lu sectors starting at %lu\n",
 		__func__, __LINE__, op, sectors, start_sector);
 
@@ -173,7 +170,7 @@ static int ps3disk_submit_request_sg(str
 static int ps3disk_submit_flush_request(struct ps3_storage_device *dev,
 					struct request *req)
 {
-	struct ps3disk_private *priv = ps3disk_priv(dev);
+	struct ps3disk_private *priv = dev->sbd.core.driver_data;
 	u64 res;
 
 	dev_dbg(&dev->sbd.core, "%s:%u: flush request\n", __func__, __LINE__);
@@ -217,7 +214,7 @@ static void ps3disk_do_request(struct ps
 static void ps3disk_request(request_queue_t *q)
 {
 	struct ps3_storage_device *dev = q->queuedata;
-	struct ps3disk_private *priv = ps3disk_priv(dev);
+	struct ps3disk_private *priv = dev->sbd.core.driver_data;
 
 	if (priv->req) {
 		dev_dbg(&dev->sbd.core, "%s:%u busy\n", __func__, __LINE__);
@@ -250,7 +247,7 @@ static irqreturn_t ps3disk_interrupt(int
 		return IRQ_HANDLED;
 	}
 
-	priv = ps3disk_priv(dev);
+	priv = dev->sbd.core.driver_data;
 	req = priv->req;
 	if (!req) {
 		dev_dbg(&dev->sbd.core,
@@ -374,7 +371,7 @@ static void ata_id_c_string(const u16 *i
 
 static int ps3disk_identify(struct ps3_storage_device *dev)
 {
-	struct ps3disk_private *priv = ps3disk_priv(dev);
+	struct ps3disk_private *priv = dev->sbd.core.driver_data;
 	struct lv1_ata_cmnd_block ata_cmnd;
 	u16 *id = dev->bounce_buf;
 	u64 res;
@@ -439,6 +436,8 @@ static int ps3disk_issue_flush(request_q
 
 static unsigned long ps3disk_mask;
 
+static DEFINE_MUTEX(ps3disk_mask_mutex);
+
 static int __devinit ps3disk_probe(struct ps3_system_bus_device *_dev)
 {
 	struct ps3_storage_device *dev = to_ps3_storage_device(&_dev->core);
@@ -448,7 +447,7 @@ static int __devinit ps3disk_probe(struc
 	struct request_queue *queue;
 	struct gendisk *gendisk;
 
-	if (dev->blk_size < KERNEL_SECTOR_SIZE) {
+	if (dev->blk_size < 512) {
 		dev_err(&dev->sbd.core,
 			"%s:%u: cannot handle block size %lu\n", __func__,
 			__LINE__, dev->blk_size);
@@ -456,13 +455,16 @@ static int __devinit ps3disk_probe(struc
 	}
 
 	BUILD_BUG_ON(PS3DISK_MAX_DISKS > BITS_PER_LONG);
+	mutex_lock(&ps3disk_mask_mutex);
 	devidx = find_first_zero_bit(&ps3disk_mask, PS3DISK_MAX_DISKS);
 	if (devidx >= PS3DISK_MAX_DISKS) {
 		dev_err(&dev->sbd.core, "%s:%u: Too many disks\n", __func__,
 			__LINE__);
+		mutex_unlock(&ps3disk_mask_mutex);
 		return -ENOSPC;
 	}
 	__set_bit(devidx, &ps3disk_mask);
+	mutex_unlock(&ps3disk_mask_mutex);
 
 	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
 	if (!priv) {
@@ -470,7 +472,7 @@ static int __devinit ps3disk_probe(struc
 		goto fail;
 	}
 
-	ps3disk_priv(dev) = priv;
+	dev->sbd.core.driver_data = priv;
 	spin_lock_init(&priv->lock);
 
 	dev->bounce_size = BOUNCE_SIZE;
@@ -499,7 +501,7 @@ static int __devinit ps3disk_probe(struc
 
 	blk_queue_bounce_limit(queue, BLK_BOUNCE_HIGH);
 
-	blk_queue_max_sectors(queue, dev->bounce_size/KERNEL_SECTOR_SIZE);
+	blk_queue_max_sectors(queue, dev->bounce_size >> 9);
 	blk_queue_segment_boundary(queue, -1UL);
 	blk_queue_dma_alignment(queue, dev->blk_size-1);
 	blk_queue_hardsect_size(queue, dev->blk_size);
@@ -529,7 +531,7 @@ static int __devinit ps3disk_probe(struc
 	gendisk->driverfs_dev = &dev->sbd.core;
 	snprintf(gendisk->disk_name, sizeof(gendisk->disk_name), PS3DISK_NAME,
 		 devidx+'a');
-	priv->blocking_factor = dev->blk_size/KERNEL_SECTOR_SIZE;
+	priv->blocking_factor = dev->blk_size >> 9;
 	set_capacity(gendisk,
 		     dev->regions[dev->region_idx].size*priv->blocking_factor);
 
@@ -549,19 +551,23 @@ fail_free_bounce:
 	kfree(dev->bounce_buf);
 fail_free_priv:
 	kfree(priv);
-	ps3disk_priv(dev) = NULL;
+	dev->sbd.core.driver_data = NULL;
 fail:
+	mutex_lock(&ps3disk_mask_mutex);
 	__clear_bit(devidx, &ps3disk_mask);
+	mutex_unlock(&ps3disk_mask_mutex);
 	return error;
 }
 
 static int ps3disk_remove(struct ps3_system_bus_device *_dev)
 {
 	struct ps3_storage_device *dev = to_ps3_storage_device(&_dev->core);
-	struct ps3disk_private *priv = ps3disk_priv(dev);
+	struct ps3disk_private *priv = dev->sbd.core.driver_data;
 
+	mutex_lock(&ps3disk_mask_mutex);
 	__clear_bit(priv->gendisk->first_minor / PS3DISK_MINORS,
 		    &ps3disk_mask);
+	mutex_unlock(&ps3disk_mask_mutex);
 	del_gendisk(priv->gendisk);
 	blk_cleanup_queue(priv->queue);
 	put_disk(priv->gendisk);
@@ -570,7 +576,7 @@ static int ps3disk_remove(struct ps3_sys
 	ps3stor_teardown(dev);
 	kfree(dev->bounce_buf);
 	kfree(priv);
-	ps3disk_priv(dev) = NULL;
+	dev->sbd.core.driver_data = NULL;
 	return 0;
 }
 

With kind regards,
 
Geert Uytterhoeven
Software Architect

Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
 
Phone:    +32 (0)2 700 8453	
Fax:      +32 (0)2 700 8622	
E-mail:   Geert.Uytterhoeven@sonycom.com	
Internet: http://www.sony-europe.com/
 	
Sony Network and Software Technology Center Europe	
A division of Sony Service Centre (Europe) N.V.	
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium	
VAT BE 0413.825.160 · RPR Brussels	
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619

^ permalink raw reply

* [patch 3/4] ps3rom: updates after final review
From: Geert Uytterhoeven @ 2007-07-19 13:54 UTC (permalink / raw)
  To: Andrew Morton
  Cc: James E.J. Bottomley, linux-scsi, Jens Axboe,
	Linux Kernel Development, Linux/PPC Development, Paul Mackerras
In-Reply-To: <Pine.LNX.4.62.0707191548070.8661@pademelon.sonytel.be>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2901 bytes --]

ps3rom: updates after final review:
  o Kill confusing ps3rom_priv() macro, open code it instead
  o kmap_atomic() cannot fail

Signed-off-by: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
---
 drivers/scsi/ps3rom.c |   15 +++++----------
 1 files changed, 5 insertions(+), 10 deletions(-)

--- a/drivers/scsi/ps3rom.c
+++ b/drivers/scsi/ps3rom.c
@@ -42,7 +42,6 @@ struct ps3rom_private {
 	struct ps3_storage_device *dev;
 	struct scsi_cmnd *curr_cmd;
 };
-#define ps3rom_priv(dev)	((dev)->sbd.core.driver_data)
 
 
 #define LV1_STORAGE_SEND_ATAPI_COMMAND	(1)
@@ -113,8 +112,6 @@ static int fill_from_dev_buffer(struct s
 	for (k = 0, req_len = 0, act_len = 0; k < cmd->use_sg; ++k, ++sgpnt) {
 		if (active) {
 			kaddr = kmap_atomic(sgpnt->page, KM_IRQ0);
-			if (!kaddr)
-				return -1;
 			len = sgpnt->length;
 			if ((req_len + len) > buflen) {
 				active = 0;
@@ -151,8 +148,6 @@ static int fetch_to_dev_buffer(struct sc
 	sgpnt = cmd->request_buffer;
 	for (k = 0, req_len = 0, fin = 0; k < cmd->use_sg; ++k, ++sgpnt) {
 		kaddr = kmap_atomic(sgpnt->page, KM_IRQ0);
-		if (!kaddr)
-			return -1;
 		len = sgpnt->length;
 		if ((req_len + len) > buflen) {
 			len = buflen - req_len;
@@ -379,7 +374,7 @@ static irqreturn_t ps3rom_interrupt(int 
 		return IRQ_HANDLED;
 	}
 
-	host = ps3rom_priv(dev);
+	host = dev->sbd.core.driver_data;
 	priv = shost_priv(host);
 	cmd = priv->curr_cmd;
 
@@ -469,7 +464,7 @@ static int __devinit ps3rom_probe(struct
 	}
 
 	priv = shost_priv(host);
-	ps3rom_priv(dev) = host;
+	dev->sbd.core.driver_data = host;
 	priv->dev = dev;
 
 	/* One device/LUN per SCSI bus */
@@ -489,7 +484,7 @@ static int __devinit ps3rom_probe(struct
 
 fail_host_put:
 	scsi_host_put(host);
-	ps3rom_priv(dev) = NULL;
+	dev->sbd.core.driver_data = NULL;
 fail_teardown:
 	ps3stor_teardown(dev);
 fail_free_bounce:
@@ -500,12 +495,12 @@ fail_free_bounce:
 static int ps3rom_remove(struct ps3_system_bus_device *_dev)
 {
 	struct ps3_storage_device *dev = to_ps3_storage_device(&_dev->core);
-	struct Scsi_Host *host = ps3rom_priv(dev);
+	struct Scsi_Host *host = dev->sbd.core.driver_data;
 
 	scsi_remove_host(host);
 	ps3stor_teardown(dev);
 	scsi_host_put(host);
-	ps3rom_priv(dev) = NULL;
+	dev->sbd.core.driver_data = NULL;
 	kfree(dev->bounce_buf);
 	return 0;
 }

With kind regards,
 
Geert Uytterhoeven
Software Architect

Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
 
Phone:    +32 (0)2 700 8453	
Fax:      +32 (0)2 700 8622	
E-mail:   Geert.Uytterhoeven@sonycom.com	
Internet: http://www.sony-europe.com/
 	
Sony Network and Software Technology Center Europe	
A division of Sony Service Centre (Europe) N.V.	
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium	
VAT BE 0413.825.160 · RPR Brussels	
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619

^ permalink raw reply

* [patch 4/4] ps3flash: updates after final review
From: Geert Uytterhoeven @ 2007-07-19 13:55 UTC (permalink / raw)
  To: Andrew Morton
  Cc: James E.J. Bottomley, linux-scsi, Jens Axboe,
	Linux Kernel Development, Linux/PPC Development, Paul Mackerras
In-Reply-To: <Pine.LNX.4.62.0707191548070.8661@pademelon.sonytel.be>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 6253 bytes --]

ps3flash: updates after final review:
  o Kill confusing ps3flash_priv() macro, open code it instead
  o ps3flash_llseek(): Add missing locking (using i_mutex)
  o Replace do_div_llr() by `/' and `%'
  o ps3flash_read(): Use a common return for the error path
  o Minor coding style fixes not reported by checkpatch

Signed-off-by: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
---
 drivers/char/ps3flash.c |   59 ++++++++++++++++++++++++++++--------------------
 1 files changed, 35 insertions(+), 24 deletions(-)

--- a/drivers/char/ps3flash.c
+++ b/drivers/char/ps3flash.c
@@ -34,7 +34,6 @@
 struct ps3flash_private {
 	struct mutex mutex;	/* Bounce buffer mutex */
 };
-#define ps3flash_priv(dev)	((dev)->sbd.core.driver_data)
 
 static struct ps3_storage_device *ps3flash_dev;
 
@@ -81,28 +80,35 @@ static ssize_t ps3flash_write_chunk(stru
 static loff_t ps3flash_llseek(struct file *file, loff_t offset, int origin)
 {
 	struct ps3_storage_device *dev = ps3flash_dev;
-	u64 size = dev->regions[dev->region_idx].size*dev->blk_size;
+	loff_t res;
 
+	mutex_lock(&file->f_mapping->host->i_mutex);
 	switch (origin) {
 	case 1:
 		offset += file->f_pos;
 		break;
 	case 2:
-		offset += size;
+		offset += dev->regions[dev->region_idx].size*dev->blk_size;
 		break;
 	}
-	if (offset < 0)
-		return -EINVAL;
+	if (offset < 0) {
+		res = -EINVAL;
+		goto out;
+	}
 
 	file->f_pos = offset;
-	return file->f_pos;
+	res = file->f_pos;
+
+out:
+	mutex_unlock(&file->f_mapping->host->i_mutex);
+	return res;
 }
 
 static ssize_t ps3flash_read(struct file *file, char __user *buf, size_t count,
 			     loff_t *pos)
 {
 	struct ps3_storage_device *dev = ps3flash_dev;
-	struct ps3flash_private *priv = ps3flash_priv(dev);
+	struct ps3flash_private *priv = dev->sbd.core.driver_data;
 	u64 size, start_sector, end_sector, offset;
 	ssize_t sectors_read;
 	size_t remaining, n;
@@ -115,15 +121,16 @@ static ssize_t ps3flash_read(struct file
 	if (*pos >= size || !count)
 		return 0;
 
-	if (*pos+count > size) {
+	if (*pos + count > size) {
 		dev_dbg(&dev->sbd.core,
 			"%s:%u Truncating count from %zu to %llu\n", __func__,
 			__LINE__, count, size - *pos);
 		count = size - *pos;
 	}
 
-	start_sector = do_div_llr(*pos, dev->blk_size, &offset);
-	end_sector = DIV_ROUND_UP(*pos+count, dev->blk_size);
+	start_sector = *pos / dev->blk_size;
+	offset = *pos % dev->blk_size;
+	end_sector = DIV_ROUND_UP(*pos + count, dev->blk_size);
 
 	remaining = count;
 	do {
@@ -134,7 +141,7 @@ static ssize_t ps3flash_read(struct file
 						     0);
 		if (sectors_read < 0) {
 			mutex_unlock(&priv->mutex);
-			return sectors_read;
+			goto fail;
 		}
 
 		n = min(remaining, sectors_read*dev->blk_size-offset);
@@ -143,7 +150,8 @@ static ssize_t ps3flash_read(struct file
 			__func__, __LINE__, n, dev->bounce_buf+offset, buf);
 		if (copy_to_user(buf, dev->bounce_buf+offset, n)) {
 			mutex_unlock(&priv->mutex);
-			return -EFAULT;
+			sectors_read = -EFAULT;
+			goto fail;
 		}
 
 		mutex_unlock(&priv->mutex);
@@ -156,13 +164,16 @@ static ssize_t ps3flash_read(struct file
 	} while (remaining > 0);
 
 	return count;
+
+fail:
+	return sectors_read;
 }
 
 static ssize_t ps3flash_write(struct file *file, const char __user *buf,
 			      size_t count, loff_t *pos)
 {
 	struct ps3_storage_device *dev = ps3flash_dev;
-	struct ps3flash_private *priv = ps3flash_priv(dev);
+	struct ps3flash_private *priv = dev->sbd.core.driver_data;
 	u64 size, chunk_sectors, start_write_sector, end_write_sector,
 	    end_read_sector, start_read_sector, head, tail, offset;
 	ssize_t res;
@@ -177,7 +188,7 @@ static ssize_t ps3flash_write(struct fil
 	if (*pos >= size || !count)
 		return 0;
 
-	if (*pos+count > size) {
+	if (*pos + count > size) {
 		dev_dbg(&dev->sbd.core,
 			"%s:%u Truncating count from %zu to %llu\n", __func__,
 			__LINE__, count, size - *pos);
@@ -186,13 +197,13 @@ static ssize_t ps3flash_write(struct fil
 
 	chunk_sectors = dev->bounce_size / dev->blk_size;
 
-	start_write_sector = do_div_llr(*pos, dev->bounce_size, &offset) *
-			     chunk_sectors;
-	end_write_sector = DIV_ROUND_UP(*pos+count, dev->bounce_size) *
+	start_write_sector = *pos / dev->bounce_size * chunk_sectors;
+	offset = *pos % dev->bounce_size;
+	end_write_sector = DIV_ROUND_UP(*pos + count, dev->bounce_size) *
 			   chunk_sectors;
 
 	end_read_sector = DIV_ROUND_UP(*pos, dev->blk_size);
-	start_read_sector = (*pos+count) / dev->blk_size;
+	start_read_sector = (*pos + count) / dev->blk_size;
 
 	/*
 	 * As we have to write in 256 KiB chunks, while we can read in blk_size
@@ -204,8 +215,8 @@ static ssize_t ps3flash_write(struct fil
 	 * All of this is complicated by using only one 256 KiB bounce buffer.
 	 */
 
-	head = end_read_sector-start_write_sector;
-	tail = end_write_sector-start_read_sector;
+	head = end_read_sector - start_write_sector;
+	tail = end_write_sector - start_read_sector;
 
 	remaining = count;
 	do {
@@ -355,7 +366,7 @@ static int __devinit ps3flash_probe(stru
 		goto fail;
 	}
 
-	ps3flash_priv(dev) = priv;
+	dev->sbd.core.driver_data = priv;
 	mutex_init(&priv->mutex);
 
 	dev->bounce_size = ps3flash_bounce_buffer.size;
@@ -381,7 +392,7 @@ fail_teardown:
 	ps3stor_teardown(dev);
 fail_free_priv:
 	kfree(priv);
-	ps3flash_priv(dev) = NULL;
+	dev->sbd.core.driver_data = NULL;
 fail:
 	ps3flash_dev = NULL;
 	return error;
@@ -393,8 +404,8 @@ static int ps3flash_remove(struct ps3_sy
 
 	misc_deregister(&ps3flash_misc);
 	ps3stor_teardown(dev);
-	kfree(ps3flash_priv(dev));
-	ps3flash_priv(dev) = NULL;
+	kfree(dev->sbd.core.driver_data);
+	dev->sbd.core.driver_data = NULL;
 	ps3flash_dev = NULL;
 	return 0;
 }

With kind regards,
 
Geert Uytterhoeven
Software Architect

Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
 
Phone:    +32 (0)2 700 8453	
Fax:      +32 (0)2 700 8622	
E-mail:   Geert.Uytterhoeven@sonycom.com	
Internet: http://www.sony-europe.com/
 	
Sony Network and Software Technology Center Europe	
A division of Sony Service Centre (Europe) N.V.	
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium	
VAT BE 0413.825.160 · RPR Brussels	
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619

^ permalink raw reply

* RE: ML403 Hard TEMAC, PLB and Linux 2.6
From: linux-ppc @ 2007-07-19 13:51 UTC (permalink / raw)
  To: Rick Moleres
  Cc: linuxppc-embedded-bounces+linux-ppc=eurotel.it, mamsadegh,
	linuxppc-embedded
In-Reply-To: <20070208162356.3E8B45C6C3A@mail77-cpk.bigfish.com>


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.1: Type: text/plain; charset="GB2312", Size: 8375 bytes --]

Hi, we are trying to use both side of an hard temac + 2 plb temac in a 
Virtex4FX12 project.
we succesfull implemented a single temac Montavista tree with eth0.
when trying to use both temac, devices are correctly registered with 
kernel-
eth0 comes up and working ok-
when manually start eth1 (/sbin/ifconfig eth1 up) kernel hang without any 
error or info in the console

# dmesg
Linux version 2.6.10_mvl401-ml40x (massimiliano@linux-yhbz) (gcc version 
3.4.3 (MontaVista 3.4.3-25.0.135.0702900 2007-04-01)) #250 Wed Jul 18 
12:20:43 CEST 2007
Eurotel motherboard init
Port by MontaVista Software, Inc. (source@mvista.com)
On node 0 totalpages: 7812
  DMA zone: 7812 pages, LIFO batch:1
  Normal zone: 0 pages, LIFO batch:1
  HighMem zone: 0 pages, LIFO batch:1
Built 1 zonelists
Kernel command line: console=ttl0 root=/dev/xsysace2 rw ip=off
Xilinx INTC #0 at 0x41200000 mapped to 0xFDFFF000
PID hash table entries: 128 (order: 7, 2048 bytes)
hr_time_init: arch_to_nsec = 8192000, nsec_to_arch = 1099511627
Console: colour dummy device 80x25
Dentry cache hash table entries: 4096 (order: 2, 16384 bytes)
Inode-cache hash table entries: 2048 (order: 1, 8192 bytes)
Memory: 28420k available (1864k kernel code, 528k data, 124k init, 0k 
highmem)
Calibrating delay loop... 252.92 BogoMIPS (lpj=126464)
Mount-cache hash table entries: 512 (order: 0, 4096 bytes)
spawn_desched_task(00000000)
desched cpu_callback 3/00000000
ksoftirqd started up.
desched cpu_callback 2/00000000
desched thread 0 started up.
NET: Registered protocol family 16
Registering platform device 'xilinx_temac.0'. Parent at platform
Registering platform device 'xilinx_temac.1'. Parent at platform
Registering platform device 'xilinx_sysace.0'. Parent at platform
Registering platform device 'xilinx_gpio.0'. Parent at platform
Registering platform device 'xilinx_gpio.1'. Parent at platform
Registering platform device 'oled_fb.0'. Parent at platform
Generic PHY: Registered new driver
oled_fb: 4096 video memory mapped to c2011000
Console: switching to colour frame buffer device 16x8
xgpio #0 at 0x40000000 mapped to 0xC2020000
xgpio #1 at 0x40020000 mapped to 0xC2040000
io scheduler noop registered
io scheduler anticipatory registered
io scheduler deadline registered
io scheduler cfq registered
loop: loaded (max 8 devices)
elevator: using anticipatory as default io scheduler
System ACE at 0x41800000 mapped to 0xC2060000, irq=4, 1000944KB
 xsysace: xsysace1 xsysace2
Universal TUN/TAP device driver 1.5 (C)1999-2002 Maxim Krasnyansky
XTemac: using FIFO direct interrupt driven mode.
eth0: Xilinx TEMAC #0 at 0x20000000 mapped to 0xC2080000, irq=0
eth0: XTemac id 1.0f, block id 5, type 8
XTemac: using FIFO direct interrupt driven mode.
eth1: Xilinx TEMAC #1 at 0x20010000 mapped to 0xC20A0000, irq=10
eth1: XTemac id 1.0f, block id 5, type 8
i2c /dev entries driver
i2c-xil_custom: registered with I2C (0)
i2c-xil_custom: registered with I2C (1)
mice: PS/2 mouse device common for all mice
NET: Registered protocol family 2
IP: routing cache hash table of 512 buckets, 4Kbytes
TCP: Hash tables configured (established 2048 bind 4096)
NET: Registered protocol family 1
NET: Registered protocol family 17
EXT3-fs warning: maximal mount count reached, running e2fsck is 
recommended
kjournald starting.  Commit interval 5 seconds
EXT3 FS on xsysace2, internal journal
EXT3-fs: recovery complete.
EXT3-fs: mounted filesystem with ordered data mode.
VFS: Mounted root (ext3 filesystem).
Freeing unused kernel memory: 124k init
eth0: XTemac: Options: 0xb8f2
eth0: XTemac: speed set to 100Mb/s
eth0: XTemac: PHY Link carrier lost.

# /sbin/ifconfig eth1 up
# eth1: XTemac: Options: 0xb8f2

Any suggestion?




"Rick Moleres" <Rick.Moleres@xilinx.com> 
Sent by: linuxppc-embedded-bounces+linux-ppc=eurotel.it@ozlabs.org
08/02/2007 17.23

To
"Ming Liu" <eemingliu@hotmail.com>, <mamsadegh@hotmail.com>
cc
linuxppc-embedded@ozlabs.org
Subject
RE: ML403 Hard TEMAC, PLB and Linux 2.6






Hi,

As Ming says the Linux 2.6 TEMAC driver is made available in EDK 8.2.2 as 
part of the BSP and Libraries generation for Linux 2.6.  Note that we made 
a recent fix for better PHY address detection and speed negotiation. I've 
attached the adapter here, and it will be available in EDK 9.1.1 when 
that's released.

Thanks,
Rick

-----Original Message-----
From: linuxppc-embedded-bounces+moleres=xilinx.com@ozlabs.org 
[mailto:linuxppc-embedded-bounces+moleres=xilinx.com@ozlabs.org] On Behalf 
Of Ming Liu
Sent: Thursday, February 08, 2007 2:31 AM
To: mamsadegh@hotmail.com
Cc: linuxppc-embedded@ozlabs.org
Subject: RE: ML403 Hard TEMAC, PLB and Linux 2.6

Hi,
In xapp902, they are using the old cores for Temac. So it will be easier 
to 
generate a project in EDK 8.2. Not only the cores there are new, but also 
it can generate the driver for linux 2.6.

BR
Ming

>From: Mohammad Sadegh Sadri <mamsadegh@hotmail.com>
>To: <christophe.alayrac@cresitt.com>
>CC: linuxppc-embedded@ozlabs.org
>Subject: RE: ML403 Hard TEMAC, PLB and Linux 2.6
>Date: Thu, 8 Feb 2007 07:13:47 +0000
>
>
>Hi
>Thanks for reply
>Well, regarding xapp902, there is a very simple question, Where can I 
find 
it? It seems that Xilinx has deleted all of the links to these files.
>
>
>
>
>
>----------------------------------------
> > Subject: Re: ML403 Hard TEMAC, PLB and Linux 2.6
> > From: christophe.alayrac@cresitt.com
> > To: mamsadegh@hotmail.com
> > CC: linuxppc-embedded@ozlabs.org
> > Date: Thu, 8 Feb 2007 06:51:45 +0100
> >
> > Le mercredi 07 février 2007 ?22:30 +0000, Mohammad Sadegh Sadri a
> > écrit :
> > Hi Mohammad,
> >
> > Please find here after some answer
> >
> > > 1- I want to use, ML403, Hard TEMAC and PLB TEMAC cores. Is there 
any 
ready to use design, or any reference design available for this?
> > > I'm using EDK 8.1 and I understood that, when I choose the ML403 
board , EDK does not use hard temac, so , should I add it manually? ( well 

this is funny, but the wizard does not use hard temac , is it true? )
> > > Is there any ready EDK projects for ML403, with TEMAC and PLB TEMAC 
modules already inserted and cofigured? (I don't want to use GSRD ) ( If 
yes would you please put the link here )
> > >
> > You can start from XAPP 902 from Xilinx, this design demonstrate TEMAC
> > use in loopback mode. Some memers from that community have tried from
> > that design as a starting point. I did not nknow if the succeed.
> > I would recommand to get EDK 8.2 SP2 and use the Wizard to build a new
> > design with TEMAC.
> >
> > > 2- Simply, Is there any driver available for linux 2.6 , for PLB 
TEMAC and Hard TEMAC modules? If yes , can you put the link here, so that 
I 
can download it?
> > >
> > For the kernel you can get it from Montavista source code site using 
GIT
> > to download it (linux-xilinx-26). This is 2.6.17.4 version (last 
week!)
> >
> > Then you will need to pacth the Kernel with TEMAC drivers (look for
> > TEMAC PAULUS MVISTA with google, or follow my messages in that mailing
> > list)
> > NOTE: If you use that TEMAC patch do not use SGDMA on your TEMAC, use
> > FIFO.
> >
> > > thanks
> > Regards
> >
> > Chris
> > > _________________________________________________________________
> > > Personalize your Live.com homepage with the news, weather, and 
photos 
you care about.
> > > http://www.live.com/getstarted.aspx?icid=T001MSN30A0701
> > > _______________________________________________
> > > Linuxppc-embedded mailing list
> > > Linuxppc-embedded@ozlabs.org
> > > https://ozlabs.org/mailman/listinfo/linuxppc-embedded
> >
>
>_________________________________________________________________
>Live Search: New search found
>http://get.live.com/search/overview
>_______________________________________________
>Linuxppc-embedded mailing list
>Linuxppc-embedded@ozlabs.org
>https://ozlabs.org/mailman/listinfo/linuxppc-embedded

_________________________________________________________________
ÓëÁª»úµÄÅóÓѽøÐн»Á÷£¬ÇëʹÓà MSN Messenger:  http://messenger.msn.com/cn 

_______________________________________________
Linuxppc-embedded mailing list
Linuxppc-embedded@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-embedded


[-- Attachment #1.2: Type: text/html, Size: 13319 bytes --]

[-- Attachment #2: adapter.tar.gz --]
[-- Type: application/octet-stream, Size: 24141 bytes --]

^ permalink raw reply

* Re: [PATCH 49/61] 8xx: Update device trees.
From: Segher Boessenkool @ 2007-07-19 14:36 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev
In-Reply-To: <20070718013604.GU15238@ld0162-tx32.am.freescale.net>

> -			d-cache-line-size = <20>;	// 32 bytes
> -			i-cache-line-size = <20>;	// 32 bytes
> -			d-cache-size = <2000>;		// L1, 8K
> -			i-cache-size = <4000>;		// L1, 16K
> +			d-cache-line-size = <d#32>;
> +			i-cache-line-size = <d#32>;
> +			d-cache-size = <d#8192>;
> +			i-cache-size = <d#16384>;

IMHO this isn't an improvement, but it's your tree :-)
You removed the comment that those caches are L1; is
there an L2 cache on this CPU?

> -		mpc8xx_pic: pic@ff000000 {
> +		PIC: pic@0 {

interrupt-controller@0

> -			cpm_pic: pic@930 {
> +			CPM_PIC: pic@930 {

similar

> +				compatible = "fsl,mpc866-smc-uart",
> +				             "fsl,cpm1-smc-uart",
> +				             "fsl,cpm1-uart",
> +				             "fsl,cpm-smc-uart",
> +				             "fsl,cpm-uart";

Do you need _all_ of these?  :-)

> +				fsl,cpm-brg = <1>;
> +				fsl,cpm-command = <0090>;

Are these two documented?  Your patch queue is too
long for me to check for myself.

> -	soc885@ff000000 {
> +	bcsr@ff080000 {

Maybe use a more generic name, I have no idea what a
"BCSR" is.

Good improvements over all, thank you!


Segher

^ permalink raw reply

* Re: [PATCH 53/61] 82xx: Set NOT_COHERENT_CACHE on 8272 with PCI.
From: Segher Boessenkool @ 2007-07-19 14:40 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev
In-Reply-To: <20070718013611.GY15238@ld0162-tx32.am.freescale.net>

> I've seen significant network corruption on a PCI network
> card (e1000) with the mpc8272ads; the corruption went away
> when I set CONFIG_NOT_COHERENT_CACHE.

That doesn't mean it's the correct fix.  Any idea what the
root cause is?

If you want this patch as a workaround, please document it
in the source code.


Segher

^ permalink raw reply

* Re: [PATCH 30/61] fsl_soc: Update the way get_brgfreq() finds things in the device tree.
From: Kumar Gala @ 2007-07-19 14:55 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev
In-Reply-To: <469E408F.1080300@freescale.com>


On Jul 18, 2007, at 11:32 AM, Scott Wood wrote:

> Kumar Gala wrote:
>>> diff --git a/arch/powerpc/boot/dts/mpc8272ads.dts b/arch/powerpc/  
>>> boot/dts/mpc8272ads.dts
>>> index 4d09dca..16a77f4 100644
>>> --- a/arch/powerpc/boot/dts/mpc8272ads.dts
>>> +++ b/arch/powerpc/boot/dts/mpc8272ads.dts
>>> @@ -119,12 +119,11 @@
>>>              #address-cells = <1>;
>>>              #size-cells = <1>;
>>>              #interrupt-cells = <2>;
>>> -            device_type = "cpm";
>>> -            model = "CPM2";
>>> +            compatible = "fsl,mpc8272-cpm", "fsl,cpm2", "fsl,cpm";
>> Does 'fsl,cpm' really mean anything useful?
>
> Yes.  It's can't be used on its own to show the complete  
> programming model, but there are lots of common things that it does  
> indicate.
>
> get_brgfreq() uses it to locate nodes which have an fsl,brg- 
> frequency property.

I think we should introduce 'fsl,cpm' in a second pass once its clear  
what all the common points are.  We can than also see if QE fits into  
it at that point.

>
>>>              ranges = <00000000 00000000 20000>;
>>>              reg = <0 20000>;
>>>              command-proc = <119c0>;
>>> -            brg-frequency = <17D7840>;
>>> +            fsl,brg-frequency = <d#25000000>;
>> Leave brg-frequency, and make a note about it being deprecated.
>
> The CPM binding is changed in so many other ways that are much  
> harder to make backward compatible that I don't really see much  
> point in doing so here.

Can you enumerate some of the other changes.

>> Also, take a look at QE it has a similar concept.
>
> It'd be nice to extend this binding to include QE (and at some  
> point down the road, merge the code)...  I just didn't have time  
> this time around.

Understand.

As I stated above, I think we should drop 'fsl,cpm' for now until we  
can come up with a good defn. of what it means rather than a  
convenience to mean (fsl-cpm2 and fsl-cpm1).

- k

^ permalink raw reply

* Re: [PATCH 53/61] 82xx: Set NOT_COHERENT_CACHE on 8272 with PCI.
From: Kumar Gala @ 2007-07-19 14:58 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev, David Gibson
In-Reply-To: <469E7350.3090308@freescale.com>


On Jul 18, 2007, at 3:08 PM, Scott Wood wrote:

> Kumar Gala wrote:
>> On Jul 17, 2007, at 10:28 PM, David Gibson wrote:
>>> Hrm... I for one would be a lot more comfortable with this patch if
>>> you had a theoretical explanation for why it's necessary, in  
>>> addition
>>> to the "seems to fix things".
>
> Sure, so would I; I just wanted to point out the issue, and make  
> available a quick fix for anyone else that might be having problems.
>
> My theoretical explanation is that the hardware is broken. :-P
>
>> I agree w/David.  Have you tried turning on the   
>> CPU_FTR_NEED_COHERENT?  Take a look at include/asm-powerpc/ 
>> cputable.h  and search for the reference to CONFIG_PPC_83xx.
>
> OK, that appears to work as well.

Can you dump the POCMRx registers and report their values and what / 
proc/iomem looks like.

- k

^ permalink raw reply

* Re: [PATCH 56/61] 82xx: Add pq2fads board support.
From: Scott Wood @ 2007-07-19 14:59 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: linuxppc-dev
In-Reply-To: <200707190154.55368.arnd@arndb.de>

Arnd Bergmann wrote:
> On Wednesday 18 July 2007, Scott Wood wrote:
> 
>>+static void __init pq2fads_setup_arch(void)
>>+{
>>+	struct device_node *np;
>>+	struct resource r;
>>+	u32 *bcsr;
> 
> 
> bcsr should be __iomem, right?

Oops, yes.

> Did you run your code through sparse?

No, installing sparse is still on my to-do list.  I guess I should raise 
its priority. :-)

-Scott

^ permalink raw reply

* Re: [PATCH 04/61] 8xx: Work around CPU15 erratum.
From: Segher Boessenkool @ 2007-07-19 15:01 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev
In-Reply-To: <20070718013259.GC15238@ld0162-tx32.am.freescale.net>

> The CPU15 erratum on MPC8xx chips can cause incorrect code execution
> under certain circumstances, where there is a conditional or indirect
> branch in the last word of a page, with a target in the last cache  
> line
> of the next page.  This patch implements one of the suggested
> workarounds, by forcing a TLB miss whenever execution crosses a page
> boundary.  This is done by invalidating the pages before and after the
> one being loaded into the TLB in the ITLB miss handler.

So you never found a bug workaround without the terrible
overhead of this one?  A shame :-(


Segher

^ permalink raw reply

* RE: ML403 Hard TEMAC, PLB and Linux 2.6
From: Rick Moleres @ 2007-07-19 15:00 UTC (permalink / raw)
  To: linux-ppc; +Cc: mamsadegh, linuxppc-embedded
In-Reply-To: <OF313D78D4.648AA3F0-ONC125731D.004BF98E-C125731D.004C2CC5@eurotel.it>

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="gb2312", Size: 9795 bytes --]

Hi,

 

The first thing I would try is to make sure eth1 comes up without first bringing up eth0.  In other words, does it come up independently?  If not, there could be something wrong with its configuration in the kernel or perhaps a hardware design issue.  If both interfaces come up independently, but not together, then there¡¯s likely a driver issue as we have not tested dual channel TEMAC with the Linux plb_temac driver.  Perhaps others on the list have.  The areas that I would look at would be:  make sure each interface is getting a unique and correct PHY address; make sure any calls to the shared registers of the two channels are protected with semaphores/spinlocks.  For example, I¡¯m pretty sure the PHY registers are shared, so any PHY accesses should be protected.  I would think other Xilinx driver accesses like Start/Stop/Reset/Get or SetOptions/etc¡­ should be protected as they may access shared registers.

 

-Rick

 

________________________________

From: Massimiliano.Patriarca@eurotel.it [mailto:Massimiliano.Patriarca@eurotel.it] On Behalf Of linux-ppc@eurotel.it
Sent: Thursday, July 19, 2007 7:51 AM
To: Rick Moleres
Cc: Ming Liu; linuxppc-embedded@ozlabs.org; linuxppc-embedded-bounces+linux-ppc=eurotel.it@ozlabs.org; mamsadegh@hotmail.com
Subject: RE: ML403 Hard TEMAC, PLB and Linux 2.6

 


Hi, we are trying to use both side of an hard temac + 2 plb temac in a Virtex4FX12 project. 
we succesfull implemented a single temac Montavista tree with eth0. 
when trying to use both temac, devices are correctly registered with kernel- 
eth0 comes up and working ok- 
when manually start eth1 (/sbin/ifconfig eth1 up) kernel hang without any error or info in the console 

# dmesg 
Linux version 2.6.10_mvl401-ml40x (massimiliano@linux-yhbz) (gcc version 3.4.3 (MontaVista 3.4.3-25.0.135.0702900 2007-04-01)) #250 Wed Jul 18 12:20:43 CEST 2007 
Eurotel motherboard init 
Port by MontaVista Software, Inc. (source@mvista.com) 
On node 0 totalpages: 7812 
  DMA zone: 7812 pages, LIFO batch:1 
  Normal zone: 0 pages, LIFO batch:1 
  HighMem zone: 0 pages, LIFO batch:1 
Built 1 zonelists 
Kernel command line: console=ttl0 root=/dev/xsysace2 rw ip=off 
Xilinx INTC #0 at 0x41200000 mapped to 0xFDFFF000 
PID hash table entries: 128 (order: 7, 2048 bytes) 
hr_time_init: arch_to_nsec = 8192000, nsec_to_arch = 1099511627 
Console: colour dummy device 80x25 
Dentry cache hash table entries: 4096 (order: 2, 16384 bytes) 
Inode-cache hash table entries: 2048 (order: 1, 8192 bytes) 
Memory: 28420k available (1864k kernel code, 528k data, 124k init, 0k highmem) 
Calibrating delay loop... 252.92 BogoMIPS (lpj=126464) 
Mount-cache hash table entries: 512 (order: 0, 4096 bytes) 
spawn_desched_task(00000000) 
desched cpu_callback 3/00000000 
ksoftirqd started up. 
desched cpu_callback 2/00000000 
desched thread 0 started up. 
NET: Registered protocol family 16 
Registering platform device 'xilinx_temac.0'. Parent at platform 
Registering platform device 'xilinx_temac.1'. Parent at platform 
Registering platform device 'xilinx_sysace.0'. Parent at platform 
Registering platform device 'xilinx_gpio.0'. Parent at platform 
Registering platform device 'xilinx_gpio.1'. Parent at platform 
Registering platform device 'oled_fb.0'. Parent at platform 
Generic PHY: Registered new driver 
oled_fb: 4096 video memory mapped to c2011000 
Console: switching to colour frame buffer device 16x8 
xgpio #0 at 0x40000000 mapped to 0xC2020000 
xgpio #1 at 0x40020000 mapped to 0xC2040000 
io scheduler noop registered 
io scheduler anticipatory registered 
io scheduler deadline registered 
io scheduler cfq registered 
loop: loaded (max 8 devices) 
elevator: using anticipatory as default io scheduler 
System ACE at 0x41800000 mapped to 0xC2060000, irq=4, 1000944KB 
 xsysace: xsysace1 xsysace2 
Universal TUN/TAP device driver 1.5 (C)1999-2002 Maxim Krasnyansky 
XTemac: using FIFO direct interrupt driven mode. 
eth0: Xilinx TEMAC #0 at 0x20000000 mapped to 0xC2080000, irq=0 
eth0: XTemac id 1.0f, block id 5, type 8 
XTemac: using FIFO direct interrupt driven mode. 
eth1: Xilinx TEMAC #1 at 0x20010000 mapped to 0xC20A0000, irq=10 
eth1: XTemac id 1.0f, block id 5, type 8 
i2c /dev entries driver 
i2c-xil_custom: registered with I2C (0) 
i2c-xil_custom: registered with I2C (1) 
mice: PS/2 mouse device common for all mice 
NET: Registered protocol family 2 
IP: routing cache hash table of 512 buckets, 4Kbytes 
TCP: Hash tables configured (established 2048 bind 4096) 
NET: Registered protocol family 1 
NET: Registered protocol family 17 
EXT3-fs warning: maximal mount count reached, running e2fsck is recommended 
kjournald starting.  Commit interval 5 seconds 
EXT3 FS on xsysace2, internal journal 
EXT3-fs: recovery complete. 
EXT3-fs: mounted filesystem with ordered data mode. 
VFS: Mounted root (ext3 filesystem). 
Freeing unused kernel memory: 124k init 
eth0: XTemac: Options: 0xb8f2 
eth0: XTemac: speed set to 100Mb/s 
eth0: XTemac: PHY Link carrier lost. 

# /sbin/ifconfig eth1 up 
# eth1: XTemac: Options: 0xb8f2 

Any suggestion? 




"Rick Moleres" <Rick.Moleres@xilinx.com> 
Sent by: linuxppc-embedded-bounces+linux-ppc=eurotel.it@ozlabs.org 

08/02/2007 17.23 

To

"Ming Liu" <eemingliu@hotmail.com>, <mamsadegh@hotmail.com> 

cc

linuxppc-embedded@ozlabs.org 

Subject

RE: ML403 Hard TEMAC, PLB and Linux 2.6

 

 

 




Hi,

As Ming says the Linux 2.6 TEMAC driver is made available in EDK 8.2.2 as part of the BSP and Libraries generation for Linux 2.6.  Note that we made a recent fix for better PHY address detection and speed negotiation. I've attached the adapter here, and it will be available in EDK 9.1.1 when that's released.

Thanks,
Rick

-----Original Message-----
From: linuxppc-embedded-bounces+moleres=xilinx.com@ozlabs.org [mailto:linuxppc-embedded-bounces+moleres=xilinx.com@ozlabs.org] On Behalf Of Ming Liu
Sent: Thursday, February 08, 2007 2:31 AM
To: mamsadegh@hotmail.com
Cc: linuxppc-embedded@ozlabs.org
Subject: RE: ML403 Hard TEMAC, PLB and Linux 2.6

Hi,
In xapp902, they are using the old cores for Temac. So it will be easier to 
generate a project in EDK 8.2. Not only the cores there are new, but also 
it can generate the driver for linux 2.6.

BR
Ming

>From: Mohammad Sadegh Sadri <mamsadegh@hotmail.com>
>To: <christophe.alayrac@cresitt.com>
>CC: linuxppc-embedded@ozlabs.org
>Subject: RE: ML403 Hard TEMAC, PLB and Linux 2.6
>Date: Thu, 8 Feb 2007 07:13:47 +0000
>
>
>Hi
>Thanks for reply
>Well, regarding xapp902, there is a very simple question, Where can I find 
it? It seems that Xilinx has deleted all of the links to these files.
>
>
>
>
>
>----------------------------------------
> > Subject: Re: ML403 Hard TEMAC, PLB and Linux 2.6
> > From: christophe.alayrac@cresitt.com
> > To: mamsadegh@hotmail.com
> > CC: linuxppc-embedded@ozlabs.org
> > Date: Thu, 8 Feb 2007 06:51:45 +0100
> >
> > Le mercredi 07 février 2007 ?22:30 +0000, Mohammad Sadegh Sadri a
> > écrit :
> > Hi Mohammad,
> >
> > Please find here after some answer
> >
> > > 1- I want to use, ML403, Hard TEMAC and PLB TEMAC cores. Is there any 
ready to use design, or any reference design available for this?
> > > I'm using EDK 8.1 and I understood that, when I choose the ML403 
board , EDK does not use hard temac, so , should I add it manually? ( well 
this is funny, but the wizard does not use hard temac , is it true? )
> > > Is there any ready EDK projects for ML403, with TEMAC and PLB TEMAC 
modules already inserted and cofigured? (I don't want to use GSRD ) ( If 
yes would you please put the link here )
> > >
> > You can start from XAPP 902 from Xilinx, this design demonstrate TEMAC
> > use in loopback mode. Some memers from that community have tried from
> > that design as a starting point. I did not nknow if the succeed.
> > I would recommand to get EDK 8.2 SP2 and use the Wizard to build a new
> > design with TEMAC.
> >
> > > 2- Simply, Is there any driver available for linux 2.6 , for PLB 
TEMAC and Hard TEMAC modules? If yes , can you put the link here, so that I 
can download it?
> > >
> > For the kernel you can get it from Montavista source code site using 
GIT
> > to download it (linux-xilinx-26). This is 2.6.17.4 version (last week!)
> >
> > Then you will need to pacth the Kernel with TEMAC drivers (look for
> > TEMAC PAULUS MVISTA with google, or follow my messages in that mailing
> > list)
> > NOTE: If you use that TEMAC patch do not use SGDMA on your TEMAC, use
> > FIFO.
> >
> > > thanks
> > Regards
> >
> > Chris
> > > _________________________________________________________________
> > > Personalize your Live.com homepage with the news, weather, and photos 
you care about.
> > > http://www.live.com/getstarted.aspx?icid=T001MSN30A0701
> > > _______________________________________________
> > > Linuxppc-embedded mailing list
> > > Linuxppc-embedded@ozlabs.org
> > > https://ozlabs.org/mailman/listinfo/linuxppc-embedded
> >
>
>_________________________________________________________________
>Live Search: New search found
>http://get.live.com/search/overview
>_______________________________________________
>Linuxppc-embedded mailing list
>Linuxppc-embedded@ozlabs.org
>https://ozlabs.org/mailman/listinfo/linuxppc-embedded

_________________________________________________________________
ÓëÁª»úµÄÅóÓѽøÐн»Á÷£¬ÇëʹÓà MSN Messenger:  http://messenger.msn.com/cn  

_______________________________________________
Linuxppc-embedded mailing list
Linuxppc-embedded@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-embedded 


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

^ 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