linux-kernel - Re: [PATCH 3/3] gpio: msm: Add device tree and irqdomain support for gpio-msm-v2

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: <20130521210627.GA599@codeaurora.org>
Date:	Tue, 21 May 2013 14:06:27 -0700
From:	Stephen Boyd <sboyd@...eaurora.org>
To:	Rohit Vaswani <rvaswani@...eaurora.org>
Cc:	Linus Walleij <linus.walleij@...aro.org>,
	Grant Likely <grant.likely@...aro.org>,
	Rob Herring <rob.herring@...xeda.com>,
	Rob Landley <rob@...dley.net>,
	Russell King <linux@....linux.org.uk>,
	David Brown <davidb@...eaurora.org>,
	Bryan Huntsman <bryanh@...eaurora.org>,
	linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/3] gpio: msm: Add device tree and irqdomain support
 for gpio-msm-v2

> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 87d5670..f3c1978 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -165,7 +165,7 @@ config GPIO_MSM_V1
>  
>  config GPIO_MSM_V2
>  	tristate "Qualcomm MSM GPIO v2"
> -	depends on GPIOLIB && ARCH_MSM
> +	depends on GPIOLIB && ARCH_MSM && OF

This doesn't actually rely on ARCH_MSM anymore so I think we can
drop that dependency.

>  	help
>  	  Say yes here to support the GPIO interface on ARM v7 based
>  	  Qualcomm MSM chips.  Most of the pins on the MSM can be
> @@ -222,7 +229,6 @@ static void msm_gpio_update_dual_edge_pos(unsigned gpio)
>  		else
>  			set_gpio_bits(BIT(INTR_POL_CTL), GPIO_INTR_CFG(gpio));
>  		val2 = readl(GPIO_IN_OUT(gpio)) & BIT(GPIO_IN);
> -		intstat = readl(GPIO_INTR_STATUS(gpio)) & BIT(INTR_STATUS);
>  		if (intstat || val == val2)

Looks like intstat is used uninitialized now?

> +
> +static int msm_gpio_probe(struct platform_device *pdev)
> +{
> +	int i, irq, ret, ngpio;
> +	struct resource *res;
> +
> +	msm_gpio.gpio_chip.label = pdev->name;
> +	msm_gpio.gpio_chip.dev = &pdev->dev;
> +	of_property_read_u32(pdev->dev.of_node, "ngpio", &ngpio);

Fail probe if the property isn't there?

> +	msm_gpio.gpio_chip.ngpio = ngpio;
> +
> +	res = platform_get_resource(&pdev->dev, IORESOURCE_MEM, 0);
> +	if (!res) {
> +		dev_err(&pdev->dev, "%s: no mem resource\n", __func__);
> +		return -EINVAL;
> +	}
> +
> +	msm_tlmm_base = devm_ioremap_resource(pdev->dev, res);
> +	if (!msm_tlmm_base) {
> +		dev_err(&pdev->dev, "Couldn't allocate memory for msm tlmm base\n");
> +		return -ENOMEM;
> +	}

devm_ioremap_resource() returns an ERR_PTR on failure, not NULL.
Also, it already prints messages on errors so you can drop all the
prints around this. Just do

	res = platform_get_resource(&pdev->dev, IORESOURCE_MEM, 0);
	msm_tlmm_base = devm_ioremap_resource(pdev->dev, res);
	if (IS_ERR(msm_tlmm_base))
		return ERR_PTR(msm_tlmm_base);
  

>  static int __init msm_gpio_init(void)
>  {
> -	int rc;
> -
> -	rc = platform_driver_register(&msm_gpio_driver);
> -	if (!rc) {
> -		rc = platform_device_register(&msm_device_gpio);
> -		if (rc)
> -			platform_driver_unregister(&msm_gpio_driver);
> -	}
> -
> -	return rc;
> +	return platform_driver_register(&msm_gpio_driver);
>  }
>  
>  static void __exit msm_gpio_exit(void)
>  {
> -	platform_device_unregister(&msm_device_gpio);
>  	platform_driver_unregister(&msm_gpio_driver);
>  }

You could use module_platform_driver here now too.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
--
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/