Suggested Patch for 'amba-pl022.c' to solve the problem of running 'spidev_test' on Phytec LPC3250 board.

Suggested Patch for 'amba-pl022.c' to solve the problem of running 'spidev_test' on Phytec LPC3250 board.

[Quentin YANG]

Hi,

I was having trouble to run 'spidev_test' on Phytec LPC3250 board when I
tried to change the SPI mode.

[root@GsnCommsModule debug]# ./spidev_test -O
can't set spi mode: Invalid argument
Aborted

'dmesg' shows:
......
spidev spi1.0: setup: unsupported mode bits 2
......


According to source code: driver/spi/spi.c
............................
    bad_bits = spi->mode & ~spi->master->mode_bits;
    if (bad_bits) {
        dev_dbg(&spi->dev, "setup: unsupported mode bits %x\n",
            bad_bits);
        return -EINVAL;
    }
..............................


Therefore, I did following change on amba-pl022.c, which is used as SPI
master controller driver
-------------------------------------------------------------------

pl022_probe(struct amba_device *adev, struct amba_id *id)
{
       ...................................

    /* Allocate master with space for data */
    master = spi_alloc_master(dev, sizeof(struct pl022));
    if (master == NULL) {
        dev_err(&adev->dev, "probe - cannot alloc SPI master\n");
        status = -ENOMEM;
        goto err_no_master;

    }

        /* Quentin Added 2010-AUG-19 */
        master->mode_bits=MODEBITS;
    .........................

}

Now I can successfully set 'SPI mode' to mode 3.

[root@GsnCommsModule debug]# ./spidev_test -OH
spi mode: 3
bits per word: 8
max speed: 500000 Hz (500 KHz)
00 00 00 00 00 00
00 00 00 00 00 00
00 00 00 00 00 00
00 00 00 00 00 00
00 00 00 00 00 00
00 00 00 00 00 00
00 00


Hope this patch can be added to amba-pl022.c in the next Linux kernel
release.


Regards,

*Quentin YANG***

*Firmware Engineer***

*Tel: +61-2-88787724***

*CHK GridSense Pty Ltd*

Unit 3, 20-36 Nancarrow Ave

Meadowbank, NSW, 2114, Australia

Web Site:  http://www.gridsense.com
------------------------------------------------------------------------------
This SF.net email is sponsored by 

Make an app they can't live without
Enter the BlackBerry Developer Challenge
http://p.sf.net/sfu/RIM-dev2dev 

Re: Suggested Patch for 'amba-pl022.c' to solve the problem of running 'spidev_test' on Phytec LPC3250 board.

[Quentin YANG]

Hi,

Forgot to say that I am running Linux Kernel 2.6.34 with LPC32xx patch from
NXP.

Regards,
Quentin



On Fri, Aug 20, 2010 at 9:38 AM, Quentin YANG <quentinyang05-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>wrote:

> Hi,
>
> I was having trouble to run 'spidev_test' on Phytec LPC3250 board when I
> tried to change the SPI mode.
>
> [root@GsnCommsModule debug]# ./spidev_test -O
> can't set spi mode: Invalid argument
> Aborted
>
> 'dmesg' shows:
> ......
> spidev spi1.0: setup: unsupported mode bits 2
> ......
>
>
> According to source code: driver/spi/spi.c
> ............................
>     bad_bits = spi->mode & ~spi->master->mode_bits;
>     if (bad_bits) {
>         dev_dbg(&spi->dev, "setup: unsupported mode bits %x\n",
>             bad_bits);
>         return -EINVAL;
>     }
> ..............................
>
>
> Therefore, I did following change on amba-pl022.c, which is used as SPI
> master controller driver
> -------------------------------------------------------------------
>
> pl022_probe(struct amba_device *adev, struct amba_id *id)
> {
>        ...................................
>
>     /* Allocate master with space for data */
>     master = spi_alloc_master(dev, sizeof(struct pl022));
>     if (master == NULL) {
>         dev_err(&adev->dev, "probe - cannot alloc SPI master\n");
>         status = -ENOMEM;
>         goto err_no_master;
>
>     }
>
>         /* Quentin Added 2010-AUG-19 */
>         master->mode_bits=MODEBITS;
>      .........................
>
> }
>
> Now I can successfully set 'SPI mode' to mode 3.
>
> [root@GsnCommsModule debug]# ./spidev_test -OH
> spi mode: 3
> bits per word: 8
> max speed: 500000 Hz (500 KHz)
> 00 00 00 00 00 00
> 00 00 00 00 00 00
> 00 00 00 00 00 00
> 00 00 00 00 00 00
> 00 00 00 00 00 00
> 00 00 00 00 00 00
> 00 00
>
>
> Hope this patch can be added to amba-pl022.c in the next Linux kernel
> release.
>
>
> Regards,
>
> *Quentin YANG***
>
> *Firmware Engineer***
>
> *Tel: +61-2-88787724***
>
> *CHK GridSense Pty Ltd*
>
> Unit 3, 20-36 Nancarrow Ave
>
> Meadowbank, NSW, 2114, Australia
>
> Web Site:  http://www.gridsense.com
>
>
>
>
------------------------------------------------------------------------------
This SF.net email is sponsored by 

