RE: Re: [V3 PATCH 1/2] x86/panic: Replace smp_send_stop() with kdump friendly version

[河合英宏 / KAWAI，HIDEHIRO]

Hi,

> From: 'Dave Young' [mailto:dyo...@redhat.com]
> Sent: Monday, July 18, 2016 6:02 PM
> On 07/15/16 at 11:50am, 河合英宏 / KAWAI，HIDEHIRO wrote:
> > Hi Dave,
> >
> > Thanks for your reply.
> >
> > > From: 'Dave Young' [mailto:dyo...@redhat.com]
> > > Sent: Wednesday, July 13, 2016 11:04 AM
> > >
> > > On 07/12/16 at 02:49am, 河合英宏 / KAWAI，HIDEHIRO wrote:
> > > > Hi Dave,
> > > >
> > > > Thanks for the comments.
> > > >
> > > > > From: Dave Young [mailto:dyo...@redhat.com]
> > > > > Sent: Monday, July 11, 2016 5:35 PM
> > > > >
> > > > > On 07/05/16 at 08:33pm, Hidehiro Kawai wrote:
[snip] 
> > > > > As for this patch I'm not sure it is safe to replace the
> > > > > smp_send_stop with the kdump friendly function. I'm also not sure if
> > > > > the kdump friendly function is safe for kdump. Will glad to hear
> > > > > opinions from other arch experts.
> > > >
> > > > This stuff depends on architectures, so I speak only about
> > > > x86 (the logic doesn't change on other architectures at this time).
> > > >
> > > > kdump path with crash_kexec_post_notifiers disabled:
> > > >  panic()
> > > >__crash_kexec()
> > > >  crash_setup_regs()
> > > >  crash_save_vmcoreinfo()
> > > >  machine_crash_shutdown()
> > > >native_machine_crash_shutdown()
> > > >  panic_smp_send_stop() /* mostly same as original
> > > > * kdump_nmi_shootdown_cpus()
> > > > */
> > > >
> > > > kdump path with crash_kexec_post_notifiers enabled:
> > > >  panic()
> > > >panic_smp_send_stop()
> > > >__crash_kexec()
> > > >  crash_setup_regs()
> > > >  crash_save_vmcoreinfo()
> > > >  machine_crash_shutdown()
> > > >native_machine_crash_shutdown()
> > > >  panic_smp_send_stop() // do nothing
> > > >
> > > > The difference is that stopping other CPUs before crash_setup_regs()
> > > > and crash_save_vmcoreinfo() or not.  Since crash_setup_regs() and
> > > > crash_save_vmcoreinfo() just save information to some memory area,
> > > > they wouldn't be affected by panic_smp_send_stop().  This means
> > > > placing panic_smp_send_stop before __crash_kexec is safe.
> > > >
> > > > BTW, I noticed my patch breaks Xen kernel.  I'll fix it in the next
> > > > version.
> > >
> > > But it does breaks stuff which depends on cpu not being disabled like 
> > > problem 1 you mentioned in patch log.
> >
> > As I mentioned in the description of this patch, we should stop
> > other CPUs ASAP to preserve current state either
> > crash_kexec_post_notifiers is enabled or not.
> > Then, all remaining procedures should work well
> > after stopping other CPUs (but keep the CPU map online).
> >
> > Vivek also mentioned similar things:
> > https://lkml.org/lkml/2015/7/14/433
> 
> The implementation in this patchset is different from suggestion in above 
> link?
> 
> I think Vivek's suggestion is a good idea, to drop smp_send_stop and do below:
> 
> stop_cpus_save_register_state;
> 
> if (!crash_kexec_post_notifiers)
>   crash_kexec()
> atomic_notifier_call_chain()
> kmsg_dump()
> 
> I'm just commenting from code flow point of view, the detail implementation
> definitely need more comments from Arch experts.
> 
> Any reason did not move the kdump friendly function to earlier point like
> before previous __crash_kexec() below?
> if (!crash_kexec_post_notifiers) {
> printk_nmi_flush_on_panic();
> __crash_kexec(NULL);
> }

The reason why the implementation differs from Vivek's is to keep
the current code flow if crash_kexec_post_notifiers is not specified.

If we apply Vivek's or your suggestion, it may always cause kdump
to fail on MIPS OCTEON due to Problem 1.  I don't want to make things
any worse.  I may post a patch for MIPS OCTEON, but I can't test it.
For other architectures, I'm not sure what problems there are.
So at first, I want to fix the case where crash_kexec_post_notifiers is
specified on x86.  Then, if all other architectures support
`stop other CPUs before crash_kexec', switch to your or Vivek's
suggesting code.

Is this acceptable?

Best regards,

Hidehiro Kawai
Hitachi, Ltd. Research & Development Group

RE: Re: [V3 PATCH 1/2] x86/panic: Replace smp_send_stop() with kdump friendly version

[河合英宏 / KAWAI，HIDEHIRO]

Hi,

> From: 'Dave Young' [mailto:dyo...@redhat.com]
> Sent: Monday, July 18, 2016 6:02 PM
> On 07/15/16 at 11:50am, 河合英宏 / KAWAI，HIDEHIRO wrote:
> > Hi Dave,
> >
> > Thanks for your reply.
> >
> > > From: 'Dave Young' [mailto:dyo...@redhat.com]
> > > Sent: Wednesday, July 13, 2016 11:04 AM
> > >
> > > On 07/12/16 at 02:49am, 河合英宏 / KAWAI，HIDEHIRO wrote:
> > > > Hi Dave,
> > > >
> > > > Thanks for the comments.
> > > >
> > > > > From: Dave Young [mailto:dyo...@redhat.com]
> > > > > Sent: Monday, July 11, 2016 5:35 PM
> > > > >
> > > > > On 07/05/16 at 08:33pm, Hidehiro Kawai wrote:
[snip] 
> > > > > As for this patch I'm not sure it is safe to replace the
> > > > > smp_send_stop with the kdump friendly function. I'm also not sure if
> > > > > the kdump friendly function is safe for kdump. Will glad to hear
> > > > > opinions from other arch experts.
> > > >
> > > > This stuff depends on architectures, so I speak only about
> > > > x86 (the logic doesn't change on other architectures at this time).
> > > >
> > > > kdump path with crash_kexec_post_notifiers disabled:
> > > >  panic()
> > > >__crash_kexec()
> > > >  crash_setup_regs()
> > > >  crash_save_vmcoreinfo()
> > > >  machine_crash_shutdown()
> > > >native_machine_crash_shutdown()
> > > >  panic_smp_send_stop() /* mostly same as original
> > > > * kdump_nmi_shootdown_cpus()
> > > > */
> > > >
> > > > kdump path with crash_kexec_post_notifiers enabled:
> > > >  panic()
> > > >panic_smp_send_stop()
> > > >__crash_kexec()
> > > >  crash_setup_regs()
> > > >  crash_save_vmcoreinfo()
> > > >  machine_crash_shutdown()
> > > >native_machine_crash_shutdown()
> > > >  panic_smp_send_stop() // do nothing
> > > >
> > > > The difference is that stopping other CPUs before crash_setup_regs()
> > > > and crash_save_vmcoreinfo() or not.  Since crash_setup_regs() and
> > > > crash_save_vmcoreinfo() just save information to some memory area,
> > > > they wouldn't be affected by panic_smp_send_stop().  This means
> > > > placing panic_smp_send_stop before __crash_kexec is safe.
> > > >
> > > > BTW, I noticed my patch breaks Xen kernel.  I'll fix it in the next
> > > > version.
> > >
> > > But it does breaks stuff which depends on cpu not being disabled like 
> > > problem 1 you mentioned in patch log.
> >
> > As I mentioned in the description of this patch, we should stop
> > other CPUs ASAP to preserve current state either
> > crash_kexec_post_notifiers is enabled or not.
> > Then, all remaining procedures should work well
> > after stopping other CPUs (but keep the CPU map online).
> >
> > Vivek also mentioned similar things:
> > https://lkml.org/lkml/2015/7/14/433
> 
> The implementation in this patchset is different from suggestion in above 
> link?
> 
> I think Vivek's suggestion is a good idea, to drop smp_send_stop and do below:
> 
> stop_cpus_save_register_state;
> 
> if (!crash_kexec_post_notifiers)
>   crash_kexec()
> atomic_notifier_call_chain()
> kmsg_dump()
> 
> I'm just commenting from code flow point of view, the detail implementation
> definitely need more comments from Arch experts.
> 
> Any reason did not move the kdump friendly function to earlier point like
> before previous __crash_kexec() below?
> if (!crash_kexec_post_notifiers) {
> printk_nmi_flush_on_panic();
> __crash_kexec(NULL);
> }

The reason why the implementation differs from Vivek's is to keep
the current code flow if crash_kexec_post_notifiers is not specified.

If we apply Vivek's or your suggestion, it may always cause kdump
to fail on MIPS OCTEON due to Problem 1.  I don't want to make things
any worse.  I may post a patch for MIPS OCTEON, but I can't test it.
For other architectures, I'm not sure what problems there are.
So at first, I want to fix the case where crash_kexec_post_notifiers is
specified on x86.  Then, if all other architectures support
`stop other CPUs before crash_kexec', switch to your or Vivek's
suggesting code.

Is this acceptable?

Best regards,

Hidehiro Kawai
Hitachi, Ltd. Research & Development Group

scripts/sign-file.c:23:30: fatal error: openssl/opensslv.h: No such file or directory

[kbuild test robot]

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
master
head:   47ef4ad2684d380dd6d596140fb79395115c3950
commit: 283e8ba2dfde54f8f27d7d0f459a07de79a39d55 MODSIGN: Change from CMS to 
PKCS#7 signing if the openssl is too old
date:   10 months ago
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 5.4.0-6) 5.4.0 20160609
reproduce:
wget 
https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
 -O ~/bin/make.cross
chmod +x ~/bin/make.cross
git checkout 283e8ba2dfde54f8f27d7d0f459a07de79a39d55
# save the attached .config to linux build tree
make.cross ARCH=arm 

All errors (new ones prefixed by >>):

>> scripts/sign-file.c:23:30: fatal error: openssl/opensslv.h: No such file or 
>> directory
   compilation terminated.
   make[2]: *** [scripts/sign-file] Error 1
   scripts/extract-cert.c:21:25: fatal error: openssl/bio.h: No such file or 
directory
   compilation terminated.
   make[2]: *** [scripts/extract-cert] Error 1
   make[2]: Target '__build' not remade because of errors.
   make[1]: *** [scripts] Error 2
   :1298:2: warning: #warning syscall userfaultfd not implemented [-Wcpp]
   :1301:2: warning: #warning syscall membarrier not implemented [-Wcpp]
   make[1]: Target 'modules_prepare' not remade because of errors.
   make: *** [sub-make] Error 2

vim +23 scripts/sign-file.c

17  #include 
18  #include 
19  #include 
20  #include 
21  #include 
22  #include 
  > 23  #include 
24  #include 
25  #include 
26  #include 

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


.config.gz
Description: Binary data

scripts/sign-file.c:23:30: fatal error: openssl/opensslv.h: No such file or directory

[kbuild test robot]

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
master
head:   47ef4ad2684d380dd6d596140fb79395115c3950
commit: 283e8ba2dfde54f8f27d7d0f459a07de79a39d55 MODSIGN: Change from CMS to 
PKCS#7 signing if the openssl is too old
date:   10 months ago
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 5.4.0-6) 5.4.0 20160609
reproduce:
wget 
https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
 -O ~/bin/make.cross
chmod +x ~/bin/make.cross
git checkout 283e8ba2dfde54f8f27d7d0f459a07de79a39d55
# save the attached .config to linux build tree
make.cross ARCH=arm 

All errors (new ones prefixed by >>):

>> scripts/sign-file.c:23:30: fatal error: openssl/opensslv.h: No such file or 
>> directory
   compilation terminated.
   make[2]: *** [scripts/sign-file] Error 1
   scripts/extract-cert.c:21:25: fatal error: openssl/bio.h: No such file or 
directory
   compilation terminated.
   make[2]: *** [scripts/extract-cert] Error 1
   make[2]: Target '__build' not remade because of errors.
   make[1]: *** [scripts] Error 2
   :1298:2: warning: #warning syscall userfaultfd not implemented [-Wcpp]
   :1301:2: warning: #warning syscall membarrier not implemented [-Wcpp]
   make[1]: Target 'modules_prepare' not remade because of errors.
   make: *** [sub-make] Error 2

vim +23 scripts/sign-file.c

17  #include 
18  #include 
19  #include 
20  #include 
21  #include 
22  #include 
  > 23  #include 
24  #include 
25  #include 
26  #include 

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


.config.gz
Description: Binary data

Re: [PATCH 5/5] f2fs: use blk_plug in all the possible paths

[Jaegeuk Kim]

On Mon, Jul 18, 2016 at 08:59:52PM -0700, Christoph Hellwig wrote:
> On Thu, Jul 14, 2016 at 08:05:02PM -0700, Jaegeuk Kim wrote:
> > >From kernel guys working on android.
> 
> Well, until it's mainline it simply doesn't matter, so NAK to this
> patch.  Tying power behavior to plugs also sounds pretty broken, so we'd
> probably come up with something better if they bothered to actually
> submit their patches upsteam or at least explaining what they want
> to archive.

Actually, the initial patch which drops plugs is not mainlined as well.
So, at this time, I just want to revert it by this patch, since I recognized
that there might be whatever possible use-cases of plugs in another way someday.
So for now, I lost the reason to eliminate the plugs.

Moreover, since every filesystems use plugs, f2fs doesn't need to be an
exceptional case in terms of that (except hm-smr). So, let me sync f2fs
with other filesystems at this moment.

I totally agree that it'd be best to see their patches upstream, but I've
witnessed that it becomes a quite tough challenge to them.

Thanks,

Re: [PATCH 5/5] f2fs: use blk_plug in all the possible paths

[Jaegeuk Kim]

On Mon, Jul 18, 2016 at 08:59:52PM -0700, Christoph Hellwig wrote:
> On Thu, Jul 14, 2016 at 08:05:02PM -0700, Jaegeuk Kim wrote:
> > >From kernel guys working on android.
> 
> Well, until it's mainline it simply doesn't matter, so NAK to this
> patch.  Tying power behavior to plugs also sounds pretty broken, so we'd
> probably come up with something better if they bothered to actually
> submit their patches upsteam or at least explaining what they want
> to archive.

Actually, the initial patch which drops plugs is not mainlined as well.
So, at this time, I just want to revert it by this patch, since I recognized
that there might be whatever possible use-cases of plugs in another way someday.
So for now, I lost the reason to eliminate the plugs.

Moreover, since every filesystems use plugs, f2fs doesn't need to be an
exceptional case in terms of that (except hm-smr). So, let me sync f2fs
with other filesystems at this moment.

I totally agree that it'd be best to see their patches upstream, but I've
witnessed that it becomes a quite tough challenge to them.

Thanks,

[Patch-V2 1/3] cxgb4: Add Chelsio LLD support Chelsio Crypto ULD

[Yeshaswi M R Gowda]

The Chelsio crypto driver is an Upper Layer Driver (ULD), making use
of the Chelsio Lower Layer Driver (LLD - cxgb4). The LLD facilitates
the basic infrastructure services of the ULD. These services include
queue allocation, deallocation and registration with LLD. The queues
are used for sending the crypto requests to the Chelsio's hardware
and for receiving the responses from the hardware.

This patch enables the services mentioned for the Chelsio's crypto
driver.

Signed-off-by: Yeshaswi M R Gowda 
---
 drivers/net/ethernet/chelsio/cxgb4/cxgb4.h |   18 +-
 drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c |   41 +-
 drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c|   80 +++-
 drivers/net/ethernet/chelsio/cxgb4/cxgb4_uld.h |   10 +
 drivers/net/ethernet/chelsio/cxgb4/sge.c   |   64 +++
 drivers/net/ethernet/chelsio/cxgb4/t4_msg.h|  437 
 drivers/net/ethernet/chelsio/cxgb4/t4fw_api.h  |  131 +-
 7 files changed, 770 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h 
b/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
index b4fceb9..4de1e39 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
@@ -346,6 +346,8 @@ struct adapter_params {
 
unsigned int max_ordird_qp;   /* Max read depth per RDMA QP */
unsigned int max_ird_adapter; /* Max read depth per adapter */
+
+   unsigned char ulp_crypto_lookaside; /* crypto lookaside support */
 };
 
 /* State needed to monitor the forward progress of SGE Ingress DMA activities
@@ -435,7 +437,7 @@ enum {
MAX_CTRL_QUEUES = NCHAN,  /* # of control Tx queues */
MAX_RDMA_QUEUES = NCHAN,  /* # of streaming RDMA Rx queues */
MAX_RDMA_CIQS = 32,/* # of  RDMA concentrator IQs */
-
+   MAX_CRYPTO_QUEUES = 32,   /* # of crypto queues */
/* # of streaming iSCSIT Rx queues */
MAX_ISCSIT_QUEUES = MAX_OFLD_QSETS,
 };
@@ -455,7 +457,8 @@ enum {
INGQ_EXTRAS = 2,/* firmware event queue and */
/*   forwarded interrupts */
MAX_INGQ = MAX_ETH_QSETS + MAX_OFLD_QSETS + MAX_RDMA_QUEUES +
-  MAX_RDMA_CIQS + MAX_ISCSIT_QUEUES + INGQ_EXTRAS,
+  MAX_RDMA_CIQS + MAX_ISCSIT_QUEUES + INGQ_EXTRAS +
+  MAX_CRYPTO_QUEUES,
 };
 
 struct adapter;
@@ -509,6 +512,10 @@ enum { /* adapter flags */
FW_OFLD_CONN   = (1 << 9),
 };
 
