On Tue, Apr 26, 2011 at 09:40, Igor Grinberg <grinberg@xxxxxxxxxxxxxx> wrote:
> Hi Nishanth,
>
> On 04/26/11 16:45, Menon, Nishanth wrote:
>
>> On Thu, Apr 21, 2011 at 09:50, Igor Grinberg <grinberg@xxxxxxxxxxxxxx> wrote:
>>> replace "printk(KERN_ERR" by "pr_err("
>>> and fix needlessly multi-lined #ifdef
>>>
>>> Signed-off-by: Igor Grinberg <grinberg@xxxxxxxxxxxxxx>
>>> ---
>>> 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@xxxxxxxxxxxxxxx
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>
>
> --
> Regards,
> Igor.
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
