Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [RFC PATCH 1/5] arm64: Move instruction encoder/decoder under
 lib/
To:     Mark Rutland <mark.rutland@arm.com>
Cc:     linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        catalin.marinas@arm.com, will@kernel.org, broonie@kernel.org
References: <20210120171745.1657762-1-jthierry@redhat.com>
 <20210120171745.1657762-2-jthierry@redhat.com>
 <20210202101755.GB59049@C02TD0UTHF1T.local>
From:   Julien Thierry <jthierry@redhat.com>
Message-ID: <9c01f293-2c98-43e6-a497-89610fe5970e@redhat.com>
Date:   Wed, 3 Feb 2021 09:26:45 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.11.0
MIME-Version: 1.0
In-Reply-To: <20210202101755.GB59049@C02TD0UTHF1T.local>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hi Mark,

On 2/2/21 11:17 AM, Mark Rutland wrote:
> Hi Julien,
> 
> On Wed, Jan 20, 2021 at 06:17:41PM +0100, Julien Thierry wrote:
>> Aarch64 instruction set encoding and decoding logic can prove useful
>> for some features/tools both part of the kernel and outside the kernel.
>>
>> Isolate the function dealing only with encoding/decoding instructions,
>> with minimal dependency on kernel utilities in order to be able to reuse
>> that code.
>>
>> Code was only moved, no code should have been added, removed nor
>> modifier.
>>
>> Signed-off-by: Julien Thierry <jthierry@redhat.com>
> 
> This looks sound, but the diff is a little hard to check.
> 

Yes, I was expecting this change to be hard to digest.

> Would it be possible to split this into two patches, where:
> 
> 1) Refactoring definitions into insn.h and insn.c, leaving those files
>     in their current locations.
> 

I'm not quite sure I understand the suggestions. After this patch insn.h 
and insn.c still contain some definitions that can't really be used 
outside of kernel code which is why I split them into insn.* and 
aarch64-insn.* (the latter containing what could be used by tools).

Or do you suggest that I still create the aarch64-insn.* files next to 
the insn.* files?


> 2) Moving insn.h and insn.c out to arch/arm64/lib/, updating includes
> > With that, the second patch might be easier for git to identify as a
> rename (if using `git format-patch -M -C`), and it'd be easier to see
> that there are no unintended changes.
> 

But it is more a split than a rename (at least in this patch). But I'm 
happy to do this in another way.

Thanks,

-- 
Julien Thierry