[PATCH V4] spi: s3c64xx: Enable Word transfer

[PATCH V4] spi: s3c64xx: Enable Word transfer

[Rajeshwari S Shinde]

This patch enables word transfer for s3c64xx spi driver.
User can set bits_per_word to 32 or 16 or 8, before calling
spi_setup, which would enable the corresponding transfer mode.

Change-Id: Ib04f9851a3ea891d2bfa527f2100acd314fe1c98
Signed-off-by: Rajeshwari S Shinde <rajeshwari.s@samsung.com>
---
Changes in V2
        - Reduced the call for s3c64xx_spi_config.
Changes in V3:
	- Corrected the coding style nits.
Chnages in V4:
	- Add switch case to support both 16 and 32 bit transfer.
	- Corrected checkpatch error.
 drivers/spi/spi-s3c64xx.c | 30 +++++++++++++++++++++++-------
 1 file changed, 23 insertions(+), 7 deletions(-)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index 512b889..1620b74 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -491,6 +491,7 @@ static void enable_datapath(struct s3c64xx_spi_driver_data *sdd,
 {
 	void __iomem *regs = sdd->regs;
 	u32 modecfg, chcfg;
+	u32 swapcfg = 0;
 
 	modecfg = readl(regs + S3C64XX_SPI_MODE_CFG);
 	modecfg &= ~(S3C64XX_SPI_MODE_TXDMA_ON | S3C64XX_SPI_MODE_RXDMA_ON);
@@ -498,6 +499,22 @@ static void enable_datapath(struct s3c64xx_spi_driver_data *sdd,
 	chcfg = readl(regs + S3C64XX_SPI_CH_CFG);
 	chcfg &= ~S3C64XX_SPI_CH_TXCH_ON;
 
+	switch (sdd->cur_bpw) {
+	case 32:
+		swapcfg = S3C64XX_SPI_SWAP_TX_HALF_WORD |
+			S3C64XX_SPI_SWAP_RX_HALF_WORD;
+	case 16:
+		swapcfg |= S3C64XX_SPI_SWAP_TX_EN |
+			S3C64XX_SPI_SWAP_RX_EN |
+			S3C64XX_SPI_SWAP_RX_BYTE |
+			S3C64XX_SPI_SWAP_TX_BYTE;
+		writel(swapcfg, regs + S3C64XX_SPI_SWAP_CFG);
+		break;
+	default:
+		writel(0, regs + S3C64XX_SPI_SWAP_CFG);
+		break;
+	}
+
 	if (dma_mode) {
 		chcfg &= ~S3C64XX_SPI_CH_RXCH_ON;
 	} else {
@@ -905,13 +922,12 @@ static int s3c64xx_spi_transfer_one_message(struct spi_master *master,
 		bpw = xfer->bits_per_word;
 		speed = xfer->speed_hz ? : spi->max_speed_hz;
 
-		if (xfer->len % (bpw / 8)) {
-			dev_err(&spi->dev,
-				"Xfer length(%u) not a multiple of word size(%u)\n",
-				xfer->len, bpw / 8);
-			status = -EIO;
-			goto out;
-		}
+		/*
+		 * Enable byte transfer if transfer length not a multiple of
+		 * word size
+		 */
+		if (xfer->len % (bpw / 8))
+			bpw = 8;
 
 		if (bpw != sdd->cur_bpw || speed != sdd->cur_speed) {
 			sdd->cur_bpw = bpw;
-- 
1.7.12.4

Re: [PATCH V4] spi: s3c64xx: Enable Word transfer

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 511 bytes --]

On Fri, Oct 18, 2013 at 11:02:36AM +0530, Rajeshwari S Shinde wrote:
> This patch enables word transfer for s3c64xx spi driver.
> User can set bits_per_word to 32 or 16 or 8, before calling
> spi_setup, which would enable the corresponding transfer mode.

> Change-Id: Ib04f9851a3ea891d2bfa527f2100acd314fe1c98

Please don't include stuff like this in upstrem submissions, it's of no
use to anyone outside your organisation.  

This patch is still not setting bits_per_word_mask as far as I can see?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH V4] spi: s3c64xx: Enable Word transfer

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 616 bytes --]

On Sat, Oct 19, 2013 at 10:58:47AM -0600, Simon Glass wrote:

> If it helps, patman (tools/patman in U-Boot tree) will strip this
> automatically (along with other Gerrit and Chrome OS stuff), run checkpatch
> and Doug has added features to automatically cc the relevant maintainers
> for Linux. I got sick of trying to remember to do all these steps.

Watch out for the automatic identification of MAINTAINERS though - you
do need to pay attention to what gets set and check it, get_maintainers
isn't 100% reliable.  You should check what it does and make sure you
understand why each person there is getting CCed.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH V4] spi: s3c64xx: Enable Word transfer

[Rajeshwari Birje]

Hi Mark,

On Sat, Oct 19, 2013 at 9:55 PM, Mark Brown <broonie@kernel.org> wrote:
> On Fri, Oct 18, 2013 at 11:02:36AM +0530, Rajeshwari S Shinde wrote:
>> This patch enables word transfer for s3c64xx spi driver.
>> User can set bits_per_word to 32 or 16 or 8, before calling
>> spi_setup, which would enable the corresponding transfer mode.
>
>> Change-Id: Ib04f9851a3ea891d2bfa527f2100acd314fe1c98
>
> Please don't include stuff like this in upstrem submissions, it's of no
> use to anyone outside your organisation.
That had got added by mistake. Sorry for the same.
>
> This patch is still not setting bits_per_word_mask as far as I can see?
Will send new version of patch including this.

-- 
Regards,
Rajeshwari Shinde

Re: [PATCH V4] spi: s3c64xx: Enable Word transfer

[Rajeshwari Birje]

Hi Mark Brown,

Sorry for spamming again.

On Tue, Oct 29, 2013 at 4:29 PM, Rajeshwari Birje
<rajeshwari.birje-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Hi Mark,
>
> On Sat, Oct 19, 2013 at 9:55 PM, Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>> On Fri, Oct 18, 2013 at 11:02:36AM +0530, Rajeshwari S Shinde wrote:
>>> This patch enables word transfer for s3c64xx spi driver.
>>> User can set bits_per_word to 32 or 16 or 8, before calling
>>> spi_setup, which would enable the corresponding transfer mode.
>>
>>> Change-Id: Ib04f9851a3ea891d2bfa527f2100acd314fe1c98
>>
>> Please don't include stuff like this in upstrem submissions, it's of no
>> use to anyone outside your organisation.
> That had got added by mistake. Sorry for the same.
>>
>> This patch is still not setting bits_per_word_mask as far as I can see?
> Will send new version of patch including this.
I had some confusion regarding this bits_per_word_mask, where do you
want me to mask the bpw.
bits_per_word is something which comes from the user, do we need to mask it?


-- 
Regards,
Rajeshwari Shinde
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V4] spi: s3c64xx: Enable Word transfer

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 571 bytes --]

