All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Serge Semin <fancer.lancer@gmail.com>
Cc: Richard Leitner <richard.leitner@skidata.com>,
	Serge Semin <Sergey.Semin@t-platforms.ru>,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [v2] usb: usb251xb: Lock i2c-bus segment the hub resides
Date: Sat, 27 Apr 2019 09:53:23 +0200	[thread overview]
Message-ID: <20190427075323.GA31478@kroah.com> (raw)

On Sat, Apr 27, 2019 at 10:39:41AM +0300, Serge Semin wrote:
> On Sat, Apr 27, 2019 at 09:32:00AM +0200, Greg Kroah-Hartman wrote:
> > On Sat, Apr 27, 2019 at 10:10:27AM +0300, Serge Semin wrote:
> > > On Sat, Apr 27, 2019 at 09:00:42AM +0200, Greg Kroah-Hartman wrote:
> > > > On Fri, Apr 26, 2019 at 01:46:50PM +0300, Serge Semin wrote:
> > > > > SMBus slave configuration is activated by CFG_SEL[1:0]=0x1 pins
> > > > > state. This is the mode the hub is supposed to be to let this driver
> > > > > work correctly. But a race condition might happen right after reset
> > > > > is cleared due to CFG_SEL[0] pin being multiplexed with SMBus SCL
> > > > > function. In case if the reset pin is handled by a i2c GPIO expander,
> > > > > which is also placed at the same i2c-bus segment as the usb251x
> > > > > SMB-interface connected to, then the hub reset clearance might
> > > > > cause the CFG_SEL[0] being latched in unpredictable state. So
> > > > > sometimes the hub configuration mode might be 0x1 (as expected),
> > > > > but sometimes being 0x0, which doesn't imply to have the hub SMBus-slave
> > > > > interface activated and consequently causes this driver failure.
> > > > > 
> > > > > In order to fix the problem we must make sure the GPIO-reset chip doesn't
> > > > > reside the same i2c-bus segment as the SMBus-interface of the hub. If
> > > > > it doesn't, we can safely block the segment for the time the reset is
> > > > > cleared to prevent anyone generating a traffic at the i2c-bus SCL lane
> > > > > connected to the CFG_SEL[0] pin. But if it does, nothing we can do, so
> > > > > just return an error. If we locked the i2c-bus segment and tried to
> > > > > communicate with the GPIO-expander, it would cause a deadlock. If we didn't
> > > > > lock the i2c-bus segment, it would randomly cause the CFG_SEL[0] bit flip.
> > > > > 
> > > > > Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
> > > > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > > 
> > > > I signed off on this?  Where?  When?
> > > > 
> > > > never add a s-o-b line that you did not create, that implies a legal
> > > > agreement.
> > > > 
> > > 
> > > Ah, shit. Sorry. I should have added Acked-by. That's what I was going to do,
> > > since you already added the first version of this into linux-next tree. But
> > > apparently copy-pasted and left as is...
> > 
> > If I have already applied a patch, I can't apply it again (or a
> > different version.)  You need to send a fix-up patch for the reported
> > issue, not a whole new one as I can not go back in time and rewrite
> > history.
> > 
> > thanks,
> > 
> > greg k-h
> 
> Ok. Should I send it in reply to this patch or as a completely separate one?

A separate one.

WARNING: multiple messages have this Message-ID (diff)
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Serge Semin <fancer.lancer@gmail.com>
Cc: Richard Leitner <richard.leitner@skidata.com>,
	Serge Semin <Sergey.Semin@t-platforms.ru>,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] usb: usb251xb: Lock i2c-bus segment the hub resides
Date: Sat, 27 Apr 2019 09:53:23 +0200	[thread overview]
Message-ID: <20190427075323.GA31478@kroah.com> (raw)
Message-ID: <20190427075323.St2-N1MFGBB0NZACXYxu03fRSyEjLiZ4F-j6S0lQRD4@z> (raw)
In-Reply-To: <20190427073940.6yzhklfus4qd6ver@mobilestation>

