Re: [PATCH v3] pinctrl: Add pinctrl-s3c24xx driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 04/11/2013 02:09 PM, Heiko Stübner wrote:
The s3c24xx pins follow a similar pattern as the other Samsung SoCs and
can therefore reuse the already introduced infrastructure.

The s3c24xx SoCs have one design oddity in that the first 4 external
interrupts do not reside in the eint pending register but in the main
interrupt controller instead. We solve this by forwarding the external
interrupt from the main controller into the irq domain of the pin bank.
The masking/acking of these interrupts is handled in the same way.

Furthermore the S3C2412/2413 SoCs contain another oddity in that they
keep the same 4 eints in the main interrupt controller and eintpend
register and requiring ack operations to happen in both. This is solved
by using different compatible properties for the wakeup eint node which
set a property accordingly.

Signed-off-by: Heiko Stuebner<heiko@xxxxxxxxx>

Reviewed-by: Sylwester Nawrocki <s.nawrocki@xxxxxxxxxxx>

Just couple nitpicks below...

---
changes since v2:
- address more comments from Tomasz Figa:
   * remove obsolete ctrl_type references
   * remove redundant check for parent_chip
   * update docs and commit message

changes since v1:
- address comments from Tomasz Figa:
   * split handling functions for eints 0-3 for s3c2412 and all others
   * change the handling for s3c2412 eints 0-3 in that they now use
     chained_irq_* for the outer parent interrupt

  .../bindings/pinctrl/samsung-pinctrl.txt           |    8 +
  drivers/gpio/gpio-samsung.c                        |    4 +
  drivers/pinctrl/Kconfig                            |    5 +
  drivers/pinctrl/Makefile                           |    1 +
  drivers/pinctrl/pinctrl-s3c24xx.c                  |  664 ++++++++++++++++++++
  drivers/pinctrl/pinctrl-samsung.c                  |   10 +
  drivers/pinctrl/pinctrl-samsung.h                  |    4 +
  7 files changed, 696 insertions(+), 0 deletions(-)
  create mode 100644 drivers/pinctrl/pinctrl-s3c24xx.c

diff --git a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
index c70fca1..e4443e3 100644
--- a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
+++ b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
@@ -7,6 +7,10 @@ on-chip controllers onto these pads.

  Required Properties:
  - compatible: should be one of the following.
+  - "samsung,s3c2413-pinctrl": for S3C64xx-compatible pin-controller,
+  - "samsung,s3c2416-pinctrl": for S3C64xx-compatible pin-controller,
+  - "samsung,s3c2440-pinctrl": for S3C64xx-compatible pin-controller,
+  - "samsung,s3c2450-pinctrl": for S3C64xx-compatible pin-controller,

Shouldn't "S3C64xx" be changed to match each SoC name ?

    - "samsung,s3c64xx-pinctrl": for S3C64xx-compatible pin-controller,
    - "samsung,exynos4210-pinctrl": for Exynos4210 compatible pin-controller.
    - "samsung,exynos4x12-pinctrl": for Exynos4x12 compatible pin-controller.
@@ -106,6 +110,10 @@ B. External Wakeup Interrupts: For supporting external wakeup interrupts, a

     - compatible: identifies the type of the external wakeup interrupt controller
       The possible values are:
+     - samsung,s3c2410-wakeup-eint: represents wakeup interrupt controller
+       found on Samsung S3C24xx SoCs except S3C2412 and S3C2413,
+     - samsung,s3c2412-wakeup-eint: represents wakeup interrupt controller
+       found on Samsung S3C2412 and S3C2413 SoCs,
       - samsung,s3c64xx-wakeup-eint: represents wakeup interrupt controller
         found on Samsung S3C64xx SoCs,
       - samsung,exynos4210-wakeup-eint: represents wakeup interrupt controller
