[PATCH v2 REGRESSION RESEND] usb: ohci-at91: use descriptor-based gpio APIs correctly

[PATCH v2 REGRESSION RESEND] usb: ohci-at91: use descriptor-based gpio APIs correctly

[Peter Rosin]

The gpiod_get* function family does not want the -gpio suffix.
Use devm_gpiod_get_index_optional instead of devm_gpiod_get_optional.
The descriptor based APIs handle active high/low automatically.
The vbus-gpios are output, request enable while getting the gpio.
Don't try to get any vbus-gpios for ports outside num-ports.

WTF? Big sigh.

Fixes: 054d4b7b577d ("usb: ohci-at91: Use descriptor-based gpio APIs")
Signed-off-by: Peter Rosin <peda@axentia.se>
---

Hi!

Resending this, since the only response I've got is that the merge
window is open and that this patch has been put on hold due to that.
But I think this regression (which happend between v4.9 and current
master) should be fixed before the merge window closes.

v1 -> v2 changes:
- use devm_gpiod_get_index_optional instead of devm_gpiod_get_optional

Cheers,
peda

 drivers/usb/host/ohci-at91.c | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
index 1b2e09c32c6b..66ec407df2db 100644
--- a/drivers/usb/host/ohci-at91.c
+++ b/drivers/usb/host/ohci-at91.c
@@ -43,7 +43,6 @@ struct at91_usbh_data {
 	struct gpio_desc *overcurrent_pin[AT91_MAX_USBH_PORTS];
 	u8 ports;				/* number of ports on root hub */
 	u8 overcurrent_supported;
-	u8 vbus_pin_active_low[AT91_MAX_USBH_PORTS];
 	u8 overcurrent_status[AT91_MAX_USBH_PORTS];
 	u8 overcurrent_changed[AT91_MAX_USBH_PORTS];
 };
@@ -268,8 +267,7 @@ static void ohci_at91_usb_set_power(struct at91_usbh_data *pdata, int port, int
 	if (!valid_port(port))
 		return;
 
-	gpiod_set_value(pdata->vbus_pin[port],
-			pdata->vbus_pin_active_low[port] ^ enable);
+	gpiod_set_value(pdata->vbus_pin[port], enable);
 }
 
 static int ohci_at91_usb_get_power(struct at91_usbh_data *pdata, int port)
@@ -277,8 +275,7 @@ static int ohci_at91_usb_get_power(struct at91_usbh_data *pdata, int port)
 	if (!valid_port(port))
 		return -EINVAL;
 
