Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932733Ab3CZMHw (ORCPT ); Tue, 26 Mar 2013 08:07:52 -0400 Received: from bear.ext.ti.com ([192.94.94.41]:60579 "EHLO bear.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932148Ab3CZMHt (ORCPT ); Tue, 26 Mar 2013 08:07:49 -0400 Date: Tue, 26 Mar 2013 14:07:08 +0200 From: Felipe Balbi To: Laxman Dewangan CC: "balbi@ti.com" , Graeme Gregory , Kishon Vijay Abraham I , Rajendra Nayak , "grant.likely@secretlab.ca" , "rob.herring@calxeda.com" , "rob@landley.net" , "gregkh@linuxfoundation.org" , "s-guiriec@ti.com" , "sameo@linux.intel.com" , "broonie@opensource.wolfsonmicro.com" , "devicetree-discuss@lists.ozlabs.org" , "linux-doc@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "linux-usb@vger.kernel.org" Subject: Re: [PATCH v3] USB: PHY: Palmas USB Transceiver Driver Message-ID: <20130326120708.GP11244@arwen.pp.htv.fi> Reply-To: References: <1362662506-14823-4-git-send-email-kishon@ti.com> <1364203926-24488-1-git-send-email-kishon@ti.com> <51501CFB.4020905@nvidia.com> <51513A40.7080905@ti.com> <515163F6.3010400@slimlogic.co.uk> <20130326102100.GD11244@arwen.pp.htv.fi> <51517859.2020407@nvidia.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="ngPZezdD7QsvFaqQ" Content-Disposition: inline In-Reply-To: <51517859.2020407@nvidia.com> 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 Content-Length: 4588 Lines: 118 --ngPZezdD7QsvFaqQ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hi, On Tue, Mar 26, 2013 at 03:58:41PM +0530, Laxman Dewangan wrote: > On Tuesday 26 March 2013 03:51 PM, Felipe Balbi wrote: > >* PGP Signed by an unknown key > > > >Hi, > > > >On Tue, Mar 26, 2013 at 09:01:42AM +0000, Graeme Gregory wrote: > >>>>>From: Graeme Gregory > >>>>> > >>>>>This is the driver for the OTG transceiver built into the Palmas > >>>>>chip. It > >>>>>handles the various USB OTG events that can be generated by cable > >>>>>insertion/removal. > >>>>> > >>>>>Signed-off-by: Graeme Gregory > >>>>>Signed-off-by: Moiz Sonasath > >>>>>Signed-off-by: Ruchika Kharwar > >>>>>Signed-off-by: Kishon Vijay Abraham I > >>>>>Signed-off-by: Sebastien Guiriec > >>>>>--- > >>>>I think this driver is more over the cable connection like vbus > >>>>detetcion or ID pin detection. > >>>>Then why not it is implemented based on extcon framework? > >>>extcon framework uses notification mechanism and Felipe dint like > >>>using notification here. right Felipe? > >>>>That way, generic usb driver (like tegra_usb driver) will get > >>>>notification through extcon. > >>>> > >>>>We need this cable detection through extcon on our tegra solution > >>>>through the Palmas. > >>>> > >>>> > >>>>+#include > >>>>+#include > >>>>+ > >>>>+static int palmas_usb_read(struct palmas *palmas, unsigned int reg, > >>>>+ unsigned int *dest) > >>>>+{ > >>>>+ unsigned int addr; > >>>>+ int slave; > >>>>+ > >>>>+ slave =3D PALMAS_BASE_TO_SLAVE(PALMAS_USB_OTG_BASE); > >>>>+ addr =3D PALMAS_BASE_TO_REG(PALMAS_USB_OTG_BASE, reg); > >>>>+ > >>>>+ return regmap_read(palmas->regmap[slave], addr, dest); > >>>> > >>>> > >>>>Please use the generic api for palmas_read()/palmas_write(0 as it will > >>>>be ease on debugging on register access. > >>>>Direct regmap_read() does not help much on this. > >>>Graeme, > >>>Any reason why you dint use palmas_read()/palmas_write here? > >>>Btw palmas_read()/palmas_write() internally uses regmap APIs. > >>Because I was not a fan of tightly coupling the child devices to the > >>parent MFD. palmas_read/write were added by Laxman. > >I guess regmap would also help abstracting SPI versus I2C connection. > >IMHO, palmas_read/write should be removed. > > > >Laxman's complaint that it doesn't help with debugging is utter > >nonsense. > palams read/write api uses the regmap only and hence not break > anything on abstraction. > in place of doing the following three statement in whole word, it > provides wrapper of palmas_read() > which actually does the same. >=20 > slave =3D PALMAS_BASE_TO_SLAVE(PALMAS_USB_OTG_BASE); > addr =3D PALMAS_BASE_TO_REG(PALMAS_USB_OTG_BASE, reg); >=20 > regmap_read(palmas->regmap[slave], addr, dest); >=20 > Above 3 lines in all the places for resgister access or make single call: > palmas_read(palmas, PALMAS_USB_OTG_BASE, PALMAS_USB_OTG_BASE, dest). >=20 > This function implement the above 3 lines. now you have explained what the problem is. Makes much more sense to use palmas_read() indeed. Duplicating the same thing all over the place isn't very nice. --=20 balbi --ngPZezdD7QsvFaqQ Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (GNU/Linux) iQIcBAEBAgAGBQJRUY9sAAoJEIaOsuA1yqREpR0P/2+DgajcHW6mRpafChn/eC74 MX2chFolDKZZ/73YWFJb0xc8sMVHeGPV36k5w818OL3kY4KOCwpxQ0ITAvKAp4hG d0t2kUvEj89CsOUOT3Wkunoxqc0ZLutWHQzGSLvNTPKF7N9L2P1eEuFPSy8cbN51 sC1CGysKFTQJOMLvf3wbWIRyxcaCpwGiTID0bY0P8e015d0rm2uq9yXhp1bs4u9b 0Kb3oGiwlNSuY3790A5+doa/8eLB9kqUEsqkIyMjnqXUzFdx8HMhmqa16+Fap0OR nQuyXJQ9UyOcJY7b0WEYNYQV5OAgMp3zRvJJ6UB1ut61m7vCnyV8RmcfSnY5dX9A +dQOG+9HrAJ8+csbYSKONtGeW5aWA7P8XTn6T/FQZXOObGNM05HurkxnVT1AhMaN GXomJQ6ICKbKPBkZywQXZpSIfCkWR4V8762eNpzZPhyhsVxftwjD9t20X/ns90Xs n2MAsIDQ4Cukt3lKyv4jM7DEvFuWOsEMM4qVlpviBgWFGcI8rGSv+zaQ3ONPzIyf hrDgjmGwfDfUd0LcAzts0oEPZVfugErDJoCY/QNQMBz4u7LojM3tksFM6RbpsleP YhUU3dW5N5EBvjb9ZN/gvAe+kzo7nHc+1DJJ4aGU4ifp7VXJ8ZQA5M5ZTgYRYkJy G5sKcZSLP9mHllphA6FP =Px6Q -----END PGP SIGNATURE----- --ngPZezdD7QsvFaqQ-- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/