From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754826Ab2CAO4V (ORCPT ); Thu, 1 Mar 2012 09:56:21 -0500 Received: from mailout1.w1.samsung.com ([210.118.77.11]:17586 "EHLO mailout1.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753475Ab2CAO4T convert rfc822-to-8bit (ORCPT ); Thu, 1 Mar 2012 09:56:19 -0500 Date: Thu, 01 Mar 2012 15:57:18 +0100 From: Marek Szyprowski Subject: RE: [PATCH v9 1/2] ARM: EXYNOS: Change System MMU platform device definitions In-reply-to: To: "'KyongHo Cho'" , "'Kyungmin Park'" Cc: linux-arm-kernel@lists.infradead.org, linux-samsung-soc@vger.kernel.org, iommu@lists.linux-foundation.org, linux-kernel@vger.kernel.org, "'Kukjin Kim'" , "'Joerg Roedel'" , "'Subash Patel'" , "'Younglak Kim'" , "'Sanghyun Lee'" Message-id: <011401ccf7bb$98d51c30$ca7f5490$%szyprowski@samsung.com> Organization: SPRC MIME-version: 1.0 X-Mailer: Microsoft Office Outlook 12.0 Content-type: text/plain; charset=iso-8859-2 Content-language: pl Content-transfer-encoding: 8BIT Thread-index: Acz2eLf5RirVyCBfQGKTmnybhSMZDQBQo70g References: <001901ccf5e3$6a363320$3ea29960$%cho@samsung.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hello, On Wednesday, February 29, 2012 1:26 AM KyongHo Cho wrote: > On Tue, Feb 28, 2012 at 4:20 PM, Kyungmin Park > wrote: > > Hi, > > > > On 2/28/12, KyongHo Cho 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 > >> --- > >>  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