physmap_of and partitions (mtd concat support)

physmap_of and partitions (mtd concat support)

[Stefan Roese]

I just noticed that physmap_of can't handle multiple devices of different type  
described in one device node. For example the Intel P30 48F4400 (64MByte) 
consists internally of 2 non-identical NOR chips. So a "simple" 

	flash@0,0 {
		#address-cells = <1>;
		#size-cells = <1>;
		compatible = "cfi-flash";
		reg = <0 0 0x04000000>;
		bank-width = <2>;
		device-width = <2>;
		partition@0 {
			label = "test-part";
			reg = <0 0x04000000>;
		};
	}

is not working. It will "detect" 2 identical 32MByte chips which is wrong. 
Adding a 2nd flash node solves this problem:

	flash@0,0 {
		#address-cells = <1>;
		#size-cells = <1>;
		compatible = "cfi-flash";
		reg = <0 0 0x02000000>;
		bank-width = <2>;
		device-width = <2>;
		partition@0 {
			label = "test-part1";
			reg = <0 0x02000000>;
		};
	}
	flash@1,0 {
		#address-cells = <1>;
		#size-cells = <1>;
		compatible = "cfi-flash";
		reg = <0 0x02000000 0x02000000>;
		bank-width = <2>;
		device-width = <2>;
		partition@0 {
			label = "test-part2";
			reg = <0 0x02000000>;
		};
	}

This way both chips are detected correctly.

Now the real problem: How should I describe a partition in the device tree 
spanning over both devices (concat)?. The current physmap_of driver doesn't 
handle concat at all (physmap.c does). I already have some ideas on how to 
implement this concat support in physmap_of. But ideas about a device-tree 
syntax for such partitions are very welcome.

Thanks.

Best regards,
Stefan

Re: physmap_of and partitions (mtd concat support)

[Grant Likely]

On Mon, Mar 23, 2009 at 4:51 AM, Stefan Roese <sr@denx.de> wrote:
> I just noticed that physmap_of can't handle multiple devices of different type
> described in one device node. For example the Intel P30 48F4400 (64MByte)
> consists internally of 2 non-identical NOR chips. So a "simple"
[...]
> Now the real problem: How should I describe a partition in the device tree
> spanning over both devices (concat)?. The current physmap_of driver doesn't
> handle concat at all (physmap.c does). I already have some ideas on how to
> implement this concat support in physmap_of. But ideas about a device-tree
> syntax for such partitions are very welcome.

Sounds to me like a physmap_of driver bug.  I don't think there is any
advantage in changing the partition syntax since concatenated flash
will always be used as a single device.  It doesn't make any sense to
try and span partitions over two nodes.

Do additional properties need to be added to describe the concat layout?

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: physmap_of and partitions (mtd concat support)

[Stefan Roese]

On Monday 23 March 2009, Grant Likely wrote:
> On Mon, Mar 23, 2009 at 4:51 AM, Stefan Roese <sr@denx.de> wrote:
> > I just noticed that physmap_of can't handle multiple devices of different
> > type described in one device node. For example the Intel P30 48F4400
> > (64MByte) consists internally of 2 non-identical NOR chips. So a "simple"
>
> [...]
>
> > Now the real problem: How should I describe a partition in the device
> > tree spanning over both devices (concat)?. The current physmap_of driver
> > doesn't handle concat at all (physmap.c does). I already have some ideas
> > on how to implement this concat support in physmap_of. But ideas about a
> > device-tree syntax for such partitions are very welcome.
>
> Sounds to me like a physmap_of driver bug.  I don't think there is any
> advantage in changing the partition syntax since concatenated flash
> will always be used as a single device.  It doesn't make any sense to
> try and span partitions over two nodes.

Yes, I would really love to make this possible with only one flash node. But 
just think about the following system configuration:

One Intel Strataflash (compatible = "cfi-flash") and one non-cfi compatible 
flash (e.g. compatible = "jedec-flash"). And the user wants to define a 
partition that spans over both flash chips. How could this be described in 
one flash node?

> Do additional properties need to be added to describe the concat layout?

Not sure. If we have multiple identical devices they can currently be 
described in one flash node. So with some changes to the physmap_of driver 
this configuration will work with concat as well. But more complex is a 
system configuration as described above. Meaning two or more non-identical 
chips. I don't see how this could be described in a sane way in one flash 
node.

Best regards,
Stefan

Re: physmap_of and partitions (mtd concat support)

[Grant Likely]

