On 23/03/2022 10:17, Aswath Govindraju wrote: > Hi Krzysztof, (...) >>> + >>> + ti,syscon-phy-pll-refclk: >>> + $ref: /schemas/types.yaml#/definitions/phandle-array >>> + items: >>> + - items: >>> + - description: Phandle to the SYSCON entry >>> + - description: USB phy control register offset within SYSCON >>> + description: Specifier for configuring frequency of ref clock input. >> >> This is a bit strange. The ref clock is the "ref" input clock, right? In >> such case use clk_set_rate()... Using syscon for managing clocks is not >> the proper way. >> > > The syscon property is not being used to set the ref clock frequency but > is rather being used to indicate the input clock frequency for USB PHY > operation. I think the description seems be misleading. I will update it > in the respin, to reflect the above description. Yes, please, it will help. > >> Plus all the issues pointed out by Roger. >> >>> + >>> + '#address-cells': >>> + const: 2 >>> + >>> + '#size-cells': >>> + const: 2 >> >> No children allowed? >> >> I understand this is a wrapper, which explains why you do not include >> usb-hcd.yaml schema. But since it is a wrapper, what is it wrapping? >> > > Yes, there is a child node, which would be the dwc3-contoller node. > Would adding the child node too in the example help capture this better? Yes, please, because then you will also spot errors when running dt_binding_check. You need patternProperties for "^usb@[0-9a-f]+$" referencing Synopsys schema. Something like: Documentation/devicetree/bindings/usb/samsung,exynos-dwc3.yaml Best regards, Krzysztof