svarbanov: ping
hverkuil: pong
svarbanov: there are three venus patches from you for v4.15. Do any of those need to be backported to v4.13/14?
hverkuil: IMO all of them could and should be backported
svarbanov: thanks, added cc for stable for those three patches.
hverkuil: thanks!
sailus: do you have 15 minutes for me later this afternoon? (somewhere between 2-4 pm Norway time).
I feel I am close to understanding the fwnode acpi patch (v15 24/32), but I have some questions.
hverkuil: Yes, sure!
14:00 Norwegian time? I'll need to leave around 15:30.
sailus: is that patch specific to ACPI ?
pinchartl: Well, yes.
You could use it on DT as well but there's no reason to.
As you can refer to any node in the tree to begin with.
mchehab: How would you like to handle the atomisp MAINTAINERS patch?
hverkuil: Hallo!
sailus: shouldn't it be an ACPI-specific function then ?
pinchartl: So... you'd like to make it ACPI specific in a way?
I guess you could add #ifdefs.
sailus: just back from a meeting. Do you have time in 10 minutes?
Yes.
sailus: I mean should it really be part of the fwnode API ?
I would expect fwnode functions to apply to both ACPI and DT
sailus: I'm looking at three files: your v15 24/32 patch, your reply containing the ACPI endpoint example and  the comment before __acpi_node_get_property_reference
 in drivers/acpi/property.c.