On Tue, Mar 24, 2009 at 3:07 AM, Stefan Roese <sr@denx.de> wrote:
> On Monday 23 March 2009, Grant Likely wrote:
>> On Mon, Mar 23, 2009 at 4:51 AM, Stefan Roese <sr@denx.de> wrote:
>> > I just noticed that physmap_of can't handle multiple devices of differ=
ent
>> > type described in one device node. For example the Intel P30 48F4400
>> > (64MByte) consists internally of 2 non-identical NOR chips. So a "simp=
le"
>>
>> [...]
>>
>> > Now the real problem: How should I describe a partition in the device
>> > tree spanning over both devices (concat)?. The current physmap_of driv=
er
>> > doesn't handle concat at all (physmap.c does). I already have some ide=
as
>> > on how to implement this concat support in physmap_of. But ideas about=
 a
>> > device-tree syntax for such partitions are very welcome.
>>
>> Sounds to me like a physmap_of driver bug. =A0I don't think there is any
>> advantage in changing the partition syntax since concatenated flash
>> will always be used as a single device. =A0It doesn't make any sense to
>> try and span partitions over two nodes.
>
> Yes, I would really love to make this possible with only one flash node. =
But
> just think about the following system configuration:
>
> One Intel Strataflash (compatible =3D "cfi-flash") and one non-cfi compat=
ible
> flash (e.g. compatible =3D "jedec-flash"). And the user wants to define a
> partition that spans over both flash chips. How could this be described i=
n
> one flash node?
>
>> Do additional properties need to be added to describe the concat layout?
>
> Not sure. If we have multiple identical devices they can currently be
> described in one flash node. So with some changes to the physmap_of drive=
r
> this configuration will work with concat as well. But more complex is a
> system configuration as described above. Meaning two or more non-identica=
l
> chips. I don't see how this could be described in a sane way in one flash
> node.

Are there any such platforms?  Is there much likelihood that such a
platform will be created?  Would it even be a good idea to span
partitions across such an arrangement given that different devices
will behave differently?

I think just leave that arrangement as hypothetical until the
situation actually occurs.  If it does occur, then strongly recommend
to not span a partition across the boundary.  If someone really
insists on doing this then we can create a new binding for the
purpose; but leave the old binding as is.  Maybe something like:

mtd {
        #address-cells =3D <1>;
        #size-cells =3D <1>;
        compatibly =3D "weird-mtd-concat";
        devices =3D <&mtd1 &mtd2 &mtd3>;
        partition1@0 {
                reg =3D <0 0x100000>;
        };
        partition2@100000 {
                reg =3D <0x100000 0x100000>;
        };
}

Where mtd1, 2 & 3 point to real flash nodes.  That way the
concatenated MTD devices could be anything NAND, NOR, SRAM, whatever
and it doesn't have to try and overload the existing device bindings.

g.


--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: physmap_of and partitions (mtd concat support)

[Stefan Roese]

On Tuesday 24 March 2009, Grant Likely wrote:
> >> Sounds to me like a physmap_of driver bug. =A0I don't think there is a=
ny
> >> advantage in changing the partition syntax since concatenated flash
> >> will always be used as a single device. =A0It doesn't make any sense to
> >> try and span partitions over two nodes.
> >
> > Yes, I would really love to make this possible with only one flash node.
> > But just think about the following system configuration:
> >
> > One Intel Strataflash (compatible =3D "cfi-flash") and one non-cfi
> > compatible flash (e.g. compatible =3D "jedec-flash"). And the user want=
s to
> > define a partition that spans over both flash chips. How could this be
> > described in one flash node?
> >
> >> Do additional properties need to be added to describe the concat layou=
t?
> >
> > Not sure. If we have multiple identical devices they can currently be
> > described in one flash node. So with some changes to the physmap_of
> > driver this configuration will work with concat as well. But more compl=
ex
> > is a system configuration as described above. Meaning two or more
> > non-identical chips. I don't see how this could be described in a sane
> > way in one flash node.
>
> Are there any such platforms?

Yes, I know some. Even though they are currently not used with a partition=
=20
spanning over those multiple chips (jedec and cfi).

> Is there much likelihood that such a=20
> platform will be created?  Would it even be a good idea to span
> partitions across such an arrangement given that different devices
> will behave differently?

OK, in the example above such a spanning partition is not so likely. But th=
ink=20
about my original example, the Intel P30 with two different cfi compatible=
=20
chips on one die. Here a partition spanning over both devices is very likel=
y.

As a sidenote: All this (concat over different chips) is possible with the=
=20
physmap.c mapping driver which was used on most of my platforms in the "old=
"=20
arch/ppc days.

