| Message ID | 570ECAB0.4050107@osg.samsung.com |
|---|---|
| State | Deferred |
| Headers | show |
> I'll write what I found so far in case someone with better knowledge about > the runtime PM API and the OMAP I2C controller driver can have an idea of > what could be causing this. Thanks for the summary. I got no other reports like this, I wonder about that. That being said, can you try this patch if it makes a change? http://patchwork.ozlabs.org/patch/609280/
Hello Wolfram, On 04/14/2016 07:12 AM, Wolfram Sang wrote: > >> I'll write what I found so far in case someone with better knowledge about >> the runtime PM API and the OMAP I2C controller driver can have an idea of >> what could be causing this. > > Thanks for the summary. I got no other reports like this, I wonder about Yes, I also wonder why I'm the only one facing this issue... maybe no one else is using the tvp5150 driver on an OMAP board with mainline? > that. That being said, can you try this patch if it makes a change? > > http://patchwork.ozlabs.org/patch/609280/ > Unfortunately it doesn't help, I just tried today's -next (next-20160414) that already has the mentioned patch but the issue is still present. Best regards,
> Yes, I also wonder why I'm the only one facing this issue... maybe no one > else is using the tvp5150 driver on an OMAP board with mainline? I wonder why it only affects tvp5150. I don't see the connection yet.
Hello Wofram, On 04/14/2016 10:19 AM, Wolfram Sang wrote: > >> Yes, I also wonder why I'm the only one facing this issue... maybe no one >> else is using the tvp5150 driver on an OMAP board with mainline? > > I wonder why it only affects tvp5150. I don't see the connection yet. > Yes, me neither. All other I2C devices are working properly on this board. The only thing I can think, is that the tvp5150 needs a reset sequence in order to be operative. It basically toggles two pins in the chip, this is done in tvp5150_init() [0] and is needed before accessing I2C registers. Maybe runtime pm has an effect on this and the chip is not reset correctly? [0]: https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/drivers/media/i2c/tvp5150.c#n1311 Best regards,
Hi, * Javier Martinez Canillas <javier@osg.samsung.com> [160414 07:28]: > Hello Wofram, > > On 04/14/2016 10:19 AM, Wolfram Sang wrote: > > > >> Yes, I also wonder why I'm the only one facing this issue... maybe no one > >> else is using the tvp5150 driver on an OMAP board with mainline? > > > > I wonder why it only affects tvp5150. I don't see the connection yet. > > > > Yes, me neither. All other I2C devices are working properly on this board. > > The only thing I can think, is that the tvp5150 needs a reset sequence in > order to be operative. It basically toggles two pins in the chip, this is > done in tvp5150_init() [0] and is needed before accessing I2C registers. > > Maybe runtime pm has an effect on this and the chip is not reset correctly? Is this with omap3 and does tvp5150 have a reset GPIO pin? If so, you could be hitting the GPIO errata where a glitch can happen when restoring the GPIO state coming back from off mode in idle. This happes for GPIO pins that are not in GPIO bank1 and have an external pull down resistor on the line. The short term workaround is to mux the reset pin to use the internal pulls by using PIN_INPUT_PULLUP | MUX_MODE7, or depending on the direction, PIN_INPUT_PULLDOWN | MUX_MODE7. The long term workaround is tho have gpio-omap.c do this dynamically with pinctrl-single.c using gpio-ranges, but that's going to take a while.. You can search for erratum 1.158 for more info. Regards, Tony > [0]: https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/drivers/media/i2c/tvp5150.c#n1311 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello Tony, On 04/14/2016 11:12 AM, Tony Lindgren wrote: > Hi, > > * Javier Martinez Canillas <javier@osg.samsung.com> [160414 07:28]: >> Hello Wofram, >> >> On 04/14/2016 10:19 AM, Wolfram Sang wrote: >>> >>>> Yes, I also wonder why I'm the only one facing this issue... maybe no one >>>> else is using the tvp5150 driver on an OMAP board with mainline? >>> >>> I wonder why it only affects tvp5150. I don't see the connection yet. >>> >> >> Yes, me neither. All other I2C devices are working properly on this board. >> >> The only thing I can think, is that the tvp5150 needs a reset sequence in >> order to be operative. It basically toggles two pins in the chip, this is >> done in tvp5150_init() [0] and is needed before accessing I2C registers. >> >> Maybe runtime pm has an effect on this and the chip is not reset correctly? > > Is this with omap3 and does tvp5150 have a reset GPIO pin? > Yes, it's a DM3730 (OMAP3) and yes the tvp5150 (actually it's a tvp5151) has a reset pin that has to be toggled, along with a power-down pin for the chip to be in an operative state before accessing the I2C registers. That is the power/reset sequence I mentioned before. > If so, you could be hitting the GPIO errata where a glitch can happen > when restoring the GPIO state coming back from off mode in idle. This > happes for GPIO pins that are not in GPIO bank1 and have an external > pull down resistor on the line. > The GPIO lines connected to these pins are: GPIO126 (bank4 pin 30) -> tvp5150 power-down pin GPIO167 (bank6 pin 7) -> tvp5150 reset pin Neither are in GPIO bank1 so they could be affected by the errata you mention but there isn't external pull down (or up) resistors on these lines AFAICT by looking at the board schematics. I've added to the cc list to other people that are familiar with the board in case I missed something. > The short term workaround is to mux the reset pin to use the internal > pulls by using PIN_INPUT_PULLUP | MUX_MODE7, or depending on the direction, > PIN_INPUT_PULLDOWN | MUX_MODE7. > I guess you meant MUX_MODE4 here since the pin has to be in GPIO mode? Also, I wonder how the issue could be related to the GPIO controller since is when enabling runtime PM for the I2C controller that things fail. IOW, disabling runtime PM for the I2C adapter shouldn't make things to work if the problem was caused by the mentioned GPIO errata. In any case, I've tried to use the internal pulls as you suggested but that didn't solve the issue. > The long term workaround is tho have gpio-omap.c do this dynamically > with pinctrl-single.c using gpio-ranges, but that's going to take a > while.. You can search for erratum 1.158 for more info. > > Regards, > > Tony > >> [0]: https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/drivers/media/i2c/tvp5150.c#n1311 I'm really out of ideas on this, I don't think that Wolfram change has to be reverted since it seems I'm the only one facing this issue and also the DTS for the board I'm using is not even in mainline yet. I'm waiting for some discussions about the Media Controller input connectors DT bindings to settle before posting the board DTS patches. Best regards,
* Javier Martinez Canillas <javier@osg.samsung.com> [160414 17:00]: > On 04/14/2016 11:12 AM, Tony Lindgren wrote: > > Is this with omap3 and does tvp5150 have a reset GPIO pin? > > Yes, it's a DM3730 (OMAP3) and yes the tvp5150 (actually it's a tvp5151) has > a reset pin that has to be toggled, along with a power-down pin for the chip > to be in an operative state before accessing the I2C registers. That is the > power/reset sequence I mentioned before. > > > If so, you could be hitting the GPIO errata where a glitch can happen > > when restoring the GPIO state coming back from off mode in idle. This > > happes for GPIO pins that are not in GPIO bank1 and have an external > > pull down resistor on the line. > > > > The GPIO lines connected to these pins are: > > GPIO126 (bank4 pin 30) -> tvp5150 power-down pin > GPIO167 (bank6 pin 7) -> tvp5150 reset pin > > Neither are in GPIO bank1 so they could be affected by the errata you > mention but there isn't external pull down (or up) resistors on these > lines AFAICT by looking at the board schematics. I've added to the cc > list to other people that are familiar with the board in case I missed > something. > > > The short term workaround is to mux the reset pin to use the internal > > pulls by using PIN_INPUT_PULLUP | MUX_MODE7, or depending on the direction, > > PIN_INPUT_PULLDOWN | MUX_MODE7. > > I guess you meant MUX_MODE4 here since the pin has to be in GPIO mode? No, the glitch affects the GPIO mode, so that's why direction + pull + safe mode is needed. > Also, I wonder how the issue could be related to the GPIO controller > since is when enabling runtime PM for the I2C controller that things > fail. IOW, disabling runtime PM for the I2C adapter shouldn't make > things to work if the problem was caused by the mentioned GPIO errata. If you block PM runtime for I2C, then it blocks deeper idle states for the whole device. Note that you can disable off mode during idle and suspend with: # echo 0 > /sys/kernel/debug/pm_debug/enable_off_mode > In any case, I've tried to use the internal pulls as you suggested but > that didn't solve the issue. OK. Just to be sure.. Did you use the safe mode mux option instead of the GPIO mux option? Note that in some cases the internal pull is not strong enough to keep the reset line up if there's an external pull down resistor. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello Tony, On 04/15/2016 10:58 AM, Tony Lindgren wrote: >> >>> The short term workaround is to mux the reset pin to use the internal >>> pulls by using PIN_INPUT_PULLUP | MUX_MODE7, or depending on the direction, >>> PIN_INPUT_PULLDOWN | MUX_MODE7. >> >> I guess you meant MUX_MODE4 here since the pin has to be in GPIO mode? > > No, the glitch affects the GPIO mode, so that's why direction + pull + > safe mode is needed. > Ah ok, thanks for the explanation and sorry for the confusion. >> Also, I wonder how the issue could be related to the GPIO controller >> since is when enabling runtime PM for the I2C controller that things >> fail. IOW, disabling runtime PM for the I2C adapter shouldn't make >> things to work if the problem was caused by the mentioned GPIO errata. > > If you block PM runtime for I2C, then it blocks deeper idle states > for the whole device. Note that you can disable off mode during idle Thanks again for this clarification. > and suspend with: > > # echo 0 > /sys/kernel/debug/pm_debug/enable_off_mode > I see thought that enable_off_mode is 0 by default when booting the board: # cat /sys/kernel/debug/pm_debug/enable_off_mode 0 So if I understood your explanation correctly, that means that the glitch should not happen for the GPIO pins since the machine doesn't enter into deeper idle states that could cause the glitch from erratum 1.158? >> In any case, I've tried to use the internal pulls as you suggested but >> that didn't solve the issue. > > OK. Just to be sure.. Did you use the safe mode mux option instead > of the GPIO mux option? > No sorry, I tested with the GPIO mux mode before since I misunderstood your previous email. But now I've tested with safe mode mux and didn't make a difference (which was expected since off mode is disabled AFAIU). > Note that in some cases the internal pull is not strong enough to > keep the reset line up if there's an external pull down resistor. > Yes, I got that from your previous email but as I mentioned, the lines in the board don't have external pull resistors. > Regards, > > Tony > Best regards,
* Javier Martinez Canillas <javier@osg.samsung.com> [160415 09:50]: > On 04/15/2016 10:58 AM, Tony Lindgren wrote: > > If you block PM runtime for I2C, then it blocks deeper idle states > > for the whole device. Note that you can disable off mode during idle > > Thanks again for this clarification. > > > and suspend with: > > > > # echo 0 > /sys/kernel/debug/pm_debug/enable_off_mode > > > > I see thought that enable_off_mode is 0 by default when booting the board: > > # cat /sys/kernel/debug/pm_debug/enable_off_mode > 0 OK so you're not hitting off mode then. > So if I understood your explanation correctly, that means that the glitch > should not happen for the GPIO pins since the machine doesn't enter into > deeper idle states that could cause the glitch from erratum 1.158? Correct. But you could still have a dependency to some other device driver that stays active if I2C keeps the whole system from hitting retention mode during idle. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index 37a88f0ae179..0b72b21b379d 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -1445,6 +1445,8 @@ omap_i2c_probe(struct platform_device *pdev) goto err_unuse_clocks; } + pm_runtime_disable(&adap->dev); + dev_info(omap->dev, "bus %d rev%d.%d at %d kHz\n", adap->nr, major, minor, omap->speed);