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;
Message-ID: <6fb7df68-9772-0552-344c-27a12ac00b0a@intel.com>
Date:   Fri, 28 Jan 2022 12:19:44 +0200
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Firefox/91.0 Thunderbird/91.5.0
Subject: Re: [Intel-wired-lan] [PATCH] e1000e: Remove useless DMA-32 fallback
 configuration
Content-Language: en-US
To:     Christophe JAILLET <christophe.jaillet@wanadoo.fr>,
        Jesse Brandeburg <jesse.brandeburg@intel.com>,
        Tony Nguyen <anthony.l.nguyen@intel.com>,
        "David S. Miller" <davem@davemloft.net>,
        Jakub Kicinski <kuba@kernel.org>
CC:     <netdev@vger.kernel.org>, <kernel-janitors@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <intel-wired-lan@lists.osuosl.org>,
        Christoph Hellwig <hch@lst.de>,
        "Neftin, Sasha" <sasha.neftin@intel.com>,
        "Avivi, Amir" <amir.avivi@intel.com>
References: <5549ec8837b3a6fab83e92c5206cc100ffd23d85.1641752508.git.christophe.jaillet@wanadoo.fr>
From:   "Neftin, Sasha" <sasha.neftin@intel.com>
In-Reply-To: <5549ec8837b3a6fab83e92c5206cc100ffd23d85.1641752508.git.christophe.jaillet@wanadoo.fr>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?c1hXSHp3d1VHR2IzamUxWFVjcnFqUlozOTVwbE1YdktQOTlJeWtFWUVWNEpu?=
 =?utf-8?B?d0hHWXk3NmhUN21lVm9NWGtHVWkyOVl0TmRzRytYa0l1dHFMV0owSTJmdEsw?=
 =?utf-8?B?c3NIMzhWc1J1VnJwWXVjWGtMVEVnWU9pK1BGY1pKTHUxNjQ1Q25zaHgwUENQ?=
 =?utf-8?B?T0pCdzVicXRnbC8wOWFwTHR2OHlPeXU4dGJwMFJYYS9EYkQ1TDJRdzBYUlBR?=
 =?utf-8?B?RkFpaHZZVUhXckdXQVdqdDhyQ3ltd3EyakJZeis3ZHA3Q1VQOEFzTFRJNW5k?=
 =?utf-8?B?RFpPQ0RKUUYrc1BoREFiM0g3RmtpSmhLN0VBSDBjQ1k5dXdCakJaTldwS0N4?=
 =?utf-8?B?UUVkODVQYUVCN0hQdjdEaWJtMCtaVUxBUjBmcytEUmtsQ0ZoYzlVcXROSExV?=
 =?utf-8?B?RHFTd0RPcjZ5aC9UdjlIM3F1MlpOckxWYXU0bmxjVGl0TWVnclE4a2MzMW9l?=
 =?utf-8?B?WE5ad0tUWklSVzVoQzducTA0YjdTdnA4T1hSOG1rOGQ4YUtHQkRBVmg3aitQ?=
 =?utf-8?B?dmlvT2FWQWJxUk15ZnJZaDQrck8vSXZkbkJvUEZ0WXpoY0FDZVFtQVdaOEty?=
 =?utf-8?B?bTZSSVpYd0VCcHIwZC8xZDBjSUo5TWJUZ0RjcEkwanRsNEd6S3pvcG1hbWV4?=
 =?utf-8?B?NnZzbVVEWlFQd3NmVlJmTGxQMlloSDhZak9icHhhWlA5aGpUdVNFZmZTbyta?=
 =?utf-8?B?SldwWEZ6R0wvQ0dvOEMyNDFCQW8wVUZlQURjNHdiY01qZm10T2twZkE0UzYz?=
 =?utf-8?B?TEZ3UmFtUXdyYitIOGkrVWUwMzFVQS9VRmtpeEJYZDY2b25NaElKYTBYb1Fp?=
 =?utf-8?B?SDAxQjdyOFBra3FrWnU2NG9ncCt0NllEb1VtSzhuZ3RhaGZaVGV0TU1xNW50?=
 =?utf-8?B?SzhnaWpPNC9hNE0raldhS0s2dDVoSTd0Y2E3RHdvWERHaVJCdDBsTzlKcVZI?=
 =?utf-8?B?cHBFdUphK3RJeERhcHA2L3o5RHg1VHVwNVBHTWt4RC9Gd3lUK0RTNHJPT2dW?=
 =?utf-8?B?bUZaczlUM09GR3lGYWNNc2xVUG56VFhxaWwrNFNoUU1WR3V6THJmN1pxY2Z3?=
 =?utf-8?B?amw0L3NVVno1VUxnK1FtMXMvYjF5MVJXY0x0c09lbFRLWXFGYWhDd25TNkZk?=
 =?utf-8?B?VXNMMHZUVFZEQ3ZFeFdOYWtxZGpYQkkvYnI0ck1TazBnV2lZVDFwaitrYlBW?=
 =?utf-8?B?ZEY0WkdjSjNZZGZxYmNabExVaHFHaFZjOTFyL2JRczRkb0VTOFNzNCt1ejEw?=
 =?utf-8?B?VEpab1J2ZjNrLzRTWktpVGhsMFl1eE1PcnBWMDRxT1hwck1XWUVWMGhMWk54?=
 =?utf-8?B?QzZWbFQ0Rk8zaXFXQnV6Qkgxby92UTRSbDNUbUE0ZExsZTRVNjd3TUZ2Z3kz?=
 =?utf-8?B?VlBNeGRmSnlvTnR4VDlLTko5UXhMUUd2clJ2MkwyVjNKRlh2VEF2azZtVzNR?=
 =?utf-8?B?bGRjVzhieGZDa0Zva0hESkphTDIvMWZsd1pSTVdTVXppQ1dTMjFqWCtXUWpm?=
 =?utf-8?B?R082dHIxZHNFNmIzcDY5OExMeXQ5RXdKWEFhSXNLeU5aVjhEaFN3alhEbTlr?=
 =?utf-8?B?aUNGbGlrem40R3dwZ2NnRG5qNi9OQ3R2NHNqRlU5dDNjMk1ReGtramZQZ2Ri?=
 =?utf-8?B?cDlnaWE0Ump5TG1OS2prVUFLTXJLaVE1ZHZISThzcktTOWcwOVhLY1VNb0Rq?=
 =?utf-8?B?UzFNV2lhOWg3VnRaVmZxeWNPM1JvcVNDcG5PM09FZEFzVUw1L3Y0bHZEczNy?=
 =?utf-8?B?clc5NnJqZit6amczWEEwcVRxVUpOWi9WQitCcUZRV05QZjRWL01JWDYyQ09k?=
 =?utf-8?B?MGRWOURza2pYV1UzM09IRmJxZ2hlZHFWNmV4ZUhBY0hLU0pRL2hUQkx1Yk5N?=
 =?utf-8?B?RnFkUjFKY2djWUwvWDVldWR4YndVQmhYRWo4TTI1OE9pUkdxYUxFWTdwV005?=
 =?utf-8?B?V2Q4eWlPS3RKUE0yWThKN09ZdWpMZThkVmVkN3Y0d2RjMDZmaGl1dEpEbExU?=
 =?utf-8?B?YTNrVGdoKzgxU1VFOUNVTDU2RTZ4dlJoRlUvaGFWamJnWDJQb29rZzYvOEZH?=
 =?utf-8?B?Zm10cXNueGphQ0Z2MnBZNFNGV2VRUmFuOVFSS01iUFZSYTNUVXd2R1pxTDY4?=
 =?utf-8?B?cXhpV2ZtT0NRcXdKSEY3N3FyYllCY3Z3M09rTVV2SmQ0M25MWXRlMGI0UEhl?=
 =?utf-8?Q?wynbTdmy/Z9JzZCZFPQlEm8=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: e2170f6b-07b2-46da-0966-08d9e247b9cc
