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

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

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5587EE90.2010105@gmail.com>
Date:	Mon, 22 Jun 2015 16:46:32 +0530
From:	Varka Bhadram <varkabhadram@...il.com>
To:	Shobhit Kumar <shobhit.kumar@...el.com>,
	linux-pwm <linux-pwm@...r.kernel.org>,
	intel-gfx <intel-gfx@...ts.freedesktop.org>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	dri-devel <dri-devel@...ts.freedesktop.org>,
	linux-gpio <linux-gpio@...r.kernel.org>
CC:	Paul Bolle <pebolle@...cali.nl>,
	Jani Nikula <jani.nikula@...el.com>,
	Samuel Ortiz <sameo@...ux.intel.com>,
	Alexandre Courbot <gnurou@...il.com>,
	David Airlie <airlied@...ux.ie>,
	Povilas Staniulis <wdmonster@...il.com>,
	Chih-Wei Huang <cwhuang@...roid-x86.org>,
	Thierry Reding <thierry.reding@...il.com>,
	Daniel Vetter <daniel.vetter@...el.com>,
	Lee Jones <lee.jones@...aro.org>,
	Linus Walleij <linus.walleij@...aro.org>,
	Paul Gortmaker <paul.gortmaker@...driver.com>,
	bloften80@...il.com
Subject: Re: [v2 5/7] pwm: crc: Add Crystalcove (CRC) PWM driver

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 <sameo@...ux.intel.com>
> Cc: Linus Walleij <linus.walleij@...aro.org>
> Cc: Alexandre Courbot <gnurou@...il.com>
> Cc: Thierry Reding <thierry.reding@...il.com>
> Cc: Paul Bolle <pebolle@...cali.nl>
> Cc: Paul Gortmaker <paul.gortmaker@...driver.com>
> Signed-off-by: Shobhit Kumar <shobhit.kumar@...el.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_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);

> +	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/