* [PATCH v1 0/5] Add sdmmc UHS support to ROC-RK3328-CC board. @ 2018-05-10 9:16 djw 2018-05-10 9:16 ` [PATCH v1 1/5] gpio: syscon: allow fetching syscon from parent node djw 2018-05-10 9:16 ` [PATCH v1 2/5] gpio: syscon: Add gpio-syscon for rockchip djw 0 siblings, 2 replies; 5+ messages in thread From: djw @ 2018-05-10 9:16 UTC (permalink / raw) To: linux-rockchip Cc: Wayne Chou, Levin Du, Heiko Stuebner, devicetree, David Wu, Arnd Bergmann, Finley Xiao, Liang Chen, William Wu, Linus Walleij, Sugar Zhang, linux-kernel, linux-gpio, Robin Murphy, Rob Herring, Rocky Hao, Will Deacon, Joseph Chen, Mark Rutland, Catalin Marinas, linux-arm-kernel From: Levin Du <djw@t-chip.com.cn> Hi all, this is an attemp to add sdmmc UHS support to the ROC-RK3328-CC board. This patch series adds a new compatible `rockchip,gpio-syscon` to the gpio-syscon driver for general Rockchip SoC usage.. A new gpio controller named `gpio_syscon10` is defined in rk3328.dtsi so that all rk3328 boards has access to it. The ROC-RK3328-CC board use the new gpio <&gpio_syscon10 1> in gpio-regulator to control the signal voltage of the sdmmc. It is essential for UHS support which requires 1.8V signal voltage. Many thanks to Heiko's great advice! Changes in v1: - Refactured for general gpio-syscon usage for Rockchip SoCs. - Add doc rockchip,gpio-syscon.txt . - Split into small patches. - Add gpio-syscon10 to rk3328 for general use. - Sort dts properties in sdmmc node. Heiko Stuebner (1): gpio: syscon: allow fetching syscon from parent node Levin Du (4): gpio: syscon: Add gpio-syscon for rockchip arm64: dts: rockchip: Add gpio-syscon10 to rk3328 arm64: dts: rockchip: Add io-domain to roc-rk3328-cc arm64: dts: rockchip: Add sdmmc UHS support for roc-rk3328-cc .../bindings/gpio/rockchip,gpio-syscon.txt | 41 ++++++++++++++++++++++ arch/arm64/boot/dts/rockchip/rk3328-roc-cc.dts | 30 ++++++++++++++++ arch/arm64/boot/dts/rockchip/rk3328.dtsi | 6 ++++ drivers/gpio/gpio-syscon.c | 32 +++++++++++++++++ 4 files changed, 109 insertions(+) create mode 100644 Documentation/devicetree/bindings/gpio/rockchip,gpio-syscon.txt -- 2.7.4 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v1 1/5] gpio: syscon: allow fetching syscon from parent node 2018-05-10 9:16 [PATCH v1 0/5] Add sdmmc UHS support to ROC-RK3328-CC board djw @ 2018-05-10 9:16 ` djw 2018-05-10 9:16 ` [PATCH v1 2/5] gpio: syscon: Add gpio-syscon for rockchip djw 1 sibling, 0 replies; 5+ messages in thread From: djw @ 2018-05-10 9:16 UTC (permalink / raw) To: linux-rockchip Cc: Wayne Chou, Levin Du, Heiko Stuebner, linux-gpio, Linus Walleij, linux-kernel From: Heiko Stuebner <heiko@sntech.de> Syscon nodes can be a simple-mfd and the syscon-users then be declared as children of this node. That way the parent-child structure can be better represented for devices that are fully embedded in the syscon. Therefore allow getting the syscon from the parent if neither a special compatible nor a gpio,syscon-dev property is defined. Signed-off-by: Heiko Stuebner <heiko@sntech.de> Signed-off-by: Levin Du <djw@t-chip.com.cn> --- Changes in v1: - New drivers/gpio/gpio-syscon.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpio/gpio-syscon.c b/drivers/gpio/gpio-syscon.c index 537cec7..7325b86 100644 --- a/drivers/gpio/gpio-syscon.c +++ b/drivers/gpio/gpio-syscon.c @@ -205,6 +205,8 @@ static int syscon_gpio_probe(struct platform_device *pdev) } else { priv->syscon = syscon_regmap_lookup_by_phandle(np, "gpio,syscon-dev"); + if (IS_ERR(priv->syscon) && np->parent) + priv->syscon = syscon_node_to_regmap(np->parent); if (IS_ERR(priv->syscon)) return PTR_ERR(priv->syscon); -- 2.7.4 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v1 2/5] gpio: syscon: Add gpio-syscon for rockchip 2018-05-10 9:16 [PATCH v1 0/5] Add sdmmc UHS support to ROC-RK3328-CC board djw 2018-05-10 9:16 ` [PATCH v1 1/5] gpio: syscon: allow fetching syscon from parent node djw @ 2018-05-10 9:16 ` djw 2018-05-10 14:56 ` Robin Murphy 1 sibling, 1 reply; 5+ messages in thread From: djw @ 2018-05-10 9:16 UTC (permalink / raw) To: linux-rockchip Cc: Wayne Chou, Levin Du, Heiko Stuebner, devicetree, Linus Walleij, linux-kernel, linux-gpio, Rob Herring, Mark Rutland, linux-arm-kernel From: Levin Du <djw@t-chip.com.cn> Some GPIOs sit in the GRF_SOC_CON registers of Rockchip SoCs, which do not belong to the general pinctrl. Adding gpio-syscon support makes controlling regulator or LED using these special pins very easy by reusing existing drivers, such as gpio-regulator and led-gpio. Signed-off-by: Levin Du <djw@t-chip.com.cn> --- Changes in v1: - Refactured for general gpio-syscon usage for Rockchip SoCs. - Add doc rockchip,gpio-syscon.txt .../bindings/gpio/rockchip,gpio-syscon.txt | 41 ++++++++++++++++++++++ drivers/gpio/gpio-syscon.c | 30 ++++++++++++++++ 2 files changed, 71 insertions(+) create mode 100644 Documentation/devicetree/bindings/gpio/rockchip,gpio-syscon.txt diff --git a/Documentation/devicetree/bindings/gpio/rockchip,gpio-syscon.txt b/Documentation/devicetree/bindings/gpio/rockchip,gpio-syscon.txt new file mode 100644 index 0000000..e4c1650 --- /dev/null +++ b/Documentation/devicetree/bindings/gpio/rockchip,gpio-syscon.txt @@ -0,0 +1,41 @@ +* Rockchip GPIO support for GRF_SOC_CON registers + +Required properties: +- compatible: Should contain "rockchip,gpio-syscon". +- gpio-controller: Marks the device node as a gpio controller. +- #gpio-cells: Should be two. The first cell is the pin number and + the second cell is used to specify the gpio polarity: + 0 = Active high, + 1 = Active low. +- gpio,syscon-dev: Should contain <grf_phandle syscon_offset 0>. + If declared as child of the grf node, the grf_phandle can be 0. + +Example: + +1. As child of grf node: + + grf: syscon@ff100000 { + compatible = "rockchip,rk3328-grf", "syscon", "simple-mfd"; + + gpio_syscon10: gpio-syscon10 { + compatible = "rockchip,gpio-syscon"; + gpio-controller; + #gpio-cells = <2>; + gpio,syscon-dev = <0 0x0428 0>; + }; + }; + + +2. Not child of grf node: + + grf: syscon@ff100000 { + compatible = "rockchip,rk3328-grf", "syscon", "simple-mfd"; + //... + }; + + gpio_syscon10: gpio-syscon10 { + compatible = "rockchip,gpio-syscon"; + gpio-controller; + #gpio-cells = <2>; + gpio,syscon-dev = <&grf 0x0428 0>; + }; diff --git a/drivers/gpio/gpio-syscon.c b/drivers/gpio/gpio-syscon.c index 7325b86..e24b408 100644 --- a/drivers/gpio/gpio-syscon.c +++ b/drivers/gpio/gpio-syscon.c @@ -135,6 +135,32 @@ static const struct syscon_gpio_data clps711x_mctrl_gpio = { .dat_bit_offset = 0x40 * 8 + 8, }; +static void rockchip_gpio_set(struct gpio_chip *chip, unsigned int offset, + int val) +{ + struct syscon_gpio_priv *priv = gpiochip_get_data(chip); + unsigned int offs; + u8 bit; + u32 data; + int ret; + + offs = priv->dreg_offset + priv->data->dat_bit_offset + offset; + bit = offs % SYSCON_REG_BITS; + data = (val ? BIT(bit) : 0) | BIT(bit + 16); + ret = regmap_write(priv->syscon, + (offs / SYSCON_REG_BITS) * SYSCON_REG_SIZE, + data); + if (ret < 0) + dev_err(chip->parent, "gpio write failed ret(%d)\n", ret); +} + +static const struct syscon_gpio_data rockchip_gpio_syscon = { + /* Rockchip GRF_SOC_CON Bits 0-15 */ + .flags = GPIO_SYSCON_FEAT_OUT, + .bit_count = 16, + .set = rockchip_gpio_set, +}; + #define KEYSTONE_LOCK_BIT BIT(0) static void keystone_gpio_set(struct gpio_chip *chip, unsigned offset, int val) @@ -175,6 +201,10 @@ static const struct of_device_id syscon_gpio_ids[] = { .compatible = "ti,keystone-dsp-gpio", .data = &keystone_dsp_gpio, }, + { + .compatible = "rockchip,gpio-syscon", + .data = &rockchip_gpio_syscon, + }, { } }; MODULE_DEVICE_TABLE(of, syscon_gpio_ids); -- 2.7.4 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v1 2/5] gpio: syscon: Add gpio-syscon for rockchip 2018-05-10 9:16 ` [PATCH v1 2/5] gpio: syscon: Add gpio-syscon for rockchip djw @ 2018-05-10 14:56 ` Robin Murphy 2018-05-11 2:16 ` Levin Du 0 siblings, 1 reply; 5+ messages in thread From: Robin Murphy @ 2018-05-10 14:56 UTC (permalink / raw) To: djw, linux-rockchip Cc: Mark Rutland, devicetree, Wayne Chou, Heiko Stuebner, Linus Walleij, linux-kernel, linux-gpio, Rob Herring, linux-arm-kernel On 10/05/18 10:16, djw@t-chip.com.cn wrote: > From: Levin Du <djw@t-chip.com.cn> > > Some GPIOs sit in the GRF_SOC_CON registers of Rockchip SoCs, > which do not belong to the general pinctrl. > > Adding gpio-syscon support makes controlling regulator or > LED using these special pins very easy by reusing existing > drivers, such as gpio-regulator and led-gpio. > > Signed-off-by: Levin Du <djw@t-chip.com.cn> > > --- > > Changes in v1: > - Refactured for general gpio-syscon usage for Rockchip SoCs. > - Add doc rockchip,gpio-syscon.txt > > .../bindings/gpio/rockchip,gpio-syscon.txt | 41 ++++++++++++++++++++++ > drivers/gpio/gpio-syscon.c | 30 ++++++++++++++++ > 2 files changed, 71 insertions(+) > create mode 100644 Documentation/devicetree/bindings/gpio/rockchip,gpio-syscon.txt > > diff --git a/Documentation/devicetree/bindings/gpio/rockchip,gpio-syscon.txt b/Documentation/devicetree/bindings/gpio/rockchip,gpio-syscon.txt > new file mode 100644 > index 0000000..e4c1650 > --- /dev/null > +++ b/Documentation/devicetree/bindings/gpio/rockchip,gpio-syscon.txt > @@ -0,0 +1,41 @@ > +* Rockchip GPIO support for GRF_SOC_CON registers > + > +Required properties: > +- compatible: Should contain "rockchip,gpio-syscon". > +- gpio-controller: Marks the device node as a gpio controller. > +- #gpio-cells: Should be two. The first cell is the pin number and I would suggest s/pin number/bit number in the associated GRF register/ here. At least in this RK3328 case there's only one pin, which isn't numbered, and if you naively considered it pin 0 of this 'bank' you'd already have the wrong number. Since we're dealing with the "random SoC-specific controls" region of the GRF as opposed to the relatively-consistent and organised pinmux parts, I don't think we should rely on any assumptions about how things are laid out. I was initially going to suggest a more specific compatible string as well, but on reflection I think the generic "rockchip,gpio-syscon" for basic "flip this single GRF bit" functionality actually is the right way to go. In the specific RK3328 GPIO_MUTE case, there look to be 4 bits in total related to this pin - the enable, value, and some pull controls (which I assume apply when the output is disabled) - if at some point in future we *did* want to start explicitly controlling the rest of them too, then would be a good time to define a separate "rockchip,rk3328-gpio-mute" binding (and probably a dedicated driver) for that specialised functionality, independently of this basic one. > + the second cell is used to specify the gpio polarity: > + 0 = Active high, > + 1 = Active low. > +- gpio,syscon-dev: Should contain <grf_phandle syscon_offset 0>. > + If declared as child of the grf node, the grf_phandle can be 0. > + > +Example: > + > +1. As child of grf node: > + > + grf: syscon@ff100000 { > + compatible = "rockchip,rk3328-grf", "syscon", "simple-mfd"; > + > + gpio_syscon10: gpio-syscon10 { > + compatible = "rockchip,gpio-syscon"; > + gpio-controller; > + #gpio-cells = <2>; > + gpio,syscon-dev = <0 0x0428 0>; > + }; > + }; > + > + > +2. Not child of grf node: > + > + grf: syscon@ff100000 { > + compatible = "rockchip,rk3328-grf", "syscon", "simple-mfd"; > + //... > + }; > + > + gpio_syscon10: gpio-syscon10 { > + compatible = "rockchip,gpio-syscon"; > + gpio-controller; > + #gpio-cells = <2>; > + gpio,syscon-dev = <&grf 0x0428 0>; > + }; > diff --git a/drivers/gpio/gpio-syscon.c b/drivers/gpio/gpio-syscon.c > index 7325b86..e24b408 100644 > --- a/drivers/gpio/gpio-syscon.c > +++ b/drivers/gpio/gpio-syscon.c > @@ -135,6 +135,32 @@ static const struct syscon_gpio_data clps711x_mctrl_gpio = { > .dat_bit_offset = 0x40 * 8 + 8, > }; > > +static void rockchip_gpio_set(struct gpio_chip *chip, unsigned int offset, > + int val) > +{ > + struct syscon_gpio_priv *priv = gpiochip_get_data(chip); > + unsigned int offs; > + u8 bit; > + u32 data; > + int ret; > + > + offs = priv->dreg_offset + priv->data->dat_bit_offset + offset; data->dat_bit_offset is always 0 here, but given that wrapping large offsets to successive GRF registers doesn't make sense (and wouldn't work anyway with this arithmetic) I don't think you even need this calculation of offs at all... > + bit = offs % SYSCON_REG_BITS; ... since it would suffice to use offset here... > + data = (val ? BIT(bit) : 0) | BIT(bit + 16); > + ret = regmap_write(priv->syscon, > + (offs / SYSCON_REG_BITS) * SYSCON_REG_SIZE, ... and priv->dreg_offset here. Robin. > + data); > + if (ret < 0) > + dev_err(chip->parent, "gpio write failed ret(%d)\n", ret); > +} > + > +static const struct syscon_gpio_data rockchip_gpio_syscon = { > + /* Rockchip GRF_SOC_CON Bits 0-15 */ > + .flags = GPIO_SYSCON_FEAT_OUT, > + .bit_count = 16, > + .set = rockchip_gpio_set, > +}; > + > #define KEYSTONE_LOCK_BIT BIT(0) > > static void keystone_gpio_set(struct gpio_chip *chip, unsigned offset, int val) > @@ -175,6 +201,10 @@ static const struct of_device_id syscon_gpio_ids[] = { > .compatible = "ti,keystone-dsp-gpio", > .data = &keystone_dsp_gpio, > }, > + { > + .compatible = "rockchip,gpio-syscon", > + .data = &rockchip_gpio_syscon, > + }, > { } > }; > MODULE_DEVICE_TABLE(of, syscon_gpio_ids); > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1 2/5] gpio: syscon: Add gpio-syscon for rockchip 2018-05-10 14:56 ` Robin Murphy @ 2018-05-11 2:16 ` Levin Du 0 siblings, 0 replies; 5+ messages in thread From: Levin Du @ 2018-05-11 2:16 UTC (permalink / raw) To: Robin Murphy, linux-rockchip Cc: Mark Rutland, devicetree, Wayne Chou, Heiko Stuebner, Linus Walleij, linux-kernel, linux-gpio, Rob Herring, linux-arm-kernel On 2018-05-10 10:56 PM, Robin Murphy wrote: > >> diff --git >> a/Documentation/devicetree/bindings/gpio/rockchip,gpio-syscon.txt >> b/Documentation/devicetree/bindings/gpio/rockchip,gpio-syscon.txt >> new file mode 100644 >> index 0000000..e4c1650 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/gpio/rockchip,gpio-syscon.txt >> @@ -0,0 +1,41 @@ >> +* Rockchip GPIO support for GRF_SOC_CON registers >> + >> +Required properties: >> +- compatible: Should contain "rockchip,gpio-syscon". >> +- gpio-controller: Marks the device node as a gpio controller. >> +- #gpio-cells: Should be two. The first cell is the pin number and > > I would suggest s/pin number/bit number in the associated GRF > register/ here. At least in this RK3328 case there's only one pin, > which isn't numbered, and if you naively considered it pin 0 of this > 'bank' you'd already have the wrong number. Since we're dealing with > the "random SoC-specific controls" region of the GRF as opposed to the > relatively-consistent and organised pinmux parts, I don't think we > should rely on any assumptions about how things are laid out. > > I was initially going to suggest a more specific compatible string as > well, but on reflection I think the generic "rockchip,gpio-syscon" for > basic "flip this single GRF bit" functionality actually is the right > way to go. In the specific RK3328 GPIO_MUTE case, there look to be 4 > bits in total related to this pin - the enable, value, and some pull > controls (which I assume apply when the output is disabled) - if at > some point in future we *did* want to start explicitly controlling the > rest of them too, then would be a good time to define a separate > "rockchip,rk3328-gpio-mute" binding (and probably a dedicated driver) > for that specialised functionality, independently of this basic one. > Good point! I'll fix the doc. >> +static void rockchip_gpio_set(struct gpio_chip *chip, unsigned int >> offset, >> + int val) >> +{ >> + struct syscon_gpio_priv *priv = gpiochip_get_data(chip); >> + unsigned int offs; >> + u8 bit; >> + u32 data; >> + int ret; >> + >> + offs = priv->dreg_offset + priv->data->dat_bit_offset + offset; > > data->dat_bit_offset is always 0 here, but given that wrapping large > offsets to successive GRF registers doesn't make sense (and wouldn't > work anyway with this arithmetic) I don't think you even need this > calculation of offs at all... > >> + bit = offs % SYSCON_REG_BITS; > > ... since it would suffice to use offset here... > >> + data = (val ? BIT(bit) : 0) | BIT(bit + 16); >> + ret = regmap_write(priv->syscon, >> + (offs / SYSCON_REG_BITS) * SYSCON_REG_SIZE, > > ... and priv->dreg_offset here. > rockchip_gpio_set() follows the conventional offset handling in the gpio-syscon driver. dreg_offset will get multiplied by 8 (dreg_offset <<= 3) in syscon_gpio_probe(). I'm not sure if this needs to simplify, or stay the same as others. Thanks Levin ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-05-11 2:16 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-05-10 9:16 [PATCH v1 0/5] Add sdmmc UHS support to ROC-RK3328-CC board djw 2018-05-10 9:16 ` [PATCH v1 1/5] gpio: syscon: allow fetching syscon from parent node djw 2018-05-10 9:16 ` [PATCH v1 2/5] gpio: syscon: Add gpio-syscon for rockchip djw 2018-05-10 14:56 ` Robin Murphy 2018-05-11 2:16 ` Levin Du
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).