From: jon-hunter@ti•com (Jon Hunter)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH 01/19] ARM: OMAP4: PM: save/restore all DPLL settings in OFF mode
Date: Wed, 25 Apr 2012 10:12:56 -0500 [thread overview]
Message-ID: <4F981478.3040201@ti.com> (raw)
In-Reply-To: <1335339230.2149.110.camel@sokoban>
Hi Tero,
On 04/25/2012 02:33 AM, Tero Kristo wrote:
> On Mon, 2012-04-23 at 11:09 -0500, Jon Hunter wrote:
>> Hi Tero,
>>
>> On 04/20/2012 04:33 AM, Tero Kristo wrote:
>>
>> [...]
>>
>>> +/**
>>> + * omap4_dpll_print_reg - dump out a single DPLL register value
>>> + * @dpll_reg: register to dump
>>> + * @name: name of the register
>>> + * @tuple: content of the register
>>> + *
>>> + * Helper dump function to print out a DPLL register value in case
>>> + * of restore failures.
>>> + */
>>> +static void omap4_dpll_print_reg(struct omap4_dpll_regs *dpll_reg, char *name,
>>> + struct dpll_reg *tuple)
>>> +{
>>> + if (tuple->offset)
>>> + pr_warn("%s - Address offset = 0x%08x, value=0x%08x\n", name,
>>> + tuple->offset, tuple->val);
>>
>> Minor nit-pick here ...
>>
>> 1. Offset is 16-bits and so we can just have "offset = 0x%04x"
>> 2. Consider dropping "Address" from "Address offset"
>> 3. Can we be consistent in our spaces for "offset = " and "value="?
>>
>>> +}
>>> +
>>> +/*
>>> + * omap4_dpll_dump_regs - dump out DPLL registers
>>> + * @dpll_reg: DPLL to dump
>>> + *
>>> + * Dump out the contents of the registers for a DPLL. Called if a
>>> + * restore for DPLL fails to lock.
>>> + */
>>> +static void omap4_dpll_dump_regs(struct omap4_dpll_regs *dpll_reg)
>>> +{
>>> + pr_warn("%s: Unable to lock dpll %s[part=%x inst=%x]:\n",
>>> + __func__, dpll_reg->name, dpll_reg->mod_partition,
>>> + dpll_reg->mod_inst);
>>> + omap4_dpll_print_reg(dpll_reg, "clksel", &dpll_reg->clksel);
>>> + omap4_dpll_print_reg(dpll_reg, "div_m2", &dpll_reg->div_m2);
>>> + omap4_dpll_print_reg(dpll_reg, "div_m3", &dpll_reg->div_m3);
>>> + omap4_dpll_print_reg(dpll_reg, "div_m4", &dpll_reg->div_m4);
>>> + omap4_dpll_print_reg(dpll_reg, "div_m5", &dpll_reg->div_m5);
>>> + omap4_dpll_print_reg(dpll_reg, "div_m6", &dpll_reg->div_m6);
>>> + omap4_dpll_print_reg(dpll_reg, "div_m7", &dpll_reg->div_m7);
>>> + omap4_dpll_print_reg(dpll_reg, "clkdcoldo", &dpll_reg->clkdcoldo);
>>> + omap4_dpll_print_reg(dpll_reg, "clkmode", &dpll_reg->clkmode);
>>> + omap4_dpll_print_reg(dpll_reg, "autoidle", &dpll_reg->autoidle);
>>> + if (dpll_reg->idlest.offset)
>>> + pr_warn("idlest - Address offset = 0x%08x, before val=0x%08x"
>>> + " after = 0x%08x\n", dpll_reg->idlest.offset,
>>> + dpll_reg->idlest.val,
>>> + omap4_dpll_read_reg(dpll_reg, &dpll_reg->idlest));
>>
>> Nit-pick ...
>>
>> 1. Same comments as above
>> 2. Consider dropping "Address offset" altogether here as we print
>> "idlest" the offset should be implied.
>> 3. Also can we be consistent in our naming of "before val and "after"?
>> For example, "val before =" and "val now = ".
>
> Okay, I'll modify both prints slightly. Question though, do we want
> these prints in the kernel in the first place? At least Tony has been
> frowning upon register dumps in the kernel and this falls into that
> category. What we could do is just to print the warning but drop the
> register dumps out.
Thanks. Good question. If we are not seeing failures often, and I would
hope not, probably ok to remove. However, I will let Tony comment here ...
Cheers
Jon
next prev parent reply other threads:[~2012-04-25 15:12 UTC|newest]
Thread overview: 63+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-04-20 9:33 [PATCH 00/19] ARM: OMAP4 device off support Tero Kristo
2012-04-20 9:33 ` [PATCH 01/19] ARM: OMAP4: PM: save/restore all DPLL settings in OFF mode Tero Kristo
2012-04-23 16:09 ` Jon Hunter
2012-04-25 7:33 ` Tero Kristo
2012-04-25 15:12 ` Jon Hunter [this message]
2012-05-04 19:22 ` Tony Lindgren
2012-05-02 10:10 ` Bedia, Vaibhav
2012-05-02 10:18 ` Shilimkar, Santosh
2012-05-02 10:55 ` Bedia, Vaibhav
2012-05-02 11:00 ` Shilimkar, Santosh
2012-05-02 11:40 ` Bedia, Vaibhav
2012-05-02 11:46 ` Shilimkar, Santosh
2012-05-02 11:55 ` Bedia, Vaibhav
2012-05-02 11:47 ` Menon, Nishanth
2012-05-02 11:55 ` Bedia, Vaibhav
2012-05-02 11:58 ` Shilimkar, Santosh
2012-05-02 12:10 ` Bedia, Vaibhav
2012-04-20 9:33 ` [PATCH 02/19] ARM: OMAP4: PM: save/restore all CM1/2 " Tero Kristo
2012-05-09 23:27 ` Kevin Hilman
2012-05-11 14:30 ` Tero Kristo
2012-04-20 9:33 ` [PATCH 03/19] ARM: OMAP4: PM: powerdomain: Add HWSAR flag to L3INIT Tero Kristo
2012-04-20 9:33 ` [PATCH 04/19] ARM: OMAP4: Add SAR ROM base address Tero Kristo
2012-04-20 9:33 ` [PATCH 05/19] ARM: OMAP4: PM: Add SAR backup support towards device OFF Tero Kristo
2012-04-24 16:35 ` Tony Lindgren
2012-04-25 7:18 ` Tero Kristo
2012-04-20 9:33 ` [PATCH 06/19] ARM: OMAP4: Auto generate SAR layout contents Tero Kristo
2012-04-24 16:37 ` Tony Lindgren
2012-04-20 9:33 ` [PATCH 07/19] ARM: OMAP4: SAR: generate overwrite data based on SAR ROM contents Tero Kristo
2012-04-20 9:33 ` [PATCH 08/19] ARM: OMAP4: PM: Add device-off support Tero Kristo
2012-04-24 17:46 ` Jon Hunter
2012-04-25 7:30 ` Tero Kristo
2012-04-20 9:33 ` [PATCH 09/19] ARM: OMAP4: PM: add errata support Tero Kristo
2012-04-20 9:33 ` [PATCH 10/19] ARM: OMAP4: PM: Work-around for ROM code BUG of IVAHD/TESLA Tero Kristo
2012-04-24 17:50 ` Jon Hunter
2012-04-25 7:31 ` Tero Kristo
2012-04-20 9:33 ` [PATCH 11/19] ARM: OMAP4: PM: save/restore CM L3INSTR registers when MPU hits OSWR/OFF mode Tero Kristo
2012-04-24 17:57 ` Jon Hunter
2012-04-25 7:31 ` Tero Kristo
2012-04-20 9:33 ` [PATCH 12/19] ARM: OMAP4: PM: update ROM return address for OSWR and OFF Tero Kristo
2012-04-24 16:39 ` Tony Lindgren
2012-04-25 7:24 ` Tero Kristo
2012-04-20 9:33 ` [PATCH 13/19] ARM: OMAP4: PM: Mark the PPI and SPI interrupts as non-secure for GP Tero Kristo
2012-04-20 9:33 ` [PATCH 14/19] ARM: OMAP4: wakeupgen: enable clocks for save_secure_all Tero Kristo
2012-04-20 9:33 ` [PATCH 15/19] ARM: OMAP4430: PM: workaround for DDR corruption on second CS Tero Kristo
2012-04-24 18:22 ` Jon Hunter
2012-04-25 7:26 ` Tero Kristo
2012-04-25 7:59 ` Shilimkar, Santosh
2012-04-25 15:16 ` Jon Hunter
2012-04-26 6:19 ` Shilimkar, Santosh
2012-04-20 9:33 ` [PATCH 16/19] TEMP: ARM: OMAP4: prevent voltage transitions Tero Kristo
2012-04-20 9:33 ` [PATCH 17/19] ARM: OMAP4: put cpu1 back to sleep if no wake request Tero Kristo
2012-04-20 9:33 ` [PATCH 18/19] ARM: OMAP4460: wakeupgen: set GIC_CPU0 backup status flag always Tero Kristo
2012-04-20 9:33 ` [PATCH 19/19] ARM: OMAP4: powerdomain: update mpu / core off counters during device off Tero Kristo
2012-04-20 12:20 ` [PATCH 00/19] ARM: OMAP4 device off support T Krishnamoorthy, Balaji
2012-04-20 12:58 ` Tero Kristo
2012-04-20 13:55 ` Kevin Hilman
2012-04-20 14:43 ` Tero Kristo
2012-04-20 14:51 ` Datta, Shubhrajyoti
2012-04-20 15:07 ` Tero Kristo
2012-04-23 6:28 ` Shubhrajyoti Datta
2012-05-09 22:46 ` Kevin Hilman
2012-05-09 23:14 ` Russell King - ARM Linux
2012-05-10 9:47 ` Tero Kristo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4F981478.3040201@ti.com \
--to=jon-hunter@ti$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox