From: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
To: Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org,
khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org,
ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org,
linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
spear-devel-nkJGhpqTU55BDgjK7y7TUQ@public.gmane.org,
paul-YHLC2tV1sDlxR4N9A70vTlRxknfHcPLb9dF7HbQ/qKg@public.gmane.org
Subject: Re: [PATCH V10 1/2] i2c/adapter: Add bus recovery infrastructure
Date: Fri, 25 Jan 2013 12:13:22 +0100 [thread overview]
Message-ID: <20130125111322.GA2649@pengutronix.de> (raw)
In-Reply-To: <59a73d182bf1ce330becf32ad780f3501a57aed0.1359106966.git.viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1877 bytes --]
On Fri, Jan 25, 2013 at 03:17:48PM +0530, Viresh Kumar wrote:
> Add i2c bus recovery infrastructure to i2c adapters as specified in the i2c
> protocol Rev. 03 section 3.1.16 titled "Bus clear".
>
> http://www.nxp.com/documents/user_manual/UM10204.pdf
>
> Sometimes during operation i2c bus hangs and we need to give dummy clocks to
> slave device to start the transfer again. Now we may have capability in the bus
> controller to generate these clocks or platform may have gpio pins which can be
> toggled to generate dummy clocks. This patch supports both.
>
> This patch also adds in generic bus recovery routines gpio or SCL line based
> which can be used by bus controller. In addition controller driver may provide
> its own version of the bus recovery routine.
>
> This doesn't support multi-master recovery for now.
>
> Reviewed-by: Paul Carpenter <paul-YHLC2tV1sDlxR4N9A70vTlRxknfHcPLb9dF7HbQ/qKg@public.gmane.org>
> Signed-off-by: Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Code-wise I am mostly satisfied, except that I need to think about
clock-stretching a little more. We could add that later, too.
However, while I understand you are keen to get rid of this series ASAP,
it seems that this eagerness results in some sloppyness which is a bit
cumbersome (most lines changed since last time have something to comment
on).
So, I'll try a different review style this time and let you do the
detective work :)
1) Please be strict and consistent with spaces around operators.
2) Make sure the warnings and printouts match the code
3) Take the printouts serious and invest a thought if they can be
further improved.
Regards,
Wolfram
--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
next prev parent reply other threads:[~2013-01-25 11:13 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-01-25 9:47 [PATCH V10 1/2] i2c/adapter: Add bus recovery infrastructure Viresh Kumar
[not found] ` <59a73d182bf1ce330becf32ad780f3501a57aed0.1359106966.git.viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2013-01-25 9:47 ` [PATCH V10 2/2] i2c/designware: Provide i2c bus recovery support Viresh Kumar
[not found] ` <f7d8d29a338704deaa7a542f9a213fc7696625c9.1359106966.git.viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2013-04-05 6:42 ` Viresh Kumar
[not found] ` <CAKohpom=DQDv6v7xwnCRvtWBxC+a5mGZeOv=ad8AjKwV2yGBow-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2013-05-13 9:18 ` Viresh Kumar
[not found] ` <CAKohpokAPohg2aAjawLN2t_Jh29CZAuiWqKz2ah-bJP9Dw4YvA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2013-05-31 10:50 ` Viresh Kumar
2013-01-25 9:48 ` [PATCH V10 1/2] i2c/adapter: Add bus recovery infrastructure Viresh Kumar
2013-01-25 11:13 ` Wolfram Sang [this message]
[not found] ` <20130125111322.GA2649-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2013-01-26 5:13 ` Viresh Kumar
[not found] ` <CAKohpok2arxhsAt3xOxRCZZjvLMTed1EYX3ZRUSQ3733wx2mMw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2013-01-31 2:28 ` Viresh Kumar
[not found] ` <CAKohpokNVPOvAc=Gbxn4ZquPnG4huukDmwhOsqaV51p0aRo8xw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2013-03-15 8:51 ` Viresh Kumar
[not found] ` <CAKohpokCqe7LHFMn0t7EivTQ7US9Fp=07nowGP2H2ujeDKbHXg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2013-03-15 9:13 ` Viresh Kumar
[not found] ` <CAKohpondDBBjTQYgV9XOFMYp_aKO+SdeRR1gjxe1ovTLuOm2Wg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2013-03-21 9:36 ` Wolfram Sang
[not found] ` <20130321093648.GA19297-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
2013-03-21 10:15 ` Viresh Kumar
[not found] ` <CAKohpo=NdN64bJ_uD4cZ_aJppz+zschYFMyoP=-2c+NfuOqFfg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2013-03-21 10:34 ` Shiraz Hashim
2013-03-24 14:07 ` Viresh Kumar
[not found] ` <CAKohpokQY=g1eUrwwydA4mMruebt2xZinLPCL5_+mkg4oediEQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2013-04-01 6:45 ` Viresh Kumar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130125111322.GA2649@pengutronix.de \
--to=w.sang-bicnvbalz9megne8c9+irq@public.gmane.org \
--cc=ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org \
--cc=khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org \
--cc=linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=paul-YHLC2tV1sDlxR4N9A70vTlRxknfHcPLb9dF7HbQ/qKg@public.gmane.org \
--cc=spear-devel-nkJGhpqTU55BDgjK7y7TUQ@public.gmane.org \
--cc=u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org \
--cc=viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).