2020-06-24 17:57:47

by Vaibhav Gupta

[permalink] [raw]
Subject: [PATCH v1] bnx2x: use generic power management

With legacy PM, drivers themselves were responsible for managing the
device's power states and takes care of register states.

After upgrading to the generic structure, PCI core will take care of
required tasks and drivers should do only device-specific operations.

The driver was also calling bnx2x_set_power_state() to set the power state
of the device by changing the device's registers' value. It is no more
needed.

Compile-tested only.

Signed-off-by: Vaibhav Gupta <[email protected]>
---
drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 15 ++++++---------
drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h | 4 +---
drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 3 +--
3 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
index ee9e9290f112..e3d92e4f2193 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
@@ -4988,8 +4988,9 @@ void bnx2x_tx_timeout(struct net_device *dev, unsigned int txqueue)
bnx2x_schedule_sp_rtnl(bp, BNX2X_SP_RTNL_TX_TIMEOUT, 0);
}

-int bnx2x_suspend(struct pci_dev *pdev, pm_message_t state)
+static int __maybe_unused bnx2x_suspend(struct device *dev_d)
{
+ struct pci_dev *pdev = to_pci_dev(dev_d);
struct net_device *dev = pci_get_drvdata(pdev);
struct bnx2x *bp;

@@ -5001,8 +5002,6 @@ int bnx2x_suspend(struct pci_dev *pdev, pm_message_t state)

rtnl_lock();

- pci_save_state(pdev);
-
if (!netif_running(dev)) {
rtnl_unlock();
return 0;
@@ -5012,15 +5011,14 @@ int bnx2x_suspend(struct pci_dev *pdev, pm_message_t state)

bnx2x_nic_unload(bp, UNLOAD_CLOSE, false);

- bnx2x_set_power_state(bp, pci_choose_state(pdev, state));
-
rtnl_unlock();

return 0;
}

-int bnx2x_resume(struct pci_dev *pdev)
+static int __maybe_unused bnx2x_resume(struct device *dev_d)
{
+ struct pci_dev *pdev = to_pci_dev(dev_d);
struct net_device *dev = pci_get_drvdata(pdev);
struct bnx2x *bp;
int rc;
@@ -5038,14 +5036,11 @@ int bnx2x_resume(struct pci_dev *pdev)

rtnl_lock();

- pci_restore_state(pdev);
-
if (!netif_running(dev)) {
rtnl_unlock();
return 0;
}

- bnx2x_set_power_state(bp, PCI_D0);
netif_device_attach(dev);

rc = bnx2x_nic_load(bp, LOAD_OPEN);
@@ -5055,6 +5050,8 @@ int bnx2x_resume(struct pci_dev *pdev)
return rc;
}

+SIMPLE_DEV_PM_OPS(bnx2x_pm_ops, bnx2x_suspend, bnx2x_resume);
+
void bnx2x_set_ctx_validation(struct bnx2x *bp, struct eth_context *cxt,
u32 cid)
{
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h
index 6f1352d51cb2..a9817cd283fe 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h
@@ -541,9 +541,7 @@ int bnx2x_change_mac_addr(struct net_device *dev, void *p);
/* NAPI poll Tx part */
int bnx2x_tx_int(struct bnx2x *bp, struct bnx2x_fp_txdata *txdata);

-/* suspend/resume callbacks */
-int bnx2x_suspend(struct pci_dev *pdev, pm_message_t state);
-int bnx2x_resume(struct pci_dev *pdev);
+extern const struct dev_pm_ops bnx2x_pm_ops;

/* Release IRQ vectors */
void bnx2x_free_irq(struct bnx2x *bp);
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
index db5107e7937c..ea60cd436f5e 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
@@ -14462,8 +14462,7 @@ static struct pci_driver bnx2x_pci_driver = {
.id_table = bnx2x_pci_tbl,
.probe = bnx2x_init_one,
.remove = bnx2x_remove_one,
- .suspend = bnx2x_suspend,
- .resume = bnx2x_resume,
+ .driver.pm = &bnx2x_pm_ops,
.err_handler = &bnx2x_err_handler,
#ifdef CONFIG_BNX2X_SRIOV
.sriov_configure = bnx2x_sriov_configure,
--
2.27.0


2020-06-25 18:21:55

by Igor Russkikh

[permalink] [raw]
Subject: Re: [EXT] [PATCH v1] bnx2x: use generic power management



On 24/06/2020 8:51 pm, Vaibhav Gupta wrote:
> External Email
>
> ----------------------------------------------------------------------
> With legacy PM, drivers themselves were responsible for managing the
> device's power states and takes care of register states.
>
> After upgrading to the generic structure, PCI core will take care of
> required tasks and drivers should do only device-specific operations.
>
> The driver was also calling bnx2x_set_power_state() to set the power state
> of the device by changing the device's registers' value. It is no more
> needed.
>
> Compile-tested only.
>
> Signed-off-by: Vaibhav Gupta <[email protected]>
> ---
> drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 15 ++++++---------
> drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h | 4 +---
> drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 3 +--
> 3 files changed, 8 insertions(+), 14 deletions(-)

Acked-by: Igor Russkikh <[email protected]>

Sudarsana, could you please give it a short sanity test and report back?

Thanks,
Igor

2020-06-26 11:48:53

by Sudarsana Reddy Kalluru

[permalink] [raw]
Subject: RE: [EXT] [PATCH v1] bnx2x: use generic power management


> -----Original Message-----
> From: Igor Russkikh <[email protected]>
> Sent: Thursday, June 25, 2020 10:16 PM
> To: Vaibhav Gupta <[email protected]>; Bjorn Helgaas
> <[email protected]>; Bjorn Helgaas <[email protected]>;
> [email protected]; Vaibhav Gupta <[email protected]>; David S.
> Miller <[email protected]>; Jakub Kicinski <[email protected]>; Ariel Elior
> <[email protected]>; Sudarsana Reddy Kalluru <[email protected]>; GR-
> everest-linux-l2 <[email protected]>
> Cc: [email protected]; [email protected]; linux-kernel-
> [email protected]; [email protected]
> Subject: Re: [EXT] [PATCH v1] bnx2x: use generic power management
>
>
>
> On 24/06/2020 8:51 pm, Vaibhav Gupta wrote:
> > External Email
> >
> > ----------------------------------------------------------------------
> > With legacy PM, drivers themselves were responsible for managing the
> > device's power states and takes care of register states.
> >
> > After upgrading to the generic structure, PCI core will take care of
> > required tasks and drivers should do only device-specific operations.
> >
> > The driver was also calling bnx2x_set_power_state() to set the power
> > state of the device by changing the device's registers' value. It is
> > no more needed.
> >
> > Compile-tested only.
> >
> > Signed-off-by: Vaibhav Gupta <[email protected]>
> > ---
> > drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 15 ++++++---------
> > drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h | 4 +---
> > drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 3 +--
> > 3 files changed, 8 insertions(+), 14 deletions(-)
>
> Acked-by: Igor Russkikh <[email protected]>
>
> Sudarsana, could you please give it a short sanity test and report back?
>
> Thanks,
> Igor

I have run the basic sanity test with these changes in the driver, didn’t see any issues.

Thanks,
Sudarsana

2020-06-26 19:15:39

by David Miller

[permalink] [raw]
Subject: Re: [PATCH v1] bnx2x: use generic power management

From: Vaibhav Gupta <[email protected]>
Date: Wed, 24 Jun 2020 23:21:17 +0530

> With legacy PM, drivers themselves were responsible for managing the
> device's power states and takes care of register states.
>
> After upgrading to the generic structure, PCI core will take care of
> required tasks and drivers should do only device-specific operations.
>
> The driver was also calling bnx2x_set_power_state() to set the power state
> of the device by changing the device's registers' value. It is no more
> needed.
>
> Compile-tested only.
>
> Signed-off-by: Vaibhav Gupta <[email protected]>

Applied, thanks.

2020-06-27 05:52:27

by Vaibhav Gupta

[permalink] [raw]
Subject: Re: [PATCH v1] bnx2x: use generic power management

On Sat, 27 Jun 2020 at 00:44, David Miller <[email protected]> wrote:
>
> From: Vaibhav Gupta <[email protected]>
> Date: Wed, 24 Jun 2020 23:21:17 +0530
>
> > With legacy PM, drivers themselves were responsible for managing the
> > device's power states and takes care of register states.
> >
> > After upgrading to the generic structure, PCI core will take care of
> > required tasks and drivers should do only device-specific operations.
> >
> > The driver was also calling bnx2x_set_power_state() to set the power state
> > of the device by changing the device's registers' value. It is no more
> > needed.
> >
> > Compile-tested only.
> >
> > Signed-off-by: Vaibhav Gupta <[email protected]>
>
> Applied, thanks.
Thanks everyone :)
-- Vaibhav Gupta