GPIO request failure with PCF pinmux

GPIO request failure with PCF pinmux

[Laurent Pinchart]

Hi Paul,

I ran into an issue with GPIO and pinmuxing in the TPU PWM driver.

The driver needs to switch the PWM pin between GPIO and function at runtime. 
To do so, I use the PWM pin GPIO (GPIO_PORT202) and the associated function 
GPIO (GPIO_FN_TPU0TO2_PORT202).

The driver configures the pin as a GPIO output when loaded with

	gpio_request_one(GPIO_PORT202, GPIOF_INIT_LOW);

This results in a call to sh_pfc_gpio_request_enable() followed by a call to 
sh_pfc_reconfig_pin() for GPIO 202. The later changes the pinmux type from 
PINMUX_TYPE_GPIO (2) to PINMUX_TYPE_OUTPUT (3).

I then call gpio_free(GPIO_PORT202) and gpio_request(GPIO_FN_TPU0TO2_PORT202) 
to switch from GPIO to function. The former calls sh_pfc_gpio_disable_free:() 
on GPIO 202.

When I later call gpio_free(GPIO_FN_TPU0TO2_PORT202) and 
gpio_request_one(GPIO_PORT202, GPIOF_INIT_LOW), the call to 
sh_pfc_gpio_request_enable() fails with

pinctrl-sh_pfc pinctrl: Unsupported mux type (3), bailing...
pinctrl-sh_pfc pinctrl-sh_pfc: request() failed for pin 202
pinctrl-sh_pfc pinctrl-sh_pfc: pin-202 (pinctrl-sh_pfc) status -524

You're more familiar with pinmux than I am. Am I doing something wrong, or is 
there a bug in the PFC pinmux implementation ?

-- 
Regards,

Laurent Pinchart

Re: GPIO request failure with PCF pinmux

[Laurent Pinchart]

On Friday 14 September 2012 22:11:00 Laurent Pinchart wrote:
> Hi Paul,
> 
> I ran into an issue with GPIO and pinmuxing in the TPU PWM driver.
> 
> The driver needs to switch the PWM pin between GPIO and function at runtime.
> To do so, I use the PWM pin GPIO (GPIO_PORT202) and the associated function
> GPIO (GPIO_FN_TPU0TO2_PORT202).
> 
> The driver configures the pin as a GPIO output when loaded with
> 
> 	gpio_request_one(GPIO_PORT202, GPIOF_INIT_LOW);
> 
> This results in a call to sh_pfc_gpio_request_enable() followed by a call to
> sh_pfc_reconfig_pin() for GPIO 202. The later changes the pinmux type from
> PINMUX_TYPE_GPIO (2) to PINMUX_TYPE_OUTPUT (3).
> 
> I then call gpio_free(GPIO_PORT202) and
> gpio_request(GPIO_FN_TPU0TO2_PORT202) to switch from GPIO to function. The
> former calls sh_pfc_gpio_disable_free:() on GPIO 202.
> 
> When I later call gpio_free(GPIO_FN_TPU0TO2_PORT202) and
> gpio_request_one(GPIO_PORT202, GPIOF_INIT_LOW), the call to
> sh_pfc_gpio_request_enable() fails with
> 
> pinctrl-sh_pfc pinctrl: Unsupported mux type (3), bailing...
> pinctrl-sh_pfc pinctrl-sh_pfc: request() failed for pin 202
> pinctrl-sh_pfc pinctrl-sh_pfc: pin-202 (pinctrl-sh_pfc) status -524
> 
> You're more familiar with pinmux than I am. Am I doing something wrong, or
> is there a bug in the PFC pinmux implementation ?

And while I'm at it, trying to set the pin to the PWM function doesn't seem to 
work anymore on v3.6. It looks like the pin stays configured as a GPIO. How am 
I supposed to configure pinmuxing from the driver ? Documentation/pinctrl.txt 
doesn't help much, and the code isn't easy to understand. There's probably no 
registered mapping (whatever a mapping is), are they supposed to come from DT 
?

-- 
Regards,

Laurent Pinchart

Re: GPIO request failure with PCF pinmux

[Paul Mundt]

