Skip to content

Commit

Permalink
PCI: pciehp: Unify controller and slot structs
Browse files Browse the repository at this point in the history 
pciehp was originally introduced together with shpchp in a single
commit, c16b4b1 ("PCI Hotplug: Add SHPC and PCI Express hot-plug
drivers"):
https://git.kernel.org/tglx/history/c/c16b4b14d980

shpchp supports up to 31 slots per controller, hence uses separate slot
and controller structs.  pciehp has a 1:1 relationship between slot and
controller and therefore never required this separation.  Nevertheless,
because much of the code had been copy-pasted between the two drivers,
pciehp likewise uses separate structs to this very day.

The artificial separation of data structures adds unnecessary complexity
and bloat to pciehp and requires constantly chasing pointers at runtime.

Simplify the driver by merging struct slot into struct controller.
Merge the slot constructor pcie_init_slot() and the destructor
pcie_cleanup_slot() into the controller counterparts.

No functional change intended.

Signed-off-by: Lukas Wunner <[email protected]>
Signed-off-by: Bjorn Helgaas <[email protected]>
  • Loading branch information
@l1k @bjorn-helgaas
l1k authored and bjorn-helgaas committed Sep 18, 2018
1 parent 80696f9 commit 5790a9c
Show file tree
Hide file tree
Showing 5 changed files with 210 additions and 282 deletions.
67 changes: 30 additions & 37 deletions drivers/pci/hotplug/pciehp.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,32 +58,13 @@ do { \

#define SLOT_NAME_SIZE 10

/**
* struct slot - PCIe hotplug slot
* @state: current state machine position
* @ctrl: pointer to the slot's controller structure
* @hotplug_slot: pointer to the structure registered with the PCI hotplug core
* @work: work item to turn the slot on or off after 5 seconds in response to
* an Attention Button press
* @lock: protects reads and writes of @state;
* protects scheduling, execution and cancellation of @work
*/
struct slot {
u8 state;
struct controller *ctrl;
struct hotplug_slot *hotplug_slot;
struct delayed_work work;
struct mutex lock;
};

/**
* struct controller - PCIe hotplug controller
* @ctrl_lock: serializes writes to the Slot Control register
* @pcie: pointer to the controller's PCIe port service device
* @reset_lock: prevents access to the Data Link Layer Link Active bit in the
* Link Status register and to the Presence Detect State bit in the Slot
* Status register during a slot reset which may cause them to flap
* @slot: pointer to the controller's slot structure
* @queue: wait queue to wake up on reception of a Command Completed event,
* used for synchronous writes to the Slot Control register
* @slot_cap: cached copy of the Slot Capabilities register
Expand All @@ -105,15 +86,23 @@ struct slot {
* that has not yet been cleared by the user
* @pending_events: used by the IRQ handler to save events retrieved from the
* Slot Status register for later consumption by the IRQ thread
* @state: current state machine position
* @lock: protects reads and writes of @state;
* protects scheduling, execution and cancellation of @work
* @work: work item to turn the slot on or off after 5 seconds
* in response to an Attention Button press
* @hotplug_slot: pointer to the structure registered with the PCI hotplug core
* @request_result: result of last user request submitted to the IRQ thread
* @requester: wait queue to wake up on completion of user request,
* used for synchronous slot enable/disable request via sysfs
*
* PCIe hotplug has a 1:1 relationship between controller and slot, hence
* unlike other drivers, the two aren't represented by separate structures.
*/
struct controller {
struct mutex ctrl_lock;
struct pcie_device *pcie;
struct rw_semaphore reset_lock;
struct slot *slot;
wait_queue_head_t queue;
u32 slot_cap;
u16 slot_ctrl;
Expand All @@ -124,6 +113,10 @@ struct controller {
unsigned int notification_enabled:1;
unsigned int power_fault_detected;
atomic_t pending_events;
u8 state;
struct mutex lock;
struct delayed_work work;
struct hotplug_slot *hotplug_slot;
int request_result;
wait_queue_head_t requester;
};
Expand Down Expand Up @@ -174,26 +167,26 @@ struct controller {
#define PSN(ctrl) (((ctrl)->slot_cap & PCI_EXP_SLTCAP_PSN) >> 19)

void pciehp_request(struct controller *ctrl, int action);
void pciehp_handle_button_press(struct slot *slot);
void pciehp_handle_disable_request(struct slot *slot);
void pciehp_handle_presence_or_link_change(struct slot *slot, u32 events);
int pciehp_configure_device(struct slot *p_slot);
void pciehp_unconfigure_device(struct slot *p_slot, bool presence);
void pciehp_handle_button_press(struct controller *ctrl);
void pciehp_handle_disable_request(struct controller *ctrl);
void pciehp_handle_presence_or_link_change(struct controller *ctrl, u32 events);
int pciehp_configure_device(struct controller *ctrl);
void pciehp_unconfigure_device(struct controller *ctrl, bool presence);
void pciehp_queue_pushbutton_work(struct work_struct *work);
struct controller *pcie_init(struct pcie_device *dev);
int pcie_init_notification(struct controller *ctrl);
void pcie_shutdown_notification(struct controller *ctrl);
void pcie_clear_hotplug_events(struct controller *ctrl);
int pciehp_power_on_slot(struct slot *slot);
void pciehp_power_off_slot(struct slot *slot);
void pciehp_get_power_status(struct slot *slot, u8 *status);

void pciehp_set_attention_status(struct slot *slot, u8 status);
void pciehp_get_latch_status(struct slot *slot, u8 *status);
int pciehp_query_power_fault(struct slot *slot);
void pciehp_green_led_on(struct slot *slot);
void pciehp_green_led_off(struct slot *slot);
void pciehp_green_led_blink(struct slot *slot);
int pciehp_power_on_slot(struct controller *ctrl);
void pciehp_power_off_slot(struct controller *ctrl);
void pciehp_get_power_status(struct controller *ctrl, u8 *status);

void pciehp_set_attention_status(struct controller *ctrl, u8 status);
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);
Expand All @@ -207,9 +200,9 @@ int pciehp_get_attention_status(struct hotplug_slot *hotplug_slot, u8 *status);
int pciehp_set_raw_indicator_status(struct hotplug_slot *h_slot, u8 status);
int pciehp_get_raw_indicator_status(struct hotplug_slot *h_slot, u8 *status);

static inline const char *slot_name(struct slot *slot)
static inline const char *slot_name(struct controller *ctrl)
{
return hotplug_slot_name(slot->hotplug_slot);
return hotplug_slot_name(ctrl->hotplug_slot);
}

#endif /* _PCIEHP_H */
53 changes: 24 additions & 29 deletions drivers/pci/hotplug/pciehp_core.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ static int get_adapter_status(struct hotplug_slot *slot, u8 *value);

static int init_slot(struct controller *ctrl)
{
struct slot *slot = ctrl->slot;
struct hotplug_slot *hotplug = NULL;
struct hotplug_slot_info *info = NULL;
struct hotplug_slot_ops *ops = NULL;
Expand Down Expand Up @@ -88,9 +87,9 @@ static int init_slot(struct controller *ctrl)

/* register this slot with the hotplug pci core */
hotplug->info = info;
hotplug->private = slot;
hotplug->private = ctrl;
hotplug->ops = ops;
slot->hotplug_slot = hotplug;
ctrl->hotplug_slot = hotplug;
snprintf(name, SLOT_NAME_SIZE, "%u", PSN(ctrl));

retval = pci_hp_initialize(hotplug,
Expand All @@ -108,7 +107,7 @@ static int init_slot(struct controller *ctrl)

static void cleanup_slot(struct controller *ctrl)
{
struct hotplug_slot *hotplug_slot = ctrl->slot->hotplug_slot;
struct hotplug_slot *hotplug_slot = ctrl->hotplug_slot;

pci_hp_destroy(hotplug_slot);
kfree(hotplug_slot->ops);
Expand All @@ -121,44 +120,44 @@ static void cleanup_slot(struct controller *ctrl)
*/
static int set_attention_status(struct hotplug_slot *hotplug_slot, u8 status)
{
struct slot *slot = hotplug_slot->private;
struct pci_dev *pdev = slot->ctrl->pcie->port;
struct controller *ctrl = hotplug_slot->private;
struct pci_dev *pdev = ctrl->pcie->port;

pci_config_pm_runtime_get(pdev);
pciehp_set_attention_status(slot, status);
pciehp_set_attention_status(ctrl, status);
pci_config_pm_runtime_put(pdev);
return 0;
}

static int get_power_status(struct hotplug_slot *hotplug_slot, u8 *value)
{
struct slot *slot = hotplug_slot->private;
struct pci_dev *pdev = slot->ctrl->pcie->port;
struct controller *ctrl = hotplug_slot->private;
struct pci_dev *pdev = ctrl->pcie->port;

pci_config_pm_runtime_get(pdev);
pciehp_get_power_status(slot, value);
pciehp_get_power_status(ctrl, value);
pci_config_pm_runtime_put(pdev);
return 0;
}

static int get_latch_status(struct hotplug_slot *hotplug_slot, u8 *value)
{
struct slot *slot = hotplug_slot->private;
struct pci_dev *pdev = slot->ctrl->pcie->port;
struct controller *ctrl = hotplug_slot->private;
struct pci_dev *pdev = ctrl->pcie->port;

pci_config_pm_runtime_get(pdev);
pciehp_get_latch_status(slot, value);
pciehp_get_latch_status(ctrl, value);
pci_config_pm_runtime_put(pdev);
return 0;
}

static int get_adapter_status(struct hotplug_slot *hotplug_slot, u8 *value)
{
struct slot *slot = hotplug_slot->private;
struct pci_dev *pdev = slot->ctrl->pcie->port;
struct controller *ctrl = hotplug_slot->private;
struct pci_dev *pdev = ctrl->pcie->port;

pci_config_pm_runtime_get(pdev);
*value = pciehp_card_present_or_link_active(slot->ctrl);
*value = pciehp_card_present_or_link_active(ctrl);
pci_config_pm_runtime_put(pdev);
return 0;
}
Expand All @@ -175,28 +174,26 @@ static int get_adapter_status(struct hotplug_slot *hotplug_slot, u8 *value)
*/
static void pciehp_check_presence(struct controller *ctrl)
{
struct slot *slot = ctrl->slot;
bool occupied;

down_read(&ctrl->reset_lock);
mutex_lock(&slot->lock);
mutex_lock(&ctrl->lock);

occupied = pciehp_card_present_or_link_active(ctrl);
if ((occupied && (slot->state == OFF_STATE ||
slot->state == BLINKINGON_STATE)) ||
(!occupied && (slot->state == ON_STATE ||
slot->state == BLINKINGOFF_STATE)))
if ((occupied && (ctrl->state == OFF_STATE ||
ctrl->state == BLINKINGON_STATE)) ||
(!occupied && (ctrl->state == ON_STATE ||
ctrl->state == BLINKINGOFF_STATE)))
pciehp_request(ctrl, PCI_EXP_SLTSTA_PDC);

mutex_unlock(&slot->lock);
mutex_unlock(&ctrl->lock);
up_read(&ctrl->reset_lock);
}

static int pciehp_probe(struct pcie_device *dev)
{
int rc;
struct controller *ctrl;
struct slot *slot;

/* If this is not a "hotplug" service, we have no business here. */
if (dev->service != PCIE_PORT_SERVICE_HP)
Expand Down Expand Up @@ -234,8 +231,7 @@ static int pciehp_probe(struct pcie_device *dev)
}

/* Publish to user space */
slot = ctrl->slot;
rc = pci_hp_add(slot->hotplug_slot);
rc = pci_hp_add(ctrl->hotplug_slot);
if (rc) {
ctrl_err(ctrl, "Publication to user space failed (%d)\n", rc);
goto err_out_shutdown_notification;
Expand All @@ -258,7 +254,7 @@ static void pciehp_remove(struct pcie_device *dev)
{
struct controller *ctrl = get_service_data(dev);

pci_hp_del(ctrl->slot->hotplug_slot);
pci_hp_del(ctrl->hotplug_slot);
pcie_shutdown_notification(ctrl);
cleanup_slot(ctrl);
pciehp_release_ctrl(ctrl);
Expand All @@ -273,14 +269,13 @@ static int pciehp_suspend(struct pcie_device *dev)
static int pciehp_resume_noirq(struct pcie_device *dev)
{
struct controller *ctrl = get_service_data(dev);
struct slot *slot = ctrl->slot;

/* pci_restore_state() just wrote to the Slot Control register */
ctrl->cmd_started = jiffies;
ctrl->cmd_busy = true;

/* clear spurious events from rediscovery of inserted card */
if (slot->state == ON_STATE || slot->state == BLINKINGOFF_STATE)
if (ctrl->state == ON_STATE || ctrl->state == BLINKINGOFF_STATE)
pcie_clear_hotplug_events(ctrl);

return 0;
Expand Down
Loading

0 comments on commit 5790a9c

Please sign in to comment.