Re: [PATCH] nvmem: core: fix nvmem_cell_write inline function

2019-09-09 Thread Nandor Han

On 9/9/19 1:18 PM, Sebastian Reichel wrote:

Hi,

On Mon, Sep 09, 2019 at 12:26:06PM +0300, Nandor Han wrote:

On 9/8/19 3:10 PM, Sebastian Reichel wrote:

From: Sebastian Reichel 

nvmem_cell_write's buf argument uses different types based on
the configuration of CONFIG_NVMEM. The function prototype for
enabled NVMEM uses 'void *' type, but the static dummy function
for disabled NVMEM uses 'const char *' instead. Fix the different
behaviour by always expecting a 'void *' typed buf argument.

Fixes: 7a78a7f7695b ("power: reset: nvmem-reboot-mode: use NVMEM as reboot mode 
write interface")
Reported-by: kbuild test robot 
Cc: Han Nandor 
Cc: Srinivas Kandagatla 
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Sebastian Reichel 
---
   include/linux/nvmem-consumer.h | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h
index 8f8be5b00060..5c17cb733224 100644
--- a/include/linux/nvmem-consumer.h
+++ b/include/linux/nvmem-consumer.h
@@ -118,7 +118,7 @@ static inline void *nvmem_cell_read(struct nvmem_cell 
*cell, size_t *len)
   }
   static inline int nvmem_cell_write(struct nvmem_cell *cell,
-   const char *buf, size_t len)
+  void *buf, size_t len)


nitpick: alignment issue?


This actually fixes the alignment issue as a side-effect.
I guess I should have mentioned it in the changelog.


Review-By: Han Nandor 


I suppose you meant to write "Reviewed-by" instead of inventing your
own tag?



Yes :)

Nandor




Re: [PATCH] nvmem: core: fix nvmem_cell_write inline function

2019-09-09 Thread Nandor Han

On 9/8/19 3:10 PM, Sebastian Reichel wrote:

From: Sebastian Reichel 

nvmem_cell_write's buf argument uses different types based on
the configuration of CONFIG_NVMEM. The function prototype for
enabled NVMEM uses 'void *' type, but the static dummy function
for disabled NVMEM uses 'const char *' instead. Fix the different
behaviour by always expecting a 'void *' typed buf argument.

Fixes: 7a78a7f7695b ("power: reset: nvmem-reboot-mode: use NVMEM as reboot mode 
write interface")
Reported-by: kbuild test robot 
Cc: Han Nandor 
Cc: Srinivas Kandagatla 
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Sebastian Reichel 
---
  include/linux/nvmem-consumer.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h
index 8f8be5b00060..5c17cb733224 100644
--- a/include/linux/nvmem-consumer.h
+++ b/include/linux/nvmem-consumer.h
@@ -118,7 +118,7 @@ static inline void *nvmem_cell_read(struct nvmem_cell 
*cell, size_t *len)
  }
  
  static inline int nvmem_cell_write(struct nvmem_cell *cell,

-   const char *buf, size_t len)
+  void *buf, size_t len)


nitpick: alignment issue?

Review-By: Han Nandor 

Nandor


Re: [PATCH] power: reset: make reboot-mode user selectable

2019-09-04 Thread Nandor Han

On 9/3/19 3:04 AM, Sebastian Reichel wrote:

Hi,

On Mon, Sep 02, 2019 at 11:16:27PM +0200, Arnd Bergmann wrote:

On Mon, Sep 2, 2019 at 10:39 PM Sebastian Reichel  wrote:

This patch does not look good to me. Better patch would be to
allow compiling CONFIG_REBOOT_MODE without CONFIG_OF. Obviously
the configuration would not be useful for anything except compile
testing, but that is also true for this patch.


Ok, I'd suggest we leave it with the bugfix you already applied then.
[caa2b55784, power: reset: nvmem-reboot-mode: add CONFIG_OF dependency]


That's also fine with me.

-- Sebastian



Sounds good to me.

--- Nandor


Re: [v5] rtc: pcf85363/pcf85263: fix error that failed to run hwclock -w

2019-08-29 Thread Nandor Han

On 8/29/19 5:14 AM, Biwen Li wrote:

Issue:
 - # hwclock -w
   hwclock: RTC_SET_TIME: Invalid argument

Why:
 - Relative commit: 8b9f9d4dc511309918c4f6793bae7387c0c638af, this patch
   will always check for unwritable registers, it will compare reg
   with max_register in regmap_writeable.

 - The pcf85363/pcf85263 has the capability of address wrapping
   which means if you access an address outside the allowed range
   (0x00-0x2f) hardware actually wraps the access to a lower address.
   The rtc-pcf85363 driver will use this feature to configure the time
   and execute 2 actions in the same i2c write operation (stopping the
   clock and configure the time). However the driver has also
   configured the `regmap maxregister` protection mechanism that will
   block accessing addresses outside valid range (0x00-0x2f).

How:
 - Split of writing regs to two parts, first part writes control
   registers about stop_enable and resets, second part writes
   RTC time and date registers.

Signed-off-by: Biwen Li 
---
Change in v5:
- drop robust explanation



LGTM +1

Nandor



Re: [v4] rtc: pcf85363/pcf85263: fix error that failed to run hwclock -w

2019-08-27 Thread Nandor Han

On 8/27/19 7:37 AM, Biwen Li wrote:

  - In drivers/rtc/rtc-pcf85363.c, CTRL_STOP_EN is 0x2e, but DT_100THS
   is 0, max_regiter is 0x2f, then reg will be equal to 0x30,
   '0x30 < 0x2f' is false,so regmap_writeable will return false.

 - The pcf85363/pcf85263 has the capability of address wrapping
   which means if you access a continuous address range across a
   certain boundary(max_register of struct regmap_config) the
   hardware actually wraps the access to a lower address. But the
   address violation check of regmap rejects such access.


nitpick: This 2 paragraphs could be combined to clear up the issue:

`
The pcf85363/pcf85263 has the capability of address wrapping
which means if you access an address outside the allowed range 
(0x00-0x2f) the hardware actually wraps the access to a lower address. 
The rtc-pf85363 driver will use this feature to configure the time and 
execute 2 actions in the same i2c write operation (stopping the clock 
and configure the time). However the driver has also configured the 
`regmap maxregister` protection mechanism that will block accessing 
addresses outside valid range (0x00-0x2f).

`

nitpick: I would also use separate buffers for this actions. Up to you :)

Otherwise LGTM +1

Nandor


Re: [EXT] Re: [v2] rtc: pcf85363/pcf85263: fix error that failed to run hwclock -w

2019-08-26 Thread Nandor Han

On 8/26/19 7:29 AM, Biwen Li wrote:


On 8/16/19 10:40 PM, Li Yang wrote:

On Fri, Aug 16, 2019 at 11:30 AM Alexandre Belloni
 wrote:


On 16/08/2019 10:50:49-0500, Li Yang wrote:

On Fri, Aug 16, 2019 at 3:05 AM Alexandre Belloni
 wrote:


On 16/08/2019 10:46:36+0800, Biwen Li wrote:

Issue:
  - # hwclock -w
hwclock: RTC_SET_TIME: Invalid argument

Why:
  - Relative patch:

https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.org
%2Flkml%2F2019%2F4%2F3%2F55data=02%7C01%7Cbiwen.li%40nxp.
com%7Cff8cebc3f1034ae3fa9608d725ff9e5e%7C686ea1d3bc2b4c6fa92cd99
c5c301635%7C0%7C0%7C637019652111923736sdata=spY6e22YOkOF
3%2BF7crSM0M6xPmOhgULDqMZLQw%2BAmdI%3Dreserved=0 , this
patch

will always check for unwritable registers, it will compare reg
with max_register in regmap_writeable.

  - In drivers/rtc/rtc-pcf85363.c, CTRL_STOP_EN is 0x2e, but

DT_100THS

is 0, max_regiter is 0x2f, then reg will be equal to 0x30,
'0x30 < 0x2f' is false,so regmap_writeable will return false.

  - Root cause: the buf[] was written to a wrong place in the file
drivers/rtc/rtc-pcf85363.c



This is not true, the RTC wraps the register accesses properly and
this


This performance hack probably deserve some explanation in the code
comment.  :)


is probably something that should be handled by regmap_writable.


The address wrapping is specific to this RTC chip.  Is it also
commonly used by other I2C devices?  I'm not sure if regmap_writable
should handle the wrapping case if it is too special.



Most of the i2c RTCs do address wrapping which is sometimes the only
way to properly set the time.


Adding Mark and Nandor to the loop.

Regards,
Leo



Hi,
`regmap` provides couple of ways to validate the registers:
max_register, callback function and write table. All of these are optional, so 
it
gives you the freedom to customize it as needed.

In this situation probably you could:
1. Avoid using the wrapping feature of pcf85363 (you can just provide
separate calls for stop, reset and time confguration). In this way the
`max_register` validation method will work fine.

Yes, I use this way. Path as follows:
Stop and reset - > set time > stop



Some of the concerns regarding this method was that it might not be 
precise enough. That because you need 2 I2C operations (one for stop and 
one for time configuration). Not sure about your case if this is a 
problem or not.



2. Replace `max_register` method validation with `callback function`
validation method, were you could make your own validation.

It is not work, show the code in as follows:

bool regmap_writeable(struct regmap *map, unsigned int reg)
{
 if (map->max_register && reg > map->max_register)
 return false;
Callback function (writeable_reg) will not be called.
 if (map->writeable_reg)
 return map->writeable_reg(map->dev, reg);


Hi Li,
   If you *replace* the `max_register` method with `callback function` 
it should work. The code above will use every method *if provided*. In 
other words if `map->max_register` is 0 will go to the next step and 
check `map->writeable_reg`. Right?




Regards,
Nandor


Re: [v2] rtc: pcf85363/pcf85263: fix error that failed to run hwclock -w

2019-08-21 Thread Nandor Han

On 8/16/19 10:40 PM, Li Yang wrote:

On Fri, Aug 16, 2019 at 11:30 AM Alexandre Belloni
 wrote:


On 16/08/2019 10:50:49-0500, Li Yang wrote:

On Fri, Aug 16, 2019 at 3:05 AM Alexandre Belloni
 wrote:


On 16/08/2019 10:46:36+0800, Biwen Li wrote:

Issue:
 - # hwclock -w
   hwclock: RTC_SET_TIME: Invalid argument

Why:
 - Relative patch: https://lkml.org/lkml/2019/4/3/55 , this patch
   will always check for unwritable registers, it will compare reg
   with max_register in regmap_writeable.

 - In drivers/rtc/rtc-pcf85363.c, CTRL_STOP_EN is 0x2e, but DT_100THS
   is 0, max_regiter is 0x2f, then reg will be equal to 0x30,
   '0x30 < 0x2f' is false,so regmap_writeable will return false.

 - Root cause: the buf[] was written to a wrong place in the file
   drivers/rtc/rtc-pcf85363.c



This is not true, the RTC wraps the register accesses properly and this


This performance hack probably deserve some explanation in the code comment.  :)


is probably something that should be handled by regmap_writable.


The address wrapping is specific to this RTC chip.  Is it also
commonly used by other I2C devices?  I'm not sure if regmap_writable
should handle the wrapping case if it is too special.



Most of the i2c RTCs do address wrapping which is sometimes the only way
to properly set the time.


Adding Mark and Nandor to the loop.

Regards,
Leo



Hi,
  `regmap` provides couple of ways to validate the registers: 
max_register, callback function and write table. All of these are 
optional, so it gives you the freedom to customize it as needed.


In this situation probably you could:
  1. Avoid using the wrapping feature of pcf85363 (you can just provide 
separate calls for stop, reset and time confguration). In this way the 
`max_register` validation method will work fine.
  2. Replace `max_register` method validation with `callback function` 
validation method, were you could make your own validation.



Regards,
   Nandor




Re: drivers/power/reset/nvmem-reboot-mode.c:27:42: error: passing argument 2 of 'nvmem_cell_write' from incompatible pointer type

2019-08-12 Thread Nandor Han

On 8/11/19 2:43 AM, kbuild test robot wrote:

Hi Han,

FYI, the error/warning still remains.

tree:   
https://kernel.googlesource.com/pub/scm/linux/kernel/git/torvalds/linux.git 
master
head:   dcbb4a153971ff8646af0c963f5698bf21bfbfdc
commit: 7a78a7f7695bf9ef9cef3c06fbc5fa4573fd0eef power: reset: 
nvmem-reboot-mode: use NVMEM as reboot mode write interface
date:   7 weeks ago
config: x86_64-randconfig-d003-201932 (attached as .config)
compiler: gcc-7 (Debian 7.4.0-10) 7.4.0
reproduce:
 git checkout 7a78a7f7695bf9ef9cef3c06fbc5fa4573fd0eef
 # save the attached .config to linux build tree
 make ARCH=x86_64

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot 

All errors (new ones prefixed by >>):

drivers/power/reset/nvmem-reboot-mode.c: In function 
'nvmem_reboot_mode_write':

drivers/power/reset/nvmem-reboot-mode.c:27:42: error: passing argument 2 of 
'nvmem_cell_write' from incompatible pointer type 
[-Werror=incompatible-pointer-types]

  ret = nvmem_cell_write(nvmem_rbm->cell, , sizeof(magic));
  ^
