linux-kernel - Re: [PATCH 2/2] serial: msm: Add support for UARTDM cores

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: <20110119222407.GD6335@n2100.arm.linux.org.uk>
Date:	Wed, 19 Jan 2011 22:24:07 +0000
From:	Russell King - ARM Linux <linux@....linux.org.uk>
To:	Stepan Moskovchenko <stepanm@...eaurora.org>
Cc:	Jamie Iles <jamie@...ieiles.com>, linux-arm-msm@...r.kernel.org,
	davidb@...eaurora.org, linux-kernel@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org, rlove@...gle.com
Subject: Re: [PATCH 2/2] serial: msm: Add support for UARTDM cores

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().
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/