| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <568D4C3D.3000906@iki.fi> Date: Wed, 6 Jan 2016 19:17:49 +0200 From: Antti Palosaari <crope@....fi> To: Peter Rosin <peda@...ator.liu.se>, Wolfram Sang <wsa@...-dreams.de> Cc: Peter Rosin <peda@...ntia.se>, Rob Herring <robh+dt@...nel.org>, Pawel Moll <pawel.moll@....com>, Mark Rutland <mark.rutland@....com>, Ian Campbell <ijc+devicetree@...lion.org.uk>, Kumar Gala <galak@...eaurora.org>, Peter Korsgaard <peter.korsgaard@...co.com>, Guenter Roeck <linux@...ck-us.net>, Jonathan Cameron <jic23@...nel.org>, Hartmut Knaack <knaack.h@....de>, Lars-Peter Clausen <lars@...afoo.de>, Peter Meerwald <pmeerw@...erw.net>, Mauro Carvalho Chehab <mchehab@....samsung.com>, Frank Rowand <frowand.list@...il.com>, Grant Likely <grant.likely@...aro.org>, Adriana Reus <adriana.reus@...el.com>, Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>, Krzysztof Kozlowski <k.kozlowski@...sung.com>, Hans Verkuil <hans.verkuil@...co.com>, Nicholas Mc Guire <hofrat@...dl.org>, Olli Salonen <olli.salonen@....fi>, linux-i2c@...r.kernel.org, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, linux-iio@...r.kernel.org, linux-media@...r.kernel.org Subject: Re: [PATCH v2 0/8] i2c mux cleanup and locking update On 01/05/2016 05:57 PM, Peter Rosin wrote: > From: Peter Rosin <peda@...ntia.se> > > Hi! > > I have a pair of boards with this i2c topology: > > GPIO ---| ------ BAT1 > | v / > I2C -----+------B---+---- MUX > | \ > EEPROM ------ BAT2 > > (B denotes the boundary between the boards) Handling of I2C muxes that close channel automatically, after the first I2C stop (P) is seen? For example channel is selected to BAT1 => there is EEPROM write => mux closes channel BAT1 => access to BAT1 will fail. Is it possible to lock whole adapter, but allow only traffic to i2c mux client? regards Antti > > The problem with this is that the GPIO controller sits on the same i2c bus > that it MUXes. For pca954x devices this is worked around by using unlocked > transfers when updating the MUX. I have no such luck as the GPIO is a general > purpose IO expander and the MUX is just a random bidirectional MUX, unaware > of the fact that it is muxing an i2c bus, and extending unlocked transfers > into the GPIO subsystem is too ugly to even think about. But the general hw > approach is sane in my opinion, with the number of connections between the > two boards minimized. To put is plainly, I need support for it. > > So, I observe that while it is needed to have the i2c bus locked during the > actual MUX update in order to avoid random garbage on the slave side, it > is not strictly a must to have it locked over the whole sequence of a full > select-transfer-deselect operation. The MUX itself needs to be locked, so > transfers to clients behind the mux are serialized, and the MUX needs to be > stable during all i2c traffic (otherwise individual mux slave segments > might see garbage). > > This series accomplishes this by adding a dt property to i2c-mux-gpio and > i2c-mux-pinctrl that can be used to state that the mux is updated by means > of the muxed master bus, and that the select-transfer-deselect operations > should be locked individually. When this holds, the i2c bus *is* locked > during muxing, since the muxing happens as part of i2c transfers. This > is true even if the MUX is updated with several transfers to the GPIO (at > least as long as *all* MUX changes are using the i2s master bus). A lock > is added to the mux so that transfers through the mux are serialized. > > Concerns: > - The locking is perhaps too complex? > - I worry about the priority inheritance aspect of the adapter lock. When > the transfers behind the mux are divided into select-transfer-deselect all > locked individually, low priority transfers get more chances to interfere > with high priority transfers. > - When doing an i2c_transfer() in_atomic() context of with irqs_disabled(), > there is a higher possibility that the mux is not returned to its idle > state after a failed (-EAGAIN) transfer due to trylock. > > To summarize the series, there's some i2c-mux infrastructure cleanup work > first (I think that part stands by itself as desireable regardless), the > locking changes are in the last three patches of the series, with the real > meat in 8/8. > > PS. needs a bunch of testing, I do not have access to all the involved hw > > Changes since v1: > - Allocate mux core and (optional) priv in a combined allocation. > - Killed dev_err messages triggered by memory allocation failure. > - Fix the device specific i2c muxes that I had overlooked. > - Rebased on top of v4.4-rc8 (was based on v4.4-rc6 previously). > > Cheers, > Peter > > Peter Rosin (8): > i2c-mux: add common core data for every mux instance > i2c-mux: move select and deselect ops to i2c_mux_core > i2c-mux: move the slave side adapter management to i2c_mux_core > i2c-mux: remove the mux dev pointer from the mux per channel data > i2c-mux: pinctrl: get rid of the driver private struct device pointer > i2c: allow adapter drivers to override the adapter locking > i2c: muxes always lock the parent adapter > i2c-mux: relax locking of the top i2c adapter during i2c controlled > muxing > > .../devicetree/bindings/i2c/i2c-mux-gpio.txt | 2 + > .../devicetree/bindings/i2c/i2c-mux-pinctrl.txt | 4 + > drivers/i2c/i2c-core.c | 59 ++--- > drivers/i2c/i2c-mux.c | 272 +++++++++++++++++---- > drivers/i2c/muxes/i2c-arb-gpio-challenge.c | 46 ++-- > drivers/i2c/muxes/i2c-mux-gpio.c | 58 ++--- > drivers/i2c/muxes/i2c-mux-pca9541.c | 58 +++-- > drivers/i2c/muxes/i2c-mux-pca954x.c | 66 ++--- > drivers/i2c/muxes/i2c-mux-pinctrl.c | 89 +++---- > drivers/i2c/muxes/i2c-mux-reg.c | 63 ++--- > drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 33 +-- > drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h | 2 +- > drivers/media/dvb-frontends/m88ds3103.c | 23 +- > drivers/media/dvb-frontends/m88ds3103_priv.h | 2 +- > drivers/media/dvb-frontends/rtl2830.c | 24 +- > drivers/media/dvb-frontends/rtl2830_priv.h | 2 +- > drivers/media/dvb-frontends/rtl2832.c | 30 ++- > drivers/media/dvb-frontends/rtl2832_priv.h | 2 +- > drivers/media/dvb-frontends/si2168.c | 29 ++- > drivers/media/dvb-frontends/si2168_priv.h | 2 +- > drivers/media/usb/cx231xx/cx231xx-core.c | 6 +- > drivers/media/usb/cx231xx/cx231xx-i2c.c | 48 ++-- > drivers/media/usb/cx231xx/cx231xx.h | 4 +- > drivers/of/unittest.c | 41 ++-- > include/linux/i2c-mux-gpio.h | 2 + > include/linux/i2c-mux-pinctrl.h | 2 + > include/linux/i2c-mux.h | 39 ++- > include/linux/i2c.h | 28 ++- > 28 files changed, 612 insertions(+), 424 deletions(-) > -- http://palosaari.fi/ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists