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

[Basavaraj Natikar <bnatikar@xxxxxxx>]

On 12/9/2022 3:51 AM, Linus Walleij wrote:
> 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.

Sure

>
> 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?

we are not going to manipulate using pinconf_to_config_param
explicitly, setting will be taken care by BIOS. We need only
for debugging purpose to check bit status

Thanks,
--
Basavaraj