| Message ID | 1478732303-13718-2-git-send-email-jgunthorpe@obsidianresearch.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 09/11/16 23:58, Jason Gunthorpe wrote: > Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> Please add a commit message, although it is cristal clear what this patch does :) Regards, Matthias > --- > drivers/fpga/zynq-fpga.c | 22 +++++++++++----------- > 1 file changed, 11 insertions(+), 11 deletions(-) > > diff --git a/drivers/fpga/zynq-fpga.c b/drivers/fpga/zynq-fpga.c > index c2fb4120bd62..e72340ea7323 100644 > --- a/drivers/fpga/zynq-fpga.c > +++ b/drivers/fpga/zynq-fpga.c > @@ -217,7 +217,7 @@ static int zynq_fpga_ops_write_init(struct fpga_manager *mgr, u32 flags, > INIT_POLL_DELAY, > INIT_POLL_TIMEOUT); > if (err) { > - dev_err(priv->dev, "Timeout waiting for PCFG_INIT"); > + dev_err(priv->dev, "Timeout waiting for PCFG_INIT\n"); > goto out_err; > } > > @@ -231,7 +231,7 @@ static int zynq_fpga_ops_write_init(struct fpga_manager *mgr, u32 flags, > INIT_POLL_DELAY, > INIT_POLL_TIMEOUT); > if (err) { > - dev_err(priv->dev, "Timeout waiting for !PCFG_INIT"); > + dev_err(priv->dev, "Timeout waiting for !PCFG_INIT\n"); > goto out_err; > } > > @@ -245,7 +245,7 @@ static int zynq_fpga_ops_write_init(struct fpga_manager *mgr, u32 flags, > INIT_POLL_DELAY, > INIT_POLL_TIMEOUT); > if (err) { > - dev_err(priv->dev, "Timeout waiting for PCFG_INIT"); > + dev_err(priv->dev, "Timeout waiting for PCFG_INIT\n"); > goto out_err; > } > } > @@ -262,7 +262,7 @@ static int zynq_fpga_ops_write_init(struct fpga_manager *mgr, u32 flags, > /* check that we have room in the command queue */ > status = zynq_fpga_read(priv, STATUS_OFFSET); > if (status & STATUS_DMA_Q_F) { > - dev_err(priv->dev, "DMA command queue full"); > + dev_err(priv->dev, "DMA command queue full\n"); > err = -EBUSY; > goto out_err; > } > @@ -331,7 +331,7 @@ static int zynq_fpga_ops_write(struct fpga_manager *mgr, > zynq_fpga_write(priv, INT_STS_OFFSET, intr_status); > > if (!((intr_status & IXR_D_P_DONE_MASK) == IXR_D_P_DONE_MASK)) { > - dev_err(priv->dev, "Error configuring FPGA"); > + dev_err(priv->dev, "Error configuring FPGA\n"); > err = -EFAULT; > } > > @@ -426,7 +426,7 @@ static int zynq_fpga_probe(struct platform_device *pdev) > priv->slcr = syscon_regmap_lookup_by_phandle(dev->of_node, > "syscon"); > if (IS_ERR(priv->slcr)) { > - dev_err(dev, "unable to get zynq-slcr regmap"); > + dev_err(dev, "unable to get zynq-slcr regmap\n"); > return PTR_ERR(priv->slcr); > } > > @@ -434,26 +434,26 @@ static int zynq_fpga_probe(struct platform_device *pdev) > > priv->irq = platform_get_irq(pdev, 0); > if (priv->irq < 0) { > - dev_err(dev, "No IRQ available"); > + dev_err(dev, "No IRQ available\n"); > return priv->irq; > } > > err = devm_request_irq(dev, priv->irq, zynq_fpga_isr, 0, > dev_name(dev), priv); > if (err) { > - dev_err(dev, "unable to request IRQ"); > + dev_err(dev, "unable to request IRQ\n"); > return err; > } > > priv->clk = devm_clk_get(dev, "ref_clk"); > if (IS_ERR(priv->clk)) { > - dev_err(dev, "input clock not found"); > + dev_err(dev, "input clock not found\n"); > return PTR_ERR(priv->clk); > } > > err = clk_prepare_enable(priv->clk); > if (err) { > - dev_err(dev, "unable to enable clock"); > + dev_err(dev, "unable to enable clock\n"); > return err; > } > > @@ -465,7 +465,7 @@ static int zynq_fpga_probe(struct platform_device *pdev) > err = fpga_mgr_register(dev, "Xilinx Zynq FPGA Manager", > &zynq_fpga_ops, priv); > if (err) { > - dev_err(dev, "unable to register FPGA manager"); > + dev_err(dev, "unable to register FPGA manager\n"); > clk_unprepare(priv->clk); > return err; > } >
On Tue, Nov 15, 2016 at 12:05:15PM +0100, Matthias Brugger wrote: > On 09/11/16 23:58, Jason Gunthorpe wrote: > >Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > > Please add a commit message, although it is cristal clear what this patch > does :) As you say, it is crystal clear already, and this is an acceptable commit message.. Please suggest a text if you want to see something different. Jason
On Tue, Nov 15, 2016 at 11:08:13AM -0700, Jason Gunthorpe wrote: > > On Tue, Nov 15, 2016 at 12:05:15PM +0100, Matthias Brugger wrote: > > On 09/11/16 23:58, Jason Gunthorpe wrote: > > >Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > > > > Please add a commit message, although it is cristal clear what this patch > > does :) That. > > As you say, it is crystal clear already, and this is an acceptable commit > message.. Please suggest a text if you want to see something > different. > It still needs a long message. Just do it. Thanks, Moritz
On Wed, Nov 16, 2016 at 10:39:26AM -0800, Moritz Fischer wrote: > > As you say, it is crystal clear already, and this is an acceptable commit > > message.. Please suggest a text if you want to see something > > different. > > > > It still needs a long message. Just do it. Can I put your reviewed-by on this when I resend the series? Will you have time to review the other patches this week? Jason
On Wed, 16 Nov 2016, Jason Gunthorpe wrote: > On Wed, Nov 16, 2016 at 10:39:26AM -0800, Moritz Fischer wrote: > > > As you say, it is crystal clear already, and this is an acceptable commit > > > message.. Please suggest a text if you want to see something > > > different. > > > > > > > It still needs a long message. Just do it. > > Can I put your reviewed-by on this when I resend the series? > > Will you have time to review the other patches this week? > > Jason > Hi Jason, I agree that this needs a long description and I'm not sure why you're pushing back on that. You will need to rebase this series on linux-next. I won't have much time this week to look at this, sorry. Not sure what the hurry is. I see the value of this, but things are busy right now. I'll be getting up to speed on how the kernel does sg and this is kind of a fundamental change to the API so I appreciate your patience. Alan
Hi Jason, On Wed, Nov 16, 2016 at 2:28 PM, atull <atull@opensource.altera.com> wrote: > On Wed, 16 Nov 2016, Jason Gunthorpe wrote: > >> On Wed, Nov 16, 2016 at 10:39:26AM -0800, Moritz Fischer wrote: >> > > As you say, it is crystal clear already, and this is an acceptable commit >> > > message.. Please suggest a text if you want to see something >> > > different. >> > > >> > >> > It still needs a long message. Just do it. >> >> Can I put your reviewed-by on this when I resend the series? For this particular patch, feel free to add my Reviewed-By: after fixing the long description. As Alan said there's currently a lot of patches in flight and we're working on coming up with a better workflow. Stay tuned. >> >> Will you have time to review the other patches this week? >> >> Jason >> > > Hi Jason, > > I agree that this needs a long description and I'm not sure > why you're pushing back on that. > > You will need to rebase this series on linux-next. I won't > have much time this week to look at this, sorry. Not sure > what the hurry is. I see the value of this, but things are > busy right now. I'll be getting up to speed on how the > kernel does sg and this is kind of a fundamental change to > the API so I appreciate your patience. Same here. We'll get prototypes next week, so I'm busy this week and next week. I agree there's no massive hurry. Thanks, Moritz
On Wed, Nov 16, 2016 at 04:28:27PM -0600, atull wrote: > I agree that this needs a long description and I'm not sure > why you're pushing back on that. *shurg* it is just an unusual thing to ask for, but it is OK for a subsystem to have a particular style like this. Other subsystems do not. I will add "This makes the driver prints display properly." > You will need to rebase this series on linux-next. I won't > have much time this week to look at this, sorry. Not sure > what the hurry is. I see the value of this, but things are > busy right now. I'll be getting up to speed on how the > kernel does sg and this is kind of a fundamental change to > the API so I appreciate your patience. The merge window is opening soon so it would be nice to land some of this in 4.10. Since I have to rebase and restructure a chunk of it, I'd like to get the bulk of the broad feedback collected before doing that.. Jason
On 15/11/16 19:08, Jason Gunthorpe wrote: > > On Tue, Nov 15, 2016 at 12:05:15PM +0100, Matthias Brugger wrote: >> On 09/11/16 23:58, Jason Gunthorpe wrote: >>> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> >> >> Please add a commit message, although it is cristal clear what this patch >> does :) > > As you say, it is crystal clear already, and this is an acceptable commit > message.. Please suggest a text if you want to see something > different. > "Function dev_err doesn't add a newline at the end of the string. This will lead to a hard to read kernel log. This patch fixes this." With this (or something similar added): Reviewed-by: Matthias Brugger <mbrugger@suse.com> Regards, Matthias
diff --git a/drivers/fpga/zynq-fpga.c b/drivers/fpga/zynq-fpga.c index c2fb4120bd62..e72340ea7323 100644 --- a/drivers/fpga/zynq-fpga.c +++ b/drivers/fpga/zynq-fpga.c @@ -217,7 +217,7 @@ static int zynq_fpga_ops_write_init(struct fpga_manager *mgr, u32 flags, INIT_POLL_DELAY, INIT_POLL_TIMEOUT); if (err) { - dev_err(priv->dev, "Timeout waiting for PCFG_INIT"); + dev_err(priv->dev, "Timeout waiting for PCFG_INIT\n"); goto out_err; } @@ -231,7 +231,7 @@ static int zynq_fpga_ops_write_init(struct fpga_manager *mgr, u32 flags, INIT_POLL_DELAY, INIT_POLL_TIMEOUT); if (err) { - dev_err(priv->dev, "Timeout waiting for !PCFG_INIT"); + dev_err(priv->dev, "Timeout waiting for !PCFG_INIT\n"); goto out_err; } @@ -245,7 +245,7 @@ static int zynq_fpga_ops_write_init(struct fpga_manager *mgr, u32 flags, INIT_POLL_DELAY, INIT_POLL_TIMEOUT); if (err) { - dev_err(priv->dev, "Timeout waiting for PCFG_INIT"); + dev_err(priv->dev, "Timeout waiting for PCFG_INIT\n"); goto out_err; } } @@ -262,7 +262,7 @@ static int zynq_fpga_ops_write_init(struct fpga_manager *mgr, u32 flags, /* check that we have room in the command queue */ status = zynq_fpga_read(priv, STATUS_OFFSET); if (status & STATUS_DMA_Q_F) { - dev_err(priv->dev, "DMA command queue full"); + dev_err(priv->dev, "DMA command queue full\n"); err = -EBUSY; goto out_err; } @@ -331,7 +331,7 @@ static int zynq_fpga_ops_write(struct fpga_manager *mgr, zynq_fpga_write(priv, INT_STS_OFFSET, intr_status); if (!((intr_status & IXR_D_P_DONE_MASK) == IXR_D_P_DONE_MASK)) { - dev_err(priv->dev, "Error configuring FPGA"); + dev_err(priv->dev, "Error configuring FPGA\n"); err = -EFAULT; } @@ -426,7 +426,7 @@ static int zynq_fpga_probe(struct platform_device *pdev) priv->slcr = syscon_regmap_lookup_by_phandle(dev->of_node, "syscon"); if (IS_ERR(priv->slcr)) { - dev_err(dev, "unable to get zynq-slcr regmap"); + dev_err(dev, "unable to get zynq-slcr regmap\n"); return PTR_ERR(priv->slcr); } @@ -434,26 +434,26 @@ static int zynq_fpga_probe(struct platform_device *pdev) priv->irq = platform_get_irq(pdev, 0); if (priv->irq < 0) { - dev_err(dev, "No IRQ available"); + dev_err(dev, "No IRQ available\n"); return priv->irq; } err = devm_request_irq(dev, priv->irq, zynq_fpga_isr, 0, dev_name(dev), priv); if (err) { - dev_err(dev, "unable to request IRQ"); + dev_err(dev, "unable to request IRQ\n"); return err; } priv->clk = devm_clk_get(dev, "ref_clk"); if (IS_ERR(priv->clk)) { - dev_err(dev, "input clock not found"); + dev_err(dev, "input clock not found\n"); return PTR_ERR(priv->clk); } err = clk_prepare_enable(priv->clk); if (err) { - dev_err(dev, "unable to enable clock"); + dev_err(dev, "unable to enable clock\n"); return err; } @@ -465,7 +465,7 @@ static int zynq_fpga_probe(struct platform_device *pdev) err = fpga_mgr_register(dev, "Xilinx Zynq FPGA Manager", &zynq_fpga_ops, priv); if (err) { - dev_err(dev, "unable to register FPGA manager"); + dev_err(dev, "unable to register FPGA manager\n"); clk_unprepare(priv->clk); return err; }
Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> --- drivers/fpga/zynq-fpga.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-)