From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43865) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b51tP-0002eY-77 for qemu-devel@nongnu.org; Mon, 23 May 2016 22:16:24 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b51tK-0007nW-5b for qemu-devel@nongnu.org; Mon, 23 May 2016 22:16:22 -0400 Received: from mx1.redhat.com ([209.132.183.28]:42451) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b51tJ-0007nS-TB for qemu-devel@nongnu.org; Mon, 23 May 2016 22:16:18 -0400 References: <573D3799.9090109@redhat.com> <573E0276.8040002@tribudubois.net> <573E77BE.4090609@redhat.com> <573F804B.4000608@tribudubois.net> From: Jason Wang Message-ID: <5743B96A.9060408@redhat.com> Date: Tue, 24 May 2016 10:16:10 +0800 MIME-Version: 1.0 In-Reply-To: <573F804B.4000608@tribudubois.net> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v4 7/8] Add ENET/Gbps Ethernet support to FEC device List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jean-Christophe DUBOIS , qemu-devel@nongnu.org, peter.maydell@linaro.org On 2016=E5=B9=B405=E6=9C=8821=E6=97=A5 05:23, Jean-Christophe DUBOIS wrot= e: > Le 20/05/2016 04:34, Jason Wang a =C3=A9crit : >> >> >> On 2016=E5=B9=B405=E6=9C=8820=E6=97=A5 02:14, Jean-Christophe DUBOIS w= rote: >>> Le 19/05/2016 05:48, Jason Wang a =C3=A9crit : >>>> >>>> >>>> On 2016=E5=B9=B405=E6=9C=8819=E6=97=A5 06:23, Jean-Christophe Dubois= wrote: >>>>> The ENET device (present in i.MX6) is "derived" from FEC and backwa= rd >>>>> compatible with it. >>>>> >>>>> This patch adds the necessary support of the added feature in the=20 >>>>> ENET >>>>> device to allow Linux to use it (on supported processors). >>>>> >>>>> Signed-off-by: Jean-Christophe Dubois >>>>> --- >>>>> >>>>> Changes since v1: >>>>> * Not present on v1 >>>>> Changes since v2: >>>>> * Not present on v2 >>>>> Changes since v3: >>>>> * Separate and fix the 2 supported interrupts >>>>> >>>>> hw/arm/fsl-imx25.c | 3 + >>>>> hw/net/imx_fec.c | 713=20 >>>>> ++++++++++++++++++++++++++++++++++++++--------- >>>>> include/hw/net/imx_fec.h | 195 ++++++++++--- >>>>> 3 files changed, 745 insertions(+), 166 deletions(-) >>>> >>>> [...] >>>> >>>>> -static Property imx_fec_properties[] =3D { >>>>> +static Property imx_eth_properties[] =3D { >>>>> DEFINE_NIC_PROPERTIES(IMXFECState, conf), >>>>> + DEFINE_PROP_BOOL("is-fec", IMXFECState, is_fec, false), >>>>> DEFINE_PROP_END_OF_LIST(), >>>>> }; >>>> >>>> It's ok to decide with "is-fec", but is it better to use a new type=20 >>>> for that? >>> >>> Well, there is a lot of common code between FEC and ENET because=20 >>> ENET is basically backward compatible with FEC. So most/all of the=20 >>> FEC code needs to go in the ENET device. >>> >>> I thought this way of doing thing was the best way to avoid=20 >>> duplicating things. >> >> You can still share almost all the codes. E.g you can have a look at=20 >> e1000.c which have 3 types of rather similar devices. > > OK, I'll change it in next patch to match the pl110 model as proposed=20 > by Peter. No more additional property... > >> >> Another question not relate to this patch, I saw version were bumped=20 >> for vmstate version. Is it better to keep the vmstate for FEC and=20 >> using a new vmstate for ENET (register array). > > Well, as I moved the FEC to register array, the VMState structure has=20 > changed and therefore I believe the vmstate version needs to be bumped. > > Am I wrong?=20 You're right. But migration to old version does not work in this way,=20 even if there aren't any changes in FEC. I'm not sure the policy for arm, but we used to do lots of works in the=20 past to make migration to older version works. You can achieve this by using two different vmstates, and keep the FEC's=20 state (by just use parts of array). Perter, any thoughts on this? Should we try to keep the migration=20 compatibility here? Thanks