From: Laurentiu Tudor <laurentiu.tudor@nxp.com>
To: Bharat Bhushan <bharat.bhushan@nxp.com>,
        Nipun Gupta <nipun.gupta@nxp.com>,
        "stuyoder@gmail.com" <stuyoder@gmail.com>,
        "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
        "cakturk@gmail.com" <cakturk@gmail.com>,
        "bretth256@gmail.com" <bretth256@gmail.com>,
        "arnd@arndb.de" <arnd@arndb.de>
CC: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "devel@driverdev.osuosl.org" <devel@driverdev.osuosl.org>
Subject: Re: [PATCH 1/2] staging: fsl-mc: Allocate IRQ's before scanning DPRC
 objects
Thread-Topic: [PATCH 1/2] staging: fsl-mc: Allocate IRQ's before scanning DPRC
 objects
Thread-Index: AQHTbnlt2BNJ65h210adI9tnCdVqlqM2T7oAgAAJMQCAAAXGAA==
Date: Wed, 6 Dec 2017 14:23:45 +0000
Message-ID: <5A27FD70.7040208@nxp.com>
References: <1512577087-5345-1-git-send-email-nipun.gupta@nxp.com>
 <5A27F0E2.6010403@nxp.com>
 <AM5PR0401MB2545FF924020D53A64D058B99A320@AM5PR0401MB2545.eurprd04.prod.outlook.com>
In-Reply-To: <AM5PR0401MB2545FF924020D53A64D058B99A320@AM5PR0401MB2545.eurprd04.prod.outlook.com>
Accept-Language: en-US
Content-Language: en-US
spamdiagnosticoutput: 1:99
spamdiagnosticmetadata: NSPM
Content-Type: text/plain; charset="Windows-1252"
Content-ID: <4B22303A953DF44595821094AB4538ED@eurprd04.prod.outlook.com>
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 27b246a2-9788-446b-ea7e-08d53cb4f57c
X-MS-Exchange-CrossTenant-originalarrivaltime: 06 Dec 2017 14:23:45.1458
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR0401MB2431
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Transfer-Encoding: 8bit
X-MIME-Autoconverted: from quoted-printable to 8bit by nfs id vB6ENsjQ015207
Content-Length: 2982
Lines: 74

Hi Bharat,

On 12/06/2017 04:03 PM, Bharat Bhushan wrote:
> Hi Laurentiu,
>
>> -----Original Message-----
>> From: Laurentiu Tudor
>> Sent: Wednesday, December 06, 2017 7:00 PM
>> To: Nipun Gupta <nipun.gupta@nxp.com>; stuyoder@gmail.com; Bharat
>> Bhushan <bharat.bhushan@nxp.com>; gregkh@linuxfoundation.org;
>> cakturk@gmail.com; bretth256@gmail.com; arnd@arndb.de
>> Cc: linux-kernel@vger.kernel.org; devel@driverdev.osuosl.org
>> Subject: Re: [PATCH 1/2] staging: fsl-mc: Allocate IRQ's before scanning DPRC
>> objects
>>
>> Hi Nipun,
>>
>> Can you polish a bit this commit message? It doesn't seem to explain why this
>> is needed.
>>
>> On 12/06/2017 06:18 PM, Nipun Gupta wrote:
>>> When DPRC probing is deferred (such as where IOMMU is not probed
>>> before the fsl-mc bus), all the devices in the DPRC containers gets
>>> initialized one after another.
>>
>> Are you referring to dprc probing being deferred (do we ever do that?) or
>> devices inside the dprc deferring the probe?
>>
>>> As IRQ's were allocated only once the
>>> DPRC scanning is completed, the devices like DPIO which uses these
>>> IRQ's at initalization fails. This patch allocates the IRQ resources
>>
>> s/initalization/initialization
>>
>>> before scanning all the objects in the DPRC container.
>>>
>>> Signed-off-by: Nipun Gupta <nipun.gupta@nxp.com>
>>> ---
>>>    drivers/staging/fsl-mc/bus/dprc-driver.c | 49 ++++++++++++++++++------
>> --------
>>>    1 file changed, 27 insertions(+), 22 deletions(-)
>>>
>>> diff --git a/drivers/staging/fsl-mc/bus/dprc-driver.c
>>> b/drivers/staging/fsl-mc/bus/dprc-driver.c
>>> index 06df528..7265431 100644
>>> --- a/drivers/staging/fsl-mc/bus/dprc-driver.c
>>> +++ b/drivers/staging/fsl-mc/bus/dprc-driver.c
>>> @@ -206,7 +206,8 @@ static void dprc_add_new_devices(struct
>> fsl_mc_device *mc_bus_dev,
>>>     * dprc_scan_objects - Discover objects in a DPRC
>>>     *
>>>     * @mc_bus_dev: pointer to the fsl-mc device that represents a DPRC
>>> object
>>> - * @total_irq_count: total number of IRQs needed by objects in the DPRC.
>>> + * @total_irq_count: If argument is provided the function populates
>>> + the
>>> + * total number of IRQs created by objects in the DPRC.
>>
>> As a side node, after a cursory look i noticed that this total_irq_count
>> parameter is used only for some sanity checks. I'm thinking of dropping it in a
>> follow-up cleanup patch.
>
> How you will ensure that number of IRQ needed are not sufficient for devices in the container?
> Do you think we need to either not enable additional devices or add irqs to irq-pool ?

In the current implementation we allocate a pool of 
FSL_MC_IRQ_POOL_MAX_TOTAL_IRQS (= 256) no mater what total_irq_count is.
I think this is enough for our current scenarios, but if in the future 
we ever hit this limit we can think of a mechanism along the lines of 
your example. Adding another chunk of irqs to the pool seems to me like 
a good idea in the future.

---
Best Regards, Laurentiu