lkml.org 
[lkml]   [2019]   [Jun]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 4/5] bus: hisi_lpc: Add .remove method to avoid driver unbind crash
    [+cc Rafael, linux-acpi]

    On Thu, Jun 20, 2019 at 06:31:55PM +0800, John Garry wrote:
    > The original driver author seemed to be under the impression that a driver
    > cannot be removed if it does not have a .remove method. Or maybe if it is
    > a built-in platform driver.
    >
    > This is not true. This crash can be created:
    >
    > root@ubuntu:/sys/bus/platform/drivers/hisi-lpc# echo HISI0191\:00 > unbind
    > root@ubuntu:/sys/bus/platform/drivers/hisi-lpc# ipmitool raw 6 1
    > Unable to handle kernel paging request at virtual address ffff000010035010
    > ...

    > The problem here is that the host goes away but the associated logical PIO
    > region remains registered, as do the child devices.
    >
    > Fix by adding a .remove method to tidy-up by removing the child devices
    > and unregistering the logical PIO region.
    >
    > Fixes: adf38bb0b595 ("HISI LPC: Support the LPC host on Hip06/Hip07 with DT bindings")
    > Signed-off-by: John Garry <john.garry@huawei.com>
    > ---
    > drivers/bus/hisi_lpc.c | 34 ++++++++++++++++++++++++++++++++--
    > 1 file changed, 32 insertions(+), 2 deletions(-)
    >
    > diff --git a/drivers/bus/hisi_lpc.c b/drivers/bus/hisi_lpc.c
    > index 6d301aafcad2..0e9f1f141c93 100644
    > --- a/drivers/bus/hisi_lpc.c
    > +++ b/drivers/bus/hisi_lpc.c
    > @@ -456,6 +456,17 @@ struct hisi_lpc_acpi_cell {
    > size_t pdata_size;
    > };
    >
    > +static void hisi_lpc_acpi_remove(struct device *hostdev)
    > +{
    > + struct acpi_device *adev = ACPI_COMPANION(hostdev);
    > + struct acpi_device *child;
    > +
    > + device_for_each_child(hostdev, NULL, hisi_lpc_acpi_remove_subdev);
    > +
    > + list_for_each_entry(child, &adev->children, node)
    > + acpi_device_clear_enumerated(child);

    There are only two other non-ACPI core callers of
    acpi_device_clear_enumerated() (i2c and spi). That always makes me
    wonder if we're getting too deep in ACPI internals.

    > +}
    > +
    > /*
    > * hisi_lpc_acpi_probe - probe children for ACPI FW
    > * @hostdev: LPC host device pointer
    > @@ -555,8 +566,7 @@ static int hisi_lpc_acpi_probe(struct device *hostdev)
    > return 0;
    >
    > fail:
    > - device_for_each_child(hostdev, NULL,
    > - hisi_lpc_acpi_remove_subdev);
    > + hisi_lpc_acpi_remove(hostdev);
    > return ret;
    > }
    >
    > @@ -626,6 +636,8 @@ static int hisi_lpc_probe(struct platform_device *pdev)
    > return ret;
    > }
    >
    > + dev_set_drvdata(dev, lpcdev);
    > +
    > io_end = lpcdev->io_host->io_start + lpcdev->io_host->size;
    > dev_info(dev, "registered range [%pa - %pa]\n",
    > &lpcdev->io_host->io_start, &io_end);
    > @@ -633,6 +645,23 @@ static int hisi_lpc_probe(struct platform_device *pdev)
    > return ret;
    > }
    >
    > +static int hisi_lpc_remove(struct platform_device *pdev)
    > +{
    > + struct device *dev = &pdev->dev;
    > + struct acpi_device *acpi_device = ACPI_COMPANION(dev);
    > + struct hisi_lpc_dev *lpcdev = dev_get_drvdata(dev);
    > + struct logic_pio_hwaddr *range = lpcdev->io_host;
    > +
    > + if (acpi_device)
    > + hisi_lpc_acpi_remove(dev);
    > + else
    > + of_platform_depopulate(dev);
    > +
    > + logic_pio_unregister_range(range);
    > +
    > + return 0;
    > +}
    > +
    > static const struct of_device_id hisi_lpc_of_match[] = {
    > { .compatible = "hisilicon,hip06-lpc", },
    > { .compatible = "hisilicon,hip07-lpc", },
    > @@ -646,5 +675,6 @@ static struct platform_driver hisi_lpc_driver = {
    > .acpi_match_table = ACPI_PTR(hisi_lpc_acpi_match),
    > },
    > .probe = hisi_lpc_probe,
    > + .remove = hisi_lpc_remove,
    > };
    > builtin_platform_driver(hisi_lpc_driver);
    > --
    > 2.17.1
    >

    \
     
     \ /
      Last update: 2019-06-21 15:56    [W:2.635 / U:0.028 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site