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

[Andrew Jones]

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

Thanks,
drew