From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Message-ID: <57D7B0F1.6030607@parrot.com> Date: Tue, 13 Sep 2016 09:55:29 +0200 From: Gregor Boirie MIME-Version: 1.0 To: Jonathan Cameron , CC: , Hartmut Knaack , Lars-Peter Clausen , Peter Meerwald-Stadler , Rob Herring , Mark Rutland , Akinobu Mita , Linus Walleij , Vlad Dogaru , Ludovic Tancerel , Marek Vasut , Crestez Dan Leonard Subject: Re: [PATCH v4 1/1] iio:pressure: initial zpa2326 barometer support References: <68c225245eb4f4972f5acaa56bdbf9d25971f0f7.1473432897.git.gregor.boirie@parrot.com> In-Reply-To: Content-Type: text/plain; charset="windows-1252"; format=flowed List-ID: Hi, On 09/10/2016 05:37 PM, Jonathan Cameron wrote: > On 09/09/16 16:01, Gregor Boirie wrote: >> Introduce driver for Murata ZPA2326 pressure and temperature sensor: >> http://www.murata.com/en-us/products/productdetail?partno=ZPA2326-0311A-R >> >> Signed-off-by: Gregor Boirie > This looks pretty good to me now. A few minor bits and bobs inline. > > I almost took it as is, but would like to have it sit on the list > in it's current form for a few more days anyway so might as well get > these few corners tidied up! [...] > > +/* > + * Debugfs stuff > + * > + * We want to prevent debugfs hooks from altering configuration while chip > + * sampling is ongoing to preserve data consistency. Moreover, a few registers, > + * when read, change hardware state, e.g. interrupt acknowlege on read. This is > + * why debugfs read and write hooks acquire exclusive access to register space. > + * > + * Also note this is mostly unusable with power management enabled systems since > + * registers content is lost during power off -> on transitions. > + */ > It would be nice perhaps to tighten up the range of addresses that > we are protecting. Thus allow reading of any addresses that are not > 'fragile'. Only supported limited debug access is hardly a big > issue however ;) Quite frankly, I wonder if this debugfs stuff should not be thrown away given that regmap already provides a way to access registers over debugfs (with very limited protection...). What do you think ? [...] Regards, Grégor. From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gregor Boirie Subject: Re: [PATCH v4 1/1] iio:pressure: initial zpa2326 barometer support Date: Tue, 13 Sep 2016 09:55:29 +0200 Message-ID: <57D7B0F1.6030607@parrot.com> References: <68c225245eb4f4972f5acaa56bdbf9d25971f0f7.1473432897.git.gregor.boirie@parrot.com> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 8bit Return-path: In-Reply-To: Sender: linux-iio-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Jonathan Cameron , linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Hartmut Knaack , Lars-Peter Clausen , Peter Meerwald-Stadler , Rob Herring , Mark Rutland , Akinobu Mita , Linus Walleij , Vlad Dogaru , Ludovic Tancerel , Marek Vasut , Crestez Dan Leonard List-Id: devicetree@vger.kernel.org Hi, On 09/10/2016 05:37 PM, Jonathan Cameron wrote: > On 09/09/16 16:01, Gregor Boirie wrote: >> Introduce driver for Murata ZPA2326 pressure and temperature sensor: >> http://www.murata.com/en-us/products/productdetail?partno=ZPA2326-0311A-R >> >> Signed-off-by: Gregor Boirie > This looks pretty good to me now. A few minor bits and bobs inline. > > I almost took it as is, but would like to have it sit on the list > in it's current form for a few more days anyway so might as well get > these few corners tidied up! [...] > > +/* > + * Debugfs stuff > + * > + * We want to prevent debugfs hooks from altering configuration while chip > + * sampling is ongoing to preserve data consistency. Moreover, a few registers, > + * when read, change hardware state, e.g. interrupt acknowlege on read. This is > + * why debugfs read and write hooks acquire exclusive access to register space. > + * > + * Also note this is mostly unusable with power management enabled systems since > + * registers content is lost during power off -> on transitions. > + */ > It would be nice perhaps to tighten up the range of addresses that > we are protecting. Thus allow reading of any addresses that are not > 'fragile'. Only supported limited debug access is hardly a big > issue however ;) Quite frankly, I wonder if this debugfs stuff should not be thrown away given that regmap already provides a way to access registers over debugfs (with very limited protection...). What do you think ? [...] Regards, Grégor.