On Sat, Apr 27, 2019 at 10:39:41AM +0300, Serge Semin wrote:
> On Sat, Apr 27, 2019 at 09:32:00AM +0200, Greg Kroah-Hartman wrote:
> > On Sat, Apr 27, 2019 at 10:10:27AM +0300, Serge Semin wrote:
> > > On Sat, Apr 27, 2019 at 09:00:42AM +0200, Greg Kroah-Hartman wrote:
> > > > On Fri, Apr 26, 2019 at 01:46:50PM +0300, Serge Semin wrote:
> > > > > SMBus slave configuration is activated by CFG_SEL[1:0]=0x1 pins
> > > > > state. This is the mode the hub is supposed to be to let this driver
> > > > > work correctly. But a race condition might happen right after reset
> > > > > is cleared due to CFG_SEL[0] pin being multiplexed with SMBus SCL
> > > > > function. In case if the reset pin is handled by a i2c GPIO expander,
> > > > > which is also placed at the same i2c-bus segment as the usb251x
> > > > > SMB-interface connected to, then the hub reset clearance might
> > > > > cause the CFG_SEL[0] being latched in unpredictable state. So
> > > > > sometimes the hub configuration mode might be 0x1 (as expected),
> > > > > but sometimes being 0x0, which doesn't imply to have the hub SMBus-slave
> > > > > interface activated and consequently causes this driver failure.
> > > > > 
> > > > > In order to fix the problem we must make sure the GPIO-reset chip doesn't
> > > > > reside the same i2c-bus segment as the SMBus-interface of the hub. If
> > > > > it doesn't, we can safely block the segment for the time the reset is
> > > > > cleared to prevent anyone generating a traffic at the i2c-bus SCL lane
> > > > > connected to the CFG_SEL[0] pin. But if it does, nothing we can do, so
> > > > > just return an error. If we locked the i2c-bus segment and tried to
> > > > > communicate with the GPIO-expander, it would cause a deadlock. If we didn't
> > > > > lock the i2c-bus segment, it would randomly cause the CFG_SEL[0] bit flip.
> > > > > 
> > > > > Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
> > > > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > > 
> > > > I signed off on this?  Where?  When?
> > > > 
> > > > never add a s-o-b line that you did not create, that implies a legal
> > > > agreement.
> > > > 
> > > 
> > > Ah, shit. Sorry. I should have added Acked-by. That's what I was going to do,
> > > since you already added the first version of this into linux-next tree. But
> > > apparently copy-pasted and left as is...
> > 
> > If I have already applied a patch, I can't apply it again (or a
> > different version.)  You need to send a fix-up patch for the reported
> > issue, not a whole new one as I can not go back in time and rewrite
> > history.
> > 
> > thanks,
> > 
> > greg k-h
> 
> Ok. Should I send it in reply to this patch or as a completely separate one?

A separate one.

             reply	other threads:[~2019-04-27  7:53 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-27  7:53 Greg Kroah-Hartman [this message]
2019-04-27  7:53 ` [PATCH v2] usb: usb251xb: Lock i2c-bus segment the hub resides Greg Kroah-Hartman
  -- strict thread matches above, loose matches on Subject: below --
2019-04-27 10:58 [v2] " Serge Semin
2019-04-27 10:58 ` [PATCH v2] " Serge Semin
2019-04-27 10:45 [v2] " Greg Kroah-Hartman
2019-04-27 10:45 ` [PATCH v2] " Greg Kroah-Hartman
2019-04-27  9:31 [v2] " Serge Semin
2019-04-27  9:31 ` [PATCH v2] " Serge Semin
2019-04-27  7:39 [v2] " Serge Semin
2019-04-27  7:39 ` [PATCH v2] " Serge Semin
2019-04-27  7:32 [v2] " Greg Kroah-Hartman
2019-04-27  7:32 ` [PATCH v2] " Greg Kroah-Hartman
2019-04-27  7:10 [v2] " Serge Semin
2019-04-27  7:10 ` [PATCH v2] " Serge Semin
2019-04-27  7:00 [v2] " Greg Kroah-Hartman
2019-04-27  7:00 ` [PATCH v2] " Greg Kroah-Hartman
2019-04-26 10:46 [v2] " Serge Semin
2019-04-26 10:46 ` [PATCH v2] " Serge Semin
2019-04-26 10:45 [v2] " Serge Semin
2019-04-25 22:32 Serge Semin
2019-04-24 14:49 Serge Semin
2019-04-24 14:49 ` [PATCH] " Serge Semin

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=20190427075323.GA31478@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=Sergey.Semin@t-platforms.ru \
    --cc=fancer.lancer@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=richard.leitner@skidata.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.