| Message ID | 20230116115050.2983406-2-dev@kicherer.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | spi: spi-fsl-qspi: support setting sampling delay through devicetree | expand |
On Mon, 16 Jan 2023 12:50:49 +0100, Mario Kicherer wrote: > Add optional sampling-delay property to delay the internal sampling point for > incoming data. > > Signed-off-by: Mario Kicherer <dev@kicherer.org> > --- > Documentation/devicetree/bindings/spi/fsl,spi-fsl-qspi.yaml | 6 ++++++ > 1 file changed, 6 insertions(+) > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/spi/fsl,spi-fsl-qspi.yaml: properties:fsl,qspi-sampling-delay: 'oneOf' conditional failed, one must be fixed: 'type' is a required property hint: A vendor boolean property can use "type: boolean" Additional properties are not allowed ('default', 'maximum', 'minimum' were unexpected) hint: A vendor boolean property can use "type: boolean" /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/spi/fsl,spi-fsl-qspi.yaml: properties:fsl,qspi-sampling-delay: 'oneOf' conditional failed, one must be fixed: 'enum' is a required property 'const' is a required property hint: A vendor string property with exact values has an implicit type from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/spi/fsl,spi-fsl-qspi.yaml: properties:fsl,qspi-sampling-delay: 'oneOf' conditional failed, one must be fixed: '$ref' is a required property 'allOf' is a required property hint: A vendor property needs a $ref to types.yaml from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml# hint: Vendor specific properties must have a type and description unless they have a defined, common suffix. from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml# doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230116115050.2983406-2-dev@kicherer.org The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
On Mon, Jan 16, 2023 at 12:50:49PM +0100, Mario Kicherer wrote: > Add optional sampling-delay property to delay the internal sampling point for > incoming data. > > Signed-off-by: Mario Kicherer <dev@kicherer.org> > --- > Documentation/devicetree/bindings/spi/fsl,spi-fsl-qspi.yaml | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/Documentation/devicetree/bindings/spi/fsl,spi-fsl-qspi.yaml b/Documentation/devicetree/bindings/spi/fsl,spi-fsl-qspi.yaml > index e58644558412..7952a4be938b 100644 > --- a/Documentation/devicetree/bindings/spi/fsl,spi-fsl-qspi.yaml > +++ b/Documentation/devicetree/bindings/spi/fsl,spi-fsl-qspi.yaml > @@ -54,6 +54,12 @@ properties: > - const: qspi_en > - const: qspi > > + fsl,qspi-sampling-delay: > + description: delay sampling of incoming data by this number of half cycles Use the common rx-sample-delay-ns property. > + minimum: 0 > + maximum: 3 > + default: 0 > + > required: > - compatible > - reg > -- > 2.34.1 >
Hello, unfortunately, the rx-sample-delay-ns property does not fit here, as we can only delay the sampling point between zero and three "half cycles" (or edges), not by an arbitrary number of nanoseconds. Regarding the bot message, I do not understand what is wrong in my patch. I found similar property descriptions in other files and also in the official doc [1] there is an equal (to me) example under "clock-frequency", if I am not missing something. Thank you for the review! Best regards, Mario [1] https://docs.kernel.org/devicetree/bindings/writing-schema.html On 2023-01-17 15:10, Rob Herring wrote: > On Mon, Jan 16, 2023 at 12:50:49PM +0100, Mario Kicherer wrote: >> Add optional sampling-delay property to delay the internal sampling >> point for >> incoming data. >> >> Signed-off-by: Mario Kicherer <dev@kicherer.org> >> --- >> Documentation/devicetree/bindings/spi/fsl,spi-fsl-qspi.yaml | 6 >> ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git >> a/Documentation/devicetree/bindings/spi/fsl,spi-fsl-qspi.yaml >> b/Documentation/devicetree/bindings/spi/fsl,spi-fsl-qspi.yaml >> index e58644558412..7952a4be938b 100644 >> --- a/Documentation/devicetree/bindings/spi/fsl,spi-fsl-qspi.yaml >> +++ b/Documentation/devicetree/bindings/spi/fsl,spi-fsl-qspi.yaml >> @@ -54,6 +54,12 @@ properties: >> - const: qspi_en >> - const: qspi >> >> + fsl,qspi-sampling-delay: >> + description: delay sampling of incoming data by this number of >> half cycles > > Use the common rx-sample-delay-ns property. > >> + minimum: 0 >> + maximum: 3 >> + default: 0 >> + >> required: >> - compatible >> - reg >> -- >> 2.34.1 >>
On 17/01/2023 17:33, Mario Kicherer wrote: > Hello, > > unfortunately, the rx-sample-delay-ns property does not fit here, as we > can only delay > the sampling point between zero and three "half cycles" (or edges), not > by an arbitrary > number of nanoseconds. Why this is a problem for FSL but not for other platforms having exactly the same constraints/property? https://lore.kernel.org/all/20221208062955.2546-6-xiangsheng.hou@mediatek.com/ Best regards, Krzysztof
On 23/01/17 06:10PM, Krzysztof Kozlowski wrote: > On 17/01/2023 17:33, Mario Kicherer wrote: > > Hello, > > > > unfortunately, the rx-sample-delay-ns property does not fit here, as we > > can only delay > > the sampling point between zero and three "half cycles" (or edges), not > > by an arbitrary > > number of nanoseconds. > > Why this is a problem for FSL but not for other platforms having exactly > the same constraints/property? Hi Mario, Please use the common delay in DT and calculate to half cycle in driver, we have the similar discussion before for fspi controller delay settings. > > Best regards, > Krzysztof >
From: "han.xu" <han.xu@nxp.com> Hi, >>> unfortunately, the rx-sample-delay-ns property does not fit here, as we >>> can only delay >>> the sampling point between zero and three "half cycles" (or edges), not >>> by an arbitrary >>> number of nanoseconds. >> >> Why this is a problem for FSL but not for other platforms having exactly >> the same constraints/property? > > Please use the common delay in DT and calculate to half cycle in driver, we have > the similar discussion before for fspi controller delay settings. Do you mean [1]? There my suggestion was to use a -degrees property (because it doesn't depend on the frequency). There wasn't any follow-up, or did I miss something? -michael [1] https://lore.kernel.org/linux-spi/62f113a0cdb0d58bf04ab0b274912eb7@walle.cc/
On 18/01/2023 09:01, Michael Walle wrote: > From: "han.xu" <han.xu@nxp.com> > > Hi, > >>>> unfortunately, the rx-sample-delay-ns property does not fit here, as we >>>> can only delay >>>> the sampling point between zero and three "half cycles" (or edges), not >>>> by an arbitrary >>>> number of nanoseconds. >>> >>> Why this is a problem for FSL but not for other platforms having exactly >>> the same constraints/property? >> >> Please use the common delay in DT and calculate to half cycle in driver, we have >> the similar discussion before for fspi controller delay settings. > > Do you mean [1]? There my suggestion was to use a -degrees property (because > it doesn't depend on the frequency). There wasn't any follow-up, or did I miss > something? > > -michael > > [1] https://lore.kernel.org/linux-spi/62f113a0cdb0d58bf04ab0b274912eb7@walle.cc/ I think the patch using existing ns property (and calculating cycles or phase shift or whatever was needed) was merged. In such case please go the same way. Best regards, Krzysztof
>>>>> unfortunately, the rx-sample-delay-ns property does not fit here, >>>>> as we >>>>> can only delay >>>>> the sampling point between zero and three "half cycles" (or edges), >>>>> not >>>>> by an arbitrary >>>>> number of nanoseconds. >>>> >>>> Why this is a problem for FSL but not for other platforms having >>>> exactly >>>> the same constraints/property? >>> >>> Please use the common delay in DT and calculate to half cycle in >>> driver, we have >>> the similar discussion before for fspi controller delay settings. >> >> Do you mean [1]? There my suggestion was to use a -degrees property >> (because >> it doesn't depend on the frequency). There wasn't any follow-up, or >> did I miss >> something? >> >> -michael >> >> [1] >> https://lore.kernel.org/linux-spi/62f113a0cdb0d58bf04ab0b274912eb7@walle.cc/ > > I think the patch using existing ns property (and calculating cycles or > phase shift or whatever was needed) was merged. In such case please go > the same way. I couldn't find anything that the fspi driver now supports this delay. I still think this is the wrong way to go. Like I said, it depends on the frequency, which means that you need to change the delay-ns property everytime you change the frequency. Think of a bootloader which patches the frequency (or something like that). But what's worse is that you cannot have an enum in the binding for this property then. Now OTOH, you could actually have a hardware which take the delay as a time period, in that case this -delay-ns makes sense and recalculating that into -degrees would be impossible. Are there standard properties which expresses the same, but just in a different way? Like for example drive strength as an impedance or in current at a specific voltage. But having two different properties for the same thing might be just as bad.. -michael
Hello Han, on my SoC (LS1021a), the QSPI clock is derived from the CPU clock (cluster1) and neither is controlled by the Linux kernel (afair) but by the RCW (or U-Boot). I could create a function to read the corresponding registers but I do not know where I should place this function and how I should call this function in a portable way in the QSPI module to convert the nanoseconds into delay cycles. I thought this would be a small and simple patch but I guess these changes will require quite some time with my knowledge level that I do not have right now. Thank you all for the review! Best regards, Mario On 2023-01-17 22:05, han.xu wrote: > On 23/01/17 06:10PM, Krzysztof Kozlowski wrote: >> On 17/01/2023 17:33, Mario Kicherer wrote: >> > Hello, >> > >> > unfortunately, the rx-sample-delay-ns property does not fit here, as we >> > can only delay >> > the sampling point between zero and three "half cycles" (or edges), not >> > by an arbitrary >> > number of nanoseconds. >> >> Why this is a problem for FSL but not for other platforms having >> exactly >> the same constraints/property? > > Hi Mario, > > Please use the common delay in DT and calculate to half cycle in > driver, we have > the similar discussion before for fspi controller delay settings. > >> >> Best regards, >> Krzysztof >>
diff --git a/Documentation/devicetree/bindings/spi/fsl,spi-fsl-qspi.yaml b/Documentation/devicetree/bindings/spi/fsl,spi-fsl-qspi.yaml index e58644558412..7952a4be938b 100644 --- a/Documentation/devicetree/bindings/spi/fsl,spi-fsl-qspi.yaml +++ b/Documentation/devicetree/bindings/spi/fsl,spi-fsl-qspi.yaml @@ -54,6 +54,12 @@ properties: - const: qspi_en - const: qspi + fsl,qspi-sampling-delay: + description: delay sampling of incoming data by this number of half cycles + minimum: 0 + maximum: 3 + default: 0 + required: - compatible - reg
Add optional sampling-delay property to delay the internal sampling point for incoming data. Signed-off-by: Mario Kicherer <dev@kicherer.org> --- Documentation/devicetree/bindings/spi/fsl,spi-fsl-qspi.yaml | 6 ++++++ 1 file changed, 6 insertions(+)