diff mbox series

ASoC: qcom: lpass-sc7180: Add maybe_unused tag for system PM ops

Message ID 1669726428-3140-1-git-send-email-quic_srivasam@quicinc.com (mailing list archive)
State Superseded
Commit e8679db2970f04ee5281c042977fff880a3c045a
Headers show
Series ASoC: qcom: lpass-sc7180: Add maybe_unused tag for system PM ops | expand

Commit Message

Srinivasa Rao Mandadapu Nov. 29, 2022, 12:53 p.m. UTC
Add __maybe_unused tag for system PM ops suspend and resume.
This is required to fix allmodconfig compilation issue.
Fixes: c3bf7699747c ("ASoC: qcom: lpass-sc7280: Add system suspend/resume PM ops")

Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
---
 sound/soc/qcom/lpass-sc7180.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Mark Brown Nov. 29, 2022, 2:43 p.m. UTC | #1
On Tue, Nov 29, 2022 at 06:23:48PM +0530, Srinivasa Rao Mandadapu wrote:
> Add __maybe_unused tag for system PM ops suspend and resume.
> This is required to fix allmodconfig compilation issue.
> Fixes: c3bf7699747c ("ASoC: qcom: lpass-sc7280: Add system suspend/resume PM ops")

That commit doesn't exist...
Mark Brown Nov. 29, 2022, 3:40 p.m. UTC | #2
On Tue, 29 Nov 2022 18:23:48 +0530, Srinivasa Rao Mandadapu wrote:
> Add __maybe_unused tag for system PM ops suspend and resume.
> This is required to fix allmodconfig compilation issue.
> Fixes: c3bf7699747c ("ASoC: qcom: lpass-sc7280: Add system suspend/resume PM ops")
> 
> 

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/1] ASoC: qcom: lpass-sc7180: Add maybe_unused tag for system PM ops
      commit: e8679db2970f04ee5281c042977fff880a3c045a

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark
Nathan Chancellor Nov. 29, 2022, 3:45 p.m. UTC | #3
On Tue, Nov 29, 2022 at 06:23:48PM +0530, Srinivasa Rao Mandadapu wrote:
> Add __maybe_unused tag for system PM ops suspend and resume.
> This is required to fix allmodconfig compilation issue.
> Fixes: c3bf7699747c ("ASoC: qcom: lpass-sc7280: Add system suspend/resume PM ops")
> 
> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>

A better solution would be replacing SET_SYSTEM_SLEEP_PM_OPS() with
SYSTEM_SLEEP_PM_OPS(), which was added to avoid needing to add these
'__maybe_unused' attributes to these functions. See commit 1a3c7bb08826
("PM: core: Add new *_PM_OPS macros, deprecate old ones") for more info.

