On Tue, Feb 13, 2024 at 11:25 AM Jonathan Cameron
<Jonathan.Cameron@xxxxxxxxxx> wrote:
>
>
> I thought about suggesting splitting this into an initial patch that just does
> the bits without the controller callbacks. Maybe it would work better that way
> with that introduced after the validate and splitting of transfers (so most
> of patches 1 and 2) as a patch 3 prior to the stm32 additions?
Unless anyone else feels the same way, I'm inclined to avoid the extra
work of splitting it up.
> > +static void __spi_unoptimize_message(struct spi_message *msg)
> > +{
> > + struct spi_controller *ctlr = msg->spi->controller;
> > +
> > + if (ctlr->unoptimize_message)
> > + ctlr->unoptimize_message(msg);
> > +
> > + msg->optimized = false;
> > + msg->opt_state = NULL;
> > +}
>
> Seems misbalanced that this doesn't take a pre_optimized flag in but
> __spi_optimize does. I'd move handling that to outside the call in both cases.
>
>
Agreed.
> > @@ -4331,10 +4463,15 @@ static int __spi_sync(struct spi_device *spi, struct spi_message *message)
> > return -ESHUTDOWN;
> > }
> >
> > - status = __spi_validate(spi, message);
> > - if (status != 0)
> > + status = spi_maybe_optimize_message(spi, message);
> > + if (status)
> > return status;
> >
> > + /*
> > + * NB: all return paths after this point must ensure that
> > + * spi_finalize_current_message() is called to avoid leaking resources.
>
> I'm not sure a catch all like that makes sense. Not sufficient to call
> the finer grained spi_maybe_unoptimize_message() ?
Hmm... this is my bias from a previous fix showing through. Maybe this
comment doesn't belong in this patch. The short answer to your
question is "it's complicated".
