From: James Hogan <james.hogan@imgtec.com> To: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Cc: Heiko Stuebner <heiko@sntech.de>, Jason Uy <jason.uy@broadcom.com>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Jiri Slaby <jslaby@suse.com>, Kefeng Wang <wangkefeng.wang@huawei.com>, Noam Camus <noamc@ezchip.com>, Heikki Krogerus <heikki.krogerus@linux.intel.com>, Wang Hongcheng <annie.wang@amd.com>, <linux-serial@vger.kernel.org>, LKML <linux-kernel@vger.kernel.org>, <bcm-kernel-feedback-list@broadcom.com>, Linux MIPS Mailing List <linux-mips@linux-mips.org>, David Daney <david.daney@cavium.com>, Russell King <linux@armlinux.org.uk>, <linux-clk@vger.kernel.org>, Viresh Kumar <viresh.kumar@st.com> Subject: Re: [PATCH v2 1/1] serial: 8250_dw: Allow hardware flow control to be used Date: Fri, 3 Mar 2017 23:23:05 +0000 [thread overview] Message-ID: <20170303232305.GU996@jhogan-linux.le.imgtec.org> (raw) In-Reply-To: <1488547866.20145.74.camel@linux.intel.com> [-- Attachment #1: Type: text/plain, Size: 1789 bytes --] On Fri, Mar 03, 2017 at 03:31:06PM +0200, Andy Shevchenko wrote: > On Fri, 2017-03-03 at 00:21 +0000, James Hogan wrote: > > The CONFIG_HAVE_CLK=n implementation of devm_clk_get() in particular > > seems highly questionable to me, given that commit 93abe8e4b13a ("clk: > > add non CONFIG_HAVE_CLK routines") which added it 5 years ago says: > > > > > These calls will return error for platforms that don't select > > > HAVE_CLK > > > > And NULL isn't an error in this API. > > Which is okay. I dunno what should be returned from clk_round_rate() if > clk is NULL. I would fix CLK framework, though I would like to gather > more details. Hmm, the common clock framwork is just one implementation of the clock API that won't use NULL as a valid clock handle. HAVE_CLK=n is just another implementation that does return NULL as a valid value, and accepts that value in the other clk functions. > Btw, I hope you also noticed this one: > > http://www.spinics.net/lists/linux-serial/msg25314.html Interesting. Following Russel's past advise[1], the following patch on top of Heiko's patch also fixes things for me on Octeon: [1] https://lists.gt.net/linux/kernel/2102623 If thats an acceptable fix I'll post it properly. Thoughts? Cheers James diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c index 223ac234ddb2..e65808c482f1 100644 --- a/drivers/tty/serial/8250/8250_dw.c +++ b/drivers/tty/serial/8250/8250_dw.c @@ -267,6 +267,8 @@ static void dw8250_set_termios(struct uart_port *p, struct ktermios *termios, rate = clk_round_rate(d->clk, baud * 16); if (rate < 0) ret = rate; + else if (rate == 0) + ret = -ENOENT; else ret = clk_set_rate(d->clk, rate); clk_prepare_enable(d->clk); [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 801 bytes --]
WARNING: multiple messages have this Message-ID (diff)
From: James Hogan <james.hogan@imgtec.com> To: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Cc: Heiko Stuebner <heiko@sntech.de>, Jason Uy <jason.uy@broadcom.com>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Jiri Slaby <jslaby@suse.com>, Kefeng Wang <wangkefeng.wang@huawei.com>, Noam Camus <noamc@ezchip.com>, Heikki Krogerus <heikki.krogerus@linux.intel.com>, Wang Hongcheng <annie.wang@amd.com>, linux-serial@vger.kernel.org, LKML <linux-kernel@vger.kernel.org>, bcm-kernel-feedback-list@broadcom.com, Linux MIPS Mailing List <linux-mips@linux-mips.org>, David Daney <david.daney@cavium.com>, Russell King <linux@armlinux.org.uk>, linux-clk@vger.kernel.org, Viresh Kumar <viresh.kumar@st.com> Subject: Re: [PATCH v2 1/1] serial: 8250_dw: Allow hardware flow control to be used Date: Fri, 3 Mar 2017 23:23:05 +0000 [thread overview] Message-ID: <20170303232305.GU996@jhogan-linux.le.imgtec.org> (raw) Message-ID: <20170303232305.dNAFs8-9QKkUuXKJpotL0pvz8pWA_o2oYm9uJNiw3eE@z> (raw) In-Reply-To: <1488547866.20145.74.camel@linux.intel.com> [-- Attachment #1: Type: text/plain, Size: 1789 bytes --] On Fri, Mar 03, 2017 at 03:31:06PM +0200, Andy Shevchenko wrote: > On Fri, 2017-03-03 at 00:21 +0000, James Hogan wrote: > > The CONFIG_HAVE_CLK=n implementation of devm_clk_get() in particular > > seems highly questionable to me, given that commit 93abe8e4b13a ("clk: > > add non CONFIG_HAVE_CLK routines") which added it 5 years ago says: > > > > > These calls will return error for platforms that don't select > > > HAVE_CLK > > > > And NULL isn't an error in this API. > > Which is okay. I dunno what should be returned from clk_round_rate() if > clk is NULL. I would fix CLK framework, though I would like to gather > more details. Hmm, the common clock framwork is just one implementation of the clock API that won't use NULL as a valid clock handle. HAVE_CLK=n is just another implementation that does return NULL as a valid value, and accepts that value in the other clk functions. > Btw, I hope you also noticed this one: > > http://www.spinics.net/lists/linux-serial/msg25314.html Interesting. Following Russel's past advise[1], the following patch on top of Heiko's patch also fixes things for me on Octeon: [1] https://lists.gt.net/linux/kernel/2102623 If thats an acceptable fix I'll post it properly. Thoughts? Cheers James diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c index 223ac234ddb2..e65808c482f1 100644 --- a/drivers/tty/serial/8250/8250_dw.c +++ b/drivers/tty/serial/8250/8250_dw.c @@ -267,6 +267,8 @@ static void dw8250_set_termios(struct uart_port *p, struct ktermios *termios, rate = clk_round_rate(d->clk, baud * 16); if (rate < 0) ret = rate; + else if (rate == 0) + ret = -ENOENT; else ret = clk_set_rate(d->clk, rate); clk_prepare_enable(d->clk); [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 801 bytes --]
next prev parent reply other threads:[~2017-03-03 23:24 UTC|newest] Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-01-11 19:48 [PATCH v2 0/1] Allow hardware flow control to be used Jason Uy 2017-01-11 19:48 ` [PATCH v2 1/1] serial: 8250_dw: " Jason Uy 2017-01-11 19:53 ` Andy Shevchenko 2017-01-13 1:33 ` Kefeng Wang 2017-01-13 1:33 ` Kefeng Wang [not found] ` <CAAG0J9-n0toSJL8Ze8Esq81dYnpfrTd42bMiR94zw_btBLjsww@mail.gmail.com> 2017-03-01 18:50 ` Andy Shevchenko 2017-03-03 0:21 ` James Hogan 2017-03-03 0:21 ` James Hogan 2017-03-03 13:31 ` Andy Shevchenko 2017-03-03 17:33 ` Ray Jui 2017-03-03 17:43 ` Jason Uy 2017-03-03 23:07 ` James Hogan 2017-03-03 23:07 ` James Hogan 2017-03-04 0:02 ` Jason Uy 2017-03-04 0:11 ` James Hogan 2017-03-04 0:11 ` James Hogan 2017-03-03 23:23 ` James Hogan [this message] 2017-03-03 23:23 ` James Hogan 2017-03-04 2:56 ` Andy Shevchenko 2017-03-04 13:09 ` [PATCH] serial: 8250_dw: Fix breakage when HAVE_CLK=n James Hogan 2017-03-04 13:09 ` James Hogan 2017-03-04 14:37 ` Andy Shevchenko 2017-03-06 10:16 ` James Hogan 2017-03-06 10:16 ` James Hogan 2017-03-06 23:38 ` Jason Uy 2017-03-05 0:44 ` Heiko Stuebner 2017-03-13 11:14 ` James Hogan 2017-03-13 11:14 ` James Hogan 2017-03-14 2:21 ` Greg Kroah-Hartman
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20170303232305.GU996@jhogan-linux.le.imgtec.org \ --to=james.hogan@imgtec.com \ --cc=andriy.shevchenko@linux.intel.com \ --cc=annie.wang@amd.com \ --cc=bcm-kernel-feedback-list@broadcom.com \ --cc=david.daney@cavium.com \ --cc=gregkh@linuxfoundation.org \ --cc=heikki.krogerus@linux.intel.com \ --cc=heiko@sntech.de \ --cc=jason.uy@broadcom.com \ --cc=jslaby@suse.com \ --cc=linux-clk@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mips@linux-mips.org \ --cc=linux-serial@vger.kernel.org \ --cc=linux@armlinux.org.uk \ --cc=noamc@ezchip.com \ --cc=viresh.kumar@st.com \ --cc=wangkefeng.wang@huawei.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.