* [PATCH 0/2] pinctrl: mediatek: mt8183: Add support for wake sources @ 2019-04-29 3:25 ` Nicolas Boichat 0 siblings, 0 replies; 18+ messages in thread From: Nicolas Boichat @ 2019-04-29 3:25 UTC (permalink / raw) To: linux-mediatek Cc: Sean Wang, Linus Walleij, Matthias Brugger, linux-gpio, linux-arm-kernel, linux-kernel, Chuanjia Liu, evgreen, swboyd This adds support for wake sources in pinctrl-mtk-common-v2, and pinctrl-mt8183. Without this patch, all interrupts that are left enabled on suspend act as wake sources (and wake sources without interrupt enabled do not). Nicolas Boichat (2): pinctrl: mediatek: Add mtk_eint_pm_ops to common-v2 pinctrl: mediatek: mt8183: Add mtk_eint_pm_ops drivers/pinctrl/mediatek/pinctrl-mt8183.c | 1 + .../pinctrl/mediatek/pinctrl-mtk-common-v2.c | 19 +++++++++++++++++++ .../pinctrl/mediatek/pinctrl-mtk-common-v2.h | 1 + 3 files changed, 21 insertions(+) -- 2.21.0.593.g511ec345e18-goog ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 0/2] pinctrl: mediatek: mt8183: Add support for wake sources @ 2019-04-29 3:25 ` Nicolas Boichat 0 siblings, 0 replies; 18+ messages in thread From: Nicolas Boichat @ 2019-04-29 3:25 UTC (permalink / raw) To: linux-mediatek Cc: Chuanjia Liu, Linus Walleij, Sean Wang, linux-kernel, evgreen, swboyd, linux-gpio, Matthias Brugger, linux-arm-kernel This adds support for wake sources in pinctrl-mtk-common-v2, and pinctrl-mt8183. Without this patch, all interrupts that are left enabled on suspend act as wake sources (and wake sources without interrupt enabled do not). Nicolas Boichat (2): pinctrl: mediatek: Add mtk_eint_pm_ops to common-v2 pinctrl: mediatek: mt8183: Add mtk_eint_pm_ops drivers/pinctrl/mediatek/pinctrl-mt8183.c | 1 + .../pinctrl/mediatek/pinctrl-mtk-common-v2.c | 19 +++++++++++++++++++ .../pinctrl/mediatek/pinctrl-mtk-common-v2.h | 1 + 3 files changed, 21 insertions(+) -- 2.21.0.593.g511ec345e18-goog _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/2] pinctrl: mediatek: Add mtk_eint_pm_ops to common-v2 2019-04-29 3:25 ` Nicolas Boichat @ 2019-04-29 3:25 ` Nicolas Boichat -1 siblings, 0 replies; 18+ messages in thread From: Nicolas Boichat @ 2019-04-29 3:25 UTC (permalink / raw) To: linux-mediatek Cc: Sean Wang, Linus Walleij, Matthias Brugger, linux-gpio, linux-arm-kernel, linux-kernel, Chuanjia Liu, evgreen, swboyd pinctrl variants that include pinctrl-mtk-common-v2.h (and not pinctrl-mtk-common.h) also need to use mtk_eint_pm_ops to setup wake mask properly, so copy over the pm_ops to v2. It is not easy to merge the 2 copies (or move mtk_eint_suspend/resume to mtk-eint.c), as we need to dereference pctrl->eint, and struct mtk_pinctrl *pctl has a different structure definition for v1 and v2. Signed-off-by: Nicolas Boichat <drinkcat@chromium.org> Reviewed-by: Chuanjia Liu <Chuanjia.Liu@mediatek.com> --- .../pinctrl/mediatek/pinctrl-mtk-common-v2.c | 19 +++++++++++++++++++ .../pinctrl/mediatek/pinctrl-mtk-common-v2.h | 1 + 2 files changed, 20 insertions(+) diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c index 20e1c890e73b30c..7e19b5a4748eafe 100644 --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c @@ -723,3 +723,22 @@ int mtk_pinconf_adv_drive_get(struct mtk_pinctrl *hw, return 0; } + +static int mtk_eint_suspend(struct device *device) +{ + struct mtk_pinctrl *pctl = dev_get_drvdata(device); + + return mtk_eint_do_suspend(pctl->eint); +} + +static int mtk_eint_resume(struct device *device) +{ + struct mtk_pinctrl *pctl = dev_get_drvdata(device); + + return mtk_eint_do_resume(pctl->eint); +} + +const struct dev_pm_ops mtk_eint_pm_ops = { + .suspend_noirq = mtk_eint_suspend, + .resume_noirq = mtk_eint_resume, +}; diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.h b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.h index 1b7da42aa1d53e4..e2048db5bb16671 100644 --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.h +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.h @@ -299,4 +299,5 @@ int mtk_pinconf_adv_drive_set(struct mtk_pinctrl *hw, int mtk_pinconf_adv_drive_get(struct mtk_pinctrl *hw, const struct mtk_pin_desc *desc, u32 *val); +extern const struct dev_pm_ops mtk_eint_pm_ops; #endif /* __PINCTRL_MTK_COMMON_V2_H */ -- 2.21.0.593.g511ec345e18-goog ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 1/2] pinctrl: mediatek: Add mtk_eint_pm_ops to common-v2 @ 2019-04-29 3:25 ` Nicolas Boichat 0 siblings, 0 replies; 18+ messages in thread From: Nicolas Boichat @ 2019-04-29 3:25 UTC (permalink / raw) To: linux-mediatek Cc: Chuanjia Liu, Linus Walleij, Sean Wang, linux-kernel, evgreen, swboyd, linux-gpio, Matthias Brugger, linux-arm-kernel pinctrl variants that include pinctrl-mtk-common-v2.h (and not pinctrl-mtk-common.h) also need to use mtk_eint_pm_ops to setup wake mask properly, so copy over the pm_ops to v2. It is not easy to merge the 2 copies (or move mtk_eint_suspend/resume to mtk-eint.c), as we need to dereference pctrl->eint, and struct mtk_pinctrl *pctl has a different structure definition for v1 and v2. Signed-off-by: Nicolas Boichat <drinkcat@chromium.org> Reviewed-by: Chuanjia Liu <Chuanjia.Liu@mediatek.com> --- .../pinctrl/mediatek/pinctrl-mtk-common-v2.c | 19 +++++++++++++++++++ .../pinctrl/mediatek/pinctrl-mtk-common-v2.h | 1 + 2 files changed, 20 insertions(+) diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c index 20e1c890e73b30c..7e19b5a4748eafe 100644 --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c @@ -723,3 +723,22 @@ int mtk_pinconf_adv_drive_get(struct mtk_pinctrl *hw, return 0; } + +static int mtk_eint_suspend(struct device *device) +{ + struct mtk_pinctrl *pctl = dev_get_drvdata(device); + + return mtk_eint_do_suspend(pctl->eint); +} + +static int mtk_eint_resume(struct device *device) +{ + struct mtk_pinctrl *pctl = dev_get_drvdata(device); + + return mtk_eint_do_resume(pctl->eint); +} + +const struct dev_pm_ops mtk_eint_pm_ops = { + .suspend_noirq = mtk_eint_suspend, + .resume_noirq = mtk_eint_resume, +}; diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.h b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.h index 1b7da42aa1d53e4..e2048db5bb16671 100644 --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.h +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.h @@ -299,4 +299,5 @@ int mtk_pinconf_adv_drive_set(struct mtk_pinctrl *hw, int mtk_pinconf_adv_drive_get(struct mtk_pinctrl *hw, const struct mtk_pin_desc *desc, u32 *val); +extern const struct dev_pm_ops mtk_eint_pm_ops; #endif /* __PINCTRL_MTK_COMMON_V2_H */ -- 2.21.0.593.g511ec345e18-goog _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] pinctrl: mediatek: Add mtk_eint_pm_ops to common-v2 2019-04-29 3:25 ` Nicolas Boichat (?) @ 2019-05-02 13:48 ` Yingjoe Chen -1 siblings, 0 replies; 18+ messages in thread From: Yingjoe Chen @ 2019-05-02 13:48 UTC (permalink / raw) To: Nicolas Boichat Cc: Chuanjia Liu, Linus Walleij, Sean Wang, linux-kernel, evgreen, swboyd, linux-gpio, linux-mediatek, Matthias Brugger, linux-arm-kernel On Mon, 2019-04-29 at 11:25 +0800, Nicolas Boichat wrote: > pinctrl variants that include pinctrl-mtk-common-v2.h (and not > pinctrl-mtk-common.h) also need to use mtk_eint_pm_ops to setup > wake mask properly, so copy over the pm_ops to v2. > > It is not easy to merge the 2 copies (or move > mtk_eint_suspend/resume to mtk-eint.c), as we need to > dereference pctrl->eint, and struct mtk_pinctrl *pctl has a > different structure definition for v1 and v2. > > Signed-off-by: Nicolas Boichat <drinkcat@chromium.org> > Reviewed-by: Chuanjia Liu <Chuanjia.Liu@mediatek.com> > --- > .../pinctrl/mediatek/pinctrl-mtk-common-v2.c | 19 +++++++++++++++++++ > .../pinctrl/mediatek/pinctrl-mtk-common-v2.h | 1 + > 2 files changed, 20 insertions(+) > > diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > index 20e1c890e73b30c..7e19b5a4748eafe 100644 > --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > @@ -723,3 +723,22 @@ int mtk_pinconf_adv_drive_get(struct mtk_pinctrl *hw, > > return 0; > } > + > +static int mtk_eint_suspend(struct device *device) > +{ > + struct mtk_pinctrl *pctl = dev_get_drvdata(device); > + > + return mtk_eint_do_suspend(pctl->eint); > +} > + > +static int mtk_eint_resume(struct device *device) > +{ > + struct mtk_pinctrl *pctl = dev_get_drvdata(device); > + > + return mtk_eint_do_resume(pctl->eint); > +} > + > +const struct dev_pm_ops mtk_eint_pm_ops = { > + .suspend_noirq = mtk_eint_suspend, > + .resume_noirq = mtk_eint_resume, > +}; This is identical to the one in pinctrl-mtk-common.c and will have name clash if both pinctrl-mtk-common.c and pinctrl-mtk-common-v2.c are built. It would be better if we try to merge both version into mtk-eint.c, this way we could also remove some global functions. Joe.C ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] pinctrl: mediatek: Add mtk_eint_pm_ops to common-v2 @ 2019-05-02 13:48 ` Yingjoe Chen 0 siblings, 0 replies; 18+ messages in thread From: Yingjoe Chen @ 2019-05-02 13:48 UTC (permalink / raw) To: Nicolas Boichat Cc: Chuanjia Liu, Linus Walleij, Sean Wang, linux-kernel, evgreen, swboyd, linux-gpio, linux-mediatek, Matthias Brugger, linux-arm-kernel On Mon, 2019-04-29 at 11:25 +0800, Nicolas Boichat wrote: > pinctrl variants that include pinctrl-mtk-common-v2.h (and not > pinctrl-mtk-common.h) also need to use mtk_eint_pm_ops to setup > wake mask properly, so copy over the pm_ops to v2. > > It is not easy to merge the 2 copies (or move > mtk_eint_suspend/resume to mtk-eint.c), as we need to > dereference pctrl->eint, and struct mtk_pinctrl *pctl has a > different structure definition for v1 and v2. > > Signed-off-by: Nicolas Boichat <drinkcat@chromium.org> > Reviewed-by: Chuanjia Liu <Chuanjia.Liu@mediatek.com> > --- > .../pinctrl/mediatek/pinctrl-mtk-common-v2.c | 19 +++++++++++++++++++ > .../pinctrl/mediatek/pinctrl-mtk-common-v2.h | 1 + > 2 files changed, 20 insertions(+) > > diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > index 20e1c890e73b30c..7e19b5a4748eafe 100644 > --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > @@ -723,3 +723,22 @@ int mtk_pinconf_adv_drive_get(struct mtk_pinctrl *hw, > > return 0; > } > + > +static int mtk_eint_suspend(struct device *device) > +{ > + struct mtk_pinctrl *pctl = dev_get_drvdata(device); > + > + return mtk_eint_do_suspend(pctl->eint); > +} > + > +static int mtk_eint_resume(struct device *device) > +{ > + struct mtk_pinctrl *pctl = dev_get_drvdata(device); > + > + return mtk_eint_do_resume(pctl->eint); > +} > + > +const struct dev_pm_ops mtk_eint_pm_ops = { > + .suspend_noirq = mtk_eint_suspend, > + .resume_noirq = mtk_eint_resume, > +}; This is identical to the one in pinctrl-mtk-common.c and will have name clash if both pinctrl-mtk-common.c and pinctrl-mtk-common-v2.c are built. It would be better if we try to merge both version into mtk-eint.c, this way we could also remove some global functions. Joe.C _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] pinctrl: mediatek: Add mtk_eint_pm_ops to common-v2 @ 2019-05-02 13:48 ` Yingjoe Chen 0 siblings, 0 replies; 18+ messages in thread From: Yingjoe Chen @ 2019-05-02 13:48 UTC (permalink / raw) To: Nicolas Boichat Cc: linux-mediatek, Chuanjia Liu, Linus Walleij, Sean Wang, linux-kernel, evgreen, swboyd, linux-gpio, Matthias Brugger, linux-arm-kernel On Mon, 2019-04-29 at 11:25 +0800, Nicolas Boichat wrote: > pinctrl variants that include pinctrl-mtk-common-v2.h (and not > pinctrl-mtk-common.h) also need to use mtk_eint_pm_ops to setup > wake mask properly, so copy over the pm_ops to v2. > > It is not easy to merge the 2 copies (or move > mtk_eint_suspend/resume to mtk-eint.c), as we need to > dereference pctrl->eint, and struct mtk_pinctrl *pctl has a > different structure definition for v1 and v2. > > Signed-off-by: Nicolas Boichat <drinkcat@chromium.org> > Reviewed-by: Chuanjia Liu <Chuanjia.Liu@mediatek.com> > --- > .../pinctrl/mediatek/pinctrl-mtk-common-v2.c | 19 +++++++++++++++++++ > .../pinctrl/mediatek/pinctrl-mtk-common-v2.h | 1 + > 2 files changed, 20 insertions(+) > > diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > index 20e1c890e73b30c..7e19b5a4748eafe 100644 > --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > @@ -723,3 +723,22 @@ int mtk_pinconf_adv_drive_get(struct mtk_pinctrl *hw, > > return 0; > } > + > +static int mtk_eint_suspend(struct device *device) > +{ > + struct mtk_pinctrl *pctl = dev_get_drvdata(device); > + > + return mtk_eint_do_suspend(pctl->eint); > +} > + > +static int mtk_eint_resume(struct device *device) > +{ > + struct mtk_pinctrl *pctl = dev_get_drvdata(device); > + > + return mtk_eint_do_resume(pctl->eint); > +} > + > +const struct dev_pm_ops mtk_eint_pm_ops = { > + .suspend_noirq = mtk_eint_suspend, > + .resume_noirq = mtk_eint_resume, > +}; This is identical to the one in pinctrl-mtk-common.c and will have name clash if both pinctrl-mtk-common.c and pinctrl-mtk-common-v2.c are built. It would be better if we try to merge both version into mtk-eint.c, this way we could also remove some global functions. Joe.C ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] pinctrl: mediatek: Add mtk_eint_pm_ops to common-v2 2019-05-02 13:48 ` Yingjoe Chen (?) @ 2019-05-03 0:52 ` Nicolas Boichat -1 siblings, 0 replies; 18+ messages in thread From: Nicolas Boichat @ 2019-05-03 0:52 UTC (permalink / raw) To: Yingjoe Chen Cc: Chuanjia Liu, Linus Walleij, Sean Wang, lkml, Evan Green, Stephen Boyd, linux-gpio, moderated list:ARM/Mediatek SoC support, Matthias Brugger, linux-arm Mailing List On Thu, May 2, 2019 at 9:48 PM Yingjoe Chen <yingjoe.chen@mediatek.com> wrote: > > On Mon, 2019-04-29 at 11:25 +0800, Nicolas Boichat wrote: > > pinctrl variants that include pinctrl-mtk-common-v2.h (and not > > pinctrl-mtk-common.h) also need to use mtk_eint_pm_ops to setup > > wake mask properly, so copy over the pm_ops to v2. > > > > It is not easy to merge the 2 copies (or move > > mtk_eint_suspend/resume to mtk-eint.c), as we need to > > dereference pctrl->eint, and struct mtk_pinctrl *pctl has a > > different structure definition for v1 and v2. > > > > Signed-off-by: Nicolas Boichat <drinkcat@chromium.org> > > Reviewed-by: Chuanjia Liu <Chuanjia.Liu@mediatek.com> > > --- > > .../pinctrl/mediatek/pinctrl-mtk-common-v2.c | 19 +++++++++++++++++++ > > .../pinctrl/mediatek/pinctrl-mtk-common-v2.h | 1 + > > 2 files changed, 20 insertions(+) > > > > diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > > index 20e1c890e73b30c..7e19b5a4748eafe 100644 > > --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > > +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > > @@ -723,3 +723,22 @@ int mtk_pinconf_adv_drive_get(struct mtk_pinctrl *hw, > > > > return 0; > > } > > + > > +static int mtk_eint_suspend(struct device *device) > > +{ > > + struct mtk_pinctrl *pctl = dev_get_drvdata(device); > > + > > + return mtk_eint_do_suspend(pctl->eint); > > +} > > + > > +static int mtk_eint_resume(struct device *device) > > +{ > > + struct mtk_pinctrl *pctl = dev_get_drvdata(device); > > + > > + return mtk_eint_do_resume(pctl->eint); > > +} > > + > > +const struct dev_pm_ops mtk_eint_pm_ops = { > > + .suspend_noirq = mtk_eint_suspend, > > + .resume_noirq = mtk_eint_resume, > > +}; > > This is identical to the one in pinctrl-mtk-common.c and will have name > clash if both pinctrl-mtk-common.c and pinctrl-mtk-common-v2.c are > built. > > It would be better if we try to merge both version into mtk-eint.c, this > way we could also remove some global functions. Argh, I didn't think about the name clash, you're right. I guess the easy way is to rename this one mtk_eint_pm_ops_v2 ... As highlighted in the commit message, it's tricky to merge the 2 sets of functions, they look identical, but they actually work on struct mtk_pinctrl that are defined differently (in pinctrl-mtk-common[-v2].h), so the ->eint member is at different addresses... I don't really see a way around this... Unless we want to change platform_set_drvdata(pdev, pctl); to pass another type of structure that could be shared (but I think that'll make the code fairly verbose, with another layer of indirection). Or just assign struct mtk_eint to that, since that contains pctl so we could get back the struct mtk_pinctrl from that, but that feels ugly as well... > > Joe.C > > > > _______________________________________________ > Linux-mediatek mailing list > Linux-mediatek@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-mediatek ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] pinctrl: mediatek: Add mtk_eint_pm_ops to common-v2 @ 2019-05-03 0:52 ` Nicolas Boichat 0 siblings, 0 replies; 18+ messages in thread From: Nicolas Boichat @ 2019-05-03 0:52 UTC (permalink / raw) To: Yingjoe Chen Cc: Chuanjia Liu, Linus Walleij, Sean Wang, lkml, Evan Green, Stephen Boyd, linux-gpio, moderated list:ARM/Mediatek SoC support, Matthias Brugger, linux-arm Mailing List On Thu, May 2, 2019 at 9:48 PM Yingjoe Chen <yingjoe.chen@mediatek.com> wrote: > > On Mon, 2019-04-29 at 11:25 +0800, Nicolas Boichat wrote: > > pinctrl variants that include pinctrl-mtk-common-v2.h (and not > > pinctrl-mtk-common.h) also need to use mtk_eint_pm_ops to setup > > wake mask properly, so copy over the pm_ops to v2. > > > > It is not easy to merge the 2 copies (or move > > mtk_eint_suspend/resume to mtk-eint.c), as we need to > > dereference pctrl->eint, and struct mtk_pinctrl *pctl has a > > different structure definition for v1 and v2. > > > > Signed-off-by: Nicolas Boichat <drinkcat@chromium.org> > > Reviewed-by: Chuanjia Liu <Chuanjia.Liu@mediatek.com> > > --- > > .../pinctrl/mediatek/pinctrl-mtk-common-v2.c | 19 +++++++++++++++++++ > > .../pinctrl/mediatek/pinctrl-mtk-common-v2.h | 1 + > > 2 files changed, 20 insertions(+) > > > > diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > > index 20e1c890e73b30c..7e19b5a4748eafe 100644 > > --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > > +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > > @@ -723,3 +723,22 @@ int mtk_pinconf_adv_drive_get(struct mtk_pinctrl *hw, > > > > return 0; > > } > > + > > +static int mtk_eint_suspend(struct device *device) > > +{ > > + struct mtk_pinctrl *pctl = dev_get_drvdata(device); > > + > > + return mtk_eint_do_suspend(pctl->eint); > > +} > > + > > +static int mtk_eint_resume(struct device *device) > > +{ > > + struct mtk_pinctrl *pctl = dev_get_drvdata(device); > > + > > + return mtk_eint_do_resume(pctl->eint); > > +} > > + > > +const struct dev_pm_ops mtk_eint_pm_ops = { > > + .suspend_noirq = mtk_eint_suspend, > > + .resume_noirq = mtk_eint_resume, > > +}; > > This is identical to the one in pinctrl-mtk-common.c and will have name > clash if both pinctrl-mtk-common.c and pinctrl-mtk-common-v2.c are > built. > > It would be better if we try to merge both version into mtk-eint.c, this > way we could also remove some global functions. Argh, I didn't think about the name clash, you're right. I guess the easy way is to rename this one mtk_eint_pm_ops_v2 ... As highlighted in the commit message, it's tricky to merge the 2 sets of functions, they look identical, but they actually work on struct mtk_pinctrl that are defined differently (in pinctrl-mtk-common[-v2].h), so the ->eint member is at different addresses... I don't really see a way around this... Unless we want to change platform_set_drvdata(pdev, pctl); to pass another type of structure that could be shared (but I think that'll make the code fairly verbose, with another layer of indirection). Or just assign struct mtk_eint to that, since that contains pctl so we could get back the struct mtk_pinctrl from that, but that feels ugly as well... > > Joe.C > > > > _______________________________________________ > Linux-mediatek mailing list > Linux-mediatek@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-mediatek _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] pinctrl: mediatek: Add mtk_eint_pm_ops to common-v2 @ 2019-05-03 0:52 ` Nicolas Boichat 0 siblings, 0 replies; 18+ messages in thread From: Nicolas Boichat @ 2019-05-03 0:52 UTC (permalink / raw) To: Yingjoe Chen Cc: Chuanjia Liu, Linus Walleij, Sean Wang, lkml, Evan Green, Stephen Boyd, linux-gpio, moderated list:ARM/Mediatek SoC support, Matthias Brugger, linux-arm Mailing List On Thu, May 2, 2019 at 9:48 PM Yingjoe Chen <yingjoe.chen@mediatek.com> wrote: > > On Mon, 2019-04-29 at 11:25 +0800, Nicolas Boichat wrote: > > pinctrl variants that include pinctrl-mtk-common-v2.h (and not > > pinctrl-mtk-common.h) also need to use mtk_eint_pm_ops to setup > > wake mask properly, so copy over the pm_ops to v2. > > > > It is not easy to merge the 2 copies (or move > > mtk_eint_suspend/resume to mtk-eint.c), as we need to > > dereference pctrl->eint, and struct mtk_pinctrl *pctl has a > > different structure definition for v1 and v2. > > > > Signed-off-by: Nicolas Boichat <drinkcat@chromium.org> > > Reviewed-by: Chuanjia Liu <Chuanjia.Liu@mediatek.com> > > --- > > .../pinctrl/mediatek/pinctrl-mtk-common-v2.c | 19 +++++++++++++++++++ > > .../pinctrl/mediatek/pinctrl-mtk-common-v2.h | 1 + > > 2 files changed, 20 insertions(+) > > > > diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > > index 20e1c890e73b30c..7e19b5a4748eafe 100644 > > --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > > +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > > @@ -723,3 +723,22 @@ int mtk_pinconf_adv_drive_get(struct mtk_pinctrl *hw, > > > > return 0; > > } > > + > > +static int mtk_eint_suspend(struct device *device) > > +{ > > + struct mtk_pinctrl *pctl = dev_get_drvdata(device); > > + > > + return mtk_eint_do_suspend(pctl->eint); > > +} > > + > > +static int mtk_eint_resume(struct device *device) > > +{ > > + struct mtk_pinctrl *pctl = dev_get_drvdata(device); > > + > > + return mtk_eint_do_resume(pctl->eint); > > +} > > + > > +const struct dev_pm_ops mtk_eint_pm_ops = { > > + .suspend_noirq = mtk_eint_suspend, > > + .resume_noirq = mtk_eint_resume, > > +}; > > This is identical to the one in pinctrl-mtk-common.c and will have name > clash if both pinctrl-mtk-common.c and pinctrl-mtk-common-v2.c are > built. > > It would be better if we try to merge both version into mtk-eint.c, this > way we could also remove some global functions. Argh, I didn't think about the name clash, you're right. I guess the easy way is to rename this one mtk_eint_pm_ops_v2 ... As highlighted in the commit message, it's tricky to merge the 2 sets of functions, they look identical, but they actually work on struct mtk_pinctrl that are defined differently (in pinctrl-mtk-common[-v2].h), so the ->eint member is at different addresses... I don't really see a way around this... Unless we want to change platform_set_drvdata(pdev, pctl); to pass another type of structure that could be shared (but I think that'll make the code fairly verbose, with another layer of indirection). Or just assign struct mtk_eint to that, since that contains pctl so we could get back the struct mtk_pinctrl from that, but that feels ugly as well... > > Joe.C > > > > _______________________________________________ > Linux-mediatek mailing list > Linux-mediatek@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-mediatek ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] pinctrl: mediatek: Add mtk_eint_pm_ops to common-v2 2019-05-03 0:52 ` Nicolas Boichat (?) @ 2019-05-03 17:08 ` Sean Wang -1 siblings, 0 replies; 18+ messages in thread From: Sean Wang @ 2019-05-03 17:08 UTC (permalink / raw) To: Nicolas Boichat Cc: Yingjoe Chen, Chuanjia Liu, Linus Walleij, lkml, Evan Green, Stephen Boyd, linux-gpio, moderated list:ARM/Mediatek SoC support, Matthias Brugger, linux-arm Mailing List Hi, Nicolas On Thu, May 2, 2019 at 5:53 PM Nicolas Boichat <drinkcat@chromium.org> wrote: > > On Thu, May 2, 2019 at 9:48 PM Yingjoe Chen <yingjoe.chen@mediatek.com> wrote: > > > > On Mon, 2019-04-29 at 11:25 +0800, Nicolas Boichat wrote: > > > pinctrl variants that include pinctrl-mtk-common-v2.h (and not > > > pinctrl-mtk-common.h) also need to use mtk_eint_pm_ops to setup > > > wake mask properly, so copy over the pm_ops to v2. > > > > > > It is not easy to merge the 2 copies (or move > > > mtk_eint_suspend/resume to mtk-eint.c), as we need to > > > dereference pctrl->eint, and struct mtk_pinctrl *pctl has a > > > different structure definition for v1 and v2. > > > > > > Signed-off-by: Nicolas Boichat <drinkcat@chromium.org> > > > Reviewed-by: Chuanjia Liu <Chuanjia.Liu@mediatek.com> > > > --- > > > .../pinctrl/mediatek/pinctrl-mtk-common-v2.c | 19 +++++++++++++++++++ > > > .../pinctrl/mediatek/pinctrl-mtk-common-v2.h | 1 + > > > 2 files changed, 20 insertions(+) > > > > > > diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > > > index 20e1c890e73b30c..7e19b5a4748eafe 100644 > > > --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > > > +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > > > @@ -723,3 +723,22 @@ int mtk_pinconf_adv_drive_get(struct mtk_pinctrl *hw, > > > > > > return 0; > > > } > > > + > > > +static int mtk_eint_suspend(struct device *device) > > > +{ > > > + struct mtk_pinctrl *pctl = dev_get_drvdata(device); > > > + > > > + return mtk_eint_do_suspend(pctl->eint); > > > +} > > > + > > > +static int mtk_eint_resume(struct device *device) > > > +{ > > > + struct mtk_pinctrl *pctl = dev_get_drvdata(device); > > > + > > > + return mtk_eint_do_resume(pctl->eint); > > > +} > > > + > > > +const struct dev_pm_ops mtk_eint_pm_ops = { > > > + .suspend_noirq = mtk_eint_suspend, > > > + .resume_noirq = mtk_eint_resume, > > > +}; > > > > This is identical to the one in pinctrl-mtk-common.c and will have name > > clash if both pinctrl-mtk-common.c and pinctrl-mtk-common-v2.c are > > built. > > > > It would be better if we try to merge both version into mtk-eint.c, this > > way we could also remove some global functions. > > Argh, I didn't think about the name clash, you're right. I guess the > easy way is to rename this one mtk_eint_pm_ops_v2 ... > > As highlighted in the commit message, it's tricky to merge the 2 sets > of functions, they look identical, but they actually work on struct > mtk_pinctrl that are defined differently (in > pinctrl-mtk-common[-v2].h), so the ->eint member is at different > addresses... > > I don't really see a way around this... Unless we want to change > platform_set_drvdata(pdev, pctl); to pass another type of structure > that could be shared (but I think that'll make the code fairly > verbose, with another layer of indirection). Or just assign struct > mtk_eint to that, since that contains pctl so we could get back the > struct mtk_pinctrl from that, but that feels ugly as well... > I agree on renaming would make the thing simple. but I wouldn't like to rename to mtk_eint_pm_ops_v2 since this would make people misunderstand that is mtk_eint_v2. How about renaming to mtk_paris_pinctrl_pm_ops and then place related logic you added into pinctrl-paris.c? Because I prefer to keep pure pinctrl hardware operations in pinctrl-mtk-common-v2.c, and for relevant to other modules (mtk eint) or others subsystem (device tree binding, GPIO subsytem, PM something like that) they should be moved to pinctrl-paris.c or pinctrl-moore.c Sean > > > > Joe.C > > > > > > > > _______________________________________________ > > Linux-mediatek mailing list > > Linux-mediatek@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-mediatek ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] pinctrl: mediatek: Add mtk_eint_pm_ops to common-v2 @ 2019-05-03 17:08 ` Sean Wang 0 siblings, 0 replies; 18+ messages in thread From: Sean Wang @ 2019-05-03 17:08 UTC (permalink / raw) To: Nicolas Boichat Cc: Chuanjia Liu, Linus Walleij, lkml, Evan Green, Stephen Boyd, linux-gpio, moderated list:ARM/Mediatek SoC support, Matthias Brugger, Yingjoe Chen, linux-arm Mailing List Hi, Nicolas On Thu, May 2, 2019 at 5:53 PM Nicolas Boichat <drinkcat@chromium.org> wrote: > > On Thu, May 2, 2019 at 9:48 PM Yingjoe Chen <yingjoe.chen@mediatek.com> wrote: > > > > On Mon, 2019-04-29 at 11:25 +0800, Nicolas Boichat wrote: > > > pinctrl variants that include pinctrl-mtk-common-v2.h (and not > > > pinctrl-mtk-common.h) also need to use mtk_eint_pm_ops to setup > > > wake mask properly, so copy over the pm_ops to v2. > > > > > > It is not easy to merge the 2 copies (or move > > > mtk_eint_suspend/resume to mtk-eint.c), as we need to > > > dereference pctrl->eint, and struct mtk_pinctrl *pctl has a > > > different structure definition for v1 and v2. > > > > > > Signed-off-by: Nicolas Boichat <drinkcat@chromium.org> > > > Reviewed-by: Chuanjia Liu <Chuanjia.Liu@mediatek.com> > > > --- > > > .../pinctrl/mediatek/pinctrl-mtk-common-v2.c | 19 +++++++++++++++++++ > > > .../pinctrl/mediatek/pinctrl-mtk-common-v2.h | 1 + > > > 2 files changed, 20 insertions(+) > > > > > > diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > > > index 20e1c890e73b30c..7e19b5a4748eafe 100644 > > > --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > > > +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > > > @@ -723,3 +723,22 @@ int mtk_pinconf_adv_drive_get(struct mtk_pinctrl *hw, > > > > > > return 0; > > > } > > > + > > > +static int mtk_eint_suspend(struct device *device) > > > +{ > > > + struct mtk_pinctrl *pctl = dev_get_drvdata(device); > > > + > > > + return mtk_eint_do_suspend(pctl->eint); > > > +} > > > + > > > +static int mtk_eint_resume(struct device *device) > > > +{ > > > + struct mtk_pinctrl *pctl = dev_get_drvdata(device); > > > + > > > + return mtk_eint_do_resume(pctl->eint); > > > +} > > > + > > > +const struct dev_pm_ops mtk_eint_pm_ops = { > > > + .suspend_noirq = mtk_eint_suspend, > > > + .resume_noirq = mtk_eint_resume, > > > +}; > > > > This is identical to the one in pinctrl-mtk-common.c and will have name > > clash if both pinctrl-mtk-common.c and pinctrl-mtk-common-v2.c are > > built. > > > > It would be better if we try to merge both version into mtk-eint.c, this > > way we could also remove some global functions. > > Argh, I didn't think about the name clash, you're right. I guess the > easy way is to rename this one mtk_eint_pm_ops_v2 ... > > As highlighted in the commit message, it's tricky to merge the 2 sets > of functions, they look identical, but they actually work on struct > mtk_pinctrl that are defined differently (in > pinctrl-mtk-common[-v2].h), so the ->eint member is at different > addresses... > > I don't really see a way around this... Unless we want to change > platform_set_drvdata(pdev, pctl); to pass another type of structure > that could be shared (but I think that'll make the code fairly > verbose, with another layer of indirection). Or just assign struct > mtk_eint to that, since that contains pctl so we could get back the > struct mtk_pinctrl from that, but that feels ugly as well... > I agree on renaming would make the thing simple. but I wouldn't like to rename to mtk_eint_pm_ops_v2 since this would make people misunderstand that is mtk_eint_v2. How about renaming to mtk_paris_pinctrl_pm_ops and then place related logic you added into pinctrl-paris.c? Because I prefer to keep pure pinctrl hardware operations in pinctrl-mtk-common-v2.c, and for relevant to other modules (mtk eint) or others subsystem (device tree binding, GPIO subsytem, PM something like that) they should be moved to pinctrl-paris.c or pinctrl-moore.c Sean > > > > Joe.C > > > > > > > > _______________________________________________ > > Linux-mediatek mailing list > > Linux-mediatek@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-mediatek _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] pinctrl: mediatek: Add mtk_eint_pm_ops to common-v2 @ 2019-05-03 17:08 ` Sean Wang 0 siblings, 0 replies; 18+ messages in thread From: Sean Wang @ 2019-05-03 17:08 UTC (permalink / raw) To: Nicolas Boichat Cc: Yingjoe Chen, Chuanjia Liu, Linus Walleij, lkml, Evan Green, Stephen Boyd, linux-gpio, moderated list:ARM/Mediatek SoC support, Matthias Brugger, linux-arm Mailing List Hi, Nicolas On Thu, May 2, 2019 at 5:53 PM Nicolas Boichat <drinkcat@chromium.org> wrote: > > On Thu, May 2, 2019 at 9:48 PM Yingjoe Chen <yingjoe.chen@mediatek.com> wrote: > > > > On Mon, 2019-04-29 at 11:25 +0800, Nicolas Boichat wrote: > > > pinctrl variants that include pinctrl-mtk-common-v2.h (and not > > > pinctrl-mtk-common.h) also need to use mtk_eint_pm_ops to setup > > > wake mask properly, so copy over the pm_ops to v2. > > > > > > It is not easy to merge the 2 copies (or move > > > mtk_eint_suspend/resume to mtk-eint.c), as we need to > > > dereference pctrl->eint, and struct mtk_pinctrl *pctl has a > > > different structure definition for v1 and v2. > > > > > > Signed-off-by: Nicolas Boichat <drinkcat@chromium.org> > > > Reviewed-by: Chuanjia Liu <Chuanjia.Liu@mediatek.com> > > > --- > > > .../pinctrl/mediatek/pinctrl-mtk-common-v2.c | 19 +++++++++++++++++++ > > > .../pinctrl/mediatek/pinctrl-mtk-common-v2.h | 1 + > > > 2 files changed, 20 insertions(+) > > > > > > diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > > > index 20e1c890e73b30c..7e19b5a4748eafe 100644 > > > --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > > > +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > > > @@ -723,3 +723,22 @@ int mtk_pinconf_adv_drive_get(struct mtk_pinctrl *hw, > > > > > > return 0; > > > } > > > + > > > +static int mtk_eint_suspend(struct device *device) > > > +{ > > > + struct mtk_pinctrl *pctl = dev_get_drvdata(device); > > > + > > > + return mtk_eint_do_suspend(pctl->eint); > > > +} > > > + > > > +static int mtk_eint_resume(struct device *device) > > > +{ > > > + struct mtk_pinctrl *pctl = dev_get_drvdata(device); > > > + > > > + return mtk_eint_do_resume(pctl->eint); > > > +} > > > + > > > +const struct dev_pm_ops mtk_eint_pm_ops = { > > > + .suspend_noirq = mtk_eint_suspend, > > > + .resume_noirq = mtk_eint_resume, > > > +}; > > > > This is identical to the one in pinctrl-mtk-common.c and will have name > > clash if both pinctrl-mtk-common.c and pinctrl-mtk-common-v2.c are > > built. > > > > It would be better if we try to merge both version into mtk-eint.c, this > > way we could also remove some global functions. > > Argh, I didn't think about the name clash, you're right. I guess the > easy way is to rename this one mtk_eint_pm_ops_v2 ... > > As highlighted in the commit message, it's tricky to merge the 2 sets > of functions, they look identical, but they actually work on struct > mtk_pinctrl that are defined differently (in > pinctrl-mtk-common[-v2].h), so the ->eint member is at different > addresses... > > I don't really see a way around this... Unless we want to change > platform_set_drvdata(pdev, pctl); to pass another type of structure > that could be shared (but I think that'll make the code fairly > verbose, with another layer of indirection). Or just assign struct > mtk_eint to that, since that contains pctl so we could get back the > struct mtk_pinctrl from that, but that feels ugly as well... > I agree on renaming would make the thing simple. but I wouldn't like to rename to mtk_eint_pm_ops_v2 since this would make people misunderstand that is mtk_eint_v2. How about renaming to mtk_paris_pinctrl_pm_ops and then place related logic you added into pinctrl-paris.c? Because I prefer to keep pure pinctrl hardware operations in pinctrl-mtk-common-v2.c, and for relevant to other modules (mtk eint) or others subsystem (device tree binding, GPIO subsytem, PM something like that) they should be moved to pinctrl-paris.c or pinctrl-moore.c Sean > > > > Joe.C > > > > > > > > _______________________________________________ > > Linux-mediatek mailing list > > Linux-mediatek@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-mediatek ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] pinctrl: mediatek: Add mtk_eint_pm_ops to common-v2 2019-05-03 17:08 ` Sean Wang (?) @ 2019-05-08 7:39 ` Nicolas Boichat -1 siblings, 0 replies; 18+ messages in thread From: Nicolas Boichat @ 2019-05-08 7:39 UTC (permalink / raw) To: Sean Wang Cc: Yingjoe Chen, Chuanjia Liu, Linus Walleij, lkml, Evan Green, Stephen Boyd, linux-gpio, moderated list:ARM/Mediatek SoC support, Matthias Brugger, linux-arm Mailing List On Sat, May 4, 2019 at 2:09 AM Sean Wang <sean.wang@kernel.org> wrote: > > Hi, Nicolas > > On Thu, May 2, 2019 at 5:53 PM Nicolas Boichat <drinkcat@chromium.org> wrote: > > > > On Thu, May 2, 2019 at 9:48 PM Yingjoe Chen <yingjoe.chen@mediatek.com> wrote: > > > > > > On Mon, 2019-04-29 at 11:25 +0800, Nicolas Boichat wrote: > > > > pinctrl variants that include pinctrl-mtk-common-v2.h (and not > > > > pinctrl-mtk-common.h) also need to use mtk_eint_pm_ops to setup > > > > wake mask properly, so copy over the pm_ops to v2. > > > > > > > > It is not easy to merge the 2 copies (or move > > > > mtk_eint_suspend/resume to mtk-eint.c), as we need to > > > > dereference pctrl->eint, and struct mtk_pinctrl *pctl has a > > > > different structure definition for v1 and v2. > > > > > > > > Signed-off-by: Nicolas Boichat <drinkcat@chromium.org> > > > > Reviewed-by: Chuanjia Liu <Chuanjia.Liu@mediatek.com> > > > > --- > > > > .../pinctrl/mediatek/pinctrl-mtk-common-v2.c | 19 +++++++++++++++++++ > > > > .../pinctrl/mediatek/pinctrl-mtk-common-v2.h | 1 + > > > > 2 files changed, 20 insertions(+) > > > > > > > > diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > > > > index 20e1c890e73b30c..7e19b5a4748eafe 100644 > > > > --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > > > > +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > > > > @@ -723,3 +723,22 @@ int mtk_pinconf_adv_drive_get(struct mtk_pinctrl *hw, > > > > > > > > return 0; > > > > } > > > > + > > > > +static int mtk_eint_suspend(struct device *device) > > > > +{ > > > > + struct mtk_pinctrl *pctl = dev_get_drvdata(device); > > > > + > > > > + return mtk_eint_do_suspend(pctl->eint); > > > > +} > > > > + > > > > +static int mtk_eint_resume(struct device *device) > > > > +{ > > > > + struct mtk_pinctrl *pctl = dev_get_drvdata(device); > > > > + > > > > + return mtk_eint_do_resume(pctl->eint); > > > > +} > > > > + > > > > +const struct dev_pm_ops mtk_eint_pm_ops = { > > > > + .suspend_noirq = mtk_eint_suspend, > > > > + .resume_noirq = mtk_eint_resume, > > > > +}; > > > > > > This is identical to the one in pinctrl-mtk-common.c and will have name > > > clash if both pinctrl-mtk-common.c and pinctrl-mtk-common-v2.c are > > > built. > > > > > > It would be better if we try to merge both version into mtk-eint.c, this > > > way we could also remove some global functions. > > > > Argh, I didn't think about the name clash, you're right. I guess the > > easy way is to rename this one mtk_eint_pm_ops_v2 ... > > > > As highlighted in the commit message, it's tricky to merge the 2 sets > > of functions, they look identical, but they actually work on struct > > mtk_pinctrl that are defined differently (in > > pinctrl-mtk-common[-v2].h), so the ->eint member is at different > > addresses... > > > > I don't really see a way around this... Unless we want to change > > platform_set_drvdata(pdev, pctl); to pass another type of structure > > that could be shared (but I think that'll make the code fairly > > verbose, with another layer of indirection). Or just assign struct > > mtk_eint to that, since that contains pctl so we could get back the > > struct mtk_pinctrl from that, but that feels ugly as well... > > > > I agree on renaming would make the thing simple. but I wouldn't like > to rename to mtk_eint_pm_ops_v2 since this would make people > misunderstand that is mtk_eint_v2. > > How about renaming to mtk_paris_pinctrl_pm_ops and then place related > logic you added into pinctrl-paris.c? Because I prefer to keep pure > pinctrl hardware operations in pinctrl-mtk-common-v2.c, and for > relevant to other modules (mtk eint) or others subsystem (device tree > binding, GPIO subsytem, PM something like that) they should be moved > to pinctrl-paris.c or pinctrl-moore.c Sounds reasonable. I uploaded a v2 that does just that. Note that we'd still have to duplicate this code between paris and moore, if we wanted to implement pm_ops in moore as well, but maybe that's ok for now. > Sean > > > > > > > Joe.C > > > > > > > > > > > > _______________________________________________ > > > Linux-mediatek mailing list > > > Linux-mediatek@lists.infradead.org > > > http://lists.infradead.org/mailman/listinfo/linux-mediatek ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] pinctrl: mediatek: Add mtk_eint_pm_ops to common-v2 @ 2019-05-08 7:39 ` Nicolas Boichat 0 siblings, 0 replies; 18+ messages in thread From: Nicolas Boichat @ 2019-05-08 7:39 UTC (permalink / raw) To: Sean Wang Cc: Chuanjia Liu, Linus Walleij, lkml, Evan Green, Stephen Boyd, linux-gpio, moderated list:ARM/Mediatek SoC support, Matthias Brugger, Yingjoe Chen, linux-arm Mailing List On Sat, May 4, 2019 at 2:09 AM Sean Wang <sean.wang@kernel.org> wrote: > > Hi, Nicolas > > On Thu, May 2, 2019 at 5:53 PM Nicolas Boichat <drinkcat@chromium.org> wrote: > > > > On Thu, May 2, 2019 at 9:48 PM Yingjoe Chen <yingjoe.chen@mediatek.com> wrote: > > > > > > On Mon, 2019-04-29 at 11:25 +0800, Nicolas Boichat wrote: > > > > pinctrl variants that include pinctrl-mtk-common-v2.h (and not > > > > pinctrl-mtk-common.h) also need to use mtk_eint_pm_ops to setup > > > > wake mask properly, so copy over the pm_ops to v2. > > > > > > > > It is not easy to merge the 2 copies (or move > > > > mtk_eint_suspend/resume to mtk-eint.c), as we need to > > > > dereference pctrl->eint, and struct mtk_pinctrl *pctl has a > > > > different structure definition for v1 and v2. > > > > > > > > Signed-off-by: Nicolas Boichat <drinkcat@chromium.org> > > > > Reviewed-by: Chuanjia Liu <Chuanjia.Liu@mediatek.com> > > > > --- > > > > .../pinctrl/mediatek/pinctrl-mtk-common-v2.c | 19 +++++++++++++++++++ > > > > .../pinctrl/mediatek/pinctrl-mtk-common-v2.h | 1 + > > > > 2 files changed, 20 insertions(+) > > > > > > > > diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > > > > index 20e1c890e73b30c..7e19b5a4748eafe 100644 > > > > --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > > > > +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > > > > @@ -723,3 +723,22 @@ int mtk_pinconf_adv_drive_get(struct mtk_pinctrl *hw, > > > > > > > > return 0; > > > > } > > > > + > > > > +static int mtk_eint_suspend(struct device *device) > > > > +{ > > > > + struct mtk_pinctrl *pctl = dev_get_drvdata(device); > > > > + > > > > + return mtk_eint_do_suspend(pctl->eint); > > > > +} > > > > + > > > > +static int mtk_eint_resume(struct device *device) > > > > +{ > > > > + struct mtk_pinctrl *pctl = dev_get_drvdata(device); > > > > + > > > > + return mtk_eint_do_resume(pctl->eint); > > > > +} > > > > + > > > > +const struct dev_pm_ops mtk_eint_pm_ops = { > > > > + .suspend_noirq = mtk_eint_suspend, > > > > + .resume_noirq = mtk_eint_resume, > > > > +}; > > > > > > This is identical to the one in pinctrl-mtk-common.c and will have name > > > clash if both pinctrl-mtk-common.c and pinctrl-mtk-common-v2.c are > > > built. > > > > > > It would be better if we try to merge both version into mtk-eint.c, this > > > way we could also remove some global functions. > > > > Argh, I didn't think about the name clash, you're right. I guess the > > easy way is to rename this one mtk_eint_pm_ops_v2 ... > > > > As highlighted in the commit message, it's tricky to merge the 2 sets > > of functions, they look identical, but they actually work on struct > > mtk_pinctrl that are defined differently (in > > pinctrl-mtk-common[-v2].h), so the ->eint member is at different > > addresses... > > > > I don't really see a way around this... Unless we want to change > > platform_set_drvdata(pdev, pctl); to pass another type of structure > > that could be shared (but I think that'll make the code fairly > > verbose, with another layer of indirection). Or just assign struct > > mtk_eint to that, since that contains pctl so we could get back the > > struct mtk_pinctrl from that, but that feels ugly as well... > > > > I agree on renaming would make the thing simple. but I wouldn't like > to rename to mtk_eint_pm_ops_v2 since this would make people > misunderstand that is mtk_eint_v2. > > How about renaming to mtk_paris_pinctrl_pm_ops and then place related > logic you added into pinctrl-paris.c? Because I prefer to keep pure > pinctrl hardware operations in pinctrl-mtk-common-v2.c, and for > relevant to other modules (mtk eint) or others subsystem (device tree > binding, GPIO subsytem, PM something like that) they should be moved > to pinctrl-paris.c or pinctrl-moore.c Sounds reasonable. I uploaded a v2 that does just that. Note that we'd still have to duplicate this code between paris and moore, if we wanted to implement pm_ops in moore as well, but maybe that's ok for now. > Sean > > > > > > > Joe.C > > > > > > > > > > > > _______________________________________________ > > > Linux-mediatek mailing list > > > Linux-mediatek@lists.infradead.org > > > http://lists.infradead.org/mailman/listinfo/linux-mediatek _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] pinctrl: mediatek: Add mtk_eint_pm_ops to common-v2 @ 2019-05-08 7:39 ` Nicolas Boichat 0 siblings, 0 replies; 18+ messages in thread From: Nicolas Boichat @ 2019-05-08 7:39 UTC (permalink / raw) To: Sean Wang Cc: Yingjoe Chen, Chuanjia Liu, Linus Walleij, lkml, Evan Green, Stephen Boyd, linux-gpio, moderated list:ARM/Mediatek SoC support, Matthias Brugger, linux-arm Mailing List On Sat, May 4, 2019 at 2:09 AM Sean Wang <sean.wang@kernel.org> wrote: > > Hi, Nicolas > > On Thu, May 2, 2019 at 5:53 PM Nicolas Boichat <drinkcat@chromium.org> wrote: > > > > On Thu, May 2, 2019 at 9:48 PM Yingjoe Chen <yingjoe.chen@mediatek.com> wrote: > > > > > > On Mon, 2019-04-29 at 11:25 +0800, Nicolas Boichat wrote: > > > > pinctrl variants that include pinctrl-mtk-common-v2.h (and not > > > > pinctrl-mtk-common.h) also need to use mtk_eint_pm_ops to setup > > > > wake mask properly, so copy over the pm_ops to v2. > > > > > > > > It is not easy to merge the 2 copies (or move > > > > mtk_eint_suspend/resume to mtk-eint.c), as we need to > > > > dereference pctrl->eint, and struct mtk_pinctrl *pctl has a > > > > different structure definition for v1 and v2. > > > > > > > > Signed-off-by: Nicolas Boichat <drinkcat@chromium.org> > > > > Reviewed-by: Chuanjia Liu <Chuanjia.Liu@mediatek.com> > > > > --- > > > > .../pinctrl/mediatek/pinctrl-mtk-common-v2.c | 19 +++++++++++++++++++ > > > > .../pinctrl/mediatek/pinctrl-mtk-common-v2.h | 1 + > > > > 2 files changed, 20 insertions(+) > > > > > > > > diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > > > > index 20e1c890e73b30c..7e19b5a4748eafe 100644 > > > > --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > > > > +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > > > > @@ -723,3 +723,22 @@ int mtk_pinconf_adv_drive_get(struct mtk_pinctrl *hw, > > > > > > > > return 0; > > > > } > > > > + > > > > +static int mtk_eint_suspend(struct device *device) > > > > +{ > > > > + struct mtk_pinctrl *pctl = dev_get_drvdata(device); > > > > + > > > > + return mtk_eint_do_suspend(pctl->eint); > > > > +} > > > > + > > > > +static int mtk_eint_resume(struct device *device) > > > > +{ > > > > + struct mtk_pinctrl *pctl = dev_get_drvdata(device); > > > > + > > > > + return mtk_eint_do_resume(pctl->eint); > > > > +} > > > > + > > > > +const struct dev_pm_ops mtk_eint_pm_ops = { > > > > + .suspend_noirq = mtk_eint_suspend, > > > > + .resume_noirq = mtk_eint_resume, > > > > +}; > > > > > > This is identical to the one in pinctrl-mtk-common.c and will have name > > > clash if both pinctrl-mtk-common.c and pinctrl-mtk-common-v2.c are > > > built. > > > > > > It would be better if we try to merge both version into mtk-eint.c, this > > > way we could also remove some global functions. > > > > Argh, I didn't think about the name clash, you're right. I guess the > > easy way is to rename this one mtk_eint_pm_ops_v2 ... > > > > As highlighted in the commit message, it's tricky to merge the 2 sets > > of functions, they look identical, but they actually work on struct > > mtk_pinctrl that are defined differently (in > > pinctrl-mtk-common[-v2].h), so the ->eint member is at different > > addresses... > > > > I don't really see a way around this... Unless we want to change > > platform_set_drvdata(pdev, pctl); to pass another type of structure > > that could be shared (but I think that'll make the code fairly > > verbose, with another layer of indirection). Or just assign struct > > mtk_eint to that, since that contains pctl so we could get back the > > struct mtk_pinctrl from that, but that feels ugly as well... > > > > I agree on renaming would make the thing simple. but I wouldn't like > to rename to mtk_eint_pm_ops_v2 since this would make people > misunderstand that is mtk_eint_v2. > > How about renaming to mtk_paris_pinctrl_pm_ops and then place related > logic you added into pinctrl-paris.c? Because I prefer to keep pure > pinctrl hardware operations in pinctrl-mtk-common-v2.c, and for > relevant to other modules (mtk eint) or others subsystem (device tree > binding, GPIO subsytem, PM something like that) they should be moved > to pinctrl-paris.c or pinctrl-moore.c Sounds reasonable. I uploaded a v2 that does just that. Note that we'd still have to duplicate this code between paris and moore, if we wanted to implement pm_ops in moore as well, but maybe that's ok for now. > Sean > > > > > > > Joe.C > > > > > > > > > > > > _______________________________________________ > > > Linux-mediatek mailing list > > > Linux-mediatek@lists.infradead.org > > > http://lists.infradead.org/mailman/listinfo/linux-mediatek ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 2/2] pinctrl: mediatek: mt8183: Add mtk_eint_pm_ops 2019-04-29 3:25 ` Nicolas Boichat @ 2019-04-29 3:25 ` Nicolas Boichat -1 siblings, 0 replies; 18+ messages in thread From: Nicolas Boichat @ 2019-04-29 3:25 UTC (permalink / raw) To: linux-mediatek Cc: Sean Wang, Linus Walleij, Matthias Brugger, linux-gpio, linux-arm-kernel, linux-kernel, Chuanjia Liu, evgreen, swboyd Setting this up will configure wake from suspend properly, and wake only for the interrupts that are setup in wake_mask, not all interrupts. Signed-off-by: Nicolas Boichat <drinkcat@chromium.org> Reviewed-by: Chuanjia Liu <Chuanjia.Liu@mediatek.com> --- drivers/pinctrl/mediatek/pinctrl-mt8183.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/pinctrl/mediatek/pinctrl-mt8183.c b/drivers/pinctrl/mediatek/pinctrl-mt8183.c index 2c7409ed16fae9c..ce93e55b79a435a 100644 --- a/drivers/pinctrl/mediatek/pinctrl-mt8183.c +++ b/drivers/pinctrl/mediatek/pinctrl-mt8183.c @@ -583,6 +583,7 @@ static struct platform_driver mt8183_pinctrl_driver = { .driver = { .name = "mt8183-pinctrl", .of_match_table = mt8183_pinctrl_of_match, + .pm = &mtk_eint_pm_ops, }, .probe = mt8183_pinctrl_probe, }; -- 2.21.0.593.g511ec345e18-goog ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/2] pinctrl: mediatek: mt8183: Add mtk_eint_pm_ops @ 2019-04-29 3:25 ` Nicolas Boichat 0 siblings, 0 replies; 18+ messages in thread From: Nicolas Boichat @ 2019-04-29 3:25 UTC (permalink / raw) To: linux-mediatek Cc: Chuanjia Liu, Linus Walleij, Sean Wang, linux-kernel, evgreen, swboyd, linux-gpio, Matthias Brugger, linux-arm-kernel Setting this up will configure wake from suspend properly, and wake only for the interrupts that are setup in wake_mask, not all interrupts. Signed-off-by: Nicolas Boichat <drinkcat@chromium.org> Reviewed-by: Chuanjia Liu <Chuanjia.Liu@mediatek.com> --- drivers/pinctrl/mediatek/pinctrl-mt8183.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/pinctrl/mediatek/pinctrl-mt8183.c b/drivers/pinctrl/mediatek/pinctrl-mt8183.c index 2c7409ed16fae9c..ce93e55b79a435a 100644 --- a/drivers/pinctrl/mediatek/pinctrl-mt8183.c +++ b/drivers/pinctrl/mediatek/pinctrl-mt8183.c @@ -583,6 +583,7 @@ static struct platform_driver mt8183_pinctrl_driver = { .driver = { .name = "mt8183-pinctrl", .of_match_table = mt8183_pinctrl_of_match, + .pm = &mtk_eint_pm_ops, }, .probe = mt8183_pinctrl_probe, }; -- 2.21.0.593.g511ec345e18-goog _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 18+ messages in thread
end of thread, other threads:[~2019-05-08 7:40 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-04-29 3:25 [PATCH 0/2] pinctrl: mediatek: mt8183: Add support for wake sources Nicolas Boichat 2019-04-29 3:25 ` Nicolas Boichat 2019-04-29 3:25 ` [PATCH 1/2] pinctrl: mediatek: Add mtk_eint_pm_ops to common-v2 Nicolas Boichat 2019-04-29 3:25 ` Nicolas Boichat 2019-05-02 13:48 ` Yingjoe Chen 2019-05-02 13:48 ` Yingjoe Chen 2019-05-02 13:48 ` Yingjoe Chen 2019-05-03 0:52 ` Nicolas Boichat 2019-05-03 0:52 ` Nicolas Boichat 2019-05-03 0:52 ` Nicolas Boichat 2019-05-03 17:08 ` Sean Wang 2019-05-03 17:08 ` Sean Wang 2019-05-03 17:08 ` Sean Wang 2019-05-08 7:39 ` Nicolas Boichat 2019-05-08 7:39 ` Nicolas Boichat 2019-05-08 7:39 ` Nicolas Boichat 2019-04-29 3:25 ` [PATCH 2/2] pinctrl: mediatek: mt8183: Add mtk_eint_pm_ops Nicolas Boichat 2019-04-29 3:25 ` Nicolas Boichat
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.