On 22/06/2022 09:21, Wangseok Lee wrote: > 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? > Looks ok. Thank you. Best regards, Krzysztof
