Re: [PATCH 2/3 v2] ide: add at91_ide driver

[Stanislaw Gruszka <stf_xl@wp.pl>]

Sunday 15 February 2009 18:29:13 Sergei Shtylyov napisał(a):

> > +#define REGS_SIZE	16
> 
>     Why? There are only 8 registers in each block.

I did such because pata_at32 have 16, but it should be 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)
> > +
> > +#define leave_16bit(cs, mode) at91_sys_write(AT91_SMC_MODE(cs), mode);
> 
>    Inline functions are preferrable.

I know. Here is semantics like in spin_lock_irqsave() so I think this
could stay in #define way.

> > +static const struct ide_port_info at91_ide_port_info __initdata = {
> > +	.port_ops	= &at91_ide_port_ops,
> > +	.tp_ops		= &at91_ide_tp_ops,
> > +	.host_flags 	= IDE_HFLAG_MMIO | IDE_HFLAG_NO_DMA | IDE_HFLAG_SINGLE |
> > +			  IDE_HFLAG_NO_IO_32BIT | IDE_HFLAG_UNMASK_IRQS,
> > +	.pio_mask 	= ATA_PIO5,
> 
>     You *can* use ATA_PIO6 here.  It just won't be actually selected by the 
> IDE core -- until CF specific mode support patch gets merged (I'm expecting 
> this to happen in the 2.6.30-rc1 merge window).

With change to ATA_PIO6 command "hdparm -p 6 /dev/hda" call function
at91_ide_set_pio_mode() and OOPS happens because ide_timing_find_mode()
returns NULL. This could be a separate patch after PIO6 changes merged.

Cheers
Stanislaw Gruszka