From mboxrd@z Thu Jan 1 00:00:00 1970 From: Linus Torvalds Subject: Re: [RFC/RFT PATCH 0/6] Switch GHES ioremap_page_range() to use fixmap Date: Tue, 31 Oct 2017 08:52:58 -0700 Message-ID: References: <20171031153832.17746-1-james.morse@arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Return-path: Received: from mail-io0-f194.google.com ([209.85.223.194]:51553 "EHLO mail-io0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751295AbdJaPw7 (ORCPT ); Tue, 31 Oct 2017 11:52:59 -0400 In-Reply-To: <20171031153832.17746-1-james.morse@arm.com> Sender: linux-acpi-owner@vger.kernel.org List-Id: linux-acpi@vger.kernel.org To: James Morse Cc: Linux ACPI , Linux Kernel Mailing List , "linux-arm-kernel@lists.infradead.org" , the arch/x86 maintainers , Catalin Marinas , Will Deacon , Thomas Gleixner , Ingo Molnar , "H . Peter Anvin" , Borislav Petkov , "Rafael J . Wysocki" , Len Brown , Tony Luck , Tyler Baicar , Dongjiu Geng , Xie XiuQi On Tue, Oct 31, 2017 at 8:38 AM, James Morse wrote: > 7 files changed, 30 insertions(+), 85 deletions(-) Lovely. I obviously can't test it, but it looks fine. I *would* suggest just making the "add fixmap entries" commits with the code that actually uses them. There's no real reason to have two commits that just add two entries that aren't used yet. If it was some meaningful helper function where a split of the commits makes each commit easier to read, that would be one thing. As it is, the split just makes it harder to look at the history of the code (ie "I wonder where this was introduced - let's use 'git blame'. Oh, that's not useful"). Linus From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932413AbdJaPxB (ORCPT ); Tue, 31 Oct 2017 11:53:01 -0400 Received: from mail-io0-f194.google.com ([209.85.223.194]:51553 "EHLO mail-io0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751295AbdJaPw7 (ORCPT ); Tue, 31 Oct 2017 11:52:59 -0400 X-Google-Smtp-Source: ABhQp+Qck46MGfiwpDKCWH2tvNdEDIg/rJ92nwgcJKJLxUWi6WSZMo2Hp0OM4wfAo/+ZqPI2EkYpNIE4Jac86H7n4Pc= MIME-Version: 1.0 In-Reply-To: <20171031153832.17746-1-james.morse@arm.com> References: <20171031153832.17746-1-james.morse@arm.com> From: Linus Torvalds Date: Tue, 31 Oct 2017 08:52:58 -0700 X-Google-Sender-Auth: g5HCX7WBrg1f7vFvX0cT14N6mFY Message-ID: Subject: Re: [RFC/RFT PATCH 0/6] Switch GHES ioremap_page_range() to use fixmap To: James Morse Cc: Linux ACPI , Linux Kernel Mailing List , "linux-arm-kernel@lists.infradead.org" , "the arch/x86 maintainers" , Catalin Marinas , Will Deacon , Thomas Gleixner , Ingo Molnar , "H . Peter Anvin" , Borislav Petkov , "Rafael J . Wysocki" , Len Brown , Tony Luck , Tyler Baicar , Dongjiu Geng , Xie XiuQi Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Oct 31, 2017 at 8:38 AM, James Morse wrote: > 7 files changed, 30 insertions(+), 85 deletions(-) Lovely. I obviously can't test it, but it looks fine. I *would* suggest just making the "add fixmap entries" commits with the code that actually uses them. There's no real reason to have two commits that just add two entries that aren't used yet. If it was some meaningful helper function where a split of the commits makes each commit easier to read, that would be one thing. As it is, the split just makes it harder to look at the history of the code (ie "I wonder where this was introduced - let's use 'git blame'. Oh, that's not useful"). Linus From mboxrd@z Thu Jan 1 00:00:00 1970 From: torvalds@linux-foundation.org (Linus Torvalds) Date: Tue, 31 Oct 2017 08:52:58 -0700 Subject: [RFC/RFT PATCH 0/6] Switch GHES ioremap_page_range() to use fixmap In-Reply-To: <20171031153832.17746-1-james.morse@arm.com> References: <20171031153832.17746-1-james.morse@arm.com> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Tue, Oct 31, 2017 at 8:38 AM, James Morse wrote: > 7 files changed, 30 insertions(+), 85 deletions(-) Lovely. I obviously can't test it, but it looks fine. I *would* suggest just making the "add fixmap entries" commits with the code that actually uses them. There's no real reason to have two commits that just add two entries that aren't used yet. If it was some meaningful helper function where a split of the commits makes each commit easier to read, that would be one thing. As it is, the split just makes it harder to look at the history of the code (ie "I wonder where this was introduced - let's use 'git blame'. Oh, that's not useful"). Linus