Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors

[Jon Medhurst (Tixy)]

On Tue, 2015-09-15 at 17:04 +0100, Punit Agrawal wrote:
> "Jon Medhurst (Tixy)"  writes:
> 
> > On Tue, 2015-09-15 at 10:37 +0100, Punit Agrawal wrote:
> >> "Jon Medhurst (Tixy)"  writes:
> >> 
> >> > On Mon, 2015-09-14 at 15:38 +0100, Punit Agrawal wrote:
> >> >> Mark Rutland  writes:
> >> >> 
> > [...]
> >> >> The way the SCP interface is defined, the sensor identifiers are
> >> >> contiguous,
> >> >
> >> > Is there any documentation other than DUI0922A? [1] From what I can seen
> >> > that just says it's a 16-bit value and doesn't put any particular
> >> > constraints on its value.
> >> 
> >> Although not explicitly stated, if you look at the Get Sensor Capability
> >> [2] and Get Sensor Info [3] commands you can indirectly infer that the
> >> Sensor IDs are contiguous.
> >
> > I personally wouldn't even indirectly infer they are contiguous from
> > what the document says. If I were implementing the firmware I would feel
> > quite in my rights to, for example, use the top 8 bits of the ID for a
> > sensor type and the bottom 8 for an index, if that made dispatching of
> > requests more efficient. Or if some optional hardware was detected as
> > missing, leaving some holes in ID space.
> 
> True. And without a command to convey the list of valid IDs, the
> consumer of the API would have to iterate over the entire 16bit space to
> locate valid IDs.

Or get IDs from device-tree :-) Anyway, I'm not arguing that the IDs
shouldn't be 0..N-1, just that it should explicitly documented in the
SCPI doc, which we're are in agreement on.

-- 
Tixy

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors

[Punit Agrawal]

"Jon Medhurst (Tixy)"  writes:

> On Tue, 2015-09-15 at 10:37 +0100, Punit Agrawal wrote:
>> "Jon Medhurst (Tixy)"  writes:
>> 
>> > On Mon, 2015-09-14 at 15:38 +0100, Punit Agrawal wrote:
>> >> Mark Rutland  writes:
>> >> 
> [...]
>> >> The way the SCP interface is defined, the sensor identifiers are
>> >> contiguous,
>> >
>> > Is there any documentation other than DUI0922A? [1] From what I can seen
>> > that just says it's a 16-bit value and doesn't put any particular
>> > constraints on its value.
>> 
>> Although not explicitly stated, if you look at the Get Sensor Capability
>> [2] and Get Sensor Info [3] commands you can indirectly infer that the
>> Sensor IDs are contiguous.
>
> I personally wouldn't even indirectly infer they are contiguous from
> what the document says. If I were implementing the firmware I would feel
> quite in my rights to, for example, use the top 8 bits of the ID for a
> sensor type and the bottom 8 for an index, if that made dispatching of
> requests more efficient. Or if some optional hardware was detected as
> missing, leaving some holes in ID space.

True. And without a command to convey the list of valid IDs, the
consumer of the API would have to iterate over the entire 16bit space to
locate valid IDs.

>
> As a specification of a 'standard' the document seems to be rather
> lacking. So, Sensor ID should be documented as being "an unsigned
> integer less than then number of sensors returned by the Get Sensor
> Capability command", or something like that. I guess clocks and other
> devices suffer from similar lack of specificity.

I was thinking of suggesting something similar as an update.

>
>>  Not the strongest guarantee I know.
>> 
>> All platforms currently using SCP (Juno R0 and R1) do indeed expose
>> contiguous identifiers.
>
> IMO, Linux drivers should be coded to the standard or written
> specification (where they are available) not the particular
> implementations available.
>
>> >
>> > [1] 
>> > http://community.arm.com/servlet/JiveServlet/download/8401-40-18262/DUI0922A_scp_message_interface.pdf
>> [2] 
>> http://arminfo.emea.arm.com/help/index.jsp?topic=/com.arm.doc.dui0922b/ch03s02s21.html
>> [3] 
>> http://arminfo.emea.arm.com/help/index.jsp?topic=/com.arm.doc.dui0922b/BABCCCJJ.html
>
> I think those links are on ARM's intranet, they return NXDOMAIN for me.

Apologies, the below should work.

[2] 
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0922b/ch03s02s21.html
[3] 
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0922b/BABCCCJJ.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors

[Jon Medhurst (Tixy)]

On Tue, 2015-09-15 at 12:03 +0100, Mark Rutland wrote:
> On Tue, Sep 15, 2015 at 11:46:02AM +0100, Jon Medhurst (Tixy) wrote:
> > On Tue, 2015-09-15 at 10:37 +0100, Punit Agrawal wrote:
> > > "Jon Medhurst (Tixy)"  writes:
> > > 
> > > > On Mon, 2015-09-14 at 15:38 +0100, Punit Agrawal wrote:
> > > >> Mark Rutland  writes:
> > > >> 
> > [...]
> > > >> The way the SCP interface is defined, the sensor identifiers are
> > > >> contiguous,
> > > >
> > > > Is there any documentation other than DUI0922A? [1] From what I can seen
> > > > that just says it's a 16-bit value and doesn't put any particular
> > > > constraints on its value.
> > > 
> > > Although not explicitly stated, if you look at the Get Sensor Capability
> > > [2] and Get Sensor Info [3] commands you can indirectly infer that the
> > > Sensor IDs are contiguous.
> > 
> > I personally wouldn't even indirectly infer they are contiguous from
> > what the document says. If I were implementing the firmware I would feel
> > quite in my rights to, for example, use the top 8 bits of the ID for a
> > sensor type and the bottom 8 for an index, if that made dispatching of
> > requests more efficient. Or if some optional hardware was detected as
> > missing, leaving some holes in ID space.
> > 
> > As a specification of a 'standard' the document seems to be rather
> > lacking. So, Sensor ID should be documented as being "an unsigned
> > integer less than then number of sensors returned by the Get Sensor
> > Capability command", or something like that. I guess clocks and other
> > devices suffer from similar lack of specificity.
> 
> I think from the PoV of the binding, this doesn't matter. The value is
> just an arbitrary opaue token written down in a spec, that the FW
> understands how to interpret.

True, it's the Linux implementation in following patches that has
assumptions, e.g. for loops iterating over id's 0..N-1
 
> 
> I only asked about how the IDs were organised because I thought there
> was additional translation in the kernel, but this is not the case.
> 
> The only potential problem is bit-width. Punit, I assume the value is
> 32-bit (or less) in the messages to the FW?

It's 16 bit.

-- 
Tixy

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors

[Mark Rutland]

On Tue, Sep 15, 2015 at 11:46:02AM +0100, Jon Medhurst (Tixy) wrote:
> On Tue, 2015-09-15 at 10:37 +0100, Punit Agrawal wrote:
> > "Jon Medhurst (Tixy)"  writes:
> > 
> > > On Mon, 2015-09-14 at 15:38 +0100, Punit Agrawal wrote:
> > >> Mark Rutland  writes:
> > >> 
> [...]
> > >> The way the SCP interface is defined, the sensor identifiers are
> > >> contiguous,
> > >
> > > Is there any documentation other than DUI0922A? [1] From what I can seen
> > > that just says it's a 16-bit value and doesn't put any particular
> > > constraints on its value.
> > 
> > Although not explicitly stated, if you look at the Get Sensor Capability
> > [2] and Get Sensor Info [3] commands you can indirectly infer that the
> > Sensor IDs are contiguous.
> 
> I personally wouldn't even indirectly infer they are contiguous from
> what the document says. If I were implementing the firmware I would feel
> quite in my rights to, for example, use the top 8 bits of the ID for a
> sensor type and the bottom 8 for an index, if that made dispatching of
> requests more efficient. Or if some optional hardware was detected as
> missing, leaving some holes in ID space.
> 
> As a specification of a 'standard' the document seems to be rather
> lacking. So, Sensor ID should be documented as being "an unsigned
> integer less than then number of sensors returned by the Get Sensor
> Capability command", or something like that. I guess clocks and other
> devices suffer from similar lack of specificity.

