Re: [PATCH v2 2/3] soundwire: qcom: add multi-master support

[Pierre-Louis Bossart]

On 10/1/26 13:06, Srinivas Kandagatla wrote:
> 
> 
> On 10/1/26 11:53 AM, Pierre-Louis Bossart wrote:
>>
>>> @@ -192,6 +194,13 @@ struct qcom_swrm_ctrl {
>>>  	const unsigned int *reg_layout;
>>>  	void __iomem *mmio;
>>>  	struct reset_control *audio_cgcr;
>>> +	u8 num_lanes;
>>> +	bool is_dependent;
>>> +	struct qcom_swrm_ctrl *peer_ctrl;
>>> +	u8 peer_first_lane;
>>> +	u8 num_peer_lanes;
>>> +	bool is_primary;
>>> +	u32 peer_dpn_offset;
>>
>> nit-pick: do you need both is_dependent and is_primary? One would think
>> that a single variable would be enough, no?
> I did try to do that which made the code bit confusing to read.

ok

>> also without context it's hard to understand if lanes are independent in
>> terms of transport or not, not sure what 'peer' means in a multi-master
>> setup with a primary controller and N dependent ones? Code like this
> For now its one, but Yes, technically we could have more dependent ones,
> I wanted to keep the patchset simple and right now the only have one
> peer and i dont know if we have other setups with more dependent
> controrllers.

even with 2 controllers, the 'peer' naming conveys a concept of equal
status/rank/ability, which isn't quite the case, is it?

>> function below is a bit cryptic...
>>
>>> +static struct qcom_swrm_ctrl *
>>> +qcom_swrm_port_target(struct qcom_swrm_ctrl *ctrl, u8 port_num, u32 *reg)
>>> +{
>>> +	u8 lane = ctrl->pconfig[port_num].lane_control;
>>> +
>>> +	if (ctrl->peer_ctrl && lane >= ctrl->peer_first_lane &&
>>> +	    lane < ctrl->peer_first_lane + ctrl->num_peer_lanes) {

and what I meant here is that it looks like there's a special
distribution of traffic between lanes?

This is not really well-documented in the MIPI spec. On the Intel side
traffic can be moved at will between lanes, but it's not a requirement.
There's also no MIPI guidance on how lanes are used e.g.
a) a new lane is used only when bandwidth is exhausted on existing lanes or
b) all lanes are used and clock is scaled according to bandwidth needs.

>>> +		*reg -= ctrl->peer_dpn_offset;
>>> +		return ctrl->peer_ctrl;
>>> +	}
>>> +	return ctrl;
>>> +}
>>
>>
>