On 24 March 2016 at 15:31, Sergei Shtylyov <sergei.shtylyov@xxxxxxxxxxxxxxxxxx> wrote: > On 03/24/2016 05:26 PM, Rafał Miłecki wrote: >>>>>>>> Thanks to switching to devm_gpiod_get: >>>>>>>> 1) We don't have to pass fwnode pointer >>>>>>>> 2) We can request initial GPIO value at getting call >>>>>>>> This was successfully tested on Netgear R6250 (BCM4708). >>>>>>>> >>>>>>>> Signed-off-by: Rafał Miłecki <zajec5@xxxxxxxxx> >>>>>>>> --- >>>>>>>> drivers/usb/host/bcma-hcd.c | 6 ++---- >>>>>>>> 1 file changed, 2 insertions(+), 4 deletions(-) >>>>>>>> >>>>>>>> diff --git a/drivers/usb/host/bcma-hcd.c >>>>>>>> b/drivers/usb/host/bcma-hcd.c >>>>>>>> index 963e2d0..172ef17 100644 >>>>>>>> --- a/drivers/usb/host/bcma-hcd.c >>>>>>>> +++ b/drivers/usb/host/bcma-hcd.c >>>>>>>> @@ -352,10 +352,8 @@ static int bcma_hcd_probe(struct bcma_device >>>>>>>> *core) >>>>>>>> usb_dev->core = core; >>>>>>>> >>>>>>>> if (core->dev.of_node) >>>>>>>> - usb_dev->gpio_desc = >>>>>>>> devm_get_gpiod_from_child(&core->dev, "vcc", >>>>>>>> - >>>>>>>> &core->dev.of_node->fwnode); >>>>>>>> - if (!IS_ERR_OR_NULL(usb_dev->gpio_desc)) >>>>>>>> - gpiod_direction_output(usb_dev->gpio_desc, 1); >>>>>>>> + usb_dev->gpio_desc = devm_gpiod_get(&core->dev, "vcc", >>>>>>>> + GPIOD_OUT_HIGH); >>>>>>> >>>>>>> >>>>>>> Do you need to check the returned descriptor? >>>>>> >>>>>> >>>>>> Do you mean like in future, after initialization? Yes. You can check >>>>>> it >>>>>> with >>>>>> grep gpio_desc drivers/usb/host/bcma-hcd.c >>>>> >>>>> >>>>> >>>>> Oh, sorry! >>>>> >>>>> I mean you need to check the pointer returned by devm_gpiod_get(). >>>> >>>> >>>> >>>> Sure. I don't need to. Only some of devices have USB power controlled >>>> using GPIO, so having this entry in DT is optional. >>> >>> >>> >>> Then use devm_gpiod_get_optional()? >> >> >> I see this function (_optional) hides err number from caller. I'm not >> sure why it should matter for us. Is there any difference at the end? > > > It only hides an error caused by missing prop, so that you can filter out > the other errors. So there isn't any point in using this _optional function with current code. It wouldn't change anything as we don't care about -ENOENT vs. other errors. We could think about adding dev_warn in case of an error (other than -ENOENT) but this wouldn't work as expected on devices that share one GPIO to power multiple USB ports. In such cases -EBUSY is expected if GPIO was already requested by us (just for a different USB controller). So I think original patch is still OK. -- Rafał -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
