[OpenWrt-Devel] Merged: ramips: Increase GB-PC2 SPI frequency to 80MHz

[Rosen Penev]

On Thu, Mar 28, 2019 at 3:16 AM Petr Štetiar <ynezz at true.cz> wrote:
>
> Christian Lamparter <chunkeey at gmail.com> [2019-03-25 21:43:26]:
>
> Hi,
>
> > > Thanks! Merged into my staging tree at https://git.openwrt.org/openwrt/staging/ynezz.git
> >
> > https://patchwork.ozlabs.org/patch/1034614/#2088615
>
> Ah, so this is actually v2, but not being marked as such. Thanks for the info.
>
> This patch simply got my attention as I was looking for some ramips candidates
> I could include together in one build test, this patch was lingering in the
> patchwork for more then a month without any comments, looked correct to me at
> the first sight, and harmless enough so I've just merged it. I'll try to be
> more careful next time.
Erm careful with what?
>
> @Rosen could you please next time include v2 in the subject and changelog in
> your patches? :-) Thanks
>
> Rosen Penev <rosenp at gmail.com> [2019-03-25 14:11:32]:
>
> > On Mon, Mar 25, 2019 at 1:43 PM Christian Lamparter <chunkeey at gmail.com> wrote:
> > >
> > > So, I think in order for this to "work as expected" the sysclock
> > > in the mt7621.dtsi should be at 220 MHz (as in the upstream
> > > drivers/staging/mt7621-dts/mt7621.dtsi) instead of 50 MHz.
> >
> > According to GCH, that commit is wrong:
> > https://github.com/openwrt/openwrt/pull/1578#issuecomment-443661067
> >
> > As far as the clock goes, it's using a PLL now:
> > https://github.com/openwrt/openwrt/blob/master/target/linux/ramips/dts/mt7621.dtsi#L223
>
> Ok, thanks for the details. In order to move forward, could you please provide
> relevant parts from dmesg (from clean latest snapshot, without your local
> modifications) so we can see the clock information from the running GP-PC2
> system?
>
> I'll then include this additional details to the commit message as well, so it
> could be easily git blamed.
I'll see when I can get some time.
>
> > > That said, I don't think this will break anything since the mt7621-spi driver
> > > from 0043-spi-add-mt7621-support.patch, just limits it at the fake "25 Mhz"
> > > (which should be ~110 MHz). So, this will just look odd and makes no sense
> > > at the first glance.
> >
> > Ah, none of my testing was done with the stock driver, only the upstream one.
>
> Honestly I find this little bit delicate, that you're submiting patches which
> are not thoroughly tested against our master. To be clear, it's not happening
> in this particular case, but I don't want to help merge patches which I'm not
> able to test myself and which might in some rare cases brick other people's
> devices.
This device specifically is not that popular. Any potential fallout
would be very minimal. Highly unlikely that there would be as there
are already devices that use 80MHz and nobody has complained.
>
> So the commit message simply contains garbage, as nobody else could confirm
> (and more likely deny) this dd values in the time of commit.
>
> We need to fix the commit message as well, please provide the dd values from
> clean latest snapshot as well and please include kernel version for the
> upstream kernel values as well (or if it was backport of upstream driver from
> version XY to version AB).
>
> > There are also other devices with an 80MHz clock in the ramips dts directory.
>
> I find this arguments quite silly and I'm tired to read them over and over
> again. It might be in the tree already for many reasons.
>
> > 80MHz looked like it was a peak so that's what I used.
>
> I'm wondering if it would hurt the performance if you would provide maximum
> clock value as written in datasheet. I mean this testing is nice, but does it
> provide some meaningful number? I think, that in order to make it a proper
> number, you would still need to test it on a few more devices, ideally from
> different production runs, and even after this we might still face
> possibility, that we get another patch from the other user, claiming, that his
> flash can do 121MHz so why it's crippled to 80MHz :-)
I tried higher values than 80 but they resulted in no speedup. I think
this is some mt7621 limitation. I've tested this on two devices with
two different flash chips (PC1 and PC2 having Winbond and Spansion
respectively).
>
> -- ynezz

_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel