[flashrom] [PATCH 04/12] use the max_data_read field of the new spi_programmer struct to simplify run_opcode.

[Stefan Tauner]

On Thu, 09 Jun 2011 00:56:08 +0200
Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net> wrote:

> Am 08.06.2011 04:55 schrieb Stefan Tauner:
> > Signed-off-by: Stefan Tauner <stefan.tauner at student.tuwien.ac.at>
> > ---
> >  ichspi.c |   41 +++++++++++++++++------------------------
> >  1 files changed, 17 insertions(+), 24 deletions(-)
> >
> > diff --git a/ichspi.c b/ichspi.c
> > index 12727d1..dff6f32 100644
> > --- a/ichspi.c
> > +++ b/ichspi.c
> > @@ -913,37 +913,30 @@ static int ich9_run_opcode(OPCODE op, uint32_t offset,
> >  static int run_opcode(OPCODE op, uint32_t offset,
> >  		      uint8_t datalength, uint8_t * data)
> >  {
> > +	uint8_t maxlength = spi_programmer->max_data_read;
> >   
> 
> Mh. I see what you're doing here, and it makes sense, but a comment
> would be appreciated in the code:
> /* The maximum data length is identical on ICH/VIA for the maximum read
> length and for the maximum write length without opcode and address.
> Opcode and address are stored in separate registers, not in the data
> registers. The only exception applies if the opcode definition
> (un)intentionally classifies said opcode incorrectly as non-address
> opcode or vice versa. */
> 
> Any suggestions on how to improve that comment?

i am not sure if this is the right place to put the second half of that
comment, because i dont really see the relevance inside that function:
it just delegates its inputs.
OTOH it makes sense to have it at a place where both versions share
code.
what about putting "The maximum payload lengths for read and write
operations without opcode and address are identical on ICH/VIA - using
max_data_read arbitrarily here." there and the rest inside
ich_spi_send_command in the "/* translate read/write array/count */"
comment-block?
-- 
Kind regards/Mit freundlichen Grüßen, Stefan Tauner