[RFC] Addition of Altera Arria10 System Resource Chip

[RFC] Addition of Altera Arria10 System Resource Chip

[tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx]

The Altera Arria10 Development Kit includes a system resource chip
that fits under the Multi-Function Device framework. The chip has
hardware monitoring functions and a GPIO expander over the SPI bus.

[RFC 1/8] dt-bindings: mfd: Add Altera Arria10 System Resource Chip
[RFC 2/8] MAINTAINERS: Addition of Altera Arria10 System Resource
[RFC 3/8] mfd: altr_a10sr: Add Altera Arria10 DevKit System Resource
[RFC 4/8] gpio: altera-a10sr: Add A10 System Resource Chip GPIO
[RFC 5/8] ARM: socfpga: dts: Add Devkit A10-SR fields for Arria10
[RFC 6/8] ARM: socfpga: dts: Add LED framework to A10-SR GPIO
[RFC 7/8] hwmon: Altera Arria10 System Resource Chip - HW Monitor
[RFC 8/8] ARM: socfpga: dts: Add Devkit Arria10-SR HWMON
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC 1/8] dt-bindings: mfd: Add Altera Arria10 System Resource Chip bindings

[tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx]

From: Thor Thayer <tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>

The Altera Arria10 Devkit System Resource chip is a Multi-Function
Device, it has two subdevices:
     - GPIO
     - HWMON

This patch adds documentation for the Altera A10-SR DT bindings.

Signed-off-by: Thor Thayer <tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>
---
 .../devicetree/bindings/mfd/altera-a10sr.txt       |   35 ++++++++++++++++++++
 1 file changed, 35 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/altera-a10sr.txt

diff --git a/Documentation/devicetree/bindings/mfd/altera-a10sr.txt b/Documentation/devicetree/bindings/mfd/altera-a10sr.txt
new file mode 100644
index 0000000..564c761
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/altera-a10sr.txt
@@ -0,0 +1,35 @@
+* Altera Arria10 Development Kit System Resource Chip
+
+Required parent device properties:
+- compatible : "altr,altr_a10sr"
+- spi-max-frequency : Maximum SPI frequency.
+- reg : the SPI Chip Select address for the Arria10 System Resource chip
+
+The A10SR consists of this varied group of sub-devices:
+
+Device                   Description
+------                   ----------
+altr_a10sr_gpio          GPIO Controller
+altr_a10sr_hwmon         Hardware Monitor
+
+The LEDs are implemented entirely in the device tree using
+the gpio-led framework.
+
+Example:
+
+        a10-sr: a10-sr@0 {
+		compatible = "altr,altr-a10sr";
+		reg = <0>;
+		spi-max-frequency = <100000>;
+
+		a10sr_gpio: a10sr_gpio {
+			compatible = "altr,a10sr-gpio";
+			gpio-controller;
+			#gpio-cells = <2>;
+			ngpios = <16>;
+		};
+
+		a10sr_hwmon: a10sr_hwmon {
+			compatible = "altr,a10sr-hwmon";
+		};
+	};
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 1/8] dt-bindings: mfd: Add Altera Arria10 System Resource Chip bindings

[Lee Jones]

On Tue, 29 Mar 2016, tthayer@opensource.altera.com wrote:

> From: Thor Thayer <tthayer@opensource.altera.com>
> 
> The Altera Arria10 Devkit System Resource chip is a Multi-Function
> Device, it has two subdevices:
>      - GPIO
>      - HWMON
> 
> This patch adds documentation for the Altera A10-SR DT bindings.
> 
> Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
> ---
>  .../devicetree/bindings/mfd/altera-a10sr.txt       |   35 ++++++++++++++++++++
>  1 file changed, 35 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/altera-a10sr.txt
> 
> diff --git a/Documentation/devicetree/bindings/mfd/altera-a10sr.txt b/Documentation/devicetree/bindings/mfd/altera-a10sr.txt
> new file mode 100644
> index 0000000..564c761
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/altera-a10sr.txt
> @@ -0,0 +1,35 @@
> +* Altera Arria10 Development Kit System Resource Chip
> +
> +Required parent device properties:
> +- compatible : "altr,altr_a10sr"
> +- spi-max-frequency : Maximum SPI frequency.
> +- reg : the SPI Chip Select address for the Arria10 System Resource chip

DT bindings are much easier to read in the following format:

- compatible		: "altr,altr_a10sr"
- spi-max-frequency	: Maximum SPI frequency.
- reg			: the SPI Chip Select address for the Arria10 System Resource chip

... also, sentences start with an uppercase char.

> +The A10SR consists of this varied group of sub-devices:
> +
> +Device                   Description
> +------                   ----------
> +altr_a10sr_gpio          GPIO Controller
> +altr_a10sr_hwmon         Hardware Monitor
> +
> +The LEDs are implemented entirely in the device tree using
> +the gpio-led framework.

This is a Linuxisum and should not live in DT bindings.

> +Example:
> +
> +        a10-sr: a10-sr@0 {

Nodes should be named after their device 'type'.

Does this device really start a address 0?

> +		compatible = "altr,altr-a10sr";
> +		reg = <0>;
> +		spi-max-frequency = <100000>;
> +
> +		a10sr_gpio: a10sr_gpio {

Device type only please.

> +			compatible = "altr,a10sr-gpio";
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +			ngpios = <16>;
> +		};
> +
> +		a10sr_hwmon: a10sr_hwmon {

Device type only please.

> +			compatible = "altr,a10sr-hwmon";
> +		};
> +	};

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 1/8] dt-bindings: mfd: Add Altera Arria10 System Resource Chip bindings

[Rob Herring]

On Wed, Mar 30, 2016 at 12:35:32PM +0100, Lee Jones wrote:
> On Tue, 29 Mar 2016, tthayer@opensource.altera.com wrote:
> 
> > From: Thor Thayer <tthayer@opensource.altera.com>
> > 
> > The Altera Arria10 Devkit System Resource chip is a Multi-Function
> > Device, it has two subdevices:
> >      - GPIO
> >      - HWMON
> > 
> > This patch adds documentation for the Altera A10-SR DT bindings.
> > 
> > Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
> > ---
> >  .../devicetree/bindings/mfd/altera-a10sr.txt       |   35 ++++++++++++++++++++
> >  1 file changed, 35 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/mfd/altera-a10sr.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/mfd/altera-a10sr.txt b/Documentation/devicetree/bindings/mfd/altera-a10sr.txt
> > new file mode 100644
> > index 0000000..564c761
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mfd/altera-a10sr.txt
> > @@ -0,0 +1,35 @@
> > +* Altera Arria10 Development Kit System Resource Chip
> > +
> > +Required parent device properties:
> > +- compatible : "altr,altr_a10sr"

Why "altr" twice?

Don't use underscores.

