Skip to content

Conversation

akanisetti
Copy link
Contributor

This PR adds GPIO support for Intel Panther Lake H SoC, implementing a new ACPI GINF method for GPIO resource enumeration and adding corresponding device tree definitions.

Adds GPIO device tree definitions for Panther Lake H with new ACPI GINF method support
Implements enhanced ACPI GPIO resource enumeration with support for both old and new GINF methods
Adds GPIO configuration overlay for test board intel_ptl_h_crb

@akanisetti akanisetti force-pushed the gpio_ptlh_main branch 4 times, most recently from 684d7fc to 8812842 Compare September 15, 2025 06:10
@akanisetti akanisetti marked this pull request as ready for review September 15, 2025 06:57
@zephyrbot zephyrbot added platform: Intel Intel Corporation area: ACPI ACPI support platform: X86 x86 and x86-64 area: GPIO labels Sep 15, 2025
as it is observed that the offset varies from
platform to platform.

config GPIO_INTEL_INT_STAT_OFFSET
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if these two actually belong to devicetree, as they are per platform (so they would be added to dts/bindings/gpio/intel,gpio.yaml).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, adding it as a dts parameter is more suitable but the value is common for all the GPIO instances of a platform. That's why, it is added as a CONFIG to be updated once for a platform instead of adding same value for all the instances in dts file.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmmm... not sure how this is usually (supposed to be) handled. @henrikbrixandersen thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

Typically, we would describe that in the devicetree instance, but there's no good way of setting the same value across a number of devicetree nodes. Only thing I can think of is having a parent node with this property, but that would be kind of superficial.

Is this GPIO controller supported in Linux? if yes, how does the Linux kernel handle/provide this offset?

Copy link
Contributor Author

@akanisetti akanisetti Sep 29, 2025

Choose a reason for hiding this comment

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

This GPIO controller is supported in Linux, a different ACPI method is used to fetch this offset directly instead of deriving from another offset like what we are doing hereLinux Ref. That ACPI method is not enabled in zephyr.

type: int
description: Pin offset of this GPIO entry

acpi-ginf:
Copy link
Contributor

Choose a reason for hiding this comment

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

Not familiar with ACPI at all, but it seems weird that a boolean describes a version. Maybe rename it to acpi-ginf-3-param or so?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will update it to acpi-ginf-3-param

acpi_child = acpi_device_get(hid, uid);
if (!acpi_child) {
printk("acpi_device_get failed\n");
LOG_ERR("acpi_device_get failed");
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: maybe have these s/printk/LOG_ERR on a different patch, so it doesn't distract from the functional changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Shall I add this change in a different comment in this PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

That would be ideal, yes (assuming you meant "commit").

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, commit

@edersondisouza edersondisouza self-assigned this Oct 6, 2025
@edersondisouza
Copy link
Contributor

@akanisetti will you update this PR?

@akanisetti
Copy link
Contributor Author

akanisetti commented Oct 6, 2025

@akanisetti will you update this PR?

Hi @edersondisouza, I am waiting for @henrikbrixandersen's response regarding #95920 (comment).
If the approach is finalized, I could add all changes at a time, instead of adding changes one by one.

@henrikbrixandersen
Copy link
Member

@akanisetti will you update this PR?

Hi @edersondisouza, I am waiting for @henrikbrixandersen's response regarding #95920 (comment). If the approach is finalized, I could add all changes at a time, instead of adding changes one by one.

I wasn't aware you were waiting for a response from me, my apologies. I don't think I have a good solution on how to obtain this offset in Zephyr.

@akanisetti
Copy link
Contributor Author

akanisetti commented Oct 6, 2025

@akanisetti will you update this PR?

Hi @edersondisouza, I am waiting for @henrikbrixandersen's response regarding #95920 (comment). If the approach is finalized, I could add all changes at a time, instead of adding changes one by one.

I wasn't aware you were waiting for a response from me, my apologies. I don't think I have a good solution on how to obtain this offset in Zephyr.

No worries @henrikbrixandersen.
If the approach of using Kconfigs to update the offset is not appropriate, I will take the other approach of adding it as a dts parameter and add it in all the instances. Just that same value has to be added for all GPIO instances of a board.
Please confirm if this (dts-parameter) approach is okay if not perfect for this scenario @henrikbrixandersen @edersondisouza.

@henrikbrixandersen
Copy link
Member

If the approach of using Kconfigs to update the offset is not appropriate, I will take the other approach of adding it as a dts parameter and add it in all the instances. Just that same value has to be added for all GPIO instances of a board.
Please confirm if this approach is okay if not perfect for this scenario @henrikbrixandersen @edersondisouza.

I would prefer a devicetree-based solution for specifying the offset.

@akanisetti
Copy link
Contributor Author

If the approach of using Kconfigs to update the offset is not appropriate, I will take the other approach of adding it as a dts parameter and add it in all the instances. Just that same value has to be added for all GPIO instances of a board.
Please confirm if this approach is okay if not perfect for this scenario @henrikbrixandersen @edersondisouza.

I would prefer a devicetree-based solution for specifying the offset.

Thank you @henrikbrixandersen,
@edersondisouza I will update the PR with all the changes within 24hours.

Enable support for ACPI_RESOURCE_TYPE_ADDRESS64 in acpi_device_mmio_get
to fetch 64bit address from resource of an ACPI device.

Signed-off-by: Anisetti Avinash Krishna <[email protected]>
@zephyrbot zephyrbot added area: Tests Issues related to a particular existing or missing test area: Devicetree Bindings area: Boards/SoCs labels Oct 7, 2025
Enable support for latest GINF method which requires 3 paramters
for each GPIO group and enables gpio support for intel_ptl_h
platform.

Signed-off-by: Anisetti Avinash Krishna <[email protected]>
Replace printk with LOG_ERR by adding a log module.

Signed-off-by: Anisetti Avinash Krishna <[email protected]>
Copy link

sonarqubecloud bot commented Oct 7, 2025

@cfriedt cfriedt merged commit ea1a839 into zephyrproject-rtos:main Oct 8, 2025
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: ACPI ACPI support area: Boards/SoCs area: Devicetree Bindings area: GPIO area: Tests Issues related to a particular existing or missing test platform: Intel Intel Corporation platform: X86 x86 and x86-64
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants