From: linux@arm.linux.org.uk (Russell King - ARM Linux)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 2/2] serial: msm: Add support for UARTDM cores
Date: Wed, 19 Jan 2011 22:24:07 +0000 [thread overview]
Message-ID: <20110119222407.GD6335@n2100.arm.linux.org.uk> (raw)
In-Reply-To: <4D3760DD.1010609@codeaurora.org>
On Wed, Jan 19, 2011 at 02:08:29PM -0800, Stepan Moskovchenko wrote:
>>> + /* Mask conditions we're ignorning. */
>>> + sr&= port->read_status_mask;
>>> + if (sr& UART_SR_RX_BREAK)
>>> + flag = TTY_BREAK;
>>> + else if (sr& UART_SR_PAR_FRAME_ERR)
>>> + flag = TTY_FRAME;
>> It doesn't look like the flag is used anywhere after it has been
>> assigned.
> An artifact of an old driver. Removed.
But still required to support proper error signalling.
>>> static void msm_init_clock(struct uart_port *port)
>>> {
>>> struct msm_port *msm_port = UART_TO_MSM(port);
>>>
>>> clk_enable(msm_port->clk);
>>> + if (msm_port->pclk)
>>> + clk_enable(msm_port->pclk);
>> NULL is a valid clk, so this should really be something like
>>
>> if (!IS_ERR(mem_port->pclk)
>> clk_enable(...);
> I don't think that will have the correct behavior. The clock is already
> checked with IS_ERR in the probe function, so we could not get here if
> the clk_get returned an error. Depending on the unit, there may or may
> not be a pclk associated with it. Thus, I use NULL to indicate that a
> pclk does not exist and should not be turned on. Regardless, at least in
> the MSM clock driver (and in drivers/clkdev) NULL is not a valid clock,
> I think this should be fine as is.
Don't invent new ways to do things. As far as drivers are concerned,
struct clk pointers are cookies. The only thing they can interpret from
them is whether they're IS_ERR() and then what the error is. They must
assume every other value is valid.
So, if you want "clock not present" assign ERR_PTR(-ENOENT) to it or
just assign the return value from the failed clk_get().
next prev parent reply other threads:[~2011-01-19 22:24 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-01-19 3:26 [PATCH 2/2] serial: msm: Add support for UARTDM cores Stepan Moskovchenko
2011-01-19 8:25 ` Jamie Iles
2011-01-19 17:31 ` David Brown
2011-01-19 17:37 ` Jamie Iles
2011-01-19 19:52 ` David Brown
2011-01-19 22:08 ` Stepan Moskovchenko
2011-01-19 22:24 ` Russell King - ARM Linux [this message]
2011-01-20 0:11 ` Stepan Moskovchenko
2011-01-20 2:38 ` Stepan Moskovchenko
2011-01-20 9:04 ` Russell King - ARM Linux
2011-01-20 21:56 ` [PATCH v2 " Stepan Moskovchenko
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=20110119222407.GD6335@n2100.arm.linux.org.uk \
--to=linux@arm.linux.org.uk \
--cc=linux-arm-kernel@lists.infradead.org \
/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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).