Re: [PATCH 4/6] platform/apple: Add new Apple Mac SMC driver

[Lee Jones <lee@xxxxxxxxxx>]

On Fri, 28 Oct 2022, Russell King (Oracle) wrote:

> On Mon, Sep 12, 2022 at 11:55:14AM +0100, Lee Jones wrote:
> > > I'm guessing this series is now dead, and Hector needs to re-spin the
> > > patch set according to your views. I'm guessing this is going to take
> > > a major re-work of the patch series.
> > > 
> > > I suspect my attempt and trying to get this upstream has made things
> > > more complicated, because I doubt Hector has updated his patch set
> > > with the review comments that have been made so far... so this is
> > > now quite a mess. I think, once this is sorted, the entire series
> > > will need to be re-reviewed entirely afresh.
> > 
> > I have no insight into what Hector is doing, or plans to do.
> 
> It seems there's no plans by Hector to address this, so it comes down
> to me.
> 
> So, guessing what you're after, would something like the following
> work for you? I don't see *any* point in creating more yet more
> platform devices unless we're on a mission to maximise wasted memory
> resources (which this split will already be doing by creating two
> small modules instead of one.)
> 
> Obviously, this is not an official patch yet, it's just to find out
> what code structure you are looking for.
> 
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index 78c6d9d99c3f..8d4c0508a2c8 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -18,6 +18,8 @@ obj-$(CONFIG_MFD_ENE_KB3930)	+= ene-kb3930.o
>  obj-$(CONFIG_MFD_EXYNOS_LPASS)	+= exynos-lpass.o
>  obj-$(CONFIG_MFD_GATEWORKS_GSC)	+= gateworks-gsc.o
>  
> +obj-$(CONFIG_APPLE_SMC)		+= apple-smc.o
> +
>  obj-$(CONFIG_HTC_PASIC3)	+= htc-pasic3.o
>  obj-$(CONFIG_HTC_I2CPLD)	+= htc-i2cpld.o
>  
> diff --git a/drivers/mfd/apple-smc.c b/drivers/mfd/apple-smc.c
> new file mode 100644
> index 000000000000..bc59d1c5e13d
> --- /dev/null
> +++ b/drivers/mfd/apple-smc.c
> @@ -0,0 +1,38 @@
> +#include <linux/mfd/core.h>
> +#include <linux/mfd/apple-smc.h>
> +
> +static const struct mfd_cell apple_smc_devs[] = {
> +	{
> +		.name = "macsmc-gpio",
> +		.of_compatible = "apple,smc-gpio",
> +	},
> +	{
> +		.name = "macsmc-hid",
> +	},
> +	{
> +		.name = "macsmc-power",
> +	},
> +	{
> +		.name = "macsmc-reboot",
> +	},
> +	{
> +		.name = "macsmc-rtc",
> +	},
> +};
> +
> +int apple_smc_mfd_probe(struct device *dev)
> +{
> +	return mfd_add_devices(dev, -1, apple_smc_devs,
> +			       ARRAY_SIZE(apple_smc_devs), NULL, 0, NULL);
> +}
> +EXPORT_SYMBOL(apple_smc_mfd_probe);
> +
> +void apple_smc_mfd_remove(struct device *dev)
> +{
> +	mfd_remove_devices(dev);
> +}
> +EXPORT_SYMBOL(apple_smc_mfd_remove);
> +
> +MODULE_AUTHOR("Hector Martin <marcan@xxxxxxxxx>");
> +MODULE_LICENSE("Dual MIT/GPL");
> +MODULE_DESCRIPTION("Apple SMC MFD core");

Conceptually interesting, not seen this one before, but clearly a
hack, no?  Pretty sure all of the other cores in MFD are represented
by a Platform Device.

Why not implement the inverse?  The Apple SMC is clearly an MFD, in
Linux terms, so why not move the Platform Device into here, fetch all
of the global resources, register the sub-devices, then call into the
rtkit implementation in drivers/platform? 

-- 
Lee Jones [李琼斯]