I think from the PoV of the binding, this doesn't matter. The value is
just an arbitrary opaue token written down in a spec, that the FW
understands how to interpret.

I only asked about how the IDs were organised because I thought there
was additional translation in the kernel, but this is not the case.

The only potential problem is bit-width. Punit, I assume the value is
32-bit (or less) in the messages to the FW?

If for the moment we assume the values can index a small array, then we
can change that later (though ideally we'd handle the most geeneral case
we're aware of from the outset). We just need to not infer any semantics
from a value alone.

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors

[Jon Medhurst (Tixy)]

On Tue, 2015-09-15 at 10:37 +0100, Punit Agrawal wrote:
> "Jon Medhurst (Tixy)"  writes:
> 
> > On Mon, 2015-09-14 at 15:38 +0100, Punit Agrawal wrote:
> >> Mark Rutland  writes:
> >> 
[...]
> >> The way the SCP interface is defined, the sensor identifiers are
> >> contiguous,
> >
> > Is there any documentation other than DUI0922A? [1] From what I can seen
> > that just says it's a 16-bit value and doesn't put any particular
> > constraints on its value.
> 
> Although not explicitly stated, if you look at the Get Sensor Capability
> [2] and Get Sensor Info [3] commands you can indirectly infer that the
> Sensor IDs are contiguous.

I personally wouldn't even indirectly infer they are contiguous from
what the document says. If I were implementing the firmware I would feel
quite in my rights to, for example, use the top 8 bits of the ID for a
sensor type and the bottom 8 for an index, if that made dispatching of
requests more efficient. Or if some optional hardware was detected as
missing, leaving some holes in ID space.

As a specification of a 'standard' the document seems to be rather
lacking. So, Sensor ID should be documented as being "an unsigned
integer less than then number of sensors returned by the Get Sensor
Capability command", or something like that. I guess clocks and other
devices suffer from similar lack of specificity.

>  Not the strongest guarantee I know.
> 
> All platforms currently using SCP (Juno R0 and R1) do indeed expose
> contiguous identifiers.

IMO, Linux drivers should be coded to the standard or written
specification (where they are available) not the particular
implementations available.

> >
> > [1] 
> > http://community.arm.com/servlet/JiveServlet/download/8401-40-18262/DUI0922A_scp_message_interface.pdf
> [2] 
> http://arminfo.emea.arm.com/help/index.jsp?topic=/com.arm.doc.dui0922b/ch03s02s21.html
> [3] 
> http://arminfo.emea.arm.com/help/index.jsp?topic=/com.arm.doc.dui0922b/BABCCCJJ.html

I think those links are on ARM's intranet, they return NXDOMAIN for me.

-- 
Tixy

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors

[Punit Agrawal]

"Jon Medhurst (Tixy)"  writes:

> On Mon, 2015-09-14 at 15:38 +0100, Punit Agrawal wrote:
>> Mark Rutland  writes:
>> 
>> >> >> +Sensor bindings for the sensors based on SCPI Message Protocol
>> >> >> +--
>> >> >> +SCPI provides an API to access the various sensors on the SoC.
>> >> >> +
>> >> >> +Required properties:
>> >> >> +- compatible : should be "arm,scpi-sensors".
>> >> >> +- #thermal-sensor-cells: should be set to 1. This property follows the
>> >> >> +   thermal device tree bindings[2].
>> >> >
>> >> > You need to specify what the valid values for this cell are.
>> >> 
>> >> The enumeration depends on the number of sensors exported by SCP
>> >> firmware - which is platform dependent. I could add add something like
>> >> if you think that is helpful -
>> >> 
>> >> "Valid cell value is a number between 0..n-1, where n is the number
>> >> of sensors exported by SCP firmware."
>> >
>> > Can the FW identifer space have holes? Or are they always contiguous?
>> 
>> The way the SCP interface is defined, the sensor identifiers are
>> contiguous,
>
> Is there any documentation other than DUI0922A? [1] From what I can seen
> that just says it's a 16-bit value and doesn't put any particular
> constraints on its value.

Although not explicitly stated, if you look at the Get Sensor Capability
[2] and Get Sensor Info [3] commands you can indirectly infer that the
Sensor IDs are contiguous. Not the strongest guarantee I know.

All platforms currently using SCP (Juno R0 and R1) do indeed expose
contiguous identifiers. Maybe we can convince the authors to make it
explicit.

>
> [1] 
> http://community.arm.com/servlet/JiveServlet/download/8401-40-18262/DUI0922A_scp_message_interface.pdf
[2] 
http://arminfo.emea.arm.com/help/index.jsp?topic=/com.arm.doc.dui0922b/ch03s02s21.html
[3] 
http://arminfo.emea.arm.com/help/index.jsp?topic=/com.arm.doc.dui0922b/BABCCCJJ.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors

[Punit Agrawal]

"Jon Medhurst (Tixy)"  writes:

> On Mon, 2015-09-14 at 15:38 +0100, Punit Agrawal wrote:
>> Mark Rutland  writes:
>> 
>> >> >> +Sensor bindings for the sensors based on SCPI Message Protocol
>> >> >> +--
>> >> >> +SCPI provides an API to access the various sensors on the SoC.
>> >> >> +
>> >> >> +Required properties:
>> >> >> +- compatible : should be "arm,scpi-sensors".
>> >> >> +- #thermal-sensor-cells: should be set to 1. This property follows the
>> >> >> +   thermal device tree bindings[2].
>> >> >
>> >> > You need to specify what the valid values for this cell are.
>> >> 
>> >> The enumeration depends on the number of sensors exported by SCP
>> >> firmware - which is platform dependent. I could add add something like
>> >> if you think that is helpful -
>> >> 
>> >> "Valid cell value is a number between 0..n-1, where n is the number
>> >> of sensors exported by SCP firmware."
>> >
>> > Can the FW identifer space have holes? Or are they always contiguous?
>> 
>> The way the SCP interface is defined, the sensor identifiers are
>> contiguous,
>
> Is there any documentation other than DUI0922A? [1] From what I can seen
> that just says it's a 16-bit value and doesn't put any particular
> constraints on its value.

Although not explicitly stated, if you look at the Get Sensor Capability
[2] and Get Sensor Info [3] commands you can indirectly infer that the
Sensor IDs are contiguous. Not the strongest guarantee I know.

All platforms currently using SCP (Juno R0 and R1) do indeed expose
contiguous identifiers. Maybe we can convince the authors to make it
explicit.

>
> [1] 
> http://community.arm.com/servlet/JiveServlet/download/8401-40-18262/DUI0922A_scp_message_interface.pdf
[2] 
http://arminfo.emea.arm.com/help/index.jsp?topic=/com.arm.doc.dui0922b/ch03s02s21.html
[3] 
http://arminfo.emea.arm.com/help/index.jsp?topic=/com.arm.doc.dui0922b/BABCCCJJ.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors

[Punit Agrawal]

"Jon Medhurst (Tixy)"  writes:

> On Tue, 2015-09-15 at 10:37 +0100, Punit Agrawal wrote:
>> "Jon Medhurst (Tixy)"  writes:
>> 
>> > On Mon, 2015-09-14 at 15:38 +0100, Punit Agrawal wrote:
>> >> Mark Rutland  writes:
>> >> 
> [...]
>> >> The way the SCP interface is defined, the sensor identifiers are
>> >> contiguous,
>> >
>> > Is there any documentation other than DUI0922A? [1] From what I can seen
>> > that just says it's a 16-bit value and doesn't put any particular
>> > constraints on its value.
>> 
>> Although not explicitly stated, if you look at the Get Sensor Capability
>> [2] and Get Sensor Info [3] commands you can indirectly infer that the
>> Sensor IDs are contiguous.
>
> I personally wouldn't even indirectly infer they are contiguous from
> what the document says. If I were implementing the firmware I would feel
> quite in my rights to, for example, use the top 8 bits of the ID for a
> sensor type and the bottom 8 for an index, if that made dispatching of
> requests more efficient. Or if some optional hardware was detected as
> missing, leaving some holes in ID space.

True. And without a command to convey the list of valid IDs, the
consumer of the API would have to iterate over the entire 16bit space to
locate valid IDs.

>
> As a specification of a 'standard' the document seems to be rather
> lacking. So, Sensor ID should be documented as being "an unsigned
> integer less than then number of sensors returned by the Get Sensor
> Capability command", or something like that. I guess clocks and other
> devices suffer from similar lack of specificity.

I was thinking of suggesting something similar as an update.

>
>>  Not the strongest guarantee I know.
>> 
>> All platforms currently using SCP (Juno R0 and R1) do indeed expose
>> contiguous identifiers.
>
> IMO, Linux drivers should be coded to the standard or written
> specification (where they are available) not the particular
> implementations available.
>
>> >
>> > [1] 
>> > http://community.arm.com/servlet/JiveServlet/download/8401-40-18262/DUI0922A_scp_message_interface.pdf
>> [2] 
>> http://arminfo.emea.arm.com/help/index.jsp?topic=/com.arm.doc.dui0922b/ch03s02s21.html
>> [3] 
>> http://arminfo.emea.arm.com/help/index.jsp?topic=/com.arm.doc.dui0922b/BABCCCJJ.html
>
> I think those links are on ARM's intranet, they return NXDOMAIN for me.

Apologies, the below should work.

[2] 
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0922b/ch03s02s21.html
[3] 
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0922b/BABCCCJJ.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors

[Jon Medhurst (Tixy)]

On Tue, 2015-09-15 at 17:04 +0100, Punit Agrawal wrote:
> "Jon Medhurst (Tixy)"  writes:
> 
> > On Tue, 2015-09-15 at 10:37 +0100, Punit Agrawal wrote:
> >> "Jon Medhurst (Tixy)"  writes:
> >> 
> >> > On Mon, 2015-09-14 at 15:38 +0100, Punit Agrawal wrote:
> >> >> Mark Rutland  writes:
> >> >> 
> > [...]
> >> >> The way the SCP interface is defined, the sensor identifiers are
> >> >> contiguous,
> >> >
> >> > Is there any documentation other than DUI0922A? [1] From what I can seen
> >> > that just says it's a 16-bit value and doesn't put any particular
> >> > constraints on its value.
> >> 
> >> Although not explicitly stated, if you look at the Get Sensor Capability
> >> [2] and Get Sensor Info [3] commands you can indirectly infer that the
> >> Sensor IDs are contiguous.
> >
> > I personally wouldn't even indirectly infer they are contiguous from
> > what the document says. If I were implementing the firmware I would feel
> > quite in my rights to, for example, use the top 8 bits of the ID for a
> > sensor type and the bottom 8 for an index, if that made dispatching of
> > requests more efficient. Or if some optional hardware was detected as
> > missing, leaving some holes in ID space.
> 
> True. And without a command to convey the list of valid IDs, the
> consumer of the API would have to iterate over the entire 16bit space to
> locate valid IDs.

Or get IDs from device-tree :-) Anyway, I'm not arguing that the IDs
shouldn't be 0..N-1, just that it should explicitly documented in the
SCPI doc, which we're are in agreement on.

