[01/11] spi-dw: expose platform data stucture.

Commit Message

[e-mail] June 23, 2011, 2 a.m. UTC

From: Dirk Brandewie <[e-mail]>

Expose the platform data structure for use by client drivers. ATM
there are not any in-tree drivers using the driver (that I can
find). This patch exposes the platform data needed for client drivers.

Signed-off-by: Dirk Brandewie <[e-mail]>
---
 drivers/spi/spi-dw.c       |    3 ---
 drivers/spi/spi-dw.h       |   18 +-----------------
 include/linux/spi/spi-dw.h |   42 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 43 insertions(+), 20 deletions(-)
 create mode 100644 include/linux/spi/spi-dw.h

Comments

Grant Likely June 23, 2011, 3:47 a.m. UTC | #1

On Wed, Jun 22, 2011 at 8:00 PM,  <[e-mail]> wrote:
> From: Dirk Brandewie <[e-mail]>
>
> Expose the platform data structure for use by client drivers. ATM
> there are not any in-tree drivers using the driver (that I can
> find). This patch exposes the platform data needed for client drivers.

?  Why would client drivers want to muck with this configuration?  I
can understand the dw_spi driver being able to have per-spi_device
configuration, but spi_drivers absolutely should not have visibility
into bus-specific details.  Am I misunderstanding something.

g.

