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

[Laxman Dewangan]

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 <gg@xxxxxxxxxxxxxxx>
> > > > >
> > > > > 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@xxxxxxxxxxxxxxx>
> > > > > Signed-off-by: Moiz Sonasath <m-sonasath@xxxxxx>
> > > > > Signed-off-by: Ruchika Kharwar <ruchika@xxxxxx>
> > > > > Signed-off-by: Kishon Vijay Abraham I <kishon@xxxxxx>
> > > > > Signed-off-by: Sebastien Guiriec <s-guiriec@xxxxxx>
> > > > > ---
> > > > 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.
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.

slave = PALMAS_BASE_TO_SLAVE(PALMAS_USB_OTG_BASE);
addr = PALMAS_BASE_TO_REG(PALMAS_USB_OTG_BASE, reg);

regmap_read(palmas->regmap[slave], addr, dest);

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).

This function implement the above 3 lines.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/