diff mbox

[v1,1/3] ARM: dts: am335x-bone: add support for beaglebone NAND cape

Message ID 1403612666-31197-2-git-send-email-pekon@ti.com
State Not Applicable
Headers show

Commit Message

pekon gupta June 24, 2014, 12:24 p.m. UTC
Beaglebone Board can be connected to expansion boards to add devices to them.
These expansion boards are called 'capes'. This patch adds support for
following versions of Beaglebone(AM335x) NAND capes
(a) NAND Device with bus-width=16, block-size=128k, page-size=2k, oob-size=64
(b) NAND Device with bus-width=16, block-size=256k, page-size=4k, oob-size=224
Further information and datasheets can be found at [1] and [2]

* How to boot from NAND using Memory Expander + NAND Cape ? *
 - Important: As BOOTSEL values are sampled only at POR, so after changing any
   setting on SW2 (DIP switch), disconnect and reconnect all board power supply
   (including mini-USB console port) to POR the beaglebone.

 - Selection of ECC scheme
  for NAND cape(a), ROM code expects BCH8_HW ecc-scheme
  for NAND cape(b), ROM code expects BCH16_HW ecc-scheme

 - Selection of boot modes can be controlled via  DIP switch(SW2) present on
   Memory Expander cape, so first boot via MMC or other sources to flash NAND
   device and then switch to SW2[SWITCH_BOOT]=ON to boot from NAND Cape.
   SW2[SWITCH_BOOT] == OFF  follow default boot order  MMC-> SPI -> UART -> USB
   SW2[SWITCH_BOOT] == ON   boot mode selected via DIP switch(SW2)

 - For NAND boot following switch settings need to be followed
   SW2[ 1] = ON   (SYSBOOT[ 0]==0: NAND boot mode selected )
   SW2[ 2] = ON   (SYSBOOT[ 1]==0:       -- do --          )
   SW2[ 3] = OFF  (SYSBOOT[ 2]==1:       -- do --          )
   SW2[ 4] = OFF  (SYSBOOT[ 3]==1:       -- do --          )
   SW2[ 5] = ON   (SYSBOOT[ 4]==0:       -- do --          )
   SW2[ 6] = OFF  (SYSBOOT[ 8]==1: 0:x8 device, 1:x16 device )
   SW2[ 7] = ON   (SYSBOOT[ 9]==0: ECC done by ROM  )
   SW2[ 8] = ON   (SYSBOOT[10]==0: Non Muxed device )
   SW2[ 9] = ON   (SYSBOOT[11]==0:    -- do --      )

[1] http://beagleboardtoys.info/index.php?title=BeagleBone_Memory_Expansion
[2] http://beagleboardtoys.info/index.php?title=BeagleBone_4Gb_16-Bit_NAND_Module

Signed-off-by: Pekon Gupta <pekon@ti.com>
Reviewed-by: Javier Martinez Canillas <javier@dowhile0.org>
---
 arch/arm/boot/dts/am335x-bone-memory-cape.dts | 127 ++++++++++++++++++++++++++
 arch/arm/boot/dts/am335x-bone.dts             |   1 +
 arch/arm/boot/dts/am335x-boneblack.dts        |   1 +
 3 files changed, 129 insertions(+)
 create mode 100644 arch/arm/boot/dts/am335x-bone-memory-cape.dts

Comments

