Hi Bruce,
On Tue, 2008-08-19 at 14:27 -0700, Bruce Leonard wrote:
> +/*
> + * Inline function for determining the size of the MTD device, independant
> + * of old or new way of doing things.
> + *
> + */
> +static inline u_int64_t device_size(struct mtd_info *a)
> +{
> + return a->num_eraseblocks == 0 ? a->size : a->num_eraseblocks * a->erasesize;
> +}
I do not think it is a good idea to do multiplication every time we need
MTD device size. It is unnecessarily large overhead in terms of speed
and code size.
Did you consider a possibility of just making mtd->size 64 bit?
Or using eraseblock:offset pairs instead of absolute address?
--
Best regards,
Artem Bityutskiy (Битюцкий Артём)
On Aug 26, 2008, at 10:40 PM, Artem Bityutskiy wrote:
> Hi Bruce,
>
> On Tue, 2008-08-19 at 14:27 -0700, Bruce Leonard wrote:
>> +/*
>> + * Inline function for determining the size of the MTD device,
>> independant
>> + * of old or new way of doing things.
>> + *
>> + */
>> +static inline u_int64_t device_size(struct mtd_info *a)
>> +{
>> + return a->num_eraseblocks == 0 ? a->size : a->num_eraseblocks * a-
>> >erasesize;
>> +}
>
> I do not think it is a good idea to do multiplication every time we
> need
> MTD device size. It is unnecessarily large overhead in terms of speed
> and code size.
>
> Did you consider a possibility of just making mtd->size 64 bit?
I did consider making size 64-bit, but it seemed less intrusive to go
the direction I did. I wanted to change as little code as possible
but at the same time make it obvious there was a fundamental change.
There's also a desire to move more in the direction of a BIO-like
aspect to the MTD layer and some of the suggestions I got early made
it seem that this would make that future move easier.
>
> Or using eraseblock:offset pairs instead of absolute address?
I didn't really see how I could convey the idea of size using
eraseblock:offset.
Bruce
Artem,
> > +/*
> > + * Inline function for determining the size of the MTD device,
independant
> > + * of old or new way of doing things.
> > + *
> > + */
> > +static inline u_int64_t device_size(struct mtd_info *a)
> > +{
> > + return a->num_eraseblocks == 0 ? a->size : a->num_eraseblocks *
a->erasesize;
> > +}
>
> I do not think it is a good idea to do multiplication every time we need
> MTD device size. It is unnecessarily large overhead in terms of speed
> and code size.
>
I'm still reluctant to change size to a 64-bit value. There's a vague
recolection of early conversations on the list that there would be little
acceptance for that. And that probably has to do with the ongoing
conversation about ABI changes. What I could do to eliminate the
multiplication is introduce the same concept that the NAND layer uses,
shift values. After all, erasesize should always be a power of 2, making
that a power of 2 multiplication which can be done via shifts. By
changing erasesize to erasesize_shift, I'd get something like this:
return a->num_eraseblocks == 0 ? a->size : a->num_eraseblocks <<
a->erasesize_shift
How would that suit you?
Bruce
[email protected] wrote:
> I'm still reluctant to change size to a 64-bit value. There's a vague
> recolection of early conversations on the list that there would be little
> acceptance for that. And that probably has to do with the ongoing
> conversation about ABI changes. What I could do to eliminate the
> multiplication is introduce the same concept that the NAND layer uses,
> shift values. After all, erasesize should always be a power of 2, making
> that a power of 2 multiplication which can be done via shifts. By
> changing erasesize to erasesize_shift, I'd get something like this:
>
> return a->num_eraseblocks == 0 ? a->size : a->num_eraseblocks <<
> a->erasesize_shift
>
> How would that suit you?
Are you sure it's always going to be a power of 2?
What if someone targets a board with 3 chips wired to shared address
and parallel data buses?
Or if someone makes a weird chip? Or if you can format it in
different ways according to desired ECC level (like you can with CDs)?
If there's ongoing conversation about ABI changes, it sounds like it
would be good for this change to be done together that, instead of
doing this change then changing the ABI _again_ shortly after and
having to support 3 different ABIs in tools instead of 2.
-- Jamie
On 27.08.2008 20:51, Jamie Lokier wrote:
> [email protected] wrote:
>
>> I'm still reluctant to change size to a 64-bit value. There's a vague
>> recolection of early conversations on the list that there would be little
>> acceptance for that. And that probably has to do with the ongoing
>> conversation about ABI changes. What I could do to eliminate the
>> multiplication is introduce the same concept that the NAND layer uses,
>> shift values. After all, erasesize should always be a power of 2, making
>> that a power of 2 multiplication which can be done via shifts. By
>> changing erasesize to erasesize_shift, I'd get something like this:
>>
>> return a->num_eraseblocks == 0 ? a->size : a->num_eraseblocks <<
>> a->erasesize_shift
>>
>> How would that suit you?
>>
>
> Are you sure it's always going to be a power of 2?
>
> What if someone targets a board with 3 chips wired to shared address
> and parallel data buses?
>
> Or if someone makes a weird chip? Or if you can format it in
> different ways according to desired ECC level (like you can with CDs)?
>
IIRC I saw a datasheet for such a chip (selectable erasesize with
non-power-of-2 default) some weeks ago and it had entered production a
few months ago. The erasesize was alwas a multiple of 16, though. Sorry
for not remembering more details.
Regards,
Carl-Daniel
--
http://www.hailfinger.org/
On Wed, 27 Aug 2008, Carl-Daniel Hailfinger wrote:
> On 27.08.2008 20:51, Jamie Lokier wrote:
>> [email protected] wrote:
>>
>>> I'm still reluctant to change size to a 64-bit value. There's a vague
>>> recolection of early conversations on the list that there would be little
>>> acceptance for that. And that probably has to do with the ongoing
>>> conversation about ABI changes. What I could do to eliminate the
>>> multiplication is introduce the same concept that the NAND layer uses,
>>> shift values. After all, erasesize should always be a power of 2, making
>>> that a power of 2 multiplication which can be done via shifts. By
>>> changing erasesize to erasesize_shift, I'd get something like this:
>>>
>>> return a->num_eraseblocks == 0 ? a->size : a->num_eraseblocks <<
>>> a->erasesize_shift
>>>
>>> How would that suit you?
>>>
>>
>> Are you sure it's always going to be a power of 2?
>>
>> What if someone targets a board with 3 chips wired to shared address
>> and parallel data buses?
>>
>> Or if someone makes a weird chip? Or if you can format it in
>> different ways according to desired ECC level (like you can with CDs)?
>>
>
> IIRC I saw a datasheet for such a chip (selectable erasesize with
> non-power-of-2 default) some weeks ago and it had entered production a
> few months ago. The erasesize was alwas a multiple of 16, though. Sorry
> for not remembering more details.
It seems like the device size is always going to have some zeros in the least
significant bits. Maybe a 32-bit size plus a shift is enough? That could be
a lot more efficient than a 64-bit size and avoid penalizing most users who
don't need >32-bit size chips.
[email protected] wrote on 08/27/2008 02:52:24 PM:
> On 27.08.2008 20:51, Jamie Lokier wrote:
> > [email protected] wrote:
> >
> >> I'm still reluctant to change size to a 64-bit value. There's a
vague
> >> recolection of early conversations on the list that there would be
little
> >> acceptance for that. And that probably has to do with the ongoing
> >> conversation about ABI changes. What I could do to eliminate the
> >> multiplication is introduce the same concept that the NAND layer
uses,
> >> shift values. After all, erasesize should always be a power of 2,
making
> >> that a power of 2 multiplication which can be done via shifts. By
> >> changing erasesize to erasesize_shift, I'd get something like this:
> >>
> >> return a->num_eraseblocks == 0 ? a->size : a->num_eraseblocks <<
> >> a->erasesize_shift
> >>
> >> How would that suit you?
> >>
> >
> > Are you sure it's always going to be a power of 2?
> >
> > What if someone targets a board with 3 chips wired to shared address
> > and parallel data buses?
> >
> > Or if someone makes a weird chip? Or if you can format it in
> > different ways according to desired ECC level (like you can with CDs)?
> >
>
> IIRC I saw a datasheet for such a chip (selectable erasesize with
> non-power-of-2 default) some weeks ago and it had entered production a
> few months ago. The erasesize was alwas a multiple of 16, though. Sorry
> for not remembering more details.
>
> Regards,
> Carl-Daniel
>
Well in that case I don't see that there's much option other than a
multiplication. If there were an odd number of eraseblocks (i.e., 3
chips) you could still do the shifting operations. But if someone has
come up with a flash part that has a non-power of two sector/erasesize,
then there's no way to do it by shift. I supose I could just change
erasesize to size64, make it a 64-bit type and do this:
return a->num_eraseblocks == 0 ? a->size : a->size64
Doesn't seem quite as elegant, but it is simpler. What ever I do, I can't
change the meaning or type of size. That's an kernel <=> userspace ABI
change, and we know what happens when I try to do that ;).
Bruce
------------------------------------------------
This e-mail may contain SEL confidential information. The opinions
expressed are not necessarily those of SEL. Any unauthorized disclosure,
distribution or other use is prohibited. If you received this e-mail in
error, please notify the sender, permanently delete it, and destroy any
printout.
Thank you.
------------------------------------------------