[PATCH 2/2] arm: omap: gpmc-smsc911x: minor style fixes

[nm@ti.com (Menon, Nishanth)]

On Tue, Apr 26, 2011 at 09:40, Igor Grinberg <grinberg@compulab.co.il> wrote:
> Hi Nishanth,
>
> On 04/26/11 16:45, Menon, Nishanth wrote:
>
>> On Thu, Apr 21, 2011 at 09:50, Igor Grinberg <grinberg@compulab.co.il> wrote:
>>> replace "printk(KERN_ERR" by "pr_err("
>>> and fix needlessly multi-lined #ifdef
>>>
>>> Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
>>> ---
>>> ?arch/arm/mach-omap2/gpmc-smsc911x.c ? ? ? ? ? ? | ? 14 +++++++-------
>>> ?arch/arm/plat-omap/include/plat/gpmc-smsc911x.h | ? ?3 +--
>>> ?2 files changed, 8 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-omap2/gpmc-smsc911x.c b/arch/arm/mach-omap2/gpmc-smsc911x.c
>>> index d30293a..b45efff 100644
>>> --- a/arch/arm/mach-omap2/gpmc-smsc911x.c
>>> +++ b/arch/arm/mach-omap2/gpmc-smsc911x.c
>> minor suggestion: wont using pr_fmt help to reduce the need to add %s,
>> __func__ for pr_err through out the file?
>
> I don't understand of what "need" are you talking about.
> I don't know of any need to add "%s, __func__" with pr_fmt...
> I've added "%s, __func__" as a meter of choice.
> IMO, it makes it easier to parse the dmesg output.
> If anyone objects it, I can remove them,
> but I think both pr_fmt and __func__ are nice here and
> way better then "printk(KERN_*" with embedded "smsc911x".
Allow me to rephrase.
I like the change of printk->pr_err - thanks for doing it. just
suggesting a minor improvement

if you add the following line before the #includes
#define pr_fmt(fmt) "%s: " fmt, __func__
then
-               printk(KERN_ERR "Failed to request GPIO%d for smsc911x IRQ\n",
-                               gpmc_cfg->gpio_irq);
+               pr_err("%s: Failed to request IRQ GPIO%d\n", __func__,
+                      gpmc_cfg->gpio_irq);
becomes,
-               printk(KERN_ERR "Failed to request GPIO%d for smsc911x IRQ\n",
-                               gpmc_cfg->gpio_irq);
+               pr_err("Failed to request IRQ GPIO%d\n", gpmc_cfg->gpio_irq);

Both give you exactly the output you would like to see (which I
personally prefer as well), but the second could be lesser lines of
code change ;)

Regards,
Nishanth Menon

>
>>> @@ -55,7 +55,7 @@ void __init gpmc_smsc911x_init(struct omap_smsc911x_platform_data *board_data)
>>> ? ? ? ?gpmc_cfg = board_data;
>>>
>>> ? ? ? ?if (gpmc_cs_request(gpmc_cfg->cs, SZ_16M, &cs_mem_base) < 0) {
>>> - ? ? ? ? ? ? ? printk(KERN_ERR "Failed to request GPMC mem for smsc911x\n");
>>> + ? ? ? ? ? ? ? pr_err("%s: Failed to request GPMC mem region\n", __func__);
>>> ? ? ? ? ? ? ? ?return;
>>> ? ? ? ?}
>>>
>>> @@ -63,8 +63,8 @@ void __init gpmc_smsc911x_init(struct omap_smsc911x_platform_data *board_data)
>>> ? ? ? ?gpmc_smsc911x_resources[0].end = cs_mem_base + 0xff;
>>>
>>> ? ? ? ?if (gpio_request(gpmc_cfg->gpio_irq, "smsc911x irq") < 0) {
>>> - ? ? ? ? ? ? ? printk(KERN_ERR "Failed to request GPIO%d for smsc911x IRQ\n",
>>> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? gpmc_cfg->gpio_irq);
>>> + ? ? ? ? ? ? ? pr_err("%s: Failed to request IRQ GPIO%d\n", __func__,
>>> + ? ? ? ? ? ? ? ? ? ? ?gpmc_cfg->gpio_irq);
>>> ? ? ? ? ? ? ? ?goto free1;
>>> ? ? ? ?}
>>>
>>> @@ -74,8 +74,8 @@ void __init gpmc_smsc911x_init(struct omap_smsc911x_platform_data *board_data)
>>> ? ? ? ?if (gpio_is_valid(gpmc_cfg->gpio_reset)) {
>>> ? ? ? ? ? ? ? ?ret = gpio_request(gpmc_cfg->gpio_reset, "smsc911x reset");
>>> ? ? ? ? ? ? ? ?if (ret) {
>>> - ? ? ? ? ? ? ? ? ? ? ? printk(KERN_ERR "Failed to request GPIO%d for smsc911x reset\n",
>>> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? gpmc_cfg->gpio_reset);
>>> + ? ? ? ? ? ? ? ? ? ? ? pr_err("%s: Failed to request reset GPIO%d\n", __func__,
>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?gpmc_cfg->gpio_reset);
>>> ? ? ? ? ? ? ? ? ? ? ? ?goto free2;
>>> ? ? ? ? ? ? ? ?}
>>>
>>> @@ -92,7 +92,7 @@ void __init gpmc_smsc911x_init(struct omap_smsc911x_platform_data *board_data)
>>> ? ? ? ? ? ? ? ? gpmc_smsc911x_resources, ARRAY_SIZE(gpmc_smsc911x_resources),
>>> ? ? ? ? ? ? ? ? &gpmc_smsc911x_config, sizeof(gpmc_smsc911x_config));
>>> ? ? ? ?if (!pdev) {
>>> - ? ? ? ? ? ? ? printk(KERN_ERR "Unable to register smsc911x device\n");
>>> + ? ? ? ? ? ? ? pr_err("%s: Unable to register platform device\n", __func__);
>>> ? ? ? ? ? ? ? ?gpio_free(gpmc_cfg->gpio_reset);
>>> ? ? ? ? ? ? ? ?goto free2;
>>> ? ? ? ?}
>>> @@ -104,5 +104,5 @@ free2:
>>> ?free1:
>>> ? ? ? ?gpmc_cs_free(gpmc_cfg->cs);
>>>
>>> - ? ? ? printk(KERN_ERR "Could not initialize smsc911x\n");
>>> + ? ? ? pr_err("Could not initialize smsc911x\n");
>>> ?}
>>> diff --git a/arch/arm/plat-omap/include/plat/gpmc-smsc911x.h b/arch/arm/plat-omap/include/plat/gpmc-smsc911x.h
>>> index d3f1579..ea6c9c8 100644
>>> --- a/arch/arm/plat-omap/include/plat/gpmc-smsc911x.h
>>> +++ b/arch/arm/plat-omap/include/plat/gpmc-smsc911x.h
>>> @@ -21,8 +21,7 @@ struct omap_smsc911x_platform_data {
>>> ? ? ? ?u32 ? ? flags;
>>> ?};
>>>
>>> -#if defined(CONFIG_SMSC911X) || \
>>> - ? ? ? defined(CONFIG_SMSC911X_MODULE)
>>> +#if defined(CONFIG_SMSC911X) || defined(CONFIG_SMSC911X_MODULE)
>>>
>>> ?extern void gpmc_smsc911x_init(struct omap_smsc911x_platform_data *d);
>>>
>>> --
>>> 1.7.3.4
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
>>> the body of a message to majordomo at vger.kernel.org
>>> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>>>
>
> --
> Regards,
> Igor.
>
>