-- 
Tixy

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors

[Jon Medhurst (Tixy)]

On Tue, 2015-09-15 at 10:37 +0100, Punit Agrawal wrote:
> "Jon Medhurst (Tixy)"  writes:
> 
> > On Mon, 2015-09-14 at 15:38 +0100, Punit Agrawal wrote:
> >> Mark Rutland  writes:
> >> 
[...]
> >> The way the SCP interface is defined, the sensor identifiers are
> >> contiguous,
> >
> > Is there any documentation other than DUI0922A? [1] From what I can seen
> > that just says it's a 16-bit value and doesn't put any particular
> > constraints on its value.
> 
> Although not explicitly stated, if you look at the Get Sensor Capability
> [2] and Get Sensor Info [3] commands you can indirectly infer that the
> Sensor IDs are contiguous.

I personally wouldn't even indirectly infer they are contiguous from
what the document says. If I were implementing the firmware I would feel
quite in my rights to, for example, use the top 8 bits of the ID for a
sensor type and the bottom 8 for an index, if that made dispatching of
requests more efficient. Or if some optional hardware was detected as
missing, leaving some holes in ID space.

As a specification of a 'standard' the document seems to be rather
lacking. So, Sensor ID should be documented as being "an unsigned
integer less than then number of sensors returned by the Get Sensor
Capability command", or something like that. I guess clocks and other
devices suffer from similar lack of specificity.

>  Not the strongest guarantee I know.
> 
> All platforms currently using SCP (Juno R0 and R1) do indeed expose
> contiguous identifiers.

IMO, Linux drivers should be coded to the standard or written
specification (where they are available) not the particular
implementations available.

> >
> > [1] 
> > http://community.arm.com/servlet/JiveServlet/download/8401-40-18262/DUI0922A_scp_message_interface.pdf
> [2] 
> http://arminfo.emea.arm.com/help/index.jsp?topic=/com.arm.doc.dui0922b/ch03s02s21.html
> [3] 
> http://arminfo.emea.arm.com/help/index.jsp?topic=/com.arm.doc.dui0922b/BABCCCJJ.html

I think those links are on ARM's intranet, they return NXDOMAIN for me.

-- 
Tixy

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors

[Mark Rutland]

On Tue, Sep 15, 2015 at 11:46:02AM +0100, Jon Medhurst (Tixy) wrote:
> On Tue, 2015-09-15 at 10:37 +0100, Punit Agrawal wrote:
> > "Jon Medhurst (Tixy)"  writes:
> > 
> > > On Mon, 2015-09-14 at 15:38 +0100, Punit Agrawal wrote:
> > >> Mark Rutland  writes:
> > >> 
> [...]
> > >> The way the SCP interface is defined, the sensor identifiers are
> > >> contiguous,
> > >
> > > Is there any documentation other than DUI0922A? [1] From what I can seen
> > > that just says it's a 16-bit value and doesn't put any particular
> > > constraints on its value.
> > 
> > Although not explicitly stated, if you look at the Get Sensor Capability
> > [2] and Get Sensor Info [3] commands you can indirectly infer that the
> > Sensor IDs are contiguous.
> 
> I personally wouldn't even indirectly infer they are contiguous from
> what the document says. If I were implementing the firmware I would feel
> quite in my rights to, for example, use the top 8 bits of the ID for a
> sensor type and the bottom 8 for an index, if that made dispatching of
> requests more efficient. Or if some optional hardware was detected as
> missing, leaving some holes in ID space.
> 
> As a specification of a 'standard' the document seems to be rather
> lacking. So, Sensor ID should be documented as being "an unsigned
> integer less than then number of sensors returned by the Get Sensor
> Capability command", or something like that. I guess clocks and other
> devices suffer from similar lack of specificity.

I think from the PoV of the binding, this doesn't matter. The value is
just an arbitrary opaue token written down in a spec, that the FW
understands how to interpret.

