Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932826Ab3CZJBs (ORCPT ); Tue, 26 Mar 2013 05:01:48 -0400 Received: from slimlogic.co.uk ([89.16.172.20]:37989 "EHLO slimlogic.co.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1759145Ab3CZJBp (ORCPT ); Tue, 26 Mar 2013 05:01:45 -0400 Message-ID: <515163F6.3010400@slimlogic.co.uk> Date: Tue, 26 Mar 2013 09:01:42 +0000 From: Graeme Gregory User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130308 Thunderbird/17.0.4 MIME-Version: 1.0 To: Kishon Vijay Abraham I CC: Laxman Dewangan , "balbi@ti.com" , 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 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> In-Reply-To: <51513A40.7080905@ti.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3859 Lines: 111 On 26/03/13 06:03, Kishon Vijay Abraham I wrote: > Hi, > > On Monday 25 March 2013 03:16 PM, Laxman Dewangan wrote: >> On Monday 25 March 2013 03:02 PM, Kishon Vijay Abraham I 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 = PALMAS_BASE_TO_SLAVE(PALMAS_USB_OTG_BASE); >> + addr = 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. >> >>> +} >>> + >>> +static int palmas_usb_write(struct palmas *palmas, unsigned int reg, >>> + unsigned int data) >>> +{ >>> + unsigned int addr; >>> + int slave; >>> + >>> + slave = PALMAS_BASE_TO_SLAVE(PALMAS_USB_OTG_BASE); >>> + addr = PALMAS_BASE_TO_REG(PALMAS_USB_OTG_BASE, reg); >>> + >>> + return regmap_write(palmas->regmap[slave], addr, data); >> >> Same as above. >> >> >> >>> + >>> + if (status != OMAP_DWC3_UNKNOWN) { >>> + palmas_usb->linkstat = status; >>> + palmas_usb->mailboxstat = dwc3_omap_mailbox(status); >> Omap specific call, why? This is generic palma driver. > > hmm.. I think we should either fall-back to the notification mechanism > or have the client drivers pass function pointer here. Felipe? >> >> >>> + >> >>> + palmas_usb->dev = &pdev->dev; >>> + >>> + palmas_usb->irq1 = regmap_irq_get_virq(palmas->irq_data, >>> + PALMAS_ID_OTG_IRQ); >>> + palmas_usb->irq2 = regmap_irq_get_virq(palmas->irq_data, >>> + PALMAS_ID_IRQ); >>> + palmas_usb->irq3 = regmap_irq_get_virq(palmas->irq_data, >>> + PALMAS_VBUS_OTG_IRQ); >>> + palmas_usb->irq4 = regmap_irq_get_virq(palmas->irq_data, >>> + PALMAS_VBUS_IRQ); >> >> Should be come from platform_get_irq() through platform driver. > > No. It can be obtained from regmap too. > > Thanks > Kishon -- 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/