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