linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] phy: zynqmp: Fix unused-function compiler warning
@ 2020-07-01  9:04 Tobias Klauser
  2020-07-01 13:19 ` Laurent Pinchart
  2020-07-01 14:10 ` [PATCH v2] " Tobias Klauser
  0 siblings, 2 replies; 11+ messages in thread
From: Tobias Klauser @ 2020-07-01  9:04 UTC (permalink / raw)
  To: Anurag Kumar Vulisha, Laurent Pinchart
  Cc: Kishon Vijay Abraham I, Vinod Koul, Michal Simek, linux-kernel,
	linux-arm-kernel

This fixes the following compiler warning when building with
CONFIG_PM && !CONFIG_PM_SLEEP:

drivers/phy/xilinx/phy-zynqmp.c:830:12: warning: ‘xpsgtr_resume’ defined but not used [-Wunused-function]
  830 | static int xpsgtr_resume(struct device *dev)
      |            ^~~~~~~~~~~~~
drivers/phy/xilinx/phy-zynqmp.c:819:12: warning: ‘xpsgtr_suspend’ defined but not used [-Wunused-function]
  819 | static int xpsgtr_suspend(struct device *dev)
      |            ^~~~~~~~~~~~~~

Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
---
 drivers/phy/xilinx/phy-zynqmp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/xilinx/phy-zynqmp.c b/drivers/phy/xilinx/phy-zynqmp.c
