| Message ID | 20190819160643.27998-5-efremov@linux.com |
|---|---|
| State | Superseded |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | Simplify PCIe hotplug indicator control | expand |
On Mon, Aug 19, 2019 at 07:06:43PM +0300, Denis Efremov wrote: > Remove pciehp_green_led_{on,off,blink}() and use pciehp_set_indicators() > instead, since the code is mostly the same. > > Signed-off-by: Denis Efremov <efremov@linux.com> Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> > --- > drivers/pci/hotplug/pciehp.h | 3 --- > drivers/pci/hotplug/pciehp_ctrl.c | 12 ++++++++--- > drivers/pci/hotplug/pciehp_hpc.c | 36 ------------------------------- > 3 files changed, 9 insertions(+), 42 deletions(-) > > diff --git a/drivers/pci/hotplug/pciehp.h b/drivers/pci/hotplug/pciehp.h > index acda513f37d7..da429345cf70 100644 > --- a/drivers/pci/hotplug/pciehp.h > +++ b/drivers/pci/hotplug/pciehp.h > @@ -170,9 +170,6 @@ void pciehp_get_power_status(struct controller *ctrl, u8 *status); > void pciehp_set_indicators(struct controller *ctrl, int pwr, int attn); > void pciehp_get_latch_status(struct controller *ctrl, u8 *status); > int pciehp_query_power_fault(struct controller *ctrl); > -void pciehp_green_led_on(struct controller *ctrl); > -void pciehp_green_led_off(struct controller *ctrl); > -void pciehp_green_led_blink(struct controller *ctrl); > bool pciehp_card_present(struct controller *ctrl); > bool pciehp_card_present_or_link_active(struct controller *ctrl); > int pciehp_check_link_status(struct controller *ctrl); > diff --git a/drivers/pci/hotplug/pciehp_ctrl.c b/drivers/pci/hotplug/pciehp_ctrl.c > index 232f7bfcfce9..862fe86e87cc 100644 > --- a/drivers/pci/hotplug/pciehp_ctrl.c > +++ b/drivers/pci/hotplug/pciehp_ctrl.c > @@ -65,7 +65,9 @@ static int board_added(struct controller *ctrl) > return retval; > } > > - pciehp_green_led_blink(ctrl); > + pciehp_set_indicators(ctrl, PCI_EXP_SLTCTL_PWR_IND_BLINK, > + PCI_EXP_SLTCTL_ATTN_IND_NONE); > + > > /* Check link training status */ > retval = pciehp_check_link_status(ctrl); > @@ -124,7 +126,9 @@ static void remove_board(struct controller *ctrl, bool safe_removal) > } > > /* turn off Green LED */ > - pciehp_green_led_off(ctrl); > + pciehp_set_indicators(ctrl, PCI_EXP_SLTCTL_PWR_IND_OFF, > + PCI_EXP_SLTCTL_ATTN_IND_NONE); > + > } > > static int pciehp_enable_slot(struct controller *ctrl); > @@ -311,7 +315,9 @@ static int pciehp_enable_slot(struct controller *ctrl) > pm_runtime_get_sync(&ctrl->pcie->port->dev); > ret = __pciehp_enable_slot(ctrl); > if (ret && ATTN_BUTTN(ctrl)) > - pciehp_green_led_off(ctrl); /* may be blinking */ > + /* may be blinking */ > + pciehp_set_indicators(ctrl, PCI_EXP_SLTCTL_PWR_IND_OFF, > + PCI_EXP_SLTCTL_ATTN_IND_NONE); > pm_runtime_put(&ctrl->pcie->port->dev); > > mutex_lock(&ctrl->state_lock); > diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c > index 8f894fd5cd27..9dc1ecd703b9 100644 > --- a/drivers/pci/hotplug/pciehp_hpc.c > +++ b/drivers/pci/hotplug/pciehp_hpc.c > @@ -447,42 +447,6 @@ void pciehp_set_indicators(struct controller *ctrl, int pwr, int attn) > } > } > > -void pciehp_green_led_on(struct controller *ctrl) > -{ > - if (!PWR_LED(ctrl)) > - return; > - > - pcie_write_cmd_nowait(ctrl, PCI_EXP_SLTCTL_PWR_IND_ON, > - PCI_EXP_SLTCTL_PIC); > - ctrl_dbg(ctrl, "%s: SLOTCTRL %x write cmd %x\n", __func__, > - pci_pcie_cap(ctrl->pcie->port) + PCI_EXP_SLTCTL, > - PCI_EXP_SLTCTL_PWR_IND_ON); > -} > - > -void pciehp_green_led_off(struct controller *ctrl) > -{ > - if (!PWR_LED(ctrl)) > - return; > - > - pcie_write_cmd_nowait(ctrl, PCI_EXP_SLTCTL_PWR_IND_OFF, > - PCI_EXP_SLTCTL_PIC); > - ctrl_dbg(ctrl, "%s: SLOTCTRL %x write cmd %x\n", __func__, > - pci_pcie_cap(ctrl->pcie->port) + PCI_EXP_SLTCTL, > - PCI_EXP_SLTCTL_PWR_IND_OFF); > -} > - > -void pciehp_green_led_blink(struct controller *ctrl) > -{ > - if (!PWR_LED(ctrl)) > - return; > - > - pcie_write_cmd_nowait(ctrl, PCI_EXP_SLTCTL_PWR_IND_BLINK, > - PCI_EXP_SLTCTL_PIC); > - ctrl_dbg(ctrl, "%s: SLOTCTRL %x write cmd %x\n", __func__, > - pci_pcie_cap(ctrl->pcie->port) + PCI_EXP_SLTCTL, > - PCI_EXP_SLTCTL_PWR_IND_BLINK); > -} > - > int pciehp_power_on_slot(struct controller *ctrl) > { > struct pci_dev *pdev = ctrl_dev(ctrl); > -- > 2.21.0 >
On Tue, Aug 20, 2019 at 2:09 AM Denis Efremov <efremov@linux.com> wrote: > > Remove pciehp_green_led_{on,off,blink}() and use pciehp_set_indicators() > instead, since the code is mostly the same. > > Signed-off-by: Denis Efremov <efremov@linux.com> > --- > drivers/pci/hotplug/pciehp.h | 3 --- > drivers/pci/hotplug/pciehp_ctrl.c | 12 ++++++++--- > drivers/pci/hotplug/pciehp_hpc.c | 36 ------------------------------- > 3 files changed, 9 insertions(+), 42 deletions(-) > > diff --git a/drivers/pci/hotplug/pciehp.h b/drivers/pci/hotplug/pciehp.h > index acda513f37d7..da429345cf70 100644 > --- a/drivers/pci/hotplug/pciehp.h > +++ b/drivers/pci/hotplug/pciehp.h > @@ -170,9 +170,6 @@ void pciehp_get_power_status(struct controller *ctrl, u8 *status); > void pciehp_set_indicators(struct controller *ctrl, int pwr, int attn); > void pciehp_get_latch_status(struct controller *ctrl, u8 *status); > int pciehp_query_power_fault(struct controller *ctrl); > -void pciehp_green_led_on(struct controller *ctrl); > -void pciehp_green_led_off(struct controller *ctrl); > -void pciehp_green_led_blink(struct controller *ctrl); > bool pciehp_card_present(struct controller *ctrl); > bool pciehp_card_present_or_link_active(struct controller *ctrl); > int pciehp_check_link_status(struct controller *ctrl); > diff --git a/drivers/pci/hotplug/pciehp_ctrl.c b/drivers/pci/hotplug/pciehp_ctrl.c > index 232f7bfcfce9..862fe86e87cc 100644 > --- a/drivers/pci/hotplug/pciehp_ctrl.c > +++ b/drivers/pci/hotplug/pciehp_ctrl.c > @@ -65,7 +65,9 @@ static int board_added(struct controller *ctrl) > return retval; > } > > - pciehp_green_led_blink(ctrl); > + pciehp_set_indicators(ctrl, PCI_EXP_SLTCTL_PWR_IND_BLINK, > + PCI_EXP_SLTCTL_ATTN_IND_NONE); I think it woud be good if you provided a helper macro for setting one of the indicators rather than open coding the _NONE constant. e.g. #define set_power_indicator(ctrl, x) pciehp_set_indicators(ctrl, (x), PCI_EXP_SLTCTL_ATTN_IND_NONE); then you can do: set_power_indicator(ctrl, PCI_EXP_SLTCTL_PWR_IND_BLINK); which is a bit nicer to read.
diff --git a/drivers/pci/hotplug/pciehp.h b/drivers/pci/hotplug/pciehp.h index acda513f37d7..da429345cf70 100644 --- a/drivers/pci/hotplug/pciehp.h +++ b/drivers/pci/hotplug/pciehp.h @@ -170,9 +170,6 @@ void pciehp_get_power_status(struct controller *ctrl, u8 *status); void pciehp_set_indicators(struct controller *ctrl, int pwr, int attn); void pciehp_get_latch_status(struct controller *ctrl, u8 *status); int pciehp_query_power_fault(struct controller *ctrl); -void pciehp_green_led_on(struct controller *ctrl); -void pciehp_green_led_off(struct controller *ctrl); -void pciehp_green_led_blink(struct controller *ctrl); bool pciehp_card_present(struct controller *ctrl); bool pciehp_card_present_or_link_active(struct controller *ctrl); int pciehp_check_link_status(struct controller *ctrl); diff --git a/drivers/pci/hotplug/pciehp_ctrl.c b/drivers/pci/hotplug/pciehp_ctrl.c index 232f7bfcfce9..862fe86e87cc 100644 --- a/drivers/pci/hotplug/pciehp_ctrl.c +++ b/drivers/pci/hotplug/pciehp_ctrl.c @@ -65,7 +65,9 @@ static int board_added(struct controller *ctrl) return retval; } - pciehp_green_led_blink(ctrl); + pciehp_set_indicators(ctrl, PCI_EXP_SLTCTL_PWR_IND_BLINK, + PCI_EXP_SLTCTL_ATTN_IND_NONE); + /* Check link training status */ retval = pciehp_check_link_status(ctrl); @@ -124,7 +126,9 @@ static void remove_board(struct controller *ctrl, bool safe_removal) } /* turn off Green LED */ - pciehp_green_led_off(ctrl); + pciehp_set_indicators(ctrl, PCI_EXP_SLTCTL_PWR_IND_OFF, + PCI_EXP_SLTCTL_ATTN_IND_NONE); + } static int pciehp_enable_slot(struct controller *ctrl); @@ -311,7 +315,9 @@ static int pciehp_enable_slot(struct controller *ctrl) pm_runtime_get_sync(&ctrl->pcie->port->dev); ret = __pciehp_enable_slot(ctrl); if (ret && ATTN_BUTTN(ctrl)) - pciehp_green_led_off(ctrl); /* may be blinking */ + /* may be blinking */ + pciehp_set_indicators(ctrl, PCI_EXP_SLTCTL_PWR_IND_OFF, + PCI_EXP_SLTCTL_ATTN_IND_NONE); pm_runtime_put(&ctrl->pcie->port->dev); mutex_lock(&ctrl->state_lock); diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c index 8f894fd5cd27..9dc1ecd703b9 100644 --- a/drivers/pci/hotplug/pciehp_hpc.c +++ b/drivers/pci/hotplug/pciehp_hpc.c @@ -447,42 +447,6 @@ void pciehp_set_indicators(struct controller *ctrl, int pwr, int attn) } } -void pciehp_green_led_on(struct controller *ctrl) -{ - if (!PWR_LED(ctrl)) - return; - - pcie_write_cmd_nowait(ctrl, PCI_EXP_SLTCTL_PWR_IND_ON, - PCI_EXP_SLTCTL_PIC); - ctrl_dbg(ctrl, "%s: SLOTCTRL %x write cmd %x\n", __func__, - pci_pcie_cap(ctrl->pcie->port) + PCI_EXP_SLTCTL, - PCI_EXP_SLTCTL_PWR_IND_ON); -} - -void pciehp_green_led_off(struct controller *ctrl) -{ - if (!PWR_LED(ctrl)) - return; - - pcie_write_cmd_nowait(ctrl, PCI_EXP_SLTCTL_PWR_IND_OFF, - PCI_EXP_SLTCTL_PIC); - ctrl_dbg(ctrl, "%s: SLOTCTRL %x write cmd %x\n", __func__, - pci_pcie_cap(ctrl->pcie->port) + PCI_EXP_SLTCTL, - PCI_EXP_SLTCTL_PWR_IND_OFF); -} - -void pciehp_green_led_blink(struct controller *ctrl) -{ - if (!PWR_LED(ctrl)) - return; - - pcie_write_cmd_nowait(ctrl, PCI_EXP_SLTCTL_PWR_IND_BLINK, - PCI_EXP_SLTCTL_PIC); - ctrl_dbg(ctrl, "%s: SLOTCTRL %x write cmd %x\n", __func__, - pci_pcie_cap(ctrl->pcie->port) + PCI_EXP_SLTCTL, - PCI_EXP_SLTCTL_PWR_IND_BLINK); -} - int pciehp_power_on_slot(struct controller *ctrl) { struct pci_dev *pdev = ctrl_dev(ctrl);
Remove pciehp_green_led_{on,off,blink}() and use pciehp_set_indicators() instead, since the code is mostly the same. Signed-off-by: Denis Efremov <efremov@linux.com> --- drivers/pci/hotplug/pciehp.h | 3 --- drivers/pci/hotplug/pciehp_ctrl.c | 12 ++++++++--- drivers/pci/hotplug/pciehp_hpc.c | 36 ------------------------------- 3 files changed, 9 insertions(+), 42 deletions(-)