Hi Philipp, On 19.05.2015 13:41, Philipp Zabel wrote: > Am Montag, den 18.05.2015, 22:08 +0300 schrieb Vladimir Zapolskiy: >> If devm_gen_pool_create() fails, the previously enabled sram->clk is >> not disabled on probe() exit. >> >> Signed-off-by: Vladimir Zapolskiy <vladimir_zapols...@mentor.com> >> --- >> drivers/misc/sram.c | 9 +++++---- >> 1 file changed, 5 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/misc/sram.c b/drivers/misc/sram.c >> index eeaaf5f..b44a423 100644 >> --- a/drivers/misc/sram.c >> +++ b/drivers/misc/sram.c >> @@ -90,16 +90,17 @@ static int sram_probe(struct platform_device *pdev) >> if (!sram) >> return -ENOMEM; >> >> + sram->pool = devm_gen_pool_create(&pdev->dev, >> + ilog2(SRAM_GRANULARITY), -1); >> + if (!sram->pool) >> + return -ENOMEM; >> + >> sram->clk = devm_clk_get(&pdev->dev, NULL); >> if (IS_ERR(sram->clk)) >> sram->clk = NULL; >> else >> clk_prepare_enable(sram->clk); > > Here you move sram->clk around, and later in patch 7 it gets moved > again. To me it looks like the two should be squashed together.
I agree with you, instead of moving sram->pool up it is better to place sram->clk right at the end of probe(), in other words this patch can be safely merged with patch 7 and the series becomes a bit shorter. Thank you for the finding, I'm going to resend the change, please let me know your opinion about "%pa" vs "0x%llx", if it is needed to be changed or not. >> >> - sram->pool = devm_gen_pool_create(&pdev->dev, ilog2(SRAM_GRANULARITY), >> -1); >> - if (!sram->pool) >> - return -ENOMEM; >> - >> /* >> * We need an additional block to mark the end of the memory region >> * after the reserved blocks from the dt are processed. > > regards > Philipp > -- With best wishes, Vladimir -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