index 8babee2ce9ec..22a0ae635797 100644
--- a/drivers/phy/xilinx/phy-zynqmp.c
+++ b/drivers/phy/xilinx/phy-zynqmp.c
@@ -815,7 +815,7 @@ static struct phy *xpsgtr_xlate(struct device *dev,
  * Power Management
  */
 
-#ifdef CONFIG_PM
+#ifdef CONFIG_PM_SLEEP
 static int xpsgtr_suspend(struct device *dev)
 {
 	struct xpsgtr_dev *gtr_dev = dev_get_drvdata(dev);
@@ -854,7 +854,7 @@ static int xpsgtr_resume(struct device *dev)
 
 	return 0;
 }
-#endif /* CONFIG_PM */
+#endif /* CONFIG_PM_SLEEP */
 
 static const struct dev_pm_ops xpsgtr_pm_ops = {
 	SET_SYSTEM_SLEEP_PM_OPS(xpsgtr_suspend, xpsgtr_resume)
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] phy: zynqmp: Fix unused-function compiler warning
  2020-07-01  9:04 [PATCH] phy: zynqmp: Fix unused-function compiler warning Tobias Klauser
@ 2020-07-01 13:19 ` Laurent Pinchart
  2020-07-01 13:36   ` Vinod Koul
  2020-07-01 14:10 ` [PATCH v2] " Tobias Klauser
  1 sibling, 1 reply; 11+ messages in thread
From: Laurent Pinchart @ 2020-07-01 13:19 UTC (permalink / raw)
  To: Tobias Klauser
  Cc: Anurag Kumar Vulisha, Kishon Vijay Abraham I, Vinod Koul,
	Michal Simek, linux-kernel, linux-arm-kernel

Hi Tobias,

Thank you for the patch.

On Wed, Jul 01, 2020 at 11:04:38AM +0200, Tobias Klauser wrote:
> This fixes the following compiler warning when building with
> CONFIG_PM && !CONFIG_PM_SLEEP:
> 
> drivers/phy/xilinx/phy-zynqmp.c:830:12: warning: ‘xpsgtr_resume’ defined but not used [-Wunused-function]
>   830 | static int xpsgtr_resume(struct device *dev)
>       |            ^~~~~~~~~~~~~
> drivers/phy/xilinx/phy-zynqmp.c:819:12: warning: ‘xpsgtr_suspend’ defined but not used [-Wunused-function]
>   819 | static int xpsgtr_suspend(struct device *dev)
>       |            ^~~~~~~~~~~~~~

Oops :-S Sorry about that.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Vinod or Kishon, can you pick this patch up, or do I need to send a pull
request ? (It's my first driver in the PHY subsystem so I don't know
what the usual practices are there)

> Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
> ---
>  drivers/phy/xilinx/phy-zynqmp.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/phy/xilinx/phy-zynqmp.c b/drivers/phy/xilinx/phy-zynqmp.c
> index 8babee2ce9ec..22a0ae635797 100644
> --- a/drivers/phy/xilinx/phy-zynqmp.c
> +++ b/drivers/phy/xilinx/phy-zynqmp.c
> @@ -815,7 +815,7 @@ static struct phy *xpsgtr_xlate(struct device *dev,
>   * Power Management
>   */
>  
> -#ifdef CONFIG_PM
> +#ifdef CONFIG_PM_SLEEP
>  static int xpsgtr_suspend(struct device *dev)
>  {
>  	struct xpsgtr_dev *gtr_dev = dev_get_drvdata(dev);
> @@ -854,7 +854,7 @@ static int xpsgtr_resume(struct device *dev)
>  
>  	return 0;
>  }
> -#endif /* CONFIG_PM */
> +#endif /* CONFIG_PM_SLEEP */
>  
>  static const struct dev_pm_ops xpsgtr_pm_ops = {
>  	SET_SYSTEM_SLEEP_PM_OPS(xpsgtr_suspend, xpsgtr_resume)

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] phy: zynqmp: Fix unused-function compiler warning
  2020-07-01 13:19 ` Laurent Pinchart
@ 2020-07-01 13:36   ` Vinod Koul
  2020-07-01 13:44     ` Laurent Pinchart
  0 siblings, 1 reply; 11+ messages in thread
From: Vinod Koul @ 2020-07-01 13:36 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Tobias Klauser, Anurag Kumar Vulisha, Kishon Vijay Abraham I,
	Michal Simek, linux-kernel, linux-arm-kernel

On 01-07-20, 16:19, Laurent Pinchart wrote:
> Hi Tobias,
> 
> Thank you for the patch.
> 
> On Wed, Jul 01, 2020 at 11:04:38AM +0200, Tobias Klauser wrote:
> > This fixes the following compiler warning when building with
> > CONFIG_PM && !CONFIG_PM_SLEEP:
> > 
> > drivers/phy/xilinx/phy-zynqmp.c:830:12: warning: ‘xpsgtr_resume’ defined but not used [-Wunused-function]
> >   830 | static int xpsgtr_resume(struct device *dev)
> >       |            ^~~~~~~~~~~~~
> > drivers/phy/xilinx/phy-zynqmp.c:819:12: warning: ‘xpsgtr_suspend’ defined but not used [-Wunused-function]
> >   819 | static int xpsgtr_suspend(struct device *dev)
> >       |            ^~~~~~~~~~~~~~
> 
> Oops :-S Sorry about that.
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Vinod or Kishon, can you pick this patch up, or do I need to send a pull
> request ? (It's my first driver in the PHY subsystem so I don't know
> what the usual practices are there)

patches are welcome :-)

> 
> > Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
> > ---
> >  drivers/phy/xilinx/phy-zynqmp.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/phy/xilinx/phy-zynqmp.c b/drivers/phy/xilinx/phy-zynqmp.c
> > index 8babee2ce9ec..22a0ae635797 100644
> > --- a/drivers/phy/xilinx/phy-zynqmp.c
> > +++ b/drivers/phy/xilinx/phy-zynqmp.c
> > @@ -815,7 +815,7 @@ static struct phy *xpsgtr_xlate(struct device *dev,
> >   * Power Management
> >   */
> >  
> > -#ifdef CONFIG_PM
> > +#ifdef CONFIG_PM_SLEEP

How about marking it as __maybe_unused instead?

-- 
~Vinod

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] phy: zynqmp: Fix unused-function compiler warning
  2020-07-01 13:36   ` Vinod Koul
@ 2020-07-01 13:44     ` Laurent Pinchart
  2020-07-01 13:55       ` Vinod Koul
  2020-07-01 14:00       ` Tobias Klauser
  0 siblings, 2 replies; 11+ messages in thread
