Happy to reply inline next time. I'm still getting used to this format :-). Here's the context around my clock frequency question-- I'd really like to understand this better: > > + clock-names: > > + description: > > + Input clock for the sensor. > > + items: > > + - const: xvclk > > + > > + clock-frequency: > > + description: > > + Frequency of the xvclk clock in Hertz. > > The frequency of clock should go via common clock framework - you have > get_rate and set_rate. Drop entire property. I am trying to be consistent with the ov8856 driver and bindings but would be happy to change. I’m not familiar with that framework though. Is there somewhere I could read about this, including the driver and device-tree changes I need to use this? > > > + minimum: 6000000 > > + default: 24000000 > > + maximum: 27000000 > > + > > + dovdd-supply: > > + description: > > + Definition of the regulator used as interface power supply. On Wed, Nov 9, 2022 at 10:19 AM Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> wrote: > > On 09/11/2022 16:12, Nicholas Roth wrote: > > Hey, > > > > I’m doing my best to follow along here. Your feedback didn’t get lost and I tried my best to follow it— I just must not have understood it correctly. > > > >> 1. There is no driver, no DTS. You received the feedback about it. > > > > Driver: I submitted the .c files to linux-media, and as part of the review they asked for me to separately submit device tree bindings (https://patchwork.kernel.org/project/linux-media/patch/20221106171129.166892-2-nicholas@xxxxxxxxxxxxx/). Are you saying that the driver and the bindings should be the same commit after all? Are you saying something else? I’m afraid I still don’t understand what you mean by this, but I want to, and I’m trying to. > > They come as one patchset. Separate patches, one patchset. Otherwise you > get checkpatch errors, right? > > > > >> 2. Wrong cc list. You were asked to use get_maintainers.pl and still > >> decide not to. > > I included the people from get_maintainers.pl, but it seems like you would like for me to include all entries, including the multiple mailing lists. Do I understand correctly? > > Yes. Do not strip some lists based on your preference. Why only some > people should receive this, not everyone involved in the subsystem? > > > >> > > > >> How can you test bindings with libcamera? > > I validate the device tree + driver on this setup, but I am happy to drop the comment about validation. > > It's not about bindings then. > > > > >> > >> Filename still does not match compatible. ovti,ov8858.yaml > > I was trying to be consistent with ov8856.yaml, but happy to change the file name if that’s the convention. Is there a doc I can read with this information or is it institutional knowledge? > > All recent submissions follow this, so the best is to take last commits. > > >> > >> The frequency of clock should go via common clock framework - you have > >> get_rate and set_rate. Drop entire property. > > I am trying to be consistent with the ov8856 driver and bindings but would be happy to change. I’m not familiar with that framework though. Is there somewhere I could read about this, including the driver and device-tree changes I need to use this? > > This is very difficult to respond. Please use inline comments, I have no > clue which part you are now commenting. This is not mailing list style > response. > > >> > >> Which driver? In OpenBSD? Which version of OpenBSD? Drop the sentence. > > Seems like your point here and in the subsequent comments is to avoid implementation specifics. That makes a ton of sense, and I’ll make those changes for v3. > > > > All of your other comments make sense explicitly and I’ll make these changes. Please help me understand what you mean by (1) and (2), correct me where I’m wrong or misunderstanding you here, and I’ll submit the v3. > > Best regards, > Krzysztof >
