linux-kernel - Re: [PATCH v4 37/38] platform/x86: intel_pmc

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:   Wed, 22 Jan 2020 14:53:00 +0200
From:   Mika Westerberg <mika.westerberg@...ux.intel.com>
To:     Lee Jones <lee.jones@...aro.org>
Cc:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Darren Hart <dvhart@...radead.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        "H . Peter Anvin" <hpa@...or.com>, x86@...nel.org,
        Zha Qipeng <qipeng.zha@...el.com>,
        "David E . Box" <david.e.box@...ux.intel.com>,
        Guenter Roeck <linux@...ck-us.net>,
        Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Wim Van Sebroeck <wim@...ux-watchdog.org>,
        Mark Brown <broonie@...nel.org>,
        platform-driver-x86@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 37/38] platform/x86: intel_pmc_ipc: Convert to MFD

On Wed, Jan 22, 2020 at 12:34:54PM +0000, Lee Jones wrote:
> > +static int intel_pmc_probe(struct platform_device *pdev)
> > +{
> > +	struct intel_scu_ipc_pdata pdata = {};
> > +	struct intel_pmc_dev *pmc;
> > +	int ret;
> > +
> > +	pmc = devm_kzalloc(&pdev->dev, sizeof(*pmc), GFP_KERNEL);
> > +	if (!pmc)
> > +		return -ENOMEM;
> > +
> > +	pmc->dev = &pdev->dev;
> > +	spin_lock_init(&pmc->gcr_lock);
> > +
> > +	ret = intel_pmc_get_resources(pdev, pmc, &pdata);
> > +	if (ret) {
> > +		dev_err(&pdev->dev, "Failed to request resources\n");
> > +		return ret;
> > +	}
> > +
> > +	pmc->scu = devm_intel_scu_ipc_register(&pdev->dev, &pdata);
> > +	if (IS_ERR(pmc->scu))
> > +		return PTR_ERR(pmc->scu);
> 
> *_register is better than *_probe.  If it was called that (or maybe
> *_init) initially I may have missed the issue altogether ...
> 
> However, I still think it the SCU IPC *device* needs to be a device
> driver and abide by the rules, ensuring it uses the device driver
> model/API.  As such, it should be registered and probed as a device.

Which type of device you suggest here? And which bus it should be
registered to? I think we can make this create a platform_device but
then we would need to do that from the PCI driver as well which seems
unnecessary since we already have the struct pci_dev.

For instance in drivers/mfd/intel-lpss* we use similar approach (the
core part is library that gets called by probe drivers (ACPI, PCI). We
don't create any additional platform_devices.

There is another twist. Ideally we would like to see the SCU IPC probed
and intialized before the MFD children so that we know the SCU IPC is
ready by the time the children devices are created. I guess we could
work it around by returning -EPROBE_DEFER but that does not feel right
to be honest.

Thanks!