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

[Laxman Dewangan <ldewangan@nvidia.com>]

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