>From b8b54ce18d724fd2c730b553a67c77f2c4a0fcf2 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Tue, 17 Aug 2021 15:25:38 +0300 Subject: [PATCH 1/1] TODO: ptp_ocp: Convert to use managed functions pcim_* and devm_* This makes the error handling much more simpler than open-coding everything and in addition makes the probe function smaller an tidier. TODO: split out unrelated changes to their own patches. Signed-off-by: Andy Shevchenko --- drivers/ptp/ptp_ocp.c | 56 ++++++++++++++++++------------------------- 1 file changed, 23 insertions(+), 33 deletions(-) diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c index 922f92637db8..d118da95a46c 100644 --- a/drivers/ptp/ptp_ocp.c +++ b/drivers/ptp/ptp_ocp.c @@ -1,6 +1,8 @@ // SPDX-License-Identifier: GPL-2.0-only /* Copyright (c) 2020 Facebook */ +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + #include #include #include @@ -303,7 +305,7 @@ static struct ocp_resource ocp_fb_resource[] = { static const struct pci_device_id ptp_ocp_pcidev_id[] = { { PCI_DEVICE_DATA(FACEBOOK, TIMECARD, &ocp_fb_resource) }, - { 0 } + { } }; MODULE_DEVICE_TABLE(pci, ptp_ocp_pcidev_id); @@ -344,6 +346,7 @@ ptp_ocp_clock_val_from_name(const char *name) for (i = 0; i < ARRAY_SIZE(ptp_ocp_clock); i++) { clk = ptp_ocp_clock[i].name; + /* FIXME: What's the point of 'n' + strlen()? */ if (!strncasecmp(name, clk, strlen(clk))) return ptp_ocp_clock[i].value; } @@ -363,6 +366,7 @@ __ptp_ocp_gettime_locked(struct ptp_ocp *bp, struct timespec64 *ts, ptp_read_system_prets(sts); iowrite32(ctrl, &bp->reg->ctrl); + /* FIXME: iopoll.h + respective macro */ for (i = 0; i < 100; i++) { ctrl = ioread32(&bp->reg->ctrl); if (ctrl & OCP_CTRL_READ_TIME_DONE) @@ -686,6 +690,9 @@ ptp_ocp_read_i2c(struct i2c_adapter *adap, u8 addr, u8 reg, u8 sz, u8 *data) reg += len; sz -= len; } + + /* FIXME: shouldn't be using word transfers then? */ + return 0; } @@ -870,21 +877,13 @@ static const struct devlink_ops ptp_ocp_devlink_ops = { .info_get = ptp_ocp_devlink_info_get, }; -static void __iomem * -__ptp_ocp_get_mem(struct ptp_ocp *bp, unsigned long start, int size) -{ - struct resource res = DEFINE_RES_MEM_NAMED(start, size, "ptp_ocp"); - - return devm_ioremap_resource(&bp->pdev->dev, &res); -} - static void __iomem * ptp_ocp_get_mem(struct ptp_ocp *bp, struct ocp_resource *r) { - unsigned long start; + resource_size_t start = pci_resource_start(bp->pdev, 0) + r->offset; + struct resource res = DEFINE_RES_MEM_NAMED(start, r->size, "ptp_ocp"); - start = pci_resource_start(bp->pdev, 0) + r->offset; - return __ptp_ocp_get_mem(bp, start, r->size); + return devm_ioremap_resource(&bp->pdev->dev, &res); } static void @@ -908,7 +907,7 @@ ptp_ocp_register_spi(struct ptp_ocp *bp, struct ocp_resource *r) struct pci_dev *pdev = bp->pdev; struct platform_device *p; struct resource res[2]; - unsigned long start; + resource_size_t start; int id; /* XXX hack to work around old FPGA */ @@ -932,7 +931,7 @@ ptp_ocp_register_spi(struct ptp_ocp *bp, struct ocp_resource *r) id += info->pci_offset; p = platform_device_register_resndata(&pdev->dev, info->name, id, - res, 2, info->data, + res, ARRAY_SIZE(res), info->data, info->data_size); if (IS_ERR(p)) return PTR_ERR(p); @@ -1036,7 +1035,6 @@ ptp_ocp_unregister_ext(struct ptp_ocp_ext_src *ext) { ext->info->enable(ext, false); pci_free_irq(ext->bp->pdev, ext->irq_vec, ext); - kfree(ext); } static int @@ -1046,14 +1044,13 @@ ptp_ocp_register_ext(struct ptp_ocp *bp, struct ocp_resource *r) struct ptp_ocp_ext_src *ext; int err; - ext = kzalloc(sizeof(*ext), GFP_KERNEL); + ext = devm_kzalloc(&pdev->dev, sizeof(*ext), GFP_KERNEL); if (!ext) return -ENOMEM; - err = -EINVAL; ext->mem = ptp_ocp_get_mem(bp, r); if (!ext->mem) - goto out; + return -EINVAL; ext->bp = bp; ext->info = r->extra; @@ -1063,16 +1060,12 @@ ptp_ocp_register_ext(struct ptp_ocp *bp, struct ocp_resource *r) ext, "ocp%d.%s", bp->id, ext->info->name); if (err) { dev_err(&pdev->dev, "Could not get irq %d\n", r->irq_vec); - goto out; + return err; } bp_assign_entry(bp, r, ext); return 0; - -out: - kfree(ext); - return err; } static int @@ -1240,7 +1233,7 @@ static struct attribute *timecard_attrs[] = { &dev_attr_gnss_sync.attr, &dev_attr_clock_source.attr, &dev_attr_available_clock_sources.attr, - NULL, + NULL }; ATTRIBUTE_GROUPS(timecard); @@ -1430,7 +1423,7 @@ ptp_ocp_probe(struct pci_dev *pdev, const struct pci_device_id *id) if (err) goto out_free; - err = pci_enable_device(pdev); + err = pcim_enable_device(pdev); if (err) { dev_err(&pdev->dev, "pci_enable_device\n"); goto out_unregister; @@ -1439,7 +1432,7 @@ ptp_ocp_probe(struct pci_dev *pdev, const struct pci_device_id *id) bp = devlink_priv(devlink); err = ptp_ocp_device_init(bp, pdev); if (err) - goto out_disable; + goto out_unregister; /* compat mode. * Older FPGA firmware only returns 2 irq's. @@ -1456,7 +1449,7 @@ ptp_ocp_probe(struct pci_dev *pdev, const struct pci_device_id *id) err = ptp_ocp_register_resources(bp, id->driver_data); if (err) - goto out; + goto out_free_irq; bp->ptp = ptp_clock_register(&bp->ptp_info, &pdev->dev); if (IS_ERR(bp->ptp)) { @@ -1477,9 +1470,8 @@ ptp_ocp_probe(struct pci_dev *pdev, const struct pci_device_id *id) out: ptp_ocp_detach(bp); - pci_set_drvdata(pdev, NULL); -out_disable: - pci_disable_device(pdev); +out_free_irq: + pci_free_irq_vectors(pdev); out_unregister: devlink_unregister(devlink); out_free: @@ -1495,8 +1487,6 @@ ptp_ocp_remove(struct pci_dev *pdev) struct devlink *devlink = priv_to_devlink(bp); ptp_ocp_detach(bp); - pci_set_drvdata(pdev, NULL); - pci_disable_device(pdev); devlink_unregister(devlink); devlink_free(devlink); @@ -1577,7 +1567,7 @@ ptp_ocp_init(void) out_notifier: class_unregister(&timecard_class); out: - pr_err(KBUILD_MODNAME ": failed to register %s: %d\n", what, err); + pr_err("failed to register %s: %d\n", what, err); return err; } -- 2.32.0