| Message ID | 1351164400-27940-2-git-send-email-stigge@antcom.de |
|---|---|
| State | New |
| Headers | show |
On Thu, Oct 25, 2012 at 01:26:40PM +0200, Roland Stigge wrote: > This patch fixes CAN clocking on i.MX53. > > Signed-off-by: Roland Stigge <stigge@antcom.de> > > --- > arch/arm/mach-imx/clk-imx51-imx53.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > --- linux-2.6.orig/arch/arm/mach-imx/clk-imx51-imx53.c > +++ linux-2.6/arch/arm/mach-imx/clk-imx51-imx53.c > @@ -426,10 +426,10 @@ int __init mx53_clocks_init(unsigned lon > clk[usb_phy2_gate] = imx_clk_gate2("usb_phy2_gate", "usb_phy_sel", MXC_CCM_CCGR4, 12); > clk[can_sel] = imx_clk_mux("can_sel", MXC_CCM_CSCMR2, 6, 2, > mx53_can_sel, ARRAY_SIZE(mx53_can_sel)); > - clk[can1_serial_gate] = imx_clk_gate2("can1_serial_gate", "can_sel", MXC_CCM_CCGR6, 22); > - clk[can1_ipg_gate] = imx_clk_gate2("can1_ipg_gate", "ipg", MXC_CCM_CCGR6, 20); > - clk[can2_serial_gate] = imx_clk_gate2("can2_serial_gate", "can_sel", MXC_CCM_CCGR4, 8); > - clk[can2_ipg_gate] = imx_clk_gate2("can2_ipg_gate", "ipg", MXC_CCM_CCGR4, 6); I just rechecked. The above matches the i.MX53 Reference Manual rev 2.1 > + clk[can1_serial_gate] = imx_clk_gate2("can1_serial_gate", "can_sel", MXC_CCM_CCGR4, 6); > + clk[can1_ipg_gate] = imx_clk_gate2("can1_ipg_gate", "ipg", MXC_CCM_CCGR4, 8); > + clk[can2_serial_gate] = imx_clk_gate2("can2_serial_gate", "can_sel", MXC_CCM_CCGR4, 6); > + clk[can2_ipg_gate] = imx_clk_gate2("can2_ipg_gate", "ipg", MXC_CCM_CCGR4, 8); This doesn't. Can you elaborate why you think this is necessary? Sascha
Hi! On 10/26/2012 10:59 AM, Sascha Hauer wrote: > On Thu, Oct 25, 2012 at 01:26:40PM +0200, Roland Stigge wrote: >> This patch fixes CAN clocking on i.MX53. >> >> Signed-off-by: Roland Stigge <stigge@antcom.de> >> >> --- >> arch/arm/mach-imx/clk-imx51-imx53.c | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> --- linux-2.6.orig/arch/arm/mach-imx/clk-imx51-imx53.c >> +++ linux-2.6/arch/arm/mach-imx/clk-imx51-imx53.c >> @@ -426,10 +426,10 @@ int __init mx53_clocks_init(unsigned lon >> clk[usb_phy2_gate] = imx_clk_gate2("usb_phy2_gate", "usb_phy_sel", MXC_CCM_CCGR4, 12); >> clk[can_sel] = imx_clk_mux("can_sel", MXC_CCM_CSCMR2, 6, 2, >> mx53_can_sel, ARRAY_SIZE(mx53_can_sel)); >> - clk[can1_serial_gate] = imx_clk_gate2("can1_serial_gate", "can_sel", MXC_CCM_CCGR6, 22); >> - clk[can1_ipg_gate] = imx_clk_gate2("can1_ipg_gate", "ipg", MXC_CCM_CCGR6, 20); >> - clk[can2_serial_gate] = imx_clk_gate2("can2_serial_gate", "can_sel", MXC_CCM_CCGR4, 8); >> - clk[can2_ipg_gate] = imx_clk_gate2("can2_ipg_gate", "ipg", MXC_CCM_CCGR4, 6); > > I just rechecked. The above matches the i.MX53 Reference Manual rev 2.1 > >> + clk[can1_serial_gate] = imx_clk_gate2("can1_serial_gate", "can_sel", MXC_CCM_CCGR4, 6); >> + clk[can1_ipg_gate] = imx_clk_gate2("can1_ipg_gate", "ipg", MXC_CCM_CCGR4, 8); >> + clk[can2_serial_gate] = imx_clk_gate2("can2_serial_gate", "can_sel", MXC_CCM_CCGR4, 6); >> + clk[can2_ipg_gate] = imx_clk_gate2("can2_ipg_gate", "ipg", MXC_CCM_CCGR4, 8); > > This doesn't. This may be right, but unfortunately, since the introduction of your can1 + can2 clocking change, the first block stopped working for me. My above patch is basically a rollback which works for the first block. An interesting hint is: can2 (which was the only one defined _before_ your can1 + can2 change) didn't work before and afterwards at all. (But I'm not using it anyway.) Thanks, Roland
On 10/26/2012 11:16 AM, Roland Stigge wrote: > Hi! > > On 10/26/2012 10:59 AM, Sascha Hauer wrote: >> On Thu, Oct 25, 2012 at 01:26:40PM +0200, Roland Stigge wrote: >>> This patch fixes CAN clocking on i.MX53. >>> >>> Signed-off-by: Roland Stigge <stigge@antcom.de> >>> >>> --- >>> arch/arm/mach-imx/clk-imx51-imx53.c | 8 ++++---- >>> 1 file changed, 4 insertions(+), 4 deletions(-) >>> >>> --- linux-2.6.orig/arch/arm/mach-imx/clk-imx51-imx53.c >>> +++ linux-2.6/arch/arm/mach-imx/clk-imx51-imx53.c >>> @@ -426,10 +426,10 @@ int __init mx53_clocks_init(unsigned lon >>> clk[usb_phy2_gate] = imx_clk_gate2("usb_phy2_gate", "usb_phy_sel", MXC_CCM_CCGR4, 12); >>> clk[can_sel] = imx_clk_mux("can_sel", MXC_CCM_CSCMR2, 6, 2, >>> mx53_can_sel, ARRAY_SIZE(mx53_can_sel)); >>> - clk[can1_serial_gate] = imx_clk_gate2("can1_serial_gate", "can_sel", MXC_CCM_CCGR6, 22); >>> - clk[can1_ipg_gate] = imx_clk_gate2("can1_ipg_gate", "ipg", MXC_CCM_CCGR6, 20); >>> - clk[can2_serial_gate] = imx_clk_gate2("can2_serial_gate", "can_sel", MXC_CCM_CCGR4, 8); >>> - clk[can2_ipg_gate] = imx_clk_gate2("can2_ipg_gate", "ipg", MXC_CCM_CCGR4, 6); >> >> I just rechecked. The above matches the i.MX53 Reference Manual rev 2.1 An old rev 1 manual has lists the same bits. >>> + clk[can1_serial_gate] = imx_clk_gate2("can1_serial_gate", "can_sel", MXC_CCM_CCGR4, 6); >>> + clk[can1_ipg_gate] = imx_clk_gate2("can1_ipg_gate", "ipg", MXC_CCM_CCGR4, 8); >>> + clk[can2_serial_gate] = imx_clk_gate2("can2_serial_gate", "can_sel", MXC_CCM_CCGR4, 6); >>> + clk[can2_ipg_gate] = imx_clk_gate2("can2_ipg_gate", "ipg", MXC_CCM_CCGR4, 8); >> >> This doesn't. > > This may be right, but unfortunately, since the introduction of your > can1 + can2 clocking change, the first block stopped working for me. You are effectively using can2's clock for can1. Are you sure you haven't mixed up can1 and can2? > My above patch is basically a rollback which works for the first block. > > An interesting hint is: can2 (which was the only one defined _before_ > your can1 + can2 change) didn't work before and afterwards at all. (But > I'm not using it anyway.) Marc
On 10/26/2012 12:30 PM, Marc Kleine-Budde wrote: >>>> + clk[can1_serial_gate] = imx_clk_gate2("can1_serial_gate", >>>> "can_sel", MXC_CCM_CCGR4, 6); + clk[can1_ipg_gate] = >>>> imx_clk_gate2("can1_ipg_gate", "ipg", MXC_CCM_CCGR4, 8); + >>>> clk[can2_serial_gate] = imx_clk_gate2("can2_serial_gate", >>>> "can_sel", MXC_CCM_CCGR4, 6); + clk[can2_ipg_gate] = >>>> imx_clk_gate2("can2_ipg_gate", "ipg", MXC_CCM_CCGR4, 8); >>> >>> This doesn't. >> >> This may be right, but unfortunately, since the introduction of >> your can1 + can2 clocking change, the first block stopped working >> for me. > > You are effectively using can2's clock for can1. Are you sure you > haven't mixed up can1 and can2? Just using the above patch and patch 1/2 from this series (missing can pinmuxing), doing like this in custom .dts: + can1: can@53fc8000 { + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_can1_1>; + clock-frequency = <66500000>; + status = "okay"; + }; Then, the can0 interface appears which can be upped as network interface. Quite straightforward, isn't it? Roland
On 10/26/2012 01:05 PM, Roland Stigge wrote: > On 10/26/2012 12:30 PM, Marc Kleine-Budde wrote: >>>>> + clk[can1_serial_gate] = imx_clk_gate2("can1_serial_gate", >>>>> "can_sel", MXC_CCM_CCGR4, 6); + clk[can1_ipg_gate] = >>>>> imx_clk_gate2("can1_ipg_gate", "ipg", MXC_CCM_CCGR4, 8); + >>>>> clk[can2_serial_gate] = imx_clk_gate2("can2_serial_gate", >>>>> "can_sel", MXC_CCM_CCGR4, 6); + clk[can2_ipg_gate] = >>>>> imx_clk_gate2("can2_ipg_gate", "ipg", MXC_CCM_CCGR4, 8); >>>> >>>> This doesn't. >>> >>> This may be right, but unfortunately, since the introduction of >>> your can1 + can2 clocking change, the first block stopped working >>> for me. >> >> You are effectively using can2's clock for can1. Are you sure you >> haven't mixed up can1 and can2? > > Just using the above patch and patch 1/2 from this series (missing can > pinmuxing), doing like this in custom .dts: > > + can1: can@53fc8000 { > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_can1_1>; > + clock-frequency = <66500000>; That was the problem, since in flexcan.c probe(): if (pdev->dev.of_node) of_property_read_u32(pdev->dev.of_node, "clock-frequency", &clock_freq); if (!clock_freq) { clk_ipg = devm_clk_get(&pdev->dev, "ipg"); if (IS_ERR(clk_ipg)) { dev_err(&pdev->dev, "no ipg clock defined\n"); err = PTR_ERR(clk_ipg); goto failed_clock; } clock_freq = clk_get_rate(clk_ipg); clk_per = devm_clk_get(&pdev->dev, "per"); if (IS_ERR(clk_per)) { dev_err(&pdev->dev, "no per clock defined\n"); err = PTR_ERR(clk_per); goto failed_clock; } } Sorry for the noise. The other patches are still current. Thanks, Roland
On 10/26/2012 03:52 PM, Roland Stigge wrote: > On 10/26/2012 01:05 PM, Roland Stigge wrote: >> On 10/26/2012 12:30 PM, Marc Kleine-Budde wrote: >>>>>> + clk[can1_serial_gate] = imx_clk_gate2("can1_serial_gate", >>>>>> "can_sel", MXC_CCM_CCGR4, 6); + clk[can1_ipg_gate] = >>>>>> imx_clk_gate2("can1_ipg_gate", "ipg", MXC_CCM_CCGR4, 8); + >>>>>> clk[can2_serial_gate] = imx_clk_gate2("can2_serial_gate", >>>>>> "can_sel", MXC_CCM_CCGR4, 6); + clk[can2_ipg_gate] = >>>>>> imx_clk_gate2("can2_ipg_gate", "ipg", MXC_CCM_CCGR4, 8); >>>>> >>>>> This doesn't. >>>> >>>> This may be right, but unfortunately, since the introduction of >>>> your can1 + can2 clocking change, the first block stopped working >>>> for me. >>> >>> You are effectively using can2's clock for can1. Are you sure you >>> haven't mixed up can1 and can2? >> >> Just using the above patch and patch 1/2 from this series (missing can >> pinmuxing), doing like this in custom .dts: >> >> + can1: can@53fc8000 { >> + pinctrl-names = "default"; >> + pinctrl-0 = <&pinctrl_can1_1>; >> + clock-frequency = <66500000>; > > That was the problem, since in flexcan.c probe(): > > > if (pdev->dev.of_node) > of_property_read_u32(pdev->dev.of_node, > "clock-frequency", &clock_freq); > > if (!clock_freq) { > clk_ipg = devm_clk_get(&pdev->dev, "ipg"); > if (IS_ERR(clk_ipg)) { > dev_err(&pdev->dev, "no ipg clock defined\n"); > err = PTR_ERR(clk_ipg); > goto failed_clock; > } > clock_freq = clk_get_rate(clk_ipg); > > clk_per = devm_clk_get(&pdev->dev, "per"); > if (IS_ERR(clk_per)) { > dev_err(&pdev->dev, "no per clock defined\n"); > err = PTR_ERR(clk_per); > goto failed_clock; > } > } > > Sorry for the noise. We have the possibility to specify the clock frequency in the device tree as the power pc has no support for generic clock yet. Marc
Roland, Do you have updates on this one? I think this patch is wrong, specifying the clock rate in the devicetree is only for PowerPC, so I think this needs further investigation. Sascha On Thu, Oct 25, 2012 at 01:26:40PM +0200, Roland Stigge wrote: > This patch fixes CAN clocking on i.MX53. > > Signed-off-by: Roland Stigge <stigge@antcom.de> > > --- > arch/arm/mach-imx/clk-imx51-imx53.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > --- linux-2.6.orig/arch/arm/mach-imx/clk-imx51-imx53.c > +++ linux-2.6/arch/arm/mach-imx/clk-imx51-imx53.c > @@ -426,10 +426,10 @@ int __init mx53_clocks_init(unsigned lon > clk[usb_phy2_gate] = imx_clk_gate2("usb_phy2_gate", "usb_phy_sel", MXC_CCM_CCGR4, 12); > clk[can_sel] = imx_clk_mux("can_sel", MXC_CCM_CSCMR2, 6, 2, > mx53_can_sel, ARRAY_SIZE(mx53_can_sel)); > - clk[can1_serial_gate] = imx_clk_gate2("can1_serial_gate", "can_sel", MXC_CCM_CCGR6, 22); > - clk[can1_ipg_gate] = imx_clk_gate2("can1_ipg_gate", "ipg", MXC_CCM_CCGR6, 20); > - clk[can2_serial_gate] = imx_clk_gate2("can2_serial_gate", "can_sel", MXC_CCM_CCGR4, 8); > - clk[can2_ipg_gate] = imx_clk_gate2("can2_ipg_gate", "ipg", MXC_CCM_CCGR4, 6); > + clk[can1_serial_gate] = imx_clk_gate2("can1_serial_gate", "can_sel", MXC_CCM_CCGR4, 6); > + clk[can1_ipg_gate] = imx_clk_gate2("can1_ipg_gate", "ipg", MXC_CCM_CCGR4, 8); > + clk[can2_serial_gate] = imx_clk_gate2("can2_serial_gate", "can_sel", MXC_CCM_CCGR4, 6); > + clk[can2_ipg_gate] = imx_clk_gate2("can2_ipg_gate", "ipg", MXC_CCM_CCGR4, 8); > clk[i2c3_gate] = imx_clk_gate2("i2c3_gate", "per_root", MXC_CCM_CCGR1, 22); > > for (i = 0; i < ARRAY_SIZE(clk); i++) >
On 29/10/12 22:20, Sascha Hauer wrote: > Roland, > > Do you have updates on this one? I think this patch is wrong, specifying > the clock rate in the devicetree is only for PowerPC, so I think this > needs further investigation. Please ignore it for now. As discussed with Marc, the issue seemed to be caused by me (wrongfully) using the "clock-frequency" dt property which caused probe() to ignore the clocks. A colleague of mine will notify me if the problem surfaces again. So for now I assume it's gone. Maybe the above strategy regarding "clock-frequency" needs to be documented better or changed, though, to prevent more confusion like in the above case. Thanks, Roland > On Thu, Oct 25, 2012 at 01:26:40PM +0200, Roland Stigge wrote: >> This patch fixes CAN clocking on i.MX53. >> >> Signed-off-by: Roland Stigge <stigge@antcom.de> >> >> --- >> arch/arm/mach-imx/clk-imx51-imx53.c | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> --- linux-2.6.orig/arch/arm/mach-imx/clk-imx51-imx53.c >> +++ linux-2.6/arch/arm/mach-imx/clk-imx51-imx53.c >> @@ -426,10 +426,10 @@ int __init mx53_clocks_init(unsigned lon >> clk[usb_phy2_gate] = imx_clk_gate2("usb_phy2_gate", "usb_phy_sel", MXC_CCM_CCGR4, 12); >> clk[can_sel] = imx_clk_mux("can_sel", MXC_CCM_CSCMR2, 6, 2, >> mx53_can_sel, ARRAY_SIZE(mx53_can_sel)); >> - clk[can1_serial_gate] = imx_clk_gate2("can1_serial_gate", "can_sel", MXC_CCM_CCGR6, 22); >> - clk[can1_ipg_gate] = imx_clk_gate2("can1_ipg_gate", "ipg", MXC_CCM_CCGR6, 20); >> - clk[can2_serial_gate] = imx_clk_gate2("can2_serial_gate", "can_sel", MXC_CCM_CCGR4, 8); >> - clk[can2_ipg_gate] = imx_clk_gate2("can2_ipg_gate", "ipg", MXC_CCM_CCGR4, 6); >> + clk[can1_serial_gate] = imx_clk_gate2("can1_serial_gate", "can_sel", MXC_CCM_CCGR4, 6); >> + clk[can1_ipg_gate] = imx_clk_gate2("can1_ipg_gate", "ipg", MXC_CCM_CCGR4, 8); >> + clk[can2_serial_gate] = imx_clk_gate2("can2_serial_gate", "can_sel", MXC_CCM_CCGR4, 6); >> + clk[can2_ipg_gate] = imx_clk_gate2("can2_ipg_gate", "ipg", MXC_CCM_CCGR4, 8); >> clk[i2c3_gate] = imx_clk_gate2("i2c3_gate", "per_root", MXC_CCM_CCGR1, 22); >> >> for (i = 0; i < ARRAY_SIZE(clk); i++) >> >
--- linux-2.6.orig/arch/arm/mach-imx/clk-imx51-imx53.c +++ linux-2.6/arch/arm/mach-imx/clk-imx51-imx53.c @@ -426,10 +426,10 @@ int __init mx53_clocks_init(unsigned lon clk[usb_phy2_gate] = imx_clk_gate2("usb_phy2_gate", "usb_phy_sel", MXC_CCM_CCGR4, 12); clk[can_sel] = imx_clk_mux("can_sel", MXC_CCM_CSCMR2, 6, 2, mx53_can_sel, ARRAY_SIZE(mx53_can_sel)); - clk[can1_serial_gate] = imx_clk_gate2("can1_serial_gate", "can_sel", MXC_CCM_CCGR6, 22); - clk[can1_ipg_gate] = imx_clk_gate2("can1_ipg_gate", "ipg", MXC_CCM_CCGR6, 20); - clk[can2_serial_gate] = imx_clk_gate2("can2_serial_gate", "can_sel", MXC_CCM_CCGR4, 8); - clk[can2_ipg_gate] = imx_clk_gate2("can2_ipg_gate", "ipg", MXC_CCM_CCGR4, 6); + clk[can1_serial_gate] = imx_clk_gate2("can1_serial_gate", "can_sel", MXC_CCM_CCGR4, 6); + clk[can1_ipg_gate] = imx_clk_gate2("can1_ipg_gate", "ipg", MXC_CCM_CCGR4, 8); + clk[can2_serial_gate] = imx_clk_gate2("can2_serial_gate", "can_sel", MXC_CCM_CCGR4, 6); + clk[can2_ipg_gate] = imx_clk_gate2("can2_ipg_gate", "ipg", MXC_CCM_CCGR4, 8); clk[i2c3_gate] = imx_clk_gate2("i2c3_gate", "per_root", MXC_CCM_CCGR1, 22); for (i = 0; i < ARRAY_SIZE(clk); i++)
This patch fixes CAN clocking on i.MX53. Signed-off-by: Roland Stigge <stigge@antcom.de> --- arch/arm/mach-imx/clk-imx51-imx53.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)