Let's replace is_pinconf with flags and add struct pcs_soc_data
so we can support SoC specific features like pin wake-up events.
Done in collaboration with Roger Quadros <[email protected]>.
Cc: Peter Ujfalusi <[email protected]>
Cc: Grygorii Strashko <[email protected]>
Cc: Prakash Manjunathappa <[email protected]>
Cc: Haojian Zhuang <[email protected]>
Cc: Linus Walleij <[email protected]>
Cc: [email protected]
Signed-off-by: Roger Quadros <[email protected]>
Signed-off-by: Tony Lindgren <[email protected]>
---
drivers/pinctrl/pinctrl-single.c | 38 +++++++++++++++++++++++++++++---------
1 file changed, 29 insertions(+), 9 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
index a82ace4..f88d3d1 100644
--- a/drivers/pinctrl/pinctrl-single.c
+++ b/drivers/pinctrl/pinctrl-single.c
@@ -150,19 +150,27 @@ struct pcs_name {
};
/**
+ * struct pcs_soc_data - SoC specific settings
+ * @flags: initial SoC specific PCS_FEAT_xxx values
+ */
+struct pcs_soc_data {
+ unsigned flags;
+};
+
+/**
* struct pcs_device - pinctrl device instance
* @res: resources
* @base: virtual address of the controller
* @size: size of the ioremapped area
* @dev: device entry
* @pctl: pin controller device
+ * @flags: mask of PCS_FEAT_xxx values
* @mutex: mutex protecting the lists
* @width: bits per mux register
* @fmask: function register mask
* @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
@@ -183,6 +191,8 @@ struct pcs_device {
unsigned size;
struct device *dev;
struct pinctrl_dev *pctl;
+ unsigned flags;
+#define PCS_FEAT_PINCONF (1 << 0)
struct mutex mutex;
unsigned width;
unsigned fmask;
@@ -190,7 +200,6 @@ struct pcs_device {
unsigned foff;
unsigned fmax;
bool bits_per_mux;
- bool is_pinconf;
unsigned bits_per_pin;
struct pcs_name *names;
struct pcs_data pins;
@@ -206,6 +215,8 @@ struct pcs_device {
void (*write)(unsigned val, void __iomem *reg);
};
+#define PCS_HAS_PINCONF (pcs->flags & PCS_FEAT_PINCONF)
+
static int pcs_pinconf_get(struct pinctrl_dev *pctldev, unsigned pin,
unsigned long *config);
static int pcs_pinconf_set(struct pinctrl_dev *pctldev, unsigned pin,
@@ -1060,7 +1071,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_HAS_PINCONF)
return 0;
/* cacluate how much properties are supported in current node */
@@ -1184,7 +1195,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_HAS_PINCONF) {
res = pcs_parse_pinconf(pcs, np, function, map);
if (res)
goto free_pingroups;
@@ -1305,7 +1316,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_HAS_PINCONF) {
dev_err(pcs->dev, "pinconf not supported\n");
goto free_pingroups;
}
@@ -1525,6 +1536,7 @@ static int pcs_probe(struct platform_device *pdev)
const struct of_device_id *match;
struct resource *res;
struct pcs_device *pcs;
+ const struct pcs_soc_data *soc;
int ret;
match = of_match_device(pcs_of_match, &pdev->dev);
@@ -1541,7 +1553,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;
+ soc = match->data;
+ pcs->flags = soc->flags;
PCS_GET_PROP_U32("pinctrl-single,register-width", &pcs->width,
"register width not specified\n");
@@ -1610,7 +1623,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_HAS_PINCONF)
pcs->desc.confops = &pcs_pinconf_ops;
pcs->desc.owner = THIS_MODULE;
@@ -1652,9 +1665,16 @@ static int pcs_remove(struct platform_device *pdev)
return 0;
}
+static const struct pcs_soc_data pinctrl_single = {
+};
+
+static const struct pcs_soc_data pinconf_single = {
+ .flags = PCS_FEAT_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);
Hi Linus W,
Any comments on the pinctrl patches 3 - 5 in this series?
These are badly needed to not break omap3 PM support when
we're moving to device tree based booting.
* Tony Lindgren <[email protected]> [131002 22:50]:
> Let's replace is_pinconf with flags and add struct pcs_soc_data
> so we can support SoC specific features like pin wake-up events.
>
> Done in collaboration with Roger Quadros <[email protected]>.
>
> Cc: Peter Ujfalusi <[email protected]>
> Cc: Grygorii Strashko <[email protected]>
> Cc: Prakash Manjunathappa <[email protected]>
> Cc: Haojian Zhuang <[email protected]>
> Cc: Linus Walleij <[email protected]>
> Cc: [email protected]
> Signed-off-by: Roger Quadros <[email protected]>
> Signed-off-by: Tony Lindgren <[email protected]>
> ---
> drivers/pinctrl/pinctrl-single.c | 38 +++++++++++++++++++++++++++++---------
> 1 file changed, 29 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
> index a82ace4..f88d3d1 100644
> --- a/drivers/pinctrl/pinctrl-single.c
> +++ b/drivers/pinctrl/pinctrl-single.c
> @@ -150,19 +150,27 @@ struct pcs_name {
> };
>
> /**
> + * struct pcs_soc_data - SoC specific settings
> + * @flags: initial SoC specific PCS_FEAT_xxx values
> + */
> +struct pcs_soc_data {
> + unsigned flags;
> +};
> +
> +/**
> * struct pcs_device - pinctrl device instance
> * @res: resources
> * @base: virtual address of the controller
> * @size: size of the ioremapped area
> * @dev: device entry
> * @pctl: pin controller device
> + * @flags: mask of PCS_FEAT_xxx values
> * @mutex: mutex protecting the lists
> * @width: bits per mux register
> * @fmask: function register mask
> * @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
> @@ -183,6 +191,8 @@ struct pcs_device {
> unsigned size;
> struct device *dev;
> struct pinctrl_dev *pctl;
> + unsigned flags;
> +#define PCS_FEAT_PINCONF (1 << 0)
> struct mutex mutex;
> unsigned width;
> unsigned fmask;
> @@ -190,7 +200,6 @@ struct pcs_device {
> unsigned foff;
> unsigned fmax;
> bool bits_per_mux;
> - bool is_pinconf;
> unsigned bits_per_pin;
> struct pcs_name *names;
> struct pcs_data pins;
> @@ -206,6 +215,8 @@ struct pcs_device {
> void (*write)(unsigned val, void __iomem *reg);
> };
>
> +#define PCS_HAS_PINCONF (pcs->flags & PCS_FEAT_PINCONF)
> +
> static int pcs_pinconf_get(struct pinctrl_dev *pctldev, unsigned pin,
> unsigned long *config);
> static int pcs_pinconf_set(struct pinctrl_dev *pctldev, unsigned pin,
> @@ -1060,7 +1071,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_HAS_PINCONF)
> return 0;
>
> /* cacluate how much properties are supported in current node */
> @@ -1184,7 +1195,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_HAS_PINCONF) {
> res = pcs_parse_pinconf(pcs, np, function, map);
> if (res)
> goto free_pingroups;
> @@ -1305,7 +1316,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_HAS_PINCONF) {
> dev_err(pcs->dev, "pinconf not supported\n");
> goto free_pingroups;
> }
> @@ -1525,6 +1536,7 @@ static int pcs_probe(struct platform_device *pdev)
> const struct of_device_id *match;
> struct resource *res;
> struct pcs_device *pcs;
> + const struct pcs_soc_data *soc;
> int ret;
>
> match = of_match_device(pcs_of_match, &pdev->dev);
> @@ -1541,7 +1553,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;
> + soc = match->data;
> + pcs->flags = soc->flags;
>
> PCS_GET_PROP_U32("pinctrl-single,register-width", &pcs->width,
> "register width not specified\n");
> @@ -1610,7 +1623,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_HAS_PINCONF)
> pcs->desc.confops = &pcs_pinconf_ops;
> pcs->desc.owner = THIS_MODULE;
>
> @@ -1652,9 +1665,16 @@ static int pcs_remove(struct platform_device *pdev)
> return 0;
> }
>
> +static const struct pcs_soc_data pinctrl_single = {
> +};
> +
> +static const struct pcs_soc_data pinconf_single = {
> + .flags = PCS_FEAT_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);
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
On Mon, Oct 7, 2013 at 7:35 PM, Tony Lindgren <[email protected]> wrote:
> Hi Linus W,
>
> Any comments on the pinctrl patches 3 - 5 in this series?
I have no problems with this patch #3, as it is just changing syntax,
not semantics.
The problems start with patch #4.
I am tormented with mixed feelings about this, because from one point of
view I feel it is breaking the promise of pinctrl-single being a
driver for platforms
where a pin is controlled by a *single* register.
If this was pinctrl-foo.c I would not have been so much bothered,
but now it is something that was supposed to be self-contained and
simple, pertaining to a single register, starting to look like something
else.
This is a bit like: "oh yeah just one register controls the pins, but under
some circumstances I also want to mess with this register over here,
and then this register over there ..." etc.
I'd like Haojian to ACK this to proceed since he's also using this driver
now. Then I feel better on continuing down this road ...
Then I have a lesser comment on patch #4 since it makes it possible
for this pin controller to support wake-up interrupt, as I don't see how
this plays out with front-end GPIO controllers, but let's discuss that
in the context of that patch.
Yours,
Linus Walleij
* Linus Walleij <[email protected]> [131008 05:03]:
> On Mon, Oct 7, 2013 at 7:35 PM, Tony Lindgren <[email protected]> wrote:
>
> > Hi Linus W,
> >
> > Any comments on the pinctrl patches 3 - 5 in this series?
>
> I have no problems with this patch #3, as it is just changing syntax,
> not semantics.
>
> The problems start with patch #4.
>
> I am tormented with mixed feelings about this, because from one point of
> view I feel it is breaking the promise of pinctrl-single being a
> driver for platforms
> where a pin is controlled by a *single* register.
It is still in that same *single* register. There are interrupt enable
and interrupt status bits for *every* pin register on most omaps.
> If this was pinctrl-foo.c I would not have been so much bothered,
> but now it is something that was supposed to be self-contained and
> simple, pertaining to a single register, starting to look like something
> else.
>
> This is a bit like: "oh yeah just one register controls the pins, but under
> some circumstances I also want to mess with this register over here,
> and then this register over there ..." etc.
Not true. If it was some other register I would have set it up as
a separate driver under drivers/irqchip.
> I'd like Haojian to ACK this to proceed since he's also using this driver
> now. Then I feel better on continuing down this road ...
>
> Then I have a lesser comment on patch #4 since it makes it possible
> for this pin controller to support wake-up interrupt, as I don't see how
> this plays out with front-end GPIO controllers, but let's discuss that
> in the context of that patch.
It's completely separate from the GPIO controller wake-up events.
Regards,
Tony
On Tue, Oct 8, 2013 at 7:55 PM, Linus Walleij <[email protected]> wrote:
> On Mon, Oct 7, 2013 at 7:35 PM, Tony Lindgren <[email protected]> wrote:
>
>> Hi Linus W,
>>
>> Any comments on the pinctrl patches 3 - 5 in this series?
>
> I have no problems with this patch #3, as it is just changing syntax,
> not semantics.
>
> The problems start with patch #4.
>
> I am tormented with mixed feelings about this, because from one point of
> view I feel it is breaking the promise of pinctrl-single being a
> driver for platforms
> where a pin is controlled by a *single* register.
>
> If this was pinctrl-foo.c I would not have been so much bothered,
> but now it is something that was supposed to be self-contained and
> simple, pertaining to a single register, starting to look like something
> else.
>
> This is a bit like: "oh yeah just one register controls the pins, but under
> some circumstances I also want to mess with this register over here,
> and then this register over there ..." etc.
>
> I'd like Haojian to ACK this to proceed since he's also using this driver
> now. Then I feel better on continuing down this road ...
>
> Then I have a lesser comment on patch #4 since it makes it possible
> for this pin controller to support wake-up interrupt, as I don't see how
> this plays out with front-end GPIO controllers, but let's discuss that
> in the context of that patch.
>
> Yours,
> Linus Walleij
>
I'm OK on both #3 & #4. So Acked-by: Haojian Zhuang <[email protected]>
Regards
Haojian
On Mon, Oct 7, 2013 at 7:35 PM, Tony Lindgren <[email protected]> wrote:
> Hi Linus W,
>
> Any comments on the pinctrl patches 3 - 5 in this series?
Yes, after good explanations to my whimsical questions only this:
Acked-by: Linus Walleij <[email protected]>
I guess you'll take these patches through ARM SoC?
Yours,
Linus Walleij
* Linus Walleij <[email protected]> [131009 06:51]:
> On Mon, Oct 7, 2013 at 7:35 PM, Tony Lindgren <[email protected]> wrote:
>
> > Hi Linus W,
> >
> > Any comments on the pinctrl patches 3 - 5 in this series?
>
> Yes, after good explanations to my whimsical questions only this:
> Acked-by: Linus Walleij <[email protected]>
OK thanks :)
> I guess you'll take these patches through ARM SoC?
I was thinking I'll set up an immutable branch for the three
pinctrl patches against -rc4 then both you and I can merge
them in as needed. Does that work for you?
I can also do a test merge with pinctrl/for-next also to make
sure there are no conflicts.
The rest of the patches should not be needed in the pinctrl
branch, but I need them all in one branch to keep things
working.
Regards,
Tony
On Wed, Oct 9, 2013 at 5:10 PM, Tony Lindgren <[email protected]> wrote:
> I was thinking I'll set up an immutable branch for the three
> pinctrl patches against -rc4 then both you and I can merge
> them in as needed. Does that work for you?
Hm it's fair enough to have them in your tree if they do
not collide ... especially if you want to do some of my
suggested follow-up moving the ioring handling down into
pinctrl-single on top of that.
> I can also do a test merge with pinctrl/for-next also to make
> sure there are no conflicts.
Sure, as long as that works you should be safe.
Yours,
Linus Walleij
* Linus Walleij <[email protected]> [131010 08:43]:
> On Wed, Oct 9, 2013 at 5:10 PM, Tony Lindgren <[email protected]> wrote:
>
> > I was thinking I'll set up an immutable branch for the three
> > pinctrl patches against -rc4 then both you and I can merge
> > them in as needed. Does that work for you?
>
> Hm it's fair enough to have them in your tree if they do
> not collide ... especially if you want to do some of my
> suggested follow-up moving the ioring handling down into
> pinctrl-single on top of that.
OK, the PRM changes will have to wait until omap3 is DT only,
probably not until v3.14.
> > I can also do a test merge with pinctrl/for-next also to make
> > sure there are no conflicts.
>
> Sure, as long as that works you should be safe.
Seems to merge with no conflicts and build just fine. Here's
the signed tag for the three pinctrl-single patches for you
to pull in too in case you are planning to make some pinctrl
wide changes. Let's consider those commits immutable now, if
I need to fix something I will post them as separate patches.
Regards,
Tony
The following changes since commit d0e639c9e06d44e713170031fe05fb60ebe680af:
Linux 3.12-rc4 (2013-10-06 14:00:20 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap tags/pinctrl-single-for-linus-for-v3.13-signed
for you to fetch changes up to dc7743aa3c49fabbc6dc9edbcf7df74d776ac32e:
pinctrl: single: Add support for auxdata (2013-10-10 15:30:47 -0700)
----------------------------------------------------------------
Changes to pinctrl-single to allow handling the wake-up
interrupts that most omaps have in each pinctrl register.
As I need these merged also into the omap tree, it was
agreed that I set them up into a separate branch for
both pinctrl tree and linux-omap tree to merge as needed.
----------------------------------------------------------------
Tony Lindgren (3):
pinctrl: single: Prepare for supporting SoC specific features
pinctrl: single: Add support for wake-up interrupts
pinctrl: single: Add support for auxdata
.../devicetree/bindings/pinctrl/pinctrl-single.txt | 11 +
drivers/pinctrl/pinctrl-single.c | 383 ++++++++++++++++++++-
include/linux/platform_data/pinctrl-single.h | 12 +
3 files changed, 397 insertions(+), 9 deletions(-)
create mode 100644 include/linux/platform_data/pinctrl-single.h