Skip to content

Conversation

JasonLin-RealTek
Copy link
Contributor

  1. add cs pin as espi driver wake up reference
  2. removed the unnecessary update of the cached date
  3. removed the unnecessary busy wait in notify funciton

#include <zephyr/pm/device.h>
#include <zephyr/pm/policy.h>
#include "reg/reg_system.h"
#define RTS5912_SCCON_REG_BASE ((SYSTEM_Type *)(DT_REG_ADDR(DT_NODELABEL(sccon))))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this macro already defined by reg/reg_system.h?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

Comment on lines 31 to 32
#define ESPI_RTS5912_ESPI_CS_PORT DEVICE_DT_GET(DT_NODELABEL(gpioa))
#define ESPI_RTS5912_ESPI_CS_PIN 4
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a property to the realtek,rts5912-espi node to get the wakeup GPIO information instead of hardcoding.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use gpios = <&gpioa 4 0>; in device tree to get CS Pin Information, thanks.

@JasonLin-RealTek JasonLin-RealTek force-pushed the implement_espi_power_consumption branch 2 times, most recently from 419e686 to 12ad3ef Compare August 21, 2025 02:06
gpio_flags_t cs_pin_config;

gpio_pin_get_config(port, pins, &cs_pin_config);
if (cs_pin_config & GPIO_GCR_INTEN_Msk) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GPIO_GCR_INTEN_Msk is an internal bit for the GPIO controller. You should check standard flags defined by gpio.h and gpio_defines.h

Suggested change
if (cs_pin_config & GPIO_GCR_INTEN_Msk) {
if (cs_pin_config & GPIO_INT_ENABLE) {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, fixed.

pinctrl-names:
required: true

gpios:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Name this cs-gpios. You should also add some help text to describe this property.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed and add the description in yaml.

0x400a0100 0x1c /* KBC */
0x400B1600 0xd0>; /* MBX */
0x400B1600 0xd0 /* MBX */
0x40090010 0x04>; /* CS pin */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cs_low reg entry has been replaced by the gpios property, so this is no longer used.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed it now.
0x40090010 0x04>; /* CS pin */

@JasonLin-RealTek JasonLin-RealTek force-pushed the implement_espi_power_consumption branch from 12ad3ef to 9c30eba Compare September 5, 2025 09:59
"emi7", "oob_tx", "oob_rx",
"oob_chg", "maf_tr", "flash_chg",
"port80", "mbx";
"port80", "mbx", "cs_low";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the cs_low interrupt is also unused. Please delete from both interrupt-names and from interrupts above.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, thanks.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cs_low is still listed in the interrupt names, and interrupt <4 0> is still listed in interrupt properties.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, delete them now.

@JasonLin-RealTek JasonLin-RealTek force-pushed the implement_espi_power_consumption branch from 9c30eba to d56a0c9 Compare September 12, 2025 09:33
@JasonLin-RealTek JasonLin-RealTek force-pushed the implement_espi_power_consumption branch from d56a0c9 to da5a6ee Compare September 22, 2025 12:23
keith-zephyr
keith-zephyr previously approved these changes Sep 22, 2025
fabiobaltieri
fabiobaltieri previously approved these changes Sep 26, 2025
@JasonLin-RealTek JasonLin-RealTek force-pushed the implement_espi_power_consumption branch from da5a6ee to 8c0a1cf Compare September 26, 2025 12:53
@fabiobaltieri
Copy link
Member

@JasonLin-RealTek hey give this a rebase (on current main), there was a transient CI issue.

@JasonLin-RealTek
Copy link
Contributor Author

@JasonLin-RealTek hey give this a rebase (on current main), there was a transient CI issue.

Hi @fabiobaltieri , fixed, thanks.

fabiobaltieri
fabiobaltieri previously approved these changes Oct 3, 2025

cs-gpios:
description: |
select the cs pin to support the espi wakeup
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a note that this property is required if CONFIG_PM is enabled.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a note now, thanks.

1. add cs pin as espi driver wake up reference
2. removed the unnecessary update of the cached date
3. removed the unnecessary busy wait in notify funciton

Signed-off-by: Lin Yu-Cheng <[email protected]>
Copy link

sonarqubecloud bot commented Oct 8, 2025

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants