Re: [PATCH V4 3/3] soc/tegra: pmc: Add support for IO pads power state and voltage

[Laxman Dewangan]

On Sunday 08 May 2016 05:43 PM, Jon Hunter wrote:
> On 06/05/16 16:32, Laxman Dewangan wrote:
> > 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.
> Aren't these used to set the voltage level and power state for the
> entire group of IOs? Confused :-(

One IO pad can have multiple IO pins.
IO Pad control the power state and voltage of all pins belongs to that 
IO pad.


Now what should we say PADS or PAD here? TEGRA_IO_PAD_UART or 
TEGRA_IO_PADS_UART?


> > > > +};
> > > > +
> > > > +/* 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.
> Now it is an enum does it matter? Or maybe just have ...
>
> enum tegra_io_pads_vconf {
> 	TEGRA_IO_PADS_1800000UV,
> 	TEGRA_IO_PADS_3300000UV,
> };

OK, TEGRA_IO_PADS_VCONF_1800000UV and TEGRA_IO_PADS_VCONF_3300000UV.
Fine?