Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 3 Jan 2023 19:16:58 +0200
From:   Leon Romanovsky <leon@kernel.org>
To:     Bjorn Helgaas <helgaas@kernel.org>
Cc:     Paul Menzel <pmenzel@molgen.mpg.de>,
        Rajat Khandelwal <rajat.khandelwal@linux.intel.com>,
        anthony.l.nguyen@intel.com, netdev@vger.kernel.org,
        rajat.khandelwal@intel.com, jesse.brandeburg@intel.com,
        linux-kernel@vger.kernel.org, edumazet@google.com,
        intel-wired-lan@lists.osuosl.org, linux-pci@vger.kernel.org,
        Bjorn Helgaas <bhelgaas@google.com>, kuba@kernel.org,
        pabeni@redhat.com, davem@davemloft.net
Subject: Re: [Intel-wired-lan] [PATCH] igc: Mask replay rollover/timeout
 errors in I225_LMVP
Message-ID: <Y7RjCkanr0Ulx3TD@unreal>
References: <Y7QYxAhcUa2JtjSy@unreal>
 <20230103142104.GA996978@bhelgaas>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230103142104.GA996978@bhelgaas>
Precedence: bulk

On Tue, Jan 03, 2023 at 08:21:04AM -0600, Bjorn Helgaas wrote:
> On Tue, Jan 03, 2023 at 02:00:04PM +0200, Leon Romanovsky wrote:
> > On Tue, Jan 03, 2023 at 05:54:02AM -0600, Bjorn Helgaas wrote:
> > > On Tue, Jan 03, 2023 at 11:54:24AM +0200, Leon Romanovsky wrote:
> > > > On Sun, Jan 01, 2023 at 11:34:21AM +0100, Paul Menzel wrote:
> > > > > Am 01.01.23 um 09:32 schrieb Leon Romanovsky:
> > > > > > On Thu, Dec 29, 2022 at 05:56:40PM +0530, Rajat Khandelwal wrote:
> > > > > > > The CPU logs get flooded with replay rollover/timeout AER errors in
> > > > > > > the system with i225_lmvp connected, usually inside thunderbolt devices.
> > > > > > > 
> > > > > > > One of the prominent TBT4 docks we use is HP G4 Hook2, which incorporates
> > > > > > > an Intel Foxville chipset, which uses the igc driver.
> > > > > > > On connecting ethernet, CPU logs get inundated with these errors. The point
> > > > > > > is we shouldn't be spamming the logs with such correctible errors as it
> > > > > > > confuses other kernel developers less familiar with PCI errors, support
> > > > > > > staff, and users who happen to look at the logs.
> > > 
> > > > > > > --- a/drivers/net/ethernet/intel/igc/igc_main.c
> > > > > > > +++ b/drivers/net/ethernet/intel/igc/igc_main.c
> > > 
> > > > > > > +static void igc_mask_aer_replay_correctible(struct igc_adapter *adapter)
> > > 
> > > > > > Shouldn't this igc_mask_aer_replay_correctible function be implemented
> > > > > > in drivers/pci/quirks.c and not in igc_probe()?
> > > > > 
> > > > > Probably. Though I think, the PCI quirk file, is getting too big.
> > > > 
> > > > As long as that file is right location, we should use it.
> > > > One can refactor quirk file later.
> > > 
> > > If a quirk like this is only needed when the driver is loaded, 
> > 
> > This is always the case with PCI devices managed through kernel, isn't it?
> > Users don't care/aware about "broken" devices unless they start to use them.
> 
> Indeed, that's usually the case.  There's a lot of stuff in quirks.c
> that could probably be in drivers instead.

NP, so or deprecate quirks.c and prohibit any change to that file or
don't allow drivers to mangle PCI in their probe routines.
Everything in-between will cause to enormous mess in long run.

Thanks

> 
> Bjorn