All of lore.kernel.org
 help / color / mirror / Atom feed
From: Charles Keepax <ckeepax@opensource.cirrus.com>
To: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
Cc: Mark Brown <broonie@kernel.org>,
	Geert Uytterhoeven <geert@linux-m68k.org>,
	mazziesaccount@gmail.com, heikki.haikola@fi.rohmeurope.com,
	mikko.mutanen@fi.rohmeurope.com,
	Greg KH <gregkh@linuxfoundation.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	Linus Walleij <linus.walleij@linaro.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	"open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
	Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>,
	Linux-Renesas <linux-renesas-soc@vger.kernel.org>
Subject: Re: [PATCH v3] regmap: regmap-irq/gpio-max77620: add level-irq support
Date: Thu, 3 Jan 2019 17:20:08 +0000	[thread overview]
Message-ID: <20190103172008.GO16508@imbe.wolfsonmicro.main> (raw)
In-Reply-To: <20190102074251.GA8865@localhost.localdomain>

On Wed, Jan 02, 2019 at 09:42:51AM +0200, Matti Vaittinen wrote:
> On Mon, Dec 31, 2018 at 07:11:27PM +0000, Mark Brown wrote:
> > On Fri, Dec 28, 2018 at 10:05:33AM +0200, Matti Vaittinen wrote:
> > 
> > > Last night - just when I was about to get some sleep - it stroke me. I
> > > think the correct thing to do would be leaving the irq_set_type to NULL
> > > for those IRQ chips which do not support type setting. If we do that,
> > > then the irq core will take care of situations where user requests type
> > > setting but the chip does not support it. Which means the regmap-irq
> > > would be no different from any other irq chip where type setting is not
> > > supported.
> > 
> > Yes, this is the best fix - let the framework handle things properly.
> > We'll need a second set of operations and to select which to use based
> > on having type information but that's fine.
> > 
> > > So at the cost of removing "const" from regmap_irq_chip we could do:
> > 
> > ...
> > 
> > > Mark, Geert, what do you think? (And maybe same for the .irq_set_wake -
> > > but I did omit this as I have never looked at the wake functionality
> > > before).
> > 
> > We need a separate struct as otherwise if there's multiple devices with
> > regmap irq_chip implementations then they'll collide with each other
> 
> Right. I must admit I didn't notice this! I was about to make a nasty
> error there...
> 

Looking at the code I think it just copies the struct anyway,
basically using it as a template so I think this should be fine.

> > you're probably right that we need to do the same thing for the wake
> > configuration.  I'll still look at applying your patch as a temporary
> > fix though.
> 
> Thanks Mark. I try to cook a patch with copying of struct irq_chip still
> at this week but I wont rush it (I have some other topics under work) as
> the regression should be fixed by the other patch.
> 

Just to check that is this patch here:

https://lore.kernel.org/lkml/20181227084443.GA23991@localhost.localdomain/

Just want to check what will be applied so I know it will fix the
regression I am seeing as well.

Thanks,
Charles

WARNING: multiple messages have this Message-ID (diff)
From: Charles Keepax <ckeepax@opensource.cirrus.com>
To: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
Cc: Mark Brown <broonie@kernel.org>,
	Geert Uytterhoeven <geert@linux-m68k.org>,
	<mazziesaccount@gmail.com>, <heikki.haikola@fi.rohmeurope.com>,
	<mikko.mutanen@fi.rohmeurope.com>,
	Greg KH <gregkh@linuxfoundation.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	Linus Walleij <linus.walleij@linaro.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	"open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
	Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>,
	Linux-Renesas <linux-renesas-soc@vger.kernel.org>
Subject: Re: [PATCH v3] regmap: regmap-irq/gpio-max77620: add level-irq support
Date: Thu, 3 Jan 2019 17:20:08 +0000	[thread overview]
Message-ID: <20190103172008.GO16508@imbe.wolfsonmicro.main> (raw)
In-Reply-To: <20190102074251.GA8865@localhost.localdomain>

On Wed, Jan 02, 2019 at 09:42:51AM +0200, Matti Vaittinen wrote:
> On Mon, Dec 31, 2018 at 07:11:27PM +0000, Mark Brown wrote:
> > On Fri, Dec 28, 2018 at 10:05:33AM +0200, Matti Vaittinen wrote:
> > 
> > > Last night - just when I was about to get some sleep - it stroke me. I
> > > think the correct thing to do would be leaving the irq_set_type to NULL
> > > for those IRQ chips which do not support type setting. If we do that,
> > > then the irq core will take care of situations where user requests type
> > > setting but the chip does not support it. Which means the regmap-irq
> > > would be no different from any other irq chip where type setting is not
> > > supported.
> > 
> > Yes, this is the best fix - let the framework handle things properly.
> > We'll need a second set of operations and to select which to use based
> > on having type information but that's fine.
> > 
> > > So at the cost of removing "const" from regmap_irq_chip we could do:
> > 
> > ...
> > 
> > > Mark, Geert, what do you think? (And maybe same for the .irq_set_wake -
> > > but I did omit this as I have never looked at the wake functionality
> > > before).
> > 
> > We need a separate struct as otherwise if there's multiple devices with
> > regmap irq_chip implementations then they'll collide with each other
> 
> Right. I must admit I didn't notice this! I was about to make a nasty
> error there...
> 

Looking at the code I think it just copies the struct anyway,
basically using it as a template so I think this should be fine.

> > you're probably right that we need to do the same thing for the wake
> > configuration.  I'll still look at applying your patch as a temporary
> > fix though.
> 
> Thanks Mark. I try to cook a patch with copying of struct irq_chip still
> at this week but I wont rush it (I have some other topics under work) as
> the regression should be fixed by the other patch.
> 

Just to check that is this patch here:

https://lore.kernel.org/lkml/20181227084443.GA23991@localhost.localdomain/

Just want to check what will be applied so I know it will fix the
regression I am seeing as well.

Thanks,
Charles

  reply	other threads:[~2019-01-03 17:20 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-18 11:59 [PATCH v3] regmap: regmap-irq/gpio-max77620: add level-irq support Matti Vaittinen
2018-12-18 15:36 ` kbuild test robot
2018-12-18 15:36   ` kbuild test robot
2018-12-19  6:48   ` Matti Vaittinen
2018-12-19 17:52     ` Mark Brown
2018-12-26 11:39 ` Geert Uytterhoeven
2018-12-27  7:35   ` Matti Vaittinen
2018-12-27  7:56     ` Matti Vaittinen
2018-12-28  8:05       ` Matti Vaittinen
2018-12-31 19:11         ` Mark Brown
2019-01-02  7:42           ` Matti Vaittinen
2019-01-03 17:20             ` Charles Keepax [this message]
2019-01-03 17:20               ` Charles Keepax
2019-01-04  8:02               ` Matti Vaittinen

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=20190103172008.GO16508@imbe.wolfsonmicro.main \
    --to=ckeepax@opensource.cirrus.com \
    --cc=broonie@kernel.org \
    --cc=geert@linux-m68k.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=heikki.haikola@fi.rohmeurope.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=matti.vaittinen@fi.rohmeurope.com \
    --cc=mazziesaccount@gmail.com \
    --cc=mikko.mutanen@fi.rohmeurope.com \
    --cc=rafael@kernel.org \
    --cc=vladimir_zapolskiy@mentor.com \
    /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.