Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753280AbdHWC4K (ORCPT ); Tue, 22 Aug 2017 22:56:10 -0400 Received: from mail-wr0-f177.google.com ([209.85.128.177]:36230 "EHLO mail-wr0-f177.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753057AbdHWC4I (ORCPT ); Tue, 22 Aug 2017 22:56:08 -0400 MIME-Version: 1.0 In-Reply-To: <20170823103420.6ffb5ce7@xhacker> References: <20170808110305.2748-1-jszhang@marvell.com> <20170823015108.GA18228@kroah.com> <20170823103420.6ffb5ce7@xhacker> From: John Stultz Date: Tue, 22 Aug 2017 19:56:07 -0700 Message-ID: Subject: Re: [PATCH] binder: let ANDROID_BINDER_IPC_32BIT be selectable on 32bit ARM To: Jisheng Zhang Cc: Greg KH , =?UTF-8?B?QXJ2ZSBIasO4bm5ldsOlZw==?= , Riley Andrews , devel@driverdev.osuosl.org, lkml , "linux-arm-kernel@lists.infradead.org" Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1946 Lines: 51 On Tue, Aug 22, 2017 at 7:34 PM, Jisheng Zhang wrote: > On Tue, 22 Aug 2017 18:51:08 -0700 Greg KH wrote: > >> On Tue, Aug 08, 2017 at 07:03:05PM +0800, Jisheng Zhang wrote: >> > As noted in commit d0bdff0db809 ("staging: Fix build issues with new >> > binder API"), we can add back the choice for 32bit ARM "once a 64bit >> > __get_user_asm_* implementation is merged." Commit e38361d032f1 ("ARM: >> > 8091/2: add get_user() support for 8 byte types") has added the >> > support, so it's time to let ANDROID_BINDER_IPC_32BIT be selectable on >> > 32bit ARM >> >> Ok, but: >> >> > >> > Signed-off-by: Jisheng Zhang >> > --- >> > drivers/android/Kconfig | 2 +- >> > 1 file changed, 1 insertion(+), 1 deletion(-) >> > >> > diff --git a/drivers/android/Kconfig b/drivers/android/Kconfig >> > index 832e885349b1..aca5dc30b97b 100644 >> > --- a/drivers/android/Kconfig >> > +++ b/drivers/android/Kconfig >> > @@ -32,7 +32,7 @@ config ANDROID_BINDER_DEVICES >> > therefore logically separated from the other devices. >> > >> > config ANDROID_BINDER_IPC_32BIT >> > - bool >> > + bool "Use old (Android 4.4 and earlier) 32-bit binder API" >> > depends on !64BIT && ANDROID_BINDER_IPC >> >> You don't actually change the depends line :( >> >> Please fix up, and test it, and then resend. > > IHOM, the dependency is correct: 64bit platforms don't support > ANDROID_BINDER_IPC_32BIT. What do you think? I think this indicates the commit message is unclear. Part of it is that the config is inverted from the description. The patch doesn't enable the 32bit legacy binder ABI on 32bit systems, it just allows the option to be unselected, so that the 64bit ABI will be used on 32bit systems. Conceptually I don't have an objection to the change (though maybe try to rework the commit message), but I don't have anything to actually test it on right now, so I'm hesitant to ack it. thanks -john