On Fri, Sep 14, 2012 at 10:11:00PM +0200, Laurent Pinchart wrote:
> Hi Paul,
> 
> I ran into an issue with GPIO and pinmuxing in the TPU PWM driver.
> 
> The driver needs to switch the PWM pin between GPIO and function at runtime. 
> To do so, I use the PWM pin GPIO (GPIO_PORT202) and the associated function 
> GPIO (GPIO_FN_TPU0TO2_PORT202).
> 
> The driver configures the pin as a GPIO output when loaded with
> 
> 	gpio_request_one(GPIO_PORT202, GPIOF_INIT_LOW);
> 
> This results in a call to sh_pfc_gpio_request_enable() followed by a call to 
> sh_pfc_reconfig_pin() for GPIO 202. The later changes the pinmux type from 
> PINMUX_TYPE_GPIO (2) to PINMUX_TYPE_OUTPUT (3).
> 
> I then call gpio_free(GPIO_PORT202) and gpio_request(GPIO_FN_TPU0TO2_PORT202) 
> to switch from GPIO to function. The former calls sh_pfc_gpio_disable_free:() 
> on GPIO 202.
> 
> When I later call gpio_free(GPIO_FN_TPU0TO2_PORT202) and 
> gpio_request_one(GPIO_PORT202, GPIOF_INIT_LOW), the call to 
> sh_pfc_gpio_request_enable() fails with
> 
> pinctrl-sh_pfc pinctrl: Unsupported mux type (3), bailing...
> pinctrl-sh_pfc pinctrl-sh_pfc: request() failed for pin 202
> pinctrl-sh_pfc pinctrl-sh_pfc: pin-202 (pinctrl-sh_pfc) status -524
> 
> You're more familiar with pinmux than I am. Am I doing something wrong, or is 
> there a bug in the PFC pinmux implementation ?
> 
That's a bug, the pin has been reconfigured to a different mux type which
we should handle. At first glance, simply handling the input/output types
ought to work ok, but we may need more work than that. How does it go
with this?

---

