On Mon, 4 Feb 2019 10:10:21 +0000 <Tudor.Ambarus@xxxxxxxxxxxxx> wrote: > + > +static void atmel_qspi_sam9x60_write_regs(const struct atmel_qspi *aq, > + const struct spi_mem_op *op, > + const struct atmel_qspi_cfg *cfg) > +{ > + /* Clear pending interrupts */ > + (void)readl_relaxed(aq->regs + QSPI_SR); > + > + /* Set QSPI Instruction Frame registers */ > + writel_relaxed(cfg->iar, aq->regs + QSPI_IAR); > + if (op->data.dir == SPI_MEM_DATA_IN) > + writel_relaxed(cfg->icr, aq->regs + QSPI_RICR); > + else > + writel_relaxed(cfg->icr, aq->regs + QSPI_ICR); Can you use WICR here (even if ICR == WICR)? > + writel_relaxed(cfg->ifr, aq->regs + QSPI_IFR); > +} Hm, so the only difference we have is the RICR vs ICR reg and the APBTFRTYP_READ vs SAMA5D2_WRITE_TRSFR bit. Not sure it deserves creating 2 hooks for that. Can we have something like ->has_ricr in the caps and then have an if/else block directly in atmel_qspi_set_cfg()?