[patch 0/3] imx: Efika: Add pata device

[patch 0/3] imx: Efika: Add pata device

[Arnaud Patard (Rtp)]

This patchset is adding ressources, clock and device declarations for the
pata device available on imx51. Last patch enables pata on efika platforms.
To be complete, this patchset needs a driver, which is about to be sent for
merge to ata developers.

v2: Created a imx51_add_mxc_pata() as suggested
v3: Fix naming

Arnaud

[patch 1/3] imx51: add pata device

[Arnaud Patard (Rtp)]

An embedded and charset-unspecified text was scrubbed...
Name: efika_pata_add_dev.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110724/fb664ab8/attachment.ksh>

[patch 2/3] imx51: add pata clock

[Arnaud Patard (Rtp)]

An embedded and charset-unspecified text was scrubbed...
Name: efika_pata_add_clock.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110724/90469113/attachment.ksh>

[patch 2/3] imx51: add pata clock

[Sascha Hauer]

On Sun, Jul 24, 2011 at 08:39:04PM +0200, Arnaud Patard wrote:
> Define the clock used by the imx51 pata device.
> 
> Signed-off-by: Arnaud Patard <arnaud.patard@rtp-net.org>
> 
> 
> Index: linux-2.6-submit/arch/arm/mach-mx5/clock-mx51-mx53.c
> ===================================================================
> --- linux-2.6-submit.orig/arch/arm/mach-mx5/clock-mx51-mx53.c	2011-07-22 23:29:09.000000000 +0200
> +++ linux-2.6-submit/arch/arm/mach-mx5/clock-mx51-mx53.c	2011-07-24 20:35:17.000000000 +0200
> @@ -1414,6 +1414,10 @@ DEFINE_CLOCK(ipu_di0_clk, 0, MXC_CCM_CCG
>  DEFINE_CLOCK(ipu_di1_clk, 0, MXC_CCM_CCGR6, MXC_CCM_CCGRx_CG6_OFFSET,
>  		NULL, NULL, &pll3_sw_clk, NULL);
>  
> +/* PATA */
> +DEFINE_CLOCK(pata_clk, 0, MXC_CCM_CCGR4, MXC_CCM_CCGRx_CG0_OFFSET,
> +		NULL, NULL, &ipg_clk, &spba_clk);
> +
>  #define _REGISTER_CLOCK(d, n, c) \
>         { \
>  		.dev_id = d, \
> @@ -1466,6 +1470,7 @@ static struct clk_lookup mx51_lookups[]
>  	_REGISTER_CLOCK("imx-ipuv3", "di0", ipu_di0_clk)
>  	_REGISTER_CLOCK("imx-ipuv3", "di1", ipu_di1_clk)
>  	_REGISTER_CLOCK(NULL, "gpc_dvfs", gpc_dvfs_clk)
> +	_REGISTER_CLOCK("imx-pata", NULL, pata_clk)

First argument to _REGISTER_CLOCK must match the device name, but the
driver you posted uses pata_imx as device name. The second argument
which you correctly set to NULL here is an additional identifier which
must match if it exists. There is only one clock in the pata driver, so
you should use clk_get(&pdev->dev, NULL) in you driver.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[patch 3/3] imx: efika: Enable pata.

[Arnaud Patard (Rtp)]

An embedded and charset-unspecified text was scrubbed...
Name: efika_pata_enable_pata.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110724/a4f92746/attachment.ksh>

[patch 0/3] imx: Efika: Add pata device

[Arnaud Patard (Rtp)]

This patchset is adding ressources, clock and device declarations for the
pata device available on imx51. Last patch enables pata on efika platforms.
To be complete, this patchset needs a driver, which is about to be sent for
merge to ata developers.

v2: Created a imx51_add_mxc_pata() as suggested
v3: Fix naming
v4: Change clock name to pata_imx

Arnaud

[patch 1/3] imx51: add pata device

[Arnaud Patard (Rtp)]

An embedded and charset-unspecified text was scrubbed...
Name: efika_pata_add_dev.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110726/7c5eda16/attachment.ksh>

[patch 1/3] imx51: add pata device

[Uwe Kleine-König]

On Tue, Jul 26, 2011 at 04:54:25PM +0200, Arnaud Patard wrote:
> Declare the pata device found on mx51
> 
> Signed-off-by: Arnaud Patard <arnaud.patard@rtp-net.org>
> Index: linux-2.6-submit/arch/arm/mach-mx5/devices-imx51.h
> ===================================================================
> --- linux-2.6-submit.orig/arch/arm/mach-mx5/devices-imx51.h	2011-07-22 23:29:13.000000000 +0200
> +++ linux-2.6-submit/arch/arm/mach-mx5/devices-imx51.h	2011-07-24 20:35:30.000000000 +0200
> @@ -52,3 +52,7 @@ extern const struct imx_mxc_pwm_data imx
>  extern const struct imx_imx_keypad_data imx51_imx_keypad_data;
>  #define imx51_add_imx_keypad(pdata)	\
>  	imx_add_imx_keypad(&imx51_imx_keypad_data, pdata)
> +
> +extern const struct imx_pata_imx_data imx51_pata_imx_data;
> +#define imx51_add_pata_imx() \
> +	imx_add_pata_imx(&imx51_pata_imx_data)
> Index: linux-2.6-submit/arch/arm/plat-mxc/devices/Kconfig
> ===================================================================
> --- linux-2.6-submit.orig/arch/arm/plat-mxc/devices/Kconfig	2011-07-22 23:29:13.000000000 +0200
> +++ linux-2.6-submit/arch/arm/plat-mxc/devices/Kconfig	2011-07-24 20:35:30.000000000 +0200
> @@ -31,6 +31,9 @@ config IMX_HAVE_PLATFORM_IMX_I2C
>  config IMX_HAVE_PLATFORM_IMX_KEYPAD
>  	bool
>  
> +config IMX_HAVE_PLATFORM_PATA_IMX
> +	bool
> +
this breaks the alphabetic ordering in this file.
arch/arm/plat-mxc/devices/Makefile is affected, too.

Other than that I don't see any issues any more. Sascha, maybe you can
fix that up while committing (assuming nobody has more objections)?
Arnaud told me on irc that he is occupied and cannot follow up soon.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[patch 0/3] imx: Efika: Add pata device

[Arnaud Patard (Rtp)]

This patchset is adding ressources, clock and device declarations for the
pata device available on imx51. Last patch enables pata on efika platforms.
To be complete, this patchset needs a driver, which is about to be sent for
merge to ata developers.

v2: created a imx51_add_mxc_pata() as suggested

Arnaud

[patch 1/3] imx51: add pata device

[Arnaud Patard (Rtp)]

An embedded and charset-unspecified text was scrubbed...
Name: efika_pata_add_dev.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110722/be54f3a6/attachment.ksh>

[patch 1/3] imx51: add pata device

[Fabio Estevam]

On Fri, Jul 22, 2011 at 7:37 AM, Arnaud Patard
<arnaud.patard@rtp-net.org> wrote:
...
> +#define imx_mxc_pata_data_entry_single(soc) ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> + ? ? ? { ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
> + ? ? ? ? ? ? ? .iobase = soc ## _ATA_BASE_ADDR, ? ? ? ? ? ? ? ? ? ? ? ?\
> + ? ? ? ? ? ? ? .irq = soc ## _MXC_INT_ATA, ? ? ? ? ? ? ? ? ? ? ? ? ? ? \

Why not imx_pata_data_entry_single instead?

 "imx_mxc" looks redundant.

Regards,

Fabio Estevam

[patch 1/3] imx51: add pata device

[Uwe Kleine-König]

On Fri, Jul 22, 2011 at 12:14:15PM -0300, Fabio Estevam wrote:
> On Fri, Jul 22, 2011 at 7:37 AM, Arnaud Patard
> <arnaud.patard@rtp-net.org> wrote:
> ...
> > +#define imx_mxc_pata_data_entry_single(soc) ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> > + ? ? ? { ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
> > + ? ? ? ? ? ? ? .iobase = soc ## _ATA_BASE_ADDR, ? ? ? ? ? ? ? ? ? ? ? ?\
> > + ? ? ? ? ? ? ? .irq = soc ## _MXC_INT_ATA, ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
> 
> Why not imx_pata_data_entry_single instead?
> 
>  "imx_mxc" looks redundant.
imx is the platform prefix, mxc_pata the device's name. So IMHO the name
is right and consistent with the other device registration stuff.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[patch 1/3] imx51: add pata device

[Arnaud Patard (Rtp)]

Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de> writes:

> On Fri, Jul 22, 2011 at 12:14:15PM -0300, Fabio Estevam wrote:
>> On Fri, Jul 22, 2011 at 7:37 AM, Arnaud Patard
>> <arnaud.patard@rtp-net.org> wrote:
>> ...
>> > +#define imx_mxc_pata_data_entry_single(soc) ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
>> > + ? ? ? { ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
>> > + ? ? ? ? ? ? ? .iobase = soc ## _ATA_BASE_ADDR, ? ? ? ? ? ? ? ? ? ? ? ?\
>> > + ? ? ? ? ? ? ? .irq = soc ## _MXC_INT_ATA, ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
>> 
>> Why not imx_pata_data_entry_single instead?
>> 
>>  "imx_mxc" looks redundant.
> imx is the platform prefix, mxc_pata the device's name. So IMHO the name
> is right and consistent with the other device registration stuff.

[patch 1/3] imx51: add pata device

[Uwe Kleine-König]

On Fri, Jul 22, 2011 at 10:28:49PM +0200, Arnaud Patard wrote:
> Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de> writes:
> 
> > On Fri, Jul 22, 2011 at 12:14:15PM -0300, Fabio Estevam wrote:
> >> On Fri, Jul 22, 2011 at 7:37 AM, Arnaud Patard
> >> <arnaud.patard@rtp-net.org> wrote:
> >> ...
> >> > +#define imx_mxc_pata_data_entry_single(soc) ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> >> > + ? ? ? { ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
> >> > + ? ? ? ? ? ? ? .iobase = soc ## _ATA_BASE_ADDR, ? ? ? ? ? ? ? ? ? ? ? ?\
> >> > + ? ? ? ? ? ? ? .irq = soc ## _MXC_INT_ATA, ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
> >> 
> >> Why not imx_pata_data_entry_single instead?
> >> 
> >>  "imx_mxc" looks redundant.
> > imx is the platform prefix, mxc_pata the device's name. So IMHO the name
> > is right and consistent with the other device registration stuff.
> 
> From looking at devices-imx51.h, I don't see no obvious consistant
> naming scheme :
> 
> extern const struct imx_fec_data imx51_fec_data;
driver name: fec
> ...
> extern const struct imx_imx_i2c_data imx51_imx_i2c_data[];
driver name: imx_i2c
> ...
> extern const struct imx_mxc_nand_data imx51_mxc_nand_data;
driver name: mxc_nand
> ...
> extern const struct imx_spi_imx_data imx51_cspi_data;
driver name: spi_imx

So the driver names are not consistent, ok. But the names used in
devices-imx51.h match these. It's always imx_${drivername}_data.

> So, I can use either imx_pata_data or keep imx_mxc_pata_data or
> imx_imx_pata_data or imx_pata_imx_data. What do you both prefer ?
So if your driver is called "mxc_pata", please use
imx_mxc_pata_data_entry_single.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[patch 1/3] imx51: add pata device

[Uwe Kleine-König]

Hello Arnaud,

On Fri, Jul 22, 2011 at 12:37:11PM +0200, Arnaud Patard wrote:
> +extern const struct imx_mxc_pata_data im51_mxc_pata_data;
imx51 please

Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[patch 0/3] imx: Efika: Add pata device

[Arnaud Patard (Rtp)]

This patchset is adding ressources, clock and device declarations for the
pata device available on imx51. Last patch enables pata on efika platforms.
To be complete, this patchset needs a driver, which is about to be sent for
merge to ata developers.

Arnaud

[patch 1/3] imx51: add pata device

[Arnaud Patard (Rtp)]

An embedded and charset-unspecified text was scrubbed...
Name: efika_pata_add_dev.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110716/2aaa6629/attachment.ksh>

[patch 1/3] imx51: add pata device

[Uwe Kleine-König]

On Sat, Jul 16, 2011 at 10:44:12PM +0200, Arnaud Patard wrote:
> Declare the pata device found on mx51
> 
> Signed-off-by: Arnaud Patard <arnaud.patard@rtp-net.org>
---
> Index: linux-2.6-submit/arch/arm/mach-mx5/devices-imx51.h
> ===================================================================
> --- linux-2.6-submit.orig/arch/arm/mach-mx5/devices-imx51.h	2011-07-16 16:16:40.000000000 +0200
> +++ linux-2.6-submit/arch/arm/mach-mx5/devices-imx51.h	2011-07-16 16:20:33.000000000 +0200
> @@ -52,3 +52,5 @@ extern const struct imx_mxc_pwm_data imx
>  extern const struct imx_imx_keypad_data imx51_imx_keypad_data;
>  #define imx51_add_imx_keypad(pdata)	\
>  	imx_add_imx_keypad(&imx51_imx_keypad_data, pdata)
> +
> +extern const struct imx_mxc_pata_data im51_mxc_pata_data;
imx51 please

> Index: linux-2.6-submit/arch/arm/plat-mxc/devices/Kconfig
> ===================================================================
> --- linux-2.6-submit.orig/arch/arm/plat-mxc/devices/Kconfig	2011-07-16 16:16:40.000000000 +0200
> +++ linux-2.6-submit/arch/arm/plat-mxc/devices/Kconfig	2011-07-16 16:20:08.000000000 +0200
> @@ -31,6 +31,9 @@ config IMX_HAVE_PLATFORM_IMX_I2C
>  config IMX_HAVE_PLATFORM_IMX_KEYPAD
>  	bool
>  
> +config IMX_HAVE_PLATFORM_IMX_PATA
> +	bool
> +
>  config IMX_HAVE_PLATFORM_IMX_SSI
>  	bool
>  
> Index: linux-2.6-submit/arch/arm/plat-mxc/devices/Makefile
> ===================================================================
> --- linux-2.6-submit.orig/arch/arm/plat-mxc/devices/Makefile	2011-07-16 16:16:40.000000000 +0200
> +++ linux-2.6-submit/arch/arm/plat-mxc/devices/Makefile	2011-07-16 16:20:08.000000000 +0200
> @@ -10,6 +10,7 @@ obj-y += platform-imx-dma.o
>  obj-$(CONFIG_IMX_HAVE_PLATFORM_IMX_FB) += platform-imx-fb.o
>  obj-$(CONFIG_IMX_HAVE_PLATFORM_IMX_I2C) += platform-imx-i2c.o
>  obj-$(CONFIG_IMX_HAVE_PLATFORM_IMX_KEYPAD) += platform-imx-keypad.o
> +obj-$(CONFIG_IMX_HAVE_PLATFORM_IMX_PATA) += platform-imx-pata.o
>  obj-$(CONFIG_IMX_HAVE_PLATFORM_IMX_SSI) += platform-imx-ssi.o
>  obj-$(CONFIG_IMX_HAVE_PLATFORM_IMX_UART) += platform-imx-uart.o
>  obj-$(CONFIG_IMX_HAVE_PLATFORM_IMX_UDC) += platform-imx_udc.o
> Index: linux-2.6-submit/arch/arm/plat-mxc/devices/platform-imx-pata.c
> ===================================================================
> --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> +++ linux-2.6-submit/arch/arm/plat-mxc/devices/platform-imx-pata.c	2011-07-16 16:20:08.000000000 +0200
> @@ -0,0 +1,38 @@
> +/*
> + * This program is free software; you can redistribute it and/or modify it under
> + * the terms of the GNU General Public License version 2 as published by the
> + * Free Software Foundation.
> + */
> +#include <mach/hardware.h>
> +#include <mach/devices-common.h>
> +
> +#define imx_mxc_pata_data_entry_single(soc)				\
> +	{								\
> +		.iobase = soc ## _ATA_BASE_ADDR,			\
> +		.irq = soc ## _MXC_INT_ATA,				\
> +	}
> +
> +#ifdef CONFIG_SOC_IMX51
> +const struct imx_mxc_pata_data im51_mxc_pata_data __initconst =
> +	imx_mxc_pata_data_entry_single(MX51);
obviously here, too

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |