Message ID | 1448065205-15762-4-git-send-email-joshua.henderson@microchip.com |
---|---|
State | Changes Requested, archived |
Headers | show |
On Friday 20 November 2015 17:17:15 Joshua Henderson wrote: > +/* PIC32 specific clks */ > +pic32_clktree { > + #address-cells = <1>; > + #size-cells = <1>; > + reg = <0x1f801200 0x200>; > + compatible = "microchip,pic32-clk"; > + interrupts = <12>; > + ranges; > + > + /* secondary oscillator; external input on SOSCI pin */ > + SOSC:sosc_clk { > + #clock-cells = <0>; > + compatible = "microchip,pic32-sosc"; > + clock-frequency = <32768>; > + reg = <0x1f801200 0x10 /* enable reg */ > + 0x1f801390 0x10>; /* status reg */ > + microchip,bit-mask = <0x02>; /* enable mask */ > + microchip,status-bit-mask = <0x10>; /* status-mask*/ > + }; > If you want to use the reg property in this way for each cell, at least use a 'ranges' that only translates the actual registers like this ranges = <0 0x1f801200 0x200> sosc_clk { ... reg = <0x000 0x10>, <0x190 0x10>; ... }; Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Nov 20, 2015 at 05:17:15PM -0700, Joshua Henderson wrote: > From: Purna Chandra Mandal <purna.mandal@microchip.com> > > Document the devicetree bindings for the clock driver found on Microchip > PIC32 class devices. > > Signed-off-by: Purna Chandra Mandal <purna.mandal@microchip.com> > Signed-off-by: Joshua Henderson <joshua.henderson@microchip.com> > --- > .../devicetree/bindings/clock/microchip,pic32.txt | 263 ++++++++++++++++++++ > 1 file changed, 263 insertions(+) > create mode 100644 Documentation/devicetree/bindings/clock/microchip,pic32.txt > > diff --git a/Documentation/devicetree/bindings/clock/microchip,pic32.txt b/Documentation/devicetree/bindings/clock/microchip,pic32.txt > new file mode 100644 > index 0000000..4cef72d > --- /dev/null > +++ b/Documentation/devicetree/bindings/clock/microchip,pic32.txt > @@ -0,0 +1,263 @@ > +Binding for a Clock hardware block found on > +certain Microchip PIC32 MCU devices. > + > +Microchip SoC clocks-node consists of few oscillators, PLL, multiplexer > +and few divider nodes. [...] > +Required properties: > +- compatible : should have "microchip,pic32-clk". > +- reg : A Base address and length of the register set. > +- interrupts : source of interrupt. > + > +Optional properties (for subnodes): > +- #clock-cells: From common clock binding, should be 0. > + > +- microchip,clock-indices: in multiplexer node clock sources always aren't linear > + and contiguous. This property helps define clock-sources with respect to > + the mux clock node. > + > +- microchip,ignore-unused : ignore gate request even if the gated clock is unused. There is some discussion about this upstream with "critical-clocks" binding. Can you use and wait for that? > +- microchip,status-bit-mask: bitmask for status check. This will be used to confirm > + particular operation by clock sub-node is completed. It is dependent sub-node. > +- microchip,bit-mask: enable mask, similar to microchip,status-bit-mask. We've generally decided not to describe clocks at this level of detail in DT. It's fine though for simple clock trees. This one seems to be borderline IMO. > +- microchip,slew-step: enable frequency slewing(stepping) during rate change; > + applicable only to sys-clock subnode. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Arnd, On 11/21/2015 1:49 PM, Arnd Bergmann wrote: > On Friday 20 November 2015 17:17:15 Joshua Henderson wrote: >> +/* PIC32 specific clks */ >> +pic32_clktree { >> + #address-cells = <1>; >> + #size-cells = <1>; >> + reg = <0x1f801200 0x200>; >> + compatible = "microchip,pic32-clk"; >> + interrupts = <12>; >> + ranges; >> + >> + /* secondary oscillator; external input on SOSCI pin */ >> + SOSC:sosc_clk { >> + #clock-cells = <0>; >> + compatible = "microchip,pic32-sosc"; >> + clock-frequency = <32768>; >> + reg = <0x1f801200 0x10 /* enable reg */ >> + 0x1f801390 0x10>; /* status reg */ >> + microchip,bit-mask = <0x02>; /* enable mask */ >> + microchip,status-bit-mask = <0x10>; /* status-mask*/ >> + }; >> > > If you want to use the reg property in this way for each cell, > at least use a 'ranges' that only translates the actual registers > like this > > ranges = <0 0x1f801200 0x200> > > sosc_clk { > ... > reg = <0x000 0x10>, <0x190 0x10>; > ... > }; > > Arnd > This does indeed seem to be the correct way to use ranges in this case. Consider it done. Thanks for the feedback, Josh -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Rob, On 11/22/2015 2:31 PM, Rob Herring wrote: > On Fri, Nov 20, 2015 at 05:17:15PM -0700, Joshua Henderson wrote: >> From: Purna Chandra Mandal <purna.mandal@microchip.com> >> >> Document the devicetree bindings for the clock driver found on Microchip >> PIC32 class devices. >> >> Signed-off-by: Purna Chandra Mandal <purna.mandal@microchip.com> >> Signed-off-by: Joshua Henderson <joshua.henderson@microchip.com> >> --- >> .../devicetree/bindings/clock/microchip,pic32.txt | 263 ++++++++++++++++++++ >> 1 file changed, 263 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/clock/microchip,pic32.txt >> >> diff --git a/Documentation/devicetree/bindings/clock/microchip,pic32.txt b/Documentation/devicetree/bindings/clock/microchip,pic32.txt >> new file mode 100644 >> index 0000000..4cef72d >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/clock/microchip,pic32.txt >> @@ -0,0 +1,263 @@ >> +Binding for a Clock hardware block found on >> +certain Microchip PIC32 MCU devices. >> + >> +Microchip SoC clocks-node consists of few oscillators, PLL, multiplexer >> +and few divider nodes. > > [...] > >> +Required properties: >> +- compatible : should have "microchip,pic32-clk". >> +- reg : A Base address and length of the register set. >> +- interrupts : source of interrupt. >> + >> +Optional properties (for subnodes): >> +- #clock-cells: From common clock binding, should be 0. >> + >> +- microchip,clock-indices: in multiplexer node clock sources always aren't linear >> + and contiguous. This property helps define clock-sources with respect to >> + the mux clock node. >> + >> +- microchip,ignore-unused : ignore gate request even if the gated clock is unused. > > There is some discussion about this upstream with "critical-clocks" > binding. Can you use and wait for that? > The way this is going, we might not have to wait. :) Is there a patch available yet to try it out? >> +- microchip,status-bit-mask: bitmask for status check. This will be used to confirm >> + particular operation by clock sub-node is completed. It is dependent sub-node. >> +- microchip,bit-mask: enable mask, similar to microchip,status-bit-mask. > > We've generally decided not to describe clocks at this level of detail > in DT. It's fine though for simple clock trees. This one seems to be > borderline IMO. > The binding example is the entire clock tree. These masks are right from the datasheet. For reference, do you have an example of a better alternative? >> +- microchip,slew-step: enable frequency slewing(stepping) during rate change; >> + applicable only to sys-clock subnode. > Thanks, Josh -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Nov 25, 2015 at 10:36:55PM -0700, Joshua Henderson wrote: > Hi Rob, > > On 11/22/2015 2:31 PM, Rob Herring wrote: > > On Fri, Nov 20, 2015 at 05:17:15PM -0700, Joshua Henderson wrote: > >> From: Purna Chandra Mandal <purna.mandal@microchip.com> > >> > >> Document the devicetree bindings for the clock driver found on Microchip > >> PIC32 class devices. > >> > >> Signed-off-by: Purna Chandra Mandal <purna.mandal@microchip.com> > >> Signed-off-by: Joshua Henderson <joshua.henderson@microchip.com> > >> --- > >> .../devicetree/bindings/clock/microchip,pic32.txt | 263 ++++++++++++++++++++ > >> 1 file changed, 263 insertions(+) > >> create mode 100644 Documentation/devicetree/bindings/clock/microchip,pic32.txt > >> > >> diff --git a/Documentation/devicetree/bindings/clock/microchip,pic32.txt b/Documentation/devicetree/bindings/clock/microchip,pic32.txt > >> new file mode 100644 > >> index 0000000..4cef72d > >> --- /dev/null > >> +++ b/Documentation/devicetree/bindings/clock/microchip,pic32.txt > >> @@ -0,0 +1,263 @@ > >> +Binding for a Clock hardware block found on > >> +certain Microchip PIC32 MCU devices. > >> + > >> +Microchip SoC clocks-node consists of few oscillators, PLL, multiplexer > >> +and few divider nodes. > > > > [...] > > > >> +Required properties: > >> +- compatible : should have "microchip,pic32-clk". BTW, this should list out the actual compatible strings. > > There is some discussion about this upstream with "critical-clocks" > > binding. Can you use and wait for that? > > > > The way this is going, we might not have to wait. :) Is there a patch available yet to try it out? Yes, googling "Lee Jones critical-clocks" should find it. > >> +- microchip,status-bit-mask: bitmask for status check. This will be used to confirm > >> + particular operation by clock sub-node is completed. It is dependent sub-node. > >> +- microchip,bit-mask: enable mask, similar to microchip,status-bit-mask. > > > > We've generally decided not to describe clocks at this level of detail > > in DT. It's fine though for simple clock trees. This one seems to be > > borderline IMO. > > > > The binding example is the entire clock tree. These masks are right from the datasheet. For reference, do you have an example of a better alternative? Okay, like I said, borderline. If this is complete, then it is fine. Adding more clocks or a newer version of the SoC with more clocks would change that opinion. Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Rob, On 11/30/2015 02:43 PM, Rob Herring wrote: > On Wed, Nov 25, 2015 at 10:36:55PM -0700, Joshua Henderson wrote: >> Hi Rob, >> >> On 11/22/2015 2:31 PM, Rob Herring wrote: >>> On Fri, Nov 20, 2015 at 05:17:15PM -0700, Joshua Henderson wrote: >>>> From: Purna Chandra Mandal <purna.mandal@microchip.com> >>>> >>>> Document the devicetree bindings for the clock driver found on Microchip >>>> PIC32 class devices. >>>> >>>> Signed-off-by: Purna Chandra Mandal <purna.mandal@microchip.com> >>>> Signed-off-by: Joshua Henderson <joshua.henderson@microchip.com> >>>> --- >>>> .../devicetree/bindings/clock/microchip,pic32.txt | 263 ++++++++++++++++++++ >>>> 1 file changed, 263 insertions(+) >>>> create mode 100644 Documentation/devicetree/bindings/clock/microchip,pic32.txt >>>> >>>> diff --git a/Documentation/devicetree/bindings/clock/microchip,pic32.txt b/Documentation/devicetree/bindings/clock/microchip,pic32.txt >>>> new file mode 100644 >>>> index 0000000..4cef72d >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/clock/microchip,pic32.txt >>>> @@ -0,0 +1,263 @@ >>>> +Binding for a Clock hardware block found on >>>> +certain Microchip PIC32 MCU devices. >>>> + >>>> +Microchip SoC clocks-node consists of few oscillators, PLL, multiplexer >>>> +and few divider nodes. >>> >>> [...] >>> >>>> +Required properties: >>>> +- compatible : should have "microchip,pic32-clk". > > BTW, this should list out the actual compatible strings. Ack. These are also being changed to microchip,pic32mzda-* due to other feedback. > >>> There is some discussion about this upstream with "critical-clocks" >>> binding. Can you use and wait for that? >>> >> >> The way this is going, we might not have to wait. :) Is there a patch available yet to try it out? > > Yes, googling "Lee Jones critical-clocks" should find it. The change for this on our side is in the queue should the stars align on timing. [...] > > Rob > Thanks, Josh -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/clock/microchip,pic32.txt b/Documentation/devicetree/bindings/clock/microchip,pic32.txt new file mode 100644 index 0000000..4cef72d --- /dev/null +++ b/Documentation/devicetree/bindings/clock/microchip,pic32.txt @@ -0,0 +1,263 @@ +Binding for a Clock hardware block found on +certain Microchip PIC32 MCU devices. + +Microchip SoC clocks-node consists of few oscillators, PLL, multiplexer +and few divider nodes. + +We will find only the base address of the clock tree, this base +address is common for some of the subnodes, not all. If no address is +specified for any of subnode base address of the clock tree will be +treated as its base. Each of subnodes follow the same common clock +binding with some additional optional properties. + + clocks_node { + reg = <>; + + spll_node { + ... + }; + + frcdiv_node { + ... + }; + + sysclk_mux_node { + ... + }; + + pbdiv_node { + ... + }; + + refoclk_node { + ... + }; + ... + }; + +This binding uses the common clock binding[1]. + +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt + +Required properties: +- compatible : should have "microchip,pic32-clk". +- reg : A Base address and length of the register set. +- interrupts : source of interrupt. + +Optional properties (for subnodes): +- #clock-cells: From common clock binding, should be 0. + +- microchip,clock-indices: in multiplexer node clock sources always aren't linear + and contiguous. This property helps define clock-sources with respect to + the mux clock node. + +- microchip,ignore-unused : ignore gate request even if the gated clock is unused. +- microchip,status-bit-mask: bitmask for status check. This will be used to confirm + particular operation by clock sub-node is completed. It is dependent sub-node. +- microchip,bit-mask: enable mask, similar to microchip,status-bit-mask. +- microchip,slew-step: enable frequency slewing(stepping) during rate change; + applicable only to sys-clock subnode. + +Example: + +/* PIC32 specific clks */ +pic32_clktree { + #address-cells = <1>; + #size-cells = <1>; + reg = <0x1f801200 0x200>; + compatible = "microchip,pic32-clk"; + interrupts = <12>; + ranges; + + /* secondary oscillator; external input on SOSCI pin */ + SOSC:sosc_clk { + #clock-cells = <0>; + compatible = "microchip,pic32-sosc"; + clock-frequency = <32768>; + reg = <0x1f801200 0x10 /* enable reg */ + 0x1f801390 0x10>; /* status reg */ + microchip,bit-mask = <0x02>; /* enable mask */ + microchip,status-bit-mask = <0x10>; /* status-mask*/ + }; + + FRCDIV:frcdiv_clk { + #clock-cells = <0>; + compatible = "microchip,pic32-frcdivclk"; + clocks = <&FRC>; + clock-output-names = "frcdiv_clk"; + }; + + /* System PLL clock */ + SYSPLL:spll_clk { + #clock-cells = <0>; + compatible = "microchip,pic32-syspll"; + reg = <0x1f801220 0x10 /* SPLL register */ + 0x1f801390 0x10>; /* CLKSTAT register */ + clocks = <&POSC>, <&FRC>; + clock-output-names = "sys_pll"; + microchip,status-bit-mask = <0x80>; /* SPLLRDY */ + }; + + /* system clock; mux with postdiv & slew */ + SYSCLK:sys_clk { + #clock-cells = <0>; + compatible = "microchip,pic32-sysclk-v2"; + reg = <0x1f8013c0 0x04>; /* SLEWCON */ + clocks = <&FRCDIV>, <&SYSPLL>, <&POSC>, <&SOSC>, + <&LPRC>, <&FRCDIV>; + microchip,clock-indices = <0>, <1>, <2>, <4>, <5>, <7>; + clock-output-names = "sys_clk"; + }; + + /* DDR Ctrl & DDR PHY PLL */ + MPLL: CLK_MPLL { + #clock-cells = <0>; + compatible = "microchip,pic32-mpll"; + reg = <0x1f800100 0x04>; /* CFGMPLL */ + clocks = <&POSC>; + clock-output-names = "pic32-mpll"; + status = "disabled"; + }; + + /* Peripheral bus1 clock */ + PBCLK1:pb1_clk { + reg = <0x1f801340 0x10>; + #clock-cells = <0>; + compatible = "microchip,pic32-pbclk"; + clocks = <&SYSCLK>; + clock-output-names = "pb1_clk"; + /* used by system modules, not gateable */ + microchip,ignore-unused; + }; + + /* Peripheral bus2 clock */ + PBCLK2:pb2_clk { + reg = <0x1f801350 0x10>; + #clock-cells = <0>; + compatible = "microchip,pic32-pbclk"; + clocks = <&SYSCLK>; + clock-output-names = "pb2_clk"; + /* avoid gating even if unused */ + microchip,ignore-unused; + }; + + /* Peripheral bus3 clock */ + PBCLK3:pb3_clk { + reg = <0x1f801360 0x10>; + #clock-cells = <0>; + compatible = "microchip,pic32-pbclk"; + clocks = <&SYSCLK>; + clock-output-names = "pb3_clk"; + }; + + /* Peripheral bus4 clock(I/O ports, GPIO) */ + PBCLK4:pb4_clk { + reg = <0x1f801370 0x10>; + #clock-cells = <0>; + compatible = "microchip,pic32-pbclk"; + clocks = <&SYSCLK>; + clock-output-names = "pb4_clk"; + }; + + /* Peripheral bus clock */ + PBCLK5:pb5_clk { + reg = <0x1f801380 0x10>; + #clock-cells = <0>; + compatible = "microchip,pic32-pbclk"; + clocks = <&SYSCLK>; + clock-output-names = "pb5_clk"; + }; + + /* Peripheral Bus6 clock; */ + PBCLK6:pb6_clk { + reg = <0x1f801390 0x10>; + compatible = "microchip,pic32-pbclk"; + clocks = <&SYSCLK>; + #clock-cells = <0>; + }; + + /* Peripheral bus7 clock */ + PBCLK7:pb7_clk { + reg = <0x1f8013A0 0x10>; + #clock-cells = <0>; + compatible = "microchip,pic32-pbclk"; + /* CPU is driven by this clock; so named */ + clock-output-names = "cpu_clk"; + clocks = <&SYSCLK>; + }; + + /* Reference Oscillator clock for SPI/I2S */ + REFCLKO1:refo1_clk { + reg = <0x1f801280 0x20>; + #clock-cells = <0>; + compatible = "microchip,pic32-refoclk"; + clocks = <&SYSCLK>, <&PBCLK1>, <&POSC>, <&FRC>, <&LPRC>, + <&SOSC>, <&SYSPLL>, <&REFIx>, <&BFRC>; + microchip,clock-indices = <0>, <1>, <2>, <3>, <4>, <5>, + <7>, <8>, <9>; + clock-output-names = "refo1_clk"; + clock-frequency = <10000000>; /* 10Mhz for I2S MCLK */ + }; + + /* Reference Oscillator clock for SQI */ + REFCLKO2:refo2_clk { + reg = <0x1f8012A0 0x20>; + #clock-cells = <0>; + compatible = "microchip,pic32-refoclk"; + clocks = <&SYSCLK>, <&PBCLK1>, <&POSC>, <&FRC>, <&LPRC>, + <&SOSC>, <&SYSPLL>, <&REFIx>, <&BFRC>; + microchip,clock-indices = <0>, <1>, <2>, <3>, <4>, <5>, + <7>, <8>, <9>; + clock-output-names = "refo2_clk"; + clock-frequency = <50000000>; /* 50MHz for SQI */ + }; + + /* Reference Oscillator clock, ADC */ + REFCLKO3:refo3_clk { + reg = <0x1f8012C0 0x20>; + compatible = "microchip,pic32-refoclk"; + clocks = <&SYSCLK>, <&PBCLK1>, <&POSC>, <&FRC>, <&LPRC>, + <&SOSC>, <&SYSPLL>, <&REFIx>, <&BFRC>; + microchip,clock-indices = <0>, <1>, <2>, <3>, <4>, <5>, + <7>, <8>, <9>; + #clock-cells = <0>; + clock-output-names = "refo3_clk"; + clock-frequency = <10000000>; /* 1Mhz */ + }; + + /* Reference Oscillator clock */ + REFCLKO4:refo4_clk { + reg = <0x1f8012E0 0x20>; + compatible = "microchip,pic32-refoclk"; + clocks = <&SYSCLK>, <&PBCLK1>, <&POSC>, <&FRC>, <&LPRC>, + <&SOSC>, <&SYSPLL>, <&REFIx>, <&BFRC>; + microchip,clock-indices = <0>,<1>,<2>,<3>,<4>,<5>,<7>, + <8>,<9>; + #clock-cells = <0>; + clock-output-names = "refo4_clk"; + clock-frequency = <25000000>; /* 25Mhz */ + }; + + /* Reference Oscillator clock, LCD */ + REFCLKO5:refo5_clk { + reg = <0x1f801300 0x20>; + compatible = "microchip,pic32-refoclk"; + clocks = <&SYSCLK>,<&PBCLK1>,<&POSC>,<&FRC>,<&LPRC>, + <&SOSC>,<&SYSPLL>,<&REFIx>,<&BFRC>; + microchip,clock-indices = <0>, <1>, <2>, <3>, <4>, <5>, + <7>, <8>,<9>; + #clock-cells = <0>; + clock-output-names = "refo5_clk"; + clock-frequency = <40000000>; /* 40Mhz */ + }; +}; + +The clock consumer should specify the desired clock by having the clocks in its +"clock" phandle cell. For example for UART: + +uart2: serial@<> { + compatible = "microchip,pic32-uart"; + reg = <>; + interrupts = <>; + clocks = <&PBCLK2>; +}