Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Received-SPF: Pass (protection.outlook.com: domain of xilinx.com designates
 149.199.62.198 as permitted sender) receiver=protection.outlook.com;
 client-ip=149.199.62.198; helo=xsj-pvapexch01.xlnx.xilinx.com;
Envelope-to: git@xilinx.com,
 linux-pci@vger.kernel.org,
 linux-arm-kernel@lists.infradead.org,
 sashal@kernel.org,
 robh@kernel.org,
 maz@kernel.org,
 lorenzo.pieralisi@arm.com,
 bhelgaas@google.com,
 monstr@monstr.eu,
 linux-kernel@vger.kernel.org,
 kw@linux.com
Subject: Re: [PATCH v2 2/2] PCI: xilinx-nwl: Enable the clock through CCF
To:     =?UTF-8?Q?Krzysztof_Wilczy=c5=84ski?= <kw@linux.com>,
        Michal Simek <michal.simek@xilinx.com>
CC:     <linux-kernel@vger.kernel.org>, <monstr@monstr.eu>,
        <git@xilinx.com>, <bharat.kumar.gogada@xilinx.com>,
        Hyun Kwon <hyun.kwon@xilinx.com>,
        "Bjorn Helgaas" <bhelgaas@google.com>,
        Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
        Marc Zyngier <maz@kernel.org>,
        Ravi Kiran Gummaluri <rgummal@xilinx.com>,
        "Rob Herring" <robh@kernel.org>, Sasha Levin <sashal@kernel.org>,
        <linux-arm-kernel@lists.infradead.org>, <linux-pci@vger.kernel.org>
References: <cover.1624454607.git.michal.simek@xilinx.com>
 <be603822953d0a815034a952b9c71bac642f22ae.1624454607.git.michal.simek@xilinx.com>
 <20210623135326.GA54420@rocinante>
From:   Michal Simek <michal.simek@xilinx.com>
Message-ID: <f15a9fca-0fb0-5f7a-e1c7-6c52df617a2e@xilinx.com>
Date:   Wed, 23 Jun 2021 16:00:03 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.11.0
MIME-Version: 1.0
In-Reply-To: <20210623135326.GA54420@rocinante>
Content-Type: text/plain; charset="utf-8"
Content-Language: en-US
Content-Transfer-Encoding: 8bit
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 Jun 2021 14:00:24.8966
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: be6f0d5f-17db-4643-992b-08d9364f3fc5
X-MS-Exchange-CrossTenant-Id: 657af505-d5df-48d0-8300-c31994686c5c
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=657af505-d5df-48d0-8300-c31994686c5c;Ip=[149.199.62.198];Helo=[xsj-pvapexch01.xlnx.xilinx.com]
X-MS-Exchange-CrossTenant-AuthSource: DM3NAM02FT019.eop-nam02.prod.protection.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Anonymous
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA0PR02MB7500
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Krzysztof,

On 6/23/21 3:53 PM, Krzysztof Wilczyński wrote:
> [+cc Sasha for visibility]
> 
> Hi Michal,
> 
> Thank you for sending v2 so promptly!  And for all the extra changes and
> fixes.  Much appreciated!
> 
>> Enable PCIE reference clock. There is no remove function that's why
>> this should be enough for simple operation.
>> Normally this clock is enabled by default by firmware but there are
>> usecases where this clock should be enabled by driver itself.
>> It is also good that clock user is recorded in clock framework.
> 
> Small nitpicks: it would be PCIe here in the above and in the error
> message (this is as per [1]), and "use cases" also in the above. 
> 
> This can be corrected when the patch will be merged by either Bjorn or
> Lorenzo, to avoid sending v3 unnecessarily, provided that they would
> have a moment to do it, of course.

Ok. Will wait for them.

> 
>> Fixes: ab597d35ef11 ("PCI: xilinx-nwl: Add support for Xilinx NWL PCIe Host Controller")
> 
> Thank you!
> 
> Does it make sense for this change to be back-ported to stable and
> long-term kernels?
> 
> I am asking to make sure we do the right thing here, as I can imagine
> that older kernels (primarily because some folks could use, for example,
> Ubuntu LTS releases for development) might often be used by people who
> work with the Xilinx FPGAs and such.

I think that make sense to do so. I haven't had a time to take look at
it closely but I think on Xilinx ZynqMP zcu102 board this missing patch
is causing hang when standard debian 5.10 is used.


> 
> [...]
>> +	err = clk_prepare_enable(pcie->clk);
>> +	if (err) {
>> +		dev_err(dev, "can't enable pcie ref clock\n");
>> +		return err;
>> +	}
>> +
> 
> As per the nitpick above, it would be "PCIe", but probably no need to
> send v3 to correct this.

I will keep my eyes on it and will update it if v3 is required.

Thanks,
Michal