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
 
Hash Suite for Android: free password hash cracker in your pocket
[<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

Powered by Openwall GNU/*/Linux Powered by OpenVZ