| Message ID | 1430316005-16480-7-git-send-email-shobhit.kumar@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 2015-04-29 at 19:30 +0530, Shobhit Kumar wrote: > --- a/drivers/pwm/Kconfig > +++ b/drivers/pwm/Kconfig > +config PWM_CRC > + bool "Intel Crystalcove (CRC) PWM support" > + depends on X86 && INTEL_SOC_PMIC > + help > + Generic PWM framework driver for Crystalcove (CRC) PMIC based PWM > + control. > --- a/drivers/pwm/Makefile > +++ b/drivers/pwm/Makefile > +obj-$(CONFIG_PWM_CRC) += pwm-crc.o PWM_CRC is a bool symbol. So pwm-crc.o can never be part of a module. (If I'm wrong, and that object file can actually be part of a module, you can stop reading here.) > --- /dev/null > +++ b/drivers/pwm/pwm-crc.c > +#include <linux/module.h> Perhaps this include is not needed. > +static const struct pwm_ops crc_pwm_ops = { > + .config = crc_pwm_config, > + .enable = crc_pwm_enable, > + .disable = crc_pwm_disable, > + .owner = THIS_MODULE, For built-in only code THIS_MODULE is basically equivalent to NULL (see include/linux/export.h). So I guess this line can be dropped. > +}; > +static struct platform_driver crystalcove_pwm_driver = { > + .probe = crystalcove_pwm_probe, > + .remove = crystalcove_pwm_remove, > + .driver = { > + .name = "crystal_cove_pwm", > + }, > +}; > + > +module_platform_driver(crystalcove_pwm_driver); Speaking from memory: for built-in only code this is equivalent to calling platform_driver_register(&crystalcove_pwm_driver); from a wrapper, and marking that wrapper with device_initcall(). > +MODULE_AUTHOR("Shobhit Kumar <shobhit.kumar@intel.com>"); > +MODULE_DESCRIPTION("Intel Crystal Cove PWM Driver"); > +MODULE_LICENSE("GPL v2"); These macros will be effectively preprocessed away for built-in only code. Paul Bolle
On Tue, 2015-05-05 at 15:08 +0530, Shobhit Kumar wrote: > The Crystalcove PMIC controls PWM signals and this driver exports that > capability as a PWM chip driver. This is platform device implementtaion > of the drivers/mfd cell device for CRC PMIC > > v2: Use the existing config callback with duty_ns and period_ns(Thierry) > > v3: Correct the subject line (Lee jones) > > CC: Samuel Ortiz <sameo@linux.intel.com> > Cc: Linus Walleij <linus.walleij@linaro.org> > Cc: Alexandre Courbot <gnurou@gmail.com> > Cc: Thierry Reding <thierry.reding@gmail.com> > Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com> The same comments can be made as for v2, see http://lkml.kernel.org/r/1430428322.2187.24.camel@x220 . Maybe you didn't receive that message. It could also be that you think my comments were invalid, or too vague, or whatever. Please say so, because then I don't have to bother you again when you send out v4. Thanks, Paul Bolle
On Wed, May 6, 2015 at 1:10 PM, Paul Bolle <pebolle@tiscali.nl> wrote: > On Tue, 2015-05-05 at 15:08 +0530, Shobhit Kumar wrote: >> The Crystalcove PMIC controls PWM signals and this driver exports that >> capability as a PWM chip driver. This is platform device implementtaion >> of the drivers/mfd cell device for CRC PMIC >> >> v2: Use the existing config callback with duty_ns and period_ns(Thierry) >> >> v3: Correct the subject line (Lee jones) >> >> CC: Samuel Ortiz <sameo@linux.intel.com> >> Cc: Linus Walleij <linus.walleij@linaro.org> >> Cc: Alexandre Courbot <gnurou@gmail.com> >> Cc: Thierry Reding <thierry.reding@gmail.com> >> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com> > > The same comments can be made as for v2, see > http://lkml.kernel.org/r/1430428322.2187.24.camel@x220 . Maybe you > didn't receive that message. > > It could also be that you think my comments were invalid, or too vague, > or whatever. Please say so, because then I don't have to bother you > again when you send out v4. > Not at all, I just missed your comments and realise my mistake later after sending next update. Somehow the mailing list filters that I have setup are not working correctly. I will look into your comments. Regards Shobhit
On Fri, May 1, 2015 at 2:42 AM, Paul Bolle <pebolle@tiscali.nl> wrote: > On Wed, 2015-04-29 at 19:30 +0530, Shobhit Kumar wrote: >> --- a/drivers/pwm/Kconfig >> +++ b/drivers/pwm/Kconfig > >> +config PWM_CRC >> + bool "Intel Crystalcove (CRC) PWM support" >> + depends on X86 && INTEL_SOC_PMIC >> + help >> + Generic PWM framework driver for Crystalcove (CRC) PMIC based PWM >> + control. > >> --- a/drivers/pwm/Makefile >> +++ b/drivers/pwm/Makefile > >> +obj-$(CONFIG_PWM_CRC) += pwm-crc.o > > PWM_CRC is a bool symbol. So pwm-crc.o can never be part of a module. I actually started this as a module but later decided to make it as bool because INTEL_SOC_PMIC on which this depends is itself a bool as well. Still it is good to keep the module based initialization. Firstly because it causes no harm and even though some of the macros are pre-processed out, gives info about the driver. Secondly there were discussion on why INTEL_SOC_PMIC is bool (note this driver also has module based initialization even when bool). I am guessing because of some tricky module load order dependencies. If ever that becomes a module, this can mostly be unchanged to be loaded as a module. Regards Shobhit > > (If I'm wrong, and that object file can actually be part of a module, > you can stop reading here.) > >> --- /dev/null >> +++ b/drivers/pwm/pwm-crc.c > >> +#include <linux/module.h> > > Perhaps this include is not needed. > >> +static const struct pwm_ops crc_pwm_ops = { >> + .config = crc_pwm_config, >> + .enable = crc_pwm_enable, >> + .disable = crc_pwm_disable, >> + .owner = THIS_MODULE, > > For built-in only code THIS_MODULE is basically equivalent to NULL (see > include/linux/export.h). So I guess this line can be dropped. > >> +}; > >> +static struct platform_driver crystalcove_pwm_driver = { >> + .probe = crystalcove_pwm_probe, >> + .remove = crystalcove_pwm_remove, >> + .driver = { >> + .name = "crystal_cove_pwm", >> + }, >> +}; >> + >> +module_platform_driver(crystalcove_pwm_driver); > > Speaking from memory: for built-in only code this is equivalent to > calling > platform_driver_register(&crystalcove_pwm_driver); > > from a wrapper, and marking that wrapper with device_initcall(). > >> +MODULE_AUTHOR("Shobhit Kumar <shobhit.kumar@intel.com>"); >> +MODULE_DESCRIPTION("Intel Crystal Cove PWM Driver"); >> +MODULE_LICENSE("GPL v2"); > > These macros will be effectively preprocessed away for built-in only > code. > > > Paul Bolle > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Hi Shobhit, On Thu, 2015-06-18 at 23:24 +0530, Shobhit Kumar wrote: > On Fri, May 1, 2015 at 2:42 AM, Paul Bolle <pebolle@tiscali.nl> wrote: > > On Wed, 2015-04-29 at 19:30 +0530, Shobhit Kumar wrote: > >> --- a/drivers/pwm/Kconfig > >> +++ b/drivers/pwm/Kconfig > > > >> +config PWM_CRC > >> + bool "Intel Crystalcove (CRC) PWM support" > >> + depends on X86 && INTEL_SOC_PMIC > >> + help > >> + Generic PWM framework driver for Crystalcove (CRC) PMIC based PWM > >> + control. > > > >> --- a/drivers/pwm/Makefile > >> +++ b/drivers/pwm/Makefile > > > >> +obj-$(CONFIG_PWM_CRC) += pwm-crc.o > > > > PWM_CRC is a bool symbol. So pwm-crc.o can never be part of a module. > > I actually started this as a module but later decided to make it as > bool because INTEL_SOC_PMIC on which this depends is itself a bool as > well. As does GPIO_CRYSTAL_COVE and that's a tristate. So? > Still it is good to keep the module based initialization. > Firstly because it causes no harm If I got a dime for every time people used an argument like that I ... I could treat myself to an ice cream. A really big ice cream. Hmm, that doesn't sound too impressive. But still, "causes no harm" is below the bar for kernel code. Kernel code needs to add value. > and even though some of the macros > are pre-processed out, gives info about the driver. None of which can't be gotten elsewhere (ie, the commit message, or the file these macro reside in). > Secondly there > were discussion on why INTEL_SOC_PMIC is bool (note this driver also > has module based initialization even when bool). Yes, there's copy and paste going on even in kernel development. > I am guessing because > of some tricky module load order dependencies. If ever that becomes a > module, this can mostly be unchanged to be loaded as a module. You put in a macro, or any other bit of code, when it's needed, not beforehand, "just in case". That's silly. Thanks, Paul Bolle
Hi Paul, On Fri, Jun 19, 2015 at 12:11 AM, Paul Bolle <pebolle@tiscali.nl> wrote: > Hi Shobhit, > > On Thu, 2015-06-18 at 23:24 +0530, Shobhit Kumar wrote: >> On Fri, May 1, 2015 at 2:42 AM, Paul Bolle <pebolle@tiscali.nl> wrote: >> > On Wed, 2015-04-29 at 19:30 +0530, Shobhit Kumar wrote: >> >> --- a/drivers/pwm/Kconfig >> >> +++ b/drivers/pwm/Kconfig >> > >> >> +config PWM_CRC >> >> + bool "Intel Crystalcove (CRC) PWM support" >> >> + depends on X86 && INTEL_SOC_PMIC >> >> + help >> >> + Generic PWM framework driver for Crystalcove (CRC) PMIC based PWM >> >> + control. >> > >> >> --- a/drivers/pwm/Makefile >> >> +++ b/drivers/pwm/Makefile >> > >> >> +obj-$(CONFIG_PWM_CRC) += pwm-crc.o >> > >> > PWM_CRC is a bool symbol. So pwm-crc.o can never be part of a module. >> >> I actually started this as a module but later decided to make it as >> bool because INTEL_SOC_PMIC on which this depends is itself a bool as >> well. > > As does GPIO_CRYSTAL_COVE and that's a tristate. So? > >> Still it is good to keep the module based initialization. >> Firstly because it causes no harm > > If I got a dime for every time people used an argument like that I ... I > could treat myself to an ice cream. A really big ice cream. Hmm, that > doesn't sound too impressive. But still, "causes no harm" is below the > bar for kernel code. Kernel code needs to add value. > >> and even though some of the macros >> are pre-processed out, gives info about the driver. > > None of which can't be gotten elsewhere (ie, the commit message, or the > file these macro reside in). > Causes no harm comment had to be read together with more info about the driver. It causes no harm while providing more info. And as you only said those macros are pre-processed out to really the defaults for built-in drivers. So what is the exact big problem with this ? I can anyway shove out these macros to end the discussion. BTW whether you buy the argument or not, please do treat yourself with ice cream for being able to make such a comment. >> Secondly there >> were discussion on why INTEL_SOC_PMIC is bool (note this driver also >> has module based initialization even when bool). > > Yes, there's copy and paste going on even in kernel development. > There are other examples in the kernel. I just gave the one which is related as well. Regards Shobhit >> I am guessing because >> of some tricky module load order dependencies. If ever that becomes a >> module, this can mostly be unchanged to be loaded as a module. > > You put in a macro, or any other bit of code, when it's needed, not > beforehand, "just in case". That's silly. > > Thanks, > > > Paul Bolle >
[Added Paul Gortmaker.] Hi Shobhit, On Fri, 2015-06-19 at 12:16 +0530, Shobhit Kumar wrote: > So what is the exact big problem with this ? The main problem I have is that it's hard to read a submitter's mind. And, I think, in cases like this we need to know if the submitter just made some silly mistake or that the mismatch (between Kconfig type and code) was intentional. So each time such a mismatch is spotted the submitter ought to be asked about it. (I'd guess that one or two new drivers are submitted _each_ day. And these mismatches are quite common. I'd say I receive answers like: - "Oops, yes bool should have been tristate"; or - "Oops, forgot to clean up after noticing tristate didn't work"; or - "I just copy-and-pasted a similar driver, the module stuff isn't actually needed" at least once a week. Not sure, I don't keep track of this stuff.) Furthermore, it appears that Paul Gortmaker is on a mission to, badly summarized, untangle the module and init code. See for instance https://lkml.org/lkml/2015/5/28/809 and https://lkml.org/lkml/2015/5/31/205 . Now, I don't know whether (other) Paul is bothered by these MODULE_* macros. But Paul did submit a series that adds builtin_platform_driver(), see https://lkml.org/lkml/2015/5/10/131 . That new macro ensures built-in only code doesn't have to use module_platform_driver(), which your patch also uses. So perhaps Paul can explain some of the non-obvious issues caused by built-in only code using module specific constructs. > I can anyway shove out these macros to end the discussion. I'd rather convince you than annoy you into doing as I suggested. > BTW whether you buy the argument or not, please do treat yourself > with ice cream for being able to make such a comment. Will do. Thanks, Paul Bolle
[Re: [Intel-gfx] [PATCH 6/8] drivers/pwm: Add Crystalcove (CRC) PWM driver] On 20/06/2015 (Sat 13:23) Paul Bolle wrote: > [Added Paul Gortmaker.] > > Hi Shobhit, > > On Fri, 2015-06-19 at 12:16 +0530, Shobhit Kumar wrote: > > So what is the exact big problem with this ? > > The main problem I have is that it's hard to read a submitter's mind. > And, I think, in cases like this we need to know if the submitter just > made some silly mistake or that the mismatch (between Kconfig type and > code) was intentional. So each time such a mismatch is spotted the > submitter ought to be asked about it. > > (I'd guess that one or two new drivers are submitted _each_ day. And > these mismatches are quite common. I'd say I receive answers like: > - "Oops, yes bool should have been tristate"; or > - "Oops, forgot to clean up after noticing tristate didn't work"; or > - "I just copy-and-pasted a similar driver, the module stuff isn't > actually needed" > at least once a week. Not sure, I don't keep track of this stuff.) > > Furthermore, it appears that Paul Gortmaker is on a mission to, badly > summarized, untangle the module and init code. See for instance > https://lkml.org/lkml/2015/5/28/809 and > https://lkml.org/lkml/2015/5/31/205 . > > Now, I don't know whether (other) Paul is bothered by these MODULE_* > macros. But Paul did submit a series that adds Yes, I agree that it would be nice to not see these mismatches, regardless of whether we can get away with it or not. > builtin_platform_driver(), see https://lkml.org/lkml/2015/5/10/131 . > That new macro ensures built-in only code doesn't have to use > module_platform_driver(), which your patch also uses. So perhaps Paul > can explain some of the non-obvious issues caused by built-in only code > using module specific constructs. In https://lkml.org/lkml/2015/5/10/125 I'd written: There are several downsides to this: 1) The code can appear modular to a reader of the code, and they won't know if the code really is modular without checking the Makefile and Kconfig to see if compilation is governed by a bool or tristate. 2) Coders of drivers may be tempted to code up an __exit function that is never used, just in order to satisfy the required three args of the modular registration function. 3) Non-modular code ends up including the <module.h> which increases CPP overhead that they don't need. 4) It hinders us from performing better separation of the module init code and the generic init code. The nature of linux means that thousands of developers are reading the code every day -- so I think that there is a genuine value in having the code convey a clear message on how it was designed to be used. Only using module related headers/macros for genuinely modular code helps us (albeit in a small way) towards achieving that. Looking at this thread, I see that one of the reasons given for this code's ambiguous module vs. built-in identity was the observation of a similar identity crisis of the related INTEL_SOC_PMIC code. Does that not back up the point above about the value in having the code speak for itself? So IMHO we probably should clarify the PMIC code vs. adding another example that looks just like it. Paul. -- > > > I can anyway shove out these macros to end the discussion. > > I'd rather convince you than annoy you into doing as I suggested. > > > BTW whether you buy the argument or not, please do treat yourself > > with ice cream for being able to make such a comment. > > Will do. > > Thanks, > > > Paul Bolle >
On Sat, Jun 20, 2015 at 11:34 PM, Paul Gortmaker <paul.gortmaker@windriver.com> wrote: > [Re: [Intel-gfx] [PATCH 6/8] drivers/pwm: Add Crystalcove (CRC) PWM driver] On 20/06/2015 (Sat 13:23) Paul Bolle wrote: > >> [Added Paul Gortmaker.] >> >> Hi Shobhit, >> >> On Fri, 2015-06-19 at 12:16 +0530, Shobhit Kumar wrote: >> > So what is the exact big problem with this ? >> >> The main problem I have is that it's hard to read a submitter's mind. >> And, I think, in cases like this we need to know if the submitter just >> made some silly mistake or that the mismatch (between Kconfig type and >> code) was intentional. So each time such a mismatch is spotted the >> submitter ought to be asked about it. >> >> (I'd guess that one or two new drivers are submitted _each_ day. And >> these mismatches are quite common. I'd say I receive answers like: >> - "Oops, yes bool should have been tristate"; or >> - "Oops, forgot to clean up after noticing tristate didn't work"; or >> - "I just copy-and-pasted a similar driver, the module stuff isn't >> actually needed" >> at least once a week. Not sure, I don't keep track of this stuff.) >> >> Furthermore, it appears that Paul Gortmaker is on a mission to, badly >> summarized, untangle the module and init code. See for instance >> https://lkml.org/lkml/2015/5/28/809 and >> https://lkml.org/lkml/2015/5/31/205 . >> >> Now, I don't know whether (other) Paul is bothered by these MODULE_* >> macros. But Paul did submit a series that adds > > Yes, I agree that it would be nice to not see these mismatches, > regardless of whether we can get away with it or not. > >> builtin_platform_driver(), see https://lkml.org/lkml/2015/5/10/131 . >> That new macro ensures built-in only code doesn't have to use >> module_platform_driver(), which your patch also uses. So perhaps Paul >> can explain some of the non-obvious issues caused by built-in only code >> using module specific constructs. > > In https://lkml.org/lkml/2015/5/10/125 I'd written: > > There are several downsides to this: > 1) The code can appear modular to a reader of the code, and they > won't know if the code really is modular without checking the > Makefile and Kconfig to see if compilation is governed by a > bool or tristate. > 2) Coders of drivers may be tempted to code up an __exit function > that is never used, just in order to satisfy the required three > args of the modular registration function. > 3) Non-modular code ends up including the <module.h> which increases > CPP overhead that they don't need. > 4) It hinders us from performing better separation of the module > init code and the generic init code. > Okay. Get the idea and the need in terms of clear separation. Its just that there are quite a few built-in drivers using module initialization that I assumed its okay. > The nature of linux means that thousands of developers are reading the > code every day -- so I think that there is a genuine value in having the > code convey a clear message on how it was designed to be used. Only > using module related headers/macros for genuinely modular code helps us > (albeit in a small way) towards achieving that. > > Looking at this thread, I see that one of the reasons given for this > code's ambiguous module vs. built-in identity was the observation of a > similar identity crisis of the related INTEL_SOC_PMIC code. Does that > not back up the point above about the value in having the code speak for > itself? So IMHO we probably should clarify the PMIC code vs. adding > another example that looks just like it. > Okay agree. I think there are quite of them lurking in the sources which would need correction. For this PWM driver I will take care as suggested. Regards Shobhit > Paul. > -- > >> >> > I can anyway shove out these macros to end the discussion. >> >> I'd rather convince you than annoy you into doing as I suggested. >> >> > BTW whether you buy the argument or not, please do treat yourself >> > with ice cream for being able to make such a comment. >> >> Will do. >> >> Thanks, >> >> >> Paul Bolle >>
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig index b1541f4..954da3e 100644 --- a/drivers/pwm/Kconfig +++ b/drivers/pwm/Kconfig @@ -183,6 +183,13 @@ config PWM_LPC32XX To compile this driver as a module, choose M here: the module will be called pwm-lpc32xx. +config PWM_CRC + bool "Intel Crystalcove (CRC) PWM support" + depends on X86 && INTEL_SOC_PMIC + help + Generic PWM framework driver for Crystalcove (CRC) PMIC based PWM + control. + config PWM_LPSS tristate "Intel LPSS PWM support" depends on X86 diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile index ec50eb5..3d38fed 100644 --- a/drivers/pwm/Makefile +++ b/drivers/pwm/Makefile @@ -35,3 +35,4 @@ obj-$(CONFIG_PWM_TIPWMSS) += pwm-tipwmss.o obj-$(CONFIG_PWM_TWL) += pwm-twl.o obj-$(CONFIG_PWM_TWL_LED) += pwm-twl-led.o obj-$(CONFIG_PWM_VT8500) += pwm-vt8500.o +obj-$(CONFIG_PWM_CRC) += pwm-crc.o diff --git a/drivers/pwm/pwm-crc.c b/drivers/pwm/pwm-crc.c new file mode 100644 index 0000000..987f3b4 --- /dev/null +++ b/drivers/pwm/pwm-crc.c @@ -0,0 +1,171 @@ +/* + * pwm-crc.c - Intel Crystal Cove PWM Driver + * + * Copyright (C) 2015 Intel Corporation. All rights reserved. + * + * 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 published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * Author: Shobhit Kumar <shobhit.kumar@intel.com> + */ + +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/regmap.h> +#include <linux/mfd/intel_soc_pmic.h> +#include <linux/pwm.h> + +#define PWM0_CLK_DIV 0x4B +#define PWM_OUTPUT_ENABLE (1<<7) +#define PWM_DIV_CLK_0 0x00 /* DIVIDECLK = BASECLK */ +#define PWM_DIV_CLK_100 0x63 /* DIVIDECLK = BASECLK/100 */ +#define PWM_DIV_CLK_128 0x7F /* DIVIDECLK = BASECLK/128 */ + +#define PWM0_DUTY_CYCLE 0x4E +#define BACKLIGHT_EN 0x51 + +#define PWM_MAX_LEVEL 0xFF + +#define PWM_BASE_CLK 6000 /* 6 MHz */ +#define PWM_MAX_PERIOD_NS 21333 /* 46.875KHz */ + +/** + * struct crystalcove_pwm - Crystal Cove PWM controller + * @chip: the abstract pwm_chip structure. + * @regmap: the regmap from the parent device. + */ +struct crystalcove_pwm { + struct pwm_chip chip; + struct platform_device *pdev; + struct regmap *regmap; +}; + +static inline struct crystalcove_pwm *to_crc_pwm(struct pwm_chip *pc) +{ + return container_of(pc, struct crystalcove_pwm, chip); +} + +static int crc_pwm_enable(struct pwm_chip *c, struct pwm_device *pwm) +{ + struct crystalcove_pwm *crc_pwm = to_crc_pwm(c); + + regmap_write(crc_pwm->regmap, BACKLIGHT_EN, 1); + + return 0; +} + +static void crc_pwm_disable(struct pwm_chip *c, struct pwm_device *pwm) +{ + struct crystalcove_pwm *crc_pwm = to_crc_pwm(c); + + regmap_write(crc_pwm->regmap, BACKLIGHT_EN, 0); +} + +static int crc_pwm_config(struct pwm_chip *c, struct pwm_device *pwm, + int duty_ns, int period_ns) +{ + struct crystalcove_pwm *crc_pwm = to_crc_pwm(c); + struct device *dev = &crc_pwm->pdev->dev; + int level, percent; + + if (period_ns > PWM_MAX_PERIOD_NS) { + dev_err(dev, "un-supported period_ns\n"); + return -1; + } + + if (pwm->period != period_ns) { + int clk_div; + + /* changing the clk divisor, need to disable fisrt */ + crc_pwm_disable(c, pwm); + clk_div = PWM_BASE_CLK * period_ns / 1000000; + + regmap_write(crc_pwm->regmap, PWM0_CLK_DIV, + clk_div | PWM_OUTPUT_ENABLE); + + /* enable back */ + crc_pwm_enable(c, pwm); + } + + if (duty_ns > period_ns) { + dev_err(dev, "duty cycle cannot be greater than cycle period\n"); + return -1; + } + + /* change the pwm duty cycle */ + percent = duty_ns * 100 / period_ns; + level = percent * PWM_MAX_LEVEL / 100; + regmap_write(crc_pwm->regmap, PWM0_DUTY_CYCLE, level); + + return 0; +} + +static const struct pwm_ops crc_pwm_ops = { + .config = crc_pwm_config, + .enable = crc_pwm_enable, + .disable = crc_pwm_disable, + .owner = THIS_MODULE, +}; + +static int crystalcove_pwm_probe(struct platform_device *pdev) +{ + struct crystalcove_pwm *pwm; + int retval; + struct device *dev = pdev->dev.parent; + struct intel_soc_pmic *pmic = dev_get_drvdata(dev); + + pwm = devm_kzalloc(&pdev->dev, sizeof(*pwm), GFP_KERNEL); + if (!pwm) + return -ENOMEM; + + pwm->chip.dev = &pdev->dev; + pwm->chip.ops = &crc_pwm_ops; + pwm->chip.base = -1; + pwm->chip.npwm = 1; + + /* get the PMIC regmap */ + pwm->regmap = pmic->regmap; + + retval = pwmchip_add(&pwm->chip); + if (retval < 0) + return retval; + + dev_dbg(&pdev->dev, "crc-pwm probe successful\n"); + platform_set_drvdata(pdev, pwm); + + return 0; +} + +static int crystalcove_pwm_remove(struct platform_device *pdev) +{ + struct crystalcove_pwm *pwm = platform_get_drvdata(pdev); + int retval; + + retval = pwmchip_remove(&pwm->chip); + if (retval < 0) + return retval; + + dev_dbg(&pdev->dev, "crc-pwm driver removed\n"); + + return 0; +} + +static struct platform_driver crystalcove_pwm_driver = { + .probe = crystalcove_pwm_probe, + .remove = crystalcove_pwm_remove, + .driver = { + .name = "crystal_cove_pwm", + }, +}; + +module_platform_driver(crystalcove_pwm_driver); + +MODULE_AUTHOR("Shobhit Kumar <shobhit.kumar@intel.com>"); +MODULE_DESCRIPTION("Intel Crystal Cove PWM Driver"); +MODULE_LICENSE("GPL v2");
The Crystalcove PMIC controls PWM signals and this driver exports that capability as a PWM chip driver. This is platform device implementtaion of the drivers/mfd cell device for CRC PMIC v2: Use the existing config callback with duty_ns and period_ns(Thierry) CC: Samuel Ortiz <sameo@linux.intel.com> Cc: Linus Walleij <linus.walleij@linaro.org> Cc: Alexandre Courbot <gnurou@gmail.com> Cc: Thierry Reding <thierry.reding@gmail.com> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com> --- drivers/pwm/Kconfig | 7 +++ drivers/pwm/Makefile | 1 + drivers/pwm/pwm-crc.c | 171 ++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 179 insertions(+) create mode 100644 drivers/pwm/pwm-crc.c