[v4,4/5] ARM: dts: DRA7: add entry for qspi mmap region

Commit Message

Vignesh Raghavendra Nov. 30, 2015, 5:15 a.m. UTC

Add qspi memory mapped region entries for DRA7xx based SoCs. Also,
update the binding documents for the controller to document this change.

Acked-by: Rob Herring <[e-mail]>
Signed-off-by: Vignesh R <[e-mail]>
---

v4: No changes.

 Documentation/devicetree/bindings/spi/ti_qspi.txt | 14 ++++++++++++++
 arch/arm/boot/dts/dra7.dtsi                       |  7 +++++--
 2 files changed, 19 insertions(+), 2 deletions(-)

Comments

Tony Lindgren Nov. 30, 2015, 10:01 p.m. UTC | #1

* Vignesh R <[e-mail]> [151129 21:16]:
> Add qspi memory mapped region entries for DRA7xx based SoCs. Also,
> update the binding documents for the controller to document this change.
> 
> Acked-by: Rob Herring <[e-mail]>
> Signed-off-by: Vignesh R <[e-mail]>
> ---
> 
> v4: No changes.

OK I'll apply patches 4 and 5 of this series into omap-for-v4.5/dt thanks.
The rest should get merged via the MTD tree.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Tony Lindgren Nov. 30, 2015, 10:34 p.m. UTC | #2

* Vignesh R <[e-mail]> [151129 21:16]:
> Add qspi memory mapped region entries for DRA7xx based SoCs. Also,
> update the binding documents for the controller to document this change.
> 
> Acked-by: Rob Herring <[e-mail]>
> Signed-off-by: Vignesh R <[e-mail]>
...
> --- a/Documentation/devicetree/bindings/spi/ti_qspi.txt
> +++ b/Documentation/devicetree/bindings/spi/ti_qspi.txt
> @@ -26,3 +26,17 @@ qspi: qspi@4b300000 {
>  	spi-max-frequency = <25000000>;
>  	ti,hwmods = "qspi";
>  };
> +
> +For dra7xx:
> +qspi: qspi@4b300000 {
> +	compatible = "ti,dra7xxx-qspi";
> +	reg = <0x4b300000 0x100>,
> +	      <0x5c000000 0x4000000>,
> +	      <0x4a002558 0x4>;
> +	reg-names = "qspi_base", "qspi_mmap",
> +		    "qspi_ctrlmod";
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	spi-max-frequency = <48000000>;
> +	ti,hwmods = "qspi";
> +};

Actually none of the IO areas above are within the same interconnect target:

0x4b300000  QSPI0 address space in L3 main interconnect
0x5c000000  QSPI1 address space in L3 main interconnect
0x4a002558  CTRL_CORE_CONTROL_IO_2 in System Control Module (SCM) in L4_CFG

The first two address spaces should be two separate instances of this driver.
The CTRL_CORE_CONTROL_IO_2 needs seems like a shared clock register that
needs to be accessed using the clock framework most likely.

So not applying, this will be impossible to move to some real interconnect
driver until these issues are fixed.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Tony Lindgren Nov. 30, 2015, 10:34 p.m. UTC | #3

* Tony Lindgren <[e-mail]> [151130 14:03]:
> * Vignesh R <[e-mail]> [151129 21:16]:
> > Add qspi memory mapped region entries for DRA7xx based SoCs. Also,
> > update the binding documents for the controller to document this change.
> > 
> > Acked-by: Rob Herring <[e-mail]>
> > Signed-off-by: Vignesh R <[e-mail]>
> > ---
> > 
> > v4: No changes.
> 
> OK I'll apply patches 4 and 5 of this series into omap-for-v4.5/dt thanks.
> The rest should get merged via the MTD tree.

Actually ddropping them after looking at the IO ranges like I commented.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Vignesh Raghavendra Dec. 1, 2015, 4:45 a.m. UTC | #4

On 12/01/2015 04:04 AM, Tony Lindgren wrote:
> * Vignesh R <[e-mail]> [151129 21:16]:
>> Add qspi memory mapped region entries for DRA7xx based SoCs. Also,
>> update the binding documents for the controller to document this change.
>>
>> Acked-by: Rob Herring <[e-mail]>
>> Signed-off-by: Vignesh R <[e-mail]>
> ...
>> --- a/Documentation/devicetree/bindings/spi/ti_qspi.txt
>> +++ b/Documentation/devicetree/bindings/spi/ti_qspi.txt
>> @@ -26,3 +26,17 @@ qspi: qspi@4b300000 {
>>  	spi-max-frequency = <25000000>;
>>  	ti,hwmods = "qspi";
>>  };
>> +
>> +For dra7xx:
>> +qspi: qspi@4b300000 {
>> +	compatible = "ti,dra7xxx-qspi";
>> +	reg = <0x4b300000 0x100>,
>> +	      <0x5c000000 0x4000000>,
>> +	      <0x4a002558 0x4>;
>> +	reg-names = "qspi_base", "qspi_mmap",
>> +		    "qspi_ctrlmod";
>> +	#address-cells = <1>;
>> +	#size-cells = <0>;
>> +	spi-max-frequency = <48000000>;
>> +	ti,hwmods = "qspi";
>> +};
> 
> Actually none of the IO areas above are within the same interconnect target:
> 
> 0x4b300000  QSPI0 address space in L3 main interconnect
> 0x5c000000  QSPI1 address space in L3 main interconnect


