| Message ID | 504A2FF2.7070607@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hello Benoit, Le 07/09/2012 19:33, Benoit Cousson a écrit : > Hi Florian, > > I've just noticed that this patch is reporting some CHECK issues. > > d1f5052 - gpio/twl4030: get platform data from device tree > CHECK: Alignment should match open parenthesis > #66: FILE: drivers/gpio/gpio-twl4030.c:412: > + of_property_read_u32(dev->of_node, "ti,debounce", > + &omap_twl_info->debounce); > > CHECK: Alignment should match open parenthesis > #68: FILE: drivers/gpio/gpio-twl4030.c:414: > + of_property_read_u32(dev->of_node, "ti,mmc-cd", > + (u32 *)&omap_twl_info->mmc_cd); > > CHECK: Alignment should match open parenthesis > #70: FILE: drivers/gpio/gpio-twl4030.c:416: > + of_property_read_u32(dev->of_node, "ti,pullups", > + &omap_twl_info->pullups); > > CHECK: Alignment should match open parenthesis > #72: FILE: drivers/gpio/gpio-twl4030.c:418: > + of_property_read_u32(dev->of_node, "ti,pulldowns", > + &omap_twl_info->pulldowns); > > CHECK: Alignment should match open parenthesis > + pdata->pullups, pdata->pulldowns, > > CHECK: Alignment should match open parenthesis > #139: FILE: drivers/gpio/gpio-twl4030.c:479: > + dev_dbg(&pdev->dev, "debounce %.03x %.01x --> %d\n", > + pdata->debounce, pdata->mmc_cd, > > total: 0 errors, 0 warnings, 6 checks, 118 lines checked > > > I fixed them since it was trivial, but next time you should ensure that the patch pass checkpatch before posting. Sorry for these errors. I however checked my patches before submitting, and had no such warnings. I redone, and remarked that these warnings appear only with the "--strict" option, which is not enabled by default. Is this the recommended guideline? Thus why not enabling it by default? > > Just let me know if you have any issue with the following update. I will test, but should not have any issue with your fix. Thank you very much for fixing my patch, I send you a virtual chocolate from Switzerland! Regards, Florian -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Florian, On 09/10/2012 10:17 AM, Florian Vaussard wrote: > Hello Benoit, > > Le 07/09/2012 19:33, Benoit Cousson a écrit : >> Hi Florian, >> >> I've just noticed that this patch is reporting some CHECK issues. >> >> d1f5052 - gpio/twl4030: get platform data from device tree >> CHECK: Alignment should match open parenthesis >> #66: FILE: drivers/gpio/gpio-twl4030.c:412: >> + of_property_read_u32(dev->of_node, "ti,debounce", >> + &omap_twl_info->debounce); >> >> CHECK: Alignment should match open parenthesis >> #68: FILE: drivers/gpio/gpio-twl4030.c:414: >> + of_property_read_u32(dev->of_node, "ti,mmc-cd", >> + (u32 *)&omap_twl_info->mmc_cd); >> >> CHECK: Alignment should match open parenthesis >> #70: FILE: drivers/gpio/gpio-twl4030.c:416: >> + of_property_read_u32(dev->of_node, "ti,pullups", >> + &omap_twl_info->pullups); >> >> CHECK: Alignment should match open parenthesis >> #72: FILE: drivers/gpio/gpio-twl4030.c:418: >> + of_property_read_u32(dev->of_node, "ti,pulldowns", >> + &omap_twl_info->pulldowns); >> >> CHECK: Alignment should match open parenthesis >> + pdata->pullups, pdata->pulldowns, >> >> CHECK: Alignment should match open parenthesis >> #139: FILE: drivers/gpio/gpio-twl4030.c:479: >> + dev_dbg(&pdev->dev, "debounce %.03x %.01x --> %d\n", >> + pdata->debounce, pdata->mmc_cd, >> >> total: 0 errors, 0 warnings, 6 checks, 118 lines checked >> >> >> I fixed them since it was trivial, but next time you should ensure >> that the patch pass checkpatch before posting. > > Sorry for these errors. I however checked my patches before submitting, > and had no such warnings. I redone, and remarked that these warnings > appear only with the "--strict" option, which is not enabled by default. > Is this the recommended guideline? Thus why not enabling it by default? That's a pretty good question :-) Maybe the --strict is more a nice to have than a strong requirement? Anyway, we'd better run checkpatch with --strict and thus fix any cosmetic details that might be in the patch. >> Just let me know if you have any issue with the following update. > > I will test, but should not have any issue with your fix. > > Thank you very much for fixing my patch, I send you a virtual chocolate > from Switzerland! Cool, I love chocolate. That being said, I'm not sure how tasteful will be the *virtual* chocolate. Regards, Benoit -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello Benoit, >> >> I fixed them since it was trivial, but next time you should ensure >> that the patch pass checkpatch before posting. >> Sorry for these errors. I however checked my patches before submitting, >> and had no such warnings. I redone, and remarked that these warnings >> appear only with the "--strict" option, which is not enabled by default. >> Is this the recommended guideline? Thus why not enabling it by default? > That's a pretty good question :-) > > Maybe the --strict is more a nice to have than a strong requirement? > Anyway, we'd better run checkpatch with --strict and thus fix any > cosmetic details that might be in the patch. Good to know, I will use --strict for the next submissions. > Cool, I love chocolate. That being said, I'm not sure how tasteful > will be the *virtual* chocolate. Regards, Benoit Even *virtual*, Swiss chocolate will be the most tasteful (hope no Belgium friends will see this thread) :-) Regards, Florian -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/gpio/gpio-twl4030.txt b/Documentation/devicetree/bindings/gpio/gpio-twl4030.txt index 16695d9..66788fd 100644 --- a/Documentation/devicetree/bindings/gpio/gpio-twl4030.txt +++ b/Documentation/devicetree/bindings/gpio/gpio-twl4030.txt @@ -11,6 +11,11 @@ Required properties: - interrupt-controller: Mark the device node as an interrupt controller The first cell is the GPIO number. The second cell is not used. +- ti,use-leds : Enables LEDA and LEDB outputs if set +- ti,debounce : if n-th bit is set, debounces GPIO-n +- ti,mmc-cd : if n-th bit is set, GPIO-n controls VMMC(n+1) +- ti,pullups : if n-th bit is set, set a pullup on GPIO-n +- ti,pulldowns : if n-th bit is set, set a pulldown on GPIO-n Example: @@ -20,4 +25,5 @@ twl_gpio: gpio { gpio-controller; #interrupt-cells = <2>; interrupt-controller; + ti,use-leds; }; diff --git a/drivers/gpio/gpio-twl4030.c b/drivers/gpio/gpio-twl4030.c index 94256fe..f923252 100644 --- a/drivers/gpio/gpio-twl4030.c +++ b/drivers/gpio/gpio-twl4030.c @@ -395,6 +395,31 @@ static int __devinit gpio_twl4030_debounce(u32 debounce, u8 mmc_cd) static int gpio_twl4030_remove(struct platform_device *pdev); +static struct twl4030_gpio_platform_data *of_gpio_twl4030(struct device *dev) +{ + struct twl4030_gpio_platform_data *omap_twl_info; + + omap_twl_info = devm_kzalloc(dev, sizeof(*omap_twl_info), GFP_KERNEL); + if (!omap_twl_info) + return NULL; + + omap_twl_info->gpio_base = -1; + + omap_twl_info->use_leds = of_property_read_bool(dev->of_node, + "ti,use-leds"); + + of_property_read_u32(dev->of_node, "ti,debounce", + &omap_twl_info->debounce); + of_property_read_u32(dev->of_node, "ti,mmc-cd", + (u32 *)&omap_twl_info->mmc_cd); + of_property_read_u32(dev->of_node, "ti,pullups", + &omap_twl_info->pullups); + of_property_read_u32(dev->of_node, "ti,pulldowns", + &omap_twl_info->pulldowns); + + return omap_twl_info; +} + static int __devinit gpio_twl4030_probe(struct platform_device *pdev) { struct twl4030_gpio_platform_data *pdata = pdev->dev.platform_data; @@ -423,39 +448,42 @@ static int __devinit gpio_twl4030_probe(struct platform_device *pdev) twl4030_gpio_irq_base = irq_base; no_irqs: - twl_gpiochip.base = -1; twl_gpiochip.ngpio = TWL4030_GPIO_MAX; twl_gpiochip.dev = &pdev->dev; - if (pdata) { - twl_gpiochip.base = pdata->gpio_base; + if (node) + pdata = of_gpio_twl4030(&pdev->dev); - /* - * NOTE: boards may waste power if they don't set pullups - * and pulldowns correctly ... default for non-ULPI pins is - * pulldown, and some other pins may have external pullups - * or pulldowns. Careful! - */ - ret = gpio_twl4030_pulls(pdata->pullups, pdata->pulldowns); - if (ret) - dev_dbg(&pdev->dev, "pullups %.05x %.05x --> %d\n", - pdata->pullups, pdata->pulldowns, - ret); - - ret = gpio_twl4030_debounce(pdata->debounce, pdata->mmc_cd); - if (ret) - dev_dbg(&pdev->dev, "debounce %.03x %.01x --> %d\n", - pdata->debounce, pdata->mmc_cd, - ret); - - /* - * NOTE: we assume VIBRA_CTL.VIBRA_EN, in MODULE_AUDIO_VOICE, - * is (still) clear if use_leds is set. - */ - if (pdata->use_leds) - twl_gpiochip.ngpio += 2; + if (pdata == NULL) { + dev_err(&pdev->dev, "Platform data is missing\n"); + return -ENXIO; } + twl_gpiochip.base = pdata->gpio_base; + + /* + * NOTE: boards may waste power if they don't set pullups + * and pulldowns correctly ... default for non-ULPI pins is + * pulldown, and some other pins may have external pullups + * or pulldowns. Careful! + */ + ret = gpio_twl4030_pulls(pdata->pullups, pdata->pulldowns); + if (ret) + dev_dbg(&pdev->dev, "pullups %.05x %.05x --> %d\n", + pdata->pullups, pdata->pulldowns, ret); + + ret = gpio_twl4030_debounce(pdata->debounce, pdata->mmc_cd); + if (ret) + dev_dbg(&pdev->dev, "debounce %.03x %.01x --> %d\n", + pdata->debounce, pdata->mmc_cd, ret); + + /* + * NOTE: we assume VIBRA_CTL.VIBRA_EN, in MODULE_AUDIO_VOICE, + * is (still) clear if use_leds is set. + */ + if (pdata->use_leds) + twl_gpiochip.ngpio += 2; + ret = gpiochip_add(&twl_gpiochip); if (ret < 0) { dev_err(&pdev->dev, "could not register gpiochip, %d\n", ret);