Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933336AbdCaMvh (ORCPT ); Fri, 31 Mar 2017 08:51:37 -0400 Received: from bh-25.webhostbox.net ([208.91.199.152]:34076 "EHLO bh-25.webhostbox.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933317AbdCaMt5 (ORCPT ); Fri, 31 Mar 2017 08:49:57 -0400 Subject: Re: [Regression] Changes to i2c-piix4.c initialisation prevent loading of sp5100_tco watchdog driver on AMD SB800 chipset To: Paul Menzel , Wolfram Sang , Zoltan Boszormenyi References: <1485728348.3220.10.camel@googlemail.com> <1488530782.2457.41.camel@users.sourceforge.net> <20170303101702.GA1669@katana> <1490944639.2653.182.camel@users.sourceforge.net> Cc: Christian Fetzer , Jean Delvare , linux-i2c@vger.kernel.org, linux-watchdog@vger.kernel.org, 853122@bugs.debian.org, Wim Van Sebroeck , Tim Small , Nehal Shah , Mika Westerberg , Andy Shevchenko , Thomas Brandon , Eddi De Pieri , linux-kernel@vger.kernel.org From: Guenter Roeck Message-ID: <585a27fb-be15-df9f-00a9-bd4c3a654142@roeck-us.net> Date: Fri, 31 Mar 2017 05:49:50 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1 MIME-Version: 1.0 In-Reply-To: <1490944639.2653.182.camel@users.sourceforge.net> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-Authenticated_sender: linux@roeck-us.net X-OutGoing-Spam-Status: No, score=-1.0 X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - bh-25.webhostbox.net X-AntiAbuse: Original Domain - vger.kernel.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - roeck-us.net X-Get-Message-Sender-Via: bh-25.webhostbox.net: authenticated_id: linux@roeck-us.net X-Authenticated-Sender: bh-25.webhostbox.net: linux@roeck-us.net X-Source: X-Source-Args: X-Source-Dir: Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2552 Lines: 66 Hi Paul, On 03/31/2017 12:17 AM, Paul Menzel wrote: > Dear Wolfram, > > > Thank you for the reply, which we talked about briefly at the > Chemnitzer LinuxTage. > > > Am Freitag, den 03.03.2017, 11:17 +0100 schrieb Wolfram Sang: >>> Unfortunately, commit 2fee61d22e (i2c: piix4: Add support for >>> multiplexed main adapter in SB800) [1] caused a regression. Tim >>> reported that to the Linux Kernel Bugtracker as bug #170741 last >>> September [2], but it looks like the affected subsystems don’t use it. >> >> Jean Delvare pointed out this issue amongst others[1] last year already. >> Let me quote: >> >> === >> >> 5* The I/O ports used for SMBus configuration and port switching are >> also needed by a watchdog driver, sp5100_tco. Both drivers request the >> region, so the first one wins, and the other driver can't be loaded. >> sp5100_tco was there first, so the changes done to the i2c-piix4 driver >> recently will cause a regression for some users by preventing them >> from using the sp5100_tco and i2c-piix4 drivers at the same time. In >> the long run I guess we will need a helper module to handle this shared >> resource. Unless IORESOURCE_MUXED can be used for that. Either way, >> that's more work than I can put into this before kernel v4.5 is >> released. For the time being, I think we should simply make it >> non-fatal if the I/O ports can't be requested, and continue without >> multiplexing (as before.) >> >> === >> >> Seems nobody had the resources, so far. > > I still don’t understand, why Jean then not immediately reverted the > commit to adhere to the Linux Kernel’s no-regression-policy. > >> I don't have the HW and not much experience with non-embedded >> platforms. I wonder, though, if we really need to convert the drivers >> to MFD ones, or if we could use the simpler MFD_SYSCON mechanism >> which helps in exactly such cases for embedded platforms. But I am >> really lacking details here and am afraid this is probably all the >> input I can give currently. > > Zoltan stepped up, and uploaded a patch for review to the Kernel.org > Bugzilla [2], also attached to this message. > Please don't send patches as attachments. request_muxed_region() can fail, and literally every other driver using it checks for that failure. Please do the same. The sp5100_tco_dev_name change in the watchdog driver is unnecessary. There are some unnecessary { } in the watchdog driver after the patch is applied. Please split the patch into two patches so they can be reviewed and applied separately. Thanks, Guenter