Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932769AbbFWMty (ORCPT ); Tue, 23 Jun 2015 08:49:54 -0400 Received: from mail-ig0-f172.google.com ([209.85.213.172]:33225 "EHLO mail-ig0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754646AbbFWMtq (ORCPT ); Tue, 23 Jun 2015 08:49:46 -0400 MIME-Version: 1.0 In-Reply-To: <5587EE90.2010105@gmail.com> References: <1434970465-12687-1-git-send-email-shobhit.kumar@intel.com> <1434970465-12687-6-git-send-email-shobhit.kumar@intel.com> <5587EE90.2010105@gmail.com> Date: Tue, 23 Jun 2015 18:19:45 +0530 Message-ID: Subject: Re: [Intel-gfx] [v2 5/7] pwm: crc: Add Crystalcove (CRC) PWM driver From: Shobhit Kumar To: Varka Bhadram Cc: Shobhit Kumar , linux-pwm , intel-gfx , linux-kernel , dri-devel , linux-gpio , Alexandre Courbot , Paul Bolle , Samuel Ortiz , Povilas Staniulis , Jani Nikula , Linus Walleij , Paul Gortmaker , bloften80@gmail.com, David Airlie , Chih-Wei Huang , Thierry Reding , Daniel Vetter , Lee Jones Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4165 Lines: 152 On Mon, Jun 22, 2015 at 4:46 PM, Varka Bhadram wrote: > 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 6000000 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 ? > Can be done. >> +#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 > > > (...) > > >> +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); > Okay. seems better. >> + 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); Yeah, will update. Regards Shobhit > >> + >> + 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. > > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/