On 12/10/2022 16:25, Hawkins, Nick wrote: > Greetings Krysztof, > > Thanks for the feedback! I have several questions below: > >>> + >>> +static ssize_t server_id_show(struct device *dev, struct >>> +device_attribute *attr, char *buf) { >>> + struct gxp_plreg_drvdata *drvdata = dev_get_drvdata(dev); >>> + int value_upper; >>> + int value_lower; >>> + ssize_t ret; >>> + u32 trans_offset; >>> + u32 trans_shift; >>> + >>> + /* read upper first */ >>> + address_translation(drvdata->server_id.upper[BYTE], >>> + &trans_offset, >>> + &trans_shift); >>> + regmap_read(drvdata->plreg_map, trans_offset, &value_upper); >>> + value_upper = value_upper >> trans_shift; >>> + value_upper = value_upper & drvdata->server_id.upper[MASK]; >>> + >>> + /* read lower last */ >>> + address_translation(drvdata->server_id.lower[BYTE], >>> + &trans_offset, >>> + &trans_shift); >>> + regmap_read(drvdata->plreg_map, trans_offset, &value_lower); >>> + value_lower = value_lower >> trans_shift; >>> + value_lower = value_lower & drvdata->server_id.lower[MASK]; >>> + >>> + ret = sprintf(buf, "0x%04x", value_upper | value_lower); >>> + >>> + return ret; >>> +} >>> + >>> +static DEVICE_ATTR_RO(server_id); > >> Missing sysfs documentation. > > Can you point me at the proper location / documentation for documenting sysfs? Thanks! Documentation/ABI/README > >>> + for (i = 0; i <= MASK; i++) { >>> + if (of_property_read_u32_index(np, "grp5", i, >>> + &drvdata->grp_intr_flags.grp5[i])) { >>> + dev_err(&pdev->dev, >>> + "grp5intsflags is missing its 'grp5' property index %d\n", i); >>> + return -ENODEV; >>> + } >>> + } >>> + >>> + np = of_get_child_by_name(pdev->dev.of_node, "pwrbtn"); >>> + if (!np) { >>> + dev_err(&pdev->dev, "%pOF is missing its 'pwrbtn' node\n", np); >>> + return -ENODEV; >>> + } >>> + >>> + for (i = 0; i <= VALUE; i++) { >>> + if (of_property_read_u32_index(np, "latch", i, >>> +&drvdata->pwrbtn.latch[i])) { > >> Undocumented properties. NAK. > > If each child node of hpe,gxp-plreg were documented with their respective properties would this be acceptable? I would need to see the bindings. Best regards, Krzysztof