linux-kernel - Re: [PATCH V6 3/3] soc/tegra: pmc: Add support for IO pads power state and voltage

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] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 20 May 2016 19:04:18 +0530
From:	Laxman Dewangan <ldewangan@...dia.com>
To:	Jon Hunter <jonathanh@...dia.com>, <thierry.reding@...il.com>,
	<airlied@...ux.ie>, <swarren@...dotorg.org>
CC:	<gnurou@...il.com>, <dri-devel@...ts.freedesktop.org>,
	<linux-tegra@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH V6 3/3] soc/tegra: pmc: Add support for IO pads power
 state and voltage


On Friday 20 May 2016 07:02 PM, Jon Hunter wrote:
> On 20/05/16 12:59, Laxman Dewangan wrote:
>> +/* tegra_io_pads_config_info: Tegra IO pads bit config info.
>> + * @dpd_config_bit: DPD configuration bit position. -1 if not supported.
>> + * @voltage_config_bit: Voltage configuration bit position. -1 if not supported.
>> + * @soc_mask: Bitwise OR of SoC masks if IO pads supported on that SoC.
>> + */
> Comment coding style :-(

I saw this style multiple places and so intentionally left here.
If comment is inside the code then
/*
  * first-line comment
  * second line
  */

but for function, it can have in single line.

Anyhow, I will correct in next cycle.


>
>> +static inline int tegra_io_pads_to_dpd_bit(const struct tegra_pmc_soc *soc,
>> +					   enum tegra_io_pads id)
>>   {
>> -	unsigned long rate, value;
>> +	if (tegra_io_pads_configs[id].soc_mask & soc->io_pads_soc_mask)
>> +		return tegra_io_pads_configs[id].dpd_config_bit;
> I realise now that we are not checking if 'id' is greater than
> TEGRA_IO_PADS_MAX anywhere. This should probably be handled here.

Do we need to check? Our parameter type is enum type and hence it is not 
expected to have outside of the MAX.
I think it will be unnecessarily check here.


>>   
>>
>> +	int ret;
>> +
>> +	ret = tegra_io_pads_to_dpd_bit(pmc->soc, id);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	*bit = ret % 32;
>> +
>> +	if (*bit < 32) {
> Isn't bit always less than 32 here now?
>
Yaah this is bug. should be if (ret < 32)

My testcase has the pad name whose dpd bit is < 32 and hence did not 
catch it..

BTW, do you have the T124 based platform for SOR testing? I have T210 
platforms where I am testing.