On Tue, Oct 29, 2013 at 04:59:11PM +0530, Rajeshwari Birje wrote:
> On Tue, Oct 29, 2013 at 4:29 PM, Rajeshwari Birje

> >> This patch is still not setting bits_per_word_mask as far as I can see?

> > Will send new version of patch including this.

> I had some confusion regarding this bits_per_word_mask, where do you
> want me to mask the bpw.
> bits_per_word is something which comes from the user, do we need to mask it?

Have you looked at the code and documentation for this feature - if it's
not clear can you please explain in more detail what needs clarifying?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH V4] spi: s3c64xx: Enable Word transfer

[Rajeshwari Birje]

Hi Mark Brown,

On Tue, Oct 29, 2013 at 10:09 PM, Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Tue, Oct 29, 2013 at 04:59:11PM +0530, Rajeshwari Birje wrote:
>> On Tue, Oct 29, 2013 at 4:29 PM, Rajeshwari Birje
>
>> >> This patch is still not setting bits_per_word_mask as far as I can see?
>
>> > Will send new version of patch including this.
>
>> I had some confusion regarding this bits_per_word_mask, where do you
>> want me to mask the bpw.
>> bits_per_word is something which comes from the user, do we need to mask it?
>
> Have you looked at the code and documentation for this feature - if it's
> not clear can you please explain in more detail what needs clarifying?