-	return gpiod_get_value(pdata->vbus_pin[port]) ^
-	       pdata->vbus_pin_active_low[port];
+	return gpiod_get_value(pdata->vbus_pin[port]);
 }
 
 /*
@@ -535,18 +532,17 @@ static int ohci_hcd_at91_drv_probe(struct platform_device *pdev)
 		pdata->ports = ports;
 
 	at91_for_each_port(i) {
-		pdata->vbus_pin[i] = devm_gpiod_get_optional(&pdev->dev,
-							     "atmel,vbus-gpio",
-							     GPIOD_IN);
+		if (i >= pdata->ports)
+			break;
+
+		pdata->vbus_pin[i] =
+			devm_gpiod_get_index_optional(&pdev->dev, "atmel,vbus",
+						      i, GPIOD_OUT_HIGH);
 		if (IS_ERR(pdata->vbus_pin[i])) {
 			err = PTR_ERR(pdata->vbus_pin[i]);
 			dev_err(&pdev->dev, "unable to claim gpio \"vbus\": %d\n", err);
 			continue;
 		}
-
-		pdata->vbus_pin_active_low[i] = gpiod_get_value(pdata->vbus_pin[i]);
-
-		ohci_at91_usb_set_power(pdata, i, 1);
 	}
 
 	at91_for_each_port(i) {
@@ -554,8 +550,8 @@ static int ohci_hcd_at91_drv_probe(struct platform_device *pdev)
 			break;
 
 		pdata->overcurrent_pin[i] =
-			devm_gpiod_get_optional(&pdev->dev,
-						"atmel,oc-gpio", GPIOD_IN);
+			devm_gpiod_get_index_optional(&pdev->dev, "atmel,oc",
+						      i, GPIOD_IN);
 		if (IS_ERR(pdata->overcurrent_pin[i])) {
 			err = PTR_ERR(pdata->overcurrent_pin[i]);
 			dev_err(&pdev->dev, "unable to claim gpio \"overcurrent\": %d\n", err);
-- 
2.1.4

Re: [PATCH v2 REGRESSION RESEND] usb: ohci-at91: use descriptor-based gpio APIs correctly

[Greg Kroah-Hartman]

On Thu, Dec 22, 2016 at 08:43:55AM +0100, Peter Rosin wrote:
> The gpiod_get* function family does not want the -gpio suffix.
> Use devm_gpiod_get_index_optional instead of devm_gpiod_get_optional.
> The descriptor based APIs handle active high/low automatically.
> The vbus-gpios are output, request enable while getting the gpio.
> Don't try to get any vbus-gpios for ports outside num-ports.
> 
> WTF? Big sigh.
> 
> Fixes: 054d4b7b577d ("usb: ohci-at91: Use descriptor-based gpio APIs")
> Signed-off-by: Peter Rosin <peda@axentia.se>
> ---
> 
> Hi!
> 
> Resending this, since the only response I've got is that the merge
> window is open and that this patch has been put on hold due to that.
> But I think this regression (which happend between v4.9 and current
> master) should be fixed before the merge window closes.

I don't merge patches before -rc1 comes out, sorry, people should have
tested linux-next better :)

I'll catch up the first week of January, relax.

greg k-h

Re: [PATCH v2 REGRESSION RESEND] usb: ohci-at91: use descriptor-based gpio APIs correctly

[Peter Rosin]

On 2016-12-22 18:27, Greg Kroah-Hartman wrote:
> On Thu, Dec 22, 2016 at 08:43:55AM +0100, Peter Rosin wrote:
>> The gpiod_get* function family does not want the -gpio suffix.
>> Use devm_gpiod_get_index_optional instead of devm_gpiod_get_optional.
>> The descriptor based APIs handle active high/low automatically.
>> The vbus-gpios are output, request enable while getting the gpio.
>> Don't try to get any vbus-gpios for ports outside num-ports.
>>
>> WTF? Big sigh.
>>
>> Fixes: 054d4b7b577d ("usb: ohci-at91: Use descriptor-based gpio APIs")
>> Signed-off-by: Peter Rosin <peda@axentia.se>
>> ---
>>
>> Hi!
>>
>> Resending this, since the only response I've got is that the merge
>> window is open and that this patch has been put on hold due to that.
>> But I think this regression (which happend between v4.9 and current
>> master) should be fixed before the merge window closes.
> 
> I don't merge patches before -rc1 comes out, sorry, people should have
> tested linux-next better :)

Neat, shift the blame for the shit patch over to the messenger :)

> I'll catch up the first week of January, relax.

As we all know, unrelated regressions are painful when trying to locate
other problems. It's seems silly to have a few extra for no good reason.

*I* know about this one, it's those you don't know about that are worst.

Cheers,
peda

Re: [PATCH v2 REGRESSION RESEND] usb: ohci-at91: use descriptor-based gpio APIs correctly

[Greg Kroah-Hartman]

On Thu, Dec 22, 2016 at 09:38:08PM +0100, Peter Rosin wrote:
> On 2016-12-22 18:27, Greg Kroah-Hartman wrote:
> > On Thu, Dec 22, 2016 at 08:43:55AM +0100, Peter Rosin wrote:
> >> The gpiod_get* function family does not want the -gpio suffix.
> >> Use devm_gpiod_get_index_optional instead of devm_gpiod_get_optional.
> >> The descriptor based APIs handle active high/low automatically.
> >> The vbus-gpios are output, request enable while getting the gpio.
> >> Don't try to get any vbus-gpios for ports outside num-ports.
> >>
> >> WTF? Big sigh.
> >>
> >> Fixes: 054d4b7b577d ("usb: ohci-at91: Use descriptor-based gpio APIs")
> >> Signed-off-by: Peter Rosin <peda@axentia.se>
> >> ---
> >>
> >> Hi!
> >>
> >> Resending this, since the only response I've got is that the merge
> >> window is open and that this patch has been put on hold due to that.
> >> But I think this regression (which happend between v4.9 and current
> >> master) should be fixed before the merge window closes.
> > 
> > I don't merge patches before -rc1 comes out, sorry, people should have
> > tested linux-next better :)
> 
> Neat, shift the blame for the shit patch over to the messenger :)

Not at all, I blame the original developer :)

> > I'll catch up the first week of January, relax.
> 
> As we all know, unrelated regressions are painful when trying to locate
> other problems. It's seems silly to have a few extra for no good reason.

I am supposed to be on vacation and not reading email until the 3rd of
January, relax, we will catch up on stuff like this, and other minor
things, soon enough, in plenty of time for 4.10-final.

thanks,

greg k-h

RE: [PATCH v2 REGRESSION RESEND] usb: ohci-at91: use descriptor-based gpio APIs correctly

[Wenyou.Yang]

> -----Original Message-----
> From: Greg Kroah-Hartman [mailto:gregkh@linuxfoundation.org]
> Sent: 2016年12月24日 2:05
> To: Peter Rosin <peda@axentia.se>
> Cc: linux-kernel@vger.kernel.org; Alan Stern <stern@rowland.harvard.edu>;
> linux-usb@vger.kernel.org; Wenyou Yang - A41535
> <Wenyou.Yang@microchip.com>
> Subject: Re: [PATCH v2 REGRESSION RESEND] usb: ohci-at91: use descriptor-
> based gpio APIs correctly
> 
> On Thu, Dec 22, 2016 at 09:38:08PM +0100, Peter Rosin wrote:
> > On 2016-12-22 18:27, Greg Kroah-Hartman wrote:
> > > On Thu, Dec 22, 2016 at 08:43:55AM +0100, Peter Rosin wrote:
> > >> The gpiod_get* function family does not want the -gpio suffix.
> > >> Use devm_gpiod_get_index_optional instead of devm_gpiod_get_optional.
> > >> The descriptor based APIs handle active high/low automatically.
> > >> The vbus-gpios are output, request enable while getting the gpio.
> > >> Don't try to get any vbus-gpios for ports outside num-ports.
> > >>
> > >> WTF? Big sigh.
> > >>
> > >> Fixes: 054d4b7b577d ("usb: ohci-at91: Use descriptor-based gpio
> > >> APIs")
> > >> Signed-off-by: Peter Rosin <peda@axentia.se>
> > >> ---
> > >>
> > >> Hi!
> > >>
> > >> Resending this, since the only response I've got is that the merge
> > >> window is open and that this patch has been put on hold due to that.
> > >> But I think this regression (which happend between v4.9 and current
> > >> master) should be fixed before the merge window closes.
> > >
> > > I don't merge patches before -rc1 comes out, sorry, people should
> > > have tested linux-next better :)
> >
> > Neat, shift the blame for the shit patch over to the messenger :)
> 
> Not at all, I blame the original developer :)

I am very very sorry. It is my ignorance. Sorry.

I tested this patch on linux-next branch on the SAMA5D3 and SAMA5D4 Xplained board.

> 
> > > I'll catch up the first week of January, relax.
> >
> > As we all know, unrelated regressions are painful when trying to
> > locate other problems. It's seems silly to have a few extra for no good reason.
> 
> I am supposed to be on vacation and not reading email until the 3rd of January,
> relax, we will catch up on stuff like this, and other minor things, soon enough, in
> plenty of time for 4.10-final.
> 
> thanks,
> 
> greg k-h


Best Regards,
Wenyou Yang