[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-id: <1433697.46kixytlq2@amdc1032>
Date: Mon, 26 Aug 2013 19:12:35 +0200
From: Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>
To: Mateusz Krawczuk <m.krawczuk@...tner.samsung.com>
Cc: kgene.kim@...sung.com, kyungmin.park@...sung.com,
t.figa@...sung.com, tomasz.figa@...il.com, rob.herring@...xeda.com,
pawel.moll@....com, mark.rutland@....com, swarren@...dotorg.org,
ian.campbell@...rix.com, rob@...dley.net, mturquette@...aro.org,
thomas.abraham@...aro.org, t.stanislaws@...sung.com,
m.chehab@...sung.com, s.nawrocki@...sung.com,
m.szyprowski@...ung.com, linux-kernel@...r.kernel.org,
linux-samsung-soc@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux@....linux.org.uk,
devicetree@...r.kernel.org, linux-doc@...r.kernel.org
Subject: Re: [PATCH RFC 4/5] clk: samsung: Add clock driver for s5pc110/s5pv210
On Monday, August 26, 2013 01:38:33 PM Mateusz Krawczuk wrote:
> This patch adds new, Common Clock Framework-based clock driver for Samsung
> S5PV210 SoCs. The driver is just added, without enabling it yet.
>
> Signed-off-by: Mateusz Krawczuk <m.krawczuk@...tner.samsung.com>
> ---
> .../bindings/clock/samsung,s5pv210-clock.txt | 72 ++
> drivers/clk/samsung/Makefile | 3 +-
> drivers/clk/samsung/clk-s5pv210.c | 732 +++++++++++++++++++++
> include/dt-bindings/clock/samsung,s5pv210-clock.h | 221 +++++++
> 4 files changed, 1027 insertions(+), 1 deletion(-)
> create mode 100644 Documentation/devicetree/bindings/clock/samsung,s5pv210-clock.txt
> create mode 100644 drivers/clk/samsung/clk-s5pv210.c
> create mode 100644 include/dt-bindings/clock/samsung,s5pv210-clock.h
[...]
> diff --git a/drivers/clk/samsung/clk-s5pv210.c b/drivers/clk/samsung/clk-s5pv210.c
> new file mode 100644
> index 0000000..861d37d
> --- /dev/null
> +++ b/drivers/clk/samsung/clk-s5pv210.c
[...]
> +static unsigned long s5pv210_get_xom(void)
This function can be marked with __init.
> +{
> + unsigned long xom = 1;
> + void __iomem *chipid_base;
> + struct device_node *np;
> +
> + np = of_find_compatible_node(NULL, NULL, "samsung,s5pv210-chipid");
> + if (np) {
> + chipid_base = of_iomap(np, 0);
How's about printing an error if !chipid_base?
Also the code can be made a bit shorter:
if (np) {
void __iomem *chipid_base = of_iomap(np, 0);
...
}
> + if (chipid_base)
> + xom = readl(chipid_base + 8);
> +
> + iounmap(chipid_base);
It seems that at least generic iounmap() accepts NULL argument but it
will be better not to call it if of_iomap() failed.
> + }
> +
> + return xom;
> +}
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
--
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/
Powered by blists - more mailing lists