| Message ID | 20181106042829.2771226-1-taoren@fb.com |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | [v2,1/2] ARM: dts: Add Facebook BMC flash layout | expand |
Hi Tao, On Tue, 6 Nov 2018, at 14:58, Tao Ren wrote: > This is the layout used by Facebook BMC systems. It describes the fixed > flash layout of a 32MB mtd device. > > Signed-off-by: Tao Ren <taoren@fb.com> > --- > Changes since v1: > - adding facebook copyright. > > .../boot/dts/facebook-bmc-flash-layout.dtsi | 33 +++++++++++++++++++ > 1 file changed, 33 insertions(+) > create mode 100644 arch/arm/boot/dts/facebook-bmc-flash-layout.dtsi > > diff --git a/arch/arm/boot/dts/facebook-bmc-flash-layout.dtsi b/arch/ > arm/boot/dts/facebook-bmc-flash-layout.dtsi > new file mode 100644 > index 000000000000..94039d7b7de5 > --- /dev/null > +++ b/arch/arm/boot/dts/facebook-bmc-flash-layout.dtsi > @@ -0,0 +1,33 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +// Copyright (c) 2018 Facebook Inc. > + > +partitions { > + compatible = "fixed-partitions"; > + #address-cells = <1>; > + #size-cells = <1>; > + > + u-boot@0 { > + reg = <0x0 0x60000>; > + label = "u-boot"; > + }; > + > + u-boot-env@60000 { > + reg = <0x60000 0x20000>; > + label = "env"; > + }; > + > + fit@80000 { > + reg = <0x80000 0x1b80000>; > + label = "fit"; > + }; > + > + data0@1c00000 { > + reg = <0x1c00000 0x400000>; > + label = "data0"; > + }; > + > + flash0@0 { > + reg = <0x0 0x2000000>; > + label = "flash0"; > + }; Is this necessary? Isn't the same thing achieved with the /dev/mtd0 device? Cheers, Andrew > +}; > -- > 2.17.1 >
On 11/7/18 4:02 PM, Andrew Jeffery wrote: >> +partitions { >> + compatible = "fixed-partitions"; >> + #address-cells = <1>; >> + #size-cells = <1>; >> + >> + u-boot@0 { >> + reg = <0x0 0x60000>; >> + label = "u-boot"; >> + }; >> + >> + u-boot-env@60000 { >> + reg = <0x60000 0x20000>; >> + label = "env"; >> + }; >> + >> + fit@80000 { >> + reg = <0x80000 0x1b80000>; >> + label = "fit"; >> + }; >> + >> + data0@1c00000 { >> + reg = <0x1c00000 0x400000>; >> + label = "data0"; >> + }; >> + >> + flash0@0 { >> + reg = <0x0 0x2000000>; >> + label = "flash0"; >> + }; > > Is this necessary? Isn't the same thing achieved with the /dev/mtd0 device? Hi Andrew, Thank you for the review! The new layout file is needed mainly because of "data0" partition: several facebook platforms use the partition as "persistent" storage. As for "flash0", technically it's not needed (as you pointed out, /dev/mtd0 covers the entire flash if master_partition is enabled). It's still here to avoid breaking some legacy applications. Thanks, Tao Ren
On Thu, 8 Nov 2018, at 10:48, Tao Ren wrote: > On 11/7/18 4:02 PM, Andrew Jeffery wrote: > >> +partitions { > >> + compatible = "fixed-partitions"; > >> + #address-cells = <1>; > >> + #size-cells = <1>; > >> + > >> + u-boot@0 { > >> + reg = <0x0 0x60000>; > >> + label = "u-boot"; > >> + }; > >> + > >> + u-boot-env@60000 { > >> + reg = <0x60000 0x20000>; > >> + label = "env"; > >> + }; > >> + > >> + fit@80000 { > >> + reg = <0x80000 0x1b80000>; > >> + label = "fit"; > >> + }; > >> + > >> + data0@1c00000 { > >> + reg = <0x1c00000 0x400000>; > >> + label = "data0"; > >> + }; > >> + > >> + flash0@0 { > >> + reg = <0x0 0x2000000>; > >> + label = "flash0"; > >> + }; > > > > Is this necessary? Isn't the same thing achieved with the /dev/mtd0 device? > > Hi Andrew, > > Thank you for the review! The new layout file is needed mainly because > of "data0" partition: several facebook platforms use the partition as > "persistent" storage. > > As for "flash0", technically it's not needed (as you pointed out, /dev/ > mtd0 covers the entire flash if master_partition is enabled). It's still > here to avoid breaking some legacy applications. This is what I expected. I think it might be worth adding a comment, given you are respinning the series to address my comments on the board devicetree patch. Anyway, thanks for the clarification. Andrew
On 11/7/18 5:45 PM, Andrew Jeffery wrote: >>> Is this necessary? Isn't the same thing achieved with the /dev/mtd0 device? >> >> Hi Andrew, >> >> Thank you for the review! The new layout file is needed mainly because >> of "data0" partition: several facebook platforms use the partition as >> "persistent" storage. >> >> As for "flash0", technically it's not needed (as you pointed out, /dev/ >> mtd0 covers the entire flash if master_partition is enabled). It's still >> here to avoid breaking some legacy applications. > > This is what I expected. I think it might be worth adding a comment, given > you are respinning the series to address my comments on the board > devicetree patch. > > Anyway, thanks for the clarification. > > Andrew Sure Andrew. I will add some comments and send out 2 updated patches together (most likely sometime tomorrow). Thanks, Tao Ren
diff --git a/arch/arm/boot/dts/facebook-bmc-flash-layout.dtsi b/arch/arm/boot/dts/facebook-bmc-flash-layout.dtsi new file mode 100644 index 000000000000..94039d7b7de5 --- /dev/null +++ b/arch/arm/boot/dts/facebook-bmc-flash-layout.dtsi @@ -0,0 +1,33 @@ +// SPDX-License-Identifier: GPL-2.0+ +// Copyright (c) 2018 Facebook Inc. + +partitions { + compatible = "fixed-partitions"; + #address-cells = <1>; + #size-cells = <1>; + + u-boot@0 { + reg = <0x0 0x60000>; + label = "u-boot"; + }; + + u-boot-env@60000 { + reg = <0x60000 0x20000>; + label = "env"; + }; + + fit@80000 { + reg = <0x80000 0x1b80000>; + label = "fit"; + }; + + data0@1c00000 { + reg = <0x1c00000 0x400000>; + label = "data0"; + }; + + flash0@0 { + reg = <0x0 0x2000000>; + label = "flash0"; + }; +};
This is the layout used by Facebook BMC systems. It describes the fixed flash layout of a 32MB mtd device. Signed-off-by: Tao Ren <taoren@fb.com> --- Changes since v1: - adding facebook copyright. .../boot/dts/facebook-bmc-flash-layout.dtsi | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 arch/arm/boot/dts/facebook-bmc-flash-layout.dtsi