RE: [PATCH] SPI: Add driver for Cadence SPI controller

From: Harini Katakam
Date: Tue Mar 18 2014 - 01:23:28 EST


Hi Mark,

> -----Original Message-----
> From: Mark Brown [mailto:broonie@xxxxxxxxxx]
> Sent: Monday, March 17, 2014 11:45 PM
> To: Josh Cartwright
> Cc: Harini Katakam; robh+dt@xxxxxxxxxx; pawel.moll@xxxxxxx;
> mark.rutland@xxxxxxx; ijc+devicetree@xxxxxxxxxxxxxx;
> galak@xxxxxxxxxxxxxx; rob@xxxxxxxxxxx; grant.likely@xxxxxxxxxx;
> devicetree@xxxxxxxxxxxxxxx; linux-doc@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; linux-spi@xxxxxxxxxxxxxxx; Michal Simek
> Subject: Re: [PATCH] SPI: Add driver for Cadence SPI controller
>
> On Mon, Mar 17, 2014 at 12:59:11PM -0500, Josh Cartwright wrote:
> > On Mon, Mar 17, 2014 at 05:30:17PM +0000, Mark Brown wrote:
> > > On Mon, Mar 17, 2014 at 05:35:36PM +0530, Harini Katakam wrote:
>
> > > > +static int __maybe_unused cdns_spi_suspend(struct device *dev) {
>
> > > This needs to call spi_master_suspend() as well (and similarly on
> > > resume).
>
> > I'm not that familiar with the SPI core, but this seems like an
> > inversion. Is there a reason why the SPI master class doesn't
> > implement
> > suspend/resume() callbacks which handle stopping/starting the queue
> > automatically for all masters?
>
> This is for users of an optional feature of the infrastructure. We probably
> should just call it anyway since it does have checks for the feature being
> used (but given all the open coding around this stuff I'd need to verify that
> the class callbacks would reliably get called).
>
> In any case that's not happening now and the driver as it stands is buggy
> since it's trampling all over the hardware without syncing with anything that
> isn't ongoing.

In case of a suspend, we are stopping an ongoing transfer and disabling the interface.
In case I add clock disable and anything else to unprepared too, it will be a cleaner
exit but it will still stop the transfer right? What do you suggest?
Should we wait for transfer to complete or a timeout to occur?

Regards,
Harini


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/