From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1752023AbbFZIRj (ORCPT <rfc822;w@1wt.eu>);
	Fri, 26 Jun 2015 04:17:39 -0400
Received: from mail-pd0-f175.google.com ([209.85.192.175]:35400 "EHLO
	mail-pd0-f175.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1751922AbbFZIRb (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Fri, 26 Jun 2015 04:17:31 -0400
Message-ID: <1435306644.7057.2.camel@ingics.com>
Subject: spmi: Question about qpnpint_irq_set_type implement
From: Axel Lin <axel.lin@ingics.com>
To: linux-kernel@vger.kernel.org
Cc: Gilad Avidov <gavidov@codeaurora.org>,
        Sagar Dharia <sdharia@codeaurora.org>,
        Josh Cartwright <joshc@codeaurora.org>,
        Kenneth Heitke <kheitke@codeaurora.org>,
        "Ivan T. Ivanov" <iivanov@mm-sol.com>,
        GregKroah-Hartman <gregkh@linuxfoundation.org>
Date: Fri, 26 Jun 2015 16:17:24 +0800
Content-Type: text/plain; charset="UTF-8"
X-Mailer: Evolution 3.12.10-0ubuntu1~14.10.1 
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi,
Current implementation in qpnpint_irq_set_type() will set (1 << irq) bit
for type.polarity_high/type.polarity_low but never clear this bit.
I'm wondering if it is intentional because the value write to
QPNPINT_REG_SET_TYPE register depends on it's original value.

Maybe it needs below changes, comments?

diff --git a/drivers/spmi/spmi-pmic-arb.c b/drivers/spmi/spmi-pmic-arb.c
index 6ea6eab..fe59cf4 100644
--- a/drivers/spmi/spmi-pmic-arb.c
+++ b/drivers/spmi/spmi-pmic-arb.c
@@ -557,18 +557,26 @@ static int qpnpint_irq_set_type(struct irq_data *d, unsigned int flow_type)
 		type.type |= 1 << irq;
 		if (flow_type & IRQF_TRIGGER_RISING)
 			type.polarity_high |= 1 << irq;
+		else
+			type.polarity_high &= ~(1 << irq);
+
 		if (flow_type & IRQF_TRIGGER_FALLING)
 			type.polarity_low  |= 1 << irq;
+		else
+			type.polarity_low &= ~(1 << irq);
 	} else {
 		if ((flow_type & (IRQF_TRIGGER_HIGH)) &&
 		    (flow_type & (IRQF_TRIGGER_LOW)))
 			return -EINVAL;
 
 		type.type &= ~(1 << irq); /* level trig */
-		if (flow_type & IRQF_TRIGGER_HIGH)
+		if (flow_type & IRQF_TRIGGER_HIGH) {
 			type.polarity_high |= 1 << irq;
-		else
+			type.polarity_low &= ~(1 << irq);
+		} else {
+			type.polarity_high &= ~(1 << irq);
 			type.polarity_low  |= 1 << irq;
+		}
 	}
 
 	qpnpint_spmi_write(d, QPNPINT_REG_SET_TYPE, &type, sizeof(type));