Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1246754 > unrolled thread

Re: [PATCH v3 20/27] ARM: dts: dra7: Fix NAND device nodes.

Started by"Franklin S Cooper Jr." <fcooper@ti.com>
First post2015-10-14 15:40 +0200
Last post2015-10-14 16:40 +0200
Articles 3 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH v3 20/27] ARM: dts: dra7: Fix NAND device nodes. "Franklin S Cooper Jr." <fcooper@ti.com> - 2015-10-14 15:40 +0200
    Re: [PATCH v3 20/27] ARM: dts: dra7: Fix NAND device nodes. Roger Quadros <rogerq@ti.com> - 2015-10-14 16:20 +0200
      Re: [PATCH v3 20/27] ARM: dts: dra7: Fix NAND device nodes. "Franklin S Cooper Jr." <fcooper@ti.com> - 2015-10-14 16:40 +0200

#1246754 — Re: [PATCH v3 20/27] ARM: dts: dra7: Fix NAND device nodes.

From"Franklin S Cooper Jr." <fcooper@ti.com>
Date2015-10-14 15:40 +0200
SubjectRe: [PATCH v3 20/27] ARM: dts: dra7: Fix NAND device nodes.
Message-ID<qjurE-8nB-27@gated-at.bofh.it>

On 09/18/2015 09:53 AM, Roger Quadros wrote:
> Add compatible id, GPMC register resource and interrupt
> resource to NAND controller nodes.
>
> The GPMC driver now implements gpiochip and irqchip so
> enable gpio-controller and interrupt-controller properties.
>
> With this the interrupt parent of NAND node changes so fix it
> accordingly.
>
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
>  arch/arm/boot/dts/dra7-evm.dts  | 5 ++++-
>  arch/arm/boot/dts/dra7.dtsi     | 4 ++++
>  arch/arm/boot/dts/dra72-evm.dts | 5 ++++-
>  3 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/boot/dts/dra7-evm.dts b/arch/arm/boot/dts/dra7-evm.dts
> index a6c82e5..8a31161 100644
> --- a/arch/arm/boot/dts/dra7-evm.dts
> +++ b/arch/arm/boot/dts/dra7-evm.dts
> @@ -585,9 +585,12 @@
>  	status = "okay";
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&nand_flash_x16>;
> -	ranges = <0 0 0 0x01000000>;	/* minimum GPMC partition = 16MB */
> +	ranges = <0 0 0x08000000 0x01000000>;	/* minimum GPMC partition = 16MB */
>  	nand@0,0 {
> +		compatible = "ti,omap2-nand";
>  		reg = <0 0 4>;		/* device IO registers */
> +		interrupt-parent = <&crossbar_mpu>;
> +		interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>;
>  		ti,nand-ecc-opt = "bch8";
>  		ti,elm-id = <&elm>;
>  		nand-bus-width = <16>;
> diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
> index 5d65db9..f0a3616 100644
> --- a/arch/arm/boot/dts/dra7.dtsi
> +++ b/arch/arm/boot/dts/dra7.dtsi
> @@ -1389,6 +1389,10 @@
>  			gpmc,num-waitpins = <2>;
>  			#address-cells = <2>;
>  			#size-cells = <1>;
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +			interrupt-controller;
> +			#interrupt-cells = <2>;
>  			status = "disabled";
>  		};
Based on the discussion on my patchset I noticed that the nand node defines the
interrupt but it is also defined in the parent node. Similar to the dma channel we
should conclude where the best place for it to be defined.  But to me it seems at
least it should only be defined once.

