Re: [PATCH 3/7] ASoC: wm8903: generalize GPIO control register bits

[Linus Walleij <linus.walleij@xxxxxxxxxx>]

On Tue, Jun 2, 2015 at 1:09 AM, Vladimir Zapolskiy <vz@xxxxxxxxx> 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@xxxxxxxxx>
> Cc: Charles Keepax <ckeepax@xxxxxxxxxxxxxxxxxxxxxxxxxxx>
> Cc: Lars-Peter Clausen <lars@xxxxxxxxxx>
> Cc: Axel Lin <axel.lin@xxxxxxxxxx>
> Cc: patches@xxxxxxxxxxxxxxxxxxxxxxxxxxx

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
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html