On Thu, 02 Aug 2018, Srinivas Kandagatla wrote: > Thanks for the review, > > On 02/08/18 09:05, Lee Jones wrote: > > 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@xxxxxxxxxx> > > > --- > > > drivers/mfd/Makefile | 2 +- > > > drivers/mfd/wcd9335-core.c | 9 ++ > > > drivers/mfd/wcd9335-irq.c | 172 ++++++++++++++++++++++++++++++++++++ > > > > Any particular reason for separating out IRQ handling? > No particular reason, I tried to follow what other codecs like wm8994, > wm8350 and 14 others do. I haven't look at them in a while, but maybe theirs are optional? Either way, I don't think you need to do it. > > > 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? > > > It would not defer here as the regmaps are already setup in probe and the > status callback is only invoked when the SLIMbus device is up. I guess now you have seen my comment below, you know that it can defer, right? > > > +// SPDX-License-Identifier: GPL-2.0 > > > +// Copyright (c) 2018, Linaro Limited > > > +// > > > > Blank line? > > > Yep. Does "yep" mean, "I'll fix it"? -- Lee Jones [李琼斯] Linaro Services Technical Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