The following patch already sets bits_per_word_mask for
drivers/spi/spi-s3c64xx.c in s3c64xx_spi_probe, hence I had a doubt do
I need to set the same again.

commit e761f4236e94f2dd36316f9892583b29ce986031
Author: Mark Brown <broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org>
Date:   Mon Apr 1 14:17:37 2013 +0100

    spi/s3c64xx: Convert to bits_per_word_mask

    The core can do the validation for us.

    Signed-off-by: Mark Brown <broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org>

It is already present as follows in s3c64xx_spi_probe
master->bits_per_word_mask = SPI_BPW_MASK(32) | SPI_BPW_MASK(16) |
                                        SPI_BPW_MASK(8);
-- 
Regards,
Rajeshwari Shinde
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V4] spi: s3c64xx: Enable Word transfer

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 461 bytes --]

On Wed, Oct 30, 2013 at 11:52:54AM +0530, Rajeshwari Birje wrote:

> The following patch already sets bits_per_word_mask for
> drivers/spi/spi-s3c64xx.c in s3c64xx_spi_probe, hence I had a doubt do
> I need to set the same again.

OK, so how did this work before then?  You're just adding new code but
the driver was previously claiming to support different bits per word
(and now I look at the code there is some handling for that in code).
Is this a bug fix?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH V4] spi: s3c64xx: Enable Word transfer

[Tomasz Figa]

On Wednesday 30 of October 2013 10:00:29 Mark Brown wrote:
> On Wed, Oct 30, 2013 at 11:52:54AM +0530, Rajeshwari Birje wrote:
> 
> > The following patch already sets bits_per_word_mask for
> > drivers/spi/spi-s3c64xx.c in s3c64xx_spi_probe, hence I had a doubt do
> > I need to set the same again.
> 
> OK, so how did this work before then?  You're just adding new code but
> the driver was previously claiming to support different bits per word
> (and now I look at the code there is some handling for that in code).
> Is this a bug fix?

That's a good question. Rajeshwari, what SPI device did you test this
patch with? Does it have a driver in mainline kernel?

Best regards,
Tomasz

Re: [PATCH V4] spi: s3c64xx: Enable Word transfer

[Rajeshwari Birje]

Hi Mark Brown ,

On Wed, Oct 30, 2013 at 10:36 PM, Tomasz Figa <t.figa-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
> On Wednesday 30 of October 2013 10:00:29 Mark Brown wrote:
>> On Wed, Oct 30, 2013 at 11:52:54AM +0530, Rajeshwari Birje wrote:
>>
>> > The following patch already sets bits_per_word_mask for
>> > drivers/spi/spi-s3c64xx.c in s3c64xx_spi_probe, hence I had a doubt do
>> > I need to set the same again.
>>
>> OK, so how did this work before then?  You're just adding new code but
>> the driver was previously claiming to support different bits per word
>> (and now I look at the code there is some handling for that in code).
>> Is this a bug fix?
No idea if it worked before, but now if I try to do 16 bit transfer
without my patch, I get following error.
"Xfer length(1) not a multiple of word size(2)"
I guess we can consider this as a bug fix then for word transfer.
>
> That's a good question. Rajeshwari, what SPI device did you test this
> patch with? Does it have a driver in mainline kernel?
@Tomasz: I tested it for W25Q32DW Winbond device which is already has
a driver in mainline Kernel
>
> Best regards,
> Tomasz
>
-- 
Regards,
Rajeshwari Shinde
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V4] spi: s3c64xx: Enable Word transfer

[Tomasz Figa]

Hi Rajeshwari,

On Thursday 31 of October 2013 16:17:15 Rajeshwari Birje wrote:
> Hi Mark Brown ,
> 
> On Wed, Oct 30, 2013 at 10:36 PM, Tomasz Figa <t.figa@samsung.com> wrote:
> > On Wednesday 30 of October 2013 10:00:29 Mark Brown wrote:
> >> On Wed, Oct 30, 2013 at 11:52:54AM +0530, Rajeshwari Birje wrote:
> >>
> >> > The following patch already sets bits_per_word_mask for
> >> > drivers/spi/spi-s3c64xx.c in s3c64xx_spi_probe, hence I had a doubt do
> >> > I need to set the same again.
> >>
> >> OK, so how did this work before then?  You're just adding new code but
> >> the driver was previously claiming to support different bits per word
> >> (and now I look at the code there is some handling for that in code).
> >> Is this a bug fix?
> No idea if it worked before, but now if I try to do 16 bit transfer
> without my patch, I get following error.
> "Xfer length(1) not a multiple of word size(2)"
> I guess we can consider this as a bug fix then for word transfer.
> >
> > That's a good question. Rajeshwari, what SPI device did you test this
> > patch with? Does it have a driver in mainline kernel?
> @Tomasz: I tested it for W25Q32DW Winbond device which is already has
> a driver in mainline Kernel

