[v3,4/4] soc/tegra: pmc: apply MBIST work around fo Tegra210

Message ID 1516699369-3513-5-git-send-email-pdeschrijver@nvidia.com
State Superseded
Headers show
Series
  • MBIST work around (WAR) for Tegra210
Related show

Commit Message

Peter De Schrijver Jan. 23, 2018, 9:22 a.m.
Apply the memory built-in self test work around when ungating certain
Tegra210 power domains.

Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
---
 drivers/soc/tegra/pmc.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Jon Hunter Jan. 24, 2018, 10:43 p.m. | #1
On 23/01/18 09:22, Peter De Schrijver wrote:
> Apply the memory built-in self test work around when ungating certain
> Tegra210 power domains.

Nit-pick .. typo in $subject.

> Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
> ---
>  drivers/soc/tegra/pmc.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
> index ce62a47..c4eff4b 100644
> --- a/drivers/soc/tegra/pmc.c
> +++ b/drivers/soc/tegra/pmc.c
> @@ -153,6 +153,7 @@ struct tegra_pmc_soc {
>  
>  	bool has_tsense_reset;
>  	bool has_gpu_clamps;
> +	bool needs_mbist_war;
>  
>  	const struct tegra_io_pad_soc *io_pads;
>  	unsigned int num_io_pads;
> @@ -431,6 +432,9 @@ static int tegra_powergate_power_up(struct tegra_powergate *pg,
>  
>  	usleep_range(10, 20);
>  
> +	if (pg->pmc->soc->needs_mbist_war)
> +		tegra210_clk_handle_mbist_war(pg->id);
> +

Be good if we could return an error from the above function.

>  	if (disable_clocks)
>  		tegra_powergate_disable_clocks(pg);
>  
> @@ -1815,6 +1819,7 @@ static void tegra20_pmc_setup_irq_polarity(struct tegra_pmc *pmc,
>  	.cpu_powergates = tegra210_cpu_powergates,
>  	.has_tsense_reset = true,
>  	.has_gpu_clamps = true,
> +	.needs_mbist_war = true,
>  	.num_io_pads = ARRAY_SIZE(tegra210_io_pads),
>  	.io_pads = tegra210_io_pads,
>  	.regs = &tegra20_pmc_regs,
> 

Otherwise looks good to me.

Cheers
Jon
Peter De Schrijver Jan. 25, 2018, 10:06 a.m. | #2
On Wed, Jan 24, 2018 at 10:43:45PM +0000, Jon Hunter wrote:
> 
> On 23/01/18 09:22, Peter De Schrijver wrote:
> > Apply the memory built-in self test work around when ungating certain
> > Tegra210 power domains.
> 
> Nit-pick .. typo in $subject.
> 
> > Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
> > ---
> >  drivers/soc/tegra/pmc.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
> > index ce62a47..c4eff4b 100644
> > --- a/drivers/soc/tegra/pmc.c
> > +++ b/drivers/soc/tegra/pmc.c
> > @@ -153,6 +153,7 @@ struct tegra_pmc_soc {
> >  
> >  	bool has_tsense_reset;
> >  	bool has_gpu_clamps;
> > +	bool needs_mbist_war;
> >  
> >  	const struct tegra_io_pad_soc *io_pads;
> >  	unsigned int num_io_pads;
> > @@ -431,6 +432,9 @@ static int tegra_powergate_power_up(struct tegra_powergate *pg,
> >  
> >  	usleep_range(10, 20);
> >  
> > +	if (pg->pmc->soc->needs_mbist_war)
> > +		tegra210_clk_handle_mbist_war(pg->id);
> > +
> 
> Be good if we could return an error from the above function.
> 

We need to also undo whatever we did before then. 

> >  	if (disable_clocks)
> >  		tegra_powergate_disable_clocks(pg);
> >  
> > @@ -1815,6 +1819,7 @@ static void tegra20_pmc_setup_irq_polarity(struct tegra_pmc *pmc,
> >  	.cpu_powergates = tegra210_cpu_powergates,
> >  	.has_tsense_reset = true,
> >  	.has_gpu_clamps = true,
> > +	.needs_mbist_war = true,
> >  	.num_io_pads = ARRAY_SIZE(tegra210_io_pads),
> >  	.io_pads = tegra210_io_pads,
> >  	.regs = &tegra20_pmc_regs,
> > 
> 
> Otherwise looks good to me.
> 
> Cheers
> Jon
> 
> -- 
> nvpublic
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jon Hunter Jan. 25, 2018, 10:57 a.m. | #3
On 25/01/18 10:06, Peter De Schrijver wrote:
> On Wed, Jan 24, 2018 at 10:43:45PM +0000, Jon Hunter wrote:
>>
>> On 23/01/18 09:22, Peter De Schrijver wrote:
>>> Apply the memory built-in self test work around when ungating certain
>>> Tegra210 power domains.
>>
>> Nit-pick .. typo in $subject.
>>
>>> Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
>>> ---
>>>  drivers/soc/tegra/pmc.c | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
>>> index ce62a47..c4eff4b 100644
>>> --- a/drivers/soc/tegra/pmc.c
>>> +++ b/drivers/soc/tegra/pmc.c
>>> @@ -153,6 +153,7 @@ struct tegra_pmc_soc {
>>>  
>>>  	bool has_tsense_reset;
>>>  	bool has_gpu_clamps;
>>> +	bool needs_mbist_war;
>>>  
>>>  	const struct tegra_io_pad_soc *io_pads;
>>>  	unsigned int num_io_pads;
>>> @@ -431,6 +432,9 @@ static int tegra_powergate_power_up(struct tegra_powergate *pg,
>>>  
>>>  	usleep_range(10, 20);
>>>  
>>> +	if (pg->pmc->soc->needs_mbist_war)
>>> +		tegra210_clk_handle_mbist_war(pg->id);
>>> +
>>
>> Be good if we could return an error from the above function.
>>
> 
> We need to also undo whatever we did before then. 

I think it is fine if on error you 'goto disable_clks'.

Cheers
Jon
Peter De Schrijver Jan. 25, 2018, 12:26 p.m. | #4
On Thu, Jan 25, 2018 at 10:57:36AM +0000, Jon Hunter wrote:
> 
> On 25/01/18 10:06, Peter De Schrijver wrote:
> > On Wed, Jan 24, 2018 at 10:43:45PM +0000, Jon Hunter wrote:
> >>
> >> On 23/01/18 09:22, Peter De Schrijver wrote:
> >>> Apply the memory built-in self test work around when ungating certain
> >>> Tegra210 power domains.
> >>
> >> Nit-pick .. typo in $subject.
> >>
> >>> Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
> >>> ---
> >>>  drivers/soc/tegra/pmc.c | 5 +++++
> >>>  1 file changed, 5 insertions(+)
> >>>
> >>> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
> >>> index ce62a47..c4eff4b 100644
> >>> --- a/drivers/soc/tegra/pmc.c
> >>> +++ b/drivers/soc/tegra/pmc.c
> >>> @@ -153,6 +153,7 @@ struct tegra_pmc_soc {
> >>>  
> >>>  	bool has_tsense_reset;
> >>>  	bool has_gpu_clamps;
> >>> +	bool needs_mbist_war;
> >>>  
> >>>  	const struct tegra_io_pad_soc *io_pads;
> >>>  	unsigned int num_io_pads;
> >>> @@ -431,6 +432,9 @@ static int tegra_powergate_power_up(struct tegra_powergate *pg,
> >>>  
> >>>  	usleep_range(10, 20);
> >>>  
> >>> +	if (pg->pmc->soc->needs_mbist_war)
> >>> +		tegra210_clk_handle_mbist_war(pg->id);
> >>> +
> >>
> >> Be good if we could return an error from the above function.
> >>
> > 
> > We need to also undo whatever we did before then. 
> 
> I think it is fine if on error you 'goto disable_clks'.

Shouldn't we do this for tegra_powergate_reset_deassert() failure as well then?
Also in case of tegra_powergate_enable_clocks() failure we should probably jump
to powergate_off because tegra_powergate_enable_clocks() will disable the
already enabled clocks on failure.

Peter.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jon Hunter Jan. 25, 2018, 1:07 p.m. | #5
On 25/01/18 12:26, Peter De Schrijver wrote:
> On Thu, Jan 25, 2018 at 10:57:36AM +0000, Jon Hunter wrote:
>>
>> On 25/01/18 10:06, Peter De Schrijver wrote:
>>> On Wed, Jan 24, 2018 at 10:43:45PM +0000, Jon Hunter wrote:
>>>>
>>>> On 23/01/18 09:22, Peter De Schrijver wrote:
>>>>> Apply the memory built-in self test work around when ungating certain
>>>>> Tegra210 power domains.
>>>>
>>>> Nit-pick .. typo in $subject.
>>>>
>>>>> Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
>>>>> ---
>>>>>  drivers/soc/tegra/pmc.c | 5 +++++
>>>>>  1 file changed, 5 insertions(+)
>>>>>
>>>>> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
>>>>> index ce62a47..c4eff4b 100644
>>>>> --- a/drivers/soc/tegra/pmc.c
>>>>> +++ b/drivers/soc/tegra/pmc.c
>>>>> @@ -153,6 +153,7 @@ struct tegra_pmc_soc {
>>>>>  
>>>>>  	bool has_tsense_reset;
>>>>>  	bool has_gpu_clamps;
>>>>> +	bool needs_mbist_war;
>>>>>  
>>>>>  	const struct tegra_io_pad_soc *io_pads;
>>>>>  	unsigned int num_io_pads;
>>>>> @@ -431,6 +432,9 @@ static int tegra_powergate_power_up(struct tegra_powergate *pg,
>>>>>  
>>>>>  	usleep_range(10, 20);
>>>>>  
>>>>> +	if (pg->pmc->soc->needs_mbist_war)
>>>>> +		tegra210_clk_handle_mbist_war(pg->id);
>>>>> +
>>>>
>>>> Be good if we could return an error from the above function.
>>>>
>>>
>>> We need to also undo whatever we did before then. 
>>
>> I think it is fine if on error you 'goto disable_clks'.
> 
> Shouldn't we do this for tegra_powergate_reset_deassert() failure as well then?
> Also in case of tegra_powergate_enable_clocks() failure we should probably jump
> to powergate_off because tegra_powergate_enable_clocks() will disable the
> already enabled clocks on failure.

Yes that looks like a bug in the existing code and we should fix that.
Good catch.

Jon

Patch

diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index ce62a47..c4eff4b 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -153,6 +153,7 @@  struct tegra_pmc_soc {
 
 	bool has_tsense_reset;
 	bool has_gpu_clamps;
+	bool needs_mbist_war;
 
 	const struct tegra_io_pad_soc *io_pads;
 	unsigned int num_io_pads;
@@ -431,6 +432,9 @@  static int tegra_powergate_power_up(struct tegra_powergate *pg,
 
 	usleep_range(10, 20);
 
+	if (pg->pmc->soc->needs_mbist_war)
+		tegra210_clk_handle_mbist_war(pg->id);
+
 	if (disable_clocks)
 		tegra_powergate_disable_clocks(pg);
 
@@ -1815,6 +1819,7 @@  static void tegra20_pmc_setup_irq_polarity(struct tegra_pmc *pmc,
 	.cpu_powergates = tegra210_cpu_powergates,
 	.has_tsense_reset = true,
 	.has_gpu_clamps = true,
+	.needs_mbist_war = true,
 	.num_io_pads = ARRAY_SIZE(tegra210_io_pads),
 	.io_pads = tegra210_io_pads,
 	.regs = &tegra20_pmc_regs,