MTD CVS update: 'mtd/kernel cfi_cmdset_0001.c cfi_probe.c nora.c octagon-5066.c vmax301.c'

MTD CVS update: 'mtd/kernel cfi_cmdset_0001.c cfi_probe.c nora.c octagon-5066.c vmax301.c'

[David Woodhouse]

Update of /home/cvs/mtd/kernel
In directory infradead.org:/tmp/cvs-serv30857/kernel

Modified Files:
	cfi_cmdset_0001.c cfi_probe.c nora.c octagon-5066.c vmax301.c 
Log Message:
Detect more than one chip in CFI code. Doesn't yet handle aliases - the
alias detection function does 'return 0' but that's not too difficult to
fix. Now we can happily do locking of the chip to avoid concurrent access,
and that means we can do asynchronous erases and stuff safely too,
hopefully tomorrow.



To unsubscribe, send "unsubscribe mtd" to majordomo@infradead.org

Re: MTD CVS update: 'mtd/kernel cfi_cmdset_0001.c cfi_probe.c nora.c octagon-5066.c vmax301.c'

[Jason Gunthorpe]

On Mon, 26 Jun 2000, David Woodhouse wrote:

> Update of /home/cvs/mtd/kernel
> Modified Files:
> 	cfi_cmdset_0001.c cfi_probe.c nora.c octagon-5066.c vmax301.c 

Could you also add bank_size? Both oct and vmax are 8bit devices so
your FIXME is wrong. Everything using mapped before this has to be 8 bit
because the JEDEC driver didn't support any higher.

Also, while you are changing things, can you widen the name feild in the
mtd_info to about 100 chars? I want to plunk detection info in there too.

Should probably also use the named initializers the rest of the kernel is
moving to for these big structs.

Thanks,
Jason



To unsubscribe, send "unsubscribe mtd" to majordomo@infradead.org

Re: MTD CVS update: 'mtd/kernel cfi_cmdset_0001.c cfi_probe.c nora.c octagon-5066.c vmax301.c'

[David Woodhouse]

jgg@deltatee.com said:
>  Could you also add bank_size? 

Do we really need it? I turned my previous question around - why does the
_map_ driver need to know about it? 

You've said, and I agree, that the map driver won't necessarily know the 
size of the chips within the area, so it can't know how the aliases are 
arranged and remap the space itself. 

So the flash probe code needs to work that out for itself, and as 
soon as the cfi_check_chipalias() function does something other than 
'return 0' that'll be fine. Presumably you have something similar.

Having worked out where each chip starts in the map, and recorded its start 
address in the per-chip data structure, why does the flash driver need to 
tell the map about it? Why not just use the 'correct' addresses?

> Both oct and vmax are 8bit devices so your FIXME is wrong. Everything
> using mapped before this has to be 8 bit because the JEDEC driver didn't
> support any higher. 

Fixed. Thanks.

>  Also, while you are changing things, can you widen the name feild in
> the mtd_info to about 100 chars? I want to plunk detection info in
> there too. 

Hmmm. How about a char * ?


jgg@deltatee.com said:
>  Should probably also use the named initializers the rest of the
> kernel is moving to for these big structs.

You mean a static initialiser that ends up in the data segment rather than 
kmalloc'd? What does it do for us?

I was aware of the netdev name changes - but that changed from a char * to 
a constant-length char[] to allow name changes to work without overflowing 
the original string.

--
dwmw2




To unsubscribe, send "unsubscribe mtd" to majordomo@infradead.org

Re: MTD CVS update: 'mtd/kernel cfi_cmdset_0001.c cfi_probe.c nora.c octagon-5066.c vmax301.c'

[Jason Gunthorpe]

On Tue, 27 Jun 2000, David Woodhouse wrote:

> You've said, and I agree, that the map driver won't necessarily know the 
> size of the chips within the area, so it can't know how the aliases are 
> arranged and remap the space itself. 

The map driver is the only driver that knows what the eletrical connection
is, if it knows the banks are 4 meg apart then it has to communicate that
up. The only thing this is used for is detection, the detector can jump
to the next bank start and do all of the banks. The only other option is
to probe every block int the mapping region looking for bank starts, which
is gross since the information is available.

> Hmmm. How about a char * ?

That might be best.
 
> jgg@deltatee.com said:
> >  Should probably also use the named initializers the rest of the
> > kernel is moving to for these big structs.
> 
> You mean a static initialiser that ends up in the data segment rather than 
> kmalloc'd? What does it do for us?

No, the C99 named member initializers for structs:

struct foo = {name: "Hello",size: 1024};

Makes those big initializers readable.

Jason




To unsubscribe, send "unsubscribe mtd" to majordomo@infradead.org

Re: MTD CVS update: 'mtd/kernel cfi_cmdset_0001.c cfi_probe.c nora.c octagon-5066.c vmax301.c'

