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;
MIME-Version: 1.0
References: <20210630034653.10260-1-manivannan.sadhasivam@linaro.org>
 <CAL_JsqLHp3kBc1VtGVRxVr_k69GqSC_JX88jo3stdM4W9Qq6AQ@mail.gmail.com> <20210712075302.GA8113@workstation>
In-Reply-To: <20210712075302.GA8113@workstation>
From:   Rob Herring <robh@kernel.org>
Date:   Mon, 12 Jul 2021 13:56:12 -0600
Message-ID: <CAL_JsqJ4R3+mx9bT4WsKRx4fS5UFbe8en+fRq65HTGcfxaxaNQ@mail.gmail.com>
Subject: Re: [PATCH v5 0/3] Add Qualcomm PCIe Endpoint driver support
To:     Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Cc:     Kishon Vijay Abraham I <kishon@ti.com>,
        Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
        Bjorn Helgaas <bhelgaas@google.com>,
        devicetree@vger.kernel.org, PCI <linux-pci@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        linux-arm-msm <linux-arm-msm@vger.kernel.org>,
        hemantk@codeaurora.org,
        Siddartha Mohanadoss <smohanad@codeaurora.org>,
        Bjorn Andersson <bjorn.andersson@linaro.org>,
        Sriharsha Allenki <sallenki@codeaurora.org>,
        skananth@codeaurora.org, vpernami@codeaurora.org,
        Veerabhadrarao Badiganti <vbadigan@codeaurora.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Mon, Jul 12, 2021 at 1:53 AM Manivannan Sadhasivam
<manivannan.sadhasivam@linaro.org> wrote:
>
> On Thu, Jul 01, 2021 at 09:25:01AM -0600, Rob Herring wrote:
> > On Tue, Jun 29, 2021 at 9:47 PM Manivannan Sadhasivam
> > <manivannan.sadhasivam@linaro.org> wrote:
> > >
> > > Hello,
> > >
> > > This series adds support for Qualcomm PCIe Endpoint controller found
> > > in platforms like SDX55. The Endpoint controller is based on the designware
> > > core with additional Qualcomm wrappers around the core.
> > >
> > > The driver is added separately unlike other Designware based drivers that
> > > combine RC and EP in a single driver. This is done to avoid complexity and
> > > to maintain this driver autonomously.
> > >
> > > The driver has been validated with an out of tree MHI function driver on
> > > SDX55 based Telit FN980 EVB connected to x86 host machine over PCIe.
> > >
> > > Thanks,
> > > Mani
> > >
> > > Changes in v5:
> > >
> > > * Removed the DBI register settings that are not needed
> > > * Used the standard definitions available in pci_regs.h
> > > * Added defines for all the register fields
> > > * Removed the left over code from previous iteration
> > >
> > > Changes in v4:
> > >
> > > * Removed the active_config settings needed for IPA integration
> > > * Switched to writel for couple of relaxed versions that sneaked in
> >
> > I thought we resolved this discussion. Use _relaxed variants unless
> > you need the stronger ones.
> >
>
> I thought the discussion was resolved in favor of using read/writel. Here
> is the last reply from Bjorn:
>
> "I think we came to the conclusion that writel() was better
> than incorrect use of writel_relaxed() followed by wmb(). And in this
> particular case it's definitely not happening in a hot code path..."

Certainly if you're needing wmb(), then you shouldn't be using
_relaxed() variants.

> IMO, it is safer to use readl/writel calls than the relaxed variants.

Sure, and it's safer to have one big lock than it is to have no locks
or lots of small locks.

> And so far the un-written rule I assumed is, only consider using the
> relaxed variants if the code is in hot path (but somehow I used the
> relaxed version in v1 :P )

If you want any real conclusion, then best to fix the 'un-written' part.

But what I say for every PCI review, is use the _relaxed() variants.

Rob