pinctrl: common handling of generic pinconfig props in dt
Subsystems (3)
Heiko Stübner
maintainercover pinctrl: common handling of generic pinconfig props in dt
Hi Linus,
following your suggestions for a common handling of things like pulls in dt, I've come up with the following solution - hopefully I've gotten the correct meaning of your explanaitions.
It handles all the pinconfigs that either ignore the argument, or have very simple one, like PIN_CONFIG_OUTPUT does.
I've also attached my current rockchip pinctrl driver to show how it is using this currently.
Heiko Stuebner (2):
pinctrl: add devicetree constants for simple generic pnconfig options
pinctrl: add function to separate combined pinconfig values
.../bindings/pinctrl/pinctrl-bindings.txt | 9 +++ drivers/pinctrl/pinconf-generic.c | 61 ++++++++++++++++++++ drivers/pinctrl/pinconf.h | 6 ++ include/dt-bindings/pinctrl/pinconfig.h | 37 ++++++++++++ 4 files changed, 113 insertions(+), 0 deletions(-) create mode 100644 include/dt-bindings/pinctrl/pinconfig.h
Signature · 1 line
1.7.2.3
Heiko Stübner
maintainerPATCH 1/2 pinctrl: add devicetree constants for simple generic pnconfig options
Most pinconfig options either ignore the argument or only have two states for them. Therefore it's possible to combine them into one value in dt bindings as bits, resulting in smaller pin definitions.
--- .../bindings/pinctrl/pinctrl-bindings.txt | 6 +++ include/dt-bindings/pinctrl/pinconfig.h | 37 ++++++++++++++++++++ 2 files changed, 43 insertions(+), 0 deletions(-) create mode 100644 include/dt-bindings/pinctrl/pinconfig.h
Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt +6 -0
@@ -126,3 +126,9 @@ device; they may be grandchildren, for example. Whether this is legal, and whether there is any interaction between the child and intermediate parent nodes, is again defined entirely by the binding for the individual pin controller device. + +== Generic pinctrl config settings == + +dt-bindings/pinctrl/pinconfig.h defines a set of constants to combine basic +generic pinconfig settings, like pulls, into one value, that can be used +in pinctrl bindings like <bank pin mux CONFIG>.
include/dt-bindings/pinctrl/pinconfig.h new file +37 -0
@@ -0,0 +1,37 @@ +/* +* Header providing constants for generic pinconf bindings. +* +* Copyright (c) 2013 MundoReader S.L. +* Author: Heiko Stuebner <[email protected]> +* +* 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 program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +*/ + +#ifndef __DT_BINDINGS_PINCTRL_PINCONFIG_H__ +#define __DT_BINDINGS_PINCTRL_PINCONFIG_H__ + +#define PINCONFIG_NONE 0 +#define PINCONFIG_BIAS_DISABLE (1 << 0) +#define PINCONFIG_BIAS_HIGH_IMPEDANCE (1 << 1) +#define PINCONFIG_BIAS_BUS_HOLD (1 << 2) +#define PINCONFIG_BIAS_PULL_PIN_DEFAULT (1 << 3) +#define PINCONFIG_BIAS_PULL_UP (1 << 4) +#define PINCONFIG_BIAS_PULL_DOWN (1 << 5) +#define PINCONFIG_DRIVE_PUSH_PULL (1 << 6) +#define PINCONFIG_DRIVE_OPEN_DRAIN (1 << 7) +#define PINCONFIG_DRIVE_OPEN_SOURCE (1 << 8) +#define PINCONFIG_INPUT_SCHMITT_ENABLE (1 << 9) +#define PINCONFIG_INPUT_SCHMITT_DISABLE (1 << 10) +#define PINCONFIG_LOW_POWER_MODE (1 << 11) +#define PINCONFIG_OUTPUT_LOW (1 << 12) +#define PINCONFIG_OUTPUT_HIGH (1 << 13) + +#endif
Signature · 1 line
1.7.2.3
Heiko Stübner
maintainerPATCH 2/2 pinctrl: add function to separate combined pinconfig values
In reply to Heiko StübnerThe previous patch introduced constants to combine pinconfig settings into one value for easier devicetree handling.
Therefore also add a function, that can separate these bitmaps into regular generic pinconfig options for handling inside pinctrl drivers.
--- .../bindings/pinctrl/pinctrl-bindings.txt | 3 + drivers/pinctrl/pinconf-generic.c | 61 ++++++++++++++++++++ drivers/pinctrl/pinconf.h | 6 ++ 3 files changed, 70 insertions(+), 0 deletions(-)
Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt +3 -0
@@ -132,3 +132,6 @@ controller device. dt-bindings/pinctrl/pinconfig.h defines a set of constants to combine basic generic pinconfig settings, like pulls, into one value, that can be used in pinctrl bindings like <bank pin mux CONFIG>. + +This value can then be split into individual generic pinconfig values in the +driver using pinconf_generic_parse_dt_bitmap.
drivers/pinctrl/pinconf-generic.c +61 -0
64 lines not on this page · raw message
drivers/pinctrl/pinconf.h +6 -0
@@ -123,3 +123,9 @@ static inline void pinconf_generic_dump_config(struct pinctrl_dev *pctldev, return; }#endif + +#ifdef CONFIG_GENERIC_PINCONF +int pinconf_generic_parse_dt_bitmap (unsigned long pinconf, +unsigned long **configs, +unsigned int *nconfigs); +#endif
Signature · 1 line
1.7.2.3
Heiko Stübner
maintainerPATCH 1/1 pinctrl: add pinctrl driver for Rockchip SoCs
In reply to Heiko StübnerThis driver adds support the Cortex-A9 based SoCs from Rockchip, so at least the RK2928, RK3066 (a and b) and RK3188. Earlier Rockchip SoCs seem to use similar mechanics for gpio handling so should be supportable with relative small changes. Pull handling on the rk3188 is currently a stub, due to it being a bit different to the earlier SoCs.
Pinmuxing as well as gpio (and interrupt-) handling tested on a rk3066a based machine.
--- .../bindings/pinctrl/rockchip,pinctrl.txt | 94 ++ drivers/pinctrl/Kconfig | 6 + drivers/pinctrl/Makefile | 1 + drivers/pinctrl/pinctrl-rockchip.c | 1357 ++++++++++++++++++++ include/dt-bindings/pinctrl/rockchip.h | 32 + 5 files changed, 1490 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.txt create mode 100644 drivers/pinctrl/pinctrl-rockchip.c create mode 100644 include/dt-bindings/pinctrl/rockchip.h
Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.txt new file +94 -0
@@ -0,0 +1,94 @@ +* Rockchip Pinmux Controller + +The Rockchip Pinmux Controller, enables the IC +to share one PAD to several functional blocks. The sharing is done by +multiplexing the PAD input/output signals. For each PAD there are up to +4 muxing options with option 0 being the use as a GPIO. + +Please refer to pinctrl-bindings.txt in this directory for details of the +common pinctrl bindings used by client devices, including the meaning of the +phrase "pin configuration node". + +The Rockchip pin configuration node is a node of a group of pins which can be +used for a specific device or function. This node represents both mux and +config of the pins in that group. The 'pins' selects the function mode(also +named pin mode) this pin can work on and the 'config' configures various pad +settings such as pull-up, etc. + +The pins are grouped into up to 5 individual pin banks which need to be +defined as gpio sub-nodes of the pinmux controller. + +Required properties for iomux controller: + - compatible: one of "rockchip,rk2928-pinctrl", "rockchip,rk3066a-pinctrl" + "rockchip,rk3066b-pinctrl", "rockchip,rk3188-pinctrl" + +Required properties for gpio sub nodes: + - compatible: "rockchip,gpio-bank" + - reg: register of the gpio bank (different than the iomux registerset) + - interrupts: base interrupt of the gpio bank in the interrupt controller + - clocks: clock that drives this bank + - gpio-controller: identifies the node as a gpio controller and pin bank. + - #gpio-cells: number of cells in GPIO specifier. Since the generic GPIO + binding is used, the amount of cells must be specified as 2. See generic + GPIO binding documentation for description of particular cells. + - interrupt-controller: identifies the controller node as interrupt-parent. + - #interrupt-cells: the value of this property should be 2 and the interrupt + cells should use the standard two-cell scheme described in + bindings/interrupt-controller/interrupts.txt + +Required properties for pin configuration node: + - rockchip,pins: 4 integers array, represents a group of pins mux and config + setting. The format is rockchip,pins = <PIN_BANK PIN_BANK_IDX MUX CONFIG>. + The MUX 0 means gpio and MUX 1 to 3 mean the specific device function. + CONFIG contains or-ed bits from the generic pinconfig constants from + dt-bindings/pinctrl/pinconfig.h + +Examples: + +#include <dt-bindings/pinctrl/pinconfig.h> +#include <dt-bindings/pinctrl/rockchip.h> + +... + +pinctrl at 20008000 { + compatible = "rockchip,rk3066a-pinctrl"; + reg = <0x20008000 0x150>; + #address-cells = <1>; + #size-cells = <1>; + ranges; + + gpio0: gpio0 at 20034000 { + compatible = "rockchip,gpio-bank"; + reg = <0x20034000 0x100>; + interrupts = <GIC_SPI 54 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&clk_gates8 9>; + + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + ... + + uart2 { + uart2_xfer: uart2-xfer { + rockchip,pins = <RK_GPIO1 8 1 PINCONFIG_BIAS_PULL_PIN_DEFAULT>, + <RK_GPIO1 9 1 PINCONFIG_BIAS_PULL_PIN_DEFAULT>; + }; + }; +}; + +uart2: serial at 20064000 { + compatible = "snps,dw-apb-uart"; + reg = <0x20064000 0x400>; + interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>; + reg-shift = <2>; + reg-io-width = <1>; + clocks = <&mux_uart2>; + status = "okay"; + + pinctrl-names = "default"; + pinctrl-0 = <&uart2_xfer>; +};
drivers/pinctrl/Kconfig +6 -0
@@ -158,6 +158,12 @@ config PINCTRL_DB8540 bool "DB8540 pin controller driver" depends on PINCTRL_NOMADIK && ARCH_U8500 +config PINCTRL_ROCKCHIP + bool + select PINMUX + select GENERIC_PINCONF + select GENERIC_IRQ_CHIP + config PINCTRL_SINGLE tristate "One-register-per-pin type device tree based pinctrl driver" depends on OF
drivers/pinctrl/Makefile +1 -0
@@ -31,6 +31,7 @@ obj-$(CONFIG_PINCTRL_NOMADIK) += pinctrl-nomadik.o obj-$(CONFIG_PINCTRL_STN8815) += pinctrl-nomadik-stn8815.o obj-$(CONFIG_PINCTRL_DB8500) += pinctrl-nomadik-db8500.o obj-$(CONFIG_PINCTRL_DB8540) += pinctrl-nomadik-db8540.o +obj-$(CONFIG_PINCTRL_ROCKCHIP) += pinctrl-rockchip.o obj-$(CONFIG_PINCTRL_SINGLE) += pinctrl-single.o obj-$(CONFIG_PINCTRL_SIRF) += sirf/ obj-$(CONFIG_PINCTRL_SUNXI) += pinctrl-sunxi.o
drivers/pinctrl/pinctrl-rockchip.c new file +1357 -0
1,357 lines not on this page · raw message
include/dt-bindings/pinctrl/rockchip.h new file +32 -0
@@ -0,0 +1,32 @@ +/* +* Header providing constants for Rockchip pinctrl bindings. +* +* Copyright (c) 2013 MundoReader S.L. +* Author: Heiko Stuebner <[email protected]> +* +* 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 program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +*/ + +#ifndef __DT_BINDINGS_ROCKCHIP_PINCTRL_H__ +#define __DT_BINDINGS_ROCKCHIP_PINCTRL_H__ + +#define RK_GPIO0 0 +#define RK_GPIO1 1 +#define RK_GPIO2 2 +#define RK_GPIO3 3 +#define RK_GPIO4 4 +#define RK_GPIO6 6 + +#define RK_FUNC_GPIO 0 +#define RK_FUNC_1 1 +#define RK_FUNC_2 2 + +#endif
Signature · 1 line
1.7.2.3
Re: [EXAMPLE PATCH] pinctrl: add pinctrl driver for Rockchip SoCs
Quoted 11 lines · Heiko St?bner
This driver adds support the Cortex-A9 based SoCs from Rockchip, so at least the RK2928, RK3066 (a and b) and RK3188. Earlier Rockchip SoCs seem to use similar mechanics for gpio handling so should be supportable with relative small changes. Pull handling on the rk3188 is currently a stub, due to it being a bit different to the earlier SoCs.
Pinmuxing as well as gpio (and interrupt-) handling tested on a rk3066a based machine.
Signed-off-by Heiko Stuebner <[email protected]>
Patch applied.
You've been super-helpful, so of course I'm happy to merge this into the pinctrl tree.
Yours,
Linus Walleij
Re: [EXAMPLE PATCH] pinctrl: add pinctrl driver for Rockchip SoCs
In reply to Heiko StübnerOn Sun, Jun 9, 2013 at 2:01 AM, Heiko St?bner <[email protected]> wrote:
(...)
drivers/pinctrl/pinctrl-rockchip.c line 40
Quoted 3 lines
drivers/pinctrl/pinctrl-rockchip.c new file +3
-0+#include <linux/irqchip/chained_irq.h> +#include <linux/clk-provider.h> +#include <dt-bindings/pinctrl/rockchip.h>
Hm I don't know if we're supposed to do things like this actually.
It depends on whether the <dt-bindings/*> hierarchy is supposed to stay in the kernel after the device trees are moved out to a separate repo.
Grant/Rob, shall we do this in drivers?
Yours,
Linus Walleij
Heiko Stübner
maintainerRe: [EXAMPLE PATCH] pinctrl: add pinctrl driver for Rockchip SoCs
Am Montag, 10. Juni 2013, 15:00:41 schrieb Linus Walleij:
Quoted 10 lines · Heiko St?bner
(...)
drivers/pinctrl/pinctrl-rockchip.c line 40
Quoted 3 lines
drivers/pinctrl/pinctrl-rockchip.c new file +3
-0+#include <linux/irqchip/chained_irq.h> +#include <linux/clk-provider.h> +#include <dt-bindings/pinctrl/rockchip.h> Hm I don't know if we're supposed to do things like this actually.
Quoted 5 lines
It depends on whether the <dt-bindings/*> hierarchy is supposed to stay in the kernel after the device trees are moved out to a separate repo.
Grant/Rob, shall we do this in drivers?
we can get rid of it, if it's controversial.
The only constant that gets used is RK_FUNC_GPIO in rockchip_pmx_gpio_set_direction and rockchip_irq_set_type to not have arbitary "0"-values when switching to the gpio function.
So it wouldn't be problem to simply keep this constant in the driver itself instead of including the dt-binding constants.
Heiko Stübner
maintainerPATCH v2 1/1 pinctrl: add pinctrl driver for Rockchip SoCs
This driver adds support the Cortex-A9 based SoCs from Rockchip, so at least the RK2928, RK3066 (a and b) and RK3188. Earlier Rockchip SoCs seem to use similar mechanics for gpio handling so should be supportable with relative small changes. Pull handling on the rk3188 is currently a stub, due to it being a bit different to the earlier SoCs.
Pinmuxing as well as gpio (and interrupt-) handling tested on a rk3066a based machine.
---
changes since v1:
don't include the dt-bindings header in the driver
Only the RK_FUNC_GPIO constant was used anyway, so it shouldn't be a problem to redefine this one constant inside the driver
.../bindings/pinctrl/rockchip,pinctrl.txt | 94 ++ drivers/pinctrl/Kconfig | 6 + drivers/pinctrl/Makefile | 1 + drivers/pinctrl/pinctrl-rockchip.c | 1359 ++++++++++++++++++++ include/dt-bindings/pinctrl/rockchip.h | 32 + 5 files changed, 1492 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.txt create mode 100644 drivers/pinctrl/pinctrl-rockchip.c create mode 100644 include/dt-bindings/pinctrl/rockchip.h
Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.txt new file +94 -0
@@ -0,0 +1,94 @@ +* Rockchip Pinmux Controller + +The Rockchip Pinmux Controller, enables the IC +to share one PAD to several functional blocks. The sharing is done by +multiplexing the PAD input/output signals. For each PAD there are up to +4 muxing options with option 0 being the use as a GPIO. + +Please refer to pinctrl-bindings.txt in this directory for details of the +common pinctrl bindings used by client devices, including the meaning of the +phrase "pin configuration node". + +The Rockchip pin configuration node is a node of a group of pins which can be +used for a specific device or function. This node represents both mux and +config of the pins in that group. The 'pins' selects the function mode(also +named pin mode) this pin can work on and the 'config' configures various pad +settings such as pull-up, etc. + +The pins are grouped into up to 5 individual pin banks which need to be +defined as gpio sub-nodes of the pinmux controller. + +Required properties for iomux controller: + - compatible: one of "rockchip,rk2928-pinctrl", "rockchip,rk3066a-pinctrl" + "rockchip,rk3066b-pinctrl", "rockchip,rk3188-pinctrl" + +Required properties for gpio sub nodes: + - compatible: "rockchip,gpio-bank" + - reg: register of the gpio bank (different than the iomux registerset) + - interrupts: base interrupt of the gpio bank in the interrupt controller + - clocks: clock that drives this bank + - gpio-controller: identifies the node as a gpio controller and pin bank. + - #gpio-cells: number of cells in GPIO specifier. Since the generic GPIO + binding is used, the amount of cells must be specified as 2. See generic + GPIO binding documentation for description of particular cells. + - interrupt-controller: identifies the controller node as interrupt-parent. + - #interrupt-cells: the value of this property should be 2 and the interrupt + cells should use the standard two-cell scheme described in + bindings/interrupt-controller/interrupts.txt + +Required properties for pin configuration node: + - rockchip,pins: 4 integers array, represents a group of pins mux and config + setting. The format is rockchip,pins = <PIN_BANK PIN_BANK_IDX MUX CONFIG>. + The MUX 0 means gpio and MUX 1 to 3 mean the specific device function. + CONFIG contains or-ed bits from the generic pinconfig constants from + dt-bindings/pinctrl/pinconfig.h + +Examples: + +#include <dt-bindings/pinctrl/pinconfig.h> +#include <dt-bindings/pinctrl/rockchip.h> + +... + +pinctrl at 20008000 { + compatible = "rockchip,rk3066a-pinctrl"; + reg = <0x20008000 0x150>; + #address-cells = <1>; + #size-cells = <1>; + ranges; + + gpio0: gpio0 at 20034000 { + compatible = "rockchip,gpio-bank"; + reg = <0x20034000 0x100>; + interrupts = <GIC_SPI 54 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&clk_gates8 9>; + + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + ... + + uart2 { + uart2_xfer: uart2-xfer { + rockchip,pins = <RK_GPIO1 8 1 PINCONFIG_BIAS_PULL_PIN_DEFAULT>, + <RK_GPIO1 9 1 PINCONFIG_BIAS_PULL_PIN_DEFAULT>; + }; + }; +}; + +uart2: serial at 20064000 { + compatible = "snps,dw-apb-uart"; + reg = <0x20064000 0x400>; + interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>; + reg-shift = <2>; + reg-io-width = <1>; + clocks = <&mux_uart2>; + status = "okay"; + + pinctrl-names = "default"; + pinctrl-0 = <&uart2_xfer>; +};
drivers/pinctrl/Kconfig +6 -0
@@ -158,6 +158,12 @@ config PINCTRL_DB8540 bool "DB8540 pin controller driver" depends on PINCTRL_NOMADIK && ARCH_U8500 +config PINCTRL_ROCKCHIP + bool + select PINMUX + select GENERIC_PINCONF + select GENERIC_IRQ_CHIP + config PINCTRL_SINGLE tristate "One-register-per-pin type device tree based pinctrl driver" depends on OF
drivers/pinctrl/Makefile +1 -0
@@ -31,6 +31,7 @@ obj-$(CONFIG_PINCTRL_NOMADIK) += pinctrl-nomadik.o obj-$(CONFIG_PINCTRL_STN8815) += pinctrl-nomadik-stn8815.o obj-$(CONFIG_PINCTRL_DB8500) += pinctrl-nomadik-db8500.o obj-$(CONFIG_PINCTRL_DB8540) += pinctrl-nomadik-db8540.o +obj-$(CONFIG_PINCTRL_ROCKCHIP) += pinctrl-rockchip.o obj-$(CONFIG_PINCTRL_SINGLE) += pinctrl-single.o obj-$(CONFIG_PINCTRL_SIRF) += sirf/ obj-$(CONFIG_PINCTRL_SUNXI) += pinctrl-sunxi.o
drivers/pinctrl/pinctrl-rockchip.c new file +1359 -0
1,359 lines not on this page · raw message
include/dt-bindings/pinctrl/rockchip.h new file +32 -0
@@ -0,0 +1,32 @@ +/* +* Header providing constants for Rockchip pinctrl bindings. +* +* Copyright (c) 2013 MundoReader S.L. +* Author: Heiko Stuebner <[email protected]> +* +* 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 program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +*/ + +#ifndef __DT_BINDINGS_ROCKCHIP_PINCTRL_H__ +#define __DT_BINDINGS_ROCKCHIP_PINCTRL_H__ + +#define RK_GPIO0 0 +#define RK_GPIO1 1 +#define RK_GPIO2 2 +#define RK_GPIO3 3 +#define RK_GPIO4 4 +#define RK_GPIO6 6 + +#define RK_FUNC_GPIO 0 +#define RK_FUNC_1 1 +#define RK_FUNC_2 2 + +#endif
Signature · 1 line
1.7.2.3
Re: [EXAMPLE PATCH] pinctrl: add pinctrl driver for Rockchip SoCs
In reply to Heiko StübnerQuoted 11 lines · Heiko St?bner
Am Montag, 10. Juni 2013, 15:00:41 schrieb Linus Walleij:
Quoted 10 lines · Heiko St?bner
(...)
drivers/pinctrl/pinctrl-rockchip.c line 40
Quoted 3 lines
drivers/pinctrl/pinctrl-rockchip.c new file +3
-0+#include <linux/irqchip/chained_irq.h> +#include <linux/clk-provider.h> +#include <dt-bindings/pinctrl/rockchip.h> Hm I don't know if we're supposed to do things like this actually.
Quoted 7 lines
Quoted 5 lines
It depends on whether the <dt-bindings/*> hierarchy is supposed to stay in the kernel after the device trees are moved out to a separate repo.
Grant/Rob, shall we do this in drivers?
we can get rid of it, if it's controversial.
We'll wait and see what the others say.
There is no point in duplicating this info if it will actually stay in the kernel.
Yours,
Linus Walleij
Re: [EXAMPLE PATCH] pinctrl: add pinctrl driver for Rockchip SoCs
In reply to Linus WalleijQuoted 9 lines · Linus Walleij
On Sun, Jun 9, 2013 at 2:01 AM, Heiko St?bner <[email protected]> wrote:
(...)
drivers/pinctrl/pinctrl-rockchip.c line 40
Quoted 3 lines
drivers/pinctrl/pinctrl-rockchip.c new file +3
-0+#include <linux/irqchip/chained_irq.h> +#include <linux/clk-provider.h> +#include <dt-bindings/pinctrl/rockchip.h> Hm I don't know if we're supposed to do things like this actually.
Quoted 5 lines
It depends on whether the <dt-bindings/*> hierarchy is supposed to stay in the kernel after the device trees are moved out to a separate repo.
Grant/Rob, shall we do this in drivers?
The entire reason I created the <dt-bindings/> directory (rather than just putting *.h into e.g. arch/arm/boot/dts) was to provide a place to share binding-defined constants between the DT files and drivers that implement that binding. It's certainly my opinion that if/when *.dts move out of the kernel, the <dt-bindings/> directory should continue to be present in the kernel, and take updates mirrored from the upstream binding repo.
Re: [EXAMPLE PATCH] pinctrl: add pinctrl driver for Rockchip SoCs
Quoted 16 lines · Stephen Warren
Quoted 9 lines · Linus Walleij
On Sun, Jun 9, 2013 at 2:01 AM, Heiko St?bner <[email protected]> wrote:
(...)
drivers/pinctrl/pinctrl-rockchip.c line 40
Quoted 3 lines
drivers/pinctrl/pinctrl-rockchip.c new file +3
-0+#include <linux/irqchip/chained_irq.h> +#include <linux/clk-provider.h> +#include <dt-bindings/pinctrl/rockchip.h> Hm I don't know if we're supposed to do things like this actually.
Quoted 5 lines
It depends on whether the <dt-bindings/*> hierarchy is supposed to stay in the kernel after the device trees are moved out to a separate repo.
Grant/Rob, shall we do this in drivers?
Quoted 7 lines
The entire reason I created the <dt-bindings/> directory (rather than just putting *.h into e.g.
arch/arm/boot/dts) was to provide a place to share binding-defined constants between the DT files and drivers that implement that binding. It's certainly my opinion that if/when *.dts move out of the kernel, the <dt-bindings/> directory should continue to be present in the kernel, and take updates mirrored from the upstream binding repo.
Yes, agreed. Only kernel headers in a dts would be a problem.
Rob
Re: [EXAMPLE PATCH] pinctrl: add pinctrl driver for Rockchip SoCs
In reply to Stephen WarrenQuoted 7 lines · Stephen Warren
The entire reason I created the <dt-bindings/> directory (rather than just putting *.h into e.g.
arch/arm/boot/dts) was to provide a place to share binding-defined constants between the DT files and drivers that implement that binding. It's certainly my opinion that if/when *.dts move out of the kernel, the <dt-bindings/> directory should continue to be present in the kernel, and take updates mirrored from the upstream binding repo.
OK thanks Stephen, makes perfect sense.
Yours,
Linus Walleij
Quoted 6 lines · Heiko St?bner
following your suggestions for a common handling of things like pulls in dt, I've come up with the following solution - hopefully I've gotten the correct meaning of your explanaitions.
It handles all the pinconfigs that either ignore the argument, or have very simple one, like
PIN_CONFIG_OUTPUTdoes.
OK patches applied. It needs some rough fixes like NULL check on kmalloc() but it'll do for a starter. Now I can tell everyone to use this and refer to it.
Quoted 2 lines
I've also attached my current rockchip pinctrl driver to show how it is using this currently.
OK I might just merge it. If you take the rest of the patches through ARM SoC they will need to pull in my tree anyway if it's intended to go in this merge window.
Yours,
Linus Walleij
Heiko Stübner
maintainerAm Montag, 10. Juni 2013, 14:52:13 schrieb Linus Walleij:
Quoted 10 lines · Heiko St?bner
Quoted 6 lines
following your suggestions for a common handling of things like pulls in dt, I've come up with the following solution - hopefully I've gotten the correct meaning of your explanaitions.
It handles all the pinconfigs that either ignore the argument, or have very simple one, like
PIN_CONFIG_OUTPUTdoes.OK patches applied. It needs some rough fixes like NULL check on
kmalloc()but it'll do for a starter.
gah, sorry ... I have urge to crawl under a rock now for forgetting something this obvious ;-) .
Apart from the NULL check what more did I mess up? Should I send a fixup patch or do you want to do it?
Quoted 9 lines
Now I can tell everyone to
use this and refer to it.Quoted 2 lines
I've also attached my current rockchip pinctrl driver to show how it is using this currently.
OK I might just merge it. If you take the rest of the patches through ARM SoC they will need to pull in my tree anyway if it's intended to go in this merge window.
go ahead if you like - this is the most current version of the driver and would be one patch less to worry about. It would after all come together during the merge window again.
And very basic boot of the SoC works even without the pinctrl driver, as the uart pins are of course set up by the bootloader.
Thanks
Heiko
Quoted 9 lines · Heiko St?bner
Am Montag, 10. Juni 2013, 14:52:13 schrieb Linus Walleij:
Quoted 8 lines · Heiko St?bner
Quoted 6 lines
following your suggestions for a common handling of things like pulls in dt, I've come up with the following solution - hopefully I've gotten the correct meaning of your explanaitions.
It handles all the pinconfigs that either ignore the argument, or have very simple one, like
PIN_CONFIG_OUTPUTdoes.
Quoted 8 lines
Quoted 2 lines
OK patches applied. It needs some rough fixes like NULL check on
kmalloc()but it'll do for a starter.gah, sorry ... I have urge to crawl under a rock now for forgetting something this obvious ;-) .
Apart from the NULL check what more did I mess up? Should I send a fixup patch or do you want to do it?
I can fix this.
But now I just wondered about one thing:
Does this design imply that we will never be able to support any more than 32 bits (well, the number of bits in unsigned long) i.e. 32 different generic pin configurations?
In that case I suspect this might not be so wise. :-(
What about instead of using bitstuffed numbers using some representation similar to what can be found in Documentation/devicetree/bindings/pinctrl/ste,nomadik.txt
So instead of using <dt-bindings/pinctrl/pinconfig.h> and enumerators it would be more like using node properties like this:
my_config: my_config {
bias-disable;
drive-push-pull = <6>;
output-high;
};It will make the device trees bigger for sure, but maybe more readable as well?
Yours,
Linus Walleij
Heiko Stübner
maintainerAm Montag, 10. Juni 2013, 15:39:26 schrieb Linus Walleij:
Quoted 21 lines · Heiko St?bner
Quoted 9 lines
Am Montag, 10. Juni 2013, 14:52:13 schrieb Linus Walleij:
Quoted 8 lines · Heiko St?bner
Quoted 6 lines
following your suggestions for a common handling of things like pulls in dt, I've come up with the following solution - hopefully I've gotten the correct meaning of your explanaitions.
It handles all the pinconfigs that either ignore the argument, or have very simple one, like
PIN_CONFIG_OUTPUTdoes.Quoted 8 lines
Quoted 2 lines
OK patches applied. It needs some rough fixes like NULL check on
kmalloc()but it'll do for a starter.gah, sorry ... I have urge to crawl under a rock now for forgetting something this obvious ;-) .
Apart from the NULL check what more did I mess up? Should I send a fixup patch or do you want to do it?
I can fix this.
Quoted 7 lines
But now I just wondered about one thing:
Does this design imply that we will never be able to support any more than 32 bits (well, the number of bits in unsigned long) i.e. 32 different generic pin configurations?
In that case I suspect this might not be so wise. :-(
hmm, yes this would be the limit ... so back to the drawing board
Quoted 8 lines
What about instead of using bitstuffed numbers using some representation similar to what can be found in
Documentation/devicetree/bindings/pinctrl/ste,nomadik.txtSo instead of using <
dt-bindings/pinctrl/pinconfig.h> and enumerators it would be more like using node properties like this:
Quoted 8 lines
my_config: my_config { bias-disable; drive-push-pull = <6>; output-high; };It will make the device trees bigger for sure, but maybe more readable as well?
There was a discussion about each of the merits of the two approaches (bitstuffed vs. individual properties) between Tomasz Figa and Jean-Christophe PLAGNIOL-VILLARD a bit back if I remember correctly.
Also I don't think having the configs central like Nomadik does would bloat the dt to much, as most of the time most pins share one config or another.
And it still would be possible to write short pin statements as
pins = <bank pin mux &config_node>;
if I'm not mistaken.
Quoted 9 lines · Heiko St?bner
Am Montag, 10. Juni 2013, 15:39:26 schrieb Linus Walleij:
Quoted 6 lines
Does this design imply that we will never be able to support any more than 32 bits (well, the number of bits in unsigned long) i.e. 32 different generic pin configurations?
In that case I suspect this might not be so wise. :-(
hmm, yes this would be the limit ... so back to the drawing board
Well, I'm not hopeless, if this is what everyone wants we might need to have it like this, perhaps make it possible to extend the config word in the device tree from 32 to 64 to 96 bits like this, if:
= <0x00000001>;
becomes:
= <PINCONFIG_BIAS_DISABLE>;
once we hit:
= <0x80000000>;
that is maybe
= <PINCONFIG_FOO>;
and then we need to ad PINCONFIG_BAR. What to do... Well enable the next 64 bits:
= <0x00000000 0x00000001>;
i.e.
= <PINCONFIG_NONE PINCONFIG_EXT1_BAR>;
Note necessary naming convention to remember to
put that after the other...
and then we get to 96 bits:
= <PINCONFIG_NONE PINCONFIG_EXT1_NONE PINCONFIG_EXT2_BAZ>;
It's getting a bit ugly ...
Quoted 6 lines
Quoted 6 lines
my_config: my_config { bias-disable; drive-push-pull = <6>; output-high; };
Quoted 6 lines
Quoted 2 lines
It will make the device trees bigger for sure, but maybe more readable as well?
There was a discussion about each of the merits of the two approaches (bitstuffed vs. individual properties) between Tomasz Figa and Jean-Christophe PLAGNIOL-VILLARD a bit back if I remember correctly.
It's OK if it's bindings for a certain system where you know you will never have more than 32 config options. But for something generic we need to make some space.
Quoted 5 lines
And it still would be possible to write short pin statements as
pins = <bank pin mux &config_node>;if I'm not mistaken.
I thinkso....
I'll take these patches out FTM so you can experiment, but I'm ready to make a late merge of this if you can come up with something that we agree upon to be future-safe.
Yours,
Linus Walleij