* [PATCH v2] ARM: zynq: Add #io-channel-cells to (x)adc node for iio-hwmon
@ 2017-03-08 20:11 ` Moritz Fischer
0 siblings, 0 replies; 36+ messages in thread
From: Moritz Fischer @ 2017-03-08 20:11 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Moritz Fischer, Michal Simek, Sören Brinkmann,
Julia Cartwright, linux-kernel, devicetree
Fix
OF: /iio_hwmon: could not get #io-channel-cells for
/amba/adc@f8007100
OF: /iio_hwmon: could not get #io-channel-cells for
/amba/adc@f8007100
OF: /iio_hwmon: could not get #io-channel-cells for
/amba/adc@f8007100
by adding the #io-channel-cells property.
Signed-off-by: Moritz Fischer <mdf@kernel.org>
Cc: Michal Simek <michal.simek@xilinx.com>
Cc: Sören Brinkmann <soren.brinkmann@xilinx.com>
Cc: Julia Cartwright <julia@ni.com>
Cc: linux-kernel@vger.kernel.org
Cc: devicetree@vger.kernel.org
---
Changes from v1:
- fix messed up commit message
---
arch/arm/boot/dts/zynq-7000.dtsi | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
index f3ac9bf..98233a8 100644
--- a/arch/arm/boot/dts/zynq-7000.dtsi
+++ b/arch/arm/boot/dts/zynq-7000.dtsi
@@ -72,6 +72,7 @@
interrupts = <0 7 4>;
interrupt-parent = <&intc>;
clocks = <&clkc 12>;
+ #io-channel-cells = <1>;
};
can0: can@e0008000 {
--
2.7.4
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH v2] ARM: zynq: Add #io-channel-cells to (x)adc node for iio-hwmon
@ 2017-03-08 20:11 ` Moritz Fischer
0 siblings, 0 replies; 36+ messages in thread
From: Moritz Fischer @ 2017-03-08 20:11 UTC (permalink / raw)
To: linux-arm-kernel
Fix
OF: /iio_hwmon: could not get #io-channel-cells for
/amba/adc at f8007100
OF: /iio_hwmon: could not get #io-channel-cells for
/amba/adc at f8007100
OF: /iio_hwmon: could not get #io-channel-cells for
/amba/adc at f8007100
by adding the #io-channel-cells property.
Signed-off-by: Moritz Fischer <mdf@kernel.org>
Cc: Michal Simek <michal.simek@xilinx.com>
Cc: S?ren Brinkmann <soren.brinkmann@xilinx.com>
Cc: Julia Cartwright <julia@ni.com>
Cc: linux-kernel at vger.kernel.org
Cc: devicetree at vger.kernel.org
---
Changes from v1:
- fix messed up commit message
---
arch/arm/boot/dts/zynq-7000.dtsi | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
index f3ac9bf..98233a8 100644
--- a/arch/arm/boot/dts/zynq-7000.dtsi
+++ b/arch/arm/boot/dts/zynq-7000.dtsi
@@ -72,6 +72,7 @@
interrupts = <0 7 4>;
interrupt-parent = <&intc>;
clocks = <&clkc 12>;
+ #io-channel-cells = <1>;
};
can0: can at e0008000 {
--
2.7.4
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH v2] ARM: zynq: Add #io-channel-cells to (x)adc node for iio-hwmon
2017-03-08 20:11 ` Moritz Fischer
(?)
@ 2017-03-16 16:16 ` Michal Simek
-1 siblings, 0 replies; 36+ messages in thread
From: Michal Simek @ 2017-03-16 16:16 UTC (permalink / raw)
To: Moritz Fischer, linux-arm-kernel
Cc: Michal Simek, Sören Brinkmann, Julia Cartwright,
linux-kernel, devicetree
Hi,
On 8.3.2017 21:11, Moritz Fischer wrote:
> Fix
>
> OF: /iio_hwmon: could not get #io-channel-cells for
> /amba/adc@f8007100
> OF: /iio_hwmon: could not get #io-channel-cells for
> /amba/adc@f8007100
> OF: /iio_hwmon: could not get #io-channel-cells for
> /amba/adc@f8007100
>
> by adding the #io-channel-cells property.
>
> Signed-off-by: Moritz Fischer <mdf@kernel.org>
> Cc: Michal Simek <michal.simek@xilinx.com>
> Cc: Sören Brinkmann <soren.brinkmann@xilinx.com>
> Cc: Julia Cartwright <julia@ni.com>
> Cc: linux-kernel@vger.kernel.org
> Cc: devicetree@vger.kernel.org
> ---
>
> Changes from v1:
> - fix messed up commit message
> ---
> arch/arm/boot/dts/zynq-7000.dtsi | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
> index f3ac9bf..98233a8 100644
> --- a/arch/arm/boot/dts/zynq-7000.dtsi
> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
> @@ -72,6 +72,7 @@
> interrupts = <0 7 4>;
> interrupt-parent = <&intc>;
> clocks = <&clkc 12>;
> + #io-channel-cells = <1>;
> };
>
> can0: can@e0008000 {
>
I think it will be good to the next step too.
It means also add iio-hwmon node too.
What do you think?
Thanks,
Michal
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2] ARM: zynq: Add #io-channel-cells to (x)adc node for iio-hwmon
@ 2017-03-16 16:16 ` Michal Simek
0 siblings, 0 replies; 36+ messages in thread
From: Michal Simek @ 2017-03-16 16:16 UTC (permalink / raw)
To: Moritz Fischer, linux-arm-kernel
Cc: linux-kernel, devicetree, Michal Simek, Sören Brinkmann,
Julia Cartwright
Hi,
On 8.3.2017 21:11, Moritz Fischer wrote:
> Fix
>
> OF: /iio_hwmon: could not get #io-channel-cells for
> /amba/adc@f8007100
> OF: /iio_hwmon: could not get #io-channel-cells for
> /amba/adc@f8007100
> OF: /iio_hwmon: could not get #io-channel-cells for
> /amba/adc@f8007100
>
> by adding the #io-channel-cells property.
>
> Signed-off-by: Moritz Fischer <mdf@kernel.org>
> Cc: Michal Simek <michal.simek@xilinx.com>
> Cc: Sören Brinkmann <soren.brinkmann@xilinx.com>
> Cc: Julia Cartwright <julia@ni.com>
> Cc: linux-kernel@vger.kernel.org
> Cc: devicetree@vger.kernel.org
> ---
>
> Changes from v1:
> - fix messed up commit message
> ---
> arch/arm/boot/dts/zynq-7000.dtsi | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
> index f3ac9bf..98233a8 100644
> --- a/arch/arm/boot/dts/zynq-7000.dtsi
> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
> @@ -72,6 +72,7 @@
> interrupts = <0 7 4>;
> interrupt-parent = <&intc>;
> clocks = <&clkc 12>;
> + #io-channel-cells = <1>;
> };
>
> can0: can@e0008000 {
>
I think it will be good to the next step too.
It means also add iio-hwmon node too.
What do you think?
Thanks,
Michal
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v2] ARM: zynq: Add #io-channel-cells to (x)adc node for iio-hwmon
@ 2017-03-16 16:16 ` Michal Simek
0 siblings, 0 replies; 36+ messages in thread
From: Michal Simek @ 2017-03-16 16:16 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On 8.3.2017 21:11, Moritz Fischer wrote:
> Fix
>
> OF: /iio_hwmon: could not get #io-channel-cells for
> /amba/adc at f8007100
> OF: /iio_hwmon: could not get #io-channel-cells for
> /amba/adc at f8007100
> OF: /iio_hwmon: could not get #io-channel-cells for
> /amba/adc at f8007100
>
> by adding the #io-channel-cells property.
>
> Signed-off-by: Moritz Fischer <mdf@kernel.org>
> Cc: Michal Simek <michal.simek@xilinx.com>
> Cc: S?ren Brinkmann <soren.brinkmann@xilinx.com>
> Cc: Julia Cartwright <julia@ni.com>
> Cc: linux-kernel at vger.kernel.org
> Cc: devicetree at vger.kernel.org
> ---
>
> Changes from v1:
> - fix messed up commit message
> ---
> arch/arm/boot/dts/zynq-7000.dtsi | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
> index f3ac9bf..98233a8 100644
> --- a/arch/arm/boot/dts/zynq-7000.dtsi
> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
> @@ -72,6 +72,7 @@
> interrupts = <0 7 4>;
> interrupt-parent = <&intc>;
> clocks = <&clkc 12>;
> + #io-channel-cells = <1>;
> };
>
> can0: can at e0008000 {
>
I think it will be good to the next step too.
It means also add iio-hwmon node too.
What do you think?
Thanks,
Michal
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2] ARM: zynq: Add #io-channel-cells to (x)adc node for iio-hwmon
2017-03-16 16:16 ` Michal Simek
(?)
@ 2017-03-16 16:39 ` Moritz Fischer
-1 siblings, 0 replies; 36+ messages in thread
From: Moritz Fischer @ 2017-03-16 16:39 UTC (permalink / raw)
To: Michal Simek
Cc: linux-arm-kernel, Sören Brinkmann, Julia Cartwright,
Linux Kernel Mailing List, Devicetree List
On Thu, Mar 16, 2017 at 9:16 AM, Michal Simek <michal.simek@xilinx.com> wrote:
> Hi,
>
> On 8.3.2017 21:11, Moritz Fischer wrote:
>> Fix
>>
>> OF: /iio_hwmon: could not get #io-channel-cells for
>> /amba/adc@f8007100
>> OF: /iio_hwmon: could not get #io-channel-cells for
>> /amba/adc@f8007100
>> OF: /iio_hwmon: could not get #io-channel-cells for
>> /amba/adc@f8007100
>>
>> by adding the #io-channel-cells property.
>>
>> Signed-off-by: Moritz Fischer <mdf@kernel.org>
>> Cc: Michal Simek <michal.simek@xilinx.com>
>> Cc: Sören Brinkmann <soren.brinkmann@xilinx.com>
>> Cc: Julia Cartwright <julia@ni.com>
>> Cc: linux-kernel@vger.kernel.org
>> Cc: devicetree@vger.kernel.org
>> ---
>>
>> Changes from v1:
>> - fix messed up commit message
>> ---
>> arch/arm/boot/dts/zynq-7000.dtsi | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
>> index f3ac9bf..98233a8 100644
>> --- a/arch/arm/boot/dts/zynq-7000.dtsi
>> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
>> @@ -72,6 +72,7 @@
>> interrupts = <0 7 4>;
>> interrupt-parent = <&intc>;
>> clocks = <&clkc 12>;
>> + #io-channel-cells = <1>;
>> };
>>
>> can0: can@e0008000 {
>>
>
> I think it will be good to the next step too.
> It means also add iio-hwmon node too.
>
> What do you think?
I hadn't put it in there since dts is supposed to describe hw,
but obviously putting the actual hwmon in there makes it more useful.
I can resubmit with the hwmon node in there.
Cheers,
Moritz
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2] ARM: zynq: Add #io-channel-cells to (x)adc node for iio-hwmon
@ 2017-03-16 16:39 ` Moritz Fischer
0 siblings, 0 replies; 36+ messages in thread
From: Moritz Fischer @ 2017-03-16 16:39 UTC (permalink / raw)
To: Michal Simek
Cc: Linux Kernel Mailing List, Devicetree List, Sören Brinkmann,
linux-arm-kernel, Julia Cartwright
On Thu, Mar 16, 2017 at 9:16 AM, Michal Simek <michal.simek@xilinx.com> wrote:
> Hi,
>
> On 8.3.2017 21:11, Moritz Fischer wrote:
>> Fix
>>
>> OF: /iio_hwmon: could not get #io-channel-cells for
>> /amba/adc@f8007100
>> OF: /iio_hwmon: could not get #io-channel-cells for
>> /amba/adc@f8007100
>> OF: /iio_hwmon: could not get #io-channel-cells for
>> /amba/adc@f8007100
>>
>> by adding the #io-channel-cells property.
>>
>> Signed-off-by: Moritz Fischer <mdf@kernel.org>
>> Cc: Michal Simek <michal.simek@xilinx.com>
>> Cc: Sören Brinkmann <soren.brinkmann@xilinx.com>
>> Cc: Julia Cartwright <julia@ni.com>
>> Cc: linux-kernel@vger.kernel.org
>> Cc: devicetree@vger.kernel.org
>> ---
>>
>> Changes from v1:
>> - fix messed up commit message
>> ---
>> arch/arm/boot/dts/zynq-7000.dtsi | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
>> index f3ac9bf..98233a8 100644
>> --- a/arch/arm/boot/dts/zynq-7000.dtsi
>> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
>> @@ -72,6 +72,7 @@
>> interrupts = <0 7 4>;
>> interrupt-parent = <&intc>;
>> clocks = <&clkc 12>;
>> + #io-channel-cells = <1>;
>> };
>>
>> can0: can@e0008000 {
>>
>
> I think it will be good to the next step too.
> It means also add iio-hwmon node too.
>
> What do you think?
I hadn't put it in there since dts is supposed to describe hw,
but obviously putting the actual hwmon in there makes it more useful.
I can resubmit with the hwmon node in there.
Cheers,
Moritz
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v2] ARM: zynq: Add #io-channel-cells to (x)adc node for iio-hwmon
@ 2017-03-16 16:39 ` Moritz Fischer
0 siblings, 0 replies; 36+ messages in thread
From: Moritz Fischer @ 2017-03-16 16:39 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Mar 16, 2017 at 9:16 AM, Michal Simek <michal.simek@xilinx.com> wrote:
> Hi,
>
> On 8.3.2017 21:11, Moritz Fischer wrote:
>> Fix
>>
>> OF: /iio_hwmon: could not get #io-channel-cells for
>> /amba/adc at f8007100
>> OF: /iio_hwmon: could not get #io-channel-cells for
>> /amba/adc at f8007100
>> OF: /iio_hwmon: could not get #io-channel-cells for
>> /amba/adc at f8007100
>>
>> by adding the #io-channel-cells property.
>>
>> Signed-off-by: Moritz Fischer <mdf@kernel.org>
>> Cc: Michal Simek <michal.simek@xilinx.com>
>> Cc: S?ren Brinkmann <soren.brinkmann@xilinx.com>
>> Cc: Julia Cartwright <julia@ni.com>
>> Cc: linux-kernel at vger.kernel.org
>> Cc: devicetree at vger.kernel.org
>> ---
>>
>> Changes from v1:
>> - fix messed up commit message
>> ---
>> arch/arm/boot/dts/zynq-7000.dtsi | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
>> index f3ac9bf..98233a8 100644
>> --- a/arch/arm/boot/dts/zynq-7000.dtsi
>> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
>> @@ -72,6 +72,7 @@
>> interrupts = <0 7 4>;
>> interrupt-parent = <&intc>;
>> clocks = <&clkc 12>;
>> + #io-channel-cells = <1>;
>> };
>>
>> can0: can at e0008000 {
>>
>
> I think it will be good to the next step too.
> It means also add iio-hwmon node too.
>
> What do you think?
I hadn't put it in there since dts is supposed to describe hw,
but obviously putting the actual hwmon in there makes it more useful.
I can resubmit with the hwmon node in there.
Cheers,
Moritz
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2] ARM: zynq: Add #io-channel-cells to (x)adc node for iio-hwmon
2017-03-16 16:39 ` Moritz Fischer
(?)
@ 2017-03-16 16:45 ` Michal Simek
-1 siblings, 0 replies; 36+ messages in thread
From: Michal Simek @ 2017-03-16 16:45 UTC (permalink / raw)
To: Moritz Fischer, Michal Simek
Cc: linux-arm-kernel, Sören Brinkmann, Julia Cartwright,
Linux Kernel Mailing List, Devicetree List
On 16.3.2017 17:39, Moritz Fischer wrote:
> On Thu, Mar 16, 2017 at 9:16 AM, Michal Simek <michal.simek@xilinx.com> wrote:
>> Hi,
>>
>> On 8.3.2017 21:11, Moritz Fischer wrote:
>>> Fix
>>>
>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>> /amba/adc@f8007100
>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>> /amba/adc@f8007100
>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>> /amba/adc@f8007100
>>>
>>> by adding the #io-channel-cells property.
>>>
>>> Signed-off-by: Moritz Fischer <mdf@kernel.org>
>>> Cc: Michal Simek <michal.simek@xilinx.com>
>>> Cc: Sören Brinkmann <soren.brinkmann@xilinx.com>
>>> Cc: Julia Cartwright <julia@ni.com>
>>> Cc: linux-kernel@vger.kernel.org
>>> Cc: devicetree@vger.kernel.org
>>> ---
>>>
>>> Changes from v1:
>>> - fix messed up commit message
>>> ---
>>> arch/arm/boot/dts/zynq-7000.dtsi | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
>>> index f3ac9bf..98233a8 100644
>>> --- a/arch/arm/boot/dts/zynq-7000.dtsi
>>> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
>>> @@ -72,6 +72,7 @@
>>> interrupts = <0 7 4>;
>>> interrupt-parent = <&intc>;
>>> clocks = <&clkc 12>;
>>> + #io-channel-cells = <1>;
>>> };
>>>
>>> can0: can@e0008000 {
>>>
>>
>> I think it will be good to the next step too.
>> It means also add iio-hwmon node too.
>>
>> What do you think?
>
> I hadn't put it in there since dts is supposed to describe hw,
> but obviously putting the actual hwmon in there makes it more useful.
I had one discussion about this with Grant in past and it is common
mistake. It is simplification of purpose of dts.
>
> I can resubmit with the hwmon node in there.
If you grep kernel tree you will see that others are using it too.
Also there is accepted binding for that that's why I can't see big
problem with it.
Thanks,
Michal
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2] ARM: zynq: Add #io-channel-cells to (x)adc node for iio-hwmon
@ 2017-03-16 16:45 ` Michal Simek
0 siblings, 0 replies; 36+ messages in thread
From: Michal Simek @ 2017-03-16 16:45 UTC (permalink / raw)
To: Moritz Fischer, Michal Simek
Cc: Linux Kernel Mailing List, Devicetree List, Sören Brinkmann,
linux-arm-kernel, Julia Cartwright
On 16.3.2017 17:39, Moritz Fischer wrote:
> On Thu, Mar 16, 2017 at 9:16 AM, Michal Simek <michal.simek@xilinx.com> wrote:
>> Hi,
>>
>> On 8.3.2017 21:11, Moritz Fischer wrote:
>>> Fix
>>>
>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>> /amba/adc@f8007100
>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>> /amba/adc@f8007100
>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>> /amba/adc@f8007100
>>>
>>> by adding the #io-channel-cells property.
>>>
>>> Signed-off-by: Moritz Fischer <mdf@kernel.org>
>>> Cc: Michal Simek <michal.simek@xilinx.com>
>>> Cc: Sören Brinkmann <soren.brinkmann@xilinx.com>
>>> Cc: Julia Cartwright <julia@ni.com>
>>> Cc: linux-kernel@vger.kernel.org
>>> Cc: devicetree@vger.kernel.org
>>> ---
>>>
>>> Changes from v1:
>>> - fix messed up commit message
>>> ---
>>> arch/arm/boot/dts/zynq-7000.dtsi | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
>>> index f3ac9bf..98233a8 100644
>>> --- a/arch/arm/boot/dts/zynq-7000.dtsi
>>> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
>>> @@ -72,6 +72,7 @@
>>> interrupts = <0 7 4>;
>>> interrupt-parent = <&intc>;
>>> clocks = <&clkc 12>;
>>> + #io-channel-cells = <1>;
>>> };
>>>
>>> can0: can@e0008000 {
>>>
>>
>> I think it will be good to the next step too.
>> It means also add iio-hwmon node too.
>>
>> What do you think?
>
> I hadn't put it in there since dts is supposed to describe hw,
> but obviously putting the actual hwmon in there makes it more useful.
I had one discussion about this with Grant in past and it is common
mistake. It is simplification of purpose of dts.
>
> I can resubmit with the hwmon node in there.
If you grep kernel tree you will see that others are using it too.
Also there is accepted binding for that that's why I can't see big
problem with it.
Thanks,
Michal
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v2] ARM: zynq: Add #io-channel-cells to (x)adc node for iio-hwmon
@ 2017-03-16 16:45 ` Michal Simek
0 siblings, 0 replies; 36+ messages in thread
From: Michal Simek @ 2017-03-16 16:45 UTC (permalink / raw)
To: linux-arm-kernel
On 16.3.2017 17:39, Moritz Fischer wrote:
> On Thu, Mar 16, 2017 at 9:16 AM, Michal Simek <michal.simek@xilinx.com> wrote:
>> Hi,
>>
>> On 8.3.2017 21:11, Moritz Fischer wrote:
>>> Fix
>>>
>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>> /amba/adc at f8007100
>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>> /amba/adc at f8007100
>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>> /amba/adc at f8007100
>>>
>>> by adding the #io-channel-cells property.
>>>
>>> Signed-off-by: Moritz Fischer <mdf@kernel.org>
>>> Cc: Michal Simek <michal.simek@xilinx.com>
>>> Cc: S?ren Brinkmann <soren.brinkmann@xilinx.com>
>>> Cc: Julia Cartwright <julia@ni.com>
>>> Cc: linux-kernel at vger.kernel.org
>>> Cc: devicetree at vger.kernel.org
>>> ---
>>>
>>> Changes from v1:
>>> - fix messed up commit message
>>> ---
>>> arch/arm/boot/dts/zynq-7000.dtsi | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
>>> index f3ac9bf..98233a8 100644
>>> --- a/arch/arm/boot/dts/zynq-7000.dtsi
>>> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
>>> @@ -72,6 +72,7 @@
>>> interrupts = <0 7 4>;
>>> interrupt-parent = <&intc>;
>>> clocks = <&clkc 12>;
>>> + #io-channel-cells = <1>;
>>> };
>>>
>>> can0: can at e0008000 {
>>>
>>
>> I think it will be good to the next step too.
>> It means also add iio-hwmon node too.
>>
>> What do you think?
>
> I hadn't put it in there since dts is supposed to describe hw,
> but obviously putting the actual hwmon in there makes it more useful.
I had one discussion about this with Grant in past and it is common
mistake. It is simplification of purpose of dts.
>
> I can resubmit with the hwmon node in there.
If you grep kernel tree you will see that others are using it too.
Also there is accepted binding for that that's why I can't see big
problem with it.
Thanks,
Michal
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2] ARM: zynq: Add #io-channel-cells to (x)adc node for iio-hwmon
@ 2017-03-16 16:51 ` Lars-Peter Clausen
0 siblings, 0 replies; 36+ messages in thread
From: Lars-Peter Clausen @ 2017-03-16 16:51 UTC (permalink / raw)
To: Michal Simek, Moritz Fischer
Cc: Linux Kernel Mailing List, Devicetree List, Sören Brinkmann,
linux-arm-kernel, Julia Cartwright
On 03/16/2017 05:45 PM, Michal Simek wrote:
> On 16.3.2017 17:39, Moritz Fischer wrote:
>> On Thu, Mar 16, 2017 at 9:16 AM, Michal Simek <michal.simek@xilinx.com> wrote:
>>> Hi,
>>>
>>> On 8.3.2017 21:11, Moritz Fischer wrote:
>>>> Fix
>>>>
>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>> /amba/adc@f8007100
>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>> /amba/adc@f8007100
>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>> /amba/adc@f8007100
>>>>
>>>> by adding the #io-channel-cells property.
>>>>
>>>> Signed-off-by: Moritz Fischer <mdf@kernel.org>
>>>> Cc: Michal Simek <michal.simek@xilinx.com>
>>>> Cc: Sören Brinkmann <soren.brinkmann@xilinx.com>
>>>> Cc: Julia Cartwright <julia@ni.com>
>>>> Cc: linux-kernel@vger.kernel.org
>>>> Cc: devicetree@vger.kernel.org
>>>> ---
>>>>
>>>> Changes from v1:
>>>> - fix messed up commit message
>>>> ---
>>>> arch/arm/boot/dts/zynq-7000.dtsi | 1 +
>>>> 1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
>>>> index f3ac9bf..98233a8 100644
>>>> --- a/arch/arm/boot/dts/zynq-7000.dtsi
>>>> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
>>>> @@ -72,6 +72,7 @@
>>>> interrupts = <0 7 4>;
>>>> interrupt-parent = <&intc>;
>>>> clocks = <&clkc 12>;
>>>> + #io-channel-cells = <1>;
>>>> };
>>>>
>>>> can0: can@e0008000 {
>>>>
>>>
>>> I think it will be good to the next step too.
>>> It means also add iio-hwmon node too.
>>>
>>> What do you think?
>>
>> I hadn't put it in there since dts is supposed to describe hw,
>> but obviously putting the actual hwmon in there makes it more useful.
>
> I had one discussion about this with Grant in past and it is common
> mistake. It is simplification of purpose of dts.
>
If the iio-hwmon binding had gone through review it would have been rejected.
>>
>> I can resubmit with the hwmon node in there.
>
> If you grep kernel tree you will see that others are using it too.
> Also there is accepted binding for that that's why I can't see big
> problem with it.
Since this is an application specific binding I wouldn't put it in the
generic DT include file. It's a bit like adding a gpio-key binding for each
of the GPIOs just in case somebody wants to use it.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2] ARM: zynq: Add #io-channel-cells to (x)adc node for iio-hwmon
@ 2017-03-16 16:51 ` Lars-Peter Clausen
0 siblings, 0 replies; 36+ messages in thread
From: Lars-Peter Clausen @ 2017-03-16 16:51 UTC (permalink / raw)
To: Michal Simek, Moritz Fischer
Cc: Linux Kernel Mailing List, Devicetree List, Sören Brinkmann,
linux-arm-kernel, Julia Cartwright
On 03/16/2017 05:45 PM, Michal Simek wrote:
> On 16.3.2017 17:39, Moritz Fischer wrote:
>> On Thu, Mar 16, 2017 at 9:16 AM, Michal Simek <michal.simek-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org> wrote:
>>> Hi,
>>>
>>> On 8.3.2017 21:11, Moritz Fischer wrote:
>>>> Fix
>>>>
>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>> /amba/adc@f8007100
>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>> /amba/adc@f8007100
>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>> /amba/adc@f8007100
>>>>
>>>> by adding the #io-channel-cells property.
>>>>
>>>> Signed-off-by: Moritz Fischer <mdf-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>>>> Cc: Michal Simek <michal.simek-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
>>>> Cc: Sören Brinkmann <soren.brinkmann-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
>>>> Cc: Julia Cartwright <julia-acOepvfBmUk@public.gmane.org>
>>>> Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>>> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>>> ---
>>>>
>>>> Changes from v1:
>>>> - fix messed up commit message
>>>> ---
>>>> arch/arm/boot/dts/zynq-7000.dtsi | 1 +
>>>> 1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
>>>> index f3ac9bf..98233a8 100644
>>>> --- a/arch/arm/boot/dts/zynq-7000.dtsi
>>>> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
>>>> @@ -72,6 +72,7 @@
>>>> interrupts = <0 7 4>;
>>>> interrupt-parent = <&intc>;
>>>> clocks = <&clkc 12>;
>>>> + #io-channel-cells = <1>;
>>>> };
>>>>
>>>> can0: can@e0008000 {
>>>>
>>>
>>> I think it will be good to the next step too.
>>> It means also add iio-hwmon node too.
>>>
>>> What do you think?
>>
>> I hadn't put it in there since dts is supposed to describe hw,
>> but obviously putting the actual hwmon in there makes it more useful.
>
> I had one discussion about this with Grant in past and it is common
> mistake. It is simplification of purpose of dts.
>
If the iio-hwmon binding had gone through review it would have been rejected.
>>
>> I can resubmit with the hwmon node in there.
>
> If you grep kernel tree you will see that others are using it too.
> Also there is accepted binding for that that's why I can't see big
> problem with it.
Since this is an application specific binding I wouldn't put it in the
generic DT include file. It's a bit like adding a gpio-key binding for each
of the GPIOs just in case somebody wants to use it.
--
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
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v2] ARM: zynq: Add #io-channel-cells to (x)adc node for iio-hwmon
@ 2017-03-16 16:51 ` Lars-Peter Clausen
0 siblings, 0 replies; 36+ messages in thread
From: Lars-Peter Clausen @ 2017-03-16 16:51 UTC (permalink / raw)
To: linux-arm-kernel
On 03/16/2017 05:45 PM, Michal Simek wrote:
> On 16.3.2017 17:39, Moritz Fischer wrote:
>> On Thu, Mar 16, 2017 at 9:16 AM, Michal Simek <michal.simek@xilinx.com> wrote:
>>> Hi,
>>>
>>> On 8.3.2017 21:11, Moritz Fischer wrote:
>>>> Fix
>>>>
>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>> /amba/adc at f8007100
>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>> /amba/adc at f8007100
>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>> /amba/adc at f8007100
>>>>
>>>> by adding the #io-channel-cells property.
>>>>
>>>> Signed-off-by: Moritz Fischer <mdf@kernel.org>
>>>> Cc: Michal Simek <michal.simek@xilinx.com>
>>>> Cc: S?ren Brinkmann <soren.brinkmann@xilinx.com>
>>>> Cc: Julia Cartwright <julia@ni.com>
>>>> Cc: linux-kernel at vger.kernel.org
>>>> Cc: devicetree at vger.kernel.org
>>>> ---
>>>>
>>>> Changes from v1:
>>>> - fix messed up commit message
>>>> ---
>>>> arch/arm/boot/dts/zynq-7000.dtsi | 1 +
>>>> 1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
>>>> index f3ac9bf..98233a8 100644
>>>> --- a/arch/arm/boot/dts/zynq-7000.dtsi
>>>> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
>>>> @@ -72,6 +72,7 @@
>>>> interrupts = <0 7 4>;
>>>> interrupt-parent = <&intc>;
>>>> clocks = <&clkc 12>;
>>>> + #io-channel-cells = <1>;
>>>> };
>>>>
>>>> can0: can at e0008000 {
>>>>
>>>
>>> I think it will be good to the next step too.
>>> It means also add iio-hwmon node too.
>>>
>>> What do you think?
>>
>> I hadn't put it in there since dts is supposed to describe hw,
>> but obviously putting the actual hwmon in there makes it more useful.
>
> I had one discussion about this with Grant in past and it is common
> mistake. It is simplification of purpose of dts.
>
If the iio-hwmon binding had gone through review it would have been rejected.
>>
>> I can resubmit with the hwmon node in there.
>
> If you grep kernel tree you will see that others are using it too.
> Also there is accepted binding for that that's why I can't see big
> problem with it.
Since this is an application specific binding I wouldn't put it in the
generic DT include file. It's a bit like adding a gpio-key binding for each
of the GPIOs just in case somebody wants to use it.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2] ARM: zynq: Add #io-channel-cells to (x)adc node for iio-hwmon
2017-03-16 16:51 ` Lars-Peter Clausen
@ 2017-03-16 17:54 ` Moritz Fischer
-1 siblings, 0 replies; 36+ messages in thread
From: Moritz Fischer @ 2017-03-16 17:54 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: Michal Simek, Linux Kernel Mailing List, Devicetree List,
Sören Brinkmann, linux-arm-kernel, Julia Cartwright
Hi Lars,
On Thu, Mar 16, 2017 at 9:51 AM, Lars-Peter Clausen <lars@metafoo.de> wrote:
> On 03/16/2017 05:45 PM, Michal Simek wrote:
>> On 16.3.2017 17:39, Moritz Fischer wrote:
>>> On Thu, Mar 16, 2017 at 9:16 AM, Michal Simek <michal.simek@xilinx.com> wrote:
>>>> Hi,
>>>>
>>>> On 8.3.2017 21:11, Moritz Fischer wrote:
>>>>> Fix
>>>>>
>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>> /amba/adc@f8007100
>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>> /amba/adc@f8007100
>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>> /amba/adc@f8007100
>>>>>
>>>>> by adding the #io-channel-cells property.
>>>>>
>>>>> Signed-off-by: Moritz Fischer <mdf@kernel.org>
>>>>> Cc: Michal Simek <michal.simek@xilinx.com>
>>>>> Cc: Sören Brinkmann <soren.brinkmann@xilinx.com>
>>>>> Cc: Julia Cartwright <julia@ni.com>
>>>>> Cc: linux-kernel@vger.kernel.org
>>>>> Cc: devicetree@vger.kernel.org
>>>>> ---
>>>>>
>>>>> Changes from v1:
>>>>> - fix messed up commit message
>>>>> ---
>>>>> arch/arm/boot/dts/zynq-7000.dtsi | 1 +
>>>>> 1 file changed, 1 insertion(+)
>>>>>
>>>>> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
>>>>> index f3ac9bf..98233a8 100644
>>>>> --- a/arch/arm/boot/dts/zynq-7000.dtsi
>>>>> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
>>>>> @@ -72,6 +72,7 @@
>>>>> interrupts = <0 7 4>;
>>>>> interrupt-parent = <&intc>;
>>>>> clocks = <&clkc 12>;
>>>>> + #io-channel-cells = <1>;
>>>>> };
>>>>>
>>>>> can0: can@e0008000 {
>>>>>
>>>>
>>>> I think it will be good to the next step too.
>>>> It means also add iio-hwmon node too.
>>>>
>>>> What do you think?
>>>
>>> I hadn't put it in there since dts is supposed to describe hw,
>>> but obviously putting the actual hwmon in there makes it more useful.
>>
>> I had one discussion about this with Grant in past and it is common
>> mistake. It is simplification of purpose of dts.
>>
>
> If the iio-hwmon binding had gone through review it would have been rejected.
>
>>>
>>> I can resubmit with the hwmon node in there.
>>
>> If you grep kernel tree you will see that others are using it too.
>> Also there is accepted binding for that that's why I can't see big
>> problem with it.
>
> Since this is an application specific binding I wouldn't put it in the
> generic DT include file. It's a bit like adding a gpio-key binding for each
> of the GPIOs just in case somebody wants to use it.
So is your suggestion to move the whole thing from .dtsi -> .dts?
Thanks,
Moritz
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v2] ARM: zynq: Add #io-channel-cells to (x)adc node for iio-hwmon
@ 2017-03-16 17:54 ` Moritz Fischer
0 siblings, 0 replies; 36+ messages in thread
From: Moritz Fischer @ 2017-03-16 17:54 UTC (permalink / raw)
To: linux-arm-kernel
Hi Lars,
On Thu, Mar 16, 2017 at 9:51 AM, Lars-Peter Clausen <lars@metafoo.de> wrote:
> On 03/16/2017 05:45 PM, Michal Simek wrote:
>> On 16.3.2017 17:39, Moritz Fischer wrote:
>>> On Thu, Mar 16, 2017 at 9:16 AM, Michal Simek <michal.simek@xilinx.com> wrote:
>>>> Hi,
>>>>
>>>> On 8.3.2017 21:11, Moritz Fischer wrote:
>>>>> Fix
>>>>>
>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>> /amba/adc at f8007100
>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>> /amba/adc at f8007100
>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>> /amba/adc at f8007100
>>>>>
>>>>> by adding the #io-channel-cells property.
>>>>>
>>>>> Signed-off-by: Moritz Fischer <mdf@kernel.org>
>>>>> Cc: Michal Simek <michal.simek@xilinx.com>
>>>>> Cc: S?ren Brinkmann <soren.brinkmann@xilinx.com>
>>>>> Cc: Julia Cartwright <julia@ni.com>
>>>>> Cc: linux-kernel at vger.kernel.org
>>>>> Cc: devicetree at vger.kernel.org
>>>>> ---
>>>>>
>>>>> Changes from v1:
>>>>> - fix messed up commit message
>>>>> ---
>>>>> arch/arm/boot/dts/zynq-7000.dtsi | 1 +
>>>>> 1 file changed, 1 insertion(+)
>>>>>
>>>>> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
>>>>> index f3ac9bf..98233a8 100644
>>>>> --- a/arch/arm/boot/dts/zynq-7000.dtsi
>>>>> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
>>>>> @@ -72,6 +72,7 @@
>>>>> interrupts = <0 7 4>;
>>>>> interrupt-parent = <&intc>;
>>>>> clocks = <&clkc 12>;
>>>>> + #io-channel-cells = <1>;
>>>>> };
>>>>>
>>>>> can0: can at e0008000 {
>>>>>
>>>>
>>>> I think it will be good to the next step too.
>>>> It means also add iio-hwmon node too.
>>>>
>>>> What do you think?
>>>
>>> I hadn't put it in there since dts is supposed to describe hw,
>>> but obviously putting the actual hwmon in there makes it more useful.
>>
>> I had one discussion about this with Grant in past and it is common
>> mistake. It is simplification of purpose of dts.
>>
>
> If the iio-hwmon binding had gone through review it would have been rejected.
>
>>>
>>> I can resubmit with the hwmon node in there.
>>
>> If you grep kernel tree you will see that others are using it too.
>> Also there is accepted binding for that that's why I can't see big
>> problem with it.
>
> Since this is an application specific binding I wouldn't put it in the
> generic DT include file. It's a bit like adding a gpio-key binding for each
> of the GPIOs just in case somebody wants to use it.
So is your suggestion to move the whole thing from .dtsi -> .dts?
Thanks,
Moritz
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2] ARM: zynq: Add #io-channel-cells to (x)adc node for iio-hwmon
2017-03-16 16:51 ` Lars-Peter Clausen
(?)
@ 2017-03-16 18:06 ` Michal Simek
-1 siblings, 0 replies; 36+ messages in thread
From: Michal Simek @ 2017-03-16 18:06 UTC (permalink / raw)
To: Lars-Peter Clausen, Michal Simek, Moritz Fischer
Cc: Linux Kernel Mailing List, Devicetree List, Sören Brinkmann,
linux-arm-kernel, Julia Cartwright
On 16.3.2017 17:51, Lars-Peter Clausen wrote:
> On 03/16/2017 05:45 PM, Michal Simek wrote:
>> On 16.3.2017 17:39, Moritz Fischer wrote:
>>> On Thu, Mar 16, 2017 at 9:16 AM, Michal Simek <michal.simek@xilinx.com> wrote:
>>>> Hi,
>>>>
>>>> On 8.3.2017 21:11, Moritz Fischer wrote:
>>>>> Fix
>>>>>
>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>> /amba/adc@f8007100
>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>> /amba/adc@f8007100
>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>> /amba/adc@f8007100
>>>>>
>>>>> by adding the #io-channel-cells property.
>>>>>
>>>>> Signed-off-by: Moritz Fischer <mdf@kernel.org>
>>>>> Cc: Michal Simek <michal.simek@xilinx.com>
>>>>> Cc: Sören Brinkmann <soren.brinkmann@xilinx.com>
>>>>> Cc: Julia Cartwright <julia@ni.com>
>>>>> Cc: linux-kernel@vger.kernel.org
>>>>> Cc: devicetree@vger.kernel.org
>>>>> ---
>>>>>
>>>>> Changes from v1:
>>>>> - fix messed up commit message
>>>>> ---
>>>>> arch/arm/boot/dts/zynq-7000.dtsi | 1 +
>>>>> 1 file changed, 1 insertion(+)
>>>>>
>>>>> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
>>>>> index f3ac9bf..98233a8 100644
>>>>> --- a/arch/arm/boot/dts/zynq-7000.dtsi
>>>>> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
>>>>> @@ -72,6 +72,7 @@
>>>>> interrupts = <0 7 4>;
>>>>> interrupt-parent = <&intc>;
>>>>> clocks = <&clkc 12>;
>>>>> + #io-channel-cells = <1>;
>>>>> };
>>>>>
>>>>> can0: can@e0008000 {
>>>>>
>>>>
>>>> I think it will be good to the next step too.
>>>> It means also add iio-hwmon node too.
>>>>
>>>> What do you think?
>>>
>>> I hadn't put it in there since dts is supposed to describe hw,
>>> but obviously putting the actual hwmon in there makes it more useful.
>>
>> I had one discussion about this with Grant in past and it is common
>> mistake. It is simplification of purpose of dts.
>>
>
> If the iio-hwmon binding had gone through review it would have been rejected.
Isn't it a time to deprecate it?
>
>>>
>>> I can resubmit with the hwmon node in there.
>>
>> If you grep kernel tree you will see that others are using it too.
>> Also there is accepted binding for that that's why I can't see big
>> problem with it.
>
> Since this is an application specific binding I wouldn't put it in the
> generic DT include file. It's a bit like adding a gpio-key binding for each
> of the GPIOs just in case somebody wants to use it.
psci is system specific too.
IIRC this driver for zynq was written by ADI or with ADI help that's why
you know much better than I what's the correct configuration.
This targets PS IP which should be present in the hw all the time.
Not sure if for all configuration but I expect at least the part of it
is there all them time.
If binding is incorrect then please remove it with removing from all
dts/dtsi files which have this. The same is for of probe function in the
driver itself.
If this is not done then this is just +1 case.
If you still insist that we shouldn't do it then please at least extend
commit message and put there example how to wire it on zynq.
Thanks,
Michal
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2] ARM: zynq: Add #io-channel-cells to (x)adc node for iio-hwmon
@ 2017-03-16 18:06 ` Michal Simek
0 siblings, 0 replies; 36+ messages in thread
From: Michal Simek @ 2017-03-16 18:06 UTC (permalink / raw)
To: Lars-Peter Clausen, Michal Simek, Moritz Fischer
Cc: linux-arm-kernel, Devicetree List, Linux Kernel Mailing List,
Sören Brinkmann, Julia Cartwright
On 16.3.2017 17:51, Lars-Peter Clausen wrote:
> On 03/16/2017 05:45 PM, Michal Simek wrote:
>> On 16.3.2017 17:39, Moritz Fischer wrote:
>>> On Thu, Mar 16, 2017 at 9:16 AM, Michal Simek <michal.simek@xilinx.com> wrote:
>>>> Hi,
>>>>
>>>> On 8.3.2017 21:11, Moritz Fischer wrote:
>>>>> Fix
>>>>>
>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>> /amba/adc@f8007100
>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>> /amba/adc@f8007100
>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>> /amba/adc@f8007100
>>>>>
>>>>> by adding the #io-channel-cells property.
>>>>>
>>>>> Signed-off-by: Moritz Fischer <mdf@kernel.org>
>>>>> Cc: Michal Simek <michal.simek@xilinx.com>
>>>>> Cc: Sören Brinkmann <soren.brinkmann@xilinx.com>
>>>>> Cc: Julia Cartwright <julia@ni.com>
>>>>> Cc: linux-kernel@vger.kernel.org
>>>>> Cc: devicetree@vger.kernel.org
>>>>> ---
>>>>>
>>>>> Changes from v1:
>>>>> - fix messed up commit message
>>>>> ---
>>>>> arch/arm/boot/dts/zynq-7000.dtsi | 1 +
>>>>> 1 file changed, 1 insertion(+)
>>>>>
>>>>> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
>>>>> index f3ac9bf..98233a8 100644
>>>>> --- a/arch/arm/boot/dts/zynq-7000.dtsi
>>>>> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
>>>>> @@ -72,6 +72,7 @@
>>>>> interrupts = <0 7 4>;
>>>>> interrupt-parent = <&intc>;
>>>>> clocks = <&clkc 12>;
>>>>> + #io-channel-cells = <1>;
>>>>> };
>>>>>
>>>>> can0: can@e0008000 {
>>>>>
>>>>
>>>> I think it will be good to the next step too.
>>>> It means also add iio-hwmon node too.
>>>>
>>>> What do you think?
>>>
>>> I hadn't put it in there since dts is supposed to describe hw,
>>> but obviously putting the actual hwmon in there makes it more useful.
>>
>> I had one discussion about this with Grant in past and it is common
>> mistake. It is simplification of purpose of dts.
>>
>
> If the iio-hwmon binding had gone through review it would have been rejected.
Isn't it a time to deprecate it?
>
>>>
>>> I can resubmit with the hwmon node in there.
>>
>> If you grep kernel tree you will see that others are using it too.
>> Also there is accepted binding for that that's why I can't see big
>> problem with it.
>
> Since this is an application specific binding I wouldn't put it in the
> generic DT include file. It's a bit like adding a gpio-key binding for each
> of the GPIOs just in case somebody wants to use it.
psci is system specific too.
IIRC this driver for zynq was written by ADI or with ADI help that's why
you know much better than I what's the correct configuration.
This targets PS IP which should be present in the hw all the time.
Not sure if for all configuration but I expect at least the part of it
is there all them time.
If binding is incorrect then please remove it with removing from all
dts/dtsi files which have this. The same is for of probe function in the
driver itself.
If this is not done then this is just +1 case.
If you still insist that we shouldn't do it then please at least extend
commit message and put there example how to wire it on zynq.
Thanks,
Michal
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v2] ARM: zynq: Add #io-channel-cells to (x)adc node for iio-hwmon
@ 2017-03-16 18:06 ` Michal Simek
0 siblings, 0 replies; 36+ messages in thread
From: Michal Simek @ 2017-03-16 18:06 UTC (permalink / raw)
To: linux-arm-kernel
On 16.3.2017 17:51, Lars-Peter Clausen wrote:
> On 03/16/2017 05:45 PM, Michal Simek wrote:
>> On 16.3.2017 17:39, Moritz Fischer wrote:
>>> On Thu, Mar 16, 2017 at 9:16 AM, Michal Simek <michal.simek@xilinx.com> wrote:
>>>> Hi,
>>>>
>>>> On 8.3.2017 21:11, Moritz Fischer wrote:
>>>>> Fix
>>>>>
>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>> /amba/adc at f8007100
>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>> /amba/adc at f8007100
>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>> /amba/adc at f8007100
>>>>>
>>>>> by adding the #io-channel-cells property.
>>>>>
>>>>> Signed-off-by: Moritz Fischer <mdf@kernel.org>
>>>>> Cc: Michal Simek <michal.simek@xilinx.com>
>>>>> Cc: S?ren Brinkmann <soren.brinkmann@xilinx.com>
>>>>> Cc: Julia Cartwright <julia@ni.com>
>>>>> Cc: linux-kernel at vger.kernel.org
>>>>> Cc: devicetree at vger.kernel.org
>>>>> ---
>>>>>
>>>>> Changes from v1:
>>>>> - fix messed up commit message
>>>>> ---
>>>>> arch/arm/boot/dts/zynq-7000.dtsi | 1 +
>>>>> 1 file changed, 1 insertion(+)
>>>>>
>>>>> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
>>>>> index f3ac9bf..98233a8 100644
>>>>> --- a/arch/arm/boot/dts/zynq-7000.dtsi
>>>>> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
>>>>> @@ -72,6 +72,7 @@
>>>>> interrupts = <0 7 4>;
>>>>> interrupt-parent = <&intc>;
>>>>> clocks = <&clkc 12>;
>>>>> + #io-channel-cells = <1>;
>>>>> };
>>>>>
>>>>> can0: can at e0008000 {
>>>>>
>>>>
>>>> I think it will be good to the next step too.
>>>> It means also add iio-hwmon node too.
>>>>
>>>> What do you think?
>>>
>>> I hadn't put it in there since dts is supposed to describe hw,
>>> but obviously putting the actual hwmon in there makes it more useful.
>>
>> I had one discussion about this with Grant in past and it is common
>> mistake. It is simplification of purpose of dts.
>>
>
> If the iio-hwmon binding had gone through review it would have been rejected.
Isn't it a time to deprecate it?
>
>>>
>>> I can resubmit with the hwmon node in there.
>>
>> If you grep kernel tree you will see that others are using it too.
>> Also there is accepted binding for that that's why I can't see big
>> problem with it.
>
> Since this is an application specific binding I wouldn't put it in the
> generic DT include file. It's a bit like adding a gpio-key binding for each
> of the GPIOs just in case somebody wants to use it.
psci is system specific too.
IIRC this driver for zynq was written by ADI or with ADI help that's why
you know much better than I what's the correct configuration.
This targets PS IP which should be present in the hw all the time.
Not sure if for all configuration but I expect at least the part of it
is there all them time.
If binding is incorrect then please remove it with removing from all
dts/dtsi files which have this. The same is for of probe function in the
driver itself.
If this is not done then this is just +1 case.
If you still insist that we shouldn't do it then please at least extend
commit message and put there example how to wire it on zynq.
Thanks,
Michal
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2] ARM: zynq: Add #io-channel-cells to (x)adc node for iio-hwmon
2017-03-16 18:06 ` Michal Simek
(?)
@ 2017-03-16 21:20 ` Lars-Peter Clausen
-1 siblings, 0 replies; 36+ messages in thread
From: Lars-Peter Clausen @ 2017-03-16 21:20 UTC (permalink / raw)
To: Michal Simek, Moritz Fischer
Cc: Linux Kernel Mailing List, Devicetree List, Sören Brinkmann,
linux-arm-kernel, Julia Cartwright
On 03/16/2017 07:06 PM, Michal Simek wrote:
> On 16.3.2017 17:51, Lars-Peter Clausen wrote:
>> On 03/16/2017 05:45 PM, Michal Simek wrote:
>>> On 16.3.2017 17:39, Moritz Fischer wrote:
>>>> On Thu, Mar 16, 2017 at 9:16 AM, Michal Simek <michal.simek@xilinx.com> wrote:
>>>>> Hi,
>>>>>
>>>>> On 8.3.2017 21:11, Moritz Fischer wrote:
>>>>>> Fix
>>>>>>
>>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>>> /amba/adc@f8007100
>>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>>> /amba/adc@f8007100
>>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>>> /amba/adc@f8007100
>>>>>>
>>>>>> by adding the #io-channel-cells property.
>>>>>>
>>>>>> Signed-off-by: Moritz Fischer <mdf@kernel.org>
>>>>>> Cc: Michal Simek <michal.simek@xilinx.com>
>>>>>> Cc: Sören Brinkmann <soren.brinkmann@xilinx.com>
>>>>>> Cc: Julia Cartwright <julia@ni.com>
>>>>>> Cc: linux-kernel@vger.kernel.org
>>>>>> Cc: devicetree@vger.kernel.org
>>>>>> ---
>>>>>>
>>>>>> Changes from v1:
>>>>>> - fix messed up commit message
>>>>>> ---
>>>>>> arch/arm/boot/dts/zynq-7000.dtsi | 1 +
>>>>>> 1 file changed, 1 insertion(+)
>>>>>>
>>>>>> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>> index f3ac9bf..98233a8 100644
>>>>>> --- a/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>> @@ -72,6 +72,7 @@
>>>>>> interrupts = <0 7 4>;
>>>>>> interrupt-parent = <&intc>;
>>>>>> clocks = <&clkc 12>;
>>>>>> + #io-channel-cells = <1>;
>>>>>> };
>>>>>>
>>>>>> can0: can@e0008000 {
>>>>>>
>>>>>
>>>>> I think it will be good to the next step too.
>>>>> It means also add iio-hwmon node too.
>>>>>
>>>>> What do you think?
>>>>
>>>> I hadn't put it in there since dts is supposed to describe hw,
>>>> but obviously putting the actual hwmon in there makes it more useful.
>>>
>>> I had one discussion about this with Grant in past and it is common
>>> mistake. It is simplification of purpose of dts.
>>>
>>
>> If the iio-hwmon binding had gone through review it would have been rejected.
>
> Isn't it a time to deprecate it?
Well, it's ABI now and has to stay forever. Deprecating it makes only sense
if there is a replacement, which there is not. The iio-hwmon bridge has its
usecases it's just instantiating it via devicetree which is not so nice.
>
>>
>>>>
>>>> I can resubmit with the hwmon node in there.
>>>
>>> If you grep kernel tree you will see that others are using it too.
>>> Also there is accepted binding for that that's why I can't see big
>>> problem with it.
>>
>> Since this is an application specific binding I wouldn't put it in the
>> generic DT include file. It's a bit like adding a gpio-key binding for each
>> of the GPIOs just in case somebody wants to use it.
>
> psci is system specific too.
>
> IIRC this driver for zynq was written by ADI or with ADI help that's why
> you know much better than I what's the correct configuration.
>
> This targets PS IP which should be present in the hw all the time.
> Not sure if for all configuration but I expect at least the part of it
> is there all them time.
>
> If binding is incorrect then please remove it with removing from all
> dts/dtsi files which have this. The same is for of probe function in the
> driver itself.
> If this is not done then this is just +1 case.
>
> If you still insist that we shouldn't do it then please at least extend
> commit message and put there example how to wire it on zynq.
There is a IIO driver for the XADC, this driver has a userspace interface
that exposes the measurements provided by the hardware. Using the hwmon
bridge will expose the same information just through a hwmon interface.
One reason for using the iio-hwmon bridge is because you have a legacy
application that expects the a hwmon interface. But new applications that
want to access the XADC should really use the IIO interface if possible.
In my opinion instantiating the hwmon bridge by default will only cause
confusion. There are now redundant interfaces and users will wonder what is
the difference between the two. Is it the same data, is it different data?
Which is the preferred interface? Which one is 'better'?
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2] ARM: zynq: Add #io-channel-cells to (x)adc node for iio-hwmon
@ 2017-03-16 21:20 ` Lars-Peter Clausen
0 siblings, 0 replies; 36+ messages in thread
From: Lars-Peter Clausen @ 2017-03-16 21:20 UTC (permalink / raw)
To: Michal Simek, Moritz Fischer
Cc: linux-arm-kernel, Devicetree List, Linux Kernel Mailing List,
Sören Brinkmann, Julia Cartwright
On 03/16/2017 07:06 PM, Michal Simek wrote:
> On 16.3.2017 17:51, Lars-Peter Clausen wrote:
>> On 03/16/2017 05:45 PM, Michal Simek wrote:
>>> On 16.3.2017 17:39, Moritz Fischer wrote:
>>>> On Thu, Mar 16, 2017 at 9:16 AM, Michal Simek <michal.simek@xilinx.com> wrote:
>>>>> Hi,
>>>>>
>>>>> On 8.3.2017 21:11, Moritz Fischer wrote:
>>>>>> Fix
>>>>>>
>>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>>> /amba/adc@f8007100
>>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>>> /amba/adc@f8007100
>>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>>> /amba/adc@f8007100
>>>>>>
>>>>>> by adding the #io-channel-cells property.
>>>>>>
>>>>>> Signed-off-by: Moritz Fischer <mdf@kernel.org>
>>>>>> Cc: Michal Simek <michal.simek@xilinx.com>
>>>>>> Cc: Sören Brinkmann <soren.brinkmann@xilinx.com>
>>>>>> Cc: Julia Cartwright <julia@ni.com>
>>>>>> Cc: linux-kernel@vger.kernel.org
>>>>>> Cc: devicetree@vger.kernel.org
>>>>>> ---
>>>>>>
>>>>>> Changes from v1:
>>>>>> - fix messed up commit message
>>>>>> ---
>>>>>> arch/arm/boot/dts/zynq-7000.dtsi | 1 +
>>>>>> 1 file changed, 1 insertion(+)
>>>>>>
>>>>>> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>> index f3ac9bf..98233a8 100644
>>>>>> --- a/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>> @@ -72,6 +72,7 @@
>>>>>> interrupts = <0 7 4>;
>>>>>> interrupt-parent = <&intc>;
>>>>>> clocks = <&clkc 12>;
>>>>>> + #io-channel-cells = <1>;
>>>>>> };
>>>>>>
>>>>>> can0: can@e0008000 {
>>>>>>
>>>>>
>>>>> I think it will be good to the next step too.
>>>>> It means also add iio-hwmon node too.
>>>>>
>>>>> What do you think?
>>>>
>>>> I hadn't put it in there since dts is supposed to describe hw,
>>>> but obviously putting the actual hwmon in there makes it more useful.
>>>
>>> I had one discussion about this with Grant in past and it is common
>>> mistake. It is simplification of purpose of dts.
>>>
>>
>> If the iio-hwmon binding had gone through review it would have been rejected.
>
> Isn't it a time to deprecate it?
Well, it's ABI now and has to stay forever. Deprecating it makes only sense
if there is a replacement, which there is not. The iio-hwmon bridge has its
usecases it's just instantiating it via devicetree which is not so nice.
>
>>
>>>>
>>>> I can resubmit with the hwmon node in there.
>>>
>>> If you grep kernel tree you will see that others are using it too.
>>> Also there is accepted binding for that that's why I can't see big
>>> problem with it.
>>
>> Since this is an application specific binding I wouldn't put it in the
>> generic DT include file. It's a bit like adding a gpio-key binding for each
>> of the GPIOs just in case somebody wants to use it.
>
> psci is system specific too.
>
> IIRC this driver for zynq was written by ADI or with ADI help that's why
> you know much better than I what's the correct configuration.
>
> This targets PS IP which should be present in the hw all the time.
> Not sure if for all configuration but I expect at least the part of it
> is there all them time.
>
> If binding is incorrect then please remove it with removing from all
> dts/dtsi files which have this. The same is for of probe function in the
> driver itself.
> If this is not done then this is just +1 case.
>
> If you still insist that we shouldn't do it then please at least extend
> commit message and put there example how to wire it on zynq.
There is a IIO driver for the XADC, this driver has a userspace interface
that exposes the measurements provided by the hardware. Using the hwmon
bridge will expose the same information just through a hwmon interface.
One reason for using the iio-hwmon bridge is because you have a legacy
application that expects the a hwmon interface. But new applications that
want to access the XADC should really use the IIO interface if possible.
In my opinion instantiating the hwmon bridge by default will only cause
confusion. There are now redundant interfaces and users will wonder what is
the difference between the two. Is it the same data, is it different data?
Which is the preferred interface? Which one is 'better'?
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v2] ARM: zynq: Add #io-channel-cells to (x)adc node for iio-hwmon
@ 2017-03-16 21:20 ` Lars-Peter Clausen
0 siblings, 0 replies; 36+ messages in thread
From: Lars-Peter Clausen @ 2017-03-16 21:20 UTC (permalink / raw)
To: linux-arm-kernel
On 03/16/2017 07:06 PM, Michal Simek wrote:
> On 16.3.2017 17:51, Lars-Peter Clausen wrote:
>> On 03/16/2017 05:45 PM, Michal Simek wrote:
>>> On 16.3.2017 17:39, Moritz Fischer wrote:
>>>> On Thu, Mar 16, 2017 at 9:16 AM, Michal Simek <michal.simek@xilinx.com> wrote:
>>>>> Hi,
>>>>>
>>>>> On 8.3.2017 21:11, Moritz Fischer wrote:
>>>>>> Fix
>>>>>>
>>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>>> /amba/adc at f8007100
>>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>>> /amba/adc at f8007100
>>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>>> /amba/adc at f8007100
>>>>>>
>>>>>> by adding the #io-channel-cells property.
>>>>>>
>>>>>> Signed-off-by: Moritz Fischer <mdf@kernel.org>
>>>>>> Cc: Michal Simek <michal.simek@xilinx.com>
>>>>>> Cc: S?ren Brinkmann <soren.brinkmann@xilinx.com>
>>>>>> Cc: Julia Cartwright <julia@ni.com>
>>>>>> Cc: linux-kernel at vger.kernel.org
>>>>>> Cc: devicetree at vger.kernel.org
>>>>>> ---
>>>>>>
>>>>>> Changes from v1:
>>>>>> - fix messed up commit message
>>>>>> ---
>>>>>> arch/arm/boot/dts/zynq-7000.dtsi | 1 +
>>>>>> 1 file changed, 1 insertion(+)
>>>>>>
>>>>>> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>> index f3ac9bf..98233a8 100644
>>>>>> --- a/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>> @@ -72,6 +72,7 @@
>>>>>> interrupts = <0 7 4>;
>>>>>> interrupt-parent = <&intc>;
>>>>>> clocks = <&clkc 12>;
>>>>>> + #io-channel-cells = <1>;
>>>>>> };
>>>>>>
>>>>>> can0: can at e0008000 {
>>>>>>
>>>>>
>>>>> I think it will be good to the next step too.
>>>>> It means also add iio-hwmon node too.
>>>>>
>>>>> What do you think?
>>>>
>>>> I hadn't put it in there since dts is supposed to describe hw,
>>>> but obviously putting the actual hwmon in there makes it more useful.
>>>
>>> I had one discussion about this with Grant in past and it is common
>>> mistake. It is simplification of purpose of dts.
>>>
>>
>> If the iio-hwmon binding had gone through review it would have been rejected.
>
> Isn't it a time to deprecate it?
Well, it's ABI now and has to stay forever. Deprecating it makes only sense
if there is a replacement, which there is not. The iio-hwmon bridge has its
usecases it's just instantiating it via devicetree which is not so nice.
>
>>
>>>>
>>>> I can resubmit with the hwmon node in there.
>>>
>>> If you grep kernel tree you will see that others are using it too.
>>> Also there is accepted binding for that that's why I can't see big
>>> problem with it.
>>
>> Since this is an application specific binding I wouldn't put it in the
>> generic DT include file. It's a bit like adding a gpio-key binding for each
>> of the GPIOs just in case somebody wants to use it.
>
> psci is system specific too.
>
> IIRC this driver for zynq was written by ADI or with ADI help that's why
> you know much better than I what's the correct configuration.
>
> This targets PS IP which should be present in the hw all the time.
> Not sure if for all configuration but I expect at least the part of it
> is there all them time.
>
> If binding is incorrect then please remove it with removing from all
> dts/dtsi files which have this. The same is for of probe function in the
> driver itself.
> If this is not done then this is just +1 case.
>
> If you still insist that we shouldn't do it then please at least extend
> commit message and put there example how to wire it on zynq.
There is a IIO driver for the XADC, this driver has a userspace interface
that exposes the measurements provided by the hardware. Using the hwmon
bridge will expose the same information just through a hwmon interface.
One reason for using the iio-hwmon bridge is because you have a legacy
application that expects the a hwmon interface. But new applications that
want to access the XADC should really use the IIO interface if possible.
In my opinion instantiating the hwmon bridge by default will only cause
confusion. There are now redundant interfaces and users will wonder what is
the difference between the two. Is it the same data, is it different data?
Which is the preferred interface? Which one is 'better'?
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2] ARM: zynq: Add #io-channel-cells to (x)adc node for iio-hwmon
2017-03-16 17:54 ` Moritz Fischer
(?)
@ 2017-03-16 21:23 ` Lars-Peter Clausen
-1 siblings, 0 replies; 36+ messages in thread
From: Lars-Peter Clausen @ 2017-03-16 21:23 UTC (permalink / raw)
To: Moritz Fischer
Cc: Michal Simek, Linux Kernel Mailing List, Devicetree List,
Sören Brinkmann, linux-arm-kernel, Julia Cartwright
On 03/16/2017 06:54 PM, Moritz Fischer wrote:
> Hi Lars,
>
> On Thu, Mar 16, 2017 at 9:51 AM, Lars-Peter Clausen <lars@metafoo.de> wrote:
>> On 03/16/2017 05:45 PM, Michal Simek wrote:
>>> On 16.3.2017 17:39, Moritz Fischer wrote:
>>>> On Thu, Mar 16, 2017 at 9:16 AM, Michal Simek <michal.simek@xilinx.com> wrote:
>>>>> Hi,
>>>>>
>>>>> On 8.3.2017 21:11, Moritz Fischer wrote:
>>>>>> Fix
>>>>>>
>>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>>> /amba/adc@f8007100
>>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>>> /amba/adc@f8007100
>>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>>> /amba/adc@f8007100
>>>>>>
>>>>>> by adding the #io-channel-cells property.
>>>>>>
>>>>>> Signed-off-by: Moritz Fischer <mdf@kernel.org>
>>>>>> Cc: Michal Simek <michal.simek@xilinx.com>
>>>>>> Cc: Sören Brinkmann <soren.brinkmann@xilinx.com>
>>>>>> Cc: Julia Cartwright <julia@ni.com>
>>>>>> Cc: linux-kernel@vger.kernel.org
>>>>>> Cc: devicetree@vger.kernel.org
>>>>>> ---
>>>>>>
>>>>>> Changes from v1:
>>>>>> - fix messed up commit message
>>>>>> ---
>>>>>> arch/arm/boot/dts/zynq-7000.dtsi | 1 +
>>>>>> 1 file changed, 1 insertion(+)
>>>>>>
>>>>>> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>> index f3ac9bf..98233a8 100644
>>>>>> --- a/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>> @@ -72,6 +72,7 @@
>>>>>> interrupts = <0 7 4>;
>>>>>> interrupt-parent = <&intc>;
>>>>>> clocks = <&clkc 12>;
>>>>>> + #io-channel-cells = <1>;
>>>>>> };
>>>>>>
>>>>>> can0: can@e0008000 {
>>>>>>
>>>>>
>>>>> I think it will be good to the next step too.
>>>>> It means also add iio-hwmon node too.
>>>>>
>>>>> What do you think?
>>>>
>>>> I hadn't put it in there since dts is supposed to describe hw,
>>>> but obviously putting the actual hwmon in there makes it more useful.
>>>
>>> I had one discussion about this with Grant in past and it is common
>>> mistake. It is simplification of purpose of dts.
>>>
>>
>> If the iio-hwmon binding had gone through review it would have been rejected.
>>
>>>>
>>>> I can resubmit with the hwmon node in there.
>>>
>>> If you grep kernel tree you will see that others are using it too.
>>> Also there is accepted binding for that that's why I can't see big
>>> problem with it.
>>
>> Since this is an application specific binding I wouldn't put it in the
>> generic DT include file. It's a bit like adding a gpio-key binding for each
>> of the GPIOs just in case somebody wants to use it.
>
> So is your suggestion to move the whole thing from .dtsi -> .dts?
The #io-channels-cells can go into the dtsi, that's a sensible thing to do.
The number of cells is part of the binding specification for the XADC and
wont change.
The hwmon bridge on the other hand I wouldn't put into any default
devicetree. In my opinion is should only used if and only if there are
legacy applications on a specific system that need to use the hwmon interface.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2] ARM: zynq: Add #io-channel-cells to (x)adc node for iio-hwmon
@ 2017-03-16 21:23 ` Lars-Peter Clausen
0 siblings, 0 replies; 36+ messages in thread
From: Lars-Peter Clausen @ 2017-03-16 21:23 UTC (permalink / raw)
To: Moritz Fischer
Cc: Devicetree List, Julia Cartwright, Michal Simek,
Linux Kernel Mailing List, Sören Brinkmann,
linux-arm-kernel
On 03/16/2017 06:54 PM, Moritz Fischer wrote:
> Hi Lars,
>
> On Thu, Mar 16, 2017 at 9:51 AM, Lars-Peter Clausen <lars@metafoo.de> wrote:
>> On 03/16/2017 05:45 PM, Michal Simek wrote:
>>> On 16.3.2017 17:39, Moritz Fischer wrote:
>>>> On Thu, Mar 16, 2017 at 9:16 AM, Michal Simek <michal.simek@xilinx.com> wrote:
>>>>> Hi,
>>>>>
>>>>> On 8.3.2017 21:11, Moritz Fischer wrote:
>>>>>> Fix
>>>>>>
>>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>>> /amba/adc@f8007100
>>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>>> /amba/adc@f8007100
>>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>>> /amba/adc@f8007100
>>>>>>
>>>>>> by adding the #io-channel-cells property.
>>>>>>
>>>>>> Signed-off-by: Moritz Fischer <mdf@kernel.org>
>>>>>> Cc: Michal Simek <michal.simek@xilinx.com>
>>>>>> Cc: Sören Brinkmann <soren.brinkmann@xilinx.com>
>>>>>> Cc: Julia Cartwright <julia@ni.com>
>>>>>> Cc: linux-kernel@vger.kernel.org
>>>>>> Cc: devicetree@vger.kernel.org
>>>>>> ---
>>>>>>
>>>>>> Changes from v1:
>>>>>> - fix messed up commit message
>>>>>> ---
>>>>>> arch/arm/boot/dts/zynq-7000.dtsi | 1 +
>>>>>> 1 file changed, 1 insertion(+)
>>>>>>
>>>>>> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>> index f3ac9bf..98233a8 100644
>>>>>> --- a/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>> @@ -72,6 +72,7 @@
>>>>>> interrupts = <0 7 4>;
>>>>>> interrupt-parent = <&intc>;
>>>>>> clocks = <&clkc 12>;
>>>>>> + #io-channel-cells = <1>;
>>>>>> };
>>>>>>
>>>>>> can0: can@e0008000 {
>>>>>>
>>>>>
>>>>> I think it will be good to the next step too.
>>>>> It means also add iio-hwmon node too.
>>>>>
>>>>> What do you think?
>>>>
>>>> I hadn't put it in there since dts is supposed to describe hw,
>>>> but obviously putting the actual hwmon in there makes it more useful.
>>>
>>> I had one discussion about this with Grant in past and it is common
>>> mistake. It is simplification of purpose of dts.
>>>
>>
>> If the iio-hwmon binding had gone through review it would have been rejected.
>>
>>>>
>>>> I can resubmit with the hwmon node in there.
>>>
>>> If you grep kernel tree you will see that others are using it too.
>>> Also there is accepted binding for that that's why I can't see big
>>> problem with it.
>>
>> Since this is an application specific binding I wouldn't put it in the
>> generic DT include file. It's a bit like adding a gpio-key binding for each
>> of the GPIOs just in case somebody wants to use it.
>
> So is your suggestion to move the whole thing from .dtsi -> .dts?
The #io-channels-cells can go into the dtsi, that's a sensible thing to do.
The number of cells is part of the binding specification for the XADC and
wont change.
The hwmon bridge on the other hand I wouldn't put into any default
devicetree. In my opinion is should only used if and only if there are
legacy applications on a specific system that need to use the hwmon interface.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v2] ARM: zynq: Add #io-channel-cells to (x)adc node for iio-hwmon
@ 2017-03-16 21:23 ` Lars-Peter Clausen
0 siblings, 0 replies; 36+ messages in thread
From: Lars-Peter Clausen @ 2017-03-16 21:23 UTC (permalink / raw)
To: linux-arm-kernel
On 03/16/2017 06:54 PM, Moritz Fischer wrote:
> Hi Lars,
>
> On Thu, Mar 16, 2017 at 9:51 AM, Lars-Peter Clausen <lars@metafoo.de> wrote:
>> On 03/16/2017 05:45 PM, Michal Simek wrote:
>>> On 16.3.2017 17:39, Moritz Fischer wrote:
>>>> On Thu, Mar 16, 2017 at 9:16 AM, Michal Simek <michal.simek@xilinx.com> wrote:
>>>>> Hi,
>>>>>
>>>>> On 8.3.2017 21:11, Moritz Fischer wrote:
>>>>>> Fix
>>>>>>
>>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>>> /amba/adc at f8007100
>>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>>> /amba/adc at f8007100
>>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>>> /amba/adc at f8007100
>>>>>>
>>>>>> by adding the #io-channel-cells property.
>>>>>>
>>>>>> Signed-off-by: Moritz Fischer <mdf@kernel.org>
>>>>>> Cc: Michal Simek <michal.simek@xilinx.com>
>>>>>> Cc: S?ren Brinkmann <soren.brinkmann@xilinx.com>
>>>>>> Cc: Julia Cartwright <julia@ni.com>
>>>>>> Cc: linux-kernel at vger.kernel.org
>>>>>> Cc: devicetree at vger.kernel.org
>>>>>> ---
>>>>>>
>>>>>> Changes from v1:
>>>>>> - fix messed up commit message
>>>>>> ---
>>>>>> arch/arm/boot/dts/zynq-7000.dtsi | 1 +
>>>>>> 1 file changed, 1 insertion(+)
>>>>>>
>>>>>> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>> index f3ac9bf..98233a8 100644
>>>>>> --- a/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>> @@ -72,6 +72,7 @@
>>>>>> interrupts = <0 7 4>;
>>>>>> interrupt-parent = <&intc>;
>>>>>> clocks = <&clkc 12>;
>>>>>> + #io-channel-cells = <1>;
>>>>>> };
>>>>>>
>>>>>> can0: can at e0008000 {
>>>>>>
>>>>>
>>>>> I think it will be good to the next step too.
>>>>> It means also add iio-hwmon node too.
>>>>>
>>>>> What do you think?
>>>>
>>>> I hadn't put it in there since dts is supposed to describe hw,
>>>> but obviously putting the actual hwmon in there makes it more useful.
>>>
>>> I had one discussion about this with Grant in past and it is common
>>> mistake. It is simplification of purpose of dts.
>>>
>>
>> If the iio-hwmon binding had gone through review it would have been rejected.
>>
>>>>
>>>> I can resubmit with the hwmon node in there.
>>>
>>> If you grep kernel tree you will see that others are using it too.
>>> Also there is accepted binding for that that's why I can't see big
>>> problem with it.
>>
>> Since this is an application specific binding I wouldn't put it in the
>> generic DT include file. It's a bit like adding a gpio-key binding for each
>> of the GPIOs just in case somebody wants to use it.
>
> So is your suggestion to move the whole thing from .dtsi -> .dts?
The #io-channels-cells can go into the dtsi, that's a sensible thing to do.
The number of cells is part of the binding specification for the XADC and
wont change.
The hwmon bridge on the other hand I wouldn't put into any default
devicetree. In my opinion is should only used if and only if there are
legacy applications on a specific system that need to use the hwmon interface.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2] ARM: zynq: Add #io-channel-cells to (x)adc node for iio-hwmon
2017-03-16 21:20 ` Lars-Peter Clausen
(?)
@ 2017-03-17 6:46 ` Michal Simek
-1 siblings, 0 replies; 36+ messages in thread
From: Michal Simek @ 2017-03-17 6:46 UTC (permalink / raw)
To: Lars-Peter Clausen, Michal Simek, Moritz Fischer
Cc: Linux Kernel Mailing List, Devicetree List, Sören Brinkmann,
linux-arm-kernel, Julia Cartwright
On 16.3.2017 22:20, Lars-Peter Clausen wrote:
> On 03/16/2017 07:06 PM, Michal Simek wrote:
>> On 16.3.2017 17:51, Lars-Peter Clausen wrote:
>>> On 03/16/2017 05:45 PM, Michal Simek wrote:
>>>> On 16.3.2017 17:39, Moritz Fischer wrote:
>>>>> On Thu, Mar 16, 2017 at 9:16 AM, Michal Simek <michal.simek@xilinx.com> wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On 8.3.2017 21:11, Moritz Fischer wrote:
>>>>>>> Fix
>>>>>>>
>>>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>>>> /amba/adc@f8007100
>>>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>>>> /amba/adc@f8007100
>>>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>>>> /amba/adc@f8007100
>>>>>>>
>>>>>>> by adding the #io-channel-cells property.
>>>>>>>
>>>>>>> Signed-off-by: Moritz Fischer <mdf@kernel.org>
>>>>>>> Cc: Michal Simek <michal.simek@xilinx.com>
>>>>>>> Cc: Sören Brinkmann <soren.brinkmann@xilinx.com>
>>>>>>> Cc: Julia Cartwright <julia@ni.com>
>>>>>>> Cc: linux-kernel@vger.kernel.org
>>>>>>> Cc: devicetree@vger.kernel.org
>>>>>>> ---
>>>>>>>
>>>>>>> Changes from v1:
>>>>>>> - fix messed up commit message
>>>>>>> ---
>>>>>>> arch/arm/boot/dts/zynq-7000.dtsi | 1 +
>>>>>>> 1 file changed, 1 insertion(+)
>>>>>>>
>>>>>>> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>>> index f3ac9bf..98233a8 100644
>>>>>>> --- a/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>>> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>>> @@ -72,6 +72,7 @@
>>>>>>> interrupts = <0 7 4>;
>>>>>>> interrupt-parent = <&intc>;
>>>>>>> clocks = <&clkc 12>;
>>>>>>> + #io-channel-cells = <1>;
>>>>>>> };
>>>>>>>
>>>>>>> can0: can@e0008000 {
>>>>>>>
>>>>>>
>>>>>> I think it will be good to the next step too.
>>>>>> It means also add iio-hwmon node too.
>>>>>>
>>>>>> What do you think?
>>>>>
>>>>> I hadn't put it in there since dts is supposed to describe hw,
>>>>> but obviously putting the actual hwmon in there makes it more useful.
>>>>
>>>> I had one discussion about this with Grant in past and it is common
>>>> mistake. It is simplification of purpose of dts.
>>>>
>>>
>>> If the iio-hwmon binding had gone through review it would have been rejected.
>>
>> Isn't it a time to deprecate it?
>
> Well, it's ABI now and has to stay forever. Deprecating it makes only sense
> if there is a replacement, which there is not. The iio-hwmon bridge has its
> usecases it's just instantiating it via devicetree which is not so nice.
>
>>
>>>
>>>>>
>>>>> I can resubmit with the hwmon node in there.
>>>>
>>>> If you grep kernel tree you will see that others are using it too.
>>>> Also there is accepted binding for that that's why I can't see big
>>>> problem with it.
>>>
>>> Since this is an application specific binding I wouldn't put it in the
>>> generic DT include file. It's a bit like adding a gpio-key binding for each
>>> of the GPIOs just in case somebody wants to use it.
>>
>> psci is system specific too.
>>
>> IIRC this driver for zynq was written by ADI or with ADI help that's why
>> you know much better than I what's the correct configuration.
>>
>> This targets PS IP which should be present in the hw all the time.
>> Not sure if for all configuration but I expect at least the part of it
>> is there all them time.
>>
>> If binding is incorrect then please remove it with removing from all
>> dts/dtsi files which have this. The same is for of probe function in the
>> driver itself.
>> If this is not done then this is just +1 case.
>>
>> If you still insist that we shouldn't do it then please at least extend
>> commit message and put there example how to wire it on zynq.
>
> There is a IIO driver for the XADC, this driver has a userspace interface
> that exposes the measurements provided by the hardware. Using the hwmon
> bridge will expose the same information just through a hwmon interface.
>
> One reason for using the iio-hwmon bridge is because you have a legacy
> application that expects the a hwmon interface. But new applications that
> want to access the XADC should really use the IIO interface if possible.
>
> In my opinion instantiating the hwmon bridge by default will only cause
> confusion. There are now redundant interfaces and users will wonder what is
> the difference between the two. Is it the same data, is it different data?
> Which is the preferred interface? Which one is 'better'?
IMHO this should be covered by documentation. One paragraph in iio-hwmon
binding can have answers for this and it will be very clear what people
should use.
Thanks,
Michal
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2] ARM: zynq: Add #io-channel-cells to (x)adc node for iio-hwmon
@ 2017-03-17 6:46 ` Michal Simek
0 siblings, 0 replies; 36+ messages in thread
From: Michal Simek @ 2017-03-17 6:46 UTC (permalink / raw)
To: Lars-Peter Clausen, Michal Simek, Moritz Fischer
Cc: linux-arm-kernel, Devicetree List, Linux Kernel Mailing List,
Sören Brinkmann, Julia Cartwright
On 16.3.2017 22:20, Lars-Peter Clausen wrote:
> On 03/16/2017 07:06 PM, Michal Simek wrote:
>> On 16.3.2017 17:51, Lars-Peter Clausen wrote:
>>> On 03/16/2017 05:45 PM, Michal Simek wrote:
>>>> On 16.3.2017 17:39, Moritz Fischer wrote:
>>>>> On Thu, Mar 16, 2017 at 9:16 AM, Michal Simek <michal.simek@xilinx.com> wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On 8.3.2017 21:11, Moritz Fischer wrote:
>>>>>>> Fix
>>>>>>>
>>>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>>>> /amba/adc@f8007100
>>>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>>>> /amba/adc@f8007100
>>>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>>>> /amba/adc@f8007100
>>>>>>>
>>>>>>> by adding the #io-channel-cells property.
>>>>>>>
>>>>>>> Signed-off-by: Moritz Fischer <mdf@kernel.org>
>>>>>>> Cc: Michal Simek <michal.simek@xilinx.com>
>>>>>>> Cc: Sören Brinkmann <soren.brinkmann@xilinx.com>
>>>>>>> Cc: Julia Cartwright <julia@ni.com>
>>>>>>> Cc: linux-kernel@vger.kernel.org
>>>>>>> Cc: devicetree@vger.kernel.org
>>>>>>> ---
>>>>>>>
>>>>>>> Changes from v1:
>>>>>>> - fix messed up commit message
>>>>>>> ---
>>>>>>> arch/arm/boot/dts/zynq-7000.dtsi | 1 +
>>>>>>> 1 file changed, 1 insertion(+)
>>>>>>>
>>>>>>> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>>> index f3ac9bf..98233a8 100644
>>>>>>> --- a/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>>> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>>> @@ -72,6 +72,7 @@
>>>>>>> interrupts = <0 7 4>;
>>>>>>> interrupt-parent = <&intc>;
>>>>>>> clocks = <&clkc 12>;
>>>>>>> + #io-channel-cells = <1>;
>>>>>>> };
>>>>>>>
>>>>>>> can0: can@e0008000 {
>>>>>>>
>>>>>>
>>>>>> I think it will be good to the next step too.
>>>>>> It means also add iio-hwmon node too.
>>>>>>
>>>>>> What do you think?
>>>>>
>>>>> I hadn't put it in there since dts is supposed to describe hw,
>>>>> but obviously putting the actual hwmon in there makes it more useful.
>>>>
>>>> I had one discussion about this with Grant in past and it is common
>>>> mistake. It is simplification of purpose of dts.
>>>>
>>>
>>> If the iio-hwmon binding had gone through review it would have been rejected.
>>
>> Isn't it a time to deprecate it?
>
> Well, it's ABI now and has to stay forever. Deprecating it makes only sense
> if there is a replacement, which there is not. The iio-hwmon bridge has its
> usecases it's just instantiating it via devicetree which is not so nice.
>
>>
>>>
>>>>>
>>>>> I can resubmit with the hwmon node in there.
>>>>
>>>> If you grep kernel tree you will see that others are using it too.
>>>> Also there is accepted binding for that that's why I can't see big
>>>> problem with it.
>>>
>>> Since this is an application specific binding I wouldn't put it in the
>>> generic DT include file. It's a bit like adding a gpio-key binding for each
>>> of the GPIOs just in case somebody wants to use it.
>>
>> psci is system specific too.
>>
>> IIRC this driver for zynq was written by ADI or with ADI help that's why
>> you know much better than I what's the correct configuration.
>>
>> This targets PS IP which should be present in the hw all the time.
>> Not sure if for all configuration but I expect at least the part of it
>> is there all them time.
>>
>> If binding is incorrect then please remove it with removing from all
>> dts/dtsi files which have this. The same is for of probe function in the
>> driver itself.
>> If this is not done then this is just +1 case.
>>
>> If you still insist that we shouldn't do it then please at least extend
>> commit message and put there example how to wire it on zynq.
>
> There is a IIO driver for the XADC, this driver has a userspace interface
> that exposes the measurements provided by the hardware. Using the hwmon
> bridge will expose the same information just through a hwmon interface.
>
> One reason for using the iio-hwmon bridge is because you have a legacy
> application that expects the a hwmon interface. But new applications that
> want to access the XADC should really use the IIO interface if possible.
>
> In my opinion instantiating the hwmon bridge by default will only cause
> confusion. There are now redundant interfaces and users will wonder what is
> the difference between the two. Is it the same data, is it different data?
> Which is the preferred interface? Which one is 'better'?
IMHO this should be covered by documentation. One paragraph in iio-hwmon
binding can have answers for this and it will be very clear what people
should use.
Thanks,
Michal
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v2] ARM: zynq: Add #io-channel-cells to (x)adc node for iio-hwmon
@ 2017-03-17 6:46 ` Michal Simek
0 siblings, 0 replies; 36+ messages in thread
From: Michal Simek @ 2017-03-17 6:46 UTC (permalink / raw)
To: linux-arm-kernel
On 16.3.2017 22:20, Lars-Peter Clausen wrote:
> On 03/16/2017 07:06 PM, Michal Simek wrote:
>> On 16.3.2017 17:51, Lars-Peter Clausen wrote:
>>> On 03/16/2017 05:45 PM, Michal Simek wrote:
>>>> On 16.3.2017 17:39, Moritz Fischer wrote:
>>>>> On Thu, Mar 16, 2017 at 9:16 AM, Michal Simek <michal.simek@xilinx.com> wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On 8.3.2017 21:11, Moritz Fischer wrote:
>>>>>>> Fix
>>>>>>>
>>>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>>>> /amba/adc at f8007100
>>>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>>>> /amba/adc at f8007100
>>>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>>>> /amba/adc at f8007100
>>>>>>>
>>>>>>> by adding the #io-channel-cells property.
>>>>>>>
>>>>>>> Signed-off-by: Moritz Fischer <mdf@kernel.org>
>>>>>>> Cc: Michal Simek <michal.simek@xilinx.com>
>>>>>>> Cc: S?ren Brinkmann <soren.brinkmann@xilinx.com>
>>>>>>> Cc: Julia Cartwright <julia@ni.com>
>>>>>>> Cc: linux-kernel at vger.kernel.org
>>>>>>> Cc: devicetree at vger.kernel.org
>>>>>>> ---
>>>>>>>
>>>>>>> Changes from v1:
>>>>>>> - fix messed up commit message
>>>>>>> ---
>>>>>>> arch/arm/boot/dts/zynq-7000.dtsi | 1 +
>>>>>>> 1 file changed, 1 insertion(+)
>>>>>>>
>>>>>>> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>>> index f3ac9bf..98233a8 100644
>>>>>>> --- a/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>>> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>>> @@ -72,6 +72,7 @@
>>>>>>> interrupts = <0 7 4>;
>>>>>>> interrupt-parent = <&intc>;
>>>>>>> clocks = <&clkc 12>;
>>>>>>> + #io-channel-cells = <1>;
>>>>>>> };
>>>>>>>
>>>>>>> can0: can at e0008000 {
>>>>>>>
>>>>>>
>>>>>> I think it will be good to the next step too.
>>>>>> It means also add iio-hwmon node too.
>>>>>>
>>>>>> What do you think?
>>>>>
>>>>> I hadn't put it in there since dts is supposed to describe hw,
>>>>> but obviously putting the actual hwmon in there makes it more useful.
>>>>
>>>> I had one discussion about this with Grant in past and it is common
>>>> mistake. It is simplification of purpose of dts.
>>>>
>>>
>>> If the iio-hwmon binding had gone through review it would have been rejected.
>>
>> Isn't it a time to deprecate it?
>
> Well, it's ABI now and has to stay forever. Deprecating it makes only sense
> if there is a replacement, which there is not. The iio-hwmon bridge has its
> usecases it's just instantiating it via devicetree which is not so nice.
>
>>
>>>
>>>>>
>>>>> I can resubmit with the hwmon node in there.
>>>>
>>>> If you grep kernel tree you will see that others are using it too.
>>>> Also there is accepted binding for that that's why I can't see big
>>>> problem with it.
>>>
>>> Since this is an application specific binding I wouldn't put it in the
>>> generic DT include file. It's a bit like adding a gpio-key binding for each
>>> of the GPIOs just in case somebody wants to use it.
>>
>> psci is system specific too.
>>
>> IIRC this driver for zynq was written by ADI or with ADI help that's why
>> you know much better than I what's the correct configuration.
>>
>> This targets PS IP which should be present in the hw all the time.
>> Not sure if for all configuration but I expect at least the part of it
>> is there all them time.
>>
>> If binding is incorrect then please remove it with removing from all
>> dts/dtsi files which have this. The same is for of probe function in the
>> driver itself.
>> If this is not done then this is just +1 case.
>>
>> If you still insist that we shouldn't do it then please at least extend
>> commit message and put there example how to wire it on zynq.
>
> There is a IIO driver for the XADC, this driver has a userspace interface
> that exposes the measurements provided by the hardware. Using the hwmon
> bridge will expose the same information just through a hwmon interface.
>
> One reason for using the iio-hwmon bridge is because you have a legacy
> application that expects the a hwmon interface. But new applications that
> want to access the XADC should really use the IIO interface if possible.
>
> In my opinion instantiating the hwmon bridge by default will only cause
> confusion. There are now redundant interfaces and users will wonder what is
> the difference between the two. Is it the same data, is it different data?
> Which is the preferred interface? Which one is 'better'?
IMHO this should be covered by documentation. One paragraph in iio-hwmon
binding can have answers for this and it will be very clear what people
should use.
Thanks,
Michal
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2] ARM: zynq: Add #io-channel-cells to (x)adc node for iio-hwmon
2017-03-17 6:46 ` Michal Simek
(?)
@ 2017-03-20 15:33 ` Michal Simek
-1 siblings, 0 replies; 36+ messages in thread
From: Michal Simek @ 2017-03-20 15:33 UTC (permalink / raw)
To: Michal Simek, Lars-Peter Clausen, Moritz Fischer
Cc: Linux Kernel Mailing List, Devicetree List, Sören Brinkmann,
linux-arm-kernel, Julia Cartwright
On 17.3.2017 07:46, Michal Simek wrote:
> On 16.3.2017 22:20, Lars-Peter Clausen wrote:
>> On 03/16/2017 07:06 PM, Michal Simek wrote:
>>> On 16.3.2017 17:51, Lars-Peter Clausen wrote:
>>>> On 03/16/2017 05:45 PM, Michal Simek wrote:
>>>>> On 16.3.2017 17:39, Moritz Fischer wrote:
>>>>>> On Thu, Mar 16, 2017 at 9:16 AM, Michal Simek <michal.simek@xilinx.com> wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> On 8.3.2017 21:11, Moritz Fischer wrote:
>>>>>>>> Fix
>>>>>>>>
>>>>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>>>>> /amba/adc@f8007100
>>>>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>>>>> /amba/adc@f8007100
>>>>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>>>>> /amba/adc@f8007100
>>>>>>>>
>>>>>>>> by adding the #io-channel-cells property.
>>>>>>>>
>>>>>>>> Signed-off-by: Moritz Fischer <mdf@kernel.org>
>>>>>>>> Cc: Michal Simek <michal.simek@xilinx.com>
>>>>>>>> Cc: Sören Brinkmann <soren.brinkmann@xilinx.com>
>>>>>>>> Cc: Julia Cartwright <julia@ni.com>
>>>>>>>> Cc: linux-kernel@vger.kernel.org
>>>>>>>> Cc: devicetree@vger.kernel.org
>>>>>>>> ---
>>>>>>>>
>>>>>>>> Changes from v1:
>>>>>>>> - fix messed up commit message
>>>>>>>> ---
>>>>>>>> arch/arm/boot/dts/zynq-7000.dtsi | 1 +
>>>>>>>> 1 file changed, 1 insertion(+)
>>>>>>>>
>>>>>>>> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>>>> index f3ac9bf..98233a8 100644
>>>>>>>> --- a/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>>>> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>>>> @@ -72,6 +72,7 @@
>>>>>>>> interrupts = <0 7 4>;
>>>>>>>> interrupt-parent = <&intc>;
>>>>>>>> clocks = <&clkc 12>;
>>>>>>>> + #io-channel-cells = <1>;
>>>>>>>> };
>>>>>>>>
>>>>>>>> can0: can@e0008000 {
>>>>>>>>
>>>>>>>
>>>>>>> I think it will be good to the next step too.
>>>>>>> It means also add iio-hwmon node too.
>>>>>>>
>>>>>>> What do you think?
>>>>>>
>>>>>> I hadn't put it in there since dts is supposed to describe hw,
>>>>>> but obviously putting the actual hwmon in there makes it more useful.
>>>>>
>>>>> I had one discussion about this with Grant in past and it is common
>>>>> mistake. It is simplification of purpose of dts.
>>>>>
>>>>
>>>> If the iio-hwmon binding had gone through review it would have been rejected.
>>>
>>> Isn't it a time to deprecate it?
>>
>> Well, it's ABI now and has to stay forever. Deprecating it makes only sense
>> if there is a replacement, which there is not. The iio-hwmon bridge has its
>> usecases it's just instantiating it via devicetree which is not so nice.
>>
>>>
>>>>
>>>>>>
>>>>>> I can resubmit with the hwmon node in there.
>>>>>
>>>>> If you grep kernel tree you will see that others are using it too.
>>>>> Also there is accepted binding for that that's why I can't see big
>>>>> problem with it.
>>>>
>>>> Since this is an application specific binding I wouldn't put it in the
>>>> generic DT include file. It's a bit like adding a gpio-key binding for each
>>>> of the GPIOs just in case somebody wants to use it.
>>>
>>> psci is system specific too.
>>>
>>> IIRC this driver for zynq was written by ADI or with ADI help that's why
>>> you know much better than I what's the correct configuration.
>>>
>>> This targets PS IP which should be present in the hw all the time.
>>> Not sure if for all configuration but I expect at least the part of it
>>> is there all them time.
>>>
>>> If binding is incorrect then please remove it with removing from all
>>> dts/dtsi files which have this. The same is for of probe function in the
>>> driver itself.
>>> If this is not done then this is just +1 case.
>>>
>>> If you still insist that we shouldn't do it then please at least extend
>>> commit message and put there example how to wire it on zynq.
>>
>> There is a IIO driver for the XADC, this driver has a userspace interface
>> that exposes the measurements provided by the hardware. Using the hwmon
>> bridge will expose the same information just through a hwmon interface.
>>
>> One reason for using the iio-hwmon bridge is because you have a legacy
>> application that expects the a hwmon interface. But new applications that
>> want to access the XADC should really use the IIO interface if possible.
>>
>> In my opinion instantiating the hwmon bridge by default will only cause
>> confusion. There are now redundant interfaces and users will wonder what is
>> the difference between the two. Is it the same data, is it different data?
>> Which is the preferred interface? Which one is 'better'?
>
> IMHO this should be covered by documentation. One paragraph in iio-hwmon
> binding can have answers for this and it will be very clear what people
> should use.
Any comment on this one?
Thanks,
Michal
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2] ARM: zynq: Add #io-channel-cells to (x)adc node for iio-hwmon
@ 2017-03-20 15:33 ` Michal Simek
0 siblings, 0 replies; 36+ messages in thread
From: Michal Simek @ 2017-03-20 15:33 UTC (permalink / raw)
To: Michal Simek, Lars-Peter Clausen, Moritz Fischer
Cc: linux-arm-kernel, Devicetree List, Linux Kernel Mailing List,
Sören Brinkmann, Julia Cartwright
On 17.3.2017 07:46, Michal Simek wrote:
> On 16.3.2017 22:20, Lars-Peter Clausen wrote:
>> On 03/16/2017 07:06 PM, Michal Simek wrote:
>>> On 16.3.2017 17:51, Lars-Peter Clausen wrote:
>>>> On 03/16/2017 05:45 PM, Michal Simek wrote:
>>>>> On 16.3.2017 17:39, Moritz Fischer wrote:
>>>>>> On Thu, Mar 16, 2017 at 9:16 AM, Michal Simek <michal.simek@xilinx.com> wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> On 8.3.2017 21:11, Moritz Fischer wrote:
>>>>>>>> Fix
>>>>>>>>
>>>>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>>>>> /amba/adc@f8007100
>>>>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>>>>> /amba/adc@f8007100
>>>>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>>>>> /amba/adc@f8007100
>>>>>>>>
>>>>>>>> by adding the #io-channel-cells property.
>>>>>>>>
>>>>>>>> Signed-off-by: Moritz Fischer <mdf@kernel.org>
>>>>>>>> Cc: Michal Simek <michal.simek@xilinx.com>
>>>>>>>> Cc: Sören Brinkmann <soren.brinkmann@xilinx.com>
>>>>>>>> Cc: Julia Cartwright <julia@ni.com>
>>>>>>>> Cc: linux-kernel@vger.kernel.org
>>>>>>>> Cc: devicetree@vger.kernel.org
>>>>>>>> ---
>>>>>>>>
>>>>>>>> Changes from v1:
>>>>>>>> - fix messed up commit message
>>>>>>>> ---
>>>>>>>> arch/arm/boot/dts/zynq-7000.dtsi | 1 +
>>>>>>>> 1 file changed, 1 insertion(+)
>>>>>>>>
>>>>>>>> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>>>> index f3ac9bf..98233a8 100644
>>>>>>>> --- a/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>>>> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>>>> @@ -72,6 +72,7 @@
>>>>>>>> interrupts = <0 7 4>;
>>>>>>>> interrupt-parent = <&intc>;
>>>>>>>> clocks = <&clkc 12>;
>>>>>>>> + #io-channel-cells = <1>;
>>>>>>>> };
>>>>>>>>
>>>>>>>> can0: can@e0008000 {
>>>>>>>>
>>>>>>>
>>>>>>> I think it will be good to the next step too.
>>>>>>> It means also add iio-hwmon node too.
>>>>>>>
>>>>>>> What do you think?
>>>>>>
>>>>>> I hadn't put it in there since dts is supposed to describe hw,
>>>>>> but obviously putting the actual hwmon in there makes it more useful.
>>>>>
>>>>> I had one discussion about this with Grant in past and it is common
>>>>> mistake. It is simplification of purpose of dts.
>>>>>
>>>>
>>>> If the iio-hwmon binding had gone through review it would have been rejected.
>>>
>>> Isn't it a time to deprecate it?
>>
>> Well, it's ABI now and has to stay forever. Deprecating it makes only sense
>> if there is a replacement, which there is not. The iio-hwmon bridge has its
>> usecases it's just instantiating it via devicetree which is not so nice.
>>
>>>
>>>>
>>>>>>
>>>>>> I can resubmit with the hwmon node in there.
>>>>>
>>>>> If you grep kernel tree you will see that others are using it too.
>>>>> Also there is accepted binding for that that's why I can't see big
>>>>> problem with it.
>>>>
>>>> Since this is an application specific binding I wouldn't put it in the
>>>> generic DT include file. It's a bit like adding a gpio-key binding for each
>>>> of the GPIOs just in case somebody wants to use it.
>>>
>>> psci is system specific too.
>>>
>>> IIRC this driver for zynq was written by ADI or with ADI help that's why
>>> you know much better than I what's the correct configuration.
>>>
>>> This targets PS IP which should be present in the hw all the time.
>>> Not sure if for all configuration but I expect at least the part of it
>>> is there all them time.
>>>
>>> If binding is incorrect then please remove it with removing from all
>>> dts/dtsi files which have this. The same is for of probe function in the
>>> driver itself.
>>> If this is not done then this is just +1 case.
>>>
>>> If you still insist that we shouldn't do it then please at least extend
>>> commit message and put there example how to wire it on zynq.
>>
>> There is a IIO driver for the XADC, this driver has a userspace interface
>> that exposes the measurements provided by the hardware. Using the hwmon
>> bridge will expose the same information just through a hwmon interface.
>>
>> One reason for using the iio-hwmon bridge is because you have a legacy
>> application that expects the a hwmon interface. But new applications that
>> want to access the XADC should really use the IIO interface if possible.
>>
>> In my opinion instantiating the hwmon bridge by default will only cause
>> confusion. There are now redundant interfaces and users will wonder what is
>> the difference between the two. Is it the same data, is it different data?
>> Which is the preferred interface? Which one is 'better'?
>
> IMHO this should be covered by documentation. One paragraph in iio-hwmon
> binding can have answers for this and it will be very clear what people
> should use.
Any comment on this one?
Thanks,
Michal
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v2] ARM: zynq: Add #io-channel-cells to (x)adc node for iio-hwmon
@ 2017-03-20 15:33 ` Michal Simek
0 siblings, 0 replies; 36+ messages in thread
From: Michal Simek @ 2017-03-20 15:33 UTC (permalink / raw)
To: linux-arm-kernel
On 17.3.2017 07:46, Michal Simek wrote:
> On 16.3.2017 22:20, Lars-Peter Clausen wrote:
>> On 03/16/2017 07:06 PM, Michal Simek wrote:
>>> On 16.3.2017 17:51, Lars-Peter Clausen wrote:
>>>> On 03/16/2017 05:45 PM, Michal Simek wrote:
>>>>> On 16.3.2017 17:39, Moritz Fischer wrote:
>>>>>> On Thu, Mar 16, 2017 at 9:16 AM, Michal Simek <michal.simek@xilinx.com> wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> On 8.3.2017 21:11, Moritz Fischer wrote:
>>>>>>>> Fix
>>>>>>>>
>>>>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>>>>> /amba/adc at f8007100
>>>>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>>>>> /amba/adc at f8007100
>>>>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>>>>> /amba/adc at f8007100
>>>>>>>>
>>>>>>>> by adding the #io-channel-cells property.
>>>>>>>>
>>>>>>>> Signed-off-by: Moritz Fischer <mdf@kernel.org>
>>>>>>>> Cc: Michal Simek <michal.simek@xilinx.com>
>>>>>>>> Cc: S?ren Brinkmann <soren.brinkmann@xilinx.com>
>>>>>>>> Cc: Julia Cartwright <julia@ni.com>
>>>>>>>> Cc: linux-kernel at vger.kernel.org
>>>>>>>> Cc: devicetree at vger.kernel.org
>>>>>>>> ---
>>>>>>>>
>>>>>>>> Changes from v1:
>>>>>>>> - fix messed up commit message
>>>>>>>> ---
>>>>>>>> arch/arm/boot/dts/zynq-7000.dtsi | 1 +
>>>>>>>> 1 file changed, 1 insertion(+)
>>>>>>>>
>>>>>>>> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>>>> index f3ac9bf..98233a8 100644
>>>>>>>> --- a/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>>>> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>>>> @@ -72,6 +72,7 @@
>>>>>>>> interrupts = <0 7 4>;
>>>>>>>> interrupt-parent = <&intc>;
>>>>>>>> clocks = <&clkc 12>;
>>>>>>>> + #io-channel-cells = <1>;
>>>>>>>> };
>>>>>>>>
>>>>>>>> can0: can at e0008000 {
>>>>>>>>
>>>>>>>
>>>>>>> I think it will be good to the next step too.
>>>>>>> It means also add iio-hwmon node too.
>>>>>>>
>>>>>>> What do you think?
>>>>>>
>>>>>> I hadn't put it in there since dts is supposed to describe hw,
>>>>>> but obviously putting the actual hwmon in there makes it more useful.
>>>>>
>>>>> I had one discussion about this with Grant in past and it is common
>>>>> mistake. It is simplification of purpose of dts.
>>>>>
>>>>
>>>> If the iio-hwmon binding had gone through review it would have been rejected.
>>>
>>> Isn't it a time to deprecate it?
>>
>> Well, it's ABI now and has to stay forever. Deprecating it makes only sense
>> if there is a replacement, which there is not. The iio-hwmon bridge has its
>> usecases it's just instantiating it via devicetree which is not so nice.
>>
>>>
>>>>
>>>>>>
>>>>>> I can resubmit with the hwmon node in there.
>>>>>
>>>>> If you grep kernel tree you will see that others are using it too.
>>>>> Also there is accepted binding for that that's why I can't see big
>>>>> problem with it.
>>>>
>>>> Since this is an application specific binding I wouldn't put it in the
>>>> generic DT include file. It's a bit like adding a gpio-key binding for each
>>>> of the GPIOs just in case somebody wants to use it.
>>>
>>> psci is system specific too.
>>>
>>> IIRC this driver for zynq was written by ADI or with ADI help that's why
>>> you know much better than I what's the correct configuration.
>>>
>>> This targets PS IP which should be present in the hw all the time.
>>> Not sure if for all configuration but I expect at least the part of it
>>> is there all them time.
>>>
>>> If binding is incorrect then please remove it with removing from all
>>> dts/dtsi files which have this. The same is for of probe function in the
>>> driver itself.
>>> If this is not done then this is just +1 case.
>>>
>>> If you still insist that we shouldn't do it then please at least extend
>>> commit message and put there example how to wire it on zynq.
>>
>> There is a IIO driver for the XADC, this driver has a userspace interface
>> that exposes the measurements provided by the hardware. Using the hwmon
>> bridge will expose the same information just through a hwmon interface.
>>
>> One reason for using the iio-hwmon bridge is because you have a legacy
>> application that expects the a hwmon interface. But new applications that
>> want to access the XADC should really use the IIO interface if possible.
>>
>> In my opinion instantiating the hwmon bridge by default will only cause
>> confusion. There are now redundant interfaces and users will wonder what is
>> the difference between the two. Is it the same data, is it different data?
>> Which is the preferred interface? Which one is 'better'?
>
> IMHO this should be covered by documentation. One paragraph in iio-hwmon
> binding can have answers for this and it will be very clear what people
> should use.
Any comment on this one?
Thanks,
Michal
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2] ARM: zynq: Add #io-channel-cells to (x)adc node for iio-hwmon
2017-03-20 15:33 ` Michal Simek
(?)
@ 2017-03-20 15:34 ` Lars-Peter Clausen
-1 siblings, 0 replies; 36+ messages in thread
From: Lars-Peter Clausen @ 2017-03-20 15:34 UTC (permalink / raw)
To: Michal Simek, Moritz Fischer
Cc: Linux Kernel Mailing List, Devicetree List, Sören Brinkmann,
linux-arm-kernel, Julia Cartwright
On 03/20/2017 04:33 PM, Michal Simek wrote:
> On 17.3.2017 07:46, Michal Simek wrote:
>> On 16.3.2017 22:20, Lars-Peter Clausen wrote:
>>> On 03/16/2017 07:06 PM, Michal Simek wrote:
>>>> On 16.3.2017 17:51, Lars-Peter Clausen wrote:
>>>>> On 03/16/2017 05:45 PM, Michal Simek wrote:
>>>>>> On 16.3.2017 17:39, Moritz Fischer wrote:
>>>>>>> On Thu, Mar 16, 2017 at 9:16 AM, Michal Simek <michal.simek@xilinx.com> wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> On 8.3.2017 21:11, Moritz Fischer wrote:
>>>>>>>>> Fix
>>>>>>>>>
>>>>>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>>>>>> /amba/adc@f8007100
>>>>>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>>>>>> /amba/adc@f8007100
>>>>>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>>>>>> /amba/adc@f8007100
>>>>>>>>>
>>>>>>>>> by adding the #io-channel-cells property.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Moritz Fischer <mdf@kernel.org>
>>>>>>>>> Cc: Michal Simek <michal.simek@xilinx.com>
>>>>>>>>> Cc: Sören Brinkmann <soren.brinkmann@xilinx.com>
>>>>>>>>> Cc: Julia Cartwright <julia@ni.com>
>>>>>>>>> Cc: linux-kernel@vger.kernel.org
>>>>>>>>> Cc: devicetree@vger.kernel.org
>>>>>>>>> ---
>>>>>>>>>
>>>>>>>>> Changes from v1:
>>>>>>>>> - fix messed up commit message
>>>>>>>>> ---
>>>>>>>>> arch/arm/boot/dts/zynq-7000.dtsi | 1 +
>>>>>>>>> 1 file changed, 1 insertion(+)
>>>>>>>>>
>>>>>>>>> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>>>>> index f3ac9bf..98233a8 100644
>>>>>>>>> --- a/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>>>>> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>>>>> @@ -72,6 +72,7 @@
>>>>>>>>> interrupts = <0 7 4>;
>>>>>>>>> interrupt-parent = <&intc>;
>>>>>>>>> clocks = <&clkc 12>;
>>>>>>>>> + #io-channel-cells = <1>;
>>>>>>>>> };
>>>>>>>>>
>>>>>>>>> can0: can@e0008000 {
>>>>>>>>>
>>>>>>>>
>>>>>>>> I think it will be good to the next step too.
>>>>>>>> It means also add iio-hwmon node too.
>>>>>>>>
>>>>>>>> What do you think?
>>>>>>>
>>>>>>> I hadn't put it in there since dts is supposed to describe hw,
>>>>>>> but obviously putting the actual hwmon in there makes it more useful.
>>>>>>
>>>>>> I had one discussion about this with Grant in past and it is common
>>>>>> mistake. It is simplification of purpose of dts.
>>>>>>
>>>>>
>>>>> If the iio-hwmon binding had gone through review it would have been rejected.
>>>>
>>>> Isn't it a time to deprecate it?
>>>
>>> Well, it's ABI now and has to stay forever. Deprecating it makes only sense
>>> if there is a replacement, which there is not. The iio-hwmon bridge has its
>>> usecases it's just instantiating it via devicetree which is not so nice.
>>>
>>>>
>>>>>
>>>>>>>
>>>>>>> I can resubmit with the hwmon node in there.
>>>>>>
>>>>>> If you grep kernel tree you will see that others are using it too.
>>>>>> Also there is accepted binding for that that's why I can't see big
>>>>>> problem with it.
>>>>>
>>>>> Since this is an application specific binding I wouldn't put it in the
>>>>> generic DT include file. It's a bit like adding a gpio-key binding for each
>>>>> of the GPIOs just in case somebody wants to use it.
>>>>
>>>> psci is system specific too.
>>>>
>>>> IIRC this driver for zynq was written by ADI or with ADI help that's why
>>>> you know much better than I what's the correct configuration.
>>>>
>>>> This targets PS IP which should be present in the hw all the time.
>>>> Not sure if for all configuration but I expect at least the part of it
>>>> is there all them time.
>>>>
>>>> If binding is incorrect then please remove it with removing from all
>>>> dts/dtsi files which have this. The same is for of probe function in the
>>>> driver itself.
>>>> If this is not done then this is just +1 case.
>>>>
>>>> If you still insist that we shouldn't do it then please at least extend
>>>> commit message and put there example how to wire it on zynq.
>>>
>>> There is a IIO driver for the XADC, this driver has a userspace interface
>>> that exposes the measurements provided by the hardware. Using the hwmon
>>> bridge will expose the same information just through a hwmon interface.
>>>
>>> One reason for using the iio-hwmon bridge is because you have a legacy
>>> application that expects the a hwmon interface. But new applications that
>>> want to access the XADC should really use the IIO interface if possible.
>>>
>>> In my opinion instantiating the hwmon bridge by default will only cause
>>> confusion. There are now redundant interfaces and users will wonder what is
>>> the difference between the two. Is it the same data, is it different data?
>>> Which is the preferred interface? Which one is 'better'?
>>
>> IMHO this should be covered by documentation. One paragraph in iio-hwmon
>> binding can have answers for this and it will be very clear what people
>> should use.
>
> Any comment on this one?
Send a patch? :)
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2] ARM: zynq: Add #io-channel-cells to (x)adc node for iio-hwmon
@ 2017-03-20 15:34 ` Lars-Peter Clausen
0 siblings, 0 replies; 36+ messages in thread
From: Lars-Peter Clausen @ 2017-03-20 15:34 UTC (permalink / raw)
To: Michal Simek, Moritz Fischer
Cc: linux-arm-kernel, Devicetree List, Linux Kernel Mailing List,
Sören Brinkmann, Julia Cartwright
On 03/20/2017 04:33 PM, Michal Simek wrote:
> On 17.3.2017 07:46, Michal Simek wrote:
>> On 16.3.2017 22:20, Lars-Peter Clausen wrote:
>>> On 03/16/2017 07:06 PM, Michal Simek wrote:
>>>> On 16.3.2017 17:51, Lars-Peter Clausen wrote:
>>>>> On 03/16/2017 05:45 PM, Michal Simek wrote:
>>>>>> On 16.3.2017 17:39, Moritz Fischer wrote:
>>>>>>> On Thu, Mar 16, 2017 at 9:16 AM, Michal Simek <michal.simek@xilinx.com> wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> On 8.3.2017 21:11, Moritz Fischer wrote:
>>>>>>>>> Fix
>>>>>>>>>
>>>>>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>>>>>> /amba/adc@f8007100
>>>>>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>>>>>> /amba/adc@f8007100
>>>>>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>>>>>> /amba/adc@f8007100
>>>>>>>>>
>>>>>>>>> by adding the #io-channel-cells property.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Moritz Fischer <mdf@kernel.org>
>>>>>>>>> Cc: Michal Simek <michal.simek@xilinx.com>
>>>>>>>>> Cc: Sören Brinkmann <soren.brinkmann@xilinx.com>
>>>>>>>>> Cc: Julia Cartwright <julia@ni.com>
>>>>>>>>> Cc: linux-kernel@vger.kernel.org
>>>>>>>>> Cc: devicetree@vger.kernel.org
>>>>>>>>> ---
>>>>>>>>>
>>>>>>>>> Changes from v1:
>>>>>>>>> - fix messed up commit message
>>>>>>>>> ---
>>>>>>>>> arch/arm/boot/dts/zynq-7000.dtsi | 1 +
>>>>>>>>> 1 file changed, 1 insertion(+)
>>>>>>>>>
>>>>>>>>> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>>>>> index f3ac9bf..98233a8 100644
>>>>>>>>> --- a/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>>>>> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>>>>> @@ -72,6 +72,7 @@
>>>>>>>>> interrupts = <0 7 4>;
>>>>>>>>> interrupt-parent = <&intc>;
>>>>>>>>> clocks = <&clkc 12>;
>>>>>>>>> + #io-channel-cells = <1>;
>>>>>>>>> };
>>>>>>>>>
>>>>>>>>> can0: can@e0008000 {
>>>>>>>>>
>>>>>>>>
>>>>>>>> I think it will be good to the next step too.
>>>>>>>> It means also add iio-hwmon node too.
>>>>>>>>
>>>>>>>> What do you think?
>>>>>>>
>>>>>>> I hadn't put it in there since dts is supposed to describe hw,
>>>>>>> but obviously putting the actual hwmon in there makes it more useful.
>>>>>>
>>>>>> I had one discussion about this with Grant in past and it is common
>>>>>> mistake. It is simplification of purpose of dts.
>>>>>>
>>>>>
>>>>> If the iio-hwmon binding had gone through review it would have been rejected.
>>>>
>>>> Isn't it a time to deprecate it?
>>>
>>> Well, it's ABI now and has to stay forever. Deprecating it makes only sense
>>> if there is a replacement, which there is not. The iio-hwmon bridge has its
>>> usecases it's just instantiating it via devicetree which is not so nice.
>>>
>>>>
>>>>>
>>>>>>>
>>>>>>> I can resubmit with the hwmon node in there.
>>>>>>
>>>>>> If you grep kernel tree you will see that others are using it too.
>>>>>> Also there is accepted binding for that that's why I can't see big
>>>>>> problem with it.
>>>>>
>>>>> Since this is an application specific binding I wouldn't put it in the
>>>>> generic DT include file. It's a bit like adding a gpio-key binding for each
>>>>> of the GPIOs just in case somebody wants to use it.
>>>>
>>>> psci is system specific too.
>>>>
>>>> IIRC this driver for zynq was written by ADI or with ADI help that's why
>>>> you know much better than I what's the correct configuration.
>>>>
>>>> This targets PS IP which should be present in the hw all the time.
>>>> Not sure if for all configuration but I expect at least the part of it
>>>> is there all them time.
>>>>
>>>> If binding is incorrect then please remove it with removing from all
>>>> dts/dtsi files which have this. The same is for of probe function in the
>>>> driver itself.
>>>> If this is not done then this is just +1 case.
>>>>
>>>> If you still insist that we shouldn't do it then please at least extend
>>>> commit message and put there example how to wire it on zynq.
>>>
>>> There is a IIO driver for the XADC, this driver has a userspace interface
>>> that exposes the measurements provided by the hardware. Using the hwmon
>>> bridge will expose the same information just through a hwmon interface.
>>>
>>> One reason for using the iio-hwmon bridge is because you have a legacy
>>> application that expects the a hwmon interface. But new applications that
>>> want to access the XADC should really use the IIO interface if possible.
>>>
>>> In my opinion instantiating the hwmon bridge by default will only cause
>>> confusion. There are now redundant interfaces and users will wonder what is
>>> the difference between the two. Is it the same data, is it different data?
>>> Which is the preferred interface? Which one is 'better'?
>>
>> IMHO this should be covered by documentation. One paragraph in iio-hwmon
>> binding can have answers for this and it will be very clear what people
>> should use.
>
> Any comment on this one?
Send a patch? :)
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v2] ARM: zynq: Add #io-channel-cells to (x)adc node for iio-hwmon
@ 2017-03-20 15:34 ` Lars-Peter Clausen
0 siblings, 0 replies; 36+ messages in thread
From: Lars-Peter Clausen @ 2017-03-20 15:34 UTC (permalink / raw)
To: linux-arm-kernel
On 03/20/2017 04:33 PM, Michal Simek wrote:
> On 17.3.2017 07:46, Michal Simek wrote:
>> On 16.3.2017 22:20, Lars-Peter Clausen wrote:
>>> On 03/16/2017 07:06 PM, Michal Simek wrote:
>>>> On 16.3.2017 17:51, Lars-Peter Clausen wrote:
>>>>> On 03/16/2017 05:45 PM, Michal Simek wrote:
>>>>>> On 16.3.2017 17:39, Moritz Fischer wrote:
>>>>>>> On Thu, Mar 16, 2017 at 9:16 AM, Michal Simek <michal.simek@xilinx.com> wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> On 8.3.2017 21:11, Moritz Fischer wrote:
>>>>>>>>> Fix
>>>>>>>>>
>>>>>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>>>>>> /amba/adc at f8007100
>>>>>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>>>>>> /amba/adc at f8007100
>>>>>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>>>>>> /amba/adc at f8007100
>>>>>>>>>
>>>>>>>>> by adding the #io-channel-cells property.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Moritz Fischer <mdf@kernel.org>
>>>>>>>>> Cc: Michal Simek <michal.simek@xilinx.com>
>>>>>>>>> Cc: S?ren Brinkmann <soren.brinkmann@xilinx.com>
>>>>>>>>> Cc: Julia Cartwright <julia@ni.com>
>>>>>>>>> Cc: linux-kernel at vger.kernel.org
>>>>>>>>> Cc: devicetree at vger.kernel.org
>>>>>>>>> ---
>>>>>>>>>
>>>>>>>>> Changes from v1:
>>>>>>>>> - fix messed up commit message
>>>>>>>>> ---
>>>>>>>>> arch/arm/boot/dts/zynq-7000.dtsi | 1 +
>>>>>>>>> 1 file changed, 1 insertion(+)
>>>>>>>>>
>>>>>>>>> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>>>>> index f3ac9bf..98233a8 100644
>>>>>>>>> --- a/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>>>>> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>>>>> @@ -72,6 +72,7 @@
>>>>>>>>> interrupts = <0 7 4>;
>>>>>>>>> interrupt-parent = <&intc>;
>>>>>>>>> clocks = <&clkc 12>;
>>>>>>>>> + #io-channel-cells = <1>;
>>>>>>>>> };
>>>>>>>>>
>>>>>>>>> can0: can at e0008000 {
>>>>>>>>>
>>>>>>>>
>>>>>>>> I think it will be good to the next step too.
>>>>>>>> It means also add iio-hwmon node too.
>>>>>>>>
>>>>>>>> What do you think?
>>>>>>>
>>>>>>> I hadn't put it in there since dts is supposed to describe hw,
>>>>>>> but obviously putting the actual hwmon in there makes it more useful.
>>>>>>
>>>>>> I had one discussion about this with Grant in past and it is common
>>>>>> mistake. It is simplification of purpose of dts.
>>>>>>
>>>>>
>>>>> If the iio-hwmon binding had gone through review it would have been rejected.
>>>>
>>>> Isn't it a time to deprecate it?
>>>
>>> Well, it's ABI now and has to stay forever. Deprecating it makes only sense
>>> if there is a replacement, which there is not. The iio-hwmon bridge has its
>>> usecases it's just instantiating it via devicetree which is not so nice.
>>>
>>>>
>>>>>
>>>>>>>
>>>>>>> I can resubmit with the hwmon node in there.
>>>>>>
>>>>>> If you grep kernel tree you will see that others are using it too.
>>>>>> Also there is accepted binding for that that's why I can't see big
>>>>>> problem with it.
>>>>>
>>>>> Since this is an application specific binding I wouldn't put it in the
>>>>> generic DT include file. It's a bit like adding a gpio-key binding for each
>>>>> of the GPIOs just in case somebody wants to use it.
>>>>
>>>> psci is system specific too.
>>>>
>>>> IIRC this driver for zynq was written by ADI or with ADI help that's why
>>>> you know much better than I what's the correct configuration.
>>>>
>>>> This targets PS IP which should be present in the hw all the time.
>>>> Not sure if for all configuration but I expect at least the part of it
>>>> is there all them time.
>>>>
>>>> If binding is incorrect then please remove it with removing from all
>>>> dts/dtsi files which have this. The same is for of probe function in the
>>>> driver itself.
>>>> If this is not done then this is just +1 case.
>>>>
>>>> If you still insist that we shouldn't do it then please at least extend
>>>> commit message and put there example how to wire it on zynq.
>>>
>>> There is a IIO driver for the XADC, this driver has a userspace interface
>>> that exposes the measurements provided by the hardware. Using the hwmon
>>> bridge will expose the same information just through a hwmon interface.
>>>
>>> One reason for using the iio-hwmon bridge is because you have a legacy
>>> application that expects the a hwmon interface. But new applications that
>>> want to access the XADC should really use the IIO interface if possible.
>>>
>>> In my opinion instantiating the hwmon bridge by default will only cause
>>> confusion. There are now redundant interfaces and users will wonder what is
>>> the difference between the two. Is it the same data, is it different data?
>>> Which is the preferred interface? Which one is 'better'?
>>
>> IMHO this should be covered by documentation. One paragraph in iio-hwmon
>> binding can have answers for this and it will be very clear what people
>> should use.
>
> Any comment on this one?
Send a patch? :)
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2] ARM: zynq: Add #io-channel-cells to (x)adc node for iio-hwmon
2017-03-20 15:34 ` Lars-Peter Clausen
@ 2017-04-07 7:11 ` Michal Simek
-1 siblings, 0 replies; 36+ messages in thread
From: Michal Simek @ 2017-04-07 7:11 UTC (permalink / raw)
To: Lars-Peter Clausen, Michal Simek, Moritz Fischer
Cc: Linux Kernel Mailing List, Devicetree List, Sören Brinkmann,
linux-arm-kernel, Julia Cartwright
On 20.3.2017 16:34, Lars-Peter Clausen wrote:
> On 03/20/2017 04:33 PM, Michal Simek wrote:
>> On 17.3.2017 07:46, Michal Simek wrote:
>>> On 16.3.2017 22:20, Lars-Peter Clausen wrote:
>>>> On 03/16/2017 07:06 PM, Michal Simek wrote:
>>>>> On 16.3.2017 17:51, Lars-Peter Clausen wrote:
>>>>>> On 03/16/2017 05:45 PM, Michal Simek wrote:
>>>>>>> On 16.3.2017 17:39, Moritz Fischer wrote:
>>>>>>>> On Thu, Mar 16, 2017 at 9:16 AM, Michal Simek <michal.simek@xilinx.com> wrote:
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> On 8.3.2017 21:11, Moritz Fischer wrote:
>>>>>>>>>> Fix
>>>>>>>>>>
>>>>>>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>>>>>>> /amba/adc@f8007100
>>>>>>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>>>>>>> /amba/adc@f8007100
>>>>>>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>>>>>>> /amba/adc@f8007100
>>>>>>>>>>
>>>>>>>>>> by adding the #io-channel-cells property.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Moritz Fischer <mdf@kernel.org>
>>>>>>>>>> Cc: Michal Simek <michal.simek@xilinx.com>
>>>>>>>>>> Cc: Sören Brinkmann <soren.brinkmann@xilinx.com>
>>>>>>>>>> Cc: Julia Cartwright <julia@ni.com>
>>>>>>>>>> Cc: linux-kernel@vger.kernel.org
>>>>>>>>>> Cc: devicetree@vger.kernel.org
>>>>>>>>>> ---
>>>>>>>>>>
>>>>>>>>>> Changes from v1:
>>>>>>>>>> - fix messed up commit message
>>>>>>>>>> ---
>>>>>>>>>> arch/arm/boot/dts/zynq-7000.dtsi | 1 +
>>>>>>>>>> 1 file changed, 1 insertion(+)
>>>>>>>>>>
>>>>>>>>>> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>>>>>> index f3ac9bf..98233a8 100644
>>>>>>>>>> --- a/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>>>>>> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>>>>>> @@ -72,6 +72,7 @@
>>>>>>>>>> interrupts = <0 7 4>;
>>>>>>>>>> interrupt-parent = <&intc>;
>>>>>>>>>> clocks = <&clkc 12>;
>>>>>>>>>> + #io-channel-cells = <1>;
>>>>>>>>>> };
>>>>>>>>>>
>>>>>>>>>> can0: can@e0008000 {
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> I think it will be good to the next step too.
>>>>>>>>> It means also add iio-hwmon node too.
>>>>>>>>>
>>>>>>>>> What do you think?
>>>>>>>>
>>>>>>>> I hadn't put it in there since dts is supposed to describe hw,
>>>>>>>> but obviously putting the actual hwmon in there makes it more useful.
>>>>>>>
>>>>>>> I had one discussion about this with Grant in past and it is common
>>>>>>> mistake. It is simplification of purpose of dts.
>>>>>>>
>>>>>>
>>>>>> If the iio-hwmon binding had gone through review it would have been rejected.
>>>>>
>>>>> Isn't it a time to deprecate it?
>>>>
>>>> Well, it's ABI now and has to stay forever. Deprecating it makes only sense
>>>> if there is a replacement, which there is not. The iio-hwmon bridge has its
>>>> usecases it's just instantiating it via devicetree which is not so nice.
>>>>
>>>>>
>>>>>>
>>>>>>>>
>>>>>>>> I can resubmit with the hwmon node in there.
>>>>>>>
>>>>>>> If you grep kernel tree you will see that others are using it too.
>>>>>>> Also there is accepted binding for that that's why I can't see big
>>>>>>> problem with it.
>>>>>>
>>>>>> Since this is an application specific binding I wouldn't put it in the
>>>>>> generic DT include file. It's a bit like adding a gpio-key binding for each
>>>>>> of the GPIOs just in case somebody wants to use it.
>>>>>
>>>>> psci is system specific too.
>>>>>
>>>>> IIRC this driver for zynq was written by ADI or with ADI help that's why
>>>>> you know much better than I what's the correct configuration.
>>>>>
>>>>> This targets PS IP which should be present in the hw all the time.
>>>>> Not sure if for all configuration but I expect at least the part of it
>>>>> is there all them time.
>>>>>
>>>>> If binding is incorrect then please remove it with removing from all
>>>>> dts/dtsi files which have this. The same is for of probe function in the
>>>>> driver itself.
>>>>> If this is not done then this is just +1 case.
>>>>>
>>>>> If you still insist that we shouldn't do it then please at least extend
>>>>> commit message and put there example how to wire it on zynq.
>>>>
>>>> There is a IIO driver for the XADC, this driver has a userspace interface
>>>> that exposes the measurements provided by the hardware. Using the hwmon
>>>> bridge will expose the same information just through a hwmon interface.
>>>>
>>>> One reason for using the iio-hwmon bridge is because you have a legacy
>>>> application that expects the a hwmon interface. But new applications that
>>>> want to access the XADC should really use the IIO interface if possible.
>>>>
>>>> In my opinion instantiating the hwmon bridge by default will only cause
>>>> confusion. There are now redundant interfaces and users will wonder what is
>>>> the difference between the two. Is it the same data, is it different data?
>>>> Which is the preferred interface? Which one is 'better'?
>>>
>>> IMHO this should be covered by documentation. One paragraph in iio-hwmon
>>> binding can have answers for this and it will be very clear what people
>>> should use.
>>
>> Any comment on this one?
>
> Send a patch? :)
>
I don't feel that I know iio enough to be good candidate to describe
this properly.
Thanks,
Michal
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v2] ARM: zynq: Add #io-channel-cells to (x)adc node for iio-hwmon
@ 2017-04-07 7:11 ` Michal Simek
0 siblings, 0 replies; 36+ messages in thread
From: Michal Simek @ 2017-04-07 7:11 UTC (permalink / raw)
To: linux-arm-kernel
On 20.3.2017 16:34, Lars-Peter Clausen wrote:
> On 03/20/2017 04:33 PM, Michal Simek wrote:
>> On 17.3.2017 07:46, Michal Simek wrote:
>>> On 16.3.2017 22:20, Lars-Peter Clausen wrote:
>>>> On 03/16/2017 07:06 PM, Michal Simek wrote:
>>>>> On 16.3.2017 17:51, Lars-Peter Clausen wrote:
>>>>>> On 03/16/2017 05:45 PM, Michal Simek wrote:
>>>>>>> On 16.3.2017 17:39, Moritz Fischer wrote:
>>>>>>>> On Thu, Mar 16, 2017 at 9:16 AM, Michal Simek <michal.simek@xilinx.com> wrote:
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> On 8.3.2017 21:11, Moritz Fischer wrote:
>>>>>>>>>> Fix
>>>>>>>>>>
>>>>>>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>>>>>>> /amba/adc at f8007100
>>>>>>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>>>>>>> /amba/adc at f8007100
>>>>>>>>>> OF: /iio_hwmon: could not get #io-channel-cells for
>>>>>>>>>> /amba/adc at f8007100
>>>>>>>>>>
>>>>>>>>>> by adding the #io-channel-cells property.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Moritz Fischer <mdf@kernel.org>
>>>>>>>>>> Cc: Michal Simek <michal.simek@xilinx.com>
>>>>>>>>>> Cc: S?ren Brinkmann <soren.brinkmann@xilinx.com>
>>>>>>>>>> Cc: Julia Cartwright <julia@ni.com>
>>>>>>>>>> Cc: linux-kernel at vger.kernel.org
>>>>>>>>>> Cc: devicetree at vger.kernel.org
>>>>>>>>>> ---
>>>>>>>>>>
>>>>>>>>>> Changes from v1:
>>>>>>>>>> - fix messed up commit message
>>>>>>>>>> ---
>>>>>>>>>> arch/arm/boot/dts/zynq-7000.dtsi | 1 +
>>>>>>>>>> 1 file changed, 1 insertion(+)
>>>>>>>>>>
>>>>>>>>>> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>>>>>> index f3ac9bf..98233a8 100644
>>>>>>>>>> --- a/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>>>>>> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>>>>>> @@ -72,6 +72,7 @@
>>>>>>>>>> interrupts = <0 7 4>;
>>>>>>>>>> interrupt-parent = <&intc>;
>>>>>>>>>> clocks = <&clkc 12>;
>>>>>>>>>> + #io-channel-cells = <1>;
>>>>>>>>>> };
>>>>>>>>>>
>>>>>>>>>> can0: can at e0008000 {
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> I think it will be good to the next step too.
>>>>>>>>> It means also add iio-hwmon node too.
>>>>>>>>>
>>>>>>>>> What do you think?
>>>>>>>>
>>>>>>>> I hadn't put it in there since dts is supposed to describe hw,
>>>>>>>> but obviously putting the actual hwmon in there makes it more useful.
>>>>>>>
>>>>>>> I had one discussion about this with Grant in past and it is common
>>>>>>> mistake. It is simplification of purpose of dts.
>>>>>>>
>>>>>>
>>>>>> If the iio-hwmon binding had gone through review it would have been rejected.
>>>>>
>>>>> Isn't it a time to deprecate it?
>>>>
>>>> Well, it's ABI now and has to stay forever. Deprecating it makes only sense
>>>> if there is a replacement, which there is not. The iio-hwmon bridge has its
>>>> usecases it's just instantiating it via devicetree which is not so nice.
>>>>
>>>>>
>>>>>>
>>>>>>>>
>>>>>>>> I can resubmit with the hwmon node in there.
>>>>>>>
>>>>>>> If you grep kernel tree you will see that others are using it too.
>>>>>>> Also there is accepted binding for that that's why I can't see big
>>>>>>> problem with it.
>>>>>>
>>>>>> Since this is an application specific binding I wouldn't put it in the
>>>>>> generic DT include file. It's a bit like adding a gpio-key binding for each
>>>>>> of the GPIOs just in case somebody wants to use it.
>>>>>
>>>>> psci is system specific too.
>>>>>
>>>>> IIRC this driver for zynq was written by ADI or with ADI help that's why
>>>>> you know much better than I what's the correct configuration.
>>>>>
>>>>> This targets PS IP which should be present in the hw all the time.
>>>>> Not sure if for all configuration but I expect at least the part of it
>>>>> is there all them time.
>>>>>
>>>>> If binding is incorrect then please remove it with removing from all
>>>>> dts/dtsi files which have this. The same is for of probe function in the
>>>>> driver itself.
>>>>> If this is not done then this is just +1 case.
>>>>>
>>>>> If you still insist that we shouldn't do it then please at least extend
>>>>> commit message and put there example how to wire it on zynq.
>>>>
>>>> There is a IIO driver for the XADC, this driver has a userspace interface
>>>> that exposes the measurements provided by the hardware. Using the hwmon
>>>> bridge will expose the same information just through a hwmon interface.
>>>>
>>>> One reason for using the iio-hwmon bridge is because you have a legacy
>>>> application that expects the a hwmon interface. But new applications that
>>>> want to access the XADC should really use the IIO interface if possible.
>>>>
>>>> In my opinion instantiating the hwmon bridge by default will only cause
>>>> confusion. There are now redundant interfaces and users will wonder what is
>>>> the difference between the two. Is it the same data, is it different data?
>>>> Which is the preferred interface? Which one is 'better'?
>>>
>>> IMHO this should be covered by documentation. One paragraph in iio-hwmon
>>> binding can have answers for this and it will be very clear what people
>>> should use.
>>
>> Any comment on this one?
>
> Send a patch? :)
>
I don't feel that I know iio enough to be good candidate to describe
this properly.
Thanks,
Michal
^ permalink raw reply [flat|nested] 36+ messages in thread
end of thread, other threads:[~2017-04-07 7:11 UTC | newest]
Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-08 20:11 [PATCH v2] ARM: zynq: Add #io-channel-cells to (x)adc node for iio-hwmon Moritz Fischer
2017-03-08 20:11 ` Moritz Fischer
2017-03-16 16:16 ` Michal Simek
2017-03-16 16:16 ` Michal Simek
2017-03-16 16:16 ` Michal Simek
2017-03-16 16:39 ` Moritz Fischer
2017-03-16 16:39 ` Moritz Fischer
2017-03-16 16:39 ` Moritz Fischer
2017-03-16 16:45 ` Michal Simek
2017-03-16 16:45 ` Michal Simek
2017-03-16 16:45 ` Michal Simek
2017-03-16 16:51 ` Lars-Peter Clausen
2017-03-16 16:51 ` Lars-Peter Clausen
2017-03-16 16:51 ` Lars-Peter Clausen
2017-03-16 17:54 ` Moritz Fischer
2017-03-16 17:54 ` Moritz Fischer
2017-03-16 21:23 ` Lars-Peter Clausen
2017-03-16 21:23 ` Lars-Peter Clausen
2017-03-16 21:23 ` Lars-Peter Clausen
2017-03-16 18:06 ` Michal Simek
2017-03-16 18:06 ` Michal Simek
2017-03-16 18:06 ` Michal Simek
2017-03-16 21:20 ` Lars-Peter Clausen
2017-03-16 21:20 ` Lars-Peter Clausen
2017-03-16 21:20 ` Lars-Peter Clausen
2017-03-17 6:46 ` Michal Simek
2017-03-17 6:46 ` Michal Simek
2017-03-17 6:46 ` Michal Simek
2017-03-20 15:33 ` Michal Simek
2017-03-20 15:33 ` Michal Simek
2017-03-20 15:33 ` Michal Simek
2017-03-20 15:34 ` Lars-Peter Clausen
2017-03-20 15:34 ` Lars-Peter Clausen
2017-03-20 15:34 ` Lars-Peter Clausen
2017-04-07 7:11 ` Michal Simek
2017-04-07 7:11 ` Michal Simek
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.