In file included from drivers/power/reset/nvmem-reboot-mode.c:10:0:
include/linux/nvmem-consumer.h:120:19: note: expected 'const char *' but 
argument is of type 'unsigned int *'
 static inline int nvmem_cell_write(struct nvmem_cell *cell,
   ^~~~
cc1: some warnings being treated as errors

vim +/nvmem_cell_write +27 drivers/power/reset/nvmem-reboot-mode.c

 18 
 19 static int nvmem_reboot_mode_write(struct reboot_mode_driver *reboot,
 20 unsigned int magic)
 21 {
 22 int ret;
 23 struct nvmem_reboot_mode *nvmem_rbm;
 24 
 25 nvmem_rbm = container_of(reboot, struct nvmem_reboot_mode, 
reboot);
 26 
   > 27  ret = nvmem_cell_write(nvmem_rbm->cell, , 
sizeof(magic));
 28 if (ret < 0)
 29 dev_err(reboot->dev, "update reboot mode bits 
failed\n");
 30 
 31 return ret;
 32 }
 33 

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation



Hi,

Seems that `nvmem-consumer.h` declares a different signature for 
`nvmem_cell_write` method depending on `CONFIG_NVMEM` configuration:


#if IS_ENABLED(CONFIG_NVMEM)

...

int nvmem_cell_write(struct nvmem_cell *cell, void *buf, size_t len);

...

#else

...
static inline int nvmem_cell_write(struct nvmem_cell *cell,
const char *buf, size_t len)
{
return -EOPNOTSUPP;
}

...

#endif /* CONFIG_NVMEM *

What's the best approach here?

Nandor


Re: [PATCH] power: reset: nvmem-reboot-mode: add CONFIG_OF dependency

2019-08-05 Thread Nandor Han

On 7/8/19 3:52 PM, Arnd Bergmann wrote:

Without CONFIG_OF, we get a build failure in the reboot-mode
implementation:

drivers/power/reset/reboot-mode.c: In function 'reboot_mode_register':
drivers/power/reset/reboot-mode.c:72:2: error: implicit declaration of function 
'for_each_property_of_node'; did you mean 'for_each_child_of_node'? 
[-Werror=implicit-function-declaration]
   for_each_property_of_node(np, prop) {

Add a Kconfig dependency like we have for the other users of
CONFIG_REBOOT_MODE.

Fixes: 7a78a7f7695b ("power: reset: nvmem-reboot-mode: use NVMEM as reboot mode 
write interface")
Signed-off-by: Arnd Bergmann 
---
  drivers/power/reset/Kconfig | 1 +
  1 file changed, 1 insertion(+)

diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig
index 8dfb105db391..a564237278ff 100644
--- a/drivers/power/reset/Kconfig
+++ b/drivers/power/reset/Kconfig
@@ -248,6 +248,7 @@ config POWER_RESET_SC27XX
  
  config NVMEM_REBOOT_MODE

tristate "Generic NVMEM reboot mode driver"
+   depends on OF
select REBOOT_MODE
help
  Say y here will enable reboot mode driver. This will



Wouldn't this be more appropriate to add the "depends on OF" to "config 
REBOOT_MODE" section, since this is an error to `reboot-mode.c` unit?


Nandor


Re: [PATCH v4 0/2] Use NVMEM as reboot-mode write interface

2019-06-27 Thread Nandor Han

Hi,


Changes since v3:

  - documentation updated according to the comments


Thanks, queued. Please fix your git/mail setup, I had to fix the
line endings (\r\n -> \n) to apply this.

-- Sebastian



Ok. Thanks Sebastian.

--
Nandor


Re: [PATCH v3 2/2] dt-bindings: power: reset: add document for NVMEM based reboot-mode

2019-05-10 Thread Nandor Han

On 5/1/19 1:47 AM, Rob Herring wrote:

Hi Rob,
  Thanks for review.


@@ -0,0 +1,32 @@
+NVMEM reboot mode driver
+
+This driver gets reboot mode magic value from reboot-mode driver
+and stores it in a NVMEM cell named "reboot-mode". Then the bootloader
+can read it and take different action according to the magic
+value stored.


This is also assuming the nvmem is writeable which is more often not the
case.

Is your usecase a platform that supports pstore? Adding on to that
binding might be a better fit.



I'm using an RTC persistent memory for storing this data. The available
memory is low and don't think pstore will fit in this case.


+The rest of the properties should follow the generic reboot-mode description
+found in reboot-mode.txt
+
+Example:
+   reboot-mode-nvmem@0 {


What's this node for?


+   compatible = "simple-mfd";


I only see 1 function.



No need to this. Will remove

Nandor


Re: [PATCH v2 1/2] power: reset: nvmem-reboot-mode: use NVMEM as reboot mode write interface

2019-04-18 Thread Nandor Han

On 4/18/19 12:56 AM, Sebastian Reichel wrote:

On Thu, Apr 11, 2019 at 05:54:09AM +, Han Nandor wrote:

Add a new reboot mode write interface that is using an NVMEM cell
to store the reboot mode magic.

Signed-off-by: Nandor Han 
---





+module_platform_driver(nvmem_reboot_mode_driver);
+
+MODULE_AUTHOR("Nandor Han ");
+MODULE_DESCRIPTION("NVMEM reboot mode driver");
+MODULE_LICENSE("GPL v2");


I suppose as of bf7fbeeae6db "GPL v2" is deprecated, before it was
often read as GPL v2 only. In both cases it makes sense to just
use "GPL". Otherwise the driver looks fine to me, but I'm waiting
for Rob's review of the DT bindings before merging this.

-- Sebastian



Thanks Sebastian.

Do you want me to send a new revision with license changed to "GPL"?..or 
will you change it when it's merged?


Nandor


Re: [PATCH 1/1] rtc: ds3232: get SRAM access using NVMEM Framework

2019-04-08 Thread Nandor Han

On 4/6/19 12:44 AM, Alexandre Belloni wrote:

Hi,

On 05/04/2019 11:14:35+, Han Nandor wrote:





`
# hexdump -n 10 -C /sys/bus/nvmem/devices/ds3232_sram0/nvmem
  74 65 73 74 69 6e 67 0a  00 00|testing...|
000a
`



Thanks for that nice description!



Glad that you like it. :)



  drivers/rtc/rtc-ds3232.c | 41 ++--
  1 file changed, 39 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/rtc-ds3232.c b/drivers/rtc/rtc-ds3232.c
index 7184e5145f12..fe615aedaa9a 100644
--- a/drivers/rtc/rtc-ds3232.c
+++ b/drivers/rtc/rtc-ds3232.c
@@ -24,6 +24,8 @@
  #include 
  #include 
  
+#include "rtc-core.h"

+


The drivers should not have to include that header, see
fd5cd21d995e67f87b3eb4adf938be85fe83ef4b (from 4.16).



Ahh...right. I did the implementation on 4.14 and it was needed there.
I will change the implementation but I'll not be able to fully test it.




  struct ds3232 {
struct device *dev;
struct regmap *regmap;
int irq;
struct rtc_device *rtc;
+   struct nvmem_config nvmem_cfg;
  


You don't actually need to keep that structure for the whole life of the
device as it is copied as soon as it is registered. I usually prefer to
have it on the stack.



Very true. Done




+   ds3232->nvmem_cfg.stride = 1;
+   ds3232->nvmem_cfg.size = DS3232_REG_SRAM_SIZE;
+   ds3232->nvmem_cfg.word_size = 1;
+   ds3232->nvmem_cfg.reg_read = ds3232_nvmem_read;
+   ds3232->nvmem_cfg.reg_write = ds3232_nvmem_write;
+   ds3232->nvmem_cfg.priv = ds3232;


Please also set the type (battery backed).



Done.



  
+	ds3232->rtc->dev.of_node = dev->of_node;


Please don't mess with rtc->dev.



In my use-case I do use the device tree to declare a nvmem cell in this 
RTC node, which is used by another driver. Without this change finding 
the cell (using `devm_nvmem_cell_get` function) was failing because of 
missing `of_node` (again this was on 4.14).


I can do a bit of digging to see if something was changed, but I will 
really appreciate your advice, because I'm not be able to test the fix 
on linux-next.



+   ds3232->rtc->nvmem_config = >nvmem_cfg;
+   rtc_nvmem_register(ds3232->rtc);


This part will not compile on v5.1-rc1.



Sorry. I will fix this, and I will at least see that it builds on 
linux-next.


Thanks for review,
Nandor


Re: [RFC PATCH 0/1] Verify if register is writeable before a write operation

2019-04-02 Thread Nandor Han

On 4/2/19 12:06 PM, Mark Brown wrote:

On Tue, Apr 02, 2019 at 08:01:21AM +, Han Nandor wrote:

Description
---
This is an RFC because I don't know if this is a bug or a normal use
case. It seems that the function `_regmap_raw_write_impl` from the regmap
framework verifies that a register is writable only using
the callback function, ignoring the other two (max allowed register,
register ranges)


Please don't send cover letters for single patches, if there is anything
that needs saying put it in the changelog of the patch or after the ---
if it's administrative stuff.  This reduces mail volume and ensures that
any important information is recorded in the changelog rather than being
lost.



Copy that. If is OK, we can still discuss about this using this patch 
and I can send a single patch once the things are clear.


Re: EXT: Re: [PATCHv1] Input: atmel_mxt_ts - fix the firmware update

2018-04-17 Thread Nandor Han

On 23/03/18 21:47, Nick Dyer wrote:

On Thu, Mar 22, 2018 at 05:43:30PM +0100, Sebastian Reichel wrote:

The automatic update mechanism will trigger an update if the
info block CRCs are different between maxtouch configuration
file (maxtouch.cfg) and chip.

The driver compared the CRCs without retrieving the chip CRC,
resulting always in a failure and firmware flashing action
triggered. The patch will fix this issue by retrieving the
chip info block CRC before the check.


Thanks for raising this, I agree it's definitely something we want to
fix.

However, I'm not convinced you're solving the problem in the best way.
You've attached it to the read_t9_resolution() code path, whereas the
info block is common between T9 and T100 and works in the same way.

Would you mind trying the below patch? I've dusted it off from some
work that I did back in 2012 and it should solve your issue.

It also has the benefit that by reading the information block and the
object table into a contiguous region of memory, we can verify the
checksum at probe time. This means we make sure that we are indeed
talking to a chip that supports object protocol correctly.



Thanks Nick. This looks like a better solution.
Sebastian will test this and we can see the results.


Nandor


Re: EXT: Re: [PATCHv1] Input: atmel_mxt_ts - fix the firmware update

2018-04-17 Thread Nandor Han

On 23/03/18 21:47, Nick Dyer wrote:

On Thu, Mar 22, 2018 at 05:43:30PM +0100, Sebastian Reichel wrote:

The automatic update mechanism will trigger an update if the
info block CRCs are different between maxtouch configuration
file (maxtouch.cfg) and chip.

The driver compared the CRCs without retrieving the chip CRC,
resulting always in a failure and firmware flashing action
triggered. The patch will fix this issue by retrieving the
chip info block CRC before the check.


Thanks for raising this, I agree it's definitely something we want to
fix.

However, I'm not convinced you're solving the problem in the best way.
You've attached it to the read_t9_resolution() code path, whereas the
info block is common between T9 and T100 and works in the same way.

Would you mind trying the below patch? I've dusted it off from some
work that I did back in 2012 and it should solve your issue.

It also has the benefit that by reading the information block and the
object table into a contiguous region of memory, we can verify the
checksum at probe time. This means we make sure that we are indeed
talking to a chip that supports object protocol correctly.



Thanks Nick. This looks like a better solution.
Sebastian will test this and we can see the results.


Nandor


Re: EXT: [PATCH 3/4] gpio: Remove VLA from xra1403 driver

2018-03-12 Thread Nandor Han

On 10/03/18 02:10, Laura Abbott wrote:


The new challenge is to remove VLAs from the kernel
(see https://lkml.org/lkml/2018/3/7/621)

This patch replaces a VLA with an appropriate call to kmalloc_array.

Signed-off-by: Laura Abbott <labb...@redhat.com>
---


This looks good to me.
Reviewed-by: Nandor Han <nandor@ge.com>

Nandor


  drivers/gpio/gpio-xra1403.c | 8 +++-
  1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-xra1403.c b/drivers/gpio/gpio-xra1403.c
index 0230e4b7a2fb..8d4c8e99b251 100644
--- a/drivers/gpio/gpio-xra1403.c
+++ b/drivers/gpio/gpio-xra1403.c
@@ -126,11 +126,16 @@ static void xra1403_dbg_show(struct seq_file *s, struct 
gpio_chip *chip)
  {
int reg;
struct xra1403 *xra = gpiochip_get_data(chip);
-   int value[xra1403_regmap_cfg.max_register];
+   int *value;
int i;
unsigned int gcr;
unsigned int gsr;
  
+	value = kmalloc_array(xra1403_regmap_cfg.max_register, sizeof(*value),

+   GFP_KERNEL);
+   if (!value)
+   return;
+
seq_puts(s, "xra reg:");
for (reg = 0; reg <= xra1403_regmap_cfg.max_register; reg++)
seq_printf(s, " %2.2x", reg);
@@ -154,6 +159,7 @@ static void xra1403_dbg_show(struct seq_file *s, struct 
gpio_chip *chip)
   (gcr & BIT(i)) ? "in" : "out",
   (gsr & BIT(i)) ? "hi" : "lo");
}
+   kfree(value);
  }
  #else
  #define xra1403_dbg_show NULL





Re: EXT: [PATCH 3/4] gpio: Remove VLA from xra1403 driver

2018-03-12 Thread Nandor Han

On 10/03/18 02:10, Laura Abbott wrote:


The new challenge is to remove VLAs from the kernel
(see https://lkml.org/lkml/2018/3/7/621)

This patch replaces a VLA with an appropriate call to kmalloc_array.

Signed-off-by: Laura Abbott 
---


This looks good to me.
Reviewed-by: Nandor Han 

Nandor


  drivers/gpio/gpio-xra1403.c | 8 +++-
  1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-xra1403.c b/drivers/gpio/gpio-xra1403.c
index 0230e4b7a2fb..8d4c8e99b251 100644
--- a/drivers/gpio/gpio-xra1403.c
+++ b/drivers/gpio/gpio-xra1403.c
@@ -126,11 +126,16 @@ static void xra1403_dbg_show(struct seq_file *s, struct 
gpio_chip *chip)
  {
int reg;
struct xra1403 *xra = gpiochip_get_data(chip);
-   int value[xra1403_regmap_cfg.max_register];
+   int *value;
int i;
unsigned int gcr;
unsigned int gsr;
  
+	value = kmalloc_array(xra1403_regmap_cfg.max_register, sizeof(*value),

+   GFP_KERNEL);
+   if (!value)
+   return;
+
seq_puts(s, "xra reg:");
for (reg = 0; reg <= xra1403_regmap_cfg.max_register; reg++)
seq_printf(s, " %2.2x", reg);
@@ -154,6 +159,7 @@ static void xra1403_dbg_show(struct seq_file *s, struct 
gpio_chip *chip)
   (gcr & BIT(i)) ? "in" : "out",
   (gsr & BIT(i)) ? "hi" : "lo");
}
+   kfree(value);
  }
  #else
  #define xra1403_dbg_show NULL





[PATCH v3 0/2] XRA1403,gpio - add XRA1403 gpio expander driver

2017-05-14 Thread Nandor Han
The patchset will add a driver to support basic functionality for
XRA1403 device. Features supported:
- get/set GPIO direction (input, output)
- get/set GPIO level (low, high)

Documentation: A gpio-xra1403.txt file was added to document the DTS
bindings related to driver.

Testing:

1.1 XRA1403 connected to iMX53 MCU
1.2 Use the GPIO tools provided by kernel from tools/gpio dir

2.1 `lsgpio`

root@csmon ppd:~# lsgpio
GPIO chip: gpiochip8, "xra1403", 16 GPIO lines
line  0: unnamed unused [output]
line  1: unnamed unused [output]
line  2: unnamed unused [output]
line  3: unnamed unused [output]
line  4: unnamed unused [output]
line  5: unnamed unused
line  6: unnamed unused
line  7: unnamed unused [output]
line  8: unnamed unused [output]
line  9: unnamed unused
line 10: unnamed unused [output]
line 11: unnamed unused
line 12: unnamed unused
line 13: unnamed unused
line 14: unnamed unused
line 15: unnamed unused [output]
GPIO chip: gpiochip7, "xra1403", 16 GPIO lines
line  0: unnamed unused
line  1: unnamed unused
line  2: unnamed unused
line  3: unnamed unused
line  4: unnamed unused
line  5: unnamed unused
line  6: unnamed unused
line  7: unnamed unused
line  8: unnamed unused
line  9: unnamed unused
line 10: unnamed unused
line 11: unnamed unused
line 12: unnamed unused
line 13: unnamed unused
line 14: unnamed unused
line 15: unnamed unused

3.1 `gpio-hammer`

root@csmon ppd:~# gpio-hammer -n gpiochip8 -o0 -o1 -o2 -o3
Hammer lines [0, 1, 2, 3] on gpiochip8, initial states: [0, 0, 0, 0]
[\] [0: 0, 1: 0, 2: 0, 3: 0]
...
[\] [0: 1, 1: 1, 2: 1, 3: 1]

When using `gpio-hammer` I also attached an oscilloscope to one of the pins and 
I was
able to monitor and validate the GPIO status.


Changes since v2: - improve the commit message by removing the "sysfs"
dependency and be more clear.
  - include "seq_file.h" header.
Note: documentation patches were accepted already.

Changes since v1: - use regmap for driver
  - small changes to documentation


Nandor Han (2):
  gpio - Add EXAR XRA1403 SPI GPIO expander driver
  Add XRA1403 support to MAINTAINERS file

 MAINTAINERS |   8 ++
 drivers/gpio/Kconfig|   5 +
 drivers/gpio/Makefile   |   1 +
 drivers/gpio/gpio-xra1403.c | 237 
 4 files changed, 251 insertions(+)
 create mode 100644 drivers/gpio/gpio-xra1403.c

-- 
2.10.1



[PATCH v3 2/2] Add XRA1403 support to MAINTAINERS file

2017-05-14 Thread Nandor Han
Add XRA1403 support to MAINTAINERS list.

Signed-off-by: Nandor Han <nandor@ge.com>
---
 MAINTAINERS | 8 
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index f9deb67..db86335 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14078,6 +14078,14 @@ L: linux-kernel@vger.kernel.org
 S: Supported
 F: drivers/char/xillybus/
 
+XRA1403 GPIO EXPANDER
+M: Nandor Han <nandor@ge.com>
+M: Semi Malinen <semi.mali...@ge.com>
+L: linux-g...@vger.kernel.org
+S: Maintained
+F: drivers/gpio/gpio-xra1403.c
+F: Documentation/devicetree/bindings/gpio/gpio-xra1403.txt
+
 XTENSA XTFPGA PLATFORM SUPPORT
 M: Max Filippov <jcmvb...@gmail.com>
 L: linux-xte...@linux-xtensa.org
-- 
2.10.1



[PATCH v3 0/2] XRA1403,gpio - add XRA1403 gpio expander driver

2017-05-14 Thread Nandor Han
The patchset will add a driver to support basic functionality for
XRA1403 device. Features supported:
- get/set GPIO direction (input, output)
- get/set GPIO level (low, high)

Documentation: A gpio-xra1403.txt file was added to document the DTS
bindings related to driver.

Testing:

1.1 XRA1403 connected to iMX53 MCU
1.2 Use the GPIO tools provided by kernel from tools/gpio dir

2.1 `lsgpio`

root@csmon ppd:~# lsgpio
GPIO chip: gpiochip8, "xra1403", 16 GPIO lines
line  0: unnamed unused [output]
line  1: unnamed unused [output]
line  2: unnamed unused [output]
line  3: unnamed unused [output]
line  4: unnamed unused [output]
line  5: unnamed unused
line  6: unnamed unused
line  7: unnamed unused [output]
line  8: unnamed unused [output]
line  9: unnamed unused
line 10: unnamed unused [output]
line 11: unnamed unused
line 12: unnamed unused
line 13: unnamed unused
line 14: unnamed unused
line 15: unnamed unused [output]
GPIO chip: gpiochip7, "xra1403", 16 GPIO lines
line  0: unnamed unused
line  1: unnamed unused
line  2: unnamed unused
line  3: unnamed unused
line  4: unnamed unused
line  5: unnamed unused
line  6: unnamed unused
line  7: unnamed unused
line  8: unnamed unused
line  9: unnamed unused
line 10: unnamed unused
line 11: unnamed unused
line 12: unnamed unused
line 13: unnamed unused
line 14: unnamed unused
line 15: unnamed unused

3.1 `gpio-hammer`

root@csmon ppd:~# gpio-hammer -n gpiochip8 -o0 -o1 -o2 -o3
Hammer lines [0, 1, 2, 3] on gpiochip8, initial states: [0, 0, 0, 0]
[\] [0: 0, 1: 0, 2: 0, 3: 0]
...
[\] [0: 1, 1: 1, 2: 1, 3: 1]

When using `gpio-hammer` I also attached an oscilloscope to one of the pins and 
I was
able to monitor and validate the GPIO status.


Changes since v2: - improve the commit message by removing the "sysfs"
dependency and be more clear.
  - include "seq_file.h" header.
Note: documentation patches were accepted already.

Changes since v1: - use regmap for driver
  - small changes to documentation


Nandor Han (2):
  gpio - Add EXAR XRA1403 SPI GPIO expander driver
  Add XRA1403 support to MAINTAINERS file

 MAINTAINERS |   8 ++
 drivers/gpio/Kconfig|   5 +
 drivers/gpio/Makefile   |   1 +
 drivers/gpio/gpio-xra1403.c | 237 
 4 files changed, 251 insertions(+)
 create mode 100644 drivers/gpio/gpio-xra1403.c

-- 
2.10.1



[PATCH v3 2/2] Add XRA1403 support to MAINTAINERS file

2017-05-14 Thread Nandor Han
Add XRA1403 support to MAINTAINERS list.

Signed-off-by: Nandor Han 
---
 MAINTAINERS | 8 
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index f9deb67..db86335 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14078,6 +14078,14 @@ L: linux-kernel@vger.kernel.org
 S: Supported
 F: drivers/char/xillybus/
 
+XRA1403 GPIO EXPANDER
+M: Nandor Han 
+M: Semi Malinen 
+L: linux-g...@vger.kernel.org
+S: Maintained
+F: drivers/gpio/gpio-xra1403.c
+F: Documentation/devicetree/bindings/gpio/gpio-xra1403.txt
+
 XTENSA XTFPGA PLATFORM SUPPORT
 M: Max Filippov 
 L: linux-xte...@linux-xtensa.org
-- 
2.10.1



[PATCH v3 1/2] gpio - Add EXAR XRA1403 SPI GPIO expander driver

2017-05-14 Thread Nandor Han
This driver support basic XRA1403 functionalities:
- set gpio direction
- get gpio direction
- set gpio high/low
- get gpio status

Signed-off-by: Nandor Han <nandor@ge.com>
Signed-off-by: Semi Malinen <semi.mali...@ge.com>
---
 drivers/gpio/Kconfig|   5 +
 drivers/gpio/Makefile   |   1 +
 drivers/gpio/gpio-xra1403.c | 237 
 3 files changed, 243 insertions(+)
 create mode 100644 drivers/gpio/gpio-xra1403.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 23ca51e..dbde3ae 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1225,6 +1225,11 @@ config GPIO_PISOSR
  GPIO driver for SPI compatible parallel-in/serial-out shift
  registers. These are input only devices.
 
+config GPIO_XRA1403
+   tristate "EXAR XRA1403 16-bit GPIO expander"
+   help
+ GPIO driver for EXAR XRA1403 16-bit SPI-based GPIO expander.
+
 endmenu
 
 menu "SPI or I2C GPIO expanders"
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 68b9627..4d9adc6 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -141,6 +141,7 @@ obj-$(CONFIG_GPIO_XGENE)+= gpio-xgene.o
 obj-$(CONFIG_GPIO_XGENE_SB)+= gpio-xgene-sb.o
 obj-$(CONFIG_GPIO_XILINX)  += gpio-xilinx.o
 obj-$(CONFIG_GPIO_XLP) += gpio-xlp.o
+obj-$(CONFIG_GPIO_XRA1403) += gpio-xra1403.o
 obj-$(CONFIG_GPIO_XTENSA)  += gpio-xtensa.o
 obj-$(CONFIG_GPIO_ZEVIO)   += gpio-zevio.o
 obj-$(CONFIG_GPIO_ZYNQ)+= gpio-zynq.o
diff --git a/drivers/gpio/gpio-xra1403.c b/drivers/gpio/gpio-xra1403.c
new file mode 100644
index 000..0230e4b
--- /dev/null
+++ b/drivers/gpio/gpio-xra1403.c
@@ -0,0 +1,237 @@
+/*
+ * GPIO driver for EXAR XRA1403 16-bit GPIO expander
+ *
+ * Copyright (c) 2017, General Electric Company
+ *
+ * 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, see <http://www.gnu.org/licenses/>.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* XRA1403 registers */
+#define XRA_GSR   0x00 /* GPIO State */
+#define XRA_OCR   0x02 /* Output Control */
+#define XRA_PIR   0x04 /* Input Polarity Inversion */
+#define XRA_GCR   0x06 /* GPIO Configuration */
+#define XRA_PUR   0x08 /* Input Internal Pull-up Resistor Enable/Disable */
+#define XRA_IER   0x0A /* Input Interrupt Enable */
+#define XRA_TSCR  0x0C /* Output Three-State Control */
+#define XRA_ISR   0x0E /* Input Interrupt Status */
+#define XRA_REIR  0x10 /* Input Rising Edge Interrupt Enable */
+#define XRA_FEIR  0x12 /* Input Falling Edge Interrupt Enable */
+#define XRA_IFR   0x14 /* Input Filter Enable/Disable */
+
+struct xra1403 {
+   struct gpio_chip  chip;
+   struct regmap *regmap;
+};
+
+static const struct regmap_config xra1403_regmap_cfg = {
+   .reg_bits = 7,
+   .pad_bits = 1,
+   .val_bits = 8,
+
+   .max_register = XRA_IFR | 0x01,
+};
+
+static unsigned int to_reg(unsigned int reg, unsigned int offset)
+{
+   return reg + (offset > 7);
+}
+
+static int xra1403_direction_input(struct gpio_chip *chip, unsigned int offset)
+{
+   struct xra1403 *xra = gpiochip_get_data(chip);
+
+   return regmap_update_bits(xra->regmap, to_reg(XRA_GCR, offset),
+   BIT(offset % 8), BIT(offset % 8));
+}
+
+static int xra1403_direction_output(struct gpio_chip *chip, unsigned int 
offset,
+   int value)
+{
+   int ret;
+   struct xra1403 *xra = gpiochip_get_data(chip);
+
+   ret = regmap_update_bits(xra->regmap, to_reg(XRA_GCR, offset),
+   BIT(offset % 8), 0);
+   if (ret)
+   return ret;
+
+   ret = regmap_update_bits(xra->regmap, to_reg(XRA_OCR, offset),
+   BIT(offset % 8), value ? BIT(offset % 8) : 0);
+
+   return ret;
+}
+
+static int xra1403_get_direction(struct gpio_chip *chip, unsigned int offset)
+{
+   int ret;
+   unsigned int val;
+   struct xra1403 *xra = gpiochip_get_data(chip);
+
+   ret = regmap_read(xra->regmap, to_reg(XRA_GCR, offset), );
+   if (ret)
+   return ret;
+
+   return !!(val & BIT(offset % 8));
+}
+
+static int xra1403_get(struct gpio_chip *chip, unsigned int offset)
+{
+   int ret;
+   unsigned int val;
+   struct xra1403 *xra = gpiochi

[PATCH v3 1/2] gpio - Add EXAR XRA1403 SPI GPIO expander driver

2017-05-14 Thread Nandor Han
This driver support basic XRA1403 functionalities:
- set gpio direction
- get gpio direction
- set gpio high/low
- get gpio status

Signed-off-by: Nandor Han 
Signed-off-by: Semi Malinen 
---
 drivers/gpio/Kconfig|   5 +
 drivers/gpio/Makefile   |   1 +
 drivers/gpio/gpio-xra1403.c | 237 
 3 files changed, 243 insertions(+)
 create mode 100644 drivers/gpio/gpio-xra1403.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 23ca51e..dbde3ae 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1225,6 +1225,11 @@ config GPIO_PISOSR
  GPIO driver for SPI compatible parallel-in/serial-out shift
  registers. These are input only devices.
 
+config GPIO_XRA1403
+   tristate "EXAR XRA1403 16-bit GPIO expander"
+   help
+ GPIO driver for EXAR XRA1403 16-bit SPI-based GPIO expander.
+
 endmenu
 
 menu "SPI or I2C GPIO expanders"
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 68b9627..4d9adc6 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -141,6 +141,7 @@ obj-$(CONFIG_GPIO_XGENE)+= gpio-xgene.o
 obj-$(CONFIG_GPIO_XGENE_SB)+= gpio-xgene-sb.o
 obj-$(CONFIG_GPIO_XILINX)  += gpio-xilinx.o
 obj-$(CONFIG_GPIO_XLP) += gpio-xlp.o
+obj-$(CONFIG_GPIO_XRA1403) += gpio-xra1403.o
 obj-$(CONFIG_GPIO_XTENSA)  += gpio-xtensa.o
 obj-$(CONFIG_GPIO_ZEVIO)   += gpio-zevio.o
 obj-$(CONFIG_GPIO_ZYNQ)+= gpio-zynq.o
diff --git a/drivers/gpio/gpio-xra1403.c b/drivers/gpio/gpio-xra1403.c
new file mode 100644
index 000..0230e4b
--- /dev/null
+++ b/drivers/gpio/gpio-xra1403.c
@@ -0,0 +1,237 @@
+/*
+ * GPIO driver for EXAR XRA1403 16-bit GPIO expander
+ *
+ * Copyright (c) 2017, General Electric Company
+ *
+ * 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, see <http://www.gnu.org/licenses/>.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* XRA1403 registers */
+#define XRA_GSR   0x00 /* GPIO State */
+#define XRA_OCR   0x02 /* Output Control */
+#define XRA_PIR   0x04 /* Input Polarity Inversion */
+#define XRA_GCR   0x06 /* GPIO Configuration */
+#define XRA_PUR   0x08 /* Input Internal Pull-up Resistor Enable/Disable */
+#define XRA_IER   0x0A /* Input Interrupt Enable */
+#define XRA_TSCR  0x0C /* Output Three-State Control */
+#define XRA_ISR   0x0E /* Input Interrupt Status */
+#define XRA_REIR  0x10 /* Input Rising Edge Interrupt Enable */
+#define XRA_FEIR  0x12 /* Input Falling Edge Interrupt Enable */
+#define XRA_IFR   0x14 /* Input Filter Enable/Disable */
+
+struct xra1403 {
+   struct gpio_chip  chip;
+   struct regmap *regmap;
+};
+
+static const struct regmap_config xra1403_regmap_cfg = {
+   .reg_bits = 7,
+   .pad_bits = 1,
+   .val_bits = 8,
+
+   .max_register = XRA_IFR | 0x01,
+};
+
+static unsigned int to_reg(unsigned int reg, unsigned int offset)
+{
+   return reg + (offset > 7);
+}
+
+static int xra1403_direction_input(struct gpio_chip *chip, unsigned int offset)
+{
+   struct xra1403 *xra = gpiochip_get_data(chip);
+
+   return regmap_update_bits(xra->regmap, to_reg(XRA_GCR, offset),
+   BIT(offset % 8), BIT(offset % 8));
+}
+
+static int xra1403_direction_output(struct gpio_chip *chip, unsigned int 
offset,
+   int value)
+{
+   int ret;
+   struct xra1403 *xra = gpiochip_get_data(chip);
+
+   ret = regmap_update_bits(xra->regmap, to_reg(XRA_GCR, offset),
+   BIT(offset % 8), 0);
+   if (ret)
+   return ret;
+
+   ret = regmap_update_bits(xra->regmap, to_reg(XRA_OCR, offset),
+   BIT(offset % 8), value ? BIT(offset % 8) : 0);
+
+   return ret;
+}
+
+static int xra1403_get_direction(struct gpio_chip *chip, unsigned int offset)
+{
+   int ret;
+   unsigned int val;
+   struct xra1403 *xra = gpiochip_get_data(chip);
+
+   ret = regmap_read(xra->regmap, to_reg(XRA_GCR, offset), );
+   if (ret)
+   return ret;
+
+   return !!(val & BIT(offset % 8));
+}
+
+static int xra1403_get(struct gpio_chip *chip, unsigned int offset)
+{
+   int ret;
+   unsigned int val;
+   struct xra1403 *xra = gpiochip_get_data(chip);
+
+   ret = re

[PATCH v2 4/4] Add XRA1403 support to MAINTAINERS file

2017-04-13 Thread Nandor Han
Add XRA1403 support to MAINTAINERS list.

Signed-off-by: Nandor Han <nandor@ge.com>
---
 MAINTAINERS | 8 
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 892e958..0c5b984 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14013,6 +14013,14 @@ L: linux-kernel@vger.kernel.org
 S: Supported
 F: drivers/char/xillybus/
 
+XRA1403 GPIO EXPANDER
+M: Nandor Han <nandor@ge.com>
+M: Semi Malinen <semi.mali...@ge.com>
+L: linux-g...@vger.kernel.org
+S: Maintained
+F: drivers/gpio/gpio-xra1403.c
+F: Documentation/devicetree/bindings/gpio/gpio-xra1403.txt
+
 XTENSA XTFPGA PLATFORM SUPPORT
 M: Max Filippov <jcmvb...@gmail.com>
 L: linux-xte...@linux-xtensa.org
-- 
2.10.1



[PATCH v2 2/4] gpio - Add EXAR XRA1403 SPI GPIO expander driver

2017-04-13 Thread Nandor Han
This is a simple driver that provides a /sys/class/gpio
interface for controlling and configuring the GPIO lines.
It does not provide support for chip select or interrupts.

Signed-off-by: Nandor Han <nandor@ge.com>
Signed-off-by: Semi Malinen <semi.mali...@ge.com>
---
 drivers/gpio/Kconfig|   5 +
 drivers/gpio/Makefile   |   1 +
 drivers/gpio/gpio-xra1403.c | 236 
 3 files changed, 242 insertions(+)
 create mode 100644 drivers/gpio/gpio-xra1403.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 63ceed2..53cbe9b 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1214,6 +1214,11 @@ config GPIO_PISOSR
  GPIO driver for SPI compatible parallel-in/serial-out shift
  registers. These are input only devices.
 
+config GPIO_XRA1403
+   tristate "EXAR XRA1403 16-bit GPIO expander"
+   help
+ GPIO driver for EXAR XRA1403 16-bit SPI-based GPIO expander.
+
 endmenu
 
 menu "SPI or I2C GPIO expanders"
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 095598e..76dc3d7 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -140,6 +140,7 @@ obj-$(CONFIG_GPIO_XGENE)+= gpio-xgene.o
 obj-$(CONFIG_GPIO_XGENE_SB)+= gpio-xgene-sb.o
 obj-$(CONFIG_GPIO_XILINX)  += gpio-xilinx.o
 obj-$(CONFIG_GPIO_XLP) += gpio-xlp.o
+obj-$(CONFIG_GPIO_XRA1403) += gpio-xra1403.o
 obj-$(CONFIG_GPIO_XTENSA)  += gpio-xtensa.o
 obj-$(CONFIG_GPIO_ZEVIO)   += gpio-zevio.o
 obj-$(CONFIG_GPIO_ZYNQ)+= gpio-zynq.o
diff --git a/drivers/gpio/gpio-xra1403.c b/drivers/gpio/gpio-xra1403.c
new file mode 100644
index 000..e6966d9
--- /dev/null
+++ b/drivers/gpio/gpio-xra1403.c
@@ -0,0 +1,236 @@
+/*
+ * GPIO driver for EXAR XRA1403 16-bit GPIO expander
+ *
+ * Copyright (c) 2017, General Electric Company
+ *
+ * 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, see <http://www.gnu.org/licenses/>.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* XRA1403 registers */
+#define XRA_GSR   0x00 /* GPIO State */
+#define XRA_OCR   0x02 /* Output Control */
+#define XRA_PIR   0x04 /* Input Polarity Inversion */
+#define XRA_GCR   0x06 /* GPIO Configuration */
+#define XRA_PUR   0x08 /* Input Internal Pull-up Resistor Enable/Disable */
+#define XRA_IER   0x0A /* Input Interrupt Enable */
+#define XRA_TSCR  0x0C /* Output Three-State Control */
+#define XRA_ISR   0x0E /* Input Interrupt Status */
+#define XRA_REIR  0x10 /* Input Rising Edge Interrupt Enable */
+#define XRA_FEIR  0x12 /* Input Falling Edge Interrupt Enable */
+#define XRA_IFR   0x14 /* Input Filter Enable/Disable */
+
+struct xra1403 {
+   struct gpio_chip  chip;
+   struct regmap *regmap;
+};
+
+static const struct regmap_config xra1403_regmap_cfg = {
+   .reg_bits = 7,
+   .pad_bits = 1,
+   .val_bits = 8,
+
+   .max_register = XRA_IFR | 0x01,
+};
+
+static unsigned int to_reg(unsigned int reg, unsigned int offset)
+{
+   return reg + (offset > 7);
+}
+
+static int xra1403_direction_input(struct gpio_chip *chip, unsigned int offset)
+{
+   struct xra1403 *xra = gpiochip_get_data(chip);
+
+   return regmap_update_bits(xra->regmap, to_reg(XRA_GCR, offset),
+   BIT(offset % 8), BIT(offset % 8));
+}
+
+static int xra1403_direction_output(struct gpio_chip *chip, unsigned int 
offset,
+   int value)
+{
+   int ret;
+   struct xra1403 *xra = gpiochip_get_data(chip);
+
+   ret = regmap_update_bits(xra->regmap, to_reg(XRA_GCR, offset),
+   BIT(offset % 8), 0);
+   if (ret)
+   return ret;
+
+   ret = regmap_update_bits(xra->regmap, to_reg(XRA_OCR, offset),
+   BIT(offset % 8), value ? BIT(offset % 8) : 0);
+
+   return ret;
+}
+
+static int xra1403_get_direction(struct gpio_chip *chip, unsigned int offset)
+{
+   int ret;
+   unsigned int val;
+   struct xra1403 *xra = gpiochip_get_data(chip);
+
+   ret = regmap_read(xra->regmap, to_reg(XRA_GCR, offset), );
+   if (ret)
+   return ret;
+
+   return !!(val & BIT(offset % 8));
+}
+
+static int xra1403_get(struct gpio_chip *chip, unsigned int offset)
+{
+   int ret;
+   unsigned int val;
+   str

[PATCH v2 0/4] XRA1403,gpio - add XRA1403 gpio expander driver

2017-04-13 Thread Nandor Han
The patchset will add a driver to support basic functionality for
XRA1403 device. Features supported:
- get/set GPIO direction (input, output)
- get/set GPIO level (low, high)

Documentation: A gpio-xra1403.txt file was added to document the DTS
bindings related to driver.

Testing:

1.1 XRA1403 connected to iMX53 MCU
1.2 Use the GPIO tools provided by kernel from tools/gpio dir

2.1 `lsgpio`

root@csmon ppd:~# lsgpio
GPIO chip: gpiochip8, "xra1403", 16 GPIO lines
line  0: unnamed unused [output]
line  1: unnamed unused [output]
line  2: unnamed unused [output]
line  3: unnamed unused [output]
line  4: unnamed unused [output]
line  5: unnamed unused
line  6: unnamed unused
line  7: unnamed unused [output]
line  8: unnamed unused [output]
line  9: unnamed unused
line 10: unnamed unused [output]
line 11: unnamed unused
line 12: unnamed unused
line 13: unnamed unused
line 14: unnamed unused
line 15: unnamed unused [output]
GPIO chip: gpiochip7, "xra1403", 16 GPIO lines
line  0: unnamed unused
line  1: unnamed unused
line  2: unnamed unused
line  3: unnamed unused
line  4: unnamed unused
line  5: unnamed unused
line  6: unnamed unused
line  7: unnamed unused
line  8: unnamed unused
line  9: unnamed unused
line 10: unnamed unused
line 11: unnamed unused
line 12: unnamed unused
line 13: unnamed unused
line 14: unnamed unused
line 15: unnamed unused

3.1 `gpio-hammer`

root@csmon ppd:~# gpio-hammer -n gpiochip8 -o0 -o1 -o2 -o3
Hammer lines [0, 1, 2, 3] on gpiochip8, initial states: [0, 0, 0, 0]
[\] [0: 0, 1: 0, 2: 0, 3: 0]
...
[\] [0: 1, 1: 1, 2: 1, 3: 1]

When using `gpio-hammer` I also attached an oscilloscope to one of the pins and 
I was
able to monitor and validate the GPIO status.

Nandor Han (4):
  dt-bindings: gpio - add exar to vendor prefixes list
  gpio - Add EXAR XRA1403 SPI GPIO expander driver
  doc,dts - add XRA1403 DTS binding documentation
  Add XRA1403 support to MAINTAINERS file

 .../devicetree/bindings/gpio/gpio-xra1403.txt  |  46 
 .../devicetree/bindings/vendor-prefixes.txt|   1 +
 MAINTAINERS|   8 +
 drivers/gpio/Kconfig   |   5 +
 drivers/gpio/Makefile  |   1 +
 drivers/gpio/gpio-xra1403.c| 236 +
 6 files changed, 297 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-xra1403.txt
 create mode 100644 drivers/gpio/gpio-xra1403.c

-- 
2.10.1



[PATCH v2 4/4] Add XRA1403 support to MAINTAINERS file

2017-04-13 Thread Nandor Han
Add XRA1403 support to MAINTAINERS list.

Signed-off-by: Nandor Han 
---
 MAINTAINERS | 8 
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 892e958..0c5b984 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14013,6 +14013,14 @@ L: linux-kernel@vger.kernel.org
 S: Supported
 F: drivers/char/xillybus/
 
+XRA1403 GPIO EXPANDER
+M: Nandor Han 
+M: Semi Malinen 
+L: linux-g...@vger.kernel.org
+S: Maintained
+F: drivers/gpio/gpio-xra1403.c
+F: Documentation/devicetree/bindings/gpio/gpio-xra1403.txt
+
 XTENSA XTFPGA PLATFORM SUPPORT
 M: Max Filippov 
 L: linux-xte...@linux-xtensa.org
-- 
2.10.1



[PATCH v2 2/4] gpio - Add EXAR XRA1403 SPI GPIO expander driver

2017-04-13 Thread Nandor Han
This is a simple driver that provides a /sys/class/gpio
interface for controlling and configuring the GPIO lines.
It does not provide support for chip select or interrupts.

Signed-off-by: Nandor Han 
Signed-off-by: Semi Malinen 
---
 drivers/gpio/Kconfig|   5 +
 drivers/gpio/Makefile   |   1 +
 drivers/gpio/gpio-xra1403.c | 236 
 3 files changed, 242 insertions(+)
 create mode 100644 drivers/gpio/gpio-xra1403.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 63ceed2..53cbe9b 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1214,6 +1214,11 @@ config GPIO_PISOSR
  GPIO driver for SPI compatible parallel-in/serial-out shift
  registers. These are input only devices.
 
+config GPIO_XRA1403
+   tristate "EXAR XRA1403 16-bit GPIO expander"
+   help
+ GPIO driver for EXAR XRA1403 16-bit SPI-based GPIO expander.
+
 endmenu
 
 menu "SPI or I2C GPIO expanders"
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 095598e..76dc3d7 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -140,6 +140,7 @@ obj-$(CONFIG_GPIO_XGENE)+= gpio-xgene.o
 obj-$(CONFIG_GPIO_XGENE_SB)+= gpio-xgene-sb.o
 obj-$(CONFIG_GPIO_XILINX)  += gpio-xilinx.o
 obj-$(CONFIG_GPIO_XLP) += gpio-xlp.o
+obj-$(CONFIG_GPIO_XRA1403) += gpio-xra1403.o
 obj-$(CONFIG_GPIO_XTENSA)  += gpio-xtensa.o
 obj-$(CONFIG_GPIO_ZEVIO)   += gpio-zevio.o
 obj-$(CONFIG_GPIO_ZYNQ)+= gpio-zynq.o
diff --git a/drivers/gpio/gpio-xra1403.c b/drivers/gpio/gpio-xra1403.c
new file mode 100644
index 000..e6966d9
--- /dev/null
+++ b/drivers/gpio/gpio-xra1403.c
@@ -0,0 +1,236 @@
+/*
+ * GPIO driver for EXAR XRA1403 16-bit GPIO expander
+ *
+ * Copyright (c) 2017, General Electric Company
+ *
+ * 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, see <http://www.gnu.org/licenses/>.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* XRA1403 registers */
+#define XRA_GSR   0x00 /* GPIO State */
+#define XRA_OCR   0x02 /* Output Control */
+#define XRA_PIR   0x04 /* Input Polarity Inversion */
+#define XRA_GCR   0x06 /* GPIO Configuration */
+#define XRA_PUR   0x08 /* Input Internal Pull-up Resistor Enable/Disable */
+#define XRA_IER   0x0A /* Input Interrupt Enable */
+#define XRA_TSCR  0x0C /* Output Three-State Control */
+#define XRA_ISR   0x0E /* Input Interrupt Status */
+#define XRA_REIR  0x10 /* Input Rising Edge Interrupt Enable */
+#define XRA_FEIR  0x12 /* Input Falling Edge Interrupt Enable */
+#define XRA_IFR   0x14 /* Input Filter Enable/Disable */
+
+struct xra1403 {
+   struct gpio_chip  chip;
+   struct regmap *regmap;
+};
+
+static const struct regmap_config xra1403_regmap_cfg = {
+   .reg_bits = 7,
+   .pad_bits = 1,
+   .val_bits = 8,
+
+   .max_register = XRA_IFR | 0x01,
+};
+
+static unsigned int to_reg(unsigned int reg, unsigned int offset)
+{
+   return reg + (offset > 7);
+}
+
+static int xra1403_direction_input(struct gpio_chip *chip, unsigned int offset)
+{
+   struct xra1403 *xra = gpiochip_get_data(chip);
+
+   return regmap_update_bits(xra->regmap, to_reg(XRA_GCR, offset),
+   BIT(offset % 8), BIT(offset % 8));
+}
+
+static int xra1403_direction_output(struct gpio_chip *chip, unsigned int 
offset,
+   int value)
+{
+   int ret;
+   struct xra1403 *xra = gpiochip_get_data(chip);
+
+   ret = regmap_update_bits(xra->regmap, to_reg(XRA_GCR, offset),
+   BIT(offset % 8), 0);
+   if (ret)
+   return ret;
+
+   ret = regmap_update_bits(xra->regmap, to_reg(XRA_OCR, offset),
+   BIT(offset % 8), value ? BIT(offset % 8) : 0);
+
+   return ret;
+}
+
+static int xra1403_get_direction(struct gpio_chip *chip, unsigned int offset)
+{
+   int ret;
+   unsigned int val;
+   struct xra1403 *xra = gpiochip_get_data(chip);
+
+   ret = regmap_read(xra->regmap, to_reg(XRA_GCR, offset), );
+   if (ret)
+   return ret;
+
+   return !!(val & BIT(offset % 8));
+}
+
+static int xra1403_get(struct gpio_chip *chip, unsigned int offset)
+{
+   int ret;
+   unsigned int val;
+   struct xra1403 *xra = gpiochip_get_data(chip);
+
+   ret = re

[PATCH v2 0/4] XRA1403,gpio - add XRA1403 gpio expander driver

2017-04-13 Thread Nandor Han
The patchset will add a driver to support basic functionality for
XRA1403 device. Features supported:
- get/set GPIO direction (input, output)
- get/set GPIO level (low, high)

Documentation: A gpio-xra1403.txt file was added to document the DTS
bindings related to driver.

Testing:

1.1 XRA1403 connected to iMX53 MCU
1.2 Use the GPIO tools provided by kernel from tools/gpio dir

2.1 `lsgpio`

root@csmon ppd:~# lsgpio
GPIO chip: gpiochip8, "xra1403", 16 GPIO lines
line  0: unnamed unused [output]
line  1: unnamed unused [output]
line  2: unnamed unused [output]
line  3: unnamed unused [output]
line  4: unnamed unused [output]
line  5: unnamed unused
line  6: unnamed unused
line  7: unnamed unused [output]
line  8: unnamed unused [output]
line  9: unnamed unused
line 10: unnamed unused [output]
line 11: unnamed unused
line 12: unnamed unused
line 13: unnamed unused
line 14: unnamed unused
line 15: unnamed unused [output]
GPIO chip: gpiochip7, "xra1403", 16 GPIO lines
line  0: unnamed unused
line  1: unnamed unused
line  2: unnamed unused
line  3: unnamed unused
line  4: unnamed unused
line  5: unnamed unused
line  6: unnamed unused
line  7: unnamed unused
line  8: unnamed unused
line  9: unnamed unused
line 10: unnamed unused
line 11: unnamed unused
line 12: unnamed unused
line 13: unnamed unused
line 14: unnamed unused
line 15: unnamed unused

3.1 `gpio-hammer`

root@csmon ppd:~# gpio-hammer -n gpiochip8 -o0 -o1 -o2 -o3
Hammer lines [0, 1, 2, 3] on gpiochip8, initial states: [0, 0, 0, 0]
[\] [0: 0, 1: 0, 2: 0, 3: 0]
...
[\] [0: 1, 1: 1, 2: 1, 3: 1]

When using `gpio-hammer` I also attached an oscilloscope to one of the pins and 
I was
able to monitor and validate the GPIO status.

Nandor Han (4):
  dt-bindings: gpio - add exar to vendor prefixes list
  gpio - Add EXAR XRA1403 SPI GPIO expander driver
  doc,dts - add XRA1403 DTS binding documentation
  Add XRA1403 support to MAINTAINERS file

 .../devicetree/bindings/gpio/gpio-xra1403.txt  |  46 
 .../devicetree/bindings/vendor-prefixes.txt|   1 +
 MAINTAINERS|   8 +
 drivers/gpio/Kconfig   |   5 +
 drivers/gpio/Makefile  |   1 +
 drivers/gpio/gpio-xra1403.c| 236 +
 6 files changed, 297 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-xra1403.txt
 create mode 100644 drivers/gpio/gpio-xra1403.c

-- 
2.10.1



[PATCH v2 1/4] dt-bindings: gpio - add exar to vendor prefixes list

2017-04-13 Thread Nandor Han
Add Exar Corporation to vendors list.

Signed-off-by: Nandor Han <nandor@ge.com>
---
 Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt 
b/Documentation/devicetree/bindings/vendor-prefixes.txt
index c7d1b72..ba9058c 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -104,6 +104,7 @@ ettus   NI Ettus Research
 eukrea  Eukréa Electromatique
 everestEverest Semiconductor Co. Ltd.
 everspin   Everspin Technologies, Inc.
+exar   Exar Corporation
 excito Excito
 ezchip EZchip Semiconductor
 faradayFaraday Technology Corporation
-- 
2.10.1



[PATCH v2 1/4] dt-bindings: gpio - add exar to vendor prefixes list

2017-04-13 Thread Nandor Han
Add Exar Corporation to vendors list.

Signed-off-by: Nandor Han 
---
 Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt 
b/Documentation/devicetree/bindings/vendor-prefixes.txt
index c7d1b72..ba9058c 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -104,6 +104,7 @@ ettus   NI Ettus Research
 eukrea  Eukréa Electromatique
 everestEverest Semiconductor Co. Ltd.
 everspin   Everspin Technologies, Inc.
+exar   Exar Corporation
 excito Excito
 ezchip EZchip Semiconductor
 faradayFaraday Technology Corporation
-- 
2.10.1



[PATCH v2 3/4] doc,dts - add XRA1403 DTS binding documentation

2017-04-13 Thread Nandor Han
Add the XRA1403 DTS binding documentation.

Signed-off-by: Nandor Han <nandor@ge.com>
---
 .../devicetree/bindings/gpio/gpio-xra1403.txt  | 46 ++
 1 file changed, 46 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-xra1403.txt

diff --git a/Documentation/devicetree/bindings/gpio/gpio-xra1403.txt 
b/Documentation/devicetree/bindings/gpio/gpio-xra1403.txt
new file mode 100644
index 000..e13cc39
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-xra1403.txt
@@ -0,0 +1,46 @@
+GPIO Driver for XRA1403 16-BIT GPIO Expander With Reset Input from EXAR
+
+The XRA1403 is an 16-bit GPIO expander with an SPI interface. Features 
available:
+   - Individually programmable inputs:
+   - Internal pull-up resistors
+   - Polarity inversion
+   - Individual interrupt enable
+   - Rising edge and/or Falling edge interrupt
+   - Input filter
+   - Individually programmable outputs
+   - Output Level Control
+   - Output Three-State Control
+
+Properties
+--
+Check documentation for SPI and GPIO controllers regarding properties needed 
to configure the node.
+
+   - compatible = "exar,xra1403".
+   - reg - SPI id of the device.
+   - gpio-controller - marks the node as gpio.
+   - #gpio-cells - should be two where the first cell is the pin number
+   and the second one is used for optional parameters.
+
+Optional properties:
+---
+   - reset-gpios: in case available used to control the device reset line.
+   - interrupt-controller - marks the node as interrupt controller.
+   - #interrupt-cells - should be two and represents the number of cells
+   needed to encode interrupt source.
+
+Example
+
+
+   gpioxra0: gpio@2 {
+   compatible = "exar,xra1403";
+   reg = <2>;
+
+   gpio-controller;
+   #gpio-cells = <2>;
+
+   interrupt-controller;
+   #interrupt-cells = <2>;
+
+   reset-gpios = < 6 GPIO_ACTIVE_LOW>;
+   spi-max-frequency = <100>;
+   };
-- 
2.10.1



[PATCH v2 3/4] doc,dts - add XRA1403 DTS binding documentation

2017-04-13 Thread Nandor Han
Add the XRA1403 DTS binding documentation.

Signed-off-by: Nandor Han 
---
 .../devicetree/bindings/gpio/gpio-xra1403.txt  | 46 ++
 1 file changed, 46 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-xra1403.txt

diff --git a/Documentation/devicetree/bindings/gpio/gpio-xra1403.txt 
b/Documentation/devicetree/bindings/gpio/gpio-xra1403.txt
new file mode 100644
index 000..e13cc39
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-xra1403.txt
@@ -0,0 +1,46 @@
+GPIO Driver for XRA1403 16-BIT GPIO Expander With Reset Input from EXAR
+
+The XRA1403 is an 16-bit GPIO expander with an SPI interface. Features 
available:
+   - Individually programmable inputs:
+   - Internal pull-up resistors
+   - Polarity inversion
+   - Individual interrupt enable
+   - Rising edge and/or Falling edge interrupt
+   - Input filter
+   - Individually programmable outputs
+   - Output Level Control
+   - Output Three-State Control
+
+Properties
+--
+Check documentation for SPI and GPIO controllers regarding properties needed 
to configure the node.
+
+   - compatible = "exar,xra1403".
+   - reg - SPI id of the device.
+   - gpio-controller - marks the node as gpio.
+   - #gpio-cells - should be two where the first cell is the pin number
+   and the second one is used for optional parameters.
+
+Optional properties:
+---
+   - reset-gpios: in case available used to control the device reset line.
+   - interrupt-controller - marks the node as interrupt controller.
+   - #interrupt-cells - should be two and represents the number of cells
+   needed to encode interrupt source.
+
+Example
+
+
+   gpioxra0: gpio@2 {
+   compatible = "exar,xra1403";
+   reg = <2>;
+
+   gpio-controller;
+   #gpio-cells = <2>;
+
+   interrupt-controller;
+   #interrupt-cells = <2>;
+
+   reset-gpios = < 6 GPIO_ACTIVE_LOW>;
+   spi-max-frequency = <100>;
+   };
-- 
2.10.1



[PATCH 2/3] doc,dts - add XRA1403 DTS binding documentation

2017-03-27 Thread Nandor Han
Add the XRA1403 DTS binding documentation.

Signed-off-by: Nandor Han <nandor@ge.com>
---
 .../devicetree/bindings/gpio/gpio-xra1403.txt  | 37 ++
 1 file changed, 37 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-xra1403.txt

diff --git a/Documentation/devicetree/bindings/gpio/gpio-xra1403.txt 
b/Documentation/devicetree/bindings/gpio/gpio-xra1403.txt
new file mode 100644
index 000..ccf5337
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-xra1403.txt
@@ -0,0 +1,37 @@
+GPIO Driver for XRA1403 16-BIT GPIO Expander With Reset Input from EXAR
+
+The XRA1403 is an 16-bit GPIO expander with an SPI interface. Features 
available:
+   - Individually programmable inputs:
+   - Internal pull-up resistors
+   - Polarity inversion
+   - Individual interrupt enable
+   - Rising edge and/or Falling edge interrupt
+   - Input filter
+   - Individually programmable outputs
+   - Output Level Control
+   - Output Three-State Control
+
+Properties
+--
+Check documentation for SPI and GPIO controllers regarding properties needed 
to configure the node.
+
+   - compatible = "exar,xra1403".
+   - reg = SPI id of the device.
+   - gpio-controller: mark the node as gpio.
+
+Optional properties:
+---
+   - reset-gpios: in case available used to control the device reset line.
+
+Example
+
+
+   gpioxra0: gpio@2 {
+   compatible = "exar,xra1403";
+   reg = <2>;
+   gpio-controller;
+   #gpio-cells = <2>;
+   reset-gpios = < 6 GPIO_ACTIVE_LOW>;
+   spi-max-frequency = <100>;
+   status = "okay";
+   };
-- 
2.10.1



[PATCH 2/3] doc,dts - add XRA1403 DTS binding documentation

2017-03-27 Thread Nandor Han
Add the XRA1403 DTS binding documentation.

Signed-off-by: Nandor Han 
---
 .../devicetree/bindings/gpio/gpio-xra1403.txt  | 37 ++
 1 file changed, 37 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-xra1403.txt

diff --git a/Documentation/devicetree/bindings/gpio/gpio-xra1403.txt 
b/Documentation/devicetree/bindings/gpio/gpio-xra1403.txt
new file mode 100644
index 000..ccf5337
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-xra1403.txt
@@ -0,0 +1,37 @@
+GPIO Driver for XRA1403 16-BIT GPIO Expander With Reset Input from EXAR
+
+The XRA1403 is an 16-bit GPIO expander with an SPI interface. Features 
available:
+   - Individually programmable inputs:
+   - Internal pull-up resistors
+   - Polarity inversion
+   - Individual interrupt enable
+   - Rising edge and/or Falling edge interrupt
+   - Input filter
+   - Individually programmable outputs
+   - Output Level Control
+   - Output Three-State Control
+
+Properties
+--
+Check documentation for SPI and GPIO controllers regarding properties needed 
to configure the node.
+
+   - compatible = "exar,xra1403".
+   - reg = SPI id of the device.
+   - gpio-controller: mark the node as gpio.
+
+Optional properties:
+---
+   - reset-gpios: in case available used to control the device reset line.
+
+Example
+
+
+   gpioxra0: gpio@2 {
+   compatible = "exar,xra1403";
+   reg = <2>;
+   gpio-controller;
+   #gpio-cells = <2>;
+   reset-gpios = < 6 GPIO_ACTIVE_LOW>;
+   spi-max-frequency = <100>;
+   status = "okay";
+   };
-- 
2.10.1



[PATCH 0/3] XRA1403,gpio - add XRA1403 gpio expander driver

2017-03-27 Thread Nandor Han
The patchset will add a driver to support basic functionality for
XRA1403 device. Features supported:
- configure gpin as input/out
- get/set gpio status

Documentation: A gpio-xra1403.txt file was added to document the DTS
bindings related to driver.

Testing:

1. XRA1403 connected to iMX53 MCU
2. Export gpio from userspace
3. Verify that corresponding gpio directories are created
   in `/sys/class/gpio/gpioXX`
4. Export gpios from first and second bank as output
5. Set the output gpio pin to high/low and verify with the 
   oscilloscope that gpio status is according with the configured
   value.

Nandor Han (3):
  gpio - Add EXAR XRA1403 SPI GPIO expander driver
  doc,dts - add XRA1403 DTS binding documentation
  Add XRA1403 support to MAINTAINERS file

 .../devicetree/bindings/gpio/gpio-xra1403.txt  |  37 +++
 .../devicetree/bindings/vendor-prefixes.txt|   1 +
 MAINTAINERS|   8 +
 drivers/gpio/Kconfig   |   5 +
 drivers/gpio/Makefile  |   1 +
 drivers/gpio/gpio-xra1403.c| 252 +
 6 files changed, 304 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-xra1403.txt
 create mode 100644 drivers/gpio/gpio-xra1403.c

-- 
2.10.1



[PATCH 0/3] XRA1403,gpio - add XRA1403 gpio expander driver

2017-03-27 Thread Nandor Han
The patchset will add a driver to support basic functionality for
XRA1403 device. Features supported:
- configure gpin as input/out
- get/set gpio status

Documentation: A gpio-xra1403.txt file was added to document the DTS
bindings related to driver.

Testing:

1. XRA1403 connected to iMX53 MCU
2. Export gpio from userspace
3. Verify that corresponding gpio directories are created
   in `/sys/class/gpio/gpioXX`
4. Export gpios from first and second bank as output
5. Set the output gpio pin to high/low and verify with the 
   oscilloscope that gpio status is according with the configured
   value.

Nandor Han (3):
  gpio - Add EXAR XRA1403 SPI GPIO expander driver
  doc,dts - add XRA1403 DTS binding documentation
  Add XRA1403 support to MAINTAINERS file

 .../devicetree/bindings/gpio/gpio-xra1403.txt  |  37 +++
 .../devicetree/bindings/vendor-prefixes.txt|   1 +
 MAINTAINERS|   8 +
 drivers/gpio/Kconfig   |   5 +
 drivers/gpio/Makefile  |   1 +
 drivers/gpio/gpio-xra1403.c| 252 +
 6 files changed, 304 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-xra1403.txt
 create mode 100644 drivers/gpio/gpio-xra1403.c

-- 
2.10.1



[PATCH 3/3] Add XRA1403 support to MAINTAINERS file

2017-03-27 Thread Nandor Han
Add the XRA1403 support to MAINTAINERS list.

Signed-off-by: Nandor Han <nandor@ge.com>
---
 MAINTAINERS | 8 
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 58b3a22..539c88c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13903,6 +13903,14 @@ L: linux-kernel@vger.kernel.org
 S: Supported
 F: drivers/char/xillybus/
 
+XRA1403 GPIO EXPANDER
+M: Nandor Han <nandor@ge.com>
+M: Semi Malinen <semi.mali...@ge.com>
+L: linux-g...@vger.kernel.org
+S: Maintained
+F: drivers/gpio/gpio-xra1403.c
+F: Documentation/devicetree/bindings/gpio/gpio-xra1403.txt
+
 XTENSA XTFPGA PLATFORM SUPPORT
 M: Max Filippov <jcmvb...@gmail.com>
 L: linux-xte...@linux-xtensa.org
-- 
2.10.1



[PATCH 3/3] Add XRA1403 support to MAINTAINERS file

2017-03-27 Thread Nandor Han
Add the XRA1403 support to MAINTAINERS list.

Signed-off-by: Nandor Han 
---
 MAINTAINERS | 8 
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 58b3a22..539c88c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13903,6 +13903,14 @@ L: linux-kernel@vger.kernel.org
 S: Supported
 F: drivers/char/xillybus/
 
+XRA1403 GPIO EXPANDER
+M: Nandor Han 
+M: Semi Malinen 
+L: linux-g...@vger.kernel.org
+S: Maintained
+F: drivers/gpio/gpio-xra1403.c
+F: Documentation/devicetree/bindings/gpio/gpio-xra1403.txt
+
 XTENSA XTFPGA PLATFORM SUPPORT
 M: Max Filippov 
 L: linux-xte...@linux-xtensa.org
-- 
2.10.1



[PATCH 1/3] gpio - Add EXAR XRA1403 SPI GPIO expander driver

2017-03-27 Thread Nandor Han
This is a simple driver that provides a /sys/class/gpio
interface for controlling and configuring the GPIO lines.
It does not provide support for chip select or interrupts.

Signed-off-by: Nandor Han <nandor@ge.com>
Signed-off-by: Semi Malinen <semi.mali...@ge.com>
---
 .../devicetree/bindings/vendor-prefixes.txt|   1 +
 drivers/gpio/Kconfig   |   5 +
 drivers/gpio/Makefile  |   1 +
 drivers/gpio/gpio-xra1403.c| 252 +
 4 files changed, 259 insertions(+)
 create mode 100644 drivers/gpio/gpio-xra1403.c

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt 
b/Documentation/devicetree/bindings/vendor-prefixes.txt
index 0ad67d5..7ca9d41 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -103,6 +103,7 @@ ettus   NI Ettus Research
 eukrea  Eukréa Electromatique
 everestEverest Semiconductor Co. Ltd.
 everspin   Everspin Technologies, Inc.
+exar   Exar Corporation
 excito Excito
 ezchip EZchip Semiconductor
 faradayFaraday Technology Corporation
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index d6e3cfd..3a6c9a3 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1208,6 +1208,11 @@ config GPIO_PISOSR
  GPIO driver for SPI compatible parallel-in/serial-out shift
  registers. These are input only devices.
 
+config GPIO_XRA1403
+   tristate "EXAR XRA1403 16-bit GPIO expander"
+   help
+ GPIO driver for EXAR XRA1403 16-bit SPI-based GPIO expander.
+
 endmenu
 
 menu "SPI or I2C GPIO expanders"
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index bd995dc..8f50844 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -139,6 +139,7 @@ obj-$(CONFIG_GPIO_XGENE)+= gpio-xgene.o
 obj-$(CONFIG_GPIO_XGENE_SB)+= gpio-xgene-sb.o
 obj-$(CONFIG_GPIO_XILINX)  += gpio-xilinx.o
 obj-$(CONFIG_GPIO_XLP) += gpio-xlp.o
+obj-$(CONFIG_GPIO_XRA1403) += gpio-xra1403.o
 obj-$(CONFIG_GPIO_XTENSA)  += gpio-xtensa.o
 obj-$(CONFIG_GPIO_ZEVIO)   += gpio-zevio.o
 obj-$(CONFIG_GPIO_ZYNQ)+= gpio-zynq.o
diff --git a/drivers/gpio/gpio-xra1403.c b/drivers/gpio/gpio-xra1403.c
new file mode 100644
index 000..1b7138a8
--- /dev/null
+++ b/drivers/gpio/gpio-xra1403.c
@@ -0,0 +1,252 @@
+/*
+ * GPIO driver for EXAR XRA1403 16-bit GPIO expander
+ *
+ * Copyright (c) 2017, General Electric Company
+ *
+ * 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, see <http://www.gnu.org/licenses/>.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* XRA1403 registers */
+#define XRA_GSR  0x00 /* GPIO State */
+#define XRA_OCR  0x02 /* Output Control */
+#define XRA_GCR  0x06 /* GPIO Configuration */
+
+/* SPI headers */
+#define XRA_READ 0x80 /* read bit of the SPI command byte */
+
+struct xra1403 {
+   struct mutex  lock;
+   struct gpio_chip  chip;
+   struct spi_device *spi;
+};
+
+static int xra1403_get_byte(struct xra1403 *xra, unsigned int addr)
+{
+   return spi_w8r8(xra->spi, XRA_READ | (addr << 1));
+}
+
+static int xra1403_get_bit(struct xra1403 *xra, unsigned int addr,
+  unsigned int bit)
+{
+   int ret;
+
+   ret = xra1403_get_byte(xra, addr + (bit > 7));
+   if (ret < 0)
+   return ret;
+
+   return !!(ret & BIT(bit % 8));
+}
+
+static int xra1403_set_bit(struct xra1403 *xra, unsigned int addr,
+  unsigned int bit, int value)
+{
+   int ret;
+   u8 mask;
+   u8 tx[2];
+
+   addr += bit > 7;
+
+   mutex_lock(>lock);
+
+   ret = xra1403_get_byte(xra, addr);
+   if (ret < 0)
+   goto out_unlock;
+
+   mask = BIT(bit % 8);
+   if (value)
+   value = ret | mask;
+   else
+   value = ret & ~mask;
+
+   if (value != ret) {
+   tx[0] = addr << 1;
+   tx[1] = value;
+   ret = spi_write(xra->spi, tx, sizeof(tx));
+   } else {
+   ret = 0;
+   }
+
+out_unlock:
+   mutex_unlock(>lock);
+
+   return ret;
+}
+
+static int xra1403_direction_input(struct gpio_chip *chip, unsigned int offset)
+{
+   return xra1403_set_bit(gpiochip_get_data(chip

[PATCH 1/3] gpio - Add EXAR XRA1403 SPI GPIO expander driver

2017-03-27 Thread Nandor Han
This is a simple driver that provides a /sys/class/gpio
interface for controlling and configuring the GPIO lines.
It does not provide support for chip select or interrupts.

Signed-off-by: Nandor Han 
Signed-off-by: Semi Malinen 
---
 .../devicetree/bindings/vendor-prefixes.txt|   1 +
 drivers/gpio/Kconfig   |   5 +
 drivers/gpio/Makefile  |   1 +
 drivers/gpio/gpio-xra1403.c| 252 +
 4 files changed, 259 insertions(+)
 create mode 100644 drivers/gpio/gpio-xra1403.c

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt 
b/Documentation/devicetree/bindings/vendor-prefixes.txt
index 0ad67d5..7ca9d41 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -103,6 +103,7 @@ ettus   NI Ettus Research
 eukrea  Eukréa Electromatique
 everestEverest Semiconductor Co. Ltd.
 everspin   Everspin Technologies, Inc.
+exar   Exar Corporation
 excito Excito
 ezchip EZchip Semiconductor
 faradayFaraday Technology Corporation
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index d6e3cfd..3a6c9a3 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1208,6 +1208,11 @@ config GPIO_PISOSR
  GPIO driver for SPI compatible parallel-in/serial-out shift
  registers. These are input only devices.
 
+config GPIO_XRA1403
+   tristate "EXAR XRA1403 16-bit GPIO expander"
+   help
+ GPIO driver for EXAR XRA1403 16-bit SPI-based GPIO expander.
+
 endmenu
 
 menu "SPI or I2C GPIO expanders"
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index bd995dc..8f50844 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -139,6 +139,7 @@ obj-$(CONFIG_GPIO_XGENE)+= gpio-xgene.o
 obj-$(CONFIG_GPIO_XGENE_SB)+= gpio-xgene-sb.o
 obj-$(CONFIG_GPIO_XILINX)  += gpio-xilinx.o
 obj-$(CONFIG_GPIO_XLP) += gpio-xlp.o
+obj-$(CONFIG_GPIO_XRA1403) += gpio-xra1403.o
 obj-$(CONFIG_GPIO_XTENSA)  += gpio-xtensa.o
 obj-$(CONFIG_GPIO_ZEVIO)   += gpio-zevio.o
 obj-$(CONFIG_GPIO_ZYNQ)+= gpio-zynq.o
diff --git a/drivers/gpio/gpio-xra1403.c b/drivers/gpio/gpio-xra1403.c
new file mode 100644
index 000..1b7138a8
--- /dev/null
+++ b/drivers/gpio/gpio-xra1403.c
@@ -0,0 +1,252 @@
+/*
+ * GPIO driver for EXAR XRA1403 16-bit GPIO expander
+ *
+ * Copyright (c) 2017, General Electric Company
+ *
+ * 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, see <http://www.gnu.org/licenses/>.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* XRA1403 registers */
+#define XRA_GSR  0x00 /* GPIO State */
+#define XRA_OCR  0x02 /* Output Control */
+#define XRA_GCR  0x06 /* GPIO Configuration */
+
+/* SPI headers */
+#define XRA_READ 0x80 /* read bit of the SPI command byte */
+
+struct xra1403 {
+   struct mutex  lock;
+   struct gpio_chip  chip;
+   struct spi_device *spi;
+};
+
+static int xra1403_get_byte(struct xra1403 *xra, unsigned int addr)
+{
+   return spi_w8r8(xra->spi, XRA_READ | (addr << 1));
+}
+
+static int xra1403_get_bit(struct xra1403 *xra, unsigned int addr,
+  unsigned int bit)
+{
+   int ret;
+
+   ret = xra1403_get_byte(xra, addr + (bit > 7));
+   if (ret < 0)
+   return ret;
+
+   return !!(ret & BIT(bit % 8));
+}
+
+static int xra1403_set_bit(struct xra1403 *xra, unsigned int addr,
+  unsigned int bit, int value)
+{
+   int ret;
+   u8 mask;
+   u8 tx[2];
+
+   addr += bit > 7;
+
+   mutex_lock(>lock);
+
+   ret = xra1403_get_byte(xra, addr);
+   if (ret < 0)
+   goto out_unlock;
+
+   mask = BIT(bit % 8);
+   if (value)
+   value = ret | mask;
+   else
+   value = ret & ~mask;
+
+   if (value != ret) {
+   tx[0] = addr << 1;
+   tx[1] = value;
+   ret = spi_write(xra->spi, tx, sizeof(tx));
+   } else {
+   ret = 0;
+   }
+
+out_unlock:
+   mutex_unlock(>lock);
+
+   return ret;
+}
+
+static int xra1403_direction_input(struct gpio_chip *chip, unsigned int offset)
+{
+   return xra1403_set_bit(gpiochip_get_data(chip), XRA_GCR, offset, 1);
+}
+
+static int

[PATCH 1/1] dmaengine: imx-sdma - correct the dma transfer residue calculation

2016-10-11 Thread Nandor Han
The residue calculation was taking in consideration that dma
transaction status will be always retrieved in the dma callback
used to inform that dma transfer is complete. However this is not
the case for all subsystems that use dma. Some subsystems use a
timer to check the dma status periodically.

Therefore the calculation was updated and residue is calculated
accordingly by a) update the residue calculation taking in
consideration the last used buffer index by using *buf_ptail* variable
and b) chn_real_count (number of bytes transferred) is initialized to
zero, when dma channel is created, to avoid using an uninitialized
value in residue calculation when dma status is checked without
waiting dma complete event.

Signed-off-by: Nandor Han <nandor@ge.com>
---
 drivers/dma/imx-sdma.c | 13 +
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index b9629b2..d1651a5 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -298,6 +298,7 @@ struct sdma_engine;
  * @event_id1  for channels that use 2 events
  * @word_size  peripheral access size
  * @buf_tail   ID of the buffer that was processed
+ * @buf_ptail  ID of the previous buffer that was processed
  * @num_bd max NUM_BD. number of descriptors currently handling
  */
 struct sdma_channel {
@@ -309,6 +310,7 @@ struct sdma_channel {
unsigned intevent_id1;
enum dma_slave_buswidth word_size;
unsigned intbuf_tail;
+   unsigned intbuf_ptail;
unsigned intnum_bd;
unsigned intperiod_len;
struct sdma_buffer_descriptor   *bd;
@@ -700,6 +702,8 @@ static void sdma_update_channel_loop(struct sdma_channel 
*sdmac)
sdmac->chn_real_count = bd->mode.count;
bd->mode.status |= BD_DONE;
bd->mode.count = sdmac->period_len;
+   sdmac->buf_ptail = sdmac->buf_tail;
+   sdmac->buf_tail = (sdmac->buf_tail + 1) % sdmac->num_bd;
 
/*
 * The callback is called from the interrupt context in order
@@ -710,9 +714,6 @@ static void sdma_update_channel_loop(struct sdma_channel 
*sdmac)
 
dmaengine_desc_get_callback_invoke(>desc, NULL);
 
-   sdmac->buf_tail++;
-   sdmac->buf_tail %= sdmac->num_bd;
-
if (error)
sdmac->status = old_status;
}
@@ -1186,6 +1187,8 @@ static struct dma_async_tx_descriptor *sdma_prep_slave_sg(
sdmac->flags = 0;
 
sdmac->buf_tail = 0;
+   sdmac->buf_ptail = 0;
+   sdmac->chn_real_count = 0;
 
dev_dbg(sdma->dev, "setting up %d entries for channel %d.\n",
sg_len, channel);
@@ -1288,6 +1291,8 @@ static struct dma_async_tx_descriptor 
*sdma_prep_dma_cyclic(
sdmac->status = DMA_IN_PROGRESS;
 
sdmac->buf_tail = 0;
+   sdmac->buf_ptail = 0;
+   sdmac->chn_real_count = 0;
sdmac->period_len = period_len;
 
sdmac->flags |= IMX_DMA_SG_LOOP;
@@ -1385,7 +1390,7 @@ static enum dma_status sdma_tx_status(struct dma_chan 
*chan,
u32 residue;
 
if (sdmac->flags & IMX_DMA_SG_LOOP)
-   residue = (sdmac->num_bd - sdmac->buf_tail) *
+   residue = (sdmac->num_bd - sdmac->buf_ptail) *
   sdmac->period_len - sdmac->chn_real_count;
else
residue = sdmac->chn_count - sdmac->chn_real_count;
-- 
2.7.1



[PATCH 1/1] dmaengine: imx-sdma - correct the dma transfer residue calculation

2016-10-11 Thread Nandor Han
The residue calculation was taking in consideration that dma
transaction status will be always retrieved in the dma callback
used to inform that dma transfer is complete. However this is not
the case for all subsystems that use dma. Some subsystems use a
timer to check the dma status periodically.

Therefore the calculation was updated and residue is calculated
accordingly by a) update the residue calculation taking in
consideration the last used buffer index by using *buf_ptail* variable
and b) chn_real_count (number of bytes transferred) is initialized to
zero, when dma channel is created, to avoid using an uninitialized
value in residue calculation when dma status is checked without
waiting dma complete event.

Signed-off-by: Nandor Han 
---
 drivers/dma/imx-sdma.c | 13 +
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index b9629b2..d1651a5 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -298,6 +298,7 @@ struct sdma_engine;
  * @event_id1  for channels that use 2 events
  * @word_size  peripheral access size
  * @buf_tail   ID of the buffer that was processed
+ * @buf_ptail  ID of the previous buffer that was processed
  * @num_bd max NUM_BD. number of descriptors currently handling
  */
 struct sdma_channel {
@@ -309,6 +310,7 @@ struct sdma_channel {
unsigned intevent_id1;
enum dma_slave_buswidth word_size;
unsigned intbuf_tail;
+   unsigned intbuf_ptail;
unsigned intnum_bd;
unsigned intperiod_len;
struct sdma_buffer_descriptor   *bd;
@@ -700,6 +702,8 @@ static void sdma_update_channel_loop(struct sdma_channel 
*sdmac)
sdmac->chn_real_count = bd->mode.count;
bd->mode.status |= BD_DONE;
bd->mode.count = sdmac->period_len;
+   sdmac->buf_ptail = sdmac->buf_tail;
+   sdmac->buf_tail = (sdmac->buf_tail + 1) % sdmac->num_bd;
 
/*
 * The callback is called from the interrupt context in order
@@ -710,9 +714,6 @@ static void sdma_update_channel_loop(struct sdma_channel 
*sdmac)
 
dmaengine_desc_get_callback_invoke(>desc, NULL);
 
-   sdmac->buf_tail++;
-   sdmac->buf_tail %= sdmac->num_bd;
-
if (error)
sdmac->status = old_status;
}
@@ -1186,6 +1187,8 @@ static struct dma_async_tx_descriptor *sdma_prep_slave_sg(
sdmac->flags = 0;
 
sdmac->buf_tail = 0;
+   sdmac->buf_ptail = 0;
+   sdmac->chn_real_count = 0;
 
dev_dbg(sdma->dev, "setting up %d entries for channel %d.\n",
sg_len, channel);
@@ -1288,6 +1291,8 @@ static struct dma_async_tx_descriptor 
*sdma_prep_dma_cyclic(
sdmac->status = DMA_IN_PROGRESS;
 
sdmac->buf_tail = 0;
+   sdmac->buf_ptail = 0;
+   sdmac->chn_real_count = 0;
sdmac->period_len = period_len;
 
sdmac->flags |= IMX_DMA_SG_LOOP;
@@ -1385,7 +1390,7 @@ static enum dma_status sdma_tx_status(struct dma_chan 
*chan,
u32 residue;
 
if (sdmac->flags & IMX_DMA_SG_LOOP)
-   residue = (sdmac->num_bd - sdmac->buf_tail) *
+   residue = (sdmac->num_bd - sdmac->buf_ptail) *
   sdmac->period_len - sdmac->chn_real_count;
else
residue = sdmac->chn_count - sdmac->chn_real_count;
-- 
2.7.1



[PATCH 0/1] dmaengine: imx-sdma - fix the dma residue calculation

2016-10-11 Thread Nandor Han
The patch will correct the calculation of the dma residue taking in 
consideration
that some subsystem can check the dma status without waiting the dma complete 
event.

Affected devices: imx based device using sdma module with cyclic channels.

Tested on imx6 and imx53 by playing wav files using `speaker-test`application, 
part
of alsa-utils package, and verify that the sound plays smoothly without 
interruptions. 
Verified also the serial communication by transferring data over the serial 
line using
various packet sizes and checked the logs and the serial port status
(`cat /proc/tty/driver/IMX-uart`) that no data is lost.

Nandor Han (1):
  dmaengine: imx-sdma - correct the dma transfer residue calculation

 drivers/dma/imx-sdma.c | 13 +
 1 file changed, 9 insertions(+), 4 deletions(-)

-- 
2.7.1



[PATCH 0/1] dmaengine: imx-sdma - fix the dma residue calculation

2016-10-11 Thread Nandor Han
The patch will correct the calculation of the dma residue taking in 
consideration
that some subsystem can check the dma status without waiting the dma complete 
event.

Affected devices: imx based device using sdma module with cyclic channels.

Tested on imx6 and imx53 by playing wav files using `speaker-test`application, 
part
of alsa-utils package, and verify that the sound plays smoothly without 
interruptions. 
Verified also the serial communication by transferring data over the serial 
line using
various packet sizes and checked the logs and the serial port status
(`cat /proc/tty/driver/IMX-uart`) that no data is lost.

Nandor Han (1):
  dmaengine: imx-sdma - correct the dma transfer residue calculation

 drivers/dma/imx-sdma.c | 13 +
 1 file changed, 9 insertions(+), 4 deletions(-)

-- 
2.7.1



[PATCH 4/4] serial: imx-serial - update RX error counters when DMA is used

2016-08-08 Thread Nandor Han
Update error counters when DMA is used for receiving data. Do
this by using DMA transaction error event instead error interrupts
to reduce interrupt load.

Tested-by: Peter Senna Tschudin <peter.se...@collabora.com>
Acked-by: Peter Senna Tschudin <peter.se...@collabora.com>
Signed-off-by: Nandor Han <nandor@ge.com>
---
 drivers/tty/serial/imx.c | 32 
 1 file changed, 32 insertions(+)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 1912136..08ccfe1 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -704,6 +704,7 @@ out:
return IRQ_HANDLED;
 }
 
+static void clear_rx_errors(struct imx_port *sport);
 static int start_rx_dma(struct imx_port *sport);
 /*
  * If the RXFIFO is filled with some data, and then we
@@ -729,6 +730,11 @@ static void imx_dma_rxint(struct imx_port *sport)
temp &= ~(UCR2_ATEN);
writel(temp, sport->port.membase + UCR2);
 
+   /* disable the rx errors interrupts */
+   temp = readl(sport->port.membase + UCR4);
+   temp &= ~UCR4_OREN;
+   writel(temp, sport->port.membase + UCR4);
+
/* tell the DMA to receive the data. */
start_rx_dma(sport);
}
@@ -961,6 +967,7 @@ static void dma_rx_callback(void *data)
 
if (status == DMA_ERROR) {
dev_err(sport->port.dev, "DMA transaction error.\n");
+   clear_rx_errors(sport);
return;
}
 
@@ -1057,6 +1064,31 @@ static int start_rx_dma(struct imx_port *sport)
return 0;
 }
 
+static void clear_rx_errors(struct imx_port *sport)
+{
+   unsigned int status_usr1, status_usr2;
+
+   status_usr1 = readl(sport->port.membase + USR1);
+   status_usr2 = readl(sport->port.membase + USR2);
+
+   if (status_usr2 & USR2_BRCD) {
+   sport->port.icount.brk++;
+   writel(USR2_BRCD, sport->port.membase + USR2);
+   } else if (status_usr1 & USR1_FRAMERR) {
+   sport->port.icount.frame++;
+   writel(USR1_FRAMERR, sport->port.membase + USR1);
+   } else if (status_usr1 & USR1_PARITYERR) {
+   sport->port.icount.parity++;
+   writel(USR1_PARITYERR, sport->port.membase + USR1);
+   }
+
+   if (status_usr2 & USR2_ORE) {
+   sport->port.icount.overrun++;
+   writel(USR2_ORE, sport->port.membase + USR2);
+   }
+
+}
+
 #define TXTL_DEFAULT 2 /* reset default */
 #define RXTL_DEFAULT 1 /* reset default */
 #define TXTL_DMA 8 /* DMA burst setting */
-- 
2.8.3



[PATCH 4/4] serial: imx-serial - update RX error counters when DMA is used

2016-08-08 Thread Nandor Han
Update error counters when DMA is used for receiving data. Do
this by using DMA transaction error event instead error interrupts
to reduce interrupt load.

Tested-by: Peter Senna Tschudin 
Acked-by: Peter Senna Tschudin 
Signed-off-by: Nandor Han 
---
 drivers/tty/serial/imx.c | 32 
 1 file changed, 32 insertions(+)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 1912136..08ccfe1 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -704,6 +704,7 @@ out:
return IRQ_HANDLED;
 }
 
+static void clear_rx_errors(struct imx_port *sport);
 static int start_rx_dma(struct imx_port *sport);
 /*
  * If the RXFIFO is filled with some data, and then we
@@ -729,6 +730,11 @@ static void imx_dma_rxint(struct imx_port *sport)
temp &= ~(UCR2_ATEN);
writel(temp, sport->port.membase + UCR2);
 
+   /* disable the rx errors interrupts */
+   temp = readl(sport->port.membase + UCR4);
+   temp &= ~UCR4_OREN;
+   writel(temp, sport->port.membase + UCR4);
+
/* tell the DMA to receive the data. */
start_rx_dma(sport);
}
@@ -961,6 +967,7 @@ static void dma_rx_callback(void *data)
 
if (status == DMA_ERROR) {
dev_err(sport->port.dev, "DMA transaction error.\n");
+   clear_rx_errors(sport);
return;
}
 
@@ -1057,6 +1064,31 @@ static int start_rx_dma(struct imx_port *sport)
return 0;
 }
 
+static void clear_rx_errors(struct imx_port *sport)
+{
+   unsigned int status_usr1, status_usr2;
+
+   status_usr1 = readl(sport->port.membase + USR1);
+   status_usr2 = readl(sport->port.membase + USR2);
+
+   if (status_usr2 & USR2_BRCD) {
+   sport->port.icount.brk++;
+   writel(USR2_BRCD, sport->port.membase + USR2);
+   } else if (status_usr1 & USR1_FRAMERR) {
+   sport->port.icount.frame++;
+   writel(USR1_FRAMERR, sport->port.membase + USR1);
+   } else if (status_usr1 & USR1_PARITYERR) {
+   sport->port.icount.parity++;
+   writel(USR1_PARITYERR, sport->port.membase + USR1);
+   }
+
+   if (status_usr2 & USR2_ORE) {
+   sport->port.icount.overrun++;
+   writel(USR2_ORE, sport->port.membase + USR2);
+   }
+
+}
+
 #define TXTL_DEFAULT 2 /* reset default */
 #define RXTL_DEFAULT 1 /* reset default */
 #define TXTL_DMA 8 /* DMA burst setting */
-- 
2.8.3



[PATCH 2/4] dmaengine: imx-sdma - update the residue calculation for cyclic channels

2016-08-08 Thread Nandor Han
The calculation of the DMA transaction residue supports only fixed
size data transfers. This implementation is not covering all
operations (e.g. data receiving) when we need to know the exact amount
of bytes transferred.

The loop channels handling was changed to clear the buffer
descriptor errors and use the bd->mode.count to calculate the
residue.

Tested-by: Peter Senna Tschudin <peter.se...@collabora.com>
Acked-by: Peter Senna Tschudin <peter.se...@collabora.com>
Reviewed-by: Vinod Koul <vinod.k...@intel.com>
Signed-off-by: Nandor Han <nandor@ge.com>
---
 drivers/dma/imx-sdma.c | 20 ++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index aa35a77..3cb4738 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -651,6 +651,8 @@ static void sdma_event_disable(struct sdma_channel *sdmac, 
unsigned int event)
 static void sdma_update_channel_loop(struct sdma_channel *sdmac)
 {
struct sdma_buffer_descriptor *bd;
+   int error = 0;
+   enum dma_status old_status = sdmac->status;
 
/*
 * loop mode. Iterate over descriptors, re-setup them and
@@ -662,10 +664,20 @@ static void sdma_update_channel_loop(struct sdma_channel 
*sdmac)
if (bd->mode.status & BD_DONE)
break;
 
-   if (bd->mode.status & BD_RROR)
+   if (bd->mode.status & BD_RROR) {
+   bd->mode.status &= ~BD_RROR;
sdmac->status = DMA_ERROR;
+   error = -EIO;
+   }
 
+  /*
+   * We use bd->mode.count to calculate the residue, since contains
+   * the number of bytes present in the current buffer descriptor.
+   */
+
+   sdmac->chn_real_count = bd->mode.count;
bd->mode.status |= BD_DONE;
+   bd->mode.count = sdmac->period_len;
 
/*
 * The callback is called from the interrupt context in order
@@ -679,6 +691,9 @@ static void sdma_update_channel_loop(struct sdma_channel 
*sdmac)
 
sdmac->buf_tail++;
sdmac->buf_tail %= sdmac->num_bd;
+
+   if (error)
+   sdmac->status = old_status;
}
 }
 
@@ -1349,7 +1364,8 @@ static enum dma_status sdma_tx_status(struct dma_chan 
*chan,
u32 residue;
 
if (sdmac->flags & IMX_DMA_SG_LOOP)
-   residue = (sdmac->num_bd - sdmac->buf_tail) * sdmac->period_len;
+   residue = (sdmac->num_bd - sdmac->buf_tail) *
+  sdmac->period_len - sdmac->chn_real_count;
else
residue = sdmac->chn_count - sdmac->chn_real_count;
 
-- 
2.8.3



[PATCH 2/4] dmaengine: imx-sdma - update the residue calculation for cyclic channels

2016-08-08 Thread Nandor Han
The calculation of the DMA transaction residue supports only fixed
size data transfers. This implementation is not covering all
operations (e.g. data receiving) when we need to know the exact amount
of bytes transferred.

The loop channels handling was changed to clear the buffer
descriptor errors and use the bd->mode.count to calculate the
residue.

Tested-by: Peter Senna Tschudin 
Acked-by: Peter Senna Tschudin 
Reviewed-by: Vinod Koul 
Signed-off-by: Nandor Han 
---
 drivers/dma/imx-sdma.c | 20 ++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index aa35a77..3cb4738 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -651,6 +651,8 @@ static void sdma_event_disable(struct sdma_channel *sdmac, 
unsigned int event)
 static void sdma_update_channel_loop(struct sdma_channel *sdmac)
 {
struct sdma_buffer_descriptor *bd;
+   int error = 0;
+   enum dma_status old_status = sdmac->status;
 
/*
 * loop mode. Iterate over descriptors, re-setup them and
@@ -662,10 +664,20 @@ static void sdma_update_channel_loop(struct sdma_channel 
*sdmac)
if (bd->mode.status & BD_DONE)
break;
 
-   if (bd->mode.status & BD_RROR)
+   if (bd->mode.status & BD_RROR) {
+   bd->mode.status &= ~BD_RROR;
sdmac->status = DMA_ERROR;
+   error = -EIO;
+   }
 
+  /*
+   * We use bd->mode.count to calculate the residue, since contains
+   * the number of bytes present in the current buffer descriptor.
+   */
+
+   sdmac->chn_real_count = bd->mode.count;
bd->mode.status |= BD_DONE;
+   bd->mode.count = sdmac->period_len;
 
/*
 * The callback is called from the interrupt context in order
@@ -679,6 +691,9 @@ static void sdma_update_channel_loop(struct sdma_channel 
*sdmac)
 
sdmac->buf_tail++;
sdmac->buf_tail %= sdmac->num_bd;
+
+   if (error)
+   sdmac->status = old_status;
}
 }
 
@@ -1349,7 +1364,8 @@ static enum dma_status sdma_tx_status(struct dma_chan 
*chan,
u32 residue;
 
if (sdmac->flags & IMX_DMA_SG_LOOP)
-   residue = (sdmac->num_bd - sdmac->buf_tail) * sdmac->period_len;
+   residue = (sdmac->num_bd - sdmac->buf_tail) *
+  sdmac->period_len - sdmac->chn_real_count;
else
residue = sdmac->chn_count - sdmac->chn_real_count;
 
-- 
2.8.3



[PATCH 1/4] dmaengine: imx-sdma - reduce transfer latency for DMA cyclic clients

2016-08-08 Thread Nandor Han
Having the SDMA driver use a tasklet for running the clients
callback introduce some issues:
  - probability to have desynchronized data because of the
race condition created since the DMA transaction status
is retrieved only when the callback is executed, leaving
plenty of time for transaction status to get altered.
  - inter-transfer latency which can leave channels idle.

Move the callback execution, for cyclic channels, to SDMA
interrupt (as advised in `Documentation/dmaengine/provider.txt`)
to (a)reduce the inter-transfer latency and (b) eliminate the
race condition possibility where DMA transaction status might
be changed by the time is read.

The responsibility of the SDMA interrupt latency
is moved to the SDMA clients which case by case should defer
the work to bottom-halves when needed.

Tested-by: Peter Senna Tschudin <peter.se...@collabora.com>
Acked-by: Peter Senna Tschudin <peter.se...@collabora.com>
Reviewed-by: Vinod Koul <vinod.k...@intel.com>
Signed-off-by: Nandor Han <nandor@ge.com>
---
 drivers/dma/imx-sdma.c | 36 
 1 file changed, 16 insertions(+), 20 deletions(-)

diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 03ec76f..aa35a77 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -648,12 +648,6 @@ static void sdma_event_disable(struct sdma_channel *sdmac, 
unsigned int event)
writel_relaxed(val, sdma->regs + chnenbl);
 }
 
-static void sdma_handle_channel_loop(struct sdma_channel *sdmac)
-{
-   if (sdmac->desc.callback)
-   sdmac->desc.callback(sdmac->desc.callback_param);
-}
-
 static void sdma_update_channel_loop(struct sdma_channel *sdmac)
 {
struct sdma_buffer_descriptor *bd;
@@ -672,13 +666,25 @@ static void sdma_update_channel_loop(struct sdma_channel 
*sdmac)
sdmac->status = DMA_ERROR;
 
bd->mode.status |= BD_DONE;
+
+   /*
+* The callback is called from the interrupt context in order
+* to reduce latency and to avoid the risk of altering the
+* SDMA transaction status by the time the client tasklet is
+* executed.
+*/
+
+   if (sdmac->desc.callback)
+   sdmac->desc.callback(sdmac->desc.callback_param);
+
sdmac->buf_tail++;
sdmac->buf_tail %= sdmac->num_bd;
}
 }
 
-static void mxc_sdma_handle_channel_normal(struct sdma_channel *sdmac)
+static void mxc_sdma_handle_channel_normal(unsigned long data)
 {
+   struct sdma_channel *sdmac = (struct sdma_channel *) data;
struct sdma_buffer_descriptor *bd;
int i, error = 0;
 
@@ -705,16 +711,6 @@ static void mxc_sdma_handle_channel_normal(struct 
sdma_channel *sdmac)
sdmac->desc.callback(sdmac->desc.callback_param);
 }
 
-static void sdma_tasklet(unsigned long data)
-{
-   struct sdma_channel *sdmac = (struct sdma_channel *) data;
-
-   if (sdmac->flags & IMX_DMA_SG_LOOP)
-   sdma_handle_channel_loop(sdmac);
-   else
-   mxc_sdma_handle_channel_normal(sdmac);
-}
-
 static irqreturn_t sdma_int_handler(int irq, void *dev_id)
 {
struct sdma_engine *sdma = dev_id;
@@ -731,8 +727,8 @@ static irqreturn_t sdma_int_handler(int irq, void *dev_id)
 
if (sdmac->flags & IMX_DMA_SG_LOOP)
sdma_update_channel_loop(sdmac);
-
-   tasklet_schedule(>tasklet);
+   else
+   tasklet_schedule(>tasklet);
 
__clear_bit(channel, );
}
@@ -1732,7 +1728,7 @@ static int sdma_probe(struct platform_device *pdev)
dma_cookie_init(>chan);
sdmac->channel = i;
 
-   tasklet_init(>tasklet, sdma_tasklet,
+   tasklet_init(>tasklet, mxc_sdma_handle_channel_normal,
 (unsigned long) sdmac);
/*
 * Add the channel to the DMAC list. Do not add channel 0 though
-- 
2.8.3



[PATCH 1/4] dmaengine: imx-sdma - reduce transfer latency for DMA cyclic clients

2016-08-08 Thread Nandor Han
Having the SDMA driver use a tasklet for running the clients
callback introduce some issues:
  - probability to have desynchronized data because of the
race condition created since the DMA transaction status
is retrieved only when the callback is executed, leaving
plenty of time for transaction status to get altered.
  - inter-transfer latency which can leave channels idle.

Move the callback execution, for cyclic channels, to SDMA
interrupt (as advised in `Documentation/dmaengine/provider.txt`)
to (a)reduce the inter-transfer latency and (b) eliminate the
race condition possibility where DMA transaction status might
be changed by the time is read.

The responsibility of the SDMA interrupt latency
is moved to the SDMA clients which case by case should defer
the work to bottom-halves when needed.

Tested-by: Peter Senna Tschudin 
Acked-by: Peter Senna Tschudin 
Reviewed-by: Vinod Koul 
Signed-off-by: Nandor Han 
---
 drivers/dma/imx-sdma.c | 36 
 1 file changed, 16 insertions(+), 20 deletions(-)

diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 03ec76f..aa35a77 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -648,12 +648,6 @@ static void sdma_event_disable(struct sdma_channel *sdmac, 
unsigned int event)
writel_relaxed(val, sdma->regs + chnenbl);
 }
 
-static void sdma_handle_channel_loop(struct sdma_channel *sdmac)
-{
-   if (sdmac->desc.callback)
-   sdmac->desc.callback(sdmac->desc.callback_param);
-}
-
 static void sdma_update_channel_loop(struct sdma_channel *sdmac)
 {
struct sdma_buffer_descriptor *bd;
@@ -672,13 +666,25 @@ static void sdma_update_channel_loop(struct sdma_channel 
*sdmac)
sdmac->status = DMA_ERROR;
 
bd->mode.status |= BD_DONE;
+
+   /*
+* The callback is called from the interrupt context in order
+* to reduce latency and to avoid the risk of altering the
+* SDMA transaction status by the time the client tasklet is
+* executed.
+*/
+
+   if (sdmac->desc.callback)
+   sdmac->desc.callback(sdmac->desc.callback_param);
+
sdmac->buf_tail++;
sdmac->buf_tail %= sdmac->num_bd;
}
 }
 
-static void mxc_sdma_handle_channel_normal(struct sdma_channel *sdmac)
+static void mxc_sdma_handle_channel_normal(unsigned long data)
 {
+   struct sdma_channel *sdmac = (struct sdma_channel *) data;
struct sdma_buffer_descriptor *bd;
int i, error = 0;
 
@@ -705,16 +711,6 @@ static void mxc_sdma_handle_channel_normal(struct 
sdma_channel *sdmac)
sdmac->desc.callback(sdmac->desc.callback_param);
 }
 
-static void sdma_tasklet(unsigned long data)
-{
-   struct sdma_channel *sdmac = (struct sdma_channel *) data;
-
-   if (sdmac->flags & IMX_DMA_SG_LOOP)
-   sdma_handle_channel_loop(sdmac);
-   else
-   mxc_sdma_handle_channel_normal(sdmac);
-}
-
 static irqreturn_t sdma_int_handler(int irq, void *dev_id)
 {
struct sdma_engine *sdma = dev_id;
@@ -731,8 +727,8 @@ static irqreturn_t sdma_int_handler(int irq, void *dev_id)
 
if (sdmac->flags & IMX_DMA_SG_LOOP)
sdma_update_channel_loop(sdmac);
-
-   tasklet_schedule(>tasklet);
+   else
+   tasklet_schedule(>tasklet);
 
__clear_bit(channel, );
}
@@ -1732,7 +1728,7 @@ static int sdma_probe(struct platform_device *pdev)
dma_cookie_init(>chan);
sdmac->channel = i;
 
-   tasklet_init(>tasklet, sdma_tasklet,
+   tasklet_init(>tasklet, mxc_sdma_handle_channel_normal,
 (unsigned long) sdmac);
/*
 * Add the channel to the DMAC list. Do not add channel 0 though
-- 
2.8.3



[PATCH 0/4] serial,dmaengine: use DMA cyclic for IMX UART driver

2016-08-08 Thread Nandor Han
Update the IMX UART driver to use cyclic DMA in order
to avoid losing serial data because of DMA start latency.

Support variable data length transfers for cyclic channels in
IMX SDMA(Smart DMA) driver and, as specified in 
"Documentation/dmaengine/provider.txt" section "General Design Notes",
run the SDMA channel callback directly from the SDMA interrupt context.

Every commit message will explain the changes.

Impacted devices:
  IMX6i,IMX53 CPU:
- clients of the IMX UART that have DMA enabled.
- cyclic SDMA clients 
(Note:Not able to find others than IMX UART driver)

Functionality tested by connecting the m53evk to my laptop using a
USB-serial converter and start sending, from my laptop 800 bytes
long packets at 8ms interval.

On the m53evk with interrupt load, generated by
connecting/disconnecting at 3s interval a USB HUB having
some devices connected (~4), I receive serial data in a
loop and check that no overruns are created using command:

`watch -n1 cat /proc/tty/driver/IMX-uart`

Also done tests were I check how the data is received  when sending packets
bigger than a buffer descriptor size (1024). In my tests I have sent 4 packets
(1028, 2048, 2048, 2048) and check that 7172 bytes were received using
the command:
`watch -n1 cat /proc/tty/driver/IMX-uart`

Result:
serinfo:1.0 driver revision:
2: uart:IMX mmio:0x5000C000 irq:49 tx:0 rx:7172 RTS|CTS|DTR|DSR|CD


Nandor Han (4):
  dmaengine: imx-sdma - reduce transfer latency for DMA cyclic clients
  dmaengine: imx-sdma - update the residue calculation for cyclic
channels
  serial: imx-serial - update UART IMX driver to use cyclic DMA
  serial: imx-serial - update RX error counters when DMA is used

 drivers/dma/imx-sdma.c   |  56 +--
 drivers/tty/serial/imx.c | 173 ++-
 2 files changed, 144 insertions(+), 85 deletions(-)

-- 
2.8.3



[PATCH 0/4] serial,dmaengine: use DMA cyclic for IMX UART driver

2016-08-08 Thread Nandor Han
Update the IMX UART driver to use cyclic DMA in order
to avoid losing serial data because of DMA start latency.

Support variable data length transfers for cyclic channels in
IMX SDMA(Smart DMA) driver and, as specified in 
"Documentation/dmaengine/provider.txt" section "General Design Notes",
run the SDMA channel callback directly from the SDMA interrupt context.

Every commit message will explain the changes.

Impacted devices:
  IMX6i,IMX53 CPU:
- clients of the IMX UART that have DMA enabled.
- cyclic SDMA clients 
(Note:Not able to find others than IMX UART driver)

Functionality tested by connecting the m53evk to my laptop using a
USB-serial converter and start sending, from my laptop 800 bytes
long packets at 8ms interval.

On the m53evk with interrupt load, generated by
connecting/disconnecting at 3s interval a USB HUB having
some devices connected (~4), I receive serial data in a
loop and check that no overruns are created using command:

`watch -n1 cat /proc/tty/driver/IMX-uart`

Also done tests were I check how the data is received  when sending packets
bigger than a buffer descriptor size (1024). In my tests I have sent 4 packets
(1028, 2048, 2048, 2048) and check that 7172 bytes were received using
the command:
`watch -n1 cat /proc/tty/driver/IMX-uart`

Result:
serinfo:1.0 driver revision:
2: uart:IMX mmio:0x5000C000 irq:49 tx:0 rx:7172 RTS|CTS|DTR|DSR|CD


Nandor Han (4):
  dmaengine: imx-sdma - reduce transfer latency for DMA cyclic clients
  dmaengine: imx-sdma - update the residue calculation for cyclic
channels
  serial: imx-serial - update UART IMX driver to use cyclic DMA
  serial: imx-serial - update RX error counters when DMA is used

 drivers/dma/imx-sdma.c   |  56 +--
 drivers/tty/serial/imx.c | 173 ++-
 2 files changed, 144 insertions(+), 85 deletions(-)

-- 
2.8.3



[PATCH 3/4] serial: imx-serial - update UART IMX driver to use cyclic DMA

2016-08-08 Thread Nandor Han
The IMX UART has a 32 bytes HW buffer which can be filled up in
2777us at 115200 baud or 80us at 4Mbaud (supported by IMX53).
Taking this in consideration there is a good probability to lose
data because of the DMA startup latency.
Our tests (explained below) indicates a latency up to 4400us when
creating interrupt load and ~70us without. When creating interrupt
load I was able to see continuous overrun errors by checking serial
driver statistics using the command:
`cat /proc/tty/driver/IMX-uart`.

Replace manual restart of DMA with cyclic DMA to eliminate data loss
due to DMA engine startup latency (similar approch to atmel_serial.c
driver). As result the DMA engine will start on the first serial data
transfer and stops only when serial port is closed.

Tests environment:
 Using the m53evk board I have used a GPIO for profiling the IMX
 serial driver.
  - The RX line and GPIO were connected to oscilloscope.
  - Run a small test program on the m53evk board that will only open
and read data from ttymxc2 port.
  - Connect the ttymxc2 port to my laptop using a USB serial converter
where another test program is running, able to send configurable
packet lengths and intervals.
  - Serial ports configured at 115200 8N1.
  - Interrupts load created by disconnecting/connecting (3s interval)
a USB hub, using a digital switch, with 4 USB devices (USB-Serial
converter, USB SD card, etc) connected.
(around 160 interrupts/second generated)
  - The GPIO was toggled HI in the `imx_int` when USR1_RRDY or USR1_AGTIM
events are received and toggled back, once the DMA configuration
is finalized, at the end of `imx_dma_rxint`.

Measurements:
The measurements were done from the end of the last byte (RX line) until
the GPIO was toggled back LOW.

Note: The GPIO toggling was done using `gpiod_set_value` method.

Tests performed:
   1. Sending 9 bytes packets at 8ms interval. Having the 9 bytes packets
  will activate the RRDY threshold event and IMX serial interrupt
  called.
  Results:
- DMA start latency (interrupt start latency +
   DMA configuration) consistently 70us when system not loaded.
- DMA start latency up to 4400us when system loaded.
   2. Sending 40 bytes packet at 8mS interval.
  Results with load:
- Able to observe overruns by running:
   `watch -n1 cat /proc/tty/driver/IMX-uart`

Tested-by: Peter Senna Tschudin <peter.se...@collabora.com>
Acked-by: Peter Senna Tschudin <peter.se...@collabora.com>
Signed-off-by: Nandor Han <nandor@ge.com>
---
 drivers/tty/serial/imx.c | 141 ++-
 1 file changed, 78 insertions(+), 63 deletions(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 0df2b1c..1912136 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -222,6 +222,9 @@ struct imx_port {
struct dma_chan *dma_chan_rx, *dma_chan_tx;
struct scatterlist  rx_sgl, tx_sgl[2];
void*rx_buf;
+   struct circ_buf rx_ring;
+   unsigned intrx_periods;
+   dma_cookie_trx_cookie;
unsigned inttx_bytes;
unsigned intdma_tx_nents;
wait_queue_head_t   dma_wait;
@@ -932,30 +935,6 @@ static void imx_timeout(unsigned long data)
 }
 
 #define RX_BUF_SIZE(PAGE_SIZE)
-static void imx_rx_dma_done(struct imx_port *sport)
-{
-   unsigned long temp;
-   unsigned long flags;
-
-   spin_lock_irqsave(>port.lock, flags);
-
-   /* re-enable interrupts to get notified when new symbols are incoming */
-   temp = readl(sport->port.membase + UCR1);
-   temp |= UCR1_RRDYEN;
-   writel(temp, sport->port.membase + UCR1);
-
-   temp = readl(sport->port.membase + UCR2);
-   temp |= UCR2_ATEN;
-   writel(temp, sport->port.membase + UCR2);
-
-   sport->dma_is_rxing = 0;
-
-   /* Is the shutdown waiting for us? */
-   if (waitqueue_active(>dma_wait))
-   wake_up(>dma_wait);
-
-   spin_unlock_irqrestore(>port.lock, flags);
-}
 
 /*
  * There are two kinds of RX DMA interrupts(such as in the MX6Q):
@@ -972,43 +951,75 @@ static void dma_rx_callback(void *data)
struct scatterlist *sgl = >rx_sgl;
struct tty_port *port = >port.state->port;
struct dma_tx_state state;
+   struct circ_buf *rx_ring = >rx_ring;
enum dma_status status;
-   unsigned int count;
-
-   /* unmap it first */
-   dma_unmap_sg(sport->port.dev, sgl, 1, DMA_FROM_DEVICE);
+   unsigned int w_bytes = 0;
+   unsigned int r_bytes;
+   unsigned int bd_size;
 
status = dmaengine_tx_status(chan, (dma_cookie_t)0, );
-   count = RX_BUF_SIZE - state.residue;
 
-   dev_dbg(sport->port.dev, "We get %d bytes.\n", count);
+   if (status == DMA_ERROR) {
+   dev

[PATCH 3/4] serial: imx-serial - update UART IMX driver to use cyclic DMA

2016-08-08 Thread Nandor Han
The IMX UART has a 32 bytes HW buffer which can be filled up in
2777us at 115200 baud or 80us at 4Mbaud (supported by IMX53).
Taking this in consideration there is a good probability to lose
data because of the DMA startup latency.
Our tests (explained below) indicates a latency up to 4400us when
creating interrupt load and ~70us without. When creating interrupt
load I was able to see continuous overrun errors by checking serial
driver statistics using the command:
`cat /proc/tty/driver/IMX-uart`.

Replace manual restart of DMA with cyclic DMA to eliminate data loss
due to DMA engine startup latency (similar approch to atmel_serial.c
driver). As result the DMA engine will start on the first serial data
transfer and stops only when serial port is closed.

Tests environment:
 Using the m53evk board I have used a GPIO for profiling the IMX
 serial driver.
  - The RX line and GPIO were connected to oscilloscope.
  - Run a small test program on the m53evk board that will only open
and read data from ttymxc2 port.
  - Connect the ttymxc2 port to my laptop using a USB serial converter
where another test program is running, able to send configurable
packet lengths and intervals.
  - Serial ports configured at 115200 8N1.
  - Interrupts load created by disconnecting/connecting (3s interval)
a USB hub, using a digital switch, with 4 USB devices (USB-Serial
converter, USB SD card, etc) connected.
(around 160 interrupts/second generated)
  - The GPIO was toggled HI in the `imx_int` when USR1_RRDY or USR1_AGTIM
events are received and toggled back, once the DMA configuration
is finalized, at the end of `imx_dma_rxint`.

Measurements:
The measurements were done from the end of the last byte (RX line) until
the GPIO was toggled back LOW.

Note: The GPIO toggling was done using `gpiod_set_value` method.

Tests performed:
   1. Sending 9 bytes packets at 8ms interval. Having the 9 bytes packets
  will activate the RRDY threshold event and IMX serial interrupt
  called.
  Results:
- DMA start latency (interrupt start latency +
   DMA configuration) consistently 70us when system not loaded.
- DMA start latency up to 4400us when system loaded.
   2. Sending 40 bytes packet at 8mS interval.
  Results with load:
- Able to observe overruns by running:
   `watch -n1 cat /proc/tty/driver/IMX-uart`

Tested-by: Peter Senna Tschudin 
Acked-by: Peter Senna Tschudin 
Signed-off-by: Nandor Han 
---
 drivers/tty/serial/imx.c | 141 ++-
 1 file changed, 78 insertions(+), 63 deletions(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 0df2b1c..1912136 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -222,6 +222,9 @@ struct imx_port {
struct dma_chan *dma_chan_rx, *dma_chan_tx;
struct scatterlist  rx_sgl, tx_sgl[2];
void*rx_buf;
+   struct circ_buf rx_ring;
+   unsigned intrx_periods;
+   dma_cookie_trx_cookie;
unsigned inttx_bytes;
unsigned intdma_tx_nents;
wait_queue_head_t   dma_wait;
@@ -932,30 +935,6 @@ static void imx_timeout(unsigned long data)
 }
 
 #define RX_BUF_SIZE(PAGE_SIZE)
-static void imx_rx_dma_done(struct imx_port *sport)
-{
-   unsigned long temp;
-   unsigned long flags;
-
-   spin_lock_irqsave(>port.lock, flags);
-
-   /* re-enable interrupts to get notified when new symbols are incoming */
-   temp = readl(sport->port.membase + UCR1);
-   temp |= UCR1_RRDYEN;
-   writel(temp, sport->port.membase + UCR1);
-
-   temp = readl(sport->port.membase + UCR2);
-   temp |= UCR2_ATEN;
-   writel(temp, sport->port.membase + UCR2);
-
-   sport->dma_is_rxing = 0;
-
-   /* Is the shutdown waiting for us? */
-   if (waitqueue_active(>dma_wait))
-   wake_up(>dma_wait);
-
-   spin_unlock_irqrestore(>port.lock, flags);
-}
 
 /*
  * There are two kinds of RX DMA interrupts(such as in the MX6Q):
@@ -972,43 +951,75 @@ static void dma_rx_callback(void *data)
struct scatterlist *sgl = >rx_sgl;
struct tty_port *port = >port.state->port;
struct dma_tx_state state;
+   struct circ_buf *rx_ring = >rx_ring;
enum dma_status status;
-   unsigned int count;
-
-   /* unmap it first */
-   dma_unmap_sg(sport->port.dev, sgl, 1, DMA_FROM_DEVICE);
+   unsigned int w_bytes = 0;
+   unsigned int r_bytes;
+   unsigned int bd_size;
 
status = dmaengine_tx_status(chan, (dma_cookie_t)0, );
-   count = RX_BUF_SIZE - state.residue;
 
-   dev_dbg(sport->port.dev, "We get %d bytes.\n", count);
+   if (status == DMA_ERROR) {
+   dev_err(sport->port.dev, "DMA transaction error.\n");
+   return;
+ 

Re: EXT: Re: [PATCH RFC 1/4] dma: imx-sdma - reduce transfer latency for DMA cyclic clients

2016-07-01 Thread Nandor Han



On 28/06/16 17:34, Vinod Koul wrote:

On Thu, Jun 09, 2016 at 03:16:30PM +0300, Nandor Han wrote:

Having the SDMA driver use a tasklet for running the clients
callback introduce some issues:
   - probability to have desynchronized data because of the
 race condition created since the DMA transaction status
 is retrieved only when the callback is executed, leaving
 plenty of time for transaction status to get altered.
   - inter-transfer latency which can leave channels idle.

Move the callback execution, for cyclic channels, to SDMA
interrupt (as advised in `Documentation/dmaengine/provider.txt`)
to (a)reduce the inter-transfer latency and (b) eliminate the
race condition possibility where DMA transaction status might
be changed by the time is read.

The responsibility of the SDMA interrupt latency
is moved to the SDMA clients which case by case should defer
the work to bottom-halves when needed.


Both of these look fine. Please change the patch titles to dmaengine: 

Are these going to be merged thru dmaengine tree or serial one?



I will send soon a V2 where I will fix the titles. If you are OK with 
all the patchset it can be merged to dmaengine tree, otherwise probably 
goes to serial one.


Let me know which is the best option.

Thanks,
   Nandor



Signed-off-by: Nandor Han <nandor@ge.com>
---
  drivers/dma/imx-sdma.c | 36 
  1 file changed, 16 insertions(+), 20 deletions(-)

diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 0f6fd42..e497847 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -654,12 +654,6 @@ static void sdma_event_disable(struct sdma_channel *sdmac, 
unsigned int event)
writel_relaxed(val, sdma->regs + chnenbl);
  }

-static void sdma_handle_channel_loop(struct sdma_channel *sdmac)
-{
-   if (sdmac->desc.callback)
-   sdmac->desc.callback(sdmac->desc.callback_param);
-}
-
  static void sdma_update_channel_loop(struct sdma_channel *sdmac)
  {
struct sdma_buffer_descriptor *bd;
@@ -678,13 +672,25 @@ static void sdma_update_channel_loop(struct sdma_channel 
*sdmac)
sdmac->status = DMA_ERROR;

bd->mode.status |= BD_DONE;
+
+   /*
+* The callback is called from the interrupt context in order
+* to reduce latency and to avoid the risk of altering the
+* SDMA transaction status by the time the client tasklet is
+* executed.
+*/
+
+   if (sdmac->desc.callback)
+   sdmac->desc.callback(sdmac->desc.callback_param);
+
sdmac->buf_tail++;
sdmac->buf_tail %= sdmac->num_bd;
}
  }

-static void mxc_sdma_handle_channel_normal(struct sdma_channel *sdmac)
+static void mxc_sdma_handle_channel_normal(unsigned long data)
  {
+   struct sdma_channel *sdmac = (struct sdma_channel *) data;
struct sdma_buffer_descriptor *bd;
int i, error = 0;

@@ -711,16 +717,6 @@ static void mxc_sdma_handle_channel_normal(struct 
sdma_channel *sdmac)
sdmac->desc.callback(sdmac->desc.callback_param);
  }

-static void sdma_tasklet(unsigned long data)
-{
-   struct sdma_channel *sdmac = (struct sdma_channel *) data;
-
-   if (sdmac->flags & IMX_DMA_SG_LOOP)
-   sdma_handle_channel_loop(sdmac);
-   else
-   mxc_sdma_handle_channel_normal(sdmac);
-}
-
  static irqreturn_t sdma_int_handler(int irq, void *dev_id)
  {
struct sdma_engine *sdma = dev_id;
@@ -737,8 +733,8 @@ static irqreturn_t sdma_int_handler(int irq, void *dev_id)

if (sdmac->flags & IMX_DMA_SG_LOOP)
sdma_update_channel_loop(sdmac);
-
-   tasklet_schedule(>tasklet);
+   else
+   tasklet_schedule(>tasklet);

__clear_bit(channel, );
}
@@ -1739,7 +1735,7 @@ static int sdma_probe(struct platform_device *pdev)
dma_cookie_init(>chan);
sdmac->channel = i;

-   tasklet_init(>tasklet, sdma_tasklet,
+   tasklet_init(>tasklet, mxc_sdma_handle_channel_normal,
 (unsigned long) sdmac);
/*
 * Add the channel to the DMAC list. Do not add channel 0 though
--
2.8.3





Re: EXT: Re: [PATCH RFC 1/4] dma: imx-sdma - reduce transfer latency for DMA cyclic clients

2016-07-01 Thread Nandor Han



On 28/06/16 17:34, Vinod Koul wrote:

On Thu, Jun 09, 2016 at 03:16:30PM +0300, Nandor Han wrote:

Having the SDMA driver use a tasklet for running the clients
callback introduce some issues:
   - probability to have desynchronized data because of the
 race condition created since the DMA transaction status
 is retrieved only when the callback is executed, leaving
 plenty of time for transaction status to get altered.
   - inter-transfer latency which can leave channels idle.

Move the callback execution, for cyclic channels, to SDMA
interrupt (as advised in `Documentation/dmaengine/provider.txt`)
to (a)reduce the inter-transfer latency and (b) eliminate the
race condition possibility where DMA transaction status might
be changed by the time is read.

The responsibility of the SDMA interrupt latency
is moved to the SDMA clients which case by case should defer
the work to bottom-halves when needed.


Both of these look fine. Please change the patch titles to dmaengine: 

Are these going to be merged thru dmaengine tree or serial one?



I will send soon a V2 where I will fix the titles. If you are OK with 
all the patchset it can be merged to dmaengine tree, otherwise probably 
goes to serial one.


Let me know which is the best option.

Thanks,
   Nandor



Signed-off-by: Nandor Han 
---
  drivers/dma/imx-sdma.c | 36 
  1 file changed, 16 insertions(+), 20 deletions(-)

diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 0f6fd42..e497847 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -654,12 +654,6 @@ static void sdma_event_disable(struct sdma_channel *sdmac, 
unsigned int event)
writel_relaxed(val, sdma->regs + chnenbl);
  }

-static void sdma_handle_channel_loop(struct sdma_channel *sdmac)
-{
-   if (sdmac->desc.callback)
-   sdmac->desc.callback(sdmac->desc.callback_param);
-}
-
  static void sdma_update_channel_loop(struct sdma_channel *sdmac)
  {
struct sdma_buffer_descriptor *bd;
@@ -678,13 +672,25 @@ static void sdma_update_channel_loop(struct sdma_channel 
*sdmac)
sdmac->status = DMA_ERROR;

bd->mode.status |= BD_DONE;
+
+   /*
+* The callback is called from the interrupt context in order
+* to reduce latency and to avoid the risk of altering the
+* SDMA transaction status by the time the client tasklet is
+* executed.
+*/
+
+   if (sdmac->desc.callback)
+   sdmac->desc.callback(sdmac->desc.callback_param);
+
sdmac->buf_tail++;
sdmac->buf_tail %= sdmac->num_bd;
}
  }

-static void mxc_sdma_handle_channel_normal(struct sdma_channel *sdmac)
+static void mxc_sdma_handle_channel_normal(unsigned long data)
  {
+   struct sdma_channel *sdmac = (struct sdma_channel *) data;
struct sdma_buffer_descriptor *bd;
int i, error = 0;

@@ -711,16 +717,6 @@ static void mxc_sdma_handle_channel_normal(struct 
sdma_channel *sdmac)
sdmac->desc.callback(sdmac->desc.callback_param);
  }

-static void sdma_tasklet(unsigned long data)
-{
-   struct sdma_channel *sdmac = (struct sdma_channel *) data;
-
-   if (sdmac->flags & IMX_DMA_SG_LOOP)
-   sdma_handle_channel_loop(sdmac);
-   else
-   mxc_sdma_handle_channel_normal(sdmac);
-}
-
  static irqreturn_t sdma_int_handler(int irq, void *dev_id)
  {
struct sdma_engine *sdma = dev_id;
@@ -737,8 +733,8 @@ static irqreturn_t sdma_int_handler(int irq, void *dev_id)

if (sdmac->flags & IMX_DMA_SG_LOOP)
sdma_update_channel_loop(sdmac);
-
-   tasklet_schedule(>tasklet);
+   else
+   tasklet_schedule(>tasklet);

__clear_bit(channel, );
}
@@ -1739,7 +1735,7 @@ static int sdma_probe(struct platform_device *pdev)
dma_cookie_init(>chan);
sdmac->channel = i;

-   tasklet_init(>tasklet, sdma_tasklet,
+   tasklet_init(>tasklet, mxc_sdma_handle_channel_normal,
 (unsigned long) sdmac);
/*
 * Add the channel to the DMAC list. Do not add channel 0 though
--
2.8.3





[PATCH RFC 1/4] dma: imx-sdma - reduce transfer latency for DMA cyclic clients

2016-06-09 Thread Nandor Han
Having the SDMA driver use a tasklet for running the clients
callback introduce some issues:
  - probability to have desynchronized data because of the
race condition created since the DMA transaction status
is retrieved only when the callback is executed, leaving
plenty of time for transaction status to get altered.
  - inter-transfer latency which can leave channels idle.

Move the callback execution, for cyclic channels, to SDMA
interrupt (as advised in `Documentation/dmaengine/provider.txt`)
to (a)reduce the inter-transfer latency and (b) eliminate the
race condition possibility where DMA transaction status might
be changed by the time is read.

The responsibility of the SDMA interrupt latency
is moved to the SDMA clients which case by case should defer
the work to bottom-halves when needed.

Signed-off-by: Nandor Han <nandor@ge.com>
---
 drivers/dma/imx-sdma.c | 36 
 1 file changed, 16 insertions(+), 20 deletions(-)

diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 0f6fd42..e497847 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -654,12 +654,6 @@ static void sdma_event_disable(struct sdma_channel *sdmac, 
unsigned int event)
writel_relaxed(val, sdma->regs + chnenbl);
 }
 
-static void sdma_handle_channel_loop(struct sdma_channel *sdmac)
-{
-   if (sdmac->desc.callback)
-   sdmac->desc.callback(sdmac->desc.callback_param);
-}
-
 static void sdma_update_channel_loop(struct sdma_channel *sdmac)
 {
struct sdma_buffer_descriptor *bd;
@@ -678,13 +672,25 @@ static void sdma_update_channel_loop(struct sdma_channel 
*sdmac)
sdmac->status = DMA_ERROR;
 
bd->mode.status |= BD_DONE;
+
+   /*
+* The callback is called from the interrupt context in order
+* to reduce latency and to avoid the risk of altering the
+* SDMA transaction status by the time the client tasklet is
+* executed.
+*/
+
+   if (sdmac->desc.callback)
+   sdmac->desc.callback(sdmac->desc.callback_param);
+
sdmac->buf_tail++;
sdmac->buf_tail %= sdmac->num_bd;
}
 }
 
-static void mxc_sdma_handle_channel_normal(struct sdma_channel *sdmac)
+static void mxc_sdma_handle_channel_normal(unsigned long data)
 {
+   struct sdma_channel *sdmac = (struct sdma_channel *) data;
struct sdma_buffer_descriptor *bd;
int i, error = 0;
 
@@ -711,16 +717,6 @@ static void mxc_sdma_handle_channel_normal(struct 
sdma_channel *sdmac)
sdmac->desc.callback(sdmac->desc.callback_param);
 }
 
-static void sdma_tasklet(unsigned long data)
-{
-   struct sdma_channel *sdmac = (struct sdma_channel *) data;
-
-   if (sdmac->flags & IMX_DMA_SG_LOOP)
-   sdma_handle_channel_loop(sdmac);
-   else
-   mxc_sdma_handle_channel_normal(sdmac);
-}
-
 static irqreturn_t sdma_int_handler(int irq, void *dev_id)
 {
struct sdma_engine *sdma = dev_id;
@@ -737,8 +733,8 @@ static irqreturn_t sdma_int_handler(int irq, void *dev_id)
 
if (sdmac->flags & IMX_DMA_SG_LOOP)
sdma_update_channel_loop(sdmac);
-
-   tasklet_schedule(>tasklet);
+   else
+   tasklet_schedule(>tasklet);
 
__clear_bit(channel, );
}
@@ -1739,7 +1735,7 @@ static int sdma_probe(struct platform_device *pdev)
dma_cookie_init(>chan);
sdmac->channel = i;
 
-   tasklet_init(>tasklet, sdma_tasklet,
+   tasklet_init(>tasklet, mxc_sdma_handle_channel_normal,
 (unsigned long) sdmac);
/*
 * Add the channel to the DMAC list. Do not add channel 0 though
-- 
2.8.3



[PATCH RFC 1/4] dma: imx-sdma - reduce transfer latency for DMA cyclic clients

2016-06-09 Thread Nandor Han
Having the SDMA driver use a tasklet for running the clients
callback introduce some issues:
  - probability to have desynchronized data because of the
race condition created since the DMA transaction status
is retrieved only when the callback is executed, leaving
plenty of time for transaction status to get altered.
  - inter-transfer latency which can leave channels idle.

Move the callback execution, for cyclic channels, to SDMA
interrupt (as advised in `Documentation/dmaengine/provider.txt`)
to (a)reduce the inter-transfer latency and (b) eliminate the
race condition possibility where DMA transaction status might
be changed by the time is read.

The responsibility of the SDMA interrupt latency
is moved to the SDMA clients which case by case should defer
the work to bottom-halves when needed.

Signed-off-by: Nandor Han 
---
 drivers/dma/imx-sdma.c | 36 
 1 file changed, 16 insertions(+), 20 deletions(-)

diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 0f6fd42..e497847 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -654,12 +654,6 @@ static void sdma_event_disable(struct sdma_channel *sdmac, 
unsigned int event)
writel_relaxed(val, sdma->regs + chnenbl);
 }
 
-static void sdma_handle_channel_loop(struct sdma_channel *sdmac)
-{
-   if (sdmac->desc.callback)
-   sdmac->desc.callback(sdmac->desc.callback_param);
-}
-
 static void sdma_update_channel_loop(struct sdma_channel *sdmac)
 {
struct sdma_buffer_descriptor *bd;
@@ -678,13 +672,25 @@ static void sdma_update_channel_loop(struct sdma_channel 
*sdmac)
sdmac->status = DMA_ERROR;
 
bd->mode.status |= BD_DONE;
+
+   /*
+* The callback is called from the interrupt context in order
+* to reduce latency and to avoid the risk of altering the
+* SDMA transaction status by the time the client tasklet is
+* executed.
+*/
+
+   if (sdmac->desc.callback)
+   sdmac->desc.callback(sdmac->desc.callback_param);
+
sdmac->buf_tail++;
sdmac->buf_tail %= sdmac->num_bd;
}
 }
 
-static void mxc_sdma_handle_channel_normal(struct sdma_channel *sdmac)
+static void mxc_sdma_handle_channel_normal(unsigned long data)
 {
+   struct sdma_channel *sdmac = (struct sdma_channel *) data;
struct sdma_buffer_descriptor *bd;
int i, error = 0;
 
@@ -711,16 +717,6 @@ static void mxc_sdma_handle_channel_normal(struct 
sdma_channel *sdmac)
sdmac->desc.callback(sdmac->desc.callback_param);
 }
 
-static void sdma_tasklet(unsigned long data)
-{
-   struct sdma_channel *sdmac = (struct sdma_channel *) data;
-
-   if (sdmac->flags & IMX_DMA_SG_LOOP)
-   sdma_handle_channel_loop(sdmac);
-   else
-   mxc_sdma_handle_channel_normal(sdmac);
-}
-
 static irqreturn_t sdma_int_handler(int irq, void *dev_id)
 {
struct sdma_engine *sdma = dev_id;
@@ -737,8 +733,8 @@ static irqreturn_t sdma_int_handler(int irq, void *dev_id)
 
if (sdmac->flags & IMX_DMA_SG_LOOP)
sdma_update_channel_loop(sdmac);
-
-   tasklet_schedule(>tasklet);
+   else
+   tasklet_schedule(>tasklet);
 
__clear_bit(channel, );
}
@@ -1739,7 +1735,7 @@ static int sdma_probe(struct platform_device *pdev)
dma_cookie_init(>chan);
sdmac->channel = i;
 
-   tasklet_init(>tasklet, sdma_tasklet,
+   tasklet_init(>tasklet, mxc_sdma_handle_channel_normal,
 (unsigned long) sdmac);
/*
 * Add the channel to the DMAC list. Do not add channel 0 though
-- 
2.8.3



[PATCH RFC 0/4] serial,dma: use DMA cyclic for IMX UART driver

2016-06-09 Thread Nandor Han
Update the IMX UART driver to use cyclic DMA in order
to avoid losing serial data because of DMA start latency.

Support variable data length transfers for cyclic channels in
IMX SDMA(Smart DMA) driver and, as specified in 
"Documentation/dmaengine/provider.txt" section "General Design Notes",
run the SDMA channel callback directly from the SDMA interrupt context.

Every commit message will explain the changes.

Impacted devices:
  IMX6i,IMX53 CPU:
- clients of the IMX UART that have DMA enabled.
- cyclic SDMA clients 
(Note:Not able to find others than IMX UART driver)

Functionality tested by connecting the m53evk to my laptop using a
USB-serial converter and start sending, from my laptop 800 bytes
long packets at 8ms interval.

On the m53evk with interrupt load, generated by
connecting/disconnecting at 3s interval a USB HUB having
some devices connected (~4), I receive serial data in a
loop and check that no overruns are created using command:

`watch -n1 cat /proc/tty/driver/IMX-uart`

Also done tests were I check how the data is received  when sending packets
bigger than a buffer descriptor size (1024). In my tests I have sent 4 packets
(1028, 2048, 2048, 2048) and check that 7172 bytes were received using
the command:
`watch -n1 cat /proc/tty/driver/IMX-uart`

Result:
serinfo:1.0 driver revision:
2: uart:IMX mmio:0x5000C000 irq:49 tx:0 rx:7172 RTS|CTS|DTR|DSR|CD


Nandor Han (4):
  dma: imx-sdma - reduce transfer latency for DMA cyclic clients
  dma: imx-sdma - update the residue calculation for cyclic channels
  serial: imx-serial - update UART IMX driver to use cyclic DMA
  serial: imx-serial - update RX error counters when DMA is used

 drivers/dma/imx-sdma.c   |  56 +--
 drivers/tty/serial/imx.c | 173 ++-
 2 files changed, 144 insertions(+), 85 deletions(-)

-- 
2.8.3



[PATCH RFC 0/4] serial,dma: use DMA cyclic for IMX UART driver

2016-06-09 Thread Nandor Han
Update the IMX UART driver to use cyclic DMA in order
to avoid losing serial data because of DMA start latency.

Support variable data length transfers for cyclic channels in
IMX SDMA(Smart DMA) driver and, as specified in 
"Documentation/dmaengine/provider.txt" section "General Design Notes",
run the SDMA channel callback directly from the SDMA interrupt context.

Every commit message will explain the changes.

Impacted devices:
  IMX6i,IMX53 CPU:
- clients of the IMX UART that have DMA enabled.
- cyclic SDMA clients 
(Note:Not able to find others than IMX UART driver)

Functionality tested by connecting the m53evk to my laptop using a
USB-serial converter and start sending, from my laptop 800 bytes
long packets at 8ms interval.

On the m53evk with interrupt load, generated by
connecting/disconnecting at 3s interval a USB HUB having
some devices connected (~4), I receive serial data in a
loop and check that no overruns are created using command:

`watch -n1 cat /proc/tty/driver/IMX-uart`

Also done tests were I check how the data is received  when sending packets
bigger than a buffer descriptor size (1024). In my tests I have sent 4 packets
(1028, 2048, 2048, 2048) and check that 7172 bytes were received using
the command:
`watch -n1 cat /proc/tty/driver/IMX-uart`

Result:
serinfo:1.0 driver revision:
2: uart:IMX mmio:0x5000C000 irq:49 tx:0 rx:7172 RTS|CTS|DTR|DSR|CD


Nandor Han (4):
  dma: imx-sdma - reduce transfer latency for DMA cyclic clients
  dma: imx-sdma - update the residue calculation for cyclic channels
  serial: imx-serial - update UART IMX driver to use cyclic DMA
  serial: imx-serial - update RX error counters when DMA is used

 drivers/dma/imx-sdma.c   |  56 +--
 drivers/tty/serial/imx.c | 173 ++-
 2 files changed, 144 insertions(+), 85 deletions(-)

-- 
2.8.3



[PATCH RFC 2/4] dma: imx-sdma - update the residue calculation for cyclic channels

2016-06-09 Thread Nandor Han
The calculation of the DMA transaction residue supports only fixed
size data transfers. This implementation is not covering all
operations (e.g. data receiving) when we need to know the exact amount
of bytes transferred.

The loop channels handling was changed to clear the buffer
descriptor errors and use the bd->mode.count to calculate the
residue.

Signed-off-by: Nandor Han <nandor@ge.com>
---
 drivers/dma/imx-sdma.c | 20 ++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index e497847..9da258a 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -657,6 +657,8 @@ static void sdma_event_disable(struct sdma_channel *sdmac, 
unsigned int event)
 static void sdma_update_channel_loop(struct sdma_channel *sdmac)
 {
struct sdma_buffer_descriptor *bd;
+   int error = 0;
+   enum dma_status old_status = sdmac->status;
 
/*
 * loop mode. Iterate over descriptors, re-setup them and
@@ -668,10 +670,20 @@ static void sdma_update_channel_loop(struct sdma_channel 
*sdmac)
if (bd->mode.status & BD_DONE)
break;
 
-   if (bd->mode.status & BD_RROR)
+   if (bd->mode.status & BD_RROR) {
+   bd->mode.status &= ~BD_RROR;
sdmac->status = DMA_ERROR;
+   error = -EIO;
+   }
 
+  /*
+   * We use bd->mode.count to calculate the residue, since contains
+   * the number of bytes present in the current buffer descriptor.
+   */
+
+   sdmac->chn_real_count = bd->mode.count;
bd->mode.status |= BD_DONE;
+   bd->mode.count = sdmac->period_len;
 
/*
 * The callback is called from the interrupt context in order
@@ -685,6 +697,9 @@ static void sdma_update_channel_loop(struct sdma_channel 
*sdmac)
 
sdmac->buf_tail++;
sdmac->buf_tail %= sdmac->num_bd;
+
+   if (error)
+   sdmac->status = old_status;
}
 }
 
@@ -1358,7 +1373,8 @@ static enum dma_status sdma_tx_status(struct dma_chan 
*chan,
u32 residue;
 
if (sdmac->flags & IMX_DMA_SG_LOOP)
-   residue = (sdmac->num_bd - sdmac->buf_tail) * sdmac->period_len;
+   residue = (sdmac->num_bd - sdmac->buf_tail) *
+  sdmac->period_len - sdmac->chn_real_count;
else
residue = sdmac->chn_count - sdmac->chn_real_count;
 
-- 
2.8.3



[PATCH RFC 2/4] dma: imx-sdma - update the residue calculation for cyclic channels

2016-06-09 Thread Nandor Han
The calculation of the DMA transaction residue supports only fixed
size data transfers. This implementation is not covering all
operations (e.g. data receiving) when we need to know the exact amount
of bytes transferred.

The loop channels handling was changed to clear the buffer
descriptor errors and use the bd->mode.count to calculate the
residue.

Signed-off-by: Nandor Han 
---
 drivers/dma/imx-sdma.c | 20 ++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index e497847..9da258a 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -657,6 +657,8 @@ static void sdma_event_disable(struct sdma_channel *sdmac, 
unsigned int event)
 static void sdma_update_channel_loop(struct sdma_channel *sdmac)
 {
struct sdma_buffer_descriptor *bd;
+   int error = 0;
+   enum dma_status old_status = sdmac->status;
 
/*
 * loop mode. Iterate over descriptors, re-setup them and
@@ -668,10 +670,20 @@ static void sdma_update_channel_loop(struct sdma_channel 
*sdmac)
if (bd->mode.status & BD_DONE)
break;
 
-   if (bd->mode.status & BD_RROR)
+   if (bd->mode.status & BD_RROR) {
+   bd->mode.status &= ~BD_RROR;
sdmac->status = DMA_ERROR;
+   error = -EIO;
+   }
 
+  /*
+   * We use bd->mode.count to calculate the residue, since contains
+   * the number of bytes present in the current buffer descriptor.
+   */
+
+   sdmac->chn_real_count = bd->mode.count;
bd->mode.status |= BD_DONE;
+   bd->mode.count = sdmac->period_len;
 
/*
 * The callback is called from the interrupt context in order
@@ -685,6 +697,9 @@ static void sdma_update_channel_loop(struct sdma_channel 
*sdmac)
 
sdmac->buf_tail++;
sdmac->buf_tail %= sdmac->num_bd;
+
+   if (error)
+   sdmac->status = old_status;
}
 }
 
@@ -1358,7 +1373,8 @@ static enum dma_status sdma_tx_status(struct dma_chan 
*chan,
u32 residue;
 
if (sdmac->flags & IMX_DMA_SG_LOOP)
-   residue = (sdmac->num_bd - sdmac->buf_tail) * sdmac->period_len;
+   residue = (sdmac->num_bd - sdmac->buf_tail) *
+  sdmac->period_len - sdmac->chn_real_count;
else
residue = sdmac->chn_count - sdmac->chn_real_count;
 
-- 
2.8.3



[PATCH RFC 4/4] serial: imx-serial - update RX error counters when DMA is used

2016-06-09 Thread Nandor Han
Update error couters when DMA is used for receiving data. Do
this by using DMA transaction error event instead error interrupts
to reduce interrupt load.

Signed-off-by: Nandor Han <nandor@ge.com>
---
 drivers/tty/serial/imx.c | 32 
 1 file changed, 32 insertions(+)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 1912136..08ccfe1 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -704,6 +704,7 @@ out:
return IRQ_HANDLED;
 }
 
+static void clear_rx_errors(struct imx_port *sport);
 static int start_rx_dma(struct imx_port *sport);
 /*
  * If the RXFIFO is filled with some data, and then we
@@ -729,6 +730,11 @@ static void imx_dma_rxint(struct imx_port *sport)
temp &= ~(UCR2_ATEN);
writel(temp, sport->port.membase + UCR2);
 
+   /* disable the rx errors interrupts */
+   temp = readl(sport->port.membase + UCR4);
+   temp &= ~UCR4_OREN;
+   writel(temp, sport->port.membase + UCR4);
+
/* tell the DMA to receive the data. */
start_rx_dma(sport);
}
@@ -961,6 +967,7 @@ static void dma_rx_callback(void *data)
 
if (status == DMA_ERROR) {
dev_err(sport->port.dev, "DMA transaction error.\n");
+   clear_rx_errors(sport);
return;
}
 
@@ -1057,6 +1064,31 @@ static int start_rx_dma(struct imx_port *sport)
return 0;
 }
 
+static void clear_rx_errors(struct imx_port *sport)
+{
+   unsigned int status_usr1, status_usr2;
+
+   status_usr1 = readl(sport->port.membase + USR1);
+   status_usr2 = readl(sport->port.membase + USR2);
+
+   if (status_usr2 & USR2_BRCD) {
+   sport->port.icount.brk++;
+   writel(USR2_BRCD, sport->port.membase + USR2);
+   } else if (status_usr1 & USR1_FRAMERR) {
+   sport->port.icount.frame++;
+   writel(USR1_FRAMERR, sport->port.membase + USR1);
+   } else if (status_usr1 & USR1_PARITYERR) {
+   sport->port.icount.parity++;
+   writel(USR1_PARITYERR, sport->port.membase + USR1);
+   }
+
+   if (status_usr2 & USR2_ORE) {
+   sport->port.icount.overrun++;
+   writel(USR2_ORE, sport->port.membase + USR2);
+   }
+
+}
+
 #define TXTL_DEFAULT 2 /* reset default */
 #define RXTL_DEFAULT 1 /* reset default */
 #define TXTL_DMA 8 /* DMA burst setting */
-- 
2.8.3



[PATCH RFC 4/4] serial: imx-serial - update RX error counters when DMA is used

2016-06-09 Thread Nandor Han
Update error couters when DMA is used for receiving data. Do
this by using DMA transaction error event instead error interrupts
to reduce interrupt load.

Signed-off-by: Nandor Han 
---
 drivers/tty/serial/imx.c | 32 
 1 file changed, 32 insertions(+)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 1912136..08ccfe1 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -704,6 +704,7 @@ out:
return IRQ_HANDLED;
 }
 
+static void clear_rx_errors(struct imx_port *sport);
 static int start_rx_dma(struct imx_port *sport);
 /*
  * If the RXFIFO is filled with some data, and then we
@@ -729,6 +730,11 @@ static void imx_dma_rxint(struct imx_port *sport)
temp &= ~(UCR2_ATEN);
writel(temp, sport->port.membase + UCR2);
 
+   /* disable the rx errors interrupts */
+   temp = readl(sport->port.membase + UCR4);
+   temp &= ~UCR4_OREN;
+   writel(temp, sport->port.membase + UCR4);
+
/* tell the DMA to receive the data. */
start_rx_dma(sport);
}
@@ -961,6 +967,7 @@ static void dma_rx_callback(void *data)
 
