| Message ID | 20191024163832.31326-4-lee.jones@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Simplify MFD Core | expand |
On Thu, Oct 24, 2019 at 05:38:25PM +0100, Lee Jones wrote: > Prior to this patch, IO regions were requested via an MFD subsytem-level > .enable() call-back and similarly released by a .disable() call-back. > Double requests/releases were avoided by a centrally handled usage count > mechanism. > > This complexity can all be avoided by handling IO regions only once during > .probe() and .remove() of the parent device. Since this is the only > legitimate user of the aforementioned usage count mechanism, this patch > will allow it to be removed from MFD core in subsequent steps. > > Suggested-by: Daniel Thompson <daniel.thompson@linaro.org> > Signed-off-by: Lee Jones <lee.jones@linaro.org> > --- > drivers/mfd/cs5535-mfd.c | 71 +++++++++++++++++----------------------- > 1 file changed, 30 insertions(+), 41 deletions(-) > > diff --git a/drivers/mfd/cs5535-mfd.c b/drivers/mfd/cs5535-mfd.c > index b35f1efa01f6..27fa8fa1ec9b 100644 > --- a/drivers/mfd/cs5535-mfd.c > +++ b/drivers/mfd/cs5535-mfd.c > @@ -27,38 +27,6 @@ enum cs5535_mfd_bars { > NR_BARS, > }; > > -static int cs5535_mfd_res_enable(struct platform_device *pdev) > -{ > - struct resource *res; > - > - res = platform_get_resource(pdev, IORESOURCE_IO, 0); > - if (!res) { > - dev_err(&pdev->dev, "can't fetch device resource info\n"); > - return -EIO; > - } > - > - if (!request_region(res->start, resource_size(res), DRV_NAME)) { > - dev_err(&pdev->dev, "can't request region\n"); > - return -EIO; > - } > - > - return 0; > -} > - > -static int cs5535_mfd_res_disable(struct platform_device *pdev) > -{ > - struct resource *res; > - > - res = platform_get_resource(pdev, IORESOURCE_IO, 0); > - if (!res) { > - dev_err(&pdev->dev, "can't fetch device resource info\n"); > - return -EIO; > - } > - > - release_region(res->start, resource_size(res)); > - return 0; > -} > - > static struct resource cs5535_mfd_resources[NR_BARS]; > > static struct mfd_cell cs5535_mfd_cells[] = { > @@ -81,17 +49,11 @@ static struct mfd_cell cs5535_mfd_cells[] = { > .name = "cs5535-pms", > .num_resources = 1, > .resources = &cs5535_mfd_resources[PMS_BAR], > - > - .enable = cs5535_mfd_res_enable, > - .disable = cs5535_mfd_res_disable, > }, > { > .name = "cs5535-acpi", > .num_resources = 1, > .resources = &cs5535_mfd_resources[ACPI_BAR], > - > - .enable = cs5535_mfd_res_enable, > - .disable = cs5535_mfd_res_disable, > }, > }; > > @@ -117,22 +79,47 @@ static int cs5535_mfd_probe(struct pci_dev *pdev, > r->end = pci_resource_end(pdev, bar); > } > > + err = pci_request_region(pdev, PMS_BAR, DRV_NAME); > + if (err) { > + dev_err(&pdev->dev, "Failed to request PMS_BAR's IO region\n"); > + goto err_disable; > + } > + > err = mfd_add_devices(&pdev->dev, PLATFORM_DEVID_NONE, cs5535_mfd_cells, > ARRAY_SIZE(cs5535_mfd_cells), NULL, 0, NULL); > if (err) { > dev_err(&pdev->dev, > "Failed to add CS5535 sub-devices: %d\n", err); > - goto err_disable; > + goto err_release_pms; > } > > - if (machine_is_olpc()) > - mfd_clone_cell("cs5535-acpi", olpc_acpi_clones, ARRAY_SIZE(olpc_acpi_clones)); > + if (machine_is_olpc()) { > + err = pci_request_region(pdev, ACPI_BAR, DRV_NAME); > + if (err) { > + dev_err(&pdev->dev, > + "Failed to request ACPI_BAR's IO region\n"); > + goto err_remove_devices; > + } I agree cs5535-acpi isn't used is the kernel but I think it stills fails a w.t.f. per minute test to have a mismatch between when a device is added and when it requests resources. Especially since I don't think this is transient within the patch series. > + > + err = mfd_clone_cell("cs5535-acpi", olpc_acpi_clones, > + ARRAY_SIZE(olpc_acpi_clones)); > + if (err) { > + dev_err(&pdev->dev, "Failed to clone MFD cell\n"); > + goto err_release_acpi; > + } > + } > > dev_info(&pdev->dev, "%zu devices registered.\n", > ARRAY_SIZE(cs5535_mfd_cells)); > > return 0; > > +err_release_acpi: > + pci_release_region(pdev, ACPI_BAR); > +err_remove_devices: > + mfd_remove_devices(&pdev->dev); > +err_release_pms: > + pci_release_region(pdev, PMS_BAR); > err_disable: > pci_disable_device(pdev); > return err; > @@ -141,6 +128,8 @@ static int cs5535_mfd_probe(struct pci_dev *pdev, > static void cs5535_mfd_remove(struct pci_dev *pdev) > { > mfd_remove_devices(&pdev->dev); > + pci_release_region(pdev, ACPI_BAR); This will issue a warning if !machine_is_olpc() . For the release region family of calls "the described resource region must match a currently busy region." Daniel.
diff --git a/drivers/mfd/cs5535-mfd.c b/drivers/mfd/cs5535-mfd.c index b35f1efa01f6..27fa8fa1ec9b 100644 --- a/drivers/mfd/cs5535-mfd.c +++ b/drivers/mfd/cs5535-mfd.c @@ -27,38 +27,6 @@ enum cs5535_mfd_bars { NR_BARS, }; -static int cs5535_mfd_res_enable(struct platform_device *pdev) -{ - struct resource *res; - - res = platform_get_resource(pdev, IORESOURCE_IO, 0); - if (!res) { - dev_err(&pdev->dev, "can't fetch device resource info\n"); - return -EIO; - } - - if (!request_region(res->start, resource_size(res), DRV_NAME)) { - dev_err(&pdev->dev, "can't request region\n"); - return -EIO; - } - - return 0; -} - -static int cs5535_mfd_res_disable(struct platform_device *pdev) -{ - struct resource *res; - - res = platform_get_resource(pdev, IORESOURCE_IO, 0); - if (!res) { - dev_err(&pdev->dev, "can't fetch device resource info\n"); - return -EIO; - } - - release_region(res->start, resource_size(res)); - return 0; -} - static struct resource cs5535_mfd_resources[NR_BARS]; static struct mfd_cell cs5535_mfd_cells[] = { @@ -81,17 +49,11 @@ static struct mfd_cell cs5535_mfd_cells[] = { .name = "cs5535-pms", .num_resources = 1, .resources = &cs5535_mfd_resources[PMS_BAR], - - .enable = cs5535_mfd_res_enable, - .disable = cs5535_mfd_res_disable, }, { .name = "cs5535-acpi", .num_resources = 1, .resources = &cs5535_mfd_resources[ACPI_BAR], - - .enable = cs5535_mfd_res_enable, - .disable = cs5535_mfd_res_disable, }, }; @@ -117,22 +79,47 @@ static int cs5535_mfd_probe(struct pci_dev *pdev, r->end = pci_resource_end(pdev, bar); } + err = pci_request_region(pdev, PMS_BAR, DRV_NAME); + if (err) { + dev_err(&pdev->dev, "Failed to request PMS_BAR's IO region\n"); + goto err_disable; + } + err = mfd_add_devices(&pdev->dev, PLATFORM_DEVID_NONE, cs5535_mfd_cells, ARRAY_SIZE(cs5535_mfd_cells), NULL, 0, NULL); if (err) { dev_err(&pdev->dev, "Failed to add CS5535 sub-devices: %d\n", err); - goto err_disable; + goto err_release_pms; } - if (machine_is_olpc()) - mfd_clone_cell("cs5535-acpi", olpc_acpi_clones, ARRAY_SIZE(olpc_acpi_clones)); + if (machine_is_olpc()) { + err = pci_request_region(pdev, ACPI_BAR, DRV_NAME); + if (err) { + dev_err(&pdev->dev, + "Failed to request ACPI_BAR's IO region\n"); + goto err_remove_devices; + } + + err = mfd_clone_cell("cs5535-acpi", olpc_acpi_clones, + ARRAY_SIZE(olpc_acpi_clones)); + if (err) { + dev_err(&pdev->dev, "Failed to clone MFD cell\n"); + goto err_release_acpi; + } + } dev_info(&pdev->dev, "%zu devices registered.\n", ARRAY_SIZE(cs5535_mfd_cells)); return 0; +err_release_acpi: + pci_release_region(pdev, ACPI_BAR); +err_remove_devices: + mfd_remove_devices(&pdev->dev); +err_release_pms: + pci_release_region(pdev, PMS_BAR); err_disable: pci_disable_device(pdev); return err; @@ -141,6 +128,8 @@ static int cs5535_mfd_probe(struct pci_dev *pdev, static void cs5535_mfd_remove(struct pci_dev *pdev) { mfd_remove_devices(&pdev->dev); + pci_release_region(pdev, ACPI_BAR); + pci_release_region(pdev, PMS_BAR); pci_disable_device(pdev); }
Prior to this patch, IO regions were requested via an MFD subsytem-level .enable() call-back and similarly released by a .disable() call-back. Double requests/releases were avoided by a centrally handled usage count mechanism. This complexity can all be avoided by handling IO regions only once during .probe() and .remove() of the parent device. Since this is the only legitimate user of the aforementioned usage count mechanism, this patch will allow it to be removed from MFD core in subsequent steps. Suggested-by: Daniel Thompson <daniel.thompson@linaro.org> Signed-off-by: Lee Jones <lee.jones@linaro.org> --- drivers/mfd/cs5535-mfd.c | 71 +++++++++++++++++----------------------- 1 file changed, 30 insertions(+), 41 deletions(-) -- 2.17.1