Re: [PATCH][uart] fit blackfin uart over sport driver into common uart inftrastructure (v2)

[Mike Frysinger <vapier.adi@gmail.com>]

On Wed, Sep 16, 2009 at 06:02, sonic zhang wrote:
> 3. Move most platform data into arch specific board files.

might as well do it right now since you've done most of it already.
i.e. all of the peripheral logic should be in the boards, not the
driver.  so the bfin_uart_pin_req_sportx lists should get moved to the
platform resources as well as the SPORT base addresses.

then we get proper device bindings between ttySS# and SPORT# by using
the .id field.  ttySS0 should always be SPORT0, ttySS1 should always
be SPORT1, etc...

> Signed-off-by: Michael Frysinger <michael.frysinger@analog.com>

you shouldnt add s-o-b tags for people who didnt opt for them ... i
was sleeping when you wrote, committed, and e-mailed out these
changes.  i also handt posted these changes yet because i knew there
were still issues to be worked out.

> --- a/drivers/serial/bfin_sport_uart.c
> +++ b/drivers/serial/bfin_sport_uart.c
> +#define DRV_NAME "bfin-sport-uart"
> +#define DEVICE_NAME    "ttySS"

should really use the dev_* functions for output rather than the pr_*
ones, but if you dont do that, you should add before the #include's:
#define pr_fmt(fmt) DRV_NAME ": " fmt

>        tclkdiv = sclk/(2 * baud_rate) - 1;
> -       tfsdiv = 12;
> -       rclkdiv = sclk/(2 * baud_rate * 3) - 1;
> +       rclkdiv = sclk/(2 * baud_rate * 2) - 1;

missing space around the "/"

> -static int sport_uart_suspend(struct platform_device *dev, pm_message_t state)
> +#ifdef CONFIG_PM
> +static int sport_uart_suspend(struct platform_device *pdev, pm_message_t state)
>  {
> -       struct sport_uart_port *sport = platform_get_drvdata(dev);
> +       struct sport_uart_port *sport = platform_get_drvdata(pdev);
>
> -       pr_debug("%s enter\n", __func__);
> +       dev_dbg(&pdev->dev, "%s enter\n", __func__);
>        if (sport)
>                uart_suspend_port(&sport_uart_reg, &sport->port);
>
>        return 0;
>  }
>
> -static int sport_uart_resume(struct platform_device *dev)
> +static int sport_uart_resume(struct platform_device *pdev)
>  {
> -       struct sport_uart_port *sport = platform_get_drvdata(dev);
> +       struct sport_uart_port *sport = platform_get_drvdata(pdev);
>
> -       pr_debug("%s enter\n", __func__);
> +       dev_dbg(&pdev->dev, "%s enter\n", __func__);
>        if (sport)
>                uart_resume_port(&sport_uart_reg, &sport->port);
>
>        return 0;
>  }

these need updating to the new dev_pm_opts structure.  see the changes
i just did to the adp558-keys driver for a simple example.

> --- a/drivers/serial/bfin_sport_uart.h
> +++ b/drivers/serial/bfin_sport_uart.h

while you're in here, you might as well add the proper #ifndef header protection
-mike
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html