if (status == DMA_ERROR) {
dev_err(sport->port.dev, "DMA transaction error.\n");
+   clear_rx_errors(sport);
return;
}
 
@@ -1057,6 +1064,31 @@ static int start_rx_dma(struct imx_port *sport)
return 0;
 }
 
+static void clear_rx_errors(struct imx_port *sport)
+{
+   unsigned int status_usr1, status_usr2;
+
+   status_usr1 = readl(sport->port.membase + USR1);
+   status_usr2 = readl(sport->port.membase + USR2);
+
+   if (status_usr2 & USR2_BRCD) {
+   sport->port.icount.brk++;
+   writel(USR2_BRCD, sport->port.membase + USR2);
+   } else if (status_usr1 & USR1_FRAMERR) {
+   sport->port.icount.frame++;
+   writel(USR1_FRAMERR, sport->port.membase + USR1);
+   } else if (status_usr1 & USR1_PARITYERR) {
+   sport->port.icount.parity++;
+   writel(USR1_PARITYERR, sport->port.membase + USR1);
+   }
+
+   if (status_usr2 & USR2_ORE) {
+   sport->port.icount.overrun++;
+   writel(USR2_ORE, sport->port.membase + USR2);
+   }
+
+}
+
 #define TXTL_DEFAULT 2 /* reset default */
 #define RXTL_DEFAULT 1 /* reset default */
 #define TXTL_DMA 8 /* DMA burst setting */
