lkml.org 
[lkml]   [2018]   [May]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 1/2] HISI LPC: Reference static MFD cells for ACPI support
From
Date
On 04/05/2018 11:03, Lee Jones wrote:
> On Fri, 04 May 2018, John Garry wrote:
>
>> On 04/05/2018 10:02, Lee Jones wrote:
>>> On Thu, 03 May 2018, 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.
>>>>>
>>>>> 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).
>>>
>>
>> Hi Lee,
>>
>>> You shouldn't be using the MFD API outside of MFD anyway. Either it
>>> is an MFD driver, or it isn't. If it is, please move it. If it's not,
>>> please don't use the API.
>>
>> We're modelling as an MFD, but it's not an MFD in the classic sense. We're
>> just using the MFD API for convenience (and to avoid code duplication), as
>> the MFD API does what we require for this driver.
>
> I know what you're doing, and it's wrong. ;)
>
>>> My current suspicion is that the driver needs splitting and only part
>>> of it ends up in MFD.
>>
>> How would you propose splitting the driver? By adding a lib function
>> specific for this driver for the ACPI probe?
>
> Look at:
>
> drivers/platform/chrome/cros_ec_lpc.c
>
> and
>
> drivers/mfd/cros_ec.c
>

Right, I see, something similar to what I suggested.

I don't really see a point in splitting the driver across drivers/mfd
and drivers/bus, and introducing dependencies. This is more especially
considering this is a legacy host controller with no potential future
developments, and not worth the effort.

If you feel strongly enough about not using the MFD API outside
drivers/mfd, then I'll look at other solutions, like using platform
device APIs directly.

Cheers,
John




\
 
 \ /
  Last update: 2018-05-04 12:22    [W:0.051 / U:1.440 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site