pinchartl: Which function in particular?
In the ACPI endpoint example you use "\_SB.PCI0.ISP", in the other two places "^LED" and "^GPIO" is used. What's the difference in notation?
v4l2_fwnode_reference_get_int_prop
oh, that question was for pinchartl, sorry :-)
sailus: v4l2_fwnode_reference_get_int_prop :-)
hverkuil: thanks :-)
sailus: pinchartl: I'll wait until you two finished the discussion. It's very confusing otherwise.
\ refers to the root of the tree whereas ^ refers to the parent device.
pinchartl: There's nothing technically specific to ACPI in the function itself.
is it useful with DT too ?
It would make sense to move it to the ACPI framework later on as it can be also used for parsing graphs.
pinchartl: Would you like to use it on DT?
sailus: I don't understand what it does, so, no
but it could be useful if you gave a DT-based example to explain the function
it would be easier to understand than an ACPI example
Interesting idea.
there's no point in doing that if it is only going to be used with acpi. acpi devs probably don't understand DT code :-)
hverkuil: I'd say they still understand that generally better than the other way around. :-)
But assuming this function would be moved to the ACPI framework I don't see that example would stay.
I'm mainly asking for an example to understand the purpose of the function myself. whether it should be part of the documentation or not, I don't know yet
but the text-based description with the ACPI example isn't clear enough to understand the purpose of the function
pinchartl: I've added documentation because it was requested during the review.
sailus: I'm not complaining about the presence of documentation, what bothers me is that I can't understand the function's purpose by reading the documentation
I can convert the example to DT if that helps.
;-)
which means to me that the documentation isn't good enough and should be improved, not that it should be removed
sailus: let's wait until we've had our discussion. I'm slowly starting to understand it, so once I've confirmed that what I think is happening is correct, perhaps I can come up with a better comment.
I'm pleased to hear that I am not the only one who doesn't understand that function :-)
I might be able to understand it by reading the code, but I haven't tried
pinchartl: You can find all ACPI framework code you'll need and more under drivers/acpi. ;-)
sailus: I don't need any ACPI code, thank you :-)
hverkuil: Did you have questions you wanted to ask?
laurent@avalon ~/src/kernel/linux-2.6.git $ find drivers/acpi -type f -name "*.[ch]" | xargs cat | wc -l
154820
laurent@avalon ~/src/kernel/linux-2.6.git $ find drivers/of -type f -name "*.[ch]" | xargs cat | wc -l
13387
no other comment
pinchartl: Useless bothering of cat.
sailus: yes
Package () { ^LED, 0, ^LED, 1 },
So this has a two references to a device, each followed by a number of arguments.
Correct.
nprops is the number of arguments.
index is which reference should be used.
fwnode_property_get_reference_args parses this information, right?
Yes, it does.
fwnode_args.fwnode is the fwnode that is referenced, fwnode_args.args are the arguments for that reference.
Next it walks over all child nodes under fwnode_args.fwnode.
No scratch that
Next it iterates over the properties. For each property it walks over all child nodes under fwnode_args.fwnode.
v4l2_fwnode_reference_get_int_prop iterates over the child nodes of fwnode.
It reads the first property name in @props in each child node.
If it finds a node with the property and the property value is the same as that given in the reference argument, then it returns the fwnode containing that property if nprops == 1, but if nprops > 1, then it continues iterating under the fwnode containing the first property!?
And then compares the value read with the value in fwnode_args.args[0].
If a match is found, it proceeds to follow that child node.
hverkuil: If nprops > 1, then the process is repeated, but starting from the child node. Next entries are picked from the @props array and fwnode_args.args array.
I think it is my lack of acpi knowledge that's the problem: what's a fwnode for ACPI? I guess Scope and Device are both fwnodes.
Is Name or Package an fwnode?
Devices are fwnodes.
Hierarchical data extension nodes are also fwnodes, i.e. Name objects that have ToUUID("dbb8e3e6-5886-4ba6-8795-1319f52a966b") in them.
These are under Device objects.
OK. So then I don't understand the endpoint example you posted. In that example the function is called with two properties: port and endpoint.
Hmm.
The first (port) would find fwnode Name (PRT0, Package())
But there is no 'endpoint' property underneath that one.
hverkuil: In ACPI there is no hierarchy for the name objects inside a device.
Ah.
The hierarchy exists only at runtime after the ACPI framework has parsed the data structure following the hierarchical data extension references.
Btw. I just posted a DT example mimicing this.
OK. So the function finds property @prop which has 1 or more references. Then it takes reference at index @index.
Correct.
Then it checks if that reference contains the properties mentioned in @props and in the arguments of the original reference.
(that's a very awkward sentence)
Child nodes under the referred node.
Not that node itself.
prop[i] + fwnode_args.args[i] specify which property should be found.
Yes.
That's basically a duplication of the property, isn't it? That's what makes this so odd, the reference contains the value of the thing it references.
No, it doesn't.
The child nodes are picked based on it.
In your example: "Package () { "remote-endpoint", Package() { \_SB.PCI0.ISP, 4, 0 } },"
the '4' refers to the 4 in: "Package () { "port", 4 }, /* CSI-2 port number */"
right?
Correct.
'4' being the csi port number.
So the first reference contains the value of what it references.
Yes.
How else would you refer to it?
In the DT you would just refer to it directly: remote-endpoint = <&isp_port4_ep>;
Yes, you do, but on ACPI you can only refer to device nodes.
Right, and that's the reason for having to do this.
Indeed.
it just looks really, really odd.
brb, I need a tea refill.
Yes, tea helps.
So in the example when looking up ("port", 0) in \_SB.PCI0.I2C2.CAM0 it will find fwnode CAM0?
or fwnode PRT0? (still not quite clear about that)
fwnode_property_get_reference_args() will return CAM0 and the two integer arguments.
In the first iteration of v4l2_fwnode_reference_get_int_prop results in finding PRT0, based on the "port" property value (0) and integer argument value (0).
OK. But the property "endpoint" is in "EP00", not in "PRT0". Unless you meant to write: @props: "port", "endpoint0"
scratch that last line.
Correct. The endpoint property can be found in the endpoint node (otherwise it wouldn't be specific to the endpoint).
OK. So after finding "port" fwnode is set to PRT0, then we loop for child nodes under PRT0 with property "endpoint". But there isn't any such property under PRT0.
hverkuil: Isn't there?
EP00 does have "endpoint" property and its value is 0.
OK, back one step.
the "port" property is found in the fwnode with the name "PRT0", right?
So the child pointer points to that fwnode.
What do you mean by the child pointer?
The child variable in fwnode_for_each_child_node(fwnode, child) {
Ah.
That'll iterate over all nodes directly under fwnode.
I.e. EP00 under PRT0.
I thought that the hierachy was:
CAM0 { PRT0, EP00 }  (i.e. PRT0 and EP00 are on the same level)
not: CAM0 { PRT0 { EP00 } }
On the ACPI table, yes.
let me look up what fwnode_for_each_child_node does.
But the tree structure is created at runtime once the ACPI framework parses the table.
The same with flattened Device tree btw.
so in this function it expects to see the hierarchy as CAM0 { PRT0 { EP00 } } ?
and if I would add a third property (FOO) it would be CAM0 { PRT0 { EP00  { FOO } } }
Yes.
Is EP00 a child of PRT0 because PRT0 contains 'Package () { "endpoint0", "EP00" },'? Thus making a child reference to EP00?
Correct. The hierarchical data extension above that tells this to the ACPI framework.
I.e. ToUUID("dbb8e3e6-5886-4ba6-8795-1319f52a966b").
hverkuil: Sorry, I need to leave.
Ah, now I am beginning to understand.
OK, TB continued.
I'll be back in the evening, or we can continue tomorrow.
Well, very late in the evening.
Tomorrow?
tomorrow.
What time would work for you?
I have nothing planned. Anything after 10am Oslo time is fine.
hverkuil: I'll have at least half an hour then, possibly an hour.
Let's continue then.
Have a nice evening!
ack. You too!
Hi everyone. I'm re-writing lirc_zilog, and I'm having trouble with the i2c device with two addresses.
So the zilog device always has address 0x70 for transmit, and 0x71 for receive. lirc_zilog has two i2c_device_id's and a kref with some complicated handling
I was wondering if anyone had seen this problem before and come up with a better solution.
i2c_new_dummy()
I don't think that existed when lirc_zilog was written.
It's used extensively in e.g. adv7604.c which has a whole bunch of different i2c addresses.
drivers/media/i2c/adv7511.c is probably a bit easier to follow.
hverkuil: ah, that is exactly what I need! Thanks
hverkuil: hello!!
ribalda: hi!
hverkuil: I have an issue with a ctrl->event. for an array control.
ctrl->flags does not show V4L2_CTRL_FLAG_HAS_PAYLOAD;
on v4l2_query_ext_ctrl. if (ctrl->is_ptr) qc->flags |= V4L2_CTRL_FLAG_HAS_PAYLOAD;
but that code is missing in fill_event. Is a bug or a feature :)
ribalda: definitely a bug.
I completely forgot about it.
can I send a patch?
yes, please.
give me a sec
:)
Hmm, that might be one to backport.
hverkuil: that is your call
hverkuil: sent
Hi, I'm trying to capture SAP audio from an analog cable source using a saa7131e card (genius tvgo a12). I'm setting the audmode to LANG2/SAP with S_TUNER but i'm still getting the main audio, am I missing something? I verified that there is SAP audio in the channel and can be captured on Windows