> ---
>  sound/soc/qcom/lpass-sc7180.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/soc/qcom/lpass-sc7180.c b/sound/soc/qcom/lpass-sc7180.c
> index b96b85a..41db661 100644
> --- a/sound/soc/qcom/lpass-sc7180.c
> +++ b/sound/soc/qcom/lpass-sc7180.c
> @@ -163,14 +163,14 @@ static int sc7180_lpass_exit(struct platform_device *pdev)
>  	return 0;
>  }
>  
> -static int sc7180_lpass_dev_resume(struct device *dev)
> +static int __maybe_unused sc7180_lpass_dev_resume(struct device *dev)
>  {
>  	struct lpass_data *drvdata = dev_get_drvdata(dev);
>  
>  	return clk_bulk_prepare_enable(drvdata->num_clks, drvdata->clks);
>  }
>  
> -static int sc7180_lpass_dev_suspend(struct device *dev)
> +static int __maybe_unused sc7180_lpass_dev_suspend(struct device *dev)
>  {
>  	struct lpass_data *drvdata = dev_get_drvdata(dev);
>  
> -- 
> 2.7.4
> 
>
Srinivasa Rao Mandadapu Nov. 29, 2022, 3:58 p.m. UTC | #4
Thanks for your tie Nathan!!!

On 11/29/2022 9:15 PM, Nathan Chancellor wrote:
> On Tue, Nov 29, 2022 at 06:23:48PM +0530, Srinivasa Rao Mandadapu wrote:
>> Add __maybe_unused tag for system PM ops suspend and resume.
>> This is required to fix allmodconfig compilation issue.
>> Fixes: c3bf7699747c ("ASoC: qcom: lpass-sc7280: Add system suspend/resume PM ops")
>>
>> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
> A better solution would be replacing SET_SYSTEM_SLEEP_PM_OPS() with
> SYSTEM_SLEEP_PM_OPS(), which was added to avoid needing to add these
> '__maybe_unused' attributes to these functions. See commit 1a3c7bb08826
> ("PM: core: Add new *_PM_OPS macros, deprecate old ones") for more info.

Tried this option but as this patch required for Kernel 5.4 version code 
base,

SYSTEM_SLEEP_PM_OPS didn't work.

>> ---
>>   sound/soc/qcom/lpass-sc7180.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/sound/soc/qcom/lpass-sc7180.c b/sound/soc/qcom/lpass-sc7180.c
>> index b96b85a..41db661 100644
>> --- a/sound/soc/qcom/lpass-sc7180.c
>> +++ b/sound/soc/qcom/lpass-sc7180.c
>> @@ -163,14 +163,14 @@ static int sc7180_lpass_exit(struct platform_device *pdev)
>>   	return 0;
>>   }
>>   
>> -static int sc7180_lpass_dev_resume(struct device *dev)
>> +static int __maybe_unused sc7180_lpass_dev_resume(struct device *dev)
>>   {
>>   	struct lpass_data *drvdata = dev_get_drvdata(dev);
>>   
>>   	return clk_bulk_prepare_enable(drvdata->num_clks, drvdata->clks);
>>   }
>>   
>> -static int sc7180_lpass_dev_suspend(struct device *dev)
>> +static int __maybe_unused sc7180_lpass_dev_suspend(struct device *dev)
>>   {
>>   	struct lpass_data *drvdata = dev_get_drvdata(dev);
>>   
>> -- 
>> 2.7.4
>>
>>
Nathan Chancellor Nov. 29, 2022, 4:05 p.m. UTC | #5
On Tue, Nov 29, 2022 at 09:28:33PM +0530, Srinivasa Rao Mandadapu wrote:
> Thanks for your tie Nathan!!!
> 
> On 11/29/2022 9:15 PM, Nathan Chancellor wrote:
> > On Tue, Nov 29, 2022 at 06:23:48PM +0530, Srinivasa Rao Mandadapu wrote:
> > > Add __maybe_unused tag for system PM ops suspend and resume.
> > > This is required to fix allmodconfig compilation issue.
> > > Fixes: c3bf7699747c ("ASoC: qcom: lpass-sc7280: Add system suspend/resume PM ops")
> > > 
> > > Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
> > A better solution would be replacing SET_SYSTEM_SLEEP_PM_OPS() with
> > SYSTEM_SLEEP_PM_OPS(), which was added to avoid needing to add these
> > '__maybe_unused' attributes to these functions. See commit 1a3c7bb08826
> > ("PM: core: Add new *_PM_OPS macros, deprecate old ones") for more info.
> 
> Tried this option but as this patch required for Kernel 5.4 version code
> base,
> 
> SYSTEM_SLEEP_PM_OPS didn't work.

Ah right, it is a more recent macro. I did not realize this patch was
needed to fix a patch destined for stable.

Perhaps keep this patch but add a second patch after it that converts
to using SYSTEM_SLEEP_PM_OPS() for future releases?

> > > ---
> > >   sound/soc/qcom/lpass-sc7180.c | 4 ++--
> > >   1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/sound/soc/qcom/lpass-sc7180.c b/sound/soc/qcom/lpass-sc7180.c
> > > index b96b85a..41db661 100644
> > > --- a/sound/soc/qcom/lpass-sc7180.c
> > > +++ b/sound/soc/qcom/lpass-sc7180.c
> > > @@ -163,14 +163,14 @@ static int sc7180_lpass_exit(struct platform_device *pdev)
> > >   	return 0;
> > >   }
> > > -static int sc7180_lpass_dev_resume(struct device *dev)
> > > +static int __maybe_unused sc7180_lpass_dev_resume(struct device *dev)
> > >   {
> > >   	struct lpass_data *drvdata = dev_get_drvdata(dev);
> > >   	return clk_bulk_prepare_enable(drvdata->num_clks, drvdata->clks);
> > >   }
> > > -static int sc7180_lpass_dev_suspend(struct device *dev)
> > > +static int __maybe_unused sc7180_lpass_dev_suspend(struct device *dev)
> > >   {
> > >   	struct lpass_data *drvdata = dev_get_drvdata(dev);
> > > -- 
> > > 2.7.4
> > > 
> > >
Matthias Kaehlcke Nov. 29, 2022, 8:08 p.m. UTC | #6
On Tue, Nov 29, 2022 at 09:28:33PM +0530, Srinivasa Rao Mandadapu wrote:
> Thanks for your tie Nathan!!!
> 
> On 11/29/2022 9:15 PM, Nathan Chancellor wrote:
> > On Tue, Nov 29, 2022 at 06:23:48PM +0530, Srinivasa Rao Mandadapu wrote:
> > > Add __maybe_unused tag for system PM ops suspend and resume.
> > > This is required to fix allmodconfig compilation issue.
> > > Fixes: c3bf7699747c ("ASoC: qcom: lpass-sc7280: Add system suspend/resume PM ops")
> > > 
> > > Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
> > A better solution would be replacing SET_SYSTEM_SLEEP_PM_OPS() with
> > SYSTEM_SLEEP_PM_OPS(), which was added to avoid needing to add these
> > '__maybe_unused' attributes to these functions. See commit 1a3c7bb08826
> > ("PM: core: Add new *_PM_OPS macros, deprecate old ones") for more info.
> 
> Tried this option but as this patch required for Kernel 5.4 version code
> base,
> 
> SYSTEM_SLEEP_PM_OPS didn't work.

Older downstream trees needing a change shouldn't impact how a change is done
upstream. The change should be what's best for the upstream kernel. Downstream
kernels can do backports which might differ from the upstream solution or
pick the missing dependencies (which might not be too hard in this case).

> > > ---
> > >   sound/soc/qcom/lpass-sc7180.c | 4 ++--
> > >   1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/sound/soc/qcom/lpass-sc7180.c b/sound/soc/qcom/lpass-sc7180.c
> > > index b96b85a..41db661 100644
> > > --- a/sound/soc/qcom/lpass-sc7180.c
> > > +++ b/sound/soc/qcom/lpass-sc7180.c
> > > @@ -163,14 +163,14 @@ static int sc7180_lpass_exit(struct platform_device *pdev)
> > >   	return 0;
> > >   }
> > > -static int sc7180_lpass_dev_resume(struct device *dev)
> > > +static int __maybe_unused sc7180_lpass_dev_resume(struct device *dev)
> > >   {
> > >   	struct lpass_data *drvdata = dev_get_drvdata(dev);
> > >   	return clk_bulk_prepare_enable(drvdata->num_clks, drvdata->clks);
> > >   }
> > > -static int sc7180_lpass_dev_suspend(struct device *dev)
> > > +static int __maybe_unused sc7180_lpass_dev_suspend(struct device *dev)
> > >   {
> > >   	struct lpass_data *drvdata = dev_get_drvdata(dev);
> > > -- 
> > > 2.7.4
> > > 
> > >
Matthias Kaehlcke Nov. 30, 2022, 5:07 p.m. UTC | #7
On Tue, Nov 29, 2022 at 06:23:48PM +0530, Srinivasa Rao Mandadapu wrote:
> Add __maybe_unused tag for system PM ops suspend and resume.
> This is required to fix allmodconfig compilation issue.
> Fixes: c3bf7699747c ("ASoC: qcom: lpass-sc7280: Add system suspend/resume PM ops")

This is incorrect, it should be '2d68148f8f85 ("ASoC: qcom: lpass-sc7180: Add
system suspend/resume PM ops")'. I see you fixed that in v2, but this patch
has already been applied ...

Srinivasa, it seems a similar patch is needed for commit c3bf7699747c ("ASoC:
qcom: lpass-sc7280: Add system suspend/resume PM ops"). For that you could use
SYSTEM_SLEEP_PM_OPS() instead of the maybe_unused tags as suggested by Nathan.

Mark, I appreciate you being responsive and picking patches quickly, it might
help though to leave 'external' reviewers at least some time to provide their
feedback :)
Mark Brown Nov. 30, 2022, 5:16 p.m. UTC | #8
On Wed, Nov 30, 2022 at 05:07:46PM +0000, Matthias Kaehlcke wrote:
> On Tue, Nov 29, 2022 at 06:23:48PM +0530, Srinivasa Rao Mandadapu wrote:

> > Add __maybe_unused tag for system PM ops suspend and resume.
> > This is required to fix allmodconfig compilation issue.
> > Fixes: c3bf7699747c ("ASoC: qcom: lpass-sc7280: Add system suspend/resume PM ops")

> This is incorrect, it should be '2d68148f8f85 ("ASoC: qcom: lpass-sc7180: Add
> system suspend/resume PM ops")'. I see you fixed that in v2, but this patch
> has already been applied ...

with the fixes tag already updated as I was applying it.

> Mark, I appreciate you being responsive and picking patches quickly, it might
> help though to leave 'external' reviewers at least some time to provide their
> feedback :)

