| Message ID | 1547798363-105371-1-git-send-email-mine260309@gmail.com |
|---|---|
| State | Under Review |
| Headers | show |
| Series | Implement OPAL_SET_SLOT_LED_STATUS opal call | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | master/apply_patch Successfully applied |
| snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot | success | Test snowpatch/job/snowpatch-skiboot on branch master |
Lei YU <mine260309@gmail.com> writes: > Add *set_led_state() in struct pci_slot_ops, and implement > OPAL_SET_SLOT_LED_STATUS opal call by checking and calling > set_led_state(). > > This function pointer is NULL by default and OPAL_UNSUPPORTED is > returned. > Each machine needs to implement it if it needs to control the device LED > on the PCIe slot. > > Signed-off-by: Lei YU <mine260309@gmail.com> > --- > core/pci-opal.c | 17 +++++++++++++++++ > core/pcie-slot.c | 3 +++ > include/pci-slot.h | 1 + > 3 files changed, 21 insertions(+) Ahh, it looks like this call has long since been defined in opal-api.h but never actually been implemented. It also appears to never have been a documented API call, so before I merge this, I need you to make an addition to doc/opal-api/ documenting the call. > diff --git a/core/pci-opal.c b/core/pci-opal.c > index a4d6eee..d05394c 100644 > --- a/core/pci-opal.c > +++ b/core/pci-opal.c > @@ -1060,3 +1060,20 @@ static int64_t opal_pci_set_pbcq_tunnel_bar(uint64_t phb_id, uint64_t addr) > return rc; > } > opal_call(OPAL_PCI_SET_PBCQ_TUNNEL_BAR, opal_pci_set_pbcq_tunnel_bar, 2); > + > +static int64_t opal_set_slot_led_status(uint64_t slot_id, uint8_t led_type, > + uint8_t led_action) > +{ > + struct pci_slot *slot = pci_slot_find(slot_id); > + int64_t rc; > + > + if (!slot) > + return OPAL_PARAMETER; > + > + if (slot->ops.set_led_state) > + rc = slot->ops.set_led_state(slot, led_type, led_action); > + else > + rc = OPAL_UNSUPPORTED; > + return rc; > +} > +opal_call(OPAL_SET_SLOT_LED_STATUS, opal_set_slot_led_status, 3); Implementation looks good. > diff --git a/core/pcie-slot.c b/core/pcie-slot.c > index 4599634..956eef2 100644 > --- a/core/pcie-slot.c > +++ b/core/pcie-slot.c > @@ -520,6 +520,9 @@ struct pci_slot *pcie_slot_create(struct phb *phb, struct pci_device *pd) > slot->ops.set_power_state = pcie_slot_set_power_state; > slot->ops.set_attention_state = pcie_slot_set_attention_state; > > + /* This is exected to be implemented by machine.c */ s/exected/expected/ Are we likely to soon have a in-tree user? I'm not too keen on keeping code around without an in-tree user.
On Fri, May 17, 2019 at 12:06 PM Stewart Smith <stewart@linux.ibm.com> wrote: > > Lei YU <mine260309@gmail.com> writes: > > Add *set_led_state() in struct pci_slot_ops, and implement > > OPAL_SET_SLOT_LED_STATUS opal call by checking and calling > > set_led_state(). > > > > This function pointer is NULL by default and OPAL_UNSUPPORTED is > > returned. > > Each machine needs to implement it if it needs to control the device LED > > on the PCIe slot. > > > > Signed-off-by: Lei YU <mine260309@gmail.com> > > --- > > core/pci-opal.c | 17 +++++++++++++++++ > > core/pcie-slot.c | 3 +++ > > include/pci-slot.h | 1 + > > 3 files changed, 21 insertions(+) > > Ahh, it looks like this call has long since been defined in opal-api.h > but never actually been implemented. > > It also appears to never have been a documented API call, so before I > merge this, I need you to make an addition to doc/opal-api/ documenting > the call. > > > diff --git a/core/pci-opal.c b/core/pci-opal.c > > index a4d6eee..d05394c 100644 > > --- a/core/pci-opal.c > > +++ b/core/pci-opal.c > > @@ -1060,3 +1060,20 @@ static int64_t opal_pci_set_pbcq_tunnel_bar(uint64_t phb_id, uint64_t addr) > > return rc; > > } > > opal_call(OPAL_PCI_SET_PBCQ_TUNNEL_BAR, opal_pci_set_pbcq_tunnel_bar, 2); > > + > > +static int64_t opal_set_slot_led_status(uint64_t slot_id, uint8_t led_type, > > + uint8_t led_action) > > +{ > > + struct pci_slot *slot = pci_slot_find(slot_id); > > + int64_t rc; > > + > > + if (!slot) > > + return OPAL_PARAMETER; > > + > > + if (slot->ops.set_led_state) > > + rc = slot->ops.set_led_state(slot, led_type, led_action); > > + else > > + rc = OPAL_UNSUPPORTED; > > + return rc; > > +} > > +opal_call(OPAL_SET_SLOT_LED_STATUS, opal_set_slot_led_status, 3); > > Implementation looks good. > > > diff --git a/core/pcie-slot.c b/core/pcie-slot.c > > index 4599634..956eef2 100644 > > --- a/core/pcie-slot.c > > +++ b/core/pcie-slot.c > > @@ -520,6 +520,9 @@ struct pci_slot *pcie_slot_create(struct phb *phb, struct pci_device *pd) > > slot->ops.set_power_state = pcie_slot_set_power_state; > > slot->ops.set_attention_state = pcie_slot_set_attention_state; > > > > + /* This is exected to be implemented by machine.c */ > > s/exected/expected/ > > Are we likely to soon have a in-tree user? I'm not too keen on keeping > code around without an in-tree user. I'll get around to hooking it up for Tuleta/ZZ at some point in the not-too distant future. > -- > Stewart Smith > OPAL Architect, IBM. > _______________________________________________ > Skiboot mailing list > Skiboot@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/skiboot
diff --git a/core/pci-opal.c b/core/pci-opal.c index a4d6eee..d05394c 100644 --- a/core/pci-opal.c +++ b/core/pci-opal.c @@ -1060,3 +1060,20 @@ static int64_t opal_pci_set_pbcq_tunnel_bar(uint64_t phb_id, uint64_t addr) return rc; } opal_call(OPAL_PCI_SET_PBCQ_TUNNEL_BAR, opal_pci_set_pbcq_tunnel_bar, 2); + +static int64_t opal_set_slot_led_status(uint64_t slot_id, uint8_t led_type, + uint8_t led_action) +{ + struct pci_slot *slot = pci_slot_find(slot_id); + int64_t rc; + + if (!slot) + return OPAL_PARAMETER; + + if (slot->ops.set_led_state) + rc = slot->ops.set_led_state(slot, led_type, led_action); + else + rc = OPAL_UNSUPPORTED; + return rc; +} +opal_call(OPAL_SET_SLOT_LED_STATUS, opal_set_slot_led_status, 3); diff --git a/core/pcie-slot.c b/core/pcie-slot.c index 4599634..956eef2 100644 --- a/core/pcie-slot.c +++ b/core/pcie-slot.c @@ -520,6 +520,9 @@ struct pci_slot *pcie_slot_create(struct phb *phb, struct pci_device *pd) slot->ops.set_power_state = pcie_slot_set_power_state; slot->ops.set_attention_state = pcie_slot_set_attention_state; + /* This is exected to be implemented by machine.c */ + slot->ops.set_led_state = NULL; + /* * State machine (SM) based reset stuff. The poll function is always * unified for all cases. diff --git a/include/pci-slot.h b/include/pci-slot.h index cd75753..2b739f0 100644 --- a/include/pci-slot.h +++ b/include/pci-slot.h @@ -102,6 +102,7 @@ struct pci_slot_ops { int64_t (*get_latch_state)(struct pci_slot *slot, uint8_t *val); int64_t (*set_power_state)(struct pci_slot *slot, uint8_t val); int64_t (*set_attention_state)(struct pci_slot *slot, uint8_t val); + int64_t (*set_led_state)(struct pci_slot *slot, uint8_t led_type, uint8_t val); /* SM based functions for reset */ void (*prepare_link_change)(struct pci_slot *slot, bool is_up);
Add *set_led_state() in struct pci_slot_ops, and implement OPAL_SET_SLOT_LED_STATUS opal call by checking and calling set_led_state(). This function pointer is NULL by default and OPAL_UNSUPPORTED is returned. Each machine needs to implement it if it needs to control the device LED on the PCIe slot. Signed-off-by: Lei YU <mine260309@gmail.com> --- core/pci-opal.c | 17 +++++++++++++++++ core/pcie-slot.c | 3 +++ include/pci-slot.h | 1 + 3 files changed, 21 insertions(+)