From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1751057AbdAPJfs (ORCPT <rfc822;w@1wt.eu>);
        Mon, 16 Jan 2017 04:35:48 -0500
Received: from mga06.intel.com ([134.134.136.31]:24037 "EHLO mga06.intel.com"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1750874AbdAPJfp (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
        Mon, 16 Jan 2017 04:35:45 -0500
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.33,238,1477983600"; 
   d="scan'208";a="809382115"
Message-ID: <1484558980.2133.134.camel@linux.intel.com>
Subject: Re: [PATCH 2/2] spi: pca2xx-pci: Allow MSI
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: Jan Kiszka <jan.kiszka@siemens.com>,
        linux-arm-kernel@lists.infradead.org
Cc: Daniel Mack <daniel@zonque.org>,
        Haojian Zhuang <haojian.zhuang@gmail.com>,
        Robert Jarzmik <robert.jarzmik@free.fr>, linux-kernel@vger.kernel.org,
        Mika Westerberg <mika.westerberg@linux.intel.com>,
        Jarkko Nikula <jarkko.nikula@bitmer.com>,
        Sascha Weisenberger <sascha.weisenberger@siemens.com>
Date: Mon, 16 Jan 2017 11:29:40 +0200
In-Reply-To: <4061bba80ecc632b219b56d8a8cd25d62658d48a.1484557560.git.jan.kiszka@siemens.com>
References: <cover.1484557560.git.jan.kiszka@siemens.com>
         <4061bba80ecc632b219b56d8a8cd25d62658d48a.1484557560.git.jan.kiszka@siemens.com>
Organization: Intel Finland Oy
Content-Type: text/plain; charset="UTF-8"
X-Mailer: Evolution 3.22.3-1 
Mime-Version: 1.0
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Mon, 2017-01-16 at 10:06 +0100, Jan Kiszka wrote:
> Now that the core is ready for edge-triggered interrupts, we can
> safely
> allow the PCI versions that provide this to enable the feature and,
> thus, have less shared interrupts.
> 

My comments below.

> -	 if (IS_ERR(ssp->clk))
> +	if (IS_ERR(ssp->clk))
>  		return PTR_ERR(ssp->clk);

This doesn't belong to the patch.
 
> +	pci_set_master(dev);
> +
> +	ret = pci_alloc_irq_vectors(dev, 1, 1, PCI_IRQ_ALL_TYPES);
> +	if (ret < 0) {
> +		clk_unregister(ssp->clk);
> +		return ret;
> +	}
> +	ssp->irq = pci_irq_vector(dev, 0);
> +

This looks good, though I would put it closer to the initial place of
ssp->irq assignment, i.e. before clock registering.

> +		pci_free_irq_vectors(dev);
> +	pci_free_irq_vectors(dev);

You know my answer, right? So, please be sure that we are using
pcim_alloc_irq_vectors().

Yes, I know there is (was?) no such API, needs to be created. Currently
this might make a mess on ->remove().

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy