lkml.org 
[lkml]   [2018]   [Apr]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [RFC PATCH 2/2] HISI LPC: Add PNP device support
    From
    Date
    On 20/04/2018 13:50, Andy Shevchenko wrote:
    > On Fri, 2018-04-20 at 18:07 +0800, John Garry wrote:
    >> Currently the driver creates an per-ACPI device mfd_cell
    >> for child devices. This does not suit devices which are
    >> PNP-compatible, as we expect PNP-compatible devices to
    >> derive PNP devices.
    >>
    >> To add PNP device support, we continue to allow the PNP
    >> scan code to create the PNP device (which have the
    >> enumeration_by_parent flag set), but expect the PNP
    >> scan to defer adding the device to allow the host probe
    >> code to do this. In addition, no longer do we create an
    >> mfd_cell (platform_device) for PNP-compatible devices.
    >>
    >> We take this approach so that host probe code can
    >> translate the IO resources of the PNP device prior
    >> to adding the device.
    >

    Hi Andy,

    Thanks for checking this.

    >> + list_for_each_entry(child, &adev->children, node) {
    >> + if (acpi_is_pnp_device(child))
    >> + continue;
    >
    > This is good candidate for a separate helper macro
    >
    > #define for_each_acpi_non_pnp_device(child, adev) \
    > ...

    Right, I did consider this, but was holding off refining until I get
    past RFC stage. In fact, we could also process each child entry in one
    loop, like this:

    list_for_each_entry(child, &adev->children, node) {
    if (acpi_is_pnp_device(child)) {
    /* Do PNP compatible device work */

    } else {
    /* otherwise, make an MFD cell ... */
    }

    >
    > (see, for example, for_each_pci_bridge() implementation as an example)
    >
    >
    >> + list_for_each_entry(child, &adev->children, node) {
    >
    >> + if (!acpi_is_pnp_device(child))
    >> + continue;
    >
    > Ditto.
    >

    ok

    >> + /*
    >> + * Prior to adding the device, we need to translate
    >> the
    >> + * resources to logical PIO addresses.
    >> + */
    >> + list_for_each_entry(pnp_res, &pnp_dev->resources,
    >> list) {
    >> + struct resource *res = &pnp_res->res;
    >> +
    >
    >> + if (res->flags | IORESOURCE_IO)
    >
    > What does this mean?

    Here we check the resource flag for each resource for this PNP device -
    for IO resources we must translate the resource value from the bus
    address to the logical PIO address.

    >
    >> + hisi_lpc_acpi_xlat_io_res(child,
    >> adev, res);
    >> + }
    >

    Regards,
    John


    \
     
     \ /
      Last update: 2018-04-20 15:10    [W:3.532 / U:0.336 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site