diff mbox

[RFC,2/3] tg3: Use pci_set_power_state() for setting PM states

Message ID 1393281245-3951-2-git-send-email-jdmason@kudzu.us
State RFC, archived
Delegated to: David Miller
Headers show

Commit Message

Jon Mason Feb. 24, 2014, 10:34 p.m. UTC
Use the existing infrastructure of pci_set_power_state() instead of
setting the relevant bits via PCI config read/write in the driver.
Also, use pci_pme_active() to set the PCI_PM_CTRL_PME_ENABLE bit in PCI
PM control register.  pci_set_power_state() and pci_pme_active() perform
the same operations as the driver did before, so there should be no
functional change.  That being said, this has not been tested.

Signed-off-by: Jon Mason <jdmason@kudzu.us>
Cc: Nithin Nayak Sujir <nsujir@broadcom.com>
Cc: Michael Chan <mchan@broadcom.com>
---
 drivers/net/ethernet/broadcom/tg3.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

Comments

Michael Chan Feb. 25, 2014, 12:15 a.m. UTC | #1
On Mon, 2014-02-24 at 15:34 -0700, Jon Mason wrote: 
> Use the existing infrastructure of pci_set_power_state() instead of
> setting the relevant bits via PCI config read/write in the driver.
> Also, use pci_pme_active() to set the PCI_PM_CTRL_PME_ENABLE bit in PCI
> PM control register.  pci_set_power_state() and pci_pme_active() perform
> the same operations as the driver did before, so there should be no
> functional change.  That being said, this has not been tested.
> 
> Signed-off-by: Jon Mason <jdmason@kudzu.us>
> Cc: Nithin Nayak Sujir <nsujir@broadcom.com>
> Cc: Michael Chan <mchan@broadcom.com>

I've reviewed the code and this patch should be ok.  The original intent
of the code was to force the power state to D0 in case it was clobbered
by an MMIO write not intended for that register due to a hardware bug.
If it was clobbered like that, pci_set_power_state() may not set it to
D0 if it thinks that it is already in D0.  There is no MMIO write up
until this point so it is ok to call pci_set_power_state().

Acked-by: Michael Chan <mchan@broadcom.com>

> ---
>  drivers/net/ethernet/broadcom/tg3.c | 12 ++----------
>  1 file changed, 2 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
> index 3167ed6..36c3fd9 100644
> --- a/drivers/net/ethernet/broadcom/tg3.c
> +++ b/drivers/net/ethernet/broadcom/tg3.c
> @@ -16362,22 +16362,14 @@ static int tg3_get_invariants(struct tg3 *tp, const struct pci_device_id *ent)
>  		 * for all chip writes not to mailbox registers.
>  		 */
>  		if (tg3_flag(tp, PCIX_MODE)) {
> -			u32 pm_reg;
> -
>  			tg3_flag_set(tp, PCIX_TARGET_HWBUG);
>  
>  			/* The chip can have it's power management PCI config
>  			 * space registers clobbered due to this bug.
>  			 * So explicitly force the chip into D0 here.
>  			 */
> -			pci_read_config_dword(tp->pdev,
> -					      tp->pdev->pm_cap + PCI_PM_CTRL,
> -					      &pm_reg);
> -			pm_reg &= ~PCI_PM_CTRL_STATE_MASK;
> -			pm_reg |= PCI_PM_CTRL_PME_ENABLE | 0 /* D0 */;
> -			pci_write_config_dword(tp->pdev,
> -					       tp->pdev->pm_cap + PCI_PM_CTRL,
> -					       pm_reg);
> +			pci_set_power_state(tp->pdev, PCI_D0);
> +			pci_pme_active(tp->pdev, true);
>  
>  			/* Also, force SERR#/PERR# in PCI command. */
>  			pci_read_config_word(tp->pdev, PCI_COMMAND, &pci_cmd);


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index 3167ed6..36c3fd9 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -16362,22 +16362,14 @@  static int tg3_get_invariants(struct tg3 *tp, const struct pci_device_id *ent)
 		 * for all chip writes not to mailbox registers.
 		 */
 		if (tg3_flag(tp, PCIX_MODE)) {
-			u32 pm_reg;
-
 			tg3_flag_set(tp, PCIX_TARGET_HWBUG);
 
 			/* The chip can have it's power management PCI config
 			 * space registers clobbered due to this bug.
 			 * So explicitly force the chip into D0 here.
 			 */
-			pci_read_config_dword(tp->pdev,
-					      tp->pdev->pm_cap + PCI_PM_CTRL,
-					      &pm_reg);
-			pm_reg &= ~PCI_PM_CTRL_STATE_MASK;
-			pm_reg |= PCI_PM_CTRL_PME_ENABLE | 0 /* D0 */;
-			pci_write_config_dword(tp->pdev,
-					       tp->pdev->pm_cap + PCI_PM_CTRL,
-					       pm_reg);
+			pci_set_power_state(tp->pdev, PCI_D0);
+			pci_pme_active(tp->pdev, true);
 
 			/* Also, force SERR#/PERR# in PCI command. */
 			pci_read_config_word(tp->pdev, PCI_COMMAND, &pci_cmd);