From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753181AbdF2NYc (ORCPT ); Thu, 29 Jun 2017 09:24:32 -0400 Received: from mx1.redhat.com ([209.132.183.28]:54728 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752614AbdF2NYZ (ORCPT ); Thu, 29 Jun 2017 09:24:25 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 501DDAC1BF Authentication-Results: ext-mx02.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx02.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=hdegoede@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 501DDAC1BF Subject: Re: [PATCH v2 1/1] gpio: gpio-wcove: Fix GPIO control register offset calculation To: Andy Shevchenko , Kuppuswamy Sathyanarayanan Cc: Linus Walleij , "linux-gpio@vger.kernel.org" , "linux-kernel@vger.kernel.org" , Sathyanarayanan Kuppuswamy Natarajan References: <4b57167184fa9aa4d1c183d4c8df6e6a600a7dbd.1498087915.git.sathyanarayanan.kuppuswamy@linux.intel.com> From: Hans de Goede Message-ID: <893836dc-9f7d-da72-7fa0-909ec0025d5e@redhat.com> Date: Thu, 29 Jun 2017 15:24:22 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Thu, 29 Jun 2017 13:24:24 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, On 29-06-17 14:30, Andy Shevchenko wrote: > +Cc: Hans > > On Mon, Jun 26, 2017 at 8:37 PM, > wrote: >> From: Kuppuswamy Sathyanarayanan >> >> According to Whiskey Cove PMIC GPIO controller specification, for GPIO >> pins 0-12, GPIO input and output register control address range from, >> >> 0x4e44-0x4e50 for GPIO outputs control register >> >> 0x4e51-0x4e5d for GPIO input control register >> >> But, currently when calculating the GPIO register offsets in to_reg() >> function, all GPIO pins in the same bank uses the same GPIO control >> register address. This logic is incorrect. This patch fixes this >> issue. > >> >> This patch also adds support to selectively skip register modification >> for virtual GPIOs. >> >> In case of Whiskey Cove PMIC, ACPI code may use up 94 virtual GPIOs. >> These virtual GPIOs are used by the ACPI code as means to access various >> non GPIO bits of PMIC. So for these virtual GPIOs, we don't need to >> manipulate the physical GPIO pin register. A similar patch has been >> merged recently by Hans for Crystal Cove PMIC GPIO driver. You can >> find more details about it in Commit 9a752b4c9ab9 ("gpio: crystalcove: >> Do not write regular gpio registers for virtual GPIOs") > > For me (disregards to content of the patch) the question is: did we > ever have a *working* solution looking to the bug fixes on this > driver?! > > I would suggest to stop applying patches on Intel PMICs without > Tested-by tag from independent testers. > > Hans, do you have anything to add / comment on this? Yes, I noticed the driver .name = "bxt_wcove_gpio", I myself have a device with a Cherry Trail Whiskey Cove variant. For those reading along here which SoC / platform a PMIC is used on (Cherry Trail vs Broxton) may seem unrelevant. But Intel has a per platform variant of its Crystal Cove and Whiskey Cove PMICs and the platform variants are really just completely different PMICs, using incompatible registermaps for one. So I would really like us to stop referring to these devices as Whiskey Cove (or wcove) and instead name them "Cherry Trail Whiskey Cove" or cht_wc, "Bay Trail Crystal Cove" / byt_crc, etc. and do so consistently! E.g. I've recently learned that there are Cherry Trail devices with Crystal Cove PMICs (Dell Venue 8 pro 5855) and enabling the Crystal Cove PMIC ACPI Opregion on those wrecks havoc because it causes the wrong registers to get modified. Specifically the regulator control registers have slightly different addresses so we are modifying the wrong regulators! Which is not a problem, right ? Anyways back to the topic. Kuppuswamy do you have access to *Cherry Trail* Whiskey Cove documentation and can you check that the GPIO ctrl and irq registers are the same there ? IOW can you check if we can re-use this driver for the Cherry Trail Whiskey Cove PMIC ? If not then the .c file should really be renamed to drivers/gpio/gpio-bxt-wcove.c And future patches should also use gpio-bxt-wcove in their subject. With that said, the patch looks good to me. Regards, Hans