Date: Tue, 16 Mar 2010 17:45:36 +0100 From: Grzegorz Bernacki <gjb@semihalf.com> To: Luiz Otavio O Souza <loos.br@gmail.com> Cc: embedded@freebsd.org, Andrew Turner <andrew@fubar.geek.nz> Subject: Re: NAND Flash Framework for review Message-ID: <4B9FB5B0.5000007@semihalf.com> In-Reply-To: <776ADF3A-4C68-486A-8640-DE0DD6B8874E@gmail.com> References: <0AE04EFA-A3EB-4939-BD81-607C00355B67@semihalf.com> <20100314165825.121d346b@fubar.geek.nz> <CC419602-A9E8-4FE2-A5A5-0BFBD8240EDD@gmail.com> <4B9E1697.9090602@semihalf.com> <20100316101044.0401295e@fubar.geek.nz> <4B9F72BC.1050609@semihalf.com> <776ADF3A-4C68-486A-8640-DE0DD6B8874E@gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
Luiz Otavio O Souza wrote:
> On Mar 16, 2010, at 8:59 AM, Grzegorz Bernacki wrote:
>
>> Andrew Turner wrote:
>>> On Mon, 15 Mar 2010 12:14:31 +0100
>>> Grzegorz Bernacki <gjb@semihalf.com> wrote:
>>>>>> Chip drivers:
>>>>>> - lnand and snand have magic numbers to figure out which drive to
>>>>>> use. We should move these to a flag in the chip parameters.
>>>>> We just need to add the chip size in nand_params and based on that
>>>>> we can calculate the number of address cycles (see below) and the
>>>>> type of chip (if chip >= 128MB and pagesize > 512 then you have a
>>>>> large page device).
>>>>>
>>>> Yes, I was thinking about adding size of page and column address to
>>>> parameters of nfc_send_address.
>>> Why not just send each address byte separately like when the command is
>>> sent? This will then push the requirement to know how many address
>>> bytes to the chip driver.
>> I choose to send whole address in one call to make implementation of mpc8572
>> driver easier. This controller requires to divide address into block number and
>> page & column number and write them into corresponding registers. It would be
>> complicated (however, not impossible) to combine full address from bytes sending
>> via consecutive nfc_send_address calls. On the other hand sending whole address
>> in one call should not complicated drivers for controllers which just send address
>> byte after byte.
>> I was thinking about adding address type parameter to nfc_send_address(). It will
>> tell controller how many address cycles chip requires. It will be defined
>> using pattern (row_bytes << 4) | (col_bytes), for example:
>> #define ADDR_TYPE_2ROW_1COL 0x21
>> #define ADDR_TYPE_2ROW_2COL 0x22
>> #define ADDR_TYPE_2ROW 0x20 (for erase command)
>> #define ADDR_TYPE_ID 0xff (for read id command)
>> It will allow to get rid of (-1) value for unused parameters. Also maybe it would be
>> better to define address as a structure
>> struct nand_addr {
>> uint32_t row;
>> uint32_t column;
>> uint8_t id;
>> uint8_t type;
>> }
>
> But then we need to keep the logic for sending the address cycles on the driver (and we'll get a lot of duplicated code on the tree).
I agree that we will duplicate the code. But on the other side in case of controller which
stores address in registers (I don't know how many such controller exists, at least one):
- I need to determine how many row and column cycles chip requires (I think is it possible
to connect different chips to one controller so it complicates it more)
- I remember which command was issued before address to determine how many bytes of
address to expect (in case of new command I need to update each such a driver)
- And then I need to combine full address from sequential calls of nfc_send_address
It sounds really complicated. And what is worse new commands for some new chips would
require not only new chip driver, but also changes in controller driver.
I will try to implement new mpc8572 driver and see how it looks. Maybe I'm exaggerating
a little bit.
> Look at my patch on how it simplify the send_address routine on nfc_mv.c
>
> And yes, you need to deal with multiple calls in some kind of drivers/controllers.
>
>> The second thing is wait for ready/busy pin. I will add it to nfc methods. When not
>> implemented it will return ENXIO by default. It will be used in nandbus_wait_ready function. It will be called in loop instead of sending STATUS command (unless it return ENXIO on first invocation).
>> Is it good approach? What do you think?
> ...
Thanks for the explanation of ready/busy pin and for your patch. I will review it and test
it tomorrow and let you know if I have any comments.
grzesiek
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4B9FB5B0.5000007>