Re: [Xen-devel] [PATCH] xen/arm: copy cpu clock-frequency to CPU DT node.

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2013-11-05 at 09:56 +0000, Ian Campbell wrote:
> On Mon, 2013-11-04 at 13:35 -0500, Jon Fraser wrote:
> > On Mon, 2013-11-04 at 17:12 +0000, Ian Campbell wrote:
> > > On Fri, 2013-11-01 at 16:58 -0400, Jon Fraser wrote:
> > > > When creating the CPU DT node, copy the clock-frequency if present.
> > > > 
> > ...
> > > Julien's the expert but I think you need to use dt_property_read_u32
> > > here, to get the correct endianness conversion (as well as for pure
> > > forms sake of using the correct API for the job).
> > > 
> > I'll fix that.
> > 
> > > >              break;
> > > >          }
> > > >      }
> > > > @@ -457,6 +459,12 @@ static int make_cpus_node(const struct domain *d, 
> > > > void *fdt,
> > > >          if ( res )
> > > >              return res;
> > > >  
> > > > +        if (clock_frequency) {
> > > > +            res = fdt_property_cell(fdt, "clock-frequency", *(u32 
> > > > *)clock_frequency);
> > > 
> > > I suppose there ought to be some API for this side of things too, but I
> > > can't see it right now...
> > > 
> > > Note that fdt_property_cell contains a cpu_to_fdt32 so it is converting
> > > while the read of the property not, so I think the code is broken as is?
> > 
> > Yesss, it is broken.  When I checked the property in /proc/device-tree,
> > I failed to realize it was endian swapped.
> 
> I guess nothing much critical is relying on this value. What is it
> supposed to be used for?
> linux/Documentation/devicetree/booting-without-of.txt seems to imply it
> is mostly optional for non-PPC.

BTW, I'm asking because I'm not sure if exposing the underlying clock
speed to the guest VCPU is the right thing to do. It can vary across
pCPUs (e.g. big.LITTLE) and with power control etc.

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel