No need to check the return value of debugfs_create_file(), so no need
to provide a fake "cast away" of the return value either.
Cc: Dan Williams <[email protected]>
Cc: Vinod Koul <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/dma/amba-pl08x.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/dma/amba-pl08x.c b/drivers/dma/amba-pl08x.c
index 464725dcad00..9adc7a2fa3d3 100644
--- a/drivers/dma/amba-pl08x.c
+++ b/drivers/dma/amba-pl08x.c
@@ -2508,9 +2508,8 @@ DEFINE_SHOW_ATTRIBUTE(pl08x_debugfs);
static void init_pl08x_debugfs(struct pl08x_driver_data *pl08x)
{
/* Expose a simple debugfs interface to view all clocks */
- (void) debugfs_create_file(dev_name(&pl08x->adev->dev),
- S_IFREG | S_IRUGO, NULL, pl08x,
- &pl08x_debugfs_fops);
+ debugfs_create_file(dev_name(&pl08x->adev->dev), S_IFREG | S_IRUGO,
+ NULL, pl08x, &pl08x_debugfs_fops);
}
#else
--
2.22.0
When calling debugfs functions, there is no need to ever check the
return value. The function can work or not, but the code logic should
never do something different based on this.
Also, because there is no need to save the file dentry, remove the
variable that was saving it as it was never even being used once set.
Cc: Daniel Mack <[email protected]>
Cc: Haojian Zhuang <[email protected]>
Cc: Robert Jarzmik <[email protected]>
Cc: Vinod Koul <[email protected]>
Cc: Dan Williams <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/dma/pxa_dma.c | 56 +++++++++----------------------------------
1 file changed, 11 insertions(+), 45 deletions(-)
diff --git a/drivers/dma/pxa_dma.c b/drivers/dma/pxa_dma.c
index b429642f3e7a..0f698f49ee26 100644
--- a/drivers/dma/pxa_dma.c
+++ b/drivers/dma/pxa_dma.c
@@ -132,7 +132,6 @@ struct pxad_device {
spinlock_t phy_lock; /* Phy association */
#ifdef CONFIG_DEBUG_FS
struct dentry *dbgfs_root;
- struct dentry *dbgfs_state;
struct dentry **dbgfs_chan;
#endif
};
@@ -326,31 +325,18 @@ static struct dentry *pxad_dbg_alloc_chan(struct pxad_device *pdev,
int ch, struct dentry *chandir)
{
char chan_name[11];
- struct dentry *chan, *chan_state = NULL, *chan_descr = NULL;
- struct dentry *chan_reqs = NULL;
+ struct dentry *chan;
void *dt;
scnprintf(chan_name, sizeof(chan_name), "%d", ch);
chan = debugfs_create_dir(chan_name, chandir);
dt = (void *)&pdev->phys[ch];
- if (chan)
- chan_state = debugfs_create_file("state", 0400, chan, dt,
- &chan_state_fops);
- if (chan_state)
- chan_descr = debugfs_create_file("descriptors", 0400, chan, dt,
- &descriptors_fops);
- if (chan_descr)
- chan_reqs = debugfs_create_file("requesters", 0400, chan, dt,
- &requester_chan_fops);
- if (!chan_reqs)
- goto err_state;
+ debugfs_create_file("state", 0400, chan, dt, &chan_state_fops);
+ debugfs_create_file("descriptors", 0400, chan, dt, &descriptors_fops);
+ debugfs_create_file("requesters", 0400, chan, dt, &requester_chan_fops);
return chan;
-
-err_state:
- debugfs_remove_recursive(chan);
- return NULL;
}
static void pxad_init_debugfs(struct pxad_device *pdev)
@@ -358,40 +344,20 @@ static void pxad_init_debugfs(struct pxad_device *pdev)
int i;
struct dentry *chandir;
- pdev->dbgfs_root = debugfs_create_dir(dev_name(pdev->slave.dev), NULL);
- if (IS_ERR(pdev->dbgfs_root) || !pdev->dbgfs_root)
- goto err_root;
-
- pdev->dbgfs_state = debugfs_create_file("state", 0400, pdev->dbgfs_root,
- pdev, &state_fops);
- if (!pdev->dbgfs_state)
- goto err_state;
-
pdev->dbgfs_chan =
- kmalloc_array(pdev->nr_chans, sizeof(*pdev->dbgfs_state),
+ kmalloc_array(pdev->nr_chans, sizeof(struct dentry *),
GFP_KERNEL);
if (!pdev->dbgfs_chan)
- goto err_alloc;
+ return;
+
+ pdev->dbgfs_root = debugfs_create_dir(dev_name(pdev->slave.dev), NULL);
+
+ debugfs_create_file("state", 0400, pdev->dbgfs_root, pdev, &state_fops);
chandir = debugfs_create_dir("channels", pdev->dbgfs_root);
- if (!chandir)
- goto err_chandir;
- for (i = 0; i < pdev->nr_chans; i++) {
+ for (i = 0; i < pdev->nr_chans; i++)
pdev->dbgfs_chan[i] = pxad_dbg_alloc_chan(pdev, i, chandir);
- if (!pdev->dbgfs_chan[i])
- goto err_chans;
- }
-
- return;
-err_chans:
-err_chandir:
- kfree(pdev->dbgfs_chan);
-err_alloc:
-err_state:
- debugfs_remove_recursive(pdev->dbgfs_root);
-err_root:
- pr_err("pxad: debugfs is not available\n");
}
static void pxad_cleanup_debugfs(struct pxad_device *pdev)
--
2.22.0
When calling debugfs functions, there is no need to ever check the
return value. The function can work or not, but the code logic should
never do something different based on this.
Cc: Sudeep Dutt <[email protected]>
Cc: Ashutosh Dixit <[email protected]>
Cc: Dan Williams <[email protected]>
Cc: Vinod Koul <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/dma/mic_x100_dma.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/dma/mic_x100_dma.c b/drivers/dma/mic_x100_dma.c
index 6a91e28d537d..584e09661507 100644
--- a/drivers/dma/mic_x100_dma.c
+++ b/drivers/dma/mic_x100_dma.c
@@ -728,10 +728,8 @@ static int mic_dma_driver_probe(struct mbus_device *mbdev)
if (mic_dma_dbg) {
mic_dma_dev->dbg_dir = debugfs_create_dir(dev_name(&mbdev->dev),
mic_dma_dbg);
- if (mic_dma_dev->dbg_dir)
- debugfs_create_file("mic_dma_reg", 0444,
- mic_dma_dev->dbg_dir, mic_dma_dev,
- &mic_dma_reg_fops);
+ debugfs_create_file("mic_dma_reg", 0444, mic_dma_dev->dbg_dir,
+ mic_dma_dev, &mic_dma_reg_fops);
}
return 0;
}
--
2.22.0
When calling debugfs functions, there is no need to ever check the
return value. The function can work or not, but the code logic should
never do something different based on this.
Also, because there is no need to save the file dentry, remove the
variables that were saving them as they were never even being used once
set.
Cc: Sinan Kaya <[email protected]>
Cc: Andy Gross <[email protected]>
Cc: David Brown <[email protected]>
Cc: Dan Williams <[email protected]>
Cc: Vinod Koul <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/dma/qcom/hidma.h | 5 +----
drivers/dma/qcom/hidma_dbg.c | 37 +++++++-----------------------------
2 files changed, 8 insertions(+), 34 deletions(-)
diff --git a/drivers/dma/qcom/hidma.h b/drivers/dma/qcom/hidma.h
index 5f9966e82c0b..36357d02333a 100644
--- a/drivers/dma/qcom/hidma.h
+++ b/drivers/dma/qcom/hidma.h
@@ -101,8 +101,6 @@ struct hidma_chan {
* It is used by the DMA complete notification to
* locate the descriptor that initiated the transfer.
*/
- struct dentry *debugfs;
- struct dentry *stats;
struct hidma_dev *dmadev;
struct hidma_desc *running;
@@ -134,7 +132,6 @@ struct hidma_dev {
struct dma_device ddev;
struct dentry *debugfs;
- struct dentry *stats;
/* sysfs entry for the channel id */
struct device_attribute *chid_attrs;
@@ -166,6 +163,6 @@ irqreturn_t hidma_ll_inthandler(int irq, void *arg);
irqreturn_t hidma_ll_inthandler_msi(int irq, void *arg, int cause);
void hidma_cleanup_pending_tre(struct hidma_lldev *llhndl, u8 err_info,
u8 err_code);
-int hidma_debug_init(struct hidma_dev *dmadev);
+void hidma_debug_init(struct hidma_dev *dmadev);
void hidma_debug_uninit(struct hidma_dev *dmadev);
#endif
diff --git a/drivers/dma/qcom/hidma_dbg.c b/drivers/dma/qcom/hidma_dbg.c
index 9523faf7acdc..994f448b64d8 100644
--- a/drivers/dma/qcom/hidma_dbg.c
+++ b/drivers/dma/qcom/hidma_dbg.c
@@ -146,17 +146,13 @@ void hidma_debug_uninit(struct hidma_dev *dmadev)
debugfs_remove_recursive(dmadev->debugfs);
}
-int hidma_debug_init(struct hidma_dev *dmadev)
+void hidma_debug_init(struct hidma_dev *dmadev)
{
- int rc = 0;
int chidx = 0;
struct list_head *position = NULL;
+ struct dentry *dir;
dmadev->debugfs = debugfs_create_dir(dev_name(dmadev->ddev.dev), NULL);
- if (!dmadev->debugfs) {
- rc = -ENODEV;
- return rc;
- }
/* walk through the virtual channel list */
list_for_each(position, &dmadev->ddev.channels) {
@@ -165,32 +161,13 @@ int hidma_debug_init(struct hidma_dev *dmadev)
chan = list_entry(position, struct hidma_chan,
chan.device_node);
sprintf(chan->dbg_name, "chan%d", chidx);
- chan->debugfs = debugfs_create_dir(chan->dbg_name,
+ dir = debugfs_create_dir(chan->dbg_name,
dmadev->debugfs);
- if (!chan->debugfs) {
- rc = -ENOMEM;
- goto cleanup;
- }
- chan->stats = debugfs_create_file("stats", S_IRUGO,
- chan->debugfs, chan,
- &hidma_chan_fops);
- if (!chan->stats) {
- rc = -ENOMEM;
- goto cleanup;
- }
+ debugfs_create_file("stats", S_IRUGO, dir, chan,
+ &hidma_chan_fops);
chidx++;
}
- dmadev->stats = debugfs_create_file("stats", S_IRUGO,
- dmadev->debugfs, dmadev,
- &hidma_dma_fops);
- if (!dmadev->stats) {
- rc = -ENOMEM;
- goto cleanup;
- }
-
- return 0;
-cleanup:
- hidma_debug_uninit(dmadev);
- return rc;
+ debugfs_create_file("stats", S_IRUGO, dmadev->debugfs, dmadev,
+ &hidma_dma_fops);
}
--
2.22.0
When calling debugfs functions, there is no need to ever check the
return value. The function can work or not, but the code logic should
never do something different based on this.
Also, because there is no need to save the file dentry, remove the
variable that was saving it as it was never even being used once set.
Cc: Vinod Koul <[email protected]>
Cc: Dan Williams <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/dma/bcm-sba-raid.c | 13 +++----------
1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/drivers/dma/bcm-sba-raid.c b/drivers/dma/bcm-sba-raid.c
index fa81d0177765..275e90fa829d 100644
--- a/drivers/dma/bcm-sba-raid.c
+++ b/drivers/dma/bcm-sba-raid.c
@@ -164,7 +164,6 @@ struct sba_device {
struct list_head reqs_free_list;
/* DebugFS directory entries */
struct dentry *root;
- struct dentry *stats;
};
/* ====== Command helper routines ===== */
@@ -1716,17 +1715,11 @@ static int sba_probe(struct platform_device *pdev)
/* Create debugfs root entry */
sba->root = debugfs_create_dir(dev_name(sba->dev), NULL);
- if (IS_ERR_OR_NULL(sba->root)) {
- dev_err(sba->dev, "failed to create debugfs root entry\n");
- sba->root = NULL;
- goto skip_debugfs;
- }
/* Create debugfs stats entry */
- sba->stats = debugfs_create_devm_seqfile(sba->dev, "stats", sba->root,
- sba_debugfs_stats_show);
- if (IS_ERR_OR_NULL(sba->stats))
- dev_err(sba->dev, "failed to create debugfs stats file\n");
+ debugfs_create_devm_seqfile(sba->dev, "stats", sba->root,
+ sba_debugfs_stats_show);
+
skip_debugfs:
/* Register DMA device with Linux async framework */
--
2.22.0
On Wed, 2019-06-12 at 14:25 +0200, Greg Kroah-Hartman wrote:
> When calling debugfs functions, there is no need to ever check the
> return value. The function can work or not, but the code logic should
> never do something different based on this.
>
Thanks Greg.
Reviewed-by: Sudeep Dutt <[email protected]>
> Cc: Sudeep Dutt <[email protected]>
> Cc: Ashutosh Dixit <[email protected]>
> Cc: Dan Williams <[email protected]>
> Cc: Vinod Koul <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Greg Kroah-Hartman <[email protected]>
> ---
> drivers/dma/mic_x100_dma.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/dma/mic_x100_dma.c b/drivers/dma/mic_x100_dma.c
> index 6a91e28d537d..584e09661507 100644
> --- a/drivers/dma/mic_x100_dma.c
> +++ b/drivers/dma/mic_x100_dma.c
> @@ -728,10 +728,8 @@ static int mic_dma_driver_probe(struct mbus_device *mbdev)
> if (mic_dma_dbg) {
> mic_dma_dev->dbg_dir = debugfs_create_dir(dev_name(&mbdev->dev),
> mic_dma_dbg);
> - if (mic_dma_dev->dbg_dir)
> - debugfs_create_file("mic_dma_reg", 0444,
> - mic_dma_dev->dbg_dir, mic_dma_dev,
> - &mic_dma_reg_fops);
> + debugfs_create_file("mic_dma_reg", 0444, mic_dma_dev->dbg_dir,
> + mic_dma_dev, &mic_dma_reg_fops);
> }
> return 0;
> }
On 6/12/2019 8:25 AM, Greg Kroah-Hartman wrote:
> When calling debugfs functions, there is no need to ever check the
> return value. The function can work or not, but the code logic should
> never do something different based on this.
>
> Also, because there is no need to save the file dentry, remove the
> variables that were saving them as they were never even being used once
> set.
>
> Cc: Sinan Kaya <[email protected]>
> Cc: Andy Gross <[email protected]>
> Cc: David Brown <[email protected]>
> Cc: Dan Williams <[email protected]>
> Cc: Vinod Koul <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Greg Kroah-Hartman <[email protected]>
Interesting. Wouldn't debugfs_create_file() blow up if dir is NULL
for some reason?
+ debugfs_create_file("stats", S_IRUGO, dir, chan,
+ &hidma_chan_fops);
Note that code ignores the return value of hidma_debug_init();
It was just trying to do clean up on debugfs failure by calling
debugfs_remove_recursive(dmadev->debugfs);
On Wed, Jun 12, 2019 at 11:24:51AM -0400, Sinan Kaya wrote:
> On 6/12/2019 8:25 AM, Greg Kroah-Hartman wrote:
> > When calling debugfs functions, there is no need to ever check the
> > return value. The function can work or not, but the code logic should
> > never do something different based on this.
> >
> > Also, because there is no need to save the file dentry, remove the
> > variables that were saving them as they were never even being used once
> > set.
> >
> > Cc: Sinan Kaya <[email protected]>
> > Cc: Andy Gross <[email protected]>
> > Cc: David Brown <[email protected]>
> > Cc: Dan Williams <[email protected]>
> > Cc: Vinod Koul <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> > Signed-off-by: Greg Kroah-Hartman <[email protected]>
>
> Interesting. Wouldn't debugfs_create_file() blow up if dir is NULL
> for some reason?
It will create a file in the root of debugfs. But how will that happen?
debugfs_create_dir() can not return NULL.
> + debugfs_create_file("stats", S_IRUGO, dir, chan,
> + &hidma_chan_fops);
>
> Note that code ignores the return value of hidma_debug_init();
> It was just trying to do clean up on debugfs failure by calling
>
> debugfs_remove_recursive(dmadev->debugfs);
Is that a problem?
thanks,
greg k-h
On 6/12/2019 11:39 AM, Greg Kroah-Hartman wrote:
>> Interesting. Wouldn't debugfs_create_file() blow up if dir is NULL
>> for some reason?
> It will create a file in the root of debugfs. But how will that happen?
> debugfs_create_dir() can not return NULL.
I see.
>
>> + debugfs_create_file("stats", S_IRUGO, dir, chan,
>> + &hidma_chan_fops);
>>
>> Note that code ignores the return value of hidma_debug_init();
>> It was just trying to do clean up on debugfs failure by calling
>>
>> debugfs_remove_recursive(dmadev->debugfs);
> Is that a problem?
I just wanted to double check. You probably want to remove the return
value on debugfs_create_file() to prevent others from doing the same
thing.
Acked-by: Sinan Kaya <[email protected]>
On Wed, Jun 12, 2019 at 12:17:31PM -0400, Sinan Kaya wrote:
> On 6/12/2019 11:39 AM, Greg Kroah-Hartman wrote:
> >> Interesting. Wouldn't debugfs_create_file() blow up if dir is NULL
> >> for some reason?
> > It will create a file in the root of debugfs. But how will that happen?
> > debugfs_create_dir() can not return NULL.
>
> I see.
>
> >
> >> + debugfs_create_file("stats", S_IRUGO, dir, chan,
> >> + &hidma_chan_fops);
> >>
> >> Note that code ignores the return value of hidma_debug_init();
> >> It was just trying to do clean up on debugfs failure by calling
> >>
> >> debugfs_remove_recursive(dmadev->debugfs);
> > Is that a problem?
>
> I just wanted to double check. You probably want to remove the return
> value on debugfs_create_file() to prevent others from doing the same
> thing.
That is my long-term goal, I have a ways to go still :)
> Acked-by: Sinan Kaya <[email protected]>
Great, thanks for the review.
greg k-h
On 12-06-19, 14:25, Greg Kroah-Hartman wrote:
> No need to check the return value of debugfs_create_file(), so no need
> to provide a fake "cast away" of the return value either.
Applied all after fixing the subsystem tag (dmaengine), thanks
--
~Vinod
On Fri, Jun 14, 2019 at 11:16:13AM +0530, Vinod Koul wrote:
> On 12-06-19, 14:25, Greg Kroah-Hartman wrote:
> > No need to check the return value of debugfs_create_file(), so no need
> > to provide a fake "cast away" of the return value either.
>
> Applied all after fixing the subsystem tag (dmaengine), thanks
Sorry about that, and thanks!
greg k-h
On Fri, Jun 14, 2019 at 11:16:13AM +0530, Vinod Koul wrote:
> On 12-06-19, 14:25, Greg Kroah-Hartman wrote:
> > No need to check the return value of debugfs_create_file(), so no need
> > to provide a fake "cast away" of the return value either.
>
> Applied all after fixing the subsystem tag (dmaengine), thanks
Oops, messed that up, sorry. Thanks for applying them!
greg k-h
Greg Kroah-Hartman <[email protected]> writes:
Hi Greg,
> When calling debugfs functions, there is no need to ever check the
> return value. The function can work or not, but the code logic should
> never do something different based on this.
>
> Also, because there is no need to save the file dentry, remove the
> variable that was saving it as it was never even being used once set.
>
> Cc: Daniel Mack <[email protected]>
> Cc: Haojian Zhuang <[email protected]>
> Cc: Robert Jarzmik <[email protected]>
> Cc: Vinod Koul <[email protected]>
> Cc: Dan Williams <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Greg Kroah-Hartman <[email protected]>
> ---
> drivers/dma/pxa_dma.c | 56 +++++++++----------------------------------
> 1 file changed, 11 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/dma/pxa_dma.c b/drivers/dma/pxa_dma.c
> index b429642f3e7a..0f698f49ee26 100644
> --- a/drivers/dma/pxa_dma.c
> +++ b/drivers/dma/pxa_dma.c
> @@ -132,7 +132,6 @@ struct pxad_device {
> spinlock_t phy_lock; /* Phy association */
> #ifdef CONFIG_DEBUG_FS
> struct dentry *dbgfs_root;
> - struct dentry *dbgfs_state;
> struct dentry **dbgfs_chan;
> #endif
> };
> @@ -326,31 +325,18 @@ static struct dentry *pxad_dbg_alloc_chan(struct pxad_device *pdev,
> int ch, struct dentry *chandir)
> {
> char chan_name[11];
> - struct dentry *chan, *chan_state = NULL, *chan_descr = NULL;
> - struct dentry *chan_reqs = NULL;
> + struct dentry *chan;
> void *dt;
>
> scnprintf(chan_name, sizeof(chan_name), "%d", ch);
> chan = debugfs_create_dir(chan_name, chandir);
> dt = (void *)&pdev->phys[ch];
>
> - if (chan)
> - chan_state = debugfs_create_file("state", 0400, chan, dt,
> - &chan_state_fops);
> - if (chan_state)
> - chan_descr = debugfs_create_file("descriptors", 0400, chan, dt,
> - &descriptors_fops);
> - if (chan_descr)
> - chan_reqs = debugfs_create_file("requesters", 0400, chan, dt,
> - &requester_chan_fops);
> - if (!chan_reqs)
> - goto err_state;
> + debugfs_create_file("state", 0400, chan, dt, &chan_state_fops);
> + debugfs_create_file("descriptors", 0400, chan, dt, &descriptors_fops);
> + debugfs_create_file("requesters", 0400, chan, dt, &requester_chan_fops);
This is not strictly equivalent.
Imagine that the debugfs_create_dir() fails and returns NULL :
- in the former case, neither "state", "descriptors" nor "requesters" would be
created
- in the new code, "state", "descriptors" nor "requesters" will be created in
the debugfs root directory
Apart from that it looks fine.
Cheers.
--
Robert
On Sat, Aug 10, 2019 at 09:27:26PM +0200, Robert Jarzmik wrote:
> Greg Kroah-Hartman <[email protected]> writes:
>
> Hi Greg,
>
> > When calling debugfs functions, there is no need to ever check the
> > return value. The function can work or not, but the code logic should
> > never do something different based on this.
> >
> > Also, because there is no need to save the file dentry, remove the
> > variable that was saving it as it was never even being used once set.
> >
> > Cc: Daniel Mack <[email protected]>
> > Cc: Haojian Zhuang <[email protected]>
> > Cc: Robert Jarzmik <[email protected]>
> > Cc: Vinod Koul <[email protected]>
> > Cc: Dan Williams <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> > Signed-off-by: Greg Kroah-Hartman <[email protected]>
> > ---
> > drivers/dma/pxa_dma.c | 56 +++++++++----------------------------------
> > 1 file changed, 11 insertions(+), 45 deletions(-)
> >
> > diff --git a/drivers/dma/pxa_dma.c b/drivers/dma/pxa_dma.c
> > index b429642f3e7a..0f698f49ee26 100644
> > --- a/drivers/dma/pxa_dma.c
> > +++ b/drivers/dma/pxa_dma.c
> > @@ -132,7 +132,6 @@ struct pxad_device {
> > spinlock_t phy_lock; /* Phy association */
> > #ifdef CONFIG_DEBUG_FS
> > struct dentry *dbgfs_root;
> > - struct dentry *dbgfs_state;
> > struct dentry **dbgfs_chan;
> > #endif
> > };
> > @@ -326,31 +325,18 @@ static struct dentry *pxad_dbg_alloc_chan(struct pxad_device *pdev,
> > int ch, struct dentry *chandir)
> > {
> > char chan_name[11];
> > - struct dentry *chan, *chan_state = NULL, *chan_descr = NULL;
> > - struct dentry *chan_reqs = NULL;
> > + struct dentry *chan;
> > void *dt;
> >
> > scnprintf(chan_name, sizeof(chan_name), "%d", ch);
> > chan = debugfs_create_dir(chan_name, chandir);
> > dt = (void *)&pdev->phys[ch];
> >
> > - if (chan)
> > - chan_state = debugfs_create_file("state", 0400, chan, dt,
> > - &chan_state_fops);
> > - if (chan_state)
> > - chan_descr = debugfs_create_file("descriptors", 0400, chan, dt,
> > - &descriptors_fops);
> > - if (chan_descr)
> > - chan_reqs = debugfs_create_file("requesters", 0400, chan, dt,
> > - &requester_chan_fops);
> > - if (!chan_reqs)
> > - goto err_state;
> > + debugfs_create_file("state", 0400, chan, dt, &chan_state_fops);
> > + debugfs_create_file("descriptors", 0400, chan, dt, &descriptors_fops);
> > + debugfs_create_file("requesters", 0400, chan, dt, &requester_chan_fops);
>
> This is not strictly equivalent.
> Imagine that the debugfs_create_dir() fails and returns NULL :
How can that happen?
> - in the former case, neither "state", "descriptors" nor "requesters" would be
> created
> - in the new code, "state", "descriptors" nor "requesters" will be created in
> the debugfs root directory
I agree, but debugfs_create_dir() does not return a NULL on an error
since many kernel releases. Neither can debugfs_create_file() so really
this test is not working at all as-is :)
thanks,
greg k-h
Greg Kroah-Hartman <[email protected]> writes:
> On Sat, Aug 10, 2019 at 09:27:26PM +0200, Robert Jarzmik wrote:
>> Greg Kroah-Hartman <[email protected]> writes:
>>
>> This is not strictly equivalent.
>> Imagine that the debugfs_create_dir() fails and returns NULL :
> How can that happen?
Well in v5.0-rc1 that could happen ... unfortunately that's also the code I
checked ...
>> - in the former case, neither "state", "descriptors" nor "requesters" would be
>> created
>> - in the new code, "state", "descriptors" nor "requesters" will be created in
>> the debugfs root directory
>
> I agree, but debugfs_create_dir() does not return a NULL on an error
> since many kernel releases. Neither can debugfs_create_file() so really
> this test is not working at all as-is :)
Ah yes, you're right, I wasn't aware of the debugfs changes ...
But checking a bit further, your original mail is 2 monthes old, and this patch
was already merged in v5.2. I probably fell in a time-space anomaly, as I
received this mail only a couple of days ago.
Have a nice day.
--
Robert