linux-kernel - Re: [PATCH] gpio: Add support for Intel Crystal Cove PMIC

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Fri, 30 May 2014 10:14:56 +0800
From:	"Zhu, Lejun" <lejun.zhu@...ux.intel.com>
To:	Linus Walleij <linus.walleij@...aro.org>
CC:	Alexandre Courbot <gnurou@...il.com>,
	Mika Westerberg <mika.westerberg@...ux.intel.com>,
	Mathias Nyman <mathias.nyman@...ux.intel.com>,
	"linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	jacob.jun.pan@...ux.intel.com, bin.yang@...el.com
Subject: Re: [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@...ux.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@...el.com>
>> Signed-off-by: Zhu, Lejun <lejun.zhu@...ux.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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/