On 09:38 Fri 09 Dec , Ryan Mallon wrote:
> On 09/12/11 02:23, Jean-Christophe PLAGNIOL-VILLARD wrote:
>
> > this will allow to use the pata_at91 on a single zImage
> >
> > Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@xxxxxxxxxxxx>
> > Cc: Nicolas Ferre <nicolas.ferre@xxxxxxxxx>
> > Cc: linux-ide@xxxxxxxxxxxxxxx
>
>
> Couple of minor comments below,
>
> ~Ryan
>
> > ---
> > Hi,
> >
> > it's depends on other patch for AT91 can we apply via at91
> >
> > Best Regards,
> > J.
> > drivers/ide/at91_ide.c | 65 +++++++++++++++++++++++++++--------------------
> > 1 files changed, 37 insertions(+), 28 deletions(-)
> >
> > diff --git a/drivers/ide/at91_ide.c b/drivers/ide/at91_ide.c
> > index 41d4155..407595b 100644
> > --- a/drivers/ide/at91_ide.c
> > +++ b/drivers/ide/at91_ide.c
> > @@ -59,41 +59,50 @@
> > #define ALT_MODE 0x00e00000
> > #define REGS_SIZE 8
> >
> > -#define enter_16bit(cs, mode) do { \
> > - mode = at91_sys_read(AT91_SMC_MODE(cs)); \
> > - at91_sys_write(AT91_SMC_MODE(cs), mode | AT91_SMC_DBW_16); \
> > -} while (0)
> > +static inline void enter_16bit(int cs, struct sam9_smc_config *smc)
> > +{
> > + sam9_smc_read_mode(0, cs, smc);
> > + smc->mode = (smc->mode & ~AT91_SMC_DBW) | AT91_SMC_DBW_16;
> > + sam9_smc_write_mode(0, cs, smc);
> > +}
> >
> > -#define leave_16bit(cs, mode) at91_sys_write(AT91_SMC_MODE(cs), mode);
> > +static inline void leave_16bit(int cs, struct sam9_smc_config *smc)
> > +{
> > + smc->mode = (smc->mode & ~AT91_SMC_DBW) | AT91_SMC_DBW_8;
> > + sam9_smc_write_mode(0, cs, smc);
> > +}
> >
> > static void set_smc_timings(const u8 chipselect, const u16 cycle,
> > const u16 setup, const u16 pulse,
> > const u16 data_float, int use_iordy)
> > {
> > - unsigned long mode = AT91_SMC_READMODE | AT91_SMC_WRITEMODE |
> > + struct sam9_smc_config smc;
> > +
> > + smc.mode = AT91_SMC_READMODE | AT91_SMC_WRITEMODE |
> > AT91_SMC_BAT_SELECT;
> >
> > /* disable or enable waiting for IORDY signal */
> > if (use_iordy)
> > - mode |= AT91_SMC_EXNWMODE_READY;
> > + smc.mode |= AT91_SMC_EXNWMODE_READY;
> >
> > - /* add data float cycles if needed */
> > - if (data_float)
> > - mode |= AT91_SMC_TDF_(data_float);
> > + /* add data ofloat cycles if needed */
>
>
> Typo: ofloat -> float?
>
> > + smc.tdf_cycles = data_float;
> >
> > - at91_sys_write(AT91_SMC_MODE(chipselect), mode);
> > + /* write SMC Setup Register */
>
>
> This comment is no longer correct.
>
> > + smc.nrd_setup = setup;
> > + smc.nwe_setup = smc.nrd_setup;
> > + smc.ncs_read_setup = 0;
> > + smc.ncs_write_setup = smc.ncs_read_setup;
> > + /* write SMC Pulse Register */
>
>
> Nor is this one.
>
> > + smc.nrd_pulse = pulse;
> > + smc.nwe_pulse = smc.nrd_pulse;
> > + smc.ncs_read_pulse = cycle;
> > + smc.ncs_write_pulse = smc.ncs_read_pulse;
> > + /* write SMC Cycle Register */
>
>
> or this one.
>
> > + smc.read_cycle = cycle;
> > + smc.write_cycle = smc.read_cycle;
> >
> > - /* setup timings in SMC */
> > - at91_sys_write(AT91_SMC_SETUP(chipselect), AT91_SMC_NWESETUP_(setup) |
> > - AT91_SMC_NCS_WRSETUP_(0) |
> > - AT91_SMC_NRDSETUP_(setup) |
> > - AT91_SMC_NCS_RDSETUP_(0));
> > - at91_sys_write(AT91_SMC_PULSE(chipselect), AT91_SMC_NWEPULSE_(pulse) |
> > - AT91_SMC_NCS_WRPULSE_(cycle) |
> > - AT91_SMC_NRDPULSE_(pulse) |
> > - AT91_SMC_NCS_RDPULSE_(cycle));
> > - at91_sys_write(AT91_SMC_CYCLE(chipselect), AT91_SMC_NWECYCLE_(cycle) |
> > - AT91_SMC_NRDCYCLE_(cycle));
> > + sam9_smc_configure(0, chipselect, &smc);
>
>
> Check return value?
set_timings don't care about it
so no need
Best Regards,
J.
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
