[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <7efc2975ed8c24a708d43dec06a48ceb@agner.ch>
Date: Tue, 22 May 2018 16:28:37 +0200
From: Stefan Agner <stefan@...er.ch>
To: Dmitry Osipenko <digetx@...il.com>
Cc: boris.brezillon@...tlin.com, dwmw2@...radead.org,
computersforpeace@...il.com, marek.vasut@...il.com,
robh+dt@...nel.org, mark.rutland@....com, thierry.reding@...il.com,
mturquette@...libre.com, sboyd@...nel.org, dev@...xeye.de,
miquel.raynal@...tlin.com, richard@....at, marcel@...wiler.com,
krzk@...nel.org, benjamin.lindqvist@...ian.se,
jonathanh@...dia.com, pdeschrijver@...dia.com, pgaikwad@...dia.com,
mirza.krak@...il.com, linux-mtd@...ts.infradead.org,
linux-tegra@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-clk@...r.kernel.org
Subject: Re: [RESEND PATCH 2/5] mtd: rawnand: add NVIDIA Tegra NAND Flash
controller driver
On 22.05.2018 15:34, Dmitry Osipenko wrote:
> On 22.05.2018 15:19, Stefan Agner wrote:
>> [review sent to my first patch sent off-ml, moving to ml thread]
>>
>> On 21.05.2018 16:05, Dmitry Osipenko wrote:
>>> Hello Stefan,
>>>
>>> I don't have expertise to review the actual NAND-related driver logic, so I only
>>> reviewed the basics. The driver code looks good to me, though I've couple minor
>>> comments.
>>>
>>> On 21.05.2018 03:16, Stefan Agner wrote:
>>>> Add support for the NAND flash controller found on NVIDIA
>>>> Tegra 2 SoCs. This implementation does not make use of the
>>>> command queue feature. Regular operations/data transfers are
>>>> done in PIO mode. Page read/writes with hardware ECC make
>>>> use of the DMA for data transfer.
>>>>
>>>> Signed-off-by: Lucas Stach <dev@...xeye.de>
>>>> Signed-off-by: Stefan Agner <stefan@...er.ch>
>>>> ---
<snip>
>>>> +static int tegra_nand_read_page(struct mtd_info *mtd, struct nand_chip *chip,
>>>> + uint8_t *buf, int oob_required, int page)
>>>> +{
>>>> + struct tegra_nand *nand = to_tegra_nand(mtd);
>>>> + u32 value, addrs;
>>>> +
>>>> + writel(NAND_CMD_READ0, nand->regs + CMD_1);
>>>> + writel(NAND_CMD_READSTART, nand->regs + CMD_2);
>>>> +
>>>> + addrs = tegra_nand_fill_address(mtd, chip, page);
>>>> +
>>>> + value = readl(nand->regs + CFG);
>>>> + value |= CFG_HW_ECC | CFG_ERR_COR;
>>>> + writel(value, nand->regs + CFG);
>>>> +
>>>> + writel(mtd->writesize - 1, nand->regs + DMA_CFG_A);
>>>> + writel(nand->data_dma, nand->regs + DATA_PTR);
>>>> +
>>>> + if (oob_required) {
>>>> + writel(mtd_ooblayout_count_freebytes(mtd) - 1,
>>>> + nand->regs + DMA_CFG_B);
>>>> + writel(nand->oob_dma, nand->regs + TAG_PTR);
>>>> + } else {
>>>> + writel(0, nand->regs + DMA_CFG_B);
>>>> + writel(0, nand->regs + TAG_PTR);
>>>> + }
>>>> +
>>>> + value = DMA_CTRL_GO | DMA_CTRL_IN | DMA_CTRL_PERF_EN |
>>>> + DMA_CTRL_REUSE | DMA_CTRL_IE_DONE | DMA_CTRL_IS_DONE |
>>>> + DMA_CTRL_BURST_8 | DMA_CTRL_EN_A;
>>>
>>> Wouldn't be more efficient to set DMA burst to 16 words? The writesize seems
>>> always aligned to at least 64 words.
>>>
>>
>> Hm, haven't tested 16 words, 8 was the setting Lucas used.
>>
>> Are you sure this is only about write size? Not sure, but isn't the ECC
>> area also DMA'd? On Colibri we use RS with t=8, hence 144 bytes parity,
>> so this would be properly aligned non the less...
>>
>
> I don't know, that's up to you to figure out. Is RS stands for Reed-Solomon and
> t=8 is the max number of redundant words?
>
RS = Reed-Solomon
t=8 maximum symbol errors
Will do some testing and check whether it fails.
--
Stefan
Powered by blists - more mailing lists