diff --git a/drivers/sh/pfc/pinctrl.c b/drivers/sh/pfc/pinctrl.c
index 2804eaa..c0857d5 100644
--- a/drivers/sh/pfc/pinctrl.c
+++ b/drivers/sh/pfc/pinctrl.c
@@ -208,6 +208,8 @@ static int sh_pfc_gpio_request_enable(struct pinctrl_dev *pctldev,
 
 		break;
 	case PINMUX_TYPE_GPIO:
+	case PINMUX_TYPE_INPUT:
+	case PINMUX_TYPE_OUTPUT:
 		break;
 	default:
 		pr_err("Unsupported mux type (%d), bailing...\n", pinmux_type);

Re: GPIO request failure with PCF pinmux

[Paul Mundt]

On Fri, Sep 14, 2012 at 10:36:34PM +0200, Laurent Pinchart wrote:
> And while I'm at it, trying to set the pin to the PWM function doesn't seem to 
> work anymore on v3.6. It looks like the pin stays configured as a GPIO. How am 
> I supposed to configure pinmuxing from the driver ? Documentation/pinctrl.txt 
> doesn't help much, and the code isn't easy to understand. There's probably no 
> registered mapping (whatever a mapping is), are they supposed to come from DT 
> ?

For the moment requesting as a function as before should be fine,
although it's possible that your config-as-gpio reconfig-as-function flow
has tripped up some of the logic. Both were tested independently, but as
you can tell I've overlooked the reuse of the same pin by the same driver
in different ways case.

And yes, you've hit on the next phase of the pfc rework. At present we
have a GPIO shim that enables function requests via the GPIO API as
before, but it's not the way we want to go forward and remains there only
for backwards compatability.

More work needs to be done both in the drivers and the board/platform to
make use of pinctrl mappings. I have some work in progress for sh-sci,
but ran out of time to get it merged for this merge window.

The DT support is something else that has to be worked out. I personally
have no interest in supporting DT, as I view the entire endeavour as
nothing but a solution in search of a problem. If someone wants DT
support for this they're welcome to hack something up, though. I won't
oppose DT changes going in, regardless of whether they're ultimately a
waste of time or not.

Re: GPIO request failure with PCF pinmux

[Laurent Pinchart]

Hi Paul,

On Tuesday 18 September 2012 16:12:08 Paul Mundt wrote:
> On Fri, Sep 14, 2012 at 10:11:00PM +0200, Laurent Pinchart wrote:
> > Hi Paul,
> > 
> > I ran into an issue with GPIO and pinmuxing in the TPU PWM driver.
> > 
> > The driver needs to switch the PWM pin between GPIO and function at
> > runtime. To do so, I use the PWM pin GPIO (GPIO_PORT202) and the
> > associated function GPIO (GPIO_FN_TPU0TO2_PORT202).
> > 
> > The driver configures the pin as a GPIO output when loaded with
> > 
> > 	gpio_request_one(GPIO_PORT202, GPIOF_INIT_LOW);
> > 
> > This results in a call to sh_pfc_gpio_request_enable() followed by a call
> > to sh_pfc_reconfig_pin() for GPIO 202. The later changes the pinmux type
> > from PINMUX_TYPE_GPIO (2) to PINMUX_TYPE_OUTPUT (3).
> > 
> > I then call gpio_free(GPIO_PORT202) and
> > gpio_request(GPIO_FN_TPU0TO2_PORT202) to switch from GPIO to function.
> > The former calls sh_pfc_gpio_disable_free:() on GPIO 202.
> > 
> > When I later call gpio_free(GPIO_FN_TPU0TO2_PORT202) and
> > gpio_request_one(GPIO_PORT202, GPIOF_INIT_LOW), the call to
> > sh_pfc_gpio_request_enable() fails with
> > 
> > pinctrl-sh_pfc pinctrl: Unsupported mux type (3), bailing...
> > pinctrl-sh_pfc pinctrl-sh_pfc: request() failed for pin 202
> > pinctrl-sh_pfc pinctrl-sh_pfc: pin-202 (pinctrl-sh_pfc) status -524
> > 
> > You're more familiar with pinmux than I am. Am I doing something wrong, or
> > is there a bug in the PFC pinmux implementation ?
> 
> That's a bug, the pin has been reconfigured to a different mux type which
> we should handle. At first glance, simply handling the input/output types
> ought to work ok, but we may need more work than that. How does it go
> with this?

That seems to work, thank you. Will you push a patch ?

> ---
> 
> diff --git a/drivers/sh/pfc/pinctrl.c b/drivers/sh/pfc/pinctrl.c
> index 2804eaa..c0857d5 100644
> --- a/drivers/sh/pfc/pinctrl.c
> +++ b/drivers/sh/pfc/pinctrl.c
> @@ -208,6 +208,8 @@ static int sh_pfc_gpio_request_enable(struct pinctrl_dev
> *pctldev,
> 
>  		break;
>  	case PINMUX_TYPE_GPIO:
> +	case PINMUX_TYPE_INPUT:
> +	case PINMUX_TYPE_OUTPUT:
>  		break;
>  	default:
>  		pr_err("Unsupported mux type (%d), bailing...\n", pinmux_type);

-- 
Regards,

Laurent Pinchart

Re: GPIO request failure with PCF pinmux

[Paul Mundt]

On Wed, Sep 19, 2012 at 05:19:44PM +0200, Laurent Pinchart wrote:
> On Tuesday 18 September 2012 16:12:08 Paul Mundt wrote:
> > That's a bug, the pin has been reconfigured to a different mux type which
> > we should handle. At first glance, simply handling the input/output types
> > ought to work ok, but we may need more work than that. How does it go
> > with this?
> 
> That seems to work, thank you. Will you push a patch ?
> 
Yes, I'll send it off with the next batch of fixes. Thanks for testing!

Re: GPIO request failure with PCF pinmux

[Laurent Pinchart]

Hi Paul,

On Tuesday 18 September 2012 16:22:38 Paul Mundt wrote:
> On Fri, Sep 14, 2012 at 10:36:34PM +0200, Laurent Pinchart wrote:
> > And while I'm at it, trying to set the pin to the PWM function doesn't
> > seem to work anymore on v3.6. It looks like the pin stays configured as a
> > GPIO. How am I supposed to configure pinmuxing from the driver ?
> > Documentation/pinctrl.txt doesn't help much, and the code isn't easy to
> > understand. There's probably no registered mapping (whatever a mapping
> > is), are they supposed to come from DT ?
> 
> For the moment requesting as a function as before should be fine,
> although it's possible that your config-as-gpio reconfig-as-function flow
> has tripped up some of the logic. Both were tested independently, but as
> you can tell I've overlooked the reuse of the same pin by the same driver
> in different ways case.
> 
> And yes, you've hit on the next phase of the pfc rework. At present we
> have a GPIO shim that enables function requests via the GPIO API as
> before, but it's not the way we want to go forward and remains there only
> for backwards compatability.
> 
> More work needs to be done both in the drivers and the board/platform to
> make use of pinctrl mappings. I have some work in progress for sh-sci,
> but ran out of time to get it merged for this merge window.

What's the status of that work ? I'll have to implement pinctrl support for 
the H1 series, and having sample code would be helpful. Are there still core 
pieces missing ?

> The DT support is something else that has to be worked out. I personally
> have no interest in supporting DT, as I view the entire endeavour as nothing
> but a solution in search of a problem. If someone wants DT support for this
> they're welcome to hack something up, though. I won't oppose DT changes
> going in, regardless of whether they're ultimately a waste of time or not.

-- 
Regards,

Laurent Pinchart