I only asked about how the IDs were organised because I thought there
was additional translation in the kernel, but this is not the case.

The only potential problem is bit-width. Punit, I assume the value is
32-bit (or less) in the messages to the FW?

If for the moment we assume the values can index a small array, then we
can change that later (though ideally we'd handle the most geeneral case
we're aware of from the outset). We just need to not infer any semantics
from a value alone.

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors

[Jon Medhurst (Tixy)]

On Tue, 2015-09-15 at 12:03 +0100, Mark Rutland wrote:
> On Tue, Sep 15, 2015 at 11:46:02AM +0100, Jon Medhurst (Tixy) wrote:
> > On Tue, 2015-09-15 at 10:37 +0100, Punit Agrawal wrote:
> > > "Jon Medhurst (Tixy)"  writes:
> > > 
> > > > On Mon, 2015-09-14 at 15:38 +0100, Punit Agrawal wrote:
> > > >> Mark Rutland  writes:
> > > >> 
> > [...]
> > > >> The way the SCP interface is defined, the sensor identifiers are
> > > >> contiguous,
> > > >
> > > > Is there any documentation other than DUI0922A? [1] From what I can seen
> > > > that just says it's a 16-bit value and doesn't put any particular
> > > > constraints on its value.
> > > 
> > > Although not explicitly stated, if you look at the Get Sensor Capability
> > > [2] and Get Sensor Info [3] commands you can indirectly infer that the
> > > Sensor IDs are contiguous.
> > 
> > I personally wouldn't even indirectly infer they are contiguous from
> > what the document says. If I were implementing the firmware I would feel
> > quite in my rights to, for example, use the top 8 bits of the ID for a
> > sensor type and the bottom 8 for an index, if that made dispatching of
> > requests more efficient. Or if some optional hardware was detected as
> > missing, leaving some holes in ID space.
> > 
> > As a specification of a 'standard' the document seems to be rather
> > lacking. So, Sensor ID should be documented as being "an unsigned
> > integer less than then number of sensors returned by the Get Sensor
> > Capability command", or something like that. I guess clocks and other
> > devices suffer from similar lack of specificity.
> 
> I think from the PoV of the binding, this doesn't matter. The value is
> just an arbitrary opaue token written down in a spec, that the FW
> understands how to interpret.

True, it's the Linux implementation in following patches that has
assumptions, e.g. for loops iterating over id's 0..N-1
 
> 
> I only asked about how the IDs were organised because I thought there
> was additional translation in the kernel, but this is not the case.
> 
> The only potential problem is bit-width. Punit, I assume the value is
> 32-bit (or less) in the messages to the FW?

It's 16 bit.

-- 
Tixy

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors

[Jon Medhurst (Tixy)]

On Mon, 2015-09-14 at 15:38 +0100, Punit Agrawal wrote:
> Mark Rutland  writes:
> 
> >> >> +Sensor bindings for the sensors based on SCPI Message Protocol
> >> >> +--
> >> >> +SCPI provides an API to access the various sensors on the SoC.
> >> >> +
> >> >> +Required properties:
> >> >> +- compatible : should be "arm,scpi-sensors".
> >> >> +- #thermal-sensor-cells: should be set to 1. This property follows the
> >> >> +thermal device tree bindings[2].
> >> >
> >> > You need to specify what the valid values for this cell are.
> >> 
> >> The enumeration depends on the number of sensors exported by SCP
> >> firmware - which is platform dependent. I could add add something like
> >> if you think that is helpful -
> >> 
> >> "Valid cell value is a number between 0..n-1, where n is the number
> >> of sensors exported by SCP firmware."
> >
> > Can the FW identifer space have holes? Or are they always contiguous?
> 
> The way the SCP interface is defined, the sensor identifiers are
> contiguous,

Is there any documentation other than DUI0922A? [1] From what I can seen
that just says it's a 16-bit value and doesn't put any particular
constraints on its value.

[1] 
http://community.arm.com/servlet/JiveServlet/download/8401-40-18262/DUI0922A_scp_message_interface.pdf

-- 
Tixy

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors

[Punit Agrawal]

Mark Rutland  writes:

> On Mon, Sep 14, 2015 at 04:01:09PM +0100, Punit Agrawal wrote:
>> Mark Rutland  writes:
>> 

[...]

>> 
>> After seeing the mapping already published, I am wondering if there is
>> any value in duplicating the information. If there are no objections,
>> I'll update this patch to add pointers instead.
>
> That's fine by me. The important part is that the value is a raw Sensor
> ID as the FW uses. So long as we state that, the IDs themselves can come
> from whatever documentation is valid for a particular instance.
>
> Thanks for the info!

Sure. I've updated the patch to explain what the valid cell values are
along with a pointer to platform documentation for Juno R0 and R1.

I've also added an example to show usage of the sensors to create a
thermal zone.

I'll post a new version shortly.

Thanks for your having a look.

>
> Thanks,
> Mark.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors

[Mark Rutland]

On Mon, Sep 14, 2015 at 04:01:09PM +0100, Punit Agrawal wrote:
> Mark Rutland  writes:
> 
> > On Mon, Sep 14, 2015 at 03:38:36PM +0100, Punit Agrawal wrote:
> >> Mark Rutland  writes:
> >> 
> >> >> >> +Sensor bindings for the sensors based on SCPI Message Protocol
> >> >> >> +--
> >> >> >> +SCPI provides an API to access the various sensors on the SoC.
> >> >> >> +
> >> >> >> +Required properties:
> >> >> >> +- compatible : should be "arm,scpi-sensors".
> >> >> >> +- #thermal-sensor-cells: should be set to 1. This property follows 
> >> >> >> the
> >> >> >> + thermal device tree bindings[2].
> >> >> >
> >> >> > You need to specify what the valid values for this cell are.
> >> >> 
> >> >> The enumeration depends on the number of sensors exported by SCP
> >> >> firmware - which is platform dependent. I could add add something like
> >> >> if you think that is helpful -
> >> >> 
> >> >> "Valid cell value is a number between 0..n-1, where n is the number
> >> >> of sensors exported by SCP firmware."
> >> >
> >> > Can the FW identifer space have holes? Or are they always contiguous?
> >> 
> >> The way the SCP interface is defined, the sensor identifiers are
> >> contiguous, but not all are temperature sensors.
> >
> > Ok. So how exactly are they enumerated for this binding?
> 
> The sensor enumeration for r0 (which I've verified) and r1 can be found
> at [0]. The valid cell values are identifiers for the temperature sensors.

Ok. That's fine by me; I was confused and thought that there was some
internal mapping.

> >> > If this is the same as the raw FW identifer value, specify that.
> >> > Otherwise, you need to specify the mapping.
> >> 
> >> I'll update the patch to add mappings for Juno r0 (and r1 if I can get
> >> my hands on one).
> >
> > If there's identical logic mapping the two, we might just be able to
> > describe that rather than having to add tables all the time.
> >
> 
> After seeing the mapping already published, I am wondering if there is
> any value in duplicating the information. If there are no objections,
> I'll update this patch to add pointers instead.

That's fine by me. The important part is that the value is a raw Sensor
ID as the FW uses. So long as we state that, the IDs themselves can come
from whatever documentation is valid for a particular instance.

Thanks for the info!

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors

[Punit Agrawal]

Mark Rutland  writes:

> On Mon, Sep 14, 2015 at 03:38:36PM +0100, Punit Agrawal wrote:
>> Mark Rutland  writes:
>> 
>> >> >> +Sensor bindings for the sensors based on SCPI Message Protocol
>> >> >> +--
>> >> >> +SCPI provides an API to access the various sensors on the SoC.
>> >> >> +
>> >> >> +Required properties:
>> >> >> +- compatible : should be "arm,scpi-sensors".
>> >> >> +- #thermal-sensor-cells: should be set to 1. This property follows the
>> >> >> +   thermal device tree bindings[2].
>> >> >
>> >> > You need to specify what the valid values for this cell are.
>> >> 
>> >> The enumeration depends on the number of sensors exported by SCP
>> >> firmware - which is platform dependent. I could add add something like
>> >> if you think that is helpful -
>> >> 
>> >> "Valid cell value is a number between 0..n-1, where n is the number
>> >> of sensors exported by SCP firmware."
>> >
>> > Can the FW identifer space have holes? Or are they always contiguous?
>> 
>> The way the SCP interface is defined, the sensor identifiers are
>> contiguous, but not all are temperature sensors.
>
> Ok. So how exactly are they enumerated for this binding?

The sensor enumeration for r0 (which I've verified) and r1 can be found
at [0]. The valid cell values are identifiers for the temperature sensors.

>
>> > If this is the same as the raw FW identifer value, specify that.
>> > Otherwise, you need to specify the mapping.
>> 
>> I'll update the patch to add mappings for Juno r0 (and r1 if I can get
>> my hands on one).
>
> If there's identical logic mapping the two, we might just be able to
> describe that rather than having to add tables all the time.
>

After seeing the mapping already published, I am wondering if there is
any value in duplicating the information. If there are no objections,
I'll update this patch to add pointers instead.

>> > There needs to be enough information for a dts author to figure out
>> > which values to place in the DT.
>> 
>> I understand. Except sometimes it is hard to get the firmware to commit to 
>> not
>> modify the ordering - discoverability and all that. :)
>
> If they do that, then things are broken regardless, no? I guess that'll
> be clear if/when I see how the mapping works.

