Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Message-ID: <69903c69a95902c0ddc8fb9e7a6762abf28aa034.camel@pengutronix.de>
Subject: Re: [PATCH v3 2/4] drm/imx: Add initial support for DCSS on iMX8MQ
From:   Philipp Zabel <p.zabel@pengutronix.de>
To:     Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>,
        Lucas Stach <l.stach@pengutronix.de>
Cc:     Laurentiu Palcu <laurentiu.palcu@nxp.com>,
        Shawn Guo <shawnguo@kernel.org>,
        Sascha Hauer <s.hauer@pengutronix.de>,
        Pengutronix Kernel Team <kernel@pengutronix.de>,
        Fabio Estevam <festevam@gmail.com>,
        NXP Linux Team <linux-imx@nxp.com>,
        linux-arm-kernel@lists.infradead.org, agx@sigxcpu.org,
        lukas@mntmn.com, linux-kernel@vger.kernel.org,
        dri-devel@lists.freedesktop.org
Date:   Fri, 06 Mar 2020 11:20:51 +0100
In-Reply-To: <20200306095830.sa5eig67phngr3fa@fsr-ub1864-141>
References: <1575625964-27102-1-git-send-email-laurentiu.palcu@nxp.com>
         <1575625964-27102-3-git-send-email-laurentiu.palcu@nxp.com>
         <03b551925d079fcc151239afa735562332cfd557.camel@pengutronix.de>
         <20200306095830.sa5eig67phngr3fa@fsr-ub1864-141>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8BIT
User-Agent: Evolution 3.30.5-1.1 
MIME-Version: 1.0
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Laurentiu,

On Fri, 2020-03-06 at 11:58 +0200, Laurentiu Palcu wrote:
> On Wed, Feb 26, 2020 at 02:19:11PM +0100, Lucas Stach wrote:
[...]
> > > +/* This function will be called from interrupt context. */
> > > +void dcss_scaler_write_sclctrl(struct dcss_scaler *scl)
> > > +{
> > > +	int chnum;
> > > +
> > > +	for (chnum = 0; chnum < 3; chnum++) {
> > > +		struct dcss_scaler_ch *ch = &scl->ch[chnum];
> > > +
> > > +		if (ch->scaler_ctrl_chgd) {
> > > +			dcss_ctxld_write_irqsafe(scl->ctxld, scl->ctx_id,
> > > +						 ch->scaler_ctrl,
> > > +						 ch->base_ofs +
> > > +						 DCSS_SCALER_CTRL);
> > 
> > Why is this using the _irqsafe variant without any locking? Won't this
> > lead to potential internal state corruption? dcss_ctxld_write is using
> > the _irqsave locking variants, so it fine with being called from IRQ
> > context.
> 
> This is only called from __dcss_ctxld_enable() which is already protected
> by lock/unlock in dcss_ctxld_kick().

You could add a lockdep_assert_held() line to the top of this function
to make it clear this depends on the lock being held.

regards
Philipp