* [v4,0/4] introduce TI reset controller for MT8192 SoC @ 2020-08-17 3:03 ` Crystal Guo 0 siblings, 0 replies; 82+ messages in thread From: Crystal Guo @ 2020-08-17 3:03 UTC (permalink / raw) To: p.zabel, robh+dt, matthias.bgg Cc: srv_heupstream, linux-mediatek, linux-arm-kernel, linux-kernel, devicetree, s-anna, afd, seiya.wang, stanley.chu, yingjoe.chen, fan.chen, yong.liang v4: fix typos on v3 commit message. v3: 1. revert v2 changes. 2. add 'reset-duration-us' property to declare a minimum delay, which needs to be waited between assert and deassert. 3. add 'mediatek,infra-reset' to compatible. v2 changes: https://patchwork.kernel.org/patch/11697371/ 1. add 'assert-deassert-together' property to introduce a new reset handler, which allows device to do serialized assert and deassert operations in a single step by 'reset' method. 2. add 'update-force' property to introduce force-update method, which forces the write operation in case the read already happens to return the correct value. 3. add 'generic-reset' to compatible v1 changes: https://patchwork.kernel.org/patch/11690523/ https://patchwork.kernel.org/patch/11690527/ Crystal Guo (4): dt-binding: reset-controller: ti: add reset-duration-us property dt-binding: reset-controller: ti: add 'mediatek,infra-reset' to compatible reset-controller: ti: introduce a new reset handler arm64: dts: mt8192: add infracfg_rst node .../bindings/reset/ti-syscon-reset.txt | 6 +++++ arch/arm64/boot/dts/mediatek/mt8192.dtsi | 11 +++++++- drivers/reset/reset-ti-syscon.c | 26 +++++++++++++++++-- 3 files changed, 40 insertions(+), 3 deletions(-) ^ permalink raw reply [flat|nested] 82+ messages in thread
* [v4,0/4] introduce TI reset controller for MT8192 SoC @ 2020-08-17 3:03 ` Crystal Guo 0 siblings, 0 replies; 82+ messages in thread From: Crystal Guo @ 2020-08-17 3:03 UTC (permalink / raw) To: p.zabel, robh+dt, matthias.bgg Cc: devicetree, yong.liang, stanley.chu, srv_heupstream, seiya.wang, linux-kernel, afd, fan.chen, linux-mediatek, yingjoe.chen, linux-arm-kernel v4: fix typos on v3 commit message. v3: 1. revert v2 changes. 2. add 'reset-duration-us' property to declare a minimum delay, which needs to be waited between assert and deassert. 3. add 'mediatek,infra-reset' to compatible. v2 changes: https://patchwork.kernel.org/patch/11697371/ 1. add 'assert-deassert-together' property to introduce a new reset handler, which allows device to do serialized assert and deassert operations in a single step by 'reset' method. 2. add 'update-force' property to introduce force-update method, which forces the write operation in case the read already happens to return the correct value. 3. add 'generic-reset' to compatible v1 changes: https://patchwork.kernel.org/patch/11690523/ https://patchwork.kernel.org/patch/11690527/ Crystal Guo (4): dt-binding: reset-controller: ti: add reset-duration-us property dt-binding: reset-controller: ti: add 'mediatek,infra-reset' to compatible reset-controller: ti: introduce a new reset handler arm64: dts: mt8192: add infracfg_rst node .../bindings/reset/ti-syscon-reset.txt | 6 +++++ arch/arm64/boot/dts/mediatek/mt8192.dtsi | 11 +++++++- drivers/reset/reset-ti-syscon.c | 26 +++++++++++++++++-- 3 files changed, 40 insertions(+), 3 deletions(-) _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 82+ messages in thread
* [v4,0/4] introduce TI reset controller for MT8192 SoC @ 2020-08-17 3:03 ` Crystal Guo 0 siblings, 0 replies; 82+ messages in thread From: Crystal Guo @ 2020-08-17 3:03 UTC (permalink / raw) To: p.zabel, robh+dt, matthias.bgg Cc: devicetree, yong.liang, stanley.chu, srv_heupstream, seiya.wang, linux-kernel, afd, fan.chen, linux-mediatek, yingjoe.chen, s-anna, linux-arm-kernel v4: fix typos on v3 commit message. v3: 1. revert v2 changes. 2. add 'reset-duration-us' property to declare a minimum delay, which needs to be waited between assert and deassert. 3. add 'mediatek,infra-reset' to compatible. v2 changes: https://patchwork.kernel.org/patch/11697371/ 1. add 'assert-deassert-together' property to introduce a new reset handler, which allows device to do serialized assert and deassert operations in a single step by 'reset' method. 2. add 'update-force' property to introduce force-update method, which forces the write operation in case the read already happens to return the correct value. 3. add 'generic-reset' to compatible v1 changes: https://patchwork.kernel.org/patch/11690523/ https://patchwork.kernel.org/patch/11690527/ Crystal Guo (4): dt-binding: reset-controller: ti: add reset-duration-us property dt-binding: reset-controller: ti: add 'mediatek,infra-reset' to compatible reset-controller: ti: introduce a new reset handler arm64: dts: mt8192: add infracfg_rst node .../bindings/reset/ti-syscon-reset.txt | 6 +++++ arch/arm64/boot/dts/mediatek/mt8192.dtsi | 11 +++++++- drivers/reset/reset-ti-syscon.c | 26 +++++++++++++++++-- 3 files changed, 40 insertions(+), 3 deletions(-) _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek ^ permalink raw reply [flat|nested] 82+ messages in thread
* [v4,1/4] dt-binding: reset-controller: ti: add reset-duration-us property 2020-08-17 3:03 ` Crystal Guo (?) @ 2020-08-17 3:03 ` Crystal Guo -1 siblings, 0 replies; 82+ messages in thread From: Crystal Guo @ 2020-08-17 3:03 UTC (permalink / raw) To: p.zabel, robh+dt, matthias.bgg Cc: srv_heupstream, linux-mediatek, linux-arm-kernel, linux-kernel, devicetree, s-anna, afd, seiya.wang, stanley.chu, yingjoe.chen, fan.chen, yong.liang, Crystal Guo introduce 'reset' method to allow device do serialized assert and deassert operations in a single step, which needs a minimum delay to be waited between assert and deassert. Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> --- Documentation/devicetree/bindings/reset/ti-syscon-reset.txt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt index 86945502ccb5..ab041032339b 100644 --- a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt +++ b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt @@ -59,6 +59,11 @@ Required properties: Please also refer to Documentation/devicetree/bindings/reset/reset.txt for common reset controller usage by consumers. +Optional properties: +- reset-duration-us: When do serialized assert and deassert operations, minimum delay in microseconds +is needed to be waited between an assert and a deassert to reset the device. This value can be 0, 0 means +that such a delay is not needed. + Example: -------- The following example demonstrates a syscon node, the reset controller node -- 2.18.0 ^ permalink raw reply related [flat|nested] 82+ messages in thread
* [v4, 1/4] dt-binding: reset-controller: ti: add reset-duration-us property @ 2020-08-17 3:03 ` Crystal Guo 0 siblings, 0 replies; 82+ messages in thread From: Crystal Guo @ 2020-08-17 3:03 UTC (permalink / raw) To: p.zabel, robh+dt, matthias.bgg Cc: devicetree, yong.liang, stanley.chu, srv_heupstream, seiya.wang, linux-kernel, afd, fan.chen, Crystal Guo, linux-mediatek, yingjoe.chen, linux-arm-kernel introduce 'reset' method to allow device do serialized assert and deassert operations in a single step, which needs a minimum delay to be waited between assert and deassert. Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> --- Documentation/devicetree/bindings/reset/ti-syscon-reset.txt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt index 86945502ccb5..ab041032339b 100644 --- a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt +++ b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt @@ -59,6 +59,11 @@ Required properties: Please also refer to Documentation/devicetree/bindings/reset/reset.txt for common reset controller usage by consumers. +Optional properties: +- reset-duration-us: When do serialized assert and deassert operations, minimum delay in microseconds +is needed to be waited between an assert and a deassert to reset the device. This value can be 0, 0 means +that such a delay is not needed. + Example: -------- The following example demonstrates a syscon node, the reset controller node -- 2.18.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 82+ messages in thread
* [v4, 1/4] dt-binding: reset-controller: ti: add reset-duration-us property @ 2020-08-17 3:03 ` Crystal Guo 0 siblings, 0 replies; 82+ messages in thread From: Crystal Guo @ 2020-08-17 3:03 UTC (permalink / raw) To: p.zabel, robh+dt, matthias.bgg Cc: devicetree, yong.liang, stanley.chu, srv_heupstream, seiya.wang, linux-kernel, afd, fan.chen, Crystal Guo, linux-mediatek, yingjoe.chen, s-anna, linux-arm-kernel introduce 'reset' method to allow device do serialized assert and deassert operations in a single step, which needs a minimum delay to be waited between assert and deassert. Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> --- Documentation/devicetree/bindings/reset/ti-syscon-reset.txt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt index 86945502ccb5..ab041032339b 100644 --- a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt +++ b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt @@ -59,6 +59,11 @@ Required properties: Please also refer to Documentation/devicetree/bindings/reset/reset.txt for common reset controller usage by consumers. +Optional properties: +- reset-duration-us: When do serialized assert and deassert operations, minimum delay in microseconds +is needed to be waited between an assert and a deassert to reset the device. This value can be 0, 0 means +that such a delay is not needed. + Example: -------- The following example demonstrates a syscon node, the reset controller node -- 2.18.0 _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek ^ permalink raw reply related [flat|nested] 82+ messages in thread
* Re: [v4,1/4] dt-binding: reset-controller: ti: add reset-duration-us property 2020-08-17 3:03 ` Crystal Guo (?) @ 2020-08-25 17:42 ` Rob Herring -1 siblings, 0 replies; 82+ messages in thread From: Rob Herring @ 2020-08-25 17:42 UTC (permalink / raw) To: Crystal Guo Cc: p.zabel, matthias.bgg, srv_heupstream, linux-mediatek, linux-arm-kernel, linux-kernel, devicetree, s-anna, afd, seiya.wang, stanley.chu, yingjoe.chen, fan.chen, yong.liang On Mon, Aug 17, 2020 at 11:03:21AM +0800, Crystal Guo wrote: > introduce 'reset' method to allow device do serialized assert and > deassert operations in a single step, which needs a minimum delay > to be waited between assert and deassert. Why is Mediatek adding to a TI binding? > > Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> > --- > Documentation/devicetree/bindings/reset/ti-syscon-reset.txt | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > index 86945502ccb5..ab041032339b 100644 > --- a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > +++ b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > @@ -59,6 +59,11 @@ Required properties: > Please also refer to Documentation/devicetree/bindings/reset/reset.txt for > common reset controller usage by consumers. > > +Optional properties: > +- reset-duration-us: When do serialized assert and deassert operations, minimum delay in microseconds > +is needed to be waited between an assert and a deassert to reset the device. This value can be 0, 0 means > +that such a delay is not needed. This goes in the reset controller node or each consumer? For the latter, it should be a cell in 'resets' if you need this. But really, I think the reset controller should enforce some minimum time that works for all consumers. Surely having a minimum time per reset isn't really needed. Rob ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4, 1/4] dt-binding: reset-controller: ti: add reset-duration-us property @ 2020-08-25 17:42 ` Rob Herring 0 siblings, 0 replies; 82+ messages in thread From: Rob Herring @ 2020-08-25 17:42 UTC (permalink / raw) To: Crystal Guo Cc: devicetree, yong.liang, stanley.chu, srv_heupstream, seiya.wang, linux-kernel, afd, fan.chen, linux-mediatek, p.zabel, matthias.bgg, yingjoe.chen, linux-arm-kernel On Mon, Aug 17, 2020 at 11:03:21AM +0800, Crystal Guo wrote: > introduce 'reset' method to allow device do serialized assert and > deassert operations in a single step, which needs a minimum delay > to be waited between assert and deassert. Why is Mediatek adding to a TI binding? > > Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> > --- > Documentation/devicetree/bindings/reset/ti-syscon-reset.txt | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > index 86945502ccb5..ab041032339b 100644 > --- a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > +++ b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > @@ -59,6 +59,11 @@ Required properties: > Please also refer to Documentation/devicetree/bindings/reset/reset.txt for > common reset controller usage by consumers. > > +Optional properties: > +- reset-duration-us: When do serialized assert and deassert operations, minimum delay in microseconds > +is needed to be waited between an assert and a deassert to reset the device. This value can be 0, 0 means > +that such a delay is not needed. This goes in the reset controller node or each consumer? For the latter, it should be a cell in 'resets' if you need this. But really, I think the reset controller should enforce some minimum time that works for all consumers. Surely having a minimum time per reset isn't really needed. Rob _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4, 1/4] dt-binding: reset-controller: ti: add reset-duration-us property @ 2020-08-25 17:42 ` Rob Herring 0 siblings, 0 replies; 82+ messages in thread From: Rob Herring @ 2020-08-25 17:42 UTC (permalink / raw) To: Crystal Guo Cc: devicetree, yong.liang, stanley.chu, srv_heupstream, seiya.wang, linux-kernel, afd, fan.chen, linux-mediatek, p.zabel, matthias.bgg, yingjoe.chen, s-anna, linux-arm-kernel On Mon, Aug 17, 2020 at 11:03:21AM +0800, Crystal Guo wrote: > introduce 'reset' method to allow device do serialized assert and > deassert operations in a single step, which needs a minimum delay > to be waited between assert and deassert. Why is Mediatek adding to a TI binding? > > Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> > --- > Documentation/devicetree/bindings/reset/ti-syscon-reset.txt | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > index 86945502ccb5..ab041032339b 100644 > --- a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > +++ b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > @@ -59,6 +59,11 @@ Required properties: > Please also refer to Documentation/devicetree/bindings/reset/reset.txt for > common reset controller usage by consumers. > > +Optional properties: > +- reset-duration-us: When do serialized assert and deassert operations, minimum delay in microseconds > +is needed to be waited between an assert and a deassert to reset the device. This value can be 0, 0 means > +that such a delay is not needed. This goes in the reset controller node or each consumer? For the latter, it should be a cell in 'resets' if you need this. But really, I think the reset controller should enforce some minimum time that works for all consumers. Surely having a minimum time per reset isn't really needed. Rob _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,1/4] dt-binding: reset-controller: ti: add reset-duration-us property 2020-08-25 17:42 ` Rob Herring (?) @ 2020-08-26 11:09 ` Crystal Guo -1 siblings, 0 replies; 82+ messages in thread From: Crystal Guo @ 2020-08-26 11:09 UTC (permalink / raw) To: Rob Herring Cc: p.zabel, matthias.bgg, srv_heupstream, linux-mediatek, linux-arm-kernel, linux-kernel, devicetree, s-anna, afd, Seiya Wang (王迺君), Stanley Chu (朱原陞), Yingjoe Chen (陳英洲), Fan Chen (陳凡), Yong Liang (梁勇) On Wed, 2020-08-26 at 01:42 +0800, Rob Herring wrote: > On Mon, Aug 17, 2020 at 11:03:21AM +0800, Crystal Guo wrote: > > introduce 'reset' method to allow device do serialized assert and > > deassert operations in a single step, which needs a minimum delay > > to be waited between assert and deassert. > > Why is Mediatek adding to a TI binding? TI reset-controller provides a common reset management, and is suitable for Mediatek SoCs, thus Mediatek wants to reuse this driver for reset. > > > > > Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> > > --- > > Documentation/devicetree/bindings/reset/ti-syscon-reset.txt | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > > index 86945502ccb5..ab041032339b 100644 > > --- a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > > +++ b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > > @@ -59,6 +59,11 @@ Required properties: > > Please also refer to Documentation/devicetree/bindings/reset/reset.txt for > > common reset controller usage by consumers. > > > > +Optional properties: > > +- reset-duration-us: When do serialized assert and deassert operations, minimum delay in microseconds > > +is needed to be waited between an assert and a deassert to reset the device. This value can be 0, 0 means > > +that such a delay is not needed. > > This goes in the reset controller node or each consumer? For the latter, > it should be a cell in 'resets' if you need this. But really, I think > the reset controller should enforce some minimum time that works for all > consumers. Surely having a minimum time per reset isn't really needed. > > Rob 'reset-duration-us' will be in the reset controller node, and it's optional. If minimum delay is needed to be waited between an assert and a deassert to reset the device, this property will be set.Otherwise no need to set this property. Best Regards Crystal ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,1/4] dt-binding: reset-controller: ti: add reset-duration-us property @ 2020-08-26 11:09 ` Crystal Guo 0 siblings, 0 replies; 82+ messages in thread From: Crystal Guo @ 2020-08-26 11:09 UTC (permalink / raw) To: Rob Herring Cc: devicetree, Yong Liang (梁勇), Stanley Chu (朱原陞), srv_heupstream, Seiya Wang (王迺君), linux-kernel, afd, Fan Chen (陳凡), linux-mediatek, p.zabel, matthias.bgg, Yingjoe Chen (陳英洲), linux-arm-kernel On Wed, 2020-08-26 at 01:42 +0800, Rob Herring wrote: > On Mon, Aug 17, 2020 at 11:03:21AM +0800, Crystal Guo wrote: > > introduce 'reset' method to allow device do serialized assert and > > deassert operations in a single step, which needs a minimum delay > > to be waited between assert and deassert. > > Why is Mediatek adding to a TI binding? TI reset-controller provides a common reset management, and is suitable for Mediatek SoCs, thus Mediatek wants to reuse this driver for reset. > > > > > Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> > > --- > > Documentation/devicetree/bindings/reset/ti-syscon-reset.txt | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > > index 86945502ccb5..ab041032339b 100644 > > --- a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > > +++ b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > > @@ -59,6 +59,11 @@ Required properties: > > Please also refer to Documentation/devicetree/bindings/reset/reset.txt for > > common reset controller usage by consumers. > > > > +Optional properties: > > +- reset-duration-us: When do serialized assert and deassert operations, minimum delay in microseconds > > +is needed to be waited between an assert and a deassert to reset the device. This value can be 0, 0 means > > +that such a delay is not needed. > > This goes in the reset controller node or each consumer? For the latter, > it should be a cell in 'resets' if you need this. But really, I think > the reset controller should enforce some minimum time that works for all > consumers. Surely having a minimum time per reset isn't really needed. > > Rob 'reset-duration-us' will be in the reset controller node, and it's optional. If minimum delay is needed to be waited between an assert and a deassert to reset the device, this property will be set.Otherwise no need to set this property. Best Regards Crystal _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,1/4] dt-binding: reset-controller: ti: add reset-duration-us property @ 2020-08-26 11:09 ` Crystal Guo 0 siblings, 0 replies; 82+ messages in thread From: Crystal Guo @ 2020-08-26 11:09 UTC (permalink / raw) To: Rob Herring Cc: devicetree, Yong Liang (梁勇), Stanley Chu (朱原陞), srv_heupstream, Seiya Wang (王迺君), linux-kernel, afd, Fan Chen (陳凡), linux-mediatek, p.zabel, matthias.bgg, Yingjoe Chen (陳英洲), s-anna, linux-arm-kernel On Wed, 2020-08-26 at 01:42 +0800, Rob Herring wrote: > On Mon, Aug 17, 2020 at 11:03:21AM +0800, Crystal Guo wrote: > > introduce 'reset' method to allow device do serialized assert and > > deassert operations in a single step, which needs a minimum delay > > to be waited between assert and deassert. > > Why is Mediatek adding to a TI binding? TI reset-controller provides a common reset management, and is suitable for Mediatek SoCs, thus Mediatek wants to reuse this driver for reset. > > > > > Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> > > --- > > Documentation/devicetree/bindings/reset/ti-syscon-reset.txt | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > > index 86945502ccb5..ab041032339b 100644 > > --- a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > > +++ b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > > @@ -59,6 +59,11 @@ Required properties: > > Please also refer to Documentation/devicetree/bindings/reset/reset.txt for > > common reset controller usage by consumers. > > > > +Optional properties: > > +- reset-duration-us: When do serialized assert and deassert operations, minimum delay in microseconds > > +is needed to be waited between an assert and a deassert to reset the device. This value can be 0, 0 means > > +that such a delay is not needed. > > This goes in the reset controller node or each consumer? For the latter, > it should be a cell in 'resets' if you need this. But really, I think > the reset controller should enforce some minimum time that works for all > consumers. Surely having a minimum time per reset isn't really needed. > > Rob 'reset-duration-us' will be in the reset controller node, and it's optional. If minimum delay is needed to be waited between an assert and a deassert to reset the device, this property will be set.Otherwise no need to set this property. Best Regards Crystal _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek ^ permalink raw reply [flat|nested] 82+ messages in thread
* [v4,2/4] dt-binding: reset-controller: ti: add 'mediatek,infra-reset' to compatible 2020-08-17 3:03 ` Crystal Guo (?) @ 2020-08-17 3:03 ` Crystal Guo -1 siblings, 0 replies; 82+ messages in thread From: Crystal Guo @ 2020-08-17 3:03 UTC (permalink / raw) To: p.zabel, robh+dt, matthias.bgg Cc: srv_heupstream, linux-mediatek, linux-arm-kernel, linux-kernel, devicetree, s-anna, afd, seiya.wang, stanley.chu, yingjoe.chen, fan.chen, yong.liang, Crystal Guo The TI syscon reset controller provides a common reset management, and is suitable for MTK SoCs. Add compatible 'mediatek,infra-reset', which denotes to use ti reset-controller driver directly. Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> --- Documentation/devicetree/bindings/reset/ti-syscon-reset.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt index ab041032339b..5a0e9365b51b 100644 --- a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt +++ b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt @@ -25,6 +25,7 @@ Required properties: "ti,k2l-pscrst" "ti,k2hk-pscrst" "ti,syscon-reset" + "mediatek,infra-reset", "ti,syscon-reset" - #reset-cells : Should be 1. Please see the reset consumer node below for usage details - ti,reset-bits : Contains the reset control register information -- 2.18.0 ^ permalink raw reply related [flat|nested] 82+ messages in thread
* [v4, 2/4] dt-binding: reset-controller: ti: add 'mediatek, infra-reset' to compatible @ 2020-08-17 3:03 ` Crystal Guo 0 siblings, 0 replies; 82+ messages in thread From: Crystal Guo @ 2020-08-17 3:03 UTC (permalink / raw) To: p.zabel, robh+dt, matthias.bgg Cc: devicetree, yong.liang, stanley.chu, srv_heupstream, seiya.wang, linux-kernel, afd, fan.chen, Crystal Guo, linux-mediatek, yingjoe.chen, linux-arm-kernel The TI syscon reset controller provides a common reset management, and is suitable for MTK SoCs. Add compatible 'mediatek,infra-reset', which denotes to use ti reset-controller driver directly. Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> --- Documentation/devicetree/bindings/reset/ti-syscon-reset.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt index ab041032339b..5a0e9365b51b 100644 --- a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt +++ b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt @@ -25,6 +25,7 @@ Required properties: "ti,k2l-pscrst" "ti,k2hk-pscrst" "ti,syscon-reset" + "mediatek,infra-reset", "ti,syscon-reset" - #reset-cells : Should be 1. Please see the reset consumer node below for usage details - ti,reset-bits : Contains the reset control register information -- 2.18.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 82+ messages in thread
* [v4, 2/4] dt-binding: reset-controller: ti: add 'mediatek, infra-reset' to compatible @ 2020-08-17 3:03 ` Crystal Guo 0 siblings, 0 replies; 82+ messages in thread From: Crystal Guo @ 2020-08-17 3:03 UTC (permalink / raw) To: p.zabel, robh+dt, matthias.bgg Cc: devicetree, yong.liang, stanley.chu, srv_heupstream, seiya.wang, linux-kernel, afd, fan.chen, Crystal Guo, linux-mediatek, yingjoe.chen, s-anna, linux-arm-kernel The TI syscon reset controller provides a common reset management, and is suitable for MTK SoCs. Add compatible 'mediatek,infra-reset', which denotes to use ti reset-controller driver directly. Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> --- Documentation/devicetree/bindings/reset/ti-syscon-reset.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt index ab041032339b..5a0e9365b51b 100644 --- a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt +++ b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt @@ -25,6 +25,7 @@ Required properties: "ti,k2l-pscrst" "ti,k2hk-pscrst" "ti,syscon-reset" + "mediatek,infra-reset", "ti,syscon-reset" - #reset-cells : Should be 1. Please see the reset consumer node below for usage details - ti,reset-bits : Contains the reset control register information -- 2.18.0 _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek ^ permalink raw reply related [flat|nested] 82+ messages in thread
* Re: [v4,2/4] dt-binding: reset-controller: ti: add 'mediatek,infra-reset' to compatible 2020-08-17 3:03 ` Crystal Guo (?) @ 2020-08-25 19:02 ` Rob Herring -1 siblings, 0 replies; 82+ messages in thread From: Rob Herring @ 2020-08-25 19:02 UTC (permalink / raw) To: Crystal Guo Cc: p.zabel, matthias.bgg, srv_heupstream, linux-mediatek, linux-arm-kernel, linux-kernel, devicetree, s-anna, afd, seiya.wang, stanley.chu, yingjoe.chen, fan.chen, yong.liang On Mon, Aug 17, 2020 at 11:03:22AM +0800, Crystal Guo wrote: > The TI syscon reset controller provides a common reset management, > and is suitable for MTK SoCs. Add compatible 'mediatek,infra-reset', > which denotes to use ti reset-controller driver directly. > > Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> > --- > Documentation/devicetree/bindings/reset/ti-syscon-reset.txt | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > index ab041032339b..5a0e9365b51b 100644 > --- a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > +++ b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > @@ -25,6 +25,7 @@ Required properties: > "ti,k2l-pscrst" > "ti,k2hk-pscrst" > "ti,syscon-reset" > + "mediatek,infra-reset", "ti,syscon-reset" You need your own binding doc. If you can use the same driver then fine, but that's a separate issue. There's also reset-simple driver if you have just array of 32-bit registers with a bit per reset. Don't repeat 'ti,reset-bits' either. > - #reset-cells : Should be 1. Please see the reset consumer node below > for usage details > - ti,reset-bits : Contains the reset control register information > -- > 2.18.0 ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,2/4] dt-binding: reset-controller: ti: add 'mediatek,infra-reset' to compatible @ 2020-08-25 19:02 ` Rob Herring 0 siblings, 0 replies; 82+ messages in thread From: Rob Herring @ 2020-08-25 19:02 UTC (permalink / raw) To: Crystal Guo Cc: devicetree, yong.liang, stanley.chu, srv_heupstream, seiya.wang, linux-kernel, afd, fan.chen, linux-mediatek, p.zabel, matthias.bgg, yingjoe.chen, linux-arm-kernel On Mon, Aug 17, 2020 at 11:03:22AM +0800, Crystal Guo wrote: > The TI syscon reset controller provides a common reset management, > and is suitable for MTK SoCs. Add compatible 'mediatek,infra-reset', > which denotes to use ti reset-controller driver directly. > > Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> > --- > Documentation/devicetree/bindings/reset/ti-syscon-reset.txt | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > index ab041032339b..5a0e9365b51b 100644 > --- a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > +++ b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > @@ -25,6 +25,7 @@ Required properties: > "ti,k2l-pscrst" > "ti,k2hk-pscrst" > "ti,syscon-reset" > + "mediatek,infra-reset", "ti,syscon-reset" You need your own binding doc. If you can use the same driver then fine, but that's a separate issue. There's also reset-simple driver if you have just array of 32-bit registers with a bit per reset. Don't repeat 'ti,reset-bits' either. > - #reset-cells : Should be 1. Please see the reset consumer node below > for usage details > - ti,reset-bits : Contains the reset control register information > -- > 2.18.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,2/4] dt-binding: reset-controller: ti: add 'mediatek,infra-reset' to compatible @ 2020-08-25 19:02 ` Rob Herring 0 siblings, 0 replies; 82+ messages in thread From: Rob Herring @ 2020-08-25 19:02 UTC (permalink / raw) To: Crystal Guo Cc: devicetree, yong.liang, stanley.chu, srv_heupstream, seiya.wang, linux-kernel, afd, fan.chen, linux-mediatek, p.zabel, matthias.bgg, yingjoe.chen, s-anna, linux-arm-kernel On Mon, Aug 17, 2020 at 11:03:22AM +0800, Crystal Guo wrote: > The TI syscon reset controller provides a common reset management, > and is suitable for MTK SoCs. Add compatible 'mediatek,infra-reset', > which denotes to use ti reset-controller driver directly. > > Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> > --- > Documentation/devicetree/bindings/reset/ti-syscon-reset.txt | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > index ab041032339b..5a0e9365b51b 100644 > --- a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > +++ b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > @@ -25,6 +25,7 @@ Required properties: > "ti,k2l-pscrst" > "ti,k2hk-pscrst" > "ti,syscon-reset" > + "mediatek,infra-reset", "ti,syscon-reset" You need your own binding doc. If you can use the same driver then fine, but that's a separate issue. There's also reset-simple driver if you have just array of 32-bit registers with a bit per reset. Don't repeat 'ti,reset-bits' either. > - #reset-cells : Should be 1. Please see the reset consumer node below > for usage details > - ti,reset-bits : Contains the reset control register information > -- > 2.18.0 _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,2/4] dt-binding: reset-controller: ti: add 'mediatek,infra-reset' to compatible 2020-08-25 19:02 ` Rob Herring (?) @ 2020-08-26 11:09 ` Crystal Guo -1 siblings, 0 replies; 82+ messages in thread From: Crystal Guo @ 2020-08-26 11:09 UTC (permalink / raw) To: Rob Herring Cc: p.zabel, matthias.bgg, srv_heupstream, linux-mediatek, linux-arm-kernel, linux-kernel, devicetree, s-anna, afd, Seiya Wang (王迺君), Stanley Chu (朱原陞), Yingjoe Chen (陳英洲), Fan Chen (陳凡), Yong Liang (梁勇) On Wed, 2020-08-26 at 03:02 +0800, Rob Herring wrote: > On Mon, Aug 17, 2020 at 11:03:22AM +0800, Crystal Guo wrote: > > The TI syscon reset controller provides a common reset management, > > and is suitable for MTK SoCs. Add compatible 'mediatek,infra-reset', > > which denotes to use ti reset-controller driver directly. > > > > Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> > > --- > > Documentation/devicetree/bindings/reset/ti-syscon-reset.txt | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > > index ab041032339b..5a0e9365b51b 100644 > > --- a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > > +++ b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > > @@ -25,6 +25,7 @@ Required properties: > > "ti,k2l-pscrst" > > "ti,k2hk-pscrst" > > "ti,syscon-reset" > > + "mediatek,infra-reset", "ti,syscon-reset" > > You need your own binding doc. If you can use the same driver then fine, > but that's a separate issue. There's also reset-simple driver if you > have just array of 32-bit registers with a bit per reset. > > Don't repeat 'ti,reset-bits' either. Do you mean I should add a Mediatek reset binding doc, although Mediatek reuse the TI reset controller directly? Best Regards Crystal > > > - #reset-cells : Should be 1. Please see the reset consumer node below > > for usage details > > - ti,reset-bits : Contains the reset control register information > > -- > > 2.18.0 ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,2/4] dt-binding: reset-controller: ti: add 'mediatek,infra-reset' to compatible @ 2020-08-26 11:09 ` Crystal Guo 0 siblings, 0 replies; 82+ messages in thread From: Crystal Guo @ 2020-08-26 11:09 UTC (permalink / raw) To: Rob Herring Cc: devicetree, Yong Liang (梁勇), Stanley Chu (朱原陞), srv_heupstream, Seiya Wang (王迺君), linux-kernel, afd, Fan Chen (陳凡), linux-mediatek, p.zabel, matthias.bgg, Yingjoe Chen (陳英洲), linux-arm-kernel On Wed, 2020-08-26 at 03:02 +0800, Rob Herring wrote: > On Mon, Aug 17, 2020 at 11:03:22AM +0800, Crystal Guo wrote: > > The TI syscon reset controller provides a common reset management, > > and is suitable for MTK SoCs. Add compatible 'mediatek,infra-reset', > > which denotes to use ti reset-controller driver directly. > > > > Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> > > --- > > Documentation/devicetree/bindings/reset/ti-syscon-reset.txt | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > > index ab041032339b..5a0e9365b51b 100644 > > --- a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > > +++ b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > > @@ -25,6 +25,7 @@ Required properties: > > "ti,k2l-pscrst" > > "ti,k2hk-pscrst" > > "ti,syscon-reset" > > + "mediatek,infra-reset", "ti,syscon-reset" > > You need your own binding doc. If you can use the same driver then fine, > but that's a separate issue. There's also reset-simple driver if you > have just array of 32-bit registers with a bit per reset. > > Don't repeat 'ti,reset-bits' either. Do you mean I should add a Mediatek reset binding doc, although Mediatek reuse the TI reset controller directly? Best Regards Crystal > > > - #reset-cells : Should be 1. Please see the reset consumer node below > > for usage details > > - ti,reset-bits : Contains the reset control register information > > -- > > 2.18.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,2/4] dt-binding: reset-controller: ti: add 'mediatek,infra-reset' to compatible @ 2020-08-26 11:09 ` Crystal Guo 0 siblings, 0 replies; 82+ messages in thread From: Crystal Guo @ 2020-08-26 11:09 UTC (permalink / raw) To: Rob Herring Cc: devicetree, Yong Liang (梁勇), Stanley Chu (朱原陞), srv_heupstream, Seiya Wang (王迺君), linux-kernel, afd, Fan Chen (陳凡), linux-mediatek, p.zabel, matthias.bgg, Yingjoe Chen (陳英洲), s-anna, linux-arm-kernel On Wed, 2020-08-26 at 03:02 +0800, Rob Herring wrote: > On Mon, Aug 17, 2020 at 11:03:22AM +0800, Crystal Guo wrote: > > The TI syscon reset controller provides a common reset management, > > and is suitable for MTK SoCs. Add compatible 'mediatek,infra-reset', > > which denotes to use ti reset-controller driver directly. > > > > Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> > > --- > > Documentation/devicetree/bindings/reset/ti-syscon-reset.txt | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > > index ab041032339b..5a0e9365b51b 100644 > > --- a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > > +++ b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > > @@ -25,6 +25,7 @@ Required properties: > > "ti,k2l-pscrst" > > "ti,k2hk-pscrst" > > "ti,syscon-reset" > > + "mediatek,infra-reset", "ti,syscon-reset" > > You need your own binding doc. If you can use the same driver then fine, > but that's a separate issue. There's also reset-simple driver if you > have just array of 32-bit registers with a bit per reset. > > Don't repeat 'ti,reset-bits' either. Do you mean I should add a Mediatek reset binding doc, although Mediatek reuse the TI reset controller directly? Best Regards Crystal > > > - #reset-cells : Should be 1. Please see the reset consumer node below > > for usage details > > - ti,reset-bits : Contains the reset control register information > > -- > > 2.18.0 _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,2/4] dt-binding: reset-controller: ti: add 'mediatek,infra-reset' to compatible 2020-08-26 11:09 ` Crystal Guo (?) @ 2020-09-02 23:25 ` Suman Anna -1 siblings, 0 replies; 82+ messages in thread From: Suman Anna @ 2020-09-02 23:25 UTC (permalink / raw) To: Crystal Guo, Rob Herring Cc: p.zabel, matthias.bgg, srv_heupstream, linux-mediatek, linux-arm-kernel, linux-kernel, devicetree, Seiya Wang (王迺君), Stanley Chu (朱原陞), Yingjoe Chen (陳英洲), Fan Chen (陳凡), Yong Liang (梁勇) Hi Rob, On 8/26/20 6:09 AM, Crystal Guo wrote: > On Wed, 2020-08-26 at 03:02 +0800, Rob Herring wrote: >> On Mon, Aug 17, 2020 at 11:03:22AM +0800, Crystal Guo wrote: >>> The TI syscon reset controller provides a common reset management, >>> and is suitable for MTK SoCs. Add compatible 'mediatek,infra-reset', >>> which denotes to use ti reset-controller driver directly. >>> >>> Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> >>> --- >>> Documentation/devicetree/bindings/reset/ti-syscon-reset.txt | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt >>> index ab041032339b..5a0e9365b51b 100644 >>> --- a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt >>> +++ b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt >>> @@ -25,6 +25,7 @@ Required properties: >>> "ti,k2l-pscrst" >>> "ti,k2hk-pscrst" >>> "ti,syscon-reset" >>> + "mediatek,infra-reset", "ti,syscon-reset" >> >> You need your own binding doc. If you can use the same driver then fine, >> but that's a separate issue. There's also reset-simple driver if you >> have just array of 32-bit registers with a bit per reset. >> >> Don't repeat 'ti,reset-bits' either. > > Do you mean I should add a Mediatek reset binding doc, although Mediatek > reuse the TI reset controller directly? Hmm, how do you envision not repeating the same bits in a separate binding? Does it help if I convert this to YAML first without a ti, prefix in the file name? The usage philosophy definitely was to use a <soc-compatible> followed by the <generic-compatible>. This is how all of our reset nodes were added as well. Looks like Andrew may have misinterpreted your comment [1] during the original binding and changed "syscon-reset" to "ti,syscon-reset" in the final version [2]. regards Suman [1] https://lore.kernel.org/patchwork/comment/876688/ [2] https://lore.kernel.org/patchwork/patch/693172/ > > Best Regards > Crystal >> >>> - #reset-cells : Should be 1. Please see the reset consumer node below >>> for usage details >>> - ti,reset-bits : Contains the reset control register information >>> -- >>> 2.18.0 > ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,2/4] dt-binding: reset-controller: ti: add 'mediatek,infra-reset' to compatible @ 2020-09-02 23:25 ` Suman Anna 0 siblings, 0 replies; 82+ messages in thread From: Suman Anna @ 2020-09-02 23:25 UTC (permalink / raw) To: Crystal Guo, Rob Herring Cc: devicetree, Yong Liang (梁勇), srv_heupstream, Seiya Wang (王迺君), linux-kernel, Fan Chen (陳凡), linux-mediatek, p.zabel, matthias.bgg, Yingjoe Chen (陳英洲), Stanley Chu (朱原陞), linux-arm-kernel Hi Rob, On 8/26/20 6:09 AM, Crystal Guo wrote: > On Wed, 2020-08-26 at 03:02 +0800, Rob Herring wrote: >> On Mon, Aug 17, 2020 at 11:03:22AM +0800, Crystal Guo wrote: >>> The TI syscon reset controller provides a common reset management, >>> and is suitable for MTK SoCs. Add compatible 'mediatek,infra-reset', >>> which denotes to use ti reset-controller driver directly. >>> >>> Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> >>> --- >>> Documentation/devicetree/bindings/reset/ti-syscon-reset.txt | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt >>> index ab041032339b..5a0e9365b51b 100644 >>> --- a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt >>> +++ b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt >>> @@ -25,6 +25,7 @@ Required properties: >>> "ti,k2l-pscrst" >>> "ti,k2hk-pscrst" >>> "ti,syscon-reset" >>> + "mediatek,infra-reset", "ti,syscon-reset" >> >> You need your own binding doc. If you can use the same driver then fine, >> but that's a separate issue. There's also reset-simple driver if you >> have just array of 32-bit registers with a bit per reset. >> >> Don't repeat 'ti,reset-bits' either. > > Do you mean I should add a Mediatek reset binding doc, although Mediatek > reuse the TI reset controller directly? Hmm, how do you envision not repeating the same bits in a separate binding? Does it help if I convert this to YAML first without a ti, prefix in the file name? The usage philosophy definitely was to use a <soc-compatible> followed by the <generic-compatible>. This is how all of our reset nodes were added as well. Looks like Andrew may have misinterpreted your comment [1] during the original binding and changed "syscon-reset" to "ti,syscon-reset" in the final version [2]. regards Suman [1] https://lore.kernel.org/patchwork/comment/876688/ [2] https://lore.kernel.org/patchwork/patch/693172/ > > Best Regards > Crystal >> >>> - #reset-cells : Should be 1. Please see the reset consumer node below >>> for usage details >>> - ti,reset-bits : Contains the reset control register information >>> -- >>> 2.18.0 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,2/4] dt-binding: reset-controller: ti: add 'mediatek,infra-reset' to compatible @ 2020-09-02 23:25 ` Suman Anna 0 siblings, 0 replies; 82+ messages in thread From: Suman Anna @ 2020-09-02 23:25 UTC (permalink / raw) To: Crystal Guo, Rob Herring Cc: devicetree, Yong Liang (梁勇), srv_heupstream, Seiya Wang (王迺君), linux-kernel, Fan Chen (陳凡), linux-mediatek, p.zabel, matthias.bgg, Yingjoe Chen (陳英洲), Stanley Chu (朱原陞), linux-arm-kernel Hi Rob, On 8/26/20 6:09 AM, Crystal Guo wrote: > On Wed, 2020-08-26 at 03:02 +0800, Rob Herring wrote: >> On Mon, Aug 17, 2020 at 11:03:22AM +0800, Crystal Guo wrote: >>> The TI syscon reset controller provides a common reset management, >>> and is suitable for MTK SoCs. Add compatible 'mediatek,infra-reset', >>> which denotes to use ti reset-controller driver directly. >>> >>> Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> >>> --- >>> Documentation/devicetree/bindings/reset/ti-syscon-reset.txt | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt >>> index ab041032339b..5a0e9365b51b 100644 >>> --- a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt >>> +++ b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt >>> @@ -25,6 +25,7 @@ Required properties: >>> "ti,k2l-pscrst" >>> "ti,k2hk-pscrst" >>> "ti,syscon-reset" >>> + "mediatek,infra-reset", "ti,syscon-reset" >> >> You need your own binding doc. If you can use the same driver then fine, >> but that's a separate issue. There's also reset-simple driver if you >> have just array of 32-bit registers with a bit per reset. >> >> Don't repeat 'ti,reset-bits' either. > > Do you mean I should add a Mediatek reset binding doc, although Mediatek > reuse the TI reset controller directly? Hmm, how do you envision not repeating the same bits in a separate binding? Does it help if I convert this to YAML first without a ti, prefix in the file name? The usage philosophy definitely was to use a <soc-compatible> followed by the <generic-compatible>. This is how all of our reset nodes were added as well. Looks like Andrew may have misinterpreted your comment [1] during the original binding and changed "syscon-reset" to "ti,syscon-reset" in the final version [2]. regards Suman [1] https://lore.kernel.org/patchwork/comment/876688/ [2] https://lore.kernel.org/patchwork/patch/693172/ > > Best Regards > Crystal >> >>> - #reset-cells : Should be 1. Please see the reset consumer node below >>> for usage details >>> - ti,reset-bits : Contains the reset control register information >>> -- >>> 2.18.0 > _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,2/4] dt-binding: reset-controller: ti: add 'mediatek,infra-reset' to compatible 2020-09-02 23:25 ` Suman Anna (?) @ 2020-09-08 18:49 ` Rob Herring -1 siblings, 0 replies; 82+ messages in thread From: Rob Herring @ 2020-09-08 18:49 UTC (permalink / raw) To: Suman Anna Cc: Crystal Guo, p.zabel, matthias.bgg, srv_heupstream, linux-mediatek, linux-arm-kernel, linux-kernel, devicetree, Seiya Wang (王迺君), Stanley Chu (朱原陞), Yingjoe Chen (陳英洲), Fan Chen (陳凡), Yong Liang (梁勇) On Wed, Sep 2, 2020 at 5:26 PM Suman Anna <s-anna@ti.com> wrote: > > Hi Rob, > > On 8/26/20 6:09 AM, Crystal Guo wrote: > > On Wed, 2020-08-26 at 03:02 +0800, Rob Herring wrote: > >> On Mon, Aug 17, 2020 at 11:03:22AM +0800, Crystal Guo wrote: > >>> The TI syscon reset controller provides a common reset management, > >>> and is suitable for MTK SoCs. Add compatible 'mediatek,infra-reset', > >>> which denotes to use ti reset-controller driver directly. > >>> > >>> Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> > >>> --- > >>> Documentation/devicetree/bindings/reset/ti-syscon-reset.txt | 1 + > >>> 1 file changed, 1 insertion(+) > >>> > >>> diff --git a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > >>> index ab041032339b..5a0e9365b51b 100644 > >>> --- a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > >>> +++ b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > >>> @@ -25,6 +25,7 @@ Required properties: > >>> "ti,k2l-pscrst" > >>> "ti,k2hk-pscrst" > >>> "ti,syscon-reset" > >>> + "mediatek,infra-reset", "ti,syscon-reset" > >> > >> You need your own binding doc. If you can use the same driver then fine, > >> but that's a separate issue. There's also reset-simple driver if you > >> have just array of 32-bit registers with a bit per reset. > >> > >> Don't repeat 'ti,reset-bits' either. > > > > Do you mean I should add a Mediatek reset binding doc, although Mediatek > > reuse the TI reset controller directly? > > Hmm, how do you envision not repeating the same bits in a separate binding? I mean 'ti,reset-bits' isn't really something that should have been in DT in the first place, but rather implied by the compatible string. > Does it help if I convert this to YAML first without a ti, prefix in the file name? No, I don't think this should be a shared binding. The driver may be able to be shared, but that's independent from the binding. Rob ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,2/4] dt-binding: reset-controller: ti: add 'mediatek,infra-reset' to compatible @ 2020-09-08 18:49 ` Rob Herring 0 siblings, 0 replies; 82+ messages in thread From: Rob Herring @ 2020-09-08 18:49 UTC (permalink / raw) To: Suman Anna Cc: devicetree, Yong Liang (梁勇), srv_heupstream, Seiya Wang (王迺君), linux-kernel, Fan Chen (陳凡), linux-mediatek, p.zabel, Yingjoe Chen (陳英洲), matthias.bgg, Crystal Guo, Stanley Chu (朱原陞), linux-arm-kernel On Wed, Sep 2, 2020 at 5:26 PM Suman Anna <s-anna@ti.com> wrote: > > Hi Rob, > > On 8/26/20 6:09 AM, Crystal Guo wrote: > > On Wed, 2020-08-26 at 03:02 +0800, Rob Herring wrote: > >> On Mon, Aug 17, 2020 at 11:03:22AM +0800, Crystal Guo wrote: > >>> The TI syscon reset controller provides a common reset management, > >>> and is suitable for MTK SoCs. Add compatible 'mediatek,infra-reset', > >>> which denotes to use ti reset-controller driver directly. > >>> > >>> Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> > >>> --- > >>> Documentation/devicetree/bindings/reset/ti-syscon-reset.txt | 1 + > >>> 1 file changed, 1 insertion(+) > >>> > >>> diff --git a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > >>> index ab041032339b..5a0e9365b51b 100644 > >>> --- a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > >>> +++ b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > >>> @@ -25,6 +25,7 @@ Required properties: > >>> "ti,k2l-pscrst" > >>> "ti,k2hk-pscrst" > >>> "ti,syscon-reset" > >>> + "mediatek,infra-reset", "ti,syscon-reset" > >> > >> You need your own binding doc. If you can use the same driver then fine, > >> but that's a separate issue. There's also reset-simple driver if you > >> have just array of 32-bit registers with a bit per reset. > >> > >> Don't repeat 'ti,reset-bits' either. > > > > Do you mean I should add a Mediatek reset binding doc, although Mediatek > > reuse the TI reset controller directly? > > Hmm, how do you envision not repeating the same bits in a separate binding? I mean 'ti,reset-bits' isn't really something that should have been in DT in the first place, but rather implied by the compatible string. > Does it help if I convert this to YAML first without a ti, prefix in the file name? No, I don't think this should be a shared binding. The driver may be able to be shared, but that's independent from the binding. Rob _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,2/4] dt-binding: reset-controller: ti: add 'mediatek,infra-reset' to compatible @ 2020-09-08 18:49 ` Rob Herring 0 siblings, 0 replies; 82+ messages in thread From: Rob Herring @ 2020-09-08 18:49 UTC (permalink / raw) To: Suman Anna Cc: devicetree, Yong Liang (梁勇), srv_heupstream, Seiya Wang (王迺君), linux-kernel, Fan Chen (陳凡), linux-mediatek, p.zabel, Yingjoe Chen (陳英洲), matthias.bgg, Crystal Guo, Stanley Chu (朱原陞), linux-arm-kernel On Wed, Sep 2, 2020 at 5:26 PM Suman Anna <s-anna@ti.com> wrote: > > Hi Rob, > > On 8/26/20 6:09 AM, Crystal Guo wrote: > > On Wed, 2020-08-26 at 03:02 +0800, Rob Herring wrote: > >> On Mon, Aug 17, 2020 at 11:03:22AM +0800, Crystal Guo wrote: > >>> The TI syscon reset controller provides a common reset management, > >>> and is suitable for MTK SoCs. Add compatible 'mediatek,infra-reset', > >>> which denotes to use ti reset-controller driver directly. > >>> > >>> Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> > >>> --- > >>> Documentation/devicetree/bindings/reset/ti-syscon-reset.txt | 1 + > >>> 1 file changed, 1 insertion(+) > >>> > >>> diff --git a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > >>> index ab041032339b..5a0e9365b51b 100644 > >>> --- a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > >>> +++ b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > >>> @@ -25,6 +25,7 @@ Required properties: > >>> "ti,k2l-pscrst" > >>> "ti,k2hk-pscrst" > >>> "ti,syscon-reset" > >>> + "mediatek,infra-reset", "ti,syscon-reset" > >> > >> You need your own binding doc. If you can use the same driver then fine, > >> but that's a separate issue. There's also reset-simple driver if you > >> have just array of 32-bit registers with a bit per reset. > >> > >> Don't repeat 'ti,reset-bits' either. > > > > Do you mean I should add a Mediatek reset binding doc, although Mediatek > > reuse the TI reset controller directly? > > Hmm, how do you envision not repeating the same bits in a separate binding? I mean 'ti,reset-bits' isn't really something that should have been in DT in the first place, but rather implied by the compatible string. > Does it help if I convert this to YAML first without a ti, prefix in the file name? No, I don't think this should be a shared binding. The driver may be able to be shared, but that's independent from the binding. Rob _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,2/4] dt-binding: reset-controller: ti: add 'mediatek,infra-reset' to compatible 2020-09-08 18:49 ` Rob Herring (?) @ 2020-09-09 15:10 ` Suman Anna -1 siblings, 0 replies; 82+ messages in thread From: Suman Anna @ 2020-09-09 15:10 UTC (permalink / raw) To: Rob Herring Cc: Crystal Guo, p.zabel, matthias.bgg, srv_heupstream, linux-mediatek, linux-arm-kernel, linux-kernel, devicetree, Seiya Wang (王迺君), Stanley Chu (朱原陞), Yingjoe Chen (陳英洲), Fan Chen (陳凡), Yong Liang (梁勇) On 9/8/20 1:49 PM, Rob Herring wrote: > On Wed, Sep 2, 2020 at 5:26 PM Suman Anna <s-anna@ti.com> wrote: >> >> Hi Rob, >> >> On 8/26/20 6:09 AM, Crystal Guo wrote: >>> On Wed, 2020-08-26 at 03:02 +0800, Rob Herring wrote: >>>> On Mon, Aug 17, 2020 at 11:03:22AM +0800, Crystal Guo wrote: >>>>> The TI syscon reset controller provides a common reset management, >>>>> and is suitable for MTK SoCs. Add compatible 'mediatek,infra-reset', >>>>> which denotes to use ti reset-controller driver directly. >>>>> >>>>> Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> >>>>> --- >>>>> Documentation/devicetree/bindings/reset/ti-syscon-reset.txt | 1 + >>>>> 1 file changed, 1 insertion(+) >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt >>>>> index ab041032339b..5a0e9365b51b 100644 >>>>> --- a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt >>>>> +++ b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt >>>>> @@ -25,6 +25,7 @@ Required properties: >>>>> "ti,k2l-pscrst" >>>>> "ti,k2hk-pscrst" >>>>> "ti,syscon-reset" >>>>> + "mediatek,infra-reset", "ti,syscon-reset" >>>> >>>> You need your own binding doc. If you can use the same driver then fine, >>>> but that's a separate issue. There's also reset-simple driver if you >>>> have just array of 32-bit registers with a bit per reset. >>>> >>>> Don't repeat 'ti,reset-bits' either. >>> >>> Do you mean I should add a Mediatek reset binding doc, although Mediatek >>> reuse the TI reset controller directly? >> >> Hmm, how do you envision not repeating the same bits in a separate binding? > > I mean 'ti,reset-bits' isn't really something that should have been in > DT in the first place, but rather implied by the compatible string. Ok, should I be deprecating this and move this data to driver then? I am assuming that is how you are envisioning the new Mediatek binding to be atleast. regards Suman > >> Does it help if I convert this to YAML first without a ti, prefix in the file name? > > No, I don't think this should be a shared binding. The driver may be > able to be shared, but that's independent from the binding. > > Rob > ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,2/4] dt-binding: reset-controller: ti: add 'mediatek,infra-reset' to compatible @ 2020-09-09 15:10 ` Suman Anna 0 siblings, 0 replies; 82+ messages in thread From: Suman Anna @ 2020-09-09 15:10 UTC (permalink / raw) To: Rob Herring Cc: devicetree, Yong Liang (梁勇), srv_heupstream, Seiya Wang (王迺君), linux-kernel, Fan Chen (陳凡), linux-mediatek, p.zabel, Yingjoe Chen (陳英洲), matthias.bgg, Crystal Guo, Stanley Chu (朱原陞), linux-arm-kernel On 9/8/20 1:49 PM, Rob Herring wrote: > On Wed, Sep 2, 2020 at 5:26 PM Suman Anna <s-anna@ti.com> wrote: >> >> Hi Rob, >> >> On 8/26/20 6:09 AM, Crystal Guo wrote: >>> On Wed, 2020-08-26 at 03:02 +0800, Rob Herring wrote: >>>> On Mon, Aug 17, 2020 at 11:03:22AM +0800, Crystal Guo wrote: >>>>> The TI syscon reset controller provides a common reset management, >>>>> and is suitable for MTK SoCs. Add compatible 'mediatek,infra-reset', >>>>> which denotes to use ti reset-controller driver directly. >>>>> >>>>> Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> >>>>> --- >>>>> Documentation/devicetree/bindings/reset/ti-syscon-reset.txt | 1 + >>>>> 1 file changed, 1 insertion(+) >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt >>>>> index ab041032339b..5a0e9365b51b 100644 >>>>> --- a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt >>>>> +++ b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt >>>>> @@ -25,6 +25,7 @@ Required properties: >>>>> "ti,k2l-pscrst" >>>>> "ti,k2hk-pscrst" >>>>> "ti,syscon-reset" >>>>> + "mediatek,infra-reset", "ti,syscon-reset" >>>> >>>> You need your own binding doc. If you can use the same driver then fine, >>>> but that's a separate issue. There's also reset-simple driver if you >>>> have just array of 32-bit registers with a bit per reset. >>>> >>>> Don't repeat 'ti,reset-bits' either. >>> >>> Do you mean I should add a Mediatek reset binding doc, although Mediatek >>> reuse the TI reset controller directly? >> >> Hmm, how do you envision not repeating the same bits in a separate binding? > > I mean 'ti,reset-bits' isn't really something that should have been in > DT in the first place, but rather implied by the compatible string. Ok, should I be deprecating this and move this data to driver then? I am assuming that is how you are envisioning the new Mediatek binding to be atleast. regards Suman > >> Does it help if I convert this to YAML first without a ti, prefix in the file name? > > No, I don't think this should be a shared binding. The driver may be > able to be shared, but that's independent from the binding. > > Rob > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,2/4] dt-binding: reset-controller: ti: add 'mediatek,infra-reset' to compatible @ 2020-09-09 15:10 ` Suman Anna 0 siblings, 0 replies; 82+ messages in thread From: Suman Anna @ 2020-09-09 15:10 UTC (permalink / raw) To: Rob Herring Cc: devicetree, Yong Liang (梁勇), srv_heupstream, Seiya Wang (王迺君), linux-kernel, Fan Chen (陳凡), linux-mediatek, p.zabel, Yingjoe Chen (陳英洲), matthias.bgg, Crystal Guo, Stanley Chu (朱原陞), linux-arm-kernel On 9/8/20 1:49 PM, Rob Herring wrote: > On Wed, Sep 2, 2020 at 5:26 PM Suman Anna <s-anna@ti.com> wrote: >> >> Hi Rob, >> >> On 8/26/20 6:09 AM, Crystal Guo wrote: >>> On Wed, 2020-08-26 at 03:02 +0800, Rob Herring wrote: >>>> On Mon, Aug 17, 2020 at 11:03:22AM +0800, Crystal Guo wrote: >>>>> The TI syscon reset controller provides a common reset management, >>>>> and is suitable for MTK SoCs. Add compatible 'mediatek,infra-reset', >>>>> which denotes to use ti reset-controller driver directly. >>>>> >>>>> Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> >>>>> --- >>>>> Documentation/devicetree/bindings/reset/ti-syscon-reset.txt | 1 + >>>>> 1 file changed, 1 insertion(+) >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt >>>>> index ab041032339b..5a0e9365b51b 100644 >>>>> --- a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt >>>>> +++ b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt >>>>> @@ -25,6 +25,7 @@ Required properties: >>>>> "ti,k2l-pscrst" >>>>> "ti,k2hk-pscrst" >>>>> "ti,syscon-reset" >>>>> + "mediatek,infra-reset", "ti,syscon-reset" >>>> >>>> You need your own binding doc. If you can use the same driver then fine, >>>> but that's a separate issue. There's also reset-simple driver if you >>>> have just array of 32-bit registers with a bit per reset. >>>> >>>> Don't repeat 'ti,reset-bits' either. >>> >>> Do you mean I should add a Mediatek reset binding doc, although Mediatek >>> reuse the TI reset controller directly? >> >> Hmm, how do you envision not repeating the same bits in a separate binding? > > I mean 'ti,reset-bits' isn't really something that should have been in > DT in the first place, but rather implied by the compatible string. Ok, should I be deprecating this and move this data to driver then? I am assuming that is how you are envisioning the new Mediatek binding to be atleast. regards Suman > >> Does it help if I convert this to YAML first without a ti, prefix in the file name? > > No, I don't think this should be a shared binding. The driver may be > able to be shared, but that's independent from the binding. > > Rob > _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,2/4] dt-binding: reset-controller: ti: add 'mediatek,infra-reset' to compatible 2020-09-09 15:10 ` Suman Anna (?) @ 2020-09-09 18:20 ` Rob Herring -1 siblings, 0 replies; 82+ messages in thread From: Rob Herring @ 2020-09-09 18:20 UTC (permalink / raw) To: Suman Anna Cc: Crystal Guo, p.zabel, matthias.bgg, srv_heupstream, linux-mediatek, linux-arm-kernel, linux-kernel, devicetree, Seiya Wang (王迺君), Stanley Chu (朱原陞), Yingjoe Chen (陳英洲), Fan Chen (陳凡), Yong Liang (梁勇) On Wed, Sep 9, 2020 at 9:10 AM Suman Anna <s-anna@ti.com> wrote: > > On 9/8/20 1:49 PM, Rob Herring wrote: > > On Wed, Sep 2, 2020 at 5:26 PM Suman Anna <s-anna@ti.com> wrote: > >> > >> Hi Rob, > >> > >> On 8/26/20 6:09 AM, Crystal Guo wrote: > >>> On Wed, 2020-08-26 at 03:02 +0800, Rob Herring wrote: > >>>> On Mon, Aug 17, 2020 at 11:03:22AM +0800, Crystal Guo wrote: > >>>>> The TI syscon reset controller provides a common reset management, > >>>>> and is suitable for MTK SoCs. Add compatible 'mediatek,infra-reset', > >>>>> which denotes to use ti reset-controller driver directly. > >>>>> > >>>>> Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> > >>>>> --- > >>>>> Documentation/devicetree/bindings/reset/ti-syscon-reset.txt | 1 + > >>>>> 1 file changed, 1 insertion(+) > >>>>> > >>>>> diff --git a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > >>>>> index ab041032339b..5a0e9365b51b 100644 > >>>>> --- a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > >>>>> +++ b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > >>>>> @@ -25,6 +25,7 @@ Required properties: > >>>>> "ti,k2l-pscrst" > >>>>> "ti,k2hk-pscrst" > >>>>> "ti,syscon-reset" > >>>>> + "mediatek,infra-reset", "ti,syscon-reset" > >>>> > >>>> You need your own binding doc. If you can use the same driver then fine, > >>>> but that's a separate issue. There's also reset-simple driver if you > >>>> have just array of 32-bit registers with a bit per reset. > >>>> > >>>> Don't repeat 'ti,reset-bits' either. > >>> > >>> Do you mean I should add a Mediatek reset binding doc, although Mediatek > >>> reuse the TI reset controller directly? > >> > >> Hmm, how do you envision not repeating the same bits in a separate binding? > > > > I mean 'ti,reset-bits' isn't really something that should have been in > > DT in the first place, but rather implied by the compatible string. > > Ok, should I be deprecating this and move this data to driver then? No, I'm just asking to not have new users. > I am assuming that is how you are envisioning the new Mediatek binding to be > atleast. Right. Rob ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,2/4] dt-binding: reset-controller: ti: add 'mediatek,infra-reset' to compatible @ 2020-09-09 18:20 ` Rob Herring 0 siblings, 0 replies; 82+ messages in thread From: Rob Herring @ 2020-09-09 18:20 UTC (permalink / raw) To: Suman Anna Cc: devicetree, Yong Liang (梁勇), srv_heupstream, Seiya Wang (王迺君), linux-kernel, Fan Chen (陳凡), linux-mediatek, p.zabel, Yingjoe Chen (陳英洲), matthias.bgg, Crystal Guo, Stanley Chu (朱原陞), linux-arm-kernel On Wed, Sep 9, 2020 at 9:10 AM Suman Anna <s-anna@ti.com> wrote: > > On 9/8/20 1:49 PM, Rob Herring wrote: > > On Wed, Sep 2, 2020 at 5:26 PM Suman Anna <s-anna@ti.com> wrote: > >> > >> Hi Rob, > >> > >> On 8/26/20 6:09 AM, Crystal Guo wrote: > >>> On Wed, 2020-08-26 at 03:02 +0800, Rob Herring wrote: > >>>> On Mon, Aug 17, 2020 at 11:03:22AM +0800, Crystal Guo wrote: > >>>>> The TI syscon reset controller provides a common reset management, > >>>>> and is suitable for MTK SoCs. Add compatible 'mediatek,infra-reset', > >>>>> which denotes to use ti reset-controller driver directly. > >>>>> > >>>>> Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> > >>>>> --- > >>>>> Documentation/devicetree/bindings/reset/ti-syscon-reset.txt | 1 + > >>>>> 1 file changed, 1 insertion(+) > >>>>> > >>>>> diff --git a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > >>>>> index ab041032339b..5a0e9365b51b 100644 > >>>>> --- a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > >>>>> +++ b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > >>>>> @@ -25,6 +25,7 @@ Required properties: > >>>>> "ti,k2l-pscrst" > >>>>> "ti,k2hk-pscrst" > >>>>> "ti,syscon-reset" > >>>>> + "mediatek,infra-reset", "ti,syscon-reset" > >>>> > >>>> You need your own binding doc. If you can use the same driver then fine, > >>>> but that's a separate issue. There's also reset-simple driver if you > >>>> have just array of 32-bit registers with a bit per reset. > >>>> > >>>> Don't repeat 'ti,reset-bits' either. > >>> > >>> Do you mean I should add a Mediatek reset binding doc, although Mediatek > >>> reuse the TI reset controller directly? > >> > >> Hmm, how do you envision not repeating the same bits in a separate binding? > > > > I mean 'ti,reset-bits' isn't really something that should have been in > > DT in the first place, but rather implied by the compatible string. > > Ok, should I be deprecating this and move this data to driver then? No, I'm just asking to not have new users. > I am assuming that is how you are envisioning the new Mediatek binding to be > atleast. Right. Rob _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,2/4] dt-binding: reset-controller: ti: add 'mediatek,infra-reset' to compatible @ 2020-09-09 18:20 ` Rob Herring 0 siblings, 0 replies; 82+ messages in thread From: Rob Herring @ 2020-09-09 18:20 UTC (permalink / raw) To: Suman Anna Cc: devicetree, Yong Liang (梁勇), srv_heupstream, Seiya Wang (王迺君), linux-kernel, Fan Chen (陳凡), linux-mediatek, p.zabel, Yingjoe Chen (陳英洲), matthias.bgg, Crystal Guo, Stanley Chu (朱原陞), linux-arm-kernel On Wed, Sep 9, 2020 at 9:10 AM Suman Anna <s-anna@ti.com> wrote: > > On 9/8/20 1:49 PM, Rob Herring wrote: > > On Wed, Sep 2, 2020 at 5:26 PM Suman Anna <s-anna@ti.com> wrote: > >> > >> Hi Rob, > >> > >> On 8/26/20 6:09 AM, Crystal Guo wrote: > >>> On Wed, 2020-08-26 at 03:02 +0800, Rob Herring wrote: > >>>> On Mon, Aug 17, 2020 at 11:03:22AM +0800, Crystal Guo wrote: > >>>>> The TI syscon reset controller provides a common reset management, > >>>>> and is suitable for MTK SoCs. Add compatible 'mediatek,infra-reset', > >>>>> which denotes to use ti reset-controller driver directly. > >>>>> > >>>>> Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> > >>>>> --- > >>>>> Documentation/devicetree/bindings/reset/ti-syscon-reset.txt | 1 + > >>>>> 1 file changed, 1 insertion(+) > >>>>> > >>>>> diff --git a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > >>>>> index ab041032339b..5a0e9365b51b 100644 > >>>>> --- a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > >>>>> +++ b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt > >>>>> @@ -25,6 +25,7 @@ Required properties: > >>>>> "ti,k2l-pscrst" > >>>>> "ti,k2hk-pscrst" > >>>>> "ti,syscon-reset" > >>>>> + "mediatek,infra-reset", "ti,syscon-reset" > >>>> > >>>> You need your own binding doc. If you can use the same driver then fine, > >>>> but that's a separate issue. There's also reset-simple driver if you > >>>> have just array of 32-bit registers with a bit per reset. > >>>> > >>>> Don't repeat 'ti,reset-bits' either. > >>> > >>> Do you mean I should add a Mediatek reset binding doc, although Mediatek > >>> reuse the TI reset controller directly? > >> > >> Hmm, how do you envision not repeating the same bits in a separate binding? > > > > I mean 'ti,reset-bits' isn't really something that should have been in > > DT in the first place, but rather implied by the compatible string. > > Ok, should I be deprecating this and move this data to driver then? No, I'm just asking to not have new users. > I am assuming that is how you are envisioning the new Mediatek binding to be > atleast. Right. Rob _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek ^ permalink raw reply [flat|nested] 82+ messages in thread
* [v4,3/4] reset-controller: ti: introduce a new reset handler 2020-08-17 3:03 ` Crystal Guo (?) @ 2020-08-17 3:03 ` Crystal Guo -1 siblings, 0 replies; 82+ messages in thread From: Crystal Guo @ 2020-08-17 3:03 UTC (permalink / raw) To: p.zabel, robh+dt, matthias.bgg Cc: srv_heupstream, linux-mediatek, linux-arm-kernel, linux-kernel, devicetree, s-anna, afd, seiya.wang, stanley.chu, yingjoe.chen, fan.chen, yong.liang, Crystal Guo Introduce ti_syscon_reset() to integrate assert and deassert together. If some modules need do serialized assert and deassert operations to reset itself, reset_control_reset can be called for convenience. Such as reset-qcom-aoss.c, it integrates assert and deassert together by 'reset' method. MTK Socs also need this method to perform reset. Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> --- drivers/reset/reset-ti-syscon.c | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/drivers/reset/reset-ti-syscon.c b/drivers/reset/reset-ti-syscon.c index a2635c21db7f..08289342f9af 100644 --- a/drivers/reset/reset-ti-syscon.c +++ b/drivers/reset/reset-ti-syscon.c @@ -15,6 +15,7 @@ * GNU General Public License for more details. */ +#include <linux/delay.h> #include <linux/mfd/syscon.h> #include <linux/module.h> #include <linux/of.h> @@ -56,6 +57,7 @@ struct ti_syscon_reset_data { struct regmap *regmap; struct ti_syscon_reset_control *controls; unsigned int nr_controls; + unsigned int reset_duration_us; }; #define to_ti_syscon_reset_data(rcdev) \ @@ -89,7 +91,7 @@ static int ti_syscon_reset_assert(struct reset_controller_dev *rcdev, mask = BIT(control->assert_bit); value = (control->flags & ASSERT_SET) ? mask : 0x0; - return regmap_update_bits(data->regmap, control->assert_offset, mask, value); + return regmap_write_bits(data->regmap, control->assert_offset, mask, value); } /** @@ -120,7 +122,7 @@ static int ti_syscon_reset_deassert(struct reset_controller_dev *rcdev, mask = BIT(control->deassert_bit); value = (control->flags & DEASSERT_SET) ? mask : 0x0; - return regmap_update_bits(data->regmap, control->deassert_offset, mask, value); + return regmap_write_bits(data->regmap, control->deassert_offset, mask, value); } /** @@ -158,9 +160,26 @@ static int ti_syscon_reset_status(struct reset_controller_dev *rcdev, !(control->flags & STATUS_SET); } +static int ti_syscon_reset(struct reset_controller_dev *rcdev, + unsigned long id) +{ + struct ti_syscon_reset_data *data = to_ti_syscon_reset_data(rcdev); + int ret; + + ret = ti_syscon_reset_assert(rcdev, id); + if (ret) + return ret; + + if (data->reset_duration_us) + usleep_range(data->reset_duration_us, data->reset_duration_us * 2); + + return ti_syscon_reset_deassert(rcdev, id); +} + static const struct reset_control_ops ti_syscon_reset_ops = { .assert = ti_syscon_reset_assert, .deassert = ti_syscon_reset_deassert, + .reset = ti_syscon_reset, .status = ti_syscon_reset_status, }; @@ -204,6 +223,9 @@ static int ti_syscon_reset_probe(struct platform_device *pdev) controls[i].flags = be32_to_cpup(list++); } + of_property_read_u32(pdev->dev.of_node, "reset-duration-us", + &data->reset_duration_us); + data->rcdev.ops = &ti_syscon_reset_ops; data->rcdev.owner = THIS_MODULE; data->rcdev.of_node = np; -- 2.18.0 ^ permalink raw reply related [flat|nested] 82+ messages in thread
* [v4,3/4] reset-controller: ti: introduce a new reset handler @ 2020-08-17 3:03 ` Crystal Guo 0 siblings, 0 replies; 82+ messages in thread From: Crystal Guo @ 2020-08-17 3:03 UTC (permalink / raw) To: p.zabel, robh+dt, matthias.bgg Cc: devicetree, yong.liang, stanley.chu, srv_heupstream, seiya.wang, linux-kernel, afd, fan.chen, Crystal Guo, linux-mediatek, yingjoe.chen, linux-arm-kernel Introduce ti_syscon_reset() to integrate assert and deassert together. If some modules need do serialized assert and deassert operations to reset itself, reset_control_reset can be called for convenience. Such as reset-qcom-aoss.c, it integrates assert and deassert together by 'reset' method. MTK Socs also need this method to perform reset. Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> --- drivers/reset/reset-ti-syscon.c | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/drivers/reset/reset-ti-syscon.c b/drivers/reset/reset-ti-syscon.c index a2635c21db7f..08289342f9af 100644 --- a/drivers/reset/reset-ti-syscon.c +++ b/drivers/reset/reset-ti-syscon.c @@ -15,6 +15,7 @@ * GNU General Public License for more details. */ +#include <linux/delay.h> #include <linux/mfd/syscon.h> #include <linux/module.h> #include <linux/of.h> @@ -56,6 +57,7 @@ struct ti_syscon_reset_data { struct regmap *regmap; struct ti_syscon_reset_control *controls; unsigned int nr_controls; + unsigned int reset_duration_us; }; #define to_ti_syscon_reset_data(rcdev) \ @@ -89,7 +91,7 @@ static int ti_syscon_reset_assert(struct reset_controller_dev *rcdev, mask = BIT(control->assert_bit); value = (control->flags & ASSERT_SET) ? mask : 0x0; - return regmap_update_bits(data->regmap, control->assert_offset, mask, value); + return regmap_write_bits(data->regmap, control->assert_offset, mask, value); } /** @@ -120,7 +122,7 @@ static int ti_syscon_reset_deassert(struct reset_controller_dev *rcdev, mask = BIT(control->deassert_bit); value = (control->flags & DEASSERT_SET) ? mask : 0x0; - return regmap_update_bits(data->regmap, control->deassert_offset, mask, value); + return regmap_write_bits(data->regmap, control->deassert_offset, mask, value); } /** @@ -158,9 +160,26 @@ static int ti_syscon_reset_status(struct reset_controller_dev *rcdev, !(control->flags & STATUS_SET); } +static int ti_syscon_reset(struct reset_controller_dev *rcdev, + unsigned long id) +{ + struct ti_syscon_reset_data *data = to_ti_syscon_reset_data(rcdev); + int ret; + + ret = ti_syscon_reset_assert(rcdev, id); + if (ret) + return ret; + + if (data->reset_duration_us) + usleep_range(data->reset_duration_us, data->reset_duration_us * 2); + + return ti_syscon_reset_deassert(rcdev, id); +} + static const struct reset_control_ops ti_syscon_reset_ops = { .assert = ti_syscon_reset_assert, .deassert = ti_syscon_reset_deassert, + .reset = ti_syscon_reset, .status = ti_syscon_reset_status, }; @@ -204,6 +223,9 @@ static int ti_syscon_reset_probe(struct platform_device *pdev) controls[i].flags = be32_to_cpup(list++); } + of_property_read_u32(pdev->dev.of_node, "reset-duration-us", + &data->reset_duration_us); + data->rcdev.ops = &ti_syscon_reset_ops; data->rcdev.owner = THIS_MODULE; data->rcdev.of_node = np; -- 2.18.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 82+ messages in thread
* [v4,3/4] reset-controller: ti: introduce a new reset handler @ 2020-08-17 3:03 ` Crystal Guo 0 siblings, 0 replies; 82+ messages in thread From: Crystal Guo @ 2020-08-17 3:03 UTC (permalink / raw) To: p.zabel, robh+dt, matthias.bgg Cc: devicetree, yong.liang, stanley.chu, srv_heupstream, seiya.wang, linux-kernel, afd, fan.chen, Crystal Guo, linux-mediatek, yingjoe.chen, s-anna, linux-arm-kernel Introduce ti_syscon_reset() to integrate assert and deassert together. If some modules need do serialized assert and deassert operations to reset itself, reset_control_reset can be called for convenience. Such as reset-qcom-aoss.c, it integrates assert and deassert together by 'reset' method. MTK Socs also need this method to perform reset. Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> --- drivers/reset/reset-ti-syscon.c | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/drivers/reset/reset-ti-syscon.c b/drivers/reset/reset-ti-syscon.c index a2635c21db7f..08289342f9af 100644 --- a/drivers/reset/reset-ti-syscon.c +++ b/drivers/reset/reset-ti-syscon.c @@ -15,6 +15,7 @@ * GNU General Public License for more details. */ +#include <linux/delay.h> #include <linux/mfd/syscon.h> #include <linux/module.h> #include <linux/of.h> @@ -56,6 +57,7 @@ struct ti_syscon_reset_data { struct regmap *regmap; struct ti_syscon_reset_control *controls; unsigned int nr_controls; + unsigned int reset_duration_us; }; #define to_ti_syscon_reset_data(rcdev) \ @@ -89,7 +91,7 @@ static int ti_syscon_reset_assert(struct reset_controller_dev *rcdev, mask = BIT(control->assert_bit); value = (control->flags & ASSERT_SET) ? mask : 0x0; - return regmap_update_bits(data->regmap, control->assert_offset, mask, value); + return regmap_write_bits(data->regmap, control->assert_offset, mask, value); } /** @@ -120,7 +122,7 @@ static int ti_syscon_reset_deassert(struct reset_controller_dev *rcdev, mask = BIT(control->deassert_bit); value = (control->flags & DEASSERT_SET) ? mask : 0x0; - return regmap_update_bits(data->regmap, control->deassert_offset, mask, value); + return regmap_write_bits(data->regmap, control->deassert_offset, mask, value); } /** @@ -158,9 +160,26 @@ static int ti_syscon_reset_status(struct reset_controller_dev *rcdev, !(control->flags & STATUS_SET); } +static int ti_syscon_reset(struct reset_controller_dev *rcdev, + unsigned long id) +{ + struct ti_syscon_reset_data *data = to_ti_syscon_reset_data(rcdev); + int ret; + + ret = ti_syscon_reset_assert(rcdev, id); + if (ret) + return ret; + + if (data->reset_duration_us) + usleep_range(data->reset_duration_us, data->reset_duration_us * 2); + + return ti_syscon_reset_deassert(rcdev, id); +} + static const struct reset_control_ops ti_syscon_reset_ops = { .assert = ti_syscon_reset_assert, .deassert = ti_syscon_reset_deassert, + .reset = ti_syscon_reset, .status = ti_syscon_reset_status, }; @@ -204,6 +223,9 @@ static int ti_syscon_reset_probe(struct platform_device *pdev) controls[i].flags = be32_to_cpup(list++); } + of_property_read_u32(pdev->dev.of_node, "reset-duration-us", + &data->reset_duration_us); + data->rcdev.ops = &ti_syscon_reset_ops; data->rcdev.owner = THIS_MODULE; data->rcdev.of_node = np; -- 2.18.0 _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek ^ permalink raw reply related [flat|nested] 82+ messages in thread
* Re: [v4,3/4] reset-controller: ti: introduce a new reset handler 2020-08-17 3:03 ` Crystal Guo (?) @ 2020-09-02 23:40 ` Suman Anna -1 siblings, 0 replies; 82+ messages in thread From: Suman Anna @ 2020-09-02 23:40 UTC (permalink / raw) To: Crystal Guo, p.zabel, robh+dt, matthias.bgg Cc: srv_heupstream, linux-mediatek, linux-arm-kernel, linux-kernel, devicetree, seiya.wang, stanley.chu, yingjoe.chen, fan.chen, yong.liang Hi Crystal, On 8/16/20 10:03 PM, Crystal Guo wrote: > Introduce ti_syscon_reset() to integrate assert and deassert together. > If some modules need do serialized assert and deassert operations > to reset itself, reset_control_reset can be called for convenience. There are multiple changes in this same patch. I think you should split this functionality away from the change for the regmap_update_bits() to regmap_write_bits(), similar to what you have done in your v2 Patch 4. > > Such as reset-qcom-aoss.c, it integrates assert and deassert together > by 'reset' method. MTK Socs also need this method to perform reset. > > Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> > --- > drivers/reset/reset-ti-syscon.c | 26 ++++++++++++++++++++++++-- > 1 file changed, 24 insertions(+), 2 deletions(-) > > diff --git a/drivers/reset/reset-ti-syscon.c b/drivers/reset/reset-ti-syscon.c > index a2635c21db7f..08289342f9af 100644 > --- a/drivers/reset/reset-ti-syscon.c > +++ b/drivers/reset/reset-ti-syscon.c > @@ -15,6 +15,7 @@ > * GNU General Public License for more details. > */ > > +#include <linux/delay.h> > #include <linux/mfd/syscon.h> > #include <linux/module.h> > #include <linux/of.h> > @@ -56,6 +57,7 @@ struct ti_syscon_reset_data { > struct regmap *regmap; > struct ti_syscon_reset_control *controls; > unsigned int nr_controls; > + unsigned int reset_duration_us; > }; > > #define to_ti_syscon_reset_data(rcdev) \ > @@ -89,7 +91,7 @@ static int ti_syscon_reset_assert(struct reset_controller_dev *rcdev, > mask = BIT(control->assert_bit); > value = (control->flags & ASSERT_SET) ? mask : 0x0; > > - return regmap_update_bits(data->regmap, control->assert_offset, mask, value); > + return regmap_write_bits(data->regmap, control->assert_offset, mask, value); > } > > /** > @@ -120,7 +122,7 @@ static int ti_syscon_reset_deassert(struct reset_controller_dev *rcdev, > mask = BIT(control->deassert_bit); > value = (control->flags & DEASSERT_SET) ? mask : 0x0; > > - return regmap_update_bits(data->regmap, control->deassert_offset, mask, value); > + return regmap_write_bits(data->regmap, control->deassert_offset, mask, value); > } > > /** > @@ -158,9 +160,26 @@ static int ti_syscon_reset_status(struct reset_controller_dev *rcdev, > !(control->flags & STATUS_SET); > } > > +static int ti_syscon_reset(struct reset_controller_dev *rcdev, > + unsigned long id) > +{ > + struct ti_syscon_reset_data *data = to_ti_syscon_reset_data(rcdev); > + int ret; > + > + ret = ti_syscon_reset_assert(rcdev, id); > + if (ret) > + return ret; > + > + if (data->reset_duration_us) > + usleep_range(data->reset_duration_us, data->reset_duration_us * 2); > + > + return ti_syscon_reset_deassert(rcdev, id); I echo Philipp's comments [1] from your original v1 series about this. We don't need a property to distinguish this, but you could add a flag using match data and Mediatek compatible, and use that within this function, or optionally set this ops based on compatible (whatever is preferred by Philipp). regards Suman [1] https://patchwork.kernel.org/comment/23519193/ > +} > + > static const struct reset_control_ops ti_syscon_reset_ops = { > .assert = ti_syscon_reset_assert, > .deassert = ti_syscon_reset_deassert, > + .reset = ti_syscon_reset, > .status = ti_syscon_reset_status, > }; > > @@ -204,6 +223,9 @@ static int ti_syscon_reset_probe(struct platform_device *pdev) > controls[i].flags = be32_to_cpup(list++); > } > > + of_property_read_u32(pdev->dev.of_node, "reset-duration-us", > + &data->reset_duration_us); > + > data->rcdev.ops = &ti_syscon_reset_ops; > data->rcdev.owner = THIS_MODULE; > data->rcdev.of_node = np; > ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,3/4] reset-controller: ti: introduce a new reset handler @ 2020-09-02 23:40 ` Suman Anna 0 siblings, 0 replies; 82+ messages in thread From: Suman Anna @ 2020-09-02 23:40 UTC (permalink / raw) To: Crystal Guo, p.zabel, robh+dt, matthias.bgg Cc: devicetree, yong.liang, srv_heupstream, seiya.wang, linux-kernel, fan.chen, linux-mediatek, yingjoe.chen, stanley.chu, linux-arm-kernel Hi Crystal, On 8/16/20 10:03 PM, Crystal Guo wrote: > Introduce ti_syscon_reset() to integrate assert and deassert together. > If some modules need do serialized assert and deassert operations > to reset itself, reset_control_reset can be called for convenience. There are multiple changes in this same patch. I think you should split this functionality away from the change for the regmap_update_bits() to regmap_write_bits(), similar to what you have done in your v2 Patch 4. > > Such as reset-qcom-aoss.c, it integrates assert and deassert together > by 'reset' method. MTK Socs also need this method to perform reset. > > Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> > --- > drivers/reset/reset-ti-syscon.c | 26 ++++++++++++++++++++++++-- > 1 file changed, 24 insertions(+), 2 deletions(-) > > diff --git a/drivers/reset/reset-ti-syscon.c b/drivers/reset/reset-ti-syscon.c > index a2635c21db7f..08289342f9af 100644 > --- a/drivers/reset/reset-ti-syscon.c > +++ b/drivers/reset/reset-ti-syscon.c > @@ -15,6 +15,7 @@ > * GNU General Public License for more details. > */ > > +#include <linux/delay.h> > #include <linux/mfd/syscon.h> > #include <linux/module.h> > #include <linux/of.h> > @@ -56,6 +57,7 @@ struct ti_syscon_reset_data { > struct regmap *regmap; > struct ti_syscon_reset_control *controls; > unsigned int nr_controls; > + unsigned int reset_duration_us; > }; > > #define to_ti_syscon_reset_data(rcdev) \ > @@ -89,7 +91,7 @@ static int ti_syscon_reset_assert(struct reset_controller_dev *rcdev, > mask = BIT(control->assert_bit); > value = (control->flags & ASSERT_SET) ? mask : 0x0; > > - return regmap_update_bits(data->regmap, control->assert_offset, mask, value); > + return regmap_write_bits(data->regmap, control->assert_offset, mask, value); > } > > /** > @@ -120,7 +122,7 @@ static int ti_syscon_reset_deassert(struct reset_controller_dev *rcdev, > mask = BIT(control->deassert_bit); > value = (control->flags & DEASSERT_SET) ? mask : 0x0; > > - return regmap_update_bits(data->regmap, control->deassert_offset, mask, value); > + return regmap_write_bits(data->regmap, control->deassert_offset, mask, value); > } > > /** > @@ -158,9 +160,26 @@ static int ti_syscon_reset_status(struct reset_controller_dev *rcdev, > !(control->flags & STATUS_SET); > } > > +static int ti_syscon_reset(struct reset_controller_dev *rcdev, > + unsigned long id) > +{ > + struct ti_syscon_reset_data *data = to_ti_syscon_reset_data(rcdev); > + int ret; > + > + ret = ti_syscon_reset_assert(rcdev, id); > + if (ret) > + return ret; > + > + if (data->reset_duration_us) > + usleep_range(data->reset_duration_us, data->reset_duration_us * 2); > + > + return ti_syscon_reset_deassert(rcdev, id); I echo Philipp's comments [1] from your original v1 series about this. We don't need a property to distinguish this, but you could add a flag using match data and Mediatek compatible, and use that within this function, or optionally set this ops based on compatible (whatever is preferred by Philipp). regards Suman [1] https://patchwork.kernel.org/comment/23519193/ > +} > + > static const struct reset_control_ops ti_syscon_reset_ops = { > .assert = ti_syscon_reset_assert, > .deassert = ti_syscon_reset_deassert, > + .reset = ti_syscon_reset, > .status = ti_syscon_reset_status, > }; > > @@ -204,6 +223,9 @@ static int ti_syscon_reset_probe(struct platform_device *pdev) > controls[i].flags = be32_to_cpup(list++); > } > > + of_property_read_u32(pdev->dev.of_node, "reset-duration-us", > + &data->reset_duration_us); > + > data->rcdev.ops = &ti_syscon_reset_ops; > data->rcdev.owner = THIS_MODULE; > data->rcdev.of_node = np; > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,3/4] reset-controller: ti: introduce a new reset handler @ 2020-09-02 23:40 ` Suman Anna 0 siblings, 0 replies; 82+ messages in thread From: Suman Anna @ 2020-09-02 23:40 UTC (permalink / raw) To: Crystal Guo, p.zabel, robh+dt, matthias.bgg Cc: devicetree, yong.liang, srv_heupstream, seiya.wang, linux-kernel, fan.chen, linux-mediatek, yingjoe.chen, stanley.chu, linux-arm-kernel Hi Crystal, On 8/16/20 10:03 PM, Crystal Guo wrote: > Introduce ti_syscon_reset() to integrate assert and deassert together. > If some modules need do serialized assert and deassert operations > to reset itself, reset_control_reset can be called for convenience. There are multiple changes in this same patch. I think you should split this functionality away from the change for the regmap_update_bits() to regmap_write_bits(), similar to what you have done in your v2 Patch 4. > > Such as reset-qcom-aoss.c, it integrates assert and deassert together > by 'reset' method. MTK Socs also need this method to perform reset. > > Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> > --- > drivers/reset/reset-ti-syscon.c | 26 ++++++++++++++++++++++++-- > 1 file changed, 24 insertions(+), 2 deletions(-) > > diff --git a/drivers/reset/reset-ti-syscon.c b/drivers/reset/reset-ti-syscon.c > index a2635c21db7f..08289342f9af 100644 > --- a/drivers/reset/reset-ti-syscon.c > +++ b/drivers/reset/reset-ti-syscon.c > @@ -15,6 +15,7 @@ > * GNU General Public License for more details. > */ > > +#include <linux/delay.h> > #include <linux/mfd/syscon.h> > #include <linux/module.h> > #include <linux/of.h> > @@ -56,6 +57,7 @@ struct ti_syscon_reset_data { > struct regmap *regmap; > struct ti_syscon_reset_control *controls; > unsigned int nr_controls; > + unsigned int reset_duration_us; > }; > > #define to_ti_syscon_reset_data(rcdev) \ > @@ -89,7 +91,7 @@ static int ti_syscon_reset_assert(struct reset_controller_dev *rcdev, > mask = BIT(control->assert_bit); > value = (control->flags & ASSERT_SET) ? mask : 0x0; > > - return regmap_update_bits(data->regmap, control->assert_offset, mask, value); > + return regmap_write_bits(data->regmap, control->assert_offset, mask, value); > } > > /** > @@ -120,7 +122,7 @@ static int ti_syscon_reset_deassert(struct reset_controller_dev *rcdev, > mask = BIT(control->deassert_bit); > value = (control->flags & DEASSERT_SET) ? mask : 0x0; > > - return regmap_update_bits(data->regmap, control->deassert_offset, mask, value); > + return regmap_write_bits(data->regmap, control->deassert_offset, mask, value); > } > > /** > @@ -158,9 +160,26 @@ static int ti_syscon_reset_status(struct reset_controller_dev *rcdev, > !(control->flags & STATUS_SET); > } > > +static int ti_syscon_reset(struct reset_controller_dev *rcdev, > + unsigned long id) > +{ > + struct ti_syscon_reset_data *data = to_ti_syscon_reset_data(rcdev); > + int ret; > + > + ret = ti_syscon_reset_assert(rcdev, id); > + if (ret) > + return ret; > + > + if (data->reset_duration_us) > + usleep_range(data->reset_duration_us, data->reset_duration_us * 2); > + > + return ti_syscon_reset_deassert(rcdev, id); I echo Philipp's comments [1] from your original v1 series about this. We don't need a property to distinguish this, but you could add a flag using match data and Mediatek compatible, and use that within this function, or optionally set this ops based on compatible (whatever is preferred by Philipp). regards Suman [1] https://patchwork.kernel.org/comment/23519193/ > +} > + > static const struct reset_control_ops ti_syscon_reset_ops = { > .assert = ti_syscon_reset_assert, > .deassert = ti_syscon_reset_deassert, > + .reset = ti_syscon_reset, > .status = ti_syscon_reset_status, > }; > > @@ -204,6 +223,9 @@ static int ti_syscon_reset_probe(struct platform_device *pdev) > controls[i].flags = be32_to_cpup(list++); > } > > + of_property_read_u32(pdev->dev.of_node, "reset-duration-us", > + &data->reset_duration_us); > + > data->rcdev.ops = &ti_syscon_reset_ops; > data->rcdev.owner = THIS_MODULE; > data->rcdev.of_node = np; > _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,3/4] reset-controller: ti: introduce a new reset handler 2020-09-02 23:40 ` Suman Anna (?) @ 2020-09-09 2:57 ` Crystal Guo -1 siblings, 0 replies; 82+ messages in thread From: Crystal Guo @ 2020-09-09 2:57 UTC (permalink / raw) To: Suman Anna, p.zabel Cc: robh+dt, matthias.bgg, srv_heupstream, linux-mediatek, linux-arm-kernel, linux-kernel, devicetree, Seiya Wang (王迺君), Stanley Chu (朱原陞), Yingjoe Chen (陳英洲), Fan Chen (陳凡), Yong Liang (梁勇) On Thu, 2020-09-03 at 07:40 +0800, Suman Anna wrote: > Hi Crystal, > > On 8/16/20 10:03 PM, Crystal Guo wrote: > > Introduce ti_syscon_reset() to integrate assert and deassert together. > > If some modules need do serialized assert and deassert operations > > to reset itself, reset_control_reset can be called for convenience. > > There are multiple changes in this same patch. I think you should split this > functionality away from the change for the regmap_update_bits() to > regmap_write_bits(), similar to what you have done in your v2 Patch 4. > Thanks for your suggestion. I will split this patch in the next version. > > > > Such as reset-qcom-aoss.c, it integrates assert and deassert together > > by 'reset' method. MTK Socs also need this method to perform reset. > > > > Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> > > --- > > drivers/reset/reset-ti-syscon.c | 26 ++++++++++++++++++++++++-- > > 1 file changed, 24 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/reset/reset-ti-syscon.c b/drivers/reset/reset-ti-syscon.c > > index a2635c21db7f..08289342f9af 100644 > > --- a/drivers/reset/reset-ti-syscon.c > > +++ b/drivers/reset/reset-ti-syscon.c > > @@ -15,6 +15,7 @@ > > * GNU General Public License for more details. > > */ > > > > +#include <linux/delay.h> > > #include <linux/mfd/syscon.h> > > #include <linux/module.h> > > #include <linux/of.h> > > @@ -56,6 +57,7 @@ struct ti_syscon_reset_data { > > struct regmap *regmap; > > struct ti_syscon_reset_control *controls; > > unsigned int nr_controls; > > + unsigned int reset_duration_us; > > }; > > > > #define to_ti_syscon_reset_data(rcdev) \ > > @@ -89,7 +91,7 @@ static int ti_syscon_reset_assert(struct reset_controller_dev *rcdev, > > mask = BIT(control->assert_bit); > > value = (control->flags & ASSERT_SET) ? mask : 0x0; > > > > - return regmap_update_bits(data->regmap, control->assert_offset, mask, value); > > + return regmap_write_bits(data->regmap, control->assert_offset, mask, value); > > } > > > > /** > > @@ -120,7 +122,7 @@ static int ti_syscon_reset_deassert(struct reset_controller_dev *rcdev, > > mask = BIT(control->deassert_bit); > > value = (control->flags & DEASSERT_SET) ? mask : 0x0; > > > > - return regmap_update_bits(data->regmap, control->deassert_offset, mask, value); > > + return regmap_write_bits(data->regmap, control->deassert_offset, mask, value); > > } > > > > /** > > @@ -158,9 +160,26 @@ static int ti_syscon_reset_status(struct reset_controller_dev *rcdev, > > !(control->flags & STATUS_SET); > > } > > > > +static int ti_syscon_reset(struct reset_controller_dev *rcdev, > > + unsigned long id) > > +{ > > + struct ti_syscon_reset_data *data = to_ti_syscon_reset_data(rcdev); > > + int ret; > > + > > + ret = ti_syscon_reset_assert(rcdev, id); > > + if (ret) > > + return ret; > > + > > + if (data->reset_duration_us) > > + usleep_range(data->reset_duration_us, data->reset_duration_us * 2); > > + > > + return ti_syscon_reset_deassert(rcdev, id); > > I echo Philipp's comments [1] from your original v1 series about this. We don't > need a property to distinguish this, but you could add a flag using match data > and Mediatek compatible, and use that within this function, or optionally set > this ops based on compatible (whatever is preferred by Philipp). > > regards > Suman > > [1] https://patchwork.kernel.org/comment/23519193/ > Hi Suman, Philipp Which method would you recommend more? 1. like v2 patch, but assign the flag "data->assert_deassert_together" directly (maybe rename "assert_deassert_together" to "reset_op_available") 2. use Mediatek compatible to decide the reset handler available or not. Thanks Crystal > > +} > > + > > static const struct reset_control_ops ti_syscon_reset_ops = { > > .assert = ti_syscon_reset_assert, > > .deassert = ti_syscon_reset_deassert, > > + .reset = ti_syscon_reset, > > .status = ti_syscon_reset_status, > > }; > > > > @@ -204,6 +223,9 @@ static int ti_syscon_reset_probe(struct platform_device *pdev) > > controls[i].flags = be32_to_cpup(list++); > > } > > > > + of_property_read_u32(pdev->dev.of_node, "reset-duration-us", > > + &data->reset_duration_us); > > + > > data->rcdev.ops = &ti_syscon_reset_ops; > > data->rcdev.owner = THIS_MODULE; > > data->rcdev.of_node = np; > > > ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,3/4] reset-controller: ti: introduce a new reset handler @ 2020-09-09 2:57 ` Crystal Guo 0 siblings, 0 replies; 82+ messages in thread From: Crystal Guo @ 2020-09-09 2:57 UTC (permalink / raw) To: Suman Anna, p.zabel Cc: devicetree, Yong Liang (梁勇), srv_heupstream, Seiya Wang (王迺君), linux-kernel, Fan Chen (陳凡), robh+dt, linux-mediatek, matthias.bgg, Yingjoe Chen (陳英洲), Stanley Chu (朱原陞), linux-arm-kernel On Thu, 2020-09-03 at 07:40 +0800, Suman Anna wrote: > Hi Crystal, > > On 8/16/20 10:03 PM, Crystal Guo wrote: > > Introduce ti_syscon_reset() to integrate assert and deassert together. > > If some modules need do serialized assert and deassert operations > > to reset itself, reset_control_reset can be called for convenience. > > There are multiple changes in this same patch. I think you should split this > functionality away from the change for the regmap_update_bits() to > regmap_write_bits(), similar to what you have done in your v2 Patch 4. > Thanks for your suggestion. I will split this patch in the next version. > > > > Such as reset-qcom-aoss.c, it integrates assert and deassert together > > by 'reset' method. MTK Socs also need this method to perform reset. > > > > Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> > > --- > > drivers/reset/reset-ti-syscon.c | 26 ++++++++++++++++++++++++-- > > 1 file changed, 24 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/reset/reset-ti-syscon.c b/drivers/reset/reset-ti-syscon.c > > index a2635c21db7f..08289342f9af 100644 > > --- a/drivers/reset/reset-ti-syscon.c > > +++ b/drivers/reset/reset-ti-syscon.c > > @@ -15,6 +15,7 @@ > > * GNU General Public License for more details. > > */ > > > > +#include <linux/delay.h> > > #include <linux/mfd/syscon.h> > > #include <linux/module.h> > > #include <linux/of.h> > > @@ -56,6 +57,7 @@ struct ti_syscon_reset_data { > > struct regmap *regmap; > > struct ti_syscon_reset_control *controls; > > unsigned int nr_controls; > > + unsigned int reset_duration_us; > > }; > > > > #define to_ti_syscon_reset_data(rcdev) \ > > @@ -89,7 +91,7 @@ static int ti_syscon_reset_assert(struct reset_controller_dev *rcdev, > > mask = BIT(control->assert_bit); > > value = (control->flags & ASSERT_SET) ? mask : 0x0; > > > > - return regmap_update_bits(data->regmap, control->assert_offset, mask, value); > > + return regmap_write_bits(data->regmap, control->assert_offset, mask, value); > > } > > > > /** > > @@ -120,7 +122,7 @@ static int ti_syscon_reset_deassert(struct reset_controller_dev *rcdev, > > mask = BIT(control->deassert_bit); > > value = (control->flags & DEASSERT_SET) ? mask : 0x0; > > > > - return regmap_update_bits(data->regmap, control->deassert_offset, mask, value); > > + return regmap_write_bits(data->regmap, control->deassert_offset, mask, value); > > } > > > > /** > > @@ -158,9 +160,26 @@ static int ti_syscon_reset_status(struct reset_controller_dev *rcdev, > > !(control->flags & STATUS_SET); > > } > > > > +static int ti_syscon_reset(struct reset_controller_dev *rcdev, > > + unsigned long id) > > +{ > > + struct ti_syscon_reset_data *data = to_ti_syscon_reset_data(rcdev); > > + int ret; > > + > > + ret = ti_syscon_reset_assert(rcdev, id); > > + if (ret) > > + return ret; > > + > > + if (data->reset_duration_us) > > + usleep_range(data->reset_duration_us, data->reset_duration_us * 2); > > + > > + return ti_syscon_reset_deassert(rcdev, id); > > I echo Philipp's comments [1] from your original v1 series about this. We don't > need a property to distinguish this, but you could add a flag using match data > and Mediatek compatible, and use that within this function, or optionally set > this ops based on compatible (whatever is preferred by Philipp). > > regards > Suman > > [1] https://patchwork.kernel.org/comment/23519193/ > Hi Suman, Philipp Which method would you recommend more? 1. like v2 patch, but assign the flag "data->assert_deassert_together" directly (maybe rename "assert_deassert_together" to "reset_op_available") 2. use Mediatek compatible to decide the reset handler available or not. Thanks Crystal > > +} > > + > > static const struct reset_control_ops ti_syscon_reset_ops = { > > .assert = ti_syscon_reset_assert, > > .deassert = ti_syscon_reset_deassert, > > + .reset = ti_syscon_reset, > > .status = ti_syscon_reset_status, > > }; > > > > @@ -204,6 +223,9 @@ static int ti_syscon_reset_probe(struct platform_device *pdev) > > controls[i].flags = be32_to_cpup(list++); > > } > > > > + of_property_read_u32(pdev->dev.of_node, "reset-duration-us", > > + &data->reset_duration_us); > > + > > data->rcdev.ops = &ti_syscon_reset_ops; > > data->rcdev.owner = THIS_MODULE; > > data->rcdev.of_node = np; > > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,3/4] reset-controller: ti: introduce a new reset handler @ 2020-09-09 2:57 ` Crystal Guo 0 siblings, 0 replies; 82+ messages in thread From: Crystal Guo @ 2020-09-09 2:57 UTC (permalink / raw) To: Suman Anna, p.zabel Cc: devicetree, Yong Liang (梁勇), srv_heupstream, Seiya Wang (王迺君), linux-kernel, Fan Chen (陳凡), robh+dt, linux-mediatek, matthias.bgg, Yingjoe Chen (陳英洲), Stanley Chu (朱原陞), linux-arm-kernel On Thu, 2020-09-03 at 07:40 +0800, Suman Anna wrote: > Hi Crystal, > > On 8/16/20 10:03 PM, Crystal Guo wrote: > > Introduce ti_syscon_reset() to integrate assert and deassert together. > > If some modules need do serialized assert and deassert operations > > to reset itself, reset_control_reset can be called for convenience. > > There are multiple changes in this same patch. I think you should split this > functionality away from the change for the regmap_update_bits() to > regmap_write_bits(), similar to what you have done in your v2 Patch 4. > Thanks for your suggestion. I will split this patch in the next version. > > > > Such as reset-qcom-aoss.c, it integrates assert and deassert together > > by 'reset' method. MTK Socs also need this method to perform reset. > > > > Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> > > --- > > drivers/reset/reset-ti-syscon.c | 26 ++++++++++++++++++++++++-- > > 1 file changed, 24 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/reset/reset-ti-syscon.c b/drivers/reset/reset-ti-syscon.c > > index a2635c21db7f..08289342f9af 100644 > > --- a/drivers/reset/reset-ti-syscon.c > > +++ b/drivers/reset/reset-ti-syscon.c > > @@ -15,6 +15,7 @@ > > * GNU General Public License for more details. > > */ > > > > +#include <linux/delay.h> > > #include <linux/mfd/syscon.h> > > #include <linux/module.h> > > #include <linux/of.h> > > @@ -56,6 +57,7 @@ struct ti_syscon_reset_data { > > struct regmap *regmap; > > struct ti_syscon_reset_control *controls; > > unsigned int nr_controls; > > + unsigned int reset_duration_us; > > }; > > > > #define to_ti_syscon_reset_data(rcdev) \ > > @@ -89,7 +91,7 @@ static int ti_syscon_reset_assert(struct reset_controller_dev *rcdev, > > mask = BIT(control->assert_bit); > > value = (control->flags & ASSERT_SET) ? mask : 0x0; > > > > - return regmap_update_bits(data->regmap, control->assert_offset, mask, value); > > + return regmap_write_bits(data->regmap, control->assert_offset, mask, value); > > } > > > > /** > > @@ -120,7 +122,7 @@ static int ti_syscon_reset_deassert(struct reset_controller_dev *rcdev, > > mask = BIT(control->deassert_bit); > > value = (control->flags & DEASSERT_SET) ? mask : 0x0; > > > > - return regmap_update_bits(data->regmap, control->deassert_offset, mask, value); > > + return regmap_write_bits(data->regmap, control->deassert_offset, mask, value); > > } > > > > /** > > @@ -158,9 +160,26 @@ static int ti_syscon_reset_status(struct reset_controller_dev *rcdev, > > !(control->flags & STATUS_SET); > > } > > > > +static int ti_syscon_reset(struct reset_controller_dev *rcdev, > > + unsigned long id) > > +{ > > + struct ti_syscon_reset_data *data = to_ti_syscon_reset_data(rcdev); > > + int ret; > > + > > + ret = ti_syscon_reset_assert(rcdev, id); > > + if (ret) > > + return ret; > > + > > + if (data->reset_duration_us) > > + usleep_range(data->reset_duration_us, data->reset_duration_us * 2); > > + > > + return ti_syscon_reset_deassert(rcdev, id); > > I echo Philipp's comments [1] from your original v1 series about this. We don't > need a property to distinguish this, but you could add a flag using match data > and Mediatek compatible, and use that within this function, or optionally set > this ops based on compatible (whatever is preferred by Philipp). > > regards > Suman > > [1] https://patchwork.kernel.org/comment/23519193/ > Hi Suman, Philipp Which method would you recommend more? 1. like v2 patch, but assign the flag "data->assert_deassert_together" directly (maybe rename "assert_deassert_together" to "reset_op_available") 2. use Mediatek compatible to decide the reset handler available or not. Thanks Crystal > > +} > > + > > static const struct reset_control_ops ti_syscon_reset_ops = { > > .assert = ti_syscon_reset_assert, > > .deassert = ti_syscon_reset_deassert, > > + .reset = ti_syscon_reset, > > .status = ti_syscon_reset_status, > > }; > > > > @@ -204,6 +223,9 @@ static int ti_syscon_reset_probe(struct platform_device *pdev) > > controls[i].flags = be32_to_cpup(list++); > > } > > > > + of_property_read_u32(pdev->dev.of_node, "reset-duration-us", > > + &data->reset_duration_us); > > + > > data->rcdev.ops = &ti_syscon_reset_ops; > > data->rcdev.owner = THIS_MODULE; > > data->rcdev.of_node = np; > > > _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,3/4] reset-controller: ti: introduce a new reset handler 2020-09-09 2:57 ` Crystal Guo (?) @ 2020-09-09 15:39 ` Suman Anna -1 siblings, 0 replies; 82+ messages in thread From: Suman Anna @ 2020-09-09 15:39 UTC (permalink / raw) To: Crystal Guo, p.zabel Cc: robh+dt, matthias.bgg, srv_heupstream, linux-mediatek, linux-arm-kernel, linux-kernel, devicetree, Seiya Wang (王迺君), Stanley Chu (朱原陞), Yingjoe Chen (陳英洲), Fan Chen (陳凡), Yong Liang (梁勇) On 9/8/20 9:57 PM, Crystal Guo wrote: > On Thu, 2020-09-03 at 07:40 +0800, Suman Anna wrote: >> Hi Crystal, >> >> On 8/16/20 10:03 PM, Crystal Guo wrote: >>> Introduce ti_syscon_reset() to integrate assert and deassert together. >>> If some modules need do serialized assert and deassert operations >>> to reset itself, reset_control_reset can be called for convenience. >> >> There are multiple changes in this same patch. I think you should split this >> functionality away from the change for the regmap_update_bits() to >> regmap_write_bits(), similar to what you have done in your v2 Patch 4. >> > > Thanks for your suggestion. > I will split this patch in the next version. > >>> >>> Such as reset-qcom-aoss.c, it integrates assert and deassert together >>> by 'reset' method. MTK Socs also need this method to perform reset. >>> >>> Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> >>> --- >>> drivers/reset/reset-ti-syscon.c | 26 ++++++++++++++++++++++++-- >>> 1 file changed, 24 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/reset/reset-ti-syscon.c b/drivers/reset/reset-ti-syscon.c >>> index a2635c21db7f..08289342f9af 100644 >>> --- a/drivers/reset/reset-ti-syscon.c >>> +++ b/drivers/reset/reset-ti-syscon.c >>> @@ -15,6 +15,7 @@ >>> * GNU General Public License for more details. >>> */ >>> >>> +#include <linux/delay.h> >>> #include <linux/mfd/syscon.h> >>> #include <linux/module.h> >>> #include <linux/of.h> >>> @@ -56,6 +57,7 @@ struct ti_syscon_reset_data { >>> struct regmap *regmap; >>> struct ti_syscon_reset_control *controls; >>> unsigned int nr_controls; >>> + unsigned int reset_duration_us; >>> }; >>> >>> #define to_ti_syscon_reset_data(rcdev) \ >>> @@ -89,7 +91,7 @@ static int ti_syscon_reset_assert(struct reset_controller_dev *rcdev, >>> mask = BIT(control->assert_bit); >>> value = (control->flags & ASSERT_SET) ? mask : 0x0; >>> >>> - return regmap_update_bits(data->regmap, control->assert_offset, mask, value); >>> + return regmap_write_bits(data->regmap, control->assert_offset, mask, value); >>> } >>> >>> /** >>> @@ -120,7 +122,7 @@ static int ti_syscon_reset_deassert(struct reset_controller_dev *rcdev, >>> mask = BIT(control->deassert_bit); >>> value = (control->flags & DEASSERT_SET) ? mask : 0x0; >>> >>> - return regmap_update_bits(data->regmap, control->deassert_offset, mask, value); >>> + return regmap_write_bits(data->regmap, control->deassert_offset, mask, value); >>> } >>> >>> /** >>> @@ -158,9 +160,26 @@ static int ti_syscon_reset_status(struct reset_controller_dev *rcdev, >>> !(control->flags & STATUS_SET); >>> } >>> >>> +static int ti_syscon_reset(struct reset_controller_dev *rcdev, >>> + unsigned long id) >>> +{ >>> + struct ti_syscon_reset_data *data = to_ti_syscon_reset_data(rcdev); >>> + int ret; >>> + >>> + ret = ti_syscon_reset_assert(rcdev, id); >>> + if (ret) >>> + return ret; >>> + >>> + if (data->reset_duration_us) >>> + usleep_range(data->reset_duration_us, data->reset_duration_us * 2); >>> + >>> + return ti_syscon_reset_deassert(rcdev, id); >> >> I echo Philipp's comments [1] from your original v1 series about this. We don't >> need a property to distinguish this, but you could add a flag using match data >> and Mediatek compatible, and use that within this function, or optionally set >> this ops based on compatible (whatever is preferred by Philipp). >> >> regards >> Suman >> >> [1] https://patchwork.kernel.org/comment/23519193/ >> > Hi Suman, Philipp > > Which method would you recommend more? > 1. like v2 patch, but assign the flag "data->assert_deassert_together" > directly (maybe rename "assert_deassert_together" to > "reset_op_available") > > 2. use Mediatek compatible to decide the reset handler available or not. I would go with this option. Anyway, I think you might have to add the reset SoC data as well, based on Rob's comment on the binding. regards Suman > > Thanks > Crystal > >>> +} >>> + >>> static const struct reset_control_ops ti_syscon_reset_ops = { >>> .assert = ti_syscon_reset_assert, >>> .deassert = ti_syscon_reset_deassert, >>> + .reset = ti_syscon_reset, >>> .status = ti_syscon_reset_status, >>> }; >>> >>> @@ -204,6 +223,9 @@ static int ti_syscon_reset_probe(struct platform_device *pdev) >>> controls[i].flags = be32_to_cpup(list++); >>> } >>> >>> + of_property_read_u32(pdev->dev.of_node, "reset-duration-us", >>> + &data->reset_duration_us); >>> + >>> data->rcdev.ops = &ti_syscon_reset_ops; >>> data->rcdev.owner = THIS_MODULE; >>> data->rcdev.of_node = np; >>> >> > ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,3/4] reset-controller: ti: introduce a new reset handler @ 2020-09-09 15:39 ` Suman Anna 0 siblings, 0 replies; 82+ messages in thread From: Suman Anna @ 2020-09-09 15:39 UTC (permalink / raw) To: Crystal Guo, p.zabel Cc: devicetree, Yong Liang (梁勇), srv_heupstream, Seiya Wang (王迺君), linux-kernel, Fan Chen (陳凡), robh+dt, linux-mediatek, matthias.bgg, Yingjoe Chen (陳英洲), Stanley Chu (朱原陞), linux-arm-kernel On 9/8/20 9:57 PM, Crystal Guo wrote: > On Thu, 2020-09-03 at 07:40 +0800, Suman Anna wrote: >> Hi Crystal, >> >> On 8/16/20 10:03 PM, Crystal Guo wrote: >>> Introduce ti_syscon_reset() to integrate assert and deassert together. >>> If some modules need do serialized assert and deassert operations >>> to reset itself, reset_control_reset can be called for convenience. >> >> There are multiple changes in this same patch. I think you should split this >> functionality away from the change for the regmap_update_bits() to >> regmap_write_bits(), similar to what you have done in your v2 Patch 4. >> > > Thanks for your suggestion. > I will split this patch in the next version. > >>> >>> Such as reset-qcom-aoss.c, it integrates assert and deassert together >>> by 'reset' method. MTK Socs also need this method to perform reset. >>> >>> Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> >>> --- >>> drivers/reset/reset-ti-syscon.c | 26 ++++++++++++++++++++++++-- >>> 1 file changed, 24 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/reset/reset-ti-syscon.c b/drivers/reset/reset-ti-syscon.c >>> index a2635c21db7f..08289342f9af 100644 >>> --- a/drivers/reset/reset-ti-syscon.c >>> +++ b/drivers/reset/reset-ti-syscon.c >>> @@ -15,6 +15,7 @@ >>> * GNU General Public License for more details. >>> */ >>> >>> +#include <linux/delay.h> >>> #include <linux/mfd/syscon.h> >>> #include <linux/module.h> >>> #include <linux/of.h> >>> @@ -56,6 +57,7 @@ struct ti_syscon_reset_data { >>> struct regmap *regmap; >>> struct ti_syscon_reset_control *controls; >>> unsigned int nr_controls; >>> + unsigned int reset_duration_us; >>> }; >>> >>> #define to_ti_syscon_reset_data(rcdev) \ >>> @@ -89,7 +91,7 @@ static int ti_syscon_reset_assert(struct reset_controller_dev *rcdev, >>> mask = BIT(control->assert_bit); >>> value = (control->flags & ASSERT_SET) ? mask : 0x0; >>> >>> - return regmap_update_bits(data->regmap, control->assert_offset, mask, value); >>> + return regmap_write_bits(data->regmap, control->assert_offset, mask, value); >>> } >>> >>> /** >>> @@ -120,7 +122,7 @@ static int ti_syscon_reset_deassert(struct reset_controller_dev *rcdev, >>> mask = BIT(control->deassert_bit); >>> value = (control->flags & DEASSERT_SET) ? mask : 0x0; >>> >>> - return regmap_update_bits(data->regmap, control->deassert_offset, mask, value); >>> + return regmap_write_bits(data->regmap, control->deassert_offset, mask, value); >>> } >>> >>> /** >>> @@ -158,9 +160,26 @@ static int ti_syscon_reset_status(struct reset_controller_dev *rcdev, >>> !(control->flags & STATUS_SET); >>> } >>> >>> +static int ti_syscon_reset(struct reset_controller_dev *rcdev, >>> + unsigned long id) >>> +{ >>> + struct ti_syscon_reset_data *data = to_ti_syscon_reset_data(rcdev); >>> + int ret; >>> + >>> + ret = ti_syscon_reset_assert(rcdev, id); >>> + if (ret) >>> + return ret; >>> + >>> + if (data->reset_duration_us) >>> + usleep_range(data->reset_duration_us, data->reset_duration_us * 2); >>> + >>> + return ti_syscon_reset_deassert(rcdev, id); >> >> I echo Philipp's comments [1] from your original v1 series about this. We don't >> need a property to distinguish this, but you could add a flag using match data >> and Mediatek compatible, and use that within this function, or optionally set >> this ops based on compatible (whatever is preferred by Philipp). >> >> regards >> Suman >> >> [1] https://patchwork.kernel.org/comment/23519193/ >> > Hi Suman, Philipp > > Which method would you recommend more? > 1. like v2 patch, but assign the flag "data->assert_deassert_together" > directly (maybe rename "assert_deassert_together" to > "reset_op_available") > > 2. use Mediatek compatible to decide the reset handler available or not. I would go with this option. Anyway, I think you might have to add the reset SoC data as well, based on Rob's comment on the binding. regards Suman > > Thanks > Crystal > >>> +} >>> + >>> static const struct reset_control_ops ti_syscon_reset_ops = { >>> .assert = ti_syscon_reset_assert, >>> .deassert = ti_syscon_reset_deassert, >>> + .reset = ti_syscon_reset, >>> .status = ti_syscon_reset_status, >>> }; >>> >>> @@ -204,6 +223,9 @@ static int ti_syscon_reset_probe(struct platform_device *pdev) >>> controls[i].flags = be32_to_cpup(list++); >>> } >>> >>> + of_property_read_u32(pdev->dev.of_node, "reset-duration-us", >>> + &data->reset_duration_us); >>> + >>> data->rcdev.ops = &ti_syscon_reset_ops; >>> data->rcdev.owner = THIS_MODULE; >>> data->rcdev.of_node = np; >>> >> > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,3/4] reset-controller: ti: introduce a new reset handler @ 2020-09-09 15:39 ` Suman Anna 0 siblings, 0 replies; 82+ messages in thread From: Suman Anna @ 2020-09-09 15:39 UTC (permalink / raw) To: Crystal Guo, p.zabel Cc: devicetree, Yong Liang (梁勇), srv_heupstream, Seiya Wang (王迺君), linux-kernel, Fan Chen (陳凡), robh+dt, linux-mediatek, matthias.bgg, Yingjoe Chen (陳英洲), Stanley Chu (朱原陞), linux-arm-kernel On 9/8/20 9:57 PM, Crystal Guo wrote: > On Thu, 2020-09-03 at 07:40 +0800, Suman Anna wrote: >> Hi Crystal, >> >> On 8/16/20 10:03 PM, Crystal Guo wrote: >>> Introduce ti_syscon_reset() to integrate assert and deassert together. >>> If some modules need do serialized assert and deassert operations >>> to reset itself, reset_control_reset can be called for convenience. >> >> There are multiple changes in this same patch. I think you should split this >> functionality away from the change for the regmap_update_bits() to >> regmap_write_bits(), similar to what you have done in your v2 Patch 4. >> > > Thanks for your suggestion. > I will split this patch in the next version. > >>> >>> Such as reset-qcom-aoss.c, it integrates assert and deassert together >>> by 'reset' method. MTK Socs also need this method to perform reset. >>> >>> Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> >>> --- >>> drivers/reset/reset-ti-syscon.c | 26 ++++++++++++++++++++++++-- >>> 1 file changed, 24 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/reset/reset-ti-syscon.c b/drivers/reset/reset-ti-syscon.c >>> index a2635c21db7f..08289342f9af 100644 >>> --- a/drivers/reset/reset-ti-syscon.c >>> +++ b/drivers/reset/reset-ti-syscon.c >>> @@ -15,6 +15,7 @@ >>> * GNU General Public License for more details. >>> */ >>> >>> +#include <linux/delay.h> >>> #include <linux/mfd/syscon.h> >>> #include <linux/module.h> >>> #include <linux/of.h> >>> @@ -56,6 +57,7 @@ struct ti_syscon_reset_data { >>> struct regmap *regmap; >>> struct ti_syscon_reset_control *controls; >>> unsigned int nr_controls; >>> + unsigned int reset_duration_us; >>> }; >>> >>> #define to_ti_syscon_reset_data(rcdev) \ >>> @@ -89,7 +91,7 @@ static int ti_syscon_reset_assert(struct reset_controller_dev *rcdev, >>> mask = BIT(control->assert_bit); >>> value = (control->flags & ASSERT_SET) ? mask : 0x0; >>> >>> - return regmap_update_bits(data->regmap, control->assert_offset, mask, value); >>> + return regmap_write_bits(data->regmap, control->assert_offset, mask, value); >>> } >>> >>> /** >>> @@ -120,7 +122,7 @@ static int ti_syscon_reset_deassert(struct reset_controller_dev *rcdev, >>> mask = BIT(control->deassert_bit); >>> value = (control->flags & DEASSERT_SET) ? mask : 0x0; >>> >>> - return regmap_update_bits(data->regmap, control->deassert_offset, mask, value); >>> + return regmap_write_bits(data->regmap, control->deassert_offset, mask, value); >>> } >>> >>> /** >>> @@ -158,9 +160,26 @@ static int ti_syscon_reset_status(struct reset_controller_dev *rcdev, >>> !(control->flags & STATUS_SET); >>> } >>> >>> +static int ti_syscon_reset(struct reset_controller_dev *rcdev, >>> + unsigned long id) >>> +{ >>> + struct ti_syscon_reset_data *data = to_ti_syscon_reset_data(rcdev); >>> + int ret; >>> + >>> + ret = ti_syscon_reset_assert(rcdev, id); >>> + if (ret) >>> + return ret; >>> + >>> + if (data->reset_duration_us) >>> + usleep_range(data->reset_duration_us, data->reset_duration_us * 2); >>> + >>> + return ti_syscon_reset_deassert(rcdev, id); >> >> I echo Philipp's comments [1] from your original v1 series about this. We don't >> need a property to distinguish this, but you could add a flag using match data >> and Mediatek compatible, and use that within this function, or optionally set >> this ops based on compatible (whatever is preferred by Philipp). >> >> regards >> Suman >> >> [1] https://patchwork.kernel.org/comment/23519193/ >> > Hi Suman, Philipp > > Which method would you recommend more? > 1. like v2 patch, but assign the flag "data->assert_deassert_together" > directly (maybe rename "assert_deassert_together" to > "reset_op_available") > > 2. use Mediatek compatible to decide the reset handler available or not. I would go with this option. Anyway, I think you might have to add the reset SoC data as well, based on Rob's comment on the binding. regards Suman > > Thanks > Crystal > >>> +} >>> + >>> static const struct reset_control_ops ti_syscon_reset_ops = { >>> .assert = ti_syscon_reset_assert, >>> .deassert = ti_syscon_reset_deassert, >>> + .reset = ti_syscon_reset, >>> .status = ti_syscon_reset_status, >>> }; >>> >>> @@ -204,6 +223,9 @@ static int ti_syscon_reset_probe(struct platform_device *pdev) >>> controls[i].flags = be32_to_cpup(list++); >>> } >>> >>> + of_property_read_u32(pdev->dev.of_node, "reset-duration-us", >>> + &data->reset_duration_us); >>> + >>> data->rcdev.ops = &ti_syscon_reset_ops; >>> data->rcdev.owner = THIS_MODULE; >>> data->rcdev.of_node = np; >>> >> > _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,3/4] reset-controller: ti: introduce a new reset handler 2020-09-09 15:39 ` Suman Anna (?) @ 2020-09-11 2:42 ` Crystal Guo -1 siblings, 0 replies; 82+ messages in thread From: Crystal Guo @ 2020-09-11 2:42 UTC (permalink / raw) To: Suman Anna, p.zabel, robh+dt, matthias.bgg Cc: srv_heupstream, linux-mediatek, linux-arm-kernel, linux-kernel, devicetree, Seiya Wang (王迺君), Stanley Chu (朱原陞), Yingjoe Chen (陳英洲), Fan Chen (陳凡), Yong Liang (梁勇) On Wed, 2020-09-09 at 23:39 +0800, Suman Anna wrote: > On 9/8/20 9:57 PM, Crystal Guo wrote: > > On Thu, 2020-09-03 at 07:40 +0800, Suman Anna wrote: > >> Hi Crystal, > >> > >> On 8/16/20 10:03 PM, Crystal Guo wrote: > >>> Introduce ti_syscon_reset() to integrate assert and deassert together. > >>> If some modules need do serialized assert and deassert operations > >>> to reset itself, reset_control_reset can be called for convenience. > >> > >> There are multiple changes in this same patch. I think you should split this > >> functionality away from the change for the regmap_update_bits() to > >> regmap_write_bits(), similar to what you have done in your v2 Patch 4. > >> > > > > Thanks for your suggestion. > > I will split this patch in the next version. > > > >>> > >>> Such as reset-qcom-aoss.c, it integrates assert and deassert together > >>> by 'reset' method. MTK Socs also need this method to perform reset. > >>> > >>> Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> > >>> --- > >>> drivers/reset/reset-ti-syscon.c | 26 ++++++++++++++++++++++++-- > >>> 1 file changed, 24 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/drivers/reset/reset-ti-syscon.c b/drivers/reset/reset-ti-syscon.c > >>> index a2635c21db7f..08289342f9af 100644 > >>> --- a/drivers/reset/reset-ti-syscon.c > >>> +++ b/drivers/reset/reset-ti-syscon.c > >>> @@ -15,6 +15,7 @@ > >>> * GNU General Public License for more details. > >>> */ > >>> > >>> +#include <linux/delay.h> > >>> #include <linux/mfd/syscon.h> > >>> #include <linux/module.h> > >>> #include <linux/of.h> > >>> @@ -56,6 +57,7 @@ struct ti_syscon_reset_data { > >>> struct regmap *regmap; > >>> struct ti_syscon_reset_control *controls; > >>> unsigned int nr_controls; > >>> + unsigned int reset_duration_us; > >>> }; > >>> > >>> #define to_ti_syscon_reset_data(rcdev) \ > >>> @@ -89,7 +91,7 @@ static int ti_syscon_reset_assert(struct reset_controller_dev *rcdev, > >>> mask = BIT(control->assert_bit); > >>> value = (control->flags & ASSERT_SET) ? mask : 0x0; > >>> > >>> - return regmap_update_bits(data->regmap, control->assert_offset, mask, value); > >>> + return regmap_write_bits(data->regmap, control->assert_offset, mask, value); > >>> } > >>> > >>> /** > >>> @@ -120,7 +122,7 @@ static int ti_syscon_reset_deassert(struct reset_controller_dev *rcdev, > >>> mask = BIT(control->deassert_bit); > >>> value = (control->flags & DEASSERT_SET) ? mask : 0x0; > >>> > >>> - return regmap_update_bits(data->regmap, control->deassert_offset, mask, value); > >>> + return regmap_write_bits(data->regmap, control->deassert_offset, mask, value); > >>> } > >>> > >>> /** > >>> @@ -158,9 +160,26 @@ static int ti_syscon_reset_status(struct reset_controller_dev *rcdev, > >>> !(control->flags & STATUS_SET); > >>> } > >>> > >>> +static int ti_syscon_reset(struct reset_controller_dev *rcdev, > >>> + unsigned long id) > >>> +{ > >>> + struct ti_syscon_reset_data *data = to_ti_syscon_reset_data(rcdev); > >>> + int ret; > >>> + > >>> + ret = ti_syscon_reset_assert(rcdev, id); > >>> + if (ret) > >>> + return ret; > >>> + > >>> + if (data->reset_duration_us) > >>> + usleep_range(data->reset_duration_us, data->reset_duration_us * 2); > >>> + > >>> + return ti_syscon_reset_deassert(rcdev, id); > >> > >> I echo Philipp's comments [1] from your original v1 series about this. We don't > >> need a property to distinguish this, but you could add a flag using match data > >> and Mediatek compatible, and use that within this function, or optionally set > >> this ops based on compatible (whatever is preferred by Philipp). > >> > >> regards > >> Suman > >> > >> [1] https://patchwork.kernel.org/comment/23519193/ > >> > > Hi Suman, Philipp > > > > Which method would you recommend more? > > 1. like v2 patch, but assign the flag "data->assert_deassert_together" > > directly (maybe rename "assert_deassert_together" to > > "reset_op_available") > > > > 2. use Mediatek compatible to decide the reset handler available or not. > > I would go with this option. Anyway, I think you might have to add the reset SoC > data as well, based on Rob's comment on the binding. > > regards > Suman Thanks for your suggestions I will add the following changes in the next version, please correct me if there is any misunderstanding. 1). revert ti-syscon-reset.txt add a new mediatek reset binding doc. 2). split the patch [v4,3/4] with the change for the regmap_update_bits() to regmap_write_bits() and the change to integrate assert and deassert together. 3). add the reset SoC data, which contains the flag "reset_op_available" to decide the reset handler available or not. 4). separate the dts patch from this patch sets > > > > Thanks > > Crystal > > > >>> +} > >>> + > >>> static const struct reset_control_ops ti_syscon_reset_ops = { > >>> .assert = ti_syscon_reset_assert, > >>> .deassert = ti_syscon_reset_deassert, > >>> + .reset = ti_syscon_reset, > >>> .status = ti_syscon_reset_status, > >>> }; > >>> > >>> @@ -204,6 +223,9 @@ static int ti_syscon_reset_probe(struct platform_device *pdev) > >>> controls[i].flags = be32_to_cpup(list++); > >>> } > >>> > >>> + of_property_read_u32(pdev->dev.of_node, "reset-duration-us", > >>> + &data->reset_duration_us); > >>> + > >>> data->rcdev.ops = &ti_syscon_reset_ops; > >>> data->rcdev.owner = THIS_MODULE; > >>> data->rcdev.of_node = np; > >>> > >> > > > ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,3/4] reset-controller: ti: introduce a new reset handler @ 2020-09-11 2:42 ` Crystal Guo 0 siblings, 0 replies; 82+ messages in thread From: Crystal Guo @ 2020-09-11 2:42 UTC (permalink / raw) To: Suman Anna, p.zabel, robh+dt, matthias.bgg Cc: devicetree, Yong Liang (梁勇), srv_heupstream, Seiya Wang (王迺君), linux-kernel, Fan Chen (陳凡), linux-mediatek, Yingjoe Chen (陳英洲), Stanley Chu (朱原陞), linux-arm-kernel On Wed, 2020-09-09 at 23:39 +0800, Suman Anna wrote: > On 9/8/20 9:57 PM, Crystal Guo wrote: > > On Thu, 2020-09-03 at 07:40 +0800, Suman Anna wrote: > >> Hi Crystal, > >> > >> On 8/16/20 10:03 PM, Crystal Guo wrote: > >>> Introduce ti_syscon_reset() to integrate assert and deassert together. > >>> If some modules need do serialized assert and deassert operations > >>> to reset itself, reset_control_reset can be called for convenience. > >> > >> There are multiple changes in this same patch. I think you should split this > >> functionality away from the change for the regmap_update_bits() to > >> regmap_write_bits(), similar to what you have done in your v2 Patch 4. > >> > > > > Thanks for your suggestion. > > I will split this patch in the next version. > > > >>> > >>> Such as reset-qcom-aoss.c, it integrates assert and deassert together > >>> by 'reset' method. MTK Socs also need this method to perform reset. > >>> > >>> Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> > >>> --- > >>> drivers/reset/reset-ti-syscon.c | 26 ++++++++++++++++++++++++-- > >>> 1 file changed, 24 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/drivers/reset/reset-ti-syscon.c b/drivers/reset/reset-ti-syscon.c > >>> index a2635c21db7f..08289342f9af 100644 > >>> --- a/drivers/reset/reset-ti-syscon.c > >>> +++ b/drivers/reset/reset-ti-syscon.c > >>> @@ -15,6 +15,7 @@ > >>> * GNU General Public License for more details. > >>> */ > >>> > >>> +#include <linux/delay.h> > >>> #include <linux/mfd/syscon.h> > >>> #include <linux/module.h> > >>> #include <linux/of.h> > >>> @@ -56,6 +57,7 @@ struct ti_syscon_reset_data { > >>> struct regmap *regmap; > >>> struct ti_syscon_reset_control *controls; > >>> unsigned int nr_controls; > >>> + unsigned int reset_duration_us; > >>> }; > >>> > >>> #define to_ti_syscon_reset_data(rcdev) \ > >>> @@ -89,7 +91,7 @@ static int ti_syscon_reset_assert(struct reset_controller_dev *rcdev, > >>> mask = BIT(control->assert_bit); > >>> value = (control->flags & ASSERT_SET) ? mask : 0x0; > >>> > >>> - return regmap_update_bits(data->regmap, control->assert_offset, mask, value); > >>> + return regmap_write_bits(data->regmap, control->assert_offset, mask, value); > >>> } > >>> > >>> /** > >>> @@ -120,7 +122,7 @@ static int ti_syscon_reset_deassert(struct reset_controller_dev *rcdev, > >>> mask = BIT(control->deassert_bit); > >>> value = (control->flags & DEASSERT_SET) ? mask : 0x0; > >>> > >>> - return regmap_update_bits(data->regmap, control->deassert_offset, mask, value); > >>> + return regmap_write_bits(data->regmap, control->deassert_offset, mask, value); > >>> } > >>> > >>> /** > >>> @@ -158,9 +160,26 @@ static int ti_syscon_reset_status(struct reset_controller_dev *rcdev, > >>> !(control->flags & STATUS_SET); > >>> } > >>> > >>> +static int ti_syscon_reset(struct reset_controller_dev *rcdev, > >>> + unsigned long id) > >>> +{ > >>> + struct ti_syscon_reset_data *data = to_ti_syscon_reset_data(rcdev); > >>> + int ret; > >>> + > >>> + ret = ti_syscon_reset_assert(rcdev, id); > >>> + if (ret) > >>> + return ret; > >>> + > >>> + if (data->reset_duration_us) > >>> + usleep_range(data->reset_duration_us, data->reset_duration_us * 2); > >>> + > >>> + return ti_syscon_reset_deassert(rcdev, id); > >> > >> I echo Philipp's comments [1] from your original v1 series about this. We don't > >> need a property to distinguish this, but you could add a flag using match data > >> and Mediatek compatible, and use that within this function, or optionally set > >> this ops based on compatible (whatever is preferred by Philipp). > >> > >> regards > >> Suman > >> > >> [1] https://patchwork.kernel.org/comment/23519193/ > >> > > Hi Suman, Philipp > > > > Which method would you recommend more? > > 1. like v2 patch, but assign the flag "data->assert_deassert_together" > > directly (maybe rename "assert_deassert_together" to > > "reset_op_available") > > > > 2. use Mediatek compatible to decide the reset handler available or not. > > I would go with this option. Anyway, I think you might have to add the reset SoC > data as well, based on Rob's comment on the binding. > > regards > Suman Thanks for your suggestions I will add the following changes in the next version, please correct me if there is any misunderstanding. 1). revert ti-syscon-reset.txt add a new mediatek reset binding doc. 2). split the patch [v4,3/4] with the change for the regmap_update_bits() to regmap_write_bits() and the change to integrate assert and deassert together. 3). add the reset SoC data, which contains the flag "reset_op_available" to decide the reset handler available or not. 4). separate the dts patch from this patch sets > > > > Thanks > > Crystal > > > >>> +} > >>> + > >>> static const struct reset_control_ops ti_syscon_reset_ops = { > >>> .assert = ti_syscon_reset_assert, > >>> .deassert = ti_syscon_reset_deassert, > >>> + .reset = ti_syscon_reset, > >>> .status = ti_syscon_reset_status, > >>> }; > >>> > >>> @@ -204,6 +223,9 @@ static int ti_syscon_reset_probe(struct platform_device *pdev) > >>> controls[i].flags = be32_to_cpup(list++); > >>> } > >>> > >>> + of_property_read_u32(pdev->dev.of_node, "reset-duration-us", > >>> + &data->reset_duration_us); > >>> + > >>> data->rcdev.ops = &ti_syscon_reset_ops; > >>> data->rcdev.owner = THIS_MODULE; > >>> data->rcdev.of_node = np; > >>> > >> > > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,3/4] reset-controller: ti: introduce a new reset handler @ 2020-09-11 2:42 ` Crystal Guo 0 siblings, 0 replies; 82+ messages in thread From: Crystal Guo @ 2020-09-11 2:42 UTC (permalink / raw) To: Suman Anna, p.zabel, robh+dt, matthias.bgg Cc: devicetree, Yong Liang (梁勇), srv_heupstream, Seiya Wang (王迺君), linux-kernel, Fan Chen (陳凡), linux-mediatek, Yingjoe Chen (陳英洲), Stanley Chu (朱原陞), linux-arm-kernel On Wed, 2020-09-09 at 23:39 +0800, Suman Anna wrote: > On 9/8/20 9:57 PM, Crystal Guo wrote: > > On Thu, 2020-09-03 at 07:40 +0800, Suman Anna wrote: > >> Hi Crystal, > >> > >> On 8/16/20 10:03 PM, Crystal Guo wrote: > >>> Introduce ti_syscon_reset() to integrate assert and deassert together. > >>> If some modules need do serialized assert and deassert operations > >>> to reset itself, reset_control_reset can be called for convenience. > >> > >> There are multiple changes in this same patch. I think you should split this > >> functionality away from the change for the regmap_update_bits() to > >> regmap_write_bits(), similar to what you have done in your v2 Patch 4. > >> > > > > Thanks for your suggestion. > > I will split this patch in the next version. > > > >>> > >>> Such as reset-qcom-aoss.c, it integrates assert and deassert together > >>> by 'reset' method. MTK Socs also need this method to perform reset. > >>> > >>> Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> > >>> --- > >>> drivers/reset/reset-ti-syscon.c | 26 ++++++++++++++++++++++++-- > >>> 1 file changed, 24 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/drivers/reset/reset-ti-syscon.c b/drivers/reset/reset-ti-syscon.c > >>> index a2635c21db7f..08289342f9af 100644 > >>> --- a/drivers/reset/reset-ti-syscon.c > >>> +++ b/drivers/reset/reset-ti-syscon.c > >>> @@ -15,6 +15,7 @@ > >>> * GNU General Public License for more details. > >>> */ > >>> > >>> +#include <linux/delay.h> > >>> #include <linux/mfd/syscon.h> > >>> #include <linux/module.h> > >>> #include <linux/of.h> > >>> @@ -56,6 +57,7 @@ struct ti_syscon_reset_data { > >>> struct regmap *regmap; > >>> struct ti_syscon_reset_control *controls; > >>> unsigned int nr_controls; > >>> + unsigned int reset_duration_us; > >>> }; > >>> > >>> #define to_ti_syscon_reset_data(rcdev) \ > >>> @@ -89,7 +91,7 @@ static int ti_syscon_reset_assert(struct reset_controller_dev *rcdev, > >>> mask = BIT(control->assert_bit); > >>> value = (control->flags & ASSERT_SET) ? mask : 0x0; > >>> > >>> - return regmap_update_bits(data->regmap, control->assert_offset, mask, value); > >>> + return regmap_write_bits(data->regmap, control->assert_offset, mask, value); > >>> } > >>> > >>> /** > >>> @@ -120,7 +122,7 @@ static int ti_syscon_reset_deassert(struct reset_controller_dev *rcdev, > >>> mask = BIT(control->deassert_bit); > >>> value = (control->flags & DEASSERT_SET) ? mask : 0x0; > >>> > >>> - return regmap_update_bits(data->regmap, control->deassert_offset, mask, value); > >>> + return regmap_write_bits(data->regmap, control->deassert_offset, mask, value); > >>> } > >>> > >>> /** > >>> @@ -158,9 +160,26 @@ static int ti_syscon_reset_status(struct reset_controller_dev *rcdev, > >>> !(control->flags & STATUS_SET); > >>> } > >>> > >>> +static int ti_syscon_reset(struct reset_controller_dev *rcdev, > >>> + unsigned long id) > >>> +{ > >>> + struct ti_syscon_reset_data *data = to_ti_syscon_reset_data(rcdev); > >>> + int ret; > >>> + > >>> + ret = ti_syscon_reset_assert(rcdev, id); > >>> + if (ret) > >>> + return ret; > >>> + > >>> + if (data->reset_duration_us) > >>> + usleep_range(data->reset_duration_us, data->reset_duration_us * 2); > >>> + > >>> + return ti_syscon_reset_deassert(rcdev, id); > >> > >> I echo Philipp's comments [1] from your original v1 series about this. We don't > >> need a property to distinguish this, but you could add a flag using match data > >> and Mediatek compatible, and use that within this function, or optionally set > >> this ops based on compatible (whatever is preferred by Philipp). > >> > >> regards > >> Suman > >> > >> [1] https://patchwork.kernel.org/comment/23519193/ > >> > > Hi Suman, Philipp > > > > Which method would you recommend more? > > 1. like v2 patch, but assign the flag "data->assert_deassert_together" > > directly (maybe rename "assert_deassert_together" to > > "reset_op_available") > > > > 2. use Mediatek compatible to decide the reset handler available or not. > > I would go with this option. Anyway, I think you might have to add the reset SoC > data as well, based on Rob's comment on the binding. > > regards > Suman Thanks for your suggestions I will add the following changes in the next version, please correct me if there is any misunderstanding. 1). revert ti-syscon-reset.txt add a new mediatek reset binding doc. 2). split the patch [v4,3/4] with the change for the regmap_update_bits() to regmap_write_bits() and the change to integrate assert and deassert together. 3). add the reset SoC data, which contains the flag "reset_op_available" to decide the reset handler available or not. 4). separate the dts patch from this patch sets > > > > Thanks > > Crystal > > > >>> +} > >>> + > >>> static const struct reset_control_ops ti_syscon_reset_ops = { > >>> .assert = ti_syscon_reset_assert, > >>> .deassert = ti_syscon_reset_deassert, > >>> + .reset = ti_syscon_reset, > >>> .status = ti_syscon_reset_status, > >>> }; > >>> > >>> @@ -204,6 +223,9 @@ static int ti_syscon_reset_probe(struct platform_device *pdev) > >>> controls[i].flags = be32_to_cpup(list++); > >>> } > >>> > >>> + of_property_read_u32(pdev->dev.of_node, "reset-duration-us", > >>> + &data->reset_duration_us); > >>> + > >>> data->rcdev.ops = &ti_syscon_reset_ops; > >>> data->rcdev.owner = THIS_MODULE; > >>> data->rcdev.of_node = np; > >>> > >> > > > _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,3/4] reset-controller: ti: introduce a new reset handler 2020-09-11 2:42 ` Crystal Guo (?) @ 2020-09-11 2:52 ` Suman Anna -1 siblings, 0 replies; 82+ messages in thread From: Suman Anna @ 2020-09-11 2:52 UTC (permalink / raw) To: Crystal Guo, p.zabel, robh+dt, matthias.bgg Cc: srv_heupstream, linux-mediatek, linux-arm-kernel, linux-kernel, devicetree, Seiya Wang (王迺君), Stanley Chu (朱原陞), Yingjoe Chen (陳英洲), Fan Chen (陳凡), Yong Liang (梁勇) On 9/10/20 9:42 PM, Crystal Guo wrote: > On Wed, 2020-09-09 at 23:39 +0800, Suman Anna wrote: >> On 9/8/20 9:57 PM, Crystal Guo wrote: >>> On Thu, 2020-09-03 at 07:40 +0800, Suman Anna wrote: >>>> Hi Crystal, >>>> >>>> On 8/16/20 10:03 PM, Crystal Guo wrote: >>>>> Introduce ti_syscon_reset() to integrate assert and deassert together. >>>>> If some modules need do serialized assert and deassert operations >>>>> to reset itself, reset_control_reset can be called for convenience. >>>> >>>> There are multiple changes in this same patch. I think you should split this >>>> functionality away from the change for the regmap_update_bits() to >>>> regmap_write_bits(), similar to what you have done in your v2 Patch 4. >>>> >>> >>> Thanks for your suggestion. >>> I will split this patch in the next version. >>> >>>>> >>>>> Such as reset-qcom-aoss.c, it integrates assert and deassert together >>>>> by 'reset' method. MTK Socs also need this method to perform reset. >>>>> >>>>> Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> >>>>> --- >>>>> drivers/reset/reset-ti-syscon.c | 26 ++++++++++++++++++++++++-- >>>>> 1 file changed, 24 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/drivers/reset/reset-ti-syscon.c b/drivers/reset/reset-ti-syscon.c >>>>> index a2635c21db7f..08289342f9af 100644 >>>>> --- a/drivers/reset/reset-ti-syscon.c >>>>> +++ b/drivers/reset/reset-ti-syscon.c >>>>> @@ -15,6 +15,7 @@ >>>>> * GNU General Public License for more details. >>>>> */ >>>>> >>>>> +#include <linux/delay.h> >>>>> #include <linux/mfd/syscon.h> >>>>> #include <linux/module.h> >>>>> #include <linux/of.h> >>>>> @@ -56,6 +57,7 @@ struct ti_syscon_reset_data { >>>>> struct regmap *regmap; >>>>> struct ti_syscon_reset_control *controls; >>>>> unsigned int nr_controls; >>>>> + unsigned int reset_duration_us; >>>>> }; >>>>> >>>>> #define to_ti_syscon_reset_data(rcdev) \ >>>>> @@ -89,7 +91,7 @@ static int ti_syscon_reset_assert(struct reset_controller_dev *rcdev, >>>>> mask = BIT(control->assert_bit); >>>>> value = (control->flags & ASSERT_SET) ? mask : 0x0; >>>>> >>>>> - return regmap_update_bits(data->regmap, control->assert_offset, mask, value); >>>>> + return regmap_write_bits(data->regmap, control->assert_offset, mask, value); >>>>> } >>>>> >>>>> /** >>>>> @@ -120,7 +122,7 @@ static int ti_syscon_reset_deassert(struct reset_controller_dev *rcdev, >>>>> mask = BIT(control->deassert_bit); >>>>> value = (control->flags & DEASSERT_SET) ? mask : 0x0; >>>>> >>>>> - return regmap_update_bits(data->regmap, control->deassert_offset, mask, value); >>>>> + return regmap_write_bits(data->regmap, control->deassert_offset, mask, value); >>>>> } >>>>> >>>>> /** >>>>> @@ -158,9 +160,26 @@ static int ti_syscon_reset_status(struct reset_controller_dev *rcdev, >>>>> !(control->flags & STATUS_SET); >>>>> } >>>>> >>>>> +static int ti_syscon_reset(struct reset_controller_dev *rcdev, >>>>> + unsigned long id) >>>>> +{ >>>>> + struct ti_syscon_reset_data *data = to_ti_syscon_reset_data(rcdev); >>>>> + int ret; >>>>> + >>>>> + ret = ti_syscon_reset_assert(rcdev, id); >>>>> + if (ret) >>>>> + return ret; >>>>> + >>>>> + if (data->reset_duration_us) >>>>> + usleep_range(data->reset_duration_us, data->reset_duration_us * 2); >>>>> + >>>>> + return ti_syscon_reset_deassert(rcdev, id); >>>> >>>> I echo Philipp's comments [1] from your original v1 series about this. We don't >>>> need a property to distinguish this, but you could add a flag using match data >>>> and Mediatek compatible, and use that within this function, or optionally set >>>> this ops based on compatible (whatever is preferred by Philipp). >>>> >>>> regards >>>> Suman >>>> >>>> [1] https://patchwork.kernel.org/comment/23519193/ >>>> >>> Hi Suman, Philipp >>> >>> Which method would you recommend more? >>> 1. like v2 patch, but assign the flag "data->assert_deassert_together" >>> directly (maybe rename "assert_deassert_together" to >>> "reset_op_available") >>> >>> 2. use Mediatek compatible to decide the reset handler available or not. >> >> I would go with this option. Anyway, I think you might have to add the reset SoC >> data as well, based on Rob's comment on the binding. >> >> regards >> Suman > > > Thanks for your suggestions > I will add the following changes in the next version, > please correct me if there is any misunderstanding. > 1). revert ti-syscon-reset.txt add a new mediatek reset binding doc. > 2). split the patch [v4,3/4] with the change for the > regmap_update_bits() to regmap_write_bits() and the change to integrate > assert and deassert together. > 3). add the reset SoC data, which contains the flag "reset_op_available" > to decide the reset handler available or not. You would also need to add your SoC-specific data equivalent of the current ti,reset-bits in your infra node. Please see Rob's comments on patch 2. Also, your new Mediatek binding should be in YAML format. regards Suman > 4). separate the dts patch from this patch sets > >>> >>> Thanks >>> Crystal >>> >>>>> +} >>>>> + >>>>> static const struct reset_control_ops ti_syscon_reset_ops = { >>>>> .assert = ti_syscon_reset_assert, >>>>> .deassert = ti_syscon_reset_deassert, >>>>> + .reset = ti_syscon_reset, >>>>> .status = ti_syscon_reset_status, >>>>> }; >>>>> >>>>> @@ -204,6 +223,9 @@ static int ti_syscon_reset_probe(struct platform_device *pdev) >>>>> controls[i].flags = be32_to_cpup(list++); >>>>> } >>>>> >>>>> + of_property_read_u32(pdev->dev.of_node, "reset-duration-us", >>>>> + &data->reset_duration_us); >>>>> + >>>>> data->rcdev.ops = &ti_syscon_reset_ops; >>>>> data->rcdev.owner = THIS_MODULE; >>>>> data->rcdev.of_node = np; >>>>> >>>> >>> >> > ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,3/4] reset-controller: ti: introduce a new reset handler @ 2020-09-11 2:52 ` Suman Anna 0 siblings, 0 replies; 82+ messages in thread From: Suman Anna @ 2020-09-11 2:52 UTC (permalink / raw) To: Crystal Guo, p.zabel, robh+dt, matthias.bgg Cc: devicetree, Yong Liang (梁勇), srv_heupstream, Seiya Wang (王迺君), linux-kernel, Fan Chen (陳凡), linux-mediatek, Yingjoe Chen (陳英洲), Stanley Chu (朱原陞), linux-arm-kernel On 9/10/20 9:42 PM, Crystal Guo wrote: > On Wed, 2020-09-09 at 23:39 +0800, Suman Anna wrote: >> On 9/8/20 9:57 PM, Crystal Guo wrote: >>> On Thu, 2020-09-03 at 07:40 +0800, Suman Anna wrote: >>>> Hi Crystal, >>>> >>>> On 8/16/20 10:03 PM, Crystal Guo wrote: >>>>> Introduce ti_syscon_reset() to integrate assert and deassert together. >>>>> If some modules need do serialized assert and deassert operations >>>>> to reset itself, reset_control_reset can be called for convenience. >>>> >>>> There are multiple changes in this same patch. I think you should split this >>>> functionality away from the change for the regmap_update_bits() to >>>> regmap_write_bits(), similar to what you have done in your v2 Patch 4. >>>> >>> >>> Thanks for your suggestion. >>> I will split this patch in the next version. >>> >>>>> >>>>> Such as reset-qcom-aoss.c, it integrates assert and deassert together >>>>> by 'reset' method. MTK Socs also need this method to perform reset. >>>>> >>>>> Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> >>>>> --- >>>>> drivers/reset/reset-ti-syscon.c | 26 ++++++++++++++++++++++++-- >>>>> 1 file changed, 24 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/drivers/reset/reset-ti-syscon.c b/drivers/reset/reset-ti-syscon.c >>>>> index a2635c21db7f..08289342f9af 100644 >>>>> --- a/drivers/reset/reset-ti-syscon.c >>>>> +++ b/drivers/reset/reset-ti-syscon.c >>>>> @@ -15,6 +15,7 @@ >>>>> * GNU General Public License for more details. >>>>> */ >>>>> >>>>> +#include <linux/delay.h> >>>>> #include <linux/mfd/syscon.h> >>>>> #include <linux/module.h> >>>>> #include <linux/of.h> >>>>> @@ -56,6 +57,7 @@ struct ti_syscon_reset_data { >>>>> struct regmap *regmap; >>>>> struct ti_syscon_reset_control *controls; >>>>> unsigned int nr_controls; >>>>> + unsigned int reset_duration_us; >>>>> }; >>>>> >>>>> #define to_ti_syscon_reset_data(rcdev) \ >>>>> @@ -89,7 +91,7 @@ static int ti_syscon_reset_assert(struct reset_controller_dev *rcdev, >>>>> mask = BIT(control->assert_bit); >>>>> value = (control->flags & ASSERT_SET) ? mask : 0x0; >>>>> >>>>> - return regmap_update_bits(data->regmap, control->assert_offset, mask, value); >>>>> + return regmap_write_bits(data->regmap, control->assert_offset, mask, value); >>>>> } >>>>> >>>>> /** >>>>> @@ -120,7 +122,7 @@ static int ti_syscon_reset_deassert(struct reset_controller_dev *rcdev, >>>>> mask = BIT(control->deassert_bit); >>>>> value = (control->flags & DEASSERT_SET) ? mask : 0x0; >>>>> >>>>> - return regmap_update_bits(data->regmap, control->deassert_offset, mask, value); >>>>> + return regmap_write_bits(data->regmap, control->deassert_offset, mask, value); >>>>> } >>>>> >>>>> /** >>>>> @@ -158,9 +160,26 @@ static int ti_syscon_reset_status(struct reset_controller_dev *rcdev, >>>>> !(control->flags & STATUS_SET); >>>>> } >>>>> >>>>> +static int ti_syscon_reset(struct reset_controller_dev *rcdev, >>>>> + unsigned long id) >>>>> +{ >>>>> + struct ti_syscon_reset_data *data = to_ti_syscon_reset_data(rcdev); >>>>> + int ret; >>>>> + >>>>> + ret = ti_syscon_reset_assert(rcdev, id); >>>>> + if (ret) >>>>> + return ret; >>>>> + >>>>> + if (data->reset_duration_us) >>>>> + usleep_range(data->reset_duration_us, data->reset_duration_us * 2); >>>>> + >>>>> + return ti_syscon_reset_deassert(rcdev, id); >>>> >>>> I echo Philipp's comments [1] from your original v1 series about this. We don't >>>> need a property to distinguish this, but you could add a flag using match data >>>> and Mediatek compatible, and use that within this function, or optionally set >>>> this ops based on compatible (whatever is preferred by Philipp). >>>> >>>> regards >>>> Suman >>>> >>>> [1] https://patchwork.kernel.org/comment/23519193/ >>>> >>> Hi Suman, Philipp >>> >>> Which method would you recommend more? >>> 1. like v2 patch, but assign the flag "data->assert_deassert_together" >>> directly (maybe rename "assert_deassert_together" to >>> "reset_op_available") >>> >>> 2. use Mediatek compatible to decide the reset handler available or not. >> >> I would go with this option. Anyway, I think you might have to add the reset SoC >> data as well, based on Rob's comment on the binding. >> >> regards >> Suman > > > Thanks for your suggestions > I will add the following changes in the next version, > please correct me if there is any misunderstanding. > 1). revert ti-syscon-reset.txt add a new mediatek reset binding doc. > 2). split the patch [v4,3/4] with the change for the > regmap_update_bits() to regmap_write_bits() and the change to integrate > assert and deassert together. > 3). add the reset SoC data, which contains the flag "reset_op_available" > to decide the reset handler available or not. You would also need to add your SoC-specific data equivalent of the current ti,reset-bits in your infra node. Please see Rob's comments on patch 2. Also, your new Mediatek binding should be in YAML format. regards Suman > 4). separate the dts patch from this patch sets > >>> >>> Thanks >>> Crystal >>> >>>>> +} >>>>> + >>>>> static const struct reset_control_ops ti_syscon_reset_ops = { >>>>> .assert = ti_syscon_reset_assert, >>>>> .deassert = ti_syscon_reset_deassert, >>>>> + .reset = ti_syscon_reset, >>>>> .status = ti_syscon_reset_status, >>>>> }; >>>>> >>>>> @@ -204,6 +223,9 @@ static int ti_syscon_reset_probe(struct platform_device *pdev) >>>>> controls[i].flags = be32_to_cpup(list++); >>>>> } >>>>> >>>>> + of_property_read_u32(pdev->dev.of_node, "reset-duration-us", >>>>> + &data->reset_duration_us); >>>>> + >>>>> data->rcdev.ops = &ti_syscon_reset_ops; >>>>> data->rcdev.owner = THIS_MODULE; >>>>> data->rcdev.of_node = np; >>>>> >>>> >>> >> > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,3/4] reset-controller: ti: introduce a new reset handler @ 2020-09-11 2:52 ` Suman Anna 0 siblings, 0 replies; 82+ messages in thread From: Suman Anna @ 2020-09-11 2:52 UTC (permalink / raw) To: Crystal Guo, p.zabel, robh+dt, matthias.bgg Cc: devicetree, Yong Liang (梁勇), srv_heupstream, Seiya Wang (王迺君), linux-kernel, Fan Chen (陳凡), linux-mediatek, Yingjoe Chen (陳英洲), Stanley Chu (朱原陞), linux-arm-kernel On 9/10/20 9:42 PM, Crystal Guo wrote: > On Wed, 2020-09-09 at 23:39 +0800, Suman Anna wrote: >> On 9/8/20 9:57 PM, Crystal Guo wrote: >>> On Thu, 2020-09-03 at 07:40 +0800, Suman Anna wrote: >>>> Hi Crystal, >>>> >>>> On 8/16/20 10:03 PM, Crystal Guo wrote: >>>>> Introduce ti_syscon_reset() to integrate assert and deassert together. >>>>> If some modules need do serialized assert and deassert operations >>>>> to reset itself, reset_control_reset can be called for convenience. >>>> >>>> There are multiple changes in this same patch. I think you should split this >>>> functionality away from the change for the regmap_update_bits() to >>>> regmap_write_bits(), similar to what you have done in your v2 Patch 4. >>>> >>> >>> Thanks for your suggestion. >>> I will split this patch in the next version. >>> >>>>> >>>>> Such as reset-qcom-aoss.c, it integrates assert and deassert together >>>>> by 'reset' method. MTK Socs also need this method to perform reset. >>>>> >>>>> Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> >>>>> --- >>>>> drivers/reset/reset-ti-syscon.c | 26 ++++++++++++++++++++++++-- >>>>> 1 file changed, 24 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/drivers/reset/reset-ti-syscon.c b/drivers/reset/reset-ti-syscon.c >>>>> index a2635c21db7f..08289342f9af 100644 >>>>> --- a/drivers/reset/reset-ti-syscon.c >>>>> +++ b/drivers/reset/reset-ti-syscon.c >>>>> @@ -15,6 +15,7 @@ >>>>> * GNU General Public License for more details. >>>>> */ >>>>> >>>>> +#include <linux/delay.h> >>>>> #include <linux/mfd/syscon.h> >>>>> #include <linux/module.h> >>>>> #include <linux/of.h> >>>>> @@ -56,6 +57,7 @@ struct ti_syscon_reset_data { >>>>> struct regmap *regmap; >>>>> struct ti_syscon_reset_control *controls; >>>>> unsigned int nr_controls; >>>>> + unsigned int reset_duration_us; >>>>> }; >>>>> >>>>> #define to_ti_syscon_reset_data(rcdev) \ >>>>> @@ -89,7 +91,7 @@ static int ti_syscon_reset_assert(struct reset_controller_dev *rcdev, >>>>> mask = BIT(control->assert_bit); >>>>> value = (control->flags & ASSERT_SET) ? mask : 0x0; >>>>> >>>>> - return regmap_update_bits(data->regmap, control->assert_offset, mask, value); >>>>> + return regmap_write_bits(data->regmap, control->assert_offset, mask, value); >>>>> } >>>>> >>>>> /** >>>>> @@ -120,7 +122,7 @@ static int ti_syscon_reset_deassert(struct reset_controller_dev *rcdev, >>>>> mask = BIT(control->deassert_bit); >>>>> value = (control->flags & DEASSERT_SET) ? mask : 0x0; >>>>> >>>>> - return regmap_update_bits(data->regmap, control->deassert_offset, mask, value); >>>>> + return regmap_write_bits(data->regmap, control->deassert_offset, mask, value); >>>>> } >>>>> >>>>> /** >>>>> @@ -158,9 +160,26 @@ static int ti_syscon_reset_status(struct reset_controller_dev *rcdev, >>>>> !(control->flags & STATUS_SET); >>>>> } >>>>> >>>>> +static int ti_syscon_reset(struct reset_controller_dev *rcdev, >>>>> + unsigned long id) >>>>> +{ >>>>> + struct ti_syscon_reset_data *data = to_ti_syscon_reset_data(rcdev); >>>>> + int ret; >>>>> + >>>>> + ret = ti_syscon_reset_assert(rcdev, id); >>>>> + if (ret) >>>>> + return ret; >>>>> + >>>>> + if (data->reset_duration_us) >>>>> + usleep_range(data->reset_duration_us, data->reset_duration_us * 2); >>>>> + >>>>> + return ti_syscon_reset_deassert(rcdev, id); >>>> >>>> I echo Philipp's comments [1] from your original v1 series about this. We don't >>>> need a property to distinguish this, but you could add a flag using match data >>>> and Mediatek compatible, and use that within this function, or optionally set >>>> this ops based on compatible (whatever is preferred by Philipp). >>>> >>>> regards >>>> Suman >>>> >>>> [1] https://patchwork.kernel.org/comment/23519193/ >>>> >>> Hi Suman, Philipp >>> >>> Which method would you recommend more? >>> 1. like v2 patch, but assign the flag "data->assert_deassert_together" >>> directly (maybe rename "assert_deassert_together" to >>> "reset_op_available") >>> >>> 2. use Mediatek compatible to decide the reset handler available or not. >> >> I would go with this option. Anyway, I think you might have to add the reset SoC >> data as well, based on Rob's comment on the binding. >> >> regards >> Suman > > > Thanks for your suggestions > I will add the following changes in the next version, > please correct me if there is any misunderstanding. > 1). revert ti-syscon-reset.txt add a new mediatek reset binding doc. > 2). split the patch [v4,3/4] with the change for the > regmap_update_bits() to regmap_write_bits() and the change to integrate > assert and deassert together. > 3). add the reset SoC data, which contains the flag "reset_op_available" > to decide the reset handler available or not. You would also need to add your SoC-specific data equivalent of the current ti,reset-bits in your infra node. Please see Rob's comments on patch 2. Also, your new Mediatek binding should be in YAML format. regards Suman > 4). separate the dts patch from this patch sets > >>> >>> Thanks >>> Crystal >>> >>>>> +} >>>>> + >>>>> static const struct reset_control_ops ti_syscon_reset_ops = { >>>>> .assert = ti_syscon_reset_assert, >>>>> .deassert = ti_syscon_reset_deassert, >>>>> + .reset = ti_syscon_reset, >>>>> .status = ti_syscon_reset_status, >>>>> }; >>>>> >>>>> @@ -204,6 +223,9 @@ static int ti_syscon_reset_probe(struct platform_device *pdev) >>>>> controls[i].flags = be32_to_cpup(list++); >>>>> } >>>>> >>>>> + of_property_read_u32(pdev->dev.of_node, "reset-duration-us", >>>>> + &data->reset_duration_us); >>>>> + >>>>> data->rcdev.ops = &ti_syscon_reset_ops; >>>>> data->rcdev.owner = THIS_MODULE; >>>>> data->rcdev.of_node = np; >>>>> >>>> >>> >> > _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,3/4] reset-controller: ti: introduce a new reset handler 2020-09-11 2:52 ` Suman Anna (?) @ 2020-09-11 6:07 ` Crystal Guo -1 siblings, 0 replies; 82+ messages in thread From: Crystal Guo @ 2020-09-11 6:07 UTC (permalink / raw) To: Suman Anna Cc: p.zabel, robh+dt, matthias.bgg, srv_heupstream, linux-mediatek, linux-arm-kernel, linux-kernel, devicetree, Seiya Wang (王迺君), Stanley Chu (朱原陞), Yingjoe Chen (陳英洲), Fan Chen (陳凡), Yong Liang (梁勇) On Fri, 2020-09-11 at 10:52 +0800, Suman Anna wrote: > On 9/10/20 9:42 PM, Crystal Guo wrote: > > On Wed, 2020-09-09 at 23:39 +0800, Suman Anna wrote: > >> On 9/8/20 9:57 PM, Crystal Guo wrote: > >>> On Thu, 2020-09-03 at 07:40 +0800, Suman Anna wrote: > >>>> Hi Crystal, > >>>> > >>>> On 8/16/20 10:03 PM, Crystal Guo wrote: > >>>>> Introduce ti_syscon_reset() to integrate assert and deassert together. > >>>>> If some modules need do serialized assert and deassert operations > >>>>> to reset itself, reset_control_reset can be called for convenience. > >>>> > >>>> There are multiple changes in this same patch. I think you should split this > >>>> functionality away from the change for the regmap_update_bits() to > >>>> regmap_write_bits(), similar to what you have done in your v2 Patch 4. > >>>> > >>> > >>> Thanks for your suggestion. > >>> I will split this patch in the next version. > >>> > >>>>> > >>>>> Such as reset-qcom-aoss.c, it integrates assert and deassert together > >>>>> by 'reset' method. MTK Socs also need this method to perform reset. > >>>>> > >>>>> Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> > >>>>> --- > >>>>> drivers/reset/reset-ti-syscon.c | 26 ++++++++++++++++++++++++-- > >>>>> 1 file changed, 24 insertions(+), 2 deletions(-) > >>>>> > >>>>> diff --git a/drivers/reset/reset-ti-syscon.c b/drivers/reset/reset-ti-syscon.c > >>>>> index a2635c21db7f..08289342f9af 100644 > >>>>> --- a/drivers/reset/reset-ti-syscon.c > >>>>> +++ b/drivers/reset/reset-ti-syscon.c > >>>>> @@ -15,6 +15,7 @@ > >>>>> * GNU General Public License for more details. > >>>>> */ > >>>>> > >>>>> +#include <linux/delay.h> > >>>>> #include <linux/mfd/syscon.h> > >>>>> #include <linux/module.h> > >>>>> #include <linux/of.h> > >>>>> @@ -56,6 +57,7 @@ struct ti_syscon_reset_data { > >>>>> struct regmap *regmap; > >>>>> struct ti_syscon_reset_control *controls; > >>>>> unsigned int nr_controls; > >>>>> + unsigned int reset_duration_us; > >>>>> }; > >>>>> > >>>>> #define to_ti_syscon_reset_data(rcdev) \ > >>>>> @@ -89,7 +91,7 @@ static int ti_syscon_reset_assert(struct reset_controller_dev *rcdev, > >>>>> mask = BIT(control->assert_bit); > >>>>> value = (control->flags & ASSERT_SET) ? mask : 0x0; > >>>>> > >>>>> - return regmap_update_bits(data->regmap, control->assert_offset, mask, value); > >>>>> + return regmap_write_bits(data->regmap, control->assert_offset, mask, value); > >>>>> } > >>>>> > >>>>> /** > >>>>> @@ -120,7 +122,7 @@ static int ti_syscon_reset_deassert(struct reset_controller_dev *rcdev, > >>>>> mask = BIT(control->deassert_bit); > >>>>> value = (control->flags & DEASSERT_SET) ? mask : 0x0; > >>>>> > >>>>> - return regmap_update_bits(data->regmap, control->deassert_offset, mask, value); > >>>>> + return regmap_write_bits(data->regmap, control->deassert_offset, mask, value); > >>>>> } > >>>>> > >>>>> /** > >>>>> @@ -158,9 +160,26 @@ static int ti_syscon_reset_status(struct reset_controller_dev *rcdev, > >>>>> !(control->flags & STATUS_SET); > >>>>> } > >>>>> > >>>>> +static int ti_syscon_reset(struct reset_controller_dev *rcdev, > >>>>> + unsigned long id) > >>>>> +{ > >>>>> + struct ti_syscon_reset_data *data = to_ti_syscon_reset_data(rcdev); > >>>>> + int ret; > >>>>> + > >>>>> + ret = ti_syscon_reset_assert(rcdev, id); > >>>>> + if (ret) > >>>>> + return ret; > >>>>> + > >>>>> + if (data->reset_duration_us) > >>>>> + usleep_range(data->reset_duration_us, data->reset_duration_us * 2); > >>>>> + > >>>>> + return ti_syscon_reset_deassert(rcdev, id); > >>>> > >>>> I echo Philipp's comments [1] from your original v1 series about this. We don't > >>>> need a property to distinguish this, but you could add a flag using match data > >>>> and Mediatek compatible, and use that within this function, or optionally set > >>>> this ops based on compatible (whatever is preferred by Philipp). > >>>> > >>>> regards > >>>> Suman > >>>> > >>>> [1] https://patchwork.kernel.org/comment/23519193/ > >>>> > >>> Hi Suman, Philipp > >>> > >>> Which method would you recommend more? > >>> 1. like v2 patch, but assign the flag "data->assert_deassert_together" > >>> directly (maybe rename "assert_deassert_together" to > >>> "reset_op_available") > >>> > >>> 2. use Mediatek compatible to decide the reset handler available or not. > >> > >> I would go with this option. Anyway, I think you might have to add the reset SoC > >> data as well, based on Rob's comment on the binding. > >> > >> regards > >> Suman > > > > > > Thanks for your suggestions > > I will add the following changes in the next version, > > please correct me if there is any misunderstanding. > > 1). revert ti-syscon-reset.txt add a new mediatek reset binding doc. > > 2). split the patch [v4,3/4] with the change for the > > regmap_update_bits() to regmap_write_bits() and the change to integrate > > assert and deassert together. > > 3). add the reset SoC data, which contains the flag "reset_op_available" > > to decide the reset handler available or not. > > You would also need to add your SoC-specific data equivalent of the current > ti,reset-bits in your infra node. Please see Rob's comments on patch 2. Also, > your new Mediatek binding should be in YAML format. > > regards > Suman > Should I add the SoC-specific data as follows? This may also modify the ti original code, is it OK? + data->reset_data = of_device_get_match_data(&pdev->dev); + + list = of_get_property(np, data->reset_data->reset_bits, &size); +static const struct common_reset_data ti_reset_data = { + .reset_op_available = false, + .reset_bits = "ti, reset-bits", +}; + +static const struct common_reset_data mediatek_reset_data = { + .reset_op_available = true, + .reset_bits = "mediatek, reset-bits", +}; + static const struct of_device_id ti_syscon_reset_of_match[] = { - { .compatible = "ti,syscon-reset", }, + { .compatible = "ti,syscon-reset", .data = &ti_reset_data}, + { .compatible = "mediatek,syscon-reset", .data = &mediatek_reset_data}, { /* sentinel */ }, }; Thanks Crystal > > 4). separate the dts patch from this patch sets > > > >>> > >>> Thanks > >>> Crystal > >>> > >>>>> +} > >>>>> + > >>>>> static const struct reset_control_ops ti_syscon_reset_ops = { > >>>>> .assert = ti_syscon_reset_assert, > >>>>> .deassert = ti_syscon_reset_deassert, > >>>>> + .reset = ti_syscon_reset, > >>>>> .status = ti_syscon_reset_status, > >>>>> }; > >>>>> > >>>>> @@ -204,6 +223,9 @@ static int ti_syscon_reset_probe(struct platform_device *pdev) > >>>>> controls[i].flags = be32_to_cpup(list++); > >>>>> } > >>>>> > >>>>> + of_property_read_u32(pdev->dev.of_node, "reset-duration-us", > >>>>> + &data->reset_duration_us); > >>>>> + > >>>>> data->rcdev.ops = &ti_syscon_reset_ops; > >>>>> data->rcdev.owner = THIS_MODULE; > >>>>> data->rcdev.of_node = np; > >>>>> > >>>> > >>> > >> > > > ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,3/4] reset-controller: ti: introduce a new reset handler @ 2020-09-11 6:07 ` Crystal Guo 0 siblings, 0 replies; 82+ messages in thread From: Crystal Guo @ 2020-09-11 6:07 UTC (permalink / raw) To: Suman Anna Cc: devicetree, Yong Liang (梁勇), srv_heupstream, Seiya Wang (王迺君), linux-kernel, Fan Chen (陳凡), robh+dt, linux-mediatek, p.zabel, matthias.bgg, Yingjoe Chen (陳英洲), Stanley Chu (朱原陞), linux-arm-kernel On Fri, 2020-09-11 at 10:52 +0800, Suman Anna wrote: > On 9/10/20 9:42 PM, Crystal Guo wrote: > > On Wed, 2020-09-09 at 23:39 +0800, Suman Anna wrote: > >> On 9/8/20 9:57 PM, Crystal Guo wrote: > >>> On Thu, 2020-09-03 at 07:40 +0800, Suman Anna wrote: > >>>> Hi Crystal, > >>>> > >>>> On 8/16/20 10:03 PM, Crystal Guo wrote: > >>>>> Introduce ti_syscon_reset() to integrate assert and deassert together. > >>>>> If some modules need do serialized assert and deassert operations > >>>>> to reset itself, reset_control_reset can be called for convenience. > >>>> > >>>> There are multiple changes in this same patch. I think you should split this > >>>> functionality away from the change for the regmap_update_bits() to > >>>> regmap_write_bits(), similar to what you have done in your v2 Patch 4. > >>>> > >>> > >>> Thanks for your suggestion. > >>> I will split this patch in the next version. > >>> > >>>>> > >>>>> Such as reset-qcom-aoss.c, it integrates assert and deassert together > >>>>> by 'reset' method. MTK Socs also need this method to perform reset. > >>>>> > >>>>> Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> > >>>>> --- > >>>>> drivers/reset/reset-ti-syscon.c | 26 ++++++++++++++++++++++++-- > >>>>> 1 file changed, 24 insertions(+), 2 deletions(-) > >>>>> > >>>>> diff --git a/drivers/reset/reset-ti-syscon.c b/drivers/reset/reset-ti-syscon.c > >>>>> index a2635c21db7f..08289342f9af 100644 > >>>>> --- a/drivers/reset/reset-ti-syscon.c > >>>>> +++ b/drivers/reset/reset-ti-syscon.c > >>>>> @@ -15,6 +15,7 @@ > >>>>> * GNU General Public License for more details. > >>>>> */ > >>>>> > >>>>> +#include <linux/delay.h> > >>>>> #include <linux/mfd/syscon.h> > >>>>> #include <linux/module.h> > >>>>> #include <linux/of.h> > >>>>> @@ -56,6 +57,7 @@ struct ti_syscon_reset_data { > >>>>> struct regmap *regmap; > >>>>> struct ti_syscon_reset_control *controls; > >>>>> unsigned int nr_controls; > >>>>> + unsigned int reset_duration_us; > >>>>> }; > >>>>> > >>>>> #define to_ti_syscon_reset_data(rcdev) \ > >>>>> @@ -89,7 +91,7 @@ static int ti_syscon_reset_assert(struct reset_controller_dev *rcdev, > >>>>> mask = BIT(control->assert_bit); > >>>>> value = (control->flags & ASSERT_SET) ? mask : 0x0; > >>>>> > >>>>> - return regmap_update_bits(data->regmap, control->assert_offset, mask, value); > >>>>> + return regmap_write_bits(data->regmap, control->assert_offset, mask, value); > >>>>> } > >>>>> > >>>>> /** > >>>>> @@ -120,7 +122,7 @@ static int ti_syscon_reset_deassert(struct reset_controller_dev *rcdev, > >>>>> mask = BIT(control->deassert_bit); > >>>>> value = (control->flags & DEASSERT_SET) ? mask : 0x0; > >>>>> > >>>>> - return regmap_update_bits(data->regmap, control->deassert_offset, mask, value); > >>>>> + return regmap_write_bits(data->regmap, control->deassert_offset, mask, value); > >>>>> } > >>>>> > >>>>> /** > >>>>> @@ -158,9 +160,26 @@ static int ti_syscon_reset_status(struct reset_controller_dev *rcdev, > >>>>> !(control->flags & STATUS_SET); > >>>>> } > >>>>> > >>>>> +static int ti_syscon_reset(struct reset_controller_dev *rcdev, > >>>>> + unsigned long id) > >>>>> +{ > >>>>> + struct ti_syscon_reset_data *data = to_ti_syscon_reset_data(rcdev); > >>>>> + int ret; > >>>>> + > >>>>> + ret = ti_syscon_reset_assert(rcdev, id); > >>>>> + if (ret) > >>>>> + return ret; > >>>>> + > >>>>> + if (data->reset_duration_us) > >>>>> + usleep_range(data->reset_duration_us, data->reset_duration_us * 2); > >>>>> + > >>>>> + return ti_syscon_reset_deassert(rcdev, id); > >>>> > >>>> I echo Philipp's comments [1] from your original v1 series about this. We don't > >>>> need a property to distinguish this, but you could add a flag using match data > >>>> and Mediatek compatible, and use that within this function, or optionally set > >>>> this ops based on compatible (whatever is preferred by Philipp). > >>>> > >>>> regards > >>>> Suman > >>>> > >>>> [1] https://patchwork.kernel.org/comment/23519193/ > >>>> > >>> Hi Suman, Philipp > >>> > >>> Which method would you recommend more? > >>> 1. like v2 patch, but assign the flag "data->assert_deassert_together" > >>> directly (maybe rename "assert_deassert_together" to > >>> "reset_op_available") > >>> > >>> 2. use Mediatek compatible to decide the reset handler available or not. > >> > >> I would go with this option. Anyway, I think you might have to add the reset SoC > >> data as well, based on Rob's comment on the binding. > >> > >> regards > >> Suman > > > > > > Thanks for your suggestions > > I will add the following changes in the next version, > > please correct me if there is any misunderstanding. > > 1). revert ti-syscon-reset.txt add a new mediatek reset binding doc. > > 2). split the patch [v4,3/4] with the change for the > > regmap_update_bits() to regmap_write_bits() and the change to integrate > > assert and deassert together. > > 3). add the reset SoC data, which contains the flag "reset_op_available" > > to decide the reset handler available or not. > > You would also need to add your SoC-specific data equivalent of the current > ti,reset-bits in your infra node. Please see Rob's comments on patch 2. Also, > your new Mediatek binding should be in YAML format. > > regards > Suman > Should I add the SoC-specific data as follows? This may also modify the ti original code, is it OK? + data->reset_data = of_device_get_match_data(&pdev->dev); + + list = of_get_property(np, data->reset_data->reset_bits, &size); +static const struct common_reset_data ti_reset_data = { + .reset_op_available = false, + .reset_bits = "ti, reset-bits", +}; + +static const struct common_reset_data mediatek_reset_data = { + .reset_op_available = true, + .reset_bits = "mediatek, reset-bits", +}; + static const struct of_device_id ti_syscon_reset_of_match[] = { - { .compatible = "ti,syscon-reset", }, + { .compatible = "ti,syscon-reset", .data = &ti_reset_data}, + { .compatible = "mediatek,syscon-reset", .data = &mediatek_reset_data}, { /* sentinel */ }, }; Thanks Crystal > > 4). separate the dts patch from this patch sets > > > >>> > >>> Thanks > >>> Crystal > >>> > >>>>> +} > >>>>> + > >>>>> static const struct reset_control_ops ti_syscon_reset_ops = { > >>>>> .assert = ti_syscon_reset_assert, > >>>>> .deassert = ti_syscon_reset_deassert, > >>>>> + .reset = ti_syscon_reset, > >>>>> .status = ti_syscon_reset_status, > >>>>> }; > >>>>> > >>>>> @@ -204,6 +223,9 @@ static int ti_syscon_reset_probe(struct platform_device *pdev) > >>>>> controls[i].flags = be32_to_cpup(list++); > >>>>> } > >>>>> > >>>>> + of_property_read_u32(pdev->dev.of_node, "reset-duration-us", > >>>>> + &data->reset_duration_us); > >>>>> + > >>>>> data->rcdev.ops = &ti_syscon_reset_ops; > >>>>> data->rcdev.owner = THIS_MODULE; > >>>>> data->rcdev.of_node = np; > >>>>> > >>>> > >>> > >> > > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,3/4] reset-controller: ti: introduce a new reset handler @ 2020-09-11 6:07 ` Crystal Guo 0 siblings, 0 replies; 82+ messages in thread From: Crystal Guo @ 2020-09-11 6:07 UTC (permalink / raw) To: Suman Anna Cc: devicetree, Yong Liang (梁勇), srv_heupstream, Seiya Wang (王迺君), linux-kernel, Fan Chen (陳凡), robh+dt, linux-mediatek, p.zabel, matthias.bgg, Yingjoe Chen (陳英洲), Stanley Chu (朱原陞), linux-arm-kernel On Fri, 2020-09-11 at 10:52 +0800, Suman Anna wrote: > On 9/10/20 9:42 PM, Crystal Guo wrote: > > On Wed, 2020-09-09 at 23:39 +0800, Suman Anna wrote: > >> On 9/8/20 9:57 PM, Crystal Guo wrote: > >>> On Thu, 2020-09-03 at 07:40 +0800, Suman Anna wrote: > >>>> Hi Crystal, > >>>> > >>>> On 8/16/20 10:03 PM, Crystal Guo wrote: > >>>>> Introduce ti_syscon_reset() to integrate assert and deassert together. > >>>>> If some modules need do serialized assert and deassert operations > >>>>> to reset itself, reset_control_reset can be called for convenience. > >>>> > >>>> There are multiple changes in this same patch. I think you should split this > >>>> functionality away from the change for the regmap_update_bits() to > >>>> regmap_write_bits(), similar to what you have done in your v2 Patch 4. > >>>> > >>> > >>> Thanks for your suggestion. > >>> I will split this patch in the next version. > >>> > >>>>> > >>>>> Such as reset-qcom-aoss.c, it integrates assert and deassert together > >>>>> by 'reset' method. MTK Socs also need this method to perform reset. > >>>>> > >>>>> Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> > >>>>> --- > >>>>> drivers/reset/reset-ti-syscon.c | 26 ++++++++++++++++++++++++-- > >>>>> 1 file changed, 24 insertions(+), 2 deletions(-) > >>>>> > >>>>> diff --git a/drivers/reset/reset-ti-syscon.c b/drivers/reset/reset-ti-syscon.c > >>>>> index a2635c21db7f..08289342f9af 100644 > >>>>> --- a/drivers/reset/reset-ti-syscon.c > >>>>> +++ b/drivers/reset/reset-ti-syscon.c > >>>>> @@ -15,6 +15,7 @@ > >>>>> * GNU General Public License for more details. > >>>>> */ > >>>>> > >>>>> +#include <linux/delay.h> > >>>>> #include <linux/mfd/syscon.h> > >>>>> #include <linux/module.h> > >>>>> #include <linux/of.h> > >>>>> @@ -56,6 +57,7 @@ struct ti_syscon_reset_data { > >>>>> struct regmap *regmap; > >>>>> struct ti_syscon_reset_control *controls; > >>>>> unsigned int nr_controls; > >>>>> + unsigned int reset_duration_us; > >>>>> }; > >>>>> > >>>>> #define to_ti_syscon_reset_data(rcdev) \ > >>>>> @@ -89,7 +91,7 @@ static int ti_syscon_reset_assert(struct reset_controller_dev *rcdev, > >>>>> mask = BIT(control->assert_bit); > >>>>> value = (control->flags & ASSERT_SET) ? mask : 0x0; > >>>>> > >>>>> - return regmap_update_bits(data->regmap, control->assert_offset, mask, value); > >>>>> + return regmap_write_bits(data->regmap, control->assert_offset, mask, value); > >>>>> } > >>>>> > >>>>> /** > >>>>> @@ -120,7 +122,7 @@ static int ti_syscon_reset_deassert(struct reset_controller_dev *rcdev, > >>>>> mask = BIT(control->deassert_bit); > >>>>> value = (control->flags & DEASSERT_SET) ? mask : 0x0; > >>>>> > >>>>> - return regmap_update_bits(data->regmap, control->deassert_offset, mask, value); > >>>>> + return regmap_write_bits(data->regmap, control->deassert_offset, mask, value); > >>>>> } > >>>>> > >>>>> /** > >>>>> @@ -158,9 +160,26 @@ static int ti_syscon_reset_status(struct reset_controller_dev *rcdev, > >>>>> !(control->flags & STATUS_SET); > >>>>> } > >>>>> > >>>>> +static int ti_syscon_reset(struct reset_controller_dev *rcdev, > >>>>> + unsigned long id) > >>>>> +{ > >>>>> + struct ti_syscon_reset_data *data = to_ti_syscon_reset_data(rcdev); > >>>>> + int ret; > >>>>> + > >>>>> + ret = ti_syscon_reset_assert(rcdev, id); > >>>>> + if (ret) > >>>>> + return ret; > >>>>> + > >>>>> + if (data->reset_duration_us) > >>>>> + usleep_range(data->reset_duration_us, data->reset_duration_us * 2); > >>>>> + > >>>>> + return ti_syscon_reset_deassert(rcdev, id); > >>>> > >>>> I echo Philipp's comments [1] from your original v1 series about this. We don't > >>>> need a property to distinguish this, but you could add a flag using match data > >>>> and Mediatek compatible, and use that within this function, or optionally set > >>>> this ops based on compatible (whatever is preferred by Philipp). > >>>> > >>>> regards > >>>> Suman > >>>> > >>>> [1] https://patchwork.kernel.org/comment/23519193/ > >>>> > >>> Hi Suman, Philipp > >>> > >>> Which method would you recommend more? > >>> 1. like v2 patch, but assign the flag "data->assert_deassert_together" > >>> directly (maybe rename "assert_deassert_together" to > >>> "reset_op_available") > >>> > >>> 2. use Mediatek compatible to decide the reset handler available or not. > >> > >> I would go with this option. Anyway, I think you might have to add the reset SoC > >> data as well, based on Rob's comment on the binding. > >> > >> regards > >> Suman > > > > > > Thanks for your suggestions > > I will add the following changes in the next version, > > please correct me if there is any misunderstanding. > > 1). revert ti-syscon-reset.txt add a new mediatek reset binding doc. > > 2). split the patch [v4,3/4] with the change for the > > regmap_update_bits() to regmap_write_bits() and the change to integrate > > assert and deassert together. > > 3). add the reset SoC data, which contains the flag "reset_op_available" > > to decide the reset handler available or not. > > You would also need to add your SoC-specific data equivalent of the current > ti,reset-bits in your infra node. Please see Rob's comments on patch 2. Also, > your new Mediatek binding should be in YAML format. > > regards > Suman > Should I add the SoC-specific data as follows? This may also modify the ti original code, is it OK? + data->reset_data = of_device_get_match_data(&pdev->dev); + + list = of_get_property(np, data->reset_data->reset_bits, &size); +static const struct common_reset_data ti_reset_data = { + .reset_op_available = false, + .reset_bits = "ti, reset-bits", +}; + +static const struct common_reset_data mediatek_reset_data = { + .reset_op_available = true, + .reset_bits = "mediatek, reset-bits", +}; + static const struct of_device_id ti_syscon_reset_of_match[] = { - { .compatible = "ti,syscon-reset", }, + { .compatible = "ti,syscon-reset", .data = &ti_reset_data}, + { .compatible = "mediatek,syscon-reset", .data = &mediatek_reset_data}, { /* sentinel */ }, }; Thanks Crystal > > 4). separate the dts patch from this patch sets > > > >>> > >>> Thanks > >>> Crystal > >>> > >>>>> +} > >>>>> + > >>>>> static const struct reset_control_ops ti_syscon_reset_ops = { > >>>>> .assert = ti_syscon_reset_assert, > >>>>> .deassert = ti_syscon_reset_deassert, > >>>>> + .reset = ti_syscon_reset, > >>>>> .status = ti_syscon_reset_status, > >>>>> }; > >>>>> > >>>>> @@ -204,6 +223,9 @@ static int ti_syscon_reset_probe(struct platform_device *pdev) > >>>>> controls[i].flags = be32_to_cpup(list++); > >>>>> } > >>>>> > >>>>> + of_property_read_u32(pdev->dev.of_node, "reset-duration-us", > >>>>> + &data->reset_duration_us); > >>>>> + > >>>>> data->rcdev.ops = &ti_syscon_reset_ops; > >>>>> data->rcdev.owner = THIS_MODULE; > >>>>> data->rcdev.of_node = np; > >>>>> > >>>> > >>> > >> > > > _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,3/4] reset-controller: ti: introduce a new reset handler 2020-09-11 6:07 ` Crystal Guo (?) @ 2020-09-11 14:26 ` Philipp Zabel -1 siblings, 0 replies; 82+ messages in thread From: Philipp Zabel @ 2020-09-11 14:26 UTC (permalink / raw) To: Crystal Guo, Suman Anna Cc: robh+dt, matthias.bgg, srv_heupstream, linux-mediatek, linux-arm-kernel, linux-kernel, devicetree, Seiya Wang (王迺君), Stanley Chu (朱原陞), Yingjoe Chen (陳英洲), Fan Chen (陳凡), Yong Liang (梁勇) Hi Crystal, On Fri, 2020-09-11 at 14:07 +0800, Crystal Guo wrote: [...] > Should I add the SoC-specific data as follows? > This may also modify the ti original code, is it OK? > > + data->reset_data = of_device_get_match_data(&pdev->dev); > + > + list = of_get_property(np, data->reset_data->reset_bits, &size); > > +static const struct common_reset_data ti_reset_data = { > + .reset_op_available = false, > + .reset_bits = "ti, reset-bits", ^ That space doesn't belong there. > +}; > + > +static const struct common_reset_data mediatek_reset_data = { > + .reset_op_available = true, > + .reset_bits = "mediatek, reset-bits", > +}; I understand Robs comments as meaning "ti,reset-bits" should have been called "reset-bits" in the first place, and you shouldn't repeat adding the vendor prefix, as that is implied by the compatible. So this should probably be just "reset-bits". Otherwise this looks like it should work. regards Philipp ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,3/4] reset-controller: ti: introduce a new reset handler @ 2020-09-11 14:26 ` Philipp Zabel 0 siblings, 0 replies; 82+ messages in thread From: Philipp Zabel @ 2020-09-11 14:26 UTC (permalink / raw) To: Crystal Guo, Suman Anna Cc: devicetree, Yong Liang (梁勇), srv_heupstream, Seiya Wang (王迺君), linux-kernel, Fan Chen (陳凡), robh+dt, linux-mediatek, matthias.bgg, Yingjoe Chen (陳英洲), Stanley Chu (朱原陞), linux-arm-kernel Hi Crystal, On Fri, 2020-09-11 at 14:07 +0800, Crystal Guo wrote: [...] > Should I add the SoC-specific data as follows? > This may also modify the ti original code, is it OK? > > + data->reset_data = of_device_get_match_data(&pdev->dev); > + > + list = of_get_property(np, data->reset_data->reset_bits, &size); > > +static const struct common_reset_data ti_reset_data = { > + .reset_op_available = false, > + .reset_bits = "ti, reset-bits", ^ That space doesn't belong there. > +}; > + > +static const struct common_reset_data mediatek_reset_data = { > + .reset_op_available = true, > + .reset_bits = "mediatek, reset-bits", > +}; I understand Robs comments as meaning "ti,reset-bits" should have been called "reset-bits" in the first place, and you shouldn't repeat adding the vendor prefix, as that is implied by the compatible. So this should probably be just "reset-bits". Otherwise this looks like it should work. regards Philipp _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,3/4] reset-controller: ti: introduce a new reset handler @ 2020-09-11 14:26 ` Philipp Zabel 0 siblings, 0 replies; 82+ messages in thread From: Philipp Zabel @ 2020-09-11 14:26 UTC (permalink / raw) To: Crystal Guo, Suman Anna Cc: devicetree, Yong Liang (梁勇), srv_heupstream, Seiya Wang (王迺君), linux-kernel, Fan Chen (陳凡), robh+dt, linux-mediatek, matthias.bgg, Yingjoe Chen (陳英洲), Stanley Chu (朱原陞), linux-arm-kernel Hi Crystal, On Fri, 2020-09-11 at 14:07 +0800, Crystal Guo wrote: [...] > Should I add the SoC-specific data as follows? > This may also modify the ti original code, is it OK? > > + data->reset_data = of_device_get_match_data(&pdev->dev); > + > + list = of_get_property(np, data->reset_data->reset_bits, &size); > > +static const struct common_reset_data ti_reset_data = { > + .reset_op_available = false, > + .reset_bits = "ti, reset-bits", ^ That space doesn't belong there. > +}; > + > +static const struct common_reset_data mediatek_reset_data = { > + .reset_op_available = true, > + .reset_bits = "mediatek, reset-bits", > +}; I understand Robs comments as meaning "ti,reset-bits" should have been called "reset-bits" in the first place, and you shouldn't repeat adding the vendor prefix, as that is implied by the compatible. So this should probably be just "reset-bits". Otherwise this looks like it should work. regards Philipp _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,3/4] reset-controller: ti: introduce a new reset handler @ 2020-09-11 14:44 ` Suman Anna 0 siblings, 0 replies; 82+ messages in thread From: Suman Anna @ 2020-09-11 14:44 UTC (permalink / raw) To: Philipp Zabel, Crystal Guo Cc: robh+dt, matthias.bgg, srv_heupstream, linux-mediatek, linux-arm-kernel, linux-kernel, devicetree, Seiya Wang (王迺君), Stanley Chu (朱原陞), Yingjoe Chen (陳英洲), Fan Chen (陳凡), Yong Liang (梁勇) On 9/11/20 9:26 AM, Philipp Zabel wrote: > Hi Crystal, > > On Fri, 2020-09-11 at 14:07 +0800, Crystal Guo wrote: > [...] >> Should I add the SoC-specific data as follows? >> This may also modify the ti original code, is it OK? >> >> + data->reset_data = of_device_get_match_data(&pdev->dev); >> + >> + list = of_get_property(np, data->reset_data->reset_bits, &size); >> >> +static const struct common_reset_data ti_reset_data = { >> + .reset_op_available = false, >> + .reset_bits = "ti, reset-bits", > ^ > That space doesn't belong there. > >> +}; >> + >> +static const struct common_reset_data mediatek_reset_data = { >> + .reset_op_available = true, >> + .reset_bits = "mediatek, reset-bits", >> +}; > > I understand Robs comments as meaning "ti,reset-bits" should have been > called "reset-bits" in the first place, and you shouldn't repeat adding > the vendor prefix, as that is implied by the compatible. So this should > probably be just "reset-bits". Hmm, not sure about that. I think Rob wants the reset data itself to be added in the driver as is being done on some other SoCs (eg: like in reset-qcom-pdc.c). regards Suman > > Otherwise this looks like it should work. > > regards > Philipp > ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,3/4] reset-controller: ti: introduce a new reset handler @ 2020-09-11 14:44 ` Suman Anna 0 siblings, 0 replies; 82+ messages in thread From: Suman Anna @ 2020-09-11 14:44 UTC (permalink / raw) To: Philipp Zabel, Crystal Guo Cc: devicetree, Yong Liang (梁勇), srv_heupstream, Seiya Wang (王迺君), linux-kernel, Fan Chen (陳凡), robh+dt, linux-mediatek, matthias.bgg, Yingjoe Chen (陳英洲), Stanley Chu (朱原陞), linux-arm-kernel On 9/11/20 9:26 AM, Philipp Zabel wrote: > Hi Crystal, > > On Fri, 2020-09-11 at 14:07 +0800, Crystal Guo wrote: > [...] >> Should I add the SoC-specific data as follows? >> This may also modify the ti original code, is it OK? >> >> + data->reset_data = of_device_get_match_data(&pdev->dev); >> + >> + list = of_get_property(np, data->reset_data->reset_bits, &size); >> >> +static const struct common_reset_data ti_reset_data = { >> + .reset_op_available = false, >> + .reset_bits = "ti, reset-bits", > ^ > That space doesn't belong there. > >> +}; >> + >> +static const struct common_reset_data mediatek_reset_data = { >> + .reset_op_available = true, >> + .reset_bits = "mediatek, reset-bits", >> +}; > > I understand Robs comments as meaning "ti,reset-bits" should have been > called "reset-bits" in the first place, and you shouldn't repeat adding > the vendor prefix, as that is implied by the compatible. So this should > probably be just "reset-bits". Hmm, not sure about that. I think Rob wants the reset data itself to be added in the driver as is being done on some other SoCs (eg: like in reset-qcom-pdc.c). regards Suman > > Otherwise this looks like it should work. > > regards > Philipp > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,3/4] reset-controller: ti: introduce a new reset handler @ 2020-09-11 14:44 ` Suman Anna 0 siblings, 0 replies; 82+ messages in thread From: Suman Anna @ 2020-09-11 14:44 UTC (permalink / raw) To: Philipp Zabel, Crystal Guo Cc: devicetree, Yong Liang (梁勇), srv_heupstream, Seiya Wang (王迺君), linux-kernel, Fan Chen (陳凡), robh+dt, linux-mediatek, matthias.bgg, Yingjoe Chen (陳英洲), Stanley Chu (朱原陞), linux-arm-kernel On 9/11/20 9:26 AM, Philipp Zabel wrote: > Hi Crystal, > > On Fri, 2020-09-11 at 14:07 +0800, Crystal Guo wrote: > [...] >> Should I add the SoC-specific data as follows? >> This may also modify the ti original code, is it OK? >> >> + data->reset_data = of_device_get_match_data(&pdev->dev); >> + >> + list = of_get_property(np, data->reset_data->reset_bits, &size); >> >> +static const struct common_reset_data ti_reset_data = { >> + .reset_op_available = false, >> + .reset_bits = "ti, reset-bits", > ^ > That space doesn't belong there. > >> +}; >> + >> +static const struct common_reset_data mediatek_reset_data = { >> + .reset_op_available = true, >> + .reset_bits = "mediatek, reset-bits", >> +}; > > I understand Robs comments as meaning "ti,reset-bits" should have been > called "reset-bits" in the first place, and you shouldn't repeat adding > the vendor prefix, as that is implied by the compatible. So this should > probably be just "reset-bits". Hmm, not sure about that. I think Rob wants the reset data itself to be added in the driver as is being done on some other SoCs (eg: like in reset-qcom-pdc.c). regards Suman > > Otherwise this looks like it should work. > > regards > Philipp > _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,3/4] reset-controller: ti: introduce a new reset handler @ 2020-09-11 14:44 ` Suman Anna 0 siblings, 0 replies; 82+ messages in thread From: Suman Anna @ 2020-09-11 14:44 UTC (permalink / raw) To: Philipp Zabel, Crystal Guo Cc: robh+dt, matthias.bgg, srv_heupstream, linux-mediatek, linux-arm-kernel, linux-kernel, devicetree, Seiya Wang (王迺君), Stanley Chu (朱原陞), Yingjoe Chen (陳英洲), Fan Chen (陳凡), Yong Liang (梁勇) <<< No Message Collected >>> ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,3/4] reset-controller: ti: introduce a new reset handler 2020-09-11 14:44 ` Suman Anna (?) @ 2020-09-14 14:00 ` Crystal Guo -1 siblings, 0 replies; 82+ messages in thread From: Crystal Guo @ 2020-09-14 14:00 UTC (permalink / raw) To: robh+dt Cc: Suman Anna, Philipp Zabel, matthias.bgg, srv_heupstream, linux-mediatek, linux-arm-kernel, linux-kernel, devicetree, Seiya Wang (王迺君), Stanley Chu (朱原陞), Yingjoe Chen (陳英洲), Fan Chen (陳凡), Yong Liang (梁勇) On Fri, 2020-09-11 at 22:44 +0800, Suman Anna wrote: > On 9/11/20 9:26 AM, Philipp Zabel wrote: > > Hi Crystal, > > > > On Fri, 2020-09-11 at 14:07 +0800, Crystal Guo wrote: > > [...] > >> Should I add the SoC-specific data as follows? > >> This may also modify the ti original code, is it OK? > >> > >> + data->reset_data = of_device_get_match_data(&pdev->dev); > >> + > >> + list = of_get_property(np, data->reset_data->reset_bits, &size); > >> > >> +static const struct common_reset_data ti_reset_data = { > >> + .reset_op_available = false, > >> + .reset_bits = "ti, reset-bits", > > ^ > > That space doesn't belong there. > > > >> +}; > >> + > >> +static const struct common_reset_data mediatek_reset_data = { > >> + .reset_op_available = true, > >> + .reset_bits = "mediatek, reset-bits", > >> +}; > > > > I understand Robs comments as meaning "ti,reset-bits" should have been > > called "reset-bits" in the first place, and you shouldn't repeat adding > > the vendor prefix, as that is implied by the compatible. So this should > > probably be just "reset-bits". > > Hmm, not sure about that. I think Rob wants the reset data itself to be added in > the driver as is being done on some other SoCs (eg: like in reset-qcom-pdc.c). > > regards > Suman > Hi Rob, Can you help to comment about this point? Modify "ti,reset-bits" to "reset-bits" or add "mediatek,reset-bits" ? Many thanks~ Crystal > > > > Otherwise this looks like it should work. > > > > regards > > Philipp > > > ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,3/4] reset-controller: ti: introduce a new reset handler @ 2020-09-14 14:00 ` Crystal Guo 0 siblings, 0 replies; 82+ messages in thread From: Crystal Guo @ 2020-09-14 14:00 UTC (permalink / raw) To: robh+dt Cc: devicetree, Yong Liang (梁勇), Stanley Chu (朱原陞), srv_heupstream, Seiya Wang (王迺君), linux-kernel, Fan Chen (陳凡), linux-mediatek, Philipp Zabel, matthias.bgg, Yingjoe Chen (陳英洲), Suman Anna, linux-arm-kernel On Fri, 2020-09-11 at 22:44 +0800, Suman Anna wrote: > On 9/11/20 9:26 AM, Philipp Zabel wrote: > > Hi Crystal, > > > > On Fri, 2020-09-11 at 14:07 +0800, Crystal Guo wrote: > > [...] > >> Should I add the SoC-specific data as follows? > >> This may also modify the ti original code, is it OK? > >> > >> + data->reset_data = of_device_get_match_data(&pdev->dev); > >> + > >> + list = of_get_property(np, data->reset_data->reset_bits, &size); > >> > >> +static const struct common_reset_data ti_reset_data = { > >> + .reset_op_available = false, > >> + .reset_bits = "ti, reset-bits", > > ^ > > That space doesn't belong there. > > > >> +}; > >> + > >> +static const struct common_reset_data mediatek_reset_data = { > >> + .reset_op_available = true, > >> + .reset_bits = "mediatek, reset-bits", > >> +}; > > > > I understand Robs comments as meaning "ti,reset-bits" should have been > > called "reset-bits" in the first place, and you shouldn't repeat adding > > the vendor prefix, as that is implied by the compatible. So this should > > probably be just "reset-bits". > > Hmm, not sure about that. I think Rob wants the reset data itself to be added in > the driver as is being done on some other SoCs (eg: like in reset-qcom-pdc.c). > > regards > Suman > Hi Rob, Can you help to comment about this point? Modify "ti,reset-bits" to "reset-bits" or add "mediatek,reset-bits" ? Many thanks~ Crystal > > > > Otherwise this looks like it should work. > > > > regards > > Philipp > > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,3/4] reset-controller: ti: introduce a new reset handler @ 2020-09-14 14:00 ` Crystal Guo 0 siblings, 0 replies; 82+ messages in thread From: Crystal Guo @ 2020-09-14 14:00 UTC (permalink / raw) To: robh+dt Cc: devicetree, Yong Liang (梁勇), Stanley Chu (朱原陞), srv_heupstream, Seiya Wang (王迺君), linux-kernel, Fan Chen (陳凡), linux-mediatek, Philipp Zabel, matthias.bgg, Yingjoe Chen (陳英洲), Suman Anna, linux-arm-kernel On Fri, 2020-09-11 at 22:44 +0800, Suman Anna wrote: > On 9/11/20 9:26 AM, Philipp Zabel wrote: > > Hi Crystal, > > > > On Fri, 2020-09-11 at 14:07 +0800, Crystal Guo wrote: > > [...] > >> Should I add the SoC-specific data as follows? > >> This may also modify the ti original code, is it OK? > >> > >> + data->reset_data = of_device_get_match_data(&pdev->dev); > >> + > >> + list = of_get_property(np, data->reset_data->reset_bits, &size); > >> > >> +static const struct common_reset_data ti_reset_data = { > >> + .reset_op_available = false, > >> + .reset_bits = "ti, reset-bits", > > ^ > > That space doesn't belong there. > > > >> +}; > >> + > >> +static const struct common_reset_data mediatek_reset_data = { > >> + .reset_op_available = true, > >> + .reset_bits = "mediatek, reset-bits", > >> +}; > > > > I understand Robs comments as meaning "ti,reset-bits" should have been > > called "reset-bits" in the first place, and you shouldn't repeat adding > > the vendor prefix, as that is implied by the compatible. So this should > > probably be just "reset-bits". > > Hmm, not sure about that. I think Rob wants the reset data itself to be added in > the driver as is being done on some other SoCs (eg: like in reset-qcom-pdc.c). > > regards > Suman > Hi Rob, Can you help to comment about this point? Modify "ti,reset-bits" to "reset-bits" or add "mediatek,reset-bits" ? Many thanks~ Crystal > > > > Otherwise this looks like it should work. > > > > regards > > Philipp > > > _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,3/4] reset-controller: ti: introduce a new reset handler 2020-09-14 14:00 ` Crystal Guo (?) @ 2020-09-29 13:54 ` Crystal Guo -1 siblings, 0 replies; 82+ messages in thread From: Crystal Guo @ 2020-09-29 13:54 UTC (permalink / raw) To: robh+dt, Suman Anna, Philipp Zabel, matthias.bgg Cc: srv_heupstream, linux-mediatek, linux-arm-kernel, linux-kernel, devicetree, Seiya Wang (王迺君), Stanley Chu (朱原陞), Yingjoe Chen (陳英洲), Fan Chen (陳凡), Yong Liang (梁勇) On Mon, 2020-09-14 at 22:00 +0800, Crystal Guo wrote: > On Fri, 2020-09-11 at 22:44 +0800, Suman Anna wrote: > > On 9/11/20 9:26 AM, Philipp Zabel wrote: > > > Hi Crystal, > > > > > > On Fri, 2020-09-11 at 14:07 +0800, Crystal Guo wrote: > > > [...] > > >> Should I add the SoC-specific data as follows? > > >> This may also modify the ti original code, is it OK? > > >> > > >> + data->reset_data = of_device_get_match_data(&pdev->dev); > > >> + > > >> + list = of_get_property(np, data->reset_data->reset_bits, &size); > > >> > > >> +static const struct common_reset_data ti_reset_data = { > > >> + .reset_op_available = false, > > >> + .reset_bits = "ti, reset-bits", > > > ^ > > > That space doesn't belong there. > > > > > >> +}; > > >> + > > >> +static const struct common_reset_data mediatek_reset_data = { > > >> + .reset_op_available = true, > > >> + .reset_bits = "mediatek, reset-bits", > > >> +}; > > > > > > I understand Robs comments as meaning "ti,reset-bits" should have been > > > called "reset-bits" in the first place, and you shouldn't repeat adding > > > the vendor prefix, as that is implied by the compatible. So this should > > > probably be just "reset-bits". > > > > Hmm, not sure about that. I think Rob wants the reset data itself to be added in > > the driver as is being done on some other SoCs (eg: like in reset-qcom-pdc.c). > > > > regards > > Suman > > > Hi Rob, > > Can you help to comment about this point? > Modify "ti,reset-bits" to "reset-bits" or add "mediatek,reset-bits" ? > > Many thanks~ > Crystal > > > > > > > Otherwise this looks like it should work. > > > > > > regards > > > Philipp > > > > > Dears, I have uploaded the changes at https://patchwork.kernel.org/cover/11805937/ Please help me to review, many thanks~~ regards Crystal > ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,3/4] reset-controller: ti: introduce a new reset handler @ 2020-09-29 13:54 ` Crystal Guo 0 siblings, 0 replies; 82+ messages in thread From: Crystal Guo @ 2020-09-29 13:54 UTC (permalink / raw) To: robh+dt, Suman Anna, Philipp Zabel, matthias.bgg Cc: devicetree, Yong Liang (梁勇), srv_heupstream, Seiya Wang (王迺君), linux-kernel, Fan Chen (陳凡), linux-mediatek, Yingjoe Chen (陳英洲), Stanley Chu (朱原陞), linux-arm-kernel On Mon, 2020-09-14 at 22:00 +0800, Crystal Guo wrote: > On Fri, 2020-09-11 at 22:44 +0800, Suman Anna wrote: > > On 9/11/20 9:26 AM, Philipp Zabel wrote: > > > Hi Crystal, > > > > > > On Fri, 2020-09-11 at 14:07 +0800, Crystal Guo wrote: > > > [...] > > >> Should I add the SoC-specific data as follows? > > >> This may also modify the ti original code, is it OK? > > >> > > >> + data->reset_data = of_device_get_match_data(&pdev->dev); > > >> + > > >> + list = of_get_property(np, data->reset_data->reset_bits, &size); > > >> > > >> +static const struct common_reset_data ti_reset_data = { > > >> + .reset_op_available = false, > > >> + .reset_bits = "ti, reset-bits", > > > ^ > > > That space doesn't belong there. > > > > > >> +}; > > >> + > > >> +static const struct common_reset_data mediatek_reset_data = { > > >> + .reset_op_available = true, > > >> + .reset_bits = "mediatek, reset-bits", > > >> +}; > > > > > > I understand Robs comments as meaning "ti,reset-bits" should have been > > > called "reset-bits" in the first place, and you shouldn't repeat adding > > > the vendor prefix, as that is implied by the compatible. So this should > > > probably be just "reset-bits". > > > > Hmm, not sure about that. I think Rob wants the reset data itself to be added in > > the driver as is being done on some other SoCs (eg: like in reset-qcom-pdc.c). > > > > regards > > Suman > > > Hi Rob, > > Can you help to comment about this point? > Modify "ti,reset-bits" to "reset-bits" or add "mediatek,reset-bits" ? > > Many thanks~ > Crystal > > > > > > > Otherwise this looks like it should work. > > > > > > regards > > > Philipp > > > > > Dears, I have uploaded the changes at https://patchwork.kernel.org/cover/11805937/ Please help me to review, many thanks~~ regards Crystal > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,3/4] reset-controller: ti: introduce a new reset handler @ 2020-09-29 13:54 ` Crystal Guo 0 siblings, 0 replies; 82+ messages in thread From: Crystal Guo @ 2020-09-29 13:54 UTC (permalink / raw) To: robh+dt, Suman Anna, Philipp Zabel, matthias.bgg Cc: devicetree, Yong Liang (梁勇), srv_heupstream, Seiya Wang (王迺君), linux-kernel, Fan Chen (陳凡), linux-mediatek, Yingjoe Chen (陳英洲), Stanley Chu (朱原陞), linux-arm-kernel On Mon, 2020-09-14 at 22:00 +0800, Crystal Guo wrote: > On Fri, 2020-09-11 at 22:44 +0800, Suman Anna wrote: > > On 9/11/20 9:26 AM, Philipp Zabel wrote: > > > Hi Crystal, > > > > > > On Fri, 2020-09-11 at 14:07 +0800, Crystal Guo wrote: > > > [...] > > >> Should I add the SoC-specific data as follows? > > >> This may also modify the ti original code, is it OK? > > >> > > >> + data->reset_data = of_device_get_match_data(&pdev->dev); > > >> + > > >> + list = of_get_property(np, data->reset_data->reset_bits, &size); > > >> > > >> +static const struct common_reset_data ti_reset_data = { > > >> + .reset_op_available = false, > > >> + .reset_bits = "ti, reset-bits", > > > ^ > > > That space doesn't belong there. > > > > > >> +}; > > >> + > > >> +static const struct common_reset_data mediatek_reset_data = { > > >> + .reset_op_available = true, > > >> + .reset_bits = "mediatek, reset-bits", > > >> +}; > > > > > > I understand Robs comments as meaning "ti,reset-bits" should have been > > > called "reset-bits" in the first place, and you shouldn't repeat adding > > > the vendor prefix, as that is implied by the compatible. So this should > > > probably be just "reset-bits". > > > > Hmm, not sure about that. I think Rob wants the reset data itself to be added in > > the driver as is being done on some other SoCs (eg: like in reset-qcom-pdc.c). > > > > regards > > Suman > > > Hi Rob, > > Can you help to comment about this point? > Modify "ti,reset-bits" to "reset-bits" or add "mediatek,reset-bits" ? > > Many thanks~ > Crystal > > > > > > > Otherwise this looks like it should work. > > > > > > regards > > > Philipp > > > > > Dears, I have uploaded the changes at https://patchwork.kernel.org/cover/11805937/ Please help me to review, many thanks~~ regards Crystal > _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek ^ permalink raw reply [flat|nested] 82+ messages in thread
* [v4,4/4] arm64: dts: mt8192: add infracfg_rst node 2020-08-17 3:03 ` Crystal Guo (?) @ 2020-08-17 3:03 ` Crystal Guo -1 siblings, 0 replies; 82+ messages in thread From: Crystal Guo @ 2020-08-17 3:03 UTC (permalink / raw) To: p.zabel, robh+dt, matthias.bgg Cc: srv_heupstream, linux-mediatek, linux-arm-kernel, linux-kernel, devicetree, s-anna, afd, seiya.wang, stanley.chu, yingjoe.chen, fan.chen, yong.liang, Crystal Guo add infracfg_rst node which is for MT8192 platform Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> --- arch/arm64/boot/dts/mediatek/mt8192.dtsi | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/arch/arm64/boot/dts/mediatek/mt8192.dtsi b/arch/arm64/boot/dts/mediatek/mt8192.dtsi index 931e1ca17220..a0cb9904706b 100644 --- a/arch/arm64/boot/dts/mediatek/mt8192.dtsi +++ b/arch/arm64/boot/dts/mediatek/mt8192.dtsi @@ -10,6 +10,7 @@ #include <dt-bindings/interrupt-controller/irq.h> #include <dt-bindings/pinctrl/mt8192-pinfunc.h> #include <dt-bindings/power/mt8192-power.h> +#include <dt-bindings/reset/ti-syscon.h> / { compatible = "mediatek,mt8192"; @@ -219,9 +220,17 @@ }; infracfg: infracfg@10001000 { - compatible = "mediatek,mt8192-infracfg", "syscon"; + compatible = "mediatek,mt8192-infracfg", "syscon", "simple-mfd"; reg = <0 0x10001000 0 0x1000>; #clock-cells = <1>; + + infracfg_rst: reset-controller { + compatible = "mediatek,infra-reset", "ti,syscon-reset"; + #reset-cells = <1>; + ti,reset-bits = < + 0x140 15 0x144 15 0 0 (ASSERT_SET | DEASSERT_SET | STATUS_NONE) /* 0: pcie */ + >; + }; }; pericfg: pericfg@10003000 { -- 2.18.0 ^ permalink raw reply related [flat|nested] 82+ messages in thread
* [v4,4/4] arm64: dts: mt8192: add infracfg_rst node @ 2020-08-17 3:03 ` Crystal Guo 0 siblings, 0 replies; 82+ messages in thread From: Crystal Guo @ 2020-08-17 3:03 UTC (permalink / raw) To: p.zabel, robh+dt, matthias.bgg Cc: devicetree, yong.liang, stanley.chu, srv_heupstream, seiya.wang, linux-kernel, afd, fan.chen, Crystal Guo, linux-mediatek, yingjoe.chen, linux-arm-kernel add infracfg_rst node which is for MT8192 platform Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> --- arch/arm64/boot/dts/mediatek/mt8192.dtsi | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/arch/arm64/boot/dts/mediatek/mt8192.dtsi b/arch/arm64/boot/dts/mediatek/mt8192.dtsi index 931e1ca17220..a0cb9904706b 100644 --- a/arch/arm64/boot/dts/mediatek/mt8192.dtsi +++ b/arch/arm64/boot/dts/mediatek/mt8192.dtsi @@ -10,6 +10,7 @@ #include <dt-bindings/interrupt-controller/irq.h> #include <dt-bindings/pinctrl/mt8192-pinfunc.h> #include <dt-bindings/power/mt8192-power.h> +#include <dt-bindings/reset/ti-syscon.h> / { compatible = "mediatek,mt8192"; @@ -219,9 +220,17 @@ }; infracfg: infracfg@10001000 { - compatible = "mediatek,mt8192-infracfg", "syscon"; + compatible = "mediatek,mt8192-infracfg", "syscon", "simple-mfd"; reg = <0 0x10001000 0 0x1000>; #clock-cells = <1>; + + infracfg_rst: reset-controller { + compatible = "mediatek,infra-reset", "ti,syscon-reset"; + #reset-cells = <1>; + ti,reset-bits = < + 0x140 15 0x144 15 0 0 (ASSERT_SET | DEASSERT_SET | STATUS_NONE) /* 0: pcie */ + >; + }; }; pericfg: pericfg@10003000 { -- 2.18.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 82+ messages in thread
* [v4,4/4] arm64: dts: mt8192: add infracfg_rst node @ 2020-08-17 3:03 ` Crystal Guo 0 siblings, 0 replies; 82+ messages in thread From: Crystal Guo @ 2020-08-17 3:03 UTC (permalink / raw) To: p.zabel, robh+dt, matthias.bgg Cc: devicetree, yong.liang, stanley.chu, srv_heupstream, seiya.wang, linux-kernel, afd, fan.chen, Crystal Guo, linux-mediatek, yingjoe.chen, s-anna, linux-arm-kernel add infracfg_rst node which is for MT8192 platform Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> --- arch/arm64/boot/dts/mediatek/mt8192.dtsi | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/arch/arm64/boot/dts/mediatek/mt8192.dtsi b/arch/arm64/boot/dts/mediatek/mt8192.dtsi index 931e1ca17220..a0cb9904706b 100644 --- a/arch/arm64/boot/dts/mediatek/mt8192.dtsi +++ b/arch/arm64/boot/dts/mediatek/mt8192.dtsi @@ -10,6 +10,7 @@ #include <dt-bindings/interrupt-controller/irq.h> #include <dt-bindings/pinctrl/mt8192-pinfunc.h> #include <dt-bindings/power/mt8192-power.h> +#include <dt-bindings/reset/ti-syscon.h> / { compatible = "mediatek,mt8192"; @@ -219,9 +220,17 @@ }; infracfg: infracfg@10001000 { - compatible = "mediatek,mt8192-infracfg", "syscon"; + compatible = "mediatek,mt8192-infracfg", "syscon", "simple-mfd"; reg = <0 0x10001000 0 0x1000>; #clock-cells = <1>; + + infracfg_rst: reset-controller { + compatible = "mediatek,infra-reset", "ti,syscon-reset"; + #reset-cells = <1>; + ti,reset-bits = < + 0x140 15 0x144 15 0 0 (ASSERT_SET | DEASSERT_SET | STATUS_NONE) /* 0: pcie */ + >; + }; }; pericfg: pericfg@10003000 { -- 2.18.0 _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek ^ permalink raw reply related [flat|nested] 82+ messages in thread
* Re: [v4,4/4] arm64: dts: mt8192: add infracfg_rst node 2020-08-17 3:03 ` Crystal Guo (?) @ 2020-09-02 23:29 ` Suman Anna -1 siblings, 0 replies; 82+ messages in thread From: Suman Anna @ 2020-09-02 23:29 UTC (permalink / raw) To: Crystal Guo, p.zabel, robh+dt, matthias.bgg Cc: srv_heupstream, linux-mediatek, linux-arm-kernel, linux-kernel, devicetree, seiya.wang, stanley.chu, yingjoe.chen, fan.chen, yong.liang Hi Crystal, On 8/16/20 10:03 PM, Crystal Guo wrote: > add infracfg_rst node which is for MT8192 platform > > Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> I understand you are posting these together for complete reference, but driver subsystem maintainers typically don't pick dts patches. In anycase, can you clarify if your registers are self-clearing registers? regards Suman > --- > arch/arm64/boot/dts/mediatek/mt8192.dtsi | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/boot/dts/mediatek/mt8192.dtsi b/arch/arm64/boot/dts/mediatek/mt8192.dtsi > index 931e1ca17220..a0cb9904706b 100644 > --- a/arch/arm64/boot/dts/mediatek/mt8192.dtsi > +++ b/arch/arm64/boot/dts/mediatek/mt8192.dtsi > @@ -10,6 +10,7 @@ > #include <dt-bindings/interrupt-controller/irq.h> > #include <dt-bindings/pinctrl/mt8192-pinfunc.h> > #include <dt-bindings/power/mt8192-power.h> > +#include <dt-bindings/reset/ti-syscon.h> > > / { > compatible = "mediatek,mt8192"; > @@ -219,9 +220,17 @@ > }; > > infracfg: infracfg@10001000 { > - compatible = "mediatek,mt8192-infracfg", "syscon"; > + compatible = "mediatek,mt8192-infracfg", "syscon", "simple-mfd"; > reg = <0 0x10001000 0 0x1000>; > #clock-cells = <1>; > + > + infracfg_rst: reset-controller { > + compatible = "mediatek,infra-reset", "ti,syscon-reset"; > + #reset-cells = <1>; > + ti,reset-bits = < > + 0x140 15 0x144 15 0 0 (ASSERT_SET | DEASSERT_SET | STATUS_NONE) /* 0: pcie */ > + >; > + }; > }; > > pericfg: pericfg@10003000 { > ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,4/4] arm64: dts: mt8192: add infracfg_rst node @ 2020-09-02 23:29 ` Suman Anna 0 siblings, 0 replies; 82+ messages in thread From: Suman Anna @ 2020-09-02 23:29 UTC (permalink / raw) To: Crystal Guo, p.zabel, robh+dt, matthias.bgg Cc: devicetree, yong.liang, srv_heupstream, seiya.wang, linux-kernel, fan.chen, linux-mediatek, yingjoe.chen, stanley.chu, linux-arm-kernel Hi Crystal, On 8/16/20 10:03 PM, Crystal Guo wrote: > add infracfg_rst node which is for MT8192 platform > > Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> I understand you are posting these together for complete reference, but driver subsystem maintainers typically don't pick dts patches. In anycase, can you clarify if your registers are self-clearing registers? regards Suman > --- > arch/arm64/boot/dts/mediatek/mt8192.dtsi | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/boot/dts/mediatek/mt8192.dtsi b/arch/arm64/boot/dts/mediatek/mt8192.dtsi > index 931e1ca17220..a0cb9904706b 100644 > --- a/arch/arm64/boot/dts/mediatek/mt8192.dtsi > +++ b/arch/arm64/boot/dts/mediatek/mt8192.dtsi > @@ -10,6 +10,7 @@ > #include <dt-bindings/interrupt-controller/irq.h> > #include <dt-bindings/pinctrl/mt8192-pinfunc.h> > #include <dt-bindings/power/mt8192-power.h> > +#include <dt-bindings/reset/ti-syscon.h> > > / { > compatible = "mediatek,mt8192"; > @@ -219,9 +220,17 @@ > }; > > infracfg: infracfg@10001000 { > - compatible = "mediatek,mt8192-infracfg", "syscon"; > + compatible = "mediatek,mt8192-infracfg", "syscon", "simple-mfd"; > reg = <0 0x10001000 0 0x1000>; > #clock-cells = <1>; > + > + infracfg_rst: reset-controller { > + compatible = "mediatek,infra-reset", "ti,syscon-reset"; > + #reset-cells = <1>; > + ti,reset-bits = < > + 0x140 15 0x144 15 0 0 (ASSERT_SET | DEASSERT_SET | STATUS_NONE) /* 0: pcie */ > + >; > + }; > }; > > pericfg: pericfg@10003000 { > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,4/4] arm64: dts: mt8192: add infracfg_rst node @ 2020-09-02 23:29 ` Suman Anna 0 siblings, 0 replies; 82+ messages in thread From: Suman Anna @ 2020-09-02 23:29 UTC (permalink / raw) To: Crystal Guo, p.zabel, robh+dt, matthias.bgg Cc: devicetree, yong.liang, srv_heupstream, seiya.wang, linux-kernel, fan.chen, linux-mediatek, yingjoe.chen, stanley.chu, linux-arm-kernel Hi Crystal, On 8/16/20 10:03 PM, Crystal Guo wrote: > add infracfg_rst node which is for MT8192 platform > > Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> I understand you are posting these together for complete reference, but driver subsystem maintainers typically don't pick dts patches. In anycase, can you clarify if your registers are self-clearing registers? regards Suman > --- > arch/arm64/boot/dts/mediatek/mt8192.dtsi | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/boot/dts/mediatek/mt8192.dtsi b/arch/arm64/boot/dts/mediatek/mt8192.dtsi > index 931e1ca17220..a0cb9904706b 100644 > --- a/arch/arm64/boot/dts/mediatek/mt8192.dtsi > +++ b/arch/arm64/boot/dts/mediatek/mt8192.dtsi > @@ -10,6 +10,7 @@ > #include <dt-bindings/interrupt-controller/irq.h> > #include <dt-bindings/pinctrl/mt8192-pinfunc.h> > #include <dt-bindings/power/mt8192-power.h> > +#include <dt-bindings/reset/ti-syscon.h> > > / { > compatible = "mediatek,mt8192"; > @@ -219,9 +220,17 @@ > }; > > infracfg: infracfg@10001000 { > - compatible = "mediatek,mt8192-infracfg", "syscon"; > + compatible = "mediatek,mt8192-infracfg", "syscon", "simple-mfd"; > reg = <0 0x10001000 0 0x1000>; > #clock-cells = <1>; > + > + infracfg_rst: reset-controller { > + compatible = "mediatek,infra-reset", "ti,syscon-reset"; > + #reset-cells = <1>; > + ti,reset-bits = < > + 0x140 15 0x144 15 0 0 (ASSERT_SET | DEASSERT_SET | STATUS_NONE) /* 0: pcie */ > + >; > + }; > }; > > pericfg: pericfg@10003000 { > _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,4/4] arm64: dts: mt8192: add infracfg_rst node 2020-09-02 23:29 ` Suman Anna (?) @ 2020-09-08 13:26 ` Crystal Guo -1 siblings, 0 replies; 82+ messages in thread From: Crystal Guo @ 2020-09-08 13:26 UTC (permalink / raw) To: Suman Anna Cc: p.zabel, robh+dt, matthias.bgg, srv_heupstream, linux-mediatek, linux-arm-kernel, linux-kernel, devicetree, Seiya Wang (王迺君), Stanley Chu (朱原陞), Yingjoe Chen (陳英洲), Fan Chen (陳凡), Yong Liang (梁勇) On Thu, 2020-09-03 at 07:29 +0800, Suman Anna wrote: > Hi Crystal, > > On 8/16/20 10:03 PM, Crystal Guo wrote: > > add infracfg_rst node which is for MT8192 platform > > > > Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> > > I understand you are posting these together for complete reference, but driver > subsystem maintainers typically don't pick dts patches. In anycase, can you > clarify if your registers are self-clearing registers? > > regards > Suman > Hi Suman, Thanks for your reply. Our reset registers are not self-clearing, it needs to set the clear bit to 1 to clear the related bit. And should I separate this dts patch from the patch sets? regards Crystal > > --- > > arch/arm64/boot/dts/mediatek/mt8192.dtsi | 11 ++++++++++- > > 1 file changed, 10 insertions(+), 1 deletion(-) > > > > diff --git a/arch/arm64/boot/dts/mediatek/mt8192.dtsi b/arch/arm64/boot/dts/mediatek/mt8192.dtsi > > index 931e1ca17220..a0cb9904706b 100644 > > --- a/arch/arm64/boot/dts/mediatek/mt8192.dtsi > > +++ b/arch/arm64/boot/dts/mediatek/mt8192.dtsi > > @@ -10,6 +10,7 @@ > > #include <dt-bindings/interrupt-controller/irq.h> > > #include <dt-bindings/pinctrl/mt8192-pinfunc.h> > > #include <dt-bindings/power/mt8192-power.h> > > +#include <dt-bindings/reset/ti-syscon.h> > > > > / { > > compatible = "mediatek,mt8192"; > > @@ -219,9 +220,17 @@ > > }; > > > > infracfg: infracfg@10001000 { > > - compatible = "mediatek,mt8192-infracfg", "syscon"; > > + compatible = "mediatek,mt8192-infracfg", "syscon", "simple-mfd"; > > reg = <0 0x10001000 0 0x1000>; > > #clock-cells = <1>; > > + > > + infracfg_rst: reset-controller { > > + compatible = "mediatek,infra-reset", "ti,syscon-reset"; > > + #reset-cells = <1>; > > + ti,reset-bits = < > > + 0x140 15 0x144 15 0 0 (ASSERT_SET | DEASSERT_SET | STATUS_NONE) /* 0: pcie */ > > + >; > > + }; > > }; > > > > pericfg: pericfg@10003000 { > > > ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,4/4] arm64: dts: mt8192: add infracfg_rst node @ 2020-09-08 13:26 ` Crystal Guo 0 siblings, 0 replies; 82+ messages in thread From: Crystal Guo @ 2020-09-08 13:26 UTC (permalink / raw) To: Suman Anna Cc: devicetree, Yong Liang (梁勇), srv_heupstream, Seiya Wang (王迺君), linux-kernel, Fan Chen (陳凡), robh+dt, linux-mediatek, p.zabel, matthias.bgg, Yingjoe Chen (陳英洲), Stanley Chu (朱原陞), linux-arm-kernel On Thu, 2020-09-03 at 07:29 +0800, Suman Anna wrote: > Hi Crystal, > > On 8/16/20 10:03 PM, Crystal Guo wrote: > > add infracfg_rst node which is for MT8192 platform > > > > Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> > > I understand you are posting these together for complete reference, but driver > subsystem maintainers typically don't pick dts patches. In anycase, can you > clarify if your registers are self-clearing registers? > > regards > Suman > Hi Suman, Thanks for your reply. Our reset registers are not self-clearing, it needs to set the clear bit to 1 to clear the related bit. And should I separate this dts patch from the patch sets? regards Crystal > > --- > > arch/arm64/boot/dts/mediatek/mt8192.dtsi | 11 ++++++++++- > > 1 file changed, 10 insertions(+), 1 deletion(-) > > > > diff --git a/arch/arm64/boot/dts/mediatek/mt8192.dtsi b/arch/arm64/boot/dts/mediatek/mt8192.dtsi > > index 931e1ca17220..a0cb9904706b 100644 > > --- a/arch/arm64/boot/dts/mediatek/mt8192.dtsi > > +++ b/arch/arm64/boot/dts/mediatek/mt8192.dtsi > > @@ -10,6 +10,7 @@ > > #include <dt-bindings/interrupt-controller/irq.h> > > #include <dt-bindings/pinctrl/mt8192-pinfunc.h> > > #include <dt-bindings/power/mt8192-power.h> > > +#include <dt-bindings/reset/ti-syscon.h> > > > > / { > > compatible = "mediatek,mt8192"; > > @@ -219,9 +220,17 @@ > > }; > > > > infracfg: infracfg@10001000 { > > - compatible = "mediatek,mt8192-infracfg", "syscon"; > > + compatible = "mediatek,mt8192-infracfg", "syscon", "simple-mfd"; > > reg = <0 0x10001000 0 0x1000>; > > #clock-cells = <1>; > > + > > + infracfg_rst: reset-controller { > > + compatible = "mediatek,infra-reset", "ti,syscon-reset"; > > + #reset-cells = <1>; > > + ti,reset-bits = < > > + 0x140 15 0x144 15 0 0 (ASSERT_SET | DEASSERT_SET | STATUS_NONE) /* 0: pcie */ > > + >; > > + }; > > }; > > > > pericfg: pericfg@10003000 { > > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,4/4] arm64: dts: mt8192: add infracfg_rst node @ 2020-09-08 13:26 ` Crystal Guo 0 siblings, 0 replies; 82+ messages in thread From: Crystal Guo @ 2020-09-08 13:26 UTC (permalink / raw) To: Suman Anna Cc: devicetree, Yong Liang (梁勇), srv_heupstream, Seiya Wang (王迺君), linux-kernel, Fan Chen (陳凡), robh+dt, linux-mediatek, p.zabel, matthias.bgg, Yingjoe Chen (陳英洲), Stanley Chu (朱原陞), linux-arm-kernel On Thu, 2020-09-03 at 07:29 +0800, Suman Anna wrote: > Hi Crystal, > > On 8/16/20 10:03 PM, Crystal Guo wrote: > > add infracfg_rst node which is for MT8192 platform > > > > Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> > > I understand you are posting these together for complete reference, but driver > subsystem maintainers typically don't pick dts patches. In anycase, can you > clarify if your registers are self-clearing registers? > > regards > Suman > Hi Suman, Thanks for your reply. Our reset registers are not self-clearing, it needs to set the clear bit to 1 to clear the related bit. And should I separate this dts patch from the patch sets? regards Crystal > > --- > > arch/arm64/boot/dts/mediatek/mt8192.dtsi | 11 ++++++++++- > > 1 file changed, 10 insertions(+), 1 deletion(-) > > > > diff --git a/arch/arm64/boot/dts/mediatek/mt8192.dtsi b/arch/arm64/boot/dts/mediatek/mt8192.dtsi > > index 931e1ca17220..a0cb9904706b 100644 > > --- a/arch/arm64/boot/dts/mediatek/mt8192.dtsi > > +++ b/arch/arm64/boot/dts/mediatek/mt8192.dtsi > > @@ -10,6 +10,7 @@ > > #include <dt-bindings/interrupt-controller/irq.h> > > #include <dt-bindings/pinctrl/mt8192-pinfunc.h> > > #include <dt-bindings/power/mt8192-power.h> > > +#include <dt-bindings/reset/ti-syscon.h> > > > > / { > > compatible = "mediatek,mt8192"; > > @@ -219,9 +220,17 @@ > > }; > > > > infracfg: infracfg@10001000 { > > - compatible = "mediatek,mt8192-infracfg", "syscon"; > > + compatible = "mediatek,mt8192-infracfg", "syscon", "simple-mfd"; > > reg = <0 0x10001000 0 0x1000>; > > #clock-cells = <1>; > > + > > + infracfg_rst: reset-controller { > > + compatible = "mediatek,infra-reset", "ti,syscon-reset"; > > + #reset-cells = <1>; > > + ti,reset-bits = < > > + 0x140 15 0x144 15 0 0 (ASSERT_SET | DEASSERT_SET | STATUS_NONE) /* 0: pcie */ > > + >; > > + }; > > }; > > > > pericfg: pericfg@10003000 { > > > _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,4/4] arm64: dts: mt8192: add infracfg_rst node 2020-09-08 13:26 ` Crystal Guo (?) @ 2020-09-08 15:51 ` Suman Anna -1 siblings, 0 replies; 82+ messages in thread From: Suman Anna @ 2020-09-08 15:51 UTC (permalink / raw) To: Crystal Guo Cc: p.zabel, robh+dt, matthias.bgg, srv_heupstream, linux-mediatek, linux-arm-kernel, linux-kernel, devicetree, Seiya Wang (王迺君), Stanley Chu (朱原陞), Yingjoe Chen (陳英洲), Fan Chen (陳凡), Yong Liang (梁勇) On 9/8/20 8:26 AM, Crystal Guo wrote: > On Thu, 2020-09-03 at 07:29 +0800, Suman Anna wrote: >> Hi Crystal, >> >> On 8/16/20 10:03 PM, Crystal Guo wrote: >>> add infracfg_rst node which is for MT8192 platform >>> >>> Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> >> >> I understand you are posting these together for complete reference, but driver >> subsystem maintainers typically don't pick dts patches. In anycase, can you >> clarify if your registers are self-clearing registers? >> >> regards >> Suman >> > Hi Suman, > > Thanks for your reply. > Our reset registers are not self-clearing, it needs to set the clear bit > to 1 to clear the related bit. > And should I separate this dts patch from the patch sets? Typically yes, but will leave it upto Philipp and Mediatek DT maintainers. regards Suman > > regards > Crystal >>> --- >>> arch/arm64/boot/dts/mediatek/mt8192.dtsi | 11 ++++++++++- >>> 1 file changed, 10 insertions(+), 1 deletion(-) >>> >>> diff --git a/arch/arm64/boot/dts/mediatek/mt8192.dtsi b/arch/arm64/boot/dts/mediatek/mt8192.dtsi >>> index 931e1ca17220..a0cb9904706b 100644 >>> --- a/arch/arm64/boot/dts/mediatek/mt8192.dtsi >>> +++ b/arch/arm64/boot/dts/mediatek/mt8192.dtsi >>> @@ -10,6 +10,7 @@ >>> #include <dt-bindings/interrupt-controller/irq.h> >>> #include <dt-bindings/pinctrl/mt8192-pinfunc.h> >>> #include <dt-bindings/power/mt8192-power.h> >>> +#include <dt-bindings/reset/ti-syscon.h> >>> >>> / { >>> compatible = "mediatek,mt8192"; >>> @@ -219,9 +220,17 @@ >>> }; >>> >>> infracfg: infracfg@10001000 { >>> - compatible = "mediatek,mt8192-infracfg", "syscon"; >>> + compatible = "mediatek,mt8192-infracfg", "syscon", "simple-mfd"; >>> reg = <0 0x10001000 0 0x1000>; >>> #clock-cells = <1>; >>> + >>> + infracfg_rst: reset-controller { >>> + compatible = "mediatek,infra-reset", "ti,syscon-reset"; >>> + #reset-cells = <1>; >>> + ti,reset-bits = < >>> + 0x140 15 0x144 15 0 0 (ASSERT_SET | DEASSERT_SET | STATUS_NONE) /* 0: pcie */ >>> + >; >>> + }; >>> }; >>> >>> pericfg: pericfg@10003000 { >>> >> > ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,4/4] arm64: dts: mt8192: add infracfg_rst node @ 2020-09-08 15:51 ` Suman Anna 0 siblings, 0 replies; 82+ messages in thread From: Suman Anna @ 2020-09-08 15:51 UTC (permalink / raw) To: Crystal Guo Cc: devicetree, Yong Liang (梁勇), srv_heupstream, Seiya Wang (王迺君), linux-kernel, Fan Chen (陳凡), robh+dt, linux-mediatek, p.zabel, matthias.bgg, Yingjoe Chen (陳英洲), Stanley Chu (朱原陞), linux-arm-kernel On 9/8/20 8:26 AM, Crystal Guo wrote: > On Thu, 2020-09-03 at 07:29 +0800, Suman Anna wrote: >> Hi Crystal, >> >> On 8/16/20 10:03 PM, Crystal Guo wrote: >>> add infracfg_rst node which is for MT8192 platform >>> >>> Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> >> >> I understand you are posting these together for complete reference, but driver >> subsystem maintainers typically don't pick dts patches. In anycase, can you >> clarify if your registers are self-clearing registers? >> >> regards >> Suman >> > Hi Suman, > > Thanks for your reply. > Our reset registers are not self-clearing, it needs to set the clear bit > to 1 to clear the related bit. > And should I separate this dts patch from the patch sets? Typically yes, but will leave it upto Philipp and Mediatek DT maintainers. regards Suman > > regards > Crystal >>> --- >>> arch/arm64/boot/dts/mediatek/mt8192.dtsi | 11 ++++++++++- >>> 1 file changed, 10 insertions(+), 1 deletion(-) >>> >>> diff --git a/arch/arm64/boot/dts/mediatek/mt8192.dtsi b/arch/arm64/boot/dts/mediatek/mt8192.dtsi >>> index 931e1ca17220..a0cb9904706b 100644 >>> --- a/arch/arm64/boot/dts/mediatek/mt8192.dtsi >>> +++ b/arch/arm64/boot/dts/mediatek/mt8192.dtsi >>> @@ -10,6 +10,7 @@ >>> #include <dt-bindings/interrupt-controller/irq.h> >>> #include <dt-bindings/pinctrl/mt8192-pinfunc.h> >>> #include <dt-bindings/power/mt8192-power.h> >>> +#include <dt-bindings/reset/ti-syscon.h> >>> >>> / { >>> compatible = "mediatek,mt8192"; >>> @@ -219,9 +220,17 @@ >>> }; >>> >>> infracfg: infracfg@10001000 { >>> - compatible = "mediatek,mt8192-infracfg", "syscon"; >>> + compatible = "mediatek,mt8192-infracfg", "syscon", "simple-mfd"; >>> reg = <0 0x10001000 0 0x1000>; >>> #clock-cells = <1>; >>> + >>> + infracfg_rst: reset-controller { >>> + compatible = "mediatek,infra-reset", "ti,syscon-reset"; >>> + #reset-cells = <1>; >>> + ti,reset-bits = < >>> + 0x140 15 0x144 15 0 0 (ASSERT_SET | DEASSERT_SET | STATUS_NONE) /* 0: pcie */ >>> + >; >>> + }; >>> }; >>> >>> pericfg: pericfg@10003000 { >>> >> > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,4/4] arm64: dts: mt8192: add infracfg_rst node @ 2020-09-08 15:51 ` Suman Anna 0 siblings, 0 replies; 82+ messages in thread From: Suman Anna @ 2020-09-08 15:51 UTC (permalink / raw) To: Crystal Guo Cc: devicetree, Yong Liang (梁勇), srv_heupstream, Seiya Wang (王迺君), linux-kernel, Fan Chen (陳凡), robh+dt, linux-mediatek, p.zabel, matthias.bgg, Yingjoe Chen (陳英洲), Stanley Chu (朱原陞), linux-arm-kernel On 9/8/20 8:26 AM, Crystal Guo wrote: > On Thu, 2020-09-03 at 07:29 +0800, Suman Anna wrote: >> Hi Crystal, >> >> On 8/16/20 10:03 PM, Crystal Guo wrote: >>> add infracfg_rst node which is for MT8192 platform >>> >>> Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> >> >> I understand you are posting these together for complete reference, but driver >> subsystem maintainers typically don't pick dts patches. In anycase, can you >> clarify if your registers are self-clearing registers? >> >> regards >> Suman >> > Hi Suman, > > Thanks for your reply. > Our reset registers are not self-clearing, it needs to set the clear bit > to 1 to clear the related bit. > And should I separate this dts patch from the patch sets? Typically yes, but will leave it upto Philipp and Mediatek DT maintainers. regards Suman > > regards > Crystal >>> --- >>> arch/arm64/boot/dts/mediatek/mt8192.dtsi | 11 ++++++++++- >>> 1 file changed, 10 insertions(+), 1 deletion(-) >>> >>> diff --git a/arch/arm64/boot/dts/mediatek/mt8192.dtsi b/arch/arm64/boot/dts/mediatek/mt8192.dtsi >>> index 931e1ca17220..a0cb9904706b 100644 >>> --- a/arch/arm64/boot/dts/mediatek/mt8192.dtsi >>> +++ b/arch/arm64/boot/dts/mediatek/mt8192.dtsi >>> @@ -10,6 +10,7 @@ >>> #include <dt-bindings/interrupt-controller/irq.h> >>> #include <dt-bindings/pinctrl/mt8192-pinfunc.h> >>> #include <dt-bindings/power/mt8192-power.h> >>> +#include <dt-bindings/reset/ti-syscon.h> >>> >>> / { >>> compatible = "mediatek,mt8192"; >>> @@ -219,9 +220,17 @@ >>> }; >>> >>> infracfg: infracfg@10001000 { >>> - compatible = "mediatek,mt8192-infracfg", "syscon"; >>> + compatible = "mediatek,mt8192-infracfg", "syscon", "simple-mfd"; >>> reg = <0 0x10001000 0 0x1000>; >>> #clock-cells = <1>; >>> + >>> + infracfg_rst: reset-controller { >>> + compatible = "mediatek,infra-reset", "ti,syscon-reset"; >>> + #reset-cells = <1>; >>> + ti,reset-bits = < >>> + 0x140 15 0x144 15 0 0 (ASSERT_SET | DEASSERT_SET | STATUS_NONE) /* 0: pcie */ >>> + >; >>> + }; >>> }; >>> >>> pericfg: pericfg@10003000 { >>> >> > _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek ^ permalink raw reply [flat|nested] 82+ messages in thread
[parent not found: <5065a23627a34212aa62df646dbf00ee@mtkmbs05n1.mediatek.inc>]
* Re: [v4,0/4] introduce TI reset controller for MT8192 SoC [not found] ` <5065a23627a34212aa62df646dbf00ee@mtkmbs05n1.mediatek.inc> 2020-09-02 3:03 ` Crystal Guo @ 2020-09-02 3:03 ` Crystal Guo 0 siblings, 0 replies; 82+ messages in thread From: Crystal Guo @ 2020-09-02 3:03 UTC (permalink / raw) To: Rob Herring, p.zabel, matthias.bgg, s-anna, afd Cc: srv_heupstream, linux-mediatek, linux-arm-kernel, linux-kernel, devicetree, Seiya Wang (王迺君), Stanley Chu (朱原陞), Yingjoe Chen (陳英洲), Fan Chen (陳凡), Yong Liang (梁勇) Hi Rob, Philipp, Matthias and all Gentle ping for this patch set. Thanks Crystal > > -----Original Message----- > From: Crystal Guo [mailto:crystal.guo@mediatek.com] > Sent: Monday, August 17, 2020 11:03 AM > To: p.zabel@pengutronix.de; robh+dt@kernel.org; matthias.bgg@gmail.com > Cc: srv_heupstream; linux-mediatek@lists.infradead.org; linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; devicetree@vger.kernel.org; s-anna@ti.com; afd@ti.com; Seiya Wang (王迺君); Stanley Chu (朱原陞); Yingjoe Chen (陳英洲); Fan Chen (陳凡); Yong Liang (梁勇) > Subject: [v4,0/4] introduce TI reset controller for MT8192 SoC > > v4: > fix typos on v3 commit message. > > v3: > 1. revert v2 changes. > 2. add 'reset-duration-us' property to declare a minimum delay, which needs to be waited between assert and deassert. > 3. add 'mediatek,infra-reset' to compatible. > > > v2 changes: > https://patchwork.kernel.org/patch/11697371/ > 1. add 'assert-deassert-together' property to introduce a new reset handler, which allows device to do serialized assert and deassert operations in a single step by 'reset' method. > 2. add 'update-force' property to introduce force-update method, which forces the write operation in case the read already happens to return the correct value. > 3. add 'generic-reset' to compatible > > v1 changes: > https://patchwork.kernel.org/patch/11690523/ > https://patchwork.kernel.org/patch/11690527/ > > Crystal Guo (4): > dt-binding: reset-controller: ti: add reset-duration-us property > dt-binding: reset-controller: ti: add 'mediatek,infra-reset' to > compatible > reset-controller: ti: introduce a new reset handler > arm64: dts: mt8192: add infracfg_rst node > > .../bindings/reset/ti-syscon-reset.txt | 6 +++++ > arch/arm64/boot/dts/mediatek/mt8192.dtsi | 11 +++++++- > drivers/reset/reset-ti-syscon.c | 26 +++++++++++++++++-- > 3 files changed, 40 insertions(+), 3 deletions(-) > > > *********************MEDIATEK Confidential/Internal Use********************* ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,0/4] introduce TI reset controller for MT8192 SoC @ 2020-09-02 3:03 ` Crystal Guo 0 siblings, 0 replies; 82+ messages in thread From: Crystal Guo @ 2020-09-02 3:03 UTC (permalink / raw) To: Rob Herring, p.zabel, matthias.bgg, s-anna, afd Cc: devicetree, Yong Liang (梁勇), srv_heupstream, Seiya Wang (王迺君), linux-kernel, Fan Chen (陳凡), linux-mediatek, Yingjoe Chen (陳英洲), Stanley Chu (朱原陞), linux-arm-kernel Hi Rob, Philipp, Matthias and all Gentle ping for this patch set. Thanks Crystal > > -----Original Message----- > From: Crystal Guo [mailto:crystal.guo@mediatek.com] > Sent: Monday, August 17, 2020 11:03 AM > To: p.zabel@pengutronix.de; robh+dt@kernel.org; matthias.bgg@gmail.com > Cc: srv_heupstream; linux-mediatek@lists.infradead.org; linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; devicetree@vger.kernel.org; s-anna@ti.com; afd@ti.com; Seiya Wang (王迺君); Stanley Chu (朱原陞); Yingjoe Chen (陳英洲); Fan Chen (陳凡); Yong Liang (梁勇) > Subject: [v4,0/4] introduce TI reset controller for MT8192 SoC > > v4: > fix typos on v3 commit message. > > v3: > 1. revert v2 changes. > 2. add 'reset-duration-us' property to declare a minimum delay, which needs to be waited between assert and deassert. > 3. add 'mediatek,infra-reset' to compatible. > > > v2 changes: > https://patchwork.kernel.org/patch/11697371/ > 1. add 'assert-deassert-together' property to introduce a new reset handler, which allows device to do serialized assert and deassert operations in a single step by 'reset' method. > 2. add 'update-force' property to introduce force-update method, which forces the write operation in case the read already happens to return the correct value. > 3. add 'generic-reset' to compatible > > v1 changes: > https://patchwork.kernel.org/patch/11690523/ > https://patchwork.kernel.org/patch/11690527/ > > Crystal Guo (4): > dt-binding: reset-controller: ti: add reset-duration-us property > dt-binding: reset-controller: ti: add 'mediatek,infra-reset' to > compatible > reset-controller: ti: introduce a new reset handler > arm64: dts: mt8192: add infracfg_rst node > > .../bindings/reset/ti-syscon-reset.txt | 6 +++++ > arch/arm64/boot/dts/mediatek/mt8192.dtsi | 11 +++++++- > drivers/reset/reset-ti-syscon.c | 26 +++++++++++++++++-- > 3 files changed, 40 insertions(+), 3 deletions(-) > > > *********************MEDIATEK Confidential/Internal Use********************* _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [v4,0/4] introduce TI reset controller for MT8192 SoC @ 2020-09-02 3:03 ` Crystal Guo 0 siblings, 0 replies; 82+ messages in thread From: Crystal Guo @ 2020-09-02 3:03 UTC (permalink / raw) To: Rob Herring, p.zabel, matthias.bgg, s-anna, afd Cc: devicetree, Yong Liang (梁勇), srv_heupstream, Seiya Wang (王迺君), linux-kernel, Fan Chen (陳凡), linux-mediatek, Yingjoe Chen (陳英洲), Stanley Chu (朱原陞), linux-arm-kernel Hi Rob, Philipp, Matthias and all Gentle ping for this patch set. Thanks Crystal > > -----Original Message----- > From: Crystal Guo [mailto:crystal.guo@mediatek.com] > Sent: Monday, August 17, 2020 11:03 AM > To: p.zabel@pengutronix.de; robh+dt@kernel.org; matthias.bgg@gmail.com > Cc: srv_heupstream; linux-mediatek@lists.infradead.org; linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; devicetree@vger.kernel.org; s-anna@ti.com; afd@ti.com; Seiya Wang (王迺君); Stanley Chu (朱原陞); Yingjoe Chen (陳英洲); Fan Chen (陳凡); Yong Liang (梁勇) > Subject: [v4,0/4] introduce TI reset controller for MT8192 SoC > > v4: > fix typos on v3 commit message. > > v3: > 1. revert v2 changes. > 2. add 'reset-duration-us' property to declare a minimum delay, which needs to be waited between assert and deassert. > 3. add 'mediatek,infra-reset' to compatible. > > > v2 changes: > https://patchwork.kernel.org/patch/11697371/ > 1. add 'assert-deassert-together' property to introduce a new reset handler, which allows device to do serialized assert and deassert operations in a single step by 'reset' method. > 2. add 'update-force' property to introduce force-update method, which forces the write operation in case the read already happens to return the correct value. > 3. add 'generic-reset' to compatible > > v1 changes: > https://patchwork.kernel.org/patch/11690523/ > https://patchwork.kernel.org/patch/11690527/ > > Crystal Guo (4): > dt-binding: reset-controller: ti: add reset-duration-us property > dt-binding: reset-controller: ti: add 'mediatek,infra-reset' to > compatible > reset-controller: ti: introduce a new reset handler > arm64: dts: mt8192: add infracfg_rst node > > .../bindings/reset/ti-syscon-reset.txt | 6 +++++ > arch/arm64/boot/dts/mediatek/mt8192.dtsi | 11 +++++++- > drivers/reset/reset-ti-syscon.c | 26 +++++++++++++++++-- > 3 files changed, 40 insertions(+), 3 deletions(-) > > > *********************MEDIATEK Confidential/Internal Use********************* _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek ^ permalink raw reply [flat|nested] 82+ messages in thread
end of thread, other threads:[~2020-09-29 13:56 UTC | newest] Thread overview: 82+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-08-17 3:03 [v4,0/4] introduce TI reset controller for MT8192 SoC Crystal Guo 2020-08-17 3:03 ` Crystal Guo 2020-08-17 3:03 ` Crystal Guo 2020-08-17 3:03 ` [v4,1/4] dt-binding: reset-controller: ti: add reset-duration-us property Crystal Guo 2020-08-17 3:03 ` [v4, 1/4] " Crystal Guo 2020-08-17 3:03 ` Crystal Guo 2020-08-25 17:42 ` [v4,1/4] " Rob Herring 2020-08-25 17:42 ` [v4, 1/4] " Rob Herring 2020-08-25 17:42 ` Rob Herring 2020-08-26 11:09 ` [v4,1/4] " Crystal Guo 2020-08-26 11:09 ` Crystal Guo 2020-08-26 11:09 ` Crystal Guo 2020-08-17 3:03 ` [v4,2/4] dt-binding: reset-controller: ti: add 'mediatek,infra-reset' to compatible Crystal Guo 2020-08-17 3:03 ` [v4, 2/4] dt-binding: reset-controller: ti: add 'mediatek, infra-reset' " Crystal Guo 2020-08-17 3:03 ` Crystal Guo 2020-08-25 19:02 ` [v4,2/4] dt-binding: reset-controller: ti: add 'mediatek,infra-reset' " Rob Herring 2020-08-25 19:02 ` Rob Herring 2020-08-25 19:02 ` Rob Herring 2020-08-26 11:09 ` Crystal Guo 2020-08-26 11:09 ` Crystal Guo 2020-08-26 11:09 ` Crystal Guo 2020-09-02 23:25 ` Suman Anna 2020-09-02 23:25 ` Suman Anna 2020-09-02 23:25 ` Suman Anna 2020-09-08 18:49 ` Rob Herring 2020-09-08 18:49 ` Rob Herring 2020-09-08 18:49 ` Rob Herring 2020-09-09 15:10 ` Suman Anna 2020-09-09 15:10 ` Suman Anna 2020-09-09 15:10 ` Suman Anna 2020-09-09 18:20 ` Rob Herring 2020-09-09 18:20 ` Rob Herring 2020-09-09 18:20 ` Rob Herring 2020-08-17 3:03 ` [v4,3/4] reset-controller: ti: introduce a new reset handler Crystal Guo 2020-08-17 3:03 ` Crystal Guo 2020-08-17 3:03 ` Crystal Guo 2020-09-02 23:40 ` Suman Anna 2020-09-02 23:40 ` Suman Anna 2020-09-02 23:40 ` Suman Anna 2020-09-09 2:57 ` Crystal Guo 2020-09-09 2:57 ` Crystal Guo 2020-09-09 2:57 ` Crystal Guo 2020-09-09 15:39 ` Suman Anna 2020-09-09 15:39 ` Suman Anna 2020-09-09 15:39 ` Suman Anna 2020-09-11 2:42 ` Crystal Guo 2020-09-11 2:42 ` Crystal Guo 2020-09-11 2:42 ` Crystal Guo 2020-09-11 2:52 ` Suman Anna 2020-09-11 2:52 ` Suman Anna 2020-09-11 2:52 ` Suman Anna 2020-09-11 6:07 ` Crystal Guo 2020-09-11 6:07 ` Crystal Guo 2020-09-11 6:07 ` Crystal Guo 2020-09-11 14:26 ` Philipp Zabel 2020-09-11 14:26 ` Philipp Zabel 2020-09-11 14:26 ` Philipp Zabel 2020-09-11 14:44 ` Suman Anna 2020-09-11 14:44 ` Suman Anna 2020-09-11 14:44 ` Suman Anna 2020-09-11 14:44 ` Suman Anna 2020-09-14 14:00 ` Crystal Guo 2020-09-14 14:00 ` Crystal Guo 2020-09-14 14:00 ` Crystal Guo 2020-09-29 13:54 ` Crystal Guo 2020-09-29 13:54 ` Crystal Guo 2020-09-29 13:54 ` Crystal Guo 2020-08-17 3:03 ` [v4,4/4] arm64: dts: mt8192: add infracfg_rst node Crystal Guo 2020-08-17 3:03 ` Crystal Guo 2020-08-17 3:03 ` Crystal Guo 2020-09-02 23:29 ` Suman Anna 2020-09-02 23:29 ` Suman Anna 2020-09-02 23:29 ` Suman Anna 2020-09-08 13:26 ` Crystal Guo 2020-09-08 13:26 ` Crystal Guo 2020-09-08 13:26 ` Crystal Guo 2020-09-08 15:51 ` Suman Anna 2020-09-08 15:51 ` Suman Anna 2020-09-08 15:51 ` Suman Anna [not found] ` <5065a23627a34212aa62df646dbf00ee@mtkmbs05n1.mediatek.inc> 2020-09-02 3:03 ` [v4,0/4] introduce TI reset controller for MT8192 SoC Crystal Guo 2020-09-02 3:03 ` Crystal Guo 2020-09-02 3:03 ` Crystal Guo
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.