lkml.org 
[lkml]   [2014]   [May]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] gpio: Add support for Intel Crystal Cove PMIC


On 5/29/2014 9:51 PM, Linus Walleij wrote:
> On Thu, May 29, 2014 at 9:21 AM, Zhu, Lejun <lejun.zhu@linux.intel.com> 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 <bin.yang@intel.com>
>> Signed-off-by: Zhu, Lejun <lejun.zhu@linux.intel.com>
>
> 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 <linux/mfd/intel_soc_pmic.h>
>
> 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



\
 
 \ /
  Last update: 2014-05-30 05:01    [W:0.058 / U:0.032 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site