Re: [PATCH v3] PCI hotplug: rpaphp: Error out on busy status from get-sensor-state

[Nathan Lynch <nathanl@linux.ibm.com>]

Mahesh Salgaonkar <mahesh@linux.ibm.com> writes:
> To avoid this issue, fix the pci hotplug driver (rpaphp) to return an error
> if the slot presence state can not be detected immediately. Current
> implementation uses rtas_get_sensor() API which blocks the slot check state
> until rtas call returns success. Change rpaphp_get_sensor_state() to invoke
> rtas_call(get-sensor-state) directly and take actions based on rtas return
> status. This patch now errors out immediately on busy return status from
> rtas_call.
>
> Please note that, only on certain PHB failures, the slot presence check
> returns BUSY condition. In normal cases it returns immediately with a
> correct presence state value. Hence this change has no impact on normal pci
> dlpar operations.

I was wondering about this. This seems to be saying -2/990x cannot
happen in other cases. I couldn't find this specified in the
architecture. It seems a bit risky to me to *always* error out on
-2/990x - won't we have intermittent slot enable failures?

> +/*
> + * RTAS call get-sensor-state(DR_ENTITY_SENSE) return values as per PAPR:
> + *    -1: Hardware Error
> + *    -2: RTAS_BUSY
> + *    -3: Invalid sensor. RTAS Parameter Error.
> + * -9000: Need DR entity to be powered up and unisolated before RTAS call
> + * -9001: Need DR entity to be powered up, but not unisolated, before RTAS call
> + * -9002: DR entity unusable
> + *  990x: Extended delay - where x is a number in the range of 0-5
> + */
> +#define RTAS_HARDWARE_ERROR	-1
> +#define RTAS_INVALID_SENSOR	-3
> +#define SLOT_UNISOLATED		-9000
> +#define SLOT_NOT_UNISOLATED	-9001
> +#define SLOT_NOT_USABLE		-9002
> +
> +static int rtas_to_errno(int rtas_rc)
> +{
> +	int rc;
> +
> +	switch (rtas_rc) {
> +	case RTAS_HARDWARE_ERROR:
> +		rc = -EIO;
> +		break;
> +	case RTAS_INVALID_SENSOR:
> +		rc = -EINVAL;
> +		break;
> +	case SLOT_UNISOLATED:
> +	case SLOT_NOT_UNISOLATED:
> +		rc = -EFAULT;
> +		break;
> +	case SLOT_NOT_USABLE:
> +		rc = -ENODEV;
> +		break;
> +	case RTAS_BUSY:
> +	case RTAS_EXTENDED_DELAY_MIN...RTAS_EXTENDED_DELAY_MAX:
> +		rc = -EBUSY;
> +		break;
> +	default:
> +		err("%s: unexpected RTAS error %d\n", __func__, rtas_rc);
> +		rc = -ERANGE;
> +		break;
> +	}
> +	return rc;
> +}

These conversions look OK to me.