First address range (configuration port: 0x4b300000) corresponds to QSPI
registers space. These registers alone are sufficient for generic SPI
communication (serial flash as well as non-flash SPI devices).

In order to speed up SPI flash reads SFI_MM_IF(SPI memory mapped
interface) is provided by QSPI IP. This _cannot_ be used with non-flash
devices.
The second address range (0x5c000000) corresponds to memory-mapped
region of SFI_MM_IF, through which SPI flash memories can be read as if
though they were RAM addresses (i.e using readl/memcpy). The SFI_MM_IF
block that takes care of communicating over SPI bus and getting the data
from flash device.
But SFI_MM_IF block needs to know some flash specific information(such
as read opcode, address bytes, dummy bytes, mode). This information must
first be populated in QSPI_SPI_SETUP*_REG(0x4B300054-60) before
accessing SFI_MM_IF address range via readl.
Both addresses space belong to same instance of the driver, one
corresponds to register space and other is memory-mapped region.
Therefore both regions are claimed by the same driver.

> 0x4a002558  CTRL_CORE_CONTROL_IO_2 in System Control Module (SCM) in L4_CFG
> 
> The first two address spaces should be two separate instances of this driver.

Not actually two instances.

> The CTRL_CORE_CONTROL_IO_2 needs seems like a shared clock register that
> needs to be accessed using the clock framework most likely.
> 

Not shared clock.
The CTRL_CORE_CONTROL_IO_2[10:8] QSPI_MEMMAPPED_CS bit fields provides a
functionality for remapping the previously described address space which
starts at 0x5C000000 L3_MAIN address to one of the four supported chip
selects.
How about using syscon to access CTRL_CORE_CONTROL_IO_2?

Tony Lindgren Dec. 1, 2015, 4:39 p.m. UTC | #5

* Vignesh R <[e-mail]> [151130 20:46]:
> On 12/01/2015 04:04 AM, Tony Lindgren wrote:
> > 
> > Actually none of the IO areas above are within the same interconnect target:
> > 
> > 0x4b300000  QSPI0 address space in L3 main interconnect
> > 0x5c000000  QSPI1 address space in L3 main interconnect
> 
> 
> First address range (configuration port: 0x4b300000) corresponds to QSPI
> registers space. These registers alone are sufficient for generic SPI
> communication (serial flash as well as non-flash SPI devices).

OK

> In order to speed up SPI flash reads SFI_MM_IF(SPI memory mapped
> interface) is provided by QSPI IP. This _cannot_ be used with non-flash
> devices.

OK

> The second address range (0x5c000000) corresponds to memory-mapped
> region of SFI_MM_IF, through which SPI flash memories can be read as if
> though they were RAM addresses (i.e using readl/memcpy). The SFI_MM_IF
> block that takes care of communicating over SPI bus and getting the data
> from flash device.

OK

> But SFI_MM_IF block needs to know some flash specific information(such
> as read opcode, address bytes, dummy bytes, mode). This information must
> first be populated in QSPI_SPI_SETUP*_REG(0x4B300054-60) before
> accessing SFI_MM_IF address range via readl.
> Both addresses space belong to same instance of the driver, one
> corresponds to register space and other is memory-mapped region.
> Therefore both regions are claimed by the same driver.

OK

> > 0x4a002558  CTRL_CORE_CONTROL_IO_2 in System Control Module (SCM) in L4_CFG
> > 
> > The first two address spaces should be two separate instances of this driver.
> 
> Not actually two instances.

OK. They are both on L3 main so that won't cause any issues for separate
interconnect driver instances. As they are still separate targets flushing
a posted write to one area will not flush anything to the other.

> > The CTRL_CORE_CONTROL_IO_2 needs seems like a shared clock register that
> > needs to be accessed using the clock framework most likely.
> > 
> 
> Not shared clock.
> The CTRL_CORE_CONTROL_IO_2[10:8] QSPI_MEMMAPPED_CS bit fields provides a
> functionality for remapping the previously described address space which
> starts at 0x5C000000 L3_MAIN address to one of the four supported chip
> selects.
> How about using syscon to access CTRL_CORE_CONTROL_IO_2?

