On Fri, Jun 16, 2017 at 11:20:10AM +0200, Simon Horman wrote:
> On Fri, Jun 16, 2017 at 10:14:45AM +0200, Wolfram Sang wrote:
> > Hi Simon,
> >
> > > * Don't add bogus ',' after KERN_DEBUG
> >
> > Well...
> >
> > > @@ -127,16 +127,17 @@ static int tmio_mmc_next_sg(struct tmio_mmc_host
> > > *host)
> > >
> > > #define STATUS_TO_TEXT(a, status, i) \
> > > do { \
> > > - if (status & TMIO_STAT_##a) { \
> > > - if (i++) \
> > > - printk(" | "); \
> > > - printk(#a); \
> > > + if ((status) & TMIO_STAT_##a) { \
> > > + if ((i)++) \
> > > + printk(KERN_DEBUG " | "); \
> > > + printk(KERN_DEBUG, #a); \
> >
> > ...still there.
>
> Oops.
>
> > > @@ -349,13 +350,22 @@ static int tmio_mmc_start_command(struct
> > > tmio_mmc_host *host, struct mmc_command
> > > }
> > >
> > > switch (mmc_resp_type(cmd)) {
> > > - case MMC_RSP_NONE: c |= RESP_NONE; break;
> > > + case MMC_RSP_NONE:
> > > + c |= RESP_NONE;
> > > + break;
> > > case MMC_RSP_R1:
> > > case MMC_RSP_R1_NO_CRC:
> > > - c |= RESP_R1; break;
> > > - case MMC_RSP_R1B: c |= RESP_R1B; break;
> > > - case MMC_RSP_R2: c |= RESP_R2; break;
> > > - case MMC_RSP_R3: c |= RESP_R3; break;
> > > + c |= RESP_R1;
> > > + break;
> > > + case MMC_RSP_R1B:
> > > + c |= RESP_R1B;
> > > + break;
> > > + case MMC_RSP_R2:
> > > + c |= RESP_R2;
> > > + break;
> > > + case MMC_RSP_R3:
> > > + c |= RESP_R3;
> > > + break;
> >
> > Very personal, I'd prefer the old way but I don't insist.
Sure, I'll leave that as-is.
I don't mind either way.
> >
> > > - * Disable auto CMD12 at IO_RW_EXTENDED and
> > > SET_BLOCK_COUNT
> > > - * when doing multiple block transfer
> > > + * Disable auto CMD12 at IO_RW_EXTENDED and
> > > + * SET_BLOCK_COUNT when doing multiple block
> > > + * transfer
> >
> > I'd think 'transfer' can go to the previous line?
Moved.
> > > if ((host->pdata->flags & TMIO_MMC_HAVE_CMD12_CTRL) &&
> > > - (cmd->opcode == SD_IO_RW_EXTENDED ||
> > > host->mrq->sbc))
> > > + (cmd->opcode == SD_IO_RW_EXTENDED ||
> > > + host->mrq->sbc))
> >
> > Another subjective taste thingie ;)
Undone.
> > > - dev_warn_once(&host->pdev->dev,
> > > - "Too many taps, skipping tuning. Please consider updating
> > > size of taps field of tmio_mmc_host\n");
> > > + dev_warn_once(&host->pdev->dev, "Too many taps, skipping
> > > tuning. Please consider updating size of taps field of tmio_mmc_host\n");
> >
> > Dito.
Also undone.
> > What kind of tests did you run with these?
>
> I checked that Lager boots and that I/O speeds are as expected using dd.
>