There's a balance with hanging on for utterly trivial patches to get
reviewed, especially in areas where reviews aren't relaible or
consistent.
Matthias Kaehlcke Nov. 30, 2022, 5:37 p.m. UTC | #9
On Wed, Nov 30, 2022 at 05:16:11PM +0000, Mark Brown wrote:
> On Wed, Nov 30, 2022 at 05:07:46PM +0000, Matthias Kaehlcke wrote:
> > On Tue, Nov 29, 2022 at 06:23:48PM +0530, Srinivasa Rao Mandadapu wrote:
> 
> > > Add __maybe_unused tag for system PM ops suspend and resume.
> > > This is required to fix allmodconfig compilation issue.
> > > Fixes: c3bf7699747c ("ASoC: qcom: lpass-sc7280: Add system suspend/resume PM ops")
> 
> > This is incorrect, it should be '2d68148f8f85 ("ASoC: qcom: lpass-sc7180: Add
> > system suspend/resume PM ops")'. I see you fixed that in v2, but this patch
> > has already been applied ...
> 
> with the fixes tag already updated as I was applying it.

The tag of the applied patch is incorrect too, which is actually what I noticed:

Fixes: a3a96e93cc88 ("ASoC: qcom: lpass-sc7280: Add system suspend/resume PM ops")