-- 
2.8.3



[PATCH RFC 3/4] serial: imx-serial - update UART IMX driver to use cyclic DMA

2016-06-09 Thread Nandor Han
The IMX UART has a 32 bytes HW buffer which can be filled up in
2777us at 115200 baud or 80us at 4Mbaud (supported by IMX53).
Taking this in consideration there is a good probability to lose
data because of the DMA startup latency.
Our tests (explained below) indicates a latency up to 4400us when
creating interrupt load and ~70us without. When creating interrupt
load I was able to see continuous overrun errors by checking serial
driver statistics using the command:
`cat /proc/tty/driver/IMX-uart`.

Replace manual restart of DMA with cyclic DMA to eliminate data loss
due to DMA engine startup latency (similar approch to atmel_serial.c
driver). As result the DMA engine will start on the first serial data
transfer and stops only when serial port is closed.

Tests environment:
 Using the m53evk board I have used a GPIO for profiling the IMX
 serial driver.
  - The RX line and GPIO were connected to oscilloscope.
  - Run a small test program on the m53evk board that will only open
and read data from ttymxc2 port.
  - Connect the ttymxc2 port to my laptop using a USB serial converter
where another test program is running, able to send configurable
packet lengths and intervals.
  - Serial ports configured at 115200 8N1.
  - Interrupts load created by disconnecting/connecting (3s interval)
