| Message ID | 20230918045125.4000083-2-k.son@samsung.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Introduce reset driver for T-HEAD th1520 SoC | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On Mon, Sep 18, 2023 at 04:51:23AM +0000, Kwanghoon Son wrote: > Add documentation to describe th1520 reset device I would appreciate if you could add some information here as to what is missing from this - the reset controller isn't just for two WDTs is it? > > Signed-off-by: Kwanghoon Son <k.son@samsung.com> > --- > .../bindings/reset/thead,th1520-reset.yaml | 44 +++++++++++++++++++ > .../dt-bindings/reset/thead,th1520-reset.h | 9 ++++ > 2 files changed, 53 insertions(+) > create mode 100644 Documentation/devicetree/bindings/reset/thead,th1520-reset.yaml > create mode 100644 include/dt-bindings/reset/thead,th1520-reset.h > > diff --git a/Documentation/devicetree/bindings/reset/thead,th1520-reset.yaml b/Documentation/devicetree/bindings/reset/thead,th1520-reset.yaml > new file mode 100644 > index 000000000000..49ea8c6a331f > --- /dev/null > +++ b/Documentation/devicetree/bindings/reset/thead,th1520-reset.yaml > @@ -0,0 +1,44 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/reset/thead,th1520-reset.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: T-HEAD th1520 SoC Reset Controller > + > +maintainers: > + - Kwanghoon Son <k.son@samsung.com> > + > +properties: > + compatible: > + items: > + - const: thead,th1520-reset > + - const: syscon iDumb question perhaps, but why is this a syscon? Cheers, Conor. > + > + reg: > + maxItems: 1 > + > + '#reset-cells': > + const: 1 > + > +required: > + - compatible > + - reg > + - '#reset-cells' > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/reset/thead,th1520-reset.h> > + > + soc { > + #address-cells = <2>; > + #size-cells = <2>; > + > + reset-controller@ffef014000 { > + compatible = "thead,th1520-reset", "syscon"; > + reg = <0xff 0xef014000 0x0 0x1000>; > + #reset-cells = <1>; > + }; > + }; > diff --git a/include/dt-bindings/reset/thead,th1520-reset.h b/include/dt-bindings/reset/thead,th1520-reset.h > new file mode 100644 > index 000000000000..ec10751814e5 > --- /dev/null > +++ b/include/dt-bindings/reset/thead,th1520-reset.h > @@ -0,0 +1,9 @@ > +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */ > + > +#ifndef DT_BINDING_RESET_TH1520_H > +#define DT_BINDING_RESET_TH1520_H > + > +#define TH1520_RESET_WDT0 0 > +#define TH1520_RESET_WDT1 1
On Mon, Sep 18, 2023 at 04:51:23AM +0000, Kwanghoon Son wrote: > Add documentation to describe th1520 reset device > > Signed-off-by: Kwanghoon Son <k.son@samsung.com> > --- > .../bindings/reset/thead,th1520-reset.yaml | 44 +++++++++++++++++++ > .../dt-bindings/reset/thead,th1520-reset.h | 9 ++++ > 2 files changed, 53 insertions(+) > create mode 100644 Documentation/devicetree/bindings/reset/thead,th1520-reset.yaml > create mode 100644 include/dt-bindings/reset/thead,th1520-reset.h > > diff --git a/Documentation/devicetree/bindings/reset/thead,th1520-reset.yaml b/Documentation/devicetree/bindings/reset/thead,th1520-reset.yaml > new file mode 100644 > index 000000000000..49ea8c6a331f > --- /dev/null > +++ b/Documentation/devicetree/bindings/reset/thead,th1520-reset.yaml > @@ -0,0 +1,44 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/reset/thead,th1520-reset.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: T-HEAD th1520 SoC Reset Controller > + > +maintainers: > + - Kwanghoon Son <k.son@samsung.com> > + > +properties: > + compatible: > + items: > + - const: thead,th1520-reset > + - const: syscon Why do you need "syscon"? Are there other functions? If so, please describe them in top-level "description". If not, drop "syscon" and use device_node_to_regmap() instead of syscon_node_to_regmap(). Rob
> > + - const: thead,th1520-reset > > + - const: syscon > > iDumb question perhaps, but why is this a syscon? This is good point. In fact, I took it from the vendor kernel, and I tried to keep it as same as I could. Vendor also followed drivers/reset/reset-imx7.c As Rob said, if don't need it, I'll change it. Regards, Kwang. > Cheers, > Conor. > > > + > > + reg: > > + maxItems: 1 > > + > > + '#reset-cells': > > + const: 1
> > > + - const: thead,th1520-reset > > > + - const: syscon > > > > iDumb question perhaps, but why is this a syscon? > > This is good point. > In fact, I took it from the vendor kernel, and I tried to keep it as same as I could. > Vendor also followed drivers/reset/reset-imx7.c > As Rob said, if don't need it, I'll change it. > > Regards, > Kwang. > I have investigated vendor kernels, but it is still not sure reset information. I thought this is about sys_reg, but without datasheet, can't tell. Therefore, should be stopped patch for a while until knows better comes out. Kwang. > > Cheers, > > Conor. > > > > > + > > > + reg: > > > + maxItems: 1 > > > + > > > + '#reset-cells': > > > + const: 1
Guo, On Tue, Sep 19, 2023 at 02:08:32PM +0900, Kwanghoon Son wrote: > > > > + - const: thead,th1520-reset > > > > + - const: syscon > > > > > > iDumb question perhaps, but why is this a syscon? > > > > This is good point. > > In fact, I took it from the vendor kernel, and I tried to keep it as same as I could. > > Vendor also followed drivers/reset/reset-imx7.c > > As Rob said, if don't need it, I'll change it. > I have investigated vendor kernels, but it is still not sure reset information. > I thought this is about sys_reg, but without datasheet, can't tell. > Therefore, should be stopped patch for a while until knows better comes out. Is there documentation in English you can provide to these people that are trying to add support for your product? Thanks, Conor.
On Tue, Sep 19, 2023 at 4:47 PM Conor Dooley <conor@kernel.org> wrote: > > Guo, > > On Tue, Sep 19, 2023 at 02:08:32PM +0900, Kwanghoon Son wrote: > > > > > + - const: thead,th1520-reset > > > > > + - const: syscon > > > > > > > > iDumb question perhaps, but why is this a syscon? > > > > > > This is good point. > > > In fact, I took it from the vendor kernel, and I tried to keep it as same as I could. > > > Vendor also followed drivers/reset/reset-imx7.c > > > As Rob said, if don't need it, I'll change it. > > > I have investigated vendor kernels, but it is still not sure reset information. > > I thought this is about sys_reg, but without datasheet, can't tell. > > Therefore, should be stopped patch for a while until knows better comes out. > > Is there documentation in English you can provide to these people that > are trying to add support for your product? Yes, Fuwei would help with this, and thx for all of your work. > > Thanks, > Conor.
On Tue, 2023-09-19 at 09:47 +0100, Conor Dooley wrote: > Guo, > > On Tue, Sep 19, 2023 at 02:08:32PM +0900, Kwanghoon Son wrote: > > > > > + - const: thead,th1520-reset > > > > > + - const: syscon > > > > > > > > iDumb question perhaps, but why is this a syscon? > > > > > > This is good point. > > > In fact, I took it from the vendor kernel, and I tried to keep it as same as I could. > > > Vendor also followed drivers/reset/reset-imx7.c > > > As Rob said, if don't need it, I'll change it. > > > I have investigated vendor kernels, but it is still not sure reset information. > > I thought this is about sys_reg, but without datasheet, can't tell. > > Therefore, should be stopped patch for a while until knows better comes out. > > Is there documentation in English you can provide to these people that > are trying to add support for your product? It can be downloaded from https://dl.sipeed.com/shareURL/LICHEE/licheepi4a/09_Doc now. Click on "TH1520 User Manual.zip" and enter a captcha. The doc about resetting is in chapter 5 of "TH1520 System User Manual.pdf".
On Thu, Nov 30, 2023 at 12:42:09AM +0800, Xi Ruoyao wrote: > On Tue, 2023-09-19 at 09:47 +0100, Conor Dooley wrote: > > Guo, > > > > On Tue, Sep 19, 2023 at 02:08:32PM +0900, Kwanghoon Son wrote: > > > > > > + - const: thead,th1520-reset > > > > > > + - const: syscon > > > > > > > > > > iDumb question perhaps, but why is this a syscon? > > > > > > > > This is good point. > > > > In fact, I took it from the vendor kernel, and I tried to keep it as same as I could. > > > > Vendor also followed drivers/reset/reset-imx7.c > > > > As Rob said, if don't need it, I'll change it. > > > > > I have investigated vendor kernels, but it is still not sure reset information. > > > I thought this is about sys_reg, but without datasheet, can't tell. > > > Therefore, should be stopped patch for a while until knows better comes out. > > > > Is there documentation in English you can provide to these people that > > are trying to add support for your product? > > It can be downloaded from > https://dl.sipeed.com/shareURL/LICHEE/licheepi4a/09_Doc now. Click on > "TH1520 User Manual.zip" and enter a captcha. The doc about resetting > is in chapter 5 of "TH1520 System User Manual.pdf". Thanks Xi! Pity there was no response from the vendor to this for months :(
diff --git a/Documentation/devicetree/bindings/reset/thead,th1520-reset.yaml b/Documentation/devicetree/bindings/reset/thead,th1520-reset.yaml new file mode 100644 index 000000000000..49ea8c6a331f --- /dev/null +++ b/Documentation/devicetree/bindings/reset/thead,th1520-reset.yaml @@ -0,0 +1,44 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/reset/thead,th1520-reset.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: T-HEAD th1520 SoC Reset Controller + +maintainers: + - Kwanghoon Son <k.son@samsung.com> + +properties: + compatible: + items: + - const: thead,th1520-reset + - const: syscon + + reg: + maxItems: 1 + + '#reset-cells': + const: 1 + +required: + - compatible + - reg + - '#reset-cells' + +additionalProperties: false + +examples: + - | + #include <dt-bindings/reset/thead,th1520-reset.h> + + soc { + #address-cells = <2>; + #size-cells = <2>; + + reset-controller@ffef014000 { + compatible = "thead,th1520-reset", "syscon"; + reg = <0xff 0xef014000 0x0 0x1000>; + #reset-cells = <1>; + }; + }; diff --git a/include/dt-bindings/reset/thead,th1520-reset.h b/include/dt-bindings/reset/thead,th1520-reset.h new file mode 100644 index 000000000000..ec10751814e5 --- /dev/null +++ b/include/dt-bindings/reset/thead,th1520-reset.h @@ -0,0 +1,9 @@ +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */ + +#ifndef DT_BINDING_RESET_TH1520_H +#define DT_BINDING_RESET_TH1520_H + +#define TH1520_RESET_WDT0 0 +#define TH1520_RESET_WDT1 1 + +#endif
Add documentation to describe th1520 reset device Signed-off-by: Kwanghoon Son <k.son@samsung.com> --- .../bindings/reset/thead,th1520-reset.yaml | 44 +++++++++++++++++++ .../dt-bindings/reset/thead,th1520-reset.h | 9 ++++ 2 files changed, 53 insertions(+) create mode 100644 Documentation/devicetree/bindings/reset/thead,th1520-reset.yaml create mode 100644 include/dt-bindings/reset/thead,th1520-reset.h