LKML Archive mirror
 help / color / mirror / Atom feed
From: Suman Anna <s-anna@ti.com>
To: Tony Lindgren <tony@atomide.com>
Cc: Kishon Vijay Abraham I <kishon@ti.com>,
	Bjorn Helgaas <bhelgaas@google.com>, <richardcochran@gmail.com>,
	Russell King <linux@arm.linux.org.uk>,
	<linux-omap@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>,
	<linux-kernel@vger.kernel.org>, <nsekhar@ti.com>
Subject: Re: [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset
Date: Wed, 27 Jan 2016 17:16:07 -0600	[thread overview]
Message-ID: <56A94FB7.6020903@ti.com> (raw)
In-Reply-To: <20160127185649.GV19432@atomide.com>

On 01/27/2016 12:56 PM, Tony Lindgren wrote:
> * Suman Anna <s-anna@ti.com> [160127 10:17]:
>> On 01/27/2016 11:31 AM, Tony Lindgren wrote:
>>> Why do you need another reset here? Can't you just implement PM runtime
>>> in the driver and do the usual pm_runtime_put_sync followed by
>>> pm_runtime_disable?
>>
>> The omap_hwmod_enable/disable code does not deal with hardresets (PRCM
>> reset lines) and so the pm_runtime_get_sync/put_sync only end up dealing
>> with clocks, and we need to invoke the reset functions separately.
>> Modules with softresets in SYSCONFIG are ok, as they are dealt with
>> properly.
> 
> Hmm _reset() in omap_hwmod.c has this to call _assert_hardreset:
> 
> 	if (oh->class->reset) {
> 		r = oh->class->reset(oh);
> 	} else {
> 		if (oh->rst_lines_cnt > 0) {
> 			for (i = 0; i < oh->rst_lines_cnt; i++)
> 				_assert_hardreset(oh, oh->rst_lines[i].name);
> 			return 0;
> 		} else {
> 			r = _ocp_softreset(oh);
> 			if (r == -ENOENT)
> 				r = 0;
> 		}
> 	}

Right, hwmod code does the initial reset.

> Care to explain what exactly the problem with the hwmod code not doing
> the reset on init?

And we only need to deassert the reset in probe. Technically, we don't
need to assert first and deassert in probe, and that was a design choice
made by Kishon.

> And why do you need to do another reset in dra7xx_pcie_remove()?

Primarily to restore the reset state back to what it was after the
driver remove gets called. We cannot call deassert twice without calling
a assert in between. Kishon had originally added the assert and deassert
only in probe, but nothing in remove, they ought to be deassert in probe
and assert in remove to match initial hardware state, and to also make
it work across multiple probe/remove.

>>> Basically I'm wondering how come we need these platform data callbacks
>>> at all.
>>
>> The hardresets are controlled through the
>> omap_device_assert(deassert)_hardreset functions, and since these are
>> limited to mach-omap2, we are invoking them through platform data callbacks.
> 
> Right.. But I'm wondering about the why you need to do this in the
> driver at all part :)

The initial reset at init time is okay, but hwmod _enable() bails out if
the resets lines are asserted. This was a change made long time back, I
believe to deal with the problems around the DSP enabling sequences. As
such, pm_runtime_get_sync() and put_sync() do not deassert and assert
the resets.

regards
Suman

  reply	other threads:[~2016-01-27 23:16 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-14 14:11 [PATCH v3 0/3] dra7xx: get pcie working in mainline Kishon Vijay Abraham I
2016-01-14 14:11 ` [PATCH v3 1/3] ARM: DRA7: hwmod: Add reset data for PCIe Kishon Vijay Abraham I
2016-02-08  1:50   ` Paul Walmsley
2016-01-14 14:11 ` [PATCH v3 2/3] ARM: DRA7: add pdata-quirks to do reset of PCIe Kishon Vijay Abraham I
2016-01-15 19:19   ` Suman Anna
2016-01-15 19:22     ` Tony Lindgren
2016-01-15 19:41       ` Suman Anna
2016-01-18  9:12         ` Sekhar Nori
2016-01-27 17:23           ` Tony Lindgren
2016-01-14 14:11 ` [PATCH v3 3/3] pci: dra7xx: use pdata callbacks to perform reset Kishon Vijay Abraham I
2016-01-27 17:31   ` Tony Lindgren
2016-01-27 18:16     ` Suman Anna
2016-01-27 18:56       ` Tony Lindgren
2016-01-27 23:16         ` Suman Anna [this message]
2016-01-28 18:31           ` Tony Lindgren
2016-01-28 21:15             ` Suman Anna
2016-02-02 10:40             ` Kishon Vijay Abraham I
2016-02-05  4:19               ` Kishon Vijay Abraham I
2016-02-08  2:48               ` Paul Walmsley
2016-02-08 20:56                 ` Suman Anna
2016-02-09  8:49                   ` Paul Walmsley
2016-02-09 17:40                     ` Suman Anna
2016-02-09 19:36                       ` Paul Walmsley
2016-02-10  1:42                         ` Suman Anna
2016-02-10  5:38                           ` Kishon Vijay Abraham I
2016-02-11 19:27                             ` Paul Walmsley
2016-02-11 22:04                               ` Suman Anna
2016-02-12  6:49                               ` Kishon Vijay Abraham I
2016-02-12 17:20                                 ` Suman Anna
2016-02-18 14:21                                   ` Sekhar Nori
2016-02-18 17:23                                     ` Paul Walmsley
2016-02-18 18:27                                       ` Suman Anna
2016-02-22  6:18                                     ` Kishon Vijay Abraham I
2016-02-22  6:31                                       ` Paul Walmsley
2016-02-22  9:55                                         ` Kishon Vijay Abraham I
2016-02-23 11:57                                           ` Kishon Vijay Abraham I
2016-02-23 18:28                                             ` Paul Walmsley
2016-02-24  6:21                                               ` Kishon Vijay Abraham I
2016-03-01  8:25                                                 ` Paul Walmsley
2016-03-01 11:55                                                   ` Kishon Vijay Abraham I
2016-03-01 14:43                                                     ` Bjorn Helgaas
2016-03-01 16:55                                                     ` Suman Anna
2016-02-11 20:43                             ` Suman Anna
2016-02-12  6:55                               ` Kishon Vijay Abraham I
2016-02-10  5:36                         ` Kishon Vijay Abraham I

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=56A94FB7.6020903@ti.com \
    --to=s-anna@ti.com \
    --cc=bhelgaas@google.com \
    --cc=kishon@ti.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=linux@arm.linux.org.uk \
    --cc=nsekhar@ti.com \
    --cc=richardcochran@gmail.com \
    --cc=tony@atomide.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).