From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758517AbcEFPop (ORCPT ); Fri, 6 May 2016 11:44:45 -0400 Received: from hqemgate14.nvidia.com ([216.228.121.143]:2187 "EHLO hqemgate14.nvidia.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758073AbcEFPon (ORCPT ); Fri, 6 May 2016 11:44:43 -0400 X-PGP-Universal: processed; by hqnvupgp08.nvidia.com on Fri, 06 May 2016 08:44:00 -0700 Message-ID: <572CB906.3090004@nvidia.com> Date: Fri, 6 May 2016 21:02:22 +0530 From: Laxman Dewangan User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.4.0 MIME-Version: 1.0 To: Jon Hunter , , , , CC: , , Subject: Re: [PATCH V4 3/3] soc/tegra: pmc: Add support for IO pads power state and voltage References: <1462531548-12914-1-git-send-email-ldewangan@nvidia.com> <1462531548-12914-4-git-send-email-ldewangan@nvidia.com> <572CAC20.9030307@nvidia.com> In-Reply-To: <572CAC20.9030307@nvidia.com> X-Originating-IP: [10.19.65.30] X-ClientProxiedBy: DRHKMAIL101.nvidia.com (10.25.59.15) To bgmail102.nvidia.com (10.25.59.11) Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Friday 06 May 2016 08:07 PM, Jon Hunter wrote: > On 06/05/16 11:45, Laxman Dewangan wrote: > + > + /* Last entry */ > + TEGRA_IO_PAD_MAX, > Nit should these be TEGRA_IO_PADS_xxx? Because this was name of single pad and hence I said TEGRA_IO_PAD_XXX. >> +}; >> + >> +/* tegra_io_pads_source_voltage: The voltage level of IO rails which source >> + * the IO pads. >> + */ >> +enum tegra_io_pads_source_voltage { >> + TEGRA_IO_PADS_SOURCE_VOLTAGE_1800000UV, >> + TEGRA_IO_PADS_SOURCE_VOLTAGE_3300000UV, >> +}; > Nit I wonder if we can make this shorter ... > > enum tegra_io_pads_vconf { > TEGRA_IO_PADS_VCONF_1V8, > TEGRA_IO_PADS_VCONF_3V3, This looks good but for voltage and current, unit is used uV/uV across the system. So wanted to have same unit. > >> -int tegra_io_rail_power_on(unsigned int id); >> -int tegra_io_rail_power_off(unsigned int id); >> +/* Power enable/disable of the IO pads */ >> +int tegra_io_pads_power_enable(enum tegra_io_pads id); >> +int tegra_io_pads_power_disable(enum tegra_io_pads id); >> +int tegra_io_pads_power_is_enabled(enum tegra_io_pads id); >> + >> +/* Set/get Tegra IO pads voltage config registers */ >> +int tegra_io_pads_set_voltage_config(enum tegra_io_pads id, >> + enum tegra_io_pads_source_voltage rail_uv); >> +int tegra_io_pads_get_voltage_config(enum tegra_io_pads id); > Ideally, for public function we should have kernel-doc descriptions. I think we can have different patches for describing all the public interfaces with proper arguments description. May be after this series, I can work on this. > > Otherwise looks fine. I would not rev this unless Thierry has some comments. > > Cheers > Jon