Make an app they can't live without
Enter the BlackBerry Developer Challenge
http://p.sf.net/sfu/RIM-dev2dev 

Re: Suggested Patch for 'amba-pl022.c' to solve the problem of running 'spidev_test' on Phytec LPC3250 board.

[Linus Walleij]

2010/8/20 Quentin YANG <quentinyang05-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:

> Hope this patch can be added to amba-pl022.c in the next Linux kernel
> release.

Sounds like it's OK but please prepare a real patch instead of code
snippets, i.e. easiest code it up in git and git format-patch ...

Yours,
Linus Walleij

------------------------------------------------------------------------------
This SF.net email is sponsored by 

Make an app they can't live without
Enter the BlackBerry Developer Challenge
http://p.sf.net/sfu/RIM-dev2dev 

Re: Suggested Patch for 'amba-pl022.c' to solve the problem of running 'spidev_test' on Phytec LPC3250 board.

[Kevin Wells]

> pl022_probe(struct amba_device *adev, struct amba_id *id)
> {
>       ...................................
>
>    /* Allocate master with space for data */
>    master = spi_alloc_master(dev, sizeof(struct pl022));
>    if (master == NULL) {
>        dev_err(&adev->dev, "probe - cannot alloc SPI master\n");
>        status = -ENOMEM;
>        goto err_no_master;
>
>    }
>
>        /* Quentin Added 2010-AUG-19 */
>        master->mode_bits=MODEBITS;
>    .........................
>
> }
>
> Now I can successfully set 'SPI mode' to mode 3.
>

It looks like some additional setup is needed in pl022_setup() to setup these
modes in hardware. Setting the mode_bits mask alone only tricks the
application into thinking SPI mode 3 has been set, but nothing will change
in hardware on the next setup if the passed mode isn't handled there too.

Some of the transfer setup config - data width, clock rate, SPI mode,
etc - are actually setup on a per transfer basis and may vary based on how
spidev (and in this case the spidev_test app) configures it. For these specific
setup items, we probably shouldn't use the static chip_info version of the
transfer config - maybe we can use the config passed in 'struct spi_transfer'
instead.

This would also reduce the size of the struct pl022_config_chip controller
data structure since some of those fields would now be handled in the
spi_transfer structure.

