[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAHQjnOO1aADVQWxNt-J3nb0bgJvc2dfk+6qTWKWx7w+-LUVSmw@mail.gmail.com>
Date: Wed, 29 Feb 2012 09:25:54 +0900
From: KyongHo Cho <pullip.cho@...sung.com>
To: 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>,
Marek Szyprowski <m.szyprowski@...sung.com>
Subject: Re: [PATCH v9 1/2] ARM: EXYNOS: Change System MMU platform device definitions
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.
True. I agree.
Thank you.
>> +SYSMMU_RESOURCE_DEFINE(EXYNOS4, fimc0, FIMC0, FIMC0);
>> +SYSMMU_RESOURCE_DEFINE(EXYNOS4, fimc1, FIMC1, FIMC1);
>> +SYSMMU_RESOURCE_DEFINE(EXYNOS4, fimc2, FIMC2, FIMC2);
>> +SYSMMU_RESOURCE_DEFINE(EXYNOS4, fimc3, FIMC3, FIMC3);
>> +SYSMMU_RESOURCE_DEFINE(EXYNOS4, jpeg, JPEG, JPEG);
>> +SYSMMU_RESOURCE_DEFINE(EXYNOS4, 2d, G2D, 2D);
>> +SYSMMU_RESOURCE_DEFINE(EXYNOS4, tv, TV, TV_M0);
>> +SYSMMU_RESOURCE_DEFINE(EXYNOS4, 2d_acp, 2D_ACP, 2D);
>> +SYSMMU_RESOURCE_DEFINE(EXYNOS4, rot, ROTATOR, ROTATOR);
>> +SYSMMU_RESOURCE_DEFINE(EXYNOS4, fimd0, FIMD0, LCD0_M0);
>> +SYSMMU_RESOURCE_DEFINE(EXYNOS4, fimd1, FIMD1, LCD1_M1);
>> +SYSMMU_RESOURCE_DEFINE(EXYNOS4, flite0, FIMC_LITE0, FIMC_LITE0);
>> +SYSMMU_RESOURCE_DEFINE(EXYNOS4, flite1, FIMC_LITE1, FIMC_LITE1);
>> +SYSMMU_RESOURCE_DEFINE(EXYNOS4, mfc_r, MFC_R, MFC_M0);
>> +SYSMMU_RESOURCE_DEFINE(EXYNOS4, mfc_l, MFC_L, MFC_M1);
>> +SYSMMU_RESOURCE(EXYNOS4, isp) {
>> + DEFINE_SYSMMU_RESOURCE(EXYNOS4, FIMC_ISP, FIMC_ISP),
>> + DEFINE_SYSMMU_RESOURCE(EXYNOS4, FIMC_DRC, FIMC_DRC),
>> + DEFINE_SYSMMU_RESOURCE(EXYNOS4, FIMC_FD, FIMC_FD),
>> + DEFINE_SYSMMU_RESOURCE(EXYNOS4, ISPCPU, FIMC_CX),
>> +};
>> +
--
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