Hi Vladimir, Am Dienstag, den 19.05.2015, 16:11 +0300 schrieb Vladimir Zapolskiy: > 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.
I'd prefer to use %pa for the phys_addr_t types. You could argue that %pa is inappropriate as those are addresses relative to the SRAM region, not physical addresses as seen by the CPU. But following that argument, using phys_addr_t in the first place would not be correct either. Which leads me to question whether we will see larger than 4 GiB SRAM regions in the foreseeable future? regards Philipp -- 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/
