Messages in this thread | | | From | Akash Asthana <> | Subject | Re: [PATCH 1/2] dt-bindings: spi: Convert QSPI bindings to YAML | Date | Fri, 13 Mar 2020 16:21:22 +0530 |
| |
Hi Stephen,
On 2/20/2020 7:59 AM, Stephen Boyd wrote: > Quoting Akash Asthana (2020-02-17 01:49:33) >> diff --git a/Documentation/devicetree/bindings/spi/qcom,spi-qcom-qspi.yaml b/Documentation/devicetree/bindings/spi/qcom,spi-qcom-qspi.yaml >> new file mode 100644 >> index 0000000..977070a >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/spi/qcom,spi-qcom-qspi.yaml >> @@ -0,0 +1,89 @@ >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >> + >> +%YAML 1.2 >> +--- >> +$id:"http://devicetree.org/schemas/spi/qcom,spi-qcom-qspi.yaml#" >> +$schema:"http://devicetree.org/meta-schemas/core.yaml#" >> + >> +title: Qualcomm Quad Serial Peripheral Interface (QSPI) >> + >> +maintainers: >> + - Mukesh Savaliya<msavaliy@codeaurora.org> >> + - Akash Asthana<akashast@codeaurora.org> >> + >> +description: | > Drop the | because it doesn't look like any formatting needs to be > maintained in the text for the description. ok >> + The QSPI controller allows SPI protocol communication in single, dual, or quad >> + wire transmission modes for read/write access to slaves such as NOR flash. >> + >> +allOf: >> + - $ref: /spi/spi-controller.yaml# >> + >> +properties: >> + compatible: >> + items: >> + - const: qcom,sdm845-qspi >> + - const: qcom,qspi-v1 >> + >> + reg: >> + description: Base register location and length. > Drop description? It doesn't seem useful. ok >> + maxItems: 1 >> + >> + interrupts: >> + maxItems: 1 >> + >> + clock-names: >> + items: >> + - const: iface >> + - const: core >> + >> + clocks: >> + items: >> + - description: AHB clock >> + - description: QSPI core clock. > Please drop the full-stop on core clock. ok >> + >> + "#address-cells": >> + const: 1 >> + >> + "#size-cells": >> + const: 0 > Aren't these two unnecessary because they're covered by the > spi-controller.yaml binding? ok > >> + >> +required: >> + - compatible >> + - reg >> + - interrupts >> + - clock-names >> + - clocks >> + - "#address-cells" >> + - "#size-cells" > These last two are also covered by spi-controller binding. ok will remove >> + >> + > Why two newlines instead of one? > >> +examples: >> + - | >> + #include <dt-bindings/clock/qcom,gcc-sdm845.h> >> + #include <dt-bindings/interrupt-controller/arm-gic.h> >> + >> + soc: soc@0 { > Remove this node from example please.
If I remove this node I am getting below compilation error.
Error: Documentation/devicetree/bindings/spi/qcom,spi-qcom-qspi.example.dts:46.1-2 syntax error FATAL ERROR: Unable to parse input tree scripts/Makefile.lib:311: recipe for target 'Documentation/devicetree/bindings/spi/qcom,spi-qcom-qspi.example.dt.yaml' failed make[1]: *** [Documentation/devicetree/bindings/spi/qcom,spi-qcom-qspi.example.dt.yaml] Error 1 Makefile:1264: recipe for target 'dt_binding_check' failed make: *** [dt_binding_check] Error 2
> >> + #address-cells = <2>; >> + #size-cells = <2>; >> + >> + qspi: spi@88df000 { >> + compatible = "qcom,sdm845-qspi", "qcom,qspi-v1"; >> + reg = <0 0x88df000 0 0x600>; >> + #address-cells = <1>; >> + #size-cells = <0>; >> + interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>; >> + clock-names = "iface", "core"; >> + clocks = <&gcc GCC_QSPI_CNOC_PERIPH_AHB_CLK>, >> + <&gcc GCC_QSPI_CORE_CLK>; > Weird tabbing here. Just use spaces and align it up. Ok, I will align it better. > >> + >> + flash@0 { >> + compatible = "jedec,spi-nor"; >> + reg = <0>; >> + spi-max-frequency = <25000000>; >> + spi-tx-bus-width = <2>; >> + spi-rx-bus-width = <2>; >> + }; > Is this flash node necessary for the example?
It's not neccessary.
I just preserved the original example from .txt binding file.
> >> + }; >> + }; >> + > Nitpick: Why newline here?
Will remove it.
Thankyou for reviewing the patch.
Regards,
Akash
> >> +...
-- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project
| |