Re: [PATCH 1/3] dt-bindings: arm/marvell: ABI unstability warning about Marvell 7K/8K

3 messages, 3 authors, 2016-02-25 · open the first message on its own page

Re: [PATCH 1/3] dt-bindings: arm/marvell: ABI unstability warning about Marvell 7K/8K

From: Mark Rutland <hidden>
Date: 2016-02-25 16:16:47

On Thu, Feb 25, 2016 at 09:09:27AM +0100, Thomas Petazzoni wrote:
Hello Rob,

On Wed, 24 Feb 2016 16:07:04 -0600, Rob Herring wrote:
quoted
You should fix the incomplete information problem. I pushed back on
this, you got more information and already the binding is closer to
reality.
"You should fix the incomplete information problem". You seem to think
that it is a simple thing that can be fixed with a magic wand. But it's
not.

Either because the internal processes are complicated, or simply
because the Linux kernel support is done without cooperation from the
HW vendor (it's not the case of this Marvell platform, but it's the
case of many other platforms).
Yes, this is a problem in some cases, and that should be considered in
those cases. There are always shades of grey.

Per the above, that isn't relevant in this case. This is a pretty
black-and-white stand against the usual rules.
quoted
Mark didn't say don't submit. First, there is value in submitting even
if not accepted immediately and you have to carry some patches for
some time. It was also suggested that you err on the side of less
information in DT if things are uncertain and you have not done that.
Submitting without merging is useless. The point of submitting is to
get the code merged, to reduce the amount of out-of-tree patches we
have to carry, and to allow users of the platform to simply run
mainline on their platform.
Submitting prototypes and RFCs is the usual way we get things reviewed
early, and to allow maintainers and others to get a feel for things
earlier. Submitting patches _for merging_ when you're not sure about
things and don't want to agree to support them is what's being pushed
back on.
So this proposal really doesn't make any sense. Just like Mark initial
statement of not submitting code so early.
As what I said was evidently ambiguous, I'm on about code being _merged_
prior to being ready. Code and bindings should certainly be posted for
review as soon as possible. However, it should be recognised when things
aren't quite ready for mainline.

Even if something's unclear about a device, you can highlight more
general issues (e.g. problematic edge cases in common subsystem code),
and that's possible to have merged even if the binding isn't ready.

If you're unsure about something, but still want it merged, then you
have to commit to maintaining that as far as reasonably possible, even
if it turns out to not be quite right.
quoted
quoted
So please get to an agreement between DT binding maintainers. And stop
saying this ridiculous statement that HW vendors should stop submitting
their code to the mainline.
You want us to agree, then you won't like the answer. Bindings must be
stable. I'll allow exceptions to that, but not without push back.

In general, if there is disagreement about whether stability is
required, then it is required. See the recent sunxi discussion. That's
more between users and platform maintainers though.
Do you realize that this all DT binding stuff is today the *biggest* to
getting HW support in the Linux kernel? It has become more complicated
to merge a 4 properties DT binding than to merge multiple thousands of
lines of driver code. 
As times have changed, pain points have moved around.

To some extent that is unavoidable; more up-front effort is required
where crutches we previously relied on are not applicable.

Elsewhere we can certainly do better.

Throwing your hands up and stating "this is unstable, it might change"
is a crutch. It prevents any real solution to the pain points you
encounter, and creates pain points for others. It only provides the
_illusion_ of support.

Deliberately violating the rules and then complaining that the rules
don't work because you don't care for them is a terrible argument.

Thanks,
Mark.

Re: [PATCH 1/3] dt-bindings: arm/marvell: ABI unstability warning about Marvell 7K/8K

From: Thomas Petazzoni <hidden>
Date: 2016-02-25 16:38:11

Hello Mark,

