Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 12 May 2023 16:02:24 +0000
From:   Charles Keepax <ckeepax@opensource.cirrus.com>
To:     Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
CC:     <broonie@kernel.org>, <lee@kernel.org>, <robh+dt@kernel.org>,
        <krzysztof.kozlowski+dt@linaro.org>, <conor+dt@kernel.org>,
        <tglx@linutronix.de>, <maz@kernel.org>, <linus.walleij@linaro.org>,
        <vkoul@kernel.org>, <lgirdwood@gmail.com>,
        <yung-chuan.liao@linux.intel.com>, <sanyog.r.kale@intel.com>,
        <alsa-devel@alsa-project.org>, <patches@opensource.cirrus.com>,
        <devicetree@vger.kernel.org>, <linux-gpio@vger.kernel.org>,
        <linux-spi@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 01/10] soundwire: bus: Allow SoundWire peripherals to
 register IRQ handlers
Message-ID: <20230512160224.GK68926@ediswmail.ad.cirrus.com>
References: <20230512122838.243002-1-ckeepax@opensource.cirrus.com>
 <20230512122838.243002-2-ckeepax@opensource.cirrus.com>
 <0471f085-14bf-c159-9b92-62983af6c19a@linux.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <0471f085-14bf-c159-9b92-62983af6c19a@linux.intel.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
Precedence: bulk

On Fri, May 12, 2023 at 08:45:51AM -0500, Pierre-Louis Bossart wrote:
> > @@ -1711,6 +1739,9 @@ static int sdw_handle_slave_alerts(struct sdw_slave *slave)
> >  				struct device *dev = &slave->dev;
> >  				struct sdw_driver *drv = drv_to_sdw_driver(dev->driver);
> >  
> > +				if (slave->prop.use_domain_irq && slave->irq)
> > +					handle_nested_irq(slave->irq);
> > +
> 
> I am a bit lost here, I can understand that alerts would be handled by a
> dedicated handler, but here the code continues and will call the
> existing interrupt_callback.
> 
> Is this intentional? I wonder if there's a risk with two entities
> dealing with the same event and programming the same registers.
> Shouldn't there be some sort of 'either or' rule?
> 

I guess there is a risk of them "handling" the IRQ twice,
although it is hard to see why you would write the driver that
way. Also since they are sequencial the second would I guess
just see that no IRQs are pending.

The intention for calling both is that it facilitates using
the same IRQ handler for I2C and SoundWire. At least on the
Cirrus devices there are a bunch of chip specific registers
that need treated exactly the same on I2C and SoundWire, but
then a couple of extra registers that need handled in the
SoundWire case. This way the handling of those can be kept
completely in the SoundWire part of the code and not ifdef-ed
into the main IRQ path.

Thanks,
Charles