I guess this change is bigger than anticipated :(

> [root@GsnCommsModule debug]# ./spidev_test -OH
> spi mode: 3
> bits per word: 8
> max speed: 500000 Hz (500 KHz)
> 00 00 00 00 00 00
> 00 00 00 00 00 00
> 00 00 00 00 00 00
> 00 00 00 00 00 00
> 00 00 00 00 00 00
> 00 00 00 00 00 00
> 00 00
>
>
> Hope this patch can be added to amba-pl022.c in the next Linux kernel
> release.
>
>
> Regards,
>
> *Quentin YANG***
>
> *Firmware Engineer***
>
> *Tel: +61-2-88787724***
>
> *CHK GridSense Pty Ltd*
>
> Unit 3, 20-36 Nancarrow Ave
>
> Meadowbank, NSW, 2114, Australia
>
> Web Site:  http://www.gridsense.com
> ------------------------------------------------------------------------------
> This SF.net email is sponsored by
>
> Make an app they can't live without
> Enter the BlackBerry Developer Challenge
> http://p.sf.net/sfu/RIM-dev2dev
> _______________________________________________
> spi-devel-general mailing list
> spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> https://lists.sourceforge.net/lists/listinfo/spi-devel-general
>

------------------------------------------------------------------------------
This SF.net email is sponsored by 

Make an app they can't live without
Enter the BlackBerry Developer Challenge
http://p.sf.net/sfu/RIM-dev2dev 

Re: Suggested Patch for 'amba-pl022.c' to solve the problem of running 'spidev_test' on Phytec LPC3250 board.

[Kevin Wells]

>>        /* Quentin Added 2010-AUG-19 */
>>        master->mode_bits=MODEBITS;
>>    .........................
>>

>
> It looks like some additional setup is needed in pl022_setup() to setup these
> modes in hardware. Setting the mode_bits mask alone only tricks the
> application into thinking SPI mode 3 has been set, but nothing will change
> in hardware on the next setup if the passed mode isn't handled there too.
>
> Some of the transfer setup config - data width, clock rate, SPI mode,
> etc - are actually setup on a per transfer basis and may vary based on how
> spidev (and in this case the spidev_test app) configures it. For these specific
> setup items, we probably shouldn't use the static chip_info version of the
> transfer config - maybe we can use the config passed in 'struct spi_transfer'
> instead.
>
> This would also reduce the size of the struct pl022_config_chip controller
> data structure since some of those fields would now be handled in the
> spi_transfer structure.
>
> I guess this change is bigger than anticipated :(
>

Hi Linus,

I use this driver quite a bit here - Is it ok if try to get the setup based on
spi->mode instead of the controller_data info?

thanks,
Kevin

------------------------------------------------------------------------------
This SF.net Dev2Dev email is sponsored by:

Show off your parallel programming skills.
Enter the Intel(R) Threading Challenge 2010.
http://p.sf.net/sfu/intel-thread-sfd

Re: Suggested Patch for 'amba-pl022.c' to solve the problem of running 'spidev_test' on Phytec LPC3250 board.

[Linus Walleij]

2010/9/1 Kevin Wells <wellsk40-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:

> I use this driver quite a bit here - Is it ok if try to get the setup based on
> spi->mode instead of the controller_data info?

I think that the stuff that comes from the modebits should take
precedence, and also I believe the relevant platform data should
be removed.

I cannot fully undertand what this means and I never used SPI
from userspace, can't you just provide a patch I can look on,
that is easiest I think. I will test it on my hardware and see what
happens.

Yours,
Linus Walleij

------------------------------------------------------------------------------
This SF.net Dev2Dev email is sponsored by:

Show off your parallel programming skills.
Enter the Intel(R) Threading Challenge 2010.
http://p.sf.net/sfu/intel-thread-sfd

RE: Suggested Patch for 'amba-pl022.c' to solve the problem of running 'spidev_test' on Phytec LPC3250 board.

[Kevin Wells]

> 2010/9/1 Kevin Wells <wellsk40-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
> 
> > I use this driver quite a bit here - Is it ok if try to get the setup
> based on
> > spi->mode instead of the controller_data info?
> 
> I think that the stuff that comes from the modebits should take
> precedence, and also I believe the relevant platform data should
> be removed.
> 
> I cannot fully undertand what this means and I never used SPI
> from userspace, can't you just provide a patch I can look on,
> that is easiest I think. I will test it on my hardware and see what
> happens.
> 

Hi Linus,

I put up a patch for this late yesterday.
It won't break any existing platform code, although some of the fields
in the SPI board info structure are no longer used. I'll remove those
fields in a follow-up series.

The only real change in this patch is that the SPI setup now comes
from the individual drivers that use SPI instead of the board info.

thanks,
Kevin

> Yours,
> Linus Walleij
> 
> -----------------------------------------------------------------------

------------------------------------------------------------------------------
This SF.net Dev2Dev email is sponsored by:

Show off your parallel programming skills.
Enter the Intel(R) Threading Challenge 2010.
http://p.sf.net/sfu/intel-thread-sfd