Re: [PATCH v14 4/5] hw/riscv: virt: Add PMU DT node to the device tree

[Atish Kumar Patra]

On Tue, Nov 29, 2022 at 3:54 PM <Conor.Dooley@microchip.com> wrote:
>
> +CC Rob, which I probably should've done earlier, so
> context all preserved
>
> On 29/11/2022 09:42, Conor Dooley wrote:
> > On 29/11/2022 09:27, Atish Kumar Patra wrote:
> >> EXTERNAL EMAIL: Do not click links or open attachments unless you know the 
> >> content is safe
> >>
> >> On Mon, Nov 28, 2022 at 11:32 PM <Conor.Dooley@microchip.com> wrote:
> >>>
> >>> On 29/11/2022 07:08, Andrew Jones wrote:
> >>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know 
> >>>> the content is safe
> >>>>
> >>>> On Mon, Nov 28, 2022 at 09:10:03PM +0000, Conor.Dooley@microchip.com 
> >>>> wrote:
> >>>>> On 28/11/2022 20:41, Atish Kumar Patra wrote:
> >>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know 
> >>>>>> the content is safe
> >>>>>>
> >>>>>> On Mon, Nov 28, 2022 at 12:38 PM <Conor.Dooley@microchip.com> wrote:
> >>>>>>>
> >>>>>>> On 28/11/2022 20:16, Atish Kumar Patra wrote:
> >>>>>>>> On Thu, Nov 24, 2022 at 5:17 AM Conor Dooley 
> >>>>>>>> <conor.dooley@microchip.com> wrote:
> >>>>>>>>>
> >>>>>>>>> On Wed, Aug 24, 2022 at 03:17:00PM -0700, Atish Patra wrote:
> >>>>>>>>>> Qemu virt machine can support few cache events and cycle/instret 
> >>>>>>>>>> counters.
> >>>>>>>>>> It also supports counter overflow for these events.
> >>>>>>>>>>
> >>>>>>>>>> Add a DT node so that OpenSBI/Linux kernel is aware of the virt 
> >>>>>>>>>> machine
> >>>>>>>>>> capabilities. There are some dummy nodes added for testing as well.
> >>>>>>>>>
> >>>>>>>>> Hey Atish!
> >>>>>>>>>
> >>>>>>>>> I was fiddling with dumping the virt machine dtb again today to 
> >>>>>>>>> check
> >>>>>>>>> some dt-binding changes I was making for the isa string would play
> >>>>>>>>> nicely with the virt machine & I noticed that this patch has 
> >>>>>>>>> introduced
> >>>>>>>>> a new validation failure:
> >>>>>>>>>
> >>>>>>>>> ./build/qemu-system-riscv64 -nographic -machine 
> >>>>>>>>> virt,dumpdtb=qemu.dtb
> >>>>>>>>>
> >>>>>>>>> dt-validate -p 
> >>>>>>>>> ../linux/Documentation/devicetree/bindings/processed-schema.json 
> >>>>>>>>> qemu.dtb
> >>>>>>>>> /home/conor/stuff/qemu/qemu.dtb: soc: pmu: 
> >>>>>>>>> {'riscv,event-to-mhpmcounters': [[1, 1, 524281, 2, 2, 524284, 
> >>>>>>>>> 65561, 65561, 524280, 65563, 65563, 524280, 65569, 65569, 524280, 
> >>>>>>>>> 0, 0, 0, 0, 0]], 'compatible': ['riscv,pmu']} should not be valid 
> >>>>>>>>> under {'type': 'object'}
> >>>>>>>>>           From schema: 
> >>>>>>>>> /home/conor/.local/lib/python3.10/site-packages/dtschema/schemas/simple-bus.yaml
> >>>>>>>>>
> >>>>>>>>> I assume this is the aforementioned "dummy" node & you have no 
> >>>>>>>>> intention
> >>>>>>>>> of creating a binding for this?
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>> It is a dummy node from Linux kernel perspective. OpenSbi use this
> >>>>>>>> node to figure out the hpmcounter mappings.
> >>>>>>>
> >>>>>>> Aye, but should it not have a binding anyway, since they're not
> >>>>>>> meant to be linux specific?
> >>>>>>>
> >>>>>> It is documented in OpenSBI.
> >>>>>> https://github.com/riscv-software-src/opensbi/blob/master/docs/pmu_support.md
> >>>>>>
> >>>>>> Are you suggesting that any non-Linux specific DT nodes should be part
> >>>>>> of Linux DT binding as well ?
> >>>>>
> >>>>> I thought the point was that they were *not* meant to be linux specific,
> >>>>> just happening to be housed there.
> >>>>>
> >>>>
> >>>> I'm not sure if there's an official policy on where DT nodes should be
> >>>> specified, but it looks like Samuel's opinion is that they should live
> >>>> in the Linux kernel, whether they're used there or not [1].
> >>>>
> >>>> [1] http://lists.infradead.org/pipermail/opensbi/2022-October/003522.html
> >>>
> >>> Yah, that was also my understanding. See also U-Boot moving to unify
> >>> their custom bindings into the linux repo:
> >>> https://lore.kernel.org/linux-devicetree/20220930001410.2802843-1-sjg@chromium.org/
> >>>
> >>
> >> This adds the U-Boot specific DT properties to the dts schema itself,
> >> not Linux kernel DT bindings.
> >
> > Yeah, sorry. I muddled things up a little there. My point was that they
> > are trying to get to a stage where dt-validate and those tools work for
> > them too. I'm not sure were I said "linux repo" rather than "dt-schema
> > repo" when I double checked the file paths in the link before pasting it
> > to make sure it was the dt-schema one.. I blame it being early.
> >
> >> I am not opposed to adding PMU DT bindings to Linux but there should
> >> be a clear policy on this.
> >> What about OpenSBI domain DT bindings ?
> >> If every other DT based open source project starts adding their DT
> >> binding to the Linux kernel, that may go downhill pretty soon.
>
> Rob, perhaps you can be a source of clarity here :) My early morning
> muddling didn't help things.
>
>
> > Maybe I am misunderstanding, but I had thought the goal was to get to
> > user-independent bindings. Rob and Krzysztof certainly labour the point
> > that the bindings should not reflect how one operating system's drivers
> > would like to see them & u-boot or FreeBSD using a property is grounds
> > for it not being removed from the bindings in the linux tree.
> >
> > I'll go and actually ask Rob.
>
> I did go & ask Rob, to which he said "I'll apply it even if no driver."
>

In Linux DT binding or dt schema repo ? I am a bit confused now as we
talked about both above :).

> Do you want to whip up a binding, or shall I?
>