[David Woodhouse]

jgg@ualberta.ca said:
>  The map driver is the only driver that knows what the eletrical
> connection is, if it knows the banks are 4 meg apart then it has to
> communicate that up. The only thing this is used for is detection, the
> detector can jump to the next bank start and do all of the banks. The
> only other option is to probe every block int the mapping region
> looking for bank starts, which is gross since the information is
> available.

Ah - you mean there are two sets of address lines which are unused - for 
example:

 A20-A19 : chip select
 A18-A17 : not even connected on the chip sockets.
 A16-A15 : present but the chips that are plugged in today don't use them
 A14-A0  : connected to the chip and in use

The map driver has no clue about the ones in the third category, but it 
definitely knows about the second, you're right.

I'm inclined to suggest that the map driver should map around the ones 
which are hardwired, and let the flash chip driver sort out the rest, 
having been given a hint as to the bank size.

Do we need to expand the map_info structure any further by including the
bank size, though? It's likely to be needed only at probe time, so shall we
pass it as an extra argument to the _probe functions instead?

> > Hmmm. How about a char * ?
> That might be best.

OK, I'll sort that out as soon as I've worked out why my cfi_cmdset_0001 code 
is oopsing. I need either NOR flash in a PC, or a ksymoops for ARMv4l.


>  No, the C99 named member initializers for structs:
> struct foo = {name: "Hello",size: 1024};
> Makes those big initializers readable. 

Oh right - yeah, that makes sense. Using the form you quote, which is in 
fact a GCC extension - C99 would be:

struct foo = { .name = "Hello", .size = 1024 };



--
dwmw2




To unsubscribe, send "unsubscribe mtd" to majordomo@infradead.org

Re: MTD CVS update: 'mtd/kernel cfi_cmdset_0001.c cfi_probe.c nora.c octagon-5066.c vmax301.c'

[Jason Gunthorpe]

On Wed, 28 Jun 2000, David Woodhouse wrote:

> >  The map driver is the only driver that knows what the eletrical
> > connection is, if it knows the banks are 4 meg apart then it has to
> > communicate that up. The only thing this is used for is detection, the
> > detector can jump to the next bank start and do all of the banks. The
> > only other option is to probe every block int the mapping region
> > looking for bank starts, which is gross since the information is
> > available.
> 
> Ah - you mean there are two sets of address lines which are unused - for 
> example:

No, not really..
 
>  A20-A19 : chip select
>  A18-A17 : not even connected on the chip sockets.
>  A16-A15 : present but the chips that are plugged in today don't use them
>  A14-A0  : connected to the chip and in use
 
> The map driver has no clue about the ones in the third category, but it 
> definitely knows about the second, you're right.

These are all just weird special cases of the same thing. If the flash is
banked in any way at all the mapped driver knows the bank size so it can
always say that 'every xxx bytes a new block of flash chips start' the
probe routine can use that to reliably locate all the flash chips.
 
> I'm inclined to suggest that the map driver should map around the ones 
> which are hardwired, and let the flash chip driver sort out the rest, 
> having been given a hint as to the bank size.

It doesn't matter at all, once you add support for any of the cases the
others you get for free.

> Do we need to expand the map_info structure any further by including the
> bank size, though? It's likely to be needed only at probe time, so shall we
> pass it as an extra argument to the _probe functions instead?

You need the bank_size to realign all IO requests from a linear flash
address to a memory address. The value needs to exist, but it could be
hidden in the private structure.
 
> >  No, the C99 named member initializers for structs:
> > struct foo = {name: "Hello",size: 1024};
> > Makes those big initializers readable. 
> 
> Oh right - yeah, that makes sense. Using the form you quote, which is in 
> fact a GCC extension - C99 would be:
> 
> struct foo = { .name = "Hello", .size = 1024 };

Hm.. the one I quoted is used all over the kernel, the C99 version
doesn't seem to be used at all. I thought they had settled on using C99
rather than the extension, oh well.

Jason



To unsubscribe, send "unsubscribe mtd" to majordomo@infradead.org

Changing mtd.name to a char *

[David Woodhouse]

jgg@ualberta.ca said:
> Hmmm. How about a char * ?
> That might be best.

Argh. The MEMGETINFO ioctl returns the first part of the struct mtd_info, 
which used to contain the actual name of the device, but now just holds a 
pointer to it.

For now, I've just shifted the name field out of the part which is available
to the user - if anyone cares, you'll have to code up a MEMGETNAME ioctl
which nicely and safely passes back the variable-length name. 

Of course, I'm still screwed because I can't currently compile armv4l 
binaries for my test board, so I can't update my utils to reflect that.

Could anyone compile the contents of the util/ directory for armv4l and 
mail them to dwmw2@redhat.com?


--
dwmw2




To unsubscribe, send "unsubscribe mtd" to majordomo@infradead.org