On Wednesday 13 April 2016 02:55 PM, Jon Hunter wrote:
On 13/04/16 10:00, Laxman Dewangan wrote:
On Wednesday 13 April 2016 02:17 PM, Jon Hunter wrote:
On 12/04/16 15:56, Laxman Dewangan wrote:
NVIDIA Tegra210 supports some of the IO interface which can operate
at 1.8V or 3.3V I/O rail voltage levels. SW needs to configure
Tegra PMC register to set different voltage level of IO interface based
on IO rail voltage from power supply i.e. power regulators.
Add APIs to set and get IO rail voltage from the client driver.
I think that we need some further explanation about the scenario when
this is used. In other words, why this configuration needs to be done in
the kernel versus the bootloader. Is this something that can change at
runtime? I could see that for SD cards it may.
Yes, SDIO3.0 support needs dynamic IO rail voltage change and so pad
voltage change.
#define GPU_RG_CNTRL 0x2d4
+static DEFINE_SPINLOCK(tegra_pmc_access_lock);
+
We already have a mutex for managing concurrent accesses, do we need
this?
Mutex is sleeping calls and we really dont need this. This is sleep for
small duration and we should do this in spinlock.
Yes but do you need to call it from a interrupt context? It seems that
these are not called very often, may be on boot, or when swapping an SD
card, and so although a spinlock would be faster, the overhead of the
mutex would be negligible in this case. I think that you need to justify
why this needs to be a spinlock with a use-case that requires it.
This is just based on my OS theory that if critical region is taking
less time, in order of us instead of ms then better to use spin lock
instead of mutex lock.
+
+static struct tegra_io_rail_voltage_bit_info
tegra210_io_rail_voltage_info[] = {
+ TEGRA_IO_RAIL_VOLTAGE(SDMMC1, 12),
+ TEGRA_IO_RAIL_VOLTAGE(SDMMC3, 13),
+ TEGRA_IO_RAIL_VOLTAGE(AUDIO_HV, 18),
+ TEGRA_IO_RAIL_VOLTAGE(DMIC, 20),
+ TEGRA_IO_RAIL_VOLTAGE(GPIO, 21),
+ TEGRA_IO_RAIL_VOLTAGE(SPI_HV, 23),
+};
+
You could simply this by having a look-up table similar to what we do
for the powergates.
Revising the power gate code, it needs ID matches with bit location but
it is not the case here. We need to have lookup from ID to bit position.
I still don't see why you could not have ...
static unsigned int tegra210_io_rail_voltage_bit[] = {
[TEGRA_IO_RAIL_SDMMC1] = 12,
...
}
You could avoid the for-loop in the lookup as well as all the extra
definitions. Seems a lot simpler.
This makes the table in larger size, max index is maximum of all the
macros used in LHS.
Also if we have 0 as valid (which is not there now) then it can be trouble.
The TRM needs to be update. There is no LATCH register in the T210.
PMC_PWR_DET and PMC_PWR_DET_VAL are registers for this. I have internal
tracking bug for correcting this.
Why do you need to program both? I think that we should be clear here
about the procedure. If the TRM is wrong, then there should be at least
a comment here describing the correct sequence.
OK, will mention the details.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
