Re: [PATCH v4 0/4] mtd:nand:omap2: clean-up of supported ECC schemes
On Wed, 2013-07-03 at 13:16 +, Gupta, Pekon wrote: [Pekon]: Yes, I'm not seeing these build issues, as I'm cleanly returning from probe with pr_err(), if the required libraries (/lib/bch.c) are not build-in the system. --- [Patch v4 1/4]: mtd:nand:omap2: clean-up BCHx_HW and BCHx_SW ECC.. @@static int omap_nand_probe(struct platform_device *pdev) + default: + pr_err(selected ECC scheme not supported or not enabled\n); + err = -EINVAL; + goto out_release_mem_region; + } --- However, if you are still seeing this, could you please send me your config? I compile tested your patches too, and did not see any issues with my omap2_defconfig. -- Best Regards, Artem Bityutskiy ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [patch] mtd: denali_dt: harmless case of testing the wrong variable
On Mon, 2013-04-01 at 09:21 +0300, Dan Carpenter wrote: There is a warning message that can't be printed because we test the wrong variable. Signed-off-by: Dan Carpenter dan.carpen...@oracle.com Thanks Dan, but there is already a similar fix for this in upstream: commit 6c67050ab108bcce2576fa1d7a0557eb0376520a Author: Sachin Kamat sachin.ka...@linaro.org -- Best Regards, Artem Bityutskiy ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] mtd: devices: elm: Removes xx literals in elm DT node
On Thu, 2013-01-24 at 12:23 +0530, Philip Avinash wrote: As part of removing generalized dependency, replace xx literal fields in DT compatible field with 52 for am335x platforms. Signed-off-by: Philip Avinash avinashphi...@ti.com Pushed to l2-mtd.git, thanks! -- Best Regards, Artem Bityutskiy signature.asc Description: This is a digitally signed message part ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] mtd: physmap_of: fix cmdline partition method w/o linux,mtd-name
I pinged dwmw2 about this already. This is his call to merge the driver. I will ping him again. On Thu, 2013-01-24 at 11:05 +0200, Baruch Siach wrote: Hi linux-mtd, Artem, David, Ping? It's been a month, and this is a simple uninitialized pointer dereference bug fix. baruch On Sun, Dec 23, 2012 at 01:10:50PM +0200, Baruch Siach wrote: Commit d68cbdd4fb (mtd: physmap_of: allow to specify the mtd name for retro compatiblity) broke cmdline partitioning using dev_name() in the kernel command line. of_property_read_string() does not touch mtd_name when linux,mtd-name is not present in the device tree, which causes map.name to be set to a random value. Fix this by initializing mtd_name to NULL. Cc: Jean-Christophe PLAGNIOL-VILLARD plagn...@jcrosoft.com Signed-off-by: Baruch Siach bar...@tkos.co.il --- drivers/mtd/maps/physmap_of.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/mtd/maps/physmap_of.c b/drivers/mtd/maps/physmap_of.c index 37cdc20..6fb2bd8 100644 --- a/drivers/mtd/maps/physmap_of.c +++ b/drivers/mtd/maps/physmap_of.c @@ -170,7 +170,7 @@ static int of_flash_probe(struct platform_device *dev) resource_size_t res_size; struct mtd_part_parser_data ppdata; bool map_indirect; - const char *mtd_name; + const char *mtd_name = NULL; match = of_match_device(of_flash_match, dev-dev); if (!match) -- Best Regards, Artem Bityutskiy signature.asc Description: This is a digitally signed message part ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] mtd: physmap_of: fix cmdline partition method w/o linux,mtd-name
On Thu, 2013-01-24 at 11:05 +0200, Baruch Siach wrote: Hi linux-mtd, Artem, David, Ping? It's been a month, and this is a simple uninitialized pointer dereference bug fix. It is in linux-mtd now. -- Best Regards, Artem Bityutskiy signature.asc Description: This is a digitally signed message part ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v4 0/3] mtd: nand: OMAP: ELM error correction support for BCH ecc
On Wed, 2013-01-16 at 12:22 +, Philip, Avinash wrote: This series is based on linux 3.8-rc2 and tested with [1]. Also this patch series depend on [1] for NAND flash device tree data and gpmc nand device tree binding documentation updates. 1. [PATCH v7 0/5] OMAP GPMC DT bindings http://www.spinics.net/lists/linux-omap/msg83505.html Tested on am335x-evm for BCH 4 and 8 bit error correction. Can you apply this patch series on l2-mtd tree as it will help RBL compatibility ecc layout for NAND flash in am335x-platforms and hardware based BCH error correction. OK, I've applied them. I dropped the part that updates the documentation. Please, handle it separately when Daniel's patches are in. Pushed to l2-mtd.git, thanks! -- Best Regards, Artem Bityutskiy signature.asc Description: This is a digitally signed message part ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 3/4] Enable ecc-mode selection in the driver
On Wed, 2013-01-16 at 12:43 +0100, Stefan Peter wrote: Hi Andrew For some reasons I do not see your patches in the mailing lists which are in CC. -- Best Regards, Artem Bityutskiy signature.asc Description: This is a digitally signed message part ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v2] mtd: cfi_cmdset_0002: Support Persistent Protection Bits (PPB) locking
On Thu, 2012-12-13 at 09:29 +0100, Stefan Roese wrote: Currently cfi_cmdset_0002.c does not support PPB locking of sectors. This patch adds support for this locking/unlocking mechanism. It is needed on some platforms, since newer U-Boot versions do support this PPB locking and protect for example their environment sector(s) this way. Some #ifdef CONFIG_OFs are missing: drivers/mtd/chips/cfi_cmdset_0002.c: In function ‘cfi_cmdset_0002’: drivers/mtd/chips/cfi_cmdset_0002.c:579:4: error: implicit declaration of function ‘of_property_read_bool’ [-Werror=implicit-function-declaration] drivers/mtd/chips/cfi_cmdset_0002.c: In function ‘do_read_secsi_onechip’: drivers/mtd/chips/cfi_cmdset_0002.c:1146:16: warning: variable ‘timeo’ set but not used [-Wunused-but-set-variable] cc1: some warnings being treated as errors make[4]: *** [drivers/mtd/chips/cfi_cmdset_0002.o] Error 1 Please, check allso the timeo warning. -- Best Regards, Artem Bityutskiy signature.asc Description: This is a digitally signed message part ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v8 2/5] mtd: omap-nand: pass device_node in platform data
On Tue, 2013-01-15 at 10:52 -0800, Tony Lindgren wrote: Artem, Looks like this patch related to making GPMC work with DT was never sent to linux-mtd or to you so adding to cc. Is this OK to apply along with the GPMC patches, or do you want to take this separately with the MTD patches? Please, go ahead! -- Best Regards, Artem Bityutskiy signature.asc Description: This is a digitally signed message part ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v8 2/5] mtd: omap-nand: pass device_node in platform data
On Tue, 2013-01-15 at 11:26 -0800, Tony Lindgren wrote: * Artem Bityutskiy dedeki...@gmail.com [130115 11:12]: On Tue, 2013-01-15 at 10:52 -0800, Tony Lindgren wrote: Artem, Looks like this patch related to making GPMC work with DT was never sent to linux-mtd or to you so adding to cc. Is this OK to apply along with the GPMC patches, or do you want to take this separately with the MTD patches? Please, go ahead! OK thanks. Can I add your Acked-by too? Yes Tony, please, do: Acked-by: Artem Bityutskiy dedeki...@gmail.com -- Best Regards, Artem Bityutskiy signature.asc Description: This is a digitally signed message part ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] mtd: cfi_cmdset_0002: Support Persistent Protection Bits (PPB) locking
On Mon, 2012-12-10 at 19:40 +0100, Stefan Roese wrote:
On 12/10/2012 04:00 PM, Artem Bityutskiy wrote:
On Fri, 2012-12-07 at 08:22 +0100, Stefan Roese wrote:
+ /*
+* Wait for some time as unlocking of all sectors takes quite long
+*/
+ timeo = jiffies + (2 * HZ); /* 2s max (un)locking */
Please, use msecs_to_jiffies() instead.
Sure, thats better.
Would you please do this instead?
AFAIK, chip-mutex protects the access to the chip itself. So that
sequences are not interrupted.
I have to admit that I haven't looked into get_chip() so far. It seems
to handle a state machine. Normally (idle state) it will just fall
through (FL_READY).
So it looks like the idea is that you first take the mutex, then call
get_chip() which will wait for the chip becoming really ready, and then
you can safely use it.
Why you need to drop the mutex here?
Not sure, that might not be necessary. Copy and past from another loop
in the same file.
Probably from 'get_chip()' ?
Why is it not an ABBA deadlock to do this:
Task 1: In the loop above, has chip locked, doing
mutex_lock(chip-mutex);
Task 2: done mutex_lock(chip-mutex), now doing
ret = get_chip(map, chip, adr + chip-start, FL_LOCKING);
I don't see two different locks/mutexes (only A) here. As get_chip()
does no request any real mutex. Please correct me if I'm wrong.
Right, there is indeed no deadlock.
In many other places UDELAY() is called:
#define UDELAY(map, chip, adr, usec) \
do { \
mutex_unlock(chip-mutex); \
cfi_udelay(usec); \
mutex_lock(chip-mutex); \
} while (0)
Why not to use this as well then for consistency?
--
Best Regards,
Artem Bityutskiy
signature.asc
Description: This is a digitally signed message part
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] mtd: cfi_cmdset_0002: Support Persistent Protection Bits (PPB) locking
On Fri, 2012-12-07 at 08:22 +0100, Stefan Roese wrote:
+ /*
+* Wait for some time as unlocking of all sectors takes quite long
+*/
+ timeo = jiffies + (2 * HZ); /* 2s max (un)locking */
Please, use msecs_to_jiffies() instead.
+ for (;;) {
+ if (chip_ready(map, adr))
+ break;
+
+ if (time_after(jiffies, timeo)) {
+ printk(KERN_ERR Waiting for chip to be ready timed
out.\n);
+ ret = -EIO;
+ break;
+ }
+ mutex_unlock(chip-mutex);
+ cfi_udelay(1);
+ mutex_lock(chip-mutex);
+ }
Would you please educate me a bit and explain what is protected by
'chip-mutex' and by 'get_chip()'.
Why you need to drop the mutex here?
Why is it not an ABBA deadlock to do this:
Task 1: In the loop above, has chip locked, doing
mutex_lock(chip-mutex);
Task 2: done mutex_lock(chip-mutex), now doing
ret = get_chip(map, chip, adr + chip-start, FL_LOCKING);
--
Best Regards,
Artem Bityutskiy
signature.asc
Description: This is a digitally signed message part
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v2 3/3] mtd: nand: omap2: Support for hardware BCH error correction
On Wed, 2012-10-31 at 12:38 +0530, Philip, Avinash wrote:
+static int erased_sector_bitflips(u_char *data, u_char *oob,
+ struct omap_nand_info *info)
+{
+ int flip_bits = 0, i;
+
+ for (i = 0; i info-nand.ecc.size; i++) {
+ flip_bits += hweight8(~data[i]);
+ if (flip_bits info-nand.ecc.strength)
+ return 0;
+ }
+
+ for (i = 0; i info-nand.ecc.bytes - 1; i++) {
+ flip_bits += hweight8(~oob[i]);
+ if (flip_bits info-nand.ecc.strength)
+ return 0;
+ }
Why do you need the second for loop?
--
Best Regards,
Artem Bityutskiy
signature.asc
Description: This is a digitally signed message part
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v2 0/3] mtd: nand: OMAP: ELM error correction support for BCH ecc
On Thu, 2012-11-22 at 14:37 +, Philip, Avinash wrote: Idea here is to make faster scanning of erased page without bit flips. For omap nand driver ecc reported by hardware is non-zero and non 0xff. So comparing with the standard vector for erased page and skipping error correction for erased page without bit flips. So you mean that when you read a page, and you see there are bit-flips, you start correcting them. But if you notice that the page is actually an erased page, you optimize this case by not running the correction machinery, but just filling the buffer with 0xFFs. Right? -- Best Regards, Artem Bityutskiy signature.asc Description: This is a digitally signed message part ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v2 0/3] mtd: nand: OMAP: ELM error correction support for BCH ecc
On Wed, 2012-11-21 at 07:01 +, Philip, Avinash wrote: I am not sure how this dependency has to be handled for this series, let me know whether you still want it to be made over l2-mtd? Artem, Is it possible for you to give ack for these patches so that these patches can go in Tony's tree where Omap-gpmc changes are present? I would prefer if people like Ivan could take a look at this first. Also, I am not sure this is a good idea. Or at least we should agree on some common strategy for bit-flips in the erased areas: + * 1. If page is erased, check with standard ecc vector (ecc vector + * for erased page to find any bit flip). If check fails, bit flip + * is present in erased page. Count the bit flips in erased page and + * if it falls under correctable level, report page with 0xFF and + * update the correctable bit information. Basically, you are working-around JFFS2 limitations. Do you suggest we do this in all the drivers? If we want to do this, may be we better do this in higher level, common to all drivers? -- Best Regards, Artem Bityutskiy signature.asc Description: This is a digitally signed message part ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 1/1] mtd: physmap_of: allow to specify the mtd name for retro compatiblity
On Wed, 2012-11-07 at 16:32 +0100, Jean-Christophe PLAGNIOL-VILLARD wrote: linux,mtd-name allow to specify the mtd name for retro capability with physmap-flash drivers as boot loader pass the mtd partition via the old device name physmap-flash. Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD plagn...@jcrosoft.com Pushed to l2-mtd.git, thanks! -- Best Regards, Artem Bityutskiy signature.asc Description: This is a digitally signed message part ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v2 0/3] mtd: nand: OMAP: ELM error correction support for BCH ecc
On Wed, 2012-10-31 at 12:38 +0530, Philip, Avinash wrote: Support to use ELM as BCH 4 8 bit error correction module. Also performance enhancement by adding single shot read_page and write_page functions for the nand flashes with page size less than 4 KB. ELM module can be used to correct errors reported by BCH 4, 8 16 bit ECC scheme. For now only 4 8 bit support is added. BCH 4 8 bit error detection support is already available in mainline kernel and works with software error correction. This series is based on [1] and tested with RFC: OMAP GPMC bindings patch series 1. linux-next/20121030 Would you please re-send a version which cleanly applies to the l2-mtd.git tree? This series has many conflicts. Thanks! git://git.infradead.org/users/dedekind/l2-mtd.git -- Best Regards, Artem Bityutskiy signature.asc Description: This is a digitally signed message part ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RESEND PATCH] mtd: davinci: add support for parition binding nodes
On Fri, 2012-11-02 at 10:22 -0400, Murali Karicheri wrote: Enhance the driver to support partition subnodes inside the nand device bindings to describe partions on the nand device. Signed-off-by: Murali Karicheri m-kariche...@ti.com Pushed to l2-mtd.git, thanks! -- Best Regards, Artem Bityutskiy signature.asc Description: This is a digitally signed message part ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH resend] Add devicetree support to m25p80 driver
On Tue, 2012-11-13 at 10:56 +0100, Michal Vanka wrote: return -ENODEV; #endif @@ -825,27 +848,39 @@ static int __devinit m25p_probe(struct spi_device *spi) dev_warn(spi-dev, unrecognized id %s\n, data-type); Still line-wrapped: $ git apply --check ~/tmp/mvanka.mbox fatal: corrupt patch at line 122 Please, make sure it applies to the l2-mtd.git tree. Send it to yourself, save your own e-mail, and verify with 'git apply --check'. -- Best Regards, Artem Bityutskiy signature.asc Description: This is a digitally signed message part ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] Devicetree support for enc28j60 SPI Ethernet chip
On Sun, 2012-10-14 at 20:30 +0200, Michal Vanka wrote: Hi, the following patch adds generic openfirmware/devicetree support to enc28j60 driver. It also adds support for custom hardware based (FPGA) interrupt line deasserter for processors with level triggered interrupts (for example Altera Nios2). Hi, please, re-send with devicetree-discuss@lists.ozlabs.org in CC. -- Best Regards, Artem Bityutskiy signature.asc Description: This is a digitally signed message part ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 1/1] mtd: st fsmc_nand: pass the ale and cmd resource via resource
On Thu, 2012-10-04 at 15:14 +0200, Jean-Christophe PLAGNIOL-VILLARD wrote: Do not use the platform_data to pass resource and be smart in the drivers. Just pass it via resource Switch to devm_request_and_ioremap at the sametime Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD plagn...@jcrosoft.com Pushed to l2-mtd.git, thanks! -- Best Regards, Artem Bityutskiy signature.asc Description: This is a digitally signed message part ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V5] mtd: m25p80: Make fast read configurable via DT
On Mon, 2012-09-17 at 16:10 +0200, Marek Vasut wrote: Bump, do you think we can get this into 3.7 please? Aiaiai! Successfully built configuration i386_defconfig,i386,, results: --- before_patching.log +++ after_patching.log @@ @@ +drivers/mtd/devices/m25p80.c: In function ‘m25p_probe’: +drivers/mtd/devices/m25p80.c:808:23: warning: unused variable ‘np’ [-Wunused-variable] -- Best Regards, Artem Bityutskiy signature.asc Description: This is a digitally signed message part ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V4] mtd: m25p80: Make fast read configurable via DT
On Thu, 2012-08-23 at 15:41 +0200, Marek Vasut wrote: Add DT property m25p,fast-read that signalises the particular chip supports fast read opcode. Signed-off-by: Marek Vasut ma...@denx.de Cc: Artem Bityutskiy artem.bityuts...@linux.intel.com Cc: David Woodhouse david.woodho...@intel.com Does not compile if (I think) OF is not enabled. drivers/mtd/devices/m25p80.c: In function 'm25p_probe': drivers/mtd/devices/m25p80.c:920:2: error: implicit declaration of function 'of_property_read_bool' [-Werror=implicit-function-declaration] cc1: some warnings being treated as errors make[4]: *** [drivers/mtd/devices/m25p80.o] Error 1 -- Best Regards, Artem Bityutskiy signature.asc Description: This is a digitally signed message part ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v2] mtd: physmap_of: Add no-unaligned-direct-access DT property
On Fri, 2012-08-17 at 15:22 +0200, Stefan Roese wrote: On some platforms (e.g. MPC5200) a direct 1:1 mapping may cause problems with JFFS2 usage, as the local bus (LPB) doesn't support unaligned accesses as implemented in the JFFS2 code via memcpy(). By defining no-unaligned-direct-access, the flash will not be exposed directly to the MTD users (e.g. JFFS2) any more. Signed-off-by: Stefan Roese s...@denx.de Pushed to l2-mtd.git, thanks! -- Best Regards, Artem Bityutskiy signature.asc Description: This is a digitally signed message part ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v6] ARM: mtd: nand: davinci: add OF support for davinci nand controller
On Mon, 2012-07-30 at 09:22 +0200, Heiko Schocher wrote: add OF support for the davinci nand controller. Signed-off-by: Heiko Schocher h...@denx.de Pushed to l2-mtd.git, thanks! -- Best Regards, Artem Bityutskiy signature.asc Description: This is a digitally signed message part ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v11] MTD: LPC32xx MLC NAND driver
On Sat, 2012-06-30 at 18:50 +0200, Roland Stigge wrote: This patch adds a driver for the MLC NAND controller of the LPC32xx SoC. Signed-off-by: Roland Stigge sti...@antcom.de Pushed to l2-mtd.git, thanks! -- Best Regards, Artem Bityutskiy signature.asc Description: This is a digitally signed message part ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v10] MTD: LPC32xx MLC NAND driver
On Sat, 2012-06-30 at 14:32 +0200, Roland Stigge wrote: Hi Artem! Thanks for the notes! On 29/06/12 14:37, Artem Bityutskiy wrote: Successfully built configuration arm-lpc32xx_defconfig,arm,arm-unknown-linux-gnueabi-, results: --- before_patching.log +++ after_patching.log @@ @@ +drivers/mtd/nand/lpc32xx_mlc.c: In function 'lpc32xx_nand_probe': +drivers/mtd/nand/lpc32xx_mlc.c:679:10: warning: variable 'sr' set but not used [-Wunused-but-set-variable] +drivers/mtd/nand/lpc32xx_mlc.c:561:24: error: bad constant expression [sparse] This is because sparse doesn't understand this local variable: uint8_t buf[mtd-writesize]; mtd-writesize is typically 2048KiB (or even larger, there are MLCs with 4 and 8 KiB NAND page size). You cannot allocate that much on the stack, linux kernel stack size is very small - only 8KiB, and there are people who use even 4KiB stacks. So this have to be fixed. The above construction is quite convenient for this case and I'd prefer this to allocating another buffer dynamically. Yes, in userspaces it is, but unfortunately in the kernel we cannot afford allocating that much on the stack. -- Best Regards, Artem Bityutskiy signature.asc Description: This is a digitally signed message part ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 2/4] mtd: lpc32xx_slc: Make wp gpio optional
On Wed, 2012-06-27 at 17:51 +0200, Roland Stigge wrote: From: Alexandre Pereira da Silva aletes@gmail.com This patch supports missing wp gpio. Signed-off-by: Alexandre Pereira da Silva aletes@gmail.com Signed-off-by: Roland Stigge sti...@antcom.de Pushed patches 2-4, thanks! -- Best Regards, Artem Bityutskiy signature.asc Description: This is a digitally signed message part ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v10] MTD: LPC32xx MLC NAND driver
On Wed, 2012-06-27 at 23:55 +0200, Roland Stigge wrote: This patch adds a driver for the MLC NAND controller of the LPC32xx SoC. Signed-off-by: Roland Stigge sti...@antcom.de Signed-off-by: Alexandre Pereira da Silva aletes@gmail.com --- Aiaiai complains like this about your patch, please, take a look: Successfully built configuration arm-lpc32xx_defconfig,arm,arm-unknown-linux-gnueabi-, results: --- before_patching.log +++ after_patching.log @@ @@ +drivers/mtd/nand/lpc32xx_mlc.c: In function 'lpc32xx_nand_probe': +drivers/mtd/nand/lpc32xx_mlc.c:679:10: warning: variable 'sr' set but not used [-Wunused-but-set-variable] +drivers/mtd/nand/lpc32xx_mlc.c:561:24: error: bad constant expression [sparse] checkpatch.pl has some complaints: checkpatch.pl results for patch [PATCH] MTD: LPC32xx MLC NAND driver ERROR:INITIALISED_STATIC: do not initialise statics to 0 or NULL #339: FILE: drivers/mtd/nand/lpc32xx_mlc.c:224: +static int use_dma = 0; total: 1 errors, 0 warnings, 1004 lines checked -- Best Regards, Artem Bityutskiy signature.asc Description: This is a digitally signed message part ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v10] MTD: LPC32xx MLC NAND driver
On Fri, 2012-06-29 at 09:56 -0300, Alexandre Pereira da Silva wrote: @@ @@ +drivers/mtd/nand/lpc32xx_mlc.c: In function 'lpc32xx_nand_probe': +drivers/mtd/nand/lpc32xx_mlc.c:679:10: warning: variable 'sr' set but not used [-Wunused-but-set-variable] gcc 4.6, aiaiai uses W=1 when building. +drivers/mtd/nand/lpc32xx_mlc.c:561:24: error: bad constant expression [sparse] This is sparse, not gcc. -- Best Regards, Artem Bityutskiy signature.asc Description: This is a digitally signed message part ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v7] MTD: LPC32xx SLC NAND driver
On Thu, 2012-06-07 at 12:22 +0200, Roland Stigge wrote:
This patch adds support for the SLC NAND controller inside the LPC32xx SoC.
Signed-off-by: Roland Stigge sti...@antcom.de
Now the write_page and write_page_raw functions return an error code,
see this commit in the l2 tree:
http://git.infradead.org/users/dedekind/l2-mtd.git/commit/49c8d9ab3b70732665249f2d993f734378ebbba9
I've amended your SLC driver, see below the diff. But I think it could
return the real return code becuse it can fail - could you please take a
look and send an incremental patch?
Please, base your work on top of the l2-mtd tree:
git://git.infradead.org/users/dedekind/l2-mtd.git
Thanks!
diff --git a/drivers/mtd/nand/lpc32xx_slc.c b/drivers/mtd/nand/lpc32xx_slc.c
index 796e37b..7e2e78d 100644
--- a/drivers/mtd/nand/lpc32xx_slc.c
+++ b/drivers/mtd/nand/lpc32xx_slc.c
@@ -673,9 +673,9 @@ static int lpc32xx_nand_read_page_raw_syndrome(struct
mtd_info *mtd,
* Write the data and OOB data to the device, use ECC with the data,
* disable ECC for the OOB data
*/
-static void lpc32xx_nand_write_page_syndrome(struct mtd_info *mtd,
-struct nand_chip *chip,
-const u8 *buf, int oob_required)
+static int lpc32xx_nand_write_page_syndrome(struct mtd_info *mtd,
+ struct nand_chip *chip,
+ const u8 *buf, int oob_required)
{
struct lpc32xx_nand_host *host = chip-priv;
u8 *pb = chip-oob_poi + chip-ecc.layout-eccpos[0];
@@ -691,20 +691,22 @@ static void lpc32xx_nand_write_page_syndrome(struct
mtd_info *mtd,
/* Write ECC data to device */
chip-write_buf(mtd, chip-oob_poi, mtd-oobsize);
+ return 0;
}
/*
* Write the data and OOB data to the device, no ECC correction with the
* data or OOB data
*/
-static void lpc32xx_nand_write_page_raw_syndrome(struct mtd_info *mtd,
-struct nand_chip *chip,
-const u8 *buf,
-int oob_required)
+static int lpc32xx_nand_write_page_raw_syndrome(struct mtd_info *mtd,
+ struct nand_chip *chip,
+ const u8 *buf,
+ int oob_required)
{
/* Raw writes can just use the FIFO interface */
chip-write_buf(mtd, buf, chip-ecc.size * chip-ecc.steps);
chip-write_buf(mtd, chip-oob_poi, mtd-oobsize);
+ return 0;
}
static bool lpc32xx_dma_filter(struct dma_chan *chan, void *param)
--
Best Regards,
Artem Bityutskiy
signature.asc
Description: This is a digitally signed message part
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v7] MTD: LPC32xx SLC NAND driver
On Wed, 2012-06-27 at 14:14 +0200, Roland Stigge wrote: Thanks for the note! I'm sending an incremental patch. There was actually only one place in the two functions that could fail (return code of lpc32xx_xfer()). Could you please check the MCL patch as well and re-send it against the l2 tree? I've squashed your change into the driver and pushed out, thanks! -- Best Regards, Artem Bityutskiy signature.asc Description: This is a digitally signed message part ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v6] MTD: LPC32xx SLC NAND driver
On Thu, 2012-06-07 at 09:11 +0100, Russell King - ARM Linux wrote: On Wed, Jun 06, 2012 at 04:38:41PM +0300, Artem Bityutskiy wrote: On Wed, 2012-06-06 at 11:20 +0200, Roland Stigge wrote: +#else +#define lpc32xx_nand_resume NULL +#define lpc32xx_nand_suspend NULL +#endif 0, not NULL. Err, what planet are you on there. These are _pointers_ to functions, so using a _pointer_ is more correct than using an _integer_. Yes, thanks, I was wrong. -- Best Regards, Artem Bityutskiy signature.asc Description: This is a digitally signed message part ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v7] MTD: LPC32xx SLC NAND driver
On Thu, 2012-06-07 at 12:22 +0200, Roland Stigge wrote: This patch adds support for the SLC NAND controller inside the LPC32xx SoC. Signed-off-by: Roland Stigge sti...@antcom.de Pushed to l2-mtd.git, thanks! -- Best Regards, Artem Bityutskiy signature.asc Description: This is a digitally signed message part ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v5] MTD: LPC32xx SLC NAND driver
On Mon, 2012-06-04 at 21:25 +0200, Roland Stigge wrote: +config MTD_NAND_SLC_LPC32XX + bool NXP LPC32xx SLC Controller Why bool and not tristate here? Why you force this to be compiled-in and make it impossible to be a kernel module like other drivers are? -- Best Regards, Artem Bityutskiy signature.asc Description: This is a digitally signed message part ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v6] MTD: LPC32xx SLC NAND driver
On Wed, 2012-06-06 at 11:20 +0200, Roland Stigge wrote:
+#ifdef CONFIG_PM
+static int lpc32xx_nand_resume(struct platform_device *pdev)
+{
+ struct lpc32xx_nand_host *host = platform_get_drvdata(pdev);
+
+ /* Re-enable NAND clock */
+ clk_enable(host-clk);
+
+ /* Fresh init of NAND controller */
+ lpc32xx_nand_setup(host);
+
+ /* Disable write protect */
+ lpc32xx_wp_disable(host);
+
+ return 0;
+}
+
+static int lpc32xx_nand_suspend(struct platform_device *pdev, pm_message_t
pm)
+{
+ u32 tmp;
+ struct lpc32xx_nand_host *host = platform_get_drvdata(pdev);
+
+ /* Force CE high */
+ tmp = readl(SLC_CTRL(host-io_base));
+ tmp = ~SLCCFG_CE_LOW;
+ writel(tmp, SLC_CTRL(host-io_base));
+
+ /* Enable write protect for safety */
+ lpc32xx_wp_enable(host);
+
+ /* Disable clock */
+ clk_disable(host-clk);
+
+ return 0;
+}
+
+#else
+#define lpc32xx_nand_resume NULL
+#define lpc32xx_nand_suspend NULL
+#endif
0, not NULL.
--
Best Regards,
Artem Bityutskiy
signature.asc
Description: This is a digitally signed message part
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v6] MTD: LPC32xx SLC NAND driver
On Wed, 2012-06-06 at 11:20 +0200, Roland Stigge wrote: drivers/mtd/nand/lpc32xx_nand_slc.c | 1060 ++ Can you please rename it to lpc32xx_slc.c? It is already in the nand sub-directory, no need to add nand to the file name. -- Best Regards, Artem Bityutskiy signature.asc Description: This is a digitally signed message part ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v6] MTD: LPC32xx SLC NAND driver
On Wed, 2012-06-06 at 16:27 +0200, Roland Stigge wrote: +#else +#define lpc32xx_nand_resume NULL +#define lpc32xx_nand_suspend NULL +#endif 0, not NULL. Can you please point me to an explanation for this? NULL sounds Right, sorry, that was incorrect. -- Best Regards, Artem Bityutskiy signature.asc Description: This is a digitally signed message part ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] MTD: LPC32xx SLC NAND driver
I am CCing few other guys who take care of several drivers which use
similar way of busy-waiting - probably you could change it?
Bastian: drivers/mtd/nand/sh_flctl.c
Lars-Peter: drivers/mtd/nand/jz4740_nand.c
Huang: drivers/mtd/nand/gpmi-nand/gpmi-lib.c
Lei Wen: drivers/mtd/nand/pxa3xx_nand.c
On Sat, 2012-05-12 at 15:29 +0200, Roland Stigge wrote:
+ /*
+* The DMA is finished, but the NAND controller may still have
+* buffered data. Wait until all the data is sent.
+*/
+ timeout = LPC32XX_DMA_SIMPLE_TIMEOUT;
+ while ((readl(SLC_STAT(host-io_base)) SLCSTAT_DMA_FIFO)
+ (timeout 0))
+ timeout--;
+ if (!timeout) {
+ dev_err(mtd-dev.parent, FIFO held data too long\n);
+ status = -EIO;
+ }
I know the MTD tree is full of this, but this is bad, I think. The
timeout should be time-backed, not CPU-cycles-backed.
I do not know the best way to do this, hopefully someone in the arm list
could suggest, but the following pattern is at least better:
/* Chip reaction time timeout in milliseconds */
#define LPC32XX_DMA_TIMEOUT 100
timeout = loops_per_jiffy * msecs_to_jiffies(LPC32XX_DMA_TIMEOUT);
while ((readl(...)) timeout-- 0)
cpu_relax();
if (!timeout)
error;
So basically I turned your hard-coded iterations count into a time-based
timeout. I also used cpu_relax() which is commonly used in tight-loops
like this. Here is a piece of documentation about cpu_relax():
The right way to perform a busy wait is:
while (my_variable != what_i_want)
cpu_relax();
The cpu_relax() call can lower CPU power consumption or yield to a
hyperthreaded twin processor; it also happens to serve as a compiler
barrier, so, once again, volatile is unnecessary. Of course, busy-
waiting is generally an anti-social act to begin with.
--
Best Regards,
Artem Bityutskiy
signature.asc
Description: This is a digitally signed message part
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] MTD: LPC32xx SLC NAND driver
On Tue, 2012-05-15 at 15:20 +0200, Roland Stigge wrote: while ((readl(...)) timeout-- 0) cpu_relax(); As I understand loops_per_jiffy, this loop will take much longer than the 100 ms you defined above? Not sure about much, but longer. The idea is that this is about the error path so if we report -EIO with a slight delay - no problem. -- Best Regards, Artem Bityutskiy signature.asc Description: This is a digitally signed message part ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 00/10] allow mxc_nand to be probed via device tree
On Sun, 2012-04-29 at 11:27 +0200, Uwe Kleine-König wrote:
Hello Artem,
On Sat, Apr 28, 2012 at 03:20:54PM +0300, Artem Bityutskiy wrote:
On Mon, 2012-04-23 at 11:22 +0200, Uwe Kleine-König wrote:
Hello,
this series aims to be able to probe mxc_nand devices via device tree.
Most of its patches reorganize the driver to only use cpu_is_mxYZ for
the case that the device is not probed via device tree.
I split this conversion in several patches to allow easier review. IMHO
it makes sense to keep this splitting for an eventual bisection.
This is tested on an i.MX27 based machine and works fine including
passing of partition data.
Applied all 11 patches from your git tree to l2-mtd.git. Note, you have
one checkpatch.pl warning:
WARNING:STATIC_CONST_CHAR_ARRAY: static const char * array should probably
be static const char * const
#69: FILE: drivers/mtd/nand/mxc_nand.c:274:
+static const char *part_probes[] = { RedBoot, cmdlinepart, ofpart,
NULL };
I noticed that, too. This is not new though and as
mtd_device_parse_register takes a const char **types applying
checkpatch's advice would yield another warning. But probably fixing
both would work. I wouldn't address this as part of this series.
Also there are still several sparse warnings, e.g.
+drivers/mtd/nand/mxc_nand.c:1249:26: warning: incorrect type in
initializer (different modifiers) [sparse]
+drivers/mtd/nand/mxc_nand.c:1249:26:expected void *data [sparse]
+drivers/mtd/nand/mxc_nand.c:1249:26:got struct mxc_nand_devtype_data
static const [toplevel] *noident [sparse]
And you added few gcc warnings, e.g.:
+drivers/mtd/nand/mxc_nand.c:1252:3: warning: initialization discards
'const' qualifier from pointer target type [enabled by default]
This is the same problem, i.e. casting away several consts. Hmm, and for
me neither gcc nor sparse provide that warning but I wonder why. At some
point I saw this warning, so I sent a patch:
Try to compile with gcc 4.6 - you can download it from here:
ftp://ftp.kernel.org/pub/tools/crosstool/files/bin/
I do not know whether it produces good binaries or not, but for
compile-testing it is much better than ancient gcc 4.3.
--
Best Regards,
Artem Bityutskiy
signature.asc
Description: This is a digitally signed message part
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 00/10] allow mxc_nand to be probed via device tree
On Mon, 2012-04-23 at 11:22 +0200, Uwe Kleine-König wrote:
Hello,
this series aims to be able to probe mxc_nand devices via device tree.
Most of its patches reorganize the driver to only use cpu_is_mxYZ for
the case that the device is not probed via device tree.
I split this conversion in several patches to allow easier review. IMHO
it makes sense to keep this splitting for an eventual bisection.
This is tested on an i.MX27 based machine and works fine including
passing of partition data.
Applied all 11 patches from your git tree to l2-mtd.git. Note, you have
one checkpatch.pl warning:
WARNING:STATIC_CONST_CHAR_ARRAY: static const char * array should probably be
static const char * const
#69: FILE: drivers/mtd/nand/mxc_nand.c:274:
+static const char *part_probes[] = { RedBoot, cmdlinepart, ofpart, NULL
};
Also there are still several sparse warnings, e.g.
+drivers/mtd/nand/mxc_nand.c:1249:26: warning: incorrect type in initializer
(different modifiers) [sparse]
+drivers/mtd/nand/mxc_nand.c:1249:26:expected void *data [sparse]
+drivers/mtd/nand/mxc_nand.c:1249:26:got struct mxc_nand_devtype_data
static const [toplevel] *noident [sparse]
And you added few gcc warnings, e.g.:
+drivers/mtd/nand/mxc_nand.c:1252:3: warning: initialization discards 'const'
qualifier from pointer target type [enabled by default]
Would you take a look and _possibly_ (if makes sense, I did not verify)
send incremental fixes?
--
Best Regards,
Artem Bityutskiy
signature.asc
Description: This is a digitally signed message part
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] MTD: Add device-tree support to fsmc_nand
On Fri, 2012-03-16 at 10:19 +0100, Stefan Roese wrote: This patch adds support to configure the FSMC NAND driver (used amongst others on SPEAr platforms) via device-tree instead of platform_data. Signed-off-by: Stefan Roese s...@denx.de Pushed to l2-mtd.git, thanks! -- Best Regards, Artem Bityutskiy signature.asc Description: This is a digitally signed message part ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCHv6] mtd: gpio-nand: add device tree bindings
On Wed, 2011-10-12 at 00:10 +0100, Jamie Iles wrote:
+#ifdef CONFIG_OF
+static const struct of_device_id gpio_nand_id_table[] = {
+ { .compatible = gpio-control-nand },
+ {}
+};
+MODULE_DEVICE_TABLE(of, gpio_nand_id_table);
...
+#else /* CONFIG_OF */
...
+#endif /* CONFIG_OF */
I wonder, why it is either OF of platform data? What if I want my kernel
to fall-back to platform data if device tree data is absent? What is the
general policy? Sorry, I am not very well aware of the DT stuff. But off
the top of my head, it is logical when things go like this: I have a
kernel with working platform data, but I can change that dynamically by
feeding it a device tree configuration. Hmm?
--
Best Regards,
Artem Bityutskiy
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCHv6] mtd: gpio-nand: add device tree bindings
On Fri, 2011-10-14 at 15:21 +0100, Jamie Iles wrote: the top of my head, it is logical when things go like this: I have a kernel with working platform data, but I can change that dynamically by feeding it a device tree configuration. Hmm? I think in general platform data and device tree should be mutually exclusive. OK, but would you please confirm this by pointing to some docs or discussions, or may be people from devicetree-discuss@lists.ozlabs.org could confirm that? -- Best Regards, Artem Bityutskiy ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCHv4] mtd: gpio-nand: add device tree bindings
On Sat, 2011-08-20 at 07:38 +0100, Jamie Iles wrote: Hi Artem, On Sat, Aug 20, 2011 at 06:39:10AM +0300, Artem Bityutskiy wrote: On Mon, 2011-08-15 at 16:24 +0100, Jamie Iles wrote: From: Jamie Iles ja...@jamieiles.com Subject: [PATCH] mtd: gpio-nand: add device tree bindings Add device tree bindings so that the gpio-nand driver may be instantiated from the device tree. This also allows the partitions to be specified in the device tree. v5: - fold dt config helpers into a single gpio_nand_of_get_config() v4: - get io sync address from gpio-control-nand,io-sync-reg property rather than a resource - clarified a few details in the binding v3: - remove redundant cast and a couple of whitespace/naming changes v2: - add CONFIG_OF guards for non-dt platforms - compatible becomes gpio-control-nand - clarify some binding details Cc: David Woodhouse dw...@infradead.org Cc: Artem Bityutskiy dedeki...@gmail.com Cc: Scott Wood scottw...@freescale.com Cc: Grant Likely grant.lik...@secretlab.ca Signed-off-by: Jamie Iles ja...@jamieiles.com Pushed this patch to l2-mtd-2.6.git, thanks. P.S. probably it should contain some reviewed-by tags? I can always add them, though, if needed. This patch needs of_read_property_u64() from http://www.mail-archive.com/devicetree-discuss@lists.ozlabs.org/msg05973.html which hasn't been merged yet for the CONFIG_OF case so I'm not sure how this should be handled. Oh, I did not realize that. Well, let's wait when it is include. I'll drop your patch for now then. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCHv4] mtd: gpio-nand: add device tree bindings
On Mon, 2011-08-15 at 16:24 +0100, Jamie Iles wrote: @@ -178,7 +249,7 @@ static int __devexit gpio_nand_remove(struct platform_device *dev) nand_release(gpiomtd-mtd_info); - res = platform_get_resource(dev, IORESOURCE_MEM, 1); + res = gpio_nand_get_io_sync(dev); Why do you call 'gpio_nand_get_io_sync(dev)' here, in 'gpio_nand_remove()' function? You should have it in gpiomtd-io_sync. Right? If this is the case, then you do not need a separate 'gpio_nand_get_io_sync()' function at all, you can make 'gpio_nand_get_config()' to fetch the io_sync information from the DT. And then you will have one single function which gets data from DT, not 2 - simpler code. Do I miss something? -- Best Regards, Artem Bityutskiy (Битюцкий Артём) ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCHv4] mtd: gpio-nand: add device tree bindings
On Mon, 2011-08-15 at 16:24 +0100, Jamie Iles wrote: From: Jamie Iles ja...@jamieiles.com Subject: [PATCH] mtd: gpio-nand: add device tree bindings Add device tree bindings so that the gpio-nand driver may be instantiated from the device tree. This also allows the partitions to be specified in the device tree. v5: - fold dt config helpers into a single gpio_nand_of_get_config() v4: - get io sync address from gpio-control-nand,io-sync-reg property rather than a resource - clarified a few details in the binding v3: - remove redundant cast and a couple of whitespace/naming changes v2: - add CONFIG_OF guards for non-dt platforms - compatible becomes gpio-control-nand - clarify some binding details Cc: David Woodhouse dw...@infradead.org Cc: Artem Bityutskiy dedeki...@gmail.com Cc: Scott Wood scottw...@freescale.com Cc: Grant Likely grant.lik...@secretlab.ca Signed-off-by: Jamie Iles ja...@jamieiles.com Pushed this patch to l2-mtd-2.6.git, thanks. P.S. probably it should contain some reviewed-by tags? I can always add them, though, if needed. -- Best Regards, Artem Bityutskiy (Битюцкий Артём) ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCHv4] mtd: gpio-nand: add device tree bindings
On Tue, 2011-08-09 at 16:12 +0100, Jamie Iles wrote:
+static int gpio_nand_of_get_options(struct device *dev,
+ struct gpio_nand_platdata *plat)
+{
+ u32 width;
+
+ if (!of_property_read_u32(dev-of_node, bank-width, width)) {
+ if (width == 2) {
+ plat-options |= NAND_BUSWIDTH_16;
+ } else if (width != 1) {
+ dev_err(dev, invalid bank-width %u\n, width);
+ return -EINVAL;
+ }
+ }
+
+ return 0;
+}
+
+static void gpio_nand_of_get_gpio(struct device *dev,
+ struct gpio_nand_platdata *plat)
+{
+ plat-gpio_rdy = of_get_gpio(dev-of_node, 0);
+ plat-gpio_nce = of_get_gpio(dev-of_node, 1);
+ plat-gpio_ale = of_get_gpio(dev-of_node, 2);
+ plat-gpio_cle = of_get_gpio(dev-of_node, 3);
+ plat-gpio_nwp = of_get_gpio(dev-of_node, 4);
+}
+
+static void gpio_nand_of_get_chip_delay(struct device *dev,
+ struct gpio_nand_platdata *plat)
+{
+ u32 chip_delay;
+
+ if (!of_property_read_u32(dev-of_node, chip-delay, chip_delay))
+ plat-chip_delay = (int)chip_delay;
+}
+
+static int gpio_nand_of_get_config(struct device *dev,
+struct gpio_nand_platdata *plat)
+{
+ int ret = gpio_nand_of_get_options(dev, plat);
+
+ if (ret 0)
+ return ret;
+
+ gpio_nand_of_get_gpio(dev, plat);
+ gpio_nand_of_get_chip_delay(dev, plat);
+
+ return 0;
+}
Do we really need to have 3 additional helper functions for
'gpio_nand_of_get_config()' ?
--
Best Regards,
Artem Bityutskiy
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCHv4] mtd: gpio-nand: add device tree bindings
On Tue, 2011-08-09 at 16:12 +0100, Jamie Iles wrote:
- res1 = platform_get_resource(dev, IORESOURCE_MEM, 1);
+ if (!dev-dev.of_node)
+ res1 = platform_get_resource(dev, IORESOURCE_MEM, 1);
+ else
+ res1 = gpio_nand_of_get_io_sync(dev-dev);
+
if (res1) {
gpiomtd-io_sync = request_and_remap(res1, 4, NAND sync,
ret);
if (!gpiomtd-io_sync) {
@@ -257,7 +362,16 @@ static int __devinit gpio_nand_probe(struct
platform_device *dev)
}
}
- memcpy(gpiomtd-plat, dev-dev.platform_data, sizeof(gpiomtd-plat));
+ if (dev-dev.of_node)
+ kfree(res1);
+
+ if (dev-dev.platform_data)
+ memcpy(gpiomtd-plat, dev-dev.platform_data,
+sizeof(gpiomtd-plat));
+ else
+ ret = gpio_nand_of_get_config(dev-dev, gpiomtd-plat);
+ if (ret)
+ goto err_nce;
So with this code you can mix platform data and DT? Say, io_sync may
come from platform data and the rest from the DT? Is this normal
practice?
--
Best Regards,
Artem Bityutskiy
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCHv4] mtd: gpio-nand: add device tree bindings
On Mon, 2011-08-15 at 15:38 +0100, Jamie Iles wrote:
On Mon, Aug 15, 2011 at 05:28:57PM +0300, Artem Bityutskiy wrote:
On Tue, 2011-08-09 at 16:12 +0100, Jamie Iles wrote:
- res1 = platform_get_resource(dev, IORESOURCE_MEM, 1);
+ if (!dev-dev.of_node)
+ res1 = platform_get_resource(dev, IORESOURCE_MEM, 1);
+ else
+ res1 = gpio_nand_of_get_io_sync(dev-dev);
+
if (res1) {
gpiomtd-io_sync = request_and_remap(res1, 4, NAND sync,
ret);
if (!gpiomtd-io_sync) {
@@ -257,7 +362,16 @@ static int __devinit gpio_nand_probe(struct
platform_device *dev)
}
}
- memcpy(gpiomtd-plat, dev-dev.platform_data, sizeof(gpiomtd-plat));
+ if (dev-dev.of_node)
+ kfree(res1);
+
+ if (dev-dev.platform_data)
+ memcpy(gpiomtd-plat, dev-dev.platform_data,
+sizeof(gpiomtd-plat));
+ else
+ ret = gpio_nand_of_get_config(dev-dev, gpiomtd-plat);
+ if (ret)
+ goto err_nce;
So with this code you can mix platform data and DT? Say, io_sync may
come from platform data and the rest from the DT? Is this normal
practice?
Well you can use platform_data with DT - it's the only way to pass
function pointers for example, but I'm not convinced it's required in
this case (there is the adjust_parts callback, but I can't see a user of
it) so I'd be inclined to change the conditionals to:
if (!dev-dev.of_node)
memcpy(gpiomtd-plat, dev-dev.platform_data, ...);
else
gpio_nand_of_get_config(...);
so that we don't use platform_data for the DT case.
Yeah, would be nice to have only one read platform data and only one
read DT information call - this is just cleaner design.
--
Best Regards,
Artem Bityutskiy
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v2] mtd: dataflash: add device tree probe support
On Fri, 2011-07-15 at 12:49 +0800, Shawn Guo wrote:
+static const struct of_device_id dataflash_dt_ids[] = {
+ { .compatible = atmel,at45xxx, },
+ { .compatible = atmel,dataflash, },
+ { /* sentinel */ }
+};
+
This should be protected with a #ifdef CONFIG_OF/#else/#endif, and
there should be a MODULE_DEVICE_TABLE().
I personally hate #ifdef stuff. But okay, I can do it since there
are people being concerned by this little waste of space.
I guess the question is - will it compile and work if CONFIG_OF is
unset?
--
Best Regards,
Artem Bityutskiy
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v3] mtd: dataflash: add device tree probe support
On Fri, 2011-07-15 at 16:38 +0800, Shawn Guo wrote: It adds device tree probe support for mtd_dataflash driver. Signed-off-by: Shawn Guo shawn@linaro.org Cc: Grant Likely grant.lik...@secretlab.ca Cc: David Woodhouse dw...@infradead.org Cc: Artem Bityutskiy artem.bityuts...@nokia.com Pushed to l2-mtd-2.6.git, thanks! -- Best Regards, Artem Bityutskiy ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v2] mtd: dataflash: add device tree probe support
On Wed, 2011-07-20 at 12:55 +0800, Shawn Guo wrote:
On Wed, Jul 20, 2011 at 07:40:38AM +0300, Artem Bityutskiy wrote:
On Fri, 2011-07-15 at 12:49 +0800, Shawn Guo wrote:
+static const struct of_device_id dataflash_dt_ids[] = {
+ { .compatible = atmel,at45xxx, },
+ { .compatible = atmel,dataflash, },
+ { /* sentinel */ }
+};
+
This should be protected with a #ifdef CONFIG_OF/#else/#endif, and
there should be a MODULE_DEVICE_TABLE().
I personally hate #ifdef stuff. But okay, I can do it since there
are people being concerned by this little waste of space.
I guess the question is - will it compile and work if CONFIG_OF is
unset?
Yes, it will compile, as 'struct of_device_id' is defined in
include/linux/mod_devicetable.h unconditionally.
And it will work correctly even though dataflash_dt_ids is not NULL, it
will not confuse MTD layer?
--
Best Regards,
Artem Bityutskiy
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v2] mtd: dataflash: add device tree probe support
On Wed, 2011-07-20 at 13:28 +0800, Shawn Guo wrote:
On Wed, Jul 20, 2011 at 08:17:45AM +0300, Artem Bityutskiy wrote:
On Wed, 2011-07-20 at 12:55 +0800, Shawn Guo wrote:
On Wed, Jul 20, 2011 at 07:40:38AM +0300, Artem Bityutskiy wrote:
On Fri, 2011-07-15 at 12:49 +0800, Shawn Guo wrote:
+static const struct of_device_id dataflash_dt_ids[] = {
+ { .compatible = atmel,at45xxx, },
+ { .compatible = atmel,dataflash, },
+ { /* sentinel */ }
+};
+
This should be protected with a #ifdef CONFIG_OF/#else/#endif, and
there should be a MODULE_DEVICE_TABLE().
I personally hate #ifdef stuff. But okay, I can do it since there
are people being concerned by this little waste of space.
I guess the question is - will it compile and work if CONFIG_OF is
unset?
Yes, it will compile, as 'struct of_device_id' is defined in
include/linux/mod_devicetable.h unconditionally.
And it will work correctly even though dataflash_dt_ids is not NULL, it
will not confuse MTD layer?
I think for non-dt case, dataflash_dt_ids is not used anyway. So yes,
it will not confuse MTD layer, at least from my testing.
And it is not error-prone? I mean, it is not very likely that someone
changing MTD could make wrong assumptions and break dataflash driver? If
so, then we probably do not need ifdefs indeed.
--
Best Regards,
Artem Bityutskiy
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v2] mtd: dataflash: add device tree probe support
On Wed, 2011-07-20 at 13:52 +0800, Shawn Guo wrote: The argument about this #ifdef thing was the driver could be used by some platforms that will never migrate to device tree. This table will be just a waste of bytes for those platforms, if we do not protect the table by '#ifdef CONFIG_OF'. OK, anyway, I picked your v2 already and less bytes wastage is a good enough argument. -- Best Regards, Artem Bityutskiy ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 3/7] drivers/mtd/nand/mpc5121_nfc.c: Add of_node_put to avoid memory leak
On Sun, 2010-08-29 at 11:52 +0200, Julia Lawall wrote: Add a call to of_node_put in the error handling code following a call to of_find_compatible_node. The semantic match that finds this problem is as follows: (http://coccinelle.lip6.fr/) Pushed to l2-mtd-2.6.git / master, thanks. -- Best Regards, Artem Bityutskiy (Битюцкий Артём) ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
