Re: [PATCH v3] USB: PHY: Palmas USB Transceiver Driver

[Felipe Balbi <balbi@ti.com>]

[-- Attachment #1: Type: text/plain, Size: 2298 bytes --]

Hi,

On Tue, Mar 26, 2013 at 09:01:42AM +0000, Graeme Gregory wrote:
> >>> From: Graeme Gregory <gg@slimlogic.co.uk>
> >>>
> >>> 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 <gg@slimlogic.co.uk>
> >>> Signed-off-by: Moiz Sonasath <m-sonasath@ti.com>
> >>> Signed-off-by: Ruchika Kharwar <ruchika@ti.com>
> >>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> >>> Signed-off-by: Sebastien Guiriec <s-guiriec@ti.com>
> >>> ---
> >>
> >> 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 <linux/of.h>
> >> +#include <linux/of_platform.h>
> >> +
> >> +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.

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.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]