On Fri, 16 Jul 2021 at 15:19, Eddie James <eajames@xxxxxxxxxxxxx> wrote: > > Set and increment the sequence number during the submit operation. > This prevents sequence number conflicts between different users of > the interface. A sequence number conflict may result in a user > getting an OCC response meant for a different command. Since the > sequence number is now modified, the checksum must be calculated and > set before submitting the command. > > Signed-off-by: Eddie James <eajames@xxxxxxxxxxxxx> Reviewed-by: Joel Stanley <joel@xxxxxxxxx> > @@ -479,11 +483,26 @@ int fsi_occ_submit(struct device *dev, const void *request, size_t req_len, > return -EINVAL; > } > > + /* Checksum the request, ignoring first byte (sequence number). */ > + for (i = 1; i < req_len - 2; ++i) > + checksum += byte_request[i]; > + This could go below, after you've got the sequence number, so the checksumming all happens in the same spot? The driver has become a bit of a maze, I can't tell how you're deciding what goes in fsi_occ_submit vs occ_write vs occ_putsram. If oyu have some ideas on how to simplify it then I would welcome those changes. > mutex_lock(&occ->occ_lock); > > - /* Extract the seq_no from the command (first byte) */ > - seq_no = *(const u8 *)request; > - rc = occ_putsram(occ, request, req_len); > + /* > + * Get a sequence number and update the counter. Avoid a sequence > + * number of 0 which would pass the response check below even if the > + * OCC response is uninitialized. Any sequence number the user is > + * trying to send is overwritten since this function is the only common > + * interface to the OCC and therefore the only place we can guarantee > + * unique sequence numbers. > + */ > + seq_no = occ->sequence_number++; > + if (!occ->sequence_number) > + occ->sequence_number = 1; > + checksum += seq_no; > + > + rc = occ_putsram(occ, request, req_len, seq_no, checksum); > if (rc) > goto done;
