diff mbox series

[v2,1/2] ARM: dts: Add Facebook BMC flash layout

Message ID 20181106042829.2771226-1-taoren@fb.com (mailing list archive)
State New, archived
Headers show
Series [v2,1/2] ARM: dts: Add Facebook BMC flash layout | expand

Commit Message

Tao Ren Nov. 6, 2018, 4:28 a.m. UTC
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

Comments

Andrew Jeffery Nov. 8, 2018, 12:02 a.m. UTC | #1
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
>
Tao Ren Nov. 8, 2018, 12:18 a.m. UTC | #2
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
Andrew Jeffery Nov. 8, 2018, 1:45 a.m. UTC | #3
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
Tao Ren Nov. 8, 2018, 2:31 a.m. UTC | #4
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 mbox series

Patch

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";
+	};
+};