From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750923AbdEBPvo (ORCPT ); Tue, 2 May 2017 11:51:44 -0400 Received: from pegasos-out.vodafone.de ([80.84.1.38]:37361 "EHLO pegasos-out.vodafone.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750733AbdEBPvm (ORCPT ); Tue, 2 May 2017 11:51:42 -0400 X-Spam-Flag: NO X-Spam-Score: -0.045 Authentication-Results: rohrpostix2.prod.vfnet.de (amavisd-new); dkim=pass header.i=@vodafone.de X-DKIM: OpenDKIM Filter v2.6.8 pegasos-out.vodafone.de D21F6564B0D Subject: Re: [PATCH 2/4] PCI: add functionality for resizing resources v3 To: Andy Shevchenko References: <1493126394-1239-1-git-send-email-deathsimple@vodafone.de> <1493126394-1239-3-git-send-email-deathsimple@vodafone.de> Cc: helgaas@kernel.org, "linux-pci@vger.kernel.org" , dri-devel@lists.freedesktop.org, Platform Driver , "linux-kernel@vger.kernel.org" From: =?UTF-8?Q?Christian_K=c3=b6nig?= Message-ID: <6238d127-1c70-8235-e247-bb6cb3f4c040@vodafone.de> Date: Tue, 2 May 2017 17:51:36 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Am 26.04.2017 um 19:00 schrieb Andy Shevchenko: > On Tue, Apr 25, 2017 at 4:19 PM, Christian König > wrote: >> From: Christian König >> >> This allows device drivers to request resizing their BARs. >> >> The function only tries to reprogram the windows of the bridge directly above >> the requesting device and only the BAR of the same type (usually mem, 64bit, >> prefetchable). This is done to make sure not to disturb other drivers by >> changing the BARs of their devices. >> >> If reprogramming the bridge BAR fails the old status is restored and -ENOSPC >> returned to the calling device driver. >> +int pci_reassign_bridge_resources(struct pci_dev *bridge, unsigned long type) >> +{ >> + const unsigned long type_mask = IORESOURCE_IO | IORESOURCE_MEM | >> + IORESOURCE_PREFETCH | IORESOURCE_MEM_64; >> + > Redundant. Redundant, but also a reminder to myself that I wanted to ask something about that. This type_mask is used already three times in this file, shouldn't we add a define for that? > [SNIP] >> + list_for_each_entry(dev_res, &saved, list) { >> + /* Skip the bridge we just assigned resources for. */ >> + if (bridge == dev_res->dev) >> + continue; >> + >> + bridge = dev_res->dev; >> + pci_setup_bridge(bridge->subordinate); >> + } >> + >> + free_list(&saved); >> + free_list(&failed); >> + return ret; > You might re-use two lines with below, but perhaps better to show > which case returns 0 explicitly and drop assignment ret = 0 above. Good point, but actually the free_list(&failed) is superfluous here since when the failed list isn't empty we end up in the cleanup path. Going to fix all other comments in the next version. Regards, Christian.