| Message ID | 1563857495-26692-10-git-send-email-hao.wu@intel.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | FPGA DFL updates | expand |
On Tue, Jul 23, 2019 at 12:51:32PM +0800, Wu Hao wrote: > STP (SignalTap) is one of the private features under the port for > debugging. This patch adds private feature driver support for it > to allow userspace applications to mmap related mmio region and > provide STP service. > > Signed-off-by: Xu Yilun <yilun.xu@intel.com> > Signed-off-by: Wu Hao <hao.wu@intel.com> > Acked-by: Moritz Fischer <mdf@kernel.org> > Acked-by: Alan Tull <atull@kernel.org> > Signed-off-by: Moritz Fischer <mdf@kernel.org> > --- > drivers/fpga/dfl-afu-main.c | 34 ++++++++++++++++++++++++++++++++++ > 1 file changed, 34 insertions(+) > > diff --git a/drivers/fpga/dfl-afu-main.c b/drivers/fpga/dfl-afu-main.c > index 15dd4cb..395f96e 100644 > --- a/drivers/fpga/dfl-afu-main.c > +++ b/drivers/fpga/dfl-afu-main.c > @@ -514,6 +514,36 @@ static void port_afu_uinit(struct platform_device *pdev, > .uinit = port_afu_uinit, > }; > > +static int port_stp_init(struct platform_device *pdev, > + struct dfl_feature *feature) > +{ > + struct resource *res = &pdev->resource[feature->resource_index]; > + > + dev_dbg(&pdev->dev, "PORT STP Init.\n"); ftrace is your friend, no need to do a lot of "look I am here!" messages. > + > + return afu_mmio_region_add(dev_get_platdata(&pdev->dev), > + DFL_PORT_REGION_INDEX_STP, > + resource_size(res), res->start, > + DFL_PORT_REGION_MMAP | DFL_PORT_REGION_READ | > + DFL_PORT_REGION_WRITE); > +} > + > +static void port_stp_uinit(struct platform_device *pdev, > + struct dfl_feature *feature) > +{ > + dev_dbg(&pdev->dev, "PORT STP UInit.\n"); Same here. Why have this function at all if it does not do anything? thanks, greg k-h
On Wed, Jul 24, 2019 at 12:11:09PM +0200, Greg KH wrote: > On Tue, Jul 23, 2019 at 12:51:32PM +0800, Wu Hao wrote: > > STP (SignalTap) is one of the private features under the port for > > debugging. This patch adds private feature driver support for it > > to allow userspace applications to mmap related mmio region and > > provide STP service. > > > > Signed-off-by: Xu Yilun <yilun.xu@intel.com> > > Signed-off-by: Wu Hao <hao.wu@intel.com> > > Acked-by: Moritz Fischer <mdf@kernel.org> > > Acked-by: Alan Tull <atull@kernel.org> > > Signed-off-by: Moritz Fischer <mdf@kernel.org> > > --- > > drivers/fpga/dfl-afu-main.c | 34 ++++++++++++++++++++++++++++++++++ > > 1 file changed, 34 insertions(+) > > > > diff --git a/drivers/fpga/dfl-afu-main.c b/drivers/fpga/dfl-afu-main.c > > index 15dd4cb..395f96e 100644 > > --- a/drivers/fpga/dfl-afu-main.c > > +++ b/drivers/fpga/dfl-afu-main.c > > @@ -514,6 +514,36 @@ static void port_afu_uinit(struct platform_device *pdev, > > .uinit = port_afu_uinit, > > }; > > > > +static int port_stp_init(struct platform_device *pdev, > > + struct dfl_feature *feature) > > +{ > > + struct resource *res = &pdev->resource[feature->resource_index]; > > + > > + dev_dbg(&pdev->dev, "PORT STP Init.\n"); > > ftrace is your friend, no need to do a lot of "look I am here!" > messages. Hi Greg, Thanks for the code review! Sure, let me remove them. > > > + > > + return afu_mmio_region_add(dev_get_platdata(&pdev->dev), > > + DFL_PORT_REGION_INDEX_STP, > > + resource_size(res), res->start, > > + DFL_PORT_REGION_MMAP | DFL_PORT_REGION_READ | > > + DFL_PORT_REGION_WRITE); > > +} > > + > > +static void port_stp_uinit(struct platform_device *pdev, > > + struct dfl_feature *feature) > > +{ > > + dev_dbg(&pdev->dev, "PORT STP UInit.\n"); > > Same here. > > Why have this function at all if it does not do anything? Let me remove them in the next version. actually uinit callback is always required in current code, i will add one more patch to change it, and remove all uinit functions who do nothing, it does save code. Thanks for the comments. Hao > > > thanks, > > greg k-h
diff --git a/drivers/fpga/dfl-afu-main.c b/drivers/fpga/dfl-afu-main.c index 15dd4cb..395f96e 100644 --- a/drivers/fpga/dfl-afu-main.c +++ b/drivers/fpga/dfl-afu-main.c @@ -514,6 +514,36 @@ static void port_afu_uinit(struct platform_device *pdev, .uinit = port_afu_uinit, }; +static int port_stp_init(struct platform_device *pdev, + struct dfl_feature *feature) +{ + struct resource *res = &pdev->resource[feature->resource_index]; + + dev_dbg(&pdev->dev, "PORT STP Init.\n"); + + return afu_mmio_region_add(dev_get_platdata(&pdev->dev), + DFL_PORT_REGION_INDEX_STP, + resource_size(res), res->start, + DFL_PORT_REGION_MMAP | DFL_PORT_REGION_READ | + DFL_PORT_REGION_WRITE); +} + +static void port_stp_uinit(struct platform_device *pdev, + struct dfl_feature *feature) +{ + dev_dbg(&pdev->dev, "PORT STP UInit.\n"); +} + +static const struct dfl_feature_id port_stp_id_table[] = { + {.id = PORT_FEATURE_ID_STP,}, + {0,} +}; + +static const struct dfl_feature_ops port_stp_ops = { + .init = port_stp_init, + .uinit = port_stp_uinit, +}; + static struct dfl_feature_driver port_feature_drvs[] = { { .id_table = port_hdr_id_table, @@ -528,6 +558,10 @@ static void port_afu_uinit(struct platform_device *pdev, .ops = &port_err_ops, }, { + .id_table = port_stp_id_table, + .ops = &port_stp_ops, + }, + { .ops = NULL, } };