lkml.org 
[lkml]   [2016]   [May]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH V4 3/3] soc/tegra: pmc: Add support for IO pads power state and voltage

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

\
 
 \ /
  Last update: 2016-05-06 18:01    [W:0.066 / U:0.124 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site