Re: [PATCH v3 3/9] remoteproc: qcom_q6v5_mss: Add MDM9607

[barnabas.czeman@mainlining.org]

On 2026-01-01 14:13, Bryan O'Donoghue wrote:
> On 31/12/2025 16:30, Barnabás Czémán wrote:
>> From: Stephan Gerhold <stephan@gerhold.net>
>> 
>> Add support for MDM9607 MSS it have different ACC settings
>> and it needs mitigation for inrush current issue.
>> 
>> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
>> [Reword the commit, add necessary flags, rework inrush current 
>> mitigation]
>> Signed-off-by: Barnabás Czémán <barnabas.czeman@mainlining.org>
>> ---
>>   drivers/remoteproc/qcom_q6v5_mss.c | 89 
>> ++++++++++++++++++++++++++++++++------
>>   1 file changed, 75 insertions(+), 14 deletions(-)
>> 
>> diff --git a/drivers/remoteproc/qcom_q6v5_mss.c 
>> b/drivers/remoteproc/qcom_q6v5_mss.c
>> index 3c404118b322..19863c576d72 100644
>> --- a/drivers/remoteproc/qcom_q6v5_mss.c
>> +++ b/drivers/remoteproc/qcom_q6v5_mss.c
>> @@ -124,6 +124,7 @@
>>   #define QDSP6v56_CLAMP_QMC_MEM		BIT(22)
>>   #define QDSP6SS_XO_CBCR		0x0038
>>   #define QDSP6SS_ACC_OVERRIDE_VAL		0x20
>> +#define QDSP6SS_ACC_OVERRIDE_VAL_9607	0x80800000
>>   #define QDSP6v55_BHS_EN_REST_ACK	BIT(0)
>> 
>>   /* QDSP6v65 parameters */
>> @@ -256,6 +257,7 @@ struct q6v5 {
>>   };
>> 
>>   enum {
>> +	MSS_MDM9607,
>>   	MSS_MSM8226,
>>   	MSS_MSM8909,
>>   	MSS_MSM8916,
>> @@ -747,15 +749,19 @@ static int q6v5proc_reset(struct q6v5 *qproc)
>>   			return ret;
>>   		}
>>   		goto pbl_wait;
>> -	} else if (qproc->version == MSS_MSM8909 ||
>> +	} else if (qproc->version == MSS_MDM9607 ||
>> +		   qproc->version == MSS_MSM8909 ||
>>   		   qproc->version == MSS_MSM8953 ||
>>   		   qproc->version == MSS_MSM8996 ||
>>   		   qproc->version == MSS_MSM8998 ||
>>   		   qproc->version == MSS_SDM660) {
>> 
>> -		if (qproc->version != MSS_MSM8909 &&
>> -		    qproc->version != MSS_MSM8953)
>> -			/* Override the ACC value if required */
>> +		/* Override the ACC value if required */
>> +		if (qproc->version == MSS_MDM9607)
>> +			writel(QDSP6SS_ACC_OVERRIDE_VAL_9607,
>> +			       qproc->reg_base + QDSP6SS_STRAP_ACC);
>> +		else if (qproc->version != MSS_MSM8909 &&
>> +			 qproc->version != MSS_MSM8953)
>>   			writel(QDSP6SS_ACC_OVERRIDE_VAL,
>>   			       qproc->reg_base + QDSP6SS_STRAP_ACC);
>> 
>> @@ -800,7 +806,7 @@ static int q6v5proc_reset(struct q6v5 *qproc)
>>   		writel(val, qproc->reg_base + QDSP6SS_PWR_CTL_REG);
>> 
>>   		if (qproc->version != MSS_MSM8909) {
>> -			int mem_pwr_ctl;
>> +			int mem_pwr_ctl, reverse = 0;
>> 
>>   			/* Deassert QDSP6 compiler memory clamp */
>>   			val = readl(qproc->reg_base + QDSP6SS_PWR_CTL_REG);
>> @@ -812,7 +818,8 @@ static int q6v5proc_reset(struct q6v5 *qproc)
>>   			writel(val, qproc->reg_base + QDSP6SS_PWR_CTL_REG);
>> 
>>   			/* Turn on L1, L2, ETB and JU memories 1 at a time */
>> -			if (qproc->version == MSS_MSM8953 ||
>> +			if (qproc->version == MSS_MDM9607 ||
>> +			    qproc->version == MSS_MSM8953 ||
>>   			    qproc->version == MSS_MSM8996) {
>>   				mem_pwr_ctl = QDSP6SS_MEM_PWR_CTL;
>>   				i = 19;
>> @@ -822,16 +829,34 @@ static int q6v5proc_reset(struct q6v5 *qproc)
>>   				i = 28;
>>   			}
>>   			val = readl(qproc->reg_base + mem_pwr_ctl);
>> -			for (; i >= 0; i--) {
>> -				val |= BIT(i);
>> -				writel(val, qproc->reg_base + mem_pwr_ctl);
>> +			if (qproc->version == MSS_MDM9607) {
>>   				/*
>> -				 * Read back value to ensure the write is done then
>> -				 * wait for 1us for both memory peripheral and data
>> -				 * array to turn on.
>> +				 * Set first 5 bits in reverse to avoid
>> +				 * "inrush current" issues.
>>   				 */
>> -				val |= readl(qproc->reg_base + mem_pwr_ctl);
>> -				udelay(1);
>> +				reverse = 6;
>> +				for (; i >= reverse; i--) {
>> +					val |= BIT(i);
>> +					writel(val, qproc->reg_base + mem_pwr_ctl);
>> +					udelay(1);
>> +				}
>> +				for (i = 0; i < reverse; i++) {
>> +					val |= BIT(i);
>> +					writel(val, qproc->reg_base + mem_pwr_ctl);
>> +					udelay(1);
>> +				}
>> +			} else {
>> +				for (; i >= 0; i--) {
>> +					val |= BIT(i);
>> +					writel(val, qproc->reg_base + mem_pwr_ctl);
>> +					/*
>> +					 * Read back value to ensure the write is done then
>> +					 * wait for 1us for both memory peripheral and data
>> +					 * array to turn on.
>> +					 */
>> +					val |= readl(qproc->reg_base + mem_pwr_ctl);
>> +					udelay(1);
> 
> Isn't the logic here inverted ?
> 
> i.e. you've written a thing and ostensibly require a delay for that 
> thing to take effect, the power to switch on in this case.
> 
> It makes more sense to write, delay and read back rather than write, 
> readback and delay surely...
This is the original reset sequence without modification, i have just 
moved it in a else case when it is not an MDM9607, MSM8917 or MSM8937.
> 
> 
>> +				}
>>   			}
>>   		} else {
>>   			/* Turn on memories */
>> @@ -2410,6 +2435,41 @@ static const struct rproc_hexagon_res 
>> msm8996_mss = {
>>   	.version = MSS_MSM8996,
>>   };
>> 
>> +static const struct rproc_hexagon_res mdm9607_mss = {
>> +	.hexagon_mba_image = "mba.mbn",
>> +	.proxy_supply = (struct qcom_mss_reg_res[]) {
>> +		{
>> +			.supply = "pll",
>> +			.uA = 100000,
>> +		},
>> +		{}
>> +	},
>> +	.proxy_clk_names = (char*[]){
>> +		"xo",
>> +		NULL
>> +	},
>> +	.active_clk_names = (char*[]){
>> +		"iface",
>> +		"bus",
>> +		"mem",
>> +		NULL
>> +	},
>> +	.proxy_pd_names = (char*[]){
>> +		"mx",
>> +		"cx",
>> +		NULL
>> +	},
>> +	.need_mem_protection = false,
>> +	.has_alt_reset = false,
>> +	.has_mba_logs = false,
>> +	.has_spare_reg = false,
>> +	.has_qaccept_regs = false,
>> +	.has_ext_bhs_reg = false,
>> +	.has_ext_cntl_regs = false,
>> +	.has_vq6 = false,
>> +	.version = MSS_MDM9607,
>> +};
>> +
>>   static const struct rproc_hexagon_res msm8909_mss = {
>>   	.hexagon_mba_image = "mba.mbn",
>>   	.proxy_supply = (struct qcom_mss_reg_res[]) {
>> @@ -2672,6 +2732,7 @@ static const struct rproc_hexagon_res 
>> msm8926_mss = {
>> 
>>   static const struct of_device_id q6v5_of_match[] = {
>>   	{ .compatible = "qcom,q6v5-pil", .data = &msm8916_mss},
>> +	{ .compatible = "qcom,mdm9607-mss-pil", .data = &mdm9607_mss},
>>   	{ .compatible = "qcom,msm8226-mss-pil", .data = &msm8226_mss},
>>   	{ .compatible = "qcom,msm8909-mss-pil", .data = &msm8909_mss},
>>   	{ .compatible = "qcom,msm8916-mss-pil", .data = &msm8916_mss},
>> 
>> --
>> 2.52.0
>> 
>>