a USB hub, using a digital switch, with 4 USB devices (USB-Serial
converter, USB SD card, etc) connected.
(around 160 interrupts/second generated)
  - The GPIO was toggled HI in the `imx_int` when USR1_RRDY or USR1_AGTIM
events are received and toggled back, once the DMA configuration
is finalized, at the end of `imx_dma_rxint`.

Measurements:
The measurements were done from the end of the last byte (RX line) until
the GPIO was toggled back LOW.

Note: The GPIO toggling was done using `gpiod_set_value` method.

Tests performed:
   1. Sending 9 bytes packets at 8ms interval. Having the 9 bytes packets
  will activate the RRDY threshold event and IMX serial interrupt
  called.
  Results:
- DMA start latency (interrupt start latency +
   DMA configuration) consistently 70us when system not loaded.
- DMA start latency up to 4400us when system loaded.
   2. Sending 40 bytes packet at 8mS interval.
  Results with load:
- Able to observe overruns by running:
   `watch -n1 cat /proc/tty/driver/IMX-uart`

Signed-off-by: Nandor Han <nandor@ge.com>
---
 drivers/tty/serial/imx.c | 141 ++-
 1 file changed, 78 insertions(+), 63 deletions(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 0df2b1c..1912136 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -222,6 +222,9 @@ struct imx_port {
struct dma_chan *dma_chan_rx, *dma_chan_tx;
struct scatterlist  rx_sgl, tx_sgl[2];
void*rx_buf;
+   struct circ_buf rx_ring;
+   unsigned intrx_periods;
+   dma_cookie_trx_cookie;
unsigned inttx_bytes;
unsigned intdma_tx_nents;
wait_queue_head_t   dma_wait;
@@ -932,30 +935,6 @@ static void imx_timeout(unsigned long data)
 }
 
 #define RX_BUF_SIZE(PAGE_SIZE)
-static void imx_rx_dma_done(struct imx_port *sport)
-{
-   unsigned long temp;
-   unsigned long flags;
-
-   spin_lock_irqsave(>port.lock, flags);
-
-   /* re-enable interrupts to get notified when new symbols are incoming */
-   temp = readl(sport->port.membase + UCR1);
-   temp |= UCR1_RRDYEN;
-   writel(temp, sport->port.membase + UCR1);
-
-   temp = readl(sport->port.membase + UCR2);
-   temp |= UCR2_ATEN;
-   writel(temp, sport->port.membase + UCR2);
-
-   sport->dma_is_rxing = 0;
-
-   /* Is the shutdown waiting for us? */
-   if (waitqueue_active(>dma_wait))
-   wake_up(>dma_wait);
-
-   spin_unlock_irqrestore(>port.lock, flags);
-}
 
 /*
  * There are two kinds of RX DMA interrupts(such as in the MX6Q):
@@ -972,43 +951,75 @@ static void dma_rx_callback(void *data)
struct scatterlist *sgl = >rx_sgl;
struct tty_port *port = >port.state->port;
struct dma_tx_state state;
+   struct circ_buf *rx_ring = >rx_ring;
enum dma_status status;
-   unsigned int count;
-
-   /* unmap it first */
-   dma_unmap_sg(sport->port.dev, sgl, 1, DMA_FROM_DEVICE);
+   unsigned int w_bytes = 0;
+   unsigned int r_bytes;
+   unsigned int bd_size;
 
status = dmaengine_tx_status(chan, (dma_cookie_t)0, );
-   count = RX_BUF_SIZE - state.residue;
 
-   dev_dbg(sport->port.dev, "We get %d bytes.\n", count);
+   if (status == DMA_ERROR) {
+   dev_err(sport->port.dev, "DMA transaction error.\n");
+   return;

[PATCH RFC 3/4] serial: imx-serial - update UART IMX driver to use cyclic DMA

2016-06-09 Thread Nandor Han
The IMX UART has a 32 bytes HW buffer which can be filled up in
2777us at 115200 baud or 80us at 4Mbaud (supported by IMX53).
Taking this in consideration there is a good probability to lose
data because of the DMA startup latency.
Our tests (explained below) indicates a latency up to 4400us when
creating interrupt load and ~70us without. When creating interrupt
load I was able to see continuous overrun errors by checking serial
driver statistics using the command:
`cat /proc/tty/driver/IMX-uart`.

Replace manual restart of DMA with cyclic DMA to eliminate data loss
due to DMA engine startup latency (similar approch to atmel_serial.c
driver). As result the DMA engine will start on the first serial data
transfer and stops only when serial port is closed.

Tests environment:
 Using the m53evk board I have used a GPIO for profiling the IMX
 serial driver.
  - The RX line and GPIO were connected to oscilloscope.
  - Run a small test program on the m53evk board that will only open
and read data from ttymxc2 port.
  - Connect the ttymxc2 port to my laptop using a USB serial converter
where another test program is running, able to send configurable
packet lengths and intervals.
  - Serial ports configured at 115200 8N1.
  - Interrupts load created by disconnecting/connecting (3s interval)
a USB hub, using a digital switch, with 4 USB devices (USB-Serial
converter, USB SD card, etc) connected.
(around 160 interrupts/second generated)
  - The GPIO was toggled HI in the `imx_int` when USR1_RRDY or USR1_AGTIM
events are received and toggled back, once the DMA configuration
is finalized, at the end of `imx_dma_rxint`.

Measurements:
The measurements were done from the end of the last byte (RX line) until
the GPIO was toggled back LOW.

Note: The GPIO toggling was done using `gpiod_set_value` method.

Tests performed:
   1. Sending 9 bytes packets at 8ms interval. Having the 9 bytes packets
  will activate the RRDY threshold event and IMX serial interrupt
  called.
  Results:
- DMA start latency (interrupt start latency +
   DMA configuration) consistently 70us when system not loaded.
- DMA start latency up to 4400us when system loaded.
   2. Sending 40 bytes packet at 8mS interval.
  Results with load:
- Able to observe overruns by running:
   `watch -n1 cat /proc/tty/driver/IMX-uart`

Signed-off-by: Nandor Han 
---
 drivers/tty/serial/imx.c | 141 ++-
 1 file changed, 78 insertions(+), 63 deletions(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 0df2b1c..1912136 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -222,6 +222,9 @@ struct imx_port {
struct dma_chan *dma_chan_rx, *dma_chan_tx;
struct scatterlist  rx_sgl, tx_sgl[2];
void*rx_buf;
+   struct circ_buf rx_ring;
+   unsigned intrx_periods;
+   dma_cookie_trx_cookie;
unsigned inttx_bytes;
unsigned intdma_tx_nents;
wait_queue_head_t   dma_wait;
@@ -932,30 +935,6 @@ static void imx_timeout(unsigned long data)
 }
 
 #define RX_BUF_SIZE(PAGE_SIZE)
-static void imx_rx_dma_done(struct imx_port *sport)
-{
-   unsigned long temp;
-   unsigned long flags;
-
-   spin_lock_irqsave(>port.lock, flags);
-
-   /* re-enable interrupts to get notified when new symbols are incoming */
-   temp = readl(sport->port.membase + UCR1);
-   temp |= UCR1_RRDYEN;
-   writel(temp, sport->port.membase + UCR1);
-
-   temp = readl(sport->port.membase + UCR2);
-   temp |= UCR2_ATEN;
-   writel(temp, sport->port.membase + UCR2);
-
-   sport->dma_is_rxing = 0;
-
-   /* Is the shutdown waiting for us? */
-   if (waitqueue_active(>dma_wait))
-   wake_up(>dma_wait);
-
-   spin_unlock_irqrestore(>port.lock, flags);
-}
 
 /*
  * There are two kinds of RX DMA interrupts(such as in the MX6Q):
@@ -972,43 +951,75 @@ static void dma_rx_callback(void *data)
struct scatterlist *sgl = >rx_sgl;
struct tty_port *port = >port.state->port;
struct dma_tx_state state;
+   struct circ_buf *rx_ring = >rx_ring;
enum dma_status status;
-   unsigned int count;
-
-   /* unmap it first */
-   dma_unmap_sg(sport->port.dev, sgl, 1, DMA_FROM_DEVICE);
+   unsigned int w_bytes = 0;
+   unsigned int r_bytes;
+   unsigned int bd_size;
 
status = dmaengine_tx_status(chan, (dma_cookie_t)0, );
-   count = RX_BUF_SIZE - state.residue;
 
-   dev_dbg(sport->port.dev, "We get %d bytes.\n", count);
+   if (status == DMA_ERROR) {
+   dev_err(sport->port.dev, "DMA transaction error.\n");
+   return;
+   }
+
+   if (!(sport->port.ignore_status_mask & UR