On Fri, 5 Nov 2021 15:12:16 +0100 Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
On 10/21/21 06:52, Samuel Holland wrote:
The GPIO and pinctrl drivers need these setters for pin configuration. Since they are DM drivers, they should not be using hardcoded base addresses. Factor out variants of the setter functions which take a pointer to the GPIO bank's MMIO registers.
Signed-off-by: Samuel Holland samuel@sholland.org
arch/arm/include/asm/arch-sunxi/gpio.h | 2 ++ arch/arm/mach-sunxi/pinmux.c | 20 ++++++++++++++++---- 2 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/arch/arm/include/asm/arch-sunxi/gpio.h b/arch/arm/include/asm/arch-sunxi/gpio.h index 2b72b2263b..106605adf5 100644 --- a/arch/arm/include/asm/arch-sunxi/gpio.h +++ b/arch/arm/include/asm/arch-sunxi/gpio.h @@ -227,7 +227,9 @@ void sunxi_gpio_set_cfgpin(u32 pin, u32 val); int sunxi_gpio_get_cfgbank(struct sunxi_gpio *pio, int bank_offset); int sunxi_gpio_get_cfgpin(u32 pin); void sunxi_gpio_set_drv(u32 pin, u32 val);
Please, add Sphinx style documentation for the new functions, preferably in the header file. Cf. https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#function-do...
I don't think this is necessary or useful, really. Those function are not really an advertised interface, more something we use internally, mostly for our SPL.
+void sunxi_gpio_set_drv_bank(struct sunxi_gpio *pio, u32 bank_offset, u32 val); void sunxi_gpio_set_pull(u32 pin, u32 val); +void sunxi_gpio_set_pull_bank(struct sunxi_gpio *pio, int bank_offset, u32 val); int sunxi_name_to_gpio(const char *name);
#if !defined CONFIG_SPL_BUILD && defined CONFIG_AXP_GPIO diff --git a/arch/arm/mach-sunxi/pinmux.c b/arch/arm/mach-sunxi/pinmux.c index cf9d9daf7c..b2093b623a 100644 --- a/arch/arm/mach-sunxi/pinmux.c +++ b/arch/arm/mach-sunxi/pinmux.c @@ -48,19 +48,31 @@ int sunxi_gpio_get_cfgpin(u32 pin) void sunxi_gpio_set_drv(u32 pin, u32 val) { u32 bank = GPIO_BANK(pin);
- u32 index = GPIO_DRV_INDEX(pin);
- u32 offset = GPIO_DRV_OFFSET(pin); struct sunxi_gpio *pio = BANK_TO_GPIO(bank);
- sunxi_gpio_set_drv_bank(pio, pin, val);
+}
+void sunxi_gpio_set_drv_bank(struct sunxi_gpio *pio, u32 bank_offset, u32 val) +{
u32 index = GPIO_DRV_INDEX(bank_offset);
u32 offset = GPIO_DRV_OFFSET(bank_offset);
clrsetbits_le32(&pio->drv[0] + index, 0x3 << offset, val << offset); }
void sunxi_gpio_set_pull(u32 pin, u32 val) { u32 bank = GPIO_BANK(pin);
- u32 index = GPIO_PULL_INDEX(pin);
- u32 offset = GPIO_PULL_OFFSET(pin); struct sunxi_gpio *pio = BANK_TO_GPIO(bank);
- sunxi_gpio_set_pull_bank(pio, pin, val);
+}
+void sunxi_gpio_set_pull_bank(struct sunxi_gpio *pio, int bank_offset, u32 val) +{
- u32 index = GPIO_PULL_INDEX(bank_offset);
- u32 offset = GPIO_PULL_OFFSET(bank_offset);
- clrsetbits_le32(&pio->pull[0] + index, 0x3 << offset, val << offset);
Please, simplify this: %s/&pio->pull[0] + index/&pio->pull[index]/
Fixing this up locally.
Cheers, Andre
Otherwise the change looks correct to me.
Best regards
Heinrich
}