A separate driver implementing some Linux generic framework would be idael :)

But if that does not fit, yeah then syscon makes sense as that IO range
will be on separate interconnect driver (and clock and possibly power domain)
instances eventually.

Regards,

Tony

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Vignesh Raghavendra Dec. 3, 2015, 10:21 a.m. UTC | #6

On 12/01/2015 10:09 PM, Tony Lindgren wrote:
> * Vignesh R <[e-mail]> [151130 20:46]:
>> On 12/01/2015 04:04 AM, Tony Lindgren wrote:
>>>
>>> Actually none of the IO areas above are within the same interconnect target:
>>>
>>> 0x4b300000  QSPI0 address space in L3 main interconnect
>>> 0x5c000000  QSPI1 address space in L3 main interconnect
>>
>>
>> First address range (configuration port: 0x4b300000) corresponds to QSPI
>> registers space. These registers alone are sufficient for generic SPI
>> communication (serial flash as well as non-flash SPI devices).
> 
> OK
> 
>> In order to speed up SPI flash reads SFI_MM_IF(SPI memory mapped
>> interface) is provided by QSPI IP. This _cannot_ be used with non-flash
>> devices.
> 
> OK
> 
>> The second address range (0x5c000000) corresponds to memory-mapped
>> region of SFI_MM_IF, through which SPI flash memories can be read as if
>> though they were RAM addresses (i.e using readl/memcpy). The SFI_MM_IF
>> block that takes care of communicating over SPI bus and getting the data
>> from flash device.
> 
> OK
> 
>> But SFI_MM_IF block needs to know some flash specific information(such
>> as read opcode, address bytes, dummy bytes, mode). This information must
>> first be populated in QSPI_SPI_SETUP*_REG(0x4B300054-60) before
>> accessing SFI_MM_IF address range via readl.
>> Both addresses space belong to same instance of the driver, one
>> corresponds to register space and other is memory-mapped region.
>> Therefore both regions are claimed by the same driver.
> 
> OK
> 
>>> 0x4a002558  CTRL_CORE_CONTROL_IO_2 in System Control Module (SCM) in L4_CFG
>>>
>>> The first two address spaces should be two separate instances of this driver.
>>
>> Not actually two instances.
> 
> OK. They are both on L3 main so that won't cause any issues for separate
> interconnect driver instances. As they are still separate targets flushing
> a posted write to one area will not flush anything to the other.
> 

I didn't quite understand what you meant by interconnect driver instance.
qspi_base and qspi_mmap region are tightly bound to each other and both
needs to be accessed by ti-qspi driver (though different targets).
Besides qspi_mmap region is only used to read data, there will not be
any write accesses to this target. Are you saying this binding is not
viable?

>>> The CTRL_CORE_CONTROL_IO_2 needs seems like a shared clock register that
>>> needs to be accessed using the clock framework most likely.
>>>
>>
>> Not shared clock.
>> The CTRL_CORE_CONTROL_IO_2[10:8] QSPI_MEMMAPPED_CS bit fields provides a
>> functionality for remapping the previously described address space which
>> starts at 0x5C000000 L3_MAIN address to one of the four supported chip
>> selects.
>> How about using syscon to access CTRL_CORE_CONTROL_IO_2?
> 
> A separate driver implementing some Linux generic framework would be idael :)
> 
> But if that does not fit, yeah then syscon makes sense as that IO range
> will be on separate interconnect driver (and clock and possibly power domain)
> instances eventually.
> 

I will go ahead with syscon for accessing CTRL_CORE_CONTROL_IO_2 register.

Vignesh Raghavendra Dec. 10, 2015, 5:04 a.m. UTC | #7

On 12/03/2015 03:51 PM, Vignesh R wrote:
> 
> 
> On 12/01/2015 10:09 PM, Tony Lindgren wrote:
>> * Vignesh R <[e-mail]> [151130 20:46]:
>>> On 12/01/2015 04:04 AM, Tony Lindgren wrote:
>>>>
...
>>
>> OK. They are both on L3 main so that won't cause any issues for separate
>> interconnect driver instances. As they are still separate targets flushing
>> a posted write to one area will not flush anything to the other.
>>
> 
> I didn't quite understand what you meant by interconnect driver instance.
> qspi_base and qspi_mmap region are tightly bound to each other and both
> needs to be accessed by ti-qspi driver (though different targets).
> Besides qspi_mmap region is only used to read data, there will not be
> any write accesses to this target. Are you saying this binding is not
> viable?
> 

