[PATCH 7/7] Samsung: Probe the serial driver using Device tree.
Grant Likely
grant.likely at secretlab.ca
Fri Sep 24 06:11:13 EST 2010
On Mon, Sep 20, 2010 at 03:50:18PM +0530, Shaju Abraham wrote:
> Added the match table for probing the serial devices from the device tree.
> Serial/samsung.c file is modified to support OF probing .Get the platform data
> from the static array as done in the case of s3c24xx_serial_init_port.Also the
> clock API (clk_get) might fail if platform id remains at -1.Since the
> struct device information is passed to the uart_add_one_port, get the struct device
> from the platform_device passed to the probe routine by the device tree probing.
>
> Signed-off-by: Shaju Abraham <shaju.abraham at linaro.org>
> ---
> arch/arm/plat-samsung/init.c | 3 ++-
> drivers/serial/s5pv210.c | 16 ++++++++++++++--
> drivers/serial/samsung.c | 10 +++++++++-
> 3 files changed, 25 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/plat-samsung/init.c b/arch/arm/plat-samsung/init.c
> index 6790edf..2c58acd 100644
> --- a/arch/arm/plat-samsung/init.c
> +++ b/arch/arm/plat-samsung/init.c
> @@ -152,8 +152,9 @@ static int __init s3c_arch_init(void)
> ret = (cpu->init)();
> if (ret != 0)
> return ret;
> -
> +#ifndef CONFIG_USE_OF
> ret = platform_add_devices(s3c24xx_uart_devs, nr_uarts);
> +#endif /*CONFIG_USE_OF*/
Ditto to comment on previous patch; you don't want to break existing
platforms. Instead, the code should be refactored so that static
devices are not registered in the dt-enabled machine description.
> return ret;
> }
>
> diff --git a/drivers/serial/s5pv210.c b/drivers/serial/s5pv210.c
> index 4a789e5..7f8bd18 100644
> --- a/drivers/serial/s5pv210.c
> +++ b/drivers/serial/s5pv210.c
> @@ -119,15 +119,27 @@ static int s5p_serial_probe(struct platform_device *pdev)
> return s3c24xx_serial_probe(pdev, s5p_uart_inf[pdev->id]);
> }
>
> +#ifdef CONFIG_USE_OF
#ifdef CONFIG_OF
> +static struct of_device_id s5p_uart_matches[] = {
> + { .compatible = "samsung,s5pv210-uart_sh"},
> + {},
> + };
> +#endif /*CONFIG_USE_OF*/
> +
> static struct platform_driver s5p_serial_driver = {
> .probe = s5p_serial_probe,
> .remove = __devexit_p(s3c24xx_serial_remove),
> .driver = {
> - .name = "s5pv210-uart",
> - .owner = THIS_MODULE,
> + .name = "s5pv210-uart",
> + .owner = THIS_MODULE,
> +#ifdef CONFIG_USE_OF
> + .of_match_table = s5p_uart_matches,
#ifdef CONFIG_OF
> +
> +#endif /*CONFIG_USE_OF */
> },
> };
>
> +
> static int __init s5pv210_serial_console_init(void)
> {
> return s3c24xx_serial_initconsole(&s5p_serial_driver, s5p_uart_inf);
> diff --git a/drivers/serial/samsung.c b/drivers/serial/samsung.c
> index 4c5eea0..317bd08 100644
> --- a/drivers/serial/samsung.c
> +++ b/drivers/serial/samsung.c
> @@ -1059,7 +1059,6 @@ static int s3c24xx_serial_init_port(struct s3c24xx_uart_port *ourport,
> return -ENODEV;
>
> cfg = s3c24xx_dev_to_cfg(&platdev->dev);
> -
> if (port->mapbase != 0)
Unrelated whitespace change.
> return 0;
>
> @@ -1145,6 +1144,15 @@ int s3c24xx_serial_probe(struct platform_device *dev,
> dbg("s3c24xx_serial_probe(%p, %p) %d\n", dev, info, probe_index);
>
> ourport = &s3c24xx_serial_ports[probe_index];
> +
> +#ifdef CONFIG_USE_OF
> + struct uart_port *port = &ourport->port;
> + dev->id = probe_index;
> + dev->dev.platform_data =
> + s3c24xx_uart_devs[probe_index]->dev.platform_data;
> + port->dev = &dev->dev;
> +#endif /*CONFIG_USE_OF */
Several issues with this. First, it breaks non-OF platforms. Second,
device drivers are not allowed to write to the "dev.platform_data"
pointer. If it is going to be locally overridden in the driver, then
the driver should maintain it's own copy of the pointer. Otherwise an
unbind/rebind of the driver could result in the driver have a wrong or
invalid platform_data pointer. Third; the configuration data
structure needs to be dynamically allocated and populated with data
from the device tree. It should not be populated from a static table.
If you do need to pass in a specific platform_data pointer, then it
should be done at device registration time by using a bus notifier.
However, that is a very rare circumstance.
g.
More information about the devicetree-discuss
mailing list