[0] 
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0922b/apas03s22.html

>
> Thanks,
> Mark.
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors

[Mark Rutland]

On Mon, Sep 14, 2015 at 03:38:36PM +0100, Punit Agrawal wrote:
> Mark Rutland  writes:
> 
> >> >> +Sensor bindings for the sensors based on SCPI Message Protocol
> >> >> +--
> >> >> +SCPI provides an API to access the various sensors on the SoC.
> >> >> +
> >> >> +Required properties:
> >> >> +- compatible : should be "arm,scpi-sensors".
> >> >> +- #thermal-sensor-cells: should be set to 1. This property follows the
> >> >> +thermal device tree bindings[2].
> >> >
> >> > You need to specify what the valid values for this cell are.
> >> 
> >> The enumeration depends on the number of sensors exported by SCP
> >> firmware - which is platform dependent. I could add add something like
> >> if you think that is helpful -
> >> 
> >> "Valid cell value is a number between 0..n-1, where n is the number
> >> of sensors exported by SCP firmware."
> >
> > Can the FW identifer space have holes? Or are they always contiguous?
> 
> The way the SCP interface is defined, the sensor identifiers are
> contiguous, but not all are temperature sensors.

Ok. So how exactly are they enumerated for this binding?

> > If this is the same as the raw FW identifer value, specify that.
> > Otherwise, you need to specify the mapping.
> 
> I'll update the patch to add mappings for Juno r0 (and r1 if I can get
> my hands on one).

If there's identical logic mapping the two, we might just be able to
describe that rather than having to add tables all the time.

> > There needs to be enough information for a dts author to figure out
> > which values to place in the DT.
> 
> I understand. Except sometimes it is hard to get the firmware to commit to not
> modify the ordering - discoverability and all that. :)

If they do that, then things are broken regardless, no? I guess that'll
be clear if/when I see how the mapping works.

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors

[Punit Agrawal]

Mark Rutland  writes:

>> >> +Sensor bindings for the sensors based on SCPI Message Protocol
>> >> +--
>> >> +SCPI provides an API to access the various sensors on the SoC.
>> >> +
>> >> +Required properties:
>> >> +- compatible : should be "arm,scpi-sensors".
>> >> +- #thermal-sensor-cells: should be set to 1. This property follows the
>> >> +  thermal device tree bindings[2].
>> >
>> > You need to specify what the valid values for this cell are.
>> 
>> The enumeration depends on the number of sensors exported by SCP
>> firmware - which is platform dependent. I could add add something like
>> if you think that is helpful -
>> 
>> "Valid cell value is a number between 0..n-1, where n is the number
>> of sensors exported by SCP firmware."
>
> Can the FW identifer space have holes? Or are they always contiguous?

The way the SCP interface is defined, the sensor identifiers are
contiguous, but not all are temperature sensors.

>
> If this is the same as the raw FW identifer value, specify that.
> Otherwise, you need to specify the mapping.

I'll update the patch to add mappings for Juno r0 (and r1 if I can get
my hands on one).

>
> There needs to be enough information for a dts author to figure out
> which values to place in the DT.

I understand. Except sometimes it is hard to get the firmware to commit to not
modify the ordering - discoverability and all that. :)

>
> Thanks,
> Mark.
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors

[Mark Rutland]

> >> +Sensor bindings for the sensors based on SCPI Message Protocol
> >> +--
> >> +SCPI provides an API to access the various sensors on the SoC.
> >> +
> >> +Required properties:
> >> +- compatible : should be "arm,scpi-sensors".
> >> +- #thermal-sensor-cells: should be set to 1. This property follows the
> >> +   thermal device tree bindings[2].
> >
> > You need to specify what the valid values for this cell are.
> 
> The enumeration depends on the number of sensors exported by SCP
> firmware - which is platform dependent. I could add add something like
> if you think that is helpful -
> 
> "Valid cell value is a number between 0..n-1, where n is the number
> of sensors exported by SCP firmware."

Can the FW identifer space have holes? Or are they always contiguous?

If this is the same as the raw FW identifer value, specify that.
Otherwise, you need to specify the mapping.

There needs to be enough information for a dts author to figure out
which values to place in the DT.

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors

[Punit Agrawal]

Hi Mark,

Thanks for taking a look.

Mark Rutland  writes:

> On Mon, Sep 14, 2015 at 02:00:14PM +0100, Punit Agrawal wrote:
>> The System Control Processor (SCP) provides access to SoC sensors via
>> the System Control and Power Interface (SCPI) Message Protocol. Add
>> bindings to allow probing of these sensors. Also support referencing
>> of the sensors for setting up thermal zones via the thermal DT
>> bindings.
>> 
>> Signed-off-by: Punit Agrawal 
>> Cc: Rob Herring 
>> Cc: Mark Rutland 
>> Cc: Sudeep Holla 
>> ---
>>  Documentation/devicetree/bindings/arm/arm,scpi.txt | 15 +++
>>  1 file changed, 15 insertions(+)
>> 
>> diff --git a/Documentation/devicetree/bindings/arm/arm,scpi.txt 
>> b/Documentation/devicetree/bindings/arm/arm,scpi.txt
>> index f002460..af66f7f 100644
>> --- a/Documentation/devicetree/bindings/arm/arm,scpi.txt
>> +++ b/Documentation/devicetree/bindings/arm/arm,scpi.txt
>> @@ -72,8 +72,18 @@ Required sub-node properties:
>>  - compatible : should be "arm,juno-scp-shmem" for Non-secure SRAM based
>> shared memory on Juno platforms
>>  
>> +Sensor bindings for the sensors based on SCPI Message Protocol
>> +--
>> +SCPI provides an API to access the various sensors on the SoC.
>> +
>> +Required properties:
>> +- compatible : should be "arm,scpi-sensors".
>> +- #thermal-sensor-cells: should be set to 1. This property follows the
>> + thermal device tree bindings[2].
>
> You need to specify what the valid values for this cell are.