The driver as of today's linux-next does not seem to support word
transfers. Do you have some additional, out of tree patches that add
such support?

Best regards,
Tomasz

Re: [PATCH V4] spi: s3c64xx: Enable Word transfer

[Rajeshwari Birje]

Hi Tomasz,

On Thu, Oct 31, 2013 at 6:07 PM, Tomasz Figa <t.figa@samsung.com> wrote:
> Hi Rajeshwari,
>
> On Thursday 31 of October 2013 16:17:15 Rajeshwari Birje wrote:
>> Hi Mark Brown ,
>>
>> On Wed, Oct 30, 2013 at 10:36 PM, Tomasz Figa <t.figa@samsung.com> wrote:
>> > On Wednesday 30 of October 2013 10:00:29 Mark Brown wrote:
>> >> On Wed, Oct 30, 2013 at 11:52:54AM +0530, Rajeshwari Birje wrote:
>> >>
>> >> > The following patch already sets bits_per_word_mask for
>> >> > drivers/spi/spi-s3c64xx.c in s3c64xx_spi_probe, hence I had a doubt do
>> >> > I need to set the same again.
>> >>
>> >> OK, so how did this work before then?  You're just adding new code but
>> >> the driver was previously claiming to support different bits per word
>> >> (and now I look at the code there is some handling for that in code).
>> >> Is this a bug fix?
>> No idea if it worked before, but now if I try to do 16 bit transfer
>> without my patch, I get following error.
>> "Xfer length(1) not a multiple of word size(2)"
>> I guess we can consider this as a bug fix then for word transfer.
>> >
>> > That's a good question. Rajeshwari, what SPI device did you test this
>> > patch with? Does it have a driver in mainline kernel?
>> @Tomasz: I tested it for W25Q32DW Winbond device which is already has
>> a driver in mainline Kernel
>
> The driver as of today's linux-next does not seem to support word
> transfers. Do you have some additional, out of tree patches that add
> such support?
Yes I set the bits_per_word to 32/16 in drivers/mtd/devices/m25p80.c
m25p_probe function and the call spi_setup(spi).
> Best regards,
> Tomasz
>
-- 
Regards,
Rajeshwari Shinde

Re: [PATCH V4] spi: s3c64xx: Enable Word transfer

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 662 bytes --]

On Wed, Nov 06, 2013 at 10:23:07AM +0530, Rajeshwari Birje wrote:
> On Thu, Oct 31, 2013 at 6:07 PM, Tomasz Figa <t.figa@samsung.com> wrote:

> > The driver as of today's linux-next does not seem to support word
> > transfers. Do you have some additional, out of tree patches that add
> > such support?

> Yes I set the bits_per_word to 32/16 in drivers/mtd/devices/m25p80.c
> m25p_probe function and the call spi_setup(spi).

This sound wrong.  If you just set bits per word then this should result
in data corruption on the bus since it should cause the words written to
the bus to be reordered.  Are you sure that the existing driver isn't
working correctly?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH V4] spi: s3c64xx: Enable Word transfer

[Rajeshwari Birje]

Hi Mark Brown,

