From: Lee Jones <lee.jones@linaro.org>
To: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
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
Date: Thu, 2 Aug 2018 09:05:25 +0100 [thread overview]
Message-ID: <20180802080525.GM4662@dell> (raw)
In-Reply-To: <20180727121806.18209-4-srinivas.kandagatla@linaro.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 <srinivas.kandagatla@linaro.org>
> ---
> 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 <linux/gpio.h>
> +#include <linux/interrupt.h>
> +#include <linux/regmap.h>
> +#include <linux/of_irq.h>
> +#include <dt-bindings/mfd/wcd9335.h>
> +#include <linux/mfd/wcd9335/wcd9335.h>
> +#include <linux/mfd/wcd9335/registers.h>
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
next prev parent reply other threads:[~2018-08-02 8:05 UTC|newest]
Thread overview: 57+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-07-27 12:17 [PATCH v2 00/10] ASoC: Add support to WCD9335 Audio Codec Srinivas Kandagatla
2018-07-27 12:17 ` Srinivas Kandagatla
2018-07-27 12:17 ` [PATCH v2 01/10] ASoC: dt-bindings: add dt bindings for wcd9335 audio codec Srinivas Kandagatla
2018-07-31 20:43 ` Rob Herring
2018-07-31 20:43 ` Rob Herring
2018-08-01 8:56 ` Srinivas Kandagatla
2018-08-01 22:20 ` Rob Herring
2018-08-01 22:20 ` Rob Herring
2018-08-02 7:33 ` Srinivas Kandagatla
2018-08-02 14:07 ` Rob Herring
2018-08-01 11:20 ` Applied "ASoC: dt-bindings: add dt bindings for wcd9335 audio codec" to the asoc tree Mark Brown
2018-08-01 11:20 ` Mark Brown
2018-08-14 16:24 ` Rob Herring
2018-08-15 10:52 ` Mark Brown
2018-08-15 10:52 ` Mark Brown
2018-07-27 12:17 ` [PATCH v2 02/10] mfd: wcd9335: add support to wcd9335 core Srinivas Kandagatla
2018-07-30 4:12 ` Vinod
2018-08-01 8:57 ` Srinivas Kandagatla
2018-08-02 8:33 ` Lee Jones
2018-08-02 9:26 ` Srinivas Kandagatla
2018-08-02 9:57 ` Lee Jones
2018-08-02 9:57 ` Lee Jones
2018-07-27 12:17 ` [PATCH v2 03/10] mfd: wcd9335: add wcd irq support Srinivas Kandagatla
2018-07-31 20:45 ` Rob Herring
2018-08-01 8:57 ` Srinivas Kandagatla
2018-08-01 22:17 ` Rob Herring
2018-08-02 7:34 ` Srinivas Kandagatla
2018-08-02 8:05 ` Lee Jones [this message]
2018-08-02 8:54 ` Srinivas Kandagatla
2018-08-02 10:01 ` Lee Jones
2018-08-02 10:07 ` Srinivas Kandagatla
2018-07-27 12:18 ` [PATCH v2 04/10] ASoC: wcd9335: add support to wcd9335 codec Srinivas Kandagatla
2018-07-27 12:18 ` Srinivas Kandagatla
2018-08-01 11:20 ` Applied "ASoC: wcd9335: add support to wcd9335 codec" to the asoc tree Mark Brown
2018-08-01 11:20 ` Mark Brown
2018-08-01 12:24 ` Srinivas Kandagatla
2018-08-02 6:32 ` Lee Jones
2018-08-02 6:32 ` Lee Jones
2018-07-27 12:18 ` [PATCH v2 05/10] ASoC: wcd9335: add CLASS-H Controller support Srinivas Kandagatla
2018-07-31 17:05 ` Mark Brown
2018-07-31 17:05 ` Mark Brown
2018-08-01 8:57 ` Srinivas Kandagatla
2018-08-01 11:20 ` Applied "ASoC: wcd9335: add CLASS-H Controller support" to the asoc tree Mark Brown
2018-08-01 11:20 ` Mark Brown
2018-08-01 12:26 ` Srinivas Kandagatla
2018-08-01 12:26 ` Srinivas Kandagatla
2018-08-02 6:31 ` Lee Jones
2018-08-02 9:52 ` Mark Brown
2018-07-27 12:18 ` [PATCH v2 06/10] ASoC: wcd9335: add basic controls Srinivas Kandagatla
2018-07-31 17:16 ` Mark Brown
2018-08-01 8:57 ` Srinivas Kandagatla
2018-07-27 12:18 ` [PATCH v2 07/10] ASoC: wcd9335: add playback dapm widgets Srinivas Kandagatla
2018-07-27 12:18 ` [PATCH v2 08/10] ASoC: wcd9335: add capture " Srinivas Kandagatla
2018-07-27 12:18 ` [PATCH v2 09/10] ASoC: wcd9335: add audio routings Srinivas Kandagatla
2018-07-27 12:18 ` [PATCH v2 10/10] ASoC: apq8096: add support to Analog audio via WCD9335 slim Srinivas Kandagatla
2018-07-30 4:49 ` [PATCH v2 00/10] ASoC: Add support to WCD9335 Audio Codec Vinod
2018-07-30 4:49 ` Vinod
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180802080525.GM4662@dell \
--to=lee.jones@linaro.org \
--cc=alsa-devel@alsa-project.org \
--cc=bgoswami@codeaurora.org \
--cc=broonie@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=lgirdwood@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=robh+dt@kernel.org \
--cc=srinivas.kandagatla@linaro.org \
--cc=tiwai@suse.com \
--cc=vkoul@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.