* [PATCH REBASE 1/2] i2c: tegra: I2_M_NOSTART functionality not supported in Tegra20 @ 2012-08-17 19:02 Laxman Dewangan [not found] ` <1345230155-4252-1-git-send-email-ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> 0 siblings, 1 reply; 7+ messages in thread From: Laxman Dewangan @ 2012-08-17 19:02 UTC (permalink / raw) To: khali-PUYAD+kWke1g9hUCZPvPmw, w.sang-bIcnvbaLZ9MEGnE8C9+IrQ, swarren-DDmLM1+adcrQT0dZR+AlfA Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-tegra-u79uwXL29TY76Z2rM5mHXA, olof-nZhT3qVonbNeoWH0uzbU5w, Laxman Dewangan Tegra20 i2c controller does not support the continue transfer which implements the I2C_M_NOSTART functionality of i2c protocol mangling. Removing the I2C_M_NOSTART functionality for Tegra20. Signed-off-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> --- This is rebased on linux-next-20120816. drivers/i2c/busses/i2c-tegra.c | 73 +++++++++++++++++++++++++++++++-------- 1 files changed, 58 insertions(+), 15 deletions(-) diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c index 7149625..554f713 100644 --- a/drivers/i2c/busses/i2c-tegra.c +++ b/drivers/i2c/busses/i2c-tegra.c @@ -27,6 +27,7 @@ #include <linux/slab.h> #include <linux/i2c-tegra.h> #include <linux/of_i2c.h> +#include <linux/of_device.h> #include <linux/module.h> #include <asm/unaligned.h> @@ -114,6 +115,15 @@ enum msg_end_type { }; /** + * struct tegra_i2c_hw_feature : Different HW support on Tegra + * @has_continue_xfer_support: Continue transfer supports. + */ + +struct tegra_i2c_hw_feature { + bool has_continue_xfer_support; +}; + +/** * struct tegra_i2c_dev - per device i2c context * @dev: device reference for power management * @adapter: core i2c layer adapter information @@ -148,6 +158,7 @@ struct tegra_i2c_dev { int msg_read; unsigned long bus_clk_rate; bool is_suspended; + bool has_continue_xfer_support; }; static void dvc_writel(struct tegra_i2c_dev *i2c_dev, u32 val, unsigned long reg) @@ -563,7 +574,17 @@ static int tegra_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], if (i2c_dev->is_suspended) return -EBUSY; - clk_prepare_enable(i2c_dev->div_clk); + /* Support I2C_M_NOSTART only if HW support continue xfer. */ + for (i = 0; i < num - 1; i++) { + if ((msgs[i + 1].flags & I2C_M_NOSTART) && + !i2c_dev->has_continue_xfer_support) { + dev_err(i2c_dev->dev, + "mesg %d have illegal flag\n", i + 1); + return -EINVAL; + } + } + + clk_prepare_enable(i2c_dev->clk); for (i = 0; i < num; i++) { enum msg_end_type end_type = MSG_END_STOP; if (i < (num - 1)) { @@ -582,8 +603,13 @@ static int tegra_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], static u32 tegra_i2c_func(struct i2c_adapter *adap) { - return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_10BIT_ADDR | - I2C_FUNC_PROTOCOL_MANGLING | I2C_FUNC_NOSTART; + struct tegra_i2c_dev *i2c_dev = i2c_get_adapdata(adap); + u32 ret = I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_10BIT_ADDR | + I2C_FUNC_PROTOCOL_MANGLING; + + if (i2c_dev->has_continue_xfer_support) + ret |= I2C_FUNC_NOSTART; + return ret; } static const struct i2c_algorithm tegra_i2c_algo = { @@ -591,6 +617,25 @@ static const struct i2c_algorithm tegra_i2c_algo = { .functionality = tegra_i2c_func, }; +static struct tegra_i2c_hw_feature tegra20_i2c_hw = { + .has_continue_xfer_support = false, +}; + +static struct tegra_i2c_hw_feature tegra30_i2c_hw = { + .has_continue_xfer_support = true, +}; + +#if defined(CONFIG_OF) +/* Match table for of_platform binding */ +static const struct of_device_id tegra_i2c_of_match[] __devinitconst = { + { .compatible = "nvidia,tegra30-i2c", .data = &tegra30_i2c_hw, }, + { .compatible = "nvidia,tegra20-i2c", .data = &tegra20_i2c_hw, }, + { .compatible = "nvidia,tegra20-i2c-dvc", .data = &tegra20_i2c_hw, }, + {}, +}; +MODULE_DEVICE_TABLE(of, tegra_i2c_of_match); +#endif + static int __devinit tegra_i2c_probe(struct platform_device *pdev) { struct tegra_i2c_dev *i2c_dev; @@ -600,6 +645,7 @@ static int __devinit tegra_i2c_probe(struct platform_device *pdev) struct clk *fast_clk; const unsigned int *prop; void __iomem *base; + const struct tegra_i2c_hw_feature *hw = &tegra20_i2c_hw; int irq; int ret = 0; @@ -659,11 +705,18 @@ static int __devinit tegra_i2c_probe(struct platform_device *pdev) i2c_dev->bus_clk_rate = be32_to_cpup(prop); } - if (pdev->dev.of_node) + if (pdev->dev.of_node) { + const struct of_device_id *match; + match = of_match_device(of_match_ptr(tegra_i2c_of_match), + &pdev->dev); + hw = match->data; i2c_dev->is_dvc = of_device_is_compatible(pdev->dev.of_node, "nvidia,tegra20-i2c-dvc"); - else if (pdev->id == 3) + } else if (pdev->id == 3) { i2c_dev->is_dvc = 1; + } + + i2c_dev->has_continue_xfer_support = hw->has_continue_xfer_support; init_completion(&i2c_dev->msg_complete); platform_set_drvdata(pdev, i2c_dev); @@ -751,16 +804,6 @@ static SIMPLE_DEV_PM_OPS(tegra_i2c_pm, tegra_i2c_suspend, tegra_i2c_resume); #define TEGRA_I2C_PM NULL #endif -#if defined(CONFIG_OF) -/* Match table for of_platform binding */ -static const struct of_device_id tegra_i2c_of_match[] __devinitconst = { - { .compatible = "nvidia,tegra20-i2c", }, - { .compatible = "nvidia,tegra20-i2c-dvc", }, - {}, -}; -MODULE_DEVICE_TABLE(of, tegra_i2c_of_match); -#endif - static struct platform_driver tegra_i2c_driver = { .probe = tegra_i2c_probe, .remove = __devexit_p(tegra_i2c_remove), -- 1.7.1.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
[parent not found: <1345230155-4252-1-git-send-email-ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>]
* [PATCH REBASE 2/2] i2c: tegra: dynamically control fast clk [not found] ` <1345230155-4252-1-git-send-email-ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> @ 2012-08-17 19:02 ` Laxman Dewangan [not found] ` <1345230155-4252-2-git-send-email-ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> 2012-08-17 21:35 ` [PATCH REBASE 1/2] i2c: tegra: I2_M_NOSTART functionality not supported in Tegra20 Stephen Warren 2012-08-18 12:47 ` Wolfram Sang 2 siblings, 1 reply; 7+ messages in thread From: Laxman Dewangan @ 2012-08-17 19:02 UTC (permalink / raw) To: khali-PUYAD+kWke1g9hUCZPvPmw, w.sang-bIcnvbaLZ9MEGnE8C9+IrQ, swarren-DDmLM1+adcrQT0dZR+AlfA Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-tegra-u79uwXL29TY76Z2rM5mHXA, olof-nZhT3qVonbNeoWH0uzbU5w, Laxman Dewangan Tegra I2C driver enables the fast clock during initialization and does not disable till driver removed. Enable this clock before transfer and disable after transfer done. Signed-off-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> --- This is rebased on linux-next-20120816. drivers/i2c/busses/i2c-tegra.c | 35 ++++++++++++++++++++++++++++------- 1 files changed, 28 insertions(+), 7 deletions(-) diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c index 554f713..24f7cee 100644 --- a/drivers/i2c/busses/i2c-tegra.c +++ b/drivers/i2c/busses/i2c-tegra.c @@ -362,12 +362,36 @@ static void tegra_dvc_init(struct tegra_i2c_dev *i2c_dev) dvc_writel(i2c_dev, val, DVC_CTRL_REG1); } +static inline int tegra_i2c_clock_enable(struct tegra_i2c_dev *i2c_dev) +{ + int ret; + ret = clk_prepare_enable(i2c_dev->fast_clk); + if (ret < 0) { + dev_err(i2c_dev->dev, + "Enabling fast clk failed, err %d\n", ret); + return ret; + } + ret = clk_prepare_enable(i2c_dev->div_clk); + if (ret < 0) { + dev_err(i2c_dev->dev, + "Enabling div clk failed, err %d\n", ret); + clk_disable_unprepare(i2c_dev->fast_clk); + } + return ret; +} + +static inline void tegra_i2c_clock_disable(struct tegra_i2c_dev *i2c_dev) +{ + clk_disable_unprepare(i2c_dev->div_clk); + clk_disable_unprepare(i2c_dev->fast_clk); +} + static int tegra_i2c_init(struct tegra_i2c_dev *i2c_dev) { u32 val; int err = 0; - clk_prepare_enable(i2c_dev->div_clk); + tegra_i2c_clock_enable(i2c_dev); tegra_periph_reset_assert(i2c_dev->div_clk); udelay(2); @@ -398,7 +422,7 @@ static int tegra_i2c_init(struct tegra_i2c_dev *i2c_dev) if (tegra_i2c_flush_fifos(i2c_dev)) err = -ETIMEDOUT; - clk_disable_unprepare(i2c_dev->div_clk); + tegra_i2c_clock_disable(i2c_dev); if (i2c_dev->irq_disabled) { i2c_dev->irq_disabled = 0; @@ -584,7 +608,7 @@ static int tegra_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], } } - clk_prepare_enable(i2c_dev->clk); + tegra_i2c_clock_enable(i2c_dev); for (i = 0; i < num; i++) { enum msg_end_type end_type = MSG_END_STOP; if (i < (num - 1)) { @@ -597,7 +621,7 @@ static int tegra_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], if (ret) break; } - clk_disable_unprepare(i2c_dev->div_clk); + tegra_i2c_clock_disable(i2c_dev); return ret ?: i; } @@ -734,8 +758,6 @@ static int __devinit tegra_i2c_probe(struct platform_device *pdev) return ret; } - clk_prepare_enable(i2c_dev->fast_clk); - i2c_set_adapdata(&i2c_dev->adapter, i2c_dev); i2c_dev->adapter.owner = THIS_MODULE; i2c_dev->adapter.class = I2C_CLASS_HWMON; @@ -749,7 +771,6 @@ static int __devinit tegra_i2c_probe(struct platform_device *pdev) ret = i2c_add_numbered_adapter(&i2c_dev->adapter); if (ret) { dev_err(&pdev->dev, "Failed to add I2C adapter\n"); - clk_disable_unprepare(i2c_dev->fast_clk); return ret; } -- 1.7.1.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
[parent not found: <1345230155-4252-2-git-send-email-ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>]
* Re: [PATCH REBASE 2/2] i2c: tegra: dynamically control fast clk [not found] ` <1345230155-4252-2-git-send-email-ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> @ 2012-08-18 18:52 ` Wolfram Sang 0 siblings, 0 replies; 7+ messages in thread From: Wolfram Sang @ 2012-08-18 18:52 UTC (permalink / raw) To: Laxman Dewangan Cc: khali-PUYAD+kWke1g9hUCZPvPmw, swarren-DDmLM1+adcrQT0dZR+AlfA, linux-i2c-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-tegra-u79uwXL29TY76Z2rM5mHXA, olof-nZhT3qVonbNeoWH0uzbU5w [-- Attachment #1: Type: text/plain, Size: 1161 bytes --] On Sat, Aug 18, 2012 at 12:32:35AM +0530, Laxman Dewangan wrote: > Tegra I2C driver enables the fast clock during initialization > and does not disable till driver removed. > Enable this clock before transfer and disable after transfer done. > > Signed-off-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> Except that this patch is affected from the flaw of the previous patch, it looks good to me. > - clk_prepare_enable(i2c_dev->clk); > + tegra_i2c_clock_enable(i2c_dev); This should be div_clk. > for (i = 0; i < num; i++) { > enum msg_end_type end_type = MSG_END_STOP; > if (i < (num - 1)) { > @@ -597,7 +621,7 @@ static int tegra_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], > if (ret) > break; > } > - clk_disable_unprepare(i2c_dev->div_clk); > + tegra_i2c_clock_disable(i2c_dev); > return ret ?: i; If that is fixed, you can add Reviewed-by: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> -- Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ | [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 198 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH REBASE 1/2] i2c: tegra: I2_M_NOSTART functionality not supported in Tegra20 [not found] ` <1345230155-4252-1-git-send-email-ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> 2012-08-17 19:02 ` [PATCH REBASE 2/2] i2c: tegra: dynamically control fast clk Laxman Dewangan @ 2012-08-17 21:35 ` Stephen Warren 2012-08-18 12:47 ` Wolfram Sang 2 siblings, 0 replies; 7+ messages in thread From: Stephen Warren @ 2012-08-17 21:35 UTC (permalink / raw) To: Laxman Dewangan, w.sang-bIcnvbaLZ9MEGnE8C9+IrQ Cc: khali-PUYAD+kWke1g9hUCZPvPmw, swarren-DDmLM1+adcrQT0dZR+AlfA, linux-i2c-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-tegra-u79uwXL29TY76Z2rM5mHXA, olof-nZhT3qVonbNeoWH0uzbU5w On 08/17/2012 01:02 PM, Laxman Dewangan wrote: > Tegra20 i2c controller does not support the continue transfer > which implements the I2C_M_NOSTART functionality of i2c > protocol mangling. > Removing the I2C_M_NOSTART functionality for Tegra20. > > Signed-off-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> I tested both these patches and they work fine. I'll hold off applying them for a while for Wolfram to ack them. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH REBASE 1/2] i2c: tegra: I2_M_NOSTART functionality not supported in Tegra20 [not found] ` <1345230155-4252-1-git-send-email-ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> 2012-08-17 19:02 ` [PATCH REBASE 2/2] i2c: tegra: dynamically control fast clk Laxman Dewangan 2012-08-17 21:35 ` [PATCH REBASE 1/2] i2c: tegra: I2_M_NOSTART functionality not supported in Tegra20 Stephen Warren @ 2012-08-18 12:47 ` Wolfram Sang [not found] ` <20120818124703.GC12839-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> 2 siblings, 1 reply; 7+ messages in thread From: Wolfram Sang @ 2012-08-18 12:47 UTC (permalink / raw) To: Laxman Dewangan Cc: khali-PUYAD+kWke1g9hUCZPvPmw, swarren-DDmLM1+adcrQT0dZR+AlfA, linux-i2c-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-tegra-u79uwXL29TY76Z2rM5mHXA, olof-nZhT3qVonbNeoWH0uzbU5w [-- Attachment #1: Type: text/plain, Size: 6057 bytes --] On Sat, Aug 18, 2012 at 12:32:34AM +0530, Laxman Dewangan wrote: > Tegra20 i2c controller does not support the continue transfer > which implements the I2C_M_NOSTART functionality of i2c > protocol mangling. > Removing the I2C_M_NOSTART functionality for Tegra20. > > Signed-off-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> > --- > This is rebased on linux-next-20120816. > > drivers/i2c/busses/i2c-tegra.c | 73 +++++++++++++++++++++++++++++++-------- > 1 files changed, 58 insertions(+), 15 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c > index 7149625..554f713 100644 > --- a/drivers/i2c/busses/i2c-tegra.c > +++ b/drivers/i2c/busses/i2c-tegra.c > @@ -27,6 +27,7 @@ > #include <linux/slab.h> > #include <linux/i2c-tegra.h> > #include <linux/of_i2c.h> > +#include <linux/of_device.h> > #include <linux/module.h> > > #include <asm/unaligned.h> > @@ -114,6 +115,15 @@ enum msg_end_type { > }; > > /** > + * struct tegra_i2c_hw_feature : Different HW support on Tegra > + * @has_continue_xfer_support: Continue transfer supports. > + */ > + > +struct tegra_i2c_hw_feature { > + bool has_continue_xfer_support; > +}; > + > +/** > * struct tegra_i2c_dev - per device i2c context > * @dev: device reference for power management > * @adapter: core i2c layer adapter information > @@ -148,6 +158,7 @@ struct tegra_i2c_dev { > int msg_read; > unsigned long bus_clk_rate; > bool is_suspended; > + bool has_continue_xfer_support; I wonder if it makes sense to carry a pointer here to the tegra_i2c_hw_feature in use instead of copying all entries by hand, since they might get more and more. > }; > > static void dvc_writel(struct tegra_i2c_dev *i2c_dev, u32 val, unsigned long reg) > @@ -563,7 +574,17 @@ static int tegra_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], > if (i2c_dev->is_suspended) > return -EBUSY; > > - clk_prepare_enable(i2c_dev->div_clk); > + /* Support I2C_M_NOSTART only if HW support continue xfer. */ > + for (i = 0; i < num - 1; i++) { > + if ((msgs[i + 1].flags & I2C_M_NOSTART) && > + !i2c_dev->has_continue_xfer_support) { > + dev_err(i2c_dev->dev, > + "mesg %d have illegal flag\n", i + 1); > + return -EINVAL; > + } > + } Drivers are requested to explicitly check for features of the I2C bus (like M_NOSTART) before using them, so I'd skip this extra check. > + > + clk_prepare_enable(i2c_dev->clk); From a glimpse, this change looks unrelated at least. Even wrong, no? > for (i = 0; i < num; i++) { > enum msg_end_type end_type = MSG_END_STOP; > if (i < (num - 1)) { > @@ -582,8 +603,13 @@ static int tegra_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], > > static u32 tegra_i2c_func(struct i2c_adapter *adap) > { > - return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_10BIT_ADDR | > - I2C_FUNC_PROTOCOL_MANGLING | I2C_FUNC_NOSTART; > + struct tegra_i2c_dev *i2c_dev = i2c_get_adapdata(adap); > + u32 ret = I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_10BIT_ADDR | > + I2C_FUNC_PROTOCOL_MANGLING; > + > + if (i2c_dev->has_continue_xfer_support) > + ret |= I2C_FUNC_NOSTART; > + return ret; > } > > static const struct i2c_algorithm tegra_i2c_algo = { > @@ -591,6 +617,25 @@ static const struct i2c_algorithm tegra_i2c_algo = { > .functionality = tegra_i2c_func, > }; > > +static struct tegra_i2c_hw_feature tegra20_i2c_hw = { > + .has_continue_xfer_support = false, > +}; > + > +static struct tegra_i2c_hw_feature tegra30_i2c_hw = { > + .has_continue_xfer_support = true, > +}; > + > +#if defined(CONFIG_OF) > +/* Match table for of_platform binding */ > +static const struct of_device_id tegra_i2c_of_match[] __devinitconst = { > + { .compatible = "nvidia,tegra30-i2c", .data = &tegra30_i2c_hw, }, > + { .compatible = "nvidia,tegra20-i2c", .data = &tegra20_i2c_hw, }, > + { .compatible = "nvidia,tegra20-i2c-dvc", .data = &tegra20_i2c_hw, }, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, tegra_i2c_of_match); > +#endif > + > static int __devinit tegra_i2c_probe(struct platform_device *pdev) > { > struct tegra_i2c_dev *i2c_dev; > @@ -600,6 +645,7 @@ static int __devinit tegra_i2c_probe(struct platform_device *pdev) > struct clk *fast_clk; > const unsigned int *prop; > void __iomem *base; > + const struct tegra_i2c_hw_feature *hw = &tegra20_i2c_hw; > int irq; > int ret = 0; > > @@ -659,11 +705,18 @@ static int __devinit tegra_i2c_probe(struct platform_device *pdev) > i2c_dev->bus_clk_rate = be32_to_cpup(prop); > } > > - if (pdev->dev.of_node) > + if (pdev->dev.of_node) { > + const struct of_device_id *match; > + match = of_match_device(of_match_ptr(tegra_i2c_of_match), > + &pdev->dev); > + hw = match->data; > i2c_dev->is_dvc = of_device_is_compatible(pdev->dev.of_node, > "nvidia,tegra20-i2c-dvc"); > - else if (pdev->id == 3) > + } else if (pdev->id == 3) { > i2c_dev->is_dvc = 1; > + } > + > + i2c_dev->has_continue_xfer_support = hw->has_continue_xfer_support; > init_completion(&i2c_dev->msg_complete); > > platform_set_drvdata(pdev, i2c_dev); > @@ -751,16 +804,6 @@ static SIMPLE_DEV_PM_OPS(tegra_i2c_pm, tegra_i2c_suspend, tegra_i2c_resume); > #define TEGRA_I2C_PM NULL > #endif > > -#if defined(CONFIG_OF) > -/* Match table for of_platform binding */ > -static const struct of_device_id tegra_i2c_of_match[] __devinitconst = { > - { .compatible = "nvidia,tegra20-i2c", }, > - { .compatible = "nvidia,tegra20-i2c-dvc", }, > - {}, > -}; > -MODULE_DEVICE_TABLE(of, tegra_i2c_of_match); > -#endif > - > static struct platform_driver tegra_i2c_driver = { > .probe = tegra_i2c_probe, > .remove = __devexit_p(tegra_i2c_remove), Thanks, Wolfram -- Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ | [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 198 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
[parent not found: <20120818124703.GC12839-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>]
* Re: [PATCH REBASE 1/2] i2c: tegra: I2_M_NOSTART functionality not supported in Tegra20 [not found] ` <20120818124703.GC12839-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> @ 2012-08-18 12:50 ` Laxman Dewangan [not found] ` <502F8F83.5030902-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> 0 siblings, 1 reply; 7+ messages in thread From: Laxman Dewangan @ 2012-08-18 12:50 UTC (permalink / raw) To: Wolfram Sang Cc: khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org, Stephen Warren, linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org Thanks for review. On Saturday 18 August 2012 06:17 PM, Wolfram Sang wrote: > * PGP Signed by an unknown key > > On Sat, Aug 18, 2012 at 12:32:34AM +0530, Laxman Dewangan wrote: >> + bool has_continue_xfer_support; > I wonder if it makes sense to carry a pointer here to the > tegra_i2c_hw_feature in use instead of copying all entries by hand, > since they might get more and more. > Ok, we can store the hw pointer in device structure. I will send the new patch. >> }; >> >> static void dvc_writel(struct tegra_i2c_dev *i2c_dev, u32 val, unsigned long reg) >> @@ -563,7 +574,17 @@ static int tegra_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], >> if (i2c_dev->is_suspended) >> return -EBUSY; >> >> - clk_prepare_enable(i2c_dev->div_clk); >> + /* Support I2C_M_NOSTART only if HW support continue xfer. */ >> + for (i = 0; i< num - 1; i++) { >> + if ((msgs[i + 1].flags& I2C_M_NOSTART)&& >> + !i2c_dev->has_continue_xfer_support) { >> + dev_err(i2c_dev->dev, >> + "mesg %d have illegal flag\n", i + 1); >> + return -EINVAL; >> + } >> + } > Drivers are requested to explicitly check for features of the I2C bus > (like M_NOSTART) before using them, so I'd skip this extra check. > Ok, I kept this as part of flag checking so illegal flag should not be passed. I will remove this on next version patch. >> + >> + clk_prepare_enable(i2c_dev->clk); > From a glimpse, this change looks unrelated at least. Even wrong, no? > It was already there, just before above check. Due to insertion of check, this code shifted, otherwise it is not a new code. ^ permalink raw reply [flat|nested] 7+ messages in thread
[parent not found: <502F8F83.5030902-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>]
* Re: [PATCH REBASE 1/2] i2c: tegra: I2_M_NOSTART functionality not supported in Tegra20 [not found] ` <502F8F83.5030902-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> @ 2012-08-18 18:29 ` Wolfram Sang 0 siblings, 0 replies; 7+ messages in thread From: Wolfram Sang @ 2012-08-18 18:29 UTC (permalink / raw) To: Laxman Dewangan Cc: khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org, Stephen Warren, linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org [-- Attachment #1: Type: text/plain, Size: 1131 bytes --] > >>- clk_prepare_enable(i2c_dev->div_clk); > >>+ /* Support I2C_M_NOSTART only if HW support continue xfer. */ > >>+ for (i = 0; i< num - 1; i++) { > >>+ if ((msgs[i + 1].flags& I2C_M_NOSTART)&& > >>+ !i2c_dev->has_continue_xfer_support) { > >>+ dev_err(i2c_dev->dev, > >>+ "mesg %d have illegal flag\n", i + 1); > >>+ return -EINVAL; > >>+ } > >>+ } > >Drivers are requested to explicitly check for features of the I2C bus > >(like M_NOSTART) before using them, so I'd skip this extra check. > > > > Ok, I kept this as part of flag checking so illegal flag should not > be passed. I will remove this on next version patch. > > >>+ > >>+ clk_prepare_enable(i2c_dev->clk); > > From a glimpse, this change looks unrelated at least. Even wrong, no? > > > > It was already there, just before above check. Due to insertion of > check, this code shifted, otherwise it is not a new code. It used to be ->div_clk and now it is ->clk? -- Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ | [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 198 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2012-08-18 18:52 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-08-17 19:02 [PATCH REBASE 1/2] i2c: tegra: I2_M_NOSTART functionality not supported in Tegra20 Laxman Dewangan [not found] ` <1345230155-4252-1-git-send-email-ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> 2012-08-17 19:02 ` [PATCH REBASE 2/2] i2c: tegra: dynamically control fast clk Laxman Dewangan [not found] ` <1345230155-4252-2-git-send-email-ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> 2012-08-18 18:52 ` Wolfram Sang 2012-08-17 21:35 ` [PATCH REBASE 1/2] i2c: tegra: I2_M_NOSTART functionality not supported in Tegra20 Stephen Warren 2012-08-18 12:47 ` Wolfram Sang [not found] ` <20120818124703.GC12839-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> 2012-08-18 12:50 ` Laxman Dewangan [not found] ` <502F8F83.5030902-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> 2012-08-18 18:29 ` Wolfram Sang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).