> I think just leave that arrangement as hypothetical until the
> situation actually occurs.  If it does occur, then strongly recommend
> to not span a partition across the boundary.  If someone really
> insists on doing this then we can create a new binding for the
> purpose; but leave the old binding as is.  Maybe something like:
>
> mtd {
>         #address-cells =3D <1>;
>         #size-cells =3D <1>;
>         compatibly =3D "weird-mtd-concat";
>         devices =3D <&mtd1 &mtd2 &mtd3>;
>         partition1@0 {
>                 reg =3D <0 0x100000>;
>         };
>         partition2@100000 {
>                 reg =3D <0x100000 0x100000>;
>         };
> }
>
> Where mtd1, 2 & 3 point to real flash nodes.  That way the
> concatenated MTD devices could be anything NAND, NOR, SRAM, whatever
> and it doesn't have to try and overload the existing device bindings.

I think I like this idea. If nobody objects or has a better idea then I cou=
ld=20
start implementing it this way in a while.

Thanks.

Best regards,
Stefan

Re: physmap_of and partitions (mtd concat support)

[Grant Likely]

On Tue, Mar 24, 2009 at 9:39 AM, Stefan Roese <sr@denx.de> wrote:
> On Tuesday 24 March 2009, Grant Likely wrote:
>> >> Sounds to me like a physmap_of driver bug. =A0I don't think there is =
any
>> >> advantage in changing the partition syntax since concatenated flash
>> >> will always be used as a single device. =A0It doesn't make any sense =
to
>> >> try and span partitions over two nodes.
>> >
>> > Yes, I would really love to make this possible with only one flash nod=
e.
>> > But just think about the following system configuration:
>> >
>> > One Intel Strataflash (compatible =3D "cfi-flash") and one non-cfi
>> > compatible flash (e.g. compatible =3D "jedec-flash"). And the user wan=
ts to
>> > define a partition that spans over both flash chips. How could this be
>> > described in one flash node?
>> >
>> >> Do additional properties need to be added to describe the concat layo=
ut?
>> >
>> > Not sure. If we have multiple identical devices they can currently be
>> > described in one flash node. So with some changes to the physmap_of
>> > driver this configuration will work with concat as well. But more comp=
lex
>> > is a system configuration as described above. Meaning two or more
>> > non-identical chips. I don't see how this could be described in a sane
>> > way in one flash node.
>>
>> Are there any such platforms?
>
> Yes, I know some. Even though they are currently not used with a partitio=
n
> spanning over those multiple chips (jedec and cfi).

OK

>> Is there much likelihood that such a
>> platform will be created? =A0Would it even be a good idea to span
>> partitions across such an arrangement given that different devices
>> will behave differently?
>
> OK, in the example above such a spanning partition is not so likely. But =
think
> about my original example, the Intel P30 with two different cfi compatibl=
e
> chips on one die. Here a partition spanning over both devices is very lik=
ely.

I agree.  Same thing when two or more flash chips are put on a board
in consecutive addresses.  I've worked on plenty of these arrangements
myself.

This case really does sound like a driver bug and that the existing
cfi-flash binding is sufficient to describe the hardware.  IIUC, when
all the flash chips are of the same type the physmap_of driver should
be smart enough to detect each of the flash chips within the reg
range.

If I'm wrong and it cannot do this, then it would be a simple matter
of adding an additional tuple to reg for each discrete chip.  A
simple, backwards compatible extension which doesn't require a new
binding.

g.

--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: physmap_of and partitions (mtd concat support)

[Stefan Roese]

On Tuesday 24 March 2009, Grant Likely wrote:
> > OK, in the example above such a spanning partition is not so likely. But
> > think about my original example, the Intel P30 with two different cfi
> > compatible chips on one die. Here a partition spanning over both devices
> > is very likely.
>
> I agree.  Same thing when two or more flash chips are put on a board
> in consecutive addresses.  I've worked on plenty of these arrangements
> myself.

Yes, multiple identical devices are no problem at all. This is handled 
correctly with the current code and device tree syntax.

> This case really does sound like a driver bug and that the existing
> cfi-flash binding is sufficient to describe the hardware.  IIUC, when
> all the flash chips are of the same type the physmap_of driver should
> be smart enough to detect each of the flash chips within the reg
> range.

*When* all are identical then this works, yes. But in the Intel P30 case the 2 
chips are not identical. And from my understanding this is not a problem/bug 
in the physmap_of driver.

> If I'm wrong and it cannot do this, then it would be a simple matter
> of adding an additional tuple to reg for each discrete chip.  A
> simple, backwards compatible extension which doesn't require a new
> binding.

So you are thinking of something like this?

	flash@f0000000,0 {
		#address-cells = <1>;
		#size-cells = <1>;
		compatible = "cfi-flash";
		reg = <0 0x00000000 0x02000000
		       0 0x02000000 0x02000000>;
                bank-width = <2>;
                device-width = <2>;
                partition@0 {
                        label = "test-part1";
                        reg = <0 0x04000000>;
                };
	};

That's also fine with me. Changes to physmap_of would be minimal this way.

Do we have a consensus that this should be the way to implement concat support 
in physmap_of?

