* [PATCH 5/7] S3C64XX: Add platform data and driver resources for IDE controller driver.
@ 2009-11-01 4:56 Thomas Abraham
2009-11-01 13:05 ` Ben Dooks
0 siblings, 1 reply; 4+ messages in thread
From: Thomas Abraham @ 2009-11-01 4:56 UTC (permalink / raw)
To: linux-arm-kernel
This patch adds the following for S3C IDE driver.
- IDE plafrom data strucure definition
- IDE driver resources
- IDE controller GPIO setup code
- IDE platform data setup code
- IDE platform device definition
Signed-off-by: Abhilash Kesavan <a.kesavan@samsung.com>
Signed-off-by: Thomas Abraham <thomas.ab@samsung.com>
---
arch/arm/plat-s3c/include/plat/devs.h | 1 +
arch/arm/plat-s3c/include/plat/ide.h | 36 +++++++++++++
arch/arm/plat-s3c64xx/dev-ide.c | 88 +++++++++++++++++++++++++++++++++
3 files changed, 125 insertions(+), 0 deletions(-)
create mode 100644 arch/arm/plat-s3c/include/plat/ide.h
create mode 100644 arch/arm/plat-s3c64xx/dev-ide.c
diff --git a/arch/arm/plat-s3c/include/plat/devs.h b/arch/arm/plat-s3c/include/plat/devs.h
index 0f540ea..4ae0c6a 100644
--- a/arch/arm/plat-s3c/include/plat/devs.h
+++ b/arch/arm/plat-s3c/include/plat/devs.h
@@ -52,6 +52,7 @@ extern struct platform_device s3c_device_nand;
extern struct platform_device s3c_device_usbgadget;
extern struct platform_device s3c_device_usb_hsotg;
+extern struct platform_device s3c_device_cfcon;
/* s3c2440 specific devices */
diff --git a/arch/arm/plat-s3c/include/plat/ide.h b/arch/arm/plat-s3c/include/plat/ide.h
new file mode 100644
index 0000000..3983891
--- /dev/null
+++ b/arch/arm/plat-s3c/include/plat/ide.h
@@ -0,0 +1,36 @@
+/* linux/arch/arm/plat-s3c/include/plat/ide.h
+ *
+ * Copyright (C) 2009 Samsung Electronics
+ * http://samsungsemi.com
+ *
+ * S3C IDE Platform data definitions
+ *
+ * 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.
+*/
+
+#ifndef __PLAT_S3C_IDE_H
+#define __PLAT_S3C_IDE_H __FILE__
+
+/**
+ * struct s3c_ide_platdata - S3C IDE driver platform data.
+ * @setup_gpio: Platform specific ide gpio setup function.
+ *
+ */
+struct s3c_ide_platdata {
+ void (*setup_gpio)(void);
+};
+
+/*
+ * s3c_ide_set_platdata() - Setup the platform specifc data for IDE driver.
+ * @pdata: Platform data for IDE driver.
+ */
+extern void s3c_ide_set_platdata(struct s3c_ide_platdata *pdata);
+
+/*
+ * s3c64xx_ide_setup_gpio() - Platform specific ide gpio setup function.
+ */
+extern void s3c64xx_ide_setup_gpio(void);
+
+#endif
diff --git a/arch/arm/plat-s3c64xx/dev-ide.c b/arch/arm/plat-s3c64xx/dev-ide.c
new file mode 100644
index 0000000..946d323
--- /dev/null
+++ b/arch/arm/plat-s3c64xx/dev-ide.c
@@ -0,0 +1,88 @@
+/* linux/arch/arm/plat-s3c64xx/dev-ide.c
+ *
+ * Copyright (C) 2009 Samsung Electronics
+ * http://samsungsemi.com
+ *
+ * S3C IDE device definition.
+ *
+ * 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 <linux/kernel.h>
+#include <linux/types.h>
+#include <linux/interrupt.h>
+#include <linux/list.h>
+#include <linux/platform_device.h>
+#include <linux/io.h>
+#include <linux/ata.h>
+#include <asm/mach/arch.h>
+#include <asm/mach/irq.h>
+#include <mach/hardware.h>
+#include <mach/map.h>
+#include <plat/regs-clock.h>
+#include <plat/devs.h>
+#include <plat/gpio-cfg.h>
+#include <mach/gpio.h>
+#include <plat/ide.h>
+
+static struct resource s3c_cfcon_resource[] = {
+ [0] = {
+ .start = S3C64XX_PA_CFCON,
+ .end = S3C64XX_PA_CFCON + SZ_1M - 1,
+ .flags = IORESOURCE_MEM,
+ },
+ [1] = {
+ .start = IRQ_CFCON,
+ .end = IRQ_CFCON,
+ .flags = IORESOURCE_IRQ,
+ },
+};
+
+struct platform_device s3c_device_cfcon = {
+ .name = "s3c-ide",
+ .id = 0,
+ .num_resources = ARRAY_SIZE(s3c_cfcon_resource),
+ .resource = s3c_cfcon_resource,
+};
+EXPORT_SYMBOL(s3c_device_cfcon);
+
+void s3c_ide_set_platdata(struct s3c_ide_platdata *pdata)
+{
+ struct s3c_ide_platdata *pd;
+
+ pd = (struct s3c_ide_platdata *)kmemdup(pdata,
+ sizeof(struct s3c_ide_platdata), GFP_KERNEL);
+ if (!pd) {
+ printk(KERN_ERR "%s: no memory for platform data\n", __func__);
+ return;
+ }
+ s3c_device_cfcon.dev.platform_data = pd;
+}
+
+void s3c64xx_ide_setup_gpio(void)
+{
+ u32 reg;
+ u8 i;
+
+ reg = __raw_readl(S3C_MEM_SYS_CFG) & (~0x3f);
+ __raw_writel(reg | 0x4030, S3C_MEM_SYS_CFG);
+
+ s3c_gpio_cfgpin(S3C64XX_GPB(4), S3C_GPIO_SFN(4));
+
+ /* Set XhiDATA[15:0] pins as CF Data[15:0] */
+ for (i = 0; i < 16; i++)
+ s3c_gpio_cfgpin(S3C64XX_GPK(i), S3C_GPIO_SFN(5));
+
+ /* Set XhiADDR[2:0] pins as CF ADDR[2:0] */
+ for (i = 0; i < 3; i++)
+ s3c_gpio_cfgpin(S3C64XX_GPL(i), S3C_GPIO_SFN(6));
+
+ /* Set Xhi ctrl pins as CF ctrl pins(IORDY, IOWR, IORD, CE[0:1]) */
+ s3c_gpio_cfgpin(S3C64XX_GPM(5), S3C_GPIO_SFN(0));
+ for (i = 0; i < 5; i++)
+ s3c_gpio_cfgpin(S3C64XX_GPM(i), S3C_GPIO_SFN(6));
+}
+
+
--
1.5.3.4
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH 5/7] S3C64XX: Add platform data and driver resources for IDE controller driver.
2009-11-01 4:56 [PATCH 5/7] S3C64XX: Add platform data and driver resources for IDE controller driver Thomas Abraham
@ 2009-11-01 13:05 ` Ben Dooks
2009-11-02 6:13 ` Thomas Abraham
2009-11-02 21:13 ` Håkon Løvdal
0 siblings, 2 replies; 4+ messages in thread
From: Ben Dooks @ 2009-11-01 13:05 UTC (permalink / raw)
To: linux-arm-kernel
On Sun, Nov 01, 2009 at 01:56:43PM +0900, Thomas Abraham wrote:
> This patch adds the following for S3C IDE driver.
> - IDE plafrom data strucure definition
> - IDE driver resources
> - IDE controller GPIO setup code
> - IDE platform data setup code
> - IDE platform device definition
>
> Signed-off-by: Abhilash Kesavan <a.kesavan@samsung.com>
> Signed-off-by: Thomas Abraham <thomas.ab@samsung.com>
> ---
> arch/arm/plat-s3c/include/plat/devs.h | 1 +
> arch/arm/plat-s3c/include/plat/ide.h | 36 +++++++++++++
> arch/arm/plat-s3c64xx/dev-ide.c | 88 +++++++++++++++++++++++++++++++++
> 3 files changed, 125 insertions(+), 0 deletions(-)
> create mode 100644 arch/arm/plat-s3c/include/plat/ide.h
> create mode 100644 arch/arm/plat-s3c64xx/dev-ide.c
>
> diff --git a/arch/arm/plat-s3c/include/plat/devs.h b/arch/arm/plat-s3c/include/plat/devs.h
> index 0f540ea..4ae0c6a 100644
> --- a/arch/arm/plat-s3c/include/plat/devs.h
> +++ b/arch/arm/plat-s3c/include/plat/devs.h
> @@ -52,6 +52,7 @@ extern struct platform_device s3c_device_nand;
>
> extern struct platform_device s3c_device_usbgadget;
> extern struct platform_device s3c_device_usb_hsotg;
> +extern struct platform_device s3c_device_cfcon;
>
> /* s3c2440 specific devices */
>
> diff --git a/arch/arm/plat-s3c/include/plat/ide.h b/arch/arm/plat-s3c/include/plat/ide.h
> new file mode 100644
> index 0000000..3983891
> --- /dev/null
> +++ b/arch/arm/plat-s3c/include/plat/ide.h
> @@ -0,0 +1,36 @@
> +/* linux/arch/arm/plat-s3c/include/plat/ide.h
> + *
> + * Copyright (C) 2009 Samsung Electronics
> + * http://samsungsemi.com
> + *
> + * S3C IDE Platform data definitions
> + *
> + * 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.
> +*/
> +
> +#ifndef __PLAT_S3C_IDE_H
> +#define __PLAT_S3C_IDE_H __FILE__
> +
> +/**
> + * struct s3c_ide_platdata - S3C IDE driver platform data.
> + * @setup_gpio: Platform specific ide gpio setup function.
> + *
> + */
> +struct s3c_ide_platdata {
> + void (*setup_gpio)(void);
> +};
> +
> +/*
> + * s3c_ide_set_platdata() - Setup the platform specifc data for IDE driver.
> + * @pdata: Platform data for IDE driver.
> + */
> +extern void s3c_ide_set_platdata(struct s3c_ide_platdata *pdata);
> +
> +/*
> + * s3c64xx_ide_setup_gpio() - Platform specific ide gpio setup function.
> + */
> +extern void s3c64xx_ide_setup_gpio(void);
> +
> +#endif
> diff --git a/arch/arm/plat-s3c64xx/dev-ide.c b/arch/arm/plat-s3c64xx/dev-ide.c
> new file mode 100644
> index 0000000..946d323
> --- /dev/null
> +++ b/arch/arm/plat-s3c64xx/dev-ide.c
> @@ -0,0 +1,88 @@
> +/* linux/arch/arm/plat-s3c64xx/dev-ide.c
> + *
> + * Copyright (C) 2009 Samsung Electronics
> + * http://samsungsemi.com
> + *
> + * S3C IDE device definition.
> + *
> + * 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 <linux/kernel.h>
> +#include <linux/types.h>
> +#include <linux/interrupt.h>
> +#include <linux/list.h>
> +#include <linux/platform_device.h>
> +#include <linux/io.h>
> +#include <linux/ata.h>
> +#include <asm/mach/arch.h>
> +#include <asm/mach/irq.h>
> +#include <mach/hardware.h>
> +#include <mach/map.h>
> +#include <plat/regs-clock.h>
> +#include <plat/devs.h>
> +#include <plat/gpio-cfg.h>
> +#include <mach/gpio.h>
> +#include <plat/ide.h>
> +
> +static struct resource s3c_cfcon_resource[] = {
> + [0] = {
> + .start = S3C64XX_PA_CFCON,
> + .end = S3C64XX_PA_CFCON + SZ_1M - 1,
> + .flags = IORESOURCE_MEM,
> + },
> + [1] = {
> + .start = IRQ_CFCON,
> + .end = IRQ_CFCON,
> + .flags = IORESOURCE_IRQ,
> + },
> +};
> +
> +struct platform_device s3c_device_cfcon = {
> + .name = "s3c-ide",
> + .id = 0,
> + .num_resources = ARRAY_SIZE(s3c_cfcon_resource),
> + .resource = s3c_cfcon_resource,
> +};
> +EXPORT_SYMBOL(s3c_device_cfcon);
> +
> +void s3c_ide_set_platdata(struct s3c_ide_platdata *pdata)
> +{
> + struct s3c_ide_platdata *pd;
> +
> + pd = (struct s3c_ide_platdata *)kmemdup(pdata,
> + sizeof(struct s3c_ide_platdata), GFP_KERNEL);
you do not need pd = (struct s3c_ide_platdata *), the result of kmemdup
is 'void *' and thus can be cast to 'struct s3c_ide_platdata *'. Removing
this will make the code flow better.
> + if (!pd) {
> + printk(KERN_ERR "%s: no memory for platform data\n", __func__);
> + return;
> + }
> + s3c_device_cfcon.dev.platform_data = pd;
doing:
if (!pd)
printk(KERN_ERR "%s: no memory for platform data\n", __func__);
else
s3c_device_cfcon.dev.platform_data = pd;
would be better.
> +}
> +
> +void s3c64xx_ide_setup_gpio(void)
> +{
> + u32 reg;
> + u8 i;
> +
> + reg = __raw_readl(S3C_MEM_SYS_CFG) & (~0x3f);
> + __raw_writel(reg | 0x4030, S3C_MEM_SYS_CFG);
less magic constants please.
> + s3c_gpio_cfgpin(S3C64XX_GPB(4), S3C_GPIO_SFN(4));
> +
> + /* Set XhiDATA[15:0] pins as CF Data[15:0] */
> + for (i = 0; i < 16; i++)
> + s3c_gpio_cfgpin(S3C64XX_GPK(i), S3C_GPIO_SFN(5));
> +
> + /* Set XhiADDR[2:0] pins as CF ADDR[2:0] */
> + for (i = 0; i < 3; i++)
> + s3c_gpio_cfgpin(S3C64XX_GPL(i), S3C_GPIO_SFN(6));
> +
> + /* Set Xhi ctrl pins as CF ctrl pins(IORDY, IOWR, IORD, CE[0:1]) */
> + s3c_gpio_cfgpin(S3C64XX_GPM(5), S3C_GPIO_SFN(0));
> + for (i = 0; i < 5; i++)
> + s3c_gpio_cfgpin(S3C64XX_GPM(i), S3C_GPIO_SFN(6));
> +}
other than the comments, this looks ok.
--
Ben
Q: What's a light-year?
A: One-third less calories than a regular year.
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH 5/7] S3C64XX: Add platform data and driver resources for IDE controller driver.
2009-11-01 13:05 ` Ben Dooks
@ 2009-11-02 6:13 ` Thomas Abraham
2009-11-02 21:13 ` Håkon Løvdal
1 sibling, 0 replies; 4+ messages in thread
From: Thomas Abraham @ 2009-11-02 6:13 UTC (permalink / raw)
To: linux-arm-kernel
On Sun, Nov 1, 2009 at 10:05 PM, Ben Dooks <ben-linux@fluff.org> wrote:
> On Sun, Nov 01, 2009 at 01:56:43PM +0900, Thomas Abraham wrote:
>> This patch adds the following for S3C IDE driver.
>> ? ? ? - IDE plafrom data strucure definition
>> ? ? ? - IDE driver resources
>> ? ? ? - IDE controller GPIO setup code
>> ? ? ? - IDE platform data setup code
>> ? ? ? - IDE platform device definition
>>
>> +void s3c_ide_set_platdata(struct s3c_ide_platdata *pdata)
>> +{
>> + ? ? struct s3c_ide_platdata *pd;
>> +
>> + ? ? pd = (struct s3c_ide_platdata *)kmemdup(pdata,
>> + ? ? ? ? ? ? sizeof(struct s3c_ide_platdata), GFP_KERNEL);
>
> you do not need pd = (struct s3c_ide_platdata *), the result of kmemdup
> is 'void *' and thus can be cast to 'struct s3c_ide_platdata *'. Removing
> this will make the code flow better.
>
> doing:
>
> ? ? ? ?if (!pd)
> ? ? ? ? ? ? ? ?printk(KERN_ERR "%s: no memory for platform data\n", __func__);
> ? ? ? ?else
> ? ? ? ? ? ? ? ?s3c_device_cfcon.dev.platform_data = pd;
>
> would be better.
Ok. I will modify the code.
>
> other than the comments, this looks ok.
>
> --
> Ben
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH 5/7] S3C64XX: Add platform data and driver resources for IDE controller driver.
2009-11-01 13:05 ` Ben Dooks
2009-11-02 6:13 ` Thomas Abraham
@ 2009-11-02 21:13 ` Håkon Løvdal
1 sibling, 0 replies; 4+ messages in thread
From: Håkon Løvdal @ 2009-11-02 21:13 UTC (permalink / raw)
To: linux-arm-kernel
2009/11/1 Ben Dooks <ben-linux@fluff.org>:
> On Sun, Nov 01, 2009 at 01:56:43PM +0900, Thomas Abraham wrote:
>> + ? ? if (!pd) {
>> + ? ? ? ? ? ? printk(KERN_ERR "%s: no memory for platform data\n", __func__);
>> + ? ? ? ? ? ? return;
>> + ? ? }
>> + ? ? s3c_device_cfcon.dev.platform_data = pd;
>
> doing:
>
> ? ? ? ?if (!pd)
> ? ? ? ? ? ? ? ?printk(KERN_ERR "%s: no memory for platform data\n", __func__);
> ? ? ? ?else
> ? ? ? ? ? ? ? ?s3c_device_cfcon.dev.platform_data = pd;
>
> would be better.
I do not quite understand why this would be better. In the former case
the normal operation (e.g. s3c_dev... = pd;) is written directly without
any extra indentation and is not buried (deep) down in some error handling.
Writing code in that style makes it very easy to read with a mental filter like
if (test) {
ERROR HANDLING, ERROR HANDLING, ERROR HANDLING, ERROR HANDLING;
return;
}
NORMAL CASE, NORMAL CASE, NORMAL CASE, ;
See also http://stackoverflow.com/questions/114342/what-are-code-smells-what-is-the-best-way-to-correct-them/223881#223881.
BR H?kon L?vdal
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2009-11-02 21:13 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-11-01 4:56 [PATCH 5/7] S3C64XX: Add platform data and driver resources for IDE controller driver Thomas Abraham
2009-11-01 13:05 ` Ben Dooks
2009-11-02 6:13 ` Thomas Abraham
2009-11-02 21:13 ` Håkon Løvdal
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).