+enum {
+   ULP_CRYPTO_LOOKASIDE = 1 << 0,
+};
+
 struct rx_sw_desc;
 
 struct sge_fl { /* SGE free-buffer queue state */
@@ -682,10 +689,12 @@ struct sge_ctrl_txq {   /* state for an SGE 
control Tx queue */
 struct sge {
struct sge_eth_txq ethtxq[MAX_ETH_QSETS];
struct sge_ofld_txq ofldtxq[MAX_OFLD_QSETS];
+   struct sge_ofld_txq cryptotxq[MAX_CRYPTO_QUEUES];
struct sge_ctrl_txq ctrlq[MAX_CTRL_QUEUES];
 
struct sge_eth_rxq ethrxq[MAX_ETH_QSETS];
struct sge_ofld_rxq iscsirxq[MAX_OFLD_QSETS];
+   struct sge_ofld_rxq cryptorxq[MAX_CRYPTO_QUEUES];
struct sge_ofld_rxq iscsitrxq[MAX_ISCSIT_QUEUES];
struct sge_ofld_rxq rdmarxq[MAX_RDMA_QUEUES];
struct sge_ofld_rxq rdmaciq[MAX_RDMA_CIQS];
@@ -699,10 +708,12 @@ struct sge {
u16 ethtxq_rover;   /* Tx queue to clean up next */
u16 iscsiqsets;  /* # of active iSCSI queue sets */
u16 niscsitq;   /* # of available iSCST Rx queues */
+   u16 ncryptoq;   /* # of available lookaside crypto queues */
u16 rdmaqs; /* # of available RDMA Rx queues */
u16 rdmaciqs;   /* # of available RDMA concentrator IQs */
u16 iscsi_rxq[MAX_OFLD_QSETS];
u16 iscsit_rxq[MAX_ISCSIT_QUEUES];
+   u16 crypto_rxq[MAX_CRYPTO_QUEUES];
u16 rdma_rxq[MAX_RDMA_QUEUES];
u16 rdma_ciq[MAX_RDMA_CIQS];
u16 timer_val[SGE_NTIMERS];
@@ -732,6 +743,7 @@ struct sge {
 #define for_each_iscsitrxq(sge, i) for (i = 0; i < (sge)->niscsitq; i++)
 #define for_each_rdmarxq(sge, i) for (i = 0; i < (sge)->rdmaqs; i++)
 #define for_each_rdmaciq(sge, i) for (i = 0; i < (sge)->rdmaciqs; i++)
+#define for_each_cryptorxq(sge, i) for (i = 0; i < (sge)->ncryptoq; i++)
 
 struct l2t_data;
 
@@ -1441,7 +1453,7 @@ int t4_fw_bye(struct adapter *adap, unsigned int mbox);
 int t4_early_init(struct adapter *adap, unsigned int mbox);
 int t4_fw_reset(struct adapter *adap, unsigned int mbox, int reset);
 int t4_fixup_host_params(struct adapter *adap, unsigned int page_size,
- unsigned int cache_line_size);
+unsigned int cache_line_size);
 int t4_fw_initialize(struct adapter *adap, unsigned int mbox);
 int t4_query_params(struct adapter *adap, unsigned int mbox, unsigned int pf,

[Patch-V2 1/3] cxgb4: Add Chelsio LLD support Chelsio Crypto ULD

[Yeshaswi M R Gowda]

The Chelsio crypto driver is an Upper Layer Driver (ULD), making use
of the Chelsio Lower Layer Driver (LLD - cxgb4). The LLD facilitates
the basic infrastructure services of the ULD. These services include
queue allocation, deallocation and registration with LLD. The queues
are used for sending the crypto requests to the Chelsio's hardware
and for receiving the responses from the hardware.

This patch enables the services mentioned for the Chelsio's crypto
driver.

Signed-off-by: Yeshaswi M R Gowda 
---
 drivers/net/ethernet/chelsio/cxgb4/cxgb4.h |   18 +-
 drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c |   41 +-
 drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c|   80 +++-
 drivers/net/ethernet/chelsio/cxgb4/cxgb4_uld.h |   10 +
 drivers/net/ethernet/chelsio/cxgb4/sge.c   |   64 +++
 drivers/net/ethernet/chelsio/cxgb4/t4_msg.h|  437 
 drivers/net/ethernet/chelsio/cxgb4/t4fw_api.h  |  131 +-
 7 files changed, 770 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h 
b/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
index b4fceb9..4de1e39 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
@@ -346,6 +346,8 @@ struct adapter_params {
 
unsigned int max_ordird_qp;   /* Max read depth per RDMA QP */
unsigned int max_ird_adapter; /* Max read depth per adapter */
+
+   unsigned char ulp_crypto_lookaside; /* crypto lookaside support */
 };
 
 /* State needed to monitor the forward progress of SGE Ingress DMA activities
@@ -435,7 +437,7 @@ enum {
MAX_CTRL_QUEUES = NCHAN,  /* # of control Tx queues */
MAX_RDMA_QUEUES = NCHAN,  /* # of streaming RDMA Rx queues */
MAX_RDMA_CIQS = 32,/* # of  RDMA concentrator IQs */
-
+   MAX_CRYPTO_QUEUES = 32,   /* # of crypto queues */
/* # of streaming iSCSIT Rx queues */
MAX_ISCSIT_QUEUES = MAX_OFLD_QSETS,
 };
@@ -455,7 +457,8 @@ enum {
INGQ_EXTRAS = 2,/* firmware event queue and */
/*   forwarded interrupts */
MAX_INGQ = MAX_ETH_QSETS + MAX_OFLD_QSETS + MAX_RDMA_QUEUES +
-  MAX_RDMA_CIQS + MAX_ISCSIT_QUEUES + INGQ_EXTRAS,
+  MAX_RDMA_CIQS + MAX_ISCSIT_QUEUES + INGQ_EXTRAS +
+  MAX_CRYPTO_QUEUES,
 };
 
 struct adapter;
@@ -509,6 +512,10 @@ enum { /* adapter flags */
FW_OFLD_CONN   = (1 << 9),
 };
 
+enum {
+   ULP_CRYPTO_LOOKASIDE = 1 << 0,
+};
+
 struct rx_sw_desc;
 
 struct sge_fl { /* SGE free-buffer queue state */
@@ -682,10 +689,12 @@ struct sge_ctrl_txq {   /* state for an SGE 
control Tx queue */
 struct sge {
struct sge_eth_txq ethtxq[MAX_ETH_QSETS];
struct sge_ofld_txq ofldtxq[MAX_OFLD_QSETS];
+   struct sge_ofld_txq cryptotxq[MAX_CRYPTO_QUEUES];
struct sge_ctrl_txq ctrlq[MAX_CTRL_QUEUES];
 
struct sge_eth_rxq ethrxq[MAX_ETH_QSETS];
struct sge_ofld_rxq iscsirxq[MAX_OFLD_QSETS];
+   struct sge_ofld_rxq cryptorxq[MAX_CRYPTO_QUEUES];
struct sge_ofld_rxq iscsitrxq[MAX_ISCSIT_QUEUES];
struct sge_ofld_rxq rdmarxq[MAX_RDMA_QUEUES];
struct sge_ofld_rxq rdmaciq[MAX_RDMA_CIQS];
@@ -699,10 +708,12 @@ struct sge {
u16 ethtxq_rover;   /* Tx queue to clean up next */
u16 iscsiqsets;  /* # of active iSCSI queue sets */
u16 niscsitq;   /* # of available iSCST Rx queues */
+   u16 ncryptoq;   /* # of available lookaside crypto queues */
u16 rdmaqs; /* # of available RDMA Rx queues */
u16 rdmaciqs;   /* # of available RDMA concentrator IQs */
u16 iscsi_rxq[MAX_OFLD_QSETS];
u16 iscsit_rxq[MAX_ISCSIT_QUEUES];
+   u16 crypto_rxq[MAX_CRYPTO_QUEUES];
u16 rdma_rxq[MAX_RDMA_QUEUES];
u16 rdma_ciq[MAX_RDMA_CIQS];
u16 timer_val[SGE_NTIMERS];
@@ -732,6 +743,7 @@ struct sge {
 #define for_each_iscsitrxq(sge, i) for (i = 0; i < (sge)->niscsitq; i++)
 #define for_each_rdmarxq(sge, i) for (i = 0; i < (sge)->rdmaqs; i++)
 #define for_each_rdmaciq(sge, i) for (i = 0; i < (sge)->rdmaciqs; i++)
+#define for_each_cryptorxq(sge, i) for (i = 0; i < (sge)->ncryptoq; i++)
 
 struct l2t_data;
 
@@ -1441,7 +1453,7 @@ int t4_fw_bye(struct adapter *adap, unsigned int mbox);
 int t4_early_init(struct adapter *adap, unsigned int mbox);
 int t4_fw_reset(struct adapter *adap, unsigned int mbox, int reset);
 int t4_fixup_host_params(struct adapter *adap, unsigned int page_size,
- unsigned int cache_line_size);
+unsigned int cache_line_size);
 int t4_fw_initialize(struct adapter *adap, unsigned int mbox);
 int t4_query_params(struct adapter *adap, unsigned int mbox, unsigned int pf,
unsigned int vf, unsigned 

Re: [PATCH] net/sched/sch_htb: clamp xstats tokens to fit into 32-bit int

[David Miller]

From: Konstantin Khlebnikov 
Date: Sat, 16 Jul 2016 17:08:56 +0300

> In kernel HTB keeps tokens in signed 64-bit in nanoseconds. In netlink
> protocol these values are converted into pshed ticks (64ns for now) and
> truncated to 32-bit. In struct tc_htb_xstats fields "tokens" and "ctokens"
> are declared as unsigned 32-bit but they could be negative thus tool 'tc'
> prints them as signed. Big values loose higher bits and/or become negative.
> 
> This patch clamps tokens in xstat into range from INT_MIN to INT_MAX.
> In this way it's easier to understand what's going on here.
> 
> Signed-off-by: Konstantin Khlebnikov 

Applied.

Re: [PATCH] net/sched/sch_htb: clamp xstats tokens to fit into 32-bit int

[David Miller]

From: Konstantin Khlebnikov 
Date: Sat, 16 Jul 2016 17:08:56 +0300

> In kernel HTB keeps tokens in signed 64-bit in nanoseconds. In netlink
> protocol these values are converted into pshed ticks (64ns for now) and
> truncated to 32-bit. In struct tc_htb_xstats fields "tokens" and "ctokens"
> are declared as unsigned 32-bit but they could be negative thus tool 'tc'
> prints them as signed. Big values loose higher bits and/or become negative.
> 
> This patch clamps tokens in xstat into range from INT_MIN to INT_MAX.
> In this way it's easier to understand what's going on here.
> 
> Signed-off-by: Konstantin Khlebnikov 

Applied.

[Patch-V2 2/3] chcr: Support for Chelsio's Crypto Hardware

[Yeshaswi M R Gowda]

The Chelsio's Crypto Hardware can perform the following operations:
SHA1, SHA224, SHA256, SHA384 and SHA512, HMAC(SHA1), HMAC(SHA224),
HMAC(SHA256), HMAC(SHA384), HAMC(SHA512), AES-128-CBC, AES-192-CBC,
AES-256-CBC, AES-128-XTS, AES-256-XTS

This patch implements the driver for above mentioned features. This
driver is an Upper Layer Driver which is attached to Chelsio's LLD
(cxgb4) and uses the queue allocated by the LLD for sending the crypto
requests to the Hardware and receiving the responses from it.

The crypto operations can be performed by Chelsio's hardware from the
userspace applications and/or from within the kernel space using the
kernel's crypto API.

The above mentioned crypto features have been tested using kernel's
tests mentioned in testmgr.h. They also have been tested from user
space using libkcapi and Openssl.

Signed-off-by: Yeshaswi M R Gowda 
---
 drivers/crypto/chelsio/Kconfig   |   21 +
 drivers/crypto/chelsio/Makefile  |4 +
 drivers/crypto/chelsio/chcr_algo.c   | 1509 ++
 drivers/crypto/chelsio/chcr_algo.h   |  503 
 drivers/crypto/chelsio/chcr_core.c   |  268 ++
 drivers/crypto/chelsio/chcr_core.h   |   80 ++
 drivers/crypto/chelsio/chcr_crypto.h |  204 +
 7 files changed, 2589 insertions(+)
 create mode 100644 drivers/crypto/chelsio/Kconfig
 create mode 100644 drivers/crypto/chelsio/Makefile
 create mode 100644 drivers/crypto/chelsio/chcr_algo.c
 create mode 100644 drivers/crypto/chelsio/chcr_algo.h
 create mode 100644 drivers/crypto/chelsio/chcr_core.c
 create mode 100644 drivers/crypto/chelsio/chcr_core.h
 create mode 100644 drivers/crypto/chelsio/chcr_crypto.h

diff --git a/drivers/crypto/chelsio/Kconfig b/drivers/crypto/chelsio/Kconfig
new file mode 100644
index 000..4266cb2
--- /dev/null
+++ b/drivers/crypto/chelsio/Kconfig
@@ -0,0 +1,21 @@
+config CRYPTO_DEV_CHELSIO
+   tristate "Chelsio Crypto Co-processor Driver"
+   depends on PCI && NETDEVICES && ETHERNET
+   select CRYPTO_SHA1
+   select CRYPTO_SHA256
+   select CRYPTO_SHA512
+   select NET_VENDOR_CHELSIO
+   select CHELSIO_T4
+   ---help---
+ The Chelsio Crypto Co-processor driver for T6 adapters.
+
+ For general information about Chelsio and our products, visit
+ our website at .
+
+ For customer support, please visit our customer support page at
+ .
+
+ Please send feedback to .
+
+ To compile this driver as a module, choose M here: the module
+ will be called chcr.
diff --git a/drivers/crypto/chelsio/Makefile b/drivers/crypto/chelsio/Makefile
new file mode 100644
index 000..7e4fda5
--- /dev/null
+++ b/drivers/crypto/chelsio/Makefile
@@ -0,0 +1,4 @@
+ ccflags-y := -Idrivers/net/ethernet/chelsio/cxgb4
+
+ obj-$(CONFIG_CRYPTO_DEV_CHELSIO) += chcr.o
+ chcr-objs :=  chcr_core.o chcr_algo.o
\ No newline at end of file
diff --git a/drivers/crypto/chelsio/chcr_algo.c 
b/drivers/crypto/chelsio/chcr_algo.c
new file mode 100644
index 000..a327b53
--- /dev/null
+++ b/drivers/crypto/chelsio/chcr_algo.c
@@ -0,0 +1,1509 @@
+/*
+ * This file is part of the Chelsio T6 Crypto driver for Linux.
+ *
+ * Copyright (c) 2003-2016 Chelsio Communications, Inc. All rights reserved.
+ *
+ * This software is available to you under a choice of one of two
+ * licenses.  You may choose to be licensed under the terms of the GNU
+ * General Public License (GPL) Version 2, available from the file
+ * COPYING in the main directory of this source tree, or the
+ * OpenIB.org BSD license below:
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ *  - Redistributions of source code must retain the above
+ *copyright notice, this list of conditions and the following
+ *disclaimer.
+ *
+ *  - Redistributions in binary form must reproduce the above
+ *copyright notice, this list of conditions and the following
+ *disclaimer in the documentation and/or other materials
+ *provided with the distribution.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
+ * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+ * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ *
+ * Written and Maintained by:
+ * Manoj Malviya (manojmalv...@chelsio.com)
+ * Atul Gupta (atul.gu...@chelsio.com)
+ * Jitendra Lulla (jlu...@chelsio.com)
+ * Yeshaswi M R Gowda (yesha...@chelsio.com)
+ *   

[Patch-V2 2/3] chcr: Support for Chelsio's Crypto Hardware

[Yeshaswi M R Gowda]

The Chelsio's Crypto Hardware can perform the following operations:
SHA1, SHA224, SHA256, SHA384 and SHA512, HMAC(SHA1), HMAC(SHA224),
HMAC(SHA256), HMAC(SHA384), HAMC(SHA512), AES-128-CBC, AES-192-CBC,
AES-256-CBC, AES-128-XTS, AES-256-XTS

This patch implements the driver for above mentioned features. This
driver is an Upper Layer Driver which is attached to Chelsio's LLD
(cxgb4) and uses the queue allocated by the LLD for sending the crypto
requests to the Hardware and receiving the responses from it.

The crypto operations can be performed by Chelsio's hardware from the
userspace applications and/or from within the kernel space using the
kernel's crypto API.

The above mentioned crypto features have been tested using kernel's
tests mentioned in testmgr.h. They also have been tested from user
space using libkcapi and Openssl.

Signed-off-by: Yeshaswi M R Gowda 
---
 drivers/crypto/chelsio/Kconfig   |   21 +
 drivers/crypto/chelsio/Makefile  |4 +
 drivers/crypto/chelsio/chcr_algo.c   | 1509 ++
 drivers/crypto/chelsio/chcr_algo.h   |  503 
 drivers/crypto/chelsio/chcr_core.c   |  268 ++
 drivers/crypto/chelsio/chcr_core.h   |   80 ++
 drivers/crypto/chelsio/chcr_crypto.h |  204 +
 7 files changed, 2589 insertions(+)
 create mode 100644 drivers/crypto/chelsio/Kconfig
 create mode 100644 drivers/crypto/chelsio/Makefile
 create mode 100644 drivers/crypto/chelsio/chcr_algo.c
 create mode 100644 drivers/crypto/chelsio/chcr_algo.h
 create mode 100644 drivers/crypto/chelsio/chcr_core.c
 create mode 100644 drivers/crypto/chelsio/chcr_core.h
 create mode 100644 drivers/crypto/chelsio/chcr_crypto.h

diff --git a/drivers/crypto/chelsio/Kconfig b/drivers/crypto/chelsio/Kconfig
new file mode 100644
index 000..4266cb2
--- /dev/null
+++ b/drivers/crypto/chelsio/Kconfig
@@ -0,0 +1,21 @@
+config CRYPTO_DEV_CHELSIO
+   tristate "Chelsio Crypto Co-processor Driver"
+   depends on PCI && NETDEVICES && ETHERNET
+   select CRYPTO_SHA1
+   select CRYPTO_SHA256
+   select CRYPTO_SHA512
+   select NET_VENDOR_CHELSIO
+   select CHELSIO_T4
+   ---help---
+ The Chelsio Crypto Co-processor driver for T6 adapters.
+
+ For general information about Chelsio and our products, visit
+ our website at .
+
+ For customer support, please visit our customer support page at
+ .
+
+ Please send feedback to .
+
+ To compile this driver as a module, choose M here: the module
+ will be called chcr.
diff --git a/drivers/crypto/chelsio/Makefile b/drivers/crypto/chelsio/Makefile
new file mode 100644
index 000..7e4fda5
--- /dev/null
+++ b/drivers/crypto/chelsio/Makefile
@@ -0,0 +1,4 @@
+ ccflags-y := -Idrivers/net/ethernet/chelsio/cxgb4
+
+ obj-$(CONFIG_CRYPTO_DEV_CHELSIO) += chcr.o
+ chcr-objs :=  chcr_core.o chcr_algo.o
\ No newline at end of file
diff --git a/drivers/crypto/chelsio/chcr_algo.c 
b/drivers/crypto/chelsio/chcr_algo.c
new file mode 100644
index 000..a327b53
--- /dev/null
+++ b/drivers/crypto/chelsio/chcr_algo.c
@@ -0,0 +1,1509 @@
+/*
+ * This file is part of the Chelsio T6 Crypto driver for Linux.
+ *
+ * Copyright (c) 2003-2016 Chelsio Communications, Inc. All rights reserved.
+ *
+ * This software is available to you under a choice of one of two
+ * licenses.  You may choose to be licensed under the terms of the GNU
+ * General Public License (GPL) Version 2, available from the file
+ * COPYING in the main directory of this source tree, or the
+ * OpenIB.org BSD license below:
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ *  - Redistributions of source code must retain the above
+ *copyright notice, this list of conditions and the following
+ *disclaimer.
+ *
+ *  - Redistributions in binary form must reproduce the above
+ *copyright notice, this list of conditions and the following
+ *disclaimer in the documentation and/or other materials
+ *provided with the distribution.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
+ * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+ * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ *
+ * Written and Maintained by:
+ * Manoj Malviya (manojmalv...@chelsio.com)
+ * Atul Gupta (atul.gu...@chelsio.com)
+ * Jitendra Lulla (jlu...@chelsio.com)
+ * Yeshaswi M R Gowda (yesha...@chelsio.com)
+ * Harsh Jain (ha...@chelsio.com)
+ */
+

[Patch-V2 0/3] crypto/chcr: Add Chelsio Crypto Driver

[Yeshaswi M R Gowda]

Hi Herbert,

This patch series contains 3 patches that add support for Chelsio's
Crypto Hardware.

The patch series has been created against Herbert Xu's tree (crypto-2.6).
It includes patches for Chelsio Low Level Driver(cxgb4) and adds the new
crypto Upper Layer Driver(chcr) under a new directory drivers/crypto/chelsio.

The first of the patch series implements necessary changes in the Chelsio
LLD for queue allocation, deallocation and registration of the ULD.

The second patch implements the Chelsio crypto driver.

The third patch contains the changes to the driver/crypto/Kconfig and
drivers/crypto/Makefile to enable the Chelsio Crypto driver.

We have included all the maintainers of respective drivers. Kindly
review the changes and provide feedback on the same.

Thank you Joe Perches and Herbert Xu for your review, I have made appropriate
changes based on them.

[V1 -> V2]

1. Some residual code cleanup
2. Adds pr_fmt with chcr (KBUILD_MODNAME) added
3. Changes var name to accomodate them <80 columns in the chcr_register_alg
4. Support for printing the crypto queue stats
5. Fix compile warnings reported by kbuild bot for certain architectures
6. Dependency fix in Kconfig.
7. If the request has the MAY_BACKLOG bit set and hardware queue is full the 
request
   is queued up else -EBUSY is returned to throttle the user. The queue when 
executed
   and processed returns -EINPROGRESS in completion.

Yeshaswi M R Gowda (3):
  cxgb4: Add Chelsio LLD support Chelsio Crypto ULD
  chcr: Support for Chelsio's Crypto Hardware
  crypto: Added Chelsio Menu to the Kconfig file

 drivers/crypto/Kconfig |2 +
 drivers/crypto/Makefile|1 +
 drivers/crypto/chelsio/Kconfig |   21 +
 drivers/crypto/chelsio/Makefile|4 +
 drivers/crypto/chelsio/chcr_algo.c | 1509 
 drivers/crypto/chelsio/chcr_algo.h |  503 +++
 drivers/crypto/chelsio/chcr_core.c |  268 
 drivers/crypto/chelsio/chcr_core.h |   80 ++
 drivers/crypto/chelsio/chcr_crypto.h   |  204 +++
 drivers/net/ethernet/chelsio/cxgb4/cxgb4.h |   18 +-
 drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c |   41 +-
 drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c|   80 +-
 drivers/net/ethernet/chelsio/cxgb4/cxgb4_uld.h |   10 +
 drivers/net/ethernet/chelsio/cxgb4/sge.c   |   64 +
 drivers/net/ethernet/chelsio/cxgb4/t4_msg.h|  437 ++
 drivers/net/ethernet/chelsio/cxgb4/t4fw_api.h  |  131 +-
 16 files changed, 3362 insertions(+), 11 deletions(-)
 create mode 100644 drivers/crypto/chelsio/Kconfig
 create mode 100644 drivers/crypto/chelsio/Makefile
 create mode 100644 drivers/crypto/chelsio/chcr_algo.c
 create mode 100644 drivers/crypto/chelsio/chcr_algo.h
 create mode 100644 drivers/crypto/chelsio/chcr_core.c
 create mode 100644 drivers/crypto/chelsio/chcr_core.h
 create mode 100644 drivers/crypto/chelsio/chcr_crypto.h

-- 
1.7.10.1

[Patch-V2 0/3] crypto/chcr: Add Chelsio Crypto Driver

[Yeshaswi M R Gowda]

Hi Herbert,

This patch series contains 3 patches that add support for Chelsio's
Crypto Hardware.

The patch series has been created against Herbert Xu's tree (crypto-2.6).
It includes patches for Chelsio Low Level Driver(cxgb4) and adds the new
crypto Upper Layer Driver(chcr) under a new directory drivers/crypto/chelsio.

The first of the patch series implements necessary changes in the Chelsio
LLD for queue allocation, deallocation and registration of the ULD.

The second patch implements the Chelsio crypto driver.

The third patch contains the changes to the driver/crypto/Kconfig and
drivers/crypto/Makefile to enable the Chelsio Crypto driver.

We have included all the maintainers of respective drivers. Kindly
review the changes and provide feedback on the same.

Thank you Joe Perches and Herbert Xu for your review, I have made appropriate
changes based on them.

[V1 -> V2]

1. Some residual code cleanup
2. Adds pr_fmt with chcr (KBUILD_MODNAME) added
3. Changes var name to accomodate them <80 columns in the chcr_register_alg
4. Support for printing the crypto queue stats
5. Fix compile warnings reported by kbuild bot for certain architectures
6. Dependency fix in Kconfig.
7. If the request has the MAY_BACKLOG bit set and hardware queue is full the 
request
   is queued up else -EBUSY is returned to throttle the user. The queue when 
executed
   and processed returns -EINPROGRESS in completion.

Yeshaswi M R Gowda (3):
  cxgb4: Add Chelsio LLD support Chelsio Crypto ULD
  chcr: Support for Chelsio's Crypto Hardware
  crypto: Added Chelsio Menu to the Kconfig file

 drivers/crypto/Kconfig |2 +
 drivers/crypto/Makefile|1 +
 drivers/crypto/chelsio/Kconfig |   21 +
 drivers/crypto/chelsio/Makefile|4 +
 drivers/crypto/chelsio/chcr_algo.c | 1509 
 drivers/crypto/chelsio/chcr_algo.h |  503 +++
 drivers/crypto/chelsio/chcr_core.c |  268 
 drivers/crypto/chelsio/chcr_core.h |   80 ++
 drivers/crypto/chelsio/chcr_crypto.h   |  204 +++
 drivers/net/ethernet/chelsio/cxgb4/cxgb4.h |   18 +-
 drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c |   41 +-
 drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c|   80 +-
 drivers/net/ethernet/chelsio/cxgb4/cxgb4_uld.h |   10 +
 drivers/net/ethernet/chelsio/cxgb4/sge.c   |   64 +
 drivers/net/ethernet/chelsio/cxgb4/t4_msg.h|  437 ++
 drivers/net/ethernet/chelsio/cxgb4/t4fw_api.h  |  131 +-
 16 files changed, 3362 insertions(+), 11 deletions(-)
 create mode 100644 drivers/crypto/chelsio/Kconfig
 create mode 100644 drivers/crypto/chelsio/Makefile
 create mode 100644 drivers/crypto/chelsio/chcr_algo.c
 create mode 100644 drivers/crypto/chelsio/chcr_algo.h
 create mode 100644 drivers/crypto/chelsio/chcr_core.c
 create mode 100644 drivers/crypto/chelsio/chcr_core.h
 create mode 100644 drivers/crypto/chelsio/chcr_crypto.h

-- 
1.7.10.1

[Patch-V2 3/3] crypto: Added Chelsio Menu to the Kconfig file

[Yeshaswi M R Gowda]

Adds the config entry for the Chelsio Crypto Driver, Makefile changes
for the same.

Signed-off-by: Yeshaswi M R Gowda 
---
 drivers/crypto/Kconfig  |2 ++
 drivers/crypto/Makefile |1 +
 2 files changed, 3 insertions(+)

diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
index d77ba2f..b44faf0 100644
--- a/drivers/crypto/Kconfig
+++ b/drivers/crypto/Kconfig
@@ -537,4 +537,6 @@ config CRYPTO_DEV_ROCKCHIP
  This driver interfaces with the hardware crypto accelerator.
  Supporting cbc/ecb chainmode, and aes/des/des3_ede cipher mode.
 
+source "drivers/crypto/chelsio/Kconfig"
+
 endif # CRYPTO_HW
diff --git a/drivers/crypto/Makefile b/drivers/crypto/Makefile
index 3c6432d..ad7250f 100644
--- a/drivers/crypto/Makefile
+++ b/drivers/crypto/Makefile
@@ -31,3 +31,4 @@ obj-$(CONFIG_CRYPTO_DEV_QCE) += qce/
 obj-$(CONFIG_CRYPTO_DEV_VMX) += vmx/
 obj-$(CONFIG_CRYPTO_DEV_SUN4I_SS) += sunxi-ss/
 obj-$(CONFIG_CRYPTO_DEV_ROCKCHIP) += rockchip/
+obj-$(CONFIG_CRYPTO_DEV_CHELSIO) += chelsio/
-- 
1.7.10.1

[Patch-V2 3/3] crypto: Added Chelsio Menu to the Kconfig file

[Yeshaswi M R Gowda]

Adds the config entry for the Chelsio Crypto Driver, Makefile changes
for the same.

Signed-off-by: Yeshaswi M R Gowda 
---
 drivers/crypto/Kconfig  |2 ++
 drivers/crypto/Makefile |1 +
 2 files changed, 3 insertions(+)

diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
index d77ba2f..b44faf0 100644
--- a/drivers/crypto/Kconfig
+++ b/drivers/crypto/Kconfig
@@ -537,4 +537,6 @@ config CRYPTO_DEV_ROCKCHIP
  This driver interfaces with the hardware crypto accelerator.
  Supporting cbc/ecb chainmode, and aes/des/des3_ede cipher mode.
 
+source "drivers/crypto/chelsio/Kconfig"
+
 endif # CRYPTO_HW
diff --git a/drivers/crypto/Makefile b/drivers/crypto/Makefile
index 3c6432d..ad7250f 100644
--- a/drivers/crypto/Makefile
+++ b/drivers/crypto/Makefile
@@ -31,3 +31,4 @@ obj-$(CONFIG_CRYPTO_DEV_QCE) += qce/
 obj-$(CONFIG_CRYPTO_DEV_VMX) += vmx/
 obj-$(CONFIG_CRYPTO_DEV_SUN4I_SS) += sunxi-ss/
 obj-$(CONFIG_CRYPTO_DEV_ROCKCHIP) += rockchip/
+obj-$(CONFIG_CRYPTO_DEV_CHELSIO) += chelsio/
-- 
1.7.10.1

Re: [PATCH v2 2/4] iio: adc: add support for Allwinner SoCs ADC

[Jonathan Cameron]

On 15/07/16 10:59, Quentin Schulz wrote:
> The Allwinner SoCs all have an ADC that can also act as a touchscreen
> controller and a thermal sensor. This patch adds the ADC driver which is
> based on the MFD for the same SoCs ADC.
> 
> This also registers the thermal adc channel in the iio map array so
> iio_hwmon could use it without modifying the Device Tree.
> 
> This driver probes on three different platform_device_id to take into
> account slight differences between Allwinner SoCs ADCs.
> 
> Signed-off-by: Quentin Schulz 

Hi Quentin,

Various bits inline.  In particular the irq handling looks flakey / racey
to me.  Definitely need some explanatory comments.

Also another note on the craziness that using extended_name to provide
the hwmon labels will cause :)

Jonathan
> ---
> 
> v2:
>  - add SUNXI_GPADC_ prefixes for defines,
>  - correct typo in Kconfig,
>  - reorder alphabetically includes, makefile,
>  - add license header,
>  - fix architecture variations not being handled in interrupt handlers or
>read raw functions,
>  - fix unability to return negative values from thermal sensor,
>  - add gotos to reduce code repetition,
>  - fix irq variable being unsigned int instead of int,
>  - remove useless dev_err and dev_info,
>  - deactivate all interrupts if probe fails,
>  - fix iio_device_register on NULL variable,
>  - deactivate ADC in the IP when probe fails or when removing driver,
> 
>  drivers/iio/adc/Kconfig   |  12 ++
>  drivers/iio/adc/Makefile  |   1 +
>  drivers/iio/adc/sunxi-gpadc-iio.c | 417 
> ++
>  3 files changed, 430 insertions(+)
>  create mode 100644 drivers/iio/adc/sunxi-gpadc-iio.c
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 25378c5..184856f 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -338,6 +338,18 @@ config NAU7802
> To compile this driver as a module, choose M here: the
> module will be called nau7802.
>  
> +config SUNXI_ADC
> + tristate "ADC driver for sunxi platforms"
> + depends on IIO
> + depends on MFD_SUNXI_ADC
> + help
> +   Say yes here to build support for Allwinner (A10, A13 and A31) SoCs
> +   ADC. This ADC provides 4 channels which can be used as an ADC or as a
> +   touchscreen input and one channel for thermal sensor.
> +
> +  To compile this driver as a module, choose M here: the module will 
> be
> +   called sunxi-gpadc-iio.
> +
>  config PALMAS_GPADC
>   tristate "TI Palmas General Purpose ADC"
>   depends on MFD_PALMAS
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index 38638d4..3e60a1d 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -37,6 +37,7 @@ obj-$(CONFIG_PALMAS_GPADC) += palmas_gpadc.o
>  obj-$(CONFIG_QCOM_SPMI_IADC) += qcom-spmi-iadc.o
>  obj-$(CONFIG_QCOM_SPMI_VADC) += qcom-spmi-vadc.o
>  obj-$(CONFIG_ROCKCHIP_SARADC) += rockchip_saradc.o
> +obj-$(CONFIG_SUNXI_ADC) += sunxi-gpadc-iio.o
>  obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
>  obj-$(CONFIG_TI_ADC0832) += ti-adc0832.o
>  obj-$(CONFIG_TI_ADC128S052) += ti-adc128s052.o
> diff --git a/drivers/iio/adc/sunxi-gpadc-iio.c 
> b/drivers/iio/adc/sunxi-gpadc-iio.c
> new file mode 100644
> index 000..87cc913
> --- /dev/null
> +++ b/drivers/iio/adc/sunxi-gpadc-iio.c
> @@ -0,0 +1,417 @@
> +/* ADC driver for sunxi platforms
> + *
> + * Copyright (c) 2016 Quentin Schulz 
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published 
> by
> + * the Free Software Foundation.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define SUNXI_GPADC_TP_CTRL0 0x00
> +#define SUNXI_GPADC_TP_CTRL1 0x04
> +#define SUNXI_GPADC_TP_CTRL2 0x08
> +#define SUNXI_GPADC_TP_CTRL3 0x0c
> +#define SUNXI_GPADC_TP_TPR   0x18
> +#define SUNXI_GPADC_TP_CDAT  0x1c
> +#define SUNXI_GPADC_TEMP_DATA0x20
> +#define SUNXI_GPADC_TP_DATA  0x24
> +
> +/* TP_CTRL0 bits */
> +#define SUNXI_GPADC_ADC_FIRST_DLY(x) ((x) << 24) /* 8 bits */
> +#define SUNXI_GPADC_ADC_FIRST_DLY_MODE   BIT(23)
> +#define SUNXI_GPADC_ADC_CLK_SELECT   BIT(22)
> +#define SUNXI_GPADC_ADC_CLK_DIVIDER(x)   ((x) << 20) /* 2 bits */
> +#define SUNXI_GPADC_FS_DIV(x)((x) << 16) /* 4 bits */
> +#define SUNXI_GPADC_T_ACQ(x) ((x) << 0)  /* 16 bits */
> +
> +/* TP_CTRL1 bits */
> +#define SUNXI_GPADC_STYLUS_UP_DEBOUNCE(x)((x) << 12) /* 8 bits */
> +#define SUNXI_GPADC_STYLUS_UP_DEBOUNCE_ENBIT(9)
> +#define SUNXI_GPADC_TOUCH_PAN_CALI_EN  

Re: [PATCH v2 2/2] iio: adc: sun4i_lradc: new driver

[Jonathan Cameron]

On 12/07/16 20:04, Alexandre Belloni wrote:
> Add an IIO driver for the Allwinner Low Resolution ADC. This ADC is usually
> used for physical buttons connected using a resistor ladder.
> 
> Signed-off-by: Alexandre Belloni 
A few small bits and pieces from me...

Jonathan
> ---
> Changes in v2:
>  - prefixed defines with SUN4I_LRADC_
>  - removed sun4i_lradc_write_raw_get_fmt
>  - set indio_dev->dev.of_node
>  - pushed devm_iio_device_register() at the end of the probe function
>  - added a remove function to call regulator_disable()
> 
>  drivers/iio/adc/Kconfig   |  10 ++
>  drivers/iio/adc/Makefile  |   3 +-
>  drivers/iio/adc/sun4i_lradc.c | 323 
> ++
>  3 files changed, 335 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/iio/adc/sun4i_lradc.c
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 25378c5882e2..87aaac6a5f44 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -384,6 +384,16 @@ config ROCKCHIP_SARADC
> To compile this driver as a module, choose M here: the
> module will be called rockchip_saradc.
>  
> +config SUN4I_LRADC
> + tristate "Allwinner sun4i LRADC driver"
> + depends on ARCH_SUNXI || COMPILE_TEST
> + select IIO_TRIGGER
> + help
> +   Say yes here to build support for the LRADC found on Allwinner SoCs.
> +
> +   To compile this driver as a module, choose M here: the module will be
> +   called sun4i_lradc.
> +
>  config TI_ADC081C
>   tristate "Texas Instruments ADC081C/ADC101C/ADC121C family"
>   depends on I2C
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index 38638d46f972..a3b165af6bde 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -1,4 +1,4 @@
> -#
> +
??
>  # Makefile for IIO ADC drivers
>  #
>  
> @@ -37,6 +37,7 @@ obj-$(CONFIG_PALMAS_GPADC) += palmas_gpadc.o
>  obj-$(CONFIG_QCOM_SPMI_IADC) += qcom-spmi-iadc.o
>  obj-$(CONFIG_QCOM_SPMI_VADC) += qcom-spmi-vadc.o
>  obj-$(CONFIG_ROCKCHIP_SARADC) += rockchip_saradc.o
> +obj-$(CONFIG_SUN4I_LRADC) += sun4i_lradc.o
>  obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
>  obj-$(CONFIG_TI_ADC0832) += ti-adc0832.o
>  obj-$(CONFIG_TI_ADC128S052) += ti-adc128s052.o
> diff --git a/drivers/iio/adc/sun4i_lradc.c b/drivers/iio/adc/sun4i_lradc.c
> new file mode 100644
> index ..cd42e1a994b9
> --- /dev/null
> +++ b/drivers/iio/adc/sun4i_lradc.c
> @@ -0,0 +1,323 @@
> +/*
> + * Driver for the LRADC present on the  Allwinner sun4i
> + *
> + * Copyright 2016 Free Electrons
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that 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.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define SUN4I_LRADC_CTRL 0x00
> +#define SUN4I_LRADC_INTC 0x04
> +#define SUN4I_LRADC_INTS 0x08
> +#define SUN4I_LRADC_DATA00x0c
> +#define SUN4I_LRADC_DATA10x10
> +
> +/* LRADC_CTRL bits */
> +#define SUN4I_LRADC_FIRST_CONVERT_DLY(x) ((x) << 24) /* 8 bits */
> +#define SUN4I_LRADC_CHAN_SELECT(x)   ((x) << 22) /* 2 bits */
> +#define SUN4I_LRADC_CONTINUE_TIME_SEL(x) ((x) << 16) /* 4 bits */
> +#define SUN4I_LRADC_KEY_MODE_SEL(x)  ((x) << 12) /* 2 bits */
> +#define SUN4I_LRADC_LEVELA_B_CNT(x)  ((x) << 8)  /* 4 bits */
> +#define SUN4I_LRADC_HOLD_EN  BIT(6)
> +#define SUN4I_LRADC_LEVELB_VOL(x)((x) << 4)  /* 2 bits */
> +#define SUN4I_LRADC_SAMPLE_RATE(x)   ((x) << 2)  /* 2 bits */
> +#define SUN4I_LRADC_EN   BIT(0)
> +
> +/* LRADC_INTC and LRADC_INTS bits */
> +#define SUN4I_LRADC_CHAN1_KEYUP_IRQ  BIT(12)
> +#define SUN4I_LRADC_CHAN1_ALRDY_HOLD_IRQ BIT(11)
> +#define SUN4I_LRADC_CHAN1_HOLD_IRQ   BIT(10)
> +#define  SUN4I_LRADC_CHAN1_KEYDOWN_IRQ   BIT(9)
> +#define SUN4I_LRADC_CHAN1_DATA_IRQ   BIT(8)
> +#define SUN4I_LRADC_CHAN0_KEYUP_IRQ  BIT(4)
> +#define SUN4I_LRADC_CHAN0_ALRDY_HOLD_IRQ BIT(3)
> +#define SUN4I_LRADC_CHAN0_HOLD_IRQ   BIT(2)
> +#define  SUN4I_LRADC_CHAN0_KEYDOWN_IRQ   BIT(1)
> +#define SUN4I_LRADC_CHAN0_DATA_IRQ   BIT(0)
> +
> +#define NUM_SUN4I_LRADC_CHANS2
> +
> +struct sun4i_lradc_state {
> + void __iomem *base;
> + struct regulator *vref_supply;
> + u32 

Re: [PATCH 06/10] iio: cros_ec_light_prox: add ChromeOS EC Light and Proximity Sensors

[Jonathan Cameron]

On 18/07/16 08:02, Enric Balletbo i Serra wrote:
> Handle Light and Proximity sensors presented by the ChromeOS EC Sensor hub.
> Creates an IIO device for each functions.
> 
> Signed-off-by: Guenter Roeck 
> Signed-off-by: Enric Balletbo i Serra 
Few more nitpicks, but basically the same as the previous..

Jonathan
> ---
>  drivers/iio/common/cros_ec_sensors/Kconfig |   9 +
>  drivers/iio/common/cros_ec_sensors/Makefile|   1 +
>  .../common/cros_ec_sensors/cros_ec_light_prox.c| 288 
> +
>  3 files changed, 298 insertions(+)
>  create mode 100644 drivers/iio/common/cros_ec_sensors/cros_ec_light_prox.c
> 
> diff --git a/drivers/iio/common/cros_ec_sensors/Kconfig 
> b/drivers/iio/common/cros_ec_sensors/Kconfig
> index 18002d6..02559d2 100644
> --- a/drivers/iio/common/cros_ec_sensors/Kconfig
> +++ b/drivers/iio/common/cros_ec_sensors/Kconfig
> @@ -20,3 +20,12 @@ config IIO_CROS_EC_SENSORS
> Accelerometers, Gyroscope and Magnetometer that are
> presented by the ChromeOS EC Sensor hub.
> Creates an IIO device for each functions.
> +
> +config IIO_CROS_EC_LIGHT_PROX
> + tristate "ChromeOS EC Light and Proximity Sensors"
> + select IIO_CROS_EC_SENSORS_CORE
> + help
> +   Module to handle Light and Proximity sensors
> +   presented by the ChromeOS EC Sensor hub.
> +   Creates an IIO device for each functions.
> +   Only one source is exposed by the EC.
> diff --git a/drivers/iio/common/cros_ec_sensors/Makefile 
> b/drivers/iio/common/cros_ec_sensors/Makefile
> index ec716ff..7aaf2a2 100644
> --- a/drivers/iio/common/cros_ec_sensors/Makefile
> +++ b/drivers/iio/common/cros_ec_sensors/Makefile
> @@ -4,3 +4,4 @@
>  
>  obj-$(CONFIG_IIO_CROS_EC_SENSORS_CORE) += cros_ec_sensors_core.o
>  obj-$(CONFIG_IIO_CROS_EC_SENSORS) += cros_ec_sensors.o
> +obj-$(CONFIG_IIO_CROS_EC_LIGHT_PROX) += cros_ec_light_prox.o
> diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_light_prox.c 
> b/drivers/iio/common/cros_ec_sensors/cros_ec_light_prox.c
> new file mode 100644
> index 000..333e927
> --- /dev/null
> +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_light_prox.c
> @@ -0,0 +1,288 @@
> +/*
> + * cros_ec_light_proxmity - Driver for light and prox sensors behing CrOS EC.
> + *
> + * Copyright (C) 2015 Google, Inc
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that 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.
> + *
> + * This driver uses the cros-ec interface to communicate with the Chrome OS
> + * EC about accelerometer data. Accelerometer access is presented through
> + * iio sysfs.
Fix this comment as I doubt this driver does that ;)
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "cros_ec_sensors_core.h"
> +
> +/*
> + * We only represent one entry for light or proximity.
> + * EC is merging different light sensors to return the
> + * what the eye would see.
> + * For proximity, we currently support only one light source.
> + */
> +#define MAX_CHANNELS (1 + 1)
> +
> +/* State data for ec_sensors iio driver. */
> +struct cros_ec_sensors_state {
> + /* Shared by all sensors */
> + struct cros_ec_sensors_core_state core;
> +
> + struct iio_chan_spec channels[MAX_CHANNELS];
> +};
> +
> +static int cros_ec_light_prox_read(struct iio_dev *indio_dev,
> +   struct iio_chan_spec const *chan,
> +   int *val, int *val2, long mask)
> +{
> + struct cros_ec_sensors_state *st = iio_priv(indio_dev);
> + u16 data = 0;
> + s64 val64;
> + int ret = IIO_VAL_INT;
> + int idx = chan->scan_index;
> +
> + mutex_lock(>core.cmd_lock);
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + if (cros_ec_sensors_read_cmd(indio_dev, 1 << idx,
> + (s16 *)) < 0)
> + ret = -EIO;
> + *val = data;
> + break;
> + case IIO_CHAN_INFO_CALIBBIAS:
> + st->core.param.cmd = MOTIONSENSE_CMD_SENSOR_OFFSET;
> + st->core.param.sensor_offset.flags = 0;
> +
> + if (cros_ec_motion_send_host_cmd(>core, 0)) {
> + ret = -EIO;
> + break;
> + }
> +
> + /* Save values */
> + st->core.calib[0].offset =
> + st->core.resp->sensor_offset.offset[0];
> +
> + *val = 

Re: [PATCH v4 5/8] drm/mediatek: add dsi interrupt control

[CK Hu]

Hi, YT:

Some comments inline.

On Fri, 2016-07-15 at 18:07 +0800, YT Shen wrote:
> From: shaoming chen 
> 
> add dsi interrupt control
> 
> Signed-off-by: shaoming chen 
> ---
>  drivers/gpu/drm/mediatek/mtk_dsi.c |  130 
> 
>  1 file changed, 130 insertions(+)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c 
> b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index 2d808e5..de5ad7f 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> @@ -18,6 +18,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -32,6 +33,13 @@
>  
>  #define DSI_START0x00
>  
> +#define DSI_INTEN0x08
> +
> +#define DSI_INTSTA   0x0c
> +#define LPRX_RD_RDY_INT_FLAG BIT(0)
> +#define CMD_DONE_INT_FLAGBIT(1)
> +#define DSI_BUSY BIT(31)

Why need LPRX_RD_RDY_INT_FLAG, CMD_DONE_INT_FLAG, and DSI_BUSY? Maybe
these three should be moved to other patch.

> +
>  #define DSI_CON_CTRL 0x10
>  #define DSI_RESETBIT(0)
>  #define DSI_EN   BIT(1)
> @@ -74,6 +82,9 @@
>  
>  #define DSI_HSTX_CKL_WC  0x64
>  
> +#define DSI_RACK 0x84
> +#define RACK BIT(0)
> +
>  #define DSI_PHY_LCCON0x104
>  #define LC_HS_TX_EN  BIT(0)
>  #define LC_ULPM_EN   BIT(1)
> @@ -134,6 +145,18 @@ struct mtk_dsi {
>   struct videomode vm;
>   int refcount;
>   bool enabled;
> + int irq_num, irq_data;
> +};
> +
> +enum {
> + DSI_INT_SLEEPOUT_DONE_FLAG  = BIT(6),
> + DSI_INT_VM_CMD_DONE_FLAG= BIT(5),
> + DSI_INT_EXT_TE_RDY_FLAG = BIT(4),
> + DSI_INT_VM_DONE_FLAG= BIT(3),
> + DSI_INT_TE_RDY_FLAG = BIT(2),
> + DSI_INT_CMD_DONE_FLAG   = BIT(1),
> + DSI_INT_LPRX_RD_RDY_FLAG= BIT(0),
> + DSI_INT_ALL_BITS= (0x7f)
>  };

I think you should use '#define' instead of 'enum'. The code would be
like below, and these definition should be moved to after DSI_INTEN or
DSI_INTSTA.

#define DSI_INT_LPRX_RD_RDY_FLAGBIT(0)
#define DSI_INT_CMD_DONE_FLAG   BIT(1)
#define DSI_INT_TE_RDY_FLAG BIT(2)
#define DSI_INT_VM_DONE_FLAGBIT(3)
#define DSI_INT_EXT_TE_RDY_FLAG BIT(4)
#define DSI_INT_VM_CMD_DONE_FLAGBIT(5)
#define DSI_INT_SLEEPOUT_DONE_FLAG  BIT(6)
#define DSI_INT_ALL_BITS(DSI_INT_LPRX_RD_RDY_FLAG | \
DSI_INT_CMD_DONE_FLAG | \
DSI_INT_TE_RDY_FLAG | \
DSI_INT_VM_DONE_FLAG | \
DSI_INT_EXT_TE_RDY_FLAG | \
DSI_INT_VM_CMD_DONE_FLAG | \
DSI_INT_SLEEPOUT_DONE_FLAG)

>  
>  static inline struct mtk_dsi *encoder_to_dsi(struct drm_encoder *e)
> @@ -440,6 +463,94 @@ static void mtk_dsi_start(struct mtk_dsi *dsi)
>   writel(1, dsi->regs + DSI_START);
>  }
>  
> +static void mtk_dsi_set_interrupt_enable(struct mtk_dsi *dsi)
> +{
> + u32 inten = DSI_INT_ALL_BITS;
> +
> + if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO)
> + inten &= ~(DSI_INT_TE_RDY_FLAG | DSI_INT_EXT_TE_RDY_FLAG);
> +
> + writel(inten, dsi->regs + DSI_INTEN);
> +}
> +
> +static void mtk_dsi_irq_wakeup(struct mtk_dsi *dsi, u32 irq_bit)
> +{
> + dsi->irq_data |= irq_bit;
> +}
> +
> +static irqreturn_t mtk_dsi_irq(int irq, void *dev_id)
> +{
> + struct mtk_dsi *dsi = dev_id;
> +
> + u32 status, tmp;
> +
> + status = readl(dsi->regs + DSI_INTSTA);
> +
> + if (status & DSI_INT_LPRX_RD_RDY_FLAG) {
> + /* write clear RD_RDY interrupt */
> + /* write clear RD_RDY interrupt must be before DSI_RACK */
> + /* because CMD_DONE will raise after DSI_RACK, */
> + /* so write clear RD_RDY after that will clear CMD_DONE too */
> + do {
> + /* send read ACK */
> + mtk_dsi_mask(dsi, DSI_RACK, RACK, RACK);
> + tmp = readl(dsi->regs + DSI_INTSTA);
> + } while (tmp & DSI_BUSY);
> +
> + mtk_dsi_mask(dsi, DSI_INTSTA, DSI_INT_LPRX_RD_RDY_FLAG, 0);
> + mtk_dsi_irq_wakeup(dsi, DSI_INT_LPRX_RD_RDY_FLAG);
> + }
> +
> + if (status & DSI_INT_CMD_DONE_FLAG) {
> + mtk_dsi_mask(dsi, DSI_INTSTA, DSI_INT_CMD_DONE_FLAG, 0);
> + mtk_dsi_irq_wakeup(dsi, DSI_INT_CMD_DONE_FLAG);
> + }
> +
> + if (status & DSI_INT_TE_RDY_FLAG) {
> + mtk_dsi_mask(dsi, DSI_INTSTA, DSI_INT_TE_RDY_FLAG, 0);
> + mtk_dsi_irq_wakeup(dsi, DSI_INT_TE_RDY_FLAG);
> + }
> +
> + if (status & DSI_INT_VM_DONE_FLAG) {
> + mtk_dsi_mask(dsi, 

Re: [PATCH v2 2/4] iio: adc: add support for Allwinner SoCs ADC

[Jonathan Cameron]

On 15/07/16 10:59, Quentin Schulz wrote:
> The Allwinner SoCs all have an ADC that can also act as a touchscreen
> controller and a thermal sensor. This patch adds the ADC driver which is
> based on the MFD for the same SoCs ADC.
> 
> This also registers the thermal adc channel in the iio map array so
> iio_hwmon could use it without modifying the Device Tree.
> 
> This driver probes on three different platform_device_id to take into
> account slight differences between Allwinner SoCs ADCs.
> 
> Signed-off-by: Quentin Schulz 

Hi Quentin,

Various bits inline.  In particular the irq handling looks flakey / racey
to me.  Definitely need some explanatory comments.

Also another note on the craziness that using extended_name to provide
the hwmon labels will cause :)

Jonathan
> ---
> 
> v2:
>  - add SUNXI_GPADC_ prefixes for defines,
>  - correct typo in Kconfig,
>  - reorder alphabetically includes, makefile,
>  - add license header,
>  - fix architecture variations not being handled in interrupt handlers or
>read raw functions,
>  - fix unability to return negative values from thermal sensor,
>  - add gotos to reduce code repetition,
>  - fix irq variable being unsigned int instead of int,
>  - remove useless dev_err and dev_info,
>  - deactivate all interrupts if probe fails,
>  - fix iio_device_register on NULL variable,
>  - deactivate ADC in the IP when probe fails or when removing driver,
> 
>  drivers/iio/adc/Kconfig   |  12 ++
>  drivers/iio/adc/Makefile  |   1 +
>  drivers/iio/adc/sunxi-gpadc-iio.c | 417 
> ++
>  3 files changed, 430 insertions(+)
>  create mode 100644 drivers/iio/adc/sunxi-gpadc-iio.c
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 25378c5..184856f 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -338,6 +338,18 @@ config NAU7802
> To compile this driver as a module, choose M here: the
> module will be called nau7802.
>  
> +config SUNXI_ADC
> + tristate "ADC driver for sunxi platforms"
> + depends on IIO
> + depends on MFD_SUNXI_ADC
> + help
> +   Say yes here to build support for Allwinner (A10, A13 and A31) SoCs
> +   ADC. This ADC provides 4 channels which can be used as an ADC or as a
> +   touchscreen input and one channel for thermal sensor.
> +
> +  To compile this driver as a module, choose M here: the module will 
> be
> +   called sunxi-gpadc-iio.
> +
>  config PALMAS_GPADC
>   tristate "TI Palmas General Purpose ADC"
>   depends on MFD_PALMAS
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index 38638d4..3e60a1d 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -37,6 +37,7 @@ obj-$(CONFIG_PALMAS_GPADC) += palmas_gpadc.o
>  obj-$(CONFIG_QCOM_SPMI_IADC) += qcom-spmi-iadc.o
>  obj-$(CONFIG_QCOM_SPMI_VADC) += qcom-spmi-vadc.o
>  obj-$(CONFIG_ROCKCHIP_SARADC) += rockchip_saradc.o
> +obj-$(CONFIG_SUNXI_ADC) += sunxi-gpadc-iio.o
>  obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
>  obj-$(CONFIG_TI_ADC0832) += ti-adc0832.o
>  obj-$(CONFIG_TI_ADC128S052) += ti-adc128s052.o
> diff --git a/drivers/iio/adc/sunxi-gpadc-iio.c 
> b/drivers/iio/adc/sunxi-gpadc-iio.c
> new file mode 100644
> index 000..87cc913
> --- /dev/null
> +++ b/drivers/iio/adc/sunxi-gpadc-iio.c
> @@ -0,0 +1,417 @@
> +/* ADC driver for sunxi platforms
> + *
> + * Copyright (c) 2016 Quentin Schulz 
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published 
> by
> + * the Free Software Foundation.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define SUNXI_GPADC_TP_CTRL0 0x00
> +#define SUNXI_GPADC_TP_CTRL1 0x04
> +#define SUNXI_GPADC_TP_CTRL2 0x08
> +#define SUNXI_GPADC_TP_CTRL3 0x0c
> +#define SUNXI_GPADC_TP_TPR   0x18
> +#define SUNXI_GPADC_TP_CDAT  0x1c
> +#define SUNXI_GPADC_TEMP_DATA0x20
> +#define SUNXI_GPADC_TP_DATA  0x24
> +
> +/* TP_CTRL0 bits */
> +#define SUNXI_GPADC_ADC_FIRST_DLY(x) ((x) << 24) /* 8 bits */
> +#define SUNXI_GPADC_ADC_FIRST_DLY_MODE   BIT(23)
> +#define SUNXI_GPADC_ADC_CLK_SELECT   BIT(22)
> +#define SUNXI_GPADC_ADC_CLK_DIVIDER(x)   ((x) << 20) /* 2 bits */
> +#define SUNXI_GPADC_FS_DIV(x)((x) << 16) /* 4 bits */
> +#define SUNXI_GPADC_T_ACQ(x) ((x) << 0)  /* 16 bits */
> +
> +/* TP_CTRL1 bits */
> +#define SUNXI_GPADC_STYLUS_UP_DEBOUNCE(x)((x) << 12) /* 8 bits */
> +#define SUNXI_GPADC_STYLUS_UP_DEBOUNCE_ENBIT(9)
> +#define SUNXI_GPADC_TOUCH_PAN_CALI_ENBIT(6)
> +#define SUNXI_GPADC_TP_DUAL_EN

Re: [PATCH v2 2/2] iio: adc: sun4i_lradc: new driver

[Jonathan Cameron]

On 12/07/16 20:04, Alexandre Belloni wrote:
> Add an IIO driver for the Allwinner Low Resolution ADC. This ADC is usually
> used for physical buttons connected using a resistor ladder.
> 
> Signed-off-by: Alexandre Belloni 
A few small bits and pieces from me...

Jonathan
> ---
> Changes in v2:
>  - prefixed defines with SUN4I_LRADC_
>  - removed sun4i_lradc_write_raw_get_fmt
>  - set indio_dev->dev.of_node
>  - pushed devm_iio_device_register() at the end of the probe function
>  - added a remove function to call regulator_disable()
> 
>  drivers/iio/adc/Kconfig   |  10 ++
>  drivers/iio/adc/Makefile  |   3 +-
>  drivers/iio/adc/sun4i_lradc.c | 323 
> ++
>  3 files changed, 335 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/iio/adc/sun4i_lradc.c
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 25378c5882e2..87aaac6a5f44 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -384,6 +384,16 @@ config ROCKCHIP_SARADC
> To compile this driver as a module, choose M here: the
> module will be called rockchip_saradc.
>  
> +config SUN4I_LRADC
> + tristate "Allwinner sun4i LRADC driver"
> + depends on ARCH_SUNXI || COMPILE_TEST
> + select IIO_TRIGGER
> + help
> +   Say yes here to build support for the LRADC found on Allwinner SoCs.
> +
> +   To compile this driver as a module, choose M here: the module will be
> +   called sun4i_lradc.
> +
>  config TI_ADC081C
>   tristate "Texas Instruments ADC081C/ADC101C/ADC121C family"
>   depends on I2C
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index 38638d46f972..a3b165af6bde 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -1,4 +1,4 @@
> -#
> +
??
>  # Makefile for IIO ADC drivers
>  #
>  
> @@ -37,6 +37,7 @@ obj-$(CONFIG_PALMAS_GPADC) += palmas_gpadc.o
>  obj-$(CONFIG_QCOM_SPMI_IADC) += qcom-spmi-iadc.o
>  obj-$(CONFIG_QCOM_SPMI_VADC) += qcom-spmi-vadc.o
>  obj-$(CONFIG_ROCKCHIP_SARADC) += rockchip_saradc.o
> +obj-$(CONFIG_SUN4I_LRADC) += sun4i_lradc.o
>  obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
>  obj-$(CONFIG_TI_ADC0832) += ti-adc0832.o
>  obj-$(CONFIG_TI_ADC128S052) += ti-adc128s052.o
> diff --git a/drivers/iio/adc/sun4i_lradc.c b/drivers/iio/adc/sun4i_lradc.c
> new file mode 100644
> index ..cd42e1a994b9
> --- /dev/null
> +++ b/drivers/iio/adc/sun4i_lradc.c
> @@ -0,0 +1,323 @@
> +/*
> + * Driver for the LRADC present on the  Allwinner sun4i
> + *
> + * Copyright 2016 Free Electrons
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that 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.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define SUN4I_LRADC_CTRL 0x00
> +#define SUN4I_LRADC_INTC 0x04
> +#define SUN4I_LRADC_INTS 0x08
> +#define SUN4I_LRADC_DATA00x0c
> +#define SUN4I_LRADC_DATA10x10
> +
> +/* LRADC_CTRL bits */
> +#define SUN4I_LRADC_FIRST_CONVERT_DLY(x) ((x) << 24) /* 8 bits */
> +#define SUN4I_LRADC_CHAN_SELECT(x)   ((x) << 22) /* 2 bits */
> +#define SUN4I_LRADC_CONTINUE_TIME_SEL(x) ((x) << 16) /* 4 bits */
> +#define SUN4I_LRADC_KEY_MODE_SEL(x)  ((x) << 12) /* 2 bits */
> +#define SUN4I_LRADC_LEVELA_B_CNT(x)  ((x) << 8)  /* 4 bits */
> +#define SUN4I_LRADC_HOLD_EN  BIT(6)
> +#define SUN4I_LRADC_LEVELB_VOL(x)((x) << 4)  /* 2 bits */
> +#define SUN4I_LRADC_SAMPLE_RATE(x)   ((x) << 2)  /* 2 bits */
> +#define SUN4I_LRADC_EN   BIT(0)
> +
> +/* LRADC_INTC and LRADC_INTS bits */
> +#define SUN4I_LRADC_CHAN1_KEYUP_IRQ  BIT(12)
> +#define SUN4I_LRADC_CHAN1_ALRDY_HOLD_IRQ BIT(11)
> +#define SUN4I_LRADC_CHAN1_HOLD_IRQ   BIT(10)
> +#define  SUN4I_LRADC_CHAN1_KEYDOWN_IRQ   BIT(9)
> +#define SUN4I_LRADC_CHAN1_DATA_IRQ   BIT(8)
> +#define SUN4I_LRADC_CHAN0_KEYUP_IRQ  BIT(4)
> +#define SUN4I_LRADC_CHAN0_ALRDY_HOLD_IRQ BIT(3)
> +#define SUN4I_LRADC_CHAN0_HOLD_IRQ   BIT(2)
> +#define  SUN4I_LRADC_CHAN0_KEYDOWN_IRQ   BIT(1)
> +#define SUN4I_LRADC_CHAN0_DATA_IRQ   BIT(0)
> +
> +#define NUM_SUN4I_LRADC_CHANS2
> +
> +struct sun4i_lradc_state {
> + void __iomem *base;
> + struct regulator *vref_supply;
> + u32 vref_mv;
> + struct completion 

Re: [PATCH 06/10] iio: cros_ec_light_prox: add ChromeOS EC Light and Proximity Sensors

[Jonathan Cameron]

On 18/07/16 08:02, Enric Balletbo i Serra wrote:
> Handle Light and Proximity sensors presented by the ChromeOS EC Sensor hub.
> Creates an IIO device for each functions.
> 
> Signed-off-by: Guenter Roeck 
> Signed-off-by: Enric Balletbo i Serra 
Few more nitpicks, but basically the same as the previous..

Jonathan
> ---
>  drivers/iio/common/cros_ec_sensors/Kconfig |   9 +
>  drivers/iio/common/cros_ec_sensors/Makefile|   1 +
>  .../common/cros_ec_sensors/cros_ec_light_prox.c| 288 
> +
>  3 files changed, 298 insertions(+)
>  create mode 100644 drivers/iio/common/cros_ec_sensors/cros_ec_light_prox.c
> 
> diff --git a/drivers/iio/common/cros_ec_sensors/Kconfig 
> b/drivers/iio/common/cros_ec_sensors/Kconfig
> index 18002d6..02559d2 100644
> --- a/drivers/iio/common/cros_ec_sensors/Kconfig
> +++ b/drivers/iio/common/cros_ec_sensors/Kconfig
> @@ -20,3 +20,12 @@ config IIO_CROS_EC_SENSORS
> Accelerometers, Gyroscope and Magnetometer that are
> presented by the ChromeOS EC Sensor hub.
> Creates an IIO device for each functions.
> +
> +config IIO_CROS_EC_LIGHT_PROX
> + tristate "ChromeOS EC Light and Proximity Sensors"
> + select IIO_CROS_EC_SENSORS_CORE
> + help
> +   Module to handle Light and Proximity sensors
> +   presented by the ChromeOS EC Sensor hub.
> +   Creates an IIO device for each functions.
> +   Only one source is exposed by the EC.
> diff --git a/drivers/iio/common/cros_ec_sensors/Makefile 
> b/drivers/iio/common/cros_ec_sensors/Makefile
> index ec716ff..7aaf2a2 100644
> --- a/drivers/iio/common/cros_ec_sensors/Makefile
> +++ b/drivers/iio/common/cros_ec_sensors/Makefile
> @@ -4,3 +4,4 @@
>  
>  obj-$(CONFIG_IIO_CROS_EC_SENSORS_CORE) += cros_ec_sensors_core.o
>  obj-$(CONFIG_IIO_CROS_EC_SENSORS) += cros_ec_sensors.o
> +obj-$(CONFIG_IIO_CROS_EC_LIGHT_PROX) += cros_ec_light_prox.o
> diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_light_prox.c 
> b/drivers/iio/common/cros_ec_sensors/cros_ec_light_prox.c
> new file mode 100644
> index 000..333e927
> --- /dev/null
> +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_light_prox.c
> @@ -0,0 +1,288 @@
> +/*
> + * cros_ec_light_proxmity - Driver for light and prox sensors behing CrOS EC.
> + *
> + * Copyright (C) 2015 Google, Inc
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that 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.
> + *
> + * This driver uses the cros-ec interface to communicate with the Chrome OS
> + * EC about accelerometer data. Accelerometer access is presented through
> + * iio sysfs.
Fix this comment as I doubt this driver does that ;)
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "cros_ec_sensors_core.h"
> +
> +/*
> + * We only represent one entry for light or proximity.
> + * EC is merging different light sensors to return the
> + * what the eye would see.
> + * For proximity, we currently support only one light source.
> + */
> +#define MAX_CHANNELS (1 + 1)
> +
> +/* State data for ec_sensors iio driver. */
> +struct cros_ec_sensors_state {
> + /* Shared by all sensors */
> + struct cros_ec_sensors_core_state core;
> +
> + struct iio_chan_spec channels[MAX_CHANNELS];
> +};
> +
> +static int cros_ec_light_prox_read(struct iio_dev *indio_dev,
> +   struct iio_chan_spec const *chan,
> +   int *val, int *val2, long mask)
> +{
> + struct cros_ec_sensors_state *st = iio_priv(indio_dev);
> + u16 data = 0;
> + s64 val64;
> + int ret = IIO_VAL_INT;
> + int idx = chan->scan_index;
> +
> + mutex_lock(>core.cmd_lock);
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + if (cros_ec_sensors_read_cmd(indio_dev, 1 << idx,
> + (s16 *)) < 0)
> + ret = -EIO;
> + *val = data;
> + break;
> + case IIO_CHAN_INFO_CALIBBIAS:
> + st->core.param.cmd = MOTIONSENSE_CMD_SENSOR_OFFSET;
> + st->core.param.sensor_offset.flags = 0;
> +
> + if (cros_ec_motion_send_host_cmd(>core, 0)) {
> + ret = -EIO;
> + break;
> + }
> +
> + /* Save values */
> + st->core.calib[0].offset =
> + st->core.resp->sensor_offset.offset[0];
> +
> + *val = st->core.calib[idx].offset;
> + break;
> + 

Re: [PATCH v4 5/8] drm/mediatek: add dsi interrupt control

[CK Hu]

Hi, YT:

Some comments inline.

On Fri, 2016-07-15 at 18:07 +0800, YT Shen wrote:
> From: shaoming chen 
> 
> add dsi interrupt control
> 
> Signed-off-by: shaoming chen 
> ---
>  drivers/gpu/drm/mediatek/mtk_dsi.c |  130 
> 
>  1 file changed, 130 insertions(+)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c 
> b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index 2d808e5..de5ad7f 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> @@ -18,6 +18,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -32,6 +33,13 @@
>  
>  #define DSI_START0x00
>  
> +#define DSI_INTEN0x08
> +
> +#define DSI_INTSTA   0x0c
> +#define LPRX_RD_RDY_INT_FLAG BIT(0)
> +#define CMD_DONE_INT_FLAGBIT(1)
> +#define DSI_BUSY BIT(31)

Why need LPRX_RD_RDY_INT_FLAG, CMD_DONE_INT_FLAG, and DSI_BUSY? Maybe
these three should be moved to other patch.

> +
>  #define DSI_CON_CTRL 0x10
>  #define DSI_RESETBIT(0)
>  #define DSI_EN   BIT(1)
> @@ -74,6 +82,9 @@
>  
>  #define DSI_HSTX_CKL_WC  0x64
>  
> +#define DSI_RACK 0x84
> +#define RACK BIT(0)
> +
>  #define DSI_PHY_LCCON0x104
>  #define LC_HS_TX_EN  BIT(0)
>  #define LC_ULPM_EN   BIT(1)
> @@ -134,6 +145,18 @@ struct mtk_dsi {
>   struct videomode vm;
>   int refcount;
>   bool enabled;
> + int irq_num, irq_data;
> +};
> +
> +enum {
> + DSI_INT_SLEEPOUT_DONE_FLAG  = BIT(6),
> + DSI_INT_VM_CMD_DONE_FLAG= BIT(5),
> + DSI_INT_EXT_TE_RDY_FLAG = BIT(4),
> + DSI_INT_VM_DONE_FLAG= BIT(3),
> + DSI_INT_TE_RDY_FLAG = BIT(2),
> + DSI_INT_CMD_DONE_FLAG   = BIT(1),
> + DSI_INT_LPRX_RD_RDY_FLAG= BIT(0),
> + DSI_INT_ALL_BITS= (0x7f)
>  };

I think you should use '#define' instead of 'enum'. The code would be
like below, and these definition should be moved to after DSI_INTEN or
DSI_INTSTA.

#define DSI_INT_LPRX_RD_RDY_FLAGBIT(0)
#define DSI_INT_CMD_DONE_FLAG   BIT(1)
#define DSI_INT_TE_RDY_FLAG BIT(2)
#define DSI_INT_VM_DONE_FLAGBIT(3)
#define DSI_INT_EXT_TE_RDY_FLAG BIT(4)
#define DSI_INT_VM_CMD_DONE_FLAGBIT(5)
#define DSI_INT_SLEEPOUT_DONE_FLAG  BIT(6)
#define DSI_INT_ALL_BITS(DSI_INT_LPRX_RD_RDY_FLAG | \
DSI_INT_CMD_DONE_FLAG | \
DSI_INT_TE_RDY_FLAG | \
DSI_INT_VM_DONE_FLAG | \
DSI_INT_EXT_TE_RDY_FLAG | \
DSI_INT_VM_CMD_DONE_FLAG | \
DSI_INT_SLEEPOUT_DONE_FLAG)

>  
>  static inline struct mtk_dsi *encoder_to_dsi(struct drm_encoder *e)
> @@ -440,6 +463,94 @@ static void mtk_dsi_start(struct mtk_dsi *dsi)
>   writel(1, dsi->regs + DSI_START);
>  }
>  
> +static void mtk_dsi_set_interrupt_enable(struct mtk_dsi *dsi)
> +{
> + u32 inten = DSI_INT_ALL_BITS;
> +
> + if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO)
> + inten &= ~(DSI_INT_TE_RDY_FLAG | DSI_INT_EXT_TE_RDY_FLAG);
> +
> + writel(inten, dsi->regs + DSI_INTEN);
> +}
> +
> +static void mtk_dsi_irq_wakeup(struct mtk_dsi *dsi, u32 irq_bit)
> +{
> + dsi->irq_data |= irq_bit;
> +}
> +
> +static irqreturn_t mtk_dsi_irq(int irq, void *dev_id)
> +{
> + struct mtk_dsi *dsi = dev_id;
> +
> + u32 status, tmp;
> +
> + status = readl(dsi->regs + DSI_INTSTA);
> +
> + if (status & DSI_INT_LPRX_RD_RDY_FLAG) {
> + /* write clear RD_RDY interrupt */
> + /* write clear RD_RDY interrupt must be before DSI_RACK */
> + /* because CMD_DONE will raise after DSI_RACK, */
> + /* so write clear RD_RDY after that will clear CMD_DONE too */
> + do {
> + /* send read ACK */
> + mtk_dsi_mask(dsi, DSI_RACK, RACK, RACK);
> + tmp = readl(dsi->regs + DSI_INTSTA);
> + } while (tmp & DSI_BUSY);
> +
> + mtk_dsi_mask(dsi, DSI_INTSTA, DSI_INT_LPRX_RD_RDY_FLAG, 0);
> + mtk_dsi_irq_wakeup(dsi, DSI_INT_LPRX_RD_RDY_FLAG);
> + }
> +
> + if (status & DSI_INT_CMD_DONE_FLAG) {
> + mtk_dsi_mask(dsi, DSI_INTSTA, DSI_INT_CMD_DONE_FLAG, 0);
> + mtk_dsi_irq_wakeup(dsi, DSI_INT_CMD_DONE_FLAG);
> + }
> +
> + if (status & DSI_INT_TE_RDY_FLAG) {
> + mtk_dsi_mask(dsi, DSI_INTSTA, DSI_INT_TE_RDY_FLAG, 0);
> + mtk_dsi_irq_wakeup(dsi, DSI_INT_TE_RDY_FLAG);
> + }
> +
> + if (status & DSI_INT_VM_DONE_FLAG) {
> + mtk_dsi_mask(dsi, DSI_INTSTA, DSI_INT_VM_DONE_FLAG, 0);
> + 

Re: [PATCH 04/10] iio: cros_ec: Add common functions for cros_ec sensors.

[Jonathan Cameron]

On 18/07/16 08:29, Peter Meerwald-Stadler wrote:
> 
>> Add the core functions to be able to support the sensors attached behind
>> the ChromeOS Embedded Controller and used by other IIO cros-ec sensor
>> drivers.
> 
> comments below from a quick read
> there is plenty on undocumented private API
Few more comments from me. Peter is of course quite correct, its the
new, undocumented ABI, which is the biggest issue.

Interesting looking device.  Any docs out there?

Jonathan
>  
>> The cros_ec_sensor_core driver matches with current driver in ChromeOS
>> 4.4 tree, so it includes all the fixes at the moment. The support for
>> this driver was made by Gwendal Grignou. The original patch and all the
>> fixes has been squashed and rebased on top of mainline.
>>
>> Signed-off-by: Gwendal Grignou 
>> Signed-off-by: Guenter Roeck 
>> [eballetbo: split, squash and rebase on top of mainline the patches
>> found in ChromeOS tree]
>> Signed-off-by: Enric Balletbo i Serra 
>> ---
>>  drivers/iio/common/Kconfig |   1 +
>>  drivers/iio/common/Makefile|   1 +
>>  drivers/iio/common/cros_ec_sensors/Kconfig |  14 +
>>  drivers/iio/common/cros_ec_sensors/Makefile|   5 +
>>  .../common/cros_ec_sensors/cros_ec_sensors_core.c  | 564 
>> +
>>  .../common/cros_ec_sensors/cros_ec_sensors_core.h  | 155 ++
>>  6 files changed, 740 insertions(+)
>>  create mode 100644 drivers/iio/common/cros_ec_sensors/Kconfig
>>  create mode 100644 drivers/iio/common/cros_ec_sensors/Makefile
>>  create mode 100644 drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
>>  create mode 100644 drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.h
>>
>> diff --git a/drivers/iio/common/Kconfig b/drivers/iio/common/Kconfig
>> index 26a6026..e108996 100644
>> --- a/drivers/iio/common/Kconfig
>> +++ b/drivers/iio/common/Kconfig
>> @@ -2,6 +2,7 @@
>>  # IIO common modules
>>  #
>>  
>> +source "drivers/iio/common/cros_ec_sensors/Kconfig"
>>  source "drivers/iio/common/hid-sensors/Kconfig"
>>  source "drivers/iio/common/ms_sensors/Kconfig"
>>  source "drivers/iio/common/ssp_sensors/Kconfig"
>> diff --git a/drivers/iio/common/Makefile b/drivers/iio/common/Makefile
>> index 585da6a..6fa760e 100644
>> --- a/drivers/iio/common/Makefile
>> +++ b/drivers/iio/common/Makefile
>> @@ -7,6 +7,7 @@
>>  #
>>  
>>  # When adding new entries keep the list in alphabetical order
>> +obj-y += cros_ec_sensors/
>>  obj-y += hid-sensors/
>>  obj-y += ms_sensors/
>>  obj-y += ssp_sensors/
>> diff --git a/drivers/iio/common/cros_ec_sensors/Kconfig 
>> b/drivers/iio/common/cros_ec_sensors/Kconfig
>> new file mode 100644
>> index 000..a30f41e
>> --- /dev/null
>> +++ b/drivers/iio/common/cros_ec_sensors/Kconfig
>> @@ -0,0 +1,14 @@
>> +#
>> +# Chrome OS Embedded Controller managed sensors library
>> +#
>> +config IIO_CROS_EC_SENSORS_CORE
>> +tristate "ChromeOS EC Sensors Core"
>> +depends on SYSFS && MFD_CROS_EC
>> +select IIO_BUFFER
>> +select IIO_TRIGGERED_BUFFER
>> +help
>> +  Base module for the ChromeOS EC Sensors module.
>> +  Contains core functions used by other IIO CrosEC sensor
>> +  driver.
> 
> 'drivers' probably
> 
>> +  Define common attributes and sysfs interrupt handler.
>> +
>> diff --git a/drivers/iio/common/cros_ec_sensors/Makefile 
>> b/drivers/iio/common/cros_ec_sensors/Makefile
>> new file mode 100644
>> index 000..95b6901
>> --- /dev/null
>> +++ b/drivers/iio/common/cros_ec_sensors/Makefile
>> @@ -0,0 +1,5 @@
>> +#
>> +# Makefile for sensors seen through the ChromeOS EC sensor hub.
>> +#
>> +
>> +obj-$(CONFIG_IIO_CROS_EC_SENSORS_CORE) += cros_ec_sensors_core.o
>> diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c 
>> b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
>> new file mode 100644
>> index 000..cb3de8f
>> --- /dev/null
>> +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
>> @@ -0,0 +1,564 @@
>> +/*
>> + * cros_ec_sensors_core - Common function for Chrome OS EC sensor driver.
>> + *
>> + * Copyright (C) 2015 Google, Inc
>> + *
>> + * This software is licensed under the terms of the GNU General Public
>> + * License version 2, as published by the Free Software Foundation, and
>> + * may be copied, distributed, and modified under those terms.
>> + *
>> + * This program is distributed in the hope that 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.
>> + *
>> + * This driver uses the cros-ec interface to communicate with the Chrome OS
>> + * EC about accelerometer data. Accelerometer access is presented through
>> + * iio sysfs.
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> 

Re: [PATCH 02/10] mfd: cros_ec: update MOTIONSENSE definitions and commands.

[Jonathan Cameron]

On 18/07/16 08:02, Enric Balletbo i Serra wrote:
> Let's update the command header to include the definitions related to
> the sensors attached behind the ChromeOS Embedded Controller. The new
> commands and definitions allow us to get information from these sensors.
> 
> Signed-off-by: Gwendal Grignou 
> Signed-off-by: Enric Balletbo i Serra 
Again, I'd be happier seeing this stuff introduced as and when it
is needed rather than in a magic patch. It's hard to review stuff
if it's broken up across multiple patches like this.

A few other bits and pieces inline.

Jonathan
> ---
>  include/linux/mfd/cros_ec_commands.h | 260 
> +++
>  1 file changed, 231 insertions(+), 29 deletions(-)
> 
> diff --git a/include/linux/mfd/cros_ec_commands.h 
> b/include/linux/mfd/cros_ec_commands.h
> index 76728ff..f26a806 100644
> --- a/include/linux/mfd/cros_ec_commands.h
> +++ b/include/linux/mfd/cros_ec_commands.h
> @@ -1290,7 +1290,13 @@ enum motionsense_command {
>  
>   /*
>* EC Rate command is a setter/getter command for the EC sampling rate
> -  * of all motion sensors in milliseconds.
> +  * in milliseconds.
> +  * It is per sensor, the EC run sample task  at the minimum of all
> +  * sensors EC_RATE.
> +  * For sensors without hardware FIFO, EC_RATE should be equals to 1/ODR
> +  * to collect all the sensor samples.
> +  * For sensor with hardware FIFO, EC_RATE is used as the maximal delay
> +  * to process of all motion sensors in milliseconds.
>*/
>   MOTIONSENSE_CMD_EC_RATE = 2,
>  
> @@ -1315,37 +1321,138 @@ enum motionsense_command {
>*/
>   MOTIONSENSE_CMD_KB_WAKE_ANGLE = 5,
>  
> - /* Number of motionsense sub-commands. */
> - MOTIONSENSE_NUM_CMDS
> -};
> + /*
> +  * Returns a single sensor data.
> +  */
Please use standard kernel documentation formats throughout.
If not you may face a Linus rant ;)
> + MOTIONSENSE_CMD_DATA = 6,
> +
> + /*
> +  * Return sensor fifo info.
> +  */
> + MOTIONSENSE_CMD_FIFO_INFO = 7,
> +
> + /*
> +  * Insert a flush element in the fifo and return sensor fifo info.
> +  * The host can use that element to synchronize its operation.
> +  */
> + MOTIONSENSE_CMD_FIFO_FLUSH = 8,
>  
> -enum motionsensor_id {
> - EC_MOTION_SENSOR_ACCEL_BASE = 0,
> - EC_MOTION_SENSOR_ACCEL_LID = 1,
> - EC_MOTION_SENSOR_GYRO = 2,
> + /*
> +  * Return a portion of the fifo.
> +  */
> + MOTIONSENSE_CMD_FIFO_READ = 9,
> +
> + /*
> +  * Perform low level calibration.
> +  * On sensors that support it, ask to do offset calibration.
> +  */
> + MOTIONSENSE_CMD_PERFORM_CALIB = 10,
> +
> + /*
> +  * Sensor Offset command is a setter/getter command for the offset
> +  * used for calibration.
> +  * The offsets can be calculated by the host, or via
> +  * PERFORM_CALIB command.
> +  */
> + MOTIONSENSE_CMD_SENSOR_OFFSET = 11,
> +
> + /*
> +  * List available activities for a MOTION sensor.
> +  * Indicates if they are enabled or disabled.
> +  */
> + MOTIONSENSE_CMD_LIST_ACTIVITIES = 12,
>  
>   /*
> -  * Note, if more sensors are added and this count changes, the padding
> -  * in ec_response_motion_sense dump command must be modified.
> +  * Activity management
> +  * Enable/Disable activity recognition.
>*/
> - EC_MOTION_SENSOR_COUNT = 3
> + MOTIONSENSE_CMD_SET_ACTIVITY = 13,
> +
> + /* Number of motionsense sub-commands. */
> + MOTIONSENSE_NUM_CMDS
>  };
>  
>  /* List of motion sensor types. */
>  enum motionsensor_type {
>   MOTIONSENSE_TYPE_ACCEL = 0,
>   MOTIONSENSE_TYPE_GYRO = 1,
> + MOTIONSENSE_TYPE_MAG = 2,
> + MOTIONSENSE_TYPE_PROX = 3,
> + MOTIONSENSE_TYPE_LIGHT = 4,
> + MOTIONSENSE_TYPE_ACTIVITY = 5,
> + MOTIONSENSE_TYPE_MAX,
>  };
>  
>  /* List of motion sensor locations. */
>  enum motionsensor_location {
>   MOTIONSENSE_LOC_BASE = 0,
>   MOTIONSENSE_LOC_LID = 1,
> + MOTIONSENSE_LOC_MAX,
>  };
>  
>  /* List of motion sensor chips. */
>  enum motionsensor_chip {
>   MOTIONSENSE_CHIP_KXCJ9 = 0,
> + MOTIONSENSE_CHIP_LSM6DS0 = 1,
> + MOTIONSENSE_CHIP_BMI160 = 2,
> + MOTIONSENSE_CHIP_SI1141 = 3,
> + MOTIONSENSE_CHIP_SI1142 = 4,
> + MOTIONSENSE_CHIP_SI1143 = 5,
> + MOTIONSENSE_CHIP_KX022 = 6,
> + MOTIONSENSE_CHIP_L3GD20H = 7,
Interesting.  So the driver needs some knowledge of what
is behind it.  I'll read on with interest ;)
> +};
> +
> +struct ec_response_motion_sensor_data {
> + /* Flags for each sensor. */
> + uint8_t flags;
> + /* sensor number the data comes from */
> + uint8_t sensor_num;
> + /* Each sensor is up to 3-axis. */
> + union {
> + int16_t data[3];
> + struct {
> + uint16_trsvd;
> +   

Re: [PATCH 05/10] iio: cros_ec_sensors: add ChromeOS EC Contiguous Sensors driver

[Jonathan Cameron]

On 18/07/16 08:02, Enric Balletbo i Serra wrote:
> Handle 3d contiguous sensors like Accelerometers, Gyroscope and
> Magnetometer that are presented by the ChromeOS EC Sensor hub.
> 
> Signed-off-by: Guenter Roeck 
> Signed-off-by: Enric Balletbo i Serra 
Various nitpicks inline.

Jonathan
> ---
>  drivers/iio/common/cros_ec_sensors/Kconfig |   8 +
>  drivers/iio/common/cros_ec_sensors/Makefile|   1 +
>  .../iio/common/cros_ec_sensors/cros_ec_sensors.c   | 322 
> +
>  3 files changed, 331 insertions(+)
>  create mode 100644 drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
> 
> diff --git a/drivers/iio/common/cros_ec_sensors/Kconfig 
> b/drivers/iio/common/cros_ec_sensors/Kconfig
> index a30f41e..18002d6 100644
> --- a/drivers/iio/common/cros_ec_sensors/Kconfig
> +++ b/drivers/iio/common/cros_ec_sensors/Kconfig
> @@ -12,3 +12,11 @@ config IIO_CROS_EC_SENSORS_CORE
> driver.
> Define common attributes and sysfs interrupt handler.
>  
> +config IIO_CROS_EC_SENSORS
> + tristate "ChromeOS EC Contiguous Sensors"
> + select IIO_CROS_EC_SENSORS_CORE
> + help
> +   Module to handle 3d contiguous sensors like
> +   Accelerometers, Gyroscope and Magnetometer that are
> +   presented by the ChromeOS EC Sensor hub.
> +   Creates an IIO device for each functions.
> diff --git a/drivers/iio/common/cros_ec_sensors/Makefile 
> b/drivers/iio/common/cros_ec_sensors/Makefile
> index 95b6901..ec716ff 100644
> --- a/drivers/iio/common/cros_ec_sensors/Makefile
> +++ b/drivers/iio/common/cros_ec_sensors/Makefile
> @@ -3,3 +3,4 @@
>  #
>  
>  obj-$(CONFIG_IIO_CROS_EC_SENSORS_CORE) += cros_ec_sensors_core.o
> +obj-$(CONFIG_IIO_CROS_EC_SENSORS) += cros_ec_sensors.o
> diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c 
> b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
> new file mode 100644
> index 000..4741118
> --- /dev/null
> +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
> @@ -0,0 +1,322 @@
> +/*
> + * cros_ec_sensors - Driver for Chrome OS Embedded Controller sensors.
> + *
> + * Copyright (C) 2015 Google, Inc
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that 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.
> + *
> + * This driver uses the cros-ec interface to communicate with the Chrome OS
> + * EC about accelerometer data. Accelerometer access is presented through
> + * iio sysfs.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "cros_ec_sensors_core.h"
> +
> +#define MAX_CHANNELS (MAX_AXIS + 1)
Really needs a prefix - chances of another define for MAX_CHANNELS in an
included header is way too high!
Same for MAX_AXIS etc...  Basically prefix everything and you'll have it
about right...
> +
> +/* State data for ec_sensors iio driver. */
> +struct cros_ec_sensors_state {
> + /* Shared by all sensors */
> + struct cros_ec_sensors_core_state core;
> +
> + struct iio_chan_spec channels[MAX_CHANNELS];
> +};
> +
> +static int cros_ec_sensors_read(struct iio_dev *indio_dev,
> +   struct iio_chan_spec const *chan,
> +   int *val, int *val2, long mask)
> +{
> + struct cros_ec_sensors_state *st = iio_priv(indio_dev);
> + s16 data = 0;
> + s64 val64;
> + int i;
> + int ret = IIO_VAL_INT;
> + int idx = chan->scan_index;
> +
> + mutex_lock(>core.cmd_lock);
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + if (st->core.read_ec_sensors_data(indio_dev,
> +   1 << idx, ) < 0)
> + ret = -EIO;
Never get any useful errors in from the function?  Seems like you are eating
it if there is.
> + *val = data;
> + break;
> + case IIO_CHAN_INFO_CALIBBIAS:
> + st->core.param.cmd = MOTIONSENSE_CMD_SENSOR_OFFSET;
> + st->core.param.sensor_offset.flags = 0;
> +
> + if (cros_ec_motion_send_host_cmd(>core, 0)) {
< 0 for consistency?
> + ret = -EIO;
> + break;
> + }
> +
> + /* Save values */
> + for (i = X; i < MAX_AXIS; i++)
> + st->core.calib[i].offset =
> + st->core.resp->sensor_offset.offset[i];
> +
> + *val = st->core.calib[idx].offset;
> + break;
> + case 

Re: [PATCH 05/10] iio: cros_ec_sensors: add ChromeOS EC Contiguous Sensors driver

[Jonathan Cameron]

On 18/07/16 08:02, Enric Balletbo i Serra wrote:
> Handle 3d contiguous sensors like Accelerometers, Gyroscope and
> Magnetometer that are presented by the ChromeOS EC Sensor hub.
> 
> Signed-off-by: Guenter Roeck 
> Signed-off-by: Enric Balletbo i Serra 
Various nitpicks inline.

Jonathan
> ---
>  drivers/iio/common/cros_ec_sensors/Kconfig |   8 +
>  drivers/iio/common/cros_ec_sensors/Makefile|   1 +
>  .../iio/common/cros_ec_sensors/cros_ec_sensors.c   | 322 
> +
>  3 files changed, 331 insertions(+)
>  create mode 100644 drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
> 
> diff --git a/drivers/iio/common/cros_ec_sensors/Kconfig 
> b/drivers/iio/common/cros_ec_sensors/Kconfig
> index a30f41e..18002d6 100644
> --- a/drivers/iio/common/cros_ec_sensors/Kconfig
> +++ b/drivers/iio/common/cros_ec_sensors/Kconfig
> @@ -12,3 +12,11 @@ config IIO_CROS_EC_SENSORS_CORE
> driver.
> Define common attributes and sysfs interrupt handler.
>  
> +config IIO_CROS_EC_SENSORS
> + tristate "ChromeOS EC Contiguous Sensors"
> + select IIO_CROS_EC_SENSORS_CORE
> + help
> +   Module to handle 3d contiguous sensors like
> +   Accelerometers, Gyroscope and Magnetometer that are
> +   presented by the ChromeOS EC Sensor hub.
> +   Creates an IIO device for each functions.
> diff --git a/drivers/iio/common/cros_ec_sensors/Makefile 
> b/drivers/iio/common/cros_ec_sensors/Makefile
> index 95b6901..ec716ff 100644
> --- a/drivers/iio/common/cros_ec_sensors/Makefile
> +++ b/drivers/iio/common/cros_ec_sensors/Makefile
> @@ -3,3 +3,4 @@
>  #
>  
>  obj-$(CONFIG_IIO_CROS_EC_SENSORS_CORE) += cros_ec_sensors_core.o
> +obj-$(CONFIG_IIO_CROS_EC_SENSORS) += cros_ec_sensors.o
> diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c 
> b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
> new file mode 100644
> index 000..4741118
> --- /dev/null
> +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
> @@ -0,0 +1,322 @@
> +/*
> + * cros_ec_sensors - Driver for Chrome OS Embedded Controller sensors.
> + *
> + * Copyright (C) 2015 Google, Inc
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that 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.
> + *
> + * This driver uses the cros-ec interface to communicate with the Chrome OS
> + * EC about accelerometer data. Accelerometer access is presented through
> + * iio sysfs.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "cros_ec_sensors_core.h"
> +
> +#define MAX_CHANNELS (MAX_AXIS + 1)
Really needs a prefix - chances of another define for MAX_CHANNELS in an
included header is way too high!
Same for MAX_AXIS etc...  Basically prefix everything and you'll have it
about right...
> +
> +/* State data for ec_sensors iio driver. */
> +struct cros_ec_sensors_state {
> + /* Shared by all sensors */
> + struct cros_ec_sensors_core_state core;
> +
> + struct iio_chan_spec channels[MAX_CHANNELS];
> +};
> +
> +static int cros_ec_sensors_read(struct iio_dev *indio_dev,
> +   struct iio_chan_spec const *chan,
> +   int *val, int *val2, long mask)
> +{
> + struct cros_ec_sensors_state *st = iio_priv(indio_dev);
> + s16 data = 0;
> + s64 val64;
> + int i;
> + int ret = IIO_VAL_INT;
> + int idx = chan->scan_index;
> +
> + mutex_lock(>core.cmd_lock);
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + if (st->core.read_ec_sensors_data(indio_dev,
> +   1 << idx, ) < 0)
> + ret = -EIO;
Never get any useful errors in from the function?  Seems like you are eating
it if there is.
> + *val = data;
> + break;
> + case IIO_CHAN_INFO_CALIBBIAS:
> + st->core.param.cmd = MOTIONSENSE_CMD_SENSOR_OFFSET;
> + st->core.param.sensor_offset.flags = 0;
> +
> + if (cros_ec_motion_send_host_cmd(>core, 0)) {
< 0 for consistency?
> + ret = -EIO;
> + break;
> + }
> +
> + /* Save values */
> + for (i = X; i < MAX_AXIS; i++)
> + st->core.calib[i].offset =
> + st->core.resp->sensor_offset.offset[i];
> +
> + *val = st->core.calib[idx].offset;
> + break;
> + case IIO_CHAN_INFO_SCALE:
> + st->core.param.cmd = 

Re: [PATCH 04/10] iio: cros_ec: Add common functions for cros_ec sensors.

[Jonathan Cameron]

On 18/07/16 08:29, Peter Meerwald-Stadler wrote:
> 
>> Add the core functions to be able to support the sensors attached behind
>> the ChromeOS Embedded Controller and used by other IIO cros-ec sensor
>> drivers.
> 
> comments below from a quick read
> there is plenty on undocumented private API
Few more comments from me. Peter is of course quite correct, its the
new, undocumented ABI, which is the biggest issue.

Interesting looking device.  Any docs out there?

Jonathan
>  
>> The cros_ec_sensor_core driver matches with current driver in ChromeOS
>> 4.4 tree, so it includes all the fixes at the moment. The support for
>> this driver was made by Gwendal Grignou. The original patch and all the
>> fixes has been squashed and rebased on top of mainline.
>>
>> Signed-off-by: Gwendal Grignou 
>> Signed-off-by: Guenter Roeck 
>> [eballetbo: split, squash and rebase on top of mainline the patches
>> found in ChromeOS tree]
>> Signed-off-by: Enric Balletbo i Serra 
>> ---
>>  drivers/iio/common/Kconfig |   1 +
>>  drivers/iio/common/Makefile|   1 +
>>  drivers/iio/common/cros_ec_sensors/Kconfig |  14 +
>>  drivers/iio/common/cros_ec_sensors/Makefile|   5 +
>>  .../common/cros_ec_sensors/cros_ec_sensors_core.c  | 564 
>> +
>>  .../common/cros_ec_sensors/cros_ec_sensors_core.h  | 155 ++
>>  6 files changed, 740 insertions(+)
>>  create mode 100644 drivers/iio/common/cros_ec_sensors/Kconfig
>>  create mode 100644 drivers/iio/common/cros_ec_sensors/Makefile
>>  create mode 100644 drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
>>  create mode 100644 drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.h
>>
>> diff --git a/drivers/iio/common/Kconfig b/drivers/iio/common/Kconfig
>> index 26a6026..e108996 100644
>> --- a/drivers/iio/common/Kconfig
>> +++ b/drivers/iio/common/Kconfig
>> @@ -2,6 +2,7 @@
>>  # IIO common modules
>>  #
>>  
>> +source "drivers/iio/common/cros_ec_sensors/Kconfig"
>>  source "drivers/iio/common/hid-sensors/Kconfig"
>>  source "drivers/iio/common/ms_sensors/Kconfig"
>>  source "drivers/iio/common/ssp_sensors/Kconfig"
>> diff --git a/drivers/iio/common/Makefile b/drivers/iio/common/Makefile
>> index 585da6a..6fa760e 100644
>> --- a/drivers/iio/common/Makefile
>> +++ b/drivers/iio/common/Makefile
>> @@ -7,6 +7,7 @@
>>  #
>>  
>>  # When adding new entries keep the list in alphabetical order
>> +obj-y += cros_ec_sensors/
>>  obj-y += hid-sensors/
>>  obj-y += ms_sensors/
>>  obj-y += ssp_sensors/
>> diff --git a/drivers/iio/common/cros_ec_sensors/Kconfig 
>> b/drivers/iio/common/cros_ec_sensors/Kconfig
>> new file mode 100644
>> index 000..a30f41e
>> --- /dev/null
>> +++ b/drivers/iio/common/cros_ec_sensors/Kconfig
>> @@ -0,0 +1,14 @@
>> +#
>> +# Chrome OS Embedded Controller managed sensors library
>> +#
>> +config IIO_CROS_EC_SENSORS_CORE
>> +tristate "ChromeOS EC Sensors Core"
>> +depends on SYSFS && MFD_CROS_EC
>> +select IIO_BUFFER
>> +select IIO_TRIGGERED_BUFFER
>> +help
>> +  Base module for the ChromeOS EC Sensors module.
>> +  Contains core functions used by other IIO CrosEC sensor
>> +  driver.
> 
> 'drivers' probably
> 
>> +  Define common attributes and sysfs interrupt handler.
>> +
>> diff --git a/drivers/iio/common/cros_ec_sensors/Makefile 
>> b/drivers/iio/common/cros_ec_sensors/Makefile
>> new file mode 100644
>> index 000..95b6901
>> --- /dev/null
>> +++ b/drivers/iio/common/cros_ec_sensors/Makefile
>> @@ -0,0 +1,5 @@
>> +#
>> +# Makefile for sensors seen through the ChromeOS EC sensor hub.
>> +#
>> +
>> +obj-$(CONFIG_IIO_CROS_EC_SENSORS_CORE) += cros_ec_sensors_core.o
>> diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c 
>> b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
>> new file mode 100644
>> index 000..cb3de8f
>> --- /dev/null
>> +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
>> @@ -0,0 +1,564 @@
>> +/*
>> + * cros_ec_sensors_core - Common function for Chrome OS EC sensor driver.
>> + *
>> + * Copyright (C) 2015 Google, Inc
>> + *
>> + * This software is licensed under the terms of the GNU General Public
>> + * License version 2, as published by the Free Software Foundation, and
>> + * may be copied, distributed, and modified under those terms.
>> + *
>> + * This program is distributed in the hope that 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.
>> + *
>> + * This driver uses the cros-ec interface to communicate with the Chrome OS
>> + * EC about accelerometer data. Accelerometer access is presented through
>> + * iio sysfs.
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> 

Re: [PATCH 02/10] mfd: cros_ec: update MOTIONSENSE definitions and commands.

[Jonathan Cameron]

On 18/07/16 08:02, Enric Balletbo i Serra wrote:
> Let's update the command header to include the definitions related to
> the sensors attached behind the ChromeOS Embedded Controller. The new
> commands and definitions allow us to get information from these sensors.
> 
> Signed-off-by: Gwendal Grignou 
> Signed-off-by: Enric Balletbo i Serra 
Again, I'd be happier seeing this stuff introduced as and when it
is needed rather than in a magic patch. It's hard to review stuff
if it's broken up across multiple patches like this.

A few other bits and pieces inline.

Jonathan
> ---
>  include/linux/mfd/cros_ec_commands.h | 260 
> +++
>  1 file changed, 231 insertions(+), 29 deletions(-)
> 
> diff --git a/include/linux/mfd/cros_ec_commands.h 
> b/include/linux/mfd/cros_ec_commands.h
> index 76728ff..f26a806 100644
> --- a/include/linux/mfd/cros_ec_commands.h
> +++ b/include/linux/mfd/cros_ec_commands.h
> @@ -1290,7 +1290,13 @@ enum motionsense_command {
>  
>   /*
>* EC Rate command is a setter/getter command for the EC sampling rate
> -  * of all motion sensors in milliseconds.
> +  * in milliseconds.
> +  * It is per sensor, the EC run sample task  at the minimum of all
> +  * sensors EC_RATE.
> +  * For sensors without hardware FIFO, EC_RATE should be equals to 1/ODR
> +  * to collect all the sensor samples.
> +  * For sensor with hardware FIFO, EC_RATE is used as the maximal delay
> +  * to process of all motion sensors in milliseconds.
>*/
>   MOTIONSENSE_CMD_EC_RATE = 2,
>  
> @@ -1315,37 +1321,138 @@ enum motionsense_command {
>*/
>   MOTIONSENSE_CMD_KB_WAKE_ANGLE = 5,
>  
> - /* Number of motionsense sub-commands. */
> - MOTIONSENSE_NUM_CMDS
> -};
> + /*
> +  * Returns a single sensor data.
> +  */
Please use standard kernel documentation formats throughout.
If not you may face a Linus rant ;)
> + MOTIONSENSE_CMD_DATA = 6,
> +
> + /*
> +  * Return sensor fifo info.
> +  */
> + MOTIONSENSE_CMD_FIFO_INFO = 7,
> +
> + /*
> +  * Insert a flush element in the fifo and return sensor fifo info.
> +  * The host can use that element to synchronize its operation.
> +  */
> + MOTIONSENSE_CMD_FIFO_FLUSH = 8,
>  
> -enum motionsensor_id {
> - EC_MOTION_SENSOR_ACCEL_BASE = 0,
> - EC_MOTION_SENSOR_ACCEL_LID = 1,
> - EC_MOTION_SENSOR_GYRO = 2,
> + /*
> +  * Return a portion of the fifo.
> +  */
> + MOTIONSENSE_CMD_FIFO_READ = 9,
> +
> + /*
> +  * Perform low level calibration.
> +  * On sensors that support it, ask to do offset calibration.
> +  */
> + MOTIONSENSE_CMD_PERFORM_CALIB = 10,
> +
> + /*
> +  * Sensor Offset command is a setter/getter command for the offset
> +  * used for calibration.
> +  * The offsets can be calculated by the host, or via
> +  * PERFORM_CALIB command.
> +  */
> + MOTIONSENSE_CMD_SENSOR_OFFSET = 11,
> +
> + /*
> +  * List available activities for a MOTION sensor.
> +  * Indicates if they are enabled or disabled.
> +  */
> + MOTIONSENSE_CMD_LIST_ACTIVITIES = 12,
>  
>   /*
> -  * Note, if more sensors are added and this count changes, the padding
> -  * in ec_response_motion_sense dump command must be modified.
> +  * Activity management
> +  * Enable/Disable activity recognition.
>*/
> - EC_MOTION_SENSOR_COUNT = 3
> + MOTIONSENSE_CMD_SET_ACTIVITY = 13,
> +
> + /* Number of motionsense sub-commands. */
> + MOTIONSENSE_NUM_CMDS
>  };
>  
>  /* List of motion sensor types. */
>  enum motionsensor_type {
>   MOTIONSENSE_TYPE_ACCEL = 0,
>   MOTIONSENSE_TYPE_GYRO = 1,
> + MOTIONSENSE_TYPE_MAG = 2,
> + MOTIONSENSE_TYPE_PROX = 3,
> + MOTIONSENSE_TYPE_LIGHT = 4,
> + MOTIONSENSE_TYPE_ACTIVITY = 5,
> + MOTIONSENSE_TYPE_MAX,
>  };
>  
>  /* List of motion sensor locations. */
>  enum motionsensor_location {
>   MOTIONSENSE_LOC_BASE = 0,
>   MOTIONSENSE_LOC_LID = 1,
> + MOTIONSENSE_LOC_MAX,
>  };
>  
>  /* List of motion sensor chips. */
>  enum motionsensor_chip {
>   MOTIONSENSE_CHIP_KXCJ9 = 0,
> + MOTIONSENSE_CHIP_LSM6DS0 = 1,
> + MOTIONSENSE_CHIP_BMI160 = 2,
> + MOTIONSENSE_CHIP_SI1141 = 3,
> + MOTIONSENSE_CHIP_SI1142 = 4,
> + MOTIONSENSE_CHIP_SI1143 = 5,
> + MOTIONSENSE_CHIP_KX022 = 6,
> + MOTIONSENSE_CHIP_L3GD20H = 7,
Interesting.  So the driver needs some knowledge of what
is behind it.  I'll read on with interest ;)
> +};
> +
> +struct ec_response_motion_sensor_data {
> + /* Flags for each sensor. */
> + uint8_t flags;
> + /* sensor number the data comes from */
> + uint8_t sensor_num;
> + /* Each sensor is up to 3-axis. */
> + union {
> + int16_t data[3];
> + struct {
> + uint16_trsvd;
> + uint32_ttimestamp;
> +

Re: [PATCH v2 4/4] hwmon: iio: add label for channels read by iio_hwmon

[Jonathan Cameron]

On 15/07/16 10:59, Quentin Schulz wrote:
> Currently, iio_hwmon only exposes values of the IIO channels it can read
> but no label by channel is exposed.
> 
> This adds exposition of sysfs files containing label for IIO channels it
> can read based on extended_name field of the iio_chan_spec of the channel.
> If the extended_name field is empty, the sysfs file is not created by
> iio_hwmon.
Hmm. This is not the intent of extended name at all.  That exists to add
a small amount of information to an constructed IIO channel name.
Typically it's used to indicate physically wired stuff like:

in_voltage0_vdd_raw for cases where that channel of the ADC is hard wired
to the vdd.  In this particular case the use might actually work as the
vdd makes it clear it's a voltage - in general that's not the case.

Use of extended_name at all in IIO is only done after extensive review.
It adds nasty custom ABI to a device, so the gain has to be considerable
to use it.

When I read your original suggestion of adding labels, I was expecting the
use of datasheet_name.  That has the advantage of being well defined by
the datasheet (if not it should not be provided) + not being used in
the construction of the IIO channel related attributes.  However, that
may still not correspond well to the expected labelling in hwmon.
Thinking more on this, the label is going to often be a function of how
the board is wired up...  Perhaps it should be a characteristic of the
channel_map (hence from DT or similar) rather than part of the IIO driver
itself?

At first glance hwmon labels appear to be pretty freeform...  However
we need to be very careful here as this is effectively defining a large
chunk of new ABI.

This isn't a thing that I have a particularly clear view on (as you
might be able to tell ;).  Other opinions sought!

Jonathan
> 
> Signed-off-by: Quentin Schulz 
> ---
> 
> patch added in v2
> 
>  drivers/hwmon/iio_hwmon.c | 77 
> +--
>  1 file changed, 68 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/hwmon/iio_hwmon.c b/drivers/hwmon/iio_hwmon.c
> index c0da4d9..28d15b2 100644
> --- a/drivers/hwmon/iio_hwmon.c
> +++ b/drivers/hwmon/iio_hwmon.c
> @@ -16,6 +16,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  /**
> @@ -57,12 +58,26 @@ static ssize_t iio_hwmon_read_val(struct device *dev,
>   return sprintf(buf, "%d\n", result);
>  }
>  
> +static ssize_t iio_hwmon_read_label(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct sensor_device_attribute *sattr = to_sensor_dev_attr(attr);
> + struct iio_hwmon_state *state = dev_get_drvdata(dev);
> + const char *label = state->channels[sattr->index].channel->extend_name;
> +
> + if (label)
> + return sprintf(buf, "%s\n", label);
> +
> + return 0;
> +}
> +
>  static int iio_hwmon_probe(struct platform_device *pdev)
>  {
>   struct device *dev = >dev;
>   struct iio_hwmon_state *st;
> - struct sensor_device_attribute *a;
> - int ret, i;
> + struct sensor_device_attribute *a, *b;
> + int ret, i, j = 0;
>   int in_i = 1, temp_i = 1, curr_i = 1, humidity_i = 1;
>   enum iio_chan_type type;
>   struct iio_channel *channels;
> @@ -92,7 +107,8 @@ static int iio_hwmon_probe(struct platform_device *pdev)
>   st->num_channels++;
>  
>   st->attrs = devm_kzalloc(dev,
> -  sizeof(*st->attrs) * (st->num_channels + 1),
> +  sizeof(*st->attrs) *
> +  (2 * st->num_channels + 1),
>GFP_KERNEL);
>   if (st->attrs == NULL) {
>   ret = -ENOMEM;
> @@ -107,6 +123,18 @@ static int iio_hwmon_probe(struct platform_device *pdev)
>   }
>  
>   sysfs_attr_init(>dev_attr.attr);
> +
> + b = NULL;
> + if (st->channels[i].channel->extend_name) {
> + b = devm_kzalloc(dev, sizeof(*b), GFP_KERNEL);
> + if (b == NULL) {
> + ret = -ENOMEM;
> + goto error_release_channels;
> + }
> +
> + sysfs_attr_init(>dev_attr.attr);
> + }
> +
>   ret = iio_get_channel_type(>channels[i], );
>   if (ret < 0)
>   goto error_release_channels;
> @@ -115,35 +143,66 @@ static int iio_hwmon_probe(struct platform_device *pdev)
>   case IIO_VOLTAGE:
>   a->dev_attr.attr.name = kasprintf(GFP_KERNEL,
> "in%d_input",
> -   in_i++);
> +   in_i);
> + if (b)
> +  

Re: [PATCH v4 2/3] iio: adc: mt2701: Add Mediatek auxadc driver for mt2701.

[Jonathan Cameron]

On 11/07/16 07:39, Zhiyong Tao wrote:
> Add Mediatek auxadc driver based on iio.
> It will register a device in iio and support iio.
> So thermal can read auxadc channel to sample data by iio device.
> It is tested successfully on mt2701 platform.
> Mt8173 and mt6577 platforms are not tested.
> But the expectation is compatible.
> 
> Signed-off-by: Zhiyong Tao 
Looks good to me.  Couple of really minor points / questions inline...

Just one thing to confirm... (you probably already answered this!)
What are the units of the voltage channels?

Jonathan
> ---
>  drivers/iio/adc/Kconfig |   13 ++
>  drivers/iio/adc/Makefile|1 +
>  drivers/iio/adc/mt6577_auxadc.c |  294 
> +++
>  3 files changed, 308 insertions(+)
>  create mode 100644 drivers/iio/adc/mt6577_auxadc.c
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 25378c5..14929fc 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -305,6 +305,19 @@ config MCP3422
> This driver can also be built as a module. If so, the module will be
> called mcp3422.
>  
> +config MEDIATEK_MT6577_AUXADC
> +tristate "MediaTek AUXADC driver"
> +depends on ARCH_MEDIATEK || COMPILE_TEST
> +depends on HAS_IOMEM
> +help
> +  Say yes here to enable support for MediaTek mt65xx AUXADC.
> +
> +  The driver supports immediate mode operation to read from one of 
> sixteen
> +  channels (external or internal).
> +
> +  This driver can also be built as a module. If so, the module will 
> be
> +  called mt6577_auxadc.
> +
>  config MEN_Z188_ADC
>   tristate "MEN 16z188 ADC IP Core support"
>   depends on MCB
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index 38638d4..8306347 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -30,6 +30,7 @@ obj-$(CONFIG_MAX1027) += max1027.o
>  obj-$(CONFIG_MAX1363) += max1363.o
>  obj-$(CONFIG_MCP320X) += mcp320x.o
>  obj-$(CONFIG_MCP3422) += mcp3422.o
> +obj-$(CONFIG_MEDIATEK_MT6577_AUXADC) += mt6577_auxadc.o
>  obj-$(CONFIG_MEN_Z188_ADC) += men_z188_adc.o
>  obj-$(CONFIG_MXS_LRADC) += mxs-lradc.o
>  obj-$(CONFIG_NAU7802) += nau7802.o
> diff --git a/drivers/iio/adc/mt6577_auxadc.c b/drivers/iio/adc/mt6577_auxadc.c
> new file mode 100644
> index 000..c7cc901
> --- /dev/null
> +++ b/drivers/iio/adc/mt6577_auxadc.c
> @@ -0,0 +1,294 @@
> +/*
> + * Copyright (c) 2016 MediaTek Inc.
> + * Author: Zhiyong Tao 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that 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.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/* Register definitions */
> +#define MT6577_AUXADC_CON00x00
> +#define MT6577_AUXADC_CON10x04
> +#define MT6577_AUXADC_CON20x10
> +#define MT6577_AUXADC_STA BIT(0)
> +
> +#define MT6577_AUXADC_DAT00x14
> +#define MT6577_AUXADC_RDY0BIT(12)
> +
> +#define MT6577_AUXADC_MISC0x94
> +#define MT6577_AUXADC_PDN_EN  BIT(14)
> +
> +#define MT6577_AUXADC_DAT_MASK0xfff
> +#define MT6577_AUXADC_SLEEP_US1000
> +#define MT6577_AUXADC_TIMEOUT_US  1
> +#define MT6577_AUXADC_POWER_READY_MS  1
> +#define MT6577_AUXADC_SAMPLE_READY_US 25
> +
> +struct mt6577_auxadc_device {
> + void __iomem *reg_base;
> + struct clk *adc_clk;
> + struct mutex lock;
> +};
> +
> +#define MT6577_AUXADC_CHANNEL(idx) { \
> + .type = IIO_VOLTAGE,\
> + .indexed = 1,   \
> + .channel = (idx),   \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED), \
> +}
> +
> +static const struct iio_chan_spec mt6577_auxadc_iio_channels[] = {
> + MT6577_AUXADC_CHANNEL(0),
> + MT6577_AUXADC_CHANNEL(1),
> + MT6577_AUXADC_CHANNEL(2),
> + MT6577_AUXADC_CHANNEL(3),
> + MT6577_AUXADC_CHANNEL(4),
> + MT6577_AUXADC_CHANNEL(5),
> + MT6577_AUXADC_CHANNEL(6),
> + MT6577_AUXADC_CHANNEL(7),
> + MT6577_AUXADC_CHANNEL(8),
> + MT6577_AUXADC_CHANNEL(9),
> + MT6577_AUXADC_CHANNEL(10),
> + MT6577_AUXADC_CHANNEL(11),
> + MT6577_AUXADC_CHANNEL(12),
> + 

Re: [PATCH 03/10] iio: core: Add double tap as possible gesture

[Jonathan Cameron]

On 18/07/16 08:02, Enric Balletbo i Serra wrote:
> From: Gwendal Grignou 
> 
> This is an interface change: however, the sysfs entry is based on string,
> so if other gestures are added on the trunk in the meantime, we will
> still be able to merge this change.
> 
> Signed-off-by: Gwendal Grignou 
> Signed-off-by: Guenter Roeck 
> [enric: Rebased and resolved conflicts]
> Signed-off-by: Enric Balletbo i Serra 
So you are creating an entire channel type for double tap?

Not keen on this I'm afraid.

The activity types in there are the moment are not events, but rather
attempts to estimate the 'likelihood' that a given activity is currently
being undertaken.

Hmm. Double tap is more of an event type.  It's one I've been wondering
how to describe for a while... At the end of the day it's just a
reasonably sophisticated filter - kind of a 'magic' version of Rate
of Change.  Unfortunately there isn't a clean mathematical definition
as it can be implemented in lots of ways.  

I guess the best may be to just have it as an event type on it's own...

What do others think?

Jonathan
> ---
>  drivers/iio/industrialio-core.c | 1 +
>  include/uapi/linux/iio/types.h  | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index e6319a9..f700e67 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -119,6 +119,7 @@ static const char * const iio_modifier_names[] = {
>   [IIO_MOD_Q] = "q",
>   [IIO_MOD_CO2] = "co2",
>   [IIO_MOD_VOC] = "voc",
> + [IIO_MOD_DOUBLE_TAP] = "double_tap",
>  };
>  
>  /* relies on pairs of these shared then separate */
> diff --git a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h
> index b0916fc..c290167 100644
> --- a/include/uapi/linux/iio/types.h
> +++ b/include/uapi/linux/iio/types.h
> @@ -79,6 +79,7 @@ enum iio_modifier {
>   IIO_MOD_CO2,
>   IIO_MOD_VOC,
>   IIO_MOD_LIGHT_UV,
> + IIO_MOD_DOUBLE_TAP,
>  };
>  
>  enum iio_event_type {
> 

Re: [PATCH] staging: iio: light: isl29018/28: remove I2C_CLASS_HWMON .class setting

[Jonathan Cameron]

On 18/07/16 09:50, Laxman Dewangan wrote:
> 
> On Saturday 16 July 2016 07:58 AM, Alison Schofield wrote:
>> I2C_CLASS_HWMON is for a hardware monitoring chip wanting
>> auto-detection.  IIO drivers don't typically use .class.
>> Remove it.
>>
>> Signed-off-by: Alison Schofield 
>> Cc: Daniel Baluta 
>>
> 
> Agree.
> Acked-by: Laxman Dewangan 
Any resulting ABI change from dropping it?
I don't think so, as it doesn't support detection anyway,
but best to be sure ;)

> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 08/10] iio: cros_ec_sensors_ring: add ChromeOS EC Sensors Ring

[Jonathan Cameron]

On 18/07/16 08:02, Enric Balletbo i Serra wrote:
> Add support for handling sensor events FIFO produced by the sensor
> hub. A single device with a buffer will collect all samples produced
> by the sensors managed by the CrosEC sensor hub.
So you are defining a different device to support the buffer?
That's 'unusual'...
> 
> Signed-off-by: Guenter Roeck 
> Signed-off-by: Enric Balletbo i Serra 
> ---
>  drivers/iio/common/cros_ec_sensors/Kconfig |   9 +
>  drivers/iio/common/cros_ec_sensors/Makefile|   1 +
>  .../common/cros_ec_sensors/cros_ec_sensors_ring.c  | 541 
> +
>  3 files changed, 551 insertions(+)
>  create mode 100644 drivers/iio/common/cros_ec_sensors/cros_ec_sensors_ring.c
> 
> diff --git a/drivers/iio/common/cros_ec_sensors/Kconfig 
> b/drivers/iio/common/cros_ec_sensors/Kconfig
> index 22b4211..778c3bf 100644
> --- a/drivers/iio/common/cros_ec_sensors/Kconfig
> +++ b/drivers/iio/common/cros_ec_sensors/Kconfig
> @@ -39,3 +39,12 @@ config IIO_CROS_EC_ACTIVITY
> Activities can be simple (low/no motion) or more complex (riding 
> train).
> They are being reported by physical devices or the EC itself.
> Creates an IIO device to manage all activities.
> +
> +config IIO_CROS_EC_SENSORS_RING
> + tristate "ChromeOS EC Sensors Ring"
> + depends on IIO_CROS_EC_SENSORS || IIO_CROS_EC_LIGHT_PROX
> + help
> +   Add support for handling sensor events FIFO produced by
> +   the sensor hub.
> +   A single device with a buffer will collect all samples produced
> +   by the sensors managed by the CrosEC sensor hub
> diff --git a/drivers/iio/common/cros_ec_sensors/Makefile 
> b/drivers/iio/common/cros_ec_sensors/Makefile
> index 8f54f1e..0eb3fc5 100644
> --- a/drivers/iio/common/cros_ec_sensors/Makefile
> +++ b/drivers/iio/common/cros_ec_sensors/Makefile
> @@ -6,3 +6,4 @@ obj-$(CONFIG_IIO_CROS_EC_SENSORS_CORE) += 
> cros_ec_sensors_core.o
>  obj-$(CONFIG_IIO_CROS_EC_SENSORS) += cros_ec_sensors.o
>  obj-$(CONFIG_IIO_CROS_EC_LIGHT_PROX) += cros_ec_light_prox.o
>  obj-$(CONFIG_IIO_CROS_EC_ACTIVITY) += cros_ec_activity.o
> +obj-$(CONFIG_IIO_CROS_EC_SENSORS_RING) += cros_ec_sensors_ring.o
> diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_ring.c 
> b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_ring.c
> new file mode 100644
> index 000..1c74df9
> --- /dev/null
> +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_ring.c
> @@ -0,0 +1,541 @@
> +/*
> + * cros_ec_sensors_ring - Driver for Chrome OS EC Sensor hub FIFO.
> + *
> + * Copyright (C) 2015 Google, Inc
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that 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.
> + *
> + * This driver uses the cros-ec interface to communicate with the Chrome OS
> + * EC about accelerometer data. Accelerometer access is presented through
> + * iio sysfs.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "cros_ec_sensors_core.h"
> +
> +/* The ring is a FIFO that return sensor information from
> + * the single EC FIFO.
> + * There are always 5 channels returned:
> +* | ID | FLAG | X | Y | Z | Timestamp |
> + * ID is the EC sensor id
> + * FLAG is for meta data, only flush bit is defined.
> + */
> +#define CROS_EC_FLUSH_BIT 1
> +
> +enum {
> + CHANNEL_SENSOR_ID,
> + CHANNEL_SENSOR_FLAG,
Ah, I'm beginning to understand.

I think you really need to demux these into the appropriate devices individual
buffers... If these are coming in fairly randomly (guessing so?) then you'll
want a top level mfd pushing data to each of the child devices (representing
the different possible sensors).

Simply pushing it into a buffer with metadata doesn't fit with the IIO ABI
at all.

Note that there is no obligation to have any triggers involved at all. Here I'd
suggest you don't as it'll just make life difficult for little gain.

> + CHANNEL_X,
> + CHANNEL_Y,
> + CHANNEL_Z,
> + CHANNEL_TIMESTAMP,
> + MAX_CHANNEL,
> +};
> +
> +enum {
> + LAST_TS,
> + NEW_TS,
> + ALL_TS
> +};
> +
> +#define CROS_EC_SENSOR_MAX 16
> +
> +struct cros_ec_fifo_info {
> + struct ec_response_motion_sense_fifo_info info;
> + uint16_t lost[CROS_EC_SENSOR_MAX];
> +};
> +
single blank line is plenty.
> +
> +struct cros_ec_sensors_ring_sample {
> + uint8_t sensor_id;
> + uint8_t flag;
> + int16_t  vector[MAX_AXIS];
> +   

Re: [PATCH v2 3/4] mfd: add support for Allwinner SoCs ADC

[Jonathan Cameron]

On 15/07/16 10:59, Quentin Schulz wrote:
> The Allwinner SoCs all have an ADC that can also act as a touchscreen
> controller and a thermal sensor. For now, only the ADC and the thermal
> sensor drivers are probed by the MFD, the touchscreen controller support
> will be added later.
> 
> Signed-off-by: Quentin Schulz 
Hmm. Previous patch includes the header this one creates.  Ordering issue?
The depends kind of prevents build failures by ensuring that can't be built
until this one is in place, but it is certainly an ugly way to do it.

Few little bits innline.
> ---
> 
> v2:
>  - add license headers,
>  - reorder alphabetically includes,
>  - add SUNXI_GPADC_ prefixes for defines,
> 
>  drivers/mfd/Kconfig |  14 +++
>  drivers/mfd/Makefile|   2 +
>  drivers/mfd/sunxi-gpadc-mfd.c   | 197 
> 
>  include/linux/mfd/sunxi-gpadc-mfd.h |  23 +
>  4 files changed, 236 insertions(+)
>  create mode 100644 drivers/mfd/sunxi-gpadc-mfd.c
>  create mode 100644 include/linux/mfd/sunxi-gpadc-mfd.h
> 
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 1bcf601..67b55d0 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -82,6 +82,20 @@ config MFD_ATMEL_FLEXCOM
> by the probe function of this MFD driver according to a device tree
> property.
>  
> +config MFD_SUNXI_ADC
> + tristate "ADC MFD core driver for sunxi platforms"
> + select MFD_CORE
> + select REGMAP_MMIO
> + help
> +   Select this to get support for Allwinner SoCs (A10, A13 and A31) ADC.
> +   This driver will only map the hardware interrupt and registers, you
> +   have to select individual drivers based on this MFD to be able to use
> +   the ADC or the thermal sensor. This will try to probe the ADC driver
> +   sunxi-gpadc-iio and the hwmon driver iio_hwmon.
> +
> +   To compile this driver as a module, choose M here: the
> +   module will be called sunxi-gpadc-mfd.
> +
>  config MFD_ATMEL_HLCDC
>   tristate "Atmel HLCDC (High-end LCD Controller)"
>   select MFD_CORE
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index 42a66e1..dcf43cd 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -201,6 +201,8 @@ obj-$(CONFIG_MFD_DLN2)+= dln2.o
>  obj-$(CONFIG_MFD_RT5033) += rt5033.o
>  obj-$(CONFIG_MFD_SKY81452)   += sky81452.o
>  
> +obj-$(CONFIG_MFD_SUNXI_ADC)  += sunxi-gpadc-mfd.o
> +
>  intel-soc-pmic-objs  := intel_soc_pmic_core.o intel_soc_pmic_crc.o
>  intel-soc-pmic-$(CONFIG_INTEL_PMC_IPC)   += intel_soc_pmic_bxtwc.o
>  obj-$(CONFIG_INTEL_SOC_PMIC) += intel-soc-pmic.o
> diff --git a/drivers/mfd/sunxi-gpadc-mfd.c b/drivers/mfd/sunxi-gpadc-mfd.c
> new file mode 100644
> index 000..f0005a6
> --- /dev/null
> +++ b/drivers/mfd/sunxi-gpadc-mfd.c
> @@ -0,0 +1,197 @@
> +/* ADC MFD core driver for sunxi platforms
> + *
> + * Copyright (c) 2016 Quentin Schulz 
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published 
> by
> + * the Free Software Foundation.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +
> +#define SUNXI_IRQ_FIFO_DATA  0
> +#define SUNXI_IRQ_TEMP_DATA  1
> +
> +static struct resource adc_resources[] = {
> + {
> + .name   = "FIFO_DATA_PENDING",
> + .start  = SUNXI_IRQ_FIFO_DATA,
> + .end= SUNXI_IRQ_FIFO_DATA,
> + .flags  = IORESOURCE_IRQ,
> + }, {
> + .name   = "TEMP_DATA_PENDING",
> + .start  = SUNXI_IRQ_TEMP_DATA,
> + .end= SUNXI_IRQ_TEMP_DATA,
> + .flags  = IORESOURCE_IRQ,
> + },
> +};
> +
> +static const struct regmap_irq sunxi_gpadc_mfd_regmap_irq[] = {
> + REGMAP_IRQ_REG(SUNXI_IRQ_FIFO_DATA, 0, BIT(16)),
> + REGMAP_IRQ_REG(SUNXI_IRQ_TEMP_DATA, 0, BIT(18)),
> +};
> +
> +static const struct regmap_irq_chip sunxi_gpadc_mfd_regmap_irq_chip = {
> + .name = "sunxi_gpadc_mfd_irq_chip",
> + .status_base = SUNXI_GPADC_TP_INT_FIFOS,
> + .ack_base = SUNXI_GPADC_TP_INT_FIFOS,
> + .mask_base = SUNXI_GPADC_TP_INT_FIFOC,
> + .init_ack_masked = true,
> + .mask_invert = true,
> + .irqs = sunxi_gpadc_mfd_regmap_irq,
> + .num_irqs = ARRAY_SIZE(sunxi_gpadc_mfd_regmap_irq),
> + .num_regs = 1,
> +};
> +
> +static struct mfd_cell sun4i_gpadc_mfd_cells[] = {
> + {
> + .name   = "sun4i-a10-gpadc-iio",
> + .resources = adc_resources,
> + .num_resources = ARRAY_SIZE(adc_resources),
> + }, {
> + .name = "iio_hwmon",
> + }
> +};
> +
> +static struct mfd_cell sun5i_gpadc_mfd_cells[] = {
> + {
> + .name   = "sun5i-a13-gpadc-iio",
> + .resources = adc_resources,

Re: [PATCH v2 4/4] hwmon: iio: add label for channels read by iio_hwmon

[Jonathan Cameron]

On 15/07/16 10:59, Quentin Schulz wrote:
> Currently, iio_hwmon only exposes values of the IIO channels it can read
> but no label by channel is exposed.
> 
> This adds exposition of sysfs files containing label for IIO channels it
> can read based on extended_name field of the iio_chan_spec of the channel.
> If the extended_name field is empty, the sysfs file is not created by
> iio_hwmon.
Hmm. This is not the intent of extended name at all.  That exists to add
a small amount of information to an constructed IIO channel name.
Typically it's used to indicate physically wired stuff like:

in_voltage0_vdd_raw for cases where that channel of the ADC is hard wired
to the vdd.  In this particular case the use might actually work as the
vdd makes it clear it's a voltage - in general that's not the case.

Use of extended_name at all in IIO is only done after extensive review.
It adds nasty custom ABI to a device, so the gain has to be considerable
to use it.

When I read your original suggestion of adding labels, I was expecting the
use of datasheet_name.  That has the advantage of being well defined by
the datasheet (if not it should not be provided) + not being used in
the construction of the IIO channel related attributes.  However, that
may still not correspond well to the expected labelling in hwmon.
Thinking more on this, the label is going to often be a function of how
the board is wired up...  Perhaps it should be a characteristic of the
channel_map (hence from DT or similar) rather than part of the IIO driver
itself?

At first glance hwmon labels appear to be pretty freeform...  However
we need to be very careful here as this is effectively defining a large
chunk of new ABI.

This isn't a thing that I have a particularly clear view on (as you
might be able to tell ;).  Other opinions sought!

Jonathan
> 
> Signed-off-by: Quentin Schulz 
> ---
> 
> patch added in v2
> 
>  drivers/hwmon/iio_hwmon.c | 77 
> +--
>  1 file changed, 68 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/hwmon/iio_hwmon.c b/drivers/hwmon/iio_hwmon.c
> index c0da4d9..28d15b2 100644
> --- a/drivers/hwmon/iio_hwmon.c
> +++ b/drivers/hwmon/iio_hwmon.c
> @@ -16,6 +16,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  /**
> @@ -57,12 +58,26 @@ static ssize_t iio_hwmon_read_val(struct device *dev,
>   return sprintf(buf, "%d\n", result);
>  }
>  
> +static ssize_t iio_hwmon_read_label(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct sensor_device_attribute *sattr = to_sensor_dev_attr(attr);
> + struct iio_hwmon_state *state = dev_get_drvdata(dev);
> + const char *label = state->channels[sattr->index].channel->extend_name;
> +
> + if (label)
> + return sprintf(buf, "%s\n", label);
> +
> + return 0;
> +}
> +
>  static int iio_hwmon_probe(struct platform_device *pdev)
>  {
>   struct device *dev = >dev;
>   struct iio_hwmon_state *st;
> - struct sensor_device_attribute *a;
> - int ret, i;
> + struct sensor_device_attribute *a, *b;
> + int ret, i, j = 0;
>   int in_i = 1, temp_i = 1, curr_i = 1, humidity_i = 1;
>   enum iio_chan_type type;
>   struct iio_channel *channels;
> @@ -92,7 +107,8 @@ static int iio_hwmon_probe(struct platform_device *pdev)
>   st->num_channels++;
>  
>   st->attrs = devm_kzalloc(dev,
> -  sizeof(*st->attrs) * (st->num_channels + 1),
> +  sizeof(*st->attrs) *
> +  (2 * st->num_channels + 1),
>GFP_KERNEL);
>   if (st->attrs == NULL) {
>   ret = -ENOMEM;
> @@ -107,6 +123,18 @@ static int iio_hwmon_probe(struct platform_device *pdev)
>   }
>  
>   sysfs_attr_init(>dev_attr.attr);
> +
> + b = NULL;
> + if (st->channels[i].channel->extend_name) {
> + b = devm_kzalloc(dev, sizeof(*b), GFP_KERNEL);
> + if (b == NULL) {
> + ret = -ENOMEM;
> + goto error_release_channels;
> + }
> +
> + sysfs_attr_init(>dev_attr.attr);
> + }
> +
>   ret = iio_get_channel_type(>channels[i], );
>   if (ret < 0)
>   goto error_release_channels;
> @@ -115,35 +143,66 @@ static int iio_hwmon_probe(struct platform_device *pdev)
>   case IIO_VOLTAGE:
>   a->dev_attr.attr.name = kasprintf(GFP_KERNEL,
> "in%d_input",
> -   in_i++);
> +   in_i);
> + if (b)
> + b->dev_attr.attr.name = 

Re: [PATCH v4 2/3] iio: adc: mt2701: Add Mediatek auxadc driver for mt2701.

[Jonathan Cameron]

On 11/07/16 07:39, Zhiyong Tao wrote:
> Add Mediatek auxadc driver based on iio.
> It will register a device in iio and support iio.
> So thermal can read auxadc channel to sample data by iio device.
> It is tested successfully on mt2701 platform.
> Mt8173 and mt6577 platforms are not tested.
> But the expectation is compatible.
> 
> Signed-off-by: Zhiyong Tao 
Looks good to me.  Couple of really minor points / questions inline...

Just one thing to confirm... (you probably already answered this!)
What are the units of the voltage channels?

Jonathan
> ---
>  drivers/iio/adc/Kconfig |   13 ++
>  drivers/iio/adc/Makefile|1 +
>  drivers/iio/adc/mt6577_auxadc.c |  294 
> +++
>  3 files changed, 308 insertions(+)
>  create mode 100644 drivers/iio/adc/mt6577_auxadc.c
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 25378c5..14929fc 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -305,6 +305,19 @@ config MCP3422
> This driver can also be built as a module. If so, the module will be
> called mcp3422.
>  
> +config MEDIATEK_MT6577_AUXADC
> +tristate "MediaTek AUXADC driver"
> +depends on ARCH_MEDIATEK || COMPILE_TEST
> +depends on HAS_IOMEM
> +help
> +  Say yes here to enable support for MediaTek mt65xx AUXADC.
> +
> +  The driver supports immediate mode operation to read from one of 
> sixteen
> +  channels (external or internal).
> +
> +  This driver can also be built as a module. If so, the module will 
> be
> +  called mt6577_auxadc.
> +
>  config MEN_Z188_ADC
>   tristate "MEN 16z188 ADC IP Core support"
>   depends on MCB
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index 38638d4..8306347 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -30,6 +30,7 @@ obj-$(CONFIG_MAX1027) += max1027.o
>  obj-$(CONFIG_MAX1363) += max1363.o
>  obj-$(CONFIG_MCP320X) += mcp320x.o
>  obj-$(CONFIG_MCP3422) += mcp3422.o
> +obj-$(CONFIG_MEDIATEK_MT6577_AUXADC) += mt6577_auxadc.o
>  obj-$(CONFIG_MEN_Z188_ADC) += men_z188_adc.o
>  obj-$(CONFIG_MXS_LRADC) += mxs-lradc.o
>  obj-$(CONFIG_NAU7802) += nau7802.o
> diff --git a/drivers/iio/adc/mt6577_auxadc.c b/drivers/iio/adc/mt6577_auxadc.c
> new file mode 100644
> index 000..c7cc901
> --- /dev/null
> +++ b/drivers/iio/adc/mt6577_auxadc.c
> @@ -0,0 +1,294 @@
> +/*
> + * Copyright (c) 2016 MediaTek Inc.
> + * Author: Zhiyong Tao 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that 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.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/* Register definitions */
> +#define MT6577_AUXADC_CON00x00
> +#define MT6577_AUXADC_CON10x04
> +#define MT6577_AUXADC_CON20x10
> +#define MT6577_AUXADC_STA BIT(0)
> +
> +#define MT6577_AUXADC_DAT00x14
> +#define MT6577_AUXADC_RDY0BIT(12)
> +
> +#define MT6577_AUXADC_MISC0x94
> +#define MT6577_AUXADC_PDN_EN  BIT(14)
> +
> +#define MT6577_AUXADC_DAT_MASK0xfff
> +#define MT6577_AUXADC_SLEEP_US1000
> +#define MT6577_AUXADC_TIMEOUT_US  1
> +#define MT6577_AUXADC_POWER_READY_MS  1
> +#define MT6577_AUXADC_SAMPLE_READY_US 25
> +
> +struct mt6577_auxadc_device {
> + void __iomem *reg_base;
> + struct clk *adc_clk;
> + struct mutex lock;
> +};
> +
> +#define MT6577_AUXADC_CHANNEL(idx) { \
> + .type = IIO_VOLTAGE,\
> + .indexed = 1,   \
> + .channel = (idx),   \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED), \
> +}
> +
> +static const struct iio_chan_spec mt6577_auxadc_iio_channels[] = {
> + MT6577_AUXADC_CHANNEL(0),
> + MT6577_AUXADC_CHANNEL(1),
> + MT6577_AUXADC_CHANNEL(2),
> + MT6577_AUXADC_CHANNEL(3),
> + MT6577_AUXADC_CHANNEL(4),
> + MT6577_AUXADC_CHANNEL(5),
> + MT6577_AUXADC_CHANNEL(6),
> + MT6577_AUXADC_CHANNEL(7),
> + MT6577_AUXADC_CHANNEL(8),
> + MT6577_AUXADC_CHANNEL(9),
> + MT6577_AUXADC_CHANNEL(10),
> + MT6577_AUXADC_CHANNEL(11),
> + MT6577_AUXADC_CHANNEL(12),
> + MT6577_AUXADC_CHANNEL(13),
> + 

Re: [PATCH 03/10] iio: core: Add double tap as possible gesture

[Jonathan Cameron]

On 18/07/16 08:02, Enric Balletbo i Serra wrote:
> From: Gwendal Grignou 
> 
> This is an interface change: however, the sysfs entry is based on string,
> so if other gestures are added on the trunk in the meantime, we will
> still be able to merge this change.
> 
> Signed-off-by: Gwendal Grignou 
> Signed-off-by: Guenter Roeck 
> [enric: Rebased and resolved conflicts]
> Signed-off-by: Enric Balletbo i Serra 
So you are creating an entire channel type for double tap?

Not keen on this I'm afraid.

The activity types in there are the moment are not events, but rather
attempts to estimate the 'likelihood' that a given activity is currently
being undertaken.

Hmm. Double tap is more of an event type.  It's one I've been wondering
how to describe for a while... At the end of the day it's just a
reasonably sophisticated filter - kind of a 'magic' version of Rate
of Change.  Unfortunately there isn't a clean mathematical definition
as it can be implemented in lots of ways.  

I guess the best may be to just have it as an event type on it's own...

What do others think?

Jonathan
> ---
>  drivers/iio/industrialio-core.c | 1 +
>  include/uapi/linux/iio/types.h  | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index e6319a9..f700e67 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -119,6 +119,7 @@ static const char * const iio_modifier_names[] = {
>   [IIO_MOD_Q] = "q",
>   [IIO_MOD_CO2] = "co2",
>   [IIO_MOD_VOC] = "voc",
> + [IIO_MOD_DOUBLE_TAP] = "double_tap",
>  };
>  
>  /* relies on pairs of these shared then separate */
> diff --git a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h
> index b0916fc..c290167 100644
> --- a/include/uapi/linux/iio/types.h
> +++ b/include/uapi/linux/iio/types.h
> @@ -79,6 +79,7 @@ enum iio_modifier {
>   IIO_MOD_CO2,
>   IIO_MOD_VOC,
>   IIO_MOD_LIGHT_UV,
> + IIO_MOD_DOUBLE_TAP,
>  };
>  
>  enum iio_event_type {
> 

Re: [PATCH] staging: iio: light: isl29018/28: remove I2C_CLASS_HWMON .class setting

[Jonathan Cameron]

On 18/07/16 09:50, Laxman Dewangan wrote:
> 
> On Saturday 16 July 2016 07:58 AM, Alison Schofield wrote:
>> I2C_CLASS_HWMON is for a hardware monitoring chip wanting
>> auto-detection.  IIO drivers don't typically use .class.
>> Remove it.
>>
>> Signed-off-by: Alison Schofield 
>> Cc: Daniel Baluta 
>>
> 
> Agree.
> Acked-by: Laxman Dewangan 
Any resulting ABI change from dropping it?
I don't think so, as it doesn't support detection anyway,
but best to be sure ;)

> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 08/10] iio: cros_ec_sensors_ring: add ChromeOS EC Sensors Ring

[Jonathan Cameron]

On 18/07/16 08:02, Enric Balletbo i Serra wrote:
> Add support for handling sensor events FIFO produced by the sensor
> hub. A single device with a buffer will collect all samples produced
> by the sensors managed by the CrosEC sensor hub.
So you are defining a different device to support the buffer?
That's 'unusual'...
> 
> Signed-off-by: Guenter Roeck 
> Signed-off-by: Enric Balletbo i Serra 
> ---
>  drivers/iio/common/cros_ec_sensors/Kconfig |   9 +
>  drivers/iio/common/cros_ec_sensors/Makefile|   1 +
>  .../common/cros_ec_sensors/cros_ec_sensors_ring.c  | 541 
> +
>  3 files changed, 551 insertions(+)
>  create mode 100644 drivers/iio/common/cros_ec_sensors/cros_ec_sensors_ring.c
> 
> diff --git a/drivers/iio/common/cros_ec_sensors/Kconfig 
> b/drivers/iio/common/cros_ec_sensors/Kconfig
> index 22b4211..778c3bf 100644
> --- a/drivers/iio/common/cros_ec_sensors/Kconfig
> +++ b/drivers/iio/common/cros_ec_sensors/Kconfig
> @@ -39,3 +39,12 @@ config IIO_CROS_EC_ACTIVITY
> Activities can be simple (low/no motion) or more complex (riding 
> train).
> They are being reported by physical devices or the EC itself.
> Creates an IIO device to manage all activities.
> +
> +config IIO_CROS_EC_SENSORS_RING
> + tristate "ChromeOS EC Sensors Ring"
> + depends on IIO_CROS_EC_SENSORS || IIO_CROS_EC_LIGHT_PROX
> + help
> +   Add support for handling sensor events FIFO produced by
> +   the sensor hub.
> +   A single device with a buffer will collect all samples produced
> +   by the sensors managed by the CrosEC sensor hub
> diff --git a/drivers/iio/common/cros_ec_sensors/Makefile 
> b/drivers/iio/common/cros_ec_sensors/Makefile
> index 8f54f1e..0eb3fc5 100644
> --- a/drivers/iio/common/cros_ec_sensors/Makefile
> +++ b/drivers/iio/common/cros_ec_sensors/Makefile
> @@ -6,3 +6,4 @@ obj-$(CONFIG_IIO_CROS_EC_SENSORS_CORE) += 
> cros_ec_sensors_core.o
>  obj-$(CONFIG_IIO_CROS_EC_SENSORS) += cros_ec_sensors.o
>  obj-$(CONFIG_IIO_CROS_EC_LIGHT_PROX) += cros_ec_light_prox.o
>  obj-$(CONFIG_IIO_CROS_EC_ACTIVITY) += cros_ec_activity.o
> +obj-$(CONFIG_IIO_CROS_EC_SENSORS_RING) += cros_ec_sensors_ring.o
> diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_ring.c 
> b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_ring.c
> new file mode 100644
> index 000..1c74df9
> --- /dev/null
> +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_ring.c
> @@ -0,0 +1,541 @@
> +/*
> + * cros_ec_sensors_ring - Driver for Chrome OS EC Sensor hub FIFO.
> + *
> + * Copyright (C) 2015 Google, Inc
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that 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.
> + *
> + * This driver uses the cros-ec interface to communicate with the Chrome OS
> + * EC about accelerometer data. Accelerometer access is presented through
> + * iio sysfs.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "cros_ec_sensors_core.h"
> +
> +/* The ring is a FIFO that return sensor information from
> + * the single EC FIFO.
> + * There are always 5 channels returned:
> +* | ID | FLAG | X | Y | Z | Timestamp |
> + * ID is the EC sensor id
> + * FLAG is for meta data, only flush bit is defined.
> + */
> +#define CROS_EC_FLUSH_BIT 1
> +
> +enum {
> + CHANNEL_SENSOR_ID,
> + CHANNEL_SENSOR_FLAG,
Ah, I'm beginning to understand.

I think you really need to demux these into the appropriate devices individual
buffers... If these are coming in fairly randomly (guessing so?) then you'll
want a top level mfd pushing data to each of the child devices (representing
the different possible sensors).

Simply pushing it into a buffer with metadata doesn't fit with the IIO ABI
at all.

Note that there is no obligation to have any triggers involved at all. Here I'd
suggest you don't as it'll just make life difficult for little gain.

> + CHANNEL_X,
> + CHANNEL_Y,
> + CHANNEL_Z,
> + CHANNEL_TIMESTAMP,
> + MAX_CHANNEL,
> +};
> +
> +enum {
> + LAST_TS,
> + NEW_TS,
> + ALL_TS
> +};
> +
> +#define CROS_EC_SENSOR_MAX 16
> +
> +struct cros_ec_fifo_info {
> + struct ec_response_motion_sense_fifo_info info;
> + uint16_t lost[CROS_EC_SENSOR_MAX];
> +};
> +
single blank line is plenty.
> +
> +struct cros_ec_sensors_ring_sample {
> + uint8_t sensor_id;
> + uint8_t flag;
> + int16_t  vector[MAX_AXIS];
> + s64  timestamp;
> +} __packed;
> +
> +/* 

Re: [PATCH v2 3/4] mfd: add support for Allwinner SoCs ADC

[Jonathan Cameron]

On 15/07/16 10:59, Quentin Schulz wrote:
> The Allwinner SoCs all have an ADC that can also act as a touchscreen
> controller and a thermal sensor. For now, only the ADC and the thermal
> sensor drivers are probed by the MFD, the touchscreen controller support
> will be added later.
> 
> Signed-off-by: Quentin Schulz 
Hmm. Previous patch includes the header this one creates.  Ordering issue?
The depends kind of prevents build failures by ensuring that can't be built
until this one is in place, but it is certainly an ugly way to do it.

Few little bits innline.
> ---
> 
> v2:
>  - add license headers,
>  - reorder alphabetically includes,
>  - add SUNXI_GPADC_ prefixes for defines,
> 
>  drivers/mfd/Kconfig |  14 +++
>  drivers/mfd/Makefile|   2 +
>  drivers/mfd/sunxi-gpadc-mfd.c   | 197 
> 
>  include/linux/mfd/sunxi-gpadc-mfd.h |  23 +
>  4 files changed, 236 insertions(+)
>  create mode 100644 drivers/mfd/sunxi-gpadc-mfd.c
>  create mode 100644 include/linux/mfd/sunxi-gpadc-mfd.h
> 
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 1bcf601..67b55d0 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -82,6 +82,20 @@ config MFD_ATMEL_FLEXCOM
> by the probe function of this MFD driver according to a device tree
> property.
>  
> +config MFD_SUNXI_ADC
> + tristate "ADC MFD core driver for sunxi platforms"
> + select MFD_CORE
> + select REGMAP_MMIO
> + help
> +   Select this to get support for Allwinner SoCs (A10, A13 and A31) ADC.
> +   This driver will only map the hardware interrupt and registers, you
> +   have to select individual drivers based on this MFD to be able to use
> +   the ADC or the thermal sensor. This will try to probe the ADC driver
> +   sunxi-gpadc-iio and the hwmon driver iio_hwmon.
> +
> +   To compile this driver as a module, choose M here: the
> +   module will be called sunxi-gpadc-mfd.
> +
>  config MFD_ATMEL_HLCDC
>   tristate "Atmel HLCDC (High-end LCD Controller)"
>   select MFD_CORE
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index 42a66e1..dcf43cd 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -201,6 +201,8 @@ obj-$(CONFIG_MFD_DLN2)+= dln2.o
>  obj-$(CONFIG_MFD_RT5033) += rt5033.o
>  obj-$(CONFIG_MFD_SKY81452)   += sky81452.o
>  
> +obj-$(CONFIG_MFD_SUNXI_ADC)  += sunxi-gpadc-mfd.o
> +
>  intel-soc-pmic-objs  := intel_soc_pmic_core.o intel_soc_pmic_crc.o
>  intel-soc-pmic-$(CONFIG_INTEL_PMC_IPC)   += intel_soc_pmic_bxtwc.o
>  obj-$(CONFIG_INTEL_SOC_PMIC) += intel-soc-pmic.o
> diff --git a/drivers/mfd/sunxi-gpadc-mfd.c b/drivers/mfd/sunxi-gpadc-mfd.c
> new file mode 100644
> index 000..f0005a6
> --- /dev/null
> +++ b/drivers/mfd/sunxi-gpadc-mfd.c
> @@ -0,0 +1,197 @@
> +/* ADC MFD core driver for sunxi platforms
> + *
> + * Copyright (c) 2016 Quentin Schulz 
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published 
> by
> + * the Free Software Foundation.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +
> +#define SUNXI_IRQ_FIFO_DATA  0
> +#define SUNXI_IRQ_TEMP_DATA  1
> +
> +static struct resource adc_resources[] = {
> + {
> + .name   = "FIFO_DATA_PENDING",
> + .start  = SUNXI_IRQ_FIFO_DATA,
> + .end= SUNXI_IRQ_FIFO_DATA,
> + .flags  = IORESOURCE_IRQ,
> + }, {
> + .name   = "TEMP_DATA_PENDING",
> + .start  = SUNXI_IRQ_TEMP_DATA,
> + .end= SUNXI_IRQ_TEMP_DATA,
> + .flags  = IORESOURCE_IRQ,
> + },
> +};
> +
> +static const struct regmap_irq sunxi_gpadc_mfd_regmap_irq[] = {
> + REGMAP_IRQ_REG(SUNXI_IRQ_FIFO_DATA, 0, BIT(16)),
> + REGMAP_IRQ_REG(SUNXI_IRQ_TEMP_DATA, 0, BIT(18)),
> +};
> +
> +static const struct regmap_irq_chip sunxi_gpadc_mfd_regmap_irq_chip = {
> + .name = "sunxi_gpadc_mfd_irq_chip",
> + .status_base = SUNXI_GPADC_TP_INT_FIFOS,
> + .ack_base = SUNXI_GPADC_TP_INT_FIFOS,
> + .mask_base = SUNXI_GPADC_TP_INT_FIFOC,
> + .init_ack_masked = true,
> + .mask_invert = true,
> + .irqs = sunxi_gpadc_mfd_regmap_irq,
> + .num_irqs = ARRAY_SIZE(sunxi_gpadc_mfd_regmap_irq),
> + .num_regs = 1,
> +};
> +
> +static struct mfd_cell sun4i_gpadc_mfd_cells[] = {
> + {
> + .name   = "sun4i-a10-gpadc-iio",
> + .resources = adc_resources,
> + .num_resources = ARRAY_SIZE(adc_resources),
> + }, {
> + .name = "iio_hwmon",
> + }
> +};
> +
> +static struct mfd_cell sun5i_gpadc_mfd_cells[] = {
> + {
> + .name   = "sun5i-a13-gpadc-iio",
> + .resources = adc_resources,
> + .num_resources = ARRAY_SIZE(adc_resources),
> +   

Re: [PATCH 01/10] mfd: cros_ec: add ChromeOS EC sensor platform information.

[Jonathan Cameron]

On 18/07/16 08:02, Enric Balletbo i Serra wrote:
> The platform information will be used for the new cros-ec sensors driver
> that presents sensors attached to the ChromeOS Embedded Controller.
> 
> Signed-off-by: Gwendal Grignou 
> Signed-off-by: Enric Balletbo i Serra 
Bring this in as an when you need it in the rest of the series.
It certainly doesn't need it's own patch.
> ---
>  include/linux/mfd/cros_ec.h | 10 ++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
> index d6539c1..cd4833a 100644
> --- a/include/linux/mfd/cros_ec.h
> +++ b/include/linux/mfd/cros_ec.h
> @@ -151,6 +151,16 @@ struct cros_ec_device {
>   int event_size;
>  };
>  
Check your kernel doc formatting.
> +/* struct cros_ec_sensor_platform - ChromeOS EC sensor platform information
> + *
> + * On top of cros_ec_devicem information cros_ec_sensors needs.
> + *
> + * @sensor_num: Id of the sensor, as reported by the EC.
> + */
> +struct cros_ec_sensor_platform {
> + u8 sensor_num;
> +};
> +
>  /* struct cros_ec_platform - ChromeOS EC platform information
>   *
>   * @ec_name: name of EC device (e.g. 'cros-ec', 'cros-pd', ...)
> 

Re: [PATCH 07/10] iio: cros_ec_activity: add ChromeOS EC Activity Sensors

[Jonathan Cameron]

On 18/07/16 08:02, Enric Balletbo i Serra wrote:
> Handle activity events detections presented by the ChromeOS
> EC Sensor hub. Activities can be simple (low/no motion) or more complex
> (riding train). They are being reported by physical devices or the EC
> itself.
> 
> Signed-off-by: Guenter Roeck 
> Signed-off-by: Enric Balletbo i Serra 
Interesting.  What docs exist?

For now this is probably more of an RFC than a finished driver, but
certainly interesting to know this stuff exists.

Jonathan
> ---
>  drivers/iio/common/cros_ec_sensors/Kconfig |  10 +
>  drivers/iio/common/cros_ec_sensors/Makefile|   1 +
>  .../iio/common/cros_ec_sensors/cros_ec_activity.c  | 294 
> +
>  3 files changed, 305 insertions(+)
>  create mode 100644 drivers/iio/common/cros_ec_sensors/cros_ec_activity.c
> 
> diff --git a/drivers/iio/common/cros_ec_sensors/Kconfig 
> b/drivers/iio/common/cros_ec_sensors/Kconfig
> index 02559d2..22b4211 100644
> --- a/drivers/iio/common/cros_ec_sensors/Kconfig
> +++ b/drivers/iio/common/cros_ec_sensors/Kconfig
> @@ -29,3 +29,13 @@ config IIO_CROS_EC_LIGHT_PROX
> presented by the ChromeOS EC Sensor hub.
> Creates an IIO device for each functions.
> Only one source is exposed by the EC.
> +
> +config IIO_CROS_EC_ACTIVITY
> + tristate "ChromeOS EC Activity Sensors"
> + select IIO_CROS_EC_SENSORS_CORE
> + help
> +   Module to handle activity events detections presented by the ChromeOS
> +   EC Sensor hub.
> +   Activities can be simple (low/no motion) or more complex (riding 
> train).
> +   They are being reported by physical devices or the EC itself.
> +   Creates an IIO device to manage all activities.
> diff --git a/drivers/iio/common/cros_ec_sensors/Makefile 
> b/drivers/iio/common/cros_ec_sensors/Makefile
> index 7aaf2a2..8f54f1e 100644
> --- a/drivers/iio/common/cros_ec_sensors/Makefile
> +++ b/drivers/iio/common/cros_ec_sensors/Makefile
> @@ -5,3 +5,4 @@
>  obj-$(CONFIG_IIO_CROS_EC_SENSORS_CORE) += cros_ec_sensors_core.o
>  obj-$(CONFIG_IIO_CROS_EC_SENSORS) += cros_ec_sensors.o
>  obj-$(CONFIG_IIO_CROS_EC_LIGHT_PROX) += cros_ec_light_prox.o
> +obj-$(CONFIG_IIO_CROS_EC_ACTIVITY) += cros_ec_activity.o
> diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_activity.c 
> b/drivers/iio/common/cros_ec_sensors/cros_ec_activity.c
> new file mode 100644
> index 000..b2e39d6
> --- /dev/null
> +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_activity.c
> @@ -0,0 +1,294 @@
> +/*
> + * cros_ec_sensors_activity - Driver for activities/gesture recognition.
> + *
> + * Copyright (C) 2015 Google, Inc
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that 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.
> + *
> + * This driver uses the cros-ec interface to communicate with the Chrome OS
> + * EC about accelerometer data. Accelerometer access is presented through
> + * iio sysfs.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "cros_ec_sensors_core.h"
> +
> +/* st data for ec_sensors iio driver. */
> +struct cros_ec_sensors_state {
> + /* Shared by all sensors */
> + struct cros_ec_sensors_core_state core;
> +
> + struct iio_chan_spec *channels;
> + unsigned int nb_activities;
> +};
> +
> +static const struct iio_event_spec cros_ec_activity_single_shot[] = {
> + {
> + .type = IIO_EV_TYPE_CHANGE,
> + /* significant motion trigger when we get out of still. */
> + .dir = IIO_EV_DIR_FALLING,
> + .mask_separate = BIT(IIO_EV_INFO_ENABLE),
> +  },
> +};
> +
> +static int ec_sensors_read(struct iio_dev *indio_dev,
> +   struct iio_chan_spec const *chan,
> +   int *val, int *val2, long mask)
> +{
> + dev_warn(_dev->dev, "%s: Not Expected: %d\n", __func__,
> +  chan->channel2);
> + return -EPERM;
> +}
> +
> +static int ec_sensors_write(struct iio_dev *indio_dev,
> +struct iio_chan_spec const *chan,
> +int val, int val2, long mask)
> +{
> + dev_warn(_dev->dev, "%s: Not Expected: %d\n", __func__,
> +  chan->channel2);
> + return -EPERM;
> +}
> +
> +static int cros_ec_read_event_config(struct iio_dev *indio_dev,
> +  const struct iio_chan_spec *chan,
> +  enum iio_event_type type,
> +  enum 

Re: [PATCH 01/10] mfd: cros_ec: add ChromeOS EC sensor platform information.

[Jonathan Cameron]

On 18/07/16 08:02, Enric Balletbo i Serra wrote:
> The platform information will be used for the new cros-ec sensors driver
> that presents sensors attached to the ChromeOS Embedded Controller.
> 
> Signed-off-by: Gwendal Grignou 
> Signed-off-by: Enric Balletbo i Serra 
Bring this in as an when you need it in the rest of the series.
It certainly doesn't need it's own patch.
> ---
>  include/linux/mfd/cros_ec.h | 10 ++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
> index d6539c1..cd4833a 100644
> --- a/include/linux/mfd/cros_ec.h
> +++ b/include/linux/mfd/cros_ec.h
> @@ -151,6 +151,16 @@ struct cros_ec_device {
>   int event_size;
>  };
>  
Check your kernel doc formatting.
> +/* struct cros_ec_sensor_platform - ChromeOS EC sensor platform information
> + *
> + * On top of cros_ec_devicem information cros_ec_sensors needs.
> + *
> + * @sensor_num: Id of the sensor, as reported by the EC.
> + */
> +struct cros_ec_sensor_platform {
> + u8 sensor_num;
> +};
> +
>  /* struct cros_ec_platform - ChromeOS EC platform information
>   *
>   * @ec_name: name of EC device (e.g. 'cros-ec', 'cros-pd', ...)
> 

Re: [PATCH 07/10] iio: cros_ec_activity: add ChromeOS EC Activity Sensors

[Jonathan Cameron]

On 18/07/16 08:02, Enric Balletbo i Serra wrote:
> Handle activity events detections presented by the ChromeOS
> EC Sensor hub. Activities can be simple (low/no motion) or more complex
> (riding train). They are being reported by physical devices or the EC
> itself.
> 
> Signed-off-by: Guenter Roeck 
> Signed-off-by: Enric Balletbo i Serra 
Interesting.  What docs exist?

For now this is probably more of an RFC than a finished driver, but
certainly interesting to know this stuff exists.

Jonathan
> ---
>  drivers/iio/common/cros_ec_sensors/Kconfig |  10 +
>  drivers/iio/common/cros_ec_sensors/Makefile|   1 +
>  .../iio/common/cros_ec_sensors/cros_ec_activity.c  | 294 
> +
>  3 files changed, 305 insertions(+)
>  create mode 100644 drivers/iio/common/cros_ec_sensors/cros_ec_activity.c
> 
> diff --git a/drivers/iio/common/cros_ec_sensors/Kconfig 
> b/drivers/iio/common/cros_ec_sensors/Kconfig
> index 02559d2..22b4211 100644
> --- a/drivers/iio/common/cros_ec_sensors/Kconfig
> +++ b/drivers/iio/common/cros_ec_sensors/Kconfig
> @@ -29,3 +29,13 @@ config IIO_CROS_EC_LIGHT_PROX
> presented by the ChromeOS EC Sensor hub.
> Creates an IIO device for each functions.
> Only one source is exposed by the EC.
> +
> +config IIO_CROS_EC_ACTIVITY
> + tristate "ChromeOS EC Activity Sensors"
> + select IIO_CROS_EC_SENSORS_CORE
> + help
> +   Module to handle activity events detections presented by the ChromeOS
> +   EC Sensor hub.
> +   Activities can be simple (low/no motion) or more complex (riding 
> train).
> +   They are being reported by physical devices or the EC itself.
> +   Creates an IIO device to manage all activities.
> diff --git a/drivers/iio/common/cros_ec_sensors/Makefile 
> b/drivers/iio/common/cros_ec_sensors/Makefile
> index 7aaf2a2..8f54f1e 100644
> --- a/drivers/iio/common/cros_ec_sensors/Makefile
> +++ b/drivers/iio/common/cros_ec_sensors/Makefile
> @@ -5,3 +5,4 @@
>  obj-$(CONFIG_IIO_CROS_EC_SENSORS_CORE) += cros_ec_sensors_core.o
>  obj-$(CONFIG_IIO_CROS_EC_SENSORS) += cros_ec_sensors.o
>  obj-$(CONFIG_IIO_CROS_EC_LIGHT_PROX) += cros_ec_light_prox.o
> +obj-$(CONFIG_IIO_CROS_EC_ACTIVITY) += cros_ec_activity.o
> diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_activity.c 
> b/drivers/iio/common/cros_ec_sensors/cros_ec_activity.c
> new file mode 100644
> index 000..b2e39d6
> --- /dev/null
> +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_activity.c
> @@ -0,0 +1,294 @@
> +/*
> + * cros_ec_sensors_activity - Driver for activities/gesture recognition.
> + *
> + * Copyright (C) 2015 Google, Inc
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that 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.
> + *
> + * This driver uses the cros-ec interface to communicate with the Chrome OS
> + * EC about accelerometer data. Accelerometer access is presented through
> + * iio sysfs.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "cros_ec_sensors_core.h"
> +
> +/* st data for ec_sensors iio driver. */
> +struct cros_ec_sensors_state {
> + /* Shared by all sensors */
> + struct cros_ec_sensors_core_state core;
> +
> + struct iio_chan_spec *channels;
> + unsigned int nb_activities;
> +};
> +
> +static const struct iio_event_spec cros_ec_activity_single_shot[] = {
> + {
> + .type = IIO_EV_TYPE_CHANGE,
> + /* significant motion trigger when we get out of still. */
> + .dir = IIO_EV_DIR_FALLING,
> + .mask_separate = BIT(IIO_EV_INFO_ENABLE),
> +  },
> +};
> +
> +static int ec_sensors_read(struct iio_dev *indio_dev,
> +   struct iio_chan_spec const *chan,
> +   int *val, int *val2, long mask)
> +{
> + dev_warn(_dev->dev, "%s: Not Expected: %d\n", __func__,
> +  chan->channel2);
> + return -EPERM;
> +}
> +
> +static int ec_sensors_write(struct iio_dev *indio_dev,
> +struct iio_chan_spec const *chan,
> +int val, int val2, long mask)
> +{
> + dev_warn(_dev->dev, "%s: Not Expected: %d\n", __func__,
> +  chan->channel2);
> + return -EPERM;
> +}
> +
> +static int cros_ec_read_event_config(struct iio_dev *indio_dev,
> +  const struct iio_chan_spec *chan,
> +  enum iio_event_type type,
> +  enum iio_event_direction dir)
> +{
> + struct 

Re: [PATCH 1/2] usb: typec: Add USB Power Delivery sink port support

[Bin Gao]

On Mon, Jul 18, 2016 at 10:07:24AM +0300, Felipe Balbi wrote:
> 
> Hi,
> 
> Bin Gao  writes:
> >> > +int pd_sink_queue_msg(struct pd_sink_msg *msg)
> >> > +{
> >> > +unsigned long flags;
> >> > +struct pd_sink_port *port;
> >> > +
> >> > +if (msg->port < 0 || msg->port >= MAX_NR_SINK_PORTS) {
> >> > +pr_err("Invalid port number\n");
> >> > +return -EINVAL;
> >> > +}
> >> > +
> >> > +port = sink_ports[msg->port];
> >> > +
> >> > +spin_lock_irqsave(>rx_lock, flags);
> >> > +list_add_tail(>list, >rx_list);
> >> > +spin_unlock_irqrestore(>rx_lock, flags);
> >> > +
> >> > +queue_work(port->rx_wq, >rx_work);
> >> 
> >> can we really queue several messages at a time? It seems unfeasible to
> >> me. It's not like we can queue several power request in a role. Why do
> >> you need this workqueue? Why don't you process message here, in place?
> > Some Type-C chargers send two messages in a short duration(less than 1 ms),
> > e.g. a SOURCE_CAPABILITY follows the previous SOURCE_CAPABILITY, or a
> > GET_SINK_CAPABILITY follows a previous SOURCE_CAPABILITY, etc. Queuing
> > message to PD stack by Type-C phy driver typically happens in a interrupt
> > context. So in this case a nested interrupt may happen. Our whole PD
> > stack while processing one message is not re-entrant so the nested
> > interrupt would cause a problem.
> 
> keep interrupts masked for as long as necessary until your message is
> processed.

Yes, that's a right way to go. 
We'll have to document this because there might be other Type-C
PHY drivers(other than Intel Whiskey Cove PMIC) to use the PD stack.

> 
> -- 
> balbi

Re: [PATCH 1/2] usb: typec: Add USB Power Delivery sink port support

[Bin Gao]

On Mon, Jul 18, 2016 at 10:07:24AM +0300, Felipe Balbi wrote:
> 
> Hi,
> 
> Bin Gao  writes:
> >> > +int pd_sink_queue_msg(struct pd_sink_msg *msg)
> >> > +{
> >> > +unsigned long flags;
> >> > +struct pd_sink_port *port;
> >> > +
> >> > +if (msg->port < 0 || msg->port >= MAX_NR_SINK_PORTS) {
> >> > +pr_err("Invalid port number\n");
> >> > +return -EINVAL;
> >> > +}
> >> > +
> >> > +port = sink_ports[msg->port];
> >> > +
> >> > +spin_lock_irqsave(>rx_lock, flags);
> >> > +list_add_tail(>list, >rx_list);
> >> > +spin_unlock_irqrestore(>rx_lock, flags);
> >> > +
> >> > +queue_work(port->rx_wq, >rx_work);
> >> 
> >> can we really queue several messages at a time? It seems unfeasible to
> >> me. It's not like we can queue several power request in a role. Why do
> >> you need this workqueue? Why don't you process message here, in place?
> > Some Type-C chargers send two messages in a short duration(less than 1 ms),
> > e.g. a SOURCE_CAPABILITY follows the previous SOURCE_CAPABILITY, or a
> > GET_SINK_CAPABILITY follows a previous SOURCE_CAPABILITY, etc. Queuing
> > message to PD stack by Type-C phy driver typically happens in a interrupt
> > context. So in this case a nested interrupt may happen. Our whole PD
> > stack while processing one message is not re-entrant so the nested
> > interrupt would cause a problem.
> 
> keep interrupts masked for as long as necessary until your message is
> processed.

Yes, that's a right way to go. 
We'll have to document this because there might be other Type-C
PHY drivers(other than Intel Whiskey Cove PMIC) to use the PD stack.

> 
> -- 
> balbi

Re: [PATCH 1/2] usb: typec: Add USB Power Delivery sink port support

[Bin Gao]

On Sat, Jul 16, 2016 at 08:49:53AM +0900, Greg Kroah-Hartman wrote:
> On Fri, Jul 15, 2016 at 03:41:10PM -0700, Bin Gao wrote:
> > On Fri, Jul 15, 2016 at 02:21:48PM +0300, Felipe Balbi wrote:
> > > Greg Kroah-Hartman  writes:
> > > > On Fri, Jul 15, 2016 at 01:38:12PM +0300, Felipe Balbi wrote:
> > > >> 
> > > >> Hi,
> > > >> 
> > > >> Bin Gao  writes:
> > > >> > +static void print_message(int port, bool is_cmsg, u8 msg, bool recv)
> > > >> > +{
> > > >> > +pr_info("sink port %d: %s message %s %s\n", port,
> > > >> > +is_cmsg ? "Control" : "Data",
> > > >> > +msg_to_string(is_cmsg, msg),
> > > >> > + recv ? "received" : "sent(wait GOODCRC)");
> > > >> > +}
> > > >> 
> > > >> this is problematic. By default, we're all using 115200 8N1 baud
> > > >> rate. This message alone prints anywhere from 50 to 100 characters (I
> > > >> didn't really count properly, these are rough numbers), and that takes:
> > > >> 
> > > >> n50chars_time = 50 / (115200 / 10) = 4.3ms
> > > >> n100chars_time = 100 / (115200 / 10) = 8.6ms
> > > >> 
> > > >> Considering you have 30ms to reply with Power Request after GoodCRC, 
> > > >> and
> > > >> considering you're printing several of these messages, they become
> > > >> really expensive and eat up valuable time from tSenderReply.
> > > >
> > > > printk() should be async, so it shouldn't be that big of a deal.
> > > 
> > > I can actually see this causing problems ;-) With this pr_info(),
> > > sometimes tSenderReply times out and Source gives a HardReset. Without
> > > pr_info(), type-c analyzer tells me we reply in less than 1ms.
> > > 
> > > > What is wrong is that this isn't using dev_info().
> > > 
> > > right, that too.
> > > 
> > > -- 
> > > balbi
> > 
> > When we don't have a struct device pointer for this driver,
> 
> Then you should fix that, as this is a driver for hardware :)
This is actualy a software stack to implement the USB PD spec.
Only the USB Type-C phy driver has a device pointer.
The PD stack vs. USB Type-C phy driver is similar to TCP/IP stack
vs. ethernet driver in the kernel. We don't have a device pointer
for TCP/IP stack code either.

Thanks,
Bin

> 
> thanks,
> 
> greg k-h

Re: [PATCH 1/2] usb: typec: Add USB Power Delivery sink port support

[Bin Gao]

On Sat, Jul 16, 2016 at 08:49:53AM +0900, Greg Kroah-Hartman wrote:
> On Fri, Jul 15, 2016 at 03:41:10PM -0700, Bin Gao wrote:
> > On Fri, Jul 15, 2016 at 02:21:48PM +0300, Felipe Balbi wrote:
> > > Greg Kroah-Hartman  writes:
> > > > On Fri, Jul 15, 2016 at 01:38:12PM +0300, Felipe Balbi wrote:
> > > >> 
> > > >> Hi,
> > > >> 
> > > >> Bin Gao  writes:
> > > >> > +static void print_message(int port, bool is_cmsg, u8 msg, bool recv)
> > > >> > +{
> > > >> > +pr_info("sink port %d: %s message %s %s\n", port,
> > > >> > +is_cmsg ? "Control" : "Data",
> > > >> > +msg_to_string(is_cmsg, msg),
> > > >> > + recv ? "received" : "sent(wait GOODCRC)");
> > > >> > +}
> > > >> 
> > > >> this is problematic. By default, we're all using 115200 8N1 baud
> > > >> rate. This message alone prints anywhere from 50 to 100 characters (I
> > > >> didn't really count properly, these are rough numbers), and that takes:
> > > >> 
> > > >> n50chars_time = 50 / (115200 / 10) = 4.3ms
> > > >> n100chars_time = 100 / (115200 / 10) = 8.6ms
> > > >> 
> > > >> Considering you have 30ms to reply with Power Request after GoodCRC, 
> > > >> and
> > > >> considering you're printing several of these messages, they become
> > > >> really expensive and eat up valuable time from tSenderReply.
> > > >
> > > > printk() should be async, so it shouldn't be that big of a deal.
> > > 
> > > I can actually see this causing problems ;-) With this pr_info(),
> > > sometimes tSenderReply times out and Source gives a HardReset. Without
> > > pr_info(), type-c analyzer tells me we reply in less than 1ms.
> > > 
> > > > What is wrong is that this isn't using dev_info().
> > > 
> > > right, that too.
> > > 
> > > -- 
> > > balbi
> > 
> > When we don't have a struct device pointer for this driver,
> 
> Then you should fix that, as this is a driver for hardware :)
This is actualy a software stack to implement the USB PD spec.
Only the USB Type-C phy driver has a device pointer.
The PD stack vs. USB Type-C phy driver is similar to TCP/IP stack
vs. ethernet driver in the kernel. We don't have a device pointer
for TCP/IP stack code either.

Thanks,
Bin

> 
> thanks,
> 
> greg k-h

Re: [PATCH v3 4/4] vfs: Use dlock list for superblock's inode list

[Al Viro]

On Fri, Jul 15, 2016 at 01:39:43PM -0400, Waiman Long wrote:
>  void iterate_bdevs(void (*func)(struct block_device *, void *), void *arg)
>  {
>   struct inode *inode, *old_inode = NULL;
> + DEFINE_DLOCK_LIST_ITER(iter);
>  
> - spin_lock(_superblock->s_inode_list_lock);
> - list_for_each_entry(inode, _superblock->s_inodes, i_sb_list) {
> - struct address_space *mapping = inode->i_mapping;
> + while (dlock_list_next(_superblock->s_inodes, )) {
> + struct address_space *mapping;
>  
> + inode   = list_entry(iter.curr, struct inode, i_sb_list);
> + mapping = inode->i_mapping;

TBH, I would very much prefer something like
DEFINE_DLOCK_LIST_ITER(iter, _superblock->s_inodes);

dlist_for_each_entry(inode, , i_sb_list) {
mapping = inode->i_mapping;

> - spin_unlock(_superblock->s_inode_list_lock);
> + spin_unlock(iter.lock);

... and this might be worth dlist_{un,re}lock();

Re: [PATCH v3 4/4] vfs: Use dlock list for superblock's inode list

[Al Viro]

On Fri, Jul 15, 2016 at 01:39:43PM -0400, Waiman Long wrote:
>  void iterate_bdevs(void (*func)(struct block_device *, void *), void *arg)
>  {
>   struct inode *inode, *old_inode = NULL;
> + DEFINE_DLOCK_LIST_ITER(iter);
>  
> - spin_lock(_superblock->s_inode_list_lock);
> - list_for_each_entry(inode, _superblock->s_inodes, i_sb_list) {
> - struct address_space *mapping = inode->i_mapping;
> + while (dlock_list_next(_superblock->s_inodes, )) {
> + struct address_space *mapping;
>  
> + inode   = list_entry(iter.curr, struct inode, i_sb_list);
> + mapping = inode->i_mapping;

TBH, I would very much prefer something like
DEFINE_DLOCK_LIST_ITER(iter, _superblock->s_inodes);

dlist_for_each_entry(inode, , i_sb_list) {
mapping = inode->i_mapping;

> - spin_unlock(_superblock->s_inode_list_lock);
> + spin_unlock(iter.lock);

... and this might be worth dlist_{un,re}lock();

Re: [PATCH net-next] macvtap: correctly free skb during socket destruction

[David Miller]

From: Jason Wang 
Date: Tue, 19 Jul 2016 11:02:59 +0800

> We should use kfree_skb() instead of kfree() to free an skb.
> 
> Fixes: 362899b8725b ("macvtap: switch to use skb array")
> Reported-by: Dan Carpenter 
> Signed-off-by: Jason Wang 

Applied, thanks Jason.

Re: [PATCH net-next] macvtap: correctly free skb during socket destruction

[David Miller]

From: Jason Wang 
Date: Tue, 19 Jul 2016 11:02:59 +0800

> We should use kfree_skb() instead of kfree() to free an skb.
> 
> Fixes: 362899b8725b ("macvtap: switch to use skb array")
> Reported-by: Dan Carpenter 
> Signed-off-by: Jason Wang 

Applied, thanks Jason.

Re: [RFC PATCH 00/30] Kernel NET policy

[David Miller]

From: "Liang, Kan" 
Date: Tue, 19 Jul 2016 01:49:41 +

> Yes, rtnl will bring some overheads. But the configuration is one
> time thing for application or socket. It only happens on receiving
> first packet.

Thanks for destroying our connection rates.

This kind of overhead is simply unacceptable.

Re: [RFC PATCH 00/30] Kernel NET policy

[David Miller]

From: "Liang, Kan" 
Date: Tue, 19 Jul 2016 01:49:41 +

> Yes, rtnl will bring some overheads. But the configuration is one
> time thing for application or socket. It only happens on receiving
> first packet.

Thanks for destroying our connection rates.

This kind of overhead is simply unacceptable.

Re: [PATCH v3 1/4] lib/dlock-list: Distributed and lock-protected lists

[Al Viro]

On Fri, Jul 15, 2016 at 01:39:40PM -0400, Waiman Long wrote:

> +struct dlock_list_head_percpu {
> + struct list_head list;
> + spinlock_t lock;
> +};

> +#define DLOCK_LIST_HEAD_PERCPU_INIT(name)\
> + {   \
> + .list.prev = ,\
> + .list.next = ,\
> + .list.lock = __SPIN_LOCK_UNLOCKED(name),\

What's .list.lock and how does that even compile?

> +extern bool dlock_list_next(struct dlock_list_head *dlist,
> + struct dlock_list_iter *iter);

Ugh...  Why not dlist_for_each_entry(), seeing that all users end up with
the same boilerplate?

Re: [PATCH v2 01/10] binfmt_flat: assorted cleanups

[Greg Ungerer]

Hi Nicolas,

On 18/07/16 13:31, Nicolas Pitre wrote:
> Remove excessive casts, do some code grouping, etc.
> No functional changes.
> 
> Signed-off-by: Nicolas Pitre 
> ---
>  fs/binfmt_flat.c | 118 
> ++-
>  1 file changed, 56 insertions(+), 62 deletions(-)
> 
> diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
> index caf9e39bb8..085059d879 100644
> --- a/fs/binfmt_flat.c
> +++ b/fs/binfmt_flat.c
> @@ -80,7 +80,7 @@ struct lib_info {
>   unsigned long text_len; /* Length of text 
> segment */
>   unsigned long entry;/* Start address for 
> this module */
>   unsigned long build_date;   /* When this one was 
> compiled */
> - short loaded;   /* Has this library 
> been loaded? */
> + bool loaded;/* Has this library 
> been loaded? */
>   } lib_list[MAX_SHARED_LIBS];
>  };
>  
> @@ -107,7 +107,7 @@ static struct linux_binfmt flat_format = {
>  static int flat_core_dump(struct coredump_params *cprm)
>  {
>   printk("Process %s:%d received signr %d and should have core dumped\n",
> - current->comm, current->pid, (int) 
> cprm->siginfo->si_signo);
> + current->comm, current->pid, cprm->siginfo->si_signo);
>   return(1);
>  }
>  
> @@ -190,7 +190,7 @@ static int decompress_exec(
>   loff_t fpos;
>   int ret, retval;
>  
> - DBG_FLT("decompress_exec(offset=%x,buf=%x,len=%x)\n",(int)offset, 
> (int)dst, (int)len);
> + DBG_FLT("decompress_exec(offset=%lx,buf=%p,len=%lx)\n",offset, dst, 
> len);
>  
>   memset(, 0, sizeof(strm));
>   strm.workspace = kmalloc(zlib_inflate_workspacesize(), GFP_KERNEL);
> @@ -358,8 +358,8 @@ calc_reloc(unsigned long r, struct lib_info *p, int 
> curid, int internalp)
>   text_len = p->lib_list[id].text_len;
>  
>   if (!flat_reloc_valid(r, start_brk - start_data + text_len)) {
> - printk("BINFMT_FLAT: reloc outside program 0x%x (0 - 
> 0x%x/0x%x)",
> -(int) 
> r,(int)(start_brk-start_data+text_len),(int)text_len);
> + printk("BINFMT_FLAT: reloc outside program 0x%lx (0 - 
> 0x%lx/0x%lx)",
> +r, start_brk-start_data+text_len, text_len);

Seeing as you have modified quite a few printk calls is it worth
while annotating them with appropriate KERN_ERR, KERN_INFO, etc?
Just a thought.

Regards
Greg

Re: [PATCH v3 1/4] lib/dlock-list: Distributed and lock-protected lists

[Al Viro]

On Fri, Jul 15, 2016 at 01:39:40PM -0400, Waiman Long wrote:

> +struct dlock_list_head_percpu {
> + struct list_head list;
> + spinlock_t lock;
> +};

> +#define DLOCK_LIST_HEAD_PERCPU_INIT(name)\
> + {   \
> + .list.prev = ,\
> + .list.next = ,\
> + .list.lock = __SPIN_LOCK_UNLOCKED(name),\

What's .list.lock and how does that even compile?

> +extern bool dlock_list_next(struct dlock_list_head *dlist,
> + struct dlock_list_iter *iter);

Ugh...  Why not dlist_for_each_entry(), seeing that all users end up with
the same boilerplate?

Re: [PATCH v2 01/10] binfmt_flat: assorted cleanups

[Greg Ungerer]

Hi Nicolas,

On 18/07/16 13:31, Nicolas Pitre wrote:
> Remove excessive casts, do some code grouping, etc.
> No functional changes.
> 
> Signed-off-by: Nicolas Pitre 
> ---
>  fs/binfmt_flat.c | 118 
> ++-
>  1 file changed, 56 insertions(+), 62 deletions(-)
> 
> diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
> index caf9e39bb8..085059d879 100644
> --- a/fs/binfmt_flat.c
> +++ b/fs/binfmt_flat.c
> @@ -80,7 +80,7 @@ struct lib_info {
>   unsigned long text_len; /* Length of text 
> segment */
>   unsigned long entry;/* Start address for 
> this module */
>   unsigned long build_date;   /* When this one was 
> compiled */
> - short loaded;   /* Has this library 
> been loaded? */
> + bool loaded;/* Has this library 
> been loaded? */
>   } lib_list[MAX_SHARED_LIBS];
>  };
>  
> @@ -107,7 +107,7 @@ static struct linux_binfmt flat_format = {
>  static int flat_core_dump(struct coredump_params *cprm)
>  {
>   printk("Process %s:%d received signr %d and should have core dumped\n",
> - current->comm, current->pid, (int) 
> cprm->siginfo->si_signo);
> + current->comm, current->pid, cprm->siginfo->si_signo);
>   return(1);
>  }
>  
> @@ -190,7 +190,7 @@ static int decompress_exec(
>   loff_t fpos;
>   int ret, retval;
>  
> - DBG_FLT("decompress_exec(offset=%x,buf=%x,len=%x)\n",(int)offset, 
> (int)dst, (int)len);
> + DBG_FLT("decompress_exec(offset=%lx,buf=%p,len=%lx)\n",offset, dst, 
> len);
>  
>   memset(, 0, sizeof(strm));
>   strm.workspace = kmalloc(zlib_inflate_workspacesize(), GFP_KERNEL);
> @@ -358,8 +358,8 @@ calc_reloc(unsigned long r, struct lib_info *p, int 
> curid, int internalp)
>   text_len = p->lib_list[id].text_len;
>  
>   if (!flat_reloc_valid(r, start_brk - start_data + text_len)) {
> - printk("BINFMT_FLAT: reloc outside program 0x%x (0 - 
> 0x%x/0x%x)",
> -(int) 
> r,(int)(start_brk-start_data+text_len),(int)text_len);
> + printk("BINFMT_FLAT: reloc outside program 0x%lx (0 - 
> 0x%lx/0x%lx)",
> +r, start_brk-start_data+text_len, text_len);

Seeing as you have modified quite a few printk calls is it worth
while annotating them with appropriate KERN_ERR, KERN_INFO, etc?
Just a thought.

Regards
Greg

Re: Kernel stability on baytrail machines

[Michal Feix]

Dne 13.7.2016 v 12:48 Pavel Machek napsal(a)


Are there any updates on the status of this issue?

The current bugzilla report [1] marks this as a power management
issue. However, many reports indicate that it would only freeze
when running X, so it's not completely clear if it's related to
the gfx driver too.

Does

"intel_idle.max_cstate=1"

fix it for you?

Yes, it does.

If you feel it is X-only problem, you may want to provide details
about your graphics subsystem (DRM enabled? framebuffer only?) and
probably cc.

It's not X-only problem. Happens even in console mode, which is KMS
switched during boot though.

...actually... you may want to verify if it happens in unaccelerated X.

As it happens even in console mode, is this relevant test?

No, no need to test with X.

Would it be possible to test in good old VGA mode?
For past few days I updated to 4.6.3 kernel and tested with X and 
without X, in console mode and no KMS. My machine is way more stable 
than with previous 4.5.* and 4.4.* kernels I tried.


With 4.6.3 kernel and X running, I had only one freeze during Firefox 
session with video playback. For the rest of two days of testing, no hang.


I was not able to hang the machine during another 2 days of testing with 
4.6.3 kernel and console mode with KMS disabled. It's fair to say, that 
stress testing of GFX is quite limited when running in console mode :-). 
I tried hard with mplayer with libcaca and with repeated kernel 
compilation task. No hang occured.


My conclussion is that 4.6.3 is surelly a huge improvement, compared to 
4.5 and 4.4. kernels regarding Bay trail stability issue.


Michal Feix

Re: Kernel stability on baytrail machines

[Michal Feix]

Dne 13.7.2016 v 12:48 Pavel Machek napsal(a)


Are there any updates on the status of this issue?

The current bugzilla report [1] marks this as a power management
issue. However, many reports indicate that it would only freeze
when running X, so it's not completely clear if it's related to
the gfx driver too.

Does

"intel_idle.max_cstate=1"

fix it for you?

Yes, it does.

If you feel it is X-only problem, you may want to provide details
about your graphics subsystem (DRM enabled? framebuffer only?) and
probably cc.

It's not X-only problem. Happens even in console mode, which is KMS
switched during boot though.

...actually... you may want to verify if it happens in unaccelerated X.

As it happens even in console mode, is this relevant test?

No, no need to test with X.

Would it be possible to test in good old VGA mode?
For past few days I updated to 4.6.3 kernel and tested with X and 
without X, in console mode and no KMS. My machine is way more stable 
than with previous 4.5.* and 4.4.* kernels I tried.


With 4.6.3 kernel and X running, I had only one freeze during Firefox 
session with video playback. For the rest of two days of testing, no hang.


I was not able to hang the machine during another 2 days of testing with 
4.6.3 kernel and console mode with KMS disabled. It's fair to say, that 
stress testing of GFX is quite limited when running in console mode :-). 
I tried hard with mplayer with libcaca and with repeated kernel 
compilation task. No hang occured.


My conclussion is that 4.6.3 is surelly a huge improvement, compared to 
4.5 and 4.4. kernels regarding Bay trail stability issue.


Michal Feix

Re: [PATCH v3 12/17] mm, compaction: more reliably increase direct compaction priority

[Joonsoo Kim]

On Mon, Jul 18, 2016 at 02:21:02PM +0200, Vlastimil Babka wrote:
> On 07/18/2016 06:41 AM, Joonsoo Kim wrote:
> >On Fri, Jul 15, 2016 at 03:37:52PM +0200, Vlastimil Babka wrote:
> >>On 07/06/2016 07:39 AM, Joonsoo Kim wrote:
> >>>On Fri, Jun 24, 2016 at 11:54:32AM +0200, Vlastimil Babka wrote:
> During reclaim/compaction loop, compaction priority can be increased by 
> the
> should_compact_retry() function, but the current code is not optimal. 
> Priority
> is only increased when compaction_failed() is true, which means that 
> compaction
> has scanned the whole zone. This may not happen even after multiple 
> attempts
> with the lower priority due to parallel activity, so we might needlessly
> struggle on the lower priority and possibly run out of compaction retry
> attempts in the process.
> 
> We can remove these corner cases by increasing compaction priority 
> regardless
> of compaction_failed(). Examining further the compaction result can be
> postponed only after reaching the highest priority. This is a simple 
> solution
> and we don't need to worry about reaching the highest priority "too soon" 
> here,
> because hen should_compact_retry() is called it means that the system is
> already struggling and the allocation is supposed to either try as hard as
> possible, or it cannot fail at all. There's not much point staying at 
> lower
> priorities with heuristics that may result in only partial compaction.
> Also we now count compaction retries only after reaching the highest 
> priority.
> >>>
> >>>I'm not sure that this patch is safe. Deferring and skip-bit in
> >>>compaction is highly related to reclaim/compaction. Just ignoring them and 
> >>>(almost)
> >>>unconditionally increasing compaction priority will result in less
> >>>reclaim and less success rate on compaction.
> >>
> >>I don't see why less reclaim? Reclaim is always attempted before
> >>compaction and compaction priority doesn't affect it. And as long as
> >>reclaim wants to retry, should_compact_retry() isn't even called, so the
> >>priority stays. I wanted to change that in v1, but Michal suggested I
> >>shouldn't.
> >
> >I assume the situation that there is no !costly highorder freepage
> >because of fragmentation. In this case, should_reclaim_retry() would
> >return false since watermark cannot be met due to absence of high
> >order freepage. Now, please see should_compact_retry() with assumption
> >that there are enough order-0 free pages. Reclaim/compaction is only
> >retried two times (SYNC_LIGHT and SYNC_FULL) with your patchset since
> >compaction_withdrawn() return false with enough freepages and
> >!COMPACT_SKIPPED.
> >
> >But, before your patchset, COMPACT_PARTIAL_SKIPPED and
> >COMPACT_DEFERRED is considered as withdrawn so will retry
> >reclaim/compaction more times.
> 
> Perhaps, but it wouldn't guarantee to reach the highest priority.

Yes.

> 
> >As I said before, more reclaim (more freepage) increase migration
> >scanner's scan range and then increase compaction success probability.
> >Therefore, your patchset which makes reclaim/compaction retry less times
> >deterministically would not be safe.
> 
> After the patchset, we are guaranteed a full compaction has
> happened. If that doesn't help, yeah maybe we can try reclaiming
> more... but where to draw the line? Reclaim everything for an

To draw the line is a difficult problem. I know that. As I said before,
one of ideas is that reclaim/compaction continue until nr_reclaimed
reaches number of lru pages at beginning phase of reclaim/compaction
loop. It would not cause persistent thrashing, I guess.

> order-3 allocation just to avoid OOM, ignoring that the system might
> be thrashing heavily? Previously it also wasn't guaranteed to
> reclaim everything, but what is the optimal number of retries?

So, you say the similar logic in other thread we talked yesterday.
The fact that it wasn't guaranteed to reclaim every thing before
doesn't mean that we could relax guarantee more.

I'm not sure below is relevant to this series but just note.

I don't know the optimal number of retries. We are in a way to find
it and I hope this discussion would help. I don't think that we can
judge the point properly with simple checking on stat information at some
moment. It only has too limited knowledge about the system so it would
wrongly advise us to invoke OOM prematurely.

I think that using compaction result isn't a good way to determine if
further reclaim/compaction is useless or not because compaction result
can vary with further reclaim/compaction itself.

If we want to check more accurately if compaction is really impossible,
scanning whole range and checking arrangement of freepage and lru(movable)
pages would more help. Although there is some possibility to fail
the compaction even if this check is passed, it would give us more
information about the system state and we 

Re: [PATCH v3 12/17] mm, compaction: more reliably increase direct compaction priority

[Joonsoo Kim]

On Mon, Jul 18, 2016 at 02:21:02PM +0200, Vlastimil Babka wrote:
> On 07/18/2016 06:41 AM, Joonsoo Kim wrote:
> >On Fri, Jul 15, 2016 at 03:37:52PM +0200, Vlastimil Babka wrote:
> >>On 07/06/2016 07:39 AM, Joonsoo Kim wrote:
> >>>On Fri, Jun 24, 2016 at 11:54:32AM +0200, Vlastimil Babka wrote:
> During reclaim/compaction loop, compaction priority can be increased by 
> the
> should_compact_retry() function, but the current code is not optimal. 
> Priority
> is only increased when compaction_failed() is true, which means that 
> compaction
> has scanned the whole zone. This may not happen even after multiple 
> attempts
> with the lower priority due to parallel activity, so we might needlessly
> struggle on the lower priority and possibly run out of compaction retry
> attempts in the process.
> 
> We can remove these corner cases by increasing compaction priority 
> regardless
> of compaction_failed(). Examining further the compaction result can be
> postponed only after reaching the highest priority. This is a simple 
> solution
> and we don't need to worry about reaching the highest priority "too soon" 
> here,
> because hen should_compact_retry() is called it means that the system is
> already struggling and the allocation is supposed to either try as hard as
> possible, or it cannot fail at all. There's not much point staying at 
> lower
> priorities with heuristics that may result in only partial compaction.
> Also we now count compaction retries only after reaching the highest 
> priority.
> >>>
> >>>I'm not sure that this patch is safe. Deferring and skip-bit in
> >>>compaction is highly related to reclaim/compaction. Just ignoring them and 
> >>>(almost)
> >>>unconditionally increasing compaction priority will result in less
> >>>reclaim and less success rate on compaction.
> >>
> >>I don't see why less reclaim? Reclaim is always attempted before
> >>compaction and compaction priority doesn't affect it. And as long as
> >>reclaim wants to retry, should_compact_retry() isn't even called, so the
> >>priority stays. I wanted to change that in v1, but Michal suggested I
> >>shouldn't.
> >
> >I assume the situation that there is no !costly highorder freepage
> >because of fragmentation. In this case, should_reclaim_retry() would
> >return false since watermark cannot be met due to absence of high
> >order freepage. Now, please see should_compact_retry() with assumption
> >that there are enough order-0 free pages. Reclaim/compaction is only
> >retried two times (SYNC_LIGHT and SYNC_FULL) with your patchset since
> >compaction_withdrawn() return false with enough freepages and
> >!COMPACT_SKIPPED.
> >
> >But, before your patchset, COMPACT_PARTIAL_SKIPPED and
> >COMPACT_DEFERRED is considered as withdrawn so will retry
> >reclaim/compaction more times.
> 
> Perhaps, but it wouldn't guarantee to reach the highest priority.

Yes.

> 
> >As I said before, more reclaim (more freepage) increase migration
> >scanner's scan range and then increase compaction success probability.
> >Therefore, your patchset which makes reclaim/compaction retry less times
> >deterministically would not be safe.
> 
> After the patchset, we are guaranteed a full compaction has
> happened. If that doesn't help, yeah maybe we can try reclaiming
> more... but where to draw the line? Reclaim everything for an

To draw the line is a difficult problem. I know that. As I said before,
one of ideas is that reclaim/compaction continue until nr_reclaimed
reaches number of lru pages at beginning phase of reclaim/compaction
loop. It would not cause persistent thrashing, I guess.

> order-3 allocation just to avoid OOM, ignoring that the system might
> be thrashing heavily? Previously it also wasn't guaranteed to
> reclaim everything, but what is the optimal number of retries?

So, you say the similar logic in other thread we talked yesterday.
The fact that it wasn't guaranteed to reclaim every thing before
doesn't mean that we could relax guarantee more.

I'm not sure below is relevant to this series but just note.

I don't know the optimal number of retries. We are in a way to find
it and I hope this discussion would help. I don't think that we can
judge the point properly with simple checking on stat information at some
moment. It only has too limited knowledge about the system so it would
wrongly advise us to invoke OOM prematurely.

I think that using compaction result isn't a good way to determine if
further reclaim/compaction is useless or not because compaction result
can vary with further reclaim/compaction itself.

If we want to check more accurately if compaction is really impossible,
scanning whole range and checking arrangement of freepage and lru(movable)
pages would more help. Although there is some possibility to fail
the compaction even if this check is passed, it would give us more
information about the system state and we 

RE: [PATCH v3 1/2] ACPI / button: Add KEY_LID_CLOSE for new usage model

[Zheng, Lv]

Hi, Benjamin

> From: Benjamin Tissoires [mailto:benjamin.tissoi...@gmail.com]
> Subject: Re: [PATCH v3 1/2] ACPI / button: Add KEY_LID_CLOSE for new
> usage model
> 
> Hi,
> 
> On Tue, Jul 12, 2016 at 12:17 PM, Lv Zheng  wrote:
> > There are many AML tables reporting wrong initial lid state, and some of
> > them never reports lid open state. As a proxy layer acting between, ACPI
> > button driver is not able to handle all such cases, but need to re-define
> > the usage model of the ACPI lid. That is:
> > 1. It's initial state is not reliable;
> > 2. There may not be an open event;
> > 3. Userspace should only take action against the close event which is
> >reliable, always sent after a real lid close.
> > This patch adds a new input key event so that the new userspace
> programs
> > can use it to handle this usage model correctly. And in the meanwhile, no
> > old programs will be broken by the userspace changes.
> > This patch also adds a button.lid_event_type parameter to allow the
> users
> > to switch between the 2 event types.
> 
> I think we are getting closer to what would be acceptable for
> user-space, but I still have a few comments:
[Lv Zheng] 
OK.

> 
> >
> > Signed-off-by: Lv Zheng 
> > Cc: Dmitry Torokhov 
> > Cc: Benjamin Tissoires 
> > Cc: Bastien Nocera: 
> > Cc: linux-in...@vger.kernel.org
> > ---
> >  drivers/acpi/button.c  |  109 +--
> -
> >  include/uapi/linux/input-event-codes.h |6 ++
> >  2 files changed, 91 insertions(+), 24 deletions(-)
> >
> > diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
> > index 148f4e5..1298ef8 100644
> > --- a/drivers/acpi/button.c
> > +++ b/drivers/acpi/button.c
> > @@ -57,6 +57,9 @@
> >  #define ACPI_BUTTON_LID_INIT_OPEN  0x01
> >  #define ACPI_BUTTON_LID_INIT_METHOD0x02
> >
> > +#define ACPI_BUTTON_LID_EVENT_KEY  0x00
> > +#define ACPI_BUTTON_LID_EVENT_SWITCH   0x01
> > +
> >  #define _COMPONENT ACPI_BUTTON_COMPONENT
> >  ACPI_MODULE_NAME("button");
> >
> > @@ -110,6 +113,7 @@ struct acpi_button {
> >  static BLOCKING_NOTIFIER_HEAD(acpi_lid_notifier);
> >  static struct acpi_device *lid_device;
> >  static u8 lid_init_state = ACPI_BUTTON_LID_INIT_METHOD;
> > +static u8 lid_event_type = ACPI_BUTTON_LID_EVENT_SWITCH;
> >
> >  /* 
> > --
> >FS Interface (/proc)
> > @@ -136,8 +140,17 @@ static int acpi_lid_notify_state(struct
> acpi_device *device, int state)
> > int ret;
> >
> > /* input layer checks if event is redundant */
> > -   input_report_switch(button->input, SW_LID, !state);
> > -   input_sync(button->input);
> > +   if (lid_event_type == ACPI_BUTTON_LID_EVENT_SWITCH) {
> > +   input_report_switch(button->input, SW_LID, !state);
> > +   input_sync(button->input);
> > +   }
> > +   if (lid_event_type == ACPI_BUTTON_LID_EVENT_KEY &&
> > +  !state) {
> > +   input_report_key(button->input, KEY_LID_CLOSE, 1);
> > +   input_sync(button->input);
> > +   input_report_key(button->input, KEY_LID_CLOSE, 0);
> > +   input_sync(button->input);
> > +   }
> >
> > if (state)
> > pm_wakeup_event(>dev, 0);
> > @@ -292,6 +305,9 @@ static int acpi_lid_update_state(struct
> acpi_device *device)
> >
> >  static void acpi_lid_initialize_state(struct acpi_device *device)
> >  {
> > +   if (lid_event_type == ACPI_BUTTON_LID_EVENT_KEY)
> > +   return;
> > +
> > switch (lid_init_state) {
> > case ACPI_BUTTON_LID_INIT_OPEN:
> > (void)acpi_lid_notify_state(device, 1);
> > @@ -436,6 +452,7 @@ static int acpi_button_add(struct acpi_device
> *device)
> >
> > case ACPI_BUTTON_TYPE_LID:
> > input_set_capability(input, EV_SW, SW_LID);
> > +   input_set_capability(input, EV_KEY, KEY_LID_CLOSE);
> 
> Here you are basically exporting the 2 input events but only update
> one of the 2. It will confuse userspace and it is generally better not
> to export unused input codes.
[Lv Zheng] 
I just do not know if it is proper to clear the capability during runtime via 
module parameters.


> 
> However, as I'll say below, I think we should keep the code that way here.
[Lv Zheng] 
Great. So we needn't think about input_clear_capability().

> 
> 
> > break;
> > }
> >
> > @@ -475,35 +492,49 @@ static int acpi_button_remove(struct
> acpi_device *device)
> >
> >  static int param_set_lid_init_state(const char *val, struct kernel_param
> *kp)
> >  {
> > -   int result = 0;
> > -
> > -   if (!strncmp(val, "open", sizeof("open") - 1)) {
> > -   lid_init_state = ACPI_BUTTON_LID_INIT_OPEN;
> > -   pr_info("Notify initial 

Re: [PATCH 3/3] f2fs: support clone_file_range

[Jaegeuk Kim]

On Mon, Jul 18, 2016 at 08:47:36PM -0700, Christoph Hellwig wrote:
> On Sat, Jul 16, 2016 at 11:24:27PM -0700, Jaegeuk Kim wrote:
> > This patch implements clone_file_range in f2fs.
> 
> [...]
> 
> > +int f2fs_clone_file_range(struct file *file_in, loff_t pos_in,
> > +   struct file *file_out, loff_t pos_out, u64 len)
> > +{
> > +   return f2fs_clone_files(file_in, pos_in, file_out, pos_out, len);
> > +}
> > +
> 
> Falling back from copy to clone should be done in the VFS.  I look into
> implementing the fallback, and the code is trivial, but we don't seem
> to have any useful coverage for copy_file_range yet, so I didn't dare
> to add it yet.  How did you test copy and clone in f2fs?  And how did
> you handle the difference in corner cases (e.g. the lacking 0 special
> case in copy)?

Frankly speaking, I confused the behaviors without being aware of corner cases.
And thus, I dropped the patches already. Instead, I realized that what I did
was moving a range of blocks from one file to another file. So, I've been
testing a new ioctl, F2FS_IOC_MOVE_RANGE.
In terms of coverage, I just did some unit tests only.

Thanks,

RE: [PATCH v3 1/2] ACPI / button: Add KEY_LID_CLOSE for new usage model

[Zheng, Lv]

Hi, Benjamin

> From: Benjamin Tissoires [mailto:benjamin.tissoi...@gmail.com]
> Subject: Re: [PATCH v3 1/2] ACPI / button: Add KEY_LID_CLOSE for new
> usage model
> 
> Hi,
> 
> On Tue, Jul 12, 2016 at 12:17 PM, Lv Zheng  wrote:
> > There are many AML tables reporting wrong initial lid state, and some of
> > them never reports lid open state. As a proxy layer acting between, ACPI
> > button driver is not able to handle all such cases, but need to re-define
> > the usage model of the ACPI lid. That is:
> > 1. It's initial state is not reliable;
> > 2. There may not be an open event;
> > 3. Userspace should only take action against the close event which is
> >reliable, always sent after a real lid close.
> > This patch adds a new input key event so that the new userspace
> programs
> > can use it to handle this usage model correctly. And in the meanwhile, no
> > old programs will be broken by the userspace changes.
> > This patch also adds a button.lid_event_type parameter to allow the
> users
> > to switch between the 2 event types.
> 
> I think we are getting closer to what would be acceptable for
> user-space, but I still have a few comments:
[Lv Zheng] 
OK.

> 
> >
> > Signed-off-by: Lv Zheng 
> > Cc: Dmitry Torokhov 
> > Cc: Benjamin Tissoires 
> > Cc: Bastien Nocera: 
> > Cc: linux-in...@vger.kernel.org
> > ---
> >  drivers/acpi/button.c  |  109 +--
> -
> >  include/uapi/linux/input-event-codes.h |6 ++
> >  2 files changed, 91 insertions(+), 24 deletions(-)
> >
> > diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
> > index 148f4e5..1298ef8 100644
> > --- a/drivers/acpi/button.c
> > +++ b/drivers/acpi/button.c
> > @@ -57,6 +57,9 @@
> >  #define ACPI_BUTTON_LID_INIT_OPEN  0x01
> >  #define ACPI_BUTTON_LID_INIT_METHOD0x02
> >
> > +#define ACPI_BUTTON_LID_EVENT_KEY  0x00
> > +#define ACPI_BUTTON_LID_EVENT_SWITCH   0x01
> > +
> >  #define _COMPONENT ACPI_BUTTON_COMPONENT
> >  ACPI_MODULE_NAME("button");
> >
> > @@ -110,6 +113,7 @@ struct acpi_button {
> >  static BLOCKING_NOTIFIER_HEAD(acpi_lid_notifier);
> >  static struct acpi_device *lid_device;
> >  static u8 lid_init_state = ACPI_BUTTON_LID_INIT_METHOD;
> > +static u8 lid_event_type = ACPI_BUTTON_LID_EVENT_SWITCH;
> >
> >  /* 
> > --
> >FS Interface (/proc)
> > @@ -136,8 +140,17 @@ static int acpi_lid_notify_state(struct
> acpi_device *device, int state)
> > int ret;
> >
> > /* input layer checks if event is redundant */
> > -   input_report_switch(button->input, SW_LID, !state);
> > -   input_sync(button->input);
> > +   if (lid_event_type == ACPI_BUTTON_LID_EVENT_SWITCH) {
> > +   input_report_switch(button->input, SW_LID, !state);
> > +   input_sync(button->input);
> > +   }
> > +   if (lid_event_type == ACPI_BUTTON_LID_EVENT_KEY &&
> > +  !state) {
> > +   input_report_key(button->input, KEY_LID_CLOSE, 1);
> > +   input_sync(button->input);
> > +   input_report_key(button->input, KEY_LID_CLOSE, 0);
> > +   input_sync(button->input);
> > +   }
> >
> > if (state)
> > pm_wakeup_event(>dev, 0);
> > @@ -292,6 +305,9 @@ static int acpi_lid_update_state(struct
> acpi_device *device)
> >
> >  static void acpi_lid_initialize_state(struct acpi_device *device)
> >  {
> > +   if (lid_event_type == ACPI_BUTTON_LID_EVENT_KEY)
> > +   return;
> > +
> > switch (lid_init_state) {
> > case ACPI_BUTTON_LID_INIT_OPEN:
> > (void)acpi_lid_notify_state(device, 1);
> > @@ -436,6 +452,7 @@ static int acpi_button_add(struct acpi_device
> *device)
> >
> > case ACPI_BUTTON_TYPE_LID:
> > input_set_capability(input, EV_SW, SW_LID);
> > +   input_set_capability(input, EV_KEY, KEY_LID_CLOSE);
> 
> Here you are basically exporting the 2 input events but only update
> one of the 2. It will confuse userspace and it is generally better not
> to export unused input codes.
[Lv Zheng] 
I just do not know if it is proper to clear the capability during runtime via 
module parameters.


> 
> However, as I'll say below, I think we should keep the code that way here.
[Lv Zheng] 
Great. So we needn't think about input_clear_capability().

> 
> 
> > break;
> > }
> >
> > @@ -475,35 +492,49 @@ static int acpi_button_remove(struct
> acpi_device *device)
> >
> >  static int param_set_lid_init_state(const char *val, struct kernel_param
> *kp)
> >  {
> > -   int result = 0;
> > -
> > -   if (!strncmp(val, "open", sizeof("open") - 1)) {
> > -   lid_init_state = ACPI_BUTTON_LID_INIT_OPEN;
> > -   pr_info("Notify initial lid state as open\n");
> > -   } else if (!strncmp(val, "method", sizeof("method") - 1)) {
> > -   

Re: [PATCH 3/3] f2fs: support clone_file_range

[Jaegeuk Kim]

On Mon, Jul 18, 2016 at 08:47:36PM -0700, Christoph Hellwig wrote:
> On Sat, Jul 16, 2016 at 11:24:27PM -0700, Jaegeuk Kim wrote:
> > This patch implements clone_file_range in f2fs.
> 
> [...]
> 
> > +int f2fs_clone_file_range(struct file *file_in, loff_t pos_in,
> > +   struct file *file_out, loff_t pos_out, u64 len)
> > +{
> > +   return f2fs_clone_files(file_in, pos_in, file_out, pos_out, len);
> > +}
> > +
> 
> Falling back from copy to clone should be done in the VFS.  I look into
> implementing the fallback, and the code is trivial, but we don't seem
> to have any useful coverage for copy_file_range yet, so I didn't dare
> to add it yet.  How did you test copy and clone in f2fs?  And how did
> you handle the difference in corner cases (e.g. the lacking 0 special
> case in copy)?

Frankly speaking, I confused the behaviors without being aware of corner cases.
And thus, I dropped the patches already. Instead, I realized that what I did
was moving a range of blocks from one file to another file. So, I've been
testing a new ioctl, F2FS_IOC_MOVE_RANGE.
In terms of coverage, I just did some unit tests only.

Thanks,

[PATCH][v2] timekeeping: Fix memory overwrite of sleep_time_bin array

[Chen Yu]

It is reported the hibernation fails at 2nd attempt, which
hangs at hibernate() -> syscore_resume() -> i8237A_resume()
-> claim_dma_lock(), because the lock has already been taken.
However there is actually no other process would like to grab
this lock on that problematic platform.

Further investigation shows that, the problem is caused by setting
/sys/power/pm_trace to 1 before the 1st hibernation, since once
pm_trace is enabled, the rtc becomes an unmeaningful value after resumed,
which might bring a significant long sleep time in timekeeping_resume,
thus in tk_debug_account_sleep_time, if the bit31 happened to be set to 1,
the fls might return 32 and then we add 1 to sleep_time_bin[32], which
caused a memory overwritten. As System.map shows:

81c9d080 b sleep_time_bin
81c9d100 B dma_spin_lock

Thus set the dma_spin_lock.val to 1, which caused this problem.

To fix this problem, we ignore those abnormal sleep time,
since no one would like to sleep that long.

Cc: Stable  # 3.17+
Suggested-by: Rafael J. Wysocki 
Reported-and-tested-by: Janek Kozicki 
Signed-off-by: Chen Yu 
---
 kernel/time/timekeeping_debug.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/kernel/time/timekeeping_debug.c b/kernel/time/timekeeping_debug.c
index f6bd652..b164bb9 100644
--- a/kernel/time/timekeeping_debug.c
+++ b/kernel/time/timekeeping_debug.c
@@ -24,6 +24,7 @@
 #include "timekeeping_internal.h"
 
 static unsigned int sleep_time_bin[32] = {0};
+#define MAX_SLEEP_TIME 0x7fff
 
 static int tk_debug_show_sleep_time(struct seq_file *s, void *data)
 {
@@ -69,6 +70,7 @@ late_initcall(tk_debug_sleep_time_init);
 
 void tk_debug_account_sleep_time(struct timespec64 *t)
 {
-   sleep_time_bin[fls(t->tv_sec)]++;
+   if ((t->tv_sec >= 0) && (t->tv_sec <= MAX_SLEEP_TIME))
+   sleep_time_bin[fls(t->tv_sec)]++;
 }
 
-- 
2.7.4

[PATCH][v2] timekeeping: Fix memory overwrite of sleep_time_bin array

[Chen Yu]

It is reported the hibernation fails at 2nd attempt, which
hangs at hibernate() -> syscore_resume() -> i8237A_resume()
-> claim_dma_lock(), because the lock has already been taken.
However there is actually no other process would like to grab
this lock on that problematic platform.

Further investigation shows that, the problem is caused by setting
/sys/power/pm_trace to 1 before the 1st hibernation, since once
pm_trace is enabled, the rtc becomes an unmeaningful value after resumed,
which might bring a significant long sleep time in timekeeping_resume,
thus in tk_debug_account_sleep_time, if the bit31 happened to be set to 1,
the fls might return 32 and then we add 1 to sleep_time_bin[32], which
caused a memory overwritten. As System.map shows:

81c9d080 b sleep_time_bin
81c9d100 B dma_spin_lock

Thus set the dma_spin_lock.val to 1, which caused this problem.

To fix this problem, we ignore those abnormal sleep time,
since no one would like to sleep that long.

Cc: Stable  # 3.17+
Suggested-by: Rafael J. Wysocki 
Reported-and-tested-by: Janek Kozicki 
Signed-off-by: Chen Yu 
---
 kernel/time/timekeeping_debug.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/kernel/time/timekeeping_debug.c b/kernel/time/timekeeping_debug.c
index f6bd652..b164bb9 100644
--- a/kernel/time/timekeeping_debug.c
+++ b/kernel/time/timekeeping_debug.c
@@ -24,6 +24,7 @@
 #include "timekeeping_internal.h"
 
 static unsigned int sleep_time_bin[32] = {0};
+#define MAX_SLEEP_TIME 0x7fff
 
 static int tk_debug_show_sleep_time(struct seq_file *s, void *data)
 {
@@ -69,6 +70,7 @@ late_initcall(tk_debug_sleep_time_init);
 
 void tk_debug_account_sleep_time(struct timespec64 *t)
 {
-   sleep_time_bin[fls(t->tv_sec)]++;
+   if ((t->tv_sec >= 0) && (t->tv_sec <= MAX_SLEEP_TIME))
+   sleep_time_bin[fls(t->tv_sec)]++;
 }
 
-- 
2.7.4

RE: [RFC PATCH 3/3] ACPI / PM: Fix EC polling issue by implementing prepare_late/finish_early suspend_ops callbacks

[Zheng, Lv]

Hi, Rafael

> From: linux-acpi-ow...@vger.kernel.org [mailto:linux-acpi-
> ow...@vger.kernel.org] On Behalf Of Rafael J. Wysocki
> Subject: Re: [RFC PATCH 3/3] ACPI / PM: Fix EC polling issue by
> implementing prepare_late/finish_early suspend_ops callbacks
> 
> On Tuesday, June 28, 2016 04:04:46 PM Lv Zheng wrote:
> > _PTS/_WAK may contain EC transactions, it is better to have them
> handled
> > with IRQ enabled. This patch moves the 2 suspend PM ops from noirq
> stage
> > to late/early stage.
> >
> > Signed-off-by: Lv Zheng 
> 
> There are systems that won't work with this patch applied, so I don't
> see a point in applying the other two.
[Lv Zheng] 
I see.
Then instead of moving _WAK/_PTS out of noirq stage, we need to think about 
tuning the performance of the EC polling mode.

Thanks and best regards
-Lv

> 
> > ---
> >  drivers/acpi/sleep.c |5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
> > index d30fce7f..c5c374c9 100644
> > --- a/drivers/acpi/sleep.c
> > +++ b/drivers/acpi/sleep.c
> > @@ -595,9 +595,10 @@ static int
> acpi_suspend_state_valid(suspend_state_t pm_state)
> >  static const struct platform_suspend_ops acpi_suspend_ops = {
> > .valid = acpi_suspend_state_valid,
> > .begin = acpi_suspend_begin,
> > -   .prepare_noirq = acpi_pm_prepare,
> > +   .prepare_late = __acpi_pm_prepare,
> > +   .prepare_noirq = acpi_pm_pre_suspend,
> > .enter = acpi_suspend_enter,
> > -   .finish_noirq = acpi_pm_finish,
> > +   .finish_early = acpi_pm_finish,
> > .end = acpi_pm_end,
> >  };
> >
> >
> 
> Thanks,
> Rafael
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [RFC PATCH 3/3] ACPI / PM: Fix EC polling issue by implementing prepare_late/finish_early suspend_ops callbacks

[Zheng, Lv]

Hi, Rafael

> From: linux-acpi-ow...@vger.kernel.org [mailto:linux-acpi-
> ow...@vger.kernel.org] On Behalf Of Rafael J. Wysocki
> Subject: Re: [RFC PATCH 3/3] ACPI / PM: Fix EC polling issue by
> implementing prepare_late/finish_early suspend_ops callbacks
> 
> On Tuesday, June 28, 2016 04:04:46 PM Lv Zheng wrote:
> > _PTS/_WAK may contain EC transactions, it is better to have them
> handled
> > with IRQ enabled. This patch moves the 2 suspend PM ops from noirq
> stage
> > to late/early stage.
> >
> > Signed-off-by: Lv Zheng 
> 
> There are systems that won't work with this patch applied, so I don't
> see a point in applying the other two.
[Lv Zheng] 
I see.
Then instead of moving _WAK/_PTS out of noirq stage, we need to think about 
tuning the performance of the EC polling mode.

Thanks and best regards
-Lv

> 
> > ---
> >  drivers/acpi/sleep.c |5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
> > index d30fce7f..c5c374c9 100644
> > --- a/drivers/acpi/sleep.c
> > +++ b/drivers/acpi/sleep.c
> > @@ -595,9 +595,10 @@ static int
> acpi_suspend_state_valid(suspend_state_t pm_state)
> >  static const struct platform_suspend_ops acpi_suspend_ops = {
> > .valid = acpi_suspend_state_valid,
> > .begin = acpi_suspend_begin,
> > -   .prepare_noirq = acpi_pm_prepare,
> > +   .prepare_late = __acpi_pm_prepare,
> > +   .prepare_noirq = acpi_pm_pre_suspend,
> > .enter = acpi_suspend_enter,
> > -   .finish_noirq = acpi_pm_finish,
> > +   .finish_early = acpi_pm_finish,
> > .end = acpi_pm_end,
> >  };
> >
> >
> 
> Thanks,
> Rafael
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3] virtio_blk: Fix a slient kernel panic

[Minfei Huang]

From: Minfei Huang 

We do a lot of memory allocation in function init_vq, and don't handle
the allocation failure properly. Then this function will return 0,
although initialization fails due to lacking memory. At that moment,
kernel will panic in guest machine, if virtio is used to drive disk.

To fix this bug, we should take care of allocation failure, and return
correct value to let caller know what happen.

Tested-by: Chao Fan 
Signed-off-by: Minfei Huang 
Signed-off-by: Minfei Huang 
---
v2:
- Remove useless initialisation to NULL
v1:
- Refactor the patch to make code more readable
---
 drivers/block/virtio_blk.c | 26 --
 1 file changed, 8 insertions(+), 18 deletions(-)

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 42758b5..4ee78c0 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -394,22 +394,16 @@ static int init_vq(struct virtio_blk *vblk)
num_vqs = 1;
 
vblk->vqs = kmalloc(sizeof(*vblk->vqs) * num_vqs, GFP_KERNEL);
-   if (!vblk->vqs) {
-   err = -ENOMEM;
-   goto out;
-   }
+   if (!vblk->vqs)
+   return -ENOMEM;
 
names = kmalloc(sizeof(*names) * num_vqs, GFP_KERNEL);
-   if (!names)
-   goto err_names;
-
callbacks = kmalloc(sizeof(*callbacks) * num_vqs, GFP_KERNEL);
-   if (!callbacks)
-   goto err_callbacks;
-
vqs = kmalloc(sizeof(*vqs) * num_vqs, GFP_KERNEL);
-   if (!vqs)
-   goto err_vqs;
+   if (!names || !callbacks || !vqs) {
+   err = -ENOMEM;
+   goto out;
+   }
 
for (i = 0; i < num_vqs; i++) {
callbacks[i] = virtblk_done;
@@ -420,7 +414,7 @@ static int init_vq(struct virtio_blk *vblk)
/* Discover virtqueues and write information to configuration.  */
err = vdev->config->find_vqs(vdev, num_vqs, vqs, callbacks, names);
if (err)
-   goto err_find_vqs;
+   goto out;
 
for (i = 0; i < num_vqs; i++) {
spin_lock_init(>vqs[i].lock);
@@ -428,16 +422,12 @@ static int init_vq(struct virtio_blk *vblk)
}
vblk->num_vqs = num_vqs;
 
- err_find_vqs:
+out:
kfree(vqs);
- err_vqs:
kfree(callbacks);
- err_callbacks:
kfree(names);
- err_names:
if (err)
kfree(vblk->vqs);
- out:
return err;
 }
 
-- 
2.7.4 (Apple Git-66)

[PATCH v3] virtio_blk: Fix a slient kernel panic

[Minfei Huang]

From: Minfei Huang 

We do a lot of memory allocation in function init_vq, and don't handle
the allocation failure properly. Then this function will return 0,
although initialization fails due to lacking memory. At that moment,
kernel will panic in guest machine, if virtio is used to drive disk.

To fix this bug, we should take care of allocation failure, and return
correct value to let caller know what happen.

Tested-by: Chao Fan 
Signed-off-by: Minfei Huang 
Signed-off-by: Minfei Huang 
---
v2:
- Remove useless initialisation to NULL
v1:
- Refactor the patch to make code more readable
---
 drivers/block/virtio_blk.c | 26 --
 1 file changed, 8 insertions(+), 18 deletions(-)

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 42758b5..4ee78c0 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -394,22 +394,16 @@ static int init_vq(struct virtio_blk *vblk)
num_vqs = 1;
 
vblk->vqs = kmalloc(sizeof(*vblk->vqs) * num_vqs, GFP_KERNEL);
-   if (!vblk->vqs) {
-   err = -ENOMEM;
-   goto out;
-   }
+   if (!vblk->vqs)
+   return -ENOMEM;
 
names = kmalloc(sizeof(*names) * num_vqs, GFP_KERNEL);
-   if (!names)
-   goto err_names;
-
callbacks = kmalloc(sizeof(*callbacks) * num_vqs, GFP_KERNEL);
-   if (!callbacks)
-   goto err_callbacks;
-
vqs = kmalloc(sizeof(*vqs) * num_vqs, GFP_KERNEL);
-   if (!vqs)
-   goto err_vqs;
+   if (!names || !callbacks || !vqs) {
+   err = -ENOMEM;
+   goto out;
+   }
 
for (i = 0; i < num_vqs; i++) {
callbacks[i] = virtblk_done;
@@ -420,7 +414,7 @@ static int init_vq(struct virtio_blk *vblk)
/* Discover virtqueues and write information to configuration.  */
err = vdev->config->find_vqs(vdev, num_vqs, vqs, callbacks, names);
if (err)
-   goto err_find_vqs;
+   goto out;
 
for (i = 0; i < num_vqs; i++) {
spin_lock_init(>vqs[i].lock);
@@ -428,16 +422,12 @@ static int init_vq(struct virtio_blk *vblk)
}
vblk->num_vqs = num_vqs;
 
- err_find_vqs:
+out:
kfree(vqs);
- err_vqs:
kfree(callbacks);
- err_callbacks:
kfree(names);
- err_names:
if (err)
kfree(vblk->vqs);
- out:
return err;
 }
 
-- 
2.7.4 (Apple Git-66)

Re: [PATCH v1] ESIA : Dummy eisa_driver_register should return error code

[Christoph Hellwig]

On Tue, Jul 19, 2016 at 12:15:01AM +0530, Arvind Yadav wrote:
> The inline eisa_driver_register stub simply allows compilation on
> systems with CONFIG_EISA disabled. the dummy eisa_driver_register
> does not register an *_eisa_driver at all. The inline
> eisa_driver_register should return to indicate lack of support
> when attempting to register an *_eisa_driver on such a system with
> CONFIG_EISA disabled.

Why?  The idea is that you can simply leave the stub in if registering
say PCI / EISA.  With your change such a driver will fail to load even
if it could serve PCI device in a !CONFIG_EISA config, which is the
behavior we want.

Re: [PATCH v1] ESIA : Dummy eisa_driver_register should return error code

[Christoph Hellwig]

On Tue, Jul 19, 2016 at 12:15:01AM +0530, Arvind Yadav wrote:
> The inline eisa_driver_register stub simply allows compilation on
> systems with CONFIG_EISA disabled. the dummy eisa_driver_register
> does not register an *_eisa_driver at all. The inline
> eisa_driver_register should return to indicate lack of support
> when attempting to register an *_eisa_driver on such a system with
> CONFIG_EISA disabled.

Why?  The idea is that you can simply leave the stub in if registering
say PCI / EISA.  With your change such a driver will fail to load even
if it could serve PCI device in a !CONFIG_EISA config, which is the
behavior we want.

Re: [PATCH net-next v3 10/12] net: dsa: support switchdev ageing time attr

[Florian Fainelli]

Le 18/07/2016 à 20:24, Andrew Lunn a écrit :
> On Mon, Jul 18, 2016 at 08:45:38PM -0400, Vivien Didelot wrote:
>> Add a new function for DSA drivers to handle the switchdev
>> SWITCHDEV_ATTR_ID_BRIDGE_AGEING_TIME attribute.
>>
>> The ageing time is passed as milliseconds.
>>
>> Also because we can have multiple logical bridges on top of a physical
>> switch and ageing time are switch-wide, call the driver function with
>> the fastest ageing time in use on the chip instead of the requested one.
>>
>> Signed-off-by: Vivien Didelot 
>> ---
>>  include/net/dsa.h |  2 ++
>>  net/dsa/slave.c   | 41 +
> 

Hi Andrew,

> Hi Florian
> 
> It looks like the SF2 can do fast ageing per port. What i don't see if
> what configuration options you have. Can you get the fast and the
> normal age time per port? Or is it global?

The normal ageing is global and the value needs to be programmed in
seconds, can can range from 10 to 1,048,575 (encoded on 20 bits). The
fast-ageing can actually be per-port, per-VLAN id, for just dynamic or
static entries etc. and is just a poor name for a flush based on any of
these criteria.
-- 
Florian

Re: [PATCH net-next v3 10/12] net: dsa: support switchdev ageing time attr

[Florian Fainelli]

Le 18/07/2016 à 20:24, Andrew Lunn a écrit :
> On Mon, Jul 18, 2016 at 08:45:38PM -0400, Vivien Didelot wrote:
>> Add a new function for DSA drivers to handle the switchdev
>> SWITCHDEV_ATTR_ID_BRIDGE_AGEING_TIME attribute.
>>
>> The ageing time is passed as milliseconds.
>>
>> Also because we can have multiple logical bridges on top of a physical
>> switch and ageing time are switch-wide, call the driver function with
>> the fastest ageing time in use on the chip instead of the requested one.
>>
>> Signed-off-by: Vivien Didelot 
>> ---
>>  include/net/dsa.h |  2 ++
>>  net/dsa/slave.c   | 41 +
> 

Hi Andrew,

> Hi Florian
> 
> It looks like the SF2 can do fast ageing per port. What i don't see if
> what configuration options you have. Can you get the fast and the
> normal age time per port? Or is it global?

The normal ageing is global and the value needs to be programmed in
seconds, can can range from 10 to 1,048,575 (encoded on 20 bits). The
fast-ageing can actually be per-port, per-VLAN id, for just dynamic or
static entries etc. and is just a poor name for a flush based on any of
these criteria.
-- 
Florian

Re: [PATCH] radix-tree: fix radix_tree_iter_retry() for tagged iterators.

[Ross Zwisler]

On Sat, Jul 16, 2016 at 04:45:31PM +0300, Konstantin Khlebnikov wrote:
> On Fri, Jul 15, 2016 at 10:00 PM, Ross Zwisler
>  wrote:
<>
> > 3) radix_tree_iter_next() via via a non-tagged iteration like
> > radix_tree_for_each_slot().  This currently happens in shmem_tag_pins()
> > and shmem_partial_swap_usage().
> >
> > I think that this case is currently unhandled.  Unlike with
> > radix_tree_iter_retry() case (#1 above) we can't rely on 'count' in the else
> > case of radix_tree_next_slot() to be zero, so I think it's possible we'll 
> > end
> > up executing code in the while() loop in radix_tree_next_slot() that assumes
> > 'slot' is valid.
> >
> > I haven't actually seen this crash on a test setup, but I don't think the
> > current code is safe.
> 
> This is becase distance between ->index and ->next_index now could be
> more that one?
>
> We could fix that by adding "iter->index = iter->next_index - 1;" into
> radix_tree_iter_next()
> right after updating next_index and tweak multi-order itreration logic
> if it depends on that.
> 
> I'd like to keep radix_tree_next_slot() as small as possible because
> this is supposed to be a fast-path.

I think it'll be exactly one?

iter->next_index = __radix_tree_iter_add(iter, 1);

So iter->index will be X, iter->next_index will be X+1, accounting for the
iterator's shift.  So, basically, whatever your height is, you'll be set up to
process one more entry, I think.

This means that radix_tree_chunk_size() will return 1.  I guess with the
current logic this is safe:

static __always_inline void **
radix_tree_next_slot(void **slot, struct radix_tree_iter *iter, unsigned flags)
{
...
} else {
long count = radix_tree_chunk_size(iter);
void *canon = slot;

while (--count > 0) {
/* code that assumes 'slot' is non-NULL */

So 'count' will be 1, the prefix decrement will make it 0, so we won't execute
the code in the while() loop.  So maybe all the cases are covered after all.

It seems like we need some unit tests in tools/testing/radix-tree around this
- I'll try and find time to add them this week.

I just feel like this isn't very organized.  We have a lot of code in
radix_tree_next_slot() that assumes that 'slot' is non-NULL, but we don't
check it anywhere.  We know it *can* be NULL, but we just happen to have
things set up so that none of the code that uses 'slot' is executed.

I personally feel like a quick check for slot==NULL at the beginning of the
function is the simplest way to keep ourselves safe, and it doesn't seem like
we'd be adding that much overhead.

Re: [PATCH] radix-tree: fix radix_tree_iter_retry() for tagged iterators.

[Ross Zwisler]

On Sat, Jul 16, 2016 at 04:45:31PM +0300, Konstantin Khlebnikov wrote:
> On Fri, Jul 15, 2016 at 10:00 PM, Ross Zwisler
>  wrote:
<>
> > 3) radix_tree_iter_next() via via a non-tagged iteration like
> > radix_tree_for_each_slot().  This currently happens in shmem_tag_pins()
> > and shmem_partial_swap_usage().
> >
> > I think that this case is currently unhandled.  Unlike with
> > radix_tree_iter_retry() case (#1 above) we can't rely on 'count' in the else
> > case of radix_tree_next_slot() to be zero, so I think it's possible we'll 
> > end
> > up executing code in the while() loop in radix_tree_next_slot() that assumes
> > 'slot' is valid.
> >
> > I haven't actually seen this crash on a test setup, but I don't think the
> > current code is safe.
> 
> This is becase distance between ->index and ->next_index now could be
> more that one?
>
> We could fix that by adding "iter->index = iter->next_index - 1;" into
> radix_tree_iter_next()
> right after updating next_index and tweak multi-order itreration logic
> if it depends on that.
> 
> I'd like to keep radix_tree_next_slot() as small as possible because
> this is supposed to be a fast-path.

I think it'll be exactly one?

iter->next_index = __radix_tree_iter_add(iter, 1);

So iter->index will be X, iter->next_index will be X+1, accounting for the
iterator's shift.  So, basically, whatever your height is, you'll be set up to
process one more entry, I think.

This means that radix_tree_chunk_size() will return 1.  I guess with the
current logic this is safe:

static __always_inline void **
radix_tree_next_slot(void **slot, struct radix_tree_iter *iter, unsigned flags)
{
...
} else {
long count = radix_tree_chunk_size(iter);
void *canon = slot;

while (--count > 0) {
/* code that assumes 'slot' is non-NULL */

So 'count' will be 1, the prefix decrement will make it 0, so we won't execute
the code in the while() loop.  So maybe all the cases are covered after all.

It seems like we need some unit tests in tools/testing/radix-tree around this
- I'll try and find time to add them this week.

I just feel like this isn't very organized.  We have a lot of code in
radix_tree_next_slot() that assumes that 'slot' is non-NULL, but we don't
check it anywhere.  We know it *can* be NULL, but we just happen to have
things set up so that none of the code that uses 'slot' is executed.

I personally feel like a quick check for slot==NULL at the beginning of the
function is the simplest way to keep ourselves safe, and it doesn't seem like
we'd be adding that much overhead.

Re: [PATCH] i2c: efm32: fix a failure path in efm32_i2c_probe()

[Wolfram Sang]

On Sat, Jul 16, 2016 at 02:36:38AM +0300, Alexey Khoroshilov wrote:
> There is the only failure path in efm32_i2c_probe(),
> where clk_disable_unprepare() is missed.
> 
> Found by Linux Driver Verification project (linuxtesting.org).
> 
> Signed-off-by: Alexey Khoroshilov 

Applied to for-next, thanks!



signature.asc
Description: PGP signature

Re: [PATCH] i2c: efm32: fix a failure path in efm32_i2c_probe()

[Wolfram Sang]

On Sat, Jul 16, 2016 at 02:36:38AM +0300, Alexey Khoroshilov wrote:
> There is the only failure path in efm32_i2c_probe(),
> where clk_disable_unprepare() is missed.
> 
> Found by Linux Driver Verification project (linuxtesting.org).
> 
> Signed-off-by: Alexey Khoroshilov 

Applied to for-next, thanks!



signature.asc
Description: PGP signature

[PATCH V2 0/2] Automatically load the vmx_crypto module if supported

[alastair]

From: Alastair D'Silva 

This series allows the vmx_crypto module to be detected and automatically
loaded via UDEV if the CPU supports the vector crypto feature.

Alastair D'Silva (2):
  powerpc: Add module autoloading based on CPU features
  crypto: vmx - Convert to CPU feature based module autoloading

 arch/powerpc/Kconfig  |  1 +
 arch/powerpc/include/asm/cpufeature.h | 70 +++
 drivers/crypto/vmx/Kconfig|  2 +-
 drivers/crypto/vmx/vmx.c  |  6 +--
 4 files changed, 74 insertions(+), 5 deletions(-)
 create mode 100644 arch/powerpc/include/asm/cpufeature.h

-- 
2.7.4

[PATCH V2 2/2] crypto: vmx - Convert to CPU feature based module autoloading

[alastair]

From: Alastair D'Silva 

This patch utilises the GENERIC_CPU_AUTOPROBE infrastructure
to automatically load the vmx_crypto module if the CPU supports
it.

Signed-off-by: Alastair D'Silva 
---
 drivers/crypto/vmx/Kconfig | 2 +-
 drivers/crypto/vmx/vmx.c   | 6 ++
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/crypto/vmx/Kconfig b/drivers/crypto/vmx/Kconfig
index 89d8208..a83ead1 100644
--- a/drivers/crypto/vmx/Kconfig
+++ b/drivers/crypto/vmx/Kconfig
@@ -1,7 +1,7 @@
 config CRYPTO_DEV_VMX_ENCRYPT
tristate "Encryption acceleration support on P8 CPU"
depends on CRYPTO_DEV_VMX
-   default y
+   default m
help
  Support for VMX cryptographic acceleration instructions on Power8 CPU.
  This module supports acceleration for AES and GHASH in hardware. If 
you
diff --git a/drivers/crypto/vmx/vmx.c b/drivers/crypto/vmx/vmx.c
index e163d57..5a40f2f 100644
--- a/drivers/crypto/vmx/vmx.c
+++ b/drivers/crypto/vmx/vmx.c
@@ -23,6 +23,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -43,9 +44,6 @@ int __init p8_init(void)
int ret = 0;
struct crypto_alg **alg_it;
 
-   if (!(cur_cpu_spec->cpu_user_features2 & PPC_FEATURE2_VEC_CRYPTO))
-   return -ENODEV;
-
for (alg_it = algs; *alg_it; alg_it++) {
ret = crypto_register_alg(*alg_it);
printk(KERN_INFO "crypto_register_alg '%s' = %d\n",
@@ -78,7 +76,7 @@ void __exit p8_exit(void)
crypto_unregister_shash(_ghash_alg);
 }
 
-module_init(p8_init);
+module_cpu_feature_match(PPC_MODULE_FEATURE_VEC_CRYPTO, p8_init);
 module_exit(p8_exit);
 
 MODULE_AUTHOR("Marcelo Cerri");
-- 
2.7.4

V2: No change

[PATCH V2 2/2] crypto: vmx - Convert to CPU feature based module autoloading

[alastair]

From: Alastair D'Silva 

This patch utilises the GENERIC_CPU_AUTOPROBE infrastructure
to automatically load the vmx_crypto module if the CPU supports
it.

Signed-off-by: Alastair D'Silva 
---
 drivers/crypto/vmx/Kconfig | 2 +-
 drivers/crypto/vmx/vmx.c   | 6 ++
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/crypto/vmx/Kconfig b/drivers/crypto/vmx/Kconfig
index 89d8208..a83ead1 100644
--- a/drivers/crypto/vmx/Kconfig
+++ b/drivers/crypto/vmx/Kconfig
@@ -1,7 +1,7 @@
 config CRYPTO_DEV_VMX_ENCRYPT
tristate "Encryption acceleration support on P8 CPU"
depends on CRYPTO_DEV_VMX
-   default y
+   default m
help
  Support for VMX cryptographic acceleration instructions on Power8 CPU.
  This module supports acceleration for AES and GHASH in hardware. If 
you
diff --git a/drivers/crypto/vmx/vmx.c b/drivers/crypto/vmx/vmx.c
index e163d57..5a40f2f 100644
--- a/drivers/crypto/vmx/vmx.c
+++ b/drivers/crypto/vmx/vmx.c
@@ -23,6 +23,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -43,9 +44,6 @@ int __init p8_init(void)
int ret = 0;
struct crypto_alg **alg_it;
 
-   if (!(cur_cpu_spec->cpu_user_features2 & PPC_FEATURE2_VEC_CRYPTO))
-   return -ENODEV;
-
for (alg_it = algs; *alg_it; alg_it++) {
ret = crypto_register_alg(*alg_it);
printk(KERN_INFO "crypto_register_alg '%s' = %d\n",
@@ -78,7 +76,7 @@ void __exit p8_exit(void)
crypto_unregister_shash(_ghash_alg);
 }
 
-module_init(p8_init);
+module_cpu_feature_match(PPC_MODULE_FEATURE_VEC_CRYPTO, p8_init);
 module_exit(p8_exit);
 
 MODULE_AUTHOR("Marcelo Cerri");
-- 
2.7.4

V2: No change

[PATCH V2 0/2] Automatically load the vmx_crypto module if supported

[alastair]

From: Alastair D'Silva 

This series allows the vmx_crypto module to be detected and automatically
loaded via UDEV if the CPU supports the vector crypto feature.

Alastair D'Silva (2):
  powerpc: Add module autoloading based on CPU features
  crypto: vmx - Convert to CPU feature based module autoloading

 arch/powerpc/Kconfig  |  1 +
 arch/powerpc/include/asm/cpufeature.h | 70 +++
 drivers/crypto/vmx/Kconfig|  2 +-
 drivers/crypto/vmx/vmx.c  |  6 +--
 4 files changed, 74 insertions(+), 5 deletions(-)
 create mode 100644 arch/powerpc/include/asm/cpufeature.h

-- 
2.7.4

[PATCH V2 1/2] powerpc: Add module autoloading based on CPU features

[alastair]

From: Alastair D'Silva 

This patch provides the necessary infrastructure to allow drivers
to be automatically loaded via UDEV. It implements the minimum
required to be able to use module_cpu_feature_match to trigger
the GENERIC_CPU_AUTOPROBE mechanisms.

The features exposed are a mirror of the cpu_user_features
(converted to an offset from a mask). This decision was made to
ensure that the behavior between features for module loading and
userspace are consistent.

Signed-off-by: Alastair D'Silva 
---
 arch/powerpc/Kconfig  |  1 +
 arch/powerpc/include/asm/cpufeature.h | 70 +++
 2 files changed, 71 insertions(+)
 create mode 100644 arch/powerpc/include/asm/cpufeature.h

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 0a9d439..a6e49db 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -164,6 +164,7 @@ config PPC
select ARCH_HAS_UBSAN_SANITIZE_ALL
select ARCH_SUPPORTS_DEFERRED_STRUCT_PAGE_INIT
select HAVE_LIVEPATCH if HAVE_DYNAMIC_FTRACE_WITH_REGS
+   select GENERIC_CPU_AUTOPROBE
 
 config GENERIC_CSUM
def_bool CPU_LITTLE_ENDIAN
diff --git a/arch/powerpc/include/asm/cpufeature.h 
b/arch/powerpc/include/asm/cpufeature.h
new file mode 100644
index 000..f6b2ac9
--- /dev/null
+++ b/arch/powerpc/include/asm/cpufeature.h
@@ -0,0 +1,70 @@
+/* CPU feature definitions for module loading, used by
+ * module_cpu_feature_match(), see asm/cputable.h for powerpc CPU features
+ *
+ * Copyright 2016 Alastair D'Silva, IBM Corporation.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#ifndef __ASM_POWERPC_CPUFEATURE_H
+#define __ASM_POWERPC_CPUFEATURE_H
+
+#include 
+
+/* Keep these in step with powerpc/include/asm/cputable.h */
+#define MAX_CPU_FEATURES (2 * 32)
+
+#define PPC_MODULE_FEATURE_32  (ilog2(PPC_FEATURE_32))
+#define PPC_MODULE_FEATURE_64  (ilog2(PPC_FEATURE_64))
+#define PPC_MODULE_FEATURE_601_INSTR   
(ilog2(PPC_FEATURE_601_INSTR))
+#define PPC_MODULE_FEATURE_HAS_ALTIVEC 
(ilog2(PPC_FEATURE_HAS_ALTIVEC))
+#define PPC_MODULE_FEATURE_HAS_FPU 
(ilog2(PPC_FEATURE_HAS_FPU))
+#define PPC_MODULE_FEATURE_HAS_MMU 
(ilog2(PPC_FEATURE_HAS_MMU))
+#define PPC_MODULE_FEATURE_HAS_4xxMAC  
(ilog2(PPC_FEATURE_HAS_4xxMAC))
+#define PPC_MODULE_FEATURE_UNIFIED_CACHE   
ilog2(PPC_FEATURE_UNIFIED_CACHE))
+#define PPC_MODULE_FEATURE_HAS_SPE 
(ilog2(PPC_FEATURE_HAS_SPE))
+#define PPC_MODULE_FEATURE_HAS_EFP_SINGLE  
(ilog2(PPC_FEATURE_HAS_EFP_SINGLE))
+#define PPC_MODULE_FEATURE_HAS_EFP_DOUBLE  
(ilog2(PPC_FEATURE_HAS_EFP_DOUBLE))
+#define PPC_MODULE_FEATURE_NO_TB   
(ilog2(PPC_FEATURE_NO_TB))
+#define PPC_MODULE_FEATURE_POWER4  
(ilog2(PPC_FEATURE_POWER4))
+#define PPC_MODULE_FEATURE_POWER5  
(ilog2(PPC_FEATURE_POWER5))
+#define PPC_MODULE_FEATURE_POWER5_PLUS 
(ilog2(PPC_FEATURE_POWER5_PLUS))
+#define PPC_MODULE_FEATURE_CELL
(ilog2(PPC_FEATURE_CELL))
+#define PPC_MODULE_FEATURE_BOOKE   
(ilog2(PPC_FEATURE_BOOKE))
+#define PPC_MODULE_FEATURE_SMT (ilog2(PPC_FEATURE_SMT))
+#define PPC_MODULE_FEATURE_ICACHE_SNOOP
(ilog2(PPC_FEATURE_ICACHE_SNOOP))
+#define PPC_MODULE_FEATURE_ARCH_2_05   
(ilog2(PPC_FEATURE_ARCH_2_05))
+#define PPC_MODULE_FEATURE_PA6T
(ilog2(PPC_FEATURE_PA6T))
+#define PPC_MODULE_FEATURE_HAS_DFP 
(ilog2(PPC_FEATURE_HAS_DFP))
+#define PPC_MODULE_FEATURE_POWER6_EXT  
(ilog2(PPC_FEATURE_POWER6_EXT))
+#define PPC_MODULE_FEATURE_ARCH_2_06   
(ilog2(PPC_FEATURE_ARCH_2_06))
+#define PPC_MODULE_FEATURE_HAS_VSX 
(ilog2(PPC_FEATURE_HAS_VSX))
+#define PPC_MODULE_FEATURE_PSERIES_PERFMON_COMPAT  
(ilog2(PPC_FEATURE_PSERIES_PERFMON_COMPAT))
+#define PPC_MODULE_FEATURE_TRUE_LE 
(ilog2(PPC_FEATURE_TRUE_LE))
+#define PPC_MODULE_FEATURE_PPC_LE  
(ilog2(PPC_FEATURE_PPC_LE))
+
+#define PPC_MODULE_FEATURE_ARCH_2_07   (32 + 
ilog2(PPC_FEATURE2_ARCH_2_07))
+#define PPC_MODULE_FEATURE_HTM (32 + 
ilog2(PPC_FEATURE2_HTM))
+#define PPC_MODULE_FEATURE_DSCR(32 + 
ilog2(PPC_FEATURE2_DSCR))
+#define PPC_MODULE_FEATURE_EBB (32 + 
ilog2(PPC_FEATURE2_EBB))
+#define PPC_MODULE_FEATURE_ISEL(32 + 
ilog2(PPC_FEATURE2_ISEL))
+#define 

[PATCH V2 1/2] powerpc: Add module autoloading based on CPU features

[alastair]

From: Alastair D'Silva 

This patch provides the necessary infrastructure to allow drivers
to be automatically loaded via UDEV. It implements the minimum
required to be able to use module_cpu_feature_match to trigger
the GENERIC_CPU_AUTOPROBE mechanisms.

The features exposed are a mirror of the cpu_user_features
(converted to an offset from a mask). This decision was made to
ensure that the behavior between features for module loading and
userspace are consistent.

Signed-off-by: Alastair D'Silva 
---
 arch/powerpc/Kconfig  |  1 +
 arch/powerpc/include/asm/cpufeature.h | 70 +++
 2 files changed, 71 insertions(+)
 create mode 100644 arch/powerpc/include/asm/cpufeature.h

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 0a9d439..a6e49db 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -164,6 +164,7 @@ config PPC
select ARCH_HAS_UBSAN_SANITIZE_ALL
select ARCH_SUPPORTS_DEFERRED_STRUCT_PAGE_INIT
select HAVE_LIVEPATCH if HAVE_DYNAMIC_FTRACE_WITH_REGS
+   select GENERIC_CPU_AUTOPROBE
 
 config GENERIC_CSUM
def_bool CPU_LITTLE_ENDIAN
diff --git a/arch/powerpc/include/asm/cpufeature.h 
b/arch/powerpc/include/asm/cpufeature.h
new file mode 100644
index 000..f6b2ac9
--- /dev/null
+++ b/arch/powerpc/include/asm/cpufeature.h
@@ -0,0 +1,70 @@
+/* CPU feature definitions for module loading, used by
+ * module_cpu_feature_match(), see asm/cputable.h for powerpc CPU features
+ *
+ * Copyright 2016 Alastair D'Silva, IBM Corporation.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#ifndef __ASM_POWERPC_CPUFEATURE_H
+#define __ASM_POWERPC_CPUFEATURE_H
+
+#include 
+
+/* Keep these in step with powerpc/include/asm/cputable.h */
+#define MAX_CPU_FEATURES (2 * 32)
+
+#define PPC_MODULE_FEATURE_32  (ilog2(PPC_FEATURE_32))
+#define PPC_MODULE_FEATURE_64  (ilog2(PPC_FEATURE_64))
+#define PPC_MODULE_FEATURE_601_INSTR   
(ilog2(PPC_FEATURE_601_INSTR))
+#define PPC_MODULE_FEATURE_HAS_ALTIVEC 
(ilog2(PPC_FEATURE_HAS_ALTIVEC))
+#define PPC_MODULE_FEATURE_HAS_FPU 
(ilog2(PPC_FEATURE_HAS_FPU))
+#define PPC_MODULE_FEATURE_HAS_MMU 
(ilog2(PPC_FEATURE_HAS_MMU))
+#define PPC_MODULE_FEATURE_HAS_4xxMAC  
(ilog2(PPC_FEATURE_HAS_4xxMAC))
+#define PPC_MODULE_FEATURE_UNIFIED_CACHE   
ilog2(PPC_FEATURE_UNIFIED_CACHE))
+#define PPC_MODULE_FEATURE_HAS_SPE 
(ilog2(PPC_FEATURE_HAS_SPE))
+#define PPC_MODULE_FEATURE_HAS_EFP_SINGLE  
(ilog2(PPC_FEATURE_HAS_EFP_SINGLE))
+#define PPC_MODULE_FEATURE_HAS_EFP_DOUBLE  
(ilog2(PPC_FEATURE_HAS_EFP_DOUBLE))
+#define PPC_MODULE_FEATURE_NO_TB   
(ilog2(PPC_FEATURE_NO_TB))
+#define PPC_MODULE_FEATURE_POWER4  
(ilog2(PPC_FEATURE_POWER4))
+#define PPC_MODULE_FEATURE_POWER5  
(ilog2(PPC_FEATURE_POWER5))
+#define PPC_MODULE_FEATURE_POWER5_PLUS 
(ilog2(PPC_FEATURE_POWER5_PLUS))
+#define PPC_MODULE_FEATURE_CELL
(ilog2(PPC_FEATURE_CELL))
+#define PPC_MODULE_FEATURE_BOOKE   
(ilog2(PPC_FEATURE_BOOKE))
+#define PPC_MODULE_FEATURE_SMT (ilog2(PPC_FEATURE_SMT))
+#define PPC_MODULE_FEATURE_ICACHE_SNOOP
(ilog2(PPC_FEATURE_ICACHE_SNOOP))
+#define PPC_MODULE_FEATURE_ARCH_2_05   
(ilog2(PPC_FEATURE_ARCH_2_05))
+#define PPC_MODULE_FEATURE_PA6T
(ilog2(PPC_FEATURE_PA6T))
+#define PPC_MODULE_FEATURE_HAS_DFP 
(ilog2(PPC_FEATURE_HAS_DFP))
+#define PPC_MODULE_FEATURE_POWER6_EXT  
(ilog2(PPC_FEATURE_POWER6_EXT))
+#define PPC_MODULE_FEATURE_ARCH_2_06   
(ilog2(PPC_FEATURE_ARCH_2_06))
+#define PPC_MODULE_FEATURE_HAS_VSX 
(ilog2(PPC_FEATURE_HAS_VSX))
+#define PPC_MODULE_FEATURE_PSERIES_PERFMON_COMPAT  
(ilog2(PPC_FEATURE_PSERIES_PERFMON_COMPAT))
+#define PPC_MODULE_FEATURE_TRUE_LE 
(ilog2(PPC_FEATURE_TRUE_LE))
+#define PPC_MODULE_FEATURE_PPC_LE  
(ilog2(PPC_FEATURE_PPC_LE))
+
+#define PPC_MODULE_FEATURE_ARCH_2_07   (32 + 
ilog2(PPC_FEATURE2_ARCH_2_07))
+#define PPC_MODULE_FEATURE_HTM (32 + 
ilog2(PPC_FEATURE2_HTM))
+#define PPC_MODULE_FEATURE_DSCR(32 + 
ilog2(PPC_FEATURE2_DSCR))
+#define PPC_MODULE_FEATURE_EBB (32 + 
ilog2(PPC_FEATURE2_EBB))
+#define PPC_MODULE_FEATURE_ISEL(32 + 
ilog2(PPC_FEATURE2_ISEL))
+#define PPC_MODULE_FEATURE_TAR (32 + 

Re: [PATCH 5/5] f2fs: use blk_plug in all the possible paths

[Christoph Hellwig]

On Thu, Jul 14, 2016 at 08:05:02PM -0700, Jaegeuk Kim wrote:
> >From kernel guys working on android.

Well, until it's mainline it simply doesn't matter, so NAK to this
patch.  Tying power behavior to plugs also sounds pretty broken, so we'd
probably come up with something better if they bothered to actually
submit their patches upsteam or at least explaining what they want
to archive.

Re: [PATCH 0/2] Automatically load the vmx_crypto module if supported

[Herbert Xu]

On Tue, Jul 19, 2016 at 11:01:55AM +1000, Michael Ellerman wrote:
>
> Can you please ask for an ack before merging arch patches?
> 
> That's a 70 line powerpc patch and a 6 line crypto patch. It has no
> reviews and no acks. I would have preferred it if we could take it via
> the powerpc tree.

Sorry, I'll delete them from the crypto tree.

Cheers,
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH 5/5] f2fs: use blk_plug in all the possible paths

[Christoph Hellwig]

On Thu, Jul 14, 2016 at 08:05:02PM -0700, Jaegeuk Kim wrote:
> >From kernel guys working on android.

Well, until it's mainline it simply doesn't matter, so NAK to this
patch.  Tying power behavior to plugs also sounds pretty broken, so we'd
probably come up with something better if they bothered to actually
submit their patches upsteam or at least explaining what they want
to archive.

Re: [PATCH 0/2] Automatically load the vmx_crypto module if supported

[Herbert Xu]

On Tue, Jul 19, 2016 at 11:01:55AM +1000, Michael Ellerman wrote:
>
> Can you please ask for an ack before merging arch patches?
> 
> That's a 70 line powerpc patch and a 6 line crypto patch. It has no
> reviews and no acks. I would have preferred it if we could take it via
> the powerpc tree.

Sorry, I'll delete them from the crypto tree.

Cheers,
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: linux-next: manual merge of the pm tree with the i2c tree

[Wolfram Sang]

> > Well, not knowing much about ACPI, I just need the conflict resolution
> > for my latest i2c/for-next and your above branch. If you want to do it,
> > fine with me. But maybe Jarkko will be back to office on Monday, too.
> 
> Unfortunately, I don't see how these branches can be merged in a sensible
> way without adding too much new code into the merge itself.
> 
> Something needs to be dropped and then rebased and applied again.

Okay, I'll drop the I2C parts. Next to the core parts which I will drop,
there was also a driver patch making use of the core changes for which I
requested some updates. Since those did not happen yet (Jarkko on
holiday?), the core patches alone are not important anyhow.

Thanks for looking into it!

   Wolfram



signature.asc
Description: PGP signature

Re: linux-next: manual merge of the pm tree with the i2c tree

[Wolfram Sang]

> > Well, not knowing much about ACPI, I just need the conflict resolution
> > for my latest i2c/for-next and your above branch. If you want to do it,
> > fine with me. But maybe Jarkko will be back to office on Monday, too.
> 
> Unfortunately, I don't see how these branches can be merged in a sensible
> way without adding too much new code into the merge itself.
> 
> Something needs to be dropped and then rebased and applied again.

Okay, I'll drop the I2C parts. Next to the core parts which I will drop,
there was also a driver patch making use of the core changes for which I
requested some updates. Since those did not happen yet (Jarkko on
holiday?), the core patches alone are not important anyhow.

Thanks for looking into it!

   Wolfram



signature.asc
Description: PGP signature

Re: [PATCH 3/3] f2fs: support clone_file_range

[Christoph Hellwig]

On Sat, Jul 16, 2016 at 11:24:27PM -0700, Jaegeuk Kim wrote:
> This patch implements clone_file_range in f2fs.

[...]

> +int f2fs_clone_file_range(struct file *file_in, loff_t pos_in,
> + struct file *file_out, loff_t pos_out, u64 len)
> +{
> + return f2fs_clone_files(file_in, pos_in, file_out, pos_out, len);
> +}
> +

Falling back from copy to clone should be done in the VFS.  I look into
implementing the fallback, and the code is trivial, but we don't seem
to have any useful coverage for copy_file_range yet, so I didn't dare
to add it yet.  How did you test copy and clone in f2fs?  And how did
you handle the difference in corner cases (e.g. the lacking 0 special
case in copy)?

Re: [PATCH 3/3] f2fs: support clone_file_range

[Christoph Hellwig]

On Sat, Jul 16, 2016 at 11:24:27PM -0700, Jaegeuk Kim wrote:
> This patch implements clone_file_range in f2fs.

[...]

> +int f2fs_clone_file_range(struct file *file_in, loff_t pos_in,
> + struct file *file_out, loff_t pos_out, u64 len)
> +{
> + return f2fs_clone_files(file_in, pos_in, file_out, pos_out, len);
> +}
> +

Falling back from copy to clone should be done in the VFS.  I look into
implementing the fallback, and the code is trivial, but we don't seem
to have any useful coverage for copy_file_range yet, so I didn't dare
to add it yet.  How did you test copy and clone in f2fs?  And how did
you handle the difference in corner cases (e.g. the lacking 0 special
case in copy)?

Re: [PATCH] hwmon-ntc_thermistor: Delete an unnecessary check before the function call "iio_channel_release"

[Guenter Roeck]

On 07/18/2016 11:54 AM, SF Markus Elfring wrote:

From: Markus Elfring 
Date: Mon, 18 Jul 2016 20:34:41 +0200

The iio_channel_release() function tests whether its argument is NULL
and then returns immediately. Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 


A much better change would be to use devm_iio_channel_get() instead of
iio_channel_get(), and to remove ntc_iio_channel_release() completely.

Guenter

Re: [PATCH] hwmon-ntc_thermistor: Delete an unnecessary check before the function call "iio_channel_release"

[Guenter Roeck]

On 07/18/2016 11:54 AM, SF Markus Elfring wrote:

From: Markus Elfring 
Date: Mon, 18 Jul 2016 20:34:41 +0200

The iio_channel_release() function tests whether its argument is NULL
and then returns immediately. Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 


A much better change would be to use devm_iio_channel_get() instead of
iio_channel_get(), and to remove ntc_iio_channel_release() completely.

Guenter

Re: [RFC PATCH V2] s390/perf: fix 'start' address of module's map

[Songshan Gong]

在 7/19/2016 9:50 AM, Songshan Gong 写道:



在 7/19/2016 9:37 AM, Songshan Gong 写道:



在 7/18/2016 10:07 PM, Jiri Olsa 写道:

On Fri, Jul 15, 2016 at 06:15:11PM +0800, Song Shan Gong wrote:

At preset, when creating module's map, perf gets 'start' address by
parsing
'/proc/modules', but it's module base address, isn't the start
address of
'.text' section. In most archs, it's OK. But for s390, it places
'GOT' and
'PLT' relocations before '.text' section. So there exists an offset
between
module base address and '.text' section, which will incur wrong symbol
resolution for modules.

Fix this bug by getting 'start' address of module's map from parsing
'/sys/module/[module name]/sections/.text', not from '/proc/modules'.

Signed-off-by: Song Shan Gong 
---
 tools/perf/arch/s390/util/Build  |  2 ++
 tools/perf/arch/s390/util/sym-handling.c | 27
+++
 tools/perf/util/machine.c|  8 
 tools/perf/util/machine.h|  1 +
 4 files changed, 38 insertions(+)
 create mode 100644 tools/perf/arch/s390/util/sym-handling.c

diff --git a/tools/perf/arch/s390/util/Build
b/tools/perf/arch/s390/util/Build
index 8a61372..5e322ed 100644
--- a/tools/perf/arch/s390/util/Build
+++ b/tools/perf/arch/s390/util/Build
@@ -2,3 +2,5 @@ libperf-y += header.o
 libperf-y += kvm-stat.o

 libperf-$(CONFIG_DWARF) += dwarf-regs.o
+
+libperf-y += sym-handling.o
diff --git a/tools/perf/arch/s390/util/sym-handling.c
b/tools/perf/arch/s390/util/sym-handling.c
new file mode 100644
index 000..ff51336
--- /dev/null
+++ b/tools/perf/arch/s390/util/sym-handling.c
@@ -0,0 +1,27 @@
+#include 
+#include 
+#include 
+#include "util.h"
+#include "machine.h"
+#include "api/fs/fs.h"
+
+int arch__fix_module_text_start(u64 *start, const char *name)
+{
+char path[PATH_MAX];
+char *module_name = NULL;
+int len;
+
+if (!(module_name = strdup(name)))
+return -1;
+
+len = strlen(module_name);
+module_name[len - 1] = '\0';
+snprintf(path, PATH_MAX, "module/%s/sections/.text",
+module_name + 1);


why can't you use 'name' in here? I can't see the reason
you allocated module_name..


Oh, yes.


Sorry, just ignore the upper reply.

I use the 'module_name' here instead of 'name', because 'name' is
something like '[module_name]', I need strip the '[' and ']', so I have
to set the tail ']' to '\0' to indicate the string end, which will need
change the constant variable 'name'.


, I found a new way to implement it.

I could use '%.*s' to format the 'path', 'name' is enough indeed.

It's so concise now. Appreciate.





+
+if (sysfs__read_ull(path, (unsigned long long *)start) < 0) {
+free(module_name);
+return -1;
+}


leaking module_name in here


yes, thanks.



+return 0;
+}


SNIP

thanks,
jirka







--
SongShan Gong

Re: [RFC PATCH V2] s390/perf: fix 'start' address of module's map

[Songshan Gong]

在 7/19/2016 9:50 AM, Songshan Gong 写道:



在 7/19/2016 9:37 AM, Songshan Gong 写道:



在 7/18/2016 10:07 PM, Jiri Olsa 写道:

On Fri, Jul 15, 2016 at 06:15:11PM +0800, Song Shan Gong wrote:

At preset, when creating module's map, perf gets 'start' address by
parsing
'/proc/modules', but it's module base address, isn't the start
address of
'.text' section. In most archs, it's OK. But for s390, it places
'GOT' and
'PLT' relocations before '.text' section. So there exists an offset
between
module base address and '.text' section, which will incur wrong symbol
resolution for modules.

Fix this bug by getting 'start' address of module's map from parsing
'/sys/module/[module name]/sections/.text', not from '/proc/modules'.

Signed-off-by: Song Shan Gong 
---
 tools/perf/arch/s390/util/Build  |  2 ++
 tools/perf/arch/s390/util/sym-handling.c | 27
+++
 tools/perf/util/machine.c|  8 
 tools/perf/util/machine.h|  1 +
 4 files changed, 38 insertions(+)
 create mode 100644 tools/perf/arch/s390/util/sym-handling.c

diff --git a/tools/perf/arch/s390/util/Build
b/tools/perf/arch/s390/util/Build
index 8a61372..5e322ed 100644
--- a/tools/perf/arch/s390/util/Build
+++ b/tools/perf/arch/s390/util/Build
@@ -2,3 +2,5 @@ libperf-y += header.o
 libperf-y += kvm-stat.o

 libperf-$(CONFIG_DWARF) += dwarf-regs.o
+
+libperf-y += sym-handling.o
diff --git a/tools/perf/arch/s390/util/sym-handling.c
b/tools/perf/arch/s390/util/sym-handling.c
new file mode 100644
index 000..ff51336
--- /dev/null
+++ b/tools/perf/arch/s390/util/sym-handling.c
@@ -0,0 +1,27 @@
+#include 
+#include 
+#include 
+#include "util.h"
+#include "machine.h"
+#include "api/fs/fs.h"
+
+int arch__fix_module_text_start(u64 *start, const char *name)
+{
+char path[PATH_MAX];
+char *module_name = NULL;
+int len;
+
+if (!(module_name = strdup(name)))
+return -1;
+
+len = strlen(module_name);
+module_name[len - 1] = '\0';
+snprintf(path, PATH_MAX, "module/%s/sections/.text",
+module_name + 1);


why can't you use 'name' in here? I can't see the reason
you allocated module_name..


Oh, yes.


Sorry, just ignore the upper reply.

I use the 'module_name' here instead of 'name', because 'name' is
something like '[module_name]', I need strip the '[' and ']', so I have
to set the tail ']' to '\0' to indicate the string end, which will need
change the constant variable 'name'.


, I found a new way to implement it.

I could use '%.*s' to format the 'path', 'name' is enough indeed.

It's so concise now. Appreciate.





+
+if (sysfs__read_ull(path, (unsigned long long *)start) < 0) {
+free(module_name);
+return -1;
+}


leaking module_name in here


yes, thanks.



+return 0;
+}


SNIP

thanks,
jirka







--
SongShan Gong