diff for duplicates of <1453120919.6020.142.camel@citrix.com> diff --git a/a/1.txt b/N1/1.txt index b5ecf92..15bba9d 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -4,12 +4,11 @@ On Mon, 2016-01-18 at 11:24 +0000, David Vrabel wrote: > > 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. ->=20 +> > There's no documentation on the interface, which makes it difficult to -> review.=C2=A0=C2=A0At a first look it looks very specific to the particul= -ar Linux +> review. At a first look it looks very specific to the particular Linux > implementation of a clk subsystem. ->=20 +> > > --- /dev/null > > +++ b/include/xen/interface/io/clkif.h > > @@ -0,0 +1,41 @@ @@ -21,7 +20,7 @@ ar Linux > > + * http://www.opensource.org/licenses/gpl-license.html > > + * http://www.gnu.org/copyleft/gpl.html > > + */ ->=20 +> > ABIs should be under a more permissive license so they can be used by > other (non-GPLv2) operating systems. @@ -32,9 +31,8 @@ implementation for one particular os. > > + unsigned long rate; > > + char clk_name[32]; ->=20 -> Where does the frontend get these names from?=C2=A0=C2=A031 character nam= -es seems +> +> Where does the frontend get these names from? 31 character names seems > rather limiting. Indeed. @@ -45,30 +43,21 @@ and tie into the host device tree. 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 =3D 4 +/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 correspondin= -g firmware table identifier (so path in the DT case), which the toolstack k= -nows, 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). +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 obs= -erves the response only needs the id). +The request would then include the index and not the name (and as David observes the response only needs the id). -As well as properly documenting the meaning of the operations=C2=A0 -the clkif.h should also define the xenstore nodes and contain the binary la= -youts of the req/rsp structs (see netif.h for examples of both, blkif.h als= -o includes examples of the former). +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). -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, b= -ut for the pvclk node in xenstore to contain the index. +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. > > + > > +DEFINE_RING_TYPES(xen_clkif, struct xen_clkif_request, struct @@ -76,5 +65,5 @@ ut for the pvclk node in xenstore to contain the index. > > +#define XEN_CLK_RING_SIZE __CONST_RING_SIZE(xen_clkif, PAGE_SIZE) > > + > > +#endif -> >=20 ->=20 +> > +> diff --git a/a/content_digest b/N1/content_digest index 7c0665e..fe7a2f8 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -21,12 +21,11 @@ "> > is ok from you side for handlding clk when doing platform device\n" "> > passhthrough. Any comments are appreciated, and your comments may\n" "> > give me a better direction.\n" - ">=20\n" + "> \n" "> There's no documentation on the interface, which makes it difficult to\n" - "> review.=C2=A0=C2=A0At a first look it looks very specific to the particul=\n" - "ar Linux\n" + "> review.\302\240\302\240At a first look it looks very specific to the particular Linux\n" "> implementation of a clk subsystem.\n" - ">=20\n" + "> \n" "> > --- /dev/null\n" "> > +++ b/include/xen/interface/io/clkif.h\n" "> > @@ -0,0 +1,41 @@\n" @@ -38,7 +37,7 @@ "> > + * http://www.opensource.org/licenses/gpl-license.html\n" "> > + * http://www.gnu.org/copyleft/gpl.html\n" "> > + */\n" - ">=20\n" + "> \n" "> ABIs should be under a more permissive license so they can be used by\n" "> other (non-GPLv2) operating systems.\n" "\n" @@ -49,9 +48,8 @@ "\n" "> > +\tunsigned long rate;\n" "> > +\tchar clk_name[32];\n" - ">=20\n" - "> Where does the frontend get these names from?=C2=A0=C2=A031 character nam=\n" - "es seems\n" + "> \n" + "> Where does the frontend get these names from?\302\240\302\24031 character names seems\n" "> rather limiting.\n" "\n" "Indeed.\n" @@ -62,30 +60,21 @@ "I think a better model might be for each clk to have it's own subdirectory\n" "under the overall clock bus node, e.g. something like:\n" "\n" - "/local/domain/<...>/clk/0/nr-clks =3D 4\n" + "/local/domain/<...>/clk/0/nr-clks = 4\n" "/local/domain/<...>/clk/0/clk-0/ ...\n" "/local/domain/<...>/clk/0/clk-1/ ...\n" "/lo\n" "cal/domain/<...>/clk/0/clk-2/ ...\n" "/local/domain/<...>/clk/0/clk-3/ ...\n" "\n" - "and for each subdirectory to contain the a node containing the correspondin=\n" - "g firmware table identifier (so path in the DT case), which the toolstack k=\n" - "nows, so it can setup the f/e and b/e to correspond as necessary, and the f=\n" - "/e device needn't necessarily use the same name as the backend/host).\n" + "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).\n" "\n" - "The request would then include the index and not the name (and as David obs=\n" - "erves the response only needs the id).\n" + "The request would then include the index and not the name (and as David observes the response only needs the id).\n" "\n" - "As well as properly documenting the meaning of the operations=C2=A0\n" - "the clkif.h should also define the xenstore nodes and contain the binary la=\n" - "youts of the req/rsp structs (see netif.h for examples of both, blkif.h als=\n" - "o includes examples of the former).\n" + "As well as properly documenting the meaning of the operations\302\240\n" + "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).\n" "\n" - "I'd also like to see a description of the DT bindings, which I assume must =\n" - "be needed such that the devices clocks property has something to refer to. =\n" - "For example maybe it doesn't make sense for xenstore to contain the path, b=\n" - "ut for the pvclk node in xenstore to contain the index.\n" + "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.\n" "\n" "> > +\n" "> > +DEFINE_RING_TYPES(xen_clkif, struct xen_clkif_request, struct\n" @@ -93,7 +82,7 @@ "> > +#define XEN_CLK_RING_SIZE __CONST_RING_SIZE(xen_clkif, PAGE_SIZE)\n" "> > +\n" "> > +#endif\n" - "> >=20\n" - >=20 + "> > \n" + > -4618f6439742e5e3f14a19d76e4b34762e9aaa216213dff3ec17449bfdb2d9f8 +bf37f177a3c2221a78fd19c1f77a1678d143defca50298a08f9d07be7b5889a8
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.