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=-8.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_SANE_2 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 81AF4C7618B for ; Wed, 24 Jul 2019 12:55:21 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 636C4218B8 for ; Wed, 24 Jul 2019 12:55:21 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726736AbfGXMzV (ORCPT ); Wed, 24 Jul 2019 08:55:21 -0400 Received: from mx2.suse.de ([195.135.220.15]:37256 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726550AbfGXMzU (ORCPT ); Wed, 24 Jul 2019 08:55:20 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 83869AD3A; Wed, 24 Jul 2019 12:55:18 +0000 (UTC) Date: Wed, 24 Jul 2019 14:55:16 +0200 From: Jean Delvare To: Andrew Cooks Cc: Wolfram Sang , linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org, Andrew Cooks , linux-acpi@vger.kernel.org, platypus-sw@opengear.com, "Tobin C . Harding" , Guenter Roeck , Will Wagner Subject: Re: [RESEND][PATCH v4 3/3] i2c: piix4: add ACPI support Message-ID: <20190724145516.342195ac@endymion> In-Reply-To: References: Organization: SUSE Linux X-Mailer: Claws Mail 3.17.3 (GTK+ 2.24.32; x86_64-suse-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-acpi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-acpi@vger.kernel.org On Mon, 26 Feb 2018 10:28:45 +1000, Andrew Cooks wrote: > Enable the i2c-piix4 SMBus controller driver to enumerate I2C slave > devices using ACPI. It builds on the related I2C mux device work > in commit 8eb5c87a92c0 ("i2c: add ACPI support for I2C mux ports") > > In the i2c-piix4 driver the adapters are enumerated as: > Main SMBus adapter Port 0, Port 2, ..., aux port (i.e., ASF adapter) > > However, in the AMD BKDG documentation[1], the implied order of ports is: > Main SMBus adapter Port 0, ASF adapter, Port 2, Port 3, ... > > This ordering difference is unfortunate. We assume that ACPI > developers will use the Linux ordering. > > [1] 52740 BIOS and Kernel Developer's Guide (BKDG) for AMD Family 16h > Models 30h-3Fh Processors > > Signed-off-by: Andrew Cooks > --- > drivers/i2c/busses/i2c-piix4.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c > index 01f1610..9a6cdc8 100644 > --- a/drivers/i2c/busses/i2c-piix4.c > +++ b/drivers/i2c/busses/i2c-piix4.c > @@ -837,6 +837,12 @@ static int piix4_add_adapter(struct pci_dev *dev, unsigned short smba, > /* set up the sysfs linkage to our parent device */ > adap->dev.parent = &dev->dev; > > + if (has_acpi_companion(&dev->dev)) { > + acpi_preset_companion(&adap->dev, > + ACPI_COMPANION(&dev->dev), > + piix4_adapter_count); After the change I proposed for the previous patch, this is no longer going to work. But I don't think it was really working before anyway. For one thing, for the same reason I want to change the previous patch: in case of failure to register some of the adapters, the numbering of later adapters would be shifted. Also giving the aux bus a different number depending on the device (4 before Hudson2, 2 for Hudson2 and later) is unlikely to match the BIOS expectations. For another, the assumption that "ACPI developers will use the Linux ordering" is unlikely to be valid. I think we are talking about BIOS developers here, and they should be OS-agnostic. If they are not, then most likely they would align with whatever Windows drivers expect. So our best chance is to stick to the datasheet. Lastly, this would be inconsistent even with our own driver. We are indeed not instantiating the adapters in the order listed by the datasheet, and i2c adapter numbering is dynamic, but we are *naming* the adapters to match the datasheet. So we should really pass the same number to the ACPI layer, for consistency if nothing else. This probably means passing one more parameter to piix4_add_adapter() and/or some more code to figure out the bus number to pass to acpi_preset_companion(), but I don't think we can avoid that, so let's just do it. > + } > + > snprintf(adap->name, sizeof(adap->name), > "SMBus PIIX4 adapter%s at %04x", name, smba); > -- Jean Delvare SUSE L3 Support