Russell King - ARM Linux wrote:
>
> On Tue, May 15, 2012 at 09:37:16PM +0900, Kyoungil Kim wrote:
> > port->baudclk_rate should be compared to the rate of port->baudclk,
> > because port->baudclk_rate was assigned as the rate of port->baudclk
> previously.
> > So to check that the current baudclk rate is same as previous rate,
> > the target of comparison sholud be the rate of port->baudclk.
> >
> > Signed-off-by: Jun-Ho, Yoon <junho78.yoon@xxxxxxxxxxx>
> > Signed-off-by: Kyoungil Kim <ki0351.kim@xxxxxxxxxxx>
> > ---
> > drivers/tty/serial/samsung.c | 4 ++--
> > 1 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
> > index d8b0aee..6a6a86a 100644
> > --- a/drivers/tty/serial/samsung.c
> > +++ b/drivers/tty/serial/samsung.c
> > @@ -1014,10 +1014,10 @@ static int
> s3c24xx_serial_cpufreq_transition(struct notifier_block *nb,
> > * a disturbance in the clock-rate over the change.
> > */
> >
> > - if (IS_ERR(port->clk))
> > + if (IS_ERR(port->baudclk) || port->baudclk == NULL)
>
> Still no. Why are you wanting to detect a NULL baud clock?
>
> As I said, drivers have no business interpreting anything but IS_ERR(clk)
> as being an error. They should not make any other assumptions.
>
> Now that I look at this driver, it makes this mistake all over the place.
> This needs to be fixed. Something like the below should do it. Please
> check.
>
Russell, thanks for your kindly pointing out.
Kyoungil, could you please re-submit for this as per Russell's suggestion
(with Russell's patch)?
Thanks.
Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@xxxxxxxxxxx>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.
> diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
> index d8b0aee..6a952b1 100644
> --- a/drivers/tty/serial/samsung.c
> +++ b/drivers/tty/serial/samsung.c
> @@ -529,7 +529,7 @@ static void s3c24xx_serial_pm(struct uart_port *port,
> unsigned int level,
>
> switch (level) {
> case 3:
> - if (!IS_ERR(ourport->baudclk) && ourport->baudclk != NULL)
> + if (!IS_ERR(ourport->baudclk))
> clk_disable(ourport->baudclk);
>
> clk_disable(ourport->clk);
> @@ -538,7 +538,7 @@ static void s3c24xx_serial_pm(struct uart_port *port,
> unsigned int level,
> case 0:
> clk_enable(ourport->clk);
>
> - if (!IS_ERR(ourport->baudclk) && ourport->baudclk != NULL)
> + if (!IS_ERR(ourport->baudclk))
> clk_enable(ourport->baudclk);
>
> break;
> @@ -713,9 +713,9 @@ static void s3c24xx_serial_set_termios(struct
> uart_port *port,
> if (ourport->baudclk != clk) {
> s3c24xx_serial_setsource(port, clk_sel);
>
> - if (ourport->baudclk != NULL && !IS_ERR(ourport->baudclk)) {
> + if (!IS_ERR(ourport->baudclk)) {
> clk_disable(ourport->baudclk);
> - ourport->baudclk = NULL;
> + ourport->baudclk = ERR_PTR(-EINVAL);
> }
>
> clk_enable(clk);
> @@ -1160,6 +1160,9 @@ static ssize_t s3c24xx_serial_show_clksrc(struct
> device *dev,
> struct uart_port *port = s3c24xx_dev_to_port(dev);
> struct s3c24xx_uart_port *ourport = to_ourport(port);
>
> + if (IS_ERR(ourport->baudclk))
> + return -EINVAL;
> +
> return snprintf(buf, PAGE_SIZE, "* %s\n", ourport->baudclk->name);
> }
>
> @@ -1200,6 +1203,7 @@ static int s3c24xx_serial_probe(struct
> platform_device *pdev)
> return -ENODEV;
> }
>
> + ourport->baudclk = ERR_PTR(-EINVAL);
> ourport->info = ourport->drv_data->info;
> ourport->cfg = (pdev->dev.platform_data) ?
> (struct s3c2410_uartcfg *)pdev->dev.platform_data :
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
