Hi Chanwoo,

Sorry for late reply. Yours was quick compared to mine. ;(

2016-09-05 17:08 GMT+09:00 Chanwoo Choi [off-list ref]:

Hi Tomasz,

I'm sorry for late reply.

On 2016년 08월 25일 23:41, Tomasz Figa wrote:

quoted

2016-08-25 23:30 GMT+09:00 Tomasz Figa [off-list ref]:

quoted

quoted

+       }
+
+#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@10580000 {
                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

quoted

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