linux-kernel - Re: [PATCH] pwm: add support for Intel Low Power Subsystem PWM

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]

Message-ID: <20140227131337.GN5018@intel.com>
Date:	Thu, 27 Feb 2014 15:13:37 +0200
From:	Mika Westerberg <mika.westerberg@...ux.intel.com>
To:	"Chew, Chiau Ee" <chiau.ee.chew@...el.com>
Cc:	Thierry Reding <thierry.reding@...il.com>,
	"linux-pwm@...r.kernel.org" <linux-pwm@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"Chew, Kean Ho" <kean.ho.chew@...el.com>,
	"Chang, Rebecca Swee Fun" <rebecca.swee.fun.chang@...el.com>
Subject: Re: [PATCH] pwm: add support for Intel Low Power Subsystem PWM

On Thu, Feb 27, 2014 at 09:03:41AM +0000, Chew, Chiau Ee wrote:
> > > > +static int pwm_lpss_probe(struct platform_device *pdev) {
> > > > +	struct device *dev = &pdev->dev;
> > > > +	struct pwm_lpss_chip *lpwm;
> > > > +	int ret;
> > > > +
> > > > +	lpwm = dev_get_platdata(dev);
> > >
> > > struct pwm_lpss_chip is defined in this source file, how can anybody
> > > else know what to fill platform_data with?
> > 
> > Good point. Chiau Ee, do you recall why that thing is there in the first place?
> 
> This is added because we would eventually want to have PCI enumeration support
> added. I think we shall find a way to better enable both the ACPI enumeration and
> PCI enumeration support.

Agreed. For now, can you please drop the platform data thing from the
driver? Thanks.

> > > > +	if (!lpwm) {
> > > > +		lpwm = pwm_lpss_acpi_get_pdata(pdev);
> > > > +		if (!lpwm)
> > > > +			return -ENODEV;
> > > > +	}
> > > [...]
> > > > +static struct platform_driver pwm_lpss_driver = {
> > > > +	.driver = {
> > > > +		.name = "pwm-lpss",
> > > > +		.acpi_match_table = ACPI_PTR(pwm_lpss_acpi_match),
> > >
> > > ACPI_PTR not needed since the table will always be there.
> > 
> > OK.
> > 
> > >
> > > > +	},
> > > > +	.probe = pwm_lpss_probe,
> > > > +	.remove = pwm_lpss_remove,
> > > > +};
> > > > +module_platform_driver(pwm_lpss_driver);
> > > > +
> > > > +MODULE_DESCRIPTION("PWM driver for Intel LPSS");
> > > > +MODULE_AUTHOR("Mika Westerberg
> > <mika.westerberg@...ux.intel.com>");
> > > > +MODULE_LICENSE("GPL");
> > >
> > > The file header says GPL v2, but "GPL" here means "GPL v2 or later".
> > 
> > OK, we will change that to GPLv2.
> > 
> > Thanks a lot for your review! We will do the changes and submit v2.
> 
> Ok. I will make the changes accordingly.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/