From: vladimir_zapolskiy@mentor•com (Vladimir Zapolskiy)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH 6/9] spi: imx: remove unnecessary bit clearing in mx51_ecspi_config
Date: Fri, 4 Mar 2016 14:06:57 +0200 [thread overview]
Message-ID: <56D97A61.2070602@mentor.com> (raw)
In-Reply-To: <20160304114737.GE21869@pengutronix.de>
On 04.03.2016 13:47, Sascha Hauer wrote:
> On Fri, Mar 04, 2016 at 12:52:15PM +0200, Vladimir Zapolskiy wrote:
>> Hi Sascha,
>>
>> On 24.02.2016 10:20, Sascha Hauer wrote:
>>> This reverts patch 1476253cef (spi: imx: fix ecspi mode setup)
>>> The patch tried to fix something by clearing bits in the cfg variable,
>>> but cfg is initialized to zero on function entry. There are no bits to
>>> clear.
>>>
>>> Signed-off-by: Sascha Hauer <s.hauer@pengutronix•de>
>>
>> I believe here the expected fix should be
>>
>> ----8<----
>>
>> diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
>> index 6a4ff27..9c9bae0 100644
>> --- a/drivers/spi/spi-imx.c
>> +++ b/drivers/spi/spi-imx.c
>> @@ -316,9 +316,10 @@ static void __maybe_unused mx51_ecspi_trigger(struct
>> spi_imx_data *spi_imx)
>> static int __maybe_unused mx51_ecspi_config(struct spi_imx_data *spi_imx,
>> struct spi_imx_config *config)
>> {
>> - u32 ctrl = MX51_ECSPI_CTRL_ENABLE, cfg = 0, dma = 0;
>> + u32 ctrl = MX51_ECSPI_CTRL_ENABLE, dma = 0;
>> u32 tx_wml_cfg, rx_wml_cfg, rxt_wml_cfg;
>> u32 clk = config->speed_hz, delay, reg;
>> + u32 cfg = readl(spi_imx->base + MX51_ECSPI_CONFIG);
>>
>
> With this change applied we would indeed have to clear and not only set the bits,
> but why would we do this?
Locally we have quite a similar to 1476253cef change (well, correct version
of it) authored by Knut Wohlrab, here is his comment from a discussion:
Please notice that iMX6 ECSPI controller support 4 channels/chip selects at
one SPI bus. The original iMX driver implementation(*) did not care about
the SPI_CPHA, SPI_CPOL and SPI_CS_HIGH configuration of the not selected
channels and set this to "0". Depending on the configuration of this
channels this could cause unwanted edges at the clock line when using this
channels the next time. To prevent this, we introduce the read of the
actual configuration/register content and set only the necessary changes.
(*) here it means the version prior to 1476253cef ("spi: imx: fix ecspi mode
setup)
--
With best wishes,
Vladimir
next prev parent reply other threads:[~2016-03-04 12:06 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-02-24 8:20 [PATCH v4] i.MX SPI DMA cleanup Sascha Hauer
2016-02-24 8:20 ` [PATCH 1/9] spi: imx: drop fallback to PIO Sascha Hauer
2016-02-24 8:20 ` [PATCH 2/9] spi: imx: initialize usedma earlier Sascha Hauer
2016-02-24 8:20 ` [PATCH 3/9] spi: imx: drop unnecessary read/modify/write Sascha Hauer
2016-02-24 8:20 ` [PATCH 4/9] spi: imx: drop unncessary dma_is_inited variable Sascha Hauer
2016-02-24 8:20 ` [PATCH 5/9] spi: imx: add support for all SPI word width for DMA Sascha Hauer
2016-03-04 10:39 ` Vladimir Zapolskiy
2016-02-24 8:20 ` [PATCH 6/9] spi: imx: remove unnecessary bit clearing in mx51_ecspi_config Sascha Hauer
2016-03-04 10:52 ` Vladimir Zapolskiy
2016-03-04 11:47 ` Sascha Hauer
2016-03-04 12:06 ` Vladimir Zapolskiy [this message]
2016-03-07 7:54 ` Sascha Hauer
2016-02-24 8:20 ` [PATCH 7/9] spi: imx: make some register defines simpler Sascha Hauer
2016-02-24 8:20 ` [PATCH 8/9] spi: imx: set MX51_ECSPI_CTRL_SMC bit in setup function Sascha Hauer
2016-02-24 8:20 ` [PATCH 9/9] spi: imx: drop bogus tests for rx/tx bufs in DMA transfer Sascha Hauer
-- strict thread matches above, loose matches on Subject: below --
2016-02-23 9:23 [PATCH v3] i.MX SPI DMA cleanup Sascha Hauer
2016-02-23 9:23 ` [PATCH 6/9] spi: imx: remove unnecessary bit clearing in mx51_ecspi_config Sascha Hauer
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=56D97A61.2070602@mentor.com \
--to=vladimir_zapolskiy@mentor$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox