Received: by 2002:a05:6a10:1a4d:0:0:0:0 with SMTP id nk13csp1660153pxb; Wed, 9 Feb 2022 01:29:03 -0800 (PST) X-Google-Smtp-Source: ABdhPJwO3XhZ61J0QWelnX7pS8YQLoeL5qCzWyDfqYpfq8AeecT8lr1z7dQs9kG8K2Vr9BL457d2 X-Received: by 2002:a63:2c05:: with SMTP id s5mr1211649pgs.106.1644398943599; Wed, 09 Feb 2022 01:29:03 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1644398943; cv=none; d=google.com; s=arc-20160816; b=WzxrsydZm+tKtWkdXBp46V7RGDfvyjJiQdXtKJPF6alDCzlyoJuR1xy2XwXzYadvRu krIpoGayASxnZjwmeQPQboqGR03IgQ39zC5pmVKWerbdbxiPIDlVFDexGfIBI06orIxT E+/+aniur8uoAxCPrGkoZoqwhRXCJEtAs2j7i6cvVcyIdLDXBtP1UshKHVzVm+UOCGGL j/1X4RBPdBY59BHoAQ+V8N9/L/klvDFMvPoISh2gtfGRjqpxvwE8i2g9KrnGVNkIWdZr bTuJpMO4VjefbcIpB4Pu/mYVn1ehRpLc5G3g57BC3yMOVqgVg0Y8L+XMuLlE8pU8buQZ 7V7Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :organization:references:in-reply-to:message-id:subject:cc:to:from :date:dkim-signature:dkim-signature; bh=RefxbU951jlfBz7uIMcFk+7I0WDLMgFTTEu0GfJJxIc=; b=RMCoAlk30rZNrQwwfONhmLV/YTwv4AXhspNrUCZGF+ht1yQzFvKThKOBf/n5aT0LW1 jjxeXsIEKCshlff9tSmqTwnb+Y9E7RsPbBCWKMpFwZOl7EHv3BpeBkYepgm65dlNYQWN q4Dje/LgEG8pDevqZ9loZCjRrBFOdJBE0ZucvmjzHJ0Bym655THyNZnCuiBGFWIFmN8G fZCAVRhyVa3LS/LBCgTzmZMFNmQFq0+T+UaeZ/S0F5OgN/jStXXJwDGHKcLq0y013LuU RnmLps39wgJEwwFnidN1pzhZKUkZeU/YsP2/rMTAiw6icFnJTWd6Rta7+BJrxTcvKTt3 /G9w== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@suse.de header.s=susede2_rsa header.b=EJjHrjSG; dkim=neutral (no key) header.i=@suse.de; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=suse.de Return-Path: Received: from lindbergh.monkeyblade.net (lindbergh.monkeyblade.net. [2620:137:e000::1:18]) by mx.google.com with ESMTPS id j62si14788825pge.93.2022.02.09.01.29.03 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 09 Feb 2022 01:29:03 -0800 (PST) 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; Authentication-Results: mx.google.com; dkim=pass header.i=@suse.de header.s=susede2_rsa header.b=EJjHrjSG; dkim=neutral (no key) header.i=@suse.de; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=suse.de Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id 93706E00FA6F; Wed, 9 Feb 2022 00:59:07 -0800 (PST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1380408AbiBHP3w (ORCPT + 99 others); Tue, 8 Feb 2022 10:29:52 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44110 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235109AbiBHP3s (ORCPT ); Tue, 8 Feb 2022 10:29:48 -0500 Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 99EC9C061576; Tue, 8 Feb 2022 07:29:47 -0800 (PST) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id 55FFC1F383; Tue, 8 Feb 2022 15:29:46 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1644334186; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=RefxbU951jlfBz7uIMcFk+7I0WDLMgFTTEu0GfJJxIc=; b=EJjHrjSGzZxeKgTcgWAfXnys77d5lTsvC4y9X6h5H+hj10/m5CR2HmfqbGvQlVOpJ4eidA 5BYmNCjNMNz3LsOl0UU9BMYVwz8PdMKrxEwUvCpyQM86GbfWkZ+vIQm82WZe6yArPgVMUc qA/1wjSie8CB5RJzbQSH9/M/J5WVEc8= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1644334186; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=RefxbU951jlfBz7uIMcFk+7I0WDLMgFTTEu0GfJJxIc=; b=UPVg8xY0uNxI/UbSRDsH4x52GSrGeUgd/FVnlZawSTwSmtwLJJv5NRWmrXE/JuwERkDVd8 SLqjwEwtO8rblMDg== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id CDC2013C99; Tue, 8 Feb 2022 15:29:45 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id Rp2wMGmMAmIxJwAAMHmgww (envelope-from ); Tue, 08 Feb 2022 15:29:45 +0000 Date: Tue, 8 Feb 2022 16:29:44 +0100 From: Jean Delvare To: Terry Bowman Cc: , , , , , , , , , , , , , , Subject: Re: [PATCH v4 5/9] i2c: piix4: Move SMBus port selection into function Message-ID: <20220208162944.3207577c@endymion.delvare> In-Reply-To: <20220130184130.176646-6-terry.bowman@amd.com> References: <20220130184130.176646-1-terry.bowman@amd.com> <20220130184130.176646-6-terry.bowman@amd.com> Organization: SUSE Linux X-Mailer: Claws Mail 3.18.0 (GTK+ 2.24.32; x86_64-suse-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,RDNS_NONE,SPF_HELO_NONE,T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sun, 30 Jan 2022 12:41:26 -0600, Terry Bowman wrote: > Move port selection code into a separate function. Refactor is in > preparation for following MMIO changes. > > Signed-off-by: Terry Bowman > --- > drivers/i2c/busses/i2c-piix4.c | 26 ++++++++++++++++---------- > 1 file changed, 16 insertions(+), 10 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c > index eab06e9e5fdf..32a30af5778a 100644 > --- a/drivers/i2c/busses/i2c-piix4.c > +++ b/drivers/i2c/busses/i2c-piix4.c > @@ -699,6 +699,19 @@ static void piix4_imc_wakeup(void) > release_region(KERNCZ_IMC_IDX, 2); > } > > +static int piix4_sb800_port_sel(u8 port) > +{ > + u8 smba_en_lo, val; > + > + outb_p(piix4_port_sel_sb800, SB800_PIIX4_SMB_IDX); > + smba_en_lo = inb_p(SB800_PIIX4_SMB_IDX + 1); > + > + val = (smba_en_lo & ~piix4_port_mask_sb800) | port; > + if (smba_en_lo != val) > + outb_p(val, SB800_PIIX4_SMB_IDX + 1); > + > + return (smba_en_lo & piix4_port_mask_sb800); > +} > /* > * Handles access to multiple SMBus ports on the SB800. > * The port is selected by bits 2:1 of the smb_en register (0x2c). > @@ -715,8 +728,7 @@ static s32 piix4_access_sb800(struct i2c_adapter *adap, u16 addr, > unsigned short piix4_smba = adapdata->smba; > int retries = MAX_TIMEOUT; > int smbslvcnt; > - u8 smba_en_lo; > - u8 port; > + u8 prev_port; > int retval; > > retval = piix4_sb800_region_request(&adap->dev); > @@ -776,18 +788,12 @@ static s32 piix4_access_sb800(struct i2c_adapter *adap, u16 addr, > } > } > > - outb_p(piix4_port_sel_sb800, SB800_PIIX4_SMB_IDX); > - smba_en_lo = inb_p(SB800_PIIX4_SMB_IDX + 1); > - > - port = adapdata->port; > - if ((smba_en_lo & piix4_port_mask_sb800) != port) > - outb_p((smba_en_lo & ~piix4_port_mask_sb800) | port, > - SB800_PIIX4_SMB_IDX + 1); > + prev_port = piix4_sb800_port_sel(adapdata->port); > > retval = piix4_access(adap, addr, flags, read_write, > command, size, data); > > - outb_p(smba_en_lo, SB800_PIIX4_SMB_IDX + 1); > + piix4_sb800_port_sel(prev_port); > > /* Release the semaphore */ > outb_p(smbslvcnt | 0x20, SMBSLVCNT); Hmm, this gets pretty inefficient. You set SB800_PIIX4_SMB_IDX to piix4_port_sel_sb800 twice and compare the new port with the previous port twice, even though the result will inevitably be the same. The original code would just unconditionally restore the original port value, which was also not always needed, but was cheaper in the end. I can't really think of a better way though, so, so be it :-( I wonder why we insist on restoring the port though. When you read multiple values from the same bus (and I2C device drivers do that a lot, obviously) you end up switching ports a lot for no good reason. I looked at the git history and it has been that way since support for the multiplexed buses was added back in 2015. There's a mutex protecting that section of code anyway, so I don't think restoring the port buys us anything we did not already have. The only benefit I can see is to leave the port setting in its original state on suspend and shutdown (and we know by experience that it can be important to the BIOS) but then we should just do it in the suspend and remove functions, instead of after every transfer. -- Jean Delvare SUSE L3 Support