linux-kernel - RE: [PATCH v2] soc/tegra: pmc: Add IO Pad table for tegra234

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <DM6PR12MB3164D87DECA91CBBC4803118B5739@DM6PR12MB3164.namprd12.prod.outlook.com>
Date:   Wed, 24 Aug 2022 18:21:39 +0000
From:   Petlozu Pravareshwar <petlozup@...dia.com>
To:     Thierry Reding <thierry.reding@...il.com>
CC:     Jonathan Hunter <jonathanh@...dia.com>,
        "p.zabel@...gutronix.de" <p.zabel@...gutronix.de>,
        "dmitry.osipenko@...labora.com" <dmitry.osipenko@...labora.com>,
        "ulf.hansson@...aro.org" <ulf.hansson@...aro.org>,
        "Kartik ." <kkartik@...dia.com>,
        "cai.huoqing@...ux.dev" <cai.huoqing@...ux.dev>,
        Sandipan Patra <spatra@...dia.com>,
        "linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v2] soc/tegra: pmc: Add IO Pad table for tegra234

> 
> 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@...dia.com>
> > ---
> >  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.