Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752733AbbHRMUd (ORCPT ); Tue, 18 Aug 2015 08:20:33 -0400 Received: from arrakis.dune.hu ([78.24.191.176]:56186 "EHLO arrakis.dune.hu" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750855AbbHRMUb (ORCPT ); Tue, 18 Aug 2015 08:20:31 -0400 MIME-Version: 1.0 In-Reply-To: <1439895231-17778-1-git-send-email-leilk.liu@mediatek.com> References: <1439895231-17778-1-git-send-email-leilk.liu@mediatek.com> From: Jonas Gorski Date: Tue, 18 Aug 2015 14:19:54 +0200 Message-ID: Subject: Re: [PATCH] spi: mediatek: fix spi incorrect endian usage and remove redundant clock To: Leilk Liu Cc: Mark Brown , Mark Rutland , Matthias Brugger , Sascha Hauer , "devicetree@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , linux-spi@vger.kernel.org, linux-mediatek@lists.infradead.org Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2496 Lines: 72 Hi, On Tue, Aug 18, 2015 at 12:53 PM, Leilk Liu wrote: > This patch fixes incorrect endian usage, removes redundant > clock in prepare_hardware/unprepare_hardware and revises > coding styles. > > Signed-off-by: Leilk Liu > > --- > Change in this patch: > 1. fix incorrect endian usage on big-endian system. > 2. delete redundant clock in prepare/unprepare_hardware. > 3. revise coding styles. The usual philosophy is to have one change per patch, so this might be better as three patches. But this is Mark's call. Since the driver isn't yet in Linus' tree, it might be a-ok to mix style improvements and actual fixes, but as soon as it landed in Linus' tree you need to keep them separate, so fixes can be easily backported. Regarding the content ... > --- > drivers/spi/spi-mt65xx.c | 163 +++++++++++++------------------ > include/linux/platform_data/spi-mt65xx.h | 2 - > 2 files changed, 69 insertions(+), 96 deletions(-) > > diff --git a/drivers/spi/spi-mt65xx.c b/drivers/spi/spi-mt65xx.c > index 519f50c..a9da887 100644 > --- a/drivers/spi/spi-mt65xx.c > +++ b/drivers/spi/spi-mt65xx.c > @@ -694,6 +662,7 @@ static int mtk_spi_resume(struct device *dev) > if (!pm_runtime_suspended(dev)) { > ret = clk_prepare_enable(mdata->spi_clk); > if (ret < 0) > + dev_err(dev, "failed to enable spi_clk (%d)\n", ret); > return ret; You need to add braces here, else the "return ret" isn't covered by the if () anymore and you always return at this place. > } > > @@ -720,8 +689,14 @@ static int mtk_spi_runtime_resume(struct device *dev) > { > struct spi_master *master = dev_get_drvdata(dev); > struct mtk_spi *mdata = spi_master_get_devdata(master); > + int ret; > + > + ret = clk_prepare_enable(mdata->spi_clk); > + if (ret < 0) > + dev_err(dev, "failed to enable spi_clk (%d)\n", ret); > + return ret; Same here. Although at least here it should be harmless, as clk_prepare_enable doesn't return > 0. > > - return clk_prepare_enable(mdata->spi_clk); > + return 0; > } > #endif /* CONFIG_PM */ > Jonas -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/