From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1764157AbcLUXG5 (ORCPT ); Wed, 21 Dec 2016 18:06:57 -0500 Received: from botnar.kaiser.cx ([176.28.20.183]:43750 "EHLO botnar.kaiser.cx" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1761107AbcLUXGv (ORCPT ); Wed, 21 Dec 2016 18:06:51 -0500 Date: Thu, 22 Dec 2016 00:06:38 +0100 From: Martin Kaiser To: Lucas Stach Cc: Alexandre Belloni , Shawn Guo , Sascha Hauer , Juergen Borleis , Fabio Estevam , linux-kernel@vger.kernel.org, rtc-linux@googlegroups.com Subject: Re: [PATCH v2] rtc: imxdi: use the security violation interrupt Message-ID: <20161221230638.GA12514@reykholt.kaiser.cx> References: <1479560614-19293-1-git-send-email-martin@kaiser.cx> <1482187289-19272-1-git-send-email-martin@kaiser.cx> <1482225821.2293.43.camel@pengutronix.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1482225821.2293.43.camel@pengutronix.de> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hello Lucas, thanks for taking the time to review my patch. Thus wrote Lucas Stach (l.stach@pengutronix.de): > > diff --git a/drivers/rtc/rtc-imxdi.c b/drivers/rtc/rtc-imxdi.c > > index 67b56b8..ec6077a0 100644 > > --- a/drivers/rtc/rtc-imxdi.c > > +++ b/drivers/rtc/rtc-imxdi.c > > @@ -109,6 +109,7 @@ > > * @rtc: pointer to rtc struct > > * @ioaddr: IO registers pointer > > * @irq: dryice normal interrupt > > + * @sec_irq: dryice security violation interrupt > This isn't used outside the probe routine, so doesn't need to be saved > in the driver data. The same goes for imxdi->irq, I made this a local variable as well. > > + rc = devm_request_irq(&pdev->dev, imxdi->sec_irq, dryice_sec_irq, > > + IRQF_SHARED, pdev->name, imxdi); > > + if (rc) { > > + dev_warn(&pdev->dev, "security violation interrupt not available.\n"); > > + /* this is not an error, see above */ > > + } > > + > Please just fold this into the "if (rc > 0)" path above. Are you sure that this would be correct? My understanding is that we should mask the interrupts by writing 0 into DIER and set the DryIce to a sane state in di_handle_state() before we install interrupt handlers. Best regards, Martin From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from botnar.kaiser.cx (botnar.kaiser.cx. [176.28.20.183]) by gmr-mx.google.com with ESMTPS id 75si7054wme.3.2016.12.21.15.07.53 for (version=TLS1 cipher=AES128-SHA bits=128/128); Wed, 21 Dec 2016 15:07:53 -0800 (PST) Date: Thu, 22 Dec 2016 00:06:38 +0100 From: Martin Kaiser To: Lucas Stach Cc: Alexandre Belloni , Shawn Guo , Sascha Hauer , Juergen Borleis , Fabio Estevam , linux-kernel@vger.kernel.org, rtc-linux@googlegroups.com Subject: [rtc-linux] Re: [PATCH v2] rtc: imxdi: use the security violation interrupt Message-ID: <20161221230638.GA12514@reykholt.kaiser.cx> References: <1479560614-19293-1-git-send-email-martin@kaiser.cx> <1482187289-19272-1-git-send-email-martin@kaiser.cx> <1482225821.2293.43.camel@pengutronix.de> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 In-Reply-To: <1482225821.2293.43.camel@pengutronix.de> Sender: "Martin Kaiser,,," Reply-To: rtc-linux@googlegroups.com List-ID: List-Post: , List-Help: , List-Archive: , List-Unsubscribe: , Hello Lucas, thanks for taking the time to review my patch. Thus wrote Lucas Stach (l.stach@pengutronix.de): > > diff --git a/drivers/rtc/rtc-imxdi.c b/drivers/rtc/rtc-imxdi.c > > index 67b56b8..ec6077a0 100644 > > --- a/drivers/rtc/rtc-imxdi.c > > +++ b/drivers/rtc/rtc-imxdi.c > > @@ -109,6 +109,7 @@ > > * @rtc: pointer to rtc struct > > * @ioaddr: IO registers pointer > > * @irq: dryice normal interrupt > > + * @sec_irq: dryice security violation interrupt > This isn't used outside the probe routine, so doesn't need to be saved > in the driver data. The same goes for imxdi->irq, I made this a local variable as well. > > + rc = devm_request_irq(&pdev->dev, imxdi->sec_irq, dryice_sec_irq, > > + IRQF_SHARED, pdev->name, imxdi); > > + if (rc) { > > + dev_warn(&pdev->dev, "security violation interrupt not available.\n"); > > + /* this is not an error, see above */ > > + } > > + > Please just fold this into the "if (rc > 0)" path above. Are you sure that this would be correct? My understanding is that we should mask the interrupts by writing 0 into DIER and set the DryIce to a sane state in di_handle_state() before we install interrupt handlers. Best regards, Martin -- You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. --- You received this message because you are subscribed to the Google Groups "rtc-linux" group. To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout.