diff for duplicates of <1453197968.6020.233.camel@citrix.com> diff --git a/a/1.txt b/N1/1.txt index d398ccd..c9e2703 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -1,6 +1,6 @@ On Tue, 2016-01-19 at 10:43 +0800, Peng Fan wrote: > Hello Ian, ->=20 +> > 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: @@ -8,14 +8,13 @@ On Tue, 2016-01-19 at 10:43 +0800, Peng Fan 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 part= -icular +> > > 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 @@ @@ -29,19 +28,19 @@ icular > > > > + * 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. -> >=20 +> > > > ... 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. ->=20 +> > 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? ->=20 +> > If it is, I'll split the interface part and send this part to > xen-devel@lists.xenproject.org for review. @@ -50,47 +49,46 @@ Yes, please. xen.git contains the canonical definition of all Xen PV protocols, copies are then taking into OSes for implementation purposes. ->=20 -> >=20 -> >=20 +> +> > +> > > > > > + unsigned long rate; > > > > + char clk_name[32]; -> > >=20 -> > > Where does the frontend get these names from?=C2=A0=C2=A031 character= - names +> > > +> > > Where does the frontend get these names from? 31 character names > > > seems > > > rather limiting. -> >=20 +> > > > Indeed. -> >=20 +> > > > At a guess I would assume they come from the device-tree given to the > > guest > > and tie into the host device tree. ->=20 +> > Yeah. the clk_name is got from DomU dts, and in Dom0 there is also a same > name. ->=20 -> >=20 +> +> > > > 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: -> >=20 -> > /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/ ... -> >=20 +> > > > 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). -> >=20 +> > > > The request would then include the index and not the name (and as David > > observes the response only needs the id). ->=20 +> > For now, I have not began the userspace libxl part for pvclk, I use the > libxl pvusb code for test (: @@ -116,61 +114,56 @@ case need to be a binding from the DT world to the pvclk world to allow the guest DT to remain consistent (i.e. so devices have something they can point at which can be resolved into a pvclk). -> >=20 +> > > > 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. ->=20 +> > The DT bindings for xen pvclk, I use this: > " -> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= -=C2=A0=C2=A0=C2=A0=C2=A0clks: clks { -> compatible =3D "xen,xen-clk"; -> #clock-cells =3D <1>; -> clock-output-names =3D "uart2_root_clk"; +> 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: > " -> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= -=C2=A0=C2=A0=C2=A0=C2=A0clocks =3D <&clks 0>, <&clks 0>; -> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0clock-names =3D "ipg", "= -per"; +> clocks = <&clks 0>, <&clks 0>; +> clock-names = "ipg", "per"; > " > 0 means the first one in clock-output-names. This binding should be defined in Documentation/devicetree/Bindings in the usual way. ->=20 +> > 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. ->=20 +> > How about the following? ->=20 +> > doamin1: > /local/domain/1/device/vclk/nr-clks > /local/domain/1/device/vclk/clk-0/name > /local/domain/1/device/vclk/clk-1/name -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= -=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0^/0 + ^/0 ->=20 +> > domain2: > /local/domain/2/device/vclk/nr-clks > /local/domain/2/device/vclk/clk-0/name > /local/domain/2/device/vclk/clk-1/name -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= -=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0^/0 + ^/0 ->=20 +> > domain0: > /local/domain/0/backend/vclk/1/0/nr-clks > /local/domain/0/backend/vclk/1/0/clk-0/name @@ -183,8 +176,7 @@ Correct. > /local/domain/0/backend/vclk/1/1/clk-1/name Should be: -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= -=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /2/0 + /2/0 In /local/domain/*/device/vclk/<N> the <N> is the devid or bus number (better to think of it as the latter in this case). @@ -198,8 +190,7 @@ In /local/domain/<X>/backend/vclk/<Y>/<Z>/ you have: > Or do not store the name and nr-clks in domain0? The f/e directory should contain the identifier which the frontend needs to -=C2=A0do the lookup from the firmware tables into pvclk space -- i.e. the n= -ame + do the lookup from the firmware tables into pvclk space -- i.e. the name given in the f/e device tree node (maybe call it dt-name?). The b/e directory should contain the name of the clock from the backend diff --git a/a/content_digest b/N1/content_digest index 2c528af..55bfd78 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -19,7 +19,7 @@ "b\0" "On Tue, 2016-01-19 at 10:43 +0800, Peng Fan wrote:\n" "> Hello Ian,\n" - ">=20\n" + "> \n" "> On Mon, Jan 18, 2016 at 12:41:59PM +0000, Ian Campbell wrote:\n" "> > On Mon, 2016-01-18 at 11:24 +0000, David Vrabel wrote:\n" "> > > On 16/01/16 05:22, Peng Fan wrote:\n" @@ -27,14 +27,13 @@ "> > > > 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\n" "> > > to\n" - "> > > review.=C2=A0=C2=A0At a first look it looks very specific to the part=\n" - "icular\n" + "> > > review.\302\240\302\240At a first look it looks very specific to the particular\n" "> > > 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" @@ -48,19 +47,19 @@ "> > > > + * 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" - "> >=20\n" + "> > \n" "> > ... along the same lines proposals for new ABIs should be made in the\n" "> > form\n" "> > of patches to xen.git:xen/include/public/io/ before being submitted as\n" "> > an\n" "> > implementation for one particular os.\n" - ">=20\n" + "> \n" "> I had no idea about this before. Do you mean that before I implement\n" "> pvclk for linux, I need to first post the clkif part to xen devel?\n" - ">=20\n" + "> \n" "> If it is, I'll split the interface part and send this part to\n" "> xen-devel@lists.xenproject.org for review.\n" "\n" @@ -69,47 +68,46 @@ "xen.git contains the canonical definition of all Xen PV protocols, copies\n" "are then taking into OSes for implementation purposes.\n" "\n" - ">=20\n" - "> >=20\n" - "> >=20\n" + "> \n" + "> > \n" + "> > \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=\n" - " names\n" + "> > > \n" + "> > > Where does the frontend get these names from?\302\240\302\24031 character names\n" "> > > seems\n" "> > > rather limiting.\n" - "> >=20\n" + "> > \n" "> > Indeed.\n" - "> >=20\n" + "> > \n" "> > At a guess I would assume they come from the device-tree given to the\n" "> > guest\n" "> > and tie into the host device tree.\n" - ">=20\n" + "> \n" "> Yeah. the clk_name is got from DomU dts, and in Dom0 there is also a same\n" "> name.\n" - ">=20\n" - "> >=20\n" + "> \n" + "> > \n" "> > I think a better model might be for each clk to have it's own\n" "> > subdirectory\n" "> > under the overall clock bus node, e.g. something like:\n" - "> >=20\n" - "> > /local/domain/<...>/clk/0/nr-clks =3D 4\n" + "> > \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" - "> >=20\n" + "> > \n" "> > and for each subdirectory to contain the a node containing the\n" "> > corresponding firmware table identifier (so path in the DT case), which\n" "> > the toolstack knows, so it can setup the f/e and b/e to correspond as\n" "> > necessary, and the f/e device needn't necessarily use the same name as\n" "> > the backend/host).\n" - "> >=20\n" + "> > \n" "> > The request would then include the index and not the name (and as David\n" "> > observes the response only needs the id).\n" - ">=20\n" + "> \n" "> For now, I have not began the userspace libxl part for pvclk, I use the\n" "> libxl pvusb code for test (:\n" "\n" @@ -135,61 +133,56 @@ "guest DT to remain consistent (i.e. so devices have something they can\n" "point at which can be resolved into a pvclk).\n" "\n" - "> >=20\n" + "> > \n" "> > I'd also like to see a description of the DT bindings, which I assume\n" "> > must be needed such that the devices clocks property has something to\n" "> > refer to. For example maybe it doesn't make sense for xenstore to\n" "> > contain the path, but for the pvclk node in xenstore to contain the\n" "> > index.\n" - ">=20\n" + "> \n" "> The DT bindings for xen pvclk, I use this:\n" "> \"\n" - "> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=\n" - "=C2=A0=C2=A0=C2=A0=C2=A0clks: clks {\n" - "> \t\t\tcompatible =3D \"xen,xen-clk\";\n" - "> \t\t\t#clock-cells =3D <1>;\n" - "> \t\t\tclock-output-names =3D \"uart2_root_clk\";\n" + "> \302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240clks: clks {\n" + "> \t\t\tcompatible = \"xen,xen-clk\";\n" + "> \t\t\t#clock-cells = <1>;\n" + "> \t\t\tclock-output-names = \"uart2_root_clk\";\n" "> \t\t};\n" "> \"\n" "> the clock-output-names will be parsed and be registered as clk root. The\n" "> device will use index to refer to the clk, as the following:\n" "> \"\n" - "> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=\n" - "=C2=A0=C2=A0=C2=A0=C2=A0clocks =3D <&clks 0>, <&clks 0>;\n" - "> \t=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0clock-names =3D \"ipg\", \"=\n" - "per\";\n" + "> \302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240clocks = <&clks 0>, <&clks 0>;\n" + "> \t\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240clock-names = \"ipg\", \"per\";\n" "> \"\n" "> 0 means the first one in clock-output-names.\n" "\n" "This binding should be defined in Documentation/devicetree/Bindings in the\n" "usual way.\n" "\n" - ">=20\n" + "> \n" "> To the xenstore part, I am not sure whether need to expose the clock\n" "> relate info to xenstore. I just want to store the clock names in xenstore\n" "> to let\n" "> user know what clocks are now handled by DomU.\n" - ">=20\n" + "> \n" "> How about the following?\n" - ">=20\n" + "> \n" "> doamin1:\n" "> /local/domain/1/device/vclk/nr-clks\n" "> /local/domain/1/device/vclk/clk-0/name\n" "> /local/domain/1/device/vclk/clk-1/name\n" "\n" - "=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=\n" - "=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0^/0\n" + "\302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240^/0\n" "\n" - ">=20\n" + "> \n" "> domain2:\n" "> /local/domain/2/device/vclk/nr-clks\n" "> /local/domain/2/device/vclk/clk-0/name\n" "> /local/domain/2/device/vclk/clk-1/name\n" "\n" - "=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=\n" - "=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0^/0\n" + "\302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240^/0\n" "\n" - ">=20\n" + "> \n" "> domain0:\n" "> /local/domain/0/backend/vclk/1/0/nr-clks\n" "> /local/domain/0/backend/vclk/1/0/clk-0/name\n" @@ -202,8 +195,7 @@ "> /local/domain/0/backend/vclk/1/1/clk-1/name\n" "\n" "Should be:\n" - "=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=\n" - "=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /2/0\n" + "\302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 /2/0\n" "\n" "In /local/domain/*/device/vclk/<N> the <N> is the devid or bus number\n" "(better to think of it as the latter in this case).\n" @@ -217,8 +209,7 @@ "> Or do not store the name and nr-clks in domain0?\n" "\n" "The f/e directory should contain the identifier which the frontend needs to\n" - "=C2=A0do the lookup from the firmware tables into pvclk space -- i.e. the n=\n" - "ame\n" + "\302\240do the lookup from the firmware tables into pvclk space -- i.e. the name\n" "given in the f/e device tree node (maybe call it dt-name?).\n" "\n" "The b/e directory should contain the name of the clock from the backend\n" @@ -230,4 +221,4 @@ "\n" Ian. -8e1c3f43752bef68e02e1c7c8622999cc3fdb6a9f2764372a074094fd3012f6a +ecae96768f4618bbfdc76e14e8c1c32c76962cadeaa368e25d4ddf8d4dcdfbe8
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.