Re: [PATCH v4 3/3] arm64: dts: mt8173: Add nor flash node

2015-10-14 Thread Daniel Kurtz 
Sergei,

On Wed, Oct 14, 2015 at 7:05 PM, Sergei Shtylyov
 wrote:
> Hello.
>
> On 10/14/2015 4:07 AM, Daniel Kurtz wrote:
>
 Add Mediatek nor flash node

 Signed-off-by: Bayi Cheng 
 ---
arch/arm64/boot/dts/mediatek/mt8173.dtsi | 15 +++
1 file changed, 15 insertions(+)

 diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
 b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
 index d18ee42..385c2e4 100644
 --- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
 +++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
 @@ -365,6 +365,21 @@
  status = "disabled";
  };

 +   nor_flash: spi@1100d000 {
>>>
>>>
>>> I don't think this is a good label name for a SPI controller.
>>
>>
>> You think "spi" is not a good name for SPI controller?
>> Or you mean the label "nor_flash"?
>
>
>I think I said clearly: "label". :-)
>
>> Would you prefer:
>>
>>spi_nor_flash: spi@1100d000
>
>
>I'd prefer the "_nor_flash" part to be dropped, to be used for the real
> flash device (if needed) which is a sub-node of this node.

The dedicated nor_flash spi bus cannot just be labeled 'spi', as that
would conflict with the general purpose 'spi' bus.  Instead, it is
given a unique label that can be used as a phandle elsewhere (e.g., in
a board file, to enable the bus).  The name "spi_nor_flash" makes it
clear that this spi bus is only for use for accessing the nor_flash.

Bayi: Actually, this reminds me.  I'd prefer if the bus node was
disabled by default, and only enabled by boards as required.  So, the
node should include:
   status = "disabled";

-Dan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 3/3] arm64: dts: mt8173: Add nor flash node

2015-10-14 Thread Sergei Shtylyov 

Hello.

On 10/14/2015 4:07 AM, Daniel Kurtz wrote:


Add Mediatek nor flash node

Signed-off-by: Bayi Cheng 
---
   arch/arm64/boot/dts/mediatek/mt8173.dtsi | 15 +++
   1 file changed, 15 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
index d18ee42..385c2e4 100644
--- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
@@ -365,6 +365,21 @@
 status = "disabled";
 };

+   nor_flash: spi@1100d000 {


I don't think this is a good label name for a SPI controller.


You think "spi" is not a good name for SPI controller?
Or you mean the label "nor_flash"?


   I think I said clearly: "label". :-)


Would you prefer:

   spi_nor_flash: spi@1100d000


   I'd prefer the "_nor_flash" part to be dropped, to be used for the real 
flash device (if needed) which is a sub-node of this node.


[...]


-Dan


MBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 3/3] arm64: dts: mt8173: Add nor flash node

2015-10-13 Thread Daniel Kurtz 
Sergei,

On Tue, Oct 13, 2015 at 10:33 PM, Sergei Shtylyov
 wrote:
> Hello.
>
> On 10/13/2015 12:39 PM, Bayi Cheng wrote:
>
>> Add Mediatek nor flash node
>>
>> Signed-off-by: Bayi Cheng 
>> ---
>>   arch/arm64/boot/dts/mediatek/mt8173.dtsi | 15 +++
>>   1 file changed, 15 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
>> b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
>> index d18ee42..385c2e4 100644
>> --- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
>> +++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
>> @@ -365,6 +365,21 @@
>> status = "disabled";
>> };
>>
>> +   nor_flash: spi@1100d000 {
>
>
>I don't think this is a good label name for a SPI controller.

You think "spi" is not a good name for SPI controller?
Or you mean the label "nor_flash"?

Would you prefer:

  spi_nor_flash: spi@1100d000


>
>> +   compatible = "mediatek,mt8173-nor";
>> +   reg = <0 0x1100d000 0 0xe0>;
>> +   clocks = <&pericfg CLK_PERI_SPI>,
>> +<&topckgen CLK_TOP_SPINFI_IFR_SEL>;
>> +   clock-names = "spi", "sf";
>> +   #address-cells = <1>;
>> +   #size-cells = <0>;
>> +
>> +   flash@0 {
>> +   compatible = "jedec,spi-nor";
>> +   reg = <0>;
>> +   };
>> +   };
>> +
>> i2c3: i2c3@1101 {
>
>
>Grr, the name should be "i2c@1101".

Don't worry.  This is already fixed in Matthias' tree:
  
https://github.com/mbgg/linux-mediatek/commit/1ee35c05d9c67771d649f687e6f76e61e76eb779

-Dan

>
> [...]
>
> MBR, Sergei
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 3/3] arm64: dts: mt8173: Add nor flash node

2015-10-13 Thread Sergei Shtylyov 

Hello.

On 10/13/2015 12:39 PM, Bayi Cheng wrote:


Add Mediatek nor flash node

Signed-off-by: Bayi Cheng 
---
  arch/arm64/boot/dts/mediatek/mt8173.dtsi | 15 +++
  1 file changed, 15 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi 
b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
index d18ee42..385c2e4 100644
--- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
@@ -365,6 +365,21 @@
status = "disabled";
};

+   nor_flash: spi@1100d000 {


   I don't think this is a good label name for a SPI controller.


+   compatible = "mediatek,mt8173-nor";
+   reg = <0 0x1100d000 0 0xe0>;
+   clocks = <&pericfg CLK_PERI_SPI>,
+<&topckgen CLK_TOP_SPINFI_IFR_SEL>;
+   clock-names = "spi", "sf";
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   flash@0 {
+   compatible = "jedec,spi-nor";
+   reg = <0>;
+   };
+   };
+
i2c3: i2c3@1101 {


   Grr, the name should be "i2c@1101".

[...]

MBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v4 3/3] arm64: dts: mt8173: Add nor flash node

2015-10-13 Thread Bayi Cheng 
Add Mediatek nor flash node

Signed-off-by: Bayi Cheng 
---
 arch/arm64/boot/dts/mediatek/mt8173.dtsi | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi 
b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
index d18ee42..385c2e4 100644
--- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
@@ -365,6 +365,21 @@
status = "disabled";
};
 
+   nor_flash: spi@1100d000 {
+   compatible = "mediatek,mt8173-nor";
+   reg = <0 0x1100d000 0 0xe0>;
+   clocks = <&pericfg CLK_PERI_SPI>,
+<&topckgen CLK_TOP_SPINFI_IFR_SEL>;
+   clock-names = "spi", "sf";
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   flash@0 {
+   compatible = "jedec,spi-nor";
+   reg = <0>;
+   };
+   };
+
i2c3: i2c3@1101 {
compatible = "mediatek,mt8173-i2c";
reg = <0 0x1101 0 0x70>,
-- 
1.8.1.1.dirty

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/