>
> Signed-off-by: Dirk Brandewie <[e-mail]>
> ---
>  drivers/spi/spi-dw.c       |    3 ---
>  drivers/spi/spi-dw.h       |   18 +-----------------
>  include/linux/spi/spi-dw.h |   42 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 43 insertions(+), 20 deletions(-)
>  create mode 100644 include/linux/spi/spi-dw.h
>
> diff --git a/drivers/spi/spi-dw.c b/drivers/spi/spi-dw.c
> index ece5f69..61f7ed8 100644
> --- a/drivers/spi/spi-dw.c
> +++ b/drivers/spi/spi-dw.c
> @@ -38,9 +38,6 @@
>  #define QUEUE_RUNNING  0
>  #define QUEUE_STOPPED  1
>
> -#define MRST_SPI_DEASSERT      0
> -#define MRST_SPI_ASSERT                1
> -
>  /* Slave spi_dev related */
>  struct chip_data {
>        u16 cr0;
> diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
> index 7a5e78d..92bee30 100644
> --- a/drivers/spi/spi-dw.h
> +++ b/drivers/spi/spi-dw.h
> @@ -3,6 +3,7 @@
>
>  #include <linux/io.h>
>  #include <linux/scatterlist.h>
> +#include <linux/spi/spi-dw.h>
>
>  /* Bit fields in CTRLR0 */
>  #define SPI_DFS_OFFSET                 0
> @@ -49,11 +50,6 @@
>  /* TX RX interrupt level threshold, max can be 256 */
>  #define SPI_INT_THRESHOLD              32
>
> -enum dw_ssi_type {
> -       SSI_MOTO_SPI = 0,
> -       SSI_TI_SSP,
> -       SSI_NS_MICROWIRE,
> -};
>
>  struct dw_spi_reg {
>        u32     ctrl0;
> @@ -208,18 +204,6 @@ static inline void spi_umask_intr(struct dw_spi *dws, u32 mask)
>        dw_writel(dws, imr, new_mask);
>  }
>
> -/*
> - * Each SPI slave device to work with dw_api controller should
> - * has such a structure claiming its working mode (PIO/DMA etc),
> - * which can be save in the "controller_data" member of the
> - * struct spi_device
> - */
> -struct dw_spi_chip {
> -       u8 poll_mode;   /* 0 for contoller polling mode */
> -       u8 type;        /* SPI/SSP/Micrwire */
> -       u8 enable_dma;
> -       void (*cs_control)(u32 command);
> -};
>
>  extern int dw_spi_add_host(struct dw_spi *dws);
>  extern void dw_spi_remove_host(struct dw_spi *dws);
> diff --git a/include/linux/spi/spi-dw.h b/include/linux/spi/spi-dw.h
> new file mode 100644
> index 0000000..787b154
> --- /dev/null
> +++ b/include/linux/spi/spi-dw.h
> @@ -0,0 +1,42 @@
> +/*
> + *
> + * Copyright (c) 2009, Intel Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program; if not, write to the Free Software Foundation, Inc.,
> + * 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
> + */
> +
> +#ifndef _SPI_DW_H_
> +#define _SPI_DW_H_
> +enum spi_dw_ssi_type {
> +       SSI_MOTO_SPI = 0,
> +       SSI_TI_SSP,
> +       SSI_NS_MICROWIRE,
> +};
> +
> +#define MRST_SPI_DEASSERT      0
> +#define MRST_SPI_ASSERT                1
> +
> +/*
> + * Each SPI slave device to work with dw_api controller should
> + * has such a structure claiming its working mode (PIO/DMA etc),
> + * which can be save in the "controller_data" member of the
> + * struct spi_device
> + */
> +struct spi_dw_chip {
> +       u8 poll_mode;   /* 0 for contoller polling mode */
> +       u8 type;        /* SPI/SSP/Micrwire */
> +       u8 enable_dma;
> +       void (*cs_control)(u32 command);
> +};
> +#endif
> --
> 1.7.3.4
>
>
> ------------------------------------------------------------------------------
> Simplify data backup and recovery for your virtual environment with vRanger.
> Installation's a snap, and flexible recovery options mean your data is safe,
> secure and there when you need it. Data protection magic?
> Nope - It's vRanger. Get your free trial download today.
> http://p.sf.net/sfu/quest-sfdev2dev
> _______________________________________________
> spi-devel-general mailing list
> [e-mail]
> https://lists.sourceforge.net/lists/listinfo/spi-devel-general
>

[e-mail] June 23, 2011, 4 a.m. UTC | #2

On 06/22/2011 08:47 PM, Grant Likely wrote:
> On Wed, Jun 22, 2011 at 8:00 PM,<[e-mail]>  wrote:
>> From: Dirk Brandewie<[e-mail]>
>>
>> Expose the platform data structure for use by client drivers. ATM
>> there are not any in-tree drivers using the driver (that I can
>> find). This patch exposes the platform data needed for client drivers.
>
> ?  Why would client drivers want to muck with this configuration?  I
> can understand the dw_spi driver being able to have per-spi_device
> configuration, but spi_drivers absolutely should not have visibility
> into bus-specific details.  Am I misunderstanding something.
>

Most of these config options don't need to be client configurable IMHO but they 
are being used ATM by drivers that aren't upstream and the current controller 
driver uses them.  This patch is to give a smooth transition (bisectable) to my 
change that reworks the core message and transfer handling code.

This allows me to provide patches to the developers of the out of tree drivers 
that should be coming in RSN and exposes the interface they are using now.

--Dirk
> g.
>
>>
>> Signed-off-by: Dirk Brandewie<[e-mail]>
>> ---
>>   drivers/spi/spi-dw.c       |    3 ---
>>   drivers/spi/spi-dw.h       |   18 +-----------------
>>   include/linux/spi/spi-dw.h |   42 ++++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 43 insertions(+), 20 deletions(-)
>>   create mode 100644 include/linux/spi/spi-dw.h
>>
>> diff --git a/drivers/spi/spi-dw.c b/drivers/spi/spi-dw.c
>> index ece5f69..61f7ed8 100644
>> --- a/drivers/spi/spi-dw.c
>> +++ b/drivers/spi/spi-dw.c
>> @@ -38,9 +38,6 @@
>>   #define QUEUE_RUNNING  0
>>   #define QUEUE_STOPPED  1
>>
>> -#define MRST_SPI_DEASSERT      0
>> -#define MRST_SPI_ASSERT                1
>> -
>>   /* Slave spi_dev related */
>>   struct chip_data {
>>         u16 cr0;
>> diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
>> index 7a5e78d..92bee30 100644
>> --- a/drivers/spi/spi-dw.h
>> +++ b/drivers/spi/spi-dw.h
>> @@ -3,6 +3,7 @@
>>
>>   #include<linux/io.h>
>>   #include<linux/scatterlist.h>
>> +#include<linux/spi/spi-dw.h>
>>
>>   /* Bit fields in CTRLR0 */
>>   #define SPI_DFS_OFFSET                 0
>> @@ -49,11 +50,6 @@
>>   /* TX RX interrupt level threshold, max can be 256 */
>>   #define SPI_INT_THRESHOLD              32
>>
>> -enum dw_ssi_type {
>> -       SSI_MOTO_SPI = 0,
>> -       SSI_TI_SSP,
>> -       SSI_NS_MICROWIRE,
>> -};
>>
>>   struct dw_spi_reg {
>>         u32     ctrl0;
>> @@ -208,18 +204,6 @@ static inline void spi_umask_intr(struct dw_spi *dws, u32 mask)
>>         dw_writel(dws, imr, new_mask);
>>   }
>>
>> -/*
>> - * Each SPI slave device to work with dw_api controller should
>> - * has such a structure claiming its working mode (PIO/DMA etc),
>> - * which can be save in the "controller_data" member of the
>> - * struct spi_device
>> - */
>> -struct dw_spi_chip {
>> -       u8 poll_mode;   /* 0 for contoller polling mode */
>> -       u8 type;        /* SPI/SSP/Micrwire */
>> -       u8 enable_dma;
>> -       void (*cs_control)(u32 command);
>> -};
>>
>>   extern int dw_spi_add_host(struct dw_spi *dws);
>>   extern void dw_spi_remove_host(struct dw_spi *dws);
>> diff --git a/include/linux/spi/spi-dw.h b/include/linux/spi/spi-dw.h
>> new file mode 100644
>> index 0000000..787b154
>> --- /dev/null
>> +++ b/include/linux/spi/spi-dw.h
>> @@ -0,0 +1,42 @@
>> +/*
>> + *
>> + * Copyright (c) 2009, Intel Corporation.
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms and conditions of the GNU General Public License,
>> + * version 2, as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope it will be useful, but WITHOUT
>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
>> + * more details.
>> + *
>> + * You should have received a copy of the GNU General Public License along with
>> + * this program; if not, write to the Free Software Foundation, Inc.,
>> + * 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
>> + */
>> +
>> +#ifndef _SPI_DW_H_
>> +#define _SPI_DW_H_
>> +enum spi_dw_ssi_type {
>> +       SSI_MOTO_SPI = 0,
>> +       SSI_TI_SSP,
>> +       SSI_NS_MICROWIRE,
>> +};
>> +
>> +#define MRST_SPI_DEASSERT      0
>> +#define MRST_SPI_ASSERT                1
>> +
>> +/*
>> + * Each SPI slave device to work with dw_api controller should
>> + * has such a structure claiming its working mode (PIO/DMA etc),
>> + * which can be save in the "controller_data" member of the
>> + * struct spi_device
>> + */
>> +struct spi_dw_chip {
>> +       u8 poll_mode;   /* 0 for contoller polling mode */
>> +       u8 type;        /* SPI/SSP/Micrwire */
>> +       u8 enable_dma;
>> +       void (*cs_control)(u32 command);
>> +};
>> +#endif
>> --
>> 1.7.3.4
>>
>>
>> ------------------------------------------------------------------------------
>> Simplify data backup and recovery for your virtual environment with vRanger.
>> Installation's a snap, and flexible recovery options mean your data is safe,
>> secure and there when you need it. Data protection magic?
>> Nope - It's vRanger. Get your free trial download today.
>> http://p.sf.net/sfu/quest-sfdev2dev
>> _______________________________________________
>> spi-devel-general mailing list
>> [e-mail]
>> https://lists.sourceforge.net/lists/listinfo/spi-devel-general
>>
>
>
>


------------------------------------------------------------------------------
Simplify data backup and recovery for your virtual environment with vRanger.
Installation's a snap, and flexible recovery options mean your data is safe,
secure and there when you need it. Data protection magic?
Nope - It's vRanger. Get your free trial download today.
http://p.sf.net/sfu/quest-sfdev2dev

Grant Likely June 23, 2011, 4:03 a.m. UTC | #3

Dirk Brandewie <[e-mail]> wrote:

>On 06/22/2011 08:47 PM, Grant Likely wrote:
>> On Wed, Jun 22, 2011 at 8:00 PM,<[e-mail]>  wrote:
>>> From: Dirk Brandewie<[e-mail]>
>>>
>>> Expose the platform data structure for use by client drivers. ATM
>>> there are not any in-tree drivers using the driver (that I can
>>> find). This patch exposes the platform data needed for client
>drivers.
>>
>> ?  Why would client drivers want to muck with this configuration?  I
>> can understand the dw_spi driver being able to have per-spi_device
>> configuration, but spi_drivers absolutely should not have visibility
>> into bus-specific details.  Am I misunderstanding something.
>>
>
>Most of these config options don't need to be client configurable IMHO
>but they 
>are being used ATM by drivers that aren't upstream and the current
>controller 
>driver uses them.  This patch is to give a smooth transition
>(bisectable) to my 
>change that reworks the core message and transfer handling code.
>
>This allows me to provide patches to the developers of the out of tree
>drivers 
>that should be coming in RSN and exposes the interface they are using
>now.

My question still stands. Are you expecting spi_driver code to manipulate this data?

g.

>
>--Dirk
>> g.
>>
>>>
>>> Signed-off-by: Dirk Brandewie<[e-mail]>
>>> ---
>>>   drivers/spi/spi-dw.c       |    3 ---
>>>   drivers/spi/spi-dw.h       |   18 +-----------------
>>>   include/linux/spi/spi-dw.h |   42
>++++++++++++++++++++++++++++++++++++++++++
>>>   3 files changed, 43 insertions(+), 20 deletions(-)
>>>   create mode 100644 include/linux/spi/spi-dw.h
>>>
>>> diff --git a/drivers/spi/spi-dw.c b/drivers/spi/spi-dw.c
>>> index ece5f69..61f7ed8 100644
>>> --- a/drivers/spi/spi-dw.c
>>> +++ b/drivers/spi/spi-dw.c
>>> @@ -38,9 +38,6 @@
>>>   #define QUEUE_RUNNING  0
>>>   #define QUEUE_STOPPED  1
>>>
>>> -#define MRST_SPI_DEASSERT      0
>>> -#define MRST_SPI_ASSERT                1
>>> -
>>>   /* Slave spi_dev related */
>>>   struct chip_data {
>>>         u16 cr0;
>>> diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
>>> index 7a5e78d..92bee30 100644
>>> --- a/drivers/spi/spi-dw.h
>>> +++ b/drivers/spi/spi-dw.h
>>> @@ -3,6 +3,7 @@
>>>
>>>   #include<linux/io.h>
>>>   #include<linux/scatterlist.h>
>>> +#include<linux/spi/spi-dw.h>
>>>
>>>   /* Bit fields in CTRLR0 */
>>>   #define SPI_DFS_OFFSET                 0
>>> @@ -49,11 +50,6 @@
>>>   /* TX RX interrupt level threshold, max can be 256 */
>>>   #define SPI_INT_THRESHOLD              32
>>>
>>> -enum dw_ssi_type {
>>> -       SSI_MOTO_SPI = 0,
>>> -       SSI_TI_SSP,
>>> -       SSI_NS_MICROWIRE,
>>> -};
>>>
>>>   struct dw_spi_reg {
>>>         u32     ctrl0;
>>> @@ -208,18 +204,6 @@ static inline void spi_umask_intr(struct dw_spi
>*dws, u32 mask)
>>>         dw_writel(dws, imr, new_mask);
>>>   }
>>>
>>> -/*
>>> - * Each SPI slave device to work with dw_api controller should
>>> - * has such a structure claiming its working mode (PIO/DMA etc),
>>> - * which can be save in the "controller_data" member of the
>>> - * struct spi_device
>>> - */
>>> -struct dw_spi_chip {
>>> -       u8 poll_mode;   /* 0 for contoller polling mode */
>>> -       u8 type;        /* SPI/SSP/Micrwire */
>>> -       u8 enable_dma;
>>> -       void (*cs_control)(u32 command);
>>> -};
>>>
>>>   extern int dw_spi_add_host(struct dw_spi *dws);
>>>   extern void dw_spi_remove_host(struct dw_spi *dws);
>>> diff --git a/include/linux/spi/spi-dw.h b/include/linux/spi/spi-dw.h
>>> new file mode 100644
>>> index 0000000..787b154
>>> --- /dev/null
>>> +++ b/include/linux/spi/spi-dw.h
>>> @@ -0,0 +1,42 @@
>>> +/*
>>> + *
>>> + * Copyright (c) 2009, Intel Corporation.
>>> + *
>>> + * This program is free software; you can redistribute it and/or
>modify it
>>> + * under the terms and conditions of the GNU General Public
>License,
>>> + * version 2, as published by the Free Software Foundation.
>>> + *
>>> + * This program is distributed in the hope it will be useful, but
>WITHOUT
>>> + * ANY WARRANTY; without even the implied warranty of
>MERCHANTABILITY or
>>> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
>License for
>>> + * more details.
>>> + *
>>> + * You should have received a copy of the GNU General Public
>License along with
>>> + * this program; if not, write to the Free Software Foundation,
>Inc.,
>>> + * 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
>>> + */
>>> +
>>> +#ifndef _SPI_DW_H_
>>> +#define _SPI_DW_H_
>>> +enum spi_dw_ssi_type {
>>> +       SSI_MOTO_SPI = 0,
>>> +       SSI_TI_SSP,
>>> +       SSI_NS_MICROWIRE,
>>> +};
>>> +
>>> +#define MRST_SPI_DEASSERT      0
>>> +#define MRST_SPI_ASSERT                1
>>> +
>>> +/*
>>> + * Each SPI slave device to work with dw_api controller should
>>> + * has such a structure claiming its working mode (PIO/DMA etc),
>>> + * which can be save in the "controller_data" member of the
>>> + * struct spi_device
>>> + */
>>> +struct spi_dw_chip {
>>> +       u8 poll_mode;   /* 0 for contoller polling mode */
>>> +       u8 type;        /* SPI/SSP/Micrwire */
>>> +       u8 enable_dma;
>>> +       void (*cs_control)(u32 command);
>>> +};
>>> +#endif
>>> --
>>> 1.7.3.4
>>>
>>>
>>>
>------------------------------------------------------------------------------
>>> Simplify data backup and recovery for your virtual environment with
>vRanger.
>>> Installation's a snap, and flexible recovery options mean your data
>is safe,
>>> secure and there when you need it. Data protection magic?
>>> Nope - It's vRanger. Get your free trial download today.
>>> http://p.sf.net/sfu/quest-sfdev2dev
>>> _______________________________________________
>>> spi-devel-general mailing list
>>> [e-mail]
>>> https://lists.sourceforge.net/lists/listinfo/spi-devel-general
>>>
>>
>>
>>

[e-mail] June 23, 2011, 4:37 a.m. UTC | #4

On 06/22/2011 09:03 PM, [e-mail] wrote:
>
>
> Dirk Brandewie<[e-mail]>  wrote:
>
>> On 06/22/2011 08:47 PM, Grant Likely wrote:
>>> On Wed, Jun 22, 2011 at 8:00 PM,<[e-mail]>   wrote:
>>>> From: Dirk Brandewie<[e-mail]>
>>>>
>>>> Expose the platform data structure for use by client drivers. ATM
>>>> there are not any in-tree drivers using the driver (that I can
>>>> find). This patch exposes the platform data needed for client
>> drivers.
>>>
>>> ?  Why would client drivers want to muck with this configuration?  I
>>> can understand the dw_spi driver being able to have per-spi_device
>>> configuration, but spi_drivers absolutely should not have visibility
>>> into bus-specific details.  Am I misunderstanding something.
>>>
>>
>> Most of these config options don't need to be client configurable IMHO
>> but they
>> are being used ATM by drivers that aren't upstream and the current
>> controller
>> driver uses them.  This patch is to give a smooth transition
>> (bisectable) to my
>> change that reworks the core message and transfer handling code.
>>
>> This allows me to provide patches to the developers of the out of tree
>> drivers
>> that should be coming in RSN and exposes the interface they are using
>> now.
>
> My question still stands. Are you expecting spi_driver code to manipulate this data?
>
>

The current drivers behaviour is driven by this data provided by the client. 
This makes the current client drivers work since some have not picked picked up 
your change moving dw_spi.h out of include/linux/spi (right answer IMHO) and 
provides the interface they are using now.

--Dirk





------------------------------------------------------------------------------
Simplify data backup and recovery for your virtual environment with vRanger.
Installation's a snap, and flexible recovery options mean your data is safe,
secure and there when you need it. Data protection magic?
Nope - It's vRanger. Get your free trial download today.
http://p.sf.net/sfu/quest-sfdev2dev

Grant Likely June 23, 2011, 5:06 a.m. UTC | #5

Dirk Brandewie <[e-mail]> wrote:

>On 06/22/2011 09:03 PM, [e-mail] wrote:
>>
>>
>> Dirk Brandewie<[e-mail]>  wrote:
>>
>>> On 06/22/2011 08:47 PM, Grant Likely wrote:
>>>> On Wed, Jun 22, 2011 at 8:00 PM,<[e-mail]>   wrote:
>>>>> From: Dirk Brandewie<[e-mail]>
>>>>>
>>>>> Expose the platform data structure for use by client drivers. ATM
>>>>> there are not any in-tree drivers using the driver (that I can
>>>>> find). This patch exposes the platform data needed for client
>>> drivers.
>>>>
>>>> ?  Why would client drivers want to muck with this configuration? 
>I
>>>> can understand the dw_spi driver being able to have per-spi_device
>>>> configuration, but spi_drivers absolutely should not have
>visibility
>>>> into bus-specific details.  Am I misunderstanding something.
>>>>
>>>
>>> Most of these config options don't need to be client configurable
>IMHO
>>> but they
>>> are being used ATM by drivers that aren't upstream and the current
>>> controller
>>> driver uses them.  This patch is to give a smooth transition
>>> (bisectable) to my
>>> change that reworks the core message and transfer handling code.
>>>
>>> This allows me to provide patches to the developers of the out of
>tree
>>> drivers
>>> that should be coming in RSN and exposes the interface they are
>using
>>> now.
>>
>> My question still stands. Are you expecting spi_driver code to
>manipulate this data?
>>
>>
>
>The current drivers behaviour is driven by this data provided by the
>client. 
>This makes the current client drivers work since some have not picked
>picked up 
>your change moving dw_spi.h out of include/linux/spi (right answer
>IMHO) and 
>provides the interface they are using now.

So the situation is that certain out-of-tree spi_drivers are reaching into internal details of a specific spi bus driver?  If so, then that is wrong and bad, and certainly will not be merged. Especially when there are no in tree users and neither does this series add any.

g.

>
>--Dirk

[e-mail] June 23, 2011, 5:16 a.m. UTC | #6

On 06/22/2011 10:06 PM, [e-mail] wrote:
>
>
> Dirk Brandewie<[e-mail]>  wrote:
>
>> On 06/22/2011 09:03 PM, [e-mail] wrote:
>>>
>>>
>>> Dirk Brandewie<[e-mail]>   wrote:
>>>
>>>> On 06/22/2011 08:47 PM, Grant Likely wrote:
>>>>> On Wed, Jun 22, 2011 at 8:00 PM,<[e-mail]>    wrote:
>>>>>> From: Dirk Brandewie<[e-mail]>
>>>>>>
>>>>>> Expose the platform data structure for use by client drivers. ATM
>>>>>> there are not any in-tree drivers using the driver (that I can
>>>>>> find). This patch exposes the platform data needed for client
>>>> drivers.
>>>>>
>>>>> ?  Why would client drivers want to muck with this configuration?
>> I
>>>>> can understand the dw_spi driver being able to have per-spi_device
>>>>> configuration, but spi_drivers absolutely should not have
>> visibility
>>>>> into bus-specific details.  Am I misunderstanding something.
>>>>>
>>>>
>>>> Most of these config options don't need to be client configurable
>> IMHO
>>>> but they
>>>> are being used ATM by drivers that aren't upstream and the current
>>>> controller
>>>> driver uses them.  This patch is to give a smooth transition
>>>> (bisectable) to my
>>>> change that reworks the core message and transfer handling code.
>>>>
>>>> This allows me to provide patches to the developers of the out of
>> tree
>>>> drivers
>>>> that should be coming in RSN and exposes the interface they are
>> using
>>>> now.
>>>
>>> My question still stands. Are you expecting spi_driver code to
>> manipulate this data?
>>>
>>>
>>
>> The current drivers behaviour is driven by this data provided by the
>> client.
>> This makes the current client drivers work since some have not picked
>> picked up
>> your change moving dw_spi.h out of include/linux/spi (right answer
>> IMHO) and
>> provides the interface they are using now.
>
> So the situation is that certain out-of-tree spi_drivers are reaching into internal details of a specific spi bus driver?
>If so, then that is wrong and bad, and certainly will not be merged. Especially when there are no in tree users and neither
>does this series add any.

OK

Since the current driver used pxa2xx_spi.c as a template I was following the 
example provided by include/linux/spi/pxa2xx_spi.h.  I have no problem dropping 
this patch until I finish the rest of the rework planned. Was trying to limit 
the amount of heartburn others on the list had with my changes.

--Dirk
>
> g.
>
>>
>> --Dirk
>


------------------------------------------------------------------------------
Simplify data backup and recovery for your virtual environment with vRanger.
Installation's a snap, and flexible recovery options mean your data is safe,
secure and there when you need it. Data protection magic?
Nope - It's vRanger. Get your free trial download today.
http://p.sf.net/sfu/quest-sfdev2dev

Patch

diff --git a/drivers/spi/spi-dw.c b/drivers/spi/spi-dw.c
index ece5f69..61f7ed8 100644
--- a/drivers/spi/spi-dw.c
+++ b/drivers/spi/spi-dw.c
@@ -38,9 +38,6 @@ 
 #define QUEUE_RUNNING	0
 #define QUEUE_STOPPED	1
 
-#define MRST_SPI_DEASSERT	0
-#define MRST_SPI_ASSERT		1
-
 /* Slave spi_dev related */
 struct chip_data {
 	u16 cr0;
diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
index 7a5e78d..92bee30 100644
--- a/drivers/spi/spi-dw.h
+++ b/drivers/spi/spi-dw.h
@@ -3,6 +3,7 @@ 
 
 #include <linux/io.h>
 #include <linux/scatterlist.h>
+#include <linux/spi/spi-dw.h>
 
 /* Bit fields in CTRLR0 */
 #define SPI_DFS_OFFSET			0
@@ -49,11 +50,6 @@ 
 /* TX RX interrupt level threshold, max can be 256 */
 #define SPI_INT_THRESHOLD		32
 
-enum dw_ssi_type {
-	SSI_MOTO_SPI = 0,
-	SSI_TI_SSP,
-	SSI_NS_MICROWIRE,
-};
 
 struct dw_spi_reg {
 	u32	ctrl0;
@@ -208,18 +204,6 @@  static inline void spi_umask_intr(struct dw_spi *dws, u32 mask)
 	dw_writel(dws, imr, new_mask);
 }
 
-/*
- * Each SPI slave device to work with dw_api controller should
- * has such a structure claiming its working mode (PIO/DMA etc),
- * which can be save in the "controller_data" member of the
- * struct spi_device
- */
-struct dw_spi_chip {
-	u8 poll_mode;	/* 0 for contoller polling mode */
-	u8 type;	/* SPI/SSP/Micrwire */
-	u8 enable_dma;
-	void (*cs_control)(u32 command);
-};
 
 extern int dw_spi_add_host(struct dw_spi *dws);
 extern void dw_spi_remove_host(struct dw_spi *dws);
diff --git a/include/linux/spi/spi-dw.h b/include/linux/spi/spi-dw.h
new file mode 100644
index 0000000..787b154
--- /dev/null
+++ b/include/linux/spi/spi-dw.h
@@ -0,0 +1,42 @@ 
+/*
+ *
+ * Copyright (c) 2009, Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#ifndef _SPI_DW_H_
+#define _SPI_DW_H_
+enum spi_dw_ssi_type {
+	SSI_MOTO_SPI = 0,
+	SSI_TI_SSP,
+	SSI_NS_MICROWIRE,
+};
+
+#define MRST_SPI_DEASSERT	0
+#define MRST_SPI_ASSERT		1
+
+/*
+ * Each SPI slave device to work with dw_api controller should
+ * has such a structure claiming its working mode (PIO/DMA etc),
+ * which can be save in the "controller_data" member of the
+ * struct spi_device
+ */
+struct spi_dw_chip {
+	u8 poll_mode;	/* 0 for contoller polling mode */
+	u8 type;	/* SPI/SSP/Micrwire */
+	u8 enable_dma;
+	void (*cs_control)(u32 command);
+};
+#endif