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

Groups > linux.kernel > #1664581

Re: [PATCH v6 00/18] mtd: nand: denali: Denali NAND IP patch bomb

Show all headers | View raw

Hi Boris,


2017-06-13 16:02 GMT+09:00 Boris Brezillon <boris.brezillon@free-electrons.com>:
> Le Tue, 13 Jun 2017 14:03:52 +0900,
> Masahiro Yamada <yamada.masahiro@socionext.com> a écrit :
>
>> This patch series intends to solve various problems.
>>
>> [1] The driver just retrieves the OOB area as-is
>>     whereas the controller uses syndrome page layout.
>> [2] ONFi devices are not working
>> [3] It can not read Bad Block Marker
>>
>> Outstanding changes are:
>>  - Fix raw/oob callbacks for syndrome page layout
>>  - Implement setup_data_interface() callback
>>  - Fix/implement more commands for ONFi devices
>>  - Allow to skip the driver internal bounce buffer
>>  - Support PIO in case DMA is not supported
>>  - Switch from ->cmdfunc over to ->cmd_ctrl
>>
>> 18 patches were merged by v2.
>> 11 patches were merged by v3.
>> 2 patches were merged by v4.
>> 5 patches were merged by v5.
>> Here is the rest of the series.
>>
>> v1: https://lkml.org/lkml/2016/11/26/144
>> v2: https://lkml.org/lkml/2017/3/22/804
>> v3: https://lkml.org/lkml/2017/3/30/90
>> v4: https://lkml.org/lkml/2017/6/5/1005
>>
>> Masahiro Yamada (18):
>>   mtd: nand: denali: set NAND_ECC_CUSTOM_PAGE_ACCESS
>>   mtd: nand: denali: remove unneeded find_valid_banks()
>>   mtd: nand: denali: handle timing parameters by setup_data_interface()
>>   mtd: nand: denali: rework interrupt handling
>>   mtd: nand: denali: fix NAND_CMD_STATUS handling
>>   mtd: nand: denali: fix NAND_CMD_PARAM handling
>
> AFAICT, patch 5 and 6 are unneeded...
>
>>   mtd: nand: denali: switch over to cmd_ctrl instead of cmdfunc
>
> ... because you're anyway switching to ->cmd_ctrl() in patch 7, which
> fixes the problem you were addressing in patch 5 and 6.
>
> Please squash those 3 patches into a single one and adjust your commit
> message accordingly (explaining that it fixes STATUS and PARAM handling).
> See below if you need an example.

Squashing 3 patches is OK, but I did not get your additional implementation.


> BTW, I also implemented ->read/write_buf_word() since the core may one
> day call these functions, and the default implementations used by the
> core when these hooks are NULL are not appropriate in your case.

I implemented

denali_read_buf()
denali_write_buf()
denali_read_buf16()
denali_write_buf16()

in 13/18 in a bit different way:
http://patchwork.ozlabs.org/patch/774961/


If you like, I can modify 13/18 so that
denali_read/write_byte() is implemented based on denali_read/write_buf().





> --->8---
> From 136727ba7b453ca1567c711037230aa6ec0f124a Mon Sep 17 00:00:00 2001
> From: Masahiro Yamada <yamada.masahiro@socionext.com>
> Date: Tue, 13 Jun 2017 14:03:57 +0900
> Subject: [PATCH] mtd: nand: denali: switch over to cmd_ctrl instead of cmdfunc
>
> The NAND_CMD_SET_FEATURES support is missing from denali_cmdfunc().
>
> Besides, we see /* TODO: Read OOB data */ comment line.
>
> It would be possible to add more commands along with the current
> implementation, but having ->cmd_ctrl() seems a better approach from
> the discussion with Boris [1].
>
> Rely on the default ->cmdfunc() from the framework and implement the
> driver's own ->cmd_ctrl(). We are also implementing
> ->read/write_buf/byte/word().
>
> Also add ->write_byte(), which is needed for write direction commands.
>
> Then, we can drop nand_onfi_get_set_features_notsupp from this driver.
>
> This transition also fixes NAND_CMD_STATUS and NAND_CMD_PARAM handling.
> NAND_CMD_STATUS was just faked by the implementation, and the only valid
> bit returned in this case was the WP bit.
> NAND_CMD_PARAM was completely broken: not only the command sent on the
> bus was NAND_CMD_STATUS instead of NAND_CMD_PARAM, but the driver was
> only reading 8 bytes of data, while the parameter page is contains
> several hundreds of bytes.