The enumeration depends on the number of sensors exported by SCP
firmware - which is platform dependent. I could add add something like
if you think that is helpful -

"Valid cell value is a number between 0..n-1, where n is the number
of sensors exported by SCP firmware."

Thanks,

Punit

>
> Mark.
>
>> +
>>  [0] http://infocenter.arm.com/help/topic/com.arm.doc.dui0922b/index.html
>>  [1] Documentation/devicetree/bindings/clock/clock-bindings.txt
>> +[2] Documentation/devicetree/bindings/thermal/thermal.txt
>>  
>>  Example:
>>  
>> @@ -122,6 +132,11 @@ scpi_protocol: scpi@2e00 {
>>  clock-output-names = "pxlclk0", "pxlclk1";
>>  };
>>  };
>> +
>> +scpi_sensors0: sensors {
>> +compatible = "arm,scpi-sensors";
>> +#thermal-sensor-cells = <1>;
>> +};
>>  };
>>  
>>  cpu@0 {
>> -- 
>> 2.5.0
>> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors

[Mark Rutland]

On Mon, Sep 14, 2015 at 02:00:14PM +0100, Punit Agrawal wrote:
> The System Control Processor (SCP) provides access to SoC sensors via
> the System Control and Power Interface (SCPI) Message Protocol. Add
> bindings to allow probing of these sensors. Also support referencing
> of the sensors for setting up thermal zones via the thermal DT
> bindings.
> 
> Signed-off-by: Punit Agrawal 
> Cc: Rob Herring 
> Cc: Mark Rutland 
> Cc: Sudeep Holla 
> ---
>  Documentation/devicetree/bindings/arm/arm,scpi.txt | 15 +++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/arm,scpi.txt 
> b/Documentation/devicetree/bindings/arm/arm,scpi.txt
> index f002460..af66f7f 100644
> --- a/Documentation/devicetree/bindings/arm/arm,scpi.txt
> +++ b/Documentation/devicetree/bindings/arm/arm,scpi.txt
> @@ -72,8 +72,18 @@ Required sub-node properties:
>  - compatible : should be "arm,juno-scp-shmem" for Non-secure SRAM based
>  shared memory on Juno platforms
>  
> +Sensor bindings for the sensors based on SCPI Message Protocol
> +--
> +SCPI provides an API to access the various sensors on the SoC.
> +
> +Required properties:
> +- compatible : should be "arm,scpi-sensors".
> +- #thermal-sensor-cells: should be set to 1. This property follows the
> +  thermal device tree bindings[2].

You need to specify what the valid values for this cell are.

Mark.

> +
>  [0] http://infocenter.arm.com/help/topic/com.arm.doc.dui0922b/index.html
>  [1] Documentation/devicetree/bindings/clock/clock-bindings.txt
> +[2] Documentation/devicetree/bindings/thermal/thermal.txt
>  
>  Example:
>  
> @@ -122,6 +132,11 @@ scpi_protocol: scpi@2e00 {
>   clock-output-names = "pxlclk0", "pxlclk1";
>   };
>   };
> +
> + scpi_sensors0: sensors {
> + compatible = "arm,scpi-sensors";
> + #thermal-sensor-cells = <1>;
> + };
>  };
>  
>  cpu@0 {
> -- 
> 2.5.0
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors

[Punit Agrawal]

The System Control Processor (SCP) provides access to SoC sensors via
the System Control and Power Interface (SCPI) Message Protocol. Add
bindings to allow probing of these sensors. Also support referencing
of the sensors for setting up thermal zones via the thermal DT
bindings.

Signed-off-by: Punit Agrawal 
Cc: Rob Herring 
Cc: Mark Rutland 
Cc: Sudeep Holla 
---
 Documentation/devicetree/bindings/arm/arm,scpi.txt | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/arm,scpi.txt 
b/Documentation/devicetree/bindings/arm/arm,scpi.txt
index f002460..af66f7f 100644
--- a/Documentation/devicetree/bindings/arm/arm,scpi.txt
+++ b/Documentation/devicetree/bindings/arm/arm,scpi.txt
@@ -72,8 +72,18 @@ Required sub-node properties:
 - compatible : should be "arm,juno-scp-shmem" for Non-secure SRAM based
   shared memory on Juno platforms
 
+Sensor bindings for the sensors based on SCPI Message Protocol
+--
+SCPI provides an API to access the various sensors on the SoC.
+
+Required properties:
+- compatible : should be "arm,scpi-sensors".
+- #thermal-sensor-cells: should be set to 1. This property follows the
+thermal device tree bindings[2].
+
 [0] http://infocenter.arm.com/help/topic/com.arm.doc.dui0922b/index.html
 [1] Documentation/devicetree/bindings/clock/clock-bindings.txt
+[2] Documentation/devicetree/bindings/thermal/thermal.txt
 
 Example:
 
@@ -122,6 +132,11 @@ scpi_protocol: scpi@2e00 {
clock-output-names = "pxlclk0", "pxlclk1";
};
};
+
+   scpi_sensors0: sensors {
+   compatible = "arm,scpi-sensors";
+   #thermal-sensor-cells = <1>;
+   };
 };
 
 cpu@0 {
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors

[Punit Agrawal]

The System Control Processor (SCP) provides access to SoC sensors via
the System Control and Power Interface (SCPI) Message Protocol. Add
bindings to allow probing of these sensors. Also support referencing
of the sensors for setting up thermal zones via the thermal DT
bindings.

Signed-off-by: Punit Agrawal 
Cc: Rob Herring 
Cc: Mark Rutland 
Cc: Sudeep Holla 
---
 Documentation/devicetree/bindings/arm/arm,scpi.txt | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/arm,scpi.txt 
b/Documentation/devicetree/bindings/arm/arm,scpi.txt
index f002460..af66f7f 100644
--- a/Documentation/devicetree/bindings/arm/arm,scpi.txt
+++ b/Documentation/devicetree/bindings/arm/arm,scpi.txt
@@ -72,8 +72,18 @@ Required sub-node properties:
 - compatible : should be "arm,juno-scp-shmem" for Non-secure SRAM based
   shared memory on Juno platforms
 
+Sensor bindings for the sensors based on SCPI Message Protocol
+--
+SCPI provides an API to access the various sensors on the SoC.
+
+Required properties:
+- compatible : should be "arm,scpi-sensors".
+- #thermal-sensor-cells: should be set to 1. This property follows the
+thermal device tree bindings[2].
+
 [0] http://infocenter.arm.com/help/topic/com.arm.doc.dui0922b/index.html
 [1] Documentation/devicetree/bindings/clock/clock-bindings.txt
+[2] Documentation/devicetree/bindings/thermal/thermal.txt
 
 Example:
 
@@ -122,6 +132,11 @@ scpi_protocol: scpi@2e00 {
clock-output-names = "pxlclk0", "pxlclk1";
};
};
+
+   scpi_sensors0: sensors {
+   compatible = "arm,scpi-sensors";
+   #thermal-sensor-cells = <1>;
+   };
 };
 
 cpu@0 {
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors

[Mark Rutland]

On Mon, Sep 14, 2015 at 02:00:14PM +0100, Punit Agrawal wrote:
> The System Control Processor (SCP) provides access to SoC sensors via
> the System Control and Power Interface (SCPI) Message Protocol. Add
> bindings to allow probing of these sensors. Also support referencing
> of the sensors for setting up thermal zones via the thermal DT
> bindings.
> 
> Signed-off-by: Punit Agrawal 
> Cc: Rob Herring 
> Cc: Mark Rutland 
> Cc: Sudeep Holla 
> ---
>  Documentation/devicetree/bindings/arm/arm,scpi.txt | 15 +++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/arm,scpi.txt 
> b/Documentation/devicetree/bindings/arm/arm,scpi.txt
> index f002460..af66f7f 100644
> --- a/Documentation/devicetree/bindings/arm/arm,scpi.txt
> +++ b/Documentation/devicetree/bindings/arm/arm,scpi.txt
> @@ -72,8 +72,18 @@ Required sub-node properties:
>  - compatible : should be "arm,juno-scp-shmem" for Non-secure SRAM based
>  shared memory on Juno platforms
>  
> +Sensor bindings for the sensors based on SCPI Message Protocol
> +--
> +SCPI provides an API to access the various sensors on the SoC.
> +
> +Required properties:
> +- compatible : should be "arm,scpi-sensors".
> +- #thermal-sensor-cells: should be set to 1. This property follows the
> +  thermal device tree bindings[2].

You need to specify what the valid values for this cell are.

Mark.

> +
>  [0] http://infocenter.arm.com/help/topic/com.arm.doc.dui0922b/index.html
>  [1] Documentation/devicetree/bindings/clock/clock-bindings.txt
> +[2] Documentation/devicetree/bindings/thermal/thermal.txt
>  
>  Example:
>  
> @@ -122,6 +132,11 @@ scpi_protocol: scpi@2e00 {
>   clock-output-names = "pxlclk0", "pxlclk1";
>   };
>   };
> +
> + scpi_sensors0: sensors {
> + compatible = "arm,scpi-sensors";
> + #thermal-sensor-cells = <1>;
> + };
>  };
>  
>  cpu@0 {
> -- 
> 2.5.0
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors

[Mark Rutland]

> >> +Sensor bindings for the sensors based on SCPI Message Protocol
> >> +--
> >> +SCPI provides an API to access the various sensors on the SoC.
> >> +
> >> +Required properties:
> >> +- compatible : should be "arm,scpi-sensors".
> >> +- #thermal-sensor-cells: should be set to 1. This property follows the
> >> +   thermal device tree bindings[2].
> >
> > You need to specify what the valid values for this cell are.
> 
> The enumeration depends on the number of sensors exported by SCP
> firmware - which is platform dependent. I could add add something like
> if you think that is helpful -
> 
> "Valid cell value is a number between 0..n-1, where n is the number
> of sensors exported by SCP firmware."

Can the FW identifer space have holes? Or are they always contiguous?

If this is the same as the raw FW identifer value, specify that.
Otherwise, you need to specify the mapping.

There needs to be enough information for a dts author to figure out
which values to place in the DT.

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors

[Punit Agrawal]

Hi Mark,

Thanks for taking a look.

Mark Rutland  writes:

> On Mon, Sep 14, 2015 at 02:00:14PM +0100, Punit Agrawal wrote:
>> The System Control Processor (SCP) provides access to SoC sensors via
>> the System Control and Power Interface (SCPI) Message Protocol. Add
>> bindings to allow probing of these sensors. Also support referencing
>> of the sensors for setting up thermal zones via the thermal DT
>> bindings.
>> 
>> Signed-off-by: Punit Agrawal 
>> Cc: Rob Herring 
>> Cc: Mark Rutland 
>> Cc: Sudeep Holla 
>> ---
>>  Documentation/devicetree/bindings/arm/arm,scpi.txt | 15 +++
>>  1 file changed, 15 insertions(+)
>> 
>> diff --git a/Documentation/devicetree/bindings/arm/arm,scpi.txt 
>> b/Documentation/devicetree/bindings/arm/arm,scpi.txt
>> index f002460..af66f7f 100644
>> --- a/Documentation/devicetree/bindings/arm/arm,scpi.txt
>> +++ b/Documentation/devicetree/bindings/arm/arm,scpi.txt
>> @@ -72,8 +72,18 @@ Required sub-node properties:
>>  - compatible : should be "arm,juno-scp-shmem" for Non-secure SRAM based
>> shared memory on Juno platforms
>>  
>> +Sensor bindings for the sensors based on SCPI Message Protocol
>> +--
>> +SCPI provides an API to access the various sensors on the SoC.
>> +
>> +Required properties:
>> +- compatible : should be "arm,scpi-sensors".
>> +- #thermal-sensor-cells: should be set to 1. This property follows the
>> + thermal device tree bindings[2].
>
> You need to specify what the valid values for this cell are.

The enumeration depends on the number of sensors exported by SCP
firmware - which is platform dependent. I could add add something like
if you think that is helpful -

"Valid cell value is a number between 0..n-1, where n is the number
of sensors exported by SCP firmware."

Thanks,

Punit

>
> Mark.
>
>> +
>>  [0] http://infocenter.arm.com/help/topic/com.arm.doc.dui0922b/index.html
>>  [1] Documentation/devicetree/bindings/clock/clock-bindings.txt
>> +[2] Documentation/devicetree/bindings/thermal/thermal.txt
>>  
>>  Example:
>>  
>> @@ -122,6 +132,11 @@ scpi_protocol: scpi@2e00 {
>>  clock-output-names = "pxlclk0", "pxlclk1";
>>  };
>>  };
>> +
>> +scpi_sensors0: sensors {
>> +compatible = "arm,scpi-sensors";
>> +#thermal-sensor-cells = <1>;
>> +};
>>  };
>>  
>>  cpu@0 {
>> -- 
>> 2.5.0
>> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors

[Punit Agrawal]

Mark Rutland  writes:

>> >> +Sensor bindings for the sensors based on SCPI Message Protocol
>> >> +--
>> >> +SCPI provides an API to access the various sensors on the SoC.
>> >> +
>> >> +Required properties:
>> >> +- compatible : should be "arm,scpi-sensors".
>> >> +- #thermal-sensor-cells: should be set to 1. This property follows the
>> >> +  thermal device tree bindings[2].
>> >
>> > You need to specify what the valid values for this cell are.
>> 
>> The enumeration depends on the number of sensors exported by SCP
>> firmware - which is platform dependent. I could add add something like
>> if you think that is helpful -
>> 
>> "Valid cell value is a number between 0..n-1, where n is the number
>> of sensors exported by SCP firmware."
>
> Can the FW identifer space have holes? Or are they always contiguous?

The way the SCP interface is defined, the sensor identifiers are
contiguous, but not all are temperature sensors.

>
> If this is the same as the raw FW identifer value, specify that.
> Otherwise, you need to specify the mapping.

I'll update the patch to add mappings for Juno r0 (and r1 if I can get
my hands on one).

