Hi Scott,

On Dec 11, 2012, at 3:09 AM, Scott Wood wrote:

> On 12/10/2012 09:24:32 AM, Pantelis Antoniou wrote:
>> Introduce on-the fly DFU NAND support.
>> Signed-off-by: Pantelis Antoniou <pa...@antoniou-consulting.com>
>> ---
>> drivers/dfu/Makefile   |   1 +
>> drivers/dfu/dfu.c      |   7 ++
>> drivers/dfu/dfu_nand.c | 194 
>> +++++++++++++++++++++++++++++++++++++++++++++++++
>> include/dfu.h          |  23 ++++++
>> 4 files changed, 225 insertions(+)
>> create mode 100644 drivers/dfu/dfu_nand.c
> 
> What is DFU?  I don't see anything in README or doc/, despite there already 
> being CONFIG symbols for it.
> 

This gets answered by a following email.

>> +static int nand_block_op(enum dfu_nand_op op, struct dfu_entity *dfu,
>> +                    u64 offset, void *buf, long *len)
>> +{
>> +    char cmd_buf[DFU_CMD_BUF_SIZE];
>> +    u64 start, count;
>> +    int ret;
>> +    int dev;
>> +    loff_t actual;
>> +
>> +    /* if buf == NULL return total size of the area */
>> +    if (buf == NULL) {
>> +            *len = dfu->data.nand.size;
>> +            return 0;
>> +    }
>> +
>> +    start = dfu->data.nand.start + offset + dfu->bad_skip;
>> +    count = *len;
>> +    if (start + count >
>> +                    dfu->data.nand.start + dfu->data.nand.size) {
>> +            printf("%s: block_op out of bounds\n", __func__);
>> +            return -1;
>> +    }
>> +    dev = nand_curr_device;
>> +    if (dev < 0 || dev >= CONFIG_SYS_MAX_NAND_DEVICE ||
>> +            !nand_info[dev].name) {
>> +            printf("%s: invalid nand device\n", __func__);
>> +            return -1;
>> +    }
>> +
>> +    sprintf(cmd_buf, "nand %s %p %llx %llx",
>> +            op == DFU_OP_READ ? "read" : "write",
>> +             buf, start, count);
>> +
>> +    debug("%s: %s 0x%p\n", __func__, cmd_buf, cmd_buf);
>> +    ret = run_command(cmd_buf, 0);
> 
> Why not use the C interface to NAND?

Sigh. That's what I'm working on right now. The original implementation for mmc 
uses the user
facing cmd api, so that's what I'm using for nand too.

The problem is that I see, is that by using this method 
(dfu_mmc|dfu_nand|dfu_foo) this piece of code 
gets duplicated for each different dfu target.

I was told that Device Model will fix everything, eventually, so I abandoned 
anything more generic than
that. I will follow up with a patch that uses the C interface, and hope that 
sometime in the
future we can consolidate. Maybe. 

> 
>> +    /* find out how much actual bytes have been written */
>> +    /* the difference is the amount of skip we must add from now on */
>> +    actual = nand_extent_skip_bad(&nand_info[dev], start, count);
> 
> ...especially since you already need to interact with it here?
> 
> -Scott

Regards

-- Pantelis

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to