On Wed, Nov 6, 2013 at 1:51 PM, Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Wed, Nov 06, 2013 at 10:23:07AM +0530, Rajeshwari Birje wrote:
>> On Thu, Oct 31, 2013 at 6:07 PM, Tomasz Figa <t.figa-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
>
>> > The driver as of today's linux-next does not seem to support word
>> > transfers. Do you have some additional, out of tree patches that add
>> > such support?
>
>> Yes I set the bits_per_word to 32/16 in drivers/mtd/devices/m25p80.c
>> m25p_probe function and the call spi_setup(spi).
>
> This sound wrong.  If you just set bits per word then this should result
> in data corruption on the bus since it should cause the words written to
> the bus to be reordered.  Are you sure that the existing driver isn't
> working correctly?
The reordering of the words is taken care by the Swap configuration
(swap_cfg) register, which I have set for both 16 and 32 bits_per_word
case. I have tested this patch and works fine for me and also has
improved the timing performance.

-- 
Regards,
Rajeshwari Shinde
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V4] spi: s3c64xx: Enable Word transfer

[Tomasz Figa]

[CCing Sylwester, Andrzej and Lukasz]

On Thursday 07 of November 2013 14:52:36 Rajeshwari Birje wrote:
> Hi Mark Brown,
> 
> On Wed, Nov 6, 2013 at 1:51 PM, Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> > On Wed, Nov 06, 2013 at 10:23:07AM +0530, Rajeshwari Birje wrote:
> >> On Thu, Oct 31, 2013 at 6:07 PM, Tomasz Figa <t.figa-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> 
wrote:
> >> > The driver as of today's linux-next does not seem to support word
> >> > transfers. Do you have some additional, out of tree patches that
> >> > add
> >> > such support?
> >> 
> >> Yes I set the bits_per_word to 32/16 in drivers/mtd/devices/m25p80.c
> >> m25p_probe function and the call spi_setup(spi).
> > 
> > This sound wrong.  If you just set bits per word then this should
> > result in data corruption on the bus since it should cause the words
> > written to the bus to be reordered.  Are you sure that the existing
> > driver isn't working correctly?
> 
> The reordering of the words is taken care by the Swap configuration
> (swap_cfg) register, which I have set for both 16 and 32 bits_per_word
> case. I have tested this patch and works fine for me and also has
> improved the timing performance.

This driver seems to have already worked fine with 32 bits per word,
using the s5c73m3 camera sensor, which uses SPI for firmware upload.

Sylwester, Andrzej or Lukasz: Can you confirm this?

Best regards,
Tomasz

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V4] spi: s3c64xx: Enable Word transfer

[Lukasz Czerwinski]

On 11/07/2013 10:39 AM, Tomasz Figa wrote:
> [CCing Sylwester, Andrzej and Lukasz]
>
> On Thursday 07 of November 2013 14:52:36 Rajeshwari Birje wrote:
>> Hi Mark Brown,
>>
>> On Wed, Nov 6, 2013 at 1:51 PM, Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>>> On Wed, Nov 06, 2013 at 10:23:07AM +0530, Rajeshwari Birje wrote:
>>>> On Thu, Oct 31, 2013 at 6:07 PM, Tomasz Figa <t.figa-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> wrote:
>>>>> The driver as of today's linux-next does not seem to support word
>>>>> transfers. Do you have some additional, out of tree patches that
>>>>> add
>>>>> such support?
>>>>
>>>> Yes I set the bits_per_word to 32/16 in drivers/mtd/devices/m25p80.c
>>>> m25p_probe function and the call spi_setup(spi).
>>>
>>> This sound wrong.  If you just set bits per word then this should
>>> result in data corruption on the bus since it should cause the words
>>> written to the bus to be reordered.  Are you sure that the existing
>>> driver isn't working correctly?
>>
>> The reordering of the words is taken care by the Swap configuration
>> (swap_cfg) register, which I have set for both 16 and 32 bits_per_word
>> case. I have tested this patch and works fine for me and also has
>> improved the timing performance.
>
> This driver seems to have already worked fine with 32 bits per word,
> using the s5c73m3 camera sensor, which uses SPI for firmware upload.
>
Yes, You are right.
> Sylwester, Andrzej or Lukasz: Can you confirm this?
>
> Best regards,
> Tomasz
>
>

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V4] spi: s3c64xx: Enable Word transfer

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 984 bytes --]

On Thu, Nov 07, 2013 at 10:39:37AM +0100, Tomasz Figa wrote:
> On Thursday 07 of November 2013 14:52:36 Rajeshwari Birje wrote:

> > The reordering of the words is taken care by the Swap configuration
> > (swap_cfg) register, which I have set for both 16 and 32 bits_per_word
> > case. I have tested this patch and works fine for me and also has
> > improved the timing performance.

