Subject: Re: [PATCH v2] ARM: zynq: Add #io-channel-cells to (x)adc node for
 iio-hwmon
To: Lars-Peter Clausen <lars@metafoo.de>,
        Michal Simek <michal.simek@xilinx.com>,
        Moritz Fischer <moritz.fischer.private@gmail.com>
References: <1489003884-2489-1-git-send-email-mdf@kernel.org>
 <e6da3f38-7039-66e5-5b9d-a44ff42aa9c9@xilinx.com>
 <CAJYdmeNc48=OOvRiUjqPBJhz9RCbzeaRGu1Tfm=hrvzkJrSsag@mail.gmail.com>
 <7e4fbe16-8686-5399-748d-af222c630338@xilinx.com>
 <37f29986-6491-67ab-144c-eb485cc1fdcc@metafoo.de>
CC: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Devicetree List <devicetree@vger.kernel.org>,
        =?UTF-8?Q?S=c3=b6ren_Brinkmann?= <soren.brinkmann@xilinx.com>,
        linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
        Julia Cartwright <julia@ni.com>
From: Michal Simek <michal.simek@xilinx.com>
Message-ID: <6767a340-b150-c21a-b6cb-5ca1ce2cd721@xilinx.com>
Date: Thu, 16 Mar 2017 19:06:08 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101
 Thunderbird/45.7.0
MIME-Version: 1.0
In-Reply-To: <37f29986-6491-67ab-144c-eb485cc1fdcc@metafoo.de>
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit
SpamDiagnosticOutput: 1:99
SpamDiagnosticMetadata: NSPM
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Mar 2017 18:06:14.0178
 (UTC)
X-MS-Exchange-CrossTenant-Id: 657af505-d5df-48d0-8300-c31994686c5c
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=657af505-d5df-48d0-8300-c31994686c5c;Ip=[149.199.60.100];Helo=[xsj-pvapsmtpgw02]
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM2PR0201MB0719
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 3214
Lines: 98

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