From: Moritz Fischer <mdf@kernel.org> To: Tom Rix <trix@redhat.com> Cc: Moritz Fischer <mdf@kernel.org>, linux-fpga@vger.kernel.org, hao.wu@intel.com, michal.simek@xilinx.com, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, russell.h.weight@intel.com, matthew.gerlach@intel.com Subject: Re: [PATCH 10/10] fpga: fpga-mgr: altera-pr-ip: Simplify registration Date: Sun, 4 Oct 2020 16:39:14 -0700 [thread overview] Message-ID: <20201004233914.GA111357@epycbox.lan> (raw) In-Reply-To: <a49b1d7c-9756-1059-f7a1-25dae460d659@redhat.com> On Sun, Oct 04, 2020 at 11:47:26AM -0700, Tom Rix wrote: > > On 10/3/20 10:14 PM, Moritz Fischer wrote: > > Simplify registration using new devm_fpga_mgr_register() API. > > Remove the now obsolete altera_pr_unregister() function. > > > > Signed-off-by: Moritz Fischer <mdf@kernel.org> > > --- > > > > We should take another look at this, IIRC correctly the point of > > splitting this up into a separate driver was to make it useable by a > > different (pci?) driver later on. > > > > It doesn't seem like this happened, and I think we should just make this > > a platform driver? > > > > --- > > drivers/fpga/altera-pr-ip-core-plat.c | 10 ---------- > > drivers/fpga/altera-pr-ip-core.c | 14 +------------- > > include/linux/fpga/altera-pr-ip-core.h | 1 - > > 3 files changed, 1 insertion(+), 24 deletions(-) > > > > diff --git a/drivers/fpga/altera-pr-ip-core-plat.c b/drivers/fpga/altera-pr-ip-core-plat.c > > index 99b9cc0e70f0..b008a6b8d2d3 100644 > > --- a/drivers/fpga/altera-pr-ip-core-plat.c > > +++ b/drivers/fpga/altera-pr-ip-core-plat.c > > @@ -28,15 +28,6 @@ static int alt_pr_platform_probe(struct platform_device *pdev) > > return alt_pr_register(dev, reg_base); > > } > > > > -static int alt_pr_platform_remove(struct platform_device *pdev) > > -{ > > - struct device *dev = &pdev->dev; > > - > > - alt_pr_unregister(dev); > > - > > - return 0; > > -} > > - > > static const struct of_device_id alt_pr_of_match[] = { > > { .compatible = "altr,a10-pr-ip", }, > > {}, > > @@ -46,7 +37,6 @@ MODULE_DEVICE_TABLE(of, alt_pr_of_match); > > > > static struct platform_driver alt_pr_platform_driver = { > > .probe = alt_pr_platform_probe, > > - .remove = alt_pr_platform_remove, > > .driver = { > > .name = "alt_a10_pr_ip", > > .of_match_table = alt_pr_of_match, > > diff --git a/drivers/fpga/altera-pr-ip-core.c b/drivers/fpga/altera-pr-ip-core.c > > index 2cf25fd5e897..dfdf21ed34c4 100644 > > --- a/drivers/fpga/altera-pr-ip-core.c > > +++ b/drivers/fpga/altera-pr-ip-core.c > > @@ -195,22 +195,10 @@ int alt_pr_register(struct device *dev, void __iomem *reg_base) > > if (!mgr) > > return -ENOMEM; > > > > - dev_set_drvdata(dev, mgr); > > - > > - return fpga_mgr_register(mgr); > > + return devm_fpga_mgr_register(dev, mgr); > > } > > EXPORT_SYMBOL_GPL(alt_pr_register); > > > > -void alt_pr_unregister(struct device *dev) > > -{ > > - struct fpga_manager *mgr = dev_get_drvdata(dev); > > - > > - dev_dbg(dev, "%s\n", __func__); > > - > > - fpga_mgr_unregister(mgr); > > -} > > -EXPORT_SYMBOL_GPL(alt_pr_unregister); > > Similar to the others, except for removing this symbol. > > A patch should do one logical thing. I was on the fence with this. Tbh, this driver should be a platform driver. I'll create a separate series for that. > > I'd rather this be split out of the patchset. > > Tom > > > - > > MODULE_AUTHOR("Matthew Gerlach <matthew.gerlach@linux.intel.com>"); > > MODULE_DESCRIPTION("Altera Partial Reconfiguration IP Core"); > > MODULE_LICENSE("GPL v2"); > > diff --git a/include/linux/fpga/altera-pr-ip-core.h b/include/linux/fpga/altera-pr-ip-core.h > > index 0b08ac20ab16..a6b4c07858cc 100644 > > --- a/include/linux/fpga/altera-pr-ip-core.h > > +++ b/include/linux/fpga/altera-pr-ip-core.h > > @@ -13,6 +13,5 @@ > > #include <linux/io.h> > > > > int alt_pr_register(struct device *dev, void __iomem *reg_base); > > -void alt_pr_unregister(struct device *dev); > > > > #endif /* _ALT_PR_IP_CORE_H */ > Cheers, Moritz
WARNING: multiple messages have this Message-ID (diff)
From: Moritz Fischer <mdf@kernel.org> To: Tom Rix <trix@redhat.com> Cc: russell.h.weight@intel.com, linux-fpga@vger.kernel.org, linux-kernel@vger.kernel.org, matthew.gerlach@intel.com, michal.simek@xilinx.com, Moritz Fischer <mdf@kernel.org>, linux-arm-kernel@lists.infradead.org, hao.wu@intel.com Subject: Re: [PATCH 10/10] fpga: fpga-mgr: altera-pr-ip: Simplify registration Date: Sun, 4 Oct 2020 16:39:14 -0700 [thread overview] Message-ID: <20201004233914.GA111357@epycbox.lan> (raw) In-Reply-To: <a49b1d7c-9756-1059-f7a1-25dae460d659@redhat.com> On Sun, Oct 04, 2020 at 11:47:26AM -0700, Tom Rix wrote: > > On 10/3/20 10:14 PM, Moritz Fischer wrote: > > Simplify registration using new devm_fpga_mgr_register() API. > > Remove the now obsolete altera_pr_unregister() function. > > > > Signed-off-by: Moritz Fischer <mdf@kernel.org> > > --- > > > > We should take another look at this, IIRC correctly the point of > > splitting this up into a separate driver was to make it useable by a > > different (pci?) driver later on. > > > > It doesn't seem like this happened, and I think we should just make this > > a platform driver? > > > > --- > > drivers/fpga/altera-pr-ip-core-plat.c | 10 ---------- > > drivers/fpga/altera-pr-ip-core.c | 14 +------------- > > include/linux/fpga/altera-pr-ip-core.h | 1 - > > 3 files changed, 1 insertion(+), 24 deletions(-) > > > > diff --git a/drivers/fpga/altera-pr-ip-core-plat.c b/drivers/fpga/altera-pr-ip-core-plat.c > > index 99b9cc0e70f0..b008a6b8d2d3 100644 > > --- a/drivers/fpga/altera-pr-ip-core-plat.c > > +++ b/drivers/fpga/altera-pr-ip-core-plat.c > > @@ -28,15 +28,6 @@ static int alt_pr_platform_probe(struct platform_device *pdev) > > return alt_pr_register(dev, reg_base); > > } > > > > -static int alt_pr_platform_remove(struct platform_device *pdev) > > -{ > > - struct device *dev = &pdev->dev; > > - > > - alt_pr_unregister(dev); > > - > > - return 0; > > -} > > - > > static const struct of_device_id alt_pr_of_match[] = { > > { .compatible = "altr,a10-pr-ip", }, > > {}, > > @@ -46,7 +37,6 @@ MODULE_DEVICE_TABLE(of, alt_pr_of_match); > > > > static struct platform_driver alt_pr_platform_driver = { > > .probe = alt_pr_platform_probe, > > - .remove = alt_pr_platform_remove, > > .driver = { > > .name = "alt_a10_pr_ip", > > .of_match_table = alt_pr_of_match, > > diff --git a/drivers/fpga/altera-pr-ip-core.c b/drivers/fpga/altera-pr-ip-core.c > > index 2cf25fd5e897..dfdf21ed34c4 100644 > > --- a/drivers/fpga/altera-pr-ip-core.c > > +++ b/drivers/fpga/altera-pr-ip-core.c > > @@ -195,22 +195,10 @@ int alt_pr_register(struct device *dev, void __iomem *reg_base) > > if (!mgr) > > return -ENOMEM; > > > > - dev_set_drvdata(dev, mgr); > > - > > - return fpga_mgr_register(mgr); > > + return devm_fpga_mgr_register(dev, mgr); > > } > > EXPORT_SYMBOL_GPL(alt_pr_register); > > > > -void alt_pr_unregister(struct device *dev) > > -{ > > - struct fpga_manager *mgr = dev_get_drvdata(dev); > > - > > - dev_dbg(dev, "%s\n", __func__); > > - > > - fpga_mgr_unregister(mgr); > > -} > > -EXPORT_SYMBOL_GPL(alt_pr_unregister); > > Similar to the others, except for removing this symbol. > > A patch should do one logical thing. I was on the fence with this. Tbh, this driver should be a platform driver. I'll create a separate series for that. > > I'd rather this be split out of the patchset. > > Tom > > > - > > MODULE_AUTHOR("Matthew Gerlach <matthew.gerlach@linux.intel.com>"); > > MODULE_DESCRIPTION("Altera Partial Reconfiguration IP Core"); > > MODULE_LICENSE("GPL v2"); > > diff --git a/include/linux/fpga/altera-pr-ip-core.h b/include/linux/fpga/altera-pr-ip-core.h > > index 0b08ac20ab16..a6b4c07858cc 100644 > > --- a/include/linux/fpga/altera-pr-ip-core.h > > +++ b/include/linux/fpga/altera-pr-ip-core.h > > @@ -13,6 +13,5 @@ > > #include <linux/io.h> > > > > int alt_pr_register(struct device *dev, void __iomem *reg_base); > > -void alt_pr_unregister(struct device *dev); > > > > #endif /* _ALT_PR_IP_CORE_H */ > Cheers, Moritz _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-10-04 23:39 UTC|newest] Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-10-04 5:14 [PATCH 00/10] Introduce devm_fpga_mgr_register() Moritz Fischer 2020-10-04 5:14 ` Moritz Fischer 2020-10-04 5:14 ` [PATCH 01/10] fpga: fpga-mgr: Add devm_fpga_mgr_register() API Moritz Fischer 2020-10-04 5:14 ` Moritz Fischer 2020-10-04 18:08 ` Tom Rix 2020-10-04 18:08 ` Tom Rix 2020-10-05 5:18 ` Wu, Hao 2020-10-05 5:18 ` Wu, Hao 2020-10-05 16:45 ` Moritz Fischer 2020-10-05 16:45 ` Moritz Fischer 2020-10-04 5:14 ` [PATCH 02/10] fpga: fpga-mgr: altera-ps-spi: Simplify registration Moritz Fischer 2020-10-04 5:14 ` Moritz Fischer 2020-10-04 18:15 ` Tom Rix 2020-10-04 18:15 ` Tom Rix 2020-10-04 5:14 ` [PATCH 03/10] fpga: fpga-mgr: dfl-fme-mgr: " Moritz Fischer 2020-10-04 5:14 ` Moritz Fischer 2020-10-04 18:22 ` Tom Rix 2020-10-04 18:22 ` Tom Rix 2020-10-04 23:40 ` Moritz Fischer 2020-10-04 23:40 ` Moritz Fischer 2020-10-04 5:14 ` [PATCH 04/10] fpga: fpga-mgr: ice40-spi: " Moritz Fischer 2020-10-04 5:14 ` Moritz Fischer 2020-10-04 18:24 ` Tom Rix 2020-10-04 18:24 ` Tom Rix 2020-10-04 5:14 ` [PATCH 05/10] fpga: fpga-mgr: machxo2-spi: " Moritz Fischer 2020-10-04 5:14 ` Moritz Fischer 2020-10-04 18:25 ` Tom Rix 2020-10-04 18:25 ` Tom Rix 2020-10-04 5:14 ` [PATCH 06/10] fpga: fpga-mgr: socfpga: " Moritz Fischer 2020-10-04 5:14 ` Moritz Fischer 2020-10-04 18:27 ` Tom Rix 2020-10-04 18:27 ` Tom Rix 2020-10-04 5:14 ` [PATCH 07/10] fpga: fpga-mgr: ts73xx: " Moritz Fischer 2020-10-04 5:14 ` Moritz Fischer 2020-10-04 18:27 ` Tom Rix 2020-10-04 18:27 ` Tom Rix 2020-10-04 5:14 ` [PATCH 08/10] fpga: fpga-mgr: xilinx-spi: " Moritz Fischer 2020-10-04 5:14 ` Moritz Fischer 2020-10-04 18:28 ` Tom Rix 2020-10-04 18:28 ` Tom Rix 2020-10-04 5:14 ` [PATCH 09/10] fpga: fpga-mgr: zynqmp: " Moritz Fischer 2020-10-04 5:14 ` Moritz Fischer 2020-10-04 18:33 ` Tom Rix 2020-10-04 18:33 ` Tom Rix 2020-10-04 5:14 ` [PATCH 10/10] fpga: fpga-mgr: altera-pr-ip: " Moritz Fischer 2020-10-04 5:14 ` Moritz Fischer 2020-10-04 18:47 ` Tom Rix 2020-10-04 18:47 ` Tom Rix 2020-10-04 23:39 ` Moritz Fischer [this message] 2020-10-04 23:39 ` Moritz Fischer 2020-11-03 7:14 [PATCH 00/10] Introduce devm_fpga_mgr_register() API Moritz Fischer 2020-11-03 7:14 ` [PATCH 10/10] fpga: fpga-mgr: altera-pr-ip: Simplify registration Moritz Fischer
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=20201004233914.GA111357@epycbox.lan \ --to=mdf@kernel.org \ --cc=hao.wu@intel.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-fpga@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=matthew.gerlach@intel.com \ --cc=michal.simek@xilinx.com \ --cc=russell.h.weight@intel.com \ --cc=trix@redhat.com \ /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: linkBe 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.