X-MS-Exchange-CrossTenant-AuthSource: CO1PR11MB4787.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Jan 2022 10:19:54.1409
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: WJ9GbIfEB6HBDc/uwv5jwzcZxrbPiznNIsdd7HwpDEpIBFUIMa3aAvlto6iujcQgedAERtjwHBIfDMMgQaIn+Q==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN6PR11MB4004
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 1/9/2022 20:23, Christophe JAILLET wrote:
> As stated in [1], dma_set_mask() with a 64-bit mask never fails if
> dev->dma_mask is non-NULL.
> So, if it fails, the 32 bits case will also fail for the same reason.
> 
> So, if dma_set_mask_and_coherent() succeeds, 'pci_using_dac' is known to be
> 1.
> 
> Simplify code and remove some dead code accordingly.
> 
> [1]: https://lkml.org/lkml/2021/6/7/398
> 
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Alexander Lobakin <alexandr.lobakin@intel.com>
> ---
>   drivers/net/ethernet/intel/e1000e/netdev.c | 22 +++++++---------------
>   1 file changed, 7 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
> index 635a95927e93..4f6ee5c44f75 100644
> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
> @@ -7385,9 +7385,9 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>   	resource_size_t flash_start, flash_len;
>   	static int cards_found;
>   	u16 aspm_disable_flag = 0;
> -	int bars, i, err, pci_using_dac;
>   	u16 eeprom_data = 0;
>   	u16 eeprom_apme_mask = E1000_EEPROM_APME;
> +	int bars, i, err;
>   	s32 ret_val = 0;
>   
>   	if (ei->flags2 & FLAG2_DISABLE_ASPM_L0S)
> @@ -7401,17 +7401,11 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>   	if (err)
>   		return err;
>   
> -	pci_using_dac = 0;
>   	err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
> -	if (!err) {
> -		pci_using_dac = 1;
> -	} else {
> -		err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
> -		if (err) {
> -			dev_err(&pdev->dev,
> -				"No usable DMA configuration, aborting\n");
> -			goto err_dma;
> -		}
> +	if (err) {
> +		dev_err(&pdev->dev,
> +			"No usable DMA configuration, aborting\n");
> +		goto err_dma;
>   	}
>   
>   	bars = pci_select_bars(pdev, IORESOURCE_MEM);
> @@ -7547,10 +7541,8 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>   
>   	netdev->priv_flags |= IFF_UNICAST_FLT;
>   
> -	if (pci_using_dac) {
> -		netdev->features |= NETIF_F_HIGHDMA;
> -		netdev->vlan_features |= NETIF_F_HIGHDMA;
> -	}
> +	netdev->features |= NETIF_F_HIGHDMA;
> +	netdev->vlan_features |= NETIF_F_HIGHDMA;
>   
>   	/* MTU range: 68 - max_hw_frame_size */
>   	netdev->min_mtu = ETH_MIN_MTU;
Thank you Christophe
Acked-by: Sasha Neftin <sasha.neftin@intel.com>