From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756011AbcIGH0C (ORCPT ); Wed, 7 Sep 2016 03:26:02 -0400 Received: from arroyo.ext.ti.com ([198.47.19.12]:35683 "EHLO arroyo.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751657AbcIGH0A (ORCPT ); Wed, 7 Sep 2016 03:26:00 -0400 Subject: Re: [PATCH] usb: phy: generic: request regulator optionally To: Stefan Agner , Mark Brown References: <20160904040416.22163-1-stefan@agner.ch> <87mvjlmpyo.fsf@linux.intel.com> <20160906082210.GE3950@sirena.org.uk> <56840b0a8520f348ee0517390f518274@agner.ch> CC: Felipe Balbi , , , , From: Roger Quadros Message-ID: <32e64ab2-2ed6-e177-b1ec-bce9e1eaa4b6@ti.com> Date: Wed, 7 Sep 2016 10:25:47 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <56840b0a8520f348ee0517390f518274@agner.ch> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Stefan, On 06/09/16 21:01, Stefan Agner wrote: > On 2016-09-06 01:22, Mark Brown wrote: >> On Tue, Sep 06, 2016 at 10:45:19AM +0300, Felipe Balbi wrote: >>> Stefan Agner writes: >> >>>> According to the device tree bindings the vcc-supply is optional. >> >> This is nonsense unless the device can work without this supply. Given >> that the supply is called VCC that doesn't seem entirely likely. > > Afaik it is kind of a generic device tree binding, I guess the physical > device can have various appearances and properties... > > A quick survey showed several device trees which do not specify > vcc-supply... > > That said, I checked the device at hand, and it actually has a USB PHY > power supply inputs, but the device tree does not model them. > >>>> + nop->vcc = devm_regulator_get_optional(dev, "vcc"); >>>> if (IS_ERR(nop->vcc)) { >>>> dev_dbg(dev, "Error getting vcc regulator: %ld\n", >>>> PTR_ERR(nop->vcc)); >>>> - if (needs_vcc) >>>> - return -EPROBE_DEFER; >>>> + if (needs_vcc || PTR_ERR(nop->vcc) == -EPROBE_DEFER) >>>> + return PTR_ERR(nop->vcc); >> >>> does this look okay from a regulator API perspective? >> >> That's how to use _get_optional() but it's really unusual that you >> should be using _get_optional(). > > Despite the above findings, I still think it is the right thing to do as > long as we specify vcc-supply to be optional. > I think the right behaviour would be that if vcc-supply is specified in the DT then failure to get that supply is a serious failure and probe should fail. So the correct fix would be to call devm_regulator_get() only if needs_vcc is true. cheers, -roger