On Thu, 25 Feb 2016 16:16:47 +0000, Mark Rutland wrote:
quoted
Either because the internal processes are complicated, or simply
because the Linux kernel support is done without cooperation from the
HW vendor (it's not the case of this Marvell platform, but it's the
case of many other platforms).
Yes, this is a problem in some cases, and that should be considered in
those cases. There are always shades of grey.
Sure.
Per the above, that isn't relevant in this case. This is a pretty
black-and-white stand against the usual rules.
I don't see why. The datasheets have not been completely written yet
for this particular chip. For other chips we've worked on where we
collaborated with the SoC vendor, we never had any datasheet, simply
because the SoC vendor doesn't have any. They have the digital logic
source code, and then tons of spreadsheets or text documents that are
not proper datasheets and that they generally cannot share with
third-parties.

Hence, even when the support for a SoC is being done in collaboration
with the SoC vendor, we don't always have a nice full datasheet that
tells us what all the registers are doing and how they are organized.
We discover things as we go.

Yes, this might be surprising to you, working at ARM, where the
technical documentation is awesome and very detailed. But trust me,
this is *NOT* what you get from many SoC vendors.
quoted
Submitting without merging is useless. The point of submitting is to
get the code merged, to reduce the amount of out-of-tree patches we
have to carry, and to allow users of the platform to simply run
mainline on their platform.
Submitting prototypes and RFCs is the usual way we get things reviewed
early, and to allow maintainers and others to get a feel for things
earlier. Submitting patches _for merging_ when you're not sure about
things and don't want to agree to support them is what's being pushed
back on.
This simply doesn't work. This initial support of a few patches (clock,
basic DT, irqchip, dmaengine) is going to be followed very soon by lots
of other patches to enable more aspects of the SoC. And we should keep
all of those patches out-of-tree, piling up hundreds of out-of-tree
patches ? Not practical at all.

And then when we'll submit them, they will all be accepted in one go,
in one kernel cycle ? Clearly not, so we would have to wait several
kernel cycles, which is clearly not what we want.

Instead, what we want is to submit the basic stuff early, and then
progressively build on top of this basic stuff by merging more and more
features. This way:

 * We don't have to pile up hundreds of out of tree patches;

 * We have a support in the mainline kernel that is progressively
   getting better as we enable more and more feature. We can show that
   4.6 has this small set of features, 4.7 has this slightly extended
   set of features supported, and so on.

So no, we clearly do not want to keep things out of tree.
quoted
So this proposal really doesn't make any sense. Just like Mark initial
statement of not submitting code so early.
As what I said was evidently ambiguous, I'm on about code being _merged_
prior to being ready. Code and bindings should certainly be posted for
review as soon as possible. However, it should be recognised when things
aren't quite ready for mainline.

Even if something's unclear about a device, you can highlight more
general issues (e.g. problematic edge cases in common subsystem code),
and that's possible to have merged even if the binding isn't ready.

If you're unsure about something, but still want it merged, then you
have to commit to maintaining that as far as reasonably possible, even
if it turns out to not be quite right.
We are perfectly fine with maintaining *code*. And we have been doing
so for several years on Marvell platforms: caring about older
platforms, converting old legacy code and legacy platforms to the
Device Tree, etc.

What we don't want to commit to is to maintain the DT stability
*before* the support for a given device is sufficiently stable/sane.
quoted
Do you realize that this all DT binding stuff is today the *biggest* to
getting HW support in the Linux kernel? It has become more complicated
to merge a 4 properties DT binding than to merge multiple thousands of
lines of driver code. 
As times have changed, pain points have moved around.

To some extent that is unavoidable; more up-front effort is required
where crutches we previously relied on are not applicable.

Elsewhere we can certainly do better.

Throwing your hands up and stating "this is unstable, it might change"
is a crutch. It prevents any real solution to the pain points you
encounter, and creates pain points for others. It only provides the
_illusion_ of support.
Could you please explicit which pain points it creates for others ?

Having unstable DT bindings specific to a platform does not create any
single pain point for anyone.

Why are you talking about "illusion" of support ? Sorry, but with
unstable DT bindings, as long as you use the DT that comes with the
kernel sources, everything works perfectly fine, and is perfectly
supported.

Even Fedora is installing DTBs in a directory that is kernel-version
specific!

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH 1/3] dt-bindings: arm/marvell: ABI unstability warning about Marvell 7K/8K

From: Maxime Ripard <hidden>
Date: 2016-02-25 21:09:06

Hi,

