All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alan Tull <atull@kernel.org>
To: Eric.Delage@zodiacaerospace.com
Cc: Jonathan Corbet <corbet@lwn.net>,
	Federico Vaga <federico.vaga@cern.ch>,
	Linux Doc Mailing List <linux-doc@vger.kernel.org>,
	linux-fpga@vger.kernel.org, linux-fpga-owner@vger.kernel.org,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Moritz Fischer <mdf@kernel.org>,
	Randy Dunlap <rdunlap@infradead.org>
Subject: Re: [PATCH v3 2/4] fpga: bridge: add devm_fpga_bridge_create
Date: Wed, 26 Sep 2018 15:13:52 -0500	[thread overview]
Message-ID: <CANk1AXTOqQQ5QycZiKpw=Rv8dY9njbah4bN-xb9Ty9UCONrKyw@mail.gmail.com> (raw)
In-Reply-To: <OFE9B73D95.73C2CEF2-ONC1258314.005AEBC6-C1258314.005C6433@zodiacaerospace.com>


[-- Attachment #1.1: Type: text/plain, Size: 14487 bytes --]

On Wed, Sep 26, 2018 at 12:21 PM <Eric.Delage@zodiacaerospace.com> wrote:

Hi Eric,

Est-ce que je comprends bien qu'il ne s'agit que d'une carte de traitement
> FPGA sans possibilité de mettre une carte-fille type FMA (à la REFLEX CES
> avec sa carte accélératrice FPGA) ?
>

IIUC your question is about specific boards.  These changes I posted are
for a subsystem with support for various FPGA devices.  This is not code
that is specific to any one board.

We can have a better discussion if we could stick to English, please.

Alan


> 4 XCKU115 pour les russes equiv. 2 cartes FPGA Xilinx avec des VU37P. Env.
> de développement SDAccel identique mais fait plutôt pour être utilisé en
> accélérateur.
> L'approche carte LB2P1/XCKU040-060 + carte FPGA est amha à investiguer
> sérieusement.
>
> Kind Regards,
> Mit freundlichen Grüssen,
> Sincères Salutations,
>
> *ZODIAC DATA SYSTEMS ZODIAC AEROSYSTEMS*
> Control Systems Division [image: logo]
>
> * Eric DELAGE*
> Head of the Monitoring Front-End Group, and
> Technical Product Manager for IFoIP and RF2P(CIe) Solutions
>
> Campus EffiScience - 5 Esplanade Anton Philips - 14460 Colombelles - France
> Tel: +33.2.31.29.49.45 - Fax: +33.2.31.80.65.49
> eric.delage@zodiacaerospace.com <Eric.DELAGE@ZodiacAerospace.Com>
> http://www.zodiacaerospace.com
> http://www.linkedin.com/in/ericdelage
>
>
>
>
> De :        Alan Tull <atull@kernel.org>
> A :        Moritz Fischer <mdf@kernel.org>, Jonathan Corbet <
> corbet@lwn.net>
> Cc :        Randy Dunlap <rdunlap@infradead.org>, Federico Vaga <
> federico.vaga@cern.ch>, linux-kernel@vger.kernel.org,
> linux-fpga@vger.kernel.org, linux-doc@vger.kernel.org, Alan Tull <
> atull@kernel.org>
> Date :        26/09/2018 18:13
> Objet :        [PATCH v3 2/4] fpga: bridge: add devm_fpga_bridge_create
> Envoyé par :        linux-fpga-owner@vger.kernel.org
> ------------------------------
>
>
>
> Add devm_fpga_bridge_create() which is the managed
> version of fpga_bridge_create().
>
> Change current bridge drivers to use
> devm_fpga_bridge_create().
>
> Signed-off-by: Alan Tull <atull@kernel.org>
> Suggested-by: Federico Vaga <federico.vaga@cern.ch>
> Acked-by: Moritz Fischer <mdf@kernel.org>
> ---
> v2: add suggested-by
> v3: remove some unclear documentation about fpga_bridge_unregister
>    add Acked-by
> ---
> Documentation/driver-api/fpga/fpga-bridge.rst |  3 ++
> drivers/fpga/altera-fpga2sdram.c              |  8 ++--
> drivers/fpga/altera-freeze-bridge.c           | 13 ++---
> drivers/fpga/altera-hps2fpga.c                |  7 ++-
> drivers/fpga/dfl-fme-br.c                     | 11 ++---
> drivers/fpga/fpga-bridge.c                    | 68
> +++++++++++++++++++++++----
> drivers/fpga/xilinx-pr-decoupler.c            |  4 +-
> include/linux/fpga/fpga-bridge.h              |  4 ++
> 8 files changed, 80 insertions(+), 38 deletions(-)
>
> diff --git a/Documentation/driver-api/fpga/fpga-bridge.rst
> b/Documentation/driver-api/fpga/fpga-bridge.rst
> index 2c2aaca..ebbcbde 100644
> --- a/Documentation/driver-api/fpga/fpga-bridge.rst
> +++ b/Documentation/driver-api/fpga/fpga-bridge.rst
> @@ -11,6 +11,9 @@ API to implement a new FPGA bridge
>    :functions: fpga_bridge_ops
>
> .. kernel-doc:: drivers/fpga/fpga-bridge.c
> +   :functions: devm_fpga_bridge_create
> +
> +.. kernel-doc:: drivers/fpga/fpga-bridge.c
>    :functions: fpga_bridge_create
>
> .. kernel-doc:: drivers/fpga/fpga-bridge.c
> diff --git a/drivers/fpga/altera-fpga2sdram.c
> b/drivers/fpga/altera-fpga2sdram.c
> index 23660cc..a78e49c 100644
> --- a/drivers/fpga/altera-fpga2sdram.c
> +++ b/drivers/fpga/altera-fpga2sdram.c
> @@ -121,18 +121,16 @@ static int alt_fpga_bridge_probe(struct
> platform_device *pdev)
>                  /* Get f2s bridge configuration saved in handoff register
> */
>                  regmap_read(sysmgr, SYSMGR_ISWGRP_HANDOFF3, &priv->mask);
>
> -                 br = fpga_bridge_create(dev, F2S_BRIDGE_NAME,
> -
>  &altera_fpga2sdram_br_ops, priv);
> +                 br = devm_fpga_bridge_create(dev, F2S_BRIDGE_NAME,
> +
> &altera_fpga2sdram_br_ops, priv);
>                  if (!br)
>                                   return -ENOMEM;
>
>                  platform_set_drvdata(pdev, br);
>
>                  ret = fpga_bridge_register(br);
> -                 if (ret) {
> -                                  fpga_bridge_free(br);
> +                 if (ret)
>                                   return ret;
> -                 }
>
>                  dev_info(dev, "driver initialized with handoff %08x\n",
> priv->mask);
>
> diff --git a/drivers/fpga/altera-freeze-bridge.c
> b/drivers/fpga/altera-freeze-bridge.c
> index ffd586c..dd58c4a 100644
> --- a/drivers/fpga/altera-freeze-bridge.c
> +++ b/drivers/fpga/altera-freeze-bridge.c
> @@ -213,7 +213,6 @@ static int altera_freeze_br_probe(struct
> platform_device *pdev)
>                  struct fpga_bridge *br;
>                  struct resource *res;
>                  u32 status, revision;
> -                 int ret;
>
>                  if (!np)
>                                   return -ENODEV;
> @@ -245,20 +244,14 @@ static int altera_freeze_br_probe(struct
> platform_device *pdev)
>
>                  priv->base_addr = base_addr;
>
> -                 br = fpga_bridge_create(dev, FREEZE_BRIDGE_NAME,
> -
>  &altera_freeze_br_br_ops, priv);
> +                 br = devm_fpga_bridge_create(dev, FREEZE_BRIDGE_NAME,
> +
> &altera_freeze_br_br_ops, priv);
>                  if (!br)
>                                   return -ENOMEM;
>
>                  platform_set_drvdata(pdev, br);
>
> -                 ret = fpga_bridge_register(br);
> -                 if (ret) {
> -                                  fpga_bridge_free(br);
> -                                  return ret;
> -                 }
> -
> -                 return 0;
> +                 return fpga_bridge_register(br);
> }
>
> static int altera_freeze_br_remove(struct platform_device *pdev)
> diff --git a/drivers/fpga/altera-hps2fpga.c
> b/drivers/fpga/altera-hps2fpga.c
> index a974d3f..77b95f2 100644
> --- a/drivers/fpga/altera-hps2fpga.c
> +++ b/drivers/fpga/altera-hps2fpga.c
> @@ -180,7 +180,8 @@ static int alt_fpga_bridge_probe(struct
> platform_device *pdev)
>                                   }
>                  }
>
> -                 br = fpga_bridge_create(dev, priv->name,
> &altera_hps2fpga_br_ops, priv);
> +                 br = devm_fpga_bridge_create(dev, priv->name,
> +
> &altera_hps2fpga_br_ops, priv);
>                  if (!br) {
>                                   ret = -ENOMEM;
>                                   goto err;
> @@ -190,12 +191,10 @@ static int alt_fpga_bridge_probe(struct
> platform_device *pdev)
>
>                  ret = fpga_bridge_register(br);
>                  if (ret)
> -                                  goto err_free;
> +                                  goto err;
>
>                  return 0;
>
> -err_free:
> -                 fpga_bridge_free(br);
> err:
>                  clk_disable_unprepare(priv->clk);
>
> diff --git a/drivers/fpga/dfl-fme-br.c b/drivers/fpga/dfl-fme-br.c
> index 7cc041d..3ff9f3a 100644
> --- a/drivers/fpga/dfl-fme-br.c
> +++ b/drivers/fpga/dfl-fme-br.c
> @@ -61,7 +61,6 @@ static int fme_br_probe(struct platform_device *pdev)
>                  struct device *dev = &pdev->dev;
>                  struct fme_br_priv *priv;
>                  struct fpga_bridge *br;
> -                 int ret;
>
>                  priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>                  if (!priv)
> @@ -69,18 +68,14 @@ static int fme_br_probe(struct platform_device *pdev)
>
>                  priv->pdata = dev_get_platdata(dev);
>
> -                 br = fpga_bridge_create(dev, "DFL FPGA FME Bridge",
> -
>  &fme_bridge_ops, priv);
> +                 br = devm_fpga_bridge_create(dev, "DFL FPGA FME Bridge",
> +
> &fme_bridge_ops, priv);
>                  if (!br)
>                                   return -ENOMEM;
>
>                  platform_set_drvdata(pdev, br);
>
> -                 ret = fpga_bridge_register(br);
> -                 if (ret)
> -                                  fpga_bridge_free(br);
> -
> -                 return ret;
> +                 return fpga_bridge_register(br);
> }
>
> static int fme_br_remove(struct platform_device *pdev)
> diff --git a/drivers/fpga/fpga-bridge.c b/drivers/fpga/fpga-bridge.c
> index c983dac..80bd8f1 100644
> --- a/drivers/fpga/fpga-bridge.c
> +++ b/drivers/fpga/fpga-bridge.c
> @@ -324,6 +324,9 @@ ATTRIBUTE_GROUPS(fpga_bridge);
>  * @br_ops:                 pointer to structure of fpga bridge ops
>  * @priv:                 FPGA bridge private data
>  *
> + * The caller of this function is responsible for freeing the bridge with
> + * fpga_bridge_free().  Using devm_fpga_bridge_create() instead is
> recommended.
> + *
>  * Return: struct fpga_bridge or NULL
>  */
> struct fpga_bridge *fpga_bridge_create(struct device *dev, const char
> *name,
> @@ -378,8 +381,8 @@ struct fpga_bridge *fpga_bridge_create(struct device
> *dev, const char *name,
> EXPORT_SYMBOL_GPL(fpga_bridge_create);
>
> /**
> - * fpga_bridge_free - free a fpga bridge and its id
> - * @bridge:                 FPGA bridge struct created by
> fpga_bridge_create
> + * fpga_bridge_free - free a fpga bridge created by fpga_bridge_create()
> + * @bridge:                 FPGA bridge struct
>  */
> void fpga_bridge_free(struct fpga_bridge *bridge)
> {
> @@ -388,9 +391,56 @@ void fpga_bridge_free(struct fpga_bridge *bridge)
> }
> EXPORT_SYMBOL_GPL(fpga_bridge_free);
>
> +static void devm_fpga_bridge_release(struct device *dev, void *res)
> +{
> +                 struct fpga_bridge *bridge = *(struct fpga_bridge **)res;
> +
> +                 fpga_bridge_free(bridge);
> +}
> +
> /**
> - * fpga_bridge_register - register a fpga bridge
> - * @bridge:                 FPGA bridge struct created by
> fpga_bridge_create
> + * devm_fpga_bridge_create - create and init a managed struct fpga_bridge
> + * @dev:                 FPGA bridge device from pdev
> + * @name:                 FPGA bridge name
> + * @br_ops:                 pointer to structure of fpga bridge ops
> + * @priv:                 FPGA bridge private data
> + *
> + * This function is intended for use in a FPGA bridge driver's probe
> function.
> + * After the bridge driver creates the struct with
> devm_fpga_bridge_create(), it
> + * should register the bridge with fpga_bridge_register().  The bridge
> driver's
> + * remove function should call fpga_bridge_unregister().  The bridge
> struct
> + * allocated with this function will be freed automatically on driver
> detach.
> + * This includes the case of a probe function returning error before
> calling
> + * fpga_bridge_register(), the struct will still get cleaned up.
> + *
> + *  Return: struct fpga_bridge or NULL
> + */
> +struct fpga_bridge
> +*devm_fpga_bridge_create(struct device *dev, const char *name,
> +                                                    const struct
> fpga_bridge_ops *br_ops, void *priv)
> +{
> +                 struct fpga_bridge **ptr, *bridge;
> +
> +                 ptr = devres_alloc(devm_fpga_bridge_release,
> sizeof(*ptr), GFP_KERNEL);
> +                 if (!ptr)
> +                                  return NULL;
> +
> +                 bridge = fpga_bridge_create(dev, name, br_ops, priv);
> +                 if (!bridge) {
> +                                  devres_free(ptr);
> +                 } else {
> +                                  *ptr = bridge;
> +                                  devres_add(dev, ptr);
> +                 }
> +
> +                 return bridge;
> +}
> +EXPORT_SYMBOL_GPL(devm_fpga_bridge_create);
> +
> +/**
> + * fpga_bridge_register - register a FPGA bridge
> + *
> + * @bridge: FPGA bridge struct
>  *
>  * Return: 0 for success, error code otherwise.
>  */
> @@ -412,8 +462,11 @@ int fpga_bridge_register(struct fpga_bridge *bridge)
> EXPORT_SYMBOL_GPL(fpga_bridge_register);
>
> /**
> - * fpga_bridge_unregister - unregister and free a fpga bridge
> - * @bridge:                 FPGA bridge struct created by
> fpga_bridge_create
> + * fpga_bridge_unregister - unregister a FPGA bridge
> + *
> + * @bridge: FPGA bridge struct
> + *
> + * This function is intended for use in a FPGA bridge driver's remove
> function.
>  */
> void fpga_bridge_unregister(struct fpga_bridge *bridge)
> {
> @@ -430,9 +483,6 @@ EXPORT_SYMBOL_GPL(fpga_bridge_unregister);
>
> static void fpga_bridge_dev_release(struct device *dev)
> {
> -                 struct fpga_bridge *bridge = to_fpga_bridge(dev);
> -
> -                 fpga_bridge_free(bridge);
> }
>
> static int __init fpga_bridge_dev_init(void)
> diff --git a/drivers/fpga/xilinx-pr-decoupler.c
> b/drivers/fpga/xilinx-pr-decoupler.c
> index 07ba153..6410361 100644
> --- a/drivers/fpga/xilinx-pr-decoupler.c
> +++ b/drivers/fpga/xilinx-pr-decoupler.c
> @@ -121,8 +121,8 @@ static int xlnx_pr_decoupler_probe(struct
> platform_device *pdev)
>
>                  clk_disable(priv->clk);
>
> -                 br = fpga_bridge_create(&pdev->dev, "Xilinx PR
> Decoupler",
> -
>  &xlnx_pr_decoupler_br_ops, priv);
> +                 br = devm_fpga_bridge_create(&pdev->dev, "Xilinx PR
> Decoupler",
> +
> &xlnx_pr_decoupler_br_ops, priv);
>                  if (!br) {
>                                   err = -ENOMEM;
>                                   goto err_clk;
> diff --git a/include/linux/fpga/fpga-bridge.h
> b/include/linux/fpga/fpga-bridge.h
> index ce550fc..817600a 100644
> --- a/include/linux/fpga/fpga-bridge.h
> +++ b/include/linux/fpga/fpga-bridge.h
> @@ -69,4 +69,8 @@ void fpga_bridge_free(struct fpga_bridge *br);
> int fpga_bridge_register(struct fpga_bridge *br);
> void fpga_bridge_unregister(struct fpga_bridge *br);
>
> +struct fpga_bridge
> +*devm_fpga_bridge_create(struct device *dev, const char *name,
> +                                                    const struct
> fpga_bridge_ops *br_ops, void *priv);
> +
> #endif /* _LINUX_FPGA_BRIDGE_H */
> --
> 2.7.4
>
>
>

[-- Attachment #1.2: Type: text/html, Size: 21463 bytes --]

[-- Attachment #2: noname --]
[-- Type: image/jpeg, Size: 4067 bytes --]

  reply	other threads:[~2018-09-26 20:13 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-26 16:12 [PATCH v3 0/4] fpga: add devm managed create APIs Alan Tull
2018-09-26 16:12 ` [PATCH v3 1/4] fpga: mgr: add devm_fpga_mgr_create Alan Tull
2018-10-15 14:54   ` Alan Tull
2018-10-15 17:32     ` Moritz Fischer
2018-10-16  7:02     ` Federico Vaga
2018-10-16  7:02       ` Federico Vaga
2018-09-26 16:12 ` [PATCH v3 2/4] fpga: bridge: add devm_fpga_bridge_create Alan Tull
2018-09-26 16:49   ` Eric.Delage
2018-09-26 20:13     ` Alan Tull [this message]
2018-09-26 20:31       ` Eric.Delage
2018-09-26 16:12 ` [PATCH v3 3/4] fpga: add devm_fpga_region_create Alan Tull
2018-10-15 20:10   ` Moritz Fischer
2018-09-26 16:12 ` [PATCH v3 4/4] docs: fpga: document programming fpgas using regions Alan Tull
2018-10-15 20:11   ` Moritz Fischer
2018-10-15 20:40     ` Alan Tull

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CANk1AXTOqQQ5QycZiKpw=Rv8dY9njbah4bN-xb9Ty9UCONrKyw@mail.gmail.com' \
    --to=atull@kernel.org \
    --cc=Eric.Delage@zodiacaerospace.com \
    --cc=corbet@lwn.net \
    --cc=federico.vaga@cern.ch \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-fpga-owner@vger.kernel.org \
    --cc=linux-fpga@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mdf@kernel.org \
    --cc=rdunlap@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.