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;
Date:   Fri, 22 May 2020 20:03:41 +0200 (CEST)
From:   =?utf-8?Q?Cl=C3=A9ment?= Leger <cleger@kalray.eu>
To:     Bjorn Andersson <bjorn.andersson@linaro.org>
Cc:     s-anna <s-anna@ti.com>, Rob Herring <robh+dt@kernel.org>,
        Mathieu Poirier <mathieu.poirier@linaro.org>,
        Loic PALLARDY <loic.pallardy@st.com>,
        Arnaud Pouliquen <arnaud.pouliquen@st.com>,
        Lokesh Vutla <lokeshvutla@ti.com>,
        linux-remoteproc <linux-remoteproc@vger.kernel.org>,
        devicetree <devicetree@vger.kernel.org>,
        linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
        linux-kernel <linux-kernel@vger.kernel.org>
Message-ID: <1739080680.4218297.1590170621467.JavaMail.zimbra@kalray.eu>
In-Reply-To: <20200522173346.GJ11847@yoga>
References: <20200325204701.16862-1-s-anna@ti.com> <20200325204701.16862-4-s-anna@ti.com> <20200521180417.GJ408178@builder.lan> <997d6f9a-64ba-7a89-e909-9a5a474120b0@ti.com> <f15b0c6d-eee8-b839-0c79-a5316dbbfa7b@ti.com> <20200522173346.GJ11847@yoga>
Subject: Re: [PATCH 3/4] remoteproc: add support for a new 64-bit trace
 version
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Thread-Topic: remoteproc: add support for a new 64-bit trace version
Thread-Index: L30XW51sgsk7NRKU7I+TV4VplJ8G6Q==
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Suman,

----- On 22 May, 2020, at 19:33, Bjorn Andersson bjorn.andersson@linaro.org=
 wrote:

> On Fri 22 May 09:54 PDT 2020, Suman Anna wrote:
>=20
>> On 5/21/20 2:42 PM, Suman Anna wrote:
>> > Hi Bjorn,
>> >=20
>> > On 5/21/20 1:04 PM, Bjorn Andersson wrote:
>> > > On Wed 25 Mar 13:47 PDT 2020, Suman Anna wrote:
> [..]
>> > > > diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc=
.h
> [..]
>> > > > +struct fw_rsc_trace2 {
>> > >=20
>> > > Sounds more like fw_rsc_trace64 to me - in particular since the vers=
ion
>> > > of trace2 is 1...
>> >=20
>> > Yeah, will rename this.
>> >=20
>> > >=20
>> > > > +=C2=A0=C2=A0=C2=A0 u32 padding;
>> > > > +=C2=A0=C2=A0=C2=A0 u64 da;
>> > > > +=C2=A0=C2=A0=C2=A0 u32 len;
>> > > > +=C2=A0=C2=A0=C2=A0 u32 reserved;
>> > >=20
>> > > What's the purpose of this reserved field?
>> >=20
>> > Partly to make sure the entire resource is aligned on an 8-byte, and
>> > partly copied over from fw_rsc_trace entry. I guess 32-bits is already
>> > large enough of a size for trace entries irrespective of 32-bit or
>> > 64-bit traces, so I doubt if we want to make the len field also a u64.
>>=20
>> Looking at this again, I can drop both padding and reserved fields, if I
>> move the len field before da. Any preferences/comments?

Not only the in structure alignment matters but also in the resource table.
Since the resource table is often packed (see [1] for instance), if a


[1] https://github.com/OpenAMP/open-amp/blob/master/apps/machine/zynqmp_r5/=
rsc_table.h

>>=20
>=20
> Sounds good to me.
>=20
> Thanks,
> Bjorn