| Message ID | 20190412120730.473-1-megous@megous.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Add basic support for RTC on Allwinner H6 SoC | expand |
On Fri, Apr 12, 2019 at 8:07 PM megous via linux-sunxi <linux-sunxi@googlegroups.com> wrote: > > From: Ondrej Jirman <megous@megous.com> > > I went through the datasheets for H6 and H5, and compared the differences. > RTCs are largely similar, but not entirely compatible. Incompatibilities > are in details not yet implemented by the rtc driver though. > > I also corrected the clock tree in H6 DTSI. Please also add DCXO clock input/output and XO clock input to the bindings and DT, and also fix up the clock tree. You can skip them in the driver for now, but please add a TODO. As long as you don't change the clock-output-name of osc24M, everything should work as before. We just want the DT to describe what is actually there. For the XO input, you could just directly reference the external crystal node. The gate for it is likely somewhere in the PRCM block, which we don't have docs for. > There's a small detail here, that's not described absolutely correctly in > DTSI, but the difference is not really that material. ext_osc32k is > originally modelled as a fixed clock that feeds into RTC module, but in > reality it's the RTC module that implements via its registers enabling and > disabling of this oscillator/clock. > > Though: > - there's no other possible user of ext_osc32k than RTC module > - there's no other possible external configuration for the crystal > circuit that would need to be handled in the dts per board > > So I guess, while the description is not perfect, this patch series still > improves the current situation. Or maybe I'm misunderstanding something, > and &ext_osc32k node just describes a fact that there's a crystal on > the board. Then, everything is perhaps fine. :) Correct. The external clock nodes are modeling the crystal, not the internal clock gate / distributor. Were the vendor to not include the crystal (for whatever reasons), the DT should be able to describe it via the absence of the clock input, and the driver should correctly use the internal (inaccurate) oscillator. I realize the clocks property is required, and the driver doesn't handle this case either, so we might have to fix that if it were to appear in the wild. > For now, the enable bit for this oscillator is toggled by the re-parenting > code automatically, as needed. That's fine. No need to increase the clock tree depth. ChenYu > This patchset is necessary for implementing the WiFi/Bluetooth support > on boards using H6 SoC. > > Please take a look. > > Thank you and regards, > Ondrej Jirman > > Ondrej Jirman (3): > dt-bindings: Add compatible for H6 RTC > rtc: sun6i: Add support for H6 RTC > arm64: dts: sun50i-h6: Add support for RTC and fix the clock tree > > .../devicetree/bindings/rtc/sun6i-rtc.txt | 1 + > arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 30 +++++++------- > drivers/rtc/rtc-sun6i.c | 40 ++++++++++++++++++- > 3 files changed, 55 insertions(+), 16 deletions(-) > > -- > 2.21.0 > > -- > You received this message because you are subscribed to the Google Groups "linux-sunxi" group. > To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com. > For more options, visit https://groups.google.com/d/optout.
Hi ChenYu, On Mon, Apr 15, 2019 at 04:18:12PM +0800, Chen-Yu Tsai wrote: > On Fri, Apr 12, 2019 at 8:07 PM megous via linux-sunxi > <linux-sunxi@googlegroups.com> wrote: > > > > From: Ondrej Jirman <megous@megous.com> > > > > I went through the datasheets for H6 and H5, and compared the differences. > > RTCs are largely similar, but not entirely compatible. Incompatibilities > > are in details not yet implemented by the rtc driver though. > > > > I also corrected the clock tree in H6 DTSI. > > Please also add DCXO clock input/output and XO clock input to the bindings > and DT, and also fix up the clock tree. You can skip them in the driver for > now, but please add a TODO. As long as you don't change the clock-output-name > of osc24M, everything should work as before. That's a bit confusing. There's no clock-output-name for osc24M, nor for input clock used in the dt-bindings or the driver. Perhaps you meant osc32k? Maybe I'm misunderstanding something? If you look at the datasheet page 349, it looks like RTC provides "hosc" clock (to plls and the system) either from XO or DCXO oscillators. The default selection depends on the voltage level on external PAD. So based on what you wrote, I suggest these actual changes/names: 1) Add DT docs for HOSC clock provided at index 3: - 3: HOSC, 24MHz clock that clocks the PLLs and most of the SoC (H6 only) 2) Add bindings description for "osc24M-dc", "osc24M-m" input clocks in addition to existing support for "osc32k". Name "osc24M-m" is based on X24MIN/MOUT pins and datasheet's "clk_24mxo" name. 3) The RTC driver would now just registers a fixed HOSC clock with a name gathered from the clock-output-names index 3 (if enabled by the new export_hosc flag - only enabled on H6). The driver would ignore the "osc24M-dc", "osc24M-m" input clocks. Or perhaps it could just support a case where only one of these are used and make it the only parent of the HOSC clock? HOSC default source selection is done based on external PAD setup, and there's no need for runtime access/selection of HOSC source at the moment. 4) In the future the RTC driver would be extended to support more refined setup/muxing/runtime selection of osc24M-dc/osc24M-m. PRCM driver would provide the osc24M-m clock, to be able for kernel to know how to gate it. The board's DTSI would have to link either "osc24M-dc", "osc24M-m" to nodes describing an external crystal (or to PRCM clock in the future). It's a boards choice on what crystals are actually used. 3 configs are possible - with one or two crystals, connected to either one of XIN/XOUT X24MIN/X24MOUT pins or both. Would that work? DT would still probably need a re-work in the future, if the PRCM clock modeling the gate would be needed. regards, o. > We just want the DT to describe what is actually there. For the XO input, > you could just directly reference the external crystal node. The gate for > it is likely somewhere in the PRCM block, which we don't have docs for. > > > There's a small detail here, that's not described absolutely correctly in > > DTSI, but the difference is not really that material. ext_osc32k is > > originally modelled as a fixed clock that feeds into RTC module, but in > > reality it's the RTC module that implements via its registers enabling and > > disabling of this oscillator/clock. > > > > Though: > > - there's no other possible user of ext_osc32k than RTC module > > - there's no other possible external configuration for the crystal > > circuit that would need to be handled in the dts per board > > > > So I guess, while the description is not perfect, this patch series still > > improves the current situation. Or maybe I'm misunderstanding something, > > and &ext_osc32k node just describes a fact that there's a crystal on > > the board. Then, everything is perhaps fine. :) > > Correct. The external clock nodes are modeling the crystal, not the internal > clock gate / distributor. > > Were the vendor to not include the crystal (for whatever reasons), the DT > should be able to describe it via the absence of the clock input, and the > driver should correctly use the internal (inaccurate) oscillator. I realize > the clocks property is required, and the driver doesn't handle this case > either, so we might have to fix that if it were to appear in the wild. > > > For now, the enable bit for this oscillator is toggled by the re-parenting > > code automatically, as needed. > > That's fine. No need to increase the clock tree depth. > > ChenYu > > > This patchset is necessary for implementing the WiFi/Bluetooth support > > on boards using H6 SoC. > > > > Please take a look. > > > > Thank you and regards, > > Ondrej Jirman > > > > Ondrej Jirman (3): > > dt-bindings: Add compatible for H6 RTC > > rtc: sun6i: Add support for H6 RTC > > arm64: dts: sun50i-h6: Add support for RTC and fix the clock tree > > > > .../devicetree/bindings/rtc/sun6i-rtc.txt | 1 + > > arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 30 +++++++------- > > drivers/rtc/rtc-sun6i.c | 40 ++++++++++++++++++- > > 3 files changed, 55 insertions(+), 16 deletions(-) > > > > -- > > 2.21.0 > > > > -- > > You received this message because you are subscribed to the Google Groups "linux-sunxi" group. > > To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com. > > For more options, visit https://groups.google.com/d/optout.
On Mon, Apr 15, 2019 at 04:22:22PM +0200, Ondřej Jirman wrote: > DT would still probably need a re-work in the future, if the PRCM clock > modeling the gate would be needed. Just reacting to that bit. I know we're pretty bad at this, and the documentation doesn't make it any easier, but if you have any idea of how it should be modelled next, then do that. There's no reason to push it to a later time, it makes everyone's life harder. Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
On Mon, Apr 15, 2019 at 10:22 PM 'Ondřej Jirman' via linux-sunxi <linux-sunxi@googlegroups.com> wrote: > > Hi ChenYu, > > On Mon, Apr 15, 2019 at 04:18:12PM +0800, Chen-Yu Tsai wrote: > > On Fri, Apr 12, 2019 at 8:07 PM megous via linux-sunxi > > <linux-sunxi@googlegroups.com> wrote: > > > > > > From: Ondrej Jirman <megous@megous.com> > > > > > > I went through the datasheets for H6 and H5, and compared the differences. > > > RTCs are largely similar, but not entirely compatible. Incompatibilities > > > are in details not yet implemented by the rtc driver though. > > > > > > I also corrected the clock tree in H6 DTSI. > > > > Please also add DCXO clock input/output and XO clock input to the bindings > > and DT, and also fix up the clock tree. You can skip them in the driver for > > now, but please add a TODO. As long as you don't change the clock-output-name > > of osc24M, everything should work as before. > > That's a bit confusing. There's no clock-output-name for osc24M, nor for input > clock used in the dt-bindings or the driver. Perhaps you meant osc32k? Maybe > I'm misunderstanding something? I meant the clock-output-names in the device node of the external 24M crystal. > If you look at the datasheet page 349, it looks like RTC provides "hosc" > clock (to plls and the system) either from XO or DCXO oscillators. > The default selection depends on the voltage level on external PAD. > > So based on what you wrote, I suggest these actual changes/names: > > 1) Add DT docs for HOSC clock provided at index 3: > > - 3: HOSC, 24MHz clock that clocks the PLLs and most of the SoC (H6 only) Correct. > 2) Add bindings description for "osc24M-dc", "osc24M-m" input clocks in > addition to existing support for "osc32k". Name "osc24M-m" is based on > X24MIN/MOUT pins and datasheet's "clk_24mxo" name. > > 3) The RTC driver would now just registers a fixed HOSC clock with a name > gathered from the clock-output-names index 3 (if enabled by the new > export_hosc flag - only enabled on H6). You don't need to do this part yet. Since the CCU drivers are hard-wired (suprise) to use the global clock name "osc24M" as hosc source. The DT references are only for show ATM, so it doesn't matter if you implement the clocks in the RTC driver. However we want the DT to be correct, so that when we do get around to doing it, we won't have to update the DT again. It's up to you though. If you want to implement basic support, that's fine by me. However you won't be able to test it without hacking the CCU driver. After describing this, it seems that when we get to doing the clk parent rework, we'll be in a bad situation if we don't get rtc changes in before the CCU changes. > The driver would ignore the "osc24M-dc", "osc24M-m" input clocks. Or perhaps > it could just support a case where only one of these are used and make it the > only parent of the HOSC clock? They should just be DCXO and XO, based on the diagram. The names are local to the RTC, so they don't need to be globally unique. Whatever matches the datasheet is best. > HOSC default source selection is done based on external PAD setup, and > there's no need for runtime access/selection of HOSC source at the moment. Is it even possible to change it? > 4) In the future the RTC driver would be extended to support more refined > setup/muxing/runtime selection of osc24M-dc/osc24M-m. PRCM driver would > provide the osc24M-m clock, to be able for kernel to know how to gate it. > > The board's DTSI would have to link either "osc24M-dc", "osc24M-m" to nodes > describing an external crystal (or to PRCM clock in the future). It's a boards > choice on what crystals are actually used. 3 configs are possible - with one or > two crystals, connected to either one of XIN/XOUT X24MIN/X24MOUT pins or both. AFAIK, osc24M-dc would link directly to the external crystal, while osc24M-m would link to the external crystal first, then PRCM if it gets implemented. > Would that work? > > DT would still probably need a re-work in the future, if the PRCM clock > modeling the gate would be needed. Yeah. We'll deal with that when we get to it. To summarize, the goal is to get the DT right the first time. Regards ChenYu > regards, > o. > > > We just want the DT to describe what is actually there. For the XO input, > > you could just directly reference the external crystal node. The gate for > > it is likely somewhere in the PRCM block, which we don't have docs for. > > > > > There's a small detail here, that's not described absolutely correctly in > > > DTSI, but the difference is not really that material. ext_osc32k is > > > originally modelled as a fixed clock that feeds into RTC module, but in > > > reality it's the RTC module that implements via its registers enabling and > > > disabling of this oscillator/clock. > > > > > > Though: > > > - there's no other possible user of ext_osc32k than RTC module > > > - there's no other possible external configuration for the crystal > > > circuit that would need to be handled in the dts per board > > > > > > So I guess, while the description is not perfect, this patch series still > > > improves the current situation. Or maybe I'm misunderstanding something, > > > and &ext_osc32k node just describes a fact that there's a crystal on > > > the board. Then, everything is perhaps fine. :) > > > > Correct. The external clock nodes are modeling the crystal, not the internal > > clock gate / distributor. > > > > Were the vendor to not include the crystal (for whatever reasons), the DT > > should be able to describe it via the absence of the clock input, and the > > driver should correctly use the internal (inaccurate) oscillator. I realize > > the clocks property is required, and the driver doesn't handle this case > > either, so we might have to fix that if it were to appear in the wild. > > > > > For now, the enable bit for this oscillator is toggled by the re-parenting > > > code automatically, as needed. > > > > That's fine. No need to increase the clock tree depth. > > > > ChenYu > > > > > This patchset is necessary for implementing the WiFi/Bluetooth support > > > on boards using H6 SoC. > > > > > > Please take a look. > > > > > > Thank you and regards, > > > Ondrej Jirman > > > > > > Ondrej Jirman (3): > > > dt-bindings: Add compatible for H6 RTC > > > rtc: sun6i: Add support for H6 RTC > > > arm64: dts: sun50i-h6: Add support for RTC and fix the clock tree > > > > > > .../devicetree/bindings/rtc/sun6i-rtc.txt | 1 + > > > arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 30 +++++++------- > > > drivers/rtc/rtc-sun6i.c | 40 ++++++++++++++++++- > > > 3 files changed, 55 insertions(+), 16 deletions(-) > > > > > > -- > > > 2.21.0 > > > > > > -- > > > You received this message because you are subscribed to the Google Groups "linux-sunxi" group. > > > To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com. > > > For more options, visit https://groups.google.com/d/optout. > > -- > You received this message because you are subscribed to the Google Groups "linux-sunxi" group. > To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com. > For more options, visit https://groups.google.com/d/optout.
On Mon, Apr 15, 2019 at 10:33 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > On Mon, Apr 15, 2019 at 04:22:22PM +0200, Ondřej Jirman wrote: > > DT would still probably need a re-work in the future, if the PRCM clock > > modeling the gate would be needed. > > Just reacting to that bit. > > I know we're pretty bad at this, and the documentation doesn't make it > any easier, but if you have any idea of how it should be modelled > next, then do that. It seems to be just a clock gate, judging by previous (A80/A83T) SoCs. However since we lack documents for the PRCM, we might need to poke around to verify that it's actually there. Or maybe we could ask Allwinner very specific questions, like: - Does the PRCM have a gate controlling XO? - Is it bit 2 in register 0x44 in the PRCM? ChenYu > There's no reason to push it to a later time, it makes everyone's life > harder. > > Maxime > > -- > Maxime Ripard, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com > > -- > You received this message because you are subscribed to the Google Groups "linux-sunxi" group. > To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com. > For more options, visit https://groups.google.com/d/optout.
On Mon, Apr 15, 2019 at 10:35:47PM +0800, Chen-Yu Tsai wrote: > On Mon, Apr 15, 2019 at 10:22 PM 'Ondřej Jirman' via linux-sunxi > <linux-sunxi@googlegroups.com> wrote: > > > > Hi ChenYu, > > > > On Mon, Apr 15, 2019 at 04:18:12PM +0800, Chen-Yu Tsai wrote: > > > On Fri, Apr 12, 2019 at 8:07 PM megous via linux-sunxi > > > <linux-sunxi@googlegroups.com> wrote: > > > > > > > > From: Ondrej Jirman <megous@megous.com> > > > > > > > > I went through the datasheets for H6 and H5, and compared the differences. > > > > RTCs are largely similar, but not entirely compatible. Incompatibilities > > > > are in details not yet implemented by the rtc driver though. > > > > > > > > I also corrected the clock tree in H6 DTSI. > > > > > > Please also add DCXO clock input/output and XO clock input to the bindings > > > and DT, and also fix up the clock tree. You can skip them in the driver for > > > now, but please add a TODO. As long as you don't change the clock-output-name > > > of osc24M, everything should work as before. > > > > That's a bit confusing. There's no clock-output-name for osc24M, nor for input > > clock used in the dt-bindings or the driver. Perhaps you meant osc32k? Maybe > > I'm misunderstanding something? > > I meant the clock-output-names in the device node of the external 24M crystal. > > > If you look at the datasheet page 349, it looks like RTC provides "hosc" > > clock (to plls and the system) either from XO or DCXO oscillators. > > The default selection depends on the voltage level on external PAD. > > > > So based on what you wrote, I suggest these actual changes/names: > > > > 1) Add DT docs for HOSC clock provided at index 3: > > > > - 3: HOSC, 24MHz clock that clocks the PLLs and most of the SoC (H6 only) > > Correct. > > > 2) Add bindings description for "osc24M-dc", "osc24M-m" input clocks in > > addition to existing support for "osc32k". Name "osc24M-m" is based on > > X24MIN/MOUT pins and datasheet's "clk_24mxo" name. > > > > 3) The RTC driver would now just registers a fixed HOSC clock with a name > > gathered from the clock-output-names index 3 (if enabled by the new > > export_hosc flag - only enabled on H6). > > You don't need to do this part yet. Since the CCU drivers are hard-wired > (suprise) to use the global clock name "osc24M" as hosc source. The DT > references are only for show ATM, so it doesn't matter if you implement > the clocks in the RTC driver. Ah, so that's how it works. And that's what "clock parent rework" refers to! :) > However we want the DT to be correct, so that when we do get around to > doing it, we won't have to update the DT again. Ok, so I'll try to come up with a new set of patches, and we'll see if I'll get the description right. > It's up to you though. If you want to implement basic support, that's > fine by me. However you won't be able to test it without hacking the > CCU driver. > > After describing this, it seems that when we get to doing the clk parent > rework, we'll be in a bad situation if we don't get rtc changes in before > the CCU changes. Yeah. > > The driver would ignore the "osc24M-dc", "osc24M-m" input clocks. Or perhaps > > it could just support a case where only one of these are used and make it the > > only parent of the HOSC clock? > > They should just be DCXO and XO, based on the diagram. The names are local > to the RTC, so they don't need to be globally unique. Whatever matches the > datasheet is best. Ok. > > HOSC default source selection is done based on external PAD setup, and > > there's no need for runtime access/selection of HOSC source at the moment. > > Is it even possible to change it? Hmm. Looks like the answer is no: DCXO_CTRL_REG: - OSC_CLK_SRC_SEL bit: (Pad select) Read/Write column contains 'U' not (R/W) > > 4) In the future the RTC driver would be extended to support more refined > > setup/muxing/runtime selection of osc24M-dc/osc24M-m. PRCM driver would > > provide the osc24M-m clock, to be able for kernel to know how to gate it. > > > > The board's DTSI would have to link either "osc24M-dc", "osc24M-m" to nodes > > describing an external crystal (or to PRCM clock in the future). It's a boards > > choice on what crystals are actually used. 3 configs are possible - with one or > > two crystals, connected to either one of XIN/XOUT X24MIN/X24MOUT pins or both. > > AFAIK, osc24M-dc would link directly to the external crystal, while osc24M-m > would link to the external crystal first, then PRCM if it gets implemented. Ok. thank you, o. > > Would that work? > > > > DT would still probably need a re-work in the future, if the PRCM clock > > modeling the gate would be needed. > > Yeah. We'll deal with that when we get to it. > > > To summarize, the goal is to get the DT right the first time. > > Regards > ChenYu > > > > regards, > > o. > > > > > We just want the DT to describe what is actually there. For the XO input, > > > you could just directly reference the external crystal node. The gate for > > > it is likely somewhere in the PRCM block, which we don't have docs for. > > > > > > > There's a small detail here, that's not described absolutely correctly in > > > > DTSI, but the difference is not really that material. ext_osc32k is > > > > originally modelled as a fixed clock that feeds into RTC module, but in > > > > reality it's the RTC module that implements via its registers enabling and > > > > disabling of this oscillator/clock. > > > > > > > > Though: > > > > - there's no other possible user of ext_osc32k than RTC module > > > > - there's no other possible external configuration for the crystal > > > > circuit that would need to be handled in the dts per board > > > > > > > > So I guess, while the description is not perfect, this patch series still > > > > improves the current situation. Or maybe I'm misunderstanding something, > > > > and &ext_osc32k node just describes a fact that there's a crystal on > > > > the board. Then, everything is perhaps fine. :) > > > > > > Correct. The external clock nodes are modeling the crystal, not the internal > > > clock gate / distributor. > > > > > > Were the vendor to not include the crystal (for whatever reasons), the DT > > > should be able to describe it via the absence of the clock input, and the > > > driver should correctly use the internal (inaccurate) oscillator. I realize > > > the clocks property is required, and the driver doesn't handle this case > > > either, so we might have to fix that if it were to appear in the wild. > > > > > > > For now, the enable bit for this oscillator is toggled by the re-parenting > > > > code automatically, as needed. > > > > > > That's fine. No need to increase the clock tree depth. > > > > > > ChenYu > > > > > > > This patchset is necessary for implementing the WiFi/Bluetooth support > > > > on boards using H6 SoC. > > > > > > > > Please take a look. > > > > > > > > Thank you and regards, > > > > Ondrej Jirman > > > > > > > > Ondrej Jirman (3): > > > > dt-bindings: Add compatible for H6 RTC > > > > rtc: sun6i: Add support for H6 RTC > > > > arm64: dts: sun50i-h6: Add support for RTC and fix the clock tree > > > > > > > > .../devicetree/bindings/rtc/sun6i-rtc.txt | 1 + > > > > arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 30 +++++++------- > > > > drivers/rtc/rtc-sun6i.c | 40 ++++++++++++++++++- > > > > 3 files changed, 55 insertions(+), 16 deletions(-) > > > > > > > > -- > > > > 2.21.0 > > > > > > > > -- > > > > You received this message because you are subscribed to the Google Groups "linux-sunxi" group. > > > > To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com. > > > > For more options, visit https://groups.google.com/d/optout. > > > > -- > > You received this message because you are subscribed to the Google Groups "linux-sunxi" group. > > To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com. > > For more options, visit https://groups.google.com/d/optout.
On Mon, Apr 15, 2019 at 04:18:12PM +0800, Chen-Yu Tsai wrote: > On Fri, Apr 12, 2019 at 8:07 PM megous via linux-sunxi > <linux-sunxi@googlegroups.com> wrote: > > > > From: Ondrej Jirman <megous@megous.com> > > > > I went through the datasheets for H6 and H5, and compared the differences. > > RTCs are largely similar, but not entirely compatible. Incompatibilities > > are in details not yet implemented by the rtc driver though. > > > > I also corrected the clock tree in H6 DTSI. > > Please also add DCXO clock input/output and XO clock input to the bindings > and DT, and also fix up the clock tree. You can skip them in the driver for > now, but please add a TODO. As long as you don't change the clock-output-name > of osc24M, everything should work as before. > > We just want the DT to describe what is actually there. For the XO input, > you could just directly reference the external crystal node. The gate for > it is likely somewhere in the PRCM block, which we don't have docs for. So I was thinking about this for a while, and came up with this: ----------------- arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi ----------------- index 64c39f663d22..ac99ddbebe5c 100644 @@ -627,14 +635,15 @@ rtc: rtc@7000000 { compatible = "allwinner,sun50i-h6-rtc"; reg = <0x07000000 0x400>; interrupts = <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>; - clock-output-names = "osc32k", "osc32k-out", "iosc"; - clocks = <&ext_osc32k>; + clock-output-names = "osc32k", "osc32k-out", "iosc", "hosc"; + clock-names = "losc", "dcxo"; + clocks = <&ext_osc32k>, <&osc24M>; #clock-cells = <1>; }; r_ccu: clock@7010000 { compatible = "allwinner,sun50i-h6-r-ccu"; reg = <0x07010000 0x400>; I'm not completely sure how (or why?) to describe in DTSI which oscillator the designer used (XO vs DCXO). This information is signalled by the pad voltage and can be determined at runtime from DCXO_CTRL_REG's OSC_CLK_SRC_SEL (bit 3). It's not possible to change at runtime. HOSC source selection is only material to the CPUS (ARISC) firmware when it wants to turn off all PLLs and the main crystal oscillator so that it knows which one to turn off. I don't see any other use for it. It's just informational. I don't think (future) crust firmware has space to be reading DTBs, so the detection will be using OSC_CLK_SRC_SEL anyway. Maybe whether XO or DCXO is used also matters if you want to do some fine tunning of DCXO (control register has pletny of options), but that's probably better done in u-boot. And there's still no need to read HOSC source from DT. The driver can just check compatible, and if it is H6 and OSC_CLK_SRC_SEL is 1, it can do it's DCXO tunning, or whatever. But neither OS nor bootloader will be using this info to gate/disable the osciallator. If we really want this in DT, maybe we can model it by having just two input clocks to RTC described in DTSI, and the DTSI for H6 would have this by default: clock-names = "losc", "dcxo"; clocks = <&ext_osc32k>, <&osc24M>; And the board designer could change it from a board file, like this: &rtc { clock-names = "losc", "xo"; clocks = <&ext_osc32k>, <&osc24M>; }; The driver could decide which oscillator is used by the presence of either dcxo or xo input clock. But in any case, the driver can also get this info from DCXO_CTRL_REG's OSC_CLK_SRC_SEL, so it doesn't need to read this from DT at all. So it's a bit pointless. So I see two options: 1) skip adding dcxo/xo to input clocks of RTC completely 2) the above What do you think? regards, o. > > There's a small detail here, that's not described absolutely correctly in > > DTSI, but the difference is not really that material. ext_osc32k is > > originally modelled as a fixed clock that feeds into RTC module, but in > > reality it's the RTC module that implements via its registers enabling and > > disabling of this oscillator/clock. > > > > Though: > > - there's no other possible user of ext_osc32k than RTC module > > - there's no other possible external configuration for the crystal > > circuit that would need to be handled in the dts per board > > > > So I guess, while the description is not perfect, this patch series still > > improves the current situation. Or maybe I'm misunderstanding something, > > and &ext_osc32k node just describes a fact that there's a crystal on > > the board. Then, everything is perhaps fine. :) > > Correct. The external clock nodes are modeling the crystal, not the internal > clock gate / distributor. > > Were the vendor to not include the crystal (for whatever reasons), the DT > should be able to describe it via the absence of the clock input, and the > driver should correctly use the internal (inaccurate) oscillator. I realize > the clocks property is required, and the driver doesn't handle this case > either, so we might have to fix that if it were to appear in the wild. > > > For now, the enable bit for this oscillator is toggled by the re-parenting > > code automatically, as needed. > > That's fine. No need to increase the clock tree depth. > > ChenYu > > > This patchset is necessary for implementing the WiFi/Bluetooth support > > on boards using H6 SoC. > > > > Please take a look. > > > > Thank you and regards, > > Ondrej Jirman > > > > Ondrej Jirman (3): > > dt-bindings: Add compatible for H6 RTC > > rtc: sun6i: Add support for H6 RTC > > arm64: dts: sun50i-h6: Add support for RTC and fix the clock tree > > > > .../devicetree/bindings/rtc/sun6i-rtc.txt | 1 + > > arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 30 +++++++------- > > drivers/rtc/rtc-sun6i.c | 40 ++++++++++++++++++- > > 3 files changed, 55 insertions(+), 16 deletions(-) > > > > -- > > 2.21.0 > > > > -- > > You received this message because you are subscribed to the Google Groups "linux-sunxi" group. > > To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com. > > For more options, visit https://groups.google.com/d/optout. > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Tue, Aug 06, 2019 at 08:30:45PM +0200, megous hlavni wrote: > On Mon, Apr 15, 2019 at 04:18:12PM +0800, Chen-Yu Tsai wrote: > > On Fri, Apr 12, 2019 at 8:07 PM megous via linux-sunxi > > <linux-sunxi@googlegroups.com> wrote: > > > > > > From: Ondrej Jirman <megous@megous.com> > > > > > > I went through the datasheets for H6 and H5, and compared the differences. > > > RTCs are largely similar, but not entirely compatible. Incompatibilities > > > are in details not yet implemented by the rtc driver though. > > > > > > I also corrected the clock tree in H6 DTSI. > > > > Please also add DCXO clock input/output and XO clock input to the bindings > > and DT, and also fix up the clock tree. You can skip them in the driver for > > now, but please add a TODO. As long as you don't change the clock-output-name > > of osc24M, everything should work as before. > > > > We just want the DT to describe what is actually there. For the XO input, > > you could just directly reference the external crystal node. The gate for > > it is likely somewhere in the PRCM block, which we don't have docs for. > > So I was thinking about this for a while, and came up with this: > > ----------------- arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi ----------------- > index 64c39f663d22..ac99ddbebe5c 100644 > @@ -627,14 +635,15 @@ > > rtc: rtc@7000000 { > compatible = "allwinner,sun50i-h6-rtc"; > reg = <0x07000000 0x400>; > interrupts = <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>; > - clock-output-names = "osc32k", "osc32k-out", "iosc"; > - clocks = <&ext_osc32k>; > + clock-output-names = "osc32k", "osc32k-out", "iosc", "hosc"; > + clock-names = "losc", "dcxo"; > + clocks = <&ext_osc32k>, <&osc24M>; > #clock-cells = <1>; > }; > > r_ccu: clock@7010000 { > compatible = "allwinner,sun50i-h6-r-ccu"; > reg = <0x07010000 0x400>; > > I'm not completely sure how (or why?) to describe in DTSI which oscillator the > designer used (XO vs DCXO). This information is signalled by the pad voltage and > can be determined at runtime from DCXO_CTRL_REG's OSC_CLK_SRC_SEL (bit 3). It's > not possible to change at runtime. > > HOSC source selection is only material to the CPUS (ARISC) firmware when it > wants to turn off all PLLs and the main crystal oscillator so that it knows > which one to turn off. I don't see any other use for it. It's just > informational. I don't think (future) crust firmware has space to be reading > DTBs, so the detection will be using OSC_CLK_SRC_SEL anyway. > > Maybe whether XO or DCXO is used also matters if you want to do some fine > tunning of DCXO (control register has pletny of options), but that's probably > better done in u-boot. And there's still no need to read HOSC source from DT. > The driver can just check compatible, and if it is H6 and OSC_CLK_SRC_SEL is 1, > it can do it's DCXO tunning, or whatever. But neither OS nor bootloader will > be using this info to gate/disable the osciallator. > > If we really want this in DT, maybe we can model it by having just two input > clocks to RTC described in DTSI, and the DTSI for H6 would have this by default: > > clock-names = "losc", "dcxo"; > clocks = <&ext_osc32k>, <&osc24M>; > > And the board designer could change it from a board file, like this: > > &rtc { > clock-names = "losc", "xo"; > clocks = <&ext_osc32k>, <&osc24M>; > }; > > The driver could decide which oscillator is used by the presence of either > dcxo or xo input clock. > > But in any case, the driver can also get this info from DCXO_CTRL_REG's > OSC_CLK_SRC_SEL, so it doesn't need to read this from DT at all. So it's a bit > pointless. > > So I see two options: > > 1) skip adding dcxo/xo to input clocks of RTC completely > 2) the above > > What do you think? I tried option 2) and it feels pointless. It just creates this clock tree: osc24M hosc plls... from: osc24M plls... and doesn't achieve anything else, other than complicating things, for no reason because no driver will ever need or use this info from the DT. So my preference is for keeping it simple and going with option 1). regards, o. > regards, > o. > > > > > There's a small detail here, that's not described absolutely correctly in > > > DTSI, but the difference is not really that material. ext_osc32k is > > > originally modelled as a fixed clock that feeds into RTC module, but in > > > reality it's the RTC module that implements via its registers enabling and > > > disabling of this oscillator/clock. > > > > > > Though: > > > - there's no other possible user of ext_osc32k than RTC module > > > - there's no other possible external configuration for the crystal > > > circuit that would need to be handled in the dts per board > > > > > > So I guess, while the description is not perfect, this patch series still > > > improves the current situation. Or maybe I'm misunderstanding something, > > > and &ext_osc32k node just describes a fact that there's a crystal on > > > the board. Then, everything is perhaps fine. :) > > > > Correct. The external clock nodes are modeling the crystal, not the internal > > clock gate / distributor. > > > > Were the vendor to not include the crystal (for whatever reasons), the DT > > should be able to describe it via the absence of the clock input, and the > > driver should correctly use the internal (inaccurate) oscillator. I realize > > the clocks property is required, and the driver doesn't handle this case > > either, so we might have to fix that if it were to appear in the wild. > > > > > For now, the enable bit for this oscillator is toggled by the re-parenting > > > code automatically, as needed. > > > > That's fine. No need to increase the clock tree depth. > > > > ChenYu > > > > > This patchset is necessary for implementing the WiFi/Bluetooth support > > > on boards using H6 SoC. > > > > > > Please take a look. > > > > > > Thank you and regards, > > > Ondrej Jirman > > > > > > Ondrej Jirman (3): > > > dt-bindings: Add compatible for H6 RTC > > > rtc: sun6i: Add support for H6 RTC > > > arm64: dts: sun50i-h6: Add support for RTC and fix the clock tree > > > > > > .../devicetree/bindings/rtc/sun6i-rtc.txt | 1 + > > > arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 30 +++++++------- > > > drivers/rtc/rtc-sun6i.c | 40 ++++++++++++++++++- > > > 3 files changed, 55 insertions(+), 16 deletions(-) > > > > > > -- > > > 2.21.0 > > > > > > -- > > > You received this message because you are subscribed to the Google Groups "linux-sunxi" group. > > > To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com. > > > For more options, visit https://groups.google.com/d/optout. > > > > _______________________________________________ > > linux-arm-kernel mailing list > > linux-arm-kernel@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi, On 06/08/2019 20:30:45+0200, Ondřej Jirman wrote: > Maybe whether XO or DCXO is used also matters if you want to do some fine > tunning of DCXO (control register has pletny of options), but that's probably > better done in u-boot. And there's still no need to read HOSC source from DT. > The driver can just check compatible, and if it is H6 and OSC_CLK_SRC_SEL is 1, > it can do it's DCXO tunning, or whatever. But neither OS nor bootloader will > be using this info to gate/disable the osciallator. > It is actually useful to be able to tweak the crystal tuning at runtime to be able to reduce clock drift and compare with a reliable source (e.g. NTP). I'm curious, what kind of options does this RTC have?
On Wed, Aug 7, 2019 at 6:55 PM Alexandre Belloni <alexandre.belloni@bootlin.com> wrote: > > Hi, > > On 06/08/2019 20:30:45+0200, Ondřej Jirman wrote: > > Maybe whether XO or DCXO is used also matters if you want to do some fine > > tunning of DCXO (control register has pletny of options), but that's probably > > better done in u-boot. And there's still no need to read HOSC source from DT. > > The driver can just check compatible, and if it is H6 and OSC_CLK_SRC_SEL is 1, > > it can do it's DCXO tunning, or whatever. But neither OS nor bootloader will > > be using this info to gate/disable the osciallator. > > > > It is actually useful to be able to tweak the crystal tuning at > runtime to be able to reduce clock drift and compare with a reliable > source (e.g. NTP). > I'm curious, what kind of options does this RTC have? It has options to set the current, trim cap value, band gap voltage, and also change the mode to just accept an external clock signal, instead of driving a crystal. The settings for the former parameters are not explained though. See page 364 of http://linux-sunxi.org/File:Allwinner_H6_V200_User_Manual_V1.1.pdf ChenYu
On Wed, Aug 07, 2019 at 12:55:02PM +0200, Alexandre Belloni wrote: > Hi, > > On 06/08/2019 20:30:45+0200, Ondřej Jirman wrote: > > Maybe whether XO or DCXO is used also matters if you want to do some fine > > tunning of DCXO (control register has pletny of options), but that's probably > > better done in u-boot. And there's still no need to read HOSC source from DT. > > The driver can just check compatible, and if it is H6 and OSC_CLK_SRC_SEL is 1, > > it can do it's DCXO tunning, or whatever. But neither OS nor bootloader will > > be using this info to gate/disable the osciallator. > > > > It is actually useful to be able to tweak the crystal tuning at > runtime to be able to reduce clock drift and compare with a reliable > source (e.g. NTP). I don't think there's a Linux kernel API that you can use to achieve that, so that's a rather theoretical concern at the moment. Also there are multiple clocks, that can drive the RTC, and you usually don't drive it from 24MHz DCXO oscillator. The reason is that you'd have to deal with the fact that the clock for RTC then becomes 24000000/750 (750 is fixed divider), which is 32000. So if you want to get 32768Hz for RTC by tuning the DCXO, it would have to have 24 576 000 Hz. And even if you could achieve that (doubtful), it would throw off timings in the rest of the system (say UART, USB, CPU, display ctl) in a major way. I guess you can try tuning 24MHz oscillator so that it's closer to the real-world 24MHz via NTP reference for other reasons. But it would be complicated, and require precise interaction with other components, like using HW timers sourced from 24MHz HOSC clock, because you can't use CPU's timers, because of inaccuracies introduced during DVFS, for example. regards, o. > I'm curious, what kind of options does this RTC have? > > -- > Alexandre Belloni, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On 08/08/2019 14:12:37+0200, Ondřej Jirman wrote: > On Wed, Aug 07, 2019 at 12:55:02PM +0200, Alexandre Belloni wrote: > > Hi, > > > > On 06/08/2019 20:30:45+0200, Ondřej Jirman wrote: > > > Maybe whether XO or DCXO is used also matters if you want to do some fine > > > tunning of DCXO (control register has pletny of options), but that's probably > > > better done in u-boot. And there's still no need to read HOSC source from DT. > > > The driver can just check compatible, and if it is H6 and OSC_CLK_SRC_SEL is 1, > > > it can do it's DCXO tunning, or whatever. But neither OS nor bootloader will > > > be using this info to gate/disable the osciallator. > > > > > > > It is actually useful to be able to tweak the crystal tuning at > > runtime to be able to reduce clock drift and compare with a reliable > > source (e.g. NTP). > > I don't think there's a Linux kernel API that you can use to achieve that, so > that's a rather theoretical concern at the moment. > There is /sys/class/rtc/rtcX/offset which is even properly documented. The reason I asked is that some RTCs have both analog (changing the oscillator capacitance) and digital (changing the RTC counter) so I'm wondering whether this interface should be extended. > Also there are multiple clocks, that can drive the RTC, and you usually don't > drive it from 24MHz DCXO oscillator. The reason is that you'd have to deal with > the fact that the clock for RTC then becomes 24000000/750 (750 is fixed > divider), which is 32000. > > So if you want to get 32768Hz for RTC by tuning the DCXO, it would have to have > 24 576 000 Hz. And even if you could achieve that (doubtful), it would throw off > timings in the rest of the system (say UART, USB, CPU, display ctl) in a major way. > > I guess you can try tuning 24MHz oscillator so that it's closer to the > real-world 24MHz via NTP reference for other reasons. But it would be > complicated, and require precise interaction with other components, like using > HW timers sourced from 24MHz HOSC clock, because you can't use CPU's timers, > because of inaccuracies introduced during DVFS, for example. > > regards, > o. > > > I'm curious, what kind of options does this RTC have? > > > > -- > > Alexandre Belloni, Bootlin > > Embedded Linux and Kernel engineering > > https://bootlin.com > > > > _______________________________________________ > > linux-arm-kernel mailing list > > linux-arm-kernel@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Fri, Aug 09, 2019 at 01:39:30AM +0200, Alexandre Belloni wrote: > On 08/08/2019 14:12:37+0200, Ondřej Jirman wrote: > > On Wed, Aug 07, 2019 at 12:55:02PM +0200, Alexandre Belloni wrote: > > > Hi, > > > > > > On 06/08/2019 20:30:45+0200, Ondřej Jirman wrote: > > > > Maybe whether XO or DCXO is used also matters if you want to do some fine > > > > tunning of DCXO (control register has pletny of options), but that's probably > > > > better done in u-boot. And there's still no need to read HOSC source from DT. > > > > The driver can just check compatible, and if it is H6 and OSC_CLK_SRC_SEL is 1, > > > > it can do it's DCXO tunning, or whatever. But neither OS nor bootloader will > > > > be using this info to gate/disable the osciallator. > > > > > > > > > > It is actually useful to be able to tweak the crystal tuning at > > > runtime to be able to reduce clock drift and compare with a reliable > > > source (e.g. NTP). > > > > I don't think there's a Linux kernel API that you can use to achieve that, so > > that's a rather theoretical concern at the moment. > > > > There is /sys/class/rtc/rtcX/offset which is even properly documented. > > The reason I asked is that some RTCs have both analog (changing the > oscillator capacitance) and digital (changing the RTC counter) so I'm > wondering whether this interface should be extended. As I wrote below, that can't be achieved by tuning DCXO. > > Also there are multiple clocks, that can drive the RTC, and you usually don't > > drive it from 24MHz DCXO oscillator. The reason is that you'd have to deal with > > the fact that the clock for RTC then becomes 24000000/750 (750 is fixed > > divider), which is 32000. > > > > So if you want to get 32768Hz for RTC by tuning the DCXO, it would have to have > > 24 576 000 Hz. And even if you could achieve that (doubtful), it would throw off > > timings in the rest of the system (say UART, USB, CPU, display ctl) in a major way. > > > > I guess you can try tuning 24MHz oscillator so that it's closer to the > > real-world 24MHz via NTP reference for other reasons. But it would be > > complicated, and require precise interaction with other components, like using > > HW timers sourced from 24MHz HOSC clock, because you can't use CPU's timers, > > because of inaccuracies introduced during DVFS, for example. > > > > regards, > > o. > > > > > I'm curious, what kind of options does this RTC have? > > > > > > -- > > > Alexandre Belloni, Bootlin > > > Embedded Linux and Kernel engineering > > > https://bootlin.com > > > > > > _______________________________________________ > > > linux-arm-kernel mailing list > > > linux-arm-kernel@lists.infradead.org > > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > > -- > Alexandre Belloni, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Ondrej Jirman <megous@megous.com> I went through the datasheets for H6 and H5, and compared the differences. RTCs are largely similar, but not entirely compatible. Incompatibilities are in details not yet implemented by the rtc driver though. I also corrected the clock tree in H6 DTSI. There's a small detail here, that's not described absolutely correctly in DTSI, but the difference is not really that material. ext_osc32k is originally modelled as a fixed clock that feeds into RTC module, but in reality it's the RTC module that implements via its registers enabling and disabling of this oscillator/clock. Though: - there's no other possible user of ext_osc32k than RTC module - there's no other possible external configuration for the crystal circuit that would need to be handled in the dts per board So I guess, while the description is not perfect, this patch series still improves the current situation. Or maybe I'm misunderstanding something, and &ext_osc32k node just describes a fact that there's a crystal on the board. Then, everything is perhaps fine. :) For now, the enable bit for this oscillator is toggled by the re-parenting code automatically, as needed. This patchset is necessary for implementing the WiFi/Bluetooth support on boards using H6 SoC. Please take a look. Thank you and regards, Ondrej Jirman Ondrej Jirman (3): dt-bindings: Add compatible for H6 RTC rtc: sun6i: Add support for H6 RTC arm64: dts: sun50i-h6: Add support for RTC and fix the clock tree .../devicetree/bindings/rtc/sun6i-rtc.txt | 1 + arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 30 +++++++------- drivers/rtc/rtc-sun6i.c | 40 ++++++++++++++++++- 3 files changed, 55 insertions(+), 16 deletions(-)