Hi Krzysztof,
On 12.07.23 22:28, Krzysztof Kozlowski wrote:
On 12/07/2023 21:50, Jakob Hauser wrote:
...
On 11.07.23 08:13, Krzysztof Kozlowski wrote:
...
This appeared in today's next next-20230711 and causes new warnings
msm8916-samsung-serranove.dtb: extcon@14: 'connector' does not match any
of the regexes: 'pinctrl-[0-9]+'
https://krzk.eu/#/builders/90/builds/40/steps/17/logs/stdio
The commit mentions rt5033, but that is not the schema being here
tested, so clearly this is wrong or bindings were not updated.
Please fix (and test your future patches).
The implementation you see in this patch follows the guidance of yours
and Rob’s. I already expressed my discontent about it before.
To solve the message, the dt-bindings of extcon device sm5502-muic [1]
would need to be changed to allow a "connector" sub-node. That’s not the
right approach.
I still have the impression that the current implementation is based on
misunderstandings. I do think Rob’s comment that excon phandle being
deprecated [2] is valid for the USB subsystem. Your suggestion to check
"ports graph", "orientation" and "usb-role-switch" applies to USB
subsystem as well [3]. Rob took the time to add more explanation [4] but
it’s still about handling connectors in the more strict sense, which is
circling around UBS subsystem.
These discussions led to a strangely mixed-up result. I was pushed to
implement the USB subsystem connector approach upon an excton subsystem
device. As the standard USB connector approach didn’t fit, we switched
to a vendor-specific connector phandle [5]. In fact it’s kind of a
workaround for the extcon phandle.
The extcon device sm5504 is a real piece of hardware. It’s not handled
by USB subsystem but by extcon subsystem. The excton subsystem has a
method implemented to get the device by phandle [6].
I am not sure if we discuss the same problem. My email was about the DTS
and bindings, not whether this works in Linux drivers. From your reply I
feel that this patch might actually not work? This would be quite
confusing...
You added new child node "connector" to the siliconmitus,sm5504-muic, so
all I would expect that we miss here only updating that binding.
Assuming that your code was working...
The patch works.
I therefore propose to use the phandle of the extcon subsystem.
extcon in the bindings? Then we would be back to square one.
If square one is a reasonable proposal, it should be considered.
Discussions can go astray. It's a process.
The extcon subsystem offers methods to access an excton device. If there
is extcon hardware installed, using one of those methods is a pretty
straight-forward and an obvious approach.
What speaks against the use of this method? Rob argued that the
complexity of connector implementation grew over time and therefore
standard connector bindings should be used. I understand this and the
example you linked in the previous discussion shows such a complexity.
But this is about USB subsystem.
USB subsystem is not involved here. Why involving it by force?
I sure don't have the full picture. However, so far the whole discussion
seems to be based on the confusion of different extcon phandles:
"virtual" ones in USB subsystem and "real" ones in extcon subsystem. If
that's the case, we've been drifting into the wrong direction all the time.
I mean
extcon subsystem, not USB subsystem. In case you disagree, I kindly ask
you to take more time to answer in more detail and especially
case-related.
Assuming your patch works, I think above is quite specific answer - new
property is missing in sm5504 binding.
I don't think this is the right way to get rid of the issue. Sure,
technically the message disappears. Contentwise, however, we're sneaking
the confusion of our discussion into the dt-bindings. Imagine what the
description of that "connector" property in
siliconmitus,sm5502-muic.yaml would look like: "Standard USB connector
node according to usb-connector.yaml for accessing the extcon device via
devicetree." A device in the extcon subsystem doesn't need this, the
extcon subsystem already provides the method to access the extcon device
via devicetree.
Well, I guess we would silently skip a description like that by changing
"additionalProperties" from false to true.
Why do I make up such a big thing if the message could be made disappear
that easily (and burning time of yours, sorry)? This mix-up we're
implementing here is confusing. It's not helpful for further development
and implementation of rt5033 and similar hardware arrangements. The
issue that came up within the samsung-serranove dts patch here is a good
indication of that.
I can prepare a patchset to dissolve this USB/extcon mix-up (basically
square one, as you called it).
Alternatively, if all my tries to clarify a possible misunderstanding
are in vain and no one else intervenes, I guess I have no other option
than preparing a patch to change the dt-bindungs of
siliconmitus,sm5502-muic.yaml.
And specifically to Krzysztof I ask for more politeness in
your way of communicating. I understand you’re answering hundreds of
requests a day but the communication we had in the past weeks is really
frustrating.
Sorry to hear that, please accept my apologies. I went through all my
replies to you in past few weeks and could not find any particular
impolite behavior from my side.
I'm not used to the fast-paced interaction on the kernel lists. Maybe I
have mistaken some of your comments. In that case sorry for my accusation.
...
Kind regards,
Jakob
