| 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
| ||
|
Message-ID: <20140414081805.GI19349@intel.com>
Date: Mon, 14 Apr 2014 11:18:05 +0300
From: Mika Westerberg <mika.westerberg@...ux.intel.com>
To: Chew Chiau Ee <chiau.ee.chew@...el.com>
Cc: Thierry Reding <thierry.reding@...il.com>,
Alan Cox <alan@...ux.intel.com>, linux-pwm@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] pwm_lpss: Add support for PCI devices
On Sat, Apr 12, 2014 at 09:58:51PM +0800, Chew Chiau Ee wrote:
> From: Alan Cox <alan@...ux.intel.com>
>
> Not all systems enumerate the PWM devices via ACPI. They can also be exposed
> via the PCI interface.
>
> Signed-off-by: Alan Cox <alan@...ux.intel.com>
> Signed-off-by: Chew, Chiau Ee <chiau.ee.chew@...el.com>
> ---
> drivers/pwm/pwm-lpss.c | 160 ++++++++++++++++++++++++++++++++++++++---------
> 1 files changed, 129 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/pwm/pwm-lpss.c b/drivers/pwm/pwm-lpss.c
> index 449e372..6f79bf8 100644
> --- a/drivers/pwm/pwm-lpss.c
> +++ b/drivers/pwm/pwm-lpss.c
> @@ -6,6 +6,7 @@
> * Author: Chew Kean Ho <kean.ho.chew@...el.com>
> * Author: Chang Rebecca Swee Fun <rebecca.swee.fun.chang@...el.com>
> * Author: Chew Chiau Ee <chiau.ee.chew@...el.com>
> + * Author: Alan Cox <alan@...ux.intel.com>
> *
> * This program is free software; you can redistribute it and/or modify
> * it under the terms of the GNU General Public License version 2 as
> @@ -19,6 +20,9 @@
> #include <linux/module.h>
> #include <linux/pwm.h>
> #include <linux/platform_device.h>
> +#include <linux/pci.h>
> +
> +static int pci_drv, plat_drv; /* So we know which drivers registered */
>
> #define PWM 0x00000000
> #define PWM_ENABLE BIT(31)
> @@ -34,6 +38,15 @@ struct pwm_lpss_chip {
> struct pwm_chip chip;
> void __iomem *regs;
> struct clk *clk;
> + unsigned long clk_rate;
> +};
> +
> +struct pwm_lpss_boardinfo {
> + unsigned long clk_rate;
> +};
> +
> +static const struct pwm_lpss_boardinfo byt_info = {
> + 25000000
> };
>
> static inline struct pwm_lpss_chip *to_lpwm(struct pwm_chip *chip)
> @@ -55,7 +68,7 @@ static int pwm_lpss_config(struct pwm_chip *chip, struct pwm_device *pwm,
> /* The equation is: base_unit = ((freq / c) * 65536) + correction */
> base_unit = freq * 65536;
>
> - c = clk_get_rate(lpwm->clk);
> + c = lpwm->clk_rate;
> if (!c)
> return -EINVAL;
>
> @@ -113,52 +126,47 @@ static const struct pwm_ops pwm_lpss_ops = {
> .owner = THIS_MODULE,
> };
>
> -static const struct acpi_device_id pwm_lpss_acpi_match[] = {
> - { "80860F09", 0 },
> - { },
> -};
> -MODULE_DEVICE_TABLE(acpi, pwm_lpss_acpi_match);
> -
> -static int pwm_lpss_probe(struct platform_device *pdev)
> +static struct pwm_lpss_chip *pwm_lpss_probe(struct device *dev,
> + struct resource *r, struct pwm_lpss_boardinfo *info)
> {
> struct pwm_lpss_chip *lpwm;
> - struct resource *r;
> int ret;
>
> - lpwm = devm_kzalloc(&pdev->dev, sizeof(*lpwm), GFP_KERNEL);
> + lpwm = devm_kzalloc(dev, sizeof(*lpwm), GFP_KERNEL);
> if (!lpwm)
> - return -ENOMEM;
> -
> - r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + return ERR_PTR(-ENOMEM);
>
> - lpwm->regs = devm_ioremap_resource(&pdev->dev, r);
> + lpwm->regs = devm_ioremap_resource(dev, r);
> if (IS_ERR(lpwm->regs))
> - return PTR_ERR(lpwm->regs);
> -
> - lpwm->clk = devm_clk_get(&pdev->dev, NULL);
> - if (IS_ERR(lpwm->clk)) {
> - dev_err(&pdev->dev, "failed to get PWM clock\n");
> - return PTR_ERR(lpwm->clk);
> + return lpwm->regs;
> +
> + if (info) {
> + lpwm->clk_rate = info->clk_rate;
> + } else {
> + lpwm->clk = devm_clk_get(dev, NULL);
> + if (IS_ERR(lpwm->clk)) {
> + dev_err(dev, "failed to get PWM clock\n");
> + return (void *)lpwm->clk;
Why the cast here?
> + }
> + lpwm->clk_rate = clk_get_rate(lpwm->clk);
> }
>
> - lpwm->chip.dev = &pdev->dev;
> + lpwm->chip.dev = dev;
> lpwm->chip.ops = &pwm_lpss_ops;
> lpwm->chip.base = -1;
> lpwm->chip.npwm = 1;
>
> ret = pwmchip_add(&lpwm->chip);
> if (ret) {
> - dev_err(&pdev->dev, "failed to add PWM chip: %d\n", ret);
> - return ret;
> + dev_err(dev, "failed to add PWM chip: %d\n", ret);
> + return ERR_PTR(ret);
> }
>
> - platform_set_drvdata(pdev, lpwm);
> - return 0;
> + return lpwm;
> }
>
> -static int pwm_lpss_remove(struct platform_device *pdev)
> +static int pwm_lpss_remove(struct pwm_lpss_chip *lpwm)
> {
> - struct pwm_lpss_chip *lpwm = platform_get_drvdata(pdev);
> u32 ctrl;
>
> ctrl = readl(lpwm->regs + PWM);
> @@ -167,17 +175,107 @@ static int pwm_lpss_remove(struct platform_device *pdev)
> return pwmchip_remove(&lpwm->chip);
> }
>
> -static struct platform_driver pwm_lpss_driver = {
> +static int pwm_lpss_probe_pci(struct pci_dev *pdev,
> + const struct pci_device_id *id)
> +{
> + struct pwm_lpss_boardinfo *info;
> + struct pwm_lpss_chip *lpwm;
> + int err;
> +
> + err = pci_enable_device(pdev);
> + if (err < 0)
> + return err;
> +
> + info = (struct pwm_lpss_boardinfo *)id->driver_data;
> + lpwm = pwm_lpss_probe(&pdev->dev, &pdev->resource[0], info);
> + if (IS_ERR(lpwm))
> + return PTR_ERR(lpwm);
> +
> + pci_set_drvdata(pdev, lpwm);
> + return 0;
> +}
> +
> +static void pwm_lpss_remove_pci(struct pci_dev *pdev)
> +{
> + struct pwm_lpss_chip *lpwm = pci_get_drvdata(pdev);
> +
> + pwm_lpss_remove(lpwm);
> + pci_disable_device(pdev);
> +}
> +
> +static struct pci_device_id pwm_lpss_pci_ids[] = {
> + { PCI_VDEVICE(INTEL, 0x0F08), (unsigned long)&byt_info},
> + { PCI_VDEVICE(INTEL, 0x0F09), (unsigned long)&byt_info},
I think non-capital letters for hex numbers are more consistent. I.e
0x0f08.
> + { 0,}
{ },
as in the platform part looks better, IMO.
> +};
> +MODULE_DEVICE_TABLE(pci, pwm_lpss_pci_ids);
> +
> +static struct pci_driver pwm_lpss_driver_pci = {
> + .name = "pwm-lpss",
> + .id_table = pwm_lpss_pci_ids,
The platform part doesn't use tabs, so I think you should follow the style
here.
> + .probe = pwm_lpss_probe_pci,
> + .remove = pwm_lpss_remove_pci,
> +};
> +
> +static int pwm_lpss_probe_platform(struct platform_device *pdev)
> +{
> + struct pwm_lpss_chip *lpwm;
> + struct resource *r;
> +
> + r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +
> + lpwm = pwm_lpss_probe(&pdev->dev, r, NULL);
> + if (IS_ERR(lpwm))
> + return PTR_ERR(lpwm);
> +
> + platform_set_drvdata(pdev, lpwm);
> + return 0;
> +}
> +
> +static int pwm_lpss_remove_platform(struct platform_device *pdev)
> +{
> + struct pwm_lpss_chip *lpwm = platform_get_drvdata(pdev);
> +
> + return pwm_lpss_remove(lpwm);
> +}
> +
> +static const struct acpi_device_id pwm_lpss_acpi_match[] = {
> + { "80860F09", 0 },
> + { },
> +};
> +MODULE_DEVICE_TABLE(acpi, pwm_lpss_acpi_match);
> +
> +static struct platform_driver pwm_lpss_driver_platform = {
> .driver = {
> .name = "pwm-lpss",
> .acpi_match_table = pwm_lpss_acpi_match,
> },
> - .probe = pwm_lpss_probe,
> - .remove = pwm_lpss_remove,
> + .probe = pwm_lpss_probe_platform,
> + .remove = pwm_lpss_remove_platform,
> };
> -module_platform_driver(pwm_lpss_driver);
> +
> +static int __init pwm_init(void)
> +{
> + pci_drv = pci_register_driver(&pwm_lpss_driver_pci);
> + plat_drv = platform_driver_register(&pwm_lpss_driver_platform);
> + if (pci_drv && plat_drv)
> + return pci_drv;
> +
> + return 0;
> +}
> +
> +static void __exit pwm_exit(void)
> +{
> + if (!pci_drv)
> + pci_unregister_driver(&pwm_lpss_driver_pci);
> + if (!plat_drv)
> + platform_driver_unregister(&pwm_lpss_driver_platform);
> +}
>
> MODULE_DESCRIPTION("PWM driver for Intel LPSS");
> MODULE_AUTHOR("Mika Westerberg <mika.westerberg@...ux.intel.com>");
> MODULE_LICENSE("GPL v2");
> MODULE_ALIAS("platform:pwm-lpss");
> +
> +module_init(pwm_init);
> +module_exit(pwm_exit);
Maybe move these to follow the function defitions, like:
pwm_init()
{
}
module_init(pwm_init);
and so on.
--
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/
Powered by blists - more mailing lists