qemu-riscv
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Atish Kumar Patra
Subject: Re: [PATCH v14 4/5] hw/riscv: virt: Add PMU DT node to the device tree
Date: Tue, 29 Nov 2022 01:27:49 -0800

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.

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.

>
>
>



reply via email to

[Prev in Thread] Current Thread [Next in Thread]