drivers: gpio: designware: add alif pinctrl support#338
drivers: gpio: designware: add alif pinctrl support#338timKnodel wants to merge 1 commit intoalifsemi:mainfrom
Conversation
|
Hi @RupeshKumar-AlifSemi, can you please review this improvement? |
There was a problem hiding this comment.
Pull request overview
This PR enables dynamic GPIO pull-up/pull-down control in the DesignWare GPIO driver by integrating Alif pinctrl support. The changes refactor pinctrl definitions to be shared between drivers.
Changes:
- Moved pinctrl macros and definitions from
pinctrl_alif.ctopinctrl_soc.hheader for reusability - Added pull-up/pull-down configuration support in the DesignWare GPIO driver when
CONFIG_PINCTRL_ALIFis enabled - Introduced DSC (Driver State Control) constants for different pull modes
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| soc/alif/common/pinctrl_soc.h | Added LPGPIO documentation, DSC constants, and relocated shared pinctrl macros from driver |
| drivers/pinctrl/pinctrl_alif.c | Removed duplicate definitions now in header, added reference to pinctrl_soc.h |
| drivers/gpio/gpio_dw.c | Added conditional pinctrl integration for dynamic pull-up/pull-down configuration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
b2e9d0e to
bb95a19
Compare
|
CI fails because of:
|
To dynamically control gpio pull-up/pull-down through the gpio API the designware driver needs to call the alif pinctrl if CONFIG_PINCTRL_ALIF is enabled. Signed-off-by: Tim Knodel <tbk@google.com>
bb95a19 to
a7a5053
Compare
|
Formatting issues fixed. |
| * 0x1000 away from the base address in port order. | ||
| */ | ||
| #define GPIO_PORT_FROM_ADDRESS(x) \ | ||
| (x < GPIO_PORT_BASE_ADDRESS ? 15U : (x - GPIO_PORT_BASE_ADDRESS) / 0x1000) |
There was a problem hiding this comment.
Would not this fail for GPIO16 (0x4300_A000) and GPIO 17 (0x4300_B000) ?
| } | ||
|
|
||
| #if CONFIG_PINCTRL_ALIF | ||
| uint32_t port_pin_value = (GPIO_PORT_FROM_ADDRESS(context->base_addr) << 3U) | (pin & 0x7U); |
There was a problem hiding this comment.
| uint32_t port_pin_value = (GPIO_PORT_FROM_ADDRESS(context->base_addr) << 3U) | (pin & 0x7U); | |
| uint32_t port_pin_value = ((GPIO_PORT_FROM_ADDRESS(context->base_addr) << 3U) | (pin & 0x7U)) << PIN_FUNC_SHIFT; |
| uint32_t *pinctrl_addr; | ||
|
|
||
| if (LPGPIO_PINCTRL_BASE && (GET_PINMUX_PORT(port_pin_value) == LPGPIO_PORT)) { | ||
| pinctrl_addr = (uint32_t *)LPGPIO_PINMUX_ADDR(port_pin_value); |
There was a problem hiding this comment.
With the above suggested change (to shift port_pin_value by PIN_FUNC_SHIFT), we might break this calculation of pinctrl_addr. So this will need to be handled separately.
To dynamically control gpio pull-up/pull-down through the gpio API the designware driver needs to call the Alif pinctrl if CONFIG_PINCTRL_ALIF is enabled. This moves some of the helper functions from the c file to the header file to simplify the driver.