From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dmitry Torokhov Subject: Re: [PATCH v2 1/3] input: keyboad: imx: add snvs power key driver Date: Wed, 13 May 2015 13:14:34 -0700 Message-ID: <20150513201434.GE11891@dtor-ws> References: <1431528462-21591-1-git-send-email-Frank.Li@freescale.com> <20150513164020.GA11891@dtor-ws> <2698983.zamV244jo6@jclayton-pc> <20150513194454.GD11891@dtor-ws> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Received: from mail-ie0-f179.google.com ([209.85.223.179]:32998 "EHLO mail-ie0-f179.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752555AbbEMUOj (ORCPT ); Wed, 13 May 2015 16:14:39 -0400 Received: by iebgx4 with SMTP id gx4so42613848ieb.0 for ; Wed, 13 May 2015 13:14:38 -0700 (PDT) Content-Disposition: inline In-Reply-To: Sender: linux-input-owner@vger.kernel.org List-Id: linux-input@vger.kernel.org To: Zhi Li Cc: Joshua Clayton , "linux-arm-kernel@lists.infradead.org" , Robin Gong , Shawn Guo , "Frank.Li@freescale.com" , linux-input@vger.kernel.org On Wed, May 13, 2015 at 02:57:00PM -0500, Zhi Li wrote: > On Wed, May 13, 2015 at 2:44 PM, Dmitry Torokhov > wrote: > > On Wed, May 13, 2015 at 02:27:46PM -0500, Zhi Li wrote: > >> On Wed, May 13, 2015 at 2:10 PM, Joshua Clayton > >> wrote: > >> > > >> > Hi Frank. > >> > > >> > On Wednesday, May 13, 2015 01:27:39 PM Zhi Li wrote: > >> >> On Wed, May 13, 2015 at 11:40 AM, Dmitry Torokhov > >> >> > >> >> wrote: > >> >> > Hi Frank, > >> >> > > >> > > >> >> > > >> >> >> + pdata->ioaddr = of_iomap(np, 0); > >> >> >> + if (IS_ERR(pdata->ioaddr)) > >> >> >> + return PTR_ERR(pdata->ioaddr); > >> >> > > >> >> > Umm, you are still leaking it on error path. Can you try doing: > >> >> > res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > >> >> > pdata->ioaddr = devm_ioremap_resource(&pdev->dev, res); > >> >> > if (IS_ERR(pdata->ioaddr)) > >> >> > > >> >> > return PTR_ERR(pdata->ioaddr); > >> >> > >> >> Thank you for comments. > >> >> but I don't use of_iomap here because the resource is shared with the > >> >> other device. > >> >> > >> >> SNVS included a rtc, a power off, ON/OFF key. > >> >> If I use platform_get_resource and devm_ioremap_resource, rtc driver > >> >> fail to probe. > >> >> > >> >> best regards > >> >> Frank Li > >> >> > >> > > >> > The fact that you cannot use of_iomap() seems a clear warning of unsafe > >> > access. > >> > Since it is a shared resource, which you are writing to as well as reading, > >> > perhaps you should set it up for safe access using the regmap API like the > >> > GPR registers. > >> > /include/linux/mfd/syscon/imx6q-iomuxc-gpr.h > >> > >> I read driver again clearfully. It is safed shared with rtc driver directly. > >> Because pwr key driver just read a irq status register and other one > >> register, which used only in this driver. > >> > >> Just w1c (write 1 clear) register. it is atomic access by hardware. > > > > It does not really matter, driver has to claim resources it is using to > > make sure other parties are not accessing its registers. If registers > > are shared you need to have a 3rd party mediator. > > I can change to use syscon. > but other two driver(rtc and power off) need change also. Makes sense. > > > > >> > >> PowerKey use difference irq number with RTC part. > > > > Yes and so they can run simultaneously and step on each other's feet, > > right? > > Yes. > > > > > Thanks. > > > > -- > > Dmitry -- Dmitry