pinctrl: qcom: add support for sparse GPIOs
Get series with b4
b4 am 20171015201811.GA13063@mithrandirVersions newest first
- v5 Latest1 of 2 patches 0 repliesNew
1 of 2 patches is in this archive. The cover letter announces 2.
Patches in v5 No applies recorded yet
| # | Subject and files | Size | Replies | Applied |
|---|---|---|---|---|
| 1/2 | pinctrl: qcom: disable GPIO groups with no pins | +0 | 1 reply |
Reviews trailers per patch
No Reviewed-by, Acked-by or Tested-by trailers for v5 yet.
Cover letter [PATCH 0/2] [v5] pinctrl: qcom: add support for sparse GPIOs
Quoted 98 lines · Timur Tabi
Quoted 3 lines · Stephen Boyd
I don't see any value
beyond system wide debug in figuring out the default pin configuration of a pin that doesn't have a consumer in Linux.I can agree with that.
Quoted 47 lines
Could we remove the pin direction finding part here in
gpiochip_add_pin_range()and lazily resolve the pin direction when a pin is requested? We would need a similar check in the msm specific debugfs code where we skip pins that aren't requested. This is basically a revert of commit72d320006177("gpio: set up initial state from .get_direction()").ACPI can still describe only the pin ranges that they care about exposing, but from the devicetree side it's been working well enough to not touch pins that aren't used by anything in Linux.
---8<----
drivers/gpio/gpiolib.c +1
-10@@ -1210,16 +1210,7 @@ int gpiochip_add_data(struct gpio_chip *chip, void *data)* wrong direction in sysfs. */ -if (chip->get_direction) { -/* -* If we have .get_direction, set up the initial -* direction flag from the hardware. -*/ -int dir = chip->get_direction (chip, i); - -if (!dir) -set_bit (FLAG_IS_OUT , &desc->flags); - }else if (!chip->direction_input) { +if (!chip->direction_input) {/* * If the chip lacks the .direction_input callback * we logically assume all lines are outputs. drivers/pinctrl/qcom/pinctrl-msm.c +1
-1@@ -24,7 +24,7 @@#include <linux/pinctrl/pinconf.h> #include <linux/pinctrl/pinconf-generic.h> #include <linux/slab.h> -#include <linux/gpio.h> +#include <linux/gpio/driver.h> #include <linux/interrupt.h> #include <linux/spinlock.h> #include <linux/reboot.h> drivers/pinctrl/qcom/pinctrl-msm.c static void msm_gpio_dbg_show_one(struct seq_file *s, · line 503
Quoted 10 lines
drivers/pinctrl/qcom/pinctrl-msm.c +6
-0@@ -494,6 +494,12 @@ static void msm_gpio_dbg_show_one(struct seq_file *s, }; g = &pctrl->soc->groups[offset]; + +if (!gpiochip_is_requested (chip, gpio)) { +seq_printf (s," %-8s:" , g->name); +return ; + } + ctl_reg =readl (pctrl->regs + g->ctl_reg); is_out = !!(ctl_reg &BIT (g->oe_bit));In order for this to work, I had to add this function from patch #1:
static int
msm_gpio_request(struct gpio_chip *chip, unsigned int offset){ struct msm_pinctrl *pctrl = gpiochip_get_data(chip); const struct msm_pingroup *g = &pctrl->soc->groups[offset]; if (!g->npins) return -ENODEV; return gpiochip_generic_request(chip, offset); }The problem with this is that none of the GPIOs are "requested", so it displays an output like this:
# cat /sys/kernel/debug/gpio gpiochip0: GPIOs 0-149, parent: platform/QCOM8002:00, QCOM8002:00: (null) : (null) : (null) : [... truncated ]] (null) : (null) : (null) : (null) : gpio36 : gpio37 : gpio38 : gpio39 : (null) : (null) :
Quoted 2 lines
It can't differentiate between GPIOs that don't exist and GPIOs that haven't been requested.
This confuses me. Why would you even want to register pins that don't exist? It sounds to me like you're lying to gpiolib and then try to work around it trying to access the GPIOs that don't exist but which you told it were there.
Why not just tell gpiolib about only the GPIOs that exist?
Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20171015/b8052700/attachment.sig>