Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
MIME-Version: 1.0
References: <20220416110507.642398-1-pgwipeout@gmail.com> <20220416110507.642398-3-pgwipeout@gmail.com>
 <308e9c47197d4f7ae5a31cfcb5a10886@kernel.org> <CAMdYzYo+YeAgT92baMOoWpra230wro_WynRcajL-__9RNkeE9Q@mail.gmail.com>
 <87zgkk9gtc.wl-maz@kernel.org> <CAMdYzYo_+7rakc=GCTueEZvH_F4Co6+=eKAUztJaafiDXSKKXQ@mail.gmail.com>
 <87sfqaa7uv.wl-maz@kernel.org> <CAMdYzYouLoYX89EWjQTRAjd-1bvJEJtfYQa2KrtFH22Kp-4Efw@mail.gmail.com>
 <878rs2c8ay.wl-maz@kernel.org> <CAMdYzYqfitnPYSUEeeUB=x9=L9xzAu6o0SBmEdF8WKeb-+b6uQ@mail.gmail.com>
 <87r15ta469.wl-maz@kernel.org>
In-Reply-To: <87r15ta469.wl-maz@kernel.org>
From:   Peter Geis <pgwipeout@gmail.com>
Date:   Tue, 19 Apr 2022 16:37:42 -0400
Message-ID: <CAMdYzYrskyNXYiwKY-O=+_69x7T4asjrfo5KNGao-_6PCwq-gw@mail.gmail.com>
Subject: Re: [PATCH v7 2/4] PCI: dwc: rockchip: add legacy interrupt support
To:     Marc Zyngier <maz@kernel.org>
Cc:     Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
        Rob Herring <robh@kernel.org>,
        =?UTF-8?Q?Krzysztof_Wilczy=C5=84ski?= <kw@linux.com>,
        Bjorn Helgaas <bhelgaas@google.com>,
        Heiko Stuebner <heiko@sntech.de>,
        "open list:ARM/Rockchip SoC..." <linux-rockchip@lists.infradead.org>,
        PCI <linux-pci@vger.kernel.org>,
        devicetree <devicetree@vger.kernel.org>,
        arm-mail-list <linux-arm-kernel@lists.infradead.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Tue, Apr 19, 2022 at 4:05 AM Marc Zyngier <maz@kernel.org> wrote:
>
> On Tue, 19 Apr 2022 01:23:23 +0100,
> Peter Geis <pgwipeout@gmail.com> wrote:
> >
> > > My only ask is that you properly initialise the HW. This will save
> > > countless amount of head-scratching once you have a decent firmware or
> > > kexec.
> >
> > The only way to ensure that in a sane way is to trigger the resets at
> > driver probe.
>
> If that can be done, that'd be great.

Okay, I'll work on implementing this then.

>
> > Can that be safely done without causing other issues with an already
> > configured card or should I power cycle it as well?
>
> Well, you are already renegotiating the link anyway, so that's a very
> moot point.

Understood, thank you.

>
> > This is starting to feature creep from the original intention of this
> > series, since a pre-configured controller would affect more than just
> > interrupts.
>
> Configuring the HW is not exactly a feature creep. If your intention
> is to keep everything as it was left, then you don't have much of a
> driver, but instead a time bomb. And we can do without another one in
> the tree.

Understood, I apologize if I'm being difficult here, I just want to
make sure I completely understand the assignment.
Your comment about kexec made it clear for me, thank you.

>
> > If you wish, as a compromise I can ensure all INTx interrupts are
> > masked at probe (which would hilariously be the opposite of
> > downstream).
>
> As far as I'm concerned, downstream doesn't exist. If someone wants
> the downstream code, they can use it directly and we don't need to
> merge this code.

Once again, you'll have no argument from me in this regard.
I've had several blocks of hardware enablement sitting out of tree
waiting for the phy code to land.
As much testing as my branch has seen, it's still only a drop in the
bucket compared to finally being mainlined.
I appreciate all of your effort and review here and I absolutely want
this done correctly.

>
> If, on the other hand, you want this driver to be useful and to be
> maintained upstream, initialising the interrupt mask is the absolute
> bare minimum.

I think resetting the whole core is the best move, since it's the only
way we can guarantee a sane configuration with the documentation we
have.

>
>         M.
>
> --
> Without deviation from the norm, progress is not possible.