> This driver seems to have already worked fine with 32 bits per word,
> using the s5c73m3 camera sensor, which uses SPI for firmware upload.

This is all sounding very confused.  Either this patch is a bug fix or
it's breaking the driver but I can't tell without more information about
what it's supposed to do.  Can someone explain in concrete terms what
the intended effect of the patch is on wire behaviour?

It is possible that the s5c73m3 driver is buggy here if it's only been
tested with the s3c64xx driver (which seems likely) since the author
could have worked around the driver behaviour.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH V4] spi: s3c64xx: Enable Word transfer

[Tomasz Figa]

On Friday 08 of November 2013 11:05:17 Mark Brown wrote:
> On Thu, Nov 07, 2013 at 10:39:37AM +0100, Tomasz Figa wrote:
> > On Thursday 07 of November 2013 14:52:36 Rajeshwari Birje wrote:
> 
> > > The reordering of the words is taken care by the Swap configuration
> > > (swap_cfg) register, which I have set for both 16 and 32 bits_per_word
> > > case. I have tested this patch and works fine for me and also has
> > > improved the timing performance.
> 
> > This driver seems to have already worked fine with 32 bits per word,
> > using the s5c73m3 camera sensor, which uses SPI for firmware upload.
> 
> This is all sounding very confused.  Either this patch is a bug fix or
> it's breaking the driver but I can't tell without more information about
> what it's supposed to do.  Can someone explain in concrete terms what
> the intended effect of the patch is on wire behaviour?
> 
> It is possible that the s5c73m3 driver is buggy here if it's only been
> tested with the s3c64xx driver (which seems likely) since the author
> could have worked around the driver behaviour.

I'm confused here as well.

Looking at the W25Q32DW datasheet[1], it does not really support 32 bit
words and using such with it seems more like an abuse of this feature.

As for the s5c73m3 sensor, we should ask Andrzej or Sylwester to look
at the datasheet and see how SPI transfer are supposed to work there.

[1] http://www.winbond.com.tw/NR/rdonlyres/D35D5D04-4CE9-46A1-8950-AEA573045507/0/W25Q32DW.pdf

Best regards,
Tomasz

Re: [PATCH V4] spi: s3c64xx: Enable Word transfer

[Andrzej Hajda]

On 11/10/2013 06:26 PM, Tomasz Figa wrote:
> On Friday 08 of November 2013 11:05:17 Mark Brown wrote:
>> On Thu, Nov 07, 2013 at 10:39:37AM +0100, Tomasz Figa wrote:
>>> On Thursday 07 of November 2013 14:52:36 Rajeshwari Birje wrote:
>>>> The reordering of the words is taken care by the Swap configuration
>>>> (swap_cfg) register, which I have set for both 16 and 32 bits_per_word
>>>> case. I have tested this patch and works fine for me and also has
>>>> improved the timing performance.
>>> This driver seems to have already worked fine with 32 bits per word,
>>> using the s5c73m3 camera sensor, which uses SPI for firmware upload.
>> This is all sounding very confused.  Either this patch is a bug fix or
>> it's breaking the driver but I can't tell without more information about
>> what it's supposed to do.  Can someone explain in concrete terms what
>> the intended effect of the patch is on wire behaviour?
>>
>> It is possible that the s5c73m3 driver is buggy here if it's only been
>> tested with the s3c64xx driver (which seems likely) since the author
>> could have worked around the driver behaviour.
> I'm confused here as well.
>
> Looking at the W25Q32DW datasheet[1], it does not really support 32 bit
> words and using such with it seems more like an abuse of this feature.
>
> As for the s5c73m3 sensor, we should ask Andrzej or Sylwester to look
> at the datasheet and see how SPI transfer are supposed to work there.
Hi all,

s5c73m3 sensor specs we have access to say nothing about the subject.
Driver is based on internal code, which is also not helpful in this matter.

Regards
Andrzej
>
> [1] http://www.winbond.com.tw/NR/rdonlyres/D35D5D04-4CE9-46A1-8950-AEA573045507/0/W25Q32DW.pdf
>
> Best regards,
> Tomasz
>
>

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html