RE: [EXT] Re: [PATCH v2 1/1] gpio: mxc: add suspend/resume support for i.mx8x SoCs

[Shenwei Wang <shenwei.wang@xxxxxxx>]

> -----Original Message-----
> From: Linus Walleij <linus.walleij@xxxxxxxxxx>
> Sent: Monday, October 17, 2022 4:39 AM
> To: Shenwei Wang <shenwei.wang@xxxxxxx>; Aisheng Dong
> <aisheng.dong@xxxxxxx>; Fabio Estevam <festevam@xxxxxxxxx>; Shawn Guo
> <shawnguo@xxxxxxxxxx>; Jacky Bai <ping.bai@xxxxxxx>
> Cc: Bartosz Golaszewski <brgl@xxxxxxxx>; linux-gpio@xxxxxxxxxxxxxxx;
> imx@xxxxxxxxxxxxxxx; Pengutronix Kernel Team <kernel@xxxxxxxxxxxxxx>
> Subject: [EXT] Re: [PATCH v2 1/1] gpio: mxc: add suspend/resume support for
> i.mx8x SoCs
> 
> Caution: EXT Email
> 
> On Fri, Oct 7, 2022 at 5:29 PM Shenwei Wang <shenwei.wang@xxxxxxx> wrote:
> 
> > On i.MX8QM/QXP/DXL SoCs, even a GPIO is selected as the wakeup source,
> > the GPIO block will be powered off when system enters into suspend
> > state. This can greatly reduce the power consumption of suspend state
> > because the whole partition can be shutdown. This is called PAD wakeup
> > feature on i.MX8x platform.
> >
> > This patch enables this wakeup feature on i.MX8QM/QXP/DXL platforms.
> >
> > Signed-off-by: Shenwei Wang <shenwei.wang@xxxxxxx>
> > ---
> >  Changes in v2:
> >  - fix a bug when finding the pad index
> 
> This:
> 
> > +#define IMX_SC_PAD_FUNC_GET_WAKEUP     9
> > +#define IMX_SC_PAD_FUNC_SET_WAKEUP     4
> > +#define IMX_SC_IRQ_GROUP_WAKE           3   /* Wakeup interrupts */
> > +#define IMX_SC_IRQ_PAD                 2    /* Pad wakeup */
> > +
> > +#define IMX_SC_PAD_WAKEUP_OFF          0
> > +#define IMX_SC_PAD_WAKEUP_LOW_LVL      4
> > +#define IMX_SC_PAD_WAKEUP_FALL_EDGE    5
> > +#define IMX_SC_PAD_WAKEUP_RISE_EDGE    6
> > +#define IMX_SC_PAD_WAKEUP_HIGH_LVL     7
> (...)
> 
> > +const struct mxc_gpio_pad_map imx8qm_pad_map[] = {
> > +       /* GPIO0 */
> > +       {  0, IMX8QM_SIM0_CLK},
> > +       {  1, IMX8QM_SIM0_RST},
> > +       {  2, IMX8QM_SIM0_IO},
> > +       {  3, IMX8QM_SIM0_PD},
> 
> this is pin control.
> 
> I think you need to think about adding this to the i.MX pin control driver instead,
> possibly cooperaring with the GPIO driver.
> 
> I'm not entirely certain about the relationship between MXC and i.MX so correct
> me if I get this wrong...

This is not pin control. The wakeup function doesn't change any PINCtrl settings.  On i.MX8
platform, there is an extra pad wakeup logic and each pin can be configured as the wakeup source 
during system suspend. Because GPIO module is powered off on i.MX8 platform in suspend 
state, the GPIO's native wakeup won't work anymore. This patch just maps a GPIO wakeup to the 
corresponding PAD wakeup, and this mapping doesn't impact or change any PINCtrl settings.
Also the pad wakeup feature here has a great power consumption advantage comparing with the 
GPIO module's native wakeup.

> 
> To me this looks like two drivers could end up fighting about the control of the
> same hardware if you don't.
> 

It won't. This patch doesn't interact with any pinctrl relating registers at all.

> Also CC i.MX Freescale pin control maintainers on all patches like this, thanks!
> (Added on to-line.)
> 

Will add them in the future.

Regards,
Shenwei

> Yours,
> Linus Walleij