From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.6 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_PASS, URIBL_BLOCKED,USER_AGENT_MUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7A004C43142 for ; Thu, 2 Aug 2018 08:05:33 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 0C04B2146D for ; Thu, 2 Aug 2018 08:05:33 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=linaro.org header.i=@linaro.org header.b="RZUXAI+H" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 0C04B2146D Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linaro.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727260AbeHBJz3 (ORCPT ); Thu, 2 Aug 2018 05:55:29 -0400 Received: from mail-wm0-f65.google.com ([74.125.82.65]:35668 "EHLO mail-wm0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726735AbeHBJz2 (ORCPT ); Thu, 2 Aug 2018 05:55:28 -0400 Received: by mail-wm0-f65.google.com with SMTP id o18-v6so1444092wmc.0 for ; Thu, 02 Aug 2018 01:05:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=q7WXrxM49cKiZDEzJD4fwIpf+oadpdyg7AkB52KE0o0=; b=RZUXAI+HVkicOKQAb/KlNm052pvArwFMF4ryDhViqv0Hccnm/Bm8oJrtf7wznAtyrH M+4OKUqgTt9lejo38GerPAKGSaV6bFXB/5JGrSww7CgAwHZ5Yz1oxCSBRu+Lpdmp67du QP4rQH2ufy+CRe68PAvK/4ilzRu+EuVRLOg2E= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=q7WXrxM49cKiZDEzJD4fwIpf+oadpdyg7AkB52KE0o0=; b=FxxcZXVseKoPiH/bycia/muUFlbBVngR5moGt2NVU+oBa6Xc2gPVs0X5wFzkVMlx16 D/4STSj1rGHHxg0BajqwjnEVeaj0iIQFls2qS2MA3xDRlqXDq1qD1upT7vJKadHO+dfh VV6soakYbc66cbCl3odO1XWYEQK+AFvZk+mMiLkbGFl7afBssYjx5rAYJ9UiiYSr9GOv 8t167sjVdecJoW7Ojd1jzUWCrhQg8lMOEW6h5bkyeR4NjWjXT0e2Gue9YjPp/O1flkwa BaFDNzIm8vLILiPW0T0YfdntwCioX4LxJJLsyEztZLzWRbckNZfO/OBOnX5LPJg2+Zp5 J+Ig== X-Gm-Message-State: AOUpUlE8u15+MRIu68yjoVIgcwPE2HLfQCd9MMk0GF0eh0rMCVQ9iGJf Via/TlNWo9Tv4saocMjRsdtkhQ== X-Google-Smtp-Source: AAOMgpdQJtdkJijAGN3izpB64Pf3dnK9x6lYAwg8gknn8cvQx5fBb+hT9kXxbfgU/SyauMjLAAmFNw== X-Received: by 2002:a1c:98a:: with SMTP id 132-v6mr1205080wmj.86.1533197128345; Thu, 02 Aug 2018 01:05:28 -0700 (PDT) Received: from dell ([95.149.160.4]) by smtp.gmail.com with ESMTPSA id b2-v6sm1578456wmh.3.2018.08.02.01.05.27 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Thu, 02 Aug 2018 01:05:27 -0700 (PDT) Date: Thu, 2 Aug 2018 09:05:25 +0100 From: Lee Jones To: Srinivas Kandagatla Cc: robh+dt@kernel.org, broonie@kernel.org, mark.rutland@arm.com, lgirdwood@gmail.com, tiwai@suse.com, bgoswami@codeaurora.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, vkoul@kernel.org, alsa-devel@alsa-project.org Subject: Re: [PATCH v2 03/10] mfd: wcd9335: add wcd irq support Message-ID: <20180802080525.GM4662@dell> References: <20180727121806.18209-1-srinivas.kandagatla@linaro.org> <20180727121806.18209-4-srinivas.kandagatla@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20180727121806.18209-4-srinivas.kandagatla@linaro.org> User-Agent: Mutt/1.9.4 (2018-02-28) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 27 Jul 2018, Srinivas Kandagatla wrote: > WCD9335 supports two lines of irqs INTR1 and INTR2. > Multiple interrupts are muxed via these lines. > INTR1 consists of all possible interrupt sources like: > Ear OCP, HPH OCP, MBHC, MAD, VBAT, and SVA > INTR2 is a subset of first interrupt sources like MAD, VBAT, and SVA > > Signed-off-by: Srinivas Kandagatla > --- > drivers/mfd/Makefile | 2 +- > drivers/mfd/wcd9335-core.c | 9 ++ > drivers/mfd/wcd9335-irq.c | 172 ++++++++++++++++++++++++++++++++++++ Any particular reason for separating out IRQ handling? > include/dt-bindings/mfd/wcd9335.h | 43 +++++++++ > include/linux/mfd/wcd9335/wcd9335.h | 3 + > 5 files changed, 228 insertions(+), 1 deletion(-) > create mode 100644 drivers/mfd/wcd9335-irq.c > create mode 100644 include/dt-bindings/mfd/wcd9335.h > > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile > index a4697370640b..210875afe78a 100644 > --- a/drivers/mfd/Makefile > +++ b/drivers/mfd/Makefile > @@ -58,7 +58,7 @@ obj-$(CONFIG_MFD_ARIZONA) += cs47l24-tables.o > endif > > obj-$(CONFIG_MFD_WCD9335) += wcd9335.o > -wcd9335-objs := wcd9335-core.o > +wcd9335-objs := wcd9335-core.o wcd9335-irq.o > > obj-$(CONFIG_MFD_WM8400) += wm8400-core.o > wm831x-objs := wm831x-core.o wm831x-irq.o wm831x-otp.o > diff --git a/drivers/mfd/wcd9335-core.c b/drivers/mfd/wcd9335-core.c > index 8f746901f4e9..6299dfb63aca 100644 > --- a/drivers/mfd/wcd9335-core.c > +++ b/drivers/mfd/wcd9335-core.c > @@ -243,12 +243,20 @@ static int wcd9335_slim_status(struct slim_device *sdev, > return ret; > } > > + wcd9335_irq_init(wcd); Why don't you check the return value? What happens if it defers? > wcd->slim_ifd = wcd->slim_ifd; > > return mfd_add_devices(wcd->dev, 0, wcd9335_devices, > ARRAY_SIZE(wcd9335_devices), NULL, 0, NULL); > } > > +static void wcd9335_slim_remove(struct slim_device *sdev) > +{ > + struct wcd9335 *wcd = dev_get_drvdata(&sdev->dev); > + > + wcd9335_irq_exit(wcd); > +} > + > static const struct slim_device_id wcd9335_slim_id[] = { > {0x217, 0x1a0, 0x1, 0x0}, > {} > @@ -259,6 +267,7 @@ static struct slim_driver wcd9335_slim_driver = { > .name = "wcd9335-slim", > }, > .probe = wcd9335_slim_probe, > + .remove = wcd9335_slim_remove, > .device_status = wcd9335_slim_status, > .id_table = wcd9335_slim_id, > }; > diff --git a/drivers/mfd/wcd9335-irq.c b/drivers/mfd/wcd9335-irq.c > new file mode 100644 > index 000000000000..84098c89419b > --- /dev/null > +++ b/drivers/mfd/wcd9335-irq.c > @@ -0,0 +1,172 @@ > +// SPDX-License-Identifier: GPL-2.0 > +// Copyright (c) 2018, Linaro Limited > +// Blank line? > +#include > +#include > +#include > +#include > +#include > +#include > +#include Please place in alphabetical order. > +static const struct regmap_irq wcd9335_irqs[] = { > + /* INTR_REG 0 */ > + [WCD9335_IRQ_SLIMBUS] = { > + .reg_offset = 0, > + .mask = BIT(0), > + }, [snipped approx 1,000,000 entries] > + [WCD9335_IRQ_SVA_OUTBOX2] = { > + .reg_offset = 3, > + .mask = BIT(6), > + }, > +}; Please use REGMAP_IRQ_REG() to significantly reduce the diff. > +static const struct regmap_irq_chip wcd9335_regmap_irq1_chip = { > + .name = "wcd9335_pin1_irq", > + .status_base = WCD9335_INTR_PIN1_STATUS0, > + .mask_base = WCD9335_INTR_PIN1_MASK0, > + .ack_base = WCD9335_INTR_PIN1_CLEAR0, > + .type_base = WCD9335_INTR_LEVEL0, > + .num_regs = 4, > + .irqs = wcd9335_irqs, > + .num_irqs = ARRAY_SIZE(wcd9335_irqs), > +}; > + > +int wcd9335_irq_init(struct wcd9335 *wcd) > +{ > + int ret; '\n' here. > + /* > + * INTR1 consists of all possible interrupt sources Ear OCP, > + * HPH OCP, MBHC, MAD, VBAT, and SVA > + * INTR2 is a subset of first interrupt sources MAD, VBAT, and SVA > + */ > + wcd->intr1 = of_irq_get_byname(wcd->dev->of_node, "intr1"); > + if (wcd->intr1 < 0 || wcd->intr1 == -EPROBE_DEFER) { > + dev_err(wcd->dev, "Unable to configure irq\n"); Nit: s/irq/IRQ/ Do you really want to issue an error message for -EPROBE_DEFER? This maybe confusing if it is initiated later. > + return wcd->intr1; > + } > + > + ret = regmap_add_irq_chip(wcd->regmap, wcd->intr1, > + IRQF_TRIGGER_HIGH | IRQF_ONESHOT, > + 0, &wcd9335_regmap_irq1_chip, > + &wcd->irq_data); > + if (ret != 0) { "if (ret)" > + dev_err(wcd->dev, "Failed to register IRQ chip: %d\n", ret); > + return ret; > + } > + > + return 0; > +} > + > +int wcd9335_irq_exit(struct wcd9335 *wcd) > +{ > + regmap_del_irq_chip(wcd->intr1, wcd->irq_data); '\n' here. > + return 0; > +} Or better still, make this a void and remove the superfluous return. > diff --git a/include/dt-bindings/mfd/wcd9335.h b/include/dt-bindings/mfd/wcd9335.h > new file mode 100644 > index 000000000000..61b6a11da00d > --- /dev/null > +++ b/include/dt-bindings/mfd/wcd9335.h > @@ -0,0 +1,43 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * This header provides macros for WCD9335 device bindings. > + * > + * Copyright (c) 2018, Linaro Limited > + */ > + > +#ifndef _DT_BINDINGS_MFD_WCD9335_H > +#define _DT_BINDINGS_MFD_WCD9335_H > + > +#define WCD9335_IRQ_SLIMBUS 0 > +#define WCD9335_IRQ_FLL_LOCK_LOSS 1 > +#define WCD9335_IRQ_HPH_PA_OCPL_FAULT 2 > +#define WCD9335_IRQ_HPH_PA_OCPR_FAULT 3 > +#define WCD9335_IRQ_EAR_PA_OCP_FAULT 4 > +#define WCD9335_IRQ_HPH_PA_CNPL_COMPLETE 5 > +#define WCD9335_IRQ_HPH_PA_CNPR_COMPLETE 6 > +#define WCD9335_IRQ_EAR_PA_CNP_COMPLETE 7 > +#define WCD9335_IRQ_MBHC_SW_DET 8 > +#define WCD9335_IRQ_MBHC_ELECT_INS_REM_DET 9 > +#define WCD9335_IRQ_MBHC_BUTTON_PRESS_DET 10 > +#define WCD9335_IRQ_MBHC_BUTTON_RELEASE_DET 11 > +#define WCD9335_IRQ_MBHC_ELECT_INS_REM_LEG_DET 12 > +#define WCD9335_IRQ_RESERVED_0 13 > +#define WCD9335_IRQ_RESERVED_1 14 > +#define WCD9335_IRQ_RESERVED_2 15 > +#define WCD9335_IRQ_LINE_PA1_CNP_COMPLETE 16 > +#define WCD9335_IRQ_LINE_PA2_CNP_COMPLETE 17 > +#define WCD9335_IRQ_LINE_PA3_CNP_COMPLETE 18 > +#define WCD9335_IRQ_LINE_PA4_CNP_COMPLETE 19 > +#define WCD9335_IRQ_SOUNDWIRE 20 > +#define WCD9335_IRQ_VDD_DIG_RAMP_COMPLETE 21 > +#define WCD9335_IRQ_RCO_ERROR 22 > +#define WCD9335_IRQ_SVA_ERROR 23 > +#define WCD9335_IRQ_MAD_AUDIO 24 > +#define WCD9335_IRQ_MAD_BEACON 25 > +#define WCD9335_IRQ_MAD_ULTRASOUND 26 > +#define WCD9335_IRQ_VBAT_ATTACK 27 > +#define WCD9335_IRQ_VBAT_RESTORE 28 > +#define WCD9335_IRQ_SVA_OUTBOX1 29 > +#define WCD9335_IRQ_SVA_OUTBOX2 30 > + > +#endif /* _DT_BINDINGS_MFD_WCD9335_H */ > diff --git a/include/linux/mfd/wcd9335/wcd9335.h b/include/linux/mfd/wcd9335/wcd9335.h > index b9d2f7af243a..1479bfe75f23 100644 > --- a/include/linux/mfd/wcd9335/wcd9335.h > +++ b/include/linux/mfd/wcd9335/wcd9335.h > @@ -39,4 +39,7 @@ struct wcd9335 { > struct regulator_bulk_data supplies[WCD9335_MAX_SUPPLY]; > }; > > +extern int wcd9335_irq_init(struct wcd9335 *wcd); > +extern int wcd9335_irq_exit(struct wcd9335 *wcd); > + > #endif /* __WCD9335_H__ */ -- Lee Jones [李琼斯] Linaro Services Technical Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog