| Message ID | 20190327210045.21066-1-jae.hyun.yoo@linux.intel.com (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
| Series | Input: pwm-beeper - make 'amp' as an option | expand |
Hi Jae Hyun, On Wed, Mar 27, 2019 at 02:00:45PM -0700, Jae Hyun Yoo wrote: > An amplifier regulator property should be an option according to > the binding document but currently it's not so this commit makes > the amplifier regulator property as an option actually for cases > that don't have the actual amplifier H/W. If regulator is not described in DTS you will get a dummy one (i.e. devm_regulator_get() will not fail) and everything is good. This patch is not needed. Thanks.
Hi Dmitry, On 3/27/2019 2:49 PM, Dmitry Torokhov wrote: > Hi Jae Hyun, > > On Wed, Mar 27, 2019 at 02:00:45PM -0700, Jae Hyun Yoo wrote: >> An amplifier regulator property should be an option according to >> the binding document but currently it's not so this commit makes >> the amplifier regulator property as an option actually for cases >> that don't have the actual amplifier H/W. > > If regulator is not described in DTS you will get a dummy one (i.e. > devm_regulator_get() will not fail) and everything is good. > > This patch is not needed. Ah, I confused because I got a null beeper->amplifier since I didn't enable CONFIG_REGULATOR. Even in that case, IS_ERR() doesn't treat a null as an error so it doesn't matter, and regulator_enable/disable functions does nothing on the null regulator param so this patch isn't needed. If CONFIG_REGULATOR is enabled, it'll get a dummy regulator as you said so it doesn't need this patch too. Please ignore this patch. Thanks, Jae
diff --git a/drivers/input/misc/pwm-beeper.c b/drivers/input/misc/pwm-beeper.c index edca0d737750..a3baa52f187f 100644 --- a/drivers/input/misc/pwm-beeper.c +++ b/drivers/input/misc/pwm-beeper.c @@ -52,7 +52,7 @@ static int pwm_beeper_on(struct pwm_beeper *beeper, unsigned long period) if (error) return error; - if (!beeper->amplifier_on) { + if (beeper->amplifier && !beeper->amplifier_on) { error = regulator_enable(beeper->amplifier); if (error) { pwm_disable(beeper->pwm); @@ -67,7 +67,7 @@ static int pwm_beeper_on(struct pwm_beeper *beeper, unsigned long period) static void pwm_beeper_off(struct pwm_beeper *beeper) { - if (beeper->amplifier_on) { + if (beeper->amplifier && beeper->amplifier_on) { regulator_disable(beeper->amplifier); beeper->amplifier_on = false; } @@ -163,9 +163,9 @@ static int pwm_beeper_probe(struct platform_device *pdev) if (IS_ERR(beeper->amplifier)) { error = PTR_ERR(beeper->amplifier); if (error != -EPROBE_DEFER) - dev_err(dev, "Failed to get 'amp' regulator: %d\n", + dev_dbg(dev, "Failed to get 'amp' regulator: %d\n", error); - return error; + beeper->amplifier = NULL; } INIT_WORK(&beeper->work, pwm_beeper_work);
An amplifier regulator property should be an option according to the binding document but currently it's not so this commit makes the amplifier regulator property as an option actually for cases that don't have the actual amplifier H/W. Fixes: 9e5492443278 ("Input: pwm-beeper - add optional amplifier regulator") Cc: David Lechner <david@lechnology.com> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com> --- drivers/input/misc/pwm-beeper.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)