| Message ID | 20221205065756.26875-8-xiangsheng.hou@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add MediaTek MT7986 SPI NAND and ECC support | expand |
On 05/12/2022 07:57, Xiangsheng Hou wrote: > Add mediatek,rx-latch-latency property which adjust read delay in the > unit of clock cycle. > > Signed-off-by: Xiangsheng Hou <xiangsheng.hou@mediatek.com> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof
Il 05/12/22 07:57, Xiangsheng Hou ha scritto: > Add mediatek,rx-latch-latency property which adjust read delay in the > unit of clock cycle. > > Signed-off-by: Xiangsheng Hou <xiangsheng.hou@mediatek.com> > Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > --- > .../devicetree/bindings/spi/mediatek,spi-mtk-snfi.yaml | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-snfi.yaml b/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-snfi.yaml > index bab23f1b11fd..6e6ff8d73fcd 100644 > --- a/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-snfi.yaml > +++ b/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-snfi.yaml > @@ -45,6 +45,13 @@ properties: > description: device-tree node of the accompanying ECC engine. > $ref: /schemas/types.yaml#/definitions/phandle > > + mediatek,rx-latch-latency: > + description: Rx delay to sample data with this value, the value > + unit is clock cycle. Can't we use nanoseconds or microseconds as a unit here, instead of clock cycles? Regards, Angelo
Hi Angelo, On Mon, 2022-12-05 at 15:21 +0100, AngeloGioacchino Del Regno wrote: > Il 05/12/22 07:57, Xiangsheng Hou ha scritto: > > Add mediatek,rx-latch-latency property which adjust read delay in > > the > > unit of clock cycle. > > > > Signed-off-by: Xiangsheng Hou <xiangsheng.hou@mediatek.com> > > Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > > --- > > .../devicetree/bindings/spi/mediatek,spi-mtk-snfi.yaml | 7 > > +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/spi/mediatek,spi- > > mtk-snfi.yaml b/Documentation/devicetree/bindings/spi/mediatek,spi- > > mtk-snfi.yaml > > index bab23f1b11fd..6e6ff8d73fcd 100644 > > --- a/Documentation/devicetree/bindings/spi/mediatek,spi-mtk- > > snfi.yaml > > +++ b/Documentation/devicetree/bindings/spi/mediatek,spi-mtk- > > snfi.yaml > > @@ -45,6 +45,13 @@ properties: > > description: device-tree node of the accompanying ECC engine. > > $ref: /schemas/types.yaml#/definitions/phandle > > > > + mediatek,rx-latch-latency: > > + description: Rx delay to sample data with this value, the > > value > > + unit is clock cycle. > > Can't we use nanoseconds or microseconds as a unit here, instead of > clock cycles? The clock cycle will be various with MediaTek SPI NAND controller which clock frequency can support 26/52/68/81/104MHz... It`s may be easy to configure and understand with clock cycle in unit. Thanks Xiangsheng Hou
Il 06/12/22 10:04, Xiangsheng Hou (侯祥胜) ha scritto: > Hi Angelo, > > On Mon, 2022-12-05 at 15:21 +0100, AngeloGioacchino Del Regno wrote: >> Il 05/12/22 07:57, Xiangsheng Hou ha scritto: >>> Add mediatek,rx-latch-latency property which adjust read delay in >>> the >>> unit of clock cycle. >>> >>> Signed-off-by: Xiangsheng Hou <xiangsheng.hou@mediatek.com> >>> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >>> --- >>> .../devicetree/bindings/spi/mediatek,spi-mtk-snfi.yaml | 7 >>> +++++++ >>> 1 file changed, 7 insertions(+) >>> >>> diff --git a/Documentation/devicetree/bindings/spi/mediatek,spi- >>> mtk-snfi.yaml b/Documentation/devicetree/bindings/spi/mediatek,spi- >>> mtk-snfi.yaml >>> index bab23f1b11fd..6e6ff8d73fcd 100644 >>> --- a/Documentation/devicetree/bindings/spi/mediatek,spi-mtk- >>> snfi.yaml >>> +++ b/Documentation/devicetree/bindings/spi/mediatek,spi-mtk- >>> snfi.yaml >>> @@ -45,6 +45,13 @@ properties: >>> description: device-tree node of the accompanying ECC engine. >>> $ref: /schemas/types.yaml#/definitions/phandle >>> >>> + mediatek,rx-latch-latency: >>> + description: Rx delay to sample data with this value, the >>> value >>> + unit is clock cycle. >> >> Can't we use nanoseconds or microseconds as a unit here, instead of >> clock cycles? > > The clock cycle will be various with MediaTek SPI NAND controller which > clock frequency can support 26/52/68/81/104MHz... > It`s may be easy to configure and understand with clock cycle in unit. > Yes, but whatever clock frequency we use, the target is to always wait for X nanoseconds, right? Waiting for 5 clock cycles at 104MHz is obviously not the same as waiting for the same 5 clock cycles at 26MHz: in that case, expressing the value in nanoseconds or microseconds would make that independent from the controller's clock frequency as the calculation from `time` to `cycles` would be performed inside of the driver. Regards, Angelo
Hi Angelo, On Tue, 2022-12-06 at 13:19 +0100, AngeloGioacchino Del Regno wrote: > > > > diff --git > > > > a/Documentation/devicetree/bindings/spi/mediatek,spi- > > > > mtk-snfi.yaml > > > > b/Documentation/devicetree/bindings/spi/mediatek,spi- > > > > mtk-snfi.yaml > > > > index bab23f1b11fd..6e6ff8d73fcd 100644 > > > > --- a/Documentation/devicetree/bindings/spi/mediatek,spi-mtk- > > > > snfi.yaml > > > > +++ b/Documentation/devicetree/bindings/spi/mediatek,spi-mtk- > > > > snfi.yaml > > > > @@ -45,6 +45,13 @@ properties: > > > > description: device-tree node of the accompanying ECC > > > > engine. > > > > $ref: /schemas/types.yaml#/definitions/phandle > > > > > > > > + mediatek,rx-latch-latency: > > > > + description: Rx delay to sample data with this value, the > > > > value > > > > + unit is clock cycle. > > > > > > Can't we use nanoseconds or microseconds as a unit here, instead > > > of > > > clock cycles? > > > > The clock cycle will be various with MediaTek SPI NAND controller > > which > > clock frequency can support 26/52/68/81/104MHz... > > It`s may be easy to configure and understand with clock cycle in > > unit. > > > > Yes, but whatever clock frequency we use, the target is to always > wait for > X nanoseconds, right? > > Waiting for 5 clock cycles at 104MHz is obviously not the same as > waiting > for the same 5 clock cycles at 26MHz: in that case, expressing the > value > in nanoseconds or microseconds would make that independent from the > controller's clock frequency as the calculation from `time` to > `cycles` > would be performed inside of the driver. There have two rx related timing properties in spi-peripheral-props. The rx-sample-delay-ns have been used in Mediatek snfi driver to adjust controller sample delay. However another spi-rx-delay-us is in microseconds. Take 52MHz for example, the clock cycle will be 19.23ns which lower than 1us. This may not easy to by one clock cycle. Thanks Xiangsheng Hou
Il 07/12/22 03:00, Xiangsheng Hou (侯祥胜) ha scritto: > Hi Angelo, > > On Tue, 2022-12-06 at 13:19 +0100, AngeloGioacchino Del Regno wrote: >>>>> diff --git >>>>> a/Documentation/devicetree/bindings/spi/mediatek,spi- >>>>> mtk-snfi.yaml >>>>> b/Documentation/devicetree/bindings/spi/mediatek,spi- >>>>> mtk-snfi.yaml >>>>> index bab23f1b11fd..6e6ff8d73fcd 100644 >>>>> --- a/Documentation/devicetree/bindings/spi/mediatek,spi-mtk- >>>>> snfi.yaml >>>>> +++ b/Documentation/devicetree/bindings/spi/mediatek,spi-mtk- >>>>> snfi.yaml >>>>> @@ -45,6 +45,13 @@ properties: >>>>> description: device-tree node of the accompanying ECC >>>>> engine. >>>>> $ref: /schemas/types.yaml#/definitions/phandle >>>>> >>>>> + mediatek,rx-latch-latency: >>>>> + description: Rx delay to sample data with this value, the >>>>> value >>>>> + unit is clock cycle. >>>> >>>> Can't we use nanoseconds or microseconds as a unit here, instead >>>> of >>>> clock cycles? >>> >>> The clock cycle will be various with MediaTek SPI NAND controller >>> which >>> clock frequency can support 26/52/68/81/104MHz... >>> It`s may be easy to configure and understand with clock cycle in >>> unit. >>> >> >> Yes, but whatever clock frequency we use, the target is to always >> wait for >> X nanoseconds, right? >> >> Waiting for 5 clock cycles at 104MHz is obviously not the same as >> waiting >> for the same 5 clock cycles at 26MHz: in that case, expressing the >> value >> in nanoseconds or microseconds would make that independent from the >> controller's clock frequency as the calculation from `time` to >> `cycles` >> would be performed inside of the driver. > > There have two rx related timing properties in spi-peripheral-props. > The rx-sample-delay-ns have been used in Mediatek snfi driver to adjust > controller sample delay. > However another spi-rx-delay-us is in microseconds. Take 52MHz for > example, the clock cycle will be 19.23ns which lower than 1us. This may > not easy to by one clock cycle. > I agree, but nothing prevents you from adding your own property for that. I propose "mediatek,rx-latch-latency-ns" or "mediatek,rx-latency-ns", so that we can specify the delay in nanoseconds: in that case, when we specify 19ns, the driver will safely round that resulting in 52MHz == 19.23ns => 19ns valid. Regards, Angelo
On Wed, 2022-12-07 at 10:48 +0100, AngeloGioacchino Del Regno wrote: > Il 07/12/22 03:00, Xiangsheng Hou (侯祥胜) ha scritto: > > Hi Angelo, > > > > On Tue, 2022-12-06 at 13:19 +0100, AngeloGioacchino Del Regno > > wrote: > > > > > > diff --git > > > > > > a/Documentation/devicetree/bindings/spi/mediatek,spi- > > > > > > mtk-snfi.yaml > > > > > > b/Documentation/devicetree/bindings/spi/mediatek,spi- > > > > > > mtk-snfi.yaml > > > > > > index bab23f1b11fd..6e6ff8d73fcd 100644 > > > > > > --- a/Documentation/devicetree/bindings/spi/mediatek,spi- > > > > > > mtk- > > > > > > snfi.yaml > > > > > > +++ b/Documentation/devicetree/bindings/spi/mediatek,spi- > > > > > > mtk- > > > > > > snfi.yaml > > > > > > @@ -45,6 +45,13 @@ properties: > > > > > > description: device-tree node of the accompanying > > > > > > ECC > > > > > > engine. > > > > > > $ref: /schemas/types.yaml#/definitions/phandle > > > > > > > > > > > > + mediatek,rx-latch-latency: > > > > > > + description: Rx delay to sample data with this value, > > > > > > the > > > > > > value > > > > > > + unit is clock cycle. > > > > > > > > > > Can't we use nanoseconds or microseconds as a unit here, > > > > > instead > > > > > of > > > > > clock cycles? > > > > > > > > The clock cycle will be various with MediaTek SPI NAND > > > > controller > > > > which > > > > clock frequency can support 26/52/68/81/104MHz... > > > > It`s may be easy to configure and understand with clock cycle > > > > in > > > > unit. > > > > > > > > > > Yes, but whatever clock frequency we use, the target is to always > > > wait for > > > X nanoseconds, right? > > > > > > Waiting for 5 clock cycles at 104MHz is obviously not the same as > > > waiting > > > for the same 5 clock cycles at 26MHz: in that case, expressing > > > the > > > value > > > in nanoseconds or microseconds would make that independent from > > > the > > > controller's clock frequency as the calculation from `time` to > > > `cycles` > > > would be performed inside of the driver. > > > > There have two rx related timing properties in spi-peripheral- > > props. > > The rx-sample-delay-ns have been used in Mediatek snfi driver to > > adjust > > controller sample delay. > > However another spi-rx-delay-us is in microseconds. Take 52MHz for > > example, the clock cycle will be 19.23ns which lower than 1us. This > > may > > not easy to by one clock cycle. > > > > I agree, but nothing prevents you from adding your own property for > that. > > I propose "mediatek,rx-latch-latency-ns" or "mediatek,rx-latency-ns", > so that > we can specify the delay in nanoseconds: in that case, when we > specify 19ns, > the driver will safely round that resulting in 52MHz == 19.23ns => > 19ns valid. Will be fixed in next series. Thanks Xiangsheng Hou
On 08/12/2022 02:15, Xiangsheng Hou (侯祥胜) wrote: >>> >>> There have two rx related timing properties in spi-peripheral- >>> props. >>> The rx-sample-delay-ns have been used in Mediatek snfi driver to >>> adjust >>> controller sample delay. >>> However another spi-rx-delay-us is in microseconds. Take 52MHz for >>> example, the clock cycle will be 19.23ns which lower than 1us. This >>> may >>> not easy to by one clock cycle. >>> >> >> I agree, but nothing prevents you from adding your own property for >> that. >> >> I propose "mediatek,rx-latch-latency-ns" or "mediatek,rx-latency-ns", >> so that >> we can specify the delay in nanoseconds: in that case, when we >> specify 19ns, >> the driver will safely round that resulting in 52MHz == 19.23ns => >> 19ns valid. > > Will be fixed in next series. I am fine with this approach, but after explanations I was also fine with clock cycles as unit. It's still quite specific unit and I think several timings on buses are clock-cycle dependent, not time dependent. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-snfi.yaml b/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-snfi.yaml index bab23f1b11fd..6e6ff8d73fcd 100644 --- a/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-snfi.yaml +++ b/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-snfi.yaml @@ -45,6 +45,13 @@ properties: description: device-tree node of the accompanying ECC engine. $ref: /schemas/types.yaml#/definitions/phandle + mediatek,rx-latch-latency: + description: Rx delay to sample data with this value, the value + unit is clock cycle. + $ref: /schemas/types.yaml#/definitions/uint32 + enum: [0, 1, 2, 3] + default: 0 + required: - compatible - reg
Add mediatek,rx-latch-latency property which adjust read delay in the unit of clock cycle. Signed-off-by: Xiangsheng Hou <xiangsheng.hou@mediatek.com> --- .../devicetree/bindings/spi/mediatek,spi-mtk-snfi.yaml | 7 +++++++ 1 file changed, 7 insertions(+)