Implement OPAL_SET_SLOT_LED_STATUS opal call
diff mbox series

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
Related show

Checks

Context Check Description
snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot success Test snowpatch/job/snowpatch-skiboot on branch master
snowpatch_ozlabs/apply_patch success master/apply_patch Successfully applied

Commit Message

Lei YU Jan. 18, 2019, 7:59 a.m. UTC
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(+)

Comments

Stewart Smith May 17, 2019, 2:05 a.m. UTC | #1
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.
Oliver O'Halloran May 17, 2019, 2:10 a.m. UTC | #2
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

Patch
diff mbox series

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);