linux-kernel - Re: [PATCH v3 3/4] serial: 8250_dw: Simplify the ref clock rate setting procedure

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200515140547.GE1634618@smile.fi.intel.com>
Date:   Fri, 15 May 2020 17:05:47 +0300
From:   Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To:     Serge Semin <Sergey.Semin@...kalelectronics.ru>
Cc:     Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jiri Slaby <jslaby@...e.com>,
        Serge Semin <fancer.lancer@...il.com>,
        Alexey Malahov <Alexey.Malahov@...kalelectronics.ru>,
        Paul Burton <paulburton@...nel.org>,
        Ralf Baechle <ralf@...ux-mips.org>,
        Arnd Bergmann <arnd@...db.de>,
        Long Cheng <long.cheng@...iatek.com>,
        Maxime Ripard <mripard@...nel.org>,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will@...nel.org>,
        Russell King <linux@...linux.org.uk>,
        linux-mips@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-mediatek@...ts.infradead.org,
        Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
        Kefeng Wang <wangkefeng.wang@...wei.com>,
        linux-serial@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 3/4] serial: 8250_dw: Simplify the ref clock rate
 setting procedure

On Thu, May 07, 2020 at 02:31:34AM +0300, Serge Semin wrote:
> Really instead of twice checking the clk_round_rate() return value
> we could do it once, and if it isn't error the clock rate can be changed.
> By doing so we decrease a number of ret-value tests and remove a weird
> goto-based construction implemented in the dw8250_set_termios() method.

>  	rate = clk_round_rate(d->clk, baud * 16);
> -	if (rate < 0)
> -		ret = rate;

> -	else if (rate == 0)
> -		ret = -ENOENT;

This case now handled differently.
I don't think it's good idea to change semantics.

So, I don't see how this, after leaving the rate==0 case, would be better than
original one.

> -	else
> +	if (rate > 0) {
>  		ret = clk_set_rate(d->clk, rate);
> +		if (!ret)
> +			p->uartclk = rate;
> +	}
>  	clk_prepare_enable(d->clk);
>  
> -	if (ret)
> -		goto out;
> -
> -	p->uartclk = rate;
> -
> -out:
>  	p->status &= ~UPSTAT_AUTOCTS;
>  	if (termios->c_cflag & CRTSCTS)
>  		p->status |= UPSTAT_AUTOCTS;

-- 
With Best Regards,
Andy Shevchenko