This is true for your other patches making similar changes to the dt.
>  
> diff --git a/arch/arm/boot/dts/dra72-evm.dts b/arch/arm/boot/dts/dra72-evm.dts
> index 6f6bd98..245f5f9 100644
> --- a/arch/arm/boot/dts/dra72-evm.dts
> +++ b/arch/arm/boot/dts/dra72-evm.dts
> @@ -395,13 +395,16 @@
>  	status = "okay";
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&nand_default>;
> -	ranges = <0 0 0 0x01000000>;	/* minimum GPMC partition = 16MB */
> +	ranges = <0 0 0x08000000 0x01000000>;	/* minimum GPMC partition = 16MB */
>  	nand@0,0 {
>  		/* To use NAND, DIP switch SW5 must be set like so:
>  		 * SW5.1 (NAND_SELn) = ON (LOW)
>  		 * SW5.9 (GPMC_WPN) = OFF (HIGH)
>  		 */
> +		compatible = "ti,omap2-nand";
>  		reg = <0 0 4>;		/* device IO registers */
> +		interrupt-parent = <&crossbar_mpu>;
> +		interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>;
>  		ti,nand-ecc-opt = "bch8";
>  		ti,elm-id = <&elm>;
>  		nand-bus-width = <16>;

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

[toc] | [next] | [standalone]


#1246809

FromRoger Quadros <rogerq@ti.com>
Date2015-10-14 16:20 +0200
Message-ID<qjv4n-Wf-37@gated-at.bofh.it>
In reply to#1246754
On 14/10/15 16:34, Franklin S Cooper Jr. wrote:
> 
> 
> On 09/18/2015 09:53 AM, Roger Quadros wrote:
>> Add compatible id, GPMC register resource and interrupt
>> resource to NAND controller nodes.
>>
>> The GPMC driver now implements gpiochip and irqchip so
>> enable gpio-controller and interrupt-controller properties.
>>
>> With this the interrupt parent of NAND node changes so fix it
>> accordingly.
>>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
>>  arch/arm/boot/dts/dra7-evm.dts  | 5 ++++-
>>  arch/arm/boot/dts/dra7.dtsi     | 4 ++++
>>  arch/arm/boot/dts/dra72-evm.dts | 5 ++++-
>>  3 files changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/dra7-evm.dts b/arch/arm/boot/dts/dra7-evm.dts
>> index a6c82e5..8a31161 100644
>> --- a/arch/arm/boot/dts/dra7-evm.dts
>> +++ b/arch/arm/boot/dts/dra7-evm.dts
>> @@ -585,9 +585,12 @@
>>  	status = "okay";
>>  	pinctrl-names = "default";
>>  	pinctrl-0 = <&nand_flash_x16>;
>> -	ranges = <0 0 0 0x01000000>;	/* minimum GPMC partition = 16MB */
>> +	ranges = <0 0 0x08000000 0x01000000>;	/* minimum GPMC partition = 16MB */
>>  	nand@0,0 {
>> +		compatible = "ti,omap2-nand";
>>  		reg = <0 0 4>;		/* device IO registers */
>> +		interrupt-parent = <&crossbar_mpu>;
>> +		interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>;
>>  		ti,nand-ecc-opt = "bch8";
>>  		ti,elm-id = <&elm>;
>>  		nand-bus-width = <16>;
>> diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
>> index 5d65db9..f0a3616 100644
>> --- a/arch/arm/boot/dts/dra7.dtsi
>> +++ b/arch/arm/boot/dts/dra7.dtsi
>> @@ -1389,6 +1389,10 @@
>>  			gpmc,num-waitpins = <2>;
>>  			#address-cells = <2>;
>>  			#size-cells = <1>;
>> +			gpio-controller;
>> +			#gpio-cells = <2>;
>> +			interrupt-controller;
>> +			#interrupt-cells = <2>;
>>  			status = "disabled";
>>  		};
> Based on the discussion on my patchset I noticed that the nand node defines the
> interrupt but it is also defined in the parent node. Similar to the dma channel we
> should conclude where the best place for it to be defined.  But to me it seems at
> least it should only be defined once.

The interrupt is defined at both places because it is used at both places.
It is used as a shared interrupt. Wait_pin interrupts are managed by the
gpmc driver and NAND specific interrupts are managed by the NAND driver.

If GPMC dma channel is going to be used only by the NAND driver then
we should define the channel in the NAND node.

> 
> This is true for your other patches making similar changes to the dt.

Yes, GPMC IRQ is defined in both GPMC and NAND nodes.

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

[toc] | [prev] | [next] | [standalone]


#1246830

From"Franklin S Cooper Jr." <fcooper@ti.com>
Date2015-10-14 16:40 +0200
Message-ID<qjvnH-1lp-1@gated-at.bofh.it>
In reply to#1246809

On 10/14/2015 09:17 AM, Roger Quadros wrote:
> On 14/10/15 16:34, Franklin S Cooper Jr. wrote:
>>
>> On 09/18/2015 09:53 AM, Roger Quadros wrote:
>>> Add compatible id, GPMC register resource and interrupt
>>> resource to NAND controller nodes.
>>>
>>> The GPMC driver now implements gpiochip and irqchip so
>>> enable gpio-controller and interrupt-controller properties.
>>>
>>> With this the interrupt parent of NAND node changes so fix it
>>> accordingly.
>>>
>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>> ---
>>>  arch/arm/boot/dts/dra7-evm.dts  | 5 ++++-
>>>  arch/arm/boot/dts/dra7.dtsi     | 4 ++++
>>>  arch/arm/boot/dts/dra72-evm.dts | 5 ++++-
>>>  3 files changed, 12 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/arm/boot/dts/dra7-evm.dts b/arch/arm/boot/dts/dra7-evm.dts
>>> index a6c82e5..8a31161 100644
>>> --- a/arch/arm/boot/dts/dra7-evm.dts
>>> +++ b/arch/arm/boot/dts/dra7-evm.dts
>>> @@ -585,9 +585,12 @@
>>>  	status = "okay";
>>>  	pinctrl-names = "default";
>>>  	pinctrl-0 = <&nand_flash_x16>;
>>> -	ranges = <0 0 0 0x01000000>;	/* minimum GPMC partition = 16MB */
>>> +	ranges = <0 0 0x08000000 0x01000000>;	/* minimum GPMC partition = 16MB */
>>>  	nand@0,0 {
>>> +		compatible = "ti,omap2-nand";
>>>  		reg = <0 0 4>;		/* device IO registers */
>>> +		interrupt-parent = <&crossbar_mpu>;
>>> +		interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>;
>>>  		ti,nand-ecc-opt = "bch8";
>>>  		ti,elm-id = <&elm>;
>>>  		nand-bus-width = <16>;
>>> diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
>>> index 5d65db9..f0a3616 100644
>>> --- a/arch/arm/boot/dts/dra7.dtsi
>>> +++ b/arch/arm/boot/dts/dra7.dtsi
>>> @@ -1389,6 +1389,10 @@
>>>  			gpmc,num-waitpins = <2>;
>>>  			#address-cells = <2>;
>>>  			#size-cells = <1>;
>>> +			gpio-controller;
>>> +			#gpio-cells = <2>;
>>> +			interrupt-controller;
>>> +			#interrupt-cells = <2>;
>>>  			status = "disabled";
>>>  		};
>> Based on the discussion on my patchset I noticed that the nand node defines the
>> interrupt but it is also defined in the parent node. Similar to the dma channel we
>> should conclude where the best place for it to be defined.  But to me it seems at
>> least it should only be defined once.
> The interrupt is defined at both places because it is used at both places.
> It is used as a shared interrupt. Wait_pin interrupts are managed by the
> gpmc driver and NAND specific interrupts are managed by the NAND driver.
>
> If GPMC dma channel is going to be used only by the NAND driver then
> we should define the channel in the NAND node.
>
>> This is true for your other patches making similar changes to the dt.
> Yes, GPMC IRQ is defined in both GPMC and NAND nodes.
Ok. I would still think you would just reuse the entry from the
parent since you know it will always be the same. But we can
go with what Tony thinks is best.
>
> --
> cheers,
> -roger

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

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web