[PATCH 3/3] AT91: initialize IDE driver on AT91SAM9263 cpu

[PATCH 3/3] AT91: initialize IDE driver on AT91SAM9263 cpu

[Stanislaw Gruszka]

Signed-off-by: Stanislaw Gruszka <stf_xl@wp.pl>

---
 arch/arm/mach-at91/at91sam9263_devices.c |   97 ++++++++++++++++++++++++++++++
 1 files changed, 97 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-at91/at91sam9263_devices.c b/arch/arm/mach-at91/at91sam9263_devices.c
index b753cb8..503651a 100644
--- a/arch/arm/mach-at91/at91sam9263_devices.c
+++ b/arch/arm/mach-at91/at91sam9263_devices.c
@@ -347,6 +347,103 @@ void __init at91_add_device_mmc(short mmc_id, struct at91_mmc_data *data)
 void __init at91_add_device_mmc(short mmc_id, struct at91_mmc_data *data) {}
 #endif
 
+/* --------------------------------------------------------------------
+ *  IDE
+ * -------------------------------------------------------------------- */
+
+#if defined(CONFIG_BLK_DEV_IDE_AT91) || defined(CONFIG_BLK_DEV_IDE_AT91_MODULE)
+
+/* Proper CS address space will be added */
+#define AT91_IDE_TASK_FILE	0x00c00000
+#define AT91_IDE_CTRL_REG	0x00e00000
+
+static struct resource ide_resources[] = {
+	[0] = {
+		.start	= AT91_IDE_TASK_FILE,
+		.end	= AT91_IDE_TASK_FILE + 16 - 1,
+		.flags	= IORESOURCE_MEM,
+	},
+	[1] = {
+		.start	= AT91_IDE_CTRL_REG,
+		.end	= AT91_IDE_CTRL_REG + 16 - 1,
+		.flags	= IORESOURCE_MEM,
+	},
+};
+
+static struct at91_ide_data ide_data;
+
+static struct platform_device at91_ide_device = {
+	.name	= "at91_ide",
+	.id 	= -1,
+	.dev	= {
+		.platform_data = &ide_data,
+	},
+	.resource = ide_resources,
+	.num_resources = ARRAY_SIZE(ide_resources),
+};
+
+void __init at91_add_device_ide(struct at91_ide_data *data)
+{
+	unsigned long ebi0_csa, addr_space;
+	u8 chipselect = data->chipselect;
+
+	/* enable PIO controlled pins, inputs with pull ups */
+	if (data->rst_pin)
+		at91_set_gpio_output(data->rst_pin, 0); /* reset card */
+
+	at91_set_gpio_input(data->irq_pin, 1);
+	at91_set_deglitch(data->irq_pin, 1);
+
+	if (data->det_pin) {
+		at91_set_gpio_input(data->det_pin, 1);
+		at91_set_deglitch(data->det_pin, 1);
+	}
+
+	/* enable EBI SMC controlled pins */
+	at91_set_A_periph(AT91_PIN_PD5, 1);  /* NWAIT */
+	at91_set_A_periph(AT91_PIN_PD8, 0);  /* CFCE1 */
+	at91_set_A_periph(AT91_PIN_PD9, 0);  /* CFCE2 */
+	at91_set_A_periph(AT91_PIN_PD14, 0); /* CFNRW */
+
+	/* assign CS4/5 to SMC with Compact Flash logic support
+	 * and fix resources addresses */
+	ebi0_csa = at91_sys_read(AT91_MATRIX_EBI0CSA);
+	switch (chipselect) {
+	case 4:
+		at91_set_A_periph(AT91_PIN_PD6, 0);  /* EBI0_NCS4/CFCS0 */
+		ebi0_csa |= AT91_MATRIX_EBI0_CS4A_SMC_CF1;
+		addr_space = AT91_CHIPSELECT_4;
+		break;
+	case 5:
+		at91_set_A_periph(AT91_PIN_PD7, 0);  /* EBI0_NCS5/CFCS1 */
+		ebi0_csa |= AT91_MATRIX_EBI0_CS5A_SMC_CF2;
+		addr_space = AT91_CHIPSELECT_5;
+		break;
+	default:
+		printk(KERN_ERR "at91_ide: bad chip select %u\n", chipselect);
+		return;
+	}
+	at91_sys_write(AT91_MATRIX_EBI0CSA, ebi0_csa);
+	ide_resources[0].start += addr_space;
+	ide_resources[0].end += addr_space;
+	ide_resources[1].start += addr_space;
+	ide_resources[1].end += addr_space;
+
+	/* turn on the card if reset pin is GPIO */
+	if (data->rst_pin)
+		at91_set_gpio_value(data->rst_pin, 1);
+
+	if (data->det_pin && at91_get_gpio_value(data->det_pin) != 0) {
+		printk(KERN_ERR "at91_ide: no Compact Flash card detected\n");
+		return;
+	}
+
+	ide_data = *data;
+	platform_device_register(&at91_ide_device);
+}
+#else
+void __init at91_add_device_ide(struct at91_ide_data *data) {}
+#endif
 
 /* --------------------------------------------------------------------
  *  NAND / SmartMedia
-- 
1.5.2.5

Re: [PATCH 3/3] AT91: initialize IDE driver on AT91SAM9263 cpu

[Andrew Victor]

hi Stanislaw,

> +void __init at91_add_device_ide(struct at91_ide_data *data)
> +{

This platform initialization code is virtually identical to the
at91_cf version submitted by Uros Bizjak, albeit for the SAM9260.
Similarly the "at91_cf_data" and "at91_ide_data" structs.

Wouldn't it be better to have a single CF initialization function, and
you specify whether you want "plain CF" mode or True-IDE mode?
You could even pass the same platform-resources structure (ie,
base-address of CF region), and have at91_ide setup the location of
the task-file and control registers from that base address.


Regards,
  Andrew Victor

Re: [PATCH 3/3] AT91: initialize IDE driver on AT91SAM9263 cpu

[Stanislaw Gruszka]

Tuesday 03 February 2009 21:27:04 Andrew Victor napisał(a):
> hi Stanislaw,
Hi.

> > +void __init at91_add_device_ide(struct at91_ide_data *data)
> > +{
> 
> This platform initialization code is virtually identical to the
> at91_cf version submitted by Uros Bizjak, albeit for the SAM9260.
> Similarly the "at91_cf_data" and "at91_ide_data" structs.
> 
> Wouldn't it be better to have a single CF initialization function, and
> you specify whether you want "plain CF" mode or True-IDE mode?
> You could even pass the same platform-resources structure (ie,
> base-address of CF region), and have at91_ide setup the location of
> the task-file and control registers from that base address.
Yes, this can be merged together. Ronetix already did a SAM9263 board
and linux patch with "full" CF device.

http://download.ronetix.info/sk-eb926x/linux/kernel/2.6.28/003_linux-2.6.28-at91-ronetix-12012009.patch

There are some difference, but I think CF stuff can have one initialization
function. Could this be a separate patch in later date? I would like marge
at91_ide into mainline without changes if there will be no objections.

Cheers
Stanislaw Gruszka

Re: [PATCH 3/3] AT91: initialize IDE driver on AT91SAM9263 cpu

[Sergei Shtylyov]

Hello.

Stanislaw Gruszka wrote:

>>> +void __init at91_add_device_ide(struct at91_ide_data *data)
>>> +{
>>>       
>> This platform initialization code is virtually identical to the
>> at91_cf version submitted by Uros Bizjak, albeit for the SAM9260.
>> Similarly the "at91_cf_data" and "at91_ide_data" structs.
>>
>> Wouldn't it be better to have a single CF initialization function, and
>> you specify whether you want "plain CF" mode or True-IDE mode?
>> You could even pass the same platform-resources structure (ie,
>> base-address of CF region), and have at91_ide setup the location of
>> the task-file and control registers from that base address.
>>     
> Yes, this can be merged together. Ronetix already did a SAM9263 board
> and linux patch with "full" CF device.
>
> http://download.ronetix.info/sk-eb926x/linux/kernel/2.6.28/003_linux-2.6.28-at91-ronetix-12012009.patch
>   

   This patch is clearly incomplete because drivers/pcmcia/at91_cf.c 
does accesses AT91RM9200 specific SMC register, but the patch does 
nothing about making this work on AT91SAM9. Though it indeed seems that 
AT91SAM9 can be supported by that driver with minimum change -- perhaps 
by using cpi_is_*()...
   And WTF is that: sound/arm/at91-ac97.c.rej? :-/

> There are some difference,

   The CF memory mappings seem compatible b/w AT91SAM9 and AT91RM9200, 
except the latter doesn't support True IDE mode.

> but I think CF stuff can have one initialization function. Could this be a separate patch in later date? I would like marge
> at91_ide into mainline without changes if there will be no objections.
>   

   I would still like 2 resources merged into 1 -- however, possibly 
from the offset 0x00c00000, i.e. not including PCMCIA compatible memory 
space.

> Cheers
> Stanislaw Gruszka
>   

MBR, Sergei

Re: [PATCH 3/3] AT91: initialize IDE driver on AT91SAM9263 cpu

[Andrew Victor]

hi,

>> http://download.ronetix.info/sk-eb926x/linux/kernel/2.6.28/003_linux-2.6.28-at91-ronetix-12012009.patch
>
>  This patch is clearly incomplete because drivers/pcmcia/at91_cf.c does
> accesses AT91RM9200 specific SMC register, but the patch does nothing about
> making this work on AT91SAM9. Though it indeed seems that AT91SAM9 can be
> supported by that driver with minimum change -- perhaps by using
> cpi_is_*()...

Rather see latest patch on http://maxim.org.za/at91_26.html.

The cpu_is_XX() is for run-time selection (and is the preferred mechanism).
Unfortunately the SMC registers are significantly different on the
SAM9 and RM9200, so we need to use #ifdefs for compile-time selection.


Regards,
  Andrew Victor

Re: [PATCH 3/3] AT91: initialize IDE driver on AT91SAM9263 cpu

[Sergei Shtylyov]

Andrew Victor wrote:

>>>http://download.ronetix.info/sk-eb926x/linux/kernel/2.6.28/003_linux-2.6.28-at91-ronetix-12012009.patch

>> This patch is clearly incomplete because drivers/pcmcia/at91_cf.c does
>>accesses AT91RM9200 specific SMC register, but the patch does nothing about
>>making this work on AT91SAM9. Though it indeed seems that AT91SAM9 can be
>>supported by that driver with minimum change -- perhaps by using
>>cpi_is_*()...

> Rather see latest patch on http://maxim.org.za/at91_26.html.

> The cpu_is_XX() is for run-time selection (and is the preferred mechanism).
> Unfortunately the SMC registers are significantly different on the
> SAM9 and RM9200, so we need to use #ifdefs for compile-time selection.

    Ah, seeing the real issue now: different encoding of the DBW fields b/w 
those 2 types of SMCs. Oh, and I've missed the different DBW field positions 
too... That however can be addressed by using the driver local #define', not 
the ones from the headers (which should be conflicting)...

> Regards,
>   Andrew Victor

WBR, Sergei

Re: [PATCH 3/3] AT91: initialize IDE driver on AT91SAM9263 cpu

[Sergei Shtylyov]

Hello. I wrote:

>>>> http://download.ronetix.info/sk-eb926x/linux/kernel/2.6.28/003_linux-2.6.28-at91-ronetix-12012009.patch 
>>>>
>
>>> This patch is clearly incomplete because drivers/pcmcia/at91_cf.c does
>>> accesses AT91RM9200 specific SMC register, but the patch does 
>>> nothing about
>>> making this work on AT91SAM9. Though it indeed seems that AT91SAM9 
>>> can be
>>> supported by that driver with minimum change -- perhaps by using
>>> cpi_is_*()...
>
>> Rather see latest patch on http://maxim.org.za/at91_26.html.
>
>> The cpu_is_XX() is for run-time selection (and is the preferred 
>> mechanism).
>> Unfortunately the SMC registers are significantly different on the
>> SAM9 and RM9200, so we need to use #ifdefs for compile-time selection.
>
>    Ah, seeing the real issue now: different encoding of the DBW fields 
> b/w those 2 types of SMCs. Oh, and I've missed the different DBW field 
> positions too... That however can be addressed by using the driver 
> local #define', not the ones from the headers (which should be 
> conflicting)...

   Well, probably not the best approach either. Since the SMC specific 
in contained in a single funtion in this case, anothe rapproach can be 
used: implement the function in the platfrom code and pass the pointer 
to it via the platfrom data. This approach is used when the driver needs 
to some board specific hook, e.g. some MMC drivers use this approach for 
checking the write protect signal wired to some GPIO in a board specific 
way).

>>   Andrew Victor
> Regards,

WBR, Sergei

Re: [PATCH 3/3] AT91: initialize IDE driver on AT91SAM9263 cpu

[Stanislaw Gruszka]

Tuesday 03 February 2009 21:27:04 Andrew Victor napisał(a):
> hi Stanislaw,
> 
> > +void __init at91_add_device_ide(struct at91_ide_data *data)
> > +{
> 
> This platform initialization code is virtually identical to the
> at91_cf version submitted by Uros Bizjak, albeit for the SAM9260.
> Similarly the "at91_cf_data" and "at91_ide_data" structs.
Shouldn't be sam9_smc_config stuff be in board file not in cpu file, 
in place where you know what is the clock frequency. Could you
assume cpu is clocked with some particular freqency?

Cheers
Stanislaw Gruszka

Re: [PATCH 3/3] AT91: initialize IDE driver on AT91SAM9263 cpu

[Sergei Shtylyov]

Hello.

Stanislaw Gruszka wrote:
> Signed-off-by: Stanislaw Gruszka <stf_xl@wp.pl>
>   
[...]
> diff --git a/arch/arm/mach-at91/at91sam9263_devices.c b/arch/arm/mach-at91/at91sam9263_devices.c
> index b753cb8..503651a 100644
> --- a/arch/arm/mach-at91/at91sam9263_devices.c
> +++ b/arch/arm/mach-at91/at91sam9263_devices.c
> @@ -347,6 +347,103 @@ void __init at91_add_device_mmc(short mmc_id, struct at91_mmc_data *data)
>  void __init at91_add_device_mmc(short mmc_id, struct at91_mmc_data *data) {}
>  #endif
>  
> +/* --------------------------------------------------------------------
> + *  IDE
> + * -------------------------------------------------------------------- */
> +
> +#if defined(CONFIG_BLK_DEV_IDE_AT91) || defined(CONFIG_BLK_DEV_IDE_AT91_MODULE)
> +
> +/* Proper CS address space will be added */
> +#define AT91_IDE_TASK_FILE	0x00c00000
> +#define AT91_IDE_CTRL_REG	0x00e00000
>   

   Why are you assuming that these offsets are not board specifec I 
