[RFC,4/5] ARM: dts: DRA7: Add memory map region entries for qspi
diff mbox

Message ID 1438072876-16338-5-git-send-email-vigneshr@ti.com
State New
Headers show

Commit Message

Vignesh Raghavendra July 28, 2015, 8:41 a.m. UTC
Add qspi memory mapped region entries for DRA7xx based SoCs.

Signed-off-by: Vignesh R <vigneshr@ti.com>
---
 arch/arm/boot/dts/am4372.dtsi | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Sekhar Nori July 31, 2015, 1:48 p.m. UTC | #1
On Tuesday 28 July 2015 02:11 PM, Vignesh R wrote:
> Add qspi memory mapped region entries for DRA7xx based SoCs.
> 
> Signed-off-by: Vignesh R <vigneshr@ti.com>
> ---
>  arch/arm/boot/dts/am4372.dtsi | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi
> index ade28c790f4b..5317a0f24ab9 100644
> --- a/arch/arm/boot/dts/am4372.dtsi
> +++ b/arch/arm/boot/dts/am4372.dtsi

The patch talks about DRA7x in subject and description but you modify
AM437x here. You have got commit text mixed up between 4/5 and 5/5.

Probably not the kind of feedback you are looking for an RFC, but since
I noticed it..

Thanks,
Sekhar

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Brown July 31, 2015, 6:19 p.m. UTC | #2
On Tue, Jul 28, 2015 at 02:11:15PM +0530, Vignesh R wrote:
> Add qspi memory mapped region entries for DRA7xx based SoCs.
> 
> Signed-off-by: Vignesh R <vigneshr@ti.com>

