* re: serial: sc16is7xx
2014-07-25 12:20 serial: sc16is7xx Dan Carpenter
@ 2014-07-25 16:54 ` Jon Ringle
0 siblings, 0 replies; 2+ messages in thread
From: Jon Ringle @ 2014-07-25 16:54 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Ringle, Jonathan, linux-serial@vger.kernel.org
On Fri, 25 Jul 2014, Dan Carpenter wrote:
> Hello Jon Ringle,
>
> The patch dfeae619d781: "serial: sc16is7xx" from Apr 24, 2014, leads
> to the following static checker warning:
>
> drivers/tty/serial/sc16is7xx.c:1168 sc16is7xx_probe()
> warn: 's->clk' isn't an ERR_PTR
>
> drivers/tty/serial/sc16is7xx.c
> 1167 out_clk:
> 1168 if (!IS_ERR(s->clk))
> ^^^^^^^^^^^^^
> This check isn't needed because we return directly but also I don't
> think we ever set s->clk so it is leaked on the remove() path as well.
>
> 1169 clk_disable_unprepare(s->clk);
> 1170
> 1171 return ret;
> 1172 }
I'm not too familiar with the clk interface, but we have this code:
| static int sc16is7xx_probe(struct device *dev,
| struct sc16is7xx_devtype *devtype,
| struct regmap *regmap, int irq, unsigned long flags)
| {
| unsigned long freq, *pfreq = dev_get_platdata(dev);
| struct clk *clk;
| int i, ret;
| struct sc16is7xx_port *s;
|
| if (IS_ERR(regmap))
| return PTR_ERR(regmap);
|
| /* Alloc port structure */
| s = devm_kzalloc(dev, sizeof(*s) +
| sizeof(struct sc16is7xx_one) * devtype->nr_uart,
| GFP_KERNEL);
| if (!s) {
| dev_err(dev, "Error allocating port structure\n");
| return -ENOMEM;
| }
|
| clk = devm_clk_get(dev, NULL);
should this be s->clk = devm_clk_get(dev, NULL); ??
| if (IS_ERR(clk)) {
| if (pfreq)
| freq = *pfreq;
| else
| return PTR_ERR(clk);
| } else {
| freq = clk_get_rate(clk);
| }
Would the above change make the clk_disable_unprepare(s->clk) then
correct? Or should the clk member of struct sc16is7xx_port be removed?
Jon
^ permalink raw reply [flat|nested] 2+ messages in thread