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@xxxxxxxxxx>
>>>> Signed-off-by: Stefan Agner <stefan@xxxxxxxx>
>>>> ---
<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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
