RE: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation region driver

2017-06-09 Thread Mani, Rajmohan
Hi Sakari, Andy, > Subject: Re: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation > region driver > > On Wed, Jun 07, 2017 at 11:40:27PM +0300, Andy Shevchenko wrote: > > On Wed, Jun 7, 2017 at 11:10 PM, Sakari Ailus <sakari.ai...@iki.fi> wrote: > > > O

RE: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation region driver

2017-06-09 Thread Mani, Rajmohan
Hi Sakari, Andy, > Subject: Re: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation > region driver > > On Wed, Jun 07, 2017 at 11:40:27PM +0300, Andy Shevchenko wrote: > > On Wed, Jun 7, 2017 at 11:10 PM, Sakari Ailus wrote: > > > On Wed, Jun 07, 201

RE: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation region driver

2017-06-09 Thread Mani, Rajmohan
Hi Andy, Thanks for the reviews and patience. > Subject: Re: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation > region driver > > On Wed, Jun 7, 2017 at 3:07 PM, Sakari Ailus <sakari.ai...@iki.fi> wrote: > > >> +static int ti_tps68470_pmic_get_power(

RE: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation region driver

2017-06-09 Thread Mani, Rajmohan
Hi Andy, Thanks for the reviews and patience. > Subject: Re: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation > region driver > > On Wed, Jun 7, 2017 at 3:07 PM, Sakari Ailus wrote: > > >> +static int ti_tps68470_pmic_get_power(struct

RE: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation region driver

2017-06-09 Thread Mani, Rajmohan
Hi Sakari, Andy, > Subject: Re: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation > region driver > > On Wed, Jun 07, 2017 at 04:37:12PM +0300, Andy Shevchenko wrote: > > >> +static acpi_status ti_pmic_com

RE: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation region driver

2017-06-09 Thread Mani, Rajmohan
Hi Sakari, Andy, > Subject: Re: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation > region driver > > On Wed, Jun 07, 2017 at 04:37:12PM +0300, Andy Shevchenko wrote: > > >> +static acpi_status ti_pmic_com

RE: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation region driver

2017-06-09 Thread Mani, Rajmohan
gt; a...@vger.kernel.org; Lee Jones <lee.jo...@linaro.org>; Linus Walleij > <linus.wall...@linaro.org>; Alexandre Courbot <gnu...@gmail.com>; Rafael J. > Wysocki <r...@rjwysocki.net>; Len Brown <l...@kernel.org> > Subject: Re: [PATCH v1 3/3] ACPI / PMIC:

RE: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation region driver

2017-06-09 Thread Mani, Rajmohan
Lee Jones ; Linus Walleij > ; Alexandre Courbot ; Rafael J. > Wysocki ; Len Brown > Subject: Re: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation > region driver > > Hi Rajmohan, > > Thanks for removing the redundant struct definition. A couple more comments >

RE: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation region driver

2017-06-09 Thread Mani, Rajmohan
Hi Hans, > > > > As PMICs are typically linked to the kernel (vs. being modules), > > there's no issue with the module name. I would suppose few if any > > PMICs will be compiled as modules in general. > > Good point about the OpRegion driver usually being built-in, in my experience > it > MUST

RE: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation region driver

2017-06-09 Thread Mani, Rajmohan
Hi Hans, > > > > As PMICs are typically linked to the kernel (vs. being modules), > > there's no issue with the module name. I would suppose few if any > > PMICs will be compiled as modules in general. > > Good point about the OpRegion driver usually being built-in, in my experience > it > MUST

RE: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation region driver

2017-06-09 Thread Mani, Rajmohan
dego...@redhat.com>; linux-kernel@vger.kernel.org; linux- > g...@vger.kernel.org; linux-a...@vger.kernel.org; Lee Jones > <lee.jo...@linaro.org>; Linus Walleij <linus.wall...@linaro.org>; Alexandre > Courbot <gnu...@gmail.com>; Rafael J. Wysocki <r...@rjwysocki.net>; Len >

RE: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation region driver

2017-06-09 Thread Mani, Rajmohan
@vger.kernel.org; Lee Jones > ; Linus Walleij ; Alexandre > Courbot ; Rafael J. Wysocki ; Len > Brown > Subject: Re: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation > region driver > > On Wed, Jun 07, 2017 at 11:40:27PM +0300, Andy Shevchenko wrote: > > On W

RE: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation region driver

2017-06-09 Thread Mani, Rajmohan
ux-g...@vger.kernel.org; linux- > a...@vger.kernel.org; Lee Jones <lee.jo...@linaro.org>; Linus Walleij > <linus.wall...@linaro.org>; Alexandre Courbot <gnu...@gmail.com>; Rafael J. > Wysocki <r...@rjwysocki.net>; Len Brown <l...@kernel.org> > Subject: Re: [P

RE: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation region driver

2017-06-09 Thread Mani, Rajmohan
Jones ; Linus Walleij > ; Alexandre Courbot ; Rafael J. > Wysocki ; Len Brown > Subject: Re: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation > region driver > > Hi, > > On 06/06/2017 04:23 PM, Andy Shevchenko wrote: > > +Cc Hans (that's why didn't delete anyth

RE: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation region driver

2017-06-09 Thread Mani, Rajmohan
: linux-kernel@vger.kernel.org; linux-g...@vger.kernel.org; linux- > a...@vger.kernel.org; Lee Jones <lee.jo...@linaro.org>; Linus Walleij > <linus.wall...@linaro.org>; Alexandre Courbot <gnu...@gmail.com>; Rafael J. > Wysocki <r...@rjwysocki.net>; Len Brown <l...@

RE: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation region driver

2017-06-09 Thread Mani, Rajmohan
.kernel.org; linux- > a...@vger.kernel.org; Lee Jones ; Linus Walleij > ; Alexandre Courbot ; Rafael J. > Wysocki ; Len Brown > Subject: Re: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation > region driver > > +Cc Hans (that's why didn't delete anything from original mail, just &

Re: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation region driver

2017-06-08 Thread Hans de Goede
Hi, On 07-06-17 22:10, Sakari Ailus wrote: Hi Andy, On Wed, Jun 07, 2017 at 04:40:13PM +0300, Andy Shevchenko wrote: On Wed, Jun 7, 2017 at 3:15 PM, Sakari Ailus wrote: On Tue, Jun 06, 2017 at 05:23:56PM +0300, Andy Shevchenko wrote: Follow the pattern, please, I

Re: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation region driver

2017-06-08 Thread Hans de Goede
Hi, On 07-06-17 22:10, Sakari Ailus wrote: Hi Andy, On Wed, Jun 07, 2017 at 04:40:13PM +0300, Andy Shevchenko wrote: On Wed, Jun 7, 2017 at 3:15 PM, Sakari Ailus wrote: On Tue, Jun 06, 2017 at 05:23:56PM +0300, Andy Shevchenko wrote: Follow the pattern, please, I suppose ti_pmic_tps68470.c

Re: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation region driver

2017-06-07 Thread Sakari Ailus
On Wed, Jun 07, 2017 at 11:40:27PM +0300, Andy Shevchenko wrote: > On Wed, Jun 7, 2017 at 11:10 PM, Sakari Ailus wrote: > > On Wed, Jun 07, 2017 at 04:40:13PM +0300, Andy Shevchenko wrote: > >> On Wed, Jun 7, 2017 at 3:15 PM, Sakari Ailus wrote: > >> >

Re: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation region driver

2017-06-07 Thread Sakari Ailus
On Wed, Jun 07, 2017 at 11:40:27PM +0300, Andy Shevchenko wrote: > On Wed, Jun 7, 2017 at 11:10 PM, Sakari Ailus wrote: > > On Wed, Jun 07, 2017 at 04:40:13PM +0300, Andy Shevchenko wrote: > >> On Wed, Jun 7, 2017 at 3:15 PM, Sakari Ailus wrote: > >> > On Tue, Jun 06, 2017 at 05:23:56PM +0300,

Re: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation region driver

2017-06-07 Thread Andy Shevchenko
On Wed, Jun 7, 2017 at 11:10 PM, Sakari Ailus wrote: > On Wed, Jun 07, 2017 at 04:40:13PM +0300, Andy Shevchenko wrote: >> On Wed, Jun 7, 2017 at 3:15 PM, Sakari Ailus wrote: >> > On Tue, Jun 06, 2017 at 05:23:56PM +0300, Andy Shevchenko wrote: >> >>

Re: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation region driver

2017-06-07 Thread Andy Shevchenko
On Wed, Jun 7, 2017 at 11:10 PM, Sakari Ailus wrote: > On Wed, Jun 07, 2017 at 04:40:13PM +0300, Andy Shevchenko wrote: >> On Wed, Jun 7, 2017 at 3:15 PM, Sakari Ailus wrote: >> > On Tue, Jun 06, 2017 at 05:23:56PM +0300, Andy Shevchenko wrote: >> >> Follow the pattern, please, I suppose >> >>

Re: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation region driver

2017-06-07 Thread Sakari Ailus
Hi Andy, On Wed, Jun 07, 2017 at 04:40:13PM +0300, Andy Shevchenko wrote: > On Wed, Jun 7, 2017 at 3:15 PM, Sakari Ailus wrote: > > On Tue, Jun 06, 2017 at 05:23:56PM +0300, Andy Shevchenko wrote: > >> Follow the pattern, please, I suppose > >> ti_pmic_tps68470.c > > > >

Re: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation region driver

2017-06-07 Thread Sakari Ailus
Hi Andy, On Wed, Jun 07, 2017 at 04:40:13PM +0300, Andy Shevchenko wrote: > On Wed, Jun 7, 2017 at 3:15 PM, Sakari Ailus wrote: > > On Tue, Jun 06, 2017 at 05:23:56PM +0300, Andy Shevchenko wrote: > >> Follow the pattern, please, I suppose > >> ti_pmic_tps68470.c > > > > This pattern is weird.

Re: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation region driver

2017-06-07 Thread Sakari Ailus
On Wed, Jun 07, 2017 at 04:37:12PM +0300, Andy Shevchenko wrote: > >> +static acpi_status ti_pmic_common_handler(u32 function, > > + acpi_physical_address address, > > + u32 bits, u64 *value, > > +

Re: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation region driver

2017-06-07 Thread Sakari Ailus
On Wed, Jun 07, 2017 at 04:37:12PM +0300, Andy Shevchenko wrote: > >> +static acpi_status ti_pmic_common_handler(u32 function, > > + acpi_physical_address address, > > + u32 bits, u64 *value, > > +

Re: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation region driver

2017-06-07 Thread Andy Shevchenko
On Wed, Jun 7, 2017 at 3:15 PM, Sakari Ailus wrote: > On Tue, Jun 06, 2017 at 05:23:56PM +0300, Andy Shevchenko wrote: >> Follow the pattern, please, I suppose >> ti_pmic_tps68470.c > > This pattern is weird. "ti" in front of the file name is redundant, and in > very few

Re: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation region driver

2017-06-07 Thread Andy Shevchenko
On Wed, Jun 7, 2017 at 3:15 PM, Sakari Ailus wrote: > On Tue, Jun 06, 2017 at 05:23:56PM +0300, Andy Shevchenko wrote: >> Follow the pattern, please, I suppose >> ti_pmic_tps68470.c > > This pattern is weird. "ti" in front of the file name is redundant, and in > very few places the vendor prefix

Re: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation region driver

2017-06-07 Thread Andy Shevchenko
On Wed, Jun 7, 2017 at 3:07 PM, Sakari Ailus wrote: >> +static int ti_tps68470_pmic_get_power(struct regmap *regmap, int reg, >> +int bitmask, u64 *value) >> +{ >> + int data; > > Shouldn't you use unsigned int here? Same in the

Re: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation region driver

2017-06-07 Thread Andy Shevchenko
On Wed, Jun 7, 2017 at 3:07 PM, Sakari Ailus wrote: >> +static int ti_tps68470_pmic_get_power(struct regmap *regmap, int reg, >> +int bitmask, u64 *value) >> +{ >> + int data; > > Shouldn't you use unsigned int here? Same in the functions below. +1,

Re: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation region driver

2017-06-07 Thread Sakari Ailus
On Tue, Jun 06, 2017 at 05:23:56PM +0300, Andy Shevchenko wrote: > Follow the pattern, please, I suppose > ti_pmic_tps68470.c This pattern is weird. "ti" in front of the file name is redundant, and in very few places the vendor prefix is used anyway. Especially when the chip has a proper name ---

Re: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation region driver

2017-06-07 Thread Sakari Ailus
On Tue, Jun 06, 2017 at 05:23:56PM +0300, Andy Shevchenko wrote: > Follow the pattern, please, I suppose > ti_pmic_tps68470.c This pattern is weird. "ti" in front of the file name is redundant, and in very few places the vendor prefix is used anyway. Especially when the chip has a proper name ---

Re: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation region driver

2017-06-07 Thread Sakari Ailus
Hi Rajmohan, Thanks for removing the redundant struct definition. A couple more comments below. Not really necessarily bugs but a few things to clean things up a bit. On Tue, Jun 06, 2017 at 04:55:18AM -0700, Rajmohan Mani wrote: > The Kabylake platform coreboot (Chrome OS equivalent of > BIOS)

Re: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation region driver

2017-06-07 Thread Sakari Ailus
Hi Rajmohan, Thanks for removing the redundant struct definition. A couple more comments below. Not really necessarily bugs but a few things to clean things up a bit. On Tue, Jun 06, 2017 at 04:55:18AM -0700, Rajmohan Mani wrote: > The Kabylake platform coreboot (Chrome OS equivalent of > BIOS)

Re: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation region driver

2017-06-06 Thread Hans de Goede
Hi, On 06/06/2017 04:23 PM, Andy Shevchenko wrote: +Cc Hans (that's why didn't delete anything from original mail, just adding my comments). Hans, if you have few minutes it would be appreciated to glance on the below for some issues if any since you did pass quite a good quest with other PMIC

Re: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation region driver

2017-06-06 Thread Hans de Goede
Hi, On 06/06/2017 04:23 PM, Andy Shevchenko wrote: +Cc Hans (that's why didn't delete anything from original mail, just adding my comments). Hans, if you have few minutes it would be appreciated to glance on the below for some issues if any since you did pass quite a good quest with other PMIC

Re: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation region driver

2017-06-06 Thread Andy Shevchenko
+Cc Hans (that's why didn't delete anything from original mail, just adding my comments). Hans, if you have few minutes it would be appreciated to glance on the below for some issues if any since you did pass quite a good quest with other PMIC drivers. On Tue, Jun 6, 2017 at 2:55 PM, Rajmohan

Re: [PATCH v1 3/3] ACPI / PMIC: Add TI PMIC TPS68470 operation region driver

2017-06-06 Thread Andy Shevchenko
+Cc Hans (that's why didn't delete anything from original mail, just adding my comments). Hans, if you have few minutes it would be appreciated to glance on the below for some issues if any since you did pass quite a good quest with other PMIC drivers. On Tue, Jun 6, 2017 at 2:55 PM, Rajmohan