[PATCH linux 15/15] drivers/fsi: Initialize CFAMs for read/write access

Jeremy Kerr jk at ozlabs.org
Thu Oct 6 17:53:31 AEDT 2016


Hi Chris,

> Send break command to CFAMs to reset their logic.  If break
> fails this serves as an indication there is no CFAM present.
> Set various slave engine mode register defaults.
> 
> Add new fields to struct fsi_master to manage hardware
> workarounds for break ids, target addresses and cfam IDs.
> 
> Limit each link scan to one CFAM.  Due to limitations in
> hardware break commands only one can be recognized per link.

I'm not sure what the restriction is there. We can only send a break
over one link? Or something else?

> +/*
> + * Issue a break command to this link
> + */
> +static int fsi_master_break(struct fsi_master *master, int link)
> +{
> +	int rc;
> +	uint32_t cmd = FSI_CMD_BREAK;
> +
> +	rc = master->write(master, link, master->break_id, master->break_offset,
> +			   &cmd, sizeof(cmd));
> +	if (rc) {
> +		dev_warn(master->dev, "break to %02x:%02x failed\n",
> +			 link, master->break_id);
> +		return -ENODEV;
> +	}
> +	udelay(200);		/* wait for logic reset to take effect */
> +
> +	rc = master->read(master, link, master->break_id,
> +			  FSI_SLAVE_BASE + FSI_SMODE, &cmd, sizeof(cmd));
> +	dev_info(master->dev, "smode after break:%08x rc:%d\n", cmd, rc);
> +	if (rc) {
> +		dev_warn(master->dev, "read smode after break failed\n");
> +		return -ENODEV;
> +	}
> +
> +	return rc;
> +}

This seems very specific to one particular master implementation. What
is a GPIO-based FSI master supposed to do here? Decode the write to
specific master addresses and then perform the GPIO sequences for a
proper break?

Instead, the break should be added as an operation on a struct
fsi_master:

  struct fsi_master {
  	struct device	*dev;
  	int		idx;
  	int		n_links;
  	int		(*read)(struct fsi_master *, int link,
  				uint8_t slave, uint32_t addr,
  				void *val, size_t size);
  	int		(*write)(struct fsi_master *, int link,
  				uint8_t slave, uint32_t addr,
  				const void *val, size_t size);
+ 	int		(*break)(struct fsi_master *, int link);
  };

- then the FSI master implementations can do what's required for
sending a break. Then, the core's defintion of fsi_master_break()
becomes:

static int fsi_master_break(struct fsi_master *master, int link)
{
	int rc = 0;

	if (master->break)
		master->break(master, link)

	return rc;
}

Cheers,


Jeremy


More information about the openbmc mailing list