From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755003AbcHXMyR convert rfc822-to-8bit (ORCPT ); Wed, 24 Aug 2016 08:54:17 -0400 Received: from mailout3.samsung.com ([203.254.224.33]:59927 "EHLO mailout3.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751063AbcHXMyM (ORCPT ); Wed, 24 Aug 2016 08:54:12 -0400 X-AuditID: cbfee68d-f79286d000007a9a-d8-57bd98dba480 MIME-version: 1.0 Content-type: text/plain; charset=utf-8 Content-transfer-encoding: 8BIT Message-id: <57BD98DA.7020901@samsung.com> Date: Wed, 24 Aug 2016 21:53:46 +0900 From: Chanwoo Choi Organization: Samsung Electronics User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 To: Tomasz Figa Cc: =?UTF-8?B?S3J6eXN6dG9mIEtvesWCb3dza2k=?= , Kukjin Kim , Rob Herring , Mark Rutland , Catalin Marinas , Will Deacon , devicetree , linux-arm-kernel , "linux-samsung-soc@vger.kernel.org" , linux-kernel , Krzysztof Kozlowski , Jaehoon Chung , "sw0312.kim" , Joonyoung Shim , InKi Dae , Jonghwa Lee , Beomho Seo , jaewon02.kim@samsung.com, human.hwang@samsung.com, Inha Song , ingi2.kim@samsung.com, Marek Szyprowski , Andrzej Hajda , Sylwester Nawrocki , chanwoo@kernel.org, Linus Walleij , "linux-gpio@vger.kernel.org" Subject: Re: [PATCH 4/7] pinctrl: samsung: Add GPFx support of Exynos5433 References: <1471328843-26653-1-git-send-email-cw00.choi@samsung.com> <1471328843-26653-5-git-send-email-cw00.choi@samsung.com> <57B6CC38.9090403@samsung.com> In-reply-to: X-Brightmail-Tracker: H4sIAAAAAAAAA02Sa0iTYRTHefZelaQ3U3scWjiKyNDyMnvMSxIRq4gsPyhdsGWvF9Jpm0r2 RVHUNK9ppMNaVpqtmTIlpjm1uUyynJaX5WUGi9IszS6iZdouRH47/M//nP/5waExexXBpeNE yaxYJIznkba4wsH3osdYhTp89+BXJzT6qI9Avd8eU2iutgCgUv0gjmRak1ZTUUSg1j+TAKne /iDRtckSHKkytATS/8ohUN6r9xiaMgzgaGbKCxUbZzCk0zVSqHz5PgcpjcMEapItA/SmtYpE Fbp2DqrXTlCoZmSAg7LVWgp1fc4lUEXZNInkrSbf9HwPHuIiUNxSAEGLdIISKOV5pGB8uI0U NN1LFxQ1y4Hgu3JzKHXSNvA8Gx+Xyop3BZ+1je2YyKKSMvmXnmhkIAO07cgHNjRkfOGDZiVl rZ1gv6GBzAe2tD1TB+Cofgz/Z5rvnqSsjRoA1dM/CXPDjtkAF8sMJhNNY8x2WF6eYJYxxgEO reQAa70T1lbPYNbZdwDmVDWT1ll3eP3FgmUPzmyDI7MySxhp0jum9BbPesYNDi0agXm/IxMB r/akmWUHU9RCebHlHoz5RMOWJZVlz0bmEOwzDgBrmJoDF9TtmLlhw4TBpc5uCxpkcm1gpfQ1 bk1m4EKZxkIAGVeo7MSsxM7waZ0eLwFQuoZT+p9TuoZTuobzNsDlwJFNikqSnIsRe3lKhAmS FFGMZ1RighKYHqp35UOhCox1BmgAQwPeOrt+Q1u4PSFMlaQlaADfdFApxnWMSjT9oCg50svH zxvxffk+3nv8/Xib7Ny4S2H2TIwwmb3AskmsOFKcEs9KNIBD23AzwN39rqFnVIXKUzp+15fa m7zxCD/1x9KsSvXoavNpceaRvIca7kpP0O+suaD0bEX8uC46BfDs9g4fFxn9C1a6u4NHtt54 uWWkfvZyyLLIJbqlOp07/OxwY94Vj6KDB05k32G9n9MupMFpVZwfmNHgLfXpORbQu++nZ60z rU0sOFrKwyWxQi93TCwR/gWyc2eDSwMAAA== X-Brightmail-Tracker: H4sIAAAAAAAAA2VSe0yNYRz2nu9ac/g6incJ+TaXtZ10TqW3TWa0fObWiDBbPvXtlDqXzlcR MxGZspKOaYnMpO1UWG6d5sjqsKbRVTdUc7CSsUhiXXyns2S8fz17fs/zvM9v+9GYYoDwpON0 SYJRxyewpCteP25nlK/yrZF+17pcUNfNFwSq/3qfQl9unAUot6MVR0U2iSvOzyZQ1VgPQJWd QyQ633MOR5VpNgJ1/Mog0Jnn7zDU192Eo4E+FcqxD2CooeE2hUyjJTJUYW8j0J2iUYBaqgpJ lN/wSIbKbW8oVNzeJEOnrDYK1X46TaD8vH4SmaskXf9gHb7Giyu7UgY4S8EbiqswnyG5120P Se7O9WNc9l0z4L5VLAyn9qSBVbECHyMYvQVdtD4mTqcJYTduj1oXFbjST6VUBaMg1lvHa4UQ NnRTuDIsLkFanPVO4ROSJSqcF0V2xer/E3ZEhCnRlHGXcnPEtj8etd8/b18ZiC052S4z2AIO 3bhcKksD48szgQsNmQA4+LSHcuK5sLH7FpkJXGkFUwygtf874RjIGTc4kteNZwKaxphF0NYc 74TLoMmkdcp7AcwovEs65T7wwrPhSSvOLIHtn4twByYlvrqvY1Izm1kMX47YgSPHg9kFs+pS HbS7FDlsyqEcmRjzkYaWn5WTOXOYDfCFvQk4P7PK4LD1EeYYuDDb4c/HT8lzwK3gr6oF01UL pqteBZgZQMEQbRD3a7RqnXDQV+S1YrJO4xut11aAyUP7ML8S1FatrwEMDdiZ8hFgjVQQfIqY qq0BkMZYdzkn3aFCHsOnHhaM+ihjcoIg1oBAaddczNMjWi+drS4pSuUfsFIdrA5SIbV/EDtP bp+4uVPBaPgkIV4QDIJxyiejXTzTQKF+1VLLvf12r/frfwlzLJoD46GBl1yvtfo0L6HiBxMj hhrN3IyhRs2TWeknMvK2pBOz3caYekt4SyNuffXj29Hi+PmndgfHPc/du3fAXZFaXapP6T2O qCMX35avnehaeGFRoqnkhyEyKxfOeJBNepX4t7BbL1E5vpp7hveKBZ86WVyM5VU+mFHkfwMO LS1kfgMAAA== DLP-Filter: Pass X-MTR: 20000000000000000@CPGS X-CFilter-Loop: Reflected Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Tomasz, I'm sorry for delay reply. On 2016년 08월 19일 20:31, Tomasz Figa wrote: > Hi Chanwoo, > > 2016-08-19 18:07 GMT+09:00 Chanwoo Choi : >> Hi Tomasz Figa, >> >> Due to wrong setting of email client, >> your reply is deleted on my email client at the company. > > I used Gmail (in plain text mode) to reply, was that related? The mistake depend on my filer setting of mail client. > >> I'm so sorry. So, I add your comment on below and then >> I reply the detailed description. > > No problem. Thanks for description. > >> >> On 2016년 08월 16일 15:27, Chanwoo Choi wrote: >>> From: Joonyoung Shim >>> >>> This patch add the support of GPFx pin of Exynos5433 SoC. Exynos5433 has >>> different memory map of GPFx from previous Exynos SoC. Exynos GPIO has >>> following register to control gpio funciton. Usually, all registers of GPIO >>> are included in same domain. >>> - CON / DAT / PUD / DRV / CONPDN / PUDPDN >>> - EINT_CON/ EINT_FLTCON0, EINT_FLTCON1 / EINT_MASK / EINT_PEND >>> >>> But, GPFx are included in two domain as following. So, this patch supports >>> the GPFx pin which handle the on separate two domains. >>> - ALIVE domain : CON / DAT / PUD / DRV / CONPDN / PUDPDN >>> - IMEM domain : EINT_CON/ EINT_FLTCON0, EINT_FLTCON1 / EINT_MASK / EINT_PEND >> >> --------- >> I'm afraid I don't get anything from the description above. Could you >> describe the layout of registers in memory map and IRQ routing of the >> pins? >> >> Best regards, >> Tomasz >> ---------- >> >> On this patch, I'm sorry that I described the wrong information about GFP1~5. >> I explained the memory map of GPF[1-5] the oppositely. Following compositions >> are correct. >> - ALIVE : WEINT_FLTCONx, WEINT_MASK, WEING_PEND >> - IMEM : CON, DAT, PUD, DRV, CONPDN, PUDPDN >> And, I add the memory map for GPF[1~5][2] and the wakeup interrupt information[1]. >> >> [1] Memory map for GPF1~5 >> [ALIVE] >> WEINT_GPA0_CON : 0x1058_0000 (ALIVE) + (0x0700 = 0x0700 + 0x0) >> WEINT_GPA1_CON : 0x1058_0000 (ALIVE) + (0x0704 = 0x0700 + 0x4) >> WEINT_GPA2_CON : 0x1058_0000 (ALIVE) + (0x0708 = 0x0700 + 0x8) >> WEINT_GPA3_CON : 0x1058_0000 (ALIVE) + (0x070C = 0x0700 + 0xC) >> >> WEINT_GPF1_CON : 0x1058_0000 (ALIVE) + (0x1704 = 0x0700 + 0x1004) >> WEINT_GPF2_CON : 0x1058_0000 (ALIVE) + (0x1708 = 0x0700 + 0x1008) >> WEINT_GPF3_CON : 0x1058_0000 (ALIVE) + (0x170C = 0x0700 + 0x100C) >> WEINT_GPF4_CON : 0x1058_0000 (ALIVE) + (0x1710 = 0x0700 + 0x10010) >> WEINT_GPF5_CON : 0x1058_0000 (ALIVE) + (0x1714 = 0x0700 + 0x1014) >> >> WEINT_GPF[x]_MASK : 0x1058_0000 (ALIVE) + (0x1900 + (x * 4)) >> WEINT_GPF[x]_PEND : 0x1058_0000 (ALIVE) + (0x1A00 + (x * 4)) >> (x : 1 ~ 5) >> >> [IMEM] >> GPF1_CON : 0X1109_0000 (IMEM) + 0x0020 >> GPF1_DAT : 0X1109_0000 (IMEM) + 0x0024 >> GPF1_PUD : 0X1109_0000 (IMEM) + 0x0028 >> GPF1_DRV : 0X1109_0000 (IMEM) + 0x002C >> GPF1_CONPDN : 0X1109_0000 (IMEM) + 0x0030 >> GPF1_PUDPDN : 0X1109_0000 (IMEM) + 0x0034 >> >> GPF2_CON : 0X1109_0000 (IMEM) + 0x0040 >> ... >> GPF3_CON : 0X1109_0000 (IMEM) + 0x0060 >> ... >> GPF4_CON : 0X1109_0000 (IMEM) + 0x0080 >> ... >> GPF5_CON : 0X1109_0000 (IMEM) + 0x00A0 >> >> [2] Interrput pin information >> - the total number of wakeup external IRQ is 64. >> ---------------------------------------------------------------------------------- >> domain| gpio : nr | wakeup interrput name | SPI number | >> ----------------------------------------------------------------------------------- >> ALIVE | GPA0 : 8 | INTREQ__EINT[0~7] | SPI[0] ~ SPI[7] | >> ALIVE | GPA1 : 8 | INTREQ__EINT[8~15] | SPI[8] ~ SPI[15] | >> ALIVE | GPA2 : 8 | INTREQ__EINT_16_63 | SPI[16] | >> ALIVE | GPA3 : 8 | INTREQ__EINT_16_63 | SPI[16] | >> ----------------------------------------------------------------------------------- >> ALIVE | GPF1 : 8 | INTREQ__EINT_16_63 | SPI[16] | >> ALIVE | GPF2 : 4 | INTREQ__EINT_16_63 | SPI[16] | >> ALIVE | GPF3 : 4 | INTREQ__EINT_16_63 | SPI[16] | >> ALIVE | GPF4 : 8 | INTREQ__EINT_16_63 | SPI[16] | >> ALIVE | GPF5 : 8 | INTREQ__EINT_16_63 | SPI[16] | >> ----------------------------------------------------------------------------------- >> >> In summary, >> If gpf[1-5] handle the CON/DAT/PUD/DRV register, >> the driver will use the drvdata->ext_base (IMEM base address) >> instead of drvdata->virt_base(ALIVE base address) >> because the CON/DAT/PUD/DRV register of gpf[1-5] are included >> in the IMEM domain. >> >> If gpf[1-5] handle the WEINT_* register, >> the driver will use the drvdata->virt_base(ALIVE base address) >> because the WEINT_* registers of gpf[1-5] are included in the ALIVE domain. > > Okay, so Krzysztof's suggestion doesn't apply because it's not the > eint base which is displaced, but the pinctrl base. I'd suggest the > following solution then: > > - make samsung_pinctrl_drv_data::virt_base an array and save there all > mapped iomem resources, > > - add unsigned pctl_base_res_idx to samsung_pin_bank_data that would > be the index of iomem resource into which the > samsung_pin_bank_data::pctl_offset is an offfset, Existing > EXYNOS_PIN_BANK* macros don't need to be changed, because the field > would be 0 by default. Then only the new bank type macro would have > another argument that would be the resource index, > > - replace samsung_pin_bank::pctl_offset with void __iomem *pctl_base > and precalculate the addresses at probe time for each bank (pctl_base > = virt_base[pctl_base_res_idx] + samsung_pin_bank_data::pctl_offset). > Since currently there is only a problem with pctl_base and eint_base > seems to be only one, EINT code can simply use virt_base[0] all the > time for now. > > Also you should document the second regs entry in the DT binding. > > What do you think? I understand. I suggest the one thing. I think that we need to add the 'eint_base_idx' for WEINT registers which is handled on pinctrl-exynos.c because the composition of gpio registers might be exchanged against the Exynos5433's GPFx. "drvdata->virt_base[pctl_res_idx]" will be used on pinctrl-samsung.c. "drvdata->virt_base[eint_res_idx]" will be used on pinctrl-exynos.c. As your suggestion, I make a patch and this patch is well working. I'll send the new patch for samsung pinctrl driver on v2 patchset. -- Best Regards, Chanwoo Choi