Thanks.

Best regards,
Stefan

Re: physmap_of and partitions (mtd concat support)

[Grant Likely]

On Wed, Mar 25, 2009 at 3:35 AM, Stefan Roese <sr@denx.de> wrote:
> On Tuesday 24 March 2009, Grant Likely wrote:
>> > OK, in the example above such a spanning partition is not so likely. B=
ut
>> > think about my original example, the Intel P30 with two different cfi
>> > compatible chips on one die. Here a partition spanning over both devic=
es
>> > is very likely.
>>
>> I agree. =A0Same thing when two or more flash chips are put on a board
>> in consecutive addresses. =A0I've worked on plenty of these arrangements
>> myself.
>
> Yes, multiple identical devices are no problem at all. This is handled
> correctly with the current code and device tree syntax.
>
>> This case really does sound like a driver bug and that the existing
>> cfi-flash binding is sufficient to describe the hardware. =A0IIUC, when
>> all the flash chips are of the same type the physmap_of driver should
>> be smart enough to detect each of the flash chips within the reg
>> range.
>
> *When* all are identical then this works, yes. But in the Intel P30 case =
the 2
> chips are not identical. And from my understanding this is not a problem/=
bug
> in the physmap_of driver.

To satisfy my own curiosity, why is physmap_of unable to probe
multiple cfi chips that are non identical?  After detecting the first
chip it and calculated then end address of it can it not do another
full cfi probe for the rest of the reg range?

Regardless, even if it could the solution you have below is probably a
better idea anyway than relying on probing.

>> If I'm wrong and it cannot do this, then it would be a simple matter
>> of adding an additional tuple to reg for each discrete chip. =A0A
>> simple, backwards compatible extension which doesn't require a new
>> binding.
>
> So you are thinking of something like this?
>
> =A0 =A0 =A0 =A0flash@f0000000,0 {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0#address-cells =3D <1>;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0#size-cells =3D <1>;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0compatible =3D "cfi-flash";
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0reg =3D <0 0x00000000 0x02000000
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 0 0x02000000 0x02000000>;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0bank-width =3D <2>;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0device-width =3D <2>;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0partition@0 {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0label =3D "test-part1";
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0reg =3D <0 0x04000000>;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0};
> =A0 =A0 =A0 =A0};

Yes, this looks good to me.  In fact, it is probably better to be
explicit about multiple chips anyway in terms of providing the driver
as much information as possible about where to probe for the chips.
It also elegantly supports sparsely mapped flash chips (ie. if the
board supports larger chips than is actually populated).

g.

--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: physmap_of and partitions (mtd concat support)

[Stefan Roese]

On Wednesday 25 March 2009, Grant Likely wrote:
> >> This case really does sound like a driver bug and that the existing
> >> cfi-flash binding is sufficient to describe the hardware. =A0IIUC, when
> >> all the flash chips are of the same type the physmap_of driver should
> >> be smart enough to detect each of the flash chips within the reg
> >> range.
> >
> > *When* all are identical then this works, yes. But in the Intel P30 case
> > the 2 chips are not identical. And from my understanding this is not a
> > problem/bug in the physmap_of driver.
>
> To satisfy my own curiosity, why is physmap_of unable to probe
> multiple cfi chips that are non identical?  After detecting the first
> chip it and calculated then end address of it can it not do another
> full cfi probe for the rest of the reg range?

The "real" probing is done in the mtd core (cfi_probe). So it's nothing tha=
t=20
could be changed in physmap_of. I have to admit that I didn't fully debug i=
t.

> Regardless, even if it could the solution you have below is probably a
> better idea anyway than relying on probing.
>
> >> If I'm wrong and it cannot do this, then it would be a simple matter
> >> of adding an additional tuple to reg for each discrete chip. =A0A
> >> simple, backwards compatible extension which doesn't require a new
> >> binding.
> >
> > So you are thinking of something like this?
> >
> > =A0 =A0 =A0 =A0flash@f0000000,0 {
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0#address-cells =3D <1>;
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0#size-cells =3D <1>;
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0compatible =3D "cfi-flash";
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0reg =3D <0 0x00000000 0x02000000
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 0 0x02000000 0x02000000>;
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0bank-width =3D <2>;
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0device-width =3D <2>;
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0partition@0 {
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0label =3D "test-part1";
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0reg =3D <0 0x04000000>;
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0};
> > =A0 =A0 =A0 =A0};
>
> Yes, this looks good to me.  In fact, it is probably better to be
> explicit about multiple chips anyway in terms of providing the driver
> as much information as possible about where to probe for the chips.
> It also elegantly supports sparsely mapped flash chips (ie. if the
> board supports larger chips than is actually populated).

OK, then we have a solution. Grant, thanks for your input here.

Best regards,
Stefan