Ezequiel Garcia June 25, 2014, 3:27 p.m. UTC | #1
On 24 Jun 05:54 PM, Pekon Gupta wrote:
> +&gpmc {
> +	ranges = <0 0 0 0x01000000>;	/* address range = 16MB (minimum GPMC partition) */
> +	nand@0,0 {
> +		status = "disabled";
> +		reg = <0 0 4>;		/* device IO registers */
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&bbcape_nand_flash_pins>;
> +		ti,nand-ecc-opt = "bch8";
> +		ti,elm-id = <&elm>;

Don't you need something like this to turn on the elm device?

&elm {
	status = "okay";
};
pekon gupta June 26, 2014, 5:43 a.m. UTC | #2
>From: Ezequiel Garcia [mailto:ezequiel@vanguardiasur.com.ar]
>>On 24 Jun 05:54 PM, Pekon Gupta wrote:
>> +&gpmc {
>> +	ranges = <0 0 0 0x01000000>;	/* address range = 16MB (minimum GPMC partition) */
>> +	nand@0,0 {
>> +		status = "disabled";
>> +		reg = <0 0 4>;		/* device IO registers */
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&bbcape_nand_flash_pins>;
>> +		ti,nand-ecc-opt = "bch8";
>> +		ti,elm-id = <&elm>;
>
>Don't you need something like this to turn on the elm device?
>
>&elm {
>	status = "okay";
>};
>
No, This was the intend of these patches to keep cape DTS as "disabled"
so that:
(1)These patches should not change the behavior of existing DTS
 (am335x-boneblack.dts or am335x-bone.dts), or conflict with
 any existing node or other cape DTS.
(2) In-case of share-pin mux where multiple capes use same pins
Cape DTS should not conflict each other.

Now, to enable the choice of cape without hacking any source file,
user can use u-boot commands (refer to example in cover-letter):
/* load DTB */
u-boot> tftp 0x81000000 am335x-boneblack.dtb
u-boot> fdt addr 0x81000000
/* disabled conflicting MMC2 (eMMC) */
u-boot> fdt set  /ocp/mmc@481d8000 status \d\i\s\a\b\l\e\d
/* enable NAND cape*/
u-boot> fdt set  /ocp/elm status \o\k\a\y
u-boot> fdt set  /ocp/gpmc status \o\k\a\y
u-boot> fdt set  /ocp/gpmc/nand status \o\k\a\y

And then boot using this DTB loaded at 0x81000000
(you can put all these commands in uEnv as part of $bootcmd)


with regards, Pekon
Tony Lindgren June 26, 2014, 10:40 a.m. UTC | #3
* Pekon Gupta <pekon@ti.com> [140624 05:26]:
> +
> +&gpmc {
> +	ranges = <0 0 0 0x01000000>;	/* address range = 16MB (minimum GPMC partition) */
> +	nand@0,0 {
> +		status = "disabled";
> +		reg = <0 0 4>;		/* device IO registers */

Hmm so what about other capes potentially also using GPMC CS0?

Can you please do a check with two .dtsi files trying to use
GPMC CS0 and see what the produced .dtb file looks like after
decompiling it with dtc?

I'd assume the 16MB GPMC partition will be just fine for many
devices and can be also be larger if NOR needs it so maybe it
can be handled for almost all the cases.

The names for devices must be unique though to avoid them
getting merged. And I don't know if gpmc.c skips disabled devices
properly.

Regards,

Tony
Guido Martínez June 26, 2014, 3:06 p.m. UTC | #4
Hi Tony, Pekon

On Thu, Jun 26, 2014 at 03:40:44AM -0700, Tony Lindgren wrote:
> * Pekon Gupta <pekon@ti.com> [140624 05:26]:
> > +
> > +&gpmc {
> > +	ranges = <0 0 0 0x01000000>;	/* address range = 16MB (minimum GPMC partition) */
> > +	nand@0,0 {
> > +		status = "disabled";
> > +		reg = <0 0 4>;		/* device IO registers */
> 
> Hmm so what about other capes potentially also using GPMC CS0?
> 
> Can you please do a check with two .dtsi files trying to use
> GPMC CS0 and see what the produced .dtb file looks like after
> decompiling it with dtc?
> 
> I'd assume the 16MB GPMC partition will be just fine for many
> devices and can be also be larger if NOR needs it so maybe it
> can be handled for almost all the cases.
> 
> The names for devices must be unique though to avoid them
> getting merged. And I don't know if gpmc.c skips disabled devices
> properly.
It doesn't. I've just sent a patch for it.

http://lists.infradead.org/pipermail/linux-arm-kernel/2014-June/266966.html

Regards,
Guido

> 
> Regards,
> 
> Tony
> --
> 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
Guido Martínez June 26, 2014, 7:48 p.m. UTC | #5
Hi Pekon,

On Tue, Jun 24, 2014 at 05:54:24PM +0530, Pekon Gupta wrote:
> Beaglebone Board can be connected to expansion boards to add devices to them.
> These expansion boards are called 'capes'. This patch adds support for
> following versions of Beaglebone(AM335x) NAND capes
> (a) NAND Device with bus-width=16, block-size=128k, page-size=2k, oob-size=64
> (b) NAND Device with bus-width=16, block-size=256k, page-size=4k, oob-size=224
> Further information and datasheets can be found at [1] and [2]
> 
> * How to boot from NAND using Memory Expander + NAND Cape ? *
>  - Important: As BOOTSEL values are sampled only at POR, so after changing any
>    setting on SW2 (DIP switch), disconnect and reconnect all board power supply
>    (including mini-USB console port) to POR the beaglebone.
> 
>  - Selection of ECC scheme
>   for NAND cape(a), ROM code expects BCH8_HW ecc-scheme
>   for NAND cape(b), ROM code expects BCH16_HW ecc-scheme
> 
>  - Selection of boot modes can be controlled via  DIP switch(SW2) present on
>    Memory Expander cape, so first boot via MMC or other sources to flash NAND
>    device and then switch to SW2[SWITCH_BOOT]=ON to boot from NAND Cape.
>    SW2[SWITCH_BOOT] == OFF  follow default boot order  MMC-> SPI -> UART -> USB
>    SW2[SWITCH_BOOT] == ON   boot mode selected via DIP switch(SW2)
> 
>  - For NAND boot following switch settings need to be followed
>    SW2[ 1] = ON   (SYSBOOT[ 0]==0: NAND boot mode selected )
>    SW2[ 2] = ON   (SYSBOOT[ 1]==0:       -- do --          )
>    SW2[ 3] = OFF  (SYSBOOT[ 2]==1:       -- do --          )
>    SW2[ 4] = OFF  (SYSBOOT[ 3]==1:       -- do --          )
>    SW2[ 5] = ON   (SYSBOOT[ 4]==0:       -- do --          )
>    SW2[ 6] = OFF  (SYSBOOT[ 8]==1: 0:x8 device, 1:x16 device )
>    SW2[ 7] = ON   (SYSBOOT[ 9]==0: ECC done by ROM  )
>    SW2[ 8] = ON   (SYSBOOT[10]==0: Non Muxed device )
>    SW2[ 9] = ON   (SYSBOOT[11]==0:    -- do --      )
> 
> [1] http://beagleboardtoys.info/index.php?title=BeagleBone_Memory_Expansion
> [2] http://beagleboardtoys.info/index.php?title=BeagleBone_4Gb_16-Bit_NAND_Module
> 
> Signed-off-by: Pekon Gupta <pekon@ti.com>
> Reviewed-by: Javier Martinez Canillas <javier@dowhile0.org>
> ---
>  arch/arm/boot/dts/am335x-bone-memory-cape.dts | 127 ++++++++++++++++++++++++++
>  arch/arm/boot/dts/am335x-bone.dts             |   1 +
>  arch/arm/boot/dts/am335x-boneblack.dts        |   1 +
>  3 files changed, 129 insertions(+)
>  create mode 100644 arch/arm/boot/dts/am335x-bone-memory-cape.dts
> 
> diff --git a/arch/arm/boot/dts/am335x-bone-memory-cape.dts b/arch/arm/boot/dts/am335x-bone-memory-cape.dts
> new file mode 100644
> index 0000000..6d8ebd8
> --- /dev/null
> +++ b/arch/arm/boot/dts/am335x-bone-memory-cape.dts
> @@ -0,0 +1,127 @@
> +/*
> + * Copyright (C) 2014 Texas Instruments Incorporated - http://www.ti.com/
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This DTS adds supports for capes using GPMC interface to connect external
> + * memory like NAND, NOR Flash to Beaglebone-White and Beaglebone-Black.
> + */
> +
> +
> +&am33xx_pinmux {
> +	bbcape_nand_flash_pins: bbcape_nand_flash_pins {
> +		pinctrl-single,pins = <
> +			0x00 (MUX_MODE0 | PIN_INPUT)	/* gpmc_ad0.gpmc_ad0 */
> +			0x04 (MUX_MODE0 | PIN_INPUT)	/* gpmc_ad1.gpmc_ad1 */
> +			0x08 (MUX_MODE0 | PIN_INPUT)	/* gpmc_ad2.gpmc_ad2 */
> +			0x0c (MUX_MODE0 | PIN_INPUT)	/* gpmc_ad3.gpmc_ad3 */
> +			0x10 (MUX_MODE0 | PIN_INPUT)	/* gpmc_ad4.gpmc_ad4 */
> +			0x14 (MUX_MODE0 | PIN_INPUT)	/* gpmc_ad5.gpmc_ad5 */
> +			0x18 (MUX_MODE0 | PIN_INPUT)	/* gpmc_ad6.gpmc_ad6 */
> +			0x1c (MUX_MODE0 | PIN_INPUT)	/* gpmc_ad7.gpmc_ad7 */
> +			0x20 (MUX_MODE0 | PIN_INPUT)	/* gpmc_ad8.gpmc_ad8 */
> +			0x24 (MUX_MODE0 | PIN_INPUT)	/* gpmc_ad9.gpmc_ad9 */
> +			0x28 (MUX_MODE0 | PIN_INPUT)	/* gpmc_ad10.gpmc_ad10 */
> +			0x2c (MUX_MODE0 | PIN_INPUT)	/* gpmc_ad11.gpmc_ad11 */
> +			0x30 (MUX_MODE0 | PIN_INPUT)	/* gpmc_ad12.gpmc_ad12 */
> +			0x34 (MUX_MODE0 | PIN_INPUT)	/* gpmc_ad13.gpmc_ad13 */
> +			0x38 (MUX_MODE0 | PIN_INPUT)	/* gpmc_ad14.gpmc_ad14 */
> +			0x3c (MUX_MODE0 | PIN_INPUT)	/* gpmc_ad15.gpmc_ad15 */
> +			0x70 (MUX_MODE0 | PIN_INPUT_PULLUP )	/* gpmc_wait0.gpmc_wait0 */
> +			0x74 (MUX_MODE0 | PIN_OUTPUT_PULLUP)	/* gpmc_wpn.gpmc_wpn */
> +			0x7c (MUX_MODE0 | PIN_OUTPUT_PULLUP)	/* gpmc_csn0.gpmc_csn0  */
> +			0x90 (MUX_MODE0 | PIN_OUTPUT)		/* gpmc_advn_ale.gpmc_advn_ale */
> +			0x94 (MUX_MODE0 | PIN_OUTPUT)		/* gpmc_oen_ren.gpmc_oen_ren */
> +			0x98 (MUX_MODE0 | PIN_OUTPUT)		/* gpmc_wen.gpmc_wen */
> +			0x9c (MUX_MODE0 | PIN_OUTPUT)		/* gpmc_be0n_cle.gpmc_be0n_cle */
> +		>;
> +	};
> +};
> +
> +
> +&gpmc {
> +	ranges = <0 0 0 0x01000000>;	/* address range = 16MB (minimum GPMC partition) */
> +	nand@0,0 {
> +		status = "disabled";
> +		reg = <0 0 4>;		/* device IO registers */
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&bbcape_nand_flash_pins>;
This doesn't seem to work as pinctrl properties are not parsed for
childs of the gpmc node. Did this work for you?

Putting this in the gpmc node makes it work, but how will we control
pins for the nand and nor independently? I believe there is currently no
support for muxing individual gpmc devices. If we want to add both
devices to the DT and enable them as needed we'd need to add support for
this, right?

> +		ti,nand-ecc-opt = "bch8";
> +		ti,elm-id = <&elm>;
> +		/* generic bindings */
> +		nand-bus-width = <16>;
> +		/* vendor specific bindings */
> +		gpmc,device-width = <2>;
> +		gpmc,sync-clk-ps = <0>;
> +		gpmc,cs-on-ns = <0>;
> +		gpmc,cs-rd-off-ns = <80>;
> +		gpmc,cs-wr-off-ns = <80>;
> +		gpmc,adv-on-ns = <0>;
> +		gpmc,adv-rd-off-ns = <80>;
> +		gpmc,adv-wr-off-ns = <80>;
> +		gpmc,we-on-ns = <20>;
> +		gpmc,we-off-ns = <60>;
> +		gpmc,oe-on-ns = <20>;
> +		gpmc,oe-off-ns = <60>;
> +		gpmc,access-ns = <40>;
> +		gpmc,rd-cycle-ns = <80>;
> +		gpmc,wr-cycle-ns = <80>;
> +		gpmc,wait-pin = <0>;
> +		gpmc,wait-on-read;
> +		gpmc,wait-on-write;
> +		gpmc,bus-turnaround-ns = <0>;
> +		gpmc,cycle2cycle-delay-ns = <0>;
> +		gpmc,clk-activation-ns = <0>;
> +		gpmc,wait-monitoring-ns = <0>;
> +		gpmc,wr-access-ns = <40>;
> +		gpmc,wr-data-mux-bus-ns = <0>;
> +		/* MTD partition table */
> +		/* All SPL-* partitions are sized to minimal length
> +		 * which can be independently programmable. For
> +		 * NAND flash this is equal to size of erase-block */
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		partition@0 {
> +			label = "NAND.SPL";
> +			reg = <0x00000000 0x00040000>;
> +		};
> +		partition@1 {
> +			label = "NAND.SPL.backup1";
> +			reg = <0x00040000 0x00040000>;
> +		};
> +		partition@2 {
> +			label = "NAND.SPL.backup2";
> +			reg = <0x00080000 0x00040000>;
> +		};
> +		partition@3 {
> +			label = "NAND.SPL.backup3";
> +			reg = <0x000c0000 0x00040000>;
> +		};
> +		partition@4 {
> +			label = "NAND.u-boot-spl-os";
> +			reg = <0x00100000 0x00080000>;
> +		};
> +		partition@5 {
> +			label = "NAND.u-boot";
> +			reg = <0x00180000 0x00100000>;
> +		};
> +		partition@6 {
> +			label = "NAND.u-boot-env";
> +			reg = <0x00280000 0x00040000>;
> +		};
> +		partition@7 {
> +			label = "NAND.u-boot-env.backup1";
> +			reg = <0x002c0000 0x00040000>;
> +		};
> +		partition@8 {
> +			label = "NAND.kernel";
> +			reg = <0x00300000 0x00700000>;
> +		};
> +		partition@9 {
> +			label = "NAND.file-system";
> +			reg = <0x00a00000 0x1f600000>;
> +		};
> +	};
> +};
> diff --git a/arch/arm/boot/dts/am335x-bone.dts b/arch/arm/boot/dts/am335x-bone.dts
> index 94ee427..f16bfcf 100644
> --- a/arch/arm/boot/dts/am335x-bone.dts
> +++ b/arch/arm/boot/dts/am335x-bone.dts
> @@ -9,6 +9,7 @@
>  
>  #include "am33xx.dtsi"
>  #include "am335x-bone-common.dtsi"
> +#include "am335x-bone-memory-cape.dts"
>  
>  &ldo3_reg {
>  	regulator-min-microvolt = <1800000>;
> diff --git a/arch/arm/boot/dts/am335x-boneblack.dts b/arch/arm/boot/dts/am335x-boneblack.dts
> index 305975d..e6d7e54 100644
> --- a/arch/arm/boot/dts/am335x-boneblack.dts
> +++ b/arch/arm/boot/dts/am335x-boneblack.dts
> @@ -9,6 +9,7 @@
>  
>  #include "am33xx.dtsi"
>  #include "am335x-bone-common.dtsi"
> +#include "am335x-bone-memory-cape.dts"
>  
>  &ldo3_reg {
>  	regulator-min-microvolt = <1800000>;
> -- 
> 1.8.5.1.163.gd7aced9
> 
> --
> 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
pekon gupta June 27, 2014, 9:06 p.m. UTC | #6
>From: Guido Martínez [mailto:guido@vanguardiasur.com.ar]
>>On Tue, Jun 24, 2014 at 05:54:24PM +0530, Pekon Gupta wrote:

[...]

>> +&gpmc {
>> +	ranges = <0 0 0 0x01000000>;	/* address range = 16MB (minimum GPMC partition) */
>> +	nand@0,0 {
>> +		status = "disabled";
>> +		reg = <0 0 4>;		/* device IO registers */
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&bbcape_nand_flash_pins>;
>This doesn't seem to work as pinctrl properties are not parsed for
>childs of the gpmc node. Did this work for you?
>Putting this in the gpmc node makes it work, but how will we control
>pins for the nand and nor independently? I believe there is currently no
>support for muxing individual gpmc devices. If we want to add both
>devices to the DT and enable them as needed we'd need to add support for
>this, right?
>
Yes, And that should be the case, because different devices would be
connected to different chip-selects, so there should be support of
providing individual pin-mux for different GPMC devices.

Currently both NAND and NOR cape share GPMC_CS0, so both NAND and NOR
capes will not work simultaneously. But I'm planning to modify NOR cape
hardware at my end to use GPMC_CS1 instead of GPMC_CS0.
- NAND on GPMC_CS0
- NOR on GPMC_CS1
In addition to pin-mux you may also require following patch:
http://www.spinics.net/lists/linux-omap/msg107950.html

Also, I should have marked this series as RFC as its not fully tested.
My main intention was to get acknowledgement about cape DTS from
various users and Tony Lindgren <tony@atomide.com>.
Now as Tony has given some acceptance for these kind of cape DTS,
I'll clean-up and re-send these patches with better testing and GPMC fixes.


with regards, pekon


with regards, pekon
pekon gupta July 1, 2014, 7:01 a.m. UTC | #7
Hi Guido,

>From: Guido Martínez
>>On Thu, Jun 26, 2014 at 03:40:44AM -0700, Tony Lindgren wrote:
>> * Pekon Gupta <pekon@ti.com> [140624 05:26]:
>> > +
>> > +&gpmc {
>> > +	ranges = <0 0 0 0x01000000>;	/* address range = 16MB (minimum GPMC partition) */
>> > +	nand@0,0 {
>> > +		status = "disabled";
>> > +		reg = <0 0 4>;		/* device IO registers */
>>
>> Hmm so what about other capes potentially also using GPMC CS0?
>>
>> Can you please do a check with two .dtsi files trying to use
>> GPMC CS0 and see what the produced .dtb file looks like after
>> decompiling it with dtc?
>>
>> I'd assume the 16MB GPMC partition will be just fine for many
>> devices and can be also be larger if NOR needs it so maybe it
>> can be handled for almost all the cases.
>>
>> The names for devices must be unique though to avoid them
>> getting merged. And I don't know if gpmc.c skips disabled devices
>> properly.
>It doesn't. I've just sent a patch for it.
>
>http://lists.infradead.org/pipermail/linux-arm-kernel/2014-June/266966.html
>
I don't think we need above patch.
Helper macro "for_each_available_child_of_node" internally takes care
of skipping nodes with status="disabled".

$KERNEL/include/linux/of.h
#define for_each_available_child_of_node(parent, child) \
	for (child = of_get_next_available_child(parent, NULL); child != NULL; \
	     child = of_get_next_available_child(parent, child))

$KERNEL/drivers/of/base.c @@ of_get_next_available_child(...) {
	...
	if (!__of_device_is_available(next))
			continue;
	...


with regards, pekon
Tony Lindgren July 1, 2014, 8:47 a.m. UTC | #8
* Gupta, Pekon <pekon@ti.com> [140627 14:08]:
> >From: Guido Martínez [mailto:guido@vanguardiasur.com.ar]
> >>On Tue, Jun 24, 2014 at 05:54:24PM +0530, Pekon Gupta wrote:
> 
> [...]
> 
> >> +&gpmc {
> >> +	ranges = <0 0 0 0x01000000>;	/* address range = 16MB (minimum GPMC partition) */
> >> +	nand@0,0 {
> >> +		status = "disabled";
> >> +		reg = <0 0 4>;		/* device IO registers */
> >> +		pinctrl-names = "default";
> >> +		pinctrl-0 = <&bbcape_nand_flash_pins>;
> >This doesn't seem to work as pinctrl properties are not parsed for
> >childs of the gpmc node. Did this work for you?
> >Putting this in the gpmc node makes it work, but how will we control
> >pins for the nand and nor independently? I believe there is currently no
> >support for muxing individual gpmc devices. If we want to add both
> >devices to the DT and enable them as needed we'd need to add support for
> >this, right?
> >
> Yes, And that should be the case, because different devices would be
> connected to different chip-selects, so there should be support of
> providing individual pin-mux for different GPMC devices.
> 
> Currently both NAND and NOR cape share GPMC_CS0, so both NAND and NOR
> capes will not work simultaneously. But I'm planning to modify NOR cape
> hardware at my end to use GPMC_CS1 instead of GPMC_CS0.
> - NAND on GPMC_CS0
> - NOR on GPMC_CS1

Hmm but we should have these working with both using CS0 without
any need to modify the hardware though?

In that case we should make sure we always set large enough GPMC
partition and that the pins are muxed for the connected GPMC
devices only.

Regards,

Tony
pekon gupta July 1, 2014, 9:07 a.m. UTC | #9
>From: Tony Lindgren [mailto:tony@atomide.com]

>>* Gupta, Pekon <pekon@ti.com> [140627 14:08]:

>> >From: Guido Martínez [mailto:guido@vanguardiasur.com.ar]

>> >>On Tue, Jun 24, 2014 at 05:54:24PM +0530, Pekon Gupta wrote:

>>

>> [...]

>>

>> >> +&gpmc {

>> >> +	ranges = <0 0 0 0x01000000>;	/* address range = 16MB (minimum GPMC partition) */

>> >> +	nand@0,0 {

>> >> +		status = "disabled";

>> >> +		reg = <0 0 4>;		/* device IO registers */

>> >> +		pinctrl-names = "default";

>> >> +		pinctrl-0 = <&bbcape_nand_flash_pins>;

>> >This doesn't seem to work as pinctrl properties are not parsed for

>> >childs of the gpmc node. Did this work for you?

>> >Putting this in the gpmc node makes it work, but how will we control

>> >pins for the nand and nor independently? I believe there is currently no

>> >support for muxing individual gpmc devices. If we want to add both

>> >devices to the DT and enable them as needed we'd need to add support for

>> >this, right?

>> >

>> Yes, And that should be the case, because different devices would be

>> connected to different chip-selects, so there should be support of

>> providing individual pin-mux for different GPMC devices.

>>

>> Currently both NAND and NOR cape share GPMC_CS0, so both NAND and NOR

>> capes will not work simultaneously. But I'm planning to modify NOR cape

>> hardware at my end to use GPMC_CS1 instead of GPMC_CS0.

>> - NAND on GPMC_CS0

>> - NOR on GPMC_CS1

>

>Hmm but we should have these working with both using CS0 without

>any need to modify the hardware though?

>

Yes, but one at a time. That mean either of 'NAND cape' or 'NOR cape'.
If you need both working simultaneously as 2 separate devices attached
to GPMC, then you need 2 separate chip-selects, which is what I'm trying
to test with [1].


>In that case we should make sure we always set large enough GPMC

>partition

Yes, this is taken care with introduction of NOR cape in
[PATCH v1 2/3] ARM: dts: am335x-bone: add support for beaglebone NOR cape
 &gpmc {
-	ranges = <0 0 0 0x01000000>;	/* address range = 16MB (minimum GPMC partition) */
+	ranges = <0 0 0x08000000 0x01000000>;	/* address offset=128MB, range=128Mb=16MB */
This GPMC partition suffices for both NAND and NOR requirements.

> and that the pins are muxed for the connected GPMC

>devices only.

>

Pin mux is something I need to re-work, because currently
I've tested either NAND or NOR individually, not together.

Also as mentioned above by Guido, pin-ctrl property is not parsed individually
for GPMC children, Instead pin-ctrl is set once for GPMC as a whole.

Do you have any suggestions on how pin-ctrl should be set if we have
multiple devices connected to GPMC like;
All these devices will share:
- control lines (gpmc_wait, gpmc_ben_cle, gpmc_advn_ale, gpmc_we, gpmc_oe_ren)
- *some* data lines (gpmc_ad[])
- *some* address lines (gpmc_a[])
- but chip-selects will be different:
	gpmc_cs0: NAND
	gpmc_cs1: NOR
	gpmc_cs2: SMSC91xx
	gpmc_cs3: Camera


with regards, Pekon

>Regards,

>

>Tony


[1] http://www.spinics.net/lists/linux-omap/msg107950.html
Tony Lindgren July 1, 2014, 1:28 p.m. UTC | #10
* Gupta, Pekon <pekon@ti.com> [140701 02:09]:
> >From: Tony Lindgren [mailto:tony@atomide.com]
> >>* Gupta, Pekon <pekon@ti.com> [140627 14:08]:
> >> >From: Guido Martínez [mailto:guido@vanguardiasur.com.ar]
> >> >>On Tue, Jun 24, 2014 at 05:54:24PM +0530, Pekon Gupta wrote:
> >>
> >> [...]
> >>
> >> >> +&gpmc {
> >> >> +	ranges = <0 0 0 0x01000000>;	/* address range = 16MB (minimum GPMC partition) */
> >> >> +	nand@0,0 {
> >> >> +		status = "disabled";
> >> >> +		reg = <0 0 4>;		/* device IO registers */
> >> >> +		pinctrl-names = "default";
> >> >> +		pinctrl-0 = <&bbcape_nand_flash_pins>;
> >> >This doesn't seem to work as pinctrl properties are not parsed for
> >> >childs of the gpmc node. Did this work for you?
> >> >Putting this in the gpmc node makes it work, but how will we control
> >> >pins for the nand and nor independently? I believe there is currently no
> >> >support for muxing individual gpmc devices. If we want to add both
> >> >devices to the DT and enable them as needed we'd need to add support for
> >> >this, right?
> >> >
> >> Yes, And that should be the case, because different devices would be
> >> connected to different chip-selects, so there should be support of
> >> providing individual pin-mux for different GPMC devices.
> >>
> >> Currently both NAND and NOR cape share GPMC_CS0, so both NAND and NOR
> >> capes will not work simultaneously. But I'm planning to modify NOR cape
> >> hardware at my end to use GPMC_CS1 instead of GPMC_CS0.
> >> - NAND on GPMC_CS0
> >> - NOR on GPMC_CS1
> >
> >Hmm but we should have these working with both using CS0 without
> >any need to modify the hardware though?
> >
> Yes, but one at a time. That mean either of 'NAND cape' or 'NOR cape'.
> If you need both working simultaneously as 2 separate devices attached
> to GPMC, then you need 2 separate chip-selects, which is what I'm trying
> to test with [1].

Right only one enabled at a time, not both enabled at the same time :)
 
> >In that case we should make sure we always set large enough GPMC
> >partition
> Yes, this is taken care with introduction of NOR cape in
> [PATCH v1 2/3] ARM: dts: am335x-bone: add support for beaglebone NOR cape
>  &gpmc {
> -	ranges = <0 0 0 0x01000000>;	/* address range = 16MB (minimum GPMC partition) */
> +	ranges = <0 0 0x08000000 0x01000000>;	/* address offset=128MB, range=128Mb=16MB */
> This GPMC partition suffices for both NAND and NOR requirements.

OK
 
> > and that the pins are muxed for the connected GPMC
> >devices only.
> >
> Pin mux is something I need to re-work, because currently
> I've tested either NAND or NOR individually, not together.
> 
> Also as mentioned above by Guido, pin-ctrl property is not parsed individually
> for GPMC children, Instead pin-ctrl is set once for GPMC as a whole.

Yes, drivers/base/pinctrl.c should already take care of that though?
 
> Do you have any suggestions on how pin-ctrl should be set if we have
> multiple devices connected to GPMC like;
> All these devices will share:
> - control lines (gpmc_wait, gpmc_ben_cle, gpmc_advn_ale, gpmc_we, gpmc_oe_ren)
> - *some* data lines (gpmc_ad[])
> - *some* address lines (gpmc_a[])
> - but chip-selects will be different:
> 	gpmc_cs0: NAND
> 	gpmc_cs1: NOR
> 	gpmc_cs2: SMSC91xx
> 	gpmc_cs3: Camera

Well the pinctrl settings should be done a the child driver level
when it probes so drivers/base/pinctrl.c does what it's supposed
to do.

Regards,

Tony
Guido Martínez July 1, 2014, 11:42 p.m. UTC | #11
Hi Pekon,

On Tue, Jul 01, 2014 at 07:01:03AM +0000, Gupta, Pekon wrote:
> >http://lists.infradead.org/pipermail/linux-arm-kernel/2014-June/266966.html
> >
> I don't think we need above patch.
> Helper macro "for_each_available_child_of_node" internally takes care
> of skipping nodes with status="disabled".
> 
> $KERNEL/include/linux/of.h
> #define for_each_available_child_of_node(parent, child) \
> 	for (child = of_get_next_available_child(parent, NULL); child != NULL; \
> 	     child = of_get_next_available_child(parent, child))
> 
> $KERNEL/drivers/of/base.c @@ of_get_next_available_child(...) {
> 	...
> 	if (!__of_device_is_available(next))
> 			continue;
> 	...
Yes, that's why I suggest using that macro. It's not currently used
in gpmc.c and my patch changes the 'for_each_child_of_node' to a
'for_each_available_child_of_node'. Or am I missing something?

Regards,
pekon gupta July 2, 2014, 5:29 a.m. UTC | #12
>From: Guido Martínez [mailto:guido@vanguardiasur.com.ar]
>>On Tue, Jul 01, 2014 at 07:01:03AM +0000, Gupta, Pekon wrote:
>> >http://lists.infradead.org/pipermail/linux-arm-kernel/2014-June/266966.html
>> >
>> I don't think we need above patch.
>> Helper macro "for_each_available_child_of_node" internally takes care
>> of skipping nodes with status="disabled".
>>
>> $KERNEL/include/linux/of.h
>> #define for_each_available_child_of_node(parent, child) \
>> 	for (child = of_get_next_available_child(parent, NULL); child != NULL; \
>> 	     child = of_get_next_available_child(parent, child))
>>
>> $KERNEL/drivers/of/base.c @@ of_get_next_available_child(...) {
>> 	...
>> 	if (!__of_device_is_available(next))
>> 			continue;
>> 	...
>Yes, that's why I suggest using that macro. It's not currently used
>in gpmc.c and my patch changes the 'for_each_child_of_node' to a
>'for_each_available_child_of_node'. Or am I missing something?
>
Oops my bad. I was probably dreaming, I already had your patch in my tree
before reviewing the code. So please ignore my previous email.

with regards, pekon
diff mbox

Patch

diff --git a/arch/arm/boot/dts/am335x-bone-memory-cape.dts b/arch/arm/boot/dts/am335x-bone-memory-cape.dts
new file mode 100644
index 0000000..6d8ebd8
--- /dev/null
+++ b/arch/arm/boot/dts/am335x-bone-memory-cape.dts
@@ -0,0 +1,127 @@ 
+/*
+ * Copyright (C) 2014 Texas Instruments Incorporated - http://www.ti.com/
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This DTS adds supports for capes using GPMC interface to connect external
+ * memory like NAND, NOR Flash to Beaglebone-White and Beaglebone-Black.
+ */
+
+
+&am33xx_pinmux {
+	bbcape_nand_flash_pins: bbcape_nand_flash_pins {
+		pinctrl-single,pins = <
+			0x00 (MUX_MODE0 | PIN_INPUT)	/* gpmc_ad0.gpmc_ad0 */
+			0x04 (MUX_MODE0 | PIN_INPUT)	/* gpmc_ad1.gpmc_ad1 */
+			0x08 (MUX_MODE0 | PIN_INPUT)	/* gpmc_ad2.gpmc_ad2 */
+			0x0c (MUX_MODE0 | PIN_INPUT)	/* gpmc_ad3.gpmc_ad3 */
+			0x10 (MUX_MODE0 | PIN_INPUT)	/* gpmc_ad4.gpmc_ad4 */
+			0x14 (MUX_MODE0 | PIN_INPUT)	/* gpmc_ad5.gpmc_ad5 */
+			0x18 (MUX_MODE0 | PIN_INPUT)	/* gpmc_ad6.gpmc_ad6 */
+			0x1c (MUX_MODE0 | PIN_INPUT)	/* gpmc_ad7.gpmc_ad7 */
+			0x20 (MUX_MODE0 | PIN_INPUT)	/* gpmc_ad8.gpmc_ad8 */
+			0x24 (MUX_MODE0 | PIN_INPUT)	/* gpmc_ad9.gpmc_ad9 */
+			0x28 (MUX_MODE0 | PIN_INPUT)	/* gpmc_ad10.gpmc_ad10 */
+			0x2c (MUX_MODE0 | PIN_INPUT)	/* gpmc_ad11.gpmc_ad11 */
+			0x30 (MUX_MODE0 | PIN_INPUT)	/* gpmc_ad12.gpmc_ad12 */
+			0x34 (MUX_MODE0 | PIN_INPUT)	/* gpmc_ad13.gpmc_ad13 */
+			0x38 (MUX_MODE0 | PIN_INPUT)	/* gpmc_ad14.gpmc_ad14 */
+			0x3c (MUX_MODE0 | PIN_INPUT)	/* gpmc_ad15.gpmc_ad15 */
+			0x70 (MUX_MODE0 | PIN_INPUT_PULLUP )	/* gpmc_wait0.gpmc_wait0 */
+			0x74 (MUX_MODE0 | PIN_OUTPUT_PULLUP)	/* gpmc_wpn.gpmc_wpn */
+			0x7c (MUX_MODE0 | PIN_OUTPUT_PULLUP)	/* gpmc_csn0.gpmc_csn0  */
+			0x90 (MUX_MODE0 | PIN_OUTPUT)		/* gpmc_advn_ale.gpmc_advn_ale */
+			0x94 (MUX_MODE0 | PIN_OUTPUT)		/* gpmc_oen_ren.gpmc_oen_ren */
+			0x98 (MUX_MODE0 | PIN_OUTPUT)		/* gpmc_wen.gpmc_wen */
+			0x9c (MUX_MODE0 | PIN_OUTPUT)		/* gpmc_be0n_cle.gpmc_be0n_cle */
+		>;
+	};
+};
+
+
+&gpmc {
+	ranges = <0 0 0 0x01000000>;	/* address range = 16MB (minimum GPMC partition) */
+	nand@0,0 {
+		status = "disabled";
+		reg = <0 0 4>;		/* device IO registers */
+		pinctrl-names = "default";
+		pinctrl-0 = <&bbcape_nand_flash_pins>;
+		ti,nand-ecc-opt = "bch8";
+		ti,elm-id = <&elm>;
+		/* generic bindings */
+		nand-bus-width = <16>;
+		/* vendor specific bindings */
+		gpmc,device-width = <2>;
+		gpmc,sync-clk-ps = <0>;
+		gpmc,cs-on-ns = <0>;
+		gpmc,cs-rd-off-ns = <80>;
+		gpmc,cs-wr-off-ns = <80>;
+		gpmc,adv-on-ns = <0>;
+		gpmc,adv-rd-off-ns = <80>;
+		gpmc,adv-wr-off-ns = <80>;
+		gpmc,we-on-ns = <20>;
+		gpmc,we-off-ns = <60>;
+		gpmc,oe-on-ns = <20>;
+		gpmc,oe-off-ns = <60>;
+		gpmc,access-ns = <40>;
+		gpmc,rd-cycle-ns = <80>;
+		gpmc,wr-cycle-ns = <80>;
+		gpmc,wait-pin = <0>;
+		gpmc,wait-on-read;
+		gpmc,wait-on-write;
+		gpmc,bus-turnaround-ns = <0>;
+		gpmc,cycle2cycle-delay-ns = <0>;
+		gpmc,clk-activation-ns = <0>;
+		gpmc,wait-monitoring-ns = <0>;
+		gpmc,wr-access-ns = <40>;
+		gpmc,wr-data-mux-bus-ns = <0>;
+		/* MTD partition table */
+		/* All SPL-* partitions are sized to minimal length
+		 * which can be independently programmable. For
+		 * NAND flash this is equal to size of erase-block */
+		#address-cells = <1>;
+		#size-cells = <1>;
+		partition@0 {
+			label = "NAND.SPL";
+			reg = <0x00000000 0x00040000>;
+		};
+		partition@1 {
+			label = "NAND.SPL.backup1";
+			reg = <0x00040000 0x00040000>;
+		};
+		partition@2 {
+			label = "NAND.SPL.backup2";
+			reg = <0x00080000 0x00040000>;
+		};
+		partition@3 {
+			label = "NAND.SPL.backup3";
+			reg = <0x000c0000 0x00040000>;
+		};
+		partition@4 {
+			label = "NAND.u-boot-spl-os";
+			reg = <0x00100000 0x00080000>;
+		};
+		partition@5 {
+			label = "NAND.u-boot";
+			reg = <0x00180000 0x00100000>;
+		};
+		partition@6 {
+			label = "NAND.u-boot-env";
+			reg = <0x00280000 0x00040000>;
+		};
+		partition@7 {
+			label = "NAND.u-boot-env.backup1";
+			reg = <0x002c0000 0x00040000>;
+		};
+		partition@8 {
+			label = "NAND.kernel";
+			reg = <0x00300000 0x00700000>;
+		};
+		partition@9 {
+			label = "NAND.file-system";
+			reg = <0x00a00000 0x1f600000>;
+		};
+	};
+};
diff --git a/arch/arm/boot/dts/am335x-bone.dts b/arch/arm/boot/dts/am335x-bone.dts
index 94ee427..f16bfcf 100644
--- a/arch/arm/boot/dts/am335x-bone.dts
+++ b/arch/arm/boot/dts/am335x-bone.dts
@@ -9,6 +9,7 @@ 
 
 #include "am33xx.dtsi"
 #include "am335x-bone-common.dtsi"
+#include "am335x-bone-memory-cape.dts"
 
 &ldo3_reg {
 	regulator-min-microvolt = <1800000>;
diff --git a/arch/arm/boot/dts/am335x-boneblack.dts b/arch/arm/boot/dts/am335x-boneblack.dts
index 305975d..e6d7e54 100644
--- a/arch/arm/boot/dts/am335x-boneblack.dts
+++ b/arch/arm/boot/dts/am335x-boneblack.dts
@@ -9,6 +9,7 @@ 
 
 #include "am33xx.dtsi"
 #include "am335x-bone-common.dtsi"
+#include "am335x-bone-memory-cape.dts"
 
 &ldo3_reg {
 	regulator-min-microvolt = <1800000>;