Re: [PATCH] SPI: SSP SPI Controller driver

2012-12-17 Thread Linus Walleij
On Tue, Dec 11, 2012 at 9:58 AM, chao bi chao...@intel.com wrote:

 1. I understand the workqueue in spi core is for driving message
 transfer, so SPI driver should not create new workqueue for this usage.
 However, the workqueue created here is not for this usage it's to call
 back to SPI protocol driver (ifx6x60.c) when DMA data transfer is
 finished, so it seems not conflict with spi core. Am I right?

So a single message can contain several transfers, and if this
is some per-transfer DMA thing, it could be valid. I need to go
in and look closer at the patch.

I've considered trying to also generalize parts of the transfer
handling but ran out of energy.

Yours,
Linus Walleij

--
LogMeIn Rescue: Anywhere, Anytime Remote support for IT. Free Trial
Remotely access PCs and mobile devices and provide instant support
Improve your efficiency, and focus on delivering more value-add services
Discover what IT Professionals Know. Rescue delivers
http://p.sf.net/sfu/logmein_12329d2d
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general


Re: [PATCH] SPI: SSP SPI Controller driver

2012-12-17 Thread Linus Walleij
On Wed, Nov 21, 2012 at 3:16 AM, chao bi chao...@intel.com wrote:

 This patch is to implement SSP SPI controller driver, which has been applied 
 and
 validated on intel Moorestown  Medfield platform. The patch are originated by
 Ken Mills ken.k.mi...@intel.com and Sylvain Centelles 
 sylvain.centel...@intel.com,
 and to be further developed by Channing chao...@intel.com and Chen Jun
 jun.d.c...@intel.com according to their integration  validation on 
 Medfield platform.

 Signed-off-by: Ken Mills ken.k.mi...@intel.com
 Signed-off-by: Sylvain Centelles sylvain.centel...@intel.com
 Signed-off-by: channing chao...@intel.com
 Signed-off-by: Chen Jun jun.d.c...@intel.com

OK...

 +#ifdef DUMP_RX

So since this #define DUMP_RX is not part of this patch and not of the
kernel at large it's basically an #if 0 and all the code within such
defines should be deleted.

But I guess you have this undocumented feature that the developer
is supposed to hack the file and insert #define DUMP_RX to use it.

Idea: if you want to keep this use the kernel verbose debug system.
In drivers/spi/Kconfig we have:

config SPI_DEBUG
boolean Debug support for SPI drivers
depends on DEBUG_KERNEL
help
  Say yes to enable debug messaging (like dev_dbg and pr_debug),
  sysfs, and debugfs support in SPI controller and protocol drivers.

So insert something like:

config SPI_VERBOSE_DEBUG
boolean Verbose debug support for SPI drivers
depends on SPI_DEBUG
   

Modify Makefile to contain:

ccflags-$(CONFIG_SPI_VERBOSE_DEBUG) := -DVERBOSE_DEBUG

Then put the above withing #ifdef CONFIG_SPI_VERBOSE_DEBUG

Then you can use the dev_vdgb() and friends from linux/device.h.

