Hi Mark Brown,
Please do let me know if you have any comments on this patch.
Regards,
Rajeshwari Shinde.
On Wed, Sep 25, 2013 at 4:59 PM, Rajeshwari Birje
<rajeshwari.birje@xxxxxxxxx> wrote:
> Hi Tomasz
>
> Sorry for the previous mail with incomplete comments, sent the same by mistake.
>
>
> On Wed, Sep 25, 2013 at 4:33 PM, Rajeshwari Birje
> <rajeshwari.birje@xxxxxxxxx> wrote:
>>
>> Hi Tomasz
>>
>>
>> On Tue, Sep 24, 2013 at 8:29 PM, Tomasz Figa <t.figa@xxxxxxxxxxx> wrote:
>>>
>>> Hi Rajeshwari,
>>>
>>> [Ccing Mark Brown]
>>>
>>> On Tuesday 24 of September 2013 19:30:35 Rajeshwari S Shinde wrote:
>>> > This patch enables word transfer for s3c64xx spi driver.
>>> > User can set bits_per_word to 32 before calling spi_setup,
>>> > which would enable the word transfer mode.
>>> >
>>> > Signed-off-by: Rajeshwari S Shinde <rajeshwari.s@xxxxxxxxxxx>
>>> > ---
>>> > drivers/spi/spi-s3c64xx.c | 28 ++++++++++++++++++----------
>>> > 1 file changed, 18 insertions(+), 10 deletions(-)
>>> >
>>> > diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
>>> > index 512b889..893361b 100644
>>> > --- a/drivers/spi/spi-s3c64xx.c
>>> > +++ b/drivers/spi/spi-s3c64xx.c
>>> > @@ -498,6 +498,17 @@ static void enable_datapath(struct s3c64xx_spi_driver_data *sdd,
>>> > chcfg = readl(regs + S3C64XX_SPI_CH_CFG);
>>> > chcfg &= ~S3C64XX_SPI_CH_TXCH_ON;
>>> >
>>> > + if(sdd->cur_bpw == 32) {
>>> > + /* For word transfer we need to swap bytes */
>>> > + u32 swapcfg = (S3C64XX_SPI_SWAP_TX_EN | S3C64XX_SPI_SWAP_TX_BYTE |
>>> > + S3C64XX_SPI_SWAP_TX_HALF_WORD |
>>> > + S3C64XX_SPI_SWAP_RX_EN |
>>> > + S3C64XX_SPI_SWAP_RX_BYTE |
>>> > + S3C64XX_SPI_SWAP_RX_HALF_WORD);
>>> > + writel(swapcfg, regs + S3C64XX_SPI_SWAP_CFG);
>>>
>>> Wasn't this driver already supposed to support 16 and 32 bits per word
>>> transfers? I fail to see any mention in the documentation about the need
>>> to enable these swaps in these cases.
>
> I get errors when I try 32 bits per word transfers.
> There is no clear documentation for this swap config register but without this
> word transfer is not working fine.
>>
>>
>>>
>>> In this case, wouldn't 16 bits per word transfers also need swapping?
>
> Yes it would need, have not tested for half word transfer.
>>>
>>>
>>> > + } else
>>> > + writel(0, regs + S3C64XX_SPI_SWAP_CFG);
>>>
>>> nit: Coding style. If you use braces for one branch, then you should use
>>> them for the other one as well.
>>
> will correct this.
>>>
>>>
>>> > +
>>> > if (dma_mode) {
>>> > chcfg &= ~S3C64XX_SPI_CH_RXCH_ON;
>>> > } else {
>>> > @@ -905,19 +916,16 @@ 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;
>>>
>>> Why is this check removed in this patch?
>
> I get "Xfer length(%u) not a multiple of word size(%u)\n" error for
> word transfer
> ex: when if bpw is 32 and len 74 the above condition fails
>>
>>
>>>
>>> > + if (speed != sdd->cur_speed) {
>>> > + sdd->cur_speed = speed;
>>>
>>> This change does not seem to be related to $subject.
>
> have split the if condition to accommodate the above error condition.
>>>
>>>
>>> > }
>>> >
>>> > - if (bpw != sdd->cur_bpw || speed != sdd->cur_speed) {
>>> > + if (xfer->len % (bpw / 8))
>>> > + sdd->cur_bpw = 8;
>>> > + else
>>> > sdd->cur_bpw = bpw;
>>> > - sdd->cur_speed = speed;
>>> > - s3c64xx_spi_config(sdd);
>>> > - }
>>> > +
>>> > + s3c64xx_spi_config(sdd);
>>>
>>> Why s3c64xx_spi_config() can't be called only if one of the parameters
>>> changed, as it was before this patch?
>
> Will correct this.
>>>
>>>
>>> Best regards,
>>> Tomasz
>>>
>>> P.S. Please remember to always add respective maintainers on Cc. You can
>>> use scripts/get_maintainer.pl script to quickly get a list of people
>>> potentially interested in a patch.
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
>>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> --
> Regards,
> Rajeshwari Shinde
--
Regards,
Rajeshwari Shinde
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
