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 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.

• 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.

• 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

---

linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev

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.

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);

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.

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@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev