Re: [v2 5/7] pwm: crc: Add Crystalcove (CRC) PWM driver

2015-06-22 Thread Varka Bhadram

Hi Shobhit Kumar,

On 06/22/2015 04:24 PM, Shobhit Kumar wrote:


The Crystalcove PMIC provides three PWM signals and this driver exports
one of them on the BYT platform which is used to control backlight for
DSI panel. This is platform device implementation 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)

v4: Address comment by Thierry & Paul
 - Commit message update and fixes for few syntax errors
 - Add PWM_CRC in Kconfig and Makefile sorted alphabetically
 - Use the PWM_BASE_CLK as 600 for better code readability
 - Remove the redundant rule of three while calculating pwm level
 - Use the platform_device in pwm_chip
 - Use builin_platform_driver

CC: Samuel Ortiz 
Cc: Linus Walleij 
Cc: Alexandre Courbot 
Cc: Thierry Reding 
Cc: Paul Bolle 
Cc: Paul Gortmaker 
Signed-off-by: Shobhit Kumar 


(...)


+
+#include 
+#include 
+#include 
+#include 
+
+#define PWM0_CLK_DIV   0x4B
+#define  PWM_OUTPUT_ENABLE (1 << 7)


Can't be BIT() macro ?


+#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_CYCLE0x4E
+#define BACKLIGHT_EN   0x51


(...)


+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(>dev, sizeof(*pwm), GFP_KERNEL);
+   if (!pwm)
+   return -ENOMEM;
+
+   pwm->chip.dev = >dev;
+   pwm->chip.ops = _pwm_ops;
+   pwm->chip.base = -1;
+   pwm->chip.npwm = 1;
+
+   /* get the PMIC regmap */
+   pwm->regmap = pmic->regmap;
+
+   retval = pwmchip_add(>chip);
+   if (retval < 0)
+   return retval;
+
+   platform_set_drvdata(pdev, pwm);
+


If you can change this oder we can simply do something like this:

platform_set_drvdata(pdev, pwm);

return pwmchip_add(>chip);


+   return 0;
+}
+
+static int crystalcove_pwm_remove(struct platform_device *pdev)
+{
+   struct crystalcove_pwm *pwm = platform_get_drvdata(pdev);
+   int retval;
+
+   retval = pwmchip_remove(>chip);
+   if (retval < 0)
+   return retval;
+
+   dev_dbg(>dev, "crc-pwm driver removed\n");


This debug message may not be required  :-)

you can directly do:

return pwmchip_remove(>chip);


+
+   return 0;
+}
+
+static struct platform_driver crystalcove_pwm_driver = {
+   .probe = crystalcove_pwm_probe,
+   .remove = crystalcove_pwm_remove,
+   .driver = {
+   .name = "crystal_cove_pwm",
+   },
+};
+
+builtin_platform_driver(crystalcove_pwm_driver);



--
Best regards,
Varka Bhadram.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/


Re: [v2 5/7] pwm: crc: Add Crystalcove (CRC) PWM driver

2015-06-22 Thread Varka Bhadram

Hi Shobhit Kumar,

On 06/22/2015 04:24 PM, Shobhit Kumar wrote:


The Crystalcove PMIC provides three PWM signals and this driver exports
one of them on the BYT platform which is used to control backlight for
DSI panel. This is platform device implementation 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)

v4: Address comment by Thierry  Paul
 - Commit message update and fixes for few syntax errors
 - Add PWM_CRC in Kconfig and Makefile sorted alphabetically
 - Use the PWM_BASE_CLK as 600 for better code readability
 - Remove the redundant rule of three while calculating pwm level
 - Use the platform_device in pwm_chip
 - Use builin_platform_driver

CC: Samuel Ortiz sa...@linux.intel.com
Cc: Linus Walleij linus.wall...@linaro.org
Cc: Alexandre Courbot gnu...@gmail.com
Cc: Thierry Reding thierry.red...@gmail.com
Cc: Paul Bolle pebo...@tiscali.nl
Cc: Paul Gortmaker paul.gortma...@windriver.com
Signed-off-by: Shobhit Kumar shobhit.ku...@intel.com


(...)


+
+#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)


Can't be BIT() macro ?


+#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_CYCLE0x4E
+#define BACKLIGHT_EN   0x51


(...)


+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;
+
+   platform_set_drvdata(pdev, pwm);
+


If you can change this oder we can simply do something like this:

platform_set_drvdata(pdev, pwm);

return pwmchip_add(pwm-chip);


+   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);


This debug message may not be required  :-)

you can directly do:

return pwmchip_remove(pwm-chip);


+
+   return 0;
+}
+
+static struct platform_driver crystalcove_pwm_driver = {
+   .probe = crystalcove_pwm_probe,
+   .remove = crystalcove_pwm_remove,
+   .driver = {
+   .name = crystal_cove_pwm,
+   },
+};
+
+builtin_platform_driver(crystalcove_pwm_driver);



--
Best regards,
Varka Bhadram.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
Please read the FAQ at  http://www.tux.org/lkml/