wonder? Theyu clearly depend on how the -CSx were wired on the board -- 
it's the same board specific data as your IRQ.

> +void __init at91_add_device_ide(struct at91_ide_data *data)
> +{
>   
[...]
> +	/* assign CS4/5 to SMC with Compact Flash logic support
> +	 * and fix resources addresses */
>   

   It again seems like board specific configuration...

> +	ebi0_csa = at91_sys_read(AT91_MATRIX_EBI0CSA);
> +	switch (chipselect) {
> +	case 4:
> +		at91_set_A_periph(AT91_PIN_PD6, 0);  /* EBI0_NCS4/CFCS0 */
> +		ebi0_csa |= AT91_MATRIX_EBI0_CS4A_SMC_CF1;
> +		addr_space = AT91_CHIPSELECT_4;
> +		break;
> +	case 5:
> +		at91_set_A_periph(AT91_PIN_PD7, 0);  /* EBI0_NCS5/CFCS1 */
> +		ebi0_csa |= AT91_MATRIX_EBI0_CS5A_SMC_CF2;
> +		addr_space = AT91_CHIPSELECT_5;
> +		break;
> +	default:
> +		printk(KERN_ERR "at91_ide: bad chip select %u\n", chipselect);
>   
> +		return;
>   

  Why do you consider it bad? Is there some SMC limitation or what?

MBR, Sergei

Re: [PATCH 3/3] AT91: initialize IDE driver on AT91SAM9263 cpu

[Stanislaw Gruszka]

Wednesday 04 February 2009 13:18:05 Sergei Shtylyov napisał(a):
> Hello.
Hi

> Stanislaw Gruszka wrote:
> > Signed-off-by: Stanislaw Gruszka <stf_xl@wp.pl>
> >   
> [...]
> > diff --git a/arch/arm/mach-at91/at91sam9263_devices.c b/arch/arm/mach-at91/at91sam9263_devices.c
> > index b753cb8..503651a 100644
> > --- a/arch/arm/mach-at91/at91sam9263_devices.c
> > +++ b/arch/arm/mach-at91/at91sam9263_devices.c
> > @@ -347,6 +347,103 @@ void __init at91_add_device_mmc(short mmc_id, struct at91_mmc_data *data)
> >  void __init at91_add_device_mmc(short mmc_id, struct at91_mmc_data *data) {}
> >  #endif
> >  
> > +/* --------------------------------------------------------------------
> > + *  IDE
> > + * -------------------------------------------------------------------- */
> > +
> > +#if defined(CONFIG_BLK_DEV_IDE_AT91) || defined(CONFIG_BLK_DEV_IDE_AT91_MODULE)
> > +
> > +/* Proper CS address space will be added */
> > +#define AT91_IDE_TASK_FILE	0x00c00000
> > +#define AT91_IDE_CTRL_REG	0x00e00000
> >   
> 
>    Why are you assuming that these offsets are not board specifec I 
> wonder? Theyu clearly depend on how the -CSx were wired on the board -- 
> it's the same board specific data as your IRQ.
Are CPU specific. Actually all AT91(except RM9200) use the same offsets,
so could be moved from here to driver code, like in at91_cf driver. In this file
only we will have proper Chip Select address space resource.

> > +void __init at91_add_device_ide(struct at91_ide_data *data)
> > +{
> >   
> [...]
> > +	/* assign CS4/5 to SMC with Compact Flash logic support
> > +	 * and fix resources addresses */
> >   
> 
>    It again seems like board specific configuration...
No, is not board specific.

> > +	ebi0_csa = at91_sys_read(AT91_MATRIX_EBI0CSA);
> > +	switch (chipselect) {
> > +	case 4:
> > +		at91_set_A_periph(AT91_PIN_PD6, 0);  /* EBI0_NCS4/CFCS0 */
> > +		ebi0_csa |= AT91_MATRIX_EBI0_CS4A_SMC_CF1;
> > +		addr_space = AT91_CHIPSELECT_4;
> > +		break;
> > +	case 5:
> > +		at91_set_A_periph(AT91_PIN_PD7, 0);  /* EBI0_NCS5/CFCS1 */
> > +		ebi0_csa |= AT91_MATRIX_EBI0_CS5A_SMC_CF2;
> > +		addr_space = AT91_CHIPSELECT_5;
> > +		break;
> > +	default:
> > +		printk(KERN_ERR "at91_ide: bad chip select %u\n", chipselect);
> >   
> > +		return;
> >   
> 
>   Why do you consider it bad? Is there some SMC limitation or what?
Yes, Compact Flash ("full" and True IDE mode) is supported only on chip select
4 or 5 for AT91SAM9263 (like for all other SAM9s).


Cheers
Stanislaw Gruszka

Re: [PATCH 3/3] AT91: initialize IDE driver on AT91SAM9263 cpu

[Sergei Shtylyov]

Stanislaw Gruszka wrote:

>>>Signed-off-by: Stanislaw Gruszka <stf_xl@wp.pl>

>>[...]

>>>diff --git a/arch/arm/mach-at91/at91sam9263_devices.c b/arch/arm/mach-at91/at91sam9263_devices.c
>>>index b753cb8..503651a 100644
>>>--- a/arch/arm/mach-at91/at91sam9263_devices.c
>>>+++ b/arch/arm/mach-at91/at91sam9263_devices.c
>>>@@ -347,6 +347,103 @@ void __init at91_add_device_mmc(short mmc_id, struct at91_mmc_data *data)
>>> void __init at91_add_device_mmc(short mmc_id, struct at91_mmc_data *data) {}
>>> #endif
>>> 
>>>+/* --------------------------------------------------------------------
>>>+ *  IDE
>>>+ * -------------------------------------------------------------------- */
>>>+
>>>+#if defined(CONFIG_BLK_DEV_IDE_AT91) || defined(CONFIG_BLK_DEV_IDE_AT91_MODULE)
>>>+
>>>+/* Proper CS address space will be added */
>>>+#define AT91_IDE_TASK_FILE	0x00c00000
>>>+#define AT91_IDE_CTRL_REG	0x00e00000

>>   Why are you assuming that these offsets are not board specifec I 
>>wonder? Theyu clearly depend on how the -CSx were wired on the board -- 
>>it's the same board specific data as your IRQ.

> Are CPU specific.

    Ah, sorry, I've missed that in the datasheet -- only looked into the SMC
section and that's described in the EBI section...

> Actually all AT91(except RM9200) use the same offsets,
> so could be moved from here to driver code,  like in at91_cf driver.
> In this file only we will have proper Chip Select address space resource.

    Then it indeed makes much sense having a single platfrom device for both 
IDE driver and the possible PCMCIA driver with a single memory resource 
covering the whole CS4/5 range as the drivers will know what subranges they 
want to use.

> Cheers
> Stanislaw Gruszka

WBR, Sergei