| Message ID | 20181213093438.29621-10-josephl@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Tegra210 DFLL support | expand |
On 13/12/2018 09:34, Joseph Lo wrote: > The DFLL hardware supports both I2C and PWM based regulator. SW driver > only touches I2C regulator when generating LUT. And shouldn't touch it > anymore once the DFLL is enabled. I am not sure that the last two sentences are above are relevant and confused me a little at first. I would be tempted to drop them. > This patch adds the protection for the APIs that only work with I2C mode > to avoid they could be called accidentally. > > Signed-off-by: Joseph Lo <josephl@nvidia.com> Furthermore, I would be tempted to squash this into patch #7. I am not sure another patch is warranted here. Cheers Jon
On 12/13/18 8:46 PM, Jon Hunter wrote: > > On 13/12/2018 09:34, Joseph Lo wrote: >> The DFLL hardware supports both I2C and PWM based regulator. SW driver >> only touches I2C regulator when generating LUT. And shouldn't touch it >> anymore once the DFLL is enabled. > > I am not sure that the last two sentences are above are relevant and > confused me a little at first. I would be tempted to drop them. Indeed, they are irrelevant. Just want to describe that once we created LUT table, it means we cached the regulator output table in driver. Then we don't need to query voltage data from regulator again. This is specific to the I2C mode only and happens in driver initialization time. Which means the two APIs we add the WARN here maybe not really necessary. Because this is suggested by Peter. Hi Peter, Just want to double confirm again, do we really need to add a WARN here? Since we don't and shouldn't access these two APIs once the driver is working, all the voltage query should be via LUT. So I think add WARN here is not really necessary. Thanks, Joseph > >> This patch adds the protection for the APIs that only work with I2C mode >> to avoid they could be called accidentally. >> >> Signed-off-by: Joseph Lo <josephl@nvidia.com> > > Furthermore, I would be tempted to squash this into patch #7. I am not > sure another patch is warranted here. > > Cheers > Jon >
On Fri, Dec 14, 2018 at 03:42:45PM +0800, Joseph Lo wrote: > On 12/13/18 8:46 PM, Jon Hunter wrote: > > > > On 13/12/2018 09:34, Joseph Lo wrote: > > > The DFLL hardware supports both I2C and PWM based regulator. SW driver > > > only touches I2C regulator when generating LUT. And shouldn't touch it > > > anymore once the DFLL is enabled. > > > > I am not sure that the last two sentences are above are relevant and > > confused me a little at first. I would be tempted to drop them. > > Indeed, they are irrelevant. Just want to describe that once we created LUT > table, it means we cached the regulator output table in driver. Then we > don't need to query voltage data from regulator again. This is specific to > the I2C mode only and happens in driver initialization time. Which means the > two APIs we add the WARN here maybe not really necessary. Because this is > suggested by Peter. > > Hi Peter, > > Just want to double confirm again, do we really need to add a WARN here? > Since we don't and shouldn't access these two APIs once the driver is > working, all the voltage query should be via LUT. So I think add WARN here > is not really necessary. It's not really necessary. It's more of a safeguard to make sure someone who modifies the code later and might not be aware of this limitation would accidently call the functions when PWM mode is in use. Peter.
diff --git a/drivers/clk/tegra/clk-dfll.c b/drivers/clk/tegra/clk-dfll.c index b3668073d9b4..93cc86f17f7b 100644 --- a/drivers/clk/tegra/clk-dfll.c +++ b/drivers/clk/tegra/clk-dfll.c @@ -1534,6 +1534,9 @@ static int find_vdd_map_entry_exact(struct tegra_dfll *td, int uV) { int i, n_voltages, reg_volt_id, align_step; + if (WARN_ON(td->pmu_if == TEGRA_DFLL_PMU_PWM)) + return -EINVAL; + align_step = uV / td->soc->alignment.step_uv; n_voltages = regulator_count_voltages(td->vdd_reg); for (i = 0; i < n_voltages; i++) { @@ -1558,6 +1561,9 @@ static int find_vdd_map_entry_min(struct tegra_dfll *td, int uV) { int i, n_voltages, reg_volt_id, align_step; + if (WARN_ON(td->pmu_if == TEGRA_DFLL_PMU_PWM)) + return -EINVAL; + align_step = uV / td->soc->alignment.step_uv; n_voltages = regulator_count_voltages(td->vdd_reg); for (i = 0; i < n_voltages; i++) {
The DFLL hardware supports both I2C and PWM based regulator. SW driver only touches I2C regulator when generating LUT. And shouldn't touch it anymore once the DFLL is enabled. This patch adds the protection for the APIs that only work with I2C mode to avoid they could be called accidentally. Signed-off-by: Joseph Lo <josephl@nvidia.com> --- *V2: - new added patch in V2 --- drivers/clk/tegra/clk-dfll.c | 6 ++++++ 1 file changed, 6 insertions(+)