On Fri, Jul 05, 2024 at 11:50:09AM GMT, Linus Walleij wrote: > Hi Inochi, > > thanks for your patch! > > Some comments below: > > On Thu, Jul 4, 2024 at 7:47 AM Inochi Amaoto <inochiama@xxxxxxxxxxx> wrote: > > > Add pinctrl support for Sophgo CV1800 series SoC. > > > > Signed-off-by: Inochi Amaoto <inochiama@xxxxxxxxxxx> > (...) > > + bias-pull-up: > > + type: boolean > > description: Setting this true will result in how many Ohms of pull up > and to what voltage? > > > + bias-pull-down: > > + type: boolean > > How many Ohms by default? It's very helpful for designers writing the > device tree to know this. > Pullup in 79k and pulldown is 87k. I will add these to the description. > > + drive-strength: > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + minimum: 0 > > + maximum: 7 > > 0,1, ... 7 ... units of what? What does the values represent? > 7 mA? I think not. This should be in mA and parsed to any custom > values. > > If you need more resolution, consider using driver-strength-microamp and > parse that value instead. > This value depends on the type of the pin. Different pin have different typical drive strength. I will switch to the real value and check it in the driver. > > + input-schmitt: > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + minimum: 0 > > + maximum: 7 > > What is this custom property? We support input-schmitt-enable > and input-schmitt-disable, if you want to add a new standard property > input-schmitt, then send a separate patch to > Documentation/devicetree/bindings/pinctrl/pincfg-node.yaml > and explain what the argument means, it should be in SI units. > > For a full custom property, use sophgo, prefixes. > Thanks for the reminder. I have seen `input-schmitt` in the generic pinconf table. I will sumbit a new patch and add this to the pincfg-node. > > + slew-rate: > > + enum: [ 0, 1 ] > > What is slew rate 0 and 1? What does it represent? > > microvolts per second how much (I don't know, just guessing) > According to the document from sophgo, 0 is fast rate and 1 is slow. > > + sophgo,bus-holder: > > + description: enable bus holder > > This description is a bit too little, we need to know what this thing > is and what it is > used for. If it is definied in some other DT binding then reference that file. > > Yours, > Linus Walleij Thanks I will improve it. Regards, Inochi
