| Message ID | 20200817030324.5690-3-crystal.guo@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | introduce TI reset controller for MT8192 SoC | expand |
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
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
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 >
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
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 >
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
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
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(+)