[U-Boot] [PATCH 15/60] gpio: tegra: header file split

[Stephen Warren]

On 04/20/2016 01:26 PM, Simon Glass wrote:
> Hi Stephen,
>
> On 19 April 2016 at 14:58, Stephen Warren <swarren at wwwdotorg.org> wrote:
>> From: Stephen Warren <swarren at nvidia.com>
>>
>> Tegra's gpio.h contains a mix of private definitions for use inside the
>> GPIO driver and custom machine-specific APIs. Move the private definitions
>> out of the global include directory since nothing should need to access
>> them. Move the public definitions to the machine-specific include
>> directory <mach/>.

>> diff --git a/drivers/gpio/tegra_gpio_priv.h b/drivers/gpio/tegra_gpio_priv.h

>> +/*
>> + * GPIO registers are split into two chunks; low and high.
>> + * On Tegra20, all low chunks appear first, then all high chunks.
>> + * In later SoCs, the low and high chunks are interleaved together.
>> + */
>> +#define GPIO_CTLR_BANK_HIGH_REGS \
>> +       uint gpio_masked_config[TEGRA_GPIO_PORTS]; \
>> +       uint gpio_masked_dir_out[TEGRA_GPIO_PORTS]; \
>> +       uint gpio_masked_out[TEGRA_GPIO_PORTS]; \
>> +       uint reserved0[TEGRA_GPIO_PORTS]; \
>> +       uint gpio_masked_int_status[TEGRA_GPIO_PORTS]; \
>> +       uint gpio_masked_int_enable[TEGRA_GPIO_PORTS]; \
>> +       uint gpio_masked_int_level[TEGRA_GPIO_PORTS]; \
>> +       uint reserved1[TEGRA_GPIO_PORTS];
>> +
>> +/* GPIO Controller registers for a single bank */
>> +struct gpio_ctlr_bank {
>> +       uint gpio_config[TEGRA_GPIO_PORTS];
>> +       uint gpio_dir_out[TEGRA_GPIO_PORTS];
>> +       uint gpio_out[TEGRA_GPIO_PORTS];
>> +       uint gpio_in[TEGRA_GPIO_PORTS];
>> +       uint gpio_int_status[TEGRA_GPIO_PORTS];
>> +       uint gpio_int_enable[TEGRA_GPIO_PORTS];
>> +       uint gpio_int_level[TEGRA_GPIO_PORTS];
>> +       uint gpio_int_clear[TEGRA_GPIO_PORTS];
>> +#ifndef CONFIG_TEGRA20
>> +       GPIO_CTLR_BANK_HIGH_REGS
>> +#endif
>> +};
>> +
>> +#ifdef CONFIG_TEGRA20
>> +struct gpio_ctlr_bank_high {
>> +       GPIO_CTLR_BANK_HIGH_REGS
>
> This seems a bit ugly. Perhaps you could havestruct
> gpio_ctlr_high_regs and include that here? It adds a level of
> indirection but that doesn't seem very important.

In newer Tegras, there's no differentiation between the two register 
sets that were "low" and "high" in Tegra20. I'd rather not saddle the 
non-Tegra20 struct layouts with some odd naming/nesting just because the 
Tegra20 layout was odd. I don't see any problem with using a #define for 
this; it doesn't seem to make the code complex.

I wonder if we should just convert away from structs for registers 
entirely. Everything in the HW docs is just numbers, matching those to 
the structs is always painful, and if we used #defines instead of 
structs, representing this HW difference would end up being much cleaner 
and avoid using a macro to "cut/paste" a register list 2 times; see the 
way the Linux kernel driver handles this.