From: Laurent Pinchart @ 2020-07-01 13:44 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Tobias Klauser, Anurag Kumar Vulisha, Kishon Vijay Abraham I,
	Michal Simek, linux-kernel, linux-arm-kernel

Hi Vinod,

On Wed, Jul 01, 2020 at 07:06:43PM +0530, Vinod Koul wrote:
> On 01-07-20, 16:19, Laurent Pinchart wrote:
> > On Wed, Jul 01, 2020 at 11:04:38AM +0200, Tobias Klauser wrote:
> > > This fixes the following compiler warning when building with
> > > CONFIG_PM && !CONFIG_PM_SLEEP:
> > > 
> > > drivers/phy/xilinx/phy-zynqmp.c:830:12: warning: ‘xpsgtr_resume’ defined but not used [-Wunused-function]
> > >   830 | static int xpsgtr_resume(struct device *dev)
> > >       |            ^~~~~~~~~~~~~
> > > drivers/phy/xilinx/phy-zynqmp.c:819:12: warning: ‘xpsgtr_suspend’ defined but not used [-Wunused-function]
> > >   819 | static int xpsgtr_suspend(struct device *dev)
> > >       |            ^~~~~~~~~~~~~~
> > 
> > Oops :-S Sorry about that.
> > 
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > 
> > Vinod or Kishon, can you pick this patch up, or do I need to send a pull
> > request ? (It's my first driver in the PHY subsystem so I don't know
> > what the usual practices are there)
> 
> patches are welcome :-)
> 
> > 
> > > Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
> > > ---
> > >  drivers/phy/xilinx/phy-zynqmp.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/phy/xilinx/phy-zynqmp.c b/drivers/phy/xilinx/phy-zynqmp.c
> > > index 8babee2ce9ec..22a0ae635797 100644
> > > --- a/drivers/phy/xilinx/phy-zynqmp.c
> > > +++ b/drivers/phy/xilinx/phy-zynqmp.c
> > > @@ -815,7 +815,7 @@ static struct phy *xpsgtr_xlate(struct device *dev,
> > >   * Power Management
> > >   */
> > >  
> > > -#ifdef CONFIG_PM
> > > +#ifdef CONFIG_PM_SLEEP
> 
> How about marking it as __maybe_unused instead?

I don't mind either, I'll let Tobias decide, but his patch seems fine,
is there a drawback in his approach ? If it's just a matter of personal
preference, I'd rather not require a v2.

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] phy: zynqmp: Fix unused-function compiler warning
  2020-07-01 13:44     ` Laurent Pinchart
@ 2020-07-01 13:55       ` Vinod Koul
  2020-07-01 14:00       ` Tobias Klauser
  1 sibling, 0 replies; 11+ messages in thread
From: Vinod Koul @ 2020-07-01 13:55 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Tobias Klauser, Anurag Kumar Vulisha, Kishon Vijay Abraham I,
	Michal Simek, linux-kernel, linux-arm-kernel

Hi Laurent,

On 01-07-20, 16:44, Laurent Pinchart wrote:

> > How about marking it as __maybe_unused instead?
> 
> I don't mind either, I'll let Tobias decide, but his patch seems fine,
> is there a drawback in his approach ? If it's just a matter of personal
> preference, I'd rather not require a v2.

__maybe_unused is the preferred way to do as Documented in
Documentation/process/coding-style.rst and makes code look neater..

Thanks
-- 
~Vinod

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] phy: zynqmp: Fix unused-function compiler warning
  2020-07-01 13:44     ` Laurent Pinchart
  2020-07-01 13:55       ` Vinod Koul
@ 2020-07-01 14:00       ` Tobias Klauser
  2020-07-01 14:01         ` Laurent Pinchart
  1 sibling, 1 reply; 11+ messages in thread
From: Tobias Klauser @ 2020-07-01 14:00 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Vinod Koul, Anurag Kumar Vulisha, Kishon Vijay Abraham I,
	Michal Simek, linux-kernel, linux-arm-kernel

On 2020-07-01 at 15:44:43 +0200, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:
> On Wed, Jul 01, 2020 at 07:06:43PM +0530, Vinod Koul wrote:
> > On 01-07-20, 16:19, Laurent Pinchart wrote:
> > > On Wed, Jul 01, 2020 at 11:04:38AM +0200, Tobias Klauser wrote:
> > > > This fixes the following compiler warning when building with
> > > > CONFIG_PM && !CONFIG_PM_SLEEP:
> > > > 
> > > > drivers/phy/xilinx/phy-zynqmp.c:830:12: warning: ‘xpsgtr_resume’ defined but not used [-Wunused-function]
> > > >   830 | static int xpsgtr_resume(struct device *dev)
> > > >       |            ^~~~~~~~~~~~~
> > > > drivers/phy/xilinx/phy-zynqmp.c:819:12: warning: ‘xpsgtr_suspend’ defined but not used [-Wunused-function]
> > > >   819 | static int xpsgtr_suspend(struct device *dev)
> > > >       |            ^~~~~~~~~~~~~~
> > > 
> > > Oops :-S Sorry about that.
> > > 
> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Thanks for the review Laurent.

> > > Vinod or Kishon, can you pick this patch up, or do I need to send a pull
> > > request ? (It's my first driver in the PHY subsystem so I don't know
> > > what the usual practices are there)
> > 
> > patches are welcome :-)
> > 
> > > 
> > > > Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
> > > > ---
> > > >  drivers/phy/xilinx/phy-zynqmp.c | 4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/phy/xilinx/phy-zynqmp.c b/drivers/phy/xilinx/phy-zynqmp.c
> > > > index 8babee2ce9ec..22a0ae635797 100644
> > > > --- a/drivers/phy/xilinx/phy-zynqmp.c
> > > > +++ b/drivers/phy/xilinx/phy-zynqmp.c
> > > > @@ -815,7 +815,7 @@ static struct phy *xpsgtr_xlate(struct device *dev,
> > > >   * Power Management
> > > >   */
> > > >  
> > > > -#ifdef CONFIG_PM
> > > > +#ifdef CONFIG_PM_SLEEP
> > 
> > How about marking it as __maybe_unused instead?
> 
> I don't mind either, I'll let Tobias decide, but his patch seems fine,
> is there a drawback in his approach ? If it's just a matter of personal
> preference, I'd rather not require a v2.

I don't mind either, it was just what seemed the more straight-forward
fix. On the other hand, it seems that marking these functions as
__maybe_unused is the more widely used method in other PHY drivers. In
addition it would have the nice side-effect of the code always being
compile-checked regardless of the value of CONFIG_PM_SLEEP.

Will send a v2 using __maybe_unused and will let you decide which one to
pick :)

Thanks

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] phy: zynqmp: Fix unused-function compiler warning
  2020-07-01 14:00       ` Tobias Klauser
