linux-kernel - Re: [RFC PATCH v2] regmap: smbus: add support for regmap over smbus

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20140415125654.GD12304@sirena.org.uk>
Date:	Tue, 15 Apr 2014 13:56:54 +0100
From:	Mark Brown <broonie@...nel.org>
To:	Lars-Peter Clausen <lars@...afoo.de>
Cc:	Boris BREZILLON <boris.brezillon@...e-electrons.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Maxime Ripard <maxime.ripard@...e-electrons.com>,
	Shuge <shuge@...winnertech.com>, kevin@...winnertech.com,
	Chen-Yu Tsai <wens@...e.org>,
	Hans de Goede <hdegoede@...hat.com>,
	Carlo Caione <carlo@...one.org>,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	dev@...ux-sunxi.org
Subject: Re: [RFC PATCH v2] regmap: smbus: add support for regmap over smbus

On Tue, Apr 15, 2014 at 02:25:15PM +0200, Lars-Peter Clausen wrote:
> On 04/15/2014 01:54 PM, Boris BREZILLON wrote:

> >And, should I keep one method to register an smbus regmap or should I
> >provide one method per access type and get rid of the
> >regmap_smbus_transfer_type enum ?

> I don't think we should leave the decision which bus to use to the driver.
> Neither should the driver have to choose whether to use smbus or native I2C.
> We want to use native I2C when available, because it is more efficient than
> going through the smbus emulation layer. On the other hand we want to
> automatically switch to smbus when native I2C is not available and the
> device can work fine with smbus. Also I'm afraid that we'll otherwise soon
> see code popping up like:

I have to say I'd misread the commit message (due to the multiple copies
that arrived before I actually reviewed it I think I just skimmed it and
went into incremental review) and was reviewing this as being for
*device* limitations not controller ones.  That also matches with what
Boris was saying about not being able to decide to use the word or byte
at a time operations automatically, though the commit message does say
it's about controller limitations.

> My suggestion is that in regmap_init_i2c() you check the capabilities of the
> I2C adapter. If it supports native I2C you setup the regmap with the
> regmap_i2c struct just as it does right now. If the adapter does not support
> native I2C, check if the device can be supported by smbus (reg_bytes == 8 &&
> val_bytes % 8 == 0). For each type of smbus operations have one regmap_bus
> struct, and if you can fallback to smbus choose the bus depending on the
> config's val_bytes.

That'd definitely be useful but potentially orthogonal, we can also do
both and expose smbus explicitly with I2C falling back to it
transparently.

If the device *is* limited to smbus and the controller supports both
(some do) I'd naively have expected that the native smbus support would
do a little better - otherwise why bother using it?  We could identify
the constraint set automatically for I2C devices though it's more for
the client driver to specify.

This means there's two changes to consider here:

 - Providing APIs for registering actual smbus devices as a convenience
   for devices with that constraint, regardless of how that is done
   behind the scenes.

 - Having the I2C implementation automatically use the smbus APIs if
   it can and either the controller is smbus only or it makes sense to
   do so for optimisation.

both of which are independently useful.

Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)