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 <cw00.choi@xxxxxxxxxxx>: >> 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 <jy0922.shim@xxxxxxxxxxx> >>> >>> 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 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html