>  		qspi: qspi@47900000 {
>  			compatible = "ti,am4372-qspi";
> -			reg = <0x47900000 0x100>;
> +			reg = <0x47900000 0x100>,
> +			      <0x30000000 0x3ffffff>;
> +			reg-names = "qspi_base", "qspi_mmap";

The DT binding document for the controller needs to be extended to
document this change in the binding.
Brian Norris July 31, 2015, 9:28 p.m. UTC | #3
On Tue, Jul 28, 2015 at 02:11:15PM +0530, Vignesh R wrote:
> Add qspi memory mapped region entries for DRA7xx based SoCs.
> 
> Signed-off-by: Vignesh R <vigneshr@ti.com>
> ---
>  arch/arm/boot/dts/am4372.dtsi | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi
> index ade28c790f4b..5317a0f24ab9 100644
> --- a/arch/arm/boot/dts/am4372.dtsi
> +++ b/arch/arm/boot/dts/am4372.dtsi
> @@ -902,7 +902,9 @@
>  
>  		qspi: qspi@47900000 {
>  			compatible = "ti,am4372-qspi";
> -			reg = <0x47900000 0x100>;
> +			reg = <0x47900000 0x100>,
> +			      <0x30000000 0x3ffffff>;

Are you sure this region is 1 byte shy of 64MB in length? Same question
for patch 5.

> +			reg-names = "qspi_base", "qspi_mmap";
>  			#address-cells = <1>;
>  			#size-cells = <0>;
>  			ti,hwmods = "qspi";
> -- 
> 2.4.6
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vignesh Raghavendra Aug. 3, 2015, 5:02 a.m. UTC | #4
On 07/31/2015 11:49 PM, Mark Brown wrote:
> On Tue, Jul 28, 2015 at 02:11:15PM +0530, Vignesh R wrote:
>> Add qspi memory mapped region entries for DRA7xx based SoCs.
>>
>> Signed-off-by: Vignesh R <vigneshr@ti.com>
> 
>>  		qspi: qspi@47900000 {
>>  			compatible = "ti,am4372-qspi";
>> -			reg = <0x47900000 0x100>;
>> +			reg = <0x47900000 0x100>,
>> +			      <0x30000000 0x3ffffff>;
>> +			reg-names = "qspi_base", "qspi_mmap";
> 
> The DT binding document for the controller needs to be extended to
> document this change in the binding.
> 

DT bindings are already documented at
Documentation/devicetree/bindings/spi/ti_qspi.txt. Did you mean to add
this node as an example in that file?
Vignesh Raghavendra Aug. 3, 2015, 5:06 a.m. UTC | #5
On 08/01/2015 02:58 AM, Brian Norris wrote:
> On Tue, Jul 28, 2015 at 02:11:15PM +0530, Vignesh R wrote:
>> Add qspi memory mapped region entries for DRA7xx based SoCs.
>>
>> Signed-off-by: Vignesh R <vigneshr@ti.com>
>> ---
>>  arch/arm/boot/dts/am4372.dtsi | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi
>> index ade28c790f4b..5317a0f24ab9 100644
>> --- a/arch/arm/boot/dts/am4372.dtsi
>> +++ b/arch/arm/boot/dts/am4372.dtsi
>> @@ -902,7 +902,9 @@
>>  
>>  		qspi: qspi@47900000 {
>>  			compatible = "ti,am4372-qspi";
>> -			reg = <0x47900000 0x100>;
>> +			reg = <0x47900000 0x100>,
>> +			      <0x30000000 0x3ffffff>;
> 
> Are you sure this region is 1 byte shy of 64MB in length? Same question
> for patch 5.
> 

Oops, my bad... Its 64MB in length, I entered offset of last byte
instead of length. Will correct this in the actual patch.

>> +			reg-names = "qspi_base", "qspi_mmap";
>>  			#address-cells = <1>;
>>  			#size-cells = <0>;
>>  			ti,hwmods = "qspi";
>> -- 
>> 2.4.6
>>
Vignesh Raghavendra Aug. 3, 2015, 5:09 a.m. UTC | #6
On 07/31/2015 07:18 PM, Sekhar Nori wrote:
> On Tuesday 28 July 2015 02:11 PM, Vignesh R wrote:
>> Add qspi memory mapped region entries for DRA7xx based SoCs.
>>
>> Signed-off-by: Vignesh R <vigneshr@ti.com>
>> ---
>>  arch/arm/boot/dts/am4372.dtsi | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi
>> index ade28c790f4b..5317a0f24ab9 100644
>> --- a/arch/arm/boot/dts/am4372.dtsi
>> +++ b/arch/arm/boot/dts/am4372.dtsi
> 
> The patch talks about DRA7x in subject and description but you modify
> AM437x here. You have got commit text mixed up between 4/5 and 5/5.
> 
> Probably not the kind of feedback you are looking for an RFC, but since
> I noticed it..

Oh, my bad.. I will correct $subject when I submit actual patch series.
Mark Brown Aug. 4, 2015, 3:52 p.m. UTC | #7
On Mon, Aug 03, 2015 at 10:32:09AM +0530, Vignesh R wrote:
> On 07/31/2015 11:49 PM, Mark Brown wrote:

> >> -			reg = <0x47900000 0x100>;
> >> +			reg = <0x47900000 0x100>,
> >> +			      <0x30000000 0x3ffffff>;
> >> +			reg-names = "qspi_base", "qspi_mmap";

> > The DT binding document for the controller needs to be extended to
> > document this change in the binding.

> DT bindings are already documented at
> Documentation/devicetree/bindings/spi/ti_qspi.txt. Did you mean to add
> this node as an example in that file?

No, I mean you're changing the binding to add this new memory region -
that new region needs to be added to the documentation.

Patch
diff mbox

diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi
index ade28c790f4b..5317a0f24ab9 100644
--- a/arch/arm/boot/dts/am4372.dtsi
+++ b/arch/arm/boot/dts/am4372.dtsi
@@ -902,7 +902,9 @@ 
 
 		qspi: qspi@47900000 {
 			compatible = "ti,am4372-qspi";
-			reg = <0x47900000 0x100>;
+			reg = <0x47900000 0x100>,
+			      <0x30000000 0x3ffffff>;
+			reg-names = "qspi_base", "qspi_mmap";
 			#address-cells = <1>;
 			#size-cells = <0>;
 			ti,hwmods = "qspi";