Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH 2/4] pwm: stm32-lp: Add power management support
To:     Thierry Reding <thierry.reding@gmail.com>,
        =?UTF-8?Q?Uwe_Kleine-K=c3=b6nig?= <u.kleine-koenig@pengutronix.de>
CC:     <jic23@kernel.org>, <robh+dt@kernel.org>, <mark.rutland@arm.com>,
        <alexandre.torgue@st.com>, <mcoquelin.stm32@gmail.com>,
        <linux-iio@vger.kernel.org>, <devicetree@vger.kernel.org>,
        <linux-arm-kernel@lists.infradead.org>,
        <linux-kernel@vger.kernel.org>, <linux-pwm@vger.kernel.org>,
        <vilhelm.gray@gmail.com>,
        <linux-stm32@st-md-mailman.stormreply.com>,
        Tomasz Duszynski <tduszyns@gmail.com>
References: <1549370429-19116-1-git-send-email-fabrice.gasnier@st.com>
 <1549370429-19116-3-git-send-email-fabrice.gasnier@st.com>
 <20190205204732.zrbhgyxnvjbwfyw4@pengutronix.de>
 <20190205222522.GB1372@mithrandir>
 <6584a59b-657d-adc9-fab2-eb1a9baba05d@st.com>
 <20190206085405.pdiwd3c6wjog5hl7@pengutronix.de>
 <20190206125545.GI21676@ulmo>
From:   Fabrice Gasnier <fabrice.gasnier@st.com>
Message-ID: <ddb93754-2734-e4c2-a209-517984c6d814@st.com>
Date:   Wed, 6 Feb 2019 15:54:50 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.4.0
MIME-Version: 1.0
In-Reply-To: <20190206125545.GI21676@ulmo>
Content-Type: text/plain; charset="utf-8"
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 2/6/19 1:55 PM, Thierry Reding wrote:
> On Wed, Feb 06, 2019 at 09:54:05AM +0100, Uwe Kleine-König wrote:
>> On Wed, Feb 06, 2019 at 09:42:48AM +0100, Fabrice Gasnier wrote:
>>> If you agree with the current approach, I can send a V2 with Tomasz's
>>> suggestion to remove the ifdefs and use __maybe_unused instead.
>>
>> I think the suspend callback should have something like:
>>
>> 	if (is_still_enabled) {
>> 		/*
>> 		 * The consumer didn't stop us, so refuse to suspend.
>> 		 */
>> 		dev_err(dev, "The consumer didn't stop us, so refuse to suspend.\n");
>> 		return -EBUSY;
>> 	}
>>
>> This way there are no bad surprises if the pwm is suspended before its
>> consumer and it's obvious what is missing.

Thierry, Uwe,

When the pwm is suspended before its consumer, the bad surprise is the
suspend request will fail... I'm not sure a new attempt may be better.
So, it looks like the only way to have this clean is by implementing the
device link e.g. via pwm_get() ?

> 
> Something that just occurred to me: perhaps as part of pwm_get() we
> should track where we were being requested from so that we could give
> hints in situations like this as to where the consumer is that forgot
> to suspend the PWM.

The current approach handles the situation where the consumer forgot to
suspend the PWM... I can add some warning about that in the suspend
routine, incl the label.

What do you think? What's the best approach ?

Please advise,
BR,
Fabrice
> 
> I suppose we already have pwm_device.label to help with this, but
> perhaps we could improve things if we stored __builtin_return_address
> during pwm_get() to help users pinpoint where they need to look.
> 
> Thierry
>