Re: [PATCH] pinctrl: amd: Add Z-state wake control bits

[Linus Walleij <linus.walleij@xxxxxxxxxx>]

Hi Basavaraj,

thanks for your patch!

On Thu, Dec 8, 2022 at 10:37 AM Basavaraj Natikar
<Basavaraj.Natikar@xxxxxxx> wrote:

> GPIO registers include Bit 27 for WakeCntrlZ used to enable wake in
> Z state. Hence add Z-state wake control bits to debugfs output to
> debug and analyze Z-states problems.
>
> Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@xxxxxxx>
> Suggested-by: Mario Limonciello <mario.limonciello@xxxxxxx>
> Tested-by: Guruvendra Punugupati <Guruvendra.Punugupati@xxxxxxx>

The patch is overall fine, but as debug-only patch hardly urgent
so it will wait until kernel v6.3.

What I want to ask is how this bit:

+#define WAKECNTRL_Z_OFF                        27

Relates to this:

static int amd_gpio_irq_set_wake(struct irq_data *d, unsigned int on)
{
        u32 pin_reg;
        unsigned long flags;
        struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
        struct amd_gpio *gpio_dev = gpiochip_get_data(gc);
        u32 wake_mask = BIT(WAKE_CNTRL_OFF_S0I3) | BIT(WAKE_CNTRL_OFF_S3);
        int err;
(...)

if this is some wake up control that is unrelated to GPIO, such
that this can wake up on say I2C traffic on that pin or similar,
is this something we actually need a new define for in
include/linux/pinctrl/pinconf-generic.h
so that you can also implement methods to manipulate it
for real, like setting this from a pin control state or so?

Yours,
Linus Walleij