[Skiboot] [PATCH 03/16] platforms/astbmc: Introduce helper function slot_init_info()

[Andrew Donnellan]

On 16/09/16 15:05, Gavin Shan wrote:
> This moves the logic initializing PCI slot to helper function
> slot_info_info() so that it can be reused by subsequent patch.

init_slot_info().

Though I'd prefer "slot_info_init()", but that's just personal preference.

> No functional changes introduced.
>
> Signed-off-by: Gavin Shan <gwshan at linux.vnet.ibm.com>

Another minor nitpick below, otherwise:

Reviewed-by: Andrew Donnellan <andrew.donnellan at au1.ibm.com>

> ---
>  platforms/astbmc/slots.c | 28 ++++++++++++++++++----------
>  1 file changed, 18 insertions(+), 10 deletions(-)
>
> diff --git a/platforms/astbmc/slots.c b/platforms/astbmc/slots.c
> index e71eb38..ff69e99 100644
> --- a/platforms/astbmc/slots.c
> +++ b/platforms/astbmc/slots.c
> @@ -115,10 +115,26 @@ static void add_slot_properties(struct pci_slot *slot,
>  			loc_code, strlen(loc_code) + 1);
>  }
>
> +static void init_slot_info(struct pci_slot *slot, bool pluggable, void *data)
> +{
> +	slot->data = data;
> +	slot->ops.add_properties = add_slot_properties;
> +
> +	slot->pluggable      = pluggable;
> +	slot->power_ctl      = false;
> +	slot->wired_lanes    = PCI_SLOT_WIRED_LANES_UNKNOWN;
> +	slot->connector_type = PCI_SLOT_CONNECTOR_PCIE_NS;
> +	slot->card_desc      = PCI_SLOT_DESC_NON_STANDARD;
> +	slot->card_mech      = PCI_SLOT_MECH_NONE;
> +	slot->power_led_ctl  = PCI_SLOT_PWR_LED_CTL_NONE;
> +	slot->attn_led_ctl   = PCI_SLOT_ATTN_LED_CTL_NONE;
> +}
> +
>  void slot_table_get_slot_info(struct phb *phb, struct pci_device *pd)
>  {
>  	const struct slot_table_entry *ent;
>  	struct pci_slot *slot;
> +	bool pluggable;
>
>  	if (!pd || pd->slot)
>  		return;
> @@ -128,15 +144,7 @@ void slot_table_get_slot_info(struct phb *phb, struct pci_device *pd)
>
>  	slot = pcie_slot_create(phb, pd);
>  	assert(slot);
> -	slot->ops.add_properties = add_slot_properties;
> -	slot->data = (void *)ent;
>
> -	slot->pluggable      = ent->etype == st_pluggable_slot;
> -	slot->power_ctl      = false;
> -	slot->wired_lanes    = PCI_SLOT_WIRED_LANES_UNKNOWN;
> -	slot->connector_type = PCI_SLOT_CONNECTOR_PCIE_NS;
> -	slot->card_desc      = PCI_SLOT_DESC_NON_STANDARD;
> -	slot->card_mech      = PCI_SLOT_MECH_NONE;
> -	slot->power_led_ctl  = PCI_SLOT_PWR_LED_CTL_NONE;
> -	slot->attn_led_ctl   = PCI_SLOT_ATTN_LED_CTL_NONE;
> +	pluggable = !!(ent->etype == st_pluggable_slot);

I don't think this requires the !! to cast to bool, a comparison 
expression should evaluate to a bool to begin with.


-- 
Andrew Donnellan              OzLabs, ADL Canberra
andrew.donnellan at au1.ibm.com  IBM Australia Limited