regulator: Fix and simplify debugfs support

If CONFIG_DEBUG_FS=y debugfs functions will never return an ERR_PTR. Instead they'll return NULL. The intent is to remove ifdefs in calling code.

Instead of checking for an ERR_PTR check for NULL. This simplifies the code and also fixes an error check that would never have worked otherwise. While we're here modernize the code to use S_IRUGO instead of 0444.

Signed-off-by Stephen Boyd <[email protected]>

---

If we're willing to sacrifice a pointer per rdev we can remove the ifdefs and the compiler should be able to optimize away the dead code.

 drivers/regulator/core.c |   15 +++++----------

 1 file changed, 5 insertions(+), 10 deletions(-)

drivers/regulator/core.c +5 -11

index e9a83f8..f20696d 100644
@@ -1145,9 +1145,8 @@ static struct regulator *create_regulator(struct regulator_dev *rdev,
  #ifdef CONFIG_DEBUG_FS
        regulator->debugfs = debugfs_create_dir(regulator->supply_name,
                                                rdev->debugfs);
-       if (IS_ERR_OR_NULL(regulator->debugfs)) {
+       if (!regulator->debugfs) {
                rdev_warn(rdev, "Failed to create debugfs directory\n");
-               regulator->debugfs = NULL;
        } else {
                debugfs_create_u32("uA_load", 0444, regulator->debugfs,
                                   &regulator->uA_load);

@@ -2712,9 +2711,8 @@ static void rdev_init_debugfs(struct regulator_dev *rdev)
  {
  #ifdef CONFIG_DEBUG_FS
        rdev->debugfs = debugfs_create_dir(rdev_get_name(rdev), debugfs_root);
-       if (IS_ERR(rdev->debugfs) || !rdev->debugfs) {
+       if (!rdev->debugfs) {
                rdev_warn(rdev, "Failed to create debugfs directory\n");
-               rdev->debugfs = NULL;
                return;
        }
 

@@ -3129,13 +3127,10 @@ static int __init regulator_init(void)
 
  #ifdef CONFIG_DEBUG_FS
        debugfs_root = debugfs_create_dir("regulator", NULL);
-       if (IS_ERR(debugfs_root) || !debugfs_root) {
+       if (!debugfs_root)
                pr_warn("regulator: Failed to create debugfs directory\n");
-               debugfs_root = NULL;
-       }
-
-       if (IS_ERR(debugfs_create_file("supply_map", 0444, debugfs_root,
-                                      NULL, &supply_map_fops)))
+       else if (!debugfs_create_file("supply_map", S_IRUGO, debugfs_root,
+                                      NULL, &supply_map_fops))
                pr_warn("regulator: Failed to create supplies debugfs\n");
  #endif
 
--

Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

If CONFIG_DEBUG_FS=y debugfs functions will never return an ERR_PTR. Instead they'll return NULL. The intent is to remove ifdefs in calling code.

Instead of checking for an ERR_PTR check for NULL. This simplifies the code and also fixes an error check that would never have worked otherwise. While we're here modernize the code to use S_IRUGO instead of 0444.

If we're willing to sacrifice a pointer per rdev we can remove the ifdefs and the compiler should be able to optimize away the dead code.

Quoted 7 lines · Stephen Boyd

If CONFIG_DEBUG_FS=y debugfs functions will never return an ERR_PTR. Instead they'll return NULL. The intent is to remove ifdefs in calling code.
Instead of checking for an ERR_PTR check for NULL. This simplifies the code and also fixes an error check that would never have worked otherwise. While we're here modernize the code to use S_IRUGO instead of 0444.

This was actually a deliberate decision to make the code more robust against change - the IS_ERR_OR_NULL doesn't make the code any bigger but it means it's less likely to break in the face of changes.

Quoted 3 lines

If we're willing to sacrifice a pointer per rdev we can remove the ifdefs and the compiler should be able to optimize away the dead code.

Personally I'd be happy to do that, the only reason I put the ifdefs in there was that it appears to be idiomatic to do so but I'm not really a big fan of it. Then again I never build kernls without debugfs support in them myself...

Quoted 3 lines · Mark Brown

This was actually a deliberate decision to make the code more robust against change - the IS_ERR_OR_NULL doesn't make the code any bigger but it means it's less likely to break in the face of changes.

More robust? The debugfs code in the regulator core looks confused on what the return value is. Sometimes it's IS_ERR, sometimes it's NULL, sometimes it's both. Might as well clean it up to be consistent and proper.