[U-Boot] [PATCH] usb: lpc32xx: Add i2c DM support

[Marek Vasut <marex@denx.de>]

On 04/19/2017 02:24 PM, Sylvain Lemieux wrote:
> On Fri, 2017-04-14 at 16:01 +0200, Marek Vasut wrote:
>> On 03/14/2017 04:28 PM, Sylvain Lemieux wrote:
>>> From: Liam Beguin <lbeguin@tycoint.com>
>>>
>>> Add DM support for i2c functions.
>>>
>>> Signed-off-by: Liam Beguin <lbeguin@tycoint.com>
>>> Signed-off-by: Sylvain Lemieux <slemieux@tycoint.com>
>>> ---
>>> Note:
>>> * This patch is require as part of the DM support of the LPC32xx I2C driver.
>>>   All I2C drivers should be converted, to DM, by the end of June 2017.
>>>
>>> * The USB driver is supporting the DM and non-DM API.
>>>
>>> * This patch depend on the following patchset:
>>>   https://lists.denx.de/pipermail/u-boot/2017-March/283672.html
>>>
>>>  drivers/usb/host/ohci-lpc32xx.c | 33 +++++++++++++++++++++++++++++++--
>>>  1 file changed, 31 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/usb/host/ohci-lpc32xx.c b/drivers/usb/host/ohci-lpc32xx.c
>>> index 9245126ed6..51be4c90c7 100644
>>> --- a/drivers/usb/host/ohci-lpc32xx.c
>>> +++ b/drivers/usb/host/ohci-lpc32xx.c
>>> @@ -14,8 +14,10 @@
>>>  #include <asm/io.h>
>>>  #include <asm/arch/cpu.h>
>>>  #include <asm/arch/clk.h>
>>> +#include <asm/arch/i2c.h>
>>>  #include <usb.h>
>>>  #include <i2c.h>
>>> +#include <dm.h>
>>>  
>>>  /* OTG I2C controller module register structures */
>>>  struct otgi2c_regs {
>>> @@ -83,12 +85,35 @@ static struct clk_pm_regs *clk_pwr = (struct clk_pm_regs *)CLK_PM_BASE;
>>>  
>>>  static int isp1301_set_value(int reg, u8 value)
>>>  {
>>> +#ifndef CONFIG_DM_I2C
>>>  	return i2c_write(ISP1301_I2C_ADDR, reg, 1, &value, 1);
>>> +#else
>>> +	struct udevice *dev;
>>> +	int ret;
>>> +
>>> +	ret = i2c_get_chip_for_busnum(I2C_2, ISP1301_I2C_ADDR,
>>> +				      1, &dev);
>>
>> I'd rather see you passing udevice around than requesting it on each
>> single i2c write.
>>
> Will do the change and submit a version 2.
> 
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	return dm_i2c_write(dev, reg, &value, 1);
>>> +#endif
>>>  }
>>>  
>>> -static void isp1301_configure(void)
>>> +static int isp1301_configure(void)
>>>  {
>>> +#ifndef CONFIG_DM_I2C
>>>  	i2c_set_bus_num(I2C_2);
>>> +#else
>>> +	int ret;
>>> +	struct udevice *bus;
>>> +
>>> +	ret = uclass_get_device_by_seq(UCLASS_I2C, I2C_2, &bus);
>>> +	if (ret) {
>>> +		debug("%s: No bus %d\n", __func__, I2C_2);
>>> +		return ret;
>>> +	}
>>> +#endif
>>>  
>>>  	/*
>>>  	 * LPC32XX only supports DAT_SE0 USB mode
>>> @@ -116,6 +141,8 @@ static void isp1301_configure(void)
>>>  
>>>  	/* Enable usb_need_clk clock after transceiver is initialized */
>>>  	setbits_le32(&clk_pwr->usb_ctrl, CLK_USBCTRL_USBDVND_EN);
>>> +
>>> +	return 0;
>>>  }
>>>  
>>>  static int usbpll_setup(void)
>>> @@ -167,7 +194,9 @@ int usb_cpu_init(void)
>>>  		return ret;
>>>  
>>>  	/* Configure ISP1301 */
>>> -	isp1301_configure();
>>> +	ret = isp1301_configure();
>>> +	if (ret)
>>> +		return ret;
>>
>> Don't we need some sort of failpath now, to undo the changes ?
>>
> With the rework of the patch, this change is no longer needed.
> 

OK, super

-- 
Best regards,
Marek Vasut