Message ID | 1413196434-5292-1-git-send-email-thierry.reding@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Hi, On Mon, Oct 13, 2014 at 3:33 AM, Thierry Reding <thierry.reding@gmail.com> wrote: [...] > diff --git a/drivers/memory/tegra/tegra-mc.c b/drivers/memory/tegra/tegra-mc.c > new file mode 100644 > index 000000000000..0f0c8be096d0 > --- /dev/null > +++ b/drivers/memory/tegra/tegra-mc.c > @@ -0,0 +1,1061 @@ > +/* > + * Copyright (C) 2014 NVIDIA CORPORATION. All rights reserved. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + */ > + > +#include <linux/clk.h> > +#include <linux/interrupt.h> > +#include <linux/io.h> > +#include <linux/iommu.h> > +#include <linux/kernel.h> > +#include <linux/module.h> You need a linux/mm.h in here (on 64-bit). > +#include <linux/of.h> > +#include <linux/of_platform.h> > +#include <linux/platform_device.h> > +#include <linux/slab.h> [...] > diff --git a/drivers/memory/tegra/tegra124-mc.c b/drivers/memory/tegra/tegra124-mc.c > new file mode 100644 > index 000000000000..db31c96fc288 > --- /dev/null > +++ b/drivers/memory/tegra/tegra124-mc.c [...] > @@ -0,0 +1,1028 @@ > +/* > + * Copyright (C) 2014 NVIDIA CORPORATION. All rights reserved. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + */ > + > +#include <linux/of.h> > +#include <linux/mm.h> > + > +#include <asm/cacheflush.h> > + > +#include <dt-bindings/memory/tegra124-mc.h> > + > +#include "tegra-mc.h" > + > +static const struct tegra_mc_client tegra124_mc_clients[] = { > + { > + .id = 0x00, > + .name = "ptcr", > + .swgroup = TEGRA_SWGROUP_PTC, > + }, { > + .id = 0x01, > + .name = "display0a", > + .swgroup = TEGRA_SWGROUP_DC, > + .smmu = { > + .reg = 0x228, > + .bit = 1, > + }, > + .latency = { > + .reg = 0x2e8, > + .shift = 0, > + .mask = 0xff, > + .def = 0xc2, > + }, > + }, { These are very verbose tables. Having a macro for the initializers could help density a lot. > +#ifdef CONFIG_ARCH_TEGRA_132_SOC > +static void tegra132_flush_dcache(struct page *page, unsigned long offset, > + size_t size) > +{ > + void *virt = page_address(page) + offset; > + > + __flush_dcache_area(virt, size); > +} > + > +static const struct tegra_smmu_ops tegra132_smmu_ops = { > + .flush_dcache = tegra132_flush_dcache, > +}; > + > +static const struct tegra_smmu_soc tegra132_smmu_soc = { > + .groups = tegra124_smmu_groups, > + .num_groups = ARRAY_SIZE(tegra124_smmu_groups), > + .clients = tegra124_mc_clients, > + .num_clients = ARRAY_SIZE(tegra124_mc_clients), > + .swgroups = tegra124_swgroups, > + .num_swgroups = ARRAY_SIZE(tegra124_swgroups), > + .supports_round_robin_arbitration = true, > + .supports_request_limit = true, > + .num_address_bits = 34, > + .num_asids = 128, > + .ops = &tegra132_smmu_ops, > +}; > + > +const struct tegra_mc_soc tegra132_mc_soc = { > + .clients = tegra124_mc_clients, > + .num_clients = ARRAY_SIZE(tegra124_mc_clients), > + .atom_size = 32, > + .smmu = &tegra132_smmu_soc, > +}; > +#endif /* CONFIG_ARCH_TEGRA_132_SOC */ This won't compile -- several of the tegra132_smmu_soc members are no longer valid. In particular: groups num_groups supports_round_robin_arbitration supports_request_limit -Olof
Hi, Oh, a few more comments: On Mon, Oct 13, 2014 at 3:33 AM, Thierry Reding <thierry.reding@gmail.com> wrote: > diff --git a/drivers/memory/Makefile b/drivers/memory/Makefile > index c32d31981be3..1c932e7e7b8d 100644 > --- a/drivers/memory/Makefile > +++ b/drivers/memory/Makefile > @@ -12,4 +12,5 @@ obj-$(CONFIG_FSL_CORENET_CF) += fsl-corenet-cf.o > obj-$(CONFIG_FSL_IFC) += fsl_ifc.o > obj-$(CONFIG_MVEBU_DEVBUS) += mvebu-devbus.o > obj-$(CONFIG_TEGRA20_MC) += tegra20-mc.o > -obj-$(CONFIG_TEGRA30_MC) += tegra30-mc.o > + > +obj-$(CONFIG_ARCH_TEGRA) += tegra/ > diff --git a/drivers/memory/tegra/Makefile b/drivers/memory/tegra/Makefile > new file mode 100644 > index 000000000000..51b9e8fcde1b > --- /dev/null > +++ b/drivers/memory/tegra/Makefile > @@ -0,0 +1,5 @@ > +obj-y = tegra-mc.o > +obj-$(CONFIG_ARCH_TEGRA_3x_SOC) += tegra30-mc.o > +obj-$(CONFIG_ARCH_TEGRA_114_SOC) += tegra114-mc.o > +obj-$(CONFIG_ARCH_TEGRA_124_SOC) += tegra124-mc.o > +obj-$(CONFIG_ARCH_TEGRA_132_SOC) += tegra124-mc.o You'll need a Kconfig and not just a makefile -- there are definitely dependencies on this driver (IOMMU in particular). Also, the problem of having a global enable bit that is only under control of TrustZone FW is a big problem -- if the bit is not set, the driver will not work (and the machine will crash). I think you'll need to come up with a way to detect that in the driver. I don't have a good idea of how it can be done though. -Olof
On 13 October 2014 12:33, Thierry Reding <thierry.reding@gmail.com> wrote: > From: Thierry Reding <treding@nvidia.com> > > The memory controller clock runs either at half or the same frequency as > the EMC clock. > > Signed-off-by: Thierry Reding <treding@nvidia.com> > --- > Changes in v3: > - split registration into a separate function that can be reused for all > SoC generations, but pass in the name and parent parameters for > clarity as well as the register address (in case it ever changes) and > the EMC spin-lock since it isn't globally available > > drivers/clk/tegra/clk-divider.c | 13 +++++++++++++ > drivers/clk/tegra/clk-tegra114.c | 7 ++++++- > drivers/clk/tegra/clk-tegra124.c | 7 ++++++- > drivers/clk/tegra/clk-tegra20.c | 8 +++++++- > drivers/clk/tegra/clk-tegra30.c | 7 ++++++- > drivers/clk/tegra/clk.h | 2 ++ > include/dt-bindings/clock/tegra114-car.h | 2 +- > include/dt-bindings/clock/tegra124-car.h | 2 +- > include/dt-bindings/clock/tegra20-car.h | 2 +- > 9 files changed, 43 insertions(+), 7 deletions(-) > > diff --git a/drivers/clk/tegra/clk-divider.c b/drivers/clk/tegra/clk-divider.c > index 290f9c1a3749..84e1b3c1fb2a 100644 > --- a/drivers/clk/tegra/clk-divider.c > +++ b/drivers/clk/tegra/clk-divider.c > @@ -185,3 +185,16 @@ struct clk *tegra_clk_register_divider(const char *name, > > return clk; > } > + > +static const struct clk_div_table mc_div_table[] = { > + { .val = 0, .div = 2 }, > + { .val = 1, .div = 1 }, > + { .val = 0, .div = 0 }, > +}; > + > +struct clk *tegra_clk_register_mc(const char *name, const char *parent_name, > + void __iomem *reg, spinlock_t *lock) > +{ > + return clk_register_divider_table(NULL, "mc", "emc_mux", 0, reg, Should take name and parent_name from the passed args? With that change: Reviewed-By: Tomeu Vizoso <tomeu.vizoso@collabora.com> Cheers, Tomeu > + 16, 1, 0, mc_div_table, lock); > +} > diff --git a/drivers/clk/tegra/clk-tegra114.c b/drivers/clk/tegra/clk-tegra114.c > index f760f31d05c4..0b03d2cf7264 100644 > --- a/drivers/clk/tegra/clk-tegra114.c > +++ b/drivers/clk/tegra/clk-tegra114.c > @@ -173,6 +173,7 @@ static DEFINE_SPINLOCK(pll_d_lock); > static DEFINE_SPINLOCK(pll_d2_lock); > static DEFINE_SPINLOCK(pll_u_lock); > static DEFINE_SPINLOCK(pll_re_lock); > +static DEFINE_SPINLOCK(emc_lock); > > static struct div_nmp pllxc_nmp = { > .divm_shift = 0, > @@ -1228,7 +1229,11 @@ static __init void tegra114_periph_clk_init(void __iomem *clk_base, > ARRAY_SIZE(mux_pllmcp_clkm), > CLK_SET_RATE_NO_REPARENT, > clk_base + CLK_SOURCE_EMC, > - 29, 3, 0, NULL); > + 29, 3, 0, &emc_lock); > + > + clk = tegra_clk_register_mc("mc", "emc_mux", clk_base + CLK_SOURCE_EMC, > + &emc_lock); > + clks[TEGRA114_CLK_MC] = clk; > > for (i = 0; i < ARRAY_SIZE(tegra_periph_clk_list); i++) { > data = &tegra_periph_clk_list[i]; > diff --git a/drivers/clk/tegra/clk-tegra124.c b/drivers/clk/tegra/clk-tegra124.c > index e3a85842ce0c..f5f9baca7bb6 100644 > --- a/drivers/clk/tegra/clk-tegra124.c > +++ b/drivers/clk/tegra/clk-tegra124.c > @@ -132,6 +132,7 @@ static DEFINE_SPINLOCK(pll_d2_lock); > static DEFINE_SPINLOCK(pll_e_lock); > static DEFINE_SPINLOCK(pll_re_lock); > static DEFINE_SPINLOCK(pll_u_lock); > +static DEFINE_SPINLOCK(emc_lock); > > /* possible OSC frequencies in Hz */ > static unsigned long tegra124_input_freq[] = { > @@ -1127,7 +1128,11 @@ static __init void tegra124_periph_clk_init(void __iomem *clk_base, > clk = clk_register_mux(NULL, "emc_mux", mux_pllmcp_clkm, > ARRAY_SIZE(mux_pllmcp_clkm), 0, > clk_base + CLK_SOURCE_EMC, > - 29, 3, 0, NULL); > + 29, 3, 0, &emc_lock); > + > + clk = tegra_clk_register_mc("mc", "emc_mux", clk_base + CLK_SOURCE_EMC, > + &emc_lock); > + clks[TEGRA124_CLK_MC] = clk; > > /* cml0 */ > clk = clk_register_gate(NULL, "cml0", "pll_e", 0, clk_base + PLLE_AUX, > diff --git a/drivers/clk/tegra/clk-tegra20.c b/drivers/clk/tegra/clk-tegra20.c > index dace2b1b5ae6..41272dcc9e22 100644 > --- a/drivers/clk/tegra/clk-tegra20.c > +++ b/drivers/clk/tegra/clk-tegra20.c > @@ -140,6 +140,8 @@ static struct cpu_clk_suspend_context { > static void __iomem *clk_base; > static void __iomem *pmc_base; > > +static DEFINE_SPINLOCK(emc_lock); > + > #define TEGRA_INIT_DATA_MUX(_name, _parents, _offset, \ > _clk_num, _gate_flags, _clk_id) \ > TEGRA_INIT_DATA(_name, NULL, NULL, _parents, _offset, \ > @@ -819,11 +821,15 @@ static void __init tegra20_periph_clk_init(void) > ARRAY_SIZE(mux_pllmcp_clkm), > CLK_SET_RATE_NO_REPARENT, > clk_base + CLK_SOURCE_EMC, > - 30, 2, 0, NULL); > + 30, 2, 0, &emc_lock); > clk = tegra_clk_register_periph_gate("emc", "emc_mux", 0, clk_base, 0, > 57, periph_clk_enb_refcnt); > clks[TEGRA20_CLK_EMC] = clk; > > + clk = tegra_clk_register_mc("mc", "emc_mux", clk_base + CLK_SOURCE_EMC, > + &emc_lock); > + clks[TEGRA20_CLK_MC] = clk; > + > /* dsi */ > clk = tegra_clk_register_periph_gate("dsi", "pll_d", 0, clk_base, 0, > 48, periph_clk_enb_refcnt); > diff --git a/drivers/clk/tegra/clk-tegra30.c b/drivers/clk/tegra/clk-tegra30.c > index 5bbacd01094f..4b9d8bd3d0bf 100644 > --- a/drivers/clk/tegra/clk-tegra30.c > +++ b/drivers/clk/tegra/clk-tegra30.c > @@ -177,6 +177,7 @@ static unsigned long input_freq; > > static DEFINE_SPINLOCK(cml_lock); > static DEFINE_SPINLOCK(pll_d_lock); > +static DEFINE_SPINLOCK(emc_lock); > > #define TEGRA_INIT_DATA_MUX(_name, _parents, _offset, \ > _clk_num, _gate_flags, _clk_id) \ > @@ -1157,11 +1158,15 @@ static void __init tegra30_periph_clk_init(void) > ARRAY_SIZE(mux_pllmcp_clkm), > CLK_SET_RATE_NO_REPARENT, > clk_base + CLK_SOURCE_EMC, > - 30, 2, 0, NULL); > + 30, 2, 0, &emc_lock); > clk = tegra_clk_register_periph_gate("emc", "emc_mux", 0, clk_base, 0, > 57, periph_clk_enb_refcnt); > clks[TEGRA30_CLK_EMC] = clk; > > + clk = tegra_clk_register_mc("mc", "emc_mux", clk_base + CLK_SOURCE_EMC, > + &emc_lock); > + clks[TEGRA30_CLK_MC] = clk; > + > /* cml0 */ > clk = clk_register_gate(NULL, "cml0", "pll_e", 0, clk_base + PLLE_AUX, > 0, 0, &cml_lock); > diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h > index 16ec8d6bb87f..4e458aa8d45c 100644 > --- a/drivers/clk/tegra/clk.h > +++ b/drivers/clk/tegra/clk.h > @@ -86,6 +86,8 @@ struct clk *tegra_clk_register_divider(const char *name, > const char *parent_name, void __iomem *reg, > unsigned long flags, u8 clk_divider_flags, u8 shift, u8 width, > u8 frac_width, spinlock_t *lock); > +struct clk *tegra_clk_register_mc(const char *name, const char *parent_name, > + void __iomem *reg, spinlock_t *lock); > > /* > * Tegra PLL: > diff --git a/include/dt-bindings/clock/tegra114-car.h b/include/dt-bindings/clock/tegra114-car.h > index fc12621fb432..534c03f8ad72 100644 > --- a/include/dt-bindings/clock/tegra114-car.h > +++ b/include/dt-bindings/clock/tegra114-car.h > @@ -49,7 +49,7 @@ > #define TEGRA114_CLK_I2S0 30 > /* 31 */ > > -/* 32 */ > +#define TEGRA114_CLK_MC 32 > /* 33 */ > #define TEGRA114_CLK_APBDMA 34 > /* 35 */ > diff --git a/include/dt-bindings/clock/tegra124-car.h b/include/dt-bindings/clock/tegra124-car.h > index 6bac637fd635..af9bc9a3ddbc 100644 > --- a/include/dt-bindings/clock/tegra124-car.h > +++ b/include/dt-bindings/clock/tegra124-car.h > @@ -48,7 +48,7 @@ > #define TEGRA124_CLK_I2S0 30 > /* 31 */ > > -/* 32 */ > +#define TEGRA124_CLK_MC 32 > /* 33 */ > #define TEGRA124_CLK_APBDMA 34 > /* 35 */ > diff --git a/include/dt-bindings/clock/tegra20-car.h b/include/dt-bindings/clock/tegra20-car.h > index 9406207cfac8..04500b243a4d 100644 > --- a/include/dt-bindings/clock/tegra20-car.h > +++ b/include/dt-bindings/clock/tegra20-car.h > @@ -49,7 +49,7 @@ > /* 30 */ > #define TEGRA20_CLK_CACHE2 31 > > -#define TEGRA20_CLK_MEM 32 > +#define TEGRA20_CLK_MC 32 > #define TEGRA20_CLK_AHBDMA 33 > #define TEGRA20_CLK_APBDMA 34 > /* 35 */ > -- > 2.1.2 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-tegra" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
> diff --git a/drivers/memory/tegra/tegra124-mc.c b/drivers/memory/tegra/tegra124-mc.c ... > +static const struct tegra_smmu_swgroup tegra124_swgroups[] = { > + { .swgroup = TEGRA_SWGROUP_DC, .reg = 0x240 }, > + { .swgroup = TEGRA_SWGROUP_DCB, .reg = 0x244 }, > + { .swgroup = TEGRA_SWGROUP_AFI, .reg = 0x238 }, > + { .swgroup = TEGRA_SWGROUP_AVPC, .reg = 0x23c }, > + { .swgroup = TEGRA_SWGROUP_HDA, .reg = 0x254 }, > + { .swgroup = TEGRA_SWGROUP_HC, .reg = 0x250 }, > + { .swgroup = TEGRA_SWGROUP_MSENC, .reg = 0x264 }, > + { .swgroup = TEGRA_SWGROUP_PPCS, .reg = 0x270 }, > + { .swgroup = TEGRA_SWGROUP_SATA, .reg = 0x274 }, > + { .swgroup = TEGRA_SWGROUP_VDE, .reg = 0x27c }, > + { .swgroup = TEGRA_SWGROUP_ISP2, .reg = 0x258 }, > + { .swgroup = TEGRA_SWGROUP_XUSB_HOST, .reg = 0x288 }, > + { .swgroup = TEGRA_SWGROUP_XUSB_DEV, .reg = 0x28c }, > + { .swgroup = TEGRA_SWGROUP_ISP2B, .reg = 0xaa4 }, > + { .swgroup = TEGRA_SWGROUP_TSEC, .reg = 0x294 }, > + { .swgroup = TEGRA_SWGROUP_A9AVP, .reg = 0x290 }, > + { .swgroup = TEGRA_SWGROUP_GPU, .reg = 0xaa8 }, I had to change the .reg of TEGRA_SWGROUP_GPU to 0xaac to get the IOMMU to work with GK20A. The reason is still not completely clear to me, but if you look at the TRM you see that 0xaa8 is basically constant, with the SMMU translation bit hardcoded to DISABLE (and the ASID field being meaningless in that case). However right after that register you have a functional one named GPUB instead of GPU, and this one is fully writeable (and has the expected effect). I will try to get more information about the why of this, but for now this setting is what works for me.
On 30.10.2014 12:03, Alexandre Courbot wrote: > I had to change the .reg of TEGRA_SWGROUP_GPU to 0xaac to get the IOMMU > to work with GK20A. The reason is still not completely clear to me, but > if you look at the TRM you see that 0xaa8 is basically constant, with > the SMMU translation bit hardcoded to DISABLE (and the ASID field being > meaningless in that case). However right after that register you have a > functional one named GPUB instead of GPU, and this one is fully > writeable (and has the expected effect). GPU has two SW group IDs, because it accesses memory both with and without translation. The bit 34 in addresses in f.ex. PTE chooses between the two. GPU is hard-wired to disable translation. For GPUB translation can be enabled. Terje
On Thu, Oct 30, 2014 at 7:18 PM, Terje Bergström <tbergstrom@nvidia.com> wrote: > On 30.10.2014 12:03, Alexandre Courbot wrote: >> I had to change the .reg of TEGRA_SWGROUP_GPU to 0xaac to get the IOMMU >> to work with GK20A. The reason is still not completely clear to me, but >> if you look at the TRM you see that 0xaa8 is basically constant, with >> the SMMU translation bit hardcoded to DISABLE (and the ASID field being >> meaningless in that case). However right after that register you have a >> functional one named GPUB instead of GPU, and this one is fully >> writeable (and has the expected effect). > > GPU has two SW group IDs, because it accesses memory both with and > without translation. The bit 34 in addresses in f.ex. PTE chooses > between the two. Indeed, to enable SMMU translation I have to program 0xaac correctly *and* set the bit 34 of every address that needs to go through the SMMU. > GPU is hard-wired to disable translation. For GPUB translation can be > enabled. So should I understand that the GPU group is for addresses without bit 34 set (hence forcibly disabled) while GPUB is used when that bit is set? Or is it something else?
On 30.10.2014 12:22, Alexandre Courbot wrote: > So should I understand that the GPU group is for addresses without bit > 34 set (hence forcibly disabled) while GPUB is used when that bit is > set? Or is it something else? That's exactly correct. And only GPUB can be programmed to be SMMU translated. Terje
On 10/30/2014 08:04 PM, Terje Bergström wrote: > On 30.10.2014 12:22, Alexandre Courbot wrote: >> So should I understand that the GPU group is for addresses without bit >> 34 set (hence forcibly disabled) while GPUB is used when that bit is >> set? Or is it something else? > > That's exactly correct. And only GPUB can be programmed to be SMMU > translated. Great, thanks for confirming! Thierry, how do you want to address this? We could change the register for the GPU group, or (maybe preferable if we want to reflect the actual hardware state) add the GPUB group. I don't know if that would be easy though since we would have the problem of the gpusrd and gpuswr clients ownership (seems like they would belong to both groups?)
On 30.10.2014 15:35, Alexandre Courbot wrote: > Great, thanks for confirming! > > Thierry, how do you want to address this? We could change the register > for the GPU group, or (maybe preferable if we want to reflect the actual > hardware state) add the GPUB group. I don't know if that would be easy > though since we would have the problem of the gpusrd and gpuswr clients > ownership (seems like they would belong to both groups?) gpusrd and gpuswr are client IDs for GPU reads and writes on MC. GPU and GPUB are SW group IDs for SMMU. There's no 1:1 or hierarchical mapping. Terje
On Thu, Oct 30, 2014 at 03:47:59PM +0200, Terje Bergström wrote: > On 30.10.2014 15:35, Alexandre Courbot wrote: > > Great, thanks for confirming! > > > > Thierry, how do you want to address this? We could change the register > > for the GPU group, or (maybe preferable if we want to reflect the actual > > hardware state) add the GPUB group. I don't know if that would be easy > > though since we would have the problem of the gpusrd and gpuswr clients > > ownership (seems like they would belong to both groups?) > > gpusrd and gpuswr are client IDs for GPU reads and writes on MC. GPU and > GPUB are SW group IDs for SMMU. There's no 1:1 or hierarchical mapping. Since the GPU client ID is effectively useless for purposes of IOMMU translation I'd lean towards just keeping the existing TEGRA_SWGROUP_GPU and update the register to point to MC_SMMU_GPUB_ASID_0. Thierry
On Mon, Oct 20, 2014 at 01:02:45PM +0200, Tomeu Vizoso wrote: > On 13 October 2014 12:33, Thierry Reding <thierry.reding@gmail.com> wrote: [...] > > +struct clk *tegra_clk_register_mc(const char *name, const char *parent_name, > > + void __iomem *reg, spinlock_t *lock) > > +{ > > + return clk_register_divider_table(NULL, "mc", "emc_mux", 0, reg, > > Should take name and parent_name from the passed args? > > With that change: > > Reviewed-By: Tomeu Vizoso <tomeu.vizoso@collabora.com> Good catch. Thanks, Thierry
On Wed, Oct 15, 2014 at 03:09:30PM -0700, Olof Johansson wrote: > Hi, > > Oh, a few more comments: > > On Mon, Oct 13, 2014 at 3:33 AM, Thierry Reding > <thierry.reding@gmail.com> wrote: > > > diff --git a/drivers/memory/Makefile b/drivers/memory/Makefile > > index c32d31981be3..1c932e7e7b8d 100644 > > --- a/drivers/memory/Makefile > > +++ b/drivers/memory/Makefile > > @@ -12,4 +12,5 @@ obj-$(CONFIG_FSL_CORENET_CF) += fsl-corenet-cf.o > > obj-$(CONFIG_FSL_IFC) += fsl_ifc.o > > obj-$(CONFIG_MVEBU_DEVBUS) += mvebu-devbus.o > > obj-$(CONFIG_TEGRA20_MC) += tegra20-mc.o > > -obj-$(CONFIG_TEGRA30_MC) += tegra30-mc.o > > + > > +obj-$(CONFIG_ARCH_TEGRA) += tegra/ > > diff --git a/drivers/memory/tegra/Makefile b/drivers/memory/tegra/Makefile > > new file mode 100644 > > index 000000000000..51b9e8fcde1b > > --- /dev/null > > +++ b/drivers/memory/tegra/Makefile > > @@ -0,0 +1,5 @@ > > +obj-y = tegra-mc.o > > +obj-$(CONFIG_ARCH_TEGRA_3x_SOC) += tegra30-mc.o > > +obj-$(CONFIG_ARCH_TEGRA_114_SOC) += tegra114-mc.o > > +obj-$(CONFIG_ARCH_TEGRA_124_SOC) += tegra124-mc.o > > +obj-$(CONFIG_ARCH_TEGRA_132_SOC) += tegra124-mc.o > > You'll need a Kconfig and not just a makefile -- there are definitely > dependencies on this driver (IOMMU in particular). This is handled within the tegra-mc driver by only setting up the IOMMU when TEGRA_IOMMU_SMMU is enabled. That config option remains in place. > Also, the problem of having a global enable bit that is only under > control of TrustZone FW is a big problem -- if the bit is not set, the > driver will not work (and the machine will crash). > > I think you'll need to come up with a way to detect that in the > driver. I don't have a good idea of how it can be done though. I don't think I ever got back to you on this. We discussed this internally and it seems like there's no way to detect this properly, so the best suggestion so far was to make it a requirement on the secure firmware to enable IOMMU or not. Since there's no way for the kernel to detect whether IOMMU was enabled or not, I think the firmware would equally have to adjust the SMMU's device tree node's status property appropriately. Stephen, does that accurately reflect what we had discussed? Thierry
On Wed, Oct 15, 2014 at 03:05:36PM -0700, Olof Johansson wrote: > Hi, > > On Mon, Oct 13, 2014 at 3:33 AM, Thierry Reding > <thierry.reding@gmail.com> wrote: > [...] > > diff --git a/drivers/memory/tegra/tegra-mc.c b/drivers/memory/tegra/tegra-mc.c > > new file mode 100644 > > index 000000000000..0f0c8be096d0 > > --- /dev/null > > +++ b/drivers/memory/tegra/tegra-mc.c > > @@ -0,0 +1,1061 @@ > > +/* > > + * Copyright (C) 2014 NVIDIA CORPORATION. All rights reserved. > > + * > > + * This program is free software; you can redistribute it and/or modify > > + * it under the terms of the GNU General Public License version 2 as > > + * published by the Free Software Foundation. > > + */ > > + > > +#include <linux/clk.h> > > +#include <linux/interrupt.h> > > +#include <linux/io.h> > > +#include <linux/iommu.h> > > +#include <linux/kernel.h> > > +#include <linux/module.h> > > You need a linux/mm.h in here (on 64-bit). Can you show what build error this fixes? I don't see any build failures (after fixing up the obvious ones you pointed out below). > > diff --git a/drivers/memory/tegra/tegra124-mc.c b/drivers/memory/tegra/tegra124-mc.c > > new file mode 100644 > > index 000000000000..db31c96fc288 > > --- /dev/null > > +++ b/drivers/memory/tegra/tegra124-mc.c > > [...] > > > > @@ -0,0 +1,1028 @@ > > +/* > > + * Copyright (C) 2014 NVIDIA CORPORATION. All rights reserved. > > + * > > + * This program is free software; you can redistribute it and/or modify > > + * it under the terms of the GNU General Public License version 2 as > > + * published by the Free Software Foundation. > > + */ > > + > > +#include <linux/of.h> > > +#include <linux/mm.h> > > + > > +#include <asm/cacheflush.h> > > + > > +#include <dt-bindings/memory/tegra124-mc.h> > > + > > +#include "tegra-mc.h" > > + > > +static const struct tegra_mc_client tegra124_mc_clients[] = { > > + { > > + .id = 0x00, > > + .name = "ptcr", > > + .swgroup = TEGRA_SWGROUP_PTC, > > + }, { > > + .id = 0x01, > > + .name = "display0a", > > + .swgroup = TEGRA_SWGROUP_DC, > > + .smmu = { > > + .reg = 0x228, > > + .bit = 1, > > + }, > > + .latency = { > > + .reg = 0x2e8, > > + .shift = 0, > > + .mask = 0xff, > > + .def = 0xc2, > > + }, > > + }, { > > These are very verbose tables. Having a macro for the initializers > could help density a lot. I've tried to use macros here, but I find that it hurts readability: ... }, { TEGRA_MC_CLIENT(0x01, "display0a", TEGRA_SWGROUP_DC), TEGRA_MC_SMMU_ENABLE(0x228, 1), TEGRA_MC_LATENCY(0x2e8, 0, 0xff, 0xc2), }, { ... The original is more readable because it immediately gives you the context, whereas with the macros you need to look up what the parameters refer to. > > +#ifdef CONFIG_ARCH_TEGRA_132_SOC > > +static void tegra132_flush_dcache(struct page *page, unsigned long offset, > > + size_t size) > > +{ > > + void *virt = page_address(page) + offset; > > + > > + __flush_dcache_area(virt, size); > > +} > > + > > +static const struct tegra_smmu_ops tegra132_smmu_ops = { > > + .flush_dcache = tegra132_flush_dcache, > > +}; > > + > > +static const struct tegra_smmu_soc tegra132_smmu_soc = { > > + .groups = tegra124_smmu_groups, > > + .num_groups = ARRAY_SIZE(tegra124_smmu_groups), > > + .clients = tegra124_mc_clients, > > + .num_clients = ARRAY_SIZE(tegra124_mc_clients), > > + .swgroups = tegra124_swgroups, > > + .num_swgroups = ARRAY_SIZE(tegra124_swgroups), > > + .supports_round_robin_arbitration = true, > > + .supports_request_limit = true, > > + .num_address_bits = 34, > > + .num_asids = 128, > > + .ops = &tegra132_smmu_ops, > > +}; > > + > > +const struct tegra_mc_soc tegra132_mc_soc = { > > + .clients = tegra124_mc_clients, > > + .num_clients = ARRAY_SIZE(tegra124_mc_clients), > > + .atom_size = 32, > > + .smmu = &tegra132_smmu_soc, > > +}; > > +#endif /* CONFIG_ARCH_TEGRA_132_SOC */ > > > This won't compile -- several of the tegra132_smmu_soc members are no > longer valid. In particular: > > groups > num_groups Fixed. > supports_round_robin_arbitration > supports_request_limit In the version that I have these are still part of the tegra_smmu_soc structure. I've been thinking of extracting the Tegra132 changes into a separate patch that can be applied once we have basic Tegra132 SoC support. It feels wrong to merge Tegra132 SMMU support if there's no support in arch/arm64 for the SoC yet. Though if nobody else thinks that's a problem that's fine with me too. Thierry
On Thu, Oct 30, 2014 at 04:08:41PM +0100, Thierry Reding wrote: > On Wed, Oct 15, 2014 at 03:09:30PM -0700, Olof Johansson wrote: > > Hi, > > > > Oh, a few more comments: > > > > On Mon, Oct 13, 2014 at 3:33 AM, Thierry Reding > > <thierry.reding@gmail.com> wrote: > > > > > diff --git a/drivers/memory/Makefile b/drivers/memory/Makefile > > > index c32d31981be3..1c932e7e7b8d 100644 > > > --- a/drivers/memory/Makefile > > > +++ b/drivers/memory/Makefile > > > @@ -12,4 +12,5 @@ obj-$(CONFIG_FSL_CORENET_CF) += fsl-corenet-cf.o > > > obj-$(CONFIG_FSL_IFC) += fsl_ifc.o > > > obj-$(CONFIG_MVEBU_DEVBUS) += mvebu-devbus.o > > > obj-$(CONFIG_TEGRA20_MC) += tegra20-mc.o > > > -obj-$(CONFIG_TEGRA30_MC) += tegra30-mc.o > > > + > > > +obj-$(CONFIG_ARCH_TEGRA) += tegra/ > > > diff --git a/drivers/memory/tegra/Makefile b/drivers/memory/tegra/Makefile > > > new file mode 100644 > > > index 000000000000..51b9e8fcde1b > > > --- /dev/null > > > +++ b/drivers/memory/tegra/Makefile > > > @@ -0,0 +1,5 @@ > > > +obj-y = tegra-mc.o > > > +obj-$(CONFIG_ARCH_TEGRA_3x_SOC) += tegra30-mc.o > > > +obj-$(CONFIG_ARCH_TEGRA_114_SOC) += tegra114-mc.o > > > +obj-$(CONFIG_ARCH_TEGRA_124_SOC) += tegra124-mc.o > > > +obj-$(CONFIG_ARCH_TEGRA_132_SOC) += tegra124-mc.o > > > > You'll need a Kconfig and not just a makefile -- there are definitely > > dependencies on this driver (IOMMU in particular). > > This is handled within the tegra-mc driver by only setting up the IOMMU > when TEGRA_IOMMU_SMMU is enabled. That config option remains in place. > > > Also, the problem of having a global enable bit that is only under > > control of TrustZone FW is a big problem -- if the bit is not set, the > > driver will not work (and the machine will crash). > > > > I think you'll need to come up with a way to detect that in the > > driver. I don't have a good idea of how it can be done though. > > I don't think I ever got back to you on this. We discussed this > internally and it seems like there's no way to detect this properly, so > the best suggestion so far was to make it a requirement on the secure > firmware to enable IOMMU or not. Since there's no way for the kernel to > detect whether IOMMU was enabled or not, I think the firmware would > equally have to adjust the SMMU's device tree node's status property > appropriately. The other option would be for the firmware not to touch the SMMU device tree node and the kernel simply assuming that if it's running in non- secure mode then there must be secure firmware and it has enabled the SMMU. Enabling the SMMU would become part of the contract between firmware and kernel, much like locking the VPR is required to get the GPU to work. Those are really the only two choices we have. Thierry
On Fri, Oct 31, 2014 at 10:27 PM, Thierry Reding <thierry.reding@gmail.com> wrote: > On Thu, Oct 30, 2014 at 04:08:41PM +0100, Thierry Reding wrote: >> On Wed, Oct 15, 2014 at 03:09:30PM -0700, Olof Johansson wrote: >> > Hi, >> > >> > Oh, a few more comments: >> > >> > On Mon, Oct 13, 2014 at 3:33 AM, Thierry Reding >> > <thierry.reding@gmail.com> wrote: >> > >> > > diff --git a/drivers/memory/Makefile b/drivers/memory/Makefile >> > > index c32d31981be3..1c932e7e7b8d 100644 >> > > --- a/drivers/memory/Makefile >> > > +++ b/drivers/memory/Makefile >> > > @@ -12,4 +12,5 @@ obj-$(CONFIG_FSL_CORENET_CF) += fsl-corenet-cf.o >> > > obj-$(CONFIG_FSL_IFC) += fsl_ifc.o >> > > obj-$(CONFIG_MVEBU_DEVBUS) += mvebu-devbus.o >> > > obj-$(CONFIG_TEGRA20_MC) += tegra20-mc.o >> > > -obj-$(CONFIG_TEGRA30_MC) += tegra30-mc.o >> > > + >> > > +obj-$(CONFIG_ARCH_TEGRA) += tegra/ >> > > diff --git a/drivers/memory/tegra/Makefile b/drivers/memory/tegra/Makefile >> > > new file mode 100644 >> > > index 000000000000..51b9e8fcde1b >> > > --- /dev/null >> > > +++ b/drivers/memory/tegra/Makefile >> > > @@ -0,0 +1,5 @@ >> > > +obj-y = tegra-mc.o >> > > +obj-$(CONFIG_ARCH_TEGRA_3x_SOC) += tegra30-mc.o >> > > +obj-$(CONFIG_ARCH_TEGRA_114_SOC) += tegra114-mc.o >> > > +obj-$(CONFIG_ARCH_TEGRA_124_SOC) += tegra124-mc.o >> > > +obj-$(CONFIG_ARCH_TEGRA_132_SOC) += tegra124-mc.o >> > >> > You'll need a Kconfig and not just a makefile -- there are definitely >> > dependencies on this driver (IOMMU in particular). >> >> This is handled within the tegra-mc driver by only setting up the IOMMU >> when TEGRA_IOMMU_SMMU is enabled. That config option remains in place. >> >> > Also, the problem of having a global enable bit that is only under >> > control of TrustZone FW is a big problem -- if the bit is not set, the >> > driver will not work (and the machine will crash). >> > >> > I think you'll need to come up with a way to detect that in the >> > driver. I don't have a good idea of how it can be done though. >> >> I don't think I ever got back to you on this. We discussed this >> internally and it seems like there's no way to detect this properly, so >> the best suggestion so far was to make it a requirement on the secure >> firmware to enable IOMMU or not. Since there's no way for the kernel to >> detect whether IOMMU was enabled or not, I think the firmware would >> equally have to adjust the SMMU's device tree node's status property >> appropriately. > > The other option would be for the firmware not to touch the SMMU device > tree node and the kernel simply assuming that if it's running in non- > secure mode then there must be secure firmware and it has enabled the > SMMU. Enabling the SMMU would become part of the contract between > firmware and kernel, much like locking the VPR is required to get the > GPU to work. > > Those are really the only two choices we have. We got the exact same problem with GPU and VPR registers, and it seems like the approach we will be taking here is to have the firmware/bootloader do whatever is needed to get the GPU working and enable the DT node once it did. IOW, the kernel will never touch protected registers, and will not freeze if the hardware is not properly set up. It would be nice for consistency if the same approach can be taken with the IOMMU. OTOH we will need to make sure that all these initialization contracts are clearly documented somewhere. Maybe a comment in the DTS to explain what is expected from the firmware to enable such nodes would be a good idea, too.
On Sat, Nov 01, 2014 at 02:38:26PM +0900, Alexandre Courbot wrote: > On Fri, Oct 31, 2014 at 10:27 PM, Thierry Reding > <thierry.reding@gmail.com> wrote: > > On Thu, Oct 30, 2014 at 04:08:41PM +0100, Thierry Reding wrote: > >> On Wed, Oct 15, 2014 at 03:09:30PM -0700, Olof Johansson wrote: > >> > Hi, > >> > > >> > Oh, a few more comments: > >> > > >> > On Mon, Oct 13, 2014 at 3:33 AM, Thierry Reding > >> > <thierry.reding@gmail.com> wrote: > >> > > >> > > diff --git a/drivers/memory/Makefile b/drivers/memory/Makefile > >> > > index c32d31981be3..1c932e7e7b8d 100644 > >> > > --- a/drivers/memory/Makefile > >> > > +++ b/drivers/memory/Makefile > >> > > @@ -12,4 +12,5 @@ obj-$(CONFIG_FSL_CORENET_CF) += fsl-corenet-cf.o > >> > > obj-$(CONFIG_FSL_IFC) += fsl_ifc.o > >> > > obj-$(CONFIG_MVEBU_DEVBUS) += mvebu-devbus.o > >> > > obj-$(CONFIG_TEGRA20_MC) += tegra20-mc.o > >> > > -obj-$(CONFIG_TEGRA30_MC) += tegra30-mc.o > >> > > + > >> > > +obj-$(CONFIG_ARCH_TEGRA) += tegra/ > >> > > diff --git a/drivers/memory/tegra/Makefile b/drivers/memory/tegra/Makefile > >> > > new file mode 100644 > >> > > index 000000000000..51b9e8fcde1b > >> > > --- /dev/null > >> > > +++ b/drivers/memory/tegra/Makefile > >> > > @@ -0,0 +1,5 @@ > >> > > +obj-y = tegra-mc.o > >> > > +obj-$(CONFIG_ARCH_TEGRA_3x_SOC) += tegra30-mc.o > >> > > +obj-$(CONFIG_ARCH_TEGRA_114_SOC) += tegra114-mc.o > >> > > +obj-$(CONFIG_ARCH_TEGRA_124_SOC) += tegra124-mc.o > >> > > +obj-$(CONFIG_ARCH_TEGRA_132_SOC) += tegra124-mc.o > >> > > >> > You'll need a Kconfig and not just a makefile -- there are definitely > >> > dependencies on this driver (IOMMU in particular). > >> > >> This is handled within the tegra-mc driver by only setting up the IOMMU > >> when TEGRA_IOMMU_SMMU is enabled. That config option remains in place. > >> > >> > Also, the problem of having a global enable bit that is only under > >> > control of TrustZone FW is a big problem -- if the bit is not set, the > >> > driver will not work (and the machine will crash). > >> > > >> > I think you'll need to come up with a way to detect that in the > >> > driver. I don't have a good idea of how it can be done though. > >> > >> I don't think I ever got back to you on this. We discussed this > >> internally and it seems like there's no way to detect this properly, so > >> the best suggestion so far was to make it a requirement on the secure > >> firmware to enable IOMMU or not. Since there's no way for the kernel to > >> detect whether IOMMU was enabled or not, I think the firmware would > >> equally have to adjust the SMMU's device tree node's status property > >> appropriately. > > > > The other option would be for the firmware not to touch the SMMU device > > tree node and the kernel simply assuming that if it's running in non- > > secure mode then there must be secure firmware and it has enabled the > > SMMU. Enabling the SMMU would become part of the contract between > > firmware and kernel, much like locking the VPR is required to get the > > GPU to work. > > > > Those are really the only two choices we have. > > We got the exact same problem with GPU and VPR registers, and it seems > like the approach we will be taking here is to have the > firmware/bootloader do whatever is needed to get the GPU working and > enable the DT node once it did. IOW, the kernel will never touch > protected registers, and will not freeze if the hardware is not > properly set up. I think the situation is slightly different. For the SMMU we still have the option to enable translations when running in secure mode with code that's pretty trivial to have in the IOMMU driver (it's just a single bit that gets written to a register). And when the IOMMU driver does that everything will work just fine. So I'm thinking that a workable alternative to what we've done for VPR would be to just always enable translations in the SMMU driver (that operation will simply be discarded in non-secure mode) and assume that it'll work. So the contract would be that running in secure mode the kernel sets everything up and when running in non-secure mode the kernel will assume that firmware set everything up already. Requiring firmware to change the device node's status to "okay" seems rather restrictive since it would have to do that even if it boots the kernel in secure mode. > It would be nice for consistency if the same approach can be taken > with the IOMMU. OTOH we will need to make sure that all these > initialization contracts are clearly documented somewhere. Maybe a > comment in the DTS to explain what is expected from the firmware to > enable such nodes would be a good idea, too. The device tree binding seems like a good place to document this. Thierry
On Mon, Nov 3, 2014 at 5:22 PM, Thierry Reding <thierry.reding@gmail.com> wrote: > On Sat, Nov 01, 2014 at 02:38:26PM +0900, Alexandre Courbot wrote: >> On Fri, Oct 31, 2014 at 10:27 PM, Thierry Reding >> <thierry.reding@gmail.com> wrote: >> > On Thu, Oct 30, 2014 at 04:08:41PM +0100, Thierry Reding wrote: >> >> On Wed, Oct 15, 2014 at 03:09:30PM -0700, Olof Johansson wrote: >> >> > Hi, >> >> > >> >> > Oh, a few more comments: >> >> > >> >> > On Mon, Oct 13, 2014 at 3:33 AM, Thierry Reding >> >> > <thierry.reding@gmail.com> wrote: >> >> > >> >> > > diff --git a/drivers/memory/Makefile b/drivers/memory/Makefile >> >> > > index c32d31981be3..1c932e7e7b8d 100644 >> >> > > --- a/drivers/memory/Makefile >> >> > > +++ b/drivers/memory/Makefile >> >> > > @@ -12,4 +12,5 @@ obj-$(CONFIG_FSL_CORENET_CF) += fsl-corenet-cf.o >> >> > > obj-$(CONFIG_FSL_IFC) += fsl_ifc.o >> >> > > obj-$(CONFIG_MVEBU_DEVBUS) += mvebu-devbus.o >> >> > > obj-$(CONFIG_TEGRA20_MC) += tegra20-mc.o >> >> > > -obj-$(CONFIG_TEGRA30_MC) += tegra30-mc.o >> >> > > + >> >> > > +obj-$(CONFIG_ARCH_TEGRA) += tegra/ >> >> > > diff --git a/drivers/memory/tegra/Makefile b/drivers/memory/tegra/Makefile >> >> > > new file mode 100644 >> >> > > index 000000000000..51b9e8fcde1b >> >> > > --- /dev/null >> >> > > +++ b/drivers/memory/tegra/Makefile >> >> > > @@ -0,0 +1,5 @@ >> >> > > +obj-y = tegra-mc.o >> >> > > +obj-$(CONFIG_ARCH_TEGRA_3x_SOC) += tegra30-mc.o >> >> > > +obj-$(CONFIG_ARCH_TEGRA_114_SOC) += tegra114-mc.o >> >> > > +obj-$(CONFIG_ARCH_TEGRA_124_SOC) += tegra124-mc.o >> >> > > +obj-$(CONFIG_ARCH_TEGRA_132_SOC) += tegra124-mc.o >> >> > >> >> > You'll need a Kconfig and not just a makefile -- there are definitely >> >> > dependencies on this driver (IOMMU in particular). >> >> >> >> This is handled within the tegra-mc driver by only setting up the IOMMU >> >> when TEGRA_IOMMU_SMMU is enabled. That config option remains in place. >> >> >> >> > Also, the problem of having a global enable bit that is only under >> >> > control of TrustZone FW is a big problem -- if the bit is not set, the >> >> > driver will not work (and the machine will crash). >> >> > >> >> > I think you'll need to come up with a way to detect that in the >> >> > driver. I don't have a good idea of how it can be done though. >> >> >> >> I don't think I ever got back to you on this. We discussed this >> >> internally and it seems like there's no way to detect this properly, so >> >> the best suggestion so far was to make it a requirement on the secure >> >> firmware to enable IOMMU or not. Since there's no way for the kernel to >> >> detect whether IOMMU was enabled or not, I think the firmware would >> >> equally have to adjust the SMMU's device tree node's status property >> >> appropriately. >> > >> > The other option would be for the firmware not to touch the SMMU device >> > tree node and the kernel simply assuming that if it's running in non- >> > secure mode then there must be secure firmware and it has enabled the >> > SMMU. Enabling the SMMU would become part of the contract between >> > firmware and kernel, much like locking the VPR is required to get the >> > GPU to work. >> > >> > Those are really the only two choices we have. >> >> We got the exact same problem with GPU and VPR registers, and it seems >> like the approach we will be taking here is to have the >> firmware/bootloader do whatever is needed to get the GPU working and >> enable the DT node once it did. IOW, the kernel will never touch >> protected registers, and will not freeze if the hardware is not >> properly set up. > > I think the situation is slightly different. For the SMMU we still have > the option to enable translations when running in secure mode with code > that's pretty trivial to have in the IOMMU driver (it's just a single > bit that gets written to a register). And when the IOMMU driver does > that everything will work just fine. > > So I'm thinking that a workable alternative to what we've done for VPR > would be to just always enable translations in the SMMU driver (that > operation will simply be discarded in non-secure mode) and assume that > it'll work. So the contract would be that running in secure mode the > kernel sets everything up and when running in non-secure mode the kernel > will assume that firmware set everything up already. > > Requiring firmware to change the device node's status to "okay" seems > rather restrictive since it would have to do that even if it boots the > kernel in secure mode. Sounds good to me, as long as the kernel can always run using the same code in both modes. > >> It would be nice for consistency if the same approach can be taken >> with the IOMMU. OTOH we will need to make sure that all these >> initialization contracts are clearly documented somewhere. Maybe a >> comment in the DTS to explain what is expected from the firmware to >> enable such nodes would be a good idea, too. > > The device tree binding seems like a good place to document this. Indeed.
diff --git a/drivers/clk/tegra/clk-divider.c b/drivers/clk/tegra/clk-divider.c index 290f9c1a3749..84e1b3c1fb2a 100644 --- a/drivers/clk/tegra/clk-divider.c +++ b/drivers/clk/tegra/clk-divider.c @@ -185,3 +185,16 @@ struct clk *tegra_clk_register_divider(const char *name, return clk; } + +static const struct clk_div_table mc_div_table[] = { + { .val = 0, .div = 2 }, + { .val = 1, .div = 1 }, + { .val = 0, .div = 0 }, +}; + +struct clk *tegra_clk_register_mc(const char *name, const char *parent_name, + void __iomem *reg, spinlock_t *lock) +{ + return clk_register_divider_table(NULL, "mc", "emc_mux", 0, reg, + 16, 1, 0, mc_div_table, lock); +} diff --git a/drivers/clk/tegra/clk-tegra114.c b/drivers/clk/tegra/clk-tegra114.c index f760f31d05c4..0b03d2cf7264 100644 --- a/drivers/clk/tegra/clk-tegra114.c +++ b/drivers/clk/tegra/clk-tegra114.c @@ -173,6 +173,7 @@ static DEFINE_SPINLOCK(pll_d_lock); static DEFINE_SPINLOCK(pll_d2_lock); static DEFINE_SPINLOCK(pll_u_lock); static DEFINE_SPINLOCK(pll_re_lock); +static DEFINE_SPINLOCK(emc_lock); static struct div_nmp pllxc_nmp = { .divm_shift = 0, @@ -1228,7 +1229,11 @@ static __init void tegra114_periph_clk_init(void __iomem *clk_base, ARRAY_SIZE(mux_pllmcp_clkm), CLK_SET_RATE_NO_REPARENT, clk_base + CLK_SOURCE_EMC, - 29, 3, 0, NULL); + 29, 3, 0, &emc_lock); + + clk = tegra_clk_register_mc("mc", "emc_mux", clk_base + CLK_SOURCE_EMC, + &emc_lock); + clks[TEGRA114_CLK_MC] = clk; for (i = 0; i < ARRAY_SIZE(tegra_periph_clk_list); i++) { data = &tegra_periph_clk_list[i]; diff --git a/drivers/clk/tegra/clk-tegra124.c b/drivers/clk/tegra/clk-tegra124.c index e3a85842ce0c..f5f9baca7bb6 100644 --- a/drivers/clk/tegra/clk-tegra124.c +++ b/drivers/clk/tegra/clk-tegra124.c @@ -132,6 +132,7 @@ static DEFINE_SPINLOCK(pll_d2_lock); static DEFINE_SPINLOCK(pll_e_lock); static DEFINE_SPINLOCK(pll_re_lock); static DEFINE_SPINLOCK(pll_u_lock); +static DEFINE_SPINLOCK(emc_lock); /* possible OSC frequencies in Hz */ static unsigned long tegra124_input_freq[] = { @@ -1127,7 +1128,11 @@ static __init void tegra124_periph_clk_init(void __iomem *clk_base, clk = clk_register_mux(NULL, "emc_mux", mux_pllmcp_clkm, ARRAY_SIZE(mux_pllmcp_clkm), 0, clk_base + CLK_SOURCE_EMC, - 29, 3, 0, NULL); + 29, 3, 0, &emc_lock); + + clk = tegra_clk_register_mc("mc", "emc_mux", clk_base + CLK_SOURCE_EMC, + &emc_lock); + clks[TEGRA124_CLK_MC] = clk; /* cml0 */ clk = clk_register_gate(NULL, "cml0", "pll_e", 0, clk_base + PLLE_AUX, diff --git a/drivers/clk/tegra/clk-tegra20.c b/drivers/clk/tegra/clk-tegra20.c index dace2b1b5ae6..41272dcc9e22 100644 --- a/drivers/clk/tegra/clk-tegra20.c +++ b/drivers/clk/tegra/clk-tegra20.c @@ -140,6 +140,8 @@ static struct cpu_clk_suspend_context { static void __iomem *clk_base; static void __iomem *pmc_base; +static DEFINE_SPINLOCK(emc_lock); + #define TEGRA_INIT_DATA_MUX(_name, _parents, _offset, \ _clk_num, _gate_flags, _clk_id) \ TEGRA_INIT_DATA(_name, NULL, NULL, _parents, _offset, \ @@ -819,11 +821,15 @@ static void __init tegra20_periph_clk_init(void) ARRAY_SIZE(mux_pllmcp_clkm), CLK_SET_RATE_NO_REPARENT, clk_base + CLK_SOURCE_EMC, - 30, 2, 0, NULL); + 30, 2, 0, &emc_lock); clk = tegra_clk_register_periph_gate("emc", "emc_mux", 0, clk_base, 0, 57, periph_clk_enb_refcnt); clks[TEGRA20_CLK_EMC] = clk; + clk = tegra_clk_register_mc("mc", "emc_mux", clk_base + CLK_SOURCE_EMC, + &emc_lock); + clks[TEGRA20_CLK_MC] = clk; + /* dsi */ clk = tegra_clk_register_periph_gate("dsi", "pll_d", 0, clk_base, 0, 48, periph_clk_enb_refcnt); diff --git a/drivers/clk/tegra/clk-tegra30.c b/drivers/clk/tegra/clk-tegra30.c index 5bbacd01094f..4b9d8bd3d0bf 100644 --- a/drivers/clk/tegra/clk-tegra30.c +++ b/drivers/clk/tegra/clk-tegra30.c @@ -177,6 +177,7 @@ static unsigned long input_freq; static DEFINE_SPINLOCK(cml_lock); static DEFINE_SPINLOCK(pll_d_lock); +static DEFINE_SPINLOCK(emc_lock); #define TEGRA_INIT_DATA_MUX(_name, _parents, _offset, \ _clk_num, _gate_flags, _clk_id) \ @@ -1157,11 +1158,15 @@ static void __init tegra30_periph_clk_init(void) ARRAY_SIZE(mux_pllmcp_clkm), CLK_SET_RATE_NO_REPARENT, clk_base + CLK_SOURCE_EMC, - 30, 2, 0, NULL); + 30, 2, 0, &emc_lock); clk = tegra_clk_register_periph_gate("emc", "emc_mux", 0, clk_base, 0, 57, periph_clk_enb_refcnt); clks[TEGRA30_CLK_EMC] = clk; + clk = tegra_clk_register_mc("mc", "emc_mux", clk_base + CLK_SOURCE_EMC, + &emc_lock); + clks[TEGRA30_CLK_MC] = clk; + /* cml0 */ clk = clk_register_gate(NULL, "cml0", "pll_e", 0, clk_base + PLLE_AUX, 0, 0, &cml_lock); diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h index 16ec8d6bb87f..4e458aa8d45c 100644 --- a/drivers/clk/tegra/clk.h +++ b/drivers/clk/tegra/clk.h @@ -86,6 +86,8 @@ struct clk *tegra_clk_register_divider(const char *name, const char *parent_name, void __iomem *reg, unsigned long flags, u8 clk_divider_flags, u8 shift, u8 width, u8 frac_width, spinlock_t *lock); +struct clk *tegra_clk_register_mc(const char *name, const char *parent_name, + void __iomem *reg, spinlock_t *lock); /* * Tegra PLL: diff --git a/include/dt-bindings/clock/tegra114-car.h b/include/dt-bindings/clock/tegra114-car.h index fc12621fb432..534c03f8ad72 100644 --- a/include/dt-bindings/clock/tegra114-car.h +++ b/include/dt-bindings/clock/tegra114-car.h @@ -49,7 +49,7 @@ #define TEGRA114_CLK_I2S0 30 /* 31 */ -/* 32 */ +#define TEGRA114_CLK_MC 32 /* 33 */ #define TEGRA114_CLK_APBDMA 34 /* 35 */ diff --git a/include/dt-bindings/clock/tegra124-car.h b/include/dt-bindings/clock/tegra124-car.h index 6bac637fd635..af9bc9a3ddbc 100644 --- a/include/dt-bindings/clock/tegra124-car.h +++ b/include/dt-bindings/clock/tegra124-car.h @@ -48,7 +48,7 @@ #define TEGRA124_CLK_I2S0 30 /* 31 */ -/* 32 */ +#define TEGRA124_CLK_MC 32 /* 33 */ #define TEGRA124_CLK_APBDMA 34 /* 35 */ diff --git a/include/dt-bindings/clock/tegra20-car.h b/include/dt-bindings/clock/tegra20-car.h index 9406207cfac8..04500b243a4d 100644 --- a/include/dt-bindings/clock/tegra20-car.h +++ b/include/dt-bindings/clock/tegra20-car.h @@ -49,7 +49,7 @@ /* 30 */ #define TEGRA20_CLK_CACHE2 31 -#define TEGRA20_CLK_MEM 32 +#define TEGRA20_CLK_MC 32 #define TEGRA20_CLK_AHBDMA 33 #define TEGRA20_CLK_APBDMA 34 /* 35 */