| Message ID | 20160314151337.GB2897@localhost.localdomain (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 03/14/2016 05:13 PM, Ladislav Michl wrote: > Hi there! > > seems it's been three years since: > [PATCH 1/1] Fix sprz319 erratum 2.1 > http://article.gmane.org/gmane.linux.ports.arm.omap/71633 > > errata: http://www.ti.com/lit/er/sprz319f/sprz319f.pdf > > Nice summary (third post by Rodrigo Lemos) can be found here: > https://github.com/RobertCNelson/stable-kernel/issues/26 > so I'm not going to repeat it here. > > Company I'm working for has few thousands of IGEPv2 boards in field. They > have hub connected to usb port with udlfb display, 3G modem and usbserial: > /: Bus 03.Port 1: Dev 1, Class=root_hub, Driver=ehci-omap/3p, 480M > |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/4p, 480M > |__ Port 1: Dev 3, If 0, Class=Vendor Specific Class, Driver=udlfb, 480M > |__ Port 3: Dev 4, If 0, Class=Vendor Specific Class, Driver=option, 480M > |__ Port 3: Dev 4, If 1, Class=Vendor Specific Class, Driver=, 480M > |__ Port 3: Dev 4, If 2, Class=Vendor Specific Class, Driver=option, 480M > |__ Port 3: Dev 4, If 3, Class=Vendor Specific Class, Driver=option, 480M > |__ Port 3: Dev 4, If 4, Class=Mass Storage, Driver=usb-storage, 480M > |__ Port 3: Dev 4, If 5, Class=Mass Storage, Driver=usb-storage, 480M > |__ Port 4: Dev 5, If 0, Class=Hub, Driver=hub/4p, 480M > |__ Port 3: Dev 6, If 0, Class=Communications, Driver=cdc_acm, 12M > |__ Port 3: Dev 6, If 1, Class=CDC Data, Driver=cdc_acm, 12M > > Few tens of those disconnects USB after while (one minute to few hours) with: > "usb usb3-port1: disabled by hub (EMI?), re-enabling..." > IGEPv2 uses DM3730 with 26MHz crystal divided by two providing 13MHz SYS_CLK, > therefore it is good candidate to fix. Patch bellow was used as a dirty hack > to test if usb disconnects go away with errata applied. And indeed, device > is already running for few days without any issues. Now question is, how should > a proper fix look like as modifiing omap2_dpll_round_rate seems easy enough. > Is that acceptable? How to test proper clock to apply errata? Comparing string is > suboptimal. Should we introduce some flag? The hack applied seems rather bad to me, as it doesn't take any input frequencies into consideration. Basically you just force the divider / multiplier to the value required for 13MHz input clock, but this is going to be wrong with any other input clock. I think a proper fix would probably be to implement new clock ops for DPLL5, something similar to what was done with the original patch a few years back. This would select the divider/multiplier factors for the DPLL based on table values provided by the erratum. -Tero > > ladis > > --- linux-4.5/drivers/clk/ti/dpll3xxx.c.orig 2016-03-14 12:43:58.085003085 +0100 > +++ linux-4.5/drivers/clk/ti/dpll3xxx.c 2016-03-14 13:17:57.893417000 +0100 > @@ -308,6 +308,7 @@ > u8 dco, sd_div, ai = 0; > u32 v; > bool errata_i810; > + const char *clk_name; > > /* 3430 ES2 TRM: 4.7.6.9 DPLL Programming Sequence */ > _omap3_noncore_dpll_bypass(clk); > @@ -370,6 +371,12 @@ > } > } > > + clk_name = clk_hw_get_name(&clk->hw); > + if (strcmp(clk_name, "dpll5_ck") == 0) { > + pr_info("fixing %s (%04lx), %06x -> 01bb05\n", clk_name, dd->mult_div1_reg, v); > + v = 0x1bb05; > + } > + > ti_clk_ll_ops->clk_writel(v, dd->mult_div1_reg); > > /* Set 4X multiplier and low-power mode */ > --- linux-4.5/drivers/clk/ti/divider.c.orig 2016-03-14 12:44:06.509211022 +0100 > +++ linux-4.5/drivers/clk/ti/divider.c 2016-03-14 13:18:21.793417000 +0100 > @@ -215,6 +215,7 @@ > struct clk_divider *divider; > unsigned int div, value; > u32 val; > + const char *clk_name; > > if (!hw || !rate) > return -EINVAL; > @@ -234,6 +235,13 @@ > val &= ~(div_mask(divider) << divider->shift); > } > val |= value << divider->shift; > + > + clk_name = clk_hw_get_name(hw); > + if (strcmp(clk_name, "dpll5_m2_ck") == 0) { > + pr_info("fixing %s (%04lx), %04x -> 0008\n", clk_name, divider->reg, val); > + val = 8; > + } > + > ti_clk_ll_ops->clk_writel(val, divider->reg); > > return 0; > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Mar 14, 2016 at 09:40:54PM +0200, Tero Kristo wrote: > On 03/14/2016 05:13 PM, Ladislav Michl wrote: > >Hi there! > > > >seems it's been three years since: > >[PATCH 1/1] Fix sprz319 erratum 2.1 > >http://article.gmane.org/gmane.linux.ports.arm.omap/71633 > > > >errata: http://www.ti.com/lit/er/sprz319f/sprz319f.pdf > > > >Nice summary (third post by Rodrigo Lemos) can be found here: > >https://github.com/RobertCNelson/stable-kernel/issues/26 > >so I'm not going to repeat it here. > > > >Company I'm working for has few thousands of IGEPv2 boards in field. They > >have hub connected to usb port with udlfb display, 3G modem and usbserial: > >/: Bus 03.Port 1: Dev 1, Class=root_hub, Driver=ehci-omap/3p, 480M > > |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/4p, 480M > > |__ Port 1: Dev 3, If 0, Class=Vendor Specific Class, Driver=udlfb, 480M > > |__ Port 3: Dev 4, If 0, Class=Vendor Specific Class, Driver=option, 480M > > |__ Port 3: Dev 4, If 1, Class=Vendor Specific Class, Driver=, 480M > > |__ Port 3: Dev 4, If 2, Class=Vendor Specific Class, Driver=option, 480M > > |__ Port 3: Dev 4, If 3, Class=Vendor Specific Class, Driver=option, 480M > > |__ Port 3: Dev 4, If 4, Class=Mass Storage, Driver=usb-storage, 480M > > |__ Port 3: Dev 4, If 5, Class=Mass Storage, Driver=usb-storage, 480M > > |__ Port 4: Dev 5, If 0, Class=Hub, Driver=hub/4p, 480M > > |__ Port 3: Dev 6, If 0, Class=Communications, Driver=cdc_acm, 12M > > |__ Port 3: Dev 6, If 1, Class=CDC Data, Driver=cdc_acm, 12M > > > >Few tens of those disconnects USB after while (one minute to few hours) with: > >"usb usb3-port1: disabled by hub (EMI?), re-enabling..." > >IGEPv2 uses DM3730 with 26MHz crystal divided by two providing 13MHz SYS_CLK, > >therefore it is good candidate to fix. Patch bellow was used as a dirty hack > >to test if usb disconnects go away with errata applied. And indeed, device > >is already running for few days without any issues. Now question is, how should > >a proper fix look like as modifiing omap2_dpll_round_rate seems easy enough. > >Is that acceptable? How to test proper clock to apply errata? Comparing string is > >suboptimal. Should we introduce some flag? > > The hack applied seems rather bad to me, as it doesn't take any input > frequencies into consideration. Basically you just force the divider / > multiplier to the value required for 13MHz input clock, but this is going to > be wrong with any other input clock. Well, hacking that took few minutes and was done only to test if issue with disconnects has anything to do with said errata. And it indeed does (doing it right way would mean I'd need to verify what's written to registers anyway, thus touching hacked functions). Otherwise I agree, but also note that previous attempt ended in various repositories, but never found its way to mainline - something I'd like to avoid, so I even didn't try to come with anything sensible. > I think a proper fix would probably be to implement new clock ops for DPLL5, > something similar to what was done with the original patch a few years back. > This would select the divider/multiplier factors for the DPLL based on table > values provided by the erratum. That seems like pretty intrusive change given the fact clock are declared as DT_CLK(NULL, "dpll5_ck", "dpll5_ck"), DT_CLK(NULL, "dpll5_m2_ck", "dpll5_m2_ck"), and registered with ti_dt_clocks_register. Or do you mean completely new clock ops including differently named DT property? Is that what you mean? Also note that for other frequencies original algo is still valid (omap2_dpll_round_rate)... ladis -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 03/14/2016 10:47 PM, Ladislav Michl wrote: > On Mon, Mar 14, 2016 at 09:40:54PM +0200, Tero Kristo wrote: >> On 03/14/2016 05:13 PM, Ladislav Michl wrote: >>> Hi there! >>> >>> seems it's been three years since: >>> [PATCH 1/1] Fix sprz319 erratum 2.1 >>> http://article.gmane.org/gmane.linux.ports.arm.omap/71633 >>> >>> errata: http://www.ti.com/lit/er/sprz319f/sprz319f.pdf >>> >>> Nice summary (third post by Rodrigo Lemos) can be found here: >>> https://github.com/RobertCNelson/stable-kernel/issues/26 >>> so I'm not going to repeat it here. >>> >>> Company I'm working for has few thousands of IGEPv2 boards in field. They >>> have hub connected to usb port with udlfb display, 3G modem and usbserial: >>> /: Bus 03.Port 1: Dev 1, Class=root_hub, Driver=ehci-omap/3p, 480M >>> |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/4p, 480M >>> |__ Port 1: Dev 3, If 0, Class=Vendor Specific Class, Driver=udlfb, 480M >>> |__ Port 3: Dev 4, If 0, Class=Vendor Specific Class, Driver=option, 480M >>> |__ Port 3: Dev 4, If 1, Class=Vendor Specific Class, Driver=, 480M >>> |__ Port 3: Dev 4, If 2, Class=Vendor Specific Class, Driver=option, 480M >>> |__ Port 3: Dev 4, If 3, Class=Vendor Specific Class, Driver=option, 480M >>> |__ Port 3: Dev 4, If 4, Class=Mass Storage, Driver=usb-storage, 480M >>> |__ Port 3: Dev 4, If 5, Class=Mass Storage, Driver=usb-storage, 480M >>> |__ Port 4: Dev 5, If 0, Class=Hub, Driver=hub/4p, 480M >>> |__ Port 3: Dev 6, If 0, Class=Communications, Driver=cdc_acm, 12M >>> |__ Port 3: Dev 6, If 1, Class=CDC Data, Driver=cdc_acm, 12M >>> >>> Few tens of those disconnects USB after while (one minute to few hours) with: >>> "usb usb3-port1: disabled by hub (EMI?), re-enabling..." >>> IGEPv2 uses DM3730 with 26MHz crystal divided by two providing 13MHz SYS_CLK, >>> therefore it is good candidate to fix. Patch bellow was used as a dirty hack >>> to test if usb disconnects go away with errata applied. And indeed, device >>> is already running for few days without any issues. Now question is, how should >>> a proper fix look like as modifiing omap2_dpll_round_rate seems easy enough. >>> Is that acceptable? How to test proper clock to apply errata? Comparing string is >>> suboptimal. Should we introduce some flag? >> >> The hack applied seems rather bad to me, as it doesn't take any input >> frequencies into consideration. Basically you just force the divider / >> multiplier to the value required for 13MHz input clock, but this is going to >> be wrong with any other input clock. > > Well, hacking that took few minutes and was done only to test if issue with > disconnects has anything to do with said errata. And it indeed does (doing it > right way would mean I'd need to verify what's written to registers anyway, > thus touching hacked functions). Otherwise I agree, but also note that previous > attempt ended in various repositories, but never found its way to mainline - > something I'd like to avoid, so I even didn't try to come with anything > sensible. > >> I think a proper fix would probably be to implement new clock ops for DPLL5, >> something similar to what was done with the original patch a few years back. >> This would select the divider/multiplier factors for the DPLL based on table >> values provided by the erratum. > > That seems like pretty intrusive change given the fact clock are declared as > DT_CLK(NULL, "dpll5_ck", "dpll5_ck"), > DT_CLK(NULL, "dpll5_m2_ck", "dpll5_m2_ck"), > and registered with ti_dt_clocks_register. Or do you mean completely new clock > ops including differently named DT property? Is that what you mean? Also note > that for other frequencies original algo is still valid > (omap2_dpll_round_rate)... Yes, I think we should add a top-level ops for dpll5, that would determine if table dividers shall be used first, if not, then just call the generic implementation. I would also add a new compatible string for the purpose, this means the users must update both kernel + DTB but I believe any OMAPx customers are doing this anyway. -Tero -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[snip] > Yes, I think we should add a top-level ops for dpll5, that would > determine if table dividers shall be used first, if not, then just call > the generic implementation. > > I would also add a new compatible string for the purpose, this means the > users must update both kernel + DTB but I believe any OMAPx customers > are doing this anyway. > I'd agree - sadly I didn't have time (and still don't have much), so I never did it, I believe there was a patch in the linux-omap tree for a while, but I don't know what happened to it - it seems not to have made it to mainline, Richard. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Mar 15, 2016 at 08:25:06AM +0000, Richard Watts wrote: > [snip] > >Yes, I think we should add a top-level ops for dpll5, that would > >determine if table dividers shall be used first, if not, then just call > >the generic implementation. Just tried that and it looks reasonable. Will send patch after cleanup. > >I would also add a new compatible string for the purpose, this means the > >users must update both kernel + DTB but I believe any OMAPx customers > >are doing this anyway. > > Should DTB also carry fixup table? > I'd agree - sadly I didn't have time (and still don't have much), so I > never did it, Was there any consensus how to pick between workarounds for 26MHz? > I believe there was a patch in the linux-omap tree for a while, but > I don't know what happened to it - it seems not to have made it to > mainline, Quick search of git history didn't find anything interesting, but it wouldn't make it easier anyway :-) ladis -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Mar 15, 2016 at 7:40 PM, Ladislav Michl <ladis@linux-mips.org> wrote: > On Tue, Mar 15, 2016 at 08:25:06AM +0000, Richard Watts wrote: >> [snip] >> >Yes, I think we should add a top-level ops for dpll5, that would >> >determine if table dividers shall be used first, if not, then just call >> >the generic implementation. > > Just tried that and it looks reasonable. Will send patch after cleanup. > >> >I would also add a new compatible string for the purpose, this means the >> >users must update both kernel + DTB but I believe any OMAPx customers >> >are doing this anyway. >> > > > Should DTB also carry fixup table? It really should be an optional flag that we can enable on a board by board basis.. >> I'd agree - sadly I didn't have time (and still don't have much), so I >> never did it, > > Was there any consensus how to pick between workarounds for 26MHz? I would just mirror what was done in u-boot for the lookup table: http://git.denx.de/?p=u-boot.git;a=commitdiff;h=a704a6d615179a25f556c99d31cbc4ee366ffb54 > >> I believe there was a patch in the linux-omap tree for a while, but >> I don't know what happened to it - it seems not to have made it to >> mainline, > > Quick search of git history didn't find anything interesting, but > it wouldn't make it easier anyway :-) Nothing ever went mainline, it seemed like it happened more often with later boards. So kernel developers never got one. Tero, if you'd like i can send you an xM that the show's the issue often enough it's easy to test changes with. Since BBxM's have dried up at most retailers, i haven't done much with them anymore for the beagleboard.org community. It's the only dm37xx based product i had supported. (i have lots of xm's so no worries about this one).. Regards,
On 03/16/2016 04:44 AM, Robert Nelson wrote: > On Tue, Mar 15, 2016 at 7:40 PM, Ladislav Michl <ladis@linux-mips.org> wrote: >> On Tue, Mar 15, 2016 at 08:25:06AM +0000, Richard Watts wrote: >>> [snip] >>>> Yes, I think we should add a top-level ops for dpll5, that would >>>> determine if table dividers shall be used first, if not, then just call >>>> the generic implementation. >> >> Just tried that and it looks reasonable. Will send patch after cleanup. >> >>>> I would also add a new compatible string for the purpose, this means the >>>> users must update both kernel + DTB but I believe any OMAPx customers >>>> are doing this anyway. >>>> >> >> Should DTB also carry fixup table? > > It really should be an optional flag that we can enable on a board by > board basis.. I am fine with just changing the compatible string for DPLL5 to a new one. Tony, any comment on this? > > >>> I'd agree - sadly I didn't have time (and still don't have much), so I >>> never did it, >> >> Was there any consensus how to pick between workarounds for 26MHz? > > I would just mirror what was done in u-boot for the lookup table: > > http://git.denx.de/?p=u-boot.git;a=commitdiff;h=a704a6d615179a25f556c99d31cbc4ee366ffb54 > >> >>> I believe there was a patch in the linux-omap tree for a while, but >>> I don't know what happened to it - it seems not to have made it to >>> mainline, >> >> Quick search of git history didn't find anything interesting, but >> it wouldn't make it easier anyway :-) > > Nothing ever went mainline, it seemed like it happened more often with > later boards. So kernel developers never got one. > > Tero, if you'd like i can send you an xM that the show's the issue > often enough it's easy to test changes with. Since BBxM's have dried > up at most retailers, i haven't done much with them anymore for the > beagleboard.org community. It's the only dm37xx based product i had > supported. (i have lots of xm's so no worries about this one).. Yea that would be nice, I don't have beagle-xm, or access to any other omap3630 hardware either. If you have spare ones around, lets discuss the shipping details in a separate email. -Tero -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Mar 16, 2016 at 1:26 AM, Tero Kristo <t-kristo@ti.com> wrote: > On 03/16/2016 04:44 AM, Robert Nelson wrote: >> >> On Tue, Mar 15, 2016 at 7:40 PM, Ladislav Michl <ladis@linux-mips.org> >> wrote: >>> >>> On Tue, Mar 15, 2016 at 08:25:06AM +0000, Richard Watts wrote: >>>> >>>> [snip] >>>>> >>>>> Yes, I think we should add a top-level ops for dpll5, that would >>>>> determine if table dividers shall be used first, if not, then just call >>>>> the generic implementation. >>> >>> >>> Just tried that and it looks reasonable. Will send patch after cleanup. >>> >>>>> I would also add a new compatible string for the purpose, this means >>>>> the >>>>> users must update both kernel + DTB but I believe any OMAPx customers >>>>> are doing this anyway. >>>>> >>> >>> Should DTB also carry fixup table? >> >> >> It really should be an optional flag that we can enable on a board by >> board basis.. > > > I am fine with just changing the compatible string for DPLL5 to a new one. > > Tony, any comment on this? > >> >> >>>> I'd agree - sadly I didn't have time (and still don't have much), so I >>>> never did it, >>> >>> >>> Was there any consensus how to pick between workarounds for 26MHz? >> >> >> I would just mirror what was done in u-boot for the lookup table: >> >> >> http://git.denx.de/?p=u-boot.git;a=commitdiff;h=a704a6d615179a25f556c99d31cbc4ee366ffb54 >> >>> >>>> I believe there was a patch in the linux-omap tree for a while, but >>>> I don't know what happened to it - it seems not to have made it to >>>> mainline, >>> >>> >>> Quick search of git history didn't find anything interesting, but >>> it wouldn't make it easier anyway :-) >> >> >> Nothing ever went mainline, it seemed like it happened more often with >> later boards. So kernel developers never got one. >> >> Tero, if you'd like i can send you an xM that the show's the issue >> often enough it's easy to test changes with. Since BBxM's have dried >> up at most retailers, i haven't done much with them anymore for the >> beagleboard.org community. It's the only dm37xx based product i had >> supported. (i have lots of xm's so no worries about this one).. > I need to look to see how the DM3730 board I have is impacted by this (if at all) and better understand how to test it, but I'm willing to help with some testing. The Logic PD SOM-LV uses an EHCI USB, but I haven't had any troubles in the past (that I am aware). I would at least be able to confirm it either still works or something broke. > > Yea that would be nice, I don't have beagle-xm, or access to any other > omap3630 hardware either. If you have spare ones around, lets discuss the > shipping details in a separate email. > > -Tero > > -- > To unsubscribe from this list: send the line "unsubscribe linux-omap" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* Tero Kristo <t-kristo@ti.com> [160315 23:27]: > On 03/16/2016 04:44 AM, Robert Nelson wrote: > >On Tue, Mar 15, 2016 at 7:40 PM, Ladislav Michl <ladis@linux-mips.org> wrote: > >>On Tue, Mar 15, 2016 at 08:25:06AM +0000, Richard Watts wrote: > >>>[snip] > >>>>Yes, I think we should add a top-level ops for dpll5, that would > >>>>determine if table dividers shall be used first, if not, then just call > >>>>the generic implementation. > >> > >>Just tried that and it looks reasonable. Will send patch after cleanup. > >> > >>>>I would also add a new compatible string for the purpose, this means the > >>>>users must update both kernel + DTB but I believe any OMAPx customers > >>>>are doing this anyway. > >>>> > >> > >>Should DTB also carry fixup table? > > > >It really should be an optional flag that we can enable on a board by > >board basis.. > > I am fine with just changing the compatible string for DPLL5 to a new one. > > Tony, any comment on this? I guess I don't quite follow, do you just mean adding a new compatible for 36xx? Can we keep this workaround always enabled for 36xx or does it have some side effects? Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 03/16/2016 04:33 PM, Tony Lindgren wrote: > * Tero Kristo <t-kristo@ti.com> [160315 23:27]: >> On 03/16/2016 04:44 AM, Robert Nelson wrote: >>> On Tue, Mar 15, 2016 at 7:40 PM, Ladislav Michl <ladis@linux-mips.org> wrote: >>>> On Tue, Mar 15, 2016 at 08:25:06AM +0000, Richard Watts wrote: >>>>> [snip] >>>>>> Yes, I think we should add a top-level ops for dpll5, that would >>>>>> determine if table dividers shall be used first, if not, then just call >>>>>> the generic implementation. >>>> >>>> Just tried that and it looks reasonable. Will send patch after cleanup. >>>> >>>>>> I would also add a new compatible string for the purpose, this means the >>>>>> users must update both kernel + DTB but I believe any OMAPx customers >>>>>> are doing this anyway. >>>>>> >>>> >>>> Should DTB also carry fixup table? >>> >>> It really should be an optional flag that we can enable on a board by >>> board basis.. >> >> I am fine with just changing the compatible string for DPLL5 to a new one. >> >> Tony, any comment on this? > > I guess I don't quite follow, do you just mean adding a new compatible > for 36xx? > > Can we keep this workaround always enabled for 36xx or does it have > some side effects? DPLL5 entry of omap36xx should have a workaround, but for detection purposes we either need: - A new compatible string in DT for the DPLL5 itself (currently it uses ti,omap3-dpll-clock, so change this to something like ti,omap3-dpll5-clock) - Or make a strcmp against the clock name in the kernel during boot, and register different clkops for it. -Tero > > Regards, > > Tony > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* Tero Kristo <t-kristo@ti.com> [160316 07:38]: > On 03/16/2016 04:33 PM, Tony Lindgren wrote: > >* Tero Kristo <t-kristo@ti.com> [160315 23:27]: > >>On 03/16/2016 04:44 AM, Robert Nelson wrote: > >>>On Tue, Mar 15, 2016 at 7:40 PM, Ladislav Michl <ladis@linux-mips.org> wrote: > >>>>On Tue, Mar 15, 2016 at 08:25:06AM +0000, Richard Watts wrote: > >>>>>[snip] > >>>>>>Yes, I think we should add a top-level ops for dpll5, that would > >>>>>>determine if table dividers shall be used first, if not, then just call > >>>>>>the generic implementation. > >>>> > >>>>Just tried that and it looks reasonable. Will send patch after cleanup. > >>>> > >>>>>>I would also add a new compatible string for the purpose, this means the > >>>>>>users must update both kernel + DTB but I believe any OMAPx customers > >>>>>>are doing this anyway. > >>>>>> > >>>> > >>>>Should DTB also carry fixup table? > >>> > >>>It really should be an optional flag that we can enable on a board by > >>>board basis.. > >> > >>I am fine with just changing the compatible string for DPLL5 to a new one. > >> > >>Tony, any comment on this? > > > >I guess I don't quite follow, do you just mean adding a new compatible > >for 36xx? > > > >Can we keep this workaround always enabled for 36xx or does it have > >some side effects? > > DPLL5 entry of omap36xx should have a workaround, but for detection purposes > we either need: > > - A new compatible string in DT for the DPLL5 itself (currently it uses > ti,omap3-dpll-clock, so change this to something like ti,omap3-dpll5-clock) > - Or make a strcmp against the clock name in the kernel during boot, and > register different clkops for it. I guess up to you. If there's a need to disable the workaround on some boards then the compatible string addition makes sense. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 16/03/16 14:42, Tony Lindgren wrote: > * Tero Kristo <t-kristo@ti.com> [160316 07:38]: >> On 03/16/2016 04:33 PM, Tony Lindgren wrote: >>> * Tero Kristo <t-kristo@ti.com> [160315 23:27]: >>>> On 03/16/2016 04:44 AM, Robert Nelson wrote: >>>>> On Tue, Mar 15, 2016 at 7:40 PM, Ladislav Michl <ladis@linux-mips.org> wrote: >>>>>> On Tue, Mar 15, 2016 at 08:25:06AM +0000, Richard Watts wrote: >>>>>>> [snip] >>>>>>>> Yes, I think we should add a top-level ops for dpll5, that would >>>>>>>> determine if table dividers shall be used first, if not, then just call >>>>>>>> the generic implementation. >>>>>> >>>>>> Just tried that and it looks reasonable. Will send patch after cleanup. >>>>>> >>>>>>>> I would also add a new compatible string for the purpose, this means the >>>>>>>> users must update both kernel + DTB but I believe any OMAPx customers >>>>>>>> are doing this anyway. >>>>>>>> >>>>>> >>>>>> Should DTB also carry fixup table? >>>>> >>>>> It really should be an optional flag that we can enable on a board by >>>>> board basis.. >>>> >>>> I am fine with just changing the compatible string for DPLL5 to a new one. >>>> >>>> Tony, any comment on this? >>> >>> I guess I don't quite follow, do you just mean adding a new compatible >>> for 36xx? >>> >>> Can we keep this workaround always enabled for 36xx or does it have >>> some side effects? >> >> DPLL5 entry of omap36xx should have a workaround, but for detection purposes >> we either need: >> >> - A new compatible string in DT for the DPLL5 itself (currently it uses >> ti,omap3-dpll-clock, so change this to something like ti,omap3-dpll5-clock) >> - Or make a strcmp against the clock name in the kernel during boot, and >> register different clkops for it. > > I guess up to you. If there's a need to disable the workaround on some > boards then the compatible string addition makes sense. Hi, I can't see a need to disable the workaround - if you are messing about with the PLL settings on DPLL5, you will have your own patch in this area anyway. However, there are several possible options for a workaround if you are using a 13MHz or 26MHz xtal - see <http://www.ti.com/lit/er/sprz319f/sprz319f.pdf> PDF p111. It might perhaps be civilised to give the user the option, since which is appropriate to your board is a matter of board characterisation. I see no shame in simply exposing something like: ti,omap3-dpll5-clock = < 443 5 8 > I have a Beagle xM or two here I can send out to anyone in need - throw me an address. OMAP3530 should also suffer from this problem, but it is not listed in the errata sheet. Other than that, the only devices I know of that have this combination of DPLL and HSUSB are OMAP3630 and DM3730 - if it's important, I can try and find the appropriate people in TI? Richard. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* Richard Watts <rrw@kynesim.co.uk> [160316 10:14]: OK > However, there are several possible options for a workaround if you are > using a 13MHz or 26MHz xtal - see > <http://www.ti.com/lit/er/sprz319f/sprz319f.pdf> PDF p111. It might perhaps > be civilised to give the user the option, since which is appropriate to your > board is a matter of board characterisation. Seems like we should just configure dpll5 based on the SYS_CLK rate automatically. > I see no shame in simply exposing something like: > > ti,omap3-dpll5-clock = < 443 5 8 > If we want a binding like that it should be Linux generic and disucced on the linux-clk list. It's usually best to stick to standard bindings and hide the quirks in the driver(s). It seem at most we just need to specify the 36xx related compatible flag for dpll5. > I have a Beagle xM or two here I can send out to anyone in need - throw me an > address. Cool, I think also Tomi Valkeinen was looking for a 36xx test board for occasional DSS testing. I have a beagle xm on loan here so I'm all set for now. > OMAP3530 should also suffer from this problem, but it is not listed in the > errata sheet. > > Other than that, the only devices I know of that have this combination of > DPLL and HSUSB are OMAP3630 and DM3730 - if it's important, I can try and > find the appropriate people in TI? AFAIK 3630 and dm3730 are exactly the same. The dm3730 is just the catalog part GP version. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
--- linux-4.5/drivers/clk/ti/dpll3xxx.c.orig 2016-03-14 12:43:58.085003085 +0100 +++ linux-4.5/drivers/clk/ti/dpll3xxx.c 2016-03-14 13:17:57.893417000 +0100 @@ -308,6 +308,7 @@ u8 dco, sd_div, ai = 0; u32 v; bool errata_i810; + const char *clk_name; /* 3430 ES2 TRM: 4.7.6.9 DPLL Programming Sequence */ _omap3_noncore_dpll_bypass(clk); @@ -370,6 +371,12 @@ } } + clk_name = clk_hw_get_name(&clk->hw); + if (strcmp(clk_name, "dpll5_ck") == 0) { + pr_info("fixing %s (%04lx), %06x -> 01bb05\n", clk_name, dd->mult_div1_reg, v); + v = 0x1bb05; + } + ti_clk_ll_ops->clk_writel(v, dd->mult_div1_reg); /* Set 4X multiplier and low-power mode */ --- linux-4.5/drivers/clk/ti/divider.c.orig 2016-03-14 12:44:06.509211022 +0100 +++ linux-4.5/drivers/clk/ti/divider.c 2016-03-14 13:18:21.793417000 +0100 @@ -215,6 +215,7 @@ struct clk_divider *divider; unsigned int div, value; u32 val; + const char *clk_name; if (!hw || !rate) return -EINVAL; @@ -234,6 +235,13 @@ val &= ~(div_mask(divider) << divider->shift); } val |= value << divider->shift; + + clk_name = clk_hw_get_name(hw); + if (strcmp(clk_name, "dpll5_m2_ck") == 0) { + pr_info("fixing %s (%04lx), %04x -> 0008\n", clk_name, divider->reg, val); + val = 8; + } + ti_clk_ll_ops->clk_writel(val, divider->reg); return 0;
Hi there! seems it's been three years since: [PATCH 1/1] Fix sprz319 erratum 2.1 http://article.gmane.org/gmane.linux.ports.arm.omap/71633 errata: http://www.ti.com/lit/er/sprz319f/sprz319f.pdf Nice summary (third post by Rodrigo Lemos) can be found here: https://github.com/RobertCNelson/stable-kernel/issues/26 so I'm not going to repeat it here. Company I'm working for has few thousands of IGEPv2 boards in field. They have hub connected to usb port with udlfb display, 3G modem and usbserial: /: Bus 03.Port 1: Dev 1, Class=root_hub, Driver=ehci-omap/3p, 480M |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/4p, 480M |__ Port 1: Dev 3, If 0, Class=Vendor Specific Class, Driver=udlfb, 480M |__ Port 3: Dev 4, If 0, Class=Vendor Specific Class, Driver=option, 480M |__ Port 3: Dev 4, If 1, Class=Vendor Specific Class, Driver=, 480M |__ Port 3: Dev 4, If 2, Class=Vendor Specific Class, Driver=option, 480M |__ Port 3: Dev 4, If 3, Class=Vendor Specific Class, Driver=option, 480M |__ Port 3: Dev 4, If 4, Class=Mass Storage, Driver=usb-storage, 480M |__ Port 3: Dev 4, If 5, Class=Mass Storage, Driver=usb-storage, 480M |__ Port 4: Dev 5, If 0, Class=Hub, Driver=hub/4p, 480M |__ Port 3: Dev 6, If 0, Class=Communications, Driver=cdc_acm, 12M |__ Port 3: Dev 6, If 1, Class=CDC Data, Driver=cdc_acm, 12M Few tens of those disconnects USB after while (one minute to few hours) with: "usb usb3-port1: disabled by hub (EMI?), re-enabling..." IGEPv2 uses DM3730 with 26MHz crystal divided by two providing 13MHz SYS_CLK, therefore it is good candidate to fix. Patch bellow was used as a dirty hack to test if usb disconnects go away with errata applied. And indeed, device is already running for few days without any issues. Now question is, how should a proper fix look like as modifiing omap2_dpll_round_rate seems easy enough. Is that acceptable? How to test proper clock to apply errata? Comparing string is suboptimal. Should we introduce some flag? ladis -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html