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 <[email protected]>
> ---
>  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.
_______________________________________________
devicetree-discuss mailing list
[email protected]
https://lists.ozlabs.org/listinfo/devicetree-discuss

Reply via email to