On Thu, Feb 25, 2016 at 04:16:47PM +0000, Mark Rutland wrote:
On Thu, Feb 25, 2016 at 09:09:27AM +0100, Thomas Petazzoni wrote:
quoted
Hello Rob,

On Wed, 24 Feb 2016 16:07:04 -0600, Rob Herring wrote:
quoted
You should fix the incomplete information problem. I pushed back on
this, you got more information and already the binding is closer to
reality.
"You should fix the incomplete information problem". You seem to think
that it is a simple thing that can be fixed with a magic wand. But it's
not.

Either because the internal processes are complicated, or simply
because the Linux kernel support is done without cooperation from the
HW vendor (it's not the case of this Marvell platform, but it's the
case of many other platforms).
Yes, this is a problem in some cases, and that should be considered in
those cases. There are always shades of grey.

Per the above, that isn't relevant in this case. This is a pretty
black-and-white stand against the usual rules.
Unless your stance, at least on the sunxi discussion was pretty much
black-and-white too. You basically said that we were supposed to
maintain a stable ABI starting right now, even though we know for a
fact that we have bindings that are simply not working.
quoted
quoted
Mark didn't say don't submit. First, there is value in submitting even
if not accepted immediately and you have to carry some patches for
some time. It was also suggested that you err on the side of less
information in DT if things are uncertain and you have not done that.
Submitting without merging is useless. The point of submitting is to
get the code merged, to reduce the amount of out-of-tree patches we
have to carry, and to allow users of the platform to simply run
mainline on their platform.
Submitting prototypes and RFCs is the usual way we get things reviewed
early, and to allow maintainers and others to get a feel for things
earlier. Submitting patches _for merging_ when you're not sure about
things and don't want to agree to support them is what's being pushed
back on.
Just to make sure we're on the same page, are you saying that we must
not merge code unless we're 100% sure of how it works, in its
entirety?
quoted
So this proposal really doesn't make any sense. Just like Mark initial
statement of not submitting code so early.
As what I said was evidently ambiguous, I'm on about code being _merged_
prior to being ready. Code and bindings should certainly be posted for
review as soon as possible. However, it should be recognised when things
aren't quite ready for mainline.

Even if something's unclear about a device, you can highlight more
general issues (e.g. problematic edge cases in common subsystem code),
and that's possible to have merged even if the binding isn't ready.

If you're unsure about something, but still want it merged, then you
have to commit to maintaining that as far as reasonably possible, even
if it turns out to not be quite right.
Except that *you* are forcing us to maintain code to deal with a use
case that simply doesn't happen, which is the definition of dead code.

Would you be ok to be forced to maintain dead code?

You've been talking about theorical use-cases, do you have any real
world one where someone actually made a case for this DT as an ABI
non-sense?

I mean, we've had the U-Boot sunxi maintainer saying they didn't care,
and actually arguing against it.

As it turns out, he's also involved in the Fedora port. If it was a
pain point for them, don't you think he would have raised it?

The only ones that seem to have a problem with it are the one not
involved in it in any way.
quoted
quoted
quoted
So please get to an agreement between DT binding maintainers. And stop
saying this ridiculous statement that HW vendors should stop submitting
their code to the mainline.
You want us to agree, then you won't like the answer. Bindings must be
stable. I'll allow exceptions to that, but not without push back.

In general, if there is disagreement about whether stability is
required, then it is required. See the recent sunxi discussion. That's
more between users and platform maintainers though.
Do you realize that this all DT binding stuff is today the *biggest* to
getting HW support in the Linux kernel? It has become more complicated
to merge a 4 properties DT binding than to merge multiple thousands of
lines of driver code. 
As times have changed, pain points have moved around.

To some extent that is unavoidable; more up-front effort is required
where crutches we previously relied on are not applicable.

Elsewhere we can certainly do better.

Throwing your hands up and stating "this is unstable, it might change"
is a crutch. It prevents any real solution to the pain points you
encounter, and creates pain points for others. It only provides the
_illusion_ of support.
https://kernelci.org/soc/mvebu/
https://kernelci.org/soc/sunxi/

I guess we should become magicians.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160225/749d2be4/attachment-0001.sig>
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help