At Tue, 3 Mar 2015 14:54:38 +0000, Mark Brown wrote: > > On Tue, Mar 03, 2015 at 10:22:59AM +0100, Takashi Iwai wrote: > > Mark Brown wrote: > > > > > The --scissors option of git am is your friend. > > > > That's still pain. > > > But it's still better than sending two mails even if you don't know > > whether it's the right patch. It's even not tag as an RFC. The patch > > was there just as a reference. > > I actually find it harder to work with TBH - it breaks up the mail to > have the extra stuff around the code in there and it's harder to apply > if it's OK. During a discussion it feels more natural to just have the > diff hunk with the mail text around it instead.
Well, this is just a matter of taste, IMO :) I find a full patch is always better, if any. > > > Well, it's either that or adding the values read back from the chip to > > > the defaults. > > > For fixing the single rw, it's easy in either way (although the latter > > sounds bad from the performance POV). But what about the block rw? > > Why should adding something to the defaults hurt performance (it should > just be a one time cost to insert the default which we've got a > reasonable chance of making back later)? I guess if there's a lot of > these registers it'll add up but they're pretty rare, usually it's a few > ID and revision registers and anything else is volatile so wouldn't get > cached at all. I caught this bug because of the currently developed HD-audio regmap support that has lots of read-only stuff (mostly for parameters, dozen of such per each widget node). Registers range of 32bit wide and there are no static default values that can be stored in a flat table. Nasty, eh? I'll be glad if any better workaround is present in regmap. > Block I/O can just get the same fix I think, the logic is basically the > same it's just what we do with differences that changes. Yeah, looks so. > > > > regmap_wrietable() call in _regmap_write(). > > > > It's superfluous with respect to what? Still a bit confused, sorry. > > > regmap_writeable() is called twice in that code path with my patch. > > First before calling _regmap_write() and again in _regmap_write(). > > The second call is superfluous in this code path although it's needed > > for other paths. > > > regmap_writeable() isn't usually that heavy, but it's still > > suboptimal. > > Oh, right. The two checks are logically distinct to me - the check in > _write() is more of an assert than something that's expected to go off, > anything relying on it is in trouble, while the one in the cache sync is > there as part of normal operation. If anyone cared about performance to > that extent it probably ought to be a build option to even check though > since the I/O is generally so slow and it's rare to implement writeable > at all it doesn't normally matter. Right, that's my assumption. But I wasn't sure whether it was acceptable. I can resend the patch if you prefer, of course, if the original patch is OK. Just let me know. thanks, Takashi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