This patch is for sc7180, so it should be:

Fixes: 2d68148f8f85 ("ASoC: qcom: lpass-sc7180: Add system suspend/resume PM ops")

> > Mark, I appreciate you being responsive and picking patches quickly, it might
> > help though to leave 'external' reviewers at least some time to provide their
> > feedback :)
> 
> There's a balance with hanging on for utterly trivial patches to get
> reviewed, especially in areas where reviews aren't relaible or
> consistent.

I understand it's a balance and I certainly can't claim to be a regular reviewer
of audio patches.
Mark Brown Nov. 30, 2022, 7:12 p.m. UTC | #10
On Wed, Nov 30, 2022 at 05:37:50PM +0000, Matthias Kaehlcke wrote:

> The tag of the applied patch is incorrect too, which is actually what I noticed:
> 
> Fixes: a3a96e93cc88 ("ASoC: qcom: lpass-sc7280: Add system suspend/resume PM ops")
> 
> This patch is for sc7180, so it should be:
> 
> Fixes: 2d68148f8f85 ("ASoC: qcom: lpass-sc7180: Add system suspend/resume PM ops")

Oh, well.  It's just a fixes tag for something that has only been in
-next, it doesn't really matter that much.
Srinivasa Rao Mandadapu Dec. 1, 2022, 3:31 a.m. UTC | #11
On 12/1/2022 12:42 AM, Mark Brown wrote:
> On Wed, Nov 30, 2022 at 05:37:50PM +0000, Matthias Kaehlcke wrote:
>
>> The tag of the applied patch is incorrect too, which is actually what I noticed:
>>
>> Fixes: a3a96e93cc88 ("ASoC: qcom: lpass-sc7280: Add system suspend/resume PM ops")
>>
>> This patch is for sc7180, so it should be:
>>
>> Fixes: 2d68148f8f85 ("ASoC: qcom: lpass-sc7180: Add system suspend/resume PM ops")
Actually with the fix already posted v2.
> Oh, well.  It's just a fixes tag for something that has only been in
> -next, it doesn't really matter that much.
Okay. Sorry for Inconvenience.
Srinivasa Rao Mandadapu Dec. 1, 2022, 7:02 a.m. UTC | #12
Thanks for your time Matthias,

