Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH v2 3/6] ARM: dts: rockchip: remove interrupts properties
 from pwm nodes rv1108.dtsi
To:     wens@kernel.org
Cc:     =?UTF-8?Q?Heiko_St=c3=bcbner?= <heiko@sntech.de>,
        Rob Herring <robh+dt@kernel.org>,
        Thierry Reding <thierry.reding@gmail.com>,
        =?UTF-8?Q?Uwe_Kleine-K=c3=b6nig?= <u.kleine-koenig@pengutronix.de>,
        Lee Jones <lee.jones@linaro.org>, linux-pwm@vger.kernel.org,
        devicetree <devicetree@vger.kernel.org>,
        linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
        "open list:ARM/Rockchip SoC..." <linux-rockchip@lists.infradead.org>,
        linux-kernel <linux-kernel@vger.kernel.org>
References: <20210411131007.21757-1-jbx6244@gmail.com>
 <20210411131007.21757-3-jbx6244@gmail.com>
 <CAGb2v67s7a4GARfAnROKS40kaYQpdW_qWX=HX6GU09jV9wrbXw@mail.gmail.com>
 <31b5ff50-afe5-b446-7d3c-943d148814d8@gmail.com>
 <CAGb2v65+A402jCPVRJdDBdxqAEYOJmFTkKB4LJCvnW89hXb8QA@mail.gmail.com>
From:   Johan Jonker <jbx6244@gmail.com>
Message-ID: <20b7c702-9412-93b4-3174-e8633bc413d7@gmail.com>
Date:   Mon, 12 Apr 2021 13:04:59 +0200
User-Agent: Mozilla/5.0 (X11; Linux i686; rv:68.0) Gecko/20100101
 Thunderbird/68.11.0
MIME-Version: 1.0
In-Reply-To: <CAGb2v65+A402jCPVRJdDBdxqAEYOJmFTkKB4LJCvnW89hXb8QA@mail.gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 4/12/21 12:33 PM, Chen-Yu Tsai wrote:
> On Mon, Apr 12, 2021 at 6:03 PM Johan Jonker <jbx6244@gmail.com> wrote:
>>
>> On 4/12/21 5:15 AM, Chen-Yu Tsai wrote:
>>> On Sun, Apr 11, 2021 at 9:11 PM Johan Jonker <jbx6244@gmail.com> wrote:
>>>>
>>>> A test with the command below gives this error:
>>>>
>>>> /arch/arm/boot/dts/rv1108-evb.dt.yaml:
>>>> pwm@10280000: 'interrupts' does not match any of the regexes:
>>>> 'pinctrl-[0-9]+'
>>>>
>>>> "interrupts" is an undocumented property, so remove them
>>>> from pwm nodes in rv1108.dtsi.
>>>>
>>>> make ARCH=arm dtbs_check
>>>> DT_SCHEMA_FILES=Documentation/devicetree/bindings/pwm/pwm-rockchip.yaml
>>>>
>>>> Signed-off-by: Johan Jonker <jbx6244@gmail.com>
>>>
>>> Given that the interrupts were specified, meaning they are wired up in hardware,
>>> shouldn't the solution be to add the interrupts property to the binding instead?
>>>
>>> After all, the device tree describes the actual hardware, not just what the
>>> implementations need.
>>>
>>> ChenYu
>>>
>>
>> Hi,
>>
>> The question of what to do with it was asked in version 1, but no answer
>> was given, so I made a proposal.
>> The device tree description should be complete, but also as lean as
>> possible. If someone manages to sneak in undocumented properties without
>> reason then the ultimate consequence should be removal I think.
>>
>> Not sure about the (missing?) rv1108 TRM, but for rk3328 the interrupt
>> is used for:
>>
>> PWM_INTSTS 0x0040 W 0x00000000 Interrupt Status Register
>>   Channel Interrupt Polarity Flag
>>     This bit is used in capture mode in order to identify the
>>     transition of the input waveform when interrupt is generated.
>>   Channel Interrupt Status
>>     Interrupt generated
>>
>> PWM_INT_EN 0x0044 W 0x00000000 Interrupt Enable Register
>>   Channel Interrupt Enable
>>
>> Is there any current realistic use/setup for it to convince rob+dt this
>> should be added to pwm-rockchip.yaml?

Found:
pwm3 combined with ir uses a irq. Keep that as it is for now.

https://github.com/rockchip-linux/kernel/blob/develop-4.19/drivers/input/remotectl/rockchip_pwm_remotectl.c

> 
> Well, the PWM core has capture support, and pwm-sti implements it with
> interrupt support, so I guess there's at least a legitimate case for
> adding that to the binding. Whether someone has an actual use case for
> it and adds code to implement it is another story.
> 
>> The rk3328 interrupt rkpwm_int seems shared between channels, but only
>> included to pwm3. What is the proper way for that?
> 
> I guess the bigger question is why was the PWM controller split into
> four device nodes, instead of just one encompassing the whole block.
> Now we'd have to introduce a new binding to support capture mode and
> interrupts.
> 
> In that case I agree with dropping the interrupts for now, as it just
> won't fit. But I would add this additional information to the commit
> message.

Will wait with adding "interrupts" to pwm-rockchip.yaml till someone
makes a solution for the whole block. Convert only current
document/binding to reduce notifications.

For Heiko: patch 3 + 5 can go in the garbage bin:
[PATCH v2 3/6] ARM: dts: rockchip: remove interrupts properties from pwm
nodes rv1108.dtsi
[PATCH v2 5/6] arm64: dts: rockchip: remove interrupts properties from
pwm nodes rk3328.dtsi

Johan

> 
> 
> Regards
> ChenYu
>