linux-kernel - Re: [PATCH 4/7] mfd: hi655x: Add hi665x pmic driver

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20151105143010.GJ18409@sirena.org.uk>
Date:	Thu, 5 Nov 2015 14:30:10 +0000
From:	Mark Brown <broonie@...nel.org>
To:	Chen Feng <puck.chen@...ilicon.com>
Cc:	w.f@...wei.com, sameo@...ux.intel.com, lee.jones@...aro.org,
	linux-kernel@...r.kernel.org, lgirdwood@...il.com,
	robh+dt@...nel.org, pawel.moll@....com, mark.rutland@....com,
	ijc+devicetree@...lion.org.uk, galak@...eaurora.org,
	joro@...tes.org, iommu@...ts.linux-foundation.org,
	haojian.zhuang@...aro.org, devicetree@...r.kernel.org,
	xuwei5@...ilicon.com, xuyiping@...ilicon.com,
	kong.kongxinwei@...ilicon.com, z.liuxinliang@...ilicon.com,
	yudongbin@...ilicon.com, weidong2@...ilicon.com,
	saberlily.xia@...ilicon.com, haojian.zhuang@...look.com,
	leo.yan@...aro.org, linuxarm@...wei.com, dan.zhao@...ilicon.com,
	peter.panshilin@...ilicon.com, qijiwen@...ilicon.com
Subject: Re: [PATCH 4/7] mfd: hi655x: Add hi665x pmic driver

On Thu, Nov 05, 2015 at 09:34:45PM +0800, Chen Feng wrote:

> +config MFD_HI655X_PMIC
> +        bool "HiSilicon Hi655X series PMU/Codec IC"

Why is this bool and not tristate?

> +        depends on ARCH_HISI

Can we have an || COMPILE_TEST here?

> +static irqreturn_t hi655x_pmic_irq_handler(int irq, void *data)
> +{
> +	struct hi655x_pmic *pmic = (struct hi655x_pmic *)data;
> +	u32 pending;
> +	u32 ret = IRQ_NONE;
> +	unsigned long offset;
> +	int i;

This looks like you should be able to use regmap_irq?

> +static int hi655x_pmic_remove(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct hi655x_pmic *pmic = platform_get_drvdata(pdev);
> +
> +	free_irq(pmic->irq, pmic);
> +	gpio_free(pmic->gpio);
> +	devm_release_mem_region(dev, pmic->res->start,
> +				resource_size(pmic->res));
> +	devm_kfree(dev, pmic);
> +	platform_set_drvdata(pdev, NULL);

There is no point in using devm_ cleanup functions in the device removal
path unless there's some ordering issue with respect to other stuff
which doesn't seem to be the case here.

> +static struct platform_driver hi655x_pmic_driver = {
> +	.driver	= {
> +		.name =	"hisi,hi655x-pmic",

We don't normally use OF style names in the Linux driver names.

Download attachment "signature.asc" of type "application/pgp-signature" (474 bytes)