diff mbox

[1/4] pinctrl: single: Prepare for supporting SoC specific features

Message ID 20130608152732.GR3331@atomide.com (mailing list archive)
State New, archived
Headers show

Commit Message

Tony Lindgren June 8, 2013, 3:27 p.m. UTC 
* Haojian Zhuang <haojian.zhuang@gmail.com> [130608 02:43]:
> 
> Manjunathappa's pinctrl-single patch on enhancing bits is already merged.
> This patch conflicts with his patch.
> 
> Could you rebase your patches?

Sure. Looks like Linus W forgot to push out the branch as I don't see
it yet in the pinctrl tree. Here's a version of this one against current
Linux next + Manjunathappa's patches.

Regards,

Tony

From: Tony Lindgren <tony@atomide.com>
Date: Sat, 8 Jun 2013 08:03:07 -0700
Subject: [PATCH] pinctrl: single: Prepare for supporting SoC specific features

Let's replace is_pinconf with flags and add struct pcs_soc so we
can support also other features like pin wake-up events. Let's
export the probe so the SoC specific modules can pass their
SoC specific data to pinctrl-single if needed.

Done in collaboration with Roger Quadros <rogerq@ti.com>.

Cc: Haojian Zhuang <haojian.zhuang@gmail.com>
Cc: Peter Ujfalusi <peter.ujfalusi@ti.com>
Cc: devicetree-discuss@lists.ozlabs.org
Signed-off-by: Roger Quadros <rogerq@ti.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>

Comments

Haojian Zhuang June 9, 2013, 5:21 a.m. UTC | #1 
On Sat, Jun 8, 2013 at 11:27 PM, Tony Lindgren <tony@atomide.com> wrote:
> * Haojian Zhuang <haojian.zhuang@gmail.com> [130608 02:43]:
>>
>> Manjunathappa's pinctrl-single patch on enhancing bits is already merged.
>> This patch conflicts with his patch.
>>
>> Could you rebase your patches?
>
> Sure. Looks like Linus W forgot to push out the branch as I don't see
> it yet in the pinctrl tree. Here's a version of this one against current
> Linux next + Manjunathappa's patches.
>
> Regards,
>
> Tony
>

Acked-by: Haojian Zhuang <haojian.zhuang@gmail.com>
Linus Walleij July 22, 2013, 9:15 p.m. UTC | #2 
On Sat, Jun 8, 2013 at 5:27 PM, Tony Lindgren <tony@atomide.com> wrote:

> Subject: [PATCH] pinctrl: single: Prepare for supporting SoC specific features
>
> Let's replace is_pinconf with flags and add struct pcs_soc so we
> can support also other features like pin wake-up events. Let's
> export the probe so the SoC specific modules can pass their
> SoC specific data to pinctrl-single if needed.

I don't quite understand this motivation. Can this be more verbose and
include a bit about the mechanics?

- Why is this necessary? For example, pinctrl-single already supports
 generic pinconf, and we can surely add a PIN_CONFIG_WAKEUP
 to include/linux/pinctrl/pinconf-generic.h.

- Also: how does this cooperate with irq_set_wake()? If a pin is
 set to GPIO it is often backed by a GPIO driver (which is calling
 pinctrl_request_gpio() etc), maybe we should just add a
 pinctrl_set_wake() then that the irqchip portions of the GPIO drivers
 can call down to so the pinctrl driver sets this bit if need be?

I think this needs some more thought, especially the latter
concern.

Yours,
Linus Walleij
Tony Lindgren July 29, 2013, 8:57 a.m. UTC | #3 
* Linus Walleij <linus.walleij@linaro.org> [130722 14:22]:
> On Sat, Jun 8, 2013 at 5:27 PM, Tony Lindgren <tony@atomide.com> wrote:
> 
> > Subject: [PATCH] pinctrl: single: Prepare for supporting SoC specific features
> >
> > Let's replace is_pinconf with flags and add struct pcs_soc so we
> > can support also other features like pin wake-up events. Let's
> > export the probe so the SoC specific modules can pass their
> > SoC specific data to pinctrl-single if needed.
> 
> I don't quite understand this motivation. Can this be more verbose and
> include a bit about the mechanics?
> 
> - Why is this necessary? For example, pinctrl-single already supports
>  generic pinconf, and we can surely add a PIN_CONFIG_WAKEUP
>  to include/linux/pinctrl/pinconf-generic.h.

