[Skiboot] [PATCH 04/16] [PATCH 04/16] opencapi5: rainier detect device

[Frederic Barrat]

On 20/08/2021 11:45, Christophe Lombard wrote:
> Update the platform_ocapi structure to store Rainier platform-specific
> values for detecting and resetting OpenCAPI devices via the module
> I2C (PCA9553)
> The unique number I2C bus ID associated to each OpenCapi device
> is get from the I2C port and engine.
> (De)Assert a reset and detect an OpenCapi device is available through
> the I2C bus id and address.
> 
> Signed-off-by: Christophe Lombard <clombard at linux.vnet.ibm.com>
> ---

> diff --git a/platforms/astbmc/rainier.c b/platforms/astbmc/rainier.c
> index 17d9fe2b..b9fd53b8 100644
> --- a/platforms/astbmc/rainier.c
> +++ b/platforms/astbmc/rainier.c

> +static void rainier_i2c_presence_init(struct pau_dev *dev)
> +{
> +	char port_name[17];
> +	struct dt_node *np;
> +	int engine, port;
> +
> +	/* Find I2C port */
> +	if (dev->i2c_bus_id)
> +		return;
> +
> +	if (get_i2c_info(dev, &engine, &port))
> +		return;
> +
> +	snprintf(port_name, sizeof(port_name), "p8_%08x_e%dp%d",
> +		 dev->pau->chip_id, engine, port);
> +
> +	dt_for_each_compatible(dt_root, np, "ibm,power10-i2c-port") {
> +		if (streq(port_name, dt_prop_get(np, "ibm,port-name"))) {
> +			dev->i2c_bus_id = dt_prop_get_u32(np, "ibm,opal-id");
> +			break;
> +		}
> +	}


HDAT doesn't create any device tree entries for the I2C bus for engine 
1. So detection doesn't work if we don't add it explicitly (which we do 
only in the BML/generic plafform patch, which is not part of this series).



> +static void rainier_pau_device_detect(struct pau *pau)
> +{
> +	struct pau_dev *dev;
> +	uint8_t detect;
> +	int64_t rc;
> +
> +	/* OpenCapi devices are possibly connected on Optical link pair:
> +	 * OP0 or OP3
> +	 * pau_index	Interface Link - OPxA/B
> +	 * 0		OPT0 -- PAU0
> +	 *		OPT1 -- no PAU, SMP only
> +	 *		OPT2 -- no PAU, SMP only
> +	 * 1		OPT3 -- PAU3
> +	 * 2		OPT4 -- PAU4 by default, but can be muxed to use PAU5 - N/A on Rainier
> +	 * 3		OPT5 -- PAU5 by default, but can be muxed to use PAU4 - N/A on Rainier
> +	 * 4		OPT6 -- PAU6 by default, but can be muxed to use PAU7 - N/A on Rainier
> +	 * 5		OPT7 -- PAU7 by default, but can be muxed to use PAU6 - N/A on Rainier
> +	 */
> +	pau_for_each_dev(dev, pau) {
> +		dev->type = PAU_DEV_TYPE_UNKNOWN;
> +
> +		rainier_i2c_presence_init(dev);
> +		if (dev->i2c_bus_id) {
> +			rc = rainier_i2c_dev_detect(dev, &detect);
> +			/* LED0 (bit 0): a high level no card is plugged */
> +			if (!rc && !(detect & platform.ocapi->i2c_predetect_pin))
> +				dev->type = PAU_DEV_TYPE_OPENCAPI;


It would be more readable if "detect & 
platform.ocapi->i2c_predetect_pin" was in rainier_i2c_dev_detect()


> +		}
> +
> +		dt_add_property_u64(dev->dn, "ibm,link-speed", 25000000000ull);
> +	}
> +}
> +
>   static void rainier_init(void)
>   {
> +


stray new line


>   	astbmc_init();
>   	rainier_init_slot_power();
>   }
> @@ -121,6 +292,14 @@ static bool rainier_probe(void)
>   	return true;
>   }
> 
> +static struct platform_ocapi rainier_ocapi = {
> +	.i2c_dev_addr		= 0x62, /* C4 >> 1 */
> +	.i2c_intreset_pin	= 0x02, /* PIN 2 - LED1 - INT/RESET */
> +	.i2c_predetect_pin	= 0x01, /* PIN 1 - LED0 - PRE-DETECT */
> +	.i2c_assert_reset	= rainier_i2c_assert_reset,
> +	.i2c_deassert_reset	= rainier_i2c_deassert_reset,
> +};
> +


It looks a bit strange because we pass the detection arguments in 
platform->ocapi, yet the reset and detect routines are in the rainier 
file. So we don't need that level of indirection for the arguments. It's 
easy to know where that is coming from and the reset/detect code could 
be moved to a more "generic" place if we ever look at another platform. 
So maybe we should have a comment saying that much.

   Fred