Probably "... page contains" instead of "... page is contains".





-- 
Best Regards
Masahiro Yamada

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH v6 00/18] mtd: nand: denali: Denali NAND IP patch bomb Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-06-13 07:10 +0200
  [PATCH v6 02/18] mtd: nand: denali: remove unneeded find_valid_banks() Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-06-13 07:10 +0200
  [PATCH v6 08/18] mtd: nand: denali: fix bank reset function to detect the number of chips Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-06-13 07:10 +0200
  [PATCH v6 01/18] mtd: nand: denali: set NAND_ECC_CUSTOM_PAGE_ACCESS Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-06-13 07:10 +0200
  [PATCH v6 16/18] mtd: nand: denali: use non-managed kmalloc() for DMA buffer Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-06-13 07:10 +0200
  [PATCH v6 09/18] mtd: nand: denali: use interrupt instead of polling for bank reset Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-06-13 07:10 +0200
  [PATCH v6 07/18] mtd: nand: denali: switch over to cmd_ctrl instead of cmdfunc Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-06-13 07:10 +0200
  [PATCH v6 05/18] mtd: nand: denali: fix NAND_CMD_STATUS handling Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-06-13 07:10 +0200
  [PATCH v6 18/18] mtd: nand: denali: avoid magic numbers and rename for clarification Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-06-13 07:10 +0200
  [PATCH v6 15/18] mtd: nand: denali: skip driver internal bounce buffer when possible Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-06-13 07:10 +0200
  [PATCH v6 03/18] mtd: nand: denali: handle timing parameters by setup_data_interface() Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-06-13 07:10 +0200
  [PATCH v6 06/18] mtd: nand: denali: fix NAND_CMD_PARAM handling Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-06-13 07:10 +0200
  [PATCH v6 13/18] mtd: nand: denali: fix raw and oob accessors for syndrome page layout Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-06-13 07:10 +0200
  [PATCH v6 17/18] mtd: nand: denali: enable bad block table scan Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-06-13 07:10 +0200
  [PATCH v6 11/18] mtd: nand: denali: merge struct nand_buf into struct denali_nand_info Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-06-13 07:10 +0200
  [PATCH v6 14/18] mtd: nand: denali: support hardware-assisted erased page detection Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-06-13 07:10 +0200
  [PATCH v6 12/18] mtd: nand: denali: use flag instead of register macro for direction Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-06-13 07:20 +0200
  [PATCH v6 04/18] mtd: nand: denali: rework interrupt handling Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-06-13 07:20 +0200
  [PATCH v6 10/18] mtd: nand: denali: propagate page to helpers via function argument Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-06-13 07:20 +0200
  Re: [PATCH v6 00/18] mtd: nand: denali: Denali NAND IP patch bomb Boris Brezillon <boris.brezillon@free-electrons.com> - 2017-06-13 09:10 +0200
    Re: [PATCH v6 00/18] mtd: nand: denali: Denali NAND IP patch bomb Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-06-13 10:10 +0200
      Re: [PATCH v6 00/18] mtd: nand: denali: Denali NAND IP patch bomb Boris Brezillon <boris.brezillon@free-electrons.com> - 2017-06-13 10:30 +0200
    Re: [PATCH v6 00/18] mtd: nand: denali: Denali NAND IP patch bomb Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-06-13 10:20 +0200
      Re: [PATCH v6 00/18] mtd: nand: denali: Denali NAND IP patch bomb Boris Brezillon <boris.brezillon@free-electrons.com> - 2017-06-13 10:40 +0200
        Re: [PATCH v6 00/18] mtd: nand: denali: Denali NAND IP patch bomb Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-06-13 11:00 +0200

csiph-web