On 07.11.2012 16:37, Philip, Avinash wrote:
> On Wed, Nov 07, 2012 at 15:18:37, Daniel Mack wrote:
>> On 05.11.2012 14:29, Philip, Avinash wrote:
>>> On Mon, Nov 05, 2012 at 18:28:22, Daniel Mack wrote:
>>>> On 05.11.2012 12:03, Philip, Avinash wrote:
>>>>> On Fri, Nov 02, 2012 at 20:55:56, Daniel Mack wrote:
>>>>>> This patch adds basic DT bindings for OMAP GPMC.
>>>>>>
>>>>>> The actual peripherals are instanciated from child nodes within the GPMC
>>>>>> node, and the only type of device that is currently supported is NAND.
>>>>>>
>>>>>> Code was added to parse the generic GPMC timing parameters and some
>>>>>> documentation with examples on how to use them.
>>>>>>
>>>>>> Successfully tested on an AM33xx board.
>>>>>>
>>>>>> Signed-off-by: Daniel Mack <zon...@gmail.com>
>>>>> [...]
>>>>>> +
>>>>>> + nand@0,0 {
>>>>>> + reg = <0 0 0>; /* CS0, offset 0 */
>>>>>> + nand-bus-width = <16>;
>>>>>> + nand-ecc-mode = "none";
>>>>>> +
>>>>>> + gpmc,sync-clk = <0>;
>>>>>> + gpmc,cs-on = <0>;
>>>>>> + gpmc,cs-rd-off = <36>;
>>>>>> + gpmc,cs-wr-off = <36>;
>>>>>> + gpmc,adv-on = <6>;
>>>>>> + gpmc,adv-rd-off = <24>;
>>>>>> + gpmc,adv-wr-off = <36>;
>>>>>> + gpmc,we-off = <30>;
>>>>>> + gpmc,oe-off = <48>;
>>>>>> + gpmc,access = <54>;
>>>>>> + gpmc,rd-cycle = <72>;
>>>>>> + gpmc,wr-cycle = <72>;
>>>>>> + gpmc,wr-access = <30>;
>>>>>> + gpmc,wr-data-mux-bus = <0>;
>>>>>> +
>>>>>> + #address-cells = <1>;
>>>>>> + #size-cells = <1>;
>>>>>> +
>>>>>
>>>>> Can you take the timings (for example) from arago tree. The timings is
>>>>> tested in am335x-evm
>>>>> So the timings can be directly used to populate GPMC timings. Timings can
>>>>> found at
>>>>>
>>>>> http://arago-project.org/git/projects/?p=linux-am33x.git;a=commitdiff;
>>>>> h=66bfbd2c5b35dc81edce0c24843c476161ab5978;hp=370630359cb8db711cf0941cd2a242e28ccfb61e
>>>>>
>>>>> [...]
>>>>>> +static int gpmc_probe_dt(struct platform_device *pdev)
>>>>>
>>>>> Can you take care of the following section mismatch.
>>>>> WARNING: vmlinux.o(.text+0x1e2d0): Section mismatch in reference
>>>>> from the function gpmc_probe_dt() to the function
>>>>> .init.text:gpmc_nand_init().
>>>>
>>>> Sore, both fixed for v4.
>>>>
>>>>> [...]
>>>>>> +
>>>>>> + val = of_get_nand_ecc_mode(child);
>>>>>> + if (val >= 0)
>>>>>> + gpmc_nand_data->ecc_opt = val;
>>>>>
>>>>> This will fail for BCH. Index of "soft_bch" is 5 & also don't have
>>>>> selection
>>>>> option between for BCH4 & BCH8 also.
>>>>> Can you use the of_property_read_u32 (as done early) to pass the ecc
>>>>> selection
>>>>> from dt file. This will help selection of BCH4 & BCH8 ecc options.
>>>>
>>>> Hmm. Shouldn't we rather teach of_get_nand_ecc_mode() that two modes and
>>>> bring the enum in sync?
>>>
>>> ecc_opt is for selecting different ecc layout and not for selecting ecc
>>> mode.
>>>
>>> In omap2 driver NAND_ECC_HW ecc mode supports 3 ecc layout
>>> OMAP_ECC_HAMMING_CODE_HW_ROMCODE
>>> OMAP_ECC_BCH4_CODE_HW
>>> OMAP_ECC_BCH8_CODE_HW
>>>
>>> So selection of ecc layout data should come from DT not ecc mode.
>>
>> Ok, I see. I would still like to set them by string rather than magic
>> numbers that map to enum entries. Valid values would be "none", "hw",
>> "hw-romcode", "bch4" and "bch8". Are you ok with that?
>
> Ok, that's nice. Better use ecc_opt instead of ecc_mode.
I did some more extensive tests that include reading the same nand pages
from both U-Boot and the kernel with BCH8 ECC, and it turns out that
->is_elm_used needs to be set in the pdata in order to make this work.
So the question is whether we actually want to have a DT property for
that or just always enable that bit in case a hardware supported ecc
mode is selected. Any opinion on this?
That's the last topic before I'm clear to send off v4.
Thanks,
Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
