diff mbox

[v2,2/4] doc: dt: add documentation for lpc1850-cgu clk driver

Message ID 1430170693-28303-3-git-send-email-manabian@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Joachim Eastwood April 27, 2015, 9:38 p.m. UTC
Add DT binding documentation for lpc1850-cgu driver.

Signed-off-by: Joachim Eastwood <manabian@gmail.com>
---
 .../devicetree/bindings/clock/lpc1850-cgu.txt      | 138 +++++++++++++++++++++
 1 file changed, 138 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/lpc1850-cgu.txt

Comments

Mike Turquette May 12, 2015, 10:12 p.m. UTC | #1
Quoting Joachim Eastwood (2015-04-27 14:38:11)
> Add DT binding documentation for lpc1850-cgu driver.
> 
> Signed-off-by: Joachim Eastwood <manabian@gmail.com>
> ---
>  .../devicetree/bindings/clock/lpc1850-cgu.txt      | 138 +++++++++++++++++++++
>  1 file changed, 138 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/lpc1850-cgu.txt
> 
> diff --git a/Documentation/devicetree/bindings/clock/lpc1850-cgu.txt b/Documentation/devicetree/bindings/clock/lpc1850-cgu.txt
> new file mode 100644
> index 000000000000..0b278ca6aee7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/lpc1850-cgu.txt
> @@ -0,0 +1,138 @@
> +* NXP LPC1850 Clock Generation Unit (CGU)
> +
> +The CGU generates multiple independent clocks for the core and the
> +peripheral blocks of the LPC18xx. Each independent clock is called
> +a base clock and itself is one of the inputs to the two Clock
> +Control Units (CCUs) which control the branch clocks to the
> +individual peripherals.
> +
> +The CGU selects the inputs to the clock generators from multiple
> +clock sources, controls the clock generation, and routes the outputs
> +of the clock generators through the clock source bus to the output
> +stages. Each output stage provides an independent clock source and
> +corresponds to one of the base clocks for the LPC18xx.
> +
> + - Above text taken from NXP LPC1850 User Manual.
> +
> +
> +This binding uses the common clock binding:
> +    Documentation/devicetree/bindings/clock/clock-bindings.txt
> +
> +Required properties:
> +- compatible:
> +       Should be "nxp,lpc1850-cgu"
> +- reg:
> +       Shall define the base and range of the address space
> +       containing clock control registers
> +- #clock-cells:
> +       Shall have value <1>.  The permitted clock-specifier values
> +       are the base clock numbers defined below.
> +- clocks:
> +       Shall contain a list of phandles for the external input
> +       sources to the CGU. The list shall be in the following
> +       order: xtal, 32khz, enet_rx_clk, enet_tx_clk, gp_clkin.
> +- clock-indices:
> +       Shall be an ordered list of numbers defining the base clock
> +       number provided by the CGU.
> +- clock-output-names:
> +       Shall be an ordered list of strings defining the names of
> +       the clocks provided by the CGU.
> +
> +Which base clocks that are available on the CGU depends on the
> +specific LPC part. Base cloks are numbered from 0 to 27.

s/cloks/clocks/

