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

[Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com>]

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.


> 
> 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.
--srini

> 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) {
>> +		*reg -= ctrl->peer_dpn_offset;
>> +		return ctrl->peer_ctrl;
>> +	}
>> +	return ctrl;
>> +}
> 
>