| Message ID | 1380729418-14393-1-git-send-email-simon.guinot@sequanux.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Simon, On Wed, Oct 02, 2013 at 05:56:58PM +0200, Simon Guinot wrote: > Signed-off-by: Simon Guinot <simon.guinot@sequanux.org> Could you elaborate on this? What symptom did you notice? How did this fix it? When was the regression introduced? thx, Jason. > --- > drivers/clk/mvebu/armada-370.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/clk/mvebu/armada-370.c b/drivers/clk/mvebu/armada-370.c > index fc777bd..81a202d 100644 > --- a/drivers/clk/mvebu/armada-370.c > +++ b/drivers/clk/mvebu/armada-370.c > @@ -39,8 +39,8 @@ static const struct coreclk_ratio a370_coreclk_ratios[] __initconst = { > }; > > static const u32 a370_tclk_freqs[] __initconst = { > - 16600000, > - 20000000, > + 166000000, > + 200000000, > }; > > static u32 __init a370_get_tclk_freq(void __iomem *sar) > -- > 1.7.10.4 >
On Wed, Oct 02, 2013 at 12:22:21PM -0400, Jason Cooper wrote: > Simon, > > On Wed, Oct 02, 2013 at 05:56:58PM +0200, Simon Guinot wrote: > > Signed-off-by: Simon Guinot <simon.guinot@sequanux.org> > > Could you elaborate on this? What symptom did you notice? How did this > fix it? When was the regression introduced? Hi Jason, While benchmarking Linux 3.12-rc3 on an Armada-370-RD, I noticed bad networking performances... After some investigations, I found out that the coalescence configuration (derived from the tclk) was suspect. While checking for the tclk value, I found this typo. There is not much to say about this patch except that the correct tclk frequencies for Armada-370 are 166 and 200MHz, not 16.6 and 20MHz. Simon > > thx, > > Jason. > > > --- > > drivers/clk/mvebu/armada-370.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/clk/mvebu/armada-370.c b/drivers/clk/mvebu/armada-370.c > > index fc777bd..81a202d 100644 > > --- a/drivers/clk/mvebu/armada-370.c > > +++ b/drivers/clk/mvebu/armada-370.c > > @@ -39,8 +39,8 @@ static const struct coreclk_ratio a370_coreclk_ratios[] __initconst = { > > }; > > > > static const u32 a370_tclk_freqs[] __initconst = { > > - 16600000, > > - 20000000, > > + 166000000, > > + 200000000, > > }; > > > > static u32 __init a370_get_tclk_freq(void __iomem *sar) > > -- > > 1.7.10.4 > >
On Wed, Oct 02, 2013 at 06:57:54PM +0200, Simon Guinot wrote: > On Wed, Oct 02, 2013 at 12:22:21PM -0400, Jason Cooper wrote: > > Simon, > > > > On Wed, Oct 02, 2013 at 05:56:58PM +0200, Simon Guinot wrote: > > > Signed-off-by: Simon Guinot <simon.guinot@sequanux.org> > > > > Could you elaborate on this? What symptom did you notice? How did this > > fix it? When was the regression introduced? > > Hi Jason, > > While benchmarking Linux 3.12-rc3 on an Armada-370-RD, I noticed bad > networking performances... After some investigations, I found out that > the coalescence configuration (derived from the tclk) was suspect. > > While checking for the tclk value, I found this typo. There is not much > to say about this patch except that the correct tclk frequencies for > Armada-370 are 166 and 200MHz, not 16.6 and 20MHz. Yes, I know it's simple, but it's a huge help to us when trying to decide whether or not to flag it for stable. fwiw, the regression was introduced by: 6b72333d clk: mvebu: add Armada 370 SoC-centric clock init and this should be applied from v3.11 onwards. thx, Jason.
On Wed, Oct 02, 2013 at 01:04:49PM -0400, Jason Cooper wrote: > On Wed, Oct 02, 2013 at 06:57:54PM +0200, Simon Guinot wrote: > > On Wed, Oct 02, 2013 at 12:22:21PM -0400, Jason Cooper wrote: > > > Simon, > > > > > > On Wed, Oct 02, 2013 at 05:56:58PM +0200, Simon Guinot wrote: > > > > Signed-off-by: Simon Guinot <simon.guinot@sequanux.org> > > > > > > Could you elaborate on this? What symptom did you notice? How did this > > > fix it? When was the regression introduced? > > > > Hi Jason, > > > > While benchmarking Linux 3.12-rc3 on an Armada-370-RD, I noticed bad > > networking performances... After some investigations, I found out that > > the coalescence configuration (derived from the tclk) was suspect. > > > > While checking for the tclk value, I found this typo. There is not much > > to say about this patch except that the correct tclk frequencies for > > Armada-370 are 166 and 200MHz, not 16.6 and 20MHz. > > Yes, I know it's simple, but it's a huge help to us when trying to > decide whether or not to flag it for stable. fwiw, the regression was > introduced by: > > 6b72333d clk: mvebu: add Armada 370 SoC-centric clock init > > and this should be applied from v3.11 onwards. Ok, I will reword this commit. Note that this bug can't be called a regression. The tclk frequencies array was wrong from the beginning: 97fa4cf4: clk: mvebu: add mvebu core clocks The commit you pointed out simply moves the typo. I am not familiar with the stable process but if needed I can provide a patch against some stable branches. Please let know how do you want handle this. Simon
On Thu, Oct 03, 2013 at 11:00:58AM +0200, Simon Guinot wrote: > On Wed, Oct 02, 2013 at 01:04:49PM -0400, Jason Cooper wrote: > > On Wed, Oct 02, 2013 at 06:57:54PM +0200, Simon Guinot wrote: > > > On Wed, Oct 02, 2013 at 12:22:21PM -0400, Jason Cooper wrote: > > > > Simon, > > > > > > > > On Wed, Oct 02, 2013 at 05:56:58PM +0200, Simon Guinot wrote: > > > > > Signed-off-by: Simon Guinot <simon.guinot@sequanux.org> > > > > > > > > Could you elaborate on this? What symptom did you notice? How did this > > > > fix it? When was the regression introduced? > > > > > > Hi Jason, > > > > > > While benchmarking Linux 3.12-rc3 on an Armada-370-RD, I noticed bad > > > networking performances... After some investigations, I found out that > > > the coalescence configuration (derived from the tclk) was suspect. > > > > > > While checking for the tclk value, I found this typo. There is not much > > > to say about this patch except that the correct tclk frequencies for > > > Armada-370 are 166 and 200MHz, not 16.6 and 20MHz. > > > > Yes, I know it's simple, but it's a huge help to us when trying to > > decide whether or not to flag it for stable. fwiw, the regression was > > introduced by: > > > > 6b72333d clk: mvebu: add Armada 370 SoC-centric clock init > > > > and this should be applied from v3.11 onwards. > > Ok, I will reword this commit. Note that this bug can't be called a > regression. The tclk frequencies array was wrong from the beginning: > > 97fa4cf4: clk: mvebu: add mvebu core clocks > > The commit you pointed out simply moves the typo. Ahh, good catch! Yes, please reword the commit. > I am not familiar with the stable process but if needed I can provide > a patch against some stable branches. Usually, there should only need to be one patch against mainline. But since the bad values were moved during the lifetime of the error, there may need to be two patches. Depends on how magical git is. I've seen it do some pretty amazing stuff. Please find the original commit introducing the error, reference that in the commit, and use 'tag --contains ...' to see which versions need the fix. You can mention that below the cutoff. That's generally all we need to flag it for stable and help the stable guys figure out which trees to apply it to. hth, Jason.
diff --git a/drivers/clk/mvebu/armada-370.c b/drivers/clk/mvebu/armada-370.c index fc777bd..81a202d 100644 --- a/drivers/clk/mvebu/armada-370.c +++ b/drivers/clk/mvebu/armada-370.c @@ -39,8 +39,8 @@ static const struct coreclk_ratio a370_coreclk_ratios[] __initconst = { }; static const u32 a370_tclk_freqs[] __initconst = { - 16600000, - 20000000, + 166000000, + 200000000, }; static u32 __init a370_get_tclk_freq(void __iomem *sar)
Signed-off-by: Simon Guinot <simon.guinot@sequanux.org> --- drivers/clk/mvebu/armada-370.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)