From: Michal Simek <email@example.com>
To: Rob Herring <firstname.lastname@example.org>, Sean Anderson <email@example.com>
Cc: Linux PWM List <firstname.lastname@example.org>,
Michal Simek <email@example.com>,
Alvaro Gamez <firstname.lastname@example.org>,
Subject: Re: [PATCH v3 1/2] dt-bindings: pwm: Add Xilinx AXI Timer
Date: Fri, 14 May 2021 10:50:37 +0200 [thread overview]
Message-ID: <email@example.com> (raw)
On 5/13/21 10:43 PM, Rob Herring wrote:
> On Thu, May 13, 2021 at 10:28 AM Sean Anderson <firstname.lastname@example.org> wrote:
>> On 5/13/21 10:33 AM, Sean Anderson wrote:
>> > On 5/12/21 10:16 PM, Rob Herring wrote:
>> > > On Tue, May 11, 2021 at 03:12:37PM -0400, Sean Anderson wrote:
>> > >> This adds a binding for the Xilinx LogiCORE IP AXI Timer. This device is
>> > >> a "soft" block, so it has many parameters which would not be
>> > >> configurable in most hardware. This binding is usually automatically
>> > >> generated by Xilinx's tools, so the names and values of some properties
>> > >> must be kept as they are. Replacement properties have been provided for
>> > >> new device trees.
>> > >
>> > > Because you have some tool generating properties is not a reason we have
>> > > to accept them upstream.
>> > These properties are already in arch/microblaze/boot/dts/system.dts and
>> > in the devicetree supplied to Linux by qemu. Removing these properties
>> > will break existing setups, which I would like to avoid.
> Already in use in upstream dts files is different than just
> 'automatically generated' by vendor tools.
>> > > 'deprecated' is for what *we* have deprecated.
>> > Ok. I will remove that then.
>> > >
>> > > In this case, I don't really see the point in defining new properties
>> > > just to have bool.
>> > I don't either, but it was requested, by Michal...
>> Err, your comment on the original bindings was
>> > Can't all these be boolean?
> With no other context, yes that's what I would ask. Now you've given
> me some context, between using the existing ones and 2 sets of
> properties to maintain, I choose the former.
>> And Michal commented
>> > I think in this case you should described what it is used by current
>> > driver in Microblaze and these options are required. The rest are by
>> > design optional.
>> > If you want to change them to different value then current binding
>> > should be deprecated and have any transition time with code alignment.
>> So that is what I tried to accomplish with this revision. I also tried
>> allowing something like
>> xlnx,one-timer-only = <0>; /* two timers */
>> xlnx,one-timer-only = <1>; /* one timer */
>> xlnx,one-timer-only; /* one timer */
>> /* property absent means two timers */
>> but I was unable to figure out how to express this with json-schema. I
>> don't think it's the best design either...
> json-schema would certainly let you, but generally we don't want
> properties to have more than 1 type.
One thing is what it is in system.dts file which was committed in 2009
and there are just small alignments there. But none is really using it.
Maybe I should just delete it.
And this version was generated by Xilinx ancient tools at that time. All
parameters there are fully describing HW and they are not changing. Only
new one can be added.
From the current microblaze code you can see which properties are really
It means from my point of view these should be listed in the binding.
clock-frequency is optional by code when clock is defined.
All other properties listed in system.dts are from my perspective
optional and that's how it should be.
I think DT binding patch should reflect this state as patch itself.
And then PWM should be added on the top as separate patch.
Note: In past we were using only parameters and name we got from tools
but over years we were fine to use for example bool properties and we
just aligned Xilinx device tree generator to match it. That's why not a
problem to deprecate any property and move to new one. Xilinx DTG is
already prepared for it and it is easy to remap it.
next prev parent reply other threads:[~2021-05-14 8:50 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-11 19:12 [PATCH v3 1/2] dt-bindings: pwm: Add Xilinx AXI Timer Sean Anderson
2021-05-11 19:12 ` [PATCH v3 2/2] clocksource: Add support for " Sean Anderson
2021-05-11 19:19 ` Sean Anderson
2021-05-12 8:31 ` kernel test robot
2021-05-14 8:59 ` Michal Simek
2021-05-14 14:40 ` Sean Anderson
2021-05-17 7:54 ` Michal Simek
2021-05-17 22:15 ` Sean Anderson
2021-05-19 7:24 ` Michal Simek
2021-05-20 20:13 ` Sean Anderson
2021-05-24 7:00 ` Michal Simek
2021-05-24 18:34 ` Sean Anderson
2021-05-25 6:11 ` Uwe Kleine-König
2021-05-25 14:30 ` Sean Anderson
2021-06-16 12:12 ` Michal Simek
2021-06-18 21:24 ` Sean Anderson
2021-06-24 16:25 ` Michal Simek
2021-05-12 18:35 ` [PATCH v3 1/2] dt-bindings: pwm: Add " Rob Herring
2021-05-13 2:16 ` Rob Herring
2021-05-13 14:33 ` Sean Anderson
2021-05-13 15:28 ` Sean Anderson
2021-05-13 20:43 ` Rob Herring
2021-05-13 21:01 ` Sean Anderson
2021-05-14 8:50 ` Michal Simek [this message]
2021-05-14 17:13 ` Sean Anderson
2021-05-17 8:28 ` Michal Simek
2021-05-17 14:40 ` Sean Anderson
2021-05-17 14:49 ` Michal Simek
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).