OK I'll take a look. I like the irqchip idea, let's see what all is
missing after that.
 
> - Also: how does this cooperate with irq_set_wake()? If a pin is
>  set to GPIO it is often backed by a GPIO driver (which is calling
>  pinctrl_request_gpio() etc), maybe we should just add a
>  pinctrl_set_wake() then that the irqchip portions of the GPIO drivers
>  can call down to so the pinctrl driver sets this bit if need be?

Yes currently we're missing the mapping between GPIO registers and
pinctrl registers. But your idea of using irqchip + pinctrl_set_wake()
might sort that issue.

Regards,

Tony
diff mbox

Patch

diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
index 2899c86..e3b1f76 100644
--- a/drivers/pinctrl/pinctrl-single.c
+++ b/drivers/pinctrl/pinctrl-single.c
@@ -26,6 +26,7 @@ 
 
 #include "core.h"
 #include "pinconf.h"
+#include "pinctrl-single.h"
 
 #define DRIVER_NAME			"pinctrl-single"
 #define PCS_MUX_PINS_NAME		"pinctrl-single,pins"
@@ -162,7 +163,6 @@  struct pcs_name {
  * @fshift:	function register shift
  * @foff:	value to turn mux off
  * @fmax:	max number of functions in fmask
- * @is_pinconf:	whether supports pinconf
  * @bits_per_pin:number of bits per pin
  * @names:	array of register names for pins
  * @pins:	physical pins on the SoC
@@ -176,6 +176,7 @@  struct pcs_name {
  * @desc:	pin controller descriptor
  * @read:	register read function to use
  * @write:	register write function to use
+ * @soc:	SoC glue
  */
 struct pcs_device {
 	struct resource *res;
@@ -190,8 +191,8 @@  struct pcs_device {
 	unsigned foff;
 	unsigned fmax;
 	bool bits_per_mux;
-	bool is_pinconf;
 	unsigned bits_per_pin;
+	unsigned flags;
 	struct pcs_name *names;
 	struct pcs_data pins;
 	struct radix_tree_root pgtree;
@@ -204,6 +205,7 @@  struct pcs_device {
 	struct pinctrl_desc desc;
 	unsigned (*read)(void __iomem *reg);
 	void (*write)(unsigned val, void __iomem *reg);
+	const struct pcs_soc *soc;
 };
 
 static int pcs_pinconf_get(struct pinctrl_dev *pctldev, unsigned pin,
@@ -1049,7 +1051,7 @@  static int pcs_parse_pinconf(struct pcs_device *pcs, struct device_node *np,
 	};
 
 	/* If pinconf isn't supported, don't parse properties in below. */
-	if (!pcs->is_pinconf)
+	if (!(pcs->flags & PCS_HAS_PINCONF))
 		return 0;
 
 	/* cacluate how much properties are supported in current node */
@@ -1173,7 +1175,7 @@  static int pcs_parse_one_pinctrl_entry(struct pcs_device *pcs,
 	(*map)->data.mux.group = np->name;
 	(*map)->data.mux.function = np->name;
 
-	if (pcs->is_pinconf) {
+	if (pcs->flags & PCS_HAS_PINCONF) {
 		res = pcs_parse_pinconf(pcs, np, function, map);
 		if (res)
 			goto free_pingroups;
@@ -1294,7 +1296,7 @@  static int pcs_parse_bits_in_pinctrl_entry(struct pcs_device *pcs,
 	(*map)->data.mux.group = np->name;
 	(*map)->data.mux.function = np->name;
 
-	if (pcs->is_pinconf) {
+	if (pcs->flags & PCS_HAS_PINCONF) {
 		dev_err(pcs->dev, "pinconf not supported\n");
 		goto free_pingroups;
 	}
@@ -1483,18 +1485,14 @@  static int pcs_add_gpio_func(struct device_node *node, struct pcs_device *pcs)
 	return ret;
 }
 
-static int pcs_probe(struct platform_device *pdev)
+int pinctrl_single_probe(struct platform_device *pdev,
+			 const struct pcs_soc *soc)
 {
 	struct device_node *np = pdev->dev.of_node;
-	const struct of_device_id *match;
 	struct resource *res;
 	struct pcs_device *pcs;
 	int ret;
 
-	match = of_match_device(pcs_of_match, &pdev->dev);
-	if (!match)
-		return -EINVAL;
-
 	pcs = devm_kzalloc(&pdev->dev, sizeof(*pcs), GFP_KERNEL);
 	if (!pcs) {
 		dev_err(&pdev->dev, "could not allocate\n");
@@ -1505,7 +1503,8 @@  static int pcs_probe(struct platform_device *pdev)
 	INIT_LIST_HEAD(&pcs->pingroups);
 	INIT_LIST_HEAD(&pcs->functions);
 	INIT_LIST_HEAD(&pcs->gpiofuncs);
-	pcs->is_pinconf = match->data;
+	pcs->flags = soc->flags;
+	pcs->soc = soc;
 
 	PCS_GET_PROP_U32("pinctrl-single,register-width", &pcs->width,
 			 "register width not specified\n");
@@ -1574,7 +1573,7 @@  static int pcs_probe(struct platform_device *pdev)
 	pcs->desc.name = DRIVER_NAME;
 	pcs->desc.pctlops = &pcs_pinctrl_ops;
 	pcs->desc.pmxops = &pcs_pinmux_ops;
-	if (pcs->is_pinconf)
+	if (pcs->flags & PCS_HAS_PINCONF)
 		pcs->desc.confops = &pcs_pinconf_ops;
 	pcs->desc.owner = THIS_MODULE;
 
@@ -1603,8 +1602,20 @@  free:
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(pinctrl_single_probe);
+
+static int pcs_probe(struct platform_device *pdev)
+{
+	const struct of_device_id *match;
+
+	match = of_match_device(pcs_of_match, &pdev->dev);
+	if (!match)
+		return -EINVAL;
+
+	return pinctrl_single_probe(pdev, (struct pcs_soc *)match->data);
+}
 
-static int pcs_remove(struct platform_device *pdev)
+int pinctrl_single_remove(struct platform_device *pdev)
 {
 	struct pcs_device *pcs = platform_get_drvdata(pdev);
 
@@ -1615,17 +1626,26 @@  static int pcs_remove(struct platform_device *pdev)
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(pinctrl_single_remove);
+
+static struct pcs_soc pinctrl_single = {
+	.flags = 0,
+};
+
+static struct pcs_soc pinconf_single = {
+	.flags = PCS_HAS_PINCONF,
+};
 
 static struct of_device_id pcs_of_match[] = {
-	{ .compatible = "pinctrl-single", .data = (void *)false },
-	{ .compatible = "pinconf-single", .data = (void *)true },
+	{ .compatible = "pinctrl-single", .data = &pinctrl_single },
+	{ .compatible = "pinconf-single", .data = &pinconf_single },
 	{ },
 };
 MODULE_DEVICE_TABLE(of, pcs_of_match);
 
 static struct platform_driver pcs_driver = {
 	.probe		= pcs_probe,
-	.remove		= pcs_remove,
+	.remove		= pinctrl_single_remove,
 	.driver = {
 		.owner		= THIS_MODULE,
 		.name		= DRIVER_NAME,
diff --git a/drivers/pinctrl/pinctrl-single.h b/drivers/pinctrl/pinctrl-single.h
new file mode 100644
index 0000000..18f3205
--- /dev/null
+++ b/drivers/pinctrl/pinctrl-single.h
@@ -0,0 +1,15 @@ 
+#define PCS_HAS_PINCONF         (1 << 0)
+
+/**
+ * struct pcs_soc - SoC specific interface to pinctrl-single
+ * @data:	SoC specific data pointer
+ * @flags:	mask of PCS_HAS_xxx values
+ */
+struct pcs_soc {
+	void *data;
+	unsigned flags;
+};
+
+extern int pinctrl_single_probe(struct platform_device *pdev,
+				const struct pcs_soc *soc);
+extern int pinctrl_single_remove(struct platform_device *pdev);