[PATCH 4/6] platform/apple: Add new Apple Mac SMC driver
Hector Martin
marcan at marcan.st
Mon Oct 31 01:58:05 PDT 2022
On 31/10/2022 17.48, Lee Jones wrote:
> On Sat, 29 Oct 2022, Hector Martin wrote:
>
>> On 09/09/2022 16.50, Lee Jones wrote:
>>>> What's the point of just having effectively an array of mfd_cell and
>>>> wrappers to call into the mfd core in the drivers/mfd/ tree and the
>>>> rest of the driver elsewhere?
>>>
>>> They should be separate drivers, with MFD registering the Platform.
>>
>> Why? What purpose does this serve? I'm still confused. There's one
>> parent device, which provides services to the child devices. There isn't
>> one parent device which wraps a platform service which is used by
>> children. This makes no sense. The platform device is the root, if it
>> exposes MFD services, then it has to be in that direction, not the other
>> way around.
>>
>> Look at how this patch series is architected. There is smc_core.c, which
>> implements SMC helpers and wrappers on top of a generic backend, and
>> registers with the MFD subsystem. And then there is smc_rtkit.c which is
>> the actual platform implementation on top of the RTKit framework, and is
>> the actual platform device entry point.
>>
>> A priori, the only thing that makes sense to me right now would be to
>> move smc_core.c into drivers/mfd, and leave smc_rtkit.c in platform.
>> That way the mfd registration would be in drivers/mfd (as would be the
>> services offered to sub-drivers), but the actual backend implementation
>> would be in platform/ (and there would eventually be others, e.g. at
>> least two more for x86 systems). That does mean that the driver entry
>> point will be in platform/, with mfd/smc_core.c serving as effectively
>> library code to plumb in the mfd stuff into one of several possible
>> platform devices. Would that work for you?
>
> Yes, sounds sensible. However, keep all of the abstraction craziness
> somewhere else and fetch and share all of your shared resources from
> the MFD (SMC) driver.
I'm not sure what you mean by that. The abstraction (smc_core.c) *is*
the shared resource. All it does is wrap ops callbacks with a mutex and
add a couple helpers for finding keys. Do you literally want us to just
have this in drivers/mfd?
// SPDX-License-Identifier: GPL-2.0-only OR MIT
/*
* Apple SMC MFD wrapper
* Copyright The Asahi Linux Contributors
*/
#include <linux/device.h>
#include "smc.h"
static const struct mfd_cell apple_smc_devs[] = {
{
.name = "macsmc-gpio",
},
{
.name = "macsmc-hid",
},
{
.name = "macsmc-power",
},
{
.name = "macsmc-reboot",
},
{
.name = "macsmc-rtc",
},
};
int apple_smc_add_mfd_devices(struct device *dev)
{
ret = mfd_add_devices(dev, -1, apple_smc_devs,
ARRAY_SIZE(apple_smc_devs), NULL, 0, NULL);
if (ret)
return dev_err_probe(dev, ret, "Subdevice initialization failed");
return 0;
}
EXPORT_SYMBOL(apple_smc_add_mfd_devices);
int apple_smc_remove_mfd_devices(struct device *dev)
{
mfd_remove_devices(smc->dev);
return 0;
}
EXPORT_SYMBOL(apple_smc_add_mfd_devices);
MODULE_AUTHOR("Hector Martin <marcan at marcan.st>");
MODULE_LICENSE("Dual MIT/GPL");
MODULE_DESCRIPTION("Apple SMC MFD wrapper");
Because this feels *immensely* silly and pointless.
- Hector
More information about the linux-arm-kernel
mailing list