From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2a00:1450:400c:c09::242; helo=mail-wm0-x242.google.com; envelope-from=leif.lindholm@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-wm0-x242.google.com (mail-wm0-x242.google.com [IPv6:2a00:1450:400c:c09::242]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 0ABCA2035260E for ; Thu, 26 Oct 2017 06:58:17 -0700 (PDT) Received: by mail-wm0-x242.google.com with SMTP id b9so8566356wmh.0 for ; Thu, 26 Oct 2017 07:02:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=poA70NKeLWKI69GUIrY1ujvns14N1EJGTR/n+M//Frk=; b=U8yG1C6Q49n4LS+JmUnMET4o0F/t+8VXWgab9u5GOPRCWbgHWEwT4luSYwavbmmKge SnQyjUwt5PGnhM9HvE2wnkRaMpBj6V6e2Jcyq8TD+t+oeX2AIrVcJpEBXO3NQBaeQav4 0lQMsKEjscFUin0ZsM3cVsF0y1Pnkst5bJasU= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=poA70NKeLWKI69GUIrY1ujvns14N1EJGTR/n+M//Frk=; b=Ph4nuYF++MvdtQlCTKgv5lxMfWOW9+3I0d9UB52YlKK2eSnVIywvIO+B1ChQWkvE18 8SaPcgkbKCPVHQ+G3bu1L7GzycUWfyY87oEfqumMXBOWKXB/0ChVbLc6zEyv9Gi4xAds A0zqsJLjuZICZSBfkOlrawi1eInM/EsoYBW2dTnta/e8sm+XIkI/LOubbOwMqB0b/eXo KNRkgceJ/uH8Axi4muwD6IxROVyP/A1QT9ZIi7EbgmxW7i5diocVhqfoF4d65qPSKnbN NuOrbhctPwdHEn/F/ACqVznEAnAH2bBfc47neeEkdgCfiaCIW5ZWDl5QHJJKRofcHMYF +1wQ== X-Gm-Message-State: AMCzsaXb14E5Z+HA+3J4F5p/DqlTtoBsAoaCcmuZu0CkJZUH3PxHcc0Y PKO5myduXP6XSEkpMnQqKAQUGA== X-Google-Smtp-Source: ABhQp+SA6wo70R2s/VESzblG38cj7Wd3lU4luJfCDdpTHlh1eY1Afxoy644BIkPRjMLuVXHZxIBD1g== X-Received: by 10.28.239.16 with SMTP id n16mr1598673wmh.49.1509026522802; Thu, 26 Oct 2017 07:02:02 -0700 (PDT) Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id c37sm6207481wrc.92.2017.10.26.07.02.01 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Thu, 26 Oct 2017 07:02:01 -0700 (PDT) Date: Thu, 26 Oct 2017 15:02:00 +0100 From: Leif Lindholm To: Marcin Wojtas Cc: edk2-devel-01 , Ard Biesheuvel , nadavh@marvell.com, Neta Zur Hershkovits , Kostya Porotchkin , Hua Jing , semihalf-dabros-jan Message-ID: <20171026140200.v33dpi54ri2e3fuu@bivouac.eciton.net> References: <1508980777-29006-1-git-send-email-mw@semihalf.com> <1508980777-29006-10-git-send-email-mw@semihalf.com> <20171026134612.nro2lhy2l3qvm7pq@bivouac.eciton.net> MIME-Version: 1.0 In-Reply-To: User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [platforms: PATCH 09/10] Marvell/Drivers: XenonDxe: Fix base clock frequency X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 26 Oct 2017 13:58:18 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Thu, Oct 26, 2017 at 03:54:41PM +0200, Marcin Wojtas wrote: > 2017-10-26 15:46 GMT+02:00 Leif Lindholm : > > On Thu, Oct 26, 2017 at 03:19:36AM +0200, Marcin Wojtas wrote: > >> Incorrectly the clock divisor was calculated relatively > >> to 255MHz instead of actual 400MHz. > > > > This describes the specific symptom, not the problem with the existing > > code. > > > >> Fix this. > >> > >> Contributed-under: TianoCore Contribution Agreement 1.1 > >> Signed-off-by: Marcin Wojtas > >> --- > >> Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHci.c | 4 ++-- > >> 1 file changed, 2 insertions(+), 2 deletions(-) > >> > >> diff --git a/Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHci.c b/Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHci.c > >> index ccbf355..0b9328b 100644 > >> --- a/Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHci.c > >> +++ b/Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHci.c > >> @@ -16,6 +16,7 @@ > >> **/ > >> > >> #include "SdMmcPciHcDxe.h" > >> +#include "XenonSdhci.h" > >> > >> /** > >> Dump the content of SD/MMC host controller's Capability Register. > >> @@ -703,9 +704,8 @@ SdMmcHcClockSupply ( > >> // > >> // Calculate a divisor for SD clock frequency > >> // > >> - ASSERT (Capability.BaseClkFreq != 0); > >> > >> - BaseClkFreq = Capability.BaseClkFreq; > > > > Why is Capability.BaseClkFreq the wrong frequency to use? > > The Capability.BaseClkFreq is UINT8 and can hold up to 0xff -> 255MHz. > An alternative would be change this generic type to UINT16 and update > field properly during initialization - do you prefer that? No, I'm still dreaming we might be able to reintegrate this into the MdeModulePkg driver in some glorious future. So what you are basically saying is that this controller is running at a higher frequency than is permitted (or even describable) by the specification? If so, _that_ needs to be in the commit message (and really, a comment by the code as well). / Leif