| Message ID | 1463415447-29903-2-git-send-email-hotran@apm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, May 16, 2016 at 09:17:25AM -0700, Hoan Tran wrote: > This patch adds the APM X-Gene hwmon device tree node documentation. > > Signed-off-by: Hoan Tran <hotran@apm.com> > --- > .../devicetree/bindings/hwmon/apm-xgene-hwmon.txt | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > create mode 100644 Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt > > diff --git a/Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt b/Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt > new file mode 100644 > index 0000000..49a482e > --- /dev/null > +++ b/Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt > @@ -0,0 +1,14 @@ > +APM X-Gene hwmon driver > + > +Hwmon driver accesses sensors over the "SLIMpro" mailbox. DT bindings describe h/w, not driver data. I'm not sure this belongs in DT and perhaps the devices for the mailbox should be created by the mailbox driver. > + > +Required properties : > + - compatible : should be "apm,xgene-slimpro-hwmon" > + - mboxes : use the label reference for the mailbox as the first parameter. > + The second parameter is the channel number. When do you expect this to be different mailbox numbers? > + > +Example : > + hwmonslimpro { > + compatible = "apm,xgene-slimpro-hwmon"; > + mboxes = <&mailbox 7>; > + }; > -- > 1.9.1 >
Hi Rob, Thanks for your review ! On Mon, May 23, 2016 at 1:30 PM, Rob Herring <robh@kernel.org> wrote: > > On Mon, May 16, 2016 at 09:17:25AM -0700, Hoan Tran wrote: > > This patch adds the APM X-Gene hwmon device tree node documentation. > > > > Signed-off-by: Hoan Tran <hotran@apm.com> > > --- > > .../devicetree/bindings/hwmon/apm-xgene-hwmon.txt | 14 ++++++++++++++ > > 1 file changed, 14 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt > > > > diff --git a/Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt b/Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt > > new file mode 100644 > > index 0000000..49a482e > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt > > @@ -0,0 +1,14 @@ > > +APM X-Gene hwmon driver > > + > > +Hwmon driver accesses sensors over the "SLIMpro" mailbox. > > DT bindings describe h/w, not driver data. How about this description: "APM X-Gene SOC sensors are accessed over the "SLIMpro" mailbox" ? > I'm not sure this belongs in > DT and perhaps the devices for the mailbox should be created by the > mailbox driver. I don't think the current mailbox supports it. > > > + > > +Required properties : > > + - compatible : should be "apm,xgene-slimpro-hwmon" > > + - mboxes : use the label reference for the mailbox as the first parameter. > > + The second parameter is the channel number. > > When do you expect this to be different mailbox numbers? No, this number is not changed. This "mboxes" property is used and required by mailbox.c when hwmon driver requests a mailbox channel Thanks and Regards Hoan > > > > + > > +Example : > > + hwmonslimpro { > > + compatible = "apm,xgene-slimpro-hwmon"; > > + mboxes = <&mailbox 7>; > > + }; > > -- > > 1.9.1 > >
On Mon, May 23, 2016 at 06:01:14PM -0700, Hoan Tran wrote: > Hi Rob, > > Thanks for your review ! > > On Mon, May 23, 2016 at 1:30 PM, Rob Herring <robh@kernel.org> wrote: > > > > On Mon, May 16, 2016 at 09:17:25AM -0700, Hoan Tran wrote: > > > This patch adds the APM X-Gene hwmon device tree node documentation. > > > > > > Signed-off-by: Hoan Tran <hotran@apm.com> > > > --- > > > .../devicetree/bindings/hwmon/apm-xgene-hwmon.txt | 14 ++++++++++++++ > > > 1 file changed, 14 insertions(+) > > > create mode 100644 Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt > > > > > > diff --git a/Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt b/Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt > > > new file mode 100644 > > > index 0000000..49a482e > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt > > > @@ -0,0 +1,14 @@ > > > +APM X-Gene hwmon driver > > > + > > > +Hwmon driver accesses sensors over the "SLIMpro" mailbox. > > > > DT bindings describe h/w, not driver data. > How about this description: "APM X-Gene SOC sensors are accessed over > the "SLIMpro" mailbox" ? > > I'm not sure this belongs in > > DT and perhaps the devices for the mailbox should be created by the > > mailbox driver. > I don't think the current mailbox supports it. Then add it. The mailbox binding is really only needed when other h/w blocks use a mailbox. As this is purely a firmware interface then the mailbox driver can create any devices it needs. The devices don't have to be in DT to be created. Rob
Hi Rob, On Wed, May 25, 2016 at 10:09 AM, Rob Herring <robh@kernel.org> wrote: > > On Mon, May 23, 2016 at 06:01:14PM -0700, Hoan Tran wrote: > > Hi Rob, > > > > Thanks for your review ! > > > > On Mon, May 23, 2016 at 1:30 PM, Rob Herring <robh@kernel.org> wrote: > > > > > > On Mon, May 16, 2016 at 09:17:25AM -0700, Hoan Tran wrote: > > > > This patch adds the APM X-Gene hwmon device tree node documentation. > > > > > > > > Signed-off-by: Hoan Tran <hotran@apm.com> > > > > --- > > > > .../devicetree/bindings/hwmon/apm-xgene-hwmon.txt | 14 ++++++++++++++ > > > > 1 file changed, 14 insertions(+) > > > > create mode 100644 Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt > > > > > > > > diff --git a/Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt b/Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt > > > > new file mode 100644 > > > > index 0000000..49a482e > > > > --- /dev/null > > > > +++ b/Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt > > > > @@ -0,0 +1,14 @@ > > > > +APM X-Gene hwmon driver > > > > + > > > > +Hwmon driver accesses sensors over the "SLIMpro" mailbox. > > > > > > DT bindings describe h/w, not driver data. > > How about this description: "APM X-Gene SOC sensors are accessed over > > the "SLIMpro" mailbox" ? > > > I'm not sure this belongs in > > > DT and perhaps the devices for the mailbox should be created by the > > > mailbox driver. > > I don't think the current mailbox supports it. > > Then add it. The mailbox binding is really only needed when other h/w > blocks use a mailbox. As this is purely a firmware interface then the > mailbox driver can create any devices it needs. The devices don't have > to be in DT to be created. Yes, I'll create a function inside mailbox. It allows client request a channel by mailbox name and index without DT node. Thanks Hoan > > > Rob
On Tue, May 24, 2016 at 6:31 AM, Hoan Tran <hotran@apm.com> wrote: > Hi Rob, > > Thanks for your review ! > > On Mon, May 23, 2016 at 1:30 PM, Rob Herring <robh@kernel.org> wrote: >> >> On Mon, May 16, 2016 at 09:17:25AM -0700, Hoan Tran wrote: >> > This patch adds the APM X-Gene hwmon device tree node documentation. >> > >> > Signed-off-by: Hoan Tran <hotran@apm.com> >> > --- >> > .../devicetree/bindings/hwmon/apm-xgene-hwmon.txt | 14 ++++++++++++++ >> > 1 file changed, 14 insertions(+) >> > create mode 100644 Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt >> > >> > diff --git a/Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt b/Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt >> > new file mode 100644 >> > index 0000000..49a482e >> > --- /dev/null >> > +++ b/Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt >> > @@ -0,0 +1,14 @@ >> > +APM X-Gene hwmon driver >> > + >> > +Hwmon driver accesses sensors over the "SLIMpro" mailbox. >> >> DT bindings describe h/w, not driver data. > How about this description: "APM X-Gene SOC sensors are accessed over > the "SLIMpro" mailbox" ? >> I'm not sure this belongs in >> DT and perhaps the devices for the mailbox should be created by the >> mailbox driver. > I don't think the current mailbox supports it. >> >> > + >> > +Required properties : >> > + - compatible : should be "apm,xgene-slimpro-hwmon" >> > + - mboxes : use the label reference for the mailbox as the first parameter. >> > + The second parameter is the channel number. >> >> When do you expect this to be different mailbox numbers? > No, this number is not changed. This "mboxes" property is used and > required by mailbox.c when hwmon driver requests a mailbox channel > I think that's inaccurate. The h/w and the firmware combined is the "platform" from Linux POV. Channels are physical resources provided by a mailbox controller. Currently the firmware listens on Channel-7 but some future revision might switch to, say, Channel-9. Or say the same firmware on next revision of h/w may have to switch to Channel-3 because it has only 4 channels. So I see the mailbox channel number as a hardware property just like an IRQ (which very often change with SoC iterations). Cheers.
Hi Jassi, Thanks for your reply ! On Tue, Jun 7, 2016 at 10:20 AM, Jassi Brar <jassisinghbrar@gmail.com> wrote: > On Tue, May 24, 2016 at 6:31 AM, Hoan Tran <hotran@apm.com> wrote: >> Hi Rob, >> >> Thanks for your review ! >> >> On Mon, May 23, 2016 at 1:30 PM, Rob Herring <robh@kernel.org> wrote: >>> >>> On Mon, May 16, 2016 at 09:17:25AM -0700, Hoan Tran wrote: >>> > This patch adds the APM X-Gene hwmon device tree node documentation. >>> > >>> > Signed-off-by: Hoan Tran <hotran@apm.com> >>> > --- >>> > .../devicetree/bindings/hwmon/apm-xgene-hwmon.txt | 14 ++++++++++++++ >>> > 1 file changed, 14 insertions(+) >>> > create mode 100644 Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt >>> > >>> > diff --git a/Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt b/Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt >>> > new file mode 100644 >>> > index 0000000..49a482e >>> > --- /dev/null >>> > +++ b/Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt >>> > @@ -0,0 +1,14 @@ >>> > +APM X-Gene hwmon driver >>> > + >>> > +Hwmon driver accesses sensors over the "SLIMpro" mailbox. >>> >>> DT bindings describe h/w, not driver data. >> How about this description: "APM X-Gene SOC sensors are accessed over >> the "SLIMpro" mailbox" ? >>> I'm not sure this belongs in >>> DT and perhaps the devices for the mailbox should be created by the >>> mailbox driver. >> I don't think the current mailbox supports it. >>> >>> > + >>> > +Required properties : >>> > + - compatible : should be "apm,xgene-slimpro-hwmon" >>> > + - mboxes : use the label reference for the mailbox as the first parameter. >>> > + The second parameter is the channel number. >>> >>> When do you expect this to be different mailbox numbers? >> No, this number is not changed. This "mboxes" property is used and >> required by mailbox.c when hwmon driver requests a mailbox channel >> > I think that's inaccurate. > > The h/w and the firmware combined is the "platform" from Linux POV. > Channels are physical resources provided by a mailbox controller. > Currently the firmware listens on Channel-7 but some future revision > might switch to, say, Channel-9. Or say the same firmware on next > revision of h/w may have to switch to Channel-3 because it has only 4 > channels. So I see the mailbox channel number as a hardware property > just like an IRQ (which very often change with SoC iterations). Agree about that. I suppose this number is not changed. But as you said, the mailbox channel number can be changed based on SoC or Firmware. It would be better if this channel number is specified inside a DT node. Hi Rob, do you have any comments ? Thanks Hoan > > Cheers.
On Tue, Jun 7, 2016 at 11:05 AM, Hoan Tran <hotran@apm.com> wrote: > Hi Jassi, > > Thanks for your reply ! > > On Tue, Jun 7, 2016 at 10:20 AM, Jassi Brar <jassisinghbrar@gmail.com> wrote: >> On Tue, May 24, 2016 at 6:31 AM, Hoan Tran <hotran@apm.com> wrote: >>> Hi Rob, >>> >>> Thanks for your review ! >>> >>> On Mon, May 23, 2016 at 1:30 PM, Rob Herring <robh@kernel.org> wrote: >>>> >>>> On Mon, May 16, 2016 at 09:17:25AM -0700, Hoan Tran wrote: >>>> > This patch adds the APM X-Gene hwmon device tree node documentation. >>>> > >>>> > Signed-off-by: Hoan Tran <hotran@apm.com> >>>> > --- >>>> > .../devicetree/bindings/hwmon/apm-xgene-hwmon.txt | 14 ++++++++++++++ >>>> > 1 file changed, 14 insertions(+) >>>> > create mode 100644 Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt >>>> > >>>> > diff --git a/Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt b/Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt >>>> > new file mode 100644 >>>> > index 0000000..49a482e >>>> > --- /dev/null >>>> > +++ b/Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt >>>> > @@ -0,0 +1,14 @@ >>>> > +APM X-Gene hwmon driver >>>> > + >>>> > +Hwmon driver accesses sensors over the "SLIMpro" mailbox. >>>> >>>> DT bindings describe h/w, not driver data. >>> How about this description: "APM X-Gene SOC sensors are accessed over >>> the "SLIMpro" mailbox" ? >>>> I'm not sure this belongs in >>>> DT and perhaps the devices for the mailbox should be created by the >>>> mailbox driver. >>> I don't think the current mailbox supports it. >>>> >>>> > + >>>> > +Required properties : >>>> > + - compatible : should be "apm,xgene-slimpro-hwmon" >>>> > + - mboxes : use the label reference for the mailbox as the first parameter. >>>> > + The second parameter is the channel number. >>>> >>>> When do you expect this to be different mailbox numbers? >>> No, this number is not changed. This "mboxes" property is used and >>> required by mailbox.c when hwmon driver requests a mailbox channel >>> >> I think that's inaccurate. >> >> The h/w and the firmware combined is the "platform" from Linux POV. >> Channels are physical resources provided by a mailbox controller. >> Currently the firmware listens on Channel-7 but some future revision >> might switch to, say, Channel-9. Or say the same firmware on next >> revision of h/w may have to switch to Channel-3 because it has only 4 >> channels. So I see the mailbox channel number as a hardware property >> just like an IRQ (which very often change with SoC iterations). > > Agree about that. I suppose this number is not changed. But as you > said, the mailbox channel number can be changed based on SoC or > Firmware. It would be better if this channel number is specified > inside a DT node. > > Hi Rob, do you have any comments ? > > Thanks > Hoan > >> >> Cheers. Hi Rob, Do you have any comments on Jassi's reply ? If not, I'll send another version which included the binding document and DT node. Thanks Hoan
diff --git a/Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt b/Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt new file mode 100644 index 0000000..49a482e --- /dev/null +++ b/Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt @@ -0,0 +1,14 @@ +APM X-Gene hwmon driver + +Hwmon driver accesses sensors over the "SLIMpro" mailbox. + +Required properties : + - compatible : should be "apm,xgene-slimpro-hwmon" + - mboxes : use the label reference for the mailbox as the first parameter. + The second parameter is the channel number. + +Example : + hwmonslimpro { + compatible = "apm,xgene-slimpro-hwmon"; + mboxes = <&mailbox 7>; + };
This patch adds the APM X-Gene hwmon device tree node documentation. Signed-off-by: Hoan Tran <hotran@apm.com> --- .../devicetree/bindings/hwmon/apm-xgene-hwmon.txt | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwmon/apm-xgene-hwmon.txt