diff mbox series

[v5,4/5] dt-bindings: mtd: Add bindings for TI's AM654 HyperBus memory controller

Message ID 20190609103227.24875-5-vigneshr@ti.com
State Changes Requested
Delegated to: Vignesh R
Headers show
Series MTD: Add Initial Hyperbus support | expand

Commit Message

Raghavendra, Vignesh June 9, 2019, 10:32 a.m. UTC
Add binding documentation for TI's HyperBus memory controller present on
AM654 SoC.

Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
---
v5:
Update binding example to show MMIO mux
Fix reg property for flash slave.

 .../devicetree/bindings/mtd/ti,am654-hbmc.txt | 51 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 2 files changed, 52 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/ti,am654-hbmc.txt

Comments

Sergei Shtylyov June 10, 2019, 4:25 p.m. UTC | #1
Hello!

On 06/09/2019 01:32 PM, Vignesh Raghavendra wrote:

> Add binding documentation for TI's HyperBus memory controller present on
> AM654 SoC.
> 
> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
> ---
> v5:
> Update binding example to show MMIO mux
> Fix reg property for flash slave.
> 
>  .../devicetree/bindings/mtd/ti,am654-hbmc.txt | 51 +++++++++++++++++++
>  MAINTAINERS                                   |  1 +
>  2 files changed, 52 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mtd/ti,am654-hbmc.txt
> 
> diff --git a/Documentation/devicetree/bindings/mtd/ti,am654-hbmc.txt b/Documentation/devicetree/bindings/mtd/ti,am654-hbmc.txt
> new file mode 100644
> index 000000000000..c2a2c2b42a92
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/ti,am654-hbmc.txt
> @@ -0,0 +1,51 @@
> +Bindings for HyperBus Memory Controller (HBMC) on TI's K3 family of SoCs
> +
> +Required properties:
> +- compatible : "ti,am654-hbmc" for AM654 SoC
> +- reg : Two entries:
> +	First entry pointed to the register space of HBMC controller
> +	Second entry pointing to the memory map region dedicated for
> +	MMIO access to attached flash devices
> +- ranges : Address translation from offset within CS to allocated MMIO
> +	   space in SoC
> +
> +Optional properties:
> +- mux-controls : phandle to the multiplexer that controls selection of
> +		 HBMC vs OSPI inside Flash SubSystem. Default is OSPI,
> +		 if property is absent.
> +		 See Documentation/devicetree/bindings/mux/reg-mux.txt
> +		 for mmio-mux binding details
> +
> +Example:
> +
> +	fss: fss@47000000 {

   What's FSS?
   Regardless of the answer, the node names should be generic, like "memory-controller@".

> +		compatible = "syscon", "simple-mfd";

   If it's "sycon", the nme should probably be "system-controller".

> +		reg = <0x0 0x47000000 0x0 0x100>;
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		ranges;
> +
> +		hbmc_mux: hbmc-mux {

   So, just "multiplexor"?

> +			compatible = "mmio-mux";
> +			#mux-control-cells = <1>;
> +			mux-reg-masks = <0x4 0x2>; /* 0: reg 0x4, bit 1 */
> +		};
> +
> +		hbmc: hbmc@47034000 {

   Should be named "memory-controller@47034000", according to the DT spec.

> +			compatible = "ti,am654-hbmc";
> +			reg = <0x0 0x47034000 0x0 0x100>,
> +				<0x5 0x00000000 0x1 0x0000000>;
> +			power-domains = <&k3_pds 55>;
> +			#address-cells = <2>;
> +			#size-cells = <1>;
> +			ranges = <0x0 0x0 0x5 0x00000000 0x4000000>, /* CS0 - 64MB */
> +				 <0x1 0x0 0x5 0x04000000 0x4000000>; /* CS1 - 64MB */
> +			mux-controls = <&hbmc_mux 0>;
> +
> +			/* Slave flash node */
> +			flash@0,0 {
> +				compatible = "cypress,hyperflash", "cfi-flash";
> +				reg = <0x0 0x0 0x4000000>;
> +			};
> +		};
> +	};
[...]

MBR, Sergei
Raghavendra, Vignesh June 17, 2019, 4:30 a.m. UTC | #2
Hi,

