[PATCH linux dev-5.4 v2] fsi: master: Add link_disable function

Jeremy Kerr jk at ozlabs.org
Tue Mar 31 16:18:31 AEDT 2020


Hi Eddie,

> The master driver should disable links that don't have slaves or links
> that fail to be accessed. To do this, add a link_disable function and
> use it in the failure path for slave break and init.

Concept looks good, but would it work to merge the disable
functionality with the existing link_enable callback?

ie., something like:

--- a/drivers/fsi/fsi-master.h
+++ b/drivers/fsi/fsi-master.h
@@ -130,6 +130,7 @@ struct fsi_master {
 				uint32_t addr, const void *val, size_t size);
 	int		(*term)(struct fsi_master *, int link, uint8_t id);
 	int		(*send_break)(struct fsi_master *, int link);
-  	int		(*link_enable)(struct fsi_master *, int link);
+  	int		(*link_enable)(struct fsi_master *, int link, bool enable);
 	int		(*link_config)(struct fsi_master *, int link,
 				       u8 t_send_delay, u8 t_echo_delay);

(and add the bool to the core api too).

- as I suspect the logic between enable and disable will be much the
same, aside from the actual value set.

Along those lines:

> --- a/drivers/fsi/fsi-master-aspeed.c
> +++ b/drivers/fsi/fsi-master-aspeed.c
> @@ -299,6 +299,20 @@ static int aspeed_master_write(struct fsi_master *master, int link,
>  	return 0;
>  }
>  
> +static int aspeed_master_link_disable(struct fsi_master *master, int link)
> +{
> +	struct fsi_master_aspeed *aspeed = to_fsi_master_aspeed(master);
> +	int idx, bit;
> +	__be32 reg;
> +
> +	idx = link / 32;
> +	bit = link % 32;
> +
> +	reg = cpu_to_be32(0x80000000 >> bit);
> +
> +	return opb_writel(aspeed, ctrl_base + FSI_MCENP0 + (4 * idx), reg);
> +}

If we don't need the delay and read-back here, should we drop it from
enable too?

Cheers,


Jeremy



More information about the openbmc mailing list