From: Sekhar Nori <nsekhar@ti.com>
To: Karl Beldan <kbeldan@baylibre.com>
Cc: <devicetree@vger.kernel.org>,
<linux-arm-kernel@lists.infradead.org>,
<linux-kernel@vger.kernel.org>, Rob Herring <robh+dt@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Russell King <linux@armlinux.org.uk>,
Santosh Shilimkar <ssantosh@kernel.org>,
Kevin Hilman <khilman@baylibre.com>,
Karl Beldan <karl.beldan+oss@gmail.com>
Subject: Re: [PATCH 3/4] ARM: dts: da850-lcdk: Add NAND to DT
Date: Wed, 10 Aug 2016 17:23:23 +0530 [thread overview]
Message-ID: <57AB15B3.1050907@ti.com> (raw)
In-Reply-To: <20160810111944.GA31616@gobelin>
On Wednesday 10 August 2016 04:49 PM, Karl Beldan wrote:
> On Wed, Aug 10, 2016 at 03:01:30PM +0530, Sekhar Nori wrote:
>> On Wednesday 10 August 2016 02:34 PM, Karl Beldan wrote:
>>> On Wed, Aug 10, 2016 at 02:01:57PM +0530, Sekhar Nori wrote:
>>>> On Tuesday 09 August 2016 10:45 PM, Karl Beldan wrote:
>>>>> This adds DT support for the NAND connected to the SoC AEMIF.
>>>>> The parameters (timings, ecc) are the same as what the board ships with
>>>>> (default AEMIF timings, 1bit ECC) and improvements will be handled in
>>>>> due course.
>>>>
>>>> I disagree that we need to be compatible to the software that ships with
>>>> the board. Thats software was last updated 3 years ago. Instead I would
>>>> concern with what the hardware supports. So, if the hardware can support
>>>> 4-bit ECC, I would use that.
>>>>
>>> I am not saying we _need_ to be compatible.
>>
>> Alright then, please drop references to what software the board ships
>> with in the commit message and in the patch itself.
>>
>
> I hadn't seen this comment before sending v2.
>
>>>
>>>> If driver is broken for 4-bit ECC, please fix that up first.
>>>>
>>> Since this issue is completely separate from my DT improvements
>>> I'll stick to resubmitting the series, applying my LCDK changes to the
>>> EVM too, besides you'll be able to compare the behavior without ECC
>>> discrepancies.
>>> I took note that you are likely to not apply without the ECC fix.
>>
>> Yeah, I would not like to apply with 1-bit ECC now and then change to
>> 4-bit ECC soon after.
>>
>
> Both mityomapl138 from mainline and hawkboard from TI's BSP release
> include the comment:
> - "4 bit mode is not supported with 16 bit NAND"
> It is not clear whether they imply that the HW has issues or if it's SW
At least the TRM says "The EMIFA supports 4-bit ECC on 8-bit/16-bit NAND
Flash.". And then the TRM goes on to describe how 4-bit ECC is supposed
to work for 16-bit NAND. I could not find any errata related to this in
the errata document.
> only, but 4-bits ECC is a different matter and I hope you'll integrate
> the current changes prior to tackling it.
Lets apply everything together. This is a new feature, not a bug fix.
There is no need to rush it. Also, for the NAND part on LCDK, per the
micron datasheet, minimum of 4-bit ECC is needed.
I will take a look at other patches in this series.
Regards,
Sekhar
next prev parent reply other threads:[~2016-08-10 18:36 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-08-09 17:15 [PATCH 0/4] Add DT support for NAND to LCDK Karl Beldan
2016-08-09 17:15 ` [PATCH 1/4] memory: ti-aemif: Get a named clock rather than an unnamed one Karl Beldan
2016-08-10 7:00 ` Karl Beldan
2016-08-10 7:27 ` Karl Beldan
2016-08-09 17:15 ` [PATCH 2/4] ARM: dts: da850: Add an aemif node Karl Beldan
2016-08-10 7:48 ` Sekhar Nori
2016-08-10 8:01 ` Karl Beldan
2016-08-10 8:02 ` Sekhar Nori
2016-08-10 8:07 ` Karl Beldan
2016-08-10 8:12 ` Sekhar Nori
2016-08-10 8:26 ` Karl Beldan
2016-08-10 8:29 ` Sekhar Nori
2016-08-10 8:34 ` Karl Beldan
2016-08-10 8:34 ` Sekhar Nori
2016-08-10 9:28 ` Karl Beldan
2016-08-10 9:42 ` Karl Beldan
2016-08-13 11:42 ` Karl Beldan
2016-08-09 17:15 ` [PATCH 3/4] ARM: dts: da850-lcdk: Add NAND to DT Karl Beldan
[not found] ` <57AAE67D.4000404@ti.com>
2016-08-10 9:04 ` Karl Beldan
2016-08-10 9:31 ` Sekhar Nori
2016-08-10 11:19 ` Karl Beldan
2016-08-10 11:53 ` Sekhar Nori [this message]
2016-08-16 23:20 ` Karl Beldan
2016-08-29 7:49 ` Karl Beldan
2016-08-10 9:29 ` Karl Beldan
2016-08-09 17:15 ` [PATCH 4/4] ARM: davinci_all_defconfig: Enable AEMIF as a module Karl Beldan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=57AB15B3.1050907@ti.com \
--to=nsekhar@ti.com \
--cc=devicetree@vger.kernel.org \
--cc=karl.beldan+oss@gmail.com \
--cc=kbeldan@baylibre.com \
--cc=khilman@baylibre.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@armlinux.org.uk \
--cc=mark.rutland@arm.com \
--cc=robh+dt@kernel.org \
--cc=ssantosh@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).