>
> There needs to be enough information for a dts author to figure out
> which values to place in the DT.

I understand. Except sometimes it is hard to get the firmware to commit to not
modify the ordering - discoverability and all that. :)

>
> Thanks,
> Mark.
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors

[Mark Rutland]

On Mon, Sep 14, 2015 at 03:38:36PM +0100, Punit Agrawal wrote:
> Mark Rutland  writes:
> 
> >> >> +Sensor bindings for the sensors based on SCPI Message Protocol
> >> >> +--
> >> >> +SCPI provides an API to access the various sensors on the SoC.
> >> >> +
> >> >> +Required properties:
> >> >> +- compatible : should be "arm,scpi-sensors".
> >> >> +- #thermal-sensor-cells: should be set to 1. This property follows the
> >> >> +thermal device tree bindings[2].
> >> >
> >> > You need to specify what the valid values for this cell are.
> >> 
> >> The enumeration depends on the number of sensors exported by SCP
> >> firmware - which is platform dependent. I could add add something like
> >> if you think that is helpful -
> >> 
> >> "Valid cell value is a number between 0..n-1, where n is the number
> >> of sensors exported by SCP firmware."
> >
> > Can the FW identifer space have holes? Or are they always contiguous?
> 
> The way the SCP interface is defined, the sensor identifiers are
> contiguous, but not all are temperature sensors.

