From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1762832AbdDSMBN (ORCPT ); Wed, 19 Apr 2017 08:01:13 -0400 Received: from mail-ve1eur01on0090.outbound.protection.outlook.com ([104.47.1.90]:48584 "EHLO EUR01-VE1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1762553AbdDSMBE (ORCPT ); Wed, 19 Apr 2017 08:01:04 -0400 Authentication-Results: pengutronix.de; dkim=none (message not signed) header.d=none;pengutronix.de; dmarc=none action=none header.from=axentia.se; Subject: Re: [PATCH v13 03/10] mux: minimal mux subsystem and gpio-based mux controller To: Philipp Zabel References: <1492101794-13444-1-git-send-email-peda@axentia.se> <1492101794-13444-4-git-send-email-peda@axentia.se> <1492592817.2970.14.camel@pengutronix.de> CC: , Greg Kroah-Hartman , Wolfram Sang , Rob Herring , Mark Rutland , Jonathan Cameron , Hartmut Knaack , Lars-Peter Clausen , Peter Meerwald-Stadler , Jonathan Corbet , , , , , Andrew Morton , Colin Ian King , Paul Gortmaker , From: Peter Rosin Organization: Axentia Technologies AB Message-ID: Date: Wed, 19 Apr 2017 14:00:54 +0200 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <1492592817.2970.14.camel@pengutronix.de> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit X-Originating-IP: [81.224.171.159] X-ClientProxiedBy: DB6PR0601CA0033.eurprd06.prod.outlook.com (10.169.209.19) To VI1PR0202MB2557.eurprd02.prod.outlook.com (10.173.79.136) X-MS-Office365-Filtering-Correlation-Id: 714dc8f4-7ed0-4968-7ec2-08d4871bbf83 X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:(22001)(201703131423075);SRVR:VI1PR0202MB2557; X-Microsoft-Exchange-Diagnostics: 1;VI1PR0202MB2557;3:bffsbYFVTc4qOZWKIp5ckluDwEgaAnya5PxBpQjU+QtSG2Pls8GG0qMjQBGHGU/IotSo5cRHlXZxge06fKE8IsPrrvREXQRGNJ7eVcD/eEX3JpgWXY1FEFHT1V59hJiTwVxqNkrLbeMeZzY+gnd5hyFC5DBk9jjZ6ajEiSxszpUexfDkrUkMr5V7PJof/fAuBhtYPhPakNfVP6iCzYRiAKuttdwo1pAH/ZumxzOMti0rQLlNkTOWhUyaxz+pgRAqKzBSNR6dkspCAmSEi7JL7JHwRAfaMLfLTat+TE2u4ESsg2/O8kkeWqfZmqvU6/F5;25:sAQl+ynwgitrFlsXF7pYy/OaLai2xAZx5TnpFZgSbICYHWhr5JH9XokkG4mp4w1BF3ybwDxKl6k5QDu7mTuJEmPgZcc7HJh9k2oocHTCFtFyBY88Qc8oCGro5+f/7IzF1N8xfXPcJrwFl/0LvPYQhR5uIPsgmcs+50J7EVXQVKTSBxswSMnJzGBlNCJTC9T8xB3InFelEV6NQg+qlJUyICccQyh5a3ATkby1wKrz9ysVXm6IVu3Bt1KMR1h/nDhH6pw1bg6o06qo/ux5HVzqNrMIoIA4H1+C88iSHcgHkONDFpMmkz07Ic5T74eYzfGL77iR1aWRVWElgg1hMtmJU5XU7MouuFS5Q5k/HlVWUoql1/Jco8p2Pjga7gX8a6kjqQRIHXUmYDI68kyowh0uQVlV6FR26zlKWdrKoMLSqjGAitLebF2vudyIX53nnZq2bHJavXE5hOZDkkENGqdrQQ== X-Microsoft-Exchange-Diagnostics: 1;VI1PR0202MB2557;31:1qFfJVr/tcy09eLgMukISfwhh+o4R9ZaBD6+848RDu6ZAUK4L27czeCkGRc5BaDxcB92JHODtwnpLKI+6Q7mO9ACJmsGed7/ulxh1ebDReb0GPNIRmkU8+zJzIi261uuHytD8/caAtknmU1OJ2VmWVZ72Qs53LPBsiW8F6XHWq8b8HS4iupp/R6JUxF9rAlnjiNN/6kdrVDUPqpBmtHuxO0LoJH3D1SSG9hkXgX+o3Gwjc5yfUrdskfrhNM/2FC21OYe8zsenaF7QeqFcHQXxsrE2N21ABpakgCz80ZmGMw= X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(6040450)(601004)(2401047)(5005006)(8121501046)(93006095)(93001095)(10201501046)(3002001)(6041248)(201703131423075)(201703061421075)(20161123562025)(20161123560025)(2016111802025)(20161123555025)(20161123564025)(6072148)(6043046);SRVR:VI1PR0202MB2557;BCL:0;PCL:0;RULEID:;SRVR:VI1PR0202MB2557; X-Microsoft-Exchange-Diagnostics: 1;VI1PR0202MB2557;4:p0Xb6AT/+8W6jVH4wh71TNDdnVGGSF/WCtF6g6g/dq9sRE2F556kbI1W2PvH37SMFsf4bsxvYj7J24p1Jd6kbeCMt2SlbGlPIzj1cwT4OHj2sI3VFRuy9cyYIP1YuqqQweCsdGsdH2Qg3Y6yl0vhKH+5g8GJScx+xkzYmZ65G/dZnSBFsnDLbG3NBkA41o5IqnxtkC9I8f22apFxJ4FQEQrkjI8tloksBiQQWVRel9ePe3BX710AGBQvYauyTTufrETNzzjUQkoOX1KGFkqzqvporaYGxVL+rLVzqgDpa0/5EAHrlnJk9p5o12ZAMTc1oTjknxIcF7L0HMvIAZEJjz3oxLbzaKItvPITCtxscGWJYLOmLXFXDqyhF4hHcCHKIW1eBEwYLTnAgfb5+ROSsWYmEARovlcD+unV9m6Gc2x9gU3B8/DclC4F2YWhv9sw/1BUQvmvkdBkwmBHmzh8gLKcqs2RodeIQk7bkpfGEBxb2PAtZIWZhwVAOzjioGMhfasJ6GXMjz1yINhywG2948CjVZUGqTpQIxv0hEv2MHR/MpEo17yZnyivcABvCokZK7PcdeHF7x1eyz/0YkhSFvmnugFvQmE+wi7qUPx9HQbbeCBpOCo2UT4wbGZtfOjSuxOkmjsv2Fe/q5T3KStC30VTNmtg5VOQFmZCgmg2N/hrNT3iWN1QWcj1ObhIhVmom1iYMRUw+UEGDoacCGuSuqYBiWiJCgfrYth9fkzf/NI= X-Forefront-PRVS: 028256169F X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10019020)(4630300001)(6049001)(6009001)(39450400003)(39400400002)(24454002)(377424004)(54094003)(25786009)(229853002)(53546009)(2906002)(36756003)(7416002)(50986999)(3260700006)(4001350100001)(54356999)(76176999)(31686004)(42186005)(66066001)(2950100002)(117156002)(6666003)(8666007)(7736002)(74482002)(86362001)(5660300001)(47776003)(33646002)(110136004)(305945005)(54906002)(53936002)(4326008)(31696002)(90366009)(50466002)(38730400002)(8676002)(189998001)(6916009)(6486002)(23676002)(6116002)(3846002)(83506001)(77096006)(64126003)(230700001)(81166006)(42262002);DIR:OUT;SFP:1102;SCL:1;SRVR:VI1PR0202MB2557;H:[192.168.0.125];FPR:;SPF:None;MLV:sfv;LANG:en; X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtWSTFQUjAyMDJNQjI1NTc7MjM6SDU2Z0QzU3ZhMi94OGhNYXhqUDF3WFda?= =?utf-8?B?QTFUbGhNUzRkOWdJTDhKcGFoNVlXeEdHOW9mZWtrOWhaeE1KdjFtL3ByZU5W?= =?utf-8?B?NDJRSEs3Q2lQSjJIUTBJMExJa0d1OVA0dmliQTJueUFBcHlNYVk2OVVsbVFY?= =?utf-8?B?bmVvaDJIYXQxRm9yQ2IySitZblpEdndIWDFOYjZlbkZvREljQXNSeUMrVDJS?= =?utf-8?B?NVdRT2tKQitSRS9sZzNNZEdCaEJXZW5YOUR6SUhQS1lLL3h0aUlyVXY0OHkw?= =?utf-8?B?aTR3NE5hWmYwUlZ6UUhJaEdyamV5a3FDc2s1ZXZBRG1DaWZaQ2o0Y21PL1I4?= =?utf-8?B?V3U4NC90WEpYRDZMRmhTNVhkMktXV0hoMy85cW8yZW5YSUs2a0QxdlZSVU0v?= =?utf-8?B?N3VCMWRLNjhucE83OXkxakVOQ3pUOWJTMlZscVBwdHc2T3V6dDh5MjBPakVn?= =?utf-8?B?d3dJMWVLNUtqa1oyYWVyei81d1FNbmVkZklYVVNNZWxXL3cyOW5tUkdmcm9W?= =?utf-8?B?Rmh0bWxSeHllUVIrY3RQYnpiRzBrMVVka1pKTGIrQ1dvdjZ2UE5GclF6K291?= =?utf-8?B?WndaT0VJSnZEZzE0VjNoRnVHK2Q3SE5JL0RFV1lRWGxKdjJSVDhEVkN5WDRX?= =?utf-8?B?cVpudkJ0Q3dRZXJ3UTRhK29NYVh2UnA0dXJPckkzNUVjaXlSUkhZei9oZVNT?= =?utf-8?B?azJvaFc5VEFIZGZHLzhDSVYxVGVQZklpMnpzaDUzR0VvQTRqZ3ZLQkxWb21t?= =?utf-8?B?RXZMYTVTRi80SVpkd3FiNWpGNWl1THNkYW1LcWtXb21VZmo0b0ZYQm50U084?= =?utf-8?B?RWlKNkcwUnB5L3lIWGViOVFNNVJIcDNPVWRZSVlOZnFkaGVjNWprNkJUMHl4?= =?utf-8?B?akl2aFlaMzJacFNQd3FmZjBoWCt3UjZBNzdtQU95Y3JTZ2VEV3djdmZPSEtE?= =?utf-8?B?T25XRFh5ZTRGYkQvb01RQTIwM0FzSjgyd1pDUTNuQ1M5TzF6cmYxWUVHOWw1?= =?utf-8?B?R1NuajBnZHpuOHl6WGlXSC9EdndIaDBvMy9seXg3T0loekcxRmFXTVI2VWxp?= =?utf-8?B?Tm5LeG1xWHJOSlJzVnhKVlJuUVNTY2hhQVlaOWJqSEhHdWZURDY3RHJ0cFdz?= =?utf-8?B?VkQwV2ZpL3VqV2RUTkF3QWFFbWRYc3NRUk9wM1lNcllpSWpmY0tZdi82bVJI?= =?utf-8?B?MEhxVEdwMmc5SWlLYkxmL294K3VhRmk3d1VyN1FaeC93L1Z1MERQempLaUhH?= =?utf-8?B?K2tEVVMxSDR0NlQ5NlV3Z1ozSE5mV2VoYW5CTjV1Wlp5NjhCQmpORmI4aldi?= =?utf-8?B?NlhENndrSGN5ZW94RXVvOXZoVXJmSEtETUdCemlPaVNOc1ZDRjZ5eXVNWEdi?= =?utf-8?B?U2dOUXdXVklSUU5mTlJmR0pVbStQOW8zUGJWZXk4THZlWkZNWlpJK1A3bFpw?= =?utf-8?B?MjlQVU10S2paOXZ0dmhXL0txUUVFVEtHenJZaHorVkU2c0pzb3ZsM1FDWE5q?= =?utf-8?B?MFBxL3JmRHFRbVkxN2JOVTdJdWNpWlpXVHE4aVRGZEJHWUtQa0VsODFaL1dW?= =?utf-8?B?a2pUeDZNS1ZoWDVOZUtsOFhWQnNFeWhJVDdZOGsyZEpTZ2NOY2MrQ2FxbG5n?= =?utf-8?B?TUVPbVVIVWJCMHZ3NXZydXBKdEwwbU9wT2NUVVhpb3pXWGdRVmxFV3VXY3Ew?= =?utf-8?B?cUR0RVoyS1VPOWZZSk16Z3U1ZXRmT0JQYi9QdWZsZGU5Z09pb3pKRHN4RGZ6?= =?utf-8?Q?KopM7PvfjnxnIYPX1wWN3xrSDsyo9bKTVdjweb4=3D?= X-Microsoft-Exchange-Diagnostics: 1;VI1PR0202MB2557;6:C/ynzWoY8zlLufduaJDg/S6/cJnWd3H9aI9YmqOaDzvVqSAVv69gL8Qefe3yF++Ni/PG1VSp81sKD0MlmygqQUFfN2HiaXTrkkxUZJqoA8A6M2cXRVOcFEoIhXmzUy/uGunkb4nxkKZIg7teCWOJp4qhOm3ws+GTzeUHactjy6y1/Zfz6xbvkbxVo0u8xQakIYs6x5DBKmcfC95NGOrIrf43JU/5XJDCn5RVfx5dbk6p57u51gAPGpiKQicQc1HTPzGsQX2AfKz05Th1J+1PA8nWdLPKNfUHwgM16t9fJizapJuqmIEAmRhEFAzgr/y4wPyWymPIFt7PTFIMyu9M/5xb8mZKGbMT/Yg9PwjQyFj9/gn75pMFsrzz18pWmCAVzWWt+haxH+ZQ3Wu5ZkiwvLjQ/mNir+e0DBSBUIGba6ITQXGqRoYIH51SzkVNm9rS85Rpj+QcH8t/ORkKIznFeA==;5:Yl9XsjN66IlenoNGUGy7Bt86AuFiR0Fy6P9NG8oZEzLW/qzap8ARRFdNGL1dZHPTnh0W1eI3JRx64Xip4322oUDfjXQPQl5Xma2H0AsMVO9rPW0NMqHs8QpNwCPO4KXOlZUM4vR2MjgRTUJIr4Ixqg==;24:1+ZS4U0yqnQLqpnFM3gPYtAbcV58nJVII9aqyIuOSBLpun22+5XhRdl+wOLwL09klMO1GPXp8uynSOB7EWpFdTme0VfxndnDpoXxVzPncw4= SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-Microsoft-Exchange-Diagnostics: 1;VI1PR0202MB2557;7:cQOeXShutHGJQJix9YPOiGHCGs7qUOIyUronlidbEp8/KAxHxkIFc9a7zk+6/ugsG15IFnhU4HjWJyHypptNZeZ4wNdLl38o7k4HHpxQKFvxOcHYZm2tJaH1fNTrdP4WrWOFFOtLuTJ7I6mQREMlWe17yDw0Bl4t6lplzuWlUTmFYDP+xAWH4mOOwMxh7DeflDIiw1QituLV1qdPbv99PcLvkZP8BabfEQtXEuXrVyzhcf08een98AQcny5KoHcgEKTckgZRPT8Z1DaadbVZMYRgmeVGYvSHSokiXqQzUrGMP7b0We+razx0KPFs41SXd9TUAUlAlcod+wHvWaAxJA== X-OriginatorOrg: axentia.se X-MS-Exchange-CrossTenant-OriginalArrivalTime: 19 Apr 2017 12:00:58.7103 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR0202MB2557 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2017-04-19 11:06, Philipp Zabel wrote: > On Thu, 2017-04-13 at 18:43 +0200, Peter Rosin wrote: >> Add a new minimalistic subsystem that handles multiplexer controllers. >> When multiplexers are used in various places in the kernel, and the >> same multiplexer controller can be used for several independent things, >> there should be one place to implement support for said multiplexer >> controller. >> >> A single multiplexer controller can also be used to control several >> parallel multiplexers, that are in turn used by different subsystems >> in the kernel, leading to a need to coordinate multiplexer accesses. >> The multiplexer subsystem handles this coordination. >> >> This new mux controller subsystem initially comes with a single backend >> driver that controls gpio based multiplexers. Even though not needed by >> this initial driver, the mux controller subsystem is prepared to handle >> chips with multiple (independent) mux controllers. >> >> Reviewed-by: Jonathan Cameron >> Signed-off-by: Peter Rosin >> --- *snip* >> +int mux_control_select(struct mux_control *mux, int state) > > If we let two of these race, ... The window for this "race" is positively huge. If there are several mux consumers of a single mux controller, it is self-evident that if one of them grabs the mux for a long time, the others will suffer. The design is that the rwsem is reader-locked for the full duration of a select/deselect operation by the mux consumer. >> +{ >> + int ret; >> + >> + if (down_read_trylock(&mux->lock)) { >> + if (mux->cached_state == state) >> + return 0; >> + /* Sigh, the mux needs updating... */ >> + up_read(&mux->lock); > > ... and both decide the mux needs updating ... > >> + } >> + >> + /* ...or it's just contended. */ >> + down_write(&mux->lock); > > ... then the last to get to down_write will just wait here forever (or > until the first consumer calls mux_control_deselect, which may never > happen)? It is vital that the mux consumer call _deselect when it is done with the mux. Not doing so will surely starve out any other mux consumers. The whole thing is designed around the fact that mux consumers should deselect the mux as soon as it's no longer needed. It's simply not possible to share something as fundamental as a mux without some cooperation. It's not like suffering mux consumers can go off and use some other mux, and it's also not possible for a "competing" mux consumer to just clobber the mux state. >> + >> + if (mux->cached_state == state) { >> + /* >> + * Hmmm, someone else changed the mux to my liking. >> + * That makes me wonder how long I waited for nothing? >> + */ >> + downgrade_write(&mux->lock); >> + return 0; >> + } >> + >> + ret = mux_control_set(mux, state); >> + if (ret < 0) { >> + if (mux->idle_state != MUX_IDLE_AS_IS) >> + mux_control_set(mux, mux->idle_state); >> + >> + up_write(&mux->lock); >> + return ret; >> + } >> + >> + downgrade_write(&mux->lock); >> + >> + return 1; >> +} >> +EXPORT_SYMBOL_GPL(mux_control_select); > > I wonder if these should be called mux_control_lock/unlock instead, > which would allow for try_lock and lock_timeout variants. Maybe, I'm not totally against it. Do others care to opine? But mux_control_try_select and mux_control_select_timeout does not look all that bad either. But maybe foo_lock is making it clearer that a foo_unlock is needed, if you compared it to foo_select and foo_unselect? I'm probably not the best person to make the call, as I know all to well what to expect from the functions... Cheers, peda