@ 2020-07-01 14:01         ` Laurent Pinchart
  2020-07-01 14:07           ` Tobias Klauser
  0 siblings, 1 reply; 11+ messages in thread
From: Laurent Pinchart @ 2020-07-01 14:01 UTC (permalink / raw)
  To: Tobias Klauser
  Cc: Vinod Koul, Anurag Kumar Vulisha, Kishon Vijay Abraham I,
	Michal Simek, linux-kernel, linux-arm-kernel

On Wed, Jul 01, 2020 at 04:00:26PM +0200, Tobias Klauser wrote:
> On 2020-07-01 at 15:44:43 +0200, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:
> > On Wed, Jul 01, 2020 at 07:06:43PM +0530, Vinod Koul wrote:
> > > On 01-07-20, 16:19, Laurent Pinchart wrote:
> > > > On Wed, Jul 01, 2020 at 11:04:38AM +0200, Tobias Klauser wrote:
> > > > > This fixes the following compiler warning when building with
> > > > > CONFIG_PM && !CONFIG_PM_SLEEP:
> > > > > 
> > > > > drivers/phy/xilinx/phy-zynqmp.c:830:12: warning: ‘xpsgtr_resume’ defined but not used [-Wunused-function]
> > > > >   830 | static int xpsgtr_resume(struct device *dev)
> > > > >       |            ^~~~~~~~~~~~~
> > > > > drivers/phy/xilinx/phy-zynqmp.c:819:12: warning: ‘xpsgtr_suspend’ defined but not used [-Wunused-function]
> > > > >   819 | static int xpsgtr_suspend(struct device *dev)
> > > > >       |            ^~~~~~~~~~~~~~
> > > > 
> > > > Oops :-S Sorry about that.
> > > > 
> > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Thanks for the review Laurent.
> 
> > > > Vinod or Kishon, can you pick this patch up, or do I need to send a pull
> > > > request ? (It's my first driver in the PHY subsystem so I don't know
> > > > what the usual practices are there)
> > > 
> > > patches are welcome :-)
> > > 
> > > > 
> > > > > Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
> > > > > ---
> > > > >  drivers/phy/xilinx/phy-zynqmp.c | 4 ++--
> > > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/phy/xilinx/phy-zynqmp.c b/drivers/phy/xilinx/phy-zynqmp.c
> > > > > index 8babee2ce9ec..22a0ae635797 100644
> > > > > --- a/drivers/phy/xilinx/phy-zynqmp.c
> > > > > +++ b/drivers/phy/xilinx/phy-zynqmp.c
> > > > > @@ -815,7 +815,7 @@ static struct phy *xpsgtr_xlate(struct device *dev,
> > > > >   * Power Management
> > > > >   */
> > > > >  
> > > > > -#ifdef CONFIG_PM
> > > > > +#ifdef CONFIG_PM_SLEEP
> > > 
> > > How about marking it as __maybe_unused instead?
> > 
> > I don't mind either, I'll let Tobias decide, but his patch seems fine,
> > is there a drawback in his approach ? If it's just a matter of personal
> > preference, I'd rather not require a v2.
> 
> I don't mind either, it was just what seemed the more straight-forward
> fix. On the other hand, it seems that marking these functions as
> __maybe_unused is the more widely used method in other PHY drivers. In
> addition it would have the nice side-effect of the code always being
> compile-checked regardless of the value of CONFIG_PM_SLEEP.

That's a good point, haven't thought about it.

> Will send a v2 using __maybe_unused and will let you decide which one to
> pick :)

