From: Mark Brown <broonie@kernel.org> To: Jarkko Nikula <jarkko.nikula@linux.intel.com> Cc: xiao jin <jin.xiao@intel.com>, daniel@zonque.org, haojian.zhuang@gmail.com, robert.jarzmik@free.fr, linux-arm-kernel@lists.infradead.org, linux-spi@vger.kernel.org, linux-kernel@vger.kernel.org, yanmin.zhang@intel.com, bo <bo.he@intel.com> Subject: Re: [PATCH] [RFC] spi: pxa2xx: Do cs if restart the SSP during pxa2xx_spi_transfer_one() Date: Thu, 7 Mar 2019 16:09:24 +0000 [thread overview] Message-ID: <20190307160924.GE6529@sirena.org.uk> (raw) In-Reply-To: <cf5b89b6-e081-5ba2-2837-7d3cfde779ae@linux.intel.com> [-- Attachment #1: Type: text/plain, Size: 1479 bytes --] On Thu, Mar 07, 2019 at 05:26:53PM +0200, Jarkko Nikula wrote: > On 3/7/19 9:24 AM, xiao jin wrote: > > The patch is to do cs again if spi-pxa2xx restar the SSP during > > pxa2xx_spi_transfer_one() > Hmm.. please correct me if I'm wrong but pxa2xx_spi_unprepare_transfer() is > called always when there is no more messages pending and the spi core should > have deasserted the CS already? Yes. > > @@ -1056,6 +1057,11 @@ static int pxa2xx_spi_transfer_one(struct spi_controller *master, > > if ((pxa2xx_spi_read(drv_data, SSCR0) != cr0) > > || (pxa2xx_spi_read(drv_data, SSCR1) & change_mask) > > != (cr1 & change_mask)) { > > + /* It needs to deassert the chip selection > > + * firstly before restart the SPP */ > > + need_cs_change = true; > > + cs_deassert(spi); > I think code comes here at the beginning of each transfer so will be hit > multiple times before pxa2xx_spi_unprepare_transfer() if SPI message > consists of multiple transfers. > This makes me wondering if the device driver setting up the "struct > spi_transfer" is maybe missing the cs_change flag set for transfers before > last one in case HW needs CS toggling between transfers? For instance what > following drivers are doing with the cs_change flag: > drivers/char/tpm/tpm_tis_spi.c: tpm_tis_spi_transfer() > drivers/input/touchscreen/ad7877.c: ad7877_read(), ad7877_read_adc() Right, this really feels like it's fixing the wrong thing. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --]
WARNING: multiple messages have this Message-ID (diff)
From: Mark Brown <broonie@kernel.org> To: Jarkko Nikula <jarkko.nikula@linux.intel.com> Cc: yanmin.zhang@intel.com, linux-kernel@vger.kernel.org, haojian.zhuang@gmail.com, linux-spi@vger.kernel.org, bo <bo.he@intel.com>, linux-arm-kernel@lists.infradead.org, robert.jarzmik@free.fr, xiao jin <jin.xiao@intel.com>, daniel@zonque.org Subject: Re: [PATCH] [RFC] spi: pxa2xx: Do cs if restart the SSP during pxa2xx_spi_transfer_one() Date: Thu, 7 Mar 2019 16:09:24 +0000 [thread overview] Message-ID: <20190307160924.GE6529@sirena.org.uk> (raw) In-Reply-To: <cf5b89b6-e081-5ba2-2837-7d3cfde779ae@linux.intel.com> [-- Attachment #1.1: Type: text/plain, Size: 1479 bytes --] On Thu, Mar 07, 2019 at 05:26:53PM +0200, Jarkko Nikula wrote: > On 3/7/19 9:24 AM, xiao jin wrote: > > The patch is to do cs again if spi-pxa2xx restar the SSP during > > pxa2xx_spi_transfer_one() > Hmm.. please correct me if I'm wrong but pxa2xx_spi_unprepare_transfer() is > called always when there is no more messages pending and the spi core should > have deasserted the CS already? Yes. > > @@ -1056,6 +1057,11 @@ static int pxa2xx_spi_transfer_one(struct spi_controller *master, > > if ((pxa2xx_spi_read(drv_data, SSCR0) != cr0) > > || (pxa2xx_spi_read(drv_data, SSCR1) & change_mask) > > != (cr1 & change_mask)) { > > + /* It needs to deassert the chip selection > > + * firstly before restart the SPP */ > > + need_cs_change = true; > > + cs_deassert(spi); > I think code comes here at the beginning of each transfer so will be hit > multiple times before pxa2xx_spi_unprepare_transfer() if SPI message > consists of multiple transfers. > This makes me wondering if the device driver setting up the "struct > spi_transfer" is maybe missing the cs_change flag set for transfers before > last one in case HW needs CS toggling between transfers? For instance what > following drivers are doing with the cs_change flag: > drivers/char/tpm/tpm_tis_spi.c: tpm_tis_spi_transfer() > drivers/input/touchscreen/ad7877.c: ad7877_read(), ad7877_read_adc() Right, this really feels like it's fixing the wrong thing. [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] [-- Attachment #2: Type: text/plain, Size: 176 bytes --] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2019-03-07 16:09 UTC|newest] Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-03-07 7:24 [PATCH] [RFC] spi: pxa2xx: Do cs if restart the SSP during pxa2xx_spi_transfer_one() xiao jin 2019-03-07 15:26 ` Jarkko Nikula 2019-03-07 15:26 ` Jarkko Nikula 2019-03-07 16:09 ` Mark Brown [this message] 2019-03-07 16:09 ` Mark Brown 2019-03-08 7:28 ` Xiao, Jin 2019-03-08 7:28 ` Xiao, Jin 2019-03-08 7:28 ` Xiao, Jin 2019-03-19 15:27 ` Jarkko Nikula 2019-03-19 15:27 ` Jarkko Nikula
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=20190307160924.GE6529@sirena.org.uk \ --to=broonie@kernel.org \ --cc=bo.he@intel.com \ --cc=daniel@zonque.org \ --cc=haojian.zhuang@gmail.com \ --cc=jarkko.nikula@linux.intel.com \ --cc=jin.xiao@intel.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-spi@vger.kernel.org \ --cc=robert.jarzmik@free.fr \ --cc=yanmin.zhang@intel.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.