[PATCH v2] gpio: octeon_gpio: Add GPIO controller driver for Octeon

Daniel Schwierzeck daniel.schwierzeck at gmail.com
Tue Jul 21 16:59:20 CEST 2020


> Hi Daniel,
> 
> On 18.07.20 15:25, Daniel Schwierzeck wrote:
> > > From: Suneel Garapati <sgarapati at marvell.com>
...
> > found another issue:
> > 
> > drivers/gpio/octeon_gpio.c: In function 'octeon_gpio_probe':
> > drivers/gpio/octeon_gpio.c:189:16: warning: implicit declaration of
> > function 'dm_pci_map_bar'; did you mean 'pci_map_bar'? [-Wimplicit-
> > function-declaration]
> >    189 |   priv->base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0,
> >        |                ^~~~~~~~~~~~~~
> >        |                pci_map_bar
> > drivers/gpio/octeon_gpio.c:189:14: warning: assignment to 'void *' from
> > 'int' makes pointer from integer without a cast [-Wint-conversion]
> >    189 |   priv->base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0,
> >        |              ^
> 
> Ah, I did not see this in my local builds, as I have enabled DM_PCI
> here and forgot to add this dependency.
> 
> > due to the dependency on DM_PCI you need to express this in Kconfig
> > with "depends on DM_PCI ...". Alternatively you need to wrap the PCI
> > specific code with a "#ifdef CONFIG_DM_PCI" as the DM_PCI specific
> > function prototypes in pci.h are also wrapped with "#ifdef
> > CONFIG_DM_PCI". The former option will build and link DM PCI code which
> > is not used and therefore bloats the U-Boot binary.
> > 
> > I guess the choice mainly depends on whether you'll add a PCI host
> > controller driver for Octeon MIPS64 in the future and can live with the
> > extra but unused PCI code until then.
> 
> We can definitely live with the temporarily unused PCI code. I don't
> want to add these #ifdefs again, which I removed explicitly upon Simon's
> request.
> 
> To solve this, I would prefer to add a "select DM_PCI & PCI" to
> arch/mips/Kconfig for Octeon, as this PCI probing construct is not
> only used in this GPIO driver, but also in many other drivers shared
> between MIPS Octeon and the also upcoming ARM64 Octeon TX/TX2 support,
> like I2C, SPI etc. Here all devices are PCI based hence the PCI probing
> dependency.
> 
> Is this okay with you? Or should I stay with the dependency in the
> drivers Kconfig file?
> 

I would also prefer a "select DM_PCI". I just struggle a bit with the
"& PCI" part. IMHO DM_PCI should not depend on PCI because DM_PCI
should be a hidden option which is selected by the SoC dependent on the
SoC specific drivers and whether them support DM. It doesn't make sense
to let the user choose all those DM_* options. Most other DM_* options
already work like this except that them are not hidden.

BTW: When you prepare a patch, you can also "select DM_I2C" and
"DM_SPI" which have the same issues with the PCI dependency.

-- 
- Daniel



More information about the U-Boot mailing list