You can keep my R-b :-)

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] phy: zynqmp: Fix unused-function compiler warning
  2020-07-01 14:01         ` Laurent Pinchart
@ 2020-07-01 14:07           ` Tobias Klauser
  2020-07-01 14:09             ` Laurent Pinchart
  0 siblings, 1 reply; 11+ messages in thread
From: Tobias Klauser @ 2020-07-01 14:07 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Vinod Koul, Anurag Kumar Vulisha, Kishon Vijay Abraham I,
	Michal Simek, linux-kernel, linux-arm-kernel

On 2020-07-01 at 16:01:08 +0200, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:
> On Wed, Jul 01, 2020 at 04:00:26PM +0200, Tobias Klauser wrote:
> > On 2020-07-01 at 15:44:43 +0200, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:
> > > On Wed, Jul 01, 2020 at 07:06:43PM +0530, Vinod Koul wrote:
> > > > On 01-07-20, 16:19, Laurent Pinchart wrote:
> > > > > On Wed, Jul 01, 2020 at 11:04:38AM +0200, Tobias Klauser wrote:
> > > > > > This fixes the following compiler warning when building with
> > > > > > CONFIG_PM && !CONFIG_PM_SLEEP:
> > > > > > 
> > > > > > drivers/phy/xilinx/phy-zynqmp.c:830:12: warning: ‘xpsgtr_resume’ defined but not used [-Wunused-function]
> > > > > >   830 | static int xpsgtr_resume(struct device *dev)
> > > > > >       |            ^~~~~~~~~~~~~
> > > > > > drivers/phy/xilinx/phy-zynqmp.c:819:12: warning: ‘xpsgtr_suspend’ defined but not used [-Wunused-function]
> > > > > >   819 | static int xpsgtr_suspend(struct device *dev)
> > > > > >       |            ^~~~~~~~~~~~~~
> > > > > 
> > > > > Oops :-S Sorry about that.
> > > > > 
> > > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > 
> > Thanks for the review Laurent.
> > 
> > > > > Vinod or Kishon, can you pick this patch up, or do I need to send a pull
> > > > > request ? (It's my first driver in the PHY subsystem so I don't know
> > > > > what the usual practices are there)
> > > > 
> > > > patches are welcome :-)
> > > > 
> > > > > 
> > > > > > Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
> > > > > > ---
> > > > > >  drivers/phy/xilinx/phy-zynqmp.c | 4 ++--
> > > > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > > > 
> > > > > > diff --git a/drivers/phy/xilinx/phy-zynqmp.c b/drivers/phy/xilinx/phy-zynqmp.c
> > > > > > index 8babee2ce9ec..22a0ae635797 100644
> > > > > > --- a/drivers/phy/xilinx/phy-zynqmp.c
> > > > > > +++ b/drivers/phy/xilinx/phy-zynqmp.c
> > > > > > @@ -815,7 +815,7 @@ static struct phy *xpsgtr_xlate(struct device *dev,
> > > > > >   * Power Management
> > > > > >   */
> > > > > >  
> > > > > > -#ifdef CONFIG_PM
> > > > > > +#ifdef CONFIG_PM_SLEEP
> > > > 
> > > > How about marking it as __maybe_unused instead?
> > > 
> > > I don't mind either, I'll let Tobias decide, but his patch seems fine,
> > > is there a drawback in his approach ? If it's just a matter of personal
> > > preference, I'd rather not require a v2.
> > 
> > I don't mind either, it was just what seemed the more straight-forward
> > fix. On the other hand, it seems that marking these functions as
> > __maybe_unused is the more widely used method in other PHY drivers. In
> > addition it would have the nice side-effect of the code always being
> > compile-checked regardless of the value of CONFIG_PM_SLEEP.
> 
> That's a good point, haven't thought about it.
> 
> > Will send a v2 using __maybe_unused and will let you decide which one to
> > pick :)
> 
> You can keep my R-b :-)

Thanks :) FWIW, I'd also drop the #ifdef CONFIG_PM in the same patch for
the same reason. Is that OK with you as well?

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] phy: zynqmp: Fix unused-function compiler warning
  2020-07-01 14:07           ` Tobias Klauser