On 11/30/2022 1:38 AM, Matthias Kaehlcke wrote:
> On Tue, Nov 29, 2022 at 09:28:33PM +0530, Srinivasa Rao Mandadapu wrote:
>> Thanks for your tie Nathan!!!
>>
>> On 11/29/2022 9:15 PM, Nathan Chancellor wrote:
>>> On Tue, Nov 29, 2022 at 06:23:48PM +0530, Srinivasa Rao Mandadapu wrote:
>>>> Add __maybe_unused tag for system PM ops suspend and resume.
>>>> This is required to fix allmodconfig compilation issue.
>>>> Fixes: c3bf7699747c ("ASoC: qcom: lpass-sc7280: Add system suspend/resume PM ops")
>>>>
>>>> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
>>> A better solution would be replacing SET_SYSTEM_SLEEP_PM_OPS() with
>>> SYSTEM_SLEEP_PM_OPS(), which was added to avoid needing to add these
>>> '__maybe_unused' attributes to these functions. See commit 1a3c7bb08826
>>> ("PM: core: Add new *_PM_OPS macros, deprecate old ones") for more info.
>> Tried this option but as this patch required for Kernel 5.4 version code
>> base,
>>
>> SYSTEM_SLEEP_PM_OPS didn't work.
> Older downstream trees needing a change shouldn't impact how a change is done
> upstream. The change should be what's best for the upstream kernel. Downstream
> kernels can do backports which might differ from the upstream solution or
> pick the missing dependencies (which might not be too hard in this case).
Okay. Will take care next time.
>
>>>> ---
>>>>    sound/soc/qcom/lpass-sc7180.c | 4 ++--
>>>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/sound/soc/qcom/lpass-sc7180.c b/sound/soc/qcom/lpass-sc7180.c
>>>> index b96b85a..41db661 100644
>>>> --- a/sound/soc/qcom/lpass-sc7180.c
>>>> +++ b/sound/soc/qcom/lpass-sc7180.c
>>>> @@ -163,14 +163,14 @@ static int sc7180_lpass_exit(struct platform_device *pdev)
>>>>    	return 0;
>>>>    }
>>>> -static int sc7180_lpass_dev_resume(struct device *dev)
>>>> +static int __maybe_unused sc7180_lpass_dev_resume(struct device *dev)
>>>>    {
>>>>    	struct lpass_data *drvdata = dev_get_drvdata(dev);
>>>>    	return clk_bulk_prepare_enable(drvdata->num_clks, drvdata->clks);
>>>>    }
>>>> -static int sc7180_lpass_dev_suspend(struct device *dev)
>>>> +static int __maybe_unused sc7180_lpass_dev_suspend(struct device *dev)
>>>>    {
>>>>    	struct lpass_data *drvdata = dev_get_drvdata(dev);
>>>> -- 
>>>> 2.7.4
>>>>
>>>>
diff mbox series

Patch

diff --git a/sound/soc/qcom/lpass-sc7180.c b/sound/soc/qcom/lpass-sc7180.c
index b96b85a..41db661 100644
--- a/sound/soc/qcom/lpass-sc7180.c
+++ b/sound/soc/qcom/lpass-sc7180.c
@@ -163,14 +163,14 @@  static int sc7180_lpass_exit(struct platform_device *pdev)
 	return 0;
 }
 
-static int sc7180_lpass_dev_resume(struct device *dev)
+static int __maybe_unused sc7180_lpass_dev_resume(struct device *dev)
 {
 	struct lpass_data *drvdata = dev_get_drvdata(dev);
 
 	return clk_bulk_prepare_enable(drvdata->num_clks, drvdata->clks);
 }
 
-static int sc7180_lpass_dev_suspend(struct device *dev)
+static int __maybe_unused sc7180_lpass_dev_suspend(struct device *dev)
 {
 	struct lpass_data *drvdata = dev_get_drvdata(dev);