Re: [RFC PATCH 2/2] HISI LPC: Add PNP device support

[John Garry]

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