@ 2020-07-01 14:09             ` Laurent Pinchart
  0 siblings, 0 replies; 11+ messages in thread
From: Laurent Pinchart @ 2020-07-01 14:09 UTC (permalink / raw)
  To: Tobias Klauser
  Cc: Vinod Koul, Anurag Kumar Vulisha, Kishon Vijay Abraham I,
	Michal Simek, linux-kernel, linux-arm-kernel

Hi Tobias,

On Wed, Jul 01, 2020 at 04:07:09PM +0200, Tobias Klauser wrote:
> On 2020-07-01 at 16:01:08 +0200, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:
> > On Wed, Jul 01, 2020 at 04:00:26PM +0200, Tobias Klauser wrote:
> >> On 2020-07-01 at 15:44:43 +0200, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:
> >>> On Wed, Jul 01, 2020 at 07:06:43PM +0530, Vinod Koul wrote:
> >>>> On 01-07-20, 16:19, Laurent Pinchart wrote:
> >>>>> On Wed, Jul 01, 2020 at 11:04:38AM +0200, Tobias Klauser wrote:
> >>>>>> This fixes the following compiler warning when building with
> >>>>>> CONFIG_PM && !CONFIG_PM_SLEEP:
> >>>>>> 
> >>>>>> drivers/phy/xilinx/phy-zynqmp.c:830:12: warning: ‘xpsgtr_resume’ defined but not used [-Wunused-function]
> >>>>>>   830 | static int xpsgtr_resume(struct device *dev)
> >>>>>>       |            ^~~~~~~~~~~~~
> >>>>>> drivers/phy/xilinx/phy-zynqmp.c:819:12: warning: ‘xpsgtr_suspend’ defined but not used [-Wunused-function]
> >>>>>>   819 | static int xpsgtr_suspend(struct device *dev)
> >>>>>>       |            ^~~~~~~~~~~~~~
> >>>>> 
> >>>>> Oops :-S Sorry about that.
> >>>>> 
> >>>>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >> 
> >> Thanks for the review Laurent.
> >> 
> >>>>> Vinod or Kishon, can you pick this patch up, or do I need to send a pull
> >>>>> request ? (It's my first driver in the PHY subsystem so I don't know
> >>>>> what the usual practices are there)
> >>>> 
> >>>> patches are welcome :-)
> >>>> 
> >>>>> 
> >>>>>> Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
> >>>>>> ---
> >>>>>>  drivers/phy/xilinx/phy-zynqmp.c | 4 ++--
> >>>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>>>>> 
> >>>>>> diff --git a/drivers/phy/xilinx/phy-zynqmp.c b/drivers/phy/xilinx/phy-zynqmp.c
> >>>>>> index 8babee2ce9ec..22a0ae635797 100644
> >>>>>> --- a/drivers/phy/xilinx/phy-zynqmp.c
> >>>>>> +++ b/drivers/phy/xilinx/phy-zynqmp.c
> >>>>>> @@ -815,7 +815,7 @@ static struct phy *xpsgtr_xlate(struct device *dev,
> >>>>>>   * Power Management
> >>>>>>   */
> >>>>>>  
> >>>>>> -#ifdef CONFIG_PM
> >>>>>> +#ifdef CONFIG_PM_SLEEP
> >>>> 
> >>>> How about marking it as __maybe_unused instead?
> >>> 
> >>> I don't mind either, I'll let Tobias decide, but his patch seems fine,
> >>> is there a drawback in his approach ? If it's just a matter of personal
> >>> preference, I'd rather not require a v2.
> >> 
> >> I don't mind either, it was just what seemed the more straight-forward
> >> fix. On the other hand, it seems that marking these functions as
> >> __maybe_unused is the more widely used method in other PHY drivers. In
> >> addition it would have the nice side-effect of the code always being
> >> compile-checked regardless of the value of CONFIG_PM_SLEEP.
> > 
> > That's a good point, haven't thought about it.
> > 
> >> Will send a v2 using __maybe_unused and will let you decide which one to
> >> pick :)
> > 
> > You can keep my R-b :-)
> 
> Thanks :) FWIW, I'd also drop the #ifdef CONFIG_PM in the same patch for
> the same reason. Is that OK with you as well?

Sure.

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2] phy: zynqmp: Fix unused-function compiler warning
  2020-07-01  9:04 [PATCH] phy: zynqmp: Fix unused-function compiler warning Tobias Klauser
  2020-07-01 13:19 ` Laurent Pinchart
