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;
Date:   Wed, 23 Jun 2021 15:53:26 +0200
From:   Krzysztof =?utf-8?Q?Wilczy=C5=84ski?= <kw@linux.com>
To:     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
Subject: Re: [PATCH v2 2/2] PCI: xilinx-nwl: Enable the clock through CCF
Message-ID: <20210623135326.GA54420@rocinante>
References: <cover.1624454607.git.michal.simek@xilinx.com>
 <be603822953d0a815034a952b9c71bac642f22ae.1624454607.git.michal.simek@xilinx.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <be603822953d0a815034a952b9c71bac642f22ae.1624454607.git.michal.simek@xilinx.com>
Precedence: bulk

[+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.

> 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.

[...]
> +	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.

1. https://lore.kernel.org/linux-pci/20171026223701.GA25649@bhelgaas-glaptop.roam.corp.google.com/

	Krzysztof