On Thu, Jan 16, 2014 at 10:12:01PM +0100, Maxime Ripard wrote:
> On Thu, Jan 16, 2014 at 07:40:03PM +0000, Mark Brown wrote:
> > > + if (status & SUN6I_INT_CTL_RF_OVF) {
> > > + sun6i_spi_drain_fifo(sspi, SUN6I_FIFO_DEPTH);
> > > + sun6i_spi_write(sspi, SUN6I_INT_STA_REG, SUN6I_INT_CTL_RF_OVF);
> > > + return IRQ_HANDLED;
> > > + }
> > This looks like an overflow - a log message would be helpful for users
> > and you should possibly be flagging an error on the current transfer.
> Hmmm, that was an attempt at receiving more bytes than the FIFO can
> handle, but I guess the FIFO full interrupt would be more appropriate
> for this.
If you've got an overflow interrupt that suggests that the data is
already corrupted, assuming the interrupt isn't misnamed.
> > > + ret = clk_set_rate(sspi->mclk, 100000000);
> > > + if (ret) {
> > > + dev_err(&pdev->dev, "Couldn't change module clock rate\n");
> > > + goto err2;
> > > + }
> > Does this really need to be fatal (or done at all)? There seems to be
> > another reasonably flexible divider in the IP and it's more common to
> > either set this per transfer to something that rounds nicely or just use
> > the default and rely on the dividers.
> The default parent of the module clock runs at 24MHz, that means that
> we won't be able to reach a spi clock higher than 12MHz, which seems
> quite low. We can always change the rate in the transfer setup code
> though, if needs be.
12MHz is actually quite a common limit for SPI interfaces (half of a
24MHz master clock like the IP itself has) but yeah, you want to go
higher if you can. Doing it on transfer setup is going to mean that
you save a little power when you don't need the extra speed too.
Attachment:
signature.asc
Description: Digital signature