> +
> +Number:                Name:                   Description:
> + 0             BASE_SAFE_CLK           Base safe clock (always on) for WWDT
> + 1             BASE_USB0_CLK           Base clock for USB0
> + 2             BASE_PERIPH_CLK         Base clock for Cortex-M0SUB subsystem,
> +                                       SPI, and SGPIO
> + 3             BASE_USB1_CLK           Base clock for USB1
> + 4             BASE_CPU_CLK            System base clock for ARM Cortex-M core
> +                                       and APB peripheral blocks #0 and #2
> + 5             BASE_SPIFI_CLK          Base clock for SPIFI
> + 6             BASE_SPI_CLK            Base clock for SPI
> + 7             BASE_PHY_RX_CLK         Base clock for Ethernet PHY Receive clock
> + 8             BASE_PHY_TX_CLK         Base clock for Ethernet PHY Transmit clock
> + 9             BASE_APB1_CLK           Base clock for APB peripheral block # 1
> +10             BASE_APB3_CLK           Base clock for APB peripheral block # 3
> +11             BASE_LCD_CLK            Base clock for LCD
> +12             BASE_ADCHS_CLK          Base clock for ADCHS
> +13             BASE_SDIO_CLK           Base clock for SD/MMC
> +14             BASE_SSP0_CLK           Base clock for SSP0
> +15             BASE_SSP1_CLK           Base clock for SSP1
> +16             BASE_UART0_CLK          Base clock for UART0
> +17             BASE_UART1_CLK          Base clock for UART1
> +18             BASE_UART2_CLK          Base clock for UART2
> +19             BASE_UART3_CLK          Base clock for UART3
> +20             BASE_OUT_CLK            Base clock for CLKOUT pin
> +24-21          -                       Reserved
> +25             BASE_AUDIO_CLK          Base clock for audio system (I2S)
> +26             BASE_CGU_OUT0_CLK       Base clock for CGU_OUT0 clock output
> +27             BASE_CGU_OUT1_CLK       Base clock for CGU_OUT1 clock output
> +
> +BASE_PERIPH_CLK and BASE_SPI_CLK is only available on LPC43xx.
> +BASE_ADCHS_CLK is only available on LPC4370.
> +
> +
> +Example board file:
> +
> +/ {
> +       clocks {
> +               xtal: xtal {
> +                       compatible = "fixed-clock";
> +                       #clock-cells = <0>;
> +                       clock-frequency = <12000000>;
> +               };
> +
> +               xtal32: xtal32 {
> +                       compatible = "fixed-clock";
> +                       #clock-cells = <0>;
> +                       clock-frequency = <32768>;
> +               };
> +
> +               enet_rx_clk: enet_rx_clk {
> +                       compatible = "fixed-clock";
> +                       #clock-cells = <0>;
> +                       clock-frequency = <0>;
> +                       clock-output-names = "enet_rx_clk";
> +               };
> +
> +               enet_tx_clk: enet_tx_clk {
> +                       compatible = "fixed-clock";
> +                       #clock-cells = <0>;
> +                       clock-frequency = <0>;
> +                       clock-output-names = "enet_tx_clk";
> +               };
> +
> +               gp_clkin: gp_clkin {
> +                       compatible = "fixed-clock";
> +                       #clock-cells = <0>;
> +                       clock-frequency = <0>;
> +                       clock-output-names = "gp_clkin";
> +               };
> +       };
> +
> +       soc {
> +               cgu: cgu@40050000 {
> +                       compatible = "nxp,lpc1850-cgu";
> +                       reg = <0x40050000 0x1000>;
> +                       #clock-cells = <1>;
> +                       clocks = <&xtal>, <&creg_clk 1>, <&enet_rx_clk>, <&enet_tx_clk>, <&gp_clkin>;
> +                       clock-indices =  <0>,  <1>,  <2>,  <3>,  <4>,  <5>,  <6>,  <7>,
> +                                        <8>,  <9>, <10>, <11>, <12>, <13>, <14>, <15>,
> +                                       <16>, <17>, <18>, <19>, <20>, <25>, <26>, <27>;
> +                       clock-output-names = "base_safe_clk",    "base_usb0_clk",
> +                                            "base_periph_clk",  "base_usb1_clk",
> +                                            "base_cpu_clk",     "base_spifi_clk",
> +                                            "base_spi_clk",     "base_phy_rx_clk",
> +                                            "base_phy_tx_clk",  "base_apb1_clk",
> +                                            "base_apb3_clk",    "base_lcd_clk",
> +                                            "base_adchs_clk",   "base_sdio_clk",
> +                                            "base_ssp0_clk",    "base_ssp1_clk",
> +                                            "base_uart0_clk",   "base_uart1_clk",
> +                                            "base_uart2_clk",   "base_uart3_clk",
> +                                            "base_out_clk",     "base_audio_clk",
> +                                            "base_cgu_out0_clk","base_cgu_out1_clk";

Why do you need to use clock-indices?

Why do you need to use clock-output-names? If all of your clock
consumers have nodes in DT then you can skip this and name them on the
consuming side. See a further explanation here:
http://lkml.kernel.org/r/<20150416192014.19585.9663@quantum>

Also note that providing a clock consumer node in the examples section
of the binding is helpful when reviewing.

Regards,
Mike

> +               };
> +       };
> +};
> -- 
> 1.8.0
>
Joachim Eastwood May 13, 2015, 7:57 a.m. UTC | #2
On 13 May 2015 at 00:12, Michael Turquette <mturquette@linaro.org> wrote:
> Quoting Joachim Eastwood (2015-04-27 14:38:11)
>> Add DT binding documentation for lpc1850-cgu driver.
>>
>> Signed-off-by: Joachim Eastwood <manabian@gmail.com>
>> ---
>>  .../devicetree/bindings/clock/lpc1850-cgu.txt      | 138 +++++++++++++++++++++
>>  1 file changed, 138 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/clock/lpc1850-cgu.txt
>>
>> diff --git a/Documentation/devicetree/bindings/clock/lpc1850-cgu.txt b/Documentation/devicetree/bindings/clock/lpc1850-cgu.txt
>> new file mode 100644
>> index 000000000000..0b278ca6aee7
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/lpc1850-cgu.txt
>> @@ -0,0 +1,138 @@
>> +* NXP LPC1850 Clock Generation Unit (CGU)
>> +
>> +The CGU generates multiple independent clocks for the core and the
>> +peripheral blocks of the LPC18xx. Each independent clock is called
>> +a base clock and itself is one of the inputs to the two Clock
>> +Control Units (CCUs) which control the branch clocks to the
>> +individual peripherals.
>> +
>> +The CGU selects the inputs to the clock generators from multiple
>> +clock sources, controls the clock generation, and routes the outputs
>> +of the clock generators through the clock source bus to the output
>> +stages. Each output stage provides an independent clock source and
>> +corresponds to one of the base clocks for the LPC18xx.
>> +
>> + - Above text taken from NXP LPC1850 User Manual.
>> +
>> +
>> +This binding uses the common clock binding:
>> +    Documentation/devicetree/bindings/clock/clock-bindings.txt
>> +
>> +Required properties:
>> +- compatible:
>> +       Should be "nxp,lpc1850-cgu"
>> +- reg:
>> +       Shall define the base and range of the address space
>> +       containing clock control registers
>> +- #clock-cells:
>> +       Shall have value <1>.  The permitted clock-specifier values
>> +       are the base clock numbers defined below.
>> +- clocks:
>> +       Shall contain a list of phandles for the external input
>> +       sources to the CGU. The list shall be in the following
>> +       order: xtal, 32khz, enet_rx_clk, enet_tx_clk, gp_clkin.
>> +- clock-indices:
>> +       Shall be an ordered list of numbers defining the base clock
>> +       number provided by the CGU.
>> +- clock-output-names:
>> +       Shall be an ordered list of strings defining the names of
>> +       the clocks provided by the CGU.
>> +
>> +Which base clocks that are available on the CGU depends on the
>> +specific LPC part. Base cloks are numbered from 0 to 27.
>
> s/cloks/clocks/
>
>> +
>> +Number:                Name:                   Description:
>> + 0             BASE_SAFE_CLK           Base safe clock (always on) for WWDT
>> + 1             BASE_USB0_CLK           Base clock for USB0
>> + 2             BASE_PERIPH_CLK         Base clock for Cortex-M0SUB subsystem,
>> +                                       SPI, and SGPIO
>> + 3             BASE_USB1_CLK           Base clock for USB1
>> + 4             BASE_CPU_CLK            System base clock for ARM Cortex-M core
>> +                                       and APB peripheral blocks #0 and #2
>> + 5             BASE_SPIFI_CLK          Base clock for SPIFI
>> + 6             BASE_SPI_CLK            Base clock for SPI
>> + 7             BASE_PHY_RX_CLK         Base clock for Ethernet PHY Receive clock
>> + 8             BASE_PHY_TX_CLK         Base clock for Ethernet PHY Transmit clock
>> + 9             BASE_APB1_CLK           Base clock for APB peripheral block # 1
>> +10             BASE_APB3_CLK           Base clock for APB peripheral block # 3
>> +11             BASE_LCD_CLK            Base clock for LCD
>> +12             BASE_ADCHS_CLK          Base clock for ADCHS
>> +13             BASE_SDIO_CLK           Base clock for SD/MMC
>> +14             BASE_SSP0_CLK           Base clock for SSP0
>> +15             BASE_SSP1_CLK           Base clock for SSP1
>> +16             BASE_UART0_CLK          Base clock for UART0
>> +17             BASE_UART1_CLK          Base clock for UART1
>> +18             BASE_UART2_CLK          Base clock for UART2
>> +19             BASE_UART3_CLK          Base clock for UART3
>> +20             BASE_OUT_CLK            Base clock for CLKOUT pin
>> +24-21          -                       Reserved
>> +25             BASE_AUDIO_CLK          Base clock for audio system (I2S)
>> +26             BASE_CGU_OUT0_CLK       Base clock for CGU_OUT0 clock output
>> +27             BASE_CGU_OUT1_CLK       Base clock for CGU_OUT1 clock output
>> +
>> +BASE_PERIPH_CLK and BASE_SPI_CLK is only available on LPC43xx.
>> +BASE_ADCHS_CLK is only available on LPC4370.
>> +
>> +
>> +Example board file:
>> +
>> +/ {
>> +       clocks {
>> +               xtal: xtal {
>> +                       compatible = "fixed-clock";
>> +                       #clock-cells = <0>;
>> +                       clock-frequency = <12000000>;
>> +               };
>> +
>> +               xtal32: xtal32 {
>> +                       compatible = "fixed-clock";
>> +                       #clock-cells = <0>;
>> +                       clock-frequency = <32768>;
>> +               };
>> +
>> +               enet_rx_clk: enet_rx_clk {
>> +                       compatible = "fixed-clock";
>> +                       #clock-cells = <0>;
>> +                       clock-frequency = <0>;
>> +                       clock-output-names = "enet_rx_clk";
>> +               };
>> +
>> +               enet_tx_clk: enet_tx_clk {
>> +                       compatible = "fixed-clock";
>> +                       #clock-cells = <0>;
>> +                       clock-frequency = <0>;
>> +                       clock-output-names = "enet_tx_clk";
>> +               };
>> +
>> +               gp_clkin: gp_clkin {
>> +                       compatible = "fixed-clock";
>> +                       #clock-cells = <0>;
>> +                       clock-frequency = <0>;
>> +                       clock-output-names = "gp_clkin";
>> +               };
>> +       };
>> +
>> +       soc {
>> +               cgu: cgu@40050000 {
>> +                       compatible = "nxp,lpc1850-cgu";
>> +                       reg = <0x40050000 0x1000>;
>> +                       #clock-cells = <1>;
>> +                       clocks = <&xtal>, <&creg_clk 1>, <&enet_rx_clk>, <&enet_tx_clk>, <&gp_clkin>;
>> +                       clock-indices =  <0>,  <1>,  <2>,  <3>,  <4>,  <5>,  <6>,  <7>,
>> +                                        <8>,  <9>, <10>, <11>, <12>, <13>, <14>, <15>,
>> +                                       <16>, <17>, <18>, <19>, <20>, <25>, <26>, <27>;
>> +                       clock-output-names = "base_safe_clk",    "base_usb0_clk",
>> +                                            "base_periph_clk",  "base_usb1_clk",
>> +                                            "base_cpu_clk",     "base_spifi_clk",
>> +                                            "base_spi_clk",     "base_phy_rx_clk",
>> +                                            "base_phy_tx_clk",  "base_apb1_clk",
>> +                                            "base_apb3_clk",    "base_lcd_clk",
>> +                                            "base_adchs_clk",   "base_sdio_clk",
>> +                                            "base_ssp0_clk",    "base_ssp1_clk",
>> +                                            "base_uart0_clk",   "base_uart1_clk",
>> +                                            "base_uart2_clk",   "base_uart3_clk",
>> +                                            "base_out_clk",     "base_audio_clk",
>> +                                            "base_cgu_out0_clk","base_cgu_out1_clk";
>
> Why do you need to use clock-indices?

Since the CGU can have up to 27 clock lines, but in this device not
all are used. So I use clock-indices so I don't need to have empty
entries in the clock-output-names array. I thought this was the
intended usage(?)

> Why do you need to use clock-output-names? If all of your clock
> consumers have nodes in DT then you can skip this and name them on the
> consuming side. See a further explanation here:
> http://lkml.kernel.org/r/<20150416192014.19585.9663@quantum>

Most of the clocks from the CGU goes to a CCU, but the CCU isn't the
real consumer. It's more like a clock router with a bunch of gates.

If you take a close look at the CCU driver you will notice that it
doesn't do clk_get on any of the clocks. My reason for doing this is
to not increase the usage counter on the CGU clock so that if there
are no consumer after a CCU the entire clock line up to the CGU can be
disabled. So this is the reason that the CCU driver uses a "special"
way to grab the clock output names from the CGU.

Hope this makes sense.

> Also note that providing a clock consumer node in the examples section
> of the binding is helpful when reviewing.

I'll add a consumer example in the next version.

regards,
Joachim Eastwood
Mike Turquette May 21, 2015, 9:17 p.m. UTC | #3
Quoting Joachim Eastwood (2015-05-13 00:57:18)
> On 13 May 2015 at 00:12, Michael Turquette <mturquette@linaro.org> wrote:
> > Quoting Joachim Eastwood (2015-04-27 14:38:11)
> >> Add DT binding documentation for lpc1850-cgu driver.
> >>
> >> Signed-off-by: Joachim Eastwood <manabian@gmail.com>
> >> ---
> >>  .../devicetree/bindings/clock/lpc1850-cgu.txt      | 138 +++++++++++++++++++++
> >>  1 file changed, 138 insertions(+)
> >>  create mode 100644 Documentation/devicetree/bindings/clock/lpc1850-cgu.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/clock/lpc1850-cgu.txt b/Documentation/devicetree/bindings/clock/lpc1850-cgu.txt
> >> new file mode 100644
> >> index 000000000000..0b278ca6aee7
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/clock/lpc1850-cgu.txt
> >> @@ -0,0 +1,138 @@
> >> +* NXP LPC1850 Clock Generation Unit (CGU)
> >> +
> >> +The CGU generates multiple independent clocks for the core and the
> >> +peripheral blocks of the LPC18xx. Each independent clock is called
> >> +a base clock and itself is one of the inputs to the two Clock
> >> +Control Units (CCUs) which control the branch clocks to the
> >> +individual peripherals.
> >> +
> >> +The CGU selects the inputs to the clock generators from multiple
> >> +clock sources, controls the clock generation, and routes the outputs
> >> +of the clock generators through the clock source bus to the output
> >> +stages. Each output stage provides an independent clock source and
> >> +corresponds to one of the base clocks for the LPC18xx.
> >> +
> >> + - Above text taken from NXP LPC1850 User Manual.
> >> +
> >> +
> >> +This binding uses the common clock binding:
> >> +    Documentation/devicetree/bindings/clock/clock-bindings.txt
> >> +
> >> +Required properties:
> >> +- compatible:
> >> +       Should be "nxp,lpc1850-cgu"
> >> +- reg:
> >> +       Shall define the base and range of the address space
> >> +       containing clock control registers
> >> +- #clock-cells:
> >> +       Shall have value <1>.  The permitted clock-specifier values
> >> +       are the base clock numbers defined below.
> >> +- clocks:
> >> +       Shall contain a list of phandles for the external input
> >> +       sources to the CGU. The list shall be in the following
> >> +       order: xtal, 32khz, enet_rx_clk, enet_tx_clk, gp_clkin.
> >> +- clock-indices:
> >> +       Shall be an ordered list of numbers defining the base clock
> >> +       number provided by the CGU.
> >> +- clock-output-names:
> >> +       Shall be an ordered list of strings defining the names of
> >> +       the clocks provided by the CGU.
> >> +
> >> +Which base clocks that are available on the CGU depends on the
> >> +specific LPC part. Base cloks are numbered from 0 to 27.
> >
> > s/cloks/clocks/
> >
> >> +
> >> +Number:                Name:                   Description:
> >> + 0             BASE_SAFE_CLK           Base safe clock (always on) for WWDT
> >> + 1             BASE_USB0_CLK           Base clock for USB0
> >> + 2             BASE_PERIPH_CLK         Base clock for Cortex-M0SUB subsystem,
> >> +                                       SPI, and SGPIO
> >> + 3             BASE_USB1_CLK           Base clock for USB1
> >> + 4             BASE_CPU_CLK            System base clock for ARM Cortex-M core
> >> +                                       and APB peripheral blocks #0 and #2
> >> + 5             BASE_SPIFI_CLK          Base clock for SPIFI
> >> + 6             BASE_SPI_CLK            Base clock for SPI
> >> + 7             BASE_PHY_RX_CLK         Base clock for Ethernet PHY Receive clock
> >> + 8             BASE_PHY_TX_CLK         Base clock for Ethernet PHY Transmit clock
> >> + 9             BASE_APB1_CLK           Base clock for APB peripheral block # 1
> >> +10             BASE_APB3_CLK           Base clock for APB peripheral block # 3
> >> +11             BASE_LCD_CLK            Base clock for LCD
> >> +12             BASE_ADCHS_CLK          Base clock for ADCHS
> >> +13             BASE_SDIO_CLK           Base clock for SD/MMC
> >> +14             BASE_SSP0_CLK           Base clock for SSP0
> >> +15             BASE_SSP1_CLK           Base clock for SSP1
> >> +16             BASE_UART0_CLK          Base clock for UART0
> >> +17             BASE_UART1_CLK          Base clock for UART1
> >> +18             BASE_UART2_CLK          Base clock for UART2
> >> +19             BASE_UART3_CLK          Base clock for UART3
> >> +20             BASE_OUT_CLK            Base clock for CLKOUT pin
> >> +24-21          -                       Reserved
> >> +25             BASE_AUDIO_CLK          Base clock for audio system (I2S)
> >> +26             BASE_CGU_OUT0_CLK       Base clock for CGU_OUT0 clock output
> >> +27             BASE_CGU_OUT1_CLK       Base clock for CGU_OUT1 clock output
> >> +
> >> +BASE_PERIPH_CLK and BASE_SPI_CLK is only available on LPC43xx.
> >> +BASE_ADCHS_CLK is only available on LPC4370.
> >> +
> >> +
> >> +Example board file:
> >> +
> >> +/ {
> >> +       clocks {
> >> +               xtal: xtal {
> >> +                       compatible = "fixed-clock";
> >> +                       #clock-cells = <0>;
> >> +                       clock-frequency = <12000000>;
> >> +               };
> >> +
> >> +               xtal32: xtal32 {
> >> +                       compatible = "fixed-clock";
> >> +                       #clock-cells = <0>;
> >> +                       clock-frequency = <32768>;
> >> +               };
> >> +
> >> +               enet_rx_clk: enet_rx_clk {
> >> +                       compatible = "fixed-clock";
> >> +                       #clock-cells = <0>;
> >> +                       clock-frequency = <0>;
> >> +                       clock-output-names = "enet_rx_clk";
> >> +               };
> >> +
> >> +               enet_tx_clk: enet_tx_clk {
> >> +                       compatible = "fixed-clock";
> >> +                       #clock-cells = <0>;
> >> +                       clock-frequency = <0>;
> >> +                       clock-output-names = "enet_tx_clk";
> >> +               };
> >> +
> >> +               gp_clkin: gp_clkin {
> >> +                       compatible = "fixed-clock";
> >> +                       #clock-cells = <0>;
> >> +                       clock-frequency = <0>;
> >> +                       clock-output-names = "gp_clkin";
> >> +               };
> >> +       };
> >> +
> >> +       soc {
> >> +               cgu: cgu@40050000 {
> >> +                       compatible = "nxp,lpc1850-cgu";
> >> +                       reg = <0x40050000 0x1000>;
> >> +                       #clock-cells = <1>;
> >> +                       clocks = <&xtal>, <&creg_clk 1>, <&enet_rx_clk>, <&enet_tx_clk>, <&gp_clkin>;
> >> +                       clock-indices =  <0>,  <1>,  <2>,  <3>,  <4>,  <5>,  <6>,  <7>,
> >> +                                        <8>,  <9>, <10>, <11>, <12>, <13>, <14>, <15>,
> >> +                                       <16>, <17>, <18>, <19>, <20>, <25>, <26>, <27>;
> >> +                       clock-output-names = "base_safe_clk",    "base_usb0_clk",
> >> +                                            "base_periph_clk",  "base_usb1_clk",
> >> +                                            "base_cpu_clk",     "base_spifi_clk",
> >> +                                            "base_spi_clk",     "base_phy_rx_clk",
> >> +                                            "base_phy_tx_clk",  "base_apb1_clk",
> >> +                                            "base_apb3_clk",    "base_lcd_clk",
> >> +                                            "base_adchs_clk",   "base_sdio_clk",
> >> +                                            "base_ssp0_clk",    "base_ssp1_clk",
> >> +                                            "base_uart0_clk",   "base_uart1_clk",
> >> +                                            "base_uart2_clk",   "base_uart3_clk",
> >> +                                            "base_out_clk",     "base_audio_clk",
> >> +                                            "base_cgu_out0_clk","base_cgu_out1_clk";
> >
> > Why do you need to use clock-indices?
> 
> Since the CGU can have up to 27 clock lines, but in this device not
> all are used. So I use clock-indices so I don't need to have empty
> entries in the clock-output-names array. I thought this was the
> intended usage(?)
> 
> > Why do you need to use clock-output-names? If all of your clock
> > consumers have nodes in DT then you can skip this and name them on the
> > consuming side. See a further explanation here:
> > http://lkml.kernel.org/r/<20150416192014.19585.9663@quantum>
> 
> Most of the clocks from the CGU goes to a CCU, but the CCU isn't the
> real consumer. It's more like a clock router with a bunch of gates.
> 
> If you take a close look at the CCU driver you will notice that it
> doesn't do clk_get on any of the clocks. My reason for doing this is
> to not increase the usage counter on the CGU clock so that if there
> are no consumer after a CCU the entire clock line up to the CGU can be
> disabled. So this is the reason that the CCU driver uses a "special"
> way to grab the clock output names from the CGU.

A call to clk_get will not enable/ungate the clock signal. It seems that
there might be some confusion around that. It is always OK to get the
clk. This will not increase the enable_count or prepare_count values.

If the CCU is a separate IP block then it might make sense for it to
call clk_get on the CGU clocks. On the other hand it might make more
sense for the CCU clocks to simply use the CGU clocks as parent clock
signal inputs.

None of the points above have anything to do with my original comment.
The use of the clock-output-names property is unnecessary if all of the
consumer devices have DT nodes of their own. Please look at the thread I
linked to and let me know what you think about getting rid of the
clock-output-names property.

> 
> Hope this makes sense.
> 
> > Also note that providing a clock consumer node in the examples section
> > of the binding is helpful when reviewing.
> 
> I'll add a consumer example in the next version.

Great.

Thanks,
Mike

> 
> regards,
> Joachim Eastwood
Joachim Eastwood May 21, 2015, 10:21 p.m. UTC | #4
On 21 May 2015 at 23:17, Michael Turquette <mturquette@linaro.org> wrote:
> Quoting Joachim Eastwood (2015-05-13 00:57:18)
>> On 13 May 2015 at 00:12, Michael Turquette <mturquette@linaro.org> wrote:
>> >
>> > Why do you need to use clock-indices?
>>
>> Since the CGU can have up to 27 clock lines, but in this device not
>> all are used. So I use clock-indices so I don't need to have empty
>> entries in the clock-output-names array. I thought this was the
>> intended usage(?)
>>
>> > Why do you need to use clock-output-names? If all of your clock
>> > consumers have nodes in DT then you can skip this and name them on the
>> > consuming side. See a further explanation here:
>> > http://lkml.kernel.org/r/<20150416192014.19585.9663@quantum>
>>
>> Most of the clocks from the CGU goes to a CCU, but the CCU isn't the
>> real consumer. It's more like a clock router with a bunch of gates.
>>
>> If you take a close look at the CCU driver you will notice that it
>> doesn't do clk_get on any of the clocks. My reason for doing this is
>> to not increase the usage counter on the CGU clock so that if there
>> are no consumer after a CCU the entire clock line up to the CGU can be
>> disabled. So this is the reason that the CCU driver uses a "special"
>> way to grab the clock output names from the CGU.
>
> A call to clk_get will not enable/ungate the clock signal. It seems that
> there might be some confusion around that. It is always OK to get the
> clk. This will not increase the enable_count or prepare_count values.
>
> If the CCU is a separate IP block then it might make sense for it to
> call clk_get on the CGU clocks. On the other hand it might make more
> sense for the CCU clocks to simply use the CGU clocks as parent clock
> signal inputs.

v3, that was sent a couple of days ago, uses of_clk_get/clk_get_name
to set the CCU clock parents. See v3 cover letter for a short
explanation.

> None of the points above have anything to do with my original comment.
> The use of the clock-output-names property is unnecessary if all of the
> consumer devices have DT nodes of their own. Please look at the thread I
> linked to and let me know what you think about getting rid of the
> clock-output-names property.

v3 removes the property. Please take a look at it when you get the time.


regards,
Joachim Eastwood
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/clock/lpc1850-cgu.txt b/Documentation/devicetree/bindings/clock/lpc1850-cgu.txt
new file mode 100644
index 000000000000..0b278ca6aee7
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/lpc1850-cgu.txt
@@ -0,0 +1,138 @@ 
+* NXP LPC1850 Clock Generation Unit (CGU)
+
+The CGU generates multiple independent clocks for the core and the
+peripheral blocks of the LPC18xx. Each independent clock is called
+a base clock and itself is one of the inputs to the two Clock
+Control Units (CCUs) which control the branch clocks to the
+individual peripherals.
+
+The CGU selects the inputs to the clock generators from multiple
+clock sources, controls the clock generation, and routes the outputs
+of the clock generators through the clock source bus to the output
+stages. Each output stage provides an independent clock source and
+corresponds to one of the base clocks for the LPC18xx.
+
+ - Above text taken from NXP LPC1850 User Manual.
+
+
+This binding uses the common clock binding:
+    Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+Required properties:
+- compatible:
+	Should be "nxp,lpc1850-cgu"
+- reg:
+	Shall define the base and range of the address space
+	containing clock control registers
+- #clock-cells:
+	Shall have value <1>.  The permitted clock-specifier values
+	are the base clock numbers defined below.
+- clocks:
+	Shall contain a list of phandles for the external input
+	sources to the CGU. The list shall be in the following
+	order: xtal, 32khz, enet_rx_clk, enet_tx_clk, gp_clkin.
+- clock-indices:
+	Shall be an ordered list of numbers defining the base clock
+	number provided by the CGU.
+- clock-output-names:
+	Shall be an ordered list of strings defining the names of
+	the clocks provided by the CGU.
+
+Which base clocks that are available on the CGU depends on the
+specific LPC part. Base cloks are numbered from 0 to 27.
+
+Number:		Name:			Description:
+ 0		BASE_SAFE_CLK		Base safe clock (always on) for WWDT
+ 1		BASE_USB0_CLK		Base clock for USB0
+ 2		BASE_PERIPH_CLK		Base clock for Cortex-M0SUB subsystem,
+					SPI, and SGPIO
+ 3		BASE_USB1_CLK		Base clock for USB1
+ 4		BASE_CPU_CLK		System base clock for ARM Cortex-M core
+					and APB peripheral blocks #0 and #2
+ 5		BASE_SPIFI_CLK		Base clock for SPIFI
+ 6		BASE_SPI_CLK		Base clock for SPI
+ 7		BASE_PHY_RX_CLK		Base clock for Ethernet PHY Receive clock
+ 8		BASE_PHY_TX_CLK		Base clock for Ethernet PHY Transmit clock
+ 9		BASE_APB1_CLK		Base clock for APB peripheral block # 1
+10		BASE_APB3_CLK		Base clock for APB peripheral block # 3
+11		BASE_LCD_CLK		Base clock for LCD
+12		BASE_ADCHS_CLK		Base clock for ADCHS
+13		BASE_SDIO_CLK		Base clock for SD/MMC
+14		BASE_SSP0_CLK		Base clock for SSP0
+15		BASE_SSP1_CLK		Base clock for SSP1
+16		BASE_UART0_CLK		Base clock for UART0
+17		BASE_UART1_CLK		Base clock for UART1
+18		BASE_UART2_CLK		Base clock for UART2
+19		BASE_UART3_CLK		Base clock for UART3
+20		BASE_OUT_CLK		Base clock for CLKOUT pin
+24-21		-			Reserved
+25		BASE_AUDIO_CLK		Base clock for audio system (I2S)
+26 		BASE_CGU_OUT0_CLK	Base clock for CGU_OUT0 clock output
+27 		BASE_CGU_OUT1_CLK	Base clock for CGU_OUT1 clock output
+
+BASE_PERIPH_CLK and BASE_SPI_CLK is only available on LPC43xx.
+BASE_ADCHS_CLK is only available on LPC4370.
+
+
+Example board file:
+
+/ {
+	clocks {
+		xtal: xtal {
+			compatible = "fixed-clock";
+			#clock-cells = <0>;
+			clock-frequency = <12000000>;
+		};
+
+		xtal32: xtal32 {
+			compatible = "fixed-clock";
+			#clock-cells = <0>;
+			clock-frequency = <32768>;
+		};
+
+		enet_rx_clk: enet_rx_clk {
+			compatible = "fixed-clock";
+			#clock-cells = <0>;
+			clock-frequency = <0>;
+			clock-output-names = "enet_rx_clk";
+		};
+
+		enet_tx_clk: enet_tx_clk {
+			compatible = "fixed-clock";
+			#clock-cells = <0>;
+			clock-frequency = <0>;
+			clock-output-names = "enet_tx_clk";
+		};
+
+		gp_clkin: gp_clkin {
+			compatible = "fixed-clock";
+			#clock-cells = <0>;
+			clock-frequency = <0>;
+			clock-output-names = "gp_clkin";
+		};
+	};
+
+	soc {
+		cgu: cgu@40050000 {
+			compatible = "nxp,lpc1850-cgu";
+			reg = <0x40050000 0x1000>;
+			#clock-cells = <1>;
+			clocks = <&xtal>, <&creg_clk 1>, <&enet_rx_clk>, <&enet_tx_clk>, <&gp_clkin>;
+			clock-indices =  <0>,  <1>,  <2>,  <3>,  <4>,  <5>,  <6>,  <7>,
+					 <8>,  <9>, <10>, <11>, <12>, <13>, <14>, <15>,
+					<16>, <17>, <18>, <19>, <20>, <25>, <26>, <27>;
+			clock-output-names = "base_safe_clk",    "base_usb0_clk",
+					     "base_periph_clk",  "base_usb1_clk",
+					     "base_cpu_clk",     "base_spifi_clk",
+					     "base_spi_clk",     "base_phy_rx_clk",
+					     "base_phy_tx_clk",  "base_apb1_clk",
+					     "base_apb3_clk",    "base_lcd_clk",
+					     "base_adchs_clk",   "base_sdio_clk",
+					     "base_ssp0_clk",    "base_ssp1_clk",
+					     "base_uart0_clk",   "base_uart1_clk",
+					     "base_uart2_clk",   "base_uart3_clk",
+					     "base_out_clk",     "base_audio_clk",
+					     "base_cgu_out0_clk","base_cgu_out1_clk";
+		};
+	};
+};