Re: [PATCH v4 03/10] mfd: cs5535-mfd: Request shared IO regions centrally

Daniel Thompson
2019-11-01T08:02:05Z

On Fri, Nov 01, 2019 at 07:45:11AM +0000, 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>

Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>

> ---
> drivers/mfd/cs5535-mfd.c | 74 ++++++++++++++++++----------------------
> 1 file changed, 33 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/mfd/cs5535-mfd.c b/drivers/mfd/cs5535-mfd.c
> index b35f1efa01f6..3b569b231510 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,11 @@ static int cs5535_mfd_probe(struct pci_dev *pdev,
> static void cs5535_mfd_remove(struct pci_dev *pdev)
> {
> mfd_remove_devices(&pdev->dev);
> +
> + if (machine_is_olpc())
> + pci_release_region(pdev, ACPI_BAR);
> +
> + pci_release_region(pdev, PMS_BAR);
> pci_disable_device(pdev);
> }
>
> --
> 2.17.1
>