lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Sat, 05 Jul 2014 12:01:19 +0530 From: Pankaj Dubey <pankaj.dubey@...sung.com> To: 'Tomasz Figa' <t.figa@...sung.com>, linux-arm-kernel@...ts.infradead.org, linux-samsung-soc@...r.kernel.org, linux-kernel@...r.kernel.org Cc: kgene.kim@...sung.com, linux@....linux.org.uk, vikas.sajjan@...sung.com, joshi@...sung.com, naushad@...sung.com, thomas.ab@...sung.com, chow.kim@...sung.com Subject: RE: [PATCH v5 1/5] ARM: EXYNOS: Add support for mapping PMU base address via DT Hi Tomasz, On Monday, June 30, 2014 Tomasz Figa wrote: > > Hi Pankaj, > > In general the patch seems quite nice, but please see few comments inline. > > On 25.06.2014 16:03, Pankaj Dubey wrote: > > Add support for mapping Samsung Power Management Unit (PMU) base > > address from device tree. > > > > Signed-off-by: Pankaj Dubey <pankaj.dubey@...sung.com> > > --- > > arch/arm/mach-exynos/common.h | 1 + > > arch/arm/mach-exynos/exynos.c | 45 > +++++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 46 insertions(+) > > [snip] > > > +static void exynos_map_pmu(void) > > +{ > > + struct device_node *np = NULL; > > nit: Unnecessary variable initialization. > OK, will correct it. > > + > > + np = of_find_matching_node(NULL, exynos_dt_pmu_match); > > + > > nit: Unnecessary blank line. > OK, will remove this. > > + if (!np) { > > + pr_err("Failed to find PMU node\n"); > > + return; > > Returning here probably doesn't make too much sense, especially when you just panic > if the mapping fails and you remove the static mapping in patch 2/5, so backwards > compatibility isn't provided anyway. > > So something like this might be a better idea: > > np = of_find_matching_node(NULL, exynos_dt_pmu_match); > if (np) > pmu_base_addr = of_iomap(np, 0); > > if (!pmu_base_addr) > panic("failed to find exynos pmu register\n"); > Yes, this will be better. I will modify this part. > > + } > > + > > + pmu_base_addr = of_iomap(np, 0); > > + > > + if (!pmu_base_addr) > > + panic("failed to find exynos pmu register\n"); } > > + > > +static void __init exynos_init_time(void) { > > + /* Nothing to do timer specific. > > + * Since platsmp.c needs pmu base address by the time > > + * DT is not unflatten so we can't use DT APIs before > > + * init_time > > + */ > > + exynos_map_pmu(); > > Would moving this to .init_irq() callback work too? There are more things happening > in .init_time() so it seems more fragile and some platforms (e.g. mach-tegra) do such > platform-specific initialization in > .init_irq() instead. > I need to check this, if it works I will update as suggested. > Best regards, > Tomasz Thanks, Pankaj Dubey -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists