Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754577AbdGUPUU (ORCPT ); Fri, 21 Jul 2017 11:20:20 -0400 Received: from mail-oi0-f68.google.com ([209.85.218.68]:33867 "EHLO mail-oi0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754548AbdGUPUR (ORCPT ); Fri, 21 Jul 2017 11:20:17 -0400 MIME-Version: 1.0 In-Reply-To: <1500648378-10294-1-git-send-email-rajmohan.mani@intel.com> References: <1500648378-10294-1-git-send-email-rajmohan.mani@intel.com> From: Andy Shevchenko Date: Fri, 21 Jul 2017 18:20:15 +0300 Message-ID: Subject: Re: [PATCH v5 0/3] TPS68470 PMIC drivers To: Rajmohan Mani Cc: "linux-kernel@vger.kernel.org" , "linux-gpio@vger.kernel.org" , "linux-acpi@vger.kernel.org" , Lee Jones , Linus Walleij , Alexandre Courbot , "Rafael J. Wysocki" , Len Brown , sakari.ailus@linux.intel.com Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5293 Lines: 137 On Fri, Jul 21, 2017 at 5:46 PM, Rajmohan Mani wrote: > This is the patch series for TPS68470 PMIC that works as a camera PMIC. > > The patch series provide the following 3 drivers, to help configure the voltage regulators, clocks and GPIOs provided by the TPS68470 PMIC, to be able to use the camera sensors connected to this PMIC. > > TPS68470 MFD driver: > This is the multi function driver that initializes the TPS68470 PMIC and supports the GPIO and Op Region functions. > > TPS68470 GPIO driver: > This is the PMIC GPIO driver that will be used by the OS GPIO layer, when the BIOS / firmware triggered GPIO access is done. > > TPS68470 Op Region driver: > This is the driver that will be invoked, when the BIOS / firmware configures the voltage / clock for the sensors / vcm devices connected to the PMIC. > All three patches are good to me (we did few rounds of internal review before posting v4) Reviewed-by: Andy Shevchenko > --- > > Update on 2 GPIO chips implementation over 1: > - Attempted to implement 2 GPIO chips, but ran into couple of > issues in the kernel, so we couldn't get it to work. > - It was decided to postpone this change, since it is not > critical > > Changes in v5: > - MFD driver: > - Fixed Kconfig description text > - Addressed other comments from Lee, related to formatting > > - GPIO driver: > - Formatted the file header text > > - Opregion driver: > - Formatted the file header text > > Changes in v4: > - MFD driver: > - Removed board specific code and FIXME comment > - Moved i2c.h include from tps68470.h to tps68470.c > - Moved the TPS68470 REVID read code after PMIC reset > - Fixed typo in debug error message (on failure of > devm_mfd_add_devices() ) > - Enhanced dependency on I2C by changing it to I2C=y > (to fix build errors if I2C is built as module > e.g tps68470.c:71: undefined reference to `__devm_regmap_init_i2c' > tps68470.c:117: undefined reference to `i2c_register_driver') > - Removed most of the unused header file definitions > - Moved devm_mfd_add_devices() after PMIC resett > - Used probe_new() and removed i2c_device_id table > > The following patch from Andy is needed for the driver to be > probed. > http://marc.info/?l=linux-acpi&m=150030081523885&w=2 > > - GPIO driver: > - Added newline at the end of Kconfig description > - Updated commit message about the descriptive > names for the GPIOs and the typical usage model > of the GPIO driver > > - Opregion driver: > - Added dependency on MFD_TPS68470 > - Converted 2 liner into one line code > > Changes in v3: > - MFD driver: > - Removed GPIO lookup table > - Reverted to probe() for consistency > - Addressed other comments from Andy > > - GPIO driver: > - Removed the code that initializes the default values > of GPIOs to zeros > - Used gpiochip_get_data() to access data inside the gpio_chip > > Changes in v2: > - MFD driver: > - Removed tps68470_* wrappers around regmap_* calls > - Removed "struct tps68470" > - used devm_mfd_add_devices and removed mutex in mfd driver > - Added reasoning about the need of having mfd driver > as bool/builtin > > - Opregion driver: > - renamed opregion driver file / internal symbol names > with tps68470_pmic* > - Made opregion driver tables as const > - Removed unused *handler_context in common handler > - Replaced "int" with "unsigned int" > - Changed to WARN macro to dev_warn() > - Destroyed mutex on error > - Added reasoning about the need of having Opregion driver > as bool/builtin > > - GPIO driver: > - Implemented get_direction() in the GPIO driver > - Setup gpio_chip.names > - Moved the GPIO lookup table code inside mfd driver > - Added reasoning about the need of having GPIO driver > as bool/builtin > > --- > > Rajmohan Mani (3): > mfd: Add new mfd device TPS68470 > gpio: Add support for TPS68470 GPIOs > ACPI / PMIC: Add TI PMIC TPS68470 operation region driver > > drivers/acpi/Kconfig | 16 ++ > drivers/acpi/Makefile | 2 + > drivers/acpi/pmic/tps68470_pmic.c | 455 ++++++++++++++++++++++++++++++++++++++ > drivers/gpio/Kconfig | 15 ++ > drivers/gpio/Makefile | 1 + > drivers/gpio/gpio-tps68470.c | 176 +++++++++++++++ > drivers/mfd/Kconfig | 18 ++ > drivers/mfd/Makefile | 1 + > drivers/mfd/tps68470.c | 106 +++++++++ > include/linux/mfd/tps68470.h | 97 ++++++++ > 10 files changed, 887 insertions(+) > create mode 100644 drivers/acpi/pmic/tps68470_pmic.c > create mode 100644 drivers/gpio/gpio-tps68470.c > create mode 100644 drivers/mfd/tps68470.c > create mode 100644 include/linux/mfd/tps68470.h > > -- > 1.9.1 > -- With Best Regards, Andy Shevchenko