[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 20 Sep 2016 10:03:30 +0900
From: Tomasz Figa <tomasz.figa@...il.com>
To: Chanwoo Choi <cw00.choi@...sung.com>
Cc: Krzysztof Kozłowski <k.kozlowski@...sung.com>,
Kukjin Kim <kgene@...nel.org>,
Rob Herring <robh+dt@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Catalin Marinas <catalin.marinas@....com>,
Will Deacon <will.deacon@....com>,
devicetree <devicetree@...r.kernel.org>,
linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
"linux-samsung-soc@...r.kernel.org"
<linux-samsung-soc@...r.kernel.org>,
linux-kernel <linux-kernel@...r.kernel.org>,
Krzysztof Kozlowski <krzk@...nel.org>,
Jaehoon Chung <jh80.chung@...sung.com>,
"sw0312.kim" <sw0312.kim@...sung.com>,
Joonyoung Shim <jy0922.shim@...sung.com>,
InKi Dae <inki.dae@...sung.com>,
Jonghwa Lee <jonghwa3.lee@...sung.com>,
Beomho Seo <beomho.seo@...sung.com>, jaewon02.kim@...sung.com,
human.hwang@...sung.com, Inha Song <ideal.song@...sung.com>,
ingi2.kim@...sung.com, Marek Szyprowski <m.szyprowski@...sung.com>,
Andrzej Hajda <a.hajda@...sung.com>,
Sylwester Nawrocki <s.nawrocki@...sung.com>,
chanwoo@...nel.org, Linus Walleij <linus.walleij@...aro.org>,
"linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>
Subject: Re: [PATCH v2 3/7] pinctrl: samsung: Add the support the multiple
IORESOURCE_MEM for one pin-bank
Hi Chanwoo,
Sorry for late reply. Yours was quick compared to mine. ;(
2016-09-05 17:08 GMT+09:00 Chanwoo Choi <cw00.choi@...sung.com>:
> Hi Tomasz,
>
> I'm sorry for late reply.
>
> On 2016년 08월 25일 23:41, Tomasz Figa wrote:
>> 2016-08-25 23:30 GMT+09:00 Tomasz Figa <tomasz.figa@...il.com>:
>>>> + }
>>>> +
>>>> +#define EXYNOS_PIN_BANK_EINTN_EXT(pins, reg, id, pctl_idx, eint_idx) \
>>>> + { \
>>>> + .type = &bank_type_off, \
>>>> + .pctl_offset = reg, \
>>>> + .nr_pins = pins, \
>>>> + .eint_type = EINT_TYPE_NONE, \
>>>> + .name = id, \
>>>> + .pctl_res_idx = pctl_idx, \
>>>> + .eint_res_idx = eint_dix \
>>>> + }
>>>
>>> Your patch 4/7 doesn't seem to use this one, so this is dead code for
>>> the time being. Please add when there is real need for it.
>>>
>>> Also it doesn't really make much sense to have index for both pctl and
>>> eint. Please define first entry of regs property as always pointing to
>>> pctl registers and by also eint registers for usual controllers. Then
>>> second regs entry would be eint registers for controllers with
>>> separate register blocks. Then there is only a need to have
>>> eint_res_idx in the driver and no need for pctl_res_idx, because it
>>> would be always 0.
>>
>> Ah, sorry, I got confused again by which registers are where in these
>> GPF banks. Let's make it the other way around and make DT contain eint
>> registers in first regs entry and hardcode eint_res_idx to 0 for the
>> time being.
>
> I got with slight confusion.
> Do you mean that you want to remove the 'eint_res_idx' because
> it is always zero(0) as your comment. And do you agree to add 'pctl_res_idx'?
>
> Also, as you commented, the eint_res_idx for both GPA and GPFx is zero(0).
>
> Example:
> pinctrl_alive: pinctrl@...80000 {
> compatible = "samsung,exynos5433-pinctrl";
> /* ALIVE domain , IMEM domain */
> reg = <0x10580000 0x1a20>, <0x11090000 0x100>;
>
> wakeup-interrupt-controller {
> compatible = "samsung,exynos7-wakeup-eint";
> interrupts = <GIC_SPI 16 0>;
> };
> };
>
> GPA's eint_res_idx is 0
> GPA's pctl_res_idx is 0
>
> GPFx's eint_res_idx is 0
> GPFx's pctl_res_idx is 1
Yeah, that's right. I just want to avoid adding eint_res_idx until it
is found to be really needed.
>
>
> However it should be still beneficial to refactor the code
>> and calculate per-bank eint_base to avoid adding the offset every
>> time, similarly to pctl_base/offset, from my suggestion below.
>
> I agree. I'll modify it according to your comment.
Thanks. Looking forward to the patch.
Best regards,
Tomasz
Powered by blists - more mailing lists