* [PATCH 00/12] Introduce davinci AEMIF driver
[not found] <1384187188-5776-1-git-send-email-ivan.khoronzhuk@ti.com>
@ 2013-11-11 16:47 ` Khoronzhuk, Ivan
[not found] ` <1384187188-5776-2-git-send-email-ivan.khoronzhuk@ti.com>
` (11 subsequent siblings)
12 siblings, 0 replies; 56+ messages in thread
From: Khoronzhuk, Ivan @ 2013-11-11 16:47 UTC (permalink / raw)
To: Shilimkar, Santosh, Rob Landley, Russell King
Cc: Mark Rutland, devicetree, Strashko, Grygorii, Pawel Moll,
Stephen Warren, Ian Campbell, Kumar Gala, Rob Herring,
linux-kernel, linux-mtd, linux-arm-kernel
These patches introduce AEMIF driver for davinci/keystone archs
and defines AEMIF and NAND devices in keystone DT.
Also some changes added to davinci NAND driver in order to reuse it
for Keystone arch.
The patches can be spitted to several series if needed, like
12 - add AEMIF/NAND device entry in DT
arm: dts: keystone: add AEMIF/NAND device entry
9-11 - reuse davinci-nand driver for keystone arch
mtd: nand: davinci: don't request AEMIF address range
mtd: nand: davinci: don't set timings if AEMIF is used
mtd: nand: davinci: reuse driver for Keystone arch
7-8 - introduce AEMIF driver
memory: davinci-aemif: add bindings for AEMIF driver
memory: davinci-aemif: introduce AEMIF driver
4-6 - move davinci-nand bindings under mtd
mtd: nand: davinci: adjust DT properties to MTD generic
mtd: nand: davinci: extend description of bindings
mtd: nand: davinci: move bindings under mtd
1-3 - separate nand fixes
mtd: nand: davinci: simplify error handling
mtd: nand: davinci: check required ti,davinci-chipselect property
mtd: nand: davinci: fix driver registration
See Documentation:
Davinci DM646x - http://www.ti.com/lit/ug/sprueq7c/sprueq7c.pdf
OMAP-L138 - http://www.ti.com/lit/ug/spruh77a/spruh77a.pdf
Kestone - http://www.ti.com/lit/ug/sprugz3a/sprugz3a.pdf
Test:
# flash_erase /dev/mtd2 0 0
Erasing 128 Kibyte @ 7de0000 -- 99 % complete flash_erase: Skipping bad block at
07e00000
flash_erase: Skipping bad block at 07e20000
flash_erase: Skipping bad block at 07e40000
flash_erase: Skipping bad block at 07e60000
Erasing 128 Kibyte @ 7e60000 -- 100 % complete
# nandtest -k -p 3 /dev/mtd2
ECC corrections: 0
ECC failures : 0
Bad blocks : 4
BBT blocks : 0
Bad block at 0x07e00000
Bad block at 0x07e20000
Bad block at 0x07e40000
Bad block at 0x07e60000
Finished pass 1 successfully
Bad block at 0x07e00000F44864
Bad block at 0x07e20000
Bad block at 0x07e40000
Bad block at 0x07e60000
Finished pass 2 successfully
Bad block at 0x07e00000
Bad block at 0x07e20000
Bad block at 0x07e40000
Bad block at 0x07e60000
Finished pass 3 successfully
Based on
git://git.kernel.org/pub/scm/linux/kernel/git/ssantosh/linux-keystone.git
keystone/master
Ivan Khoronzhuk (12):
mtd: nand: davinci: fix driver registration
mtd: nand: davinci: check required ti,davinci-chipselect property
mtd: nand: davinci: simplify error handling
mtd: nand: davinci: move bindings under mtd
mtd: nand: davinci: extend description of bindings
mtd: nand: davinci: adjust DT properties to MTD generic
memory: davinci-aemif: introduce AEMIF driver
memory: davinci-aemif: add bindings for AEMIF driver
mtd: nand: davinci: reuse driver for Keystone arch
mtd: nand: davinci: don't set timings if AEMIF is used
mtd: nand: davinci: don't request AEMIF address range
arm: dts: keystone: add AEMIF/NAND device entry
.../devicetree/bindings/arm/davinci/nand.txt | 46 ---
.../bindings/memory-controllers/davinci-aemif.txt | 198 ++++++++++
.../devicetree/bindings/mtd/davinci-nand.txt | 94 +++++
arch/arm/boot/dts/keystone.dts | 63 +++
drivers/memory/Kconfig | 11 +
drivers/memory/Makefile | 1 +
drivers/memory/davinci-aemif.c | 415 ++++++++++++++++++++
drivers/mtd/nand/Kconfig | 6 +-
drivers/mtd/nand/davinci_nand.c | 95 +++--
9 files changed, 839 insertions(+), 90 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/arm/davinci/nand.txt
create mode 100644 Documentation/devicetree/bindings/memory-controllers/davinci-aemif.txt
create mode 100644 Documentation/devicetree/bindings/mtd/davinci-nand.txt
create mode 100644 drivers/memory/davinci-aemif.c
--
1.7.9.5
^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH 01/12] mtd: nand: davinci: fix driver registration
[not found] ` <1384187188-5776-2-git-send-email-ivan.khoronzhuk@ti.com>
@ 2013-11-11 16:52 ` Khoronzhuk, Ivan
2013-11-12 15:45 ` Santosh Shilimkar
0 siblings, 1 reply; 56+ messages in thread
From: Khoronzhuk, Ivan @ 2013-11-11 16:52 UTC (permalink / raw)
To: Shilimkar, Santosh, Rob Landley, Russell King
Cc: Mark Rutland, devicetree, Strashko, Grygorii, Pawel Moll,
Stephen Warren, Ian Campbell, Kumar Gala, Rob Herring,
linux-kernel, linux-mtd, linux-arm-kernel
When kernel is booted using DT, there is no guarantee that Davinci
NAND device has been created already at the time when driver init
function is executed. Therefore, platform_driver_probe() can't be used
because this may result the Davinci NAND driver will never be probed.
The driver probing has to be made with core mechanism.
Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
---
drivers/mtd/nand/davinci_nand.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
index b77a01e..d87213f 100644
--- a/drivers/mtd/nand/davinci_nand.c
+++ b/drivers/mtd/nand/davinci_nand.c
@@ -877,6 +877,7 @@ static int __exit nand_davinci_remove(struct platform_device *pdev)
}
static struct platform_driver nand_davinci_driver = {
+ .probe = nand_davinci_probe,
.remove = __exit_p(nand_davinci_remove),
.driver = {
.name = "davinci_nand",
@@ -886,7 +887,7 @@ static struct platform_driver nand_davinci_driver = {
};
MODULE_ALIAS("platform:davinci_nand");
-module_platform_driver_probe(nand_davinci_driver, nand_davinci_probe);
+module_platform_driver(nand_davinci_driver);
MODULE_LICENSE("GPL");
MODULE_AUTHOR("Texas Instruments");
--
1.7.9.5
^ permalink raw reply related [flat|nested] 56+ messages in thread
* [PATCH 02/12] mtd: nand: davinci: check required ti,davinci-chipselect property
[not found] ` <1384187188-5776-3-git-send-email-ivan.khoronzhuk@ti.com>
@ 2013-11-11 16:53 ` Khoronzhuk, Ivan
2013-11-12 15:50 ` [PATCH 02/12] mtd: nand: davinci: check required ti, davinci-chipselect property Santosh Shilimkar
0 siblings, 1 reply; 56+ messages in thread
From: Khoronzhuk, Ivan @ 2013-11-11 16:53 UTC (permalink / raw)
To: Shilimkar, Santosh, Rob Landley, Russell King
Cc: Mark Rutland, devicetree, Strashko, Grygorii, Pawel Moll,
Stephen Warren, Ian Campbell, Kumar Gala, Rob Herring,
linux-kernel, linux-mtd, linux-arm-kernel
The property "ti,davinci-chipselect" is required. So we have to check
if it is set.
Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
---
drivers/mtd/nand/davinci_nand.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
index d87213f..8e1c88e 100644
--- a/drivers/mtd/nand/davinci_nand.c
+++ b/drivers/mtd/nand/davinci_nand.c
@@ -541,10 +541,14 @@ static struct davinci_nand_pdata
GFP_KERNEL);
pdev->dev.platform_data = pdata;
if (!pdata)
- return NULL;
+ return ERR_PTR(-ENOMEM);
+
if (!of_property_read_u32(pdev->dev.of_node,
"ti,davinci-chipselect", &prop))
pdev->id = prop;
+ else
+ return ERR_PTR(-EINVAL);
+
if (!of_property_read_u32(pdev->dev.of_node,
"ti,davinci-mask-ale", &prop))
pdata->mask_ale = prop;
@@ -598,6 +602,10 @@ static int __init nand_davinci_probe(struct platform_device *pdev)
nand_ecc_modes_t ecc_mode;
pdata = nand_davinci_get_pdata(pdev);
+ if (IS_ERR(pdata)) {
+ return PTR_ERR(pdata);
+ }
+
/* insist on board-specific configuration */
if (!pdata)
return -ENODEV;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 56+ messages in thread
* [PATCH 03/12] mtd: nand: davinci: simplify error handling
[not found] ` <1384187188-5776-4-git-send-email-ivan.khoronzhuk@ti.com>
@ 2013-11-11 16:54 ` Khoronzhuk, Ivan
2013-11-12 15:52 ` Santosh Shilimkar
0 siblings, 1 reply; 56+ messages in thread
From: Khoronzhuk, Ivan @ 2013-11-11 16:54 UTC (permalink / raw)
To: Shilimkar, Santosh, Rob Landley, Russell King
Cc: Mark Rutland, devicetree, Strashko, Grygorii, Pawel Moll,
Stephen Warren, Ian Campbell, Kumar Gala, Rob Herring,
linux-kernel, linux-mtd, linux-arm-kernel
There is not needed to use a lot of names for err handling.
It complicates code support and reading.
Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
---
drivers/mtd/nand/davinci_nand.c | 46 +++++++++++++++------------------------
1 file changed, 17 insertions(+), 29 deletions(-)
diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
index 8e1c88e..9bcbaa9 100644
--- a/drivers/mtd/nand/davinci_nand.c
+++ b/drivers/mtd/nand/davinci_nand.c
@@ -617,8 +617,7 @@ static int __init nand_davinci_probe(struct platform_device *pdev)
info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
if (!info) {
dev_err(&pdev->dev, "unable to allocate memory\n");
- ret = -ENOMEM;
- goto err_nomem;
+ return -ENOMEM;
}
platform_set_drvdata(pdev, info);
@@ -627,20 +626,16 @@ static int __init nand_davinci_probe(struct platform_device *pdev)
res2 = platform_get_resource(pdev, IORESOURCE_MEM, 1);
if (!res1 || !res2) {
dev_err(&pdev->dev, "resource missing\n");
- ret = -EINVAL;
- goto err_nomem;
+ return -EINVAL;
}
vaddr = devm_ioremap_resource(&pdev->dev, res1);
- if (IS_ERR(vaddr)) {
- ret = PTR_ERR(vaddr);
- goto err_ioremap;
- }
+ if (IS_ERR(vaddr))
+ return PTR_ERR(vaddr);
+
base = devm_ioremap_resource(&pdev->dev, res2);
- if (IS_ERR(base)) {
- ret = PTR_ERR(base);
- goto err_ioremap;
- }
+ if (IS_ERR(base))
+ return PTR_ERR(base);
info->dev = &pdev->dev;
info->base = base;
@@ -707,7 +702,7 @@ static int __init nand_davinci_probe(struct platform_device *pdev)
spin_unlock_irq(&davinci_nand_lock);
if (ret == -EBUSY)
- goto err_ecc;
+ return ret;
info->chip.ecc.calculate = nand_davinci_calculate_4bit;
info->chip.ecc.correct = nand_davinci_correct_4bit;
@@ -723,8 +718,7 @@ static int __init nand_davinci_probe(struct platform_device *pdev)
info->chip.ecc.strength = pdata->ecc_bits;
break;
default:
- ret = -EINVAL;
- goto err_ecc;
+ return -EINVAL;
}
info->chip.ecc.mode = ecc_mode;
@@ -732,7 +726,7 @@ static int __init nand_davinci_probe(struct platform_device *pdev)
if (IS_ERR(info->clk)) {
ret = PTR_ERR(info->clk);
dev_dbg(&pdev->dev, "unable to get AEMIF clock, err %d\n", ret);
- goto err_clk;
+ return ret;
}
ret = clk_prepare_enable(info->clk);
@@ -761,7 +755,7 @@ static int __init nand_davinci_probe(struct platform_device *pdev)
info->core_chipsel);
if (ret < 0) {
dev_dbg(&pdev->dev, "NAND timing values setup fail\n");
- goto err_timing;
+ goto err;
}
spin_lock_irq(&davinci_nand_lock);
@@ -777,7 +771,7 @@ static int __init nand_davinci_probe(struct platform_device *pdev)
ret = nand_scan_ident(&info->mtd, pdata->mask_chipsel ? 2 : 1, NULL);
if (ret < 0) {
dev_dbg(&pdev->dev, "no NAND chip(s) found\n");
- goto err_scan;
+ goto err;
}
/* Update ECC layout if needed ... for 1-bit HW ECC, the default
@@ -791,7 +785,7 @@ static int __init nand_davinci_probe(struct platform_device *pdev)
if (!chunks || info->mtd.oobsize < 16) {
dev_dbg(&pdev->dev, "too small\n");
ret = -EINVAL;
- goto err_scan;
+ goto err;
}
/* For small page chips, preserve the manufacturer's
@@ -822,7 +816,7 @@ static int __init nand_davinci_probe(struct platform_device *pdev)
dev_warn(&pdev->dev, "no 4-bit ECC support yet "
"for 4KiB-page NAND\n");
ret = -EIO;
- goto err_scan;
+ goto err;
syndrome_done:
info->chip.ecc.layout = &info->ecclayout;
@@ -830,7 +824,7 @@ syndrome_done:
ret = nand_scan_tail(&info->mtd);
if (ret < 0)
- goto err_scan;
+ goto err;
if (pdata->parts)
ret = mtd_device_parse_register(&info->mtd, NULL, NULL,
@@ -843,7 +837,7 @@ syndrome_done:
NULL, 0);
}
if (ret < 0)
- goto err_scan;
+ goto err;
val = davinci_nand_readl(info, NRCSR_OFFSET);
dev_info(&pdev->dev, "controller rev. %d.%d\n",
@@ -851,8 +845,7 @@ syndrome_done:
return 0;
-err_scan:
-err_timing:
+err:
clk_disable_unprepare(info->clk);
err_clk_enable:
@@ -860,11 +853,6 @@ err_clk_enable:
if (ecc_mode == NAND_ECC_HW_SYNDROME)
ecc4_busy = false;
spin_unlock_irq(&davinci_nand_lock);
-
-err_ecc:
-err_clk:
-err_ioremap:
-err_nomem:
return ret;
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 56+ messages in thread
* [PATCH 04/12] mtd: nand: davinci: move bindings under mtd
[not found] ` <1384187188-5776-5-git-send-email-ivan.khoronzhuk@ti.com>
@ 2013-11-11 16:55 ` Khoronzhuk, Ivan
2013-11-12 15:53 ` Santosh Shilimkar
0 siblings, 1 reply; 56+ messages in thread
From: Khoronzhuk, Ivan @ 2013-11-11 16:55 UTC (permalink / raw)
To: Shilimkar, Santosh, Rob Landley, Russell King
Cc: Mark Rutland, devicetree, Strashko, Grygorii, Pawel Moll,
Stephen Warren, Ian Campbell, Kumar Gala, Rob Herring,
linux-kernel, linux-mtd, linux-arm-kernel
Move bindings under mtd. Do this in order to make davinci-nand
driver usable by keystone architecture.
Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
---
.../{arm/davinci/nand.txt => mtd/davinci-nand.txt} | 0
1 file changed, 0 insertions(+), 0 deletions(-)
rename Documentation/devicetree/bindings/{arm/davinci/nand.txt => mtd/davinci-nand.txt} (100%)
diff --git a/Documentation/devicetree/bindings/arm/davinci/nand.txt b/Documentation/devicetree/bindings/mtd/davinci-nand.txt
similarity index 100%
rename from Documentation/devicetree/bindings/arm/davinci/nand.txt
rename to Documentation/devicetree/bindings/mtd/davinci-nand.txt
--
1.7.9.5
^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH 05/12] mtd: nand: davinci: extend description of bindings
[not found] ` <1384187188-5776-6-git-send-email-ivan.khoronzhuk@ti.com>
@ 2013-11-11 16:58 ` Khoronzhuk, Ivan
2013-11-12 15:55 ` Santosh Shilimkar
0 siblings, 1 reply; 56+ messages in thread
From: Khoronzhuk, Ivan @ 2013-11-11 16:58 UTC (permalink / raw)
To: Shilimkar, Santosh, Rob Landley, Russell King
Cc: Mark Rutland, devicetree, Strashko, Grygorii, Pawel Moll,
Stephen Warren, Ian Campbell, Kumar Gala, Rob Herring,
linux-kernel, linux-mtd, linux-arm-kernel
Extend bindings for davinci_nand driver to be more clear.
Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
---
.../devicetree/bindings/mtd/davinci-nand.txt | 77 ++++++++++++++------
1 file changed, 54 insertions(+), 23 deletions(-)
diff --git a/Documentation/devicetree/bindings/mtd/davinci-nand.txt b/Documentation/devicetree/bindings/mtd/davinci-nand.txt
index 3545ea7..d2a3fc0 100644
--- a/Documentation/devicetree/bindings/mtd/davinci-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/davinci-nand.txt
@@ -1,36 +1,67 @@
-* Texas Instruments Davinci NAND
+Device tree bindings for Texas instruments Davinci NAND controller
-This file provides information, what the device node for the
-davinci nand interface contain.
+This file provides information, what the device node for the davinci NAND
+interface contains.
+
+Documentation:
+Davinci DM646x - http://www.ti.com/lit/ug/sprueq7c/sprueq7c.pdf
Required properties:
-- compatible: "ti,davinci-nand";
-- reg : contain 2 offset/length values:
- - offset and length for the access window
- - offset and length for accessing the aemif control registers
-- ti,davinci-chipselect: Indicates on the davinci_nand driver which
- chipselect is used for accessing the nand.
+
+- compatible: "ti,davinci-nand"
+
+- reg: Contains 2 offset/length values:
+ - offset and length for the access window.
+ - offset and length for accessing the AEMIF
+ control registers.
+
+- ti,davinci-chipselect: number of chipselect. Indicates on the
+ davinci_nand driver which chipselect is used
+ for accessing the nand.
+ Can be in the range [0-3].
Recommended properties :
-- ti,davinci-mask-ale: mask for ale
-- ti,davinci-mask-cle: mask for cle
-- ti,davinci-mask-chipsel: mask for chipselect
-- ti,davinci-ecc-mode: ECC mode valid values for davinci driver:
- - "none"
- - "soft"
- - "hw"
-- ti,davinci-ecc-bits: used ECC bits, currently supported 1 or 4.
-- ti,davinci-nand-buswidth: buswidth 8 or 16
-- ti,davinci-nand-use-bbt: use flash based bad block table support.
-
-nand device bindings may contain additional sub-nodes describing
-partitions of the address space. See partition.txt for more detail.
+
+- ti,davinci-mask-ale: mask for ALE. Needed for executing address
+ phase. These offset will be added to the base
+ address for the chip select space the NAND Flash
+ device is connected to.
+ If not set equal to 0x08.
+
+- ti,davinci-mask-cle: mask for CLE. Needed for executing command
+ phase. These offset will be added to the base
+ address for the chip select space the NAND Flash
+ device is connected to.
+ If not set equal to 0x10.
+
+- ti,davinci-mask-chipsel: mask for chipselect address. Needed to mask
+ addresses for given chipselect.
+
+- ti,davinci-ecc-mode: operation mode of the NAND ecc mode. ECC mode
+ valid values for davinci driver:
+ - "none"
+ - "soft"
+ - "hw"
+
+- ti,davinci-ecc-bits: used ECC bits, currently supported 1 or 4.
+
+- ti,davinci-nand-buswidth: buswidth 8 or 16.
+
+- ti,davinci-nand-use-bbt: use flash based bad block table support. OOB
+ identifier is saved in OOB area.
+
+Nand device bindings may contain additional sub-nodes describing partitions of
+the address space. See partition.txt for more detail. The NAND Flash timing
+values must be programmed in the chip select’s node of AEMIF
+memory-controller (see Documentation/devicetree/bindings/memory-controllers/
+davinci-aemif.txt).
Example(da850 EVM ):
+
nand_cs3@62000000 {
compatible = "ti,davinci-nand";
reg = <0x62000000 0x807ff
- 0x68000000 0x8000>;
+ 0x68000000 0x8000>;
ti,davinci-chipselect = <1>;
ti,davinci-mask-ale = <0>;
ti,davinci-mask-cle = <0>;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 56+ messages in thread
* [PATCH 06/12] mtd: nand: davinci: adjust DT properties to MTD generic
[not found] ` <1384187188-5776-7-git-send-email-ivan.khoronzhuk@ti.com>
@ 2013-11-11 17:01 ` Khoronzhuk, Ivan
2013-11-26 7:03 ` Sekhar Nori
0 siblings, 1 reply; 56+ messages in thread
From: Khoronzhuk, Ivan @ 2013-11-11 17:01 UTC (permalink / raw)
To: Shilimkar, Santosh, Rob Landley, Russell King
Cc: Mark Rutland, devicetree, Strashko, Grygorii, Pawel Moll,
Stephen Warren, Ian Campbell, Kumar Gala, Rob Herring,
linux-kernel, linux-mtd, linux-arm-kernel
The properties davinci-ecc-mode, davinci-nand-use-bbt, davinci-nand-buswidth
are MTD generic. Correct names for them are: nand-ecc-mode, nand-on-flash-bbt,
nand-bus-width accordingly. So rename them in dts and documentation.
Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
---
.../devicetree/bindings/mtd/davinci-nand.txt | 25 ++++++++++++++++----
drivers/mtd/nand/davinci_nand.c | 11 ++++++---
2 files changed, 28 insertions(+), 8 deletions(-)
diff --git a/Documentation/devicetree/bindings/mtd/davinci-nand.txt b/Documentation/devicetree/bindings/mtd/davinci-nand.txt
index d2a3fc0..befaa5b 100644
--- a/Documentation/devicetree/bindings/mtd/davinci-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/davinci-nand.txt
@@ -37,7 +37,7 @@ Recommended properties :
- ti,davinci-mask-chipsel: mask for chipselect address. Needed to mask
addresses for given chipselect.
-- ti,davinci-ecc-mode: operation mode of the NAND ecc mode. ECC mode
+- nand-ecc-mode: operation mode of the NAND ecc mode. ECC mode
valid values for davinci driver:
- "none"
- "soft"
@@ -45,10 +45,25 @@ Recommended properties :
- ti,davinci-ecc-bits: used ECC bits, currently supported 1 or 4.
-- ti,davinci-nand-buswidth: buswidth 8 or 16.
+- nand-bus-width: buswidth 8 or 16. If not present 8.
+
+- nand-on-flash-bbt: use flash based bad block table support. OOB
+ identifier is saved in OOB area. If not present
+ false.
+
+Deprecated properties:
+
+- ti,davinci-ecc-mode: operation mode of the NAND ecc mode. ECC mode
+ valid values for davinci driver:
+ - "none"
+ - "soft"
+ - "hw"
+
+- ti,davinci-nand-buswidth: buswidth 8 or 16. If not present 8.
- ti,davinci-nand-use-bbt: use flash based bad block table support. OOB
- identifier is saved in OOB area.
+ identifier is saved in OOB area. If not present
+ false.
Nand device bindings may contain additional sub-nodes describing partitions of
the address space. See partition.txt for more detail. The NAND Flash timing
@@ -66,9 +81,9 @@ nand_cs3@62000000 {
ti,davinci-mask-ale = <0>;
ti,davinci-mask-cle = <0>;
ti,davinci-mask-chipsel = <0>;
- ti,davinci-ecc-mode = "hw";
+ nand-ecc-mode = "hw";
ti,davinci-ecc-bits = <4>;
- ti,davinci-nand-use-bbt;
+ nand-on-flash-bbt;
partition@180000 {
label = "ubifs";
diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
index 9bcbaa9..78b8d66 100644
--- a/drivers/mtd/nand/davinci_nand.c
+++ b/drivers/mtd/nand/davinci_nand.c
@@ -534,7 +534,6 @@ static struct davinci_nand_pdata
struct davinci_nand_pdata *pdata;
const char *mode;
u32 prop;
- int len;
pdata = devm_kzalloc(&pdev->dev,
sizeof(struct davinci_nand_pdata),
@@ -559,6 +558,8 @@ static struct davinci_nand_pdata
"ti,davinci-mask-chipsel", &prop))
pdata->mask_chipsel = prop;
if (!of_property_read_string(pdev->dev.of_node,
+ "nand-ecc-mode", &mode) ||
+ !of_property_read_string(pdev->dev.of_node,
"ti,davinci-ecc-mode", &mode)) {
if (!strncmp("none", mode, 4))
pdata->ecc_mode = NAND_ECC_NONE;
@@ -571,11 +572,15 @@ static struct davinci_nand_pdata
"ti,davinci-ecc-bits", &prop))
pdata->ecc_bits = prop;
if (!of_property_read_u32(pdev->dev.of_node,
+ "nand-bus-width", &prop) ||
+ !of_property_read_u32(pdev->dev.of_node,
"ti,davinci-nand-buswidth", &prop))
if (prop == 16)
pdata->options |= NAND_BUSWIDTH_16;
- if (of_find_property(pdev->dev.of_node,
- "ti,davinci-nand-use-bbt", &len))
+ if (of_property_read_bool(pdev->dev.of_node,
+ "nand-on-flash-bbt") ||
+ of_property_read_bool(pdev->dev.of_node,
+ "ti,davinci-nand-use-bbt"))
pdata->bbt_options = NAND_BBT_USE_FLASH;
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 56+ messages in thread
* [PATCH 07/12] memory: davinci-aemif: introduce AEMIF driver
[not found] ` <1384187188-5776-8-git-send-email-ivan.khoronzhuk@ti.com>
@ 2013-11-11 17:06 ` Khoronzhuk, Ivan
2013-11-12 16:08 ` Santosh Shilimkar
2013-11-26 7:20 ` Sekhar Nori
0 siblings, 2 replies; 56+ messages in thread
From: Khoronzhuk, Ivan @ 2013-11-11 17:06 UTC (permalink / raw)
To: Shilimkar, Santosh, Rob Landley, Russell King
Cc: Mark Rutland, devicetree, Strashko, Grygorii, Pawel Moll,
Stephen Warren, Ian Campbell, Kumar Gala, Rob Herring,
linux-kernel, linux-mtd, linux-arm-kernel
Add new AEMIF driver for EMIF16 davinci controller. The EMIF16 module
is intended to provide a glue-less interface to a variety of
asynchronous memory devices like ASRA M, NOR and NAND memory. A total
of 256M bytes of any of these memories can be accessed at any given
time via four chip selects with 64M byte access per chip select.
Synchronous memories such as DDR1 SD RAM, SDR SDRAM and Mobile SDR
are not supported.
See http://www.ti.com/lit/ug/sprugz3a/sprugz3a.pdf
Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
---
drivers/memory/Kconfig | 11 ++
drivers/memory/Makefile | 1 +
drivers/memory/davinci-aemif.c | 415 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 427 insertions(+)
create mode 100644 drivers/memory/davinci-aemif.c
diff --git a/drivers/memory/Kconfig b/drivers/memory/Kconfig
index 29a11db..010e75e 100644
--- a/drivers/memory/Kconfig
+++ b/drivers/memory/Kconfig
@@ -7,6 +7,17 @@ menuconfig MEMORY
if MEMORY
+config TI_DAVINCI_AEMIF
+ bool "Texas Instruments DaVinci AEMIF driver"
+ depends on (ARCH_DAVINCI || ARCH_KEYSTONE) && OF
+ help
+ This driver is for the AEMIF module available in Texas Instruments
+ SoCs. AEMIF stands for Asynchronous External Memory Interface and
+ is intended to provide a glue-less interface to a variety of
+ asynchronuous memory devices like ASRAM, NOR and NAND memory. A total
+ of 256M bytes of any of these memories can be accessed at a given
+ time via four chip selects with 64M byte access per chip select.
+
config TI_EMIF
tristate "Texas Instruments EMIF driver"
depends on ARCH_OMAP2PLUS
diff --git a/drivers/memory/Makefile b/drivers/memory/Makefile
index 969d923..af14126 100644
--- a/drivers/memory/Makefile
+++ b/drivers/memory/Makefile
@@ -5,6 +5,7 @@
ifeq ($(CONFIG_DDR),y)
obj-$(CONFIG_OF) += of_memory.o
endif
+obj-$(CONFIG_TI_DAVINCI_AEMIF) += davinci-aemif.o
obj-$(CONFIG_TI_EMIF) += emif.o
obj-$(CONFIG_MVEBU_DEVBUS) += mvebu-devbus.o
obj-$(CONFIG_TEGRA20_MC) += tegra20-mc.o
diff --git a/drivers/memory/davinci-aemif.c b/drivers/memory/davinci-aemif.c
new file mode 100644
index 0000000..e36b74b
--- /dev/null
+++ b/drivers/memory/davinci-aemif.c
@@ -0,0 +1,415 @@
+/*
+ * DaVinci/Keystone AEMIF driver
+ *
+ * Copyright (C) 2010 - 2013 Texas Instruments Incorporated. http://www.ti.com/
+ * Copyright (C) Heiko Schocher <hs@denx.de>
+ * Copyright (C) Ivan Khoronzhuk <ivan.khoronzhuk@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.
+ */
+
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+
+#define TA_SHIFT 2
+#define RHOLD_SHIFT 4
+#define RSTROBE_SHIFT 7
+#define RSETUP_SHIFT 13
+#define WHOLD_SHIFT 17
+#define WSTROBE_SHIFT 20
+#define WSETUP_SHIFT 26
+#define EW_SHIFT 30
+#define SS_SHIFT 31
+
+#define TA(x) ((x) << TA_SHIFT)
+#define RHOLD(x) ((x) << RHOLD_SHIFT)
+#define RSTROBE(x) ((x) << RSTROBE_SHIFT)
+#define RSETUP(x) ((x) << RSETUP_SHIFT)
+#define WHOLD(x) ((x) << WHOLD_SHIFT)
+#define WSTROBE(x) ((x) << WSTROBE_SHIFT)
+#define WSETUP(x) ((x) << WSETUP_SHIFT)
+#define EW(x) ((x) << EW_SHIFT)
+#define SS(x) ((x) << SS_SHIFT)
+
+#define ASIZE_MAX 0x1
+#define TA_MAX 0x3
+#define RHOLD_MAX 0x7
+#define RSTROBE_MAX 0x3f
+#define RSETUP_MAX 0xf
+#define WHOLD_MAX 0x7
+#define WSTROBE_MAX 0x3f
+#define WSETUP_MAX 0xf
+#define EW_MAX 0x1
+#define SS_MAX 0x1
+#define NUM_CS 4
+
+#define TA_VAL(x) (((x) & TA(TA_MAX)) >> TA_SHIFT)
+#define RHOLD_VAL(x) (((x) & RHOLD(RHOLD_MAX)) >> RHOLD_SHIFT)
+#define RSTROBE_VAL(x) (((x) & RSTROBE(RSTROBE_MAX)) >> RSTROBE_SHIFT)
+#define RSETUP_VAL(x) (((x) & RSETUP(RSETUP_MAX)) >> RSETUP_SHIFT)
+#define WHOLD_VAL(x) (((x) & WHOLD(WHOLD_MAX)) >> WHOLD_SHIFT)
+#define WSTROBE_VAL(x) (((x) & WSTROBE(WSTROBE_MAX)) >> WSTROBE_SHIFT)
+#define WSETUP_VAL(x) (((x) & WSETUP(WSETUP_MAX)) >> WSETUP_SHIFT)
+#define EW_VAL(x) (((x) & EW(EW_MAX)) >> EW_SHIFT)
+#define SS_VAL(x) (((x) & SS(SS_MAX)) >> SS_SHIFT)
+
+#define NRCSR_OFFSET 0x00
+#define AWCCR_OFFSET 0x04
+#define A1CR_OFFSET 0x10
+
+#define ACR_ASIZE_MASK 0x3
+#define ACR_EW_MASK BIT(30)
+#define ACR_SS_MASK BIT(31)
+#define ASIZE_16BIT 1
+
+#define CONFIG_MASK (TA(TA_MAX) | \
+ RHOLD(RHOLD_MAX) | \
+ RSTROBE(RSTROBE_MAX) | \
+ RSETUP(RSETUP_MAX) | \
+ WHOLD(WHOLD_MAX) | \
+ WSTROBE(WSTROBE_MAX) | \
+ WSETUP(WSETUP_MAX) | \
+ EW(EW_MAX) | SS(SS_MAX) | \
+ ASIZE_MAX)
+
+#define DRV_NAME "davinci-aemif"
+
+/**
+ * structure to hold cs parameters
+ * @cs: chip-select number
+ * @wstrobe: write strobe width, ns
+ * @rstrobe: read strobe width, ns
+ * @wsetup: write setup width, ns
+ * @whold: write hold width, ns
+ * @rsetup: read setup width, ns
+ * @rhold: read hold width, ns
+ * @ta: minimum turn around time, ns
+ * @enable_ss: enable/disable select strobe mode
+ * @enable_ew: enable/disable extended wait mode
+ * @asize: width of the asynchronous device's data bus
+ */
+struct davinci_aemif_cs_data {
+ u8 cs;
+ u16 wstrobe;
+ u16 rstrobe;
+ u8 wsetup;
+ u8 whold;
+ u8 rsetup;
+ u8 rhold;
+ u8 ta;
+ u8 enable_ss;
+ u8 enable_ew;
+ u8 asize;
+};
+
+/**
+ * structure to hold device data
+ * @base: base address of AEMIF registers
+ * @clk: source clock
+ * @clk_rate: clock's rate in kHz
+ * @num_cs: number of assigned chip-selects
+ * @cs_data: array of chip-select settings
+ */
+struct davinci_aemif_device {
+ struct device *dev;
+ void __iomem *base;
+ struct clk *clk;
+ unsigned long clk_rate;
+ u8 num_cs;
+ int cs_offset;
+ struct davinci_aemif_cs_data cs_data[NUM_CS];
+};
+
+static struct davinci_aemif_device *aemif;
+/**
+ * davinci_aemif_calc_rate - calculate timing data.
+ * @wanted: The cycle time needed in nanoseconds.
+ * @clk: The input clock rate in kHz.
+ * @max: The maximum divider value that can be programmed.
+ *
+ * On success, returns the calculated timing value minus 1 for easy
+ * programming into AEMIF timing registers, else negative errno.
+ */
+static int davinci_aemif_calc_rate(int wanted, unsigned long clk, int max)
+{
+ int result;
+
+ result = DIV_ROUND_UP((wanted * clk), NSEC_PER_MSEC) - 1;
+
+ dev_dbg(aemif->dev, "%s: result %d from %ld, %d\n", __func__, result,
+ clk, wanted);
+
+ /* It is generally OK to have a more relaxed timing than requested... */
+ if (result < 0)
+ result = 0;
+
+ /* ... But configuring tighter timings is not an option. */
+ else if (result > max)
+ result = -EINVAL;
+
+ return result;
+}
+
+/**
+ * davinci_aemif_config_abus - configure async bus parameters
+ * @data: aemif chip select configuration
+ *
+ * This function programs the given timing values (in real clock) into the
+ * AEMIF registers taking the AEMIF clock into account.
+ *
+ * This function does not use any locking while programming the AEMIF
+ * because it is expected that there is only one user of a given
+ * chip-select.
+ *
+ * Returns 0 on success, else negative errno.
+ */
+static int davinci_aemif_config_abus(struct davinci_aemif_cs_data *data)
+{
+ int ta, rhold, rstrobe, rsetup, whold, wstrobe, wsetup;
+ unsigned offset;
+ u32 set, val;
+
+ offset = A1CR_OFFSET + (data->cs - aemif->cs_offset) * 4;
+
+ ta = davinci_aemif_calc_rate(data->ta, aemif->clk_rate, TA_MAX);
+ rhold = davinci_aemif_calc_rate(data->rhold, aemif->clk_rate,
+ RHOLD_MAX);
+ rstrobe = davinci_aemif_calc_rate(data->rstrobe, aemif->clk_rate,
+ RSTROBE_MAX);
+ rsetup = davinci_aemif_calc_rate(data->rsetup, aemif->clk_rate,
+ RSETUP_MAX);
+ whold = davinci_aemif_calc_rate(data->whold, aemif->clk_rate,
+ WHOLD_MAX);
+ wstrobe = davinci_aemif_calc_rate(data->wstrobe, aemif->clk_rate,
+ WSTROBE_MAX);
+ wsetup = davinci_aemif_calc_rate(data->wsetup, aemif->clk_rate,
+ WSETUP_MAX);
+
+ if (ta < 0 || rhold < 0 || rstrobe < 0 || rsetup < 0 ||
+ whold < 0 || wstrobe < 0 || wsetup < 0) {
+ dev_err(aemif->dev, "%s: cannot get suitable timings\n",
+ __func__);
+ return -EINVAL;
+ }
+
+ set = TA(ta) | RHOLD(rhold) | RSTROBE(rstrobe) | RSETUP(rsetup) |
+ WHOLD(whold) | WSTROBE(wstrobe) | WSETUP(wsetup);
+
+ set |= (data->asize & ACR_ASIZE_MASK);
+ if (data->enable_ew)
+ set |= ACR_EW_MASK;
+ if (data->enable_ss)
+ set |= ACR_SS_MASK;
+
+ val = readl(aemif->base + offset);
+ val &= ~CONFIG_MASK;
+ val |= set;
+ writel(val, aemif->base + offset);
+
+ return 0;
+}
+
+inline int davinci_aemif_cycles_to_nsec(int val)
+{
+ return ((val + 1) * NSEC_PER_MSEC) / aemif->clk_rate;
+}
+
+/**
+ * davinci_aemif_get_hw_params - function to read hw register values
+ * @data: ptr to cs data
+ *
+ * This function reads the defaults from the registers and update
+ * the timing values. Required for get/set commands and also for
+ * the case when driver needs to use defaults in hardware.
+ */
+static void davinci_aemif_get_hw_params(struct davinci_aemif_cs_data *data)
+{
+ u32 val, offset;
+ offset = A1CR_OFFSET + (data->cs - aemif->cs_offset) * 4;
+
+ val = readl(aemif->base + offset);
+ data->ta = davinci_aemif_cycles_to_nsec(TA_VAL(val));
+ data->rhold = davinci_aemif_cycles_to_nsec(RHOLD_VAL(val));
+ data->rstrobe = davinci_aemif_cycles_to_nsec(RSTROBE_VAL(val));
+ data->rsetup = davinci_aemif_cycles_to_nsec(RSETUP_VAL(val));
+ data->whold = davinci_aemif_cycles_to_nsec(WHOLD_VAL(val));
+ data->wstrobe = davinci_aemif_cycles_to_nsec(WSTROBE_VAL(val));
+ data->wsetup = davinci_aemif_cycles_to_nsec(WSETUP_VAL(val));
+ data->enable_ew = EW_VAL(val);
+ data->enable_ss = SS_VAL(val);
+ data->asize = val & ASIZE_MAX;
+}
+
+/**
+ * of_davinci_aemif_parse_abus_config - parse CS configuration from DT
+ * @np: device node ptr
+ *
+ * This function update the emif async bus configuration based on the values
+ * configured in a cs device binding node.
+ */
+static int of_davinci_aemif_parse_abus_config(struct device_node *np)
+{
+ struct davinci_aemif_cs_data *data;
+ unsigned long cs;
+ u32 val;
+
+ if (kstrtoul(np->name + 2, 10, &cs) < 0) {
+ dev_dbg(aemif->dev, "cs name is incorrect");
+ return -EINVAL;
+ }
+
+ if (cs - aemif->cs_offset >= NUM_CS || cs < aemif->cs_offset) {
+ dev_dbg(aemif->dev, "cs number is incorrect %lu", cs);
+ return -EINVAL;
+ }
+
+ if (aemif->num_cs >= NUM_CS) {
+ dev_dbg(aemif->dev, "cs count is more than %d", NUM_CS);
+ return -EINVAL;
+ }
+
+ data = &aemif->cs_data[aemif->num_cs++];
+ data->cs = cs;
+
+ /* read the current value in the hw register */
+ davinci_aemif_get_hw_params(data);
+
+ /* override the values from device node */
+ of_property_read_u8(np, "ti,davinci-cs-ta", &data->ta);
+ of_property_read_u8(np, "ti,davinci-cs-rhold", &data->rhold);
+ of_property_read_u16(np, "ti,davinci-cs-rstrobe", &data->rstrobe);
+ of_property_read_u8(np, "ti,davinci-cs-rsetup", &data->rsetup);
+ of_property_read_u8(np, "ti,davinci-cs-whold", &data->whold);
+ of_property_read_u16(np, "ti,davinci-cs-wstrobe", &data->wstrobe);
+ of_property_read_u8(np, "ti,davinci-cs-wsetup", &data->wsetup);
+ if (!of_property_read_u32(np, "ti,bus-width", &val))
+ if (val == 16)
+ data->asize = 1;
+ data->enable_ew = of_property_read_bool(np, "ti,davinci-cs-ew");
+ data->enable_ss = of_property_read_bool(np, "ti,davinci-cs-ss");
+ return 0;
+}
+
+static const struct of_device_id davinci_aemif_of_match[] = {
+ { .compatible = "ti,davinci-aemif", },
+ { .compatible = "ti,keystone-aemif", },
+ { .compatible = "ti,omap-L138-aemif", },
+ {},
+};
+
+static const struct of_device_id davinci_cs_of_match[] = {
+ { .compatible = "ti,davinci-cs", },
+ {},
+};
+
+static int davinci_aemif_probe(struct platform_device *pdev)
+{
+ int ret = -ENODEV, i;
+ struct resource *res;
+ struct device *dev = &pdev->dev;
+ struct device_node *np = dev->of_node;
+
+ if (np == NULL)
+ return 0;
+
+ if (aemif) {
+ dev_err(dev, "davinci_aemif driver is in use currently\n");
+ return -EBUSY;
+ }
+
+ aemif = devm_kzalloc(dev, sizeof(*aemif), GFP_KERNEL);
+
+ if (!aemif) {
+ dev_err(dev, "cannot allocate memory for aemif\n");
+ return -ENOMEM;
+ }
+
+ aemif->clk = devm_clk_get(dev, "aemif");
+ if (IS_ERR(aemif->clk)) {
+ dev_err(dev, "cannot get clock 'aemif'\n");
+ return PTR_ERR(aemif->clk);
+ }
+
+ clk_prepare_enable(aemif->clk);
+ aemif->clk_rate = clk_get_rate(aemif->clk) / 1000;
+
+ aemif->dev = dev;
+
+ if (of_device_is_compatible(np, "ti,omap-L138-aemif"))
+ aemif->cs_offset = 2;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ aemif->base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(aemif->base)) {
+ ret = PTR_ERR(aemif->base);
+ goto error;
+ }
+
+ /*
+ * For every controller device node, there is a cs device node that
+ * describe the bus configuration parameters. This functions iterate
+ * over these nodes and update the cs data array.
+ */
+ for_each_matching_node(np, of_match_ptr(davinci_cs_of_match)) {
+ ret = of_davinci_aemif_parse_abus_config(np);
+ if (ret < 0)
+ goto error;
+ }
+
+ for (i = 0; i < aemif->num_cs; i++) {
+ ret = davinci_aemif_config_abus(&aemif->cs_data[i]);
+ if (ret < 0) {
+ dev_err(dev, "Error configuring chip select %d\n",
+ aemif->cs_data[i].cs);
+ goto error;
+ }
+ }
+
+ /*
+ * Create a child devices explicitly from here to
+ * guarantee that the child will be probed after the AEMIF timing
+ * parameters are set.
+ */
+ for_each_matching_node(np, of_match_ptr(davinci_cs_of_match)) {
+ ret = of_platform_populate(np, NULL, NULL, dev);
+ if (ret < 0)
+ goto error;
+ }
+
+ return 0;
+error:
+ clk_disable_unprepare(aemif->clk);
+ return ret;
+}
+
+static int davinci_aemif_remove(struct platform_device *pdev)
+{
+ clk_disable_unprepare(aemif->clk);
+ return 0;
+}
+
+static struct platform_driver davinci_aemif_driver = {
+ .probe = davinci_aemif_probe,
+ .remove = davinci_aemif_remove,
+ .driver = {
+ .name = DRV_NAME,
+ .owner = THIS_MODULE,
+ .of_match_table = of_match_ptr(davinci_aemif_of_match),
+ },
+};
+
+module_platform_driver(davinci_aemif_driver);
+
+MODULE_AUTHOR("Murali Karicheri <m-karicheri2@ti.com>");
+MODULE_AUTHOR("Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>");
+MODULE_DESCRIPTION("Texas Instruments AEMIF driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:" DRV_NAME);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 56+ messages in thread
* [PATCH 08/12] memory: davinci-aemif: add bindings for AEMIF driver
[not found] ` <1384187188-5776-9-git-send-email-ivan.khoronzhuk@ti.com>
@ 2013-11-11 17:09 ` Khoronzhuk, Ivan
0 siblings, 0 replies; 56+ messages in thread
From: Khoronzhuk, Ivan @ 2013-11-11 17:09 UTC (permalink / raw)
To: Shilimkar, Santosh, Rob Landley, Russell King
Cc: Mark Rutland, devicetree, Strashko, Grygorii, Pawel Moll,
Stephen Warren, Ian Campbell, Kumar Gala, Rob Herring,
linux-kernel, linux-mtd, linux-arm-kernel
Add bindings for AEMIF controller drivers/memory/davinici-aemif.c
Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
---
.../bindings/memory-controllers/davinci-aemif.txt | 198 ++++++++++++++++++++
1 file changed, 198 insertions(+)
create mode 100644 Documentation/devicetree/bindings/memory-controllers/davinci-aemif.txt
diff --git a/Documentation/devicetree/bindings/memory-controllers/davinci-aemif.txt b/Documentation/devicetree/bindings/memory-controllers/davinci-aemif.txt
new file mode 100644
index 0000000..5e5319a
--- /dev/null
+++ b/Documentation/devicetree/bindings/memory-controllers/davinci-aemif.txt
@@ -0,0 +1,198 @@
+* Device tree bindings for Texas instruments Davinci/Keystone AEMIF controller
+
+Th Async External Memory Interface (EMIF16/AEMIF) controller is intended to
+provide a glue-less interface to a variety of asynchronous memory devices like
+ASRA M, NOR and NAND memory. A total of 256M bytes of any of these memories
+can be accessed at any given time via four chip selects with 64M byte access
+per chip select. Synchronous memories such as DDR1 SD RAM, SDR SDRAM
+and Mobile SDR are not supported.
+
+Documentation:
+Davinci DM646x - http://www.ti.com/lit/ug/sprueq7c/sprueq7c.pdf
+OMAP-L138 - http://www.ti.com/lit/ug/spruh77a/spruh77a.pdf
+Kestone - http://www.ti.com/lit/ug/sprugz3a/sprugz3a.pdf
+
+Required properties:
+
+- compatible: "ti,davinci-aemif"
+ "ti,keystone-aemif"
+ "ti,omap-L138-aemif"
+
+- #address-cells: Must be 2. The first cell is the memory partition
+ number. The 0 partition is for chip selects. And the
+ second cell is the offset into the partition, for the 0
+ partition it corresponds to chip select offset.
+
+- #size-cells: Must be set to 1.
+
+- reg: contains offset/length value for AEMIF control registers
+ space.
+
+- ranges: Must be set up to reflect the memory layout for 4
+ chipselects and for AEMIF control range.
+
+- clocks: phandle reference to the controller clock. Required only
+ if clock tree data present in device tree.
+ See clock-bindings.txt
+
+- clock-names: clock name. It has to be "aemif". Required only if clock
+ tree data present in device tree, in another case don't
+ use it.
+ See clock-bindings.txt
+
+- clock-ranges: Empty property indicating that child nodes can inherit
+ named clocks. Required only if clock tree data present
+ in device tree.
+ See clock-bindings.txt
+
+
+Child chip-select (cs) nodes contain the memory devices nodes connected to
+such as NOR (e.g. cfi-flash) and NAND (ti,davinci-nand, see davinci-nand.txt).
+There might be board specific devices like FPGAs.
+
+Required child cs node properties:
+
+- compatible: "ti,davinci-cs"
+
+- #address-cells: Must be 2. The first cell is the memory partition
+ number. The 0 partition is for chip selects. And the
+ second cell is the offset into the partition, for the 0
+ partition it corresponds to chip select offset.
+
+- #size-cells: Must be 1.
+
+- ranges: Empty property indicating that child nodes can inherit
+ memory layout.
+
+- clock-ranges: Empty property indicating that child nodes can inherit
+ named clocks. Required only if clock tree data present
+ in device tree.
+
+Optional child cs node properties:
+
+
+- ti,bus-width: width of the asynchronous device's data bus
+ 8 or 16 if not preset 8
+
+- ti,davinci-cs-ss: enable/disable select strobe mode
+ In select strobe mode chip select behaves as
+ the strobe and is active only during the strobe
+ period. If present then enable.
+
+- ti,davinci-cs-ew: enable/disable extended wait mode
+ if set, the controller monitors the EMIFWAIT pin
+ mapped to that chip select to determine if the
+ device wants to extend the strobe period. If
+ present then enable.
+
+- ti,davinci-cs-ta: minimum turn around time, ns
+ Time between the end of one asynchronous memory
+ access and the start of another asynchronous
+ memory access. This delay is not incurred
+ between a read followed by read or a write
+ followed by a write to same chip select.
+
+- ti,davinci-cs-rsetup: read setup width, ns
+ Time between the beginning of a memory cycle
+ and the activation of read strobe.
+ Minimum value is 1 (0 treated as 1).
+
+- ti,davinci-cs-rstobe: read strobe width, ns
+ Time between the activation and deactivation of
+ the read strobe.
+ Minimum value is 1 (0 treated as 1).
+
+- ti,davinci-cs-rhold: read hold width, ns
+ Time between the deactivation of the read
+ strobe and the end of the cycle (which may be
+ either an address change or the deactivation of
+ the chip select signal.
+ Minimum value is 1 (0 treated as 1).
+
+- ti,davinci-cs-wsetup: write setup width, ns
+ Time between the beginning of a memory cycle
+ and the activation of write strobe.
+ Minimum value is 1 (0 treated as 1).
+
+- ti,davinci-cs-wstrobe: write strobe width, ns
+ Time between the activation and deactivation of
+ the write strobe.
+ Minimum value is 1 (0 treated as 1).
+
+- ti,davinci-cs-whold: write hold width, ns
+ Time between the deactivation of the write
+ strobe and the end of the cycle (which may be
+ either an address change or the deactivation of
+ the chip select signal.
+ Minimum value is 1 (0 treated as 1).
+
+If any of the above parameters are absent, current parameter value will be taken
+from the corresponding HW reg.
+
+The name for cs node must be in format csN, where N is the cs number.
+For compatibles "ti,davinci-aemif" and "ti,keystone-aemif" N = [0-3].
+For compatible "ti,omap-L138-aemif" N = [2-5].
+
+Example for aemif, davinci nand and nor flash chip select shown below.
+
+memory-controller@21000A00 {
+ compatible = "ti,keystone-aemif";
+ #address-cells = <2>;
+ #size-cells = <1>;
+ clocks = <&clkaemif 0>;
+ clock-names = "aemif";
+ clock-ranges;
+ reg = <0x2100A00 0x00000100>;
+ ranges = <0 0 0x70000000 0x10000000
+ 1 0 0x21000A00 0x0000100>;
+
+ nand:cs2 {
+ compatible = "ti,davinci-cs";
+ #address-cells = <2>;
+ #size-cells = <1>;
+ clock-ranges;
+ ranges;
+
+ /* all timings in nanoseconds */
+ ti,davinci-cs-ta = <0>;
+ ti,davinci-cs-rhold = <7>;
+ ti,davinci-cs-rstrobe = <42>;
+ ti,davinci-cs-rsetup = <14>;
+ ti,davinci-cs-whold = <7>;
+ ti,davinci-cs-wstrobe = <42>;
+ ti,davinci-cs-wsetup = <14>;
+
+ nand@0,0x8000000 {
+ compatible = "ti,davinci-nand";
+ reg = <0 0x8000000 0x4000000
+ 1 0x0000000 0x0000100>;
+
+ .. see davinci-nand.txt
+ };
+ };
+
+ nor:cs0 {
+ compatible = "ti,davinci-cs";
+ #address-cells = <2>;
+ #size-cells = <1>;
+ clock-ranges;
+ ranges;
+
+ /* all timings in nanoseconds */
+ ti,davinci-cs-ta = <0>;
+ ti,davinci-cs-rhold = <8>;
+ ti,davinci-cs-rstrobe = <40>;
+ ti,davinci-cs-rsetup = <14>;
+ ti,davinci-cs-whold = <7>;
+ ti,davinci-cs-wstrobe = <40>;
+ ti,davinci-cs-wsetup = <14>;
+ ti,davinci-cs-asize = <1>;
+
+ flash@0,0x0000000 {
+ compatible = "cfi-flash";
+ reg = <0 0x0000000 0x4000000>;
+
+ ...
+ };
+ };
+};
--
1.7.9.5
^ permalink raw reply related [flat|nested] 56+ messages in thread
* [PATCH 09/12] mtd: nand: davinci: reuse driver for Keystone arch
[not found] ` <1384187188-5776-10-git-send-email-ivan.khoronzhuk@ti.com>
@ 2013-11-11 17:09 ` Khoronzhuk, Ivan
2013-11-12 16:09 ` Santosh Shilimkar
0 siblings, 1 reply; 56+ messages in thread
From: Khoronzhuk, Ivan @ 2013-11-11 17:09 UTC (permalink / raw)
To: Shilimkar, Santosh, Rob Landley, Russell King
Cc: Mark Rutland, devicetree, Strashko, Grygorii, Pawel Moll,
Stephen Warren, Ian Campbell, Kumar Gala, Rob Herring,
linux-kernel, linux-mtd, linux-arm-kernel
The Keystone arch has compatible nand device, so reuse it.
In case with Keystone it depends on TI_DAVINCI_AEMIF because AEMIF
driver is responsible to set timings.
See http://www.ti.com/lit/ug/sprugz3a/sprugz3a.pdf
Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
---
.../devicetree/bindings/mtd/davinci-nand.txt | 8 +++++---
drivers/mtd/nand/Kconfig | 6 +++---
drivers/mtd/nand/davinci_nand.c | 1 +
3 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/Documentation/devicetree/bindings/mtd/davinci-nand.txt b/Documentation/devicetree/bindings/mtd/davinci-nand.txt
index befaa5b..cfb18ab 100644
--- a/Documentation/devicetree/bindings/mtd/davinci-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/davinci-nand.txt
@@ -1,14 +1,16 @@
-Device tree bindings for Texas instruments Davinci NAND controller
+Device tree bindings for Texas instruments Davinci/Keystone NAND controller
-This file provides information, what the device node for the davinci NAND
-interface contains.
+This file provides information, what the device node for the davinci/keystone
+NAND interface contains.
Documentation:
Davinci DM646x - http://www.ti.com/lit/ug/sprueq7c/sprueq7c.pdf
+Kestone - http://www.ti.com/lit/ug/sprugz3a/sprugz3a.pdf
Required properties:
- compatible: "ti,davinci-nand"
+ "ti,keystone-nand"
- reg: Contains 2 offset/length values:
- offset and length for the access window.
diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index d885298..62dbcd5 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -492,11 +492,11 @@ config MTD_NAND_SH_FLCTL
for NAND Flash using FLCTL.
config MTD_NAND_DAVINCI
- tristate "Support NAND on DaVinci SoC"
- depends on ARCH_DAVINCI
+ tristate "Support NAND on DaVinci/Keystone SoC"
+ depends on ARCH_DAVINCI || (ARCH_KEYSTONE && TI_DAVINCI_AEMIF)
help
Enable the driver for NAND flash chips on Texas Instruments
- DaVinci processors.
+ DaVinci/Keystone processors.
config MTD_NAND_TXX9NDFMC
tristate "NAND Flash support for TXx9 SoC"
diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
index 78b8d66..4705214 100644
--- a/drivers/mtd/nand/davinci_nand.c
+++ b/drivers/mtd/nand/davinci_nand.c
@@ -523,6 +523,7 @@ static struct nand_ecclayout hwecc4_2048 __initconst = {
#if defined(CONFIG_OF)
static const struct of_device_id davinci_nand_of_match[] = {
{.compatible = "ti,davinci-nand", },
+ {.compatible = "ti,keystone-nand", },
{},
};
MODULE_DEVICE_TABLE(of, davinci_nand_of_match);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 56+ messages in thread
* [PATCH 10/12] mtd: nand: davinci: don't set timings if AEMIF is used
[not found] ` <1384187188-5776-11-git-send-email-ivan.khoronzhuk@ti.com>
@ 2013-11-11 17:10 ` Khoronzhuk, Ivan
2013-11-12 16:13 ` Santosh Shilimkar
2013-11-13 5:02 ` Sekhar Nori
0 siblings, 2 replies; 56+ messages in thread
From: Khoronzhuk, Ivan @ 2013-11-11 17:10 UTC (permalink / raw)
To: Shilimkar, Santosh, Rob Landley, Russell King
Cc: Mark Rutland, devicetree, Strashko, Grygorii, Pawel Moll,
Stephen Warren, Ian Campbell, Kumar Gala, Rob Herring,
linux-kernel, linux-mtd, linux-arm-kernel
If Davinci AEMIF is used we don't need to set timings and bus width.
It is done by AEMIF driver (drivers/memory/davinci-aemfi.c).
Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
---
drivers/mtd/nand/davinci_nand.c | 22 +++++++++++++++-------
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
index 4705214..879e915 100644
--- a/drivers/mtd/nand/davinci_nand.c
+++ b/drivers/mtd/nand/davinci_nand.c
@@ -742,27 +742,35 @@ static int __init nand_davinci_probe(struct platform_device *pdev)
goto err_clk_enable;
}
+#if !IS_ENABLED(CONFIG_TI_DAVINCI_AEMIF)
/*
- * Setup Async configuration register in case we did not boot from
- * NAND and so bootloader did not bother to set it up.
+ * Setup Async configuration register in case we did not boot
+ * from NAND and so bootloader did not bother to set it up.
*/
- val = davinci_nand_readl(info, A1CR_OFFSET + info->core_chipsel * 4);
+ val = davinci_nand_readl(info, A1CR_OFFSET +
+ info->core_chipsel * 4);
- /* Extended Wait is not valid and Select Strobe mode is not used */
+ /*
+ * Extended Wait is not valid and Select Strobe mode is not
+ * used
+ */
val &= ~(ACR_ASIZE_MASK | ACR_EW_MASK | ACR_SS_MASK);
if (info->chip.options & NAND_BUSWIDTH_16)
val |= 0x1;
- davinci_nand_writel(info, A1CR_OFFSET + info->core_chipsel * 4, val);
+ davinci_nand_writel(info, A1CR_OFFSET +
+ info->core_chipsel * 4, val);
ret = 0;
if (info->timing)
- ret = davinci_aemif_setup_timing(info->timing, info->base,
- info->core_chipsel);
+ ret = davinci_aemif_setup_timing(info->timing,
+ info->base,
+ info->core_chipsel);
if (ret < 0) {
dev_dbg(&pdev->dev, "NAND timing values setup fail\n");
goto err;
}
+#endif
spin_lock_irq(&davinci_nand_lock);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 56+ messages in thread
* [PATCH 11/12] mtd: nand: davinci: don't request AEMIF address range
[not found] ` <1384187188-5776-12-git-send-email-ivan.khoronzhuk@ti.com>
@ 2013-11-11 17:12 ` Khoronzhuk, Ivan
2013-11-12 16:15 ` Santosh Shilimkar
0 siblings, 1 reply; 56+ messages in thread
From: Khoronzhuk, Ivan @ 2013-11-11 17:12 UTC (permalink / raw)
To: Shilimkar, Santosh, Rob Landley, Russell King
Cc: Mark Rutland, devicetree, Strashko, Grygorii, Pawel Moll,
Stephen Warren, Ian Campbell, Kumar Gala, Rob Herring,
linux-kernel, linux-mtd, linux-arm-kernel
The AEMIF driver registers are used to setup timings for each chip
select. The same registers range is used to setup NAND settings.
The AEMIF and NAND drivers not use the same registers in this range.
In case with AEMIF driver, the memory address range is requested
already by AEMIF, so we cannot request it twice, just ioremap.
Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
---
drivers/mtd/nand/davinci_nand.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
index 879e915..bb6b7e5 100644
--- a/drivers/mtd/nand/davinci_nand.c
+++ b/drivers/mtd/nand/davinci_nand.c
@@ -639,9 +639,11 @@ static int __init nand_davinci_probe(struct platform_device *pdev)
if (IS_ERR(vaddr))
return PTR_ERR(vaddr);
- base = devm_ioremap_resource(&pdev->dev, res2);
- if (IS_ERR(base))
- return PTR_ERR(base);
+ base = devm_ioremap(&pdev->dev, res2->start, resource_size(res2));
+ if (!base) {
+ dev_err(&pdev->dev, "ioremap failed for resource %pR\n", res2);
+ return -EADDRNOTAVAIL;
+ }
info->dev = &pdev->dev;
info->base = base;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 56+ messages in thread
* [PATCH 12/12] arm: dts: keystone: add AEMIF/NAND device entry
[not found] ` <1384187188-5776-13-git-send-email-ivan.khoronzhuk@ti.com>
@ 2013-11-11 17:13 ` Khoronzhuk, Ivan
2013-11-12 16:19 ` Santosh Shilimkar
0 siblings, 1 reply; 56+ messages in thread
From: Khoronzhuk, Ivan @ 2013-11-11 17:13 UTC (permalink / raw)
To: Shilimkar, Santosh, Rob Landley, Russell King
Cc: Mark Rutland, devicetree, Strashko, Grygorii, Pawel Moll,
Stephen Warren, Ian Campbell, Kumar Gala, Rob Herring,
linux-kernel, linux-mtd, linux-arm-kernel
Add AEMIF/NAND device entry.
Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
---
arch/arm/boot/dts/keystone.dts | 63 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 63 insertions(+)
diff --git a/arch/arm/boot/dts/keystone.dts b/arch/arm/boot/dts/keystone.dts
index 100bdf5..998da98 100644
--- a/arch/arm/boot/dts/keystone.dts
+++ b/arch/arm/boot/dts/keystone.dts
@@ -179,5 +179,68 @@
interrupts = <GIC_SPI 300 IRQ_TYPE_EDGE_RISING>;
clocks = <&clkspi>;
};
+
+ aemif@021000A00 {
+ compatible = "ti,keystone-aemif";
+ #address-cells = <2>;
+ #size-cells = <1>;
+ clocks = <&clkaemif>;
+ clock-names = "aemif";
+ clock-ranges;
+
+ reg = <0x2100A00 0x00000100>;
+ ranges = <0 0 0x30000000 0x10000000
+ 1 0 0x21000A00 0x00000100>;
+
+ nand:cs0 {
+ compatible = "ti,davinci-cs";
+ #address-cells = <2>;
+ #size-cells = <1>;
+ clock-ranges;
+ ranges;
+
+ /* all timings in nanoseconds */
+ ti,davinci-cs-ta = <12>;
+ ti,davinci-cs-rhold = <6>;
+ ti,davinci-cs-rstrobe = <23>;
+ ti,davinci-cs-rsetup = <9>;
+ ti,davinci-cs-whold = <8>;
+ ti,davinci-cs-wstrobe = <23>;
+ ti,davinci-cs-wsetup = <8>;
+
+ nand@0,0 {
+ compatible = "ti,keystone-nand";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ reg = <0 0 0x4000000
+ 1 0 0x0000100>;
+
+ ti,davinci-chipselect = <0>;
+ ti,davinci-mask-ale = <0x2000>;
+ ti,davinci-mask-cle = <0x4000>;
+ ti,davinci-mask-chipsel = <0>;
+ nand-ecc-mode = "hw";
+ ti,davinci-ecc-bits = <4>;
+ nand-on-flash-bbt;
+
+ partition@0 {
+ label = "u-boot";
+ reg = <0x0 0x100000>;
+ read-only;
+ };
+
+ partition@100000 {
+ label = "params";
+ reg = <0x100000 0x80000>;
+ read-only;
+ };
+
+ partition@180000 {
+ label = "ubifs";
+ reg = <0x180000 0x7E80000>;
+ };
+ };
+ };
+ };
};
};
--
1.7.9.5
^ permalink raw reply related [flat|nested] 56+ messages in thread
* Re: [PATCH 01/12] mtd: nand: davinci: fix driver registration
2013-11-11 16:52 ` [PATCH 01/12] mtd: nand: davinci: fix driver registration Khoronzhuk, Ivan
@ 2013-11-12 15:45 ` Santosh Shilimkar
0 siblings, 0 replies; 56+ messages in thread
From: Santosh Shilimkar @ 2013-11-12 15:45 UTC (permalink / raw)
To: Khoronzhuk, Ivan
Cc: Mark Rutland, devicetree, Strashko, Grygorii, Russell King,
Pawel Moll, Stephen Warren, Ian Campbell, Kumar Gala,
Rob Herring, linux-kernel, linux-mtd, Rob Landley,
linux-arm-kernel
On Monday 11 November 2013 11:52 AM, Khoronzhuk, Ivan wrote:
>
> When kernel is booted using DT, there is no guarantee that Davinci
> NAND device has been created already at the time when driver init
> function is executed. Therefore, platform_driver_probe() can't be used
> because this may result the Davinci NAND driver will never be probed.
> The driver probing has to be made with core mechanism.
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
> ---
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH 02/12] mtd: nand: davinci: check required ti, davinci-chipselect property
2013-11-11 16:53 ` [PATCH 02/12] mtd: nand: davinci: check required ti,davinci-chipselect property Khoronzhuk, Ivan
@ 2013-11-12 15:50 ` Santosh Shilimkar
2013-11-18 18:25 ` ivan.khoronzhuk
0 siblings, 1 reply; 56+ messages in thread
From: Santosh Shilimkar @ 2013-11-12 15:50 UTC (permalink / raw)
To: Khoronzhuk, Ivan
Cc: Mark Rutland, devicetree, Strashko, Grygorii, Russell King,
Pawel Moll, Stephen Warren, Ian Campbell, Kumar Gala,
Rob Herring, linux-kernel, linux-mtd, Rob Landley,
linux-arm-kernel
On Monday 11 November 2013 11:53 AM, Khoronzhuk, Ivan wrote:
> The property "ti,davinci-chipselect" is required. So we have to check
> if it is set.
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
> ---
> drivers/mtd/nand/davinci_nand.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
> index d87213f..8e1c88e 100644
> --- a/drivers/mtd/nand/davinci_nand.c
> +++ b/drivers/mtd/nand/davinci_nand.c
> @@ -541,10 +541,14 @@ static struct davinci_nand_pdata
> GFP_KERNEL);
> pdev->dev.platform_data = pdata;
> if (!pdata)
> - return NULL;
> + return ERR_PTR(-ENOMEM);
> +
This change don't follow commit message.
> if (!of_property_read_u32(pdev->dev.of_node,
> "ti,davinci-chipselect", &prop))
> pdev->id = prop;
> + else
> + return ERR_PTR(-EINVAL);
> +
So the check already exist but the error case wasn't handled.
This should be reflected in change log.
> if (!of_property_read_u32(pdev->dev.of_node,
> "ti,davinci-mask-ale", &prop))
> pdata->mask_ale = prop;
> @@ -598,6 +602,10 @@ static int __init nand_davinci_probe(struct platform_device *pdev)
> nand_ecc_modes_t ecc_mode;
>
> pdata = nand_davinci_get_pdata(pdev);
> + if (IS_ERR(pdata)) {
> + return PTR_ERR(pdata);
> + }
> +
Again not related to commit log. You might want to split this patch
then.
Regards,
Santosh
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH 03/12] mtd: nand: davinci: simplify error handling
2013-11-11 16:54 ` [PATCH 03/12] mtd: nand: davinci: simplify error handling Khoronzhuk, Ivan
@ 2013-11-12 15:52 ` Santosh Shilimkar
0 siblings, 0 replies; 56+ messages in thread
From: Santosh Shilimkar @ 2013-11-12 15:52 UTC (permalink / raw)
To: Khoronzhuk, Ivan
Cc: Mark Rutland, devicetree, Strashko, Grygorii, Russell King,
Pawel Moll, Stephen Warren, Ian Campbell, Kumar Gala,
Rob Herring, linux-kernel, linux-mtd, Rob Landley,
linux-arm-kernel
On Monday 11 November 2013 11:54 AM, Khoronzhuk, Ivan wrote:
> There is not needed to use a lot of names for err handling.
> It complicates code support and reading.
>
This is not always true but looking at the patch, the labels
are just useless since no special handling per label.
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
> ---
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH 04/12] mtd: nand: davinci: move bindings under mtd
2013-11-11 16:55 ` [PATCH 04/12] mtd: nand: davinci: move bindings under mtd Khoronzhuk, Ivan
@ 2013-11-12 15:53 ` Santosh Shilimkar
0 siblings, 0 replies; 56+ messages in thread
From: Santosh Shilimkar @ 2013-11-12 15:53 UTC (permalink / raw)
To: Khoronzhuk, Ivan
Cc: Mark Rutland, devicetree, Strashko, Grygorii, Russell King,
Pawel Moll, Stephen Warren, Ian Campbell, Kumar Gala,
Rob Herring, linux-kernel, linux-mtd, Rob Landley,
linux-arm-kernel
On Monday 11 November 2013 11:55 AM, Khoronzhuk, Ivan wrote:
> Move bindings under mtd. Do this in order to make davinci-nand
> driver usable by keystone architecture.
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
> ---
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH 05/12] mtd: nand: davinci: extend description of bindings
2013-11-11 16:58 ` [PATCH 05/12] mtd: nand: davinci: extend description of bindings Khoronzhuk, Ivan
@ 2013-11-12 15:55 ` Santosh Shilimkar
0 siblings, 0 replies; 56+ messages in thread
From: Santosh Shilimkar @ 2013-11-12 15:55 UTC (permalink / raw)
To: Khoronzhuk, Ivan
Cc: Mark Rutland, devicetree, Strashko, Grygorii, Russell King,
Pawel Moll, Stephen Warren, Ian Campbell, Kumar Gala,
Rob Herring, linux-kernel, linux-mtd, Rob Landley,
linux-arm-kernel
On Monday 11 November 2013 11:58 AM, Khoronzhuk, Ivan wrote:
> Extend bindings for davinci_nand driver to be more clear.
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
> ---
Looks fine to me but it needs blessing from DT guys.
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH 07/12] memory: davinci-aemif: introduce AEMIF driver
2013-11-11 17:06 ` [PATCH 07/12] memory: davinci-aemif: introduce AEMIF driver Khoronzhuk, Ivan
@ 2013-11-12 16:08 ` Santosh Shilimkar
2013-11-18 19:15 ` ivan.khoronzhuk
2013-11-26 7:20 ` Sekhar Nori
1 sibling, 1 reply; 56+ messages in thread
From: Santosh Shilimkar @ 2013-11-12 16:08 UTC (permalink / raw)
To: Khoronzhuk, Ivan
Cc: Mark Rutland, devicetree, Strashko, Grygorii, Russell King,
Pawel Moll, Stephen Warren, Greg Kroah-Hartman, Ian Campbell,
Kumar Gala, Rob Herring, linux-kernel, linux-mtd, Rob Landley,
linux-arm-kernel
+ Greg KH (drivers/memory/* patches goes through his queue)
On Monday 11 November 2013 12:06 PM, Khoronzhuk, Ivan wrote:
> Add new AEMIF driver for EMIF16 davinci controller. The EMIF16 module
> is intended to provide a glue-less interface to a variety of
> asynchronous memory devices like ASRA M, NOR and NAND memory. A total
> of 256M bytes of any of these memories can be accessed at any given
> time via four chip selects with 64M byte access per chip select.
>
> Synchronous memories such as DDR1 SD RAM, SDR SDRAM and Mobile SDR
> are not supported.
>
> See http://www.ti.com/lit/ug/sprugz3a/sprugz3a.pdf
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
> ---
> drivers/memory/Kconfig | 11 ++
> drivers/memory/Makefile | 1 +
> drivers/memory/davinci-aemif.c | 415 ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 427 insertions(+)
> create mode 100644 drivers/memory/davinci-aemif.c
>
> diff --git a/drivers/memory/Kconfig b/drivers/memory/Kconfig
> index 29a11db..010e75e 100644
> --- a/drivers/memory/Kconfig
> +++ b/drivers/memory/Kconfig
> @@ -7,6 +7,17 @@ menuconfig MEMORY
>
> if MEMORY
>
> +config TI_DAVINCI_AEMIF
s/TI_DAVINCI_AEMIF/TI_AEMIF
> + bool "Texas Instruments DaVinci AEMIF driver"
Drop DaVinci above since its used on more SOCs.
> + depends on (ARCH_DAVINCI || ARCH_KEYSTONE) && OF
> + help
> + This driver is for the AEMIF module available in Texas Instruments
> + SoCs. AEMIF stands for Asynchronous External Memory Interface and
> + is intended to provide a glue-less interface to a variety of
> + asynchronuous memory devices like ASRAM, NOR and NAND memory. A total
> + of 256M bytes of any of these memories can be accessed at a given
> + time via four chip selects with 64M byte access per chip select.
> +
> config TI_EMIF
> tristate "Texas Instruments EMIF driver"
> depends on ARCH_OMAP2PLUS
> diff --git a/drivers/memory/Makefile b/drivers/memory/Makefile
> index 969d923..af14126 100644
> --- a/drivers/memory/Makefile
> +++ b/drivers/memory/Makefile
> @@ -5,6 +5,7 @@
> ifeq ($(CONFIG_DDR),y)
> obj-$(CONFIG_OF) += of_memory.o
> endif
> +obj-$(CONFIG_TI_DAVINCI_AEMIF) += davinci-aemif.o
Change this accordingly once the config is renamed.
> obj-$(CONFIG_TI_EMIF) += emif.o
> obj-$(CONFIG_MVEBU_DEVBUS) += mvebu-devbus.o
> obj-$(CONFIG_TEGRA20_MC) += tegra20-mc.o
> diff --git a/drivers/memory/davinci-aemif.c b/drivers/memory/davinci-aemif.c
> new file mode 100644
> index 0000000..e36b74b
> --- /dev/null
> +++ b/drivers/memory/davinci-aemif.c
> @@ -0,0 +1,415 @@
> +/*
> + * DaVinci/Keystone AEMIF driver
s/{DaVinci/Keystone}/TI
> + *
> + * Copyright (C) 2010 - 2013 Texas Instruments Incorporated. http://www.ti.com/
> + * Copyright (C) Heiko Schocher <hs@denx.de>
> + * Copyright (C) Ivan Khoronzhuk <ivan.khoronzhuk@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.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +
> +#define TA_SHIFT 2
> +#define RHOLD_SHIFT 4
> +#define RSTROBE_SHIFT 7
> +#define RSETUP_SHIFT 13
> +#define WHOLD_SHIFT 17
> +#define WSTROBE_SHIFT 20
> +#define WSETUP_SHIFT 26
> +#define EW_SHIFT 30
> +#define SS_SHIFT 31
> +
> +#define TA(x) ((x) << TA_SHIFT)
> +#define RHOLD(x) ((x) << RHOLD_SHIFT)
> +#define RSTROBE(x) ((x) << RSTROBE_SHIFT)
> +#define RSETUP(x) ((x) << RSETUP_SHIFT)
> +#define WHOLD(x) ((x) << WHOLD_SHIFT)
> +#define WSTROBE(x) ((x) << WSTROBE_SHIFT)
> +#define WSETUP(x) ((x) << WSETUP_SHIFT)
> +#define EW(x) ((x) << EW_SHIFT)
> +#define SS(x) ((x) << SS_SHIFT)
> +
> +#define ASIZE_MAX 0x1
> +#define TA_MAX 0x3
> +#define RHOLD_MAX 0x7
> +#define RSTROBE_MAX 0x3f
> +#define RSETUP_MAX 0xf
> +#define WHOLD_MAX 0x7
> +#define WSTROBE_MAX 0x3f
> +#define WSETUP_MAX 0xf
> +#define EW_MAX 0x1
> +#define SS_MAX 0x1
> +#define NUM_CS 4
> +
> +#define TA_VAL(x) (((x) & TA(TA_MAX)) >> TA_SHIFT)
> +#define RHOLD_VAL(x) (((x) & RHOLD(RHOLD_MAX)) >> RHOLD_SHIFT)
> +#define RSTROBE_VAL(x) (((x) & RSTROBE(RSTROBE_MAX)) >> RSTROBE_SHIFT)
> +#define RSETUP_VAL(x) (((x) & RSETUP(RSETUP_MAX)) >> RSETUP_SHIFT)
> +#define WHOLD_VAL(x) (((x) & WHOLD(WHOLD_MAX)) >> WHOLD_SHIFT)
> +#define WSTROBE_VAL(x) (((x) & WSTROBE(WSTROBE_MAX)) >> WSTROBE_SHIFT)
> +#define WSETUP_VAL(x) (((x) & WSETUP(WSETUP_MAX)) >> WSETUP_SHIFT)
> +#define EW_VAL(x) (((x) & EW(EW_MAX)) >> EW_SHIFT)
> +#define SS_VAL(x) (((x) & SS(SS_MAX)) >> SS_SHIFT)
> +
> +#define NRCSR_OFFSET 0x00
> +#define AWCCR_OFFSET 0x04
> +#define A1CR_OFFSET 0x10
> +
> +#define ACR_ASIZE_MASK 0x3
> +#define ACR_EW_MASK BIT(30)
> +#define ACR_SS_MASK BIT(31)
> +#define ASIZE_16BIT 1
> +
> +#define CONFIG_MASK (TA(TA_MAX) | \
> + RHOLD(RHOLD_MAX) | \
> + RSTROBE(RSTROBE_MAX) | \
> + RSETUP(RSETUP_MAX) | \
> + WHOLD(WHOLD_MAX) | \
> + WSTROBE(WSTROBE_MAX) | \
> + WSETUP(WSETUP_MAX) | \
> + EW(EW_MAX) | SS(SS_MAX) | \
> + ASIZE_MAX)
> +
> +#define DRV_NAME "davinci-aemif"
s/davinci-aemif/ti-aemif
> +
> +/**
> + * structure to hold cs parameters
> + * @cs: chip-select number
> + * @wstrobe: write strobe width, ns
> + * @rstrobe: read strobe width, ns
> + * @wsetup: write setup width, ns
> + * @whold: write hold width, ns
> + * @rsetup: read setup width, ns
> + * @rhold: read hold width, ns
> + * @ta: minimum turn around time, ns
> + * @enable_ss: enable/disable select strobe mode
> + * @enable_ew: enable/disable extended wait mode
> + * @asize: width of the asynchronous device's data bus
> + */
> +struct davinci_aemif_cs_data {
> + u8 cs;
> + u16 wstrobe;
> + u16 rstrobe;
> + u8 wsetup;
> + u8 whold;
> + u8 rsetup;
> + u8 rhold;
> + u8 ta;
> + u8 enable_ss;
> + u8 enable_ew;
> + u8 asize;
> +};
> +
I suggest you drop davinci prefix throughout the
driver.
[..]
> +static const struct of_device_id davinci_aemif_of_match[] = {
> + { .compatible = "ti,davinci-aemif", },
> + { .compatible = "ti,keystone-aemif", },
> + { .compatible = "ti,omap-L138-aemif", },
> + {},
> +};
Do we need three seperate compatible or "ti,aemif" should
be enough ? Are there differences which we need to handled
based on compatibles ?
> +
> +static const struct of_device_id davinci_cs_of_match[] = {
> + { .compatible = "ti,davinci-cs", },
> + {},
> +};
> +
> +static int davinci_aemif_probe(struct platform_device *pdev)
> +{
> + int ret = -ENODEV, i;
> + struct resource *res;
> + struct device *dev = &pdev->dev;
> + struct device_node *np = dev->of_node;
> +
> + if (np == NULL)
> + return 0;
> +
> + if (aemif) {
> + dev_err(dev, "davinci_aemif driver is in use currently\n");
> + return -EBUSY;
> + }
> +
> + aemif = devm_kzalloc(dev, sizeof(*aemif), GFP_KERNEL);
> +
Drop this extra line.
> + if (!aemif) {
> + dev_err(dev, "cannot allocate memory for aemif\n");
> + return -ENOMEM;
> + }
> +
> + aemif->clk = devm_clk_get(dev, "aemif");
> + if (IS_ERR(aemif->clk)) {
> + dev_err(dev, "cannot get clock 'aemif'\n");
> + return PTR_ERR(aemif->clk);
> + }
> +
> + clk_prepare_enable(aemif->clk);
> + aemif->clk_rate = clk_get_rate(aemif->clk) / 1000;
> +
1000 divider ? Define a marco and use it to be clear.
Other than above comments patch looks fine to me.
Regards,
Santosh
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH 09/12] mtd: nand: davinci: reuse driver for Keystone arch
2013-11-11 17:09 ` [PATCH 09/12] mtd: nand: davinci: reuse driver for Keystone arch Khoronzhuk, Ivan
@ 2013-11-12 16:09 ` Santosh Shilimkar
0 siblings, 0 replies; 56+ messages in thread
From: Santosh Shilimkar @ 2013-11-12 16:09 UTC (permalink / raw)
To: Khoronzhuk, Ivan
Cc: Mark Rutland, devicetree, Strashko, Grygorii, Russell King,
Pawel Moll, Stephen Warren, Ian Campbell, Kumar Gala,
Rob Herring, linux-kernel, linux-mtd, Rob Landley,
linux-arm-kernel
On Monday 11 November 2013 12:09 PM, Khoronzhuk, Ivan wrote:
> The Keystone arch has compatible nand device, so reuse it.
> In case with Keystone it depends on TI_DAVINCI_AEMIF because AEMIF
> driver is responsible to set timings.
>
> See http://www.ti.com/lit/ug/sprugz3a/sprugz3a.pdf
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
> ---
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH 10/12] mtd: nand: davinci: don't set timings if AEMIF is used
2013-11-11 17:10 ` [PATCH 10/12] mtd: nand: davinci: don't set timings if AEMIF is used Khoronzhuk, Ivan
@ 2013-11-12 16:13 ` Santosh Shilimkar
2013-11-18 11:35 ` Grygorii Strashko
2013-11-13 5:02 ` Sekhar Nori
1 sibling, 1 reply; 56+ messages in thread
From: Santosh Shilimkar @ 2013-11-12 16:13 UTC (permalink / raw)
To: Khoronzhuk, Ivan
Cc: Mark Rutland, devicetree, Strashko, Grygorii, Russell King,
Pawel Moll, Stephen Warren, Ian Campbell, Kumar Gala,
Rob Herring, linux-kernel, linux-mtd, Rob Landley,
linux-arm-kernel
On Monday 11 November 2013 12:10 PM, Khoronzhuk, Ivan wrote:
> If Davinci AEMIF is used we don't need to set timings and bus width.
> It is done by AEMIF driver (drivers/memory/davinci-aemfi.c).
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
> ---
> drivers/mtd/nand/davinci_nand.c | 22 +++++++++++++++-------
> 1 file changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
> index 4705214..879e915 100644
> --- a/drivers/mtd/nand/davinci_nand.c
> +++ b/drivers/mtd/nand/davinci_nand.c
> @@ -742,27 +742,35 @@ static int __init nand_davinci_probe(struct platform_device *pdev)
> goto err_clk_enable;
> }
>
> +#if !IS_ENABLED(CONFIG_TI_DAVINCI_AEMIF)
>
Instead above #if, just use a variable.
bool aemif = IS_ENABLED(CONFIG_TI_DAVINCI_AEMIF) and then skip
the below code. #if block in the middle of the code looks ugly.
Other than that patch looks fine to me.
Regards,
Santosh
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH 11/12] mtd: nand: davinci: don't request AEMIF address range
2013-11-11 17:12 ` [PATCH 11/12] mtd: nand: davinci: don't request AEMIF address range Khoronzhuk, Ivan
@ 2013-11-12 16:15 ` Santosh Shilimkar
0 siblings, 0 replies; 56+ messages in thread
From: Santosh Shilimkar @ 2013-11-12 16:15 UTC (permalink / raw)
To: Khoronzhuk, Ivan
Cc: Mark Rutland, devicetree, Strashko, Grygorii, Russell King,
Pawel Moll, Stephen Warren, Ian Campbell, Kumar Gala,
Rob Herring, linux-kernel, linux-mtd, Rob Landley,
linux-arm-kernel
On Monday 11 November 2013 12:12 PM, Khoronzhuk, Ivan wrote:
> The AEMIF driver registers are used to setup timings for each chip
> select. The same registers range is used to setup NAND settings.
> The AEMIF and NAND drivers not use the same registers in this range.
>
> In case with AEMIF driver, the memory address range is requested
> already by AEMIF, so we cannot request it twice, just ioremap.
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
> ---
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH 12/12] arm: dts: keystone: add AEMIF/NAND device entry
2013-11-11 17:13 ` [PATCH 12/12] arm: dts: keystone: add AEMIF/NAND device entry Khoronzhuk, Ivan
@ 2013-11-12 16:19 ` Santosh Shilimkar
0 siblings, 0 replies; 56+ messages in thread
From: Santosh Shilimkar @ 2013-11-12 16:19 UTC (permalink / raw)
To: Khoronzhuk, Ivan
Cc: Mark Rutland, devicetree, Strashko, Grygorii, Russell King,
Pawel Moll, Stephen Warren, Ian Campbell, Kumar Gala,
Rob Herring, linux-kernel, linux-mtd, Rob Landley,
linux-arm-kernel
On Monday 11 November 2013 12:13 PM, Khoronzhuk, Ivan wrote:
> Add AEMIF/NAND device entry.
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
> ---
> arch/arm/boot/dts/keystone.dts | 63 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 63 insertions(+)
>
> diff --git a/arch/arm/boot/dts/keystone.dts b/arch/arm/boot/dts/keystone.dts
> index 100bdf5..998da98 100644
> --- a/arch/arm/boot/dts/keystone.dts
> +++ b/arch/arm/boot/dts/keystone.dts
> @@ -179,5 +179,68 @@
> interrupts = <GIC_SPI 300 IRQ_TYPE_EDGE_RISING>;
> clocks = <&clkspi>;
> };
> +
> + aemif@021000A00 {
> + compatible = "ti,keystone-aemif";
> + #address-cells = <2>;
> + #size-cells = <1>;
> + clocks = <&clkaemif>;
> + clock-names = "aemif";
> + clock-ranges;
> +
> + reg = <0x2100A00 0x00000100>;
> + ranges = <0 0 0x30000000 0x10000000
> + 1 0 0x21000A00 0x00000100>;
> +
> + nand:cs0 {
> + compatible = "ti,davinci-cs";
> + #address-cells = <2>;
> + #size-cells = <1>;
> + clock-ranges;
> + ranges;
> +
> + /* all timings in nanoseconds */
> + ti,davinci-cs-ta = <12>;
> + ti,davinci-cs-rhold = <6>;
> + ti,davinci-cs-rstrobe = <23>;
> + ti,davinci-cs-rsetup = <9>;
> + ti,davinci-cs-whold = <8>;
> + ti,davinci-cs-wstrobe = <23>;
> + ti,davinci-cs-wsetup = <8>;
> +
> + nand@0,0 {
> + compatible = "ti,keystone-nand";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + reg = <0 0 0x4000000
> + 1 0 0x0000100>;
> +
> + ti,davinci-chipselect = <0>;
> + ti,davinci-mask-ale = <0x2000>;
> + ti,davinci-mask-cle = <0x4000>;
> + ti,davinci-mask-chipsel = <0>;
> + nand-ecc-mode = "hw";
> + ti,davinci-ecc-bits = <4>;
> + nand-on-flash-bbt;
> +
> + partition@0 {
> + label = "u-boot";
> + reg = <0x0 0x100000>;
> + read-only;
> + };
> +
> + partition@100000 {
> + label = "params";
> + reg = <0x100000 0x80000>;
> + read-only;
> + };
> +
> + partition@180000 {
> + label = "ubifs";
> + reg = <0x180000 0x7E80000>;
> + };
Lets create now board specific k2hk-evm.dts and rename keystone.dts
to keystone.dtsi and include that in boards. That way you can add the
board specific stuff in board files and common soc stuff in keystone.dtsi.
You can update that once the bindings are blessed by DT folks.
Regards,
Santosh
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH 10/12] mtd: nand: davinci: don't set timings if AEMIF is used
2013-11-11 17:10 ` [PATCH 10/12] mtd: nand: davinci: don't set timings if AEMIF is used Khoronzhuk, Ivan
2013-11-12 16:13 ` Santosh Shilimkar
@ 2013-11-13 5:02 ` Sekhar Nori
2013-11-13 14:14 ` Santosh Shilimkar
1 sibling, 1 reply; 56+ messages in thread
From: Sekhar Nori @ 2013-11-13 5:02 UTC (permalink / raw)
To: Khoronzhuk, Ivan, Shilimkar, Santosh, Rob Landley, Russell King
Cc: Mark Rutland, devicetree, Strashko, Grygorii, Pawel Moll,
Ian Campbell, Stephen Warren, Kumar Gala, Rob Herring,
linux-kernel, linux-mtd, linux-arm-kernel
On Monday 11 November 2013 10:40 PM, Khoronzhuk, Ivan wrote:
> If Davinci AEMIF is used we don't need to set timings and bus width.
> It is done by AEMIF driver (drivers/memory/davinci-aemfi.c).
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
> ---
> drivers/mtd/nand/davinci_nand.c | 22 +++++++++++++++-------
> 1 file changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
> index 4705214..879e915 100644
> --- a/drivers/mtd/nand/davinci_nand.c
> +++ b/drivers/mtd/nand/davinci_nand.c
> @@ -742,27 +742,35 @@ static int __init nand_davinci_probe(struct platform_device *pdev)
> goto err_clk_enable;
> }
>
> +#if !IS_ENABLED(CONFIG_TI_DAVINCI_AEMIF)
This is a hack! Just because AEMIF driver is enabled, it does not
guarantee that the timings have been setup by it. Instead of configuring
timings in two drivers, why not just convert everyone over to use the
new driver. Dont worry about breaking old platforms, I will help test
and ack them.
Thanks,
Sekhar
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH 10/12] mtd: nand: davinci: don't set timings if AEMIF is used
2013-11-13 5:02 ` Sekhar Nori
@ 2013-11-13 14:14 ` Santosh Shilimkar
2013-11-14 10:53 ` Sekhar Nori
0 siblings, 1 reply; 56+ messages in thread
From: Santosh Shilimkar @ 2013-11-13 14:14 UTC (permalink / raw)
To: Sekhar Nori
Cc: Mark Rutland, devicetree, Strashko, Grygorii, Russell King,
Pawel Moll, Stephen Warren, Ian Campbell, Kumar Gala,
Rob Herring, linux-kernel, linux-mtd, Rob Landley, Khoronzhuk,
Ivan, linux-arm-kernel
On Wednesday 13 November 2013 12:02 AM, Sekhar Nori wrote:
> On Monday 11 November 2013 10:40 PM, Khoronzhuk, Ivan wrote:
>> If Davinci AEMIF is used we don't need to set timings and bus width.
>> It is done by AEMIF driver (drivers/memory/davinci-aemfi.c).
>>
>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
>> ---
>> drivers/mtd/nand/davinci_nand.c | 22 +++++++++++++++-------
>> 1 file changed, 15 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
>> index 4705214..879e915 100644
>> --- a/drivers/mtd/nand/davinci_nand.c
>> +++ b/drivers/mtd/nand/davinci_nand.c
>> @@ -742,27 +742,35 @@ static int __init nand_davinci_probe(struct platform_device *pdev)
>> goto err_clk_enable;
>> }
>>
>> +#if !IS_ENABLED(CONFIG_TI_DAVINCI_AEMIF)
>
> This is a hack! Just because AEMIF driver is enabled, it does not
> guarantee that the timings have been setup by it. Instead of configuring
> timings in two drivers, why not just convert everyone over to use the
> new driver. Dont worry about breaking old platforms, I will help test
> and ack them.
>
How about you take a stab at and convert the DaVinci code over to make
use of new driver. We are trying to re-use as much as possible from the
common drivers and also making an option so that the DaVinci arch can
move over to these drivers if they want to.
Thanks.
Regards,
Santosh
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH 10/12] mtd: nand: davinci: don't set timings if AEMIF is used
2013-11-13 14:14 ` Santosh Shilimkar
@ 2013-11-14 10:53 ` Sekhar Nori
2013-11-14 14:36 ` Santosh Shilimkar
0 siblings, 1 reply; 56+ messages in thread
From: Sekhar Nori @ 2013-11-14 10:53 UTC (permalink / raw)
To: Santosh Shilimkar, Khoronzhuk, Ivan
Cc: Mark Rutland, devicetree, Strashko, Grygorii, Russell King,
Pawel Moll, Stephen Warren, Ian Campbell, Kumar Gala,
Rob Herring, linux-kernel, linux-mtd, Rob Landley,
linux-arm-kernel
On Wednesday 13 November 2013 07:44 PM, Santosh Shilimkar wrote:
> On Wednesday 13 November 2013 12:02 AM, Sekhar Nori wrote:
>> On Monday 11 November 2013 10:40 PM, Khoronzhuk, Ivan wrote:
>>> If Davinci AEMIF is used we don't need to set timings and bus width.
>>> It is done by AEMIF driver (drivers/memory/davinci-aemfi.c).
>>>
>>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
>>> ---
>>> drivers/mtd/nand/davinci_nand.c | 22 +++++++++++++++-------
>>> 1 file changed, 15 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
>>> index 4705214..879e915 100644
>>> --- a/drivers/mtd/nand/davinci_nand.c
>>> +++ b/drivers/mtd/nand/davinci_nand.c
>>> @@ -742,27 +742,35 @@ static int __init nand_davinci_probe(struct platform_device *pdev)
>>> goto err_clk_enable;
>>> }
>>>
>>> +#if !IS_ENABLED(CONFIG_TI_DAVINCI_AEMIF)
>>
>> This is a hack! Just because AEMIF driver is enabled, it does not
>> guarantee that the timings have been setup by it. Instead of configuring
>> timings in two drivers, why not just convert everyone over to use the
>> new driver. Dont worry about breaking old platforms, I will help test
>> and ack them.
>>
> How about you take a stab at and convert the DaVinci code over to make
> use of new driver. We are trying to re-use as much as possible from the
> common drivers and also making an option so that the DaVinci arch can
> move over to these drivers if they want to.
Sure I could.
Ivan,
The AEMIF driver does not use platfrom_data currently. Is that something
you can add or you want me to take a stab at that as well?
Thanks,
Sekhar
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH 10/12] mtd: nand: davinci: don't set timings if AEMIF is used
2013-11-14 10:53 ` Sekhar Nori
@ 2013-11-14 14:36 ` Santosh Shilimkar
2013-11-18 19:35 ` ivan.khoronzhuk
2013-11-21 17:07 ` Sekhar Nori
0 siblings, 2 replies; 56+ messages in thread
From: Santosh Shilimkar @ 2013-11-14 14:36 UTC (permalink / raw)
To: Nori, Sekhar
Cc: Mark Rutland, devicetree, Strashko, Grygorii, Russell King,
Pawel Moll, Stephen Warren, Ian Campbell, Kumar Gala,
Rob Herring, linux-kernel, linux-mtd, Rob Landley, Khoronzhuk,
Ivan, linux-arm-kernel
On Thursday 14 November 2013 05:53 AM, Nori, Sekhar wrote:
> On Wednesday 13 November 2013 07:44 PM, Santosh Shilimkar wrote:
>> On Wednesday 13 November 2013 12:02 AM, Sekhar Nori wrote:
>>> On Monday 11 November 2013 10:40 PM, Khoronzhuk, Ivan wrote:
>>>> If Davinci AEMIF is used we don't need to set timings and bus width.
>>>> It is done by AEMIF driver (drivers/memory/davinci-aemfi.c).
>>>>
>>>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
>>>> ---
>>>> drivers/mtd/nand/davinci_nand.c | 22 +++++++++++++++-------
>>>> 1 file changed, 15 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
>>>> index 4705214..879e915 100644
>>>> --- a/drivers/mtd/nand/davinci_nand.c
>>>> +++ b/drivers/mtd/nand/davinci_nand.c
>>>> @@ -742,27 +742,35 @@ static int __init nand_davinci_probe(struct platform_device *pdev)
>>>> goto err_clk_enable;
>>>> }
>>>>
>>>> +#if !IS_ENABLED(CONFIG_TI_DAVINCI_AEMIF)
>>>
>>> This is a hack! Just because AEMIF driver is enabled, it does not
>>> guarantee that the timings have been setup by it. Instead of configuring
>>> timings in two drivers, why not just convert everyone over to use the
>>> new driver. Dont worry about breaking old platforms, I will help test
>>> and ack them.
>>>
>> How about you take a stab at and convert the DaVinci code over to make
>> use of new driver. We are trying to re-use as much as possible from the
>> common drivers and also making an option so that the DaVinci arch can
>> move over to these drivers if they want to.
>
> Sure I could.
>
Thanks
> Ivan,
>
> The AEMIF driver does not use platfrom_data currently. Is that something
> you can add or you want me to take a stab at that as well?
>
The AEMIF new driver is device tree only and thats the direction. So the
better thing would be to convert Davinci to DT and then make use of this
new driver. Thats how most of the new drivers used on arm socs are
moving. Adding platform data to new driver is a step in backward
direction so I would want to avoid that.
Once you start using new driver the $subject patch code skipping
is trivial. Till then I would like to proceed with current approach
which allows Nand support for Keystone.
Regards,
Santosh
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH 10/12] mtd: nand: davinci: don't set timings if AEMIF is used
2013-11-12 16:13 ` Santosh Shilimkar
@ 2013-11-18 11:35 ` Grygorii Strashko
2013-11-18 14:08 ` Santosh Shilimkar
0 siblings, 1 reply; 56+ messages in thread
From: Grygorii Strashko @ 2013-11-18 11:35 UTC (permalink / raw)
To: Santosh Shilimkar, Khoronzhuk, Ivan
Cc: Mark Rutland, devicetree, Russell King, Pawel Moll,
Stephen Warren, Ian Campbell, Sekhar Nori, Kumar Gala,
Rob Herring, linux-kernel, linux-mtd, Rob Landley,
linux-arm-kernel
On 11/12/2013 06:13 PM, Santosh Shilimkar wrote:
> On Monday 11 November 2013 12:10 PM, Khoronzhuk, Ivan wrote:
>> If Davinci AEMIF is used we don't need to set timings and bus width.
>> It is done by AEMIF driver (drivers/memory/davinci-aemfi.c).
>>
>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
>> ---
>> drivers/mtd/nand/davinci_nand.c | 22 +++++++++++++++-------
>> 1 file changed, 15 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
>> index 4705214..879e915 100644
>> --- a/drivers/mtd/nand/davinci_nand.c
>> +++ b/drivers/mtd/nand/davinci_nand.c
>> @@ -742,27 +742,35 @@ static int __init nand_davinci_probe(struct platform_device *pdev)
>> goto err_clk_enable;
>> }
>>
>> +#if !IS_ENABLED(CONFIG_TI_DAVINCI_AEMIF)
>>
> Instead above #if, just use a variable.
> bool aemif = IS_ENABLED(CONFIG_TI_DAVINCI_AEMIF) and then skip
> the below code. #if block in the middle of the code looks ugly.
Yes, this is the hack.
The problem is that this part of code contains the call of Davinci
platform function davinci_aemif_setup_timing() which is not accessible
if Kernel is built for Keystone only.
That's why "#if" has been used.
This part of code has to be removed together with Davinci aemif platform
code (aemif.c), once Davinci will be converted to DT and use
new driver.
The corresponding comment can be added here. Is it ok?
>
> Other than that patch looks fine to me.
>
> Regards,
> Santosh
>
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH 10/12] mtd: nand: davinci: don't set timings if AEMIF is used
2013-11-18 11:35 ` Grygorii Strashko
@ 2013-11-18 14:08 ` Santosh Shilimkar
2013-11-18 19:32 ` ivan.khoronzhuk
0 siblings, 1 reply; 56+ messages in thread
From: Santosh Shilimkar @ 2013-11-18 14:08 UTC (permalink / raw)
To: Grygorii Strashko
Cc: Mark Rutland, devicetree, Russell King, Pawel Moll,
Stephen Warren, Ian Campbell, Sekhar Nori, Kumar Gala,
Rob Herring, linux-kernel, linux-mtd, Rob Landley, Khoronzhuk,
Ivan, linux-arm-kernel
On Monday 18 November 2013 06:35 AM, Grygorii Strashko wrote:
> On 11/12/2013 06:13 PM, Santosh Shilimkar wrote:
>> On Monday 11 November 2013 12:10 PM, Khoronzhuk, Ivan wrote:
>>> If Davinci AEMIF is used we don't need to set timings and bus width.
>>> It is done by AEMIF driver (drivers/memory/davinci-aemfi.c).
>>>
>>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
>>> ---
>>> drivers/mtd/nand/davinci_nand.c | 22 +++++++++++++++-------
>>> 1 file changed, 15 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
>>> index 4705214..879e915 100644
>>> --- a/drivers/mtd/nand/davinci_nand.c
>>> +++ b/drivers/mtd/nand/davinci_nand.c
>>> @@ -742,27 +742,35 @@ static int __init nand_davinci_probe(struct platform_device *pdev)
>>> goto err_clk_enable;
>>> }
>>>
>>> +#if !IS_ENABLED(CONFIG_TI_DAVINCI_AEMIF)
>>>
>> Instead above #if, just use a variable.
>> bool aemif = IS_ENABLED(CONFIG_TI_DAVINCI_AEMIF) and then skip
>> the below code. #if block in the middle of the code looks ugly.
>
> Yes, this is the hack.
> The problem is that this part of code contains the call of Davinci platform function davinci_aemif_setup_timing() which is not accessible if Kernel is built for Keystone only.
> That's why "#if" has been used.
>
> This part of code has to be removed together with Davinci aemif platform code (aemif.c), once Davinci will be converted to DT and use
> new driver.
>
> The corresponding comment can be added here. Is it ok?
>
Thats more or less what I proposed in another reply. Lets clearly
document it on why such a check was added.
Regards,
Santosh
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH 02/12] mtd: nand: davinci: check required ti, davinci-chipselect property
2013-11-12 15:50 ` [PATCH 02/12] mtd: nand: davinci: check required ti, davinci-chipselect property Santosh Shilimkar
@ 2013-11-18 18:25 ` ivan.khoronzhuk
0 siblings, 0 replies; 56+ messages in thread
From: ivan.khoronzhuk @ 2013-11-18 18:25 UTC (permalink / raw)
To: Santosh Shilimkar
Cc: Mark Rutland, devicetree, Strashko, Grygorii, Russell King,
Pawel Moll, Stephen Warren, Ian Campbell, Kumar Gala,
Rob Herring, linux-kernel, linux-mtd, Rob Landley,
linux-arm-kernel
On 11/12/2013 05:50 PM, Santosh Shilimkar wrote:
> On Monday 11 November 2013 11:53 AM, Khoronzhuk, Ivan wrote:
>> The property "ti,davinci-chipselect" is required. So we have to check
>> if it is set.
>>
>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
>> ---
>> drivers/mtd/nand/davinci_nand.c | 10 +++++++++-
>> 1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
>> index d87213f..8e1c88e 100644
>> --- a/drivers/mtd/nand/davinci_nand.c
>> +++ b/drivers/mtd/nand/davinci_nand.c
>> @@ -541,10 +541,14 @@ static struct davinci_nand_pdata
>> GFP_KERNEL);
>> pdev->dev.platform_data = pdata;
>> if (!pdata)
>> - return NULL;
>> + return ERR_PTR(-ENOMEM);
>> +
> This change don't follow commit message.
>
I'll move it to separate patch, that will be first patch
>> if (!of_property_read_u32(pdev->dev.of_node,
>> "ti,davinci-chipselect", &prop))
>> pdev->id = prop;
>> + else
>> + return ERR_PTR(-EINVAL);
>> +
> So the check already exist but the error case wasn't handled.
> This should be reflected in change log.
>
The error will be handled at return by:
pdata = nand_davinci_get_pdata(pdev);
if (IS_ERR(pdata))
return PTR_ERR(pdata);
or did you mean I should add dev_err().
>> if (!of_property_read_u32(pdev->dev.of_node,
>> "ti,davinci-mask-ale", &prop))
>> pdata->mask_ale = prop;
>> @@ -598,6 +602,10 @@ static int __init nand_davinci_probe(struct platform_device *pdev)
>> nand_ecc_modes_t ecc_mode;
>>
>> pdata = nand_davinci_get_pdata(pdev);
>> + if (IS_ERR(pdata)) {
>> + return PTR_ERR(pdata);
>> + }
>> +
> Again not related to commit log. You might want to split this patch
> then.
>
I'll move it in the same separate patch.
> Regards,
> Santosh
>
--
Regards,
Ivan Khoronzhuk
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH 07/12] memory: davinci-aemif: introduce AEMIF driver
2013-11-12 16:08 ` Santosh Shilimkar
@ 2013-11-18 19:15 ` ivan.khoronzhuk
0 siblings, 0 replies; 56+ messages in thread
From: ivan.khoronzhuk @ 2013-11-18 19:15 UTC (permalink / raw)
To: Santosh Shilimkar
Cc: Mark Rutland, devicetree, Strashko, Grygorii, Russell King,
Pawel Moll, Stephen Warren, Greg Kroah-Hartman, Ian Campbell,
Kumar Gala, Rob Herring, linux-kernel, linux-mtd, Rob Landley,
linux-arm-kernel
On 11/12/2013 06:08 PM, Santosh Shilimkar wrote:
> + Greg KH (drivers/memory/* patches goes through his queue)
>
> On Monday 11 November 2013 12:06 PM, Khoronzhuk, Ivan wrote:
>> Add new AEMIF driver for EMIF16 davinci controller. The EMIF16 module
>> is intended to provide a glue-less interface to a variety of
>> asynchronous memory devices like ASRA M, NOR and NAND memory. A total
>> of 256M bytes of any of these memories can be accessed at any given
>> time via four chip selects with 64M byte access per chip select.
>>
>> Synchronous memories such as DDR1 SD RAM, SDR SDRAM and Mobile SDR
>> are not supported.
>>
>> See http://www.ti.com/lit/ug/sprugz3a/sprugz3a.pdf
>>
>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
>> ---
>> drivers/memory/Kconfig | 11 ++
>> drivers/memory/Makefile | 1 +
>> drivers/memory/davinci-aemif.c | 415 ++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 427 insertions(+)
>> create mode 100644 drivers/memory/davinci-aemif.c
>>
>> diff --git a/drivers/memory/Kconfig b/drivers/memory/Kconfig
>> index 29a11db..010e75e 100644
>> --- a/drivers/memory/Kconfig
>> +++ b/drivers/memory/Kconfig
>> @@ -7,6 +7,17 @@ menuconfig MEMORY
>>
>> if MEMORY
>>
>> +config TI_DAVINCI_AEMIF
> s/TI_DAVINCI_AEMIF/TI_AEMIF
>
Ok
>> + bool "Texas Instruments DaVinci AEMIF driver"
> Drop DaVinci above since its used on more SOCs.
>> + depends on (ARCH_DAVINCI || ARCH_KEYSTONE) && OF
>> + help
>> + This driver is for the AEMIF module available in Texas Instruments
>> + SoCs. AEMIF stands for Asynchronous External Memory Interface and
>> + is intended to provide a glue-less interface to a variety of
>> + asynchronuous memory devices like ASRAM, NOR and NAND memory. A total
>> + of 256M bytes of any of these memories can be accessed at a given
>> + time via four chip selects with 64M byte access per chip select.
>> +
>> config TI_EMIF
>> tristate "Texas Instruments EMIF driver"
>> depends on ARCH_OMAP2PLUS
>> diff --git a/drivers/memory/Makefile b/drivers/memory/Makefile
>> index 969d923..af14126 100644
>> --- a/drivers/memory/Makefile
>> +++ b/drivers/memory/Makefile
>> @@ -5,6 +5,7 @@
>> ifeq ($(CONFIG_DDR),y)
>> obj-$(CONFIG_OF) += of_memory.o
>> endif
>> +obj-$(CONFIG_TI_DAVINCI_AEMIF) += davinci-aemif.o
>
> Change this accordingly once the config is renamed.
>
Ok
>> obj-$(CONFIG_TI_EMIF) += emif.o
>> obj-$(CONFIG_MVEBU_DEVBUS) += mvebu-devbus.o
>> obj-$(CONFIG_TEGRA20_MC) += tegra20-mc.o
>> diff --git a/drivers/memory/davinci-aemif.c b/drivers/memory/davinci-aemif.c
>> new file mode 100644
>> index 0000000..e36b74b
>> --- /dev/null
>> +++ b/drivers/memory/davinci-aemif.c
>> @@ -0,0 +1,415 @@
>> +/*
>> + * DaVinci/Keystone AEMIF driver
> s/{DaVinci/Keystone}/TI
>> + *
>> + * Copyright (C) 2010 - 2013 Texas Instruments Incorporated. http://www.ti.com/
>> + * Copyright (C) Heiko Schocher <hs@denx.de>
>> + * Copyright (C) Ivan Khoronzhuk <ivan.khoronzhuk@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.
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/err.h>
>> +#include <linux/io.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/platform_device.h>
>> +
>> +#define TA_SHIFT 2
>> +#define RHOLD_SHIFT 4
>> +#define RSTROBE_SHIFT 7
>> +#define RSETUP_SHIFT 13
>> +#define WHOLD_SHIFT 17
>> +#define WSTROBE_SHIFT 20
>> +#define WSETUP_SHIFT 26
>> +#define EW_SHIFT 30
>> +#define SS_SHIFT 31
>> +
>> +#define TA(x) ((x) << TA_SHIFT)
>> +#define RHOLD(x) ((x) << RHOLD_SHIFT)
>> +#define RSTROBE(x) ((x) << RSTROBE_SHIFT)
>> +#define RSETUP(x) ((x) << RSETUP_SHIFT)
>> +#define WHOLD(x) ((x) << WHOLD_SHIFT)
>> +#define WSTROBE(x) ((x) << WSTROBE_SHIFT)
>> +#define WSETUP(x) ((x) << WSETUP_SHIFT)
>> +#define EW(x) ((x) << EW_SHIFT)
>> +#define SS(x) ((x) << SS_SHIFT)
>> +
>> +#define ASIZE_MAX 0x1
>> +#define TA_MAX 0x3
>> +#define RHOLD_MAX 0x7
>> +#define RSTROBE_MAX 0x3f
>> +#define RSETUP_MAX 0xf
>> +#define WHOLD_MAX 0x7
>> +#define WSTROBE_MAX 0x3f
>> +#define WSETUP_MAX 0xf
>> +#define EW_MAX 0x1
>> +#define SS_MAX 0x1
>> +#define NUM_CS 4
>> +
>> +#define TA_VAL(x) (((x) & TA(TA_MAX)) >> TA_SHIFT)
>> +#define RHOLD_VAL(x) (((x) & RHOLD(RHOLD_MAX)) >> RHOLD_SHIFT)
>> +#define RSTROBE_VAL(x) (((x) & RSTROBE(RSTROBE_MAX)) >> RSTROBE_SHIFT)
>> +#define RSETUP_VAL(x) (((x) & RSETUP(RSETUP_MAX)) >> RSETUP_SHIFT)
>> +#define WHOLD_VAL(x) (((x) & WHOLD(WHOLD_MAX)) >> WHOLD_SHIFT)
>> +#define WSTROBE_VAL(x) (((x) & WSTROBE(WSTROBE_MAX)) >> WSTROBE_SHIFT)
>> +#define WSETUP_VAL(x) (((x) & WSETUP(WSETUP_MAX)) >> WSETUP_SHIFT)
>> +#define EW_VAL(x) (((x) & EW(EW_MAX)) >> EW_SHIFT)
>> +#define SS_VAL(x) (((x) & SS(SS_MAX)) >> SS_SHIFT)
>> +
>> +#define NRCSR_OFFSET 0x00
>> +#define AWCCR_OFFSET 0x04
>> +#define A1CR_OFFSET 0x10
>> +
>> +#define ACR_ASIZE_MASK 0x3
>> +#define ACR_EW_MASK BIT(30)
>> +#define ACR_SS_MASK BIT(31)
>> +#define ASIZE_16BIT 1
>> +
>> +#define CONFIG_MASK (TA(TA_MAX) | \
>> + RHOLD(RHOLD_MAX) | \
>> + RSTROBE(RSTROBE_MAX) | \
>> + RSETUP(RSETUP_MAX) | \
>> + WHOLD(WHOLD_MAX) | \
>> + WSTROBE(WSTROBE_MAX) | \
>> + WSETUP(WSETUP_MAX) | \
>> + EW(EW_MAX) | SS(SS_MAX) | \
>> + ASIZE_MAX)
>> +
>> +#define DRV_NAME "davinci-aemif"
> s/davinci-aemif/ti-aemif
Ok
>> +
>> +/**
>> + * structure to hold cs parameters
>> + * @cs: chip-select number
>> + * @wstrobe: write strobe width, ns
>> + * @rstrobe: read strobe width, ns
>> + * @wsetup: write setup width, ns
>> + * @whold: write hold width, ns
>> + * @rsetup: read setup width, ns
>> + * @rhold: read hold width, ns
>> + * @ta: minimum turn around time, ns
>> + * @enable_ss: enable/disable select strobe mode
>> + * @enable_ew: enable/disable extended wait mode
>> + * @asize: width of the asynchronous device's data bus
>> + */
>> +struct davinci_aemif_cs_data {
>> + u8 cs;
>> + u16 wstrobe;
>> + u16 rstrobe;
>> + u8 wsetup;
>> + u8 whold;
>> + u8 rsetup;
>> + u8 rhold;
>> + u8 ta;
>> + u8 enable_ss;
>> + u8 enable_ew;
>> + u8 asize;
>> +};
>> +
> I suggest you drop davinci prefix throughout the
> driver.
>
> [..]
>
Seems there is nothing to prevent
Ok
>> +static const struct of_device_id davinci_aemif_of_match[] = {
>> + { .compatible = "ti,davinci-aemif", },
>> + { .compatible = "ti,keystone-aemif", },
>> + { .compatible = "ti,omap-L138-aemif", },
>> + {},
>> +};
> Do we need three seperate compatible or "ti,aemif" should
> be enough ? Are there differences which we need to handled
> based on compatibles ?
>
We need separate compatibles. Depending on this cs start number is evaluated.
For omap-L138-aemif it is 2 for keystone-aemif it is 0.
>> +
>> +static const struct of_device_id davinci_cs_of_match[] = {
>> + { .compatible = "ti,davinci-cs", },
>> + {},
>> +};
>> +
>> +static int davinci_aemif_probe(struct platform_device *pdev)
>> +{
>> + int ret = -ENODEV, i;
>> + struct resource *res;
>> + struct device *dev = &pdev->dev;
>> + struct device_node *np = dev->of_node;
>> +
>> + if (np == NULL)
>> + return 0;
>> +
>> + if (aemif) {
>> + dev_err(dev, "davinci_aemif driver is in use currently\n");
>> + return -EBUSY;
>> + }
>> +
>> + aemif = devm_kzalloc(dev, sizeof(*aemif), GFP_KERNEL);
>> +
> Drop this extra line.
>
Ok, I will
>> + if (!aemif) {
>> + dev_err(dev, "cannot allocate memory for aemif\n");
>> + return -ENOMEM;
>> + }
>> +
>> + aemif->clk = devm_clk_get(dev, "aemif");
>> + if (IS_ERR(aemif->clk)) {
>> + dev_err(dev, "cannot get clock 'aemif'\n");
>> + return PTR_ERR(aemif->clk);
>> + }
>> +
>> + clk_prepare_enable(aemif->clk);
>> + aemif->clk_rate = clk_get_rate(aemif->clk) / 1000;
>> +
> 1000 divider ? Define a marco and use it to be clear.
Is it OK if I use MSEC_PER_SEC (linux/time.h).
>
> Other than above comments patch looks fine to me.
>
> Regards,
> Santosh
>
--
Regards,
Ivan Khoronzhuk
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH 10/12] mtd: nand: davinci: don't set timings if AEMIF is used
2013-11-18 14:08 ` Santosh Shilimkar
@ 2013-11-18 19:32 ` ivan.khoronzhuk
0 siblings, 0 replies; 56+ messages in thread
From: ivan.khoronzhuk @ 2013-11-18 19:32 UTC (permalink / raw)
To: Santosh Shilimkar, Grygorii Strashko
Cc: Mark Rutland, devicetree, Russell King, Pawel Moll,
Stephen Warren, Ian Campbell, Sekhar Nori, Kumar Gala,
Rob Herring, linux-kernel, linux-mtd, Rob Landley,
linux-arm-kernel
On 11/18/2013 04:08 PM, Santosh Shilimkar wrote:
> On Monday 18 November 2013 06:35 AM, Grygorii Strashko wrote:
>> On 11/12/2013 06:13 PM, Santosh Shilimkar wrote:
>>> On Monday 11 November 2013 12:10 PM, Khoronzhuk, Ivan wrote:
>>>> If Davinci AEMIF is used we don't need to set timings and bus width.
>>>> It is done by AEMIF driver (drivers/memory/davinci-aemfi.c).
>>>>
>>>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
>>>> ---
>>>> drivers/mtd/nand/davinci_nand.c | 22 +++++++++++++++-------
>>>> 1 file changed, 15 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
>>>> index 4705214..879e915 100644
>>>> --- a/drivers/mtd/nand/davinci_nand.c
>>>> +++ b/drivers/mtd/nand/davinci_nand.c
>>>> @@ -742,27 +742,35 @@ static int __init nand_davinci_probe(struct platform_device *pdev)
>>>> goto err_clk_enable;
>>>> }
>>>>
>>>> +#if !IS_ENABLED(CONFIG_TI_DAVINCI_AEMIF)
>>>>
>>> Instead above #if, just use a variable.
>>> bool aemif = IS_ENABLED(CONFIG_TI_DAVINCI_AEMIF) and then skip
>>> the below code. #if block in the middle of the code looks ugly.
>>
>> Yes, this is the hack.
>> The problem is that this part of code contains the call of Davinci platform function davinci_aemif_setup_timing() which is not accessible if Kernel is built for Keystone only.
>> That's why "#if" has been used.
>>
>> This part of code has to be removed together with Davinci aemif platform code (aemif.c), once Davinci will be converted to DT and use
>> new driver.
>>
>> The corresponding comment can be added here. Is it ok?
>>
> Thats more or less what I proposed in another reply. Lets clearly
> document it on why such a check was added.
>
> Regards,
> Santosh
>
Ok, I'll add more explanation on it.
--
Regards,
Ivan Khoronzhuk
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH 10/12] mtd: nand: davinci: don't set timings if AEMIF is used
2013-11-14 14:36 ` Santosh Shilimkar
@ 2013-11-18 19:35 ` ivan.khoronzhuk
2013-11-21 17:07 ` Sekhar Nori
1 sibling, 0 replies; 56+ messages in thread
From: ivan.khoronzhuk @ 2013-11-18 19:35 UTC (permalink / raw)
To: Santosh Shilimkar, Nori, Sekhar
Cc: Mark Rutland, devicetree, Strashko, Grygorii, Russell King,
Pawel Moll, Stephen Warren, Ian Campbell, Kumar Gala,
Rob Herring, linux-kernel, linux-mtd, Rob Landley,
linux-arm-kernel
On 11/14/2013 04:36 PM, Santosh Shilimkar wrote:
> On Thursday 14 November 2013 05:53 AM, Nori, Sekhar wrote:
>> On Wednesday 13 November 2013 07:44 PM, Santosh Shilimkar wrote:
>>> On Wednesday 13 November 2013 12:02 AM, Sekhar Nori wrote:
>>>> On Monday 11 November 2013 10:40 PM, Khoronzhuk, Ivan wrote:
>>>>> If Davinci AEMIF is used we don't need to set timings and bus width.
>>>>> It is done by AEMIF driver (drivers/memory/davinci-aemfi.c).
>>>>>
>>>>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
>>>>> ---
>>>>> drivers/mtd/nand/davinci_nand.c | 22 +++++++++++++++-------
>>>>> 1 file changed, 15 insertions(+), 7 deletions(-)
>>>>>
>>>>> diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
>>>>> index 4705214..879e915 100644
>>>>> --- a/drivers/mtd/nand/davinci_nand.c
>>>>> +++ b/drivers/mtd/nand/davinci_nand.c
>>>>> @@ -742,27 +742,35 @@ static int __init nand_davinci_probe(struct platform_device *pdev)
>>>>> goto err_clk_enable;
>>>>> }
>>>>>
>>>>> +#if !IS_ENABLED(CONFIG_TI_DAVINCI_AEMIF)
>>>>
>>>> This is a hack! Just because AEMIF driver is enabled, it does not
>>>> guarantee that the timings have been setup by it. Instead of configuring
>>>> timings in two drivers, why not just convert everyone over to use the
>>>> new driver. Dont worry about breaking old platforms, I will help test
>>>> and ack them.
>>>>
>>> How about you take a stab at and convert the DaVinci code over to make
>>> use of new driver. We are trying to re-use as much as possible from the
>>> common drivers and also making an option so that the DaVinci arch can
>>> move over to these drivers if they want to.
>>
>> Sure I could.
>>
> Thanks
>
>> Ivan,
>>
>> The AEMIF driver does not use platfrom_data currently. Is that something
>> you can add or you want me to take a stab at that as well?
>>
> The AEMIF new driver is device tree only and thats the direction. So the
> better thing would be to convert Davinci to DT and then make use of this
> new driver. Thats how most of the new drivers used on arm socs are
> moving. Adding platform data to new driver is a step in backward
> direction so I would want to avoid that.
>
> Once you start using new driver the $subject patch code skipping
> is trivial. Till then I would like to proceed with current approach
> which allows Nand support for Keystone.
>
> Regards,
> Santosh
>
Agree with Santosh
--
Regards,
Ivan Khoronzhuk
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH 10/12] mtd: nand: davinci: don't set timings if AEMIF is used
2013-11-14 14:36 ` Santosh Shilimkar
2013-11-18 19:35 ` ivan.khoronzhuk
@ 2013-11-21 17:07 ` Sekhar Nori
2013-11-22 17:38 ` Santosh Shilimkar
1 sibling, 1 reply; 56+ messages in thread
From: Sekhar Nori @ 2013-11-21 17:07 UTC (permalink / raw)
To: Santosh Shilimkar
Cc: Mark Rutland, devicetree, Strashko, Grygorii, Russell King,
Pawel Moll, Stephen Warren, Ian Campbell, Kumar Gala,
Rob Herring, linux-kernel, linux-mtd, Rob Landley, Khoronzhuk,
Ivan, linux-arm-kernel
On 11/14/2013 8:06 PM, Santosh Shilimkar wrote:
> On Thursday 14 November 2013 05:53 AM, Nori, Sekhar wrote:
>> On Wednesday 13 November 2013 07:44 PM, Santosh Shilimkar wrote:
>>> On Wednesday 13 November 2013 12:02 AM, Sekhar Nori wrote:
>>>> On Monday 11 November 2013 10:40 PM, Khoronzhuk, Ivan wrote:
>>>>> If Davinci AEMIF is used we don't need to set timings and bus width.
>>>>> It is done by AEMIF driver (drivers/memory/davinci-aemfi.c).
>>>>>
>>>>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
>>>>> ---
>>>>> drivers/mtd/nand/davinci_nand.c | 22 +++++++++++++++-------
>>>>> 1 file changed, 15 insertions(+), 7 deletions(-)
>>>>>
>>>>> diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
>>>>> index 4705214..879e915 100644
>>>>> --- a/drivers/mtd/nand/davinci_nand.c
>>>>> +++ b/drivers/mtd/nand/davinci_nand.c
>>>>> @@ -742,27 +742,35 @@ static int __init nand_davinci_probe(struct platform_device *pdev)
>>>>> goto err_clk_enable;
>>>>> }
>>>>>
>>>>> +#if !IS_ENABLED(CONFIG_TI_DAVINCI_AEMIF)
>>>>
>>>> This is a hack! Just because AEMIF driver is enabled, it does not
>>>> guarantee that the timings have been setup by it. Instead of configuring
>>>> timings in two drivers, why not just convert everyone over to use the
>>>> new driver. Dont worry about breaking old platforms, I will help test
>>>> and ack them.
>>>>
>>> How about you take a stab at and convert the DaVinci code over to make
>>> use of new driver. We are trying to re-use as much as possible from the
>>> common drivers and also making an option so that the DaVinci arch can
>>> move over to these drivers if they want to.
>>
>> Sure I could.
>>
> Thanks
>
>> Ivan,
>>
>> The AEMIF driver does not use platfrom_data currently. Is that something
>> you can add or you want me to take a stab at that as well?
>>
> The AEMIF new driver is device tree only and thats the direction. So the
> better thing would be to convert Davinci to DT and then make use of this
> new driver. Thats how most of the new drivers used on arm socs are
> moving. Adding platform data to new driver is a step in backward
> direction so I would want to avoid that.
Yes, that would be the ideal way. But the reality is that there is close
to zero chance of all the DaVinci platforms using AEMIF ever getting
converted to DT. This means that we will never be able to get rid of
this piece of code.
I do agree platform data is not the direction you want to take on ARM
but at the same time its not really a deprecated mechanism as far as the
broader kernel is concerned.
So, can we consider adding platform data mechanism to the AEMIF driver?
Thanks,
Sekhar
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH 10/12] mtd: nand: davinci: don't set timings if AEMIF is used
2013-11-21 17:07 ` Sekhar Nori
@ 2013-11-22 17:38 ` Santosh Shilimkar
2013-11-24 9:46 ` Sekhar Nori
0 siblings, 1 reply; 56+ messages in thread
From: Santosh Shilimkar @ 2013-11-22 17:38 UTC (permalink / raw)
To: Nori, Sekhar
Cc: Mark Rutland, devicetree, Strashko, Grygorii, Russell King,
Pawel Moll, Stephen Warren, Ian Campbell, Kumar Gala,
Rob Herring, linux-kernel, linux-mtd, Rob Landley, Khoronzhuk,
Ivan, linux-arm-kernel
On Thursday 21 November 2013 12:07 PM, Nori, Sekhar wrote:
> On 11/14/2013 8:06 PM, Santosh Shilimkar wrote:
>> On Thursday 14 November 2013 05:53 AM, Nori, Sekhar wrote:
>>> On Wednesday 13 November 2013 07:44 PM, Santosh Shilimkar wrote:
>>>> On Wednesday 13 November 2013 12:02 AM, Sekhar Nori wrote:
>>>>> On Monday 11 November 2013 10:40 PM, Khoronzhuk, Ivan wrote:
>>>>>> If Davinci AEMIF is used we don't need to set timings and bus width.
>>>>>> It is done by AEMIF driver (drivers/memory/davinci-aemfi.c).
>>>>>>
>>>>>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
>>>>>> ---
>>>>>> drivers/mtd/nand/davinci_nand.c | 22 +++++++++++++++-------
>>>>>> 1 file changed, 15 insertions(+), 7 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
>>>>>> index 4705214..879e915 100644
>>>>>> --- a/drivers/mtd/nand/davinci_nand.c
>>>>>> +++ b/drivers/mtd/nand/davinci_nand.c
>>>>>> @@ -742,27 +742,35 @@ static int __init nand_davinci_probe(struct platform_device *pdev)
>>>>>> goto err_clk_enable;
>>>>>> }
>>>>>>
>>>>>> +#if !IS_ENABLED(CONFIG_TI_DAVINCI_AEMIF)
>>>>>
>>>>> This is a hack! Just because AEMIF driver is enabled, it does not
>>>>> guarantee that the timings have been setup by it. Instead of configuring
>>>>> timings in two drivers, why not just convert everyone over to use the
>>>>> new driver. Dont worry about breaking old platforms, I will help test
>>>>> and ack them.
>>>>>
>>>> How about you take a stab at and convert the DaVinci code over to make
>>>> use of new driver. We are trying to re-use as much as possible from the
>>>> common drivers and also making an option so that the DaVinci arch can
>>>> move over to these drivers if they want to.
>>>
>>> Sure I could.
>>>
>> Thanks
>>
>>> Ivan,
>>>
>>> The AEMIF driver does not use platfrom_data currently. Is that something
>>> you can add or you want me to take a stab at that as well?
>>>
>> The AEMIF new driver is device tree only and thats the direction. So the
>> better thing would be to convert Davinci to DT and then make use of this
>> new driver. Thats how most of the new drivers used on arm socs are
>> moving. Adding platform data to new driver is a step in backward
>> direction so I would want to avoid that.
>
> Yes, that would be the ideal way. But the reality is that there is close
> to zero chance of all the DaVinci platforms using AEMIF ever getting
> converted to DT. This means that we will never be able to get rid of
> this piece of code.
>
Well that means some of those platforms support will get deprecated
soon considering other basic frameworks like irq, timer wheel etc
is assuming the DT direction. But its your call because if you
want those platforms to live, you need to convert them to DT.
> I do agree platform data is not the direction you want to take on ARM
> but at the same time its not really a deprecated mechanism as far as the
> broader kernel is concerned.
>
> So, can we consider adding platform data mechanism to the AEMIF driver?
>
Sorry but answer is "NO". If the above hack is really bothering you,
we can actually rip out the above code in question and move it
to mach-davinci. Infact thats the best direction to make the
mtd nandn driver independent of any platform quirks(dt, pdata etc)
Ivan already has a patch to do that and he is going to post
that patch. With that patch, we won't need $subject patch.
Regards,
Santosh
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH 10/12] mtd: nand: davinci: don't set timings if AEMIF is used
2013-11-22 17:38 ` Santosh Shilimkar
@ 2013-11-24 9:46 ` Sekhar Nori
2013-11-25 20:00 ` [PATCH] ARM: davinci: aemif: get rid of davinci-nand driver dependency on aemif Ivan Khoronzhuk
2013-11-26 0:55 ` [PATCH 10/12] mtd: nand: davinci: don't set timings if AEMIF is used Santosh Shilimkar
0 siblings, 2 replies; 56+ messages in thread
From: Sekhar Nori @ 2013-11-24 9:46 UTC (permalink / raw)
To: Santosh Shilimkar
Cc: Mark Rutland, devicetree, Strashko, Grygorii, Russell King,
Pawel Moll, Stephen Warren, Ian Campbell, Kumar Gala,
Rob Herring, linux-kernel, linux-mtd, Rob Landley, Khoronzhuk,
Ivan, linux-arm-kernel
On Friday 22 November 2013 11:08 PM, Santosh Shilimkar wrote:
> On Thursday 21 November 2013 12:07 PM, Nori, Sekhar wrote:
>> On 11/14/2013 8:06 PM, Santosh Shilimkar wrote:
>>> On Thursday 14 November 2013 05:53 AM, Nori, Sekhar wrote:
>>>> On Wednesday 13 November 2013 07:44 PM, Santosh Shilimkar wrote:
>>>>> On Wednesday 13 November 2013 12:02 AM, Sekhar Nori wrote:
>>>>>> On Monday 11 November 2013 10:40 PM, Khoronzhuk, Ivan wrote:
>>>>>>> If Davinci AEMIF is used we don't need to set timings and bus width.
>>>>>>> It is done by AEMIF driver (drivers/memory/davinci-aemfi.c).
>>>>>>>
>>>>>>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
>>>>>>> ---
>>>>>>> drivers/mtd/nand/davinci_nand.c | 22 +++++++++++++++-------
>>>>>>> 1 file changed, 15 insertions(+), 7 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
>>>>>>> index 4705214..879e915 100644
>>>>>>> --- a/drivers/mtd/nand/davinci_nand.c
>>>>>>> +++ b/drivers/mtd/nand/davinci_nand.c
>>>>>>> @@ -742,27 +742,35 @@ static int __init nand_davinci_probe(struct platform_device *pdev)
>>>>>>> goto err_clk_enable;
>>>>>>> }
>>>>>>>
>>>>>>> +#if !IS_ENABLED(CONFIG_TI_DAVINCI_AEMIF)
>>>>>>
>>>>>> This is a hack! Just because AEMIF driver is enabled, it does not
>>>>>> guarantee that the timings have been setup by it. Instead of configuring
>>>>>> timings in two drivers, why not just convert everyone over to use the
>>>>>> new driver. Dont worry about breaking old platforms, I will help test
>>>>>> and ack them.
>>>>>>
>>>>> How about you take a stab at and convert the DaVinci code over to make
>>>>> use of new driver. We are trying to re-use as much as possible from the
>>>>> common drivers and also making an option so that the DaVinci arch can
>>>>> move over to these drivers if they want to.
>>>>
>>>> Sure I could.
>>>>
>>> Thanks
>>>
>>>> Ivan,
>>>>
>>>> The AEMIF driver does not use platfrom_data currently. Is that something
>>>> you can add or you want me to take a stab at that as well?
>>>>
>>> The AEMIF new driver is device tree only and thats the direction. So the
>>> better thing would be to convert Davinci to DT and then make use of this
>>> new driver. Thats how most of the new drivers used on arm socs are
>>> moving. Adding platform data to new driver is a step in backward
>>> direction so I would want to avoid that.
>>
>> Yes, that would be the ideal way. But the reality is that there is close
>> to zero chance of all the DaVinci platforms using AEMIF ever getting
>> converted to DT. This means that we will never be able to get rid of
>> this piece of code.
>>
> Well that means some of those platforms support will get deprecated
> soon considering other basic frameworks like irq, timer wheel etc
> is assuming the DT direction. But its your call because if you
> want those platforms to live, you need to convert them to DT.
>
>> I do agree platform data is not the direction you want to take on ARM
>> but at the same time its not really a deprecated mechanism as far as the
>> broader kernel is concerned.
>>
>> So, can we consider adding platform data mechanism to the AEMIF driver?
>>
> Sorry but answer is "NO". If the above hack is really bothering you,
> we can actually rip out the above code in question and move it
> to mach-davinci. Infact thats the best direction to make the
> mtd nandn driver independent of any platform quirks(dt, pdata etc)
>
> Ivan already has a patch to do that and he is going to post
> that patch. With that patch, we won't need $subject patch.
>
Okay I will wait for the patch.
Thanks,
Sekhar
^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH] ARM: davinci: aemif: get rid of davinci-nand driver dependency on aemif
2013-11-24 9:46 ` Sekhar Nori
@ 2013-11-25 20:00 ` Ivan Khoronzhuk
2013-11-27 8:35 ` Sekhar Nori
2013-11-26 0:55 ` [PATCH 10/12] mtd: nand: davinci: don't set timings if AEMIF is used Santosh Shilimkar
1 sibling, 1 reply; 56+ messages in thread
From: Ivan Khoronzhuk @ 2013-11-25 20:00 UTC (permalink / raw)
To: nsekhar
Cc: mark.rutland, devicetree, grygorii.strashko, linux, pawel.moll,
swarren, ijc+devicetree, galak, rob.herring, linux-kernel,
linux-mtd, rob, Ivan Khoronzhuk, linux-arm-kernel
The problem that the set timings code contains the call of Davinci
platform function davinci_aemif_setup_timing() which is not
accessible if kernel is built for another platform like Keystone.
The Keysone platform is going to use TI AEMIF driver.
If TI AEMIF is used we don't need to set timings and bus width.
It is done by AEMIF driver.
To get rid of davinci-nand driver dependency on aemif platform code
we moved aemif code to davinci platform.
The platform AEMIF code (aemif.c) has to be removed once Davinci
will be converted to DT and use ti-aemif.c driver.
Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
---
arch/arm/mach-davinci/aemif.c | 73 ++++++++++++++++++++++-
arch/arm/mach-davinci/board-da830-evm.c | 3 +
arch/arm/mach-davinci/board-da850-evm.c | 3 +
arch/arm/mach-davinci/board-dm355-evm.c | 5 ++
arch/arm/mach-davinci/board-dm355-leopard.c | 5 ++
arch/arm/mach-davinci/board-dm365-evm.c | 4 ++
arch/arm/mach-davinci/board-dm644x-evm.c | 5 ++
arch/arm/mach-davinci/board-dm646x-evm.c | 3 +
arch/arm/mach-davinci/board-mityomapl138.c | 3 +
arch/arm/mach-davinci/board-neuros-osd2.c | 9 ++-
arch/arm/mach-davinci/devices-tnetv107x.c | 3 +
drivers/mtd/nand/davinci_nand.c | 23 -------
include/linux/platform_data/mtd-davinci-aemif.h | 5 +-
13 files changed, 116 insertions(+), 28 deletions(-)
diff --git a/arch/arm/mach-davinci/aemif.c b/arch/arm/mach-davinci/aemif.c
index f091a90..51b14c6 100644
--- a/arch/arm/mach-davinci/aemif.c
+++ b/arch/arm/mach-davinci/aemif.c
@@ -16,6 +16,7 @@
#include <linux/time.h>
#include <linux/platform_data/mtd-davinci-aemif.h>
+#include <linux/platform_data/mtd-davinci.h>
/* Timing value configuration */
@@ -43,6 +44,17 @@
WSTROBE(WSTROBE_MAX) | \
WSETUP(WSETUP_MAX))
+static inline unsigned int davinci_aemif_readl(void __iomem *base, int offset)
+{
+ return readl_relaxed(base + offset);
+}
+
+static inline void davinci_aemif_writel(void __iomem *base,
+ int offset, unsigned long value)
+{
+ writel_relaxed(value, base + offset);
+}
+
/*
* aemif_calc_rate - calculate timing data.
* @wanted: The cycle time needed in nanoseconds.
@@ -86,7 +98,7 @@ static int aemif_calc_rate(int wanted, unsigned long clk, int max)
*
* Returns 0 on success, else negative errno.
*/
-int davinci_aemif_setup_timing(struct davinci_aemif_timing *t,
+static int davinci_aemif_setup_timing(struct davinci_aemif_timing *t,
void __iomem *base, unsigned cs)
{
unsigned set, val;
@@ -130,4 +142,61 @@ int davinci_aemif_setup_timing(struct davinci_aemif_timing *t,
return 0;
}
-EXPORT_SYMBOL(davinci_aemif_setup_timing);
+
+/**
+ * davinci_aemif_setup - setup AEMIF interface by davinci_nand_pdata
+ * @pdev - link to platform device to setup settings for
+ *
+ * This function does not use any locking while programming the AEMIF
+ * because it is expected that there is only one user of a given
+ * chip-select.
+ *
+ * Returns 0 on success, else negative errno.
+ */
+int davinci_aemif_setup(struct platform_device *pdev)
+{
+ struct davinci_nand_pdata *pdata = dev_get_platdata(&pdev->dev);
+ uint32_t val;
+ struct resource *res;
+ void __iomem *base;
+ int ret = 0;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+ if (!res) {
+ dev_err(&pdev->dev, "cannot get IORESOURCE_MEM\n");
+ return -ENOMEM;
+ }
+
+ base = ioremap(res->start, resource_size(res));
+ if (!base) {
+ dev_err(&pdev->dev, "ioremap failed for resource %pR\n", res);
+ ret = -ENOMEM;
+ goto err;
+ }
+
+ /*
+ * Setup Async configuration register in case we did not boot
+ * from NAND and so bootloader did not bother to set it up.
+ */
+ val = davinci_aemif_readl(base, A1CR_OFFSET + pdev->id * 4);
+ /*
+ * Extended Wait is not valid and Select Strobe mode is not
+ * used
+ */
+ val &= ~(ACR_ASIZE_MASK | ACR_EW_MASK | ACR_SS_MASK);
+ if (pdata->options & NAND_BUSWIDTH_16)
+ val |= 0x1;
+
+ davinci_aemif_writel(base, A1CR_OFFSET + pdev->id * 4, val);
+
+ if (pdata->timing)
+ ret = davinci_aemif_setup_timing(pdata->timing, base, pdev->id);
+
+ if (ret < 0)
+ dev_dbg(&pdev->dev, "NAND timing values setup fail\n");
+
+err:
+ iounmap(base);
+ return ret;
+}
+EXPORT_SYMBOL(davinci_aemif_setup);
diff --git a/arch/arm/mach-davinci/board-da830-evm.c b/arch/arm/mach-davinci/board-da830-evm.c
index d1f45af..5623131 100644
--- a/arch/arm/mach-davinci/board-da830-evm.c
+++ b/arch/arm/mach-davinci/board-da830-evm.c
@@ -419,6 +419,9 @@ static inline void da830_evm_init_nand(int mux_mode)
if (ret)
pr_warning("da830_evm_init: NAND device not registered.\n");
+ if (davinci_aemif_setup(&da830_evm_nand_device))
+ pr_warn("%s: Cannot configure AEMIF.\n", __func__);
+
gpio_direction_output(mux_mode, 1);
}
#else
diff --git a/arch/arm/mach-davinci/board-da850-evm.c b/arch/arm/mach-davinci/board-da850-evm.c
index e0af0ec..234c5bb 100644
--- a/arch/arm/mach-davinci/board-da850-evm.c
+++ b/arch/arm/mach-davinci/board-da850-evm.c
@@ -358,6 +358,9 @@ static inline void da850_evm_setup_nor_nand(void)
platform_add_devices(da850_evm_devices,
ARRAY_SIZE(da850_evm_devices));
+
+ if (davinci_aemif_setup(&da850_evm_nandflash_device))
+ pr_warn("%s: Cannot configure AEMIF.\n", __func__);
}
}
diff --git a/arch/arm/mach-davinci/board-dm355-evm.c b/arch/arm/mach-davinci/board-dm355-evm.c
index ecdc7d4..f35095f 100644
--- a/arch/arm/mach-davinci/board-dm355-evm.c
+++ b/arch/arm/mach-davinci/board-dm355-evm.c
@@ -27,6 +27,7 @@
#include <linux/platform_data/mtd-davinci.h>
#include <linux/platform_data/mmc-davinci.h>
#include <linux/platform_data/usb-davinci.h>
+#include <linux/platform_data/mtd-davinci-aemif.h>
#include <asm/mach-types.h>
#include <asm/mach/arch.h>
@@ -395,6 +396,10 @@ static __init void dm355_evm_init(void)
platform_add_devices(davinci_evm_devices,
ARRAY_SIZE(davinci_evm_devices));
+
+ if (davinci_aemif_setup(&davinci_nand_device))
+ pr_warn("%s: Cannot configure AEMIF.\n", __func__);
+
evm_init_i2c();
davinci_serial_init(dm355_serial_device);
diff --git a/arch/arm/mach-davinci/board-dm355-leopard.c b/arch/arm/mach-davinci/board-dm355-leopard.c
index 43bacbf..7f45c5a 100644
--- a/arch/arm/mach-davinci/board-dm355-leopard.c
+++ b/arch/arm/mach-davinci/board-dm355-leopard.c
@@ -23,6 +23,7 @@
#include <linux/platform_data/mmc-davinci.h>
#include <linux/platform_data/mtd-davinci.h>
#include <linux/platform_data/usb-davinci.h>
+#include <linux/platform_data/mtd-davinci-aemif.h>
#include <asm/mach-types.h>
#include <asm/mach/arch.h>
@@ -253,6 +254,10 @@ static __init void dm355_leopard_init(void)
platform_add_devices(davinci_leopard_devices,
ARRAY_SIZE(davinci_leopard_devices));
+
+ if (davinci_aemif_setup(&davinci_nand_device))
+ pr_warn("%s: Cannot configure AEMIF.\n", __func__);
+
leopard_init_i2c();
davinci_serial_init(dm355_serial_device);
diff --git a/arch/arm/mach-davinci/board-dm365-evm.c b/arch/arm/mach-davinci/board-dm365-evm.c
index e08a868..88c01c3 100644
--- a/arch/arm/mach-davinci/board-dm365-evm.c
+++ b/arch/arm/mach-davinci/board-dm365-evm.c
@@ -39,6 +39,7 @@
#include <linux/platform_data/mmc-davinci.h>
#include <linux/platform_data/mtd-davinci.h>
#include <linux/platform_data/keyscan-davinci.h>
+#include <linux/platform_data/mtd-davinci-aemif.h>
#include <media/ths7303.h>
#include <media/tvp514x.h>
@@ -665,6 +666,9 @@ fail:
platform_add_devices(dm365_evm_nand_devices,
ARRAY_SIZE(dm365_evm_nand_devices));
+
+ if (davinci_aemif_setup(&davinci_nand_device))
+ pr_warn("%s: Cannot configure AEMIF.\n", __func__);
} else {
/* no OneNAND support yet */
}
diff --git a/arch/arm/mach-davinci/board-dm644x-evm.c b/arch/arm/mach-davinci/board-dm644x-evm.c
index 987605b7..5602957 100644
--- a/arch/arm/mach-davinci/board-dm644x-evm.c
+++ b/arch/arm/mach-davinci/board-dm644x-evm.c
@@ -778,6 +778,11 @@ static __init void davinci_evm_init(void)
/* only one device will be jumpered and detected */
if (HAS_NAND) {
platform_device_register(&davinci_evm_nandflash_device);
+
+ if (davinci_aemif_setup(&davinci_evm_nandflash_device))
+ pr_warn("%s: Cannot configure AEMIF.\n",
+ __func__);
+
evm_leds[7].default_trigger = "nand-disk";
if (HAS_NOR)
pr_warning("WARNING: both NAND and NOR flash "
diff --git a/arch/arm/mach-davinci/board-dm646x-evm.c b/arch/arm/mach-davinci/board-dm646x-evm.c
index 13d0801..ae129bc 100644
--- a/arch/arm/mach-davinci/board-dm646x-evm.c
+++ b/arch/arm/mach-davinci/board-dm646x-evm.c
@@ -805,6 +805,9 @@ static __init void evm_init(void)
platform_device_register(&davinci_nand_device);
+ if (davinci_aemif_setup(&davinci_nand_device))
+ pr_warn("%s: Cannot configure AEMIF.\n", __func__);
+
dm646x_init_edma(dm646x_edma_rsv);
if (HAS_ATA)
diff --git a/arch/arm/mach-davinci/board-mityomapl138.c b/arch/arm/mach-davinci/board-mityomapl138.c
index 7aa105b..98a66ff 100644
--- a/arch/arm/mach-davinci/board-mityomapl138.c
+++ b/arch/arm/mach-davinci/board-mityomapl138.c
@@ -432,6 +432,9 @@ static void __init mityomapl138_setup_nand(void)
{
platform_add_devices(mityomapl138_devices,
ARRAY_SIZE(mityomapl138_devices));
+
+ if (davinci_aemif_setup(&mityomapl138_nandflash_device))
+ pr_warn("%s: Cannot configure AEMIF.\n", __func__);
}
static const short mityomap_mii_pins[] = {
diff --git a/arch/arm/mach-davinci/board-neuros-osd2.c b/arch/arm/mach-davinci/board-neuros-osd2.c
index bb680af..a7d6668 100644
--- a/arch/arm/mach-davinci/board-neuros-osd2.c
+++ b/arch/arm/mach-davinci/board-neuros-osd2.c
@@ -31,6 +31,7 @@
#include <linux/platform_data/mmc-davinci.h>
#include <linux/platform_data/mtd-davinci.h>
#include <linux/platform_data/usb-davinci.h>
+#include <linux/platform_data/mtd-davinci-aemif.h>
#include <asm/mach-types.h>
#include <asm/mach/arch.h>
@@ -192,9 +193,15 @@ static __init void davinci_ntosd2_init(void)
davinci_cfg_reg(DM644X_ATAEN_DISABLE);
/* only one device will be jumpered and detected */
- if (HAS_NAND)
+ if (HAS_NAND) {
platform_device_register(
&davinci_ntosd2_nandflash_device);
+
+ if (davinci_aemif_setup(
+ &davinci_ntosd2_nandflash_device))
+ pr_warn("%s: Cannot configure AEMIF.\n",
+ __func__);
+ }
}
platform_add_devices(davinci_ntosd2_devices,
diff --git a/arch/arm/mach-davinci/devices-tnetv107x.c b/arch/arm/mach-davinci/devices-tnetv107x.c
index 01d8686..de4862e 100644
--- a/arch/arm/mach-davinci/devices-tnetv107x.c
+++ b/arch/arm/mach-davinci/devices-tnetv107x.c
@@ -324,6 +324,9 @@ static int __init nand_init(int chipsel, struct davinci_nand_pdata *data)
return ret;
}
+ if (davinci_aemif_setup(pdev))
+ pr_warn("%s: Cannot configure AEMIF.\n", __func__);
+
return platform_device_register(pdev);
}
diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
index b77a01e..6cc506c 100644
--- a/drivers/mtd/nand/davinci_nand.c
+++ b/drivers/mtd/nand/davinci_nand.c
@@ -734,28 +734,6 @@ static int __init nand_davinci_probe(struct platform_device *pdev)
goto err_clk_enable;
}
- /*
- * Setup Async configuration register in case we did not boot from
- * NAND and so bootloader did not bother to set it up.
- */
- val = davinci_nand_readl(info, A1CR_OFFSET + info->core_chipsel * 4);
-
- /* Extended Wait is not valid and Select Strobe mode is not used */
- val &= ~(ACR_ASIZE_MASK | ACR_EW_MASK | ACR_SS_MASK);
- if (info->chip.options & NAND_BUSWIDTH_16)
- val |= 0x1;
-
- davinci_nand_writel(info, A1CR_OFFSET + info->core_chipsel * 4, val);
-
- ret = 0;
- if (info->timing)
- ret = davinci_aemif_setup_timing(info->timing, info->base,
- info->core_chipsel);
- if (ret < 0) {
- dev_dbg(&pdev->dev, "NAND timing values setup fail\n");
- goto err_timing;
- }
-
spin_lock_irq(&davinci_nand_lock);
/* put CSxNAND into NAND mode */
@@ -844,7 +822,6 @@ syndrome_done:
return 0;
err_scan:
-err_timing:
clk_disable_unprepare(info->clk);
err_clk_enable:
diff --git a/include/linux/platform_data/mtd-davinci-aemif.h b/include/linux/platform_data/mtd-davinci-aemif.h
index 05b2934..97948ac 100644
--- a/include/linux/platform_data/mtd-davinci-aemif.h
+++ b/include/linux/platform_data/mtd-davinci-aemif.h
@@ -10,6 +10,8 @@
#ifndef _MACH_DAVINCI_AEMIF_H
#define _MACH_DAVINCI_AEMIF_H
+#include <linux/platform_device.h>
+
#define NRCSR_OFFSET 0x00
#define AWCCR_OFFSET 0x04
#define A1CR_OFFSET 0x10
@@ -31,6 +33,5 @@ struct davinci_aemif_timing {
u8 ta;
};
-int davinci_aemif_setup_timing(struct davinci_aemif_timing *t,
- void __iomem *base, unsigned cs);
+int davinci_aemif_setup(struct platform_device *pdev);
#endif
--
1.7.9.5
^ permalink raw reply related [flat|nested] 56+ messages in thread
* Re: [PATCH 10/12] mtd: nand: davinci: don't set timings if AEMIF is used
2013-11-24 9:46 ` Sekhar Nori
2013-11-25 20:00 ` [PATCH] ARM: davinci: aemif: get rid of davinci-nand driver dependency on aemif Ivan Khoronzhuk
@ 2013-11-26 0:55 ` Santosh Shilimkar
1 sibling, 0 replies; 56+ messages in thread
From: Santosh Shilimkar @ 2013-11-26 0:55 UTC (permalink / raw)
To: Sekhar Nori
Cc: Mark Rutland, devicetree, Strashko, Grygorii, Russell King,
Pawel Moll, Stephen Warren, Ian Campbell, Kumar Gala,
Rob Herring, linux-kernel, linux-mtd, Rob Landley, Khoronzhuk,
Ivan, linux-arm-kernel
On Sunday 24 November 2013 04:46 AM, Sekhar Nori wrote:
> On Friday 22 November 2013 11:08 PM, Santosh Shilimkar wrote:
>> On Thursday 21 November 2013 12:07 PM, Nori, Sekhar wrote:
>>> On 11/14/2013 8:06 PM, Santosh Shilimkar wrote:
[..]
>>> Yes, that would be the ideal way. But the reality is that there is close
>>> to zero chance of all the DaVinci platforms using AEMIF ever getting
>>> converted to DT. This means that we will never be able to get rid of
>>> this piece of code.
>>>
>> Well that means some of those platforms support will get deprecated
>> soon considering other basic frameworks like irq, timer wheel etc
>> is assuming the DT direction. But its your call because if you
>> want those platforms to live, you need to convert them to DT.
>>
>>> I do agree platform data is not the direction you want to take on ARM
>>> but at the same time its not really a deprecated mechanism as far as the
>>> broader kernel is concerned.
>>>
>>> So, can we consider adding platform data mechanism to the AEMIF driver?
>>>
>> Sorry but answer is "NO". If the above hack is really bothering you,
>> we can actually rip out the above code in question and move it
>> to mach-davinci. Infact thats the best direction to make the
>> mtd nandn driver independent of any platform quirks(dt, pdata etc)
>>
>> Ivan already has a patch to do that and he is going to post
>> that patch. With that patch, we won't need $subject patch.
>>
>
> Okay I will wait for the patch.
>
Ivan has posted a patch already and you are copied on it.
Regards,
Santosh
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH 06/12] mtd: nand: davinci: adjust DT properties to MTD generic
2013-11-11 17:01 ` [PATCH 06/12] mtd: nand: davinci: adjust DT properties to MTD generic Khoronzhuk, Ivan
@ 2013-11-26 7:03 ` Sekhar Nori
2013-11-26 10:30 ` Grygorii Strashko
0 siblings, 1 reply; 56+ messages in thread
From: Sekhar Nori @ 2013-11-26 7:03 UTC (permalink / raw)
To: Khoronzhuk, Ivan, Shilimkar, Santosh, Rob Landley, Russell King
Cc: Mark Rutland, devicetree, Strashko, Grygorii, Pawel Moll,
Ian Campbell, Stephen Warren, Kumar Gala, Rob Herring,
linux-kernel, linux-mtd, linux-arm-kernel
On Monday 11 November 2013 10:31 PM, Khoronzhuk, Ivan wrote:
> The properties davinci-ecc-mode, davinci-nand-use-bbt, davinci-nand-buswidth
> are MTD generic. Correct names for them are: nand-ecc-mode, nand-on-flash-bbt,
> nand-bus-width accordingly. So rename them in dts and documentation.
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
> ---
> .../devicetree/bindings/mtd/davinci-nand.txt | 25 ++++++++++++++++----
> drivers/mtd/nand/davinci_nand.c | 11 ++++++---
> 2 files changed, 28 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mtd/davinci-nand.txt b/Documentation/devicetree/bindings/mtd/davinci-nand.txt
> index d2a3fc0..befaa5b 100644
> --- a/Documentation/devicetree/bindings/mtd/davinci-nand.txt
> +++ b/Documentation/devicetree/bindings/mtd/davinci-nand.txt
> @@ -37,7 +37,7 @@ Recommended properties :
> - ti,davinci-mask-chipsel: mask for chipselect address. Needed to mask
> addresses for given chipselect.
>
> -- ti,davinci-ecc-mode: operation mode of the NAND ecc mode. ECC mode
> +- nand-ecc-mode: operation mode of the NAND ecc mode. ECC mode
> valid values for davinci driver:
> - "none"
> - "soft"
> @@ -45,10 +45,25 @@ Recommended properties :
>
> - ti,davinci-ecc-bits: used ECC bits, currently supported 1 or 4.
>
> -- ti,davinci-nand-buswidth: buswidth 8 or 16.
Shouldn't we keep the documentation for existing bindings while just
marking them as deprecated?
Thanks,
Sekhar
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH 07/12] memory: davinci-aemif: introduce AEMIF driver
2013-11-11 17:06 ` [PATCH 07/12] memory: davinci-aemif: introduce AEMIF driver Khoronzhuk, Ivan
2013-11-12 16:08 ` Santosh Shilimkar
@ 2013-11-26 7:20 ` Sekhar Nori
2013-11-26 15:05 ` Santosh Shilimkar
2013-11-26 17:44 ` ivan.khoronzhuk
1 sibling, 2 replies; 56+ messages in thread
From: Sekhar Nori @ 2013-11-26 7:20 UTC (permalink / raw)
To: Khoronzhuk, Ivan, Shilimkar, Santosh, Rob Landley, Russell King
Cc: Mark Rutland, devicetree, Strashko, Grygorii, Pawel Moll,
Ian Campbell, Stephen Warren, Kumar Gala, Rob Herring,
linux-kernel, linux-mtd, linux-arm-kernel
On Monday 11 November 2013 10:36 PM, Khoronzhuk, Ivan wrote:
> Add new AEMIF driver for EMIF16 davinci controller. The EMIF16 module
> is intended to provide a glue-less interface to a variety of
> asynchronous memory devices like ASRA M, NOR and NAND memory. A total
> of 256M bytes of any of these memories can be accessed at any given
> time via four chip selects with 64M byte access per chip select.
>
> Synchronous memories such as DDR1 SD RAM, SDR SDRAM and Mobile SDR
> are not supported.
>
> See http://www.ti.com/lit/ug/sprugz3a/sprugz3a.pdf
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
> ---
> drivers/memory/Kconfig | 11 ++
> drivers/memory/Makefile | 1 +
> drivers/memory/davinci-aemif.c | 415 ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 427 insertions(+)
> create mode 100644 drivers/memory/davinci-aemif.c
>
> diff --git a/drivers/memory/Kconfig b/drivers/memory/Kconfig
> index 29a11db..010e75e 100644
> --- a/drivers/memory/Kconfig
> +++ b/drivers/memory/Kconfig
> @@ -7,6 +7,17 @@ menuconfig MEMORY
>
> if MEMORY
>
> +config TI_DAVINCI_AEMIF
> + bool "Texas Instruments DaVinci AEMIF driver"
This driver cannot be a module? If not, why not?
> + depends on (ARCH_DAVINCI || ARCH_KEYSTONE) && OF
> + help
> + This driver is for the AEMIF module available in Texas Instruments
> + SoCs. AEMIF stands for Asynchronous External Memory Interface and
> + is intended to provide a glue-less interface to a variety of
> + asynchronuous memory devices like ASRAM, NOR and NAND memory. A total
> + of 256M bytes of any of these memories can be accessed at a given
> + time via four chip selects with 64M byte access per chip select.
> +
> config TI_EMIF
> tristate "Texas Instruments EMIF driver"
> depends on ARCH_OMAP2PLUS
> diff --git a/drivers/memory/Makefile b/drivers/memory/Makefile
> index 969d923..af14126 100644
> --- a/drivers/memory/Makefile
> +++ b/drivers/memory/Makefile
> @@ -5,6 +5,7 @@
> ifeq ($(CONFIG_DDR),y)
> obj-$(CONFIG_OF) += of_memory.o
> endif
> +obj-$(CONFIG_TI_DAVINCI_AEMIF) += davinci-aemif.o
> obj-$(CONFIG_TI_EMIF) += emif.o
> obj-$(CONFIG_MVEBU_DEVBUS) += mvebu-devbus.o
> obj-$(CONFIG_TEGRA20_MC) += tegra20-mc.o
> diff --git a/drivers/memory/davinci-aemif.c b/drivers/memory/davinci-aemif.c
> new file mode 100644
> index 0000000..e36b74b
> --- /dev/null
> +++ b/drivers/memory/davinci-aemif.c
> @@ -0,0 +1,415 @@
> +/*
> + * DaVinci/Keystone AEMIF driver
> + *
> + * Copyright (C) 2010 - 2013 Texas Instruments Incorporated. http://www.ti.com/
> + * Copyright (C) Heiko Schocher <hs@denx.de>
> + * Copyright (C) Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
Just checking: You meant Author: ? Code is already copyrighted to TI by
line above.
> + *
> + * 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.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +
> +#define TA_SHIFT 2
> +#define RHOLD_SHIFT 4
> +#define RSTROBE_SHIFT 7
> +#define RSETUP_SHIFT 13
> +#define WHOLD_SHIFT 17
> +#define WSTROBE_SHIFT 20
> +#define WSETUP_SHIFT 26
> +#define EW_SHIFT 30
> +#define SS_SHIFT 31
> +
> +#define TA(x) ((x) << TA_SHIFT)
> +#define RHOLD(x) ((x) << RHOLD_SHIFT)
> +#define RSTROBE(x) ((x) << RSTROBE_SHIFT)
> +#define RSETUP(x) ((x) << RSETUP_SHIFT)
> +#define WHOLD(x) ((x) << WHOLD_SHIFT)
> +#define WSTROBE(x) ((x) << WSTROBE_SHIFT)
> +#define WSETUP(x) ((x) << WSETUP_SHIFT)
> +#define EW(x) ((x) << EW_SHIFT)
> +#define SS(x) ((x) << SS_SHIFT)
> +
> +#define ASIZE_MAX 0x1
> +#define TA_MAX 0x3
> +#define RHOLD_MAX 0x7
> +#define RSTROBE_MAX 0x3f
> +#define RSETUP_MAX 0xf
> +#define WHOLD_MAX 0x7
> +#define WSTROBE_MAX 0x3f
> +#define WSETUP_MAX 0xf
> +#define EW_MAX 0x1
> +#define SS_MAX 0x1
> +#define NUM_CS 4
> +
> +#define TA_VAL(x) (((x) & TA(TA_MAX)) >> TA_SHIFT)
> +#define RHOLD_VAL(x) (((x) & RHOLD(RHOLD_MAX)) >> RHOLD_SHIFT)
> +#define RSTROBE_VAL(x) (((x) & RSTROBE(RSTROBE_MAX)) >> RSTROBE_SHIFT)
> +#define RSETUP_VAL(x) (((x) & RSETUP(RSETUP_MAX)) >> RSETUP_SHIFT)
> +#define WHOLD_VAL(x) (((x) & WHOLD(WHOLD_MAX)) >> WHOLD_SHIFT)
> +#define WSTROBE_VAL(x) (((x) & WSTROBE(WSTROBE_MAX)) >> WSTROBE_SHIFT)
> +#define WSETUP_VAL(x) (((x) & WSETUP(WSETUP_MAX)) >> WSETUP_SHIFT)
> +#define EW_VAL(x) (((x) & EW(EW_MAX)) >> EW_SHIFT)
> +#define SS_VAL(x) (((x) & SS(SS_MAX)) >> SS_SHIFT)
> +
> +#define NRCSR_OFFSET 0x00
> +#define AWCCR_OFFSET 0x04
> +#define A1CR_OFFSET 0x10
> +
> +#define ACR_ASIZE_MASK 0x3
> +#define ACR_EW_MASK BIT(30)
> +#define ACR_SS_MASK BIT(31)
> +#define ASIZE_16BIT 1
> +
> +#define CONFIG_MASK (TA(TA_MAX) | \
> + RHOLD(RHOLD_MAX) | \
> + RSTROBE(RSTROBE_MAX) | \
> + RSETUP(RSETUP_MAX) | \
> + WHOLD(WHOLD_MAX) | \
> + WSTROBE(WSTROBE_MAX) | \
> + WSETUP(WSETUP_MAX) | \
> + EW(EW_MAX) | SS(SS_MAX) | \
> + ASIZE_MAX)
> +
> +#define DRV_NAME "davinci-aemif"
> +
> +/**
> + * structure to hold cs parameters
> + * @cs: chip-select number
> + * @wstrobe: write strobe width, ns
> + * @rstrobe: read strobe width, ns
> + * @wsetup: write setup width, ns
> + * @whold: write hold width, ns
> + * @rsetup: read setup width, ns
> + * @rhold: read hold width, ns
> + * @ta: minimum turn around time, ns
> + * @enable_ss: enable/disable select strobe mode
> + * @enable_ew: enable/disable extended wait mode
> + * @asize: width of the asynchronous device's data bus
> + */
> +struct davinci_aemif_cs_data {
> + u8 cs;
> + u16 wstrobe;
> + u16 rstrobe;
> + u8 wsetup;
> + u8 whold;
> + u8 rsetup;
> + u8 rhold;
> + u8 ta;
> + u8 enable_ss;
> + u8 enable_ew;
> + u8 asize;
> +};
> +
> +/**
> + * structure to hold device data
> + * @base: base address of AEMIF registers
> + * @clk: source clock
> + * @clk_rate: clock's rate in kHz
> + * @num_cs: number of assigned chip-selects
> + * @cs_data: array of chip-select settings
> + */
> +struct davinci_aemif_device {
> + struct device *dev;
> + void __iomem *base;
> + struct clk *clk;
> + unsigned long clk_rate;
> + u8 num_cs;
> + int cs_offset;
> + struct davinci_aemif_cs_data cs_data[NUM_CS];
> +};
> +
> +static struct davinci_aemif_device *aemif;
> +/**
> + * davinci_aemif_calc_rate - calculate timing data.
> + * @wanted: The cycle time needed in nanoseconds.
> + * @clk: The input clock rate in kHz.
> + * @max: The maximum divider value that can be programmed.
> + *
> + * On success, returns the calculated timing value minus 1 for easy
> + * programming into AEMIF timing registers, else negative errno.
> + */
> +static int davinci_aemif_calc_rate(int wanted, unsigned long clk, int max)
> +{
> + int result;
> +
> + result = DIV_ROUND_UP((wanted * clk), NSEC_PER_MSEC) - 1;
> +
> + dev_dbg(aemif->dev, "%s: result %d from %ld, %d\n", __func__, result,
> + clk, wanted);
> +
> + /* It is generally OK to have a more relaxed timing than requested... */
> + if (result < 0)
> + result = 0;
> +
> + /* ... But configuring tighter timings is not an option. */
> + else if (result > max)
> + result = -EINVAL;
> +
> + return result;
> +}
> +
> +/**
> + * davinci_aemif_config_abus - configure async bus parameters
> + * @data: aemif chip select configuration
> + *
> + * This function programs the given timing values (in real clock) into the
> + * AEMIF registers taking the AEMIF clock into account.
> + *
> + * This function does not use any locking while programming the AEMIF
> + * because it is expected that there is only one user of a given
> + * chip-select.
> + *
> + * Returns 0 on success, else negative errno.
> + */
> +static int davinci_aemif_config_abus(struct davinci_aemif_cs_data *data)
> +{
> + int ta, rhold, rstrobe, rsetup, whold, wstrobe, wsetup;
> + unsigned offset;
> + u32 set, val;
> +
> + offset = A1CR_OFFSET + (data->cs - aemif->cs_offset) * 4;
> +
> + ta = davinci_aemif_calc_rate(data->ta, aemif->clk_rate, TA_MAX);
> + rhold = davinci_aemif_calc_rate(data->rhold, aemif->clk_rate,
> + RHOLD_MAX);
> + rstrobe = davinci_aemif_calc_rate(data->rstrobe, aemif->clk_rate,
> + RSTROBE_MAX);
> + rsetup = davinci_aemif_calc_rate(data->rsetup, aemif->clk_rate,
> + RSETUP_MAX);
> + whold = davinci_aemif_calc_rate(data->whold, aemif->clk_rate,
> + WHOLD_MAX);
> + wstrobe = davinci_aemif_calc_rate(data->wstrobe, aemif->clk_rate,
> + WSTROBE_MAX);
> + wsetup = davinci_aemif_calc_rate(data->wsetup, aemif->clk_rate,
> + WSETUP_MAX);
> +
> + if (ta < 0 || rhold < 0 || rstrobe < 0 || rsetup < 0 ||
> + whold < 0 || wstrobe < 0 || wsetup < 0) {
> + dev_err(aemif->dev, "%s: cannot get suitable timings\n",
> + __func__);
> + return -EINVAL;
> + }
> +
> + set = TA(ta) | RHOLD(rhold) | RSTROBE(rstrobe) | RSETUP(rsetup) |
> + WHOLD(whold) | WSTROBE(wstrobe) | WSETUP(wsetup);
> +
> + set |= (data->asize & ACR_ASIZE_MASK);
> + if (data->enable_ew)
> + set |= ACR_EW_MASK;
> + if (data->enable_ss)
> + set |= ACR_SS_MASK;
> +
> + val = readl(aemif->base + offset);
> + val &= ~CONFIG_MASK;
> + val |= set;
> + writel(val, aemif->base + offset);
> +
> + return 0;
> +}
> +
> +inline int davinci_aemif_cycles_to_nsec(int val)
> +{
> + return ((val + 1) * NSEC_PER_MSEC) / aemif->clk_rate;
> +}
> +
> +/**
> + * davinci_aemif_get_hw_params - function to read hw register values
> + * @data: ptr to cs data
> + *
> + * This function reads the defaults from the registers and update
> + * the timing values. Required for get/set commands and also for
> + * the case when driver needs to use defaults in hardware.
> + */
> +static void davinci_aemif_get_hw_params(struct davinci_aemif_cs_data *data)
> +{
> + u32 val, offset;
> + offset = A1CR_OFFSET + (data->cs - aemif->cs_offset) * 4;
> +
> + val = readl(aemif->base + offset);
> + data->ta = davinci_aemif_cycles_to_nsec(TA_VAL(val));
> + data->rhold = davinci_aemif_cycles_to_nsec(RHOLD_VAL(val));
> + data->rstrobe = davinci_aemif_cycles_to_nsec(RSTROBE_VAL(val));
> + data->rsetup = davinci_aemif_cycles_to_nsec(RSETUP_VAL(val));
> + data->whold = davinci_aemif_cycles_to_nsec(WHOLD_VAL(val));
> + data->wstrobe = davinci_aemif_cycles_to_nsec(WSTROBE_VAL(val));
> + data->wsetup = davinci_aemif_cycles_to_nsec(WSETUP_VAL(val));
> + data->enable_ew = EW_VAL(val);
> + data->enable_ss = SS_VAL(val);
> + data->asize = val & ASIZE_MAX;
> +}
> +
> +/**
> + * of_davinci_aemif_parse_abus_config - parse CS configuration from DT
> + * @np: device node ptr
> + *
> + * This function update the emif async bus configuration based on the values
> + * configured in a cs device binding node.
> + */
> +static int of_davinci_aemif_parse_abus_config(struct device_node *np)
> +{
> + struct davinci_aemif_cs_data *data;
> + unsigned long cs;
> + u32 val;
> +
> + if (kstrtoul(np->name + 2, 10, &cs) < 0) {
> + dev_dbg(aemif->dev, "cs name is incorrect");
> + return -EINVAL;
> + }
> +
> + if (cs - aemif->cs_offset >= NUM_CS || cs < aemif->cs_offset) {
> + dev_dbg(aemif->dev, "cs number is incorrect %lu", cs);
> + return -EINVAL;
> + }
> +
> + if (aemif->num_cs >= NUM_CS) {
> + dev_dbg(aemif->dev, "cs count is more than %d", NUM_CS);
> + return -EINVAL;
> + }
> +
> + data = &aemif->cs_data[aemif->num_cs++];
> + data->cs = cs;
> +
> + /* read the current value in the hw register */
> + davinci_aemif_get_hw_params(data);
> +
> + /* override the values from device node */
> + of_property_read_u8(np, "ti,davinci-cs-ta", &data->ta);
> + of_property_read_u8(np, "ti,davinci-cs-rhold", &data->rhold);
> + of_property_read_u16(np, "ti,davinci-cs-rstrobe", &data->rstrobe);
> + of_property_read_u8(np, "ti,davinci-cs-rsetup", &data->rsetup);
> + of_property_read_u8(np, "ti,davinci-cs-whold", &data->whold);
> + of_property_read_u16(np, "ti,davinci-cs-wstrobe", &data->wstrobe);
> + of_property_read_u8(np, "ti,davinci-cs-wsetup", &data->wsetup);
> + if (!of_property_read_u32(np, "ti,bus-width", &val))
> + if (val == 16)
> + data->asize = 1;
> + data->enable_ew = of_property_read_bool(np, "ti,davinci-cs-ew");
> + data->enable_ss = of_property_read_bool(np, "ti,davinci-cs-ss");
> + return 0;
> +}
> +
> +static const struct of_device_id davinci_aemif_of_match[] = {
> + { .compatible = "ti,davinci-aemif", },
> + { .compatible = "ti,keystone-aemif", },
> + { .compatible = "ti,omap-L138-aemif", },
Please use ti,da850-aemif instead. We dont use the omap-like paper part
number for these devices in kernel to avoid confusion with real OMAP
devices.
> + {},
> +};
> +
> +static const struct of_device_id davinci_cs_of_match[] = {
> + { .compatible = "ti,davinci-cs", },
> + {},
> +};
> +
> +static int davinci_aemif_probe(struct platform_device *pdev)
> +{
> + int ret = -ENODEV, i;
> + struct resource *res;
> + struct device *dev = &pdev->dev;
> + struct device_node *np = dev->of_node;
> +
> + if (np == NULL)
> + return 0;
> +
> + if (aemif) {
> + dev_err(dev, "davinci_aemif driver is in use currently\n");
> + return -EBUSY;
> + }
Why expressly prevent multiple AEMIF devices? Its entirely conceivable
to have two memories like NAND and NOR flash connect to two different
AEMIF interfaces.
> +
> + aemif = devm_kzalloc(dev, sizeof(*aemif), GFP_KERNEL);
> +
> + if (!aemif) {
> + dev_err(dev, "cannot allocate memory for aemif\n");
> + return -ENOMEM;
> + }
> +
> + aemif->clk = devm_clk_get(dev, "aemif");
If this is the only clock you care about, you can use NULL for
connection id.
> + if (IS_ERR(aemif->clk)) {
> + dev_err(dev, "cannot get clock 'aemif'\n");
> + return PTR_ERR(aemif->clk);
> + }
> +
> + clk_prepare_enable(aemif->clk);
> + aemif->clk_rate = clk_get_rate(aemif->clk) / 1000;
> +
> + aemif->dev = dev;
> +
> + if (of_device_is_compatible(np, "ti,omap-L138-aemif"))
> + aemif->cs_offset = 2;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + aemif->base = devm_ioremap_resource(dev, res);
> + if (IS_ERR(aemif->base)) {
> + ret = PTR_ERR(aemif->base);
> + goto error;
> + }
> +
> + /*
> + * For every controller device node, there is a cs device node that
> + * describe the bus configuration parameters. This functions iterate
> + * over these nodes and update the cs data array.
> + */
> + for_each_matching_node(np, of_match_ptr(davinci_cs_of_match)) {
> + ret = of_davinci_aemif_parse_abus_config(np);
> + if (ret < 0)
> + goto error;
> + }
> +
> + for (i = 0; i < aemif->num_cs; i++) {
> + ret = davinci_aemif_config_abus(&aemif->cs_data[i]);
> + if (ret < 0) {
> + dev_err(dev, "Error configuring chip select %d\n",
> + aemif->cs_data[i].cs);
> + goto error;
> + }
> + }
> +
> + /*
> + * Create a child devices explicitly from here to
> + * guarantee that the child will be probed after the AEMIF timing
> + * parameters are set.
> + */
> + for_each_matching_node(np, of_match_ptr(davinci_cs_of_match)) {
> + ret = of_platform_populate(np, NULL, NULL, dev);
> + if (ret < 0)
> + goto error;
> + }
> +
> + return 0;
> +error:
> + clk_disable_unprepare(aemif->clk);
> + return ret;
> +}
> +
> +static int davinci_aemif_remove(struct platform_device *pdev)
> +{
> + clk_disable_unprepare(aemif->clk);
> + return 0;
> +}
> +
> +static struct platform_driver davinci_aemif_driver = {
> + .probe = davinci_aemif_probe,
> + .remove = davinci_aemif_remove,
> + .driver = {
> + .name = DRV_NAME,
> + .owner = THIS_MODULE,
> + .of_match_table = of_match_ptr(davinci_aemif_of_match),
> + },
> +};
> +
> +module_platform_driver(davinci_aemif_driver);
> +
> +MODULE_AUTHOR("Murali Karicheri <m-karicheri2@ti.com>");
Is this correct? If yes, I would have expected him to sign-off on the
patch too. If not, please drop this.
Thanks,
Sekhar
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH 06/12] mtd: nand: davinci: adjust DT properties to MTD generic
2013-11-26 7:03 ` Sekhar Nori
@ 2013-11-26 10:30 ` Grygorii Strashko
2013-11-26 10:49 ` ivan.khoronzhuk
0 siblings, 1 reply; 56+ messages in thread
From: Grygorii Strashko @ 2013-11-26 10:30 UTC (permalink / raw)
To: Sekhar Nori, Khoronzhuk, Ivan, Shilimkar, Santosh, Rob Landley,
Russell King
Cc: Mark Rutland, devicetree, Pawel Moll, Ian Campbell,
Stephen Warren, Kumar Gala, Rob Herring, linux-kernel, linux-mtd,
linux-arm-kernel
On 11/26/2013 09:03 AM, Sekhar Nori wrote:
> On Monday 11 November 2013 10:31 PM, Khoronzhuk, Ivan wrote:
>> The properties davinci-ecc-mode, davinci-nand-use-bbt, davinci-nand-buswidth
>> are MTD generic. Correct names for them are: nand-ecc-mode, nand-on-flash-bbt,
>> nand-bus-width accordingly. So rename them in dts and documentation.
>>
>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
>> ---
>> .../devicetree/bindings/mtd/davinci-nand.txt | 25 ++++++++++++++++----
>> drivers/mtd/nand/davinci_nand.c | 11 ++++++---
>> 2 files changed, 28 insertions(+), 8 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/mtd/davinci-nand.txt b/Documentation/devicetree/bindings/mtd/davinci-nand.txt
>> index d2a3fc0..befaa5b 100644
>> --- a/Documentation/devicetree/bindings/mtd/davinci-nand.txt
>> +++ b/Documentation/devicetree/bindings/mtd/davinci-nand.txt
>> @@ -37,7 +37,7 @@ Recommended properties :
>> - ti,davinci-mask-chipsel: mask for chipselect address. Needed to mask
>> addresses for given chipselect.
>>
>> -- ti,davinci-ecc-mode: operation mode of the NAND ecc mode. ECC mode
>> +- nand-ecc-mode: operation mode of the NAND ecc mode. ECC mode
>> valid values for davinci driver:
>> - "none"
>> - "soft"
>> @@ -45,10 +45,25 @@ Recommended properties :
>>
>> - ti,davinci-ecc-bits: used ECC bits, currently supported 1 or 4.
>>
>> -- ti,davinci-nand-buswidth: buswidth 8 or 16.
>
> Shouldn't we keep the documentation for existing bindings while just
> marking them as deprecated?
I haven't found any requirements about this in doc.
It's done in both way in other drivers - some keep properties in bindings
documentation and some are not.
Regards,
-grygorii
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH 06/12] mtd: nand: davinci: adjust DT properties to MTD generic
2013-11-26 10:30 ` Grygorii Strashko
@ 2013-11-26 10:49 ` ivan.khoronzhuk
0 siblings, 0 replies; 56+ messages in thread
From: ivan.khoronzhuk @ 2013-11-26 10:49 UTC (permalink / raw)
To: Grygorii Strashko, Sekhar Nori, Shilimkar, Santosh, Rob Landley,
Russell King
Cc: Mark Rutland, devicetree, Pawel Moll, Ian Campbell,
Stephen Warren, Kumar Gala, Rob Herring, linux-kernel, linux-mtd,
linux-arm-kernel
On 11/26/2013 12:30 PM, Grygorii Strashko wrote:
> On 11/26/2013 09:03 AM, Sekhar Nori wrote:
>> On Monday 11 November 2013 10:31 PM, Khoronzhuk, Ivan wrote:
>>> The properties davinci-ecc-mode, davinci-nand-use-bbt, davinci-nand-buswidth
>>> are MTD generic. Correct names for them are: nand-ecc-mode, nand-on-flash-bbt,
>>> nand-bus-width accordingly. So rename them in dts and documentation.
>>>
>>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
>>> ---
>>> .../devicetree/bindings/mtd/davinci-nand.txt | 25 ++++++++++++++++----
>>> drivers/mtd/nand/davinci_nand.c | 11 ++++++---
>>> 2 files changed, 28 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/mtd/davinci-nand.txt b/Documentation/devicetree/bindings/mtd/davinci-nand.txt
>>> index d2a3fc0..befaa5b 100644
>>> --- a/Documentation/devicetree/bindings/mtd/davinci-nand.txt
>>> +++ b/Documentation/devicetree/bindings/mtd/davinci-nand.txt
>>> @@ -37,7 +37,7 @@ Recommended properties :
>>> - ti,davinci-mask-chipsel: mask for chipselect address. Needed to mask
>>> addresses for given chipselect.
>>>
>>> -- ti,davinci-ecc-mode: operation mode of the NAND ecc mode. ECC mode
>>> +- nand-ecc-mode: operation mode of the NAND ecc mode. ECC mode
>>> valid values for davinci driver:
>>> - "none"
>>> - "soft"
>>> @@ -45,10 +45,25 @@ Recommended properties :
>>>
>>> - ti,davinci-ecc-bits: used ECC bits, currently supported 1 or 4.
>>>
>>> -- ti,davinci-nand-buswidth: buswidth 8 or 16.
>>
>> Shouldn't we keep the documentation for existing bindings while just
>> marking them as deprecated?
>
> I haven't found any requirements about this in doc.
> It's done in both way in other drivers - some keep properties in bindings
> documentation and some are not.
>
> Regards,
> -grygorii
>
For me this is not needed, but for example from mtd:
Documentation/devicetree/bindings/mtd/gpmc-nand.txt
,it contains deprecated properties, like a lot of others.
And this sad me I should remain deprecated properties.
Anyway if we have these properties in driver we should describe them.
--
Regards,
Ivan Khoronzhuk
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH 07/12] memory: davinci-aemif: introduce AEMIF driver
2013-11-26 7:20 ` Sekhar Nori
@ 2013-11-26 15:05 ` Santosh Shilimkar
2013-11-26 17:21 ` Sekhar Nori
2013-11-26 17:44 ` ivan.khoronzhuk
1 sibling, 1 reply; 56+ messages in thread
From: Santosh Shilimkar @ 2013-11-26 15:05 UTC (permalink / raw)
To: Sekhar Nori
Cc: Mark Rutland, devicetree, Strashko, Grygorii, Russell King,
Pawel Moll, Stephen Warren, Ian Campbell, Kumar Gala,
Rob Herring, linux-kernel, linux-mtd, Rob Landley, Khoronzhuk,
Ivan, linux-arm-kernel
On Tuesday 26 November 2013 02:20 AM, Sekhar Nori wrote:
> On Monday 11 November 2013 10:36 PM, Khoronzhuk, Ivan wrote:
>> Add new AEMIF driver for EMIF16 davinci controller. The EMIF16 module
>> is intended to provide a glue-less interface to a variety of
>> asynchronous memory devices like ASRA M, NOR and NAND memory. A total
>> of 256M bytes of any of these memories can be accessed at any given
>> time via four chip selects with 64M byte access per chip select.
>>
>> Synchronous memories such as DDR1 SD RAM, SDR SDRAM and Mobile SDR
>> are not supported.
>>
>> See http://www.ti.com/lit/ug/sprugz3a/sprugz3a.pdf
>>
>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
>> ---
[...]
>> +static int davinci_aemif_probe(struct platform_device *pdev)
>> +{
>> + int ret = -ENODEV, i;
>> + struct resource *res;
>> + struct device *dev = &pdev->dev;
>> + struct device_node *np = dev->of_node;
>> +
>> + if (np == NULL)
>> + return 0;
>> +
>> + if (aemif) {
>> + dev_err(dev, "davinci_aemif driver is in use currently\n");
>> + return -EBUSY;
>> + }
>
> Why expressly prevent multiple AEMIF devices? Its entirely conceivable
> to have two memories like NAND and NOR flash connect to two different
> AEMIF interfaces.
>
Ivan wanted me to clarify the Keystone hardware which supports
1 instance of controller with 4 CS. That allows already four
devices to be connected. Currently NAND and NOR are connected on it
and two more slots are free.
Since driver support what hardware is, lets not build a driver for
hardware which don't exist. And if at all such a support would be
needed in future, we can always add it.
Regards,
Santosh
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH 07/12] memory: davinci-aemif: introduce AEMIF driver
2013-11-26 15:05 ` Santosh Shilimkar
@ 2013-11-26 17:21 ` Sekhar Nori
2013-11-26 18:26 ` Santosh Shilimkar
0 siblings, 1 reply; 56+ messages in thread
From: Sekhar Nori @ 2013-11-26 17:21 UTC (permalink / raw)
To: Santosh Shilimkar
Cc: Mark Rutland, devicetree, Strashko, Grygorii, Russell King,
Pawel Moll, Stephen Warren, Ian Campbell, Kumar Gala,
Rob Herring, linux-kernel, linux-mtd, Rob Landley, Khoronzhuk,
Ivan, linux-arm-kernel
On 11/26/2013 8:35 PM, Santosh Shilimkar wrote:
> On Tuesday 26 November 2013 02:20 AM, Sekhar Nori wrote:
>> On Monday 11 November 2013 10:36 PM, Khoronzhuk, Ivan wrote:
>>> Add new AEMIF driver for EMIF16 davinci controller. The EMIF16 module
>>> is intended to provide a glue-less interface to a variety of
>>> asynchronous memory devices like ASRA M, NOR and NAND memory. A total
>>> of 256M bytes of any of these memories can be accessed at any given
>>> time via four chip selects with 64M byte access per chip select.
>>>
>>> Synchronous memories such as DDR1 SD RAM, SDR SDRAM and Mobile SDR
>>> are not supported.
>>>
>>> See http://www.ti.com/lit/ug/sprugz3a/sprugz3a.pdf
>>>
>>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
>>> ---
>
> [...]
>
>>> +static int davinci_aemif_probe(struct platform_device *pdev)
>>> +{
>>> + int ret = -ENODEV, i;
>>> + struct resource *res;
>>> + struct device *dev = &pdev->dev;
>>> + struct device_node *np = dev->of_node;
>>> +
>>> + if (np == NULL)
>>> + return 0;
>>> +
>>> + if (aemif) {
>>> + dev_err(dev, "davinci_aemif driver is in use currently\n");
>>> + return -EBUSY;
>>> + }
>>
>> Why expressly prevent multiple AEMIF devices? Its entirely conceivable
>> to have two memories like NAND and NOR flash connect to two different
>> AEMIF interfaces.
>>
> Ivan wanted me to clarify the Keystone hardware which supports
> 1 instance of controller with 4 CS. That allows already four
> devices to be connected. Currently NAND and NOR are connected on it
> and two more slots are free.
>
> Since driver support what hardware is, lets not build a driver for
> hardware which don't exist. And if at all such a support would be
> needed in future, we can always add it.
I understand the lack of hardware part, but its common to write the
driver such that it can handle multiple instances. Is there any gain on
current hardware because of this check? From what I am hearing, the code
in question wont be exercised at all. So why go all the way and add it
in first place?
Thanks,
Sekhar
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH 07/12] memory: davinci-aemif: introduce AEMIF driver
2013-11-26 7:20 ` Sekhar Nori
2013-11-26 15:05 ` Santosh Shilimkar
@ 2013-11-26 17:44 ` ivan.khoronzhuk
2013-11-26 18:30 ` Santosh Shilimkar
2013-11-27 4:05 ` Sekhar Nori
1 sibling, 2 replies; 56+ messages in thread
From: ivan.khoronzhuk @ 2013-11-26 17:44 UTC (permalink / raw)
To: Sekhar Nori, Shilimkar, Santosh, Rob Landley, Russell King
Cc: Mark Rutland, devicetree, Strashko, Grygorii, Pawel Moll,
Ian Campbell, Stephen Warren, Kumar Gala, Rob Herring,
linux-kernel, linux-mtd, linux-arm-kernel
On 11/26/2013 09:20 AM, Sekhar Nori wrote:
> On Monday 11 November 2013 10:36 PM, Khoronzhuk, Ivan wrote:
>> Add new AEMIF driver for EMIF16 davinci controller. The EMIF16 module
>> is intended to provide a glue-less interface to a variety of
>> asynchronous memory devices like ASRA M, NOR and NAND memory. A total
>> of 256M bytes of any of these memories can be accessed at any given
>> time via four chip selects with 64M byte access per chip select.
>>
>> Synchronous memories such as DDR1 SD RAM, SDR SDRAM and Mobile SDR
>> are not supported.
>>
>> See http://www.ti.com/lit/ug/sprugz3a/sprugz3a.pdf
>>
>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
>> ---
>> drivers/memory/Kconfig | 11 ++
>> drivers/memory/Makefile | 1 +
>> drivers/memory/davinci-aemif.c | 415 ++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 427 insertions(+)
>> create mode 100644 drivers/memory/davinci-aemif.c
>>
>> diff --git a/drivers/memory/Kconfig b/drivers/memory/Kconfig
>> index 29a11db..010e75e 100644
>> --- a/drivers/memory/Kconfig
>> +++ b/drivers/memory/Kconfig
>> @@ -7,6 +7,17 @@ menuconfig MEMORY
>>
>> if MEMORY
>>
>> +config TI_DAVINCI_AEMIF
>> + bool "Texas Instruments DaVinci AEMIF driver"
>
> This driver cannot be a module? If not, why not?
>
Yes it can be a module, I will correct it.
>> + depends on (ARCH_DAVINCI || ARCH_KEYSTONE) && OF
>> + help
>> + This driver is for the AEMIF module available in Texas Instruments
>> + SoCs. AEMIF stands for Asynchronous External Memory Interface and
>> + is intended to provide a glue-less interface to a variety of
>> + asynchronuous memory devices like ASRAM, NOR and NAND memory. A total
>> + of 256M bytes of any of these memories can be accessed at a given
>> + time via four chip selects with 64M byte access per chip select.
>> +
>> config TI_EMIF
>> tristate "Texas Instruments EMIF driver"
>> depends on ARCH_OMAP2PLUS
>> diff --git a/drivers/memory/Makefile b/drivers/memory/Makefile
>> index 969d923..af14126 100644
>> --- a/drivers/memory/Makefile
>> +++ b/drivers/memory/Makefile
>> @@ -5,6 +5,7 @@
>> ifeq ($(CONFIG_DDR),y)
>> obj-$(CONFIG_OF) += of_memory.o
>> endif
>> +obj-$(CONFIG_TI_DAVINCI_AEMIF) += davinci-aemif.o
>> obj-$(CONFIG_TI_EMIF) += emif.o
>> obj-$(CONFIG_MVEBU_DEVBUS) += mvebu-devbus.o
>> obj-$(CONFIG_TEGRA20_MC) += tegra20-mc.o
>> diff --git a/drivers/memory/davinci-aemif.c b/drivers/memory/davinci-aemif.c
>> new file mode 100644
>> index 0000000..e36b74b
>> --- /dev/null
>> +++ b/drivers/memory/davinci-aemif.c
>> @@ -0,0 +1,415 @@
>> +/*
>> + * DaVinci/Keystone AEMIF driver
>> + *
>> + * Copyright (C) 2010 - 2013 Texas Instruments Incorporated. http://www.ti.com/
>> + * Copyright (C) Heiko Schocher <hs@denx.de>
>> + * Copyright (C) Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
>
> Just checking: You meant Author: ? Code is already copyrighted to TI by
> line above.
>
It will be
Author:
Heiko Schocher <hs@denx.de>
Ivan Khoronzhuk <ivan.khoronzhuk@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.
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/err.h>
>> +#include <linux/io.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/platform_device.h>
>> +
>> +#define TA_SHIFT 2
>> +#define RHOLD_SHIFT 4
>> +#define RSTROBE_SHIFT 7
>> +#define RSETUP_SHIFT 13
>> +#define WHOLD_SHIFT 17
>> +#define WSTROBE_SHIFT 20
>> +#define WSETUP_SHIFT 26
>> +#define EW_SHIFT 30
>> +#define SS_SHIFT 31
>> +
>> +#define TA(x) ((x) << TA_SHIFT)
>> +#define RHOLD(x) ((x) << RHOLD_SHIFT)
>> +#define RSTROBE(x) ((x) << RSTROBE_SHIFT)
>> +#define RSETUP(x) ((x) << RSETUP_SHIFT)
>> +#define WHOLD(x) ((x) << WHOLD_SHIFT)
>> +#define WSTROBE(x) ((x) << WSTROBE_SHIFT)
>> +#define WSETUP(x) ((x) << WSETUP_SHIFT)
>> +#define EW(x) ((x) << EW_SHIFT)
>> +#define SS(x) ((x) << SS_SHIFT)
>> +
>> +#define ASIZE_MAX 0x1
>> +#define TA_MAX 0x3
>> +#define RHOLD_MAX 0x7
>> +#define RSTROBE_MAX 0x3f
>> +#define RSETUP_MAX 0xf
>> +#define WHOLD_MAX 0x7
>> +#define WSTROBE_MAX 0x3f
>> +#define WSETUP_MAX 0xf
>> +#define EW_MAX 0x1
>> +#define SS_MAX 0x1
>> +#define NUM_CS 4
>> +
>> +#define TA_VAL(x) (((x) & TA(TA_MAX)) >> TA_SHIFT)
>> +#define RHOLD_VAL(x) (((x) & RHOLD(RHOLD_MAX)) >> RHOLD_SHIFT)
>> +#define RSTROBE_VAL(x) (((x) & RSTROBE(RSTROBE_MAX)) >> RSTROBE_SHIFT)
>> +#define RSETUP_VAL(x) (((x) & RSETUP(RSETUP_MAX)) >> RSETUP_SHIFT)
>> +#define WHOLD_VAL(x) (((x) & WHOLD(WHOLD_MAX)) >> WHOLD_SHIFT)
>> +#define WSTROBE_VAL(x) (((x) & WSTROBE(WSTROBE_MAX)) >> WSTROBE_SHIFT)
>> +#define WSETUP_VAL(x) (((x) & WSETUP(WSETUP_MAX)) >> WSETUP_SHIFT)
>> +#define EW_VAL(x) (((x) & EW(EW_MAX)) >> EW_SHIFT)
>> +#define SS_VAL(x) (((x) & SS(SS_MAX)) >> SS_SHIFT)
>> +
>> +#define NRCSR_OFFSET 0x00
>> +#define AWCCR_OFFSET 0x04
>> +#define A1CR_OFFSET 0x10
>> +
>> +#define ACR_ASIZE_MASK 0x3
>> +#define ACR_EW_MASK BIT(30)
>> +#define ACR_SS_MASK BIT(31)
>> +#define ASIZE_16BIT 1
>> +
>> +#define CONFIG_MASK (TA(TA_MAX) | \
>> + RHOLD(RHOLD_MAX) | \
>> + RSTROBE(RSTROBE_MAX) | \
>> + RSETUP(RSETUP_MAX) | \
>> + WHOLD(WHOLD_MAX) | \
>> + WSTROBE(WSTROBE_MAX) | \
>> + WSETUP(WSETUP_MAX) | \
>> + EW(EW_MAX) | SS(SS_MAX) | \
>> + ASIZE_MAX)
>> +
>> +#define DRV_NAME "davinci-aemif"
>> +
>> +/**
>> + * structure to hold cs parameters
>> + * @cs: chip-select number
>> + * @wstrobe: write strobe width, ns
>> + * @rstrobe: read strobe width, ns
>> + * @wsetup: write setup width, ns
>> + * @whold: write hold width, ns
>> + * @rsetup: read setup width, ns
>> + * @rhold: read hold width, ns
>> + * @ta: minimum turn around time, ns
>> + * @enable_ss: enable/disable select strobe mode
>> + * @enable_ew: enable/disable extended wait mode
>> + * @asize: width of the asynchronous device's data bus
>> + */
>> +struct davinci_aemif_cs_data {
>> + u8 cs;
>> + u16 wstrobe;
>> + u16 rstrobe;
>> + u8 wsetup;
>> + u8 whold;
>> + u8 rsetup;
>> + u8 rhold;
>> + u8 ta;
>> + u8 enable_ss;
>> + u8 enable_ew;
>> + u8 asize;
>> +};
>> +
>> +/**
>> + * structure to hold device data
>> + * @base: base address of AEMIF registers
>> + * @clk: source clock
>> + * @clk_rate: clock's rate in kHz
>> + * @num_cs: number of assigned chip-selects
>> + * @cs_data: array of chip-select settings
>> + */
>> +struct davinci_aemif_device {
>> + struct device *dev;
>> + void __iomem *base;
>> + struct clk *clk;
>> + unsigned long clk_rate;
>> + u8 num_cs;
>> + int cs_offset;
>> + struct davinci_aemif_cs_data cs_data[NUM_CS];
>> +};
>> +
>> +static struct davinci_aemif_device *aemif;
>> +/**
>> + * davinci_aemif_calc_rate - calculate timing data.
>> + * @wanted: The cycle time needed in nanoseconds.
>> + * @clk: The input clock rate in kHz.
>> + * @max: The maximum divider value that can be programmed.
>> + *
>> + * On success, returns the calculated timing value minus 1 for easy
>> + * programming into AEMIF timing registers, else negative errno.
>> + */
>> +static int davinci_aemif_calc_rate(int wanted, unsigned long clk, int max)
>> +{
>> + int result;
>> +
>> + result = DIV_ROUND_UP((wanted * clk), NSEC_PER_MSEC) - 1;
>> +
>> + dev_dbg(aemif->dev, "%s: result %d from %ld, %d\n", __func__, result,
>> + clk, wanted);
>> +
>> + /* It is generally OK to have a more relaxed timing than requested... */
>> + if (result < 0)
>> + result = 0;
>> +
>> + /* ... But configuring tighter timings is not an option. */
>> + else if (result > max)
>> + result = -EINVAL;
>> +
>> + return result;
>> +}
>> +
>> +/**
>> + * davinci_aemif_config_abus - configure async bus parameters
>> + * @data: aemif chip select configuration
>> + *
>> + * This function programs the given timing values (in real clock) into the
>> + * AEMIF registers taking the AEMIF clock into account.
>> + *
>> + * This function does not use any locking while programming the AEMIF
>> + * because it is expected that there is only one user of a given
>> + * chip-select.
>> + *
>> + * Returns 0 on success, else negative errno.
>> + */
>> +static int davinci_aemif_config_abus(struct davinci_aemif_cs_data *data)
>> +{
>> + int ta, rhold, rstrobe, rsetup, whold, wstrobe, wsetup;
>> + unsigned offset;
>> + u32 set, val;
>> +
>> + offset = A1CR_OFFSET + (data->cs - aemif->cs_offset) * 4;
>> +
>> + ta = davinci_aemif_calc_rate(data->ta, aemif->clk_rate, TA_MAX);
>> + rhold = davinci_aemif_calc_rate(data->rhold, aemif->clk_rate,
>> + RHOLD_MAX);
>> + rstrobe = davinci_aemif_calc_rate(data->rstrobe, aemif->clk_rate,
>> + RSTROBE_MAX);
>> + rsetup = davinci_aemif_calc_rate(data->rsetup, aemif->clk_rate,
>> + RSETUP_MAX);
>> + whold = davinci_aemif_calc_rate(data->whold, aemif->clk_rate,
>> + WHOLD_MAX);
>> + wstrobe = davinci_aemif_calc_rate(data->wstrobe, aemif->clk_rate,
>> + WSTROBE_MAX);
>> + wsetup = davinci_aemif_calc_rate(data->wsetup, aemif->clk_rate,
>> + WSETUP_MAX);
>> +
>> + if (ta < 0 || rhold < 0 || rstrobe < 0 || rsetup < 0 ||
>> + whold < 0 || wstrobe < 0 || wsetup < 0) {
>> + dev_err(aemif->dev, "%s: cannot get suitable timings\n",
>> + __func__);
>> + return -EINVAL;
>> + }
>> +
>> + set = TA(ta) | RHOLD(rhold) | RSTROBE(rstrobe) | RSETUP(rsetup) |
>> + WHOLD(whold) | WSTROBE(wstrobe) | WSETUP(wsetup);
>> +
>> + set |= (data->asize & ACR_ASIZE_MASK);
>> + if (data->enable_ew)
>> + set |= ACR_EW_MASK;
>> + if (data->enable_ss)
>> + set |= ACR_SS_MASK;
>> +
>> + val = readl(aemif->base + offset);
>> + val &= ~CONFIG_MASK;
>> + val |= set;
>> + writel(val, aemif->base + offset);
>> +
>> + return 0;
>> +}
>> +
>> +inline int davinci_aemif_cycles_to_nsec(int val)
>> +{
>> + return ((val + 1) * NSEC_PER_MSEC) / aemif->clk_rate;
>> +}
>> +
>> +/**
>> + * davinci_aemif_get_hw_params - function to read hw register values
>> + * @data: ptr to cs data
>> + *
>> + * This function reads the defaults from the registers and update
>> + * the timing values. Required for get/set commands and also for
>> + * the case when driver needs to use defaults in hardware.
>> + */
>> +static void davinci_aemif_get_hw_params(struct davinci_aemif_cs_data *data)
>> +{
>> + u32 val, offset;
>> + offset = A1CR_OFFSET + (data->cs - aemif->cs_offset) * 4;
>> +
>> + val = readl(aemif->base + offset);
>> + data->ta = davinci_aemif_cycles_to_nsec(TA_VAL(val));
>> + data->rhold = davinci_aemif_cycles_to_nsec(RHOLD_VAL(val));
>> + data->rstrobe = davinci_aemif_cycles_to_nsec(RSTROBE_VAL(val));
>> + data->rsetup = davinci_aemif_cycles_to_nsec(RSETUP_VAL(val));
>> + data->whold = davinci_aemif_cycles_to_nsec(WHOLD_VAL(val));
>> + data->wstrobe = davinci_aemif_cycles_to_nsec(WSTROBE_VAL(val));
>> + data->wsetup = davinci_aemif_cycles_to_nsec(WSETUP_VAL(val));
>> + data->enable_ew = EW_VAL(val);
>> + data->enable_ss = SS_VAL(val);
>> + data->asize = val & ASIZE_MAX;
>> +}
>> +
>> +/**
>> + * of_davinci_aemif_parse_abus_config - parse CS configuration from DT
>> + * @np: device node ptr
>> + *
>> + * This function update the emif async bus configuration based on the values
>> + * configured in a cs device binding node.
>> + */
>> +static int of_davinci_aemif_parse_abus_config(struct device_node *np)
>> +{
>> + struct davinci_aemif_cs_data *data;
>> + unsigned long cs;
>> + u32 val;
>> +
>> + if (kstrtoul(np->name + 2, 10, &cs) < 0) {
>> + dev_dbg(aemif->dev, "cs name is incorrect");
>> + return -EINVAL;
>> + }
>> +
>> + if (cs - aemif->cs_offset >= NUM_CS || cs < aemif->cs_offset) {
>> + dev_dbg(aemif->dev, "cs number is incorrect %lu", cs);
>> + return -EINVAL;
>> + }
>> +
>> + if (aemif->num_cs >= NUM_CS) {
>> + dev_dbg(aemif->dev, "cs count is more than %d", NUM_CS);
>> + return -EINVAL;
>> + }
>> +
>> + data = &aemif->cs_data[aemif->num_cs++];
>> + data->cs = cs;
>> +
>> + /* read the current value in the hw register */
>> + davinci_aemif_get_hw_params(data);
>> +
>> + /* override the values from device node */
>> + of_property_read_u8(np, "ti,davinci-cs-ta", &data->ta);
>> + of_property_read_u8(np, "ti,davinci-cs-rhold", &data->rhold);
>> + of_property_read_u16(np, "ti,davinci-cs-rstrobe", &data->rstrobe);
>> + of_property_read_u8(np, "ti,davinci-cs-rsetup", &data->rsetup);
>> + of_property_read_u8(np, "ti,davinci-cs-whold", &data->whold);
>> + of_property_read_u16(np, "ti,davinci-cs-wstrobe", &data->wstrobe);
>> + of_property_read_u8(np, "ti,davinci-cs-wsetup", &data->wsetup);
>> + if (!of_property_read_u32(np, "ti,bus-width", &val))
>> + if (val == 16)
>> + data->asize = 1;
>> + data->enable_ew = of_property_read_bool(np, "ti,davinci-cs-ew");
>> + data->enable_ss = of_property_read_bool(np, "ti,davinci-cs-ss");
>> + return 0;
>> +}
>> +
>> +static const struct of_device_id davinci_aemif_of_match[] = {
>> + { .compatible = "ti,davinci-aemif", },
>> + { .compatible = "ti,keystone-aemif", },
>> + { .compatible = "ti,omap-L138-aemif", },
>
> Please use ti,da850-aemif instead. We dont use the omap-like paper part
> number for these devices in kernel to avoid confusion with real OMAP
> devices.
>
Ok, I'll replace ti,omap-L138-aemif -> ti,da850-aemif
>> + {},
>> +};
>> +
>> +static const struct of_device_id davinci_cs_of_match[] = {
>> + { .compatible = "ti,davinci-cs", },
>> + {},
>> +};
>> +
>> +static int davinci_aemif_probe(struct platform_device *pdev)
>> +{
>> + int ret = -ENODEV, i;
>> + struct resource *res;
>> + struct device *dev = &pdev->dev;
>> + struct device_node *np = dev->of_node;
>> +
>> + if (np == NULL)
>> + return 0;
>> +
>> + if (aemif) {
>> + dev_err(dev, "davinci_aemif driver is in use currently\n");
>> + return -EBUSY;
>> + }
>
> Why expressly prevent multiple AEMIF devices? Its entirely conceivable
> to have two memories like NAND and NOR flash connect to two different
> AEMIF interfaces.
>
It can be, but I'm not sure if it is needed. Currently I've not seen case where
more than 2 cses were used, I mean we have 2 cs free, why do we need the second AEMIF
controller?
>> +
>> + aemif = devm_kzalloc(dev, sizeof(*aemif), GFP_KERNEL);
>> +
>> + if (!aemif) {
>> + dev_err(dev, "cannot allocate memory for aemif\n");
>> + return -ENOMEM;
>> + }
>> +
>> + aemif->clk = devm_clk_get(dev, "aemif");
>
> If this is the only clock you care about, you can use NULL for
> connection id.
>
Yes I can, I'll replace aemif name on NULL.
>> + if (IS_ERR(aemif->clk)) {
>> + dev_err(dev, "cannot get clock 'aemif'\n");
>> + return PTR_ERR(aemif->clk);
>> + }
>> +
>> + clk_prepare_enable(aemif->clk);
>> + aemif->clk_rate = clk_get_rate(aemif->clk) / 1000;
>> +
>> + aemif->dev = dev;
>> +
>> + if (of_device_is_compatible(np, "ti,omap-L138-aemif"))
>> + aemif->cs_offset = 2;
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + aemif->base = devm_ioremap_resource(dev, res);
>> + if (IS_ERR(aemif->base)) {
>> + ret = PTR_ERR(aemif->base);
>> + goto error;
>> + }
>> +
>> + /*
>> + * For every controller device node, there is a cs device node that
>> + * describe the bus configuration parameters. This functions iterate
>> + * over these nodes and update the cs data array.
>> + */
>> + for_each_matching_node(np, of_match_ptr(davinci_cs_of_match)) {
>> + ret = of_davinci_aemif_parse_abus_config(np);
>> + if (ret < 0)
>> + goto error;
>> + }
>> +
>> + for (i = 0; i < aemif->num_cs; i++) {
>> + ret = davinci_aemif_config_abus(&aemif->cs_data[i]);
>> + if (ret < 0) {
>> + dev_err(dev, "Error configuring chip select %d\n",
>> + aemif->cs_data[i].cs);
>> + goto error;
>> + }
>> + }
>> +
>> + /*
>> + * Create a child devices explicitly from here to
>> + * guarantee that the child will be probed after the AEMIF timing
>> + * parameters are set.
>> + */
>> + for_each_matching_node(np, of_match_ptr(davinci_cs_of_match)) {
>> + ret = of_platform_populate(np, NULL, NULL, dev);
>> + if (ret < 0)
>> + goto error;
>> + }
>> +
>> + return 0;
>> +error:
>> + clk_disable_unprepare(aemif->clk);
>> + return ret;
>> +}
>> +
>> +static int davinci_aemif_remove(struct platform_device *pdev)
>> +{
>> + clk_disable_unprepare(aemif->clk);
>> + return 0;
>> +}
>> +
>> +static struct platform_driver davinci_aemif_driver = {
>> + .probe = davinci_aemif_probe,
>> + .remove = davinci_aemif_remove,
>> + .driver = {
>> + .name = DRV_NAME,
>> + .owner = THIS_MODULE,
>> + .of_match_table = of_match_ptr(davinci_aemif_of_match),
>> + },
>> +};
>> +
>> +module_platform_driver(davinci_aemif_driver);
>> +
>> +MODULE_AUTHOR("Murali Karicheri <m-karicheri2@ti.com>");
>
> Is this correct? If yes, I would have expected him to sign-off on the
> patch too. If not, please drop this.
I will add sign-off
>
> Thanks,
> Sekhar
>
--
Regards,
Ivan Khoronzhuk
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH 07/12] memory: davinci-aemif: introduce AEMIF driver
2013-11-26 17:21 ` Sekhar Nori
@ 2013-11-26 18:26 ` Santosh Shilimkar
2013-11-26 18:29 ` ivan.khoronzhuk
2013-11-27 0:37 ` Brian Norris
0 siblings, 2 replies; 56+ messages in thread
From: Santosh Shilimkar @ 2013-11-26 18:26 UTC (permalink / raw)
To: Sekhar Nori
Cc: Mark Rutland, devicetree, Strashko, Grygorii, Russell King,
Pawel Moll, Stephen Warren, Ian Campbell, Kumar Gala,
Rob Herring, linux-kernel, linux-mtd, Rob Landley, Khoronzhuk,
Ivan, linux-arm-kernel
On Tuesday 26 November 2013 12:21 PM, Sekhar Nori wrote:
> On 11/26/2013 8:35 PM, Santosh Shilimkar wrote:
>> On Tuesday 26 November 2013 02:20 AM, Sekhar Nori wrote:
>>> On Monday 11 November 2013 10:36 PM, Khoronzhuk, Ivan wrote:
>>>> Add new AEMIF driver for EMIF16 davinci controller. The EMIF16 module
>>>> is intended to provide a glue-less interface to a variety of
>>>> asynchronous memory devices like ASRA M, NOR and NAND memory. A total
>>>> of 256M bytes of any of these memories can be accessed at any given
>>>> time via four chip selects with 64M byte access per chip select.
>>>>
>>>> Synchronous memories such as DDR1 SD RAM, SDR SDRAM and Mobile SDR
>>>> are not supported.
>>>>
>>>> See http://www.ti.com/lit/ug/sprugz3a/sprugz3a.pdf
>>>>
>>>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
>>>> ---
>>
>> [...]
>>
>>>> +static int davinci_aemif_probe(struct platform_device *pdev)
>>>> +{
>>>> + int ret = -ENODEV, i;
>>>> + struct resource *res;
>>>> + struct device *dev = &pdev->dev;
>>>> + struct device_node *np = dev->of_node;
>>>> +
>>>> + if (np == NULL)
>>>> + return 0;
>>>> +
>>>> + if (aemif) {
>>>> + dev_err(dev, "davinci_aemif driver is in use currently\n");
>>>> + return -EBUSY;
>>>> + }
>>>
>>> Why expressly prevent multiple AEMIF devices? Its entirely conceivable
>>> to have two memories like NAND and NOR flash connect to two different
>>> AEMIF interfaces.
>>>
>> Ivan wanted me to clarify the Keystone hardware which supports
>> 1 instance of controller with 4 CS. That allows already four
>> devices to be connected. Currently NAND and NOR are connected on it
>> and two more slots are free.
>>
>> Since driver support what hardware is, lets not build a driver for
>> hardware which don't exist. And if at all such a support would be
>> needed in future, we can always add it.
>
> I understand the lack of hardware part, but its common to write the
> driver such that it can handle multiple instances. Is there any gain on
> current hardware because of this check? From what I am hearing, the code
> in question wont be exercised at all. So why go all the way and add it
> in first place?
>
Fair enough. The check can be dropped.
Regards,
Santosh
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH 07/12] memory: davinci-aemif: introduce AEMIF driver
2013-11-26 18:26 ` Santosh Shilimkar
@ 2013-11-26 18:29 ` ivan.khoronzhuk
2013-11-27 0:37 ` Brian Norris
1 sibling, 0 replies; 56+ messages in thread
From: ivan.khoronzhuk @ 2013-11-26 18:29 UTC (permalink / raw)
To: Santosh Shilimkar, Sekhar Nori
Cc: Mark Rutland, devicetree, Strashko, Grygorii, Russell King,
Pawel Moll, Stephen Warren, Ian Campbell, Kumar Gala,
Rob Herring, linux-kernel, linux-mtd, Rob Landley,
linux-arm-kernel
On 11/26/2013 08:26 PM, Santosh Shilimkar wrote:
> Fair enough
Ok, I will do
--
Regards,
Ivan Khoronzhuk
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH 07/12] memory: davinci-aemif: introduce AEMIF driver
2013-11-26 17:44 ` ivan.khoronzhuk
@ 2013-11-26 18:30 ` Santosh Shilimkar
2013-11-26 18:30 ` ivan.khoronzhuk
2013-11-27 4:05 ` Sekhar Nori
1 sibling, 1 reply; 56+ messages in thread
From: Santosh Shilimkar @ 2013-11-26 18:30 UTC (permalink / raw)
To: ivan.khoronzhuk
Cc: Mark Rutland, devicetree, Strashko, Grygorii, Russell King,
Pawel Moll, Stephen Warren, Ian Campbell, Sekhar Nori,
Kumar Gala, Rob Herring, linux-kernel, linux-mtd, Rob Landley,
linux-arm-kernel
Ivan,
On Tuesday 26 November 2013 12:44 PM, ivan.khoronzhuk wrote:
> On 11/26/2013 09:20 AM, Sekhar Nori wrote:
>> On Monday 11 November 2013 10:36 PM, Khoronzhuk, Ivan wrote:
>>> +static struct platform_driver davinci_aemif_driver = {
>>> + .probe = davinci_aemif_probe,
>>> + .remove = davinci_aemif_remove,
>>> + .driver = {
>>> + .name = DRV_NAME,
>>> + .owner = THIS_MODULE,
>>> + .of_match_table = of_match_ptr(davinci_aemif_of_match),
>>> + },
>>> +};
>>> +
>>> +module_platform_driver(davinci_aemif_driver);
>>> +
>>> +MODULE_AUTHOR("Murali Karicheri <m-karicheri2@ti.com>");
>>
>> Is this correct? If yes, I would have expected him to sign-off on the
>> patch too. If not, please drop this.
>
> I will add sign-off
>
Please add you as a MODULE_AUTHOR as well since you re-wrote the
downstream driver. Ok to have both of you sign of on it.
Regards,
Santosh
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH 07/12] memory: davinci-aemif: introduce AEMIF driver
2013-11-26 18:30 ` Santosh Shilimkar
@ 2013-11-26 18:30 ` ivan.khoronzhuk
0 siblings, 0 replies; 56+ messages in thread
From: ivan.khoronzhuk @ 2013-11-26 18:30 UTC (permalink / raw)
To: Santosh Shilimkar
Cc: Mark Rutland, devicetree, Strashko, Grygorii, Russell King,
Pawel Moll, Stephen Warren, Ian Campbell, Sekhar Nori,
Kumar Gala, Rob Herring, linux-kernel, linux-mtd, Rob Landley,
linux-arm-kernel
On 11/26/2013 08:30 PM, Santosh Shilimkar wrote:
> Ivan,
>
> On Tuesday 26 November 2013 12:44 PM, ivan.khoronzhuk wrote:
>> On 11/26/2013 09:20 AM, Sekhar Nori wrote:
>>> On Monday 11 November 2013 10:36 PM, Khoronzhuk, Ivan wrote:
>
>>>> +static struct platform_driver davinci_aemif_driver = {
>>>> + .probe = davinci_aemif_probe,
>>>> + .remove = davinci_aemif_remove,
>>>> + .driver = {
>>>> + .name = DRV_NAME,
>>>> + .owner = THIS_MODULE,
>>>> + .of_match_table = of_match_ptr(davinci_aemif_of_match),
>>>> + },
>>>> +};
>>>> +
>>>> +module_platform_driver(davinci_aemif_driver);
>>>> +
>>>> +MODULE_AUTHOR("Murali Karicheri <m-karicheri2@ti.com>");
>>>
>>> Is this correct? If yes, I would have expected him to sign-off on the
>>> patch too. If not, please drop this.
>>
>> I will add sign-off
>>
> Please add you as a MODULE_AUTHOR as well since you re-wrote the
> downstream driver. Ok to have both of you sign of on it.
>
> Regards,
> Santosh
>
Ok
--
Regards,
Ivan Khoronzhuk
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH 07/12] memory: davinci-aemif: introduce AEMIF driver
2013-11-26 18:26 ` Santosh Shilimkar
2013-11-26 18:29 ` ivan.khoronzhuk
@ 2013-11-27 0:37 ` Brian Norris
2013-11-27 10:22 ` ivan.khoronzhuk
1 sibling, 1 reply; 56+ messages in thread
From: Brian Norris @ 2013-11-27 0:37 UTC (permalink / raw)
To: Santosh Shilimkar
Cc: Mark Rutland, devicetree, Strashko, Grygorii, Russell King,
Pawel Moll, Stephen Warren, Ian Campbell, Sekhar Nori,
Kumar Gala, Rob Herring, linux-kernel, linux-mtd, Rob Landley,
Khoronzhuk, Ivan, linux-arm-kernel
On Tue, Nov 26, 2013 at 01:26:44PM -0500, Santosh Shilimkar wrote:
> On Tuesday 26 November 2013 12:21 PM, Sekhar Nori wrote:
> > On 11/26/2013 8:35 PM, Santosh Shilimkar wrote:
> >> On Tuesday 26 November 2013 02:20 AM, Sekhar Nori wrote:
> >>> On Monday 11 November 2013 10:36 PM, Khoronzhuk, Ivan wrote:
> >>>> +static int davinci_aemif_probe(struct platform_device *pdev)
> >>>> +{
> >>>> + int ret = -ENODEV, i;
> >>>> + struct resource *res;
> >>>> + struct device *dev = &pdev->dev;
> >>>> + struct device_node *np = dev->of_node;
> >>>> +
> >>>> + if (np == NULL)
> >>>> + return 0;
> >>>> +
> >>>> + if (aemif) {
> >>>> + dev_err(dev, "davinci_aemif driver is in use currently\n");
> >>>> + return -EBUSY;
> >>>> + }
> >>>
> >>> Why expressly prevent multiple AEMIF devices? Its entirely conceivable
> >>> to have two memories like NAND and NOR flash connect to two different
> >>> AEMIF interfaces.
> >>>
> >> Ivan wanted me to clarify the Keystone hardware which supports
> >> 1 instance of controller with 4 CS. That allows already four
> >> devices to be connected. Currently NAND and NOR are connected on it
> >> and two more slots are free.
> >>
> >> Since driver support what hardware is, lets not build a driver for
> >> hardware which don't exist. And if at all such a support would be
> >> needed in future, we can always add it.
> >
> > I understand the lack of hardware part, but its common to write the
> > driver such that it can handle multiple instances. Is there any gain on
> > current hardware because of this check? From what I am hearing, the code
> > in question wont be exercised at all. So why go all the way and add it
> > in first place?
> >
> Fair enough. The check can be dropped.
Hmm, while the sentiment expressed by Sekhar is noble (to avoid
unnecessarily constraining the driver), removing the check is not
enough. You're still using a global static variable 'aemif', and it is
important not to accidentally re-use this struct if a second device ever
becomes available. So for the current implementation, the check is
necessary IMO. If instead, you were to make 'aemif' a per-device struct
(like with platform_set_drvdata()), then you would not have this issue.
Brian
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH 07/12] memory: davinci-aemif: introduce AEMIF driver
2013-11-26 17:44 ` ivan.khoronzhuk
2013-11-26 18:30 ` Santosh Shilimkar
@ 2013-11-27 4:05 ` Sekhar Nori
1 sibling, 0 replies; 56+ messages in thread
From: Sekhar Nori @ 2013-11-27 4:05 UTC (permalink / raw)
To: ivan.khoronzhuk, Shilimkar, Santosh, Rob Landley, Russell King
Cc: Mark Rutland, devicetree, Strashko, Grygorii, Pawel Moll,
Ian Campbell, Stephen Warren, Kumar Gala, Rob Herring,
linux-kernel, linux-mtd, linux-arm-kernel
On Tuesday 26 November 2013 11:14 PM, ivan.khoronzhuk wrote:
>>> +static int davinci_aemif_probe(struct platform_device *pdev)
>>> +{
>>> + int ret = -ENODEV, i;
>>> + struct resource *res;
>>> + struct device *dev = &pdev->dev;
>>> + struct device_node *np = dev->of_node;
>>> +
>>> + if (np == NULL)
>>> + return 0;
>>> +
>>> + if (aemif) {
>>> + dev_err(dev, "davinci_aemif driver is in use currently\n");
>>> + return -EBUSY;
>>> + }
>>
>> Why expressly prevent multiple AEMIF devices? Its entirely conceivable
>> to have two memories like NAND and NOR flash connect to two different
>> AEMIF interfaces.
>>
>
> It can be, but I'm not sure if it is needed. Currently I've not seen case where
> more than 2 cses were used, I mean we have 2 cs free, why do we need the second AEMIF
> controller?
One usual reason is pinmux constraints. Its probably not a concern on
the device you are working with right now but as devices get smaller,
functionality on pins is multiplexed to handle multiple different use cases.
Thanks,
Sekhar
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH] ARM: davinci: aemif: get rid of davinci-nand driver dependency on aemif
2013-11-25 20:00 ` [PATCH] ARM: davinci: aemif: get rid of davinci-nand driver dependency on aemif Ivan Khoronzhuk
@ 2013-11-27 8:35 ` Sekhar Nori
2013-11-27 11:01 ` ivan.khoronzhuk
0 siblings, 1 reply; 56+ messages in thread
From: Sekhar Nori @ 2013-11-27 8:35 UTC (permalink / raw)
To: Ivan Khoronzhuk
Cc: mark.rutland, devicetree, grygorii.strashko, linux, pawel.moll,
swarren, ijc+devicetree, galak, rob.herring, linux-kernel,
linux-mtd, rob, Brian Norris, David Woodhouse, linux-arm-kernel
+ MTD maintainers
On Tuesday 26 November 2013 01:30 AM, Ivan Khoronzhuk wrote:
> The problem that the set timings code contains the call of Davinci
> platform function davinci_aemif_setup_timing() which is not
> accessible if kernel is built for another platform like Keystone.
>
> The Keysone platform is going to use TI AEMIF driver.
> If TI AEMIF is used we don't need to set timings and bus width.
> It is done by AEMIF driver.
>
> To get rid of davinci-nand driver dependency on aemif platform code
> we moved aemif code to davinci platform.
>
> The platform AEMIF code (aemif.c) has to be removed once Davinci
> will be converted to DT and use ti-aemif.c driver.
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
> ---
> arch/arm/mach-davinci/aemif.c | 73 ++++++++++++++++++++++-
> arch/arm/mach-davinci/board-da830-evm.c | 3 +
> arch/arm/mach-davinci/board-da850-evm.c | 3 +
> arch/arm/mach-davinci/board-dm355-evm.c | 5 ++
> arch/arm/mach-davinci/board-dm355-leopard.c | 5 ++
> arch/arm/mach-davinci/board-dm365-evm.c | 4 ++
> arch/arm/mach-davinci/board-dm644x-evm.c | 5 ++
> arch/arm/mach-davinci/board-dm646x-evm.c | 3 +
> arch/arm/mach-davinci/board-mityomapl138.c | 3 +
> arch/arm/mach-davinci/board-neuros-osd2.c | 9 ++-
> arch/arm/mach-davinci/devices-tnetv107x.c | 3 +
> drivers/mtd/nand/davinci_nand.c | 23 -------
> include/linux/platform_data/mtd-davinci-aemif.h | 5 +-
> 13 files changed, 116 insertions(+), 28 deletions(-)
[...]
> +
> +/**
> + * davinci_aemif_setup - setup AEMIF interface by davinci_nand_pdata
> + * @pdev - link to platform device to setup settings for
> + *
> + * This function does not use any locking while programming the AEMIF
> + * because it is expected that there is only one user of a given
> + * chip-select.
> + *
> + * Returns 0 on success, else negative errno.
> + */
> +int davinci_aemif_setup(struct platform_device *pdev)
> +{
> + struct davinci_nand_pdata *pdata = dev_get_platdata(&pdev->dev);
> + uint32_t val;
> + struct resource *res;
> + void __iomem *base;
> + int ret = 0;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> + if (!res) {
> + dev_err(&pdev->dev, "cannot get IORESOURCE_MEM\n");
> + return -ENOMEM;
> + }
> +
> + base = ioremap(res->start, resource_size(res));
> + if (!base) {
> + dev_err(&pdev->dev, "ioremap failed for resource %pR\n", res);
> + ret = -ENOMEM;
> + goto err;
> + }
> +
> + /*
> + * Setup Async configuration register in case we did not boot
> + * from NAND and so bootloader did not bother to set it up.
> + */
> + val = davinci_aemif_readl(base, A1CR_OFFSET + pdev->id * 4);
The AEMIF clock has to be enabled before this. The NAND driver might
load much later.
> + /*
> + * Extended Wait is not valid and Select Strobe mode is not
> + * used
> + */
> + val &= ~(ACR_ASIZE_MASK | ACR_EW_MASK | ACR_SS_MASK);
> + if (pdata->options & NAND_BUSWIDTH_16)
> + val |= 0x1;
> +
> + davinci_aemif_writel(base, A1CR_OFFSET + pdev->id * 4, val);
> +
> + if (pdata->timing)
> + ret = davinci_aemif_setup_timing(pdata->timing, base, pdev->id);
> +
> + if (ret < 0)
> + dev_dbg(&pdev->dev, "NAND timing values setup fail\n");
> +
> +err:
> + iounmap(base);
> + return ret;
> +}
> +EXPORT_SYMBOL(davinci_aemif_setup);
No need to export this symbol as nothing apart from platform code uses it.
> static const short mityomap_mii_pins[] = {
> diff --git a/arch/arm/mach-davinci/board-neuros-osd2.c b/arch/arm/mach-davinci/board-neuros-osd2.c
> index bb680af..a7d6668 100644
> --- a/arch/arm/mach-davinci/board-neuros-osd2.c
> +++ b/arch/arm/mach-davinci/board-neuros-osd2.c
> @@ -31,6 +31,7 @@
> #include <linux/platform_data/mmc-davinci.h>
> #include <linux/platform_data/mtd-davinci.h>
> #include <linux/platform_data/usb-davinci.h>
> +#include <linux/platform_data/mtd-davinci-aemif.h>
>
> #include <asm/mach-types.h>
> #include <asm/mach/arch.h>
> @@ -192,9 +193,15 @@ static __init void davinci_ntosd2_init(void)
> davinci_cfg_reg(DM644X_ATAEN_DISABLE);
>
> /* only one device will be jumpered and detected */
> - if (HAS_NAND)
> + if (HAS_NAND) {
> platform_device_register(
> &davinci_ntosd2_nandflash_device);
> +
> + if (davinci_aemif_setup(
> + &davinci_ntosd2_nandflash_device))
> + pr_warn("%s: Cannot configure AEMIF.\n",
> + __func__);
This is looking really ugly. Can you shorten
davinci_ntosd2_nandflash_device to just "ntosd2_nandflash" or similar?
I am yet to test this. Will test once the next version is posted.
Overall, I cannot say I am fan of this approach (mostly because we are
ending up having two drivers for the same hardware in kernel). But given
that the other option of adding platform device support to AEMIF driver
is not acceptable to you, I guess I will live with this.
Thanks,
Sekhar
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH 07/12] memory: davinci-aemif: introduce AEMIF driver
2013-11-27 0:37 ` Brian Norris
@ 2013-11-27 10:22 ` ivan.khoronzhuk
0 siblings, 0 replies; 56+ messages in thread
From: ivan.khoronzhuk @ 2013-11-27 10:22 UTC (permalink / raw)
To: Brian Norris, Santosh Shilimkar
Cc: Mark Rutland, devicetree, Strashko, Grygorii, Russell King,
Pawel Moll, Stephen Warren, Ian Campbell, Sekhar Nori,
Kumar Gala, Rob Herring, linux-kernel, linux-mtd, Rob Landley,
linux-arm-kernel
On 11/27/2013 02:37 AM, Brian Norris wrote:
> On Tue, Nov 26, 2013 at 01:26:44PM -0500, Santosh Shilimkar wrote:
>> On Tuesday 26 November 2013 12:21 PM, Sekhar Nori wrote:
>>> On 11/26/2013 8:35 PM, Santosh Shilimkar wrote:
>>>> On Tuesday 26 November 2013 02:20 AM, Sekhar Nori wrote:
>>>>> On Monday 11 November 2013 10:36 PM, Khoronzhuk, Ivan wrote:
>>>>>> +static int davinci_aemif_probe(struct platform_device *pdev)
>>>>>> +{
>>>>>> + int ret = -ENODEV, i;
>>>>>> + struct resource *res;
>>>>>> + struct device *dev = &pdev->dev;
>>>>>> + struct device_node *np = dev->of_node;
>>>>>> +
>>>>>> + if (np == NULL)
>>>>>> + return 0;
>>>>>> +
>>>>>> + if (aemif) {
>>>>>> + dev_err(dev, "davinci_aemif driver is in use currently\n");
>>>>>> + return -EBUSY;
>>>>>> + }
>>>>>
>>>>> Why expressly prevent multiple AEMIF devices? Its entirely conceivable
>>>>> to have two memories like NAND and NOR flash connect to two different
>>>>> AEMIF interfaces.
>>>>>
>>>> Ivan wanted me to clarify the Keystone hardware which supports
>>>> 1 instance of controller with 4 CS. That allows already four
>>>> devices to be connected. Currently NAND and NOR are connected on it
>>>> and two more slots are free.
>>>>
>>>> Since driver support what hardware is, lets not build a driver for
>>>> hardware which don't exist. And if at all such a support would be
>>>> needed in future, we can always add it.
>>>
>>> I understand the lack of hardware part, but its common to write the
>>> driver such that it can handle multiple instances. Is there any gain on
>>> current hardware because of this check? From what I am hearing, the code
>>> in question wont be exercised at all. So why go all the way and add it
>>> in first place?
>>>
>> Fair enough. The check can be dropped.
>
> Hmm, while the sentiment expressed by Sekhar is noble (to avoid
> unnecessarily constraining the driver), removing the check is not
> enough. You're still using a global static variable 'aemif', and it is
> important not to accidentally re-use this struct if a second device ever
> becomes available. So for the current implementation, the check is
> necessary IMO. If instead, you were to make 'aemif' a per-device struct
> (like with platform_set_drvdata()), then you would not have this issue.
>
> Brian
>
Yes, that is the plan to make it a per-device.
--
Regards,
Ivan Khoronzhuk
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH] ARM: davinci: aemif: get rid of davinci-nand driver dependency on aemif
2013-11-27 8:35 ` Sekhar Nori
@ 2013-11-27 11:01 ` ivan.khoronzhuk
2013-11-27 13:07 ` Sekhar Nori
0 siblings, 1 reply; 56+ messages in thread
From: ivan.khoronzhuk @ 2013-11-27 11:01 UTC (permalink / raw)
To: Sekhar Nori
Cc: mark.rutland, devicetree, grygorii.strashko, linux, pawel.moll,
swarren, ijc+devicetree, galak, rob.herring, linux-kernel,
linux-mtd, rob, Brian Norris, David Woodhouse, linux-arm-kernel
On 11/27/2013 10:35 AM, Sekhar Nori wrote:
> + MTD maintainers
>
> On Tuesday 26 November 2013 01:30 AM, Ivan Khoronzhuk wrote:
>> The problem that the set timings code contains the call of Davinci
>> platform function davinci_aemif_setup_timing() which is not
>> accessible if kernel is built for another platform like Keystone.
>>
>> The Keysone platform is going to use TI AEMIF driver.
>> If TI AEMIF is used we don't need to set timings and bus width.
>> It is done by AEMIF driver.
>>
>> To get rid of davinci-nand driver dependency on aemif platform code
>> we moved aemif code to davinci platform.
>>
>> The platform AEMIF code (aemif.c) has to be removed once Davinci
>> will be converted to DT and use ti-aemif.c driver.
>>
>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
>> ---
>> arch/arm/mach-davinci/aemif.c | 73 ++++++++++++++++++++++-
>> arch/arm/mach-davinci/board-da830-evm.c | 3 +
>> arch/arm/mach-davinci/board-da850-evm.c | 3 +
>> arch/arm/mach-davinci/board-dm355-evm.c | 5 ++
>> arch/arm/mach-davinci/board-dm355-leopard.c | 5 ++
>> arch/arm/mach-davinci/board-dm365-evm.c | 4 ++
>> arch/arm/mach-davinci/board-dm644x-evm.c | 5 ++
>> arch/arm/mach-davinci/board-dm646x-evm.c | 3 +
>> arch/arm/mach-davinci/board-mityomapl138.c | 3 +
>> arch/arm/mach-davinci/board-neuros-osd2.c | 9 ++-
>> arch/arm/mach-davinci/devices-tnetv107x.c | 3 +
>> drivers/mtd/nand/davinci_nand.c | 23 -------
>> include/linux/platform_data/mtd-davinci-aemif.h | 5 +-
>> 13 files changed, 116 insertions(+), 28 deletions(-)
>
> [...]
>
>> +
>> +/**
>> + * davinci_aemif_setup - setup AEMIF interface by davinci_nand_pdata
>> + * @pdev - link to platform device to setup settings for
>> + *
>> + * This function does not use any locking while programming the AEMIF
>> + * because it is expected that there is only one user of a given
>> + * chip-select.
>> + *
>> + * Returns 0 on success, else negative errno.
>> + */
>> +int davinci_aemif_setup(struct platform_device *pdev)
>> +{
>> + struct davinci_nand_pdata *pdata = dev_get_platdata(&pdev->dev);
>> + uint32_t val;
>> + struct resource *res;
>> + void __iomem *base;
>> + int ret = 0;
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>> + if (!res) {
>> + dev_err(&pdev->dev, "cannot get IORESOURCE_MEM\n");
>> + return -ENOMEM;
>> + }
>> +
>> + base = ioremap(res->start, resource_size(res));
>> + if (!base) {
>> + dev_err(&pdev->dev, "ioremap failed for resource %pR\n", res);
>> + ret = -ENOMEM;
>> + goto err;
>> + }
>> +
>> + /*
>> + * Setup Async configuration register in case we did not boot
>> + * from NAND and so bootloader did not bother to set it up.
>> + */
>> + val = davinci_aemif_readl(base, A1CR_OFFSET + pdev->id * 4);
>
> The AEMIF clock has to be enabled before this. The NAND driver might
> load much later.
>
Ok
>> + /*
>> + * Extended Wait is not valid and Select Strobe mode is not
>> + * used
>> + */
>> + val &= ~(ACR_ASIZE_MASK | ACR_EW_MASK | ACR_SS_MASK);
>> + if (pdata->options & NAND_BUSWIDTH_16)
>> + val |= 0x1;
>> +
>> + davinci_aemif_writel(base, A1CR_OFFSET + pdev->id * 4, val);
>> +
>> + if (pdata->timing)
>> + ret = davinci_aemif_setup_timing(pdata->timing, base, pdev->id);
>> +
>> + if (ret < 0)
>> + dev_dbg(&pdev->dev, "NAND timing values setup fail\n");
>> +
>> +err:
>> + iounmap(base);
>> + return ret;
>> +}
>> +EXPORT_SYMBOL(davinci_aemif_setup);
>
> No need to export this symbol as nothing apart from platform code uses it.
>
Ok
>> static const short mityomap_mii_pins[] = {
>> diff --git a/arch/arm/mach-davinci/board-neuros-osd2.c b/arch/arm/mach-davinci/board-neuros-osd2.c
>> index bb680af..a7d6668 100644
>> --- a/arch/arm/mach-davinci/board-neuros-osd2.c
>> +++ b/arch/arm/mach-davinci/board-neuros-osd2.c
>> @@ -31,6 +31,7 @@
>> #include <linux/platform_data/mmc-davinci.h>
>> #include <linux/platform_data/mtd-davinci.h>
>> #include <linux/platform_data/usb-davinci.h>
>> +#include <linux/platform_data/mtd-davinci-aemif.h>
>>
>> #include <asm/mach-types.h>
>> #include <asm/mach/arch.h>
>> @@ -192,9 +193,15 @@ static __init void davinci_ntosd2_init(void)
>> davinci_cfg_reg(DM644X_ATAEN_DISABLE);
>>
>> /* only one device will be jumpered and detected */
>> - if (HAS_NAND)
>> + if (HAS_NAND) {
>> platform_device_register(
>> &davinci_ntosd2_nandflash_device);
>> +
>> + if (davinci_aemif_setup(
>> + &davinci_ntosd2_nandflash_device))
>> + pr_warn("%s: Cannot configure AEMIF.\n",
>> + __func__);
>
> This is looking really ugly. Can you shorten
> davinci_ntosd2_nandflash_device to just "ntosd2_nandflash" or similar?
The rename is not related to the patch, so I won't do this.
>
> I am yet to test this. Will test once the next version is posted.
>
> Overall, I cannot say I am fan of this approach (mostly because we are
> ending up having two drivers for the same hardware in kernel). But given
> that the other option of adding platform device support to AEMIF driver
> is not acceptable to you, I guess I will live with this.
>
> Thanks,
> Sekhar
>
--
Regards,
Ivan Khoronzhuk
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH] ARM: davinci: aemif: get rid of davinci-nand driver dependency on aemif
2013-11-27 11:01 ` ivan.khoronzhuk
@ 2013-11-27 13:07 ` Sekhar Nori
2013-11-27 13:21 ` ivan.khoronzhuk
0 siblings, 1 reply; 56+ messages in thread
From: Sekhar Nori @ 2013-11-27 13:07 UTC (permalink / raw)
To: ivan.khoronzhuk
Cc: mark.rutland, devicetree, grygorii.strashko, linux, pawel.moll,
swarren, ijc+devicetree, galak, rob.herring, linux-kernel,
linux-mtd, rob, Brian Norris, David Woodhouse, linux-arm-kernel
On Wednesday 27 November 2013 04:31 PM, ivan.khoronzhuk wrote:
>>> @@ -192,9 +193,15 @@ static __init void davinci_ntosd2_init(void)
>>> davinci_cfg_reg(DM644X_ATAEN_DISABLE);
>>>
>>> /* only one device will be jumpered and detected */
>>> - if (HAS_NAND)
>>> + if (HAS_NAND) {
>>> platform_device_register(
>>> &davinci_ntosd2_nandflash_device);
>>> +
>>> + if (davinci_aemif_setup(
>>> + &davinci_ntosd2_nandflash_device))
>>> + pr_warn("%s: Cannot configure AEMIF.\n",
>>> + __func__);
>>
>> This is looking really ugly. Can you shorten
>> davinci_ntosd2_nandflash_device to just "ntosd2_nandflash" or similar?
>
> The rename is not related to the patch, so I won't do this.
The reason I asked you to rename is not because I want to push some
random clean-up into this patch. You had to introduce a line so broken
that its almost unreadable. I offered the suggestion as a way to improve
the readability of the code you are introducing in *this* patch.
Thanks,
Sekhar
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH] ARM: davinci: aemif: get rid of davinci-nand driver dependency on aemif
2013-11-27 13:07 ` Sekhar Nori
@ 2013-11-27 13:21 ` ivan.khoronzhuk
0 siblings, 0 replies; 56+ messages in thread
From: ivan.khoronzhuk @ 2013-11-27 13:21 UTC (permalink / raw)
To: Sekhar Nori
Cc: mark.rutland, devicetree, grygorii.strashko, linux, pawel.moll,
swarren, ijc+devicetree, galak, rob.herring, linux-kernel,
linux-mtd, rob, Brian Norris, David Woodhouse, linux-arm-kernel
On 11/27/2013 03:07 PM, Sekhar Nori wrote:
> On Wednesday 27 November 2013 04:31 PM, ivan.khoronzhuk wrote:
>>>> @@ -192,9 +193,15 @@ static __init void davinci_ntosd2_init(void)
>>>> davinci_cfg_reg(DM644X_ATAEN_DISABLE);
>>>>
>>>> /* only one device will be jumpered and detected */
>>>> - if (HAS_NAND)
>>>> + if (HAS_NAND) {
>>>> platform_device_register(
>>>> &davinci_ntosd2_nandflash_device);
>>>> +
>>>> + if (davinci_aemif_setup(
>>>> + &davinci_ntosd2_nandflash_device))
>>>> + pr_warn("%s: Cannot configure AEMIF.\n",
>>>> + __func__);
>>>
>>> This is looking really ugly. Can you shorten
>>> davinci_ntosd2_nandflash_device to just "ntosd2_nandflash" or similar?
>>
>> The rename is not related to the patch, so I won't do this.
>
> The reason I asked you to rename is not because I want to push some
> random clean-up into this patch. You had to introduce a line so broken
> that its almost unreadable. I offered the suggestion as a way to improve
> the readability of the code you are introducing in *this* patch.
>
> Thanks,
> Sekhar
>
Ok, I will rename
--
Regards,
Ivan Khoronzhuk
^ permalink raw reply [flat|nested] 56+ messages in thread
end of thread, other threads:[~2013-11-27 13:21 UTC | newest]
Thread overview: 56+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <1384187188-5776-1-git-send-email-ivan.khoronzhuk@ti.com>
2013-11-11 16:47 ` [PATCH 00/12] Introduce davinci AEMIF driver Khoronzhuk, Ivan
[not found] ` <1384187188-5776-2-git-send-email-ivan.khoronzhuk@ti.com>
2013-11-11 16:52 ` [PATCH 01/12] mtd: nand: davinci: fix driver registration Khoronzhuk, Ivan
2013-11-12 15:45 ` Santosh Shilimkar
[not found] ` <1384187188-5776-3-git-send-email-ivan.khoronzhuk@ti.com>
2013-11-11 16:53 ` [PATCH 02/12] mtd: nand: davinci: check required ti,davinci-chipselect property Khoronzhuk, Ivan
2013-11-12 15:50 ` [PATCH 02/12] mtd: nand: davinci: check required ti, davinci-chipselect property Santosh Shilimkar
2013-11-18 18:25 ` ivan.khoronzhuk
[not found] ` <1384187188-5776-4-git-send-email-ivan.khoronzhuk@ti.com>
2013-11-11 16:54 ` [PATCH 03/12] mtd: nand: davinci: simplify error handling Khoronzhuk, Ivan
2013-11-12 15:52 ` Santosh Shilimkar
[not found] ` <1384187188-5776-5-git-send-email-ivan.khoronzhuk@ti.com>
2013-11-11 16:55 ` [PATCH 04/12] mtd: nand: davinci: move bindings under mtd Khoronzhuk, Ivan
2013-11-12 15:53 ` Santosh Shilimkar
[not found] ` <1384187188-5776-6-git-send-email-ivan.khoronzhuk@ti.com>
2013-11-11 16:58 ` [PATCH 05/12] mtd: nand: davinci: extend description of bindings Khoronzhuk, Ivan
2013-11-12 15:55 ` Santosh Shilimkar
[not found] ` <1384187188-5776-7-git-send-email-ivan.khoronzhuk@ti.com>
2013-11-11 17:01 ` [PATCH 06/12] mtd: nand: davinci: adjust DT properties to MTD generic Khoronzhuk, Ivan
2013-11-26 7:03 ` Sekhar Nori
2013-11-26 10:30 ` Grygorii Strashko
2013-11-26 10:49 ` ivan.khoronzhuk
[not found] ` <1384187188-5776-8-git-send-email-ivan.khoronzhuk@ti.com>
2013-11-11 17:06 ` [PATCH 07/12] memory: davinci-aemif: introduce AEMIF driver Khoronzhuk, Ivan
2013-11-12 16:08 ` Santosh Shilimkar
2013-11-18 19:15 ` ivan.khoronzhuk
2013-11-26 7:20 ` Sekhar Nori
2013-11-26 15:05 ` Santosh Shilimkar
2013-11-26 17:21 ` Sekhar Nori
2013-11-26 18:26 ` Santosh Shilimkar
2013-11-26 18:29 ` ivan.khoronzhuk
2013-11-27 0:37 ` Brian Norris
2013-11-27 10:22 ` ivan.khoronzhuk
2013-11-26 17:44 ` ivan.khoronzhuk
2013-11-26 18:30 ` Santosh Shilimkar
2013-11-26 18:30 ` ivan.khoronzhuk
2013-11-27 4:05 ` Sekhar Nori
[not found] ` <1384187188-5776-9-git-send-email-ivan.khoronzhuk@ti.com>
2013-11-11 17:09 ` [PATCH 08/12] memory: davinci-aemif: add bindings for " Khoronzhuk, Ivan
[not found] ` <1384187188-5776-10-git-send-email-ivan.khoronzhuk@ti.com>
2013-11-11 17:09 ` [PATCH 09/12] mtd: nand: davinci: reuse driver for Keystone arch Khoronzhuk, Ivan
2013-11-12 16:09 ` Santosh Shilimkar
[not found] ` <1384187188-5776-11-git-send-email-ivan.khoronzhuk@ti.com>
2013-11-11 17:10 ` [PATCH 10/12] mtd: nand: davinci: don't set timings if AEMIF is used Khoronzhuk, Ivan
2013-11-12 16:13 ` Santosh Shilimkar
2013-11-18 11:35 ` Grygorii Strashko
2013-11-18 14:08 ` Santosh Shilimkar
2013-11-18 19:32 ` ivan.khoronzhuk
2013-11-13 5:02 ` Sekhar Nori
2013-11-13 14:14 ` Santosh Shilimkar
2013-11-14 10:53 ` Sekhar Nori
2013-11-14 14:36 ` Santosh Shilimkar
2013-11-18 19:35 ` ivan.khoronzhuk
2013-11-21 17:07 ` Sekhar Nori
2013-11-22 17:38 ` Santosh Shilimkar
2013-11-24 9:46 ` Sekhar Nori
2013-11-25 20:00 ` [PATCH] ARM: davinci: aemif: get rid of davinci-nand driver dependency on aemif Ivan Khoronzhuk
2013-11-27 8:35 ` Sekhar Nori
2013-11-27 11:01 ` ivan.khoronzhuk
2013-11-27 13:07 ` Sekhar Nori
2013-11-27 13:21 ` ivan.khoronzhuk
2013-11-26 0:55 ` [PATCH 10/12] mtd: nand: davinci: don't set timings if AEMIF is used Santosh Shilimkar
[not found] ` <1384187188-5776-12-git-send-email-ivan.khoronzhuk@ti.com>
2013-11-11 17:12 ` [PATCH 11/12] mtd: nand: davinci: don't request AEMIF address range Khoronzhuk, Ivan
2013-11-12 16:15 ` Santosh Shilimkar
[not found] ` <1384187188-5776-13-git-send-email-ivan.khoronzhuk@ti.com>
2013-11-11 17:13 ` [PATCH 12/12] arm: dts: keystone: add AEMIF/NAND device entry Khoronzhuk, Ivan
2013-11-12 16:19 ` Santosh Shilimkar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).