From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Zhu, Lejun" Subject: Re: [PATCH] gpio: Add support for Intel Crystal Cove PMIC Date: Fri, 30 May 2014 10:14:56 +0800 Message-ID: <5387E9A0.4050700@linux.intel.com> References: <1401348092-24468-1-git-send-email-lejun.zhu@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Return-path: Received: from mga09.intel.com ([134.134.136.24]:14015 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752165AbaE3CPA (ORCPT ); Thu, 29 May 2014 22:15:00 -0400 In-Reply-To: Sender: linux-gpio-owner@vger.kernel.org List-Id: linux-gpio@vger.kernel.org To: Linus Walleij Cc: Alexandre Courbot , Mika Westerberg , Mathias Nyman , "linux-gpio@vger.kernel.org" , "linux-kernel@vger.kernel.org" , jacob.jun.pan@linux.intel.com, bin.yang@intel.com On 5/29/2014 9:51 PM, Linus Walleij wrote: > On Thu, May 29, 2014 at 9:21 AM, Zhu, Lejun wrote: > >> Devices based on Intel SoC products such as Baytrail have a Power >> Management IC. In the PMIC there are subsystems for voltage regulation, >> A/D conversion, GPIO and PWMs. The PMIC in Baytrail-T platform is >> called Crystal Cove. >> >> This patch adds support for the GPIO function in Crystal Cove. >> >> Signed-off-by: Yang, Bin >> Signed-off-by: Zhu, Lejun > > You have gotten several Reviewed-by and Acked-by tags for this > driver have you not? Please collect them here so I don't have to > do it. I'll do it. >> --- >> v2: >> - Use IRQ chip helper to provide irqdomain. >> - Implement .remove and can now build as a module. >> - Various fix for unreadable or ugly code pieces. >> v3: >> - More fix in irq_handler and probe. >> v4: >> - Minor fix of one return statement. >> v5: >> - intel_soc_pmic_readb/writeb/setb/clearb are removed from the MFD >> driver, so use regmap_* API here instead. >> - Check for unsupported IRQ type and return -EINVAL. >> - Create to_cg() and to_reg() inline functions to make the code >> readable. > > Pretty cool eh? :-) > >> +#include > > As you realize I cannot merge this patch until that > header file exists, so the MFD core patch needs to go > in first. > >> + >> +#define NUM_GPIO 16 > > Too generic name, say CRYSTAL_NUM_GPIO or something. > >> + gpiochip_irqchip_add(&cg->chip, &crystalcove_irqchip, 0, >> + handle_simple_irq, IRQ_TYPE_NONE); >> + >> + retval = request_threaded_irq(irq, NULL, crystalcove_gpio_irq_handler, >> + IRQF_ONESHOT, KBUILD_MODNAME, cg); >> + >> + if (retval) { >> + dev_warn(&pdev->dev, "request irq failed: %d\n", retval); >> + goto out; >> + } >> + >> + retval = gpiochip_add(&cg->chip); >> + if (retval) { >> + dev_warn(&pdev->dev, "add gpio chip error: %d\n", retval); >> + goto out_free_irq; >> + } > > As mentioned, swap these around. > > But the driver is looking real good now, if you just fix the above > you have my Reviewed-by so you can merge it along with the > MFD patch in the MFD tree if you like. Thank you. I will ask Lee Jones if he will do that. Best Regards Lejun