Skip to contentSkip to conversation

Pinctrl/gpio driver for Intel Baytrail platforms

PATCH In review Mathias Nyman 5 participants 30 messages Jun 18, 2013 – May 2, 2014 Pin control

lore
Share & tools
lore
#0 lore raw

v3 cover Pinctrl/gpio driver for Intel Baytrail platforms

This is the third version of the Intel baytrail GPIO driver, now moved to Pinctrl subsystem.

As Linus W. pointed out the Intel Baytrail gpio looks more like a pin controller with pad muxing and gpio-to-pad mappings than a pure gpio controller.
Even if firmware does its best to make sure we don't need to worry about the muxings and other pinctrl features, there's a possibility these features might be useful in the future and then pinctrl is easier to build on.

Pinctrl also provides some generic gpio structures that are useful for this driver.

This driver is built on top of linux-pinctrl tree and needs the "pinctrl: add pin list based GPIO ranges" patch by Christian Ruppert (commit 2ff3477efd7086544b9e298fc63afab0645921b4)

Changes since v2:
- move driver from gpio/gpio-baytrail to drivers/pinctrl/pinctrl-baytrail
- use generic pinctrl_gpio_range structures from pinctrl instead of custom gpio_banks

Changes since v1:
- generic cleanups suggested by Andy S.
- removed text about pin muxing
- added missing 44:th pin to SUS controller
- added level triggering option
- prevent "forever loop" in case pin is stuck in "interrupt triggered" status,

Mathias Nyman (1):
pinctrl: add Intel BayTrail GPIO/pinctrl support

 drivers/pinctrl/Kconfig            |   12 +
 drivers/pinctrl/Makefile           |    1 +
 drivers/pinctrl/pinctrl-baytrail.c |  543 ++++++++++++++++++++++++++++++++++++
 3 files changed, 556 insertions(+), 0 deletions(-)
 create mode 100644 drivers/pinctrl/pinctrl-baytrail.c
Signature · 1 line
1.7.4.1
#1 lore raw

PATCH v3 1/1 pinctrl: add Intel BayTrail GPIO/pinctrl support

Add support for gpio on Intel BayTrail platforms. BayTrail supports 3 banks of gpios called SCORE, NCORE ans SUS with 102, 28 and 44 gpio pins. Supports gpio interrupts and ACPI gpio events

Pins may be muxed to alternate function instead of gpio by firmware. This driver does not touch the pin muxing and expect firmare to set pin muxing and pullup/down properties properly.

---
 drivers/pinctrl/Kconfig            |   12 +
 drivers/pinctrl/Makefile           |    1 +
 drivers/pinctrl/pinctrl-baytrail.c |  543 ++++++++++++++++++++++++++++++++++++
 3 files changed, 556 insertions(+), 0 deletions(-)
 create mode 100644 drivers/pinctrl/pinctrl-baytrail.c
drivers/pinctrl/Kconfig +12 -0
index 269c040..e01976f 100644
@@ -58,6 +58,18 @@ config PINCTRL_AT91
 	help
 	  Say Y here to enable the at91 pinctrl driver
 
+config PINCTRL_BAYTRAIL
+	bool "Intel Baytrail GPIO pin control"
+	depends on GPIOLIB && ACPI && X86
+        select IRQ_DOMAIN
+	help
+	  driver for memory mapped GPIO functionality on Intel Baytrail
+	  platforms. Supports 3 banks with 102, 28 and 44 gpios.
+	  Most pins are usually muxed to some other functionality by firmware,
+	  so only a small amount is available for gpio use.
+
+	  Requires ACPI device enumeration code to set up a platform device.
+
 config PINCTRL_BCM2835
 	bool
 	select PINMUX
drivers/pinctrl/Makefile +1 -0
index ff38aca..9031afd 100644
@@ -16,6 +16,7 @@ obj-$(CONFIG_PINCTRL_AB9540)	+= pinctrl-ab9540.o
 obj-$(CONFIG_PINCTRL_AB8505)	+= pinctrl-ab8505.o
 obj-$(CONFIG_PINCTRL_AT91)	+= pinctrl-at91.o
 obj-$(CONFIG_PINCTRL_BCM2835)	+= pinctrl-bcm2835.o