@ 2020-07-01 14:10 ` Tobias Klauser
  2020-07-01 15:05   ` Vinod Koul
  1 sibling, 1 reply; 11+ messages in thread
From: Tobias Klauser @ 2020-07-01 14:10 UTC (permalink / raw)
  To: Anurag Kumar Vulisha, Laurent Pinchart
  Cc: Kishon Vijay Abraham I, Vinod Koul, Michal Simek, linux-kernel,
	linux-arm-kernel

Mark xpsgtr_suspend and xpsgtr_resume as __maybe_unused to fix the
following compiler warning when building with !CONFIG_PM_SLEEP:

drivers/phy/xilinx/phy-zynqmp.c:830:12: warning: ‘xpsgtr_resume’ defined but not used [-Wunused-function]
  830 | static int xpsgtr_resume(struct device *dev)
      |            ^~~~~~~~~~~~~
drivers/phy/xilinx/phy-zynqmp.c:819:12: warning: ‘xpsgtr_suspend’ defined but not used [-Wunused-function]
  819 | static int xpsgtr_suspend(struct device *dev)
      |            ^~~~~~~~~~~~~~

Also drop the existing #ifdef CONFIG_PM so the functions are always
compile-checked regardless of CONFIG_PM and/or CONFIG_PM_SLEEP being
set.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
---
 drivers/phy/xilinx/phy-zynqmp.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/phy/xilinx/phy-zynqmp.c b/drivers/phy/xilinx/phy-zynqmp.c
