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

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

[Basavaraj Natikar]

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@amd.com>
Suggested-by: Mario Limonciello <mario.limonciello@amd.com>
Tested-by: Guruvendra Punugupati <Guruvendra.Punugupati@amd.com>
---
 drivers/pinctrl/pinctrl-amd.c | 7 +++++++
 drivers/pinctrl/pinctrl-amd.h | 1 +
 2 files changed, 8 insertions(+)

diff --git a/drivers/pinctrl/pinctrl-amd.c b/drivers/pinctrl/pinctrl-amd.c
index 6be896871718..e944003bb84a 100644
--- a/drivers/pinctrl/pinctrl-amd.c
+++ b/drivers/pinctrl/pinctrl-amd.c
@@ -218,6 +218,7 @@ static void amd_gpio_dbg_show(struct seq_file *s, struct gpio_chip *gc)
 	char *orientation;
 	char debounce_value[40];
 	char *debounce_enable;
+	char *wake_cntrlz;
 
 	for (bank = 0; bank < gpio_dev->hwbank_num; bank++) {
 		unsigned int time = 0;
@@ -305,6 +306,12 @@ static void amd_gpio_dbg_show(struct seq_file *s, struct gpio_chip *gc)
 				wake_cntrl2 = " ∅";
 			seq_printf(s, "S4/S5 %s| ", wake_cntrl2);
 
+			if (pin_reg & BIT(WAKECNTRL_Z_OFF))
+				wake_cntrlz = "⏰";
+			else
+				wake_cntrlz = " ∅";
+			seq_printf(s, "Z %s| ", wake_cntrlz);
+
 			if (pin_reg & BIT(PULL_UP_ENABLE_OFF)) {
 				pull_up_enable = "+";
 				if (pin_reg & BIT(PULL_UP_SEL_OFF))
diff --git a/drivers/pinctrl/pinctrl-amd.h b/drivers/pinctrl/pinctrl-amd.h
index c8635998465d..81ae8319a1f0 100644
--- a/drivers/pinctrl/pinctrl-amd.h
+++ b/drivers/pinctrl/pinctrl-amd.h
@@ -42,6 +42,7 @@
 #define OUTPUT_ENABLE_OFF		23
 #define SW_CNTRL_IN_OFF			24
 #define SW_CNTRL_EN_OFF			25
+#define WAKECNTRL_Z_OFF			27
 #define INTERRUPT_STS_OFF		28
 #define WAKE_STS_OFF			29
 
-- 
2.25.1

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

[Linus Walleij]

Hi Basavaraj,

thanks for your patch!

On Thu, Dec 8, 2022 at 10:37 AM Basavaraj Natikar
<Basavaraj.Natikar@amd.com> 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@amd.com>
> Suggested-by: Mario Limonciello <mario.limonciello@amd.com>
> Tested-by: Guruvendra Punugupati <Guruvendra.Punugupati@amd.com>

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

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

[Basavaraj Natikar]

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@amd.com> 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@amd.com>
>> Suggested-by: Mario Limonciello <mario.limonciello@amd.com>
>> Tested-by: Guruvendra Punugupati <Guruvendra.Punugupati@amd.com>
> 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

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

[Linus Walleij]

On Thu, Dec 8, 2022 at 10:37 AM Basavaraj Natikar
<Basavaraj.Natikar@amd.com> 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@amd.com>
> Suggested-by: Mario Limonciello <mario.limonciello@amd.com>
> Tested-by: Guruvendra Punugupati <Guruvendra.Punugupati@amd.com>

Patch applied!

Yours,
Linus Walleij