[PATCH linux dev-4.7 v3] drivers/fsi: Add hub master support

[Jeremy Kerr]

Hi Chris,

> Scan for links off of the mFSI master on CFAM. Add any devices
> detected to the existing list of FSI devices and notify any
> clients.

Looking pretty good now, but there are still some changes required in
the new code:

> @@ -242,12 +404,50 @@ static int fsi_slave_scan(struct fsi_slave *slave)
>  		type = (conf & FSI_SLAVE_CONF_TYPE_MASK)
>  			>> FSI_SLAVE_CONF_TYPE_SHIFT;
>  
> -		/*
> -		 * Unused address areas are marked by a zero type value; this
> -		 * skips the defined address areas
> -		 */
> -		if (type != 0 && slots != 0) {
> +		switch (type) {
> +		case 0:
> +			/*
> +			 * Unused address areas are marked by a zero type
> +			 * value; this skips the defined address areas
> +			 */
> +			break;
> +
> +		case FSI_ENGID_HUB_MASTER:
> +			hub_master_class = class_create(THIS_MODULE,
> +								"hub-master");

Why are you creating a hub-specific class, and why are you doing that
here?

If we want to use classes (which we may want to at some point), we
should have a fsi-master class, rather than one only for hub masters.
And then one for the slave engines too.

However, this is not the place to do this. This will cause a WARN_ON on
the second hub we find.

I assume this is simply so that you can use device_create(). However,
since we're not using classes at present, just create the struct device
in the same way we have in other places in this file:

			hub->master.dev = kzalloc(sizeof(*hub->master.dev),
						GFP_KERNEL);
			if (!hub->master.dev)
				return -ENODEV;

			device_initialize(hub->master.dev);

Then, you want to use an appropriate name (just using a number is a bit
obscure), and set the parent too:

			dev_set_name(hub->master.dev, "hub@%02x",
						hub->master.idx);
			hub->master.dev->parent = &slave->dev;

... and we probably want a dev->release function too, to free the
device.

> +		case FSI_ENGID_HUB_LINK:
> +			conf_link_count++;
> +
> +			break;
> +
> +		default:
>  			/* create device */

We've lost the (slots != 0) check from this condition.

Regards,


Jeremy