[alsa-devel] [PATCH 3/7] ASoC: wm8903: generalize GPIO control register bits

[Linus Walleij]

On Tue, Jun 2, 2015 at 1:09 AM, Vladimir Zapolskiy <vz at mleia.com> wrote:

> All GPIO1/2/3/4/5 control registers have the same bit map, but in
> implementation of gpiolib callbacks WM8903_GPn_*, WM8903_GP1_* and
> WM8903_GP2_* macro are mixed up. Replace particular GPIOn control
> register bit definitions with generic ones and save ~150 LoCs.
>
> No functional change.
>
> Signed-off-by: Vladimir Zapolskiy <vz at mleia.com>
> Cc: Charles Keepax <ckeepax at opensource.wolfsonmicro.com>
> Cc: Lars-Peter Clausen <lars at metafoo.de>
> Cc: Axel Lin <axel.lin at ingics.com>
> Cc: patches at opensource.wolfsonmicro.com

This comment is not about the refactoring as such, which is OK...

> +#define WM8903_GPn_FN_MASK                      0x1F00  /* GPn_FN - [12:8] */
> +#define WM8903_GPn_FN_SHIFT                          8  /* GPn_FN - [12:8] */
> +#define WM8903_GPn_FN_WIDTH                          5  /* GPn_FN - [12:8] */

Function selection?

> +#define WM8903_GPn_PD                           0x0008  /* GPn_PD */
> +#define WM8903_GPn_PD_MASK                      0x0008  /* GPn_PD */
> +#define WM8903_GPn_PD_SHIFT                          3  /* GPn_PD */
> +#define WM8903_GPn_PD_WIDTH                          1  /* GPn_PD */
> +#define WM8903_GPn_PU                           0x0004  /* GPn_PU */
> +#define WM8903_GPn_PU_MASK                      0x0004  /* GPn_PU */
> +#define WM8903_GPn_PU_SHIFT                          2  /* GPn_PU */
> +#define WM8903_GPn_PU_WIDTH                          1  /* GPn_PU */

Pull-down/pull-up?

That is pin control, not GPIO.

I know I should probably be a bit relaxed on enforcing strict frameworks
on ASoC and DRI/DRM alike, because the drivers are complex and sometimes
need to be on top of things rather than split things apart and set up
complex cobwebs of subsystem cross-dependencies, but I'd
like to know a bit more about the design philisophy here.

I guess this dates back to the time before the pin control subsystem?

Yours,
Linus Walleij