From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jarkko Nikula Subject: Re: [PATCH 1/2] spi: pxa2xx: fix SCR (divisor) calculation Date: Thu, 11 Apr 2019 14:55:53 +0300 Message-ID: <34b8442a-15ac-dea2-a088-fde8b1966e1e@linux.intel.com> References: <1554900696-28858-1-git-send-email-f.suligoi@asem.it> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Cc: linux-arm-kernel@lists.infradead.org, linux-spi@vger.kernel.org, linux-kernel@vger.kernel.org To: Flavio Suligoi , Daniel Mack , Haojian Zhuang , Robert Jarzmik , Mark Brown Return-path: In-Reply-To: <1554900696-28858-1-git-send-email-f.suligoi@asem.it> Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org List-Id: linux-spi.vger.kernel.org On 4/10/19 3:51 PM, Flavio Suligoi wrote: > Calculate the divisor for the SCR (Serial Clock Rate), avoiding > that the SSP transmission rate can be greater than the device rate. > > When the division between the SSP clock and the device rate generates > a reminder, we have to increment by one the divisor. > In this way the resulting SSP clock will never be greater than the > device SPI max frequency. > > For example, with: > > - ssp_clk = 50 MHz > - dev freq = 15 MHz > > without this patch the SSP clock will be greater than 15 MHz: > > - 25 MHz for PXA25x_SSP and CE4100_SSP > - 16,56 MHz for the others > > Instead, with this patch, we have in both case an SSP clock of 12.5MHz, > so the max rate of the SPI device clock is respected. > > Signed-off-by: Flavio Suligoi > --- > drivers/spi/spi-pxa2xx.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c > index f7068cc..c9560a1 100644 > --- a/drivers/spi/spi-pxa2xx.c > +++ b/drivers/spi/spi-pxa2xx.c > @@ -884,10 +884,15 @@ static unsigned int ssp_get_clk_div(struct driver_data *drv_data, int rate) > > rate = min_t(int, ssp_clk, rate); > > + /* > + * Calculate the divisor for the SCR (Serial Clock Rate), avoiding > + * that the SSP transmission rate can be greater than the device rate > + */ > if (ssp->type == PXA25x_SSP || ssp->type == CE4100_SSP) > - return (ssp_clk / (2 * rate) - 1) & 0xff; > + return (ssp_clk / (2 * rate) - 1 + > + (ssp_clk % (2 * rate) ? 1 : 0)) & 0xff; > else > - return (ssp_clk / rate - 1) & 0xfff; > + return (ssp_clk / rate - 1 + (ssp_clk % rate ? 1 : 0)) & 0xfff; > } > I think DIV_ROUND_UP() - 1 would also fix this? I realized we have also another issue here with the low rates. Calculated divider will underflow due masking with 0xff or 0xfff when the rate is low enough. Would you want to fix that by setting the ctlr->min_speed_hz so that spi core can validate the rate? I'm asking since it goes well together with your fix. Maybe one patch setting the min_speed_hz and getting masking off from here and then another fixing the rate calculation. -- Jarkko 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 X-Spam-Level: X-Spam-Status: No, score=-7.0 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E269DC10F14 for ; Thu, 11 Apr 2019 11:56:11 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id B028E2073F for ; Thu, 11 Apr 2019 11:56:11 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="LdwxynwX" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org B028E2073F Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.intel.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender:Content-Type: Content-Transfer-Encoding:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date:Message-ID:From: References:To:Subject:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=J2n+HyaEogN5j1wVPXyGf6TEEZ6UHpSSci4NZuxbEsg=; b=LdwxynwXFyPeW5rAo7ULEPE1M GSirbEflxGvrlvwF3Rsz5BEV6S+o3HiPcfvH7TzLw7h7Cm6c75F12bynmdzMVZ5HF8ZYO2UuJiHGz CNk07d3Z6FpwrsiAn0Ps7Baw1o3yHe6A04tCSLtCjGbUxbuGMC+B1fnhKXCKfWiY5JJzAmd9DKZwH ItxsFl/CTnyVm+qOtcIpRG4EJ/24hw4jMsvcS6cAUQKeX8eZS112tiyjptytyJQmBn/37Xd+WDWd5 FMHNvwCEFKOoi3jYoalvDqDJUKhu1G9S5TNJ3J0nuo6Y9kDVTeGQWZOwde/5Yl1dugWF34nurmZTt nUfNToN+Q==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1hEYJP-0000bN-0v; Thu, 11 Apr 2019 11:56:11 +0000 Received: from mga05.intel.com ([192.55.52.43]) by bombadil.infradead.org with esmtps (Exim 4.90_1 #2 (Red Hat Linux)) id 1hEYJL-0000aa-JZ for linux-arm-kernel@lists.infradead.org; Thu, 11 Apr 2019 11:56:08 +0000 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga105.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 11 Apr 2019 04:56:06 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.60,337,1549958400"; d="scan'208";a="134858982" Received: from unknown (HELO [10.237.72.155]) ([10.237.72.155]) by orsmga006.jf.intel.com with ESMTP; 11 Apr 2019 04:56:02 -0700 Subject: Re: [PATCH 1/2] spi: pxa2xx: fix SCR (divisor) calculation To: Flavio Suligoi , Daniel Mack , Haojian Zhuang , Robert Jarzmik , Mark Brown References: <1554900696-28858-1-git-send-email-f.suligoi@asem.it> From: Jarkko Nikula Message-ID: <34b8442a-15ac-dea2-a088-fde8b1966e1e@linux.intel.com> Date: Thu, 11 Apr 2019 14:55:53 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 MIME-Version: 1.0 In-Reply-To: <1554900696-28858-1-git-send-email-f.suligoi@asem.it> Content-Language: en-US X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20190411_045607_649464_58D18562 X-CRM114-Status: GOOD ( 22.64 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-spi@vger.kernel.org Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 4/10/19 3:51 PM, Flavio Suligoi wrote: > Calculate the divisor for the SCR (Serial Clock Rate), avoiding > that the SSP transmission rate can be greater than the device rate. > > When the division between the SSP clock and the device rate generates > a reminder, we have to increment by one the divisor. > In this way the resulting SSP clock will never be greater than the > device SPI max frequency. > > For example, with: > > - ssp_clk = 50 MHz > - dev freq = 15 MHz > > without this patch the SSP clock will be greater than 15 MHz: > > - 25 MHz for PXA25x_SSP and CE4100_SSP > - 16,56 MHz for the others > > Instead, with this patch, we have in both case an SSP clock of 12.5MHz, > so the max rate of the SPI device clock is respected. > > Signed-off-by: Flavio Suligoi > --- > drivers/spi/spi-pxa2xx.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c > index f7068cc..c9560a1 100644 > --- a/drivers/spi/spi-pxa2xx.c > +++ b/drivers/spi/spi-pxa2xx.c > @@ -884,10 +884,15 @@ static unsigned int ssp_get_clk_div(struct driver_data *drv_data, int rate) > > rate = min_t(int, ssp_clk, rate); > > + /* > + * Calculate the divisor for the SCR (Serial Clock Rate), avoiding > + * that the SSP transmission rate can be greater than the device rate > + */ > if (ssp->type == PXA25x_SSP || ssp->type == CE4100_SSP) > - return (ssp_clk / (2 * rate) - 1) & 0xff; > + return (ssp_clk / (2 * rate) - 1 + > + (ssp_clk % (2 * rate) ? 1 : 0)) & 0xff; > else > - return (ssp_clk / rate - 1) & 0xfff; > + return (ssp_clk / rate - 1 + (ssp_clk % rate ? 1 : 0)) & 0xfff; > } > I think DIV_ROUND_UP() - 1 would also fix this? I realized we have also another issue here with the low rates. Calculated divider will underflow due masking with 0xff or 0xfff when the rate is low enough. Would you want to fix that by setting the ctlr->min_speed_hz so that spi core can validate the rate? I'm asking since it goes well together with your fix. Maybe one patch setting the min_speed_hz and getting masking off from here and then another fixing the rate calculation. -- Jarkko _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel