On 21/06/2022 21:44, Krzysztof Kozlowski wrote: > On 21/06/2022 09:42, Wangseok Lee wrote: >>>> >>>> samsung,syscon-bus-s-fsys: >>>> description: >>>> Phandle to bus-s path of fsys block, this register >>>> are used for enabling bus-s. >>>> $ref: /schemas/types.yaml#/definitions/phandle >>>> >>>> samsung,syscon-bus-p-fsys: >>>> description: >>>> Phandle to bus-p path of fsys block, this register >>>> are used for enabling bus-p. >>>> $ref: /schemas/types.yaml#/definitions/phandle >>> >>> This two look unspecific and hacky workaround for missing drivers. Looks >>> like instead of implementing interconnect or clock driver, you decided >>> to poke some other registers. Why this cannot be an interconnect driver? >>> >>> >> >> bus-s, bus-p is a register that exists in the sysreg of the fsys block. >> It is the same block as "fsys-sysreg" but is separated separately in >> hardware. > > Two points here: > 1. If it is in FSYS, why it cannot be accessed with samsung,fsys-sysreg? > 2. If it is only register, shuld be described like this. You must > describe item: > https://protect2.fireeye.com/v1/url?k=0f529a57-50c9a332-0f531118-000babff32e3-50938d8198077d59&q=1&e=32284e69-bbed-4d09-b6d6-0a43428aebf5&u=https%3A%2F%2Felixir.bootlin.com%2Flinux%2Fv5.18-rc1%2Fsource%2FDocumentation%2Fdevicetree%2Fbindings%2Fsoc%2Fsamsung%2Fexynos-usi.yaml%23L42 > It would be better to access with fsys-sysreg, but their h/w address are far from each other. The fsys block consists of a system register and an additional control system register. "bus-s-fsys" and "bus-p-fsys" are additional control system register. sysreg and additional control sysreg addresses are far from each other and there are h/w registers that perform different functions between them. >> So, get resource is performed separately from "fsys-sysreg". >> They set pcie slave, dbi related control settings, >> naming "bus-x" seems to be interconnect. >> I will add this description to property. >> I don't think it need to use the interconnect driver, >> so please let me know your opinion. > > Please document both in the bindings and in the driver usage of this > register. Writing there "0" or "1" is not enough. If the documentation > is good, I am fine with it. If the explanation is obfuscated/not > sufficient, it will look like avoiding to implement a driver, which I > don't want to accept. > I think i should add enough description. Is it sufficient to modify the name and description of property like this? samsung,fsys-bus-s: description: Phandle to bus-s of fsys block, this register is additional control sysreg in fsys block and this is used for pcie slave control setting. $ref: /schemas/types.yaml#/definitions/phandle samsung,fsys-bus-p: description: Phandle to bus-p of fsys block, this register is additional control sysreg in fsys block and this is used for pcie dbi control setting. $ref: /schemas/types.yaml#/definitions/phandle > Best regards, > Krzysztof Thank you for kindness reivew. Best regards, Wangseok Lee
