On Thu, Feb 14, 2019 at 10:56 AM Enrico Weigelt, metux IT consult <lkml@xxxxxxxxx> wrote: > On 08.02.19 15:25, Linus Walleij wrote: > > >> +config GPIO_AMD_FCH > >> + tristate "GPIO support for AMD Fusion Controller Hub (G-series SOCs)" > >> + select GPIO_GENERIC > > > > You are selecting GPIO_GENERIC, is this necessary? I thought > > X86 was already selecting this. > > Doesn't look so - at least haven't found anything where it's > automatically selected on x86. OTOH, that wouldn't make much > sense to me - I somewhat doubt that x86 can't run w/o that. Sorry my bad. You should select this if you use it, but are you using the GPIO MMIO abstractions? The hallmark of those implementations are that they call bgpio_init() and this driver does not, and it seems with the funny layout of the registers it can't even use it anyway. > IMHO, dependencies should always be direct - indirect ones could > suddenly change in subtle ways. Agreed. > >> + priv->gc.owner = THIS_MODULE; > >> + priv->gc.parent = &pdev->dev; > >> + priv->gc.label = dev_name(&pdev->dev); > >> + priv->gc.base = priv->pdata->gpio_base; > > > > No please, use priv->gc.base = -1; > > Could I also leave that field untouched (IOW: =0) ? No unfortunately not, because 0 is a valid GPIO base. Yours, Linus Walleij
