From: Rob Herring <robh@kernel.org> Date: 2018-08-23 00:47:34
The default DT string handling in the kernel is node names and
compatibles are case insensitive and property names are case sensitive
(Sparc is the the only variation and is opposite). It seems only PPC
(and perhaps only Power Macs?) needs to support case insensitive
comparisons. It was probably a mistake to follow PPC for new arches
and we should have made everything case sensitive from the start. So I
have a few questions for the DT historians. :)
What PPC systems are case insensitive? Can we limit that to certain systems?
AFAICT, dtc at least (if not anything FDT based) has always been case
sensitive at least for node and property names. I'm not sure about
compatible strings?
Anyone see potential issues with switching all platforms except PPC
and Sparc to case sensitive comparisons?
Rob
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2018-08-23 01:03:28
On Wed, 2018-08-22 at 19:47 -0500, Rob Herring wrote:
The default DT string handling in the kernel is node names and
compatibles are case insensitive and property names are case sensitive
(Sparc is the the only variation and is opposite). It seems only PPC
(and perhaps only Power Macs?) needs to support case insensitive
comparisons. It was probably a mistake to follow PPC for new arches
and we should have made everything case sensitive from the start. So I
have a few questions for the DT historians. :)
Open Firmware itself is insensitive.
What PPC systems are case insensitive? Can we limit that to certain systems?
All PowerMacs at least, the problem is that I don't have DT images or
access to all the historical systems (and yes some people occasionally
still use them) to properly test a change in that area.
AFAICT, dtc at least (if not anything FDT based) has always been case
sensitive at least for node and property names. I'm not sure about
compatible strings?
Anyone see potential issues with switching all platforms except PPC
and Sparc to case sensitive comparisons?
From: Rob Herring <robh@kernel.org> Date: 2018-08-23 01:26:28
On Wed, Aug 22, 2018 at 8:14 PM Benjamin Herrenschmidt
[off-list ref] wrote:
On Wed, 2018-08-22 at 19:47 -0500, Rob Herring wrote:
quoted
The default DT string handling in the kernel is node names and
compatibles are case insensitive and property names are case sensitive
(Sparc is the the only variation and is opposite). It seems only PPC
(and perhaps only Power Macs?) needs to support case insensitive
comparisons. It was probably a mistake to follow PPC for new arches
and we should have made everything case sensitive from the start. So I
have a few questions for the DT historians. :)
Open Firmware itself is insensitive.
Doesn't it depend on the implementation? Otherwise, how is Sparc different?
quoted
What PPC systems are case insensitive? Can we limit that to certain systems?
All PowerMacs at least, the problem is that I don't have DT images or
access to all the historical systems (and yes some people occasionally
still use them) to properly test a change in that area.
I'm temped to break them so I can find folks to provide me with DT dumps. :)
Rob
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2018-08-23 01:29:01
On Wed, 2018-08-22 at 20:26 -0500, Rob Herring wrote:
On Wed, Aug 22, 2018 at 8:14 PM Benjamin Herrenschmidt
[off-list ref] wrote:
quoted
On Wed, 2018-08-22 at 19:47 -0500, Rob Herring wrote:
quoted
The default DT string handling in the kernel is node names and
compatibles are case insensitive and property names are case sensitive
(Sparc is the the only variation and is opposite). It seems only PPC
(and perhaps only Power Macs?) needs to support case insensitive
comparisons. It was probably a mistake to follow PPC for new arches
and we should have made everything case sensitive from the start. So I
have a few questions for the DT historians. :)
Open Firmware itself is insensitive.
Doesn't it depend on the implementation? Otherwise, how is Sparc different?
Not sure ... Forth itself is insensitive for words but maybe not for
string comparisons.
quoted
quoted
What PPC systems are case insensitive? Can we limit that to certain systems?
All PowerMacs at least, the problem is that I don't have DT images or
access to all the historical systems (and yes some people occasionally
still use them) to properly test a change in that area.
I'm temped to break them so I can find folks to provide me with DT dumps. :)
I have a collection of DT dumps but I'm not sure about the legality of
publishing them...
Cheers,
Ben.
From: Grant Likely <hidden> Date: 2018-08-23 09:02:38
On 23/08/2018 02:29, Benjamin Herrenschmidt wrote:
On Wed, 2018-08-22 at 20:26 -0500, Rob Herring wrote:
quoted
On Wed, Aug 22, 2018 at 8:14 PM Benjamin Herrenschmidt
[off-list ref] wrote:
quoted
On Wed, 2018-08-22 at 19:47 -0500, Rob Herring wrote:
quoted
The default DT string handling in the kernel is node names and
compatibles are case insensitive and property names are case sensitive
(Sparc is the the only variation and is opposite). It seems only PPC
(and perhaps only Power Macs?) needs to support case insensitive
comparisons. It was probably a mistake to follow PPC for new arches
and we should have made everything case sensitive from the start. So I
have a few questions for the DT historians. :)
Open Firmware itself is insensitive.
Doesn't it depend on the implementation? Otherwise, how is Sparc differe=
nt?
Not sure ... Forth itself is insensitive for words but maybe not for
string comparisons.
What problem are you trying to solve? I would think making everything
case insensitive would be the direction to go if you do anything. Least
possibility of breaking existing platforms in that scenario.
g.
quoted
quoted
quoted
What PPC systems are case insensitive? Can we limit that to certain sy=
stems?
quoted
quoted
All PowerMacs at least, the problem is that I don't have DT images or
access to all the historical systems (and yes some people occasionally
still use them) to properly test a change in that area.
I'm temped to break them so I can find folks to provide me with DT dumps=
. :)
I have a collection of DT dumps but I'm not sure about the legality of
publishing them...
Cheers,
Ben.
IMPORTANT NOTICE: The contents of this email and any attachments are confid=
ential and may also be privileged. If you are not the intended recipient, p=
lease notify the sender immediately and do not disclose the contents to any=
other person, use it for any purpose, or store or copy the information in =
any medium. Thank you.
From: Rob Herring <robh@kernel.org> Date: 2018-08-23 11:43:42
On Thu, Aug 23, 2018 at 4:02 AM Grant Likely [off-list ref] wrote:
On 23/08/2018 02:29, Benjamin Herrenschmidt wrote:
quoted
On Wed, 2018-08-22 at 20:26 -0500, Rob Herring wrote:
quoted
On Wed, Aug 22, 2018 at 8:14 PM Benjamin Herrenschmidt
[off-list ref] wrote:
quoted
On Wed, 2018-08-22 at 19:47 -0500, Rob Herring wrote:
quoted
The default DT string handling in the kernel is node names and
compatibles are case insensitive and property names are case sensitive
(Sparc is the the only variation and is opposite). It seems only PPC
(and perhaps only Power Macs?) needs to support case insensitive
comparisons. It was probably a mistake to follow PPC for new arches
and we should have made everything case sensitive from the start. So I
have a few questions for the DT historians. :)
Open Firmware itself is insensitive.
Doesn't it depend on the implementation? Otherwise, how is Sparc different?
Not sure ... Forth itself is insensitive for words but maybe not for
string comparisons.
What problem are you trying to solve?
I'm looking at removing device_node.name and using full_name instead
(which now is only the local node name plus unit-address). This means
replacing of_node_cmp() (and still some strcmp) calls in a lot of
places. I need to use either strncmp or strncasecmp instead.
I would think making everything
case insensitive would be the direction to go if you do anything. Least
possibility of breaking existing platforms in that scenario.
Really? Even if all the "new" arches are effectively case sensitive?
Anything using dtc and libfdt are (and json-schema certainly will be).
But I frequently say the kernel's job is not DT validation, so you
pass crap in, you get undefined results.
Rob
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2018-08-23 11:47:30
On Thu, 2018-08-23 at 06:43 -0500, Rob Herring wrote:
On Thu, Aug 23, 2018 at 4:02 AM Grant Likely [off-list ref] wrote:
quoted
What problem are you trying to solve?
I'm looking at removing device_node.name and using full_name instead
(which now is only the local node name plus unit-address). This means
replacing of_node_cmp() (and still some strcmp) calls in a lot of
places. I need to use either strncmp or strncasecmp instead.
quoted
I would think making everything
case insensitive would be the direction to go if you do anything. Least
possibility of breaking existing platforms in that scenario.
Really? Even if all the "new" arches are effectively case sensitive?
Anything using dtc and libfdt are (and json-schema certainly will be).
But I frequently say the kernel's job is not DT validation, so you
pass crap in, you get undefined results.
I tend to agree with Grant. Let's put it this way:
What is the drawback of being case insensitive ?
Do we expect that there exist a case where we will want to distinguish
between nodes that have the same name with a different case ?
If not, I don't see the point of being strict about it.
Cheers,
Ben.
From: Grant Likely <hidden> Date: 2018-08-23 11:56:10
On 23/08/2018 12:47, Benjamin Herrenschmidt wrote:
On Thu, 2018-08-23 at 06:43 -0500, Rob Herring wrote:
quoted
On Thu, Aug 23, 2018 at 4:02 AM Grant Likely [off-list ref] wrot=
e:
quoted
quoted
What problem are you trying to solve?
I'm looking at removing device_node.name and using full_name instead
(which now is only the local node name plus unit-address). This means
replacing of_node_cmp() (and still some strcmp) calls in a lot of
places. I need to use either strncmp or strncasecmp instead.
Makes sense. Simplifies the code.
quoted
quoted
I would think making everything
case insensitive would be the direction to go if you do anything. Least
possibility of breaking existing platforms in that scenario.
Really? Even if all the "new" arches are effectively case sensitive?
Anything using dtc and libfdt are (and json-schema certainly will be).
But I frequently say the kernel's job is not DT validation, so you
pass crap in, you get undefined results.
I tend to agree with Grant. Let's put it this way:
What is the drawback of being case insensitive ?
Do we expect that there exist a case where we will want to distinguish
between nodes that have the same name with a different case ? >
If not, I don't see the point of being strict about it.
I'd also add that any place where there are two nodes or props with the
same name, but different case, then it is probably a bug (or a really
bad design). Going the direction of case-insensitive eliminates the
possibility.
I do understand that that you'd like the kernel to be strict about what
it accepts, but that strictness probably makes more sense back in DTC
where it can also be checked against schema.
g.
IMPORTANT NOTICE: The contents of this email and any attachments are confid=
ential and may also be privileged. If you are not the intended recipient, p=
lease notify the sender immediately and do not disclose the contents to any=
other person, use it for any purpose, or store or copy the information in =
any medium. Thank you.
From: Rob Herring <robh@kernel.org> Date: 2018-08-23 12:08:43
On Thu, Aug 23, 2018 at 6:48 AM Benjamin Herrenschmidt
[off-list ref] wrote:
On Thu, 2018-08-23 at 06:43 -0500, Rob Herring wrote:
quoted
On Thu, Aug 23, 2018 at 4:02 AM Grant Likely [off-list ref] wrote:
quoted
What problem are you trying to solve?
I'm looking at removing device_node.name and using full_name instead
(which now is only the local node name plus unit-address). This means
replacing of_node_cmp() (and still some strcmp) calls in a lot of
places. I need to use either strncmp or strncasecmp instead.
quoted
I would think making everything
case insensitive would be the direction to go if you do anything. Least
possibility of breaking existing platforms in that scenario.
Really? Even if all the "new" arches are effectively case sensitive?
Anything using dtc and libfdt are (and json-schema certainly will be).
But I frequently say the kernel's job is not DT validation, so you
pass crap in, you get undefined results.
I tend to agree with Grant. Let's put it this way:
What is the drawback of being case insensitive ?
It's a more expensive comparison. I don't think it's a hot path, but
we do do a lot of string compares.
Property names are case sensitive already. It would be nice if
everything was treated the same way.
If we're case sensitive then it is well defined which node we'll match
and which one will be ignored. Otherwise, it is whichever one we
happen to match first.
Do we expect that there exist a case where we will want to distinguish
between nodes that have the same name with a different case ?
If someone has a DT with a node in the wrong case (as defined in the
DT spec or a binding doc) and the kernel accepts it, then it's an ABI.
Yes, as Grant says, validation is the place that should catch it, but
we're not there yet and it's cheap (cheaper in fact) for the kernel to
do.
Rob
On Thu, Aug 23, 2018 at 11:03:28AM +1000, Benjamin Herrenschmidt wrote:
On Wed, 2018-08-22 at 19:47 -0500, Rob Herring wrote:
quoted
The default DT string handling in the kernel is node names and
compatibles are case insensitive and property names are case sensitive
(Sparc is the the only variation and is opposite). It seems only PPC
(and perhaps only Power Macs?) needs to support case insensitive
comparisons. It was probably a mistake to follow PPC for new arches
and we should have made everything case sensitive from the start. So I
have a few questions for the DT historians. :)
Open Firmware itself is insensitive.
Some implementations are, yes. But those are technically broken.
quoted
What PPC systems are case insensitive? Can we limit that to certain systems?
All PowerMacs at least, the problem is that I don't have DT images or
access to all the historical systems (and yes some people occasionally
still use them) to properly test a change in that area.
If one implementation does case insensitive, it will most likely just work,
because people do not make insane names differing only in case on purpose.
Now people write other things that they only test against that implementation,
and those things now only work with case-insensitive. And you do not know
without testing if anything breaks. (But the laws of big numbers are against
you here). Since there isn't really a drawback to doing case-insensitive
always, that is a much safer way forward, much less work for everyone, even
if technically the wrong thing to do :-)
Segher
On Thu, Aug 23, 2018 at 11:29:01AM +1000, Benjamin Herrenschmidt wrote:
On Wed, 2018-08-22 at 20:26 -0500, Rob Herring wrote:
quoted
On Wed, Aug 22, 2018 at 8:14 PM Benjamin Herrenschmidt
[off-list ref] wrote:
quoted
On Wed, 2018-08-22 at 19:47 -0500, Rob Herring wrote:
quoted
The default DT string handling in the kernel is node names and
compatibles are case insensitive and property names are case sensitive
(Sparc is the the only variation and is opposite). It seems only PPC
(and perhaps only Power Macs?) needs to support case insensitive
comparisons. It was probably a mistake to follow PPC for new arches
and we should have made everything case sensitive from the start. So I
have a few questions for the DT historians. :)
Open Firmware itself is insensitive.
Doesn't it depend on the implementation? Otherwise, how is Sparc different?
Only COMPARE is standardised, and that is case-sensitive comparison. Many
systems have other words to do case-insensitive comparisons, or words where
some runtime flag determines the case-sensitivity.
Btw. A node name in Open Firmware is generically
driver-name@unit-address:device-arguments
where driver-name is the part that is in the "name" property; this whole
case-sensitivity business is even worse for FDT, where you also treat the
unit address as part of the name. In real Open Firmware the address is
compared *as a number* (or as a few numbers), so it is naturally case-
insensitive (it does not care if you write 01a0 or 01A0, or 1a0 or 000001a0
etc.)
Segher
From: Grant Likely <hidden> Date: 2018-08-23 12:48:00
On 23/08/2018 13:08, Rob Herring wrote:
On Thu, Aug 23, 2018 at 6:48 AM Benjamin Herrenschmidt
[off-list ref] wrote:
quoted
On Thu, 2018-08-23 at 06:43 -0500, Rob Herring wrote:
quoted
On Thu, Aug 23, 2018 at 4:02 AM Grant Likely [off-list ref] wro=
te:
quoted
quoted
quoted
What problem are you trying to solve?
I'm looking at removing device_node.name and using full_name instead
(which now is only the local node name plus unit-address). This means
replacing of_node_cmp() (and still some strcmp) calls in a lot of
places. I need to use either strncmp or strncasecmp instead.
quoted
I would think making everything
case insensitive would be the direction to go if you do anything. Leas=
t
quoted
quoted
quoted
possibility of breaking existing platforms in that scenario.
Really? Even if all the "new" arches are effectively case sensitive?
Anything using dtc and libfdt are (and json-schema certainly will be).
But I frequently say the kernel's job is not DT validation, so you
pass crap in, you get undefined results.
I tend to agree with Grant. Let's put it this way:
What is the drawback of being case insensitive ?
It's a more expensive comparison. I don't think it's a hot path, but
we do do a lot of string compares.
I'd hazard to argue that the cost of the string compare will not be a
source of performance problems when compared to doing a linear search
everytime a DT lookup is performed. The search algorithm is far more
problematic.
Property names are case sensitive already. It would be nice if
everything was treated the same way.
If we're case sensitive then it is well defined which node we'll match
and which one will be ignored. Otherwise, it is whichever one we
happen to match first.
If case-insensitive-always is chosen, then the kernel should probably
complain at add node time if another matching node name already exists.
That's a relatively easy thing to test.
You are right however that in the FDT world we're case-sensitive now and
if it is relaxed then we're never going back. You could try switching
everyone over to case-sensitive and see if anything falls out in
linux-next for a few cycles. I would not touch the PPC or SPARC
behaviour. I don't think it is worth the risk to make the kernel more
strict when we cannot test the result. Only if everything was changed to
case-insensitive would it make sense to touch PPC and SPARC at the same
time because it reduces the number of platform variations.
If you do change compatible matches to be case sensitive, then you
should be prepared to fix bugs where the driver uses a different case
from the DTS. In which case drivers will need to be modified to accept
the deviant property names.
g.
IMPORTANT NOTICE: The contents of this email and any attachments are confid=
ential and may also be privileged. If you are not the intended recipient, p=
lease notify the sender immediately and do not disclose the contents to any=
other person, use it for any purpose, or store or copy the information in =
any medium. Thank you.
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2018-08-23 21:49:19
On Thu, 2018-08-23 at 07:19 -0500, Segher Boessenkool wrote:
If one implementation does case insensitive, it will most likely just work,
because people do not make insane names differing only in case on purpose.
Apple did :-)
ide, vs IDE, ata vs ATA, I've seen all sort of crap there, esp. on old
machines.
Now people write other things that they only test against that implementation,
and those things now only work with case-insensitive. And you do not know
without testing if anything breaks. (But the laws of big numbers are against
you here). Since there isn't really a drawback to doing case-insensitive
always, that is a much safer way forward, much less work for everyone, even
if technically the wrong thing to do :-)
Segher
From: Michael Ellerman <mpe@ellerman.id.au> Date: 2018-08-24 05:39:19
Rob Herring [off-list ref] writes:
The default DT string handling in the kernel is node names and
compatibles are case insensitive and property names are case sensitive
(Sparc is the the only variation and is opposite). It seems only PPC
(and perhaps only Power Macs?) needs to support case insensitive
comparisons. It was probably a mistake to follow PPC for new arches
and we should have made everything case sensitive from the start. So I
have a few questions for the DT historians. :)
What PPC systems are case insensitive? Can we limit that to certain systems?
AFAICT, dtc at least (if not anything FDT based) has always been case
sensitive at least for node and property names. I'm not sure about
compatible strings?
Anyone see potential issues with switching all platforms except PPC
and Sparc to case sensitive comparisons?
Looking at some old device trees, on ppc we do definitely have mixed
case compatible strings, and I see a few hundred of those appear in the
kernel source. So I think for us it's too late.
But I don't see why that prevents other arches from switching to case
sensitive, I don't think it really hurts us in any way.
cheers
From: Rob Herring <robh@kernel.org> Date: 2018-08-24 15:14:01
On Thu, Aug 23, 2018 at 7:37 AM Segher Boessenkool
[off-list ref] wrote:
On Thu, Aug 23, 2018 at 11:29:01AM +1000, Benjamin Herrenschmidt wrote:
quoted
On Wed, 2018-08-22 at 20:26 -0500, Rob Herring wrote:
quoted
On Wed, Aug 22, 2018 at 8:14 PM Benjamin Herrenschmidt
[off-list ref] wrote:
quoted
On Wed, 2018-08-22 at 19:47 -0500, Rob Herring wrote:
quoted
The default DT string handling in the kernel is node names and
compatibles are case insensitive and property names are case sensitive
(Sparc is the the only variation and is opposite). It seems only PPC
(and perhaps only Power Macs?) needs to support case insensitive
comparisons. It was probably a mistake to follow PPC for new arches
and we should have made everything case sensitive from the start. So I
have a few questions for the DT historians. :)
Open Firmware itself is insensitive.
Doesn't it depend on the implementation? Otherwise, how is Sparc different?
Only COMPARE is standardised, and that is case-sensitive comparison. Many
systems have other words to do case-insensitive comparisons, or words where
some runtime flag determines the case-sensitivity.
Btw. A node name in Open Firmware is generically
driver-name@unit-address:device-arguments
where driver-name is the part that is in the "name" property; this whole
case-sensitivity business is even worse for FDT, where you also treat the
unit address as part of the name. In real Open Firmware the address is
compared *as a number* (or as a few numbers), so it is naturally case-
insensitive (it does not care if you write 01a0 or 01A0, or 1a0 or 000001a0
etc.)
True, but it is very rare at least in Linux to look at the
unit-addresses. dtc now does some checking on unit-addresses too, so
we should at least be consistent.
Another question: Is there ever a case where the node name in the path
aka 'driver-name' doesn't match the 'name' property? I think this
generally can't happen on FDT as the 'name' property is generated when
we unflatten it though I suppose one could craft an FDT with name
properties.
There's also various places in the kernel that check for a NULL name
which doesn't seem like it could happen either other than the root
node.
Rob
On Fri, Aug 24, 2018 at 10:14:01AM -0500, Rob Herring wrote:
Another question: Is there ever a case where the node name in the path
aka 'driver-name' doesn't match the 'name' property? I think this
generally can't happen on FDT as the 'name' property is generated when
we unflatten it though I suppose one could craft an FDT with name
properties.
In Open Firmware, it *is* the "name" property :-)
There's also various places in the kernel that check for a NULL name
which doesn't seem like it could happen either other than the root
node.
In Open Firmware the root node is required to have a "name" property, too.
There are systems that violate that rule (as with most rules...)
Segher