linux-kernel - Re: [PATCH 1/2] HISI LPC: Reference static MFD cells for ACPI support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 4 May 2018 13:29:53 +0100
From:   John Garry <john.garry@...wei.com>
To:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        <xuwei5@...wei.com>, <mika.westerberg@...ux.intel.com>,
        <lee.jones@...aro.org>
CC:     <rjw@...ysocki.net>, <linux-kernel@...r.kernel.org>,
        <arnd@...db.de>, <graeme.gregory@...aro.org>, <helgaas@...nel.org>,
        <z.liuxinliang@...ilicon.com>, <linuxarm@...wei.com>
Subject: Re: [PATCH 1/2] HISI LPC: Reference static MFD cells for ACPI support

On 03/05/2018 17:54, Andy Shevchenko wrote:
> On Thu, 2018-05-03 at 23:08 +0800, John Garry wrote:
>> Currently for ACPI support the driver models the host as
>> an MFD. For a device connected to the LPC bus, we dynamically
>> create an MFD cell for that device, configuring the cell
>> name and ACPI match parameters manually. This makes supporting
>> named devices and also special setup handling for certain devices
>> awkward, as we would need to introduce some special ACPI device
>> handling according to device HID.
>>
>> To avoid this, create reference static MFD cells for known
>> child devices, so when adding an MFD cell we can fix the cell
>> platform data as required. For this, a setup callback function
>> is added.

Hi Andy,

>>
>> For now, only the IPMI cell is added.
>
>> +static const struct mfd_cell *hisi_lpc_acpi_mfd_get_cell(const char
>> *hid)
>> +{
>> +	const struct hisi_lpc_acpi_mfd_cell *cell =
>> hisi_lpc_acpi_mfd_cells;
>> +
>> +	for (; cell && cell->mfd_cell.name; cell++) {
>> +		const struct mfd_cell *mfd_cell = &cell->mfd_cell;
>> +		const struct mfd_cell_acpi_match *acpi_match;
>> +
>> +		acpi_match = mfd_cell->acpi_match;
>> +		if (!strcmp(acpi_match->pnpid, hid))
>> +			return mfd_cell;
>> +	}
>> +
>> +	return NULL;
>> +}
>
> I'm not sure I understand why MFD core can't do it (as seen in lines
> drivers/mfd/core.c:105 and below).
>

Right, I think MFD core does the same.

>> +	/* allocate the mfd cells, one per child */
>> +	size = sizeof(*mfd_cells);
>>  	mfd_cells = devm_kcalloc(hostdev, cell_num, size,
>> GFP_KERNEL);
>>  	if (!mfd_cells)
>>  		return -ENOMEM;
>
> And since you have structures already, I'm not sure why you need another
> allocation for them. Only what you would need is to apply resources and
> call devm_mfd_add_devices() per each found device.

I was creating a copy as the originals are in the hisi_lpc_acpi_mfd_cell 
struct type, while we would require an array of mfd_cells.

>
>> +		cell = container_of(mfd_cell_ref, typeof(*cell),
>> mfd_cell);
>
> Why we can't iterate over inherited type of objects directly?
>

We could do.

Thanks,
John