Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751820AbdHAPOt (ORCPT ); Tue, 1 Aug 2017 11:14:49 -0400 Received: from mail.free-electrons.com ([62.4.15.54]:45935 "EHLO mail.free-electrons.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751352AbdHAPOr (ORCPT ); Tue, 1 Aug 2017 11:14:47 -0400 Date: Tue, 1 Aug 2017 17:14:34 +0200 From: Boris Brezillon To: Arnd Bergmann Cc: Wolfram Sang , linux-i2c@vger.kernel.org, Jonathan Corbet , linux-doc@vger.kernel.org, Greg Kroah-Hartman , Przemyslaw Sroka , Arkadiusz Golec , Alan Douglas , Bartosz Folta , Damian Kos , Alicja Jurasik-Urbaniak , Jan Kotas , Cyprian Wronka , Alexandre Belloni , Thomas Petazzoni , Nishanth Menon , Rob Herring , Pawel Moll , Mark Rutland , Ian Campbell , Kumar Gala , devicetree@vger.kernel.org, Linux Kernel Mailing List Subject: Re: [RFC 2/5] i3c: Add core I3C infrastructure Message-ID: <20170801171434.2bc2aa84@bbrezillon> In-Reply-To: References: <1501518290-5723-1-git-send-email-boris.brezillon@free-electrons.com> <1501518290-5723-3-git-send-email-boris.brezillon@free-electrons.com> <20170731231509.77d1fba4@bbrezillon> <20170801142936.5df48702@bbrezillon> <20170801153414.6ce34ee8@bbrezillon> <20170801155813.4b811a4d@bbrezillon> X-Mailer: Claws Mail 3.13.2 (GTK+ 2.24.30; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2324 Lines: 66 On Tue, 1 Aug 2017 16:22:21 +0200 Arnd Bergmann wrote: > On Tue, Aug 1, 2017 at 3:58 PM, Boris Brezillon > wrote: > > On Tue, 1 Aug 2017 15:34:14 +0200 > > Boris Brezillon wrote: > >> On Tue, 1 Aug 2017 15:11:44 +0200 > >> Arnd Bergmann wrote: > >> > On Tue, Aug 1, 2017 at 2:29 PM, Boris Brezillon > >> > wrote: > > I just realized I forgot to add a "depends on I2C" in the I3C Kconfig > > entry. Indeed, I'm unconditionally calling functions provided by the > > I2C framework which have no dummy wrapper when I2C support is disabled. > > I could of course conditionally compile some portion of the I3C > > framework so that it still builds when I2C is disabled but I'm not sure > > it's worth the trouble. > > > > This "depends on I2C" should also solve the I2C+I3C driver issue, since > > I2C is necessarily enabled when I3C is. > > > > Am I missing something? > > That should solve another part of the problem, as a combined driver then > just needs 'depends on I3C'. > > On top of that, the i3c_driver structure could also contain callback > pointers for the i2c subsystem, e.g. i2c_probe(), i2c_remove() etc. > When the i2c_probe() callback exists, the i3c layer could construct > a 'struct i2c_driver' with those callbacks and register that under the > cover. This would mean that combined drivers no longer need to > register two driver objects. That should work. Actually, i2c_driver contains a few more hooks, like ->alert(), ->command() and ->detect(). Of course we could assume that I3C/I2C drivers do not need them, but I'm wondering if it's not easier to just add an i2c_driver pointer inside the i3c_driver struct and let the driver populate it if it needs to supports both protocols. Something like: struct i3c_driver { ... struct i2c_driver *i2c_compat; ... }; and then in I3C/I2C drivers: static struct i2c_driver my_i2c_driver = { ... }; static struct i3c_driver my_i3c_driver = { ... .i2c_compat = &my_i2c_driver, ... }; module_i3c_driver(my_i3c_driver); Of course, you'll have a few fields of ->i2c_compat that would be filled by the core (like the driver name which can be extracted from my_i3c_driver->driver.name).