[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [RFC/WIP] xen: clk: introudce pvclk for device passthrough
Hello Ian, On Mon, Jan 18, 2016 at 12:41:59PM +0000, Ian Campbell wrote: >On Mon, 2016-01-18 at 11:24 +0000, David Vrabel wrote: >> On 16/01/16 05:22, Peng Fan wrote: >> > This patch was just a initial patch, not sure whether this way >> > is ok from you side for handlding clk when doing platform device >> > passhthrough. Any comments are appreciated, and your comments may >> > give me a better direction. >> >> There's no documentation on the interface, which makes it difficult to >> review. At a first look it looks very specific to the particular Linux >> implementation of a clk subsystem. >> >> > --- /dev/null >> > +++ b/include/xen/interface/io/clkif.h >> > @@ -0,0 +1,41 @@ >> > +/* >> > + * The code contained herein is licensed under the GNU General Public >> > + * License. You may obtain a copy of the GNU General Public License >> > + * Version 2 or later at the following locations: >> > + * >> > + * http://www.opensource.org/licenses/gpl-license.html >> > + * http://www.gnu.org/copyleft/gpl.html >> > + */ >> >> ABIs should be under a more permissive license so they can be used by >> other (non-GPLv2) operating systems. > >... along the same lines proposals for new ABIs should be made in the form >of patches to xen.git:xen/include/public/io/ before being submitted as an >implementation for one particular os. I had no idea about this before. Do you mean that before I implement pvclk for linux, I need to first post the clkif part to xen devel? If it is, I'll split the interface part and send this part to xen-devel@xxxxxxxxxxxxxxxxxxxx for review. > > >> > + unsigned long rate; >> > + char clk_name[32]; >> >> Where does the frontend get these names from? 31 character names seems >> rather limiting. > >Indeed. > >At a guess I would assume they come from the device-tree given to the guest >and tie into the host device tree. Yeah. the clk_name is got from DomU dts, and in Dom0 there is also a same name. > >I think a better model might be for each clk to have it's own subdirectory >under the overall clock bus node, e.g. something like: > >/local/domain/<...>/clk/0/nr-clks = 4 >/local/domain/<...>/clk/0/clk-0/ ... >/local/domain/<...>/clk/0/clk-1/ ... >/lo >cal/domain/<...>/clk/0/clk-2/ ... >/local/domain/<...>/clk/0/clk-3/ ... > >and for each subdirectory to contain the a node containing the corresponding >firmware table identifier (so path in the DT case), which the toolstack knows, >so it can setup the f/e and b/e to correspond as necessary, and the f/e device >needn't necessarily use the same name as the backend/host). > >The request would then include the index and not the name (and as David >observes the response only needs the id). For now, I have not began the userspace libxl part for pvclk, I use the libxl pvusb code for test (: The id acctually means what operation is needed, such as CLK_PREPARE, CLK_UNPREPARE, CLK_SET_RATE, CLK_GET_RATE. I'll add more text to document this in V2. > >As well as properly documenting the meaning of the operations >the clkif.h should also define the xenstore nodes and contain the binary >layouts of the req/rsp structs (see netif.h for examples of both, blkif.h also >includes examples of the former). ok. I'll take netif/blkif for reference. > >I'd also like to see a description of the DT bindings, which I assume must be >needed such that the devices clocks property has something to refer to. For >example maybe it doesn't make sense for xenstore to contain the path, but for >the pvclk node in xenstore to contain the index. The DT bindings for xen pvclk, I use this: " clks: clks { compatible = "xen,xen-clk"; #clock-cells = <1>; clock-output-names = "uart2_root_clk"; }; " the clock-output-names will be parsed and be registered as clk root. The device will use index to refer to the clk, as the following: " clocks = <&clks 0>, <&clks 0>; clock-names = "ipg", "per"; " 0 means the first one in clock-output-names. To the xenstore part, I am not sure whether need to expose the clock relate info to xenstore. I just want to store the clock names in xenstore to let user know what clocks are now handled by DomU. How about the following? doamin1: /local/domain/1/device/vclk/nr-clks /local/domain/1/device/vclk/clk-0/name /local/domain/1/device/vclk/clk-1/name domain2: /local/domain/2/device/vclk/nr-clks /local/domain/2/device/vclk/clk-0/name /local/domain/2/device/vclk/clk-1/name domain0: /local/domain/0/backend/vclk/1/0/nr-clks /local/domain/0/backend/vclk/1/0/clk-0/name /local/domain/0/backend/vclk/1/0/clk-1/name /local/domain/0/backend/vclk/1/1/nr-clks /local/domain/0/backend/vclk/1/1/clk-0/name /local/domain/0/backend/vclk/1/1/clk-1/name Or do not store the name and nr-clks in domain0? Thanks, Peng. > >> > + >> > +DEFINE_RING_TYPES(xen_clkif, struct xen_clkif_request, struct >> > xen_clkif_response); >> > +#define XEN_CLK_RING_SIZE __CONST_RING_SIZE(xen_clkif, PAGE_SIZE) >> > + >> > +#endif >> > >> _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |