Re: [PATCH 5/9] mtd: rawnand: bcm47xx: Implement the exec_op() interface

[Miquel Raynal <miquel.raynal@xxxxxxxxxxx>]

Hi Boris,

> > > +static int
> > > +bcm47xxnflash_ops_bcm4706_exec_cmd_addr(struct nand_chip *chip,
> > > +					const struct nand_subop *subop)
> > > +{
> > > +	struct bcm47xxnflash *b47n = nand_get_controller_data(chip);
> > > +	u32 nctl = 0, col = 0, row = 0, ncols = 0, nrows = 0;
> > > +	unsigned int i, j;
> > > +
> > > +	for (i = 0; i < subop->ninstrs; i++) {
> > > +		const struct nand_op_instr *instr = &subop->instrs[i];
> > > +
> > > +		switch (instr->type) {
> > > +		case NAND_OP_CMD_INSTR:
> > > +			if (WARN_ON_ONCE((nctl & NCTL_CMD0) &&
> > > +					 (nctl & NCTL_CMD1W)))
> > > +				return -EINVAL;
> > > +			else if (nctl & NCTL_CMD0)
> > > +				nctl |= NCTL_CMD1W |
> > > +					((u32)instr->ctx.cmd.opcode << 8);
> > > +			else
> > > +				nctl |= NCTL_CMD0 | instr->ctx.cmd.opcode;
> > > +			break;
> > > +		case NAND_OP_ADDR_INSTR:
> > > +			for (j = 0; j < instr->ctx.addr.naddrs; j++) {
> > > +				u32 addr = instr->ctx.addr.addrs[j];
> > > +
> > > +				if (i < 2) {    
> > 
> > Don't you mean j here?              ^
> >   
> 
> Nice catch! Indeed, it should be j.
> 
> > > +					col |= addr << i * 8;    
> > 
> > I'm not sure this will work, addr is 32-bit and col as well, I bet you
> > won't end up with what you expect.  
> 
> Well, assuming I use j that's really what I want. addr is an u32 to
> allow for a shift greater than 8, but the value has be extracted
> from the instr->ctx.addr.addrs array which is an u8 array, thus
> making addr <= 0xff.

Oh that's absolutely right. It's fine then!

Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/