Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1031036AbdIZOqI (ORCPT ); Tue, 26 Sep 2017 10:46:08 -0400 Received: from mail-lf0-f66.google.com ([209.85.215.66]:32971 "EHLO mail-lf0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1030516AbdIZOqF (ORCPT ); Tue, 26 Sep 2017 10:46:05 -0400 X-Google-Smtp-Source: AOwi7QAe/bMjwSY/SKlwKViEMgoX0XOTvbcIeL/R/p0sXLAeimtHD6haPPMP3V3pO8z2Vo7VN20mFQ== Subject: Re: [PATCH v1 1/5] clk: tegra: Add AHB DMA clock entry To: Peter De Schrijver Cc: Thierry Reding , Jonathan Hunter , Laxman Dewangan , Prashant Gaikwad , Michael Turquette , Stephen Boyd , Rob Herring , Vinod Koul , linux-tegra@vger.kernel.org, devicetree@vger.kernel.org, dmaengine@vger.kernel.org, linux-clk@vger.kernel.org, linux-kernel@vger.kernel.org References: <20170926095611.GW6290@tbergstrom-lnx.Nvidia.com> From: Dmitry Osipenko Message-ID: Date: Tue, 26 Sep 2017 17:46:01 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: <20170926095611.GW6290@tbergstrom-lnx.Nvidia.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4827 Lines: 113 On 26.09.2017 12:56, Peter De Schrijver wrote: > On Tue, Sep 26, 2017 at 02:22:02AM +0300, Dmitry Osipenko wrote: >> AHB DMA presents on Tegra20/30. Add missing entries, so that driver >> for AHB DMA could be implemented. >> >> Signed-off-by: Dmitry Osipenko >> --- >> drivers/clk/tegra/clk-id.h | 1 + >> drivers/clk/tegra/clk-tegra-periph.c | 1 + >> drivers/clk/tegra/clk-tegra20.c | 6 ++++++ >> drivers/clk/tegra/clk-tegra30.c | 2 ++ >> 4 files changed, 10 insertions(+) >> >> diff --git a/drivers/clk/tegra/clk-id.h b/drivers/clk/tegra/clk-id.h >> index 689f344377a7..c1661b47bbda 100644 >> --- a/drivers/clk/tegra/clk-id.h >> +++ b/drivers/clk/tegra/clk-id.h >> @@ -12,6 +12,7 @@ enum clk_id { >> tegra_clk_amx, >> tegra_clk_amx1, >> tegra_clk_apb2ape, >> + tegra_clk_ahbdma, >> tegra_clk_apbdma, >> tegra_clk_apbif, >> tegra_clk_ape, >> diff --git a/drivers/clk/tegra/clk-tegra-periph.c b/drivers/clk/tegra/clk-tegra-periph.c >> index 848255cc0209..95a3d8c95f06 100644 >> --- a/drivers/clk/tegra/clk-tegra-periph.c >> +++ b/drivers/clk/tegra/clk-tegra-periph.c >> @@ -823,6 +823,7 @@ static struct tegra_periph_init_data gate_clks[] = { >> GATE("timer", "clk_m", 5, 0, tegra_clk_timer, CLK_IS_CRITICAL), >> GATE("isp", "clk_m", 23, 0, tegra_clk_isp, 0), >> GATE("vcp", "clk_m", 29, 0, tegra_clk_vcp, 0), >> + GATE("ahbdma", "clk_m", 33, 0, tegra_clk_ahbdma, 0), > > Parent for this should be hclk on Tegra30 and later chips as well.. > It looks like other clocks have a wrong parent too here, aren't they? Like for example "apbdma" should have "pclk" as a parent, isn't it? >> GATE("apbdma", "clk_m", 34, 0, tegra_clk_apbdma, 0), >> GATE("kbc", "clk_32k", 36, TEGRA_PERIPH_ON_APB | TEGRA_PERIPH_NO_RESET, tegra_clk_kbc, 0), >> GATE("fuse", "clk_m", 39, TEGRA_PERIPH_ON_APB, tegra_clk_fuse, 0), >> diff --git a/drivers/clk/tegra/clk-tegra20.c b/drivers/clk/tegra/clk-tegra20.c >> index 837e5cbd60e9..e76c0d292ca7 100644 >> --- a/drivers/clk/tegra/clk-tegra20.c >> +++ b/drivers/clk/tegra/clk-tegra20.c >> @@ -449,6 +449,7 @@ static struct tegra_devclk devclks[] __initdata = { >> { .con_id = "audio", .dt_id = TEGRA20_CLK_AUDIO }, >> { .con_id = "audio_2x", .dt_id = TEGRA20_CLK_AUDIO_2X }, >> { .dev_id = "tegra20-ac97", .dt_id = TEGRA20_CLK_AC97 }, >> + { .dev_id = "tegra-ahbdma", .dt_id = TEGRA20_CLK_AHBDMA }, > > This isn't needed if you use DT bindings to get the clock handle. > Yes, I added it for consistency. Shouldn't we get rid of that all legacy stuff already? >> { .dev_id = "tegra-apbdma", .dt_id = TEGRA20_CLK_APBDMA }, >> { .dev_id = "rtc-tegra", .dt_id = TEGRA20_CLK_RTC }, >> { .dev_id = "timer", .dt_id = TEGRA20_CLK_TIMER }, >> @@ -806,6 +807,11 @@ static void __init tegra20_periph_clk_init(void) >> clk_base, 0, 3, periph_clk_enb_refcnt); >> clks[TEGRA20_CLK_AC97] = clk; >> >> + /* ahbdma */ >> + clk = tegra_clk_register_periph_gate("ahbdma", "hclk", 0, clk_base, >> + 0, 33, periph_clk_enb_refcnt); >> + clks[TEGRA20_CLK_AHBDMA] = clk; >> + > > You can use the generic definition here if you correct the entry above. > Good point, same applies to "apbdma". Thank you for the suggestion. >> /* apbdma */ >> clk = tegra_clk_register_periph_gate("apbdma", "pclk", 0, clk_base, >> 0, 34, periph_clk_enb_refcnt); >> diff --git a/drivers/clk/tegra/clk-tegra30.c b/drivers/clk/tegra/clk-tegra30.c >> index a2d163f759b4..e99701557f29 100644 >> --- a/drivers/clk/tegra/clk-tegra30.c >> +++ b/drivers/clk/tegra/clk-tegra30.c >> @@ -612,6 +612,7 @@ static struct tegra_devclk devclks[] __initdata = { >> { .con_id = "fuse_burn", .dev_id = "fuse-tegra", .dt_id = TEGRA30_CLK_FUSE_BURN }, >> { .con_id = "apbif", .dev_id = "tegra30-ahub", .dt_id = TEGRA30_CLK_APBIF }, >> { .con_id = "hda2hdmi", .dev_id = "tegra30-hda", .dt_id = TEGRA30_CLK_HDA2HDMI }, >> + { .dev_id = "tegra-ahbdma", .dt_id = TEGRA30_CLK_AHBDMA }, > > Same as for Tegra20. > >> { .dev_id = "tegra-apbdma", .dt_id = TEGRA30_CLK_APBDMA }, >> { .dev_id = "rtc-tegra", .dt_id = TEGRA30_CLK_RTC }, >> { .dev_id = "timer", .dt_id = TEGRA30_CLK_TIMER }, >> @@ -788,6 +789,7 @@ static struct tegra_clk tegra30_clks[tegra_clk_max] __initdata = { >> [tegra_clk_extern3] = { .dt_id = TEGRA30_CLK_EXTERN3, .present = true }, >> [tegra_clk_disp1] = { .dt_id = TEGRA30_CLK_DISP1, .present = true }, >> [tegra_clk_disp2] = { .dt_id = TEGRA30_CLK_DISP2, .present = true }, >> + [tegra_clk_ahbdma] = { .dt_id = TEGRA30_CLK_AHBDMA, .present = true }, >> [tegra_clk_apbdma] = { .dt_id = TEGRA30_CLK_APBDMA, .present = true }, >> [tegra_clk_rtc] = { .dt_id = TEGRA30_CLK_RTC, .present = true }, >> [tegra_clk_timer] = { .dt_id = TEGRA30_CLK_TIMER, .present = true }, > > Cheers, > > Peter. > -- Dmitry