Ok. So how exactly are they enumerated for this binding?

> > If this is the same as the raw FW identifer value, specify that.
> > Otherwise, you need to specify the mapping.
> 
> I'll update the patch to add mappings for Juno r0 (and r1 if I can get
> my hands on one).

If there's identical logic mapping the two, we might just be able to
describe that rather than having to add tables all the time.

> > There needs to be enough information for a dts author to figure out
> > which values to place in the DT.
> 
> I understand. Except sometimes it is hard to get the firmware to commit to not
> modify the ordering - discoverability and all that. :)

If they do that, then things are broken regardless, no? I guess that'll
be clear if/when I see how the mapping works.

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors

[Punit Agrawal]

Mark Rutland  writes:

> On Mon, Sep 14, 2015 at 03:38:36PM +0100, Punit Agrawal wrote:
>> Mark Rutland  writes:
>> 
>> >> >> +Sensor bindings for the sensors based on SCPI Message Protocol
>> >> >> +--
>> >> >> +SCPI provides an API to access the various sensors on the SoC.
>> >> >> +
>> >> >> +Required properties:
>> >> >> +- compatible : should be "arm,scpi-sensors".
>> >> >> +- #thermal-sensor-cells: should be set to 1. This property follows the
>> >> >> +   thermal device tree bindings[2].
>> >> >
>> >> > You need to specify what the valid values for this cell are.
>> >> 
>> >> The enumeration depends on the number of sensors exported by SCP
>> >> firmware - which is platform dependent. I could add add something like
>> >> if you think that is helpful -
>> >> 
>> >> "Valid cell value is a number between 0..n-1, where n is the number
>> >> of sensors exported by SCP firmware."
>> >
>> > Can the FW identifer space have holes? Or are they always contiguous?
>> 
>> The way the SCP interface is defined, the sensor identifiers are
>> contiguous, but not all are temperature sensors.
>
> Ok. So how exactly are they enumerated for this binding?

The sensor enumeration for r0 (which I've verified) and r1 can be found
at [0]. The valid cell values are identifiers for the temperature sensors.

>
>> > If this is the same as the raw FW identifer value, specify that.
>> > Otherwise, you need to specify the mapping.
>> 
>> I'll update the patch to add mappings for Juno r0 (and r1 if I can get
>> my hands on one).
>
> If there's identical logic mapping the two, we might just be able to
> describe that rather than having to add tables all the time.
>

After seeing the mapping already published, I am wondering if there is
any value in duplicating the information. If there are no objections,
I'll update this patch to add pointers instead.

>> > There needs to be enough information for a dts author to figure out
>> > which values to place in the DT.
>> 
>> I understand. Except sometimes it is hard to get the firmware to commit to 
>> not
>> modify the ordering - discoverability and all that. :)
>
> If they do that, then things are broken regardless, no? I guess that'll
> be clear if/when I see how the mapping works.

[0] 
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0922b/apas03s22.html

>
> Thanks,
> Mark.
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors

[Mark Rutland]

On Mon, Sep 14, 2015 at 04:01:09PM +0100, Punit Agrawal wrote:
> Mark Rutland  writes:
> 
> > On Mon, Sep 14, 2015 at 03:38:36PM +0100, Punit Agrawal wrote:
> >> Mark Rutland  writes:
> >> 
> >> >> >> +Sensor bindings for the sensors based on SCPI Message Protocol
> >> >> >> +--
> >> >> >> +SCPI provides an API to access the various sensors on the SoC.
> >> >> >> +
> >> >> >> +Required properties:
> >> >> >> +- compatible : should be "arm,scpi-sensors".
> >> >> >> +- #thermal-sensor-cells: should be set to 1. This property follows 
> >> >> >> the
> >> >> >> + thermal device tree bindings[2].
> >> >> >
> >> >> > You need to specify what the valid values for this cell are.
> >> >> 
> >> >> The enumeration depends on the number of sensors exported by SCP
> >> >> firmware - which is platform dependent. I could add add something like
> >> >> if you think that is helpful -
> >> >> 
> >> >> "Valid cell value is a number between 0..n-1, where n is the number
> >> >> of sensors exported by SCP firmware."
> >> >
> >> > Can the FW identifer space have holes? Or are they always contiguous?
> >> 
> >> The way the SCP interface is defined, the sensor identifiers are
> >> contiguous, but not all are temperature sensors.
> >
> > Ok. So how exactly are they enumerated for this binding?
> 
> The sensor enumeration for r0 (which I've verified) and r1 can be found
> at [0]. The valid cell values are identifiers for the temperature sensors.

Ok. That's fine by me; I was confused and thought that there was some
internal mapping.

> >> > If this is the same as the raw FW identifer value, specify that.
> >> > Otherwise, you need to specify the mapping.
> >> 
> >> I'll update the patch to add mappings for Juno r0 (and r1 if I can get
> >> my hands on one).
> >
> > If there's identical logic mapping the two, we might just be able to
> > describe that rather than having to add tables all the time.
> >
> 
> After seeing the mapping already published, I am wondering if there is
> any value in duplicating the information. If there are no objections,
> I'll update this patch to add pointers instead.

That's fine by me. The important part is that the value is a raw Sensor
ID as the FW uses. So long as we state that, the IDs themselves can come
from whatever documentation is valid for a particular instance.

Thanks for the info!

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors

[Punit Agrawal]

Mark Rutland  writes:

> On Mon, Sep 14, 2015 at 04:01:09PM +0100, Punit Agrawal wrote:
>> Mark Rutland  writes:
>> 

[...]

>> 
>> After seeing the mapping already published, I am wondering if there is
>> any value in duplicating the information. If there are no objections,
>> I'll update this patch to add pointers instead.
>
> That's fine by me. The important part is that the value is a raw Sensor
> ID as the FW uses. So long as we state that, the IDs themselves can come
> from whatever documentation is valid for a particular instance.
>
> Thanks for the info!

Sure. I've updated the patch to explain what the valid cell values are
along with a pointer to platform documentation for Juno R0 and R1.

I've also added an example to show usage of the sensors to create a
thermal zone.

I'll post a new version shortly.

Thanks for your having a look.

>
> Thanks,
> Mark.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 1/5] Documentation: add DT bindings for ARM SCPI sensors

[Jon Medhurst (Tixy)]

On Mon, 2015-09-14 at 15:38 +0100, Punit Agrawal wrote:
> Mark Rutland  writes:
> 
> >> >> +Sensor bindings for the sensors based on SCPI Message Protocol
> >> >> +--
> >> >> +SCPI provides an API to access the various sensors on the SoC.
> >> >> +
> >> >> +Required properties:
> >> >> +- compatible : should be "arm,scpi-sensors".
> >> >> +- #thermal-sensor-cells: should be set to 1. This property follows the
> >> >> +thermal device tree bindings[2].
> >> >
> >> > You need to specify what the valid values for this cell are.
> >> 
> >> The enumeration depends on the number of sensors exported by SCP
> >> firmware - which is platform dependent. I could add add something like
> >> if you think that is helpful -
> >> 
> >> "Valid cell value is a number between 0..n-1, where n is the number
> >> of sensors exported by SCP firmware."
> >
> > Can the FW identifer space have holes? Or are they always contiguous?
> 
> The way the SCP interface is defined, the sensor identifiers are
> contiguous,

Is there any documentation other than DUI0922A? [1] From what I can seen
that just says it's a 16-bit value and doesn't put any particular
constraints on its value.

[1] 
http://community.arm.com/servlet/JiveServlet/download/8401-40-18262/DUI0922A_scp_message_interface.pdf

-- 
Tixy

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/