On Mon, 21 Oct 2019, Daniel Thompson wrote: > On Mon, Oct 21, 2019 at 11:58:16AM +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.thomp...@linaro.org> > > Signed-off-by: Lee Jones <lee.jo...@linaro.org> > > --- > > drivers/mfd/cs5535-mfd.c | 72 +++++++++++++++++----------------------- > > 1 file changed, 30 insertions(+), 42 deletions(-) > > > > diff --git a/drivers/mfd/cs5535-mfd.c b/drivers/mfd/cs5535-mfd.c > > index 9ce6bbcdbda1..053e33447808 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, > > }, > > [ACPI_BAR] = { > > .name = "cs5535-acpi", > > .num_resources = 1, > > .resources = &cs5535_mfd_resources[ACPI_BAR], > > - > > - .enable = cs5535_mfd_res_enable, > > - .disable = cs5535_mfd_res_disable, > > }, > > }; > > > > @@ -109,7 +71,6 @@ static int cs5535_mfd_probe(struct pci_dev *pdev, > > if (err) > > return err; > > > > - /* fill in IO range for each cell; subdrivers handle the region */ > > for (i = 0; i < NR_BARS; i++) { > > struct mfd_cell *cell = &cs5535_mfd_cells[i]; > > struct resource *r = &cs5535_mfd_resources[i]; > > @@ -122,22 +83,47 @@ static int cs5535_mfd_probe(struct pci_dev *pdev, > > r->end = pci_resource_end(pdev, i); > > } > > > > + 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 CS5532 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; > > + } > > + } > > Making the request_region() conditional on machine_is_olpc() seems to be > best on the assumption that the cs5535-acpi is not otherwise used. > > I suspect the assumption is true but you have to combine knowledge from > several bits of code to figure that out.
It is not used. Will reply to your other comment. -- Lee Jones [李琼斯] Linaro Services Technical Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog