[flashrom] [PATCH] ICH SPI Preop handling

[FENG Yu Ning]

Carl-Daniel Hailfinger wrote:
> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>
>
> Index: flashrom-ich9_multicommand_preop/ichspi.c
> ===================================================================
> --- flashrom-ich9_multicommand_preop/ichspi.c   (Revision 725)
> +++ flashrom-ich9_multicommand_preop/ichspi.c   (Arbeitskopie)
> @@ -745,18 +745,25 @@
>  int ich_spi_send_multicommand(struct spi_command *spicommands)
>  {
>        int ret = 0;
> +       int oppos, preoppos;
>        while ((spicommands->writecnt || spicommands->readcnt) && !ret) {
> +               /* Will the next loop iteration be taken? */
> +               if ((spicommands + 1)->writecnt || (spicommands + 1)->readcnt) {

Are we sure there is a (spicommands + 1)?

> +                       preoppos = find_preop(curopcodes, spicommands->writearr[0]);
> +                       oppos = find_opcode(curopcodes, (spicommands + 1)->writearr[0]);
> +                       /* Is the current op a preop for the next op? */

s/current op/current byte/
easier to parse?

> +                       if ((oppos != -1) && (preoppos != -1) &&
> +                           (curopcodes->opcode[oppos].atomic == preoppos)) {
> +                               printf_debug("opcode 0x%02x will be run as PREOP\n",
> +                                            spicommands->writearr[0]);
> +                               ret = 0;
> +                               spicommands++;
> +                               continue;
> +                       }
> +               }
> +
>                ret = ich_spi_send_command(spicommands->writecnt, spicommands->readcnt,
>                                           spicommands->writearr, spicommands->readarr);
> -               /* This awful hack needs to be smarter.
> -                */
> -               if ((ret == SPI_INVALID_OPCODE) &&
> -                   ((spicommands->writearr[0] == JEDEC_WREN) ||
> -                    (spicommands->writearr[0] == JEDEC_EWSR))) {
> -                       printf_debug(" due to SPI master limitation, ignoring"
> -                                    " and hoping it will be run as PREOP\n");
> -                       ret = 0;
> -               }
>                spicommands++;
>        }
>        return ret;
>

I will write it this way:

int ich_spi_send_multicommand(struct spi_command *cmd)
{
        int ret, op, pre;
        /* better if I could write cmd->wcnt, cmd->rcnt */
        for (; cmd->writecnt || cmd->readcnt; cmd++) {
                /* if we are sure there is a (cmd + 1) */
                if ((cmd + 1)->writecnt || (cmd + 1)->readcnt) {
                        /* my bad to invent this eye-hurting 'curopcodes' */
                        pre = find_preop (curopcodes,  cmd->writearr[0]);
                        op  = find_opcode(curopcodes, (cmd + 1)->writearr[0]);
                        if ((op != -1) && (pre != -1) &&
                            /* see struct _OPCODE comments, atomic
value is error prone.
                               I am considering flattening struct
OPCODES, so that we can
                               write curop[i].atomic and curpre[i]
instead of this looong
                               expression */
                            (curopcodes->opcode[op].atomic-- == pre))
                                continue;
                }

                ret = ich_spi_send_command(cmd->writecnt, cmd->readcnt,
                                           cmd->writearr, cmd->readarr);
                if (ret) break;
        }
        return ret;
}

do you think it is easier to read?

yu ning