On 16/04/2019 12:13, Thierry Reding wrote:
> On Tue, Apr 16, 2019 at 11:29:02AM +0100, Jon Hunter wrote:
>> Commit 5f84bb1a4099 ("soc/tegra: pmc: Add sysfs entries for reset info")
>> added support for reading the Tegra reset source and level from sysfs.
>> However, there are a few issues with this commit which are ...
>> 1. The number of reset sources for Tegra210 is defined as 5 but it
>> should be 6.
>> 2. The number of reset sources for Tegra186 is defined as 13 but it
>> should be 15.
>> 3. The SoC data variables num_reset_sources and num_reset_levels are
>> defined but never used.
>>
>> Fix the above by ...
>> 1. Removing the reset source 'AOTAG' from the tegra30_reset_sources
>> because this is only applicable for Tegra210.
>> 2. Adding a new tegra210_reset_sources structure for Tegra210 reset
>> sources.
>> 3. Padding the reset sources structures with 'UNKNOWN' for any bit
>> field values that are not valid. Although it should not really be
>> necessary as these values should never be returned, it avoids
>> having to check the value read from the register.
>> 4. Removing the unused SoC data variables num_reset_sources and
>> num_reset_levels.
>>
>> Fixes: 5f84bb1a4099 ("soc/tegra: pmc: Add sysfs entries for reset info")
>> Signed-off-by: Jon Hunter <jonathanh@xxxxxxxxxx>
>> ---
>> drivers/soc/tegra/pmc.c | 32 +++++++++++++++-----------------
>> 1 file changed, 15 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
>> index 0c5f79528e5f..61c61994f17d 100644
>> --- a/drivers/soc/tegra/pmc.c
>> +++ b/drivers/soc/tegra/pmc.c
>> @@ -237,9 +237,7 @@ struct tegra_pmc_soc {
>> bool invert);
>>
>> const char * const *reset_sources;
>> - unsigned int num_reset_sources;
>> const char * const *reset_levels;
>> - unsigned int num_reset_levels;
>
> Can we keep these and make sure that we properly initialize them? That
> way we can check the bounds in the sysfs implementation to make sure we
> don't accidentally return kernel memory beyond the array if someone
> managed to inject an invalid value somehow. Or if we read a wrong value
> for whatever reason. Like you said, it should never happen, but better
> safe than sorry.
OK will do.
Jon
--
nvpublic
