* [PATCH 1/6] serial: samsung: Keep a copy of platform data in driver's private data
2011-06-20 11:02 [PATCH 0/6] Add basic device tree support for Samsung's Exynos4 platform Thomas Abraham
@ 2011-06-20 11:02 ` Thomas Abraham
2011-06-20 15:54 ` Grant Likely
2011-06-20 11:02 ` [PATCH 2/6] serial: samsung: Add device tree support for s5pv210 uart driver Thomas Abraham
` (4 subsequent siblings)
5 siblings, 1 reply; 21+ messages in thread
From: Thomas Abraham @ 2011-06-20 11:02 UTC (permalink / raw)
To: linux-arm-kernel
The driver depends on pdev->dev.platform_data to retrive information
about the platform data even after the initialization. To add device
tree support, this has to be changed in way that the platform data
is avialable from driver's private data. This patch adds support
for keeping a copy of the plaform data in s3c24xx_uart_info and using
it when needed after the initialization.
Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
---
drivers/tty/serial/s5pv210.c | 12 ++++++++++--
drivers/tty/serial/samsung.c | 24 ++++++++++++++++++++----
drivers/tty/serial/samsung.h | 4 +++-
3 files changed, 33 insertions(+), 7 deletions(-)
diff --git a/drivers/tty/serial/s5pv210.c b/drivers/tty/serial/s5pv210.c
index d6b2423..3b2021a 100644
--- a/drivers/tty/serial/s5pv210.c
+++ b/drivers/tty/serial/s5pv210.c
@@ -27,9 +27,13 @@
static int s5pv210_serial_setsource(struct uart_port *port,
struct s3c24xx_uart_clksrc *clk)
{
- struct s3c2410_uartcfg *cfg = port->dev->platform_data;
+ struct s3c24xx_uart_port *ourport;
+ struct s3c2410_uartcfg *cfg;
unsigned long ucon = rd_regl(port, S3C2410_UCON);
+ ourport = container_of(port, struct s3c24xx_uart_port, port);
+ cfg = &ourport->info->cfg;
+
if ((cfg->clocks_size) == 1)
return 0;
@@ -50,9 +54,13 @@ static int s5pv210_serial_setsource(struct uart_port *port,
static int s5pv210_serial_getsource(struct uart_port *port,
struct s3c24xx_uart_clksrc *clk)
{
- struct s3c2410_uartcfg *cfg = port->dev->platform_data;
+ struct s3c24xx_uart_port *ourport;
+ struct s3c2410_uartcfg *cfg;
u32 ucon = rd_regl(port, S3C2410_UCON);
+ ourport = container_of(port, struct s3c24xx_uart_port, port);
+ cfg = &ourport->info->cfg;
+
clk->divisor = 1;
if ((cfg->clocks_size) == 1)
diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
index 7ead421..77d900f 100644
--- a/drivers/tty/serial/samsung.c
+++ b/drivers/tty/serial/samsung.c
@@ -42,6 +42,7 @@
#include <linux/delay.h>
#include <linux/clk.h>
#include <linux/cpufreq.h>
+#include <linux/slab.h>
#include <asm/irq.h>
@@ -169,10 +170,13 @@ static inline struct s3c24xx_uart_info *s3c24xx_port_to_info(struct uart_port *p
static inline struct s3c2410_uartcfg *s3c24xx_port_to_cfg(struct uart_port *port)
{
+ struct s3c24xx_uart_port *ourport;
+
if (port->dev == NULL)
return NULL;
- return (struct s3c2410_uartcfg *)port->dev->platform_data;
+ ourport = container_of(port, struct s3c24xx_uart_port, port);
+ return &ourport->info->cfg;
}
static int s3c24xx_serial_rx_fifocnt(struct s3c24xx_uart_port *ourport,
@@ -1053,7 +1057,7 @@ static int s3c24xx_serial_init_port(struct s3c24xx_uart_port *ourport,
struct platform_device *platdev)
{
struct uart_port *port = &ourport->port;
- struct s3c2410_uartcfg *cfg;
+ struct s3c2410_uartcfg *cfg = platdev->dev.platform_data;
struct resource *res;
int ret;
@@ -1062,14 +1066,24 @@ static int s3c24xx_serial_init_port(struct s3c24xx_uart_port *ourport,
if (platdev == NULL)
return -ENODEV;
- cfg = s3c24xx_dev_to_cfg(&platdev->dev);
-
if (port->mapbase != 0)
return 0;
+ if (cfg) {
+ memcpy((void *)&info->cfg, cfg, sizeof(struct s3c2410_uartcfg));
+ info->cfg.clocks = kzalloc(sizeof(struct s3c24xx_uart_clksrc) *
+ cfg->clocks_size, GFP_KERNEL);
+ if (!info->cfg.clocks)
+ return -ENOMEM;
+ memcpy(info->cfg.clocks, cfg->clocks,
+ sizeof(struct s3c24xx_uart_clksrc) * cfg->clocks_size);
+ }
+
+ cfg = &info->cfg;
if (cfg->hwport > CONFIG_SERIAL_SAMSUNG_UARTS) {
printk(KERN_ERR "%s: port %d bigger than %d\n", __func__,
cfg->hwport, CONFIG_SERIAL_SAMSUNG_UARTS);
+ kfree(info->cfg.clocks);
return -ERANGE;
}
@@ -1181,11 +1195,13 @@ EXPORT_SYMBOL_GPL(s3c24xx_serial_probe);
int __devexit s3c24xx_serial_remove(struct platform_device *dev)
{
struct uart_port *port = s3c24xx_dev_to_port(&dev->dev);
+ struct s3c24xx_uart_info *info = s3c24xx_port_to_info(port);
if (port) {
s3c24xx_serial_cpufreq_deregister(to_ourport(port));
device_remove_file(&dev->dev, &dev_attr_clock_source);
uart_remove_one_port(&s3c24xx_uart_drv, port);
+ kfree(info->cfg.clocks);
}
return 0;
diff --git a/drivers/tty/serial/samsung.h b/drivers/tty/serial/samsung.h
index a69d9a5..4f2f6f5 100644
--- a/drivers/tty/serial/samsung.h
+++ b/drivers/tty/serial/samsung.h
@@ -24,6 +24,9 @@ struct s3c24xx_uart_info {
unsigned int has_divslot:1;
+ /* copy of platform data */
+ struct s3c2410_uartcfg cfg;
+
/* clock source control */
int (*get_clksrc)(struct uart_port *, struct s3c24xx_uart_clksrc *clk);
@@ -56,7 +59,6 @@ struct s3c24xx_uart_port {
/* conversion functions */
#define s3c24xx_dev_to_port(__dev) (struct uart_port *)dev_get_drvdata(__dev)
-#define s3c24xx_dev_to_cfg(__dev) (struct s3c2410_uartcfg *)((__dev)->platform_data)
/* register access controls */
--
1.6.6.rc2
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 1/6] serial: samsung: Keep a copy of platform data in driver's private data
2011-06-20 11:02 ` [PATCH 1/6] serial: samsung: Keep a copy of platform data in driver's private data Thomas Abraham
@ 2011-06-20 15:54 ` Grant Likely
2011-06-21 11:07 ` Thomas Abraham
0 siblings, 1 reply; 21+ messages in thread
From: Grant Likely @ 2011-06-20 15:54 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Jun 20, 2011 at 5:02 AM, Thomas Abraham
<thomas.abraham@linaro.org> wrote:
> The driver depends on pdev->dev.platform_data to retrive information
> about the platform data even after the initialization. To add device
> tree support, this has to be changed in way that the platform data
> is avialable from driver's private data. This patch adds support
> for keeping a copy of the plaform data in s3c24xx_uart_info and using
> it when needed after the initialization.
>
> Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
> ---
> ?drivers/tty/serial/s5pv210.c | ? 12 ++++++++++--
> ?drivers/tty/serial/samsung.c | ? 24 ++++++++++++++++++++----
> ?drivers/tty/serial/samsung.h | ? ?4 +++-
> ?3 files changed, 33 insertions(+), 7 deletions(-)
Hi Thomas.
Don't forget you need to cc Alan Cox and the linux-serial mailing list
for tty driver patches.
Comments below...
>
> diff --git a/drivers/tty/serial/s5pv210.c b/drivers/tty/serial/s5pv210.c
> index d6b2423..3b2021a 100644
> --- a/drivers/tty/serial/s5pv210.c
> +++ b/drivers/tty/serial/s5pv210.c
> @@ -27,9 +27,13 @@
> ?static int s5pv210_serial_setsource(struct uart_port *port,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct s3c24xx_uart_clksrc *clk)
> ?{
> - ? ? ? struct s3c2410_uartcfg *cfg = port->dev->platform_data;
> + ? ? ? struct s3c24xx_uart_port *ourport;
> + ? ? ? struct s3c2410_uartcfg *cfg;
> ? ? ? ?unsigned long ucon = rd_regl(port, S3C2410_UCON);
>
> + ? ? ? ourport = container_of(port, struct s3c24xx_uart_port, port);
> + ? ? ? cfg = &ourport->info->cfg;
> +
> ? ? ? ?if ((cfg->clocks_size) == 1)
> ? ? ? ? ? ? ? ?return 0;
>
> @@ -50,9 +54,13 @@ static int s5pv210_serial_setsource(struct uart_port *port,
> ?static int s5pv210_serial_getsource(struct uart_port *port,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct s3c24xx_uart_clksrc *clk)
> ?{
> - ? ? ? struct s3c2410_uartcfg *cfg = port->dev->platform_data;
> + ? ? ? struct s3c24xx_uart_port *ourport;
> + ? ? ? struct s3c2410_uartcfg *cfg;
> ? ? ? ?u32 ucon = rd_regl(port, S3C2410_UCON);
>
> + ? ? ? ourport = container_of(port, struct s3c24xx_uart_port, port);
> + ? ? ? cfg = &ourport->info->cfg;
> +
> ? ? ? ?clk->divisor = 1;
>
> ? ? ? ?if ((cfg->clocks_size) == 1)
> diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
> index 7ead421..77d900f 100644
> --- a/drivers/tty/serial/samsung.c
> +++ b/drivers/tty/serial/samsung.c
> @@ -42,6 +42,7 @@
> ?#include <linux/delay.h>
> ?#include <linux/clk.h>
> ?#include <linux/cpufreq.h>
> +#include <linux/slab.h>
>
> ?#include <asm/irq.h>
>
> @@ -169,10 +170,13 @@ static inline struct s3c24xx_uart_info *s3c24xx_port_to_info(struct uart_port *p
>
> ?static inline struct s3c2410_uartcfg *s3c24xx_port_to_cfg(struct uart_port *port)
> ?{
> + ? ? ? struct s3c24xx_uart_port *ourport;
> +
> ? ? ? ?if (port->dev == NULL)
> ? ? ? ? ? ? ? ?return NULL;
>
> - ? ? ? return (struct s3c2410_uartcfg *)port->dev->platform_data;
> + ? ? ? ourport = container_of(port, struct s3c24xx_uart_port, port);
> + ? ? ? return &ourport->info->cfg;
> ?}
>
> ?static int s3c24xx_serial_rx_fifocnt(struct s3c24xx_uart_port *ourport,
> @@ -1053,7 +1057,7 @@ static int s3c24xx_serial_init_port(struct s3c24xx_uart_port *ourport,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct platform_device *platdev)
> ?{
> ? ? ? ?struct uart_port *port = &ourport->port;
> - ? ? ? struct s3c2410_uartcfg *cfg;
> + ? ? ? struct s3c2410_uartcfg *cfg = platdev->dev.platform_data;
> ? ? ? ?struct resource *res;
> ? ? ? ?int ret;
>
> @@ -1062,14 +1066,24 @@ static int s3c24xx_serial_init_port(struct s3c24xx_uart_port *ourport,
> ? ? ? ?if (platdev == NULL)
> ? ? ? ? ? ? ? ?return -ENODEV;
>
> - ? ? ? cfg = s3c24xx_dev_to_cfg(&platdev->dev);
> -
> ? ? ? ?if (port->mapbase != 0)
> ? ? ? ? ? ? ? ?return 0;
>
> + ? ? ? if (cfg) {
> + ? ? ? ? ? ? ? memcpy((void *)&info->cfg, cfg, sizeof(struct s3c2410_uartcfg));
"info->cfg = *cfg;" should be sufficient.
> + ? ? ? ? ? ? ? info->cfg.clocks = kzalloc(sizeof(struct s3c24xx_uart_clksrc) *
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? cfg->clocks_size, GFP_KERNEL);
> + ? ? ? ? ? ? ? if (!info->cfg.clocks)
> + ? ? ? ? ? ? ? ? ? ? ? return -ENOMEM;
> + ? ? ? ? ? ? ? memcpy(info->cfg.clocks, cfg->clocks,
> + ? ? ? ? ? ? ? ? ? ? ? sizeof(struct s3c24xx_uart_clksrc) * cfg->clocks_size);
> + ? ? ? }
ewwh. There has to be a better way to do this. Part of the point of
putting a copy of pdata into the private data structure is to simplify
the driver so that kzallocing wouldn't be necessary. With that clock
table, the driver actually gets more complex because both DT and
non-DT paths now need to kzalloc a clock array.
>From bogus@does.not.exist.com Wed Jun 1 12:03:18 2011
From: bogus@does.not.exist.com ()
Date: Wed, 01 Jun 2011 16:03:18 -0000
Subject: No subject
Message-ID: <mailman.30.1308585324.24103.linux-arm-kernel@lists.infradead.org>
a static array of one or two entries; min & max baud are always set to
0, and names are one of:
- uclk & pclk
- uclk
- uclk1
- fclk (with divisor either 10 or 0)
- pclk_low & uclk1
You could also make the clock structure a static array of 2 elements
in the private data structure. That would simplify both this code and
the followon DT patch.
Also, peaking forward at what the 2nd patch does, I think that it
might just be a little premature to try and decode the clock info from
the DT. But I'll address that issue when replying to the second
patch.
> +
> + =A0 =A0 =A0 cfg =3D &info->cfg;
> =A0 =A0 =A0 =A0if (cfg->hwport > CONFIG_SERIAL_SAMSUNG_UARTS) {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0printk(KERN_ERR "%s: port %d bigger than %=
d\n", __func__,
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 cfg->hwport, CONFIG_SERIAL_SA=
MSUNG_UARTS);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 kfree(info->cfg.clocks);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return -ERANGE;
> =A0 =A0 =A0 =A0}
>
> @@ -1181,11 +1195,13 @@ EXPORT_SYMBOL_GPL(s3c24xx_serial_probe);
> =A0int __devexit s3c24xx_serial_remove(struct platform_device *dev)
> =A0{
> =A0 =A0 =A0 =A0struct uart_port *port =3D s3c24xx_dev_to_port(&dev->dev);
> + =A0 =A0 =A0 struct s3c24xx_uart_info *info =3D s3c24xx_port_to_info(por=
t);
>
> =A0 =A0 =A0 =A0if (port) {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0s3c24xx_serial_cpufreq_deregister(to_ourpo=
rt(port));
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0device_remove_file(&dev->dev, &dev_attr_cl=
ock_source);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0uart_remove_one_port(&s3c24xx_uart_drv, po=
rt);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 kfree(info->cfg.clocks);
> =A0 =A0 =A0 =A0}
>
> =A0 =A0 =A0 =A0return 0;
> diff --git a/drivers/tty/serial/samsung.h b/drivers/tty/serial/samsung.h
> index a69d9a5..4f2f6f5 100644
> --- a/drivers/tty/serial/samsung.h
> +++ b/drivers/tty/serial/samsung.h
> @@ -24,6 +24,9 @@ struct s3c24xx_uart_info {
>
> =A0 =A0 =A0 =A0unsigned int =A0 =A0 =A0 =A0 =A0 =A0has_divslot:1;
>
> + =A0 =A0 =A0 /* copy of platform data */
I'd change this to "copy of /configuration/ data" since the data
doesn't necessarily come from the platform_data pointer anymore.
> + =A0 =A0 =A0 struct s3c2410_uartcfg =A0cfg;
> +
> =A0 =A0 =A0 =A0/* clock source control */
>
> =A0 =A0 =A0 =A0int (*get_clksrc)(struct uart_port *, struct s3c24xx_uart_=
clksrc *clk);
> @@ -56,7 +59,6 @@ struct s3c24xx_uart_port {
> =A0/* conversion functions */
>
> =A0#define s3c24xx_dev_to_port(__dev) (struct uart_port *)dev_get_drvdata=
(__dev)
> -#define s3c24xx_dev_to_cfg(__dev) (struct s3c2410_uartcfg *)((__dev)->pl=
atform_data)
>
> =A0/* register access controls */
>
> --
> 1.6.6.rc2
>
>
> _______________________________________________
> linaro-dev mailing list
> linaro-dev at lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/linaro-dev
>
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply [flat|nested] 21+ messages in thread* [PATCH 1/6] serial: samsung: Keep a copy of platform data in driver's private data
2011-06-20 15:54 ` Grant Likely
@ 2011-06-21 11:07 ` Thomas Abraham
0 siblings, 0 replies; 21+ messages in thread
From: Thomas Abraham @ 2011-06-21 11:07 UTC (permalink / raw)
To: linux-arm-kernel
Hi Grant,
On 20 June 2011 21:24, Grant Likely <grant.likely@secretlab.ca> wrote:
> On Mon, Jun 20, 2011 at 5:02 AM, Thomas Abraham
> <thomas.abraham@linaro.org> wrote:
>> The driver depends on pdev->dev.platform_data to retrive information
>> about the platform data even after the initialization. To add device
>> tree support, this has to be changed in way that the platform data
>> is avialable from driver's private data. This patch adds support
>> for keeping a copy of the plaform data in s3c24xx_uart_info and using
>> it when needed after the initialization.
>>
>> Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
>> ---
>> ?drivers/tty/serial/s5pv210.c | ? 12 ++++++++++--
>> ?drivers/tty/serial/samsung.c | ? 24 ++++++++++++++++++++----
>> ?drivers/tty/serial/samsung.h | ? ?4 +++-
>> ?3 files changed, 33 insertions(+), 7 deletions(-)
>
> Hi Thomas.
>
> Don't forget you need to cc Alan Cox and the linux-serial mailing list
> for tty driver patches.
Ok. I will do that when I submit the next version of the patch.
>
> Comments below...
>
>>
>> diff --git a/drivers/tty/serial/s5pv210.c b/drivers/tty/serial/s5pv210.c
>> index d6b2423..3b2021a 100644
>> --- a/drivers/tty/serial/s5pv210.c
>> +++ b/drivers/tty/serial/s5pv210.c
>> @@ -27,9 +27,13 @@
<snip>
>> + ? ? ? if (cfg) {
>> + ? ? ? ? ? ? ? memcpy((void *)&info->cfg, cfg, sizeof(struct s3c2410_uartcfg));
>
> "info->cfg = *cfg;" should be sufficient.
>
>> + ? ? ? ? ? ? ? info->cfg.clocks = kzalloc(sizeof(struct s3c24xx_uart_clksrc) *
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? cfg->clocks_size, GFP_KERNEL);
>> + ? ? ? ? ? ? ? if (!info->cfg.clocks)
>> + ? ? ? ? ? ? ? ? ? ? ? return -ENOMEM;
>> + ? ? ? ? ? ? ? memcpy(info->cfg.clocks, cfg->clocks,
>> + ? ? ? ? ? ? ? ? ? ? ? sizeof(struct s3c24xx_uart_clksrc) * cfg->clocks_size);
>> + ? ? ? }
>
> ewwh. There has to be a better way to do this. ?Part of the point of
> putting a copy of pdata into the private data structure is to simplify
> the driver so that kzallocing wouldn't be necessary. ?With that clock
> table, the driver actually gets more complex because both DT and
> non-DT paths now need to kzalloc a clock array.
>
> From what I can tell, the list of clocks on all mainlined platforms is
> a static array of one or two entries; min & max baud are always set to
> 0, and names are one of:
> - uclk & pclk
> - uclk
> - uclk1
> - fclk (with divisor either 10 or 0)
> - pclk_low & uclk1
>
> You could also make the clock structure a static array of 2 elements
> in the private data structure. ?That would simplify both this code and
> the followon DT patch.
>
> Also, peaking forward at what the 2nd patch does, I think that it
> might just be a little premature to try and decode the clock info from
> the DT. ?But I'll address that issue when replying to the second
> patch.
Thanks for the suggestion. I had not thought about these issues. One
possible option in this case is using Sylwester's suggestion of
changing exynos4 clkdev support to be similar to the omap clkdev
support. That way, an additional level of indirection is possible. The
uart driver could than be modified to lookup clock with generic names
like "uart_clksrc0", "uart_clksrc1" and "uart_clksrc2". With this,
there will be no need to pass clock names to the uart driver. I will
check if this can be done.
>
>> +
>> + ? ? ? cfg = &info->cfg;
>> ? ? ? ?if (cfg->hwport > CONFIG_SERIAL_SAMSUNG_UARTS) {
>> ? ? ? ? ? ? ? ?printk(KERN_ERR "%s: port %d bigger than %d\n", __func__,
>> ? ? ? ? ? ? ? ? ? ? ? cfg->hwport, CONFIG_SERIAL_SAMSUNG_UARTS);
>> + ? ? ? ? ? ? ? kfree(info->cfg.clocks);
>> ? ? ? ? ? ? ? ?return -ERANGE;
>> ? ? ? ?}
>>
>> @@ -1181,11 +1195,13 @@ EXPORT_SYMBOL_GPL(s3c24xx_serial_probe);
>> ?int __devexit s3c24xx_serial_remove(struct platform_device *dev)
>> ?{
>> ? ? ? ?struct uart_port *port = s3c24xx_dev_to_port(&dev->dev);
>> + ? ? ? struct s3c24xx_uart_info *info = s3c24xx_port_to_info(port);
>>
>> ? ? ? ?if (port) {
>> ? ? ? ? ? ? ? ?s3c24xx_serial_cpufreq_deregister(to_ourport(port));
>> ? ? ? ? ? ? ? ?device_remove_file(&dev->dev, &dev_attr_clock_source);
>> ? ? ? ? ? ? ? ?uart_remove_one_port(&s3c24xx_uart_drv, port);
>> + ? ? ? ? ? ? ? kfree(info->cfg.clocks);
>> ? ? ? ?}
>>
>> ? ? ? ?return 0;
>> diff --git a/drivers/tty/serial/samsung.h b/drivers/tty/serial/samsung.h
>> index a69d9a5..4f2f6f5 100644
>> --- a/drivers/tty/serial/samsung.h
>> +++ b/drivers/tty/serial/samsung.h
>> @@ -24,6 +24,9 @@ struct s3c24xx_uart_info {
>>
>> ? ? ? ?unsigned int ? ? ? ? ? ?has_divslot:1;
>>
>> + ? ? ? /* copy of platform data */
>
> I'd change this to "copy of /configuration/ data" since the data
> doesn't necessarily come from the platform_data pointer anymore.
Ok. I will change this. Thanks for reviewing the patches.
Thomas.
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 2/6] serial: samsung: Add device tree support for s5pv210 uart driver
2011-06-20 11:02 [PATCH 0/6] Add basic device tree support for Samsung's Exynos4 platform Thomas Abraham
2011-06-20 11:02 ` [PATCH 1/6] serial: samsung: Keep a copy of platform data in driver's private data Thomas Abraham
@ 2011-06-20 11:02 ` Thomas Abraham
2011-06-20 16:43 ` Grant Likely
2011-06-20 11:02 ` [PATCH 3/6] watchdog: s3c2410: Add support for device tree based probe Thomas Abraham
` (3 subsequent siblings)
5 siblings, 1 reply; 21+ messages in thread
From: Thomas Abraham @ 2011-06-20 11:02 UTC (permalink / raw)
To: linux-arm-kernel
For device tree based probe, the dependecy on pdev->id to attach a
corresponding default port info to the driver's private data is
removed. The fifosize parameter is obtained from the device tree
node and the next available instance of port info is updated
with the fifosize value and attached to the driver's private data.
The samsung uart core driver is also modified to parse the device
tree node and pick up the platform data from the node.
Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
---
.../bindings/tty/serial/samsung_uart.txt | 50 +++++++++++
drivers/tty/serial/s5pv210.c | 27 ++++++-
drivers/tty/serial/samsung.c | 90 ++++++++++++++++++++
3 files changed, 166 insertions(+), 1 deletions(-)
create mode 100644 Documentation/devicetree/bindings/tty/serial/samsung_uart.txt
diff --git a/Documentation/devicetree/bindings/tty/serial/samsung_uart.txt b/Documentation/devicetree/bindings/tty/serial/samsung_uart.txt
new file mode 100644
index 0000000..4c0783d
--- /dev/null
+++ b/Documentation/devicetree/bindings/tty/serial/samsung_uart.txt
@@ -0,0 +1,50 @@
+* Samsung's UART Controller
+
+The Samsung's UART controller is used for serial communications on all of
+Samsung's s3c24xx, s3c64xx and s5p series application processors.
+
+Required properties:
+- compatible : should be specific to the application processor
+ - "samsung,s5pv210-uart" , for s5pc110, s5pv210 and Exynos4 family.
+ - "samsung,s3c6400-uart", for s3c64xx, s5p64xx and s5pc100.
+ - "samsung,s3c2410-uart", for s3c2410.
+ - "samsung,s3c2412-uart", for s3c2412.
+ - "samsung,s3c2440-uart", for s3c244x.
+
+- reg : base physical address of the controller and length of memory mapped
+ region.
+
+- interrupts : Three interrupt numbers should be specified in the following
+ order - TX interrupt, RX interrupt, Error Interrupt.
+
+- hwport : Instance number of the UART controller in the processor.
+ (ToDo: Remove this from the driver).
+
+- flags : Not used, but set value as 0. (ToDo: Remove this flag from driver).
+
+- uart_flags : Additional serial core flags to passed to the serial core
+ when the driver is registred. For example: UPF_CONS_FLOW.
+
+- has_fracval : Set to 1, if the controller supports fractional part of
+ for the baud divisor, otherwise, set to 0.
+
+- ucon_default : Default board specific setting of UCON register.
+
+- ulcon_default : Default board specific setting of ULCON register.
+
+- ufcon_default : Default board specific setting of UFCON register.
+
+- uart_clksrc : One or more child nodes representing the clock sources that
+ could be used to derive the buad rate. Each of these child nodes
+ has four required properties.
+
+ - name : Name of the parent clock.
+ - divisor : divisor from the clock to the uart controller.
+ - min_baud : Minimum baud rate for which this clock can be used.
+ Set to zero, if there is no limitation.
+ - max_buad : Maximum baud rate for which this clock can be used.
+ Set to zero, if there is no limitation.
+
+Optional properties:
+- fifosize: Size of the tx/rx fifo used in the controller. If not specified,
+ the default value of the fifosize is 16.
diff --git a/drivers/tty/serial/s5pv210.c b/drivers/tty/serial/s5pv210.c
index 3b2021a..9aacbda 100644
--- a/drivers/tty/serial/s5pv210.c
+++ b/drivers/tty/serial/s5pv210.c
@@ -18,6 +18,7 @@
#include <linux/init.h>
#include <linux/serial_core.h>
#include <linux/serial.h>
+#include <linux/of.h>
#include <asm/irq.h>
#include <mach/hardware.h>
@@ -131,15 +132,39 @@ static struct s3c24xx_uart_info *s5p_uart_inf[] = {
/* device management */
static int s5p_serial_probe(struct platform_device *pdev)
{
- return s3c24xx_serial_probe(pdev, s5p_uart_inf[pdev->id]);
+ const void *prop;
+ unsigned int port = pdev->id;
+ unsigned int fifosize = 16;
+ static unsigned int probe_index;
+
+ if (pdev->dev.of_node) {
+ prop = of_get_property(pdev->dev.of_node, "fifosize", NULL);
+ if (prop)
+ fifosize = be32_to_cpu(*(u32 *)prop);
+ s5p_uart_inf[probe_index]->fifosize = fifosize;
+ port = probe_index++;
+ }
+
+ return s3c24xx_serial_probe(pdev, s5p_uart_inf[port]);
}
+#ifdef CONFIG_OF
+static const struct of_device_id s5pv210_uart_match[] = {
+ { .compatible = "samsung,s5pv210-uart" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, s5pv210_uart_match);
+#else
+#define s5pv210_uart_match NULL
+#endif
+
static struct platform_driver s5p_serial_driver = {
.probe = s5p_serial_probe,
.remove = __devexit_p(s3c24xx_serial_remove),
.driver = {
.name = "s5pv210-uart",
.owner = THIS_MODULE,
+ .of_match_table = s5pv210_uart_match,
},
};
diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
index 77d900f..1e58a7b 100644
--- a/drivers/tty/serial/samsung.c
+++ b/drivers/tty/serial/samsung.c
@@ -43,6 +43,7 @@
#include <linux/clk.h>
#include <linux/cpufreq.h>
#include <linux/slab.h>
+#include <linux/of.h>
#include <asm/irq.h>
@@ -1047,6 +1048,91 @@ static inline void s3c24xx_serial_cpufreq_deregister(struct s3c24xx_uart_port *p
}
#endif
+/*
+ * s3c24xx_serial_parse_dt
+ *
+ * Parse the platform data from the device tree and keep a copy of it.
+ */
+
+static int s3c24xx_serial_parse_dt(struct device_node *np,
+ struct s3c2410_uartcfg *cfg)
+{
+ struct s3c24xx_uart_clksrc *clksrc = NULL;
+ struct device_node *child = NULL;
+ unsigned int len;
+ const void *prop;
+
+ if (!np)
+ return -EINVAL;
+
+ prop = of_get_property(np, "hwport", &len);
+ if (prop && len)
+ cfg->hwport = be32_to_cpu(*(u32 *)prop);
+
+ prop = of_get_property(np, "flags", &len);
+ if (prop && len)
+ cfg->flags = be32_to_cpu(*(u32 *)prop);
+
+ prop = of_get_property(np, "uart_flags", &len);
+ if (prop && len)
+ cfg->uart_flags = be32_to_cpu(*(u32 *)prop);
+
+ prop = of_get_property(np, "has_fracval", &len);
+ if (prop && len)
+ cfg->has_fracval = be32_to_cpu(*(u32 *)prop);
+
+ prop = of_get_property(np, "ucon_default", &len);
+ if (prop && len)
+ cfg->ucon = be32_to_cpu(*(u32 *)prop);
+
+ prop = of_get_property(np, "ulcon_default", &len);
+ if (prop && len)
+ cfg->ulcon = be32_to_cpu(*(u32 *)prop);
+
+ prop = of_get_property(np, "ufcon_default", &len);
+ if (prop && len)
+ cfg->ufcon = be32_to_cpu(*(u32 *)prop);
+
+ /* Count the number of clock source options available */
+ len = 0;
+
+ while ((child = of_get_next_child(np, child)))
+ len++;
+
+ if (len) {
+ clksrc = kzalloc(sizeof(struct s3c24xx_uart_clksrc) * len,
+ GFP_KERNEL);
+ if (!clksrc)
+ return -ENOMEM;
+ }
+
+ cfg->clocks = clksrc;
+ cfg->clocks_size = len;
+
+ /* Read the information about all clock sources */
+ for_each_child_of_node(np, child) {
+ prop = of_get_property(child, "clk_name", &len);
+ if (prop && len)
+ clksrc->name = (const char *)prop;
+
+ prop = of_get_property(child, "divisor", &len);
+ if (prop && len)
+ clksrc->divisor = be32_to_cpu(*(u32 *)prop);
+
+ prop = of_get_property(child, "min_baud", &len);
+ if (prop && len)
+ clksrc->min_baud = be32_to_cpu(*(u32 *)prop);
+
+ prop = of_get_property(child, "max_baud", &len);
+ if (prop && len)
+ clksrc->max_baud = be32_to_cpu(*(u32 *)prop);
+
+ clksrc++;
+ }
+
+ return 0;
+}
+
/* s3c24xx_serial_init_port
*
* initialise a single serial port from the platform device given
@@ -1077,6 +1163,10 @@ static int s3c24xx_serial_init_port(struct s3c24xx_uart_port *ourport,
return -ENOMEM;
memcpy(info->cfg.clocks, cfg->clocks,
sizeof(struct s3c24xx_uart_clksrc) * cfg->clocks_size);
+ } else {
+ ret = s3c24xx_serial_parse_dt(platdev->dev.of_node, &info->cfg);
+ if (ret)
+ return ret;
}
cfg = &info->cfg;
--
1.6.6.rc2
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 2/6] serial: samsung: Add device tree support for s5pv210 uart driver
2011-06-20 11:02 ` [PATCH 2/6] serial: samsung: Add device tree support for s5pv210 uart driver Thomas Abraham
@ 2011-06-20 16:43 ` Grant Likely
2011-06-21 11:26 ` Thomas Abraham
` (2 more replies)
0 siblings, 3 replies; 21+ messages in thread
From: Grant Likely @ 2011-06-20 16:43 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Jun 20, 2011 at 5:02 AM, Thomas Abraham
<thomas.abraham@linaro.org> wrote:
> For device tree based probe, the dependecy on pdev->id to attach a
> corresponding default port info to the driver's private data is
> removed. The fifosize parameter is obtained from the device tree
> node and the next available instance of port info is updated
> with the fifosize value and attached to the driver's private data.
>
> The samsung uart core driver is also modified to parse the device
> tree node and pick up the platform data from the node.
>
> Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
> ---
> ?.../bindings/tty/serial/samsung_uart.txt ? ? ? ? ? | ? 50 +++++++++++
> ?drivers/tty/serial/s5pv210.c ? ? ? ? ? ? ? ? ? ? ? | ? 27 ++++++-
> ?drivers/tty/serial/samsung.c ? ? ? ? ? ? ? ? ? ? ? | ? 90 ++++++++++++++++++++
> ?3 files changed, 166 insertions(+), 1 deletions(-)
> ?create mode 100644 Documentation/devicetree/bindings/tty/serial/samsung_uart.txt
>
> diff --git a/Documentation/devicetree/bindings/tty/serial/samsung_uart.txt b/Documentation/devicetree/bindings/tty/serial/samsung_uart.txt
> new file mode 100644
> index 0000000..4c0783d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/tty/serial/samsung_uart.txt
> @@ -0,0 +1,50 @@
> +* Samsung's UART Controller
> +
> +The Samsung's UART controller is used for serial communications on all of
> +Samsung's s3c24xx, s3c64xx and s5p series application processors.
> +
> +Required properties:
> +- compatible : should be specific to the application processor
> + ? ? ? - "samsung,s5pv210-uart" , for s5pc110, s5pv210 and Exynos4 family.
> + ? ? ? - "samsung,s3c6400-uart", for s3c64xx, s5p64xx and s5pc100.
> + ? ? ? - "samsung,s3c2410-uart", for s3c2410.
> + ? ? ? - "samsung,s3c2412-uart", for s3c2412.
> + ? ? ? - "samsung,s3c2440-uart", for s3c244x.
> +
> +- reg : ? ? ? ?base physical address of the controller and length of memory mapped
> + ? ? ? region.
> +
> +- interrupts : Three interrupt numbers should be specified in the following
> + ? ? ? order - TX interrupt, RX interrupt, Error Interrupt.
> +
> +- hwport : Instance number of the UART controller in the processor.
> + ? ? ? (ToDo: Remove this from the driver).
> +
> +- flags : Not used, but set value as 0. (ToDo: Remove this flag from driver).
If they are to be removed, then you should drop them from the documentation.
> +
> +- uart_flags : Additional serial core flags to passed to the serial core
> + ? ? ? when the driver is registred. For example: UPF_CONS_FLOW.
Underscores are discouraged in property and node names. Use '-' instead.
For custom properties, you should prefix the property name with 'samsung,'.
This looks very much like directly encoding the Linux flags into the
device tree. The binding should be completely contained within
itself, and not refer to OS implementation details. It is fine to use
the same values that Linux happens to use, but they need to still be
explicitly documented as to what they mean. Also, a 'flags' property
usually isn't very friendly to mere-mortals when the explicit
behaviour can be enabled with the presence of a named property. For
example; something like a "samsung,uart-has-rtscts" to enable rts/cts.
> +
> +- has_fracval : Set to 1, if the controller supports fractional part of
> + ? ? ? for the baud divisor, otherwise, set to 0.
Boolean stuff often doesn't need a value. If the property is present,
it is a '1'. If it isn't, then it is a '0'.
> +
> +- ucon_default : Default board specific setting of UCON register.
> +
> +- ulcon_default : Default board specific setting of ULCON register.
> +
> +- ufcon_default : Default board specific setting of UFCON register.
I think I've commented on this before, but I do try to avoid direct
coding registers into the DT. That said, sometimes there really isn't
a nice human-friendly way of encoding things and direct register
values is the best approach.
> +
> +- uart_clksrc : One or more child nodes representing the clock sources that
> + ? ? ? could be used to derive the buad rate. Each of these child nodes
> + ? ? ? has four required properties.
> +
> + ? ? ? - name : Name of the parent clock.
> + ? ? ? - divisor : divisor from the clock to the uart controller.
> + ? ? ? - min_baud : Minimum baud rate for which this clock can be used.
> + ? ? ? ? ? ? ? ? ? ? ? Set to zero, if there is no limitation.
> + ? ? ? - max_buad : Maximum baud rate for which this clock can be used.
typo: s/buad/baud/
> + ? ? ? ? ? ? ? ? ? ? ? Set to zero, if there is no limitation.
This looks to be directly encoding the current Linux implementation
details into the device tree (it is a direct copy of the config
structure members), and it doesn't use the common clock binding. It's
fine to use sub nodes for each clock attachment, particularly because
it looks like the uart is able to apply it's own divisor to the clock
input, but I would definitely encode the data using the existing
struct clock binding.
> +
> +Optional properties:
> +- fifosize: Size of the tx/rx fifo used in the controller. If not specified,
> + ? ? ? the default value of the fifosize is 16.
> diff --git a/drivers/tty/serial/s5pv210.c b/drivers/tty/serial/s5pv210.c
> index 3b2021a..9aacbda 100644
> --- a/drivers/tty/serial/s5pv210.c
> +++ b/drivers/tty/serial/s5pv210.c
> @@ -18,6 +18,7 @@
> ?#include <linux/init.h>
> ?#include <linux/serial_core.h>
> ?#include <linux/serial.h>
> +#include <linux/of.h>
>
> ?#include <asm/irq.h>
> ?#include <mach/hardware.h>
> @@ -131,15 +132,39 @@ static struct s3c24xx_uart_info *s5p_uart_inf[] = {
> ?/* device management */
> ?static int s5p_serial_probe(struct platform_device *pdev)
> ?{
> - ? ? ? return s3c24xx_serial_probe(pdev, s5p_uart_inf[pdev->id]);
> + ? ? ? const void *prop;
> + ? ? ? unsigned int port = pdev->id;
> + ? ? ? unsigned int fifosize = 16;
> + ? ? ? static unsigned int probe_index;
> +
> + ? ? ? if (pdev->dev.of_node) {
> + ? ? ? ? ? ? ? prop = of_get_property(pdev->dev.of_node, "fifosize", NULL);
> + ? ? ? ? ? ? ? if (prop)
> + ? ? ? ? ? ? ? ? ? ? ? fifosize = be32_to_cpu(*(u32 *)prop);
Okay, this is getting ugly (not your fault, but this pattern has
become too common. Can you craft and post a patch that adds the
following functions to drivers/of/base.c and include/linux/of.h
/* offset in cells, not bytes */
int dt_decode_u32(struct *property, int offset, u32 *out_val)
{
if (!property || !property->value)
return -EINVAL;
if ((offset + 1) * 4 > property->length)
return -EINVAL;
*out_val = of_read_number(property->value + (offset * 4), 1);
return 0;
}
int dt_decode_u64(struct *property, int offset, u64 *out_val)
{
...
}
int dt_decode_string(struct *property, int index, char **out_string);
{
...
}
Plus a set of companion functions:
int dt_getprop_u32(struct device_node *np, char *propname, int offset,
u32 *out_val)
{
return dt_decode_u32(of_find_property(np, propname, NULL),
offset, out_val);
}
int dt_getprop_u64(struct *device_node, char *propname, int offset,
u64 *out_val);
{
...
}
int dt_getprop_string(struct *device_node, char *propname, int index,
char **out_string);
{
...
}
Then you'll be able to simply do the following to decode each
property, with fifosize being left alone if the property cannot be
found or decoded:
dt_getprop_u32(pdev->dev.of_node, "samsung,fifosize", &fifosize);
> +#ifdef CONFIG_OF
> +static const struct of_device_id s5pv210_uart_match[] = {
> + ? ? ? { .compatible = "samsung,s5pv210-uart" },
> + ? ? ? {},
> +};
> +MODULE_DEVICE_TABLE(of, s5pv210_uart_match);
> +#else
> +#define s5pv210_uart_match NULL
> +#endif
> +
> ?static struct platform_driver s5p_serial_driver = {
> ? ? ? ?.probe ? ? ? ? ?= s5p_serial_probe,
> ? ? ? ?.remove ? ? ? ? = __devexit_p(s3c24xx_serial_remove),
> ? ? ? ?.driver ? ? ? ? = {
> ? ? ? ? ? ? ? ?.name ? = "s5pv210-uart",
> ? ? ? ? ? ? ? ?.owner ?= THIS_MODULE,
> + ? ? ? ? ? ? ? .of_match_table = s5pv210_uart_match,
> ? ? ? ?},
> ?};
>
> diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
> index 77d900f..1e58a7b 100644
> --- a/drivers/tty/serial/samsung.c
> +++ b/drivers/tty/serial/samsung.c
> @@ -43,6 +43,7 @@
> ?#include <linux/clk.h>
> ?#include <linux/cpufreq.h>
> ?#include <linux/slab.h>
> +#include <linux/of.h>
>
> ?#include <asm/irq.h>
>
> @@ -1047,6 +1048,91 @@ static inline void s3c24xx_serial_cpufreq_deregister(struct s3c24xx_uart_port *p
> ?}
> ?#endif
>
> +/*
> + * s3c24xx_serial_parse_dt
> + *
> + * Parse the platform data from the device tree and keep a copy of it.
> + */
> +
> +static int s3c24xx_serial_parse_dt(struct device_node *np,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? struct s3c2410_uartcfg *cfg)
> +{
> + ? ? ? struct s3c24xx_uart_clksrc *clksrc = NULL;
> + ? ? ? struct device_node *child = NULL;
> + ? ? ? unsigned int len;
> + ? ? ? const void *prop;
> +
> + ? ? ? if (!np)
> + ? ? ? ? ? ? ? return -EINVAL;
> +
> + ? ? ? prop = of_get_property(np, "hwport", &len);
> + ? ? ? if (prop && len)
> + ? ? ? ? ? ? ? cfg->hwport = be32_to_cpu(*(u32 *)prop);
> +
> + ? ? ? prop = of_get_property(np, "flags", &len);
> + ? ? ? if (prop && len)
> + ? ? ? ? ? ? ? cfg->flags = be32_to_cpu(*(u32 *)prop);
> +
> + ? ? ? prop = of_get_property(np, "uart_flags", &len);
> + ? ? ? if (prop && len)
> + ? ? ? ? ? ? ? cfg->uart_flags = be32_to_cpu(*(u32 *)prop);
> +
> + ? ? ? prop = of_get_property(np, "has_fracval", &len);
> + ? ? ? if (prop && len)
> + ? ? ? ? ? ? ? cfg->has_fracval = be32_to_cpu(*(u32 *)prop);
> +
> + ? ? ? prop = of_get_property(np, "ucon_default", &len);
> + ? ? ? if (prop && len)
> + ? ? ? ? ? ? ? cfg->ucon = be32_to_cpu(*(u32 *)prop);
> +
> + ? ? ? prop = of_get_property(np, "ulcon_default", &len);
> + ? ? ? if (prop && len)
> + ? ? ? ? ? ? ? cfg->ulcon = be32_to_cpu(*(u32 *)prop);
> +
> + ? ? ? prop = of_get_property(np, "ufcon_default", &len);
> + ? ? ? if (prop && len)
> + ? ? ? ? ? ? ? cfg->ufcon = be32_to_cpu(*(u32 *)prop);
> +
> + ? ? ? /* Count the number of clock source options available */
> + ? ? ? len = 0;
> +
> + ? ? ? while ((child = of_get_next_child(np, child)))
> + ? ? ? ? ? ? ? len++;
> +
> + ? ? ? if (len) {
> + ? ? ? ? ? ? ? clksrc = kzalloc(sizeof(struct s3c24xx_uart_clksrc) * len,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? GFP_KERNEL);
> + ? ? ? ? ? ? ? if (!clksrc)
> + ? ? ? ? ? ? ? ? ? ? ? return -ENOMEM;
> + ? ? ? }
> +
> + ? ? ? cfg->clocks = clksrc;
> + ? ? ? cfg->clocks_size = len;
> +
> + ? ? ? /* Read the information about all clock sources */
> + ? ? ? for_each_child_of_node(np, child) {
> + ? ? ? ? ? ? ? prop = of_get_property(child, "clk_name", &len);
> + ? ? ? ? ? ? ? if (prop && len)
> + ? ? ? ? ? ? ? ? ? ? ? clksrc->name = (const char *)prop;
> +
> + ? ? ? ? ? ? ? prop = of_get_property(child, "divisor", &len);
> + ? ? ? ? ? ? ? if (prop && len)
> + ? ? ? ? ? ? ? ? ? ? ? clksrc->divisor = be32_to_cpu(*(u32 *)prop);
> +
> + ? ? ? ? ? ? ? prop = of_get_property(child, "min_baud", &len);
> + ? ? ? ? ? ? ? if (prop && len)
> + ? ? ? ? ? ? ? ? ? ? ? clksrc->min_baud = be32_to_cpu(*(u32 *)prop);
> +
> + ? ? ? ? ? ? ? prop = of_get_property(child, "max_baud", &len);
> + ? ? ? ? ? ? ? if (prop && len)
> + ? ? ? ? ? ? ? ? ? ? ? clksrc->max_baud = be32_to_cpu(*(u32 *)prop);
> +
> + ? ? ? ? ? ? ? clksrc++;
> + ? ? ? }
> +
> + ? ? ? return 0;
> +}
> +
> ?/* s3c24xx_serial_init_port
> ?*
> ?* initialise a single serial port from the platform device given
> @@ -1077,6 +1163,10 @@ static int s3c24xx_serial_init_port(struct s3c24xx_uart_port *ourport,
> ? ? ? ? ? ? ? ? ? ? ? ?return -ENOMEM;
> ? ? ? ? ? ? ? ?memcpy(info->cfg.clocks, cfg->clocks,
> ? ? ? ? ? ? ? ? ? ? ? ?sizeof(struct s3c24xx_uart_clksrc) * cfg->clocks_size);
> + ? ? ? } else {
> + ? ? ? ? ? ? ? ret = s3c24xx_serial_parse_dt(platdev->dev.of_node, &info->cfg);
> + ? ? ? ? ? ? ? if (ret)
> + ? ? ? ? ? ? ? ? ? ? ? return ret;
> ? ? ? ?}
>
> ? ? ? ?cfg = &info->cfg;
> --
> 1.6.6.rc2
>
>
> _______________________________________________
> linaro-dev mailing list
> linaro-dev at lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/linaro-dev
>
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply [flat|nested] 21+ messages in thread* [PATCH 2/6] serial: samsung: Add device tree support for s5pv210 uart driver
2011-06-20 16:43 ` Grant Likely
@ 2011-06-21 11:26 ` Thomas Abraham
2011-06-21 11:27 ` Mark Brown
2011-06-22 16:22 ` Thomas Abraham
2 siblings, 0 replies; 21+ messages in thread
From: Thomas Abraham @ 2011-06-21 11:26 UTC (permalink / raw)
To: linux-arm-kernel
Hi Grant,
On 20 June 2011 22:13, Grant Likely <grant.likely@secretlab.ca> wrote:
>
> For custom properties, you should prefix the property name with 'samsung,'.
>
> This looks very much like directly encoding the Linux flags into the
> device tree. ?The binding should be completely contained within
> itself, and not refer to OS implementation details. ?It is fine to use
> the same values that Linux happens to use, but they need to still be
> explicitly documented as to what they mean. ?Also, a 'flags' property
> usually isn't very friendly to mere-mortals when the explicit
> behaviour can be enabled with the presence of a named property. ?For
> example; something like a "samsung,uart-has-rtscts" to enable rts/cts.
I will ensure that the next version of the patch do not have any linux
specific bindings.
>
>> +
>> +- has_fracval : Set to 1, if the controller supports fractional part of
>> + ? ? ? for the baud divisor, otherwise, set to 0.
>
> Boolean stuff often doesn't need a value. ?If the property is present,
> it is a '1'. ?If it isn't, then it is a '0'.
>
>> +
>> +- ucon_default : Default board specific setting of UCON register.
>> +
>> +- ulcon_default : Default board specific setting of ULCON register.
>> +
>> +- ufcon_default : Default board specific setting of UFCON register.
>
> I think I've commented on this before, but I do try to avoid direct
> coding registers into the DT. ?That said, sometimes there really isn't
> a nice human-friendly way of encoding things and direct register
> values is the best approach.
Instead of default register values, is it acceptable to include custom
properties like "samsung,txfifo-trig-level" and then convert it to
corresponding register settings?
>
>> +
>> +- uart_clksrc : One or more child nodes representing the clock sources that
>> + ? ? ? could be used to derive the buad rate. Each of these child nodes
>> + ? ? ? has four required properties.
>> +
>> + ? ? ? - name : Name of the parent clock.
>> + ? ? ? - divisor : divisor from the clock to the uart controller.
>> + ? ? ? - min_baud : Minimum baud rate for which this clock can be used.
>> + ? ? ? ? ? ? ? ? ? ? ? Set to zero, if there is no limitation.
>> + ? ? ? - max_buad : Maximum baud rate for which this clock can be used.
>
> typo: s/buad/baud/
>
>> + ? ? ? ? ? ? ? ? ? ? ? Set to zero, if there is no limitation.
>
> This looks to be directly encoding the current Linux implementation
> details into the device tree (it is a direct copy of the config
> structure members), and it doesn't use the common clock binding. ?It's
> fine to use sub nodes for each clock attachment, particularly because
> it looks like the uart is able to apply it's own divisor to the clock
> input, but I would definitely encode the data using the existing
> struct clock binding.
>
>> +
>> +Optional properties:
>> +- fifosize: Size of the tx/rx fifo used in the controller. If not specified,
>> + ? ? ? the default value of the fifosize is 16.
>> diff --git a/drivers/tty/serial/s5pv210.c b/drivers/tty/serial/s5pv210.c
>> index 3b2021a..9aacbda 100644
>> --- a/drivers/tty/serial/s5pv210.c
>> +++ b/drivers/tty/serial/s5pv210.c
>> @@ -18,6 +18,7 @@
>> ?#include <linux/init.h>
>> ?#include <linux/serial_core.h>
>> ?#include <linux/serial.h>
>> +#include <linux/of.h>
>>
>> ?#include <asm/irq.h>
>> ?#include <mach/hardware.h>
>> @@ -131,15 +132,39 @@ static struct s3c24xx_uart_info *s5p_uart_inf[] = {
>> ?/* device management */
>> ?static int s5p_serial_probe(struct platform_device *pdev)
>> ?{
>> - ? ? ? return s3c24xx_serial_probe(pdev, s5p_uart_inf[pdev->id]);
>> + ? ? ? const void *prop;
>> + ? ? ? unsigned int port = pdev->id;
>> + ? ? ? unsigned int fifosize = 16;
>> + ? ? ? static unsigned int probe_index;
>> +
>> + ? ? ? if (pdev->dev.of_node) {
>> + ? ? ? ? ? ? ? prop = of_get_property(pdev->dev.of_node, "fifosize", NULL);
>> + ? ? ? ? ? ? ? if (prop)
>> + ? ? ? ? ? ? ? ? ? ? ? fifosize = be32_to_cpu(*(u32 *)prop);
>
> Okay, this is getting ugly (not your fault, but this pattern has
> become too common. ?Can you craft and post a patch that adds the
> following functions to drivers/of/base.c and include/linux/of.h
>
> /* offset in cells, not bytes */
> int dt_decode_u32(struct *property, int offset, u32 *out_val)
> {
> ? ? ? ?if (!property || !property->value)
> ? ? ? ? ? ? ? ?return -EINVAL;
> ? ? ? ?if ((offset + 1) * 4 > property->length)
> ? ? ? ? ? ? ? ?return -EINVAL;
> ? ? ? ?*out_val = of_read_number(property->value + (offset * 4), 1);
> ? ? ? ?return 0;
> }
> int dt_decode_u64(struct *property, int offset, u64 *out_val)
> {
> ...
> }
> int dt_decode_string(struct *property, int index, char **out_string);
> {
> ...
> }
>
> Plus a set of companion functions:
> int dt_getprop_u32(struct device_node *np, char *propname, int offset,
> u32 *out_val)
> {
> ? ? ? ?return dt_decode_u32(of_find_property(np, propname, NULL),
> offset, out_val);
> }
> int dt_getprop_u64(struct *device_node, char *propname, int offset,
> u64 *out_val);
> {
> ...
> }
> int dt_getprop_string(struct *device_node, char *propname, int index,
> char **out_string);
> {
> ...
> }
>
> Then you'll be able to simply do the following to decode each
> property, with fifosize being left alone if the property cannot be
> found or decoded:
>
> dt_getprop_u32(pdev->dev.of_node, "samsung,fifosize", &fifosize);
Sure. I will write the above listed functions and submit a patch.
Thanks,
Thomas.
^ permalink raw reply [flat|nested] 21+ messages in thread* [PATCH 2/6] serial: samsung: Add device tree support for s5pv210 uart driver
2011-06-20 16:43 ` Grant Likely
2011-06-21 11:26 ` Thomas Abraham
@ 2011-06-21 11:27 ` Mark Brown
2011-06-22 16:22 ` Thomas Abraham
2 siblings, 0 replies; 21+ messages in thread
From: Mark Brown @ 2011-06-21 11:27 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Jun 20, 2011 at 10:43:50AM -0600, Grant Likely wrote:
> I think I've commented on this before, but I do try to avoid direct
> coding registers into the DT. That said, sometimes there really isn't
> a nice human-friendly way of encoding things and direct register
> values is the best approach.
Hrm, that's going to mean a *lot* of code doing parsing, especially if
we also follow through and also have proper parsers for all the
bitfields and don't just push the magic numbers down a level. In
principal I agree with you that that's what we should be doing but in
practice it seems like an awful lot of effort on all sides.
I'm not against it but if we're going to go down this road I think we
need to put some work into helpers to cut down on the parsing code. The
obvious one is a helper which maps a table of strings into numbers for
things like selecting multi-function pin functions.
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 2/6] serial: samsung: Add device tree support for s5pv210 uart driver
2011-06-20 16:43 ` Grant Likely
2011-06-21 11:26 ` Thomas Abraham
2011-06-21 11:27 ` Mark Brown
@ 2011-06-22 16:22 ` Thomas Abraham
2011-06-23 20:08 ` Grant Likely
2 siblings, 1 reply; 21+ messages in thread
From: Thomas Abraham @ 2011-06-22 16:22 UTC (permalink / raw)
To: linux-arm-kernel
Hi Grant,
On 20 June 2011 22:13, Grant Likely <grant.likely@secretlab.ca> wrote:
> Okay, this is getting ugly (not your fault, but this pattern has
> become too common. ?Can you craft and post a patch that adds the
> following functions to drivers/of/base.c and include/linux/of.h
>
> /* offset in cells, not bytes */
> int dt_decode_u32(struct *property, int offset, u32 *out_val)
> {
> ? ? ? ?if (!property || !property->value)
> ? ? ? ? ? ? ? ?return -EINVAL;
> ? ? ? ?if ((offset + 1) * 4 > property->length)
> ? ? ? ? ? ? ? ?return -EINVAL;
> ? ? ? ?*out_val = of_read_number(property->value + (offset * 4), 1);
> ? ? ? ?return 0;
> }
> int dt_decode_u64(struct *property, int offset, u64 *out_val)
> {
> ...
> }
> int dt_decode_string(struct *property, int index, char **out_string);
> {
> ...
> }
>
> Plus a set of companion functions:
> int dt_getprop_u32(struct device_node *np, char *propname, int offset,
> u32 *out_val)
> {
> ? ? ? ?return dt_decode_u32(of_find_property(np, propname, NULL),
> offset, out_val);
> }
> int dt_getprop_u64(struct *device_node, char *propname, int offset,
> u64 *out_val);
> {
> ...
> }
> int dt_getprop_string(struct *device_node, char *propname, int index,
> char **out_string);
> {
> ...
> }
>
> Then you'll be able to simply do the following to decode each
> property, with fifosize being left alone if the property cannot be
> found or decoded:
>
> dt_getprop_u32(pdev->dev.of_node, "samsung,fifosize", &fifosize);
I have added the functions as you have suggested and the diff is
listed below. Could you please review the diff and suggest any changes
required.
drivers/of/base.c | 129 ++++++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/of.h | 10 ++++
2 files changed, 139 insertions(+), 0 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 632ebae..73f0144 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -596,6 +596,135 @@ struct device_node
*of_find_node_by_phandle(phandle handle)
EXPORT_SYMBOL(of_find_node_by_phandle);
/**
+ * of_read_property_u32 - Reads a indexed 32-bit property value
+ * @prop: property to read from.
+ * @offset: cell number to read.
+ * value: returned cell value
+ *
+ * Returns a indexed 32-bit value of a property cell, -EINVAL in case the cell
+ * does not exist
+ */
+int of_read_property_u32(struct property *prop, u32 offset, u32 *value)
+{
+ if (!prop || !prop->value)
+ return -EINVAL;
+ if ((offset + 1) * 4 > prop->length)
+ return -EINVAL;
+
+ *value = of_read_ulong(prop->value + (offset * 4), 1);
+ return 0;
+}
+EXPORT_SYMBOL(of_read_property_u32);
+
+/**
+ * of_getprop_u32 - Find a property in a device node and read a 32-bit value
+ * @np: device node from which the property value is to be read.
+ * @propname name of the property to be searched.
+ * @offset: cell number to read.
+ * @value: returned value of the cell
+ *
+ * Search for a property in a device node and read a indexed 32-bit value of a
+ * property cell. Returns the 32-bit cell value, -EINVAL in case the
property or
+ * the indexed cell does not exist.
+ */
+int
+of_getprop_u32(struct device_node *np, char *propname, int offset, u32 *value)
+{
+ return of_read_property_u32(of_find_property(np, propname, NULL),
+ offset, value);
+}
+EXPORT_SYMBOL(of_getprop_u32);
+
+/**
+ * of_read_property_u64 - Reads a indexed 64-bit property value
+ * @prop: property to read from.
+ * @offset: cell number to read (each cell is 64-bits).
+ * @value: returned cell value
+ *
+ * Returns a indexed 64-bit value of a property cell, -EINVAL in case the cell
+ * does not exist
+ */
+int of_read_property_u64(struct property *prop, int offset, u64 *value)
+{
+ if (!prop || !prop->value)
+ return -EINVAL;
+ if ((offset + 1) * 8 > prop->length)
+ return -EINVAL;
+
+ *value = of_read_number(prop->value + (offset * 8), 2);
+ return 0;
+}
+EXPORT_SYMBOL(of_read_property_u64);
+
+/**
+ * of_getprop_u64 - Find a property in a device node and read a 64-bit value
+ * @np: device node from which the property value is to be read.
+ * @propname name of the property to be searched.
+ * @offset: cell number to read (each cell is 64-bits).
+ * @value: returned value of the cell
+ *
+ * Search for a property in a device node and read a indexed 64-bit value of a
+ * property cell. Returns the 64-bit cell value, -EINVAL in case the
property or
+ * the indexed cell does not exist.
+ */
+int
+of_getprop_u64(struct device_node *np, char *propname, int offset, u64 *value)
+{
+ return of_read_property_u64(of_find_property(np, propname, NULL),
+ offset, value);
+}
+EXPORT_SYMBOL(of_getprop_u64);
+
+/**
+ * of_read_property_string - Returns a pointer to a indexed null terminated
+ * property value string
+ * @prop: property to read from.
+ * @offset: index of the property string to be read.
+ * @string: pointer to a null terminated string, valid only if the return
+ * value is 0.
+ *
+ * Returns a pointer to a indexed null terminated property cell string, -EINVAL
+ * in case the cell does not exist.
+ */
+int of_read_property_string(struct property *prop, int offset, char **string)
+{
+ char *c;
+
+ if (!prop || !prop->value)
+ return -EINVAL;
+
+ c = (char *)prop->value;
+ while (offset--)
+ while (*c++)
+ ;
+
+ *string = c;
+ return 0;
+}
+
+/**
+ * of_getprop_string - Find a property in a device node and read a null
+ * terminated string property
+ * @np: device node from which the property value is to be read.
+ * @propname name of the property to be searched.
+ * @offset: cell number to read (each cell contains a null-terminated
+ * string).
+ * @string: pointer to a null terminated string, valid only if the return
+ * value is 0.
+ *
+ * Search for a property in a device node and return a pointer to a indexed
+ * string value of a property cell. Returns a pointer to a string, -EINVAL
+ * in case the property or the indexed cell does not exist.
+ */
+int of_getprop_string(struct device_node *np, char *propname, int offset,
+ char **string)
+{
+ return of_read_property_string(of_find_property(np, propname, NULL),
+ offset, string);
+}
+EXPORT_SYMBOL(of_getprop_string);
+
+/**
* of_parse_phandle - Resolve a phandle property to a device_node pointer
* @np: Pointer to device node holding phandle property
* @phandle_name: Name of property holding a phandle value
diff --git a/include/linux/of.h b/include/linux/of.h
index bfc0ed1..aff2786 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -195,6 +195,16 @@ extern struct device_node *of_find_node_with_property(
extern struct property *of_find_property(const struct device_node *np,
const char *name,
int *lenp);
+extern int of_read_property_u32(struct property *prop, u32 offset, u32 *value);
+extern int of_getprop_u32(struct device_node *np, char *propname, int offset,
+ u32 *value);
+extern int of_read_property_u64(struct property *prop, int offset, u64 *value);
+extern int of_getprop_u64(struct device_node *np, char *propname, int offset,
+ u64 *value);
+extern int of_read_property_string(struct property *prop, int offset,
+ char **string);
+extern int of_getprop_string(struct device_node *np, char *propname,
int offset,
+ char **string);
extern int of_device_is_compatible(const struct device_node *device,
const char *);
extern int of_device_is_available(const struct device_node *device);
--
1.6.6.rc2
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 2/6] serial: samsung: Add device tree support for s5pv210 uart driver
2011-06-22 16:22 ` Thomas Abraham
@ 2011-06-23 20:08 ` Grant Likely
2011-06-24 12:27 ` Thomas Abraham
0 siblings, 1 reply; 21+ messages in thread
From: Grant Likely @ 2011-06-23 20:08 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Jun 22, 2011 at 10:22 AM, Thomas Abraham
<thomas.abraham@linaro.org> wrote:
>
> I have added the functions as you have suggested and the diff is
> listed below. Could you please review the diff and suggest any changes
> required.
Thanks Thomas. Comments below...
> ?drivers/of/base.c ?| ?129 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> ?include/linux/of.h | ? 10 ++++
> ?2 files changed, 139 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 632ebae..73f0144 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -596,6 +596,135 @@ struct device_node
> *of_find_node_by_phandle(phandle handle)
> ?EXPORT_SYMBOL(of_find_node_by_phandle);
>
> ?/**
> + * of_read_property_u32 - Reads a indexed 32-bit property value
> + * @prop: ? ? ?property to read from.
> + * @offset: ? ?cell number to read.
> + * value: ? ? ?returned cell value
> + *
> + * Returns a indexed 32-bit value of a property cell, -EINVAL in case the cell
> + * does not exist
> + */
> +int of_read_property_u32(struct property *prop, u32 offset, u32 *value)
> +{
> + ? ? ? if (!prop || !prop->value)
> + ? ? ? ? ? ? ? return -EINVAL;
Hmmm, it would probably be a good idea to differentiate return code
depending on whether or not the property was found vs. the property
data not large enough.
> + ? ? ? if ((offset + 1) * 4 > prop->length)
> + ? ? ? ? ? ? ? return -EINVAL;
> +
> + ? ? ? *value = of_read_ulong(prop->value + (offset * 4), 1);
> + ? ? ? return 0;
Despite the fact that this is exactly what I asked you to write, this
ends up being rather ugly. (I originally put in the '*4' to match the
behaviour of the existing of_read_number(), but that was a mistake.
tglx also pointed it out). How about this instead:
int of_read_property_u32(struct property *prop, unsigned int offset,
u32 *out_value)
{
if (!prop || !out_value)
return -EINVAL;
if (!prop->value)
return -ENODATA;
if (offset + sizeof(*out_value) > prop->length)
return -EOVERFLOW;
*out_value = be32_to_cpup(prop->data + offset);
return 0;
}
int of_read_property_u64(struct property *prop, unsigned int offset,
u64 *out_value)
{
if (!prop || !out_value)
return -EINVAL;
if (!prop->value)
return -ENODATA;
if (offset + sizeof(*out_value) > prop->length)
return -EOVERFLOW;
*out_value = be32_to_cpup(prop->value + offset);
*out_value = (*out_value << 32) | be32_to_cpup(prop->value +
offset + sizeof(u32));
return 0;
}
> +}
> +EXPORT_SYMBOL(of_read_property_u32);
EXPORT_SYMBOL_GPL()
> +
> +/**
> + * of_getprop_u32 - Find a property in a device node and read a 32-bit value
> + * @np: ? ? ? ? ? ? ? ?device node from which the property value is to be read.
> + * @propname ? name of the property to be searched.
> + * @offset: ? ?cell number to read.
> + * @value: ? ? returned value of the cell
> + *
> + * Search for a property in a device node and read a indexed 32-bit value of a
> + * property cell. Returns the 32-bit cell value, -EINVAL in case the
> property or
> + * the indexed cell does not exist.
> + */
> +int
> +of_getprop_u32(struct device_node *np, char *propname, int offset, u32 *value)
> +{
> + ? ? ? return of_read_property_u32(of_find_property(np, propname, NULL),
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? offset, value);
> +}
> +EXPORT_SYMBOL(of_getprop_u32);
> +
> +/**
> + * of_read_property_u64 - Reads a indexed 64-bit property value
> + * @prop: ? ? ?property to read from.
> + * @offset: ? ?cell number to read (each cell is 64-bits).
> + * @value: ? ? returned cell value
> + *
> + * Returns a indexed 64-bit value of a property cell, -EINVAL in case the cell
> + * does not exist
> + */
> +int of_read_property_u64(struct property *prop, int offset, u64 *value)
> +{
> + ? ? ? if (!prop || !prop->value)
> + ? ? ? ? ? ? ? return -EINVAL;
> + ? ? ? if ((offset + 1) * 8 > prop->length)
> + ? ? ? ? ? ? ? return -EINVAL;
> +
> + ? ? ? *value = of_read_number(prop->value + (offset * 8), 2);
> + ? ? ? return 0;
> +}
> +EXPORT_SYMBOL(of_read_property_u64);
> +
> +/**
> + * of_getprop_u64 - Find a property in a device node and read a 64-bit value
> + * @np: ? ? ? ? ? ? ? ?device node from which the property value is to be read.
> + * @propname ? name of the property to be searched.
> + * @offset: ? ?cell number to read (each cell is 64-bits).
> + * @value: ? ? returned value of the cell
> + *
> + * Search for a property in a device node and read a indexed 64-bit value of a
> + * property cell. Returns the 64-bit cell value, -EINVAL in case the
> property or
> + * the indexed cell does not exist.
> + */
> +int
> +of_getprop_u64(struct device_node *np, char *propname, int offset, u64 *value)
> +{
> + ? ? ? return of_read_property_u64(of_find_property(np, propname, NULL),
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? offset, value);
> +}
> +EXPORT_SYMBOL(of_getprop_u64);
> +
> +/**
> + * of_read_property_string - Returns a pointer to a indexed null terminated
> + * ? ? ? ? ? ? ? ? ? ? ? ? ? ? property value string
> + * @prop: ? ? ?property to read from.
> + * @offset: ? ?index of the property string to be read.
> + * @string: ? ?pointer to a null terminated string, valid only if the return
> + * ? ? ? ? ? ? value is 0.
> + *
> + * Returns a pointer to a indexed null terminated property cell string, -EINVAL
> + * in case the cell does not exist.
> + */
> +int of_read_property_string(struct property *prop, int offset, char **string)
> +{
> + ? ? ? char *c;
> +
> + ? ? ? if (!prop || !prop->value)
> + ? ? ? ? ? ? ? return -EINVAL;
Ditto here about return values.
> +
> + ? ? ? c = (char *)prop->value;
You don't need the cast. prop->value is a void* pointer. However,
'c' does need to be const char.
> + ? ? ? while (offset--)
> + ? ? ? ? ? ? ? while (*c++)
> + ? ? ? ? ? ? ? ? ? ? ? ;
Rather than open coding this, you should use the string library
functions. Something like:
c = prop->value;
while (offset-- && (c - prop->value) < prop->size)
c += strlen(c) + 1;
if ((c - prop->value) + strlen(c) >= prop->size)
return -EOVERFLOW;
*out_value = c;
Plus it catches more error conditions that way.
g.
^ permalink raw reply [flat|nested] 21+ messages in thread* [PATCH 2/6] serial: samsung: Add device tree support for s5pv210 uart driver
2011-06-23 20:08 ` Grant Likely
@ 2011-06-24 12:27 ` Thomas Abraham
2011-06-26 23:27 ` Grant Likely
0 siblings, 1 reply; 21+ messages in thread
From: Thomas Abraham @ 2011-06-24 12:27 UTC (permalink / raw)
To: linux-arm-kernel
Hi Grant,
On 24 June 2011 01:38, Grant Likely <grant.likely@secretlab.ca> wrote:
> Despite the fact that this is exactly what I asked you to write, this
> ends up being rather ugly. ?(I originally put in the '*4' to match the
> behaviour of the existing of_read_number(), but that was a mistake.
> tglx also pointed it out). ?How about this instead:
Your draft implementation below is suggesting that the offset should
be in bytes and not cells and so maybe you are suggesting the new
approach to support multi-format property values. I have changed the
implementation as per your code below.
>
> int of_read_property_u32(struct property *prop, unsigned int offset,
> u32 *out_value)
> {
> ? ? ? ?if (!prop || !out_value)
> ? ? ? ? ? ? ? ?return -EINVAL;
> ? ? ? ?if (!prop->value)
> ? ? ? ? ? ? ? ?return -ENODATA;
> ? ? ? ?if (offset + sizeof(*out_value) > prop->length)
> ? ? ? ? ? ? ? ?return -EOVERFLOW;
> ? ? ? ?*out_value = be32_to_cpup(prop->data + offset);
> ? ? ? ?return 0;
> }
[...]
>> +/**
>> + * of_read_property_string - Returns a pointer to a indexed null terminated
>> + * ? ? ? ? ? ? ? ? ? ? ? ? ? ? property value string
>> + * @prop: ? ? ?property to read from.
>> + * @offset: ? ?index of the property string to be read.
>> + * @string: ? ?pointer to a null terminated string, valid only if the return
>> + * ? ? ? ? ? ? value is 0.
>> + *
>> + * Returns a pointer to a indexed null terminated property cell string, -EINVAL
>> + * in case the cell does not exist.
>> + */
>> +int of_read_property_string(struct property *prop, int offset, char **string)
>> +{
>> + ? ? ? char *c;
>> +
>> + ? ? ? if (!prop || !prop->value)
>> + ? ? ? ? ? ? ? return -EINVAL;
>
> Ditto here about return values.
>
>> +
>> + ? ? ? c = (char *)prop->value;
>
> You don't need the cast. ?prop->value is a void* pointer. ?However,
> 'c' does need to be const char.
>
>> + ? ? ? while (offset--)
>> + ? ? ? ? ? ? ? while (*c++)
>> + ? ? ? ? ? ? ? ? ? ? ? ;
>
> Rather than open coding this, you should use the string library
> functions. ?Something like:
>
> ? ? ? ?c = prop->value;
> ? ? ? ?while (offset-- && (c - prop->value) < prop->size)
> ? ? ? ? ? ? ? ?c += strlen(c) + 1;
> ? ? ? ?if ((c - prop->value) + strlen(c) >= prop->size)
> ? ? ? ? ? ? ? ?return -EOVERFLOW;
> ? ? ? ?*out_value = c;
>
> Plus it catches more error conditions that way.
If we change the offset to represent bytes and not cell index in the
u32 and u64 versions, shouldn't offset represent bytes for the string
version as well? In case of multi-format property values, there could
be u32 or u64 values intermixed with string values. So, some
modifications have been made to your above implementation of the
string version.
The new diff is listed below. Would there be any changes required. If
this is acceptable, I will submit a separate patch.
drivers/of/base.c | 142 ++++++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/of.h | 12 ++++
2 files changed, 154 insertions(+), 0 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 632ebae..e9acbea 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -596,6 +596,148 @@ struct device_node
*of_find_node_by_phandle(phandle handle)
EXPORT_SYMBOL(of_find_node_by_phandle);
/**
+ * of_read_property_u32 - Reads a 32-bit property value
+ * @prop: property to read from.
+ * @offset: offset into property value to read from (in bytes).
+ * @out_value: returned value.
+ *
+ * Returns a 32-bit value from a property. Returns -EINVAL, if the
property does
+ * not exist, -ENODATA, if property does not have a value, -EOVERFLOW, if
+ * reading from offset overshoots the length of the property.
+ */
+int of_read_property_u32(struct property *prop, u32 offset, u32 *out_value)
+{
+ if (!prop || !out_value)
+ return -EINVAL;
+ if (!prop->value)
+ return -ENODATA;
+ if (offset + sizeof(*out_value) > prop->length)
+ return -EOVERFLOW;
+
+ *out_value = be32_to_cpup(prop->value + offset);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(of_read_property_u32);
+
+/**
+ * of_getprop_u32 - Find a property in a device node and read a 32-bit value
+ * @np: device node from which the property value is to be read.
+ * @propname name of the property to be searched.
+ * @offset: offset into property value to read from (in bytes).
+ * @out_value: returned value.
+ *
+ * Search for a property in a device node and read a 32-bit value from a
+ * property. Returns -EINVAL, if the property does not exist, -ENODATA, if
+ * property does not have a value, -EOVERFLOW, if reading from offset
overshoots
+ * the length of the property.
+ */
+int of_getprop_u32(struct device_node *np, char *propname, int offset,
+ u32 *out_value)
+{
+ return of_read_property_u32(of_find_property(np, propname, NULL),
+ offset, out_value);
+}
+EXPORT_SYMBOL_GPL(of_getprop_u32);
+
+/**
+ * of_read_property_u64 - Reads a 64-bit property value
+ * @prop: property to read from.
+ * @offset: offset into property value to read from (in bytes).
+ * @out_value: returned value.
+ *
+ * Returns a 64-bit value from a property. Returns -EINVAL, if the
property does
+ * not exist, -ENODATA, if property does not have a value, -EOVERFLOW, if
+ * reading from offset overshoots the length of the property.
+ */
+int of_read_property_u64(struct property *prop, int offset, u64 *out_value)
+{
+ if (!prop || !out_value)
+ return -EINVAL;
+ if (!prop->value)
+ return -ENODATA;
+ if (offset + sizeof(*out_value) > prop->length)
+ return -EOVERFLOW;
+ *out_value = be32_to_cpup(prop->value + offset);
+ *out_value = (*out_value << 32) |
+ be32_to_cpup(prop->value + offset + sizeof(u32));
+ return 0;
+}
+EXPORT_SYMBOL_GPL(of_read_property_u64);
+
+/**
+ * of_getprop_u64 - Find a property in a device node and read a 64-bit value
+ * @np: device node from which the property value is to be read.
+ * @propname name of the property to be searched.
+ * @offset: offset into property value to read from (in bytes).
+ * @out_value: returned value.
+ *
+ * Search for a property in a device node and read a 64-bit value from a
+ * property. Returns -EINVAL, if the property does not exist, -ENODATA, if
+ * property does not have a value, -EOVERFLOW, if reading from offset
overshoots
+ * the length of the property.
+ */
+int of_getprop_u64(struct device_node *np, char *propname, int offset,
+ u64 *out_value)
+{
+ return of_read_property_u64(of_find_property(np, propname, NULL),
+ offset, out_value);
+}
+EXPORT_SYMBOL_GPL(of_getprop_u64);
+
+/**
+ * of_read_property_string - Returns a pointer to a null terminated
+ * property string value
+ * @prop: property to read from.
+ * @offset: offset into property value to read from (in bytes).
+ * @string: pointer to a null terminated string, valid only if the return
+ * value is 0.
+ *
+ * Returns a pointer to a null terminated property string value.
Returns -EINVAL,
+ * if the property does not exist, -ENODATA, if property does not have a value,
+ * -EOVERFLOW, if the offset overshoots the length of the property, -EILSEQ, if
+ * the string is not null-terminated within the length of the property.
+ */
+int
+of_read_property_string(struct property *prop, int offset, char **out_string)
+{
+ if (!prop || !out_string)
+ return -EINVAL;
+ if (!prop->value)
+ return -ENODATA;
+ if (offset >= prop->length)
+ return -EOVERFLOW;
+ if (strnlen(prop->value + offset, prop->length - offset)
+ + offset == prop->length)
+ return -EILSEQ;
+ *out_string = prop->value + offset;
+ return 0;
+}
+EXPORT_SYMBOL_GPL(of_read_property_string);
+
+/**
+ * of_getprop_string - Find a property in a device node and read a null
+ * terminated string value
+ * @np: device node from which the property value is to be read.
+ * @propname name of the property to be searched.
+ * @offset: offset into property value to read from (in bytes).
+ * @out_string: pointer to a null terminated string, valid only if the return
+ * value is 0.
+ *
+ * Search for a property in a device node and return a pointer to a string
+ * value of a property. Returns -EINVAL, if the property does not exist,
+ * -ENODATA, if property does not have a value, -EOVERFLOW, if the offset
+ * overshoots the length of the property, -EILSEQ, if the string is not
+ * null-terminated within the length of the property.
+ */
+int of_getprop_string(struct device_node *np, char *propname, int offset,
+ char **out_string)
+{
+ return of_read_property_string(of_find_property(np, propname, NULL),
+ offset, out_string);
+}
+EXPORT_SYMBOL_GPL(of_getprop_string);
+
+/**
* of_parse_phandle - Resolve a phandle property to a device_node pointer
* @np: Pointer to device node holding phandle property
* @phandle_name: Name of property holding a phandle value
diff --git a/include/linux/of.h b/include/linux/of.h
index bfc0ed1..f5c1065 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -195,6 +195,18 @@ extern struct device_node *of_find_node_with_property(
extern struct property *of_find_property(const struct device_node *np,
const char *name,
int *lenp);
+extern int of_read_property_u32(struct property *prop, u32 offset,
+ u32 *out_value);
+extern int of_getprop_u32(struct device_node *np, char *propname, int offset,
+ u32 *out_value);
+extern int of_read_property_u64(struct property *prop, int offset,
+ u64 *out_value);
+extern int of_getprop_u64(struct device_node *np, char *propname, int offset,
+ u64 *out_value);
+extern int of_read_property_string(struct property *prop, int offset,
+ char **out_string);
+extern int of_getprop_string(struct device_node *np, char *propname,
int offset,
+ char **out_string);
extern int of_device_is_compatible(const struct device_node *device,
const char *);
extern int of_device_is_available(const struct device_node *device);
Thanks,
Thomas.
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 2/6] serial: samsung: Add device tree support for s5pv210 uart driver
2011-06-24 12:27 ` Thomas Abraham
@ 2011-06-26 23:27 ` Grant Likely
0 siblings, 0 replies; 21+ messages in thread
From: Grant Likely @ 2011-06-26 23:27 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Jun 24, 2011 at 6:27 AM, Thomas Abraham
<thomas.abraham@linaro.org> wrote:
> Hi Grant,
>
> On 24 June 2011 01:38, Grant Likely <grant.likely@secretlab.ca> wrote:
>> Despite the fact that this is exactly what I asked you to write, this
>> ends up being rather ugly. ?(I originally put in the '*4' to match the
>> behaviour of the existing of_read_number(), but that was a mistake.
>> tglx also pointed it out). ?How about this instead:
>
> Your draft implementation below is suggesting that the offset should
> be in bytes and not cells and so maybe you are suggesting the new
> approach to support multi-format property values. I have changed the
> implementation as per your code below.
I've been going back and forth on this. The offset is most flexible
if it is in bytes, but most DT data is organized into cells, so a byte
offset is a little intuitive. For string properties, it really
doesn't make sense to have the offset in bytes because the offset
generally either cannot be known until after reading earlier values in
the property, or is meaningless without the earlier data in the case
of mixed value properties. However, I think I've gotten caught up
into a case of feature creep on these functions and I've tried to make
them as flexible as possible. The reality is that it will almost
never be useful to obtain only the 2nd or 3rd cell in a property. In
those cases, the driver probably needs to parse all the data in the
property, and therefore it is better to obtain a pointer to the entire
data block for parsing instead of searching for the property over and
over again (which is what of_getprop_u32() will do).
So, I'm changing the design (for the last time, I promise!). Rather
than trying to solve some future theoretical use case, the functions
should focus on what is actually needed immediately. Let's drop the
u64 version, and only implement of_getprop_u32() and
of_getprop_string(). u64 can always be added at a later date if we
need it. Also, we'll drop the offset parameter because I don't
foresee any situation where it is the right thing to do. Something
like this (modifying your latest code):
/**
* of_property_read_u32 - Find a property in a device node and read a
32-bit value
* @np: ? ? ? ? ? ? ? ?device node from which the property value is to be read.
* @propname: ? name of the property to be searched.
* @out_value: returned value.
*
* Search for a property in a device node and read a 32-bit value from a
* property. Returns -EINVAL, if the property does not exist, -ENODATA, if
* property does not have a value, -EOVERFLOW, if the property data isn't large
* enough, and 0 on success.
*
* The out_value is only modified if a valid u32 can be decoded.
*/
int of_property_read_u32(struct device_node *np, char *propname, u32 *out_value)
{
? ? ? struct property *prop = of_find_property(np, propname, NULL),
? ? ? if (!prop)
? ? ? ? ? ? ? return -EINVAL;
? ? ? if (!prop->value)
? ? ? ? ? ? ? return -ENODATA;
? ? ? if (sizeof(*out_value) > prop->length)
? ? ? ? ? ? ? return -EOVERFLOW;
? ? ? *out_value = be32_to_cpup(prop->value);
? ? ? return 0;
}
EXPORT_SYMBOL_GPL(of_property_read_u32);
/**
* of_property_read_u32 - Find a property in a device node and read a
32-bit value
* @np: ? ? ? ? ? ? ? ?device node from which the property value is to be read.
* @propname: ? name of the property to be searched.
* @out_string: ? ?pointer to a null terminated string, valid only if the return
* ? ? ? ? ? ? value is 0.
*
* Returns a pointer to a null terminated property string value.
Returns -EINVAL,
* if the property does not exist, -ENODATA, if property does not have a value,
* -EILSEQ, if the string is not null-terminated within the length of
the property.
*
* The out_string value is only modified if a valid string can be decoded.
*/
int of_property_read_u32(struct device_node *np, char *propname, char
**out_string)
{
? ? ? struct property *prop = of_find_property(np, propname, NULL),
? ? ? if (!prop)
? ? ? ? ? ? ? return -EINVAL;
? ? ? if (!prop->value)
? ? ? ? ? ? ? return -ENODATA;
? ? ? if (strnlen(prop->value, prop->length) == prop->length)
? ? ? ? ? ? ? return -EILSEQ;
? ? ? *out_string = prop->value;
? ? ? return 0;
}
EXPORT_SYMBOL_GPL(of_property_read_u32);
I think overall this will be the most useful, and it can always be
expanded at a later date if more use cases show up.
g.
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 3/6] watchdog: s3c2410: Add support for device tree based probe
2011-06-20 11:02 [PATCH 0/6] Add basic device tree support for Samsung's Exynos4 platform Thomas Abraham
2011-06-20 11:02 ` [PATCH 1/6] serial: samsung: Keep a copy of platform data in driver's private data Thomas Abraham
2011-06-20 11:02 ` [PATCH 2/6] serial: samsung: Add device tree support for s5pv210 uart driver Thomas Abraham
@ 2011-06-20 11:02 ` Thomas Abraham
2011-06-20 16:50 ` Grant Likely
2011-06-20 11:02 ` [PATCH 4/6] mmc: sdhci-s3c: " Thomas Abraham
` (2 subsequent siblings)
5 siblings, 1 reply; 21+ messages in thread
From: Thomas Abraham @ 2011-06-20 11:02 UTC (permalink / raw)
To: linux-arm-kernel
This patch adds the of_match_table to enable s3c2410-wdt driver
to be probed when watchdog device node is found in the device tree.
Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
---
.../devicetree/bindings/watchdog/samsung-wdt.txt | 12 ++++++++++++
drivers/watchdog/s3c2410_wdt.c | 10 ++++++++++
2 files changed, 22 insertions(+), 0 deletions(-)
create mode 100644 Documentation/devicetree/bindings/watchdog/samsung-wdt.txt
diff --git a/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt b/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt
new file mode 100644
index 0000000..f2617e8
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt
@@ -0,0 +1,12 @@
+* Samsung's Watchdog Timer Controller
+
+The Samsung's Watchdog controller is used for resuming system operation
+after a preset amount of time during which the WDT reset event has not
+occured.
+
+Required properties:
+- compatible : should be "samsung,s3c2410-wdt"
+- reg : base physical address of the controller and length of memory mapped
+ region.
+- interrupts : interrupt number to the cpu.
+
diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
index f7f5aa0..30da88f 100644
--- a/drivers/watchdog/s3c2410_wdt.c
+++ b/drivers/watchdog/s3c2410_wdt.c
@@ -589,6 +589,15 @@ static int s3c2410wdt_resume(struct platform_device *dev)
#define s3c2410wdt_resume NULL
#endif /* CONFIG_PM */
+#ifdef CONFIG_OF
+static const struct of_device_id s3c2410_wdt_match[] = {
+ { .compatible = "samsung,s3c2410-wdt" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, s3c2410_wdt_match);
+#else
+#define s3c2410_wdt_match NULL
+#endif
static struct platform_driver s3c2410wdt_driver = {
.probe = s3c2410wdt_probe,
@@ -599,6 +608,7 @@ static struct platform_driver s3c2410wdt_driver = {
.driver = {
.owner = THIS_MODULE,
.name = "s3c2410-wdt",
+ .of_match_table = s3c2410_wdt_match,
},
};
--
1.6.6.rc2
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 3/6] watchdog: s3c2410: Add support for device tree based probe
2011-06-20 11:02 ` [PATCH 3/6] watchdog: s3c2410: Add support for device tree based probe Thomas Abraham
@ 2011-06-20 16:50 ` Grant Likely
2011-06-22 9:05 ` Wim Van Sebroeck
0 siblings, 1 reply; 21+ messages in thread
From: Grant Likely @ 2011-06-20 16:50 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Jun 20, 2011 at 5:02 AM, Thomas Abraham
<thomas.abraham@linaro.org> wrote:
> This patch adds the of_match_table to enable s3c2410-wdt driver
> to be probed when watchdog device node is found in the device tree.
>
> Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
Acked-by: Grant Likely <grant.likely@secretlab.ca>
You need to send this to Wim and the linux-watchdog mailing lists. As
far as I'm concerned, it can be merged immediately.
g.
> ---
> ?.../devicetree/bindings/watchdog/samsung-wdt.txt ? | ? 12 ++++++++++++
> ?drivers/watchdog/s3c2410_wdt.c ? ? ? ? ? ? ? ? ? ? | ? 10 ++++++++++
> ?2 files changed, 22 insertions(+), 0 deletions(-)
> ?create mode 100644 Documentation/devicetree/bindings/watchdog/samsung-wdt.txt
>
> diff --git a/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt b/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt
> new file mode 100644
> index 0000000..f2617e8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt
> @@ -0,0 +1,12 @@
> +* Samsung's Watchdog Timer Controller
> +
> +The Samsung's Watchdog controller is used for resuming system operation
> +after a preset amount of time during which the WDT reset event has not
> +occured.
> +
> +Required properties:
> +- compatible : should be "samsung,s3c2410-wdt"
> +- reg : base physical address of the controller and length of memory mapped
> + ? ? ? region.
> +- interrupts : interrupt number to the cpu.
> +
> diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
> index f7f5aa0..30da88f 100644
> --- a/drivers/watchdog/s3c2410_wdt.c
> +++ b/drivers/watchdog/s3c2410_wdt.c
> @@ -589,6 +589,15 @@ static int s3c2410wdt_resume(struct platform_device *dev)
> ?#define s3c2410wdt_resume ?NULL
> ?#endif /* CONFIG_PM */
>
> +#ifdef CONFIG_OF
> +static const struct of_device_id s3c2410_wdt_match[] = {
> + ? ? ? { .compatible = "samsung,s3c2410-wdt" },
> + ? ? ? {},
> +};
> +MODULE_DEVICE_TABLE(of, s3c2410_wdt_match);
> +#else
> +#define s3c2410_wdt_match NULL
> +#endif
>
> ?static struct platform_driver s3c2410wdt_driver = {
> ? ? ? ?.probe ? ? ? ? ?= s3c2410wdt_probe,
> @@ -599,6 +608,7 @@ static struct platform_driver s3c2410wdt_driver = {
> ? ? ? ?.driver ? ? ? ? = {
> ? ? ? ? ? ? ? ?.owner ?= THIS_MODULE,
> ? ? ? ? ? ? ? ?.name ? = "s3c2410-wdt",
> + ? ? ? ? ? ? ? .of_match_table = s3c2410_wdt_match,
> ? ? ? ?},
> ?};
>
> --
> 1.6.6.rc2
>
>
> _______________________________________________
> linaro-dev mailing list
> linaro-dev at lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/linaro-dev
>
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply [flat|nested] 21+ messages in thread* [PATCH 3/6] watchdog: s3c2410: Add support for device tree based probe
2011-06-20 16:50 ` Grant Likely
@ 2011-06-22 9:05 ` Wim Van Sebroeck
0 siblings, 0 replies; 21+ messages in thread
From: Wim Van Sebroeck @ 2011-06-22 9:05 UTC (permalink / raw)
To: linux-arm-kernel
Hi Thomas, Grant,
> On Mon, Jun 20, 2011 at 5:02 AM, Thomas Abraham
> <thomas.abraham@linaro.org> wrote:
> > This patch adds the of_match_table to enable s3c2410-wdt driver
> > to be probed when watchdog device node is found in the device tree.
> >
> > Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
>
> Acked-by: Grant Likely <grant.likely@secretlab.ca>
>
> You need to send this to Wim and the linux-watchdog mailing lists. As
> far as I'm concerned, it can be merged immediately.
Yep, it looks OK. Please sent the patch so that it can be included.
Kind regards,
Wim.
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 4/6] mmc: sdhci-s3c: Add support for device tree based probe
2011-06-20 11:02 [PATCH 0/6] Add basic device tree support for Samsung's Exynos4 platform Thomas Abraham
` (2 preceding siblings ...)
2011-06-20 11:02 ` [PATCH 3/6] watchdog: s3c2410: Add support for device tree based probe Thomas Abraham
@ 2011-06-20 11:02 ` Thomas Abraham
2011-06-20 16:51 ` Grant Likely
2011-06-20 11:02 ` [PATCH 5/6] arm: dts: Add nodes in smdkv310 device tree source file Thomas Abraham
2011-06-20 11:02 ` [PATCH 6/6] arm: exynos4: Add a new Exynos4 device tree enabled machine Thomas Abraham
5 siblings, 1 reply; 21+ messages in thread
From: Thomas Abraham @ 2011-06-20 11:02 UTC (permalink / raw)
To: linux-arm-kernel
Add of_match_table to enable sdhci-s3c driver to be probed when a compatible
sdhci device node is found in device tree.
Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
---
This is temporary patch. sdhci-s3c driver has to be moved to sdhci-pltfm
based driver.
drivers/mmc/host/sdhci-s3c.c | 11 +++++++++++
1 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
index 69e3ee3..5ccbee0 100644
--- a/drivers/mmc/host/sdhci-s3c.c
+++ b/drivers/mmc/host/sdhci-s3c.c
@@ -629,6 +629,16 @@ static int sdhci_s3c_resume(struct platform_device *dev)
#define sdhci_s3c_resume NULL
#endif
+#ifdef CONFIG_OF
+static const struct of_device_id s3c_sdhci_match[] = {
+ { .compatible = "samsung,s3c-sdhci" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, s3c_sdhci_match);
+#else
+#define s3c_sdhci_match NULL
+#endif
+
static struct platform_driver sdhci_s3c_driver = {
.probe = sdhci_s3c_probe,
.remove = __devexit_p(sdhci_s3c_remove),
@@ -637,6 +647,7 @@ static struct platform_driver sdhci_s3c_driver = {
.driver = {
.owner = THIS_MODULE,
.name = "s3c-sdhci",
+ .of_match_table = s3c_sdhci_match,
},
};
--
1.6.6.rc2
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 4/6] mmc: sdhci-s3c: Add support for device tree based probe
2011-06-20 11:02 ` [PATCH 4/6] mmc: sdhci-s3c: " Thomas Abraham
@ 2011-06-20 16:51 ` Grant Likely
0 siblings, 0 replies; 21+ messages in thread
From: Grant Likely @ 2011-06-20 16:51 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Jun 20, 2011 at 5:02 AM, Thomas Abraham
<thomas.abraham@linaro.org> wrote:
> Add of_match_table to enable sdhci-s3c driver to be probed when a compatible
> sdhci device node is found in device tree.
>
> Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
> ---
> This is temporary patch. sdhci-s3c driver has to be moved to sdhci-pltfm
> based driver.
... and the binding needs to be documented. :-)
g.
>
> ?drivers/mmc/host/sdhci-s3c.c | ? 11 +++++++++++
> ?1 files changed, 11 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
> index 69e3ee3..5ccbee0 100644
> --- a/drivers/mmc/host/sdhci-s3c.c
> +++ b/drivers/mmc/host/sdhci-s3c.c
> @@ -629,6 +629,16 @@ static int sdhci_s3c_resume(struct platform_device *dev)
> ?#define sdhci_s3c_resume NULL
> ?#endif
>
> +#ifdef CONFIG_OF
> +static const struct of_device_id s3c_sdhci_match[] = {
> + ? ? ? { .compatible = "samsung,s3c-sdhci" },
> + ? ? ? {},
> +};
> +MODULE_DEVICE_TABLE(of, s3c_sdhci_match);
> +#else
> +#define s3c_sdhci_match NULL
> +#endif
> +
> ?static struct platform_driver sdhci_s3c_driver = {
> ? ? ? ?.probe ? ? ? ? ?= sdhci_s3c_probe,
> ? ? ? ?.remove ? ? ? ? = __devexit_p(sdhci_s3c_remove),
> @@ -637,6 +647,7 @@ static struct platform_driver sdhci_s3c_driver = {
> ? ? ? ?.driver ? ? ? ? = {
> ? ? ? ? ? ? ? ?.owner ?= THIS_MODULE,
> ? ? ? ? ? ? ? ?.name ? = "s3c-sdhci",
> + ? ? ? ? ? ? ? .of_match_table = s3c_sdhci_match,
> ? ? ? ?},
> ?};
>
> --
> 1.6.6.rc2
>
>
> _______________________________________________
> linaro-dev mailing list
> linaro-dev at lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/linaro-dev
>
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 5/6] arm: dts: Add nodes in smdkv310 device tree source file
2011-06-20 11:02 [PATCH 0/6] Add basic device tree support for Samsung's Exynos4 platform Thomas Abraham
` (3 preceding siblings ...)
2011-06-20 11:02 ` [PATCH 4/6] mmc: sdhci-s3c: " Thomas Abraham
@ 2011-06-20 11:02 ` Thomas Abraham
2011-06-20 11:02 ` [PATCH 6/6] arm: exynos4: Add a new Exynos4 device tree enabled machine Thomas Abraham
5 siblings, 0 replies; 21+ messages in thread
From: Thomas Abraham @ 2011-06-20 11:02 UTC (permalink / raw)
To: linux-arm-kernel
Include device tree nodes for watchdog timer, sdhci instance 0 and 1,
and uart instances 0 to 3.
Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
---
arch/arm/boot/dts/exynos4-smdkv310.dts | 135 +++++++++++++++++++++++++++++++-
1 files changed, 133 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/exynos4-smdkv310.dts b/arch/arm/boot/dts/exynos4-smdkv310.dts
index dd6c80a..721563f 100644
--- a/arch/arm/boot/dts/exynos4-smdkv310.dts
+++ b/arch/arm/boot/dts/exynos4-smdkv310.dts
@@ -1,11 +1,142 @@
+/*
+ * Samsung's Exynos4 based smdkv310 board device tree source.
+ *
+ * Copyright (c) 2010-2011 Samsung Electronics Co., Ltd.
+ * http://www.samsung.com
+ * Copyright (c) 2010-2011 Linaro Ltd.
+ * www.linaro.org
+ *
+ * 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.
+*/
+
/dts-v1/;
/include/ "skeleton.dtsi"
/ {
model = "Samsung Exynos4 SMDKV310 eval board";
- compatible = "samsung,smdkv310", "samsung,s5pv310";
+ compatible = "samsung,smdkv310", "samsung,s5pv310", "samsung,exynos4210";
memory {
- reg = <0x40000000 0x08000000>;
+ reg = <0x40000000 0x80000000>;
+ };
+
+ chosen {
+ bootargs = "root=/dev/mmcblk0p1 rootfstype=ext3 rootwait console=ttySAC1,115200";
+ };
+
+ soc {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ interrupt-parent = <&intc>;
+ compatible = "simple-bus";
+ ranges;
+
+ intc:interrupt-controller at 0x10501000 {
+ compatible = "samsung,exynos4-gic","arm,cortex-a9-gic";
+ #interrupt-cells = <1>;
+ interrupt-controller;
+ reg = <0x10501000 0x1000>, <0x10500100 0x100>;
+ irq-start = <61>;
+ };
+
+ watchdog at 0x10060000 {
+ compatible = "samsung,s3c2410-wdt";
+ reg = <0x10060000 0x400>;
+ interrupts = <552>;
+ };
+
+ sdhci at 0x12530000 {
+ compatible = "samsung,s3c-sdhci";
+ reg = <0x12530000 0x1000>;
+ interrupts = <362>;
+ };
+
+ sdhci at 0x12510000 {
+ compatible = "samsung,s3c-sdhci";
+ reg = <0x12510000 0x1000>;
+ interrupts = <360>;
+ };
+
+ serial at 0x13800000 {
+ compatible = "samsung,s5pv210-uart";
+ reg = <0x13800000 0x100>;
+ interrupts = <16 18 17>;
+ fifosize = <256>;
+ hwport = <0>;
+ flags = <0>;
+ uart_flags = <0>;
+ has_fracval = <1>;
+ ucon_default = <0x3c5>;
+ ulcon_default = <0x3>;
+ ufcon_default = <0x111>;
+ uart_clksrc0 {
+ clk_name = "uclk1";
+ divisor = <1>;
+ min_baud = <0>;
+ max_baud = <0>;
+ };
+ };
+
+ serial at 0x13810000 {
+ compatible = "samsung,s5pv210-uart";
+ reg = <0x13810000 0x100>;
+ interrupts = <20 22 21>;
+ fifosize = <64>;
+ hwport = <1>;
+ flags = <0>;
+ uart_flags = <0>;
+ has_fracval = <1>;
+ ucon_default = <0x3c5>;
+ ulcon_default = <0x3>;
+ ufcon_default = <0x111>;
+ uart_clksrc0 {
+ clk_name = "uclk1";
+ divisor = <1>;
+ min_baud = <0>;
+ max_baud = <0>;
+ };
+ };
+
+ serial at 0x13820000 {
+ compatible = "samsung,s5pv210-uart";
+ reg = <0x13820000 0x100>;
+ interrupts = <24 26 25>;
+ fifosize = <16>;
+ hwport = <1>;
+ flags = <0>;
+ uart_flags = <0>;
+ has_fracval = <1>;
+ ucon_default = <0x3c5>;
+ ulcon_default = <0x3>;
+ ufcon_default = <0x111>;
+ uart_clksrc0 {
+ clk_name = "uclk1";
+ divisor = <1>;
+ min_baud = <0>;
+ max_baud = <0>;
+ };
+ };
+
+ serial at 0x13830000 {
+ compatible = "samsung,s5pv210-uart";
+ reg = <0x13830000 0x100>;
+ interrupts = <28 30 29>;
+ fifosize = <16>;
+ hwport = <1>;
+ flags = <0>;
+ uart_flags = <0>;
+ has_fracval = <1>;
+ ucon_default = <0x3c5>;
+ ulcon_default = <0x3>;
+ ufcon_default = <0x111>;
+ uart_clksrc0 {
+ clk_name = "uclk1";
+ divisor = <1>;
+ min_baud = <0>;
+ max_baud = <0>;
+ };
+ };
};
};
--
1.6.6.rc2
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 6/6] arm: exynos4: Add a new Exynos4 device tree enabled machine
2011-06-20 11:02 [PATCH 0/6] Add basic device tree support for Samsung's Exynos4 platform Thomas Abraham
` (4 preceding siblings ...)
2011-06-20 11:02 ` [PATCH 5/6] arm: dts: Add nodes in smdkv310 device tree source file Thomas Abraham
@ 2011-06-20 11:02 ` Thomas Abraham
2011-06-20 16:55 ` Grant Likely
5 siblings, 1 reply; 21+ messages in thread
From: Thomas Abraham @ 2011-06-20 11:02 UTC (permalink / raw)
To: linux-arm-kernel
Basic Exynos4 machine with device tree support that can boot on a Exynos4
based smdkv310 board and bring up the console.
Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
---
Documentation/devicetree/bindings/arm/samsung.txt | 3 +-
arch/arm/mach-exynos4/Kconfig | 11 +++
arch/arm/mach-exynos4/Makefile | 1 +
arch/arm/mach-exynos4/mach-exynos4-dt.c | 94 +++++++++++++++++++++
4 files changed, 108 insertions(+), 1 deletions(-)
create mode 100644 arch/arm/mach-exynos4/mach-exynos4-dt.c
diff --git a/Documentation/devicetree/bindings/arm/samsung.txt b/Documentation/devicetree/bindings/arm/samsung.txt
index 594cb97..80d29bb 100644
--- a/Documentation/devicetree/bindings/arm/samsung.txt
+++ b/Documentation/devicetree/bindings/arm/samsung.txt
@@ -4,6 +4,7 @@ Samsung Exynos4 S5PV310 SoC based SMDKV310 eval board
Samsung's Exynos4 family of application processors.
Required root node properties:
- - compatible = "samsung,smdkv310","samsung,s5pv310"
+ - compatible = "samsung,smdkv310","samsung,s5pv310", "samsung,exynos4210'.
(a) "samsung,smdkv310" - for Samsung's SMDKV310 eval board.
(b) "samsung,s5pv310" - for boards based on S5PV310 SoC.
+ (c) "samsung,exynos4210" - for boards based on Exynos4210 processor.
diff --git a/arch/arm/mach-exynos4/Kconfig b/arch/arm/mach-exynos4/Kconfig
index 1435fc3..412e0c5 100644
--- a/arch/arm/mach-exynos4/Kconfig
+++ b/arch/arm/mach-exynos4/Kconfig
@@ -186,6 +186,17 @@ config MACH_NURI
help
Machine support for Samsung Mobile NURI Board.
+config MACH_EXYNOS4_DT
+ bool "Samsung's Exynos4 Machine with DT support"
+ select CPU_EXYNOS4210
+ select USE_OF
+ select S3C_DEV_WDT
+ select S3C_DEV_HSMMC
+ select S3C_DEV_HSMMC2
+ select EXYNOS4_SETUP_SDHCI
+ help
+ Machine support for Samsung Exynos4 machine with device tree enabled.
+
endmenu
comment "Configuration for HSMMC bus width"
diff --git a/arch/arm/mach-exynos4/Makefile b/arch/arm/mach-exynos4/Makefile
index 60fe5ec..6491e5b 100644
--- a/arch/arm/mach-exynos4/Makefile
+++ b/arch/arm/mach-exynos4/Makefile
@@ -36,6 +36,7 @@ obj-$(CONFIG_MACH_SMDKV310) += mach-smdkv310.o
obj-$(CONFIG_MACH_ARMLEX4210) += mach-armlex4210.o
obj-$(CONFIG_MACH_UNIVERSAL_C210) += mach-universal_c210.o
obj-$(CONFIG_MACH_NURI) += mach-nuri.o
+obj-$(CONFIG_MACH_EXYNOS4_DT) += mach-exynos4-dt.o
# device support
diff --git a/arch/arm/mach-exynos4/mach-exynos4-dt.c b/arch/arm/mach-exynos4/mach-exynos4-dt.c
new file mode 100644
index 0000000..34267d8
--- /dev/null
+++ b/arch/arm/mach-exynos4/mach-exynos4-dt.c
@@ -0,0 +1,94 @@
+/*
+ * Samsung's Exynos4210 device tree enabled machine.
+ *
+ * Copyright (c) 2010-2011 Samsung Electronics Co., Ltd.
+ * http://www.samsung.com
+ * Copyright (c) 2010-2011 Linaro Ltd.
+ * www.linaro.org
+ *
+ * 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/serial_core.h>
+#include <linux/platform_device.h>
+#include <linux/io.h>
+#include <linux/of_platform.h>
+#include <linux/irq.h>
+
+#include <asm/mach/arch.h>
+#include <asm/mach-types.h>
+
+#include <plat/regs-serial.h>
+#include <plat/exynos4.h>
+#include <plat/cpu.h>
+#include <plat/sdhci.h>
+
+#include <mach/map.h>
+
+static struct s3c_sdhci_platdata smdkv310_hsmmc0_pdata __initdata = {
+ .cd_type = S3C_SDHCI_CD_INTERNAL,
+ .clk_type = S3C_SDHCI_CLK_DIV_EXTERNAL,
+#ifdef CONFIG_EXYNOS4_SDHCI_CH0_8BIT
+ .max_width = 8,
+ .host_caps = MMC_CAP_8_BIT_DATA,
+#endif
+};
+
+static struct s3c_sdhci_platdata smdkv310_hsmmc2_pdata __initdata = {
+ .cd_type = S3C_SDHCI_CD_INTERNAL,
+ .clk_type = S3C_SDHCI_CLK_DIV_EXTERNAL,
+#ifdef CONFIG_EXYNOS4_SDHCI_CH2_8BIT
+ .max_width = 8,
+ .host_caps = MMC_CAP_8_BIT_DATA,
+#endif
+};
+
+static const struct of_dev_auxdata exynos4_auxdata_lookup[] __initconst = {
+ OF_DEV_AUXDATA("samsung,s3c-sdhci", EXYNOS4_PA_HSMMC(2),
+ "s3c-sdhci.2", &s3c_hsmmc2_def_platdata),
+ OF_DEV_AUXDATA("samsung,s3c-sdhci", EXYNOS4_PA_HSMMC(0),
+ "s3c-sdhci.0", &s3c_hsmmc0_def_platdata),
+ OF_DEV_AUXDATA("samsung,s5pv210-uart", S5P_PA_UART0,
+ "s5pv210-uart.0", NULL),
+ OF_DEV_AUXDATA("samsung,s5pv210-uart", S5P_PA_UART1,
+ "s5pv210-uart.1", NULL),
+ {},
+};
+
+static void __init exynos4_dt_map_io(void)
+{
+ s5p_init_io(NULL, 0, S5P_VA_CHIPID);
+ s3c24xx_init_clocks(24000000);
+}
+
+static const struct of_device_id intc_of_match[] __initconst = {
+ { .compatible = "samsung,exynos4-gic", },
+ {}
+};
+
+static void __init exynos4_dt_machine_init(void)
+{
+ s3c_sdhci0_set_platdata(&smdkv310_hsmmc0_pdata);
+ s3c_sdhci2_set_platdata(&smdkv310_hsmmc2_pdata);
+
+ irq_domain_generate_simple(intc_of_match, EXYNOS4_PA_GIC_DIST, 0);
+ of_platform_populate(NULL, of_default_bus_match_table,
+ exynos4_auxdata_lookup, NULL);
+}
+
+static char const *exynos4_dt_compat[] __initdata = {
+ "samsung,exynos4210",
+ NULL
+};
+
+DT_MACHINE_START(SMDKV310, "Samsung Exynos4 DT")
+ /* Maintainer: Kukjin Kim <kgene.kim@samsung.com> */
+ .boot_params = S5P_PA_SDRAM + 0x100,
+ .init_irq = exynos4_init_irq,
+ .map_io = exynos4_dt_map_io,
+ .init_machine = exynos4_dt_machine_init,
+ .timer = &exynos4_timer,
+ .dt_compat = exynos4_dt_compat,
+MACHINE_END
--
1.6.6.rc2
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 6/6] arm: exynos4: Add a new Exynos4 device tree enabled machine
2011-06-20 11:02 ` [PATCH 6/6] arm: exynos4: Add a new Exynos4 device tree enabled machine Thomas Abraham
@ 2011-06-20 16:55 ` Grant Likely
2011-06-21 11:30 ` Thomas Abraham
0 siblings, 1 reply; 21+ messages in thread
From: Grant Likely @ 2011-06-20 16:55 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Jun 20, 2011 at 5:02 AM, Thomas Abraham
<thomas.abraham@linaro.org> wrote:
> Basic Exynos4 machine with device tree support that can boot on a Exynos4
> based smdkv310 board and bring up the console.
>
> Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
> ---
> ?Documentation/devicetree/bindings/arm/samsung.txt | ? ?3 +-
> ?arch/arm/mach-exynos4/Kconfig ? ? ? ? ? ? ? ? ? ? | ? 11 +++
> ?arch/arm/mach-exynos4/Makefile ? ? ? ? ? ? ? ? ? ?| ? ?1 +
> ?arch/arm/mach-exynos4/mach-exynos4-dt.c ? ? ? ? ? | ? 94 +++++++++++++++++++++
> ?4 files changed, 108 insertions(+), 1 deletions(-)
> ?create mode 100644 arch/arm/mach-exynos4/mach-exynos4-dt.c
>
> diff --git a/Documentation/devicetree/bindings/arm/samsung.txt b/Documentation/devicetree/bindings/arm/samsung.txt
> index 594cb97..80d29bb 100644
> --- a/Documentation/devicetree/bindings/arm/samsung.txt
> +++ b/Documentation/devicetree/bindings/arm/samsung.txt
> @@ -4,6 +4,7 @@ Samsung Exynos4 S5PV310 SoC based SMDKV310 eval board
> ? ? Samsung's Exynos4 family of application processors.
>
> ?Required root node properties:
> - ? ?- compatible = "samsung,smdkv310","samsung,s5pv310"
> + ? ?- compatible = "samsung,smdkv310","samsung,s5pv310", "samsung,exynos4210'.
> ? ? ? ? (a) "samsung,smdkv310" - for Samsung's SMDKV310 eval board.
> ? ? ? ? (b) "samsung,s5pv310" ?- for boards based on S5PV310 SoC.
> + ? ? ? (c) "samsung,exynos4210" - for boards based on Exynos4210 processor.
> diff --git a/arch/arm/mach-exynos4/Kconfig b/arch/arm/mach-exynos4/Kconfig
> index 1435fc3..412e0c5 100644
> --- a/arch/arm/mach-exynos4/Kconfig
> +++ b/arch/arm/mach-exynos4/Kconfig
> @@ -186,6 +186,17 @@ config MACH_NURI
> ? ? ? ?help
> ? ? ? ? ?Machine support for Samsung Mobile NURI Board.
>
> +config MACH_EXYNOS4_DT
> + ? ? ? bool "Samsung's Exynos4 Machine with DT support"
> + ? ? ? select CPU_EXYNOS4210
> + ? ? ? select USE_OF
> + ? ? ? select S3C_DEV_WDT
> + ? ? ? select S3C_DEV_HSMMC
> + ? ? ? select S3C_DEV_HSMMC2
> + ? ? ? select EXYNOS4_SETUP_SDHCI
> + ? ? ? help
> + ? ? ? ? Machine support for Samsung Exynos4 machine with device tree enabled.
> +
> ?endmenu
>
> ?comment "Configuration for HSMMC bus width"
> diff --git a/arch/arm/mach-exynos4/Makefile b/arch/arm/mach-exynos4/Makefile
> index 60fe5ec..6491e5b 100644
> --- a/arch/arm/mach-exynos4/Makefile
> +++ b/arch/arm/mach-exynos4/Makefile
> @@ -36,6 +36,7 @@ obj-$(CONFIG_MACH_SMDKV310) ? ? ? ? ? += mach-smdkv310.o
> ?obj-$(CONFIG_MACH_ARMLEX4210) ? ? ? ? ?+= mach-armlex4210.o
> ?obj-$(CONFIG_MACH_UNIVERSAL_C210) ? ? ?+= mach-universal_c210.o
> ?obj-$(CONFIG_MACH_NURI) ? ? ? ? ? ? ? ? ? ? ? ?+= mach-nuri.o
> +obj-$(CONFIG_MACH_EXYNOS4_DT) ? ? ? ? ?+= mach-exynos4-dt.o
>
> ?# device support
>
> diff --git a/arch/arm/mach-exynos4/mach-exynos4-dt.c b/arch/arm/mach-exynos4/mach-exynos4-dt.c
> new file mode 100644
> index 0000000..34267d8
> --- /dev/null
> +++ b/arch/arm/mach-exynos4/mach-exynos4-dt.c
> @@ -0,0 +1,94 @@
> +/*
> + * Samsung's Exynos4210 device tree enabled machine.
> + *
> + * Copyright (c) 2010-2011 Samsung Electronics Co., Ltd.
> + * ? ? ? ? ? ? http://www.samsung.com
> + * Copyright (c) 2010-2011 Linaro Ltd.
> + * ? ? ? ? ? ? www.linaro.org
> + *
> + * 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/serial_core.h>
> +#include <linux/platform_device.h>
> +#include <linux/io.h>
> +#include <linux/of_platform.h>
> +#include <linux/irq.h>
> +
> +#include <asm/mach/arch.h>
> +#include <asm/mach-types.h>
> +
> +#include <plat/regs-serial.h>
> +#include <plat/exynos4.h>
> +#include <plat/cpu.h>
> +#include <plat/sdhci.h>
> +
> +#include <mach/map.h>
> +
> +static struct s3c_sdhci_platdata smdkv310_hsmmc0_pdata __initdata = {
> + ? ? ? .cd_type ? ? ? ? ? ? ? ?= S3C_SDHCI_CD_INTERNAL,
> + ? ? ? .clk_type ? ? ? ? ? ? ? = S3C_SDHCI_CLK_DIV_EXTERNAL,
> +#ifdef CONFIG_EXYNOS4_SDHCI_CH0_8BIT
> + ? ? ? .max_width ? ? ? ? ? ? ?= 8,
> + ? ? ? .host_caps ? ? ? ? ? ? ?= MMC_CAP_8_BIT_DATA,
> +#endif
> +};
> +
> +static struct s3c_sdhci_platdata smdkv310_hsmmc2_pdata __initdata = {
> + ? ? ? .cd_type ? ? ? ? ? ? ? ?= S3C_SDHCI_CD_INTERNAL,
> + ? ? ? .clk_type ? ? ? ? ? ? ? = S3C_SDHCI_CLK_DIV_EXTERNAL,
> +#ifdef CONFIG_EXYNOS4_SDHCI_CH2_8BIT
> + ? ? ? .max_width ? ? ? ? ? ? ?= 8,
> + ? ? ? .host_caps ? ? ? ? ? ? ?= MMC_CAP_8_BIT_DATA,
> +#endif
> +};
> +
> +static const struct of_dev_auxdata exynos4_auxdata_lookup[] __initconst = {
> + ? ? ? OF_DEV_AUXDATA("samsung,s3c-sdhci", EXYNOS4_PA_HSMMC(2),
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "s3c-sdhci.2", &s3c_hsmmc2_def_platdata),
> + ? ? ? OF_DEV_AUXDATA("samsung,s3c-sdhci", EXYNOS4_PA_HSMMC(0),
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "s3c-sdhci.0", &s3c_hsmmc0_def_platdata),
> + ? ? ? OF_DEV_AUXDATA("samsung,s5pv210-uart", S5P_PA_UART0,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "s5pv210-uart.0", NULL),
> + ? ? ? OF_DEV_AUXDATA("samsung,s5pv210-uart", S5P_PA_UART1,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "s5pv210-uart.1", NULL),
> + ? ? ? {},
> +};
> +
> +static void __init exynos4_dt_map_io(void)
> +{
> + ? ? ? s5p_init_io(NULL, 0, S5P_VA_CHIPID);
> + ? ? ? s3c24xx_init_clocks(24000000);
> +}
> +
> +static const struct of_device_id intc_of_match[] __initconst = {
> + ? ? ? { .compatible = "samsung,exynos4-gic", },
> + ? ? ? {}
> +};
> +
> +static void __init exynos4_dt_machine_init(void)
> +{
> + ? ? ? s3c_sdhci0_set_platdata(&smdkv310_hsmmc0_pdata);
> + ? ? ? s3c_sdhci2_set_platdata(&smdkv310_hsmmc2_pdata);
Are these two lines still necessary?
Otherwise, the patch looks good to me.
g.
> +
> + ? ? ? irq_domain_generate_simple(intc_of_match, EXYNOS4_PA_GIC_DIST, 0);
> + ? ? ? of_platform_populate(NULL, of_default_bus_match_table,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? exynos4_auxdata_lookup, NULL);
> +}
> +
> +static char const *exynos4_dt_compat[] __initdata = {
> + ? ? ? "samsung,exynos4210",
> + ? ? ? NULL
> +};
> +
> +DT_MACHINE_START(SMDKV310, "Samsung Exynos4 DT")
> + ? ? ? /* Maintainer: Kukjin Kim <kgene.kim@samsung.com> */
> + ? ? ? .boot_params ? ?= S5P_PA_SDRAM + 0x100,
> + ? ? ? .init_irq ? ? ? = exynos4_init_irq,
> + ? ? ? .map_io ? ? ? ? = exynos4_dt_map_io,
> + ? ? ? .init_machine ? = exynos4_dt_machine_init,
> + ? ? ? .timer ? ? ? ? ?= &exynos4_timer,
> + ? ? ? .dt_compat ? ? ?= exynos4_dt_compat,
> +MACHINE_END
> --
> 1.6.6.rc2
>
>
> _______________________________________________
> linaro-dev mailing list
> linaro-dev at lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/linaro-dev
>
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply [flat|nested] 21+ messages in thread* [PATCH 6/6] arm: exynos4: Add a new Exynos4 device tree enabled machine
2011-06-20 16:55 ` Grant Likely
@ 2011-06-21 11:30 ` Thomas Abraham
0 siblings, 0 replies; 21+ messages in thread
From: Thomas Abraham @ 2011-06-21 11:30 UTC (permalink / raw)
To: linux-arm-kernel
On 20 June 2011 22:25, Grant Likely <grant.likely@secretlab.ca> wrote:
<snip>
>> +
>> +static const struct of_dev_auxdata exynos4_auxdata_lookup[] __initconst = {
>> + ? ? ? OF_DEV_AUXDATA("samsung,s3c-sdhci", EXYNOS4_PA_HSMMC(2),
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "s3c-sdhci.2", &s3c_hsmmc2_def_platdata),
>> + ? ? ? OF_DEV_AUXDATA("samsung,s3c-sdhci", EXYNOS4_PA_HSMMC(0),
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "s3c-sdhci.0", &s3c_hsmmc0_def_platdata),
>> + ? ? ? OF_DEV_AUXDATA("samsung,s5pv210-uart", S5P_PA_UART0,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "s5pv210-uart.0", NULL),
>> + ? ? ? OF_DEV_AUXDATA("samsung,s5pv210-uart", S5P_PA_UART1,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "s5pv210-uart.1", NULL),
>> + ? ? ? {},
>> +};
<snip>
>> +static void __init exynos4_dt_machine_init(void)
>> +{
>> + ? ? ? s3c_sdhci0_set_platdata(&smdkv310_hsmmc0_pdata);
>> + ? ? ? s3c_sdhci2_set_platdata(&smdkv310_hsmmc2_pdata);
>
> Are these two lines still necessary?
Yes, but these are temporary. The data from smdkv310_hsmmc0_pdata is
copied to s3c_hsmmc2_def_platdata which is then used as platform data.
I will remove them in the next version of the patch.
Thanks,
Thomas.
>
> Otherwise, the patch looks good to me.
>
> g.
^ permalink raw reply [flat|nested] 21+ messages in thread