From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752531AbdLNDbn (ORCPT ); Wed, 13 Dec 2017 22:31:43 -0500 Received: from mail-bl2nam02on0053.outbound.protection.outlook.com ([104.47.38.53]:44104 "EHLO NAM02-BL2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752243AbdLNDbk (ORCPT ); Wed, 13 Dec 2017 22:31:40 -0500 Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=andrew.cooks@opengear.com; Subject: Re: [PATCH 2/2] i2c: fix piix4 aux port number To: Jean Delvare Cc: Wolfram Sang , linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org References: <5e47610fa40bb07b214d1409ae721a5d762eb888.1511391626.git.andrew.cooks@opengear.com> <20171207152958.61ca5ea0@endymion> From: Andrew Cooks Message-ID: Date: Thu, 14 Dec 2017 13:31:19 +1000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-Version: 1.0 In-Reply-To: <20171207152958.61ca5ea0@endymion> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [59.167.150.161] X-ClientProxiedBy: MEXPR01CA0091.ausprd01.prod.outlook.com (10.171.14.152) To BLUPR15MB0017.namprd15.prod.outlook.com (10.161.124.156) X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 695107b3-7528-4dfc-0770-08d542a32eaf X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:(4534020)(4602075)(4627115)(201703031133081)(201702281549075)(5600026)(4604075)(2017052603307);SRVR:BLUPR15MB0017; X-Microsoft-Exchange-Diagnostics: 1;BLUPR15MB0017;3:btm9MG1EWxui8nCxMDlJDEoQ8axaVRpWa0sajTBby6e5d2XRlfmo9a7TV0GjML1OUbovXNKzi+oeT3mGWTGeK0UA9dmi4KZvyQAiUsnn7VBC8p7i3hEZdhuNguU1hQXDO6rAn5fvZNwYCzlZni1r+dOWe/F1oxG7v+zvcgehJTZ0RpnyMVMySfhuEptUS0b9HpejxLIumhfh5l2CnXVfUmBvMNBY2GX7IZHFBRlaG9Ii2QWU/wlQij+hrGq4Djls;25:pP3rPC7LHHLMyYKAg3xSL5IWQPw1V1Y0zOJRyB1bRGkPFNDgixj6vhQtcoDrR7K94xnYx6DusT1CxbL2Jj2kqHEtTsBytKIweBUu2ZQNKCouZw1nb/k7xaTpclEatnbEP3EDrnt6lAo/AhQeNJFqurls4duDxQ0pFXlw+1jG/hSiD+gVi/eldHMpdt9IIGI3ixDFHy2cWKbPmZetjc22zSyW84fcV6/ZdfzEW0RqeYBamK9EqqchcLjfUSOSg2FBGv9PxCdJ3nRdHEPFvoHOBz6mbKjstkg1RabLtaRWz+iFJsAdcseeLGnUfS46LwISoRAUXnY4IkkBqJljt+FN3OMUphJcXs32mgY60mjOiQM=;31:aoaig17041bFc3AwhltlUZ8THMxcPEWpvFcPzyhM76D7gbPFgRnEABVXasX5uJ67Cix5gb8er/LvpdNTee+Yi9+iNqT3P8f7NOWSbvVznKo4vwJO9iw3HBtBPns1NNPP0BodAN1RX9xGijmOoOsIJvFD/uzYXSKKva+Q25CAkojnyufoPs6KAL6NPwWJHLNumYpmZy2y1cefNwRO/625MrowtszuaBY7i8vszQme15c= X-MS-TrafficTypeDiagnostic: BLUPR15MB0017: X-Microsoft-Exchange-Diagnostics: 1;BLUPR15MB0017;20:VbikktJBEzkieznIUbDdyEqLb7e7yanSqKZMLv+mZnJMME1TEaAlMmF1SEeinyUbQSPTafKnXWwc6Faw+8DF4bgdhhGsC/hTz3+HogJ0w5CQ8p+p73967ivd5shqv8qOkVRT1o4IHhQVgHlZZVxaKqbXeiDxMBj560iI0llg6Uo=;4:kxuWneB3rYKNPGNee0v8VQyRoUWYY/J5r9v15voaxndfzPV9BrhzYX+54tUWVOxmiuflRnzuQzaLMxMw5pBPBHiItv2op3/5wQmDi4L8kphzP8J3v5045OqTGOWgTx1nDvtQthXeZioB6v5Bu/aQ3qZEC6h4C4xqJYvpHJzJer453ZjdAEy+s5P377kgIngbSTagwUwGjj4z5i+TMEE/sAwiDvqUcrSWS83FtVwYs9qlKry9NmjOD2vs8hGxjmK8HG4UyCYOP2CB+uI4JJl0/VuW5CNT2uhzERgBCsNIZqi+rK43EBR+UHxPtlh+aNHE X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:(767451399110); X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(6040450)(2401047)(5005006)(8121501046)(93006095)(93001095)(10201501046)(3231023)(3002001)(6041248)(20161123562025)(20161123558100)(20161123560025)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(20161123564025)(20161123555025)(6072148)(201708071742011);SRVR:BLUPR15MB0017;BCL:0;PCL:0;RULEID:(100000803101)(100110400095);SRVR:BLUPR15MB0017; X-Forefront-PRVS: 05214FD68E X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10009020)(6049001)(39840400004)(376002)(396003)(366004)(346002)(189003)(51444003)(24454002)(199004)(5660300001)(31686004)(8936002)(6486002)(6246003)(81166006)(65826007)(2906002)(25786009)(16526018)(3846002)(64126003)(6306002)(68736007)(4326008)(77096006)(966005)(230700001)(316002)(305945005)(97736004)(83506002)(8676002)(117156002)(53546011)(31696002)(86362001)(6116002)(90366009)(53936002)(575784001)(2486003)(65956001)(23676004)(16576012)(65806001)(2950100002)(229853002)(36756003)(58126008)(7736002)(52116002)(76176011)(478600001)(105586002)(47776003)(6916009)(6666003)(59450400001)(50466002)(106356001)(52146003)(386003)(66066001)(81156014);DIR:OUT;SFP:1101;SCL:1;SRVR:BLUPR15MB0017;H:[192.168.254.67];FPR:;SPF:None;PTR:InfoNoRecords;MX:1;A:1;LANG:en; X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtCTFVQUjE1TUIwMDE3OzIzOnYvSHRxRWNyWWIwNWV0Ris4UkRyY3BBRUJ6?= =?utf-8?B?MTlVVHZWQVRNSUlsTGtadC9mSFV4bXFEa1RiTVlTUEhrVUVHN0JTM0JVRU9K?= =?utf-8?B?VlVCQytuUlpBY2VUOWRnWGVvSUQ4NHdsN1hhNlYxQWZoN3B4N2cyd2ZqVm1t?= =?utf-8?B?RUxlN2VSbHhpRkR5SmkvN1ZVb0VVRjQzMXB2Q0Rjd0FrYkRTaVp3REMyS3U0?= =?utf-8?B?UVphdW5DMmtUOThKYWZVcjVsMWFSOTB6Nis5ZkRwODlFQWlQTUw4cXpOU1Bt?= =?utf-8?B?bEROWVB6cUJIZm1sWVcwZGpPeTk0a0tXZzRBMjJuSk4wMzNqWUtQU1RLTkow?= =?utf-8?B?ajV4RW1aMUpKUzRKTlZQb1RFSlYzMEIyZk1YVkVpZGtGT096OXpYakdxT1BQ?= =?utf-8?B?a2RnNkUwazJLeGVaYm5xTGJELzVibWxpbytObU5LQXZGeXZCeFRoWU0vVk1v?= =?utf-8?B?N2oyeUYzU1NzaEtHbzBpY29TZHNySzE4WDNabnFZWlRVZHhyd0V3UktKcHVl?= =?utf-8?B?czJLemVpbEdiSTBCMEdaQ3NpMGE3bmlWQkZPYmJ4R1dIWDJ3Mjd5TGtiU0RT?= =?utf-8?B?NHVvTHlyZU1vQ3JUTjBsd2htdDFUVzcvdUdob2xjV0JpdXAyTmVVSHJUQ0NN?= =?utf-8?B?OG1tbHZwd3UyWVlGQVFob2hxdTh1cXNJMGNGOVNvYkpjN21FYkNnV2loeUV2?= =?utf-8?B?Wjl2Yko4bUhBNDNIbW9iOUs1Zlg0dVkyRmNlcVJ5dks2ZW5Xem4xTkxDMTF5?= =?utf-8?B?WGRXZ1VWNlFhMEhCZnlraWtiaEdabEhJbWlJYk03dUc1SUgrNG9nQlg2QlJL?= =?utf-8?B?ZUZ3akdvc3V3VWV2RGxpcXN4clF6RjNxY1A4aXdsOTRoN1JUTUM4c282K1BD?= =?utf-8?B?UVd3aVk4TnpzTmwybERoMldTSUQ4YmRPS3dZVGZkWjltSTQvK3RCRk5HZTA3?= =?utf-8?B?RGJ3SHN2dDZyR2tlaURxOUJJOWxvR2plbVYrUmdUUXNmRlN0K0xuV1BlUjR3?= =?utf-8?B?TmZSY0hqVDhDQVJWQ01VWllEcm8wVTRKZCszbE9VL0FiT0x3WXorUlE5Tkl2?= =?utf-8?B?bkpvTlNXQUh5dUN6cnM1YUttNXRBa2xmZy9aSnMyZ1pSVkNEWDd4dXAxanRn?= =?utf-8?B?RG5TYlovaDdsaGtkbWxKZ3YrRTZ1WUk4cm91OW1WMnlZS2JZNmhXSEFtU1Mv?= =?utf-8?B?NlorUGpWa2VXbFN0cG9XelVUU0E2a25CeUs1RlRnVFNMUHlITjB2Ri9OdGhq?= =?utf-8?B?REdxUEVYei92bDZBVjRIMkVOM0tCNnlKc3hLTW9kZ0dDOVQwOFB4bk8wNGlK?= =?utf-8?B?NG1weFpMU2NWTEcwSWpJZ0p4V0ZZbmROcHBMZENidDdKK1VkM3MxaHBqZjNN?= =?utf-8?B?cURWUWtJbm5CMllCZzYwbEFvTklKZ2R2cExRNUswK0hSRUVUdkJUTnczUi8v?= =?utf-8?B?cERYTExxVDdRdW9WNXplYlRKMGtFdXVtSjFZZndHYzdETUZMZS9Eb0k0aDkz?= =?utf-8?B?VlpSYi8xMENRR2NlbW5rYW9TRGd5U212NWFQRDl1ZHdhYkg5TmtpaTZSMGZu?= =?utf-8?B?aFBoUzkwYzlkVHVuTUF6cE9admxSNHlreCsyN3lMTVEzb2tieUpVZlRIYnJJ?= =?utf-8?B?WDU2cm1odUhBbms3ckFiMU9pZ1Jzdy9WVDloNVZ3OGJxdzVrNzBuY0F1WlZv?= =?utf-8?B?MXd3S1JyRXBab2MyRmllN0oyM3hoNkVBbWplVXlER2tvWlBwaWlBREdsY3FJ?= =?utf-8?B?ZDlKUW5RYjdKT0Y1K1BUZWsyZEVoZUV4eWYvbXNWRHlVNjhKOXBFMkFpejZI?= =?utf-8?B?OVVpUXRUNXl3WmhyOFVjKzZSb2Zjc0pXbEFFRityVW0vWUxCZG1nMmZvUXhp?= =?utf-8?B?VFNiWC95NkZxVG9iZnJPdUk4OVFEVVB2V1JzeDljZHRjZXBseHdUSDd5RE5Z?= =?utf-8?B?aWpDOG9mRktmeUNBbjdzb1ZpS1hrN1Q3bUR0NmxzKzJTa1FRMzBkazMzV2xM?= =?utf-8?Q?5xpjz+?= X-Microsoft-Exchange-Diagnostics: 1;BLUPR15MB0017;6:N9T1tVEYjywTSxj2H7NZ6BeWBEBy2vODC+6tEto1bzkqIguBg0EyrUBWS8Ns+A2E7XmuyPUWP32mdNrat3tFaeziLR82RvZstFc6WD/hN800AKO1MOy6cUlSPwZ1d15VotFnHQTCMdFXAtz9FsXGtBtKNQIPRbDbJKAjZWzA/MVXqJKOBvm9BMw+aKGxqCmH98LD7zKEI56UaSvTl1u9HFUQszRgnmgQl6IZvAX3H4mDhtJRFTgvLXC0NAP3SY4EH8SbUKBBrjacHwdRoriQyPtFGV86/z+JEfqr1JPFF8ED3CHCQuQyltt6FkRToqjcGrDbNJd58B2I/xQbW1oIrFsrKXJU2m1ebh/I2Msudh8=;5:vjt+YblFwbHevKPs1vR9yBAUcXNjBNp9U2nHoesKaxCl667w7IvHt6KbuieqAsCzh4xJborYZ2BOfDv6Q/tOumPqvfQExb5w1TL/uAXmO1lP+I1Vy0e8jOICnJ2lur7Er2nNVxMLDCfZUTc7BA1xJ3Eb1GMFTFhgJhiwReR8p6M=;24:ujLtnhQ9Tc8WzkyHofvIVe3cwg1zw3+1T+JrmAvgPdpYgVuhu6cxCrCOM5fLyKIfcxKh5DV65MD/8Squbi4nvydw0nMD9M2xxzdfdnZY1ws=;7:B6BDHQmxOP/bEOjmjSWIZUc9OJppReGMugxQLUXHx6xk8AkiNwXWxVo+NhLqWPMeWzlWPMYh1EC98oDAUDDZ9SkAn3Tmiqy5fSHaMtY3UtZ5sxNv7BTtclc5X7KC6Qv+2bgdhordkb08FQE0j4CEi98mhWUuLXRsy36xYdVUXXupfPCRSCtUhk+uaN+EOQ+muFxljlZzNC6rcFY8fE6EIZ3WgVrPekY3f6uhV0YWPS3745i/6NxekjCyUW+w1cuw SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: opengear.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 Dec 2017 03:31:35.6086 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 695107b3-7528-4dfc-0770-08d542a32eaf X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: a6251c26-d21f-4164-a225-1f4eaebf5f9a X-MS-Exchange-Transport-CrossTenantHeadersStamped: BLUPR15MB0017 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Jean On 08/12/17 00:29, Jean Delvare wrote: > Hi Andrew, > > On Thu, 23 Nov 2017 13:09:38 +1000, Andrew Cooks wrote: >> Let the aux port use port number one (not zero), to match the AMD >> documentation and enable mapping ACPI _ADR to port number. >> >> This fixes ACPI-based enumeration of I2C slave peripherals that are >> defined for the aux SMBus port. >> >> Signed-off-by: Andrew Cooks >> --- >> drivers/i2c/busses/i2c-piix4.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c >> index 9260cfa..f980f0b 100644 >> --- a/drivers/i2c/busses/i2c-piix4.c >> +++ b/drivers/i2c/busses/i2c-piix4.c >> @@ -975,7 +975,7 @@ static int piix4_probe(struct pci_dev *dev, const struct pci_device_id *id) >> if (retval > 0) { >> /* Try to add the aux adapter if it exists, >> * piix4_add_adapter will clean up if this fails */ >> - piix4_add_adapter(dev, retval, false, 0, false, >> + piix4_add_adapter(dev, retval, false, 1, false, >> is_sb800 ? piix4_aux_port_name_sb800 : "", >> &piix4_aux_adapter); >> } > > The port number has consequences. In piix4_adap_remove, port 0 is > handled differently. We assume that for each controller (main or aux) > exactly one adapter has port number 0. Your change above breaks this > assumption. I see the problem, thanks. > > Also, if the port numbers are supposed to match the documentation, and > the aux controller is port 1, then I wonder how are the muxed ports of > the main controller numbered? The driver numbers them from 1 to 3, but > I guess the documentation numbers them from 2 to 4 to avoid colliding > with the aux controller? I have vague memories of discussion this > before... If it is important that aux port number matches the > documentation then I guess the same holds for the muxed ports on the > main controller. The documentation[1][2][3] uses labels Port 0 (00b), Port 2 (01b), Port 3 (10b), Port 4 (11b). This led me down a rabbit hole of thinking that port 1 is intended to be the aux controller, but now I think that the labels in the documentation is unfortunate, because the labels don't match the binary values. (If 01b had been reserved for Port 1 it would've been fine, but it's not.) Matching the adapter order of the documentation would have been nice, because ACPI developers rely on that as a reference. If the adapter ordering is different between ACPI and the driver, the peripherals will be mapped to the incorrect adapter. In my particular case I can fix the ACPI DSDT to match the enumeration order of the driver right now, but I won't be able to change it easily. Similarly, there might be other users who need a different enumeration order and who aren't able to change their ACPI tables as easily. > > If we number muxed ports from 2 to 4 then the test in piix4_adap_remove > could be simply changed to check for adapdata->port <= 1. I think the existing piix4_adap_remove code is correct and that using the port in the way I initially proposed is incorrect. > > Please note that I just sent a fix for this specific function, so any > patch touching the same area should go on top of it. I'll bounce it to > you. > I've sent a version 2 of the patch set, based on top of that fix. Thanks! Andrew [1] http://support.amd.com/TechDocs/52740_16h_Models_30h-3Fh_BKDG.pdf [2] http://support.amd.com/TechDocs/55072_AMD_Family_15h_Models_70h-7Fh_BKDG.pdf [3] http://support.amd.com/TechDocs/50742_15h_Models_60h-6Fh_BKDG.pdf