diff mbox series

[09/21] gpio: add driver for ADI ADSP-SC5xx platform

Message ID 20240912-test-v1-9-458fa57c8ccf@analog.com (mailing list archive)
State New
Headers show
Series Adding support of ADI ARMv8 ADSP-SC598 SoC. | expand

Commit Message

Arturs Artamonovs via B4 Relay Sept. 12, 2024, 6:24 p.m. UTC
From: Arturs Artamonovs <arturs.artamonovs@analog.com>

Add ADSP-SC5xx GPIO driver.
- Support all GPIO ports
- Each gpio support seperate PINT interrupt controller

Signed-off-by: Arturs Artamonovs <Arturs.Artamonovs@analog.com>
Co-developed-by: Nathan Barrett-Morrison <nathan.morrison@timesys.com>
Signed-off-by: Nathan Barrett-Morrison <nathan.morrison@timesys.com>
Co-developed-by: Greg Malysa <greg.malysa@timesys.com>
Signed-off-by: Greg Malysa <greg.malysa@timesys.com>
---
 drivers/gpio/Kconfig              |   8 +++
 drivers/gpio/Makefile             |   1 +
 drivers/gpio/gpio-adi-adsp-port.c | 145 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 154 insertions(+)

Comments

Arnd Bergmann Sept. 13, 2024, 7:38 a.m. UTC | #1
On Thu, Sep 12, 2024, at 18:24, Arturs Artamonovs via B4 Relay wrote:
> +
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/soc/adi/adsp-gpio-port.h>
> +#include "gpiolib.h"
> +
> +static int adsp_gpio_direction_input(struct gpio_chip *chip, unsigned 
> int offset)
> +{
> +	struct adsp_gpio_port *port = to_adsp_gpio_port(chip);
> +
> +	__adsp_gpio_writew(port, BIT(offset), ADSP_PORT_REG_DIR_CLEAR);
> +	__adsp_gpio_writew(port, BIT(offset), ADSP_PORT_REG_INEN_SET);
> +	return 0;

What is the purpose of these __adsp_gpio_writew() in a global header?
Can't those be moved into this file directly?

> \ No newline at end of file

Whitespace damage?

   Arnd
kernel test robot Sept. 14, 2024, 2:29 p.m. UTC | #2
Hi Arturs,

kernel test robot noticed the following build errors:

[auto build test ERROR on da3ea35007d0af457a0afc87e84fddaebc4e0b63]

url:    https://github.com/intel-lab-lkp/linux/commits/Arturs-Artamonovs-via-B4-Relay/arm64-Add-ADI-ADSP-SC598-SoC/20240913-022308
base:   da3ea35007d0af457a0afc87e84fddaebc4e0b63
patch link:    https://lore.kernel.org/r/20240912-test-v1-9-458fa57c8ccf%40analog.com
patch subject: [PATCH 09/21] gpio: add driver for ADI ADSP-SC5xx platform
config: arm64-randconfig-r071-20240914 (https://download.01.org/0day-ci/archive/20240914/202409142215.olyOwnPE-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 14.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240914/202409142215.olyOwnPE-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202409142215.olyOwnPE-lkp@intel.com/

All errors (new ones prefixed by >>):

   aarch64-linux-ld: Unexpected GOT/PLT entries detected!
   aarch64-linux-ld: Unexpected run-time procedure linkages detected!
   aarch64-linux-ld: drivers/gpio/gpio-adi-adsp-port.o: in function `adsp_gpio_probe':
>> gpio-adi-adsp-port.c:(.text+0x1cc): undefined reference to `adsp_attach_pint_to_gpio'
Krzysztof Kozlowski Sept. 16, 2024, 6:50 a.m. UTC | #3
On 12/09/2024 20:24, Arturs Artamonovs via B4 Relay wrote:
> From: Arturs Artamonovs <arturs.artamonovs@analog.com>
> 
> Add ADSP-SC5xx GPIO driver.
> - Support all GPIO ports
> - Each gpio support seperate PINT interrupt controller
> 
> Signed-off-by: Arturs Artamonovs <Arturs.Artamonovs@analog.com>
> Co-developed-by: Nathan Barrett-Morrison <nathan.morrison@timesys.com>
> Signed-off-by: Nathan Barrett-Morrison <nathan.morrison@timesys.com>
> Co-developed-by: Greg Malysa <greg.malysa@timesys.com>
> Signed-off-by: Greg Malysa <greg.malysa@timesys.com>

Your SoB chain is odd. Author is Greg, but Greg is not the first person
in the chain? And no final SoB? This is really odd and not correct. I am
not sure what you even want to say here.

...

> +
> +module_platform_driver(adsp_gpio_driver);
> +
> +MODULE_AUTHOR("Greg Malysa <greg.malysa@timesys.com>");
> +MODULE_DESCRIPTION("Analog Devices GPIO driver");
> +MODULE_LICENSE("GPL v2");
> \ No newline at end of file

Please review all your patches before sending for such stuff.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 58f43bcced7c1f29fad5960771817f500ef67ce1..b02693f5b4cec95a59f19aa1bacf7ed72236865a 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -147,6 +147,14 @@  config GPIO_74XX_MMIO
 	    8 bits:	74244 (Input), 74273 (Output)
 	    16 bits:	741624 (Input), 7416374 (Output)
 
+config GPIO_ADI_ADSP_PORT
+	bool "ADI ADSP PORT GPIO driver"
+	depends on OF_GPIO
+	select GPIO_GENERIC
+	help
+	  Say Y to enable the ADSP PORT-based GPIO driver for Analog Devices
+	  ADSP chips.
+
 config GPIO_ALTERA
 	tristate "Altera GPIO"
 	depends on OF_GPIO
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 64dd6d9d730d5a22564821df71375113e31fe057..fb02c7807a674c8a38d1128e6a25bb7c7f1f4aab 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -24,6 +24,7 @@  obj-$(CONFIG_GPIO_104_IDI_48)		+= gpio-104-idi-48.o
 obj-$(CONFIG_GPIO_104_IDIO_16)		+= gpio-104-idio-16.o
 obj-$(CONFIG_GPIO_74X164)		+= gpio-74x164.o
 obj-$(CONFIG_GPIO_74XX_MMIO)		+= gpio-74xx-mmio.o
+obj-$(CONFIG_GPIO_ADI_ADSP_PORT)	+= gpio-adi-adsp-port.o
 obj-$(CONFIG_GPIO_ADNP)			+= gpio-adnp.o
 obj-$(CONFIG_GPIO_ADP5520)		+= gpio-adp5520.o
 obj-$(CONFIG_GPIO_AGGREGATOR)		+= gpio-aggregator.o
diff --git a/drivers/gpio/gpio-adi-adsp-port.c b/drivers/gpio/gpio-adi-adsp-port.c
new file mode 100644
index 0000000000000000000000000000000000000000..a7a1867495bbdd121cda9b99991865a035dfa117
--- /dev/null
+++ b/drivers/gpio/gpio-adi-adsp-port.c
@@ -0,0 +1,145 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ADSP PORT gpio driver
+ *
+ * (C) Copyright 2022-2024 - Analog Devices, Inc.
+ */
+
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/soc/adi/adsp-gpio-port.h>
+#include "gpiolib.h"
+
+static int adsp_gpio_direction_input(struct gpio_chip *chip, unsigned int offset)
+{
+	struct adsp_gpio_port *port = to_adsp_gpio_port(chip);
+
+	__adsp_gpio_writew(port, BIT(offset), ADSP_PORT_REG_DIR_CLEAR);
+	__adsp_gpio_writew(port, BIT(offset), ADSP_PORT_REG_INEN_SET);
+	return 0;
+}
+
+static int adsp_gpio_direction_output(struct gpio_chip *chip, unsigned int offset,
+	int value)
+{
+	struct adsp_gpio_port *port = to_adsp_gpio_port(chip);
+
+	/*
+	 * For open drain ports, they've already been configured by pinctrl and
+	 * we should not modify their output characteristics
+	 */
+	if (port->open_drain & BIT(offset))
+		return 0;
+
+	__adsp_gpio_writew(port, BIT(offset), ADSP_PORT_REG_INEN_CLEAR);
+
+	if (value)
+		__adsp_gpio_writew(port, BIT(offset), ADSP_PORT_REG_DATA_SET);
+	else
+		__adsp_gpio_writew(port, BIT(offset), ADSP_PORT_REG_DATA_CLEAR);
+
+	__adsp_gpio_writew(port, BIT(offset), ADSP_PORT_REG_DIR_SET);
+	return 0;
+}
+
+static void adsp_gpio_set_value(struct gpio_chip *chip, unsigned int offset, int value)
+{
+	struct adsp_gpio_port *port = to_adsp_gpio_port(chip);
+
+	/*
+	 * For open drain ports, set as input if driving a 1, set as output
+	 * if driving a 0
+	 */
+	if (port->open_drain & BIT(offset)) {
+		if (value) {
+			__adsp_gpio_writew(port, BIT(offset), ADSP_PORT_REG_DIR_CLEAR);
+			__adsp_gpio_writew(port, BIT(offset), ADSP_PORT_REG_INEN_SET);
+		} else {
+			__adsp_gpio_writew(port, BIT(offset), ADSP_PORT_REG_INEN_CLEAR);
+			__adsp_gpio_writew(port, BIT(offset), ADSP_PORT_REG_DATA_CLEAR);
+			__adsp_gpio_writew(port, BIT(offset), ADSP_PORT_REG_DIR_SET);
+		}
+	} else {
+		if (value)
+			__adsp_gpio_writew(port, BIT(offset), ADSP_PORT_REG_DATA_SET);
+		else
+			__adsp_gpio_writew(port, BIT(offset), ADSP_PORT_REG_DATA_CLEAR);
+	}
+}
+
+static int adsp_gpio_get_value(struct gpio_chip *chip, unsigned int offset)
+{
+	struct adsp_gpio_port *port = to_adsp_gpio_port(chip);
+
+	return !!(__adsp_gpio_readw(port, ADSP_PORT_REG_DATA) & BIT(offset));
+}
+
+static int adsp_gpio_to_irq(struct gpio_chip *chip, unsigned int offset)
+{
+	struct adsp_gpio_port *port = to_adsp_gpio_port(chip);
+	irq_hw_number_t irq = offset + port->irq_offset;
+	int map = irq_find_mapping(port->irq_domain, irq);
+
+	if (map)
+		return map;
+
+	return irq_create_mapping(port->irq_domain, irq);
+}
+
+static int adsp_gpio_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct adsp_gpio_port *gpio;
+	int ret;
+
+	gpio = devm_kzalloc(dev, sizeof(*gpio), GFP_KERNEL);
+	if (!gpio)
+		return -ENOMEM;
+
+	gpio->regs = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(gpio->regs))
+		return PTR_ERR(gpio->regs);
+
+	gpio->dev = dev;
+
+	ret = adsp_attach_pint_to_gpio(gpio);
+	if  (ret)
+		dev_err_probe(gpio->dev, ret, "error attaching interupt to gpio pin\n");
+
+	spin_lock_init(&gpio->lock);
+
+	gpio->gpio.label = "adsp-gpio";
+	gpio->gpio.direction_input = adsp_gpio_direction_input;
+	gpio->gpio.direction_output = adsp_gpio_direction_output;
+	gpio->gpio.get = adsp_gpio_get_value;
+	gpio->gpio.set = adsp_gpio_set_value;
+	gpio->gpio.to_irq = adsp_gpio_to_irq;
+	gpio->gpio.request = gpiochip_generic_request;
+	gpio->gpio.free = gpiochip_generic_free;
+	gpio->gpio.ngpio = ADSP_PORT_NGPIO;
+	gpio->gpio.parent = dev;
+	gpio->gpio.base = -1;
+	return devm_gpiochip_add_data(dev, &gpio->gpio, gpio);
+}
+
+static const struct of_device_id adsp_gpio_of_match[] = {
+	{ .compatible = "adi,adsp-port-gpio", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, adsp_gpio_of_match);
+
+static struct platform_driver adsp_gpio_driver = {
+	.driver = {
+		.name = "adsp-port-gpio",
+		.of_match_table = adsp_gpio_of_match,
+	},
+	.probe = adsp_gpio_probe,
+};
+
+module_platform_driver(adsp_gpio_driver);
+
+MODULE_AUTHOR("Greg Malysa <greg.malysa@timesys.com>");
+MODULE_DESCRIPTION("Analog Devices GPIO driver");
+MODULE_LICENSE("GPL v2");
\ No newline at end of file