linux-kernel - Re: [PATCH 2/6] soc/tegra: pmc: Fix early initialisation of PMC

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20160630100306.GE1776@ulmo.ba.sec>
Date:	Thu, 30 Jun 2016 12:03:06 +0200
From:	Thierry Reding <thierry.reding@...il.com>
To:	Jon Hunter <jonathanh@...dia.com>
Cc:	Stephen Warren <swarren@...dotorg.org>,
	Alexandre Courbot <gnurou@...il.com>,
	linux-tegra@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/6] soc/tegra: pmc: Fix early initialisation of PMC

On Wed, Jun 29, 2016 at 05:17:09PM +0100, Jon Hunter wrote:
> 
> On 28/06/16 11:38, Jon Hunter wrote:
> > During early initialisation, the available power partitions for a given
> > device is configured as well as the polarity of the PMC interrupt. Both
> > of which should only be configured if there is a valid device node for
> > the PMC device. This is because the soc data used for configuring the
> > power partitions is only available if a device node for the PMC is found
> > and the code to configure the interrupt polarity uses the device node
> > pointer directly.
> > 
> > Some early device-tree images may not have this device node and so fix
> > this by ensuring the device node pointer is valid when configuring these
> > items.
> > 
> > Signed-off-by: Jon Hunter <jonathanh@...dia.com>
> > ---
> >  drivers/soc/tegra/pmc.c | 34 ++++++++++++++++++----------------
> >  1 file changed, 18 insertions(+), 16 deletions(-)
> > 
> > diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
> > index 52a9e9703668..2e031c4ad547 100644
> > --- a/drivers/soc/tegra/pmc.c
> > +++ b/drivers/soc/tegra/pmc.c
> > @@ -1550,27 +1550,29 @@ static int __init tegra_pmc_early_init(void)
> >  		return -ENXIO;
> >  	}
> >  
> > -	/* Create a bit-map of the available and valid partitions */
> > -	for (i = 0; i < pmc->soc->num_powergates; i++)
> > -		if (pmc->soc->powergates[i])
> > -			set_bit(i, pmc->powergates_available);
> > -
> >  	mutex_init(&pmc->powergates_lock);
> >  
> > -	/*
> > -	 * Invert the interrupt polarity if a PMC device tree node exists and
> > -	 * contains the nvidia,invert-interrupt property.
> > -	 */
> > -	invert = of_property_read_bool(np, "nvidia,invert-interrupt");
> > +	if (np) {
> > +		/* Create a bit-map of the available and valid partitions */
> > +		for (i = 0; i < pmc->soc->num_powergates; i++)
> > +			if (pmc->soc->powergates[i])
> > +				set_bit(i, pmc->powergates_available);
> >  
> > -	value = tegra_pmc_readl(PMC_CNTRL);
> > +		/*
> > +		 * Invert the interrupt polarity if a PMC device tree node
> > +		 * exists and contains the nvidia,invert-interrupt property.
> > +		 */
> > +		invert = of_property_read_bool(np, "nvidia,invert-interrupt");
> >  
> > -	if (invert)
> > -		value |= PMC_CNTRL_INTR_POLARITY;
> > -	else
> > -		value &= ~PMC_CNTRL_INTR_POLARITY;
> > +		value = tegra_pmc_readl(PMC_CNTRL);
> >  
> > -	tegra_pmc_writel(value, PMC_CNTRL);
> > +		if (invert)
> > +			value |= PMC_CNTRL_INTR_POLARITY;
> > +		else
> > +			value &= ~PMC_CNTRL_INTR_POLARITY;
> > +
> > +		tegra_pmc_writel(value, PMC_CNTRL);
> > +	}
> >  
> >  	return 0;
> >  }
> 
> By the way, the more I think about this, if there is no valid 'pmc'
> node in the device-tree blob, then I wonder if we should even bother
> mapping the pmc address space at all? The reason being, if there is
> no 'pmc' node then we cannot look-up the SoC data and so all the
> public PMC APIs are pretty useless AFAICT. I wonder if we should do
> something like ...

I think it's fine as-is. The PMC driver does more than just powergates
and it'd be useful to have the rest continue to work even on old DTBs.
Everything should already be properly guarded against this exception.

Thierry

Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)