On Tue, Dec 26, 2023 at 8:18 PM Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> wrote: > > On 26/12/2023 11:04, Jingbao Qiu wrote: > > Add devicetree binding for Sophgo CV1800 SoC MFD subsys. > > Subject and commit msg: there is no such hardware as "MFD subsys". Is > this a PMIC? Does not look like. You must describe here hardware, not > Linux subsystem. > I don't think this is a PMIC device. the RTC restart and 8051 configure register share one common range address, and the address have other function that power management. the datasheet link: Link: https://github.com/milkv-duo/duo-files/blob/main/duo/datasheet/CV1800B-CV1801B-Preliminary-Datasheet-full-en.pdf chapter: 3.9 RTC 3.12 8051 subsystem > > > > Signed-off-by: Jingbao Qiu <qiujingbao.dlmu@xxxxxxxxx> > > --- > > Please mention the dependency here. Thanks,I will fix it. > > > .../bindings/mfd/sophgo,cv1800-subsys.yaml | 51 +++++++++++++++++++ > > 1 file changed, 51 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/mfd/sophgo,cv1800-subsys.yaml > > > > diff --git a/Documentation/devicetree/bindings/mfd/sophgo,cv1800-subsys.yaml b/Documentation/devicetree/bindings/mfd/sophgo,cv1800-subsys.yaml > > new file mode 100644 > > index 000000000000..c2a071c8a2de > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/mfd/sophgo,cv1800-subsys.yaml > > @@ -0,0 +1,51 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/mfd/sophgo,cv1800-subsys.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Sophgo CV1800 SoC subsys controller > > + > > +maintainers: > > + - Jingbao Qiu <qiujingbao.dlmu@xxxxxxxxx> > > + > > +description: > > + The Sophgo CV1800 SoC subsys controller contains many functions > > What is "subsys"? Why is it in MFD directory? SoC components like > system-controllers do not go to MFD. This device has multiple different functions, because they have 8051 subsystem, so I named him "subsys". I will carefully consider and rename it. > > > + for example, RTC and restart. In addition, CV1800 has an 8051 > > + subsystem, which is configured through registers at this controller. > > + > > +properties: > > + compatible: > > + items: > > + - const: sophgo,cv1800b-subsys > > + - const: syscon > > + - const: simple-mfd > > + > > + reg: > > + maxItems: 1 > > + > > + rtc: > > + $ref: /schemas/rtc/sophgo,cv1800-rtc.yaml# > > Your patchset is not bisectable. What's more, you have dependency > between patches, so bindings cannot go via separate trees: mfd and rtc. > You need to make this *EXPLICIT* in the cover letter or patch changelog. ok,I will fix it. > > I do not see any resources in MFD block, so why having it as separate > node? What other devices you did not describe here? You mentioned > restart and 8051, so where are they? Which driver implements them? > I'am sorry for that other drivers have not been implemented yet. I will implement it after rtc. They have the same address range, so I use mfd to describe them. > > Best regards, > Krzysztof > Best regards, Jingbao Qiu