From mboxrd@z Thu Jan 1 00:00:00 1970 From: Adrian Fiergolski Subject: Re: [PATCH] i2c: Add support for NXP PCA984x family. Date: Mon, 11 Dec 2017 14:15:29 +0100 Message-ID: References: <20171211111022.30333-1-adrian.fiergolski@cern.ch> <38777c0b-fc81-3af5-e96d-d0ab1bb83d85@axentia.se> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-eopbgr00083.outbound.protection.outlook.com ([40.107.0.83]:19264 "EHLO EUR02-AM5-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751400AbdLKNPd (ORCPT ); Mon, 11 Dec 2017 08:15:33 -0500 In-Reply-To: <38777c0b-fc81-3af5-e96d-d0ab1bb83d85@axentia.se> Content-Language: en-GB Sender: linux-i2c-owner@vger.kernel.org List-Id: linux-i2c@vger.kernel.org To: Peter Rosin , Rodolfo Giometti , linux-i2c@vger.kernel.org Cc: giometti@linux.it On 11.12.2017 at 13:51, Peter Rosin wrote: > On 2017-12-11 12:25, Rodolfo Giometti wrote: >> On 11/12/17 12:10, Adrian Fiergolski wrote: >>> This patch exetends the current i2c-mux-pca954x driver and adds suuport for >>> a newer PCA984x family of the I2C switches and multiplexers from NXP. >>> In probe function, the patch supports device ID function, introduced in the >>> new family, and checks it against configuration provided by the >>> devicetree. Moreover, it also performs software reset which is now >>> available for the new devices. >>> The reset of the code remains common with the legacy PCA954x devices. >>> >>> Signed-off-by: Adrian Fiergolski >>> --- >>> .../{i2c-mux-pca954x.txt => i2c-mux-pca9x4x.txt} | 5 +- >>> arch/arm/configs/aspeed_g4_defconfig | 2 +- >>> arch/arm/configs/aspeed_g5_defconfig | 2 +- >>> arch/arm/configs/multi_v7_defconfig | 2 +- >>> arch/arm/configs/pxa_defconfig | 2 +- >>> arch/arm/configs/tegra_defconfig | 2 +- >>> arch/arm64/configs/defconfig | 2 +- >>> arch/powerpc/configs/85xx-hw.config | 2 +- >>> drivers/i2c/muxes/Kconfig | 6 +- >>> drivers/i2c/muxes/Makefile | 2 +- >>> drivers/i2c/muxes/i2c-mux-pca9541.c | 4 +- >>> .../muxes/{i2c-mux-pca954x.c => i2c-mux-pca9x4x.c} | 295 ++++++++++++++++----- >>> .../linux/platform_data/{pca954x.h => pca9x4x.h} | 15 +- >>> 13 files changed, 246 insertions(+), 95 deletions(-) >>> rename Documentation/devicetree/bindings/i2c/{i2c-mux-pca954x.txt => i2c-mux-pca9x4x.txt} (91%) >>> rename drivers/i2c/muxes/{i2c-mux-pca954x.c => i2c-mux-pca9x4x.c} (56%) >>> rename include/linux/platform_data/{pca954x.h => pca9x4x.h} (80%) >>> >>> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt b/Documentation/devicetree/bindings/i2c/i2c-mux-pca9x4x.txt >>> similarity index 91% >>> rename from Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt >>> rename to Documentation/devicetree/bindings/i2c/i2c-mux-pca9x4x.txt >>> index aa097045a10e..cf9a075ca1dd 100644 >>> --- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt >>> +++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca9x4x.txt >>> @@ -1,10 +1,11 @@ >>> -* NXP PCA954x I2C bus switch >>> +* NXP PCA9x4x I2C bus switch >>> >>> Required Properties: >>> >>> - compatible: Must contain one of the following. >>> "nxp,pca9540", "nxp,pca9542", "nxp,pca9543", "nxp,pca9544", >>> - "nxp,pca9545", "nxp,pca9546", "nxp,pca9547", "nxp,pca9548" >>> + "nxp,pca9545", "nxp,pca9546", "nxp,pca9547", "nxp,pca9548", >>> + "nxp,pca9846", "nxp,pca9847", "nxp,pca9848", "nxp,pca9849" >>> >>> - reg: The I2C address of the device. >>> >>> diff --git a/arch/arm/configs/aspeed_g4_defconfig b/arch/arm/configs/aspeed_g4_defconfig >>> index d23b9d56a88b..a461ad3cf63d 100644 >>> --- a/arch/arm/configs/aspeed_g4_defconfig >>> +++ b/arch/arm/configs/aspeed_g4_defconfig >>> @@ -116,7 +116,7 @@ CONFIG_I2C=y >>> CONFIG_I2C_CHARDEV=y >>> CONFIG_I2C_MUX=y >>> CONFIG_I2C_MUX_PCA9541=y >>> -CONFIG_I2C_MUX_PCA954x=y >>> +CONFIG_I2C_MUX_PCA9x4x=y >> Nak. >> >> I'm not sure you should break backward compatibility. You should keep the >> CONFIG_I2C_MUX_PCA954x configuration setting and the current name convention. > Right, definitely avoid the mass rename. In addition to the pointless > churn, pca9x4x is just plain wrong since it matches e.g. pca9745 which > has nothing whatsoever to do with this driver. > > I'll add more comments once the rename noise is gone. > > Cheers, > Peter > Thanks for comments. What do you suggest then ? I could keep CONFIG_I2C_MUX_PCA954x, but then, since there is a common source, if enabled, the PCA985x family support would be provided as well anyway. I could try to move PCA985x features to a separate file which would be included by i2c-mux-pca954x.c if a dedicated CONFIG_I2C_MUX_PCA984x is enabled, but it doesn't seem to be an elegant solution to me. I understand, that your main concern regards CONFIG_I2C_MUX_PCA954x only. The fact that internal data types and files have been renamed to a more generic and actual pca9x5x style is fine, right ? Cheers, Adrian