diff --git a/drivers/gpio/gpio-samsung.c b/drivers/gpio/gpio-samsung.c
index dc06a6f..73017b9 100644
--- a/drivers/gpio/gpio-samsung.c
+++ b/drivers/gpio/gpio-samsung.c
@@ -3026,6 +3026,10 @@ static __init int samsung_gpiolib_init(void)
  	*/
  	struct device_node *pctrl_np;
  	static const struct of_device_id exynos_pinctrl_ids[] = {
+		{ .compatible = "samsung,s3c2413-pinctrl", },
+		{ .compatible = "samsung,s3c2416-pinctrl", },
+		{ .compatible = "samsung,s3c2440-pinctrl", },
+		{ .compatible = "samsung,s3c2450-pinctrl", },
  		{ .compatible = "samsung,s3c64xx-pinctrl", },
  		{ .compatible = "samsung,exynos4210-pinctrl", },
  		{ .compatible = "samsung,exynos4x12-pinctrl", },
diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index 7402ac9..58d73ac 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -226,6 +226,11 @@ config PINCTRL_EXYNOS5440
  	select PINMUX
  	select PINCONF

+config PINCTRL_S3C24XX
+	bool "Samsung S3C24XX SoC pinctrl driver"
+	depends on ARCH_S3C24XX
+	select PINCTRL_SAMSUNG
+
  config PINCTRL_S3C64XX
  	bool "Samsung S3C64XX SoC pinctrl driver"
  	depends on ARCH_S3C64XX
diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
index 21d34c2..1ccdfd8 100644
--- a/drivers/pinctrl/Makefile
+++ b/drivers/pinctrl/Makefile
@@ -45,6 +45,7 @@ obj-$(CONFIG_PINCTRL_COH901)	+= pinctrl-coh901.o
  obj-$(CONFIG_PINCTRL_SAMSUNG)	+= pinctrl-samsung.o
  obj-$(CONFIG_PINCTRL_EXYNOS)	+= pinctrl-exynos.o
  obj-$(CONFIG_PINCTRL_EXYNOS5440)	+= pinctrl-exynos5440.o
+obj-$(CONFIG_PINCTRL_S3C24XX)	+= pinctrl-s3c24xx.o
  obj-$(CONFIG_PINCTRL_S3C64XX)	+= pinctrl-s3c64xx.o
  obj-$(CONFIG_PINCTRL_XWAY)	+= pinctrl-xway.o
  obj-$(CONFIG_PINCTRL_LANTIQ)	+= pinctrl-lantiq.o
diff --git a/drivers/pinctrl/pinctrl-s3c24xx.c b/drivers/pinctrl/pinctrl-s3c24xx.c
new file mode 100644
index 0000000..4b03c8f
--- /dev/null
+++ b/drivers/pinctrl/pinctrl-s3c24xx.c
@@ -0,0 +1,664 @@
+/*
+ * S3C24XX specific support for Samsung pinctrl/gpiolib driver.
+ *
+ * Copyright (c) 2013 Heiko Stuebner<heiko@xxxxxxxxx>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This file contains the SamsungS3C24XX specific information required by the
+ * the Samsung pinctrl/gpiolib driver. It also includes the implementation of

s/the Samsung/Samsung

+ * external gpio and wakeup interrupt support.
+ */
+
[...]
+/**
+ * struct s3c24xx_eint_domain_data: per irq-domain data
+ * @bank

Missing description.

+ * @eint_data: common data
+ * eint0_3_parent_only: live eints 0-3 only in the main intc
+ */
+struct s3c24xx_eint_domain_data {
+	struct samsung_pin_bank *bank;
+	struct s3c24xx_eint_data *eint_data;
+	bool eint0_3_parent_only;
+};
+
+static int s3c24xx_eint_get_trigger(unsigned int type)
+{
+	int trigger;
+
+	switch (type) {
+	case IRQ_TYPE_EDGE_RISING:
+		trigger = EINT_EDGE_RISING;
+		break;
+	case IRQ_TYPE_EDGE_FALLING:
+		trigger = EINT_EDGE_FALLING;
+		break;
+	case IRQ_TYPE_EDGE_BOTH:
+		trigger = EINT_EDGE_BOTH;
+		break;
+	case IRQ_TYPE_LEVEL_HIGH:
+		trigger = EINT_LEVEL_HIGH;
+		break;
+	case IRQ_TYPE_LEVEL_LOW:
+		trigger = EINT_LEVEL_LOW;
+		break;

EINT_* could be directly returned above, and the local variable is not
needed then. That's presumably just a matter of personal preference though.

+	default:
+		return -EINVAL;
+	}
+
+	return trigger;
+}

+static int s3c24xx_eint_type(struct irq_data *data, unsigned int type)
+{
+	struct samsung_pin_bank *bank = irq_data_get_irq_chip_data(data);
+	struct samsung_pinctrl_drv_data *d = bank->drvdata;
+	int index = bank->eint_offset + data->hwirq;
+	void __iomem *reg;
+	int trigger;
+	u8 shift;
+	u32 val;
+
+	trigger = s3c24xx_eint_get_trigger(type);
+	if (trigger<  0) {
+		dev_err(d->dev, "unsupported external interrupt type\n");
+		return -EINVAL;
+	}
+
+	s3c24xx_eint_set_handler(data->irq, type);
+
+	/* Set up interrupt trigger */
+	reg = d->virt_base + EINT_REG(index);
+	shift = EINT_OFFS(index);
+
+	val = readl(reg);
+	val&= ~(EINT_MASK<<  shift);
+	val |= trigger<<  shift;
+	writel(val, reg);
+
+	s3c24xx_eint_set_function(d, bank, data->hwirq);
+
+	return 0;
+}
+
+/* handling of EINTs 0-3 on all except S3C2412 and S3C2413 */

s/handling/Handling ?

+
+static void s3c2410_eint0_3_ack(struct irq_data *data)
+{
+	struct samsung_pin_bank *bank = irq_data_get_irq_chip_data(data);
+	struct s3c24xx_eint_domain_data *ddata = bank->irq_domain->host_data;
+	struct s3c24xx_eint_data *eint_data = ddata->eint_data;
+	int parent_irq = eint_data->parents[data->hwirq];
+	struct irq_chip *parent_chip = irq_get_chip(parent_irq);
+
+	parent_chip->irq_ack(irq_get_irq_data(parent_irq));
+}
+
+static void s3c2410_eint0_3_mask(struct irq_data *data)
+{
+	struct samsung_pin_bank *bank = irq_data_get_irq_chip_data(data);
+	struct s3c24xx_eint_domain_data *ddata = bank->irq_domain->host_data;
+	struct s3c24xx_eint_data *eint_data = ddata->eint_data;
+	int parent_irq = eint_data->parents[data->hwirq];
+	struct irq_chip *parent_chip = irq_get_chip(parent_irq);
+
+	parent_chip->irq_mask(irq_get_irq_data(parent_irq));
+}
+
+static void s3c2410_eint0_3_unmask(struct irq_data *data)
+{
+	struct samsung_pin_bank *bank = irq_data_get_irq_chip_data(data);
+	struct s3c24xx_eint_domain_data *ddata = bank->irq_domain->host_data;
+	struct s3c24xx_eint_data *eint_data = ddata->eint_data;
+	int parent_irq = eint_data->parents[data->hwirq];
+	struct irq_chip *parent_chip = irq_get_chip(parent_irq);
+
+	parent_chip->irq_unmask(irq_get_irq_data(parent_irq));
+}
+
+static struct irq_chip s3c2410_eint0_3_chip = {
+	.name		= "s3c2410-eint0_3",
+	.irq_ack	= s3c2410_eint0_3_ack,
+	.irq_mask	= s3c2410_eint0_3_mask,
+	.irq_unmask	= s3c2410_eint0_3_unmask,
+	.irq_set_type	= s3c24xx_eint_type,
+};
+
+static void s3c2410_demux_eint0_3(unsigned int irq, struct irq_desc *desc)
+{
+	struct irq_data *data = irq_desc_get_irq_data(desc);
+	struct s3c24xx_eint_data *eint_data = irq_get_handler_data(irq);
+	unsigned int virq;
+
+	/* the first 4 eints have a simple 1 to 1 mapping */
+	virq = irq_linear_revmap(eint_data->domains[data->hwirq], data->hwirq);
+	/* Something must be really wrong if an unmapped EINT
+	 * was unmasked...
+	 */

Not a proper multi line comment style.

+	BUG_ON(!virq);
+
+	generic_handle_irq(virq);
+}
+
+/* handling of EINTs 0-3 on S3C2412 and S3C2413 */

s/handling/Handling ?

+static void s3c2412_demux_eint0_3(unsigned int irq, struct irq_desc *desc)
+{
+	struct irq_chip *chip = irq_get_chip(irq);
+	struct irq_data *data = irq_desc_get_irq_data(desc);
+	struct s3c24xx_eint_data *eint_data = irq_get_handler_data(irq);
+	unsigned int virq;
+
+	chained_irq_enter(chip, desc);
+
+	/* the first 4 eints have a simple 1 to 1 mapping */
+	virq = irq_linear_revmap(eint_data->domains[data->hwirq], data->hwirq);
+	/* Something must be really wrong if an unmapped EINT
+	 * was unmasked...
+	 */
+	BUG_ON(!virq);
+
+	generic_handle_irq(virq);
+
+	chained_irq_exit(chip, desc);
+}
+
+/* handling of all other eints */

s/handling/Handling ?

+static inline void s3c24xx_demux_eint(unsigned int irq, struct irq_desc *desc,
+				      u32 offset, u32 range)
+{
+	struct irq_chip *chip = irq_get_chip(irq);
+	struct s3c24xx_eint_data *data = irq_get_handler_data(irq);
+	struct samsung_pinctrl_drv_data *d = data->drvdata;
+	unsigned int pend, mask;
+
+	chained_irq_enter(chip, desc);
+
+	pend = readl(d->virt_base + EINTPEND_REG);
+	mask = readl(d->virt_base + EINTMASK_REG);
+
+	pend&= ~mask;
+	pend&= range;
+
+	while (pend) {
+		unsigned int virq;
+
+		irq = __ffs(pend);
+		pend&= ~(1<<  irq);
+		virq = irq_linear_revmap(data->domains[irq], irq - offset);
+		/* Something must be really wrong if an unmapped EINT
+		 * was unmasked...
+		 */
+		BUG_ON(!virq);
+
+		generic_handle_irq(virq);
+	}
+
+	chained_irq_exit(chip, desc);
+}

+static int s3c24xx_eint_init(struct samsung_pinctrl_drv_data *d)
+{
+	struct device *dev = d->dev;
+	const struct of_device_id *match;
+	struct device_node *eint_np = NULL;
+	struct device_node *np;
+	struct samsung_pin_bank *bank;
+	struct s3c24xx_eint_data *eint_data;
+	const struct irq_domain_ops *ops;
+	unsigned int i;
+	bool eint0_3_parent_only;
+	irq_flow_handler_t *handlers;
+
+	for_each_child_of_node(dev->of_node, np) {
+		match = of_match_node(s3c24xx_eint_irq_ids, np);
+		if (match) {
+			eint_np = np;
+			eint0_3_parent_only = (bool)match->data;
+			break;
+		}
+	}
+	if (!eint_np)
+		return -ENODEV;
+
+	eint_data = devm_kzalloc(dev, sizeof(*eint_data), GFP_KERNEL);
+	if (!eint_data) {
+		dev_err(dev, "could not allocate memory for wkup eint data\n");

k*alloc functions already log any errors, you could just return -ENOMEM,
without printing anything.

+		return -ENOMEM;
+	}
+	eint_data->drvdata = d;
+
+	handlers = eint0_3_parent_only ? s3c2410_eint_handlers
+				       : s3c2412_eint_handlers;
+	for (i = 0; i<  NUM_EINT_IRQ; ++i) {
+		unsigned int irq;
+
+		irq = irq_of_parse_and_map(eint_np, i);
+		if (!irq) {
+			dev_err(dev, "failed to get wakeup EINT IRQ %d\n", i);
+			return -ENXIO;
+		}
+
+		eint_data->parents[i] = irq;
+		irq_set_chained_handler(irq, handlers[i]);
+		irq_set_handler_data(irq, eint_data);
+	}
+
+	bank = d->ctrl->pin_banks;
+	for (i = 0; i<  d->ctrl->nr_banks; ++i, ++bank) {
+		struct s3c24xx_eint_domain_data *ddata;
+		unsigned int mask;
+		unsigned int irq;
+		unsigned int pin;
+
+		if (bank->eint_type != EINT_TYPE_WKUP)
+			continue;
+
+		ddata = devm_kzalloc(dev, sizeof(*ddata), GFP_KERNEL);
+		if (!ddata) {
+			dev_err(dev, "failed to allocate domain data\n");

Ditto.

+			return -ENOMEM;
+		}
+		ddata->bank = bank;
+		ddata->eint_data = eint_data;
+		ddata->eint0_3_parent_only = eint0_3_parent_only;
+
+		ops = (bank->eint_offset == 0) ?&s3c24xx_gpf_irq_ops
+					       :&s3c24xx_gpg_irq_ops;
+
+		bank->irq_domain = irq_domain_add_linear(bank->of_node,
+				bank->nr_pins, ops, ddata);
+		if (!bank->irq_domain) {
+			dev_err(dev, "wkup irq domain add failed\n");
+			return -ENXIO;
+		}
+
+		irq = bank->eint_offset;
+		mask = bank->eint_mask;
+		for (pin = 0; mask; ++pin, mask>>= 1) {
+			if (irq>  NUM_EINT)
+				break;
+			if (!(mask&  1))
+				continue;
+			eint_data->domains[irq] = bank->irq_domain;
+			++irq;
+		}
+	}
+
+	return 0;
+}

Regards,
Sylwester
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux SoC Development]     [Linux Rockchip Development]     [Linux USB Development]     [Video for Linux]     [Linux Audio Users]     [Linux SCSI]     [Yosemite News]

  Powered by Linux