Re: [PATCH 09/24] gpio: Add ZTE ZX279128S GPIO driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] gpio: mmio: 16-bit I/O access to 32-bit aligned registers may cause bus faults
--

commit 827bf2313ced7a306a6c929c27e8898eb758b78c
Author: Navid Ghahremani <ghahramani.navid@gmail.com>

gpio: Add ZTE ZX279128S GPIO driver
    
Expose the six banks using the generic GPIO implementation and enable
the register clock before accessing the bank.

> diff --git a/drivers/gpio/gpio-zx279128s.c b/drivers/gpio/gpio-zx279128s.c
> new file mode 100644
> index 0000000000000..8f37b908bb007
> --- /dev/null
> +++ b/drivers/gpio/gpio-zx279128s.c
[ ... ]
> +/*
> + * Each bank has 16 lines, one bit per line in each register. The registers
> + * at 0x04-0x10 and 0x28-0x34 control the interrupts, which are not supported
> + * yet.
> + */
> +#define ZX279128S_GPIO_DIR	0x00	/* 1: output */
> +#define ZX279128S_GPIO_DATA	0x14	/* line level */
> +#define ZX279128S_GPIO_SET	0x18	/* write 1: drive high */
> +#define ZX279128S_GPIO_CLEAR	0x1c	/* write 1: drive low */
> +
> +static int zx279128s_gpio_probe(struct platform_device *pdev)
> +{
> +	struct gpio_generic_chip_config config = { };
[ ... ]
> +	clk = devm_clk_get_enabled(dev, NULL);
> +	if (IS_ERR(clk))
> +		return dev_err_probe(dev, PTR_ERR(clk), "failed to get the clock\n");
> +
> +	config.dev = dev;
> +	config.sz = 2;

[Severity: High]
Does configuring the GPIO framework for 16-bit I/O access on 32-bit aligned
hardware registers risk triggering bus faults?

The register offsets defined above (0x14, 0x18, 0x1c) are 4-byte aligned,
which indicates they are likely 32-bit hardware registers. Setting config.sz
to 2 causes the generic framework to use 16-bit memory accessors, and
performing 16-bit accesses to 32-bit peripheral registers on ARM APB/AHB
buses typically causes bus faults.

Would it be safer to set config.sz = 4 for safe 32-bit accesses, and
explicitly restrict the line count by setting chip->gc.ngpio = 16 before
calling gpio_generic_chip_init()?

> +	config.dat = base + ZX279128S_GPIO_DATA;
> +	config.set = base + ZX279128S_GPIO_SET;
> +	config.clr = base + ZX279128S_GPIO_CLEAR;
> +	config.dirout = base + ZX279128S_GPIO_DIR;
> +
> +	ret = gpio_generic_chip_init(chip, &config);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261010130024.177503-1-ghahramani.navid@gmail.com?part=9