On Thu, 2019-05-09 at 10:55 +0000, kernel@xxxxxxxxxxxxxxxx wrote:
> From: Martin Sperl <kernel@xxxxxxxxxxxxxxxx>
>
> When an error occurs in ctlr->prepare_message or spi_map_msg
> then spi_resources is not cleared leaving unexpected entries and
> memory.
>
> Also there is an ordering issue:
> On ititialization:
> * prepare_message
> * spi_map_msg
>
> and when releasing:
> * spi_res_release (outside of finalize)
> -> this restores the spi structures
> * spi_unmap_msg
> * unprepare_message
>
> So the ordering is wrong in the case that prepare_message is
> modifying the spi_message and spi_message.resources.
>
> Especially the dma unmapping of all the translations are not done.
>
> There is still an ambiguity where is the "best" place where to place
> spi_res_release - it definitely has to be after spi_unmap_msg,
> but if it should be before or after unprepare message is not well
> defined.
>
> Ideally this dma unmap and unprepare_message would be executed
> by spi_res_release and thus in the guaranteed order they were applied.
>
> This incidently implements a better way for the reverted
> commit c9ba7a16d0f1 ("spi: Release spi_res after finalizing message")
>
> Signed-off-by: Martin Sperl <kernel@xxxxxxxxxxxxxxxx>
> ---
> drivers/spi/spi.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index 8eb7460dd744..1dfb19140bbe 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -1181,8 +1181,6 @@ static int spi_transfer_one_message(struct
> spi_controller *ctlr,
> if (msg->status && ctlr->handle_err)
> ctlr->handle_err(ctlr, msg);
>
> - spi_res_release(ctlr, msg);
> -
> spi_finalize_current_message(ctlr);
>
> return ret;
> @@ -1448,6 +1446,15 @@ void spi_finalize_current_message(struct spi_controller
> *ctlr)
> }
> }
>
> + /* where to put the release is a slight nightmare because
> + * ctlr->prepare_message may add to resources as well.
> + * so the question is: call it before unprepare or after?
> + * for now leave it after - the asumption here is that
> + * if prepare_message is using spi_res for callbacks,
> + * then no unprepare_message is used
> + */
> + spi_res_release(ctlr, mesg);
> +
> spin_lock_irqsave(&ctlr->queue_lock, flags);
> ctlr->cur_msg = NULL;
> ctlr->cur_msg_prepared = false;
> --
> 2.11.0
Tested-by: Nicolas Saenz Julienne <nsaenzjulienne@xxxxxxx>
Attachment:
signature.asc
Description: This is a digitally signed message part