+obj-$(CONFIG_PINCTRL_BAYTRAIL)	+= pinctrl-baytrail.o
 obj-$(CONFIG_PINCTRL_IMX)	+= pinctrl-imx.o
 obj-$(CONFIG_PINCTRL_IMX35)	+= pinctrl-imx35.o
 obj-$(CONFIG_PINCTRL_IMX51)	+= pinctrl-imx51.o
drivers/pinctrl/pinctrl-baytrail.c new file +543 -0

543 lines not on this page · raw message

Signature · 1 line
1.7.4.1
#2 lore raw

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

Quoted 9 lines · Mathias Nyman

Add support for gpio on Intel BayTrail platforms. BayTrail supports 3 banks of gpios called SCORE, NCORE ans SUS with 102, 28 and 44 gpio pins. Supports gpio interrupts and ACPI gpio events

Pins may be muxed to alternate function instead of gpio by firmware. This driver does not touch the pin muxing and expect firmare to set pin muxing and pullup/down properties properly.

I have a feeling this driver will evolve quite a bit and eventually register a pure pinctrl interface as well (currently it's only using the ranges as some data container...)

Anyway, it's a good starting point and obviously (I guess?) gets your hardware up an ticking, so let's take this as a starting point.

So patch applied.

This thing only seems to use gpiolib-acpi.c for the basic device matching and IRQ handling backend, is that correct?

What I'm thinking of moving forward is that I have seen ACPI fragments with things like "PullUp" etc, which is pinctrl domain, so we may come to need some generic ACPI helpers inside drivers/pinctrl as well sooner or later.

Yours,
Linus Walleij

#3 lore raw

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

Quoted 17 lines · Linus Walleij
Quoted 9 lines · Mathias Nyman

Add support for gpio on Intel BayTrail platforms. BayTrail supports 3 banks of gpios called SCORE, NCORE ans SUS with 102, 28 and 44 gpio pins. Supports gpio interrupts and ACPI gpio events

Pins may be muxed to alternate function instead of gpio by firmware. This driver does not touch the pin muxing and expect firmare to set pin muxing and pullup/down properties properly.

I have a feeling this driver will evolve quite a bit and eventually register a pure pinctrl interface as well (currently it's only using the ranges as some data container...)

Quoted 6 lines

Anyway, it's a good starting point and obviously (I guess?) gets your hardware up an ticking, so let's take this as a starting point.

So patch applied.

Thanks, much appreciated.
We'll see how it evolves.

Quoted 2 lines

This thing only seems to use gpiolib-acpi.c for the basic device matching and IRQ handling backend, is that correct?

It only uses gpiolib-acpi.c for handling GPIO-signaled ACPI Events, which are like SCI events on hardware reduced ACPI platforms.

Basically It's ACPI saying "I have a firmware method that needs to be run if a certain gpio is triggered, please do that for me"

So the acpi_gpiochip_request_interrupts() registers interrupt handlers which call ACPI firmware methods for those gpio interrupts.

Otherwise gpiolib-acpi.c is useful for other device drivers to translate ACPI gpio resource numbers to linux gpio numbers. In ACPI tables the gpio resource numbers are per controller and zero based.

Quoted 5 lines

What I'm thinking of moving forward is that I have seen ACPI fragments with things like "PullUp" etc, which is pinctrl domain, so we may come to need some generic ACPI helpers inside drivers/pinctrl as well sooner or later.

Probably yes, I'm not an expert on ACPI (or pinctrl), but ACPI5 added the GpioInt and GpioIo resources for devices. GpioInt resource descriptor has the follwing arguments: GpioInt(EdgeLevel, ActiveLevel, Shared, PinConfig, DebounceTimeout, ResourceSource, esourceSourceIndex, ResourceUsage, DescriptorName, VendorData) {PinList}

Where the PinConfig argument can be PullDefault, PullUp, PullDown, PullNone or some vendor specific value.

ResourceSource tells which gpio controller the pin belongs to.

Right now drivers only have helpers for translating the ACPI gpio pin number to linux gpio number.

-Mathias

#4 lore raw

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

In reply to Mathias Nyman
Quoted 10 lines · Mathias Nyman

Pins may be muxed to alternate function instead of gpio by firmware. This driver does not touch the pin muxing and expect firmare to set pin muxing and pullup/down properties properly.

---
 drivers/pinctrl/Kconfig            |   12 +
 drivers/pinctrl/Makefile           |    1 +
 drivers/pinctrl/pinctrl-baytrail.c |  543 ++++++++++++++++++++++++++++++++++++

I know it's been ten months since you posted this driver, but I have a question. If this driver does not touch the pin muxing, and it doesn't even call pinctrl_register(), then why is it in drivers/pinctrl? It's not a pinctrl driver. Why isn't this a regular GPIO drivers in drivers/gpio?

Signature · 3 lines
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the
Code Aurora Forum, hosted by The Linux Foundation.
#5 lore raw

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

Quoted 14 lines · Timur Tabi
Quoted 10 lines · Mathias Nyman

Pins may be muxed to alternate function instead of gpio by firmware. This driver does not touch the pin muxing and expect firmare to set pin muxing and pullup/down properties properly.

---
  drivers/pinctrl/Kconfig            |   12 +
  drivers/pinctrl/Makefile           |    1 +
  drivers/pinctrl/pinctrl-baytrail.c |  543 ++++++++++++++++++++++++++++++++++++
Quoted 6 lines

I know it's been ten months since you posted this driver, but I have a question. If this driver does not touch the pin muxing, and it doesn't even call pinctrl_register(), then why is it in drivers/pinctrl? It's not a pinctrl driver. Why isn't this a regular GPIO drivers in drivers/gpio?

This was the conclusion we reached after some discussion with Linus W.
Initially this was just a GPIO driver, but Linus correctly spotted that Baytrail has many pinctrl-like features (like pin muxing, etc) that we might need to address in the future.

threads where this was discussed:

http://marc.info/?l=linux-kernel&m=136994203308585&w=2
http://marc.info/?l=linux-kernel&m=137113578604763&w=2

-Mathias

#6 lore raw

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

Quoted 11 lines · Mathias Nyman
Quoted 1 line

This was the conclusion we reached after some discussion with Linus W. Initially this was just a GPIO driver, but Linus correctly spotted that Baytrail has many pinctrl-like features (like pin muxing, etc) that we might need to address in the future.

threads where this was discussed:

http://marc.info/?l=linux-kernel&m=136994203308585&w=2
http://marc.info/?l=linux-kernel&m=137113578604763&w=2

So this is the interesting part:

Quoted 1 line

We expect BIOS to set all pin configurations correctly.

This device will only be used on an ACPI system, right? And isn't ACPI supposed to hide all the pinctrl programming from the OS? I thought that was the whole point behind ACPI and the reason why ARM64 isn't going to use device trees.

Signature · 3 lines
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the
Code Aurora Forum, hosted by the Linux Foundation.
#7 lore raw

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

Quoted 13 lines · Timur Tabi
Quoted 11 lines · Mathias Nyman
Quoted 1 line

This was the conclusion we reached after some discussion with Linus W. Initially this was just a GPIO driver, but Linus correctly spotted that Baytrail has many pinctrl-like features (like pin muxing, etc) that we might need to address in the future.

threads where this was discussed:

http://marc.info/?l=linux-kernel&m=136994203308585&w=2
http://marc.info/?l=linux-kernel&m=137113578604763&w=2

Quoted 9 lines

So this is the interesting part:

Quoted 1 line

We expect BIOS to set all pin configurations correctly.

This device will only be used on an ACPI system, right? And isn't ACPI supposed to hide all the pinctrl programming from the OS? I thought that was the whole point behind ACPI and the reason why ARM64 isn't going to use device trees.

This was my starting point as well, and the driver was initially submitted as a GPIO driver. But Linus W. suggested pinctrl instead, and as he's the maintainer of both those subsystem I trust his judgment.

-Mathias

#8 lore raw

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

Quoted 10 lines · Mathias Nyman
Quoted 6 lines

This device will only be used on an ACPI system, right? And isn't ACPI supposed to hide all the pinctrl programming from the OS? I thought that was the whole point behind ACPI and the reason why ARM64 isn't going to use device trees.

This was my starting point as well, and the driver was initially submitted as a GPIO driver. But Linus W. suggested pinctrl instead, and as he's the maintainer of both those subsystem I trust his judgment.

Do you think, for an ACPI pinctrl driver, that we will need to specify any function groups? When I look at the ASL that configures GPIOs, I see only lines like this:

GpioIo (Exclusive, PullDefault, , , , "\\GIO0") {0x1D, 0x1E}

This tells me that ACPI will never use any of the names that are defined. I see that .get_function_name is called on my ACPI system, but I don't see where it is used.

The reason I ask is because I would like to make a "generic" ACPI pinctrl/gpio driver that doesn't specify any pin groups. So if we use the same pinctrl/gpio hardware on multiple SOCs, the only thing that the driver needs from ACPI is the number of pins.

Signature · 3 lines
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the
Code Aurora Forum, hosted by the Linux Foundation.
#9 lore raw

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

Quoted 18 lines · Timur Tabi
Quoted 10 lines · Mathias Nyman
Quoted 6 lines

This device will only be used on an ACPI system, right? And isn't ACPI supposed to hide all the pinctrl programming from the OS? I thought that was the whole point behind ACPI and the reason why ARM64 isn't going to use device trees.

This was my starting point as well, and the driver was initially submitted as a GPIO driver. But Linus W. suggested pinctrl instead, and as he's the maintainer of both those subsystem I trust his judgment.

Do you think, for an ACPI pinctrl driver, that we will need to specify any function groups? When I look at the ASL that configures GPIOs, I see only lines like this:

GpioIo (Exclusive, PullDefault, , , , "\\GIO0") {0x1D, 0x1E}

Quoted 9 lines

This tells me that ACPI will never use any of the names that are defined. I see that .get_function_name is called on my ACPI system, but I don't see where it is used.

The reason I ask is because I would like to make a "generic" ACPI pinctrl/gpio driver that doesn't specify any pin groups. So if we use the same pinctrl/gpio hardware on multiple SOCs, the only thing that the driver needs from ACPI is the number of pins.

Mika, Rafael (and me) have done some effort already on supporting ACPI gpio resources.

ACPI code creates a platfrom device out of the GPIO device defined ACPI tables, with all the resources set, and a hook back to the ACPI node.

Helper functions to translate the ACPI GpioIO and GpioInt resources to linux gpio numbers can be found in gpio/gpiolib-acpi.c together with other ACPI and gpio related helper functions.

see also
Documentation/acpi/enumeration.txt for some GPIO and ACPI related info

-Mathias

#10 lore raw

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

Quoted 4 lines · Mathias Nyman

Helper functions to translate the ACPI GpioIO and GpioInt resources to linux gpio numbers can be found in gpio/gpiolib-acpi.c together with other ACPI and gpio related helper functions.

Does this also cover pin control and pin muxing? Sorry, but I sometimes I get confused. It appears to me that the GpioIo and GpioInt functions require pin muxing, but I don't see that support in gpiolib-acpi.

Signature · 3 lines
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the
Code Aurora Forum, hosted by The Linux Foundation.
#11 lore raw

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

Quoted 10 lines · Timur Tabi
Quoted 4 lines · Mathias Nyman

Helper functions to translate the ACPI GpioIO and GpioInt resources to linux gpio numbers can be found in gpio/gpiolib-acpi.c together with other ACPI and gpio related helper functions.

Does this also cover pin control and pin muxing? Sorry, but I sometimes I get confused. It appears to me that the GpioIo and GpioInt functions require pin muxing, but I don't see that support in gpiolib-acpi.

It doesn't do any pin control nor muxing and I'm not sure if it is required. Can you elaborate why you think pin muxing is required with GpioIo/GpioInt resources?

#12 lore raw

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

Quoted 3 lines · Mika

It doesn't do any pin control nor muxing and I'm not sure if it is required. Can you elaborate why you think pin muxing is required with GpioIo/GpioInt resources?

How are the pin muxes normally configured in ACPI? All of our GPIOs have a pinmux on them, and so if you want to use the pin for the non-default functionality, you need to configure the mux. Isn't that supposed to happen with the through the pinctrl driver? That is, when the kernel parses the ASL, and it seems a command to configure pin #3 to function #4, it calls the local pinctrl driver to do that?

Signature · 3 lines
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the
Code Aurora Forum, hosted by The Linux Foundation.
#13 lore raw

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

Quoted 1 line · Timur Tabi

How are the pin muxes normally configured in ACPI?

VERY good question.

Quoted 6 lines

All of our GPIOs have a
pinmux on them, and so if you want to use the pin for the non-default functionality, you need to configure the mux. Isn't that supposed to happen with the through the pinctrl driver? That is, when the kernel parses the ASL, and it seems a command to configure pin #3 to function #4, it calls the local pinctrl driver to do that?

Let's suspect this is true. That is how GPIO work after all is it not?

I guess the other option would be that all systems must come with a tailored set-up written in ASL, specific for each board configuration.

Yours,
Linus Walleij

#14 lore raw

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

Quoted 7 lines · Linus Walleij
Quoted 6 lines
Quoted 6 lines

All of our GPIOs have a
pinmux on them, and so if you want to use the pin for the non-default functionality, you need to configure the mux. Isn't that supposed to happen with the through the pinctrl driver? That is, when the kernel parses the ASL, and it seems a command to configure pin #3 to function #4, it calls the local pinctrl driver to do that?

Let's suspect this is true. That is how GPIO work after all is it not?

With device trees, function groups are defined by name in the driver, and the device tree contains these same names as links between nodes. This works well.

In ACPI, it appears that no such infrastructure exists.

Quoted 2 lines

I guess the other option would be that all systems must come with a tailored set-up written in ASL, specific for each board configuration.

That makes sense (U-Boot does fixups of the device tree based on actual board config, so it's conceivable that UEFI could do the same for ASL).
However, that would require either

1) UEFI programs the GPIO muxes directly, without specifying any mux information in ASL and without using the Linux driver to program the muxes.

or

2) ASL contains code to program the muxes, and Linux parses that ASL and calls the pinctrl driver to program the hardware.

I don't know which direction the Community wants to use. If we go with option #1, then there is no need for the pinctrl driver to specify function groups at all. This means we would not need a separate pinctrl driver for each SOC, like we do today.

Signature · 3 lines
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the
Code Aurora Forum, hosted by The Linux Foundation.
#15 lore raw

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

In reply to Linus Walleij
Quoted 5 lines · Linus Walleij
Quoted 1 line · Timur Tabi

How are the pin muxes normally configured in ACPI?

VERY good question.

Typically this is done by the boot firmware (BIOS in this case). So the OS doesn't need to deal with that.

AFAICT ACPI doesn't know anything about pin muxing.

#16 lore raw

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

Quoted 4 lines · Mika

Typically this is done by the boot firmware (BIOS in this case). So the OS doesn't need to deal with that.

AFAICT ACPI doesn't know anything about pin muxing.

In that case, would it be correct to say that a Linux pinctrl driver for an ACPI-only platform should not define pinmux function groups, because they'll never be used?

Signature · 3 lines
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the
Code Aurora Forum, hosted by The Linux Foundation.
#17 lore raw

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

Quoted 9 lines · Timur Tabi
Quoted 4 lines · Mika

Typically this is done by the boot firmware (BIOS in this case). So the OS doesn't need to deal with that.

AFAICT ACPI doesn't know anything about pin muxing.

In that case, would it be correct to say that a Linux pinctrl driver for an ACPI-only platform should not define pinmux function groups, because they'll never be used?

No because you can have a GPIO controller on ACPI machine that allows you to do some sort of pin muxing. What I'm saying is that ACPI itself doesn't have such knowledge and hence we don't support such things in gpiolib-acpi.c.

#18 lore raw

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

In reply to Timur Tabi
Quoted 9 lines · Timur Tabi
Quoted 3 lines · Mika

It doesn't do any pin control nor muxing and I'm not sure if it is required. Can you elaborate why you think pin muxing is required with GpioIo/GpioInt resources?

How are the pin muxes normally configured in ACPI? All of our GPIOs have a pinmux on them, and so if you want to use the pin for the non-default functionality, you need to configure the mux. Isn't that supposed to happen with the through the pinctrl driver?

It's the BIOS that handles all this even though there are several "alternate functions" in the GPIO hardware. BIOS goes and configures those according what the platform needs and those that are GPIOs/IRQs it will create corresponding GpioIo/GpioInt along with the device that uses them.

Of course if you have a custom board and your BIOS doesn't handle this, you are going to need some sort of pinctrl driver.

Quoted 3 lines

That is, when the kernel parses the ASL, and it seems a command to configure pin #3 to function #4, it calls the local pinctrl driver to do that?

I'm not aware of ASL code that allows you to do that. Do you have examples?

#19 lore raw

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

Quoted 5 lines · Mika
Quoted 3 lines
Quoted 3 lines

That is, when the kernel parses the ASL, and it seems a command to configure pin #3 to function #4, it calls the local pinctrl driver to do that?

I'm not aware of ASL code that allows you to do that. Do you have examples?

No, that's my point. I was expecting the pinmux functions of the pinctrl driver are used by ACPI, but apparently they aren't, and that's why I'm asking. I'm wondering why a pinctrl driver for an ACPI platform should be defining pinmux function groups. I haven't gotten a straight answer to that question.

Signature · 3 lines
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the
Code Aurora Forum, hosted by The Linux Foundation.
#20 lore raw

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

Quoted 10 lines · Timur Tabi
Quoted 5 lines · Mika
Quoted 3 lines
Quoted 3 lines

That is, when the kernel parses the ASL, and it seems a command to configure pin #3 to function #4, it calls the local pinctrl driver to do that?

I'm not aware of ASL code that allows you to do that. Do you have examples?

No, that's my point. I was expecting the pinmux functions of the pinctrl driver are used by ACPI, but apparently they aren't, and that's why I'm asking.

Which functions?

Quoted 3 lines

I'm wondering why a pinctrl driver for an
ACPI platform should be defining pinmux function groups. I haven't gotten a straight answer to that question.

Are you asking why pinctrl-baytrail.c uses pinctrl_gpio_ranges even if it is a pretty much standard GPIO driver? Mathias probably can answer that question better than me.

#21 lore raw

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

Quoted 9 lines · Mika
Quoted 3 lines · Timur Tabi

I'm wondering why a pinctrl driver for an
ACPI platform should be defining pinmux function groups. I haven't gotten a straight answer to that question.

Are you asking why pinctrl-baytrail.c uses pinctrl_gpio_ranges even if it is a pretty much standard GPIO driver? Mathias probably can answer that question better than me.

That was done on my request since any other approach obviously would lead to code duplication of exactly what GPIO ranges does.

Yours,
Linus Walleij

#22 lore raw

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

In reply to Westerberg, Mika
Quoted 5 lines · Mika
Quoted 3 lines
Quoted 3 lines

No, that's my point. I was expecting the pinmux functions of the pinctrl driver are used by ACPI, but apparently they aren't, and that's why I'm asking.

Which functions?

The functions in struct pinmux_ops, like get_function_groups. Will these functions ever be called on an ACPI system?

Signature · 3 lines
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the
Code Aurora Forum, hosted by the Linux Foundation.
#23 lore raw

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

Quoted 9 lines · Timur Tabi
Quoted 5 lines · Mika
Quoted 3 lines
Quoted 3 lines

No, that's my point. I was expecting the pinmux functions of the pinctrl driver are used by ACPI, but apparently they aren't, and that's why I'm asking.

Which functions?

The functions in struct pinmux_ops, like get_function_groups. Will these functions ever be called on an ACPI system?

Well, if you have an ACPI system (like normal PC) it is perfectly fine to have pin controller/mux hardware there which is not dependent at all on ACPI.

If you happen to have pin controller/mux driver that drives that hardware, I'm sure your pinmux functions gets called.

#24 lore raw

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

Quoted 11 lines · Mika
Quoted 9 lines · Timur Tabi
Quoted 5 lines · Mika
Quoted 3 lines
Quoted 3 lines

No, that's my point. I was expecting the pinmux functions of the pinctrl driver are used by ACPI, but apparently they aren't, and that's why I'm asking.

Which functions?

The functions in struct pinmux_ops, like get_function_groups. Will these functions ever be called on an ACPI system?

Quoted 6 lines

Well, if you have an ACPI system (like normal PC) it is perfectly fine to have pin controller/mux hardware there which is not dependent at all on ACPI.

If you happen to have pin controller/mux driver that drives that hardware, I'm sure your pinmux functions gets called.

True as far as the driver itself goes.

Pin controllers are however configured into a certain state with platform data or device tree data. This configuration would need to be stored in ACPI to fill the equivalent role in such a system, and that in turn implies some thought given to it during design of any ACPI-pinctrl bindings.

Yours,
Linus Walleij

#25 lore raw

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

In reply to Westerberg, Mika
Quoted 2 lines · Mika

If you happen to have pin controller/mux driver that drives that hardware, I'm sure your pinmux functions gets called.

Actually, I don't think they do. On a device-tree system, the functions get called automatically by the pinctrl layer when it parses the device tree. This happens in response to probe, remove, and various power management changes.

AFAIK, there is nothing at all like that for ACPI systems. Do you have an example of a driver making pinmux calls directly?

Signature · 3 lines
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the
Code Aurora Forum, hosted by The Linux Foundation.
#26 lore raw

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

Quoted 6 lines · Timur Tabi
Quoted 2 lines · Mika

If you happen to have pin controller/mux driver that drives that hardware, I'm sure your pinmux functions gets called.

Actually, I don't think they do. On a device-tree system, the functions get called automatically by the pinctrl layer when it parses the device tree. This happens in response to probe, remove, and various power management changes.

I would be interested in understanding what exactly the flow is in that situation, so care to educate me? What does the driver do to trigger this and what exactly does happen in response to that?

Rafael

#27 lore raw

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

Quoted 5 lines · Rafael J. Wysocki
Quoted 1 line

I would be interested in understanding what exactly the flow is in that situation, so care to educate me? What does the driver do to trigger this and what exactly does happen in response to that?

I only just learned some of this myself, so I'm no expert. My understanding is that the all of the pinctrl-* properties and nodes are scanned by the pinctrl layer itself. So you could have a SATA controller node that points to a pin control node (via phandles). When the SATA driver is probed, the pinctrl layer notices the phandles and automatically calls the pinctrl layer to configure the pins and pin muxes.

Signature · 3 lines
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the
Code Aurora Forum, hosted by The Linux Foundation.
#28 lore raw

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

Quoted 7 lines · Timur Tabi
Quoted 4 lines · Rafael J. Wysocki

I would be interested in understanding what exactly the flow is in that situation, so care to educate me? What does the driver do to trigger this and what exactly does happen in response to that?

Quoted 6 lines

I only just learned some of this myself, so I'm no expert. My understanding is that the all of the pinctrl-* properties and nodes are scanned by the pinctrl layer itself. So you could have a SATA controller node that points to a pin control node (via phandles). When the SATA driver is probed, the pinctrl layer notices the phandles and automatically calls the pinctrl layer to configure the pins and pin muxes.

There is a global pin control map for the system spanning all possible pin controller instances.

Right before a device probe() function is called, the device core will attempt to grab and activate a pin control setting tied to this device and named "default". (It can also handle some PM states.)

The table of states can come from:
(a) Platform data or
(b) Device tree

And if there was an ACPI pin controller it would be case (c) and need to have intelligent bindings allowing such a table to be built so that the device core can make use of it.

When the different states (such as "default") are enabled, this results in calls into the pin control driver. In most cases that ends up with simple register writes but I guess an ACPI driver would result in calling some esoteric bytecode or whatever or both.

There is no escape from the fact that this needs being handled from the pin control subsystem though, it can't be sneaked into the ACPI core or something. You may want to add new states apart from the ones defined in include/linux/pinctrl/pinctrl-state.h, as I know ACPI is a bit picky about it's states and what they are named (D-states right?)

Yours,
Linus Walleij (pretending to understand ACPI)

#29 lore raw

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

In reply to Timur Tabi
Quoted 5 lines · Timur Tabi

I know it's been ten months since you posted this driver, but I have a question. If this driver does not touch the pin muxing, and it doesn't even call pinctrl_register(), then why is it in drivers/pinctrl? It's not a pinctrl driver. Why isn't this a regular GPIO drivers in drivers/gpio?

- It uses the GPIO ranges concept in the pinctrl framework, because it internally actually deals with pin numbers, not GPIO numbers or predictable offsets. This way the basic pin control properties that ACPI should "hide" have already leaked to the driver, see score_pins[], ncore_pins[] etc.

- Pin control is not only about muxing, it is also about electrical config, see e.g. include/linux/pinctrl/pinconf-generic.h ACPI contain very unnerving stuff like PullDefault, PullUp, etc. In Linux terms that stuff is NOT GPIO, it is pin control. That ACPI choose to call it "GPIO" does not matter one bit. Now I am informed that this is read-only information, yet you may want to model it to Linux in the end, and then pin control provides the right infrastructure.

- Since the driver already knows this, it can be augmented to register a pin controller struct and display the actual names of all the pins in debugfs. And the pull state of each maybe?

- The magic muxing and biasing etc registers obviously exist in the register range of this IP block. Someone may start hacking to do fun stuff. Like bypass ACPI when it's doing something wrong. (Oh that never would happen, right?) Then the driver is in the right place to start hacking.

Yours,
Linus Walleij