Hi Daniel,
On 18.07.20 15:25, Daniel Schwierzeck wrote:
From: Suneel Garapati <sgarap...@marvell.com>
Add support for GPIO controllers found on Octeon II/III and Octeon TX
TX2 SoC platforms.
Signed-off-by: Aaron Williams <awilli...@marvell.com>
Signed-off-by: Suneel Garapati <sgarap...@marvell.com>
Signed-off-by: Stefan Roese <s...@denx.de>
Cc: Simon Glass <s...@chromium.org>
Cc: Daniel Schwierzeck <daniel.schwierz...@gmail.com>
Cc: Aaron Williams <awilli...@marvell.com>
Cc: Chandrakala Chavva <ccha...@marvell.com>
---
v2 (Stefan):
- Removed #ifdef's for Octeon vs OcteonTX/TX2 completely
The differentiation is now made via driver data / compatible
string
RFC -> v1 (Stefan)
- Separated this patch from the OcteonTX/TX2 RFC patch series into a
single patch. This is useful, as the upcoming MIPS Octeon support will
use this GPIO driver.
- Added MIPS Octeon II/III support (big endian). Rename driver and its
function names from "octeontx" to "octeon" to better match all Octeon
platforms.
- Moved from union to defines / bitmasks. This makes the driver usage
on little- and big-endian platforms much easier.
- Used clrbits_64() instead of clrbits_le64() and friends to support
usage on little- and big-endian systems
- Removed dev->req_seq assignment
- Enhanced Kconfig text
- Rewrote GPIO_BIT macro
- Dropped many macros to calculate the registers offsets and implemented
simple functions for this (easier to read)
- Used GENMASK_ULL and FIELD_GET helpers
- Minor cosmetic changes (dropped brackets etc)
- Reword commit text and subject
drivers/gpio/Kconfig | 10 ++
drivers/gpio/Makefile | 1 +
drivers/gpio/octeon_gpio.c | 253 +++++++++++++++++++++++++++++++++++++
3 files changed, 264 insertions(+)
create mode 100644 drivers/gpio/octeon_gpio.c
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index d87f6cc105..451896f400 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -316,6 +316,16 @@ config PIC32_GPIO
help
Say yes here to support Microchip PIC32 GPIOs.
+config OCTEON_GPIO
+ bool "Octeon II/III/TX/TX2 GPIO driver"
+ depends on DM_GPIO && (ARCH_OCTEON || ARCH_OCTEONTX || ARCH_OCTEONTX2)
+ default y
+ help
+ Add support for the Marvell Octeon GPIO driver. This is used with
+ various Octeon parts such as Octeon II/III and OcteonTX/TX2.
+ Octeon II/III has 32 GPIOs (count defined via DT) and OcteonTX/TX2
+ has 64 GPIOs (count defined via internal register).
+
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?
config STM32_GPIO
bool "ST STM32 GPIO driver"
depends on DM_GPIO && (ARCH_STM32 || ARCH_STM32MP)
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 7638259007..eb6364adb4 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -58,6 +58,7 @@ obj-$(CONFIG_HIKEY_GPIO) += hi6220_gpio.o
obj-$(CONFIG_HSDK_CREG_GPIO) += hsdk-creg-gpio.o
obj-$(CONFIG_IMX_RGPIO2P) += imx_rgpio2p.o
obj-$(CONFIG_PIC32_GPIO) += pic32_gpio.o
+obj-$(CONFIG_OCTEON_GPIO) += octeon_gpio.o
obj-$(CONFIG_MVEBU_GPIO) += mvebu_gpio.o
obj-$(CONFIG_MSM_GPIO) += msm_gpio.o
obj-$(CONFIG_$(SPL_)PCF8575_GPIO) += pcf8575_gpio.o
diff --git a/drivers/gpio/octeon_gpio.c b/drivers/gpio/octeon_gpio.c
new file mode 100644
index 0000000000..d7ac9a1910
--- /dev/null
+++ b/drivers/gpio/octeon_gpio.c
@@ -0,0 +1,253 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018 Marvell International Ltd.
+ *
+ * (C) Copyright 2011
+ * eInfochips Ltd. <www.einfochips.com>
+ * Written-by: Ajay Bhargav <ajay.bhar...@einfochips.com>
+ *
+ * (C) Copyright 2010
+ * Marvell Semiconductor <www.marvell.com>
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <errno.h>
+#include <fdtdec.h>
+#include <pci_ids.h>
+#include <asm/bitops.h>
+#include <asm/gpio.h>
+#include <asm/io.h>
+#include <linux/bitfield.h>
+#include <linux/compat.h>
+#include <dt-bindings/gpio/gpio.h>
don't use common.h anymore. Also some headers are unused. I can build
the driver with:
#include <dm.h>
#include <pci.h>
#include <pci_ids.h>
#include <asm/gpio.h>
#include <asm/io.h>
#include <linux/bitfield.h>
#include <linux/compat.h>
#include <dt-bindings/gpio/gpio.h>
Yes, thanks. I'll change this in v3.
Thanks,
Stefan
