diff for duplicates of <20100126115258.GM15759@rakim.wolfsonmicro.main> diff --git a/a/1.txt b/N1/1.txt index edc138e..4aa576e 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -1,15 +1,15 @@ On Tue, Jan 26, 2010 at 08:17:25PM +0900, jassi brar wrote: > On Tue, Jan 26, 2010 at 7:47 PM, Mark Brown -> >> + default: -> >> + s3c_ac97_cold_reset(ac97); -> >> + s3c_ac97_warm_reset(ac97); -> >> + break; -> >> + } +> >> + ? ? default: +> >> + ? ? ? ? ? ? s3c_ac97_cold_reset(ac97); +> >> + ? ? ? ? ? ? s3c_ac97_warm_reset(ac97); +> >> + ? ? ? ? ? ? break; +> >> + ? ? } > > This automatic cold and warm reset looks a bit fishy - obviously if this > > code path ever gets hit in normal operation then it's going to seriously -> > disrupt things since it'll reset the CODEC registers. A warm reset by +> > disrupt things since it'll reset the CODEC registers. ?A warm reset by > > itself wouldn't be a problem but I'd rather see explicit cold resets in > > the callers where that's required. @@ -40,9 +40,9 @@ Ideally the core would keep track of this and transparently trigger the warm reset when required, but driver local is fine since that's not there yet. -> >> + ac_glbctrl = S3C_AC97_GLBCTRL_ACLINKON; -> >> + writel(ac_glbctrl, s3c_ac97.regs + S3C_AC97_GLBCTRL); -> >> + msleep(1); +> >> + ? ? ac_glbctrl = S3C_AC97_GLBCTRL_ACLINKON; +> >> + ? ? writel(ac_glbctrl, s3c_ac97.regs + S3C_AC97_GLBCTRL); +> >> + ? ? msleep(1); > > This also looks a bit odd, ACLINKON sounds like bringing up the link > > which is what a warm reset does. @@ -55,9 +55,9 @@ by the warm reset? It's the structure of the code I'm commenting on rather than the operations that get performed on the hardware. > >> +lb5: -> >> + free_irq(irq_res->start, NULL); +> >> + ? ? free_irq(irq_res->start, NULL); -> > Perhaps a better name than 'lb' - err, or fail or something. lb means +> > Perhaps a better name than 'lb' - err, or fail or something. ?lb means > > nothing to me at least. > means label to me :) diff --git a/a/content_digest b/N1/content_digest index e58abe4..ca3f14c 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -2,30 +2,24 @@ "ref\01264485101-13782-2-git-send-email-jassisinghbrar@gmail.com\0" "ref\020100126104723.GF15759@rakim.wolfsonmicro.main\0" "ref\01b68c6791001260317k44893521vc0310ea35bac8ee1@mail.gmail.com\0" - "From\0Mark Brown <broonie@opensource.wolfsonmicro.com>\0" - "Subject\0Re: [PATCH 2/3] ASoC: AC97: S3C: Add controller driver\0" + "From\0broonie@opensource.wolfsonmicro.com (Mark Brown)\0" + "Subject\0[PATCH 2/3] ASoC: AC97: S3C: Add controller driver\0" "Date\0Tue, 26 Jan 2010 11:52:58 +0000\0" - "To\0jassi brar <jassisinghbrar@gmail.com>\0" - "Cc\0alsa-devel@alsa-project.org" - linux-samsung-soc@vger.kernel.org - Jassi Brar <jassi.brar@samsung.com> - ben-linux@fluff.org - linux-arm-kernel@lists.infradead.org - " lrg@slimlogic.co.uk\0" + "To\0linux-arm-kernel@lists.infradead.org\0" "\00:1\0" "b\0" "On Tue, Jan 26, 2010 at 08:17:25PM +0900, jassi brar wrote:\n" "> On Tue, Jan 26, 2010 at 7:47 PM, Mark Brown\n" "\n" - "> >> + \302\240 \302\240 default:\n" - "> >> + \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 s3c_ac97_cold_reset(ac97);\n" - "> >> + \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 s3c_ac97_warm_reset(ac97);\n" - "> >> + \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 break;\n" - "> >> + \302\240 \302\240 }\n" + "> >> + ? ? default:\n" + "> >> + ? ? ? ? ? ? s3c_ac97_cold_reset(ac97);\n" + "> >> + ? ? ? ? ? ? s3c_ac97_warm_reset(ac97);\n" + "> >> + ? ? ? ? ? ? break;\n" + "> >> + ? ? }\n" "\n" "> > This automatic cold and warm reset looks a bit fishy - obviously if this\n" "> > code path ever gets hit in normal operation then it's going to seriously\n" - "> > disrupt things since it'll reset the CODEC registers. \302\240A warm reset by\n" + "> > disrupt things since it'll reset the CODEC registers. ?A warm reset by\n" "> > itself wouldn't be a problem but I'd rather see explicit cold resets in\n" "> > the callers where that's required.\n" "\n" @@ -56,9 +50,9 @@ "warm reset when required, but driver local is fine since that's not\n" "there yet.\n" "\n" - "> >> + \302\240 \302\240 ac_glbctrl = S3C_AC97_GLBCTRL_ACLINKON;\n" - "> >> + \302\240 \302\240 writel(ac_glbctrl, s3c_ac97.regs + S3C_AC97_GLBCTRL);\n" - "> >> + \302\240 \302\240 msleep(1);\n" + "> >> + ? ? ac_glbctrl = S3C_AC97_GLBCTRL_ACLINKON;\n" + "> >> + ? ? writel(ac_glbctrl, s3c_ac97.regs + S3C_AC97_GLBCTRL);\n" + "> >> + ? ? msleep(1);\n" "\n" "> > This also looks a bit odd, ACLINKON sounds like bringing up the link\n" "> > which is what a warm reset does.\n" @@ -71,9 +65,9 @@ "rather than the operations that get performed on the hardware.\n" "\n" "> >> +lb5:\n" - "> >> + \302\240 \302\240 free_irq(irq_res->start, NULL);\n" + "> >> + ? ? free_irq(irq_res->start, NULL);\n" "\n" - "> > Perhaps a better name than 'lb' - err, or fail or something. \302\240lb means\n" + "> > Perhaps a better name than 'lb' - err, or fail or something. ?lb means\n" "> > nothing to me at least.\n" "\n" "> means label to me :)\n" @@ -81,4 +75,4 @@ "If you'd spelt out label I'd probably have been OK, but in English lb is\n" normally only an abbreviation for pounds as a unit of weight. -e1c9621156c738efbd6e512de1758d1ba8cd32a2929f4fce1f6e8aa3999a7f99 +95fb801492283ef7d0ad27184803534c7dd414d0fb15fb2551c621e714c1e046
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.