On 10/06/19 9:55 PM, Sergei Shtylyov wrote:
> Hello!
> 
> On 06/09/2019 01:32 PM, Vignesh Raghavendra wrote:
> 
>> Add binding documentation for TI's HyperBus memory controller present on
>> AM654 SoC.
>>
>> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
>> ---
>> v5:
>> Update binding example to show MMIO mux
>> Fix reg property for flash slave.
>>
>>  .../devicetree/bindings/mtd/ti,am654-hbmc.txt | 51 +++++++++++++++++++
>>  MAINTAINERS                                   |  1 +
>>  2 files changed, 52 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/mtd/ti,am654-hbmc.txt
>>
>> diff --git a/Documentation/devicetree/bindings/mtd/ti,am654-hbmc.txt b/Documentation/devicetree/bindings/mtd/ti,am654-hbmc.txt
>> new file mode 100644
>> index 000000000000..c2a2c2b42a92
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mtd/ti,am654-hbmc.txt
>> @@ -0,0 +1,51 @@
>> +Bindings for HyperBus Memory Controller (HBMC) on TI's K3 family of SoCs
>> +
>> +Required properties:
>> +- compatible : "ti,am654-hbmc" for AM654 SoC
>> +- reg : Two entries:
>> +	First entry pointed to the register space of HBMC controller
>> +	Second entry pointing to the memory map region dedicated for
>> +	MMIO access to attached flash devices
>> +- ranges : Address translation from offset within CS to allocated MMIO
>> +	   space in SoC
>> +
>> +Optional properties:
>> +- mux-controls : phandle to the multiplexer that controls selection of
>> +		 HBMC vs OSPI inside Flash SubSystem. Default is OSPI,
>> +		 if property is absent.
>> +		 See Documentation/devicetree/bindings/mux/reg-mux.txt
>> +		 for mmio-mux binding details
>> +
>> +Example:
>> +
>> +	fss: fss@47000000 {
> 
>    What's FSS?

Flash SubSystem (FSS). Will highlight that in mux control definition above

>    Regardless of the answer, the node names should be generic, like "memory-controller@".
> 
>> +		compatible = "syscon", "simple-mfd";
> 
>    If it's "sycon", the nme should probably be "system-controller".

FSS is like a subchip that has all Flash controllers like Octal SPI
controllers and HyperBus controller within it. Some of their controls
are in the below address range. So I think its more closer to
"system-controller"

> 
>> +		reg = <0x0 0x47000000 0x0 0x100>;
>> +		#address-cells = <2>;
>> +		#size-cells = <2>;
>> +		ranges;
>> +
>> +		hbmc_mux: hbmc-mux {
> 
>    So, just "multiplexor"?

Ok

> 
>> +			compatible = "mmio-mux";
>> +			#mux-control-cells = <1>;
>> +			mux-reg-masks = <0x4 0x2>; /* 0: reg 0x4, bit 1 */
>> +		};
>> +
>> +		hbmc: hbmc@47034000 {
> 
>    Should be named "memory-controller@47034000", according to the DT spec.

IMO, since HyperBus is a bus protocol and has a specification, I think
its should have a separate generic name like SPI etc.

I will change this to "hyperbus@47034000" to common name of spec.

> 
>> +			compatible = "ti,am654-hbmc";
>> +			reg = <0x0 0x47034000 0x0 0x100>,
>> +				<0x5 0x00000000 0x1 0x0000000>;
>> +			power-domains = <&k3_pds 55>;
>> +			#address-cells = <2>;
>> +			#size-cells = <1>;
>> +			ranges = <0x0 0x0 0x5 0x00000000 0x4000000>, /* CS0 - 64MB */
>> +				 <0x1 0x0 0x5 0x04000000 0x4000000>; /* CS1 - 64MB */
>> +			mux-controls = <&hbmc_mux 0>;
>> +
>> +			/* Slave flash node */
>> +			flash@0,0 {
>> +				compatible = "cypress,hyperflash", "cfi-flash";
>> +				reg = <0x0 0x0 0x4000000>;
>> +			};
>> +		};
>> +	};
> [...]
> 
> MBR, Sergei
>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/mtd/ti,am654-hbmc.txt b/Documentation/devicetree/bindings/mtd/ti,am654-hbmc.txt
new file mode 100644
index 000000000000..c2a2c2b42a92
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/ti,am654-hbmc.txt
@@ -0,0 +1,51 @@ 
+Bindings for HyperBus Memory Controller (HBMC) on TI's K3 family of SoCs
+
+Required properties:
+- compatible : "ti,am654-hbmc" for AM654 SoC
+- reg : Two entries:
+	First entry pointed to the register space of HBMC controller
+	Second entry pointing to the memory map region dedicated for
+	MMIO access to attached flash devices
+- ranges : Address translation from offset within CS to allocated MMIO
+	   space in SoC
+
+Optional properties:
+- mux-controls : phandle to the multiplexer that controls selection of
+		 HBMC vs OSPI inside Flash SubSystem. Default is OSPI,
+		 if property is absent.
+		 See Documentation/devicetree/bindings/mux/reg-mux.txt
+		 for mmio-mux binding details
+
+Example:
+
+	fss: fss@47000000 {
+		compatible = "syscon", "simple-mfd";
+		reg = <0x0 0x47000000 0x0 0x100>;
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		hbmc_mux: hbmc-mux {
+			compatible = "mmio-mux";
+			#mux-control-cells = <1>;
+			mux-reg-masks = <0x4 0x2>; /* 0: reg 0x4, bit 1 */
+		};
+
+		hbmc: hbmc@47034000 {
+			compatible = "ti,am654-hbmc";
+			reg = <0x0 0x47034000 0x0 0x100>,
+				<0x5 0x00000000 0x1 0x0000000>;
+			power-domains = <&k3_pds 55>;
+			#address-cells = <2>;
+			#size-cells = <1>;
+			ranges = <0x0 0x0 0x5 0x00000000 0x4000000>, /* CS0 - 64MB */
+				 <0x1 0x0 0x5 0x04000000 0x4000000>; /* CS1 - 64MB */
+			mux-controls = <&hbmc_mux 0>;
+
+			/* Slave flash node */
+			flash@0,0 {
+				compatible = "cypress,hyperflash", "cfi-flash";
+				reg = <0x0 0x0 0x4000000>;
+			};
+		};
+	};
diff --git a/MAINTAINERS b/MAINTAINERS
index 258cf0d9eb8f..d6773a8e3ba2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7344,6 +7344,7 @@  S:	Supported
 F:	drivers/mtd/hyperbus/
 F:	include/linux/mtd/hyperbus.h
 F:	Documentation/devicetree/bindings/mtd/cypress,hyperflash.txt
+F:	Documentation/devicetree/bindings/mtd/ti,am654-hbmc.txt
 
 HYPERVISOR VIRTUAL CONSOLE DRIVER
 L:	linuxppc-dev@lists.ozlabs.org