ptp: ocp: change sysfs attr group handling

In the detach path, the driver calls sysfs_remove_group() for the
groups it believes has been registered.  However, if the group was
never previously registered, then this causes a splat.

Instead, compute the groups that should be registered in advance,
and then call sysfs_create_groups(), which registers them all at once.

Update the error handling appropriately.

Fixes: c205d53c4923 ("ptp: ocp: Add firmware capability bits for feature gating")
Reported-by: Zheyu Ma <zheyuma97@gmail.com>
Signed-off-by: Jonathan Lemon <jonathan.lemon@gmail.com>
Link: https://lore.kernel.org/r/20220517214600.10606-1-jonathan.lemon@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
index 36c0e188216b1880510ba76140bf2b362e9b6f1d..860672d6a03c04ca590aa0c809653555a3255ef7 100644 (file)
--- a/drivers/ptp/ptp_ocp.c
+++ b/drivers/ptp/ptp_ocp.c
@@ -300,7 +300,7 @@ struct ptp_ocp {
        struct platform_device  *spi_flash;
        struct clk_hw           *i2c_clk;
        struct timer_list       watchdog;
-       const struct ocp_attr_group *attr_tbl;
+       const struct attribute_group **attr_group;
        const struct ptp_ocp_eeprom_map *eeprom_map;
        struct dentry           *debug_root;
        time64_t                gnss_lost;
@@ -1836,6 +1836,42 @@ ptp_ocp_signal_init(struct ptp_ocp *bp)
                                             bp->signal_out[i]->mem);
 }
 
+static void
+ptp_ocp_attr_group_del(struct ptp_ocp *bp)
+{
+       sysfs_remove_groups(&bp->dev.kobj, bp->attr_group);
+       kfree(bp->attr_group);
+}
+
+static int
+ptp_ocp_attr_group_add(struct ptp_ocp *bp,
+                      const struct ocp_attr_group *attr_tbl)
+{
+       int count, i;
+       int err;
+
+       count = 0;
+       for (i = 0; attr_tbl[i].cap; i++)
+               if (attr_tbl[i].cap & bp->fw_cap)
+                       count++;
+
+       bp->attr_group = kcalloc(count + 1, sizeof(struct attribute_group *),
+                                GFP_KERNEL);
+       if (!bp->attr_group)
+               return -ENOMEM;
+
+       count = 0;
+       for (i = 0; attr_tbl[i].cap; i++)
+               if (attr_tbl[i].cap & bp->fw_cap)
+                       bp->attr_group[count++] = attr_tbl[i].group;
+
+       err = sysfs_create_groups(&bp->dev.kobj, bp->attr_group);
+       if (err)
+               bp->attr_group[0] = NULL;
+
+       return err;
+}
+
 static void
 ptp_ocp_sma_init(struct ptp_ocp *bp)
 {
@@ -1905,7 +1941,6 @@ ptp_ocp_fb_board_init(struct ptp_ocp *bp, struct ocp_resource *r)
        bp->flash_start = 1024 * 4096;
        bp->eeprom_map = fb_eeprom_map;
        bp->fw_version = ioread32(&bp->image->version);
-       bp->attr_tbl = fb_timecard_groups;
        bp->fw_cap = OCP_CAP_BASIC;
 
        ver = bp->fw_version & 0xffff;
@@ -1919,6 +1954,10 @@ ptp_ocp_fb_board_init(struct ptp_ocp *bp, struct ocp_resource *r)
        ptp_ocp_sma_init(bp);
        ptp_ocp_signal_init(bp);
 
+       err = ptp_ocp_attr_group_add(bp, fb_timecard_groups);
+       if (err)
+               return err;
+
        err = ptp_ocp_fb_set_pins(bp);
        if (err)
                return err;
@@ -3389,7 +3428,6 @@ ptp_ocp_complete(struct ptp_ocp *bp)
 {
        struct pps_device *pps;
        char buf[32];
-       int i, err;
 
        if (bp->gnss_port != -1) {
                sprintf(buf, "ttyS%d", bp->gnss_port);
@@ -3414,14 +3452,6 @@ ptp_ocp_complete(struct ptp_ocp *bp)
        if (pps)
                ptp_ocp_symlink(bp, pps->dev, "pps");
 
-       for (i = 0; bp->attr_tbl[i].cap; i++) {
-               if (!(bp->attr_tbl[i].cap & bp->fw_cap))
-                       continue;
-               err = sysfs_create_group(&bp->dev.kobj, bp->attr_tbl[i].group);
-               if (err)
-                       return err;
-       }
-
        ptp_ocp_debugfs_add_device(bp);
 
        return 0;
@@ -3493,15 +3523,11 @@ static void
 ptp_ocp_detach_sysfs(struct ptp_ocp *bp)
 {
        struct device *dev = &bp->dev;
-       int i;
 
        sysfs_remove_link(&dev->kobj, "ttyGNSS");
        sysfs_remove_link(&dev->kobj, "ttyMAC");
        sysfs_remove_link(&dev->kobj, "ptp");
        sysfs_remove_link(&dev->kobj, "pps");
-       if (bp->attr_tbl)
-               for (i = 0; bp->attr_tbl[i].cap; i++)
-                       sysfs_remove_group(&dev->kobj, bp->attr_tbl[i].group);
 }
 
 static void
@@ -3511,6 +3537,7 @@ ptp_ocp_detach(struct ptp_ocp *bp)
 
        ptp_ocp_debugfs_remove_device(bp);
        ptp_ocp_detach_sysfs(bp);
+       ptp_ocp_attr_group_del(bp);
        if (timer_pending(&bp->watchdog))
                del_timer_sync(&bp->watchdog);
        if (bp->ts0)