[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2] hw/block/nand: Decommission the NAND museum
|
From: |
Philippe Mathieu-Daudé |
|
Subject: |
Re: [PATCH v2] hw/block/nand: Decommission the NAND museum |
|
Date: |
Mon, 14 Dec 2020 01:23:04 +0100 |
|
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.5.0 |
On 12/14/20 1:11 AM, Philippe Mathieu-Daudé wrote:
> On 10/19/20 6:05 PM, Peter Maydell wrote:
>> On Tue, 15 Sep 2020 at 18:52, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>>>
>>> This is the QEMU equivalent of this Linux commit (but 7 years later):
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f7025a43a9da2
>>>
>>> The MTD subsystem has its own small museum of ancient NANDs
>>> in a form of the CONFIG_MTD_NAND_MUSEUM_IDS configuration option.
>>> The museum contains stone age NANDs with 256 bytes pages, as well
>>> as iron age NANDs with 512 bytes per page and up to 8MiB page size.
>>>
>>> It is with great sorrow that I inform you that the museum is being
>>> decommissioned. The MTD subsystem is out of budget for Kconfig
>>> options and already has too many of them, and there is a general
>>> kernel trend to simplify the configuration menu.
>>>
>>> We remove the stone age exhibits along with closing the museum,
>>> but some of the iron age ones are transferred to the regular NAND
>>> depot. Namely, only those which have unique device IDs are
>>> transferred, and the ones which have conflicting device IDs are
>>> removed.
>>>
>>> The machine using this device are:
>>> - axis-dev88
>>> - tosa (via tc6393xb_init)
>>> - spitz based (akita, borzoi, terrier)
>>>
>>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> ---
>>> Peter, as 4 of the 5 machines are ARM-based, can this go via your tree?
>>> ---
>>> hw/block/nand.c | 13 ++++++-------
>>> 1 file changed, 6 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/hw/block/nand.c b/hw/block/nand.c
>>> index 5c8112ed5a4..5f01ba2bc44 100644
>>> --- a/hw/block/nand.c
>>> +++ b/hw/block/nand.c
>>> @@ -138,7 +138,7 @@ static void mem_and(uint8_t *dest, const uint8_t *src,
>>> size_t n)
>>> # define ADDR_SHIFT 16
>>> # include "nand.c"
>>>
>>> -/* Information based on Linux drivers/mtd/nand/nand_ids.c */
>>> +/* Information based on Linux drivers/mtd/nand/raw/nand_ids.c */
>>> static const struct {
>>> int size;
>>> int width;
>>> @@ -154,15 +154,14 @@ static const struct {
>>> [0xe8] = { 1, 8, 8, 4, 0 },
>>> [0xec] = { 1, 8, 8, 4, 0 },
>>> [0xea] = { 2, 8, 8, 4, 0 },
>>> - [0xd5] = { 4, 8, 9, 4, 0 },
>>> [0xe3] = { 4, 8, 9, 4, 0 },
>>> [0xe5] = { 4, 8, 9, 4, 0 },
>>> - [0xd6] = { 8, 8, 9, 4, 0 },
>>>
>>> - [0x39] = { 8, 8, 9, 4, 0 },
>>> - [0xe6] = { 8, 8, 9, 4, 0 },
>>> - [0x49] = { 8, 16, 9, 4, NAND_BUSWIDTH_16 },
>>> - [0x59] = { 8, 16, 9, 4, NAND_BUSWIDTH_16 },
>>> + [0x6b] = { 4, 8, 9, 4, 0 },
>>> + [0xe3] = { 4, 8, 9, 4, 0 },
>>> + [0xe5] = { 4, 8, 9, 4, 0 },
>>
>> This line adds an entry for 0xe5, but there is already one
>> further up in the array (you can see it in this hunk).
>>
>> More generally, it doesn't seem to match the referenced
>> kernel commit, which deletes 14 lines and adds 5
>> (which are a subset of the 14 deleted, really, so
>> they probably show up for us as "9 deletions" since
>> we don't have the #ifdef...#endif the kernel does).
>
> Well, this is what this patch intended to do...
> 14 deletions:
>
> - [0x6e] = { 1, 8, 8, 4, 0 },
> - [0x64] = { 2, 8, 8, 4, 0 },
> - [0x6b] = { 4, 8, 9, 4, 0 },
> - [0xe8] = { 1, 8, 8, 4, 0 },
> - [0xec] = { 1, 8, 8, 4, 0 },
> - [0xea] = { 2, 8, 8, 4, 0 },
> - [0xd5] = { 4, 8, 9, 4, 0 },
> - [0xe3] = { 4, 8, 9, 4, 0 },
> - [0xe5] = { 4, 8, 9, 4, 0 },
> - [0xd6] = { 8, 8, 9, 4, 0 },
> -
> - [0x39] = { 8, 8, 9, 4, 0 },
> - [0xe6] = { 8, 8, 9, 4, 0 },
> - [0x49] = { 8, 16, 9, 4, NAND_BUSWIDTH_16 },
> - [0x59] = { 8, 16, 9, 4, NAND_BUSWIDTH_16 },
>
> and 5 additions:
>
> + [0x6b] = { 4, 8, 9, 4, 0 },
> + [0xe3] = { 4, 8, 9, 4, 0 },
> + [0xe5] = { 4, 8, 9, 4, 0 },
> + [0xd6] = { 8, 8, 9, 4, 0 },
> + [0xe6] = { 8, 8, 9, 4, 0 },
OK, the kernel commit has 9+5=14 removals and 5 "additions",
because the "additions" change the hex index from lowercase
to uppercase. I.e.:
- {"NAND 4MiB 3,3V 8-bit", 0xe5, 512, 4, 0x2000, SP_OPTIONS},
+ {"NAND 4MiB 3,3V 8-bit", 0xE5, 512, 4, 0x2000, SP_OPTIONS},
This explain why the resulting diff is 9 removals:
>
> When combined, the resulting diff is:
>
> - [0x6e] = { 1, 8, 8, 4, 0 },
> - [0x64] = { 2, 8, 8, 4, 0 },
> [0x6b] = { 4, 8, 9, 4, 0 },
> - [0xe8] = { 1, 8, 8, 4, 0 },
> - [0xec] = { 1, 8, 8, 4, 0 },
> - [0xea] = { 2, 8, 8, 4, 0 },
> - [0xd5] = { 4, 8, 9, 4, 0 },
> [0xe3] = { 4, 8, 9, 4, 0 },
> [0xe5] = { 4, 8, 9, 4, 0 },
> [0xd6] = { 8, 8, 9, 4, 0 },
> -
> - [0x39] = { 8, 8, 9, 4, 0 },
> [0xe6] = { 8, 8, 9, 4, 0 },
> - [0x49] = { 8, 16, 9, 4, NAND_BUSWIDTH_16 },
> - [0x59] = { 8, 16, 9, 4, NAND_BUSWIDTH_16 },
>
> But trying to commit that I get (while only removing lines!):
>
> ERROR: code indent should never use tabs
> #14: FILE: hw/block/nand.c:150:
> + [0x6b] = { 4,^I8,^I9, 4, 0 },$
>
> ERROR: code indent should never use tabs
> #15: FILE: hw/block/nand.c:151:
> + [0xe3] = { 4,^I8,^I9, 4, 0 },$
>
> ERROR: code indent should never use tabs
> #16: FILE: hw/block/nand.c:152:
> + [0xe5] = { 4,^I8,^I9, 4, 0 },$
>
> ERROR: code indent should never use tabs
> #17: FILE: hw/block/nand.c:153:
> + [0xd6] = { 8,^I8,^I9, 4, 0 },$
>
> ERROR: code indent should never use tabs
> #18: FILE: hw/block/nand.c:154:
> + [0xe6] = { 8,^I8,^I9, 4, 0 },$
>
> So I changed the tabs by space and committed.
>
> TBH I don't understand what happened. I suppose I
> messed while replacing tabs by space... I feel
> ashamed, sorry.
>
>>
>>> + [0xd6] = { 8, 8, 9, 4, 0 },
>>> + [0xe6] = { 8, 8, 9, 4, 0 },
>>>
>>> [0x33] = { 16, 8, 9, 5, 0 },
>>> [0x73] = { 16, 8, 9, 5, 0 },
>>
>> thanks
>> -- PMM
>>
>