* [PATCH 0/2] pinctrl: mediatek: Add direction control and gpio_request_enable Support @ 2016-02-03 1:24 Biao Huang 2016-02-03 1:24 ` [PATCH 1/2] pinctrl: mediatek: fix direction control issue Biao Huang 2016-02-03 1:24 ` [PATCH 2/2] pinctrl: mediatek: Add gpio_request_enable support Biao Huang 0 siblings, 2 replies; 10+ messages in thread From: Biao Huang @ 2016-02-03 1:24 UTC (permalink / raw) To: linux-arm-kernel 1. Set direction to input when do input-enable/disable and input-schmitt-enable/disable properties. 2. Add gpio_request_enable support to set pin to gpio function. Biao Huang (2): pinctrl: mediatek: fix direction control issue pinctrl: mediatek: Add gpio_request_enable support drivers/pinctrl/mediatek/pinctrl-mtk-common.c | 38 +++++++++++++++++++++++++ 1 file changed, 38 insertions(+) -- 1.7.9.5 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] pinctrl: mediatek: fix direction control issue 2016-02-03 1:24 [PATCH 0/2] pinctrl: mediatek: Add direction control and gpio_request_enable Support Biao Huang @ 2016-02-03 1:24 ` Biao Huang 2016-02-04 3:03 ` Hongzhou Yang 2016-02-05 13:56 ` Linus Walleij 2016-02-03 1:24 ` [PATCH 2/2] pinctrl: mediatek: Add gpio_request_enable support Biao Huang 1 sibling, 2 replies; 10+ messages in thread From: Biao Huang @ 2016-02-03 1:24 UTC (permalink / raw) To: linux-arm-kernel Since input-enable/disable and input-schmitt-enable/disable are workable when gpio direction is input, so add direction setting when do input-enable/disable and input-schmitt-enable/disable properties. Signed-off-by: Biao Huang <biao.huang@mediatek.com> --- drivers/pinctrl/mediatek/pinctrl-mtk-common.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common.c index 8cac73d..6eb01c9 100644 --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common.c +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common.c @@ -352,6 +352,7 @@ static int mtk_pconf_parse_conf(struct pinctrl_dev *pctldev, ret = mtk_pconf_set_pull_select(pctl, pin, true, false, arg); break; case PIN_CONFIG_INPUT_ENABLE: + mtk_pmx_gpio_set_direction(pctldev, NULL, pin, true); ret = mtk_pconf_set_ies_smt(pctl, pin, arg, param); break; case PIN_CONFIG_OUTPUT: @@ -359,6 +360,7 @@ static int mtk_pconf_parse_conf(struct pinctrl_dev *pctldev, ret = mtk_pmx_gpio_set_direction(pctldev, NULL, pin, false); break; case PIN_CONFIG_INPUT_SCHMITT_ENABLE: + mtk_pmx_gpio_set_direction(pctldev, NULL, pin, true); ret = mtk_pconf_set_ies_smt(pctl, pin, arg, param); break; case PIN_CONFIG_DRIVE_STRENGTH: -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 1/2] pinctrl: mediatek: fix direction control issue 2016-02-03 1:24 ` [PATCH 1/2] pinctrl: mediatek: fix direction control issue Biao Huang @ 2016-02-04 3:03 ` Hongzhou Yang 2016-02-05 13:52 ` Linus Walleij 2016-02-05 13:56 ` Linus Walleij 1 sibling, 1 reply; 10+ messages in thread From: Hongzhou Yang @ 2016-02-04 3:03 UTC (permalink / raw) To: linux-arm-kernel On Wed, 2016-02-03 at 09:24 +0800, Biao Huang wrote: > Since input-enable/disable and input-schmitt-enable/disable are > workable when gpio direction is input, so add direction setting > when do input-enable/disable and input-schmitt-enable/disable > properties. > > Signed-off-by: Biao Huang <biao.huang@mediatek.com> > --- > drivers/pinctrl/mediatek/pinctrl-mtk-common.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common.c > index 8cac73d..6eb01c9 100644 > --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common.c > +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common.c > @@ -352,6 +352,7 @@ static int mtk_pconf_parse_conf(struct pinctrl_dev *pctldev, > ret = mtk_pconf_set_pull_select(pctl, pin, true, false, arg); > break; > case PIN_CONFIG_INPUT_ENABLE: > + mtk_pmx_gpio_set_direction(pctldev, NULL, pin, true); > ret = mtk_pconf_set_ies_smt(pctl, pin, arg, param); Hi Linus, >From pinctrl-bindings.txt, 'input-enable' property defined as following: input-enable - enable input on pin (no effect on output) Since input enable and input direction are two different settings on our SOC, could you tell me the exact meaning of this property? Input enable only? Or set input direction at the same time? Thanks, Hongzhou ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] pinctrl: mediatek: fix direction control issue 2016-02-04 3:03 ` Hongzhou Yang @ 2016-02-05 13:52 ` Linus Walleij 2016-02-12 2:07 ` Hongzhou Yang 0 siblings, 1 reply; 10+ messages in thread From: Linus Walleij @ 2016-02-05 13:52 UTC (permalink / raw) To: linux-arm-kernel On Thu, Feb 4, 2016 at 4:03 AM, Hongzhou Yang <hongzhou.yang@mediatek.com> wrote: > On Wed, 2016-02-03 at 09:24 +0800, Biao Huang wrote: >> Since input-enable/disable and input-schmitt-enable/disable are >> workable when gpio direction is input, so add direction setting >> when do input-enable/disable and input-schmitt-enable/disable >> properties. >> >> Signed-off-by: Biao Huang <biao.huang@mediatek.com> >> --- >> drivers/pinctrl/mediatek/pinctrl-mtk-common.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common.c >> index 8cac73d..6eb01c9 100644 >> --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common.c >> +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common.c >> @@ -352,6 +352,7 @@ static int mtk_pconf_parse_conf(struct pinctrl_dev *pctldev, >> ret = mtk_pconf_set_pull_select(pctl, pin, true, false, arg); >> break; >> case PIN_CONFIG_INPUT_ENABLE: >> + mtk_pmx_gpio_set_direction(pctldev, NULL, pin, true); >> ret = mtk_pconf_set_ies_smt(pctl, pin, arg, param); > > Hi Linus, > > From pinctrl-bindings.txt, 'input-enable' property defined as following: > input-enable - enable input on pin (no effect on output) > > Since input enable and input direction are two different settings on our > SOC, What does this mean? How can input have a "direction"? Isn't the direction of an input always inbound, into the SoC? Please elaborate. > could you tell me the exact meaning of this property? Input enable > only? Or set input direction at the same time? This was added in commit 8ba3f4d00078e7a49c60c0bd6298f29402c3a0a0 "pinctrl: Adds slew-rate, input-enable/disable" by Sherman Yin so let's ask him first. I interpret it as if there is a single bit setting a pin to input, these two boolean flags toggle that bit, so it would be the first setting on your SoC in that case. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] pinctrl: mediatek: fix direction control issue 2016-02-05 13:52 ` Linus Walleij @ 2016-02-12 2:07 ` Hongzhou Yang 2016-02-16 8:54 ` Yingjoe Chen 0 siblings, 1 reply; 10+ messages in thread From: Hongzhou Yang @ 2016-02-12 2:07 UTC (permalink / raw) To: linux-arm-kernel On Fri, 2016-02-05 at 14:52 +0100, Linus Walleij wrote: > On Thu, Feb 4, 2016 at 4:03 AM, Hongzhou Yang > <hongzhou.yang@mediatek.com> wrote: > > On Wed, 2016-02-03 at 09:24 +0800, Biao Huang wrote: > >> Since input-enable/disable and input-schmitt-enable/disable are > >> workable when gpio direction is input, so add direction setting > >> when do input-enable/disable and input-schmitt-enable/disable > >> properties. > >> > >> Signed-off-by: Biao Huang <biao.huang@mediatek.com> > >> --- > >> drivers/pinctrl/mediatek/pinctrl-mtk-common.c | 2 ++ > >> 1 file changed, 2 insertions(+) > >> > >> diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common.c > >> index 8cac73d..6eb01c9 100644 > >> --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common.c > >> +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common.c > >> @@ -352,6 +352,7 @@ static int mtk_pconf_parse_conf(struct pinctrl_dev *pctldev, > >> ret = mtk_pconf_set_pull_select(pctl, pin, true, false, arg); > >> break; > >> case PIN_CONFIG_INPUT_ENABLE: > >> + mtk_pmx_gpio_set_direction(pctldev, NULL, pin, true); > >> ret = mtk_pconf_set_ies_smt(pctl, pin, arg, param); > > > > Hi Linus, > > > > From pinctrl-bindings.txt, 'input-enable' property defined as following: > > input-enable - enable input on pin (no effect on output) > > > > Since input enable and input direction are two different settings on our > > SOC, > > What does this mean? How can input have a "direction"? > > Isn't the direction of an input always inbound, into the SoC? > > Please elaborate. Sorry for the late reply. There are two conditions if external device input data into SOC. 1. Change GPIO direction to input. 2. Switch on input. > > could you tell me the exact meaning of this property? Input enable > > only? Or set input direction at the same time? > > This was added in commit 8ba3f4d00078e7a49c60c0bd6298f29402c3a0a0 > "pinctrl: Adds slew-rate, input-enable/disable" > by Sherman Yin so let's ask him first. Thanks for this info. >From Sherman's comment, PIN_CONFIG_INPUT_ENABLE seems only to stand for the condition 2. * @PIN_CONFIG_INPUT_ENABLE: enable the pin's input. Note that this does not affect the pin's ability to drive output. 1 enables input, 0 disables input. Then I get confuse if we can change direction under this property. Thanks. Hongzhou ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] pinctrl: mediatek: fix direction control issue 2016-02-12 2:07 ` Hongzhou Yang @ 2016-02-16 8:54 ` Yingjoe Chen 2016-02-16 12:28 ` Linus Walleij 0 siblings, 1 reply; 10+ messages in thread From: Yingjoe Chen @ 2016-02-16 8:54 UTC (permalink / raw) To: linux-arm-kernel On Thu, 2016-02-11 at 18:07 -0800, Hongzhou Yang wrote: > On Fri, 2016-02-05 at 14:52 +0100, Linus Walleij wrote: > > On Thu, Feb 4, 2016 at 4:03 AM, Hongzhou Yang > > <hongzhou.yang@mediatek.com> wrote: > > > On Wed, 2016-02-03 at 09:24 +0800, Biao Huang wrote: > > >> Since input-enable/disable and input-schmitt-enable/disable are > > >> workable when gpio direction is input, so add direction setting > > >> when do input-enable/disable and input-schmitt-enable/disable > > >> properties. > > >> > > >> Signed-off-by: Biao Huang <biao.huang@mediatek.com> > > >> --- > > >> drivers/pinctrl/mediatek/pinctrl-mtk-common.c | 2 ++ > > >> 1 file changed, 2 insertions(+) > > >> > > >> diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common.c > > >> index 8cac73d..6eb01c9 100644 > > >> --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common.c > > >> +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common.c > > >> @@ -352,6 +352,7 @@ static int mtk_pconf_parse_conf(struct pinctrl_dev *pctldev, > > >> ret = mtk_pconf_set_pull_select(pctl, pin, true, false, arg); > > >> break; > > >> case PIN_CONFIG_INPUT_ENABLE: > > >> + mtk_pmx_gpio_set_direction(pctldev, NULL, pin, true); > > >> ret = mtk_pconf_set_ies_smt(pctl, pin, arg, param); > > > > > > Hi Linus, > > > > > > From pinctrl-bindings.txt, 'input-enable' property defined as following: > > > input-enable - enable input on pin (no effect on output) > > > > > > Since input enable and input direction are two different settings on our > > > SOC, > > > > What does this mean? How can input have a "direction"? > > > > Isn't the direction of an input always inbound, into the SoC? > > > > Please elaborate. > > Sorry for the late reply. > There are two conditions if external device input data into SOC. > 1. Change GPIO direction to input. > 2. Switch on input. > > > > > could you tell me the exact meaning of this property? Input enable > > > only? Or set input direction at the same time? > > > > This was added in commit 8ba3f4d00078e7a49c60c0bd6298f29402c3a0a0 > > "pinctrl: Adds slew-rate, input-enable/disable" > > by Sherman Yin so let's ask him first. > > Thanks for this info. > From Sherman's comment, PIN_CONFIG_INPUT_ENABLE seems only to stand for > the condition 2. > > * @PIN_CONFIG_INPUT_ENABLE: enable the pin's input. Note that this does > not affect the pin's ability to drive output. 1 enables input, 0 > disables input. > > Then I get confuse if we can change direction under this property. Hi, To make it more clear, for a GPIO pin to get input on MTK chip, we must: - Enable input for the pad (input-enable) - Set mux to GPIO mode - Set GPIO direction to input Currently user need to add a dts node to set pinmux to GPIO mode with input-enable property, the driver need to call gpio_direction_input to set direction. Since input-enable and GPIO direction are 2 different things, we now think this patch is inappropriate. Instead, user should not required to add pinmux node in dts if all he want is use it as plain GPIO input. Patch [2/2] remove the need to set it to GPIO mode, Biao will send a new patch to remove the need for input-enable. Joe.C ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] pinctrl: mediatek: fix direction control issue 2016-02-16 8:54 ` Yingjoe Chen @ 2016-02-16 12:28 ` Linus Walleij 0 siblings, 0 replies; 10+ messages in thread From: Linus Walleij @ 2016-02-16 12:28 UTC (permalink / raw) To: linux-arm-kernel On Tue, Feb 16, 2016 at 9:54 AM, Yingjoe Chen <yingjoe.chen@mediatek.com> wrote: > To make it more clear, for a GPIO pin to get input on MTK chip, we must: > > - Enable input for the pad (input-enable) This should be done by the pin control portions of the driver in response to the PIN_CONFIG_INPUT_ENABLE generic pin config setting. Possibly *also* by just writing the hardware registers from the callbacks right below here: > - Set mux to GPIO mode This should be done by just writing the hardware registers in these callbacks from struct pinmux_ops: int (*gpio_request_enable) (struct pinctrl_dev *pctldev, struct pinctrl_gpio_range *range, unsigned offset); void (*gpio_disable_free) (struct pinctrl_dev *pctldev, struct pinctrl_gpio_range *range, unsigned offset); int (*gpio_set_direction) (struct pinctrl_dev *pctldev, struct pinctrl_gpio_range *range, unsigned offset, bool input); In respons to these functions being called from the corresponding GPIO driver: extern int pinctrl_request_gpio(unsigned gpio); extern void pinctrl_free_gpio(unsigned gpio); extern int pinctrl_gpio_direction_input(unsigned gpio); extern int pinctrl_gpio_direction_output(unsigned gpio); > - Set GPIO direction to input This should be done in these callbacks from struct gpio_chip: int (*direction_input)(struct gpio_chip *chip, unsigned offset); int (*direction_output)(struct gpio_chip *chip, unsigned offset, int value); By just writing the hardware registers. > Currently user need to add a dts node to set pinmux to GPIO mode with > input-enable property, the driver need to call gpio_direction_input to > set direction. It seems like: - Your GPIO driver is not calling the pinctrl_request_gpio/ pinctrl_gpio_direction_* etc. - Your pinmux driver is not implementing the GPIO-specifif .gpio_* functions > Since input-enable and GPIO direction are 2 different things, we now > think this patch is inappropriate. Instead, user should not required to > add pinmux node in dts if all he want is use it as plain GPIO input. If you sort out the above callbacks (also read Documentation/pinctrl.txt about GPIO) I think it is clear what you need to do. Usually you should only need a special pinmux/pinconf node for a GPIO line if you need to specify pull-up or similar electronic things for that GPIO. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] pinctrl: mediatek: fix direction control issue 2016-02-03 1:24 ` [PATCH 1/2] pinctrl: mediatek: fix direction control issue Biao Huang 2016-02-04 3:03 ` Hongzhou Yang @ 2016-02-05 13:56 ` Linus Walleij 1 sibling, 0 replies; 10+ messages in thread From: Linus Walleij @ 2016-02-05 13:56 UTC (permalink / raw) To: linux-arm-kernel On Wed, Feb 3, 2016 at 2:24 AM, Biao Huang <biao.huang@mediatek.com> wrote: > Since input-enable/disable and input-schmitt-enable/disable are > workable when gpio direction is input, so add direction setting > when do input-enable/disable and input-schmitt-enable/disable > properties. > > Signed-off-by: Biao Huang <biao.huang@mediatek.com> This patch applied for fixes. I see now that it was part of the mt2701 patch set, so sorry for missing it... Yours, Linus Walleij ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] pinctrl: mediatek: Add gpio_request_enable support 2016-02-03 1:24 [PATCH 0/2] pinctrl: mediatek: Add direction control and gpio_request_enable Support Biao Huang 2016-02-03 1:24 ` [PATCH 1/2] pinctrl: mediatek: fix direction control issue Biao Huang @ 2016-02-03 1:24 ` Biao Huang 2016-02-05 13:58 ` Linus Walleij 1 sibling, 1 reply; 10+ messages in thread From: Biao Huang @ 2016-02-03 1:24 UTC (permalink / raw) To: linux-arm-kernel Implement the .gpio_request_enable() callbacks in struct pinmux_ops in mediatek pinctrl driver. Make sure that when gpio_request is called, GPIO on the pin is enabled. Signed-off-by: Biao Huang <biao.huang@mediatek.com> --- drivers/pinctrl/mediatek/pinctrl-mtk-common.c | 36 +++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common.c index 6eb01c9..cbc2204 100644 --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common.c +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common.c @@ -43,6 +43,7 @@ #define MAX_GPIO_MODE_PER_REG 5 #define GPIO_MODE_BITS 3 +#define GPIO_MODE_PREFIX "GPIO" static const char * const mtk_gpio_functions[] = { "func0", "func1", "func2", "func3", @@ -735,12 +736,47 @@ static int mtk_pmx_set_mux(struct pinctrl_dev *pctldev, return 0; } +static int mtk_pmx_find_gpio_mode(struct mtk_pinctrl *pctl, + unsigned offset) +{ + const struct mtk_desc_pin *pin = pctl->devdata->pins + offset; + const struct mtk_desc_function *func = pin->functions; + + while (func && func->name) { + if (!strncmp(func->name, GPIO_MODE_PREFIX, + sizeof(GPIO_MODE_PREFIX)-1)) + return func->muxval; + func++; + } + return -EINVAL; +} + +static int mtk_pmx_gpio_request_enable(struct pinctrl_dev *pctldev, + struct pinctrl_gpio_range *range, + unsigned offset) +{ + unsigned long muxval; + struct mtk_pinctrl *pctl = pinctrl_dev_get_drvdata(pctldev); + + muxval = mtk_pmx_find_gpio_mode(pctl, offset); + + if (muxval < 0) { + dev_err(pctl->dev, "invalid gpio pin %d.\n", offset); + return -EINVAL; + } + + mtk_pmx_set_mode(pctldev, offset, muxval); + + return 0; +} + static const struct pinmux_ops mtk_pmx_ops = { .get_functions_count = mtk_pmx_get_funcs_cnt, .get_function_name = mtk_pmx_get_func_name, .get_function_groups = mtk_pmx_get_func_groups, .set_mux = mtk_pmx_set_mux, .gpio_set_direction = mtk_pmx_gpio_set_direction, + .gpio_request_enable = mtk_pmx_gpio_request_enable, }; static int mtk_gpio_direction_input(struct gpio_chip *chip, -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/2] pinctrl: mediatek: Add gpio_request_enable support 2016-02-03 1:24 ` [PATCH 2/2] pinctrl: mediatek: Add gpio_request_enable support Biao Huang @ 2016-02-05 13:58 ` Linus Walleij 0 siblings, 0 replies; 10+ messages in thread From: Linus Walleij @ 2016-02-05 13:58 UTC (permalink / raw) To: linux-arm-kernel On Wed, Feb 3, 2016 at 2:24 AM, Biao Huang <biao.huang@mediatek.com> wrote: > Implement the .gpio_request_enable() callbacks in struct pinmux_ops > in mediatek pinctrl driver. Make sure that when gpio_request is called, > GPIO on the pin is enabled. > > Signed-off-by: Biao Huang <biao.huang@mediatek.com> Patch applied to my devel-mt2701 branch. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-02-16 12:28 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-02-03 1:24 [PATCH 0/2] pinctrl: mediatek: Add direction control and gpio_request_enable Support Biao Huang 2016-02-03 1:24 ` [PATCH 1/2] pinctrl: mediatek: fix direction control issue Biao Huang 2016-02-04 3:03 ` Hongzhou Yang 2016-02-05 13:52 ` Linus Walleij 2016-02-12 2:07 ` Hongzhou Yang 2016-02-16 8:54 ` Yingjoe Chen 2016-02-16 12:28 ` Linus Walleij 2016-02-05 13:56 ` Linus Walleij 2016-02-03 1:24 ` [PATCH 2/2] pinctrl: mediatek: Add gpio_request_enable support Biao Huang 2016-02-05 13:58 ` Linus Walleij
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).