Re: [PATCH 3/3 v2] AT91: initialize Compact Flash on AT91SAM9263 cpu

[Stanislaw Gruszka <stf_xl@wp.pl>]

Friday 06 February 2009 13:03:54 Sergei Shtylyov napisał(a):
> > +/* --------------------------------------------------------------------
> > + *  Compact Flash (PCMCIA or IDE)
> > + * -------------------------------------------------------------------- */
> > +
> > +#if defined(CONFIG_AT91_CF) || defined(CONFIG_AT91_CF_MODULE) || \
> > +    defined(CONFIG_BLK_DEV_IDE_AT91) || defined(CONFIG_BLK_DEV_IDE_AT91_MODULE)
> >   
> 
>    Ugh... what are trying to achieve with this, marginal memory save?
Some memory save. There are users which count every byte. 

> > +	cf_resources[0].start = addr_space;
> > +	cf_resources[0].end = addr_space + SZ_256M - 1;
> >   
> 
>    In the datasheet the CF mode address space is just 0x1000000, i.e. 16 
> MB, not 256. Moreover, address lines above A22 are not connected to CF 
> at all.
We take here all particular chip select address space 
CS5: 0x50000000 - 0x5fffffff
CS6: 0x60000000 - 0x6fffffff 

> > +	if (data->det_pin) {
> > +		/* check if device is present */
> > +		at91_set_gpio_input(data->det_pin, 1);
> > +		at91_set_deglitch(data->det_pin, 1);
> > +		if (at91_get_gpio_value(data->det_pin) != 0) {
> > +			printk(KERN_ERR "AT91: no CF card detected\n");
> > +			return;
> > +		}
> > +	}
> >   
> 
>    Bart, I'm not sure: don't we support warm-plug now? Or it won't avail 
> us with CF?
Uhh, this code is wrong at least for PCMCIA as we hotplug there. I don't think
hotplug is supported for ATA devices.

> > +	if (data->rst_pin) {
> > +		/* reset the card */
> > +		int reset = (data->flags & AT91_CF_TRUE_IDE) ? 0 : 1;
> > +
> > +		at91_set_gpio_output(data->rst_pin, reset);
> > +		udelay(25); /* taken from ATA docs, CFA not specify it ? */
> > +		at91_set_gpio_output(data->rst_pin, !reset);
> > +	}
> >   
> 
>    Is there really a need to hard reset CF on every reboot?
I think this have sense for ATA devices.
 
> > +	if (data->vcc_pin)
> > +		/* initially off */
> > +		at91_set_gpio_output(data->vcc_pin, 0);
> >   
> 
>    Why? Moreover, resetting CF without power applied (or cutting off 
> power afterwards) just doesn't make sense...
Right.

Stanislaw Gruszka