From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932335AbcETOGi (ORCPT ); Fri, 20 May 2016 10:06:38 -0400 Received: from hqemgate15.nvidia.com ([216.228.121.64]:8906 "EHLO hqemgate15.nvidia.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751937AbcETOGh (ORCPT ); Fri, 20 May 2016 10:06:37 -0400 X-PGP-Universal: processed; by hqnvupgp08.nvidia.com on Fri, 20 May 2016 07:05:16 -0700 Subject: Re: [PATCH V6 3/3] soc/tegra: pmc: Add support for IO pads power state and voltage To: Laxman Dewangan , , , References: <1463745564-19297-1-git-send-email-ldewangan@nvidia.com> <1463745564-19297-4-git-send-email-ldewangan@nvidia.com> <573F11F5.7010702@nvidia.com> <573F125A.20102@nvidia.com> CC: , , , From: Jon Hunter Message-ID: <573F19E6.8000507@nvidia.com> Date: Fri, 20 May 2016 15:06:30 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.2 MIME-Version: 1.0 In-Reply-To: <573F125A.20102@nvidia.com> X-Originating-IP: [10.21.132.103] X-ClientProxiedBy: UKMAIL101.nvidia.com (10.26.138.13) To UKMAIL102.nvidia.com (10.26.138.15) Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 20/05/16 14:34, Laxman Dewangan wrote: > > 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. The enum does not prevent someone from passing a large number so I think we should check. >>> >>> + 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. Yes, I should be able to test on the t124-nyan-big. Cheers Jon -- nvpublic