| Message ID | 1487175261-7051-5-git-send-email-atull@kernel.org (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
hi Alan On 2/15/2017 10:14 AM, Alan Tull wrote: > Document that getting a reference to a FPGA Manager has been > separated from locking the FPGA Mangager for use. > > fpga_mgr_lock/unlock functions get/release mutex. > > of_fpga_mgr_get, fpga_mgr_get, and fpga_mgr_put no longer lock > the FPGA manager mutex. > > This makes it more straigtforward to save a reference to > a FPGA manager and only attempting to lock it when programming > the FPGA. New to the FPGA world, but I like the idea of shorter lock. Separating the lock from fpga_mgr_get will give underline FPGA device drivers more flexibility to acquire the mgr pointer. One newbie question, since the underline FPGA device driver does the fpga_mgr_register during probe, each manager instance belongs to that FPGA device only. What's the use to keep tracking the usage reference with fpga_mgr_put/get function, or is it enough to increase/decrease dev reference count in fpga_mgr_register/unregister function? Thanks, Yi > > Signed-off-by: Alan Tull <atull@kernel.org> > --- > Documentation/fpga/fpga-mgr.txt | 19 ++++++++++++++++++- > 1 file changed, 18 insertions(+), 1 deletion(-) > > diff --git a/Documentation/fpga/fpga-mgr.txt b/Documentation/fpga/fpga-mgr.txt > index 78f197f..06d5d5b 100644 > --- a/Documentation/fpga/fpga-mgr.txt > +++ b/Documentation/fpga/fpga-mgr.txt > @@ -53,13 +53,26 @@ To get/put a reference to a FPGA manager: > struct fpga_manager *of_fpga_mgr_get(struct device_node *node); > struct fpga_manager *fpga_mgr_get(struct device *dev); > > -Given a DT node or device, get an exclusive reference to a FPGA manager. > +Given a DT node or device, get an reference to a FPGA manager. Pointer > +can be saved until you are ready to program the FPGA. > > void fpga_mgr_put(struct fpga_manager *mgr); > > Release the reference. > > > +To get exclusive control of a FPGA manager: > +------------------------------------------- > + > + int fpga_mgr_lock(struct fpga_magager *mgr); > + > +Call fpga_mgr_lock and verify that it returns 0 before attempting to > +program the FPGA. > + > + void fpga_mgr_unlock(struct fpga_magager *mgr); > + > +Call fpga_mgr_unlock when done programming the FPGA. > + > To register or unregister the low level FPGA-specific driver: > ------------------------------------------------------------- > > @@ -95,11 +108,13 @@ int ret; > > /* Get exclusive control of FPGA manager */ > struct fpga_manager *mgr = of_fpga_mgr_get(mgr_node); > +ret = fpga_mgr_lock(mgr); > > /* Load the buffer to the FPGA */ > ret = fpga_mgr_buf_load(mgr, &info, buf, count); > > /* Release the FPGA manager */ > +fpga_mgr_unlock(mgr); > fpga_mgr_put(mgr); > > > @@ -124,11 +139,13 @@ int ret; > > /* Get exclusive control of FPGA manager */ > struct fpga_manager *mgr = of_fpga_mgr_get(mgr_node); > +ret = fpga_mgr_lock(mgr); > > /* Get the firmware image (path) and load it to the FPGA */ > ret = fpga_mgr_firmware_load(mgr, &info, path); > > /* Release the FPGA manager */ > +fpga_mgr_unlock(mgr); > fpga_mgr_put(mgr); > > -- To unsubscribe from this list: send the line "unsubscribe linux-fpga" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Alan, small nits inline On Wed, Feb 15, 2017 at 8:14 AM, Alan Tull <atull@kernel.org> wrote: > Document that getting a reference to a FPGA Manager has been > separated from locking the FPGA Mangager for use. > > fpga_mgr_lock/unlock functions get/release mutex. > > of_fpga_mgr_get, fpga_mgr_get, and fpga_mgr_put no longer lock > the FPGA manager mutex. > > This makes it more straigtforward to save a reference to > a FPGA manager and only attempting to lock it when programming > the FPGA. > > Signed-off-by: Alan Tull <atull@kernel.org> Acked-by: Moritz Fischer <mdf@kernel.org> > --- > Documentation/fpga/fpga-mgr.txt | 19 ++++++++++++++++++- > 1 file changed, 18 insertions(+), 1 deletion(-) > > diff --git a/Documentation/fpga/fpga-mgr.txt b/Documentation/fpga/fpga-mgr.txt > index 78f197f..06d5d5b 100644 > --- a/Documentation/fpga/fpga-mgr.txt > +++ b/Documentation/fpga/fpga-mgr.txt > @@ -53,13 +53,26 @@ To get/put a reference to a FPGA manager: > struct fpga_manager *of_fpga_mgr_get(struct device_node *node); > struct fpga_manager *fpga_mgr_get(struct device *dev); > > -Given a DT node or device, get an exclusive reference to a FPGA manager. > +Given a DT node or device, get an reference to a FPGA manager. Pointer Nits: get *a* reference, 'A' pointer or 'The' pointer. > +can be saved until you are ready to program the FPGA. > > void fpga_mgr_put(struct fpga_manager *mgr); > > Release the reference. > > > +To get exclusive control of a FPGA manager: > +------------------------------------------- > + > + int fpga_mgr_lock(struct fpga_magager *mgr); > + > +Call fpga_mgr_lock and verify that it returns 0 before attempting to > +program the FPGA. > + > + void fpga_mgr_unlock(struct fpga_magager *mgr); > + > +Call fpga_mgr_unlock when done programming the FPGA. > + > To register or unregister the low level FPGA-specific driver: > ------------------------------------------------------------- > > @@ -95,11 +108,13 @@ int ret; > > /* Get exclusive control of FPGA manager */ > struct fpga_manager *mgr = of_fpga_mgr_get(mgr_node); > +ret = fpga_mgr_lock(mgr); > > /* Load the buffer to the FPGA */ > ret = fpga_mgr_buf_load(mgr, &info, buf, count); > > /* Release the FPGA manager */ > +fpga_mgr_unlock(mgr); > fpga_mgr_put(mgr); > > > @@ -124,11 +139,13 @@ int ret; > > /* Get exclusive control of FPGA manager */ > struct fpga_manager *mgr = of_fpga_mgr_get(mgr_node); > +ret = fpga_mgr_lock(mgr); > > /* Get the firmware image (path) and load it to the FPGA */ > ret = fpga_mgr_firmware_load(mgr, &info, path); > > /* Release the FPGA manager */ > +fpga_mgr_unlock(mgr); > fpga_mgr_put(mgr); > > > -- > 2.7.4 > Cheers, Moritz -- To unsubscribe from this list: send the line "unsubscribe linux-fpga" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Feb 17, 2017 at 11:14 AM, Li, Yi <yi1.li@linux.intel.com> wrote: > hi Alan > > > On 2/15/2017 10:14 AM, Alan Tull wrote: >> >> Document that getting a reference to a FPGA Manager has been >> separated from locking the FPGA Mangager for use. >> >> fpga_mgr_lock/unlock functions get/release mutex. >> >> of_fpga_mgr_get, fpga_mgr_get, and fpga_mgr_put no longer lock >> the FPGA manager mutex. >> >> This makes it more straigtforward to save a reference to >> a FPGA manager and only attempting to lock it when programming >> the FPGA. > > > New to the FPGA world, but I like the idea of shorter lock. Separating the > lock from fpga_mgr_get will give underline FPGA device drivers more > flexibility to acquire the mgr pointer. > One newbie question, since the underline FPGA device driver does the > fpga_mgr_register during probe, each manager instance belongs to that FPGA > device only. What's the use to keep tracking the usage reference with > fpga_mgr_put/get function, or is it enough to increase/decrease dev > reference count in fpga_mgr_register/unregister function? Hi Yi, That's a good though but the code that creates the fpga mgr device might not be the code that needs to hold a reference to it. The device tree implementation is an example of this. The fpga mgr devices are created completely separately from the fpga regions. Alan > > Thanks, > Yi > >> >> Signed-off-by: Alan Tull <atull@kernel.org> >> --- >> Documentation/fpga/fpga-mgr.txt | 19 ++++++++++++++++++- >> 1 file changed, 18 insertions(+), 1 deletion(-) >> >> diff --git a/Documentation/fpga/fpga-mgr.txt >> b/Documentation/fpga/fpga-mgr.txt >> index 78f197f..06d5d5b 100644 >> --- a/Documentation/fpga/fpga-mgr.txt >> +++ b/Documentation/fpga/fpga-mgr.txt >> @@ -53,13 +53,26 @@ To get/put a reference to a FPGA manager: >> struct fpga_manager *of_fpga_mgr_get(struct device_node *node); >> struct fpga_manager *fpga_mgr_get(struct device *dev); >> -Given a DT node or device, get an exclusive reference to a FPGA >> manager. >> +Given a DT node or device, get an reference to a FPGA manager. Pointer >> +can be saved until you are ready to program the FPGA. >> void fpga_mgr_put(struct fpga_manager *mgr); >> Release the reference. >> +To get exclusive control of a FPGA manager: >> +------------------------------------------- >> + >> + int fpga_mgr_lock(struct fpga_magager *mgr); >> + >> +Call fpga_mgr_lock and verify that it returns 0 before attempting to >> +program the FPGA. >> + >> + void fpga_mgr_unlock(struct fpga_magager *mgr); >> + >> +Call fpga_mgr_unlock when done programming the FPGA. >> + >> To register or unregister the low level FPGA-specific driver: >> ------------------------------------------------------------- >> @@ -95,11 +108,13 @@ int ret; >> /* Get exclusive control of FPGA manager */ >> struct fpga_manager *mgr = of_fpga_mgr_get(mgr_node); >> +ret = fpga_mgr_lock(mgr); >> /* Load the buffer to the FPGA */ >> ret = fpga_mgr_buf_load(mgr, &info, buf, count); >> /* Release the FPGA manager */ >> +fpga_mgr_unlock(mgr); >> fpga_mgr_put(mgr); >> @@ -124,11 +139,13 @@ int ret; >> /* Get exclusive control of FPGA manager */ >> struct fpga_manager *mgr = of_fpga_mgr_get(mgr_node); >> +ret = fpga_mgr_lock(mgr); >> /* Get the firmware image (path) and load it to the FPGA */ >> ret = fpga_mgr_firmware_load(mgr, &info, path); >> /* Release the FPGA manager */ >> +fpga_mgr_unlock(mgr); >> fpga_mgr_put(mgr); >> > > -- To unsubscribe from this list: send the line "unsubscribe linux-fpga" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Feb 17, 2017 at 11:52 AM, Moritz Fischer <mdf@kernel.org> wrote: > Alan, > > small nits inline Hi Moritz, Thanks for the review! Alan > > On Wed, Feb 15, 2017 at 8:14 AM, Alan Tull <atull@kernel.org> wrote: >> Document that getting a reference to a FPGA Manager has been >> separated from locking the FPGA Mangager for use. >> >> fpga_mgr_lock/unlock functions get/release mutex. >> >> of_fpga_mgr_get, fpga_mgr_get, and fpga_mgr_put no longer lock >> the FPGA manager mutex. >> >> This makes it more straigtforward to save a reference to >> a FPGA manager and only attempting to lock it when programming >> the FPGA. >> >> Signed-off-by: Alan Tull <atull@kernel.org> > Acked-by: Moritz Fischer <mdf@kernel.org> > >> --- >> Documentation/fpga/fpga-mgr.txt | 19 ++++++++++++++++++- >> 1 file changed, 18 insertions(+), 1 deletion(-) >> >> diff --git a/Documentation/fpga/fpga-mgr.txt b/Documentation/fpga/fpga-mgr.txt >> index 78f197f..06d5d5b 100644 >> --- a/Documentation/fpga/fpga-mgr.txt >> +++ b/Documentation/fpga/fpga-mgr.txt >> @@ -53,13 +53,26 @@ To get/put a reference to a FPGA manager: >> struct fpga_manager *of_fpga_mgr_get(struct device_node *node); >> struct fpga_manager *fpga_mgr_get(struct device *dev); >> >> -Given a DT node or device, get an exclusive reference to a FPGA manager. >> +Given a DT node or device, get an reference to a FPGA manager. Pointer > > Nits: get *a* reference, 'A' pointer or 'The' pointer. > >> +can be saved until you are ready to program the FPGA. >> >> void fpga_mgr_put(struct fpga_manager *mgr); >> >> Release the reference. >> >> >> +To get exclusive control of a FPGA manager: >> +------------------------------------------- >> + >> + int fpga_mgr_lock(struct fpga_magager *mgr); >> + >> +Call fpga_mgr_lock and verify that it returns 0 before attempting to >> +program the FPGA. >> + >> + void fpga_mgr_unlock(struct fpga_magager *mgr); >> + >> +Call fpga_mgr_unlock when done programming the FPGA. >> + >> To register or unregister the low level FPGA-specific driver: >> ------------------------------------------------------------- >> >> @@ -95,11 +108,13 @@ int ret; >> >> /* Get exclusive control of FPGA manager */ >> struct fpga_manager *mgr = of_fpga_mgr_get(mgr_node); >> +ret = fpga_mgr_lock(mgr); >> >> /* Load the buffer to the FPGA */ >> ret = fpga_mgr_buf_load(mgr, &info, buf, count); >> >> /* Release the FPGA manager */ >> +fpga_mgr_unlock(mgr); >> fpga_mgr_put(mgr); >> >> >> @@ -124,11 +139,13 @@ int ret; >> >> /* Get exclusive control of FPGA manager */ >> struct fpga_manager *mgr = of_fpga_mgr_get(mgr_node); >> +ret = fpga_mgr_lock(mgr); >> >> /* Get the firmware image (path) and load it to the FPGA */ >> ret = fpga_mgr_firmware_load(mgr, &info, path); >> >> /* Release the FPGA manager */ >> +fpga_mgr_unlock(mgr); >> fpga_mgr_put(mgr); >> >> >> -- >> 2.7.4 >> > > Cheers, > > Moritz -- To unsubscribe from this list: send the line "unsubscribe linux-fpga" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/fpga/fpga-mgr.txt b/Documentation/fpga/fpga-mgr.txt index 78f197f..06d5d5b 100644 --- a/Documentation/fpga/fpga-mgr.txt +++ b/Documentation/fpga/fpga-mgr.txt @@ -53,13 +53,26 @@ To get/put a reference to a FPGA manager: struct fpga_manager *of_fpga_mgr_get(struct device_node *node); struct fpga_manager *fpga_mgr_get(struct device *dev); -Given a DT node or device, get an exclusive reference to a FPGA manager. +Given a DT node or device, get an reference to a FPGA manager. Pointer +can be saved until you are ready to program the FPGA. void fpga_mgr_put(struct fpga_manager *mgr); Release the reference. +To get exclusive control of a FPGA manager: +------------------------------------------- + + int fpga_mgr_lock(struct fpga_magager *mgr); + +Call fpga_mgr_lock and verify that it returns 0 before attempting to +program the FPGA. + + void fpga_mgr_unlock(struct fpga_magager *mgr); + +Call fpga_mgr_unlock when done programming the FPGA. + To register or unregister the low level FPGA-specific driver: ------------------------------------------------------------- @@ -95,11 +108,13 @@ int ret; /* Get exclusive control of FPGA manager */ struct fpga_manager *mgr = of_fpga_mgr_get(mgr_node); +ret = fpga_mgr_lock(mgr); /* Load the buffer to the FPGA */ ret = fpga_mgr_buf_load(mgr, &info, buf, count); /* Release the FPGA manager */ +fpga_mgr_unlock(mgr); fpga_mgr_put(mgr); @@ -124,11 +139,13 @@ int ret; /* Get exclusive control of FPGA manager */ struct fpga_manager *mgr = of_fpga_mgr_get(mgr_node); +ret = fpga_mgr_lock(mgr); /* Get the firmware image (path) and load it to the FPGA */ ret = fpga_mgr_firmware_load(mgr, &info, path); /* Release the FPGA manager */ +fpga_mgr_unlock(mgr); fpga_mgr_put(mgr);
Document that getting a reference to a FPGA Manager has been separated from locking the FPGA Mangager for use. fpga_mgr_lock/unlock functions get/release mutex. of_fpga_mgr_get, fpga_mgr_get, and fpga_mgr_put no longer lock the FPGA manager mutex. This makes it more straigtforward to save a reference to a FPGA manager and only attempting to lock it when programming the FPGA. Signed-off-by: Alan Tull <atull@kernel.org> --- Documentation/fpga/fpga-mgr.txt | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)