From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 03C86C433F5 for ; Mon, 17 Jan 2022 14:23:29 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id A6B1880F9E; Mon, 17 Jan 2022 15:23:27 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=kernel.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=kernel.org header.i=@kernel.org header.b="Q3myDfBS"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id C599C80960; Mon, 17 Jan 2022 15:23:26 +0100 (CET) Received: from ams.source.kernel.org (ams.source.kernel.org [IPv6:2604:1380:4601:e00::1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 49D7D81430 for ; Mon, 17 Jan 2022 15:23:22 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=kernel.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=kabel@kernel.org Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id CDABFB8108F; Mon, 17 Jan 2022 14:23:21 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id CE03CC36AE7; Mon, 17 Jan 2022 14:23:18 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1642429400; bh=/q0ul8qJlF2LZsGrMMyvwYyJ3UL0+aChfhJieB1i8CI=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=Q3myDfBSAqyVO7Zg9tpIhOI8F9QEDTqOmEHYiSKhK0RbrUevKj1W2M6f7xoF1OL4n n2sjvoVHIqq4ATnPGTh0Dp//cfqXOcdFknGp1ulrgJbqCrsj2GNytoMvNZygO6yGZO KT3oYqzByVggOzUME3tBJYIlk/OB19lx2Pdlh4dGhBU2FUxBpWg1L8mnvbtVZt1Y8L oBzTl0/WcSVGsBGPYWm7LI7bVdInsLSXH/IPtu3+wl9saB8FFPo81sE1zWpeOu78Vj QiFDELV0p2b6PwO8MFeYu0WMDF0oQ8zAnqSqdZAnZPVHwfVm0TEwXfF51Ao4duzJEO 3MRP/69dARsSw== Date: Mon, 17 Jan 2022 15:23:15 +0100 From: Marek =?UTF-8?B?QmVow7pu?= To: Moti Buskila Cc: Stefan Roese , Mario Six , Dennis Gilmore , Kostya Porotchkin , Pali =?UTF-8?B?Um9ow6Fy?= , "u-boot@lists.denx.de" , Marek =?UTF-8?B?QmVow7pu?= Subject: Re: [EXT] [PATCH u-boot-marvell] ddr: marvell: a38x: fix SPLIT_OUT_MIX state decision Message-ID: <20220117152315.1315d7ce@thinkpad> In-Reply-To: References: <20220112160659.6213-1-kabel@kernel.org> X-Mailer: Claws Mail 3.18.0 (GTK+ 2.24.33; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.2 at phobos.denx.de X-Virus-Status: Clean Hello Moti, since you're the author of the original version of this patch, could you please review it and if it is okay, put it into mv-ddr-marvell? Thanks. Marek On Mon, 17 Jan 2022 06:52:08 +0000 Moti Buskila wrote: > Hi Assaf, > I've got this email a few days ago. > Is it related to what you=E2=80=99ve send me?=20 > Thanks >=20 >=20 > -----Original Message----- > From: Marek Beh=C3=BAn =20 > Sent: Wednesday, January 12, 2022 6:07 PM > To: Stefan Roese ; Mario Six ; Dennis Gil= more ; Kostya Porotchkin > Cc: Pali Roh=C3=A1r ; u-boot@lists.denx.de; Moti Buskila= ; Marek Beh=C3=BAn > Subject: [EXT] [PATCH u-boot-marvell] ddr: marvell: a38x: fix SPLIT_OUT_M= IX state decision >=20 > External Email >=20 > ---------------------------------------------------------------------- > From: Marek Beh=C3=BAn >=20 > This is a cleaned up and fixed version of a patch > mv_ddr: a380: fix SPLIT_OUT_MIX state decision >=20 > in each pattern cycle the bus state can be changed > in order to avoide it, need to back to the same bus state on each > pattern cycle > by > Moti Boskula >=20 > The original patch is not in Marvell's mv-ddr-marvell repository. It was = gives to us by Marvell to fix an issues with DDR training on some boards, b= ut it cannot be applied as is to mv-ddr-marvell, because it is a very dirty= draft patch that would certainly break other things, mainly > DDR4 training code in mv-ddr-marvell, since it changes common functions. >=20 > I have cleaned up the patch and removed stuff that seemed unnecessary (wh= en removed, it still fixed things). Note that I don't understand completely= what the code does exactly, since I haven't studied the DDR training code = extensively (and I suspect that no one besides some few people in Marvell u= nderstand the code completely). >=20 > Anyway after the cleanup the patch still fixes isssues with DDR training = on the failing boards. >=20 > There was also a problem with the original patch on some of the Allied Te= lesis' x530 boards, reported by Chris Packham. I have asked Chris to send m= e some logs, and managed to fix it: > - if you look at the change, you'll notice that it introduces > subtraction of cur_start_win[] and cur_end_win[] members, depending on > a bit set in the current_byte_status variable > - the original patch subtracted cur_start_win[] if either > BYTE_SPLIT_OUT_MIX or BYTE_HOMOGENEOUS_SPLIT_OUT bits were set, but > subtracted cur_end_win[] only if the first one (BYTE_SPLIT_OUT_MIX) > was set > - from Chris Packham logs I discovered that the x530 board where the > original patch introduced DDR training failure, only the > BYTE_HOMOGENEOUS_SPLIT_OUT bit was set, and on our boards where the > patch is needed only the BYTE_SPLIT_OUT_MIX is set in the > current_byte_status variable > - this led me to the hypothesis that both cur_start_win[] and > cur_end_win[] should be subtracted only if BYTE_SPLIT_OUT_MIX bit is > set, the BYTE_HOMOGENEOUS_SPLIT_OUT bit shouldn't be considered at all > - this hypothesis also gains credibility when considering the commit > title ("fix SPLIT_OUT_MIX state decision") >=20 > Hopefully this will fix things without breaking anything else. >=20 > Signed-off-by: Marek Beh=C3=BAn > --- > .../a38x/ddr3_training_centralization.c | 26 +++++++++++++++++++ > 1 file changed, 26 insertions(+) >=20 > diff --git a/drivers/ddr/marvell/a38x/ddr3_training_centralization.c b/dr= ivers/ddr/marvell/a38x/ddr3_training_centralization.c > index 648b37ef6f..42308b6965 100644 > --- a/drivers/ddr/marvell/a38x/ddr3_training_centralization.c > +++ b/drivers/ddr/marvell/a38x/ddr3_training_centralization.c > @@ -55,6 +55,7 @@ static int ddr3_tip_centralization(u32 dev_num, u32 mod= e) > enum hws_training_ip_stat training_result[MAX_INTERFACE_NUM]; > u32 if_id, pattern_id, bit_id; > u8 bus_id; > + u8 current_byte_status; > u8 cur_start_win[BUS_WIDTH_IN_BITS]; > u8 centralization_result[MAX_INTERFACE_NUM][BUS_WIDTH_IN_BITS]; > u8 cur_end_win[BUS_WIDTH_IN_BITS]; > @@ -166,6 +167,10 @@ static int ddr3_tip_centralization(u32 dev_num, u32 = mode) > result[search_dir_id][7])); > } > =20 > + current_byte_status =3D > + mv_ddr_tip_sub_phy_byte_status_get(if_id, > + bus_id); > + > for (bit_id =3D 0; bit_id < BUS_WIDTH_IN_BITS; > bit_id++) { > /* check if this code is valid for 2 edge, probably not :( */ @@ -1= 74,11 +179,32 @@ static int ddr3_tip_centralization(u32 dev_num, u32 mode) > [HWS_LOW2HIGH] > [bit_id], > EDGE_1); > + if (current_byte_status & > + BYTE_SPLIT_OUT_MIX) { > + if (cur_start_win[bit_id] >=3D 64) > + cur_start_win[bit_id] -=3D 64; > + else > + cur_start_win[bit_id] =3D 0; > + DEBUG_CENTRALIZATION_ENGINE > + (DEBUG_LEVEL_INFO, > + ("pattern %d IF %d pup %d bit %d subtract 64 adll from start\n", > + pattern_id, if_id, bus_id, bit_id)); > + } > cur_end_win[bit_id] =3D > GET_TAP_RESULT(result > [HWS_HIGH2LOW] > [bit_id], > EDGE_1); > + if (cur_end_win[bit_id] >=3D 64 && > + (current_byte_status & > + BYTE_SPLIT_OUT_MIX)) { > + cur_end_win[bit_id] -=3D 64; > + DEBUG_CENTRALIZATION_ENGINE > + (DEBUG_LEVEL_INFO, > + ("pattern %d IF %d pup %d bit %d subtract 64 adll from end\n", > + pattern_id, if_id, bus_id, bit_id)); > + } > + > /* window length */ > current_window[bit_id] =3D > cur_end_win[bit_id] - > -- > 2.34.1 >=20