| Message ID | 1502474205-1557-2-git-send-email-eajames@linux.vnet.ibm.com |
|---|---|
| State | Accepted, archived |
| Headers | show |
On Fri, 2017-08-11 at 12:56 -0500, Eddie James wrote: > > From: "Edward A. James" <eajames@us.ibm.com> > > > Signed-off-by: Edward A. James <eajames@us.ibm.com> > --- > .../devicetree/bindings/i2c/ibm,cffps1.txt | 21 +++++++++++++++++++++ > 1 file changed, 21 insertions(+) > create mode 100644 Documentation/devicetree/bindings/i2c/ibm,cffps1.txt > > diff --git a/Documentation/devicetree/bindings/i2c/ibm,cffps1.txt b/Documentation/devicetree/bindings/i2c/ibm,cffps1.txt > new file mode 100644 > index 0000000..f68a0a6 > --- /dev/null > +++ b/Documentation/devicetree/bindings/i2c/ibm,cffps1.txt > @@ -0,0 +1,21 @@ > +Device-tree bindings for IBM Common Form Factor Power Supply Version 1 > +------------------------------------------------------------------ ---- > + > +Required properties: > + - compatible = "ibm,cffps1"; > > + - reg = < I2C bus address >; : Address of the power supply on the > > + I2C bus. Should we define properties for these registers? * OPERATION * ON_OFF_CONFIG * SYS_CONFIG Cheers, Andrew > + > +Example: > + > > + i2c-bus@100 { > + #address-cells = <1>; > + #size-cells = <0>; > + #interrupt-cells = <1>; > + < more properties > > + > > + power-supply@68 { > + compatible = "ibm,cffps1"; > + reg = <0x68>; > + }; > + };
On 08/14/2017 02:33 AM, Andrew Jeffery wrote: > On Fri, 2017-08-11 at 12:56 -0500, Eddie James wrote: >>> From: "Edward A. James" <eajames@us.ibm.com> >> >>> Signed-off-by: Edward A. James <eajames@us.ibm.com> >> --- >> .../devicetree/bindings/i2c/ibm,cffps1.txt | 21 > +++++++++++++++++++++ >> 1 file changed, 21 insertions(+) >> create mode 100644 > Documentation/devicetree/bindings/i2c/ibm,cffps1.txt >> >> diff --git a/Documentation/devicetree/bindings/i2c/ibm,cffps1.txt > b/Documentation/devicetree/bindings/i2c/ibm,cffps1.txt >> new file mode 100644 >> index 0000000..f68a0a6 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/i2c/ibm,cffps1.txt >> @@ -0,0 +1,21 @@ >> +Device-tree bindings for IBM Common Form Factor Power Supply Version > 1 >> +------------------------------------------------------------------ > ---- >> + >> +Required properties: >> + - compatible = "ibm,cffps1"; >>> + - reg = < I2C bus address >; : Address of the > power supply on the >>> + I2C bus. > Should we define properties for these registers? > > * OPERATION > * ON_OFF_CONFIG > * SYS_CONFIG We could I suppose. I don't see why though; is it normal to document specific registers on devices? And why just those ones? The driver doesn't use them. Presumably someone who wants to use them can look at the spec. Thanks for the review! Eddie > > Cheers, > > Andrew > >> + >> +Example: >> + >>> + i2c-bus@100 { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + #interrupt-cells = <1>; >> + < more properties > >> + >>> + power-supply@68 { >> + compatible = "ibm,cffps1"; >> + reg = <0x68>; >> + }; >> + };
On Mon, 2017-08-14 at 10:09 -0500, Eddie James wrote: > > On 08/14/2017 02:33 AM, Andrew Jeffery wrote: > > On Fri, 2017-08-11 at 12:56 -0500, Eddie James wrote: > > > From: "Edward A. James" <eajames@us.ibm.com> > > > > > > > > > Signed-off-by: Edward A. James <eajames@us.ibm.com> > > > > > > --- > > > .../devicetree/bindings/i2c/ibm,cffps1.txt | 21 > > > > +++++++++++++++++++++ > > > 1 file changed, 21 insertions(+) > > > create mode 100644 > > > > Documentation/devicetree/bindings/i2c/ibm,cffps1.txt > > > > > > diff --git a/Documentation/devicetree/bindings/i2c/ibm,cffps1.txt > > > > b/Documentation/devicetree/bindings/i2c/ibm,cffps1.txt > > > new file mode 100644 > > > index 0000000..f68a0a6 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/i2c/ibm,cffps1.txt > > > @@ -0,0 +1,21 @@ > > > +Device-tree bindings for IBM Common Form Factor Power Supply Version > > > > 1 > > > +------------------------------------------------------------------ > > > > ---- > > > + > > > +Required properties: > > > + - compatible = "ibm,cffps1"; > > > + - reg = < I2C bus address >; : Address of the > > > > power supply on the > > > + I2C bus. > > > > Should we define properties for these registers? > > > > * OPERATION > > * ON_OFF_CONFIG > > * SYS_CONFIG > > We could I suppose. I don't see why though; is it normal to document > specific registers on devices? It's not normal to document specific registers, no. However, it is normal to expose the device's configuration fields as properties. Reading back I chose my words poorly, but my query was about the properties of the device that can be configured through these registers, not really about the registers themselves. > And why just those ones? Because from a quick scan of the datasheet, these seem to be the remaining registers that contained fields that could be configured. > The driver doesn't use them. Presumably someone who wants to use them can > look at the spec. Upstream's stance is that the bindings describe the hardware, and the strong preferences is that the description is complete at the time of submission. Though maybe if there are optional properties it's not so significant. Cheers, Andrew > > Thanks for the review! > Eddie > > > > > Cheers, > > > > Andrew > > > > > + > > > +Example: > > > + > > > > + i2c-bus@100 { > > > > > > + #address-cells = <1>; > > > + #size-cells = <0>; > > > + #interrupt-cells = <1>; > > > + < more properties > > > > + > > > > + power-supply@68 { > > > > > > + compatible = "ibm,cffps1"; > > > + reg = <0x68>; > > > + }; > > > + }; > >
diff --git a/Documentation/devicetree/bindings/i2c/ibm,cffps1.txt b/Documentation/devicetree/bindings/i2c/ibm,cffps1.txt new file mode 100644 index 0000000..f68a0a6 --- /dev/null +++ b/Documentation/devicetree/bindings/i2c/ibm,cffps1.txt @@ -0,0 +1,21 @@ +Device-tree bindings for IBM Common Form Factor Power Supply Version 1 +---------------------------------------------------------------------- + +Required properties: + - compatible = "ibm,cffps1"; + - reg = < I2C bus address >; : Address of the power supply on the + I2C bus. + +Example: + + i2c-bus@100 { + #address-cells = <1>; + #size-cells = <0>; + #interrupt-cells = <1>; + < more properties > + + power-supply@68 { + compatible = "ibm,cffps1"; + reg = <0x68>; + }; + };