Re: [PATCH] pwm_lpss: Add support for PCI devices
On Mon, May 12, 2014 at 04:18:16PM -0700, Darren Hart wrote: > On Sat, Apr 12, 2014 at 09:58:51PM +0800, Chew Chiau Ee wrote: > > From: Alan Cox > > > > Not all systems enumerate the PWM devices via ACPI. They can also be exposed > > via the PCI interface. > > > > Signed-off-by: Alan Cox > > Signed-off-by: Chew, Chiau Ee > > --- > > drivers/pwm/pwm-lpss.c | 160 > > ++- > > 1 files changed, 129 insertions(+), 31 deletions(-) [...] > Has this landed anywhere? I didn't see it in mainline or next, am I looking in > the wrong place? This was applied to the pwm/for-next tree and has been in linux-next since next-20140429. > If it's just still pending, I ran into one issue integrating it > with 3.14.2: > > CC [M] drivers/pwm/pwm-lpss.o > drivers/pwm/pwm-lpss.c: In function ‘pwm_lpss_probe_pci’: > drivers/pwm/pwm-lpss.c:192:2: warning: passing argument 3 of > ‘pwm_lpss_probe’ > discards ‘const’ qualifier from pointer target type [enabled by default] > drivers/pwm/pwm-lpss.c:130:30: note: expected ‘struct pwm_lpss_boardinfo *’ > but argument is of type ‘const struct pwm_lpss_boardinfo *’ > > Can we make the third argument to pwm_lpss_probe a const? The following is > working for me: > > static struct pwm_lpss_chip *pwm_lpss_probe(struct device *dev, > struct resource *r, > const struct pwm_lpss_boardinfo > *info) A fix like that was applied last week and is in next-20140512. Thierry pgpHGoyDMuzXg.pgp Description: PGP signature
Re: [PATCH] pwm_lpss: Add support for PCI devices
On Mon, May 12, 2014 at 04:18:16PM -0700, Darren Hart wrote: On Sat, Apr 12, 2014 at 09:58:51PM +0800, Chew Chiau Ee wrote: From: Alan Cox a...@linux.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 a...@linux.intel.com Signed-off-by: Chew, Chiau Ee chiau.ee.c...@intel.com --- drivers/pwm/pwm-lpss.c | 160 ++- 1 files changed, 129 insertions(+), 31 deletions(-) [...] Has this landed anywhere? I didn't see it in mainline or next, am I looking in the wrong place? This was applied to the pwm/for-next tree and has been in linux-next since next-20140429. If it's just still pending, I ran into one issue integrating it with 3.14.2: CC [M] drivers/pwm/pwm-lpss.o drivers/pwm/pwm-lpss.c: In function ‘pwm_lpss_probe_pci’: drivers/pwm/pwm-lpss.c:192:2: warning: passing argument 3 of ‘pwm_lpss_probe’ discards ‘const’ qualifier from pointer target type [enabled by default] drivers/pwm/pwm-lpss.c:130:30: note: expected ‘struct pwm_lpss_boardinfo *’ but argument is of type ‘const struct pwm_lpss_boardinfo *’ Can we make the third argument to pwm_lpss_probe a const? The following is working for me: static struct pwm_lpss_chip *pwm_lpss_probe(struct device *dev, struct resource *r, const struct pwm_lpss_boardinfo *info) A fix like that was applied last week and is in next-20140512. Thierry pgpHGoyDMuzXg.pgp Description: PGP signature
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
>
> Not all systems enumerate the PWM devices via ACPI. They can also be exposed
> via the PCI interface.
>
> Signed-off-by: Alan Cox
> Signed-off-by: Chew, Chiau Ee
> ---
> 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
> * Author: Chang Rebecca Swee Fun
> * Author: Chew Chiau Ee
> + * Author: Alan Cox
> *
> * 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
> #include
> #include
> +#include
> +
> +static int pci_drv, plat_drv;/* So we know which drivers registered
> */
>
> #define PWM 0x
> #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 = {
> + 2500
> };
>
> 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)
Has this landed anywhere? I didn't see it in mainline or next, am I looking in
the wrong place? If it's just still pending, I ran into one issue integrating it
with 3.14.2:
CC [M] drivers/pwm/pwm-lpss.o
drivers/pwm/pwm-lpss.c: In function ‘pwm_lpss_probe_pci’:
drivers/pwm/pwm-lpss.c:192:2: warning: passing argument 3 of ‘pwm_lpss_probe’
discards ‘const’ qualifier from pointer target type [enabled by default]
drivers/pwm/pwm-lpss.c:130:30: note: expected ‘struct pwm_lpss_boardinfo *’
but argument is of type ‘const struct pwm_lpss_boardinfo *’
Can we make the third argument to pwm_lpss_probe a const? The following is
working for me:
static struct pwm_lpss_chip *pwm_lpss_probe(struct device *dev,
struct resource *r,
const struct pwm_lpss_boardinfo
*info)
Thanks,
--
Darren Hart Open Source Technology Center
darren.h...@intel.com Intel Corporation
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
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 a...@linux.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 a...@linux.intel.com
Signed-off-by: Chew, Chiau Ee chiau.ee.c...@intel.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.c...@intel.com
* Author: Chang Rebecca Swee Fun rebecca.swee.fun.ch...@intel.com
* Author: Chew Chiau Ee chiau.ee.c...@intel.com
+ * Author: Alan Cox a...@linux.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 0x
#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 = {
+ 2500
};
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)
Has this landed anywhere? I didn't see it in mainline or next, am I looking in
the wrong place? If it's just still pending, I ran into one issue integrating it
with 3.14.2:
CC [M] drivers/pwm/pwm-lpss.o
drivers/pwm/pwm-lpss.c: In function ‘pwm_lpss_probe_pci’:
drivers/pwm/pwm-lpss.c:192:2: warning: passing argument 3 of ‘pwm_lpss_probe’
discards ‘const’ qualifier from pointer target type [enabled by default]
drivers/pwm/pwm-lpss.c:130:30: note: expected ‘struct pwm_lpss_boardinfo *’
but argument is of type ‘const struct pwm_lpss_boardinfo *’
Can we make the third argument to pwm_lpss_probe a const? The following is
working for me:
static struct pwm_lpss_chip *pwm_lpss_probe(struct device *dev,
struct resource *r,
const struct pwm_lpss_boardinfo
*info)
Thanks,
--
Darren Hart Open Source Technology Center
darren.h...@intel.com Intel Corporation
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] pwm_lpss: Add support for PCI devices
> On Tue, Apr 15, 2014 at 08:41:02AM +, Chew, Chiau Ee wrote:
> >
> >
> > > > +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(>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(>dev, r);
> > > > + lpwm->regs = devm_ioremap_resource(dev, r);
> > > > if (IS_ERR(lpwm->regs))
> > > > - return PTR_ERR(lpwm->regs);
> > > > -
> > > > - lpwm->clk = devm_clk_get(>dev, NULL);
> > > > - if (IS_ERR(lpwm->clk)) {
> > > > - dev_err(>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?
> >
> > Alan, please do correct me if I describe wrongly on the purpose that you add
> the cast here.
> >
> > As you can see, pwm_lpss_probe() is expected to return a pointer of type
> struct pwm_lpss_chip. On the other hand,
> > the return of devm_clk_get() would be a pointer of type struct clk. So
> > if were to use lpwm->clk as the return value of pwm_lpss_probe() , a cast
> would be needed since the pointer type is different from the expected one. The
> compiler would complain on " warning: return from incompatible pointer type"
> > if without the cast.
>
> I think you can use ERR_CAST() here.
You are rightERR_CAST() can be used.
Ok...i will clean up the patch and resend a v2 version.
Thanks for the review and feedback.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] pwm_lpss: Add support for PCI devices
> On Mon, Apr 14, 2014 at 02:05:25AM +, Chew, Chiau Ee wrote:
> > > > MODULE_DESCRIPTION("PWM driver for Intel LPSS");
> > > >MODULE_AUTHOR("Mika Westerberg
> > > >");
> > > > MODULE_LICENSE("GPL v2");
> > > > MODULE_ALIAS("platform:pwm-lpss");
> > >
> > > Looks a good idea to combine pci and acpi driver together.
> > > Since pci driver is added, here the alias need to be refined.
> > > Others look good.
> > >
> > > Thanks,
> > > -Aubrey
> >
> > Ok. I will change it to MODULE_ALIAS("pci/platform:pwm-lpss");
>
> Hmm, does that really work like that?
>
> For PCI and ACPI you already have tables with MODULE_DEVICE_TABLE().
>
> For pure platform driver (which is probably not going to be used) you nede to
> have MODULE_ALIAS() if you want modprobe to load it automagically.
Ok, looks like MODULE_ALIAS() is a redundant. I will remove it.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] pwm_lpss: Add support for PCI devices
On Tue, Apr 15, 2014 at 08:41:02AM +, Chew, Chiau Ee wrote:
>
>
> > > +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(>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(>dev, r);
> > > + lpwm->regs = devm_ioremap_resource(dev, r);
> > > if (IS_ERR(lpwm->regs))
> > > - return PTR_ERR(lpwm->regs);
> > > -
> > > - lpwm->clk = devm_clk_get(>dev, NULL);
> > > - if (IS_ERR(lpwm->clk)) {
> > > - dev_err(>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?
>
> Alan, please do correct me if I describe wrongly on the purpose that you add
> the cast here.
>
> As you can see, pwm_lpss_probe() is expected to return a pointer of type
> struct pwm_lpss_chip. On the other hand,
> the return of devm_clk_get() would be a pointer of type struct clk. So if
> were to use lpwm->clk as the return value of pwm_lpss_probe() ,
> a cast would be needed since the pointer type is different from the expected
> one. The compiler would complain on " warning: return from incompatible
> pointer type"
> if without the cast.
I think you can use ERR_CAST() here.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] pwm_lpss: Add support for PCI devices
> > +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(>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(>dev, r);
> > + lpwm->regs = devm_ioremap_resource(dev, r);
> > if (IS_ERR(lpwm->regs))
> > - return PTR_ERR(lpwm->regs);
> > -
> > - lpwm->clk = devm_clk_get(>dev, NULL);
> > - if (IS_ERR(lpwm->clk)) {
> > - dev_err(>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?
Alan, please do correct me if I describe wrongly on the purpose that you add
the cast here.
As you can see, pwm_lpss_probe() is expected to return a pointer of type struct
pwm_lpss_chip. On the other hand,
the return of devm_clk_get() would be a pointer of type struct clk. So if were
to use lpwm->clk as the return value of pwm_lpss_probe() ,
a cast would be needed since the pointer type is different from the expected
one. The compiler would complain on " warning: return from incompatible pointer
type"
if without the cast.
> > +
> > +static struct pci_device_id pwm_lpss_pci_ids[] = {
> > + { PCI_VDEVICE(INTEL, 0x0F08), (unsigned long)_info},
> > + { PCI_VDEVICE(INTEL, 0x0F09), (unsigned long)_info},
>
> I think non-capital letters for hex numbers are more consistent. I.e 0x0f08.
>
Ok. Noted.
> > + { 0,}
>
> { },
>
> as in the platform part looks better, IMO.
Ok. Noted.
>
> > +};
> > +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.
>
Ok. Noted
> >
> > MODULE_DESCRIPTION("PWM driver for Intel LPSS");
> MODULE_AUTHOR("Mika
> > Westerberg ");
> > 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.
Ok. Noted.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] pwm_lpss: Add support for PCI devices
+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?
Alan, please do correct me if I describe wrongly on the purpose that you add
the cast here.
As you can see, pwm_lpss_probe() is expected to return a pointer of type struct
pwm_lpss_chip. On the other hand,
the return of devm_clk_get() would be a pointer of type struct clk. So if were
to use lpwm-clk as the return value of pwm_lpss_probe() ,
a cast would be needed since the pointer type is different from the expected
one. The compiler would complain on warning: return from incompatible pointer
type
if without the cast.
+
+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.
Ok. Noted.
+ { 0,}
{ },
as in the platform part looks better, IMO.
Ok. Noted.
+};
+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.
Ok. Noted
MODULE_DESCRIPTION(PWM driver for Intel LPSS);
MODULE_AUTHOR(Mika
Westerberg mika.westerb...@linux.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.
Ok. Noted.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] pwm_lpss: Add support for PCI devices
On Tue, Apr 15, 2014 at 08:41:02AM +, Chew, Chiau Ee wrote:
+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?
Alan, please do correct me if I describe wrongly on the purpose that you add
the cast here.
As you can see, pwm_lpss_probe() is expected to return a pointer of type
struct pwm_lpss_chip. On the other hand,
the return of devm_clk_get() would be a pointer of type struct clk. So if
were to use lpwm-clk as the return value of pwm_lpss_probe() ,
a cast would be needed since the pointer type is different from the expected
one. The compiler would complain on warning: return from incompatible
pointer type
if without the cast.
I think you can use ERR_CAST() here.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] pwm_lpss: Add support for PCI devices
On Mon, Apr 14, 2014 at 02:05:25AM +, Chew, Chiau Ee wrote: MODULE_DESCRIPTION(PWM driver for Intel LPSS); MODULE_AUTHOR(Mika Westerberg mika.westerb...@linux.intel.com); MODULE_LICENSE(GPL v2); MODULE_ALIAS(platform:pwm-lpss); Looks a good idea to combine pci and acpi driver together. Since pci driver is added, here the alias need to be refined. Others look good. Thanks, -Aubrey Ok. I will change it to MODULE_ALIAS(pci/platform:pwm-lpss); Hmm, does that really work like that? For PCI and ACPI you already have tables with MODULE_DEVICE_TABLE(). For pure platform driver (which is probably not going to be used) you nede to have MODULE_ALIAS() if you want modprobe to load it automagically. Ok, looks like MODULE_ALIAS() is a redundant. I will remove it. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] pwm_lpss: Add support for PCI devices
On Tue, Apr 15, 2014 at 08:41:02AM +, Chew, Chiau Ee wrote:
+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?
Alan, please do correct me if I describe wrongly on the purpose that you add
the cast here.
As you can see, pwm_lpss_probe() is expected to return a pointer of type
struct pwm_lpss_chip. On the other hand,
the return of devm_clk_get() would be a pointer of type struct clk. So
if were to use lpwm-clk as the return value of pwm_lpss_probe() , a cast
would be needed since the pointer type is different from the expected one. The
compiler would complain on warning: return from incompatible pointer type
if without the cast.
I think you can use ERR_CAST() here.
You are rightERR_CAST() can be used.
Ok...i will clean up the patch and resend a v2 version.
Thanks for the review and feedback.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] pwm_lpss: Add support for PCI devices
On Mon, Apr 14, 2014 at 02:05:25AM +, Chew, Chiau Ee wrote:
> > > MODULE_DESCRIPTION("PWM driver for Intel LPSS");
> > >MODULE_AUTHOR("Mika
> > > Westerberg ");
> > > MODULE_LICENSE("GPL v2");
> > > MODULE_ALIAS("platform:pwm-lpss");
> >
> > Looks a good idea to combine pci and acpi driver together.
> > Since pci driver is added, here the alias need to be refined.
> > Others look good.
> >
> > Thanks,
> > -Aubrey
>
> Ok. I will change it to MODULE_ALIAS("pci/platform:pwm-lpss");
Hmm, does that really work like that?
For PCI and ACPI you already have tables with MODULE_DEVICE_TABLE().
For pure platform driver (which is probably not going to be used) you nede
to have MODULE_ALIAS() if you want modprobe to load it automagically.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
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
>
> Not all systems enumerate the PWM devices via ACPI. They can also be exposed
> via the PCI interface.
>
> Signed-off-by: Alan Cox
> Signed-off-by: Chew, Chiau Ee
> ---
> 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
> * Author: Chang Rebecca Swee Fun
> * Author: Chew Chiau Ee
> + * Author: Alan Cox
> *
> * 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
> #include
> #include
> +#include
> +
> +static int pci_drv, plat_drv;/* So we know which drivers registered
> */
>
> #define PWM 0x
> #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 = {
> + 2500
> };
>
> 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(>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(>dev, r);
> + lpwm->regs = devm_ioremap_resource(dev, r);
> if (IS_ERR(lpwm->regs))
> - return PTR_ERR(lpwm->regs);
> -
> - lpwm->clk = devm_clk_get(>dev, NULL);
> - if (IS_ERR(lpwm->clk)) {
> - dev_err(>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 = >dev;
> + lpwm->chip.dev = dev;
> lpwm->chip.ops = _lpss_ops;
> lpwm->chip.base = -1;
> lpwm->chip.npwm = 1;
>
> ret = pwmchip_add(>chip);
> if (ret) {
> - dev_err(>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(>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(>dev, >resource[0], info);
> + if (IS_ERR(lpwm))
> + return PTR_ERR(lpwm);
> +
> + pci_set_drvdata(pdev, lpwm);
> + return
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 a...@linux.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 a...@linux.intel.com
Signed-off-by: Chew, Chiau Ee chiau.ee.c...@intel.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.c...@intel.com
* Author: Chang Rebecca Swee Fun rebecca.swee.fun.ch...@intel.com
* Author: Chew Chiau Ee chiau.ee.c...@intel.com
+ * Author: Alan Cox a...@linux.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 0x
#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 = {
+ 2500
};
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
Re: [PATCH] pwm_lpss: Add support for PCI devices
On Mon, Apr 14, 2014 at 02:05:25AM +, Chew, Chiau Ee wrote: MODULE_DESCRIPTION(PWM driver for Intel LPSS); MODULE_AUTHOR(Mika Westerberg mika.westerb...@linux.intel.com); MODULE_LICENSE(GPL v2); MODULE_ALIAS(platform:pwm-lpss); Looks a good idea to combine pci and acpi driver together. Since pci driver is added, here the alias need to be refined. Others look good. Thanks, -Aubrey Ok. I will change it to MODULE_ALIAS(pci/platform:pwm-lpss); Hmm, does that really work like that? For PCI and ACPI you already have tables with MODULE_DEVICE_TABLE(). For pure platform driver (which is probably not going to be used) you nede to have MODULE_ALIAS() if you want modprobe to load it automagically. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] pwm_lpss: Add support for PCI devices
> > MODULE_DESCRIPTION("PWM driver for Intel LPSS");
> >MODULE_AUTHOR("Mika
> > Westerberg ");
> > MODULE_LICENSE("GPL v2");
> > MODULE_ALIAS("platform:pwm-lpss");
>
> Looks a good idea to combine pci and acpi driver together.
> Since pci driver is added, here the alias need to be refined.
> Others look good.
>
> Thanks,
> -Aubrey
Ok. I will change it to MODULE_ALIAS("pci/platform:pwm-lpss");
Thanks,
Chiau Ee
N�r��yb�X��ǧv�^�){.n�+{zX����ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z� �w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i
Re: [PATCH] pwm_lpss: Add support for PCI devices
On 2014/4/12 21:58, Chew Chiau Ee wrote:
> From: Alan Cox
>
> Not all systems enumerate the PWM devices via ACPI. They can also be exposed
> via the PCI interface.
>
> Signed-off-by: Alan Cox
> Signed-off-by: Chew, Chiau Ee
> ---
> 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
> * Author: Chang Rebecca Swee Fun
> * Author: Chew Chiau Ee
> + * Author: Alan Cox
> *
> * 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
> #include
> #include
> +#include
> +
> +static int pci_drv, plat_drv;/* So we know which drivers registered
> */
>
> #define PWM 0x
> #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 = {
> + 2500
> };
>
> 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(>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(>dev, r);
> + lpwm->regs = devm_ioremap_resource(dev, r);
> if (IS_ERR(lpwm->regs))
> - return PTR_ERR(lpwm->regs);
> -
> - lpwm->clk = devm_clk_get(>dev, NULL);
> - if (IS_ERR(lpwm->clk)) {
> - dev_err(>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;
> + }
> + lpwm->clk_rate = clk_get_rate(lpwm->clk);
> }
>
> - lpwm->chip.dev = >dev;
> + lpwm->chip.dev = dev;
> lpwm->chip.ops = _lpss_ops;
> lpwm->chip.base = -1;
> lpwm->chip.npwm = 1;
>
> ret = pwmchip_add(>chip);
> if (ret) {
> - dev_err(>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(>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(>dev, >resource[0], info);
> + if (IS_ERR(lpwm))
> + return PTR_ERR(lpwm);
> +
> + pci_set_drvdata(pdev, lpwm);
> + return 0;
> +}
> +
> +static void
Re: [PATCH] pwm_lpss: Add support for PCI devices
On 2014/4/12 21:58, Chew Chiau Ee wrote:
From: Alan Cox a...@linux.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 a...@linux.intel.com
Signed-off-by: Chew, Chiau Ee chiau.ee.c...@intel.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.c...@intel.com
* Author: Chang Rebecca Swee Fun rebecca.swee.fun.ch...@intel.com
* Author: Chew Chiau Ee chiau.ee.c...@intel.com
+ * Author: Alan Cox a...@linux.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 0x
#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 = {
+ 2500
};
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;
+ }
+ 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);
+
+
RE: [PATCH] pwm_lpss: Add support for PCI devices
MODULE_DESCRIPTION(PWM driver for Intel LPSS);
MODULE_AUTHOR(Mika
Westerberg mika.westerb...@linux.intel.com);
MODULE_LICENSE(GPL v2);
MODULE_ALIAS(platform:pwm-lpss);
Looks a good idea to combine pci and acpi driver together.
Since pci driver is added, here the alias need to be refined.
Others look good.
Thanks,
-Aubrey
Ok. I will change it to MODULE_ALIAS(pci/platform:pwm-lpss);
Thanks,
Chiau Ee
N�r��yb�X��ǧv�^�){.n�+{zX����ܨ}���Ơz�j:+v���zZ+��+zf���h���~i���z� �w���?��)ߢf��^jǫy�m��@A�a���
0��h���i
[PATCH] pwm_lpss: Add support for PCI devices
From: Alan Cox
Not all systems enumerate the PWM devices via ACPI. They can also be exposed
via the PCI interface.
Signed-off-by: Alan Cox
Signed-off-by: Chew, Chiau Ee
---
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
* Author: Chang Rebecca Swee Fun
* Author: Chew Chiau Ee
+ * Author: Alan Cox
*
* 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
#include
#include
+#include
+
+static int pci_drv, plat_drv; /* So we know which drivers registered */
#define PWM0x
#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 = {
+ 2500
};
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(>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(>dev, r);
+ lpwm->regs = devm_ioremap_resource(dev, r);
if (IS_ERR(lpwm->regs))
- return PTR_ERR(lpwm->regs);
-
- lpwm->clk = devm_clk_get(>dev, NULL);
- if (IS_ERR(lpwm->clk)) {
- dev_err(>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;
+ }
+ lpwm->clk_rate = clk_get_rate(lpwm->clk);
}
- lpwm->chip.dev = >dev;
+ lpwm->chip.dev = dev;
lpwm->chip.ops = _lpss_ops;
lpwm->chip.base = -1;
lpwm->chip.npwm = 1;
ret = pwmchip_add(>chip);
if (ret) {
- dev_err(>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(>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(>dev, >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[] = {
+
[PATCH] pwm_lpss: Add support for PCI devices
From: Alan Cox a...@linux.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 a...@linux.intel.com
Signed-off-by: Chew, Chiau Ee chiau.ee.c...@intel.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.c...@intel.com
* Author: Chang Rebecca Swee Fun rebecca.swee.fun.ch...@intel.com
* Author: Chew Chiau Ee chiau.ee.c...@intel.com
+ * Author: Alan Cox a...@linux.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 PWM0x
#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 = {
+ 2500
};
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;
+ }
+ 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