> > +- spi-max-frequency : Maximum SPI frequency.
> > +- reg : the SPI Chip Select address for the Arria10 System Resource chip
> 
> DT bindings are much easier to read in the following format:
> 
> - compatible		: "altr,altr_a10sr"
> - spi-max-frequency	: Maximum SPI frequency.
> - reg			: the SPI Chip Select address for the Arria10 System Resource chip
> 
> ... also, sentences start with an uppercase char.
> 
> > +The A10SR consists of this varied group of sub-devices:
> > +
> > +Device                   Description
> > +------                   ----------
> > +altr_a10sr_gpio          GPIO Controller
> > +altr_a10sr_hwmon         Hardware Monitor
> > +
> > +The LEDs are implemented entirely in the device tree using
> > +the gpio-led framework.
> 
> This is a Linuxisum and should not live in DT bindings.
> 
> > +Example:
> > +
> > +        a10-sr: a10-sr@0 {
> 
> Nodes should be named after their device 'type'.
> 
> Does this device really start a address 0?

Being a SPI device, I imagine so.

Re: [RFC 1/8] dt-bindings: mfd: Add Altera Arria10 System Resource Chip bindings

[Thor Thayer]

Hi Lee,

On 03/30/2016 06:35 AM, Lee Jones wrote:
> On Tue, 29 Mar 2016, tthayer@opensource.altera.com wrote:
>
>> From: Thor Thayer <tthayer@opensource.altera.com>
>>

[..snip..]

>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mfd/altera-a10sr.txt
>> @@ -0,0 +1,35 @@
>> +* Altera Arria10 Development Kit System Resource Chip
>> +
>> +Required parent device properties:
>> +- compatible : "altr,altr_a10sr"
>> +- spi-max-frequency : Maximum SPI frequency.
>> +- reg : the SPI Chip Select address for the Arria10 System Resource chip
>
> DT bindings are much easier to read in the following format:
>
> - compatible		: "altr,altr_a10sr"
> - spi-max-frequency	: Maximum SPI frequency.
> - reg			: the SPI Chip Select address for the Arria10 System Resource chip
>
> ... also, sentences start with an uppercase char.
>

OK.

>> +The A10SR consists of this varied group of sub-devices:
>> +
>> +Device                   Description
>> +------                   ----------
>> +altr_a10sr_gpio          GPIO Controller
>> +altr_a10sr_hwmon         Hardware Monitor
>> +
>> +The LEDs are implemented entirely in the device tree using
>> +the gpio-led framework.
>
> This is a Linuxisum and should not live in DT bindings.
>

I was following the format of other mfd binding documents such as 
Documentation/devicetree/bindings/mfd/da9055.txt so I'll need your help 
understanding this.

I'm not familiar with the phrase Linuxisum. A Google search turns up 
several threads referencing Linuxisum but I can't seem to find the 
definition. One thread seems to imply that an existing driver such as 
GPIO is a Linuxisum and should not be re-defined. Am I understanding 
correctly?

>> +Example:
>> +
>> +        a10-sr: a10-sr@0 {
>
> Nodes should be named after their device 'type'.
>
> Does this device really start a address 0?

OK. If I understand, this should be named after mfd then?
>
>> +		compatible = "altr,altr-a10sr";
>> +		reg = <0>;
>> +		spi-max-frequency = <100000>;
>> +
>> +		a10sr_gpio: a10sr_gpio {
>
> Device type only please.

And this would be a gpio?

Thanks for your comments and for reviewing!

>
>> +			compatible = "altr,a10sr-gpio";
>> +			gpio-controller;
>> +			#gpio-cells = <2>;
>> +			ngpios = <16>;
>> +		};
>> +
>> +		a10sr_hwmon: a10sr_hwmon {
>
> Device type only please.
>
>> +			compatible = "altr,a10sr-hwmon";
>> +		};
>> +	};
>

Re: [RFC 1/8] dt-bindings: mfd: Add Altera Arria10 System Resource Chip bindings

[Lee Jones]

On Thu, 31 Mar 2016, Thor Thayer wrote:
> On 03/30/2016 06:35 AM, Lee Jones wrote:
> >On Tue, 29 Mar 2016, tthayer@opensource.altera.com wrote:
> >
> >>From: Thor Thayer <tthayer@opensource.altera.com>

[...]

> >>+The A10SR consists of this varied group of sub-devices:
> >>+
> >>+Device                   Description
> >>+------                   ----------
> >>+altr_a10sr_gpio          GPIO Controller
> >>+altr_a10sr_hwmon         Hardware Monitor
> >>+
> >>+The LEDs are implemented entirely in the device tree using
> >>+the gpio-led framework.
> >
> >This is a Linuxisum and should not live in DT bindings.
> 
> I was following the format of other mfd binding documents such as
> Documentation/devicetree/bindings/mfd/da9055.txt so I'll need your
> help understanding this.
> 
> I'm not familiar with the phrase Linuxisum. A Google search turns up
> several threads referencing Linuxisum but I can't seem to find the
> definition. One thread seems to imply that an existing driver such
> as GPIO is a Linuxisum and should not be re-defined. Am I
> understanding correctly?

Linuxisum is a made up word.  Actually, it looks like I placed a
superfluous 'u' in there, but I assume most people would get the gist.
Some examples ending in "ism" which might push the point across are
"colloquialism" and "feminism", where the "ism" can probably be taken
to mean "pertaining to".  So in the example above, we might reasonably
conclude that I meant "pertaining to Linux", which I did.

In other words "the gpio-led framework" is something we have in Linux,
but might not exist in other OSes.  And considering DT is supposed to
be OS agnostic and the documentation relevant to all OSes, you can not
and should not document Linuxisms.

> >>+Example:
> >>+
> >>+        a10-sr: a10-sr@0 {
> >
> >Nodes should be named after their device 'type'.
> >
> >Does this device really start a address 0?
> 
> OK. If I understand, this should be named after mfd then?

MFDs are usually a little tougher, but in your case I think it should
be "resource-manager" or similar.

> >>+		compatible = "altr,altr-a10sr";
> >>+		reg = <0>;
> >>+		spi-max-frequency = <100000>;
> >>+
> >>+		a10sr_gpio: a10sr_gpio {
> >
> >Device type only please.
> 
> And this would be a gpio?

Exactly.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 1/8] dt-bindings: mfd: Add Altera Arria10 System Resource Chip bindings

[Thor Thayer]

On 04/01/2016 03:14 AM, Lee Jones wrote:
> On Thu, 31 Mar 2016, Thor Thayer wrote:
>> On 03/30/2016 06:35 AM, Lee Jones wrote:
>>> On Tue, 29 Mar 2016, tthayer@opensource.altera.com wrote:
>>>
>>>> From: Thor Thayer <tthayer@opensource.altera.com>
>
> [...]
>
>>>> +The A10SR consists of this varied group of sub-devices:
>>>> +
>>>> +Device                   Description
>>>> +------                   ----------
>>>> +altr_a10sr_gpio          GPIO Controller
>>>> +altr_a10sr_hwmon         Hardware Monitor
>>>> +
>>>> +The LEDs are implemented entirely in the device tree using
>>>> +the gpio-led framework.
>>>
>>> This is a Linuxisum and should not live in DT bindings.
>>
>> I was following the format of other mfd binding documents such as
>> Documentation/devicetree/bindings/mfd/da9055.txt so I'll need your
>> help understanding this.
>>
>> I'm not familiar with the phrase Linuxisum. A Google search turns up
>> several threads referencing Linuxisum but I can't seem to find the
>> definition. One thread seems to imply that an existing driver such
>> as GPIO is a Linuxisum and should not be re-defined. Am I
>> understanding correctly?
>
> Linuxisum is a made up word.  Actually, it looks like I placed a
> superfluous 'u' in there, but I assume most people would get the gist.
> Some examples ending in "ism" which might push the point across are
> "colloquialism" and "feminism", where the "ism" can probably be taken
> to mean "pertaining to".  So in the example above, we might reasonably
> conclude that I meant "pertaining to Linux", which I did.
>
> In other words "the gpio-led framework" is something we have in Linux,
> but might not exist in other OSes.  And considering DT is supposed to
> be OS agnostic and the documentation relevant to all OSes, you can not
> and should not document Linuxisms.
>

Got it. Thanks for the explanation! I'll make the changes.

>>>> +Example:
>>>> +
>>>> +        a10-sr: a10-sr@0 {
>>>
>>> Nodes should be named after their device 'type'.
>>>
>>> Does this device really start a address 0?
>>
>> OK. If I understand, this should be named after mfd then?
>
> MFDs are usually a little tougher, but in your case I think it should
> be "resource-manager" or similar.
>
>>>> +		compatible = "altr,altr-a10sr";
>>>> +		reg = <0>;
>>>> +		spi-max-frequency = <100000>;
>>>> +
>>>> +		a10sr_gpio: a10sr_gpio {
>>>
>>> Device type only please.
>>
>> And this would be a gpio?
>
> Exactly.
>

Re: [RFC 1/8] dt-bindings: mfd: Add Altera Arria10 System Resource Chip bindings

[Thor Thayer]

Hi Lee

On 03/30/2016 06:35 AM, Lee Jones wrote:
> On Tue, 29 Mar 2016, tthayer@opensource.altera.com wrote:
>
>> From: Thor Thayer <tthayer@opensource.altera.com>
>>
>> The Altera Arria10 Devkit System Resource chip is a Multi-Function
>> Device, it has two subdevices:
>>       - GPIO
>>       - HWMON
>>
>> This patch adds documentation for the Altera A10-SR DT bindings.
>>
>> Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
>> ---
>>   .../devicetree/bindings/mfd/altera-a10sr.txt       |   35 ++++++++++++++++++++
>>   1 file changed, 35 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/mfd/altera-a10sr.txt
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/altera-a10sr.txt b/Documentation/devicetree/bindings/mfd/altera-a10sr.txt
>> new file mode 100644
>> index 0000000..564c761
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mfd/altera-a10sr.txt
>> @@ -0,0 +1,35 @@
>> +* Altera Arria10 Development Kit System Resource Chip
>> +
>> +Required parent device properties:
>> +- compatible : "altr,altr_a10sr"
>> +- spi-max-frequency : Maximum SPI frequency.
>> +- reg : the SPI Chip Select address for the Arria10 System Resource chip
>
> DT bindings are much easier to read in the following format:
>
> - compatible		: "altr,altr_a10sr"
> - spi-max-frequency	: Maximum SPI frequency.
> - reg			: the SPI Chip Select address for the Arria10 System Resource chip
>
> ... also, sentences start with an uppercase char.
>
>> +The A10SR consists of this varied group of sub-devices:
>> +
>> +Device                   Description
>> +------                   ----------
>> +altr_a10sr_gpio          GPIO Controller
>> +altr_a10sr_hwmon         Hardware Monitor
>> +
>> +The LEDs are implemented entirely in the device tree using
>> +the gpio-led framework.
>
> This is a Linuxisum and should not live in DT bindings.
>
>> +Example:
>> +
>> +        a10-sr: a10-sr@0 {
>
> Nodes should be named after their device 'type'.
>
> Does this device really start a address 0?
>

I see in the documentation on device trees there are a number of 
categories I can use. GPIO is easy because it is one of the categories 
but I'm not sure about the new device I'm adding since the a10sr is a 
new device.

I believe I should only call out the name and address on the SPI bus like:

a10sr@0 {

>> +		compatible = "altr,altr-a10sr";
>> +		reg = <0>;
>> +		spi-max-frequency = <100000>;
>> +
>> +		a10sr_gpio: a10sr_gpio {
>
> Device type only please.
>

and this would be a10sr_gpio: gpio-controller {

Does that seem correct?

>> +			compatible = "altr,a10sr-gpio";
>> +			gpio-controller;
>> +			#gpio-cells = <2>;
>> +			ngpios = <16>;
>> +		};
>> +
>> +		a10sr_hwmon: a10sr_hwmon {
>
> Device type only please.
>
I need to revisit where this will live (hwmon does not seem to be the 
correct place) so it will change but I can follow the format above if it 
is correct.

Thanks for reviewing.

>> +			compatible = "altr,a10sr-hwmon";
>> +		};
>> +	};
>

Re: [RFC 1/8] dt-bindings: mfd: Add Altera Arria10 System Resource Chip bindings

[Lee Jones]

On Fri, 15 Apr 2016, Thor Thayer wrote:
> On 03/30/2016 06:35 AM, Lee Jones wrote:
> >On Tue, 29 Mar 2016, tthayer@opensource.altera.com wrote:
> >
> >>From: Thor Thayer <tthayer@opensource.altera.com>
> >>
> >>The Altera Arria10 Devkit System Resource chip is a Multi-Function
> >>Device, it has two subdevices:
> >>      - GPIO
> >>      - HWMON
> >>
> >>This patch adds documentation for the Altera A10-SR DT bindings.
> >>
> >>Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
> >>---
> >>  .../devicetree/bindings/mfd/altera-a10sr.txt       |   35 ++++++++++++++++++++
> >>  1 file changed, 35 insertions(+)
> >>  create mode 100644 Documentation/devicetree/bindings/mfd/altera-a10sr.txt
> >>
> >>diff --git a/Documentation/devicetree/bindings/mfd/altera-a10sr.txt b/Documentation/devicetree/bindings/mfd/altera-a10sr.txt
> >>new file mode 100644
> >>index 0000000..564c761
> >>--- /dev/null
> >>+++ b/Documentation/devicetree/bindings/mfd/altera-a10sr.txt
> >>@@ -0,0 +1,35 @@
> >>+* Altera Arria10 Development Kit System Resource Chip
> >>+
> >>+Required parent device properties:
> >>+- compatible : "altr,altr_a10sr"
> >>+- spi-max-frequency : Maximum SPI frequency.
> >>+- reg : the SPI Chip Select address for the Arria10 System Resource chip
> >
> >DT bindings are much easier to read in the following format:
> >
> >- compatible		: "altr,altr_a10sr"
> >- spi-max-frequency	: Maximum SPI frequency.
> >- reg			: the SPI Chip Select address for the Arria10 System Resource chip
> >
> >... also, sentences start with an uppercase char.
> >
> >>+The A10SR consists of this varied group of sub-devices:
> >>+
> >>+Device                   Description
> >>+------                   ----------
> >>+altr_a10sr_gpio          GPIO Controller
> >>+altr_a10sr_hwmon         Hardware Monitor
> >>+
> >>+The LEDs are implemented entirely in the device tree using
> >>+the gpio-led framework.
> >
> >This is a Linuxisum and should not live in DT bindings.
> >
> >>+Example:
> >>+
> >>+        a10-sr: a10-sr@0 {
> >
> >Nodes should be named after their device 'type'.
> >
> >Does this device really start a address 0?
> >
> 
> I see in the documentation on device trees there are a number of
> categories I can use. GPIO is easy because it is one of the
> categories but I'm not sure about the new device I'm adding since
> the a10sr is a new device.

It's always difficult with MFDs as they are by their very nature, more
than one device.  But how about 'resource-manager'?

> I believe I should only call out the name and address on the SPI bus like:
> 
> a10sr@0 {

Correct.

> >>+		compatible = "altr,altr-a10sr";
> >>+		reg = <0>;
> >>+		spi-max-frequency = <100000>;
> >>+
> >>+		a10sr_gpio: a10sr_gpio {
> >
> >Device type only please.
> >
> 
> and this would be a10sr_gpio: gpio-controller {
> 
> Does that seem correct?

Also correct.  No address though?

> >>+			compatible = "altr,a10sr-gpio";
> >>+			gpio-controller;
> >>+			#gpio-cells = <2>;
> >>+			ngpios = <16>;
> >>+		};
> >>+
> >>+		a10sr_hwmon: a10sr_hwmon {
> >
> >Device type only please.
> >
> I need to revisit where this will live (hwmon does not seem to be
> the correct place) so it will change but I can follow the format
> above if it is correct.
> 
> Thanks for reviewing.
> 
> >>+			compatible = "altr,a10sr-hwmon";
> >>+		};
> >>+	};
> >

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 1/8] dt-bindings: mfd: Add Altera Arria10 System Resource Chip bindings

[Thor Thayer]

Hi Lee,

On 04/18/2016 02:43 AM, Lee Jones wrote:
> On Fri, 15 Apr 2016, Thor Thayer wrote:
>> On 03/30/2016 06:35 AM, Lee Jones wrote:
>>> On Tue, 29 Mar 2016, tthayer@opensource.altera.com wrote:
>>>
>>>> From: Thor Thayer <tthayer@opensource.altera.com>
>>>>
>>>> The Altera Arria10 Devkit System Resource chip is a Multi-Function
>>>> Device, it has two subdevices:
>>>>       - GPIO
>>>>       - HWMON
>>>>
>>>> This patch adds documentation for the Altera A10-SR DT bindings.
>>>>
>>>> Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
>>>> ---
>>>>   .../devicetree/bindings/mfd/altera-a10sr.txt       |   35 ++++++++++++++++++++
>>>>   1 file changed, 35 insertions(+)
>>>>   create mode 100644 Documentation/devicetree/bindings/mfd/altera-a10sr.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/mfd/altera-a10sr.txt b/Documentation/devicetree/bindings/mfd/altera-a10sr.txt
>>>> new file mode 100644
>>>> index 0000000..564c761
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/mfd/altera-a10sr.txt
>>>> @@ -0,0 +1,35 @@
>>>> +* Altera Arria10 Development Kit System Resource Chip
>>>> +
>>>> +Required parent device properties:
>>>> +- compatible : "altr,altr_a10sr"
>>>> +- spi-max-frequency : Maximum SPI frequency.
>>>> +- reg : the SPI Chip Select address for the Arria10 System Resource chip
>>>
>>> DT bindings are much easier to read in the following format:
>>>
>>> - compatible		: "altr,altr_a10sr"
>>> - spi-max-frequency	: Maximum SPI frequency.
>>> - reg			: the SPI Chip Select address for the Arria10 System Resource chip
>>>
>>> ... also, sentences start with an uppercase char.
>>>
>>>> +The A10SR consists of this varied group of sub-devices:
>>>> +
>>>> +Device                   Description
>>>> +------                   ----------
>>>> +altr_a10sr_gpio          GPIO Controller
>>>> +altr_a10sr_hwmon         Hardware Monitor
>>>> +
>>>> +The LEDs are implemented entirely in the device tree using
>>>> +the gpio-led framework.
>>>
>>> This is a Linuxisum and should not live in DT bindings.
>>>
>>>> +Example:
>>>> +
>>>> +        a10-sr: a10-sr@0 {
>>>
>>> Nodes should be named after their device 'type'.
>>>
>>> Does this device really start a address 0?
>>>
>>
>> I see in the documentation on device trees there are a number of
>> categories I can use. GPIO is easy because it is one of the
>> categories but I'm not sure about the new device I'm adding since
>> the a10sr is a new device.
>
> It's always difficult with MFDs as they are by their very nature, more
> than one device.  But how about 'resource-manager'?
>
OK. Yes, that would be a good name. Thanks.

>> I believe I should only call out the name and address on the SPI bus like:
>>
>> a10sr@0 {
>
> Correct.
>
>>>> +		compatible = "altr,altr-a10sr";
>>>> +		reg = <0>;
>>>> +		spi-max-frequency = <100000>;
>>>> +
>>>> +		a10sr_gpio: a10sr_gpio {
>>>
>>> Device type only please.
>>>
>>
>> and this would be a10sr_gpio: gpio-controller {
>>
>> Does that seem correct?
>
> Also correct.  No address though?
>

Thank you. It is at a fixed address inside the SPI device. When making 
this binding I followed the format of other gpio controllers like the 
tps65086 and lp3943 which didn't have an address.

Thanks for the clarification.

>>>> +			compatible = "altr,a10sr-gpio";
>>>> +			gpio-controller;
>>>> +			#gpio-cells = <2>;
>>>> +			ngpios = <16>;
>>>> +		};
>>>> +
>>>> +		a10sr_hwmon: a10sr_hwmon {
>>>
>>> Device type only please.
>>>
>> I need to revisit where this will live (hwmon does not seem to be
>> the correct place) so it will change but I can follow the format
>> above if it is correct.
>>
>> Thanks for reviewing.
>>
>>>> +			compatible = "altr,a10sr-hwmon";
>>>> +		};
>>>> +	};
>>>
>

Re: [RFC 1/8] dt-bindings: mfd: Add Altera Arria10 System Resource Chip bindings

[Lee Jones]

On Fri, 15 Apr 2016, Thor Thayer wrote:
> On 03/30/2016 06:35 AM, Lee Jones wrote:
> >On Tue, 29 Mar 2016, tthayer@opensource.altera.com wrote:
> >
> >>From: Thor Thayer <tthayer@opensource.altera.com>
> >>
> >>The Altera Arria10 Devkit System Resource chip is a Multi-Function
> >>Device, it has two subdevices:
> >>      - GPIO
> >>      - HWMON
> >>
> >>This patch adds documentation for the Altera A10-SR DT bindings.
> >>
> >>Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
> >>---
> >>  .../devicetree/bindings/mfd/altera-a10sr.txt       |   35 ++++++++++++++++++++
> >>  1 file changed, 35 insertions(+)
> >>  create mode 100644 Documentation/devicetree/bindings/mfd/altera-a10sr.txt
> >>
> >>diff --git a/Documentation/devicetree/bindings/mfd/altera-a10sr.txt b/Documentation/devicetree/bindings/mfd/altera-a10sr.txt
> >>new file mode 100644
> >>index 0000000..564c761
> >>--- /dev/null
> >>+++ b/Documentation/devicetree/bindings/mfd/altera-a10sr.txt
> >>@@ -0,0 +1,35 @@
> >>+* Altera Arria10 Development Kit System Resource Chip
> >>+
> >>+Required parent device properties:
> >>+- compatible : "altr,altr_a10sr"
> >>+- spi-max-frequency : Maximum SPI frequency.
> >>+- reg : the SPI Chip Select address for the Arria10 System Resource chip

[...]

> >>+		a10sr_hwmon: a10sr_hwmon {
> >
> >Device type only please.
> >
> I need to revisit where this will live (hwmon does not seem to be
> the correct place) so it will change but I can follow the format
> above if it is correct.

BTW, "hwmon" is a subsystem in Linux, therefore is a Linuxism and is
not allowed in DT.  What does the device *actually* do?

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 1/8] dt-bindings: mfd: Add Altera Arria10 System Resource Chip bindings

[Thor Thayer]

Hi Lee,

On 04/18/2016 02:45 AM, Lee Jones wrote:
> On Fri, 15 Apr 2016, Thor Thayer wrote:
>> On 03/30/2016 06:35 AM, Lee Jones wrote:
>>> On Tue, 29 Mar 2016, tthayer@opensource.altera.com wrote:
>>>
>>>> From: Thor Thayer <tthayer@opensource.altera.com>
>>>>
>>>> The Altera Arria10 Devkit System Resource chip is a Multi-Function
>>>> Device, it has two subdevices:
>>>>       - GPIO
>>>>       - HWMON
>>>>
>>>> This patch adds documentation for the Altera A10-SR DT bindings.
>>>>
>>>> Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
>>>> ---
>>>>   .../devicetree/bindings/mfd/altera-a10sr.txt       |   35 ++++++++++++++++++++
>>>>   1 file changed, 35 insertions(+)
>>>>   create mode 100644 Documentation/devicetree/bindings/mfd/altera-a10sr.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/mfd/altera-a10sr.txt b/Documentation/devicetree/bindings/mfd/altera-a10sr.txt
>>>> new file mode 100644
>>>> index 0000000..564c761
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/mfd/altera-a10sr.txt
>>>> @@ -0,0 +1,35 @@
>>>> +* Altera Arria10 Development Kit System Resource Chip
>>>> +
>>>> +Required parent device properties:
>>>> +- compatible : "altr,altr_a10sr"
>>>> +- spi-max-frequency : Maximum SPI frequency.
>>>> +- reg : the SPI Chip Select address for the Arria10 System Resource chip
>
> [...]
>
>>>> +		a10sr_hwmon: a10sr_hwmon {
>>>
>>> Device type only please.
>>>
>> I need to revisit where this will live (hwmon does not seem to be
>> the correct place) so it will change but I can follow the format
>> above if it is correct.
>
> BTW, "hwmon" is a subsystem in Linux, therefore is a Linuxism and is
> not allowed in DT.  What does the device *actually* do?
>

OK. I'll be careful not to introduce the Linux subsystem name.

This module indicates whether the power supplies are at the correct 
voltage. It uses a boolean instead of giving an actual voltage value as 
required by HWMON. In other words it is a comparator instead of an 
Analog-to-Digital Converter.

I could call it a power supply supervisor or voltage status monitor but 
it only acts in a passive role. There is no output to trigger an error - 
only polling, so supervisor doesn't seem like a good name.

Maybe something like this?

	power_supply_status {
		compatible = "altr,a10sr-hwmon";
	}

Thanks for reviewing and helping me figure out the device tree naming.

Re: [RFC 1/8] dt-bindings: mfd: Add Altera Arria10 System Resource Chip bindings

[Lee Jones]

On Mon, 18 Apr 2016, Thor Thayer wrote:

> Hi Lee,
> 
> On 04/18/2016 02:45 AM, Lee Jones wrote:
> >On Fri, 15 Apr 2016, Thor Thayer wrote:
> >>On 03/30/2016 06:35 AM, Lee Jones wrote:
> >>>On Tue, 29 Mar 2016, tthayer@opensource.altera.com wrote:
> >>>
> >>>>From: Thor Thayer <tthayer@opensource.altera.com>
> >>>>
> >>>>The Altera Arria10 Devkit System Resource chip is a Multi-Function
> >>>>Device, it has two subdevices:
> >>>>      - GPIO
> >>>>      - HWMON
> >>>>
> >>>>This patch adds documentation for the Altera A10-SR DT bindings.
> >>>>
> >>>>Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
> >>>>---
> >>>>  .../devicetree/bindings/mfd/altera-a10sr.txt       |   35 ++++++++++++++++++++
> >>>>  1 file changed, 35 insertions(+)
> >>>>  create mode 100644 Documentation/devicetree/bindings/mfd/altera-a10sr.txt
> >>>>
> >>>>diff --git a/Documentation/devicetree/bindings/mfd/altera-a10sr.txt b/Documentation/devicetree/bindings/mfd/altera-a10sr.txt
> >>>>new file mode 100644
> >>>>index 0000000..564c761
> >>>>--- /dev/null
> >>>>+++ b/Documentation/devicetree/bindings/mfd/altera-a10sr.txt
> >>>>@@ -0,0 +1,35 @@
> >>>>+* Altera Arria10 Development Kit System Resource Chip
> >>>>+
> >>>>+Required parent device properties:
> >>>>+- compatible : "altr,altr_a10sr"
> >>>>+- spi-max-frequency : Maximum SPI frequency.
> >>>>+- reg : the SPI Chip Select address for the Arria10 System Resource chip
> >
> >[...]
> >
> >>>>+		a10sr_hwmon: a10sr_hwmon {
> >>>
> >>>Device type only please.
> >>>
> >>I need to revisit where this will live (hwmon does not seem to be
> >>the correct place) so it will change but I can follow the format
> >>above if it is correct.
> >
> >BTW, "hwmon" is a subsystem in Linux, therefore is a Linuxism and is
> >not allowed in DT.  What does the device *actually* do?
> >
> 
> OK. I'll be careful not to introduce the Linux subsystem name.
> 
> This module indicates whether the power supplies are at the correct
> voltage. It uses a boolean instead of giving an actual voltage value
> as required by HWMON. In other words it is a comparator instead of
> an Analog-to-Digital Converter.
> 
> I could call it a power supply supervisor or voltage status monitor
> but it only acts in a passive role. There is no output to trigger an
> error - only polling, so supervisor doesn't seem like a good name.
> 
> Maybe something like this?
> 
> 	power_supply_status {
> 		compatible = "altr,a10sr-hwmon";
> 	}
> 
> Thanks for reviewing and helping me figure out the device tree naming.

Does it have its own address space?  How complex is the device?  Not
very, by the sounds of it.  In which case, does it really need its own
driver?

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 1/8] dt-bindings: mfd: Add Altera Arria10 System Resource Chip bindings

[Thor Thayer]

On 04/19/2016 02:23 AM, Lee Jones wrote:
> On Mon, 18 Apr 2016, Thor Thayer wrote:
>
>> Hi Lee,
>>
>> On 04/18/2016 02:45 AM, Lee Jones wrote:
>>> On Fri, 15 Apr 2016, Thor Thayer wrote:
>>>> On 03/30/2016 06:35 AM, Lee Jones wrote:
>>>>> On Tue, 29 Mar 2016, tthayer@opensource.altera.com wrote:
>>>>>
>>>>>> From: Thor Thayer <tthayer@opensource.altera.com>
>>>>>>
>>>>>> The Altera Arria10 Devkit System Resource chip is a Multi-Function
>>>>>> Device, it has two subdevices:
>>>>>>       - GPIO
>>>>>>       - HWMON
>>>>>>
>>>>>> This patch adds documentation for the Altera A10-SR DT bindings.
>>>>>>
>>>>>> Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
>>>>>> ---
>>>>>>   .../devicetree/bindings/mfd/altera-a10sr.txt       |   35 ++++++++++++++++++++
>>>>>>   1 file changed, 35 insertions(+)
>>>>>>   create mode 100644 Documentation/devicetree/bindings/mfd/altera-a10sr.txt
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/mfd/altera-a10sr.txt b/Documentation/devicetree/bindings/mfd/altera-a10sr.txt
>>>>>> new file mode 100644
>>>>>> index 0000000..564c761
>>>>>> --- /dev/null
>>>>>> +++ b/Documentation/devicetree/bindings/mfd/altera-a10sr.txt
>>>>>> @@ -0,0 +1,35 @@
>>>>>> +* Altera Arria10 Development Kit System Resource Chip
>>>>>> +
>>>>>> +Required parent device properties:
>>>>>> +- compatible : "altr,altr_a10sr"
>>>>>> +- spi-max-frequency : Maximum SPI frequency.
>>>>>> +- reg : the SPI Chip Select address for the Arria10 System Resource chip
>>>
>>> [...]
>>>
>>>>>> +		a10sr_hwmon: a10sr_hwmon {
>>>>>
>>>>> Device type only please.
>>>>>
>>>> I need to revisit where this will live (hwmon does not seem to be
>>>> the correct place) so it will change but I can follow the format
>>>> above if it is correct.
>>>
>>> BTW, "hwmon" is a subsystem in Linux, therefore is a Linuxism and is
>>> not allowed in DT.  What does the device *actually* do?
>>>
>>
>> OK. I'll be careful not to introduce the Linux subsystem name.
>>
>> This module indicates whether the power supplies are at the correct
>> voltage. It uses a boolean instead of giving an actual voltage value
>> as required by HWMON. In other words it is a comparator instead of
>> an Analog-to-Digital Converter.
>>
>> I could call it a power supply supervisor or voltage status monitor
>> but it only acts in a passive role. There is no output to trigger an
>> error - only polling, so supervisor doesn't seem like a good name.
>>
>> Maybe something like this?
>>
>> 	power_supply_status {
>> 		compatible = "altr,a10sr-hwmon";
>> 	}
>>
>> Thanks for reviewing and helping me figure out the device tree naming.
>
> Does it have its own address space?  How complex is the device?  Not
> very, by the sounds of it.  In which case, does it really need its own
> driver?
>
Yes, you are correct that the voltage status is not very complex but I'd 
need a driver to expose these signals.

I initially started with an MFD because it was similar to the other MFD 
drivers. The device has GPI, GPO, voltage status, device enables, device 
present indications, and device resets.

There is a discussion now on where the voltage status driver should live 
(iio/ , hwmon/, misc/). It isn't clear to me where the device enables, 
device present indications and voltage status would go. I'm leaning 
toward a driver in the misc/ directory that would cover all of these. In 
that case, this wouldn't be a MFD driver.

Any thoughts or suggestions?

Thanks,

Thor

Re: [RFC 1/8] dt-bindings: mfd: Add Altera Arria10 System Resource Chip bindings

[Guenter Roeck]

On 04/19/2016 07:38 AM, Thor Thayer wrote:
>
>
> On 04/19/2016 02:23 AM, Lee Jones wrote:
>> On Mon, 18 Apr 2016, Thor Thayer wrote:
>>
>>> Hi Lee,
>>>
>>> On 04/18/2016 02:45 AM, Lee Jones wrote:
>>>> On Fri, 15 Apr 2016, Thor Thayer wrote:
>>>>> On 03/30/2016 06:35 AM, Lee Jones wrote:
>>>>>> On Tue, 29 Mar 2016, tthayer@opensource.altera.com wrote:
>>>>>>
>>>>>>> From: Thor Thayer <tthayer@opensource.altera.com>
>>>>>>>
>>>>>>> The Altera Arria10 Devkit System Resource chip is a Multi-Function
>>>>>>> Device, it has two subdevices:
>>>>>>>       - GPIO
>>>>>>>       - HWMON
>>>>>>>
>>>>>>> This patch adds documentation for the Altera A10-SR DT bindings.
>>>>>>>
>>>>>>> Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
>>>>>>> ---
>>>>>>>   .../devicetree/bindings/mfd/altera-a10sr.txt       |   35 ++++++++++++++++++++
>>>>>>>   1 file changed, 35 insertions(+)
>>>>>>>   create mode 100644 Documentation/devicetree/bindings/mfd/altera-a10sr.txt
>>>>>>>
>>>>>>> diff --git a/Documentation/devicetree/bindings/mfd/altera-a10sr.txt b/Documentation/devicetree/bindings/mfd/altera-a10sr.txt
>>>>>>> new file mode 100644
>>>>>>> index 0000000..564c761
>>>>>>> --- /dev/null
>>>>>>> +++ b/Documentation/devicetree/bindings/mfd/altera-a10sr.txt
>>>>>>> @@ -0,0 +1,35 @@
>>>>>>> +* Altera Arria10 Development Kit System Resource Chip
>>>>>>> +
>>>>>>> +Required parent device properties:
>>>>>>> +- compatible : "altr,altr_a10sr"
>>>>>>> +- spi-max-frequency : Maximum SPI frequency.
>>>>>>> +- reg : the SPI Chip Select address for the Arria10 System Resource chip
>>>>
>>>> [...]
>>>>
>>>>>>> +        a10sr_hwmon: a10sr_hwmon {
>>>>>>
>>>>>> Device type only please.
>>>>>>
>>>>> I need to revisit where this will live (hwmon does not seem to be
>>>>> the correct place) so it will change but I can follow the format
>>>>> above if it is correct.
>>>>
>>>> BTW, "hwmon" is a subsystem in Linux, therefore is a Linuxism and is
>>>> not allowed in DT.  What does the device *actually* do?
>>>>
>>>
>>> OK. I'll be careful not to introduce the Linux subsystem name.
>>>
>>> This module indicates whether the power supplies are at the correct
>>> voltage. It uses a boolean instead of giving an actual voltage value
>>> as required by HWMON. In other words it is a comparator instead of
>>> an Analog-to-Digital Converter.
>>>
>>> I could call it a power supply supervisor or voltage status monitor
>>> but it only acts in a passive role. There is no output to trigger an
>>> error - only polling, so supervisor doesn't seem like a good name.
>>>
>>> Maybe something like this?
>>>
>>>     power_supply_status {
>>>         compatible = "altr,a10sr-hwmon";
>>>     }
>>>
>>> Thanks for reviewing and helping me figure out the device tree naming.
>>
>> Does it have its own address space?  How complex is the device?  Not
>> very, by the sounds of it.  In which case, does it really need its own
>> driver?
>>
> Yes, you are correct that the voltage status is not very complex but I'd need a driver to expose these signals.
>

A completely different option might be to expose the signals as gpio pins.

Guenter

> I initially started with an MFD because it was similar to the other MFD drivers. The device has GPI, GPO, voltage status, device enables, device present indications, and device resets.
>
> There is a discussion now on where the voltage status driver should live (iio/ , hwmon/, misc/). It isn't clear to me where the device enables, device present indications and voltage status would go. I'm leaning toward a driver in the misc/ directory that would cover all of these. In that case, this wouldn't be a MFD driver.
>
> Any thoughts or suggestions?
>
> Thanks,
>
> Thor
>

[RFC 2/8] MAINTAINERS: Addition of Altera Arria10 System Resource Chip

[tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx]

From: Thor Thayer <tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>

Add maintainer for the Altera Arria10 files.

Signed-off-by: Thor Thayer <tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>
---
 MAINTAINERS |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 972f25d..567ce60 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -608,6 +608,14 @@ L:	linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
 S:	Maintained
 F:	drivers/gpio/gpio-altera.c
 
+ALTERA SYSTEM RESOURCE DRIVER FOR ARRIA10 DEVKIT
+M:	Thor Thayer <tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>
+S:	Maintained
+F:	drivers/mfd/altera-a10sr.c
+F:	include/linux/mfd/altera-a10sr.h
+F:	drivers/gpio/gpio-altera-a10sr.c
+F:	drivers/hwmon/altera-a10sr-hwmon.c
+
 ALTERA TRIPLE SPEED ETHERNET DRIVER
 M:	Vince Bridgers <vbridger-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>
 L:	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 2/8] MAINTAINERS: Addition of Altera Arria10 System Resource Chip

[Lee Jones]

On Tue, 29 Mar 2016, tthayer@opensource.altera.com wrote:

> From: Thor Thayer <tthayer@opensource.altera.com>
> 
> Add maintainer for the Altera Arria10 files.
> 
> Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
> ---
>  MAINTAINERS |    8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 972f25d..567ce60 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -608,6 +608,14 @@ L:	linux-gpio@vger.kernel.org
>  S:	Maintained
>  F:	drivers/gpio/gpio-altera.c
>  
> +ALTERA SYSTEM RESOURCE DRIVER FOR ARRIA10 DEVKIT
> +M:	Thor Thayer <tthayer@opensource.altera.com>
> +S:	Maintained
> +F:	drivers/mfd/altera-a10sr.c
> +F:	include/linux/mfd/altera-a10sr.h
> +F:	drivers/gpio/gpio-altera-a10sr.c
> +F:	drivers/hwmon/altera-a10sr-hwmon.c

Alphabetical makes more sense.

>  ALTERA TRIPLE SPEED ETHERNET DRIVER
>  M:	Vince Bridgers <vbridger@opensource.altera.com>
>  L:	netdev@vger.kernel.org

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC 3/8] mfd: altr_a10sr: Add Altera Arria10 DevKit System Resource Chip

[tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx]

From: Thor Thayer <tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>

Add support for the Altera Arria10 Development Kit System Resource
chip which is implemented using a MAX5 as a external gpio extender,
and hardware monitor with the regmap framework over a SPI bus.

Signed-off-by: Thor Thayer <tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>
---
 drivers/mfd/Kconfig              |   11 +++
 drivers/mfd/Makefile             |    2 +
 drivers/mfd/altera-a10sr.c       |  177 ++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/altera-a10sr.h |  146 +++++++++++++++++++++++++++++++
 4 files changed, 336 insertions(+)
 create mode 100644 drivers/mfd/altera-a10sr.c
 create mode 100644 include/linux/mfd/altera-a10sr.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index eea61e3..d1edf81 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -18,6 +18,17 @@ config MFD_CS5535
 	  This is the core driver for CS5535/CS5536 MFD functions.  This is
           necessary for using the board's GPIO and MFGPT functionality.
 
+config MFD_ALTERA_A10SR
+       bool "Altera Arria10 DevKit System Resource chip"
+       depends on ARCH_SOCFPGA && SPI_MASTER=y
+       select REGMAP_SPI
+       select MFD_CORE
+       help
+         Support for the Altera Arria10 DevKit MAX5 System Resource chip
+         using the SPI interface. This driver provides common support for
+         accessing the external gpio extender (LEDs & buttons) and
+         hw monitor.
+
 config MFD_ACT8945A
 	tristate "Active-semi ACT8945A"
 	select MFD_CORE
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 5eaa6465d..4f1ff91 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -203,3 +203,5 @@ intel-soc-pmic-objs		:= intel_soc_pmic_core.o intel_soc_pmic_crc.o
 intel-soc-pmic-$(CONFIG_INTEL_PMC_IPC)	+= intel_soc_pmic_bxtwc.o
 obj-$(CONFIG_INTEL_SOC_PMIC)	+= intel-soc-pmic.o
 obj-$(CONFIG_MFD_MT6397)	+= mt6397-core.o
+
+obj-$(CONFIG_MFD_ALTERA_A10SR)	+= altera-a10sr.o
diff --git a/drivers/mfd/altera-a10sr.c b/drivers/mfd/altera-a10sr.c
new file mode 100644
index 0000000..13665d4
--- /dev/null
+++ b/drivers/mfd/altera-a10sr.c
@@ -0,0 +1,177 @@
+/*
+ * Copyright Altera Corporation (C) 2014-2016. All Rights Reserved
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * SPI access for Altera Arria10 MAX5 System Resource Chip
+ *
+ * Adapted from DA9052
+ * Copyright(c) 2011 Dialog Semiconductor Ltd.
+ * Author: David Dajun Chen <dchen-WBD+wuPFNBhBDgjK7y7TUQ@public.gmane.org>
+ */
+
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/input.h>
+#include <linux/mfd/altera-a10sr.h>
+#include <linux/mfd/core.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/spi/spi.h>
+
+static const struct mfd_cell altr_a10sr_subdev_info[] = {
+};
+
+static bool altr_a10sr_reg_readable(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case ALTR_A10SR_VERSION_READ:
+	case ALTR_A10SR_LED_RD_REG:
+	case ALTR_A10SR_PBDSW_RD_REG:
+	case ALTR_A10SR_PBDSW_IRQ_CLR_REG:
+	case ALTR_A10SR_PBDSW_IRQ_RD_REG:
+	case ALTR_A10SR_PWR_GOOD1_RD_REG:
+	case ALTR_A10SR_PWR_GOOD2_RD_REG:
+	case ALTR_A10SR_PWR_GOOD3_RD_REG:
+	case ALTR_A10SR_FMCAB_RD_REG:
+	case ALTR_A10SR_HPS_RST_RD_REG:
+	case ALTR_A10SR_USB_QSPI_RD_REG:
+	case ALTR_A10SR_SFPA_RD_REG:
+	case ALTR_A10SR_SFPB_RD_REG:
+	case ALTR_A10SR_I2C_M_RD_REG:
+	case ALTR_A10SR_WARM_RST_RD_REG:
+	case ALTR_A10SR_WR_KEY_RD_REG:
+	case ALTR_A10SR_PMBUS_RD_REG:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static bool altr_a10sr_reg_writeable(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case ALTR_A10SR_LED_WR_REG:
+	case ALTR_A10SR_PBDSW_IRQ_CLR_REG:
+	case ALTR_A10SR_FMCAB_WR_REG:
+	case ALTR_A10SR_HPS_RST_WR_REG:
+	case ALTR_A10SR_USB_QSPI_WR_REG:
+	case ALTR_A10SR_SFPA_WR_REG:
+	case ALTR_A10SR_SFPB_WR_REG:
+	case ALTR_A10SR_WARM_RST_WR_REG:
+	case ALTR_A10SR_WR_KEY_WR_REG:
+	case ALTR_A10SR_PMBUS_WR_REG:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static bool altr_a10sr_reg_volatile(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case ALTR_A10SR_PBDSW_RD_REG:
+	case ALTR_A10SR_PBDSW_IRQ_RD_REG:
+	case ALTR_A10SR_PWR_GOOD1_RD_REG:
+	case ALTR_A10SR_PWR_GOOD2_RD_REG:
+	case ALTR_A10SR_PWR_GOOD3_RD_REG:
+	case ALTR_A10SR_HPS_RST_RD_REG:
+	case ALTR_A10SR_I2C_M_RD_REG:
+	case ALTR_A10SR_WARM_RST_RD_REG:
+	case ALTR_A10SR_WR_KEY_RD_REG:
+	case ALTR_A10SR_PMBUS_RD_REG:
+		return true;
+	default:
+		return false;
+	}
+}
+
+const struct regmap_config altr_a10sr_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+
+	.cache_type = REGCACHE_NONE,
+
+	.use_single_rw = true,
+	.read_flag_mask = 1,
+	.write_flag_mask = 0,
+
+	.max_register = ALTR_A10SR_WR_KEY_RD_REG,
+	.readable_reg = altr_a10sr_reg_readable,
+	.writeable_reg = altr_a10sr_reg_writeable,
+	.volatile_reg = altr_a10sr_reg_volatile,
+
+};
+
+static int altr_a10sr_spi_probe(struct spi_device *spi)
+{
+	int ret;
+	struct altr_a10sr *a10sr;
+
+	a10sr = devm_kzalloc(&spi->dev, sizeof(*a10sr),
+			     GFP_KERNEL);
+	if (!a10sr)
+		return -ENOMEM;
+
+	spi->mode = SPI_MODE_3;
+	spi->bits_per_word = 8;
+	spi_setup(spi);
+
+	a10sr->dev = &spi->dev;
+
+	spi_set_drvdata(spi, a10sr);
+
+	a10sr->regmap = devm_regmap_init_spi(spi, &altr_a10sr_regmap_config);
+	if (IS_ERR(a10sr->regmap)) {
+		ret = PTR_ERR(a10sr->regmap);
+		dev_err(&spi->dev, "Allocate register map Failed: %d\n", ret);
+		return ret;
+	}
+
+	ret = mfd_add_devices(a10sr->dev, PLATFORM_DEVID_AUTO,
+			      altr_a10sr_subdev_info,
+			      ARRAY_SIZE(altr_a10sr_subdev_info),
+			      NULL, 0, NULL);
+	if (ret)
+		dev_err(a10sr->dev, "mfd_add_devices failed: %d\n", ret);
+
+	return ret;
+}
+
+static int altr_a10sr_spi_remove(struct spi_device *spi)
+{
+	mfd_remove_devices(&spi->dev);
+
+	return 0;
+}
+
+static const struct of_device_id altr_a10sr_spi_of_match[] = {
+	{ .compatible = "altr,altr-a10sr" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, altr_a10sr_spi_of_match);
+
+static struct spi_driver altr_a10sr_spi_driver = {
+	.probe = altr_a10sr_spi_probe,
+	.remove = altr_a10sr_spi_remove,
+	.driver = {
+		.name = "altr_a10sr",
+		.of_match_table = altr_a10sr_spi_of_match,
+	},
+};
+
+module_spi_driver(altr_a10sr_spi_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Thor Thayer");
+MODULE_DESCRIPTION("Altera Arria10 DevKit System Resource MFD Driver");
diff --git a/include/linux/mfd/altera-a10sr.h b/include/linux/mfd/altera-a10sr.h
new file mode 100644
index 0000000..7087afc
--- /dev/null
+++ b/include/linux/mfd/altera-a10sr.h
@@ -0,0 +1,146 @@
+/*
+ * Copyright Altera Corporation (C) 2014-2016. All Rights Reserved
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Declarations for Altera Arria10 MAX5 System Resource Chip
+ *
+ * Adapted from DA9052
+ * Copyright(c) 2011 Dialog Semiconductor Ltd.
+ * Author: David Dajun Chen <dchen-WBD+wuPFNBhBDgjK7y7TUQ@public.gmane.org>
+ */
+
+#ifndef __MFD_ALTERA_A10SR_H
+#define __MFD_ALTERA_A10SR_H
+
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <linux/completion.h>
+#include <linux/list.h>
+#include <linux/mfd/core.h>
+
+/* Write registers are always on even addresses */
+#define  WRITE_REG_MASK              0xFE
+/* Odd registers are always on odd addresses */
+#define  READ_REG_MASK               0x01
+
+#define ALTR_A10SR_BITS_PER_REGISTER  8
+/*
+ * To find the correct register, we divide the input GPIO by
+ * the number of GPIO in each register. We then need to multiply
+ * by 2 because the reads are at odd addresses.
+ */
+#define ALTR_A10SR_REG_OFFSET(X)     (((X) / ALTR_A10SR_BITS_PER_REGISTER) << 1)
+#define ALTR_A10SR_REG_BIT(X)        ((X) % ALTR_A10SR_BITS_PER_REGISTER)
+#define ALTR_A10SR_REG_BIT_CHG(X, Y) ((X) << ALTR_A10SR_REG_BIT(Y))
+#define ALTR_A10SR_REG_BIT_MASK(X)   (1 << ALTR_A10SR_REG_BIT(X))
+
+/* Arria10 System Controller Register Defines */
+#define ALTR_A10SR_NOP                0x00    /* No Change */
+#define ALTR_A10SR_VERSION_READ       0x01    /* MAX5 Version Read */
+#define ALTR_A10SR_LED_WR_REG         0x02    /* LED - Upper 4 bits */
+#define ALTR_A10SR_LED_RD_REG         0x03    /* LED - Upper 4 bits */
+#define ALTR_A10SR_PBDSW_RD_REG       0x05    /* PB & DIP SW - Input only */
+#define ALTR_A10SR_PBDSW_IRQ_CLR_REG  0x06    /* PB & DIP SW Flag Clear */
+#define ALTR_A10SR_PBDSW_IRQ_RD_REG   0x07    /* PB & DIP SW Flag Read */
+#define ALTR_A10SR_PWR_GOOD1_RD_REG   0x09    /* Power Good1 Read */
+#define ALTR_A10SR_PWR_GOOD2_RD_REG   0x0B    /* Power Good2 Read */
+#define ALTR_A10SR_PWR_GOOD3_RD_REG   0x0D    /* Power Good3 Read */
+#define ALTR_A10SR_FMCAB_WR_REG       0x0E    /* FMCA/B & PCIe Pwr Enable */
+#define ALTR_A10SR_FMCAB_RD_REG       0x0F    /* FMCA/B & PCIe Pwr Enable */
+#define ALTR_A10SR_HPS_RST_WR_REG     0x10    /* HPS Reset */
+#define ALTR_A10SR_HPS_RST_RD_REG     0x11    /* HPS Reset */
+#define ALTR_A10SR_USB_QSPI_WR_REG    0x12    /* USB, BQSPI, FILE Reset */
+#define ALTR_A10SR_USB_QSPI_RD_REG    0x13    /* USB, BQSPI, FILE Reset */
+#define ALTR_A10SR_SFPA_WR_REG        0x14    /* SFPA Control Reg */
+#define ALTR_A10SR_SFPA_RD_REG        0x15    /* SFPA Control Reg */
+#define ALTR_A10SR_SFPB_WR_REG        0x16    /* SFPB Control Reg */
+#define ALTR_A10SR_SFPB_RD_REG        0x17    /* SFPB Control Reg */
+#define ALTR_A10SR_I2C_M_RD_REG       0x19    /* I2C Master Select */
+#define ALTR_A10SR_WARM_RST_WR_REG    0x1A    /* HPS Warm Reset */
+#define ALTR_A10SR_WARM_RST_RD_REG    0x1B    /* HPS Warm Reset */
+#define ALTR_A10SR_WR_KEY_WR_REG      0x1C    /* HPS Warm Reset Key */
+#define ALTR_A10SR_WR_KEY_RD_REG      0x1D    /* HPS Warm Reset Key */
+#define ALTR_A10SR_PMBUS_WR_REG       0x1E    /* HPS PM Bus */
+#define ALTR_A10SR_PMBUS_RD_REG       0x1F    /* HPS PM Bus */
+
+struct altr_a10sr {
+	struct device *dev;
+	struct regmap *regmap;
+};
+
+/* Device I/O API */
+static inline int altr_a10sr_reg_read(struct altr_a10sr *a10sr,
+				      unsigned char reg)
+{
+	int val, ret;
+
+	ret = regmap_read(a10sr->regmap, (reg | READ_REG_MASK), &val);
+	if (ret < 0)
+		return ret;
+
+	return val;
+}
+
+static inline int altr_a10sr_reg_write(struct altr_a10sr *a10sr,
+				       unsigned char reg, unsigned char val)
+{
+	int ret;
+
+	ret = regmap_write(a10sr->regmap, (reg & WRITE_REG_MASK), val);
+	if (ret < 0)
+		return ret;
+
+	return ret;
+}
+
+static inline int altr_a10sr_group_read(struct altr_a10sr *a10sr,
+					unsigned char reg,
+					unsigned int reg_cnt,
+					unsigned char *val)
+{
+	return regmap_bulk_read(a10sr->regmap, reg, val, reg_cnt);
+}
+
+static inline int altr_a10sr_group_write(struct altr_a10sr *a10sr,
+					 unsigned char reg,
+					 unsigned int reg_cnt,
+					 unsigned char *val)
+{
+	return regmap_bulk_write(a10sr->regmap, reg, val, reg_cnt);
+}
+
+static inline int altr_a10sr_reg_update(struct altr_a10sr *a10sr,
+					unsigned char reg,
+					unsigned char bit_mask,
+					unsigned char reg_val)
+{
+	int rval, ret;
+
+	/*
+	 * We can't use the standard regmap_update_bits function because
+	 * the read register has a different address than the write register.
+	 * Therefore, just do a read, modify, write operation here.
+	 */
+	ret = regmap_read(a10sr->regmap, (reg | READ_REG_MASK), &rval);
+	if (ret < 0)
+		return ret;
+
+	rval = ((rval & ~bit_mask) | (reg_val & bit_mask));
+
+	ret = regmap_write(a10sr->regmap, (reg & WRITE_REG_MASK), rval);
+
+	return ret;
+}
+
+#endif /* __MFD_ALTERA_A10SR_H */
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 3/8] mfd: altr_a10sr: Add Altera Arria10 DevKit System Resource Chip

[Lee Jones]

On Tue, 29 Mar 2016, tthayer@opensource.altera.com wrote:

> From: Thor Thayer <tthayer@opensource.altera.com>
> 
> Add support for the Altera Arria10 Development Kit System Resource
> chip which is implemented using a MAX5 as a external gpio extender,
> and hardware monitor with the regmap framework over a SPI bus.
> 
> Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
> ---
>  drivers/mfd/Kconfig              |   11 +++
>  drivers/mfd/Makefile             |    2 +
>  drivers/mfd/altera-a10sr.c       |  177 ++++++++++++++++++++++++++++++++++++++
>  include/linux/mfd/altera-a10sr.h |  146 +++++++++++++++++++++++++++++++
>  4 files changed, 336 insertions(+)
>  create mode 100644 drivers/mfd/altera-a10sr.c
>  create mode 100644 include/linux/mfd/altera-a10sr.h
> 
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index eea61e3..d1edf81 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -18,6 +18,17 @@ config MFD_CS5535
>  	  This is the core driver for CS5535/CS5536 MFD functions.  This is
>            necessary for using the board's GPIO and MFGPT functionality.
>  
> +config MFD_ALTERA_A10SR
> +       bool "Altera Arria10 DevKit System Resource chip"
> +       depends on ARCH_SOCFPGA && SPI_MASTER=y

Depends on OF ?

> +       select REGMAP_SPI
> +       select MFD_CORE
> +       help
> +         Support for the Altera Arria10 DevKit MAX5 System Resource chip
> +         using the SPI interface. This driver provides common support for
> +         accessing the external gpio extender (LEDs & buttons) and
> +         hw monitor.
> +
>  config MFD_ACT8945A
>  	tristate "Active-semi ACT8945A"
>  	select MFD_CORE
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index 5eaa6465d..4f1ff91 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -203,3 +203,5 @@ intel-soc-pmic-objs		:= intel_soc_pmic_core.o intel_soc_pmic_crc.o
>  intel-soc-pmic-$(CONFIG_INTEL_PMC_IPC)	+= intel_soc_pmic_bxtwc.o
>  obj-$(CONFIG_INTEL_SOC_PMIC)	+= intel-soc-pmic.o
>  obj-$(CONFIG_MFD_MT6397)	+= mt6397-core.o
> +
> +obj-$(CONFIG_MFD_ALTERA_A10SR)	+= altera-a10sr.o
> diff --git a/drivers/mfd/altera-a10sr.c b/drivers/mfd/altera-a10sr.c
> new file mode 100644
> index 0000000..13665d4
> --- /dev/null
> +++ b/drivers/mfd/altera-a10sr.c
> @@ -0,0 +1,177 @@
> +/*
> + * Copyright Altera Corporation (C) 2014-2016. All Rights Reserved
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program.  If not, see <http://www.gnu.org/licenses/>.

Any chance you can use the shorter copyright header?

> + * SPI access for Altera Arria10 MAX5 System Resource Chip
> + *
> + * Adapted from DA9052
> + * Copyright(c) 2011 Dialog Semiconductor Ltd.
> + * Author: David Dajun Chen <dchen@diasemi.com>
> + */
> +
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/input.h>
> +#include <linux/mfd/altera-a10sr.h>
> +#include <linux/mfd/core.h>
> +#include <linux/module.h>
> +#include <linux/of_platform.h>
> +#include <linux/spi/spi.h>
> +
> +static const struct mfd_cell altr_a10sr_subdev_info[] = {
> +};

Eh?  What's the point of this?

> +static bool altr_a10sr_reg_readable(struct device *dev, unsigned int reg)
> +{
> +	switch (reg) {
> +	case ALTR_A10SR_VERSION_READ:
> +	case ALTR_A10SR_LED_RD_REG:
> +	case ALTR_A10SR_PBDSW_RD_REG:
> +	case ALTR_A10SR_PBDSW_IRQ_CLR_REG:
> +	case ALTR_A10SR_PBDSW_IRQ_RD_REG:
> +	case ALTR_A10SR_PWR_GOOD1_RD_REG:
> +	case ALTR_A10SR_PWR_GOOD2_RD_REG:
> +	case ALTR_A10SR_PWR_GOOD3_RD_REG:
> +	case ALTR_A10SR_FMCAB_RD_REG:
> +	case ALTR_A10SR_HPS_RST_RD_REG:
> +	case ALTR_A10SR_USB_QSPI_RD_REG:
> +	case ALTR_A10SR_SFPA_RD_REG:
> +	case ALTR_A10SR_SFPB_RD_REG:
> +	case ALTR_A10SR_I2C_M_RD_REG:
> +	case ALTR_A10SR_WARM_RST_RD_REG:
> +	case ALTR_A10SR_WR_KEY_RD_REG:
> +	case ALTR_A10SR_PMBUS_RD_REG:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
> +static bool altr_a10sr_reg_writeable(struct device *dev, unsigned int reg)
> +{
> +	switch (reg) {
> +	case ALTR_A10SR_LED_WR_REG:
> +	case ALTR_A10SR_PBDSW_IRQ_CLR_REG:
> +	case ALTR_A10SR_FMCAB_WR_REG:
> +	case ALTR_A10SR_HPS_RST_WR_REG:
> +	case ALTR_A10SR_USB_QSPI_WR_REG:
> +	case ALTR_A10SR_SFPA_WR_REG:
> +	case ALTR_A10SR_SFPB_WR_REG:
> +	case ALTR_A10SR_WARM_RST_WR_REG:
> +	case ALTR_A10SR_WR_KEY_WR_REG:
> +	case ALTR_A10SR_PMBUS_WR_REG:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
> +static bool altr_a10sr_reg_volatile(struct device *dev, unsigned int reg)
> +{
> +	switch (reg) {
> +	case ALTR_A10SR_PBDSW_RD_REG:
> +	case ALTR_A10SR_PBDSW_IRQ_RD_REG:
> +	case ALTR_A10SR_PWR_GOOD1_RD_REG:
> +	case ALTR_A10SR_PWR_GOOD2_RD_REG:
> +	case ALTR_A10SR_PWR_GOOD3_RD_REG:
> +	case ALTR_A10SR_HPS_RST_RD_REG:
> +	case ALTR_A10SR_I2C_M_RD_REG:
> +	case ALTR_A10SR_WARM_RST_RD_REG:
> +	case ALTR_A10SR_WR_KEY_RD_REG:
> +	case ALTR_A10SR_PMBUS_RD_REG:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
> +const struct regmap_config altr_a10sr_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +
> +	.cache_type = REGCACHE_NONE,
> +
> +	.use_single_rw = true,
> +	.read_flag_mask = 1,
> +	.write_flag_mask = 0,
> +
> +	.max_register = ALTR_A10SR_WR_KEY_RD_REG,
> +	.readable_reg = altr_a10sr_reg_readable,
> +	.writeable_reg = altr_a10sr_reg_writeable,
> +	.volatile_reg = altr_a10sr_reg_volatile,
> +
> +};
> +
> +static int altr_a10sr_spi_probe(struct spi_device *spi)
> +{
> +	int ret;
> +	struct altr_a10sr *a10sr;
> +
> +	a10sr = devm_kzalloc(&spi->dev, sizeof(*a10sr),
> +			     GFP_KERNEL);
> +	if (!a10sr)
> +		return -ENOMEM;
> +
> +	spi->mode = SPI_MODE_3;
> +	spi->bits_per_word = 8;
> +	spi_setup(spi);
> +
> +	a10sr->dev = &spi->dev;
> +
> +	spi_set_drvdata(spi, a10sr);
> +
> +	a10sr->regmap = devm_regmap_init_spi(spi, &altr_a10sr_regmap_config);
> +	if (IS_ERR(a10sr->regmap)) {
> +		ret = PTR_ERR(a10sr->regmap);
> +		dev_err(&spi->dev, "Allocate register map Failed: %d\n", ret);

Proper English would be better.

"Failed to allocate register map"

> +		return ret;
> +	}
> +
> +	ret = mfd_add_devices(a10sr->dev, PLATFORM_DEVID_AUTO,
> +			      altr_a10sr_subdev_info,
> +			      ARRAY_SIZE(altr_a10sr_subdev_info),
> +			      NULL, 0, NULL);

This call does, precisely, nothing.

> +	if (ret)
> +		dev_err(a10sr->dev, "mfd_add_devices failed: %d\n", ret);

Users don't care about function names.

"Failed to register sub-devices" would be better.

> +	return ret;
> +}
> +
> +static int altr_a10sr_spi_remove(struct spi_device *spi)
> +{
> +	mfd_remove_devices(&spi->dev);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id altr_a10sr_spi_of_match[] = {
> +	{ .compatible = "altr,altr-a10sr" },

I'm thinking that putting "altr" twice is unnecessary.

> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, altr_a10sr_spi_of_match);
> +
> +static struct spi_driver altr_a10sr_spi_driver = {
> +	.probe = altr_a10sr_spi_probe,
> +	.remove = altr_a10sr_spi_remove,
> +	.driver = {
> +		.name = "altr_a10sr",
> +		.of_match_table = altr_a10sr_spi_of_match,

of_match_ptr()?

> +	},
> +};
> +
> +module_spi_driver(altr_a10sr_spi_driver);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR("Thor Thayer");

Email.

> +MODULE_DESCRIPTION("Altera Arria10 DevKit System Resource MFD Driver");
> diff --git a/include/linux/mfd/altera-a10sr.h b/include/linux/mfd/altera-a10sr.h
> new file mode 100644
> index 0000000..7087afc
> --- /dev/null
> +++ b/include/linux/mfd/altera-a10sr.h
> @@ -0,0 +1,146 @@
> +/*
> + * Copyright Altera Corporation (C) 2014-2016. All Rights Reserved
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program.  If not, see <http://www.gnu.org/licenses/>.

Shorter Copyright?

> + * Declarations for Altera Arria10 MAX5 System Resource Chip
> + *
> + * Adapted from DA9052
> + * Copyright(c) 2011 Dialog Semiconductor Ltd.
> + * Author: David Dajun Chen <dchen@diasemi.com>
> + */
> +
> +#ifndef __MFD_ALTERA_A10SR_H
> +#define __MFD_ALTERA_A10SR_H
> +
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +#include <linux/completion.h>
> +#include <linux/list.h>
> +#include <linux/mfd/core.h>

Alphabetical please.

> +/* Write registers are always on even addresses */
> +#define  WRITE_REG_MASK              0xFE
> +/* Odd registers are always on odd addresses */
> +#define  READ_REG_MASK               0x01
> +
> +#define ALTR_A10SR_BITS_PER_REGISTER  8
> +/*
> + * To find the correct register, we divide the input GPIO by
> + * the number of GPIO in each register. We then need to multiply
> + * by 2 because the reads are at odd addresses.
> + */
> +#define ALTR_A10SR_REG_OFFSET(X)     (((X) / ALTR_A10SR_BITS_PER_REGISTER) << 1)
> +#define ALTR_A10SR_REG_BIT(X)        ((X) % ALTR_A10SR_BITS_PER_REGISTER)
> +#define ALTR_A10SR_REG_BIT_CHG(X, Y) ((X) << ALTR_A10SR_REG_BIT(Y))
> +#define ALTR_A10SR_REG_BIT_MASK(X)   (1 << ALTR_A10SR_REG_BIT(X))
> +
> +/* Arria10 System Controller Register Defines */
> +#define ALTR_A10SR_NOP                0x00    /* No Change */
> +#define ALTR_A10SR_VERSION_READ       0x01    /* MAX5 Version Read */
> +#define ALTR_A10SR_LED_WR_REG         0x02    /* LED - Upper 4 bits */
> +#define ALTR_A10SR_LED_RD_REG         0x03    /* LED - Upper 4 bits */
> +#define ALTR_A10SR_PBDSW_RD_REG       0x05    /* PB & DIP SW - Input only */
> +#define ALTR_A10SR_PBDSW_IRQ_CLR_REG  0x06    /* PB & DIP SW Flag Clear */
> +#define ALTR_A10SR_PBDSW_IRQ_RD_REG   0x07    /* PB & DIP SW Flag Read */
> +#define ALTR_A10SR_PWR_GOOD1_RD_REG   0x09    /* Power Good1 Read */
> +#define ALTR_A10SR_PWR_GOOD2_RD_REG   0x0B    /* Power Good2 Read */
> +#define ALTR_A10SR_PWR_GOOD3_RD_REG   0x0D    /* Power Good3 Read */
> +#define ALTR_A10SR_FMCAB_WR_REG       0x0E    /* FMCA/B & PCIe Pwr Enable */
> +#define ALTR_A10SR_FMCAB_RD_REG       0x0F    /* FMCA/B & PCIe Pwr Enable */
> +#define ALTR_A10SR_HPS_RST_WR_REG     0x10    /* HPS Reset */
> +#define ALTR_A10SR_HPS_RST_RD_REG     0x11    /* HPS Reset */
> +#define ALTR_A10SR_USB_QSPI_WR_REG    0x12    /* USB, BQSPI, FILE Reset */
> +#define ALTR_A10SR_USB_QSPI_RD_REG    0x13    /* USB, BQSPI, FILE Reset */
> +#define ALTR_A10SR_SFPA_WR_REG        0x14    /* SFPA Control Reg */
> +#define ALTR_A10SR_SFPA_RD_REG        0x15    /* SFPA Control Reg */
> +#define ALTR_A10SR_SFPB_WR_REG        0x16    /* SFPB Control Reg */
> +#define ALTR_A10SR_SFPB_RD_REG        0x17    /* SFPB Control Reg */
> +#define ALTR_A10SR_I2C_M_RD_REG       0x19    /* I2C Master Select */
> +#define ALTR_A10SR_WARM_RST_WR_REG    0x1A    /* HPS Warm Reset */
> +#define ALTR_A10SR_WARM_RST_RD_REG    0x1B    /* HPS Warm Reset */
> +#define ALTR_A10SR_WR_KEY_WR_REG      0x1C    /* HPS Warm Reset Key */
> +#define ALTR_A10SR_WR_KEY_RD_REG      0x1D    /* HPS Warm Reset Key */
> +#define ALTR_A10SR_PMBUS_WR_REG       0x1E    /* HPS PM Bus */
> +#define ALTR_A10SR_PMBUS_RD_REG       0x1F    /* HPS PM Bus */
> +
> +struct altr_a10sr {
> +	struct device *dev;
> +	struct regmap *regmap;
> +};
> +
> +/* Device I/O API */
> +static inline int altr_a10sr_reg_read(struct altr_a10sr *a10sr,
> +				      unsigned char reg)
> +{
> +	int val, ret;
> +
> +	ret = regmap_read(a10sr->regmap, (reg | READ_REG_MASK), &val);
> +	if (ret < 0)
> +		return ret;
> +
> +	return val;
> +}
> +
> +static inline int altr_a10sr_reg_write(struct altr_a10sr *a10sr,
> +				       unsigned char reg, unsigned char val)
> +{
> +	int ret;
> +
> +	ret = regmap_write(a10sr->regmap, (reg & WRITE_REG_MASK), val);
> +	if (ret < 0)
> +		return ret;
> +
> +	return ret;
> +}
> +
> +static inline int altr_a10sr_group_read(struct altr_a10sr *a10sr,
> +					unsigned char reg,
> +					unsigned int reg_cnt,
> +					unsigned char *val)
> +{
> +	return regmap_bulk_read(a10sr->regmap, reg, val, reg_cnt);
> +}
> +
> +static inline int altr_a10sr_group_write(struct altr_a10sr *a10sr,
> +					 unsigned char reg,
> +					 unsigned int reg_cnt,
> +					 unsigned char *val)
> +{
> +	return regmap_bulk_write(a10sr->regmap, reg, val, reg_cnt);
> +}

All of the above are completely superfluous.  Just use the regmap_*
API directly.

> +static inline int altr_a10sr_reg_update(struct altr_a10sr *a10sr,
> +					unsigned char reg,
> +					unsigned char bit_mask,
> +					unsigned char reg_val)
> +{
> +	int rval, ret;
> +
> +	/*
> +	 * We can't use the standard regmap_update_bits function because
> +	 * the read register has a different address than the write register.
> +	 * Therefore, just do a read, modify, write operation here.
> +	 */
> +	ret = regmap_read(a10sr->regmap, (reg | READ_REG_MASK), &rval);
> +	if (ret < 0)
> +		return ret;
> +
> +	rval = ((rval & ~bit_mask) | (reg_val & bit_mask));
> +
> +	ret = regmap_write(a10sr->regmap, (reg & WRITE_REG_MASK), rval);
> +
> +	return ret;
> +}

Why can't you use the Regmap update function(s)?

> +#endif /* __MFD_ALTERA_A10SR_H */

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 3/8] mfd: altr_a10sr: Add Altera Arria10 DevKit System Resource Chip

[Thor Thayer]

Hi Lee,

On 03/30/2016 06:51 AM, Lee Jones wrote:
> On Tue, 29 Mar 2016, tthayer@opensource.altera.com wrote:
>
>> From: Thor Thayer <tthayer@opensource.altera.com>
>>
>> Add support for the Altera Arria10 Development Kit System Resource
>> chip which is implemented using a MAX5 as a external gpio extender,
>> and hardware monitor with the regmap framework over a SPI bus.
>>
>> Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
>> ---

[...]

>> index 0000000..13665d4
>> --- /dev/null
>> +++ b/drivers/mfd/altera-a10sr.c
>> @@ -0,0 +1,177 @@
>> +/*
>> + * Copyright Altera Corporation (C) 2014-2016. All Rights Reserved
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms and conditions of the GNU General Public License,
>> + * version 2, as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope it will be useful, but WITHOUT
>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
>> + * more details.
>> + *
>> + * You should have received a copy of the GNU General Public License along with
>> + * this program.  If not, see <http://www.gnu.org/licenses/>.
>
> Any chance you can use the shorter copyright header?
>

This is the header that Altera has specified and that we're operating 
under. We haven't received guidance on Intel's header yet but it may 
change as a result of our acquisition by Intel.

[...]

>> +
>> +	/*
>> +	 * We can't use the standard regmap_update_bits function because
>> +	 * the read register has a different address than the write register.
>> +	 * Therefore, just do a read, modify, write operation here.
>> +	 */
>> +	ret = regmap_read(a10sr->regmap, (reg | READ_REG_MASK), &rval);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	rval = ((rval & ~bit_mask) | (reg_val & bit_mask));
>> +
>> +	ret = regmap_write(a10sr->regmap, (reg & WRITE_REG_MASK), rval);
>> +
>> +	return ret;
>> +}
>
> Why can't you use the Regmap update function(s)?

The read register has a different address than the write register which 
is handled in this function with the masks (read address is odd, write 
address is even).

Thank you for the review of my patch set. I will implement the changes 
that you pointed out. Thank you for reviewing!

>
>> +#endif /* __MFD_ALTERA_A10SR_H */
>

Re: [RFC 3/8] mfd: altr_a10sr: Add Altera Arria10 DevKit System Resource Chip

[Lee Jones]

Mr Brown,

On Wed, 30 Mar 2016, Thor Thayer wrote:
> On 03/30/2016 06:51 AM, Lee Jones wrote:
> >On Tue, 29 Mar 2016, tthayer@opensource.altera.com wrote:
> >
> >>From: Thor Thayer <tthayer@opensource.altera.com>
> >>
> >>Add support for the Altera Arria10 Development Kit System Resource
> >>chip which is implemented using a MAX5 as a external gpio extender,
> >>and hardware monitor with the regmap framework over a SPI bus.
> >>
> >>Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
> >>---
> 
> [...]
> 
> >>index 0000000..13665d4
> >>--- /dev/null
> >>+++ b/drivers/mfd/altera-a10sr.c
> >>@@ -0,0 +1,177 @@
> >>+/*
> >>+ * Copyright Altera Corporation (C) 2014-2016. All Rights Reserved
> >>+ *
> >>+ * This program is free software; you can redistribute it and/or modify it
> >>+ * under the terms and conditions of the GNU General Public License,
> >>+ * version 2, as published by the Free Software Foundation.
> >>+ *
> >>+ * This program is distributed in the hope it will be useful, but WITHOUT
> >>+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> >>+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> >>+ * more details.
> >>+ *
> >>+ * You should have received a copy of the GNU General Public License along with
> >>+ * this program.  If not, see <http://www.gnu.org/licenses/>.
> >
> >Any chance you can use the shorter copyright header?
> >
> 
> This is the header that Altera has specified and that we're
> operating under. We haven't received guidance on Intel's header yet
> but it may change as a result of our acquisition by Intel.

Fair enough.

> >>+
> >>+	/*
> >>+	 * We can't use the standard regmap_update_bits function because
> >>+	 * the read register has a different address than the write register.
> >>+	 * Therefore, just do a read, modify, write operation here.
> >>+	 */
> >>+	ret = regmap_read(a10sr->regmap, (reg | READ_REG_MASK), &rval);
> >>+	if (ret < 0)
> >>+		return ret;
> >>+
> >>+	rval = ((rval & ~bit_mask) | (reg_val & bit_mask));
> >>+
> >>+	ret = regmap_write(a10sr->regmap, (reg & WRITE_REG_MASK), rval);
> >>+
> >>+	return ret;
> >>+}
> >
> >Why can't you use the Regmap update function(s)?
> 
> The read register has a different address than the write register
> which is handled in this function with the masks (read address is
> odd, write address is even).

Mark, do we have an API which handled such a configuration?

> Thank you for the review of my patch set. I will implement the
> changes that you pointed out. Thank you for reviewing!

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 3/8] mfd: altr_a10sr: Add Altera Arria10 DevKit System Resource Chip

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 525 bytes --]

On Wed, Mar 30, 2016 at 03:52:39PM +0100, Lee Jones wrote:
> On Wed, 30 Mar 2016, Thor Thayer wrote:

> > The read register has a different address than the write register
> > which is handled in this function with the masks (read address is
> > odd, write address is even).

> Mark, do we have an API which handled such a configuration?

No, but it sounds like this is a regmap with seven bit register values
and a read/write bit which should be using read_flag_mask rather than
trying to treat these as separate registers.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [RFC 3/8] mfd: altr_a10sr: Add Altera Arria10 DevKit System Resource Chip

[Lee Jones]

On Wed, 30 Mar 2016, Mark Brown wrote:
> On Wed, Mar 30, 2016 at 03:52:39PM +0100, Lee Jones wrote:
> > On Wed, 30 Mar 2016, Thor Thayer wrote:
> 
> > > The read register has a different address than the write register
> > > which is handled in this function with the masks (read address is
> > > odd, write address is even).
> 
> > Mark, do we have an API which handled such a configuration?
> 
> No, but it sounds like this is a regmap with seven bit register values
> and a read/write bit which should be using read_flag_mask rather than
> trying to treat these as separate registers.

Nice one.  Thanks Mark.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC 4/8] gpio: altera-a10sr: Add A10 System Resource Chip GPIO support.

[tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx]

From: Thor Thayer <tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>

Add the GPIO functionality for the Altera Arria10 MAX5 System Resource
Chip. The A10 MAX5 has 12 bits of GPIO assigned to switches, buttons,
and LEDs as a GPIO extender on the SPI bus.

Signed-off-by: Thor Thayer <tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>
---
 drivers/gpio/Kconfig             |    8 ++
 drivers/gpio/Makefile            |    1 +
 drivers/gpio/gpio-altera-a10sr.c |  158 ++++++++++++++++++++++++++++++++++++++
 drivers/mfd/altera-a10sr.c       |    4 +
 include/linux/mfd/altera-a10sr.h |   22 ++++++
 5 files changed, 193 insertions(+)
 create mode 100644 drivers/gpio/gpio-altera-a10sr.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 5f3429f..7ea2d8f 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -767,6 +767,14 @@ config GPIO_ADP5520
 	  This option enables support for on-chip GPIO found
 	  on Analog Devices ADP5520 PMICs.
 
+config GPIO_ALTERA_A10SR
+	tristate "Altera Arria10 System Resource GPIO"
+	depends on MFD_ALTERA_A10SR
+	help
+	  Driver for Arria10 Development Kit GPIO expansion which
+	  includes reads of pushbuttons and DIP switches as well
+	  as writes to LEDs.
+
 config GPIO_ARIZONA
 	tristate "Wolfson Microelectronics Arizona class devices"
 	depends on MFD_ARIZONA
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 1e0b74f..cc29464 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -21,6 +21,7 @@ obj-$(CONFIG_GPIO_ADNP)		+= gpio-adnp.o
 obj-$(CONFIG_GPIO_ADP5520)	+= gpio-adp5520.o
 obj-$(CONFIG_GPIO_ADP5588)	+= gpio-adp5588.o
 obj-$(CONFIG_GPIO_ALTERA)  	+= gpio-altera.o
+obj-$(CONFIG_GPIO_ALTERA_A10SR)	+= gpio-altera-a10sr.o
 obj-$(CONFIG_GPIO_AMD8111)	+= gpio-amd8111.o
 obj-$(CONFIG_GPIO_AMDPT)	+= gpio-amdpt.o
 obj-$(CONFIG_GPIO_ARIZONA)	+= gpio-arizona.o
diff --git a/drivers/gpio/gpio-altera-a10sr.c b/drivers/gpio/gpio-altera-a10sr.c
new file mode 100644
index 0000000..be5308b
--- /dev/null
+++ b/drivers/gpio/gpio-altera-a10sr.c
@@ -0,0 +1,158 @@
+/*
+ *  Copyright Altera Corporation (C) 2014-2016. All Rights Reserved
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * GPIO driver for  Altera Arria10 MAX5 System Resource Chip
+ *
+ * Adapted from gpio-da9052.c
+ * Copyright(c) 2011 Dialog Semiconductor Ltd.
+ * Author: David Dajun Chen <dchen-WBD+wuPFNBhBDgjK7y7TUQ@public.gmane.org>
+ */
+
+#include <linux/fs.h>
+#include <linux/gpio.h>
+#include <linux/mfd/altera-a10sr.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/seq_file.h>
+#include <linux/syscalls.h>
+#include <linux/uaccess.h>
+
+struct altr_a10sr_gpio {
+	struct altr_a10sr *a10sc;
+	struct gpio_chip gp;
+};
+
+static inline struct altr_a10sr_gpio *to_altr_a10sr_gpio(struct gpio_chip *chip)
+{
+	return container_of(chip, struct altr_a10sr_gpio, gp);
+}
+
+static int altr_a10sr_gpio_get(struct gpio_chip *gc, unsigned int nr)
+{
+	struct altr_a10sr_gpio *gpio = to_altr_a10sr_gpio(gc);
+	int ret;
+	unsigned char reg = ALTR_A10SR_LED_RD_REG + ALTR_A10SR_REG_OFFSET(nr);
+
+	ret = altr_a10sr_reg_read(gpio->a10sc, reg);
+
+	if (ret < 0)
+		return ret;
+
+	if (ret & (1 << ALTR_A10SR_REG_BIT(nr)))
+		return 1;
+
+	return 0;
+}
+
+static void altr_a10sr_gpio_set(struct gpio_chip *gc, unsigned int nr,
+				int value)
+{
+	struct altr_a10sr_gpio *gpio = to_altr_a10sr_gpio(gc);
+	int ret;
+	unsigned char reg = ALTR_A10SR_LED_WR_REG + ALTR_A10SR_REG_OFFSET(nr);
+
+	ret = altr_a10sr_reg_update(gpio->a10sc, reg,
+				    ALTR_A10SR_REG_BIT_MASK(nr),
+				    ALTR_A10SR_REG_BIT_CHG(value, nr));
+	if (ret != 0)
+		dev_err(gpio->a10sc->dev,
+			"Failed to update gpio reg : %d", ret);
+}
+
+static int altr_a10sr_gpio_direction_input(struct gpio_chip *gc,
+					   unsigned int nr)
+{
+	if ((nr >= ALTR_A10SR_IN_VALID_RANGE_LO) &&
+	    (nr <= ALTR_A10SR_IN_VALID_RANGE_HI))
+		return 0;
+	return -EINVAL;
+}
+
+static int altr_a10sr_gpio_direction_output(struct gpio_chip *gc,
+					    unsigned int nr, int value)
+{
+	if ((nr >= ALTR_A10SR_OUT_VALID_RANGE_LO) &&
+	    (nr <= ALTR_A10SR_OUT_VALID_RANGE_HI))
+		return 0;
+	return -EINVAL;
+}
+
+static struct gpio_chip altr_a10sr_gc = {
+	.label = "altr_a10sr_gpio",
+	.owner = THIS_MODULE,
+	.get = altr_a10sr_gpio_get,
+	.set = altr_a10sr_gpio_set,
+	.direction_input = altr_a10sr_gpio_direction_input,
+	.direction_output = altr_a10sr_gpio_direction_output,
+	.can_sleep = true,
+	.ngpio = 16,
+	.base = -1,
+};
+
+static int altr_a10sr_gpio_probe(struct platform_device *pdev)
+{
+	struct altr_a10sr_gpio *gpio;
+	int ret;
+
+	gpio = devm_kzalloc(&pdev->dev, sizeof(*gpio), GFP_KERNEL);
+	if (!gpio)
+		return -ENOMEM;
+
+	gpio->a10sc = dev_get_drvdata(pdev->dev.parent);
+
+	gpio->gp = altr_a10sr_gc;
+
+	gpio->gp.of_node = pdev->dev.of_node;
+
+	ret = gpiochip_add(&gpio->gp);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "Could not register gpiochip, %d\n", ret);
+		return ret;
+	}
+
+	platform_set_drvdata(pdev, gpio);
+
+	return 0;
+}
+
+static int altr_a10sr_gpio_remove(struct platform_device *pdev)
+{
+	struct altr_a10sr_gpio *gpio = platform_get_drvdata(pdev);
+
+	gpiochip_remove(&gpio->gp);
+
+	return 0;
+}
+
+static const struct of_device_id altr_a10sr_gpio_of_match[] = {
+	{ .compatible = "altr,a10sr-gpio" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, altr_a10sr_gpio_of_match);
+
+static struct platform_driver altr_a10sr_gpio_driver = {
+	.probe = altr_a10sr_gpio_probe,
+	.remove = altr_a10sr_gpio_remove,
+	.driver = {
+		.name	= "altr_a10sr_gpio",
+		.of_match_table = altr_a10sr_gpio_of_match,
+	},
+};
+
+module_platform_driver(altr_a10sr_gpio_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Thor Thayer");
+MODULE_DESCRIPTION("Altera Arria10 System Resource Chip GPIO");
diff --git a/drivers/mfd/altera-a10sr.c b/drivers/mfd/altera-a10sr.c
index 13665d4..517b895 100644
--- a/drivers/mfd/altera-a10sr.c
+++ b/drivers/mfd/altera-a10sr.c
@@ -30,6 +30,10 @@
 #include <linux/spi/spi.h>
 
 static const struct mfd_cell altr_a10sr_subdev_info[] = {
+	{
+		.name = "altr_a10sr_gpio",
+		.of_compatible = "altr,a10sr-gpio",
+	},
 };
 
 static bool altr_a10sr_reg_readable(struct device *dev, unsigned int reg)
diff --git a/include/linux/mfd/altera-a10sr.h b/include/linux/mfd/altera-a10sr.h
index 7087afc..6d254a1 100644
--- a/include/linux/mfd/altera-a10sr.h
+++ b/include/linux/mfd/altera-a10sr.h
@@ -50,9 +50,31 @@
 #define ALTR_A10SR_VERSION_READ       0x01    /* MAX5 Version Read */
 #define ALTR_A10SR_LED_WR_REG         0x02    /* LED - Upper 4 bits */
 #define ALTR_A10SR_LED_RD_REG         0x03    /* LED - Upper 4 bits */
+/* LED register Bit Definitions */
+#define ALTR_A10SR_LED_MASK               0xF0    /* LED - Mask Upper 4 bits */
+#define ALTR_A10SR_LED_VALID_SHIFT        4       /* LED - Upper 4 bits valid */
+#define ALTR_A10SR_LED_VALID_NUM          4       /* LED - # valid LEDs */
+#define ALTR_A10SR_OUT_VALID_RANGE_LO     ALTR_A10SR_LED_VALID_SHIFT
+#define ALTR_A10SR_OUT_VALID_RANGE_HI     7
+
 #define ALTR_A10SR_PBDSW_RD_REG       0x05    /* PB & DIP SW - Input only */
 #define ALTR_A10SR_PBDSW_IRQ_CLR_REG  0x06    /* PB & DIP SW Flag Clear */
 #define ALTR_A10SR_PBDSW_IRQ_RD_REG   0x07    /* PB & DIP SW Flag Read */
+/* Pushbutton & DIP Switch Bit Definitions */
+#define ALTR_A10SR_PB_DWS_PB_MASK         0xF0    /* PB - Upper 4 bits */
+#define ALTR_A10SR_PB_DWS_DWS_MASK        0x0F    /* DWS - Lower 4 bits */
+#define ALTR_A10SR_PB_VALID_NUM           4       /* # valid PB */
+#define ALTR_A10SR_IRQ_PB_3_SHIFT         7       /* Pushbutton 4 */
+#define ALTR_A10SR_IRQ_PB_2_SHIFT         6       /* Pushbutton 3 */
+#define ALTR_A10SR_IRQ_PB_1_SHIFT         5       /* Pushbutton 2 */
+#define ALTR_A10SR_IRQ_PB_0_SHIFT         4       /* Pushbutton 1 */
+#define ALTR_A10SR_IRQ_DSW_3_SHIFT        3       /* DIP SW 3 */
+#define ALTR_A10SR_IRQ_DSW_2_SHIFT        2       /* DIP SW 2 */
+#define ALTR_A10SR_IRQ_DSW_1_SHIFT        1       /* DIP SW 1 */
+#define ALTR_A10SR_IRQ_DSW_O_SHIFT        0       /* DIP SW 0 */
+#define ALTR_A10SR_IN_VALID_RANGE_LO      8
+#define ALTR_A10SR_IN_VALID_RANGE_HI      15
+
 #define ALTR_A10SR_PWR_GOOD1_RD_REG   0x09    /* Power Good1 Read */
 #define ALTR_A10SR_PWR_GOOD2_RD_REG   0x0B    /* Power Good2 Read */
 #define ALTR_A10SR_PWR_GOOD3_RD_REG   0x0D    /* Power Good3 Read */
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 4/8] gpio: altera-a10sr: Add A10 System Resource Chip GPIO support.

[Lee Jones]

On Tue, 29 Mar 2016, tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org wrote:

> From: Thor Thayer <tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>
> 
> Add the GPIO functionality for the Altera Arria10 MAX5 System Resource
> Chip. The A10 MAX5 has 12 bits of GPIO assigned to switches, buttons,
> and LEDs as a GPIO extender on the SPI bus.
> 
> Signed-off-by: Thor Thayer <tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>
> ---
>  drivers/gpio/Kconfig             |    8 ++
>  drivers/gpio/Makefile            |    1 +
>  drivers/gpio/gpio-altera-a10sr.c |  158 ++++++++++++++++++++++++++++++++++++++
>  drivers/mfd/altera-a10sr.c       |    4 +

Seperate patch please.

>  include/linux/mfd/altera-a10sr.h |   22 ++++++
>  5 files changed, 193 insertions(+)
>  create mode 100644 drivers/gpio/gpio-altera-a10sr.c

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 4/8] gpio: altera-a10sr: Add A10 System Resource Chip GPIO support.

[Linus Walleij]

On Tue, Mar 29, 2016 at 9:13 PM,  <tthayer@opensource.altera.com> wrote:

> From: Thor Thayer <tthayer@opensource.altera.com>
>
> Add the GPIO functionality for the Altera Arria10 MAX5 System Resource
> Chip. The A10 MAX5 has 12 bits of GPIO assigned to switches, buttons,
> and LEDs as a GPIO extender on the SPI bus.
>
> Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>

OK...

As Lee says: split off the MFD patch so it is a pure GPIO driver
patch.

> +#include <linux/gpio.h>

You should instead #include <linux/gpio/driver.h>

> +#include <linux/mfd/altera-a10sr.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/seq_file.h>
> +#include <linux/syscalls.h>
> +#include <linux/uaccess.h>

Syscalls and uaccess??? I don't think so.

> +struct altr_a10sr_gpio {
> +       struct altr_a10sr *a10sc;
> +       struct gpio_chip gp;
> +};

Add some kerneldoc.

> +static inline struct altr_a10sr_gpio *to_altr_a10sr_gpio(struct gpio_chip *chip)
> +{
> +       return container_of(chip, struct altr_a10sr_gpio, gp);
> +}

Don't use this old design pattern.

Use [devm_]gpiochip_add_data() and use gpiochip_get_data(gc) to get
a data pointer from the gpiochip.

> +static int altr_a10sr_gpio_get(struct gpio_chip *gc, unsigned int nr)
> +{
> +       struct altr_a10sr_gpio *gpio = to_altr_a10sr_gpio(gc);

So this becomes
struct altr_a10sr_gpio *gpio = gpiochip_get_data(gc);

> +       int ret;
> +       unsigned char reg = ALTR_A10SR_LED_RD_REG + ALTR_A10SR_REG_OFFSET(nr);
> +
> +       ret = altr_a10sr_reg_read(gpio->a10sc, reg);
> +
> +       if (ret < 0)
> +               return ret;
> +
> +       if (ret & (1 << ALTR_A10SR_REG_BIT(nr)))
> +               return 1;

Do this instead:

return !!(ret & (1 << ALTR_A10SR_REG_BIT(nr)))

It raises the question whether ALTR_A10SR_REG_BIT
is just a reimplementation of the BIT() macro from
<linux/bitops.h>, please check this.

> +static int altr_a10sr_gpio_direction_input(struct gpio_chip *gc,
> +                                          unsigned int nr)
> +{
> +       if ((nr >= ALTR_A10SR_IN_VALID_RANGE_LO) &&
> +           (nr <= ALTR_A10SR_IN_VALID_RANGE_HI))
> +               return 0;
> +       return -EINVAL;
> +}
> +
> +static int altr_a10sr_gpio_direction_output(struct gpio_chip *gc,
> +                                           unsigned int nr, int value)
> +{
> +       if ((nr >= ALTR_A10SR_OUT_VALID_RANGE_LO) &&
> +           (nr <= ALTR_A10SR_OUT_VALID_RANGE_HI))
> +               return 0;
> +       return -EINVAL;
> +}

Does this mean that all lines are *always* input and output
at the same time?

If there is no .set_direction() callback and all lines are both
input and output it kind of implies that all lines are also
implicitly open drain do you agree?

Please check:
- If there is really no direction setting anywhere
- For example if some lines are hardwired as input and
  some lines are hardwired as output
- If that is not the case, verify that all lines are really
  open drain, they should be if all are both input and
  output at the same time.

> +       ret = gpiochip_add(&gpio->gp);
> +       if (ret < 0) {
> +               dev_err(&pdev->dev, "Could not register gpiochip, %d\n", ret);
> +               return ret;
> +       }

Use devm_gpiochip_add_data() instead.

Yours,
Linus Walleij

Re: [RFC 4/8] gpio: altera-a10sr: Add A10 System Resource Chip GPIO support.

[Thor Thayer]

Hi Linus,

On 04/01/2016 07:17 AM, Linus Walleij wrote:
> On Tue, Mar 29, 2016 at 9:13 PM,  <tthayer@opensource.altera.com> wrote:
>
>> From: Thor Thayer <tthayer@opensource.altera.com>
>>
>> Add the GPIO functionality for the Altera Arria10 MAX5 System Resource
>> Chip. The A10 MAX5 has 12 bits of GPIO assigned to switches, buttons,
>> and LEDs as a GPIO extender on the SPI bus.
>>
>> Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
>
> OK...
>
> As Lee says: split off the MFD patch so it is a pure GPIO driver
> patch.
>

ACK

>> +#include <linux/gpio.h>
>
> You should instead #include <linux/gpio/driver.h>
>
>> +#include <linux/mfd/altera-a10sr.h>
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/seq_file.h>
>> +#include <linux/syscalls.h>
>> +#include <linux/uaccess.h>
>
> Syscalls and uaccess??? I don't think so.
>
OK.

>> +struct altr_a10sr_gpio {
>> +       struct altr_a10sr *a10sc;
>> +       struct gpio_chip gp;
>> +};
>
> Add some kerneldoc.

OK. To clarify, is this comment referring to the bindings document or 
something different?

>
>> +static inline struct altr_a10sr_gpio *to_altr_a10sr_gpio(struct gpio_chip *chip)
>> +{
>> +       return container_of(chip, struct altr_a10sr_gpio, gp);
>> +}
>
> Don't use this old design pattern.
>
> Use [devm_]gpiochip_add_data() and use gpiochip_get_data(gc) to get
> a data pointer from the gpiochip.
>
>> +static int altr_a10sr_gpio_get(struct gpio_chip *gc, unsigned int nr)
>> +{
>> +       struct altr_a10sr_gpio *gpio = to_altr_a10sr_gpio(gc);
>
> So this becomes
> struct altr_a10sr_gpio *gpio = gpiochip_get_data(gc);
>

Got it. Thanks!

>> +       int ret;
>> +       unsigned char reg = ALTR_A10SR_LED_RD_REG + ALTR_A10SR_REG_OFFSET(nr);
>> +
>> +       ret = altr_a10sr_reg_read(gpio->a10sc, reg);
>> +
>> +       if (ret < 0)
>> +               return ret;
>> +
>> +       if (ret & (1 << ALTR_A10SR_REG_BIT(nr)))
>> +               return 1;
>
> Do this instead:
>
> return !!(ret & (1 << ALTR_A10SR_REG_BIT(nr)))
>
> It raises the question whether ALTR_A10SR_REG_BIT
> is just a reimplementation of the BIT() macro from
> <linux/bitops.h>, please check this.
>

Got it. Yes, I will check that. Thanks.

>> +static int altr_a10sr_gpio_direction_input(struct gpio_chip *gc,
>> +                                          unsigned int nr)
>> +{
>> +       if ((nr >= ALTR_A10SR_IN_VALID_RANGE_LO) &&
>> +           (nr <= ALTR_A10SR_IN_VALID_RANGE_HI))
>> +               return 0;
>> +       return -EINVAL;
>> +}
>> +
>> +static int altr_a10sr_gpio_direction_output(struct gpio_chip *gc,
>> +                                           unsigned int nr, int value)
>> +{
>> +       if ((nr >= ALTR_A10SR_OUT_VALID_RANGE_LO) &&
>> +           (nr <= ALTR_A10SR_OUT_VALID_RANGE_HI))
>> +               return 0;
>> +       return -EINVAL;
>> +}
>
> Does this mean that all lines are *always* input and output
> at the same time?
>
> If there is no .set_direction() callback and all lines are both
> input and output it kind of implies that all lines are also
> implicitly open drain do you agree?
>
> Please check:
> - If there is really no direction setting anywhere
> - For example if some lines are hardwired as input and
>    some lines are hardwired as output
> - If that is not the case, verify that all lines are really
>    open drain, they should be if all are both input and
>    output at the same time.
>

I see your point. I'll investigate how to do this properly for your 2nd 
check above. Registers are hard-wired as input or output so I'll need to 
handle this properly and is why I didn't implement the .set_direction 
callback. Thanks for the explanation.

In my case, there are 12 valid GPIOs out of the 16 bits (the first 4 
bits are unused). Bits 4-7 are output and bits 8-15 are inputs. I was 
using the IN_VALID range for the inputs and the OUT_VALID range for the 
outputs.

>> +       ret = gpiochip_add(&gpio->gp);
>> +       if (ret < 0) {
>> +               dev_err(&pdev->dev, "Could not register gpiochip, %d\n", ret);
>> +               return ret;
>> +       }
>
> Use devm_gpiochip_add_data() instead.
>

OK. Thanks for reviewing!

> Yours,
> Linus Walleij
>

Re: [RFC 4/8] gpio: altera-a10sr: Add A10 System Resource Chip GPIO support.

[Linus Walleij]

On Fri, Apr 1, 2016 at 10:34 PM, Thor Thayer
<tthayer@opensource.altera.com> wrote:
> On 04/01/2016 07:17 AM, Linus Walleij wrote:

>>> +struct altr_a10sr_gpio {
>>> +       struct altr_a10sr *a10sc;
>>> +       struct gpio_chip gp;
>>> +};
>>
>> Add some kerneldoc.
>
> OK. To clarify, is this comment referring to the bindings document or
> something different?

Document the data structure.
Documentation/kernel-doc-nano-HOWTO.txt

>> Please check:
>> - If there is really no direction setting anywhere
>> - For example if some lines are hardwired as input and
>>    some lines are hardwired as output
>> - If that is not the case, verify that all lines are really
>>    open drain, they should be if all are both input and
>>    output at the same time.
>>
>
> I see your point. I'll investigate how to do this properly for your 2nd
> check above. Registers are hard-wired as input or output so I'll need to
> handle this properly and is why I didn't implement the .set_direction
> callback. Thanks for the explanation.
>
> In my case, there are 12 valid GPIOs out of the 16 bits (the first 4 bits
> are unused). Bits 4-7 are output and bits 8-15 are inputs. I was using the
> IN_VALID range for the inputs and the OUT_VALID range for the outputs.

It sounds like should implement direction_[input|output]() callbacks
and just return 0 if the user is asking for the hardwired direction and return
error if it tries to set an input-only to output and vice versa.

Yours,
Linus Walleij

[RFC 5/8] ARM: socfpga: dts: Add Devkit A10-SR fields for Arria10

[tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx]

From: Thor Thayer <tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>

Add the Altera Arria10 System Resource node. This is a Multi-Function
device with GPIO and HWMON support.

Signed-off-by: Thor Thayer <tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>
---
 arch/arm/boot/dts/socfpga_arria10.dtsi       |   15 +++++++++++++++
 arch/arm/boot/dts/socfpga_arria10_socdk.dtsi |   17 +++++++++++++++++
 2 files changed, 32 insertions(+)

diff --git a/arch/arm/boot/dts/socfpga_arria10.dtsi b/arch/arm/boot/dts/socfpga_arria10.dtsi
index 1c5e139..069b0a0 100644
--- a/arch/arm/boot/dts/socfpga_arria10.dtsi
+++ b/arch/arm/boot/dts/socfpga_arria10.dtsi
@@ -563,6 +563,21 @@
 			status = "disabled";
 		};
 
+		spi1: spi@ffda5000 {
+			compatible = "snps,dw-apb-ssi";
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <0xffda5000 0x100>;
+			interrupts = <0 102 4>;
+			num-chipselect = <4>;
+			bus-num = <0>;
+			/*32bit_access;*/
+			tx-dma-channel = <&pdma 16>;
+			rx-dma-channel = <&pdma 17>;
+			clocks = <&spi_m_clk>;
+			status = "disabled";
+		};
+
 		sdr: sdr@ffc25000 {
 			compatible = "syscon";
 			reg = <0xffcfb100 0x80>;
diff --git a/arch/arm/boot/dts/socfpga_arria10_socdk.dtsi b/arch/arm/boot/dts/socfpga_arria10_socdk.dtsi
index 567df98..095fd72 100644
--- a/arch/arm/boot/dts/socfpga_arria10_socdk.dtsi
+++ b/arch/arm/boot/dts/socfpga_arria10_socdk.dtsi
@@ -70,6 +70,23 @@
 	status = "okay";
 };
 
+&spi1 {
+	status = "okay";
+
+	a10_sysres: a10_sysres@0 {
+		compatible = "altr,altr-a10sr";
+		reg = <0>;
+		spi-max-frequency = <1000000>;
+
+		a10sr_gpio: a10sr_gpio {
+			compatible = "altr,a10sr-gpio";
+			gpio-controller;
+			#gpio-cells = <2>;
+			ngpios = <16>;
+		};
+	};
+};
+
 &i2c1 {
 	speed-mode = <0>;
 	status = "okay";
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 5/8] ARM: socfpga: dts: Add Devkit A10-SR fields for Arria10

[Dinh Nguyen]

On Tue, 29 Mar 2016, tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org wrote:

> From: Thor Thayer <tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>
> 
> Add the Altera Arria10 System Resource node. This is a Multi-Function
> device with GPIO and HWMON support.
> 
> Signed-off-by: Thor Thayer <tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>
> 
> +		spi1: spi@ffda5000 {
> +			compatible = "snps,dw-apb-ssi";
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			reg = <0xffda5000 0x100>;
> +			interrupts = <0 102 4>;
> +			num-chipselect = <4>;
> +			bus-num = <0>;
> +			/*32bit_access;*/
> +			tx-dma-channel = <&pdma 16>;
> +			rx-dma-channel = <&pdma 17>;
> +			clocks = <&spi_m_clk>;
> +			status = "disabled";
> +		};
> +

I think you need to split this into 2 patches, 1 for adding the SPI nodes, and
the 2nd for adding the slave device.

BR,
Dinh
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 5/8] ARM: socfpga: dts: Add Devkit A10-SR fields for Arria10

[Thor Thayer]

On 03/30/2016 12:42 PM, Dinh Nguyen wrote:
> On Tue, 29 Mar 2016, tthayer@opensource.altera.com wrote:
>
>> From: Thor Thayer <tthayer@opensource.altera.com>
>>
>> Add the Altera Arria10 System Resource node. This is a Multi-Function
>> device with GPIO and HWMON support.
>>
>> Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
>>
>> +		spi1: spi@ffda5000 {
>> +			compatible = "snps,dw-apb-ssi";
>> +			#address-cells = <1>;
>> +			#size-cells = <0>;
>> +			reg = <0xffda5000 0x100>;
>> +			interrupts = <0 102 4>;
>> +			num-chipselect = <4>;
>> +			bus-num = <0>;
>> +			/*32bit_access;*/
>> +			tx-dma-channel = <&pdma 16>;
>> +			rx-dma-channel = <&pdma 17>;
>> +			clocks = <&spi_m_clk>;
>> +			status = "disabled";
>> +		};
>> +
>
> I think you need to split this into 2 patches, 1 for adding the SPI nodes, and
> the 2nd for adding the slave device.
>
> BR,
> Dinh
>

OK. Thanks for reviewing.

[RFC 6/8] ARM: socfpga: dts: Add LED framework to A10-SR GPIO

[tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx]

From: Thor Thayer <tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>

Add the LED framework to the Arria10 System Resource chip GPIO hooks.

Signed-off-by: Thor Thayer <tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>
---
 arch/arm/boot/dts/socfpga_arria10_socdk.dtsi |   24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/arch/arm/boot/dts/socfpga_arria10_socdk.dtsi b/arch/arm/boot/dts/socfpga_arria10_socdk.dtsi
index 095fd72..fc0b7a0 100644
--- a/arch/arm/boot/dts/socfpga_arria10_socdk.dtsi
+++ b/arch/arm/boot/dts/socfpga_arria10_socdk.dtsi
@@ -31,6 +31,30 @@
 		reg = <0x0 0x40000000>; /* 1GB */
 	};
 
+	a10_leds {
+		compatible = "gpio-leds";
+
+		a10sr_led0 {
+			label = "a10sr_led0";
+			gpios = <&a10sr_gpio 4 1>;
+		};
+
+		a10sr_led1 {
+			label = "a10sr_led1";
+			gpios = <&a10sr_gpio 5 1>;
+		};
+
+		a10sr_led2 {
+			label = "a10sr_led2";
+			gpios = <&a10sr_gpio 6 1>;
+		};
+
+		a10sr_led3 {
+			label = "a10sr_led3";
+			gpios = <&a10sr_gpio 7 1>;
+		};
+	};
+
 	soc {
 		clkmgr@ffd04000 {
 			clocks {
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC 8/8] ARM: socfpga: dts: Add Devkit Arria10-SR HWMON

[tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx]

From: Thor Thayer <tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>

Add the hwmon node to the Altera Arria10 System Resource node.

Signed-off-by: Thor Thayer <tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>
---
 arch/arm/boot/dts/socfpga_arria10_socdk.dtsi |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/socfpga_arria10_socdk.dtsi b/arch/arm/boot/dts/socfpga_arria10_socdk.dtsi
index fc0b7a0..fe70d03 100644
--- a/arch/arm/boot/dts/socfpga_arria10_socdk.dtsi
+++ b/arch/arm/boot/dts/socfpga_arria10_socdk.dtsi
@@ -108,6 +108,10 @@
 			#gpio-cells = <2>;
 			ngpios = <16>;
 		};
+
+		a10sr_hwmon: a10sr_hwmon {
+			compatible = "altr,a10sr-hwmon";
+		};
 	};
 };
 
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] Addition of Altera Arria10 System Resource Chip

[Lee Jones]

On Tue, 29 Mar 2016, tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org wrote:

You need to submit this properly, using `git send-email`, since it
will look after you by listing each patch, together with their author,
and provide a nice diff-stat, which is very helpful to reviewers.

> The Altera Arria10 Development Kit includes a system resource chip
> that fits under the Multi-Function Device framework. The chip has
> hardware monitoring functions and a GPIO expander over the SPI bus.
> 
> [RFC 1/8] dt-bindings: mfd: Add Altera Arria10 System Resource Chip
> [RFC 2/8] MAINTAINERS: Addition of Altera Arria10 System Resource
> [RFC 3/8] mfd: altr_a10sr: Add Altera Arria10 DevKit System Resource
> [RFC 4/8] gpio: altera-a10sr: Add A10 System Resource Chip GPIO
> [RFC 5/8] ARM: socfpga: dts: Add Devkit A10-SR fields for Arria10
> [RFC 6/8] ARM: socfpga: dts: Add LED framework to A10-SR GPIO
> [RFC 7/8] hwmon: Altera Arria10 System Resource Chip - HW Monitor
> [RFC 8/8] ARM: socfpga: dts: Add Devkit Arria10-SR HWMON

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC 7/8] hwmon: Altera Arria10 System Resource Chip - HW Monitor

[tthayer]

From: Thor Thayer <tthayer@opensource.altera.com>

This patch adds the hwmon functionality to the Arria10 System
Resource Chip. The hwmon encapsulates the PCIe Enable, USB Enable,
and all the Power Good signals on the System Controller.

Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
---
 drivers/hwmon/Kconfig              |    9 +
 drivers/hwmon/Makefile             |    1 +
 drivers/hwmon/altera-a10sr-hwmon.c |  544 ++++++++++++++++++++++++++++++++++++
 drivers/mfd/altera-a10sr.c         |    4 +
 include/linux/mfd/altera-a10sr.h   |  107 +++++--
 5 files changed, 645 insertions(+), 20 deletions(-)
 create mode 100644 drivers/hwmon/altera-a10sr-hwmon.c

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 5c2d13a..edea31a 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -81,6 +81,15 @@ config SENSORS_ABITUGURU3
 	  This driver can also be built as a module.  If so, the module
 	  will be called abituguru3.
 
+config SENSORS_ALTERA_A10SR
+	bool "Altera Arria10 System Status"
+	depends on MFD_ALTERA_A10SR
+	help
+	  If you say yes here you get support for the power ready status
+	  for the Arria10's external power supplies on the Arria10 DevKit.
+	  These values are read over the SPI bus from the Arria10 System
+	  Resource chip.
+
 config SENSORS_AD7314
 	tristate "Analog Devices AD7314 and compatibles"
 	depends on SPI
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 58cc3ac..7a75dc8 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -43,6 +43,7 @@ obj-$(CONFIG_SENSORS_ADT7411)	+= adt7411.o
 obj-$(CONFIG_SENSORS_ADT7462)	+= adt7462.o
 obj-$(CONFIG_SENSORS_ADT7470)	+= adt7470.o
 obj-$(CONFIG_SENSORS_ADT7475)	+= adt7475.o
+obj-$(CONFIG_SENSORS_ALTERA_A10SR) += altera-a10sr-hwmon.o
 obj-$(CONFIG_SENSORS_APPLESMC)	+= applesmc.o
 obj-$(CONFIG_SENSORS_ARM_SCPI)	+= scpi-hwmon.o
 obj-$(CONFIG_SENSORS_ASC7621)	+= asc7621.o
diff --git a/drivers/hwmon/altera-a10sr-hwmon.c b/drivers/hwmon/altera-a10sr-hwmon.c
new file mode 100644
index 0000000..e789eed
--- /dev/null
+++ b/drivers/hwmon/altera-a10sr-hwmon.c
@@ -0,0 +1,544 @@
+/*
+ *  Copyright Altera Corporation (C) 2014-2016. All Rights Reserved
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * HW Monitor driver for  Altera Arria10 MAX5 System Resource Chip
+ * Adapted from DA9052
+ */
+
+#include <linux/err.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/mfd/altera-a10sr.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+#define ALTR_A10SR_1V0_BIT_POS          ALTR_A10SR_PG1_1V0_SHIFT
+#define ALTR_A10SR_0V95_BIT_POS         ALTR_A10SR_PG1_0V95_SHIFT
+#define ALTR_A10SR_0V9_BIT_POS          ALTR_A10SR_PG1_0V9_SHIFT
+#define ALTR_A10SR_10V_BIT_POS          ALTR_A10SR_PG1_10V_SHIFT
+#define ALTR_A10SR_5V0_BIT_POS          ALTR_A10SR_PG1_5V0_SHIFT
+#define ALTR_A10SR_3V3_BIT_POS          ALTR_A10SR_PG1_3V3_SHIFT
+#define ALTR_A10SR_2V5_BIT_POS          ALTR_A10SR_PG1_2V5_SHIFT
+#define ALTR_A10SR_1V8_BIT_POS          ALTR_A10SR_PG1_1V8_SHIFT
+#define ALTR_A10SR_OP_FLAG_BIT_POS      ALTR_A10SR_PG1_OP_FLAG_SHIFT
+/* 2nd register needs an offset of 8 to get to 2nd register */
+#define ALTR_A10SR_FBC2MP_BIT_POS       (8 + ALTR_A10SR_PG2_FBC2MP_SHIFT)
+#define ALTR_A10SR_FAC2MP_BIT_POS       (8 + ALTR_A10SR_PG2_FAC2MP_SHIFT)
+#define ALTR_A10SR_FMCBVADJ_BIT_POS     (8 + ALTR_A10SR_PG2_FMCBVADJ_SHIFT)
+#define ALTR_A10SR_FMCAVADJ_BIT_POS     (8 + ALTR_A10SR_PG2_FMCAVADJ_SHIFT)
+#define ALTR_A10SR_HL_VDDQ_BIT_POS      (8 + ALTR_A10SR_PG2_HL_VDDQ_SHIFT)
+#define ALTR_A10SR_HL_VDD_BIT_POS       (8 + ALTR_A10SR_PG2_HL_VDD_SHIFT)
+#define ALTR_A10SR_HL_HPS_BIT_POS       (8 + ALTR_A10SR_PG2_HL_HPS_SHIFT)
+#define ALTR_A10SR_HPS_BIT_POS          (8 + ALTR_A10SR_PG2_HPS_SHIFT)
+/* 3rd register needs an offset of 16 to get to 3rd register */
+#define ALTR_A10SR_PCIE_WAKE_BIT_POS    (16 + ALTR_A10SR_PG3_PCIE_WAKE_SHIFT)
+#define ALTR_A10SR_PCIE_PR_BIT_POS      (16 + ALTR_A10SR_PG3_PCIE_PR_SHIFT)
+#define ALTR_A10SR_FMCB_PR_BIT_POS      (16 + ALTR_A10SR_PG3_FMCB_PR_SHIFT)
+#define ALTR_A10SR_FMCA_PR_BIT_POS      (16 + ALTR_A10SR_PG3_FMCA_PR_SHIFT)
+#define ALTR_A10SR_FILE_PR_BIT_POS      (16 + ALTR_A10SR_PG3_FILE_PR_SHIFT)
+#define ALTR_A10SR_BF_PR_BIT_POS        (16 + ALTR_A10SR_PG3_BF_PR_SHIFT)
+#define ALTR_A10SR_10V_FAIL_BIT_POS     (16 + ALTR_A10SR_PG3_10V_FAIL_SHIFT)
+#define ALTR_A10SR_FAM2C_BIT_POS        (16 + ALTR_A10SR_PG3_FAM2C_SHIFT)
+/* FMCA/B & PCIE Enables need an offset of 24 */
+#define ALTR_A10SR_FMCB_AUXEN_POS       (24 + ALTR_A10SR_FMCB_AUXEN_SHIFT)
+#define ALTR_A10SR_FMCB_EN_POS          (24 + ALTR_A10SR_FMCB_EN_SHIFT)
+#define ALTR_A10SR_FMCA_AUXEN_POS       (24 + ALTR_A10SR_FMCA_AUXEN_SHIFT)
+#define ALTR_A10SR_FMCA_EN_POS          (24 + ALTR_A10SR_FMCA_EN_SHIFT)
+#define ALTR_A10SR_PCIE_AUXEN_POS       (24 + ALTR_A10SR_PCIE_AUXEN_SHIFT)
+#define ALTR_A10SR_PCIE_EN_POS          (24 + ALTR_A10SR_PCIE_EN_SHIFT)
+/* HPS Resets need an offset of 32 */
+#define ALTR_A10SR_HPS_RST_UART_POS     (32 + ALTR_A10SR_HPS_UARTA_RSTN_SHIFT)
+#define ALTR_A10SR_HPS_RST_WARM_POS     (32 + ALTR_A10SR_HPS_WARM_RSTN_SHIFT)
+#define ALTR_A10SR_HPS_RST_WARM1_POS    (32 + ALTR_A10SR_HPS_WARM_RST1N_SHIFT)
+#define ALTR_A10SR_HPS_RST_COLD_POS     (32 + ALTR_A10SR_HPS_COLD_RSTN_SHIFT)
+#define ALTR_A10SR_HPS_RST_NPOR_POS     (32 + ALTR_A10SR_HPS_NPOR_SHIFT)
+#define ALTR_A10SR_HPS_RST_NRST_POS     (32 + ALTR_A10SR_HPS_NRST_SHIFT)
+#define ALTR_A10SR_HPS_RST_ENET_POS     (32 + ALTR_A10SR_HPS_ENET_RSTN_SHIFT)
+#define ALTR_A10SR_HPS_RST_ENETINT_POS  (32 + ALTR_A10SR_HPS_ENET_INTN_SHIFT)
+/* Peripheral Resets need an offset of 40 */
+#define ALTR_A10SR_PER_RST_USB_POS      (40 + ALTR_A10SR_USB_RST_SHIFT)
+#define ALTR_A10SR_PER_RST_BQSPI_POS    (40 + ALTR_A10SR_BQSPI_RST_N_SHIFT)
+#define ALTR_A10SR_PER_RST_FILE_POS     (40 + ALTR_A10SR_FILE_RST_N_SHIFT)
+#define ALTR_A10SR_PER_RST_PCIE_POS     (40 + ALTR_A10SR_PCIE_PERST_N_SHIFT)
+/* HWMON - Read Entire Register */
+#define ALTR_A10SR_ENTIRE_REG           (88)
+#define ALTR_A10SR_ENTIRE_REG_MASK      (0xFF)
+#define ALTR_A10SR_VERSION              (0 + ALTR_A10SR_ENTIRE_REG)
+#define ALTR_A10SR_LED                  (1 + ALTR_A10SR_ENTIRE_REG)
+#define ALTR_A10SR_PB                   (2 + ALTR_A10SR_ENTIRE_REG)
+#define ALTR_A10SR_PBF                  (3 + ALTR_A10SR_ENTIRE_REG)
+#define ALTR_A10SR_PG1                  (4 + ALTR_A10SR_ENTIRE_REG)
+#define ALTR_A10SR_PG2                  (5 + ALTR_A10SR_ENTIRE_REG)
+#define ALTR_A10SR_PG3                  (6 + ALTR_A10SR_ENTIRE_REG)
+#define ALTR_A10SR_FMCAB                (7 + ALTR_A10SR_ENTIRE_REG)
+#define ALTR_A10SR_HPS_RST              (8 + ALTR_A10SR_ENTIRE_REG)
+#define ALTR_A10SR_PER_RST              (9 + ALTR_A10SR_ENTIRE_REG)
+#define ALTR_A10SR_SFPA                 (10 + ALTR_A10SR_ENTIRE_REG)
+#define ALTR_A10SR_SFPB                 (11 + ALTR_A10SR_ENTIRE_REG)
+#define ALTR_A10SR_I2C_MASTER           (12 + ALTR_A10SR_ENTIRE_REG)
+#define ALTR_A10SR_WARM_RST             (13 + ALTR_A10SR_ENTIRE_REG)
+#define ALTR_A10SR_WARM_RST_KEY         (14 + ALTR_A10SR_ENTIRE_REG)
+#define ALTR_A10SR_PMBUS                (15 + ALTR_A10SR_ENTIRE_REG)
+
+struct altr_a10sr_hwmon {
+	struct altr_a10sr	*a10sr;
+	struct device		*class_device;
+};
+
+static const char *const hwmon_names[] = {
+	[ALTR_A10SR_1V0_BIT_POS]       = "1.0V PWR Good",
+	[ALTR_A10SR_0V95_BIT_POS]      = "0.95V PWR Good",
+	[ALTR_A10SR_0V9_BIT_POS]       = "0.9V PWR Good",
+	[ALTR_A10SR_5V0_BIT_POS]       = "5.0V PWR Good",
+	[ALTR_A10SR_3V3_BIT_POS]       = "3.3V PWR Good",
+	[ALTR_A10SR_2V5_BIT_POS]       = "2.5V PWR Good",
+	[ALTR_A10SR_1V8_BIT_POS]       = "1.8V PWR Good",
+	[ALTR_A10SR_OP_FLAG_BIT_POS]   = "PWR On Complete",
+
+	[ALTR_A10SR_FBC2MP_BIT_POS]    = "FBC2MP PWR Good",
+	[ALTR_A10SR_FAC2MP_BIT_POS]    = "FAC2MP PWR Good",
+	[ALTR_A10SR_FMCBVADJ_BIT_POS]  = "FMCBVADJ PWR Good",
+	[ALTR_A10SR_FMCAVADJ_BIT_POS]  = "FMCAVADJ PWR Good",
+	[ALTR_A10SR_HL_VDDQ_BIT_POS]   = "HILO VDDQ PWR Good",
+	[ALTR_A10SR_HL_VDD_BIT_POS]    = "HILO VDD PWR Good",
+	[ALTR_A10SR_HL_HPS_BIT_POS]    = "HILO HPS PWR Good",
+	[ALTR_A10SR_HPS_BIT_POS]       = "HPS PWR Good",
+
+	[ALTR_A10SR_PCIE_WAKE_BIT_POS] = "PCIE WAKEn",
+	[ALTR_A10SR_PCIE_PR_BIT_POS]   = "PCIE PRESENTn",
+	[ALTR_A10SR_FMCB_PR_BIT_POS]   = "FMCB PRESENTn",
+	[ALTR_A10SR_FMCA_PR_BIT_POS]   = "FMCA PRESENTn",
+	[ALTR_A10SR_FILE_PR_BIT_POS]   = "FILE PRESENTn",
+	[ALTR_A10SR_BF_PR_BIT_POS]     = "BF PRESENTn",
+	[ALTR_A10SR_10V_FAIL_BIT_POS]  = "10V FAILn",
+	[ALTR_A10SR_FAM2C_BIT_POS]     = "FAM2C PWR Good",
+};
+
+static ssize_t altr_a10sr_read_status(struct device *dev,
+				      struct device_attribute *devattr,
+				      char *buf)
+{
+	struct altr_a10sr_hwmon *hwmon = dev_get_drvdata(dev);
+	int ret, index = to_sensor_dev_attr(devattr)->index;
+	int mask = ALTR_A10SR_REG_BIT_MASK(index);
+	unsigned char reg = ALTR_A10SR_PWR_GOOD1_RD_REG +
+			    ALTR_A10SR_REG_OFFSET(index);
+
+	/* Check if this is an entire register read */
+	if (index >= ALTR_A10SR_ENTIRE_REG) {
+		reg = ((index - ALTR_A10SR_ENTIRE_REG) << 1) + 1;
+		mask = ALTR_A10SR_ENTIRE_REG_MASK;
+	}
+
+	ret = altr_a10sr_reg_read(hwmon->a10sr, reg);
+	if (ret < 0)
+		return ret;
+
+	return sprintf(buf, "0x%X\n", (ret & mask));
+}
+
+static ssize_t altr_a10sr_hwmon_show_name(struct device *dev,
+					  struct device_attribute *devattr,
+					  char *buf)
+{
+	return sprintf(buf, "altr_a10sr\n");
+}
+
+static ssize_t show_label(struct device *dev,
+			  struct device_attribute *devattr, char *buf)
+{
+	return sprintf(buf, "%s\n",
+		       hwmon_names[to_sensor_dev_attr(devattr)->index]);
+}
+
+static ssize_t set_enable(struct device *dev,
+			  struct device_attribute *dev_attr,
+			  const char *buf, size_t count)
+{
+	unsigned long val;
+	struct altr_a10sr_hwmon *hwmon = dev_get_drvdata(dev);
+	int ret, index = to_sensor_dev_attr(dev_attr)->index;
+	int mask = ALTR_A10SR_REG_BIT_MASK(index);
+	unsigned char reg = (ALTR_A10SR_PWR_GOOD1_RD_REG & WRITE_REG_MASK) +
+			    ALTR_A10SR_REG_OFFSET(index);
+	int res = kstrtol(buf, 10, &val);
+
+	if (res < 0)
+		return res;
+
+	/* Check if this is an entire register write */
+	if (index >= ALTR_A10SR_ENTIRE_REG) {
+		reg = ((index - ALTR_A10SR_ENTIRE_REG) << 1);
+		mask = ALTR_A10SR_ENTIRE_REG_MASK;
+	}
+
+	ret = altr_a10sr_reg_update(hwmon->a10sr, reg, mask, val);
+	if (ret < 0)
+		return ret;
+
+	return count;
+}
+
+/* First Power Good Register Bits */
+static SENSOR_DEVICE_ATTR(1v0_input, S_IRUGO, altr_a10sr_read_status, NULL,
+			  ALTR_A10SR_1V0_BIT_POS);
+static SENSOR_DEVICE_ATTR(1v0_label, S_IRUGO, show_label, NULL,
+			  ALTR_A10SR_1V0_BIT_POS);
+static SENSOR_DEVICE_ATTR(0v95_input, S_IRUGO, altr_a10sr_read_status, NULL,
+			  ALTR_A10SR_0V95_BIT_POS);
+static SENSOR_DEVICE_ATTR(0v95_label, S_IRUGO, show_label, NULL,
+			  ALTR_A10SR_0V95_BIT_POS);
+static SENSOR_DEVICE_ATTR(0v9_input, S_IRUGO, altr_a10sr_read_status, NULL,
+			  ALTR_A10SR_0V9_BIT_POS);
+static SENSOR_DEVICE_ATTR(0v9_label, S_IRUGO, show_label, NULL,
+			  ALTR_A10SR_0V9_BIT_POS);
+static SENSOR_DEVICE_ATTR(5v0_input, S_IRUGO, altr_a10sr_read_status, NULL,
+			  ALTR_A10SR_5V0_BIT_POS);
+static SENSOR_DEVICE_ATTR(5v0_label, S_IRUGO, show_label, NULL,
+			  ALTR_A10SR_5V0_BIT_POS);
+static SENSOR_DEVICE_ATTR(3v3_input, S_IRUGO, altr_a10sr_read_status, NULL,
+			  ALTR_A10SR_3V3_BIT_POS);
+static SENSOR_DEVICE_ATTR(3v3_label, S_IRUGO, show_label, NULL,
+			  ALTR_A10SR_3V3_BIT_POS);
+static SENSOR_DEVICE_ATTR(2v5_input, S_IRUGO, altr_a10sr_read_status, NULL,
+			  ALTR_A10SR_2V5_BIT_POS);
+static SENSOR_DEVICE_ATTR(2v5_label, S_IRUGO, show_label, NULL,
+			  ALTR_A10SR_2V5_BIT_POS);
+static SENSOR_DEVICE_ATTR(1v8_input, S_IRUGO, altr_a10sr_read_status, NULL,
+			  ALTR_A10SR_1V8_BIT_POS);
+static SENSOR_DEVICE_ATTR(1v8_label, S_IRUGO, show_label, NULL,
+			  ALTR_A10SR_1V8_BIT_POS);
+static SENSOR_DEVICE_ATTR(opflag_input, S_IRUGO, altr_a10sr_read_status, NULL,
+			  ALTR_A10SR_OP_FLAG_BIT_POS);
+static SENSOR_DEVICE_ATTR(opflag_label, S_IRUGO, show_label, NULL,
+			  ALTR_A10SR_OP_FLAG_BIT_POS);
+/* Second Power Good Register Bits */
+static SENSOR_DEVICE_ATTR(fbc2mp_input, S_IRUGO, altr_a10sr_read_status, NULL,
+			  ALTR_A10SR_FBC2MP_BIT_POS);
+static SENSOR_DEVICE_ATTR(fbc2mp_label, S_IRUGO, show_label, NULL,
+			  ALTR_A10SR_FBC2MP_BIT_POS);
+static SENSOR_DEVICE_ATTR(fac2mp_input, S_IRUGO, altr_a10sr_read_status, NULL,
+			  ALTR_A10SR_FAC2MP_BIT_POS);
+static SENSOR_DEVICE_ATTR(fac2mp_label, S_IRUGO, show_label, NULL,
+			  ALTR_A10SR_FAC2MP_BIT_POS);
+static SENSOR_DEVICE_ATTR(fmcbvadj_input, S_IRUGO, altr_a10sr_read_status, NULL,
+			  ALTR_A10SR_FMCBVADJ_BIT_POS);
+static SENSOR_DEVICE_ATTR(fmcbvadj_label, S_IRUGO, show_label, NULL,
+			  ALTR_A10SR_FMCBVADJ_BIT_POS);
+static SENSOR_DEVICE_ATTR(fmcavadj_input, S_IRUGO, altr_a10sr_read_status, NULL,
+			  ALTR_A10SR_FMCAVADJ_BIT_POS);
+static SENSOR_DEVICE_ATTR(fmcavadj_label, S_IRUGO, show_label, NULL,
+			  ALTR_A10SR_FMCAVADJ_BIT_POS);
+static SENSOR_DEVICE_ATTR(hl_vddq_input, S_IRUGO, altr_a10sr_read_status, NULL,
+			  ALTR_A10SR_HL_VDDQ_BIT_POS);
+static SENSOR_DEVICE_ATTR(hl_vddq_label, S_IRUGO, show_label, NULL,
+			  ALTR_A10SR_HL_VDDQ_BIT_POS);
+static SENSOR_DEVICE_ATTR(hl_vdd_input, S_IRUGO, altr_a10sr_read_status, NULL,
+			  ALTR_A10SR_HL_VDD_BIT_POS);
+static SENSOR_DEVICE_ATTR(hl_vdd_label, S_IRUGO, show_label, NULL,
+			  ALTR_A10SR_HL_VDD_BIT_POS);
+static SENSOR_DEVICE_ATTR(hlhps_vdd_input, S_IRUGO, altr_a10sr_read_status,
+			  NULL, ALTR_A10SR_HL_HPS_BIT_POS);
+static SENSOR_DEVICE_ATTR(hlhps_vdd_label, S_IRUGO, show_label, NULL,
+			  ALTR_A10SR_HL_HPS_BIT_POS);
+static SENSOR_DEVICE_ATTR(hps_input, S_IRUGO, altr_a10sr_read_status, NULL,
+			  ALTR_A10SR_HPS_BIT_POS);
+static SENSOR_DEVICE_ATTR(hps_label, S_IRUGO, show_label, NULL,
+			  ALTR_A10SR_HPS_BIT_POS);
+/* Third Power Good Register Bits */
+static SENSOR_DEVICE_ATTR(pcie_wake_input, S_IRUGO, altr_a10sr_read_status,
+			  NULL, ALTR_A10SR_PCIE_WAKE_BIT_POS);
+static SENSOR_DEVICE_ATTR(pcie_wake_label, S_IRUGO, show_label, NULL,
+			  ALTR_A10SR_PCIE_WAKE_BIT_POS);
+static SENSOR_DEVICE_ATTR(pcie_pr_input, S_IRUGO, altr_a10sr_read_status, NULL,
+			  ALTR_A10SR_PCIE_PR_BIT_POS);
+static SENSOR_DEVICE_ATTR(pcie_pr_label, S_IRUGO, show_label, NULL,
+			  ALTR_A10SR_PCIE_PR_BIT_POS);
+static SENSOR_DEVICE_ATTR(fmcb_pr_input, S_IRUGO, altr_a10sr_read_status, NULL,
+			  ALTR_A10SR_FMCB_PR_BIT_POS);
+static SENSOR_DEVICE_ATTR(fmcb_pr_label, S_IRUGO, show_label, NULL,
+			  ALTR_A10SR_FMCB_PR_BIT_POS);
+static SENSOR_DEVICE_ATTR(fmca_pr_input, S_IRUGO, altr_a10sr_read_status, NULL,
+			  ALTR_A10SR_FMCA_PR_BIT_POS);
+static SENSOR_DEVICE_ATTR(fmca_pr_label, S_IRUGO, show_label, NULL,
+			  ALTR_A10SR_FMCA_PR_BIT_POS);
+static SENSOR_DEVICE_ATTR(file_pr_input, S_IRUGO, altr_a10sr_read_status, NULL,
+			  ALTR_A10SR_FILE_PR_BIT_POS);
+static SENSOR_DEVICE_ATTR(file_pr_label, S_IRUGO, show_label, NULL,
+			  ALTR_A10SR_FILE_PR_BIT_POS);
+static SENSOR_DEVICE_ATTR(bf_pr_input, S_IRUGO, altr_a10sr_read_status, NULL,
+			  ALTR_A10SR_BF_PR_BIT_POS);
+static SENSOR_DEVICE_ATTR(bf_pr_label, S_IRUGO, show_label, NULL,
+			  ALTR_A10SR_BF_PR_BIT_POS);
+static SENSOR_DEVICE_ATTR(10v_fail_input, S_IRUGO, altr_a10sr_read_status,
+			  NULL, ALTR_A10SR_10V_FAIL_BIT_POS);
+static SENSOR_DEVICE_ATTR(10v_fail_label, S_IRUGO, show_label, NULL,
+			  ALTR_A10SR_10V_FAIL_BIT_POS);
+static SENSOR_DEVICE_ATTR(fam2c_input, S_IRUGO, altr_a10sr_read_status, NULL,
+			  ALTR_A10SR_FAM2C_BIT_POS);
+static SENSOR_DEVICE_ATTR(fam2c_label, S_IRUGO, show_label, NULL,
+			  ALTR_A10SR_FAM2C_BIT_POS);
+/* Peripheral Enable bits */
+static SENSOR_DEVICE_ATTR(fmcb_aux_en, S_IRUGO | S_IWUSR,
+			  altr_a10sr_read_status, set_enable,
+			  ALTR_A10SR_FMCB_AUXEN_POS);
+static SENSOR_DEVICE_ATTR(fmcb_en, S_IRUGO | S_IWUSR,
+			  altr_a10sr_read_status, set_enable,
+			  ALTR_A10SR_FMCB_EN_POS);
+static SENSOR_DEVICE_ATTR(fmca_aux_en, S_IRUGO | S_IWUSR,
+			  altr_a10sr_read_status, set_enable,
+			  ALTR_A10SR_FMCA_AUXEN_POS);
+static SENSOR_DEVICE_ATTR(fmca_en, S_IRUGO | S_IWUSR,
+			  altr_a10sr_read_status, set_enable,
+			  ALTR_A10SR_FMCA_EN_POS);
+static SENSOR_DEVICE_ATTR(pcie_aux_en, S_IRUGO | S_IWUSR,
+			  altr_a10sr_read_status, set_enable,
+			  ALTR_A10SR_PCIE_AUXEN_POS);
+static SENSOR_DEVICE_ATTR(pcie_en, S_IRUGO | S_IWUSR,
+			  altr_a10sr_read_status, set_enable,
+			  ALTR_A10SR_PCIE_EN_POS);
+/* HPS Reset bits */
+static SENSOR_DEVICE_ATTR(hps_uart_rst, S_IRUGO,
+			  altr_a10sr_read_status, set_enable,
+			  ALTR_A10SR_HPS_RST_UART_POS);
+static SENSOR_DEVICE_ATTR(hps_warm_rst, S_IRUGO,
+			  altr_a10sr_read_status, set_enable,
+			  ALTR_A10SR_HPS_RST_WARM_POS);
+static SENSOR_DEVICE_ATTR(hps_warm1_rst, S_IRUGO,
+			  altr_a10sr_read_status, set_enable,
+			  ALTR_A10SR_HPS_RST_WARM1_POS);
+static SENSOR_DEVICE_ATTR(hps_cold_rst, S_IRUGO,
+			  altr_a10sr_read_status, set_enable,
+			  ALTR_A10SR_HPS_RST_COLD_POS);
+static SENSOR_DEVICE_ATTR(hps_npor, S_IRUGO,
+			  altr_a10sr_read_status, set_enable,
+			  ALTR_A10SR_HPS_RST_NPOR_POS);
+static SENSOR_DEVICE_ATTR(hps_nrst, S_IRUGO,
+			  altr_a10sr_read_status, set_enable,
+			  ALTR_A10SR_HPS_RST_NRST_POS);
+static SENSOR_DEVICE_ATTR(hps_enet_rst, S_IRUGO | S_IWUSR,
+			  altr_a10sr_read_status, set_enable,
+			  ALTR_A10SR_HPS_RST_ENET_POS);
+static SENSOR_DEVICE_ATTR(hps_enet_int, S_IRUGO | S_IWUSR,
+			  altr_a10sr_read_status, set_enable,
+			  ALTR_A10SR_HPS_RST_ENETINT_POS);
+/* Peripheral Reset bits */
+static SENSOR_DEVICE_ATTR(usb_reset, S_IRUGO | S_IWUSR, altr_a10sr_read_status,
+			  set_enable, ALTR_A10SR_PER_RST_USB_POS);
+static SENSOR_DEVICE_ATTR(bqspi_resetn, S_IRUGO | S_IWUSR,
+			  altr_a10sr_read_status, set_enable,
+			  ALTR_A10SR_PER_RST_BQSPI_POS);
+static SENSOR_DEVICE_ATTR(file_resetn, S_IRUGO | S_IWUSR,
+			  altr_a10sr_read_status, set_enable,
+			  ALTR_A10SR_PER_RST_FILE_POS);
+static SENSOR_DEVICE_ATTR(pcie_perstn, S_IRUGO | S_IWUSR,
+			  altr_a10sr_read_status, set_enable,
+			  ALTR_A10SR_PER_RST_PCIE_POS);
+/* Entire Byte Read */
+static SENSOR_DEVICE_ATTR(max5_version, S_IRUGO, altr_a10sr_read_status,
+			  NULL, ALTR_A10SR_VERSION);
+static SENSOR_DEVICE_ATTR(max5_led, S_IRUGO, altr_a10sr_read_status,
+			  NULL, ALTR_A10SR_LED);
+static SENSOR_DEVICE_ATTR(max5_button, S_IRUGO, altr_a10sr_read_status,
+			  NULL, ALTR_A10SR_PB);
+static SENSOR_DEVICE_ATTR(max5_button_irq, S_IRUGO | S_IWUSR,
+			  altr_a10sr_read_status, set_enable, ALTR_A10SR_PBF);
+static SENSOR_DEVICE_ATTR(max5_pg1, S_IRUGO, altr_a10sr_read_status,
+			  NULL, ALTR_A10SR_PG1);
+static SENSOR_DEVICE_ATTR(max5_pg2, S_IRUGO, altr_a10sr_read_status,
+			  NULL, ALTR_A10SR_PG2);
+static SENSOR_DEVICE_ATTR(max5_pg3, S_IRUGO, altr_a10sr_read_status,
+			  NULL, ALTR_A10SR_PG3);
+static SENSOR_DEVICE_ATTR(max5_fmcab, S_IRUGO, altr_a10sr_read_status,
+			  NULL, ALTR_A10SR_FMCAB);
+static SENSOR_DEVICE_ATTR(max5_hps_resets, S_IRUGO | S_IWUSR,
+			  altr_a10sr_read_status, set_enable,
+			  ALTR_A10SR_HPS_RST);
+static SENSOR_DEVICE_ATTR(max5_per_resets, S_IRUGO | S_IWUSR,
+			  altr_a10sr_read_status, set_enable,
+			  ALTR_A10SR_PER_RST);
+static SENSOR_DEVICE_ATTR(max5_sfpa, S_IRUGO | S_IWUSR,
+			  altr_a10sr_read_status, set_enable, ALTR_A10SR_SFPA);
+static SENSOR_DEVICE_ATTR(max5_sfpb, S_IRUGO | S_IWUSR,
+			  altr_a10sr_read_status, set_enable, ALTR_A10SR_SFPB);
+static SENSOR_DEVICE_ATTR(max5_i2c_master, S_IRUGO | S_IWUSR,
+			  altr_a10sr_read_status, set_enable,
+			  ALTR_A10SR_I2C_MASTER);
+static SENSOR_DEVICE_ATTR(max5_pmbus, S_IRUGO | S_IWUSR,
+			  altr_a10sr_read_status, set_enable,
+			  ALTR_A10SR_PMBUS);
+
+static DEVICE_ATTR(name, S_IRUGO, altr_a10sr_hwmon_show_name, NULL);
+
+static struct attribute *altr_a10sr_attr[] = {
+	&dev_attr_name.attr,
+	/* First Power Good Register */
+	&sensor_dev_attr_1v0_input.dev_attr.attr,
+	&sensor_dev_attr_1v0_label.dev_attr.attr,
+	&sensor_dev_attr_0v95_input.dev_attr.attr,
+	&sensor_dev_attr_0v95_label.dev_attr.attr,
+	&sensor_dev_attr_0v9_input.dev_attr.attr,
+	&sensor_dev_attr_0v9_label.dev_attr.attr,
+	&sensor_dev_attr_5v0_input.dev_attr.attr,
+	&sensor_dev_attr_5v0_label.dev_attr.attr,
+	&sensor_dev_attr_3v3_input.dev_attr.attr,
+	&sensor_dev_attr_3v3_label.dev_attr.attr,
+	&sensor_dev_attr_2v5_input.dev_attr.attr,
+	&sensor_dev_attr_2v5_label.dev_attr.attr,
+	&sensor_dev_attr_1v8_input.dev_attr.attr,
+	&sensor_dev_attr_1v8_label.dev_attr.attr,
+	&sensor_dev_attr_opflag_input.dev_attr.attr,
+	&sensor_dev_attr_opflag_label.dev_attr.attr,
+	/* Second Power Good Register */
+	&sensor_dev_attr_fbc2mp_input.dev_attr.attr,
+	&sensor_dev_attr_fbc2mp_label.dev_attr.attr,
+	&sensor_dev_attr_fac2mp_input.dev_attr.attr,
+	&sensor_dev_attr_fac2mp_label.dev_attr.attr,
+	&sensor_dev_attr_fmcbvadj_input.dev_attr.attr,
+	&sensor_dev_attr_fmcbvadj_label.dev_attr.attr,
+	&sensor_dev_attr_fmcavadj_input.dev_attr.attr,
+	&sensor_dev_attr_fmcavadj_label.dev_attr.attr,
+	&sensor_dev_attr_hl_vddq_input.dev_attr.attr,
+	&sensor_dev_attr_hl_vddq_label.dev_attr.attr,
+	&sensor_dev_attr_hl_vdd_input.dev_attr.attr,
+	&sensor_dev_attr_hl_vdd_label.dev_attr.attr,
+	&sensor_dev_attr_hlhps_vdd_input.dev_attr.attr,
+	&sensor_dev_attr_hlhps_vdd_label.dev_attr.attr,
+	&sensor_dev_attr_hps_input.dev_attr.attr,
+	&sensor_dev_attr_hps_label.dev_attr.attr,
+	/* Third Power Good Register */
+	&sensor_dev_attr_pcie_wake_input.dev_attr.attr,
+	&sensor_dev_attr_pcie_wake_label.dev_attr.attr,
+	&sensor_dev_attr_pcie_pr_input.dev_attr.attr,
+	&sensor_dev_attr_pcie_pr_label.dev_attr.attr,
+	&sensor_dev_attr_fmcb_pr_input.dev_attr.attr,
+	&sensor_dev_attr_fmcb_pr_label.dev_attr.attr,
+	&sensor_dev_attr_fmca_pr_input.dev_attr.attr,
+	&sensor_dev_attr_fmca_pr_label.dev_attr.attr,
+	&sensor_dev_attr_file_pr_input.dev_attr.attr,
+	&sensor_dev_attr_file_pr_label.dev_attr.attr,
+	&sensor_dev_attr_bf_pr_input.dev_attr.attr,
+	&sensor_dev_attr_bf_pr_label.dev_attr.attr,
+	&sensor_dev_attr_10v_fail_input.dev_attr.attr,
+	&sensor_dev_attr_10v_fail_label.dev_attr.attr,
+	&sensor_dev_attr_fam2c_input.dev_attr.attr,
+	&sensor_dev_attr_fam2c_label.dev_attr.attr,
+	/* Peripheral Enable Register */
+	&sensor_dev_attr_fmcb_aux_en.dev_attr.attr,
+	&sensor_dev_attr_fmcb_en.dev_attr.attr,
+	&sensor_dev_attr_fmca_aux_en.dev_attr.attr,
+	&sensor_dev_attr_fmca_en.dev_attr.attr,
+	&sensor_dev_attr_pcie_aux_en.dev_attr.attr,
+	&sensor_dev_attr_pcie_en.dev_attr.attr,
+	/* HPS Reset bits */
+	&sensor_dev_attr_hps_uart_rst.dev_attr.attr,
+	&sensor_dev_attr_hps_warm_rst.dev_attr.attr,
+	&sensor_dev_attr_hps_warm1_rst.dev_attr.attr,
+	&sensor_dev_attr_hps_cold_rst.dev_attr.attr,
+	&sensor_dev_attr_hps_npor.dev_attr.attr,
+	&sensor_dev_attr_hps_nrst.dev_attr.attr,
+	&sensor_dev_attr_hps_enet_rst.dev_attr.attr,
+	&sensor_dev_attr_hps_enet_int.dev_attr.attr,
+	/* Peripheral Reset bits */
+	&sensor_dev_attr_usb_reset.dev_attr.attr,
+	&sensor_dev_attr_bqspi_resetn.dev_attr.attr,
+	&sensor_dev_attr_file_resetn.dev_attr.attr,
+	&sensor_dev_attr_pcie_perstn.dev_attr.attr,
+	/* Byte Value Register */
+	&sensor_dev_attr_max5_version.dev_attr.attr,
+	&sensor_dev_attr_max5_led.dev_attr.attr,
+	&sensor_dev_attr_max5_button.dev_attr.attr,
+	&sensor_dev_attr_max5_button_irq.dev_attr.attr,
+	&sensor_dev_attr_max5_pg1.dev_attr.attr,
+	&sensor_dev_attr_max5_pg2.dev_attr.attr,
+	&sensor_dev_attr_max5_pg3.dev_attr.attr,
+	&sensor_dev_attr_max5_fmcab.dev_attr.attr,
+	&sensor_dev_attr_max5_hps_resets.dev_attr.attr,
+	&sensor_dev_attr_max5_per_resets.dev_attr.attr,
+	&sensor_dev_attr_max5_sfpa.dev_attr.attr,
+	&sensor_dev_attr_max5_sfpb.dev_attr.attr,
+	&sensor_dev_attr_max5_i2c_master.dev_attr.attr,
+	&sensor_dev_attr_max5_pmbus.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group altr_a10sr_attr_group = {
+	.attrs = altr_a10sr_attr
+};
+
+static int altr_a10sr_hwmon_probe(struct platform_device *pdev)
+{
+	struct altr_a10sr_hwmon *hwmon;
+	int ret;
+
+	hwmon = devm_kzalloc(&pdev->dev, sizeof(*hwmon), GFP_KERNEL);
+	if (!hwmon)
+		return -ENOMEM;
+
+	hwmon->a10sr = dev_get_drvdata(pdev->dev.parent);
+
+	platform_set_drvdata(pdev, hwmon);
+
+	ret = sysfs_create_group(&pdev->dev.kobj, &altr_a10sr_attr_group);
+	if (ret)
+		goto err_mem;
+
+	hwmon->class_device = hwmon_device_register(&pdev->dev);
+	if (IS_ERR(hwmon->class_device)) {
+		ret = PTR_ERR(hwmon->class_device);
+		goto err_sysfs;
+	}
+
+	return 0;
+
+err_sysfs:
+	sysfs_remove_group(&pdev->dev.kobj, &altr_a10sr_attr_group);
+err_mem:
+	return ret;
+}
+
+static int altr_a10sr_hwmon_remove(struct platform_device *pdev)
+{
+	struct altr_a10sr_hwmon *hwmon = platform_get_drvdata(pdev);
+
+	hwmon_device_unregister(hwmon->class_device);
+	sysfs_remove_group(&pdev->dev.kobj, &altr_a10sr_attr_group);
+
+	return 0;
+}
+
+static const struct of_device_id altr_a10sr_hwmon_of_match[] = {
+	{ .compatible = "altr,a10sr-hwmon" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, altr_a10sr_hwmon_of_match);
+
+static struct platform_driver altr_a10sr_hwmon_driver = {
+	.probe = altr_a10sr_hwmon_probe,
+	.remove = altr_a10sr_hwmon_remove,
+	.driver = {
+		.name = "altr_a10sr_hwmon",
+		.of_match_table = altr_a10sr_hwmon_of_match,
+	},
+};
+
+module_platform_driver(altr_a10sr_hwmon_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Thor Thayer");
+MODULE_DESCRIPTION("HW Monitor driver for Altera Arria10 System Resource Chip");
diff --git a/drivers/mfd/altera-a10sr.c b/drivers/mfd/altera-a10sr.c
index 517b895..3eedad7 100644
--- a/drivers/mfd/altera-a10sr.c
+++ b/drivers/mfd/altera-a10sr.c
@@ -34,6 +34,10 @@ static const struct mfd_cell altr_a10sr_subdev_info[] = {
 		.name = "altr_a10sr_gpio",
 		.of_compatible = "altr,a10sr-gpio",
 	},
+	{
+		.name = "altr_a10sr_hwmon",
+		.of_compatible = "altr,a10sr-hwmon",
+	},
 };
 
 static bool altr_a10sr_reg_readable(struct device *dev, unsigned int reg)
diff --git a/include/linux/mfd/altera-a10sr.h b/include/linux/mfd/altera-a10sr.h
index 6d254a1..2bfc63e 100644
--- a/include/linux/mfd/altera-a10sr.h
+++ b/include/linux/mfd/altera-a10sr.h
@@ -75,26 +75,93 @@
 #define ALTR_A10SR_IN_VALID_RANGE_LO      8
 #define ALTR_A10SR_IN_VALID_RANGE_HI      15
 
-#define ALTR_A10SR_PWR_GOOD1_RD_REG   0x09    /* Power Good1 Read */
-#define ALTR_A10SR_PWR_GOOD2_RD_REG   0x0B    /* Power Good2 Read */
-#define ALTR_A10SR_PWR_GOOD3_RD_REG   0x0D    /* Power Good3 Read */
-#define ALTR_A10SR_FMCAB_WR_REG       0x0E    /* FMCA/B & PCIe Pwr Enable */
-#define ALTR_A10SR_FMCAB_RD_REG       0x0F    /* FMCA/B & PCIe Pwr Enable */
-#define ALTR_A10SR_HPS_RST_WR_REG     0x10    /* HPS Reset */
-#define ALTR_A10SR_HPS_RST_RD_REG     0x11    /* HPS Reset */
-#define ALTR_A10SR_USB_QSPI_WR_REG    0x12    /* USB, BQSPI, FILE Reset */
-#define ALTR_A10SR_USB_QSPI_RD_REG    0x13    /* USB, BQSPI, FILE Reset */
-#define ALTR_A10SR_SFPA_WR_REG        0x14    /* SFPA Control Reg */
-#define ALTR_A10SR_SFPA_RD_REG        0x15    /* SFPA Control Reg */
-#define ALTR_A10SR_SFPB_WR_REG        0x16    /* SFPB Control Reg */
-#define ALTR_A10SR_SFPB_RD_REG        0x17    /* SFPB Control Reg */
-#define ALTR_A10SR_I2C_M_RD_REG       0x19    /* I2C Master Select */
-#define ALTR_A10SR_WARM_RST_WR_REG    0x1A    /* HPS Warm Reset */
-#define ALTR_A10SR_WARM_RST_RD_REG    0x1B    /* HPS Warm Reset */
-#define ALTR_A10SR_WR_KEY_WR_REG      0x1C    /* HPS Warm Reset Key */
-#define ALTR_A10SR_WR_KEY_RD_REG      0x1D    /* HPS Warm Reset Key */
-#define ALTR_A10SR_PMBUS_WR_REG       0x1E    /* HPS PM Bus */
-#define ALTR_A10SR_PMBUS_RD_REG       0x1F    /* HPS PM Bus */
+#define ALTR_A10SR_PWR_GOOD1_RD_REG  0x09     /* Power Good1 Read */
+/* Power Good #1 Register Bit Definitions */
+#define ALTR_A10SR_PG1_OP_FLAG_SHIFT    7       /* Power On Complete */
+#define ALTR_A10SR_PG1_1V8_SHIFT        6       /* 1.8V Power Good */
+#define ALTR_A10SR_PG1_2V5_SHIFT        5       /* 2.5V Power Good */
+#define ALTR_A10SR_PG1_3V3_SHIFT        4       /* 3.3V Power Good */
+#define ALTR_A10SR_PG1_5V0_SHIFT        3       /* 5.0V Power Good */
+#define ALTR_A10SR_PG1_0V9_SHIFT        2       /* 0.9V Power Good */
+#define ALTR_A10SR_PG1_0V95_SHIFT       1       /* 0.95V Power Good */
+#define ALTR_A10SR_PG1_1V0_SHIFT        0       /* 1.0V Power Good */
+
+#define ALTR_A10SR_PWR_GOOD2_RD_REG  0x0B     /* Power Good2 Read */
+/* Power Good #2 Register Bit Definitions */
+#define ALTR_A10SR_PG2_HPS_SHIFT        7       /* HPS Power Good */
+#define ALTR_A10SR_PG2_HL_HPS_SHIFT     6       /* HILOHPS_VDD Power Good */
+#define ALTR_A10SR_PG2_HL_VDD_SHIFT     5       /* HILO VDD Power Good */
+#define ALTR_A10SR_PG2_HL_VDDQ_SHIFT    4       /* HILO VDDQ Power Good */
+#define ALTR_A10SR_PG2_FMCAVADJ_SHIFT   3       /* FMCA VADJ Power Good */
+#define ALTR_A10SR_PG2_FMCBVADJ_SHIFT   2       /* FMCB VADJ Power Good */
+#define ALTR_A10SR_PG2_FAC2MP_SHIFT     1       /* FAC2MP Power Good */
+#define ALTR_A10SR_PG2_FBC2MP_SHIFT     0       /* FBC2MP Power Good */
+
+#define ALTR_A10SR_PWR_GOOD3_RD_REG  0x0D   /* Power Good3 Read */
+/* Power Good #3 Register Bit Definitions */
+#define ALTR_A10SR_PG3_FAM2C_SHIFT      7       /* FAM2C Power Good */
+#define ALTR_A10SR_PG3_10V_FAIL_SHIFT   6       /* 10V Fail n */
+#define ALTR_A10SR_PG3_BF_PR_SHIFT      5       /* BF Present n */
+#define ALTR_A10SR_PG3_FILE_PR_SHIFT    4       /* File Present n */
+#define ALTR_A10SR_PG3_FMCA_PR_SHIFT    3       /* FMCA Present n */
+#define ALTR_A10SR_PG3_FMCB_PR_SHIFT    2       /* FMCB Present n */
+#define ALTR_A10SR_PG3_PCIE_PR_SHIFT    1       /* PCIE Present n */
+#define ALTR_A10SR_PG3_PCIE_WAKE_SHIFT  0       /* PCIe Wake N */
+
+#define ALTR_A10SR_FMCAB_WR_REG      0x0E    /* FMCA/B & PCIe Pwr Enable */
+#define ALTR_A10SR_FMCAB_RD_REG      0x0F    /* FMCA/B & PCIe Pwr Enable */
+/* FMCA/B & PCIe Power Bit Definitions */
+#define ALTR_A10SR_PCIE_EN_SHIFT        7       /* PCIe Pwr Enable */
+#define ALTR_A10SR_PCIE_AUXEN_SHIFT     6       /* PCIe Aux Pwr Enable */
+#define ALTR_A10SR_FMCA_EN_SHIFT        5       /* FMCA Pwr Enable */
+#define ALTR_A10SR_FMCA_AUXEN_SHIFT     4       /* FMCA Aux Pwr Enable */
+#define ALTR_A10SR_FMCB_EN_SHIFT        3       /* FMCB Pwr Enable */
+#define ALTR_A10SR_FMCB_AUXEN_SHIFT     2       /* FMCB Aux Pwr Enable */
+
+#define ALTR_A10SR_HPS_RST_WR_REG    0x10    /* HPS Reset */
+#define ALTR_A10SR_HPS_RST_RD_REG    0x11    /* HPS Reset */
+/* HPS Reset Bit Definitions */
+#define ALTR_A10SR_HPS_UARTA_RSTN_SHIFT 7       /* UARTA Reset n */
+#define ALTR_A10SR_HPS_WARM_RSTN_SHIFT  6       /* WARM Reset n */
+#define ALTR_A10SR_HPS_WARM_RST1N_SHIFT 5       /* WARM Reset1 n */
+#define ALTR_A10SR_HPS_COLD_RSTN_SHIFT  4       /* COLD Reset n */
+#define ALTR_A10SR_HPS_NPOR_SHIFT       3       /* N Power On Reset */
+#define ALTR_A10SR_HPS_NRST_SHIFT       2       /* N Reset */
+#define ALTR_A10SR_HPS_ENET_RSTN_SHIFT  1       /* Ethernet Reset n */
+#define ALTR_A10SR_HPS_ENET_INTN_SHIFT  0       /* Ethernet IRQ n */
+
+#define ALTR_A10SR_USB_QSPI_WR_REG   0x12    /* USB, BQSPI, FILE Reset */
+#define ALTR_A10SR_USB_QSPI_RD_REG   0x13    /* USB, BQSPI, FILE Reset */
+/* USB/QSPI/FILE Reset Bit Definitions */
+#define ALTR_A10SR_USB_RST_SHIFT        7       /* USB Reset */
+#define ALTR_A10SR_BQSPI_RST_N_SHIFT    6       /* BQSPI Reset n */
+#define ALTR_A10SR_FILE_RST_N_SHIFT     5       /* FILE Reset n */
+#define ALTR_A10SR_PCIE_PERST_N_SHIFT   4       /* PCIe PE Reset n */
+
+#define ALTR_A10SR_SFPA_WR_REG       0x14    /* SFPA Control Reg */
+#define ALTR_A10SR_SFPA_RD_REG       0x15    /* SFPA Control Reg */
+#define ALTR_A10SR_SFPB_WR_REG       0x16    /* SFPB Control Reg */
+#define ALTR_A10SR_SFPB_RD_REG       0x17    /* SFPB Control Reg */
+/* SFPA Bit Definitions */
+#define ALTR_A10SR_SFP_TXDIS_SHIFT      7       /* SFPA TX Disable */
+#define ALTR_A10SR_SFP_RATESEL10        0x60    /* SFPA_Rate Select [1:0] */
+#define ALTR_A10SR_SFP_LOS_SHIFT        4       /* SFPA LOS */
+#define ALTR_A10SR_SFP_FAULT_SHIFT      3       /* SFPA Fault */
+
+#define ALTR_A10SR_I2C_M_RD_REG      0x19    /* I2C Master Select */
+
+#define ALTR_A10SR_WARM_RST_WR_REG   0x1A    /* HPS Warm Reset */
+#define ALTR_A10SR_WARM_RST_RD_REG   0x1B    /* HPS Warm Reset */
+
+#define ALTR_A10SR_WR_KEY_WR_REG     0x1C    /* HPS Warm Reset Key */
+#define ALTR_A10SR_WR_KEY_RD_REG     0x1D    /* HPS Warm Reset Key */
+
+#define ALTR_A10SR_PMBUS_WR_REG      0x1E    /* HPS PM Bus */
+#define ALTR_A10SR_PMBUS_RD_REG      0x1F    /* HPS PM Bus */
+/* PM Bus Bit Definitions */
+#define ALTR_A10SR_PMBUS_EN_SHIFT       7       /* PMBus FPGA Enable */
+#define ALTR_A10SR_PMBUS_DISN_SHIFT     6       /* PMBus HPS Enable */
+#define ALTR_A10SR_PMBUS_ALERTN_SHIFT   5       /* PMBus Alert */
 
 struct altr_a10sr {
 	struct device *dev;
-- 
1.7.9.5

Re: [RFC 7/8] hwmon: Altera Arria10 System Resource Chip - HW Monitor

[Guenter Roeck]

On Tue, Mar 29, 2016 at 02:13:10PM -0500, tthayer@opensource.altera.com wrote:
> From: Thor Thayer <tthayer@opensource.altera.com>
> 
> This patch adds the hwmon functionality to the Arria10 System
> Resource Chip. The hwmon encapsulates the PCIe Enable, USB Enable,
> and all the Power Good signals on the System Controller.
> 

I may be completely wrong, but a glance through the driver suggests
that, if anything, this should be a regulator driver, not a hwmon driver.
A hardware monitoring driver would be expected to report the voltages,
not (just) the voltage status. Am I missing something ?

Please have a look into Documentation/hwmon/sysfs-interface for
acceptable hwmon attribute names and their meaning.

Thanks,
Guenter

> Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
> ---
>  drivers/hwmon/Kconfig              |    9 +
>  drivers/hwmon/Makefile             |    1 +
>  drivers/hwmon/altera-a10sr-hwmon.c |  544 ++++++++++++++++++++++++++++++++++++
>  drivers/mfd/altera-a10sr.c         |    4 +
>  include/linux/mfd/altera-a10sr.h   |  107 +++++--
>  5 files changed, 645 insertions(+), 20 deletions(-)
>  create mode 100644 drivers/hwmon/altera-a10sr-hwmon.c
> 
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 5c2d13a..edea31a 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -81,6 +81,15 @@ config SENSORS_ABITUGURU3
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called abituguru3.
>  
> +config SENSORS_ALTERA_A10SR
> +	bool "Altera Arria10 System Status"
> +	depends on MFD_ALTERA_A10SR
> +	help
> +	  If you say yes here you get support for the power ready status
> +	  for the Arria10's external power supplies on the Arria10 DevKit.
> +	  These values are read over the SPI bus from the Arria10 System
> +	  Resource chip.
> +
>  config SENSORS_AD7314
>  	tristate "Analog Devices AD7314 and compatibles"
>  	depends on SPI
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 58cc3ac..7a75dc8 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -43,6 +43,7 @@ obj-$(CONFIG_SENSORS_ADT7411)	+= adt7411.o
>  obj-$(CONFIG_SENSORS_ADT7462)	+= adt7462.o
>  obj-$(CONFIG_SENSORS_ADT7470)	+= adt7470.o
>  obj-$(CONFIG_SENSORS_ADT7475)	+= adt7475.o
> +obj-$(CONFIG_SENSORS_ALTERA_A10SR) += altera-a10sr-hwmon.o
>  obj-$(CONFIG_SENSORS_APPLESMC)	+= applesmc.o
>  obj-$(CONFIG_SENSORS_ARM_SCPI)	+= scpi-hwmon.o
>  obj-$(CONFIG_SENSORS_ASC7621)	+= asc7621.o
> diff --git a/drivers/hwmon/altera-a10sr-hwmon.c b/drivers/hwmon/altera-a10sr-hwmon.c
> new file mode 100644
> index 0000000..e789eed
> --- /dev/null
> +++ b/drivers/hwmon/altera-a10sr-hwmon.c
> @@ -0,0 +1,544 @@
> +/*
> + *  Copyright Altera Corporation (C) 2014-2016. All Rights Reserved
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program.  If not, see <http://www.gnu.org/licenses/>.
> + *
> + * HW Monitor driver for  Altera Arria10 MAX5 System Resource Chip
> + * Adapted from DA9052
> + */
> +
> +#include <linux/err.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/mfd/altera-a10sr.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +#define ALTR_A10SR_1V0_BIT_POS          ALTR_A10SR_PG1_1V0_SHIFT
> +#define ALTR_A10SR_0V95_BIT_POS         ALTR_A10SR_PG1_0V95_SHIFT
> +#define ALTR_A10SR_0V9_BIT_POS          ALTR_A10SR_PG1_0V9_SHIFT
> +#define ALTR_A10SR_10V_BIT_POS          ALTR_A10SR_PG1_10V_SHIFT
> +#define ALTR_A10SR_5V0_BIT_POS          ALTR_A10SR_PG1_5V0_SHIFT
> +#define ALTR_A10SR_3V3_BIT_POS          ALTR_A10SR_PG1_3V3_SHIFT
> +#define ALTR_A10SR_2V5_BIT_POS          ALTR_A10SR_PG1_2V5_SHIFT
> +#define ALTR_A10SR_1V8_BIT_POS          ALTR_A10SR_PG1_1V8_SHIFT
> +#define ALTR_A10SR_OP_FLAG_BIT_POS      ALTR_A10SR_PG1_OP_FLAG_SHIFT
> +/* 2nd register needs an offset of 8 to get to 2nd register */
> +#define ALTR_A10SR_FBC2MP_BIT_POS       (8 + ALTR_A10SR_PG2_FBC2MP_SHIFT)
> +#define ALTR_A10SR_FAC2MP_BIT_POS       (8 + ALTR_A10SR_PG2_FAC2MP_SHIFT)
> +#define ALTR_A10SR_FMCBVADJ_BIT_POS     (8 + ALTR_A10SR_PG2_FMCBVADJ_SHIFT)
> +#define ALTR_A10SR_FMCAVADJ_BIT_POS     (8 + ALTR_A10SR_PG2_FMCAVADJ_SHIFT)
> +#define ALTR_A10SR_HL_VDDQ_BIT_POS      (8 + ALTR_A10SR_PG2_HL_VDDQ_SHIFT)
> +#define ALTR_A10SR_HL_VDD_BIT_POS       (8 + ALTR_A10SR_PG2_HL_VDD_SHIFT)
> +#define ALTR_A10SR_HL_HPS_BIT_POS       (8 + ALTR_A10SR_PG2_HL_HPS_SHIFT)
> +#define ALTR_A10SR_HPS_BIT_POS          (8 + ALTR_A10SR_PG2_HPS_SHIFT)
> +/* 3rd register needs an offset of 16 to get to 3rd register */
> +#define ALTR_A10SR_PCIE_WAKE_BIT_POS    (16 + ALTR_A10SR_PG3_PCIE_WAKE_SHIFT)
> +#define ALTR_A10SR_PCIE_PR_BIT_POS      (16 + ALTR_A10SR_PG3_PCIE_PR_SHIFT)
> +#define ALTR_A10SR_FMCB_PR_BIT_POS      (16 + ALTR_A10SR_PG3_FMCB_PR_SHIFT)
> +#define ALTR_A10SR_FMCA_PR_BIT_POS      (16 + ALTR_A10SR_PG3_FMCA_PR_SHIFT)
> +#define ALTR_A10SR_FILE_PR_BIT_POS      (16 + ALTR_A10SR_PG3_FILE_PR_SHIFT)
> +#define ALTR_A10SR_BF_PR_BIT_POS        (16 + ALTR_A10SR_PG3_BF_PR_SHIFT)
> +#define ALTR_A10SR_10V_FAIL_BIT_POS     (16 + ALTR_A10SR_PG3_10V_FAIL_SHIFT)
> +#define ALTR_A10SR_FAM2C_BIT_POS        (16 + ALTR_A10SR_PG3_FAM2C_SHIFT)
> +/* FMCA/B & PCIE Enables need an offset of 24 */
> +#define ALTR_A10SR_FMCB_AUXEN_POS       (24 + ALTR_A10SR_FMCB_AUXEN_SHIFT)
> +#define ALTR_A10SR_FMCB_EN_POS          (24 + ALTR_A10SR_FMCB_EN_SHIFT)
> +#define ALTR_A10SR_FMCA_AUXEN_POS       (24 + ALTR_A10SR_FMCA_AUXEN_SHIFT)
> +#define ALTR_A10SR_FMCA_EN_POS          (24 + ALTR_A10SR_FMCA_EN_SHIFT)
> +#define ALTR_A10SR_PCIE_AUXEN_POS       (24 + ALTR_A10SR_PCIE_AUXEN_SHIFT)
> +#define ALTR_A10SR_PCIE_EN_POS          (24 + ALTR_A10SR_PCIE_EN_SHIFT)
> +/* HPS Resets need an offset of 32 */
> +#define ALTR_A10SR_HPS_RST_UART_POS     (32 + ALTR_A10SR_HPS_UARTA_RSTN_SHIFT)
> +#define ALTR_A10SR_HPS_RST_WARM_POS     (32 + ALTR_A10SR_HPS_WARM_RSTN_SHIFT)
> +#define ALTR_A10SR_HPS_RST_WARM1_POS    (32 + ALTR_A10SR_HPS_WARM_RST1N_SHIFT)
> +#define ALTR_A10SR_HPS_RST_COLD_POS     (32 + ALTR_A10SR_HPS_COLD_RSTN_SHIFT)
> +#define ALTR_A10SR_HPS_RST_NPOR_POS     (32 + ALTR_A10SR_HPS_NPOR_SHIFT)
> +#define ALTR_A10SR_HPS_RST_NRST_POS     (32 + ALTR_A10SR_HPS_NRST_SHIFT)
> +#define ALTR_A10SR_HPS_RST_ENET_POS     (32 + ALTR_A10SR_HPS_ENET_RSTN_SHIFT)
> +#define ALTR_A10SR_HPS_RST_ENETINT_POS  (32 + ALTR_A10SR_HPS_ENET_INTN_SHIFT)
> +/* Peripheral Resets need an offset of 40 */
> +#define ALTR_A10SR_PER_RST_USB_POS      (40 + ALTR_A10SR_USB_RST_SHIFT)
> +#define ALTR_A10SR_PER_RST_BQSPI_POS    (40 + ALTR_A10SR_BQSPI_RST_N_SHIFT)
> +#define ALTR_A10SR_PER_RST_FILE_POS     (40 + ALTR_A10SR_FILE_RST_N_SHIFT)
> +#define ALTR_A10SR_PER_RST_PCIE_POS     (40 + ALTR_A10SR_PCIE_PERST_N_SHIFT)
> +/* HWMON - Read Entire Register */
> +#define ALTR_A10SR_ENTIRE_REG           (88)
> +#define ALTR_A10SR_ENTIRE_REG_MASK      (0xFF)
> +#define ALTR_A10SR_VERSION              (0 + ALTR_A10SR_ENTIRE_REG)
> +#define ALTR_A10SR_LED                  (1 + ALTR_A10SR_ENTIRE_REG)
> +#define ALTR_A10SR_PB                   (2 + ALTR_A10SR_ENTIRE_REG)
> +#define ALTR_A10SR_PBF                  (3 + ALTR_A10SR_ENTIRE_REG)
> +#define ALTR_A10SR_PG1                  (4 + ALTR_A10SR_ENTIRE_REG)
> +#define ALTR_A10SR_PG2                  (5 + ALTR_A10SR_ENTIRE_REG)
> +#define ALTR_A10SR_PG3                  (6 + ALTR_A10SR_ENTIRE_REG)
> +#define ALTR_A10SR_FMCAB                (7 + ALTR_A10SR_ENTIRE_REG)
> +#define ALTR_A10SR_HPS_RST              (8 + ALTR_A10SR_ENTIRE_REG)
> +#define ALTR_A10SR_PER_RST              (9 + ALTR_A10SR_ENTIRE_REG)
> +#define ALTR_A10SR_SFPA                 (10 + ALTR_A10SR_ENTIRE_REG)
> +#define ALTR_A10SR_SFPB                 (11 + ALTR_A10SR_ENTIRE_REG)
> +#define ALTR_A10SR_I2C_MASTER           (12 + ALTR_A10SR_ENTIRE_REG)
> +#define ALTR_A10SR_WARM_RST             (13 + ALTR_A10SR_ENTIRE_REG)
> +#define ALTR_A10SR_WARM_RST_KEY         (14 + ALTR_A10SR_ENTIRE_REG)
> +#define ALTR_A10SR_PMBUS                (15 + ALTR_A10SR_ENTIRE_REG)
> +
> +struct altr_a10sr_hwmon {
> +	struct altr_a10sr	*a10sr;
> +	struct device		*class_device;
> +};
> +
> +static const char *const hwmon_names[] = {
> +	[ALTR_A10SR_1V0_BIT_POS]       = "1.0V PWR Good",
> +	[ALTR_A10SR_0V95_BIT_POS]      = "0.95V PWR Good",
> +	[ALTR_A10SR_0V9_BIT_POS]       = "0.9V PWR Good",
> +	[ALTR_A10SR_5V0_BIT_POS]       = "5.0V PWR Good",
> +	[ALTR_A10SR_3V3_BIT_POS]       = "3.3V PWR Good",
> +	[ALTR_A10SR_2V5_BIT_POS]       = "2.5V PWR Good",
> +	[ALTR_A10SR_1V8_BIT_POS]       = "1.8V PWR Good",
> +	[ALTR_A10SR_OP_FLAG_BIT_POS]   = "PWR On Complete",
> +
> +	[ALTR_A10SR_FBC2MP_BIT_POS]    = "FBC2MP PWR Good",
> +	[ALTR_A10SR_FAC2MP_BIT_POS]    = "FAC2MP PWR Good",
> +	[ALTR_A10SR_FMCBVADJ_BIT_POS]  = "FMCBVADJ PWR Good",
> +	[ALTR_A10SR_FMCAVADJ_BIT_POS]  = "FMCAVADJ PWR Good",
> +	[ALTR_A10SR_HL_VDDQ_BIT_POS]   = "HILO VDDQ PWR Good",
> +	[ALTR_A10SR_HL_VDD_BIT_POS]    = "HILO VDD PWR Good",
> +	[ALTR_A10SR_HL_HPS_BIT_POS]    = "HILO HPS PWR Good",
> +	[ALTR_A10SR_HPS_BIT_POS]       = "HPS PWR Good",
> +
> +	[ALTR_A10SR_PCIE_WAKE_BIT_POS] = "PCIE WAKEn",
> +	[ALTR_A10SR_PCIE_PR_BIT_POS]   = "PCIE PRESENTn",
> +	[ALTR_A10SR_FMCB_PR_BIT_POS]   = "FMCB PRESENTn",
> +	[ALTR_A10SR_FMCA_PR_BIT_POS]   = "FMCA PRESENTn",
> +	[ALTR_A10SR_FILE_PR_BIT_POS]   = "FILE PRESENTn",
> +	[ALTR_A10SR_BF_PR_BIT_POS]     = "BF PRESENTn",
> +	[ALTR_A10SR_10V_FAIL_BIT_POS]  = "10V FAILn",
> +	[ALTR_A10SR_FAM2C_BIT_POS]     = "FAM2C PWR Good",
> +};
> +
> +static ssize_t altr_a10sr_read_status(struct device *dev,
> +				      struct device_attribute *devattr,
> +				      char *buf)
> +{
> +	struct altr_a10sr_hwmon *hwmon = dev_get_drvdata(dev);
> +	int ret, index = to_sensor_dev_attr(devattr)->index;
> +	int mask = ALTR_A10SR_REG_BIT_MASK(index);
> +	unsigned char reg = ALTR_A10SR_PWR_GOOD1_RD_REG +
> +			    ALTR_A10SR_REG_OFFSET(index);
> +
> +	/* Check if this is an entire register read */
> +	if (index >= ALTR_A10SR_ENTIRE_REG) {
> +		reg = ((index - ALTR_A10SR_ENTIRE_REG) << 1) + 1;
> +		mask = ALTR_A10SR_ENTIRE_REG_MASK;
> +	}
> +
> +	ret = altr_a10sr_reg_read(hwmon->a10sr, reg);
> +	if (ret < 0)
> +		return ret;
> +
> +	return sprintf(buf, "0x%X\n", (ret & mask));
> +}
> +
> +static ssize_t altr_a10sr_hwmon_show_name(struct device *dev,
> +					  struct device_attribute *devattr,
> +					  char *buf)
> +{
> +	return sprintf(buf, "altr_a10sr\n");
> +}
> +
> +static ssize_t show_label(struct device *dev,
> +			  struct device_attribute *devattr, char *buf)
> +{
> +	return sprintf(buf, "%s\n",
> +		       hwmon_names[to_sensor_dev_attr(devattr)->index]);
> +}
> +
> +static ssize_t set_enable(struct device *dev,
> +			  struct device_attribute *dev_attr,
> +			  const char *buf, size_t count)
> +{
> +	unsigned long val;
> +	struct altr_a10sr_hwmon *hwmon = dev_get_drvdata(dev);
> +	int ret, index = to_sensor_dev_attr(dev_attr)->index;
> +	int mask = ALTR_A10SR_REG_BIT_MASK(index);
> +	unsigned char reg = (ALTR_A10SR_PWR_GOOD1_RD_REG & WRITE_REG_MASK) +
> +			    ALTR_A10SR_REG_OFFSET(index);
> +	int res = kstrtol(buf, 10, &val);
> +
> +	if (res < 0)
> +		return res;
> +
> +	/* Check if this is an entire register write */
> +	if (index >= ALTR_A10SR_ENTIRE_REG) {
> +		reg = ((index - ALTR_A10SR_ENTIRE_REG) << 1);
> +		mask = ALTR_A10SR_ENTIRE_REG_MASK;
> +	}
> +
> +	ret = altr_a10sr_reg_update(hwmon->a10sr, reg, mask, val);
> +	if (ret < 0)
> +		return ret;
> +
> +	return count;
> +}
> +
> +/* First Power Good Register Bits */
> +static SENSOR_DEVICE_ATTR(1v0_input, S_IRUGO, altr_a10sr_read_status, NULL,
> +			  ALTR_A10SR_1V0_BIT_POS);
> +static SENSOR_DEVICE_ATTR(1v0_label, S_IRUGO, show_label, NULL,
> +			  ALTR_A10SR_1V0_BIT_POS);
> +static SENSOR_DEVICE_ATTR(0v95_input, S_IRUGO, altr_a10sr_read_status, NULL,
> +			  ALTR_A10SR_0V95_BIT_POS);
> +static SENSOR_DEVICE_ATTR(0v95_label, S_IRUGO, show_label, NULL,
> +			  ALTR_A10SR_0V95_BIT_POS);
> +static SENSOR_DEVICE_ATTR(0v9_input, S_IRUGO, altr_a10sr_read_status, NULL,
> +			  ALTR_A10SR_0V9_BIT_POS);
> +static SENSOR_DEVICE_ATTR(0v9_label, S_IRUGO, show_label, NULL,
> +			  ALTR_A10SR_0V9_BIT_POS);
> +static SENSOR_DEVICE_ATTR(5v0_input, S_IRUGO, altr_a10sr_read_status, NULL,
> +			  ALTR_A10SR_5V0_BIT_POS);
> +static SENSOR_DEVICE_ATTR(5v0_label, S_IRUGO, show_label, NULL,
> +			  ALTR_A10SR_5V0_BIT_POS);
> +static SENSOR_DEVICE_ATTR(3v3_input, S_IRUGO, altr_a10sr_read_status, NULL,
> +			  ALTR_A10SR_3V3_BIT_POS);
> +static SENSOR_DEVICE_ATTR(3v3_label, S_IRUGO, show_label, NULL,
> +			  ALTR_A10SR_3V3_BIT_POS);
> +static SENSOR_DEVICE_ATTR(2v5_input, S_IRUGO, altr_a10sr_read_status, NULL,
> +			  ALTR_A10SR_2V5_BIT_POS);
> +static SENSOR_DEVICE_ATTR(2v5_label, S_IRUGO, show_label, NULL,
> +			  ALTR_A10SR_2V5_BIT_POS);
> +static SENSOR_DEVICE_ATTR(1v8_input, S_IRUGO, altr_a10sr_read_status, NULL,
> +			  ALTR_A10SR_1V8_BIT_POS);
> +static SENSOR_DEVICE_ATTR(1v8_label, S_IRUGO, show_label, NULL,
> +			  ALTR_A10SR_1V8_BIT_POS);
> +static SENSOR_DEVICE_ATTR(opflag_input, S_IRUGO, altr_a10sr_read_status, NULL,
> +			  ALTR_A10SR_OP_FLAG_BIT_POS);
> +static SENSOR_DEVICE_ATTR(opflag_label, S_IRUGO, show_label, NULL,
> +			  ALTR_A10SR_OP_FLAG_BIT_POS);
> +/* Second Power Good Register Bits */
> +static SENSOR_DEVICE_ATTR(fbc2mp_input, S_IRUGO, altr_a10sr_read_status, NULL,
> +			  ALTR_A10SR_FBC2MP_BIT_POS);
> +static SENSOR_DEVICE_ATTR(fbc2mp_label, S_IRUGO, show_label, NULL,
> +			  ALTR_A10SR_FBC2MP_BIT_POS);
> +static SENSOR_DEVICE_ATTR(fac2mp_input, S_IRUGO, altr_a10sr_read_status, NULL,
> +			  ALTR_A10SR_FAC2MP_BIT_POS);
> +static SENSOR_DEVICE_ATTR(fac2mp_label, S_IRUGO, show_label, NULL,
> +			  ALTR_A10SR_FAC2MP_BIT_POS);
> +static SENSOR_DEVICE_ATTR(fmcbvadj_input, S_IRUGO, altr_a10sr_read_status, NULL,
> +			  ALTR_A10SR_FMCBVADJ_BIT_POS);
> +static SENSOR_DEVICE_ATTR(fmcbvadj_label, S_IRUGO, show_label, NULL,
> +			  ALTR_A10SR_FMCBVADJ_BIT_POS);
> +static SENSOR_DEVICE_ATTR(fmcavadj_input, S_IRUGO, altr_a10sr_read_status, NULL,
> +			  ALTR_A10SR_FMCAVADJ_BIT_POS);
> +static SENSOR_DEVICE_ATTR(fmcavadj_label, S_IRUGO, show_label, NULL,
> +			  ALTR_A10SR_FMCAVADJ_BIT_POS);
> +static SENSOR_DEVICE_ATTR(hl_vddq_input, S_IRUGO, altr_a10sr_read_status, NULL,
> +			  ALTR_A10SR_HL_VDDQ_BIT_POS);
> +static SENSOR_DEVICE_ATTR(hl_vddq_label, S_IRUGO, show_label, NULL,
> +			  ALTR_A10SR_HL_VDDQ_BIT_POS);
> +static SENSOR_DEVICE_ATTR(hl_vdd_input, S_IRUGO, altr_a10sr_read_status, NULL,
> +			  ALTR_A10SR_HL_VDD_BIT_POS);
> +static SENSOR_DEVICE_ATTR(hl_vdd_label, S_IRUGO, show_label, NULL,
> +			  ALTR_A10SR_HL_VDD_BIT_POS);
> +static SENSOR_DEVICE_ATTR(hlhps_vdd_input, S_IRUGO, altr_a10sr_read_status,
> +			  NULL, ALTR_A10SR_HL_HPS_BIT_POS);
> +static SENSOR_DEVICE_ATTR(hlhps_vdd_label, S_IRUGO, show_label, NULL,
> +			  ALTR_A10SR_HL_HPS_BIT_POS);
> +static SENSOR_DEVICE_ATTR(hps_input, S_IRUGO, altr_a10sr_read_status, NULL,
> +			  ALTR_A10SR_HPS_BIT_POS);
> +static SENSOR_DEVICE_ATTR(hps_label, S_IRUGO, show_label, NULL,
> +			  ALTR_A10SR_HPS_BIT_POS);
> +/* Third Power Good Register Bits */
> +static SENSOR_DEVICE_ATTR(pcie_wake_input, S_IRUGO, altr_a10sr_read_status,
> +			  NULL, ALTR_A10SR_PCIE_WAKE_BIT_POS);
> +static SENSOR_DEVICE_ATTR(pcie_wake_label, S_IRUGO, show_label, NULL,
> +			  ALTR_A10SR_PCIE_WAKE_BIT_POS);
> +static SENSOR_DEVICE_ATTR(pcie_pr_input, S_IRUGO, altr_a10sr_read_status, NULL,
> +			  ALTR_A10SR_PCIE_PR_BIT_POS);
> +static SENSOR_DEVICE_ATTR(pcie_pr_label, S_IRUGO, show_label, NULL,
> +			  ALTR_A10SR_PCIE_PR_BIT_POS);
> +static SENSOR_DEVICE_ATTR(fmcb_pr_input, S_IRUGO, altr_a10sr_read_status, NULL,
> +			  ALTR_A10SR_FMCB_PR_BIT_POS);
> +static SENSOR_DEVICE_ATTR(fmcb_pr_label, S_IRUGO, show_label, NULL,
> +			  ALTR_A10SR_FMCB_PR_BIT_POS);
> +static SENSOR_DEVICE_ATTR(fmca_pr_input, S_IRUGO, altr_a10sr_read_status, NULL,
> +			  ALTR_A10SR_FMCA_PR_BIT_POS);
> +static SENSOR_DEVICE_ATTR(fmca_pr_label, S_IRUGO, show_label, NULL,
> +			  ALTR_A10SR_FMCA_PR_BIT_POS);
> +static SENSOR_DEVICE_ATTR(file_pr_input, S_IRUGO, altr_a10sr_read_status, NULL,
> +			  ALTR_A10SR_FILE_PR_BIT_POS);
> +static SENSOR_DEVICE_ATTR(file_pr_label, S_IRUGO, show_label, NULL,
> +			  ALTR_A10SR_FILE_PR_BIT_POS);
> +static SENSOR_DEVICE_ATTR(bf_pr_input, S_IRUGO, altr_a10sr_read_status, NULL,
> +			  ALTR_A10SR_BF_PR_BIT_POS);
> +static SENSOR_DEVICE_ATTR(bf_pr_label, S_IRUGO, show_label, NULL,
> +			  ALTR_A10SR_BF_PR_BIT_POS);
> +static SENSOR_DEVICE_ATTR(10v_fail_input, S_IRUGO, altr_a10sr_read_status,
> +			  NULL, ALTR_A10SR_10V_FAIL_BIT_POS);
> +static SENSOR_DEVICE_ATTR(10v_fail_label, S_IRUGO, show_label, NULL,
> +			  ALTR_A10SR_10V_FAIL_BIT_POS);
> +static SENSOR_DEVICE_ATTR(fam2c_input, S_IRUGO, altr_a10sr_read_status, NULL,
> +			  ALTR_A10SR_FAM2C_BIT_POS);
> +static SENSOR_DEVICE_ATTR(fam2c_label, S_IRUGO, show_label, NULL,
> +			  ALTR_A10SR_FAM2C_BIT_POS);
> +/* Peripheral Enable bits */
> +static SENSOR_DEVICE_ATTR(fmcb_aux_en, S_IRUGO | S_IWUSR,
> +			  altr_a10sr_read_status, set_enable,
> +			  ALTR_A10SR_FMCB_AUXEN_POS);
> +static SENSOR_DEVICE_ATTR(fmcb_en, S_IRUGO | S_IWUSR,
> +			  altr_a10sr_read_status, set_enable,
> +			  ALTR_A10SR_FMCB_EN_POS);
> +static SENSOR_DEVICE_ATTR(fmca_aux_en, S_IRUGO | S_IWUSR,
> +			  altr_a10sr_read_status, set_enable,
> +			  ALTR_A10SR_FMCA_AUXEN_POS);
> +static SENSOR_DEVICE_ATTR(fmca_en, S_IRUGO | S_IWUSR,
> +			  altr_a10sr_read_status, set_enable,
> +			  ALTR_A10SR_FMCA_EN_POS);
> +static SENSOR_DEVICE_ATTR(pcie_aux_en, S_IRUGO | S_IWUSR,
> +			  altr_a10sr_read_status, set_enable,
> +			  ALTR_A10SR_PCIE_AUXEN_POS);
> +static SENSOR_DEVICE_ATTR(pcie_en, S_IRUGO | S_IWUSR,
> +			  altr_a10sr_read_status, set_enable,
> +			  ALTR_A10SR_PCIE_EN_POS);
> +/* HPS Reset bits */
> +static SENSOR_DEVICE_ATTR(hps_uart_rst, S_IRUGO,
> +			  altr_a10sr_read_status, set_enable,
> +			  ALTR_A10SR_HPS_RST_UART_POS);
> +static SENSOR_DEVICE_ATTR(hps_warm_rst, S_IRUGO,
> +			  altr_a10sr_read_status, set_enable,
> +			  ALTR_A10SR_HPS_RST_WARM_POS);
> +static SENSOR_DEVICE_ATTR(hps_warm1_rst, S_IRUGO,
> +			  altr_a10sr_read_status, set_enable,
> +			  ALTR_A10SR_HPS_RST_WARM1_POS);
> +static SENSOR_DEVICE_ATTR(hps_cold_rst, S_IRUGO,
> +			  altr_a10sr_read_status, set_enable,
> +			  ALTR_A10SR_HPS_RST_COLD_POS);
> +static SENSOR_DEVICE_ATTR(hps_npor, S_IRUGO,
> +			  altr_a10sr_read_status, set_enable,
> +			  ALTR_A10SR_HPS_RST_NPOR_POS);
> +static SENSOR_DEVICE_ATTR(hps_nrst, S_IRUGO,
> +			  altr_a10sr_read_status, set_enable,
> +			  ALTR_A10SR_HPS_RST_NRST_POS);
> +static SENSOR_DEVICE_ATTR(hps_enet_rst, S_IRUGO | S_IWUSR,
> +			  altr_a10sr_read_status, set_enable,
> +			  ALTR_A10SR_HPS_RST_ENET_POS);
> +static SENSOR_DEVICE_ATTR(hps_enet_int, S_IRUGO | S_IWUSR,
> +			  altr_a10sr_read_status, set_enable,
> +			  ALTR_A10SR_HPS_RST_ENETINT_POS);
> +/* Peripheral Reset bits */
> +static SENSOR_DEVICE_ATTR(usb_reset, S_IRUGO | S_IWUSR, altr_a10sr_read_status,
> +			  set_enable, ALTR_A10SR_PER_RST_USB_POS);
> +static SENSOR_DEVICE_ATTR(bqspi_resetn, S_IRUGO | S_IWUSR,
> +			  altr_a10sr_read_status, set_enable,
> +			  ALTR_A10SR_PER_RST_BQSPI_POS);
> +static SENSOR_DEVICE_ATTR(file_resetn, S_IRUGO | S_IWUSR,
> +			  altr_a10sr_read_status, set_enable,
> +			  ALTR_A10SR_PER_RST_FILE_POS);
> +static SENSOR_DEVICE_ATTR(pcie_perstn, S_IRUGO | S_IWUSR,
> +			  altr_a10sr_read_status, set_enable,
> +			  ALTR_A10SR_PER_RST_PCIE_POS);
> +/* Entire Byte Read */
> +static SENSOR_DEVICE_ATTR(max5_version, S_IRUGO, altr_a10sr_read_status,
> +			  NULL, ALTR_A10SR_VERSION);
> +static SENSOR_DEVICE_ATTR(max5_led, S_IRUGO, altr_a10sr_read_status,
> +			  NULL, ALTR_A10SR_LED);
> +static SENSOR_DEVICE_ATTR(max5_button, S_IRUGO, altr_a10sr_read_status,
> +			  NULL, ALTR_A10SR_PB);
> +static SENSOR_DEVICE_ATTR(max5_button_irq, S_IRUGO | S_IWUSR,
> +			  altr_a10sr_read_status, set_enable, ALTR_A10SR_PBF);
> +static SENSOR_DEVICE_ATTR(max5_pg1, S_IRUGO, altr_a10sr_read_status,
> +			  NULL, ALTR_A10SR_PG1);
> +static SENSOR_DEVICE_ATTR(max5_pg2, S_IRUGO, altr_a10sr_read_status,
> +			  NULL, ALTR_A10SR_PG2);
> +static SENSOR_DEVICE_ATTR(max5_pg3, S_IRUGO, altr_a10sr_read_status,
> +			  NULL, ALTR_A10SR_PG3);
> +static SENSOR_DEVICE_ATTR(max5_fmcab, S_IRUGO, altr_a10sr_read_status,
> +			  NULL, ALTR_A10SR_FMCAB);
> +static SENSOR_DEVICE_ATTR(max5_hps_resets, S_IRUGO | S_IWUSR,
> +			  altr_a10sr_read_status, set_enable,
> +			  ALTR_A10SR_HPS_RST);
> +static SENSOR_DEVICE_ATTR(max5_per_resets, S_IRUGO | S_IWUSR,
> +			  altr_a10sr_read_status, set_enable,
> +			  ALTR_A10SR_PER_RST);
> +static SENSOR_DEVICE_ATTR(max5_sfpa, S_IRUGO | S_IWUSR,
> +			  altr_a10sr_read_status, set_enable, ALTR_A10SR_SFPA);
> +static SENSOR_DEVICE_ATTR(max5_sfpb, S_IRUGO | S_IWUSR,
> +			  altr_a10sr_read_status, set_enable, ALTR_A10SR_SFPB);
> +static SENSOR_DEVICE_ATTR(max5_i2c_master, S_IRUGO | S_IWUSR,
> +			  altr_a10sr_read_status, set_enable,
> +			  ALTR_A10SR_I2C_MASTER);
> +static SENSOR_DEVICE_ATTR(max5_pmbus, S_IRUGO | S_IWUSR,
> +			  altr_a10sr_read_status, set_enable,
> +			  ALTR_A10SR_PMBUS);
> +
> +static DEVICE_ATTR(name, S_IRUGO, altr_a10sr_hwmon_show_name, NULL);
> +
> +static struct attribute *altr_a10sr_attr[] = {
> +	&dev_attr_name.attr,
> +	/* First Power Good Register */
> +	&sensor_dev_attr_1v0_input.dev_attr.attr,
> +	&sensor_dev_attr_1v0_label.dev_attr.attr,
> +	&sensor_dev_attr_0v95_input.dev_attr.attr,
> +	&sensor_dev_attr_0v95_label.dev_attr.attr,
> +	&sensor_dev_attr_0v9_input.dev_attr.attr,
> +	&sensor_dev_attr_0v9_label.dev_attr.attr,
> +	&sensor_dev_attr_5v0_input.dev_attr.attr,
> +	&sensor_dev_attr_5v0_label.dev_attr.attr,
> +	&sensor_dev_attr_3v3_input.dev_attr.attr,
> +	&sensor_dev_attr_3v3_label.dev_attr.attr,
> +	&sensor_dev_attr_2v5_input.dev_attr.attr,
> +	&sensor_dev_attr_2v5_label.dev_attr.attr,
> +	&sensor_dev_attr_1v8_input.dev_attr.attr,
> +	&sensor_dev_attr_1v8_label.dev_attr.attr,
> +	&sensor_dev_attr_opflag_input.dev_attr.attr,
> +	&sensor_dev_attr_opflag_label.dev_attr.attr,
> +	/* Second Power Good Register */
> +	&sensor_dev_attr_fbc2mp_input.dev_attr.attr,
> +	&sensor_dev_attr_fbc2mp_label.dev_attr.attr,
> +	&sensor_dev_attr_fac2mp_input.dev_attr.attr,
> +	&sensor_dev_attr_fac2mp_label.dev_attr.attr,
> +	&sensor_dev_attr_fmcbvadj_input.dev_attr.attr,
> +	&sensor_dev_attr_fmcbvadj_label.dev_attr.attr,
> +	&sensor_dev_attr_fmcavadj_input.dev_attr.attr,
> +	&sensor_dev_attr_fmcavadj_label.dev_attr.attr,
> +	&sensor_dev_attr_hl_vddq_input.dev_attr.attr,
> +	&sensor_dev_attr_hl_vddq_label.dev_attr.attr,
> +	&sensor_dev_attr_hl_vdd_input.dev_attr.attr,
> +	&sensor_dev_attr_hl_vdd_label.dev_attr.attr,
> +	&sensor_dev_attr_hlhps_vdd_input.dev_attr.attr,
> +	&sensor_dev_attr_hlhps_vdd_label.dev_attr.attr,
> +	&sensor_dev_attr_hps_input.dev_attr.attr,
> +	&sensor_dev_attr_hps_label.dev_attr.attr,
> +	/* Third Power Good Register */
> +	&sensor_dev_attr_pcie_wake_input.dev_attr.attr,
> +	&sensor_dev_attr_pcie_wake_label.dev_attr.attr,
> +	&sensor_dev_attr_pcie_pr_input.dev_attr.attr,
> +	&sensor_dev_attr_pcie_pr_label.dev_attr.attr,
> +	&sensor_dev_attr_fmcb_pr_input.dev_attr.attr,
> +	&sensor_dev_attr_fmcb_pr_label.dev_attr.attr,
> +	&sensor_dev_attr_fmca_pr_input.dev_attr.attr,
> +	&sensor_dev_attr_fmca_pr_label.dev_attr.attr,
> +	&sensor_dev_attr_file_pr_input.dev_attr.attr,
> +	&sensor_dev_attr_file_pr_label.dev_attr.attr,
> +	&sensor_dev_attr_bf_pr_input.dev_attr.attr,
> +	&sensor_dev_attr_bf_pr_label.dev_attr.attr,
> +	&sensor_dev_attr_10v_fail_input.dev_attr.attr,
> +	&sensor_dev_attr_10v_fail_label.dev_attr.attr,
> +	&sensor_dev_attr_fam2c_input.dev_attr.attr,
> +	&sensor_dev_attr_fam2c_label.dev_attr.attr,
> +	/* Peripheral Enable Register */
> +	&sensor_dev_attr_fmcb_aux_en.dev_attr.attr,
> +	&sensor_dev_attr_fmcb_en.dev_attr.attr,
> +	&sensor_dev_attr_fmca_aux_en.dev_attr.attr,
> +	&sensor_dev_attr_fmca_en.dev_attr.attr,
> +	&sensor_dev_attr_pcie_aux_en.dev_attr.attr,
> +	&sensor_dev_attr_pcie_en.dev_attr.attr,
> +	/* HPS Reset bits */
> +	&sensor_dev_attr_hps_uart_rst.dev_attr.attr,
> +	&sensor_dev_attr_hps_warm_rst.dev_attr.attr,
> +	&sensor_dev_attr_hps_warm1_rst.dev_attr.attr,
> +	&sensor_dev_attr_hps_cold_rst.dev_attr.attr,
> +	&sensor_dev_attr_hps_npor.dev_attr.attr,
> +	&sensor_dev_attr_hps_nrst.dev_attr.attr,
> +	&sensor_dev_attr_hps_enet_rst.dev_attr.attr,
> +	&sensor_dev_attr_hps_enet_int.dev_attr.attr,
> +	/* Peripheral Reset bits */
> +	&sensor_dev_attr_usb_reset.dev_attr.attr,
> +	&sensor_dev_attr_bqspi_resetn.dev_attr.attr,
> +	&sensor_dev_attr_file_resetn.dev_attr.attr,
> +	&sensor_dev_attr_pcie_perstn.dev_attr.attr,
> +	/* Byte Value Register */
> +	&sensor_dev_attr_max5_version.dev_attr.attr,
> +	&sensor_dev_attr_max5_led.dev_attr.attr,
> +	&sensor_dev_attr_max5_button.dev_attr.attr,
> +	&sensor_dev_attr_max5_button_irq.dev_attr.attr,
> +	&sensor_dev_attr_max5_pg1.dev_attr.attr,
> +	&sensor_dev_attr_max5_pg2.dev_attr.attr,
> +	&sensor_dev_attr_max5_pg3.dev_attr.attr,
> +	&sensor_dev_attr_max5_fmcab.dev_attr.attr,
> +	&sensor_dev_attr_max5_hps_resets.dev_attr.attr,
> +	&sensor_dev_attr_max5_per_resets.dev_attr.attr,
> +	&sensor_dev_attr_max5_sfpa.dev_attr.attr,
> +	&sensor_dev_attr_max5_sfpb.dev_attr.attr,
> +	&sensor_dev_attr_max5_i2c_master.dev_attr.attr,
> +	&sensor_dev_attr_max5_pmbus.dev_attr.attr,
> +	NULL
> +};
> +
> +static const struct attribute_group altr_a10sr_attr_group = {
> +	.attrs = altr_a10sr_attr
> +};
> +
> +static int altr_a10sr_hwmon_probe(struct platform_device *pdev)
> +{
> +	struct altr_a10sr_hwmon *hwmon;
> +	int ret;
> +
> +	hwmon = devm_kzalloc(&pdev->dev, sizeof(*hwmon), GFP_KERNEL);
> +	if (!hwmon)
> +		return -ENOMEM;
> +
> +	hwmon->a10sr = dev_get_drvdata(pdev->dev.parent);
> +
> +	platform_set_drvdata(pdev, hwmon);
> +
> +	ret = sysfs_create_group(&pdev->dev.kobj, &altr_a10sr_attr_group);
> +	if (ret)
> +		goto err_mem;
> +
> +	hwmon->class_device = hwmon_device_register(&pdev->dev);
> +	if (IS_ERR(hwmon->class_device)) {
> +		ret = PTR_ERR(hwmon->class_device);
> +		goto err_sysfs;
> +	}
> +
> +	return 0;
> +
> +err_sysfs:
> +	sysfs_remove_group(&pdev->dev.kobj, &altr_a10sr_attr_group);
> +err_mem:
> +	return ret;
> +}
> +
> +static int altr_a10sr_hwmon_remove(struct platform_device *pdev)
> +{
> +	struct altr_a10sr_hwmon *hwmon = platform_get_drvdata(pdev);
> +
> +	hwmon_device_unregister(hwmon->class_device);
> +	sysfs_remove_group(&pdev->dev.kobj, &altr_a10sr_attr_group);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id altr_a10sr_hwmon_of_match[] = {
> +	{ .compatible = "altr,a10sr-hwmon" },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, altr_a10sr_hwmon_of_match);
> +
> +static struct platform_driver altr_a10sr_hwmon_driver = {
> +	.probe = altr_a10sr_hwmon_probe,
> +	.remove = altr_a10sr_hwmon_remove,
> +	.driver = {
> +		.name = "altr_a10sr_hwmon",
> +		.of_match_table = altr_a10sr_hwmon_of_match,
> +	},
> +};
> +
> +module_platform_driver(altr_a10sr_hwmon_driver);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR("Thor Thayer");
> +MODULE_DESCRIPTION("HW Monitor driver for Altera Arria10 System Resource Chip");
> diff --git a/drivers/mfd/altera-a10sr.c b/drivers/mfd/altera-a10sr.c
> index 517b895..3eedad7 100644
> --- a/drivers/mfd/altera-a10sr.c
> +++ b/drivers/mfd/altera-a10sr.c
> @@ -34,6 +34,10 @@ static const struct mfd_cell altr_a10sr_subdev_info[] = {
>  		.name = "altr_a10sr_gpio",
>  		.of_compatible = "altr,a10sr-gpio",
>  	},
> +	{
> +		.name = "altr_a10sr_hwmon",
> +		.of_compatible = "altr,a10sr-hwmon",
> +	},
>  };
>  
>  static bool altr_a10sr_reg_readable(struct device *dev, unsigned int reg)
> diff --git a/include/linux/mfd/altera-a10sr.h b/include/linux/mfd/altera-a10sr.h
> index 6d254a1..2bfc63e 100644
> --- a/include/linux/mfd/altera-a10sr.h
> +++ b/include/linux/mfd/altera-a10sr.h
> @@ -75,26 +75,93 @@
>  #define ALTR_A10SR_IN_VALID_RANGE_LO      8
>  #define ALTR_A10SR_IN_VALID_RANGE_HI      15
>  
> -#define ALTR_A10SR_PWR_GOOD1_RD_REG   0x09    /* Power Good1 Read */
> -#define ALTR_A10SR_PWR_GOOD2_RD_REG   0x0B    /* Power Good2 Read */
> -#define ALTR_A10SR_PWR_GOOD3_RD_REG   0x0D    /* Power Good3 Read */
> -#define ALTR_A10SR_FMCAB_WR_REG       0x0E    /* FMCA/B & PCIe Pwr Enable */
> -#define ALTR_A10SR_FMCAB_RD_REG       0x0F    /* FMCA/B & PCIe Pwr Enable */
> -#define ALTR_A10SR_HPS_RST_WR_REG     0x10    /* HPS Reset */
> -#define ALTR_A10SR_HPS_RST_RD_REG     0x11    /* HPS Reset */
> -#define ALTR_A10SR_USB_QSPI_WR_REG    0x12    /* USB, BQSPI, FILE Reset */
> -#define ALTR_A10SR_USB_QSPI_RD_REG    0x13    /* USB, BQSPI, FILE Reset */
> -#define ALTR_A10SR_SFPA_WR_REG        0x14    /* SFPA Control Reg */
> -#define ALTR_A10SR_SFPA_RD_REG        0x15    /* SFPA Control Reg */
> -#define ALTR_A10SR_SFPB_WR_REG        0x16    /* SFPB Control Reg */
> -#define ALTR_A10SR_SFPB_RD_REG        0x17    /* SFPB Control Reg */
> -#define ALTR_A10SR_I2C_M_RD_REG       0x19    /* I2C Master Select */
> -#define ALTR_A10SR_WARM_RST_WR_REG    0x1A    /* HPS Warm Reset */
> -#define ALTR_A10SR_WARM_RST_RD_REG    0x1B    /* HPS Warm Reset */
> -#define ALTR_A10SR_WR_KEY_WR_REG      0x1C    /* HPS Warm Reset Key */
> -#define ALTR_A10SR_WR_KEY_RD_REG      0x1D    /* HPS Warm Reset Key */
> -#define ALTR_A10SR_PMBUS_WR_REG       0x1E    /* HPS PM Bus */
> -#define ALTR_A10SR_PMBUS_RD_REG       0x1F    /* HPS PM Bus */
> +#define ALTR_A10SR_PWR_GOOD1_RD_REG  0x09     /* Power Good1 Read */
> +/* Power Good #1 Register Bit Definitions */
> +#define ALTR_A10SR_PG1_OP_FLAG_SHIFT    7       /* Power On Complete */
> +#define ALTR_A10SR_PG1_1V8_SHIFT        6       /* 1.8V Power Good */
> +#define ALTR_A10SR_PG1_2V5_SHIFT        5       /* 2.5V Power Good */
> +#define ALTR_A10SR_PG1_3V3_SHIFT        4       /* 3.3V Power Good */
> +#define ALTR_A10SR_PG1_5V0_SHIFT        3       /* 5.0V Power Good */
> +#define ALTR_A10SR_PG1_0V9_SHIFT        2       /* 0.9V Power Good */
> +#define ALTR_A10SR_PG1_0V95_SHIFT       1       /* 0.95V Power Good */
> +#define ALTR_A10SR_PG1_1V0_SHIFT        0       /* 1.0V Power Good */
> +
> +#define ALTR_A10SR_PWR_GOOD2_RD_REG  0x0B     /* Power Good2 Read */
> +/* Power Good #2 Register Bit Definitions */
> +#define ALTR_A10SR_PG2_HPS_SHIFT        7       /* HPS Power Good */
> +#define ALTR_A10SR_PG2_HL_HPS_SHIFT     6       /* HILOHPS_VDD Power Good */
> +#define ALTR_A10SR_PG2_HL_VDD_SHIFT     5       /* HILO VDD Power Good */
> +#define ALTR_A10SR_PG2_HL_VDDQ_SHIFT    4       /* HILO VDDQ Power Good */
> +#define ALTR_A10SR_PG2_FMCAVADJ_SHIFT   3       /* FMCA VADJ Power Good */
> +#define ALTR_A10SR_PG2_FMCBVADJ_SHIFT   2       /* FMCB VADJ Power Good */
> +#define ALTR_A10SR_PG2_FAC2MP_SHIFT     1       /* FAC2MP Power Good */
> +#define ALTR_A10SR_PG2_FBC2MP_SHIFT     0       /* FBC2MP Power Good */
> +
> +#define ALTR_A10SR_PWR_GOOD3_RD_REG  0x0D   /* Power Good3 Read */
> +/* Power Good #3 Register Bit Definitions */
> +#define ALTR_A10SR_PG3_FAM2C_SHIFT      7       /* FAM2C Power Good */
> +#define ALTR_A10SR_PG3_10V_FAIL_SHIFT   6       /* 10V Fail n */
> +#define ALTR_A10SR_PG3_BF_PR_SHIFT      5       /* BF Present n */
> +#define ALTR_A10SR_PG3_FILE_PR_SHIFT    4       /* File Present n */
> +#define ALTR_A10SR_PG3_FMCA_PR_SHIFT    3       /* FMCA Present n */
> +#define ALTR_A10SR_PG3_FMCB_PR_SHIFT    2       /* FMCB Present n */
> +#define ALTR_A10SR_PG3_PCIE_PR_SHIFT    1       /* PCIE Present n */
> +#define ALTR_A10SR_PG3_PCIE_WAKE_SHIFT  0       /* PCIe Wake N */
> +
> +#define ALTR_A10SR_FMCAB_WR_REG      0x0E    /* FMCA/B & PCIe Pwr Enable */
> +#define ALTR_A10SR_FMCAB_RD_REG      0x0F    /* FMCA/B & PCIe Pwr Enable */
> +/* FMCA/B & PCIe Power Bit Definitions */
> +#define ALTR_A10SR_PCIE_EN_SHIFT        7       /* PCIe Pwr Enable */
> +#define ALTR_A10SR_PCIE_AUXEN_SHIFT     6       /* PCIe Aux Pwr Enable */
> +#define ALTR_A10SR_FMCA_EN_SHIFT        5       /* FMCA Pwr Enable */
> +#define ALTR_A10SR_FMCA_AUXEN_SHIFT     4       /* FMCA Aux Pwr Enable */
> +#define ALTR_A10SR_FMCB_EN_SHIFT        3       /* FMCB Pwr Enable */
> +#define ALTR_A10SR_FMCB_AUXEN_SHIFT     2       /* FMCB Aux Pwr Enable */
> +
> +#define ALTR_A10SR_HPS_RST_WR_REG    0x10    /* HPS Reset */
> +#define ALTR_A10SR_HPS_RST_RD_REG    0x11    /* HPS Reset */
> +/* HPS Reset Bit Definitions */
> +#define ALTR_A10SR_HPS_UARTA_RSTN_SHIFT 7       /* UARTA Reset n */
> +#define ALTR_A10SR_HPS_WARM_RSTN_SHIFT  6       /* WARM Reset n */
> +#define ALTR_A10SR_HPS_WARM_RST1N_SHIFT 5       /* WARM Reset1 n */
> +#define ALTR_A10SR_HPS_COLD_RSTN_SHIFT  4       /* COLD Reset n */
> +#define ALTR_A10SR_HPS_NPOR_SHIFT       3       /* N Power On Reset */
> +#define ALTR_A10SR_HPS_NRST_SHIFT       2       /* N Reset */
> +#define ALTR_A10SR_HPS_ENET_RSTN_SHIFT  1       /* Ethernet Reset n */
> +#define ALTR_A10SR_HPS_ENET_INTN_SHIFT  0       /* Ethernet IRQ n */
> +
> +#define ALTR_A10SR_USB_QSPI_WR_REG   0x12    /* USB, BQSPI, FILE Reset */
> +#define ALTR_A10SR_USB_QSPI_RD_REG   0x13    /* USB, BQSPI, FILE Reset */
> +/* USB/QSPI/FILE Reset Bit Definitions */
> +#define ALTR_A10SR_USB_RST_SHIFT        7       /* USB Reset */
> +#define ALTR_A10SR_BQSPI_RST_N_SHIFT    6       /* BQSPI Reset n */
> +#define ALTR_A10SR_FILE_RST_N_SHIFT     5       /* FILE Reset n */
> +#define ALTR_A10SR_PCIE_PERST_N_SHIFT   4       /* PCIe PE Reset n */
> +
> +#define ALTR_A10SR_SFPA_WR_REG       0x14    /* SFPA Control Reg */
> +#define ALTR_A10SR_SFPA_RD_REG       0x15    /* SFPA Control Reg */
> +#define ALTR_A10SR_SFPB_WR_REG       0x16    /* SFPB Control Reg */
> +#define ALTR_A10SR_SFPB_RD_REG       0x17    /* SFPB Control Reg */
> +/* SFPA Bit Definitions */
> +#define ALTR_A10SR_SFP_TXDIS_SHIFT      7       /* SFPA TX Disable */
> +#define ALTR_A10SR_SFP_RATESEL10        0x60    /* SFPA_Rate Select [1:0] */
> +#define ALTR_A10SR_SFP_LOS_SHIFT        4       /* SFPA LOS */
> +#define ALTR_A10SR_SFP_FAULT_SHIFT      3       /* SFPA Fault */
> +
> +#define ALTR_A10SR_I2C_M_RD_REG      0x19    /* I2C Master Select */
> +
> +#define ALTR_A10SR_WARM_RST_WR_REG   0x1A    /* HPS Warm Reset */
> +#define ALTR_A10SR_WARM_RST_RD_REG   0x1B    /* HPS Warm Reset */
> +
> +#define ALTR_A10SR_WR_KEY_WR_REG     0x1C    /* HPS Warm Reset Key */
> +#define ALTR_A10SR_WR_KEY_RD_REG     0x1D    /* HPS Warm Reset Key */
> +
> +#define ALTR_A10SR_PMBUS_WR_REG      0x1E    /* HPS PM Bus */
> +#define ALTR_A10SR_PMBUS_RD_REG      0x1F    /* HPS PM Bus */
> +/* PM Bus Bit Definitions */
> +#define ALTR_A10SR_PMBUS_EN_SHIFT       7       /* PMBus FPGA Enable */
> +#define ALTR_A10SR_PMBUS_DISN_SHIFT     6       /* PMBus HPS Enable */
> +#define ALTR_A10SR_PMBUS_ALERTN_SHIFT   5       /* PMBus Alert */
>  
>  struct altr_a10sr {
>  	struct device *dev;
> -- 
> 1.7.9.5
> 

Re: [RFC 7/8] hwmon: Altera Arria10 System Resource Chip - HW Monitor

[Thor Thayer]

On 03/29/2016 03:16 PM, Guenter Roeck wrote:
> On Tue, Mar 29, 2016 at 02:13:10PM -0500, tthayer@opensource.altera.com wrote:
>> From: Thor Thayer <tthayer@opensource.altera.com>
>>
>> This patch adds the hwmon functionality to the Arria10 System
>> Resource Chip. The hwmon encapsulates the PCIe Enable, USB Enable,
>> and all the Power Good signals on the System Controller.
>>
>
> I may be completely wrong, but a glance through the driver suggests
> that, if anything, this should be a regulator driver, not a hwmon driver.
> A hardware monitoring driver would be expected to report the voltages,
> not (just) the voltage status. Am I missing something ?
>
> Please have a look into Documentation/hwmon/sysfs-interface for
> acceptable hwmon attribute names and their meaning.
>
> Thanks,
> Guenter
>

Hi Guenter,

<adding voltage and current regulator framework moderators>

Yes, I see your point. In looking at the regulator drivers, I interpret 
those as being controlled by the driver whereas this chip is passively 
reporting status.

The success/fail indication seemed at first glance to fit the hwmon 
model. I thought the fan indication would be a good analog but even it 
reports speed and not success/fail.

After reading the referenced document, I agree that hwmon probably isn't 
appropriate. However, the regulator doesn't seem appropriate either (the 
only status appears to be tied to battery properties).

Thanks for reviewing!

Thor

>> Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
>> ---
>>   drivers/hwmon/Kconfig              |    9 +
>>   drivers/hwmon/Makefile             |    1 +
>>   drivers/hwmon/altera-a10sr-hwmon.c |  544 ++++++++++++++++++++++++++++++++++++
>>   drivers/mfd/altera-a10sr.c         |    4 +
>>   include/linux/mfd/altera-a10sr.h   |  107 +++++--
>>   5 files changed, 645 insertions(+), 20 deletions(-)
>>   create mode 100644 drivers/hwmon/altera-a10sr-hwmon.c
>>
>> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
>> index 5c2d13a..edea31a 100644
>> --- a/drivers/hwmon/Kconfig
>> +++ b/drivers/hwmon/Kconfig
>> @@ -81,6 +81,15 @@ config SENSORS_ABITUGURU3
>>   	  This driver can also be built as a module.  If so, the module
>>   	  will be called abituguru3.
>>
>> +config SENSORS_ALTERA_A10SR
>> +	bool "Altera Arria10 System Status"
>> +	depends on MFD_ALTERA_A10SR
>> +	help
>> +	  If you say yes here you get support for the power ready status
>> +	  for the Arria10's external power supplies on the Arria10 DevKit.
>> +	  These values are read over the SPI bus from the Arria10 System
>> +	  Resource chip.
>> +
>>   config SENSORS_AD7314
>>   	tristate "Analog Devices AD7314 and compatibles"
>>   	depends on SPI
>> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
>> index 58cc3ac..7a75dc8 100644
>> --- a/drivers/hwmon/Makefile
>> +++ b/drivers/hwmon/Makefile
>> @@ -43,6 +43,7 @@ obj-$(CONFIG_SENSORS_ADT7411)	+= adt7411.o
>>   obj-$(CONFIG_SENSORS_ADT7462)	+= adt7462.o
>>   obj-$(CONFIG_SENSORS_ADT7470)	+= adt7470.o
>>   obj-$(CONFIG_SENSORS_ADT7475)	+= adt7475.o
>> +obj-$(CONFIG_SENSORS_ALTERA_A10SR) += altera-a10sr-hwmon.o
>>   obj-$(CONFIG_SENSORS_APPLESMC)	+= applesmc.o
>>   obj-$(CONFIG_SENSORS_ARM_SCPI)	+= scpi-hwmon.o
>>   obj-$(CONFIG_SENSORS_ASC7621)	+= asc7621.o
>> diff --git a/drivers/hwmon/altera-a10sr-hwmon.c b/drivers/hwmon/altera-a10sr-hwmon.c
>> new file mode 100644
>> index 0000000..e789eed
>> --- /dev/null
>> +++ b/drivers/hwmon/altera-a10sr-hwmon.c
>> @@ -0,0 +1,544 @@
>> +/*
>> + *  Copyright Altera Corporation (C) 2014-2016. All Rights Reserved
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms and conditions of the GNU General Public License,
>> + * version 2, as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope it will be useful, but WITHOUT
>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
>> + * more details.
>> + *
>> + * You should have received a copy of the GNU General Public License along with
>> + * this program.  If not, see <http://www.gnu.org/licenses/>.
>> + *
>> + * HW Monitor driver for  Altera Arria10 MAX5 System Resource Chip
>> + * Adapted from DA9052
>> + */
>> +
>> +#include <linux/err.h>
>> +#include <linux/hwmon.h>
>> +#include <linux/hwmon-sysfs.h>
>> +#include <linux/init.h>
>> +#include <linux/kernel.h>
>> +#include <linux/mfd/altera-a10sr.h>
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/slab.h>
>> +
>> +#define ALTR_A10SR_1V0_BIT_POS          ALTR_A10SR_PG1_1V0_SHIFT
>> +#define ALTR_A10SR_0V95_BIT_POS         ALTR_A10SR_PG1_0V95_SHIFT
>> +#define ALTR_A10SR_0V9_BIT_POS          ALTR_A10SR_PG1_0V9_SHIFT
>> +#define ALTR_A10SR_10V_BIT_POS          ALTR_A10SR_PG1_10V_SHIFT
>> +#define ALTR_A10SR_5V0_BIT_POS          ALTR_A10SR_PG1_5V0_SHIFT
>> +#define ALTR_A10SR_3V3_BIT_POS          ALTR_A10SR_PG1_3V3_SHIFT
>> +#define ALTR_A10SR_2V5_BIT_POS          ALTR_A10SR_PG1_2V5_SHIFT
>> +#define ALTR_A10SR_1V8_BIT_POS          ALTR_A10SR_PG1_1V8_SHIFT
>> +#define ALTR_A10SR_OP_FLAG_BIT_POS      ALTR_A10SR_PG1_OP_FLAG_SHIFT
>> +/* 2nd register needs an offset of 8 to get to 2nd register */
>> +#define ALTR_A10SR_FBC2MP_BIT_POS       (8 + ALTR_A10SR_PG2_FBC2MP_SHIFT)
>> +#define ALTR_A10SR_FAC2MP_BIT_POS       (8 + ALTR_A10SR_PG2_FAC2MP_SHIFT)
>> +#define ALTR_A10SR_FMCBVADJ_BIT_POS     (8 + ALTR_A10SR_PG2_FMCBVADJ_SHIFT)
>> +#define ALTR_A10SR_FMCAVADJ_BIT_POS     (8 + ALTR_A10SR_PG2_FMCAVADJ_SHIFT)
>> +#define ALTR_A10SR_HL_VDDQ_BIT_POS      (8 + ALTR_A10SR_PG2_HL_VDDQ_SHIFT)
>> +#define ALTR_A10SR_HL_VDD_BIT_POS       (8 + ALTR_A10SR_PG2_HL_VDD_SHIFT)
>> +#define ALTR_A10SR_HL_HPS_BIT_POS       (8 + ALTR_A10SR_PG2_HL_HPS_SHIFT)
>> +#define ALTR_A10SR_HPS_BIT_POS          (8 + ALTR_A10SR_PG2_HPS_SHIFT)
>> +/* 3rd register needs an offset of 16 to get to 3rd register */
>> +#define ALTR_A10SR_PCIE_WAKE_BIT_POS    (16 + ALTR_A10SR_PG3_PCIE_WAKE_SHIFT)
>> +#define ALTR_A10SR_PCIE_PR_BIT_POS      (16 + ALTR_A10SR_PG3_PCIE_PR_SHIFT)
>> +#define ALTR_A10SR_FMCB_PR_BIT_POS      (16 + ALTR_A10SR_PG3_FMCB_PR_SHIFT)
>> +#define ALTR_A10SR_FMCA_PR_BIT_POS      (16 + ALTR_A10SR_PG3_FMCA_PR_SHIFT)
>> +#define ALTR_A10SR_FILE_PR_BIT_POS      (16 + ALTR_A10SR_PG3_FILE_PR_SHIFT)
>> +#define ALTR_A10SR_BF_PR_BIT_POS        (16 + ALTR_A10SR_PG3_BF_PR_SHIFT)
>> +#define ALTR_A10SR_10V_FAIL_BIT_POS     (16 + ALTR_A10SR_PG3_10V_FAIL_SHIFT)
>> +#define ALTR_A10SR_FAM2C_BIT_POS        (16 + ALTR_A10SR_PG3_FAM2C_SHIFT)
>> +/* FMCA/B & PCIE Enables need an offset of 24 */
>> +#define ALTR_A10SR_FMCB_AUXEN_POS       (24 + ALTR_A10SR_FMCB_AUXEN_SHIFT)
>> +#define ALTR_A10SR_FMCB_EN_POS          (24 + ALTR_A10SR_FMCB_EN_SHIFT)
>> +#define ALTR_A10SR_FMCA_AUXEN_POS       (24 + ALTR_A10SR_FMCA_AUXEN_SHIFT)
>> +#define ALTR_A10SR_FMCA_EN_POS          (24 + ALTR_A10SR_FMCA_EN_SHIFT)
>> +#define ALTR_A10SR_PCIE_AUXEN_POS       (24 + ALTR_A10SR_PCIE_AUXEN_SHIFT)
>> +#define ALTR_A10SR_PCIE_EN_POS          (24 + ALTR_A10SR_PCIE_EN_SHIFT)
>> +/* HPS Resets need an offset of 32 */
>> +#define ALTR_A10SR_HPS_RST_UART_POS     (32 + ALTR_A10SR_HPS_UARTA_RSTN_SHIFT)
>> +#define ALTR_A10SR_HPS_RST_WARM_POS     (32 + ALTR_A10SR_HPS_WARM_RSTN_SHIFT)
>> +#define ALTR_A10SR_HPS_RST_WARM1_POS    (32 + ALTR_A10SR_HPS_WARM_RST1N_SHIFT)
>> +#define ALTR_A10SR_HPS_RST_COLD_POS     (32 + ALTR_A10SR_HPS_COLD_RSTN_SHIFT)
>> +#define ALTR_A10SR_HPS_RST_NPOR_POS     (32 + ALTR_A10SR_HPS_NPOR_SHIFT)
>> +#define ALTR_A10SR_HPS_RST_NRST_POS     (32 + ALTR_A10SR_HPS_NRST_SHIFT)
>> +#define ALTR_A10SR_HPS_RST_ENET_POS     (32 + ALTR_A10SR_HPS_ENET_RSTN_SHIFT)
>> +#define ALTR_A10SR_HPS_RST_ENETINT_POS  (32 + ALTR_A10SR_HPS_ENET_INTN_SHIFT)
>> +/* Peripheral Resets need an offset of 40 */
>> +#define ALTR_A10SR_PER_RST_USB_POS      (40 + ALTR_A10SR_USB_RST_SHIFT)
>> +#define ALTR_A10SR_PER_RST_BQSPI_POS    (40 + ALTR_A10SR_BQSPI_RST_N_SHIFT)
>> +#define ALTR_A10SR_PER_RST_FILE_POS     (40 + ALTR_A10SR_FILE_RST_N_SHIFT)
>> +#define ALTR_A10SR_PER_RST_PCIE_POS     (40 + ALTR_A10SR_PCIE_PERST_N_SHIFT)
>> +/* HWMON - Read Entire Register */
>> +#define ALTR_A10SR_ENTIRE_REG           (88)
>> +#define ALTR_A10SR_ENTIRE_REG_MASK      (0xFF)
>> +#define ALTR_A10SR_VERSION              (0 + ALTR_A10SR_ENTIRE_REG)
>> +#define ALTR_A10SR_LED                  (1 + ALTR_A10SR_ENTIRE_REG)
>> +#define ALTR_A10SR_PB                   (2 + ALTR_A10SR_ENTIRE_REG)
>> +#define ALTR_A10SR_PBF                  (3 + ALTR_A10SR_ENTIRE_REG)
>> +#define ALTR_A10SR_PG1                  (4 + ALTR_A10SR_ENTIRE_REG)
>> +#define ALTR_A10SR_PG2                  (5 + ALTR_A10SR_ENTIRE_REG)
>> +#define ALTR_A10SR_PG3                  (6 + ALTR_A10SR_ENTIRE_REG)
>> +#define ALTR_A10SR_FMCAB                (7 + ALTR_A10SR_ENTIRE_REG)
>> +#define ALTR_A10SR_HPS_RST              (8 + ALTR_A10SR_ENTIRE_REG)
>> +#define ALTR_A10SR_PER_RST              (9 + ALTR_A10SR_ENTIRE_REG)
>> +#define ALTR_A10SR_SFPA                 (10 + ALTR_A10SR_ENTIRE_REG)
>> +#define ALTR_A10SR_SFPB                 (11 + ALTR_A10SR_ENTIRE_REG)
>> +#define ALTR_A10SR_I2C_MASTER           (12 + ALTR_A10SR_ENTIRE_REG)
>> +#define ALTR_A10SR_WARM_RST             (13 + ALTR_A10SR_ENTIRE_REG)
>> +#define ALTR_A10SR_WARM_RST_KEY         (14 + ALTR_A10SR_ENTIRE_REG)
>> +#define ALTR_A10SR_PMBUS                (15 + ALTR_A10SR_ENTIRE_REG)
>> +
>> +struct altr_a10sr_hwmon {
>> +	struct altr_a10sr	*a10sr;
>> +	struct device		*class_device;
>> +};
>> +
>> +static const char *const hwmon_names[] = {
>> +	[ALTR_A10SR_1V0_BIT_POS]       = "1.0V PWR Good",
>> +	[ALTR_A10SR_0V95_BIT_POS]      = "0.95V PWR Good",
>> +	[ALTR_A10SR_0V9_BIT_POS]       = "0.9V PWR Good",
>> +	[ALTR_A10SR_5V0_BIT_POS]       = "5.0V PWR Good",
>> +	[ALTR_A10SR_3V3_BIT_POS]       = "3.3V PWR Good",
>> +	[ALTR_A10SR_2V5_BIT_POS]       = "2.5V PWR Good",
>> +	[ALTR_A10SR_1V8_BIT_POS]       = "1.8V PWR Good",
>> +	[ALTR_A10SR_OP_FLAG_BIT_POS]   = "PWR On Complete",
>> +
>> +	[ALTR_A10SR_FBC2MP_BIT_POS]    = "FBC2MP PWR Good",
>> +	[ALTR_A10SR_FAC2MP_BIT_POS]    = "FAC2MP PWR Good",
>> +	[ALTR_A10SR_FMCBVADJ_BIT_POS]  = "FMCBVADJ PWR Good",
>> +	[ALTR_A10SR_FMCAVADJ_BIT_POS]  = "FMCAVADJ PWR Good",
>> +	[ALTR_A10SR_HL_VDDQ_BIT_POS]   = "HILO VDDQ PWR Good",
>> +	[ALTR_A10SR_HL_VDD_BIT_POS]    = "HILO VDD PWR Good",
>> +	[ALTR_A10SR_HL_HPS_BIT_POS]    = "HILO HPS PWR Good",
>> +	[ALTR_A10SR_HPS_BIT_POS]       = "HPS PWR Good",
>> +
>> +	[ALTR_A10SR_PCIE_WAKE_BIT_POS] = "PCIE WAKEn",
>> +	[ALTR_A10SR_PCIE_PR_BIT_POS]   = "PCIE PRESENTn",
>> +	[ALTR_A10SR_FMCB_PR_BIT_POS]   = "FMCB PRESENTn",
>> +	[ALTR_A10SR_FMCA_PR_BIT_POS]   = "FMCA PRESENTn",
>> +	[ALTR_A10SR_FILE_PR_BIT_POS]   = "FILE PRESENTn",
>> +	[ALTR_A10SR_BF_PR_BIT_POS]     = "BF PRESENTn",
>> +	[ALTR_A10SR_10V_FAIL_BIT_POS]  = "10V FAILn",
>> +	[ALTR_A10SR_FAM2C_BIT_POS]     = "FAM2C PWR Good",
>> +};
>> +
>> +static ssize_t altr_a10sr_read_status(struct device *dev,
>> +				      struct device_attribute *devattr,
>> +				      char *buf)
>> +{
>> +	struct altr_a10sr_hwmon *hwmon = dev_get_drvdata(dev);
>> +	int ret, index = to_sensor_dev_attr(devattr)->index;
>> +	int mask = ALTR_A10SR_REG_BIT_MASK(index);
>> +	unsigned char reg = ALTR_A10SR_PWR_GOOD1_RD_REG +
>> +			    ALTR_A10SR_REG_OFFSET(index);
>> +
>> +	/* Check if this is an entire register read */
>> +	if (index >= ALTR_A10SR_ENTIRE_REG) {
>> +		reg = ((index - ALTR_A10SR_ENTIRE_REG) << 1) + 1;
>> +		mask = ALTR_A10SR_ENTIRE_REG_MASK;
>> +	}
>> +
>> +	ret = altr_a10sr_reg_read(hwmon->a10sr, reg);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	return sprintf(buf, "0x%X\n", (ret & mask));
>> +}
>> +
>> +static ssize_t altr_a10sr_hwmon_show_name(struct device *dev,
>> +					  struct device_attribute *devattr,
>> +					  char *buf)
>> +{
>> +	return sprintf(buf, "altr_a10sr\n");
>> +}
>> +
>> +static ssize_t show_label(struct device *dev,
>> +			  struct device_attribute *devattr, char *buf)
>> +{
>> +	return sprintf(buf, "%s\n",
>> +		       hwmon_names[to_sensor_dev_attr(devattr)->index]);
>> +}
>> +
>> +static ssize_t set_enable(struct device *dev,
>> +			  struct device_attribute *dev_attr,
>> +			  const char *buf, size_t count)
>> +{
>> +	unsigned long val;
>> +	struct altr_a10sr_hwmon *hwmon = dev_get_drvdata(dev);
>> +	int ret, index = to_sensor_dev_attr(dev_attr)->index;
>> +	int mask = ALTR_A10SR_REG_BIT_MASK(index);
>> +	unsigned char reg = (ALTR_A10SR_PWR_GOOD1_RD_REG & WRITE_REG_MASK) +
>> +			    ALTR_A10SR_REG_OFFSET(index);
>> +	int res = kstrtol(buf, 10, &val);
>> +
>> +	if (res < 0)
>> +		return res;
>> +
>> +	/* Check if this is an entire register write */
>> +	if (index >= ALTR_A10SR_ENTIRE_REG) {
>> +		reg = ((index - ALTR_A10SR_ENTIRE_REG) << 1);
>> +		mask = ALTR_A10SR_ENTIRE_REG_MASK;
>> +	}
>> +
>> +	ret = altr_a10sr_reg_update(hwmon->a10sr, reg, mask, val);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	return count;
>> +}
>> +
>> +/* First Power Good Register Bits */
>> +static SENSOR_DEVICE_ATTR(1v0_input, S_IRUGO, altr_a10sr_read_status, NULL,
>> +			  ALTR_A10SR_1V0_BIT_POS);
>> +static SENSOR_DEVICE_ATTR(1v0_label, S_IRUGO, show_label, NULL,
>> +			  ALTR_A10SR_1V0_BIT_POS);
>> +static SENSOR_DEVICE_ATTR(0v95_input, S_IRUGO, altr_a10sr_read_status, NULL,
>> +			  ALTR_A10SR_0V95_BIT_POS);
>> +static SENSOR_DEVICE_ATTR(0v95_label, S_IRUGO, show_label, NULL,
>> +			  ALTR_A10SR_0V95_BIT_POS);
>> +static SENSOR_DEVICE_ATTR(0v9_input, S_IRUGO, altr_a10sr_read_status, NULL,
>> +			  ALTR_A10SR_0V9_BIT_POS);
>> +static SENSOR_DEVICE_ATTR(0v9_label, S_IRUGO, show_label, NULL,
>> +			  ALTR_A10SR_0V9_BIT_POS);
>> +static SENSOR_DEVICE_ATTR(5v0_input, S_IRUGO, altr_a10sr_read_status, NULL,
>> +			  ALTR_A10SR_5V0_BIT_POS);
>> +static SENSOR_DEVICE_ATTR(5v0_label, S_IRUGO, show_label, NULL,
>> +			  ALTR_A10SR_5V0_BIT_POS);
>> +static SENSOR_DEVICE_ATTR(3v3_input, S_IRUGO, altr_a10sr_read_status, NULL,
>> +			  ALTR_A10SR_3V3_BIT_POS);
>> +static SENSOR_DEVICE_ATTR(3v3_label, S_IRUGO, show_label, NULL,
>> +			  ALTR_A10SR_3V3_BIT_POS);
>> +static SENSOR_DEVICE_ATTR(2v5_input, S_IRUGO, altr_a10sr_read_status, NULL,
>> +			  ALTR_A10SR_2V5_BIT_POS);
>> +static SENSOR_DEVICE_ATTR(2v5_label, S_IRUGO, show_label, NULL,
>> +			  ALTR_A10SR_2V5_BIT_POS);
>> +static SENSOR_DEVICE_ATTR(1v8_input, S_IRUGO, altr_a10sr_read_status, NULL,
>> +			  ALTR_A10SR_1V8_BIT_POS);
>> +static SENSOR_DEVICE_ATTR(1v8_label, S_IRUGO, show_label, NULL,
>> +			  ALTR_A10SR_1V8_BIT_POS);
>> +static SENSOR_DEVICE_ATTR(opflag_input, S_IRUGO, altr_a10sr_read_status, NULL,
>> +			  ALTR_A10SR_OP_FLAG_BIT_POS);
>> +static SENSOR_DEVICE_ATTR(opflag_label, S_IRUGO, show_label, NULL,
>> +			  ALTR_A10SR_OP_FLAG_BIT_POS);
>> +/* Second Power Good Register Bits */
>> +static SENSOR_DEVICE_ATTR(fbc2mp_input, S_IRUGO, altr_a10sr_read_status, NULL,
>> +			  ALTR_A10SR_FBC2MP_BIT_POS);
>> +static SENSOR_DEVICE_ATTR(fbc2mp_label, S_IRUGO, show_label, NULL,
>> +			  ALTR_A10SR_FBC2MP_BIT_POS);
>> +static SENSOR_DEVICE_ATTR(fac2mp_input, S_IRUGO, altr_a10sr_read_status, NULL,
>> +			  ALTR_A10SR_FAC2MP_BIT_POS);
>> +static SENSOR_DEVICE_ATTR(fac2mp_label, S_IRUGO, show_label, NULL,
>> +			  ALTR_A10SR_FAC2MP_BIT_POS);
>> +static SENSOR_DEVICE_ATTR(fmcbvadj_input, S_IRUGO, altr_a10sr_read_status, NULL,
>> +			  ALTR_A10SR_FMCBVADJ_BIT_POS);
>> +static SENSOR_DEVICE_ATTR(fmcbvadj_label, S_IRUGO, show_label, NULL,
>> +			  ALTR_A10SR_FMCBVADJ_BIT_POS);
>> +static SENSOR_DEVICE_ATTR(fmcavadj_input, S_IRUGO, altr_a10sr_read_status, NULL,
>> +			  ALTR_A10SR_FMCAVADJ_BIT_POS);
>> +static SENSOR_DEVICE_ATTR(fmcavadj_label, S_IRUGO, show_label, NULL,
>> +			  ALTR_A10SR_FMCAVADJ_BIT_POS);
>> +static SENSOR_DEVICE_ATTR(hl_vddq_input, S_IRUGO, altr_a10sr_read_status, NULL,
>> +			  ALTR_A10SR_HL_VDDQ_BIT_POS);
>> +static SENSOR_DEVICE_ATTR(hl_vddq_label, S_IRUGO, show_label, NULL,
>> +			  ALTR_A10SR_HL_VDDQ_BIT_POS);
>> +static SENSOR_DEVICE_ATTR(hl_vdd_input, S_IRUGO, altr_a10sr_read_status, NULL,
>> +			  ALTR_A10SR_HL_VDD_BIT_POS);
>> +static SENSOR_DEVICE_ATTR(hl_vdd_label, S_IRUGO, show_label, NULL,
>> +			  ALTR_A10SR_HL_VDD_BIT_POS);
>> +static SENSOR_DEVICE_ATTR(hlhps_vdd_input, S_IRUGO, altr_a10sr_read_status,
>> +			  NULL, ALTR_A10SR_HL_HPS_BIT_POS);
>> +static SENSOR_DEVICE_ATTR(hlhps_vdd_label, S_IRUGO, show_label, NULL,
>> +			  ALTR_A10SR_HL_HPS_BIT_POS);
>> +static SENSOR_DEVICE_ATTR(hps_input, S_IRUGO, altr_a10sr_read_status, NULL,
>> +			  ALTR_A10SR_HPS_BIT_POS);
>> +static SENSOR_DEVICE_ATTR(hps_label, S_IRUGO, show_label, NULL,
>> +			  ALTR_A10SR_HPS_BIT_POS);
>> +/* Third Power Good Register Bits */
>> +static SENSOR_DEVICE_ATTR(pcie_wake_input, S_IRUGO, altr_a10sr_read_status,
>> +			  NULL, ALTR_A10SR_PCIE_WAKE_BIT_POS);
>> +static SENSOR_DEVICE_ATTR(pcie_wake_label, S_IRUGO, show_label, NULL,
>> +			  ALTR_A10SR_PCIE_WAKE_BIT_POS);
>> +static SENSOR_DEVICE_ATTR(pcie_pr_input, S_IRUGO, altr_a10sr_read_status, NULL,
>> +			  ALTR_A10SR_PCIE_PR_BIT_POS);
>> +static SENSOR_DEVICE_ATTR(pcie_pr_label, S_IRUGO, show_label, NULL,
>> +			  ALTR_A10SR_PCIE_PR_BIT_POS);
>> +static SENSOR_DEVICE_ATTR(fmcb_pr_input, S_IRUGO, altr_a10sr_read_status, NULL,
>> +			  ALTR_A10SR_FMCB_PR_BIT_POS);
>> +static SENSOR_DEVICE_ATTR(fmcb_pr_label, S_IRUGO, show_label, NULL,
>> +			  ALTR_A10SR_FMCB_PR_BIT_POS);
>> +static SENSOR_DEVICE_ATTR(fmca_pr_input, S_IRUGO, altr_a10sr_read_status, NULL,
>> +			  ALTR_A10SR_FMCA_PR_BIT_POS);
>> +static SENSOR_DEVICE_ATTR(fmca_pr_label, S_IRUGO, show_label, NULL,
>> +			  ALTR_A10SR_FMCA_PR_BIT_POS);
>> +static SENSOR_DEVICE_ATTR(file_pr_input, S_IRUGO, altr_a10sr_read_status, NULL,
>> +			  ALTR_A10SR_FILE_PR_BIT_POS);
>> +static SENSOR_DEVICE_ATTR(file_pr_label, S_IRUGO, show_label, NULL,
>> +			  ALTR_A10SR_FILE_PR_BIT_POS);
>> +static SENSOR_DEVICE_ATTR(bf_pr_input, S_IRUGO, altr_a10sr_read_status, NULL,
>> +			  ALTR_A10SR_BF_PR_BIT_POS);
>> +static SENSOR_DEVICE_ATTR(bf_pr_label, S_IRUGO, show_label, NULL,
>> +			  ALTR_A10SR_BF_PR_BIT_POS);
>> +static SENSOR_DEVICE_ATTR(10v_fail_input, S_IRUGO, altr_a10sr_read_status,
>> +			  NULL, ALTR_A10SR_10V_FAIL_BIT_POS);
>> +static SENSOR_DEVICE_ATTR(10v_fail_label, S_IRUGO, show_label, NULL,
>> +			  ALTR_A10SR_10V_FAIL_BIT_POS);
>> +static SENSOR_DEVICE_ATTR(fam2c_input, S_IRUGO, altr_a10sr_read_status, NULL,
>> +			  ALTR_A10SR_FAM2C_BIT_POS);
>> +static SENSOR_DEVICE_ATTR(fam2c_label, S_IRUGO, show_label, NULL,
>> +			  ALTR_A10SR_FAM2C_BIT_POS);
>> +/* Peripheral Enable bits */
>> +static SENSOR_DEVICE_ATTR(fmcb_aux_en, S_IRUGO | S_IWUSR,
>> +			  altr_a10sr_read_status, set_enable,
>> +			  ALTR_A10SR_FMCB_AUXEN_POS);
>> +static SENSOR_DEVICE_ATTR(fmcb_en, S_IRUGO | S_IWUSR,
>> +			  altr_a10sr_read_status, set_enable,
>> +			  ALTR_A10SR_FMCB_EN_POS);
>> +static SENSOR_DEVICE_ATTR(fmca_aux_en, S_IRUGO | S_IWUSR,
>> +			  altr_a10sr_read_status, set_enable,
>> +			  ALTR_A10SR_FMCA_AUXEN_POS);
>> +static SENSOR_DEVICE_ATTR(fmca_en, S_IRUGO | S_IWUSR,
>> +			  altr_a10sr_read_status, set_enable,
>> +			  ALTR_A10SR_FMCA_EN_POS);
>> +static SENSOR_DEVICE_ATTR(pcie_aux_en, S_IRUGO | S_IWUSR,
>> +			  altr_a10sr_read_status, set_enable,
>> +			  ALTR_A10SR_PCIE_AUXEN_POS);
>> +static SENSOR_DEVICE_ATTR(pcie_en, S_IRUGO | S_IWUSR,
>> +			  altr_a10sr_read_status, set_enable,
>> +			  ALTR_A10SR_PCIE_EN_POS);
>> +/* HPS Reset bits */
>> +static SENSOR_DEVICE_ATTR(hps_uart_rst, S_IRUGO,
>> +			  altr_a10sr_read_status, set_enable,
>> +			  ALTR_A10SR_HPS_RST_UART_POS);
>> +static SENSOR_DEVICE_ATTR(hps_warm_rst, S_IRUGO,
>> +			  altr_a10sr_read_status, set_enable,
>> +			  ALTR_A10SR_HPS_RST_WARM_POS);
>> +static SENSOR_DEVICE_ATTR(hps_warm1_rst, S_IRUGO,
>> +			  altr_a10sr_read_status, set_enable,
>> +			  ALTR_A10SR_HPS_RST_WARM1_POS);
>> +static SENSOR_DEVICE_ATTR(hps_cold_rst, S_IRUGO,
>> +			  altr_a10sr_read_status, set_enable,
>> +			  ALTR_A10SR_HPS_RST_COLD_POS);
>> +static SENSOR_DEVICE_ATTR(hps_npor, S_IRUGO,
>> +			  altr_a10sr_read_status, set_enable,
>> +			  ALTR_A10SR_HPS_RST_NPOR_POS);
>> +static SENSOR_DEVICE_ATTR(hps_nrst, S_IRUGO,
>> +			  altr_a10sr_read_status, set_enable,
>> +			  ALTR_A10SR_HPS_RST_NRST_POS);
>> +static SENSOR_DEVICE_ATTR(hps_enet_rst, S_IRUGO | S_IWUSR,
>> +			  altr_a10sr_read_status, set_enable,
>> +			  ALTR_A10SR_HPS_RST_ENET_POS);
>> +static SENSOR_DEVICE_ATTR(hps_enet_int, S_IRUGO | S_IWUSR,
>> +			  altr_a10sr_read_status, set_enable,
>> +			  ALTR_A10SR_HPS_RST_ENETINT_POS);
>> +/* Peripheral Reset bits */
>> +static SENSOR_DEVICE_ATTR(usb_reset, S_IRUGO | S_IWUSR, altr_a10sr_read_status,
>> +			  set_enable, ALTR_A10SR_PER_RST_USB_POS);
>> +static SENSOR_DEVICE_ATTR(bqspi_resetn, S_IRUGO | S_IWUSR,
>> +			  altr_a10sr_read_status, set_enable,
>> +			  ALTR_A10SR_PER_RST_BQSPI_POS);
>> +static SENSOR_DEVICE_ATTR(file_resetn, S_IRUGO | S_IWUSR,
>> +			  altr_a10sr_read_status, set_enable,
>> +			  ALTR_A10SR_PER_RST_FILE_POS);
>> +static SENSOR_DEVICE_ATTR(pcie_perstn, S_IRUGO | S_IWUSR,
>> +			  altr_a10sr_read_status, set_enable,
>> +			  ALTR_A10SR_PER_RST_PCIE_POS);
>> +/* Entire Byte Read */
>> +static SENSOR_DEVICE_ATTR(max5_version, S_IRUGO, altr_a10sr_read_status,
>> +			  NULL, ALTR_A10SR_VERSION);
>> +static SENSOR_DEVICE_ATTR(max5_led, S_IRUGO, altr_a10sr_read_status,
>> +			  NULL, ALTR_A10SR_LED);
>> +static SENSOR_DEVICE_ATTR(max5_button, S_IRUGO, altr_a10sr_read_status,
>> +			  NULL, ALTR_A10SR_PB);
>> +static SENSOR_DEVICE_ATTR(max5_button_irq, S_IRUGO | S_IWUSR,
>> +			  altr_a10sr_read_status, set_enable, ALTR_A10SR_PBF);
>> +static SENSOR_DEVICE_ATTR(max5_pg1, S_IRUGO, altr_a10sr_read_status,
>> +			  NULL, ALTR_A10SR_PG1);
>> +static SENSOR_DEVICE_ATTR(max5_pg2, S_IRUGO, altr_a10sr_read_status,
>> +			  NULL, ALTR_A10SR_PG2);
>> +static SENSOR_DEVICE_ATTR(max5_pg3, S_IRUGO, altr_a10sr_read_status,
>> +			  NULL, ALTR_A10SR_PG3);
>> +static SENSOR_DEVICE_ATTR(max5_fmcab, S_IRUGO, altr_a10sr_read_status,
>> +			  NULL, ALTR_A10SR_FMCAB);
>> +static SENSOR_DEVICE_ATTR(max5_hps_resets, S_IRUGO | S_IWUSR,
>> +			  altr_a10sr_read_status, set_enable,
>> +			  ALTR_A10SR_HPS_RST);
>> +static SENSOR_DEVICE_ATTR(max5_per_resets, S_IRUGO | S_IWUSR,
>> +			  altr_a10sr_read_status, set_enable,
>> +			  ALTR_A10SR_PER_RST);
>> +static SENSOR_DEVICE_ATTR(max5_sfpa, S_IRUGO | S_IWUSR,
>> +			  altr_a10sr_read_status, set_enable, ALTR_A10SR_SFPA);
>> +static SENSOR_DEVICE_ATTR(max5_sfpb, S_IRUGO | S_IWUSR,
>> +			  altr_a10sr_read_status, set_enable, ALTR_A10SR_SFPB);
>> +static SENSOR_DEVICE_ATTR(max5_i2c_master, S_IRUGO | S_IWUSR,
>> +			  altr_a10sr_read_status, set_enable,
>> +			  ALTR_A10SR_I2C_MASTER);
>> +static SENSOR_DEVICE_ATTR(max5_pmbus, S_IRUGO | S_IWUSR,
>> +			  altr_a10sr_read_status, set_enable,
>> +			  ALTR_A10SR_PMBUS);
>> +
>> +static DEVICE_ATTR(name, S_IRUGO, altr_a10sr_hwmon_show_name, NULL);
>> +
>> +static struct attribute *altr_a10sr_attr[] = {
>> +	&dev_attr_name.attr,
>> +	/* First Power Good Register */
>> +	&sensor_dev_attr_1v0_input.dev_attr.attr,
>> +	&sensor_dev_attr_1v0_label.dev_attr.attr,
>> +	&sensor_dev_attr_0v95_input.dev_attr.attr,
>> +	&sensor_dev_attr_0v95_label.dev_attr.attr,
>> +	&sensor_dev_attr_0v9_input.dev_attr.attr,
>> +	&sensor_dev_attr_0v9_label.dev_attr.attr,
>> +	&sensor_dev_attr_5v0_input.dev_attr.attr,
>> +	&sensor_dev_attr_5v0_label.dev_attr.attr,
>> +	&sensor_dev_attr_3v3_input.dev_attr.attr,
>> +	&sensor_dev_attr_3v3_label.dev_attr.attr,
>> +	&sensor_dev_attr_2v5_input.dev_attr.attr,
>> +	&sensor_dev_attr_2v5_label.dev_attr.attr,
>> +	&sensor_dev_attr_1v8_input.dev_attr.attr,
>> +	&sensor_dev_attr_1v8_label.dev_attr.attr,
>> +	&sensor_dev_attr_opflag_input.dev_attr.attr,
>> +	&sensor_dev_attr_opflag_label.dev_attr.attr,
>> +	/* Second Power Good Register */
>> +	&sensor_dev_attr_fbc2mp_input.dev_attr.attr,
>> +	&sensor_dev_attr_fbc2mp_label.dev_attr.attr,
>> +	&sensor_dev_attr_fac2mp_input.dev_attr.attr,
>> +	&sensor_dev_attr_fac2mp_label.dev_attr.attr,
>> +	&sensor_dev_attr_fmcbvadj_input.dev_attr.attr,
>> +	&sensor_dev_attr_fmcbvadj_label.dev_attr.attr,
>> +	&sensor_dev_attr_fmcavadj_input.dev_attr.attr,
>> +	&sensor_dev_attr_fmcavadj_label.dev_attr.attr,
>> +	&sensor_dev_attr_hl_vddq_input.dev_attr.attr,
>> +	&sensor_dev_attr_hl_vddq_label.dev_attr.attr,
>> +	&sensor_dev_attr_hl_vdd_input.dev_attr.attr,
>> +	&sensor_dev_attr_hl_vdd_label.dev_attr.attr,
>> +	&sensor_dev_attr_hlhps_vdd_input.dev_attr.attr,
>> +	&sensor_dev_attr_hlhps_vdd_label.dev_attr.attr,
>> +	&sensor_dev_attr_hps_input.dev_attr.attr,
>> +	&sensor_dev_attr_hps_label.dev_attr.attr,
>> +	/* Third Power Good Register */
>> +	&sensor_dev_attr_pcie_wake_input.dev_attr.attr,
>> +	&sensor_dev_attr_pcie_wake_label.dev_attr.attr,
>> +	&sensor_dev_attr_pcie_pr_input.dev_attr.attr,
>> +	&sensor_dev_attr_pcie_pr_label.dev_attr.attr,
>> +	&sensor_dev_attr_fmcb_pr_input.dev_attr.attr,
>> +	&sensor_dev_attr_fmcb_pr_label.dev_attr.attr,
>> +	&sensor_dev_attr_fmca_pr_input.dev_attr.attr,
>> +	&sensor_dev_attr_fmca_pr_label.dev_attr.attr,
>> +	&sensor_dev_attr_file_pr_input.dev_attr.attr,
>> +	&sensor_dev_attr_file_pr_label.dev_attr.attr,
>> +	&sensor_dev_attr_bf_pr_input.dev_attr.attr,
>> +	&sensor_dev_attr_bf_pr_label.dev_attr.attr,
>> +	&sensor_dev_attr_10v_fail_input.dev_attr.attr,
>> +	&sensor_dev_attr_10v_fail_label.dev_attr.attr,
>> +	&sensor_dev_attr_fam2c_input.dev_attr.attr,
>> +	&sensor_dev_attr_fam2c_label.dev_attr.attr,
>> +	/* Peripheral Enable Register */
>> +	&sensor_dev_attr_fmcb_aux_en.dev_attr.attr,
>> +	&sensor_dev_attr_fmcb_en.dev_attr.attr,
>> +	&sensor_dev_attr_fmca_aux_en.dev_attr.attr,
>> +	&sensor_dev_attr_fmca_en.dev_attr.attr,
>> +	&sensor_dev_attr_pcie_aux_en.dev_attr.attr,
>> +	&sensor_dev_attr_pcie_en.dev_attr.attr,
>> +	/* HPS Reset bits */
>> +	&sensor_dev_attr_hps_uart_rst.dev_attr.attr,
>> +	&sensor_dev_attr_hps_warm_rst.dev_attr.attr,
>> +	&sensor_dev_attr_hps_warm1_rst.dev_attr.attr,
>> +	&sensor_dev_attr_hps_cold_rst.dev_attr.attr,
>> +	&sensor_dev_attr_hps_npor.dev_attr.attr,
>> +	&sensor_dev_attr_hps_nrst.dev_attr.attr,
>> +	&sensor_dev_attr_hps_enet_rst.dev_attr.attr,
>> +	&sensor_dev_attr_hps_enet_int.dev_attr.attr,
>> +	/* Peripheral Reset bits */
>> +	&sensor_dev_attr_usb_reset.dev_attr.attr,
>> +	&sensor_dev_attr_bqspi_resetn.dev_attr.attr,
>> +	&sensor_dev_attr_file_resetn.dev_attr.attr,
>> +	&sensor_dev_attr_pcie_perstn.dev_attr.attr,
>> +	/* Byte Value Register */
>> +	&sensor_dev_attr_max5_version.dev_attr.attr,
>> +	&sensor_dev_attr_max5_led.dev_attr.attr,
>> +	&sensor_dev_attr_max5_button.dev_attr.attr,
>> +	&sensor_dev_attr_max5_button_irq.dev_attr.attr,
>> +	&sensor_dev_attr_max5_pg1.dev_attr.attr,
>> +	&sensor_dev_attr_max5_pg2.dev_attr.attr,
>> +	&sensor_dev_attr_max5_pg3.dev_attr.attr,
>> +	&sensor_dev_attr_max5_fmcab.dev_attr.attr,
>> +	&sensor_dev_attr_max5_hps_resets.dev_attr.attr,
>> +	&sensor_dev_attr_max5_per_resets.dev_attr.attr,
>> +	&sensor_dev_attr_max5_sfpa.dev_attr.attr,
>> +	&sensor_dev_attr_max5_sfpb.dev_attr.attr,
>> +	&sensor_dev_attr_max5_i2c_master.dev_attr.attr,
>> +	&sensor_dev_attr_max5_pmbus.dev_attr.attr,
>> +	NULL
>> +};
>> +
>> +static const struct attribute_group altr_a10sr_attr_group = {
>> +	.attrs = altr_a10sr_attr
>> +};
>> +
>> +static int altr_a10sr_hwmon_probe(struct platform_device *pdev)
>> +{
>> +	struct altr_a10sr_hwmon *hwmon;
>> +	int ret;
>> +
>> +	hwmon = devm_kzalloc(&pdev->dev, sizeof(*hwmon), GFP_KERNEL);
>> +	if (!hwmon)
>> +		return -ENOMEM;
>> +
>> +	hwmon->a10sr = dev_get_drvdata(pdev->dev.parent);
>> +
>> +	platform_set_drvdata(pdev, hwmon);
>> +
>> +	ret = sysfs_create_group(&pdev->dev.kobj, &altr_a10sr_attr_group);
>> +	if (ret)
>> +		goto err_mem;
>> +
>> +	hwmon->class_device = hwmon_device_register(&pdev->dev);
>> +	if (IS_ERR(hwmon->class_device)) {
>> +		ret = PTR_ERR(hwmon->class_device);
>> +		goto err_sysfs;
>> +	}
>> +
>> +	return 0;
>> +
>> +err_sysfs:
>> +	sysfs_remove_group(&pdev->dev.kobj, &altr_a10sr_attr_group);
>> +err_mem:
>> +	return ret;
>> +}
>> +
>> +static int altr_a10sr_hwmon_remove(struct platform_device *pdev)
>> +{
>> +	struct altr_a10sr_hwmon *hwmon = platform_get_drvdata(pdev);
>> +
>> +	hwmon_device_unregister(hwmon->class_device);
>> +	sysfs_remove_group(&pdev->dev.kobj, &altr_a10sr_attr_group);
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct of_device_id altr_a10sr_hwmon_of_match[] = {
>> +	{ .compatible = "altr,a10sr-hwmon" },
>> +	{ },
>> +};
>> +MODULE_DEVICE_TABLE(of, altr_a10sr_hwmon_of_match);
>> +
>> +static struct platform_driver altr_a10sr_hwmon_driver = {
>> +	.probe = altr_a10sr_hwmon_probe,
>> +	.remove = altr_a10sr_hwmon_remove,
>> +	.driver = {
>> +		.name = "altr_a10sr_hwmon",
>> +		.of_match_table = altr_a10sr_hwmon_of_match,
>> +	},
>> +};
>> +
>> +module_platform_driver(altr_a10sr_hwmon_driver);
>> +
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_AUTHOR("Thor Thayer");
>> +MODULE_DESCRIPTION("HW Monitor driver for Altera Arria10 System Resource Chip");
>> diff --git a/drivers/mfd/altera-a10sr.c b/drivers/mfd/altera-a10sr.c
>> index 517b895..3eedad7 100644
>> --- a/drivers/mfd/altera-a10sr.c
>> +++ b/drivers/mfd/altera-a10sr.c
>> @@ -34,6 +34,10 @@ static const struct mfd_cell altr_a10sr_subdev_info[] = {
>>   		.name = "altr_a10sr_gpio",
>>   		.of_compatible = "altr,a10sr-gpio",
>>   	},
>> +	{
>> +		.name = "altr_a10sr_hwmon",
>> +		.of_compatible = "altr,a10sr-hwmon",
>> +	},
>>   };
>>
>>   static bool altr_a10sr_reg_readable(struct device *dev, unsigned int reg)
>> diff --git a/include/linux/mfd/altera-a10sr.h b/include/linux/mfd/altera-a10sr.h
>> index 6d254a1..2bfc63e 100644
>> --- a/include/linux/mfd/altera-a10sr.h
>> +++ b/include/linux/mfd/altera-a10sr.h
>> @@ -75,26 +75,93 @@
>>   #define ALTR_A10SR_IN_VALID_RANGE_LO      8
>>   #define ALTR_A10SR_IN_VALID_RANGE_HI      15
>>
>> -#define ALTR_A10SR_PWR_GOOD1_RD_REG   0x09    /* Power Good1 Read */
>> -#define ALTR_A10SR_PWR_GOOD2_RD_REG   0x0B    /* Power Good2 Read */
>> -#define ALTR_A10SR_PWR_GOOD3_RD_REG   0x0D    /* Power Good3 Read */
>> -#define ALTR_A10SR_FMCAB_WR_REG       0x0E    /* FMCA/B & PCIe Pwr Enable */
>> -#define ALTR_A10SR_FMCAB_RD_REG       0x0F    /* FMCA/B & PCIe Pwr Enable */
>> -#define ALTR_A10SR_HPS_RST_WR_REG     0x10    /* HPS Reset */
>> -#define ALTR_A10SR_HPS_RST_RD_REG     0x11    /* HPS Reset */
>> -#define ALTR_A10SR_USB_QSPI_WR_REG    0x12    /* USB, BQSPI, FILE Reset */
>> -#define ALTR_A10SR_USB_QSPI_RD_REG    0x13    /* USB, BQSPI, FILE Reset */
>> -#define ALTR_A10SR_SFPA_WR_REG        0x14    /* SFPA Control Reg */
>> -#define ALTR_A10SR_SFPA_RD_REG        0x15    /* SFPA Control Reg */
>> -#define ALTR_A10SR_SFPB_WR_REG        0x16    /* SFPB Control Reg */
>> -#define ALTR_A10SR_SFPB_RD_REG        0x17    /* SFPB Control Reg */
>> -#define ALTR_A10SR_I2C_M_RD_REG       0x19    /* I2C Master Select */
>> -#define ALTR_A10SR_WARM_RST_WR_REG    0x1A    /* HPS Warm Reset */
>> -#define ALTR_A10SR_WARM_RST_RD_REG    0x1B    /* HPS Warm Reset */
>> -#define ALTR_A10SR_WR_KEY_WR_REG      0x1C    /* HPS Warm Reset Key */
>> -#define ALTR_A10SR_WR_KEY_RD_REG      0x1D    /* HPS Warm Reset Key */
>> -#define ALTR_A10SR_PMBUS_WR_REG       0x1E    /* HPS PM Bus */
>> -#define ALTR_A10SR_PMBUS_RD_REG       0x1F    /* HPS PM Bus */
>> +#define ALTR_A10SR_PWR_GOOD1_RD_REG  0x09     /* Power Good1 Read */
>> +/* Power Good #1 Register Bit Definitions */
>> +#define ALTR_A10SR_PG1_OP_FLAG_SHIFT    7       /* Power On Complete */
>> +#define ALTR_A10SR_PG1_1V8_SHIFT        6       /* 1.8V Power Good */
>> +#define ALTR_A10SR_PG1_2V5_SHIFT        5       /* 2.5V Power Good */
>> +#define ALTR_A10SR_PG1_3V3_SHIFT        4       /* 3.3V Power Good */
>> +#define ALTR_A10SR_PG1_5V0_SHIFT        3       /* 5.0V Power Good */
>> +#define ALTR_A10SR_PG1_0V9_SHIFT        2       /* 0.9V Power Good */
>> +#define ALTR_A10SR_PG1_0V95_SHIFT       1       /* 0.95V Power Good */
>> +#define ALTR_A10SR_PG1_1V0_SHIFT        0       /* 1.0V Power Good */
>> +
>> +#define ALTR_A10SR_PWR_GOOD2_RD_REG  0x0B     /* Power Good2 Read */
>> +/* Power Good #2 Register Bit Definitions */
>> +#define ALTR_A10SR_PG2_HPS_SHIFT        7       /* HPS Power Good */
>> +#define ALTR_A10SR_PG2_HL_HPS_SHIFT     6       /* HILOHPS_VDD Power Good */
>> +#define ALTR_A10SR_PG2_HL_VDD_SHIFT     5       /* HILO VDD Power Good */
>> +#define ALTR_A10SR_PG2_HL_VDDQ_SHIFT    4       /* HILO VDDQ Power Good */
>> +#define ALTR_A10SR_PG2_FMCAVADJ_SHIFT   3       /* FMCA VADJ Power Good */
>> +#define ALTR_A10SR_PG2_FMCBVADJ_SHIFT   2       /* FMCB VADJ Power Good */
>> +#define ALTR_A10SR_PG2_FAC2MP_SHIFT     1       /* FAC2MP Power Good */
>> +#define ALTR_A10SR_PG2_FBC2MP_SHIFT     0       /* FBC2MP Power Good */
>> +
>> +#define ALTR_A10SR_PWR_GOOD3_RD_REG  0x0D   /* Power Good3 Read */
>> +/* Power Good #3 Register Bit Definitions */
>> +#define ALTR_A10SR_PG3_FAM2C_SHIFT      7       /* FAM2C Power Good */
>> +#define ALTR_A10SR_PG3_10V_FAIL_SHIFT   6       /* 10V Fail n */
>> +#define ALTR_A10SR_PG3_BF_PR_SHIFT      5       /* BF Present n */
>> +#define ALTR_A10SR_PG3_FILE_PR_SHIFT    4       /* File Present n */
>> +#define ALTR_A10SR_PG3_FMCA_PR_SHIFT    3       /* FMCA Present n */
>> +#define ALTR_A10SR_PG3_FMCB_PR_SHIFT    2       /* FMCB Present n */
>> +#define ALTR_A10SR_PG3_PCIE_PR_SHIFT    1       /* PCIE Present n */
>> +#define ALTR_A10SR_PG3_PCIE_WAKE_SHIFT  0       /* PCIe Wake N */
>> +
>> +#define ALTR_A10SR_FMCAB_WR_REG      0x0E    /* FMCA/B & PCIe Pwr Enable */
>> +#define ALTR_A10SR_FMCAB_RD_REG      0x0F    /* FMCA/B & PCIe Pwr Enable */
>> +/* FMCA/B & PCIe Power Bit Definitions */
>> +#define ALTR_A10SR_PCIE_EN_SHIFT        7       /* PCIe Pwr Enable */
>> +#define ALTR_A10SR_PCIE_AUXEN_SHIFT     6       /* PCIe Aux Pwr Enable */
>> +#define ALTR_A10SR_FMCA_EN_SHIFT        5       /* FMCA Pwr Enable */
>> +#define ALTR_A10SR_FMCA_AUXEN_SHIFT     4       /* FMCA Aux Pwr Enable */
>> +#define ALTR_A10SR_FMCB_EN_SHIFT        3       /* FMCB Pwr Enable */
>> +#define ALTR_A10SR_FMCB_AUXEN_SHIFT     2       /* FMCB Aux Pwr Enable */
>> +
>> +#define ALTR_A10SR_HPS_RST_WR_REG    0x10    /* HPS Reset */
>> +#define ALTR_A10SR_HPS_RST_RD_REG    0x11    /* HPS Reset */
>> +/* HPS Reset Bit Definitions */
>> +#define ALTR_A10SR_HPS_UARTA_RSTN_SHIFT 7       /* UARTA Reset n */
>> +#define ALTR_A10SR_HPS_WARM_RSTN_SHIFT  6       /* WARM Reset n */
>> +#define ALTR_A10SR_HPS_WARM_RST1N_SHIFT 5       /* WARM Reset1 n */
>> +#define ALTR_A10SR_HPS_COLD_RSTN_SHIFT  4       /* COLD Reset n */
>> +#define ALTR_A10SR_HPS_NPOR_SHIFT       3       /* N Power On Reset */
>> +#define ALTR_A10SR_HPS_NRST_SHIFT       2       /* N Reset */
>> +#define ALTR_A10SR_HPS_ENET_RSTN_SHIFT  1       /* Ethernet Reset n */
>> +#define ALTR_A10SR_HPS_ENET_INTN_SHIFT  0       /* Ethernet IRQ n */
>> +
>> +#define ALTR_A10SR_USB_QSPI_WR_REG   0x12    /* USB, BQSPI, FILE Reset */
>> +#define ALTR_A10SR_USB_QSPI_RD_REG   0x13    /* USB, BQSPI, FILE Reset */
>> +/* USB/QSPI/FILE Reset Bit Definitions */
>> +#define ALTR_A10SR_USB_RST_SHIFT        7       /* USB Reset */
>> +#define ALTR_A10SR_BQSPI_RST_N_SHIFT    6       /* BQSPI Reset n */
>> +#define ALTR_A10SR_FILE_RST_N_SHIFT     5       /* FILE Reset n */
>> +#define ALTR_A10SR_PCIE_PERST_N_SHIFT   4       /* PCIe PE Reset n */
>> +
>> +#define ALTR_A10SR_SFPA_WR_REG       0x14    /* SFPA Control Reg */
>> +#define ALTR_A10SR_SFPA_RD_REG       0x15    /* SFPA Control Reg */
>> +#define ALTR_A10SR_SFPB_WR_REG       0x16    /* SFPB Control Reg */
>> +#define ALTR_A10SR_SFPB_RD_REG       0x17    /* SFPB Control Reg */
>> +/* SFPA Bit Definitions */
>> +#define ALTR_A10SR_SFP_TXDIS_SHIFT      7       /* SFPA TX Disable */
>> +#define ALTR_A10SR_SFP_RATESEL10        0x60    /* SFPA_Rate Select [1:0] */
>> +#define ALTR_A10SR_SFP_LOS_SHIFT        4       /* SFPA LOS */
>> +#define ALTR_A10SR_SFP_FAULT_SHIFT      3       /* SFPA Fault */
>> +
>> +#define ALTR_A10SR_I2C_M_RD_REG      0x19    /* I2C Master Select */
>> +
>> +#define ALTR_A10SR_WARM_RST_WR_REG   0x1A    /* HPS Warm Reset */
>> +#define ALTR_A10SR_WARM_RST_RD_REG   0x1B    /* HPS Warm Reset */
>> +
>> +#define ALTR_A10SR_WR_KEY_WR_REG     0x1C    /* HPS Warm Reset Key */
>> +#define ALTR_A10SR_WR_KEY_RD_REG     0x1D    /* HPS Warm Reset Key */
>> +
>> +#define ALTR_A10SR_PMBUS_WR_REG      0x1E    /* HPS PM Bus */
>> +#define ALTR_A10SR_PMBUS_RD_REG      0x1F    /* HPS PM Bus */
>> +/* PM Bus Bit Definitions */
>> +#define ALTR_A10SR_PMBUS_EN_SHIFT       7       /* PMBus FPGA Enable */
>> +#define ALTR_A10SR_PMBUS_DISN_SHIFT     6       /* PMBus HPS Enable */
>> +#define ALTR_A10SR_PMBUS_ALERTN_SHIFT   5       /* PMBus Alert */
>>
>>   struct altr_a10sr {
>>   	struct device *dev;
>> --
>> 1.7.9.5
>>

Re: [RFC 7/8] hwmon: Altera Arria10 System Resource Chip - HW Monitor

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 641 bytes --]

On Tue, Mar 29, 2016 at 04:43:41PM -0500, Thor Thayer wrote:

> Yes, I see your point. In looking at the regulator drivers, I interpret
> those as being controlled by the driver whereas this chip is passively
> reporting status.

If the device isn't doing regulation it's not a regulator.

> After reading the referenced document, I agree that hwmon probably isn't
> appropriate. However, the regulator doesn't seem appropriate either (the
> only status appears to be tied to battery properties).

We report status for regulators but that's tied to monitoring that's
built into a device that does regulation.

How about IIO or power supply?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [RFC 7/8] hwmon: Altera Arria10 System Resource Chip - HW Monitor

[Thor Thayer]

On 03/29/2016 05:29 PM, Mark Brown wrote:
> On Tue, Mar 29, 2016 at 04:43:41PM -0500, Thor Thayer wrote:
>
>> Yes, I see your point. In looking at the regulator drivers, I interpret
>> those as being controlled by the driver whereas this chip is passively
>> reporting status.
>
> If the device isn't doing regulation it's not a regulator.
>
>> After reading the referenced document, I agree that hwmon probably isn't
>> appropriate. However, the regulator doesn't seem appropriate either (the
>> only status appears to be tied to battery properties).
>
> We report status for regulators but that's tied to monitoring that's
> built into a device that does regulation.
>
> How about IIO or power supply?
>

Hi Mark,

Thanks for the clarification and for pointing out other frameworks to 
look at.

Yes, I think the iio may be a good place but I'll need to investigate 
where it should go. I'm not sure it fits into the adc subdirectory. I 
see similar functionality in the MAX1363 part but it's not actually an 
ADC - it only has the supervisor compare functionality.

Thank you!

Re: [RFC 7/8] hwmon: Altera Arria10 System Resource Chip - HW Monitor

[Guenter Roeck]

On Tue, Mar 29, 2016 at 04:43:41PM -0500, Thor Thayer wrote:
> 
> 
> On 03/29/2016 03:16 PM, Guenter Roeck wrote:
> >On Tue, Mar 29, 2016 at 02:13:10PM -0500, tthayer@opensource.altera.com wrote:
> >>From: Thor Thayer <tthayer@opensource.altera.com>
> >>
> >>This patch adds the hwmon functionality to the Arria10 System
> >>Resource Chip. The hwmon encapsulates the PCIe Enable, USB Enable,
> >>and all the Power Good signals on the System Controller.
> >>
> >
> >I may be completely wrong, but a glance through the driver suggests
> >that, if anything, this should be a regulator driver, not a hwmon driver.
> >A hardware monitoring driver would be expected to report the voltages,
> >not (just) the voltage status. Am I missing something ?
> >
> >Please have a look into Documentation/hwmon/sysfs-interface for
> >acceptable hwmon attribute names and their meaning.
> >
> >Thanks,
> >Guenter
> >
> 
> Hi Guenter,
> 
> <adding voltage and current regulator framework moderators>
> 
> Yes, I see your point. In looking at the regulator drivers, I interpret
> those as being controlled by the driver whereas this chip is passively
> reporting status.
> 
> The success/fail indication seemed at first glance to fit the hwmon model. I
> thought the fan indication would be a good analog but even it reports speed
> and not success/fail.
> 
Yes, alarm and/or fault attributes are supposed to be secondary.

> After reading the referenced document, I agree that hwmon probably isn't
> appropriate. However, the regulator doesn't seem appropriate either (the
> only status appears to be tied to battery properties).
> 
Not really sure myself where this would fit if it is just status bits.

Does the chip report anything else besides the status ? One of the attributes
includes "pmbus", so one could conclude that there must be a PMBus compatible
chip somewhere.

Thanks,
Guenter

Re: [RFC 7/8] hwmon: Altera Arria10 System Resource Chip - HW Monitor

[Lee Jones]

On Tue, 29 Mar 2016, tthayer@opensource.altera.com wrote:

> From: Thor Thayer <tthayer@opensource.altera.com>
> 
> This patch adds the hwmon functionality to the Arria10 System
> Resource Chip. The hwmon encapsulates the PCIe Enable, USB Enable,
> and all the Power Good signals on the System Controller.
> 
> Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
> ---
>  drivers/hwmon/Kconfig              |    9 +
>  drivers/hwmon/Makefile             |    1 +
>  drivers/hwmon/altera-a10sr-hwmon.c |  544 ++++++++++++++++++++++++++++++++++++
>  drivers/mfd/altera-a10sr.c         |    4 +
>  include/linux/mfd/altera-a10sr.h   |  107 +++++--
>  5 files changed, 645 insertions(+), 20 deletions(-)
>  create mode 100644 drivers/hwmon/altera-a10sr-hwmon.c

[...]

> diff --git a/drivers/mfd/altera-a10sr.c b/drivers/mfd/altera-a10sr.c
> index 517b895..3eedad7 100644
> --- a/drivers/mfd/altera-a10sr.c
> +++ b/drivers/mfd/altera-a10sr.c
> @@ -34,6 +34,10 @@ static const struct mfd_cell altr_a10sr_subdev_info[] = {
>  		.name = "altr_a10sr_gpio",
>  		.of_compatible = "altr,a10sr-gpio",
>  	},
> +	{
> +		.name = "altr_a10sr_hwmon",
> +		.of_compatible = "altr,a10sr-hwmon",
> +	},
>  };
>  
>  static bool altr_a10sr_reg_readable(struct device *dev, unsigned int reg)

This belongs in a patch of its own.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html