Re: [PATCH -next] platform: surface: fix non-PM_SLEEP build warnings
On Tue, Dec 15, 2020 at 1:49 AM Randy Dunlap wrote: > On 12/14/20 3:19 AM, Andy Shevchenko wrote: > > On Mon, Dec 14, 2020 at 2:53 AM Randy Dunlap wrote: > >> On 12/12/20 11:07 AM, Andy Shevchenko wrote: > >>> On Sat, Dec 12, 2020 at 7:05 PM Randy Dunlap > >>> wrote: > > > > ... > > > >>> Here [1] is the rationale behind annotation vs. ifdeffery. > >>> > >>> [1]: https://lore.kernel.org/patchwork/patch/732981/ > >>> > >> Thanks for the link. I'll send a v2. > >> > >> Could we add that to the Linux BKP (Best Known Practices) > >> document? > > > > Perhaps. The author of that is Arnd, maybe he has something to add. > > > > Where is it located? My search foo could not find it. Closest what I know is [2]. [2]: https://kernelnewbies.org/FAQ/CodingStyle -- With Best Regards, Andy Shevchenko
Re: [PATCH -next] platform: surface: fix non-PM_SLEEP build warnings
On 12/14/20 3:19 AM, Andy Shevchenko wrote: > On Mon, Dec 14, 2020 at 2:53 AM Randy Dunlap wrote: >> On 12/12/20 11:07 AM, Andy Shevchenko wrote: >>> On Sat, Dec 12, 2020 at 7:05 PM Randy Dunlap wrote: > > ... > >>> Here [1] is the rationale behind annotation vs. ifdeffery. >>> >>> [1]: https://lore.kernel.org/patchwork/patch/732981/ >>> >> Thanks for the link. I'll send a v2. >> >> Could we add that to the Linux BKP (Best Known Practices) >> document? > > Perhaps. The author of that is Arnd, maybe he has something to add. > Where is it located? My search foo could not find it. thanks. -- ~Randy
Re: [PATCH -next] platform: surface: fix non-PM_SLEEP build warnings
On Mon, Dec 14, 2020 at 2:53 AM Randy Dunlap wrote: > On 12/12/20 11:07 AM, Andy Shevchenko wrote: > > On Sat, Dec 12, 2020 at 7:05 PM Randy Dunlap wrote: ... > > Here [1] is the rationale behind annotation vs. ifdeffery. > > > > [1]: https://lore.kernel.org/patchwork/patch/732981/ > > > Thanks for the link. I'll send a v2. > > Could we add that to the Linux BKP (Best Known Practices) > document? Perhaps. The author of that is Arnd, maybe he has something to add. -- With Best Regards, Andy Shevchenko
Re: [PATCH -next] platform: surface: fix non-PM_SLEEP build warnings
On 12/12/20 11:07 AM, Andy Shevchenko wrote: > On Sat, Dec 12, 2020 at 7:05 PM Randy Dunlap wrote: >> On 12/12/20 5:24 AM, Andy Shevchenko wrote: >>> On Fri, Dec 11, 2020 at 9:20 PM Randy Dunlap wrote: > > ... > +#ifdef CONFIG_PM_SLEEP static int surface_gpe_suspend(struct device *dev) >>> >>> Perhaps __maybe_unused ? >> >> Yes, I am aware of that option. >> I don't know why it would be preferred though. > > Here [1] is the rationale behind annotation vs. ifdeffery. > > [1]: https://lore.kernel.org/patchwork/patch/732981/ > Thanks for the link. I'll send a v2. Could we add that to the Linux BKP (Best Known Practices) document? -- ~Randy
Re: [PATCH -next] platform: surface: fix non-PM_SLEEP build warnings
On Sat, Dec 12, 2020 at 7:05 PM Randy Dunlap wrote: > On 12/12/20 5:24 AM, Andy Shevchenko wrote: > > On Fri, Dec 11, 2020 at 9:20 PM Randy Dunlap wrote: ... > >> +#ifdef CONFIG_PM_SLEEP > >> static int surface_gpe_suspend(struct device *dev) > > > > Perhaps __maybe_unused ? > > Yes, I am aware of that option. > I don't know why it would be preferred though. Here [1] is the rationale behind annotation vs. ifdeffery. [1]: https://lore.kernel.org/patchwork/patch/732981/ -- With Best Regards, Andy Shevchenko
Re: [PATCH -next] platform: surface: fix non-PM_SLEEP build warnings
On 12/12/20 5:24 AM, Andy Shevchenko wrote: > On Fri, Dec 11, 2020 at 9:20 PM Randy Dunlap wrote: >> >> Fix build warnings when CONFIG_PM_SLEEP is not enabled and these >> functions are not used: >> >> ../drivers/platform/surface/surface_gpe.c:189:12: warning: >> ‘surface_gpe_resume’ defined but not used [-Wunused-function] >> static int surface_gpe_resume(struct device *dev) >> ^~ >> ../drivers/platform/surface/surface_gpe.c:184:12: warning: >> ‘surface_gpe_suspend’ defined but not used [-Wunused-function] >> static int surface_gpe_suspend(struct device *dev) >> ^~~ > > ... > >> +#ifdef CONFIG_PM_SLEEP >> static int surface_gpe_suspend(struct device *dev) > > Perhaps __maybe_unused ? > Yes, I am aware of that option. I don't know why it would be preferred though. -- ~Randy
Re: [PATCH -next] platform: surface: fix non-PM_SLEEP build warnings
On Fri, Dec 11, 2020 at 9:20 PM Randy Dunlap wrote: > > Fix build warnings when CONFIG_PM_SLEEP is not enabled and these > functions are not used: > > ../drivers/platform/surface/surface_gpe.c:189:12: warning: > ‘surface_gpe_resume’ defined but not used [-Wunused-function] > static int surface_gpe_resume(struct device *dev) > ^~ > ../drivers/platform/surface/surface_gpe.c:184:12: warning: > ‘surface_gpe_suspend’ defined but not used [-Wunused-function] > static int surface_gpe_suspend(struct device *dev) > ^~~ ... > +#ifdef CONFIG_PM_SLEEP > static int surface_gpe_suspend(struct device *dev) Perhaps __maybe_unused ? -- With Best Regards, Andy Shevchenko
Re: [PATCH -next] platform: surface: fix non-PM_SLEEP build warnings
On 12/11/20 9:41 PM, Randy Dunlap wrote:
On 12/11/20 12:23 PM, Maximilian Luz wrote:
On 12/11/20 8:03 PM, Randy Dunlap wrote:
Fix build warnings when CONFIG_PM_SLEEP is not enabled and these
functions are not used:
../drivers/platform/surface/surface_gpe.c:189:12: warning: ‘surface_gpe_resume’
defined but not used [-Wunused-function]
static int surface_gpe_resume(struct device *dev)
^~
../drivers/platform/surface/surface_gpe.c:184:12: warning:
‘surface_gpe_suspend’ defined but not used [-Wunused-function]
static int surface_gpe_suspend(struct device *dev)
^~~
Fixes: 274335f1c557 ("platform/surface: Add Driver to set up lid GPEs on MS Surface
device")
Signed-off-by: Randy Dunlap
Cc: Maximilian Luz
Cc: Hans de Goede
Cc: platform-driver-...@vger.kernel.org
---
drivers/platform/surface/surface_gpe.c | 2 ++
1 file changed, 2 insertions(+)
--- linux-next-20201210.orig/drivers/platform/surface/surface_gpe.c
+++ linux-next-20201210/drivers/platform/surface/surface_gpe.c
@@ -181,6 +181,7 @@ static int surface_lid_enable_wakeup(str
return 0;
}
+#ifdef CONFIG_PM_SLEEP
static int surface_gpe_suspend(struct device *dev)
{
return surface_lid_enable_wakeup(dev, true);
@@ -190,6 +191,7 @@ static int surface_gpe_resume(struct dev
{
return surface_lid_enable_wakeup(dev, false);
}
+#endif
static SIMPLE_DEV_PM_OPS(surface_gpe_pm, surface_gpe_suspend,
surface_gpe_resume);
Right, thanks.
I assume this covers all instances of this warning in platform/surface?
Otherwise, a "platform: surface: gpe: ..." subject would make more sense.
It should cover all of surface/. It was an allmodconfig and then I disabled
CONFIG_PM and CONFIG_PM_SLEEP etc.
Perfect, thanks!
As for prefixes, how many levels do we want to use?
(that's mostly rhetorical, although I would read answers :)
Looking at platform/x86 and past commit messages, I'd prefer something
like
platform/surface: :
This would be similar to the platform/x86 style. So two or three,
depending on how you count "platform/surface". I agree that this
probably tends to get a bit long, so I propose we drop the surface_
prefix on the component part to help with that. So, for example,
"surface_gpe" will become "gpe".
As for the rest:
Reviewed-by: Maximilian Luz
thanks.
Regards,
Max
Re: [PATCH -next] platform: surface: fix non-PM_SLEEP build warnings
On 12/11/20 12:23 PM, Maximilian Luz wrote:
> On 12/11/20 8:03 PM, Randy Dunlap wrote:
>> Fix build warnings when CONFIG_PM_SLEEP is not enabled and these
>> functions are not used:
>>
>> ../drivers/platform/surface/surface_gpe.c:189:12: warning:
>> ‘surface_gpe_resume’ defined but not used [-Wunused-function]
>> static int surface_gpe_resume(struct device *dev)
>> ^~
>> ../drivers/platform/surface/surface_gpe.c:184:12: warning:
>> ‘surface_gpe_suspend’ defined but not used [-Wunused-function]
>> static int surface_gpe_suspend(struct device *dev)
>> ^~~
>>
>> Fixes: 274335f1c557 ("platform/surface: Add Driver to set up lid GPEs on MS
>> Surface device")
>> Signed-off-by: Randy Dunlap
>> Cc: Maximilian Luz
>> Cc: Hans de Goede
>> Cc: platform-driver-...@vger.kernel.org
>> ---
>> drivers/platform/surface/surface_gpe.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> --- linux-next-20201210.orig/drivers/platform/surface/surface_gpe.c
>> +++ linux-next-20201210/drivers/platform/surface/surface_gpe.c
>> @@ -181,6 +181,7 @@ static int surface_lid_enable_wakeup(str
>> return 0;
>> }
>> +#ifdef CONFIG_PM_SLEEP
>> static int surface_gpe_suspend(struct device *dev)
>> {
>> return surface_lid_enable_wakeup(dev, true);
>> @@ -190,6 +191,7 @@ static int surface_gpe_resume(struct dev
>> {
>> return surface_lid_enable_wakeup(dev, false);
>> }
>> +#endif
>> static SIMPLE_DEV_PM_OPS(surface_gpe_pm, surface_gpe_suspend,
>> surface_gpe_resume);
>>
>
> Right, thanks.
>
> I assume this covers all instances of this warning in platform/surface?
> Otherwise, a "platform: surface: gpe: ..." subject would make more sense.
It should cover all of surface/. It was an allmodconfig and then I disabled
CONFIG_PM and CONFIG_PM_SLEEP etc.
As for prefixes, how many levels do we want to use?
(that's mostly rhetorical, although I would read answers :)
> As for the rest:
>
> Reviewed-by: Maximilian Luz
thanks.
--
~Randy
Re: [PATCH -next] platform: surface: fix non-PM_SLEEP build warnings
On 12/11/20 8:03 PM, Randy Dunlap wrote:
Fix build warnings when CONFIG_PM_SLEEP is not enabled and these
functions are not used:
../drivers/platform/surface/surface_gpe.c:189:12: warning: ‘surface_gpe_resume’
defined but not used [-Wunused-function]
static int surface_gpe_resume(struct device *dev)
^~
../drivers/platform/surface/surface_gpe.c:184:12: warning:
‘surface_gpe_suspend’ defined but not used [-Wunused-function]
static int surface_gpe_suspend(struct device *dev)
^~~
Fixes: 274335f1c557 ("platform/surface: Add Driver to set up lid GPEs on MS Surface
device")
Signed-off-by: Randy Dunlap
Cc: Maximilian Luz
Cc: Hans de Goede
Cc: platform-driver-...@vger.kernel.org
---
drivers/platform/surface/surface_gpe.c |2 ++
1 file changed, 2 insertions(+)
--- linux-next-20201210.orig/drivers/platform/surface/surface_gpe.c
+++ linux-next-20201210/drivers/platform/surface/surface_gpe.c
@@ -181,6 +181,7 @@ static int surface_lid_enable_wakeup(str
return 0;
}
+#ifdef CONFIG_PM_SLEEP
static int surface_gpe_suspend(struct device *dev)
{
return surface_lid_enable_wakeup(dev, true);
@@ -190,6 +191,7 @@ static int surface_gpe_resume(struct dev
{
return surface_lid_enable_wakeup(dev, false);
}
+#endif
static SIMPLE_DEV_PM_OPS(surface_gpe_pm, surface_gpe_suspend, surface_gpe_resume);
Right, thanks.
I assume this covers all instances of this warning in platform/surface?
Otherwise, a "platform: surface: gpe: ..." subject would make more sense.
As for the rest:
Reviewed-by: Maximilian Luz
Regards,
Max
[PATCH -next] platform: surface: fix non-PM_SLEEP build warnings
Fix build warnings when CONFIG_PM_SLEEP is not enabled and these
functions are not used:
../drivers/platform/surface/surface_gpe.c:189:12: warning: ‘surface_gpe_resume’
defined but not used [-Wunused-function]
static int surface_gpe_resume(struct device *dev)
^~
../drivers/platform/surface/surface_gpe.c:184:12: warning:
‘surface_gpe_suspend’ defined but not used [-Wunused-function]
static int surface_gpe_suspend(struct device *dev)
^~~
Fixes: 274335f1c557 ("platform/surface: Add Driver to set up lid GPEs on MS
Surface device")
Signed-off-by: Randy Dunlap
Cc: Maximilian Luz
Cc: Hans de Goede
Cc: platform-driver-...@vger.kernel.org
---
drivers/platform/surface/surface_gpe.c |2 ++
1 file changed, 2 insertions(+)
--- linux-next-20201210.orig/drivers/platform/surface/surface_gpe.c
+++ linux-next-20201210/drivers/platform/surface/surface_gpe.c
@@ -181,6 +181,7 @@ static int surface_lid_enable_wakeup(str
return 0;
}
+#ifdef CONFIG_PM_SLEEP
static int surface_gpe_suspend(struct device *dev)
{
return surface_lid_enable_wakeup(dev, true);
@@ -190,6 +191,7 @@ static int surface_gpe_resume(struct dev
{
return surface_lid_enable_wakeup(dev, false);
}
+#endif
static SIMPLE_DEV_PM_OPS(surface_gpe_pm, surface_gpe_suspend,
surface_gpe_resume);