As I stated above qspi_base and qspi_mmap region are tightly bound,
there is no way to use qspi_mmap region w/o accessing qspi_base. So I am
planning to keep them as it is. I will move qspi_ctrlmod to use syscon.
Something like:

qspi: qspi@4b300000 {
       compatible = "ti,dra7xxx-qspi";
       reg = <0x4b300000 0x100>,
             <0x5c000000 0x4000000>,
       reg-names = "qspi_base", "qspi_mmap";
       syscon-chipselects = <&scm_conf 0x558>;
       #address-cells = <1>;
       #size-cells = <0>;
       spi-max-frequency = <48000000>;
       ti,hwmods = "qspi";
};

Do you think this is not viable in future?

Tony Lindgren Dec. 10, 2015, 5:44 p.m. UTC | #8

* Vignesh R <[e-mail]> [151209 21:05]:
> 
> 
> On 12/03/2015 03:51 PM, Vignesh R wrote:
> > 
> > 
> > On 12/01/2015 10:09 PM, Tony Lindgren wrote:
> >> * Vignesh R <[e-mail]> [151130 20:46]:
> >>> On 12/01/2015 04:04 AM, Tony Lindgren wrote:
> >>>>
> ...
> >>
> >> OK. They are both on L3 main so that won't cause any issues for separate
> >> interconnect driver instances. As they are still separate targets flushing
> >> a posted write to one area will not flush anything to the other.
> >>
> > 
> > I didn't quite understand what you meant by interconnect driver instance.
> > qspi_base and qspi_mmap region are tightly bound to each other and both
> > needs to be accessed by ti-qspi driver (though different targets).
> > Besides qspi_mmap region is only used to read data, there will not be
> > any write accesses to this target. Are you saying this binding is not
> > viable?
> > 
> 
> As I stated above qspi_base and qspi_mmap region are tightly bound,
> there is no way to use qspi_mmap region w/o accessing qspi_base. So I am
> planning to keep them as it is. I will move qspi_ctrlmod to use syscon.
> Something like:
> 
> qspi: qspi@4b300000 {
>        compatible = "ti,dra7xxx-qspi";
>        reg = <0x4b300000 0x100>,
>              <0x5c000000 0x4000000>,
>        reg-names = "qspi_base", "qspi_mmap";
>        syscon-chipselects = <&scm_conf 0x558>;
>        #address-cells = <1>;
>        #size-cells = <0>;
>        spi-max-frequency = <48000000>;
>        ti,hwmods = "qspi";
> };
> 
> Do you think this is not viable in future?

That's OK, they are on the same interconnect instance. And with the
syscon you're not directly tinkering with the SCM registers. So
yeah, that should work in the long run too.

The absolute addresses will get changed to just offsets from the
interconnect target. But if you use the Linux standard functions like
platform_get_resource_byname + devm_request_mem_region + devm_ioremap
then no changes are needed to your driver later on.

Thanks,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/Documentation/devicetree/bindings/spi/ti_qspi.txt b/Documentation/devicetree/bindings/spi/ti_qspi.txt
index 601a360531a5..334aa3f32cbc 100644
--- a/Documentation/devicetree/bindings/spi/ti_qspi.txt
+++ b/Documentation/devicetree/bindings/spi/ti_qspi.txt
@@ -26,3 +26,17 @@  qspi: qspi@4b300000 {
 	spi-max-frequency = <25000000>;
 	ti,hwmods = "qspi";
 };
+
+For dra7xx:
+qspi: qspi@4b300000 {
+	compatible = "ti,dra7xxx-qspi";
+	reg = <0x4b300000 0x100>,
+	      <0x5c000000 0x4000000>,
+	      <0x4a002558 0x4>;
+	reg-names = "qspi_base", "qspi_mmap",
+		    "qspi_ctrlmod";
+	#address-cells = <1>;
+	#size-cells = <0>;
+	spi-max-frequency = <48000000>;
+	ti,hwmods = "qspi";
+};
diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
index fe99231cbde5..debe7523643d 100644
--- a/arch/arm/boot/dts/dra7.dtsi
+++ b/arch/arm/boot/dts/dra7.dtsi
@@ -1153,8 +1153,11 @@ 
 
 		qspi: qspi@4b300000 {
 			compatible = "ti,dra7xxx-qspi";
-			reg = <0x4b300000 0x100>;
-			reg-names = "qspi_base";
+			reg = <0x4b300000 0x100>,
+			      <0x5c000000 0x4000000>,
+			      <0x4a002558 0x4>;
+			reg-names = "qspi_base", "qspi_mmap",
+				    "qspi_ctrlmod";
 			#address-cells = <1>;
 			#size-cells = <0>;
 			ti,hwmods = "qspi";