Thanks for all of the comments. All feedback is ACK'd and will be added in v2 -- what follows is just commentary on some comments. On Sun, Jan 29, 2023 at 5:33 AM Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> wrote: > > On 29/01/2023 12:05, Krzysztof Kozlowski wrote: > > On 28/01/2023 21:26, Danny Kaehn wrote: > >> This is a USB HID device which includes an I2C controller and 8 GPIO pins. > >> > > Thank you for your patch. There is something to discuss/improve. > > > >> The binding allows describing the chip's gpio and i2c controller in DT > >> using the subnodes named "gpio" and "i2c", respectively. This is > >> intended to be used in configurations where the CP2112 is permanently > >> connected in hardware. > >> > >> Signed-off-by: Danny Kaehn <kaehndan@xxxxxxxxx> > >> --- > >> .../bindings/hid/silabs,cp2112.yaml | 82 +++++++++++++++++++ > > > > There is no "hid" directory, so I think such devices where going to > > different place, didn't they? Good point, I didn't notice other hid-related bindings went into input/ -- will change > > > >> + While USB devices typically aren't described in DeviceTree, doing so with the > >> + CP2112 allows use of its i2c and gpio controllers with other DT nodes when > >> + the chip is expected to be found on a USB port. > > > > Drop these three and replace with description of the hardware. Understood. I noticed that a similar usb-based binding included a similar description (net/marvell,mvusb.yaml) but I understand why we would not want this in new bindings. > > > >> + i2c: > >> + $ref: /schemas/i2c/i2c-controller.yaml# > > > > This is not specific enough. What controller is there? > > OK, assuming this is tightly wired (with cp2112 I2C controller), then > the compatible could be skipped as it is inferred from parent one. Yet > still you need description and unevaluatedProperties. > Great point, will update -- I didn't quite understand that child nodes of the root could have properties/unevaluatedProperties/etc.. but I see now that that is well-documented (just not often done in existing bindings)! > > > > Missing unevaluatedProperties: false, anyway. > > > >> + gpio: > >> + $ref: /schemas/gpio/gpio.yaml# > > > > Same comments. > > Description, unevaluatedProperties and constraints on properties (line > names, reserved ranges, ranges). > Will add. Thanks, Danny Kaehn
