From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from kvm5.telegraphics.com.au (kvm5.telegraphics.com.au [98.124.60.144]) by lists.ozlabs.org (Postfix) with ESMTP id 410zxp1YbXzDqG0 for ; Wed, 6 Jun 2018 16:57:17 +1000 (AEST) Date: Wed, 6 Jun 2018 16:57:31 +1000 (AEST) From: Finn Thain To: Geert Uytterhoeven cc: Benjamin Herrenschmidt , Michael Schmitz , linuxppc-dev , linux-m68k , Linux Kernel Mailing List Subject: Re: [PATCH 08/11] macintosh/via-pmu: Replace via-pmu68k driver with via-pmu driver In-Reply-To: Message-ID: References: <7ce9a8c3996506ae6f9ab16b3cf73c287e205a89.1527909627.git.fthain@telegraphics.com.au> <8597ac466215f573d8bea40e74fdec2bf72fabbb.1527909627.git.fthain@telegraphics.com.au> <878b389293851359e53de186bf302ce52d95ad95.1527909627.git.fthain@telegraphics.com.au> <166b4c3a7347fe27f12bda86d50533cd01ee11b9.1527909627.git.fthain@telegraphics.com.au> <73507a3e9444da17c222cf74df41842ce30d15ee.1527909627.git.fthain@telegraphics.com.au> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On Mon, 4 Jun 2018, Geert Uytterhoeven wrote: > > > Don't call pmu_shutdown() or pmu_restart() on early PowerBooks: the > > PMU device found in these PowerBooks isn't supported. > > Shouldn't that be a separate patch? > > > --- a/arch/m68k/mac/misc.c > > +++ b/arch/m68k/mac/misc.c > > > @@ -477,9 +445,8 @@ void mac_poweroff(void) > > macintosh_config->adb_type == MAC_ADB_CUDA) { > > cuda_shutdown(); > > #endif > > -#ifdef CONFIG_ADB_PMU68K > > - } else if (macintosh_config->adb_type == MAC_ADB_PB1 > > - || macintosh_config->adb_type == MAC_ADB_PB2) { > > +#ifdef CONFIG_ADB_PMU > > + } else if (macintosh_config->adb_type == MAC_ADB_PB2) { > > pmu_shutdown(); > > #endif > > } > > @@ -519,9 +486,8 @@ void mac_reset(void) > > macintosh_config->adb_type == MAC_ADB_CUDA) { > > cuda_restart(); > > #endif > > -#ifdef CONFIG_ADB_PMU68K > > - } else if (macintosh_config->adb_type == MAC_ADB_PB1 > > - || macintosh_config->adb_type == MAC_ADB_PB2) { > > +#ifdef CONFIG_ADB_PMU > > + } else if (macintosh_config->adb_type == MAC_ADB_PB2) { > > pmu_restart(); > > #endif > > } else if (CPU_IS_030) { > The stability problem here is bigger than just pmu_restart() and pmu_shutdown(), so those hunks are insufficient. You need to prevent the via-pmu68k driver from loading in the first place and to drop all the PMU_68K_V1 cases. But splitting this patch in that way creates potential merge conflicts, which is a hassle. E.g. this hunk: - .... - switch (macintosh_config->adb_type) { - case MAC_ADB_PB1: - pmu_kind = PMU_68K_V1; - break; - case MAC_ADB_PB2: - pmu_kind = PMU_68K_V2; - break; - default: - pmu_kind = PMU_UNKNOWN; - return -ENODEV; - } - ... would get split over two patches. The way I see it, having no PMU driver for PMU_68K_V1 machines is a bug. And loading any of the existing PMU drivers on that hardware is also a bug. These bugs are equivalent in that either one means you can't use the keyboard, trackpad etc. So there's no value in cherry-picking the other bug. If you are using v4.17 on a PMU_68K_V1 machine, you probably already have CONFIG_ADB_PMU68K=n. With that config, here's nothing to be gained from bisecting these changes. Does that make sense? Did I overlook other possible reason(s) to split up this patch? --