Messages in this thread |  | | Date | Tue, 5 Jul 2022 11:46:43 +0200 | Subject | Re: [PATCH v5 06/15] dt-bindings: mfd: amd,pensando-elbasr: Add AMD Pensando Elba System Resource chip | From | Krzysztof Kozlowski <> |
| |
On 04/07/2022 01:41, Brad Larson wrote: > Hi Krzysztof, > > On Mon, Jun 20, 2022 at 5:56 AM Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org> wrote: >> >> On 13/06/2022 21:56, Brad Larson wrote: >>> From: Brad Larson <blarson@amd.com> >>> >>> Add support for the AMD Pensando Elba SoC System Resource chip >>> using the SPI interface. The Elba SR is a Multi-function Device >>> supporting device register access using CS0, smbus interface for >>> FRU and board peripherals using CS1, dual Lattice I2C masters for >>> transceiver management using CS2, and CS3 for flash access. >>> >>> Signed-off-by: Brad Larson <blarson@amd.com> >>> --- >>> .../bindings/mfd/amd,pensando-elbasr.yaml | 93 +++++++++++++++++++ >>> 1 file changed, 93 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/mfd/amd,pensando-elbasr.yaml >> ... >>> + >>> +examples: >>> + - | >>> + #include <dt-bindings/reset/amd,pensando-elba-reset.h> >>> + #include <dt-bindings/interrupt-controller/arm-gic.h> >>> + >>> + spi0 { >> >> Just "spi" > > Changed to spi > >>> + #address-cells = <1>; >>> + #size-cells = <0>; >>> + num-cs = <4>; >>> + >>> + spi@0 { >> >> "spi" is for SPI controllers. Use generic name matching the device. >> Usually this is "system-controller", however Rob pointed out your >> inaccurate bindings and example. > > Proposed the below change node in the reply to Rob. The model I > followed for this was the Altera mfd/altera-a10sr.c
You pointed driver, so how is it related to bindings? Do not mix Linux implementation with the bindings.
> > spi@0 { > sr_regs@0 { > rstc: reset-controller@0 {
No underscores in node names. sr_regs is not generic name.
> > dw_i2c@1 {
Again, not a generic name. If it is i2c controller, should be i2c. If it is i2c device/client, should be something generic matching class of the device.
> > lattice_i2c@2 { > > flash@3 {
This looks ok, depending on compatible.
> > Regards, > Brad
Best regards, Krzysztof
|  |