index 8babee2ce9ec..2b0f921b6ee3 100644
--- a/drivers/phy/xilinx/phy-zynqmp.c
+++ b/drivers/phy/xilinx/phy-zynqmp.c
@@ -815,8 +815,7 @@ static struct phy *xpsgtr_xlate(struct device *dev,
  * Power Management
  */
 
-#ifdef CONFIG_PM
-static int xpsgtr_suspend(struct device *dev)
+static int __maybe_unused xpsgtr_suspend(struct device *dev)
 {
 	struct xpsgtr_dev *gtr_dev = dev_get_drvdata(dev);
 
@@ -827,7 +826,7 @@ static int xpsgtr_suspend(struct device *dev)
 	return 0;
 }
 
-static int xpsgtr_resume(struct device *dev)
+static int __maybe_unused xpsgtr_resume(struct device *dev)
 {
 	struct xpsgtr_dev *gtr_dev = dev_get_drvdata(dev);
 	unsigned int icm_cfg0, icm_cfg1;
@@ -854,7 +853,6 @@ static int xpsgtr_resume(struct device *dev)
 
 	return 0;
 }
-#endif /* CONFIG_PM */
 
 static const struct dev_pm_ops xpsgtr_pm_ops = {
 	SET_SYSTEM_SLEEP_PM_OPS(xpsgtr_suspend, xpsgtr_resume)
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] phy: zynqmp: Fix unused-function compiler warning
  2020-07-01 14:10 ` [PATCH v2] " Tobias Klauser
@ 2020-07-01 15:05   ` Vinod Koul
  0 siblings, 0 replies; 11+ messages in thread
From: Vinod Koul @ 2020-07-01 15:05 UTC (permalink / raw)
  To: Tobias Klauser
  Cc: Anurag Kumar Vulisha, Laurent Pinchart, Kishon Vijay Abraham I,
	Michal Simek, linux-kernel, linux-arm-kernel

On 01-07-20, 16:10, Tobias Klauser wrote:
> Mark xpsgtr_suspend and xpsgtr_resume as __maybe_unused to fix the
> following compiler warning when building with !CONFIG_PM_SLEEP:
> 
> drivers/phy/xilinx/phy-zynqmp.c:830:12: warning: ‘xpsgtr_resume’ defined but not used [-Wunused-function]
>   830 | static int xpsgtr_resume(struct device *dev)
>       |            ^~~~~~~~~~~~~
> drivers/phy/xilinx/phy-zynqmp.c:819:12: warning: ‘xpsgtr_suspend’ defined but not used [-Wunused-function]
>   819 | static int xpsgtr_suspend(struct device *dev)
>       |            ^~~~~~~~~~~~~~
> 
> Also drop the existing #ifdef CONFIG_PM so the functions are always
> compile-checked regardless of CONFIG_PM and/or CONFIG_PM_SLEEP being
> set.

Applied, thanks

-- 
~Vinod

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2020-07-01 15:05 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-01  9:04 [PATCH] phy: zynqmp: Fix unused-function compiler warning Tobias Klauser
2020-07-01 13:19 ` Laurent Pinchart
2020-07-01 13:36   ` Vinod Koul
2020-07-01 13:44     ` Laurent Pinchart
2020-07-01 13:55       ` Vinod Koul
2020-07-01 14:00       ` Tobias Klauser
2020-07-01 14:01         ` Laurent Pinchart
2020-07-01 14:07           ` Tobias Klauser
2020-07-01 14:09             ` Laurent Pinchart
2020-07-01 14:10 ` [PATCH v2] " Tobias Klauser
2020-07-01 15:05   ` Vinod Koul

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).