>
> On Mon, Aug 08, 2022 at 08:14:20PM +0000, Petlozu Pravareshwar wrote:
> > Add IO PAD table for tegra234 to allow configuring dpd mode and
> > switching the pins to 1.8V or 3.3V as needed.
> >
> > In tegra234, DPD registers are reorganized such that there is a
> > DPD_REQ register and a DPD_STATUS register per pad group.
> > This change accordingly updates the PMC driver.
> >
> > Signed-off-by: Petlozu Pravareshwar <petlozup@xxxxxxxxxx>
> > ---
> > drivers/soc/tegra/pmc.c | 109
> > ++++++++++++++++++++++++++++++++++++++--
> > 1 file changed, 105 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c index
> > 5611d14d3ba2..34d36a28f7d6 100644
> > --- a/drivers/soc/tegra/pmc.c
> > +++ b/drivers/soc/tegra/pmc.c
> > @@ -266,11 +266,22 @@ struct tegra_powergate {
> > struct reset_control *reset;
> > };
> >
> > +enum tegra_dpd_reg {
> > + TEGRA_PMC_IO_INVALID_DPD,
> > + TEGRA_PMC_IO_CSI_DPD,
> > + TEGRA_PMC_IO_DISP_DPD,
> > + TEGRA_PMC_IO_QSPI_DPD,
> > + TEGRA_PMC_IO_UFS_DPD,
> > + TEGRA_PMC_IO_EDP_DPD,
> > + TEGRA_PMC_IO_SDMMC1_HV_DPD,
> > +};
> > +
> > struct tegra_io_pad_soc {
> > enum tegra_io_pad id;
> > unsigned int dpd;
> > unsigned int voltage;
> > const char *name;
> > + enum tegra_dpd_reg reg_index;
> > };
> >
> > struct tegra_pmc_regs {
> > @@ -284,6 +295,8 @@ struct tegra_pmc_regs {
> > unsigned int rst_source_mask;
> > unsigned int rst_level_shift;
> > unsigned int rst_level_mask;
> > + const unsigned int *reorg_dpd_req;
> > + const unsigned int *reorg_dpd_status;
> > };
> >
> > struct tegra_wake_event {
> > @@ -364,6 +377,7 @@ struct tegra_pmc_soc {
> > bool has_blink_output;
> > bool has_usb_sleepwalk;
> > bool supports_core_domain;
> > + bool has_reorg_hw_dpd_reg_impl;
> > };
> >
> > /**
> > @@ -1546,6 +1560,14 @@ static int
> tegra_io_pad_get_dpd_register_bit(struct tegra_pmc *pmc,
> > if (pad->dpd == UINT_MAX)
> > return -ENOTSUPP;
> >
> > + if (pmc->soc->has_reorg_hw_dpd_reg_impl) {
> > + *mask = BIT(pad->dpd);
> > + *status = pmc->soc->regs->reorg_dpd_status[pad-
> >reg_index];
> > + *request = pmc->soc->regs->reorg_dpd_req[pad-
> >reg_index];
> > +
> > + goto done;
> > + }
> > +
> > *mask = BIT(pad->dpd % 32);
> >
> > if (pad->dpd < 32) {
> > @@ -1556,6 +1578,7 @@ static int
> tegra_io_pad_get_dpd_register_bit(struct tegra_pmc *pmc,
> > *request = pmc->soc->regs->dpd2_req;
> > }
> >
> > +done:
> > return 0;
> > }
> >
>
> All of this looks "bolted on". Can we not instead rework the existing register
> definitions to work with the new dpd_status and dpd_request arrays? It
> means that we'd probably need a bit of duplication of data since we would
> no longer programmatically determine the register offsets like we used to,
> but it would save the extra flag and make the code much more readable, in
> my opinion.
>
> Thierry
Yes. Agree that we can rework the patch to make the code more readable and
to save the extra flags. Will update the patch.
Thanks.
