Fixes: 847b8bf62eb4 ("rtc: sun6i: Expose the 32kHz oscillator")
Cc: Maxime Ripard <[email protected]>
Cc: Rob Herring <[email protected]>
Signed-off-by: Alexey Klimov <[email protected]>
---
drivers/rtc/rtc-sun6i.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/rtc/rtc-sun6i.c b/drivers/rtc/rtc-sun6i.c
index 39cbc12..7e7da60 100644
--- a/drivers/rtc/rtc-sun6i.c
+++ b/drivers/rtc/rtc-sun6i.c
@@ -193,12 +193,12 @@ static void __init sun6i_rtc_clk_init(struct device_node *node)
rtc = kzalloc(sizeof(*rtc), GFP_KERNEL);
if (!rtc)
return;
- spin_lock_init(&rtc->lock);
clk_data = kzalloc(sizeof(*clk_data) + sizeof(*clk_data->hws),
GFP_KERNEL);
if (!clk_data)
return;
+
spin_lock_init(&rtc->lock);
rtc->base = of_io_request_and_map(node, 0, of_node_full_name(node));
--
1.9.1
The memory allocated for rtc and clk_data will never be freed in
sun6i_rtc_clk_init() in case of error and return. This patch adds
required error path with memory freeing.
Fixes: 847b8bf62eb4 ("rtc: sun6i: Expose the 32kHz oscillator")
Cc: Maxime Ripard <[email protected]>
Cc: Rob Herring <[email protected]>
Signed-off-by: Alexey Klimov <[email protected]>
---
drivers/rtc/rtc-sun6i.c | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/rtc/rtc-sun6i.c b/drivers/rtc/rtc-sun6i.c
index 7e7da60..77bc4d3 100644
--- a/drivers/rtc/rtc-sun6i.c
+++ b/drivers/rtc/rtc-sun6i.c
@@ -197,14 +197,14 @@ static void __init sun6i_rtc_clk_init(struct device_node *node)
clk_data = kzalloc(sizeof(*clk_data) + sizeof(*clk_data->hws),
GFP_KERNEL);
if (!clk_data)
- return;
+ goto out_rtc_free;
spin_lock_init(&rtc->lock);
rtc->base = of_io_request_and_map(node, 0, of_node_full_name(node));
if (IS_ERR(rtc->base)) {
pr_crit("Can't map RTC registers");
- return;
+ goto out_clk_data_free;
}
/* Switch to the external, more precise, oscillator */
@@ -216,7 +216,7 @@ static void __init sun6i_rtc_clk_init(struct device_node *node)
/* Deal with old DTs */
if (!of_get_property(node, "clocks", NULL))
- return;
+ goto out_clk_data_free;
rtc->int_osc = clk_hw_register_fixed_rate_with_accuracy(NULL,
"rtc-int-osc",
@@ -225,7 +225,7 @@ static void __init sun6i_rtc_clk_init(struct device_node *node)
300000000);
if (IS_ERR(rtc->int_osc)) {
pr_crit("Couldn't register the internal oscillator\n");
- return;
+ goto out_clk_data_free;
}
parents[0] = clk_hw_get_name(rtc->int_osc);
@@ -240,12 +240,19 @@ static void __init sun6i_rtc_clk_init(struct device_node *node)
rtc->losc = clk_register(NULL, &rtc->hw);
if (IS_ERR(rtc->losc)) {
pr_crit("Couldn't register the LOSC clock\n");
- return;
+ goto out_clk_data_free;
}
clk_data->num = 1;
clk_data->hws[0] = &rtc->hw;
of_clk_add_hw_provider(node, of_clk_hw_onecell_get, clk_data);
+
+ return;
+
+out_clk_data_free:
+ kfree(clk_data);
+out_rtc_free:
+ kfree(rtc);
}
CLK_OF_DECLARE_DRIVER(sun6i_rtc_clk, "allwinner,sun6i-a31-rtc",
sun6i_rtc_clk_init);
--
1.9.1
Signed-off-by: Alexey Klimov <[email protected]>
---
drivers/rtc/rtc-sun6i.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/rtc/rtc-sun6i.c b/drivers/rtc/rtc-sun6i.c
index 77bc4d3..1886b85 100644
--- a/drivers/rtc/rtc-sun6i.c
+++ b/drivers/rtc/rtc-sun6i.c
@@ -362,7 +362,7 @@ static int sun6i_rtc_setalarm(struct device *dev, struct rtc_wkalrm *wkalrm)
unsigned long time_now = 0;
unsigned long time_set = 0;
unsigned long time_gap = 0;
- int ret = 0;
+ int ret;
ret = sun6i_rtc_gettime(dev, &tm_now);
if (ret < 0) {
--
1.9.1
On Wed, Jul 12, 2017 at 6:59 PM, Alexey Klimov <[email protected]> wrote:
> The memory allocated for rtc and clk_data will never be freed in
> sun6i_rtc_clk_init() in case of error and return. This patch adds
> required error path with memory freeing.
>
> Fixes: 847b8bf62eb4 ("rtc: sun6i: Expose the 32kHz oscillator")
> Cc: Maxime Ripard <[email protected]>
> Cc: Rob Herring <[email protected]>
> Signed-off-by: Alexey Klimov <[email protected]>
> ---
> drivers/rtc/rtc-sun6i.c | 17 ++++++++++++-----
> 1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/rtc/rtc-sun6i.c b/drivers/rtc/rtc-sun6i.c
> index 7e7da60..77bc4d3 100644
> --- a/drivers/rtc/rtc-sun6i.c
> +++ b/drivers/rtc/rtc-sun6i.c
> @@ -197,14 +197,14 @@ static void __init sun6i_rtc_clk_init(struct device_node *node)
> clk_data = kzalloc(sizeof(*clk_data) + sizeof(*clk_data->hws),
> GFP_KERNEL);
> if (!clk_data)
> - return;
> + goto out_rtc_free;
>
> spin_lock_init(&rtc->lock);
>
> rtc->base = of_io_request_and_map(node, 0, of_node_full_name(node));
> if (IS_ERR(rtc->base)) {
> pr_crit("Can't map RTC registers");
> - return;
> + goto out_clk_data_free;
> }
>
> /* Switch to the external, more precise, oscillator */
> @@ -216,7 +216,7 @@ static void __init sun6i_rtc_clk_init(struct device_node *node)
>
> /* Deal with old DTs */
> if (!of_get_property(node, "clocks", NULL))
> - return;
> + goto out_clk_data_free;
>
> rtc->int_osc = clk_hw_register_fixed_rate_with_accuracy(NULL,
> "rtc-int-osc",
> @@ -225,7 +225,7 @@ static void __init sun6i_rtc_clk_init(struct device_node *node)
> 300000000);
> if (IS_ERR(rtc->int_osc)) {
> pr_crit("Couldn't register the internal oscillator\n");
> - return;
> + goto out_clk_data_free;
> }
>
> parents[0] = clk_hw_get_name(rtc->int_osc);
> @@ -240,12 +240,19 @@ static void __init sun6i_rtc_clk_init(struct device_node *node)
> rtc->losc = clk_register(NULL, &rtc->hw);
> if (IS_ERR(rtc->losc)) {
> pr_crit("Couldn't register the LOSC clock\n");
> - return;
> + goto out_clk_data_free;
You should also unregister the fixed rate clk above.
> }
>
> clk_data->num = 1;
> clk_data->hws[0] = &rtc->hw;
> of_clk_add_hw_provider(node, of_clk_hw_onecell_get, clk_data);
> +
> + return;
> +
> +out_clk_data_free:
> + kfree(clk_data);
> +out_rtc_free:
> + kfree(rtc);
rtc can not be freed. It has already been assigned to sun6i_rtc, a static
variable within this module. It then gets used by the actual RTC driver
later in this file to get the register base pointer. This was done to
avoid issues with trying to map the same I/O addresses twice.
Regards
ChenYu
> }
> CLK_OF_DECLARE_DRIVER(sun6i_rtc_clk, "allwinner,sun6i-a31-rtc",
> sun6i_rtc_clk_init);
> --
> 1.9.1
>
On Wed, Jul 12, 2017 at 6:59 PM, Alexey Klimov <[email protected]> wrote:
A bit more description would be nice.
> Signed-off-by: Alexey Klimov <[email protected]>
Otherwise,
Reviewed-by: Chen-Yu Tsai <[email protected]>
On Wed, Jul 12, 2017 at 6:59 PM, Alexey Klimov <[email protected]> wrote:
A bit more description would be nice. Maybe a kernel message that
prompted this fix?
> Fixes: 847b8bf62eb4 ("rtc: sun6i: Expose the 32kHz oscillator")
> Cc: Maxime Ripard <[email protected]>
> Cc: Rob Herring <[email protected]>
> Signed-off-by: Alexey Klimov <[email protected]>
Otherwise,
Reviewed-by: Chen-Yu Tsai <[email protected]>
On 12/07/2017 at 11:59:48 +0100, Alexey Klimov wrote:
> Fixes: 847b8bf62eb4 ("rtc: sun6i: Expose the 32kHz oscillator")
> Cc: Maxime Ripard <[email protected]>
> Cc: Rob Herring <[email protected]>
> Signed-off-by: Alexey Klimov <[email protected]>
> ---
> drivers/rtc/rtc-sun6i.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
Applied, after adding a proper commit message.
--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
Hi,
On 12/07/2017 at 11:59:50 +0100, Alexey Klimov wrote:
> Signed-off-by: Alexey Klimov <[email protected]>
> ---
> drivers/rtc/rtc-sun6i.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/rtc/rtc-sun6i.c b/drivers/rtc/rtc-sun6i.c
> index 77bc4d3..1886b85 100644
> --- a/drivers/rtc/rtc-sun6i.c
> +++ b/drivers/rtc/rtc-sun6i.c
> @@ -362,7 +362,7 @@ static int sun6i_rtc_setalarm(struct device *dev, struct rtc_wkalrm *wkalrm)
> unsigned long time_now = 0;
> unsigned long time_set = 0;
> unsigned long time_gap = 0;
> - int ret = 0;
> + int ret;
>
I don't see any reason that would justify that change.
> ret = sun6i_rtc_gettime(dev, &tm_now);
> if (ret < 0) {
> --
> 1.9.1
>
--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com