Because I think that's what it is essentially: verbose debugging.

 +static void dump_trailer(const struct device *dev, char *buf, int len, int 
 sz)
 +{
 +   int tlen1 = (len  sz ? len : sz);
 +   int tlen2 =  ((len - sz)  sz) ? sz : (len - sz);
 +   unsigned char *p;
 +   static char msg[MAX_SPI_TRANSFER_SIZE];
 +
 +   memset(msg, '\0', sizeof(msg));
 +   p = buf;
 +   while (p  buf + tlen1)
 +   sprintf(msg, %s%02x, msg, (unsigned int)*p++);
 +
 +   if (tlen2  0) {
 +   sprintf(msg, %s ., msg);
 +   p = (buf+len) - tlen2;
 +   while (p  buf + len)
 +   sprintf(msg, %s%02x, msg, (unsigned int)*p++);
 +   }
 +
 +   dev_info(dev, DUMP: %p[0:%d ... %d:%d]:%s, buf, tlen1 - 1,
 +  len-tlen2, len - 1, msg);

dev_vdbg().

(...)
 +static inline u32 is_tx_fifo_empty(struct ssp_driver_context *drv_context)

Change return type to bool if you're just returning 0 or 1.

 +{
 +   u32 sssr;
 +   sssr = read_SSSR(drv_context-ioaddr);
 +   if ((sssr  SSSR_TFL_MASK) || (sssr  SSSR_TNF) == 0)
 +   return 0;
 +   else
 +   return 1;
 +}

return false/true.

 +static inline u32 is_rx_fifo_empty(struct ssp_driver_context *drv_context)
 +{
 +   return ((read_SSSR(drv_context-ioaddr)  SSSR_RNE) == 0);
 +}

Dito. Here it is even more obvious.

(...)
 +static void flush(struct ssp_driver_context *drv_context)
 +{
 +   void *reg = drv_context-ioaddr;
 +   u32 i = 0;
 +
 +   /* If the transmit fifo is not empty, reset the interface. */
 +   if (!is_tx_fifo_empty(drv_context)) {
 +   dev_err(drv_context-pdev-dev,
 +   TX FIFO not empty. Reset of SPI IF);
 +   disable_interface(drv_context);
 +   return;
 +   }
 +
 +   dev_dbg(drv_context-pdev-dev,  SSSR=%x\r\n, read_SSSR(reg));
 +   while (!is_rx_fifo_empty(drv_context)  (i  SPI_FIFO_SIZE + 1)) {
 +   read_SSDR(reg);
 +   i++;
 +   }
 +   WARN(i  0, %d words flush occured\n, i);

WARN really? Why not dev_warn()?

 +static int null_writer(struct ssp_driver_context *drv_context)
 +static int null_reader(struct ssp_driver_context *drv_context)
 +static int u8_writer(struct ssp_driver_context *drv_context)
 +static int u8_reader(struct ssp_driver_context *drv_context)
 +static int u16_writer(struct ssp_driver_context *drv_context)
 +static int u16_reader(struct ssp_driver_context *drv_context)
 +static int u32_writer(struct ssp_driver_context *drv_context)
 +static int u32_reader(struct ssp_driver_context *drv_context)

These seem to all be designed to return 0 or 1 and should then be
bool. It seems strange actually, you would expect that such a
function returns the number of bytes or words read/written.

 +static bool chan_filter(struct dma_chan *chan, void *param)
 +static void unmap_dma_buffers(struct ssp_driver_context *drv_context)
 +static void intel_mid_ssp_spi_dma_done(void *arg)
 +static void intel_mid_ssp_spi_dma_init(struct ssp_driver_context 
 *drv_context)
 +static void intel_mid_ssp_spi_dma_exit(struct ssp_driver_context 
 *drv_context)
 +static void dma_transfer(struct ssp_driver_context 

Re: [PATCH 3/4 v2] spi: sh-msiof: Add device tree parsing to driver

2012-12-17 Thread Bastian Hecht
Hi Nobuhiro,

2012/12/17 Nobuhiro Iwamatsu iwama...@nigauri.org:
 Hi, Bastian.

 On Wed, Dec 12, 2012 at 8:54 PM, Bastian Hecht hec...@gmail.com wrote:
 From: Bastian Hecht hec...@gmail.com

 This adds the capability to retrieve setup data from the device tree
 node. The usage of platform data is still available.

 Signed-off-by: Bastian Hecht hechtb+rene...@gmail.com
 ---
 v2:
 - renamed property chip_select to num-cs
 - renamed property tx_fifo_size to renesas,tx-fifo-size
 - renamed property rx_fifo_size to renesas,rx-fifo-size

  drivers/spi/spi-sh-msiof.c |   56 
 +++-
  1 file changed, 55 insertions(+), 1 deletion(-)

 diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
 index 96358d0..8b40d08 100644
 --- a/drivers/spi/spi-sh-msiof.c
 +++ b/drivers/spi/spi-sh-msiof.c
 @@ -20,6 +20,7 @@
  #include linux/io.h
  #include linux/kernel.h
  #include linux/module.h
 +#include linux/of.h
  #include linux/platform_device.h
  #include linux/pm_runtime.h

 @@ -592,6 +593,37 @@ static u32 sh_msiof_spi_txrx_word(struct spi_device 
 *spi, unsigned nsecs,
 return 0;
  }

 +#ifdef CONFIG_OF
 +static struct sh_msiof_spi_info *sh_msiof_spi_parse_dt(struct device *dev)
 +{
 +   struct sh_msiof_spi_info *info;
 +   struct device_node *np = dev-of_node;
 +   u32 num_cs = 0;
 +
 +   info = devm_kzalloc(dev, sizeof(struct sh_msiof_spi_info), 
 GFP_KERNEL);
 +   if (!info) {
 +   dev_err(dev, failed to allocate setup data\n);
 +   return NULL;
 +   }
 +
 +   /* Parse the MSIOF properties */
 +   of_property_read_u32(np, num-cs, num_cs);
 +   of_property_read_u32(np, renesas,tx-fifo-size,
 +   info-tx_fifo_override);
 +   of_property_read_u32(np, renesas,rx-fifo-size,
 +   info-rx_fifo_override);
 +
 +   info-num_chipselect = num_cs;
 +
 +   return info;
 +}
 +#else
 +static struct sh_msiof_spi_info *sh_msiof_spi_parse_dt(struct device *dev)
 +{
 +   return NULL;
 +}
 +#endif
 +
  static int sh_msiof_spi_probe(struct platform_device *pdev)
  {
 struct resource *r;
 @@ -610,7 +642,17 @@ static int sh_msiof_spi_probe(struct platform_device 
 *pdev)
 p = spi_master_get_devdata(master);

 platform_set_drvdata(pdev, p);
 -   p-info = pdev-dev.platform_data;
 +   if (pdev-dev.of_node)
 +   p-info = sh_msiof_spi_parse_dt(pdev-dev);
 +   else
 +   p-info = pdev-dev.platform_data;
 +
 +   if (!p-info) {
 +   dev_err(pdev-dev, failed to obtain device info\n);
 +   ret = -ENXIO;
 +   goto err1;
 +   }
 +
 init_completion(p-done);

 p-clk = clk_get(pdev-dev, NULL);
 @@ -715,6 +757,17 @@ static int sh_msiof_spi_runtime_nop(struct device *dev)
 return 0;
  }

 +#ifdef CONFIG_OF
 +static const struct of_device_id sh_msiof_match[] = {
 +   { .compatible = renesas,sh-msiof, },
 +   { .compatible = renesas,sh-mobile-msiof, },
 +   {},
 +};
 +MODULE_DEVICE_TABLE(of, sh_msiof_match);
 +#else
 +#define sh_msiof_match NULL
 +#endif

 You can remove ifdef if you use of_match_ptr().

 +
  static struct dev_pm_ops sh_msiof_spi_dev_pm_ops = {
 .runtime_suspend = sh_msiof_spi_runtime_nop,
 .runtime_resume = sh_msiof_spi_runtime_nop,
 @@ -727,6 +780,7 @@ static struct platform_driver sh_msiof_spi_drv = {
 .name   = spi_sh_msiof,
 .owner  = THIS_MODULE,
 .pm = sh_msiof_spi_dev_pm_ops,
 +   .of_match_table = sh_msiof_match,

 You can use of_match_ptr(sh_msiof_match).


Ok very nice, will do so.

Thanks,

 Bastian

 },
  };
  module_platform_driver(sh_msiof_spi_drv);
 --
 1.7.9.5

 Best regards,
   Nobuhiro

 --
 Nobuhiro Iwamatsu
iwamatsu at {nigauri.org / debian.org}
GPG ID: 40AD1FA6

--
LogMeIn Rescue: Anywhere, Anytime Remote support for IT. Free Trial
Remotely access PCs and mobile devices and provide instant support
Improve your efficiency, and focus on delivering more value-add services
Discover what IT Professionals Know. Rescue delivers
http://p.sf.net/sfu/logmein_12329d2d
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general


Re: [PATCH 1/1] spi/atmel: add DT support

2012-12-17 Thread Jean-Christophe PLAGNIOL-VILLARD
On 01:03 Sat 15 Dec , Grant Likely wrote:
 On Wed, 12 Dec 2012 16:13:08 +0100, Jean-Christophe PLAGNIOL-VILLARD 
 plagn...@jcrosoft.com wrote:
  On 14:37 Fri 23 Nov , Nicolas Ferre wrote:
   On 11/23/2012 01:44 PM, Jean-Christophe PLAGNIOL-VILLARD :
the atmel_spi use only gpio for chip select

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD plagn...@jcrosoft.com
   
   Seems simple and nice:
   
   Acked-by: Nicolas Ferre nicolas.fe...@atmel.com
  grant is ok to have this for 3.8?
 
 Not sure how I missed this one. It's pretty straight forward and not
 risky, so no problem. However;
 
the atmel_spi use only gpio for chip select
 
 I know you know how to write a proper commit message. I'll need
 something better than the above before I commit it. I won't make you
 resubmit the patch, but do send me a better description.
 
ok replace with this please

spi/atmel: add DT support

Use the newly introduce cs-gpios dt support on atmel.
We do not use the hardware cs as it's wired and have buges and limitations.
As the the controller's belief that only active-low devices/systems exists.

As done on non-dt system.

Best Regards,
J.
 g.
 

--
LogMeIn Rescue: Anywhere, Anytime Remote support for IT. Free Trial
Remotely access PCs and mobile devices and provide instant support
Improve your efficiency, and focus on delivering more value-add services
Discover what IT Professionals Know. Rescue delivers
http://p.sf.net/sfu/logmein_12329d2d
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general


Re: [PATCH 1/1] spi/atmel: add DT support

2012-12-17 Thread Grant Likely
On Mon, 17 Dec 2012 11:13:51 +0100, Jean-Christophe PLAGNIOL-VILLARD 
plagn...@jcrosoft.com wrote:
 On 01:03 Sat 15 Dec , Grant Likely wrote:
  On Wed, 12 Dec 2012 16:13:08 +0100, Jean-Christophe PLAGNIOL-VILLARD 
  plagn...@jcrosoft.com wrote:
   On 14:37 Fri 23 Nov , Nicolas Ferre wrote:
On 11/23/2012 01:44 PM, Jean-Christophe PLAGNIOL-VILLARD :
 the atmel_spi use only gpio for chip select
 
 Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD 
 plagn...@jcrosoft.com

Seems simple and nice:

Acked-by: Nicolas Ferre nicolas.fe...@atmel.com
   grant is ok to have this for 3.8?
  
  Not sure how I missed this one. It's pretty straight forward and not
  risky, so no problem. However;
  
 the atmel_spi use only gpio for chip select
  
  I know you know how to write a proper commit message. I'll need
  something better than the above before I commit it. I won't make you
  resubmit the patch, but do send me a better description.
  
 ok replace with this please
 
 spi/atmel: add DT support
 
 Use the newly introduce cs-gpios dt support on atmel.
 We do not use the hardware cs as it's wired and have buges and limitations.
 As the the controller's belief that only active-low devices/systems exists.
 
 As done on non-dt system.

Done, thanks.

g.


--
LogMeIn Rescue: Anywhere, Anytime Remote support for IT. Free Trial
Remotely access PCs and mobile devices and provide instant support
Improve your efficiency, and focus on delivering more value-add services
Discover what IT Professionals Know. Rescue delivers
http://p.sf.net/sfu/logmein_12329d2d
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general


[PATCH] spi: remove check for bits_per_word on transfer

2012-12-17 Thread Laxman Dewangan
When spi client does the spi transfer and does not sets
the bits_per_word for each transfer then set it as default
of spi device in spi core before calling low level transfer.

Removing the similar code from different low level drivers as
it its duplicate checks and it is no more require.

Signed-off-by: Laxman Dewangan ldewan...@nvidia.com
---
This is the continuation of feedback got from Jonas on
change
spi: make sure all transfer has bits_per_word set
where I made the change in only tegra slink driver.
This patch remove the similar code form all drivers.

 drivers/spi/spi-altera.c|2 +-
 drivers/spi/spi-bfin-sport.c|3 +--
 drivers/spi/spi-bfin5xx.c   |3 +--
 drivers/spi/spi-bitbang.c   |6 +++---
 drivers/spi/spi-clps711x.c  |2 +-
 drivers/spi/spi-coldfire-qspi.c |3 +--
 drivers/spi/spi-ep93xx.c|2 +-
 drivers/spi/spi-s3c64xx.c   |2 +-
 drivers/spi/spi-sirf.c  |3 +--
 drivers/spi/spi-tegra20-slink.c |9 +++--
 drivers/spi/spi-txx9.c  |6 ++
 11 files changed, 16 insertions(+), 25 deletions(-)

diff --git a/drivers/spi/spi-altera.c b/drivers/spi/spi-altera.c
index 5e7314a..a537f8d 100644
--- a/drivers/spi/spi-altera.c
+++ b/drivers/spi/spi-altera.c
@@ -134,7 +134,7 @@ static int altera_spi_txrx(struct spi_device *spi, struct 
spi_transfer *t)
hw-tx = t-tx_buf;
hw-rx = t-rx_buf;
hw-count = 0;
-   hw-bytes_per_word = (t-bits_per_word ? : spi-bits_per_word) / 8;
+   hw-bytes_per_word = t-bits_per_word / 8;
hw-len = t-len / hw-bytes_per_word;
 
if (hw-irq = 0) {
diff --git a/drivers/spi/spi-bfin-sport.c b/drivers/spi/spi-bfin-sport.c
index ac7ffca..39b0d17 100644
--- a/drivers/spi/spi-bfin-sport.c
+++ b/drivers/spi/spi-bfin-sport.c
@@ -416,8 +416,7 @@ bfin_sport_spi_pump_transfers(unsigned long data)
drv_data-cs_change = transfer-cs_change;
 
/* Bits per word setup */
-   bits_per_word = transfer-bits_per_word ? :
-   message-spi-bits_per_word ? : 8;
+   bits_per_word = transfer-bits_per_word;
if (bits_per_word % 16 == 0)
drv_data-ops = bfin_sport_transfer_ops_u16;
else
diff --git a/drivers/spi/spi-bfin5xx.c b/drivers/spi/spi-bfin5xx.c
index 0429d83..7d7c991 100644
--- a/drivers/spi/spi-bfin5xx.c
+++ b/drivers/spi/spi-bfin5xx.c
@@ -642,8 +642,7 @@ static void bfin_spi_pump_transfers(unsigned long data)
drv_data-cs_change = transfer-cs_change;
 
/* Bits per word setup */
-   bits_per_word = transfer-bits_per_word ? :
-   message-spi-bits_per_word ? : 8;
+   bits_per_word = transfer-bits_per_word;
if (bits_per_word % 16 == 0) {
drv_data-n_bytes = bits_per_word/8;
drv_data-len = (transfer-len)  1;
diff --git a/drivers/spi/spi-bitbang.c b/drivers/spi/spi-bitbang.c
index 8b3d8ef..61beaec 100644
--- a/drivers/spi/spi-bitbang.c
+++ b/drivers/spi/spi-bitbang.c
@@ -69,7 +69,7 @@ static unsigned bitbang_txrx_8(
unsignedns,
struct spi_transfer *t
 ) {
-   unsignedbits = t-bits_per_word ? : spi-bits_per_word;
+   unsignedbits = t-bits_per_word;
unsignedcount = t-len;
const u8*tx = t-tx_buf;
u8  *rx = t-rx_buf;
@@ -95,7 +95,7 @@ static unsigned bitbang_txrx_16(
unsignedns,
struct spi_transfer *t
 ) {
-   unsignedbits = t-bits_per_word ? : spi-bits_per_word;
+   unsignedbits = t-bits_per_word;
unsignedcount = t-len;
const u16   *tx = t-tx_buf;
u16 *rx = t-rx_buf;
@@ -121,7 +121,7 @@ static unsigned bitbang_txrx_32(
unsignedns,
struct spi_transfer *t
 ) {
-   unsignedbits = t-bits_per_word ? : spi-bits_per_word;
+   unsignedbits = t-bits_per_word;
unsignedcount = t-len;
const u32   *tx = t-tx_buf;
u32 *rx = t-rx_buf;
diff --git a/drivers/spi/spi-clps711x.c b/drivers/spi/spi-clps711x.c
index 1366c46..a11cbf0 100644
--- a/drivers/spi/spi-clps711x.c
+++ b/drivers/spi/spi-clps711x.c
@@ -68,7 +68,7 @@ static int spi_clps711x_setup_xfer(struct spi_device *spi,
   struct spi_transfer *xfer)
 {
u32 speed = xfer-speed_hz ? : spi-max_speed_hz;
-   u8 bpw = xfer-bits_per_word ? : spi-bits_per_word;
+   u8 bpw = xfer-bits_per_word;
struct spi_clps711x_data *hw = spi_master_get_devdata(spi-master);
 
if (bpw != 8) {
diff --git a/drivers/spi/spi-coldfire-qspi.c b/drivers/spi/spi-coldfire-qspi.c
index 58466b8..7b5cc9e 100644
--- a/drivers/spi/spi-coldfire-qspi.c
+++ b/drivers/spi/spi-coldfire-qspi.c
@@ -329,8 +329,7 @@ static int 

[PATCH] spi: remove check for bits_per_word on transfer

2012-12-17 Thread Laxman Dewangan
When spi client does the spi transfer and does not sets
the bits_per_word for each transfer then set it as default
of spi device in spi core before calling low level transfer.

Removing the similar code from different low level drivers as
it its duplicate checks and it is no more require.

Signed-off-by: Laxman Dewangan ldewan...@nvidia.com
---
This is the continuation of feedback got from Jonas on
change
spi: make sure all transfer has bits_per_word set
where I made the change in only tegra slink driver.
This patch remove the similar code form all drivers.

 drivers/spi/spi-altera.c|2 +-
 drivers/spi/spi-bfin-sport.c|3 +--
 drivers/spi/spi-bfin5xx.c   |3 +--
 drivers/spi/spi-bitbang.c   |6 +++---
 drivers/spi/spi-clps711x.c  |2 +-
 drivers/spi/spi-coldfire-qspi.c |3 +--
 drivers/spi/spi-ep93xx.c|2 +-
 drivers/spi/spi-s3c64xx.c   |2 +-
 drivers/spi/spi-sirf.c  |3 +--
 drivers/spi/spi-tegra20-slink.c |9 +++--
 drivers/spi/spi-txx9.c  |6 ++
 11 files changed, 16 insertions(+), 25 deletions(-)

diff --git a/drivers/spi/spi-altera.c b/drivers/spi/spi-altera.c
index 5e7314a..a537f8d 100644
--- a/drivers/spi/spi-altera.c
+++ b/drivers/spi/spi-altera.c
@@ -134,7 +134,7 @@ static int altera_spi_txrx(struct spi_device *spi, struct 
spi_transfer *t)
hw-tx = t-tx_buf;
hw-rx = t-rx_buf;
hw-count = 0;
-   hw-bytes_per_word = (t-bits_per_word ? : spi-bits_per_word) / 8;
+   hw-bytes_per_word = t-bits_per_word / 8;
hw-len = t-len / hw-bytes_per_word;
 
if (hw-irq = 0) {
diff --git a/drivers/spi/spi-bfin-sport.c b/drivers/spi/spi-bfin-sport.c
index ac7ffca..39b0d17 100644
--- a/drivers/spi/spi-bfin-sport.c
+++ b/drivers/spi/spi-bfin-sport.c
@@ -416,8 +416,7 @@ bfin_sport_spi_pump_transfers(unsigned long data)
drv_data-cs_change = transfer-cs_change;
 
/* Bits per word setup */
-   bits_per_word = transfer-bits_per_word ? :
-   message-spi-bits_per_word ? : 8;
+   bits_per_word = transfer-bits_per_word;
if (bits_per_word % 16 == 0)
drv_data-ops = bfin_sport_transfer_ops_u16;
else
diff --git a/drivers/spi/spi-bfin5xx.c b/drivers/spi/spi-bfin5xx.c
index 0429d83..7d7c991 100644
--- a/drivers/spi/spi-bfin5xx.c
+++ b/drivers/spi/spi-bfin5xx.c
@@ -642,8 +642,7 @@ static void bfin_spi_pump_transfers(unsigned long data)
drv_data-cs_change = transfer-cs_change;
 
/* Bits per word setup */
-   bits_per_word = transfer-bits_per_word ? :
-   message-spi-bits_per_word ? : 8;
+   bits_per_word = transfer-bits_per_word;
if (bits_per_word % 16 == 0) {
drv_data-n_bytes = bits_per_word/8;
drv_data-len = (transfer-len)  1;
diff --git a/drivers/spi/spi-bitbang.c b/drivers/spi/spi-bitbang.c
index 8b3d8ef..61beaec 100644
--- a/drivers/spi/spi-bitbang.c
+++ b/drivers/spi/spi-bitbang.c
@@ -69,7 +69,7 @@ static unsigned bitbang_txrx_8(
unsignedns,
struct spi_transfer *t
 ) {
-   unsignedbits = t-bits_per_word ? : spi-bits_per_word;
+   unsignedbits = t-bits_per_word;
unsignedcount = t-len;
const u8*tx = t-tx_buf;
u8  *rx = t-rx_buf;
@@ -95,7 +95,7 @@ static unsigned bitbang_txrx_16(
unsignedns,
struct spi_transfer *t
 ) {
-   unsignedbits = t-bits_per_word ? : spi-bits_per_word;
+   unsignedbits = t-bits_per_word;
unsignedcount = t-len;
const u16   *tx = t-tx_buf;
u16 *rx = t-rx_buf;
@@ -121,7 +121,7 @@ static unsigned bitbang_txrx_32(
unsignedns,
struct spi_transfer *t
 ) {
-   unsignedbits = t-bits_per_word ? : spi-bits_per_word;
+   unsignedbits = t-bits_per_word;
unsignedcount = t-len;
const u32   *tx = t-tx_buf;
u32 *rx = t-rx_buf;
diff --git a/drivers/spi/spi-clps711x.c b/drivers/spi/spi-clps711x.c
index 1366c46..a11cbf0 100644
--- a/drivers/spi/spi-clps711x.c
+++ b/drivers/spi/spi-clps711x.c
@@ -68,7 +68,7 @@ static int spi_clps711x_setup_xfer(struct spi_device *spi,
   struct spi_transfer *xfer)
 {
u32 speed = xfer-speed_hz ? : spi-max_speed_hz;
-   u8 bpw = xfer-bits_per_word ? : spi-bits_per_word;
+   u8 bpw = xfer-bits_per_word;
struct spi_clps711x_data *hw = spi_master_get_devdata(spi-master);
 
if (bpw != 8) {
diff --git a/drivers/spi/spi-coldfire-qspi.c b/drivers/spi/spi-coldfire-qspi.c
index 58466b8..7b5cc9e 100644
--- a/drivers/spi/spi-coldfire-qspi.c
+++ b/drivers/spi/spi-coldfire-qspi.c
@@ -329,8 +329,7 @@ static int 

Re: [PATCH] spi: make sure all transfer has bits_per_word set

2012-12-17 Thread Laxman Dewangan
On Thursday 06 December 2012 07:45 PM, Grant Likely wrote:
 On Mon, 12 Nov 2012 11:03:56 +0100, Jonas Gorskijonas.gor...@gmail.com  
 wrote:
 Hi,

 On 9 November 2012 10:06, Laxman Dewanganldewan...@nvidia.com  wrote:
 When spi client does the spi transfer and does not sets
 the bits_per_word for each transfer then set it as default
 of spi device in spi core before calling low level transfer.
 I like that (not that it counts ... )!

 Removing the similar code from spi-tegra20-slink driver as
 it is not required.
 Not sure if this should be part of *this* patch.
 Also spi-tegra20-slink isn't the only one fixing up the bits_per_word
 for transfers, so it would be nice if you could remove it from the
 other drivers, too.


Just sent the patch to remove the similar code from most of driver.
Tested only tegra driver.


 In a future patch, maybe even do the same with speed_hz?
 That sounds reasonable. Could you craft one up and see how it looks?




Ok, I will prepare change and send the patch.

Thanks,
Laxman

--
LogMeIn Rescue: Anywhere, Anytime Remote support for IT. Free Trial
Remotely access PCs and mobile devices and provide instant support
Improve your efficiency, and focus on delivering more value-add services
Discover what IT Professionals Know. Rescue delivers
http://p.sf.net/sfu/logmein_12329d2d
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general


Re: [PATCH] spi: remove check for bits_per_word on transfer

2012-12-17 Thread Russell King - ARM Linux
On Mon, Dec 17, 2012 at 11:21:57PM +0530, Laxman Dewangan wrote:
 When spi client does the spi transfer and does not sets
 the bits_per_word for each transfer then set it as default
 of spi device in spi core before calling low level transfer.

Err, sorry?  To me, the above doesn't make too much sense when considering
what's in the patch.  Please can you improve the description?  Thanks.

 
 Removing the similar code from different low level drivers as
 it its duplicate checks and it is no more require.
 
 Signed-off-by: Laxman Dewangan ldewan...@nvidia.com
 ---
 This is the continuation of feedback got from Jonas on
 change
 spi: make sure all transfer has bits_per_word set
 where I made the change in only tegra slink driver.
 This patch remove the similar code form all drivers.
 
  drivers/spi/spi-altera.c|2 +-
  drivers/spi/spi-bfin-sport.c|3 +--
  drivers/spi/spi-bfin5xx.c   |3 +--
  drivers/spi/spi-bitbang.c   |6 +++---
  drivers/spi/spi-clps711x.c  |2 +-
  drivers/spi/spi-coldfire-qspi.c |3 +--
  drivers/spi/spi-ep93xx.c|2 +-
  drivers/spi/spi-s3c64xx.c   |2 +-
  drivers/spi/spi-sirf.c  |3 +--
  drivers/spi/spi-tegra20-slink.c |9 +++--
  drivers/spi/spi-txx9.c  |6 ++
  11 files changed, 16 insertions(+), 25 deletions(-)
 
 diff --git a/drivers/spi/spi-altera.c b/drivers/spi/spi-altera.c
 index 5e7314a..a537f8d 100644
 --- a/drivers/spi/spi-altera.c
 +++ b/drivers/spi/spi-altera.c
 @@ -134,7 +134,7 @@ static int altera_spi_txrx(struct spi_device *spi, struct 
 spi_transfer *t)
   hw-tx = t-tx_buf;
   hw-rx = t-rx_buf;
   hw-count = 0;
 - hw-bytes_per_word = (t-bits_per_word ? : spi-bits_per_word) / 8;
 + hw-bytes_per_word = t-bits_per_word / 8;
   hw-len = t-len / hw-bytes_per_word;
  
   if (hw-irq = 0) {
 diff --git a/drivers/spi/spi-bfin-sport.c b/drivers/spi/spi-bfin-sport.c
 index ac7ffca..39b0d17 100644
 --- a/drivers/spi/spi-bfin-sport.c
 +++ b/drivers/spi/spi-bfin-sport.c
 @@ -416,8 +416,7 @@ bfin_sport_spi_pump_transfers(unsigned long data)
   drv_data-cs_change = transfer-cs_change;
  
   /* Bits per word setup */
 - bits_per_word = transfer-bits_per_word ? :
 - message-spi-bits_per_word ? : 8;
 + bits_per_word = transfer-bits_per_word;
   if (bits_per_word % 16 == 0)
   drv_data-ops = bfin_sport_transfer_ops_u16;
   else
 diff --git a/drivers/spi/spi-bfin5xx.c b/drivers/spi/spi-bfin5xx.c
 index 0429d83..7d7c991 100644
 --- a/drivers/spi/spi-bfin5xx.c
 +++ b/drivers/spi/spi-bfin5xx.c
 @@ -642,8 +642,7 @@ static void bfin_spi_pump_transfers(unsigned long data)
   drv_data-cs_change = transfer-cs_change;
  
   /* Bits per word setup */
 - bits_per_word = transfer-bits_per_word ? :
 - message-spi-bits_per_word ? : 8;
 + bits_per_word = transfer-bits_per_word;
   if (bits_per_word % 16 == 0) {
   drv_data-n_bytes = bits_per_word/8;
   drv_data-len = (transfer-len)  1;
 diff --git a/drivers/spi/spi-bitbang.c b/drivers/spi/spi-bitbang.c
 index 8b3d8ef..61beaec 100644
 --- a/drivers/spi/spi-bitbang.c
 +++ b/drivers/spi/spi-bitbang.c
 @@ -69,7 +69,7 @@ static unsigned bitbang_txrx_8(
   unsignedns,
   struct spi_transfer *t
  ) {
 - unsignedbits = t-bits_per_word ? : spi-bits_per_word;
 + unsignedbits = t-bits_per_word;
   unsignedcount = t-len;
   const u8*tx = t-tx_buf;
   u8  *rx = t-rx_buf;
 @@ -95,7 +95,7 @@ static unsigned bitbang_txrx_16(
   unsignedns,
   struct spi_transfer *t
  ) {
 - unsignedbits = t-bits_per_word ? : spi-bits_per_word;
 + unsignedbits = t-bits_per_word;
   unsignedcount = t-len;
   const u16   *tx = t-tx_buf;
   u16 *rx = t-rx_buf;
 @@ -121,7 +121,7 @@ static unsigned bitbang_txrx_32(
   unsignedns,
   struct spi_transfer *t
  ) {
 - unsignedbits = t-bits_per_word ? : spi-bits_per_word;
 + unsignedbits = t-bits_per_word;
   unsignedcount = t-len;
   const u32   *tx = t-tx_buf;
   u32 *rx = t-rx_buf;
 diff --git a/drivers/spi/spi-clps711x.c b/drivers/spi/spi-clps711x.c
 index 1366c46..a11cbf0 100644
 --- a/drivers/spi/spi-clps711x.c
 +++ b/drivers/spi/spi-clps711x.c
 @@ -68,7 +68,7 @@ static int spi_clps711x_setup_xfer(struct spi_device *spi,
  struct spi_transfer *xfer)
  {
   u32 speed = xfer-speed_hz ? : spi-max_speed_hz;
 - u8 bpw = xfer-bits_per_word ? : spi-bits_per_word;
 + u8 bpw = xfer-bits_per_word;
   struct spi_clps711x_data *hw = spi_master_get_devdata(spi-master);
  
   if (bpw != 8) {
 diff 

[SPAM] Óculos de sol Tree Hill

2012-12-17 Thread Tree Hill
Seu cliente de e-mail não pode ler este e-mail.
Para visualizá-lo on-line, por favor, clique aqui:
http://www.vixdroid.com/display.php?M=3705336C=0c825fa573c599c48e0ef209c58e96dcS=1179L=79N=472


Para parar de receber nossos
Emails:http://www.vixdroid.com/unsubscribe.php?M=3705336C=0c825fa573c599c48e0ef209c58e96dcL=79N=1179
--
LogMeIn Rescue: Anywhere, Anytime Remote support for IT. Free Trial
Remotely access PCs and mobile devices and provide instant support
Improve your efficiency, and focus on delivering more value-add services
Discover what IT Professionals Know. Rescue delivers
http://p.sf.net/sfu/logmein_12329d2d
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general


[SPAM] 你好:税/票――代/开

2012-12-17 Thread 13544100624
您好:
本公司优惠代理全国各地大城市发票;

贵公司如有需要,欢迎您的来电与我联系: 
 负责人:张先生(0)13544100624

--
LogMeIn Rescue: Anywhere, Anytime Remote support for IT. Free Trial
Remotely access PCs and mobile devices and provide instant support
Improve your efficiency, and focus on delivering more value-add services
Discover what IT Professionals Know. Rescue delivers
http://p.sf.net/sfu/logmein_12329d2d
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general


^……――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――####################・・・・・・・・・・・・・・・・・・・・・・・・・・・・・・・・・・・・

2012-12-17 Thread farm228694
 


 

--
LogMeIn Rescue: Anywhere, Anytime Remote support for IT. Free Trial
Remotely access PCs and mobile devices and provide instant support
Improve your efficiency, and focus on delivering more value-add services
Discover what IT Professionals Know. Rescue delivers
http://p.sf.net/sfu/logmein_12329d2d
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general


Re: [PATCH] SPI: SSP SPI Controller driver

2012-12-17 Thread chao bi
Dear Linus,
Thanks for your kind comments. Seems you were viewing the 1st version, I've
submitted 2nd version and to deliver the 3rd version soon, will include you
for review.

Please see my comments inline below.

On Mon, 2012-12-17 at 12:23 +0100, Linus Walleij wrote:
 On Wed, Nov 21, 2012 at 3:16 AM, chao bi chao...@intel.com wrote:
 
  +#ifdef DUMP_RX
 
 So since this #define DUMP_RX is not part of this patch and not of the
 kernel at large it's basically an #if 0 and all the code within such
 defines should be deleted.
 
 But I guess you have this undocumented feature that the developer
 is supposed to hack the file and insert #define DUMP_RX to use it.

yes, so I'm to deleted it in this patch, and the DUMP feature is to
submitted as another patch, which is referred to your method.

  +static inline u32 is_tx_fifo_empty(struct ssp_driver_context *drv_context)
 
 Change return type to bool if you're just returning 0 or 1.
 
  +{
  +   u32 sssr;
  +   sssr = read_SSSR(drv_context-ioaddr);
  +   if ((sssr  SSSR_TFL_MASK) || (sssr  SSSR_TNF) == 0)
  +   return 0;
  +   else
  +   return 1;
  +}
 
 return false/true.
 
  +static inline u32 is_rx_fifo_empty(struct ssp_driver_context *drv_context)
  +{
  +   return ((read_SSSR(drv_context-ioaddr)  SSSR_RNE) == 0);
  +}
 
 Dito. Here it is even more obvious.
 

It's already done in 2nd version.

  +static void flush(struct ssp_driver_context *drv_context)
  +{
  +   void *reg = drv_context-ioaddr;
  +   u32 i = 0;
  +
  +   /* If the transmit fifo is not empty, reset the interface. */
  +   if (!is_tx_fifo_empty(drv_context)) {
  +   dev_err(drv_context-pdev-dev,
  +   TX FIFO not empty. Reset of SPI IF);
  +   disable_interface(drv_context);
  +   return;
  +   }
  +
  +   dev_dbg(drv_context-pdev-dev,  SSSR=%x\r\n, read_SSSR(reg));
  +   while (!is_rx_fifo_empty(drv_context)  (i  SPI_FIFO_SIZE + 1)) {
  +   read_SSDR(reg);
  +   i++;
  +   }
  +   WARN(i  0, %d words flush occured\n, i);
 
 WARN really? Why not dev_warn()?

It's suppose that SPI FIFO is empty after each transfer, so call flush()
before each time of transfer, if any remain data in SPI FIFO,
It shows some kinds of Error must be happened in last transfer and
deserves a WARN() to record it.

  +static int null_writer(struct ssp_driver_context *drv_context)
  +static int null_reader(struct ssp_driver_context *drv_context)
  +static int u8_writer(struct ssp_driver_context *drv_context)
  +static int u8_reader(struct ssp_driver_context *drv_context)
  +static int u16_writer(struct ssp_driver_context *drv_context)
  +static int u16_reader(struct ssp_driver_context *drv_context)
  +static int u32_writer(struct ssp_driver_context *drv_context)
  +static int u32_reader(struct ssp_driver_context *drv_context)
 
 These seem to all be designed to return 0 or 1 and should then be
 bool. It seems strange actually, you would expect that such a
 function returns the number of bytes or words read/written.
 

these functions are only used by PIO transfer, its name shows how many
bytes to be written/read, and to return whether write/read is success.
If returns bytes of read/written seems make no sense for PIO transfer,
so I think return type of boot is enough, what do you think?

  +/**
  + * sram_to_ddr_cpy() - Copy data from Langwell SDRAM to DDR
  + * @drv_context:   Pointer to the private driver context
  + */
  +static void sram_to_ddr_cpy(struct ssp_driver_context *drv_context)
  +{
  +   u32 length = drv_context-len;
  +
  +   if ((drv_context-quirks  QUIRKS_DMA_USE_NO_TRAIL)
  +(drv_context-len  drv_context-rx_fifo_threshold *
  +   drv_context-n_bytes))
  +   length = TRUNCATE(drv_context-len,
  +   drv_context-rx_fifo_threshold * 
  drv_context-n_bytes);
 
 TRUNCATE is a too generic name but I'll leave that comment for
 the header file where it's defined.
 
In version 2, most contents have been moved to c file to avoid name
conflicts.

 It looks very strange.
 
 Isn't this simply an arithmetic soup construction to say:
 
 length = drv_context-len / (drv_context-rx_fifo_threshold *
 drv_context-n_bytes);

I think TRUNCATE() is different:
#define TRUNCATE(x, a) ((x)  ~((a)-1))

  +static void int_transfer_complete(struct ssp_driver_context *drv_context)
  +{
  +   void *reg = drv_context-ioaddr;
  +   struct spi_message *msg;
  +   struct device *dev = drv_context-pdev-dev;
  +
  +   if (unlikely(drv_context-quirks  QUIRKS_USE_PM_QOS))
  +   pm_qos_update_request(drv_context-pm_qos_req,
  +   PM_QOS_DEFAULT_VALUE);
 
 It's weird that using PM QoS is treated as an unlikely oddity.
 Should it not be the other way around?
 
  +
  +   if (unlikely(drv_context-quirks  QUIRKS_SRAM_ADDITIONAL_CPY))
  +