| Message ID | 20180918191233.17178-1-linus.walleij@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] sound: soc: wm8903: Pull in platform data | expand |
On Tue, Sep 18, 2018 at 12:12:33PM -0700, Linus Walleij wrote: > This variable is only used inside the file, and we should > never hard-code the GPIO base, so delete this. Adding the Cirrus people again... hard coding the GPIO base is needed for board file based systems if they're using GPIO numbers isn't it? > > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- > sound/soc/codecs/wm8903.c | 8 +------- > 1 file changed, 1 insertion(+), 7 deletions(-) > > diff --git a/sound/soc/codecs/wm8903.c b/sound/soc/codecs/wm8903.c > index 23e43ff40ded..545512dc4c45 100644 > --- a/sound/soc/codecs/wm8903.c > +++ b/sound/soc/codecs/wm8903.c > @@ -144,7 +144,6 @@ struct wm8903_platform_data { > > int micdet_delay; /* Delay after microphone detection (ms) */ > > - int gpio_base; > u32 gpio_cfg[WM8903_NUM_GPIO]; /* Default register values for GPIO pin mux */ > }; > > @@ -1876,17 +1875,12 @@ static const struct gpio_chip wm8903_template_chip = { > > static void wm8903_init_gpio(struct wm8903_priv *wm8903) > { > - struct wm8903_platform_data *pdata = wm8903->pdata; > int ret; > > wm8903->gpio_chip = wm8903_template_chip; > wm8903->gpio_chip.ngpio = WM8903_NUM_GPIO; > wm8903->gpio_chip.parent = wm8903->dev; > - > - if (pdata->gpio_base) > - wm8903->gpio_chip.base = pdata->gpio_base; > - else > - wm8903->gpio_chip.base = -1; > + wm8903->gpio_chip.base = -1; > > ret = gpiochip_add_data(&wm8903->gpio_chip, wm8903); > if (ret != 0) > -- > 2.17.1 >
On Tue, Sep 18, 2018 at 12:52 PM Mark Brown <broonie@kernel.org> wrote: > On Tue, Sep 18, 2018 at 12:12:33PM -0700, Linus Walleij wrote: > > This variable is only used inside the file, and we should > > never hard-code the GPIO base, so delete this. > > Adding the Cirrus people again... Sorry about all beginners mistakes :/ > hard coding the GPIO base is needed > for board file based systems if they're using GPIO numbers isn't it? It is needed if there are board files hardcoding GPIOs referencing the global number range on this specific gpiochip. Since there are no boards defining platform data (hence patch 1) I assume all users are using device tree and thus dynamic GPIO number assignment, so this can go. Even if new board files are added they should use machine descriptor tables for referencing the individual GPIO chip and offset. In this case by the label "wm8903" and local offset. Yours, Linus Walleij
On Tue, Sep 18, 2018 at 04:51:23PM -0700, Linus Walleij wrote: > Since there are no boards defining platform data (hence patch 1) > I assume all users are using device tree and thus dynamic > GPIO number assignment, so this can go. > Even if new board files are added they should use machine > descriptor tables for referencing the individual GPIO chip and > offset. In this case by the label "wm8903" and local offset. I tend to worry about breaking out of tree users unless there's an active reason to do it, is there something coming along - this looked more like just a cleanup?
On Tue, Sep 18, 2018 at 5:09 PM Mark Brown <broonie@kernel.org> wrote: > On Tue, Sep 18, 2018 at 04:51:23PM -0700, Linus Walleij wrote: > > > Since there are no boards defining platform data (hence patch 1) > > I assume all users are using device tree and thus dynamic > > GPIO number assignment, so this can go. > > > Even if new board files are added they should use machine > > descriptor tables for referencing the individual GPIO chip and > > offset. In this case by the label "wm8903" and local offset. > > I tend to worry about breaking out of tree users unless there's an > active reason to do it, is there something coming along - this looked > more like just a cleanup? This is to get rid of the global GPIO numberspace, little by little. As long as we hardcode the base of the GPIO chip anywhere, that numberspace prevails. Yours, Linus Walleij
diff --git a/sound/soc/codecs/wm8903.c b/sound/soc/codecs/wm8903.c index 23e43ff40ded..545512dc4c45 100644 --- a/sound/soc/codecs/wm8903.c +++ b/sound/soc/codecs/wm8903.c @@ -144,7 +144,6 @@ struct wm8903_platform_data { int micdet_delay; /* Delay after microphone detection (ms) */ - int gpio_base; u32 gpio_cfg[WM8903_NUM_GPIO]; /* Default register values for GPIO pin mux */ }; @@ -1876,17 +1875,12 @@ static const struct gpio_chip wm8903_template_chip = { static void wm8903_init_gpio(struct wm8903_priv *wm8903) { - struct wm8903_platform_data *pdata = wm8903->pdata; int ret; wm8903->gpio_chip = wm8903_template_chip; wm8903->gpio_chip.ngpio = WM8903_NUM_GPIO; wm8903->gpio_chip.parent = wm8903->dev; - - if (pdata->gpio_base) - wm8903->gpio_chip.base = pdata->gpio_base; - else - wm8903->gpio_chip.base = -1; + wm8903->gpio_chip.base = -1; ret = gpiochip_add_data(&wm8903->gpio_chip, wm8903); if (ret != 0)
This variable is only used inside the file, and we should never hard-code the GPIO base, so delete this. Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- sound/soc/codecs/wm8903.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)