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

[Eddie James]

On 3/31/20 12:18 AM, Jeremy Kerr wrote:
> 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?


No, the delay is required by the FSI spec for enabling the link.


Thanks for the review!

Eddie


>
> Cheers,
>
>
> Jeremy
>