| Message ID | 20170314152829.29887-1-slemieux.tyco@gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Marek Vasut |
| Headers | show |
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. > + 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 ? > /* setup USB clocks and PLL */ > ret = usbpll_setup(); >
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. > > /* setup USB clocks and PLL */ > > ret = usbpll_setup(); > > > > Sylvain
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
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); + 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; /* setup USB clocks and PLL */ ret = usbpll_setup();