* [PATCH] spi: spi_mpc8xxx.c: Fix QE mode Litte Endian @ 2010-05-14 9:14 Joakim Tjernlund [not found] ` <1273828467-4593-1-git-send-email-Joakim.Tjernlund-SNLAxHN9vbcOP4wsBPIw7w@public.gmane.org> 0 siblings, 1 reply; 8+ messages in thread From: Joakim Tjernlund @ 2010-05-14 9:14 UTC (permalink / raw) To: Anton Vorontsov, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f Cc: Joakim Tjernlund QE mode uses Little Endian so words > 8 bits are byte swapped. Workaround this by always enforcing wordsize 8 for words > 8 bits. Unfortunately this will not work for LSB transfers where wordsize is > 8 bits so disable these for now. Also move the different quirks into its own function to keep mpc8xxx_spi_setup_transfer() sane. Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund-SNLAxHN9vbcOP4wsBPIw7w@public.gmane.org> --- drivers/spi/spi_mpc8xxx.c | 101 +++++++++++++++++++++++++++++++-------------- 1 files changed, 70 insertions(+), 31 deletions(-) diff --git a/drivers/spi/spi_mpc8xxx.c b/drivers/spi/spi_mpc8xxx.c index c312d0e..c85dbbb 100644 --- a/drivers/spi/spi_mpc8xxx.c +++ b/drivers/spi/spi_mpc8xxx.c @@ -285,36 +285,12 @@ static void mpc8xxx_spi_chipselect(struct spi_device *spi, int value) } } -static -int mpc8xxx_spi_setup_transfer(struct spi_device *spi, struct spi_transfer *t) +static int +mspi_apply_cpu_mode_quirks(struct spi_mpc8xxx_cs *cs, + struct spi_device *spi, + struct mpc8xxx_spi *mpc8xxx_spi, + int bits_per_word) { - struct mpc8xxx_spi *mpc8xxx_spi; - u8 bits_per_word, pm; - u32 hz; - struct spi_mpc8xxx_cs *cs = spi->controller_state; - - mpc8xxx_spi = spi_master_get_devdata(spi->master); - - if (t) { - bits_per_word = t->bits_per_word; - hz = t->speed_hz; - } else { - bits_per_word = 0; - hz = 0; - } - - /* spi_transfer level calls that work per-word */ - if (!bits_per_word) - bits_per_word = spi->bits_per_word; - - /* Make sure its a bit width we support [4..16, 32] */ - if ((bits_per_word < 4) - || ((bits_per_word > 16) && (bits_per_word != 32))) - return -EINVAL; - - if (!hz) - hz = spi->max_speed_hz; - cs->rx_shift = 0; cs->tx_shift = 0; if (bits_per_word <= 8) { @@ -338,19 +314,82 @@ int mpc8xxx_spi_setup_transfer(struct spi_device *spi, struct spi_transfer *t) return -EINVAL; if (mpc8xxx_spi->flags & SPI_QE_CPU_MODE && - spi->mode & SPI_LSB_FIRST) { + spi->mode & SPI_LSB_FIRST) { cs->tx_shift = 0; if (bits_per_word <= 8) cs->rx_shift = 8; else cs->rx_shift = 0; } - mpc8xxx_spi->rx_shift = cs->rx_shift; mpc8xxx_spi->tx_shift = cs->tx_shift; mpc8xxx_spi->get_rx = cs->get_rx; mpc8xxx_spi->get_tx = cs->get_tx; + return bits_per_word; +} + +static int +mspi_apply_qe_mode_quirks(struct spi_mpc8xxx_cs *cs, + struct spi_device *spi, + int bits_per_word) +{ + /* QE uses Little Endian for words > 8 + * so transform all words > 8 into 8 bits + * Unfortnatly that doesn't work for LSB so + * reject these for now */ + /* Note: 32 bits word, LSB works iff + * tfcr/rfcr is set to CPMFCR_GBL */ + if (spi->mode & SPI_LSB_FIRST && + bits_per_word > 8) + return -EINVAL; + if (bits_per_word > 8) + return 8; /* pretend its 8 bits */ + return bits_per_word; +} + +static +int mpc8xxx_spi_setup_transfer(struct spi_device *spi, struct spi_transfer *t) +{ + struct mpc8xxx_spi *mpc8xxx_spi; + int bits_per_word; + u8 pm; + u32 hz; + struct spi_mpc8xxx_cs *cs = spi->controller_state; + + mpc8xxx_spi = spi_master_get_devdata(spi->master); + + if (t) { + bits_per_word = t->bits_per_word; + hz = t->speed_hz; + } else { + bits_per_word = 0; + hz = 0; + } + + /* spi_transfer level calls that work per-word */ + if (!bits_per_word) + bits_per_word = spi->bits_per_word; + + /* Make sure its a bit width we support [4..16, 32] */ + if ((bits_per_word < 4) + || ((bits_per_word > 16) && (bits_per_word != 32))) + return -EINVAL; + + if (!hz) + hz = spi->max_speed_hz; + + if (!(mpc8xxx_spi->flags & SPI_CPM_MODE)) + bits_per_word = mspi_apply_cpu_mode_quirks(cs, spi, + mpc8xxx_spi, + bits_per_word); + else if (mpc8xxx_spi->flags & SPI_QE) + bits_per_word = mspi_apply_qe_mode_quirks(cs, spi, + bits_per_word); + + if (bits_per_word < 0) + return bits_per_word; + if (bits_per_word == 32) bits_per_word = 0; else -- 1.6.4.4 ------------------------------------------------------------------------------ ^ permalink raw reply related [flat|nested] 8+ messages in thread
[parent not found: <1273828467-4593-1-git-send-email-Joakim.Tjernlund-SNLAxHN9vbcOP4wsBPIw7w@public.gmane.org>]
* [PATCH] spi: spidev_test.c: Make transfer size suitable for wordsize 32 [not found] ` <1273828467-4593-1-git-send-email-Joakim.Tjernlund-SNLAxHN9vbcOP4wsBPIw7w@public.gmane.org> @ 2010-05-14 9:14 ` Joakim Tjernlund [not found] ` <1273828467-4593-2-git-send-email-Joakim.Tjernlund-SNLAxHN9vbcOP4wsBPIw7w@public.gmane.org> 2010-05-14 12:22 ` [PATCH] spi: spi_mpc8xxx.c: Fix QE mode Litte Endian Anton Vorontsov 1 sibling, 1 reply; 8+ messages in thread From: Joakim Tjernlund @ 2010-05-14 9:14 UTC (permalink / raw) To: Anton Vorontsov, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f Cc: Joakim Tjernlund Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund-SNLAxHN9vbcOP4wsBPIw7w@public.gmane.org> --- Documentation/spi/spidev_test.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/Documentation/spi/spidev_test.c b/Documentation/spi/spidev_test.c index 10abd37..3978e69 100644 --- a/Documentation/spi/spidev_test.c +++ b/Documentation/spi/spidev_test.c @@ -45,7 +45,6 @@ static void transfer(int fd) 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xDE, 0xAD, 0xBE, 0xEF, 0xBA, 0xAD, - 0xF0, 0x0D, }; uint8_t rx[ARRAY_SIZE(tx)] = {0, }; struct spi_ioc_transfer tr = { -- 1.6.4.4 ------------------------------------------------------------------------------ ^ permalink raw reply related [flat|nested] 8+ messages in thread
[parent not found: <1273828467-4593-2-git-send-email-Joakim.Tjernlund-SNLAxHN9vbcOP4wsBPIw7w@public.gmane.org>]
* Re: [PATCH] spi: spidev_test.c: Make transfer size suitable for wordsize 32 [not found] ` <1273828467-4593-2-git-send-email-Joakim.Tjernlund-SNLAxHN9vbcOP4wsBPIw7w@public.gmane.org> @ 2010-05-14 12:22 ` Anton Vorontsov [not found] ` <20100514122239.GB24394-wnGakbxT3iijyJ0x5qLZdcN33GVbZNy3@public.gmane.org> 0 siblings, 1 reply; 8+ messages in thread From: Anton Vorontsov @ 2010-05-14 12:22 UTC (permalink / raw) To: Joakim Tjernlund; +Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f On Fri, May 14, 2010 at 11:14:27AM +0200, Joakim Tjernlund wrote: > > Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund-SNLAxHN9vbcOP4wsBPIw7w@public.gmane.org> Acked-by: Anton Vorontsov <cbouatmailru-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> Thanks! > --- > Documentation/spi/spidev_test.c | 1 - > 1 files changed, 0 insertions(+), 1 deletions(-) > > diff --git a/Documentation/spi/spidev_test.c b/Documentation/spi/spidev_test.c > index 10abd37..3978e69 100644 > --- a/Documentation/spi/spidev_test.c > +++ b/Documentation/spi/spidev_test.c > @@ -45,7 +45,6 @@ static void transfer(int fd) > 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, > 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, > 0xDE, 0xAD, 0xBE, 0xEF, 0xBA, 0xAD, > - 0xF0, 0x0D, > }; > uint8_t rx[ARRAY_SIZE(tx)] = {0, }; > struct spi_ioc_transfer tr = { > -- > 1.6.4.4 -- Anton Vorontsov email: cbouatmailru-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org irc://irc.freenode.net/bd2 ------------------------------------------------------------------------------ ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <20100514122239.GB24394-wnGakbxT3iijyJ0x5qLZdcN33GVbZNy3@public.gmane.org>]
* Re: [PATCH] spi: spidev_test.c: Make transfer size suitable for wordsize 32 [not found] ` <20100514122239.GB24394-wnGakbxT3iijyJ0x5qLZdcN33GVbZNy3@public.gmane.org> @ 2010-05-22 8:39 ` Grant Likely [not found] ` <AANLkTinWmX39klqQvBPpZY44YdTK3ESb6uu6EomW7fZb-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 8+ messages in thread From: Grant Likely @ 2010-05-22 8:39 UTC (permalink / raw) To: Anton Vorontsov Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Joakim Tjernlund On Fri, May 14, 2010 at 6:22 AM, Anton Vorontsov <cbouatmailru-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: > On Fri, May 14, 2010 at 11:14:27AM +0200, Joakim Tjernlund wrote: >> >> Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund-SNLAxHN9vbcOP4wsBPIw7w@public.gmane.org> > > Acked-by: Anton Vorontsov <cbouatmailru-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> I don't think I'll apply this one. I know it's just sample code, but it still smells of masking a corner case. If you really want to do this, then I'd suggest adding a runtime switch to change the transfer size. g. ------------------------------------------------------------------------------ ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <AANLkTinWmX39klqQvBPpZY44YdTK3ESb6uu6EomW7fZb-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH] spi: spidev_test.c: Make transfer size suitable for wordsize 32 [not found] ` <AANLkTinWmX39klqQvBPpZY44YdTK3ESb6uu6EomW7fZb-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2010-05-22 14:34 ` Joakim Tjernlund [not found] ` <OF066BF1BA.ACBE06A8-ONC125772B.004FB6E4-C125772B.00501681-SNLAxHN9vbcOP4wsBPIw7w@public.gmane.org> 0 siblings, 1 reply; 8+ messages in thread From: Joakim Tjernlund @ 2010-05-22 14:34 UTC (permalink / raw) To: Grant Likely Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, glikely-s3s/WqlpOiPyB63q8FvJNQ glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org wrote on 2010/05/22 10:39:38: > > On Fri, May 14, 2010 at 6:22 AM, Anton Vorontsov <cbouatmailru-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: > > On Fri, May 14, 2010 at 11:14:27AM +0200, Joakim Tjernlund wrote: > >> > >> Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund-SNLAxHN9vbcOP4wsBPIw7w@public.gmane.org> > > > > Acked-by: Anton Vorontsov <cbouatmailru-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> > > I don't think I'll apply this one. I know it's just sample code, but > it still smells of masking a corner case. If you really want to do > this, then I'd suggest adding a runtime switch to change the transfer > size. How so? The current version don't stand a chance to do any ops. with word size 32 as all transfers has to have a wordsize aligned len. This is not a corner case. Jocke ------------------------------------------------------------------------------ ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <OF066BF1BA.ACBE06A8-ONC125772B.004FB6E4-C125772B.00501681-SNLAxHN9vbcOP4wsBPIw7w@public.gmane.org>]
* Re: [PATCH] spi: spidev_test.c: Make transfer size suitable for wordsize 32 [not found] ` <OF066BF1BA.ACBE06A8-ONC125772B.004FB6E4-C125772B.00501681-SNLAxHN9vbcOP4wsBPIw7w@public.gmane.org> @ 2010-05-25 19:41 ` Grant Likely [not found] ` <AANLkTinZUXo2fjD6ijdTWfDhom6TF6e0goiXuVUshPxN-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 8+ messages in thread From: Grant Likely @ 2010-05-25 19:41 UTC (permalink / raw) To: Joakim Tjernlund; +Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f On Sat, May 22, 2010 at 8:34 AM, Joakim Tjernlund <joakim.tjernlund-SNLAxHN9vbcOP4wsBPIw7w@public.gmane.org> wrote: > glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org wrote on 2010/05/22 10:39:38: >> >> On Fri, May 14, 2010 at 6:22 AM, Anton Vorontsov <cbouatmailru-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: >> > On Fri, May 14, 2010 at 11:14:27AM +0200, Joakim Tjernlund wrote: >> >> >> >> Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund-SNLAxHN9vbcOP4wsBPIw7w@public.gmane.org> >> > >> > Acked-by: Anton Vorontsov <cbouatmailru-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> >> >> I don't think I'll apply this one. I know it's just sample code, but >> it still smells of masking a corner case. If you really want to do >> this, then I'd suggest adding a runtime switch to change the transfer >> size. > > How so? The current version don't stand a chance to do any ops. with word size 32 > as all transfers has to have a wordsize aligned len. This is not a corner case. but it also changes the transfer size for 16 and 8 bpw transfers to something, for lack of a better word, more boring. I'd rather see a patch that clamps the transfers size based on the bpw setting. Bonus points for a patch that adds the ability to change the transfer size and/or data from the command line. Cheers, g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ------------------------------------------------------------------------------ ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <AANLkTinZUXo2fjD6ijdTWfDhom6TF6e0goiXuVUshPxN-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH] spi: spidev_test.c: Make transfer size suitable for wordsize 32 [not found] ` <AANLkTinZUXo2fjD6ijdTWfDhom6TF6e0goiXuVUshPxN-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2010-05-25 20:28 ` Joakim Tjernlund 0 siblings, 0 replies; 8+ messages in thread From: Joakim Tjernlund @ 2010-05-25 20:28 UTC (permalink / raw) To: Grant Likely Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, glikely-s3s/WqlpOiPyB63q8FvJNQ glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org wrote on 2010/05/25 21:41:15: > > On Sat, May 22, 2010 at 8:34 AM, Joakim Tjernlund > <joakim.tjernlund-SNLAxHN9vbcOP4wsBPIw7w@public.gmane.org> wrote: > > glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org wrote on 2010/05/22 10:39:38: > >> > >> On Fri, May 14, 2010 at 6:22 AM, Anton Vorontsov <cbouatmailru@gmail.com> wrote: > >> > On Fri, May 14, 2010 at 11:14:27AM +0200, Joakim Tjernlund wrote: > >> >> > >> >> Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund-SNLAxHN9vbcOP4wsBPIw7w@public.gmane.org> > >> > > >> > Acked-by: Anton Vorontsov <cbouatmailru-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> > >> > >> I don't think I'll apply this one. I know it's just sample code, but > >> it still smells of masking a corner case. If you really want to do > >> this, then I'd suggest adding a runtime switch to change the transfer > >> size. > > > > How so? The current version don't stand a chance to do any ops. with word size 32 > > as all transfers has to have a wordsize aligned len. This is not a corner case. > > but it also changes the transfer size for 16 and 8 bpw transfers to > something, for lack of a better word, more boring. I'd rather see a That is a rather lame argument. > patch that clamps the transfers size based on the bpw setting. Bonus > points for a patch that adds the ability to change the transfer size > and/or data from the command line. This is better though and yes it would be much better if one could control the size. I didn't bother with that though as I just wanted to fix the driver and the current one doesn't work at all so I did the minimum fix to get it going. I am fine either way whether you apply this patch or not, but I won't work on fixing spidev_test until the next time I need it though. Too much other stuff on my plate and the test eq. has already been claimed by others. ------------------------------------------------------------------------------ ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] spi: spi_mpc8xxx.c: Fix QE mode Litte Endian [not found] ` <1273828467-4593-1-git-send-email-Joakim.Tjernlund-SNLAxHN9vbcOP4wsBPIw7w@public.gmane.org> 2010-05-14 9:14 ` [PATCH] spi: spidev_test.c: Make transfer size suitable for wordsize 32 Joakim Tjernlund @ 2010-05-14 12:22 ` Anton Vorontsov 1 sibling, 0 replies; 8+ messages in thread From: Anton Vorontsov @ 2010-05-14 12:22 UTC (permalink / raw) To: Joakim Tjernlund; +Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f On Fri, May 14, 2010 at 11:14:26AM +0200, Joakim Tjernlund wrote: > QE mode uses Little Endian so words > 8 bits are byte swapped. > Workaround this by always enforcing wordsize 8 for words > > 8 bits. Unfortunately this will not work for LSB transfers > where wordsize is > 8 bits so disable these for now. > > Also move the different quirks into its own function to keep > mpc8xxx_spi_setup_transfer() sane. > > Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund-SNLAxHN9vbcOP4wsBPIw7w@public.gmane.org> > --- Looks OK, thanks! Acked-by: Anton Vorontsov <cbouatmailru-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> -- Anton Vorontsov email: cbouatmailru-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org irc://irc.freenode.net/bd2 ------------------------------------------------------------------------------ ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2010-05-25 20:28 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2010-05-14 9:14 [PATCH] spi: spi_mpc8xxx.c: Fix QE mode Litte Endian Joakim Tjernlund [not found] ` <1273828467-4593-1-git-send-email-Joakim.Tjernlund-SNLAxHN9vbcOP4wsBPIw7w@public.gmane.org> 2010-05-14 9:14 ` [PATCH] spi: spidev_test.c: Make transfer size suitable for wordsize 32 Joakim Tjernlund [not found] ` <1273828467-4593-2-git-send-email-Joakim.Tjernlund-SNLAxHN9vbcOP4wsBPIw7w@public.gmane.org> 2010-05-14 12:22 ` Anton Vorontsov [not found] ` <20100514122239.GB24394-wnGakbxT3iijyJ0x5qLZdcN33GVbZNy3@public.gmane.org> 2010-05-22 8:39 ` Grant Likely [not found] ` <AANLkTinWmX39klqQvBPpZY44YdTK3ESb6uu6EomW7fZb-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2010-05-22 14:34 ` Joakim Tjernlund [not found] ` <OF066BF1BA.ACBE06A8-ONC125772B.004FB6E4-C125772B.00501681-SNLAxHN9vbcOP4wsBPIw7w@public.gmane.org> 2010-05-25 19:41 ` Grant Likely [not found] ` <AANLkTinZUXo2fjD6ijdTWfDhom6TF6e0goiXuVUshPxN-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2010-05-25 20:28 ` Joakim Tjernlund 2010-05-14 12:22 ` [PATCH] spi: spi_mpc8xxx.c: Fix QE mode Litte Endian Anton Vorontsov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).