linux-kernel - Re: [PATCH V2 2/5] spmi: pmic-arb: rename pa_xx to pmic_arb

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <fb76cd144632eb1bb178550191ea9250@codeaurora.org>
Date:   Tue, 18 Jul 2017 10:22:44 +0530
From:   kgunda@...eaurora.org
To:     Stephen Boyd <sboyd@...eaurora.org>
Cc:     gregkh@...uxfoundation.org,
        Abhijeet Dharmapurikar <adharmap@...eaurora.org>,
        Christophe JAILLET <christophe.jaillet@...adoo.fr>,
        linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
        linux-arm-msm-owner@...r.kernel.org
Subject: Re: [PATCH V2 2/5] spmi: pmic-arb: rename pa_xx to pmic_arb_xx and
 other code cleanup

On 2017-07-14 23:50, Stephen Boyd wrote:
> On 07/10, Kiran Gunda wrote:
>> 
>> -static u16 pmic_arb_find_apid(struct spmi_pmic_arb *pa, u16 ppid)
>> +static u16 pmic_arb_find_apid(struct spmi_pmic_arb *pmic_arb, u16 
>> ppid)
>>  {
>> +	struct apid_data *apid_info = 
>> &pmic_arb->apid_data[pmic_arb->last_apid];
>>  	u32 regval, offset;
>> -	u16 apid;
>> -	u16 id;
>> +	u16 id, i;
>> 
>>  	/*
>> -	 * PMIC_ARB_REG_CHNL is a table in HW mapping channel to ppid.
>> +	 * PMIC_ARB_REG_APID is a table in HW mapping apid to ppid.
>>  	 * ppid_to_apid is an in-memory invert of that table.
>>  	 */
>> -	for (apid = pa->last_apid; ; apid++) {
>> -		offset = PMIC_ARB_REG_CHNL(apid);
>> -		if (offset >= pa->core_size)
>> +	for (i = pmic_arb->last_apid; ; i++, apid_info++) {
> 
> Can we keep apid instead of replacing it with i please? That will
> reduce the noise in this function.
> 
Sure. Will do that in the next patch.
>> +		offset = PMIC_ARB_REG_CHNL(i);
>> +		if (offset >= pmic_arb->core_size)
>>  			break;
>> 
>> -		regval = readl_relaxed(pa->cnfg +
>> -				      SPMI_OWNERSHIP_TABLE_REG(apid));
>> -		pa->apid_data[apid].owner = SPMI_OWNERSHIP_PERIPH2OWNER(regval);
>> +		regval = readl_relaxed(pmic_arb->cnfg +
>> +					SPMI_OWNERSHIP_TABLE_REG(i));
>> +		apid_info->owner = SPMI_OWNERSHIP_PERIPH2OWNER(regval);
>> 
>> -		regval = readl_relaxed(pa->core + offset);
>> +		regval = readl_relaxed(pmic_arb->core + offset);
>>  		if (!regval)
>>  			continue;
>> 
>>  		id = (regval >> 8) & PMIC_ARB_PPID_MASK;
>> -		pa->ppid_to_apid[id] = apid | PMIC_ARB_CHAN_VALID;
>> -		pa->apid_data[apid].ppid = id;
>> +		pmic_arb->ppid_to_apid[id] = i | PMIC_ARB_APID_VALID;
>> +		apid_info->ppid = id;
>>  		if (id == ppid) {
>> -			apid |= PMIC_ARB_CHAN_VALID;
>> +			i |= PMIC_ARB_APID_VALID;
>>  			break;
>>  		}
>>  	}
>> -	pa->last_apid = apid & ~PMIC_ARB_CHAN_VALID;
>> +	pmic_arb->last_apid = i & ~PMIC_ARB_APID_VALID;
>> 
>> -	return apid;
>> +	return i;
>>  }
> 
> Otherwise patch looks ok.