[PATCH] SPI: DUAL and QUAD support

[yuhang wang]

Hi,

Yes, you are right. I will correct it.

Thanks

2013/7/29 Matthieu CASTET <matthieu.castet at parrot.com>:
> Le Mon, 29 Jul 2013 15:25:51 +0100,
> yuhang wang <wangyuhang2014 at gmail.com> a écrit :
>
>> Hi,
>>
>> 2013/7/29 Matthieu CASTET <matthieu.castet at parrot.com>:
>> > Le Mon, 29 Jul 2013 11:53:14 +0100,
>> > wangyuhang <wangyuhang2014 at gmail.com> a écrit :
>> >
>> >> Hi,
>> >>
>> >> modify two things.
>> >> 1:
>> >> >> @@ -1316,6 +1324,12 @@ int spi_setup(struct spi_device *spi)
>> >> >>         /* help drivers fail *cleanly* when they need options
>> >> >>          * that aren't supported with their current master
>> >> >>          */
>> >> >> +       if (((spi->mode >> 8) & 0x03) == 0x03 ||
>> >> >> +               ((spi->mode >> 10) & 0x03) == 0x03) {
>> >> >> +               dev_err(&spi->dev,
>> >> >> +               "setup: can not select dual and quad at the same
>> >> >> time\n")
>> >> >> +               return -EINVAL;
>> >> >> +       }
>> >> >> +               return -EINVAL;
>> >> >> +       }
>> >>
>> >> >This code won't work if the constants you added for mode flags
>> >> >change value.  More importantly, anyone searching the code for
>> >> >SPI_TX_DUAL, etc. won't find this code.
>> >>
>> >> so use the certain macro to replace.
>> >>
>> >> 2:
>> >> >You need to make sure that existing userspace binaries can run
>> >> >unmodified, changing the types will change the layout of the ioctl
>> >> >arguments.  This may mean that you need to add new ioctls if you
>> >> >need to change types.
>> >>
>> >> I recovered the SPI_IOC_RD_LSB_FIRST ioctl.
>> >> Add new ioctl to fit u16 mode.
>> >> In addition,may SPI_IOC_RD_LSB_FIRST should be del in a better way.
>> >>
>> >> Signed-off-by: wangyuhang <wangyuhang2014 at gmail.com>
>> >> ---
>> >
>> >>  /* IOCTL commands */
>> >> @@ -54,6 +61,8 @@
>> >>   * @tx_buf: Holds pointer to userspace buffer with transmit data,
>> >> or null.
>> >>   *     If no data is provided, zeroes are shifted out.
>> >>   * @rx_buf: Holds pointer to userspace buffer for receive data, or
>> >> null.
>> >> + * @tx_nbits: number of bits used for writting.
>> >> + * @rx_nbits: number of bits used for reading.
>> >>   * @len: Length of tx and rx buffers, in bytes.
>> >>   * @speed_hz: Temporary override of the device's bitrate.
>> >>   * @bits_per_word: Temporary override of the device's wordsize.
>> >> @@ -85,6 +94,8 @@
>> >>  struct spi_ioc_transfer {
>> >>         __u64           tx_buf;
>> >>         __u64           rx_buf;
>> >> +       __u8            tx_nbits;
>> >> +       __u8            rx_nbits;
>> >>
>> >>         __u32           len;
>> >>         __u32           speed_hz;
>> > You still break old userspace ABI.
>> >
>> > You need to create a new structure, if you need to add fields.
>> >
>> I dont think adding the members will cause the program in userspace
>> any errors. User do not have to set these members.
> Did you try to run on your patch kernel a program build without the
> patch ?
>
>
> if I build my program with :
>
>> >>  struct spi_ioc_transfer {
>> >>         __u64           tx_buf;
>> >>         __u64           rx_buf;
>> >>
>> >>         __u32           len;
>> >>         __u32           speed_hz;
>
> Now I run it on a new kernel.
> The kernel won't parse correctly my structure : len will go in
> tx_nbits, rx_nbits and len.
>
> Keep in mind that it is binary interface, not source interface. You
> should not have to rebuild your binary.
>
> Matthieu