From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-17.9 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id C6323C433F5 for ; Wed, 8 Sep 2021 13:15:02 +0000 (UTC) Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 0DE04610FF for ; Wed, 8 Sep 2021 13:15:02 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 0DE04610FF Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 6020A8326B; Wed, 8 Sep 2021 15:15:00 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=kernel.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=kernel.org header.i=@kernel.org header.b="uQ0mcu3M"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 8A44D83347; Wed, 8 Sep 2021 15:14:58 +0200 (CEST) Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 3A9B18326B for ; Wed, 8 Sep 2021 15:14:55 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=kernel.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=pali@kernel.org Received: by mail.kernel.org (Postfix) with ESMTPSA id 7B402610FF; Wed, 8 Sep 2021 13:14:53 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1631106893; bh=QCu0eEXgtFXIFRuZJOl3cwR4QHGsMYZiGpwVJEIVCX8=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=uQ0mcu3MjlSbgT/Q596mtWZthwMMzRPsvUPoMzQWU2l52iGg6kLxsl0tvVLeOZHsf 2Hh1BNYzGnwQCf6XB5cm24ymwRXLXk73olcEuuwHVO5knd5Txv2ceyWfcAsNXz7ihF k0gDSYcBWBpYnxIz0vAcEIHY7OTeS7SMHCoLzQZj90oaYiKfD3mN69IPqmsVyYRjRp 66vJM/hQ3Gb7EMun+HUw9fPygXZxyHgsPg6aVX5SHdG87HwCTTMoYsGvdib0EZuWtz ZHoONTT2i43Ki4HnLT3ByFHLG+f1lUNSu3s/aKmHwPZRxu6BNp5R7hWPwzlgzahoxe z+lm/G9WZuMwA== Received: by pali.im (Postfix) id 0BE84708; Wed, 8 Sep 2021 15:14:50 +0200 (CEST) Date: Wed, 8 Sep 2021 15:14:50 +0200 From: Pali =?utf-8?B?Um9ow6Fy?= To: Bin Meng Cc: Marek =?utf-8?B?QmVow7pu?= , Simon Glass , Stefan Roese , U-Boot Mailing List Subject: Re: [PATCH u-boot] dm: pci: Fix handling of errors when scanning device Message-ID: <20210908131450.ypbefsovm55ze2yn@pali> References: <20210907160708.31547-1-marek.behun@nic.cz> <20210908140256.393aeb9a@dellmb> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: NeoMutt/20180716 X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.34 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.2 at phobos.denx.de X-Virus-Status: Clean On Wednesday 08 September 2021 20:57:06 Bin Meng wrote: > On Wed, Sep 8, 2021 at 8:02 PM Marek Behún wrote: > > > > On Wed, 8 Sep 2021 09:06:28 +0800 > > Bin Meng wrote: > > > > > On Wed, Sep 8, 2021 at 12:07 AM Marek Behún > > > wrote: > > > > > > > > From: Pali Rohár > > > > > > > > Some PCIe controller's read_config() method support indicating error > > > > directly via return value, but some cannot distinguish all-ones (or > > > > all-zeros) read response from an error. > > > > > > > > The current code in pci_bind_bus_devices() interprets all-ones / > > > > all-zeros in PCI_VENDOR_ID register as "nothing connected", and > > > > continues the cycle, but an error returned via return value breaks > > > > the cycle. > > > > > > > > This is wrong for the PCIe controllers which return this error via > > > > return value. > > > > > > > > Handle all errors when reading PCI_VENDOR_ID the same way. > > > > > > > > This fixes enumeration of PCI devices for example when there is a > > > > PCI bridge connected behind another PCI bridge and not all ports are > > > > connected to a device, and the controller (for example Aardvark) > > > > translates the UR error (Unsupported Request) as -EOPNOTSUPP. > > > > > > > > Signed-off-by: Pali Rohár > > > > Signed-off-by: Marek Behún > > > > --- > > > > drivers/pci/pci-uclass.c | 9 +-------- > > > > 1 file changed, 1 insertion(+), 8 deletions(-) > > > > > > > > > > Is it possible to update these PCI controller drivers to fill in > > > vendor id to all zeros when there is an errror? > > > > It is, but we think it is wrong. If the PCI controller supports > > reporting errors, they should be propagated. > > > > What we might want to change in those controller drivers, though, is > > error numbers, so that they are the same for each controller. For > > example Unsupported Request should be always translated to EOPNOTSUPP. > > So if it is a real error that for some reason the controller cannot > read the configuration space, we should abort instead of continue > looping. Also 0xFFFF value signals a real error that controllers cannot access configuration space. Just it do not say what error it is. And also there is no way to distinguish between "no error happened ans read value is 0xFFFF" and "error happened". Anyway, I think that for multifunction device you need to continue looping as on another function number there may be working function. For non-multifunction devices we can abort touching configuration space. But looking at the code, it is already implemented in this way - just it 'continue;' the loop. > Not sure what is the best approach though. Handle errors (independently on how they are signaled) in the same way. We just know that card must not have PCI vendor 0xFFFF nor 0x0000 so when reading this register we know that value 0xFFFF for sure signals some (unknown) error. There is a lot of stuff which could be improved, but this is the simple step to fix usage of Bridge behind Bridge. > FWIW, > Reviewed-by: Bin Meng