Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751411AbdH1PNL (ORCPT ); Mon, 28 Aug 2017 11:13:11 -0400 Received: from www.zeus03.de ([194.117.254.33]:48080 "EHLO mail.zeus03.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751259AbdH1PNJ (ORCPT ); Mon, 28 Aug 2017 11:13:09 -0400 Date: Mon, 28 Aug 2017 17:13:06 +0200 From: Wolfram Sang To: Baolin Wang Cc: Baolin Wang , Mark Rutland , Rob Herring , andriy.shevchenko@linux.intel.com, linux-i2c@vger.kernel.org, devicetree@vger.kernel.org, LKML , Mark Brown Subject: Re: [RESEND PATCH v4 2/2] i2c: Add Spreadtrum I2C controller driver Message-ID: <20170828151306.GA10688@katana> References: <20170827153054.ijqxbjk25zpskojl@ninjato> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="LZvS9be/3tNcYl/X" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2881 Lines: 70 --LZvS9be/3tNcYl/X Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable > >> + /* > >> + * If we did not get one ACK from slave when writing data, we sh= ould > >> + * dump all registers to check I2C status. > > > > Why? I would say no. NACK from a slave can always happen, e.g. when an > > EEPROM is busy erasing a page. >=20 > For our I2C controller databook, if the master did not get one ACK > from slave when writing data to salve, we should send one STOP signal > to abort this data transfer or generate one repeated START signal to > start one new data transfer cycle. Considering our I2C usage Yes, so far so good. > scenarios, we should dump registers to analyze I2C status and notify > to user to re-start new data transfer. I disagree here. You notify the users by returning -EIO. The upper layer (e.g. the i2c client driver) will handle it, like the EEPROM driver might retry after a while. This all is expected behaviour, so no need to print the registers to the logfile. If you really, really want to keep it, make it debug output. But I think the sentence "we should dump all registers" needs to be rephrased. > As I explained before, in our Spreadtrum platform, our regulator > driver will depend on I2C driver and the regulator driver uses > subsys_initcall() level to initialize. Moreover some other drivers > like GPU, they will depend on regulator to set voltage and they also > need initialization much earlier. >=20 > Since it is arch_initcall() level, Andy suggested I should get rid of > tristate (use bool) and drop module.h here and all leftovers like > MODULE_*() calls including module_exit(). I see. So the driver is really so essential for proper bootup that it is not even allowed to be unloaded. I might make an exception here and allow arch_initcall() then. But I do wonder: did you try deferred probing all over the place? --LZvS9be/3tNcYl/X Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEOZGx6rniZ1Gk92RdFA3kzBSgKbYFAlmkMv4ACgkQFA3kzBSg KbYoDxAAkDn/t1M/0oACkmTaiBL49b9FxkEsRGAT1KzO3FX/XMolsfhfaT9lfkCs LvL0TGzUIc5zjTpkI1UGH4KG3vtUFsXPFU7vL37y4aWHnHdog6KKgI87JHN/mB+q ezCowPxj9ODbgB4VzefvTx6742a2mdcQM/W6m59VXgCLPAiy7RLU+JeLTAWnmhE1 uhhKafY19IOfaKSTWOx08YeIJWKUse3Ce2YAWP1qQq9qf/UnwHAesJuB6SxVQkPT Tual0b0btGgnotrb9nsZipbJH6A+oiH8TXNZwMq5ORm9Kx7XZwknkwsahvZ87Jmj S7ynh/fqex8ArHIDLOyeXJsFGXrB5qUs0OxzlCDDv/Ev27xNQN1K8p+IwyH5Nx0S qts7yTN49MxP5rMHKR3qZRuc/vE8r1O0NfeQ5r6LMvGfgVqtYKu2mbJZYcqTHFMi XFseg7hGoukCf0juwuss8Kk4gbJVioDeMfEc6mD3Hh0GLisXfAkwO0Dv8Ag3lOuP uqIx371hiR4XtkgdkN34Ws3kEbXMiFstZsIfDnTtSjBxlNbgkQ2rgl2CFW5K0sYC 0f0bSJW3qPYU7vl3wiWYypOevcDwqJZ0PV1zHCuJ2uhbSzrzKoJZ1Xe0POImKaG9 8/rWM3L+hMFglH5SMpo8cruBDsyWEeFmZweGZ5BNz+0dSPPPCi8= =RICq -----END PGP SIGNATURE----- --LZvS9be/3tNcYl/X--