| 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 linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-id: <011401ccf7bb$98d51c30$ca7f5490$%szyprowski@samsung.com>
Date: Thu, 01 Mar 2012 15:57:18 +0100
From: Marek Szyprowski <m.szyprowski@...sung.com>
To: 'KyongHo Cho' <pullip.cho@...sung.com>,
'Kyungmin Park' <kyungmin.park@...sung.com>
Cc: linux-arm-kernel@...ts.infradead.org,
linux-samsung-soc@...r.kernel.org,
iommu@...ts.linux-foundation.org, linux-kernel@...r.kernel.org,
'Kukjin Kim' <kgene.kim@...sung.com>,
'Joerg Roedel' <joro@...tes.org>,
'Subash Patel' <subash.ramaswamy@...aro.org>,
'Younglak Kim' <younglak1004.kim@...sung.com>,
'Sanghyun Lee' <sanghyun75.lee@...sung.com>
Subject: RE: [PATCH v9 1/2] ARM: EXYNOS: Change System MMU platform device
definitions
Hello,
On Wednesday, February 29, 2012 1:26 AM KyongHo Cho wrote:
> On Tue, Feb 28, 2012 at 4:20 PM, Kyungmin Park
> <kyungmin.park@...sung.com> wrote:
> > Hi,
> >
> > On 2/28/12, KyongHo Cho <pullip.cho@...sung.com> wrote:
> >> Handling System MMUs with an identifier is not flexible to manage
> >> System MMU platform devices because of the following reasons:
> >> 1. A device driver which needs to handle System MMU must know the ID.
> >> 2. A System MMU may not present in some implementations of Exynos family.
> >> 3. Handling System MMU with IOMMU API does not require an ID.
> >>
> >> This patch is the result of removing ID of System MMUs.
> >> Instead, a device driver that needs to handle its System MMU must
> >> use IOMMU API while its descriptor of platform device is given.
> >>
> >> This patch also includes the following enhancements:
> >> - A System MMU device becomes a child if its power domain device.
> >> - clkdev
> >>
> >> Signed-off-by: KyongHo Cho <pullip.cho@...sung.com>
> >> ---
> >> arch/arm/mach-exynos/Kconfig | 11 +-
> >> arch/arm/mach-exynos/Makefile | 2 +-
> >> arch/arm/mach-exynos/clock-exynos4.c | 79 ++--
> >> arch/arm/mach-exynos/clock-exynos4.h | 2 +
> >> arch/arm/mach-exynos/clock-exynos4210.c | 11 +
> >> arch/arm/mach-exynos/clock-exynos4212.c | 28 ++-
> >> arch/arm/mach-exynos/clock-exynos5.c | 90 +++++
> >> arch/arm/mach-exynos/dev-sysmmu.c | 451
> >> ++++++++++++-----------
> >> arch/arm/mach-exynos/include/mach/irqs.h | 179 +++++-----
> >> arch/arm/mach-exynos/include/mach/map.h | 38 ++
> >> arch/arm/mach-exynos/include/mach/regs-clock.h | 5 +
> >> arch/arm/mach-exynos/include/mach/regs-sysmmu.h | 28 --
> >> arch/arm/mach-exynos/include/mach/sysmmu.h | 84 +++--
> >> arch/arm/mach-exynos/mach-armlex4210.c | 1 -
> >> arch/arm/mach-exynos/mach-smdkv310.c | 1 -
> >> arch/arm/plat-s5p/sysmmu.c | 13 +-
> >> arch/arm/plat-samsung/include/plat/devs.h | 1 -
> >> 17 files changed, 618 insertions(+), 406 deletions(-)
> > It's too big to cover these changes. Can you make it more small changes.
> > 1. Remove existing ones since no user at this time.
> > 2. Add new define and data structures.
> > 3. Support new system mmu features based on generic iommu.
> >
>
> Your suggestion sounds good. Thank you.
>
> >> static int exynos4_clk_hdmiphy_ctrl(struct clk *clk, int enable)
> >> {
> >> return s5p_gatectrl(S5P_HDMI_PHY_CONTROL, clk, enable);
> >> @@ -679,61 +684,55 @@ static struct clk exynos4_init_clocks_off[] = {
> >> .enable = exynos4_clk_ip_peril_ctrl,
> >> .ctrlbit = (1 << 14),
> >> }, {
> >> - .name = "SYSMMU_MDMA",
> >> + .name = SYSMMU_CLOCK_NAME,
> >> + .devname = SYSMMU_CLOCK_DEVNAME(mfc_l, 0),
> >> + .enable = exynos4_clk_ip_mfc_ctrl,
> >> + .ctrlbit = (1 << 1),
> >> + }, {
> >> + .name = SYSMMU_CLOCK_NAME,
> >> + .devname = SYSMMU_CLOCK_DEVNAME(mfc_r, 1),
> >> + .enable = exynos4_clk_ip_mfc_ctrl,
> >> + .ctrlbit = (1 << 2),
> >> + }, {
> >> + .name = SYSMMU_CLOCK_NAME,
> >> + .devname = SYSMMU_CLOCK_DEVNAME(tv, 2),
> >> + .enable = exynos4_clk_ip_tv_ctrl,
> >> + .ctrlbit = (1 << 4),
> >> + }, {
> >> + .name = SYSMMU_CLOCK_NAME,
> >> + .devname = SYSMMU_CLOCK_DEVNAME(jpeg, 3),
> >> + .enable = exynos4_clk_ip_cam_ctrl,
> >> + .ctrlbit = (1 << 11),
> >> + }, {
> >> + .name = SYSMMU_CLOCK_NAME,
> >> + .devname = SYSMMU_CLOCK_DEVNAME(rot, 4),
> >> .enable = exynos4_clk_ip_image_ctrl,
> >> - .ctrlbit = (1 << 5),
> >> + .ctrlbit = (1 << 4),
> >> }, {
> >> - .name = "SYSMMU_FIMC0",
> >> + .name = SYSMMU_CLOCK_NAME,
> >> + .devname = SYSMMU_CLOCK_DEVNAME(gsc0, 5),
> > It's exynos4 series clock and don't have gsc name. I don't know LSI
> > decides to use gsc instead of fimc for both exynos4 and exynos5. but
> > there's no name gsc at Spec..
>
> gsc0 is just a name of the platform device of FIMC0 in Exynos4.
> It is also used for GSclaler0 in Exynos5.
> I wanted to reduce waste of platform device definitions that do not exist
> in an application processor.
>
> If it looks confused, I will find another way.
> >> +#define SYSMMU_RESOURCE_NAME(core, ipname) sysmmures_##core##_##ipname
> >> +
> >> +#define SYSMMU_RESOURCE(core, ipname) \
> >> + static struct resource SYSMMU_RESOURCE_NAME(core, ipname)[] __initdata =
> >> +
> >> +#define DEFINE_SYSMMU_RESOURCE(core, mem, irq) \
> >> + DEFINE_RES_MEM_NAMED(core##_PA_SYSMMU_##mem, SZ_4K, #mem), \
> >> + DEFINE_RES_IRQ_NAMED(core##_IRQ_SYSMMU_##irq##_0, #mem)
> >> +
> >> +#define SYSMMU_RESOURCE_DEFINE(core, ipname, mem, irq) \
> >> + SYSMMU_RESOURCE(core, ipname) { \
> >> + DEFINE_SYSMMU_RESOURCE(core, mem, irq) \
> >> + }
> >>
> >> -struct platform_device exynos4_device_sysmmu = {
> >> - .name = "s5p-sysmmu",
> >> - .id = 32,
> >> - .num_resources = ARRAY_SIZE(exynos4_sysmmu_resource),
> >> - .resource = exynos4_sysmmu_resource,
> >> +struct sysmmu_resource_map {
> >> + struct platform_device *pdev;
> >> + struct resource *res;
> >> + u32 rnum;
> >> + struct device *pdd;
> >> + char *clocknames;
> >> };
> >> -EXPORT_SYMBOL(exynos4_device_sysmmu);
> >>
> >> -static struct clk *sysmmu_clk[S5P_SYSMMU_TOTAL_IPNUM];
> >> -void sysmmu_clk_init(struct device *dev, sysmmu_ips ips)
> >> -{
> >> - sysmmu_clk[ips] = clk_get(dev, sysmmu_ips_name[ips]);
> >> - if (IS_ERR(sysmmu_clk[ips]))
> >> - sysmmu_clk[ips] = NULL;
> >> - else
> >> - clk_put(sysmmu_clk[ips]);
> >> +#define SYSMMU_RESOURCE_MAPPING(core, ipname, resname) { \
> >> + .pdev = &SYSMMU_PLATDEV(ipname), \
> >> + .res = SYSMMU_RESOURCE_NAME(EXYNOS##core, resname), \
> >> + .rnum = ARRAY_SIZE(SYSMMU_RESOURCE_NAME(EXYNOS##core, resname)),\
> >> + .clocknames = SYSMMU_CLOCK_NAME, \
> >> }
> >>
> >> -void sysmmu_clk_enable(sysmmu_ips ips)
> >> -{
> >> - if (sysmmu_clk[ips])
> >> - clk_enable(sysmmu_clk[ips]);
> >> +#define SYSMMU_RESOURCE_MAPPING_MC(core, ipname, resname, pdata) { \
> >> + .pdev = &SYSMMU_PLATDEV(ipname), \
> >> + .res = SYSMMU_RESOURCE_NAME(EXYNOS##core, resname), \
> >> + .rnum = ARRAY_SIZE(SYSMMU_RESOURCE_NAME(EXYNOS##core, resname)),\
> >> + .clocknames = SYSMMU_CLOCK_NAME "," SYSMMU_CLOCK_NAME2, \
> >> +}
> >> +
> >> +#ifdef CONFIG_EXYNOS_DEV_PD
> >> +#define SYSMMU_RESOURCE_MAPPING_PD(core, ipname, resname, pd) { \
> >> + .pdev = &SYSMMU_PLATDEV(ipname), \
> >> + .res = &SYSMMU_RESOURCE_NAME(EXYNOS##core, resname), \
> >> + .rnum = ARRAY_SIZE(SYSMMU_RESOURCE_NAME(EXYNOS##core, resname)),\
> >> + .clocknames = SYSMMU_CLOCK_NAME, \
> >> + .pdd = &exynos##core##_device_pd[pd].dev, \
> >> +}
> >> +
> >> +#define SYSMMU_RESOURCE_MAPPING_MCPD(core, ipname, resname, pd, pdata) {\
> >> + .pdev = &SYSMMU_PLATDEV(ipname), \
> >> + .res = &SYSMMU_RESOURCE_NAME(EXYNOS##core, resname), \
> >> + .rnum = ARRAY_SIZE(SYSMMU_RESOURCE_NAME(EXYNOS##core, resname)),\
> >> + .clocknames = SYSMMU_CLOCK_NAME "," SYSMMU_CLOCK_NAME2, \
> >> + .pdd = &exynos##core##_device_pd[pd].dev, \
> >> }
> >> +#else
> >> +#define SYSMMU_RESOURCE_MAPPING_PD(core, ipname, resname, pd) \
> >> + SYSMMU_RESOURCE_MAPPING(core, ipname, resname)
> >> +#define SYSMMU_RESOURCE_MAPPING_MCPD(core, ipname, resname, pd, pdata) \
> >> + SYSMMU_RESOURCE_MAPPING_MC(core, ipname, resname, pdata)
> >> +
> >> +#endif /* CONFIG_EXYNOS_DEV_PD */
> >> +
> >> +#ifdef CONFIG_ARCH_EXYNOS4
> > Below definitions are almost __initdata. So don't need to use ARCH_EXYNOS4.
> > and another reason is that it can detect the duplicated entry if it's
> > used both exynos4 and exynos5.
FIMC devices use s5p-fimc name in the mainline kernel. The driver is already there so
there, so please don't change it. 4 more lines with proper 'fimc' name will not cause
any problems.
Best regards
--
Marek Szyprowski
Samsung Poland R&D Center
--
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