Re: [PATCH 1/2] clk: qcom: Add DT bindings for ipq6018 apss clock controller

[Sivaprakash Murugesan <sivaprak@codeaurora.org>]

Hi Sibi,

Thanks for the review.

On 2/27/2020 4:08 PM, Sibi Sankar wrote:
> Hey Sivaprakash,
>
> On 2020-02-27 15:25, Sivaprakash Murugesan wrote:
>> add dt-binding for ipq6018 apss clock controller
>>
>> Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
>> ---
>>  .../devicetree/bindings/clock/qcom,apsscc.yaml     | 58 
>> ++++++++++++++++++++++
>>  include/dt-bindings/clock/qcom,apss-ipq6018.h      | 26 ++++++++++
>>  2 files changed, 84 insertions(+)
>>  create mode 100644 
>> Documentation/devicetree/bindings/clock/qcom,apsscc.yaml
>>  create mode 100644 include/dt-bindings/clock/qcom,apss-ipq6018.h
>>
>> diff --git a/Documentation/devicetree/bindings/clock/qcom,apsscc.yaml
>> b/Documentation/devicetree/bindings/clock/qcom,apsscc.yaml
>> new file mode 100644
>> index 0000000..7433721
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/qcom,apsscc.yaml
>> @@ -0,0 +1,58 @@
>> +# SPDX-License-Identifier: GPL-2.0-only
>
> Dual license
ok.
>
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/bindings/clock/qcom,apsscc.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Qualcomm IPQ6018 APSS Clock Controller Binding
>> +
>> +maintainers:
>> +  - Stephen Boyd <sboyd@kernel.org>
>> +
>> +description: |
>> +  Qualcomm IPQ6018 APSS clock control module which supports the 
>> clocks with
>> +  frequencies above 800Mhz.
>> +
>> +properties:
>> +  compatible :
>> +    const: qcom,apss-ipq6018
>
> Please use qcom,<chip>-<device>
> instead.
>
ok.
>> +
>> +  clocks:
>> +    description: clocks required for this controller.
>> +    maxItems: 4
>> +
>> +  clock-names:
>> +    description: clock output names of required clocks.
>> +    maxItems: 4
>> +
>> +  '#clock-cells':
>> +    const: 1
>> +
>> +  '#reset-cells':
>> +    const: 1
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - '#clock-cells'
>> +  - '#reset-cells'
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +      #include <dt-bindings/clock/qcom,gcc-ipq6018.h>
>> +      apss_clk: qcom,apss_clk@b111000 {
>> +            compatible = "qcom,apss-ipq6018";
>> +            clocks = <&xo>, <&gcc GPLL0>,
>> +                        <&gcc GPLL2>, <&gcc GPLL4>;
>> +            clock-names = "xo", "gpll0",
>> +                         "gpll2", "gpll4";
>> +            reg = <0xb11100c 0x5ff4>;
>> +            #clock-cells = <1>;
>> +            #reset-cells = <1>;
>> +      };
>> +...
>> diff --git a/include/dt-bindings/clock/qcom,apss-ipq6018.h
>> b/include/dt-bindings/clock/qcom,apss-ipq6018.h
>> new file mode 100644
>> index 0000000..ed9d7d8
>> --- /dev/null
>> +++ b/include/dt-bindings/clock/qcom,apss-ipq6018.h
>> @@ -0,0 +1,26 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * Copyright (c) 2016-2018, The Linux Foundation. All rights reserved.
>> + *
>> + * Permission to use, copy, modify, and/or distribute this software 
>> for any
>> + * purpose with or without fee is hereby granted, provided that the 
>> above
>> + * copyright notice and this permission notice appear in all copies.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL 
>> WARRANTIES
>> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
>> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE 
>> LIABLE FOR
>> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY 
>> DAMAGES
>> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER 
>> IN AN
>> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING 
>> OUT OF
>> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
>
> ^^ is not needed just the SPDX
> license identifier is enough.
ok.
>
>> + */
>> +
>> +#ifndef _DT_BINDINGS_CLOCK_QCA_APSS_IPQ6018_H
>> +#define _DT_BINDINGS_CLOCK_QCA_APSS_IPQ6018_H
>> +
>> +#define APSS_PLL_EARLY                0
>> +#define APSS_PLL                1
>> +#define APCS_ALIAS0_CLK_SRC            2
>> +#define APCS_ALIAS0_CORE_CLK            3
>> +
>> +#endif
>